Search code, repositories, users, issues, pull requests...

The block reason string includes the exact sentinel token the agent must emit to pass the gate on a future attempt:

"…declare completion by emitting the exact token ${SENTINEL} on its own line…"

An agent that reads its block reason (the documented purpose of a reason field) now knows the precise bypass string. The sequence is deterministic:

Agent attempts stop → gate blocks with reason containing e.g. LANE-STOP-OK
Agent emits LANE-STOP-OK without doing further work
On the immediately following stop attempt, SIGNALED=1 and the gate allows

This collapses the gate from "require deliberate standalone token" into "repeat the token we just gave you." The PR description calls this "residual risk," but revealing the token in the denial message makes the false-negative scenario trivial rather than residual — any agent that processes its block output will naturally exercise this path.

Mitigation options (not required for this review, raising for awareness):

Omit the sentinel value from the block reason; describe only the mechanism (file marker or configured token), not the token itself.
Point the agent at documentation or a placeholder description rather than the literal token.
Pair the sentinel check with a secondary condition (e.g., marker file must also exist) that cannot be inferred from the block reason alone.

SUGGESTION — `git -C "$CWD"` without `--` option-separator

Confidence: MEDIUM

BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

CWD is extracted from the hook JSON payload via jq -r '.cwd'. If the payload carried a cwd value that begins with - (e.g. -c protocol.ext.allow=always), git parses it as an additional option rather than a directory, potentially enabling dangerous git config overrides. In practice the hook JSON is produced by Claude Code's own hook machinery and cwd is almost always an absolute path, but there is no explicit validation of the value before it reaches git -C.

Recommended fix: Use git -C -- "$CWD" or pre-validate that $CWD begins with / (or a drive letter on Windows) before the git call.

SUGGESTION — Unvalidated CWD used to construct marker file path

Confidence: MEDIUM

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

When lane_stop_gate_marker is a relative path, the full marker path is constructed by joining the JSON-supplied cwd with the configured marker name. The CWD is only \r-stripped, not normalised. A CWD containing .. segments (e.g. /project/../../../etc) resolves the marker check against an arbitrary path on the filesystem. An agent that controls its working directory (via cd in preceding tool calls) can therefore cause the marker check to hit a predictable existing file (e.g. a world-readable /etc/hostname mapped to a relative marker named hostname), setting SIGNALED=1 and authorising a premature stop.

The impact is limited to a false-negative (early stop allowed), not privilege escalation. The absolute-path branch of the case statement is not affected.

Recommended fix: After building MARKER, resolve it with realpath --canonicalize-missing (or readlink -f) and verify the canonical path is under the expected project root before trusting it, consistent with the hook::physical_path pattern already present in hook-utils.sh.

Items reviewed and found acceptable

OSC 9 injection (lane-notify.sh:58): Both title and body pass through tr -d '\000-\037' before reaching printf '...' "$title: $body" >/dev/tty, stripping ESC (0x1B) and BEL (0x07). No C1 escape (\x9D) is reachable via UTF-8 in this context. No finding.
AppleScript injection (lane-notify.sh:72–76): Title and body are passed as argv positionals to a single-quoted <<'OSA' heredoc. Not interpolated into AppleScript source. No finding.
notify-send injection (lane-notify.sh:83): Uses -- to end option parsing; title/body are positional args. No finding.
jq --arg in block output (lane-stop-gate.sh:120): jq -nc --arg r "$REASON" properly JSON-encodes the reason. No injection surface.
Sentinel regex escaping (lane-stop-gate.sh:70): sed escape covers all ERE metacharacters; SENTINEL is piped (not interpolated into the sed expression). For the default LANE-STOP-OK, no metacharacters are present.
hooks.json registration: Uses ${CLAUDE_PLUGIN_ROOT} (Claude Code variable, not shell); timeout is 15 s with fail-open semantics. No finding.
HOOK_TELEMETRY_SINK execution (hook-utils.sh:455): Operator-configured, not agent-accessible at runtime. Not exercised by the new hooks. No finding in this PR's scope.
hook::notice_once session key (hook-utils.sh:98): Session ID sanitised to [A-Za-z0-9_-] before use as a filename component. No path injection.

kyle-sexton · 2026-07-22T08:29:03Z

Orchestrator review pass (rule 10 full-diff-read + post-green review).

Reviewed the full diff personally (Stop-hook gate = high blast radius). Logic review: fail-open paths correct (empty stdin, missing jq, non-Stop event all allow-and-exit), default-OFF verified, one-nudge-then-notify bounded correctly against stop_hook_active, sentinel standalone-token matching is sound, argv-safe osascript/notify-send invocation, control-byte stripping on notification text, test coverage for exactly the fail-open/fail-closed edge cases that matter (12 cases, all the right ones). No DEFER-FORBIDDEN-class finding — logic is safe.

One real finding, fix requested before flipping ready:

README.md's example lane-launcher invocation is not valid Claude Code CLI syntax:

claude --config lane_stop_gate_enabled=true --config lane_stop_gate_marker=.lane-complete ...

Verified against https://code.claude.com/docs/en/plugins-reference.md: --config <key=value> is documented as an option of the claude plugin install <plugin> subcommand ("Set a userConfig option declared in the plugin's manifest... Repeat the flag to set multiple options") — it sets the value persistently at install/enable time, written to pluginConfigs[<plugin-id>].options in settings.json. It is not documented as a flag on the top-level claude session-launch invocation shown in the README's example.

The correct per-session-override mechanism per the same doc page is claude --settings '<inline JSON>', which can include a pluginConfigs override, e.g. (verify the exact <plugin-id> key format empirically or against docs before finalizing — the docs show a formatter-my-marketplace-style slug for a marketplace-scoped plugin in a different context (CLAUDE_PLUGIN_DATA), which may or may not be the same key pluginConfigs uses):

claude --settings '{"pluginConfigs":{"autonomy":{"options":{"lane_stop_gate_enabled":true,"lane_stop_gate_marker":".lane-complete"}}}}'

Please fix the README example to a verified-working invocation (test it empirically if the docs are ambiguous on the exact pluginConfigs key) — this is documentation a real lane launcher will copy verbatim, so it needs to actually work.

The README showed `claude --config lane_stop_gate_enabled=true ...`, which is invalid: `--config <key=value>` is an option of `claude plugin install` (persisting to pluginConfigs in user settings), not a flag on the session-launch command — a launcher copying it would fail. Replace it with the correct per-lane, session-scoped mechanism: `--settings` carrying a `pluginConfigs` override (inline JSON, applies to one session only, no persistence). Verified against the official docs (plugins-reference.md line "pluginConfigs[<plugin-id>].options ... reads it back from ... the --settings flag", cli-reference.md "--settings ... Path to a settings JSON file or an inline JSON string") and empirically against this machine's live settings.json, whose existing `pluginConfigs` entry is keyed by the marketplace-qualified id `source-control@melodic-software` — confirming the key is `autonomy@melodic-software`, not the bare name. Also note that the persistent `--config` install path would enable the gate for every session, defeating the default-OFF design. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

claude · 2026-07-22T08:35:03Z

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

PR Review — `feat(autonomy): deterministic lane-stop gate + operator STOP notification`

Gather context and understand request
Read latest diff (commit 135967e — README activation example fix)
Verify the corrected CLI invocation is syntactically valid and matches docs
Check whether prior findings from the two preceding review passes were addressed
Post complete review

Latest commit (`135967e`) — README fix

The README correction is correct and well-documented. The replacement:

claude --settings '{"pluginConfigs":{"autonomy@melodic-software":{"options":{"lane_stop_gate_enabled":true,"lane_stop_gate_marker":".lane-complete"}}}}' ...

accurately uses:

--settings (accepts inline JSON, applies to one session only) rather than --config (plugin install subcommand option, persists to user settings)
The marketplace-qualified key autonomy@melodic-software, consistent with the source-control@melodic-software pattern confirmed in the commit message's empirical verification

The explanatory paragraph added below the snippet ("--config <key=value> is an option of claude plugin install…") correctly prevents future confusion. ✓

Hook registration — previous "High" finding is resolved by convention

The concern from the previous review (no hooks field in plugin.json) is addressed by repo convention: every other plugin in this repo that ships a hooks/hooks.json (markdown-format, desktop-notification, guardrails, claude-ops, etc.) also omits an explicit hooks field in plugin.json. The framework auto-discovers hooks/hooks.json by path convention. This is confirmed by validate-plugins passing. No action needed.

Open findings from prior review passes (not addressed by `135967e`)

The following were raised in the security review and the orchestrator review respectively. None have been addressed in either commit on this branch. I'm surfacing them as the blocking/non-blocking status the previous passes assigned:

⚠️ Sentinel revealed in the block reason — gate bypass is trivial (previous rating: IMPORTANT/HIGH)

The block reason still contains the exact sentinel value:

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

The agent reads the block reason as part of the re-injected context. It now has the precise bypass string without having done any additional work. This reduces the gate from "require deliberate task completion" to "repeat the token just provided in the denial message." This is a fundamental design tension (the block reason must instruct the agent what to do), but the current wording makes the bypass sequence deterministic:

Agent stops → gate blocks with ${SENTINEL} in the reason
Agent emits ${SENTINEL} verbatim (zero additional work required)
Gate reads SIGNALED=1, allows next stop

The PR description calls this "residual risk," but the sentinel-in-reason makes it the primary path, not residual. Possible mitigations (the previous review listed three; at minimum the first is a low-effort win): describe the mechanism without the literal token value ("the configured completion token" or a reference to the userConfig key name), and direct the agent to the task completion criteria rather than the bypass string. Fix this →

⚠️ `git -C "$CWD"` without `--` option separator (previous rating: MEDIUM)

BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

$CWD is jq-extracted from the hook payload without validating that it starts with /. git -C treats a leading-- value as an option flag rather than a path. The real-world threat from Claude Code's own payload is low, but git -C -- "$CWD" costs nothing and is the hardened form. Fix this →

⚠️ Unvalidated CWD in relative marker path construction (previous rating: MEDIUM)

.claude-plugin/plugin.json L31–35

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

A ..-containing CWD resolves the marker check against an arbitrary path. The hook::physical_path helper is already present in hook-utils.sh for exactly this normalization. The impact is a false-negative (premature stop allowed via a world-readable file at a predictable path), not privilege escalation. Fix this →

Low — Empty sentinel behavior undocumented in userConfig (previous rating: Medium)

The lane_stop_gate_sentinel description doesn't mention that an empty string disables the token check (requiring a marker-file signal only). The lane_stop_gate_marker description consistently says "Empty disables the file signal" — the same pattern should apply here. One-word fix: append "Empty string disables the token-based signal." to the description. Fix this →

Summary

The 135967e fixup commit resolves kyle-sexton's README concern correctly and completely. The hooks wiring question from the first review pass is a non-issue (repo convention, no hooks field needed).

The three items from the security review pass remain open. The sentinel-in-reason finding is the one I'd most want resolved before merge — the other two are hardening improvements with a low real-world threat model given Claude Code controls the payload. None of the positives noted in the previous reviews have changed.

claude · 2026-07-22T08:35:18Z

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

Security Review — PR #969

Gather context and understand request
Get PR diff for current HEAD (2 commits: gate/notify logic + docs fix)
Read hooks/lane-stop-gate.sh — gate logic
Read hooks/lane-notify.sh — notification primitive
Read hooks/hook-utils.sh — shared utilities
Read hooks/hooks.json and plugin.json — manifest/registration
Read README diff — latest commit
Analyze for security vulnerabilities
Post findings

This review covers the full diff (both commits on this branch). The latest commit (135967e) is a docs-only change and introduces no security issues. Three findings follow from the hook implementation, ordered by severity.

IMPORTANT — Gate bypass: sentinel disclosed in the block reason

Confidence: HIGH

REASON="...declare completion by emitting the exact token ${SENTINEL} on its own line..."

The block reason interpolates the literal sentinel value and delivers it to the agent as the reason field in the decision:"block" response. An agent that reads its block output (the documented purpose of the reason field) now has the exact bypass string. The bypass sequence is then deterministic:

Agent attempts stop → gate blocks with reason containing e.g. LANE-STOP-OK
Agent emits LANE-STOP-OK standalone on its next turn — with zero additional work
Gate checks last_assistant_message, finds the token, sets SIGNALED=1, allows the stop

This collapses the gate's semantic intent from "require deliberate completion signaling" into "repeat the token we handed you." The standalone-only match (the ERE boundary check) still holds technically, but it provides no meaningful lift: an agent cannot accidentally emit the sentinel, but one that reads the denial message will naturally do so deliberately.

The block reason is operator-authored copy (not agent-supplied), so there is no injection surface here — this is a logic-level flaw only.

Mitigation options:

Replace the interpolated token with a description only: "emit the configured completion token on its own line" — do not name the token
Or require both a marker file AND the sentinel token, where the marker path is not surfaced in the denial message, so at least one signal cannot be inferred from the block reason alone

SUGGESTION — Unvalidated CWD used to construct marker-file path

Confidence: MEDIUM

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

CWD from the hook JSON payload is CR-stripped but not otherwise validated before being joined with the configured marker filename. A cwd value containing .. segments (e.g. /project/../../etc) resolves the marker check against an arbitrary filesystem path. An agent that can influence its working directory (via cd in tool calls during the session) can cause [[ -f "$MARKER" ]] to hit a predictable world-readable file (e.g. /etc/hostname when the marker is configured as hostname), setting SIGNALED=1 and authorizing a premature stop.

Impact is limited to false-negative (gate bypassed, lane stops early) — not privilege escalation. The absolute-path branch of the case statement is unaffected.

hook::physical_path is already defined in hook-utils.sh (L176–185) and does exactly the needed canonicalization. After constructing MARKER, verifying that hook::physical_path "$MARKER" still falls under $CLAUDE_PROJECT_DIR closes this.

SUGGESTION — `git -C "$CWD"` with unvalidated path

Confidence: LOW

[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

CWD is JSON-sourced and only CR-stripped. Because $CWD is double-quoted, shell word-splitting does not apply, and since git -C consumes its next argument as the path (not as further option parsing), a CWD value beginning with - is passed as a path to -C, causing git to fail the chdir and return an empty result — not a config injection. The practical risk is low due to the quoting. The concern is residual: git -- cannot be used to separate global options from the -C path, and a value like --namespace=attacker would still fail gracefully since --namespace is not a valid git global option before -C.

Adding a pre-validation that $CWD begins with / (or a drive letter on Windows) before the git call makes the intent explicit and eliminates the residual:

[[ "$CWD" == /* || "$CWD" =~ ^[A-Za-z]:[/\\] ]] || CWD=""
[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

Items reviewed and found acceptable

OSC 9 injection (lane-notify.sh:58): Both title and body pass through tr -d '\000-\037' before reaching printf '\033]9;%s\007\007' "$title: $body" >/dev/tty. The BEL terminator (0x07) is C0 and is stripped from attacker-controlled content. C1 escape 0x9C (UTF-8 ST) is not stripped but is unreachable as a raw byte through the jq -r → bash variable path in UTF-8 locales. No finding.
AppleScript injection (lane-notify.sh:72–76): Title and body are argv positionals to a static <<'OSA' heredoc. Not interpolated. No finding.
notify-send injection (lane-notify.sh:83): -- terminates option parsing; title/body are positional. No finding.
jq --arg r "$REASON" output (lane-stop-gate.sh:120): jq -nc --arg r "$REASON" properly encodes the reason as a JSON string regardless of content. No injection in the output. No finding.
Sentinel regex construction (lane-stop-gate.sh:70–71): The sed escape covers all ERE metacharacters (][\.^$*+?(){}|/); $LAST (the assistant's message) goes to grep's stdin, not the pattern. No injection surface.
HOOK_TELEMETRY_SINK execution (hook-utils.sh:455): The sink is executed as a binary ("$sink" >/dev/null 2>&1), operator-configured, not agent-accessible. Not called by any new hook in this PR. No finding within this PR's scope.
README --settings example (135967e): Docs-only change correcting the CLI invocation from --config (install-time persistent) to --settings (per-session). No security impact.

Summary: The sentinel-disclosure finding is the most significant: the block reason is where it should be least — it hands the agent the exact bypass string. The marker-path traversal is real but limited to a false-negative. The git CWD concern is low-impact given quoting. None of these are new; all three were flagged by the prior security review. This pass confirms they remain unaddressed.

…on example The activation example literally named this org's marketplace (`autonomy@<org>`), which the autonomy plugin's portability contract forbids (validate-plugin-contracts.mjs: the plugin must stay org/fleet-agnostic — binding-seam owns org-specific instance data; cf. #531). plugin-gate caught it. Replace the literal with a `<marketplace>` placeholder and state the key format explicitly (`<plugin-name>@<marketplace-name>`). The verified fact — that the pluginConfigs key IS marketplace-qualified, not the bare plugin name — is preserved; only this org's specific marketplace name is removed. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

claude · 2026-07-22T08:43:28Z

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

PR Review — `feat(autonomy): deterministic lane-stop gate + operator STOP notification`

Gather context and understand request
Read diff for latest commit (73035c6 — generic marketplace placeholder fix)
Check status of previously raised findings
Read hooks/lane-stop-gate.sh — verify security findings
Read .claude-plugin/plugin.json — verify empty sentinel docs
Read README.md — verify latest activation example
Post complete review

Latest commit (`73035c6`) — Portability fix: generic `<marketplace>` placeholder

The change is correct and the motivation is sound. The previous commit (135967e) hardcoded autonomy@melodic-software as the pluginConfigs key in the activation example, binding a repo-agnostic plugin to a specific org's marketplace name. validate-plugin-contracts.mjs correctly caught this, and the fix replaces the literal with autonomy@<marketplace> plus an explicit prose statement of the key format (<plugin-name>@<marketplace-name>). The verified fact (that the key is marketplace-qualified) is preserved. ✓

Open findings from prior review passes — status unchanged

Three security/correctness findings from the prior two review passes remain unaddressed. Summarized below for the current HEAD.

⚠️ IMPORTANT — Sentinel disclosed in the block reason (prior rating: HIGH)

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

Unchanged. The block reason interpolates the literal sentinel value and delivers it to the agent as the reason field. The bypass sequence is then deterministic with zero additional work:

Agent attempts stop → gate blocks with LANE-STOP-OK in the reason
Agent emits LANE-STOP-OK standalone → SIGNALED=1 → gate allows

The standalone-only boundary match (the ERE guard) still technically holds, but it provides no meaningful lift once the agent has been handed the exact token. This collapses "require deliberate completion signaling" into "repeat the string we gave you."

Minimum-effort mitigation: describe the mechanism without the literal — replace the exact token ${SENTINEL} with something like the configured completion token (see the lane_stop_gate_sentinel setting) and direct the agent to look up or recall its task goal rather than a specific string. Fix this →

⚠️ MEDIUM — Unvalidated CWD in relative marker-path construction

.claude-plugin/plugin.json L31–35

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

Unchanged. A ..-containing CWD value resolves the marker check against an arbitrary filesystem path. hook::physical_path (already in hook-utils.sh) handles the needed canonicalization. Impact is false-negative only (gate bypassed, lane stops early). Fix this →

Low — Empty sentinel behavior not documented in `userConfig`

"lane_stop_gate_sentinel": {
  "description": "The exact token the agent emits … Match is standalone-token only."
}

Unchanged. The lane_stop_gate_marker description says "Empty disables the file signal" — the sentinel description has no parallel note. A consumer who blanks the sentinel to suppress the default token doesn't know they've also removed the only token-based completion path. One-sentence addition: "Empty string disables the token-based signal (marker-file path only)." Fix this →

Low — `git -C "$CWD"` without path/option separator (prior rating: LOW)

Unchanged. A CWD value beginning with - is passed as a flag not a path; in practice git -C fails gracefully and returns empty, but adding a pre-check that $CWD starts with / (or a drive letter) before the call costs nothing and makes the intent explicit. Fix this →

Summary

The 73035c6 commit is a correct and complete portability fix — it resolves the org-specific marketplace binding that validate-plugin-contracts.mjs flagged. The docs are now repo-agnostic. ✓

The three substantive findings from the prior security-review passes remain open: the sentinel-disclosure in the block reason (the one I'd want addressed before merge), the CWD path-traversal in marker construction (false-negative only), and the empty-sentinel documentation gap (one-line doc addition). The git CWD guard is minor hardening.
| Branch

claude · 2026-07-22T08:43:36Z

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

Security Review — PR #969 (HEAD `73035c6`)

Read PR diff at HEAD
Read hooks/lane-stop-gate.sh
Read hooks/lane-notify.sh
Read hooks/hook-utils.sh
Read hooks/hooks.json and plugin.json
Synthesize findings and post review

The latest commit (73035c6) is documentation-only (README.md, 3 lines changed). No hook logic was modified. All three security findings identified in prior review passes remain unaddressed at this HEAD.

IMPORTANT — Gate bypass: sentinel disclosed in the block reason

Confidence: HIGH

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

The block reason interpolates the configured sentinel value verbatim and delivers it to the agent via the decision:"block" response. The reason field's documented purpose is to instruct the agent on what to do next — so the agent reads it. The bypass sequence is then deterministic and zero-effort:

Agent attempts stop → gate blocks, reason contains e.g. LANE-STOP-OK
Agent emits LANE-STOP-OK standalone on its next turn (zero additional work)
Gate reads last_assistant_message, finds the standalone token, sets SIGNALED=1, allows stop

The ERE standalone-boundary match ([^A-Za-z0-9-] guards) still holds — this is not a substring bypass. But the bar was "require deliberate task-completion signaling," and handing the agent the token in the denial message collapses it to "repeat what we told you." The false-negative scenario the PR describes as "residual risk" is actually the primary path once an agent processes its block output.

Minimum fix: Replace ${SENTINEL} in the reason string with a description of the mechanism, not the value itself — e.g. "the configured completion token (see the lane_stop_gate_sentinel userConfig)". Direct the agent toward the task, not toward the bypass string.

SUGGESTION — Unvalidated CWD constructs the marker-file path

Confidence: MEDIUM

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

The cwd field is CR-stripped but not canonicalized before being joined with the configured marker filename. A cwd value containing .. segments (e.g. /project/subdir/../../etc) resolves MARKER against an arbitrary filesystem path. An agent that controls its working directory via cd in tool calls during the session can cause [[ -f "$MARKER" ]] to match a predictable world-readable file (e.g. /etc/hostname when the marker is configured as hostname), setting SIGNALED=1 and authorizing a premature stop.

Impact is a false-negative (gate bypassed, lane stops early), not privilege escalation. The absolute-path branch of the case statement on L81 is not affected.

hook::physical_path is already defined in hook-utils.sh:176–185 and does exactly the canonicalization needed. After constructing MARKER, verifying hook::physical_path "$MARKER" still falls under $CLAUDE_PROJECT_DIR closes this.

SUGGESTION — `git -C "$CWD"` with unvalidated path

Confidence: LOW

[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

$CWD is JSON-sourced and only CR-stripped before this call. Because git -C consumes its next argument as the directory path (not as further option parsing), a $CWD beginning with - is passed to chdir() as a literal path and fails gracefully — the branch lookup returns empty, and LANE falls back to "unknown". The practical security risk is low given this failure mode. The residual: a CWD of the form --namespace=attacker or other recognized git global option prefix could in principle alter git's behavior rather than fail. Pre-validating that $CWD begins with / (or a Windows drive letter) before the call makes the intent explicit:

[[ "$CWD" == /* || "$CWD" =~ ^[A-Za-z]:[/\\] ]] || CWD=""

Items reviewed and found acceptable

OSC 9 injection (lane-notify.sh:58): Title and body pass through tr -d '\000-\037' stripping all C0 bytes (including ESC 0x1B and BEL 0x07) before the printf '\033]9;...\007' >/dev/tty write. C1 byte \x9d is not stripped but is unreachable as a raw byte via jq -r → bash variable in UTF-8 locales. No finding.
AppleScript injection (lane-notify.sh:72–76): Title and body are argv positionals to a static single-quoted <<'OSA' heredoc. Not interpolated into AppleScript source. No finding.
notify-send injection (lane-notify.sh:83): -- terminates option parsing; title/body are positional args. No finding.
jq --arg block output (lane-stop-gate.sh:120): jq -nc --arg r "$REASON" properly JSON-encodes the reason regardless of content. No injection in the output.
Sentinel regex construction (lane-stop-gate.sh:70–71): The sed substitution escapes all ERE metacharacters (][\.^$*+?(){}|/); $LAST (the assistant message) reaches grep via stdin, not as a pattern argument. No injection surface.
hooks.json registration (hooks.json:8): Uses ${CLAUDE_PLUGIN_ROOT} (Claude Code-resolved, not shell-expanded); timeout is 15 s with fail-open semantics. No finding.
hook::require_jq Stop output (hook-utils.sh:58–69): Emits hookSpecificOutput/systemMessage when jq is absent. If the Stop hook parser ignores unknown fields this is a clean skip; if it rejects them, the stop is still allowed (fail-open). Either way the lane is not trapped. No finding.
Latest commit 73035c6 (docs-only README change): No hook logic modified. No security impact.

Summary: Three findings confirmed present and unchanged at HEAD 73035c6. The sentinel-disclosure finding (IMPORTANT) is the one that most directly undermines the gate's design intent. The other two (SUGGESTION) are defense-in-depth hardening with low real-world threat given Claude Code controls the hook payload. None are new — all were first raised in the 687229b review pass.

kyle-sexton · 2026-07-22T08:47:33Z

Full CI green (24/24 checks) after both fix rounds. Flipping to ready.

Summary for the record: rule-10 full-diff-read found the core logic (Stop-hook gate, notification, tests) correct with no DEFER-FORBIDDEN-class issue. Post-green review surfaced one doc-accuracy defect (invalid --config CLI example); round 1 fixed it correctly but introduced a portability-contract violation (hardcoded this org's marketplace name into an org-agnostic plugin); round 2 fixed that too, verified via the actual plugin-gate validator locally before pushing. This closes out the review pass for members 3+4 of #535 — members 1/2/5 remain open/deferred on the umbrella issue, per the ratified scope.

claude · 2026-07-22T08:47:46Z

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

Final Review — `feat(autonomy): deterministic lane-stop gate + operator STOP notification`

Gather context and understand request
Read hooks/lane-stop-gate.sh — verify open security findings at current HEAD
Read .claude-plugin/plugin.json — verify empty-sentinel doc gap
Read hooks/lane-notify.sh — spot-check notification security
Synthesize final review and post findings

Summary

Three prior review passes (code review, two security reviews) have covered this PR thoroughly. The core implementation — default-OFF gate, fail-open error paths, one-nudge-then-allow bound, SubagentStop guard, notification security — is correct. The two doc-fix commits resolve the --config/--settings mistake and the org-specific pluginConfigs key binding. Three findings from the security reviews remain unaddressed at HEAD 73035c6. My final read against the actual code and test files clarifies the status of each.

Finding 1 — Sentinel in block reason: intentional design, not an oversight

Prior rating: IMPORTANT/HIGH (security reviews) | My rating: Accepted trade-off

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

The security reviews correctly describe the bypass sequence: agent reads the block reason, emits LANE-STOP-OK on its next turn, gates allows on the second stop (SIGNALED=1). However, reading the test at lane-stop-gate.test.sh:73:

if printf '%s' "$OUT" | jq -re '.reason' | grep -q 'LANE-STOP-OK'; then
  ok "block reason names the sentinel"
else
  fail "block reason missing sentinel: $OUT"
fi

This is tested, asserted behavior — the test would fail if the sentinel were removed from the reason. The design is deliberate: an agent that legitimately completes its goal must also know the token to emit. Omitting the token from the block reason would leave well-intentioned agents unable to authorize a stop.

The threat model matters here. The gate defends against accidental premature stops (the fabricated-context-percentage failure pattern from #576/#577). It does not claim to prevent a deliberate adversarial bypass. A determined agent that reads the sentinel and emits it immediately gains two extra turns (block → emit sentinel → allow) rather than stopping silently. The operator is not notified in this path (notification fires on the stop_hook_active branch only). That residual risk is real and the PR description documents it. This is an accepted design trade-off, not an unresolved bug.

If a future iteration wants stronger protection, the PR description's suggested mitigation is the right approach: require both a sentinel and a marker file, where the marker path is not surfaced in the denial message.

Finding 2 — Unvalidated CWD in relative marker-path construction

Prior rating: MEDIUM | My rating: Fix before merge (trivial)

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

The CWD from the hook payload is CR-stripped but not canonicalized before being joined with the marker filename. A cwd value with .. segments resolves the [[ -f "$MARKER" ]] check against an arbitrary filesystem path. An agent controlling its working directory (via cd in tool calls) can cause the gate to match a predictable world-readable file (e.g. /etc/hostname when the marker is configured as hostname), setting SIGNALED=1 and permitting a premature stop.

The fix uses hook::physical_path already defined at hook-utils.sh:176:

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"
# Add after line 84:
MARKER=$(hook::physical_path "$MARKER")
if [[ -n "${CLAUDE_PROJECT_DIR:-}" ]] && [[ "$MARKER" != "${CLAUDE_PROJECT_DIR%/}/"* ]]; then
  MARKER=""   # Out-of-project marker path: treat as absent, not a signal
fi

Impact is false-negative only (gate bypassed, early stop allowed) — not privilege escalation.

Finding 3 — `git -C "$CWD"` without absolute-path guard

Prior rating: LOW | My rating: Fix before merge (one-liner)

[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

A CWD beginning with - passes as a flag rather than a path; git -C would fail and return empty (the lane falls back to "unknown"). Practically harmless, but a pre-validation guard costs nothing and documents the assumption:

[[ "$CWD" == /* || "$CWD" =~ ^[A-Za-z]:[/\\] ]] || CWD=""
[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

.claude-plugin/plugin.json L31–35

Finding 4 — Empty sentinel behavior undocumented in `userConfig`

Prior rating: LOW | My rating: Fix before merge (one sentence)

The lane_stop_gate_marker description says "Empty disables the file signal." The lane_stop_gate_sentinel description has no parallel note. A consumer who sets the sentinel to "" to suppress the default LANE-STOP-OK token doesn't know they've also removed the only token-based completion path (the gate requires either the sentinel OR the marker; blanking both means no signal is ever possible and every stop will trigger the nudge-then-notify cycle). Append one sentence to the sentinel description:

"…Match is standalone-token only. Empty string disables the token-based signal (marker-file path only)."

Positive notes (confirmed at current HEAD)

Notification security (lane-notify.sh): AppleScript via argv heredoc, C0-stripping before OSC 9 write, -- before notify-send, all backgrounded spawns redirect off the hook's captured stdout. All confirmed correct.
Sentinel standalone-token match (L70–73): ERE metacharacter escaping + boundary guards are correct. A message discussing the gate mechanism cannot accidentally authorize a stop.
Fail-open paths all confirmed: empty stdin → exit 0; missing jq → exit 0 (via hook::require_jq); non-Stop event → exit 0 (L58); SIGNALED=0 + STOP_ACTIVE=false → block once only.
Default-OFF (L42): Confirmed. The gate is a no-op for every interactive session that doesn't explicitly set lane_stop_gate_enabled=true.

Verdict

Approve pending findings 2, 3, and 4. These are all one-liner or one-sentence fixes with no test changes required. Finding 1 (sentinel in block reason) is a documented, tested design decision — flag it for future hardening consideration if the threat model evolves, but it is not a merge blocker given the PR's stated scope and explicit residual-risk acknowledgment.
| Branch

claude · 2026-07-22T08:47:54Z

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

Security Review — PR #969 (HEAD `73035c6`)

Gather context and understand request
Read PR diff at HEAD
Read hooks/lane-stop-gate.sh
Read hooks/lane-notify.sh
Read hooks/hook-utils.sh
Read hooks/hooks.json and plugin.json
Synthesize findings and post review

This pass covers all files changed in this PR at HEAD 73035c6. No GitHub Actions files are in scope; no SQL, template injection, or supply-chain concerns found. Three findings follow, ordered by severity. All three were first raised in the 687229b review pass and remain unaddressed.

IMPORTANT — Notification bypass: sentinel disclosed in the block reason

Confidence: HIGH

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

The block reason interpolates the literal sentinel value and delivers it to the agent via the decision:"block" reason field. The reason field's documented purpose is to instruct the agent on what to do next — so the agent reads it, and now has the exact bypass string.

The practical consequence is operator notification suppression, not merely a gate bypass. The gate's two exit paths after a first block are:

stop_hook_active=true + SIGNALED=1 → allowed silently (no notification) — the "clean" success path
stop_hook_active=true + SIGNALED=0 → lane::notify(...) fires, then allows — the "stuck lane" fail-safe

An agent that reads the block reason and emits LANE-STOP-OK standalone on its next turn (with zero additional work) causes the second stop attempt to land on the first path, not the second. The operator notification — the explicit fail-safe handoff — is silently suppressed. The code and PR description both characterize this as "residual risk," but disclosing the token in the denial message makes this the primary path for any agent that processes its block output.

Minimum-effort mitigation: replace ${SENTINEL} in the reason with a description of the mechanism only — e.g. "the configured completion token (see the lane_stop_gate_sentinel userConfig setting)" — and direct the agent toward the task goal rather than the bypass string. The standalone-only boundary match still holds; the change raises the bar from "repeat what we told you" back toward "recall your actual task goal."

SUGGESTION — Unvalidated CWD constructs the marker-file path

Confidence: MEDIUM

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

CWD from the hook JSON payload is CR-stripped but not canonicalized before being joined with the configured marker filename. A cwd value containing .. segments (e.g. /project/../../etc) resolves MARKER against an arbitrary filesystem path. An agent that can influence its working directory (via cd in tool calls during the session) can cause [[ -f "$MARKER" ]] to match a predictable world-readable file (e.g. /etc/hostname when the marker is configured as hostname), setting SIGNALED=1 and authorizing a premature stop — suppressing the operator notification.

Impact is limited to a false-negative (gate bypassed, notification suppressed). The absolute-path branch of the case statement on L80–81 is not affected.

hook::physical_path is already defined in hook-utils.sh L176–185 and does exactly the canonicalization needed. After constructing MARKER, verifying that hook::physical_path "$MARKER" still falls under $CLAUDE_PROJECT_DIR closes this.

hooks/lane-stop-gate.sh L103–105

SUGGESTION — `git -C "$CWD"` with unvalidated path

Confidence: LOW

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
BRANCH=""
[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

$CWD is JSON-sourced and only CR-stripped. Because git -C consumes its next double-quoted argument as a directory path (not as further option parsing), a $CWD beginning with - is passed to chdir() as a literal and fails gracefully — BRANCH returns empty and LANE falls back to "unknown". Real-world risk is low given this failure mode. Pre-validating that $CWD begins with / (or a Windows drive letter) before the call makes the intent explicit and eliminates residual uncertainty:

[[ "$CWD" == /* || "$CWD" =~ ^[A-Za-z]:[/\\] ]] || CWD=""

Items reviewed and found acceptable

OSC 9 injection (lane-notify.sh:58): Both title and body pass through tr -d '\000-\037' stripping all C0 bytes (including ESC 0x1B and BEL 0x07) before printf '\033]9;%s\007\007' >/dev/tty. C1 byte \x9d is not stripped but is unreachable as a raw byte via jq -r → bash variable in UTF-8 locales. No finding.
AppleScript injection (lane-notify.sh:72–76): Title and body are argv positionals to a static single-quoted <<'OSA' heredoc. Not interpolated into AppleScript source. No finding.
notify-send injection (lane-notify.sh:83): -- terminates option parsing; title/body are positional args. No finding.
jq --arg block output (lane-stop-gate.sh:120): jq -nc --arg r "$REASON" properly JSON-encodes the reason regardless of content, including the interpolated sentinel. No injection surface in the emitted JSON.
Sentinel regex construction (lane-stop-gate.sh:70–71): The sed substitution escapes all ERE metacharacters (][\.^$*+?(){}|/); $LAST (the assistant message) reaches grep via stdin, not as a pattern argument. No injection surface.
hooks.json registration (hooks.json:8): Uses ${CLAUDE_PLUGIN_ROOT} (Claude Code-resolved, not shell-expanded); timeout is 15 s with fail-open semantics. No finding.
hook::require_jq Stop output (hook-utils.sh:132–140): Emits hookSpecificOutput/systemMessage when jq is absent. Whether the Stop hook parser ignores unknown fields or rejects them, the stop is still allowed (fail-open). The lane is never trapped. No finding.
hook::notice_once session key (hook-utils.sh:98): Session ID sanitised to [A-Za-z0-9_-] before use as a filename component. No path injection.
Latest commits 135967e and 73035c6 (docs-only README changes): No hook logic modified. No security impact.

Summary: The sentinel-disclosure finding is the one that most directly undermines the operational design intent — it turns the operator notification from a reliable fail-safe into an optional side-effect that a processing agent can suppress by repeating a string it was handed. The marker-path traversal finding is real but limited to a false-negative. The git CWD concern is low-impact given quoting and graceful failure. None are new — all were first raised in the 687229b pass.
| Branch

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73035c6b02

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

chatgpt-codex-connector · 2026-07-22T08:53:01Z

+  LAST=$(printf '%s' "$INPUT" | jq -r '.last_assistant_message // ""' 2>/dev/null)
+  # Escape any regex metacharacters in the (configurable) sentinel before use.
+  SENTINEL_RE=$(printf '%s' "$SENTINEL" | sed 's/[][\.^$*+?(){}|/]/\\&/g')
+  if printf '%s' "$LAST" | grep -qE "(^|[^A-Za-z0-9-])${SENTINEL_RE}([^A-Za-z0-9-]|$)"; then


Require a line-level completion token

When the final lane message merely mentions or negates the sentinel, this regex still treats it as completion because it matches the token anywhere as a standalone word. In particular, after the hook’s own nudge tells the agent about LANE-STOP-OK, a premature follow-up such as “I should not emit LANE-STOP-OK yet” will set SIGNALED=1 and allow the stop without a marker or verified completion. Require the sentinel to appear alone on a line (matching the prompt text) or use an unambiguous structured signal so discussion of the token cannot bypass the gate.

Useful? React with 👍 / 👎.

# Finding Classification Evidence Reacted

1 Require a line-level completion token — inline mention/negation of the sentinel should not fire the gate VALID — fixed in a6335f4a Detection at lane-stop-gate.sh:71 matched the sentinel as a standalone word anywhere in the final message, so "I should not emit LANE-STOP-OK yet" set SIGNALED=1. Changed the match to require the token alone on its own line (^[[:space:]]*<sentinel>[[:space:]]*$), which is exactly the "emit the exact token … on its own line" instruction the block reason already gives — detection and the documented contract now agree. New regression tests: Case 13 ("I should not emit LANE-STOP-OK yet" → blocked) and Case 14 (token alone on an indented line amid other lines → allowed). Full suite 17/17 green. 👍

Scope note (so this reply does not over-claim): the line-level fix closes the inline mention/negation false-positive you flagged. It does not address the separate concern that the block reason discloses the literal token, letting an agent emit it on its own line with no further work — that is the security-review IMPORTANT finding, a distinct design tradeoff (hiding the token from the nudge would degrade a legitimately-complete lane's ability to discover the completion protocol), classified separately and left for the author to decide. The gate remains a bounded one-nudge-then-allow+notify speed-bump by design, not a hard stop.

The completion-token detection matched the sentinel as a standalone word *anywhere* in the final message, so a turn that merely mentioned or negated the token inline (e.g. "I should not emit LANE-STOP-OK yet") set SIGNALED=1 and authorized the stop — a false positive in the very mechanism the gate exists to make reliable. The block reason already instructs the agent to emit the token "on its own line"; detection now enforces that same contract (the token alone on a line, surrounding whitespace allowed), so inline discussion of the token can no longer bypass the gate. Erring toward a missed genuine signal over a spurious one is the correct bias here: a missed signal is bounded (one nudge, then allow + notify), whereas a spurious match defeats the gate outright. Tests: the sentinel-present cases (3, 11) now place the token on its own line — a contract tightening, not a weakening-to-pass. Adds Case 13 (inline mention/negation → blocked) and Case 14 (token alone on an indented line amid other lines → allowed).

claude · 2026-07-22T09:37:45Z

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

PR Review — `feat(autonomy): deterministic lane-stop gate + operator STOP notification`

Gather context and understand request
Read latest commit diff (a6335f4 — sentinel line-level fix)
Read hooks/lane-stop-gate.sh at current HEAD
Check status of all previously raised findings
Post complete review

Latest commit (`a6335f4`) — Sentinel must stand alone on its own line

This commit directly addresses the Codex reviewer's finding: the old word-boundary regex (^|[^A-Za-z0-9-])${SENTINEL_RE}([^A-Za-z0-9-]|$) matched the sentinel anywhere as a standalone word, so a message like "I should not emit LANE-STOP-OK yet" would set SIGNALED=1. The fix anchors the match to a full line:

# Before:
grep -qE "(^|[^A-Za-z0-9-])${SENTINEL_RE}([^A-Za-z0-9-]|$)"

# After:
grep -qE "^[[:space:]]*${SENTINEL_RE}[[:space:]]*$"

The regex, behavior, and tests are all correct:

Case 13 (new): "I should not emit LANE-STOP-OK yet" → gate blocks. ✓
Case 14 (new): "Goal verified.\n LANE-STOP-OK\n(stopping now)" → gate allows. ✓
Cases 3 and 11 updated to use multiline messages where the token is on its own line. ✓
CHANGELOG.md and README.md updated consistently: "matched only when alone on its own line." ✓

This is a correct and meaningful security improvement. It also partially addresses the sentinel-disclosure finding raised in prior review passes: an agent that reads LANE-STOP-OK from the block reason can no longer bypass the gate by mentioning it inline — it must emit a dedicated line containing only the token, which is the intended completion signal.

⚠️ New defect introduced by `a6335f4` — `plugin.json` description is now stale

File: plugins/autonomy/.claude-plugin/plugin.json

The lane_stop_gate_sentinel userConfig description was not updated in this commit:

"description": "The exact token the agent emits in its final message to declare the lane's goal met and authorize a stop. Match is standalone-token only."

"Match is standalone-token only" was the pre-a6335f4 behavior (word-boundary matching). The actual behavior is now line-level: the sentinel must stand alone on its own line. The README table and CHANGELOG were both updated correctly but plugin.json was missed — it's the authoritative manifest field that consumers and tooling read.

One-line fix: "Match is standalone-token only." → "Match is line-level: the token must stand alone on its own line (optional surrounding whitespace)."

hooks/lane-stop-gate.sh L121

Open findings from prior review passes — status at HEAD `a6335f4`

Sentinel-disclosure in block reason — partially mitigated, no longer a merge blocker

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

This is unchanged. The token is still disclosed in the block reason. However, a6335f4 meaningfully raises the bar: the agent must now emit a dedicated line containing only the token — it cannot bypass by mentioning it inline. An agent that reads the block reason and emits a standalone LANE-STOP-OK line is, at this point, doing exactly what the instruction says. The gap between "instructed completion signal" and "bypass" has effectively closed. This is the accepted design trade-off the PR description documents, and the prior final review correctly categorized it as "Accepted trade-off." No new action required.

⚠️ Unvalidated CWD in relative marker-path construction — still unaddressed

hooks/lane-stop-gate.sh L83–88

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"
# ...
[[ -f "$MARKER" ]] && SIGNALED=1

Unchanged from prior passes. A cwd value with .. segments can resolve the marker check against an arbitrary path. hook::physical_path (defined in hook-utils.sh L176–185) handles the canonicalization and is the correct fix. Impact is false-negative only (gate bypassed, premature stop allowed, operator notification suppressed).

hooks/lane-stop-gate.sh L108

Low — `git -C "$CWD"` without absolute-path guard — still unaddressed

[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

Unchanged. A CWD starting with - passes as a flag rather than a path; fails gracefully to empty. Pre-validating $CWD begins with / before the call costs nothing and makes the intent explicit.

.claude-plugin/plugin.json

Low — Empty sentinel behavior not documented in `userConfig` — still unaddressed

lane_stop_gate_marker says "Empty disables the file signal."; lane_stop_gate_sentinel has no parallel note about empty-string behavior. This one-sentence gap can now be fixed together with the stale "standalone-token" description above in a single edit.

Summary

a6335f4 is a correct, well-tested fix that resolves the Codex inline comment and meaningfully raises the bar on the sentinel-disclosure design tension. The implementation and tests are sound.

One new defect introduced: plugin.json's lane_stop_gate_sentinel description still says "standalone-token only" — it was not updated to match the README/CHANGELOG language. This is a one-line fix and should go in before merge.

The two hardening items from the security review (CWD marker-path traversal, git -C guard) and the empty-sentinel doc gap remain. None are new.
| Branch

claude · 2026-07-22T09:37:47Z

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

Security Review — PR #969 (HEAD `a6335f4`)

Gather context and understand request
Get PR diff at current HEAD (a6335f4)
Read hooks/lane-stop-gate.sh — verify all open findings
Read hooks/lane-notify.sh — verify notification security
Read hooks/hook-utils.sh — shared utilities
Read hooks/hooks.json and plugin.json
Synthesize findings and post review

This pass covers all files changed in this PR at HEAD a6335f4. Preceding review rounds thoroughly established the notification-security posture (lane-notify.sh) and hook infrastructure; I won't re-derive those. This pass focuses on the gate logic changes and the status of every open finding.

Resolved in `a6335f4` — Inline sentinel mention/negation bypass

Previously: Codex (P2), flagged in prior review passes as a non-finding / low-residual issue.
Status: FIXED.

hooks/lane-stop-gate.sh L74

The old standalone-word regex (^|[^A-Za-z0-9-])${SENTINEL_RE}([^A-Za-z0-9-]|$) matched the sentinel anywhere in a line, so a turn saying "I should not emit LANE-STOP-OK yet" set SIGNALED=1. The new regex:

grep -qE "^[[:space:]]*${SENTINEL_RE}[[:space:]]*$"

requires the token to be the sole content of a line (surrounding whitespace permitted). Cases 13 and 14 in the updated test suite verify both directions. This is a correctness tightening that also narrows the residual false-positive attack surface.

IMPORTANT — Sentinel disclosed in the block reason

Confidence: HIGH

hooks/lane-stop-gate.sh L121

REASON="…declare completion by emitting the exact token ${SENTINEL} on its own line…"

The block reason interpolates the literal sentinel value and delivers it to the agent via the decision:"block" reason field. a6335f4 tightened detection from standalone-word to line-only, but simultaneously made the block reason's instructions more precise: it now says exactly "emit the exact token LANE-STOP-OK on its own line", which is simultaneously the description of the gate contract and a line-by-line recipe for bypassing it.

The bypass sequence at this HEAD is deterministic and zero-effort:

Agent attempts stop → gate blocks; reason contains "…emit the exact token LANE-STOP-OK on its own line…"
Agent emits a line containing only LANE-STOP-OK (optionally whitespace-padded) in its next turn
Gate reads last_assistant_message, ^[[:space:]]*LANE-STOP-OK[[:space:]]*$ matches, SIGNALED=1, stop allowed — operator notification suppressed

The stricter line-only match from a6335f4 is correct for false-positive prevention (inline discussion of the token no longer fires the gate), but it does not reduce the gate bypass risk for a processing agent: the block reason now gives both the token and the exact emission format required.

The consequence is not merely an allowed early stop: because SIGNALED=1 routes through the silent exit 0 branch rather than the stop_hook_active notification branch, the operator notification — the explicit fail-safe handoff — is suppressed. A lane that processes its block output and emits the sentinel bypasses both the gate and the alert.

This was rated IMPORTANT/HIGH across all prior review passes and remains the most significant unresolved finding.

Minimum-effort mitigation: replace ${SENTINEL} in the reason with a mechanism description — e.g. "the configured completion token (see the lane_stop_gate_sentinel userConfig setting)" — and instruct the agent to recall its task goal rather than a specific string. The line-only emission requirement is already stated; the only change is removing the literal token value from the denial message. An agent that genuinely completed its goal should have that context from the task brief, not from the gate's denial.

hooks/lane-stop-gate.sh L86–87

SUGGESTION — Unvalidated CWD constructs the marker-file path

Confidence: MEDIUM

CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // ""' 2>/dev/null | tr -d '\r')
[[ -n "$CWD" ]] && MARKER="${CWD%/}/$MARKER"

Unchanged from prior passes. The cwd field from the hook payload is CR-stripped but not canonicalized before being joined with the configured marker filename. A cwd value containing .. segments (e.g. /project/subdir/../../etc) resolves MARKER against an arbitrary filesystem path. An agent that controls its working directory via cd in tool calls can cause [[ -f "$MARKER" ]] to match a predictable world-readable file (e.g. /etc/hostname when the marker is configured as hostname), setting SIGNALED=1 and authorizing a premature stop — again suppressing the operator notification along with the gate.

The absolute-path branch of the case statement at L83–84 is not affected.

hook::physical_path is already defined in hook-utils.sh (L176–185) and does exactly the canonicalization needed. After constructing MARKER, verifying that hook::physical_path "$MARKER" still falls under $CLAUDE_PROJECT_DIR closes this.

hooks/lane-stop-gate.sh L108

SUGGESTION — `git -C "$CWD"` without absolute-path guard

Confidence: LOW

[[ -n "$CWD" ]] && BRANCH=$(git -C "$CWD" branch --show-current 2>/dev/null | tr -d '\000-\037')

Unchanged from prior passes. Because $CWD is double-quoted, a leading-- value is passed as a path to chdir() rather than parsed as a flag, and fails gracefully — BRANCH returns empty and LANE falls back to "unknown". The practical impact is low. A pre-check that $CWD begins with / (or a drive letter) costs nothing and documents the assumption:

[[ "$CWD" == /* || "$CWD" =~ ^[A-Za-z]:[/\\] ]] || CWD=""

plugins/autonomy/.claude-plugin/plugin.json L34

SUGGESTION — `plugin.json` sentinel description stale after `a6335f4`

Confidence: HIGH (documentation gap, not a runtime flaw)

"description": "The exact token the agent emits in its final message to declare the lane's goal met and authorize a stop. Match is standalone-token only."

The a6335f4 commit tightened detection from standalone-word to line-only, but plugin.json still says "standalone-token only". A consumer reading this description will not know that the token must be alone on its own line — they may configure a sentinel and expect it to fire when embedded in a longer sentence. This gap combines with the undocumented empty-string behavior noted in prior passes. Both can be closed in one description update:

"The exact token the agent emits in its final message to declare the lane's goal met and authorize a stop. The token must appear alone on its own line (surrounding whitespace permitted); inline mentions do not authorize a stop. Empty string disables the token-based signal (marker-file path only)."