fix(hooks): guard-governed-enqueue reads one slug, and it strips the .git a clone URL carries - #18390
Conversation
…`.git` a clone URL carries `slug_of` and the inlined copy in the bare `gh pr merge <n>` target derivation ran the same sed, and the path character class owns `.` and is greedy, so the optional `\(\.git\)` group matched EMPTY: an origin of `https://github.com/objectstack-ai/cloud.git` read back as the slug `objectstack-ai/cloud.git`, which equals no `owner/repo` this guard is ever asked about. A sibling cloned with the URL `git clone` hands out therefore resolved nothing and was never audited, and a bare `gh pr merge <n>` in such a clone derived a slug the API answers 404 for — this guard's fail-open branch. One reader now, called from both sites, and the suffix comes off after the match rather than out of the character class, which still has to own `.` (`objectstack.ai` is a legitimate repository name) and whose race with the group GNU and BSD sed are free to settle differently. The self-test pins both shapes: four unpinned spellings of the sibling's origin (https/ssh x with and without `.git`, plus a trailing slash) resolve and are audited, and a bare `gh pr merge <n>` run from a checkout whose origin carries the suffix derives `#13794`. 56 passed / 0 failed before, 62 / 0 after. Claude-Session: https://claude.ai/code/session_01Bz6hxDBqK62NP2W1LATvnt Co-authored-by: Claude <noreply@anthropic.com>
…`git@github.com:` to https `git remote get-url` honours `url.<base>.insteadOf`, and this container injects one through `GIT_CONFIG_COUNT`/`GIT_CONFIG_KEY_n`, so the two `git@github.com:` rows were handed `https://github.com/…` and were second copies of the https rows — green over a branch of the reader they never reached. Measured, not assumed: with the injection unset the same remote reads back as the ssh URL. Claude-Session: https://claude.ai/code/session_01Bz6hxDBqK62NP2W1LATvnt Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS 维护者速读(终稿)改了什么 — 为什么改 — 这是一个会「放行」的守卫缺陷: 风险与代价(含回滚) — 低:两个 hook 脚本,不进任何发布产物( 席位意见 — 席位在契约复审档复核 PASS;自测在本席容器复跑 62 / 0(main 56 / 0);两条申报的偏差都有实测理由。建议批准。这是同文件串行链 #18366 → #18367 → #18384 的第一棒,批得早后两张才能开工。 你要做的 — 在本 PR 上给一次 APPROVED(os-zhuang 或 hotlong 任一)。之后由本席按裁决 C 落地(清标、ready、auto-merge SQUASH);⛔ 席位不自行批。 Generated by Claude Code |
Fixes #18366
guard-governed-enqueue.shread a checkout'soriginwith onesedexpression written outTWICE — once in
slug_of, once inlined in the baregh pr merge Ntarget derivation. The pathcharacter class owns
.and is greedy, so the optional\(\.git\)group matched EMPTY and anorigin of
https://github.com/objectstack-ai/cloud.gitread back as the slugobjectstack-ai/cloud.git, which equals noowner/repothis guard is ever asked about. Measuredon
8b81ab60before the fix, both halves reproduce:git clonehands out resolved NOTHING, so the guardnever audited it — the same fixture answers
blockon a bare-URL sibling andallowon a.gitone;gh pr merge N(no-R) in such a clone derived a slug the API answers 404 for, andthis guard's read-failure branch ALLOWS. On any conventionally cloned checkout that spelling
was unguarded.
维护者速读(草稿)
改了什么 — 把
.claude/hooks/guard-governed-enqueue.sh里重复两遍的 origin 读法收成一个slug_of函数(两处调用它),并让它在匹配之后剥掉结尾的.git;自测文件新增两组用例:四种origin 拼写(https/ssh × 有无
.git,外加结尾斜杠)的 sibling 都能被解析并被审计,以及在一个origin 带
.git的 checkout 里执行不带-R的gh pr merge N时,守卫推导出的仓名是 API 认得的那个。除这两个文件外没有别的改动。
为什么改 — 这是一个会「放行」的守卫缺陷:
git clone默认给出的 URL 就带.git,而守卫用错的仓名去读 PR 会拿到 404,它的设计是读不到就放行(硬约束线在 merge queue 那一侧)。也就是说,
在任何按常规方式克隆的 checkout 上,
gh pr merge N这个写法此前是不设防的;同时带.git的sibling 永远解析不到,受管面的判定会落在错误的树上(或没有树)。
风险与代价(含回滚) — 风险低:改动只在两个 hook 脚本里,不影响任何发布产物(
.claude/**不发布,因此打
skip-changeset)。新读法对旧的 bare 拼写字节等价,只多剥一个后缀;GitHub 本身拒绝以
.git结尾的仓库名,所以不会误剥合法仓名。代价是守卫会开始拦截此前被放行的gh pr merge N—— 这正是它应该做的。回滚 = revert 本 PR 的两个 commit,守卫退回今天的行为,merge queue 侧的硬线不受影响。
席位意见 — (留空,待席位定稿)
你要做的 — 这是规则层(
.claude/hooks/**)受管面改动:PR 保持 draft,⛔ 不由任何 agent 席位合并、入队或 arm auto-merge。需要维护者(或授权账号)的一次 APPROVED,之后由拥有它的席位落地。
The fix
One reader, two call sites, and the suffix comes off AFTER the match rather than out of the
character class — the class still has to own
.(objectstack.aiis a legitimate repositoryname), and which of the two wins the class-vs-group race is exactly the kind of thing GNU and
BSD sed are free to settle differently. A trailing
.gitis removed by the shell either way,and nothing legitimate is lost with it: GitHub refuses a repository name that ends in
.git.The self-test pins both shapes the card asks for. The
.gitsibling rows are asserted asAGREEMENT with the register (the same
$sibling_wantthe neighbouring case derives), never as acopied verdict. The bare
gh pr merge Ncase runs a copy of the hook from a checkout it buildsitself, because the hook derives its repo root from its own path.
Two things that case does NOT do, both deliberate and both measured rather than assumed:
OS_GOVERNED_ENQUEUE_FIXTUREanswers EVERY path, so the 404that makes the real guard fail open cannot be reproduced without the network — under the
fixture a wrong slug blocks exactly like a right one, and a verdict row would pass in both
worlds. The discriminator is the slug itself, which the refusal prints as
OWNER/REPO#NUMBER.git@github.com:rows unsetGIT_CONFIG_COUNT/GIT_CONFIG_PARAMETERS. Thiscontainer injects
url.https://github.com/.insteadOf git@github.com:through the environmentand
git remote get-urlHONOURS it, so without the unset git hands the hookhttps://...andboth ssh rows are silent duplicates of the https ones. On a machine that injects nothing,
env -uon an unset name is a no-op.Evidence
Self-test,
bash .claude/hooks/guard-governed-enqueue.selftest.sh:origin/main8b81ab60(before and afterpnpm install)56 passed, 0 failed62 passed, 0 failedFiring controls (each: mutate, prove the mutation reached the disk by blob hash, run, restore with
git checkout HEAD -- path, prove the restore by blob hash equality plus an emptygit diff HEAD):sed, no suffix strip)57 passed, 5 failedgh pr merge Nrows red, and the three.gitspelling rows red:dropped from the[:/]separator class60 passed, 2 failedgit@github.com:rows redEvery one of the six new rows is red under one of the two, so none of them is a phantom.
Gates —
node scripts/pm/dispatch-gates.mjs --commandsderived 13 families for these two paths;all 13 ran, all exit 0, and
--ranreconciles13 derived, 13 run, 0 NOT-MEASURED, 0 UNRUN (a DERIVED zero — all 13 recorded an exit code and none of them is 3).pnpm --filter @objectstack/lint run check:doc-formula-expressionsfirst exited 3 (PREREQUISITENOT MET — unbuilt
@objectstack/formula/@objectstack/lint, nothing measured); it exits 0after
turbo run buildfor those two packages.Also run:
bash -non both files (0, 0); the self-test under all threeOS_GOVERNED_ENQUEUE_SIBLING_ROOTstates — unset, an empty directory, a directory that does notexist —
62 passed, 0 failedeach;node scripts/pm/check-governed-merges.mjs --test .claude/hooks/guard-governed-enqueue.shexits 3 (GOVERNED, as expected for this surface);pnpm check:pm-settings-deny-rosterexits 0 (its roster lives under.claude, so its silence inthe derivation is not evidence in either direction);
grep -naPfor control bytes over both filesfinds none. There is no shellcheck in this tree: NOT MEASURED, with
bash -nplus the 62-casematrix as what stands in for it.
Acceptance notes
.claude/**publishes nothing from any released package, so this carriesskip-changeset.branch) and [finding] guard-governed-enqueue watches three enqueue spellings and a compliant seat can reach NONE of them — the CCR auto_merge route the protocol mandates is invisible to it #18384 (the ccr
auto_mergeroute arm). Neither is addressed by this PR and bothremain open.
repo_rootis not injectable the way the sibling root now is,which is why the bare
gh pr merge Ncase has to build a checkout and run a copy of the hookfrom it. That is a test-shape cost, not a defect, and the next PR to touch this file is the one
that would carry it if it ever becomes one.
Generated by Claude Code