diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 2f4f1ba..50dcf8d 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "throughline", "description": "Continuous, state-aware session memory for Claude Code. Captures what you did and what is (commands, file changes, decisions, and live git/PR state), then hands it off with judgment at session wrap-up. Readable, committable artifacts; binds to Claude's native memory.", - "version": "0.5.1", + "version": "0.5.2", "author": { "name": "Dynamic Agency", "email": "support@dynamicagency.com" diff --git a/CHANGELOG.md b/CHANGELOG.md index 91273f6..bedc4d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,49 @@ All notable changes to throughline are documented here. Format loosely follows [Keep a Changelog](https://keepachangelog.com/); this project uses semantic versioning. +## [0.5.2] + +Hot-path perf batch plus issue #15, which turned out to have a second, +un-cosmetic bug underneath it - caught by an independent code-review pass on +this release before merge. Verified by the full 143-assertion suite plus +shellcheck under both Homebrew and an `apt-get install shellcheck` Ubuntu +container. + +### Changed +- **One jq invocation per capture/prompt fire, not two**: `session-capture.sh` + and `session-prompt.sh` (the hottest hooks - every matched tool call and + every prompt submit) each separately resolved `session_id` via + `tl_resolve_sid` and then re-ran a full jq program to build the record line. + Both are now produced by a single jq call (`session_id` and the line joined + by a tab), removing the per-fire process. This matters most for + `session-prompt.sh`, which runs synchronously ahead of prompt processing. + The id is piped through the shared `clean` def (control-char/backtick + stripping) before joining, so the split is unambiguous even for a + session_id containing a literal tab; `tl_safe_sid` still runs exactly once, + as the single sanitizer. The split-and-sanitize step itself is a new shared + `tl_split_sid_line` helper in `_lib.sh`, not hand-duplicated between the two + hooks (an initial version did duplicate it verbatim - flagged by review and + factored out before merge, since that duplication is exactly the drift + class `tl_resolve_sid` itself exists to prevent). Cold-path hooks + (flush/precompact/onboard) are unchanged. + +### Fixed +- **Issue #15**: `redact()`'s generic keyword+separator rule left an orphaned + trailing quote on a quoted secret value (`password="X"` redacted to + `password=***"` instead of `password=***`). No security impact on its own - + the secret itself was always masked - purely malformed output. The + value-capture group now distinguishes a balanced quoted value (`"..."`, both + quotes consumed) from an unterminated one (opening quote with no matching + close) from a bare unquoted run. +- **Multi-word unterminated-quote leak** (pre-existing, found by review while + fixing #15 above): the unterminated-quote case's first fix 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 (`password=*** sesame`). This gap + predates #15 and was never specific to this release, but was caught here + because the new comment/CHANGELOG language claimed the unterminated case was + fully handled, and the case is now genuinely fully masked to match. + ## [0.5.1] Calmer follow-up pass on the three cleanups v0.5.0's review deferred rather diff --git a/hooks/_lib.sh b/hooks/_lib.sh index d206be8..b16b71b 100755 --- a/hooks/_lib.sh +++ b/hooks/_lib.sh @@ -128,6 +128,30 @@ tl_resolve_sid() { tl_safe_sid "$_tl_raw" } +# Split a "sidline" string - the shape session-capture.sh's and +# session-prompt.sh's single-jq-call optimization emits (session_id and the +# formatted record line joined by one literal tab, so each hot-path hook makes +# one jq call instead of two) - into the two pieces, sanitizing the id via the +# same tl_safe_sid that tl_resolve_sid uses. Shared here rather than +# hand-duplicated in both hooks: that duplication is exactly the drift class +# tl_resolve_sid itself exists to prevent (see its comment above) - a future +# change to the split or sanitization, edited in one hook but not mirrored in +# the other, would silently desync how the two hot-path hooks derive a session +# id from the identically-shaped jq output. +# Args: $1 = the "sidline" string. The id half is assumed to already have +# passed through the jq-side `clean` def before being joined (both callers do +# this), so it cannot itself contain a tab - the split below is unambiguous +# regardless of what the raw, pre-`clean` session_id contained. +# Sets (does not print) $_tl_split_sid and $_tl_split_line: an OUT-PARAMETER +# pair, not a same-named-temporary collision guard, so despite the `_tl_` +# prefix these are meant to be read by the caller immediately after the call - +# the same convention tl_active uses for $_tl_active_reason. +tl_split_sid_line() { + _tl_tab=$(printf '\t') + _tl_split_sid=$(tl_safe_sid "${1%%"$_tl_tab"*}") + _tl_split_line=${1#*"$_tl_tab"} +} + # Replace control characters (including newlines) with a space. Shared by # session-flush.sh and session-precompact.sh so their reason/trigger fields # can't break the `` marker they're embedded in. (The jq `clean` def in @@ -306,16 +330,32 @@ tl_jq_redact_defs() { # "password X" still mask X. That aggressiveness is CORRECT for commands but # WRONG for prose (it eats the word after any keyword+copula, inverting # "password is not the problem" -> "password is ***"), which is exactly why - # prompts use redact_prompt below instead. Value group has no @ or / - # exclusion - it matches the sentinel when present, else the unbounded run up - # to whitespace/quote - so a secret containing either char is fully masked. + # prompts use redact_prompt below instead. Value group tries, in order: a + # BALANCED quoted value ("..."), consumed whole (both quotes included in the + # match, so the replacement drops them entirely instead of leaving an + # orphaned trailing quote - issue #15); the sentinel; an UNTERMINATED quoted + # value (opening quote present but no closing quote anywhere after it, e.g. + # truncated/malformed input) - by the time this alternative is even tried, + # the balanced alternative has already proven no closing quote exists + # anywhere later in the string, so it is safe (and necessary) to consume the + # REST OF THE LINE rather than stopping at the first whitespace: an earlier + # version of this alternative stopped at whitespace like the bare-unquoted + # case below, which silently masked only the first WORD of a multi-word + # unterminated secret (`password="open sesame` -> `password=*** sesame`, + # leaking "sesame") while claiming the case was fully handled - excluding + # only \r/\n (not stopping at end-of-string) so it can't cross into a + # different line of a multi-line captured command; then the bare unquoted + # run, which DOES intentionally stop at the first whitespace (a bare, + # unquoted "password X" has no signal that X is meant to span multiple + # words, unlike an opening quote). No @ or / exclusion on the unquoted + # alternatives, so a secret containing either char is still fully masked. def redact: _pem | _auth_scheme | gsub("(?i)\\btoken\\s+(?[A-Za-z0-9._\\-]+)"; "Token ***") | _url | _prefix_tokens - | gsub("(?i)(?\\w*(?:token|secret|password|passwd|api[_-]?key|access[_-]?key|credential|auth(?:orization)?|client[_-]?id)\\w*)(?\\s*[:=]\\s*|\\s+(?:is|was|are)\\s+|\\s+)(?\"?(?:\(M)|[^\\s\"]+))"; "\(.k)\(.s)***") + | gsub("(?i)(?\\w*(?:token|secret|password|passwd|api[_-]?key|access[_-]?key|credential|auth(?:orization)?|client[_-]?id)\\w*)(?\\s*[:=]\\s*|\\s+(?:is|was|are)\\s+|\\s+)(?\"[^\"]*\"|\(M)|\"[^\\r\\n]*|[^\\s\"]+)"; "\(.k)\(.s)***") | _unmask; # Prose-safe redaction for user prompts (issue #5), and for the WebSearch # query / Task description branches in session-capture.sh (also prose). diff --git a/hooks/session-capture.sh b/hooks/session-capture.sh index 4149833..312798c 100755 --- a/hooks/session-capture.sh +++ b/hooks/session-capture.sh @@ -38,24 +38,37 @@ bufdir="$data/buffer" mkdir -p "$bufdir" 2>/dev/null || { tl_err "mkdir failed for buffer dir"; exit 0; } -# Session id is resolved independently of the formatted line below (rather than -# splitting both out of one combined jq call), via the shared tl_resolve_sid -# (also used by flush/precompact) so it is derived identically everywhere a -# buffer filename is keyed off it. -sid=$(tl_resolve_sid "$input") -# Drop records with no usable session id rather than poisoning a shared -# "nosession" bucket that flush never stamps and onboard re-warns about forever. -# Breadcrumbed like the other silent-loss paths below: currently unreachable -# (Claude Code always supplies a UUID session_id) but if that assumption ever -# breaks, the loss should be visible rather than untraceable. -[ -n "$sid" ] || { tl_err "dropped action: no usable session_id"; exit 0; } - # The redact/clean defs are shared with session-prompt.sh (via # tl_jq_redact_defs in _lib.sh) so the rule set can't drift between the two # capture-side hooks; the outcome($t) def and the tool-name dispatch below are -# specific to PostToolUse and stay here. Concatenated into one jq program so -# this remains a single jq invocation on the hot path. -line=$(printf '%s' "$input" | jq -r --arg root "$root" "$(tl_jq_redact_defs)"' +# specific to PostToolUse and stay here. +# +# Session id and the formatted line are produced by ONE jq invocation +# (sid-tab-line), not two: this is the hottest hook in the plugin (fires on +# every matched tool call), and a second full jq process just to re-derive +# .session_id was pure overhead. The trivial `.session_id // ""` expression +# below is byte-identical to tl_resolve_sid's own (_lib.sh) — only that +# expression is re-inlined here; the actual SANITIZER (tl_safe_sid) still runs +# exactly once, in shell, on the extracted id, same as tl_resolve_sid does. That +# distinction matters: the historical desync bug this consolidation must not +# reintroduce came from two DIFFERENT DERIVATION MECHANISMS (shell vs. jq) +# disagreeing on a session id, not from two identical jq expressions — see +# tl_resolve_sid's comment. Cold-path hooks (flush/precompact/onboard, which +# fire once per session/compaction rather than per tool) are unchanged and +# still call tl_resolve_sid directly. +# +# The id is piped through `clean` (control-char + backtick stripping, defined +# in tl_jq_redact_defs) before being joined with the tab delimiter, so a raw +# session_id that happened to contain a literal tab can never be misread as +# the sid/line boundary — tl_safe_sid (run by the shared tl_split_sid_line +# helper below, _lib.sh) maps every disallowed byte (control chars, tab, +# backtick, space, ...) to the same `_` regardless of whether `clean` already +# turned it into a space first, so this is a no-op for any session_id shaped +# like an actual UUID (the only shape Claude Code emits today) and produces +# the identical final sanitized id even in the currently-unreachable case of a +# stranger one. A regression test locks in that capture and flush agree on the +# filename for a tab-containing id. +out=$(printf '%s' "$input" | jq -r --arg root "$root" "$(tl_jq_redact_defs)"' # Observable outcome from the tool result. The Claude Code Bash tool_response # exposes "interrupted" but NOT an exit code, so a plain non-zero exit is not # visible to a PostToolUse hook and is deliberately left unmarked rather than @@ -74,7 +87,8 @@ line=$(printf '%s' "$input" | jq -r --arg root "$root" "$(tl_jq_redact_defs)"' or ((($r.exit_code? // $r.code? // $r.returncode? // 0) | tostring) != "0")) then " `[failed]`" else "" end; - (.tool_name as $t | + ((.session_id // "") | clean) as $sid + | ($sid + "\t" + (.tool_name as $t | if $t == "Bash" then "**bash** " + ((.tool_input.description // "") | redact | clean) + outcome($t) + " - `" + ((.tool_input.command // "") | redact | clean | clamp(200; "…[truncated]")) + "`" @@ -130,9 +144,22 @@ line=$(printf '%s' "$input" | jq -r --arg root "$root" "$(tl_jq_redact_defs)"' # other branches rely on to preserve a literal `*` in content like a # glob pattern or command. "**" + ($t | clean | gsub("\\*"; "")) + "**" + outcome($t) - end) + end)) ' 2>/dev/null) || { tl_err "jq filter failed"; exit 0; } +# tl_split_sid_line (_lib.sh) does the tab-split + sanitize; shared with +# session-prompt.sh so the two hot-path hooks can't drift on how they derive a +# session id from this identically-shaped jq output — see its comment. +tl_split_sid_line "$out" +sid=$_tl_split_sid +line=$_tl_split_line +# Drop records with no usable session id rather than poisoning a shared +# "nosession" bucket that flush never stamps and onboard re-warns about forever. +# Breadcrumbed like the other silent-loss paths below: currently unreachable +# (Claude Code always supplies a UUID session_id) but if that assumption ever +# breaks, the loss should be visible rather than untraceable. +[ -n "$sid" ] || { tl_err "dropped action: no usable session_id"; exit 0; } + [ -n "$line" ] || exit 0 tl_append_line "$bufdir" "$sid" "$line" diff --git a/hooks/session-prompt.sh b/hooks/session-prompt.sh index f1a25bc..59ab1bf 100755 --- a/hooks/session-prompt.sh +++ b/hooks/session-prompt.sh @@ -26,11 +26,16 @@ bufdir="$data/buffer" # why the breadcrumb lives at the data-dir root, not under buffer/. mkdir -p "$bufdir" 2>/dev/null || { tl_err "mkdir failed for buffer dir"; exit 0; } -# Same shared session-id derivation as capture/flush/precompact so the prompt -# line lands on the same buffer file as the actions it precedes. -sid=$(tl_resolve_sid "$input") -[ -n "$sid" ] || { tl_err "dropped prompt: no usable session_id"; exit 0; } - +# Session id and the formatted line are produced by ONE jq invocation +# (sid-tab-line, split by the shared tl_split_sid_line in _lib.sh), not two, +# for the same reason as session-capture.sh: this hook runs SYNCHRONOUSLY +# ahead of prompt processing (see below), so a second full jq process per +# keystroke-adjacent submit is exactly the wrong place to spend it. See +# tl_split_sid_line's comment for why joining on a `clean`-passed id is safe +# even for a session_id containing a literal tab. Cold-path hooks +# (flush/precompact/onboard) are unchanged and still call tl_resolve_sid +# directly. +# # Build the prompt line. Three deliberate choices, all different from the # command capture path: # 1. redact_prompt, NOT redact: prompts are prose, and the command-tuned @@ -42,13 +47,23 @@ sid=$(tl_resolve_sid "$input") # inside that window slips past redaction. The final clamp(200) is what # actually lands in the buffer; its ellipsis reflects the real length. # 3. Concatenated with the shared defs into one jq program (single invocation). -line=$(printf '%s' "$input" | jq -r "$(tl_jq_redact_defs)"' - ((.prompt // "") | clamp(2000; "") | redact_prompt | clean) as $p - | if ($p | gsub("^\\s+|\\s+$"; "")) == "" then "" - else "**prompt** " + ($p | clamp(200; "…[truncated]")) - end +out=$(printf '%s' "$input" | jq -r "$(tl_jq_redact_defs)"' + ((.session_id // "") | clean) as $sid + | ((.prompt // "") | clamp(2000; "") | redact_prompt | clean) as $p + | ($sid + "\t" + + (if ($p | gsub("^\\s+|\\s+$"; "")) == "" then "" + else "**prompt** " + ($p | clamp(200; "…[truncated]")) + end)) ' 2>/dev/null) || { tl_err "jq filter failed"; exit 0; } +# tl_split_sid_line (_lib.sh) does the tab-split + sanitize; shared with +# session-capture.sh so the two hot-path hooks can't drift on how they derive +# a session id from this identically-shaped jq output — see its comment. +tl_split_sid_line "$out" +sid=$_tl_split_sid +line=$_tl_split_line +[ -n "$sid" ] || { tl_err "dropped prompt: no usable session_id"; exit 0; } + # Empty / whitespace-only prompts produce no line. [ -n "$line" ] || exit 0 diff --git a/tests/run.sh b/tests/run.sh index 7974679..59bccc7 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -174,6 +174,36 @@ hasnt "URL credential with no nearby keyword is not stored" "$PLAIN_LINE" 'hunte hasnt "internal sentinel never leaks into the buffer" "$PLAIN_LINE" 'TLREDACTSENTINEL' has "URL host/path is preserved" "$PLAIN_LINE" 'example.com/data' +# 2e10. issue #15: a quoted secret value ("...") is fully masked with NO +# orphaned trailing quote left in the output - the value-capture group +# used to optionally consume a LEADING quote but never a matching +# trailing one, so `password="X"` redacted to `password=***"` (the +# secret itself was masked; only the stray quote was cosmetic, but it +# is malformed output). +cap '{"session_id":"T","tool_name":"Bash","tool_input":{"description":"quotedpw","command":"config: password=\"hunter2superlongvalue\""}}' +QUOTED_LINE=$(grep quotedpw "$BUF/session-T.md") +hasnt "quoted secret value is not stored" "$QUOTED_LINE" 'hunter2superlongvalue' +hasnt "no orphaned trailing quote after the mask" "$QUOTED_LINE" '***"' +has "quoted secret value is masked" "$QUOTED_LINE" 'password=***' + +# 2e11. an UNTERMINATED quoted value (opening quote, no closing quote anywhere +# after it - malformed/truncated input) is still masked rather than +# silently falling through to cleartext, matching this rule's +# pre-#15-fix behavior for exactly that shape. +cap '{"session_id":"T","tool_name":"Bash","tool_input":{"description":"unclosedpw","command":"config: password=\"hunter2superlongvalue"}}' +hasnt "unterminated quoted secret is not stored" "$(grep unclosedpw "$BUF/session-T.md")" 'hunter2superlongvalue' + +# 2e12. a MULTI-WORD unterminated quoted value is masked IN FULL, not just its +# first token - an earlier version of the unterminated-quote fallback +# stopped at the first whitespace (like the bare-unquoted case), which +# masked only "open" in `password="open sesame` and left "sesame" in +# cleartext right after the *** marker. +cap '{"session_id":"T","tool_name":"Bash","tool_input":{"description":"unclosedmultiword","command":"config: password=\"open sesame"}}' +MULTIWORD_LINE=$(grep unclosedmultiword "$BUF/session-T.md") +hasnt "second word of an unterminated multi-word secret is not stored" "$MULTIWORD_LINE" 'sesame' +hasnt "first word of an unterminated multi-word secret is not stored" "$MULTIWORD_LINE" 'open' +has "unterminated multi-word secret is masked in full" "$MULTIWORD_LINE" 'password=***' + # 2f. redaction applies to the Bash *description* field, not just command cap '{"session_id":"T","tool_name":"Bash","tool_input":{"description":"deploy with ghp_abcdefghij1234567890","command":"true"}}' DESC_LINE=$(grep '\*\*bash\*\* deploy' "$BUF/session-T.md")