Repository navigation
Redact secret_request values from transcripts, pushes and memory (#1531) - #1555
Conversation
A secret given through secret_request rides the run env, so an agent that runs `echo $STRIPE_API_KEY` used to put the value into tool output, the transcript and coder-store.json. store.appendMessage/updateMessage now replace the thread's secret values (>= 6 chars) with [secret:NAME]. Every message write goes through those two, and the renderer push, summaries and terminal session record read back from the store. Two paths carry text past the store and are redacted too: the session transcript queue (user/event on append) and the run auto-record POSTed to the memory server, which the new test caught leaking. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24d72615dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Every runner/adapter message write lands here or in updateMessage, and | ||
| // the renderer push, summaries and session transcript read back from the | ||
| // store, so redacting here covers them all (#1531 secret_request). | ||
| list.push(threadSecrets.redact(threadId, message)); |
There was a problem hiding this comment.
When a provider prints a requested secret to stderr and exits nonzero, markRunFailed stores the unredacted shortError(text) in thread.lastError; this append-time redaction only protects the duplicate event message. The thread row is then written to coder-store.json and returned by listThreads, so the secret still leaks through both disk and threads:changed pushes. Apply the thread's redaction to failure/thread metadata before calling updateThread as well.
Useful? React with 👍 / 👎.
| await proxy.store(payload); | ||
| // The body quotes the run's final text, which may echo a secret (#1531). | ||
| await proxy.store(threadSecrets.redact(String(thread.id || ""), payload)); | ||
| } catch { |
There was a problem hiding this comment.
If a secret crosses the 1,200-character boundary in the final run text, buildRunBody truncates it first, leaving only a prefix that the later exact-value replacement cannot recognize; that raw secret prefix is then posted to shared memory. This is especially significant for long credentials such as PEM values. Redact outcome.text before buildRunBody slices it rather than redacting only the completed payload.
Useful? React with 👍 / 👎.
#1555 redacts secret_request values in appendMessage/updateMessage, but work-log items (tool commands and output) were stored and pushed as is. appendWorkLog and updateWorkLogItem now redact the incoming item/patch the same way, so a printed $NAME leaves only [secret:NAME] behind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1555 redacts secret_request values in appendMessage/updateMessage, but work-log items (tool commands and output) were stored and pushed as is. appendWorkLog and updateWorkLogItem now redact the incoming item/patch the same way, so a printed $NAME leaves only [secret:NAME] behind. Co-authored-by: Willem <willem@willems-mbp.home> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
#1555 redacts secret_request values in appendMessage/updateMessage, but work-log items (tool commands and output) were stored and pushed as is. appendWorkLog and updateWorkLogItem now redact the incoming item/patch the same way, so a printed $NAME leaves only [secret:NAME] behind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up to #1542. A secret given through
secret_requestis in the run env, soecho $STRIPE_API_KEYused to put the value into tool output, the transcript andcoder-store.json.threadSecrets.redact(threadId, value)deep-replaces the thread's secret values (>= 6 chars, longest first) with[secret:NAME]. It returns the input untouched when the thread has no secrets.store.appendMessageandstore.updateMessage. Every runner and adapter message write goes through one of these two. Thethread:updatedstream-tail push, the summaries and the terminal session record all read back from the store./api/store). The new test caught the auto-record leaking the raw value.Test: in
electron/test/secret-request.test.js, an agent echoes$STRIPE_API_KEY, a tool result carrying the value is appended and patched, and a fake memory server captures/api/sessionand/api/store. The test asserts[secret:STRIPE_API_KEY]is present and the value is absent in messages, pushed details, the transcript and every file under userData. It fails when the store redaction is removed.npm run test:electron: 4467 pass, 0 fail.🤖 Generated with Claude Code