Skip to content

Sanitize an unregistered attribution key in every violation producer - #513

Open
ronleizrowice-ant wants to merge 2 commits into
anthropics:mainfrom
ronleizrowice-ant:fix/sanitize-unregistered-violation-command
Open

Sanitize an unregistered attribution key in every violation producer#513
ronleizrowice-ant wants to merge 2 commits into
anthropics:mainfrom
ronleizrowice-ant:fix/sanitize-unregistered-violation-command

Conversation

@ronleizrowice-ant

Copy link
Copy Markdown
Contributor

Summary

resolveCommandText returned an unregistered attribution key verbatim, while the proxy path's own fallback already collapsed control characters. The key reaches the violation store over carriers the sandboxed process can write to (the observe socket's event field, the macOS log tag, the proxy username), so an unregistered key could put newlines or escape sequences into the <sandbox_violations> block through two of the three producers.

All three producers now resolve through one function, and the rule is uniform: every wrapped invocation registers its command text under its attribution key (the commandId when the embedder passes one, else the command itself — the same key the platform wrappers encode), and resolveCommandText returns that registered text unchanged for a key this process wrapped and control-character-collapsed text for anything else. Previously an invocation with no commandId, or one whose commandId equalled its text, skipped registration as redundant; with a sanitizing fallback that would have turned its own multi-line command into a single line in the report and for ignoreViolations matching, so those now register like any other.

This is the other half of what Claude Code currently carries as a dist patch on 0.0.73 (the first half is the ancestor pinning in #485 / its rebase); with both merged that patch can be dropped.

Test plan

New test/sandbox/command-text-registry.test.ts: registered id → verbatim text (control characters kept); id equal to text registers; an un-keyed invocation registers under its own command and resolves to the full text past the 100-character key; a key no invocation registered is control-stripped. registerCommandText / resolveCommandText are exported for the test, following the existing "Exported for testing" precedent in sandbox-utils.ts.

Driven end-to-end through the package entry on macOS (seatbelt log monitor) and Linux (seccomp observer + proxy): a multi-line command hitting a write deny reports its raw multi-line text in the violation's command with no commandId, with commandId equal to the command, and with a distinct commandId + commandText; a proxy request from inside the sandbox with a forged srt.<base64> username is recorded with the forged key's control characters collapsed.

…ucer

resolveCommandText returned an unknown key verbatim, so a violation
attributed through the observe socket or the macOS log tag could carry
control characters the proxy path already collapsed. All three producers
now resolve through the same registry-or-sanitize function, and an
invocation whose commandId equals its text registers that text so its
violations still report the raw multi-line command.

Remote-Dev: homespace
With the unknown-key fallback sanitizing, the trusted/untrusted split is
simplest as: a key this process wrapped resolves to the embedder's text,
anything else is forged and gets control characters collapsed. So drop
the "absent id skips registration" special case and key the registry by
commandId ?? command, the same key every platform wrapper encodes. An
un-keyed multi-line command now reports (and matches ignoreViolations
against) its raw text, past the 100-character key, like a keyed one.

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registering un-keyed commands under the truncated attribution key creates collisions: two commands that share the first 100 characters map to the same key, and the later registration overwrites the earlier trusted text. A delayed violation from the first invocation can then be attributed (and filtered by ignoreViolations) using the second command. Could un-keyed entries use a collision-resistant generated ID, or avoid treating a truncated command prefix as a trusted registry key?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants