From 393b60cae92f854e3021e212a8d5bd7b06c0af21 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 24 Oct 2026 23:47:39 +0000 Subject: [PATCH] fix(pregate): verify goes stale when a link on the way to the absent user policy dangles A fresh run refuses a user policy path with a dangling parent link, but verify looked only at the leaf, so the parent's ENOENT read as continued absence and an old defaults pass stayed publishable. The loader and verify now share one absence check. Found by the security review of #3286. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/pregate/config.ts | 7 ++++++- lib/pregate/index.ts | 7 +++++-- test/pregate-user-policy.test.ts | 20 +++++++++++++++++++- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/pregate/config.ts b/lib/pregate/config.ts index 53526b99b..e1f2b3e20 100644 --- a/lib/pregate/config.ts +++ b/lib/pregate/config.ts @@ -240,11 +240,16 @@ function danglingLink(file: string): boolean { return false; } +/** Nothing at `file` and no dangling link on the way to it: the only state that means "no user policy", for the loader and verify alike. */ +export function userPolicyAbsent(file: string): boolean { + try { fs.statSync(file); return false; } catch (e) { return (e as NodeJS.ErrnoException).code === 'ENOENT' && !danglingLink(file); } +} + /** 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) { - if ((e as NodeJS.ErrnoException).code === 'ENOENT' && !danglingLink(file)) return null; + if ((e as NodeJS.ErrnoException).code === 'ENOENT' && userPolicyAbsent(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/lib/pregate/index.ts b/lib/pregate/index.ts index 4a385c13b..d6a52462f 100644 --- a/lib/pregate/index.ts +++ b/lib/pregate/index.ts @@ -22,7 +22,7 @@ import { regenCheck } from './checks/regen'; import { secretsCheck } from './checks/secrets'; import { straysCheck } from './checks/strays'; import { timingCheck } from './checks/timing'; -import { NON_DOWNGRADABLE } from './config'; +import { NON_DOWNGRADABLE, userPolicyAbsent } from './config'; import { sha256, withFingerprint } from './context'; import type { CheckDef, CheckResult, PregateContext, Stage } from './types'; @@ -155,7 +155,10 @@ function userPolicyChange(file: PregateFile): string | null { } const absent = file.policy_user_path; if (!absent) return null; - try { if (!fs.lstatSync(absent, { throwIfNoEntry: false })) return null; } catch (e) { return `cannot confirm ${absent} is still absent (${(e as Error).message}); rerun gstack-pregate`; } + if (userPolicyAbsent(absent)) return null; + let leaf: fs.Stats | undefined; + try { leaf = fs.lstatSync(absent, { throwIfNoEntry: false }); } catch { leaf = undefined; } + if (!leaf) return `cannot confirm ${absent} is still absent (a link on the way to it is dangling or unreadable); rerun gstack-pregate`; return `${absent} now holds a user-owned policy, and pregate.json was recorded under the defaults; rerun gstack-pregate`; } diff --git a/test/pregate-user-policy.test.ts b/test/pregate-user-policy.test.ts index fb6d2981c..b56a32032 100644 --- a/test/pregate-user-policy.test.ts +++ b/test/pregate-user-policy.test.ts @@ -14,7 +14,7 @@ import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { loadConfigFromUser, validateConfig } from '../lib/pregate/config'; import { buildContext, sha256, userConfigRejection } from '../lib/pregate/context'; -import { runPregate } from '../lib/pregate/index'; +import { runPregate, verifyPregate } from '../lib/pregate/index'; import { BASE_PREGATE, BIN, cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; import { runScriptSync } from '../lib/script-invocation'; @@ -317,6 +317,24 @@ describe('gstack-pregate with a user-owned policy', () => { } finally { fs.rmSync(outside, { recursive: true, force: true }); } }); + // Value: protects=a defaults pass stops verifying once a link on the way to the absent user path dangles, the state a fresh + // run refuses; fails_when=verify checks only the leaf and reads the broken parent as continued absence; why_new=security + // review of #3286; seam=verifyPregate with the recorded tree passed in, so only the policy check differs; no_claim=the + // bin's verify wiring (the stale-policy test covers it) + test('verify goes stale when a parent of the absent user path becomes a dangling link', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'pregate-dangle-')); + try { + const userPolicy = path.join(dir, 'projects', 's', 'pregate.json'); + const record = { schema_version: 1, tree: 'T', checks: [], policy: 'defaults', policy_user_path: userPolicy, verdict: 'pass' } as any; + expect(verifyPregate(dir, record, { tree: 'T' }).ok).toBe(true); + fs.symlinkSync(path.join(dir, 'not-yet-cloned'), path.join(dir, 'projects')); + expect((loadConfigFromUser(userPolicy) as { errors: { code: string }[] } | null)?.errors.map(e => e.code)).toEqual(['PREGATE_CONFIG_INVALID']); + const v = verifyPregate(dir, record, { tree: 'T' }); + expect([v.ok, v.code]).toEqual([false, 'PREGATE_STALE']); + expect(v.message).toContain(`cannot confirm ${userPolicy} is still absent`); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); + // Value: protects=--explain says when the user file was looked for and absent, and names a `local` id that matches no workflow lane; // fails_when=either line is dropped and a typo'd slug or lane id silently does nothing; why_new=both lines are new; seam=none; // no_claim=the line wording beyond its path and id