fix(agent): restrict secret redaction to credential fields to stop corrupting code output (#33801)

[rodboev]

Summary

The regex-based secret redactor (agent/redact.py:redact_sensitive_text()) runs over the full text of tool output from execute_code, terminal, and write_file, corrupting code syntax when patterns like MAX_TOKENS=100, "apiKey": "test", or ${{ secrets.DEPLOY_TOKEN }} match the ENV-assignment, JSON-field, or prefix regexes. Users report spending 30+ minutes per session working around the corruption, and execute_code is effectively unusable for tasks involving credentialed API calls.

The function already has a code_file=True parameter that skips the ENV-assignment (_ENV_ASSIGN_RE) and JSON-field (_JSON_FIELD_RE) regexes, and file_tools.py already passes it. The terminal and execute_code tools do not, so both get false-positive redaction on code output. A second false-positive class (documented in the issue thread by @yzzztech) comes from _PREFIX_RE matching credential-shaped prefixes inside GitHub Actions template references (${{ secrets.* }}, ${{ vars.* }}), shell variable references ($VARNAME, ${VARNAME}), and similar template syntax. These are variable references, not bare credentials, and redacting them breaks CI/CD workflow files.

This PR adds code_file=True to all four redact_sensitive_text() calls in code_execution_tool.py (3 calls) and terminal_tool.py (1 call), matching the approach in open PR #33840. It goes further by making _PREFIX_RE template-context-aware when code_file=True: matches inside ${{ ... }}, ${...}, or $VARNAME contexts are preserved. Bare credential-shaped strings (e.g. a literal sk-proj-... token in output) are still redacted. Auth headers, private keys, DB connection strings, JWTs, and Telegram tokens are unaffected by code_file=True and continue to be redacted in all tool output.

Fixes #33801

Changes

• tools/code_execution_tool.py: pass code_file=True to all 3 redact_sensitive_text() calls (lines 1028, 1429, 1430)
• tools/terminal_tool.py: pass code_file=True to the redact_sensitive_text() call (line 2338)
• agent/redact.py: add _preserve_template_prefix() helper and _TEMPLATE_CONTEXT_RE regex; when code_file=True, _PREFIX_RE matches inside template variable references are preserved instead of masked (+~15 lines)
• tests/agent/test_redact.py: add TestCodeFileParameter class with 9 tests covering ENV skip, JSON skip, bare prefix still redacted, GitHub Actions template preserved (using real sk- prefix inside ${{ }}), shell variable preserved (using real ghp_ prefix inside ${}), trailing credential after template still redacted, auth header still redacted, JWT still redacted, private key still redacted (+~55 lines)
• tests/tools/test_terminal_output_transform_hook.py: add test_terminal_output_code_file_skips_env_assignment confirming the terminal tool passes code_file=True (+~15 lines)

Validation

Scenario	Before	After
MAX_TOKENS=100 in execute_code output	MAX_TOKENS=***	MAX_TOKENS=100
"apiKey": "test" in terminal output	"apiKey": "***"	"apiKey": "test"
${{ secrets.DEPLOY_TOKEN }} in write_file/terminal	Partially mangled if token value matches ghp_ etc.	Preserved (template context)
Bare sk-proj-abc123... in terminal output	Redacted	Still redacted
Authorization: Bearer <token> in code output	Redacted	Still redacted
JWT (eyJhbG...) in execute_code stderr	Redacted	Still redacted
Private key block in terminal output	Redacted	Still redacted
read_file output (file_tools.py)	Already passes code_file=True	Unchanged
Chat-completion redaction (agent layer)	Does not pass code_file=True	Unchanged

Test plan

• pytest tests/agent/test_redact.py -v --timeout=0 — 83 passed
• pytest tests/tools/test_terminal_output_transform_hook.py -v --timeout=0 — 10 passed
• New: TestCodeFileParameter (9 tests) covering all code_file=True behaviors including template-context preservation with real prefix patterns
• New: test_terminal_output_code_file_skips_env_assignment confirming terminal integration
• Existing: test_terminal_output_transform_still_runs_strip_and_redact confirms prefix redaction on bare secrets

Not in scope

Moving redaction to a display-only layer (the suggestion in the issue body and PR #16849's display_redaction_only config key) is deliberately left out. That is a larger architectural change that would require auditing every call site where tool output enters the model context. The code_file=True approach is narrowly scoped, backward-compatible, and addresses the reported corruption without changing the redaction architecture. The _PREFIX_RE template-context fix is also conservative: it preserves matches only inside recognized template syntaxes, not all occurrences.

[pfrenssen]

This PR is a great step forward, but it leaves one redaction class untouched that has an outsized impact: _DB_CONNECTION_RE is explicitly excluded from code_file=True and continues to redact in all tool output.

The problem is that read_file (in file_tools.py) already passes code_file=True, so even after this PR merges, any agent reading back a Python config file that contains an f-string DSN template will see phantom corruption. Example:

Actual bytes on disk (correct):

        return f"postgresql://{auth}@{self.pg_host}:{self.pg_port}/{self.pg_database}"

The consequence is a hallucination feedback loop: the agent writes correctly, reads back via read_file, sees *** and missing lines, concludes the write failed, retries, reads back, sees *** again… burns entire iteration budget fighting phantom corruption. The file was correct the entire time.

This bit a Kanban worker today — two consecutive 90-iteration timeouts on a task that needed to write a pydantic-settings config.py. The worker never had a chance because it couldn't verify its own output.

_DB_CONNECTION_RE needs the same awareness this PR gives _PREFIX_RE: the following f-string templates have zero live credentials and should not match:

• f"postgresql://{user}:{pass}@{host}:{port}/{database}"
• f"postgresql://{auth}@{host}:{port}/{database}"

Only literal <scheme>://<user>:<password>@<host>:<port>/<db> should trigger redaction.

[rodboev]

Good catch — I hadn't considered that _DB_CONNECTION_RE would still fire when read_file passes code_file=True. I've extended the same template-awareness to it: when code_file=True, the redaction sub now checks whether the password group is an f-string placeholder ({...}) and preserves the original text in that case. Literal credentials still get masked. Two tests added for both paths.

[teknium1]

Thanks for this — the diagnosis was on the right track and the test suite was thorough. We landed a root-cause fix for #33801 in #54061 (merged: 674e16e).

One thing your fix didn't catch, which is why we went a different route: the _DB_CONNSTR_RE template guard here checks group(2).endswith("}"), but on the actual reported multi-line repro the password group [^@]+ is greedy across newlines and has already overrun past the closing } (it grabs the closing quote, the blank line, and the start of the next line up to a stray @decorator). So group(2) ends in whitespace, not }, and the guard doesn't fire — the displayed line stays corrupted.

The merged fix forbids whitespace in the userinfo/password groups ([^:\s]+ / [^@\s]+) so the match can never span a line — which kills the catastrophic line-dropping — and then applies the brace-template preservation under code_file=True. Your code_file=True plumbing on terminal/execute_code output is also in the merged change. Closing as addressed by #54061.