mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 02:30:39 +01:00
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>
This commit is contained in:
1 parent
d65304a0e1
commit
393b60cae9
3 files changed
+30
-4
No files matched your search
@@ -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' };
|
||||
}
|
||||
|
||||
@@ -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`;
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user