fix(claude-ops): key lane markers in the target repo, and scope stop-gate arming to the installs that asked - #2050
Conversation
Unverified batch preserved from a session that ended before its findings were falsified against origin/main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…auncher-hardening
…auncher-hardening
Drops two changes that no filed finding motivates and repairs the fixture that was meant to pin the object-format key. resolve_config returns to its state on main. The per-query failure guard it had grown is a real gap, but a separate one — the launcher's marker keying, its arm-option plumbing, and its arm-id injection are all downstream of a config that already parsed, so nothing here depends on it. An explicitly empty sentinel is treated as absent again, matching main. The marker and the sentinel are not symmetric at the gate: an empty marker turns that channel off, while an empty sentinel is replaced by the default token, so recording one would change only which source the gate consults — it would shadow the user-level sentinel — with no behavior to show for it. The SHA-256 fixture never built a SHA-256 repository. The suite's `git` PATH stub answers pull, rev-parse, and hash-object and exits 0 for everything else, so `git init --object-format=sha256` reported success while creating nothing, and the fixture then compared two SHA-1 keys — it failed on the fixed launcher and would have kept failing. It now calls the real git. The multi-install fixture put the non-requesting entry first, where the last-match option read picks the requesting entry's marker either way. The non-requesting entry is now last, so the assertion distinguishes a scoped read from an unscoped one. Comment prose no longer says an arm id overrides an entry's own `false`. The gate does not trust this channel in either direction; the defect is that the launcher marks an install the lane never asked to arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ane fixes Records the repo-marker key scoping, the explicit-empty marker distinction, and the per-entry arm-id injection the branch already carries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…auncher-hardening
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 1m 39s —— View job Security review complete
Scope: all 5 files changed in this PR ( No security issues found. Details:
Per instructions, this review defers to zizmor's advisory lane for supply-chain/unpinned-action risk, dangerous workflow triggers, excessive permissions, and template injection — none of the changed files are GitHub Actions workflows, so that lane is not applicable here anyway. |
|
Claude finished @kyle-sexton's task in 3m 57s —— View job Claude's review
SummaryAll three fixes are correct and the reasoning in the PR body checks out against the actual code. #1 — marker key scoping. #2 — empty marker vs. absent. The #3 — arm-id scoping. Scope discipline: the middle commit's rollback (dropping the Not independently verified: this review environment's Bash tool required interactive approval for every git/test invocation (including read-only ones like No inline comments — I didn't find any defects to anchor to a line. |
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…auncher-hardening Renumber the claude-ops lane fixes to 0.27.5; main published 0.27.4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 34s —— View job Code review complete
SummaryAll three fixes are correct, and I traced each end-to-end rather than taking the PR body's reasoning at face value: #1 — marker key scoping. #2 — empty marker vs. absent. I traced the full round trip through the (unchanged, different-plugin) consumer: #3 — arm-id scoping. Test coverage. The new SHA-256 cross-format fixture correctly delegates to One documentation nit (posted inline at Not independently verified: this environment's Bash tool requires interactive approval for No blocking issues found. Branch: |
|
Claude finished @kyle-sexton's task in 2m 9s —— View job Security review complete
Scope: all 5 files changed in this PR, diffed against No security issues found.
This repo's zizmor advisory lane covers supply-chain/unpinned-action risk, dangerous workflow triggers, excessive permissions, and template injection; none of the changed files are GitHub Actions workflows, so that lane doesn't apply here. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…auncher-hardening
|
Claude finished @kyle-sexton's task in 1m 4s —— View job Security review complete
Scope: all 5 files changed in this PR, diffed against No security issues found.Reasoning, independently traced against the diff and the surrounding (unchanged) code:
This is a |
|
Claude finished @kyle-sexton's task in 3m 25s —— View job Code review complete
SummaryThis is a All three fixes are correct, well-reasoned, and match what the PR body claims:
New finding — test-coverage gap in KEY="$(printf '%s' "$(git -C "$REPO" rev-parse --show-toplevel)" | git hash-object --stdin)"The Test execution: this environment's Bash tool required interactive approval to run No blocking issues. Security-relevant aspects are covered by the dedicated security lane. |
The `-C` scoping added a third bullet to a list still introduced as two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 6s —— View job Security review complete
Scope: all 5 files changed in this PR, diffed against No security issues found.I independently re-traced each change rather than relying on the prior review passes on this PR (all of which also found nothing):
This is the fifth independent review pass on this PR (across security and code-review lanes); each has reached the same conclusion via independent tracing. No blocking security issues. |
|
Claude finished @kyle-sexton's task in 1m 22s —— View job Code review complete
SummaryThis is a -# Two properties the key must have, both learned the hard way:
+# Three properties the key must have, all learned the hard way:I confirmed this is the only change ( I also re-diffed the full PR against the merge-base ( Everything substantive from the prior review passes still holds (marker-key scoping fix in Outstanding from a previous pass, not part of this PR's file set: the earlier-noted test-coverage gap in Test execution: as with every prior pass on this PR, this environment's Bash tool required interactive approval for No blocking issues. No new inline comments — the one from the prior pass is now resolved by |
…auncher-hardening
|
Claude finished @kyle-sexton's task in 53s —— View job Security review complete
Scope: all 5 files changed in this PR, diffed against No security issues found.This is a
This is the seventh independent review pass on this PR (across security and code-review lanes), all reaching the same conclusion. No blocking security issues. |
|
Claude finished @kyle-sexton's task in 47s —— View job Code review complete
SummaryThis is a The two new commits on the branch are a merge of I also re-confirmed the full PR diff against Nothing new to review. All findings from the prior full code-review pass (31297111954) still hold:
Test execution: as with every prior pass on this PR, this environment's Bash tool requires interactive approval to run No blocking issues. No new inline comments — this synchronize introduced no changes in scope for this PR. |
…auncher-hardening Renumber the lane fixes to 0.27.6; main published 0.27.5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 15s —— View job Security review complete
Scope: all 5 files changed in this PR, diffed against No security issues found.This is another
This is now the eighth independent review pass on this PR (across security and code-review lanes), all reaching the same conclusion via independent tracing. No blocking security issues. |
…auncher-hardening
|
Claude finished @kyle-sexton's task in 1m 7s —— View job Security review complete
Scope: all 5 files changed in this PR, diffed against No security issues found.This is another One of the merged-in files is worth calling out even though it's outside this PR's diff: Re-confirming the reasoning independently against the unchanged diff:
This is now the ninth independent review pass on this PR (across security and code-review lanes), all reaching the same conclusion via independent tracing, and the underlying diff has not changed since the eighth pass. No blocking security issues. |
1 participant
Three bot-filed P2 defects in the
claude-opslanes launcher, all verified to reproduce atorigin/mainand to stop reproducing here. Every fix is covered by a new assertion that FAILS against the pre-fix launcher and passes against this one.1. The launch-commit marker key was digested in the wrong repository (#1383)
git hash-objectuses the object format of whatever repository it resolves. The launcher called it unscoped, so it keyed on the caller's format while taking the toplevel from the repository--reponames. Reached from a SHA-1 working directory, a SHA-256 target produced a 40-character key, whileskills/lanes/context/refresh.md's probe runs inside that checkout and computed the 64-character one — the launcher wrote its marker to a directory the probe never reads and staleness detection was silently off.The comment above the key asserted the two sides agree because both call
git rev-parse --show-toplevel. That settles the path and says nothing about the digest, so the invariant it claimed did not hold.Both digests are now taken with
-C "$REPO". The anchor is$REPO(guaranteed byresolve_repoto be an existing directory) rather than the hashed$top(a string git handed back) — anchoring on a path that may not exist would fail the digest into theunkeyedfallback and collapse every such repo onto one key.Scope is broader than the filed report:
restart-consumer.shderived its ledger key the same unscoped way and is fixed with it. The hand-recompute snippets in the README, the changelog, andrefresh.mdalready run inside the target repository and were correct as written; they are untouched.2. An explicitly empty stop-gate marker was read as an absent one (#1865)
gate_option_from_settingsendedselect(type == "string") ] | last // empty, which prints nothing for an explicit""and nothing for an absent key.[[ -n "$marker" ]]then dropped--markerfor both, the arm record carried no marker key at all, andlane-stop-gate.sh's precedence (managed ▷ arm record ▷ user settings ▷ default) walked past it to the user-level marker — where a marker file left over from another lane can authorize a stop this lane never signaled.A
v:prefix now carries "the lane set this" through the shell, so an explicit empty value reaches the helper as--marker "",lane-stop-gate-arm.shrecords{"marker": ""}, and the gate's[[ -n "$MARKER" ]]guard leaves the marker channel off instead of falling through.The sentinel is deliberately not symmetric.
lane-stop-gate.shsubstitutes the default token for an empty sentinel, so emptiness is not a configured value there; recording one would buy no behavior change while shadowing the user-level sentinel. An empty sentinel is therefore still treated as absent, and a fixture pins that asymmetry.3. The stop-gate arm id reached installs that never asked for it (#1865)
Arming keyed off an any-quantifier over the
autonomy/autonomy@*namespace, then injectedlane_stop_gate_arm_idinto every entry in it, and option extraction took its last match from any entry rather than a requesting one.The gate never treats this channel as a trusted verdict in either direction, so an id landing on an entry set to
falsewas not overriding thatfalse. What it did do is mark installs the lane never asked to arm — leaving the settings handed toclaudean inaccurate record of what was requested, and letting a non-requesting entry's marker reach the arm call. One shared filter now defines "an entry that requested the gate", and detection, option extraction, and injection all use it.Arming every discovered helper script is unchanged and deliberate.
Tests
lane-launcher.test.shgrows a SHA-256 cross-format marker fixture (skipped where git cannot create a SHA-256 repository) and four gate-arming fixtures. Against the pre-fix launcher with this test file, six assertions fail:marker: written under the TARGET repo's object-format keymarker: nothing is written under the caller-format keyarm: an explicitly empty marker still reaches the helperarm: options come from the requesting entryarm: a disabled sibling's marker never reaches the helperarm: the explicitly-disabled entry receives no arm idAll six pass here; the suite is 193 assertions, 0 failures.
Verification (independent re-run)
The pre-fix control was reproduced by copying the scripts directory to a scratch path, replacing
lane-launcher.shwithorigin/main's, and running this branch's unchanged test file againstit:
lane-launcher.test: FAIL — 6 case(s) failedthere,PASS — 193 caseshere. The six failuresare exactly the list above, so no fixture is passing on both trees.
The SHA-256 block executed rather than skipping —
git version 2.54.0.windows.1creates--object-format=sha256repositories, and cases 158-160 report PASS. The skip guard remains becausethe format is not universally compiled in.
Beyond the arm stub:
lane-stop-gate-arm.shinvoked directly with--marker ""writes"marker": ""into the arm record, while omitting the flag writes nomarkerkey at all. Thelauncher's explicit-empty distinction therefore survives to the gate, whose
gate_optionreturns theempty string (via the same
v:idiom) rather than falling through to user settings.Folding the key test and the value test into one
selectadds no type fragility: jq'sandshort-circuits, so a non-autonomy scalar entry is never indexed, and an autonomy entry whose value
is a scalar errors identically under the old and new filters.
Gates from the worktree root, all green:
check-changelog-parity.sh--check/--check-bump origin/main/--check-order,check-shell-portability.sh,check-skill-portability.sh,check-silent-skips.sh,check-plugin-manifest-presence.sh,check-changed-skills.sh origin/main(0 errors; one pre-existing SKILL.md-length warning),
validate-plugins.sh,markdownlint-cli2onthe changelog, and
shellcheck -xon all three changed scripts.Version renumbered to
0.27.6—mainpublished0.27.4and then0.27.5while this branch was inflight.
Related
No linked issue
The three findings were filed as review threads on merged PRs #1383 and #1865, not as issues. Those PRs are referenced for provenance only — this PR closes nothing.
A fourth thread on #1383,
PRRT_kwDOTCGFQM6TzlNw, needs no change here: the vacuous-traversal escapeit describes was already closed by
353baf64(#1851), and a control at353baf64^reproduces it.Its adjacent defence-in-depth observation — the three preflight
jqsubstitutions inlane-launcher.shthat ignore exit status — is deliberately left for a separate change. Lines 1-406of that file are byte-identical to
mainand all seven$(jq …)command substitutions in it areunchanged; the only edit above the marker-key block is line 407, where the property list's own count
went from "Two" to "Three".