fix(agentcore): disable repository hooks for Git operations - #446
Merged
Conversation
A stage that had already produced its artifacts died with
git_commit_failed, and the only detail captured was npm's warnings:
commit_failed — npm warn Unknown project config "strict-peer-dependencies"
npm warn Unknown project config "auto-install-peers"
The cause is the USER repo's own pre-commit hook. A project using husky +
lint-staged installs hooks that shell out to npm/npx, but the runtime
deliberately never installs the checkout's dependencies (see the
inode-budget notes in workspace.js), so the hook exits non-zero and the
stage's completed work is lost for a reason unrelated to that work.
Set `core.hooksPath` to a hook-free path for every engine-owned git
invocation, in runGit — the single choke point all of them pass through,
so operations added later inherit the policy.
Two reasons the engine must not execute repo hooks:
- CORRECTNESS: the failure above. Hooks assuming an installed dependency
tree cannot work in this runtime.
- SECURITY: repo hooks are arbitrary untrusted code running inside the
agent runtime, and pushes carry a short-lived credential in the
environment. A pre-push hook could read AIDLC_GIT_PASSWORD, abort the
push, and surface the value in the returned error detail.
`--no-verify` is NOT sufficient and would have left the same class of bug
open: it covers pre-commit and commit-msg only, so prepare-commit-msg can
still abort a commit and pre-push runs on every push. `-c core.hooksPath`
disables every hook for the invocation without mutating the repository's
own configuration.
Tests drive REAL git in throwaway repos and pin the hooks via
`core.hooksPath` rather than `.git/hooks`, which is the stronger
assertion: it proves the engine's own `-c core.hooksPath` overrides a
hooksPath the repository configured for itself. Cases cover pre-commit,
commit-msg and prepare-commit-msg (commit succeeds, hook never ran, tree
left clean), and a credential-bearing pre-push (push succeeds, the hook
never ran, and the token appears nowhere in the result). All five fail
without the change.
Rebased onto latest main (b9ab0b6).
I confirm the licensing of this contribution under the repository's MIT-0
license.
(cherry picked from commit 2570211)
JWThewes
requested review from
eipasteur,
jeromevdl,
leandrodamascena and
svozza
as code owners
September 8, 2026 16:27
jeromevdl
reviewed
Sep 9, 2026
jeromevdl
reviewed
Sep 9, 2026
jeromevdl
reviewed
Sep 9, 2026
jeromevdl
left a comment
jeromevdl
left a comment
Contributor
There was a problem hiding this comment.
2 small comments before approving
Sanitize inherited repository and identity overrides in the shared Git runner, and pass the protected runner into workspace directory-trust callbacks. Add regressions for credential preservation, explicit overrides, and fresh and warm checkouts.
Contributor
Author
|
Thanks @jeromevdl — addressed both in 316ce41: directory-trust setup now uses the wrapped runner, and environment sanitization lives in the shared Git runner. Added regressions; all 345 affected local tests and CI checks pass. |
jeromevdl
approved these changes
Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem / Motivation
AgentCore-owned Git operations currently execute repository-controlled hooks. Hooks that depend on unavailable checkout dependencies can reject engine commits after a stage has produced work, while credential-bearing operations such as push can expose the short-lived Git credential to arbitrary repository hook code.
Why it matters
A failing
prepare-commit-msgcan leave completed stage work uncommitted, and a maliciouspre-pushorpost-checkouthook can observe credentials inside the AgentCore runtime. The existing--no-verifyapproach is incomplete because it does not suppress every hook.What changed
-c core.hooksPath=/dev/nullper invocation and strips inherited repository/index and author/committerGIT_*overrides for both engine and workspace commands. Explicit credential and per-command environment overrides remain supported; repository and user Git configuration remain untouched.git-engine.jsandworkspace.jsclone, checkout, branch, commit, merge, fetch, and push paths through the shared policy while preserving injection and return contracts. Directory-trust callbacks receive the wrapped runner as well..git/hooks; fresh-clone exposure requires an inherited/global hook path targeting tracked checkout content.Attribution
The first commit retains Olivier Rossant as author and records the original PR #363 commit via the cherry-pick provenance line. The subsequent commits are the maintainer-owned completion covering the separate workspace Git path and shared environment sanitization.
Tests
pre-commit,commit-msg,prepare-commit-msg, credential-bearingpre-push, warmpost-checkout, inherited fresh-clonepost-checkout, configuration non-mutation, and exact remote SHA delivery.git diff --check: clean for the review changes.js-yamlfinding and two moderate findings); no dependencies changed in this PR.Manual verification
Reviewed every changed hunk and searched production AgentCore sources for direct Git process execution. The only direct
spawn('git', ...)calls outside the shared runner are test fixture helpers. Currentmainhas no changed-path overlap since the branch base, and its merge tree is conflict-free.Related work
Supersedes #363 after this replacement's required checks and automated review are green. Do not close #363 before that condition is met.
no linked issue: this PR replaces and credits an existing pull request rather than resolving a separately tracked issue.