From 3f0cb6936335c2e77915daa1acad3400000499cf Mon Sep 17 00:00:00 2001 From: Anurag Singh Date: Tue, 27 Oct 2026 15:05:54 +0530 Subject: [PATCH 1/4] feat(hook-manager): add a lefthook adapter Why: lefthook refuses to install while the whole-user scope's global core.hooksPath is set. The shim hides that value from lefthook's scoped git config read, so lefthook's own target resolves to the repository's common hooks directory, linked worktrees included. The registry carries the adapter with no home because lefthook keeps nothing outside the repository, and the new lefthook-hooks-path agent guard blocks the two flags that would damage the setup: --force writes over the dispatcher and --reset-hooks-path unsets it. The contract records the npm postinstall path the shim cannot reach. Generated-by: DeepSeek V4.1 Flash --- docs/labels-and-capabilities.md | 2 +- docs/setup/sandbox-troubleshooting.md | 64 ++++++++- docs/setup/secure-agent-setup.md | 11 +- .../agent-guard/src/agent_guard/__init__.py | 32 ++++- .../guards/lefthook_hooks_path.py | 80 +++++++++++ .../step-p-sandbox-allowlists.md | 38 +++--- tools/agent-guard/README.md | 2 + tools/agent-guard/src/agent_guard/__init__.py | 32 ++++- tools/agent-guard/tests/test_skill_guards.py | 74 +++++++++- tools/agent-isolation/README.md | 4 +- tools/agent-isolation/hook-manager-shim.sh | 73 ++++++---- .../tests/test_hook_manager_shim.py | 129 ++++++++++++++++++ tools/dev/sync-hook-managers.py | 25 ++-- tools/dev/tests/test_sync_hook_managers.py | 11 +- tools/hook-manager/README.md | 20 ++- tools/hook-manager/registry.json | 34 +++++ .../specs/agent-isolation-sandbox.md | 9 +- 17 files changed, 565 insertions(+), 75 deletions(-) create mode 100644 plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py diff --git a/docs/labels-and-capabilities.md b/docs/labels-and-capabilities.md index 85951ea47..522538c1c 100644 --- a/docs/labels-and-capabilities.md +++ b/docs/labels-and-capabilities.md @@ -389,7 +389,7 @@ or a contract-free mix of substrates (e.g. `tools/spec-inventory` is | [`tools/skill-reconciler-diff`](../tools/skill-reconciler-diff/) | `substrate:framework-dev` | Deterministic structural diff between two skill trees — parses frontmatter, section headings, step inventory, placeholders, support files, and safety-baseline clauses into a JSON diff object for the `skill-reconciler` skill | | [`tools/vcs`](../tools/vcs/) | `contract:source-control` | Backend-dispatching implementation of the source-control (VCS) capability ([`tools/github/source-control.md`](../tools/github/source-control.md)); complete Git, Mercurial (Hg), Fossil, and Subversion (SVN) backends | | [`tools/sourcehut`](../tools/sourcehut/) | `contract:tracker` + `contract:source-control` + `contract:mail-archive` | SourceHut (sr.ht) forge bridge: todo.sr.ht, lists.sr.ht, builds.sr.ht, and git/hg repository reads | -| [`tools/hook-manager`](../tools/hook-manager/) | `contract:hook-manager` | Contract for git-hook managers under the secure agent setup: the install-target and home rules, and the registry of adapters (prek today) that the hook-manager shim, the whole-user dispatcher and `sandbox-lint` are generated from | +| [`tools/hook-manager`](../tools/hook-manager/) | `contract:hook-manager` | Contract for git-hook managers under the secure agent setup: the install-target and home rules, and the registry of adapters (prek and lefthook today) that the hook-manager shim, the whole-user dispatcher and `sandbox-lint` are generated from | | [`tools/tracker`](../tools/tracker/) | `contract:tracker` | Interface spec for the tracker contract's read-only contributor-activity queries (issues filed, triaged and commented on; issues created; first maintainer reply). Pure interface spec; GitHub and Jira implement it, and the rest of the contract is described by `tools/github/tool.md` | | [`tools/body-field-contract`](../tools/body-field-contract/) | `contract:body-field` | The body-field contract: how every reader and writer finds a tracker body's `### ` fields, with a language-neutral conformance suite run against every implementation. Pure interface spec; implementations: `tools/github-body-field/` (reference), `tools/vetted-ops/`, `tools/local-tracker/`, `tools/jira/` | | [`tools/security-tracker`](../tools/security-tracker/) | `contract:security-tracker` | Adapter contract for the security tracker the `security-*` skills read and write, selected by `tracker.platform`. Pure interface spec; backends: `tools/github/` (default), `tools/local-tracker/`, `tools/jira/` | diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index a94d08a2b..98a7fdf48 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -61,19 +61,24 @@ - [Root cause](#root-cause-10) - [Fix](#fix-10) - [Notes](#notes-9) - - [Reads of `~/.claude/magpie` or `/tmp/claude-` ask for approval every time](#reads-of-claudemagpie-or-tmpclaude-uid-ask-for-approval-every-time) + - [`lefthook install` refuses with "Custom hooks paths are not supported by default"](#lefthook-install-refuses-with-custom-hooks-paths-are-not-supported-by-default) - [Symptom](#symptom-11) - [Root cause](#root-cause-11) - [Fix](#fix-11) - [Notes](#notes-10) - - [Every Bash call fails with `bwrap: Can't create file … Read-only file system`](#every-bash-call-fails-with-bwrap-cant-create-file--read-only-file-system) + - [Reads of `~/.claude/magpie` or `/tmp/claude-` ask for approval every time](#reads-of-claudemagpie-or-tmpclaude-uid-ask-for-approval-every-time) - [Symptom](#symptom-12) - [Root cause](#root-cause-12) - [Fix](#fix-12) - - [`git status` shows files you never created, such as `.mcp.json` or `.bash_profile`](#git-status-shows-files-you-never-created-such-as-mcpjson-or-bash_profile) + - [Notes](#notes-11) + - [Every Bash call fails with `bwrap: Can't create file … Read-only file system`](#every-bash-call-fails-with-bwrap-cant-create-file--read-only-file-system) - [Symptom](#symptom-13) - [Root cause](#root-cause-13) - [Fix](#fix-13) + - [`git status` shows files you never created, such as `.mcp.json` or `.bash_profile`](#git-status-shows-files-you-never-created-such-as-mcpjson-or-bash_profile) + - [Symptom](#symptom-14) + - [Root cause](#root-cause-14) + - [Fix](#fix-14) - [Adding a new entry](#adding-a-new-entry) @@ -1236,6 +1241,59 @@ resolved path. Confirm with `git hook run pre-commit` from the agent. --- +## `lefthook install` refuses with "Custom hooks paths are not supported by default" + +### Symptom + +A repository that uses lefthook has no working hooks, and `lefthook install` +fails: + +```console +$ lefthook install +core.hooksPath is set globally to '/Users//.claude/git-hooks' +Custom hooks paths are not supported by default. +... +``` + +### Root cause + +Whole-user scope sets a global `core.hooksPath`, and lefthook refuses to install +while a scoped `git config --global` read returns one. +That is the safe failure: the alternative, `--force`, writes lefthook's hook +into the shared directory and replaces the framework's dispatcher for every +repository on the host. + +### Fix + +Run `lefthook install` through the framework shim, which hides the global config +from that read and leaves the hook in the repository's own +`$(git rev-parse --git-common-dir)/hooks/`: + +```bash +~/.claude/bin/lefthook install +``` + +With `~/.claude/bin` ahead of the real binary on PATH, plain `lefthook install` +does that already. +The setup installs the shim only for a repository that uses lefthook: a root +`lefthook.*`, `.lefthook.*` or `-local` config, or an installed hook mentioning +`call_lefthook`. + +### Notes + +- Do not run `lefthook install --reset-hooks-path` or `--force` to get past the + error: the first unsets the whole-user setup, and the second overwrites the + dispatcher for every repository. + The agent-guard `lefthook-hooks-path` guard blocks both, and needs + `MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1` to proceed deliberately. +- An npm `postinstall` runs `node_modules/.bin/lefthook`, which the shim does + not cover; that install refuses for the same reason, and the repository needs + one run through the shim. +- Per-project scope has no global `core.hooksPath`, so the refusal does not + happen there. + +--- + ## Reads of `~/.claude/magpie` or `/tmp/claude-` ask for approval every time ### Symptom diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index a1701791c..de6317696 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -1438,10 +1438,19 @@ PATH) that rewrites **only** `prek install` — injecting in the repo-local `.git/hooks/` where the dispatcher finds it. Every other `prek` command passes straight through, and the shim is a no-op on hosts with no global `core.hooksPath`. +lefthook refuses to install at all while a global `core.hooksPath` is set, so +the same shim installed as `~/.claude/bin/lefthook` runs `lefthook install` +with `GIT_CONFIG_GLOBAL=/dev/null`; lefthook then resolves its own target to the +common dir. +`lefthook install --force` and `--reset-hooks-path` would still get that +override through the shim (they cannot reach the shared dir or unset the setup), +and the agent-guard `lefthook-hooks-path` guard blocks an agent from trying. +The shim covers the CLI only: an npm `postinstall` runs +`node_modules/.bin/lefthook`, ahead of `~/.claude/bin` on PATH. The shim, the host-only homes and the sandbox-lint checks all follow the [hook-manager contract](../../tools/hook-manager/README.md): one script serves every adapter in its registry, keyed by the name it is -installed under, and other hook managers (pre-commit, lefthook, husky, …) +installed under, and other hook managers (pre-commit, husky, …) join by adding an adapter there. Validated behaviour under this flavour (`core.hooksPath` global + diff --git a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py index cc847e17f..d7cba00ae 100644 --- a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py +++ b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py @@ -2337,8 +2337,9 @@ def guard_vetted_ops_spelling(seg: Segment, cwd: str | None = None, harness: str # Only commands in these families are inspected; everything else takes the # instant fast path. Bundled and contributed guards alike operate on the -# `gh` / `git` outbound/destructive surface. -GUARDED_HEADS = frozenset({"gh", "git"}) +# `gh` / `git` outbound/destructive surface and on `lefthook`, whose +# `install` can undo the whole-user hook setup. +GUARDED_HEADS = frozenset({"gh", "git", "lefthook"}) # Colon-separated extra guard directories, searched before the discovered ones. # Skill-owned guards no longer need it (``framework_root`` finds them in place); @@ -2597,12 +2598,30 @@ def run(self, args: list[str]) -> str | None: return _run(args, cwd=self.cwd) +def guarded_head(argv: list[str]) -> str | None: + """The guarded command family a segment runs, or None. + + The head is usually the bare name, but a manager installed from a package + tree is reached by path (``node_modules/.bin/lefthook``), so the basename + counts too. + """ + if not argv: + return None + head = argv[0] + if head in GUARDED_HEADS: + return head + base = head.rsplit("/", 1)[-1] + return base if base in GUARDED_HEADS else None + + def command_kinds(seg: Segment) -> set[str]: """The command-family tags a segment matches, e.g. ``{"git", "git:commit"}``.""" kinds: set[str] = set() if not seg.argv: return kinds - head = seg.argv[0] + head = guarded_head(seg.argv) + if head is None: + return kinds kinds.add(head) if head == "git": # A TRIGGERS = ["git:commit"] contributed guard must still fire behind @@ -2615,6 +2634,11 @@ def command_kinds(seg: Segment) -> set[str]: # Same for TRIGGERS = ["gh:pr"] behind `gh -R pr ...`. sub = gh_subcommand(seg.argv) kinds.add(f"{head}:{sub[0] if sub else seg.argv[1]}") + elif head == "lefthook": + # lefthook accepts global flags before the subcommand. + lefthook_sub = next((tok for tok in seg.argv[1:] if not tok.startswith("-")), None) + if lefthook_sub: + kinds.add(f"lefthook:{lefthook_sub}") return kinds @@ -2782,7 +2806,7 @@ def dispatch(command: str, cwd: str | None = None, harness: str | None = None) - if reason: return reason continue - if not seg.argv or seg.argv[0] not in GUARDED_HEADS: + if not seg.argv or guarded_head(seg.argv) is None: continue # fast path — non-guarded command family # Bundled guards (trusted, in-process; each self-filters its command). for builtin in BUILTIN_GUARDS: diff --git a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py new file mode 100644 index 000000000..64182ea86 --- /dev/null +++ b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py @@ -0,0 +1,80 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""setup-isolated-setup-install lefthook hooks-path guard (skill-contributed). + +`lefthook install --force` writes lefthook's hook into the directory the global +`core.hooksPath` names, over the framework's dispatcher, and +`--reset-hooks-path` unsets the whole-user setup. Both are destructive when +that path is the framework's own. Discovered by the agent-guard dispatcher; +import-free (uses ``ctx``). +""" + +import os + +TRIGGERS = ["lefthook:install"] + +FRAMEWORK_HOOKS_PATHS = ("~/.claude/git-hooks", "~/.claude-config/git-hooks") + + +def _install_arguments(argv): + """The arguments after the ``install`` subcommand, or an empty list.""" + for index, token in enumerate(argv[1:], start=1): + if token.startswith("-"): + continue + return argv[index + 1 :] if token == "install" else [] + return [] + + +def _destructive_flag(argument): + if argument in ("-f", "--force"): + return "--force" + if argument in ("-r", "--reset-hooks-path"): + return "--reset-hooks-path" + if argument.startswith("-") and not argument.startswith("--"): + if "f" in argument[1:]: + return "--force" + if "r" in argument[1:]: + return "--reset-hooks-path" + return None + + +def _is_framework_path(value): + actual = os.path.realpath(os.path.expanduser(value)) + return any(actual == os.path.realpath(os.path.expanduser(name)) for name in FRAMEWORK_HOOKS_PATHS) + + +def guard(ctx): + flag = None + for argument in _install_arguments(ctx.argv): + flag = _destructive_flag(argument) + if flag: + break + if flag is None: + return None + if ctx.override("MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH"): + return None + hooks_path = ctx.run(["git", "config", "--global", "--get", "core.hooksPath"]) + if not hooks_path or not _is_framework_path(hooks_path): + return None + effect = "unset" if flag == "--reset-hooks-path" else "replace" + return ( + f"agent-guard[lefthook-hooks-path]: `lefthook install {flag}` would {effect} the " + f"framework's global core.hooksPath ({hooks_path}). Run plain `lefthook install`: " + "the ~/.claude/bin/lefthook shim keeps the hook in this repository's own hooks " + "dir. Override: MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1." + ) diff --git a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md index 8238917ff..142b73bf8 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md +++ b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md @@ -103,10 +103,10 @@ acknowledge it explicitly; no silent proceed: > runs the framework's own logic (the `post-checkout` > sandbox-allowlist sync) **and then chains through to that > repo's own `.git/hooks/`**. Your per-repo hooks keep -> firing, and a repo with no hook is a clean no-op. A bundled -> `prek` PATH shim makes `prek install` write its shim into the -> repo-local `.git/hooks/` (via `--git-dir`) so the dispatcher -> can find it. +> firing, and a repo with no hook is a clean no-op. Bundled +> PATH shims for `prek` and `lefthook` make their `install` write +> into the repo-local `.git/hooks/` (prek via `--git-dir`, lefthook +> with the global config hidden) so the dispatcher can find it. > > Either way, whole-user scope is reversible: > `git config --global --unset core.hooksPath` restores per-repo @@ -326,30 +326,36 @@ for the full rationale + the validated behaviour matrix. with the original argv + inherited stdin, so a failing local `pre-commit` / `pre-push` still aborts the git operation. -2. **Install the hook-manager PATH shim** so `prek install` writes its - shim into the repo-local `.git/hooks/` (where the dispatcher +2. **Install the hook-manager PATH shim** so a manager's `install` writes its + hook into the repo-local `.git/hooks/` (where the dispatcher chains) instead of the shared dir. Copy [`tools/agent-isolation/hook-manager-shim.sh`](../../../../tools/agent-isolation/hook-manager-shim.sh) into `~/.claude/bin/` (or symlink from `~/.claude-config/bin/`), mode `0755`, once for each adapter in the [hook-manager registry](../../../../tools/hook-manager/registry.json) with an install strategy other than `none` - that a repository the operator works in uses — `~/.claude/bin/prek` today. + that a repository the operator works in uses. A manager whose install strategy is `none` needs no install fix and gets no shim. A repository uses a manager when one of its config files is at the root (or tracked below it, for prek's nested projects) or one of its installed hooks runs it ([Detection](../../../../tools/hook-manager/README.md#detection)); for prek that is `prek.toml`, `.pre-commit-config.yaml` or - `.pre-commit-config.yml`. Install no shim for a manager no repository uses. The shim - takes the tool from the name it is installed under. For prek it - rewrites only `prek install` (injecting - `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller - already passed `--git-dir`, asked for `--help`, or is outside a - git work tree); **every other `prek` invocation passes through - unchanged**. It is a no-op on hosts with no global - `core.hooksPath`. A host that installed the older `prek-shim.sh` - as `~/.claude/bin/prek` keeps working; replacing it is optional. + `.pre-commit-config.yml`; for lefthook, its root `lefthook.*`, + `.lefthook.*` and `-local` names. + Install no shim for a manager no repository uses. + The shim takes the tool from the name it is installed under and rewrites only + the install subcommand: for prek it injects + `--git-dir "$(git rev-parse --git-common-dir)"`; for lefthook it runs the + install with `GIT_CONFIG_GLOBAL=/dev/null`, which hides the global + `core.hooksPath` from lefthook's own check and lets its target resolve to the + common dir. + Both leave the install alone when the caller asked for `--help` or is outside + a git work tree (prek also when `--git-dir` is already passed), and every + other invocation passes through unchanged. + It is a no-op on hosts with no global `core.hooksPath`. + A host that installed the older `prek-shim.sh` as `~/.claude/bin/prek` keeps + working; replacing it is optional. 3. **Surface the PATH line for the operator to add** to their `~/.bashrc` / `~/.zshrc` (per the golden rules — never edit the diff --git a/tools/agent-guard/README.md b/tools/agent-guard/README.md index 3c8e317b4..6ba1cdec5 100644 --- a/tools/agent-guard/README.md +++ b/tools/agent-guard/README.md @@ -136,6 +136,7 @@ way — see [Contributing guards](#contributing-guards)): | `mention` | `pr-management-triage` | `gh pr comment` / `gh issue comment` that `@`-mentions anyone other than the PR/issue author; **any** `@`-mention in `gh pr edit --body[-file]` | denoise: author-directed feedback never pings maintainers; body edits stay silent. Exempt: the operator commenting on their **own** PR/issue (author == authenticated `gh` user), and the `MAGPIE_ALLOW_MENTIONS=1` override | | `mark-ready` | `pr-management-triage` | adding `ready for maintainer review` while the PR head SHA has GitHub Actions runs awaiting approval | Golden rule 1b | | `security-language` | `security-issue-fix` | a CVE id / security-fix language in a **public** `gh pr create`/`gh pr edit` title/body (not comments) | public-PR scrubbing | +| `lefthook-hooks-path` | `setup-isolated-setup-install` | `lefthook install --force` / `-f` or `--reset-hooks-path` / `-r` while the global `core.hooksPath` is the framework's | the first writes over the dispatcher for every repository, the second unsets the whole-user setup; the shim hides the global path, so a plain `lefthook install` is all that is needed. Override: `MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1` | A denied command is **not** posted/run; the model is shown the reason and the deterministic fix (e.g. "use a backtick `` `login` `` instead of `@login`"). @@ -223,6 +224,7 @@ MAGPIE_ALLOW_MENTIONS=1 gh pr comment 123 --body "@reviewer please take anot MAGPIE_ALLOW_COAUTHOR=1 git commit -m "…" # not for AI co-authorship MAGPIE_ALLOW_MARK_READY=1 gh pr edit 123 --add-label "ready for maintainer review" MAGPIE_ALLOW_SECURITY_LANG=1 gh pr create --title "…" # disclosure already public +MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1 lefthook install --reset-hooks-path # whole-user setup is being removed on purpose MAGPIE_ALLOW_EMPTY_PUSH=1 git push --force … MAGPIE_ALLOW_VETTED_PATH=1 uv run --project tools/vetted-ops vetted-op-read … # developing the dispatcher ``` diff --git a/tools/agent-guard/src/agent_guard/__init__.py b/tools/agent-guard/src/agent_guard/__init__.py index cc847e17f..d7cba00ae 100644 --- a/tools/agent-guard/src/agent_guard/__init__.py +++ b/tools/agent-guard/src/agent_guard/__init__.py @@ -2337,8 +2337,9 @@ def guard_vetted_ops_spelling(seg: Segment, cwd: str | None = None, harness: str # Only commands in these families are inspected; everything else takes the # instant fast path. Bundled and contributed guards alike operate on the -# `gh` / `git` outbound/destructive surface. -GUARDED_HEADS = frozenset({"gh", "git"}) +# `gh` / `git` outbound/destructive surface and on `lefthook`, whose +# `install` can undo the whole-user hook setup. +GUARDED_HEADS = frozenset({"gh", "git", "lefthook"}) # Colon-separated extra guard directories, searched before the discovered ones. # Skill-owned guards no longer need it (``framework_root`` finds them in place); @@ -2597,12 +2598,30 @@ def run(self, args: list[str]) -> str | None: return _run(args, cwd=self.cwd) +def guarded_head(argv: list[str]) -> str | None: + """The guarded command family a segment runs, or None. + + The head is usually the bare name, but a manager installed from a package + tree is reached by path (``node_modules/.bin/lefthook``), so the basename + counts too. + """ + if not argv: + return None + head = argv[0] + if head in GUARDED_HEADS: + return head + base = head.rsplit("/", 1)[-1] + return base if base in GUARDED_HEADS else None + + def command_kinds(seg: Segment) -> set[str]: """The command-family tags a segment matches, e.g. ``{"git", "git:commit"}``.""" kinds: set[str] = set() if not seg.argv: return kinds - head = seg.argv[0] + head = guarded_head(seg.argv) + if head is None: + return kinds kinds.add(head) if head == "git": # A TRIGGERS = ["git:commit"] contributed guard must still fire behind @@ -2615,6 +2634,11 @@ def command_kinds(seg: Segment) -> set[str]: # Same for TRIGGERS = ["gh:pr"] behind `gh -R pr ...`. sub = gh_subcommand(seg.argv) kinds.add(f"{head}:{sub[0] if sub else seg.argv[1]}") + elif head == "lefthook": + # lefthook accepts global flags before the subcommand. + lefthook_sub = next((tok for tok in seg.argv[1:] if not tok.startswith("-")), None) + if lefthook_sub: + kinds.add(f"lefthook:{lefthook_sub}") return kinds @@ -2782,7 +2806,7 @@ def dispatch(command: str, cwd: str | None = None, harness: str | None = None) - if reason: return reason continue - if not seg.argv or seg.argv[0] not in GUARDED_HEADS: + if not seg.argv or guarded_head(seg.argv) is None: continue # fast path — non-guarded command family # Bundled guards (trusted, in-process; each self-filters its command). for builtin in BUILTIN_GUARDS: diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index 99cdb3193..63a62d3c8 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -38,6 +38,11 @@ if (REPO_ROOT / "plugins" / "magpie-security" / "skills" / "issue-fix" / "guards").is_dir() else REPO_ROOT / "skills" / "security-issue-fix" / "guards" ) +SETUP_GUARDS = ( + REPO_ROOT / "plugins" / "magpie-setup" / "skills" / "isolated-setup-install" / "guards" + if (REPO_ROOT / "plugins" / "magpie-setup" / "skills" / "isolated-setup-install" / "guards").is_dir() + else REPO_ROOT / "skills" / "setup-isolated-setup-install" / "guards" +) @pytest.fixture(autouse=True) @@ -47,9 +52,13 @@ def _wire_skill_guards(monkeypatch): "MAGPIE_ALLOW_MENTIONS", "MAGPIE_ALLOW_MARK_READY", "MAGPIE_ALLOW_SECURITY_LANG", + "MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH", ): monkeypatch.delenv(name, raising=False) - monkeypatch.setenv("MAGPIE_GUARD_DIRS", f"{TRIAGE_GUARDS}{os.pathsep}{SECURITY_GUARDS}") + monkeypatch.setenv( + "MAGPIE_GUARD_DIRS", + f"{TRIAGE_GUARDS}{os.pathsep}{SECURITY_GUARDS}{os.pathsep}{SETUP_GUARDS}", + ) def fake_run(handler): @@ -80,6 +89,7 @@ def test_guard_files_exist(): assert (TRIAGE_GUARDS / "mention.py").is_file() assert (TRIAGE_GUARDS / "mark_ready.py").is_file() assert (SECURITY_GUARDS / "security_language.py").is_file() + assert (SETUP_GUARDS / "lefthook_hooks_path.py").is_file() # --- mention guard (skill-owned) ------------------------------------------- # @@ -566,3 +576,65 @@ def test_prefix_wrappers_reach_the_mention_guard(monkeypatch, prefix): def test_an_override_on_env_still_counts(monkeypatch): monkeypatch.setattr(agent_guard, "_run", fake_run(gh_stub(author="alice"))) assert dispatch('env MAGPIE_ALLOW_MENTIONS=1 gh pr comment 5 --body "@bob ping"') is None + + +# --- lefthook hooks-path guard (skill-owned) ------------------------------- # + +FRAMEWORK_HOOKS = "~/.claude/git-hooks" + + +def hooks_path_stub(value): + def handler(args): + if args[:2] == ["git", "config"]: + return value + return None + + return handler + + +@pytest.mark.parametrize("hooks_path", [FRAMEWORK_HOOKS, "~/.claude-config/git-hooks"]) +@pytest.mark.parametrize( + "command, flag", + [ + ("lefthook install --force", "--force"), + ("lefthook install -f", "--force"), + ("lefthook install --reset-hooks-path", "--reset-hooks-path"), + ("lefthook install -r", "--reset-hooks-path"), + ("lefthook install -fr", "--force"), + ("lefthook --verbose install --force", "--force"), + ("./node_modules/.bin/lefthook install --force", "--force"), + ], +) +def test_lefthook_destructive_install_denied(monkeypatch, hooks_path, command, flag): + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(hooks_path))) + reason = dispatch(command) + assert reason and "lefthook-hooks-path" in reason and flag in reason + + +@pytest.mark.parametrize( + "command", + [ + "lefthook install", + "lefthook install --verbose", + "lefthook run pre-commit", + "lefthook --version", + ], +) +def test_lefthook_plain_invocations_allowed(monkeypatch, command): + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(FRAMEWORK_HOOKS))) + assert dispatch(command) is None + + +def test_lefthook_destructive_install_allowed_for_another_global_path(monkeypatch): + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub("/opt/other-hooks"))) + assert dispatch("lefthook install --force") is None + + +def test_lefthook_destructive_install_allowed_without_a_global_path(monkeypatch): + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(None))) + assert dispatch("lefthook install --reset-hooks-path") is None + + +def test_lefthook_destructive_install_override(monkeypatch): + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(FRAMEWORK_HOOKS))) + assert dispatch("MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1 lefthook install --force") is None diff --git a/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index 226353a7a..e70f4951b 100644 --- a/tools/agent-isolation/README.md +++ b/tools/agent-isolation/README.md @@ -97,8 +97,8 @@ per runtime — see [`docs/adapters/add-a-harness.md`](../../docs/adapters/add-a | [`claude-term-bg.sh`](claude-term-bg.sh) | **Opt-in quality-of-life helper (not a security control).** Keeps a calm baseline background and tints it only when Claude genuinely wants you to act (never while working, and never when it merely *finished* a turn), so a window you've tabbed away from can't sit blocked unnoticed. Distinguishes "blocked on a decision" from "finished and idle" — which look identical at the `Stop` event — via three signals across six hooks: `Stop` → `stop` (heuristic — tints only if the final assistant message reads as a question/request; a completion stays calm; needs `python3`/`python`, else defaults calm); `PreToolUse` (matcher `AskUserQuestion`) → `wait` (exact — a structured question was posed); `PostToolUse` (matcher `*`) → `reset` (calm while working, and clears the tint the instant you approve a permission prompt or answer a question); `Notification` → `notify` (tints for permission prompts only — the plain idle ping is a no-op so it can't wipe a pending question's tint); and `UserPromptSubmit` + `SessionStart` → `reset` (you replied / fresh session clears any stale tint). Writes the OSC escape to the Claude pty discovered by walking the process tree (hooks have no controlling tty); the only deterministic reset is an explicit `CLAUDE_RESET_BG` colour via OSC 11 (iTerm2 ignores OSC 111). Colours overridable via `CLAUDE_WAIT_BG` / `CLAUDE_RESET_BG`. Tested on iTerm2 + macOS; fail-soft elsewhere. See [`docs/setup/secure-agent-setup.md` → *Waiting-for-input terminal tint*](../../docs/setup/secure-agent-setup.md#waiting-for-input-terminal-tint). | | [`sandbox-add-project-root.sh`](sandbox-add-project-root.sh) | Adds the current adopter repo's project root (and, with `--all-worktrees`, every linked git worktree's working dir) as an explicit absolute path to `sandbox.filesystem.allowRead` and `allowWrite` in the project-local, gitignored `/.claude/settings.local.json` — one entry per worktree, each in that worktree's own settings file. Defensive against [issue #197](https://github.com/apache/magpie/issues/197) — `allowRead: ["."]` does not in practice cover CWD because the harness pre-resolves the `.` literal away from the read side. The same harness behaviour drops the committed home-directory entries, so it also adds the dev-tool paths `prek` and `uv` need (`~/.gitconfig`, `~/.config/git`, `~/.cache`, `~/.local/share/uv`, `~/.local/bin`; the cache and uv dirs writable) as absolute paths — never the credential paths; `--no-tool-paths` skips them. See [`prek` or `uv` not found inside the sandbox](../../docs/setup/sandbox-troubleshooting.md#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox). Never modifies user-scope or committed project-scope. Idempotent, atomic, tolerant of missing prereqs. Invoked from `setup-isolated-setup-install`, `/magpie-setup:setup` (adopt / upgrade / worktree-init), and the `post-checkout` git hook installed by `/magpie-setup:setup adopt`. | | [`git-global-post-checkout.sh`](git-global-post-checkout.sh) | Universal `post-checkout` git hook installed at `~/.claude/git-hooks/post-checkout` when the operator picks the **simple whole-user** flavour in `setup-isolated-setup-install`. Activated by `git config --global core.hooksPath ~/.claude/git-hooks/` so every `git checkout` / `git clone` / `git worktree add` across the host invokes it. Best-effort + idempotent + `\|\| true`: invokes `sandbox-add-project-root.sh` for any worktree with a `.claude/` directory. Trade-off documented in [`docs/setup/secure-agent-setup.md` → *Per-project vs whole-user scope*](../../docs/setup/secure-agent-setup.md#per-project-vs-whole-user-scope): `core.hooksPath` shadows per-repo `.git/hooks/*` across every repo on the host. The **dispatcher** flavour (below) supersedes this file. | -| [`git-hook-dispatcher.sh`](git-hook-dispatcher.sh) | Universal, basename-keyed git-hook **dispatcher** for the **whole-user dispatcher** flavour. Symlinked to every hook name under `~/.claude/git-hooks/`. For each git operation it runs the framework's own logic for that hook type (the `post-checkout` sandbox-allowlist sync) **and then chains through to the repo-local `.git/hooks/`** (resolved via `git rev-parse --git-common-dir`, worktree-safe; `exec`-ed with original argv + inherited stdin so a failing local hook still aborts the git op). Restores per-repo hooks (prek / pre-commit / husky / hand-written) under global `core.hooksPath`; a repo with no local hook is a clean no-op. Outside the sandbox it also gives every hook manager in the [hook-manager registry](../hook-manager/registry.json) its host-only home (the contract's home rule). See [`docs/setup/secure-agent-setup.md` → *Whole-user with the per-repo dispatcher*](../../docs/setup/secure-agent-setup.md#whole-user-with-the-per-repo-dispatcher). | -| [`hook-manager-shim.sh`](hook-manager-shim.sh) | Transparent front for a git-hook manager, installed as `~/.claude/bin/` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour, one install per adapter in the [hook-manager registry](../hook-manager/registry.json) whose install strategy is not `none` (`prek` today). The tool is the name it is installed under. Rewrites **only** the adapter's install subcommand so the hook lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir — for prek, injecting `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`, asked for `--help`, or is outside a git work tree. Every other invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. Implements the install-target rule of the [hook-manager contract](../hook-manager/README.md). | +| [`git-hook-dispatcher.sh`](git-hook-dispatcher.sh) | Universal, basename-keyed git-hook **dispatcher** for the **whole-user dispatcher** flavour. Symlinked to every hook name under `~/.claude/git-hooks/`. For each git operation it runs the framework's own logic for that hook type (the `post-checkout` sandbox-allowlist sync) **and then chains through to the repo-local `.git/hooks/`** (resolved via `git rev-parse --git-common-dir`, worktree-safe; `exec`-ed with original argv + inherited stdin so a failing local hook still aborts the git op). Restores per-repo hooks (prek / pre-commit / lefthook / husky / hand-written) under global `core.hooksPath`; a repo with no local hook is a clean no-op. Outside the sandbox it also gives every hook manager in the [hook-manager registry](../hook-manager/registry.json) its host-only home (the contract's home rule). See [`docs/setup/secure-agent-setup.md` → *Whole-user with the per-repo dispatcher*](../../docs/setup/secure-agent-setup.md#whole-user-with-the-per-repo-dispatcher). | +| [`hook-manager-shim.sh`](hook-manager-shim.sh) | Transparent front for a git-hook manager, installed as `~/.claude/bin/` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour, one install per adapter in the [hook-manager registry](../hook-manager/registry.json) whose install strategy is not `none` (`prek` and `lefthook` today). The tool is the name it is installed under. Rewrites **only** the adapter's install subcommand so the hook lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir: for prek it injects `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`; for lefthook it runs the install with `GIT_CONFIG_GLOBAL=/dev/null` so lefthook's own `core.hooksPath` refusal does not fire. Both leave help output and a non-repository invocation alone. Every other invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. Implements the install-target rule of the [hook-manager contract](../hook-manager/README.md). | ## Usage at a glance diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index ed4a83a41..2b3b72592 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -17,10 +17,11 @@ # under the License. # # hook-manager-shim.sh — a transparent front for a git-hook manager -# (prek today; see tools/hook-manager/registry.json), installed as -# `~/.claude/bin/` with `~/.claude/bin` prepended to PATH so it -# shadows the real binary. The tool is the name it is installed -# under (`basename "$0"`), so one script serves every adapter. +# (prek and lefthook today; see tools/hook-manager/registry.json), +# installed as `~/.claude/bin/` with `~/.claude/bin` prepended +# to PATH so it shadows the real binary. The tool is the name it is +# installed under (`basename "$0"`), so one script serves every +# adapter. # # WHY THIS EXISTS # --------------- @@ -48,6 +49,13 @@ # checkout's hooks dir) — UNLESS the caller already chose a # target, asked for help, or we are not inside a git work tree. # git-dir-flag: inject ` ` (prek: --git-dir) +# hide-global-config: run the install with +# GIT_CONFIG_GLOBAL=/dev/null, for a manager that writes to +# the common dir itself but refuses while a global +# core.hooksPath is set (lefthook). The override applies even +# with `--force` / `--reset-hooks-path`: through this shim +# those must not write into the shared dir or unset the +# whole-user setup. Run the real binary to bypass it. # * every other invocation (`prek run`, `prek autoupdate`, # `--version`, bare `prek`, ...) is passed through to the real # binary **unchanged**. @@ -65,6 +73,7 @@ tool="$(basename "$0")" # BEGIN generated by tools/dev/sync-hook-managers.py; do not edit adapter() { case "$1" in + lefthook) install_subcommand=install; install_strategy=hide-global-config ;; prek) install_subcommand=install; install_strategy=git-dir-flag; install_flag=--git-dir ;; *) return 1 ;; esac @@ -104,28 +113,42 @@ fi shift # drop the install subcommand -# Respect a target the caller chose, and never interfere with help output. -inject=1 -for _a in "$@"; do - case "$_a" in - -h|--help) inject=0; break ;; - "$install_flag"|"$install_flag"=*) inject=0; break ;; - esac -done +# --- install strategies: generated branches keyed by install_strategy ------ +case "$install_strategy" in + git-dir-flag) + # Respect a target the caller chose, and never interfere with help output. + inject=1 + for _a in "$@"; do + case "$_a" in + -h|--help) inject=0; break ;; + "$install_flag"|"$install_flag"=*) inject=0; break ;; + esac + done -if [ "$inject" = 1 ]; then - if gitdir="$(git rev-parse --git-common-dir 2>/dev/null)"; then - case "$gitdir" in - /*) : ;; # already absolute - *) gitdir="$(cd "$gitdir" 2>/dev/null && pwd -P || echo "$gitdir")" ;; - esac - case "$install_strategy" in - git-dir-flag) + if [ "$inject" = 1 ]; then + if gitdir="$(git rev-parse --git-common-dir 2>/dev/null)"; then + case "$gitdir" in + /*) : ;; # already absolute + *) gitdir="$(cd "$gitdir" 2>/dev/null && pwd -P || echo "$gitdir")" ;; + esac exec "$real_tool" "$install_subcommand" "$install_flag" "$gitdir" "$@" - ;; - esac - fi - # not in a git work tree: let the real tool produce its own error -fi + fi + # not in a git work tree: let the real tool produce its own error + fi + ;; + hide-global-config) + # Never interfere with help output. + for _a in "$@"; do + if [ "$_a" = "-h" ] || [ "$_a" = "--help" ]; then + exec "$real_tool" "$install_subcommand" "$@" + fi + done + + if git rev-parse --git-common-dir >/dev/null 2>&1; then + GIT_CONFIG_GLOBAL=/dev/null exec "$real_tool" "$install_subcommand" "$@" + fi + # not in a git work tree: let the real tool produce its own error + ;; +esac exec "$real_tool" "$install_subcommand" "$@" diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index c84b13517..4f4800633 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -42,6 +42,47 @@ def _setup(tmp_path: Path, name: str = "prek") -> tuple[dict[str, str], Path]: return env, argv +def _setup_lefthook(tmp_path: Path) -> tuple[dict[str, str], Path, Path]: + """A fake lefthook that refuses while a global core.hooksPath is visible. + + It records the global config it was given, then installs a hook where + `git rev-parse --git-path hooks` resolves, as lefthook does. + """ + real = tmp_path / "real" + real.mkdir() + record = tmp_path / "record" + shared = tmp_path / "shared-hooks" + (real / "lefthook").write_text( + "#!/bin/sh\n" + f'printf "GIT_CONFIG_GLOBAL=%s\\n" "${{GIT_CONFIG_GLOBAL-}}" > {record}\n' + f'printf "%s\\n" "$@" >> {record}\n' + 'for arg in "$@"; do\n' + ' case "$arg" in -h|--help) exit 0 ;; esac\n' + "done\n" + 'if [ "$1" = install ]; then\n' + ' if [ "${GIT_CONFIG_GLOBAL-}" != /dev/null ] ' + '&& [ -n "$(git config --global --get core.hooksPath 2>/dev/null)" ]; then\n' + ' echo "core.hooksPath is set globally" >&2\n' + " exit 1\n" + " fi\n" + ' hooks="$(git rev-parse --path-format=absolute --git-path hooks)"\n' + ' mkdir -p "$hooks" && printf "#!/bin/sh\\n" > "$hooks/pre-commit"\n' + "fi\n" + ) + (real / "lefthook").chmod(0o755) + shims = tmp_path / "shims" + shims.mkdir() + (shims / "lefthook").symlink_to(SHIM) + home = tmp_path / "home" + home.mkdir() + (home / ".gitconfig").write_text(f"[core]\n\thooksPath = {shared}\n") + env = dict(os.environ) + for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_COUNT", "GIT_CONFIG_KEY_0", "GIT_CONFIG_VALUE_0"): + env.pop(name, None) + env.update(PATH=f"{shims}:{real}:/usr/bin:/bin", HOME=str(home)) + return env, record, shared + + def _run( env: dict[str, str], cwd: Path, *args: str, name: str = "prek" ) -> subprocess.CompletedProcess[str]: @@ -98,6 +139,94 @@ def test_a_linked_worktree_targets_the_main_checkout(tmp_path: Path) -> None: ] +def test_lefthook_install_hides_the_global_hooks_path(tmp_path: Path) -> None: + env, record, shared = _setup_lefthook(tmp_path) + repo = _repo(tmp_path) + assert _run(env, repo, "install", name="lefthook").returncode == 0 + assert record.read_text().splitlines() == ["GIT_CONFIG_GLOBAL=/dev/null", "install"] + assert (repo / ".git" / "hooks" / "pre-commit").is_file() + assert not (shared / "pre-commit").exists() + + +def test_lefthook_install_in_a_linked_worktree_targets_the_main_checkout( + tmp_path: Path, +) -> None: + env, record, _shared = _setup_lefthook(tmp_path) + repo = _repo(tmp_path) + subprocess.run( + ["git", "-C", str(repo), "-c", "commit.gpgsign=false", "commit", "-q", "--allow-empty", "-m", "x"], + check=True, + env=dict( + os.environ, + GIT_AUTHOR_NAME="t", + GIT_AUTHOR_EMAIL="t@t", + GIT_COMMITTER_NAME="t", + GIT_COMMITTER_EMAIL="t@t", + ), + ) + worktree = tmp_path / "wt" + subprocess.run( + ["git", "-C", str(repo), "worktree", "add", "-q", str(worktree)], check=True + ) + assert _run(env, worktree, "install", name="lefthook").returncode == 0 + assert record.read_text().splitlines()[0] == "GIT_CONFIG_GLOBAL=/dev/null" + assert (repo / ".git" / "hooks" / "pre-commit").is_file() + + +@pytest.mark.parametrize( + "args", + [ + ["install", "--force"], + ["install", "-f"], + ["install", "--reset-hooks-path"], + ["install", "-r"], + ["install", "-fr"], + ], +) +def test_lefthook_destructive_flags_cannot_reach_the_global_dir( + tmp_path: Path, args: list[str] +) -> None: + env, record, shared = _setup_lefthook(tmp_path) + repo = _repo(tmp_path) + assert _run(env, repo, *args, name="lefthook").returncode == 0 + assert record.read_text().splitlines()[0] == "GIT_CONFIG_GLOBAL=/dev/null" + assert (repo / ".git" / "hooks" / "pre-commit").is_file() + assert not (shared / "pre-commit").exists() + + +def test_lefthook_other_invocations_pass_through(tmp_path: Path) -> None: + env, record, _shared = _setup_lefthook(tmp_path) + assert ( + _run(env, _repo(tmp_path), "run", "pre-commit", name="lefthook").returncode == 0 + ) + assert record.read_text().splitlines() == [ + "GIT_CONFIG_GLOBAL=", + "run", + "pre-commit", + ] + + +def test_lefthook_help_passes_through(tmp_path: Path) -> None: + env, record, _shared = _setup_lefthook(tmp_path) + assert ( + _run(env, _repo(tmp_path), "install", "--help", name="lefthook").returncode == 0 + ) + assert record.read_text().splitlines() == [ + "GIT_CONFIG_GLOBAL=", + "install", + "--help", + ] + + +def test_lefthook_install_outside_a_work_tree_passes_through(tmp_path: Path) -> None: + env, record, _shared = _setup_lefthook(tmp_path) + outside = tmp_path / "outside" + outside.mkdir() + env["GIT_CEILING_DIRECTORIES"] = str(tmp_path) + assert _run(env, outside, "install", name="lefthook").returncode == 1 + assert record.read_text().splitlines() == ["GIT_CONFIG_GLOBAL=", "install"] + + @pytest.mark.parametrize( "args", [ diff --git a/tools/dev/sync-hook-managers.py b/tools/dev/sync-hook-managers.py index 63f15fa2d..c590c7b50 100755 --- a/tools/dev/sync-hook-managers.py +++ b/tools/dev/sync-hook-managers.py @@ -54,7 +54,7 @@ SETTINGS = (Path(".claude/settings.json"), Path("tools/sandbox-lint/expected.json")) #: Install strategies the shim implements. A new one lands with its shim branch. -STRATEGIES = {"git-dir-flag", "none"} +STRATEGIES = {"git-dir-flag", "hide-global-config", "none"} _NAME = re.compile(r"^[a-z][a-z0-9-]*$") _ENV = re.compile(r"^[A-Z][A-Z0-9_]*$") @@ -111,17 +111,18 @@ def validate(registry: dict[str, Any]) -> list[str]: f"{where}.install.strategy: {strategy!r} is not one the shim implements " f"({', '.join(sorted(STRATEGIES))})" ) - if strategy != "none": - if not isinstance(install.get("subcommand"), str) or not _NAME.match(install["subcommand"]): - errors.append(f"{where}.install.subcommand: missing") - if strategy == "git-dir-flag" and not ( - isinstance(install.get("flag"), str) and _FLAG.match(install["flag"]) - ): - errors.append( - f"{where}.install.flag: git-dir-flag needs the long option that takes the git dir" - ) - elif "flag" in install: - errors.append(f"{where}.install.flag: strategy 'none' takes no flag") + if strategy != "none" and ( + not isinstance(install.get("subcommand"), str) or not _NAME.match(install["subcommand"]) + ): + errors.append(f"{where}.install.subcommand: missing") + if strategy == "git-dir-flag" and not ( + isinstance(install.get("flag"), str) and _FLAG.match(install["flag"]) + ): + errors.append( + f"{where}.install.flag: git-dir-flag needs the long option that takes the git dir" + ) + elif strategy != "git-dir-flag" and "flag" in install: + errors.append(f"{where}.install.flag: strategy {strategy!r} takes no flag") home = adapter.get("home") if home is None: continue # a manager that keeps nothing outside the repository diff --git a/tools/dev/tests/test_sync_hook_managers.py b/tools/dev/tests/test_sync_hook_managers.py index d761d4281..ab7fca4be 100644 --- a/tools/dev/tests/test_sync_hook_managers.py +++ b/tools/dev/tests/test_sync_hook_managers.py @@ -115,13 +115,22 @@ def test_strategy_none_adapter_omits_shim_and_needs_no_subcommand(registry: dict " prek) install_subcommand=install; install_strategy=git-dir-flag; install_flag=--git-dir ;;" in shim ) + # Lefthook needs a rewrite too, but has no flag to inject + assert " lefthook) install_subcommand=install; install_strategy=hide-global-config ;;" in shim def test_strategy_none_with_flag_is_refused(registry: dict[str, Any]) -> None: broken = copy.deepcopy(registry) broken["adapters"]["simple-git-hooks"]["install"]["flag"] = "--git-dir" errors = mod.validate(broken) - assert any("strategy 'none' takes no flag" in e for e in errors), errors + assert any("takes no flag" in e for e in errors), errors + + +def test_hide_global_config_with_flag_is_refused(registry: dict[str, Any]) -> None: + broken = copy.deepcopy(registry) + broken["adapters"]["lefthook"]["install"]["flag"] = "--git-dir" + errors = mod.validate(broken) + assert any("takes no flag" in e for e in errors), errors def _add_demo(tree: Path, registry: dict[str, Any]) -> None: diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index 0414e1e96..5399db97d 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -72,7 +72,7 @@ It leaves the install alone when the caller already chose a target, asked for he | Strategy | What the shim does | Status | |---|---|---| | `git-dir-flag` | Inserts ` ` after the install subcommand (prek: `--git-dir`; prek refuses a bare install while a global `core.hooksPath` is set). | implemented | -| `hide-global-config` | Runs the install with `GIT_CONFIG_GLOBAL=/dev/null`, for a manager that writes to the common dir but refuses while a global `core.hooksPath` is set (pre-commit, lefthook). | planned | +| `hide-global-config` | Runs the install with `GIT_CONFIG_GLOBAL=/dev/null`, for a manager that writes to the common dir but refuses while a global `core.hooksPath` is set (pre-commit, lefthook). The shim applies it to every install, including one carrying `--force` or `--reset-hooks-path`, so neither can reach the shared directory through the shim; run the real binary to do that. | implemented | | `hooks-path-env` | Runs the install with `GIT_CONFIG_COUNT=1`, `GIT_CONFIG_KEY_0=core.hooksPath`, `GIT_CONFIG_VALUE_0=/hooks`, for a manager that writes into whatever `core.hooksPath` names (overcommit, CaptainHook). The value must be that absolute path: git treats an empty `core.hooksPath` as set, and hooks then resolve against the filesystem root. | planned | | `none` | The manager already writes to the repository's own hooks directory whatever the global value (simple-git-hooks, cargo-husky, the JGit-based Maven plugins). | implemented | @@ -114,6 +114,12 @@ Detection decides what the setup configures *for a repository*; it never decides How prek configures itself, from its source (v0.5.7): it reads one project config per directory, `prek.toml`, then `.pre-commit-config.yaml`, then `.pre-commit-config.yml` (`crates/prek-consts/src/lib.rs`); searches upward from the current directory to the git root for the workspace root, and below it for nested project configs, honouring `.gitignore` and `.prekignore` (`crates/prek/src/workspace.rs`); takes `--config ` instead, which disables discovery and is baked into the installed hook; and reads a user-level `prek.toml` under the platform config directory that holds only update and display options, so it says nothing about whether a repository uses prek. The two YAML names are shared with pre-commit, so a repository with one uses both managers as far as detection goes; configuring a home the repository's hooks do not end up using is harmless. +How lefthook configures itself, from its source (v2.2.2): it reads `lefthook`, `.lefthook` or `.config/lefthook` with one of `.yml`, `.yaml`, `.json`, `.jsonc`, `.toml`, and the same three names with a `-local` suffix as the repository's local override (`internal/config/loader.go`). +Only the bare root names are in the registry: the contract's `detect.config_files` holds file names, not directories, so a `.config/lefthook.yml` at the repository root is not detected by rule 1; the installed hook's `call_lefthook` marker still catches it by rule 3. +`lefthook install` writes to `$(git rev-parse --git-path hooks)` and refuses while a scoped `git config --local` or `--global` read returns any `core.hooksPath` (`internal/command/install.go`). +`GIT_CONFIG_GLOBAL=/dev/null` hides the global value from that read, and the path then resolves to the common directory, linked worktrees included. +Remote configs are cloned into `/lefthook-remotes`, inside `.git`, so the registry omits `home`. + ## Registry [`registry.json`](registry.json) holds one adapter per manager under `adapters.`: @@ -149,7 +155,7 @@ Its operations are carried out by the scripts generated from the registry: | Operation | Run by | Invocation | Behaviour | |---|---|---|---| -| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install` | Applies the adapter's install strategy — for prek, runs `prek install --git-dir [args…]`. Left unchanged when `args` already carry the flag or `-h` / `--help`, or outside a git work tree. Every other subcommand passes through to the real binary, the next `` on `PATH` that is not the shim. Exits 127 when the name it is installed under has no adapter or no real binary is found. | +| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install` | Applies the adapter's install strategy: `git-dir-flag` runs `prek install --git-dir [args…]`; `hide-global-config` runs the install with `GIT_CONFIG_GLOBAL=/dev/null`. Left unchanged when `args` already carry the flag, ask for `-h` / `--help`, or outside a git work tree; a `hide-global-config` install carrying `--force` or `--reset-hooks-path` still gets the override, so neither reaches the shared directory through the shim. Every other subcommand passes through to the real binary, the next `` on `PATH` that is not the shim. Exits 127 when the name it is installed under has no adapter or no real binary is found. | | Host home | [`git-hook-dispatcher.sh`](../agent-isolation/git-hook-dispatcher.sh), from `~/.claude/git-hooks/` | Every hook git runs, before it chains to the repository's own hook | Outside the sandbox (no `SANDBOX_RUNTIME`), exports `=~/.local/share/` for every registered manager unless the operator's value lies outside `~/.cache`, and moves `XDG_CACHE_HOME` to `~/.local/share/prek-host/cache` by the same rule. Sandboxed hooks keep their values. | | Home check | [`sandbox-lint`](../sandbox-lint/) | `uv run --project tools/sandbox-lint sandbox-lint` | Fails a settings file whose `permissions.deny` lacks `Edit(/**)` for any registered manager, and, for a project that uses a manager (see [Detection](#detection)), one whose `env.` is missing or overlaps the default home. | | Registry sync | [`tools/dev/sync-hook-managers.py`](../dev/sync-hook-managers.py) | `tools/dev/sync-hook-managers.py [--check]`; the `sync-hook-managers` prek hook | Validates `registry.json`, rewrites the generated blocks of the three scripts above, and checks the committed settings and the sandbox-lint baseline. Exits 1 when a block changed (re-stage and commit), or, with `--check`, when one would. | @@ -175,6 +181,7 @@ What a repository or an operator configures is where each manager's environments | `cargo-husky` | Rust | `none`; detected by a hook mentioning `cargo-husky` | none (no external home) | shipping (in linked worktrees `build.rs` skips hook install: searches for a `.git` directory) | | `git-build-hook` | Java | `none`; detected by `pom.xml` (nested too) or a hook mentioning `git-build-hook` | none (no external home) | shipping (JGit `getGitDir()` targets `/hooks` in linked worktrees; `configure` goal can set repo-local `core.hooksPath`) | | `git-code-format-maven-plugin` | Java | `none`; detected by `pom.xml` (nested too) or a hook mentioning `git-code-format-maven-plugin` | none (no external home) | shipping (JGit `getCommonDirectory()` targets common hooks dir, coexisting cleanly in linked worktrees) | +| `lefthook` | Go | `hide-global-config`; detected by `lefthook.yml`, `lefthook.yaml`, `lefthook.json`, `lefthook.jsonc`, `lefthook.toml`, their `.lefthook` and `-local` variants, or a hook mentioning `call_lefthook` | none: remote configs live under `/lefthook-remotes`, inside `.git` | shipping | | `prek` | Rust | `git-dir-flag` (`--git-dir`); detected by `prek.toml`, `.pre-commit-config.yaml` or `.pre-commit-config.yml` (nested too) or a hook mentioning `prek` | `PREK_HOME`: `~/.cache/prek` → sandbox `~/.cache/prek-sandbox`, host `~/.local/share/prek-host` | shipping | | `simple-git-hooks` | JavaScript | `none`; detected by `package.json`, `.simple-git-hooks.{js,cjs,json}`, `simple-git-hooks.{js,cjs,json}` or a hook mentioning `simple-git-hooks` | none (no external home) | shipping | @@ -199,13 +206,18 @@ What a repository or an operator configures is where each manager's environments `AbstractMavenGitCodeFormatMojo.getOrCreateHooksDirectory()` uses `gitRepository().getCommonDirectory().toPath().resolve(HOOKS_DIR)`. Because it targets the common directory, hooks land in `$(git rev-parse --git-common-dir)/hooks/` and coexist cleanly in linked worktrees. Installs a bash wrapper invoking `mvn git-code-format-maven-plugin:format-code`. + - `lefthook` (v2.2.2, verified against the release binary through the shim): + Refuses a bare install while a scoped `git config --global` read returns any `core.hooksPath`; the shim's `GIT_CONFIG_GLOBAL=/dev/null` hides it, and the install then writes to `$(git rev-parse --git-path hooks)`, the common hooks directory. + `--force` and `--reset-hooks-path` still get the override through the shim, so neither reaches the shared directory or unsets the setup. + In a linked worktree the same install lands in the main checkout's `.git/hooks/`. + npm's `postinstall` runs `node_modules/.bin/lefthook` ahead of `~/.claude/bin`, so the shim does not cover it: that install refuses, and the repository needs one run through the shim. + A fix would be the session-start hook noticing hooks in the shared directory that are not the dispatcher; the shim cannot reach an npm-run child ahead of it on `PATH`. - **JGit in linked worktrees (Maven):** The linked-worktree limitation applies only to `git-build-hook` (`InstallMojo`), which uses `getGitDir()` and creates hooks under `/worktrees//hooks/`. Because C-git commits read hooks exclusively from `$(git rev-parse --git-common-dir)/hooks/`, hooks installed by `git-build-hook` from within a worktree are ignored by Git. Conversely, `git-code-format-maven-plugin` explicitly calls `getCommonDirectory()`, so its hooks populate the common directory and function across worktrees. - **Gradle & Go hook tools:** - Standard Gradle plugins and Go hook utilities remain unadapted and unregistered until someone tests them under the dispatcher. - `lefthook` (Go) refuses install when a global `core.hooksPath` is set and is reserved for the planned `hide-global-config` install strategy. + Standard Gradle plugins and other Go hook utilities remain unadapted and unregistered until someone tests them under the dispatcher. ## Adding an adapter diff --git a/tools/hook-manager/registry.json b/tools/hook-manager/registry.json index 8f2c5966d..32eb26f05 100644 --- a/tools/hook-manager/registry.json +++ b/tools/hook-manager/registry.json @@ -41,6 +41,40 @@ "strategy": "none" } }, + "lefthook": { + "language": "Go", + "upstream": "https://github.com/evilmartians/lefthook", + "detect": { + "config_files": [ + "lefthook.yml", + "lefthook.yaml", + "lefthook.json", + "lefthook.jsonc", + "lefthook.toml", + ".lefthook.yml", + ".lefthook.yaml", + ".lefthook.json", + ".lefthook.jsonc", + ".lefthook.toml", + "lefthook-local.yml", + "lefthook-local.yaml", + "lefthook-local.json", + "lefthook-local.jsonc", + "lefthook-local.toml", + ".lefthook-local.yml", + ".lefthook-local.yaml", + ".lefthook-local.json", + ".lefthook-local.jsonc", + ".lefthook-local.toml" + ], + "nested": false, + "hook_marker": "call_lefthook" + }, + "install": { + "subcommand": "install", + "strategy": "hide-global-config" + } + }, "prek": { "language": "Rust", "upstream": "https://github.com/j178/prek", diff --git a/tools/spec-loop/specs/agent-isolation-sandbox.md b/tools/spec-loop/specs/agent-isolation-sandbox.md index a2e56efd2..126b57f04 100644 --- a/tools/spec-loop/specs/agent-isolation-sandbox.md +++ b/tools/spec-loop/specs/agent-isolation-sandbox.md @@ -364,6 +364,12 @@ See `docs/adapters/goose.md`. `tools/agent-isolation/hook-manager-shim.sh` (installed as `~/.claude/bin/`, keyed by that name), the dispatcher and `sandbox-lint`, and checks the committed settings against it. + The shim's per-adapter install strategy keeps the target local: prek + gets `--git-dir`, and lefthook, which refuses while a global + `core.hooksPath` is set, gets `GIT_CONFIG_GLOBAL=/dev/null` and has no + home (#1822). + The `lefthook-hooks-path` agent guard blocks `lefthook install --force` + and `--reset-hooks-path` while that path is the framework's. The settings template likewise denies `Edit(~/.cache/ponymail-attachments/**)` (#1828): the PonyMail MCP saves report attachments there from outside the sandbox, and a sandboxed command that could rename directories under that @@ -382,7 +388,8 @@ See `docs/adapters/goose.md`. shim, verify check 19d; the default-home deny and the dispatcher's host home stay on for every registered manager, because both homes are per user (#1819). - `registry.json` registers prek (install strategy `git-dir-flag`) and + `registry.json` registers prek (install strategy `git-dir-flag`), + lefthook (`hide-global-config`, no home) and four managers with install strategy `none`, which already write to the repository's own hooks directory whatever the global `core.hooksPath`: `simple-git-hooks`, `cargo-husky`, `git-build-hook` and From aa70424bb0bf337d643eadbe1b93a510005a5e97 Mon Sep 17 00:00:00 2001 From: Anurag Singh Date: Thu, 29 Oct 2026 14:28:22 +0530 Subject: [PATCH 2/4] fix(hook-manager): apply cross-platform review feedback Why: review flagged three portability and consistency gaps in the lefthook adapter. guarded_head now recognizes backslash-delimited and .exe command paths, the hooks-path guard compares framework paths with os.path.normcase, and the shim's help passthrough uses the same case pattern as the other install strategy. --- .../agent-guard/src/agent_guard/__init__.py | 4 +++- .../guards/lefthook_hooks_path.py | 7 +++++-- tools/agent-guard/src/agent_guard/__init__.py | 4 +++- tools/agent-guard/tests/test_guards.py | 21 +++++++++++++++++++ tools/agent-guard/tests/test_skill_guards.py | 7 +++++++ tools/agent-isolation/hook-manager-shim.sh | 6 +++--- .../tests/test_hook_manager_shim.py | 7 ++++--- 7 files changed, 46 insertions(+), 10 deletions(-) diff --git a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py index d7cba00ae..7d9077663 100644 --- a/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py +++ b/plugins/magpie-agent-guard/tools/agent-guard/src/agent_guard/__init__.py @@ -2610,7 +2610,9 @@ def guarded_head(argv: list[str]) -> str | None: head = argv[0] if head in GUARDED_HEADS: return head - base = head.rsplit("/", 1)[-1] + base = head.replace("\\", "/").rsplit("/", 1)[-1] + if base.lower().endswith(".exe"): + base = base[:-4] return base if base in GUARDED_HEADS else None diff --git a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py index 64182ea86..2af7f5486 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py +++ b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py @@ -54,8 +54,11 @@ def _destructive_flag(argument): def _is_framework_path(value): - actual = os.path.realpath(os.path.expanduser(value)) - return any(actual == os.path.realpath(os.path.expanduser(name)) for name in FRAMEWORK_HOOKS_PATHS) + actual = os.path.normcase(os.path.realpath(os.path.expanduser(value))) + return any( + actual == os.path.normcase(os.path.realpath(os.path.expanduser(name))) + for name in FRAMEWORK_HOOKS_PATHS + ) def guard(ctx): diff --git a/tools/agent-guard/src/agent_guard/__init__.py b/tools/agent-guard/src/agent_guard/__init__.py index d7cba00ae..7d9077663 100644 --- a/tools/agent-guard/src/agent_guard/__init__.py +++ b/tools/agent-guard/src/agent_guard/__init__.py @@ -2610,7 +2610,9 @@ def guarded_head(argv: list[str]) -> str | None: head = argv[0] if head in GUARDED_HEADS: return head - base = head.rsplit("/", 1)[-1] + base = head.replace("\\", "/").rsplit("/", 1)[-1] + if base.lower().endswith(".exe"): + base = base[:-4] return base if base in GUARDED_HEADS else None diff --git a/tools/agent-guard/tests/test_guards.py b/tools/agent-guard/tests/test_guards.py index d7d3454c4..b829f40ed 100644 --- a/tools/agent-guard/tests/test_guards.py +++ b/tools/agent-guard/tests/test_guards.py @@ -51,6 +51,27 @@ def _stub(args, cwd=None): return _stub +# --------------------------------------------------------------------------- # +# guarded_head (shared helper, stays in the engine) +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "head, expected", + [ + ("lefthook", "lefthook"), + ("./node_modules/.bin/lefthook", "lefthook"), + (r"node_modules\.bin\lefthook", "lefthook"), + (r"C:\tools\lefthook.exe", "lefthook"), + ("lefthook.exe", "lefthook"), + ("/usr/bin/gh", "gh"), + ("curl", None), + ], +) +def test_guarded_head_recognizes_paths_from_every_platform(head, expected): + assert agent_guard.guarded_head([head, "install"]) == expected + + # --------------------------------------------------------------------------- # # find_mentions / strip_code (shared helper, stays in the engine) # --------------------------------------------------------------------------- # diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index 63a62d3c8..30fa40fd7 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -638,3 +638,10 @@ def test_lefthook_destructive_install_allowed_without_a_global_path(monkeypatch) def test_lefthook_destructive_install_override(monkeypatch): monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(FRAMEWORK_HOOKS))) assert dispatch("MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1 lefthook install --force") is None + + +def test_lefthook_framework_path_comparison_ignores_case(monkeypatch): + monkeypatch.setattr(os.path, "normcase", str.lower) + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(FRAMEWORK_HOOKS.upper()))) + reason = dispatch("lefthook install --force") + assert reason and "lefthook-hooks-path" in reason diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index 2b3b72592..6ae1a8f95 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -139,9 +139,9 @@ case "$install_strategy" in hide-global-config) # Never interfere with help output. for _a in "$@"; do - if [ "$_a" = "-h" ] || [ "$_a" = "--help" ]; then - exec "$real_tool" "$install_subcommand" "$@" - fi + case "$_a" in + -h|--help) exec "$real_tool" "$install_subcommand" "$@" ;; + esac done if git rev-parse --git-common-dir >/dev/null 2>&1; then diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index 4f4800633..eb69990d7 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -206,15 +206,16 @@ def test_lefthook_other_invocations_pass_through(tmp_path: Path) -> None: ] -def test_lefthook_help_passes_through(tmp_path: Path) -> None: +@pytest.mark.parametrize("help_flag", ["-h", "--help"]) +def test_lefthook_help_passes_through(tmp_path: Path, help_flag: str) -> None: env, record, _shared = _setup_lefthook(tmp_path) assert ( - _run(env, _repo(tmp_path), "install", "--help", name="lefthook").returncode == 0 + _run(env, _repo(tmp_path), "install", help_flag, name="lefthook").returncode == 0 ) assert record.read_text().splitlines() == [ "GIT_CONFIG_GLOBAL=", "install", - "--help", + help_flag, ] From 440da680e9f65e746e9bfd24c0a3dca69b7af0a8 Mon Sep 17 00:00:00 2001 From: Anurag Singh Date: Fri, 30 Oct 2026 12:11:03 +0530 Subject: [PATCH 3/4] fix(hook-manager): apply maintainer review feedback Why: the lefthook shim override ran on every host, so an install on a machine with no global core.hooksPath silently lost the operator's credential.helper, url.*.insteadOf and safe.directory even though it had nothing to hide; it now sets GIT_CONFIG_GLOBAL only when the global config actually carries a core.hooksPath, which makes the documented no-op true. The guard now reads the flag name before any =value, so lefthook's --force=true and --reset-hooks-path=true spellings are caught when the real binary runs outside the shim, while an explicit =false stays allowed. The new troubleshooting section and the secure-setup paragraph break one sentence per line. Also regenerates the isolated-setup fingerprint for the rebase onto main. Generated-by: DeepSeek V4.1 Flash --- docs/setup/sandbox-troubleshooting.md | 36 ++++++------------- docs/setup/secure-agent-setup.md | 12 ++----- .../guards/lefthook_hooks_path.py | 16 ++++++--- .../step-p-sandbox-allowlists.md | 6 ++-- .../setup_preflight/isolated_fingerprint.py | 2 +- tools/agent-guard/tests/test_skill_guards.py | 15 ++++++++ tools/agent-isolation/README.md | 2 +- tools/agent-isolation/hook-manager-shim.sh | 18 +++++++--- .../tests/test_hook_manager_shim.py | 31 +++++++++++++++- tools/hook-manager/README.md | 4 +-- 10 files changed, 90 insertions(+), 52 deletions(-) diff --git a/docs/setup/sandbox-troubleshooting.md b/docs/setup/sandbox-troubleshooting.md index 98a7fdf48..308fc23c2 100644 --- a/docs/setup/sandbox-troubleshooting.md +++ b/docs/setup/sandbox-troubleshooting.md @@ -1245,8 +1245,7 @@ resolved path. Confirm with `git hook run pre-commit` from the agent. ### Symptom -A repository that uses lefthook has no working hooks, and `lefthook install` -fails: +A repository that uses lefthook has no working hooks, and `lefthook install` fails: ```console $ lefthook install @@ -1257,40 +1256,27 @@ Custom hooks paths are not supported by default. ### Root cause -Whole-user scope sets a global `core.hooksPath`, and lefthook refuses to install -while a scoped `git config --global` read returns one. -That is the safe failure: the alternative, `--force`, writes lefthook's hook -into the shared directory and replaces the framework's dispatcher for every -repository on the host. +Whole-user scope sets a global `core.hooksPath`, and lefthook refuses to install while a scoped `git config --global` read returns one. +That is the safe failure: the alternative, `--force`, writes lefthook's hook into the shared directory and replaces the framework's dispatcher for every repository on the host. ### Fix -Run `lefthook install` through the framework shim, which hides the global config -from that read and leaves the hook in the repository's own -`$(git rev-parse --git-common-dir)/hooks/`: +Run `lefthook install` through the framework shim, which hides the global config from that read and leaves the hook in the repository's own `$(git rev-parse --git-common-dir)/hooks/`: ```bash ~/.claude/bin/lefthook install ``` -With `~/.claude/bin` ahead of the real binary on PATH, plain `lefthook install` -does that already. -The setup installs the shim only for a repository that uses lefthook: a root -`lefthook.*`, `.lefthook.*` or `-local` config, or an installed hook mentioning -`call_lefthook`. +With `~/.claude/bin` ahead of the real binary on PATH, plain `lefthook install` does that already. +The setup installs the shim only for a repository that uses lefthook: a root `lefthook.*`, `.lefthook.*` or `-local` config, or an installed hook mentioning `call_lefthook`. ### Notes -- Do not run `lefthook install --reset-hooks-path` or `--force` to get past the - error: the first unsets the whole-user setup, and the second overwrites the - dispatcher for every repository. - The agent-guard `lefthook-hooks-path` guard blocks both, and needs - `MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1` to proceed deliberately. -- An npm `postinstall` runs `node_modules/.bin/lefthook`, which the shim does - not cover; that install refuses for the same reason, and the repository needs - one run through the shim. -- Per-project scope has no global `core.hooksPath`, so the refusal does not - happen there. +- Do not run `lefthook install --reset-hooks-path` or `--force` to get past the error: the first unsets the whole-user setup, and the second overwrites the dispatcher for every repository. + The agent-guard `lefthook-hooks-path` guard blocks both, in the `--flag` and `--flag=true` spellings, and needs `MAGPIE_ALLOW_LEFTHOOK_HOOKS_PATH=1` to proceed deliberately. +- An npm `postinstall` runs `node_modules/.bin/lefthook`, which the shim does not cover; that install refuses for the same reason, and the repository needs one run through the shim. +- The shim hides the global config only while it holds a `core.hooksPath`, so on a host without one the install keeps the operator's `credential.helper`, `url.*.insteadOf` and `safe.directory`. +- Per-project scope has no global `core.hooksPath`, so the refusal does not happen there. --- diff --git a/docs/setup/secure-agent-setup.md b/docs/setup/secure-agent-setup.md index de6317696..f05f43304 100644 --- a/docs/setup/secure-agent-setup.md +++ b/docs/setup/secure-agent-setup.md @@ -1438,15 +1438,9 @@ PATH) that rewrites **only** `prek install` — injecting in the repo-local `.git/hooks/` where the dispatcher finds it. Every other `prek` command passes straight through, and the shim is a no-op on hosts with no global `core.hooksPath`. -lefthook refuses to install at all while a global `core.hooksPath` is set, so -the same shim installed as `~/.claude/bin/lefthook` runs `lefthook install` -with `GIT_CONFIG_GLOBAL=/dev/null`; lefthook then resolves its own target to the -common dir. -`lefthook install --force` and `--reset-hooks-path` would still get that -override through the shim (they cannot reach the shared dir or unset the setup), -and the agent-guard `lefthook-hooks-path` guard blocks an agent from trying. -The shim covers the CLI only: an npm `postinstall` runs -`node_modules/.bin/lefthook`, ahead of `~/.claude/bin` on PATH. +lefthook refuses to install at all while a global `core.hooksPath` is set, so the same shim installed as `~/.claude/bin/lefthook` runs `lefthook install` with `GIT_CONFIG_GLOBAL=/dev/null` whenever the global config holds one, and lefthook then resolves its own target to the common dir. +`lefthook install --force` and `--reset-hooks-path` would still get that override through the shim (they cannot reach the shared dir or unset the setup), and the agent-guard `lefthook-hooks-path` guard blocks an agent from trying. +The shim covers the CLI only: an npm `postinstall` runs `node_modules/.bin/lefthook`, ahead of `~/.claude/bin` on PATH. The shim, the host-only homes and the sandbox-lint checks all follow the [hook-manager contract](../../tools/hook-manager/README.md): one script serves every adapter in its registry, keyed by the name it is diff --git a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py index 2af7f5486..9ee414377 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py +++ b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py @@ -41,14 +41,20 @@ def _install_arguments(argv): def _destructive_flag(argument): - if argument in ("-f", "--force"): + # lefthook takes the `--flag=value` spelling for its boolean flags, so match + # the name and read the value separately: `--force=true` is destructive, + # `--force=false` is not. + name, _, value = argument.partition("=") + if value.lower() in ("false", "0", "f", "no", "off"): + return None + if name in ("-f", "--force"): return "--force" - if argument in ("-r", "--reset-hooks-path"): + if name in ("-r", "--reset-hooks-path"): return "--reset-hooks-path" - if argument.startswith("-") and not argument.startswith("--"): - if "f" in argument[1:]: + if name.startswith("-") and not name.startswith("--"): + if "f" in name[1:]: return "--force" - if "r" in argument[1:]: + if "r" in name[1:]: return "--reset-hooks-path" return None diff --git a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md index 142b73bf8..986b14c3f 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md +++ b/plugins/magpie-setup/skills/isolated-setup-install/step-p-sandbox-allowlists.md @@ -347,9 +347,9 @@ for the full rationale + the validated behaviour matrix. The shim takes the tool from the name it is installed under and rewrites only the install subcommand: for prek it injects `--git-dir "$(git rev-parse --git-common-dir)"`; for lefthook it runs the - install with `GIT_CONFIG_GLOBAL=/dev/null`, which hides the global - `core.hooksPath` from lefthook's own check and lets its target resolve to the - common dir. + install with `GIT_CONFIG_GLOBAL=/dev/null` while the global config holds one, + which hides the global `core.hooksPath` from lefthook's own check and lets + its target resolve to the common dir. Both leave the install alone when the caller asked for `--help` or is outside a git work tree (prek also when `--git-dir` is already passed), and every other invocation passes through unchanged. diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index b00c19107..512ea97c8 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:51a5a6c8256be7ff" +FRAMEWORK_FINGERPRINT = "sha256:fe4166bf665b4d1d" diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index 30fa40fd7..aa7cde37e 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -598,7 +598,10 @@ def handler(args): [ ("lefthook install --force", "--force"), ("lefthook install -f", "--force"), + ("lefthook install --force=true", "--force"), + ("lefthook install --force=1", "--force"), ("lefthook install --reset-hooks-path", "--reset-hooks-path"), + ("lefthook install --reset-hooks-path=true", "--reset-hooks-path"), ("lefthook install -r", "--reset-hooks-path"), ("lefthook install -fr", "--force"), ("lefthook --verbose install --force", "--force"), @@ -625,6 +628,18 @@ def test_lefthook_plain_invocations_allowed(monkeypatch, command): assert dispatch(command) is None +@pytest.mark.parametrize( + "command", + [ + "lefthook install --force=false", + "lefthook install --reset-hooks-path=false", + ], +) +def test_lefthook_switched_off_destructive_flag_allowed(monkeypatch, command): + monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub(FRAMEWORK_HOOKS))) + assert dispatch(command) is None + + def test_lefthook_destructive_install_allowed_for_another_global_path(monkeypatch): monkeypatch.setattr(agent_guard, "_run", fake_run(hooks_path_stub("/opt/other-hooks"))) assert dispatch("lefthook install --force") is None diff --git a/tools/agent-isolation/README.md b/tools/agent-isolation/README.md index e70f4951b..0ec2a0de5 100644 --- a/tools/agent-isolation/README.md +++ b/tools/agent-isolation/README.md @@ -98,7 +98,7 @@ per runtime — see [`docs/adapters/add-a-harness.md`](../../docs/adapters/add-a | [`sandbox-add-project-root.sh`](sandbox-add-project-root.sh) | Adds the current adopter repo's project root (and, with `--all-worktrees`, every linked git worktree's working dir) as an explicit absolute path to `sandbox.filesystem.allowRead` and `allowWrite` in the project-local, gitignored `/.claude/settings.local.json` — one entry per worktree, each in that worktree's own settings file. Defensive against [issue #197](https://github.com/apache/magpie/issues/197) — `allowRead: ["."]` does not in practice cover CWD because the harness pre-resolves the `.` literal away from the read side. The same harness behaviour drops the committed home-directory entries, so it also adds the dev-tool paths `prek` and `uv` need (`~/.gitconfig`, `~/.config/git`, `~/.cache`, `~/.local/share/uv`, `~/.local/bin`; the cache and uv dirs writable) as absolute paths — never the credential paths; `--no-tool-paths` skips them. See [`prek` or `uv` not found inside the sandbox](../../docs/setup/sandbox-troubleshooting.md#prek-or-uv-not-found-or-cannot-write-its-cache-inside-the-sandbox). Never modifies user-scope or committed project-scope. Idempotent, atomic, tolerant of missing prereqs. Invoked from `setup-isolated-setup-install`, `/magpie-setup:setup` (adopt / upgrade / worktree-init), and the `post-checkout` git hook installed by `/magpie-setup:setup adopt`. | | [`git-global-post-checkout.sh`](git-global-post-checkout.sh) | Universal `post-checkout` git hook installed at `~/.claude/git-hooks/post-checkout` when the operator picks the **simple whole-user** flavour in `setup-isolated-setup-install`. Activated by `git config --global core.hooksPath ~/.claude/git-hooks/` so every `git checkout` / `git clone` / `git worktree add` across the host invokes it. Best-effort + idempotent + `\|\| true`: invokes `sandbox-add-project-root.sh` for any worktree with a `.claude/` directory. Trade-off documented in [`docs/setup/secure-agent-setup.md` → *Per-project vs whole-user scope*](../../docs/setup/secure-agent-setup.md#per-project-vs-whole-user-scope): `core.hooksPath` shadows per-repo `.git/hooks/*` across every repo on the host. The **dispatcher** flavour (below) supersedes this file. | | [`git-hook-dispatcher.sh`](git-hook-dispatcher.sh) | Universal, basename-keyed git-hook **dispatcher** for the **whole-user dispatcher** flavour. Symlinked to every hook name under `~/.claude/git-hooks/`. For each git operation it runs the framework's own logic for that hook type (the `post-checkout` sandbox-allowlist sync) **and then chains through to the repo-local `.git/hooks/`** (resolved via `git rev-parse --git-common-dir`, worktree-safe; `exec`-ed with original argv + inherited stdin so a failing local hook still aborts the git op). Restores per-repo hooks (prek / pre-commit / lefthook / husky / hand-written) under global `core.hooksPath`; a repo with no local hook is a clean no-op. Outside the sandbox it also gives every hook manager in the [hook-manager registry](../hook-manager/registry.json) its host-only home (the contract's home rule). See [`docs/setup/secure-agent-setup.md` → *Whole-user with the per-repo dispatcher*](../../docs/setup/secure-agent-setup.md#whole-user-with-the-per-repo-dispatcher). | -| [`hook-manager-shim.sh`](hook-manager-shim.sh) | Transparent front for a git-hook manager, installed as `~/.claude/bin/` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour, one install per adapter in the [hook-manager registry](../hook-manager/registry.json) whose install strategy is not `none` (`prek` and `lefthook` today). The tool is the name it is installed under. Rewrites **only** the adapter's install subcommand so the hook lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir: for prek it injects `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`; for lefthook it runs the install with `GIT_CONFIG_GLOBAL=/dev/null` so lefthook's own `core.hooksPath` refusal does not fire. Both leave help output and a non-repository invocation alone. Every other invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. Implements the install-target rule of the [hook-manager contract](../hook-manager/README.md). | +| [`hook-manager-shim.sh`](hook-manager-shim.sh) | Transparent front for a git-hook manager, installed as `~/.claude/bin/` (with `~/.claude/bin` prepended to PATH) for the whole-user dispatcher flavour, one install per adapter in the [hook-manager registry](../hook-manager/registry.json) whose install strategy is not `none` (`prek` and `lefthook` today). The tool is the name it is installed under. Rewrites **only** the adapter's install subcommand so the hook lands in the repo-local `.git/hooks/` where the dispatcher chains, instead of the shared `core.hooksPath` dir: for prek it injects `--git-dir "$(git rev-parse --git-common-dir)"` unless the caller already passed `--git-dir`; for lefthook it runs the install with `GIT_CONFIG_GLOBAL=/dev/null` while the global config holds a `core.hooksPath`, so lefthook's own `core.hooksPath` refusal does not fire and a host without one keeps its global settings. Both leave help output and a non-repository invocation alone. Every other invocation passes through unchanged; no-op on hosts with no global `core.hooksPath`. Implements the install-target rule of the [hook-manager contract](../hook-manager/README.md). | ## Usage at a glance diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index 6ae1a8f95..2eda31aa3 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -52,10 +52,12 @@ # hide-global-config: run the install with # GIT_CONFIG_GLOBAL=/dev/null, for a manager that writes to # the common dir itself but refuses while a global -# core.hooksPath is set (lefthook). The override applies even -# with `--force` / `--reset-hooks-path`: through this shim -# those must not write into the shared dir or unset the -# whole-user setup. Run the real binary to bypass it. +# core.hooksPath is set (lefthook). The override applies only +# when the global config actually holds a core.hooksPath, so an +# ordinary host keeps its credential and url.* settings, and +# even then it applies with `--force` / `--reset-hooks-path`: +# through this shim those must not write into the shared dir or +# unset the whole-user setup. Run the real binary to bypass it. # * every other invocation (`prek run`, `prek autoupdate`, # `--version`, bare `prek`, ...) is passed through to the real # binary **unchanged**. @@ -145,7 +147,13 @@ case "$install_strategy" in done if git rev-parse --git-common-dir >/dev/null 2>&1; then - GIT_CONFIG_GLOBAL=/dev/null exec "$real_tool" "$install_subcommand" "$@" + # Hide the global config only when it holds a core.hooksPath to hide: the + # override also costs the install the operator's credential.helper, + # url.*.insteadOf and safe.directory, and the manager can clone remote + # configs while installing. + if [ -n "$(git config --global --get core.hooksPath 2>/dev/null)" ]; then + GIT_CONFIG_GLOBAL=/dev/null exec "$real_tool" "$install_subcommand" "$@" + fi fi # not in a git work tree: let the real tool produce its own error ;; diff --git a/tools/agent-isolation/tests/test_hook_manager_shim.py b/tools/agent-isolation/tests/test_hook_manager_shim.py index eb69990d7..ad284a763 100644 --- a/tools/agent-isolation/tests/test_hook_manager_shim.py +++ b/tools/agent-isolation/tests/test_hook_manager_shim.py @@ -79,10 +79,17 @@ def _setup_lefthook(tmp_path: Path) -> tuple[dict[str, str], Path, Path]: env = dict(os.environ) for name in ("GIT_CONFIG_GLOBAL", "GIT_CONFIG_COUNT", "GIT_CONFIG_KEY_0", "GIT_CONFIG_VALUE_0"): env.pop(name, None) - env.update(PATH=f"{shims}:{real}:/usr/bin:/bin", HOME=str(home)) + env.update(PATH=f"{shims}:{real}:/usr/bin:/bin", HOME=str(home), GIT_CONFIG_NOSYSTEM="1") return env, record, shared +def _set_global_hooks_path(env: dict[str, str], value: str | None) -> None: + """Rewrite the fake home's global config: a `core.hooksPath`, or none.""" + (Path(env["HOME"]) / ".gitconfig").write_text( + f"[core]\n\thooksPath = {value}\n" if value else "[user]\n\tname = t\n" + ) + + def _run( env: dict[str, str], cwd: Path, *args: str, name: str = "prek" ) -> subprocess.CompletedProcess[str]: @@ -148,6 +155,28 @@ def test_lefthook_install_hides_the_global_hooks_path(tmp_path: Path) -> None: assert not (shared / "pre-commit").exists() +def test_lefthook_install_hides_a_custom_global_hooks_path(tmp_path: Path) -> None: + env, record, _shared = _setup_lefthook(tmp_path) + other = tmp_path / "other-hooks" + _set_global_hooks_path(env, str(other)) + repo = _repo(tmp_path) + assert _run(env, repo, "install", name="lefthook").returncode == 0 + assert record.read_text().splitlines() == ["GIT_CONFIG_GLOBAL=/dev/null", "install"] + assert (repo / ".git" / "hooks" / "pre-commit").is_file() + assert not (other / "pre-commit").exists() + + +def test_lefthook_install_without_a_global_hooks_path_keeps_the_global_config( + tmp_path: Path, +) -> None: + env, record, _shared = _setup_lefthook(tmp_path) + _set_global_hooks_path(env, None) + repo = _repo(tmp_path) + assert _run(env, repo, "install", name="lefthook").returncode == 0 + assert record.read_text().splitlines() == ["GIT_CONFIG_GLOBAL=", "install"] + assert (repo / ".git" / "hooks" / "pre-commit").is_file() + + def test_lefthook_install_in_a_linked_worktree_targets_the_main_checkout( tmp_path: Path, ) -> None: diff --git a/tools/hook-manager/README.md b/tools/hook-manager/README.md index 5399db97d..86d685fbf 100644 --- a/tools/hook-manager/README.md +++ b/tools/hook-manager/README.md @@ -72,7 +72,7 @@ It leaves the install alone when the caller already chose a target, asked for he | Strategy | What the shim does | Status | |---|---|---| | `git-dir-flag` | Inserts ` ` after the install subcommand (prek: `--git-dir`; prek refuses a bare install while a global `core.hooksPath` is set). | implemented | -| `hide-global-config` | Runs the install with `GIT_CONFIG_GLOBAL=/dev/null`, for a manager that writes to the common dir but refuses while a global `core.hooksPath` is set (pre-commit, lefthook). The shim applies it to every install, including one carrying `--force` or `--reset-hooks-path`, so neither can reach the shared directory through the shim; run the real binary to do that. | implemented | +| `hide-global-config` | Runs the install with `GIT_CONFIG_GLOBAL=/dev/null` whenever the global config holds a `core.hooksPath`, for a manager that writes to the common dir but refuses while one is set (pre-commit, lefthook); a host without one keeps its global `credential.helper` / `url.*.insteadOf` settings. The override also applies to an install carrying `--force` or `--reset-hooks-path`, so neither can reach the shared directory through the shim; run the real binary to do that. | implemented | | `hooks-path-env` | Runs the install with `GIT_CONFIG_COUNT=1`, `GIT_CONFIG_KEY_0=core.hooksPath`, `GIT_CONFIG_VALUE_0=/hooks`, for a manager that writes into whatever `core.hooksPath` names (overcommit, CaptainHook). The value must be that absolute path: git treats an empty `core.hooksPath` as set, and hooks then resolve against the filesystem root. | planned | | `none` | The manager already writes to the repository's own hooks directory whatever the global value (simple-git-hooks, cargo-husky, the JGit-based Maven plugins). | implemented | @@ -155,7 +155,7 @@ Its operations are carried out by the scripts generated from the registry: | Operation | Run by | Invocation | Behaviour | |---|---|---|---| -| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install` | Applies the adapter's install strategy: `git-dir-flag` runs `prek install --git-dir [args…]`; `hide-global-config` runs the install with `GIT_CONFIG_GLOBAL=/dev/null`. Left unchanged when `args` already carry the flag, ask for `-h` / `--help`, or outside a git work tree; a `hide-global-config` install carrying `--force` or `--reset-hooks-path` still gets the override, so neither reaches the shared directory through the shim. Every other subcommand passes through to the real binary, the next `` on `PATH` that is not the shim. Exits 127 when the name it is installed under has no adapter or no real binary is found. | +| Install rewrite | [`hook-manager-shim.sh`](../agent-isolation/hook-manager-shim.sh), installed as `~/.claude/bin/` | ` [args…]`, e.g. `prek install` | Applies the adapter's install strategy: `git-dir-flag` runs `prek install --git-dir [args…]`; `hide-global-config` runs the install with `GIT_CONFIG_GLOBAL=/dev/null` when the global config holds a `core.hooksPath`. Left unchanged when `args` already carry the flag, ask for `-h` / `--help`, or outside a git work tree; a `hide-global-config` install carrying `--force` or `--reset-hooks-path` still gets the override, so neither reaches the shared directory through the shim. Every other subcommand passes through to the real binary, the next `` on `PATH` that is not the shim. Exits 127 when the name it is installed under has no adapter or no real binary is found. | | Host home | [`git-hook-dispatcher.sh`](../agent-isolation/git-hook-dispatcher.sh), from `~/.claude/git-hooks/` | Every hook git runs, before it chains to the repository's own hook | Outside the sandbox (no `SANDBOX_RUNTIME`), exports `=~/.local/share/` for every registered manager unless the operator's value lies outside `~/.cache`, and moves `XDG_CACHE_HOME` to `~/.local/share/prek-host/cache` by the same rule. Sandboxed hooks keep their values. | | Home check | [`sandbox-lint`](../sandbox-lint/) | `uv run --project tools/sandbox-lint sandbox-lint` | Fails a settings file whose `permissions.deny` lacks `Edit(/**)` for any registered manager, and, for a project that uses a manager (see [Detection](#detection)), one whose `env.` is missing or overlaps the default home. | | Registry sync | [`tools/dev/sync-hook-managers.py`](../dev/sync-hook-managers.py) | `tools/dev/sync-hook-managers.py [--check]`; the `sync-hook-managers` prek hook | Validates `registry.json`, rewrites the generated blocks of the three scripts above, and checks the committed settings and the sandbox-lint baseline. Exits 1 when a block changed (re-stage and commit), or, with `--check`, when one would. | From 772474e469bd33a261d45bd51fd9aff6649e15b7 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 30 Oct 2026 20:55:29 +0100 Subject: [PATCH 4/4] fix(agent-guard): deny a lefthook bundle that sets --force before an =false The lefthook guard read a false-ish `=value` before it looked at short switch bundles, so `lefthook install -fr=false`, which still sets --force, was allowed (the previous version denied it). The value now applies only to a single flag or to the last switch of a bundle; every switch before it counts as set. Tests deny `-fr=false` and `-rf=false` and keep `-f=false` allowed. Also reword the shim comment that called the hand-written install strategy branches "generated": the sync script generates only the adapter() block. The secure-setup fingerprint is restamped. Generated-by: Claude Opus 5 --- .../guards/lefthook_hooks_path.py | 14 ++++++++------ .../setup/setup_preflight/isolated_fingerprint.py | 2 +- tools/agent-guard/tests/test_skill_guards.py | 3 +++ tools/agent-isolation/hook-manager-shim.sh | 4 +++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py index 9ee414377..6a706ce9c 100644 --- a/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py +++ b/plugins/magpie-setup/skills/isolated-setup-install/guards/lefthook_hooks_path.py @@ -45,16 +45,18 @@ def _destructive_flag(argument): # the name and read the value separately: `--force=true` is destructive, # `--force=false` is not. name, _, value = argument.partition("=") - if value.lower() in ("false", "0", "f", "no", "off"): - return None + off = value.lower() in ("false", "0", "f", "no", "off") if name in ("-f", "--force"): - return "--force" + return None if off else "--force" if name in ("-r", "--reset-hooks-path"): - return "--reset-hooks-path" + return None if off else "--reset-hooks-path" if name.startswith("-") and not name.startswith("--"): - if "f" in name[1:]: + # In a bundle (`-fr=false`) the value belongs to the last switch only; + # every switch before it is set. + letters = name[1:-1] if off else name[1:] + if "f" in letters: return "--force" - if "r" in name[1:]: + if "r" in letters: return "--reset-hooks-path" return None diff --git a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py index 512ea97c8..cc7123eb0 100644 --- a/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py +++ b/plugins/magpie-setup/skills/setup/setup_preflight/isolated_fingerprint.py @@ -21,4 +21,4 @@ for installs that do not carry the framework source (see `isolated.py`). """ -FRAMEWORK_FINGERPRINT = "sha256:fe4166bf665b4d1d" +FRAMEWORK_FINGERPRINT = "sha256:0aa5bf7fa52f9309" diff --git a/tools/agent-guard/tests/test_skill_guards.py b/tools/agent-guard/tests/test_skill_guards.py index aa7cde37e..e2bb7869a 100644 --- a/tools/agent-guard/tests/test_skill_guards.py +++ b/tools/agent-guard/tests/test_skill_guards.py @@ -604,6 +604,8 @@ def handler(args): ("lefthook install --reset-hooks-path=true", "--reset-hooks-path"), ("lefthook install -r", "--reset-hooks-path"), ("lefthook install -fr", "--force"), + ("lefthook install -fr=false", "--force"), + ("lefthook install -rf=false", "--reset-hooks-path"), ("lefthook --verbose install --force", "--force"), ("./node_modules/.bin/lefthook install --force", "--force"), ], @@ -633,6 +635,7 @@ def test_lefthook_plain_invocations_allowed(monkeypatch, command): [ "lefthook install --force=false", "lefthook install --reset-hooks-path=false", + "lefthook install -f=false", ], ) def test_lefthook_switched_off_destructive_flag_allowed(monkeypatch, command): diff --git a/tools/agent-isolation/hook-manager-shim.sh b/tools/agent-isolation/hook-manager-shim.sh index 2eda31aa3..9bceb38b0 100755 --- a/tools/agent-isolation/hook-manager-shim.sh +++ b/tools/agent-isolation/hook-manager-shim.sh @@ -115,7 +115,9 @@ fi shift # drop the install subcommand -# --- install strategies: generated branches keyed by install_strategy ------ +# --- install strategies: one hand-written branch per install_strategy ------ +# (tools/dev/sync-hook-managers.py lists them in STRATEGIES and generates only +# the adapter() block above.) case "$install_strategy" in git-dir-flag) # Respect a target the caller chose, and never interfere with help output.