Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions src/hook.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { resolvePolicy } from './policy-resolver.mjs';
import { engineForPolicy, resolveStagedPolicy } from './scan-target.mjs';
import { scanEntries } from './scan-session.mjs';
import {
GIT_POLICY_INPUT_COMMANDS,
GIT_POLICY_TRANSITION_COMMANDS,
GIT_REF_MUTATION_COMMANDS,
SAFE_NON_COMMIT_GIT,
Expand Down Expand Up @@ -271,6 +272,17 @@ function hookPreToolUse(input) {
'aimhooman cannot determine the policy that will apply after dynamic execution; run the Git commit separately.'
);
}
// Some ref-mutation verbs have read-only listing forms that move no
// ref and so cannot bypass the reference-transaction guard. Reading
// a repository (`git branch | grep`, `git remote -v | grep origin`,
// `git stash list | head`) is everyday work; refusing it behind a
// pipeline forced developers out of their normal workflow. A real
// mutation still carries a mutating flag or positional and stays
// subject to every check below. This sits above the transition veto
// because a command that reads cannot be made unsafe by whatever ran
// before it; it stays below the checks above, which are about not
// being able to see the repository at all.
if (gitReadOnlyRefCommand(gitCommand.verb, gitCommand.args || [])) continue;
if (gitCommand.policyTransitionRisk) {
return emitDecision(
'deny',
Expand All @@ -279,14 +291,6 @@ function hookPreToolUse(input) {
}
targetRepo = openRepo(gitCommand.cwd);
const targetProfile = enforcementPolicy(targetRepo).profile;
// Some ref-mutation verbs have read-only listing forms that move no
// ref and so cannot bypass the reference-transaction guard. Reading
// a repository (`git branch | grep`, `git remote -v | grep origin`,
// `git stash list | head`) is everyday work; refusing it behind a
// pipeline forced developers out of their normal workflow. A real
// mutation still carries a mutating flag or positional and stays
// subject to every check below.
if (gitReadOnlyRefCommand(gitCommand.verb, gitCommand.args || [])) continue;
// An unresolved subcommand/alias may itself move a ref. When its
// hook path or execution context is altered, there is no safe
// content snapshot to fall back to, so treat it like a direct ref
Expand Down Expand Up @@ -791,9 +795,12 @@ function resolveGitAliases(parsed) {
} : resolved;
commands.push(effective);
if (effective.verb === 'add') aliasAddPaths.push(...effective.addPaths);
// The same narrowing the parser applies. Without it every commit-like
// candidate is re-stamped here after parseGit has run, and the veto
// below fires again on lines where nothing moved a policy input.
aliasPolicyTransitionRisk ||= effective.verb === 'unknown'
|| GIT_POLICY_TRANSITION_COMMANDS.has(effective.verb)
|| GIT_REF_MUTATION_COMMANDS.has(effective.verb);
|| (GIT_POLICY_INPUT_COMMANDS.has(effective.verb)
&& !gitReadOnlyRefCommand(effective.verb, effective.args || []));
aliasIndexMutationRisk ||= effective.verb === 'unknown'
|| effective.indexMutationRisk
|| gitIndexMutationRisk(effective.verb, effective.args);
Expand Down
28 changes: 24 additions & 4 deletions src/shell-parse.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ export function parseGit(cmd, initialCwd = process.cwd()) {
targetUncertain: commandTargetUncertain,
pathDialectUncertain: commandPathDialectUncertain,
});
policyTransitionRisk = true;
policyTransitionRisk ||= raisesPolicyTransitionRisk(verb, verbArgs);
}
else if (verb === 'add') {
const paths = toks.slice(i + 1).filter((t) => !t.startsWith('-'));
Expand Down Expand Up @@ -464,7 +464,7 @@ export function parseGit(cmd, initialCwd = process.cwd()) {
pathDialectUncertain: commandPathDialectUncertain,
classification: 'unknown',
});
policyTransitionRisk ||= GIT_POLICY_TRANSITION_COMMANDS.has(verb);
policyTransitionRisk ||= raisesPolicyTransitionRisk(verb, verbArgs);
} else if (GIT_POLICY_TRANSITION_COMMANDS.has(verb)
|| GIT_REF_MUTATION_COMMANDS.has(verb)) {
commands.push({
Expand All @@ -481,7 +481,7 @@ export function parseGit(cmd, initialCwd = process.cwd()) {
pathDialectUncertain: commandPathDialectUncertain,
classification: 'mutation',
});
policyTransitionRisk = true;
policyTransitionRisk ||= raisesPolicyTransitionRisk(verb, verbArgs);
}
}
const indirectCommits = [
Expand Down Expand Up @@ -666,6 +666,26 @@ export const GIT_POLICY_TRANSITION_COMMANDS = new Set([
'update-ref',
]);

// Verbs whose effect a later command's policy resolution reads: they move HEAD,
// the index, or the worktree that `.aimhooman.json` is resolved from. Only these
// raise the transition flag that refuses a later Git command on the same line.
//
// Deliberately a third set rather than a narrowing of the two around it. Those
// two also decide whether a verb becomes a modelled candidate at all, so
// dropping `push` from ref mutation would take the installed-guard check for
// `git push` with it. `commit` is absent on purpose: the state it moves is state
// this tool just supervised through pre-commit, commit-msg and the ref guard,
// unlike `symbolic-ref` or `stash pop`, which move it unwatched.
export const GIT_POLICY_INPUT_COMMANDS = new Set([
'am', 'branch', 'checkout', 'cherry-pick', 'merge', 'pull', 'read-tree', 'rebase',
'replace', 'reset', 'restore', 'revert', 'rm', 'stash', 'switch', 'symbolic-ref',
'update-index', 'update-ref',
]);

function raisesPolicyTransitionRisk(verb, args = []) {
return GIT_POLICY_INPUT_COMMANDS.has(verb) && !gitReadOnlyRefCommand(verb, args);
}

// These verbs can create a commit or move a branch/reference. Their final
// safety boundary is the managed reference-transaction hook, so PreToolUse
// must not let config/environment/prefix indirection disable that boundary.
Expand Down Expand Up @@ -755,7 +775,7 @@ const GIT_TAG_MUTATING_FLAGS = new Set([

const GIT_BRANCH_READONLY_FLAGS = new Set([
'-a', '--all', '-r', '--remotes', '-l', '--list', '-v', '--verbose', '-vv',
'-q', '--quiet', '--no-color', '--color',
'-q', '--quiet', '--no-color', '--color', '--show-current',
'--sort', '--format', '--contains', '--no-contains', '--merged', '--no-merged',
'--points-at',
]);
Expand Down
72 changes: 72 additions & 0 deletions tests/hook.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3144,3 +3144,75 @@ test('an unwritable .git/info produces the same verdict as a writable one', asyn
}
}
});

// One running flag was raised after every commit and every ref-affecting verb,
// then stamped onto every later command on the same line. So the second Git
// command was refused whatever it was, and told to "run the Git commit
// separately" even on lines with no commit in them. What actually matters is
// whether the earlier command moved something a later policy resolution reads:
// HEAD, the index, or the worktree.
test('a second Git command is allowed when the first moved no policy input', async () => {
const dir = makeHookRepo('clean', 'aim-hook-second-command-');
try {
for (const command of [
'git commit -m x && git push',
'git fetch && git rebase origin/main',
'git remote -v && git push',
'git tag -l && git push --tags',
'git stash list && git commit -m x',
'git branch --show-current && git commit -m x',
]) {
assert.equal(
await invokePreToolUse(dir, { tool_name: 'Bash', tool_input: { command } }),
null,
command,
);
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

// The other half of the same decision. These four move HEAD or the index
// outside any guarded path, so a commit after them is resolved against state
// the tool never saw. They stay denied.
test('a second Git command is denied when the first moved a policy input', async () => {
const dir = makeHookRepo('clean', 'aim-hook-policy-input-');
try {
for (const command of [
'git symbolic-ref HEAD refs/heads/other && git commit -m x',
'git update-ref refs/heads/main HEAD && git commit -m x',
'git rm --cached .aimhooman.json && git commit -m x',
'git stash pop && git commit -m x',
'git checkout other-branch && git commit -m x',
]) {
const out = await invokePreToolUse(dir, { tool_name: 'Bash', tool_input: { command } });
assert.equal(out?.permissionDecision, 'deny', command);
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

// The listing carve-out was only ever exercised on clean, which is why a change
// that turned it off under strict could ship green.
test('read-only Git ref-command listings are allowed under strict too', async () => {
const dir = makeHookRepo('strict', 'aim-hook-readonly-strict-');
try {
for (const command of [
'git branch | grep feat',
'git branch -a | head',
'git remote -v | grep origin',
'git stash list | head',
'git notes list | head',
]) {
assert.equal(
await invokePreToolUse(dir, { tool_name: 'Bash', tool_input: { command } }),
null,
command,
);
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
});