Skip to content

feat(agent): read the token through a secret store(#1) - #239

Open
Stephanya Casanova (savacano28) wants to merge 8 commits into
mainfrom
sca/agent-secret-store
Open

Stephanya Casanova (savacano28) wants to merge 8 commits into
mainfrom
sca/agent-secret-store

Conversation

@savacano28

@savacano28 Stephanya Casanova (savacano28) commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

The agent read the token once at startup and kept it in memory, in the client of each thread. It now goes through a SecretStore trait and is read for each request, held in Zeroizing buffers that are wiped once used, and sent in a header marked sensitive.

flowchart LR
  R[Request] --> S["SecretStore.get()"] --> C[(config.toml)]
  S --> H[Bearer header<br/>wiped after use]
Loading

Proposed changes

Groundwork for keeping the agent token in the OS secret store (#273). This PR only changes how the agent reads the token. The token stays in the config, and the installers don't change.

  • Before: the token was read once at start-up, copied into each thread, and kept in memory for the whole run.
  • After: the agent asks a SecretStore for the token before each call and wipes it from memory right after.
    • ConfigStore (the only store for now) reads the token from the config.
    • The request header holds the token in a buffer that is wiped after the call, and is marked sensitive (hidden from debug output).
    • Printing the settings shows <redacted> instead of the token.
  • New dependencies: bytes, zeroize (already in the dependency tree).

Testing Instructions

Build the branch and install the agent in one mode per OS. On Windows, run the NSIS installer directly, since branch builds aren't release-signed. Then check each row:

Automated: cargo test --locked

Manual (Windows Sandbox, build this branch, then run the NSIS installer directly):

  1. Install the agent (service mode) → it shows online and an inject works.
  2. Search openaev-agent.log for the token → not found.
  3. While the agent runs, set a wrong token in the config → calls fail. Put the right one back → the agent is online again without restart.
  4. Swap the exe of an agent installed from the current release → still online.

Related issues

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

  • Impact for users: none. Same files, same installers, same behaviour. A changed token in the config is now used without a restart.
  • Limit: the HTTP/TLS libraries and the config loader keep their own copies of the token, which the agent can't wipe. The token is still in clear text on disk;

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.91%. Comparing base (a03efd7) to head (69723a1).

Files with missing lines Patch % Lines
src/config/settings.rs 53.12% 15 Missing ⚠️
src/api/manage_jobs.rs 0.00% 5 Missing ⚠️
src/api/mod.rs 85.18% 4 Missing ⚠️
src/main.rs 0.00% 3 Missing ⚠️
src/secrets/mod.rs 0.00% 3 Missing ⚠️
src/process/agent_job.rs 0.00% 2 Missing ⚠️
src/process/keep_alive.rs 0.00% 2 Missing ⚠️
src/api/register_agent.rs 0.00% 1 Missing ⚠️
src/secrets/config_store.rs 93.33% 1 Missing ⚠️

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #239      +/-   ##
==========================================
+ Coverage   32.26%   38.91%   +6.65%     
==========================================
  Files          12       14       +2     
  Lines         685      740      +55     
==========================================
+ Hits          221      288      +67     
+ Misses        464      452      -12     
Flag Coverage Δ
agent 38.91% <60.00%> (+6.65%) ⬆️

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

Files with missing lines Coverage Δ
src/api/register_agent.rs 0.00% <0.00%> (ø)
src/secrets/config_store.rs 93.33% <93.33%> (ø)
src/process/agent_job.rs 0.00% <0.00%> (ø)
src/process/keep_alive.rs 0.00% <0.00%> (ø)
src/main.rs 0.00% <0.00%> (ø)
src/secrets/mod.rs 0.00% <0.00%> (ø)
src/api/mod.rs 89.23% <85.18%> (+1.47%) ⬆️
src/api/manage_jobs.rs 0.00% <0.00%> (ø)
src/config/settings.rs 61.81% <53.12%> (+61.81%) ⬆️

... 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.

@savacano28
Stephanya Casanova (savacano28) changed the base branch from main to sca/windows-config-acl October 14, 2026 13:23
@savacano28
Stephanya Casanova (savacano28) changed the base branch from sca/windows-config-acl to main October 14, 2026 13:27
@savacano28
Stephanya Casanova (savacano28) added this pull request to stack #243 October 14, 2026 13:28
@savacano28 Stephanya Casanova (savacano28) changed the title feat(agent): read the token through a secret store feat(agent): read the token through a secret store (#1) Oct 14, 2026
@savacano28 Stephanya Casanova (savacano28) added the filigran team Item from the Filigran team. label Oct 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Token import can corrupt the configuration on write failure and remove unrelated TOML settings.

2 open findings
What changed in this PR

Introduces per-request token retrieval through a secret-store abstraction, reducing long-lived plaintext credentials in memory.

Changes:

  • Adds SecretStore, config-backed storage, and token import support.
  • Reads and redacts tokens securely for API requests and settings output.
  • Updates worker threads and tests for shared secret stores.
File Description
Cargo.toml Adds zeroize.
Cargo.lock Locks the dependency update.
src/​api/​mod.rs Retrieves tokens per request and marks headers sensitive.
src/​api/​manage_jobs.rs Propagates authorization failures.
src/​api/​register_agent.rs Propagates authorization failures.
src/​config/​settings.rs Makes tokens optional and redacts debug output.
src/​main.rs Opens and shares the secret store.
src/​process/​agent_job.rs Passes the store to the jobs client.
src/​process/​keep_alive.rs Passes the store to the heartbeat client.
src/​secrets/​mod.rs Defines and opens secret stores.
src/​secrets/​config_store.rs Implements config-backed token retrieval.
src/​secrets/​import.rs Imports and removes plaintext tokens.
src/​tests/​api/​client.rs Tests per-request authorization behavior.
src/​tests/​config/​mod.rs Registers settings tests.
src/​tests/​config/​settings_tests.rs Tests token redaction.
src/​tests/​mod.rs Registers secret-store tests.
src/​tests/​secrets/​mod.rs Adds test secret-store utilities.
src/​tests/​secrets/​config_store_tests.rs Tests config-backed storage.
src/​tests/​secrets/​import_tests.rs Tests token migration scenarios.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/secrets/import.rs Outdated
Comment thread src/secrets/import.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Token copies remain in non-zeroizing settings and request-header allocations, undermining the stated memory-wiping guarantee.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/api/mod.rs Outdated
Comment thread src/config/settings.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cleanup ordering can cause a newer bootstrap token to be replaced by a stale config token after restart.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Comment thread src/secrets/import.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Configuration parser diagnostics can expose tokens in logs during malformed live edits.

3 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/secrets/config_store.rs Outdated
Comment thread src/api/mod.rs
Comment thread src/secrets/config_store.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Production file-backed token reload and recovery need automated coverage before approval.

0 open findings

3 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Add isolated file-backed ConfigStore reload and recovery tests

src/​secrets/​config_store.rs:18

The only ConfigStore test requests refresh_token, returning before this loading path. The rotation and recovery tests use MemoryStore, so they do not verify the production behavior promised by this PR. Please add isolated file-backed tests that read a token, edit it and observe the new value through the same ConfigStore, then verify an unreadable or invalid configuration returns an error and succeeds after repair. Avoid changing shared configuration files or process-wide environment variables in parallel tests.

🧠 Review effort: Balanced

@savacano28
Stephanya Casanova (savacano28) force-pushed the sca/agent-secret-store branch 2 times, most recently from 6d504e8 to 120710f Compare October 16, 2026 09:26
@guillaumejparis Guillaume (guillaumejparis) changed the title feat(agent): read the token through a secret store (#1) feat(agent): read the token through a secret store(#1) Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants