close
Skip to content

Unredacted LLM-authored tool titles on deny paths #2274

Description

@LucaButBoring

#1902 stopped the PreToolUse hook-block deny path from publishing an unredacted event.title, by redacting once in _redacted_hook_block() and using that value on every surface the helper touches. The other deny paths in chat_runner.py — invalid tool name and hook execution error — still interpolate the raw title, and so do fourteen audit calls. It was left out of that PR deliberately (separate paths, each needing its own test) and is currently recorded only in the PR description.

event.title is model-authored text. _select_tool_title returns raw_input["description"] whenever the backend supplies it and falls back to the SDK-provided title only when it does not. Its docstring names the Bash tool as a backend that supplies description, so this holds for shell tools too. A model that writes a credential or an exfiltration URL into a tool description gets it published verbatim at each site below.

1. Nine deny rows publish the raw title

Each site rejects the call and appends a blocked row built from the raw title:

except ValueError as e:
    await client.reject_tool(event.request_id)
    slot.append("tool", f"🚫 {event.title} (invalid: {e})", "msg msg-tool")

Invalid tool name, row suffix (invalid: {e}):

Hook execution error, row suffix (hook error):

The interpolated {e} needs no redaction. _validate_tool_name raises either Tool name cannot be empty or Tool name exceeds max length {MAX_TOOL_NAME_LEN} (ref), and neither message echoes the offending name. Only the title carries model-authored text.

Every other title-bearing surface in this file already applies redact_exfiltration_urls and then redact_credentials, at all ten sites that redact a title. These nine apply neither.

2. Fourteen audit records carry the raw title

Every sel().log_tool_invocation() call that passes tool_name=event.title writes the unredacted title into the SEL audit. Eight of them sit directly beneath the rows above and leak the same string twice:

sel().log_tool_invocation(
    ...
    tool_name=event.title,
    outcome="denied",
)

The remaining six sit on paths whose transcript row is already redacted, so the audit record is the only leaking surface there:

Audit call outcome= Row on that path
3673 invoked redacted
4130 denied _deny_title, redacted
4362 rejected redacted
4431 auto_approved redacted
4530 rejected redacted
4751 approved redacted

One deny row has no audit call at all: the trust-reads read-only bash path rejects, appends, and continues, so it has one surface rather than two. Seventeen surfaces across the deny paths, and six more on the audit-only paths.

Impact: the transcript row is broadcast to the dashboard and persisted to the ConversationLog, and the audit record is persisted to the SEL log. A credential the model put in a tool description therefore survives the session on disk in two places. Redacting later does not unwrite what was already stored, so every deny that has already happened stays leaked.

Proposed fix

Redact once at the top of each branch and use that value for both the row and tool_name=, as _redacted_hook_block does:

title, _ = redact_exfiltration_urls(event.title)
title, _ = redact_credentials(title)

The nine sites are identical in structure within each path: reject, append a blocked row, audit the denial. #1902 consolidated the four hook-block instances of that shape into _reject_hook_blocked() so a path added later could not reintroduce the defect by omission, and the same consolidation applies here. It would leave one redaction site per path rather than nine.

Consolidating and redacting are separable. Redaction alone closes the leak; consolidation is what stops a tenth site from reopening it.

Scope boundary

Each site sits on a different permission path — auto-approve, trust-reads, trust/YOLO, interactive approval — so each needs its own test rather than one parametrised case. test_blocked_row_and_audit_redact_the_model_authored_title from #1902 is the template: drive the path with a credential-shaped title, then assert the credential appears in no transcript row and in no audit tool_name. Assemble the credential at runtime rather than as a single literal, so the fixture stays credential-shaped for the redactor without tripping scripts/scrub-lint.sh or Semgrep's detected-aws-access-key-id-value:

secret = "AKIA" + "1234567890ABCDEF"

Extending redaction to the audit calls changes audit content from the raw title to the redacted title, in the same way #1902 changed it on the hook-block path. A consumer that keys on exact tool names sees the redacted form for any title that contained a credential or a suspicious URL; titles containing neither are unchanged, because both functions return their input when nothing matches.

Metadata

Metadata

Assignees

Labels

area: agentsACP runtime, sub-agents, session lifecyclearea: dashboardDashboard UI and its backend handlersclaimedAn automated fix is in progresssecurityHardening, trust boundary, or sandbox escape

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions