Skip to content

fix(agent): redact secrets from assistant output - #21044

Closed
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/redact-agent-output-20785
Closed

fix(agent): redact secrets from assistant output#21044
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/redact-agent-output-20785

Conversation

@LeonSGP43

@LeonSGP43 LeonSGP43 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • force existing secret redaction at assistant output boundaries even when general log redaction is disabled
  • redact streamed deltas, interim assistant commentary, reasoning callbacks, stored assistant content/reasoning, and iteration-limit summaries
  • add focused regressions for visible output, reasoning output, interim callbacks, and stored assistant messages

Why This Matters

Issue #20785 reports systemic secret leakage in assistant-visible output paths. This PR keeps the response scoped to that boundary: force redaction on streamed deltas, interim commentary, reasoning callbacks, persisted assistant content, and iteration-limit summaries even when broader log-redaction settings are off.

Verification

  • scripts/run_tests.sh tests/run_agent/test_agent_output_redaction.py
  • scripts/run_tests.sh tests/run_agent/test_agent_output_redaction.py tests/cli/test_reasoning_command.py::TestReasoningDeltasFiredFlag tests/run_agent/test_run_agent_codex_responses.py::test_stream_delta_strips_leaked_memory_context tests/run_agent/test_run_agent_codex_responses.py::test_stream_delta_strips_leaked_memory_context_across_chunks tests/run_agent/test_run_agent_codex_responses.py::test_interim_commentary_is_not_marked_already_streamed_without_callbacks tests/run_agent/test_run_agent_codex_responses.py::test_interim_commentary_is_not_marked_already_streamed_when_stream_callback_fails tests/run_agent/test_run_agent_codex_responses.py::test_interim_commentary_preserves_assistant_content
  • scripts/run_tests.sh tests/agent/test_redact.py tests/run_agent/test_agent_output_redaction.py
  • python -m py_compile run_agent.py tests/run_agent/test_agent_output_redaction.py
  • git diff --check

Closes #20785

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 7, 2026
@LeonSGP43

Copy link
Copy Markdown
Contributor Author

Packaging follow-up as of 2026-05-07.

  • This PR remains narrowly scoped to #20785: force secret redaction at assistant-output boundaries even when broader log redaction is disabled.
  • The body already lists the relevant changed-area verification for the redaction paths, including the focused redaction suites, the reasoning callback coverage, python -m py_compile, and git diff --check.
  • Lint (ruff + ty) is red for workflow reasons, not because Ruff found a new blocker in this diff: the job completed its analysis and then failed when its PR-comment step tried to post to the issue with 403 Resource not accessible by integration.
  • The red GitHub Tests / test run (25477914181) failed in a broad ACP/gateway/CLI/tooling set rather than one of the targeted redaction regressions listed in the PR body. I have not reproduced a redaction-specific regression from this diff.

@egilewski

Copy link
Copy Markdown
Contributor

merge conflicts

This PR does not merge cleanly with the base branch. Please rebase or merge current main and resolve the conflicts if it's still relevant.

Signed: GPT-5.5-low in Codex

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jun 29, 2026
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, and for the careful scoping — the intent (scrub secrets at every assistant-output boundary) is exactly right. Closing without merging for three reasons:

1. The core of this already landed and closed the issue. The stored-assistant-content scrub you add is already on main at the persistence boundary in agent/chat_completion_helpers.build_assistant_message (redact_sensitive_text(_san_content), from #19798), and #21193 (merged 2026-05-07) flipped redact_sensitive_text to ON by default — which is what closed #20785's short-term half.

2. Force-redacting stored reasoning_content breaks thinking-provider replay. The one net-new behavior here — forcing redaction on the stored reasoning_content / reasoning — is an invariant break. That field is echoed back byte-for-byte to DeepSeek v4 / Kimi / Moonshot / Xiaomi MiMo thinking modes (they 400 if it's modified), and it parallels Anthropic's signed reasoning_details / anthropic_content_blocks. A reasoning string that happens to contain a token-shaped value gets mutated by the redactor, so the next replay fails with HTTP 400. It wouldn't surface in unit tests (only fires when reasoning contains a secret), but it breaks real multi-turn sessions on those providers.

3. The reporter's own follow-up notes regex can't solve the reported problem. The dangerous case — natural-language leakage ("I used the password XXXX to log in") — matches no pattern; the durable fix is taint-tracking / opaque credential references, which is a separate design track.

The safe subset of your change — redacting streamed display deltas / interim commentary (output the model never sees echoed back) — is a legitimate defense-in-depth addition, but it needs to be ported onto the refactored agent/chat_completion_helpers.py and must specifically avoid touching stored reasoning_content. If you'd like to resubmit just that display-path piece, happy to review it.

@teknium1 teknium1 closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Credential leakage in agent output: systemic failure to redact secrets in chat and reasoning blocks

4 participants