v0.5.2: collapse hot-path jq spawns, fix orphaned-quote redaction (#15)#18
Merged
Merged
Conversation
…edaction session-capture.sh and session-prompt.sh (the hottest hooks - every matched tool call and every prompt submit) each ran jq twice: once via tl_resolve_sid to extract session_id, once to build the record line. Both now run one jq call (session_id + line joined by a tab, split in shell), which matters most for session-prompt.sh since UserPromptSubmit runs synchronously ahead of prompt processing. The id is passed through the shared `clean` def before joining so the split stays unambiguous even for a session_id containing a literal tab; tl_safe_sid remains the single sanitizer, run once in shell - only the trivial `.session_id // ""` jq expression is duplicated, not the sanitization logic that caused a real desync bug once before. Also fixes issue #15: redact()'s generic keyword rule left an orphaned trailing quote on a quoted secret value (password="X" -> password=***"). No security impact, cosmetic only. The value-capture group now distinguishes a balanced quoted value from an unterminated one (still masked, not silently left in cleartext) from a bare unquoted run. Verified: full 140-assertion suite (4 new) plus shellcheck under both Homebrew and an apt-get install shellcheck Ubuntu container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4iokjvAbBLgrRJYwCVtqb
…tion) Two confirmed findings from an independent review pass on the v0.5.2 diff: 1. redact()'s unterminated-quote fallback stopped at the first whitespace (like the bare-unquoted case), so a multi-word unterminated secret (password="open sesame, no closing quote) only masked its first word and left the rest in cleartext. Pre-existing (not introduced by this branch), but the branch's new comment/CHANGELOG language incorrectly claimed the case was fully handled, and the new test only covered a single-token value. Now consumes to end-of-line (safe once the balanced-quote alternative has already proven no closing quote exists later in the string), with a regression test for the multi-word case. 2. The sid/line tab-split-and-sanitize block was hand-copied verbatim into session-prompt.sh instead of being factored into a shared helper - reintroducing the exact duplication class _lib.sh's own history says caused a real desynced-filename bug previously. Factored into tl_split_sid_line in _lib.sh, called by both hot-path hooks. Verified: full 143-assertion suite (3 new) plus shellcheck under both Homebrew and an apt-get install shellcheck Ubuntu container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4iokjvAbBLgrRJYwCVtqb
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.
Summary
jqspawns per fire insession-capture.shandsession-prompt.sh(session_id viatl_resolve_sid+ a second call for the record line) into one, joined via a tab and split in shell. Matters most forsession-prompt.sh, which runs synchronously ahead of prompt processing.cleandef before joining, so the split stays unambiguous even for a session_id containing a literal tab (the exact historical desync bug class) —tl_safe_sidremains the single sanitizer, run once in shell. Only the trivial.session_id // ""jq expression is duplicated, matchingtl_resolve_sid's own.redact()'s generic keyword rule left an orphaned trailing quote on a quoted secret value (password="X"→password=***"). No security impact (secret was always masked), purely malformed output. Now distinguishes balanced-quoted / unterminated-quoted / bare-unquoted value shapes.Test plan
tests/run.sh: 140/140 passing (136 + 4 new assertions: quoted-value redaction, unterminated-quote fallback — both locally and in an Ubuntu container)shellcheck -s sh hooks/*.sh tests/*.sh: clean under Homebrew 0.11.0 AND apt-installed 0.9.0 in a freshubuntu:24.04container (the documented CI-parity trap from v0.5.0)tl_resolve_sid(flush-equivalent) independently derive the same sanitized filename — existing test 5d already locks this inCo-Authored-By: Claude Opus 5 noreply@anthropic.com