You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(secrets): DPAPI migration failure is unobservable, no warning and no debug trace #487
Found while reviewing #464 at commit c46016c. Not a merge blocker; filing so it is tracked rather than lost in the review thread. Related to #461 (Windows support).
What this is not: not a regression (before the keychain-windows backend existed, every Windows host stored secrets as plaintext unconditionally, so a DPAPI-broken host lands exactly where it already was: an unrealized improvement, not a new exposure), and not a design error in the never-fail-the-read posture, which is correct and documented in the code comment. The write path is already loud about the identical failure.
The gap is purely observability: every failure path in _acq_secret_migrate_windows returns 0, the caller ignores the return, and nothing at all is emitted. Observed with a stub whose encrypt step always fails:
resolve -> rc=0, value returned correctly (no message)
after 4 consecutive reads: file still plaintext on disk
ACQ_DEBUG=1: no output whatsoever
Consequence: on a host where PowerShell is present but DPAPI is unusable (e.g. Constrained Language Mode on a locked-down managed device), a user has no way to learn that the encryption-at-rest guarantee is not being met. For usai the 7-day key expiry pulls them onto the loud write path within a week; a github token has no forced expiry, so it can sit plaintext indefinitely. It also spends a PowerShell subprocess on every read, forever.
This is the case AGENTS.md's "Capture errors and document failures clearly" has in mind: the continuation is deliberate and correct here, but a security-relevant degradation should not be invisible.
Suggested fix: an acq_debug line on every migration failure plus a once-per-key stderr warning. No change to the best-effort, never-fail-the-read behavior.
Verified fixed by the merged #464 changes. Current main includes the migration observability path in acq.backends/secret-store.sh: failed Windows DPAPI legacy migration now calls acq_debug, emits a once-per-key warning through the preserved warning fd, leaves the legacy plaintext readable, and records a per-key warning marker. There is also a regression test covering the failed-migration/read-still-succeeds/warn-once behavior in test/bats/45-secret-store-unit.bats.\n\nLocal reproduction with a failing PowerShell stub showed:\n\n\nacq: warning: could not encrypt the 'acq.github' secret at rest with Windows DPAPI (DPAPI encryption failed); it remains in plaintext on disk. Re-run 'acq secret set' once DPAPI is available.\nr1=[legacyPLAINTEXT]\nr2=[legacyPLAINTEXT]\nat-rest=plaintext\nmarker=present\n\n\nThe focused bats runner could not be executed in this worktree because vendored bats submodules are not initialized, but the direct reproduction confirms the issue's observed failure mode is no longer silent.
Activity
fix(secrets): make DPAPI legacy migration race-safe and observable
fix(secrets): harden DPAPI envelope store per adversarial review
fix(secrets): make DPAPI legacy migration race-safe and observable
fix(secrets): harden DPAPI envelope store per adversarial review
fix(secrets): make DPAPI legacy migration race-safe and observable
fix(secrets): harden DPAPI envelope store per adversarial review
feat(secrets): add Windows-native DPAPI secret backend (#464)
mogul commentedon Oct 3, 2026
Verified fixed by the merged #464 changes. Current main includes the migration observability path in acq.backends/secret-store.sh: failed Windows DPAPI legacy migration now calls acq_debug, emits a once-per-key warning through the preserved warning fd, leaves the legacy plaintext readable, and records a per-key warning marker. There is also a regression test covering the failed-migration/read-still-succeeds/warn-once behavior in test/bats/45-secret-store-unit.bats.\n\nLocal reproduction with a failing PowerShell stub showed:\n\n
\nacq: warning: could not encrypt the 'acq.github' secret at rest with Windows DPAPI (DPAPI encryption failed); it remains in plaintext on disk. Re-run 'acq secret set' once DPAPI is available.\nr1=[legacyPLAINTEXT]\nr2=[legacyPLAINTEXT]\nat-rest=plaintext\nmarker=present\n\n\nThe focused bats runner could not be executed in this worktree because vendored bats submodules are not initialized, but the direct reproduction confirms the issue's observed failure mode is no longer silent.