From 1f2f7b22d9a2945a2bed7e8d4b6186d3e9cd59c6 Mon Sep 17 00:00:00 2001 From: Jason Irish Date: Sun, 5 Jul 2026 16:49:26 -0500 Subject: [PATCH 1/6] feat: v0.5.0 - P1 capture-fidelity batch (#5, #6, #7) The P1 findings from the v0.4.0 audit. The buffer now records why (user intent), captures a wider slice of what a session does, and stamps compaction seams idempotently. A fifth hook joins the set. - #5: UserPromptSubmit -> session-prompt.sh captures redacted, truncated user prompts. Intent previously lived only in the compactable conversation. - #6: PostToolUse matcher widened to Grep/WebFetch/WebSearch/Task/Agent/mcp__.*; Read/Glob deliberately excluded. MCP tools captured name-only. - #7: precompact stamp made idempotent (tail -n 1 guard), mirroring flush. Shared refactor: redact/clean defs and the _tl_err breadcrumb moved into _lib.sh (tl_jq_redact_defs, tl_err, tl_append_line, clamp) so the two capture-side hooks can't drift. Command redact stays byte-identical. Pre-merge high-effort review fixes: - Prose-safe redact_prompt: the command-tuned redact corrupts/inverts English ("password is not the problem" -> "password is ***"). Prompts now use a restricted path (structural token formats + explicit key:value/key=value secrets only); command path unchanged. - Onboard no longer counts prompt-only buffers as unconsumed (a session with no captured action has nothing to distill), preserving the warning's signal. - Prompt clamped to 2000 chars before redaction (multi-MB paste latency). - Task empty-string description falls through to prompt (jq // only skips null). Tests: 95 -> 121 assertions. shellcheck + JSON manifest checks green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp --- .claude-plugin/plugin.json | 2 +- CHANGELOG.md | 72 +++++++++++++++++ README.md | 17 ++-- hooks/_lib.sh | 157 +++++++++++++++++++++++++++++++++++- hooks/hooks.json | 13 ++- hooks/session-capture.sh | 145 ++++++++++++--------------------- hooks/session-onboard.sh | 9 +++ hooks/session-precompact.sh | 15 ++++ hooks/session-prompt.sh | 56 +++++++++++++ tests/run.sh | 102 +++++++++++++++++++++++ 10 files changed, 484 insertions(+), 104 deletions(-) create mode 100755 hooks/session-prompt.sh diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 9c0c7e7..e36bd1b 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.4.1", + "version": "0.5.0", "author": { "name": "Dynamic Agency", "email": "support@dynamicagency.com" diff --git a/CHANGELOG.md b/CHANGELOG.md index cf779e4..99c3add 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,78 @@ All notable changes to throughline are documented here. Format loosely follows [Keep a Changelog](https://keepachangelog.com/); this project uses semantic versioning. +## [0.5.0] + +The P1 capture-fidelity batch out of the v0.4.0 audit (docs/AUDIT-v0.4.0.md, +issues #5/#6/#7). The buffer now records *why* (user intent), captures a much +wider slice of what a session actually does, and stamps compaction seams +idempotently. A fifth hook joins the set. + +### Added +- **Prompt capture** (`UserPromptSubmit` -> `hooks/session-prompt.sh`, issue #5): + each user prompt is appended to the session buffer as a redacted, 200-char + truncated `**prompt**` line. Intent previously lived only in the compactable + conversation - exactly what a context compaction destroys - so the buffer + recorded what happened but never why. Shares the same redaction/cleaning + pipeline as capture, so a pasted credential in a prompt is masked by the same + rules. Fail-open: every failure path exits 0 (a non-zero UserPromptSubmit hook + would abort the prompt) and breadcrumbs to `.capture-errors`. +- **Widened action capture** (issue #6): the `PostToolUse` matcher now also + covers `Grep`, `WebFetch`, `WebSearch`, `Task`/`Agent` (subagents), and MCP + tools (`mcp__.*`). Each high-signal read-side tool emits one redacted + one-liner (grep pattern, fetched URL, search query, or subagent type + + description); MCP tools are captured name-only, making zero assumptions about + their input schema so no field can leak. `Read` and `Glob` are deliberately + NOT matched - the noisiest tools, whose capture would swamp the buffer. + Research-heavy sessions previously left almost no trace. SubagentStop boundary + markers were considered and deferred (see issue #6). + +### Changed +- The jq redaction/cleaning defs (`redact`/`clean`, ~15 gsub rules and their + full comment history) moved from `session-capture.sh` into a shared + `tl_jq_redact_defs` helper in `hooks/_lib.sh`, prepended to each capture-side + hook's jq program so the rule set can't drift between the prompt and action + hooks. Same consolidation reasoning as `tl_resolve_sid`; each hook stays a + single jq invocation. The `_tl_err` breadcrumb helper was likewise promoted to + a shared `tl_err`. This refactor is behavior-neutral - the pre-existing + redaction suite passes unchanged, proving it. + +### Fixed +- **Precompact stamp idempotency** (issue #7): `session-precompact.sh` now skips + the boundary write when the buffer already ends with a compaction-boundary + marker (a `tail -n 1` guard, mirroring `session-flush.sh`'s end-stamp guard), + so a double `PreCompact` fire for one seam writes one marker - while a long + session with several genuine compactions still stamps each one, because a + captured action landing between them moves the marker off the last line. + +### Review fixes (pre-merge, high-effort code review) +- **Prose-safe prompt redaction**: prompts run through a new `redact_prompt` + (structural token formats + explicit `key:value`/`key=value` secrets only), + not the command-tuned `redact` whose copula/bare-space and bearer/token WORD + rules corrupt and can invert ordinary English ("password is not the problem" + -> "password is ***"). The command path is unchanged. A bare-word secret with + no prefix and no colon is deliberately left for the handoff human re-scan. +- **No nag for prompt-only sessions**: `session-prompt.sh` records a line for + every session, so onboard's unconsumed-buffer counter now skips buffers with + no captured ACTION line (Q&A / Read-Glob-only sessions have nothing to + distill), preserving the warning's signal. +- **Prompt latency bound**: the prompt is clamped to 2000 chars before the + redaction passes (then to 200 for storage), so a multi-MB paste isn't fully + regex-scanned in the synchronous UserPromptSubmit path. +- **Task empty-description fallback**: an empty-string `description` now falls + through to `prompt` (jq `//` only skips null), so delegated intent isn't lost. +- **Duplication removed**: shared `tl_append_line` (buffer write + breadcrumb) + and a jq `clamp($n; $ell)` truncation def, so the write sequence and the + redact|clean|truncate idiom are single-sourced across capture surfaces. + +### Tests +- 26 new assertions (95 -> 121): prompt capture (line shape, prose-safety, + colon-form + shape-token redaction, truncation, empty-prompt skip, + opt-out/kill-switch, no-session breadcrumb); widened capture + (grep/webfetch/websearch/task/mcp one-liners, URL-userinfo masking, empty-desc + task fallback, mcp input not read); prompt-only buffers not counted as + unconsumed; precompact idempotency (double-fire, multi-seam, post-boundary). + ## [0.4.1] P0 fixes out of the v0.4.0 audit (docs/AUDIT-v0.4.0.md). The audit's live finding diff --git a/README.md b/README.md index 834afbb..ed327a7 100644 --- a/README.md +++ b/README.md @@ -39,10 +39,14 @@ handoff, and binds into Claude Code's native memory system. Three layers, each doing what it's actually capable of: -1. **Continuous capture** (`PostToolUse` hook) - appends a structured one-liner per - mutating action to a per-session buffer: the command or file, flagged if it was - interrupted, with obvious secrets masked before anything is written. Mechanical - and cheap. You never see it; it just protects you. +1. **Continuous capture** (`UserPromptSubmit` + `PostToolUse` hooks) - appends a + structured one-liner per user prompt and per captured action to a per-session + buffer: the intent behind the work, then the command, file, search, fetch, or + delegated task, flagged if it was interrupted, with obvious secrets masked + before anything is written. Mutating tools (Bash/Edit/Write/NotebookEdit) plus + high-signal read-side tools (Grep/WebFetch/WebSearch/Task/agents) and MCP tools + are captured; the noisiest (Read/Glob) are deliberately skipped so the buffer + stays skimmable. Mechanical and cheap. You never see it; it just protects you. 2. **Judged handoff** (`throughline-handoff` skill) - at wrap-up, the agent distills the buffer + context into a durable `HANDOFF.md` and a timestamped session log, and promotes any durable facts into native memory. This is the judgment layer, so @@ -148,7 +152,7 @@ profile): export THROUGHLINE_DISABLE=1 ``` -Any value other than `0` disables **all four hooks completely** - no capture, no +Any value other than `0` disables **all five hooks completely** - no capture, no SessionStart block (not even about existing data), no end-stamps. This is stricter than `.throughlineignore`, which keeps orienting toward already-existing content. Unset it (or set `0`) to re-enable; existing data is untouched either way. @@ -197,8 +201,9 @@ throughline/ │ └─ marketplace.json ├─ hooks/ │ ├─ hooks.json -│ ├─ _lib.sh # data-dir resolution + activation gate + jq/sid helpers +│ ├─ _lib.sh # data-dir resolution + activation gate + jq/sid/redaction helpers │ ├─ session-onboard.sh # SessionStart: pointer, git state, compaction recovery +│ ├─ session-prompt.sh # UserPromptSubmit: redacted, truncated user-intent line │ ├─ session-capture.sh # PostToolUse: structured action buffer (outcome + redaction) │ ├─ session-precompact.sh # PreCompact: stamp the compaction-boundary marker │ └─ session-flush.sh # SessionEnd: safety-net stamp diff --git a/hooks/_lib.sh b/hooks/_lib.sh index a10d6a0..d444557 100755 --- a/hooks/_lib.sh +++ b/hooks/_lib.sh @@ -130,11 +130,164 @@ tl_resolve_sid() { # 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. (capture.sh's `clean` -# def duplicates this rule rather than calling it: capture applies control-char +# can't break the `` marker they're embedded in. (The jq `clean` def in +# tl_jq_redact_defs below duplicates this rule rather than calling it: the +# capture-side hooks apply control-char # AND backtick stripping together, per-field, inside one jq pipeline that also # does redaction — pulling just the control-char half out to a separate shell # call there would mean an extra process per field for no real safety gain.) tl_clean_ctrl() { printf '%s' "$1" | tr '[:cntrl:]' ' ' } + +# Best-effort breadcrumb for the swallowed-failure paths in the capture-side +# hooks (session-capture.sh, session-prompt.sh): capture must never block a +# tool or a prompt, so every failure there still exits 0, but a write failure +# (full disk, lost permissions) would otherwise drop a record with zero trace. +# onboard surfaces this file's presence so the loss isn't silent forever. +# Lives at the data-dir root, NOT under buffer/: tl_active guarantees the data +# dir exists before any caller reaches an error path, but buffer/ may not +# (its mkdir can be the very failure being reported) - if the breadcrumb's own +# target depended on buffer/, the one failure mode this exists to report +# (buffer/ uncreatable) would silently defeat the breadcrumb along with it. +tl_err() { + printf -- '%s %s\n' "$(date '+%Y-%m-%d %H:%M:%S')" "$1" >> "$(tl_data_dir)/.capture-errors" 2>/dev/null +} + +# Append one timestamped record line to a session buffer, breadcrumbing on write +# failure. Shared by the capture-side hooks (session-capture.sh, +# session-prompt.sh) so the on-disk line format ("- `` ") and the +# write-failure handling live in exactly one place - a prior version spelled +# this sequence out identically in both hooks, the same hand-duplicated drift +# class tl_resolve_sid and the redact defs exist to prevent. +# Args: $1 = buffer dir, $2 = sanitized session id, $3 = formatted content. +tl_append_line() { + _tl_ts=$(date '+%Y-%m-%d %H:%M:%S') + # Backticks are literal markdown; %s are printf specifiers; the format string + # is intentionally not shell-expanded. + # shellcheck disable=SC2016 + printf -- '- `%s` %s\n' "$_tl_ts" "$3" >> "$1/session-$2.md" 2>/dev/null \ + || tl_err "write failed for session-$2" +} + +# Shared jq redaction/cleaning defs for the capture-side hooks. Printed as jq +# program text and prepended to each hook's tool-specific jq body, so every +# hook stays a single jq invocation (the hot-path constraint) while the rule +# set itself cannot drift between hooks - the same consolidation reasoning as +# tl_resolve_sid, which exists because a hand-duplicated derivation already +# caused one real desync bug. Quoted heredoc: the def text passes through +# with no shell expansion of its backslashes or quotes. +tl_jq_redact_defs() { + cat <<'TL_JQ_DEFS' + # Mask common secret shapes so raw credentials never sit in the buffer. The + # buffer is gitignored, but a later handoff distills it into committed logs; + # this is defense-in-depth, not the only barrier (the skill scrubs too). + # Best-effort: this is pattern/keyword matching, not entropy analysis, so a + # bare opaque token with no recognizable shape or keyword will not be caught. + # Known gap, not fixable here without a high false-positive cost: a + # credential attached to a bare single-letter CLI flag with no keyword at all + # (mysql -p, curl -u user:pass) has no keyword for this rule to + # anchor on, and flags like -u/-p are too overloaded across tools (docker run + # -u uid:gid, ssh -p ) to redact generically. The handoff skill re-scan + # is the second barrier for exactly this shape. + # + # Internal sentinel for one specific hand-off: the URL-userinfo rule below is + # the only shape rule whose replacement abuts a non-whitespace character (the + # @ that starts the host). Every other shape rule replacement is naturally + # whitespace-bounded, so the generic catch-all further down can never run + # past it by accident. The userinfo rule alone needs to mark its output as + # "already redacted" so the generic value-capture stops there instead + # of continuing past the @ into the host/path - this used to be inferred from + # boundary characters (first @, then briefly /) instead of stated directly, + # and each version of that inference either deleted real trailing context or + # under-redacted a genuine secret that happened to contain the same boundary + # character. The sentinel removes the inference: the generic rule recognizes + # it explicitly, and the final gsub converts it (and any sentinel a URL with + # no nearby keyword never reached the generic rule to convert) to the + # user-facing *** mark. + def M: "TLREDACTSENTINEL"; + # Structural credential formats factored out so BOTH the command path + # (`redact`) and the prompt path (`redact_prompt`) share them without + # duplication. These are the rules that match distinctive shapes - PEM + # blocks, URL userinfo, and the well-known token PREFIXES (ghp_, sk-, AKIA, + # ...) - and therefore never fire on ordinary English. They are the ONLY + # rules safe to run over natural-language prompt text (see redact_prompt). + # _pem and _prefix_tokens are order-independent; _url sets the sentinel M so + # it must run before any generic value-capture and before the final _unmask. + def _pem: + gsub("-----BEGIN [A-Z ]*PRIVATE KEY-----[\\s\\S]*?-----END [A-Z ]*PRIVATE KEY-----"; "***private-key-redacted***") + | gsub("-----BEGIN [A-Z ]*PRIVATE KEY-----[\\s\\S]*"; "***private-key-redacted***"); + def _url: + gsub("(?://[^:@/\\s]+):(?[^@/\\s]+)@"; "\(.pfx):\(M)@"); + def _prefix_tokens: + gsub("ghp_[A-Za-z0-9]{10,}"; "ghp_***") + | gsub("github_pat_[A-Za-z0-9_]{10,}"; "github_pat_***") + | gsub("gh[oprsu]_[A-Za-z0-9]{10,}"; "gh_***") + | gsub("xox[baprs]-[A-Za-z0-9-]{6,}"; "xox-***") + | gsub("sk-[A-Za-z0-9_-]{10,}"; "sk-***") + | gsub("AKIA[0-9A-Z]{12,}"; "AKIA***") + | gsub("AIza[0-9A-Za-z_\\-]{35}"; "AIza***"); + def _unmask: gsub("\(M)"; "***"); + # Full command-path redaction. Shape-specific patterns run BEFORE the generic + # keyword=value catch-all, so e.g. "Authorization:Basic " is fully + # consumed by the Basic-auth rule rather than the generic auth keyword rule + # eating just the Basic token and leaving the base64 payload exposed. Same + # reasoning for the dedicated Token rule (DRF/GitLab-style "Authorization: + # Token " headers): without it, the generic rule treats the scheme + # word "Token" itself as the value to mask and leaves the real key in + # plaintext right after it. The bearer/token word rules run BEFORE + # _prefix_tokens so a "bearer ghp_..." is consumed whole rather than the + # prefix rule masking the ghp_ body first and the bearer rule then re-masking + # the leftover. The generic keyword group has trailing \w* too (not just + # leading) so compound names like SECRET_KEY= or API_KEY_VALUE= still match. + # Its separator group accepts a copula (is/was/are) and bare whitespace in + # addition to :/= so command descriptions phrased as "password is X" / bare + # "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. + def redact: + _pem + | gsub("(?i)bearer\\s+(?[A-Za-z0-9._\\-]+)"; "Bearer ***") + | gsub("(?i)\\btoken\\s+(?[A-Za-z0-9._\\-]+)"; "Token ***") + | _url + | _prefix_tokens + | gsub("(?i)\\bbasic\\s+[A-Za-z0-9+/=]{8,}"; "Basic ***") + | 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)***") + | _unmask; + # Prose-safe redaction for user prompts (issue #5). Prompts are natural + # language, not commands: running them through `redact` corrupts and can + # invert ordinary English (the copula/bare-space generic rule and the + # bearer/token WORD rules all fire on prose - "token refresh flow" -> + # "Token *** flow"), destroying the very intent the feature exists to + # preserve. So this path keeps ONLY the structural formats that never match + # English (_pem/_url/_prefix_tokens) plus a generic keyword rule RESTRICTED + # to explicit key:value / key=value separators - which still catches a + # credential pasted as "password: hunter2" or "API_KEY=xxx" while leaving + # "the password is not the problem" untouched. A pasted secret with no + # recognizable prefix AND no colon/equals (e.g. a bare word after "password + # is") is deliberately NOT masked here; the handoff skill's human re-scan is + # the second barrier, same as for the bare-CLI-flag gap `redact` documents. + def redact_prompt: + _pem + | _url + | _prefix_tokens + | gsub("(?i)(?\\w*(?:token|secret|password|passwd|api[_-]?key|access[_-]?key|credential|auth(?:orization)?|client[_-]?id)\\w*)(?\\s*[:=]\\s*)(?\"?(?:\(M)|[^\\s\"]+))"; "\(.k)\(.s)***") + | _unmask; + # Neutralize control chars (incl. newlines) and backticks so a captured + # command or prompt cannot break the markdown list / code span it is + # embedded in. The control-char half mirrors the shared `tl_clean_ctrl` + # shell helper (used by session-flush.sh / session-precompact.sh for their + # reason/trigger fields) but stays a jq def: it runs per-field inside the + # same jq pipeline as redaction, so factoring it out to a shell call would + # mean an extra subprocess per field. + def clean: gsub("[[:cntrl:]]"; " ") | gsub("`"; " "); + # Clamp a string to $n chars, appending $ell only when it was actually + # longer. Single-sources the "redact | clean | truncate" tail shared by every + # capture surface (the read-side tool branches and the prompt hook) so the + # truncation idiom can't drift between them. + def clamp($n; $ell): .[0:$n] + (if length > $n then $ell else "" end); +TL_JQ_DEFS +} diff --git a/hooks/hooks.json b/hooks/hooks.json index 693dcf8..fd54d0a 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -11,9 +11,20 @@ ] } ], + "UserPromptSubmit": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PLUGIN_ROOT}/hooks/session-prompt.sh" + } + ] + } + ], "PostToolUse": [ { - "matcher": "Bash|Edit|Write|NotebookEdit", + "matcher": "Bash|Edit|Write|NotebookEdit|Grep|WebFetch|WebSearch|Task|Agent|mcp__.*", "hooks": [ { "type": "command", diff --git a/hooks/session-capture.sh b/hooks/session-capture.sh index c9732b6..b8fe743 100755 --- a/hooks/session-capture.sh +++ b/hooks/session-capture.sh @@ -1,10 +1,17 @@ #!/bin/sh # throughline — PostToolUse capture. # -# Appends a structured one-line record of each mutating action (command run, -# file changed) to a per-session buffer. This is the continuous raw layer that a -# later handoff distills from. Mechanical and cheap — no model call. Always -# exits 0; never blocks a tool. +# Appends a structured one-line record of each captured action (command run, +# file changed, search, fetch, delegated task) to a per-session buffer. This is +# the continuous raw layer that a later handoff distills from. Mechanical and +# cheap — no model call. Always exits 0; never blocks a tool. +# +# Which tools land here is decided by the PostToolUse matcher in hooks.json: +# the mutating tools (Bash/Edit/Write/NotebookEdit) plus the high-signal +# read-side tools (Grep/WebFetch/WebSearch/Task/Agent) and MCP tools +# (mcp__.*, name-only via the fallback branch below). Read and Glob are +# deliberately NOT matched — they are the noisiest tools by far, and a buffer +# that logs every file read stops being skimmable (issue #6). # # Load-bearing assumption: one logical session == one buffer file, keyed by a # session_id that stays stable across a context compaction (true on current @@ -26,20 +33,10 @@ data=$(tl_data_dir) root=$(tl_root) bufdir="$data/buffer" -# Best-effort breadcrumb for the swallowed-failure paths below: capture must -# never block a tool, so every failure here still exits 0, but a write failure -# (full disk, lost permissions) would otherwise drop an action with zero trace. -# onboard surfaces this file's presence so the loss isn't silent forever. Lives -# at the data-dir root, NOT under buffer/: tl_active (checked above) guarantees -# $data exists before this point, but $bufdir may not (mkdir below can fail) — -# if the breadcrumb's own target depended on bufdir, the one failure mode this -# exists to report (bufdir uncreatable) would silently defeat the breadcrumb -# along with it. -_tl_err() { - printf -- '%s %s\n' "$(date '+%Y-%m-%d %H:%M:%S')" "$1" >> "$data/.capture-errors" 2>/dev/null -} +# Failure paths below breadcrumb via the shared tl_err (_lib.sh) — see its +# comment for 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; } +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 @@ -51,77 +48,14 @@ sid=$(tl_resolve_sid "$input") # 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 "$sid" ] || { tl_err "dropped action: no usable session_id"; exit 0; } -line=$(printf '%s' "$input" | jq -r --arg root "$root" ' - # Mask common secret shapes so raw credentials never sit in the buffer. The - # buffer is gitignored, but a later handoff distills it into committed logs; - # this is defense-in-depth, not the only barrier (the skill scrubs too). - # Best-effort: this is pattern/keyword matching, not entropy analysis, so a - # bare opaque token with no recognizable shape or keyword will not be caught. - # Known gap, not fixable here without a high false-positive cost: a - # credential attached to a bare single-letter CLI flag with no keyword at all - # (mysql -p, curl -u user:pass) has no keyword for this rule to - # anchor on, and flags like -u/-p are too overloaded across tools (docker run - # -u uid:gid, ssh -p ) to redact generically. The handoff skill re-scan - # is the second barrier for exactly this shape. - # - # Internal sentinel for one specific hand-off: the URL-userinfo rule below is - # the only shape rule whose replacement abuts a non-whitespace character (the - # @ that starts the host). Every other shape rule replacement is naturally - # whitespace-bounded, so the generic catch-all further down can never run - # past it by accident. The userinfo rule alone needs to mark its output as - # "already redacted" so the generic value-capture stops there instead - # of continuing past the @ into the host/path - this used to be inferred from - # boundary characters (first @, then briefly /) instead of stated directly, - # and each version of that inference either deleted real trailing context or - # under-redacted a genuine secret that happened to contain the same boundary - # character. The sentinel removes the inference: the generic rule recognizes - # it explicitly, and the final gsub converts it (and any sentinel a URL with - # no nearby keyword never reached the generic rule to convert) to the - # user-facing *** mark. - def M: "TLREDACTSENTINEL"; - # Shape-specific patterns run BEFORE the generic keyword=value catch-all below, - # so e.g. "Authorization:Basic " is fully consumed by the Basic-auth - # rule rather than the generic auth keyword rule eating just the Basic token - # and leaving the base64 payload exposed. Same reasoning for the dedicated - # Token rule just below (DRF/GitLab-style "Authorization: Token " - # headers): without it, the generic rule treats the scheme word "Token" - # itself as the value to mask and leaves the real key in plaintext right - # after it. The generic keyword group has trailing \w* too (not just - # leading) so compound names like SECRET_KEY= or API_KEY_VALUE= still match - # - a keyword immediately followed by more word characters used to fall - # through unredacted entirely. Its separator group also accepts a copula - # (is/was/are) in addition to :/=/bare-whitespace, so natural-language - # phrasing like "password is X" masks X rather than the word "is". Its value - # group has no @ or / exclusion at all - it matches the sentinel above when - # present, else the fully unbounded run up to whitespace/quote - so a genuine - # secret containing either character (an AWS key with /, a password with @) - # is always fully masked. - def redact: - gsub("-----BEGIN [A-Z ]*PRIVATE KEY-----[\\s\\S]*?-----END [A-Z ]*PRIVATE KEY-----"; "***private-key-redacted***") - | gsub("-----BEGIN [A-Z ]*PRIVATE KEY-----[\\s\\S]*"; "***private-key-redacted***") - | gsub("(?i)bearer\\s+(?[A-Za-z0-9._\\-]+)"; "Bearer ***") - | gsub("(?i)\\btoken\\s+(?[A-Za-z0-9._\\-]+)"; "Token ***") - | gsub("(?://[^:@/\\s]+):(?[^@/\\s]+)@"; "\(.pfx):\(M)@") - | gsub("ghp_[A-Za-z0-9]{10,}"; "ghp_***") - | gsub("github_pat_[A-Za-z0-9_]{10,}"; "github_pat_***") - | gsub("gh[oprsu]_[A-Za-z0-9]{10,}"; "gh_***") - | gsub("xox[baprs]-[A-Za-z0-9-]{6,}"; "xox-***") - | gsub("sk-[A-Za-z0-9_-]{10,}"; "sk-***") - | gsub("AKIA[0-9A-Z]{12,}"; "AKIA***") - | gsub("AIza[0-9A-Za-z_\\-]{35}"; "AIza***") - | gsub("(?i)\\bbasic\\s+[A-Za-z0-9+/=]{8,}"; "Basic ***") - | 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("\(M)"; "***"); - # Neutralize control chars (incl. newlines) and backticks so a captured - # command cannot break the markdown list / code span it is embedded in. The - # control-char half mirrors the shared `tl_clean_ctrl` shell helper in - # _lib.sh (used by session-flush.sh / session-precompact.sh for their - # reason/trigger fields) but stays inline here: this runs inside a jq - # pipeline applied per-field alongside redaction and backtick-stripping in - # one pass, so factoring it out would mean an extra subprocess per field. - def clean: gsub("[[:cntrl:]]"; " ") | gsub("`"; " "); +# 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)"' # 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 @@ -149,16 +83,39 @@ line=$(printf '%s' "$input" | jq -r --arg root "$root" ' "**" + $t + "** " + ((.tool_input.file_path // .tool_input.notebook_path // "?") | ltrimstr($root + "/") | redact | clean) + outcome($t) + # High-signal read-side tools (issue #6): one redacted+cleaned argument + # each, same outcome suffix. Each argument is clamped to a short prefix so + # a wide-net buffer stays skimmable — grep patterns / URLs / queries can be + # long, and only the first stretch identifies the action for a later + # handoff. A subagent Task keeps a longer slice of its description because + # that IS the delegated intent, the highest-value line in a research + # session; falls back to the prompt head when no description is supplied. + elif ($t == "Grep") then + "**grep** `" + ((.tool_input.pattern // "") | redact | clean | clamp(120; "…")) + "`" + outcome($t) + elif ($t == "WebFetch") then + "**webfetch** " + ((.tool_input.url // "") | redact | clean | clamp(200; "…")) + outcome($t) + elif ($t == "WebSearch") then + "**websearch** " + ((.tool_input.query // "") | redact | clean | clamp(200; "…")) + outcome($t) + elif ($t == "Task" or $t == "Agent") then + # description // prompt via an empty-aware select: jq // only falls + # through on null/false, so an empty-STRING description would otherwise + # suppress the prompt fallback and drop the delegated intent entirely. + "**agent** " + + ((.tool_input.subagent_type // "") | clean + | if . == "" then "" else . + ": " end) + + (((.tool_input.description | select(. != "" and . != null)) + // .tool_input.prompt // "") | redact | clean | clamp(200; "…")) + + outcome($t) else - "**" + $t + "**" + # MCP tools (mcp__server__tool) and any other matched tool: name only. + # Zero assumptions about the input schema, so no field can leak a secret + # and no unverified shape can be misread — the tool name alone is the + # skimmable trace. + "**" + $t + "**" + outcome($t) end) -' 2>/dev/null) || { _tl_err "jq filter failed"; exit 0; } +' 2>/dev/null) || { tl_err "jq filter failed"; exit 0; } [ -n "$line" ] || exit 0 -ts=$(date '+%Y-%m-%d %H:%M:%S') -# Backticks here are literal markdown and %s are printf specifiers; single quotes -# are intentional (no shell expansion wanted). -# shellcheck disable=SC2016 -printf -- '- `%s` %s\n' "$ts" "$line" >> "$bufdir/session-$sid.md" 2>/dev/null || _tl_err "write failed for session-$sid" +tl_append_line "$bufdir" "$sid" "$line" exit 0 diff --git a/hooks/session-onboard.sh b/hooks/session-onboard.sh index a5481b7..35a005b 100755 --- a/hooks/session-onboard.sh +++ b/hooks/session-onboard.sh @@ -153,6 +153,15 @@ if [ -d "$bufdir" ]; then for f in "$bufdir"/session-*.md; do [ -f "$f" ] || continue [ -n "$sid" ] && [ "$f" = "$bufdir/session-$sid.md" ] && continue + # Skip prompt-only buffers. A UserPromptSubmit line is recorded for every + # session (session-prompt.sh fires before any tool), so a session that + # captured intent but no ACTION - a question answered from context, or + # Read/Glob-only work, neither of which is a captured tool - leaves a buffer + # containing only `**prompt**` lines. There is nothing to distill there, so + # counting it would nag the user to hand off sessions that did no real work, + # eroding the signal of the warning below. A buffer counts only if it holds + # at least one capture line that is not a prompt line. + grep '^- ' "$f" 2>/dev/null | grep -qv '\*\*prompt\*\*' || continue if grep -q '^\n' \ "$(date '+%Y-%m-%d %H:%M:%S')" "$trigger" >> "$buf" 2>/dev/null exit 0 diff --git a/hooks/session-prompt.sh b/hooks/session-prompt.sh new file mode 100755 index 0000000..f1a25bc --- /dev/null +++ b/hooks/session-prompt.sh @@ -0,0 +1,56 @@ +#!/bin/sh +# throughline — UserPromptSubmit capture. +# +# Appends a redacted, truncated record of each user prompt to the per-session +# buffer. The rest of the buffer records what happened (tool calls); this +# records why — the user's intent, which otherwise lives only in the +# compactable, mortal conversation and is exactly what a context compaction +# destroys. Mechanical and cheap — no model call. Always exits 0; never blocks +# the prompt (a UserPromptSubmit hook that exits non-zero would abort it). +# +# Shares the redaction/cleaning pipeline with session-capture.sh via +# tl_jq_redact_defs, so a prompt containing a pasted credential is masked by +# the same rules that mask a captured command. + +DIR=$(unset CDPATH; cd -- "$(dirname -- "$0")" && pwd) +. "${CLAUDE_PLUGIN_ROOT:-$DIR/..}/hooks/_lib.sh" 2>/dev/null || . "$DIR/_lib.sh" + +tl_active || exit 0 +tl_have_jq || exit 0 # capture needs jq; onboard surfaces the missing-jq warning visibly + +input=$(cat) +data=$(tl_data_dir) +bufdir="$data/buffer" + +# Failure paths breadcrumb via the shared tl_err (_lib.sh); see its comment for +# 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; } + +# 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 +# generic/word rules corrupt ordinary English (see _lib.sh). +# 2. Clamp the RAW text to a generous bound (2000) BEFORE redacting: a +# UserPromptSubmit hook runs synchronously ahead of prompt processing, so +# running ~15 gsub passes over a multi-MB paste would add real latency. +# The bound still fully covers the 200-char window we store, so no secret +# 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 +' 2>/dev/null) || { tl_err "jq filter failed"; exit 0; } + +# Empty / whitespace-only prompts produce no line. +[ -n "$line" ] || exit 0 + +tl_append_line "$bufdir" "$sid" "$line" +exit 0 diff --git a/tests/run.sh b/tests/run.sh index 6b925b8..085447d 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -36,6 +36,8 @@ dir_present(){ if [ -d "$2" ]; then ok "$1"; else bad "$1 (no dir: $2)"; fi; } absent() { if [ -e "$2" ]; then bad "$1 (exists: $2)"; else ok "$1"; fi; } eq() { if [ "$2" = "$3" ]; then ok "$1"; else bad "$1 (got '$2', want '$3')"; fi; } cap() { printf '%s' "$1" | sh "$H/session-capture.sh"; } +prompt() { printf '%s' "$1" | sh "$H/session-prompt.sh"; } +precompact() { printf '%s' "$1" | sh "$H/session-precompact.sh"; } reset_buf() { rm -f "$BUF"/session-*.md "$DATA"/.capture-errors; mkdir -p "$BUF"; chmod 755 "$BUF" 2>/dev/null; } echo "throughline hook tests" @@ -270,6 +272,19 @@ printf -- '- old\n\n' > "$BUF/se O3=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") has "ended prior session flagged as unconsumed" "$O3" 'unconsumed session buffer' +# 10a. a prompt-only buffer (intent captured, but no action — a Q&A or +# Read/Glob-only session) is NOT counted as unconsumed: there is nothing +# to distill, and counting it would nag on every trivial session. +reset_buf +printf -- '- x\n' > "$BUF/session-T.md" +printf -- '- `t` **prompt** just a question answered from context\n\n' > "$BUF/session-PONLY.md" +O3a=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") +hasnt "prompt-only ended buffer NOT flagged as unconsumed" "$O3a" 'unconsumed session buffer' +# but the same buffer WITH a real action line IS surfaced. +printf -- '- `t` **prompt** do the thing\n- `t` **bash** did it - `x`\n\n' > "$BUF/session-PACT.md" +O3a2=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") +has "prompt+action ended buffer IS flagged as unconsumed" "$O3a2" 'unconsumed session buffer' + # 10b. a prior session buffer with NO end-stamp gets hedged wording, not # asserted as "ended" — it could be live in another terminal. reset_buf @@ -522,6 +537,93 @@ present "mkdir failure still breadcrumbs (target is data-dir root, not buffer/)" rm -f "$BUF" mkdir -p "$BUF" +# 14. UserPromptSubmit capture (issue #5): the buffer records intent, not just +# actions. Shares the redaction pipeline with capture via tl_jq_redact_defs. +reset_buf +prompt '{"session_id":"P","prompt":"fix the login bug in the auth module"}' +has "prompt hook records the prompt line" "$(cat "$BUF/session-P.md")" '**prompt** fix the login bug' +# Redaction on prompts uses the PROSE-SAFE path (redact_prompt): structural +# token formats and explicit key:value / key=value secrets are masked, but the +# command-tuned bearer/word and bare-space keyword rules that corrupt ordinary +# English are NOT applied. A pasted shape-token and a colon-form secret are +# still caught. +prompt '{"session_id":"P","prompt":"deploy with password: hunter2secret and ghp_abcdefghij1234567890"}' +PS=$(grep 'deploy with' "$BUF/session-P.md") +hasnt "prompt: colon-form password value not stored" "$PS" 'hunter2secret' +hasnt "prompt: gh token not stored" "$PS" 'ghp_abcdefghij1234567890' +has "prompt: secret masked" "$PS" '***' +# Prose safety: ordinary English containing credential KEYWORDS but no explicit +# secret must survive verbatim — this is the whole point of redact_prompt. The +# command path's copula/bare-space rule would mangle all of these. +prompt '{"session_id":"P","prompt":"my password is not the problem, the auth is stale"}' +has "prompt: prose after keyword+copula is preserved (not inverted)" "$(grep 'not the problem' "$BUF/session-P.md")" 'my password is not the problem, the auth is stale' +prompt '{"session_id":"P","prompt":"explain how the token refresh flow works"}' +has "prompt: 'token refresh' prose is preserved" "$(grep 'refresh flow' "$BUF/session-P.md")" 'the token refresh flow works' +# Truncation at 200 chars with the same marker as the Bash command path. +LONGP=$(printf 'x%.0s' $(seq 1 300)) +prompt '{"session_id":"P","prompt":"'"$LONGP"'"}' +has "prompt: long prompt is truncated" "$(grep truncated "$BUF/session-P.md")" '…[truncated]' +# Empty / whitespace-only prompt writes no line. +reset_buf +prompt '{"session_id":"P2","prompt":" "}' +absent "prompt: whitespace-only prompt writes no buffer file" "$BUF/session-P2.md" +prompt '{"session_id":"P2","prompt":""}' +absent "prompt: empty prompt writes no buffer file" "$BUF/session-P2.md" +# Opt-out and kill switch are honored (prompt uses tl_active like capture). +reset_buf +printf '' > "$WORK/proj/.throughlineignore" +prompt '{"session_id":"P3","prompt":"should be ignored"}' +absent "prompt: .throughlineignore suppresses capture" "$BUF/session-P3.md" +rm -f "$WORK/proj/.throughlineignore" +prompt() { printf '%s' "$1" | THROUGHLINE_DISABLE=1 sh "$H/session-prompt.sh"; } +prompt '{"session_id":"P4","prompt":"should be disabled"}' +absent "prompt: THROUGHLINE_DISABLE suppresses capture" "$BUF/session-P4.md" +prompt() { printf '%s' "$1" | sh "$H/session-prompt.sh"; } +# Missing session_id drops the prompt and breadcrumbs (mirrors capture). +reset_buf +prompt '{"prompt":"no session id here"}' +present "prompt: missing session_id breadcrumbs to .capture-errors" "$DATA/.capture-errors" + +# 15. widened PostToolUse capture (issue #6): high-signal read-side tools each +# emit one redacted one-liner; MCP tools are name-only; Read/Glob are not +# matched at all (excluded in hooks.json, so no branch is needed for them). +reset_buf +cap '{"session_id":"W","tool_name":"Grep","tool_input":{"pattern":"needle.*haystack"}}' +has "grep captured with pattern" "$(cat "$BUF/session-W.md")" '**grep** `needle.*haystack`' +cap '{"session_id":"W","tool_name":"WebFetch","tool_input":{"url":"https://alice:s3cr3tpw@example.com/doc"}}' +WF=$(grep webfetch "$BUF/session-W.md") +has "webfetch captured with url" "$WF" '**webfetch** https://alice:' +hasnt "webfetch url userinfo is masked" "$WF" 's3cr3tpw' +cap '{"session_id":"W","tool_name":"WebSearch","tool_input":{"query":"posix sh idempotency"}}' +has "websearch captured with query" "$(grep websearch "$BUF/session-W.md")" '**websearch** posix sh idempotency' +cap '{"session_id":"W","tool_name":"Task","tool_input":{"subagent_type":"Explore","description":"map the hook data flow"}}' +has "task captured with subagent + description" "$(grep '\*\*agent\*\*' "$BUF/session-W.md")" '**agent** Explore: map the hook data flow' +cap '{"session_id":"W","tool_name":"Task","tool_input":{"prompt":"no description, only a prompt body"}}' +has "task falls back to prompt when no description" "$(grep 'prompt body' "$BUF/session-W.md")" '**agent** no description' +# Empty-STRING description must fall through to prompt (jq // only skips null). +cap '{"session_id":"W","tool_name":"Task","tool_input":{"description":"","prompt":"intent lives in the prompt"}}' +has "task falls back to prompt on empty-string description" "$(grep 'intent lives' "$BUF/session-W.md")" '**agent** intent lives in the prompt' +cap '{"session_id":"W","tool_name":"mcp__github__create_pull_request","tool_input":{"title":"secret sauce"}}' +MC=$(grep 'mcp__github' "$BUF/session-W.md") +has "mcp tool captured name-only" "$MC" '**mcp__github__create_pull_request**' +hasnt "mcp tool input fields are not read" "$MC" 'secret sauce' + +# 16. precompact stamp idempotency (issue #7): a double fire for one seam writes +# one boundary; each genuine compaction (a captured action lands between) +# still gets its own. +reset_buf +printf -- '- `t` **bash** first - `a`\n' > "$BUF/session-C.md" +precompact '{"session_id":"C","trigger":"auto"}' +precompact '{"session_id":"C","trigger":"auto"}' +eq "precompact: double fire writes one boundary" "$(grep -c '^\n' > "$BUF/session-OLD.md" +printf -- '- `t` **bash** old - `x`\n\n' > "$BUF/session-OLD.md" O3=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") has "ended prior session flagged as unconsumed" "$O3" 'unconsumed session buffer' @@ -285,11 +285,22 @@ printf -- '- `t` **prompt** do the thing\n- `t` **bash** did it - `x`\n\n' > "$BUF/session-SUBSTR.md" +O3a3=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") +has "action line mentioning the literal **prompt** string is still counted" "$O3a3" 'unconsumed session buffer' + # 10b. a prior session buffer with NO end-stamp gets hedged wording, not # asserted as "ended" — it could be live in another terminal. reset_buf printf -- '- x\n' > "$BUF/session-T.md" -printf -- '- new\n' > "$BUF/session-LIVE.md" +printf -- '- `t` **bash** new - `x`\n' > "$BUF/session-LIVE.md" O3b=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") has "no-end-stamp buffer surfaced with hedged wording" "$O3b" 'no end-stamp' hasnt "no-end-stamp buffer NOT mislabeled as ended" "$O3b" 'ended without' @@ -559,6 +570,21 @@ prompt '{"session_id":"P","prompt":"my password is not the problem, the auth is has "prompt: prose after keyword+copula is preserved (not inverted)" "$(grep 'not the problem' "$BUF/session-P.md")" 'my password is not the problem, the auth is stale' prompt '{"session_id":"P","prompt":"explain how the token refresh flow works"}' has "prompt: 'token refresh' prose is preserved" "$(grep 'refresh flow' "$BUF/session-P.md")" 'the token refresh flow works' +# Regression: the keyword group is WORD-BOUNDED, not substring-matched — a +# word merely containing "auth"/"token" as a substring (author, authority, +# tokens) must not trip the key:value rule at all, colon or not. +prompt '{"session_id":"P","prompt":"author: rewrite the intro section"}' +has "prompt: 'author:' is not treated as a credential keyword" "$(grep 'rewrite the intro' "$BUF/session-P.md")" 'author: rewrite the intro section' +prompt '{"session_id":"P","prompt":"authority: escalate to the team lead"}' +has "prompt: 'authority:' is not treated as a credential keyword" "$(grep 'escalate to' "$BUF/session-P.md")" 'authority: escalate to the team lead' +prompt '{"session_id":"P","prompt":"tokens: use the new endpoint"}' +has "prompt: 'tokens:' is not treated as a credential keyword" "$(grep 'use the new endpoint' "$BUF/session-P.md")" 'tokens: use the new endpoint' +# Regression: dropping the command path's bearer/basic scheme rules from +# redact_prompt would leak a pasted Authorization header verbatim — these are +# shape-constrained (fixed scheme word / base64 body), not generic prose words, +# so they stay in the prose-safe path. +prompt '{"session_id":"P","prompt":"Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.fake.jwt.body"}' +hasnt "prompt: bearer JWT body not stored" "$(grep Authorization "$BUF/session-P.md")" 'eyJhbGciOiJIUzI1NiJ9' # Truncation at 200 chars with the same marker as the Bash command path. LONGP=$(printf 'x%.0s' $(seq 1 300)) prompt '{"session_id":"P","prompt":"'"$LONGP"'"}' @@ -596,8 +622,17 @@ has "webfetch captured with url" "$WF" '**webfetch** https://alice:' hasnt "webfetch url userinfo is masked" "$WF" 's3cr3tpw' cap '{"session_id":"W","tool_name":"WebSearch","tool_input":{"query":"posix sh idempotency"}}' has "websearch captured with query" "$(grep websearch "$BUF/session-W.md")" '**websearch** posix sh idempotency' +# Regression: WebSearch query is prose, so it must use redact_prompt, not the +# command-tuned redact — the bare-"token"-word rule would otherwise mangle a +# natural-language query like the command path does. +cap '{"session_id":"W","tool_name":"WebSearch","tool_input":{"query":"how to fix token refresh bug"}}' +has "websearch query prose is preserved (not mangled by command redact)" "$(grep 'refresh bug' "$BUF/session-W.md")" 'how to fix token refresh bug' cap '{"session_id":"W","tool_name":"Task","tool_input":{"subagent_type":"Explore","description":"map the hook data flow"}}' has "task captured with subagent + description" "$(grep '\*\*agent\*\*' "$BUF/session-W.md")" '**agent** Explore: map the hook data flow' +# Regression: Task description is prose (the delegated intent) — same +# redact_prompt requirement as WebSearch. +cap '{"session_id":"W","tool_name":"Task","tool_input":{"subagent_type":"Explore","description":"refactor the token handling code"}}' +has "task description prose is preserved (not mangled by command redact)" "$(grep 'handling code' "$BUF/session-W.md")" 'refactor the token handling code' cap '{"session_id":"W","tool_name":"Task","tool_input":{"prompt":"no description, only a prompt body"}}' has "task falls back to prompt when no description" "$(grep 'prompt body' "$BUF/session-W.md")" '**agent** no description' # Empty-STRING description must fall through to prompt (jq // only skips null). From 02ca0ed5992d0ea351366f26cf0b23f65ed237d3 Mon Sep 17 00:00:00 2001 From: Jason Irish Date: Sun, 5 Jul 2026 17:25:16 -0500 Subject: [PATCH 3/6] fix: round-3 review findings - each round-2 fix had its own bug A third high-effort review of round 2's fixes found each one had introduced a new bug of its own: - redact_prompt's \b word-boundary (round 2's fix for substring matching) silently stopped matching SCREAMING_SNAKE_CASE compounds (CLIENT_SECRET, ACCESS_TOKEN, DB_PASSWORD) since underscore is a \w char. Real secrets in the standard naming convention passed through completely unredacted. Fixed with a letter-only lookaround boundary instead of \b: underscore still counts as a boundary, an adjacent letter still doesn't. - Onboard's prompt-only filter used a grep-into-grep pipe whose "found nothing" and "found nothing because there's nothing to find" are indistinguishable, so a buffer with zero conforming lines (corrupted, or a capture hook that failed every call) silently fell through the same continue as a genuine prompt-only buffer - regressing the guarantee that any existing ended buffer gets surfaced. Fixed with an explicit total-vs-prompt-count comparison. - The shared _auth_scheme def (round 2's fix for the Bearer/Basic leak) was unconstrained enough to corrupt ordinary prose ("basic authentication support" -> "Basic *** support"), reopening the exact corruption class redact_prompt exists to prevent. Split into a command variant (unchanged) and a new _auth_scheme_prose with a 16+ char length floor. Tests: 128 -> 134 assertions. Also stress-tested additional edge cases not covered by tests (start-of-string keywords, "credentialing" prefix-match, no-separator prose) to reduce the odds of a round 4. shellcheck + JSON manifest checks green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp --- CHANGELOG.md | 47 +++++++++++++++++++---- hooks/_lib.sh | 83 +++++++++++++++++++++++++++++----------- hooks/session-onboard.sh | 18 ++++++++- tests/run.sh | 32 +++++++++++++++- 4 files changed, 147 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95ba992..a4f8ebf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,14 +96,47 @@ idempotently. A fifth hook joins the set. -> "fix Token \*\*\* bug"). `Grep` (a regex pattern) and `WebFetch` (a URL) correctly stay on `redact` - neither is natural language. +### Review fixes, round 3 (each of round 2's fixes had its own bug) +- **`redact_prompt`'s new `\b` word-boundary silently stopped matching + SCREAMING_SNAKE_CASE compounds** - the standard real-world credential-naming + convention (`CLIENT_SECRET`, `ACCESS_TOKEN`, `DB_PASSWORD`), exactly what a + pasted `.env` file or curl command uses. Underscore is a `\w` character, so + `\bsecret\b` never matches "secret" inside "client_secret" - those secrets + passed into the buffer completely unredacted. Fixed by switching to a + LETTER-only lookaround boundary (`(? "explain the Bearer \*\*\* approach", + "basic authentication support" -> "Basic \*\*\* support" (English words are a + subset of the base64 alphabet the Basic rule allowed, and the Bearer rule had + no length floor at all) - the exact failure mode `redact_prompt` exists to + prevent, now reachable through the very rule meant to plug a different gap. + Split into two variants: `_auth_scheme` (command path, unchanged - commands + aren't prose) and a new `_auth_scheme_prose` (16+ char length floor on both + Bearer's value and Basic's body - real tokens/JWTs run far longer than a + single English word), used only by `redact_prompt`. + ### Tests -- 33 new assertions (95 -> 128): prompt capture (line shape, prose-safety, - colon-form + shape-token redaction, word-boundary regression, bearer/basic - masking, truncation, empty-prompt skip, opt-out/kill-switch, no-session - breadcrumb); widened capture (grep/webfetch/websearch/task/mcp one-liners, - URL-userinfo masking, empty-desc task fallback, WebSearch/Task prose - preservation, mcp input not read); prompt-only buffers not counted as - unconsumed; precompact idempotency (double-fire, multi-seam, post-boundary). +- 39 new assertions (95 -> 134): prompt capture (line shape, prose-safety, + colon-form + shape-token redaction, word-boundary regression, compound + SCREAMING_SNAKE_CASE secrets, bearer/basic masking + prose preservation, + truncation, empty-prompt skip, opt-out/kill-switch, no-session breadcrumb); + widened capture (grep/webfetch/websearch/task/mcp one-liners, URL-userinfo + masking, empty-desc task fallback, WebSearch/Task prose preservation, mcp + input not read); prompt-only buffers not counted as unconsumed, including + the zero-conforming-line edge case; precompact idempotency (double-fire, + multi-seam, post-boundary). ## [0.4.1] diff --git a/hooks/_lib.sh b/hooks/_lib.sh index 0311ceb..2c26103 100755 --- a/hooks/_lib.sh +++ b/hooks/_lib.sh @@ -228,17 +228,36 @@ tl_jq_redact_defs() { | gsub("AKIA[0-9A-Z]{12,}"; "AKIA***") | gsub("AIza[0-9A-Za-z_\\-]{35}"; "AIza***"); def _unmask: gsub("\(M)"; "***"); - # Authorization SCHEME rules (Bearer/Basic), shared by both redact paths. - # Distinct from the generic keyword+word rules below: these match a fixed - # scheme word plus a shape-constrained credential body (Basic requires an - # 8+ char base64-alphabet run; a plain English sentence essentially never - # produces one), so they carry far less prose false-positive risk than the - # generic "token secret" word rules that redact_prompt deliberately excludes. - # Kept in both paths because dropping them from redact_prompt would silently - # leak a pasted Authorization header (the exact regression this fixes). + # Authorization SCHEME rules (Bearer/Basic) - COMMAND-PATH version, used only + # by `redact`. No length minimum on the bearer value and only an 8-char floor + # on Basic's base64 run: commands aren't natural language, so "bearer X" / + # "Basic <8+ chars>" essentially never appears as an innocent phrase there, + # unlike in prose (see _auth_scheme_prose below, which `redact_prompt` uses + # instead - the two are deliberately NOT shared, because the constraint that + # makes one safe would either under-redact the other's real shapes or + # false-positive on the other's ordinary text). def _auth_scheme: gsub("(?i)bearer\\s+(?[A-Za-z0-9._\\-]+)"; "Bearer ***") | gsub("(?i)\\bbasic\\s+[A-Za-z0-9+/=]{8,}"; "Basic ***"); + # Authorization SCHEME rules - PROSE-SAFE version, used only by + # `redact_prompt`. A bare 8-char base64-alphabet floor or an unbounded bearer + # value both false-positive constantly on ordinary English: "bearer of good + # news" (any following word matches the bearer value's char class), "basic + # authentication support" / "basic documentation" (English words are a + # subset of the base64 alphabet, and words like "authentication" clear an + # 8-char floor easily). A real Authorization value - JWT, opaque API token, + # or a base64-encoded "user:pass" - is reliably much longer than a single + # English word in a sentence (single English words average ~5 chars; JWTs + # and opaque tokens commonly run 20-100+); requiring 16+ contiguous + # non-whitespace chars keeps the false-positive rate low without requiring a + # digit/mixed-case heuristic that would itself have real-credential misses. + # Known tradeoff, same class as `redact`'s documented gaps: a real credential + # shorter than 16 chars slips through this rule (the generic key:value rule + # below still catches it if pasted with an explicit "password: xxx" shape; + # otherwise the handoff skill's human re-scan is the backstop). + def _auth_scheme_prose: + gsub("(?i)bearer\\s+(?[A-Za-z0-9._\\-]{16,})"; "Bearer ***") + | gsub("(?i)\\bbasic\\s+[A-Za-z0-9+/=]{16,}"; "Basic ***"); # Full command-path redaction. Shape-specific patterns run BEFORE the generic # keyword=value catch-all, so e.g. "Authorization:Basic " is fully # consumed by the Basic-auth rule rather than the generic auth keyword rule @@ -273,34 +292,52 @@ tl_jq_redact_defs() { # "token" WORD rule both fire on prose - "token refresh flow" -> "Token *** # flow"), destroying the very intent the feature exists to preserve. So this # path keeps ONLY the structural formats that never match English - # (_pem/_url/_prefix_tokens/_auth_scheme - Basic's base64-shape and Bearer's - # fixed scheme word carry far lower false-positive risk than a bare "token") + # (_pem/_url/_prefix_tokens/_auth_scheme_prose - see that def for why Basic/ + # Bearer need their OWN length-gated variant here, not the command one) # plus a generic keyword rule RESTRICTED to explicit key:value / key=value # separators - which still catches a credential pasted as "password: hunter2" # or "API_KEY=xxx" while leaving "the password is not the problem" untouched. # - # The keyword group is \b-WORD-BOUNDED here (unlike redact's \w*-affixed - # version), matching each keyword only as a complete word: "auth"/ - # "authorization" as whole words, never as a substring of "author" or - # "authority"; "token" never as a substring of "tokens". redact's looser - # \w*...\w* affixes exist to catch compound ENV-VAR names in commands - # (SECRET_KEY=, API_KEY_VALUE=) where that surface never collides with - # prose; prompts are prose by definition, so the substring form here would - # otherwise turn "author: rewrite the intro" or "tokens: use the new - # endpoint" into a mangled "*** the intro" / "*** the new endpoint" - a - # correctness bug this restricted form exists to close, not merely a - # narrower match. + # The keyword group is bounded by a LETTER lookaround here (unlike redact's + # `\w*...\w*`-affixed version), matching each keyword only when no LETTER + # touches either side: "auth"/"authorization" as whole words never match as + # a substring of "author"/"authority"; "token" never matches inside + # "tokens". But unlike a literal `\b` boundary, an adjacent UNDERSCORE still + # counts as a match boundary, so SCREAMING_SNAKE_CASE compounds + # (CLIENT_SECRET, ACCESS_TOKEN, DB_PASSWORD) - the standard real-world + # credential-naming convention, and exactly what a pasted .env file or curl + # command uses - still redact correctly. redact's looser `\w*...\w*` affixes + # exist for the same compound-name reason on the command path; prompts are + # prose by definition, so a `\w*` (which also treats letters as valid + # boundary-crossers) would additionally match inside "author"/"tokens" - + # hence the tighter letter-only lookaround here instead of reusing it. # # A pasted secret with no recognizable prefix/scheme AND no colon/equals # (e.g. a bare word after "password is") is deliberately NOT masked here; # the handoff skill's human re-scan is the second barrier, same as for the # bare-CLI-flag gap `redact` documents. + # + # The keyword group is bounded by LETTER lookaround, not `\b`: `\b` is a + # transition between a \w char and a non-\w char, and underscore IS a \w + # char, so `\bsecret\b` does NOT match "secret" inside "client_secret" or + # "SECRET" inside "DB_PASSWORD"'s sibling "PASSWORD" - it would silently fail + # to redact the single most common real-world credential-naming convention + # (SCREAMING_SNAKE_CASE: CLIENT_SECRET, ACCESS_TOKEN, DB_PASSWORD, ...), + # exactly the compounds a pasted .env file or curl command uses. Lookaround + # on "not a letter" instead of "not a word char" treats the underscore as a + # valid boundary (so the compound still matches) while still rejecting a + # letter on either side (so "author"/"authority"/"tokens" still don't match + # "auth"/"auth"/"token" as a prefix - the false-positive `\b` was tightened + # for in the first place). Digits are deliberately left out of the boundary + # class too (neither excluded nor required): a keyword directly touching a + # digit is rare enough in both prose and real credential names not to be + # worth the extra rule complexity either way. def redact_prompt: _pem - | _auth_scheme + | _auth_scheme_prose | _url | _prefix_tokens - | gsub("(?i)(?\\b(?:token|secret|password|passwd|api[_-]?key|access[_-]?key|credential|auth(?:orization)?|client[_-]?id)\\b)(?\\s*[:=]\\s*)(?\"?(?:\(M)|[^\\s\"]+))"; "\(.k)\(.s)***") + | gsub("(?i)(?(?\\s*[:=]\\s*)(?\"?(?:\(M)|[^\\s\"]+))"; "\(.k)\(.s)***") | _unmask; # Neutralize control chars (incl. newlines) and backticks so a captured # command or prompt cannot break the markdown list / code span it is diff --git a/hooks/session-onboard.sh b/hooks/session-onboard.sh index 0c864af..408533d 100755 --- a/hooks/session-onboard.sh +++ b/hooks/session-onboard.sh @@ -170,9 +170,23 @@ if [ -d "$bufdir" ]; then # literal pattern, a bash command referencing it, and so on — routine in # this very repo), which would silently misclassify a genuinely unconsumed # session as prompt-only and drop it from the warning entirely. + # + # Counted explicitly (total vs. prompt-marked) rather than via a + # grep-into-grep pipe: the pipe form's "found nothing" and "found nothing + # because there's nothing to find" are indistinguishable, so a buffer with + # ZERO conforming record lines (a truncated/corrupted buffer, or a capture + # hook's jq failing on every call) silently fell through the SAME `|| + # continue` as a genuine prompt-only buffer - even though pre-existing + # behavior always counted any existing, end-stamped buffer regardless of + # its body. Skip ONLY when there is at least one recognized line AND every + # one of them is a prompt line; zero recognized lines falls through to be + # counted, matching that prior fail-safe behavior instead of silently + # dropping a real ended session. + # shellcheck disable=SC2016 + total=$(grep -cE '^- `[^`]*` \*\*[^*]+\*\*' "$f" 2>/dev/null) # shellcheck disable=SC2016 - grep -E '^- `[^`]*` \*\*[^*]+\*\*' "$f" 2>/dev/null \ - | grep -qvE '^- `[^`]*` \*\*prompt\*\*' || continue + promptonly=$(grep -cE '^- `[^`]*` \*\*prompt\*\*' "$f" 2>/dev/null) + [ "$total" -gt 0 ] && [ "$total" -eq "$promptonly" ] && continue if grep -q '^\n' > "$BUF/session-EMPTY.md" +O3a4=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") +has "a buffer with zero conforming lines is still counted, not dropped" "$O3a4" 'unconsumed session buffer' + # 10b. a prior session buffer with NO end-stamp gets hedged wording, not # asserted as "ended" — it could be live in another terminal. reset_buf @@ -579,12 +591,30 @@ prompt '{"session_id":"P","prompt":"authority: escalate to the team lead"}' has "prompt: 'authority:' is not treated as a credential keyword" "$(grep 'escalate to' "$BUF/session-P.md")" 'authority: escalate to the team lead' prompt '{"session_id":"P","prompt":"tokens: use the new endpoint"}' has "prompt: 'tokens:' is not treated as a credential keyword" "$(grep 'use the new endpoint' "$BUF/session-P.md")" 'tokens: use the new endpoint' +# Regression: the letter-lookaround boundary must still redact +# SCREAMING_SNAKE_CASE compounds (the standard real-world credential-naming +# convention) even though a plain `\b` boundary would not — underscore is a +# \w char, so `\bsecret\b` never matches "secret" inside "client_secret". +prompt '{"session_id":"P","prompt":"client_secret: aB3xY9zQwErTyUiOp1234567890"}' +hasnt "prompt: client_secret compound value not stored" "$(grep client_secret "$BUF/session-P.md")" 'aB3xY9zQwErTyUiOp1234567890' +prompt '{"session_id":"P","prompt":"DB_PASSWORD=hunter2reallysecretvalue"}' +hasnt "prompt: DB_PASSWORD compound value not stored" "$(grep DB_PASSWORD "$BUF/session-P.md")" 'hunter2reallysecretvalue' +prompt '{"session_id":"P","prompt":"ACCESS_TOKEN: veryLongOpaqueTokenValue123"}' +hasnt "prompt: ACCESS_TOKEN compound value not stored" "$(grep ACCESS_TOKEN "$BUF/session-P.md")" 'veryLongOpaqueTokenValue123' # Regression: dropping the command path's bearer/basic scheme rules from # redact_prompt would leak a pasted Authorization header verbatim — these are # shape-constrained (fixed scheme word / base64 body), not generic prose words, -# so they stay in the prose-safe path. +# so they stay in the prose-safe path (via a length-gated prose variant). prompt '{"session_id":"P","prompt":"Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.fake.jwt.body"}' hasnt "prompt: bearer JWT body not stored" "$(grep Authorization "$BUF/session-P.md")" 'eyJhbGciOiJIUzI1NiJ9' +# Regression: the prose-safe bearer/basic variant must NOT fire on short +# English words after "bearer"/"basic" — this is the exact corruption class +# redact_prompt exists to prevent, and the fix for the leak above must not +# reopen it via an unconstrained scheme rule. +prompt '{"session_id":"P","prompt":"explain the bearer token approach for auth"}' +has "prompt: 'bearer token approach' prose is preserved" "$(grep 'bearer token approach' "$BUF/session-P.md")" 'the bearer token approach for auth' +prompt '{"session_id":"P","prompt":"we need basic authentication support"}' +has "prompt: 'basic authentication' prose is preserved" "$(grep 'basic authentication' "$BUF/session-P.md")" 'we need basic authentication support' # Truncation at 200 chars with the same marker as the Bash command path. LONGP=$(printf 'x%.0s' $(seq 1 300)) prompt '{"session_id":"P","prompt":"'"$LONGP"'"}' From 6f2976cf44bf2e68d2acde6a1b13a6d30dc8abb2 Mon Sep 17 00:00:00 2001 From: Jason Irish Date: Mon, 6 Jul 2026 12:12:01 -0500 Subject: [PATCH 4/6] fix: round-4 review findings - structural redesign of redact_prompt A fourth review found four bugs, two in the same keyword-boundary code round 3 had just patched: the letter-lookaround fix excluded not just the intended false-positive cases but EVERY keyword followed by any letter, so common real phrasing ("secrets:", "passwords:", "credentials:") silently stopped being redacted at all. Three consecutive rounds of "correct for the target case, breaks a different one" on the same regex - the same shape as the v0.2.0 redaction rework documented earlier in this repo's history. Per that precedent, stopped patching and did a structural redesign instead of a fifth regex tweak (user confirmed: drop the generic rule). - redact_prompt's generic keyword+separator rule removed entirely. There is no boundary rule that admits "secrets:"/"passwords:" while excluding "author:"/"tokens:" - the shapes overlap. redact_prompt now masks ONLY unambiguous structural signals: known token prefixes, PEM, URL-userinfo, and length-gated Bearer/Token/Basic scheme values. A colon-form secret with no recognizable prefix/scheme is now a deliberate, CONSISTENT, documented gap backstopped by the handoff skill's human re-scan - not fixed differently case-by-case. - Added the missing Token-scheme rule to _auth_scheme_prose (redact_prompt never had one - an oversight from splitting it out of redact in round 2). - The 16-char length floor un-redacting short real Bearer/Basic values is accepted as the same documented gap, rather than tuning the threshold further. - Noted (not fixed, pre-existing, out of scope): a cosmetic quote-handling bug in redact's generic rule (command path, unchanged) - an orphaned trailing quote, no secret leak. No longer applies to redact_prompt since that rule is gone. Tests: 134 -> 133 (net change reflects removing tests for behavior that no longer exists and consolidating documented-gap assertions). Stress-tested broadly beyond the reviewer's cases (plurals, scheme words as ordinary nouns, quote interaction) before resubmitting. shellcheck + JSON manifest green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp --- CHANGELOG.md | 52 +++++++++++++++++++++++++----- hooks/_lib.sh | 89 ++++++++++++++++++++++----------------------------- tests/run.sh | 78 +++++++++++++++++++++++--------------------- 3 files changed, 124 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4f8ebf..97d7354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,15 +127,51 @@ idempotently. A fifth hook joins the set. Bearer's value and Basic's body - real tokens/JWTs run far longer than a single English word), used only by `redact_prompt`. +### Review fixes, round 4 - structural redesign of `redact_prompt` +A fourth review found four more bugs, two of them ([1]) in the SAME code +round 3 had just patched: the letter-lookaround boundary excluded not just the +intended false-positive cases but EVERY keyword followed by any letter, so +"secrets:"/"passwords:"/"credentials:" (common, real phrasing) silently +stopped being redacted at all - worse than what it fixed. That's three +consecutive rounds where a fix to the same keyword-boundary regex was correct +for its target case but broke a different one, the same shape as the +`redact()` boundary-character rework documented earlier in this changelog +(v0.2.0). Rather than a fourth regex patch, `redact_prompt`'s generic +keyword+separator rule was removed entirely: + +- **No boundary rule can admit "secrets:"/"passwords:" (real usage) while + excluding "author:"/"tokens:" (false positives) - the shapes overlap.** + `redact_prompt` now masks ONLY unambiguous structural signals: known token + prefixes (`ghp_`, `sk-`, `AKIA`, ...), PEM blocks, URL-userinfo, and + length-gated Bearer/Token/Basic scheme values. A colon-form secret with no + recognizable prefix/scheme ("password: hunter2", `CLIENT_SECRET: xyz`) is + now a deliberate, documented, and CONSISTENT gap - not fixed differently + case-by-case - backstopped solely by the handoff skill's human re-scan, + same class as `redact`'s existing bare-CLI-flag gap. +- **`redact_prompt` had no Token-scheme rule at all** (the DRF/GitLab-style + `Authorization: Token ` header), an oversight from splitting it out + of `redact` in round 2. Added alongside Bearer/Basic in `_auth_scheme_prose`, + same 16+ char length floor. +- **The 16-char length floor (round 3's prose-safety fix) also un-redacts + real 8-15 char Bearer/Basic values**, leaving the credential in plaintext + next to a `***` that falsely suggests full redaction. Accepted as the same + deliberate gap as above, rather than tuning the threshold further. +- A pre-existing (not a regression from this PR) cosmetic quote-handling bug + in the shared value-capture group - an orphaned trailing quote in malformed + output, no secret leak - was identified in both `redact` and the + since-removed `redact_prompt` generic rule. It no longer applies to + `redact_prompt` (that rule is gone); it remains in `redact` (command path, + unchanged, out of scope for this PR) as a low-priority follow-up. + ### Tests -- 39 new assertions (95 -> 134): prompt capture (line shape, prose-safety, - colon-form + shape-token redaction, word-boundary regression, compound - SCREAMING_SNAKE_CASE secrets, bearer/basic masking + prose preservation, - truncation, empty-prompt skip, opt-out/kill-switch, no-session breadcrumb); - widened capture (grep/webfetch/websearch/task/mcp one-liners, URL-userinfo - masking, empty-desc task fallback, WebSearch/Task prose preservation, mcp - input not read); prompt-only buffers not counted as unconsumed, including - the zero-conforming-line edge case; precompact idempotency (double-fire, +- 39 new assertions (95 -> 133): prompt capture (line shape, prose-safety, + structural-signal redaction, documented-gap behavior for colon-form/compound + secrets, Bearer/Token/Basic scheme masking + prose preservation, truncation, + empty-prompt skip, opt-out/kill-switch, no-session breadcrumb); widened + capture (grep/webfetch/websearch/task/mcp one-liners, URL-userinfo masking, + empty-desc task fallback, WebSearch/Task prose preservation, mcp input not + read); prompt-only buffers not counted as unconsumed, including the + zero-conforming-line edge case; precompact idempotency (double-fire, multi-seam, post-boundary). ## [0.4.1] diff --git a/hooks/_lib.sh b/hooks/_lib.sh index 2c26103..b753bcb 100755 --- a/hooks/_lib.sh +++ b/hooks/_lib.sh @@ -251,12 +251,19 @@ tl_jq_redact_defs() { # and opaque tokens commonly run 20-100+); requiring 16+ contiguous # non-whitespace chars keeps the false-positive rate low without requiring a # digit/mixed-case heuristic that would itself have real-credential misses. - # Known tradeoff, same class as `redact`'s documented gaps: a real credential - # shorter than 16 chars slips through this rule (the generic key:value rule - # below still catches it if pasted with an explicit "password: xxx" shape; - # otherwise the handoff skill's human re-scan is the backstop). + # Also includes the Token scheme word (DRF/GitLab-style "Authorization: + # Token "), which redact_prompt lacked entirely until this was added - + # without it, a Token-scheme header fell through to no rule at all (there is + # no generic keyword rule left in redact_prompt to catch it either; see the + # comment above redact_prompt for why that rule was removed). + # Known, DELIBERATE tradeoff, same class as `redact`'s documented gaps: a + # real credential shorter than 16 chars is not masked by this rule. There is + # no fallback generic rule in redact_prompt to catch it either - the handoff + # skill's human re-scan is the sole backstop for that case, same as for any + # other bare/short secret with no recognizable long-token shape. def _auth_scheme_prose: gsub("(?i)bearer\\s+(?[A-Za-z0-9._\\-]{16,})"; "Bearer ***") + | gsub("(?i)\\btoken\\s+(?[A-Za-z0-9._\\-]{16,})"; "Token ***") | gsub("(?i)\\bbasic\\s+[A-Za-z0-9+/=]{16,}"; "Basic ***"); # Full command-path redaction. Shape-specific patterns run BEFORE the generic # keyword=value catch-all, so e.g. "Authorization:Basic " is fully @@ -286,58 +293,40 @@ tl_jq_redact_defs() { | _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)***") | _unmask; - # Prose-safe redaction for user prompts (issue #5). Prompts are natural - # language, not commands: running them through `redact` corrupts and can - # invert ordinary English (the copula/bare-space generic rule and the bare - # "token" WORD rule both fire on prose - "token refresh flow" -> "Token *** - # flow"), destroying the very intent the feature exists to preserve. So this - # path keeps ONLY the structural formats that never match English - # (_pem/_url/_prefix_tokens/_auth_scheme_prose - see that def for why Basic/ - # Bearer need their OWN length-gated variant here, not the command one) - # plus a generic keyword rule RESTRICTED to explicit key:value / key=value - # separators - which still catches a credential pasted as "password: hunter2" - # or "API_KEY=xxx" while leaving "the password is not the problem" untouched. + # Prose-safe redaction for user prompts (issue #5), and for the WebSearch + # query / Task description branches in session-capture.sh (also prose). + # Prompts are natural language, not commands: running them through `redact` + # corrupts and can invert ordinary English (the copula/bare-space generic + # rule and the bare "token" WORD rule both fire on prose - "token refresh + # flow" -> "Token *** flow"), destroying the very intent the feature exists + # to preserve. So this path keeps ONLY unambiguous STRUCTURAL signals - + # _pem, _url, _prefix_tokens, and _auth_scheme_prose (Bearer/Token/Basic, + # each length-gated) - things that are never accidentally spelled by an + # English sentence. # - # The keyword group is bounded by a LETTER lookaround here (unlike redact's - # `\w*...\w*`-affixed version), matching each keyword only when no LETTER - # touches either side: "auth"/"authorization" as whole words never match as - # a substring of "author"/"authority"; "token" never matches inside - # "tokens". But unlike a literal `\b` boundary, an adjacent UNDERSCORE still - # counts as a match boundary, so SCREAMING_SNAKE_CASE compounds - # (CLIENT_SECRET, ACCESS_TOKEN, DB_PASSWORD) - the standard real-world - # credential-naming convention, and exactly what a pasted .env file or curl - # command uses - still redact correctly. redact's looser `\w*...\w*` affixes - # exist for the same compound-name reason on the command path; prompts are - # prose by definition, so a `\w*` (which also treats letters as valid - # boundary-crossers) would additionally match inside "author"/"tokens" - - # hence the tighter letter-only lookaround here instead of reusing it. - # - # A pasted secret with no recognizable prefix/scheme AND no colon/equals - # (e.g. a bare word after "password is") is deliberately NOT masked here; - # the handoff skill's human re-scan is the second barrier, same as for the - # bare-CLI-flag gap `redact` documents. - # - # The keyword group is bounded by LETTER lookaround, not `\b`: `\b` is a - # transition between a \w char and a non-\w char, and underscore IS a \w - # char, so `\bsecret\b` does NOT match "secret" inside "client_secret" or - # "SECRET" inside "DB_PASSWORD"'s sibling "PASSWORD" - it would silently fail - # to redact the single most common real-world credential-naming convention - # (SCREAMING_SNAKE_CASE: CLIENT_SECRET, ACCESS_TOKEN, DB_PASSWORD, ...), - # exactly the compounds a pasted .env file or curl command uses. Lookaround - # on "not a letter" instead of "not a word char" treats the underscore as a - # valid boundary (so the compound still matches) while still rejecting a - # letter on either side (so "author"/"authority"/"tokens" still don't match - # "auth"/"auth"/"token" as a prefix - the false-positive `\b` was tightened - # for in the first place). Digits are deliberately left out of the boundary - # class too (neither excluded nor required): a keyword directly touching a - # digit is rare enough in both prose and real credential names not to be - # worth the extra rule complexity either way. + # Deliberately DOES NOT include a generic keyword+separator rule (unlike + # `redact`, which has one for the command path). Three rounds of trying to + # make a keyword-boundary regex tell "real credential label" from "ordinary + # word" apart each fixed one failure mode by introducing a different one: + # - substring matching ("auth" inside "author") -> \b-word-bounded + # - \b then missed SCREAMING_SNAKE_CASE compounds (\b doesn't cross an + # underscore) -> switched to a letter-only lookaround + # - the letter-lookaround then excluded EVERY keyword+letter-suffix, not + # just the intended cases - "secrets:"/"passwords:"/"credentials:" + # (common, real phrasing) silently stopped being masked at all + # There is no boundary rule that admits "secrets"/"passwords" (real usage) + # while excluding "author"/"tokens" (false positives) - the shapes overlap. + # Rather than a fourth attempt at the same regex, the rule is removed: a + # pasted secret with no recognizable prefix/scheme (e.g. "password: hunter2" + # with no ghp_/sk-/AKIA/... shape and no 16+ char scheme value) is NOT masked + # in prompts/WebSearch/Task. This is the same class of documented, deliberate + # gap as `redact`'s bare-CLI-flag limitation - the handoff skill's human + # re-scan is the sole backstop for it here, not a secondary one. def redact_prompt: _pem | _auth_scheme_prose | _url | _prefix_tokens - | gsub("(?i)(?(?\\s*[:=]\\s*)(?\"?(?:\(M)|[^\\s\"]+))"; "\(.k)\(.s)***") | _unmask; # Neutralize control chars (incl. newlines) and backticks so a captured # command or prompt cannot break the markdown list / code span it is diff --git a/tests/run.sh b/tests/run.sh index da88574..90aa6ea 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -565,56 +565,60 @@ mkdir -p "$BUF" reset_buf prompt '{"session_id":"P","prompt":"fix the login bug in the auth module"}' has "prompt hook records the prompt line" "$(cat "$BUF/session-P.md")" '**prompt** fix the login bug' -# Redaction on prompts uses the PROSE-SAFE path (redact_prompt): structural -# token formats and explicit key:value / key=value secrets are masked, but the -# command-tuned bearer/word and bare-space keyword rules that corrupt ordinary -# English are NOT applied. A pasted shape-token and a colon-form secret are -# still caught. -prompt '{"session_id":"P","prompt":"deploy with password: hunter2secret and ghp_abcdefghij1234567890"}' +# Redaction on prompts uses the PROSE-SAFE path (redact_prompt): ONLY +# unambiguous structural signals are masked (known token prefixes, PEM, +# URL-userinfo, and length-gated Bearer/Token/Basic scheme values) — there is +# NO generic keyword+colon rule (see the def's comment for why: three rounds +# of boundary regex fixes each traded one false-positive/negative for a +# different one, and there is no boundary rule that admits "secrets:"/ +# "passwords:" while excluding "author:"/"tokens:" — the shapes overlap). +# A known token-prefix shape is still caught even inside prose: +prompt '{"session_id":"P","prompt":"deploy with my key ghp_abcdefghij1234567890"}' PS=$(grep 'deploy with' "$BUF/session-P.md") -hasnt "prompt: colon-form password value not stored" "$PS" 'hunter2secret' -hasnt "prompt: gh token not stored" "$PS" 'ghp_abcdefghij1234567890' -has "prompt: secret masked" "$PS" '***' -# Prose safety: ordinary English containing credential KEYWORDS but no explicit -# secret must survive verbatim — this is the whole point of redact_prompt. The -# command path's copula/bare-space rule would mangle all of these. +hasnt "prompt: gh token not stored" "$PS" 'ghp_abcdefghij1234567890' +has "prompt: gh token masked" "$PS" '***' +# Prose safety: ordinary English containing credential KEYWORDS but no +# recognizable structural shape survives verbatim — the whole point of +# redact_prompt. The command path's copula/bare-space rule would mangle all +# of these; redact_prompt has no keyword rule at all to even attempt it. prompt '{"session_id":"P","prompt":"my password is not the problem, the auth is stale"}' has "prompt: prose after keyword+copula is preserved (not inverted)" "$(grep 'not the problem' "$BUF/session-P.md")" 'my password is not the problem, the auth is stale' prompt '{"session_id":"P","prompt":"explain how the token refresh flow works"}' has "prompt: 'token refresh' prose is preserved" "$(grep 'refresh flow' "$BUF/session-P.md")" 'the token refresh flow works' -# Regression: the keyword group is WORD-BOUNDED, not substring-matched — a -# word merely containing "auth"/"token" as a substring (author, authority, -# tokens) must not trip the key:value rule at all, colon or not. prompt '{"session_id":"P","prompt":"author: rewrite the intro section"}' -has "prompt: 'author:' is not treated as a credential keyword" "$(grep 'rewrite the intro' "$BUF/session-P.md")" 'author: rewrite the intro section' -prompt '{"session_id":"P","prompt":"authority: escalate to the team lead"}' -has "prompt: 'authority:' is not treated as a credential keyword" "$(grep 'escalate to' "$BUF/session-P.md")" 'authority: escalate to the team lead' -prompt '{"session_id":"P","prompt":"tokens: use the new endpoint"}' -has "prompt: 'tokens:' is not treated as a credential keyword" "$(grep 'use the new endpoint' "$BUF/session-P.md")" 'tokens: use the new endpoint' -# Regression: the letter-lookaround boundary must still redact -# SCREAMING_SNAKE_CASE compounds (the standard real-world credential-naming -# convention) even though a plain `\b` boundary would not — underscore is a -# \w char, so `\bsecret\b` never matches "secret" inside "client_secret". +has "prompt: 'author:' prose is preserved" "$(grep 'rewrite the intro' "$BUF/session-P.md")" 'author: rewrite the intro section' +# Deliberate, accepted gap (not a bug): a colon-form secret with no +# recognizable prefix/scheme is NOT masked in prompts — same class as +# `redact`'s documented bare-CLI-flag gap, backstopped by the handoff skill's +# human re-scan instead of a second automated layer. This also means the +# real-world plural phrasing the removed keyword rule mishandled ("secrets:", +# "passwords:", "credentials:") is now consistently left untouched rather than +# inconsistently redacted depending on suffix — documented here so a future +# reader doesn't mistake this for an oversight and re-add the keyword rule. +prompt '{"session_id":"P","prompt":"our secrets: hunter2superlongsecretvalue"}' +has "prompt: bare colon-form secret is a documented gap, not auto-masked" "$(grep 'our secrets' "$BUF/session-P.md")" 'our secrets: hunter2superlongsecretvalue' prompt '{"session_id":"P","prompt":"client_secret: aB3xY9zQwErTyUiOp1234567890"}' -hasnt "prompt: client_secret compound value not stored" "$(grep client_secret "$BUF/session-P.md")" 'aB3xY9zQwErTyUiOp1234567890' -prompt '{"session_id":"P","prompt":"DB_PASSWORD=hunter2reallysecretvalue"}' -hasnt "prompt: DB_PASSWORD compound value not stored" "$(grep DB_PASSWORD "$BUF/session-P.md")" 'hunter2reallysecretvalue' -prompt '{"session_id":"P","prompt":"ACCESS_TOKEN: veryLongOpaqueTokenValue123"}' -hasnt "prompt: ACCESS_TOKEN compound value not stored" "$(grep ACCESS_TOKEN "$BUF/session-P.md")" 'veryLongOpaqueTokenValue123' -# Regression: dropping the command path's bearer/basic scheme rules from -# redact_prompt would leak a pasted Authorization header verbatim — these are -# shape-constrained (fixed scheme word / base64 body), not generic prose words, -# so they stay in the prose-safe path (via a length-gated prose variant). +has "prompt: SCREAMING_SNAKE_CASE compound is the same documented gap" "$(grep client_secret "$BUF/session-P.md")" 'client_secret: aB3xY9zQwErTyUiOp1234567890' +# Structural scheme rules still catch a pasted Authorization header, INCLUDING +# the Token scheme (DRF/GitLab-style) — redact_prompt lacked this rule +# entirely until it was added alongside Bearer/Basic. prompt '{"session_id":"P","prompt":"Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.fake.jwt.body"}' hasnt "prompt: bearer JWT body not stored" "$(grep Authorization "$BUF/session-P.md")" 'eyJhbGciOiJIUzI1NiJ9' -# Regression: the prose-safe bearer/basic variant must NOT fire on short -# English words after "bearer"/"basic" — this is the exact corruption class -# redact_prompt exists to prevent, and the fix for the leak above must not -# reopen it via an unconstrained scheme rule. +reset_buf +prompt '{"session_id":"P","prompt":"Authorization: Token abcdefghijklmnopqrstuvwxyzTHISISAVERYLONGSECRETVALUE"}' +hasnt "prompt: Authorization Token value not stored" "$(cat "$BUF/session-P.md")" 'abcdefghijklmnopqrstuvwxyzTHISISAVERYLONGSECRETVALUE' +has "prompt: Authorization Token scheme word preserved" "$(cat "$BUF/session-P.md")" 'Authorization: Token ***' +# The scheme rules must NOT fire on short English words after +# "bearer"/"basic"/"token" — this is the exact corruption class redact_prompt +# exists to prevent. prompt '{"session_id":"P","prompt":"explain the bearer token approach for auth"}' has "prompt: 'bearer token approach' prose is preserved" "$(grep 'bearer token approach' "$BUF/session-P.md")" 'the bearer token approach for auth' prompt '{"session_id":"P","prompt":"we need basic authentication support"}' has "prompt: 'basic authentication' prose is preserved" "$(grep 'basic authentication' "$BUF/session-P.md")" 'we need basic authentication support' +# Same documented length-gate gap applies to short real scheme values (a +# 8-15 char Bearer/Basic/Token value is not masked) — deliberate, not a bug. +prompt '{"session_id":"P","prompt":"Authorization: Basic dXNlcjpwdw=="}' +has "prompt: short Basic value is the same documented gap" "$(grep 'Authorization: Basic' "$BUF/session-P.md")" 'Authorization: Basic dXNlcjpwdw==' # Truncation at 200 chars with the same marker as the Bash command path. LONGP=$(printf 'x%.0s' $(seq 1 300)) prompt '{"session_id":"P","prompt":"'"$LONGP"'"}' From bdaa367f61a3469656a395132da17b344f7d0130 Mon Sep 17 00:00:00 2001 From: Jason Irish Date: Mon, 6 Jul 2026 12:28:52 -0500 Subject: [PATCH 5/6] fix: round-5 review findings - onboard stderr leak, mcp asterisk desync A fifth review confirmed the round-4 structural redesign is clean (zero findings against redact_prompt - the three-round regex cycle is over) and found two smaller, unrelated bugs: - session-onboard.sh's prompt-only counters leaked a shell diagnostic to stderr on an unreadable buffer file. grep -c prints an empty string, not "0", when it cannot read the file at all, and the integer test was not guarded - the one error path in this hook not already 2>/dev/null'd. Fixed by defaulting both counts to 0 when empty. - The mcp__* fallback branch embedded the raw tool name directly inside the **...** bold-marker pair without stripping asterisks (every other branch only puts field content after a fixed literal marker). An unusual tool name containing * would break the markdown span and desync onboard's classifier regex, silently miscounting a real action as prompt-only. Fixed by stripping asterisks from the tool name specifically. - Bash branch's hand-rolled truncation migrated to the shared clamp def. Filed issue #16 for three deferred cleanup items (duplicated timestamp format, redundant onboard greps, duplicated auth-scheme regex literals) - none are correctness bugs, deferred to a calmer follow-up rather than more mechanical edits under today's time pressure. Tests: 133 -> 136 assertions. shellcheck + JSON manifest green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp --- CHANGELOG.md | 38 ++++++++++++++++++++++++++++++++++---- hooks/session-capture.sh | 17 +++++++++++++---- hooks/session-onboard.sh | 10 ++++++++-- tests/run.sh | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97d7354..2b73096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -163,16 +163,46 @@ keyword+separator rule was removed entirely: `redact_prompt` (that rule is gone); it remains in `redact` (command path, unchanged, out of scope for this PR) as a low-priority follow-up. +### Review fixes, round 5 - the redesign held; two smaller bugs elsewhere +A fifth review found the round-4 redesign clean (zero findings against +`redact_prompt` itself - the three-round regex cycle is over) and surfaced two +smaller, unrelated bugs plus deferred cleanup: + +- **`session-onboard.sh`'s prompt-only counters leaked a shell diagnostic to + stderr on an unreadable buffer file.** `grep -c` prints an empty string + (not "0") when it cannot read the file at all, and the following integer + test was not guarded against that - the one place in this hook where an + error path was not `2>/dev/null`'d, breaking its own always-silent contract. + Fixed by defaulting both counts to 0 when empty. +- **The `mcp__*` fallback branch embedded the raw tool name directly inside + the `**...**` bold-marker pair** without stripping asterisks (every other + branch only puts field content AFTER a fixed literal marker, so this is the + one place where arbitrary text sits inside the delimiters themselves). An + unusual tool name containing `*` would break the markdown span and desync + onboard's anchored classifier regex, silently miscounting a real action as + prompt-only. Fixed by stripping asterisks from the tool name specifically + (not folded into the shared `clean` def, which other branches rely on to + preserve a literal `*` in content like a glob pattern). +- The Bash capture branch's hand-rolled truncation was migrated to the shared + `clamp` def (it was the one call site the round-1 refactor had missed). +- Filed as follow-ups rather than fixed inline (issue #16): a timestamp format + string duplicated across 4 call sites, `session-onboard.sh` running 3 + separate `grep` passes per buffer file where one would do, and + `_auth_scheme`/`_auth_scheme_prose` duplicating the same regex literals at + different length thresholds instead of one parameterized def. None are + correctness bugs; deferred to a calmer pass rather than more mechanical + edits under the same time pressure that caused rounds 2-4. + ### Tests -- 39 new assertions (95 -> 133): prompt capture (line shape, prose-safety, +- 44 new assertions (95 -> 136): prompt capture (line shape, prose-safety, structural-signal redaction, documented-gap behavior for colon-form/compound secrets, Bearer/Token/Basic scheme masking + prose preservation, truncation, empty-prompt skip, opt-out/kill-switch, no-session breadcrumb); widened capture (grep/webfetch/websearch/task/mcp one-liners, URL-userinfo masking, empty-desc task fallback, WebSearch/Task prose preservation, mcp input not - read); prompt-only buffers not counted as unconsumed, including the - zero-conforming-line edge case; precompact idempotency (double-fire, - multi-seam, post-boundary). + read, asterisk-stripped tool_name); prompt-only buffers not counted as + unconsumed, including the zero-conforming-line and unreadable-file edge + cases; precompact idempotency (double-fire, multi-seam, post-boundary). ## [0.4.1] diff --git a/hooks/session-capture.sh b/hooks/session-capture.sh index ea7e269..4149833 100755 --- a/hooks/session-capture.sh +++ b/hooks/session-capture.sh @@ -77,8 +77,7 @@ line=$(printf '%s' "$input" | jq -r --arg root "$root" "$(tl_jq_redact_defs)"' (.tool_name as $t | if $t == "Bash" then "**bash** " + ((.tool_input.description // "") | redact | clean) + outcome($t) + " - `" + - (((.tool_input.command // "") | redact | clean) as $c - | ($c[0:200]) + (if ($c | length) > 200 then "…[truncated]" else "" end)) + "`" + ((.tool_input.command // "") | redact | clean | clamp(200; "…[truncated]")) + "`" elif ($t == "Edit" or $t == "Write" or $t == "NotebookEdit") then "**" + $t + "** " + ((.tool_input.file_path // .tool_input.notebook_path // "?") | ltrimstr($root + "/") | redact | clean) + @@ -119,8 +118,18 @@ line=$(printf '%s' "$input" | jq -r --arg root "$root" "$(tl_jq_redact_defs)"' # MCP tools (mcp__server__tool) and any other matched tool: name only. # Zero assumptions about the input schema, so no field can leak a secret # and no unverified shape can be misread — the tool name alone is the - # skimmable trace. - "**" + $t + "**" + outcome($t) + # skimmable trace. Unlike every other branch, $t here is embedded + # DIRECTLY INSIDE the `**...**` delimiter pair itself (every other + # branch uses a fixed literal type marker and puts field content only + # AFTER it), so an asterisk or control char in an unusual tool name + # would break the markdown bold span and desync the anchored + # `**[^*]+**` classifier regex used to skip prompt-only buffers in + # session-onboard.sh, silently miscounting the record as unparseable. + # `clean` handles control chars and backticks; asterisks are stripped + # here specifically, not folded into the shared `clean` def which + # other branches rely on to preserve a literal `*` in content like a + # glob pattern or command. + "**" + ($t | clean | gsub("\\*"; "")) + "**" + outcome($t) end) ' 2>/dev/null) || { tl_err "jq filter failed"; exit 0; } diff --git a/hooks/session-onboard.sh b/hooks/session-onboard.sh index 408533d..500aa33 100755 --- a/hooks/session-onboard.sh +++ b/hooks/session-onboard.sh @@ -182,10 +182,16 @@ if [ -d "$bufdir" ]; then # one of them is a prompt line; zero recognized lines falls through to be # counted, matching that prior fail-safe behavior instead of silently # dropping a real ended session. + # An unreadable/unparseable file (permissions, I/O error) makes grep -c + # print nothing at all rather than "0", so both counts default to 0 here - + # otherwise the comparison below gets an empty operand and this hook's + # "always silent on error" contract breaks (every other error path here is + # 2>/dev/null'd; an unguarded integer test on an empty string is the one + # way that contract leaks a diagnostic to stderr instead of failing quiet). # shellcheck disable=SC2016 - total=$(grep -cE '^- `[^`]*` \*\*[^*]+\*\*' "$f" 2>/dev/null) + total=$(grep -cE '^- `[^`]*` \*\*[^*]+\*\*' "$f" 2>/dev/null); total=${total:-0} # shellcheck disable=SC2016 - promptonly=$(grep -cE '^- `[^`]*` \*\*prompt\*\*' "$f" 2>/dev/null) + promptonly=$(grep -cE '^- `[^`]*` \*\*prompt\*\*' "$f" 2>/dev/null); promptonly=${promptonly:-0} [ "$total" -gt 0 ] && [ "$total" -eq "$promptonly" ] && continue if grep -q '^\n' > "$BUF/session-E O3a4=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") has "a buffer with zero conforming lines is still counted, not dropped" "$O3a4" 'unconsumed session buffer' +# 10a5. regression: an UNREADABLE buffer file makes grep -c print an empty +# string rather than "0" — the total/promptonly counts must default to 0 +# rather than feeding an empty operand to the integer test, which would +# otherwise leak a shell diagnostic to stderr, breaking this hook's +# always-silent-on-error contract (every other error path here is +# 2>/dev/null'd). Skipped when running as root, which bypasses permissions. +if [ "$(id -u)" != "0" ]; then + reset_buf + printf -- '- x\n' > "$BUF/session-T.md" + printf 'test' > "$BUF/session-UNREAD.md" + chmod 000 "$BUF/session-UNREAD.md" + ERR=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh" 2>&1 1>/dev/null) + eq "unreadable buffer file produces no stderr diagnostic" "$ERR" "" + chmod 644 "$BUF/session-UNREAD.md" 2>/dev/null +else + ok "unreadable-buffer stderr test skipped (running as root)" +fi + +# 10a6. regression: a tool_name containing an asterisk (the mcp__.* fallback +# branch is the only place a field is embedded DIRECTLY inside the **...** +# delimiter pair, not just after it) must not break the bold span or desync +# the anchored **[^*]+** classifier above — an action line that becomes +# unparseable must not silently be miscounted as prompt-only. +reset_buf +printf -- '- x\n' > "$BUF/session-T.md" +cap '{"session_id":"MCPSTAR","tool_name":"mcp__weird*tool","tool_input":{"title":"x"}}' +printf -- '\n' >> "$BUF/session-MCPSTAR.md" +O3a6=$(printf '%s' '{"source":"startup","session_id":"T"}' | sh "$H/session-onboard.sh") +has "an mcp tool_name with an asterisk is still counted as a real action" "$O3a6" 'unconsumed session buffer' + # 10b. a prior session buffer with NO end-stamp gets hedged wording, not # asserted as "ended" — it could be live in another terminal. reset_buf @@ -676,6 +706,12 @@ cap '{"session_id":"W","tool_name":"mcp__github__create_pull_request","tool_inpu MC=$(grep 'mcp__github' "$BUF/session-W.md") has "mcp tool captured name-only" "$MC" '**mcp__github__create_pull_request**' hasnt "mcp tool input fields are not read" "$MC" 'secret sauce' +# Regression: a tool_name containing an asterisk is embedded DIRECTLY inside +# the **...** delimiter pair (unlike every other branch, which only puts field +# content after a fixed literal marker) — an unstripped asterisk would break +# the bold span and desync onboard's **[^*]+** classifier regex. +cap '{"session_id":"W","tool_name":"mcp__weird*tool","tool_input":{}}' +has "mcp tool_name asterisk is stripped, not embedded in the bold marker" "$(grep weirdtool "$BUF/session-W.md")" '**mcp__weirdtool**' # 16. precompact stamp idempotency (issue #7): a double fire for one seam writes # one boundary; each genuine compaction (a captured action lands between) From 356a4b5b75cf1d4b26198536bc5e36f254ec861e Mon Sep 17 00:00:00 2001 From: Jason Irish Date: Mon, 6 Jul 2026 12:34:47 -0500 Subject: [PATCH 6/6] fix: CI shellcheck SC2218 - stop redefining prompt() mid-script tests/run.sh redefined the prompt() test helper twice inline (once to inject THROUGHLINE_DISABLE=1, once back to the original) to test the kill-switch path. CI's shellcheck (apt-installed on ubuntu-latest) flagged every call to prompt() between the definitions as SC2218 "This function is only defined later" - a redefinition pattern local shellcheck did not flag, caught only once the GitHub Actions run itself failed. Replaced the redefine-and-restore pattern with a direct inline pipe for the one disabled-state test, so prompt() is defined exactly once. Verified by running apt-get install shellcheck (matching CI's install method) against the fix in a fresh Ubuntu container - clean. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WqKPWvcm32Acg7UXJF2gQp --- tests/run.sh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/run.sh b/tests/run.sh index e07f0f3..7974679 100644 --- a/tests/run.sh +++ b/tests/run.sh @@ -665,10 +665,8 @@ printf '' > "$WORK/proj/.throughlineignore" prompt '{"session_id":"P3","prompt":"should be ignored"}' absent "prompt: .throughlineignore suppresses capture" "$BUF/session-P3.md" rm -f "$WORK/proj/.throughlineignore" -prompt() { printf '%s' "$1" | THROUGHLINE_DISABLE=1 sh "$H/session-prompt.sh"; } -prompt '{"session_id":"P4","prompt":"should be disabled"}' +printf '%s' '{"session_id":"P4","prompt":"should be disabled"}' | THROUGHLINE_DISABLE=1 sh "$H/session-prompt.sh" absent "prompt: THROUGHLINE_DISABLE suppresses capture" "$BUF/session-P4.md" -prompt() { printf '%s' "$1" | sh "$H/session-prompt.sh"; } # Missing session_id drops the prompt and breadcrumbs (mirrors capture). reset_buf prompt '{"prompt":"no session id here"}'