From 41dfeed45a9f923295d28c0983809baa12f03c4a Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 25 Jun 2026 16:54:59 -0400 Subject: [PATCH] feat(#1724): complete install write-confinement (copyWithPathReplacement, installCodexConfig) (ADR-1239 Phase B) (#1725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#1724): complete install write-confinement (copyWithPathReplacement, installCodexConfig) ADR-1239 Phase B (parent #1679). PR #1706 (2a) confined the layout-driven plan path and _copyStaged's inline guard; this completes the destSubpath write-confinement acceptance criterion for the two remaining write sites and canonicalizes _copyStaged. - copyWithPathReplacement: new required confinementRoot param; a fail-closed gate (assertDestWithinConfigHome + hasExistingSymlinkBetween) runs BEFORE the rmSync/mkdirSync; root threaded through recursion + all 4 call sites (stageRoot for pristine staging, targetDir for the 3 install sites); writes go through the validated absolute path. Exported for behavioral testing. - installCodexConfig: confines config.toml, agents/, and per-agent agents/.toml (name from agent frontmatter) via the canonical gate + symlink-escape guard (parity with the other two functions). - _copyStaged: fail-closed when configDir omitted (all callers pass it); delegates strict-subpath to the canonical gate, keeps its symlink guard, writes through the validated absolute path. Reuses the existing assertDestWithinConfigHome (handles absolute dests via path.resolve) and hasExistingSymlinkBetween — no new module. Behavioral regression tests (escape/dest==root/fail-closed/symlink/name-injection), red-first proven; cross-platform symlink tests use t.skip not bare return. Closes #1724 Co-Authored-By: Claude Opus 4.8 * chore(#1724): backfill changeset PR number (#1725) Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/tidy-tunas-click.md | 5 + bin/install.js | 94 +++-- scripts/lint-test-file-count.allowlist.json | 3 +- .../fix-1679-destsubpath-confinement.test.cjs | 18 +- tests/install-write-confinement.test.cjs | 385 ++++++++++++++++++ 5 files changed, 474 insertions(+), 31 deletions(-) create mode 100644 .changeset/tidy-tunas-click.md create mode 100644 tests/install-write-confinement.test.cjs diff --git a/.changeset/tidy-tunas-click.md b/.changeset/tidy-tunas-click.md new file mode 100644 index 000000000..68724cb7e --- /dev/null +++ b/.changeset/tidy-tunas-click.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 1725 +--- +**Installer writes are now confined to the declared config home** — the workflow/skill emit path (`copyWithPathReplacement`) and the Codex config writer (`installCodexConfig`) now reject any destination that escapes the install root: crafted or absolute paths, path-separator agent names, and pre-existing symlinks are refused before any delete or write. Fail-closed: an install write with no declared root is rejected rather than written unconfined. diff --git a/bin/install.js b/bin/install.js index 49eec7c3f..13e178ec1 100755 --- a/bin/install.js +++ b/bin/install.js @@ -5629,8 +5629,21 @@ function writeCopilotHookConfig(targetDir) { * Reads agent .md files from source, extracts metadata, writes .toml configs. */ function installCodexConfig(targetDir, agentsSrc, sandboxTier = 'codex-agent-sandbox') { - const configPath = path.join(targetDir, 'config.toml'); - const agentsTomlDir = path.join(targetDir, 'agents'); + // ADR-1239 Phase B write-confinement: every Codex config write stays under targetDir. + const configPath = assertDestWithinConfigHome(targetDir, 'config.toml'); + const agentsTomlDir = assertDestWithinConfigHome(targetDir, 'agents'); + const resolvedTargetRoot = path.resolve(targetDir); + // Symlink-escape guard (parity with _copyStaged / copyWithPathReplacement): the + // lexical gate above does not resolve symlinks, so a pre-existing config.toml or + // agents/ symlink could redirect writes outside targetDir. Reject those. + if ( + hasExistingSymlinkBetween(resolvedTargetRoot, configPath) || + hasExistingSymlinkBetween(resolvedTargetRoot, path.resolve(agentsTomlDir)) + ) { + throw new Error( + `installCodexConfig: a Codex config path under "${targetDir}" contains a symlink escaping the install root — refusing to write`, + ); + } fs.mkdirSync(agentsTomlDir, { recursive: true }); const agentEntries = fs.readdirSync(agentsSrc).filter(f => f.startsWith('gsd-') && f.endsWith('.md')); @@ -5675,7 +5688,16 @@ function installCodexConfig(targetDir, agentsSrc, sandboxTier = 'codex-agent-san // follows the same config-driven precedence as the Claude .md effort key. const effortCfg = readGsdEffectiveEffortConfig(targetDir); const tomlContent = generateCodexAgentToml(name, content, modelOverrides, runtimeResolver, effortCfg, sandboxTier); - fs.writeFileSync(path.join(agentsTomlDir, `${name}.toml`), tomlContent); + // Confine the per-agent write to the agents/ dir itself: a crafted agent + // `name` containing path separators must not escape agents/ (which would let + // it clobber config.toml or write elsewhere under the configHome). + const agentTomlPath = assertDestWithinConfigHome(agentsTomlDir, `${name}.toml`); + if (hasExistingSymlinkBetween(resolvedTargetRoot, agentTomlPath)) { + throw new Error( + `installCodexConfig: agent toml path "${agentTomlPath}" contains a symlink escaping the install root — refusing to write`, + ); + } + fs.writeFileSync(agentTomlPath, tomlContent); } const gsdBlock = generateCodexConfigBlock(agents, targetDir); @@ -6738,22 +6760,26 @@ function _copyStaged(stagedDir, destDir, kind, configDir) { // Defense-in-depth: verify destDir is within the install root even if the // upstream assertDestWithinConfigHome check was somehow bypassed. This guards // the actual write site against any future call-site drift. - if (configDir !== undefined) { - const resolvedDest = path.resolve(destDir); - const resolvedRoot = path.resolve(configDir); - if (resolvedDest === resolvedRoot || !resolvedDest.startsWith(resolvedRoot + path.sep)) { - throw new Error( - `_copyStaged: destDir "${destDir}" must be strictly inside the install root "${configDir}", not the root itself — refusing to write`, - ); - } - // Symlink-escape guard: reject if any path component between configDir and - // destDir is a symlink that would redirect writes outside configDir. - if (hasExistingSymlinkBetween(resolvedRoot, resolvedDest)) { - throw new Error( - `_copyStaged: destDir "${destDir}" contains a symlink escaping the install root "${configDir}" — refusing to write`, - ); - } + // Fail-closed: every _copyStaged write must declare its install root so the gate + // can confine it. All callers pass configDir; an omitted root is a bug, not a copy. + if (configDir === undefined) { + throw new Error( + '_copyStaged: configDir (install root) is required to confine writes — refusing to write', + ); } + // Strict-subpath + NUL containment via the canonical gate (shared with the + // layout-driven install plan); throws if destDir escapes the install root. + // destDir here is an absolute path; path.resolve(configDir, absoluteDest) returns it unchanged, so the gate's strict-subpath check still correctly confines it to configDir. + const resolvedDest = assertDestWithinConfigHome(configDir, destDir); + // Symlink-escape guard: reject if any path component between configDir and + // destDir is a symlink that would redirect writes outside configDir. + if (hasExistingSymlinkBetween(path.resolve(configDir), resolvedDest)) { + throw new Error( + `_copyStaged: destDir "${destDir}" contains a symlink escaping the install root "${configDir}" — refusing to write`, + ); + } + // Use the validated absolute path for the actual writes below. + destDir = resolvedDest; if (!fs.existsSync(stagedDir)) return; fs.mkdirSync(destDir, { recursive: true }); @@ -7286,7 +7312,7 @@ function uninstallRuntimeArtifacts(runtime, configDir, scope) { * @param {boolean} isCommand - Whether the source is a command directory * @param {boolean} isGlobal - Whether the install is global */ -function copyWithPathReplacement(srcDir, destDir, pathPrefix, runtime, isCommand = false, isGlobal = false) { +function copyWithPathReplacement(srcDir, destDir, pathPrefix, runtime, isCommand = false, isGlobal = false, confinementRoot) { const isOpencode = runtime === 'opencode'; const isKilo = runtime === 'kilo'; const isGemini = runtime === 'gemini'; @@ -7302,6 +7328,25 @@ function copyWithPathReplacement(srcDir, destDir, pathPrefix, runtime, isCommand const isCline = runtime === 'cline'; const dirName = getDirName(runtime); + // ADR-1239 Phase B write-confinement: refuse to wipe/write a destDir that + // escapes the caller-declared install root. Runs BEFORE the rmSync below so a + // crafted destDir can never delete or write outside confinementRoot. + if (confinementRoot === undefined) { + throw new Error( + 'copyWithPathReplacement: confinementRoot is required to confine writes to the install root — refusing to write', + ); + } + const resolvedConfinementRoot = path.resolve(confinementRoot); + const resolvedDestDir = assertDestWithinConfigHome(confinementRoot, destDir); + if (hasExistingSymlinkBetween(resolvedConfinementRoot, resolvedDestDir)) { + throw new Error( + `copyWithPathReplacement: destDir "${destDir}" contains a symlink escaping the install root "${confinementRoot}" — refusing to write`, + ); + } + // Use the validated absolute path for all writes below so the gate validates + // exactly what is written (a relative destDir would otherwise resolve to cwd). + destDir = resolvedDestDir; + // Clean install: remove existing destination to prevent orphaned files if (fs.existsSync(destDir)) { fs.rmSync(destDir, { recursive: true }); @@ -7315,7 +7360,7 @@ function copyWithPathReplacement(srcDir, destDir, pathPrefix, runtime, isCommand const destPath = path.join(destDir, entry.name); if (entry.isDirectory()) { - copyWithPathReplacement(srcPath, destPath, pathPrefix, runtime, isCommand, isGlobal); + copyWithPathReplacement(srcPath, destPath, pathPrefix, runtime, isCommand, isGlobal, confinementRoot); } else if (entry.name.endsWith('.md')) { // Replace ~/.claude/ and $HOME/.claude/ and ./.claude/ with runtime-appropriate paths // Skip generic replacement for Copilot — convertClaudeToCopilotContent handles all paths @@ -8903,7 +8948,7 @@ function populatePristineDir({ packageSrc, pristineDir, modified, runtime, pathP const srcDir = path.join(packageSrc, top); const stageDir = path.join(stageRoot, top); if (!fs.existsSync(srcDir)) continue; - copyWithPathReplacement(srcDir, stageDir, pathPrefix, runtime, false, isGlobal); + copyWithPathReplacement(srcDir, stageDir, pathPrefix, runtime, false, isGlobal, stageRoot); } for (const relPath of safeModified) { @@ -9846,7 +9891,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { fs.mkdirSync(commandsDir, { recursive: true }); const gsdSrc = _stageSkills(_commandsDir); const gsdDest = path.join(commandsDir, 'gsd'); - copyWithPathReplacement(gsdSrc, gsdDest, pathPrefix, runtime, true, isGlobal); + copyWithPathReplacement(gsdSrc, gsdDest, pathPrefix, runtime, true, isGlobal, targetDir); if (verifyInstalled(gsdDest, 'commands/gsd')) { console.log(` ${green}✓${reset} Installed commands/gsd`); } else { @@ -9928,7 +9973,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { const skillSrc = path.join(src, 'gsd-core'); const skillDest = path.join(targetDir, 'gsd-core'); const savedGsdArtifacts = preserveUserArtifacts(skillDest, USER_OWNED_ARTIFACTS); - copyWithPathReplacement(skillSrc, skillDest, pathPrefix, runtime, false, isGlobal); + copyWithPathReplacement(skillSrc, skillDest, pathPrefix, runtime, false, isGlobal, targetDir); restoreUserArtifacts(skillDest, savedGsdArtifacts); if (verifyInstalled(skillDest, 'gsd-core')) { console.log(` ${green}✓${reset} Installed workflow assets`); @@ -9948,7 +9993,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { const commandsSrc = path.join(src, 'commands', 'gsd'); const commandsDest = path.join(skillDest, 'commands', 'gsd'); if (fs.existsSync(commandsSrc)) { - copyWithPathReplacement(commandsSrc, commandsDest, pathPrefix, runtime, true, isGlobal); + copyWithPathReplacement(commandsSrc, commandsDest, pathPrefix, runtime, true, isGlobal, targetDir); console.log(` ${green}✓${reset} Installed command bodies to gsd-core/commands/gsd/ (workflow delegation targets)`); } } @@ -12365,6 +12410,7 @@ module.exports = { processAttribution, applyRuntimeContentRewritesForCommandsInPlace, _copyStaged, + copyWithPathReplacement, }; // Main logic — only run when not loaded as a module for testing diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 557b7c67f..dcf03c2b1 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -142,9 +142,10 @@ "install-regressions.test.cjs", "install-runtime-artifacts.test.cjs", "install-update-marker.test.cjs", + "install-write-confinement.test.cjs", "install.test.cjs" ], - "issue": "TBD" + "issue": "1679" }, "validate": { "files": [ diff --git a/tests/fix-1679-destsubpath-confinement.test.cjs b/tests/fix-1679-destsubpath-confinement.test.cjs index 9b7869296..8e8e4dd92 100644 --- a/tests/fix-1679-destsubpath-confinement.test.cjs +++ b/tests/fix-1679-destsubpath-confinement.test.cjs @@ -487,15 +487,17 @@ describe('M1: _copyStaged rejects dest equal to configRoot', () => { }); test('M1: _copyStaged throws when destDir equals configRoot (was silently accepted before fix)', () => { - // dest === configRoot: the old guard used !== which let this slip through. - // The new guard uses === which must throw. + // dest === configRoot: the canonical gate (assertDestWithinConfigHome) rejects + // resolved === root with "escapes configHome" / "not configHome itself". assert.throws( () => _copyStaged(stagedDir, configDir, { kind: 'commands', destSubpath: '.', prefix: 'gsd-' }, configDir), (err) => { assert.ok(err instanceof Error, 'must be an Error'); assert.ok( - err.message.includes('_copyStaged') && - (err.message.includes('root itself') || err.message.includes('outside') || err.message.includes('inside')), + err.message.includes('escapes configHome') || + err.message.includes('not configHome itself') || + err.message.includes('outside') || + err.message.includes('inside'), `expected confinement error in: ${err.message}`, ); return true; @@ -511,8 +513,12 @@ describe('M1: _copyStaged rejects dest equal to configRoot', () => { (err) => { assert.ok(err instanceof Error, 'must be an Error'); assert.ok( - err.message.includes('_copyStaged'), - `expected _copyStaged error in: ${err.message}`, + // After EDIT 1, _copyStaged delegates to assertDestWithinConfigHome which + // emits "escapes configHome"; the old "_copyStaged" prefix is no longer present. + err.message.includes('escapes configHome') || + err.message.includes('strict subpath') || + err.message.includes('refusing'), + `expected confinement error in: ${err.message}`, ); return true; }, diff --git a/tests/install-write-confinement.test.cjs b/tests/install-write-confinement.test.cjs new file mode 100644 index 000000000..79d84e322 --- /dev/null +++ b/tests/install-write-confinement.test.cjs @@ -0,0 +1,385 @@ +'use strict'; + +/** + * Behavioral regression tests for ADR-1239 Phase B write-confinement. + * + * Tests cover: + * - copyWithPathReplacement: happy path, escape rejection, dest===root, + * fail-closed (no confinementRoot), symlink escape + * - installCodexConfig: happy path, agent name-injection rejection + * - _copyStaged: escape rejection, symlink escape (regression preserved) + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const { cleanup } = require('./helpers.cjs'); + +process.env['GSD_TEST_MODE'] = '1'; +const { + copyWithPathReplacement, + installCodexConfig, + _copyStaged, +} = require('../bin/install.js'); + +// --------------------------------------------------------------------------- +// copyWithPathReplacement +// --------------------------------------------------------------------------- + +describe('copyWithPathReplacement write-confinement', () => { + test('1. happy path: file is written under confinementRoot', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-happy-')); + try { + const srcDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-src-')); + try { + fs.writeFileSync(path.join(srcDir, 'test.md'), '---\nname: test\n---\nbody\n', 'utf8'); + const destDir = path.join(root, 'sub', 'dest'); + copyWithPathReplacement(srcDir, destDir, '~/.claude/', 'claude', false, false, root); + // The dest dir and its content must exist inside root + const written = fs.existsSync(path.join(destDir, 'test.md')); + assert.ok(written, 'test.md must have been written to destDir under root'); + assert.ok( + path.resolve(destDir).startsWith(path.resolve(root) + path.sep), + 'destDir must be under root', + ); + } finally { + cleanup(srcDir); + } + } finally { + cleanup(root); + } + }); + + test('2. escape rejected: destDir outside confinementRoot → throws, nothing written at escape path', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-root-')); + const escapeName = 'gsd-cwpr-escape-' + Date.now(); + const escapePath = path.join(os.tmpdir(), escapeName); + try { + const srcDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-src2-')); + try { + fs.writeFileSync(path.join(srcDir, 'evil.md'), '# evil\n', 'utf8'); + // destDir resolves outside root via parent traversal + const destDir = path.join(root, '..', escapeName); + assert.throws( + () => copyWithPathReplacement(srcDir, destDir, '~/.claude/', 'claude', false, false, root), + /escap|must be a strict subpath|refusing/i, + ); + // Nothing must have been created at the escape path + assert.ok(!fs.existsSync(escapePath), 'must not create anything at the escape path'); + } finally { + cleanup(srcDir); + } + } finally { + cleanup(root); + // also remove escapePath if it was somehow created (defensive) + if (fs.existsSync(escapePath)) cleanup(escapePath); + } + }); + + test('3. dest === root rejected: throws when destDir equals confinementRoot', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-eqroot-')); + try { + const srcDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-src3-')); + try { + fs.writeFileSync(path.join(srcDir, 'x.md'), '# x\n', 'utf8'); + assert.throws( + () => copyWithPathReplacement(srcDir, root, '~/.claude/', 'claude', false, false, root), + /escap|must be a strict subpath|refusing|configHome itself/i, + ); + } finally { + cleanup(srcDir); + } + } finally { + cleanup(root); + } + }); + + test('4. fail-closed: omitting confinementRoot throws with descriptive message', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-fc-')); + try { + const srcDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-src4-')); + try { + fs.writeFileSync(path.join(srcDir, 'y.md'), '# y\n', 'utf8'); + const destDir = path.join(root, 'sub'); + assert.throws( + () => copyWithPathReplacement(srcDir, destDir, '~/.claude/', 'claude', false, false, undefined), + /confinementRoot is required/, + ); + } finally { + cleanup(srcDir); + } + } finally { + cleanup(root); + } + }); + + test('5. symlink escape: destDir via symlink outside root → throws', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-syml-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-out-')); + try { + const srcDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cwpr-src5-')); + try { + fs.writeFileSync(path.join(srcDir, 'z.md'), '# z\n', 'utf8'); + const linkPath = path.join(root, 'link'); + try { + fs.symlinkSync(outside, linkPath); + } catch (_symlinkErr) { + // Symlink creation unsupported on this platform/privilege — skip test body + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const destDir = path.join(linkPath, 'sub'); + assert.throws( + () => copyWithPathReplacement(srcDir, destDir, '~/.claude/', 'claude', false, false, root), + /symlink|escap|confinement|install root/i, + ); + // Nothing written to outside + assert.strictEqual(fs.readdirSync(outside).length, 0, 'must not write to the outside dir via symlink'); + } finally { + cleanup(srcDir); + } + } finally { + // unlink the symlink before cleanup to avoid crossing boundaries + try { fs.unlinkSync(path.join(root, 'link')); } catch { /* already gone */ } + cleanup(root); + cleanup(outside); + } + }); +}); + +// --------------------------------------------------------------------------- +// installCodexConfig +// --------------------------------------------------------------------------- + +describe('installCodexConfig write-confinement', () => { + test('6. happy path: config.toml and agents/.toml written under targetDir', () => { + const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-happy-')); + try { + const agentsSrc = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-src-')); + try { + // Minimal valid agent frontmatter + fs.writeFileSync( + path.join(agentsSrc, 'gsd-foo.md'), + '---\nname: gsd-foo\ndescription: x\n---\nbody\n', + 'utf8', + ); + const count = installCodexConfig(targetDir, agentsSrc); + assert.strictEqual(count, 1, 'must return count of 1 agent processed'); + assert.ok(fs.existsSync(path.join(targetDir, 'config.toml')), 'config.toml must exist under targetDir'); + assert.ok(fs.existsSync(path.join(targetDir, 'agents', 'gsd-foo.toml')), 'agents/gsd-foo.toml must exist under targetDir'); + } finally { + cleanup(agentsSrc); + } + } finally { + cleanup(targetDir); + } + }); + + test('7a. name-injection rejected: frontmatter name "../../evil" must throw, nothing written at escape', () => { + const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-inj-')); + // Use a unique escape name derived from the targetDir basename so the escape + // path can never collide with pre-existing files in os.tmpdir(). + // agentsTomlDir = resolve(targetDir, 'agents'); ../../.toml from + // there = resolve(targetDir, '../.toml') = dirname(targetDir)/.toml + const escapeName = path.basename(targetDir) + '-escape'; + const escapePath = path.join(path.dirname(targetDir), escapeName + '.toml'); + try { + const agentsSrc = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-src-inj-')); + try { + fs.writeFileSync( + path.join(agentsSrc, 'gsd-evil.md'), + `---\nname: ../../${escapeName}\ndescription: injected\n---\nbody\n`, + 'utf8', + ); + assert.throws( + () => installCodexConfig(targetDir, agentsSrc), + /escap|strict subpath|refusing|NUL/i, + ); + // Verify nothing was written at the escape location + assert.ok(!fs.existsSync(escapePath), 'no file/dir written at escape location'); + } finally { + cleanup(agentsSrc); + } + } finally { + cleanup(targetDir); + if (fs.existsSync(escapePath)) cleanup(escapePath); + } + }); + + test('7b. name-injection: "../config" and "../evil" must both throw (clobber-prevention, tighter agentsTomlDir root)', () => { + // With confinement rooted at agentsTomlDir (not targetDir), a name like + // "../config" resolves to targetDir/config.toml — still inside the configHome + // but OUTSIDE agents/ — so the gate must throw (clobber prevention). + // Similarly "../evil" resolves to targetDir/evil.toml, also outside agents/. + // Both must throw regardless of whether they escape targetDir. + + // Case A: "../config" — would clobber config.toml, must throw. + const targetDirA = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-inj2a-')); + try { + const agentsSrcA = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-src-inj2a-')); + try { + fs.writeFileSync( + path.join(agentsSrcA, 'gsd-clobber.md'), + '---\nname: ../config\ndescription: clobber attempt\n---\nbody\n', + 'utf8', + ); + assert.throws( + () => installCodexConfig(targetDirA, agentsSrcA), + /escap|strict subpath|refusing|NUL/i, + 'name "../config" must throw — it escapes agents/ even though it stays inside targetDir', + ); + } finally { + cleanup(agentsSrcA); + } + } finally { + cleanup(targetDirA); + } + + // Case B: "../evil" — escapes agents/, must throw (new behavior with agentsTomlDir root). + const targetDirB = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-inj2b-')); + try { + const agentsSrcB = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-src-inj2b-')); + try { + fs.writeFileSync( + path.join(agentsSrcB, 'gsd-up.md'), + '---\nname: ../up-escape-attempt\ndescription: boundary test\n---\nbody\n', + 'utf8', + ); + assert.throws( + () => installCodexConfig(targetDirB, agentsSrcB), + /escap|strict subpath|refusing|NUL/i, + 'name "../evil" must throw — it escapes agents/ (resolves to targetDir/evil.toml)', + ); + } finally { + cleanup(agentsSrcB); + } + } finally { + cleanup(targetDirB); + } + + // Case C: "../../evil" still throws (escapes both agents/ and targetDir). + const targetDirC = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-inj2c-')); + try { + const agentsSrcC = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-src-inj2c-')); + try { + fs.writeFileSync( + path.join(agentsSrcC, 'gsd-deep.md'), + '---\nname: ../../deep-escape\ndescription: deep escape\n---\nbody\n', + 'utf8', + ); + assert.throws( + () => installCodexConfig(targetDirC, agentsSrcC), + /escap|strict subpath|refusing|NUL/i, + ); + } finally { + cleanup(agentsSrcC); + } + } finally { + cleanup(targetDirC); + } + }); + + test('10. symlink-escape: agents/ is a symlink outside targetDir → throws', (t) => { + const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-syml-')); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-syml-out-')); + try { + const agentsSrc = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-icc-syml-src-')); + try { + fs.writeFileSync( + path.join(agentsSrc, 'gsd-foo.md'), + '---\nname: gsd-foo\ndescription: x\n---\nbody\n', + 'utf8', + ); + const agentsLink = path.join(targetDir, 'agents'); + try { + fs.symlinkSync(outsideDir, agentsLink); + } catch (_symlinkErr) { + // Symlink creation unsupported on this platform/privilege — skip + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + assert.throws( + () => installCodexConfig(targetDir, agentsSrc), + /symlink|escap|refusing/i, + ); + // Nothing must have been written to the outside dir via the symlink + assert.strictEqual(fs.readdirSync(outsideDir).length, 0, 'must not write to the outside dir via symlink'); + } finally { + cleanup(agentsSrc); + } + } finally { + try { fs.unlinkSync(path.join(targetDir, 'agents')); } catch { /* already gone */ } + cleanup(targetDir); + cleanup(outsideDir); + } + }); +}); + +// --------------------------------------------------------------------------- +// _copyStaged +// --------------------------------------------------------------------------- + +describe('_copyStaged write-confinement', () => { + test('8. escape rejected (regression): destDir escaping configDir → throws', () => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-cfg-')); + const stagedDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-staged-')); + const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-outside-')); + try { + fs.writeFileSync(path.join(stagedDir, 'help.md'), '# help\n', 'utf8'); + assert.throws( + () => _copyStaged(stagedDir, outsideDir, { kind: 'commands', destSubpath: 'commands', prefix: 'gsd-' }, configDir), + /escap|strict subpath|refusing|configHome/i, + ); + } finally { + cleanup(configDir); + cleanup(stagedDir); + cleanup(outsideDir); + } + }); + + test('9. symlink escape rejected: destDir containing symlink to outside → throws', (t) => { + const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-syml-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-syml-out-')); + const stagedDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-staged2-')); + try { + fs.writeFileSync(path.join(stagedDir, 'help.md'), '# help\n', 'utf8'); + const linkPath = path.join(configDir, 'link'); + try { + fs.symlinkSync(outside, linkPath); + } catch (symlinkErr) { + // Symlink creation unsupported on this platform/privilege — skip + t.skip('symlink creation unsupported on this platform/privilege'); + return; + } + const destDir = path.join(linkPath, 'sub'); + assert.throws( + () => _copyStaged(stagedDir, destDir, { kind: 'commands', destSubpath: 'commands/link/sub', prefix: 'gsd-' }, configDir), + /symlink|escap|confinement|install root/i, + ); + assert.strictEqual(fs.readdirSync(outside).length, 0, 'must not have written to outside dir'); + } finally { + try { fs.unlinkSync(path.join(configDir, 'link')); } catch { /* already gone */ } + cleanup(configDir); + cleanup(outside); + cleanup(stagedDir); + } + }); + + test('11. fail-closed: omitting configDir throws with descriptive message', () => { + const stagedDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cs-fc-staged-')); + const destDir = path.join(os.tmpdir(), 'gsd-cs-fc-dest-' + Date.now()); + try { + fs.writeFileSync(path.join(stagedDir, 'help.md'), '# help\n', 'utf8'); + assert.throws( + () => _copyStaged(stagedDir, destDir, { kind: 'commands', destSubpath: 'commands', prefix: 'gsd-' }, undefined), + /configDir.*required|required to confine/i, + ); + } finally { + cleanup(stagedDir); + if (fs.existsSync(destDir)) cleanup(destDir); + } + }); +});