feat(autonomy): deterministic lane-stop gate + operator STOP notification by kyle-sexton · Pull Request #969 · melodic-software/claude-code-plugins

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

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)."

plugins/autonomy/.claude-plugin/plugin.json L31–35

Items reviewed and found acceptable

a6335f4 sentinel line-only match (L74): grep -qE "^[[:space:]]*${SENTINEL_RE}[[:space:]]*$" is correct POSIX behavior — ^/$ match per line in multi-line input; surrounding whitespace is permitted but the token must be the sole non-whitespace content of its line. Cases 13 (inline mention blocked) and 14 (indented token on its own line allowed) verify both edges. ✓
OSC 9 injection (lane-notify.sh:58): C0 stripping via tr -d '\000-\037' before printf '\033]9;...' >/dev/tty. No finding.
AppleScript injection (lane-notify.sh:72–76): argv heredoc pattern. No finding.
notify-send injection (lane-notify.sh:83): -- flag terminator. No finding.
jq --arg r "$REASON" output (L123): Proper JSON encoding regardless of REASON content. No injection in the emitted JSON — including the sentinel value now interpolated into REASON. No finding for the output encoding; the sentinel-disclosure finding above is a logic issue, not an injection surface.
Sentinel ERE escaping (L73): The sed escape covers all ERE metacharacters; $LAST reaches grep via stdin. No injection surface.
hooks.json registration (hooks.json:8): ${CLAUDE_PLUGIN_ROOT} is Claude Code-resolved (not shell-expanded); 15 s timeout with fail-open semantics. No finding.
Default-OFF (L42): Confirmed; the gate is a no-op for all non-opted-in sessions. No finding.
SubagentStop guard (L58): [[ "$EVENT" == "Stop" ]] || exit 0 correctly placed after stdin parsing. No finding.

Summary: a6335f4 correctly resolves the inline-mention false-positive (Codex P2) — the line-only match is the right fix. Three findings remain open: the sentinel-disclosure in the block reason (IMPORTANT — suppresses the operator notification on a deliberate bypass), the CWD path-traversal in marker construction (SUGGESTION — false-negative only), and the git CWD absolute-path guard (SUGGESTION — low impact). A new documentation-gap finding is introduced by a6335f4: the plugin.json sentinel description still says "standalone-token only" when the implementation now requires a dedicated line.
| Branch

kyle-sexton · 2026-07-22T09:40:07Z

Classification of the HEAD (73035c6) review findings — PR-Review (comment) and Security-Review (comment). The two 08:21 passes referenced the stale commit 687229b4 and are superseded by these HEAD passes; the earlier PR-Review "hooks registration wiring" High was not repeated at HEAD (resolved between passes).

#	Finding	Severity	Classification	Evidence
1	Line-level completion token — inline mention/negation of the sentinel bypasses the gate (codex P2)	P2	VALID — fixed in `a6335f4a`	Detection now requires the token alone on its own line; regression tests added (Case 13 blocked / Case 14 allowed). See the threaded reply on the codex comment.
2	Sentinel disclosed in the block reason ("repeat the token we gave you")	IMPORTANT	VALID (defer) — author design call	Real but bounded: the gate is one-nudge-then-allow+notify by design, so a determined agent's stop is allowed after one nudge regardless of this — the residual impact of disclosure is mainly that the agent skips the second attempt and thus the operator alert, not a new hard-gate bypass. Hiding the literal token from the nudge trades away a legitimately-complete lane's ability to discover the completion protocol (it may not be able to read `userConfig`). Whether to make that trade is a design decision for the author, not a clear-cut defect; not fixed here to avoid silently choosing a priority.
3	Unvalidated CWD (`..`) in relative marker-path construction (L83–84)	MEDIUM / SUGGESTION	VALID (defer) — defense-in-depth	`cwd` originates from Claude Code's own hook payload; impact is a false-negative (premature stop), not privilege escalation, and only on the relative-marker branch. Legitimate hardening via the existing `hook::physical_path`, out of scope for this line-level correctness fix.
4	Empty-sentinel behavior not documented in `lane_stop_gate_sentinel` `userConfig`	Low	VALID (defer) — doc nit	One-sentence parallel to the `lane_stop_gate_marker` note ("Empty string disables the token-based signal"). No behavior change; cheap follow-up.
5	`git -C "$CWD"` without `--`/path validation (L105)	Low / SUGGESTION	VALID (defer) — graceful today	A `-`-leading `cwd` makes `git -C` fail and `BRANCH` falls back to `"unknown"`; cheap explicit-intent hardening, no current defect.

This pass fixed the one core-purpose correctness finding (#1 — the gate firing on a mention of its own token). Findings #2–#5 are a genuine design tradeoff (#2) plus defense-in-depth/doc follow-ups (#3–#5) with no DEFER-FORBIDDEN security class among them (all are false-negative-only, or design/doc), so they are classified and left for the author rather than swept into this line-level fix.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6335f4ac1

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

kyle-sexton · 2026-07-22T16:13:53Z

🔒 merge-drive lane claiming this PR for on-branch fix work this cycle (amendment-round: 16, worker tier). Will fix clear branch-owned CI/bot findings, defer non-DEFER-FORBIDDEN items with reply+resolve, push, then re-check the merge gate.

… match The `lane_stop_gate_sentinel` description still described the pre-a6335f4a word-boundary behavior ("Match is standalone-token only") after detection was tightened to require the token alone on its own line, leaving the one surface a configuring operator actually reads contradicting README, CHANGELOG, and the block reason the gate emits.

claude · 2026-07-25T06:51:46Z

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

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

Gather context and understand request
Read diff for latest commit (e64b988 — sentinel userConfig description update)
Read plugin.json — verify description fix is correct and complete
Read lane-stop-gate.sh — verify status of remaining open findings
Post complete review

Latest commit (`e64b988`) — Sentinel description update

The change is correct. The lane_stop_gate_sentinel userConfig description is updated from:

"Match is standalone-token only."

to:

"Matched only when alone on its own line."

This aligns the manifest with the behavior introduced in a6335f4 (the grep -qE "^[[:space:]]*${SENTINEL_RE}[[:space:]]*$" pattern). A consumer reading this description will now understand that the token must occupy a dedicated line, not merely appear as a standalone word anywhere in the message. ✓

⚠️ Remaining gap — empty-sentinel behavior still undocumented

This was flagged in the two prior review passes and this commit was an opportunity to close it alongside the stale-wording fix. The lane_stop_gate_sentinel description still has no note about empty-string semantics, while the parallel lane_stop_gate_marker description already says "Empty disables the file signal." A consumer who sets the sentinel to "" to suppress the LANE-STOP-OK default will be unaware they've also removed the only token-based completion path — every stop will trigger the nudge-then-notify cycle with no route to a signaled clean exit.

One-sentence addition closes both gaps that e64b988 was positioned to fix together:

"Matched only when alone on its own line. Empty string disables the token-based signal (marker-file path only)."

hooks/lane-stop-gate.sh L121

Status of all prior open findings at HEAD `e64b988`

#	Finding	Prior rating	Status
1	Sentinel disclosed in block reason (`L121`)	IMPORTANT (design tradeoff)	Unchanged — deferred, accepted
2	Unvalidated CWD in relative marker-path (`L86–87`)	MEDIUM/SUGGESTION	Unchanged — unaddressed
3	`git -C "$CWD"` without absolute-path guard (`L108`)	LOW	Unchanged — unaddressed
4	Empty sentinel behavior undocumented in `userConfig`	Low	Unchanged — still open
5	Stale "standalone-token only" description	New defect from `a6335f4`	✓ Fixed by `e64b988`

Finding #1 remains an accepted design tradeoff per the orchestrator's ruling — the a6335f4 line-level match raises the bar meaningfully (inline mention no longer bypasses the gate). Findings #2 and #3 are defense-in-depth hardening that has been consistently deferred. Finding #4 is the one item this commit was positioned to close but did not.

Summary

e64b988 correctly resolves the stale description defect introduced by a6335f4. The description wording now accurately reflects the line-level match semantics and matches the README/CHANGELOG language. ✓

The empty-sentinel documentation gap (finding #4) remains the one low-effort open item — a one-sentence addition to the description that keeps the lane_stop_gate_sentinel and lane_stop_gate_marker parallel notes consistent. All other findings are unchanged from the prior pass.
| Branch

claude · 2026-07-25T06:51:55Z

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

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

Gather context — review PR diff at HEAD
Read hooks/lane-stop-gate.sh — gate logic
Read hooks/lane-notify.sh — notification
Read hooks/hook-utils.sh — shared utilities (key functions: physical_path, require_jq, buffer_stdin)
Read plugin.json — manifest
Verify status of all prior open findings
Post security findings

Latest commit (`e64b988`) — Documentation fix

e64b988 changes one line in plugin.json: the lane_stop_gate_sentinel description is updated from "Match is standalone-token only." to "Matched only when alone on its own line." — aligning the manifest description with the detection logic introduced in a6335f4. This is a pure documentation correction with no security surface. The change is correct. ✓

Open security findings — status at HEAD `e64b988`

All three substantive findings from prior review passes remain unaddressed. Confirming each against the current code:

IMPORTANT — Sentinel disclosed in the block reason

Confidence: 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 via the decision:"block" reason field. a6335f4 tightened detection to line-only, but simultaneously made the bypass recipe more precise: the block reason now reads "emit the exact token LANE-STOP-OK on its own line", which is also the exact output required to pass the gate with zero additional work.

The bypass sequence is deterministic:

Agent attempts stop → gate blocks, reason contains e.g. LANE-STOP-OK and the exact emission format
Agent emits a line containing only LANE-STOP-OK on its next turn
^[[:space:]]*LANE-STOP-OK[[:space:]]*$ matches → SIGNALED=1 → stop allowed, operator notification suppressed

The operator notification is the fail-safe handoff. An agent that reads its block output can suppress it without doing any additional work toward the lane's stated goal.

Note: the previous final review classified this as an accepted trade-off (the sentinel is in the test at lane-stop-gate.test.sh:73 as an explicit assertion, and omitting it from the nudge would prevent legitimately-complete lanes that don't otherwise know the token from signaling completion). This classification is reasonable. The risk documented here is: the gate is a bounded speed-bump (one-nudge-then-allow), not a hard stop; the notification is best-effort. An agent that bypasses the gate still gets exactly one extra turn before the operator is alerted either way. Operators relying on this gate for hard enforcement should be aware of this.

Minimum-effort mitigation if desired: replace ${SENTINEL} in the reason with "the configured completion token (see the lane_stop_gate_sentinel userConfig)". This requires the agent to have set the token itself (as the lane launcher) rather than read it from the denial.

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

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"
...
[[ -f "$MARKER" ]] && SIGNALED=1

Unchanged. The cwd from the hook payload is CR-stripped but not canonicalized before being joined with the configured marker filename. A cwd containing .. segments resolves MARKER against an arbitrary filesystem path. An agent that controlled its working directory via cd in preceding tool calls can cause the marker check 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 while suppressing the operator notification.

Impact is limited to a false-negative (gate bypassed, premature stop allowed). hook::physical_path is already defined in hook-utils.sh:176 for exactly this canonicalization.

hooks/lane-stop-gate.sh L108

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')

Unchanged. $CWD is double-quoted, so a leading-- value reaches git -C as a literal path argument and chdir() will fail — BRANCH returns empty and LANE falls back to "unknown". No code execution risk; failure is graceful. Adding a pre-check ([[ "$CWD" == /* ]] || CWD="") documents the intent and eliminates any residual ambiguity about future git option handling.

plugins/autonomy/.claude-plugin/plugin.json L31–36

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

e64b988 corrected the stale "standalone-token only" wording ✓, but did not add the empty-string behavior note. The lane_stop_gate_marker description says "Empty disables the file signal." The lane_stop_gate_sentinel description has no parallel. 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 additional sentence closes this: "Empty string disables the token-based signal (marker-file path only)."