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
4 changes: 4 additions & 0 deletions src/guard.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ export function scanProposedCommits(repo, commits, { rejectNote, limits = {} })
policyMigrationContexts: reviewContexts,
limits,
messageScope: authoredLocally ? 'commit' : 'changes-only',
// This guard runs over history the developer is receiving, not
// writing. A policy it cannot parse must not wedge every pull,
// merge and reset with no way forward.
tolerateUnreadablePolicy: true,
});
}
catch (error) {
Expand Down
29 changes: 22 additions & 7 deletions src/policy-resolver.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function resolvePolicy(repo, options = {}) {
if (!repo?.root || !repo?.stateDir) {
throw new TypeError('repo must be an open repository');
}
const { target = 'worktree', revision, profile, strictFloor } = options;
const { target = 'worktree', revision, profile, strictFloor, tolerant = false } = options;
let resolved;
if (target === 'worktree') {
resolved = resolveWorktreePolicy(repo);
Expand All @@ -48,11 +48,26 @@ export function resolvePolicy(repo, options = {}) {
if (typeof revision !== 'string' || !revision) {
throw new PolicyTargetError('commit target needs a revision');
}
resolved = resolveGitPolicy(
repo,
readCommitPath(repo, revision, POLICY_PATH),
'commit-policy',
);
const result = readCommitPath(repo, revision, POLICY_PATH);
try {
resolved = resolveGitPolicy(repo, result, 'commit-policy');
} catch (error) {
// History is not the caller's to fix. A policy a future version
// wrote cannot be read here, and throwing turns every pull, merge
// and reset that carries it into a refusal with no way forward. The
// caller asks for tolerance and decides the fallback; the worktree,
// the index, and anything the developer asked about directly still
// get the loud error.
if (!tolerant || !(error instanceof ProjectPolicyError)) throw error;
return {
profile: null,
source: 'commit-policy',
target: result.target,
policy_object_id: result.oid,
policy_mode: result.mode,
unparseable: error.message,
};
}
} else {
throw new PolicyTargetError('must be worktree, staged, or commit');
}
Expand Down Expand Up @@ -141,7 +156,7 @@ function resolveGitPolicy(repo, result, source) {
};
}

function localFallback(repo, target) {
export function localFallback(repo, target) {
const config = loadConfig(repo.stateDir);
return {
profile: config.profile,
Expand Down
47 changes: 38 additions & 9 deletions src/scan-target.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Engine, newEngineWithDiagnostics } from './scan.mjs';
import { loadOverrides } from './state.mjs';
import { applyExplicitProfile, applyStrictFloor, resolvePolicy } from './policy-resolver.mjs';
import { applyExplicitProfile, applyStrictFloor, localFallback, resolvePolicy } from './policy-resolver.mjs';
import {
GitRevisionError,
stagedEntries,
Expand Down Expand Up @@ -155,19 +155,41 @@ export function resolveStagedPolicy(repo, explicitProfile) {
function scanCommit(repo, options) {
if (!options.revision) throw new TypeError('commit scan needs a revision');
const snapshot = commitSnapshot(repo, options.revision);
const rawPolicy = resolvePolicy(repo, { target: 'commit', revision: snapshot.commit });
// The final ref guard scans history the developer did not write, so a policy
// this version cannot parse must not turn every pull, merge and reset into a
// refusal. It asks for tolerance; the profile then comes from local config
// and the reason is reported. Direct questions (check --commit, audit) still
// get the loud error.
const tolerant = Boolean(options.tolerateUnreadablePolicy);
const unreadablePolicies = [];
const resolvedPolicy = resolvePolicy(repo, { target: 'commit', revision: snapshot.commit, tolerant });
if (resolvedPolicy.unparseable) unreadablePolicies.push(resolvedPolicy.unparseable);
const rawPolicy = resolvedPolicy.unparseable
? {
...localFallback(repo, resolvedPolicy.target),
policy_object_id: resolvedPolicy.policy_object_id,
policy_mode: resolvedPolicy.policy_mode,
}
: resolvedPolicy;
// Bind policy-migration acks to the repository's current HEAD — the state an
// ack is recorded against (policy-review --head <repo HEAD> --transition X) —
// matching scanRange and resolveStagedPolicy. Probing with snapshot.commit
// almost never matched a real ack, so `check --commit X` spuriously blocked
// transitions that `check --range` honored. Exact-match security on every
// ack field is unchanged; a null head simply fails closed (no ack honored).
const headBaseline = headPolicy(repo);
const headBaseline = headPolicy(repo, tolerant);
const head = headBaseline?.target?.startsWith('commit:')
? headBaseline.target.slice('commit:'.length) : null;
const strictParentPolicies = snapshot.parents
.map((parent) => resolvePolicy(repo, { target: 'commit', revision: parent }))
.filter(isVersionedStrict);
const parentPolicies = snapshot.parents
.map((parent) => resolvePolicy(repo, { target: 'commit', revision: parent, tolerant }));
for (const parent of parentPolicies) {
if (parent.unparseable) unreadablePolicies.push(parent.unparseable);
}
// A parent whose policy cannot be read might have been strict. Counting it
// toward the floor keeps the guard conservative; ignoring it would let an
// unreadable file switch strict enforcement off.
const unreadableParent = parentPolicies.some((parent) => parent.unparseable);
const strictParentPolicies = parentPolicies.filter(isVersionedStrict);
const policyMigrationContexts = options.policyMigrationContexts ?? (head ? [{
head,
transition: snapshot.commit,
Expand All @@ -181,7 +203,8 @@ function scanCommit(repo, options) {
newMode: rawPolicy.policy_mode,
})
));
const floor = strictParentPolicies.length > 0 && !acknowledged && !isVersionedStrict(rawPolicy)
const floor = (strictParentPolicies.length > 0 || unreadableParent)
&& !acknowledged && !isVersionedStrict(rawPolicy)
? 'parent-strict-floor'
: null;
const policy = effectivePolicy(
Expand All @@ -205,6 +228,12 @@ function scanCommit(repo, options) {
const accumulator = createAccumulator(options.limits);
accumulator.addSkipped(loaded.skipped);
const diagnostics = [...loaded.diagnostics];
for (const message of unreadablePolicies) {
diagnostics.push({
level: 'warning',
message: `${message}; scanned ${snapshot.commit} under the ${policy.profile} profile instead`,
});
}

if (snapshot.shallowBoundary) {
const message = 'shallow repository: commit scan cannot prove parent policy; fetch full history (e.g. fetch-depth: 0)';
Expand Down Expand Up @@ -550,9 +579,9 @@ function effectivePolicy(raw, explicitProfile, floorSource, enforcedObjectIds =
return applyExplicitProfile(floored, explicitProfile);
}

function headPolicy(repo) {
function headPolicy(repo, tolerant = false) {
try {
return resolvePolicy(repo, { target: 'commit', revision: 'HEAD' });
return resolvePolicy(repo, { target: 'commit', revision: 'HEAD', tolerant });
} catch (error) {
if (error instanceof GitRevisionError) return null;
throw error;
Expand Down
57 changes: 57 additions & 0 deletions tests/policy-resolver.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
resolvePolicy,
} from '../src/policy-resolver.mjs';
import { loadProjectPolicy, ProjectPolicyError, saveConfig } from '../src/state.mjs';
import { scanGitTarget } from '../src/scan-target.mjs';

function freshRepo() {
const dir = mkdtempSync(join(tmpdir(), 'aim-policy-target-'));
Expand Down Expand Up @@ -199,3 +200,59 @@ test('worktree policy resolution rejects valid and dangling symlinks', {
rmSync(dir, { recursive: true, force: true });
}
});

// A policy this version cannot parse, anywhere in fetched history, used to make
// the final ref guard refuse every update that would introduce it — pull, reset
// and merge alike, with no message pointing at a way out. The guard needs a
// resolution it can carry on from; the loud error stays for the worktree, the
// index, and anything the developer asked about directly.
test('a commit policy this version cannot parse resolves tolerantly instead of throwing', () => {
const dir = freshRepo();
try {
const repo = openRepo(dir);
writeFileSync(join(dir, '.aimhooman.json'), JSON.stringify({ schema_version: 1, profile: 'clean', exclude: ['vendor/**'] }));
execFileSync('git', ['add', '.aimhooman.json'], { cwd: dir });
execFileSync('git', ['commit', '-q', '-m', 'unreadable policy'], { cwd: dir });

assert.throws(
() => resolvePolicy(repo, { target: 'commit', revision: 'HEAD' }),
ProjectPolicyError,
'the direct question still gets the loud answer',
);

const tolerant = resolvePolicy(repo, { target: 'commit', revision: 'HEAD', tolerant: true });
assert.equal(tolerant.profile, null, 'no profile can be read from it');
assert.match(tolerant.unparseable, /unsupported field/, 'and the reason travels with the result');
} finally {
rmSync(dir, { recursive: true, force: true });
}
});

// The other half: the guard that receives history must be able to finish a scan
// of a commit whose policy it cannot read, under a profile it can reach, and say
// which commit it fell back on.
test('a commit scan tolerates an unreadable policy and reports the fallback', () => {
const dir = freshRepo();
try {
const repo = openRepo(dir);
writeFileSync(join(dir, '.aimhooman.json'), JSON.stringify({ schema_version: 1, profile: 'clean', exclude: ['vendor/**'] }));
execFileSync('git', ['add', '.aimhooman.json'], { cwd: dir });
execFileSync('git', ['commit', '-q', '-m', 'unreadable policy'], { cwd: dir });
const head = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim();

assert.throws(
() => scanGitTarget(repo, { kind: 'commit', revision: head }),
ProjectPolicyError,
'a direct scan still refuses loudly',
);

const scan = scanGitTarget(repo, { kind: 'commit', revision: head, tolerateUnreadablePolicy: true });
assert.ok(scan.profile, 'the scan runs under a profile it could reach');
assert.ok(
scan.diagnostics.some((d) => d.level === 'warning' && d.message.includes(head)),
'and names the commit it could not read the policy of',
);
} finally {
rmSync(dir, { recursive: true, force: true });
}
});