audit: COLLIE_AUDIT_CONTENT=none redacts by field role, fail closed - #111
Merged
Conversation
…ping the body The audit trail records every write-level action, which is right — sending keystrokes to a pane is arbitrary code execution and who-did-what-when is worth keeping. It also records up to 120 characters of the message itself, and up to 120 characters of the prompt binding, which is a slice of whatever happened to be on the terminal at the time. For a personal deployment that is fine. For one where the trail is wanted but the bodies are not, there is currently no way to have one without the other. This adds one config value: COLLIE_AUDIT_CONTENT=preview (default, byte-for-byte today's behaviour) COLLIE_AUDIT_CONTENT=none every string inside `detail` becomes the length The envelope is untouched either way: ts, action, paneId, session, device, and every boolean and number inside detail. So submitted, textDelivered and promptBinding.checked/passed still answer the questions the log is for. Two small decisions, in case they are worth arguing about: - It keeps the LENGTH rather than a shorter preview. A truncated secret is still a secret, and the question a reader asks of a redacted trail is usually whether anything was sent at all, which a count answers. - It recurses, so nesting is covered. The prompt binding is nested and holds the terminal fragment; a redaction reaching only the top level would have left the more sensitive half in place. sanitize and formatAuditLine take the mode as a defaulted parameter, so every existing caller and test is unaffected. Tested: the existing suite passes unchanged (590 tests), plus four new cases covering the default being untouched, the envelope surviving, nothing of the message or the terminal surviving at any depth, and the length being preserved. Happy to adjust the naming, or drop the env var and take a constructor argument only, if you would rather not add configuration for this. (cherry picked from commit 497bf97)
`none` redacted every string in `detail`, which is the wrong axis: it decides
by what a value IS rather than where it CAME FROM, so it throws away the key
names and outcome flags that make the trail readable while a
content-bearing field added later under a new name is only covered by
accident of being a string.
Redaction is now by key name against a metadata allowlist in audit.ts, matched
at any depth (an array's members inherit the array's own key). A string
survives only if its key names an action parameter; everything else redacts,
so a field added later leaks nothing until someone deliberately classifies it.
Classification derived from every audit.record() call site in bridge/server.ts:
reply text content operator-typed message body
submit metadata flag
submitted metadata outcome flag
textDelivered metadata outcome flag
promptBinding.checked metadata flag
promptBinding.passed metadata flag
promptBinding.expected content slice of the pane screen
promptBinding.reason metadata enum
keys keys metadata key names ("ctrl+c")
sent metadata outcome flag
promptBinding.* as above
upload filename content client-declared, arbitrary
size metadata number
saved metadata server-generated name
tab.create workspaceId metadata id
label content operator-typed
cwd content operator-typed path
workspace.create label content operator-typed
cwd content operator-typed path
tab.rename tabId metadata id
label content operator-typed
tab.close tabId metadata id
pane.rename label content operator-typed
pane.close (no detail)
`label` and `cwd` are operator-typed, so they redact — a pane label can carry
whatever the operator felt like naming a thing, and a path is a disclosure.
A reader who needs them has the ids next to them. So does an upload's
`filename`: it is whatever the phone declared, while the server-generated
`saved` keeps the entry identifiable.
Also here:
- The redaction is the constant `⟨redacted⟩`, not `⟨n chars⟩`. An exact length
is content: an 8-character redaction next to a password prompt narrows the
secret, and "was anything sent" is answered by the entry existing at all.
- sanitize() drops function-valued properties, in BOTH modes. It used to copy
them through, and JSON.stringify then CALLS a copied own `toJSON` — an entry
carrying `{ toJSON: () => secret }` re-injected its return value into the
line, past the preview cap and past every redaction decision.
- AuditLog takes `(append, { now?, content? })` rather than three positional
slots — the slot after `append` is claimed by different arguments on
different branches, and a transposed argument here is a redaction quietly
turning itself off.
- config.ts: the new field's JSDoc had landed between deviceHeader's doc and
deviceHeader, orphaning it. Each doc sits above its own field again, and the
field is typed as AuditContent rather than a repeated literal union.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #107 — the feature and its name are @shuangwangnyc's (their commit is cherry-picked here with authorship preserved); the reshape on top changes how the redaction decides.
What changed relative to #107:
noneno longer redacts every string by type — a string survives only if its key is on a metadata allowlist (METADATA_KEYSinbridge/audit.ts); everything else redacts, so a detail field added later under a new name can never leak by being forgotten. Key names likekeys: ["ctrl+c"]stay legible;text,promptBinding.expected, labels, cwds, and the client-declared uploadfilenameredact.⟨redacted⟩, never⟨n chars⟩— an exact length is itself content (a 9-character redaction next to a password prompt narrows the secret).toJSONhole is closed in both modes —sanitizedrops function-valued properties, so a copied owntoJSONcan no longer re-inject unredacted content (or escape the 120-char preview cap) at stringify time.AuditLogtakes an options object instead of a third positional slot (the slot afterappendis claimed differently across branches).server.test.tscfg factory,.env.example/README/ARCHITECTURE pointers.No version bump — release deferred; the bump rides the next
chore(release):commit.Tests: 617 pass (root suite + collie-ctl lifecycle),
tsc --noEmitclean.🤖 Generated with Claude Code