Repository navigation
fix(device): one stable device ID across root and user runs - #243
Merged
ashishkurmi merged 17 commits intoOct 17, 2026
Merged
ashishkurmi merged 17 commits into
ashishkurmi merged 17 commits into
Conversation
…evice-id-resolver # Conflicts: # CHANGELOG.md # internal/device/device.go # internal/device/device_test.go
Firmware defaults like "Default string", "Not Specified", "System Serial Number" or all-zeros merged distinct machines into one device_id. A shared case-insensitive denylist now makes every probe fall through to the next source.
ioreg -l dumps the whole I/O registry (several MB) and can blow the hooks' 1s budget on a busy Mac; -rd1 -c IOPlatformExpertDevice returns the same IOPlatformSerialNumber line ~10x faster.
Telemetry, the run gate, enforcers and hooks each probed the serial with their own budget and could disagree within one run. Gather and SerialNumber now share a process-wide cached result; failures aren't cached so a later caller with a longer budget retries.
Linux product_serial is root-only and dmidecode needs root, so root runs reported the hardware serial while user runs (systemd --user, hooks) fell to /etc/machine-id: one machine, two devices. Privileged runs now persist the resolved ID atomically to /etc/stepsecurity/device-id (0644) or C:\ProgramData\StepSecurity\device-id, and unprivileged runs read it before probing. Invalid file content is ignored.
When every source failed, the literal "unknown" was uploaded as device_id, merging all such machines. Now a random UUID is generated and persisted (machine-wide when privileged, else the per-user state dir) and reused on later runs. A probe cut short by its deadline mints nothing; device.ID returns "" and telemetry retries on the run deadline or fails the run instead of uploading without an identity (replacing a dead empty check).
The gate reused the device_id cached in last-run.json and never re-probed, so after an identity change it gated as one device while telemetry uploaded as another. It now always asks the process-wide resolver; the existing mismatch reset then discards the stale cadence.
The event ingest contract for an empty device_id is unverified; keep the previous behavior for hooks.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
Persistence failures, concurrent initialization, and transient probe fallbacks can still create inconsistent device identities.
3 open findings
What changed in this PR
Centralizes device identity across telemetry, hooks, policies, and WSL, building on configurable IDs to keep root and user runs consistent.
Changes:
- Adds shared identity persistence, placeholder filtering, and UUID fallback.
- Resets cadence and inventory caches when identity changes.
- Adds regression coverage and documents configurable IDs.
| File | Description |
|---|---|
| README.md | Documents custom device IDs. |
| internal/telemetry/wsl_scan.go | Uses effective host identity. |
| internal/telemetry/telemetry.go | Unifies identity and cache binding. |
| internal/state/state.go | Binds inventory to device identity. |
| internal/state/device_id_test.go | Tests identity-driven inventory resets. |
| internal/rungate/state.go | Resets cadence on identity changes. |
| internal/rungate/state_test.go | Tests identity-bound completion stamps. |
| internal/rungate/evaluate.go | Revalidates identity each run. |
| internal/rungate/evaluate_test.go | Tests overrides and stale identities. |
| internal/heartbeat/heartbeat.go | Records configured identity. |
| internal/executor/write_test.go | Tests atomic file replacement. |
| internal/executor/mock.go | Adds mock identity persistence. |
| internal/executor/executor.go | Adds atomic file writing. |
| internal/device/serial.go | Rejects placeholder serials. |
| internal/device/serial_test.go | Tests placeholder filtering. |
| internal/device/resources_test.go | Updates identity-related test setup. |
| internal/device/id.go | Resolves and persists shared identities. |
| internal/device/id_test.go | Tests persistence and fallback behavior. |
| internal/device/device.go | Centralizes identity caching and selection. |
| internal/device/device_windows.go | Filters Windows placeholder values. |
| internal/device/device_test.go | Tests configured and guest identities. |
| internal/device/device_other.go | Filters fallback Windows probe values. |
| internal/config/config.go | Supports device ID configuration. |
| internal/config/config_test.go | Tests identity configuration persistence. |
| internal/config/config_nonint_test.go | Tests file-supplied device IDs. |
| internal/buildinfo/version.go | Bumps version to 1.18.0. |
| internal/aiagents/identity/identity.go | Uses effective identity for hooks. |
| internal/aiagents/identity/identity_test.go | Tests hook identity fallback behavior. |
| cmd/stepsecurity-dev-machine-guard/main.go | Shares identity with policy consumers. |
| cmd/stepsecurity-dev-machine-guard/main_devicepolicy_test.go | Tests WSL policy identity. |
| CHANGELOG.md | Records configurable identity support. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+48
to
+50
| if id := probeSerial(ctx, exec); usableSerial(id) { | ||
| if root && readIDFile(exec, path) != id { | ||
| writeIDFile(exec, path, id) |
Comment on lines
+73
to
+77
| id := uuid.NewString() | ||
| if path != "" { | ||
| writeIDFile(exec, path, id) | ||
| } | ||
| return id |
Comment on lines
+73
to
+77
| id := uuid.NewString() | ||
| if path != "" { | ||
| writeIDFile(exec, path, id) | ||
| } | ||
| return id |
…ted generated IDs A probe that falls back to MachineGuid or /etc/machine-id no longer overwrites the machine-wide ID. Generated UUIDs are published create-once, so racing processes converge on one ID, and an ID that can't be persisted reports unknown instead of an ephemeral identity.
Load wrote DeviceID on every call, racing concurrent hook invocations.
…d IDs" This reverts commit 9228adf.
ashishkurmi
approved these changes
Oct 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #236 (agent-configured device IDs), which is reopened. This PR includes its original configuration feature; review #236 first. The temporary central-management reconciliation has been reverted.
The same machine could register under different IDs across root and user runs, accept placeholder serials shared by unrelated machines, or report
unknownafter failed probes.config.jsondevice_id still takes precedence.Devices currently reporting invalid placeholder serials intentionally get a new identity. Configured IDs must be unique within the tenant; there is no backend ownership preflight or central rename mechanism. Changing a configured ID creates a separate registration.
#246 builds on this resolver for VDI support. #247 is withdrawn.
Validation: original resolver/configuration regression coverage restored by reverting the central-management compatibility change. Diff reviewed; regular CI, smoke and cross-platform checks are left to CI. No new real root/SYSTEM/multi-user validation is claimed.