Skip to content

SUB-185: bounded redacted tool-result capture - #1

Merged
Pavel-Tk merged 2 commits into
mainfrom
ptkachykby/sub-185-plugin-tool-capture
Aug 20, 2026
Merged

SUB-185: bounded redacted tool-result capture#1
Pavel-Tk merged 2 commits into
mainfrom
ptkachykby/sub-185-plugin-tool-capture

Conversation

@Pavel-Tk

Copy link
Copy Markdown
Contributor

Summary

  • parse transcript blocks before bounding, scan full blocks with streaming overlap, quarantine any credential-bearing block, then apply a 65,536-byte UTF-8 ceiling and digest
  • emit separate five-role messages for text, tool calls, and tool results with call-ID pairing, tool-name backfill, provenance reason codes, and the approved capture metadata
  • capture sidechains by default with collision-free record/block coordinates and session ancestry; retain an explicit config/environment kill-switch
  • reserve spool capacity for boundary events, refuse newest events under pressure, count duplicates, and persist content-free eviction/quarantine/drop/duplicate status counters
  • preserve capture schema v2 and the 262,144-byte canonical event ceiling

Tests

  • uv run ruff check .
  • uv run pytest -q — 131 passed
  • uv run python scripts/verify_public_plugin_candidate.py --root . — no findings
  • uv run python -m compileall -q src scripts

No release, tag, or SHA256SUMS change is included. Release work follows review.

Linear: SUB-185

@Pavel-Tk Pavel-Tk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Adversarial credential-safety review — REQUEST CHANGES

I found 4 CRITICAL blockers. Branch checks pass (131 passed, ruff, candidate verifier, compileall), but the exploit suite is 10 failed, 7 passed.

CRITICAL 1: scanning resets at tool-result structure boundaries

_tool_result_text() inserts "\n" between inner text blocks, while _parse_record() scans each top-level block independently. I split one configured synthetic credential after super-secret-:

  • across two top-level tool_result blocks; and
  • across two inner text blocks in one tool_result.

Both raw halves were retained. A receiver can reassemble the credential. The redaction stream must cover the original structural boundaries (or conservatively quarantine adjacent pieces); joining first with an inserted character and scanning envelopes independently does not meet the split-block invariant.

CRITICAL 2: new tool identity fields are sliced before credential scanning

At transcript.py:249-250,267, tool_use.id, tool_use.name, and tool_result.tool_use_id are sliced to 512 characters before normalization/redaction. With a valid 600-character configured ANTHROPIC_API_KEY placed in the new id/name fields, the event retained the raw 512-character prefix in tool_call_id, tool_name, and source_identity; the block was not quarantined. Scan the complete block, including metadata, before any bound.

CRITICAL 3: pairing is not exact, one-to-one, or lineage-scoped

At transcript.py:284-314:

  • an orphan clears tool_name/source_identity but retains tool_call_id; the current schema-v2 server contract treats that fallback as attributable identity;
  • one call plus two results with the same id attributed both results as Read;
  • a root call plus a different sidechain's result using the same id attributed the sidechain result as TrustedTool.

A trustworthy pair must be exactly one call and one result in the same lineage/structural context. Every orphan/ambiguous path must remove all server-attributable fallback identity, not just tool_name and source_identity.

CRITICAL 4: process-local spool locking can consume the boundary reserve and erase counters

Each hook constructs a new DurableSpool, but admission and stats use only a per-instance threading.Lock. A deterministic two-instance race let both ordinary writes observe the same available slot. The spool reached 3/3 ordinary entries and the next session_end was refused with spool capacity unavailable.

The same stale-instance pattern erased a persisted loss: instance A wrote dropped=1; instance B then recorded a duplicate from its stale counter snapshot and rewrote the file to dropped=0, duplicates=1. Restart preserved the incorrect zero.

Also, for the supported max_bytes=300000, a 220KB ordinary item was admitted but only 75KB was reserved, so a legal 80KB pre_compress event (below the 262,144 event cap) was refused. Admission + write and counter read-modify-write need a root-scoped interprocess transaction. Every accepted spool configuration must reserve one maximum legal boundary event.

Verified passing cases

The former 65,536 boundary, tool-use input secret, UTF-8 adjacency, orphan-result secret quarantine, blank-content digest, sidechain kill-switch, normal single-instance counter restart, post-redaction 262,144 cap, and exact new schema-v2 payload acceptance by the current server all passed.

Full reviewer log: /tmp/review185_log.md on the review host.

@Pavel-Tk Pavel-Tk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Security re-check — commit 908d197

OVERALL VERDICT: PASS. I could not reproduce any of the four previously reported credential/capture safety defects. I used fresh, out-of-tree probes rather than the committed regression tests.

  1. Cross-block and 512-boundary credential detection — PASS. A new synthetic configured credential was split across two adjacent text blocks; both blocks were fully quarantined (content="", retained_bytes=0, redaction_codes=["credential_detected"]). It was also split (a) across the 512-character cutoff inside a tool id and (b) exactly between a 512-character tool id and its adjacent tool name. Both whole tool blocks were quarantined and all tool identity fields were cleared.
  2. Full tool id/name scanning before bounds — PASS. Separate fresh cases placed the complete credential after offsets 577 in id and 603 in name. Both were detected before bounding; each block was emptied and tool_call_id, tool_name, and source_identity were null.
  3. Unattributable tool results — PASS. Fresh cases produced: unmatched result → orphaned_tool_result; two calls sharing one id → ambiguous_tool_call_id; root call with sidechain result → cross_stream_tool_result. In all three results, tool_call_id, tool_name, and source_identity were null.
  4. Real cross-process spool race/accounting — PASS. Two independently initialized OS worker processes raced against the same spool. At the reserved slot, a pre_compress boundary was accepted while the ordinary event was refused; stored ids contained both seeds plus the boundary, with dropped=1. In a second simultaneous two-process refusal race, both refusals were preserved atomically as dropped=2. Accounting balanced exactly: attempted = stored + dropped in both phases; no unrecorded loss.

Full plugin suite: 148 passed in 6.54s.

No merge was performed.

@Pavel-Tk
Pavel-Tk merged commit aa2752f into main Aug 20, 2026
3 checks passed
@Pavel-Tk
Pavel-Tk deleted the ptkachykby/sub-185-plugin-tool-capture branch August 20, 2026 14:22

@Pavel-Tk Pavel-Tk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Follow-up review of 908d197 — REQUEST CHANGES (2 CRITICAL remain)

I re-ran the original regression set and added fresh boundary/process variants.

Resolved and verified:

  • the original same-record top-level and nested block splits;
  • secrets spanning the former 65,536 content bound and the new 512-character tool-field bound, including UTF-8;
  • tool input/orphan quarantine and content-free digest;
  • orphan, duplicate-result, and ordinary cross-sidechain attribution cases;
  • session/scope redaction and 262,144-byte event cap;
  • small-spool full boundary reservation;
  • a real two-OS-process admission race and six-process counter RMW/restart race.

The full branch suite passes: 148 passed. The committed adversarial regression file passes: 17 passed. My fresh suite is 8 passed, 2 failed, exposing these remaining blockers:

CRITICAL 1: joint scanning still resets at each JSONL record

The joint scan at transcript.py:271-274 covers only one record's message.content. I split configured fresh-secret-0123456789 between two adjacent tool_result records in the same session/sidechain. Both raw halves remained in the two emitted messages. The receiver can reconstruct the exact configured credential.

The security invariant was specifically that no path leaves partial credential fragments across tool-result boundaries. Moving the boundary from two blocks in one record to two blocks in adjacent records must not restore the leak. The scan/overlap must span the transcript's adjacent visible block stream, not reset per JSONL record; a detection must quarantine every contributing block.

CRITICAL 2: provenance lineage is computed from 512-character wire-bounded values

_coordinates() bounds agent_id, then _lineage_key() at transcript.py:354-363 uses that bounded value for trust pairing. I used two distinct concurrent sidechain agent IDs:

"a" * 512 + "-one"
"a" * 512 + "-two"

A tool_call(id="shared", name="TrustedTool") in the first and a forged tool_result(tool_use_id="shared") in the second collided after bounding. The forged result retained tool_call_id="shared", tool_name="TrustedTool", and source_identity="TrustedTool" with no reason code.

Use a non-emitted, content-free digest of the complete redacted/raw lineage coordinate for pairing, or another collision-resistant full-coordinate key. Do not make provenance decisions from display-bounded fields.

Counts

  • Full plugin suite: 148 passed, 0 failed
  • Committed review regressions: 17 passed, 0 failed
  • Fresh variants: 8 passed, 2 failed
  • Remaining CRITICAL findings: 2

Artifacts: /tmp/review185_reverify_fresh.py, /tmp/review185_reverify_fresh_output_final.txt, /tmp/review185_reverify_full_suite.txt.

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.

1 participant