Merge pull request #4666 from open-gsd/fix/4652-containment-at-boundaries

2026-09-12 11:44:24 -04:00
parent 7bcfbe4542 e1f169a378
commit a0270a7945
12 changed files with 818 additions and 14 deletions
--- a/.changeset/sharp-tigers-sing.md
+++ b/.changeset/sharp-tigers-sing.md
@@ -0,0 +1,5 @@
+---
+type: Security
+pr: 4666
+---
+**Path containment at every boundary that takes a directory or filename from the command line** — `todo complete` followed a traversal name outside the todos root and moved the file it found there, `check predicate --phase-dir` let a blocking gate return a passing verdict on evidence from a directory the caller chose, and the shared `resolvePath` helper — used by `check decision-coverage-plan` and `check gap-analysis.plan-post` — accepted a phase directory outside the project. All boundaries now validate against their managed root and reject with a usage error before touching the filesystem. (#4327, #4354)
--- a/CONTEXT.md
+++ b/CONTEXT.md
@@ -357,7 +357,7 @@ A bundle delivering one optional GSD feature, toggled as a unit at install or af
 Generated description of what the five-step loop (Discuss → Plan → Execute → Verify → Ship) exposes as extension points: per-step loop points, agent roles, and core artifacts. Sourced from structured `<!-- gsd:loop-host ... -->` HTML-comment markers embedded near the top of each of the five step workflow files (`discuss-phase.md`, `plan-phase.md`, `execute-phase.md`, `verify-work.md`, `ship.md`). Generated by `scripts/gen-loop-host-contract.cjs` → `gsd-core/bin/lib/loop-host-contract.cjs` (ADR-894 §3 phase 3a-impl-2). Covers exactly the 12 canonical points (discuss:pre/post, plan:pre/post, execute:pre/wave:pre/wave:post/post, verify:pre/post, ship:pre/post). The generator enforces a drift guard: every declared non-orchestrator agent role must correspond to an actual agent reference in the workflow file. Consumed by `gen-capability-registry.cjs` (replaces the former inline `LOOP_HOST_CONTRACT` constant). Run `node scripts/gen-loop-host-contract.cjs --write` after editing a workflow step marker.

 ### Gate Predicate Evaluator Module
-Pure, deps-injected evaluator for capability gate `check.predicate` blocks (#2008, ADR-2008). Prior to #2008 the registry validator accepted `check.predicate` (one of exactly-one-of `query`/`predicate`/`agentVerdict`) and the loop-resolver rendered it, but nothing EVALUATED a declared predicate — only `check.query` was enforced (dispatched via `gsd_run check <query>`), and built-in gates like `security` worked only via hard-coded `capId` prose branches in `ship.md`/`execute-phase.md`/`verify-work.md`. This module is the generic evaluation path: `evaluatePredicate(predicate, context, deps) → { block, message, details? }` dispatches by `predicate.kind` through a `KIND_TABLE`. Built-in kinds: `command-exit-zero` runs a declared command in a bounded `sh -c` subprocess (production binding: `shell-command-projection.execTool`) at the project root, inheriting env; exit 0 ⇒ pass, non-zero ⇒ block, timeout (SIGTERM) ⇒ block; `artifact-frontmatter-equals` resolves a phase or project-level Markdown artifact through injected dependencies and compares a frontmatter field by scalar string value. Command predicates interpolate `${PHASE_NUMBER}`/`${PHASE_DIR}`/`${PHASE_REQ_IDS}` from gate context. A THROWN error (malformed predicate, non-positive/non-finite timeout, command >4096 chars, missing dependencies, unknown kind) maps at the CLI seam to a non-zero check-command exit, which the workflow's two-step gate contract treats as a step-1 command failure routed per `onError` — so an evaluator bug is never conflated with a legitimate block decision. Leaf pure module (no fs/child_process/config — subprocess and artifact seams injected). CLI entry: `gsd_run check predicate --predicate '<json>' [--phase-dir …] [--phase-number …] [--phase-req-ids …] --raw`, wired into `check-command-router.cts:cmdCheckPredicate`; the four generic workflow gate-dispatch sites (`execute:wave:post`, `execute:post`, `plan:post`, and `ship:pre` — the last wired by #3559, which had resolved gates generically but enforced only two hardcoded `capId`s) branch on `check` shape (`query` vs `predicate`). Source of truth: `src/gate-predicate-evaluator.cts`. Docs: `docs/reference/gate-predicates.md`, `docs/how-to/command-exit-zero-gate.md`.
+Pure, deps-injected evaluator for capability gate `check.predicate` blocks (#2008, ADR-2008). Prior to #2008 the registry validator accepted `check.predicate` (one of exactly-one-of `query`/`predicate`/`agentVerdict`) and the loop-resolver rendered it, but nothing EVALUATED a declared predicate — only `check.query` was enforced (dispatched via `gsd_run check <query>`), and built-in gates like `security` worked only via hard-coded `capId` prose branches in `ship.md`/`execute-phase.md`/`verify-work.md`. This module is the generic evaluation path: `evaluatePredicate(predicate, context, deps) → { block, message, details? }` dispatches by `predicate.kind` through a `KIND_TABLE`. Built-in kinds: `command-exit-zero` runs a declared command in a bounded `sh -c` subprocess (production binding: `shell-command-projection.execTool`) at the project root, inheriting env; exit 0 ⇒ pass, non-zero ⇒ block, timeout (SIGTERM) ⇒ block; `artifact-frontmatter-equals` resolves a phase or project-level Markdown artifact through injected dependencies and compares a frontmatter field by scalar string value. Command predicates interpolate `${PHASE_NUMBER}`/`${PHASE_DIR}`/`${PHASE_REQ_IDS}` from gate context. A THROWN error (malformed predicate, non-positive/non-finite timeout, command >4096 chars, missing dependencies, unknown kind) maps at the CLI seam to a non-zero check-command exit, which the workflow's two-step gate contract treats as a step-1 command failure routed per `onError` — so an evaluator bug is never conflated with a legitimate block decision. Leaf pure module (no fs/child_process/config — subprocess and artifact seams injected). CLI entry: `gsd_run check predicate --predicate '<json>' [--phase-dir …] [--phase-number …] [--phase-req-ids …] --raw`, wired into `check-command-router.cts:cmdCheckPredicate`. `--phase-dir` is CONFINED to the project root at that router boundary (#4354, epic #4636 Phase 2): an unconfined value let a BLOCKING gate return `block: false` on an artifact in a directory the caller chose, and the same value interpolates into `${PHASE_DIR}` for `command-exit-zero`, so confining at the boundary covers BOTH kinds while keeping this module's fs-free leaf contract intact — a fix inside the evaluator would have required giving a pure module a filesystem dependency and would still have missed the interpolation path. The four generic workflow gate-dispatch sites (`execute:wave:post`, `execute:post`, `plan:post`, and `ship:pre` — the last wired by #3559, which had resolved gates generically but enforced only two hardcoded `capId`s) branch on `check` shape (`query` vs `predicate`). Source of truth: `src/gate-predicate-evaluator.cts`. Docs: `docs/reference/gate-predicates.md`, `docs/how-to/command-exit-zero-gate.md`.

 ### Capability Registry
 Generated central manifest projecting all co-located Capability declarations into one validated artifact for runtime resolution and for the install, surface, config, and loop-extension adapters. Mirrors the research-profiles / package-identity generation pattern (co-located source → generated central file). Generated by `scripts/gen-capability-registry.cjs` → `gsd-core/bin/lib/capability-registry.cjs` (ADR-894 §5 phase 3a-impl). Role-partitioned indexes: `bySkill`, `byAgent`, `byLoopPoint` (hook ordering materialized), `configKeys` (ownership map: key→capId), `configSchema` (full per-key schema: key→{ owner, type, default, description }), `runtimes`, `requiresClosure(id)`. Each feature capability's entry in `capabilities` now includes the optional `activationKey` field (the dotted config key that gates the whole capability, e.g. `"graphify.enabled"`; absent means no config gate). ADR-857 phase 3b adds `configSchema` with validated type/default/description per key, sourced from each capability's `.config` slice. ADR-857 phase 4a adds two derived views: `capabilityClusters` (`{ <capId>: [<skill stems>] }` — each cap's skills array, sorted, derived from the capability's `skills` declaration; consistency-gated against the hand-authored `CLUSTERS`) and `profileMembership` (`{ <capId>: { tier, profiles: [...] } }` — the tier-derived index: suffix of `PROFILE_RANK` starting at the capability's tier). Both views cover the same capability set: only capabilities that own skills (non-empty `skills` array). The generator enforces a HARD gate (throws) if a capId matching a `CLUSTERS` key has a mismatched skill set, and emits SOFT `⚠ pending-reconciliation` warnings to stderr (never to the file) for skills not yet in the hand-authored profile at the capability's tier. `install` and `surface` are UNTOUCHED (still read hand-authored constants; derived views are emitted and tested but unconsumed until cutover). Validated against the Loop Host Contract (12 points; generated by `gen-loop-host-contract.cjs` from workflow markers, phase 3a-impl-2). Run `node scripts/gen-capability-registry.cjs --write` after editing any `capabilities/<id>/capability.json`.
--- a/docs/CLI-TOOLS.md
+++ b/docs/CLI-TOOLS.md
@@ -1177,6 +1177,20 @@ from `todos/pending/` to `todos/completed/` and upserts `completed:` and
 `status: completed` inside the file's frontmatter block. Unknown flags are
 rejected loudly.

+`<filename>` is a **basename inside the todos root**, not a path. A basename
+guard runs first, before `<filename>` is joined onto any directory: a value
+containing an embedded separator (either `/` or `\`, e.g. `sub/name.md` or
+`sub\name.md`), a value whose own basename differs from itself (e.g.
+`a/../../b.md`, `../sibling.md`), a bare `.` or `..`, an absolute path (e.g.
+`/etc/passwd`), or a NUL byte is rejected as a usage error **before** any file
+is read or moved (#4327, #4652). A traversal that only escapes the `pending`/
+`completed` subdirectory without leaving the todos root (`../sibling.md`) is
+caught by this same guard, not by containment. Containment against the todos
+root still runs afterward as defense-in-depth for the resolved source and
+target paths, so neither half of the move can land outside the root. The
+check covers both halves of the move, and `--dry-run` is rejected on the same
+terms rather than previewing a resolved outside path.
+
 ```bash
 # UAT audit — scan all phases for unresolved items
 node gsd-tools.cjs audit-uat
--- a/docs/reference/gate-predicates.md
+++ b/docs/reference/gate-predicates.md
@@ -81,6 +81,18 @@ the gate context; all others are left untouched for `sh` to interpret:

 An undefined placeholder interpolates to the empty string.

+**`--phase-dir` is confined to the project.** The value is validated to resolve
+inside the project root before any predicate is evaluated; one that escapes is
+rejected as a usage error rather than evaluated. This applies to both kinds —
+`artifact-frontmatter-equals` resolves its artifact under that directory, and
+`command-exit-zero` interpolates it into `${PHASE_DIR}` — so an unconfined value
+would let a **blocking** gate return `block: false` on evidence from a directory
+the caller chose (#4354). An absolute path inside the project is still accepted;
+absolute is not a synonym for escaping. `${PHASE_DIR}` always interpolates the
+**resolved absolute path**, even when `--phase-dir` was given as a relative
+value — a command relying on `${PHASE_DIR}` staying relative must not assume
+that.
+
 **Sandbox.** cwd = project root; env = inherited from the GSD process; killed
 (SIGTERM) on timeout. The command runs as the user, on the user's machine —
 there is no sandbox boundary vs. the user's own shell. See ADR-2008 "Trust
--- a/scripts/lib/macos-conformance-tier.generated.cjs
+++ b/scripts/lib/macos-conformance-tier.generated.cjs
@@ -30,6 +30,7 @@ module.exports = {
  "tests/capability-trust.test.cjs",
  "tests/changeset-parse.test.cjs",
  "tests/check-contract-drift.test.cjs",
+  "tests/check-predicate.test.cjs",
  "tests/check-ui-safety-gate.test.cjs",
  "tests/check-update-config-dir.test.cjs",
  "tests/chunked-planning-parallel.test.cjs",
--- a/scripts/lib/platform-conformance-tier.generated.cjs
+++ b/scripts/lib/platform-conformance-tier.generated.cjs
@@ -30,6 +30,7 @@ module.exports = {
  "tests/check-env.test.cjs",
  "tests/check-gap-analysis-plan-post-e2e.test.cjs",
  "tests/check-glossary-refs.test.cjs",
+  "tests/check-predicate.test.cjs",
  "tests/check-tdd-review-checkpoint-e2e.test.cjs",
  "tests/check-ui-safety-gate.test.cjs",
  "tests/check-update-config-dir.test.cjs",
--- a/src/check-command-router.cts
+++ b/src/check-command-router.cts
@@ -90,7 +90,12 @@ function readIfExists(filePath: string): string {
 }

 function resolvePath(inputPath: string, projectDir: string): string {
-  return path.isAbsolute(inputPath) ? inputPath : path.join(projectDir, inputPath);
+  const candidate = path.isAbsolute(inputPath) ? inputPath : path.join(projectDir, inputPath);
+  const check = validatePath(candidate, projectDir, { allowAbsolute: true });
+  if (!check.safe) {
+    error(`path escapes its allowed directory: ${inputPath}`, ERROR_REASON.USAGE);
+  }
+  return check.resolved;
 }

 interface WorkflowConfig {
@@ -1194,8 +1199,9 @@ function cmdGapAnalysisPlanPost(projectDir: string, args: string[], raw: boolean
    error('gap-analysis.plan-post requires a phase-dir argument: check gap-analysis.plan-post <phase-dir> [phase-req-ids]', ERROR_REASON.SDK_MISSING_ARG);
    return;
  }
+  const resolvedPhaseDir = resolvePath(phaseDir, projectDir);
  const phaseReqIds = args[3] ?? undefined;
-  const result = runGapAnalysis(projectDir, phaseDir, { phaseReqIds });
+  const result = runGapAnalysis(projectDir, resolvedPhaseDir, { phaseReqIds });
  // Uniform gate contract: block = false (gap-analysis is always advisory, never blocks).
  // `message` carries the human-readable gap analysis report so the dispatch's
  // advisory branch can surface it. --raw emits JSON (rawValue=undefined), not
@@ -1362,10 +1368,15 @@ function cmdCheckPredicate(projectDir: string, args: string[], raw: boolean): vo
    error('predicate --predicate value must be valid JSON', ERROR_REASON.USAGE);
    return;
  }
+  const rawPhaseDir = flags['phase-dir'];
+  let resolvedPhaseDir: string | undefined = rawPhaseDir;
+  if (typeof rawPhaseDir === 'string' && rawPhaseDir !== '') {
+    resolvedPhaseDir = resolvePath(rawPhaseDir, projectDir);
+  }
  const ctx = {
    cwd: projectDir,
    phaseNumber: flags['phase-number'],
-    phaseDir: flags['phase-dir'],
+    phaseDir: resolvedPhaseDir,
    phaseReqIds: flags['phase-req-ids'],
  };
  let result;
--- a/src/commands.cts
+++ b/src/commands.cts
@@ -11,7 +11,7 @@ import path from 'node:path';
 import { normalizeEol } from './text-lines.cjs';
 import { execGit, platformWriteSync, platformReadSync, platformEnsureDir, isSpawnTimeout, retryRenameSync } from './shell-command-projection.cjs';
 import { escapeRegex } from './pattern.cjs';
-import { requireSafePath, sanitizeForDisplay } from './security.cjs';
+import { requireSafePath, sanitizeForDisplay, validatePath } from './security.cjs';
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 import ioMod = require('./io.cjs');
 const { output, error, ERROR_REASON } = ioMod;
@@ -3490,13 +3490,61 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod
  const todosRoot = todosDir(cwd);
  const pendingDir = path.join(todosRoot, 'pending');
  const completedDir = path.join(todosRoot, 'completed');
-  const sourcePath = path.join(pendingDir, filename as string);

-  if (!fs.existsSync(sourcePath)) {
+  // #4652: containment against todosRoot only rejects paths that leave the
+  // root — it cannot express "a todo name is a basename, not a path" (see
+  // #4327). `../sibling.md`, `a/../../b.md`, and `sub/name.md` all resolve
+  // to a location inside todosRoot (or inside pending/) and would pass
+  // containment, yet none of them is a bare filename. Reject on basename
+  // shape FIRST, before any path is even joined — same predicate shape as
+  // findPhaseArtifact in check-command-router.cts. Checking both `/` and
+  // `\` explicitly (not just path.basename) matters on POSIX, where a
+  // literal backslash is just an ordinary filename character to
+  // path.basename but not to path.win32.basename or to the user's intent.
+  const rawFilename = filename as string;
+  if (
+    rawFilename === '.' ||
+    rawFilename === '..' ||
+    rawFilename.includes('\0') ||
+    rawFilename.includes('/') ||
+    rawFilename.includes('\\') ||
+    path.basename(rawFilename) !== rawFilename ||
+    path.win32.basename(rawFilename) !== rawFilename
+  ) {
+    error(`todo name must be a plain filename inside the pending directory, not a path: ${rawFilename}`, ERROR_REASON.USAGE);
+  }
+
+  const sourcePath = path.join(pendingDir, filename as string);
+  const targetPath = path.join(completedDir, filename as string);
+
+  const sourceCheck = validatePath(sourcePath, todosRoot, { allowAbsolute: true });
+  if (!sourceCheck.safe) {
+    error(`todo file escapes its allowed directory: ${filename as string}`, ERROR_REASON.USAGE);
+  }
+  const targetCheck = validatePath(targetPath, todosRoot, { allowAbsolute: true });
+  if (!targetCheck.safe) {
+    error(`todo file escapes its allowed directory: ${filename as string}`, ERROR_REASON.USAGE);
+  }
+
+  const resolvedSource = sourceCheck.resolved;
+  const resolvedTarget = targetCheck.resolved;
+
+  if (!fs.existsSync(resolvedSource)) {
    error(`Todo not found: ${filename as string}`);
  }

-  const content = fs.readFileSync(sourcePath, 'utf-8');
+  // #4652: a name that IS a bare basename can still resolve to something that
+  // is not a regular file — a directory, symlink-to-directory, FIFO or socket
+  // sitting in pending/ under an ordinary-looking name. `.` and `..` no longer
+  // reach here (the basename guard above rejects them first), so this is not
+  // about traversal; it stops fs.readFileSync from throwing an uncaught EISDIR
+  // with an absolute-path stack trace where every sibling case gives a clean
+  // USAGE rejection.
+  if (!fs.statSync(resolvedSource).isFile()) {
+    error(`todo name is not a file: ${filename as string}`, ERROR_REASON.USAGE);
+  }
+
+  const content = fs.readFileSync(resolvedSource, 'utf-8');
  const today = realClock.localToday();

  // #4096: --dry-run mirrors `milestone complete --dry-run` (#2118) — every
@@ -3509,8 +3557,8 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod
      file: filename,
      date: today,
      would_move: {
-        source: path.relative(cwd, sourcePath).split(path.sep).join('/'),
-        target: path.relative(cwd, path.join(completedDir, filename as string)).split(path.sep).join('/'),
+        source: path.relative(cwd, resolvedSource).split(path.sep).join('/'),
+        target: path.relative(cwd, resolvedTarget).split(path.sep).join('/'),
      },
      would_set: { completed: today, status: 'completed' },
    }, raw);
@@ -3523,8 +3571,8 @@ function cmdTodoComplete(cwd: string, filename: string | undefined, options: Tod

  const completedContent = upsertTodoCompletionFields(content, today);

-  platformWriteSync(path.join(completedDir, filename as string), completedContent);
-  fs.unlinkSync(sourcePath);
+  platformWriteSync(resolvedTarget, completedContent);
+  fs.unlinkSync(resolvedSource);

  output({ completed: true, file: filename, date: today }, raw, 'completed');
 }
--- 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 <phase-dir>` ───────────────────────────────
+//
+// Boundary 3: `check decision-coverage-plan <phase-dir>` 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 <phase-dir>` 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<decisions>\n## Implementation Decisions\n\n- **D-01:** Use pattern X\n</decisions>\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<decisions>\n## Implementation Decisions\n\n- **D-01:** Use pattern X\n</decisions>\n',
+    );
+    fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\n\n## tasks\n\n- D-01: Use pattern X\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<decisions>\n## Implementation Decisions\n\n- **D-01:** Use pattern X\n</decisions>\n',
+    );
+    fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), '# Plan\n\n## tasks\n\n- D-01: Use pattern X\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');
+  });
+});
--- 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,219 @@ 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 #4652] a relative --phase-dir must resolve against --cwd, not the real process cwd, and must not leak the outside file', () => {
+    // The real process cwd (outsideDir) contains a foreign SECURITY.md; the
+    // CLI is told --cwd projDir with a relative --phase-dir '.'. Before #4652,
+    // cmdCheckPredicate validated the joined (projDir + '.') path but passed
+    // the RAW, un-joined '.' into ctx.phaseDir, which findPhaseArtifact then
+    // resolved against the real process cwd (outsideDir) — leaking foreign
+    // frontmatter. The fix must reject this, and the leaked value must never
+    // appear in the output.
+    fs.writeFileSync(
+      path.join(outsideDir, 'SECURITY.md'),
+      '---\nstatus: LEAKED_VALUE\n---\n# Security\n',
+    );
+
+    const predicate = JSON.stringify({
+      kind: 'artifact-frontmatter-equals',
+      artifact: 'SECURITY.md',
+      field: 'status',
+      equals: 'NOPE',
+    });
+
+    const result = runGsdTools(
+      ['--json-errors', 'check', 'predicate', '--cwd', projDir, '--predicate', predicate, '--phase-dir', '.', '--raw'],
+      outsideDir,
+    );
+
+    const combinedOutput = `${result.output || ''}${result.error || ''}`;
+    assert.ok(
+      !combinedOutput.includes('LEAKED_VALUE'),
+      `the outside file's frontmatter value must never leak into the output (got: ${combinedOutput})`,
+    );
+    assert.strictEqual(
+      result.success && JSON.parse(result.output).block === false,
+      false,
+      `a relative --phase-dir must not resolve against the real process cwd and must not pass ` +
+        `(currently: ${combinedOutput})`,
+    );
+  });
+
+  test('[#4652] a --phase-dir that is a symlink inside the project resolving outside the project is rejected', (t) => {
+    fs.writeFileSync(
+      path.join(outsideDir, 'SECURITY.md'),
+      '---\nstatus: passed\n---\n# Security\n',
+    );
+    const linkPath = path.join(projDir, '.planning', 'phases', 'linked-out');
+    try {
+      fs.symlinkSync(outsideDir, linkPath, 'dir');
+    } catch (e) {
+      if (e.code === 'EPERM') {
+        t.skip('symlink creation is not permitted on this platform (EPERM)');
+        return;
+      }
+      throw e;
+    }
+
+    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', linkPath, '--raw'],
+      projDir,
+    );
+
+    assert.strictEqual(
+      result.success,
+      false,
+      `a --phase-dir symlink resolving outside the project must be rejected ` +
+        `(currently: ${result.success ? `SUCCEEDED with output ${result.output}` : 'failed for an unrelated reason'})`,
+    );
+  });
+
+  test('[#4652] a relative --phase-dir interpolates ${PHASE_DIR} as the resolved ABSOLUTE path, not the relative value', () => {
+    const phaseDir = path.join(projDir, '.planning', 'phases', '05-x');
+    fs.writeFileSync(path.join(phaseDir, 'marker.txt'), 'marker\n');
+
+    const predicate = JSON.stringify({
+      kind: 'command-exit-zero',
+      command: 'echo "${PHASE_DIR}" > "${PHASE_DIR}/interpolated.txt"',
+    });
+
+    const result = runGsdTools(
+      ['check', 'predicate', '--predicate', predicate, '--phase-dir', '.planning/phases/05-x', '--raw'],
+      projDir,
+    );
+
+    assert.ok(result.success, `Command failed: ${result.error}`);
+    const interpolated = fs.readFileSync(path.join(phaseDir, 'interpolated.txt'), 'utf-8').trim();
+    assert.strictEqual(
+      interpolated,
+      fs.realpathSync(phaseDir),
+      `${'${PHASE_DIR}'} must interpolate the resolved absolute path, not the relative --phase-dir value`,
+    );
+  });
+
+  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');
+  });
+});
--- 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,226 @@ 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'})`,
+      );
+    });
+  }
+
+  // '.' and '..' resolve to the pending dir itself (which IS inside the
+  // root, so containment passes) but are not a todo name — before #4652
+  // this fell through to an uncaught EISDIR with an absolute-path stack
+  // trace instead of a clean rejection.
+  for (const name of ['.', '..']) {
+    test(`[regression #4652] "todo complete ${name}" is rejected cleanly (no uncaught EISDIR / stack trace)`, () => {
+      const result = runGsdTools(['todo', 'complete', name], tmpDir);
+      assert.strictEqual(result.success, false, `"${name}" must be rejected`);
+      assert.ok(
+        !/at\s+\S+\s+\(.*\.c?ts?:\d+/.test(result.error || ''),
+        `rejection must not leak a stack trace (got: ${result.error})`,
+      );
+      assert.ok(
+        !(result.error || '').includes('EISDIR'),
+        `rejection must be a clean USAGE error, not an uncaught EISDIR (got: ${result.error})`,
+      );
+    });
+  }
+
+  test('[#4327] an absolute filename is rejected as a non-basename before any join — the outside file is untouched', () => {
+    // A basename guard added since #4327 rejects any filename containing `/`
+    // or `\` BEFORE it is ever joined against pendingDir — so an absolute
+    // path never reaches path.join, containment, or the filesystem at all.
+    // It is a USAGE rejection, not a containment/escape check and not a
+    // plain "not found". This test also pins the thing that actually
+    // matters: the real outside file is never read, moved, or deleted.
+    const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-todo-outside-'));
+    try {
+      const outsideFile = path.join(outsideDir, 'evil.md');
+      const sentinel = '---\nstatus: pending\n---\nSENTINEL\n';
+      fs.writeFileSync(outsideFile, sentinel);
+      const result = runGsdTools(['--json-errors', 'todo', 'complete', outsideFile], tmpDir);
+
+      assert.strictEqual(result.success, false, 'the command must fail (a filename containing a separator is rejected)');
+      const parsed = JSON.parse(result.error);
+      assert.strictEqual(parsed.ok, false);
+      assert.strictEqual(parsed.reason, 'usage');
+      assert.ok(fs.existsSync(outsideFile), 'the real outside file must still exist');
+      assert.strictEqual(
+        fs.readFileSync(outsideFile, 'utf-8'),
+        sentinel,
+        'the real outside file must never be read/touched',
+      );
+      const completedDir = path.join(tmpDir, '.planning', 'todos', 'completed');
+      if (fs.existsSync(completedDir)) {
+        assert.ok(
+          !fs.readdirSync(completedDir).includes('evil.md'),
+          'the outside file must never land inside completed/',
+        );
+      }
+    } 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 <traversal> --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);
+    }
+  });
+
+  test('[#4652] a symlink inside pending/ whose target is a real file outside the todos root is rejected — the outside target is untouched', (t) => {
+    const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-todo-outside-'));
+    const linkPath = path.join(pendingDir, 'linked.md');
+    try {
+      const outsideFile = path.join(outsideDir, 'real-target.md');
+      const sentinel = '---\nstatus: pending\n---\nSENTINEL-SYMLINK\n';
+      fs.writeFileSync(outsideFile, sentinel);
+      try {
+        fs.symlinkSync(outsideFile, linkPath, 'file');
+      } catch (e) {
+        if (e.code === 'EPERM') {
+          t.skip('symlink creation is not permitted on this platform (EPERM)');
+          return;
+        }
+        throw e;
+      }
+
+      const result = runGsdTools(['todo', 'complete', 'linked.md'], tmpDir);
+
+      assert.strictEqual(result.success, false, 'a symlink pointing outside the todos root must be rejected');
+      assert.ok(fs.existsSync(outsideFile), 'the outside symlink target must still exist');
+      assert.strictEqual(
+        fs.readFileSync(outsideFile, 'utf-8'),
+        sentinel,
+        'the outside symlink target content must be byte-for-byte untouched',
+      );
+    } finally {
+      cleanup(outsideDir);
+      try { fs.unlinkSync(linkPath); } catch { /* not created, or already gone */ }
+    }
+  });
+});
+
 // ─────────────────────────────────────────────────────────────────────────────
 // todo match-phase command
 // ─────────────────────────────────────────────────────────────────────────────
--- 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)}`,
+        );
+      },
+    ), { seed: 4652, numRuns: 200 });
+  });
+
+  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));
+      },
+    ), { seed: 4652, numRuns: 200 });
+  });
+});
+
+// ─── #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 <name>
+        const todoResult = runGsdTools(['--json-errors', 'todo', 'complete', escaping], tmpDir);
+        assertUsageRejection(todoResult, `todo complete "${escaping}"`);
+
+        // Boundary 2: check predicate --phase-dir <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 <phase-dir> <context-path>
+        const contextPath = path.join(tmpDir, 'CONTEXT.md');
+        fs.writeFileSync(contextPath, '# Context\n\n<decisions>\n## Implementation Decisions\n\n- **D-01:** x\n</decisions>\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 <phase-dir>
+        const gapResult = runGsdTools(
+          ['--json-errors', 'check', 'gap-analysis.plan-post', escaping, '--raw'],
+          tmpDir,
+        );
+        assertUsageRejection(gapResult, `check gap-analysis.plan-post "${escaping}"`);
+      } finally {
+        cleanup(tmpDir);
+      }
+    });
+  }
+});