From 3925839f2a5fe047dbe9497eddb745a3407acf01 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 12 Sep 2026 09:09:20 -0400 Subject: [PATCH] test(#4652): failing-first coverage for the four unconfined boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 of epic #4636, absorbing #4327 and #4354. Tests only; no fix. These MUST fail. Four CLI boundaries join externally-supplied input to a managed root with no containment validation. Each was driven through the real CLI and confirmed unconfined before the assertions were written: todo complete src/commands.cts cmdTodoComplete check predicate --phase-dir check-command-router cmdCheckPredicate check decision-coverage-plan check-command-router resolvePath check gap-analysis.plan-post check-command-router Boundary 1 is worse than the issue describes. #4327 reports that a traversal name "resolves outside the todos root", which reads as an information leak. Measured, it is destructive: `todo complete ../../../../b1out/leak.md` exited 0, MOVED the outside file into completed/, and unlinked the original. The file was gone. cmdTodoComplete ends in fs.unlinkSync(sourcePath), so an unconfined name does not merely read across the boundary, it consumes across it. Boundary 2 reproduces #4354 exactly: a BLOCKING gate returned {"block":false,"details":{"match":true}} sourced entirely from a SECURITY.md in a directory the caller chose, outside the project. Boundaries 3 and 4 are not named in the epic. Both accepted an outside phase dir and exited 0. Rows that exist because they are the ones nobody enumerates: - ORDERING. A real file is created outside the todos root, then the traversal name targeting it is asserted rejected AND the outside file asserted still present and unmoved. #4327 notes the existence check and the move target BOTH follow the unvalidated join, so a rejection that lands after the read has already leaked — and, per the finding above, after the unlink has already destroyed. - `a/../../b.md` — looks balanced, resolves outside. - --dry-run must reject too; a preview must not leak a resolved outside path. - ${PHASE_DIR} interpolation into a command-exit-zero predicate is the SECOND predicate kind, which a fix inside gate-predicate-evaluator.cts would miss. - An absolute path INSIDE the project must still be accepted at every boundary — absolute is not a synonym for escaping. Cross-boundary rows loop over one shared list of escaping inputs and assert all four reject with the same shape, so four sites adopting one predicate cannot drift into four rejection contracts. Property tests cover BOTH directions — outside is always rejected, inside is always accepted. A property asserting only rejection is satisfied by a predicate that rejects everything, which is the degenerate-implementation trap found in Phase 1's review. Both are seeded. Regressions fold into the owning module suites rather than a new tests/fix-NNNN-*.test.cjs, per scripts/lint-regression-test-names.cjs. Co-Authored-By: Claude Opus 5 --- .../check-gap-analysis-plan-post-e2e.test.cjs | 134 ++++++++++++++++ tests/check-predicate.test.cjs | 125 ++++++++++++++- tests/commands.test.cjs | 146 ++++++++++++++++++ tests/security.test.cjs | 139 ++++++++++++++++- 4 files changed, 542 insertions(+), 2 deletions(-) diff --git a/tests/check-gap-analysis-plan-post-e2e.test.cjs b/tests/check-gap-analysis-plan-post-e2e.test.cjs index 039918f04..c298e9c8d 100644 --- a/tests/check-gap-analysis-plan-post-e2e.test.cjs +++ b/tests/check-gap-analysis-plan-post-e2e.test.cjs @@ -20,6 +20,7 @@ const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const os = require('os'); const { spawnSync } = require('child_process'); const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); @@ -717,3 +718,136 @@ describe('resolveLoopHooks plan:post — pure function against real registry', ( assert.strictEqual(entry.gates[0].capId, 'gap-analysis'); }); }); + +// ─── #4652: containment boundaries — resolvePath (check-command-router.cts:92) +// and `check gap-analysis.plan-post ` ─────────────────────────────── +// +// Boundary 3: `check decision-coverage-plan ` resolves the phase-dir +// positional via `resolvePath()`, which just does +// `path.isAbsolute(p) ? p : path.join(projectDir, p)` — no containment check. +// Boundary 4: `check gap-analysis.plan-post ` takes `args[2]` +// unconfined and joins it directly in `runGapAnalysis` (gap-checker.cts). + +function runDecisionCoveragePlan(extraFlags, phaseDir, contextPath, cwd) { + return runGsdTools(['query', 'check.decision-coverage-plan', ...extraFlags, phaseDir, contextPath], cwd); +} + +describe('resolvePath / check decision-coverage-plan — containment boundary (#4652)', () => { + let tmpDir; + let phaseDir; + let outsideDir; + + beforeEach(() => { + tmpDir = createTempProject(); + phaseDir = path.join(tmpDir, '.planning', 'phases', '01-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-decision-outside-')); + }); + + afterEach(() => { + cleanup(tmpDir); + cleanup(outsideDir); + }); + + test('[RED #4652] an outside phase-dir is rejected (currently resolves and proceeds unconfined)', () => { + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + fs.writeFileSync( + contextPath, + '# Phase Context\n\n\n## Implementation Decisions\n\n- **D-01:** Use pattern X\n\n', + ); + fs.writeFileSync(path.join(outsideDir, '01-PLAN.md'), '# Plan\n\nImplements D-01.\n'); + const relOutside = path.relative(tmpDir, outsideDir); + + const result = runGsdTools( + ['--json-errors', 'query', 'check.decision-coverage-plan', relOutside, contextPath], + tmpDir, + ); + + assert.strictEqual( + result.success, + false, + `an outside phase-dir must be rejected before evaluating plan coverage ` + + `(currently: ${result.success ? `SUCCEEDED with output ${result.output}` : 'failed for an unrelated reason'})`, + ); + }); + + test('[regression] a valid in-project relative phase-dir still proceeds', () => { + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + fs.writeFileSync( + contextPath, + '# Phase Context\n\n\n## Implementation Decisions\n\n- **D-01:** Use pattern X\n\n', + ); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\n\nImplements D-01.\n'); + const relPhaseDir = path.relative(tmpDir, phaseDir); + + const result = runDecisionCoveragePlan([], relPhaseDir, contextPath, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.passed, true, 'in-project phase-dir with covered decision must pass'); + }); + + test('[regression] an absolute path INSIDE the project is accepted', () => { + const contextPath = path.join(phaseDir, 'CONTEXT.md'); + fs.writeFileSync( + contextPath, + '# Phase Context\n\n\n## Implementation Decisions\n\n- **D-01:** Use pattern X\n\n', + ); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\n\nImplements D-01.\n'); + + const result = runDecisionCoveragePlan([], phaseDir, contextPath, tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.passed, true, 'absolute in-project phase-dir must be accepted'); + }); +}); + +describe('check gap-analysis.plan-post — containment boundary (#4652)', () => { + let tmpDir; + let phaseDir; + let outsideDir; + + beforeEach(() => { + tmpDir = createTempProject(); + phaseDir = path.join(tmpDir, '.planning', 'phases', '01-test'); + fs.mkdirSync(phaseDir, { recursive: true }); + const init = runGsdTools('config-ensure-section', tmpDir); + assert.ok(init.success, `config-ensure-section failed: ${init.error}`); + outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-gap-outside-')); + }); + + afterEach(() => { + cleanup(tmpDir); + cleanup(outsideDir); + }); + + test('[RED #4652] an outside phase-dir is rejected (currently resolves and proceeds unconfined)', () => { + fs.writeFileSync(path.join(outsideDir, '01-PLAN.md'), '# Plan\n\nSome content.\n'); + const relOutside = path.relative(tmpDir, outsideDir); + + const result = runGsdTools(['--json-errors', 'check', 'gap-analysis.plan-post', relOutside, '--raw'], tmpDir); + + assert.strictEqual( + result.success, + false, + `an outside phase-dir must be rejected ` + + `(currently: ${result.success ? `SUCCEEDED with output ${result.output}` : 'failed for an unrelated reason'})`, + ); + }); + + test('[regression] a valid phase-dir still proceeds (advisory, block:false)', () => { + writeRequirements(path.join(tmpDir, '.planning'), ['REQ-01']); + writePlan(phaseDir, '01', '# Plan\n\nImplements REQ-01.\n'); + + const r = runGapCheck([phaseDir], tmpDir); + assert.ok(r.success, `check failed: ${r.error}`); + const out = JSON.parse(r.output); + assert.strictEqual(out.block, false, 'gap-analysis is always advisory'); + }); + + test('[regression] a missing phase-dir argument still gives the existing SDK_MISSING_ARG error', () => { + const result = runGsdTools(['--json-errors', 'check', 'gap-analysis.plan-post', '--raw'], tmpDir); + assert.strictEqual(result.success, false, 'must fail when phaseDir omitted'); + const parsed = JSON.parse(result.error); + assert.strictEqual(parsed.reason, 'sdk_missing_arg', 'must keep the existing SDK_MISSING_ARG reason'); + }); +}); diff --git a/tests/check-predicate.test.cjs b/tests/check-predicate.test.cjs index f35d7bec0..6dd35181c 100644 --- a/tests/check-predicate.test.cjs +++ b/tests/check-predicate.test.cjs @@ -13,11 +13,15 @@ * (a 100ms timeout killing `sleep 1`), so there is no orphan/leak risk. */ -const { describe, test } = require('node:test'); +const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const os = require('os'); const { evaluatePredicate } = require('../gsd-core/bin/lib/gate-predicate-evaluator.cjs'); const { buildPredicateDeps, parsePredicateFlags } = require('../gsd-core/bin/lib/check-command-router.cjs'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); // ─── buildPredicateDeps: real subprocess exit mapping ───────────────────────── @@ -153,3 +157,122 @@ describe('partitionPredicateArgs (#4130 follow-up)', () => { assert.deepEqual(positionals, ['p1', 'p2', 'p3']); }); }); + +// ─── #4354: `check predicate --phase-dir` containment boundary ─────────────── +// +// cmdCheckPredicate passes the `--phase-dir` flag VERBATIM into PredicateContext +// (src/check-command-router.cts) with no containment validation. Both predicate +// kinds read/interpolate that value: `artifact-frontmatter-equals` resolves it +// as `targetDir` for `findPhaseArtifact`, and `command-exit-zero` interpolates +// it into `${PHASE_DIR}` in the shelled-out command. These tests reproduce the +// issue's exact repro and prove the boundary is currently unconfined. + +describe('check predicate --phase-dir — containment boundary (#4354)', () => { + let projDir; + let outsideDir; + + beforeEach(() => { + projDir = createTempProject(); + fs.mkdirSync(path.join(projDir, '.planning', 'phases', '05-x'), { recursive: true }); + outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-predicate-outside-')); + }); + + afterEach(() => { + cleanup(projDir); + cleanup(outsideDir); + }); + + test('[RED #4354] the issue\'s exact repro: artifact-frontmatter-equals against a foreign SECURITY.md via an outside --phase-dir must be rejected, not evaluated', () => { + fs.writeFileSync( + path.join(outsideDir, 'SECURITY.md'), + '---\nstatus: passed\n---\n# Security\n', + ); + + const predicate = JSON.stringify({ + kind: 'artifact-frontmatter-equals', + artifact: 'SECURITY.md', + field: 'status', + equals: 'passed', + }); + + const result = runGsdTools( + ['--json-errors', 'check', 'predicate', '--predicate', predicate, '--phase-dir', outsideDir, '--raw'], + projDir, + ); + + // CURRENT BUG (documented, not asserted as desired): this command today + // succeeds and prints {"block":false,...} — a BLOCKING gate passing on + // foreign evidence read from OUTSIDE the project. REQUIRED behavior: + // the outside --phase-dir must be rejected before evaluation. + assert.strictEqual( + result.success, + false, + `an outside --phase-dir must be rejected before evaluating the predicate ` + + `(currently: ${result.success ? `SUCCEEDED with output ${result.output}` : 'failed for an unrelated reason'})`, + ); + }); + + test('[RED #4354] a command-exit-zero predicate interpolating ${PHASE_DIR} with an outside --phase-dir must also be rejected', () => { + fs.writeFileSync(path.join(outsideDir, 'marker.txt'), 'outside-marker\n'); + + const predicate = JSON.stringify({ + kind: 'command-exit-zero', + command: 'test -f "${PHASE_DIR}/marker.txt"', + }); + + const result = runGsdTools( + ['--json-errors', 'check', 'predicate', '--predicate', predicate, '--phase-dir', outsideDir, '--raw'], + projDir, + ); + + assert.strictEqual( + result.success, + false, + `a command-exit-zero predicate interpolating an outside --phase-dir must be rejected ` + + `(currently: ${result.success ? `SUCCEEDED with output ${result.output}` : 'failed for an unrelated reason'})`, + ); + }); + + test('[regression] a valid in-project --phase-dir still evaluates', () => { + const phaseDir = path.join(projDir, '.planning', 'phases', '05-x'); + fs.writeFileSync( + path.join(phaseDir, 'SECURITY.md'), + '---\nstatus: passed\n---\n# Security\n', + ); + const predicate = JSON.stringify({ + kind: 'artifact-frontmatter-equals', + artifact: 'SECURITY.md', + field: 'status', + equals: 'passed', + }); + + const result = runGsdTools( + ['check', 'predicate', '--predicate', predicate, '--phase-dir', phaseDir, '--raw'], + projDir, + ); + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.strictEqual(parsed.block, false, 'in-project phase-dir evaluation must still pass'); + }); + + test('[regression] no --phase-dir at all still falls back to cwd and evaluates', () => { + fs.writeFileSync( + path.join(projDir, 'SECURITY.md'), + '---\nstatus: passed\n---\n# Security\n', + ); + const predicate = JSON.stringify({ + kind: 'artifact-frontmatter-equals', + artifact: 'SECURITY.md', + field: 'status', + equals: 'passed', + }); + + const result = runGsdTools( + ['check', 'predicate', '--predicate', predicate, '--raw'], + projDir, + ); + assert.ok(result.success, `Command failed: ${result.error}`); + const parsed = JSON.parse(result.output); + assert.strictEqual(parsed.block, false, 'cwd-fallback evaluation must still pass'); + }); +}); diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index 94f30e8ed..30280193c 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -13,6 +13,7 @@ const { test, describe, after, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const os = require('os'); const { runGsdTools, createTempProject, createTempDir, cleanup } = require('./helpers.cjs'); const { splitLines } = require('../gsd-core/bin/lib/text-lines.cjs'); const fc = require('./helpers/fast-check-setup.cjs'); @@ -798,6 +799,151 @@ describe('todo complete command', () => { }); }); +// ───────────────────────────────────────────────────────────────────────────── +// todo complete — containment boundary (#4327) +// +// cmdTodoComplete joins the externally-supplied `filename` into +// todosDir(cwd)/pending with NO containment validation (src/commands.cts). +// These tests prove the boundary is currently unconfined — a traversal name +// is neither rejected before the existence check nor before the move. +// ───────────────────────────────────────────────────────────────────────────── + +describe('todo complete — containment boundary (#4327)', () => { + let tmpDir; + let pendingDir; + + beforeEach(() => { + tmpDir = createTempProject(); + pendingDir = path.join(tmpDir, '.planning', 'todos', 'pending'); + fs.mkdirSync(pendingDir, { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // ── Regressions: normal completion must keep working ───────────────────── + + test('[regression] a valid existing todo name completes and the file moves to completed/', () => { + fs.writeFileSync(path.join(pendingDir, 'ok-name.md'), '---\nstatus: pending\n---\n'); + const result = runGsdTools(['todo', 'complete', 'ok-name.md'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.ok(!fs.existsSync(path.join(pendingDir, 'ok-name.md')), 'removed from pending'); + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'todos', 'completed', 'ok-name.md')), + 'present in completed', + ); + }); + + test('[regression] a name with dots like "2026-09-12.some.todo.md" completes', () => { + fs.writeFileSync(path.join(pendingDir, '2026-09-12.some.todo.md'), '---\nstatus: pending\n---\n'); + const result = runGsdTools(['todo', 'complete', '2026-09-12.some.todo.md'], tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + assert.ok( + fs.existsSync(path.join(tmpDir, '.planning', 'todos', 'completed', '2026-09-12.some.todo.md')), + 'dotted-name todo completes', + ); + }); + + test('[regression] a missing name still produces the existing "Todo not found" error', () => { + const result = runGsdTools(['todo', 'complete', 'does-not-exist.md'], tmpDir); + assert.ok(!result.success, 'must still fail'); + assert.ok(result.error.includes('not found'), 'error must still mention "not found"'); + }); + + // ── MUST BE REJECTED — currently unconfined (RED) ───────────────────────── + + const ESCAPING_NAMES = ['../../escaped', '../sibling.md', 'sub/name.md', 'a/../../b.md']; + + for (const name of ESCAPING_NAMES) { + test(`[RED #4327] "todo complete ${name}" must be rejected (currently unconfined)`, () => { + const targetPath = path.join(pendingDir, name); + fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + fs.writeFileSync(targetPath, '---\nstatus: pending\n---\nSENTINEL\n'); + + const result = runGsdTools(['todo', 'complete', name], tmpDir); + assert.strictEqual( + result.success, + false, + `"${name}" must be rejected as an escaping/invalid todo name (currently ` + + `${result.success ? 'SUCCEEDED — unconfined join, no containment check' : 'failed for an unrelated reason'})`, + ); + }); + } + + test('[RED #4327] an absolute path outside the project is rejected', () => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-todo-outside-')); + try { + const outsideFile = path.join(outsideDir, 'evil.md'); + fs.writeFileSync(outsideFile, '---\nstatus: pending\n---\nSENTINEL\n'); + const result = runGsdTools(['todo', 'complete', outsideFile], tmpDir); + assert.strictEqual(result.success, false, 'an absolute path outside the project must be rejected'); + } finally { + cleanup(outsideDir); + } + }); + + // ── CRITICAL ORDERING (#4327): the existence check AND the move target ──── + // both follow the unvalidated join, so a rejection that happens after the + // read has already leaked. Prove the outside file is neither read-through + // nor moved/deleted by a (today, absent) rejection. + + test('[RED #4327] CRITICAL ORDERING: a traversal name resolving to a real outside file is rejected WITHOUT the outside file being moved or deleted', () => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-todo-outside-')); + try { + const outsideFile = path.join(outsideDir, 'leak-target.md'); + const sentinel = '---\nstatus: pending\n---\nSENTINEL-LEAK\n'; + fs.writeFileSync(outsideFile, sentinel); + const relName = path.relative(pendingDir, outsideFile); + + const result = runGsdTools(['todo', 'complete', relName], tmpDir); + + assert.strictEqual( + result.success, + false, + `traversal name "${relName}" resolving to ${outsideFile} must be rejected`, + ); + assert.ok( + fs.existsSync(outsideFile), + 'the outside file must still exist — a rejected completion must not move/delete it', + ); + assert.strictEqual( + fs.readFileSync(outsideFile, 'utf-8'), + sentinel, + 'the outside file content must be byte-for-byte untouched', + ); + const completedDir = path.join(tmpDir, '.planning', 'todos', 'completed'); + if (fs.existsSync(completedDir)) { + assert.ok( + !fs.readdirSync(completedDir).includes('leak-target.md'), + 'the outside file must never land inside completed/', + ); + } + } finally { + cleanup(outsideDir); + } + }); + + test('[RED #4327] "todo complete --dry-run" must be rejected — a dry run must not leak a resolved outside path', () => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-todo-outside-')); + try { + const outsideFile = path.join(outsideDir, 'dry-leak.md'); + fs.writeFileSync(outsideFile, '---\nstatus: pending\n---\n'); + const relName = path.relative(pendingDir, outsideFile); + + const result = runGsdTools(['todo', 'complete', relName, '--dry-run'], tmpDir); + + assert.strictEqual( + result.success, + false, + `dry-run completion of traversal name "${relName}" resolving to ${outsideFile} must be rejected`, + ); + } finally { + cleanup(outsideDir); + } + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // todo match-phase command // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/security.test.cjs b/tests/security.test.cjs index e75f0944e..8467bdd2c 100644 --- a/tests/security.test.cjs +++ b/tests/security.test.cjs @@ -9,7 +9,8 @@ const assert = require('node:assert/strict'); const path = require('path'); const os = require('os'); const fs = require('fs'); -const { cleanup } = require('./helpers.cjs'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); +const fc = require('./helpers/fast-check-setup.cjs'); const { validatePath, @@ -1209,3 +1210,139 @@ describe('SECURE: ASVS level scaling (#1627)', () => { }); }); } + +// ─── #4652: validatePath property tests (both directions) ──────────────────── +// +// PR1: any relative path with `..` segments that resolves OUTSIDE the root is +// ALWAYS rejected. PR2: any path that resolves INSIDE the root is ALWAYS +// accepted. Both directions are required — a predicate rejecting everything +// would vacuously satisfy PR1 alone. + +describe('validatePath — containment properties (#4652)', () => { + // A path SEGMENT: letters/digits/dash/underscore, non-empty, never '.' or '..' + // by construction so every generated escaping path is escaping ONLY via the + // deliberately-injected `..` components below (never an accidental one). + const segmentArb = fc + .stringMatching(/^[A-Za-z0-9_-]+$/) + .filter((s) => s.length > 0 && s !== '.' && s !== '..'); + + test('PR1: a relative path with enough leading ".." segments to resolve OUTSIDE the root is ALWAYS rejected', () => { + fc.assert(fc.property( + fc.array(segmentArb, { minLength: 1, maxLength: 4 }), // base depth below an anchor + fc.integer({ min: 1, max: 8 }), // extra ".." beyond the base depth + fc.array(segmentArb, { minLength: 0, maxLength: 3 }), // trailing segments after escaping + (baseSegments, extraUp, tailSegments) => { + // Root sits `baseSegments.length` levels below a stable anchor. + const anchor = path.resolve('/gsd-root-anchor'); + const root = path.join(anchor, ...baseSegments); + // Enough ".." to exit past the anchor itself, guaranteeing the resolved + // path is OUTSIDE root (and outside the anchor) regardless of anchor + // depth on this OS. + const upCount = baseSegments.length + extraUp; + const traversal = path.join(...Array(upCount).fill('..'), ...tailSegments, 'target'); + + const result = validatePath(traversal, root); + assert.strictEqual( + result.safe, + false, + `traversal ${JSON.stringify(traversal)} against root ${root} must be rejected, got: ${JSON.stringify(result)}`, + ); + }, + )); + }); + + test('PR2: a path that resolves INSIDE the root (no traversal beyond it) is ALWAYS accepted', () => { + fc.assert(fc.property( + fc.array(segmentArb, { minLength: 1, maxLength: 5 }), + (segments) => { + const root = path.resolve('/gsd-root-anchor-in'); + const relPath = path.join(...segments); + + const result = validatePath(relPath, root); + assert.strictEqual( + result.safe, + true, + `in-root path ${JSON.stringify(relPath)} against root ${root} must be accepted, got: ${JSON.stringify(result)}`, + ); + assert.strictEqual(result.resolved, path.resolve(root, relPath)); + }, + )); + }); +}); + +// ─── #4652: cross-boundary containment — same escaping inputs, all four +// boundaries, same rejection shape ──────────────────────────────────────────── +// +// One shared list of escaping inputs is driven through all four containment +// boundaries named in #4652 (todo complete, check predicate --phase-dir, +// check decision-coverage-plan's resolvePath, check gap-analysis.plan-post). +// Each boundary is asserted to reject with the SAME error shape: +// `{ ok: false, reason: 'usage', message }` (ERROR_REASON.USAGE) under +// `--json-errors`. None of these boundaries validate today, so every row is +// expected to FAIL until the fix lands (RED). + +describe('cross-boundary containment — shared escaping inputs, same rejection shape (#4652)', () => { + const ESCAPING_INPUTS = ['../../escaped', '../sibling', 'a/../../b']; + + function setupProject() { + const tmpDir = createTempProject(); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '05-x'); + fs.mkdirSync(phaseDir, { recursive: true }); + const pendingDir = path.join(tmpDir, '.planning', 'todos', 'pending'); + fs.mkdirSync(pendingDir, { recursive: true }); + return { tmpDir, phaseDir, pendingDir }; + } + + function assertUsageRejection(result, label) { + assert.strictEqual( + result.success, + false, + `${label} must be rejected (currently: ${result.success ? `SUCCEEDED with output ${result.output}` : 'failed for an unrelated reason'})`, + ); + let parsed = null; + try { parsed = JSON.parse(result.error); } catch (_) { /* not JSON — also a failure to fix */ } + assert.ok(parsed, `${label}: stderr must be JSON under --json-errors, got: ${result.error}`); + assert.strictEqual(parsed.ok, false, `${label}: parsed.ok must be false`); + assert.strictEqual(parsed.reason, 'usage', `${label}: reason must be ERROR_REASON.USAGE ("usage"), got: ${parsed.reason}`); + } + + for (const escaping of ESCAPING_INPUTS) { + test(`[RED #4652] "${escaping}" is rejected identically (reason: usage) at all four boundaries`, () => { + const { tmpDir } = setupProject(); + try { + // Boundary 1: todo complete + const todoResult = runGsdTools(['--json-errors', 'todo', 'complete', escaping], tmpDir); + assertUsageRejection(todoResult, `todo complete "${escaping}"`); + + // Boundary 2: check predicate --phase-dir + const predicate = JSON.stringify({ + kind: 'command-exit-zero', + command: 'true', + }); + const predicateResult = runGsdTools( + ['--json-errors', 'check', 'predicate', '--predicate', predicate, '--phase-dir', escaping, '--raw'], + tmpDir, + ); + assertUsageRejection(predicateResult, `check predicate --phase-dir "${escaping}"`); + + // Boundary 3: check decision-coverage-plan + const contextPath = path.join(tmpDir, 'CONTEXT.md'); + fs.writeFileSync(contextPath, '# Context\n\n\n## Implementation Decisions\n\n- **D-01:** x\n\n'); + const decisionResult = runGsdTools( + ['--json-errors', 'query', 'check.decision-coverage-plan', escaping, contextPath], + tmpDir, + ); + assertUsageRejection(decisionResult, `check decision-coverage-plan "${escaping}"`); + + // Boundary 4: check gap-analysis.plan-post + const gapResult = runGsdTools( + ['--json-errors', 'check', 'gap-analysis.plan-post', escaping, '--raw'], + tmpDir, + ); + assertUsageRejection(gapResult, `check gap-analysis.plan-post "${escaping}"`); + } finally { + cleanup(tmpDir); + } + }); + } +});