Skip to content

chore(git-hooks): run the guardrails git hooks from this repository's lefthook remote - #9563

Merged
kyle-sexton merged 5 commits into
mainfrom
chore/guardrails-self-remote
Oct 24, 2026
Merged

kyle-sexton merged 5 commits into
mainfrom
chore/guardrails-self-remote

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

Refs: #9426

Summary

This repository's lefthook.yml had no guardrails git-hook jobs and no remote, so the guardrails session-start notice reported all three hooks missing here. It now runs them the way every consumer repository does: one lefthook remotes: entry tracking this repository's own main (owner decision; remote mode from #9504).

Fix

  • lefthook.yml: append the remotes: snippet install.mjs plan --scope remote returns (ref: main, refetch_frequency: 24h, plugins/guardrails/git-hooks/lefthook-remote.yml). The existing check-affected pre-push job, its comments and order are unchanged.
  • Header comment: lefthook install also fetches the remote's hooks.
  • .lefthook/<hook>/guardrails-*.sh: exit 0 unless run from lefthook-remotes/. This repository is its own remote, so lefthook found each script both in the work tree and in the fetched copy of main and ran both: every check ran twice, and a checked-out branch ran its own hook code. Consumers never have these files in their work tree, so they are unaffected.
  • lefthook.yml comments and docs/conventions/check-placement/ (README, adopting-git-hooks): .lefthook/ joins the paths to read before committing or pushing on an untrusted branch, and these hooks fire at commit too.
  • lefthook-remote.test.sh: asserts the work-tree copy of each script exits 0 without running; bump: none guardrails fragment.

Verification

  • lefthook install: sync hooks: ✔️(pre-commit, pre-push, commit-msg)
  • install.mjs verify: ok for commit-msg-convention, pre-commit-content and reference-verify
  • install.mjs plan --scope remote: {"manager":"lefthook","actions":[],"decisions":[]}
  • lefthook-remote.test.sh: 22 passed, 0 failed.
  • A staged test token: lefthook run pre-commit --force reports the secret finding once (the work-tree shim traces [[ ... == */lefthook-remotes/* ]] then exit 0).

Related

🤖 Generated with Claude Code

… lefthook remote

Add the `remotes:` entry `install.mjs plan --scope remote` returns, so this
repository runs commit-msg-convention, pre-commit-content and reference-verify
from its own main, the same way every consumer repository does. The header
comment now says `lefthook install` also fetches them.

Refs: #9426

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Oct 24, 2026
kyle-sexton and others added 4 commits October 24, 2026 00:17
…this repository

This repository is its own lefthook remote, so lefthook found each guardrails
script both in the work tree's .lefthook/ and in the fetched copy of main, and
ran both: every check ran twice, and a checked-out branch ran its own hook
code. Each .lefthook/ script now exits 0 unless it runs from lefthook-remotes/.

The lefthook.yml comments and the check-placement docs now name .lefthook/
among the paths to read before committing or pushing on an untrusted branch,
and say these hooks fire at commit too.

Refs: #9426

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d test it observably

On Windows lefthook passes a backslash script path, so the lefthook-remotes
guard never matched and every hook skipped silently. Each .lefthook/ script
now turns backslashes into slashes first, matches the narrower
info/lefthook-remotes/ segment, and runs its entry script from the normalized
path.

The lefthook-remote test runs each script beside a stub entry script that
reports a run: from a work tree it must not run, from a fetched copy it must,
including from a backslash path. Without the guard the work-tree cases fail.

The untrusted-branch text also names lefthook-local.yml and .lefthook-local/,
which lefthook runs at commit as well.

Refs: #9426

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton kyle-sexton removed the do-not-merge Hard merge gate: do not merge while applied. label Oct 24, 2026
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 24, 2026 04:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T04:33:01.177096Z b9bbf77 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed b9bbf778cb52f836dfc659511f9214f5c13b0ec4: 0 finding(s), 0 as inline comments.

Security review: PR #9563 (head b9bbf77)

No security issues found.

critical-findings: 0

What I read: all 8 files in the diff and the PR context. I read the PR text as data. It contains no embedded directives.

Scope checks

  • Manifests: no .claude-plugin/plugin.json or marketplace.json change, so there are no manifest <field> lines.
  • Trust-gate paths: no change to .github/standards/trusted-actors/**, .github/actions/check-trusted-trigger/**, CODEOWNERS or .gitmodules, so no hold.
  • Instruction-surface deletions: none. The docs and comment edits widen the "read before committing or pushing on an untrusted branch" guidance to lefthook-local.yml, .lefthook/ and .lefthook-local/. No standing rule is deleted, narrowed or softened.
  • Actions workflows: no workflow files changed.

Logic and trust-boundary analysis

  • Work-tree wrappers: the three .lefthook/*/guardrails-*.sh wrappers now exit 0 unless BASH_SOURCE contains /info/lefthook-remotes/. This is the intended narrowing. A checked-out branch's own copy of a hook script no longer runs at commit or push time, which removes a path where an untrusted branch could run its own hook code. Only a script launched from the work tree is skipped, and only in this repository, because consumers have no .lefthook/ in their work tree.
  • Path normalisation: ${BASH_SOURCE[0]//\\//} is plain parameter expansion with no eval or word splitting. "${src%/*}" is quoted at the exec. I found no injection surface.
  • Remote pin: lefthook.yml tracks the mutable ref: main of this repository. That is the owner's recorded decision (guardrails: shared install-git-hooks copies fail consumer CI (mode 100644, linted vendored scripts) #9426) and matches how every consumer is configured, so it is not a new finding. Anyone who can push to main controls the code that runs on each contributor's commit. Branch protection on main is the control for that.
  • Fail-open if lefthook's cache path changes: the guard fails open. If a future lefthook release moved its cache away from info/lefthook-remotes/, the wrappers would exit 0 silently, including the pre-commit secret scan, for this repository and for consumers. That is speculative and has no exploit path in this diff. The new test pins the current layout and would catch it.
  • Test script: lefthook-remote.test.sh writes only under a mktemp -d directory, cleans it up through a trap and an explicit rm -rf, and quotes its paths. No concern.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed b9bbf778cb52f836dfc659511f9214f5c13b0ec4: 0 finding(s), 0 as inline comments.

No blocking or advisory findings on head b9bbf77.

What I read: the full diff (8 files), the PR context, and the header and trap lines of plugins/guardrails/git-hooks/lefthook-remote.test.sh. I did not run any tests or scripts. The test results in the PR description are author-claimed and unverified by me.

What I checked:

  • Wrapper guard (.lefthook/*/guardrails-*.sh): ${BASH_SOURCE[0]//\\//} converts backslashes to slashes. The */info/lefthook-remotes/* match then exits 0 in the work tree and runs the entry script from the fetched copy. The ${src%/*}/../../plugins/... path resolves from the fetched copy's root.
  • Test changes: the sed extraction of $target matches the new exec bash "${src%/*}/..." line. The stub layout resolves ../../plugins/guardrails/git-hooks/... correctly for both the work-tree and fetched roots. The backslash-named file resolves to the stub the second root creates. $?:$out reads the exit status of the preceding assignment. The script has no set -e, so a nonzero wrapper exit can't abort it.
  • Trap override: trap ... EXIT at line 42 is replaced by the later WORK trap at line 86. The explicit rm -rf "$GUARD" before that point covers the normal path, so a leak only happens if the script dies in between. I'm not reporting it.
  • Closing issue: the context has no ## Closing issue section, only Refs: #9426, which this PR doesn't close. There were no acceptance criteria to check.
  • Manifests: the diff changes no plugin.json or marketplace.json, so there are no manifest <field> lines.
  • Rules, paved paths and holds: I did not resolve the path-scoped rule files under .claude/rules/ or the paved-path standards index, so nothing was checked against them. No hold class applies. The PR touches no trusted-actors, CODEOWNERS or .gitmodules path, and it adds no model calls or infrastructure.

Minor note, not counted as a finding: the changelog fragment says "No plugin file ships changed", but plugins/guardrails/git-hooks/lefthook-remote.test.sh is under the plugin directory. It is a test file, so bump: none is reasonable.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pr-verify verifier failed head b9bbf778cb52f836dfc659511f9214f5c13b0ec4. What it wrote, as data:

Summary: The PR is linked to #9426 by a `Refs:` line, so it can deliver only part of the issue, and its body has to say what remains. It doesn't. The body describes the delivered part: this repository now runs the guardrails hooks from its own lefthook remote, and the `.lefthook/` wrappers do nothing when run from the work tree, which touches item 11 and the owner's remote-mode decision. It never lists what is left of #9426. Open items include the `reference-verify.sh` jq argument-limit failure (items 4 and 7), the repo-side exemption list (5), the `cli-flag-verify` prompt parsing and wrong line numbers (6), standalone drift detection for consumers (10), advisory stale-path findings (12), Codex and Cursor manifest discovery (13), the commit-msg rejection naming the wrong source file (14), the LFS/TMPDIR checkout failing open, and slowness. The code itself looks correct and the test covers the new guard.
- PR body: add a section listing the remaining work of #9426, or move this PR to `Refs:` an issue it fully delivers. Remaining items include the reference-verify jq argument limit (items 4 and 7), the repo-side exemption list (5), cli-flag-verify reading flags inside a quoted prompt and reporting wrong line numbers (6), a standalone drift check for consumers (10), advisory stale-path findings (12), Codex/Cursor manifest discovery (13), the commit-msg rejection naming the wrong source file (14), the LFS/TMPDIR checkout failing open, and the pre-commit and reference-verify slowness.
- PR body: also say which part of #9426 this delivers. The closest match is item 11 (the hook jobs run the branch's own copy), applied to this repository acting as its own remote. As written, the body explains the self-hosting change but doesn't tie it to an item in the issue.
- .lefthook/pre-commit/guardrails-pre-commit-content.sh:7: the guard trusts any path containing `*/info/lefthook-remotes/*`. That is fine for lefthook's cache layout, but it is a path-string check, not a provenance check. The README note in the diff about reading `.lefthook/` changes before committing on untrusted branches is the real mitigation; keep it.

Comment thread lefthook.yml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pr-verify verifier failed head b9bbf778cb52f836dfc659511f9214f5c13b0ec4. What it wrote about this file, as data:

- lefthook.yml:121: this repository now runs hooks fetched from its own `main` with `refetch_frequency: 24h`. Each clone's hooks therefore follow `main` within a day, in line with the owner's decision recorded on #9426.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pr-verify verifier failed head b9bbf778cb52f836dfc659511f9214f5c13b0ec4. What it wrote about this file, as data:

- plugins/guardrails/git-hooks/lefthook-remote.test.sh:136: `trap 'rm -rf "$GUARD"' EXIT` replaces any EXIT trap set earlier in this test. Check that no earlier cleanup trap exists, or chain onto it.

Merged via the queue into main with commit c963d90 Oct 24, 2026
55 of 66 checks passed
@kyle-sexton
kyle-sexton deleted the chore/guardrails-self-remote branch October 24, 2026 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant