Skip to content

Commit 13d86e7

Browse files
christiangdaclaude
andauthored
fix: harden hardware identifier collection across all platforms (#14)
* chore: ai config * chore: ai config * fix: remove unnecessary workflow * fix: filter OEM placeholder in Windows PowerShell fallback paths 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> * fix: harden Linux CPU and disk serial collection 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> * test: add args-aware overrides to mockExecutor 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> * fix: reject null UUIDs and lock Apple Silicon CPU path on darwin 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> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 79926bf commit 13d86e7

File tree

10 files changed

+763
-251
lines changed

10 files changed

+763
-251
lines changed

.github/copilot-instructions.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ Since this is a library build in native go, the files are mostly organized follo
3333
## Code Style
3434

3535
- Follow Go's idiomatic style defined in
36-
- #fetch https://google.github.io/styleguide/go/guide
37-
- #fetch https://google.github.io/styleguide/go/decisions
38-
- #fetch https://google.github.io/styleguide/go/best-practices
39-
- #fetch https://golang.org/doc/effective_go.html
36+
- <https://google.github.io/styleguide/go/guide>
37+
- <https://google.github.io/styleguide/go/decisions>
38+
- <https://google.github.io/styleguide/go/best-practices>
39+
- <https://golang.org/doc/effective_go.html>
4040
- Use meaningful names for variables, functions, and packages.
4141
- Keep functions small and focused on a single task.
4242
- Use comments to explain complex logic or decisions.

.github/workflows/main.yml

Lines changed: 0 additions & 186 deletions
This file was deleted.

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
.github/copilot-instructions.md

darwin.go

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ var (
1717
ioregSerialRe = regexp.MustCompile(`"IOPlatformSerialNumber"\s*=\s*"([^"]+)"`)
1818
)
1919

20+
// nullUUID is the all-zero UUID that some firmware implementations return
21+
// when no real hardware UUID is programmed. It must be rejected so it
22+
// cannot contribute to the machine ID.
23+
const nullUUID = "00000000-0000-0000-0000-000000000000"
24+
2025
// spHardwareDataType represents the JSON output of `system_profiler SPHardwareDataType -json`.
2126
type spHardwareDataType struct {
2227
SPHardwareDataType []spHardwareEntry `json:"SPHardwareDataType"`
@@ -90,17 +95,22 @@ func collectIdentifiers(ctx context.Context, p *Provider, diag *DiagnosticInfo)
9095
}
9196

9297
// macOSHardwareUUID retrieves hardware UUID using system_profiler with JSON parsing.
98+
// Null UUIDs (all zeros) are rejected so the fallback path is triggered.
9399
func macOSHardwareUUID(ctx context.Context, executor CommandExecutor, logger *slog.Logger) (string, error) {
94100
output, err := executeCommand(ctx, executor, logger, "system_profiler", "SPHardwareDataType", "-json")
95101
if err == nil {
96102
uuid, parseErr := extractHardwareField(output, func(e spHardwareEntry) string {
97103
return e.PlatformUUID
98104
})
99105
if parseErr == nil {
100-
return uuid, nil
101-
}
102-
103-
if logger != nil {
106+
if uuid == nullUUID {
107+
if logger != nil {
108+
logger.Debug("system_profiler returned null UUID, falling back")
109+
}
110+
} else {
111+
return uuid, nil
112+
}
113+
} else if logger != nil {
104114
logger.Debug("system_profiler UUID parsing failed", "error", parseErr)
105115
}
106116
}
@@ -114,6 +124,7 @@ func macOSHardwareUUID(ctx context.Context, executor CommandExecutor, logger *sl
114124
}
115125

116126
// macOSHardwareUUIDViaIOReg retrieves hardware UUID using ioreg as fallback.
127+
// Null UUIDs (all zeros) are rejected with ErrNotFound.
117128
func macOSHardwareUUIDViaIOReg(ctx context.Context, executor CommandExecutor, logger *slog.Logger) (string, error) {
118129
output, err := executeCommand(ctx, executor, logger, "ioreg", "-d2", "-c", "IOPlatformExpertDevice")
119130
if err != nil {
@@ -122,6 +133,14 @@ func macOSHardwareUUIDViaIOReg(ctx context.Context, executor CommandExecutor, lo
122133

123134
match := ioregUUIDRe.FindStringSubmatch(output)
124135
if len(match) > 1 {
136+
if match[1] == nullUUID {
137+
if logger != nil {
138+
logger.Debug("ioreg returned null UUID")
139+
}
140+
141+
return "", &ParseError{Source: "ioreg output", Err: ErrNotFound}
142+
}
143+
125144
return match[1], nil
126145
}
127146

@@ -182,6 +201,13 @@ func macOSSerialNumberViaIOReg(ctx context.Context, executor CommandExecutor, lo
182201
// producing "ChipType:" — this trailing colon is preserved for backward
183202
// compatibility with existing license activations.
184203
// Falls back to system_profiler chip_type only if sysctl fails entirely.
204+
//
205+
// Known quirk: if machdep.cpu.brand_string succeeds but machdep.cpu.features
206+
// errors (e.g. transient syscall failure under sandboxing), the result
207+
// degrades to just cpuBrand instead of "cpuBrand:features". This produces a
208+
// different hash for the same machine across calls. The divergence is
209+
// preserved intentionally — changing it would invalidate every existing
210+
// license activation generated under the current behavior.
185211
func macOSCPUInfo(ctx context.Context, executor CommandExecutor, logger *slog.Logger) (string, error) {
186212
// Primary: sysctl (backward compatible)
187213
output, err := executeCommand(ctx, executor, logger, "sysctl", "-n", "machdep.cpu.brand_string")

0 commit comments

Comments
 (0)