Skip to content

fix: harden hardware identifier collection across all platforms#14

Merged
christiangda merged 7 commits intomainfrom
chore/clean-code
Apr 11, 2026
Merged

fix: harden hardware identifier collection across all platforms#14
christiangda merged 7 commits intomainfrom
chore/clean-code

Conversation

@christiangda
Copy link
Copy Markdown
Contributor

@christiangda christiangda commented Apr 11, 2026

Summary

Audits and bug fixes for the Windows, Linux, and Darwin hardware collectors, plus a test-only mock enhancement and earlier clean-code chores. Every fix aligns runtime behavior with what the documentation already promises — no public API changes.

Platform fixes

Windows (aa9eada)

The OEM BIOS placeholder "To be filled by O.E.M." was correctly filtered on the wmic paths but leaked through every PowerShell fallback for CPU, system UUID, and disk serials — only motherboard serial handled it. Centralize filtering in parsePowerShellValue / parsePowerShellMultipleValues so every PowerShell-backed collector rejects the placeholder consistently with wmic. ErrOEMPlaceholder was already documented as a sentinel error — the fix makes it actually fire.

Linux (8c823b1)

Three correctness bugs:

  1. parseCPUInfo returned ":::" when /proc/cpuinfo was empty or malformed, silently contributing a fixed string to the machine ID and reducing entropy. Now returns ErrNotFound so the caller records a proper component error.
  2. linuxDiskSerials* did not filter the OEM placeholder, unlike the motherboard path (which uses isValidSerial). All disk serial candidates now route through isValidSerial.
  3. linuxDiskSerials returned (nil, nil) when both lsblk and /sys/block failed — "no disks on this system" was indistinguishable from "collection is broken." Now returns ErrNotFound only when both backends errored AND no valid serial was produced.

Also parameterizes sysBlockDir via a package-private variable so linuxDiskSerialsSys can be tested against a fake /sys/block tree built with t.TempDir().

Darwin (cb1240a)

macOSHardwareUUID accepted the null UUID "00000000-0000-0000-0000-000000000000" from either system_profiler or ioreg, letting it contribute to the machine ID on hardware where firmware had not programmed a real one. Both paths now reject null UUIDs (fallback → ioreg on the first, ParseError/ErrNotFound on the second). A new doc comment on macOSCPUInfo documents the intentional Apple-Silicon divergence quirk (brand_string OK + features error → degraded cpuBrand result), preserved for backward compatibility with existing license activations.

Test infrastructure (2dd5e80)

mockExecutor previously keyed outputs/errors by command name only, making it impossible to test code that invokes the same command with different arguments — which is exactly what macOSCPUInfo does (sysctl -n machdep.cpu.brand_string + sysctl -n machdep.cpu.features). Added setOutputForArgs / setErrorForArgs that take precedence over the name-only maps. Existing setOutput / setError behavior is unchanged so no existing tests needed updating. This unlocked the Apple Silicon exact-output test ("Apple M1 Pro:" with the trailing colon preserved).

Chore work (carried from earlier commits on the branch)

  • bc5bb2f — remove unnecessary workflow
  • edf1d0e, 2ed0434 — AI config updates

Test coverage added

~32 new tests across the three platforms:

  • Windows (7): OEM placeholder rejection in parsePowerShellValue / parsePowerShellMultipleValues, CPU PowerShell OEM + empty paths, UUID PowerShell OEM path, disk serials filtering OEM + all-OEM failure.
  • Linux (~15): parseCPUInfo empty / no-fields / unknown-fields-only, lsblk OEM filter, linuxDiskSerialsSys direct tests (success, loop-device skip, OEM+empty filter, missing dir), both-backends-fail → ErrNotFound, partial success, cross-backend deduplication, cross-backend OEM filter, readFirstValidFromLocations success path via tempfile.
  • Darwin (8): null UUID rejection from system_profiler / ioreg / both sources, Apple Silicon trailing colon, Intel brand+features, brand-OK features-error degradation, parseStorageJSON missing-internal-field edge case.
  • Mock (2): args-aware override precedence and args-aware error precedence.

Verification

  • make test (darwin) — pass
  • GOOS=linux go vet ./... + GOOS=linux go test -c — pass (could not execute on darwin host; CI will validate runtime)
  • GOOS=windows go vet ./... + GOOS=windows go test -c — pass
  • golangci-lint run ./... — 0 issues

Test plan

  • CI green on Linux runner (validates the t.TempDir()-based linuxDiskSerialsSys tests and the parseCPUInfo signature change)
  • CI green on Windows runner (validates the new OEM-filter tests)
  • CI green on macOS runner (validates Apple Silicon CPU test and null UUID rejection)
  • Spot-check Diagnostics() on a real machine now reports ErrOEMPlaceholder / ErrNotFound where it previously accepted garbage

🤖 Generated with Claude Code

christiangda and others added 7 commits April 11, 2026 16:08
PowerShell fallbacks for CPU, system UUID, and disk serials were returning
"To be filled by O.E.M." as a valid identifier — only motherboard serial
handled it. Centralize filtering in parsePowerShellValue /
parsePowerShellMultipleValues so every PowerShell-backed collector rejects
OEM placeholders consistently with the wmic path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three correctness bugs in the Linux collector:

1. parseCPUInfo returned ":::" when /proc/cpuinfo was empty or contained
   no recognized fields, silently contributing a fixed string to the
   machine ID and reducing entropy. Now returns ErrNotFound so the
   caller records a proper component error.

2. linuxDiskSerials, linuxDiskSerialsLSBLK, and linuxDiskSerialsSys did
   not filter the BIOS "To be filled by O.E.M." placeholder, unlike the
   motherboard path which uses isValidSerial. Route all disk serial
   candidates through isValidSerial so the placeholder is rejected
   consistently.

3. linuxDiskSerials returned (nil, nil) when both lsblk and /sys/block
   failed, making "no disks on this system" indistinguishable from
   "collection is broken." It now returns ErrNotFound only when both
   backends errored AND no valid serial was produced.

Also parameterize sysBlockDir via a package-private variable so
linuxDiskSerialsSys can be tested against a fake /sys/block tree built
with t.TempDir().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The existing mock keys registered outputs and errors by command name
only, which made it impossible to test code that invokes the same
command with different arguments (e.g. two sysctl subcommands on
macOS). Add setOutputForArgs / setErrorForArgs that take precedence
over the name-only maps so such flows can be exercised deterministically.
Existing setOutput / setError behavior is unchanged.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
macOSHardwareUUID accepted "00000000-0000-0000-0000-000000000000" from
either system_profiler or ioreg, letting a null UUID contribute to the
machine ID on hardware where firmware hasn't programmed a real one.
Reject the null UUID in both the system_profiler path (falling back to
ioreg) and the ioreg path (returning ParseError/ErrNotFound).

Also add Apple Silicon and Intel CPU tests that use the new args-aware
mock executor to exercise the exact production paths that were
previously impossible to test:

  - brand_string = "Apple M1 Pro", features = "" → "Apple M1 Pro:"
    (trailing colon preserved for license activation compatibility).
  - brand_string + non-empty features → "brand:features".
  - brand_string OK but features errors → degraded "brand" result.

The degraded path is intentionally preserved to avoid invalidating
existing license activations; a new doc comment on macOSCPUInfo
explains the divergence, and TestMacOSCPUInfoBrandOKFeaturesErrorDegrades
locks the behavior in so any future change fails loudly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@christiangda christiangda self-assigned this Apr 11, 2026
@christiangda christiangda merged commit 13d86e7 into main Apr 11, 2026
8 checks passed
@christiangda christiangda deleted the chore/clean-code branch April 11, 2026 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant