From d65304a0e1fb7b7125addb5463f8534e2509639f Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 24 Oct 2026 23:46:44 +0000 Subject: [PATCH] fix(pregate): refuse a user policy whose link chain passes through the checkout userConfigRejection checked the lexical path and the final realpath only. A state root linked into the checkout, or a link hop inside it, let a branch pick which existing out-of-repo policy governs it. Every hop of the resolution chain is now checked. Found by the security review of #3286. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/pregate/context.ts | 25 ++++++++++++++++++++++++- test/pregate-user-policy.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/pregate/context.ts b/lib/pregate/context.ts index 833015e5d..ba9b4e532 100644 --- a/lib/pregate/context.ts +++ b/lib/pregate/context.ts @@ -134,6 +134,28 @@ function realish(p: string): string { while (!fs.existsSync(head) && path.dirname(head) !== head) { tail.unshift(path.basename(head)); head = path.dirname(head); } try { return path.join(fs.realpathSync.native(head), ...tail); } catch { return path.resolve(p); } } +/** + * Every location the path to `p` passes through, each symlink followed one hop at a time: a link inside the checkout picks + * the file even when the last hop points back outside it. A missing component ends the walk with the rest joined lexically. + */ +function resolutionChain(p: string): string[] { + const chain: string[] = []; + let cur = path.parse(path.resolve(p)).root; + let rest = path.relative(cur, path.resolve(p)).split(path.sep).filter(Boolean); + for (let hops = 0; rest.length && hops <= 40;) { + const next = path.join(cur, rest.shift()!); + let st: fs.Stats | undefined; + try { st = fs.lstatSync(next, { throwIfNoEntry: false }); } catch { st = undefined; } + chain.push(st ? next : path.join(next, ...rest)); + if (!st) break; + if (!st.isSymbolicLink()) { cur = next; continue; } + hops++; + const target = path.resolve(cur, fs.readlinkSync(next)); + cur = path.parse(target).root; + rest = [...path.relative(cur, target).split(path.sep).filter(Boolean), ...rest]; + } + return chain; +} const within = (root: string, p: string) => { const rel = path.relative(root, p); return rel !== '..' && !rel.startsWith(`..${path.sep}`) && !path.isAbsolute(rel); }; /** Dotenv files Bun loads from the launch directory unless started with --no-env-file. */ @@ -156,7 +178,8 @@ export function defaultUserConfigPath(root: string): string { /** Why a user policy path must not be read, or null: a relative path (no HOME) or one a PR could commit is never trusted. */ export function userConfigRejection(repoRoot: string, file: string): string | null { if (!path.isAbsolute(file)) return 'not an absolute path'; - if (within(path.resolve(repoRoot), path.resolve(file)) || within(realish(repoRoot), realish(file))) return 'inside the repository; a PR could supply it'; + const repo = path.resolve(repoRoot), real = realish(repoRoot); + if (within(repo, path.resolve(file)) || [file, ...resolutionChain(file)].some(p => within(real, realish(p)))) return 'inside the repository; a PR could supply it'; return null; } diff --git a/test/pregate-user-policy.test.ts b/test/pregate-user-policy.test.ts index 3139b35b2..fb6d2981c 100644 --- a/test/pregate-user-policy.test.ts +++ b/test/pregate-user-policy.test.ts @@ -295,6 +295,28 @@ describe('gstack-pregate with a user-owned policy', () => { } finally { fs.rmSync(outside, { recursive: true, force: true }); } }); + // Value: protects=a policy reached through the checkout is refused even when the last link points back outside it, so a + // branch cannot steer which existing out-of-repo policy governs it; fails_when=containment checks only the lexical path and + // the final target, and a state root or a link hop inside the checkout picks the file; why_new=security review of #3286; + // seam=none; no_claim=case-only path differences + test('a policy whose link chain passes through the checkout is inside, even when its final target is outside', () => { + const { repo } = setup(); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'pregate-chain-')); + try { + const permissive = path.join(outside, 'permissive.json'); + fs.writeFileSync(permissive, '{}'); + fs.mkdirSync(path.join(repo.work, '.st', 'projects', 'x'), { recursive: true }); + fs.symlinkSync(permissive, path.join(repo.work, '.st', 'projects', 'x', 'pregate.json')); + fs.symlinkSync(path.join(repo.work, '.st'), path.join(outside, 'root')); + expect(userConfigRejection(repo.work, path.join(outside, 'root', 'projects', 'x', 'pregate.json'))).toContain('inside the repository'); + fs.symlinkSync(permissive, path.join(repo.work, '.st', 'hop.json')); + fs.symlinkSync(path.join(repo.work, '.st', 'hop.json'), path.join(outside, 'leaf.json')); + expect(userConfigRejection(repo.work, path.join(outside, 'leaf.json'))).toContain('inside the repository'); + fs.symlinkSync(permissive, path.join(outside, 'direct.json')); + expect(userConfigRejection(repo.work, path.join(outside, 'direct.json'))).toBeNull(); + } finally { fs.rmSync(outside, { 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