refactor(#4653): replace the allowAbsolute flag with a named acceptance policy
Phase 3 of epic #4636, stage 3b. Satisfies #4653's criterion that `opts.allowAbsolute` become "a named acceptance policy on the predicate, not a per-call-site boolean". The flag was actively misleading at the call site. `{ allowAbsolute: true }` reads as "containment is relaxed here". It never was: an absolute path that resolves outside the root is rejected exactly as a traversal is. The flag only ever controlled whether an absolute candidate was CONSIDERED. On a security predicate that is the wrong thing for a reviewer to have to infer, and 31 call sites were asking them to infer it. PathAcceptance.RelativeOnly relative candidates only PathAcceptance.AbsoluteInsideRoot absolute accepted, containment unchanged The three exported wrappers take the policy and translate it inward. validatePath keeps its internal `{ allowAbsolute }` opts and its body untouched — the engine is not re-derived here either, only the exported surface is renamed. MEASURED, NOT ESTIMATED. 31 call sites across 10 files, counted by walking the AST with the repo's own @typescript-eslint/parser rather than grepping: a text match would have folded in the options-type declaration, default parameter values and comments. All 31 pass the literal `true`; none passes `false` or a dynamic value, so the migration is uniform and `RelativeOnly` is purely the existing default made nameable. audit.cts alone holds 18 of them. This migration is compiler-verified in a way the containment-value migration in the previous commit was not: the parameter type changed from an object to a string union, so any missed site is a build error rather than a silent behavioral difference. That is why a 31-site mechanical edit is acceptable in the phase whose stated risk is the width of mechanical change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -25,6 +25,7 @@ const {
|
||||
validatePromptStructure,
|
||||
assertWithinRoot,
|
||||
tryWithinRoot,
|
||||
PathAcceptance,
|
||||
} = require('../gsd-core/bin/lib/security.cjs');
|
||||
|
||||
// ─── Path Traversal Prevention ──────────────────────────────────────────────
|
||||
@@ -58,12 +59,12 @@ describe('assertWithinRoot / tryWithinRoot — engine invariance', () => {
|
||||
});
|
||||
|
||||
test('allows absolute paths within base when opted in', () => {
|
||||
const r = tryWithinRoot(path.join(base, 'src/file.js'), base, { allowAbsolute: true });
|
||||
const r = tryWithinRoot(path.join(base, 'src/file.js'), base, PathAcceptance.AbsoluteInsideRoot);
|
||||
assert.ok(r !== null);
|
||||
});
|
||||
|
||||
test('rejects absolute paths outside base even when opted in', () => {
|
||||
assert.equal(tryWithinRoot('/etc/passwd', base, { allowAbsolute: true }), null);
|
||||
assert.equal(tryWithinRoot('/etc/passwd', base, PathAcceptance.AbsoluteInsideRoot), null);
|
||||
});
|
||||
|
||||
test('rejects null bytes', () => {
|
||||
@@ -1367,7 +1368,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
|
||||
});
|
||||
|
||||
test('returns the resolved path for an absolute input INSIDE the root when {allowAbsolute:true}', () => {
|
||||
const resolved = assertWithinRoot(path.join(base, 'src/file.js'), base, null, { allowAbsolute: true });
|
||||
const resolved = assertWithinRoot(path.join(base, 'src/file.js'), base, null, PathAcceptance.AbsoluteInsideRoot);
|
||||
assert.equal(resolved, path.resolve(base, 'src/file.js'));
|
||||
});
|
||||
|
||||
@@ -1376,7 +1377,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
|
||||
});
|
||||
|
||||
test('throws on an absolute path outside the root even with {allowAbsolute:true}', () => {
|
||||
assert.throws(() => assertWithinRoot('/etc/passwd', base, null, { allowAbsolute: true }));
|
||||
assert.throws(() => assertWithinRoot('/etc/passwd', base, null, PathAcceptance.AbsoluteInsideRoot));
|
||||
});
|
||||
|
||||
test('throws on a null byte', () => {
|
||||
@@ -1406,7 +1407,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
|
||||
});
|
||||
|
||||
test('returns the resolved path for an absolute input INSIDE the root when {allowAbsolute:true}', () => {
|
||||
const resolved = tryWithinRoot(path.join(base, 'src/file.js'), base, { allowAbsolute: true });
|
||||
const resolved = tryWithinRoot(path.join(base, 'src/file.js'), base, PathAcceptance.AbsoluteInsideRoot);
|
||||
assert.equal(resolved, path.resolve(base, 'src/file.js'));
|
||||
});
|
||||
|
||||
@@ -1416,7 +1417,7 @@ describe('assertWithinRoot / tryWithinRoot — narrowed export (#4653)', () => {
|
||||
});
|
||||
|
||||
test('returns exactly null for an absolute path outside the root even with {allowAbsolute:true}', () => {
|
||||
const result = tryWithinRoot('/etc/passwd', base, { allowAbsolute: true });
|
||||
const result = tryWithinRoot('/etc/passwd', base, PathAcceptance.AbsoluteInsideRoot);
|
||||
assert.strictEqual(result, null);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user