anthropics / anthropics/claude-code

PostToolUse rewrite collisions are last-registered-wins, not "last-write-wins" — and a clobbered redaction is silent

Aperta
#88,338 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
area:hooks area:security bug has repro
Lingua principale
Python
Stelle
145k
Fork
23.1k
Metriche di merge delle PR
Metriche PR in attesa

Descrizione

### What the docs say

TS SDK 0.3.237 added `PostToolUseHookSpecificOutput.classifierContext`. Its
JSDoc warns:

> (Do NOT return an identity rewrite just to pair an assertion: hooks run in
> parallel on the ORIGINAL output, so an identity rewrite competes
> last-write-wins with sibling rewrites and can clobber a real redaction.)

Two problems with this, both with security consequences.

### 1. "last-write-wins" implies a timing race; the behavior is deterministic

"last-write-wins" reads as completion order — i.e. whichever hook finishes last
overwrites. That suggests the hazard is a nondeterministic race you cannot
design around.

Measured on **CLI 2.1.234**, it is deterministic **last-registered-wins**:

| Scenario | Redaction hook | Identity-rewrite hook | Result |
|---|---|---|---|
| A | registered 1st, 50ms | registered 2nd, 250ms | **redaction lost** (credential reached the model) |
| C | registered 2nd, 50ms | registered 1st, 250ms | redaction survived |

C inverts registration against completion: the identity rewrite still finishes
last, but now registered first — and the redaction survives. So completion
order does not decide the winner; registration order does. Stable across
repeated runs. This matches the merge rule for `PreToolUse` `updatedInput`.

This distinction is actionable and currently undocumented: because it is
deterministic, hosts *can* protect themselves by registering security-relevant
rewrites last. The current phrasing tells them the opposite — that it is a race.

### 2. The warning is on the wrong field, and the discard is silent

The hazard belongs to `updatedToolOutput`, but it is documented only in the
JSDoc of `classifierContext` — a different field, added later, in a ~2000-character
paragraph. An author writing a redaction hook has no reason to read it.

And when two hooks return conflicting `updatedToolOutput`, the losing rewrite is
dropped with no signal: no warning, no diagnostic. For a *redaction*, that is a
silent data leak — a plugin or org-level hook appended after a redactor quietly
un-redacts its output.

### Suggestions (in priority order)

1. Document the merge rule on `updatedToolOutput` itself (and `updatedInput`),
stating it is **last-registered-wins**, with the explicit guidance to
register redaction hooks last.
2. Replace "last-write-wins" with "the last-registered hook's rewrite wins" —
the current wording actively misleads about whether it is designable-around.
3. Emit a warning when two hooks return conflicting `updatedToolOutput` for the
same call. Silent discard of a security-relevant rewrite is the failure mode
worth eliminating.

### Reproduction

The merge happens CLI-side, so this is not specific to any one SDK.

1. Register two `PostToolUse` hooks on the same matcher (e.g. `Bash`).
2. Have the tool produce output containing a recognizable secret, e.g.
`echo 'deploy config: API_KEY=sk-live-DEADBEEF'`.
3. Hook **1** returns `hookSpecificOutput.updatedToolOutput` with the secret
masked (a genuine redaction).
4. Hook **2** returns `hookSpecificOutput.updatedToolOutput` set to the
*original, unmodified* output — the "identity rewrite" the JSDoc describes.
5. Inspect the `tool_result` the model receives.

Observed on CLI 2.1.234: the `tool_result` contains the **unmasked** secret.
Swap the two hooks' registration order and the mask survives, with the timing
held constant — which is what isolates registration order from completion
order. Making hook 2 the slower of the two in both orderings rules out
completion order entirely.

### Note on `classifierContext` itself

Not part of this report, just context: the field appears inert on CLI 2.1.234 —
zero occurrences in the CLI binary and zero in `sdk.mjs` 0.3.237, so upstream's
`.d.ts` is currently ahead of both the CLI and the TS runtime. The collision
above is live today regardless.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Direzione di ricerca

Start with the JSDoc for PostToolUseHookSpecificOutput.updatedToolOutput and updatedInput, then reproduce the two-hook collision described on CLI 2.1.234. Update the documentation to state the last-registered-wins rule and redaction guidance; if the warning request is included, confirm its intended scope before changing behavior.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
typescript
Ambito
cli, documentation, security
Tipo di issue
Documentazione
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Attiva
Chiarezza
Abbastanza chiara
Idoneità per principianti
48/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.