From 55b73dbdc79da220f01c9e2718ee77e440cc71ac Mon Sep 17 00:00:00 2001 From: arlophoenix Date: Mon, 19 Oct 2026 09:37:52 +1300 Subject: [PATCH] fix(pregate): decide a missing base policy from the tree, and catch dangling parent links cat-file -e checks the blob, so a missing or corrupt object still read as no base copy; ls-tree reads the tree entry, and any failure refuses. The dangling-symlink check now covers every path component, not just the file. Co-Authored-By: Claude Opus 5.5 --- lib/pregate/config.ts | 16 +++++++++--- test/pregate-user-policy.test.ts | 43 ++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/lib/pregate/config.ts b/lib/pregate/config.ts index 0c188c51f..9b7f2bd4b 100644 --- a/lib/pregate/config.ts +++ b/lib/pregate/config.ts @@ -218,8 +218,9 @@ export function loadConfigFromBase(repoRoot: string, base: string, knownChecks: if (!sha.ok || !sha.out) return { unavailable: true, base: base.replace(/^origin\//, ''), detail: `${ref} is not a resolvable commit` }; const show = git(repoRoot, ['show', `${sha.out}:${PREGATE_CONFIG_PATH}`]); if (!show.ok) { - // Absent only when git says so: a timeout, an oversized blob or a corrupt object must refuse, not hand the run to the user file. - if (git(repoRoot, ['cat-file', '-e', `${sha.out}:${PREGATE_CONFIG_PATH}`]).ok) return { unavailable: true, base: base.replace(/^origin\//, ''), detail: `${ref}:${PREGATE_CONFIG_PATH} exists but git show could not read it` }; + // Absent only when the tree says so: a timeout, an oversized blob or a missing/corrupt object must refuse, not hand the run to the user file. + const entry = git(repoRoot, ['ls-tree', '--name-only', sha.out, '--', PREGATE_CONFIG_PATH]); + if (!entry.ok || entry.out) return { unavailable: true, base: base.replace(/^origin\//, ''), detail: `${ref}:${PREGATE_CONFIG_PATH} could not be read and git cannot confirm it is absent` }; return { config: DEFAULT_CONFIG, errors: [], present: false, label: `${ref}@${sha.out.slice(0, 12)} (no ${PREGATE_CONFIG_PATH})`, kind: 'none' }; } return { ...parseConfig(show.out, knownChecks), present: true, label: `${ref}@${sha.out.slice(0, 12)}:${PREGATE_CONFIG_PATH}`, kind: 'base' }; @@ -231,12 +232,19 @@ export function loadConfigFromHead(repoRoot: string, knownChecks: readonly strin return { ...parseConfig(fs.readFileSync(file, 'utf8'), knownChecks), present: true, label: `head (unreviewed) ${PREGATE_CONFIG_PATH}`, kind: 'head' }; } +/** A symlink to nothing on the way to `file` (a dotfiles link not yet cloned): a policy the user meant to have, not an absent one. */ +function danglingLink(file: string): boolean { + for (let p = file; path.dirname(p) !== p; p = path.dirname(p)) { + try { if (fs.lstatSync(p, { throwIfNoEntry: false })?.isSymbolicLink() && !fs.existsSync(p)) return true; } catch { return true; } + } + return false; +} + /** The user-owned copy for a repo that keeps `.gstack/` out of git; the caller consults it only when the base has none. */ export function loadConfigFromUser(file: string, knownChecks: readonly string[] = []): ConfigLoad | null { let text: string; try { text = fs.readFileSync(file, 'utf8'); } catch (e) { - // A dangling symlink is a policy the user meant to have, not an absent one. - if ((e as NodeJS.ErrnoException).code === 'ENOENT' && !fs.lstatSync(file, { throwIfNoEntry: false })?.isSymbolicLink()) return null; + if ((e as NodeJS.ErrnoException).code === 'ENOENT' && !danglingLink(file)) return null; // An unreadable policy must refuse, never fall back to defaults (its guards would silently vanish). return { ...parseConfig('{}', knownChecks), errors: [{ code: 'PREGATE_CONFIG_INVALID', path: file, message: `cannot read the user-owned policy: ${(e as Error).message}` }], present: true, label: `user ${file}`, kind: 'user' }; } diff --git a/test/pregate-user-policy.test.ts b/test/pregate-user-policy.test.ts index 91bbcff7b..2abdf5fa9 100644 --- a/test/pregate-user-policy.test.ts +++ b/test/pregate-user-policy.test.ts @@ -32,6 +32,15 @@ function setup(): { repo: FixtureRepo; userPolicy: string; run: (extra?: string[ const run = (extra: string[] = []) => runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', ...extra], { GSTACK_PROJECT_SLUG: SLUG }); return { repo, userPolicy, run }; } +/** setup() with a base that commits `.gstack/pregate.json` (raw text, or a policy object). */ +function setupWithBase(base: string | Record): { repo: FixtureRepo; userPolicy: string } { + const repo = makeFixtureRepo({ pregate: null, baseFiles: { '.gstack/pregate.json': typeof base === 'string' ? base : JSON.stringify(base) } }); + repos.push(repo); + commit(repo, { 'lib/util.js': 'exports.add = (a, b) => b + a;\n' }); + const userPolicy = path.join(repo.home, 'projects', SLUG, 'pregate.json'); + fs.mkdirSync(path.dirname(userPolicy), { recursive: true }); + return { repo, userPolicy }; +} const writePolicy = (file: string, policy: Record) => fs.writeFileSync(file, JSON.stringify({ runner: 'node --test {files}', ...policy })); describe('pregate.json local', () => { @@ -62,6 +71,13 @@ describe('loadConfigFromUser', () => { const dangling = loadConfigFromUser(userPolicy); expect(dangling && 'errors' in dangling ? dangling.errors.map(e => [e.code, e.path]) : null).toEqual([['PREGATE_CONFIG_INVALID', userPolicy]]); fs.unlinkSync(userPolicy); + // A dangling link one level up (the project directory) is the same intent. + const dir = path.dirname(userPolicy); + fs.rmSync(dir, { recursive: true }); + fs.symlinkSync(path.join(path.dirname(dir), 'not-yet-cloned'), dir); + expect(loadConfigFromUser(userPolicy)).not.toBeNull(); + fs.unlinkSync(dir); + fs.mkdirSync(dir); // Malformed JSON reaches the sealed/price_file probe too; it must still come back as a refusal, never a throw. fs.writeFileSync(userPolicy, '{"local": ['); const bad = loadConfigFromUser(userPolicy); @@ -235,19 +251,26 @@ describe('gstack-pregate with a user-owned policy', () => { } }, 60_000); - // Value: protects=a base copy that exists but git show cannot read (here over spawnSync's 1 MB buffer) refuses instead of - // letting the user file replace it; fails_when=every git show failure is read as "no base copy" and the unreviewed user - // file governs a repo that commits its policy; why_new=the user-file fallback is new; seam=none; no_claim=timeouts and - // corrupt objects (same branch, not separately driven) + // Value: protects=a base copy that exists but git show cannot read (over spawnSync's 1 MB buffer, or its blob missing) + // refuses instead of letting the user file replace it; fails_when=a git show failure is read as "no base copy" and the + // unreviewed user file governs a repo that commits its policy; why_new=the user-file fallback is new; seam=none; + // no_claim=timeouts (they take the same failure branch) test('a base copy git show cannot read refuses rather than reading the user file', () => { - const repo = makeFixtureRepo({ pregate: null, baseFiles: { '.gstack/pregate.json': `{"runner": "node --test {files}"${' '.repeat(1_200_000)}}\n` } }); - repos.push(repo); - commit(repo, { 'lib/util.js': 'exports.add = (a, b) => b + a;\n' }); - const userPolicy = path.join(repo.home, 'projects', SLUG, 'pregate.json'); - fs.mkdirSync(path.dirname(userPolicy), { recursive: true }); + const { repo, userPolicy } = setupWithBase(`{"runner": "node --test {files}"${' '.repeat(1_200_000)}}\n`); writePolicy(userPolicy, { local: LOCAL_ALL }); const loaded = buildContext({ repoRoot: repo.work, base: 'main', policyFrom: 'base', allowRepoCommands: false, env: { ...process.env, ...repo.env }, progress: () => {}, cores: 16, testBackend: 'local', userConfigPath: userPolicy }); - expect('refused' in loaded ? [loaded.refused.code, loaded.refused.message] : null).toEqual(['POLICY_SOURCE_UNAVAILABLE', 'origin/main:.gstack/pregate.json exists but git show could not read it']); + expect('refused' in loaded ? [loaded.refused.code, loaded.refused.message] : null).toEqual(['POLICY_SOURCE_UNAVAILABLE', 'origin/main:.gstack/pregate.json could not be read and git cannot confirm it is absent']); + }, 60_000); + + // Value: protects=absence is decided from the tree, not the blob; fails_when=cat-file -e (a blob check) stands in for absence + // and a missing or corrupt blob lets the user file govern; why_new=the fallback is new; seam=none; no_claim=partial clones + test('a base copy whose blob is missing refuses rather than reading the user file', () => { + const { repo, userPolicy } = setupWithBase({ runner: 'node --test {files}' }); + const blob = repo.git('rev-parse', 'origin/main:.gstack/pregate.json'); + fs.rmSync(path.join(repo.work, '.git', 'objects', blob.slice(0, 2), blob.slice(2))); + writePolicy(userPolicy, { local: LOCAL_ALL }); + const loaded = buildContext({ repoRoot: repo.work, base: 'main', policyFrom: 'base', allowRepoCommands: false, env: { ...process.env, ...repo.env }, progress: () => {}, cores: 16, testBackend: 'local', userConfigPath: userPolicy }); + expect('refused' in loaded ? loaded.refused.code : null).toBe('POLICY_SOURCE_UNAVAILABLE'); }, 60_000); // Value: protects=the containment check sees through a symlinked state root and does not mistake an in-repo `..name`