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