From 7a02f985747edd46ddb693f2e89f59fd019ab14a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 17:29:58 -0400 Subject: [PATCH] fix(#3493): confine key_links from:/to: to the project directory (#3506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cmdVerifyKeyLinks` resolved `from:` and `to:` with `path.join(cwd, )` where the value comes verbatim from plan YAML. `path.join` normalizes `../` rather than rejecting it, so a plan travelling with a repository could name any file the process can read, and the command reports whether the link's `pattern` matched it — an arbitrary-file-read oracle reachable from `verify-phase`. Both reads now go through `validatePath` in src/security.cts, the existing realpath-based confinement seam already used at 11 call sites. Not a missing capability — a bypassed one. Two defects in that seam, found by adversarial review and fixed here because they affect all 11 callers: 1. A dangling in-project symlink escaped confinement. A link to an EXISTING outside path was refused (realpath lands outside) while a link to a MISSING outside path took the parent-resolution fallback and was accepted — an existence oracle for arbitrary absolute paths. lstat succeeds on a dangling link and throws ENOENT on a truly absent path; an unresolvable link is now refused. A symlink resolving inside the project is still accepted. 2. A canonicalized base was compared against an uncanonicalized path when a file and its parent were both missing, wrongly refusing legitimate in-project paths on any non-canonical cwd (every macOS temp dir). This was a live regression in this PR: the wave-pending classification (#1202) depends on the not-yet-created case. Resolution now walks up to the nearest existing ancestor. Two adjacent aborts fixed: the from: read sat outside the per-link try, so a non-ENOENT errno killed the whole command; and an empty from: read the cwd directory, throwing EISDIR. Both now fail per-link. The issue was filed as a fourth ADR-0174 consolidation loss. It is not one — validatePath/requireSafePath never went away, only the SDK's name for the concept did. This is an instance of epic #3473's F2 family. The ADR-0174 loss count is three. Closes #3493 Co-Authored-By: Claude Opus 5 --- .changeset/patient-lynx-march.md | 5 + .changeset/serene-eagles-roar.md | 5 + docs/CLI-TOOLS.md | 2 + src/security.cts | 46 ++++++- src/verify.cts | 57 +++++++- tests/security.test.cjs | 151 +++++++++++++++++++++ tests/verify.test.cjs | 217 +++++++++++++++++++++++++++++++ 7 files changed, 475 insertions(+), 8 deletions(-) create mode 100644 .changeset/patient-lynx-march.md create mode 100644 .changeset/serene-eagles-roar.md diff --git a/.changeset/patient-lynx-march.md b/.changeset/patient-lynx-march.md new file mode 100644 index 000000000..71a390204 --- /dev/null +++ b/.changeset/patient-lynx-march.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 3506 +--- +**Path validation no longer accepts a symbolic link whose target is missing** — `validatePath` canonicalizes a path with `realpath`, and for a path that does not exist yet it fell back to canonicalizing the parent directory instead. A symlink inside the project pointing at a **non-existent** location outside it took that fallback and was accepted, while a symlink pointing at an **existing** outside location was correctly refused — a difference an attacker could use to test whether arbitrary absolute paths exist. Such a link is now refused outright. The same fallback also compared an uncanonicalized path against a canonicalized base when several leading directories were missing, wrongly refusing legitimate not-yet-created paths on any non-canonical working directory (every macOS temp directory, for one); it now canonicalizes from the nearest existing ancestor. (#3493) diff --git a/.changeset/serene-eagles-roar.md b/.changeset/serene-eagles-roar.md new file mode 100644 index 000000000..bbde4df7c --- /dev/null +++ b/.changeset/serene-eagles-roar.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 3506 +--- +**`verify key-links` no longer reads files outside the project** — `from:` and `to:` were taken verbatim from plan frontmatter and resolved with `path.join(cwd, …)`, which normalizes `../` rather than rejecting it, so a plan carried in an untrusted repository could name any file the process could read and learn from the reported result whether a supplied pattern matched its contents. Both paths now resolve through the project's realpath-based confinement seam; a path that escapes is refused without being read, reported as `path_rejected`, and never counts as verified. (#3493) diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 2dd5d4b22..f11c6f217 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -547,6 +547,8 @@ node gsd-tools.cjs verify artifacts node gsd-tools.cjs verify key-links ``` +`verify key-links` confines each link's `from:`/`to:` to the project directory (#3493): a path that resolves outside the project (via `../` traversal, an absolute path, or a symlink) is never read. That link's `links[]` entry reports `path_rejected: "from"` or `path_rejected: "to"` (whichever field was rejected) alongside `verified: false`, without echoing the underlying path-confinement error (which would embed an absolute host path). A rejected link fails independently — it does not abort evaluation of the other links in the same plan, and does not set `path_rejected` on links whose paths resolve inside the project. + --- ## Validation Commands diff --git a/src/security.cts b/src/security.cts index 261bdbe75..9f609e39c 100644 --- a/src/security.cts +++ b/src/security.cts @@ -56,12 +56,50 @@ export function validatePath(filePath: unknown, baseDir: unknown, opts: { allowA try { resolvedPath = fs.realpathSync(resolvedPath); } catch { - const parentDir = path.dirname(resolvedPath); + // realpathSync failed — either resolvedPath doesn't exist at all, or it's + // a dangling symlink (the link itself exists but its target doesn't). + // lstat (unlike stat/realpath) stats the link itself and does NOT follow + // it, so it succeeds for a dangling symlink and throws ENOENT for a + // genuinely absent path. That's the discriminator: without it, a dangling + // symlink to a non-existent OUTSIDE path would fall through to the + // parent-resolution fallback below and be re-accepted as an in-project + // path, while a symlink to an EXISTING outside path is correctly + // rejected via the realpathSync success branch above — a state + // difference an attacker can use as an existence oracle for arbitrary + // absolute paths. try { - const realParent = fs.realpathSync(parentDir); - resolvedPath = path.join(realParent, path.basename(resolvedPath)); + if (fs.lstatSync(resolvedPath).isSymbolicLink()) { + return { safe: false, resolved: '', error: 'Path is an unresolvable symbolic link' }; + } } catch { - // Parent doesn't exist either — keep the resolved path as-is + // lstat also threw — resolvedPath (and its would-be link) genuinely + // doesn't exist. Fall through to ancestor resolution below. + } + // Walk up to the nearest ancestor that exists and realpath THAT, then + // re-append the remaining (not-yet-created) segments. This canonicalizes + // resolvedPath the same way resolvedBase was canonicalized above, + // regardless of how many leading directories are missing — a single + // parent-only check would leave resolvedPath un-canonicalized whenever + // the parent is also missing, which breaks the startsWith comparison + // below on any non-canonical cwd (e.g. macOS /var/... vs + // /private/var/...). + let ancestor = path.dirname(resolvedPath); + const remainder: string[] = [path.basename(resolvedPath)]; + for (;;) { + try { + const realAncestor = fs.realpathSync(ancestor); + resolvedPath = path.join(realAncestor, ...remainder); + break; + } catch { + const parent = path.dirname(ancestor); + if (parent === ancestor) { + // Reached filesystem root without finding an existing ancestor — + // keep resolvedPath as-is. + break; + } + remainder.unshift(path.basename(ancestor)); + ancestor = parent; + } } } const normalizedBase = resolvedBase + path.sep; diff --git a/src/verify.cts b/src/verify.cts index d571ded9b..02c70c082 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -27,6 +27,7 @@ const { findOrphanSummaries, findUnsummarizedPlans } = coreUtilsMod; import planningScopeMod = require('./planning-scope.cjs'); const { SCOPE } = planningScopeMod; import { execGit, platformReadSync as safeReadFile } from './shell-command-projection.cjs'; +import { validatePath } from './security.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; import { detectSchemaFiles, checkSchemaDrift } from './schema-detect.cjs'; import { extractTaggedBlocks } from './markdown-sectionizer.cjs'; @@ -1222,8 +1223,39 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi detail: '', }; - const fromPath = (link['from'] as string) || ''; - const sourceContent = safeReadFile(path.join(cwd, fromPath)); + const fromRaw = link['from']; + const fromPath = typeof fromRaw === 'string' ? fromRaw : ''; + let sourceContent: string | null = null; + if (fromPath !== '') { + // An empty/missing `from:` is a malformed plan, not a path-confinement + // violation — validatePath's traversal check (and path_rejected) only + // applies to a non-empty path that actually resolves outside the + // project. Leave sourceContent as null so the existing not-found / + // pending classification below runs unchanged. Note this guard is + // narrower than it may look: `from: "."` is a non-empty string, so it + // still reaches validatePath and safeReadFile below, and DOES read the + // cwd directory (yielding "Source read failed: EISDIR") — this branch + // only short-circuits the true empty-string case. + const fromCheck = validatePath(fromPath, cwd); + if (!fromCheck.safe) { + // Do not echo result.error — it embeds absolute host paths. + check['path_rejected'] = 'from'; + check['detail'] = 'Source path rejected — resolves outside the project directory'; + results.push(check); + continue; + } + try { + sourceContent = safeReadFile(fromCheck.resolved); + } catch (err) { + // Report the errno only — never the message or path (untrusted `from:` + // can trigger EISDIR/EACCES, which platformReadSync re-throws for any + // non-ENOENT errno). A single bad link must not abort the whole command. + const code = (err as NodeJS.ErrnoException)?.code ?? 'unknown'; + check['detail'] = `Source read failed: ${code}`; + results.push(check); + continue; + } + } if (!sourceContent) { // Check if the missing file is promised by a plan at the same or later wave. const promised = getPromisedFiles(); @@ -1269,11 +1301,28 @@ function cmdVerifyKeyLinks(cwd: string, planFilePath: string, raw: boolean): voi check['verified'] = true; check['detail'] = 'Pattern found in source'; } else { - const targetContent = safeReadFile(path.join(cwd, (link['to'] as string) || '')); + const toRaw = link['to']; + const toPath = typeof toRaw === 'string' ? toRaw : ''; + let targetContent: string | null = null; + if (toPath !== '') { + // An empty/missing `to:` is a malformed plan, not a + // path-confinement violation — only a non-empty path that + // actually resolves outside the project is path_rejected. + const toCheck = validatePath(toPath, cwd); + if (!toCheck.safe) { + // Do not read a rejected `to:` — treat as no target content + // and do not echo result.error, which embeds absolute host + // paths. + check['path_rejected'] = 'to'; + check['detail'] = `Pattern "${link['pattern'] as string}" not found in source; target path rejected — resolves outside the project directory`; + } else { + targetContent = safeReadFile(toCheck.resolved); + } + } if (targetContent && pat.test(targetContent)) { check['verified'] = true; check['detail'] = 'Pattern found in target'; - } else { + } else if (!check['path_rejected']) { check['detail'] = `Pattern "${link['pattern'] as string}" not found in source or target`; } } diff --git a/tests/security.test.cjs b/tests/security.test.cjs index 0dcb2178b..d7386e963 100644 --- a/tests/security.test.cjs +++ b/tests/security.test.cjs @@ -9,6 +9,7 @@ const assert = require('node:assert/strict'); const path = require('path'); const os = require('os'); const fs = require('fs'); +const { cleanup } = require('./helpers.cjs'); const { validatePath, @@ -92,6 +93,156 @@ describe('validatePath', () => { const result = validatePath('src/../lib/file.js', base); assert.ok(result.safe); }); + + // ─── Dangling symlink + non-canonical base regression coverage ─────────── + // + // Helpers scoped to this describe block. `withSymlinkGuard` matches the + // skip-on-unsupported-platform convention used elsewhere (see + // tests/commands.test.cjs "B12" for the same EPERM/EACCES/ENOTSUP pattern). + + function withSymlinkGuard(t, fn) { + try { + fn(); + } catch (error) { + if (error && ['EPERM', 'EACCES', 'ENOTSUP'].includes(error.code)) { + t.skip('symlink creation is not available on this platform'); + return false; + } + throw error; + } + return true; + } + + function makeScratchDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-sec-validatepath-')); + } + + // Returns a `base` string that is guaranteed non-canonical relative to + // `canonicalDir` — either the real tmpdir (already non-canonical on macOS, + // where os.tmpdir() lives under /var and realpaths to /private/var), or a + // freshly created symlink alias when the platform's tmpdir happens to be + // canonical already (e.g. Linux), so the test is meaningful everywhere. + function makeNonCanonicalBase(canonicalDir, t) { + const realCanonicalDir = fs.realpathSync(canonicalDir); + if (realCanonicalDir !== canonicalDir) { + return { base: canonicalDir, canonical: realCanonicalDir, symlinked: false, aliasParent: null }; + } + const aliasParent = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-sec-alias-')); + const alias = path.join(aliasParent, 'link'); + let ok = withSymlinkGuard(t, () => fs.symlinkSync(canonicalDir, alias, 'dir')); + if (!ok) { + cleanup(aliasParent); + return null; + } + return { base: alias, canonical: canonicalDir, symlinked: true, aliasParent }; + } + + test('in-project symlink to an existing in-project target: still safe:true', (t) => { + const scratch = makeScratchDir(); + try { + const targetPath = path.join(scratch, 'target.txt'); + fs.writeFileSync(targetPath, 'in-project content'); + const linkPath = path.join(scratch, 'link.txt'); + const ok = withSymlinkGuard(t, () => fs.symlinkSync(targetPath, linkPath, 'file')); + if (ok) { + const result = validatePath('link.txt', scratch); + assert.ok(result.safe, `expected safe:true, got error: ${result.error}`); + assert.equal(result.resolved, fs.realpathSync(targetPath)); + } + } finally { + cleanup(scratch); + } + }); + + test('in-project symlink to an existing OUTSIDE target: safe:false (unchanged)', (t) => { + const scratch = makeScratchDir(); + const outside = makeScratchDir(); + try { + const outsideTarget = path.join(outside, 'outside-target.txt'); + fs.writeFileSync(outsideTarget, 'outside content'); + const linkPath = path.join(scratch, 'evil-link.txt'); + const ok = withSymlinkGuard(t, () => fs.symlinkSync(outsideTarget, linkPath, 'file')); + if (ok) { + const result = validatePath('evil-link.txt', scratch); + assert.ok(!result.safe); + } + } finally { + cleanup(scratch); + cleanup(outside); + } + }); + + test('in-project DANGLING symlink to a non-existent OUTSIDE target: safe:false (BLOCKER-1 regression)', (t) => { + const scratch = makeScratchDir(); + try { + const nonExistentOutsideTarget = path.join(os.tmpdir(), `gsd-sec-nonexistent-${process.pid}-${Date.now()}`); + const linkPath = path.join(scratch, 'dangling-link.txt'); + const ok = withSymlinkGuard(t, () => fs.symlinkSync(nonExistentOutsideTarget, linkPath, 'file')); + if (ok) { + const result = validatePath('dangling-link.txt', scratch); + assert.ok(!result.safe, 'a dangling symlink must not be accepted as an in-project path'); + assert.ok( + result.error && result.error.includes('unresolvable symbolic link'), + `expected the unresolvable-symlink error, got: ${result.error}`, + ); + } + } finally { + cleanup(scratch); + } + }); + + test('not-yet-created file in an EXISTING in-project dir, non-canonical base: safe:true', (t) => { + const scratch = makeScratchDir(); + let nc = null; + try { + nc = makeNonCanonicalBase(scratch, t); + if (nc) { + fs.mkdirSync(path.join(nc.canonical, 'existingSub')); + const result = validatePath('existingSub/newfile.txt', nc.base); + assert.ok(result.safe, `expected safe:true, got error: ${result.error}`); + assert.equal(result.resolved, path.join(nc.canonical, 'existingSub', 'newfile.txt')); + } + } finally { + cleanup(scratch); + if (nc && nc.aliasParent) cleanup(nc.aliasParent); + } + }); + + test('not-yet-created file in a not-yet-created SUBDIR, non-canonical base, nested two levels deep: safe:true (BLOCKER-2 regression)', (t) => { + const scratch = makeScratchDir(); + let nc = null; + try { + nc = makeNonCanonicalBase(scratch, t); + if (nc) { + // Neither 'sub1' nor 'sub1/sub2' exist — the immediate-parent-only + // fallback fails here, which is exactly the BLOCKER-2 scenario. + const result = validatePath('sub1/sub2/newfile.txt', nc.base); + assert.ok(result.safe, `expected safe:true, got error: ${result.error}`); + assert.equal(result.resolved, path.join(nc.canonical, 'sub1', 'sub2', 'newfile.txt')); + } + } finally { + cleanup(scratch); + if (nc && nc.aliasParent) cleanup(nc.aliasParent); + } + }); + + test('../ escape from a not-yet-created subdir, non-canonical base: still safe:false', (t) => { + const scratch = makeScratchDir(); + let nc = null; + try { + nc = makeNonCanonicalBase(scratch, t); + if (nc) { + // sub1/sub2 don't exist, and the .. segments escape not just the + // not-yet-created subdirs but the base itself — the ancestor walk-up + // must not turn this into an accepted path. + const result = validatePath('sub1/sub2/../../../escape.txt', nc.base); + assert.ok(!result.safe, 'escaping via .. through not-yet-created dirs must still be rejected'); + } + } finally { + cleanup(scratch); + if (nc && nc.aliasParent) cleanup(nc.aliasParent); + } + }); }); describe('requireSafePath', () => { diff --git a/tests/verify.test.cjs b/tests/verify.test.cjs index 478fc4733..8a3311d61 100644 --- a/tests/verify.test.cjs +++ b/tests/verify.test.cjs @@ -6,6 +6,7 @@ const { test, describe, 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, createTempGitProject, cleanup } = require('./helpers.cjs'); const { gitOrThrow } = require('./helpers/git-fixture.cjs'); const { runHook } = require('./helpers/process-seam.cjs'); @@ -1844,6 +1845,222 @@ describe('verify key-links command', () => { `Expected "No must_haves.key_links" in error: ${output.error}` ); }); + + // ── #3493: path confinement — from:/to: are untrusted plan frontmatter and + // must never be readable outside the project directory. ──────────────────── + + test('from: traversal outside project is rejected — not read, per-link failure only (#3493)', () => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3493-outside-')); + try { + const outsideFile = path.join(outsideDir, 'secret.txt'); + fs.writeFileSync(outsideFile, 'top-secret-oracle-bait\n'); + const traversalFrom = path.relative(tmpDir, outsideFile); + + writePlanWithKeyLinks(tmpDir, [ + `- from: "${traversalFrom.split(path.sep).join('/')}"`, + ' to: "src/b.js"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'module.exports = {};\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.links[0].verified, false); + assert.strictEqual(output.links[0].path_rejected, 'from'); + assert.strictEqual(output.all_verified, false); + // Load-bearing: pins the confinement-specific detail text. This is NOT + // the same as asserting the secret content is absent from the JSON — + // that assertion is tautological here (no code path ever echoes file + // *content* into `detail`/output, confined or not — a no-pattern link + // only ever reports whether `to:` text appears in the source, never + // the source's own bytes), so it would pass even without the + // path-confinement fix. This assertion, by contrast, DOES fail + // pre-fix: without confinement the outside file is actually read, the + // no-pattern branch falls through to "Target not referenced in + // source", and `path_rejected` is never set at all. + assert.strictEqual( + output.links[0].detail, + 'Source path rejected — resolves outside the project directory', + ); + } finally { + cleanup(outsideDir); + } + }); + + test('to: traversal is rejected — outside file content never read even when pattern would match it (#3493)', () => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3493-outside-')); + try { + const outsideFile = path.join(outsideDir, 'secret.txt'); + fs.writeFileSync(outsideFile, 'oracleMarkerXYZ\n'); + const traversalTo = path.relative(tmpDir, outsideFile); + + writePlanWithKeyLinks(tmpDir, [ + '- from: "src/a.js"', + ` to: "${traversalTo.split(path.sep).join('/')}"`, + ' pattern: "oracleMarkerXYZ"', + ]); + // Pattern deliberately absent from the (valid) source so the check must + // fall through to the target read — proving the oracle stays closed. + fs.writeFileSync(path.join(tmpDir, 'src', 'a.js'), 'const x = 1;\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual( + output.links[0].verified, + false, + `Expected verified:false — a matching outside file must never flip this true: ${JSON.stringify(output.links[0])}`, + ); + assert.strictEqual(output.links[0].path_rejected, 'to'); + } finally { + cleanup(outsideDir); + } + }); + + test('absolute from: path is rejected (#3493)', () => { + const absoluteDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3493-absolute-')); + try { + const absoluteFrom = path.join(absoluteDir, 'gsd-3493-absolute-probe.txt'); + fs.writeFileSync(absoluteFrom, 'irrelevant\n'); + + writePlanWithKeyLinks(tmpDir, [ + `- from: "${absoluteFrom.split(path.sep).join('/')}"`, + ' to: "src/b.js"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'module.exports = {};\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.links[0].verified, false); + assert.strictEqual(output.links[0].path_rejected, 'from'); + } finally { + cleanup(absoluteDir); + } + }); + + test('a symlink inside the project pointing outside it is rejected as from: (#3493)', (t) => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3493-outside-')); + try { + const outsideFile = path.join(outsideDir, 'secret.txt'); + fs.writeFileSync(outsideFile, 'top-secret-oracle-bait\n'); + const symlinkPath = path.join(tmpDir, 'src', 'linked.js'); + try { + fs.symlinkSync(outsideFile, symlinkPath, 'file'); + } catch (error) { + if (error && ['EPERM', 'EACCES', 'ENOTSUP'].includes(error.code)) { + t.skip('symlink creation is not available on this platform'); + return; + } + throw error; + } + + writePlanWithKeyLinks(tmpDir, [ + '- from: "src/linked.js"', + ' to: "src/b.js"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'module.exports = {};\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.links[0].verified, false); + assert.strictEqual(output.links[0].path_rejected, 'from'); + } finally { + cleanup(outsideDir); + } + }); + + test('normal in-project from:/to: still verifies with no path_rejected field (#3493 no-regression)', () => { + writePlanWithKeyLinks(tmpDir, [ + '- from: "src/a.js"', + ' to: "src/b.js"', + ' pattern: "import.*b"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'a.js'), "import { x } from './b';\n"); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'exports.x = 1;\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.all_verified, true); + assert.strictEqual(output.links[0].verified, true); + assert.strictEqual(output.links[0].path_rejected, undefined); + }); + + test('a rejected first link does not abort the second, valid link (#3493 per-link failure)', () => { + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3493-outside-')); + const outsideFile = path.join(outsideDir, 'secret.txt'); + fs.writeFileSync(outsideFile, 'irrelevant\n'); + const traversalFrom = path.relative(tmpDir, outsideFile); + + writePlanWithKeyLinks(tmpDir, [ + `- from: "${traversalFrom.split(path.sep).join('/')}"`, + ' to: "src/b.js"', + '- from: "src/a.js"', + ' to: "src/b.js"', + ' pattern: "import.*b"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'a.js'), "import { x } from './b';\n"); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'exports.x = 1;\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.links.length, 2, `Expected both links reported: ${JSON.stringify(output.links)}`); + assert.strictEqual(output.links[0].path_rejected, 'from'); + assert.strictEqual(output.links[0].verified, false); + assert.strictEqual(output.links[1].path_rejected, undefined); + assert.strictEqual(output.links[1].verified, true, `Second link must still evaluate: ${JSON.stringify(output.links[1])}`); + assert.strictEqual(output.all_verified, false); + + cleanup(outsideDir); + }); + + test('empty from: is a malformed link, not a path-confinement rejection (#3493)', () => { + writePlanWithKeyLinks(tmpDir, [ + '- from: ""', + ' to: "src/b.js"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'b.js'), 'module.exports = {};\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.links[0].verified, false); + assert.strictEqual(output.links[0].path_rejected, undefined); + assert.strictEqual( + output.links[0].detail, + 'Source file not found (from: must be a relative file path; describe components/endpoints in via:)', + ); + }); + + test('empty to: with a non-matching pattern is a malformed link, not a path-confinement rejection (#3493)', () => { + writePlanWithKeyLinks(tmpDir, [ + '- from: "src/a.js"', + ' to: ""', + ' pattern: "oracleMarkerXYZ"', + ]); + fs.writeFileSync(path.join(tmpDir, 'src', 'a.js'), 'const x = 1;\n'); + + const result = runGsdTools('verify key-links .planning/phases/01-test/01-01-PLAN.md', tmpDir); + assert.ok(result.success, `Command failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.links[0].verified, false); + assert.strictEqual(output.links[0].path_rejected, undefined); + assert.strictEqual( + output.links[0].detail, + 'Pattern "oracleMarkerXYZ" not found in source or target', + ); + }); });