Repository navigation
fix(redact): route JSONL payloads through the structure-aware pass - #244
Conversation
`redact_auto` picks between the JSON walker and the flat pass by parsing
the whole input as one `serde_json::Value` (`crates/atlas-redact/src/lib.rs`,
the `starts_with('{')` branch). JSONL parses as no single value, so every
ndjson tool result, `jq -c` run and transcript read a line at a time fell
through to `redact` even though `redact_json` has handled that shape since
it was written.
Three things went wrong on that path, all of them the reasons the walker
exists. Every `message_id` cleared the entropy threshold and came back
`[REDACTED]`. The tally counted one replacement per line rather than one
per secret, which inflates the "N secrets redacted" figure the promotion
and import confirmations show. And a flat span that ran past a JSON escape
left a line that no longer parses:
{"cmd": "psql \"host=db.internal user=app password=hunter2 sslmode=require\""}
came back as
{"cmd": "psql \"[REDACTED]""}
`is_jsonl` claims an input when it has two or more lines and every
non-empty one parses as its own object or array, and `redact_auto` hands
those to `redact_json`. The check is strict on purpose, so a prose payload
that merely opens with a brace keeps the flat path, and it stops at the
first line that does not parse, so a non-JSONL payload costs one line
rather than a second parse of the whole input.
`a_jsonl_payload_takes_the_structured_path_too` in `tests/auto.rs` covers
all three symptoms and fails on the unmodified source at the shredded id.
`a_multi_line_payload_that_is_not_jsonl_still_takes_the_flat_path` pins
the other direction, and the JSONL case is added to the existing
idempotence list.
|
@ahammadnafiz please review this |
ahammadnafiz
left a comment
ahammadnafiz
left a comment
There was a problem hiding this comment.
Reproduced everything in the description on d9ee007: atlas-redact 119 passed, clippy clean under -D warnings, atlas-checkpoint 365 passed, and with src/lib.rs reverted the new test fails at auto.rs:160 with "message id shredded". The dispatch was an oversight rather than a decision (the commit that added redact_auto never mentions line-delimited input), and record_tool_call passes the flattened tool output as raw text, so the motivating case is real. Merging.
One thing worth a follow-up, not a blocker. is_jsonl bails on the first line that does not parse, and Atlas itself produces that shape for large terminal output: OutputBuffer::push (crates/atlas-terminal/src/command.rs:104) drops from the front at a char boundary once output passes the 1 MiB limit, and terminal_output (crates/atlas-acp-thread/src/thread.rs:1985) prepends [earlier output dropped]\n. So a cat/tail of a transcript over 1 MiB gets all three symptoms back. With your fixture plus one cut-off fourth line:
redact_auto: count=6, both message_ids -> [REDACTED], line 3 -> {"cmd": "psql \"[REDACTED]""}
redact_json: count=4, both ids kept, lines 1-3 parse, only the cut line takes the flat fallback
redact_json's per-line fallback already handles this (tests/robustness.rs:173). The strictness is a fair call for a prose log that opens with a brace; something like "every line except the first and last must parse" keeps that guard and covers the truncation marker. A test with the marker would pin whichever way you go.
Separately, the escape-eating is a flat-layer bug that stays reachable for prose: the keyword-DSN value class in crates/atlas-redact/src/dsn.rs:51 is [^\s"']+ and does not exclude a backslash. the tool printed {"cmd": "psql \"host=... sslmode=require\""} and exited 0 still comes back with the stray quote. Adding \\ to that class is a one-character fix for its own PR.
Minor: each JSONL line is parsed twice (is_jsonl then redact_json), about 12% on a 200k-line payload in release. The walker is still twice as fast as the flat pass on the same input, so not worth restructuring.
What?
redact_autosent JSONL payloads to the flat redactor instead of the JSON walker.redact_jsonhas handled JSONL since it was written ("Accepts a single JSON value or JSONL (one value per line)"); only the dispatch in front of it could not name the shape, so the walker was never reached.Why?
The dispatch decides by parsing the whole input as one
serde_json::Value, and JSONL parses as no single value, so it fell through toredact. Three things follow, and they are the three reasons the walker exists.Capture::record_tool_callincrates/atlas-checkpoint/src/capture.rsscrubs every toolargumentsandresultthroughredact_auto, and its own doc comment names the first two: the flat pass "shreds the identifiers the walk exists to protect ({"message_id": …})", because every id clears the entropy threshold, and it counts one replacement per line rather than one per secret, which inflates the redaction tallyscrub_or_flaghands toadd_redaction_countsand the app shows on the promotion and import confirmations.The third is worse. A flat span that runs past a JSON escape leaves a line that no longer parses. This input:
came back as:
The keyword-DSN span ends at
require\, taking the backslash of the closing\"with it, so the string terminates early and the trailing quote is stray. The walker never has this problem because serde re-escapes the leaf on the way out.None of this is hypothetical for Atlas. ACP-hosted Claude Code writes JSONL transcripts that
crates/atlas-checkpoint/src/import.rsalready reads, and an agent that cats, tails or greps one of those files puts JSONL straight into a tool result, which is the highest-risk content in a Session.How?
is_jsonlclaims an input when it has two or more lines and every non-empty line parses as its own JSON object or array;redact_autohands those toredact_json. It is strict on purpose, so a payload that merely opens with a brace and then continues in prose keeps the flat path. It stops at the first line that does not parse, so a non-JSONL payload costs one line rather than a second parse of the whole input, and it only runs at all once the existing whole-input parse has already failed.a_jsonl_payload_takes_the_structured_path_tooincrates/atlas-redact/tests/auto.rscovers all three symptoms in one fixture: both ids survive, both secrets are gone, the count is 2 rather than 3, and every output line still parses. On the unmodified source it fails at the first shredded id.a_multi_line_payload_that_is_not_jsonl_still_takes_the_flat_pathpins the other direction, and the JSONL case is added to the existingredact_auto_is_idempotentlist.Verified on macOS 26.2 with rustc 1.95.0, running what CI runs per crate:
cd crates/atlas-redact && cargo test --lockedcd crates/atlas-redact && cargo clippy --locked --all-targetscd crates/atlas-redact && cargo clippy --locked --all-targets -- -D warningssrc/lib.rsreverted, tests kepta_jsonl_payload_takes_the_structured_path_tooFAILED, "message id shredded"cd crates/atlas-checkpoint && cargo test --lockedChecklist
atlas-checkpoint's suite instead, which is the crate that callsredact_auto.