Skip to content

finding(hooks): guard-main-checkout.sh never reads notebook_path, so every NotebookEdit is judged by CLAUDE_PROJECT_DIR — wrong in BOTH directions, though the matcher routes that tool to it #7260

Description

@yinlianghui

Found while porting the sibling repo's self-test matrix for this hook (#6451). Out of scope for that card — that PR pins today's behaviour and says the hole out loud; it deliberately does not change the hook (.claude/** is governed surface, and the dispatch fence forbids weakening or "fixing" the guard in a matrix-only PR). Filed rather than fixed.

This is the objectui copy of objectstack-ai/objectstack#11810. The two hooks' executable lines are identical, so the defect is the same one, present here.

The mismatch

.claude/settings.json routes three tools at this hook:

"matcher": "Edit|Write|NotebookEdit"

.claude/hooks/guard-main-checkout.sh:28 extracts exactly one key:

file="$(printf '%s' "$input" | jq -r '.tool_input.file_path // empty' ...)"

NotebookEdit does not carry file_path. Its path key is notebook_path. So every notebook edit yields an empty file, falls into the no-path branch at line 36 (d="${CLAUDE_PROJECT_DIR:-$PWD}") and is judged by the SESSION's root instead of by the file being written. The jq-less fallback at line 31 greps for the same "file_path" key, so it misses too.

Wrong in both directions — measured

The verdict becomes a constant per session, independent of where the notebook actually lives:

session CLAUDE_PROJECT_DIR notebook being edited verdict today
shared primary checkout in that same checkout block right answer, wrong reason
shared primary checkout inside a linked WORKTREE block WRONG — refuses the mandated location
shared primary checkout in no repo at all (scratchpad, /tmp) block WRONG — refuses a file in no repo
a linked worktree in the shared primary checkout allow WRONG — unguarded edit into the shared tree
no repo at all in the shared primary checkout allow WRONG — same, session rooted outside any repo

Four of the five are wrong. The fail-open rows are the serious ones: an agent working correctly out of a worktree can write a notebook straight into the shared checkout and the guard waves it through.

All five are pinned as cases in the matrix landing on #6451, under a KNOWN HOLE banner naming this card. When this is fixed they become block / allow / allow by the notebook's own path.

Shape of the fix (not asserted)

Reading .tool_input.notebook_path as a second source alongside file_path (and the same alternation in the grep fallback) is the obvious move, but the matcher-vs-extractor pairing is the thing to get right, not just this one key — a future tool routed at this hook with a third path key repeats the bug. Both repos should move together, and the matrix on #6451 gives the change a red/green.

Filed unassigned; no domain:* or type label set.

Activity

  1. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 · domain:skills / priority:p2 / pm:queue

    Anchor read, not guessed. .claude/hooks/guard-main-checkout.sh ⇒ domain:skills(治理面)⇒ 草稿 PR + 人工合并。

    ⭐ 复核 objectui origin/main a472b07(2026-09-05T01:04:19Z),这个错配是精确的:

    .claude/settings.json:19               "matcher": "Edit|Write|NotebookEdit",
    .claude/hooks/guard-main-checkout.sh:28  file="$(… jq -r '.tool_input.file_path // empty' …)"
    notebook_path 在该钩子中                  0 命中
    

    ⇒ matcher 把三个工具路由过来,提取器只认一个键。 NotebookEdit 不带 file_path,它的路径键是 notebook_path ⇒ 每一次 notebook 编辑都得到空 file,落进无路径分支(d="${CLAUDE_PROJECT_DIR:-$PWD}"),由会话根目录而不是被写文件来判决。⚠️ 无 jq 的回退分支 grep 同一个 "file_path" 键 ⇒ 同样漏掉。

    定级 p2 —— 而五分之四判错是它不能更低的理由

    会话 CLAUDE_PROJECT_DIR 被编辑的 notebook 今日判决
    共享主检出 同一检出内 block 对的答案,错的理由
    共享主检出 在 linked worktree 内 block ❌ 拒绝了被强制要求的位置
    共享主检出 不在任何仓库里(scratchpad、/tmp) block ❌ 拒绝一个不在仓库里的文件
    一个 linked worktree 在共享主检出里 allow ❌ 对共享树的无守卫写入
    不在任何仓库 在共享主检出里 allow ❌ 同上

    ⇒ ⭐ fail-open 那两行是要命的:一个正确地在 worktree 里工作的 agent,可以把 notebook 直接写进共享检出,而守卫放行。 那正是这道钩子存在的全部理由所要阻止的事。

    ⛔ 不是 p1,理由只有一条,而且是频率而非严重性:这是一个 TypeScript 单仓,NotebookEdit 的实际使用量极低。⚠️ ⭐ 升级条件写在这里:一旦本仓出现常规的 notebook 工作流,它立刻是 p1 —— 严重性已经满格,只差发生率。

    ⭐ 与 #7259 是一对,必须一起考虑

    两张卡同源(都来自 #6451 的矩阵移植)、同文件、同治理面、且上游各有对应卡(#7259 ↔ objectstack#11809,本卡 ↔ objectstack#11810)。

    ⚠️ 但两者的错法不同,不要合并成一个「修钩子」的 PR 而不分别验证:

    ⇒ ⭐ 修 #7259 不会修好本卡,反之亦然。

    ⛔ 修法要抓的是那对配对,不只是这个键

    把 .tool_input.notebook_path 作为与 file_path 并列的第二来源来读(grep 回退里做同样的择一)是显然的动作,但要做对的是 matcher 与提取器的配对,而不只是这一个键 —— 将来任何一个被路由到这道钩子、带第三种路径键的工具,都会把这个 bug 重演一遍。

    ⇒ ⭐ 本席把它列为验收条件而非建议:修复应当让「matcher 里列了某个工具」与「提取器认得它的路径键」之间有一条可检查的关系,而不是再多写一个 //。⛔ 只加一个 notebook_path 的 PR 会在下一个工具上重现。

    ⭐ 红/绿已就位,两个仓要一起动

    五行全部已作为用例钉在 #6451 落地的矩阵里,置于点名本卡的 KNOWN HOLE 横幅之下 ⇒ 修好后它们变成 block / allow / allow,按 notebook 自己的路径判决。 ⛔ 修复 PR 必须同时翻转它们。

    而两个钩子的可执行行完全相同 ⇒ ⛔ 两个仓一起动,否则守卫语义分叉。

    ⛔ 本席不认领、不派发、不合并、不代裁。


    Generated by Claude Code

  2. self-assigned this
    on Sep 5, 2026
  3. os-steve commented on Sep 5, 2026

    @os-steve
    Collaborator

    Claim: PM loop round 5 — graded p2 at first touch (triage's grade and route stand); one cross-repo M flight with objectstack #11810 (its twin), taken over under the cross-lane takeover rule
    Session: session_019RfFHiRCSs3JXLK4cwcfox
    Branch: claude/issue-7260-guard-hook-notebook-path (objectui) · claude/issue-11810-guard-hook-notebook-path (objectstack)
    Worktree: objectui-issue-7260 · objectstack-issue-11810
    Domain: domain:skills
    File surface: .claude/hooks/guard-main-checkout.sh + .claude/hooks/guard-main-checkout.selftest.sh in BOTH repos — the hook reads the path from the key its routed tool actually carries (file_path for Edit/Write, notebook_path for NotebookEdit, in jq and in the grep fallback), the matrix's KNOWN HOLE #7260 / #11810 section flips to the intended verdicts (block / allow / allow by the notebook's own path) and the banner goes, and the self-test gains the pairing check triage set as the acceptance condition: every tool settings.json routes to this hook has a path key the hook reads. ⛔ No change to the */worktrees/* predicate (#7259 / #11809, serial behind this flight), ⛔ no edit to settings.json's matcher, ⛔ the two hooks' executable lines stay identical across the repos (stop on breach; explain in the report)
    Container & model: M (two repos, two governed draft PRs, one matrix flip each), mode:subagent, model: opus (dispatch-gates.mjs --tier on the four hook paths at 52b59d6dc, 2026-09-05T01:51Z: no path-derived mandate)
    Clause-②: no (PreToolUse guard; no published contract)
    Serial constraints cleared: no open PR touches .claude/hooks/guard-main-checkout* in either repo (search 01:5xZ — objectstack hits by body mention only, PR #15650 verified by file list as scripts/pm/**; objectui zero hits); #7259 (objectui) is the same file's second card, a different defect, hard-serial behind this flight (pm:blocked, Blocked-by: #7260), and objectstack #11809 follows it. GOVERNED in both repos (.claude/**; objectstack check-governed-merges.mjs --test exit 3 at 01:51Z) ⇒ two draft PRs, in-seat review, os-zhuang + hotlong requested, human merge; objectstack skip-changeset, objectui per check-changeset-presence.mjs. Self-tests on main at dispatch: 87 passed, 0 failed in each repo (01:51Z).

    Decision re-read (01:5xZ): triage's p2 (5548281023) and its acceptance condition (a checkable matcher-to-extractor pairing, not one more key) stand; the "two repos move together" condition is met by one dev holding one worktree per repo. Unknown-key behaviour is the seat's call and is ruled in the brief: a tool the matcher routes here whose path key the hook cannot find is a schema drift and blocks, never a silent fallback to the session root.


    Generated by Claude Code

  4. claude commented on Sep 5, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 7260,
      "status": "done",
      "branch": "claude/issue-7260-guard-hook-notebook-path",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7686",
      "premise_still_valid": true,
      "summary": "Both repos moved together in one flight. objectui PR https://github.com/objectstack-ai/objectui/pull/7686 (draft, Fixes #7260) and objectstack PR https://github.com/objectstack-ai/objectstack/pull/15665 (draft, Fixes #11810, skip-changeset applied at open and confirmed by readback alongside the size labeler's size/m). guard-main-checkout.sh now carries one tool-to-path-key table (known_path_keys='Edit=file_path Write=file_path MultiEdit=file_path NotebookEdit=notebook_path') and reads the key the routed tool actually carries, in the jq path and in the jq-less text scan alike; a notebook is judged by the notebook's own path exactly as a file edit is. The acceptance condition triage set is met mechanically, not by convention: the self-test reads known_path_keys out of the hook, splits settings.json's matcher on the pipe, and reds if a routed tool has no row (proved by ablation B). Per the seat's ruling on unknown keys, a tool IN the table carrying no path under its own key is schema drift and blocks with a message naming the tool and the key, never the silent fallback to the session root; a tool NOT in the table keeps today's permissive read. Three matrix rows pinned as INTENDED (not under a known-hole banner) contradicted that rule and were flipped and listed in both PR bodies; a fourth section had its probe re-pointed at an unrouted tool so it still measures the cwd fallback it was written for. The known hole for the */worktrees/* predicate (#7259 / #11809) is untouched, as are settings.json and guard-main-checkout-bash.sh. The two hooks' executable lines remain byte-identical across repos: comments-stripped diff empty, full diff exactly the 8 comment lines the objectstack copy already carried.",
      "tests": "A2 VERIFIED against the live tool schema, not assumed: NotebookEdit takes notebook_path (absolute, required) and new_source (required) plus cell_id/cell_type/edit_mode, with no file_path key anywhere; the matrix's nbpay helper is a faithful subset. A1 VERIFIED: self-tests on both branch tips cut from origin/main returned '87 passed, 0 failed' in each repo before the fix, and '116 passed, 0 failed' in each repo after. Run through the shared lock (scripts/pm/os-verify-lock.sh; every run ended 'VERDICT command-exit 0'). Old matrix against the NEW hook produced exactly 9 failures, the 9 rows named in the PR bodies and nothing else. A3 VERIFIED: the jq-less fallback is exercised, not assumed - the notebook rows, the drift row and a new tool_name decoy row all run a second time with jq removed from PATH via the existing nojq fixture. NON-VACUITY: three mutations, each on a THROWAWAY COPY driven through GUARD_MAIN_CHECKOUT_HOOK, so the tree was never mutated and there is no restore leg to get wrong - the worktree hook blob was checked equal to its HEAD blob at the start of each run (5d243648 objectstack, 619260d7 objectui). There is no rebuild leg to declare and none is claimed: the hook is an interpreted shell script the matrix executes directly by path, so no dist/ product sits between the mutation and the run. Each mutation was proved ON DISK before the matrix ran, by grepping the mutant for the injected text (count >= 1) AND the removed text (count 0), with the run declared void if either failed. Results identical in both repos: point NotebookEdit's row at file_path -> 107 passed, 9 failed; delete NotebookEdit's row -> 111 passed, 5 failed, including 'FAIL NotebookEdit is routed to this hook but has no row in known_path_keys'; neutralise the drift arm -> 107 passed, 9 failed; unmutated control -> 116 passed, 0 failed. GATES objectstack: family derived by the repo's own tool (node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at 5f871cbfd; 2 paths vs merge base 7087f99c1; 13 commands), all 13 run and all exit 0, exit codes captured by redirecting BEFORE any pipe; closed with '. dispatch-gates --ran: 13 derived famil(ies) accounted for - 13 run, 0 NOT-MEASURED.' Two needed a prerequisite first (check:doc-formula-expressions exits 3 PREREQUISITE NOT MET until @objectstack/formula and @objectstack/lint are built; it is green after that, and the exit-3 runs are recorded as not-measured rather than counted as passes). check:bash32-floor is the gate that actually covers this surface and is green: '27 tracked shell file(s) under scripts/**, .claude/hooks/**, .githooks/** name no bash 4+ construct'. Derivation staleness recorded rather than hidden: the tool reports the tree behind origin/main (791a0cbe6) with one gate source changed in that range, scripts/pm/check-skill-line-ratchet.mjs, whose population is a roster of skills/** and .claude/skills/** and so is not a lead for a .claude/hooks/** change set; re-deriving after a fetch produced a byte-identical 13-command list. GATES objectui: no dispatch-gates there, so the family was found by grep -l '\\.claude' scripts/*.mjs cross-read against package.json's check:* scripts - check-control-bytes, check-shell-escape-residue, check-governed-queue-guard --self-test (132 cases), check-skills-paths, check-skill-eval-tokens and check-changeset-presence all exit 0. check-skill-examples exits 2 PRECONDITION NOT MET and is recorded as NOT MEASURED, not as a pass: its own source declares SCAN_ROOTS = ['skills', '.claude/skills'], which this .claude/hooks/** change set does not enter, so building the ten package closures its --build-filter names would measure the same tree as main. CHANGESET: objectui has no skip-changeset label, so scripts/check-changeset-presence.mjs decided and was followed - 'No source or published contract of a released package changed in this range, so no changeset is owed'; none added. pnpm lint is NOT MEASURED in either repo by eslint's OWN configuration rather than by a narrowing of mine: asked directly about the two changed files it answers, per file, 'File ignored because no matching configuration was supplied.' Both are shell, outside eslint's configured surface entirely, so this diff cannot move any verdict about a file eslint does lint. CI (the brief restored the wait for this card, so this was polled in the foreground to completion, never by ending a turn): objectstack PR 15665 at 5f871cbfd, 29 check runs, 17 success / 12 skipped / 0 failures; objectui PR 7686 at 6e43f3c, 30 check runs, 27 success / 3 skipped / 0 failures. Both PR bodies were read back in full after creation and survived byte-for-byte; the only delta is the platform's own auto-appended signature footer, which duplicates the one in the body (left alone rather than PATCHed, since editing demotes the session-URL footer to the bare form).",
      "mcp_calls": "0 - the REST channel was probed first (repo-scoped read 200, /rate_limit 15000/hr) and carried every read and write: issue and comment reads, both PR creations, the label add, the label and body readbacks, the check-run polling, the duplicate sweep and the new finding. No MCP GitHub call was made.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as objectui#7692 (label finding, unassigned): this repo has no bash-version floor gate, so its OWN shell (scripts/setup.sh, scripts/ensure-chromium-ready.sh, e2e/live/ci/*.sh) is held to nothing, while the sibling repo gates every tracked shell file at bash 3.2 over 27 files. Deduped against 618 enumerated objectui issues, open AND closed, by REST list endpoints plus local grep - no bash-floor card exists. Recorded as a ratchet, not a live defect, with the reading bounded honestly: a grep of ten of the sibling gate's nineteen constructs found no violation today. Explicitly NOT subsumed by #7263 (the .claude/hooks/** port-pin gap): every one of this repo's eight hooks has a same-named counterpart in the sibling repo - measured, set difference empty - so a port pin would inherit the floor transitively for those, but the four non-ported shell files are reached by neither card."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions