From 2ed0434661b997e0cafa4bb1fa7705131da51dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:08:36 +0200 Subject: [PATCH 1/7] chore: ai config --- .github/copilot-instructions.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 8cf9b7a..7bd7d12 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -33,10 +33,10 @@ Since this is a library build in native go, the files are mostly organized follo ## Code Style - Follow Go's idiomatic style defined in - - #fetch https://google.github.io/styleguide/go/guide - - #fetch https://google.github.io/styleguide/go/decisions - - #fetch https://google.github.io/styleguide/go/best-practices - - #fetch https://golang.org/doc/effective_go.html + - + - + - + - - Use meaningful names for variables, functions, and packages. - Keep functions small and focused on a single task. - Use comments to explain complex logic or decisions. From edf1d0e5cbcb131bfa67be2d285a85fe52fc8f2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:08:59 +0200 Subject: [PATCH 2/7] chore: ai config --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) create mode 120000 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 0000000..02dd134 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +.github/copilot-instructions.md \ No newline at end of file From bc5bb2f6e6640af6411b1ce1597509e5348a3602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:11:19 +0200 Subject: [PATCH 3/7] fix: remove unnecessary workflow --- .github/workflows/main.yml | 186 ------------------------------------- 1 file changed, 186 deletions(-) delete mode 100644 .github/workflows/main.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml deleted file mode 100644 index e76a111..0000000 --- a/.github/workflows/main.yml +++ /dev/null @@ -1,186 +0,0 @@ -name: Main - -on: - push: - branches: - - main - -permissions: - contents: read - -jobs: - test: - name: Test (${{ matrix.os }}) - runs-on: ${{ matrix.os }} - strategy: - fail-fast: false - matrix: - os: [ubuntu-latest, macos-latest, windows-latest] - steps: - - name: Check out code - uses: actions/checkout@v6 - - - name: Set up Go - uses: actions/setup-go@v6 - with: - go-version-file: ./go.mod - - - name: Test - run: go test -v -race -tags=unit ./... - - - name: Build CLI - run: go build -v -o ${{ runner.temp }}/machineid${{ matrix.os == 'windows-latest' && '.exe' || '' }} ./cmd/machineid/ - - - name: Generate machine ID (smoke test) - shell: bash - run: | - BIN="${{ runner.temp }}/machineid${{ matrix.os == 'windows-latest' && '.exe' || '' }}" - - echo "--- Version ---" - "$BIN" -version - - echo "--- Default ID (CPU + motherboard + UUID) ---" - ID=$("$BIN") - echo "ID: $ID" - echo "Length: ${#ID}" - - # Verify the ID is a 64-char hex string - if [[ ${#ID} -ne 64 ]]; then - echo "ERROR: expected 64-char ID, got ${#ID}" - exit 1 - fi - if [[ ! "$ID" =~ ^[0-9a-f]{64}$ ]]; then - echo "ERROR: ID is not a valid hex string" - exit 1 - fi - - echo "--- Validate round-trip ---" - "$BIN" -validate "$ID" - - # Exercise additional CLI modes. - # On Windows CI, WMI can be transiently unavailable after repeated - # invocations, so these are non-fatal — the core validation above - # already proved the binary works. - ERRORS=0 - - echo "--- All components (JSON + diagnostics) ---" - "$BIN" -all -json -diagnostics || { echo "WARN: -all failed (non-fatal)"; ERRORS=$((ERRORS+1)); } - - echo "--- VM-friendly ---" - "$BIN" -vm || { echo "WARN: -vm failed (non-fatal)"; ERRORS=$((ERRORS+1)); } - - echo "--- Format 32 ---" - "$BIN" -format 32 || { echo "WARN: -format 32 failed (non-fatal)"; ERRORS=$((ERRORS+1)); } - - echo "--- Salt ---" - "$BIN" -salt "ci-test" || { echo "WARN: -salt failed (non-fatal)"; ERRORS=$((ERRORS+1)); } - - if [[ $ERRORS -gt 0 ]]; then - echo "WARN: $ERRORS non-critical exercise(s) failed on ${{ matrix.os }} (WMI transient issue)" - fi - - echo "Smoke test passed on ${{ matrix.os }}" - - report: - name: Report & Build - needs: test - runs-on: ubuntu-latest - env: - MAKE_DEBUG: true - steps: - - name: Check out code - uses: actions/checkout@v6 - - - name: Set up Go - uses: actions/setup-go@v6 - with: - go-version-file: ./go.mod - - - name: Summary Information - env: - COMMIT_MESSAGE: ${{ github.event.head_commit.message }} - COMMIT_AUTHOR: ${{ github.event.head_commit.author.name }} - run: | - echo "# Push Summary" > "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" - echo "**Repository:** ${{ github.repository }}" >> "$GITHUB_STEP_SUMMARY" - echo "**Push:** $COMMIT_MESSAGE" >> "$GITHUB_STEP_SUMMARY" - echo "**Author:** $COMMIT_AUTHOR" >> "$GITHUB_STEP_SUMMARY" - echo "**Branch:** ${{ github.ref }}" >> "$GITHUB_STEP_SUMMARY" - echo "" >> "$GITHUB_STEP_SUMMARY" - - - name: Tools and versions - run: | - echo "## Tools and versions" >> $GITHUB_STEP_SUMMARY - - ubuntu_version=$(lsb_release -a 2>&1 | grep "Description" | awk '{print $2, $3, $4}') - echo "Ubuntu version: $ubuntu_version" - echo "**Ubuntu Version:** $ubuntu_version" >> $GITHUB_STEP_SUMMARY - - bash_version=$(bash --version | head -n 1 | awk '{print $4}') - echo "Bash version: $bash_version" - echo "**Bash Version:** $bash_version" >> $GITHUB_STEP_SUMMARY - - git_version=$(git --version | awk '{print $3}') - echo "Git version: $git_version" - echo "**Git Version:** $git_version" >> $GITHUB_STEP_SUMMARY - - go_version=$(go version | awk '{print $3}') - echo "Go version: $go_version" - echo "**Go Version:** $go_version" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - - - name: Lines of code - env: - GH_TOKEN: ${{ github.token }} - run: | - export TOOL_NAME="scc" - export GIT_ORG="boyter" - export GIT_REPO="scc" - export OS=$(uname -s) - export OS_ARCH=$(uname -m) - # Normalize architecture names to match asset naming - [[ "$OS_ARCH" == "aarch64" ]] && OS_ARCH="arm64" - [[ "$OS_ARCH" == "x86_64" ]] && OS_ARCH="x86_64" - export ASSETS_NAME=$(gh release view --repo ${GIT_ORG}/${GIT_REPO} --json assets -q "[.assets[] | select(.name | contains(\"${TOOL_NAME}\") and contains(\"${OS}\") and contains(\"${OS_ARCH}\"))] | sort_by(.createdAt) | last.name") - - gh release download --repo $GIT_ORG/$GIT_REPO --pattern $ASSETS_NAME - - # Extract based on file extension - if [[ "$ASSETS_NAME" == *.tar.gz ]]; then - tar -xzf $ASSETS_NAME - elif [[ "$ASSETS_NAME" == *.zip ]]; then - unzip $ASSETS_NAME - fi - - rm $ASSETS_NAME - - mv $TOOL_NAME ~/go/bin/$TOOL_NAME - ~/go/bin/$TOOL_NAME --version - - scc --format html-table . | tee -a $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - - - name: Test Coverage - run: | - set -o pipefail - - echo "## Test Coverage" >> $GITHUB_STEP_SUMMARY - - make test 2>&1 | tee -a $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - - go install github.com/vladopajic/go-test-coverage/v2@latest - - echo "" >> $GITHUB_STEP_SUMMARY - echo "### Coverage report" >> $GITHUB_STEP_SUMMARY - go-test-coverage --config=./.testcoverage.yml | sed 's/PASS/PASS ✅/g' | sed 's/FAIL/FAIL ❌/g' | tee -a $GITHUB_STEP_SUMMARY - - - name: Build - run: | - echo "## Build" >> $GITHUB_STEP_SUMMARY - - make build 2>&1 | tee -a $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "Build completed successfully." >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY From aa9eada16063c11fa40a9301430417adb8c17e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:25:49 +0200 Subject: [PATCH 4/7] fix: filter OEM placeholder in Windows PowerShell fallback paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- windows.go | 21 ++++------ windows_test.go | 107 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 12 deletions(-) diff --git a/windows.go b/windows.go index d8571d0..041280d 100644 --- a/windows.go +++ b/windows.go @@ -205,25 +205,31 @@ func parseWmicMultipleValues(output, prefix string) []string { } // parsePowerShellValue extracts a trimmed, non-empty value from PowerShell output. +// OEM placeholder strings (see biosFirmwareMessage) are rejected with ErrOEMPlaceholder. func parsePowerShellValue(output string) (string, error) { value := strings.TrimSpace(output) if value == "" { return "", &ParseError{Source: "PowerShell output", Err: ErrEmptyValue} } + if value == biosFirmwareMessage { + return "", &ParseError{Source: "PowerShell output", Err: ErrOEMPlaceholder} + } return value, nil } // parsePowerShellMultipleValues extracts multiple trimmed, non-empty values from PowerShell output. +// OEM placeholder lines (see biosFirmwareMessage) are filtered out. func parsePowerShellMultipleValues(output string) []string { var values []string lines := strings.SplitSeq(output, "\n") for line := range lines { value := strings.TrimSpace(line) - if value != "" { - values = append(values, value) + if value == "" || value == biosFirmwareMessage { + continue } + values = append(values, value) } return values @@ -284,16 +290,7 @@ func windowsMotherboardSerial(ctx context.Context, executor CommandExecutor, log return "", ErrAllMethodsFailed } - value, parseErr := parsePowerShellValue(psOutput) - if parseErr != nil { - return "", parseErr - } - - if value == biosFirmwareMessage { - return "", &ParseError{Source: "PowerShell output", Err: ErrOEMPlaceholder} - } - - return value, nil + return parsePowerShellValue(psOutput) } // windowsSystemUUID retrieves system UUID using wmic or PowerShell. diff --git a/windows_test.go b/windows_test.go index 0f56702..531cb1e 100644 --- a/windows_test.go +++ b/windows_test.go @@ -139,6 +139,20 @@ func TestParsePowerShellValueEmpty(t *testing.T) { } } +func TestParsePowerShellValueOEMPlaceholder(t *testing.T) { + _, err := parsePowerShellValue(" To be filled by O.E.M. ") + if err == nil { + t.Fatal("Expected error for OEM placeholder value") + } + var parseErr *ParseError + if !errors.As(err, &parseErr) { + t.Errorf("Expected ParseError, got %T", err) + } + if !errors.Is(err, ErrOEMPlaceholder) { + t.Errorf("Expected ErrOEMPlaceholder, got %v", err) + } +} + // --- parsePowerShellMultipleValues tests --- func TestParsePowerShellMultipleValuesValid(t *testing.T) { @@ -165,6 +179,25 @@ func TestParsePowerShellMultipleValuesAllEmpty(t *testing.T) { } } +func TestParsePowerShellMultipleValuesFiltersOEM(t *testing.T) { + output := "WD-12345\nTo be filled by O.E.M.\nWD-67890\n" + values := parsePowerShellMultipleValues(output) + if len(values) != 2 { + t.Fatalf("Expected 2 values (OEM filtered), got %d: %v", len(values), values) + } + if values[0] != "WD-12345" || values[1] != "WD-67890" { + t.Errorf("Unexpected values: %v", values) + } +} + +func TestParsePowerShellMultipleValuesAllOEM(t *testing.T) { + output := "To be filled by O.E.M.\nTo be filled by O.E.M.\n" + values := parsePowerShellMultipleValues(output) + if len(values) != 0 { + t.Errorf("Expected 0 values when all OEM, got %d", len(values)) + } +} + // --- windowsCPUID tests --- func TestWindowsCPUIDWmicSuccess(t *testing.T) { @@ -222,6 +255,34 @@ func TestWindowsCPUIDWmicParseFailFallback(t *testing.T) { } } +func TestWindowsCPUIDPowerShellOEMPlaceholder(t *testing.T) { + mock := newMockExecutor() + mock.setError("wmic", fmt.Errorf("wmic not found")) + mock.setOutput("powershell", "To be filled by O.E.M.") + + _, err := windowsCPUID(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error for OEM placeholder from PowerShell fallback") + } + if !errors.Is(err, ErrOEMPlaceholder) { + t.Errorf("Expected ErrOEMPlaceholder, got %v", err) + } +} + +func TestWindowsCPUIDPowerShellEmpty(t *testing.T) { + mock := newMockExecutor() + mock.setError("wmic", fmt.Errorf("wmic not found")) + mock.setOutput("powershell", " ") + + _, err := windowsCPUID(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error for empty PowerShell output") + } + if !errors.Is(err, ErrEmptyValue) { + t.Errorf("Expected ErrEmptyValue, got %v", err) + } +} + func TestWindowsCPUIDWithLogger(t *testing.T) { var buf bytes.Buffer logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})) @@ -427,6 +488,19 @@ func TestWindowsSystemUUIDViaPowerShellEmpty(t *testing.T) { } } +func TestWindowsSystemUUIDViaPowerShellOEMPlaceholder(t *testing.T) { + mock := newMockExecutor() + mock.setOutput("powershell", "To be filled by O.E.M.") + + _, err := windowsSystemUUIDViaPowerShell(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error for OEM placeholder from PowerShell") + } + if !errors.Is(err, ErrOEMPlaceholder) { + t.Errorf("Expected ErrOEMPlaceholder, got %v", err) + } +} + // --- windowsDiskSerials tests --- func TestWindowsDiskSerialsWmicSuccess(t *testing.T) { @@ -487,6 +561,39 @@ func TestWindowsDiskSerialsPowerShellEmpty(t *testing.T) { } } +func TestWindowsDiskSerialsPowerShellFiltersOEM(t *testing.T) { + mock := newMockExecutor() + mock.setError("wmic", fmt.Errorf("wmic not found")) + mock.setOutput("powershell", "WD-12345\nTo be filled by O.E.M.\nWD-67890\n") + + result, err := windowsDiskSerials(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(result) != 2 { + t.Fatalf("Expected 2 serials (OEM filtered), got %d: %v", len(result), result) + } + for _, s := range result { + if s == "To be filled by O.E.M." { + t.Error("OEM placeholder leaked into disk serials") + } + } +} + +func TestWindowsDiskSerialsPowerShellAllOEM(t *testing.T) { + mock := newMockExecutor() + mock.setError("wmic", fmt.Errorf("wmic not found")) + mock.setOutput("powershell", "To be filled by O.E.M.\nTo be filled by O.E.M.\n") + + _, err := windowsDiskSerials(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error when PowerShell returns only OEM placeholders") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) + } +} + func TestWindowsDiskSerialsWmicEmptyFallback(t *testing.T) { mock := newMockExecutor() mock.setOutput("wmic", "SerialNumber=\r\n") // wmic returns empty serials From 8c823b1276f71a9460d557774878dd884fa96eaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:30:09 +0200 Subject: [PATCH 5/7] 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) --- linux.go | 93 +++++++++++------ linux_test.go | 274 ++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 325 insertions(+), 42 deletions(-) diff --git a/linux.go b/linux.go index 9ff49a5..1198c00 100644 --- a/linux.go +++ b/linux.go @@ -69,11 +69,14 @@ func linuxCPUID(logger *slog.Logger) (string, error) { logger.Debug("read CPU info", "path", path) } - return parseCPUInfo(string(data)), nil + return parseCPUInfo(string(data)) } // parseCPUInfo extracts CPU information from /proc/cpuinfo content. -func parseCPUInfo(content string) string { +// Returns ErrNotFound when none of the expected fields are present, so an +// empty or malformed /proc/cpuinfo does not silently contribute a fixed +// all-colons string to the machine ID. +func parseCPUInfo(content string) (string, error) { lines := strings.Split(content, "\n") var processor, vendorID, modelName, flags string @@ -96,8 +99,11 @@ func parseCPUInfo(content string) string { } } - // Combine CPU information for unique identifier - return fmt.Sprintf("%s:%s:%s:%s", processor, vendorID, modelName, flags) + if processor == "" && vendorID == "" && modelName == "" && flags == "" { + return "", &ParseError{Source: "/proc/cpuinfo", Err: ErrNotFound} + } + + return fmt.Sprintf("%s:%s:%s:%s", processor, vendorID, modelName, flags), nil } // linuxSystemUUID retrieves system UUID from DMI. @@ -171,49 +177,72 @@ func isNonEmpty(value string) bool { return value != "" } +// sysBlockDir is the root directory scanned by linuxDiskSerialsSys. +// It is a variable so tests can inject a fake /sys/block tree. +var sysBlockDir = "/sys/block" + // linuxDiskSerials retrieves disk serial numbers using various methods. // Results are deduplicated across sources to prevent the same serial -// from appearing multiple times. +// from appearing multiple times. OEM placeholder strings (see +// biosFirmwareMessage) are filtered out. +// Returns ErrNotFound when both backends fail and no valid serial was found. func linuxDiskSerials(ctx context.Context, executor CommandExecutor, logger *slog.Logger) ([]string, error) { seen := make(map[string]struct{}) var serials []string - // Try using lsblk command first - if lsblkSerials, err := linuxDiskSerialsLSBLK(ctx, executor, logger); err == nil { - for _, s := range lsblkSerials { - if _, exists := seen[s]; !exists { - seen[s] = struct{}{} - serials = append(serials, s) + appendValid := func(src []string) { + for _, s := range src { + if !isValidSerial(s) { + continue + } + if _, exists := seen[s]; exists { + continue } + seen[s] = struct{}{} + serials = append(serials, s) } + } + + lsblkErr := error(nil) + sysErr := error(nil) + + // Try using lsblk command first + if lsblkSerials, err := linuxDiskSerialsLSBLK(ctx, executor, logger); err == nil { + appendValid(lsblkSerials) if logger != nil { logger.Debug("collected disk serials via lsblk", "count", len(lsblkSerials)) } - } else if logger != nil { - logger.Debug("lsblk failed, trying /sys/block", "error", err) + } else { + lsblkErr = err + if logger != nil { + logger.Debug("lsblk failed, trying /sys/block", "error", err) + } } // Try reading from /sys/block if sysSerials, err := linuxDiskSerialsSys(logger); err == nil { - for _, s := range sysSerials { - if _, exists := seen[s]; !exists { - seen[s] = struct{}{} - serials = append(serials, s) - } - } + appendValid(sysSerials) if logger != nil { logger.Debug("collected disk serials via /sys/block", "count", len(sysSerials)) } - } else if logger != nil { - logger.Debug("/sys/block read failed", "error", err) + } else { + sysErr = err + if logger != nil { + logger.Debug("/sys/block read failed", "error", err) + } + } + + if len(serials) == 0 && lsblkErr != nil && sysErr != nil { + return nil, ErrNotFound } return serials, nil } // linuxDiskSerialsLSBLK retrieves disk serials using lsblk command. +// OEM placeholder strings are filtered out. func linuxDiskSerialsLSBLK(ctx context.Context, executor CommandExecutor, logger *slog.Logger) ([]string, error) { output, err := executeCommand(ctx, executor, logger, "lsblk", "-d", "-n", "-o", "SERIAL") if err != nil { @@ -224,35 +253,37 @@ func linuxDiskSerialsLSBLK(ctx context.Context, executor CommandExecutor, logger lines := strings.SplitSeq(output, "\n") for line := range lines { serial := strings.TrimSpace(line) - if serial != "" { - serials = append(serials, serial) + if !isValidSerial(serial) { + continue } + serials = append(serials, serial) } return serials, nil } // linuxDiskSerialsSys retrieves disk serials from /sys/block. +// OEM placeholder strings are filtered out. func linuxDiskSerialsSys(logger *slog.Logger) ([]string, error) { var serials []string - blockDir := "/sys/block" - entries, err := os.ReadDir(blockDir) + entries, err := os.ReadDir(sysBlockDir) if err != nil { return nil, err } for _, entry := range entries { if entry.IsDir() && !strings.HasPrefix(entry.Name(), "loop") { - serialFile := filepath.Join(blockDir, entry.Name(), "device", "serial") + serialFile := filepath.Join(sysBlockDir, entry.Name(), "device", "serial") if data, err := os.ReadFile(serialFile); err == nil { serial := strings.TrimSpace(string(data)) - if serial != "" { - serials = append(serials, serial) + if !isValidSerial(serial) { + continue + } + serials = append(serials, serial) - if logger != nil { - logger.Debug("read disk serial from sysfs", "disk", entry.Name(), "path", serialFile) - } + if logger != nil { + logger.Debug("read disk serial from sysfs", "disk", entry.Name(), "path", serialFile) } } } diff --git a/linux_test.go b/linux_test.go index 2180cf5..81593c3 100644 --- a/linux_test.go +++ b/linux_test.go @@ -8,6 +8,8 @@ import ( "errors" "fmt" "log/slog" + "os" + "path/filepath" "testing" ) @@ -23,7 +25,10 @@ stepping : 10 cpu MHz : 2600.000 flags : fpu vme de pse tsc msr pae mce ` - result := parseCPUInfo(content) + result, err := parseCPUInfo(content) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } expected := "0:GenuineIntel:Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz:fpu vme de pse tsc msr pae mce" if result != expected { t.Errorf("Expected %q, got %q", expected, result) @@ -31,10 +36,16 @@ flags : fpu vme de pse tsc msr pae mce } func TestParseCPUInfoEmpty(t *testing.T) { - result := parseCPUInfo("") - expected := ":::" - if result != expected { - t.Errorf("Expected %q for empty input, got %q", expected, result) + _, err := parseCPUInfo("") + if err == nil { + t.Fatal("Expected error for empty input") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) + } + var parseErr *ParseError + if !errors.As(err, &parseErr) { + t.Errorf("Expected ParseError, got %T", err) } } @@ -43,7 +54,10 @@ func TestParseCPUInfoPartial(t *testing.T) { vendor_id : AuthenticAMD model name : AMD Ryzen 9 5950X ` - result := parseCPUInfo(content) + result, err := parseCPUInfo(content) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } expected := "3:AuthenticAMD:AMD Ryzen 9 5950X:" if result != expected { t.Errorf("Expected %q, got %q", expected, result) @@ -52,10 +66,24 @@ model name : AMD Ryzen 9 5950X func TestParseCPUInfoNoColon(t *testing.T) { content := "some line without colon\nanother line\n" - result := parseCPUInfo(content) - expected := ":::" - if result != expected { - t.Errorf("Expected %q, got %q", expected, result) + _, err := parseCPUInfo(content) + if err == nil { + t.Fatal("Expected error when no fields present") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) + } +} + +func TestParseCPUInfoUnknownFieldsOnly(t *testing.T) { + // Lines have colons but none are recognized fields — should still error. + content := "some unknown key : value\nanother : thing\n" + _, err := parseCPUInfo(content) + if err == nil { + t.Fatal("Expected error when no recognized fields present") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) } } @@ -71,7 +99,10 @@ vendor_id : GenuineIntel model name : Intel Core i7 flags : fpu vme avx ` - result := parseCPUInfo(content) + result, err := parseCPUInfo(content) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } // Last processor's values should win expected := "1:GenuineIntel:Intel Core i7:fpu vme avx" if result != expected { @@ -190,6 +221,227 @@ func TestLinuxDiskSerialsLSBLKSkipsEmpty(t *testing.T) { } } +func TestLinuxDiskSerialsLSBLKFiltersOEM(t *testing.T) { + mock := newMockExecutor() + mock.setOutput("lsblk", "WD-12345\nTo be filled by O.E.M.\nSAMSUNG-67890\n") + + serials, err := linuxDiskSerialsLSBLK(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(serials) != 2 { + t.Fatalf("Expected 2 serials (OEM filtered), got %d: %v", len(serials), serials) + } + for _, s := range serials { + if s == biosFirmwareMessage { + t.Error("OEM placeholder leaked into lsblk disk serials") + } + } +} + +// --- linuxDiskSerialsSys tests (with injected sysBlockDir) --- + +func withSysBlockDir(t *testing.T, dir string) { + t.Helper() + orig := sysBlockDir + sysBlockDir = dir + t.Cleanup(func() { sysBlockDir = orig }) +} + +func writeFakeDisk(t *testing.T, root, name, serial string) { + t.Helper() + deviceDir := filepath.Join(root, name, "device") + if err := os.MkdirAll(deviceDir, 0o755); err != nil { + t.Fatalf("mkdir %q: %v", deviceDir, err) + } + if err := os.WriteFile(filepath.Join(deviceDir, "serial"), []byte(serial), 0o644); err != nil { + t.Fatalf("write serial %q: %v", name, err) + } +} + +func TestLinuxDiskSerialsSysSuccess(t *testing.T) { + tmp := t.TempDir() + withSysBlockDir(t, tmp) + + writeFakeDisk(t, tmp, "sda", "WD-AAA\n") + writeFakeDisk(t, tmp, "sdb", "SAMSUNG-BBB\n") + + serials, err := linuxDiskSerialsSys(nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(serials) != 2 { + t.Fatalf("Expected 2 serials, got %d: %v", len(serials), serials) + } +} + +func TestLinuxDiskSerialsSysSkipsLoop(t *testing.T) { + tmp := t.TempDir() + withSysBlockDir(t, tmp) + + writeFakeDisk(t, tmp, "sda", "WD-AAA\n") + writeFakeDisk(t, tmp, "loop0", "SHOULD-SKIP\n") + + serials, err := linuxDiskSerialsSys(nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(serials) != 1 || serials[0] != "WD-AAA" { + t.Errorf("Expected [WD-AAA], got %v", serials) + } +} + +func TestLinuxDiskSerialsSysFiltersOEMAndEmpty(t *testing.T) { + tmp := t.TempDir() + withSysBlockDir(t, tmp) + + writeFakeDisk(t, tmp, "sda", "WD-REAL\n") + writeFakeDisk(t, tmp, "sdb", "To be filled by O.E.M.\n") + writeFakeDisk(t, tmp, "sdc", "\n") + + serials, err := linuxDiskSerialsSys(nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(serials) != 1 || serials[0] != "WD-REAL" { + t.Errorf("Expected [WD-REAL], got %v", serials) + } +} + +func TestLinuxDiskSerialsSysMissingDir(t *testing.T) { + withSysBlockDir(t, filepath.Join(t.TempDir(), "does-not-exist")) + + _, err := linuxDiskSerialsSys(nil) + if err == nil { + t.Error("Expected error when sysBlockDir does not exist") + } +} + +// --- linuxDiskSerials end-to-end with both backends failing --- + +func TestLinuxDiskSerialsBothBackendsFail(t *testing.T) { + // lsblk errors, sys dir does not exist → ErrNotFound. + withSysBlockDir(t, filepath.Join(t.TempDir(), "nope")) + mock := newMockExecutor() + mock.setError("lsblk", fmt.Errorf("lsblk not found")) + + _, err := linuxDiskSerials(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error when both backends fail") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) + } +} + +func TestLinuxDiskSerialsPartialSuccess(t *testing.T) { + // lsblk succeeds, /sys/block missing → no error, lsblk results returned. + withSysBlockDir(t, filepath.Join(t.TempDir(), "nope")) + mock := newMockExecutor() + mock.setOutput("lsblk", "SERIAL-X\n") + + serials, err := linuxDiskSerials(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(serials) != 1 || serials[0] != "SERIAL-X" { + t.Errorf("Expected [SERIAL-X], got %v", serials) + } +} + +func TestLinuxDiskSerialsDeduplicatesAcrossBackends(t *testing.T) { + tmp := t.TempDir() + withSysBlockDir(t, tmp) + writeFakeDisk(t, tmp, "sda", "SHARED\n") + writeFakeDisk(t, tmp, "sdb", "ONLY-SYS\n") + + mock := newMockExecutor() + mock.setOutput("lsblk", "SHARED\nONLY-LSBLK\n") + + serials, err := linuxDiskSerials(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + // Expected: SHARED, ONLY-LSBLK, ONLY-SYS — no duplicates. + if len(serials) != 3 { + t.Fatalf("Expected 3 unique serials, got %d: %v", len(serials), serials) + } + count := map[string]int{} + for _, s := range serials { + count[s]++ + } + for k, c := range count { + if c != 1 { + t.Errorf("Serial %q appeared %d times", k, c) + } + } +} + +func TestLinuxDiskSerialsFiltersOEMAcrossBackends(t *testing.T) { + tmp := t.TempDir() + withSysBlockDir(t, tmp) + writeFakeDisk(t, tmp, "sda", "GOOD-SYS\n") + + mock := newMockExecutor() + mock.setOutput("lsblk", "GOOD-LSBLK\nTo be filled by O.E.M.\n") + + serials, err := linuxDiskSerials(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + for _, s := range serials { + if s == biosFirmwareMessage { + t.Error("OEM placeholder leaked through linuxDiskSerials") + } + } + if len(serials) != 2 { + t.Errorf("Expected 2 serials, got %d: %v", len(serials), serials) + } +} + +// --- readFirstValidFromLocations success path --- + +func TestReadFirstValidFromLocationsFindsFile(t *testing.T) { + tmp := t.TempDir() + goodPath := filepath.Join(tmp, "good") + if err := os.WriteFile(goodPath, []byte(" HELLO \n"), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + + locations := []string{ + filepath.Join(tmp, "missing"), + goodPath, + } + + value, err := readFirstValidFromLocations(locations, isNonEmpty, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if value != "HELLO" { + t.Errorf("Expected %q, got %q", "HELLO", value) + } +} + +func TestReadFirstValidFromLocationsSkipsInvalid(t *testing.T) { + tmp := t.TempDir() + invalidPath := filepath.Join(tmp, "invalid") + goodPath := filepath.Join(tmp, "good") + if err := os.WriteFile(invalidPath, []byte("00000000-0000-0000-0000-000000000000\n"), 0o644); err != nil { + t.Fatalf("write invalid: %v", err) + } + if err := os.WriteFile(goodPath, []byte("real-uuid\n"), 0o644); err != nil { + t.Fatalf("write good: %v", err) + } + + value, err := readFirstValidFromLocations([]string{invalidPath, goodPath}, isValidUUID, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if value != "real-uuid" { + t.Errorf("Expected %q, got %q", "real-uuid", value) + } +} + // --- linuxDiskSerials deduplication tests --- func TestLinuxDiskSerialsDeduplicated(t *testing.T) { From 2dd5e801193703e868470933fd60eb2348b14fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:31:38 +0200 Subject: [PATCH 6/7] 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) --- executor_test.go | 120 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 3 deletions(-) diff --git a/executor_test.go b/executor_test.go index 4a92e76..cc63b73 100644 --- a/executor_test.go +++ b/executor_test.go @@ -3,6 +3,7 @@ package machineid import ( "context" "fmt" + "strings" "sync" "testing" "time" @@ -10,12 +11,27 @@ import ( // mockExecutor is a test double that implements CommandExecutor for testing. // It is safe for concurrent use (required by Windows concurrent collection). +// +// Resolution order for each Execute call: +// 1. Args-specific error (setErrorForArgs) +// 2. Args-specific output (setOutputForArgs) +// 3. Command-name error (setError) +// 4. Command-name output (setOutput) +// 5. fallback error +// +// The args-specific map lets tests distinguish calls that share a command +// name but differ in arguments (e.g. `sysctl -n machdep.cpu.brand_string` +// vs `sysctl -n machdep.cpu.features`). type mockExecutor struct { mu sync.RWMutex // outputs maps command name to expected output outputs map[string]string // errors maps command name to expected error errors map[string]error + // outputsByArgs maps "name\x00arg1\x00arg2..." to expected output + outputsByArgs map[string]string + // errorsByArgs maps "name\x00arg1\x00arg2..." to expected error + errorsByArgs map[string]error // callCount tracks how many times each command was called callCount map[string]int } @@ -23,12 +39,23 @@ type mockExecutor struct { // newMockExecutor creates a new mock executor for testing. func newMockExecutor() *mockExecutor { return &mockExecutor{ - outputs: make(map[string]string), - errors: make(map[string]error), - callCount: make(map[string]int), + outputs: make(map[string]string), + errors: make(map[string]error), + outputsByArgs: make(map[string]string), + errorsByArgs: make(map[string]error), + callCount: make(map[string]int), } } +// argsKey builds the internal lookup key for an args-specific mock entry. +// Using NUL as separator avoids collisions with args that contain spaces. +func argsKey(name string, args []string) string { + if len(args) == 0 { + return name + } + return name + "\x00" + strings.Join(args, "\x00") +} + // Execute implements CommandExecutor interface. func (m *mockExecutor) Execute(ctx context.Context, name string, args ...string) (string, error) { m.mu.Lock() @@ -38,6 +65,16 @@ func (m *mockExecutor) Execute(ctx context.Context, name string, args ...string) m.mu.RLock() defer m.mu.RUnlock() + key := argsKey(name, args) + + // Args-specific entries take precedence over plain command-name entries. + if err, exists := m.errorsByArgs[key]; exists { + return "", err + } + if output, exists := m.outputsByArgs[key]; exists { + return output, nil + } + if err, exists := m.errors[name]; exists { return "", err } @@ -50,6 +87,7 @@ func (m *mockExecutor) Execute(ctx context.Context, name string, args ...string) } // setOutput configures the mock to return the given output for a command. +// Matches any invocation of the command regardless of arguments. func (m *mockExecutor) setOutput(command, output string) { m.mu.Lock() defer m.mu.Unlock() @@ -57,12 +95,31 @@ func (m *mockExecutor) setOutput(command, output string) { } // setError configures the mock to return an error for a command. +// Matches any invocation of the command regardless of arguments. func (m *mockExecutor) setError(command string, err error) { m.mu.Lock() defer m.mu.Unlock() m.errors[command] = err } +// setOutputForArgs configures the mock to return the given output only when +// the command is invoked with the exact args slice. Takes precedence over +// setOutput. +func (m *mockExecutor) setOutputForArgs(command string, args []string, output string) { + m.mu.Lock() + defer m.mu.Unlock() + m.outputsByArgs[argsKey(command, args)] = output +} + +// setErrorForArgs configures the mock to return an error only when the +// command is invoked with the exact args slice. Takes precedence over +// setError. +func (m *mockExecutor) setErrorForArgs(command string, args []string, err error) { + m.mu.Lock() + defer m.mu.Unlock() + m.errorsByArgs[argsKey(command, args)] = err +} + // TestExecuteTimeout tests that command execution respects timeout. func TestExecuteTimeout(t *testing.T) { executor := &defaultCommandExecutor{} @@ -87,3 +144,60 @@ func TestExecuteCommandWithNilExecutor(t *testing.T) { t.Logf("Command execution with nil executor: %v", err) } } + +// TestMockExecutorArgsAwareOverride verifies that setOutputForArgs takes +// precedence over setOutput and that different arg slices resolve +// independently — required for tests that exercise multiple sysctl +// subcommands. +func TestMockExecutorArgsAwareOverride(t *testing.T) { + m := newMockExecutor() + m.setOutput("sysctl", "generic-output") + m.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.brand_string"}, "Apple M1 Pro") + m.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.features"}, "") + + ctx := context.Background() + + brand, err := m.Execute(ctx, "sysctl", "-n", "machdep.cpu.brand_string") + if err != nil { + t.Fatalf("brand call: %v", err) + } + if brand != "Apple M1 Pro" { + t.Errorf("brand = %q, want %q", brand, "Apple M1 Pro") + } + + features, err := m.Execute(ctx, "sysctl", "-n", "machdep.cpu.features") + if err != nil { + t.Fatalf("features call: %v", err) + } + if features != "" { + t.Errorf("features = %q, want empty", features) + } + + // An un-overridden invocation should fall through to setOutput. + generic, err := m.Execute(ctx, "sysctl", "-a") + if err != nil { + t.Fatalf("generic call: %v", err) + } + if generic != "generic-output" { + t.Errorf("generic = %q, want %q", generic, "generic-output") + } +} + +// TestMockExecutorArgsAwareError verifies setErrorForArgs takes precedence. +func TestMockExecutorArgsAwareError(t *testing.T) { + m := newMockExecutor() + m.setOutput("sysctl", "generic") + m.setErrorForArgs("sysctl", []string{"-n", "machdep.cpu.features"}, fmt.Errorf("nope")) + + ctx := context.Background() + if _, err := m.Execute(ctx, "sysctl", "-n", "machdep.cpu.features"); err == nil { + t.Error("Expected args-specific error") + } + out, err := m.Execute(ctx, "sysctl", "-n", "machdep.cpu.brand_string") + if err != nil { + t.Fatalf("generic call: %v", err) + } + if out != "generic" { + t.Errorf("generic = %q, want %q", out, "generic") + } +} From cb1240a18fcc3b45b37e36169559e86f66751af0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Gonz=C3=A1lez=20Di=20Antonio?= Date: Sat, 11 Apr 2026 16:34:05 +0200 Subject: [PATCH 7/7] fix: reject null UUIDs and lock Apple Silicon CPU path on darwin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- darwin.go | 34 ++++++++-- darwin_test.go | 170 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 4 deletions(-) diff --git a/darwin.go b/darwin.go index b664b6f..330d320 100644 --- a/darwin.go +++ b/darwin.go @@ -17,6 +17,11 @@ var ( ioregSerialRe = regexp.MustCompile(`"IOPlatformSerialNumber"\s*=\s*"([^"]+)"`) ) +// nullUUID is the all-zero UUID that some firmware implementations return +// when no real hardware UUID is programmed. It must be rejected so it +// cannot contribute to the machine ID. +const nullUUID = "00000000-0000-0000-0000-000000000000" + // spHardwareDataType represents the JSON output of `system_profiler SPHardwareDataType -json`. type spHardwareDataType struct { SPHardwareDataType []spHardwareEntry `json:"SPHardwareDataType"` @@ -90,6 +95,7 @@ func collectIdentifiers(ctx context.Context, p *Provider, diag *DiagnosticInfo) } // macOSHardwareUUID retrieves hardware UUID using system_profiler with JSON parsing. +// Null UUIDs (all zeros) are rejected so the fallback path is triggered. func macOSHardwareUUID(ctx context.Context, executor CommandExecutor, logger *slog.Logger) (string, error) { output, err := executeCommand(ctx, executor, logger, "system_profiler", "SPHardwareDataType", "-json") if err == nil { @@ -97,10 +103,14 @@ func macOSHardwareUUID(ctx context.Context, executor CommandExecutor, logger *sl return e.PlatformUUID }) if parseErr == nil { - return uuid, nil - } - - if logger != nil { + if uuid == nullUUID { + if logger != nil { + logger.Debug("system_profiler returned null UUID, falling back") + } + } else { + return uuid, nil + } + } else if logger != nil { logger.Debug("system_profiler UUID parsing failed", "error", parseErr) } } @@ -114,6 +124,7 @@ func macOSHardwareUUID(ctx context.Context, executor CommandExecutor, logger *sl } // macOSHardwareUUIDViaIOReg retrieves hardware UUID using ioreg as fallback. +// Null UUIDs (all zeros) are rejected with ErrNotFound. func macOSHardwareUUIDViaIOReg(ctx context.Context, executor CommandExecutor, logger *slog.Logger) (string, error) { output, err := executeCommand(ctx, executor, logger, "ioreg", "-d2", "-c", "IOPlatformExpertDevice") if err != nil { @@ -122,6 +133,14 @@ func macOSHardwareUUIDViaIOReg(ctx context.Context, executor CommandExecutor, lo match := ioregUUIDRe.FindStringSubmatch(output) if len(match) > 1 { + if match[1] == nullUUID { + if logger != nil { + logger.Debug("ioreg returned null UUID") + } + + return "", &ParseError{Source: "ioreg output", Err: ErrNotFound} + } + return match[1], nil } @@ -182,6 +201,13 @@ func macOSSerialNumberViaIOReg(ctx context.Context, executor CommandExecutor, lo // producing "ChipType:" — this trailing colon is preserved for backward // compatibility with existing license activations. // Falls back to system_profiler chip_type only if sysctl fails entirely. +// +// Known quirk: if machdep.cpu.brand_string succeeds but machdep.cpu.features +// errors (e.g. transient syscall failure under sandboxing), the result +// degrades to just cpuBrand instead of "cpuBrand:features". This produces a +// different hash for the same machine across calls. The divergence is +// preserved intentionally — changing it would invalidate every existing +// license activation generated under the current behavior. func macOSCPUInfo(ctx context.Context, executor CommandExecutor, logger *slog.Logger) (string, error) { // Primary: sysctl (backward compatible) output, err := executeCommand(ctx, executor, logger, "sysctl", "-n", "machdep.cpu.brand_string") diff --git a/darwin_test.go b/darwin_test.go index 3863251..f3afd4e 100644 --- a/darwin_test.go +++ b/darwin_test.go @@ -721,3 +721,173 @@ func TestMacOSCPUInfoAllFailErrorType(t *testing.T) { t.Errorf("Expected ErrAllMethodsFailed, got %v", err) } } + +// --- Null UUID rejection (D1) --- + +// TestMacOSHardwareUUIDRejectsNullFromSystemProfiler verifies that a null UUID +// from system_profiler triggers the ioreg fallback rather than being accepted. +func TestMacOSHardwareUUIDRejectsNullFromSystemProfiler(t *testing.T) { + mock := newMockExecutor() + mock.setOutput("system_profiler", `{ + "SPHardwareDataType": [{ + "platform_UUID": "00000000-0000-0000-0000-000000000000", + "serial_number": "C02TEST" + }] + }`) + mock.setOutput("ioreg", `"IOPlatformUUID" = "REAL-UUID-FROM-IOREG"`) + + result, err := macOSHardwareUUID(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if result != "REAL-UUID-FROM-IOREG" { + t.Errorf("Expected 'REAL-UUID-FROM-IOREG', got %q", result) + } +} + +func TestMacOSHardwareUUIDRejectsNullFromSystemProfilerWithLogger(t *testing.T) { + var buf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug})) + + mock := newMockExecutor() + mock.setOutput("system_profiler", `{ + "SPHardwareDataType": [{ + "platform_UUID": "00000000-0000-0000-0000-000000000000" + }] + }`) + mock.setOutput("ioreg", `"IOPlatformUUID" = "REAL-UUID"`) + + if _, err := macOSHardwareUUID(context.Background(), mock, logger); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !bytes.Contains(buf.Bytes(), []byte("system_profiler returned null UUID")) { + t.Error("Expected 'system_profiler returned null UUID' log") + } +} + +// TestMacOSHardwareUUIDViaIORegRejectsNull verifies that ioreg null UUIDs +// return ErrNotFound so no garbage propagates into the machine ID. +func TestMacOSHardwareUUIDViaIORegRejectsNull(t *testing.T) { + mock := newMockExecutor() + mock.setOutput("ioreg", `"IOPlatformUUID" = "00000000-0000-0000-0000-000000000000"`) + + _, err := macOSHardwareUUIDViaIOReg(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error for null UUID from ioreg") + } + var parseErr *ParseError + if !errors.As(err, &parseErr) { + t.Errorf("Expected ParseError, got %T", err) + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) + } +} + +// TestMacOSHardwareUUIDBothNull verifies both sources returning null fails. +func TestMacOSHardwareUUIDBothNull(t *testing.T) { + mock := newMockExecutor() + mock.setOutput("system_profiler", `{ + "SPHardwareDataType": [{ + "platform_UUID": "00000000-0000-0000-0000-000000000000" + }] + }`) + mock.setOutput("ioreg", `"IOPlatformUUID" = "00000000-0000-0000-0000-000000000000"`) + + _, err := macOSHardwareUUID(context.Background(), mock, nil) + if err == nil { + t.Fatal("Expected error when both sources return null UUID") + } + if !errors.Is(err, ErrNotFound) { + t.Errorf("Expected ErrNotFound, got %v", err) + } +} + +// --- Apple Silicon CPU path via args-aware mock (D2) --- + +// TestMacOSCPUInfoAppleSiliconTrailingColon exercises the exact Apple Silicon +// production path: brand_string returns the chip name, features returns empty, +// producing "ChipType:" with a trailing colon that must be preserved for +// backward compatibility with existing license activations. +func TestMacOSCPUInfoAppleSiliconTrailingColon(t *testing.T) { + mock := newMockExecutor() + mock.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.brand_string"}, "Apple M1 Pro") + mock.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.features"}, "") + + result, err := macOSCPUInfo(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if result != "Apple M1 Pro:" { + t.Errorf("Expected 'Apple M1 Pro:' (trailing colon), got %q", result) + } +} + +// TestMacOSCPUInfoIntelBrandAndFeatures exercises the Intel production path +// where both brand_string and features return real data. +func TestMacOSCPUInfoIntelBrandAndFeatures(t *testing.T) { + mock := newMockExecutor() + mock.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.brand_string"}, "Intel(R) Core(TM) i7-9750H") + mock.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.features"}, "FPU VME DE PSE") + + result, err := macOSCPUInfo(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expected := "Intel(R) Core(TM) i7-9750H:FPU VME DE PSE" + if result != expected { + t.Errorf("Expected %q, got %q", expected, result) + } +} + +// TestMacOSCPUInfoBrandOKFeaturesErrorDegrades documents the D3 quirk: when +// brand_string succeeds but features errors, the code returns just cpuBrand. +// This is intentionally preserved for backward compatibility (see the doc +// comment on macOSCPUInfo) — the test locks in the current behavior so any +// future change that would break existing license activations fails loudly. +func TestMacOSCPUInfoBrandOKFeaturesErrorDegrades(t *testing.T) { + mock := newMockExecutor() + mock.setOutputForArgs("sysctl", []string{"-n", "machdep.cpu.brand_string"}, "Apple M1 Pro") + mock.setErrorForArgs("sysctl", []string{"-n", "machdep.cpu.features"}, fmt.Errorf("sandboxed")) + + result, err := macOSCPUInfo(context.Background(), mock, nil) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + // Degraded path: returns cpuBrand without trailing colon. + if result != "Apple M1 Pro" { + t.Errorf("Expected 'Apple M1 Pro' (degraded path), got %q", result) + } +} + +// --- parseStorageJSON edge cases --- + +// TestParseStorageJSONMissingInternalField verifies that entries without the +// is_internal_disk field are treated as non-internal (the current behavior). +func TestParseStorageJSONMissingInternalField(t *testing.T) { + jsonOutput := `{ + "SPStorageDataType": [ + { + "_name": "Volume", + "physical_drive": { + "device_name": "SOME DISK" + } + }, + { + "_name": "Internal", + "physical_drive": { + "device_name": "APPLE SSD", + "is_internal_disk": "yes" + } + } + ] + }` + + result, err := parseStorageJSON(jsonOutput) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if len(result) != 1 || result[0] != "APPLE SSD" { + t.Errorf("Expected [APPLE SSD], got %v", result) + } +}