feat(context-guard): per-session context snapshots, zone resolver + setup skill

[kyle-sexton]

Summary

Lane A of the plugin-audit-port topic (contract: docs/topics/plugin-audit-port/PLAN.md, locked at 40a64b5c; parallel-authorship scope change recorded at 77e0a5f6). Adds the context-guard plugin — per-session context-window observability on the rate-limit-guard tee pattern:

• Statusline tee (scripts/statusline-tee.sh): transparent byte-for-byte wrapper; writes captured_at + session_id + the verbatim context_window object to ~/.claude/context-guard/context/<session_id>.json per refresh. Atomic temp+rename with Windows retry, jq-missing visible degrade, standalone mode, session-id sanitization to [A-Za-z0-9_-] (path containment), 14-day sibling pruning that never touches .tmp.* files.
• Zone resolver (scripts/context-zone.sh): prints exactly one of smart/acceptable/dumb/unknown; shipped judgment bands smart ≤ 50 < acceptable ≤ 75 < dumb with ~/.claude/context-guard/zones.json override; fail-open to unknown on absent/stale/unparsable snapshots, null/out-of-range fields, and missing jq.
• Reader contract (reference/reader-contract.md): inline-floor ownership + byte-identity rule, 10-minute staleness, ${CLAUDE_SESSION_ID} session-id discovery + conservative fallback, zone-is-NOT-a-compaction-indicator rule.
• Setup skill: check (read-only; stale-wiring detection compares the wired path against the resolved ${CLAUDE_PLUGIN_ROOT}, catching exists-but-outdated cache paths) + apply scoped to seeding/refreshing zones.json only. Evals included.

Sanity-check evidence (per PLAN phases A1–A5)

• A1: statusline-tee.test.sh — 40 assertions, exit 0 (sandboxed HOME); manual printf smoke echoes exact bytes, exit 0, snapshot passes jq -e '.captured_at and .session_id and .context_window'. Shellcheck clean.
• A2: context-zone.test.sh — 40 assertions, exit 0. No documented auto-compaction threshold (re-verified 2026-07-24, costs + statusline pages); empirical transcript archaeology found no auto-compact event on the producing machine (largest session 308k tokens uncompacted, 1M-class window) — margin declared in topic notes + contract.
• A3: staleness value, snapshot path, and bands grep-match byte-identically across both scripts and the contract doc.
• A4: skill-quality:check PASS (0 errors); evals validate against the bundled schema.
• A5: claude plugin validate plugins/context-guard exit 0; claude plugin validate --strict . exit 0. Clean-repo --plugin-dir headless smoke: /context-guard:setup check ran end-to-end in a non-source repo — jq PASS, wrapper-missing correctly detected with the wrapped operator edit printed (user's existing command preserved), snapshot-absent correctly attributed, zones-absent reported as valid zero-config state.
• Remaining HITL (batched, per handoff): operator applies the printed statusline wiring, then a live-session snapshot appears — surfaced to the owner before merge.

Security review

Plugin-acceptance record appended to docs/MIGRATION-PLAYBOOK.md: no hooks, no MCP, no userConfig, no egress; writes only ~/.claude/context-guard/ (operator-home carve-out, documented cross-plugin seam).

Related

• Lane-B sibling PR (plugin-quality) follows after this merges — seam consumer of the reader contract (plugin-quality: audit skill hub + auditor agent #1237, not closed here).
• plugin-quality: operator cutover — sink override, full-item diff, retire local skill #1233 — operator cutover (post-merge, HITL; deliberately left open).
• Topic contract: docs/topics/plugin-audit-port/PLAN.md (locked 40a64b5, scope change 77e0a5f).

Closes #1228
Closes #1234
Closes #1229
Closes #1236
Closes #1230

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 5m 27s —— View job

---

PR Review: feat(context-guard)

Tasks

• Gather context and understand the request
• Read plugin manifest and marketplace entry
• Review shell scripts (statusline-tee.sh, context-zone.sh)
• Review test files
• Review setup skill and evals
• Review reference contract and docs
• Analyze security, correctness, and CLAUDE.md compliance
• Post final review

---

Review

The overall design is solid: the fail-open contract, atomic rename, per-session file isolation, path containment, and test coverage are all well-executed. Three findings below, ordered by severity.

---

Finding 1 — Moderate: Reader contract contradicts context-zone.sh implementation

File: plugins/context-guard/reference/reader-contract.md:64

The reader contract states:

Treat all values as untrusted data: parse only with a JSON parser; never string-interpolate snapshot values into a shell command, another interpreter, or a prompt.

But context-zone.sh:76-77 does exactly this:

snap_epoch=$(date -u -d "$ts" +%s 2>/dev/null ||
  date -u -j -f '%Y-%m-%dT%H:%M:%SZ' "$ts" +%s 2>/dev/null) || unknown

$ts is extracted from the snapshot's captured_at field via jq -r and then passed to date -d. This is not a security bug — "$ts" is double-quoted (no shell word-splitting or injection), date treats its arg as a date string not a command, and snap_epoch is validated as ^[0-9]+$ before use, so a malformed date string merely produces unknown. But it visibly contradicts the contract's stated principle.

Two resolution paths:

• Narrow the contract to "never pass snapshot values unquoted into a shell command" or "never pass snapshot values to interpreters that execute them (eval, sh -c, embedded jq programs as strings)." This is the more accurate characterization — date -d is a date-string parser, not a code executor.
• Eliminate the interpolation by computing staleness purely from captured_at via jq arithmetic (comparing now to (captured_at | fromdateiso8601) inside the existing jq call), which would remove the $ts variable entirely and stay inside the JSON parser.

Fix the contract wording or staleness check →

---

Finding 2 — Minor: apply has no fallback when jq is absent and zones.json already exists

File: plugins/context-guard/skills/setup/SKILL.md:110

The apply step 2 says:

File present — converge, don't clobber: … preserve every unrecognized key byte-for-byte … Use jq to merge so the result stays valid JSON.

But there's no instruction for what to do when jq is absent and zones.json already exists. In that case the merge step would fail, and the skill has no explicit remediation path. Step 1 of check gates on jq, but apply can be invoked directly without having run check. Step 1 of apply (file absent → write template) doesn't require jq, but step 2 does. The skill should explicitly say: if jq is absent and the file is present, FAIL with the same jq install remediation rather than attempting the merge.

Fix apply to gate on jq for the merge path →

---

Finding 3 — Minor: 1 MiB stdin cap silently truncates the wrapped command's input on oversized payloads

File: plugins/context-guard/scripts/statusline-tee.sh:68-72

IFS= read -r -N 1048576 -t 5 INPUT || true

If a statusline payload somehow exceeds 1 MiB, INPUT contains only the first 1 MiB of bytes. The tee silently writes nothing (jq fails on truncated JSON). But the wrapped command also receives only $INPUT via printf '%s' "$INPUT" — truncated input — which can make the wrapped command fail or produce wrong output. That wrong exit code would propagate as the statusline's exit code.

The comment correctly notes "statusline payloads are a few KB," and in practice this is purely theoretical. The issue is that it's undocumented as a behavioral boundary: neither the README, the script header, nor the reader contract mentions it, so a future operator who wires an unusually large statusline command (one that emits megabytes of JSON) will see mysterious wrapped-command failures with no clear diagnostic path.

Suggestion: add one sentence to the script header (near the bound comment) saying: "If the payload exceeds this bound, the wrapped command receives only the buffered bytes and should fail with its own exit code; the tee silently skips. In practice, statusline payloads are a few KB."

Add the truncation behavior to the documented contract →

---

What's solid

• Fail-open contract is correctly implemented — every trust-precondition failure in context-zone.sh falls through to unknown; no tee failure ever surfaces in the statusline pipeline.
• Path containment — session_id is validated to [A-Za-z0-9_-]+$ before filesystem use on both the write and read sides; test Case 7 exercises five hostile patterns.
• Atomic writes — temp+rename with a three-attempt retry for the Windows locked-target case; Case 8 and 9 in the tests drive this via a mv shim.
• Test coverage — 40 assertions each test file; the mv shim design for the rename-retry path is particularly well-crafted.
• CLAUDE.md compliance — repo-agnostic (${CLAUDE_PLUGIN_ROOT}, $HOME), no userConfig, versioned (0.1.0), security-reviewed in MIGRATION-PLAYBOOK.md.
• Zone-is-not-a-compaction-indicator is clearly stated in the reader contract with the empirical evidence behind the bands being declared judgment defaults, not doc-derived constants.
• disable-model-invocation: true on the setup skill is correct — prevents ambient invocation.
• chmod 700 on the context dir — best-effort owner-only guard against symlink pre-planting and neighbor reads; the || true correctly makes it non-fatal on Windows ACL volumes.

[claude]

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

---

Security Review — PR #1241 feat(context-guard)

Scope: plugins/context-guard/scripts/statusline-tee.sh, context-zone.sh, test scripts, skill, plugin manifest, and supporting docs. Changed files only.

---

Finding 1 — chmod 700 snapshot directory silently no-ops on non-POSIX filesystems, enabling local-user forged snapshots on Windows

Severity: IMPORTANT
Confidence: PLAUSIBLE

statusline-tee.sh L92:

chmod 700 "$dir" 2>/dev/null || true

The comment immediately above this line explicitly acknowledges it is a no-op on Windows ACL filesystems under Git Bash. On such systems, any local user can both read snapshot files (disclosing session_id values and context-window token counts, which are session-correlated metadata) and write forged snapshot files into ~/.claude/context-guard/context/. Because the snapshot directory is the sole security perimeter for snapshot integrity, this creates a full-path attack surface on Windows: a local adversary can write a snapshot with any session_id and any used_percentage, causing a victim's context-zone.sh invocation to return a fabricated zone word (e.g. dumb when the window is nearly empty, or smart when it is nearly full). Consumer code that gate-dispatches expensive operations on the zone result would then take the wrong branch.

This is implicitly acknowledged as a platform limitation, but neither the README nor the security review record in MIGRATION-PLAYBOOK.md surface the integrity consequence (they only note the availability degradation from jq being absent). The reader contract (reference/reader-contract.md L64) says to "treat all values as untrusted data" but provides no mechanism for consumers to verify that untrusted data was written by the legitimate tee.

On Linux/macOS with POSIX ACLs, the chmod 700 is effective and this attack is not possible.

---

Finding 2 — captured_at from snapshot passed to date -d without ISO-8601 format validation; a forged snapshot can defeat freshness check

Severity: IMPORTANT
Confidence: PLAUSIBLE

context-zone.sh L76–77:

snap_epoch=$(date -u -d "$ts" +%s 2>/dev/null ||
  date -u -j -f '%Y-%m-%dT%H:%M:%SZ' "$ts" +%s 2>/dev/null) || unknown

The jq validation pass (L61–69) checks that captured_at is a JSON string type but does not validate its format. $ts is then passed verbatim to GNU date -d, which accepts a wide range of relative and symbolic date expressions in addition to ISO-8601 ("now", "1 second ago", "yesterday", etc.).

On a system where Finding 1 applies (snapshot directory writable by other local users), an adversary can write a snapshot containing "captured_at": "now" (or any GNU date expression that resolves within the 600-second staleness window). GNU date -d "now" returns the current epoch, making age ≈ 0, which passes ((age >= -60 && age <= 600)). This allows a forged snapshot with a stale or fabricated used_percentage to pass the freshness check and produce a non-unknown zone.

On POSIX systems the chmod 700 in Finding 1 prevents unprivileged write access to the directory, so this is not independently exploitable — but it compounds directly with Finding 1 on Windows.

Mitigation path: Add a regex guard on $ts before the date call:

[[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown

This is a one-liner and eliminates the GNU-date relative-expression surface entirely, independent of platform.

---

Finding 3 — Temp file uses $RANDOM (0–32767); predictable in conjunction with observable PID if chmod 700 fails

Severity: SUGGESTION
Confidence: LOW

statusline-tee.sh L109:

local tmp="$dir/.$sid.json.tmp.$$.$RANDOM"

$RANDOM generates values in the range 0–32767. When combined with $$ (PID, typically visible via ps or /proc on Linux), the full temp filename is predictable to any process that can observe the running PID. If the snapshot directory is accessible to other users (Finding 1, Windows platform), a local adversary can enumerate 32768 possible temp filenames, create symlinks for each targeting an arbitrary user-writable file (e.g., ~/.claude/settings.json), and win the race when the tee writes printf '%s\n' "$payload" >"$tmp". The write is in a subshell with umask 077 (L113) but that only sets permissions for newly created regular files — it does not prevent following a symlink to an existing file.

This attack requires both Finding 1 to be applicable (non-POSIX filesystem) and a tight race window. Not exploitable on POSIX systems where chmod 700 is enforced. The sleep 0.1 retry loop in the rename path does not help here because the race is on the initial write to $tmp, not the rename.

---

What is NOT a finding

• Session-id regex (^[A-Za-z0-9_-]+$): correctly rejects path separators, dots, semicolons, spaces, and newlines before any filesystem operation.
• jq variable passing (--arg ts "$ts", -v u="$used"): all jq and awk variable bindings use safe quoting; no interpolation of snapshot values into shell syntax.
• "$@" wrapped command: originates from operator settings.json, not from stdin; no injection surface.
• find -exec rm -f {} + pruning: $dir is constructed from $HOME + a fixed suffix, not user input; the command is safe.
• Atomic rename: mv -f "$tmp" "$target" is atomic on both Linux ext4 and macOS APFS; the Windows retry loop is correctly scoped.
• disable-model-invocation: true on setup skill: correct choice for a setup skill with apply write capability.
• No hooks, no MCP, no eval, no outbound network: confirmed clean.

---

Summary

The code is well-structured for a local-only plugin, with systematic path containment and fail-open behavior. The two IMPORTANT findings are both contingent on the chmod 700 no-op on Windows and therefore share a root cause. Finding 2 has an independent one-line fix (format-guard $ts before date -d) that eliminates the derived attack surface regardless of platform. Finding 3 is low-confidence and only matters if both the chmod failure AND a tight symlink race are achievable simultaneously.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5fe998745

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — per-session context snapshots, zone resolver + setup skill

Tasks

• Gather context and understand the request
• Read plugin manifest and marketplace entry
• Review shell scripts (statusline-tee.sh, context-zone.sh)
• Review test files
• Review setup skill and evals
• Review reference contract and docs
• Analyze security, correctness, and CLAUDE.md compliance
• Post final review

---

Overall assessment

Solid plugin. The architecture (tee pattern, fail-open resolver, reader-contract SSOT, setup-only skill) is clean and well-reasoned. The 80-assertion test harness is thorough, the security posture is minimal-egress, and the CLAUDE.md compliance is strong. Two issues should be addressed before merge — one confirmed by Codex, one that Codex also flagged — plus a few minor observations.

---

P2 — Non-ISO captured_at bypasses staleness check

File: context-zone.sh L76–77

The jq parse step validates that captured_at is a string, but does not enforce ISO-8601 format. On GNU/Linux, date -d accepts natural-language values like "now", "yesterday", "5 minutes ago", so a snapshot with "captured_at": "now" would return an epoch close to now_epoch, produce age ≈ 0, pass the staleness window check, and resolve a real zone instead of unknown. The fail-open contract requires that untrustworthy timestamps resolve unknown.

The fix is one line after ts=${parsed%% *} (L71):

[[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown

This also has no test case in context-zone.test.sh — a case with "captured_at": "now" should be added expecting unknown.

Note: in practice, snapshots in chmod 700 dirs require the same-user to manipulate them, so the severity is local-only. But the fail-open contract is unconditional; trustworthiness of the path isn't an argument for accepting unvalidated data.

Fix this →

---

P2 — Inline env-assignment commands break the setup skill's wiring template

File: skills/setup/SKILL.md L65–75

When an existing statusLine.command begins with an inline env assignment (e.g. THEME=dark my-statusline), the printed template becomes:

"command": "bash \"/path/to/statusline-tee.sh\" THEME=dark my-statusline"

Bash then receives THEME=dark as the executable name (not an assignment), so "$@" inside the tee script tries to run THEME=dark as a binary and fails. The setup skill promises to "preserve the user's current command verbatim" — this breaks that promise.

Fix options:

1. Detect the pattern: if the current command starts with WORD=VALUE, the printed edit should wrap with sh -c '...':

   "command": "bash \"/path/to/statusline-tee.sh\" sh -c 'THEME=dark my-statusline'"

2. At minimum, warn: add a caveat under the wrapped template that commands starting with VAR=value assignments need to be wrapped in sh -c '...'.

Fix this →

---

Minor

apply "reset" is underspecified. SKILL.md L116 says "set the two recognized keys to the shipped defaults ONLY if the operator asked for a reset" but evals.json eval #4 (apply-preserves-unrecognized-keys) doesn't cover the reset path. The boundary between "just run apply" and "asked for a reset" is ambiguous and left to the LLM's interpretation. A note clarifying what "reset" means (e.g. apply reset or explicit operator language "reset to defaults") would remove drift between implementations.

smart_max_used_percentage: 0 rejected. context-zone.sh L94 requires smart_max > 0, so an operator who wants zero entries in the smart zone (their baseline is always acceptable) cannot express this without editing the script. This is a narrow edge case, and the current documentation says nothing can ever be smart with this config, so operators who need it would likely just use a value of 1. Not a blocker, but worth a comment in the reader contract's validity section (0 < smart_max).

$RANDOM in temp filename may be absent under --posix. statusline-tee.sh L109: $RANDOM is a Bash extension. While the script requires Bash and #!/usr/bin/env bash is set, very hardened environments with BASHOPTS=posix can suppress it. The $$ PID alone provides adequate disambiguation; dropping $RANDOM simplifies the name. Not a correctness issue since the || return 0 guard handles write failures.

---

What's working well

• Transparency contract is tight. Every failure path in the tee returns 0 and leaves $@ untouched. The set +o pipefail around the passthrough pipeline (L140–143) correctly handles the SIGPIPE case for stdin-ignoring wrapped commands.
• Test coverage is strong. The mv-shim approach for Windows locked-target simulation is exactly the right pattern. Cases 8/9 (retry-succeeds / retry-exhausted) are non-obvious to test and are both present.
• Pruning logic is correct. ! -name '.*' ! -name '*.tmp.*' in the find guard is redundant (dot-prefixed already excluded by ! -name '.*') but harmless, and the defense-in-depth is appropriate given the pruning is irreversible.
• Float-safe band comparison. Using awk for used_percentage comparison avoids bash integer arithmetic on floats (e.g. 50.5). The test for 50.5 in context-zone.test.sh confirms this path.
• CLAUDE.md compliance. The ~/.claude/context-guard/ carve-out instead of ${CLAUDE_PLUGIN_DATA} is clearly justified (cross-plugin seam) and documented in the reader contract. The plugin has no hardcoded paths, no userConfig secrets, no egress, no hooks, no MCP.

[claude]

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

---

Security Review — PR #1241 feat(context-guard)

Tasks

• Gather context
• Read changed shell scripts (statusline-tee.sh, context-zone.sh)
• Read test files, skill, plugin manifest, and security review record
• Analyze for injection, path traversal, secrets handling, and logic flaws
• Post security findings

---

Scope: All files added or modified in this PR. The triggering commit (5d34d8e) only set the executable bit on four shell scripts and fixed a markdown lint warning — no new logic. The security-relevant surface is in the earlier commits. Changed shell scripts reviewed in full; manifests, docs, and skill also reviewed.

---

Finding 1 — IMPORTANT / PLAUSIBLE: chmod 700 no-op on Windows enables snapshot forgery

statusline-tee.sh L92:

chmod 700 "$dir" 2>/dev/null || true

The comment above this line explicitly acknowledges it is a no-op on Windows ACL filesystems under Git Bash. On such systems the ~/.claude/context-guard/context/ directory remains accessible to all local users, so any co-user can:

• Read any <session_id>.json snapshot — exposing session_id values and context-window token counts (session-correlated metadata that a legitimate user may prefer private).
• Write a forged snapshot with an arbitrary session_id and any used_percentage — causing context-zone.sh invocations to return a fabricated zone (dumb on an empty window, smart on a full one). Consumers that gate dispatch decisions on the zone word would take the wrong branch.

The snapshot directory is the sole security perimeter for snapshot integrity: there is no HMAC, no in-process channel, and no signature. On Linux/macOS with POSIX ACLs the chmod 700 is effective. On Windows it is not, and neither the README nor the security review record in MIGRATION-PLAYBOOK.md surfaces the integrity consequence — only the availability degradation from missing jq is mentioned.

The reader contract (reference/reader-contract.md L64) instructs consumers to "treat all values as untrusted data" but provides no mechanism for them to verify that untrusted data came from the legitimate tee. On Windows that guidance is load-bearing and currently unmet.

---

Finding 2 — IMPORTANT / PLAUSIBLE: captured_at not format-validated before date -d; staleness check bypassable on Windows

context-zone.sh L71, L76–78:

ts=${parsed%% *}    # extracted from .captured_at — validated as string type only
...
snap_epoch=$(date -u -d "$ts" +%s 2>/dev/null ||
  date -u -j -f '%Y-%m-%dT%H:%M:%SZ' "$ts" +%s 2>/dev/null) || unknown
[[ "$snap_epoch" =~ ^[0-9]+$ ]] || unknown
age=$((now_epoch - snap_epoch))
((age >= -60 && age <= STALENESS_SECONDS)) || unknown

The jq validation pass (L61–69) checks that captured_at is a JSON string type — not that it is a valid ISO-8601 timestamp. On GNU/Linux date -d accepts a wide range of natural-language expressions ("now", "1 second ago", "yesterday"). A snapshot with "captured_at": "now" passes through jq's type check, resolves to an epoch ≈ now_epoch (age ≈ 0), clears the ^[0-9]+$ guard, and yields age ≈ 0 < 600 — so any used_percentage in such a forged snapshot produces a non-unknown zone and passes the staleness check.

On POSIX systems Finding 1's chmod 700 blocks unprivileged write access to the snapshot directory, so this is not independently exploitable. On Windows, where Finding 1 applies, this compounds directly: the two attacks share the same root cause and the same mitigation path.

The fix is a one-liner inserted before line 76, regardless of platform:

[[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown

This eliminates the relative-expression surface and makes context-zone.sh self-hardening independent of the directory permission state.

Note: the MIGRATION-PLAYBOOK.md security review record (L814) states that "snapshot values are only ever passed through jq, never interpolated into a shell" — this is factually inaccurate: $ts is extracted from the snapshot via jq -r and then passed to date -d "$ts". The || true quoting prevents word-splitting, so there is no arbitrary command execution, but date -d does interpret its argument as a date expression, not merely as bytes. The review record should be corrected alongside any fix.

Fix: add format guard before date -d →

---

Finding 3 — SUGGESTION / LOW: Temp filename uses $RANDOM (15-bit entropy) with observable PID

statusline-tee.sh L109:

local tmp="$dir/.$sid.json.tmp.$$.$RANDOM"

$RANDOM yields 0–32767 (15 bits). $$ (PID) is visible to local users via ps or /proc. When the PID is known, a local adversary can enumerate the 32768 candidate temp filenames, pre-plant symlinks for each pointing to a target file (e.g. ~/.claude/settings.json), and win the write race at line 114. The umask 077 subshell only sets permissions on newly created regular files — it does not prevent following a pre-existing symlink.

This is only exploitable when Finding 1 holds (Windows, no effective chmod), and requires a tight race on a short-lived temp file. In practice: low-confidence. The sleep 0.1 retry on rename is irrelevant — the race is on the initial >"$tmp" write, not the rename. Not exploitable on POSIX systems where chmod 700 is enforced.

---

What is NOT a finding

• session_id used as a filename: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both the write side (tee, L84) and the read side (zone resolver, L52) — path separators, dots, spaces, semicolons, and newlines all rejected.
• jq variable binding for snapshot values: --arg ts "$ts" in tee and -v u="$used" / -v s / -v a in awk are safe: values are bound as strings/variables, not interpolated into program text.
• "$@" wrapped command: originates from the operator's settings.json, not from snapshot data or stdin content. No injection surface.
• printf '%s' "$INPUT" | "$@": $INPUT is piped as data, not interpolated into a shell command. The wrapped command cannot interpret it as code.
• find -exec rm -f {} + pruning: $dir is constructed from $HOME plus a fixed literal suffix; -type f excludes symlinks (find uses lstat by default, so symlinks of type l do not match -type f); -maxdepth 1 prevents traversal.
• Standalone-mode jq expression (L152–153): $INPUT piped to jq -r '...' where the program is a fixed literal — no injection.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.
• No hooks, no MCP, no eval, no outbound network: confirmed clean.

---

Summary

The two IMPORTANT findings share the root cause: chmod 700 is ineffective on Windows, making the snapshot directory accessible to co-users. Finding 2 has an independent one-line fix ($ts format guard before date -d) that eliminates the freshness-bypass surface regardless of platform and costs nothing. Finding 1's underlying platform gap (no POSIX ACLs on Windows) cannot be fixed in the scripts alone, but should be surfaced in the README and security review record as an explicit Windows-scope limitation on snapshot integrity — not just on availability. Finding 3 is theoretical on any realistic deployment.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d34d8e1ee

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — per-session context snapshots, zone resolver + setup skill

Tasks

• Gather context and understand the request
• Read core scripts (statusline-tee.sh, context-zone.sh)
• Read test files
• Read setup skill and evals
• Read reference contract and plugin manifest
• Analyze and post review

---

Overall Assessment

The plugin is well-architected and thoroughly tested. The fail-open contract, atomic rename, path containment, float-safe band comparison, and SIGPIPE handling are all correct. Two issues warrant fixes before merge; three minor points are worth noting.

---

P2 — Non-ISO captured_at bypasses the staleness check

File: context-zone.sh:61-77

The jq validation at L63 confirms captured_at is a JSON string but does not validate its format. $ts is then handed verbatim to GNU date -d at L76:

snap_epoch=$(date -u -d "$ts" +%s 2>/dev/null || ...

GNU date -d accepts relative/symbolic expressions — "now", "yesterday", "5 minutes ago", "1 week". A snapshot with "captured_at": "now" produces age ≈ 0, passes ((age >= -60 && age <= 600)), and resolves a real zone instead of unknown. The fail-open contract ("absent, stale, or unparsable → unknown") is unconditional; accepting unvalidated timestamps breaks it.

A one-liner after L71 (ts=${parsed%% *}) closes this entirely:

[[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown

The test suite also has no case for a non-ISO captured_at. context-zone.test.sh should add a case with "captured_at": "now" expecting unknown. (The write_snapshot helper writes whatever is passed as $4, so this is a one-liner addition.)

Fix context-zone.sh + add test →

---

P2 — apply has no jq guard for the merge path; inline env-assignment wiring is silently broken

File: skills/setup/SKILL.md:99-118

Issue A — jq-absent merge: apply step 2 (file present, converge-don't-clobber) requires jq to merge. There is no instruction for the case where jq is absent and zones.json already exists. apply can be invoked directly without check, so the check step 1 jq gate is not a precondition here. Without a guard, the skill would either silently skip the merge or fail ambiguously. The fix is to add a sentence: if jq is absent and the file already exists, FAIL with the same install-jq remediation (same as check step 1). Step 1 (file absent → write template) doesn't require jq, so only step 2 needs the guard.

Issue B — inline env-assignment wiring template: The template at L65-75 prints:

"command": "bash \"<plugin-root>/scripts/statusline-tee.sh\" <current statusline command>"

If the operator's current command starts with an inline env assignment (e.g. THEME=dark my-statusline), this becomes:

"command": "bash ".../statusline-tee.sh" THEME=dark my-statusline"

Bash receives THEME=dark as the executable name — not a variable assignment — so "$@" in the tee tries to exec THEME=dark and fails. The skill promises to "preserve the user's current command verbatim," which breaks silently. The fix is a caveat appended to the template: commands starting with VAR=value must be wrapped in sh -c '...':

"command": "bash \"<plugin-root>/scripts/statusline-tee.sh\" sh -c 'THEME=dark my-statusline'"

Add jq guard to apply step 2 + caveat for env-assignment wiring →

---

Minor — Reader contract principle contradicts implementation

File: reference/reader-contract.md:63-64 / context-zone.sh:76

The contract states: "never string-interpolate snapshot values into a shell command, another interpreter, or a prompt." The implementation passes $ts (from the snapshot's captured_at field, via jq -r) to date -d "$ts". This is safe in practice: $ts is double-quoted (no word-splitting), date is a date parser not a code executor, and snap_epoch is validated as ^[0-9]+$ before use. But it visibly contradicts the stated principle.

If Finding 1's format guard is applied, $ts will be proven well-formed before date -d sees it, which eliminates the surface being guarded against. Either narrow the contract to say "never pass snapshot values to code interpreters (eval, sh -c)," or add a comment above L76 citing the format guard as the prerequisite. This is a documentation accuracy issue, not a correctness bug.

Narrow the contract language →

---

Minor — apply reset path is underspecified

File: skills/setup/SKILL.md:115-116

Step 2 says "set the two recognized keys to the shipped defaults ONLY if the operator asked for a reset (otherwise leave valid existing values alone)." What constitutes "asking for a reset" is left to the LLM's interpretation — no argument form (apply reset) or required phrasing is specified. The evals.json eval #4 (apply-preserves-unrecognized-keys) does not cover the reset path. Future skill implementations may disagree on what triggers it. A single sentence clarifying the trigger (e.g., "the operator used the word 'reset' in their request, or passed reset as the argument") removes the ambiguity.

---

Minor — Pruning TOCTOU on concurrent session refresh

File: statusline-tee.sh:106-107

find ... -mmin +20160 -exec rm -f {} + collects paths at scan time; if session B atomically renames a fresh snapshot into place between find collecting B.json and rm unlinking it, B's new snapshot is deleted. This is a genuine race condition — but it is benign: session B overwrites the file on its next statusline refresh (seconds later), and the prune only fires at the 14-day cutoff, making the race astronomically rare on any live machine. Not worth fixing, but worth a comment near the find call acknowledging the TOCTOU so future maintainers don't introduce a "fix" that actually makes things worse.

---

What's solid

• Fail-open is airtight. Every precondition failure in context-zone.sh routes to unknown; every tee failure in statusline-tee.sh returns 0. The wrapped statusline is untouchable.
• Path containment is thorough. [A-Za-z0-9_-]+$ enforced on both write (tee_snapshot) and read (context-zone.sh:52), with five hostile patterns exercised in Case 7.
• Atomic write is correct. Temp file in same directory + rename-with-retry; the umask 077 subshell keeps the snapshot owner-only without perturbing the wrapped command's umask; the sleep 0.1 retry correctly targets Windows FILE_SHARE_DELETE.
• Float-safe band comparison. awk for used_percentage avoids bash integer coercion on floats; used=50.5 is tested.
• SIGPIPE handled correctly. set +o pipefail scoped to exactly the passthrough pipeline at L140-143; printf's stderr is silenced for the same case.
• Test suite is comprehensive. The mv shim pattern (Cases 8/9) for deterministic rename-failure testing is exactly right. Cases 1-17 cover all documented failure modes, including the unwritable-dir case (Case 14) and the 200KB oversized payload / stdin-ignoring command (Case 15).
• CLAUDE.md compliance is complete. ${CLAUDE_PLUGIN_ROOT} and $HOME-anchored paths; no hardcoded values; no userConfig; versioned at 0.1.0; cross-plugin seam carve-out documented and justified; security review recorded in MIGRATION-PLAYBOOK.md.
• disable-model-invocation: true on setup skill — correct for a skill whose apply writes a file; prevents ambient invocation without explicit operator intent.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5fdd119699

ℹ️ 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".

[claude]

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

---

Security Review — PR #1241 feat(context-guard)

Tasks

• Gather context
• Read changed shell scripts (statusline-tee.sh, context-zone.sh)
• Read setup skill, evals, plugin manifest, and security review record
• Analyze for injection, path traversal, secrets handling, and logic flaws
• Post security findings

---

Scope: All files added in this PR. Head commit (5fdd119) only updates the README catalog row — the security-relevant surface is in earlier commits; reviewed in full. Three prior automated reviews have reported the same two IMPORTANT findings; neither has been addressed in the current HEAD. Findings below are independently verified against the current code.

---

Finding 1 — IMPORTANT / CONFIRMED: captured_at not ISO-8601 validated before date -d; staleness check bypassable

context-zone.sh L71–78

ts=${parsed%% *}    # .captured_at — validated as string type only, not format
...
snap_epoch=$(date -u -d "$ts" +%s 2>/dev/null ||
  date -u -j -f '%Y-%m-%dT%H:%M:%SZ' "$ts" +%s 2>/dev/null) || unknown
[[ "$snap_epoch" =~ ^[0-9]+$ ]] || unknown
age=$((now_epoch - snap_epoch))
((age >= -60 && age <= STALENESS_SECONDS)) || unknown

The jq validation pass (L61–69) checks that .captured_at is a JSON string type, but does not validate its format. On GNU/Linux, date -d accepts natural-language expressions ("now", "1 second ago", "yesterday"). A snapshot with "captured_at": "now" passes the type check, resolves to an epoch ≈ now_epoch, clears the ^[0-9]+$ guard, and yields age ≈ 0 < 600 — so any used_percentage in that snapshot produces a real zone instead of unknown.

The fail-open contract is unconditional; trustworthiness of the directory path is not an argument for accepting unvalidated data. Even on POSIX systems where chmod 700 is effective, the legitimate tee can only write well-formed ISO-8601 timestamps — the format guard costs nothing and closes this surface regardless of platform.

The fix is a single line inserted after L71 (before the date -d call):

[[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown

A test case for "captured_at": "now" is also missing from context-zone.test.sh — no existing assertion exercises a non-ISO string value.

Fix: add ISO-8601 format guard before date -d →

---

Finding 2 — IMPORTANT / PLAUSIBLE: chmod 700 no-op on Windows enables snapshot forgery and session-metadata disclosure

statusline-tee.sh L92

chmod 700 "$dir" 2>/dev/null || true

On Windows ACL filesystems under Git Bash, this is silently a no-op (the comment acknowledges this). The snapshot directory then has no effective access control, allowing any co-user on the machine to:

• Read ~/.claude/context-guard/context/*.json — disclosing session_id values and context-window token counts (session-correlated metadata).
• Write forged snapshots — any used_percentage with any valid session_id would resolve to a real zone word rather than unknown, because the snapshot directory is the sole security perimeter for snapshot integrity: there is no HMAC, no in-process channel, no signature. On Linux/macOS with POSIX ACLs the chmod 700 is effective. On Windows it is not.

The security review record (MIGRATION-PLAYBOOK.md L831) states the verdict is "1 bounded to operator-wired transparent scripts with sanitized untrusted input" — but does not surface the Windows-platform scope limitation on snapshot integrity, only the availability degradation from missing jq mentioned under Code Execution. The README similarly omits this.

This cannot be fully fixed in the scripts alone (Windows ACL is a platform constraint), but the security review record and README should explicitly state that on Windows, snapshot integrity is not enforced by file permissions and zone results should be treated as advisory-only in that environment.

---

Finding 3 — IMPORTANT / CONFIRMED: Security review record contains a false claim that weakens its ACCEPT justification

docs/MIGRATION-PLAYBOOK.md L813

"snapshot values are only ever passed through jq, never interpolated into a shell"

This is factually incorrect. At context-zone.sh L76, $ts is extracted from the snapshot via jq -r and then passed to date -d "$ts". This does not enable arbitrary command execution — "$ts" is double-quoted (no word-splitting) and date -d is a date-string parser, not a code executor — but date -d does interpret its argument as a date expression, not merely as bytes, which is precisely what Finding 1 exploits.

The claim underlies the ACCEPT verdict's code-execution justification and is the reason Finding 1 was not caught during the review. It should be corrected alongside the format guard: "snapshot values are only ever passed through jq or to date parsers; snapshot values are never passed to eval, sh -c, or any code executor."

---

Finding 4 — SUGGESTION / LOW: Temp filename has 15-bit entropy with observable PID

statusline-tee.sh L109

local tmp="$dir/.$sid.json.tmp.$$.$RANDOM"

$RANDOM produces 0–32767 (15 bits). $$ (PID) is observable by local users via ps or /proc. When both are known, an adversary can pre-plant symlinks for all 32768 candidate temp filenames and win the write race at the initial >"$tmp" redirect. The umask 077 subshell sets permissions on newly created regular files but does not prevent following a pre-existing symlink. The sleep 0.1 retry applies to the rename, not the write.

This requires Finding 2 to apply (Windows, no effective chmod) and a tight race window. Low-confidence; not exploitable on POSIX systems. The || return 0 guard on write failure means at worst the tee skips for that refresh. Noted for completeness.

---

What is NOT a finding

• session_id used as a filename: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both the write side (tee, L84) and the read side (zone resolver, L52) — path separators, dots, spaces, semicolons rejected.
• jq --arg ts "$ts" in the tee (L96): $ts is generated by date -u inside the tee itself, not from snapshot data. Safe.
• jq variable binding in awk (-v u="$used", -v s="$smart_max", -v a="$acceptable_max"): awk -v binds values as strings, not as awk program text. Even a malicious zones.json cannot inject awk code. Output is bounded to one of four known words.
• printf '%s' "$INPUT" | "$@": "$@" originates from the operator's settings.json, not from stdin data. No injection surface.
• find -exec rm -f {} + pruning: -type f uses lstat, excluding symlinks from matching. $dir is constructed from $HOME plus a fixed literal. Safe.
• No hooks, no MCP, no eval, no outbound network: confirmed clean.
• disable-model-invocation: true on the setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

Three findings: one IMPORTANT with a confirmed one-line fix (Finding 1 — ISO-8601 format guard on $ts before date -d); one IMPORTANT that needs documentation scope clarification in the security record and README rather than a code fix (Finding 2 — Windows chmod gap); one IMPORTANT that is a documentation correction to the ACCEPT justification to avoid perpetuating the wrong claim in future reviews (Finding 3 — inaccurate MIGRATION-PLAYBOOK assertion). Finding 1 is the only one with a code change and the prior three automated reviews have all flagged it. The remaining code is well-structured.

[claude]

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

---

PR Review: feat(context-guard) — per-session context snapshots, zone resolver + setup skill

Tasks

• Gather context and understand the request
• Read current state of shell scripts (after latest hardening commit)
• Read setup skill, evals, and reference contract
• Check security review record in MIGRATION-PLAYBOOK.md
• Analyze what prior findings were addressed vs. remain open
• Post final review

---

Overall Assessment

Ready to merge. The hardening commit (163a3a1) is a comprehensive, test-first response to the prior review round. Every P2 / IMPORTANT finding across the four automated reviews has been addressed. One minor documentation precision issue remains in the security review record.

---

What Was Fixed in 163a3a1

All of the following findings from the prior four reviews are confirmed resolved:

Finding	Source	Status
captured_at ISO-8601 format gate before date -d	All 4 reviews (primary finding)	✅ Fixed — context-zone.sh:81
Test cases for non-ISO captured_at ("now", "yesterday", "1 second ago")	Multiple reviews	✅ Added — context-zone.test.sh:115-119
Session ID equality check in jq (copied/renamed snapshot must not answer)	Codex P2 (2nd round)	✅ Fixed — context-zone.sh:66
Test for session_id mismatch	Codex P2	✅ Added — context-zone.test.sh:121-125
Pruning TOCTOU: per-file mtime re-check before unlink	Codex P2 (2nd round)	✅ Fixed — statusline-tee.sh:113-117
1 MiB truncation boundary not documented	Codex P2 (3rd round)	✅ Documented — statusline-tee.sh:67-70
Temp file: $RANDOM entropy + pre-planted symlink race	Security reviews	✅ Fixed — $RANDOM$RANDOM + set -o noclobber — statusline-tee.sh:122-128
SKILL.md: inline env-assignment wiring broken (THEME=dark cmd)	Multiple reviews (P2)	✅ Fixed — sh -c variant printed for shell-syntax commands — SKILL.md:88-100
SKILL.md apply: no jq guard for merge path when file exists	Multiple reviews (P2)	✅ Fixed — explicit FAIL + remediation — SKILL.md:132-135
Reader contract "untrusted data" contradicted the implementation	Multiple reviews	✅ Fixed — restated precisely ("validate format before lenient parser; never execute") — reader-contract.md:63-73
Windows chmod 700 integrity gap undocumented	Security reviews	✅ Documented in reader-contract, README, and MIGRATION-PLAYBOOK.md

The updated test harness now runs 44 assertions in context-zone.test.sh (up from 40), all green per the commit message.

---

Remaining Minor Item

File: docs/MIGRATION-PLAYBOOK.md:812

The Code Execution (1) bullet in the security review record still reads:

"snapshot values are only ever passed through jq, never interpolated into a shell"

This is imprecise: $ts is extracted from the snapshot via jq -r and then passed to date -d "$ts". The actual risk is now fully mitigated by the ISO-8601 format gate at context-zone.sh:81, and the reader contract and README both use accurate language ("validate format before lenient parsing; never execute"). But the security record's ACCEPT justification still contains the inaccurate claim, which the prior security review flagged as "Finding 3 — IMPORTANT / CONFIRMED."

The Data Egress bullet was correctly updated in 163a3a1; only the Code Execution bullet was not. A one-word adjustment ("and snapshot values are validated to their documented format before any typed parser (e.g. date -d for the ISO-8601 timestamp) and never passed to a code executor") would bring the record into alignment with everything else.

Fix the remaining imprecision →

Not a blocker.

---

Nit: apply reset trigger still implicit

File: skills/setup/SKILL.md:129

Step 2 says "ONLY if the operator asked for a reset" but doesn't define what constitutes asking for a reset (e.g., argument reset, the word "reset" in the request). evals.json eval #4 exercises the preserve-unrecognized-keys path but not the reset path. The LLM is left to interpolate. Not a correctness bug for the current scope, but noting it for the apply eval gap.

---

What Remains Solid

• Fail-open contract — every precondition failure in context-zone.sh correctly routes to unknown; all 44 test assertions cover the full failure mode surface including the two new forged-timestamp and session-id-mismatch cases.
• TOCTOU mitigation is correct: the per-file find "$f" -maxdepth 0 -mmin +20160 re-check runs inside the -exec sh -c loop, so a file atomically refreshed between scan and delete is now spared.
• set -o noclobber + $RANDOM$RANDOM — both are additive hardening. The noclobber guard is the meaningful one (refuses to follow a pre-planted symlink); the widened entropy is defense-in-depth.
• Shell-syntax guard in SKILL.md is the right approach: detect env-assignment/pipe/;/&& patterns and emit the sh -c '...' variant, not the argv-only form. The detection scope is realistic.
• jq guard in apply now clearly distinguishes the two paths: file absent (no jq needed → write template) vs. file present (jq required for merge → FAIL with remediation). This is the correct gate.
• Reader contract language now accurately describes the security posture: format-gate captured_at before any date parsing, require session_id to match, never pass to code executors. The Windows limitation is stated honestly with the practical consequence (zone is a routing hint, not a security input).
• CLAUDE.md compliance remains complete: ${CLAUDE_PLUGIN_ROOT} anchoring, no hardcoded paths, no userConfig, versioned 0.1.0, disable-model-invocation: true on the write-capable skill.

[claude]

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

---

Security Review — PR #1241 feat(context-guard)

Tasks

• Gather context and understand the request
• Get PR diff (HEAD: 163a3a1) and read changed files
• Audit shell scripts for injection, path traversal, logic flaws
• Audit skill and manifest for access-control gaps
• Audit docs/playbook for false security claims
• Post security findings

---

Scope: All 15 files added or changed in this PR, reviewed at HEAD 163a3a19. Prior automated reviews flagged two IMPORTANT findings; both were addressed in 163a3a19 ("harden resolver + tee per PR review findings"). This review audits the current state independently.

---

Finding 1 — SUGGESTION / PLAUSIBLE: Code Execution entry in security record still contains an inaccurate claim

docs/MIGRATION-PLAYBOOK.md L812–813

The Code Execution (1) item still reads:

"snapshot values are only ever passed through jq, never interpolated into a shell"

At the current HEAD, context-zone.sh L74–86 extracts $ts from .captured_at via jq -r, then passes it to date -d "$ts" after the new ISO-8601 format guard (L81). That's not "only ever passed through jq" — it reaches date -d as well. The double-quoting and format gate (^[0-9]{4}-…Z$) mean this cannot cause code execution, but the claim in the security record is factually wrong and undermines the trust of the ACCEPT verdict.

The Data Egress (5) item in the same record was correctly updated in 163a3a19 (L830–831): "the resolver format-gates captured_at and requires the embedded session id to match, so forgery cannot ride a lenient parser." The Code Execution item should match. Suggested correction:

"…and captured_at is format-gated to strict ISO-8601 before being passed to date -d; no snapshot value is passed to eval, sh -c, or any code executor."

Fix the Code Execution claim →

---

Previously flagged IMPORTANT findings — confirmed resolved at HEAD

The two IMPORTANT vulnerabilities raised by prior automated reviews have been addressed in 163a3a19:

• ISO-8601 format guard (context-zone.sh L81): [[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown — eliminates the natural-language date -d bypass. A test case for "captured_at": "now" was added to the test suite. ✅
• Session-id cross-check (context-zone.sh L66): .session_id? // null) != $sid in the jq validation pass — a copied or renamed snapshot cannot answer for a different session. ✅
• set -o noclobber on temp write (statusline-tee.sh L127): the initial >"$tmp" redirect now refuses to follow a pre-planted symlink or overwrite a pre-existing path at the temp name. ✅
• Doubled $RANDOM (statusline-tee.sh L122): $RANDOM$RANDOM raises temp-filename entropy to ~30 bits. ✅
• Windows chmod gap (reader-contract.md L68–73, MIGRATION-PLAYBOOK.md L826–831): both files now explicitly disclose the platform limitation, forbid attaching security decisions to zone words, and scope the consequence to routing hints only. ✅

---

What is NOT a finding

• Session-id filesystem use: validated to ^[A-Za-z0-9_-]+$ before any path construction on both write and read sides; path separators, dots, and semicolons all rejected.
• jq --arg ts "$ts" in the tee: $ts is produced by date -u inside the tee itself, not from snapshot input. No untrusted data reaches this call.
• awk variable binding (-v u=, -v s=, -v a=): awk -v binds values as strings, not as awk program text; output is constrained to three hardcoded words. Safe even with a crafted zones.json.
• find -exec sh -c '...' _ {} + pruning: filenames from find are passed as separate "$@" positional parameters; find "$f" and rm -f "$f" double-quote throughout; -type f excludes symlinks at the outer find level.
• mv -f "$tmp" "$target" rename: on Linux/macOS, rename(2) replaces a symlink at the destination path rather than following it, so a pre-planted symlink at $target cannot redirect the write to a sensitive file.
• printf '%s' "$INPUT" | "$@" passthrough: "$@" comes from the operator's settings.json, not from stdin data; no injection surface.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

One SUGGESTION: the Code Execution entry in MIGRATION-PLAYBOOK.md still says "snapshot values are only ever passed through jq" while $ts also reaches date -d (safely, after the format guard). This is a documentation accuracy issue in the ACCEPT justification, not an exploitable vulnerability. All IMPORTANT vulnerabilities identified in prior reviews are confirmed resolved at HEAD 163a3a19.

[kyle-sexton]

Review + security-review findings triaged — classification and disposition (fixes in 163a3a19):

Finding	Source	Verdict	Disposition
Non-ISO captured_at bypasses staleness (date -d "now")	codex P2, claude review F1, security review F2	VALID	Strict ISO-8601 format gate before any date parsing + red-green tests
Snapshot's embedded session_id not checked against requested id	codex P2	VALID	jq pass now requires equality; mismatch → unknown + test
Inline env-assignment statuslines break the argv wiring template	codex P2, claude review P2	VALID	Setup prints the sh -c '<original>' variant when shell syntax is detected
Prune TOCTOU can delete a concurrently refreshed sibling snapshot	codex P2	VALID (low sev)	Per-file mtime re-check immediately before unlink; residual microsecond race documented as fail-open + self-healing
Reader contract "untrusted data" wording contradicted date -d "$ts"	claude review F1 (moderate)	VALID	Both paths taken: format-validate-before-lenient-parse now implemented AND the contract wording restated precisely (never execute; validate format before parsing)
apply merge path undefined when jq absent + zones.json exists	claude review F2 (minor)	VALID	Explicit FAIL + install remediation; template write (no jq) unaffected
1 MiB stdin truncation undocumented	claude review F3 (minor)	VALID	Documented in the script header as a behavioral boundary
chmod 700 no-op on Windows ACL volumes → local forgery surface	security review F1 (IMPORTANT)	VALID (platform limitation)	No runtime fix exists in Git Bash; stated honestly in README, reader contract, and the playbook security record — zones are ROUTING hints and consumers must never attach security/egress decisions to a zone word. Compounding forgery vectors (lenient date parse, foreign session id) closed by the fixes above
$$.$RANDOM temp-name predictability + symlink follow	security review F3 (LOW)	VALID (low sev)	Entropy widened ($RANDOM$RANDOM) and the write runs under noclobber, which refuses pre-planted symlinks/existing paths

Tests: 44/44 (zone) + 40/40 (tee), shellcheck clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 163a3a19e0

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — per-session context snapshots, zone resolver + setup skill

Tasks

• Gather context and understand the request
• Read current state of changed files (HEAD: 78e8c8c)
• Review latest hardening commits (163a3a1, 78e8c8c)
• Check whether the open Codex finding (single-quote escaping in sh -c wrapper) was addressed
• Analyze overall correctness, security, and CLAUDE.md compliance
• Post final review

---

Review — HEAD 78e8c8c

The plugin is in very good shape. Prior rounds caught four IMPORTANT findings; all were addressed before this synchronize event, plus the 78e8c8c commit closed the stdin-truncation boundary by draining in 1MiB blocks. One new finding (surfaced by Codex at commit 163a3a19, after Kyle's triage at 06:14:50Z) is the only remaining open item.

---

P2 (open) — Single quotes inside sh -c '<original command>' are unescaped

File: plugins/context-guard/skills/setup/SKILL.md:97

"command": "bash \"<plugin-root>/scripts/statusline-tee.sh\" sh -c '<current statusline command>'"

If the operator's existing statusline command contains a single quote — e.g. printf '%s\n' hello — the template becomes:

bash ".../statusline-tee.sh" sh -c 'printf '%s\n' hello'

The '%s\n' closes the outer single-quoted string early, so sh -c receives a broken script and the operator's statusline stops working. The shell-syntax guard correctly detects that pipes, &&, ;, and inline env assignments need the sh -c variant — but the template itself is not safe for commands that contain single quotes.

Exact fix: the template should instruct that any ' in the original command be replaced with '\'' before embedding. Simplest form to document:

"command": "bash \"<plugin-root>/scripts/statusline-tee.sh\" sh -c '<single-quote-escaped original command>'"

with the note: "Replace each ' in the original with '\'' — e.g. printf '%s' x becomes sh -c 'printf '\''%s'\'' x'."

Alternatively, steer operators to the bash -c "..." form with double-quote escaping when the command is complex enough to need quoting (simpler to communicate, avoids the '\'' syntax).

Fix this →

---

Minor (open) — MIGRATION-PLAYBOOK line 812 claim is stale

File: docs/MIGRATION-PLAYBOOK.md:812

"snapshot values are only ever passed through jq, never interpolated into a shell"

$ts (extracted from .captured_at via jq -r) is now format-gated to strict ISO-8601 before being passed to date -d "$ts", so the security posture is correct. But this sentence still says "never interpolated into a shell" — date -d "$ts" IS a shell command receiving a snapshot-derived value. Lines 830–831 already state the accurate description ("the resolver format-gates captured_at and requires the embedded session id to match"). The stale claim on line 812 contradicts the updated record and could mislead future security reviewers. A one-word edit ("never passed to a code interpreter") would align it with the reader contract's precise language at line 66–67 of reader-contract.md.

---

Confirmed-closed (previously flagged, now resolved at HEAD)

Finding	Fix
captured_at non-ISO bypasses staleness check (date -d "now")	ISO-8601 regex gate at context-zone.sh:81; red-green tests for "now", "yesterday", "1 second ago"
Snapshot session_id not checked against requested id	jq equality check at context-zone.sh:66; mismatch → unknown; test at line 122
Inline env-assignment statuslines break direct argv wiring	Shell-syntax guard added at SKILL.md:88-100
apply merge path undefined when jq absent and zones.json exists	Explicit FAIL + install remediation at SKILL.md:132-134
Prune TOCTOU could delete a concurrently refreshed sibling	Per-file mtime re-check before unlink at statusline-tee.sh:121-124
$RANDOM temp-name entropy + symlink follow via pre-planted path	$RANDOM$RANDOM widens to 30 bits; noclobber refuses pre-existing paths at statusline-tee.sh:129-138
1 MiB stdin cap silently truncated wrapped command's input	Loop-drain reads until EOF in 1 MiB blocks at statusline-tee.sh:73-82; Case 15b test for >1 MiB round-trip
Reader contract wording contradicted date -d "$ts"	Contract now says "validate format BEFORE handing to a lenient parser" and "never pass to anything that executes them (eval, sh -c)" — accurate at reader-contract.md:63-67
Windows chmod 700 scope limitation undocumented	Reader contract, README, and playbook record (lines 826-831) now explicitly surface the Windows integrity caveat

---

What's solid at HEAD

• Fail-open is airtight. Every precondition failure in context-zone.sh routes to unknown; every tee failure returns 0. set +o pipefail is correctly scoped to the passthrough pipeline only.
• Path containment is thorough. [A-Za-z0-9_-]+$ enforced on both write and read sides; session-id mismatch between embedded and requested id now also caught.
• Test coverage is comprehensive. 40 tee assertions (including Case 15b for >1 MiB payload), 44 zone assertions (including "now"/"yesterday" rejection and session-id impersonation). The mv-shim pattern for deterministic Windows locked-target simulation remains well-crafted.
• Float-safe band comparison via awk is correct; 50.5 is tested.
• CLAUDE.md compliance is complete: repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam documented and justified, security review updated.

---

One P2 to address before merge (single-quote escaping in the sh -c template in SKILL.md); the stale claim in MIGRATION-PLAYBOOK is minor cleanup. Everything else is resolved.

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD 78e8c8c

Tasks

• Gather context from prior reviews
• Get PR diff at HEAD (78e8c8c)
• Read changed files (scripts, skill, docs)
• Audit 78e8c8c stdin-drain fix for new attack surface
• Check MIGRATION-PLAYBOOK.md security record accuracy
• Check SKILL.md sh -c single-quote escaping (last open Codex finding)
• Post security findings

---

Scope: All 15 files changed in this PR, reviewed at HEAD 78e8c8c. The two prior IMPORTANT findings (ISO-8601 format gate on $ts, session-id equality check) were both addressed in 163a3a1 and are confirmed resolved. This review covers the full current state plus the new stdin-drain commit.

---

No new attack surface in 78e8c8c

The stdin-drain loop introduced in the latest commit:

_chunk=""
while IFS= read -r -N 1048576 -t 5 _chunk; do
  INPUT+="$_chunk"
  _chunk=""
done
INPUT+="$_chunk"

INPUT is passed only to jq (as stdin data, not interpolated into jq program text) and to printf '%s' "$INPUT" | "$@" (piped as data to the wrapped command). The unbounded accumulation is bounded in practice by the statusline JSON schema (a few KB); and even for arbitrarily large payloads, all data flows into jq or the wrapped command as bytes, not as shell syntax. No new injection surface.

---

Finding 1 — SUGGESTION / CONFIRMED: Code Execution claim in security record remains factually wrong

File: docs/MIGRATION-PLAYBOOK.md L812

"snapshot values are only ever passed through jq, never interpolated into a shell"

At HEAD, context-zone.sh L81–86 extracts $ts from .captured_at via jq -r, then passes it to date -d "$ts" after the ISO-8601 format gate:

[[ "$ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || unknown
...
snap_epoch=$(date -u -d "$ts" +%s 2>/dev/null || ...)

The format gate makes this safe — no natural-language date -d bypass is possible after it. But the claim in the security record is still factually wrong; $ts reaches a typed parser (date -d) after passing through jq. The Data Egress (5) bullet at L830–831 was correctly updated in 163a3a1 to reference the format gate and session-id match requirement — the Code Execution (1) bullet at L812 was not.

This inaccuracy was flagged as IMPORTANT/CONFIRMED by three prior security reviews. It remains unfixed. Future reviewers relying on the record will have a false picture of the surface.

Suggested correction for L812–813:

"…sanitized to [A-Za-z0-9_-] before filename use; captured_at is format-gated to strict ISO-8601 UTC before being passed to date -d; no snapshot value is passed to eval, sh -c, or any code executor."

Fix the Code Execution claim →

---

Finding 2 — SUGGESTION / CONFIRMED: sh -c template does not escape single quotes in the wrapped command

File: plugins/context-guard/skills/setup/SKILL.md L97

"command": "bash \"<plugin-root>/scripts/statusline-tee.sh\" sh -c '<current statusline command>'"

Any existing statusLine.command containing a single quote (e.g. printf '%s\n' 'hello') will produce a broken template where the sh -c argument closes prematurely. This is not a security injection — the content comes from the user's own trusted settings.json and the skill never applies the template automatically. But the printed edit will silently fail or produce unintended shell behavior when the user pastes and applies it, which is the exact contract the skill promises to preserve ("preserve the user's current command verbatim").

The correct POSIX-safe escaping replaces ' with '\'' in the wrapped command before embedding it. The skill should at minimum note that commands containing single quotes require POSIX escaping (sed "s/'/'\'''/g") or an alternative (e.g. storing the command in a temp script file and invoking it). This finding was flagged by Codex in the last review round but not addressed in 78e8c8c.

Fix or document the single-quote escaping requirement →

---

Previously confirmed IMPORTANT findings — resolved at HEAD

All IMPORTANT vulnerabilities from the prior four review rounds are confirmed addressed:

Finding	Fixed in	Verification
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	Format gate at L81; test cases for "now", "yesterday", "1 second ago" → unknown ✅
Snapshot session_id not cross-checked (copied/renamed snapshot answers for other session)	163a3a1	jq gate at L66 requires .session_id == $sid ✅
Temp file pre-planted symlink race ($RANDOM 15-bit entropy + >"$tmp" follows symlink)	163a3a1	set -o noclobber at L134 refuses pre-existing path; $RANDOM$RANDOM doubles entropy to ~30 bits ✅
Prune TOCTOU: concurrent sibling refresh deleted by racing rm	163a3a1	Per-file mtime re-check before unlink at L123 ✅
Windows chmod 700 no-op integrity gap undocumented	163a3a1	Disclosed in reader-contract, README, and Data Egress (5) in the playbook ✅
Inline env-assignment wiring template broken (THEME=dark cmd)	163a3a1	sh -c variant printed for shell-syntax commands at SKILL.md L88–100 ✅
1 MiB stdin cap silently truncated wrapped command input	78e8c8c	Drain loop reads to EOF; L75–82 ✅

What is NOT a finding

• session_id path containment: validated to ^[A-Za-z0-9_-]+$ before filesystem use on both write (L94) and read (L52) sides.
• jq and awk variable bindings: --arg ts "$ts", -v u=, -v s=, -v a= — all bind values as typed data, not as program text. Output is constrained to four known words.
• printf '%s' "$INPUT" | "$@" passthrough: $INPUT is piped as data; "$@" originates from the operator's settings.json, not from stdin. No injection surface.
• find -exec sh -c '...' _ {} + pruning: filenames produced by find flow into "$f" as double-quoted positional parameters; -type f (using lstat) excludes symlinks from the outer scan.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target (does not follow it); the pre-planted symlink attack path at $target is not viable.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true: correct on the write-capable setup skill.

---

Summary

No CRITICAL or IMPORTANT security vulnerabilities remain at HEAD 78e8c8c. Two SUGGESTION-level items persist: a stale inaccurate claim in the MIGRATION-PLAYBOOK security record (Finding 1, flagged 3× previously, one-line fix), and the single-quote escaping gap in the sh -c wiring template (Finding 2, flagged by Codex in the prior round, documentation note needed). Neither is an exploitable vulnerability. The 78e8c8c stdin-drain fix is clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78e8c8c83a

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — HEAD 288d0dd

Tasks

• Gather context and understand the request
• Read changed files at HEAD (288d0dd)
• Verify POSIX-escape fix in SKILL.md
• Verify apply reset mode is now explicit
• Verify MIGRATION-PLAYBOOK.md stale claim status
• Post final review

---

Assessment — Ready to Merge

The 288d0dd commit addresses both P2 items remaining after 78e8c8c. One minor documentation accuracy issue persists in the security review record (stale since 163a3a1); it is not a blocker.

---

What 288d0dd Fixed

P2 resolved — Single-quote POSIX escaping in sh -c template
SKILL.md:97–106

The template now names the placeholder <escaped original command> and provides the explicit rule:

<escaped original command> is the original command POSIX-escaped for single-quote embedding: replace every ' in it with '\'' before substituting (then JSON-escape the whole command string as usual). Show the final, fully escaped line — never hand the operator a template with raw quotes left to fix. Verify your printed edit round-trips: mentally unquote it back and confirm it reproduces the original command byte-for-byte.

A command like printf '%s\n' 'hello' now produces a correct, complete, LLM-generated edit rather than a broken one. The round-trip verification instruction is the right safety net for a skill that cannot test the output by running it.

P2 resolved — Apply reset mode is now explicit and deterministic
SKILL.md:135–145

apply is now mode-explicit:

• apply (no argument): REPAIR-ONLY — valid recognized band values are left untouched and reported; only missing/invalid keys are set to defaults.
• apply reset: sets both recognized band keys to shipped defaults.

argument-hint: "check | apply [reset]" makes the optional argument discoverable. Eval #4 now names apply reset as the explicit route to shipped defaults, closing the "what did the operator mean by 'reset'?" ambiguity from prior reviews.

---

Remaining Minor Item (Not a Blocker)

File: docs/MIGRATION-PLAYBOOK.md:812

The Code Execution (1) bullet in the security review record still reads:

"snapshot values are only ever passed through jq, never interpolated into a shell"

At HEAD, $ts (from .captured_at) is extracted via jq -r and then passed to date -d "$ts" after the ISO-8601 format gate at context-zone.sh:81. The actual security posture is correct and the format gate makes this unexploitable. But the sentence in the security record is factually wrong and has been flagged as SUGGESTION/CONFIRMED by every security review since 163a3a1. The Data Egress (5) bullet at lines 830–831 was updated correctly in 163a3a1 and accurately describes the surface; the Code Execution bullet was not. A single-sentence correction would close this:

"…sanitized to [A-Za-z0-9_-] before filename use; captured_at is format-gated to strict ISO-8601 UTC before being passed to date -d; no snapshot value is passed to eval, sh -c, or any code executor."

Fix the Code Execution claim →

---

Confirmed Resolved Across All Review Rounds

Finding	Fixed in	Status
captured_at ISO-8601 gate before date -d	163a3a1	✅
Session-id equality check (copied snapshot can't answer for another session)	163a3a1	✅
SKILL.md sh -c template broken for inline env-assignment commands	163a3a1	✅
SKILL.md apply no jq guard for merge-path when file present	163a3a1	✅
Prune TOCTOU: per-file mtime re-check before unlink	163a3a1	✅
Temp-file $RANDOM entropy + symlink follow via pre-planted path	163a3a1	✅
1 MiB stdin cap silently truncated wrapped command's input	78e8c8c	✅
Reader contract "untrusted data" wording contradicted date -d "$ts"	163a3a1	✅
Windows chmod 700 scope limitation undocumented	163a3a1	✅
Single quotes inside sh -c '<original command>' unescaped	288d0dd	✅
Apply reset mode trigger ambiguous; LLM left to interpret	288d0dd	✅

---

What Remains Solid

• Fail-open is airtight. Every precondition failure in context-zone.sh routes to unknown; the ISO-8601 format gate at L81 and session-id equality check at L66 are both defensive-correct. The tee never surfaces failures to the operator's statusline pipeline.
• Test harness is comprehensive. 40 tee assertions (including >1 MiB round-trip) + 44 zone assertions (including "now"/"yesterday" → unknown and session-id mismatch → unknown). The mv-shim approach for deterministic Windows locked-target testing is well-crafted.
• POSIX-escape round-trip verification — the instruction to "mentally unquote it back and confirm it reproduces the original command byte-for-byte" is the right guard for a skill that generates edits without running them.
• Apply mode semantics are now unambiguous — REPAIR-ONLY vs. apply reset is deterministic, and eval ci: onboard to the CI platform (ci-workflows + standards) #4 validates the REPAIR-ONLY path against a concrete example with custom-but-valid bands.
• CLAUDE.md compliance is complete — repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam carve-out documented, disable-model-invocation: true on the write-capable skill.

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD 288d0ddb

Tasks

• Gather context
• Get PR diff at HEAD (288d0ddb) and read changed files
• Audit shell scripts for injection, path traversal, logic flaws
• Audit skill, evals, and manifest
• Check MIGRATION-PLAYBOOK.md security record accuracy
• Post security findings

---

Scope: All 15 files changed in this PR, reviewed at HEAD 288d0ddb. The latest commit changes only SKILL.md and evals.json — shell-script logic was last changed in 163a3a1 (78e8c8c changed the stdin-drain loop). All findings are independently verified against the current tree.

---

No CRITICAL or IMPORTANT vulnerabilities at HEAD

All IMPORTANT vulnerabilities identified across prior automated reviews are confirmed resolved:

Finding	Fixed in	Verification
captured_at non-ISO bypass (date -d "now" → fresh zone)	163a3a1	Format gate at context-zone.sh:81; test cases for "now", "yesterday", "1 second ago" → unknown ✅
Snapshot session_id not checked against requested id	163a3a1	jq equality at context-zone.sh:66; mismatch → unknown; test at line 122 ✅
Temp file $RANDOM entropy + pre-planted symlink race	163a3a1	set -o noclobber at statusline-tee.sh:134 (O_CREAT|O_EXCL refuses any pre-existing path including symlinks); $RANDOM$RANDOM at L129 for ~30-bit entropy ✅
Prune TOCTOU deletes concurrently-refreshed sibling	163a3a1	Per-file mtime re-check at statusline-tee.sh:123 before unlink ✅
Windows chmod 700 scope limitation undocumented	163a3a1	Disclosed in reader-contract (L68–73), README, and MIGRATION-PLAYBOOK.md L826–831 ✅
Inline env-assignment statuslines break argv wiring	163a3a1	Shell-syntax guard at SKILL.md:88–100 ✅
sh -c template breaks on single quotes in wrapped command	288d0ddb	POSIX-escape instruction at SKILL.md:102–106 (replace ' with '\''; JSON-escape after; verify round-trip) ✅
1 MiB stdin cap truncated wrapped command input	78e8c8c	Drain loop to EOF in 1 MiB blocks at statusline-tee.sh:73–82 ✅

---

Finding 1 — SUGGESTION / CONFIRMED: Security record still contains a false claim

File: docs/MIGRATION-PLAYBOOK.md L812–813

The Code Execution (1) bullet still reads:

"snapshot values are only ever passed through jq, never interpolated into a shell"

At HEAD, context-zone.sh L81–86 extracts $ts from .captured_at via jq -r, format-gates it to strict ISO-8601 UTC, then passes it to date -d "$ts". That's not "only ever through jq" — $ts reaches a typed date parser. The format gate makes this fully safe, but the ACCEPT justification states a false factual premise that any future security reviewer will notice (and every prior reviewer has).

The Data Egress (5) bullet at L830–831 was correctly updated in 163a3a1: "the resolver format-gates captured_at and requires the embedded session id to match, so forgery cannot ride a lenient parser." The Code Execution bullet should align. One-line fix:

"…sanitized to [A-Za-z0-9_-] before filename use; captured_at is format-gated to strict ISO-8601 UTC before being passed to date -d; no snapshot value is passed to eval, sh -c, or any code executor."

This finding has been flagged in every prior review round. It is a documentation-accuracy issue in the ACCEPT justification, not an exploitable vulnerability.

Fix the Code Execution claim →

---

What is NOT a finding

• session_id path containment: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both write (L94) and read (L52) sides; five hostile patterns exercised in tests.
• jq variable binding (--arg ts "$ts" in tee, -v u=/-v s=/-v a= in awk): all bind values as typed data, not as program text.
• $ts in tee (L105): generated by date -u inside the tee itself, not from stdin data; --arg ts "$ts" is safe.
• printf '%s' "$INPUT" | "$@" passthrough: $INPUT is piped as data; "$@" originates from operator's settings.json, not from stdin content — no injection surface.
• find -exec sh -c '...' _ {} + pruning: filenames flow into "$f" as double-quoted positional parameters; -type f (lstat) excludes symlinks from the outer scan; $dir is $HOME + fixed literal suffix.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target with the new file rather than following it — a pre-planted symlink at the target path cannot redirect the write.
• Stdin drain loop (78e8c8c): INPUT reaches only jq (piped as stdin data) and printf '%s' "$INPUT" | "$@" (piped as data); no injection surface regardless of payload size.
• POSIX-escape instruction in SKILL.md: the sh -c template source is the operator's own settings.json (trusted); the POSIX-escape and JSON-escape instructions are correct; the verify-round-trip instruction is an appropriate correctness check.
• apply reset argument: the argument-hint field is now "check | apply [reset]" and the skill logic is mode-explicit. apply without argument is repair-only; apply reset overwrites to shipped defaults. Both preserve unrecognized keys. No write path is ambiguous.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

One SUGGESTION remains (stale Code Execution claim in MIGRATION-PLAYBOOK.md L812, flagged in every prior round). No CRITICAL or IMPORTANT vulnerabilities exist at HEAD 288d0ddb. The 288d0ddb commit (POSIX-escape instruction + apply reset mode) introduces no new attack surface.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e26c47761

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — HEAD cf5628a

Tasks

• Gather context and understand the request
• Read statusline-tee.sh — verify the same-session non-regression fix (cf5628a)
• Read statusline-tee.test.sh — confirm new red-green test for the fix
• Verify all prior round findings are resolved at HEAD
• Post final review

---

Assessment — Ready to Merge

cf5628a closes the final open finding from the Codex review (same-session snapshot regression). All findings across all thirteen prior review rounds are now resolved.

---

What cf5628a Fixed

Codex Finding — "Serialize same-session snapshot writes" (last open item)
statusline-tee.sh:140–157

The rename loop now reads the target's captured_at before each attempt:

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then
  rm -f "$tmp" 2>/dev/null
  return 0
fi

If the target already carries a strictly newer timestamp, the older in-flight write removes its temp file and returns — the newer snapshot is preserved. The implementation is correct on four counts:

1. ISO-8601 lexical comparison is valid: both $ts (written by date -u '+%Y-%m-%dT%H:%M:%SZ') and the target's captured_at (same path) are zero-padded UTC strings of identical length and format — lexical order equals chronological order.
2. Re-checked per retry, not once at entry: a fresher snapshot that lands between the check and the rename on attempt N will be caught on attempt N+1. The residual microsecond race is accepted and documented — coarse zone bands and the next refresh make it inconsequential.
3. Fail-safe on bad/absent target: jq -r ... || existing_ts="" combined with [[ -n "$existing_ts" && ... ]] ensures that if the target is unreadable, absent, or malformed, existing_ts stays empty and the check short-circuits — the write proceeds rather than falsely blocking on corrupt state.
4. Correct temp-file cleanup on abort: rm -f "$tmp" is called before the early return 0 — no orphaned temp files.

Test Case 16b (statusline-tee.test.sh:298–311) pre-seeds a snapshot with a captured_at 2 minutes in the future, runs the tee, and asserts the future-timestamped snapshot survived. This is the correct observable form of the race — the seed mimics a newer concurrent write that already landed.

---

Complete Finding Resolution Across All Review Rounds

Finding	Fixed in	Status
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	✅
Snapshot session_id not cross-checked	163a3a1	✅
Inline env-assignment statuslines break argv wiring template	163a3a1	✅
apply merge path undefined when jq absent and zones.json exists	163a3a1	✅
Prune TOCTOU could delete a concurrently refreshed sibling	163a3a1	✅
Temp-file $RANDOM entropy + pre-planted symlink race	163a3a1	✅
Reader contract "untrusted data" wording contradicted date -d "$ts"	163a3a1	✅
Windows chmod 700 scope limitation undocumented	163a3a1	✅
1 MiB stdin cap silently truncated wrapped command's input	78e8c8c	✅
Single quotes inside sh -c '<original command>' unescaped	288d0dd	✅
Apply reset mode trigger ambiguous	288d0dd	✅
Security record Code Execution claim factually wrong	9e26c47	✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	✅

---

What's Solid at HEAD

• Fail-open is airtight. ISO-8601 gate at context-zone.sh:81, session-id equality check at L66, and every other precondition failure routes to unknown. No tee failure ever reaches the statusline pipeline.
• Path containment is thorough. [A-Za-z0-9_-]+$ enforced on both write (L94) and read (L52) sides; five hostile patterns exercised in tests.
• Test coverage is comprehensive. 40 tee assertions (Cases 1–17 including 15b and 16b) + 44 zone assertions covering every documented failure mode. The mv-shim approach for deterministic Windows locked-target simulation remains well-crafted.
• Security record is internally consistent. MIGRATION-PLAYBOOK.md lines 811–814 and 826–831 both accurately describe the mechanism; the ACCEPT verdict is factually sound.
• CLAUDE.md compliance is complete. Repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam documented, disable-model-invocation: true on the write-capable setup skill.

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD cf5628a

Tasks

• Gather context and read changed files
• Audit shell scripts for injection, path traversal, logic flaws
• Audit skill, evals, and manifest
• Check MIGRATION-PLAYBOOK.md security record accuracy
• Post security findings

---

Scope: All 15 files added in this PR, reviewed at HEAD cf5628a. The HEAD commit changes only statusline-tee.sh and statusline-tee.test.sh (anti-regression check on the rename loop). Prior reviews flagged two IMPORTANT findings; both were addressed in 163a3a1 and are confirmed resolved. This review covers the current state, with close attention to the new rename-loop logic in cf5628a.

---

Finding 1 — SUGGESTION / PLAUSIBLE: Anti-regression check converts forged-snapshot timing race into persistent DoS on Windows

statusline-tee.sh L148–151

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then
  rm -f "$tmp" 2>/dev/null
  return 0
fi

existing_ts is read from the target snapshot (which is writable by any co-user on Windows, where chmod 700 is a no-op). It is compared lexically without ISO-8601 format validation against $ts (the current refresh's timestamp, generated internally by date -u). Any string that sorts lexically later than the current time — including "2999-12-31T23:59:59Z" or any ISO-8601 future date — causes the tee to skip the rename and return 0.

Impact on the pre-existing Windows scope limitation:

Before cf5628a, an adversary on Windows who wrote a forged snapshot would lose control of it on the next legitimate statusline refresh (mv -f unconditionally overwrites). To maintain the forgery, they had to win a write race against each refresh cycle.

After cf5628a, an adversary on Windows can write a snapshot with a far-future captured_at (e.g. "2999-12-31T23:59:59Z") once. Every subsequent legitimate refresh checks "2999..." > "2026..." → true and silently skips. The result is a permanent snapshot freeze for that session_id:

• The zone resolver rejects the far-future timestamp as stale (age << -60 → unknown), so the zone output is conservative fail-open.
• But no fresh snapshot ever lands — the session is locked to unknown until the forged file is manually deleted.

A more targeted variant uses a captured_at within the clock-skew window (e.g., 50 seconds in the future). The resolver accepts it as fresh (age ≈ -50, within [-60, 600]), producing a real zone from the forged used_percentage. The tee's anti-regression check prevents overwrite for ~10 minutes (until the forged timestamp goes stale), at which point the adversary refreshes the forged snapshot. Before cf5628a this required continuous racing; now it requires a single write every ~10 minutes.

What keeps this bounded: This is Windows-only (requires the already-disclosed chmod 700 no-op), the reader contract explicitly prohibits attaching security decisions to zone words, and the consequence is still a routing-hint inaccuracy, not a privilege escalation. No new vulnerability class.

Potential mitigation: Add an ISO-8601 format gate on existing_ts before the comparison — the same guard used in context-zone.sh L81 — so a non-ISO value (or a value out of a plausible range, e.g. year > current + 1) skips the anti-regression skip and proceeds to the rename. This does not fully close the Windows chmod gap but removes the far-future freeze path.

---

Previously confirmed IMPORTANT findings — all resolved at HEAD

Finding	Fixed in	Verification
captured_at non-ISO bypass (date -d "now" → fresh zone)	163a3a1	ISO-8601 gate at context-zone.sh:81 ✅
Snapshot session_id not checked (copied/renamed snapshot answers for other session)	163a3a1	jq equality at context-zone.sh:66 ✅
Temp file $RANDOM entropy + pre-planted symlink race	163a3a1	set -o noclobber at statusline-tee.sh:134 + $RANDOM$RANDOM at L129 ✅
Prune TOCTOU deletes a concurrently-refreshed sibling	163a3a1	Per-file mtime re-check before unlink at statusline-tee.sh:123 ✅
Windows chmod 700 scope limitation undocumented	163a3a1	Disclosed in reader-contract, README, and MIGRATION-PLAYBOOK.md L826–831 ✅
Inline env-assignment statuslines break argv wiring template	163a3a1	Shell-syntax guard at SKILL.md L88–100 ✅
Single quotes in sh -c template break commands containing '	288d0dd	POSIX-escape instruction at SKILL.md L102–106 ✅
1 MiB stdin cap silently truncated wrapped command input	78e8c8c	Drain loop to EOF at statusline-tee.sh:73–82 ✅
Security record Code Execution claim factually wrong	9e26c47	MIGRATION-PLAYBOOK.md:811–814 now accurate ✅

What is NOT a finding

• existing_ts comparison injection risk: [[ "$existing_ts" > "$ts" ]] uses bash's [[ string comparison — no command execution, no word splitting. Even a crafted string value cannot inject commands here. Safe.
• session_id path containment: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both write (L94) and read (L52) sides; five hostile patterns exercised in tests.
• jq variable binding: --arg ts "$ts" in the tee and -v u=/-v s=/-v a= in awk bind values as typed data, not as program text.
• printf '%s' "$INPUT" | "$@" passthrough: "$@" originates from the operator's settings.json, not from stdin data. No injection surface.
• find -exec sh -c '...' _ {} + pruning: filenames flow into "$f" as double-quoted positional parameters; -type f (lstat) excludes symlinks from the outer scan.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target atomically — no symlink-follow on the destination side.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

One SUGGESTION: the anti-regression check in cf5628a reads existing_ts from the target file without format-validating it, which converts the pre-existing Windows forged-snapshot timing race into a persistent snapshot-freeze path. This is a marginal worsening of the already-disclosed Windows chmod 700 limitation, not a new vulnerability class. All nine IMPORTANT findings from prior review rounds are confirmed resolved at HEAD.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cf5628acb8

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — HEAD e7e2dd0

Tasks

• Gather context and understand the request
• Read the HEAD commit's changes (e7e2dd0)
• Verify the project-level statusLine scope fix in SKILL.md
• Confirm remaining issues and post final review

---

Assessment — Ready to Merge

e7e2dd0 closes the final Codex P2 finding (project-level statusLine shadow). All IMPORTANT findings across all prior review rounds are resolved. Two minor documentation nits in the latest commit; one SUGGESTION carried forward from the last security review.

---

What e7e2dd0 Fixed

Codex P2 — Project-level statusLine override silently blocks the tee

SKILL.md:30–37

Step 2 now reads all three settings scopes (user ~/.claude/settings.json, project .claude/settings.json, local .claude/settings.local.json), determines the EFFECTIVE command by scope precedence (most specific wins), and evaluates the wiring states against it. Step 5 targets the owning scope's file for the printed edit. When a shadow exists, the skill names it explicitly and surfaces the remove-the-override alternative.

Before this fix, an operator with a project-level statusLine shadow could successfully apply the user-file wiring edit and the tee would still never run — the project override takes precedence. The fix eliminates that failure mode: the printed edit now targets the file that actually controls the effective command.

The implementation is correct:

• Scope read is read-only; apply still writes only zones.json
• Stale-wiring detection (path comparison vs ${CLAUDE_PLUGIN_ROOT}) is unchanged and applies to the effective scope's command
• The sh -c variant and POSIX-escape instruction at L94–112 remain unaffected

---

Minor — Step 6 dotfiles reminder still names "user-scope file" after multi-scope fix

File: SKILL.md:119

"the printed edit changes a durable user-scope file the operator maintains"

With the new fix, the printed edit may target the project or local settings file rather than the user file. The dotfiles reminder is most relevant for the user file (project/local settings files are typically already tracked in the project repo or are intentionally gitignored), so it's not harmful here — but the phrasing is imprecise. A small update such as "When the edit targets the user ~/.claude/settings.json…" or just "the settings file the operator maintains" would match the actual scope.

Not a correctness bug.

---

Minor — evals.json eval #1 expectation now undercounts files read

File: evals/evals.json:11

"Reads settings.json only; never writes settings, pluginConfigs, or the snapshot directory"

check now reads three settings files (user, project, local), not one. The "only" in the expectation is ambiguous — it was written to mean "reads but never writes" — but taken literally it's now wrong. The write constraint remains accurate; just the read scope expanded. A note in the expectation clarifying "reads all settings scopes; never writes" would be precise.

Also, no eval covers the new shadow-detection path (project-level override present and named). Eval #2 (stale wiring) and eval #1 (clean state) both assume a single-scope world. This is a gap for the new feature, but not a blocker — the core correctness of SKILL.md is the SSOT.

---

Carried forward — SUGGESTION from last security review

File: statusline-tee.sh:148

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then

existing_ts is read from the on-disk snapshot without ISO-8601 format validation before the lexical comparison. On Windows (where chmod 700 is a no-op), an adversary can write a snapshot with a far-future captured_at ("2999-12-31T23:59:59Z"), causing every subsequent legitimate refresh to skip the rename. The resolver correctly returns unknown for a far-future timestamp (staleness check: age << -60 → unknown), so zone output remains conservative fail-open — but the session's snapshot is permanently frozen until the forged file is deleted.

This is a marginal worsening of the already-disclosed Windows chmod 700 platform limitation, not a new vulnerability class. The consequence is routing-hint disruption (always unknown), not privilege escalation or data corruption.

The independent fix is the same format gate used at context-zone.sh:81, applied after line 148:

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
[[ "$existing_ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || existing_ts=""

This prevents any non-ISO string from causing a freeze; it does not prevent a valid far-future ISO-8601 timestamp (that would also require a plausible-range check). Noting it for completeness — the MIGRATION-PLAYBOOK security record's Windows disclosure already covers the root cause.

---

Complete Finding Resolution Across All Review Rounds

Finding	Fixed in	Status
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	✅
Snapshot session_id not cross-checked	163a3a1	✅
Inline env-assignment statuslines break argv wiring template	163a3a1	✅
apply merge path undefined when jq absent and zones.json exists	163a3a1	✅
Prune TOCTOU could delete a concurrently refreshed sibling	163a3a1	✅
Temp-file $RANDOM entropy + pre-planted symlink race	163a3a1	✅
Reader contract "untrusted data" wording contradicted date -d "$ts"	163a3a1	✅
Windows chmod 700 scope limitation undocumented	163a3a1	✅
1 MiB stdin cap silently truncated wrapped command's input	78e8c8c	✅
Single quotes inside sh -c '<original command>' unescaped	288d0dd	✅
Apply reset mode trigger ambiguous	288d0dd	✅
Security record Code Execution claim factually wrong	9e26c47	✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	✅
Project-level statusLine shadow silently blocks the tee	e7e2dd0	✅

---

What Remains Solid

• Fail-open is airtight. ISO-8601 gate at context-zone.sh:81, session-id equality check at L66, and every other precondition failure routes to unknown.
• Anti-regression rename is correct. ISO-8601 lexical comparison re-checked on every retry; fail-safe on absent/malformed target; temp file cleaned up on early return.
• Test coverage — 40 tee assertions (including >1 MiB round-trip, Cases 15b and 16b) + 44 zone assertions (including "now"/"yesterday" → unknown and session-id mismatch → unknown).
• Security record accurate. MIGRATION-PLAYBOOK.md Code Execution (1) now correctly states the ISO-8601 format gate on captured_at before date -d; Data Egress (5) discloses the Windows chmod 700 limitation honestly.
• CLAUDE.md compliance is complete. Repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam documented and justified, disable-model-invocation: true on the write-capable skill.

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD e7e2dd0

Tasks

• Gather context and understand the request
• Get PR diff at HEAD (e7e2dd0) and read changed files
• Audit shell scripts, skill, and docs for security issues
• Check whether the last open SUGGESTION (existing_ts format validation) was addressed
• Post security findings

---

Scope: All 15 files changed in this PR, reviewed at HEAD e7e2dd0. The HEAD commit changes only plugins/context-guard/skills/setup/SKILL.md — shell-script logic was last changed in 163a3a1 (78e8c8c added the stdin drain loop; cf5628a added the anti-regression check).

---

HEAD commit (e7e2dd0) — no new attack surface

The commit changes SKILL.md steps 2 and 5 to resolve the EFFECTIVE statusLine command across user, project, and local settings scopes (most-specific wins), then targets the printed edit at the owning scope's file. Reviewed for security:

• Multi-scope settings file reads are local file reads (~/.claude/settings.json, .claude/settings.json, .claude/settings.local.json). Read-only; no untrusted input processed from these files beyond reading the current command string.
• Printed edit targeting — the edit is still print-only (operator applies by hand, skill never writes settings.json). The POSIX-escape and round-trip verification instructions from 288d0dd are preserved intact at SKILL.md:108–112.
• No shell execution paths changed, no injection surface introduced, no new file writes.

---

Finding 1 — SUGGESTION / PLAUSIBLE: existing_ts not ISO-8601 validated before anti-regression comparison (unchanged from cf5628a)

statusline-tee.sh L148–149

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then

existing_ts is read from the target snapshot's .captured_at via jq -r without ISO-8601 format validation before the lexical string comparison. By contrast, the zone resolver does format-gate the same field at context-zone.sh:81. The security review record's Data Egress (5) bullet (L829–832) notes that "the resolver format-gates captured_at" but doesn't cover the tee's additional read of the same field.

This is exploitable only on Windows where chmod 700 is a no-op (the already-disclosed scope limitation). On such systems, an adversary can write a snapshot with "captured_at": "2999-12-31T23:59:59Z" once; every subsequent legitimate refresh sees "2999..." > current_ts → skips → returns 0 without updating. The zone resolver's age check (age >= -60) rejects a far-future timestamp as well, so the consequence is permanent unknown (fail-open), not a forged zone word. The more targeted variant (a captured_at 1–60 seconds in the future, within the skew window) requires timing precision and continuous writes to sustain.

Mitigation (one line), same as the format gate used by context-zone.sh:

[[ "$existing_ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || existing_ts=""

Inserting this between L148 and L149 removes the far-future freeze path (non-ISO or out-of-format value falls through to the rename, overwriting the forged file).

This finding was first raised in the cf5628a security review and remains unaddressed at HEAD. Classified SUGGESTION because the consequence on POSIX systems (where chmod 700 is effective) is zero, and on Windows the worst-case outcome is fail-open unknown, not a forged zone or privilege escalation.

Fix: add ISO-8601 format gate on existing_ts before the anti-regression comparison →

---

All prior IMPORTANT findings — confirmed resolved at HEAD

Finding	Fixed in	Verification
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	Format gate at context-zone.sh:81 ✅
Snapshot session_id not checked against requested id	163a3a1	jq equality at context-zone.sh:66 ✅
Temp file $RANDOM entropy + pre-planted symlink race	163a3a1	set -o noclobber at statusline-tee.sh:134; $RANDOM$RANDOM at L129 ✅
Prune TOCTOU deletes a concurrently refreshed sibling	163a3a1	Per-file mtime re-check at statusline-tee.sh:123 ✅
Windows chmod 700 scope limitation undocumented	163a3a1	Disclosed in reader-contract, README, and MIGRATION-PLAYBOOK.md L826–831 ✅
Inline env-assignment statuslines break argv wiring template	163a3a1	Shell-syntax guard at SKILL.md:94–106 ✅
Single quotes inside sh -c template break commands containing '	288d0dd	POSIX-escape instruction at SKILL.md:108–112 ✅
1 MiB stdin cap silently truncated wrapped command input	78e8c8c	Drain loop to EOF at statusline-tee.sh:73–82 ✅
Security record Code Execution claim factually wrong	9e26c47	MIGRATION-PLAYBOOK.md:811–814 now accurate ✅
Overlapping older same-session write can regress newer snapshot	cf5628a	Anti-regression check at statusline-tee.sh:148–152 ✅
Setup targets wrong scope when project/local statusLine shadows user file	e7e2dd0	Scope resolution at SKILL.md:30–37 ✅

What is NOT a finding

• session_id path containment: validated to ^[A-Za-z0-9_-]+$ before filesystem use on both write (L94) and read (L52) sides.
• $ts in tee (L105): generated by date -u inside the tee, not from external data; --arg ts "$ts" to jq is safe.
• [[ string comparison on existing_ts: uses bash's [[ — no command execution, no word-splitting; a crafted string value cannot inject shell commands here.
• printf '%s' "$INPUT" | "$@" passthrough: "$@" originates from the operator's settings.json; $INPUT is piped as data. No injection surface.
• find -exec sh -c '...' _ {} + pruning: filenames flow into "$f" as double-quoted positional parameters; -type f (lstat) excludes symlinks; $dir is $HOME + fixed literal suffix.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target atomically, not following it.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

One SUGGESTION remains (Finding 1 — existing_ts not ISO-8601 validated before the anti-regression comparison in statusline-tee.sh:148, first raised in the cf5628a security review). No CRITICAL or IMPORTANT vulnerabilities exist at HEAD e7e2dd0. The e7e2dd0 commit (setup scope resolution) introduces no new attack surface.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7e2dd0ff1

ℹ️ 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".

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD f6a6c21

Tasks

• Gather context from prior reviews
• Get PR diff at HEAD (f6a6c21) and read changed files
• Audit shell scripts for injection, path traversal, logic flaws
• Audit skill, evals, and manifest
• Check MIGRATION-PLAYBOOK.md security record accuracy
• Post security findings

---

Scope: All 15 files changed in this PR, reviewed at HEAD f6a6c21. The HEAD commit is a comment-only change to statusline-tee.sh (documenting the accepted equal-second writer race). No code logic changed since e7e2dd0. All security-relevant surfaces independently audited.

---

No CRITICAL or IMPORTANT vulnerabilities at HEAD

All IMPORTANT vulnerabilities identified across prior automated reviews are confirmed resolved:

Finding	Fixed in	Verification
captured_at non-ISO bypass (date -d "now" → fresh zone)	163a3a1	Format gate at context-zone.sh:81 ✅
Snapshot session_id not checked against requested id	163a3a1	jq equality at context-zone.sh:66 ✅
Temp file $RANDOM entropy + pre-planted symlink race	163a3a1	set -o noclobber at statusline-tee.sh:134; $RANDOM$RANDOM at L129 ✅
Prune TOCTOU deletes a concurrently refreshed sibling	163a3a1	Per-file mtime re-check at statusline-tee.sh:123 ✅
Windows chmod 700 scope limitation undocumented	163a3a1	Disclosed in reader-contract.md:68–73, README, and MIGRATION-PLAYBOOK.md L826–831 ✅
Inline env-assignment statuslines break argv wiring template	163a3a1	Shell-syntax guard at SKILL.md:94–106 ✅
Single quotes inside sh -c template break commands containing '	288d0dd	POSIX-escape instruction at SKILL.md:108–112 ✅
1 MiB stdin cap silently truncated wrapped command input	78e8c8c	Drain loop to EOF at statusline-tee.sh:73–82 ✅
Security record Code Execution claim factually wrong	9e26c47	MIGRATION-PLAYBOOK.md:811–814 now accurate ✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	Anti-regression check at statusline-tee.sh:154–158 ✅
Setup targets wrong scope when project/local statusLine shadows user file	e7e2dd0	Scope resolution at SKILL.md:30–37 ✅
Equal-second tie undocumented	f6a6c21	Guard comment now names both residual races at statusline-tee.sh:146–153 ✅

---

Finding 1 — SUGGESTION / PLAUSIBLE: existing_ts not ISO-8601 validated before anti-regression comparison

statusline-tee.sh L154–155

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then

existing_ts is read from the on-disk snapshot's .captured_at via jq -r without ISO-8601 format validation before the lexical > comparison. The zone resolver applies this exact gate at context-zone.sh:81; the tee's anti-regression check does not.

On POSIX systems (Linux, macOS): not exploitable — chmod 700 is effective, the directory is owner-only.

On Windows (already-disclosed platform limitation, where chmod 700 is a no-op): an adversary who can write to the snapshot directory can set "captured_at": "2999-12-31T23:59:59Z". The string "2999..." sorts lexically later than any current timestamp, so every subsequent legitimate refresh sees "2999..." > "$ts" → removes its temp file and returns 0 without updating. The snapshot is permanently frozen. The consequence: the zone resolver's staleness check (age = now_epoch − snap_epoch, where snap_epoch is year 2999) produces an age far below −60, which fails ((age >= -60 && age <= 600)) → the resolver returns unknown. So the attack outcome is permanently degraded-to-fail-open, not a forged green zone.

The targeted variant (forged captured_at ~30 seconds in the future) passes both the format gate and the staleness check, enabling a short-window forged zone — but this requires the adversary to continuously refresh the forged timestamp (every 30–60 seconds) to stay within the staleness window, which is comparable to the pre-cf5628a race difficulty.

Independent one-line fix (mirrors context-zone.sh:81):

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
[[ "$existing_ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || existing_ts=""

This eliminates the far-future freeze path (a non-ISO or impossible-format value forces existing_ts="", causing the anti-regression check to skip and the rename to proceed, overwriting the forged file). It does not fully close the short-window forged-zone variant, which is bounded by the Windows chmod gap root cause already disclosed.

This finding was first raised in the cf5628a security review and remains open at HEAD.

---

What is NOT a finding

• [[ string comparison on existing_ts: uses bash's [[ — no command execution, no word-splitting; a crafted string cannot inject shell commands.
• $ts in the tee (L105): generated by date -u inside the tee, not from external data; --arg ts "$ts" to jq is safe.
• Session-id path containment: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both write (L94) and read (L52) sides.
• jq and awk variable bindings (--arg ts "$ts", -v u=, -v s=, -v a=): bind values as typed data, not program text; awk output constrained to three known words.
• printf '%s' "$INPUT" | "$@" passthrough: "$@" originates from operator's settings.json, not from stdin data. No injection surface.
• find -exec sh -c '...' _ {} + pruning: filenames flow into "$f" as double-quoted positional parameters; -type f uses lstat, excluding symlinks; $dir is $HOME + fixed literal suffix.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target atomically — a pre-planted symlink at the destination cannot redirect the write.
• rm -f "$tmp" in noclobber failure path: removes the symlink itself if $tmp was a pre-planted symlink; if absent, is a silent no-op.
• Stdin drain loop: INPUT flows only into jq (as piped stdin data) and printf '%s' "$INPUT" | "$@" (as piped data). No injection surface regardless of payload size.
• MIGRATION-PLAYBOOK.md Code Execution claim: accurately states the mechanism at L811–814; Data Egress (5) at L829–831 correctly discloses the Windows platform limitation.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

One SUGGESTION remains (existing_ts not ISO-8601 validated before the anti-regression comparison in statusline-tee.sh:154, introduced in cf5628a and still open at HEAD). The far-future freeze path it enables is Windows-only, fail-open in outcome (unknown rather than a forged green zone), and the root cause is the existing disclosed chmod 700 platform limitation. No CRITICAL or IMPORTANT vulnerabilities exist at HEAD f6a6c21. The HEAD commit (comment-only) introduces no new attack surface.

[claude]

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

---

PR Review: feat(context-guard) — HEAD f6a6c21

Tasks

• Gather context and understand the request
• Read current state of changed files (HEAD: f6a6c21)
• Check the latest commit's changes
• Verify the equal-second race documentation fix
• Check whether the existing_ts ISO-8601 format gate (carried-forward SUGGESTION) was applied
• Post final review

---

Assessment — Ready to Merge

f6a6c21 documents the equal-second writer tie as an accepted race, closing the Codex finding from commit e7e2dd0. One SUGGESTION from the cf5628a security review round still stands and was not addressed in this commit; see below. All IMPORTANT findings across all prior review rounds remain resolved.

---

What f6a6c21 Fixed

Codex finding — Equal-second same-session tie is now an accepted, documented race
statusline-tee.sh:146–153

The guard comment now explicitly names both residual races and the reasoning for accepting each:

"Two residual races are ACCEPTED by design rather than serialized: the check-to-rename microsecond window, and an equal-second tie (captured_at is second-precision, so strict > cannot order two writers born in the same second and the older may win). Worst case for both is a snapshot carrying context data at most one refresh/one second older than the newest — zone bands are coarse and the next refresh supersedes; a lock would add a cross-platform dependency (flock is absent on macOS) for no behavioral difference."

The analysis is correct. flock is absent on stock macOS; a lock-file approach would require a cross-platform stub or a tool dependency for a worst-case consequence that is already fail-open and self-healing (coarse 25-point zone bands; next statusline refresh supersedes). Accepting and documenting is the right call.

---

Carried-Forward SUGGESTION (Not a Blocker)

File: statusline-tee.sh:154–155

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then

existing_ts is read from the target snapshot without ISO-8601 format validation before the lexical > comparison. By contrast, the zone resolver format-gates the same field at context-zone.sh:81 before passing it to date -d. On Windows (where chmod 700 is a no-op, the disclosed platform limitation), an adversary could write a snapshot with "captured_at": "2999-12-31T23:59:59Z" once, permanently suppressing further tee writes for that session. The zone resolver correctly returns unknown for such a far-future timestamp (age << -60 → staleness check fails), so the consequence is permanent fail-open unknown rather than a forged zone.

The independent one-line fix (same gate used by context-zone.sh):

[[ "$existing_ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || existing_ts=""

This was first flagged in the cf5628a security review and carried forward in the e7e2dd0 round. The current commit doesn't add it. Not a blocker for merge given the Windows-only scope, fail-open consequence, and the fact that the root cause (chmod gap) is already honestly disclosed in the security record and reader contract.

Apply the format gate →

---

Complete Finding Resolution Across All Review Rounds

Finding	Fixed in	Status
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	✅
Snapshot session_id not cross-checked	163a3a1	✅
Inline env-assignment statuslines break argv wiring template	163a3a1	✅
apply merge path undefined when jq absent and zones.json exists	163a3a1	✅
Prune TOCTOU could delete a concurrently refreshed sibling	163a3a1	✅
Temp-file $RANDOM entropy + pre-planted symlink race	163a3a1	✅
Reader contract "untrusted data" wording contradicted date -d "$ts"	163a3a1	✅
Windows chmod 700 scope limitation undocumented	163a3a1	✅
1 MiB stdin cap silently truncated wrapped command's input	78e8c8c	✅
Single quotes inside sh -c '<original command>' unescaped	288d0dd	✅
Apply reset mode trigger ambiguous	288d0dd	✅
Security record Code Execution claim factually wrong	9e26c47	✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	✅
Project-level statusLine shadow silently blocks the tee	e7e2dd0	✅
Equal-second same-session tie undocumented	f6a6c21	✅
existing_ts not ISO-8601 validated before anti-regression comparison	—	SUGGESTION (open, not a blocker)

---

What Remains Solid at HEAD

• Fail-open is airtight. ISO-8601 gate at context-zone.sh:81, session-id equality check at L66, every other precondition failure routes to unknown.
• Race acceptance reasoning is sound. The equal-second and check-to-rename races are correctly modeled as a bounded, self-healing worst case: coarse 25-point zone bands and the next refresh make the practical exposure negligible; the cross-platform lock cost is real.
• Test coverage remains comprehensive. 40 tee assertions (Cases 1–17, including >1 MiB round-trip in 15b and anti-regression in 16b) + 44 zone assertions.
• Security record and reader contract are accurate. MIGRATION-PLAYBOOK.md lines 811–814 and 826–831 both correctly describe the mechanism; Windows scope limitation is honestly disclosed.
• CLAUDE.md compliance is complete. Repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam documented, disable-model-invocation: true on the write-capable setup skill.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6a6c21099

ℹ️ 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".

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD b96ab6b

Tasks

• Gather context and understand the request
• Get PR diff and read changed files at HEAD
• Audit shell scripts for injection, path traversal, logic flaws
• Audit skill, evals, and manifest
• Check MIGRATION-PLAYBOOK.md security record accuracy
• Check whether remaining SUGGESTION (existing_ts format gate) was addressed
• Post security findings

---

Scope: All 15 files changed in this PR, reviewed at HEAD b96ab6b. The HEAD commit changes only SKILL.md and evals.json (documentation fix: byte-for-byte → semantic preservation for zones.json unrecognized keys). Shell-script logic was last modified in 163a3a1 (78e8c8c added the stdin drain loop; cf5628a added the anti-regression check). All surfaces independently audited.

---

No CRITICAL or IMPORTANT vulnerabilities at HEAD

All IMPORTANT vulnerabilities identified across prior automated reviews are confirmed resolved:

Finding	Fixed in	Verification
captured_at non-ISO bypass (date -d "now" → fresh zone)	163a3a1	Format gate at context-zone.sh:81 ✅
Snapshot session_id not checked against requested id	163a3a1	jq equality at context-zone.sh:66 ✅
Temp file $RANDOM entropy + pre-planted symlink race	163a3a1	set -o noclobber at statusline-tee.sh:134; $RANDOM$RANDOM at L129 ✅
Prune TOCTOU deletes a concurrently refreshed sibling	163a3a1	Per-file mtime re-check at statusline-tee.sh:123 ✅
Windows chmod 700 scope limitation undocumented	163a3a1	Disclosed in reader-contract.md:68–73, README, and MIGRATION-PLAYBOOK.md L826–831 ✅
Inline env-assignment statuslines break argv wiring template	163a3a1	Shell-syntax guard at SKILL.md:94–106 ✅
Single quotes in sh -c template break commands containing '	288d0dd	POSIX-escape instruction at SKILL.md:108–112 ✅
1 MiB stdin cap silently truncated wrapped command input	78e8c8c	Drain loop to EOF at statusline-tee.sh:73–82 ✅
Security record Code Execution claim factually wrong	9e26c47	MIGRATION-PLAYBOOK.md:811–814 now accurate ✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	Anti-regression check at statusline-tee.sh:154 ✅
Setup targets wrong scope when project/local statusLine shadows user file	e7e2dd0	Scope resolution at SKILL.md:30–37 ✅

---

Finding 1 — SUGGESTION / PLAUSIBLE: existing_ts not ISO-8601 validated before anti-regression comparison (carried forward from cf5628a)

statusline-tee.sh L154–155

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then

existing_ts is read from the on-disk snapshot's .captured_at via jq -r without ISO-8601 format validation before the lexical > comparison. The zone resolver applies this exact gate at context-zone.sh:81; the tee's anti-regression check does not.

On POSIX systems (Linux, macOS): not exploitable — chmod 700 is effective.

On Windows (the already-disclosed platform limitation, chmod 700 is a no-op): a co-user can write a snapshot with "captured_at": "2999-12-31T23:59:59Z" once. The string "2999..." sorts lexically later than any current timestamp, so every subsequent legitimate refresh sees "2999..." > "$ts" → removes its temp file and returns 0 without updating. The session's snapshot is permanently frozen. The zone resolver correctly rejects a far-future timestamp (age << −60 → fails the staleness check), so the consequence is permanent fail-open unknown, not a forged green zone.

The targeted variant (a captured_at ~30 seconds in the future) passes the staleness check but requires the adversary to continuously refresh the forged timestamp to sustain it, which is comparable to the difficulty of the pre-cf5628a race.

The independent one-line fix (mirrors context-zone.sh:81):

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
[[ "$existing_ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || existing_ts=""

This eliminates the far-future freeze path; a non-ISO or out-of-format value causes the check to skip and the rename to proceed, overwriting the forged file. It does not close the short-window sustained-refresh variant, which is bounded by the Windows chmod 700 root cause already disclosed.

This finding was first raised in the cf5628a security review and remains open at HEAD b96ab6b.

Fix: add ISO-8601 format gate on existing_ts →

---

What is NOT a finding

• session_id path containment: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both write (L94) and read (L52) sides.
• $ts in the tee (L105): generated by date -u inside the tee itself, not from external data; --arg ts "$ts" to jq is safe.
• [[ string comparison on existing_ts: uses bash's [[ — no command execution, no word-splitting; a crafted string cannot inject shell commands.
• rm -f "$tmp" in noclobber failure path: removes the symlink itself on Linux if $tmp was pre-planted; if absent, silent no-op. Cannot write through it.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target atomically — a pre-planted symlink at the destination cannot redirect the write.
• find -exec sh -c '...' _ {} + pruning: filenames flow into "$f" as double-quoted positional parameters; -type f (lstat) excludes symlinks from the outer scan; $dir is $HOME + fixed literal suffix.
• awk variable binding (-v u=, -v s=, -v a=): binds values as typed strings, not as awk program text; output is constrained to three hardcoded words.
• printf '%s' "$INPUT" | "$@" passthrough: "$@" originates from operator's settings.json, not from stdin data. No injection surface.
• Stdin drain loop: INPUT flows only into jq (piped as stdin data) and printf '%s' "$INPUT" | "$@" (piped as data). No injection surface.
• HEAD commit (b96ab6b) — documentation-only: changes SKILL.md and evals.json to clarify that zones.json preservation is semantic (value-level, not lexical). No code logic altered; no new attack surface.
• MIGRATION-PLAYBOOK.md Code Execution claim: accurately states the mechanism at L811–814.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

One SUGGESTION remains (existing_ts not ISO-8601 validated before the anti-regression comparison in statusline-tee.sh:154, introduced in cf5628a and carried forward through all subsequent reviews). The far-future freeze it enables on Windows is fail-open in outcome (unknown rather than a forged green zone) and shares its root cause with the already-disclosed Windows chmod 700 platform limitation. No CRITICAL or IMPORTANT vulnerabilities exist at HEAD b96ab6b. The HEAD commit (semantic preservation documentation fix) introduces no new attack surface.

[claude]

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

---

PR Review: feat(context-guard) — HEAD b96ab6b

Tasks

• Gather context and understand the request
• Read HEAD commit changes (b96ab6b)
• Verify zones.json semantic preservation fix
• Check remaining open SUGGESTION (existing_ts ISO-8601 gate)
• Audit all prior round findings at HEAD
• Post final review

---

Assessment — Ready to Merge

b96ab6b closes the final Codex P2 finding (byte-level preservation of zones.json unrecognized keys was unachievable via jq merge). All IMPORTANT findings across all prior review rounds are resolved. One SUGGESTION remains open from cf5628a (carried forward through four review rounds, not a blocker).

---

What b96ab6b Fixed

Codex finding — zones.json semantic vs. byte-level preservation

SKILL.md:147–151 / evals.json:45

The prior contract claimed apply "preserves every unrecognized key byte-for-byte." A jq merge reserializes the entire document — Unicode escape spellings normalize ("blue" → "blue"), whitespace may change — so byte-identity was structurally unachievable. The fix is correct on all counts:

• The new language is accurate: value-level (semantic) preservation — same keys, same JSON values; lexical formatting may normalize.
• Consumers are correctly redirected: parse zones.json as JSON, never depend on raw bytes.
• Eval ci: onboard to the CI platform (ci-workflows + standards) #4 now says "semantic preservation — lexical formatting may normalize" and "preserves the unrecognized display_color key and its value" — a precise statement of what jq actually delivers.
• No behavior change; only the contract description was wrong.

---

Carried-Forward SUGGESTION (Not a Blocker)

File: statusline-tee.sh:154–155

existing_ts=$(jq -r '.captured_at // empty' "$target" 2>/dev/null) || existing_ts=""
if [[ -n "$existing_ts" && "$existing_ts" > "$ts" ]]; then

existing_ts is read from the on-disk snapshot's .captured_at without ISO-8601 format validation before the lexical > comparison. By contrast, the zone resolver applies the exact gate at context-zone.sh:81. On Windows (where chmod 700 is a no-op, already-disclosed), a co-user could write "captured_at": "2999-12-31T23:59:59Z" once to permanently suppress tee writes for that session. The zone resolver's staleness check (age << -60 → unknown) makes the outcome fail-open (unknown), not a forged zone word.

The one-line fix mirrors context-zone.sh:81:

[[ "$existing_ts" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$ ]] || existing_ts=""

This was first raised in the cf5628a security review and has been carried through four rounds without being addressed. Windows-only; fail-open consequence. Not a blocker.

Apply the format gate →

---

Two Minor Nits (Not Blockers)

Eval #1 expectation "Reads settings.json only" — evals.json:11 — After the e7e2dd0 scope-resolution fix, check reads three settings files (user, project, local), not one. The "only" reads as "reads one settings file" but was intended to mean "reads but never writes." Ambiguous but not wrong about the write constraint; clarifying to "reads all settings scopes; never writes" would be precise.

Step 6 dotfiles reminder — SKILL.md:119 — still says "user-scope file the operator maintains," but after e7e2dd0 the printed edit may target the project or local settings file. The reminder is most relevant for ~/.claude/settings.json (the dotfiles-tracked one), so not wrong in practice, but slightly imprecise after the multi-scope fix.

---

Complete Finding Resolution Across All Review Rounds

Finding	Fixed in	Status
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	✅
Snapshot session_id not cross-checked	163a3a1	✅
Inline env-assignment statuslines break argv wiring template	163a3a1	✅
apply merge path undefined when jq absent and zones.json exists	163a3a1	✅
Prune TOCTOU could delete a concurrently refreshed sibling	163a3a1	✅
Temp-file $RANDOM entropy + pre-planted symlink race	163a3a1	✅
Reader contract "untrusted data" wording contradicted date -d "$ts"	163a3a1	✅
Windows chmod 700 scope limitation undocumented	163a3a1	✅
1 MiB stdin cap silently truncated wrapped command's input	78e8c8c	✅
Single quotes inside sh -c '<original command>' unescaped	288d0dd	✅
Apply reset mode trigger ambiguous	288d0dd	✅
Security record Code Execution claim factually wrong	9e26c47	✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	✅
Project-level statusLine shadow silently blocks the tee	e7e2dd0	✅
Equal-second same-session tie undocumented	f6a6c21	✅
zones.json byte-level preservation unachievable via jq merge	b96ab6b	✅
existing_ts not ISO-8601 validated before anti-regression comparison	—	SUGGESTION (open, not a blocker)

---

What's Solid at HEAD

• Fail-open is airtight. ISO-8601 gate at context-zone.sh:81, session-id equality check at L66, and every other precondition failure routes to unknown. The tee never surfaces failures to the operator's statusline pipeline.
• Semantic preservation contract is now technically accurate. jq deserializes and reserializes; the old "byte-for-byte" claim was wrong about what the code actually does. The new language correctly describes value identity with normalization of lexical representation.
• Zones.json consumer guidance is correct. reader-contract.md:136 already said "The byte-identity rule covers the inlined defaults only" (not the unrecognized keys), consistent with the updated SKILL.md language.
• Test coverage — 40 tee assertions (including >1 MiB round-trip, Cases 15b and 16b) + 44 zone assertions (including "now"/"yesterday" → unknown and session-id mismatch → unknown).
• Security record and reader contract are accurate. MIGRATION-PLAYBOOK.md lines 811–814 and 826–831 both correctly describe the mechanism; Windows scope limitation is honestly disclosed.
• CLAUDE.md compliance is complete. Repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam documented and justified, disable-model-invocation: true on the write-capable setup skill.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b96ab6b707

ℹ️ 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".

[claude]

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

---

PR Review: feat(context-guard) — HEAD ef0bf08

Tasks

• Gather context and understand the request
• Read HEAD commit changes (ef0bf08)
• Verify plausibility-ceiling fix in statusline-tee.sh
• Confirm all prior findings resolved
• Post final review

---

Assessment — Ready to Merge

ef0bf085 closes the last open finding carried through four review rounds (existing_ts not validated before the anti-regression comparison). The approach taken is actually more complete than the ISO-8601 format-gate that was repeatedly suggested — it also closes valid-but-implausible ISO timestamps that a pure format gate would have admitted.

---

What ef0bf085 Fixed

Last open SUGGESTION — Implausibly-future existing_ts can permanently freeze the tee

statusline-tee.sh:140–166

The fix introduces a plausibility ceiling: guard_ceiling = now + 5 minutes. The no-regression guard now honors an existing snapshot only when all four conditions hold:

if [[ -n "$existing_ts" && -n "$guard_ceiling" && "$existing_ts" > "$ts" &&
  ! "$existing_ts" > "$guard_ceiling" ]]; then

This correctly handles every case:

existing_ts	Condition 4 (! existing_ts > ceiling)	Outcome
Normal concurrent refresh (seconds ahead)	ceiling is 5 min → existing ≤ ceiling → true	Honored ✅
2-minute-ahead (Case 16b)	within 5-min window → true	Honored ✅
"9999-12-31T23:59:59Z" (Case 16c)	9999 > ceiling → false	Replaced ✅
Any non-ISO string starting with letters (e.g. "now")	letter > "2026..." → false	Replaced ✅
guard_ceiling empty (date fails)	[[ -n "" ]] → false → short-circuits	Always writes (fail toward freshness) ✅

The ceiling approach is strictly superior to the suggested ISO-8601 format gate: a pure format gate would pass "9999-12-31T23:59:59Z" (valid ISO-8601, just implausible), leaving the far-future freeze unaddressed. The plausibility ceiling closes both non-ISO strings and valid-but-implausible ISO dates in a single check.

Test Case 16c (statusline-tee.test.sh:313–322) seeds sess-42.json with "9999-12-31T23:59:59Z", runs the tee, and asserts the written snapshot has used_percentage: 8 (the fresh tee value), confirming the far-future file was overwritten. This is the correct red-green observable form.

---

Complete Finding Resolution Across All 17 Findings

Finding	Fixed in	Status
captured_at non-ISO date -d bypass ("now" → fresh zone)	163a3a1	✅
Snapshot session_id not cross-checked	163a3a1	✅
Inline env-assignment statuslines break argv wiring template	163a3a1	✅
apply merge path undefined when jq absent and zones.json exists	163a3a1	✅
Prune TOCTOU could delete a concurrently refreshed sibling	163a3a1	✅
Temp-file $RANDOM entropy + pre-planted symlink race	163a3a1	✅
Reader contract "untrusted data" wording contradicted date -d "$ts"	163a3a1	✅
Windows chmod 700 scope limitation undocumented	163a3a1	✅
1 MiB stdin cap silently truncated wrapped command's input	78e8c8c	✅
Single quotes inside sh -c '<original command>' unescaped	288d0dd	✅
Apply reset mode trigger ambiguous	288d0dd	✅
Security record Code Execution claim factually wrong	9e26c47	✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	✅
Project-level statusLine shadow silently blocks the tee	e7e2dd0	✅
Equal-second same-session tie undocumented	f6a6c21	✅
zones.json byte-level preservation unachievable via jq merge	b96ab6b	✅
Implausibly-future existing_ts permanently suppresses tee writes	ef0bf085	✅

---

What's Solid at HEAD

• Fail-open is airtight. ISO-8601 gate at context-zone.sh:81, session-id equality check at L66, and every other precondition failure routes to unknown. The tee never surfaces failures to the statusline pipeline.
• Anti-regression guard is now complete. Plausibility ceiling (now + 5 min) closes the far-future freeze path; guard_ceiling computation failure disables the guard and fails toward freshness; Cases 16b and 16c validate both the normal-race and the implausible-future paths independently.
• Test coverage is comprehensive. 44 tee assertions across 17 cases (including Case 15b for >1 MiB round-trip, Case 16b for same-session non-regression, Case 16c for plausibility ceiling) + 44 zone assertions (including "now"/"yesterday" → unknown and session-id mismatch → unknown).
• Security record is accurate. MIGRATION-PLAYBOOK.md lines 811–814 and 826–831 correctly describe the mechanism; Windows chmod 700 limitation is honestly disclosed with the routing-hints-only consequence.
• CLAUDE.md compliance is complete. Repo-agnostic paths, no userConfig, versioned 0.1.0, cross-plugin seam documented and justified, disable-model-invocation: true on the write-capable setup skill.

No open findings remain. All 17 are confirmed resolved at HEAD ef0bf085.

[claude]

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

---

Security Review — PR #1241 feat(context-guard) — HEAD ef0bf085

Tasks

• Gather context from prior reviews
• Get PR diff at HEAD (ef0bf085) and read changed files
• Audit the plausibility ceiling fix for new attack surface
• Verify the existing_ts ISO-8601 suggestion is resolved
• Check all prior IMPORTANT findings remain resolved
• Post security findings

---

Scope: All 15 files changed in this PR, reviewed at HEAD ef0bf085dad27db20e08454941fbedc2a58424d2. The HEAD commit changes only statusline-tee.sh and statusline-tee.test.sh — specifically the plausibility ceiling added to the no-regression guard introduced in cf5628a. All shell-script logic reviewed against the current tree.

---

No CRITICAL or IMPORTANT vulnerabilities at HEAD

All IMPORTANT vulnerabilities identified across prior automated reviews are confirmed resolved:

Finding	Fixed in	Verification
captured_at non-ISO bypass (date -d "now" → fresh zone)	163a3a1	Format gate at context-zone.sh:81 ✅
Snapshot session_id not checked against requested id	163a3a1	jq equality at context-zone.sh:66 ✅
Temp file $RANDOM entropy + pre-planted symlink race	163a3a1	set -o noclobber at statusline-tee.sh:134; $RANDOM$RANDOM at L129 ✅
Prune TOCTOU deletes a concurrently refreshed sibling	163a3a1	Per-file mtime re-check before unlink at statusline-tee.sh:123 ✅
Windows chmod 700 scope limitation undocumented	163a3a1	Disclosed in reader-contract.md:68–73, README, and MIGRATION-PLAYBOOK.md ✅
Inline env-assignment statuslines break argv wiring template	163a3a1	Shell-syntax guard at SKILL.md:94–106 ✅
Single quotes inside sh -c template break commands containing '	288d0dd	POSIX-escape instruction at SKILL.md:108–112 ✅
1 MiB stdin cap silently truncated wrapped command input	78e8c8c	Drain loop to EOF at statusline-tee.sh:73–82 ✅
Security record Code Execution claim factually wrong	9e26c47	MIGRATION-PLAYBOOK.md:811–814 accurate ✅
Overlapping older same-session write can regress a newer snapshot	cf5628a	Anti-regression check at statusline-tee.sh:165 ✅
Setup targets wrong scope when project/local statusLine shadows user file	e7e2dd0	Scope resolution at SKILL.md:30–37 ✅
Far-future captured_at permanently suppresses tee writes (freeze DoS)	ef0bf085	Plausibility ceiling at statusline-tee.sh:140–166 ✅

---

ef0bf085 — Plausibility ceiling: no new attack surface

The HEAD commit adds two things:

1. guard_ceiling computation (statusline-tee.sh:147–148):

guard_ceiling=$(date -u -d '+5 minutes' '+%Y-%m-%dT%H:%M:%SZ' 2>/dev/null ||
  date -u -v '+5M' '+%Y-%m-%dT%H:%M:%SZ' 2>/dev/null) || guard_ceiling=""

Both date arguments are fixed literals — no user or snapshot data involved. The || guard_ceiling="" makes any date-command failure disable the guard entirely, failing toward freshness. No injection surface.

2. Updated anti-regression condition (statusline-tee.sh:165–166):

if [[ -n "$existing_ts" && -n "$guard_ceiling" && "$existing_ts" > "$ts" &&
  ! "$existing_ts" > "$guard_ceiling" ]]; then

The added ! "$existing_ts" > "$guard_ceiling" clause (meaning: existing_ts ≤ guard_ceiling) constrains the guard to only honor timestamps in the range (ts, ts+5min]. Traced through the attack paths that were previously concerning:

existing_ts value	> ts	! > guard_ceiling	Guard fires?	Outcome
"2999-12-31T23:59:59Z" (far-future)	TRUE	FALSE	No	Rename proceeds; forged file replaced ✓
"now" (non-ISO, sorts after '2')	TRUE	FALSE	No	Rename proceeds ✓
"abc" (non-ISO, 'a' > '2')	TRUE	FALSE	No	Rename proceeds ✓
"2026-07-24T07:07:00Z" (2 min ahead, within ceiling)	TRUE	TRUE	Yes	Guard fires (intended: newer snapshot exists)
"1970-01-01T00:00:00Z" (old)	FALSE	—	No	Rename proceeds ✓

The guard_ceiling acts as an escape hatch for every value that could cause a permanent freeze. The POSIX [[ string comparison with ! is syntactically correct (! negates the entire binary expression inside [[ ]]).

Short-window forged snapshot (residual, previously noted): An adversary on Windows who writes captured_at ≈ 30 seconds ahead can still trigger the guard and suppress a single refresh cycle, while also passing the zone resolver's staleness check (age ≈ −30 ∈ [−60, 600]). However: sustaining this requires refreshing the forged file every ~30–60 seconds; when the adversary stops, the legitimate tee immediately recovers on the next refresh. The zone word is a routing hint; the reader contract explicitly prohibits attaching security or egress decisions to it. This remains within the scope of the already-disclosed Windows chmod 700 platform limitation, not a new vulnerability class.

Test case 16c exercises the far-future freeze path correctly: seeds the snapshot with "captured_at":"9999-12-31T23:59:59Z" and asserts the legitimate write supersedes it.

---

What is NOT a finding

• guard_ceiling date arguments: both are fixed literals ('+5 minutes', '+5M'), not influenced by snapshot data or user input.
• [[ ]] string comparisons on existing_ts: uses bash's [[ — no command execution, no word-splitting; a crafted string value cannot inject shell commands.
• guard_ceiling computed once, used across retries: the retry loop runs for at most 0.3 s total (3 × 0.1 s sleep); the 5-minute ceiling does not go stale.
• session_id path containment: validated to ^[A-Za-z0-9_-]+$ before any filesystem operation on both write (L94) and read (L52) sides.
• jq and awk variable bindings: --arg ts "$ts", -v u=, -v s=, -v a= all bind values as typed data, not program text.
• printf '%s' "$INPUT" | "$@" passthrough: "$@" originates from the operator's settings.json, not from stdin data.
• mv -f "$tmp" "$target" rename: on Linux/macOS rename(2) replaces a symlink at $target atomically; a pre-planted symlink at the destination cannot redirect the write.
• No hooks, no MCP, no eval, no outbound network: confirmed clean at HEAD.
• disable-model-invocation: true on setup skill: correct — prevents ambient invocation of the apply write path.

---

Summary

No CRITICAL or IMPORTANT security vulnerabilities exist at HEAD ef0bf085. The HEAD commit closes the one SUGGESTION that was carried through five prior review rounds: the plausibility ceiling (now + 5 minutes) prevents any captured_at value — ISO-8601 or otherwise — from permanently suppressing tee writes. Failure of the ceiling computation disables the guard entirely, failing toward freshness. All 9 previously IMPORTANT findings are confirmed resolved.