Skip to content

feat(context-guard): version-independent statusline shim for both guard plugins - #1252

Merged
kyle-sexton merged 6 commits into
mainfrom
feat/statusline-shim-durable-wiring
Jul 26, 2026
Merged

feat(context-guard): version-independent statusline shim for both guard plugins#1252
kyle-sexton merged 6 commits into
mainfrom
feat/statusline-shim-durable-wiring

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Both guard plugins' statusline wiring pointed at a version-pinned plugin-cache path. ${CLAUDE_PLUGIN_ROOT} changes on every plugin update and the old version directory is pruned ~14 days later (plugins reference, fetched 2026-07-24), so that wiring stops teeing silently at the next version bump and then takes the operator's whole statusline down when the path disappears (bash <missing> → 127, and the wrapped statusline never runs).

This ships scripts/statusline-shim.sh in context-guard and rate-limit-guard as the durable wiring target: the operator wires the shim once, and it resolves whichever tee version is installed at run time.

  • Resolution — newest installed tee by mtime across marketplaces (not a version sort: 0.9.0 sorts after 0.10.0 lexically and version dirs carry no semver guarantee), skipping transient temp_* cache clones. The marketplace name is never assumed, so the shim stays repo-agnostic.
  • Transparent in every path — tee found → exec bash <tee> "$@"; no tee → exec "$@" (statusline unaffected, only the snapshot is lost); no tee and no wrapped command → one diagnostic line instead of a blank bar, never an empty exec.
  • Free on the statusline path — pure Bash builtins (glob + -nt), no subprocess. Measured at ~14 ms added per refresh.

Setup-contract changes

  • context-guard:setup apply installs the shim alongside the zones.json seed/repair — a widening of its previously zones-only scope (the A4 EXEC-SHAPE entry is annotated in PLAN.md).
  • rate-limit-guard:setup gains an apply action for the first time; it writes the shim and nothing else.
  • The installed copy is byte-identical to the shipped source, so check's drift test is a plain cmp. Idempotent.
  • check gained an installed-shim state, and now classifies a statusLine wired to a plugin-cache path as LEGACY regardless of whether that file currently exists (the old rule only compared paths).
  • Statusline wiring stays print-only — neither plugin ever edits settings.json.

Security

New write surface (an executable into operator home) → delta security review recorded in docs/MIGRATION-PLAYBOOK.md, verdict ACCEPT. The copy is byte-identical to already-reviewed bundled code, lands in each plugin's already-accepted operator-home carve-out, and is inert until the operator wires it — so the base record's "no kill switch needed: nothing runs unless the operator wires it" justification survives verbatim. ${CLAUDE_PLUGIN_DATA} was considered and rejected as the shim's home (deleted on uninstall → reintroduces the exact 127 failure; per-plugin-identity path would hardcode the marketplace name into operator settings).

Composition + measured cost

The tees are transparent wrappers, so they nest — each through its own shim, innermost command owning stdout and the exit code. Both setup skills print that combined form and state the measured cost: ~0.6–0.9 s per statusline refresh per tee on Windows/Git Bash (process-spawn bound: jq + date; the shim itself is ~14 ms). Display latency, not input latency.

Verification

  • New black-box harness per plugin, 25 assertions each, all passing: resolution (newest-by-mtime beating the lexically-later name, temp_* clones skipped even when newer, marketplace name not assumed, sibling plugins ignored), transparency (stdin bytes, stdout, exit codes 0 and non-zero, both with and without a tee), the no-tee/no-args case, unset HOME, and two-shim chaining including a missing middle tee.
  • End-to-end run with the real tees: both contract files written, stdin preserved byte-for-byte (1025/1025), rc=0.
  • Existing suites still green: context-zone 44, context-guard tee 44, rate-limit-guard tee 34.
  • shellcheck clean; validate-plugins.sh, cross-plugin drift, changelog parity, skill-portability, skill leaf names, silent skips, hook argv all pass; check-changed-skills.sh PASS on both setup skills (0 errors).

Both plugins bumped to 0.2.0 with changelog entries.

Related

Closes #1251

…rd plugins

Replaces version-pinned statusline wiring with a shim the operator wires
once. ${CLAUDE_PLUGIN_ROOT} is version-pinned and the old version directory
is pruned ~14 days after an update (plugins reference, fetched 2026-07-24),
so a statusLine wired to <plugin-root>/scripts/statusline-tee.sh silently
stops teeing at the next version bump and then takes the operator's WHOLE
statusline down once the path disappears (bash <missing> -> 127).

scripts/statusline-shim.sh (both plugins) resolves the newest installed tee
at run time — newest by mtime across marketplaces, skipping transient temp_*
cache clones — and is transparent in every path: no tee installed degrades
to running the wrapped statusline alone; a wired-standalone shim prints one
diagnostic line rather than leaving a blank bar. Pure Bash builtins, so it
adds no subprocess to the statusline path.

setup apply installs it byte-identically to ~/.claude/<plugin>/bin/, which
widens context-guard's apply scope (previously zones.json only) and gives
rate-limit-guard an apply action for the first time. The copy is inert until
the operator applies the printed settings.json edit, so the context-guard
review record's no-kill-switch justification survives; recorded as a delta
security review in MIGRATION-PLAYBOOK.md. setup check gained an installed-
shim state check and now classifies cache-path wiring as LEGACY regardless
of whether that file still exists.

Both setup skills print the sibling-composition wiring (each tee through its
own shim, innermost command owning stdout) and state the measured per-tee
cost: ~0.6-0.9 s per refresh on Windows/Git Bash, process-spawn bound.

Resolves the plugin-audit-port PLAN's stable-shim open question (stress-test
#4) in favor of the shim; the interim [ -f ] guard and an inline settings
glob were both weighed and rejected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 48s —— View job


PR Review: feat(context-guard): version-independent statusline shim for both guard plugins

Status: Review complete.

Tasks:

  • Gather context and understand the request
  • Read the changed files
  • Review shim scripts for correctness and security
  • Review test harnesses
  • Review SKILL.md and documentation changes
  • Review security documentation (MIGRATION-PLAYBOOK.md)
  • Compile and post findings

Summary

The problem statement is correct and well-understood: ${CLAUDE_PLUGIN_ROOT} is version-pinned, the old directory is pruned ~14 days after an update, and wiring settings.json to it produces either a silent tee-stop (on version bump) or a 127 exit that takes the entire statusline down (after pruning). The shim is the right structural fix. The core implementation — glob+-nt scan, temp_* skip, exec transparency, and the no-args guard — is correct.


Findings

1. Settings-scope coverage asymmetry between the two SKILL.md files (medium)

context-guard/skills/setup/SKILL.md step 3 says to read all three settings scopes (user ~/.claude/settings.json, project .claude/settings.json, local .claude/settings.local.json), determine the most-specific effective statusLine, and print the edit targeting that file. If a project-level statusLine shadows the user-level one, the skill must print the edit for the project file, not the user file.

rate-limit-guard/skills/setup/SKILL.md step 3 only says:

read (never write) the user's ~/.claude/settings.json, and note any project-level statusLine that shadows it.

The local settings file is not mentioned, and "note any project-level shadow" is weaker than "print the edit targeting the shadowing file". An operator whose statusline is wired at project or local scope would get LEGACY detection applied to the wrong file (and the printed edit would target the wrong file too). The asymmetry isn't obviously intentional — rate-limit-guard is not architecturally simpler in a way that would justify reading fewer scopes.

Fix this →


2. Comment drift between the two shims on line 25 (low)

context-guard/scripts/statusline-shim.sh L25:

#   tee not found  → exec "$@"              (wrapped statusline runs unchanged;
#                                            the ONLY loss is the snapshot)

rate-limit-guard/scripts/statusline-shim.sh L25:

#   tee not found  → exec "$@"              (wrapped statusline runs unchanged;
#                                            the ONLY loss is the tee file)

"The tee file" sounds like the script file itself disappears; "the snapshot" (context-guard) or "the contract file" (matching the wording used elsewhere in rate-limit-guard docs) would be clearer. The two shims are intended to stay in sync — this comment divergence shows how drift accumulates without a drift gate on the script files themselves.


3. No CI gate enforcing shim-to-shim behavioral equivalence (low)

The two shim scripts are identical modulo PLUGIN_NAME and a few comments. The PR mentions cross-plugin drift CI checks pass, but those appear to cover SKILL.md files and skill names, not the new scripts/ files. A future one-line change to one shim but not the other would not be caught until the test harnesses diverge (and the tests are also nearly-identical, so a symmetric mistake in both would pass both). This is a pre-existing pattern in the repo, but it's worth noting given the PR adds two more such mirror files.


4. exit 0 after printf is redundant (cosmetic)

context-guard/scripts/statusline-shim.sh L82 / rate-limit-guard L83:

printf '%s: statusline tee not found — run /%s:setup check\n' \
  "$PLUGIN_NAME" "$PLUGIN_NAME"
exit 0

printf exits 0 on success (which is certain for this format string/args), and this is the last statement, so exit 0 is redundant. Not wrong, but every other code path uses exec (which doesn't return), making this the only path where the script exits normally. The explicit exit 0 is arguably a clarity choice, but it's noise if the intent is "fall off the end."


What's done well

Resolution algorithm: The mtime comparison with the temp_* skip is correct and handles the lexical-sort pitfall cleanly. The early-return when HOME is unset ([[ -n "${HOME:-}" ]] || return 0) is properly defensive. The -f guard after the glob handles the no-match expansion case correctly.

Transparency: All three exit paths preserve the invariant — tee or no tee, the wrapped command's exit code reaches the caller, and stdin is never buffered by the shim. The (($#)) guard before exec "$@" correctly prevents an empty exec with no args.

Test harness: The 11 scenarios cover the non-obvious cases (mtime vs lexical sort, temp_* skip even when newer, chaining with a missing middle tee). The plant_tee/make_wrapped/run helper structure is clean and self-contained. The stderr capture pattern (write to a temp file, capture after run) is correct.

Security delta review: The reasoning holds. The "inert until wired" property is the right basis for accepting the new write surface without a kill switch, and the ${CLAUDE_PLUGIN_DATA} rejection is correct (deleted on uninstall = the exact 127 failure this PR removes).

Docs/changelog discipline: Both plugins get changelog entries, version bumps to 0.2.0, README updates, updated evals, and the A4 EXEC-SHAPE decision recorded and annotated in PLAN.md. The delta review in MIGRATION-PLAYBOOK.md is properly scoped.


Verdict

The core implementation is correct and ready. Finding #1 (scope asymmetry in rate-limit-guard's check step 3) is the only issue with a real behavior consequence — an operator with project-scope wiring could get misleading output. Findings #2–4 are low-severity. Whether to fix before merge or in a follow-up is your call; #1 is the one I'd address before this lands.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 44s —— View job


Security Review

  • Load tools and set up task list
  • Fetch PR diff
  • Read changed shell scripts (statusline-shim.sh for both plugins)
  • Read setup SKILL.md files for both plugins
  • Review MIGRATION-PLAYBOOK.md delta security record
  • Analyze for injection, path traversal, privilege escalation, and other vulnerabilities
  • Post security findings

Findings

Scope: statusline-shim.sh (both plugins), setup SKILL.md (both plugins), MIGRATION-PLAYBOOK.md delta review, and test harnesses. GitHub Actions workflows were not changed; no CI surface is in scope.


SUGGESTION — mtime-based tee resolution has no integrity bound, creating a durable malware persistence vector

Severity: SUGGESTION · Confidence: PLAUSIBLE (requires prior same-user compromise)

context-guard shim, lines 57–66 / rate-limit-guard shim, lines 57–66

for cand in "$cache"/*/"$PLUGIN_NAME"/*/scripts/statusline-tee.sh; do
  [[ -f "$cand" ]] || continue
  rest="${cand#"$cache"/}"; mkt="${rest%%/*}"
  [[ "$mkt" == temp_* ]] && continue
  if [[ -z "$RESOLVED" || "$cand" -nt "$RESOLVED" ]]; then
    RESOLVED="$cand"
  fi
done

The shim resolves the tee by selecting the newest-by-mtime entry under $HOME/.claude/plugins/cache/ matching the glob, then exec bash "$RESOLVED" "$@" without any integrity check. The glob is open to any marketplace directory name (that's intentional for repo-agnosticism), which means any process running as the same user can create ~/.claude/plugins/cache/<any-name>/context-guard/999.0.0/scripts/statusline-tee.sh with a future mtime and have the shim execute it on the next statusline refresh.

The delta security review in MIGRATION-PLAYBOOK.md states the shim "has no untrusted input (its only inputs are its own argv and the cache directory listing)." The cache directory listing is an untrusted input from a defense-in-depth perspective: it's under $HOME and therefore writable by anything running as this user. This specific vector — plant a glob-matching file, get repeated execution on every refresh — is not analysed in the review record.

The privilege concern is bounded: this does not escalate privilege (the planting process must already run as this user). What it does provide is a stable, self-reinvoking execution vector on every statusline refresh that survives process exits. An attacker who can write to $HOME doesn't need to stay resident — the shim keeps re-executing the planted file until the operator runs apply and the planted file happens to be newer than the legitimate tee. That's a meaningful persistence quality for a drive-by or supply-chain-style local infection, even if it's not a privilege escalation.

Mitigation options to consider: hash the resolved path against the expected tee's checksum before exec'ing, or restrict the glob to a single known-good marketplace name (acknowledged as a design trade-off in the PR).


SUGGESTION — check's tamper indicator is framed as routine version lag, not a security signal

Severity: SUGGESTION · Confidence: CONFIRMED (by design)

context-guard SKILL.md, check step 2 / rate-limit-guard SKILL.md, check step 2

Present but differing — INFO, not FAIL: the installed copy is an older (or hand-edited)
revision that still resolves the newest tee. Report the shipped # shim-revision: marker
against the installed one and offer apply as the refresh.

check uses cmp -s to compare the installed shim against the bundled source. A diff is classified INFO with "older revision" framing regardless of cause — a routine version-lag and a tampered file produce identical output. An operator reading the INFO would naturally assume they haven't run apply since the last plugin update, not that the file may have been modified by something other than the plugin. This weakens check's value as a tamper-detection primitive.

Since the installed shim is the active statusline command (wired directly into settings.json), a modified shim executes on every session event and refresh interval. Keeping tamper and version-lag indistinguishable means an operator is unlikely to investigate further.

Mitigation: add a SHA of the shipped source to the # shim-revision: marker; check can then distinguish "this matches a known revision" from "this matches no known revision," producing a stronger signal when the diff is of the latter kind.


No findings — items examined and cleared

  • Command injection via $@: exec "$@" is properly double-quoted; arguments come from the operator's own settings.json. No shell metacharacter expansion at exec time.
  • exec bash "$RESOLVED": $RESOLVED is a path produced by a glob under the user's own $HOME; double-quoting prevents word-splitting or glob re-expansion at the exec call.
  • temp_* filter: [[ "$mkt" == temp_* ]] in [[ ]] context performs correct glob-style prefix matching — this is the intended Bash semantics.
  • Empty exec guard: (($#)) safely tests argument count before exec "$@"; $# is a special parameter always defined even under set -u.
  • HOME unset handling: [[ -n "${HOME:-}" ]] || return 0 correctly short-circuits resolution when HOME is absent.
  • Heredoc in test harness (plant_tee): \$# and \$@ are correctly escaped so they are owned by the fake-tee runtime, not the test shell. No injection.
  • Path traversal: PLUGIN_NAME is hardcoded in both shims; the remaining glob wildcards are bounded by the cache directory under HOME.
  • No network/telemetry surface: the shim reads no config and opens no sockets.
  • Write scope: apply stays inside ~/.claude/<plugin-name>/bin/; both SKILL.md files explicitly forbid writing into the sibling plugin's directory.

Both findings are defence-in-depth gaps contingent on prior same-user compromise, not new attack surfaces. The shim's design trade-offs (marketplace-agnostic glob, no resident process) are documented and the base-record justification ("inert until wired") holds. The delta security review in MIGRATION-PLAYBOOK.md covers the new write surface correctly but doesn't address the mtime-plant vector or the tamper-framing issue noted above.

…ord resolution evidence

check step 2 compared the installed shim against the shipped source with no
state for "the shipped source does not exist" — which is every consumer still
on 0.1.0, including this machine at authoring time. Both setup skills now
report that branch as INFO (installed plugin predates the shim; /plugin update
then re-run) and explicitly never call the operator's copy drifted there.

Both shims record the empirical basis for newest-by-mtime resolution: the cache
copy does NOT preserve source timestamps (a source committed 03:08 installed at
12:38 carried 12:38), so newest mtime is most-recently-installed even against an
orphaned older version directory. Notes the authoritative alternative
(~/.claude/plugins/installed_plugins.json) and why it is not used: undocumented
internal schema, and it would put a jq spawn on every statusline refresh.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 10s —— View job


PR Review: feat(context-guard): version-independent statusline shim for both guard plugins

Status: Review complete.

Tasks:

  • Gather context and understand the request
  • Review shim scripts for correctness and behavioral equivalence
  • Review SKILL.md files for scope asymmetry (prior review finding chore: initialize marketplace scaffold #1)
  • Review test harnesses
  • Review pre-shim version-skew check (commit 3c24a5d)
  • Compile and post findings

Context

This is a second-pass review. Two prior Claude reviews already landed (commit b34e5d4 review + security review). The latest push (commit 3c24a5d) adds:

  • A pre-shim version-skew state in both plugins' check step 2: when ${CLAUDE_PLUGIN_ROOT}/scripts/statusline-shim.sh does not exist (operator is on plugin < 0.2.0), skip the drift comparison entirely and report INFO rather than a false drift warning.
  • An empirical note in both shim scripts documenting that the cache copy carries the install timestamp, not the source commit timestamp — validating the mtime-based resolution algorithm.

Findings

1. Scope asymmetry in rate-limit-guard step 3 is still unaddressed (medium — carry-over from prior review)

context-guard SKILL.md L58–65:

read (never write) every settings scope that can carry a statusLine (user ~/.claude/settings.json, project .claude/settings.json, local .claude/settings.local.json) and determine which one owns the EFFECTIVE command (the most specific scope wins) … the printed edit in step 6 targets THAT scope's file

rate-limit-guard SKILL.md L57–58:

read (never write) the user's ~/.claude/settings.json, and note any project-level statusLine that shadows it.

The local settings file (.claude/settings.local.json) is not mentioned. "Note any project-level shadow" is also weaker than "determine the effective command and target that file." An operator with a project-scope or local-scope statusLine would get LEGACY detection applied to the wrong file and the printed edit targeting the wrong file. The new commit does not address this; it touched rate-limit-guard's SKILL.md only to add the shipped-source-absent branch in step 2.

Fix this →


2. New commit: pre-shim version-skew state is correct (no action needed)

context-guard SKILL.md L53–57 / rate-limit-guard L52–56

The new "SHIPPED source is absent" branch correctly handles the transition case: if the operator is running plugin version < 0.2.0, ${CLAUDE_PLUGIN_ROOT}/scripts/statusline-shim.sh does not exist, so comparing the operator's installed copy against it would be comparing against nothing. Skipping the comparison and reporting INFO (not a false FAIL/drift) is the right behavior. The remediation (/plugin update, re-run check) is clear. The instruction "never report the operator's installed copy as drifted on this branch" prevents false positives when an operator who previously had 0.2.0 installed later rolls back. Both plugins handle this branch identically.


3. New commit: empirical mtime note strengthens the design (no action needed)

context-guard shim L37–45 / rate-limit-guard L38–46

The observation that the cache copy carries install time (not source commit time) is the key premise of the mtime-based resolution algorithm. Recording the empirical basis here is the right call: it documents why newest-by-mtime equals most-recently-installed even against an orphaned older version directory, and it explains why the installed_plugins.json alternative was evaluated and rejected (undocumented schema, jq spawn cost on every refresh). The note is in both shims identically.


4. Comment drift on shim line 25 still present (low — carry-over)

context-guard shim L24: the ONLY loss is the snapshot
rate-limit-guard shim L24: the ONLY loss is the tee file

"The tee file" sounds like the script file disappears. "The contract file" or "the tee output" would match rate-limit-guard's elsewhere-used terminology and would parallel context-guard's "the snapshot" clearly. Not addressed in the new commit.


What's done well in this push

The pre-shim-absent logic is a meaningful gap filled: without it, check would crash or mis-classify on an operator running the brand-new plugin version before a prior apply had run, or on a consumer who has never been on 0.2.0 at all. The empirical timestamp note also closes a documentation gap — the mtime claim was previously asserted without evidence; now it has a measurement to back it up.


Verdict

Finding #1 (scope asymmetry in rate-limit-guard step 3) is the one outstanding issue with real behavior impact and was not addressed in this push. Findings #4 is low-severity. Findings #2 and #3 confirm the new commit's changes are correct. The overall implementation — shim resolution, transparency, test coverage, security delta review — remains sound.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 16s —— View job


Security Review — PR #1252

Tasks:

  • Gather context and understand the request
  • Read the PR diff and changed files
  • Audit shim scripts for injection, path traversal, and privilege issues
  • Audit SKILL.md for instruction-injection and credential exposure
  • Audit MIGRATION-PLAYBOOK.md delta security record
  • Review test harnesses for supply-chain / injection risk
  • Compile and post security findings

Scope: statusline-shim.sh and statusline-shim.test.sh (both plugins), skills/setup/SKILL.md (both plugins), MIGRATION-PLAYBOOK.md delta record, plugin.json version bumps, evals.json updates. No GitHub Actions changes in this PR.


Findings

IMPORTANT — Open-glob mtime-plant is an undisclosed persistent execution vector · Confidence: PLAUSIBLE

Location: context-guard/scripts/statusline-shim.sh L67–76 / rate-limit-guard/scripts/statusline-shim.sh L67–76

for cand in "$cache"/*/"$PLUGIN_NAME"/*/scripts/statusline-tee.sh; do
  [[ -f "$cand" ]] || continue
  rest="${cand#"$cache"/}"; mkt="${rest%%/*}"
  [[ "$mkt" == temp_* ]] && continue
  if [[ -z "$RESOLVED" || "$cand" -nt "$RESOLVED" ]]; then
    RESOLVED="$cand"
  fi
done
...
exec bash "$RESOLVED" "$@"

The temp_* filter is the only guard on marketplace directory names. Any process running as the same user can create $HOME/.claude/plugins/cache/any-name/context-guard/999.9.9/scripts/statusline-tee.sh with a future mtime and have it executed on every subsequent statusline refresh. Because the shim is wired into settings.json, this gives the planted file automatic, repeating invocation on every statusline refresh interval until the operator manually inspects the cache — with no visible error or diagnostic output (the statusline continues functioning normally, transparency is preserved).

This is a new attack surface introduced by this PR. The prior version-pinned wiring only executed one specific fixed path; the new glob-based resolution executes whatever matches the pattern, making the attack surface as wide as the operator's home directory. The MIGRATION-PLAYBOOK.md delta review states "the shim itself has no untrusted input (its only inputs are its own argv and the cache directory listing)" — but the cache directory listing is writable by same-user processes, and the review doesn't name the mtime-plant scenario as an acknowledged residual risk.

Precondition: attacker already has same-user write access to $HOME (this is not a privilege escalation). Persistence quality: the planted file fires on every statusline refresh, survives the attacker's process exit, is located in a directory that looks like a legitimate plugin cache, and produces no diagnostic output.

Mitigation paths discussed in the prior review but not addressed in the delta record: checksum the resolved path against the bundled tee's hash before exec'ing; or restrict the glob to a documented known-good marketplace name (breaking repo-agnosticism but bounding the glob).


SUGGESTION — exec bash "$RESOLVED" bypasses the execute-bit check · Confidence: CONFIRMED

Location: context-guard/scripts/statusline-shim.sh L82 / rate-limit-guard/scripts/statusline-shim.sh L82

exec bash "$RESOLVED" "$@"

Because bash is invoked explicitly as the interpreter, the target file's execute bit is immaterial — bash reads and executes any readable file. For the mtime-plant attack described above, this slightly lowers the barrier: a world-readable planted file suffices; the attacker does not need to chmod +x it. The design choice is intentional (Git Bash on Windows requires explicit bash invocation per the script's stated shell requirement), but the security side-effect is that the file-permission layer provides no defense in depth here.


Items examined and cleared

  • exec "$@" command injection$@ in both exec bash "$RESOLVED" "$@" and the fallback exec "$@" is properly double-quoted. Arguments originate from the operator's own settings.json command line.
  • Path traversal via glob wildcards — bash's * wildcard does not match /; the marketplace segment of the glob pattern is bounded to a single path component. A directory named ../../etc cannot be matched.
  • Symlink attack — requires the same same-user write access as the mtime-plant; no additional surface.
  • temp_* filter correctness[[ "$mkt" == temp_* ]] in a [[ ]] context is correct bash glob-style prefix matching.
  • Empty exec guard(($#)) safely tests argument count before exec "$@"; $# is always defined, so no set -u abort risk.
  • HOME unset handling[[ -n "${HOME:-}" ]] || return 0 is a correct defensive short-circuit.
  • Test harness heredoc injection$marker values are hardcoded test strings; \$# and \$@ in heredocs are correctly escaped for the generated script's runtime, not the test shell's expansion.
  • SKILL.md credential exposure — both SKILL.md files read settings.json only for the statusLine key; no tokens or secrets are extracted or printed.
  • plugin.json / evals.json changes — version bumps only; no new trust surfaces, dependencies, or permissions.
  • GitHub Actions — no workflow files changed; no pull_request_target, workflow_run, or github context injection surface introduced.
  • Network/telemetry — the shim reads no config and opens no sockets.

The core implementation is correctly scoped and the ACCEPT verdict for the new write surface (inert until wired, byte-identical to reviewed source, inside the already-accepted operator-home carve-out) is sound. The IMPORTANT finding above is the one item where the delta security record's characterization leaves an acknowledged residual risk unnamed — whether that's a blocker before merge is a judgment call for the maintainers.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3c24a5d5c1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/rate-limit-guard/skills/setup/SKILL.md
…ng form

Codex review (#1252, P2): the sibling-composition template substituted
<current statusline command> raw, so an operator whose statusline carries shell
syntax (THEME=dark my-statusline, a pipe, &&, ;) got THEME=dark as the wrapped
EXECUTABLE. Verified: raw substitution through the chain fails "THEME=dark:
command not found" (127); sh -c 'THEME=dark my-statusline' through the same
chain renders and exits 0.

Both setup skills now state that the innermost ARGV substitution rule is
unchanged by nesting — only the shim paths nest — and print the sh -c variant
of the combined form. rate-limit-guard's setup skill had no shell-syntax guard
at all (it predates the context-guard one); it now carries the full guard plus
the escaping rule.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 0s —— View job


PR Review: feat(context-guard): version-independent statusline shim for both guard plugins

Status: Review complete.

Tasks:

  • Gather context and understand the request
  • Inspect the latest commit (6b1c35f) — shell-syntax guard for combined wiring form
  • Check if scope asymmetry in rate-limit-guard step 3 (carry-over finding) was addressed
  • Check comment drift on shim line 24/25 (carry-over finding)
  • Review the combined-wiring shell-syntax fix (Codex finding)
  • Compile and post findings

Latest commit (6b1c35f): shell-syntax guard for combined wiring form

The Codex P2 finding is correctly addressed. Both plugins' setup skills now consistently handle shell-syntax commands across all wiring forms.

What changed in this commit:

  • context-guard SKILL.md — the <escaped original command> escaping rule was moved before the combined (two-shim) template so it's introduced on first use rather than after; an explicit "The shell-syntax guard applies UNCHANGED to this form" paragraph was added, followed by the concrete sh -c example for the shell-syntax + combined form. The ordering change makes the prose flow correctly: rule introduced, plain ARGV form shown, then rule extended to the combined form.

  • rate-limit-guard SKILL.md — gained the full shell-syntax guard section for the first time (it predated the context-guard one). Now carries: the single-shim shell-syntax template, the POSIX escaping rule, the combined-form template, and the explicit combined-form shell-syntax guard — all matching context-guard's structure.

The commit message states it was verified against the failure case: THEME=dark my-statusline raw → THEME=dark: command not found (127); sh -c 'THEME=dark my-statusline' through the same chain → renders and exits 0. That's the right verification for this fix.

One structural observation (no action needed): Both combined-wiring sections still show the plain <current statusline command> form first, then follow with the sh -c form and "use this when the command carries shell syntax." This is intentional and correct — two alternatives, with explicit selection criteria. The existing test harnesses confirm end-to-end transparency, including sh -c invocations.


Carry-over findings (not addressed in this commit)

1. Scope asymmetry in rate-limit-guard step 3 — still present (medium)

context-guard/skills/setup/SKILL.md L58–65:

read (never write) every settings scope that can carry a statusLine (user ~/.claude/settings.json, project .claude/settings.json, local .claude/settings.local.json) and determine which one owns the EFFECTIVE command (the most specific scope wins) … the printed edit in step 6 targets THAT scope's file

rate-limit-guard/skills/setup/SKILL.md L57–58:

read (never write) the user's ~/.claude/settings.json, and note any project-level statusLine that shadows it.

The local settings file (.claude/settings.local.json) is still not mentioned in rate-limit-guard's step 3, and "note any project-level shadow" is weaker than "determine the effective command and target that file." An operator whose statusLine lives at project or local scope gets LEGACY detection applied to the wrong file and the printed edit targeting the wrong file. The shell-syntax fix this commit addresses is in step 6; step 3 was not touched. This is the third review cycle this finding has survived — flagged at b34e5d4 review and 3c24a5d review.

Fix this →


2. Comment drift on shim line 24 — still present (low)

context-guard/scripts/statusline-shim.sh L24: the ONLY loss is the snapshot
rate-limit-guard/scripts/statusline-shim.sh L24: the ONLY loss is the tee file

"The tee file" sounds like the script file itself is missing; "the contract file" or "the tee output" matches rate-limit-guard's elsewhere-used terminology and parallels context-guard's "the snapshot." Unchanged from the prior two reviews.


Verdict

The latest commit is correct and clean — the Codex P2 finding (shell-syntax in combined wiring) is properly fixed in both plugins. Finding #1 (scope asymmetry in rate-limit-guard step 3) remains the only issue with real operator-impact. Finding #2 is low-severity polish. The core implementation — shim resolution, transparency, test coverage, security reasoning — continues to hold.

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 25s —— View job


Security Review — PR #1252

Tasks:

  • Load tools
  • Fetch PR diff / list changed files
  • Read shim scripts (both plugins)
  • Read SKILL.md files (both plugins)
  • Review MIGRATION-PLAYBOOK.md delta record
  • Audit latest commit (6b1c35f) specifically
  • Compile and post findings

Scope: statusline-shim.sh and statusline-shim.test.sh (both plugins), skills/setup/SKILL.md (both plugins), MIGRATION-PLAYBOOK.md delta record, plugin.json / evals.json bumps. No GitHub Actions files changed.

The three commits on this branch add the shim scripts, fix the pre-shim version-skew state, and (latest, 6b1c35f) apply the shell-syntax guard to the combined wiring form. Each is covered below.


IMPORTANT — Open-glob mtime-plant vector not named as acknowledged residual risk in the delta record · Confidence: PLAUSIBLE (same-user compromise required)

Location: context-guard/scripts/statusline-shim.sh L67–76 / rate-limit-guard/scripts/statusline-shim.sh L67–76

The resolution loop's only guard on marketplace directory names is the temp_* prefix filter:

for cand in "$cache"/*/"$PLUGIN_NAME"/*/scripts/statusline-tee.sh; do
  [[ -f "$cand" ]] || continue
  rest="${cand#"$cache"/}"; mkt="${rest%%/*}"
  [[ "$mkt" == temp_* ]] && continue
  if [[ -z "$RESOLVED" || "$cand" -nt "$RESOLVED" ]]; then
    RESOLVED="$cand"
  fi
done
exec bash "$RESOLVED" "$@"

$HOME/.claude/plugins/cache/ is writeable by any process running as the operator. An attacker or supply-chain-compromised tool with same-user write access can:

  1. mkdir -p "$HOME/.claude/plugins/cache/evil/context-guard/999.9.9/scripts/"
  2. Write arbitrary bash to statusline-tee.sh in that path
  3. Set a future mtime with touch -t 203001010000

The shim then executes the planted file on every subsequent statusline refresh. The statusline continues rendering normally (exec bash "$RESOLVED" "$@" preserves transparency), so there is no visible signal. The planted file survives the attacker's process exit and keeps firing until apply is re-run with a legitimate newer tee.

The MIGRATION-PLAYBOOK.md delta review states: "the shim itself has no untrusted input (its only inputs are its own argv and the cache directory listing)" — but the cache directory listing is writable by same-user processes and is therefore not free of untrusted input from a defense-in-depth standpoint. This specific scenario is not named as an acknowledged residual risk in the record.

Precondition: attacker already has same-user write access to $HOME (no privilege escalation). Mitigation paths already documented in the prior review: hash the resolved path against the bundled tee before exec'ing, or restrict the marketplace wildcard to a documented known-good name (breaking repo-agnosticism).

Recommendation: add a one-sentence acknowledgement in the delta review that the mtime-plant vector is a known, accepted residual risk contingent on prior same-user write-access compromise, and note which mitigation was rejected and why (consistent with how the ${CLAUDE_PLUGIN_DATA} rejection is documented).


SUGGESTION — exec bash "$RESOLVED" bypasses the execute-bit as a defense layer · Confidence: CONFIRMED

Location: context-guard/scripts/statusline-shim.sh L82 / rate-limit-guard/scripts/statusline-shim.sh L82

Because bash is invoked explicitly, any readable file matching the glob is executed regardless of its execute bit. The design requires explicit bash invocation (Git Bash / Windows compatibility, documented in the script), so this is intentional — but it means file permissions provide no defense in depth against the mtime-plant vector above. Worth noting alongside the acknowledged residual risk.


SUGGESTION — check's drift report does not distinguish version-lag from tampering · Confidence: CONFIRMED

Location: context-guard/skills/setup/SKILL.md L50–52 / rate-limit-guard/skills/setup/SKILL.md L49–51

Present but differing — INFO, not FAIL: the installed copy is an older (or hand-edited)
revision that still resolves the newest tee. Report the shipped # shim-revision: marker
against the installed one and offer apply as the refresh.

cmp -s can tell whether two files differ; it cannot tell whether the diff is version-lag or tampering. Both cases are framed as "older (or hand-edited) revision," which is the routine explanation an operator would accept without further investigation. Since the installed shim is the active statusline command (wired directly into settings.json), a tampered copy executes on every refresh. Adding the SHA of the bundled source to the # shim-revision: marker — and having check compare hashes rather than only content — would distinguish "matches a known good revision" from "matches no known revision."


Latest commit (6b1c35f) — no new security surfaces introduced

The commit applies the shell-syntax guard to the combined wiring form in both SKILL.md files. Specifically examined:

  • POSIX escaping instruction (''\''): this is the correct and complete escape for POSIX single-quoted strings. An operator whose existing statusline command contains ' will get a syntactically valid sh -c '...' invocation if Claude applies the instruction correctly. No injection vector.
  • sh -c invocation scope: the string passed to sh -c is derived from the operator's own existing settings.json command — there is no external or untrusted source involved. The operator reviews the printed output before applying it.
  • rate-limit-guard SKILL.md previously had no shell-syntax guard at all: the commit adds the full guard plus the escaping rule. The added text is correct; it does not introduce new execution surfaces.

Items examined and cleared

  • exec "$@" injection: double-quoted; arguments come from the operator's own settings.json. No shell metacharacter expansion at exec time.
  • Glob path traversal: * does not match /; marketplace and version segments are each bounded to one path component. A directory named ../../etc cannot be matched.
  • temp_* filter correctness: [[ "$mkt" == temp_* ]] in [[ ]] context is correct bash glob-style prefix matching.
  • Empty exec guard: (($#)) safely tests argument count; $# is always defined.
  • HOME unset handling: [[ -n "${HOME:-}" ]] || return 0 correctly short-circuits when HOME is absent.
  • Test harness heredoc injection: \$# and \$@ are correctly escaped; test variables are hardcoded strings, not operator-controlled input.
  • Credential exposure in SKILL.md: both files read settings.json for the statusLine key only; no tokens or secrets are extracted or printed.
  • plugin.json / evals.json: version bumps only; no new trust surfaces or permissions.
  • GitHub Actions: no workflow files changed.
  • Network / telemetry: the shim reads no config and opens no sockets.

Verdict

The core implementation is sound and the ACCEPT verdict for the new write surface is correct. The IMPORTANT finding names the one gap in the security record: the mtime-plant vector is a real (same-user-bounded) residual risk that the delta review does not acknowledge by name. Closing that gap requires only a one-sentence addition to the delta record — the mitigation analysis already exists in the prior review comments. The two SUGGESTION findings are defense-in-depth gaps; neither is a blocker.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Per-finding classification — all reviewer findings on this PR

Validated against head 6b1c35fbc270234166eaa08a87bb4740aa8a165c by reading the files directly, not by trusting the review text. One row per finding as raised, so a carry-over re-raised in a later cycle gets its own row.

# Source Severity Finding Classification Evidence Reacted
1 5072981310 #1 medium rate-limit-guard setup step 3 reads only user settings.json; omits project + local scopes and effective-scope targeting VALID — fix now rate-limit-guard/skills/setup/SKILL.md L57-58 reads read (never write) the user's ~/.claude/settings.json, and note any project-level statusLine that shadows it, vs context-guard L58-65 every settings scope … (user …, project .claude/settings.json, local .claude/settings.local.json) + the printed edit in step 6 targets THAT scope's file 👍
2 5072981310 #2 low Contract-comment drift between the mirrored shims on the no-tee line VALID — fix now rate-limit-guard/scripts/statusline-shim.sh L24 the ONLY loss is the tee file vs context-guard L24 the ONLY loss is the snapshot; tee file reads as "the script is missing", and L27 of the same file already says contract directory 👍
3 5072981310 #3 low No CI gate enforces shim-to-shim behavioral equivalence VALID (defer) Confirmed: scripts/cross-plugin-source-registry.txt lists only hooks/hook-utils.sh, reference/artifact-protocol.md, reference/standards-contract.md; check-cross-plugin-source-drift.sh compares sha256sum and only clusters byte-identical files, so deliberately non-identical mirrors are structurally out of its reach. A new equivalence gate is a CI-design change beyond this PR's branch-owned scope — belongs in its own issue
4 5072981310 #4 cosmetic exit 0 after the final printf is redundant VALID (defer) Confirmed present (context-guard L92 / rate-limit-guard L93). The script sets set -uo pipefail with no -e, and this is the one non-exec normal-exit path, so the explicit exit 0 documents the terminal branch. Cosmetic-only; not worth a churn commit on a branch under active security review
5 5072983710 SUGGESTION mtime-based tee resolution has no integrity bound (open marketplace glob permits a same-user planted tee) VALID (defer) Same root cause as #9/#13 below. Confirmed at statusline-shim.sh L67-77: the only name guard is [[ "$mkt" == temp_* ]] && continue. Disposition is the security-record amendment in #13 — see there
6 5072983710 SUGGESTION # shim-revision: is a bare counter, so check cannot distinguish version-lag from tampering VALID (defer) Confirmed: marker is # shim-revision: 1 (context-guard L54 / rate-limit-guard L55), and check's "Present but differing" state is a cmp + marker report. Adding a source SHA and hash comparison changes the check contract and the setup-skill spec — a design change, not a branch-owned fix; needs its own issue and its own security read
7 5073032526 #1 medium (carry-over) Scope asymmetry still unaddressed after 3c24a5d VALID — fix now Re-raise of #1; git show --stat 6b1c35fb touched rate-limit-guard/skills/setup/SKILL.md for the shell-syntax guard only, leaving L57-58 unchanged 👍
8 5073032526 #4 low (carry-over) Comment drift still present after 3c24a5d VALID — fix now Re-raise of #2; 6b1c35f touched no file under scripts/
9 5073033418 IMPORTANT Open-glob mtime-plant vector not named as an accepted residual risk in the delta security record VALID (defer) Confirmed inaccurate doc claim: docs/MIGRATION-PLAYBOOK.md L852-853 still reads the shim itself has no untrusted input (its only inputs are its own argv and the cache directory listing), and that listing is same-user writable. Deferred deliberately — see the note under the table 👍
10 5073033418 SUGGESTION exec bash "$RESOLVED" makes the target's execute bit immaterial VALID (defer) Confirmed and intentional: explicit bash invocation is the documented Git Bash / Windows requirement (statusline-shim.sh L16-18). Rides with the #9 record amendment as a defense-in-depth note; no code change warranted
11 5073176853 #1 medium (3rd cycle) Scope asymmetry — still present VALID — fix now Re-raise of #1/#7; verified still present at 6b1c35f 👍
12 5073176853 #2 low (3rd cycle) Comment drift on shim line 24 — still present VALID — fix now Re-raise of #2/#8; verified still present at 6b1c35f
13 5073177394 IMPORTANT Delta record should name the mtime-plant vector as an accepted residual risk VALID (defer) Root-cause duplicate of #5/#9; the finding is real and the recommended remedy is a one-sentence acceptance statement. Deferred deliberately — see the note under the table 👍
14 5073177394 SUGGESTION Execute-bit bypass worth noting alongside the acknowledged residual risk VALID (defer) Duplicate of #10; rides with the same record amendment
15 5073177394 SUGGESTION Put the bundled source's SHA in # shim-revision: and have check compare hashes VALID (defer) Duplicate of #6; same design-change disposition
16 3647320851 P2 (codex) Preserve shell-syntax commands in the combined wiring form VALID — already fixed Fixed in 6b1c35f; thread resolved. No further action

Why the security-record findings (#5 / #9 / #10 / #13 / #14) are deferred, not fixed here

They are all one root cause and the recommended remedy is to write an acceptance statement into docs/MIGRATION-PLAYBOOK.md — declaring the mtime-plant vector a known, accepted residual risk. That is a security-posture decision on this repo's plugin-acceptance record, and the finding is correct that the record's current "no untrusted input" sentence is inaccurate. Signing an accepted-risk statement is the maintainer's call, not an automated one, so this loop classifies and reports it rather than authoring the verdict. The concrete text the review recommends:

the mtime-plant vector is a known, accepted residual risk contingent on prior same-user write-access compromise

together with why the two named mitigations (hash the resolved path before exec; restrict the marketplace wildcard to a known-good name) were rejected — the latter breaking repo-agnosticism — recorded the same way the ${CLAUDE_PLUGIN_DATA} rejection already is.

Worth weighing when signing: the stated precondition (same-user write access to $HOME) already permits editing ~/.claude/settings.json or the installed shim directly, so the vector widens the surface rather than the capability.

Fixing now

#1/#7/#11 (settings-scope asymmetry) and #2/#8/#12 (comment drift) — both branch-owned, both verified still present at 6b1c35f. Follow-up with the commit SHA below.

check step 3 read only the user's ~/.claude/settings.json and merely noted a
project-level shadow, so an operator whose statusLine lives at project or local
scope got LEGACY detection applied to the wrong file and the step 6 edit printed
for the wrong file — it would apply cleanly and never run. context-guard's step
3 already scanned all three scopes and targeted the effective one; this closes
the asymmetry with the same wording.

The shim's no-tee contract line called the lost artifact "the tee file", which
reads as the tee SCRIPT being gone rather than the snapshot it writes; the same
header already says "contract directory" three lines down.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 23s —— View job


PR Review: fix(rate-limit-guard): scan every settings scope in setup check step 3

Status: Review complete.

Tasks:


What this commit fixes

This commit closes the two outstanding carry-over findings that survived three prior review cycles. Both changes are surgical — only two files touched, each targeting the exact identified gap.

Finding #1/#7/#11 — Settings-scope asymmetry in rate-limit-guard step 3: FIXED ✓

rate-limit-guard/skills/setup/SKILL.md L57–64

Before:

read (never write) the user's ~/.claude/settings.json, and note any project-level statusLine that shadows it.

After — now matches context-guard verbatim:

read (never write) every settings scope that can carry a statusLine (user ~/.claude/settings.json,
project .claude/settings.json, local .claude/settings.local.json) and determine which one owns the
EFFECTIVE command (the most specific scope wins). All wiring states below are evaluated against that
effective command, and the printed edit in step 6 targets THAT scope's file — wiring the user file
while a project-level statusLine shadows it would apply cleanly and never run; when a shadow exists,
say so explicitly and print the edit for the shadowing file (or note that removing the override is
the alternative).

The fix is complete: all three scopes are named, effective-scope determination is explicit, the step 6 targeting is keyed to the right file, and the shadow case has a concrete action (not just "note it"). An operator with a project-scope or local-scope statusLine no longer gets misleading output.

Finding #2/#8/#12 — Comment drift on shim L24: FIXED ✓

rate-limit-guard/scripts/statusline-shim.sh L24

Before: the ONLY loss is the tee file
After: the ONLY loss is the contract file

"Contract file" matches the "contract directory" language three lines below and makes it clear it's the snapshot/output that's lost, not the tee script itself. Consistent with context-guard's "the snapshot."


All prior findings — final state

Finding Description Status
#1/#7/#11 Scope asymmetry in rate-limit-guard step 3 Fixed in this commit
#2/#8/#12 Comment drift tee filecontract file on shim L24 Fixed in this commit
#16 Shell-syntax guard missing from combined wiring form Fixed in 6b1c35f
#3 No CI gate for shim-to-shim behavioral equivalence Deferred (own issue; beyond branch scope)
#4 Redundant exit 0 after final printf Deferred (cosmetic; explicit exit documents terminal branch)
#5/#9/#13 mtime-plant vector not named as accepted residual risk in delta record Deferred (maintainer security-posture call, not automated)
#6/#15 check cannot distinguish version-lag from tampering Deferred (design change; needs own issue)
#10/#14 exec bash bypasses execute-bit; rides with delta-record amendment Deferred

Verdict

All branch-owned findings are resolved. The implementation — shim resolution algorithm, transparency, test coverage (25 assertions per plugin), security delta review, changelog discipline, and wiring guidance — is correct and complete. The three deferred items are documented, intentional, and appropriately scoped to follow-up work rather than this branch.

Ready to merge.

@claude

claude Bot commented Jul 25, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 11s —— View job


Security Review — PR #1252 (commit 46c19e3)

Tasks:

  • Load tools
  • Fetch PR diff / read changed files
  • Audit latest commit (46c19e3) — rate-limit-guard scope fix
  • Re-audit shim scripts (both plugins) at HEAD
  • Audit SKILL.md files (both plugins) at HEAD
  • Review MIGRATION-PLAYBOOK.md delta record
  • Compile and post findings

Scope: statusline-shim.sh and statusline-shim.test.sh (both plugins), skills/setup/SKILL.md (both plugins), MIGRATION-PLAYBOOK.md delta record, plugin.json / evals.json bumps. No GitHub Actions files changed.


Latest commit (46c19e3) — no new security surfaces introduced

The commit fixes settings-scope asymmetry in rate-limit-guard step 3 (carry-over finding #1/#7/#11) and corrects a comment in the shim (carry-over finding #2/#8/#12). Changes are:

  • rate-limit-guard/skills/setup/SKILL.md step 3: now reads three settings paths (~/.claude/settings.json, .claude/settings.json, .claude/settings.local.json) instead of one, to determine the effective scope. This is entirely read-only — the skill's "never write" constraint is unchanged. No new execution surface. The three target paths are fixed literals; no user-controlled content reaches the path derivation.
  • rate-limit-guard/scripts/statusline-shim.sh L24: comment change "tee file""contract file". Pure documentation.

Both changes are clean.


IMPORTANT — mtime-plant vector still absent from the delta security record · Confidence: PLAUSIBLE (same-user compromise required)

Location: docs/MIGRATION-PLAYBOOK.md L852–857

The delta review record still reads:

The shim itself has no untrusted input (its only inputs are its own argv and the cache directory listing), performs no filesystem writes, and execs either the resolved tee or the wrapped command. The resolution glob skips transient temp_* marketplace directories…

The claim "no untrusted input" is inaccurate from a defense-in-depth standpoint: $HOME/.claude/plugins/cache/ is writable by any process running as the operator, making the cache directory listing a same-user-writable input. Any such process can create $HOME/.claude/plugins/cache/<any-name>/<plugin>/999.9.9/scripts/statusline-tee.sh with a future mtime and have the shim execute it on every subsequent statusline refresh — silently and persistently, because the statusline continues rendering normally.

This finding has been raised in every review cycle (four times, across two security reviews) and kyle-sexton's classification table explicitly defers it as a security-posture decision for the maintainers. The MIGRATION-PLAYBOOK.md was not touched in this commit. The recommended remedy — a one-sentence acceptance statement naming the vector as a known residual risk, with the rationale for why neither named mitigation was applied — remains unimplemented. The two mitigations discussed across prior reviews:

  1. Hash the resolved path against the bundled tee's checksum before exec — not implemented (would require a subprocess for the hash, adding latency to every refresh).
  2. Restrict the marketplace wildcard to a documented known-good name — not implemented (breaks the repo-agnostic design goal).

Neither rejection is currently recorded in the delta review, even though the ${CLAUDE_PLUGIN_DATA} rejection is documented to the same standard. The gap is in the record, not the code.


SUGGESTION — exec bash "$RESOLVED" bypasses the execute bit as a defense layer · Confidence: CONFIRMED

Location: context-guard/scripts/statusline-shim.sh L82 / rate-limit-guard/scripts/statusline-shim.sh L82

Intentional (documented Git Bash / Windows requirement). A planted file needs only to be readable, not executable. Worth noting alongside the mtime-plant acknowledgment — no code change warranted.


SUGGESTION — check drift report does not distinguish version-lag from tampering · Confidence: CONFIRMED

Location: context-guard/skills/setup/SKILL.md L50–52 / rate-limit-guard/skills/setup/SKILL.md L49–51

cmp -s on the installed shim reports "older (or hand-edited) revision" for both routine version-lag and tampering. Since the installed shim is the active statusline command, a tampered copy would execute on every refresh with no visible signal beyond an INFO-level drift notice. Adding a source SHA to # shim-revision: and comparing hashes in check would let it distinguish "matches a known revision" from "matches no known revision." This is a design-change in the check contract — a separate issue, not a blocker.


Items examined and cleared

  • exec "$@" injection — double-quoted; arguments sourced from the operator's own settings.json. No metacharacter expansion at exec time.
  • Glob path traversal* does not match /; marketplace and version segments are bounded to a single path component each. ../../etc-style names cannot match.
  • temp_* filter correctness[[ "$mkt" == temp_* ]] inside [[ ]] is correct Bash glob-style prefix matching.
  • Empty exec guard(($#)) safely tests argument count; $# is always defined under set -u.
  • HOME unset handling[[ -n "${HOME:-}" ]] || return 0 correctly short-circuits resolution.
  • Latest commit scope expansion — step 3 now reads .claude/settings.local.json, which may reside in an operator-controlled project directory. The operation is strictly read-only (read (never write) constraint in both SKILL.md files); no path is derived from user-controlled content.
  • Test harness heredoc\$# and \$@ in plant_tee's heredoc are correctly escaped. Test variable values are hardcoded.
  • SKILL.md credential exposure — both files read settings.json only for the statusLine key; no tokens or secrets extracted or printed.
  • plugin.json / evals.json — version bumps only; no new trust surfaces or permissions.
  • GitHub Actions — no workflow files changed.
  • Network / telemetry — shim reads no config and opens no sockets.

The core implementation and the ACCEPT verdict for the new write surface remain sound. The one open item is the IMPORTANT finding above, which requires only a record amendment — the technical implementation is not at issue.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Follow-up to the classification above — the two VALID — fix now findings are fixed in 46c19e3f7a3524c267e43d357a49fbcec34eba58.

Verified before pushing: statusline-shim.test.sh 25/25 pass, shellcheck clean, validate-plugins.sh pass, check-skill.sh on the changed skill PASS (0 errors — the two warnings are pre-existing), markdownlint-cli2 0 errors.

The remaining findings stay open by classification, not by omission: the security-record amendment (#5 / #9 / #10 / #13 / #14) needs a maintainer to sign the accepted-residual-risk statement, and the two design changes (#3 CI equivalence gate, #6 / #15 hashed # shim-revision:) want their own issues. No thread resolved and no merge from this pass.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46c19e3f7a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/context-guard/scripts/statusline-shim.sh Outdated
Comment thread plugins/context-guard/skills/setup/SKILL.md Outdated
kyle-sexton added a commit that referenced this pull request Jul 25, 2026
#1274)

Closes #1268

## Summary

`docs/topics/plugin-audit-port/` rejected `context: fork` for the
deep-audit phase on the stated grounds that a fork "inherits the
degraded history the gate exists to escape" (`PLAN.md:78-79`, and again
in the alternatives table at `:457`).

That is false for `context: fork`. Per the [skills
reference](https://code.claude.com/docs/en/skills) (fetched 2026-07-24)
a forked skill "won't have access to your conversation history" — it
would satisfy the fresh-eyes requirement it was being rejected for.

**The decision is unchanged.** The real basis was already recorded in
the same PLAN's `[EXEC-SHAPE]` agent decision at `:550`: plugin agents
start with fresh context, and the Brief requires a *named* subagent,
which a forked skill is not. Both prose sites now carry that basis, and
`design/design-resolution.md` — which stated the rule with no rationale
at all — gains it.

A wrong rationale in a design record propagates into later decisions
that cite it, which is why the reason is corrected even though the
outcome stands.

## Test plan

- `grep -rn -i "degraded history" docs/topics/plugin-audit-port/` — no
matches
- The `[EXEC-SHAPE]` decision at `:550` is unmodified; the corrected
text points at it rather than restating it
- Docs-only change; no plugin version or CHANGELOG bump applies

## Related

**One claim is deliberately attributed rather than asserted.** History
inheritance is *documented* for the Agent tool's separate `fork`
subagent type. #1258 reports empirically that Agent-tool forks did
**not** inherit the conversation, contradicting that doc. The corrected
text therefore says "documented for", not "does". The correction here
does not depend on how #1258 resolves — the named-subagent requirement
is the load-bearing basis, and the skills reference settles the
`context: fork` half independently.

Prior art, both closed: #1053 fixed the identical inversion in
`docs-hygiene`'s `audit-derivability` rubric and evals; #1062 clarified
its Hard Rules wording. This is the same class of error in a different
file, not a duplicate.

**Adjacent, non-overlapping:** #1252 also edits
`plugin-audit-port/PLAN.md`, at `:551` — the table row directly below
the `[EXEC-SHAPE]` row this PR points at. It does not touch that row,
and this PR does not touch `:551`.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…durable-wiring

# Conflicts:
#	docs/topics/plugin-audit-port/PLAN.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9c276e95c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/context-guard/skills/setup/SKILL.md Outdated
Comment thread plugins/context-guard/skills/setup/SKILL.md
kyle-sexton added a commit that referenced this pull request Jul 26, 2026
…n specifies

Closes #1417

## Summary

`docs/conventions/topic-docs/README.md` specifies a required check that a
merged PR carries no path under the contract-slice dir. The check was
never built, so the convention has been unenforced for its entire life
and 17 slices reached `main` — 6 of them on a single day. The only place
`docs/topics/` reached CI at all was `scripts/docs-only-paths.txt`, as a
docs-only ALLOWLIST entry, which makes such a PR cheaper to merge rather
than blocking it.

Evidence the rule is real and was being enforced by hand: PR #1286 was
closed rather than merged, explicitly because its content was contract
tier under `docs/topics/`.

## The deletion exemption

The convention's own step 4 is a final commit that PRUNES the slice, so a
literal "no path under the contract dir appears in the diff" reading
would red-line the very commit that satisfies it. This gate keys on where
a path LANDS: removals pass, a history-preserving `git mv` out of the
contract dir (step 3's graduation) passes, and only an add, edit, or
rename-into is red-lined.

That requires knowing a path's status, which `--name-only` cannot express,
so the gate reads `--name-status`. Deliberate deviation from the letter of
the convention in service of its intent; the three-dot `base...HEAD` range
is unchanged.

## Existing debt

The 17 pre-existing slices are grandfathered by slug in
`scripts/contract-slice-baseline.txt`, using the same stale-guarded idiom
as `changelog-parity-baseline.txt` and `orphaned-fixtures-baseline.txt`:
`--check` fails on an entry whose slice no longer exists, so an exemption
cannot outlive its debt and a future slice cannot inherit a grandfathered
slug. Graduating and pruning them is tracked separately.

This is why the gate can land now instead of after a 71-file cleanup: it
stops the bleed immediately while each slice graduates on its own PR, by
whoever owns it.

## Verification

The 11-case suite covers the add, pure-deletion, untouched, grandfathered,
new-slug-despite-baseline, graduation-out, rename-into, unresolvable-base,
live-baseline, stale-baseline, and usage paths.

Measured against the four open PRs that carry `docs/topics/` paths, rather
than asserted: #1318, #1252, and #1096 pass on their baseline exemptions;
#1400 fails, correctly, because it adds two slices that are not
pre-existing debt.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 26, 2026
…n specifies (#1429)

Closes #1417

## Problem

`docs/conventions/topic-docs/README.md:305-309` specifies a required
check that the net PR diff
carries no path under the contract-slice dir:

> 5. Enforcement: a required check that the net PR diff (`git diff
--name-only base...head`)
> contains no path under the resolved `<contract_dir>/**` (default
`docs/topics/**`).

It was never built. The convention has been unenforced for its entire
life, and `main` currently
holds **19** contract slices — **8 of them landed on 2026-07-25 alone**,
two of those while this PR
was open.

The only place `docs/topics/` reaches CI today is
`scripts/docs-only-paths.txt:43`, and it is there
as a docs-only **allowlist** entry — which makes a PR confined to it
*cheaper* to merge by skipping
the heavy lanes. Nothing blocks it.

The rule is real and people have been enforcing it by hand: **PR #1286
was closed rather than
merged**, explicitly because its content was contract tier under
`docs/topics/`. That is the cost
of the missing gate — correct behaviour depending on whoever is looking
remembering an unenforced
rule, and 19 directories showing how reliably that scales.

**#1400 merged while this PR was open**, landing two more slices. It was
flagged there, and it is
the sharpest available evidence for the gate: the failure mode is live,
not historical.

## Why the gate permits deletions

The convention's own step 4 is *a final commit that prunes the slice*. A
literal reading — "no path
under the contract dir appears in the diff" — would red-line the very
commit that satisfies the
convention.

So the gate keys on where a path **lands**, not on whether it appears:

| Change | Verdict |
|---|---|
| Delete under `docs/topics/` | pass — this is the prune step |
| `git mv docs/topics/x/PLAN.md docs/adr/…` | pass — this is step 3's
history-preserving graduation |
| Add / edit under `docs/topics/` | **fail** |
| Rename *into* `docs/topics/` | **fail** |
| Diff never touches `docs/topics/` | pass |

Knowing a path's status requires `--name-status`; `--name-only` cannot
express it. That is a
deliberate deviation from the letter of the convention in service of its
intent, called out in the
script header. The three-dot `base...HEAD` range is unchanged, so a
slice `main` gained after a
branch forked stays out of scope and no stale branch is forced to
merge-from-main over someone
else's violation.

## Why this lands before the cleanup

`scripts/contract-slice-baseline.txt` grandfathers the 19 existing
slugs, using the same
stale-guarded idiom as `changelog-parity-baseline.txt` and
`orphaned-fixtures-baseline.txt`:
`--check` fails on an entry whose slice no longer exists, so an
exemption cannot outlive its debt
and a future slice cannot silently inherit a grandfathered slug.

Exemptions are resolved from the **base revision**, not the working
tree, so a PR cannot add a
slice and grandfather its own slug in the same diff. The diff is judged
against the union of the
base and head contract roots, so a PR that relocates `contract_dir`
cannot leave the root it
selected uninspected either. Both bypasses were live in earlier pushes
and were caught in review.

The alternative — prune all 19 first, then gate — is a ~1.3 MB change
requiring a graduation
judgement on each slice by whoever owns it, and it would conflict every
open PR that carries those
paths. Gating first stops the bleed immediately while each slice
graduates on its own PR at its own
pace. The burn-down is #1419; each prune PR drops its own baseline line,
and the stale guard means
the debt cannot be quietly abandoned half-done.

## Verification

`scripts/check-contract-slice-prune.test.sh` — 19 cases, all green: add,
pure deletion, untouched
tree, grandfathered slug, new slug despite a baseline, graduation out,
rename in,
unresolvable-base-ref (fail-closed, exit 2), live baseline entry, stale
baseline entry, usage,
self-grandfathering rejected, a pre-existing entry still exempting,
`contract_dir` resolved from
the concern file, a relocated root moving the gate's scope, a
root-equivalent value exiting 2, a
slug-less baseline surviving `set -u`, both base and head roots policed,
and a grandfathered slice
migrating to a relocated root.

Four review findings were raised across two rounds and all four were
reproduced before being
fixed — two bypasses (self-grandfathering; a relocation leaving its own
root uninspected) and two
fail-open / crash defects (`contract_dir` ignoring the concern file; the
gate aborting under
`set -u` once the baseline empties, which is the exact end state #1419
drives toward). See the
resolved threads; each carries its reproduction and the case that pins
it.

**Measured against the open PRs that actually carry `docs/topics/`
paths, rather than asserted.**
Because the gate reads the baseline from the base revision, these were
run against a base that
already carries it — the post-merge condition:

| PR | Slice | Result |
|---|---|---|
| #1252 | `plugin-audit-port` | passes on baseline exemption |
| #1096 | `fresh-eyes-checkpoint-audit` | passes on baseline exemption |
| #1318 | `context-engineering-claude-5` + the two slices #1400 landed |
fails until rebased onto a `main` carrying the updated baseline |

#1318's failure is an artefact of it predating #1400's merge, not a
defect: its branch adds those
two files relative to its own fork point. Once rebased, they are on
`main` and in the baseline, so
they leave its diff entirely.

The self-grandfathering bypass was verified closed by re-running the
reviewer's own reproduction
against the fix.

Also verified: `shellcheck` clean, `actionlint` clean, `shfmt` clean,
the org comment-hygiene policy
reports zero violations in the new files, and both scripts carry the
executable bit.

## Wiring

`contract-slice-prune-gate` is added to `ci-status`'s `needs:` list.
That aggregate derives its
lane list from the needs graph, and `ci-status` is already a required
status check on the ruleset,
so the new gate becomes required with **no ruleset edit**. Job naming
matches the existing
precedent (`silent-skip-gate`, `orphaned-fixture-gate`,
`changelog-parity-gate`). The self-test runs
unconditionally so a broken gate cannot mask a regression; the PR-diff
step is event-gated.

## Related

- #1419 — graduate and prune the grandfathered slices. Each prune PR
drops its own baseline line,
and this gate's stale guard fails once an entry outlives its slice. Not
closed by this PR. Its
  inventory needs updating to 19 once this lands.
- #1400 — merged while this PR was open, landing two more slices; both
added to the baseline as
debt rather than treated as incoming work. Flagged there before it
merged.
- #1252, #1096 — open PRs carrying grandfathered slice paths; verified
passing on their baseline
  exemptions. #1318 needs a rebase past #1400 (see Verification).
- #1286 — closed by hand for carrying contract-tier content, which is
the manual enforcement this
  gate replaces.
- `docs/conventions/topic-docs/README.md` — the convention specifying
this check as step 5.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compose both sides rather than take either:

- `apply reset` -> `apply defaults` (main's clean-break rename) now also reaches
  the frontmatter argument-hint, the evals expectation, and the `apply` section
  body that the textual merge left on the old token.
- context-guard's Purpose keeps main's rewritten narrow-write rationale and
  widens "exactly ONE writable artifact" to the operator-home directory, which
  is what this branch actually changed.
- rate-limit-guard's Purpose keeps main's three-unwritable-surfaces enumeration
  and the headless reconfiguration recipe, and states separately why a shim —
  not configuration — is what obliges the new `apply`.
- Both plugins land on 0.3.0: main took 0.2.0 (context-guard) and 0.2.1
  (rate-limit-guard) while this branch was open, so the shipped entries move
  under a new heading with main's releases preserved verbatim beneath.

Also addresses the four live review findings:

- The shim resolves the cache from the effective config root
  ${CLAUDE_CONFIG_DIR:-$HOME/.claude}, not $HOME. CLAUDE_CONFIG_DIR is
  documented as relocating settings, session history AND plugins
  (https://code.claude.com/docs/en/env-vars, fetched 2026-07-25), so a
  $HOME-anchored shim resolved nothing forever for an operator running the
  documented multi-account alias. Both shims, shim-revision 2.
- `check` unwraps recognized guard shims before composing, so a statusline
  already wired through the sibling (or through itself) is not wrapped twice.
- The combined sibling wiring is gated on the sibling shim existing, because
  the sibling shim is written by that plugin's own `apply` — naming a missing
  path is the `bash <missing>` 127 failure this change exists to remove.
- Uninstall is ordered: unwrap `statusLine` first, then delete the directory.
  The shim's own fallback cannot cover the reverse order; it lives in the file
  being deleted.

Harnesses grew three cases each (relocated config dir, CLAUDE_CONFIG_DIR
beating a HOME cache, empty value falling back): 31/31 both plugins. Existing
suites unchanged and green: context-zone 44, context-guard tee 44,
rate-limit-guard tee 34. Two evals added per setup skill.
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@github-actions

Copy link
Copy Markdown

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-running the job, or pushing a new commit, will retry the review.

@github-actions

Copy link
Copy Markdown

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Re-running the job, or pushing a new commit, will retry the review.

@kyle-sexton
kyle-sexton merged commit b47f720 into main Jul 26, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the feat/statusline-shim-durable-wiring branch July 26, 2026 01:06

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07a0807853

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +82 to +84
for cand in "$cache"/*/"$PLUGIN_NAME"/*/scripts/statusline-tee.sh; do
# An unmatched glob expands to the literal pattern; -f rejects it.
[[ -f "$cand" ]] || continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop resolving tees from uninstalled cache entries

When an operator uninstalls either plugin while leaving the advertised durable wiring in place, Claude Code retains the now-orphaned cache version until garbage collection (the repository records seven days after update/uninstall in docs/topics/plugin-fleet-sync-skill/PLAN.md:29). This glob treats any remaining file as installed, so the shim continues executing the uninstalled tee and writing context/rate-limit snapshots instead of taking its documented passthrough fallback. Verify that the candidate belongs to an active install, or require unwiring before uninstall; the symmetric rate-limit shim has the same issue.

Useful? React with 👍 / 👎.

Comment on lines +106 to +110
**Unwrap before you compose.** `<current statusline command>` below means the operator's OWN
renderer, never the raw effective `command` string. Before substituting, strip every leading
guard-shim invocation from that string — `bash <path>/context-guard/bin/statusline-shim.sh` and
`bash <path>/rate-limit-guard/bin/statusline-shim.sh`, in whatever order they appear — plus any
legacy `bash <plugin-cache>/…/statusline-tee.sh` prefix, and treat what remains as the renderer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Unwrap the generated sh -c adapter on reruns

When check is rerun on the shell-syntax form this skill itself generated, stripping only the guard prefix leaves sh -c '<original>', which these instructions then treat as the renderer and wrap again because it contains quoting. Applying the new edit therefore changes bash <shim> sh -c '<original>' into bash <shim> sh -c 'sh -c ...', and every subsequent check/application adds another shell, contradicting the stated idempotence and accumulating process and quoting overhead. Recognize and unwrap the generated sh -c adapter before recomposition; the rate-limit setup has the symmetric instructions.

Useful? React with 👍 / 👎.

kyle-sexton added a commit that referenced this pull request Jul 31, 2026
…ing sh -c wrap (#1844)

Fixes #1787

## Summary

Two findings that landed on #1252 after it merged, so the
thread-resolution gate never saw them.
Both were re-verified present on `origin/main` before this branch
started, and both are fixed here
with regression coverage.

1. **P1 — an uninstalled plugin's tee kept executing through the shim.**
`resolve_tee()` picked the
newest `scripts/statusline-tee.sh` by mtime, skipping only `temp_*`
marketplace clones. It never
asked whether the plugin was still installed, so an uninstalled plugin
kept teeing and kept
   writing snapshots, with no signal to the operator.
2. **P2 — `setup` added one `sh -c` layer per run.** "Unwrap before you
compose" stripped guard-shim
prefixes but not the `sh -c '<escaped …>'` adapter the skill's own
shell-syntax guard prints.

## Fix

### The orphan marker, not `installed_plugins.json`

`claude plugin uninstall` does not delete the version directory. The
plugins reference, under
[Plugin caching and file
resolution](https://code.claude.com/docs/en/plugins-reference) (fetched
2026-07-30), states: *"When you update or uninstall a plugin, the
previous version directory is
marked as orphaned and removed automatically 14 days later. The grace
period lets concurrent Claude
Code sessions that already loaded the old version keep running without
errors."* Uninstall is named
explicitly, so `scripts/statusline-tee.sh` stays on disk — and stays
executable by an mtime-only
shim — for that whole window.

`resolve_tee()` now skips a candidate whose version directory carries
the orphan marker.

The issue asked for a **supported** signal, not the internal file the
shim header deliberately
rejected. That rejection stands, on both of its original grounds:
`installed_plugins.json` is
undocumented and reading it would put a `jq` spawn on every statusline
refresh. The orphan marker
beats it on both — the behavior it reports is documented, and the test
is a shell builtin, so the
file's pure-builtins invariant is preserved (`[[ -e … ]]`, no
subprocess).

**What is documented vs. what is measured**, recorded at the site rather
than assumed:

- The *marking* is documented (quoted above).
- The marker's *on-disk spelling* is not. Measured on Claude Code
2.1.220 against a relocated
`CLAUDE_CONFIG_DIR`: an uninstall writes `<version-dir>/.orphaned_at`
holding an epoch-ms stamp
and leaves `scripts/statusline-tee.sh` in place. Reproducible on any
live cache — every superseded
version directory carries the marker and the currently installed one
does not.
- A directory can also be marker-less while merely *staged* (a newer
version fetched for a pending
update), so absence of the marker is not a claim of installation. mtime
still picks the winner
  among unmarked candidates, exactly as before.
- **Fallback if upstream renames or drops the marker:** the test simply
finds nothing and resolution
degrades to today's mtime-only behavior — a stale tee, never a broken
statusline.

### Unwrapping is now a fixpoint, not one pass

"Unwrap before you compose" became two rules applied until a pass strips
nothing:

1. guard-shim prefixes (unchanged);
2. a generated `sh -c '<single-quoted string>'` adapter with nothing
after the closing quote **and
whose carried string itself contains shell syntax** — unescaped back by
dropping `sh -c` and the
   outer quotes and replacing `'\''` with `'`.

Iterating matters: an operator may already carry several layers from
earlier reruns, and a single
peel over three layers leaves three. The shell-syntax-guard step now
also states that it applies
only to the *unwrapped* renderer.

**The second condition on rule 2 is provenance, and it is
load-bearing.** The first push of this
branch peeled any bare `sh -c '<string>'`, and disclosed that as a
deliberate, behaviorally
equivalent narrowing. Review showed that claim was wrong, so it is now
fixed rather than documented:
an operator whose genuine renderer is `sh -c 'ulimit -n'` would have had
the layer dropped, and
since `ulimit -n` carries no shell syntax the guard would not restore it
— leaving the shim
`exec`-ing a shell builtin with no shell, so the statusline exits 127
instead of rendering.

Requiring shell syntax in the carried string is a provenance test, not a
heuristic: this skill emits
the adapter **only** for a renderer that carries shell syntax, so an `sh
-c` over a string carrying
none cannot have come from it and is the operator's own. Both target
cases still hold — every
generated layer wraps something with shell syntax, so nested *generated*
layers collapse until what
remains is the operator's own, while the genuine-builtin case is
preserved verbatim. A trailing word
(`sh -c '…' extra`) marks a real command and is left alone.

Stated precisely, because the distinction matters: the fixpoint does not
always terminate at a bare
renderer. Over a hand-written `sh -c 'sh -c '\''my-statusline
--flag'\'''` the outer layer peels (its
carried string contains quoting) and the inner one does not
(`my-statusline --flag` carries no shell
syntax), leaving exactly one layer. That is the correct outcome — the
surviving layer is the
operator's own — and it is still idempotent and non-compounding, which
is what the issue asked for.

`context-guard` 0.4.3 → 0.4.4 with a matching `## [0.4.4]` CHANGELOG
entry. (The branch originally
bumped 0.4.2 → 0.4.3; the shared `hook-utils.sh` sync claimed 0.4.3 on
main while this branch was
open, so it was renumbered after fast-forwarding rather than co-owning a
released version.)

## Verification

Regression coverage per the acceptance criteria — and the shim cases are
proven **non-vacuous**,
not merely green:

| Evidence | Result |
| --- | --- |
| `bash plugins/context-guard/scripts/statusline-shim.test.sh`
(post-fix) | **36 passed, 0 failed** |
| Same suite against the **pre-fix** shim | **3 failed** — `want [] got
[TEE:uninstalled]`, `[TEE:orphaned] does not contain [TEE:installed]`,
`[TEE:orphaned] unexpectedly contains [TEE:orphaned]` |
| Live-cache measurement of the marker | 5 superseded `context-guard`
version dirs all carry `.orphaned_at`; the installed one does not |
| Doc claim re-fetched this session | uninstall named explicitly in the
quoted sentence |

Two new shim cases: an uninstalled plugin's orphaned tee is not executed
*and the statusline still
renders with the wrapped exit code preserved*; and an orphaned directory
loses to an installed
sibling **even when it is newer by mtime**, so only the marker can
decide the second case.

The P2 fix is a prose-instruction change, so its regressions live where
instruction behavior is
graded — two evals, one per direction:

- **id 8, `rerun-does-not-compound-the-sh-c-wrap`** — pins the exact
rerun in the issue and asserts
  one `sh -c` layer, never a nested one. Traced by hand:
`…shim.sh sh -c 'THEME=dark my-statusline --flag'` → rule 1 strips the
shim → the carried string
has an inline env assignment, so rule 2 recovers `THEME=dark
my-statusline --flag` → second pass
  strips nothing → re-wrap reproduces the input byte-for-byte.
- **id 9, `genuine-sh-c-renderer-is-not-peeled`** — the opposite
direction, added in response to
review. `sh -c 'ulimit -n'` carries no shell syntax, so rule 2 does not
fire and the adapter
  survives into the printed wiring.

## Review findings addressed on this branch

| Finding | Disposition |
| --- | --- |
| Codex P2 — "Preserve genuine `sh -c` renderers" (`sh -c 'ulimit -n'`
peeled to a broken `exec`) | **Fixed**, not just documented. Rule 2 now
requires shell syntax in the carried string as a provenance test; eval 9
pins it. |
| Claude review — garbled fallback sentence in the 0.4.4 changelog entry
| **Fixed.** Rewritten to "should upstream rename or drop the marker,
resolution degrades to exactly what it does today — a stale tee, never a
broken statusline." |

Gates, all run from the worktree root against `origin/main`:

| Gate | Result |
| --- | --- |
| `shellcheck` (shim + test) | clean |
| `scripts/check-shell-portability.sh origin/main` | PASS — 2 files, no
unexcused GNU-only constructs |
| `scripts/check-changed-skills.sh origin/main` | PASS — 0 errors, 2
warnings (both pre-existing: soft line target, no Gotchas surface) |
| `scripts/check-changelog-parity.sh --check` | PASS |
| `scripts/check-changelog-parity.sh --check-bump origin/main` | PASS |
| `scripts/check-changelog-parity.sh --check-order` | PASS — 71
changelogs, newest-first, no duplicate versions |
| `markdownlint-cli2 "plugins/context-guard/**/*.md"` | PASS — 0 issues,
4 files |
| `scripts/check-cross-plugin-source-drift.sh --check` | PASS |
| `scripts/check-silent-skips.sh` | PASS |
| `scripts/validate-plugins.sh` | PASS — manifests + catalog |

Gate scoping notes, stated rather than assumed: `check-silent-skips.sh`
reads only
`plugins/*/hooks/*.sh`, so the new `[[ -e … ]] && continue` in
`scripts/statusline-shim.sh` is
outside its corpus by construction — it is not an unreported silent skip
but a resolution filter in
a non-hook script, documented inline at the site.
`check-cross-plugin-source-drift.sh` passes
because the two plugins' shims are already not byte-identical and are
correctly unregistered.

## Related

- Refs #1252 — the merged PR both findings were stranded on. Its two
threads are resolvable only
after this merges; recorded here as the remaining acceptance item rather
than actioned on the
  branch.
- Refs #1777 — the stranded-findings sweep that surfaced them.
- **Follow-up, not fixed here:**
`plugins/rate-limit-guard/scripts/statusline-shim.sh` carries the
identical `resolve_tee()` defect — same glob, same `temp_*`-only filter,
no orphan check — so an
uninstalled `rate-limit-guard` keeps teeing for the same ~14-day window.
It is out of scope for
#1787, which is scoped to `context-guard`, and porting it would pull a
second plugin's version
bump, changelog, and test suite into this diff. The two shims are
deliberately *not* a registered
byte-identical cluster (they differ in plugin name and header prose), so
no drift gate will
  surface it — filing it is the only thing that keeps it visible.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 9, 2026
…usline renderer (#1999)

## Summary

`/context-guard:setup check` printed an operator's own `sh -c
'<command>'` renderer back to them
wrapped in a second one — `sh -c 'sh -c '\''ulimit -n'\'''` — adding one
more shell on every
statusline refresh.

PR #1844 fixed the mechanism the finding named: unwrap rule 2
(`skills/setup/SKILL.md:143-153`)
correctly declines to peel an `sh -c` the operator wrote themselves,
because the string it carries
holds no shell syntax. But the finding's complaint kept reproducing by a
second path. The
*preserved* renderer then reached the shell-syntax guard, which listed
bare **quoting** among the
triggers needing an adapter — so it matched on its own quote characters
and was wrapped anyway. The
escape hatch that was supposed to stop that had a true condition and a
**false** rationale: it
claimed such a command meant "rule 2 above having been skipped", when
rule 2 had in fact run and
declined.

A finding is discharged when its complaint stops reproducing, not when
the line it pointed at
changes. Decisive proof it had not: a faithful implementation of the
prose failed this skill's own
**eval 9** (`skills/setup/evals/evals.json:107-118`), whose fourth
expectation is "prints exactly
one shim invocation and exactly one `sh -c` layer".

## What changed

**Quoting is no longer a trigger, and the trigger test is scoped to
top-level syntax.** The
`statusLine` `command` field "runs in a shell"
([docs](https://code.claude.com/docs/en/statusline), fetched
2026-08-07), so that shell splits the
line into words and consumes the quotes before `statusline-shim.sh` sees
ARGV, and the shim `exec`s
those words unchanged. A quoted argument therefore already survives the
plain wrapped form intact —
including an operator's `sh -c '<string>'`, where `sh` is the executable
and `-c` and the carried
string are two ordinary ARGV words.

The guard now fires only on syntax no ARGV word can express — an inline
env assignment, a pipe,
`&&`, `||`, `;`, a trailing `&`, a redirection — **and only where it
stands unquoted at the top
level**. That second clause matters on its own: without it, "carries
shell syntax" reads as a
substring test, and an implementer meeting `sh -c 'a | b'` at the guard
sees a pipe and wraps it,
reproducing the double wrap by a third path.

**Rule 2's provenance test is now three explicit branches, because
scoping the guard silently
rescoped it too.** Rule 2 cites the guard to define "carries shell
syntax", so narrowing the guard
narrowed rule 2 with it — and a wrap whose syntax sits inside inner
quotes then looked
operator-written. Two of the three branches are therefore keyed to the
**shape** of the carried
string, not the syntax in it, and are explicitly exempt from the
top-level scoping:

- **A — the carried string is itself an `sh -c '<string>'`.** Always a
generated layer: an
operator's renderer is at most one `sh -c` deep. Without this the peel
stops one layer early and
  hands back the two-layer wrap it exists to collapse.
- **B — the carried string begins with a guard-shim prefix.** This skill
never puts a shim inside
an adapter. Sealed there it is invisible to rule 1, which strips only
*leading* prefixes, so the
composed wiring named the sibling shim a **second** time and ran its tee
twice per refresh — the
`context → rate → rate → renderer` duplication this skill exists to
prevent, reachable for anyone
  who wired `rate-limit-guard` first and ran the pre-0.4.8 guard.
- **C — the carried string is a command the guard would wrap.** The only
shape this skill's own
  adapter ever carries.

Absent all three it is the operator's and is preserved. One shape stays
ambiguous **by design** — a
single `sh -c` over a merely-quoted command, which the buggy guard also
emitted and which carries no
evidence either way. It is preserved: one spurious shell per refresh is
cheaper than a statusline
broken by peeling on a guess. The prose says so rather than implying a
re-run cleans it up.

**The hatch's condition and rationale now agree**, because the hatch is
gone and the guard's own
condition does the work. The idempotency claim is scoped to what is
actually true: for an input that
is itself `sh -c '<string>'`, the two tests leave exactly one layer.
That does **not** generalize to
a layer count — a plain renderer takes none, and `sh -c 'ulimit -n' &&
echo ok` correctly takes
**two**, since `&&` cannot be an ARGV word and peeling the inner `sh -c`
would strand the builtin.
What is invariant is that peel and wrap are inverses, which is what
makes a re-run byte-identical at
whatever count the renderer needs.

**Three evals now pin this.** Eval 10
(`multiple-generated-layers-collapse-in-one-run`) was added
earlier on this branch and deleted by `81c03090e7` with no rationale in
the commit message or the
CHANGELOG; it covers the nested-layer case exactly, and the scoping
change broke it. It is restored
with a fifth expectation pinning branch A. Eval 11
(`adapter-hiding-a-sibling-shim-is-peeled`) is
new and pins branch B.

**`check`'s shim-drift report stopped claiming a differing installed
copy is harmless.** It said an
older or hand-edited copy "still resolves the newest tee", which stopped
being true when
`# shim-revision: 3` added the orphan skip: a copy predating it picks by
mtime alone, so it also
resolves a tee left behind by an uninstalled plugin and keeps teeing for
the whole ~14-day grace
window. It now states which of the two behaviors the installed copy has.

`0.4.7` → `0.4.8` with the matching CHANGELOG entry.

## Verification

- Traced end to end for eval 9's input `sh -c 'ulimit -n'`: rule 1 finds
no shim prefix; rule 2
declines on both branches (carried string is not an `sh -c` shape, and
the guard would leave it
alone); the guard finds no unquoted top-level trigger; the plain wrapped
form is printed — one
  shim invocation, one `sh -c` layer. All four expectations met.
- Eval 10's input `bash <shim> sh -c 'sh -c '\''THEME=dark
my-statusline'\'''`: rule 1 strips the
shim, rule 2's nested branch peels the outer layer, rule 2's guard
branch peels the inner one, a
third pass strips nothing, the guard fires once on `THEME=dark
my-statusline`. One layer.
- Eval 11's input (a `rate-limit-guard` shim sealed inside a preserved
adapter): branch B peels the
adapter, exposing the shim to rule 1, which strips it. The combined
wiring names each shim exactly
  once.
- Eval 8 is unaffected: rule 2 peels, the env assignment lands unquoted
at top level, the guard
fires once, output is byte-identical to the wiring already in
settings.json.
- No eval was loosened; one was restored and strengthened, one added.
Evals 1-7 carry no renderer
  whose behavior the trigger-list change moves.
- **A fresh-context verifier CHALLENGED this fix twice, and was right
both times.** Round one caught
that the guard scoping had silently regressed the nested-layer case;
round two caught a false
idempotency invariant and the sealed-sibling-shim duplication. Branches
A and B, both restored and
  new evals, and the scoped invariant all come from those challenges.
- Gates green from the worktree root: changelog parity (`--check`,
`--check-order`, `--check-bump`
against `origin/main`), `check-changed-skills.sh` (context-guard/setup
PASS, 0 errors),
orphaned-fixture gate, shell-portability, skill-portability,
markdownlint, and the context-guard
  suite (shim 36, tee 47, zone 73 — 0 failures).
- SKILL.md is 391/500 lines.

## Related

- Reopens review finding `PRRT_kwDOTCGFQM6Tzj7l` on #1252, which a prior
pass closed as
  already-fixed and a fresh-context verifier correctly challenged.
- Builds on `3cbfaa89ae` (#1844), which fixed unwrap rule 2 and the
shim's orphan skip. That
  behavior is untouched here.
- Out of scope, filed for follow-up:
`plugins/rate-limit-guard/skills/setup/SKILL.md` still lists
bare quoting as a trigger and has no adapter-peel rule at all, so the
sibling skill reproduces
  this same finding on its own surface. Not touched here.

No linked issue

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

context-guard / rate-limit-guard: durable statusline wiring (version-independent shim)

1 participant