[Stephanya Casanova (savacano28)]

Proposed changes

Groundwork to keep the agent token in an OS secret store. No OS store is plugged in yet (#240 adds DPAPI), so the agent behaves like #239.

• Feature flag [secrets] os_store, off by default. Off: the token is read from the config on each request, as in feat(agent): read the token on each request instead of keeping it in memory (#1) #239. Nothing is written or deleted.
• import(), used only when the flag is on and an OS store exists: moves the token into the store, reads it back, then removes the clear-text copies.
  • A bootstrap file (openaev-agent-token.bootstrap) wins over the config. It is the way to replace a stored token.
  • A verified stored token wins over an older config token.
  • A store read error is not treated as a missing token: nothing is overwritten.
  • A pending marker (openaev-agent-token.pending) makes sure an unverified write is never trusted.
• replace_file(): rewrites the config in one step and keeps its ACL / mode. On Windows, a backup is kept and put back if the swap fails halfway. Leftovers are recovered or deleted at the next start.
• If the token line cannot be removed: retried, then a clear error (read-only config: an administrator must remove the line).
• The token becomes optional in the config.

flowchart TD
  S[Agent start] --> F{os_store?}
  F -- off --> C[Read the token from the config<br/>on each request, as in #239]
  F -- on --> I[Token from bootstrap file or config]
  I --> W[Write to the store, read back]
  W -- fails --> K[Keep clear text, retry next start]
  W -- ok --> D[Remove config line, delete bootstrap file]

Loading

Testing Instructions

1. Same steps as feat(agent): read the token on each request instead of keeping it in memory (#1) #239, flag off (default): the agent must behave exactly the same.
2. Add [secrets] / os_store = true to the config and restart: nothing changes in this PR (no OS store yet), the agent stays online.
3. Optional: write the token to openaev-agent-token.bootstrap next to the exe and remove it from the config: the agent stays online.

Automated: cargo test --locked.

Related issues

• Part of feat(agent): encrypt the token with DPAPI on Windows (#1) #240
• Depends on feat(agent): read the token on each request instead of keeping it in memory (#1) #239

Checklist

• I consider the submitted work as finished
• I tested the code for its functionality
• I wrote test cases for the relevant uses case
• I added/update the relevant documentation (either on github or on notion)
• Where necessary I refactored code to improve the overall quality
• For bug fix -> I implemented a test that covers the bug

Further comments

• Edge cases covered by tests: crash during the move or the config rewrite, failed or mismatching read-back, store read errors, older config token, leftover backup holding the token.
• Limitation: import() is only exercised by unit tests until feat(agent): encrypt the token with DPAPI on Windows (#1) #240.
• Attention: the bootstrap file wins over the config, so the install folder must not be writable by non-admins (tracked separately).
• New deps: toml_edit, windows-sys.
• Not included: OS stores (feat(agent): encrypt the token with DPAPI on Windows (#1) #240), installer changes.
• Released together with feat(agent): encrypt the token with DPAPI on Windows (#1) #240. Split only to ease the review.

[codecov]

Codecov Report

❌ Patch coverage is 76.83824% with 63 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.95%. Comparing base (8c462f0) to head (857ff51).

Files with missing lines	Patch %	Lines
src/secrets/mod.rs	55.73%	27 Missing ⚠️
src/secrets/import.rs	87.17%	15 Missing ⚠️
src/main.rs	0.00%	12 Missing ⚠️
src/config/settings.rs	11.11%	8 Missing ⚠️
src/secrets/replace.rs	98.27%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (76.83%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@                     Coverage Diff                     @@
##           sca/agent-secret-store     #245       +/-   ##
===========================================================
+ Coverage                   39.86%   49.95%   +10.08%     
===========================================================
  Files                          14       16        +2     
  Lines                         750     1011      +261     
===========================================================
+ Hits                          299      505      +206     
- Misses                        451      506       +55     

Flag	Coverage Δ
agent	49.95% <76.83%> (+10.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/secrets/config_store.rs	89.74% <100.00%> (+0.45%)	⬆️
src/secrets/replace.rs	98.27% <98.27%> (ø)
src/config/settings.rs	59.64% <11.11%> (-5.74%)	⬇️
src/main.rs	0.00% <0.00%> (ø)
src/secrets/import.rs	87.17% <87.17%> (ø)
src/secrets/mod.rs	57.14% <55.73%> (+57.14%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🟡 Changes recommended

Migration can overwrite newer stored tokens, and Windows replacement failure cleanup can delete the configuration.

2 open issues

• Legacy config can overwrite a newer stored token · New
• Windows replacement failure can delete the configuration · New

What changed in this PR

Adds token-migration groundwork for OS secret stores while keeping production on clear-text storage until an OS backend is connected.

Changes:

• Reads bootstrap tokens before config tokens and makes config tokens optional.
• Adds verified migration, clear-text cleanup, and permission-preserving file replacement.
• Adds migration and replacement tests and supporting dependencies.

File	Description
src/​tests/​secrets/​replace_tests.rs	Tests replacement, permissions, and failure handling.
src/​tests/​secrets/​mod.rs	Adds writable store doubles and test modules.
src/​tests/​secrets/​import_tests.rs	Tests migration precedence and cleanup.
src/​tests/​secrets/​config_store_tests.rs	Tests bootstrap reads and precedence.
src/​tests/​config/​settings_tests.rs	Tests optional config tokens.
src/​secrets/​replace.rs	Adds platform-specific file replacement.
src/​secrets/​mod.rs	Adds migration setup and bootstrap reading.
src/​secrets/​import.rs	Implements verified token migration and cleanup.
src/​secrets/​config_store.rs	Reads bootstrap tokens before config tokens.
src/​main.rs	Passes the startup token into store initialization.
src/​config/​settings.rs	Makes tokens optional and adds path helpers.
Cargo.toml	Adds TOML editing and Windows API dependencies.
Cargo.lock	Locks the added dependencies.

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Windows migration recovery can block restart or leave plaintext backups once migration is enabled.

2 open issues

• Clean up plaintext backups after successful config replacement · New
• Recover interrupted config replacements before loading settings · New

1 previously missed issue

In code that hasn't changed since last review

Test import rollback when credential read-back fails or mismatches

src/​secrets/​import.rs:80

The import tests cover a failed set, but not a successful set followed by a failed or mismatching read-back. Add a test store for each case and assert that import returns an error while leaving both the config and bootstrap contents unchanged. These tests protect the check that prevents deletion of the only usable credential copy.

2 resolved since last review

• Windows replacement failure can delete the configuration
• Legacy config can overwrite a newer stored token

_{Review effort: Balanced}

[Copilot]

🟡 Changes recommended

Retrying a config-only import after failed verification can delete the only valid token.

1 open issue

• Prevent config-only retries from deleting tokens after verification mismatch · New

2 resolved since last review

• Recover interrupted config replacements before loading settings
• Clean up plaintext backups after successful config replacement

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Unresolved migration-safety issues and cross-platform credential cleanup require fixes and human validation.

2 open issues

• Do not overwrite newer credentials when token reads fail · New
• Sync marker parent directory before storing to ensure durability · New

1 resolved since last review

• Prevent config-only retries from deleting tokens after verification mismatch

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

An unresolved pending-token guard and security-sensitive Windows recovery behavior require correction and platform-specific human validation.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Return an error when pending token has no verification source

src/​secrets/​import.rs:94

If a pending marker remains but the bootstrap/config token is no longer available, this branch only logs and import() returns Ok(()), even though it rejected the stored token as unverified. Once an OS store is enabled, open() interprets that success as permission to use the store, so requests can use the unverified value. Return an error for this pending-without-source case and add a regression test with a readable stored token, a pending marker, and no clear-text token. No OS store is enabled in this PR, so this currently affects the migration helper rather than live requests.

2 resolved since last review

• Sync marker parent directory before storing to ensure durability
• Do not overwrite newer credentials when token reads fail

_{Review effort: Balanced}