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. 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 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 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) + } +} 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") + } +} 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) { 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