golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

feat(#1704): destSubpath write-confinement (ADR-1239 Phase B) (#1706)

Browse Source 
* feat(#1679): confine install writes within configHome

ADR-1239 Phase B write-confinement: a pure assertDestWithinConfigHome(configDir, destSubpath) rejects a destSubpath that escapes configHome (path traversal / NUL byte) at plan-build time on BOTH the install and uninstall plan paths; surface.applySurface and installOpencodeFamilySkills route through it, and _copyStaged carries a defense-in-depth containment check. Security-load-bearing for the Phase C third-party-descriptor loader.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(#1704): add changeset for destSubpath write-confinement

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(#1704): fix windows path-portability in confinement test

The N1 'accepts a true child subpath' assertion compared against path.join (no drive resolution) while the helper uses path.resolve — on Windows that mismatches the C: drive prefix. Compute the expected via path.resolve to mirror the helper. Windows-CI-only failure (local gsd-test is Mac+Linux).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-25 13:20:25 -04:00
committed by GitHub
parent 584dbd6a47
commit fb5f89db10
7 changed files with 901 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/zesty-rams-march.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Security
pr: 1706
---
**Install write-confinement (ADR-1239 Phase B)** — the installer now rejects any runtime-descriptor `destSubpath` that would write or delete outside the user's config home (path traversal, the config root itself, NUL bytes) and refuses to follow a pre-existing symlink that escapes it. Hardening only; no change to legitimate installs.

2
CONTEXT.md
View File

@@ -164,7 +164,7 @@ Module owning the per-runtime mapping from artifact kind to filesystem placement
Sibling Module to Runtime Artifact Layout Module. Owns projection from canonical Claude-authored command/agent/skill markdown into runtime-specific artifact bodies, including converter selection, frontmatter/body normalization, runtime path rewrites, and staged artifact generation. Runtime Artifact Layout remains responsible for filesystem placement (`kind`, destination subpath, prefix, nesting); Runtime Artifact Conversion owns the content Implementation behind that placement seam so install, uninstall/surface parity, and future plugin/package projections stop reaching back through `bin/install.js` for converter functions or `GSD_TEST_MODE`-guarded installer exports. Chosen direction: sibling Module, not an expanded Layout Module, to preserve ADR-3660's narrow placement responsibility while deepening artifact content locality. First slice: relocate only the layout-reached conversion family (`convertClaudeCommandTo*Skill`, converted command-file emitters, `buildKimiAgentArtifacts`) plus the minimal helper closure they need; do not leave helper dependencies in `bin/install.js` because that would preserve the same shallow seam under a new filename. Installer integration decision: `bin/install.js` imports the conversion Module at top level and re-exports the moved names for compatibility; the conversion Module must not import `bin/install.js` or Runtime Artifact Layout, so the dependency direction becomes installer/layout Adapters -> conversion Module, never conversion -> installer. First-slice Interface decision: export the existing compatibility names only; do not introduce a grouped `convertRuntimeArtifact` Interface until after relocation proves byte-for-byte behavior. SHIPPED (ADR-1508): the converter family relocated in #1510 Phase 1 (`getDirName`→runtime-name-policy, `processAttribution` here); #1511 Phase 2 moved the content-rewrite engine here in full — `_applyRuntimeRewrites` (per-runtime switch, injected attribution), the staged-content walkers `applyRuntimeContentRewritesInPlace`/`applyRuntimeContentRewritesForCommandsInPlace`, `computePathPrefix` (private; `_computePathPrefix` for tests), and the deep public seam `rewriteStagedSkillBodies`/`rewriteStagedCommandBodies({runtime,configDir,scope,homedir?,platform?,resolveAttribution?})`. `bin/install.js` binds these back (single owner, exports preserved); `getCommitAttribution` stays in `bin/install.js` (impure install-time config I/O) and is injected. The `getInstallExports` relay in Runtime Artifact Layout Module was deleted; the dependency direction installer/layout → conversion (never upward) is now enforced. Exception: opencode and kilo path-prefix rewriting is a deliberate `bin/install.js`-owned pre-conversion step (`applyOpencodeFamilyPathPrefix`) per #784, not a violation of the single-owner rule. Source: `gsd-core/bin/lib/runtime-artifact-conversion.cjs` (generated from `src/runtime-artifact-conversion.cts`). Also exports `resolveVersionFrom(libDir)` — a lazy, defensive GSD-version resolver (installed-tree `gsd-core/VERSION` first, then the source/npm `package.json` three dirs up, both validated against the repo's shared semver-prefix shape, degrading to `''` on failure) that replaced a module-load-time `require('../../../package.json')` which crashed on runtimes whose root carries no `package.json` (e.g. Codex) (#1383).
### Runtime Artifact Install Plan Module
Module owning install-time staging and content-rewrite selection for a pre-resolved Runtime Artifact Layout. Interface: `createRuntimeArtifactInstallPlan({ layout, resolvedProfile, homedir?, platform?, resolveAttribution?, deps? }) -> { ok:true, plan:{ items, cleanupDirs } } | { ok:false, kind:'stage_failed'|'rewrite_failed', message, cleanupDirs, failedKind? }`. It iterates `layout.kinds` in order, calls each kind's `stage(resolvedProfile)`, delegates `commands` to Runtime Artifact Conversion `rewriteStagedCommandBodies`, delegates `skills` and `kimi-agents` to `rewriteStagedSkillBodies`, leaves non-rewritten kinds unchanged, and projects copy items as `{ kind, sourceDir, destDir }`. It deliberately does not prune, copy, run legacy migrations, print output, or execute cleanup; those remain Installer Module adapter responsibilities until later slices wire the plan into `bin/install.js`. Source: `gsd-core/bin/lib/runtime-artifact-install-plan.cjs` (generated from `src/runtime-artifact-install-plan.cts`). See Runtime Artifact Layout Module and Runtime Artifact Conversion Module.
Module owning install-time staging and content-rewrite selection for a pre-resolved Runtime Artifact Layout. Interface: `createRuntimeArtifactInstallPlan({ layout, resolvedProfile, homedir?, platform?, resolveAttribution?, deps? }) -> { ok:true, plan:{ items, cleanupDirs } } | { ok:false, kind:'stage_failed'|'rewrite_failed', message, cleanupDirs, failedKind? }`. It iterates `layout.kinds` in order, calls each kind's `stage(resolvedProfile)`, delegates `commands` to Runtime Artifact Conversion `rewriteStagedCommandBodies`, delegates `skills` and `kimi-agents` to `rewriteStagedSkillBodies`, leaves non-rewritten kinds unchanged, and projects copy items as `{ kind, sourceDir, destDir }`. It deliberately does not prune, copy, run legacy migrations, print output, or execute cleanup; those remain Installer Module adapter responsibilities until later slices wire the plan into `bin/install.js`. **Write-confinement (ADR-1239 Phase B / #1679):** the exported pure `assertDestWithinConfigHome(configDir, destSubpath) -> resolvedDest` is the security gate — every kind's `destDir` is computed through it on both the install and uninstall plan paths, so a `destSubpath` that escapes `configHome` (`../../etc`, a NUL byte, etc.) is rejected at plan-build time with a clear error; `surface.cjs:applySurface` and `bin/install.js:installOpencodeFamilySkills` route their joins through the same helper, and `_copyStaged` carries a defense-in-depth containment check. This is security-load-bearing for the Phase C third-party-descriptor loader (which is where an untrusted `destSubpath` could arrive). Source: `gsd-core/bin/lib/runtime-artifact-install-plan.cjs` (generated from `src/runtime-artifact-install-plan.cts`). See Runtime Artifact Layout Module and Runtime Artifact Conversion Module.
### Command Roster Module
Tiny read-only helper Module owning discovery of canonical `commands/gsd/*.md` command stems for artifact conversion and runtime projection. It is a sibling dependency of Runtime Artifact Conversion Module, not part of conversion itself: conversion consumes a roster to safely rewrite `gsd:` / `/gsd-` references, while roster discovery owns filesystem/catalog knowledge. First slice: extract existing `readGsdCommandNames` behavior behind this Module instead of moving it into Runtime Artifact Conversion Module or keeping it as installer-owned state.

55
bin/install.js
View File

@@ -364,6 +364,7 @@ const {
resolveRuntimeArtifactLayout,
} = require(path.join(_gsdLibDir, 'runtime-artifact-layout.cjs'));
const {
assertDestWithinConfigHome,
createRuntimeArtifactInstallPlan,
createRuntimeArtifactUninstallPlan,
} = require(path.join(_gsdLibDir, 'runtime-artifact-install-plan.cjs'));
@@ -6633,13 +6634,20 @@ function migrateLegacyDevPreferencesToSkill(targetDir, saved, runtime, scope = '
const skillsKindEntry = layout.kinds.find((k) => k.kind === 'skills');
if (!skillsKindEntry) return false; // runtime has no skills layout at this scope (e.g. cline local)
const stemName = skillsKindEntry.prefix === '' ? 'dev-preferences' : 'gsd-dev-preferences';
skillDir = path.join(targetDir, skillsKindEntry.destSubpath, stemName);
skillDir = path.join(assertDestWithinConfigHome(targetDir, skillsKindEntry.destSubpath), stemName);
} else {
// Legacy fallback for callers that have not yet been updated to pass runtime
skillDir = path.join(targetDir, 'skills', 'gsd-dev-preferences');
skillDir = path.join(assertDestWithinConfigHome(targetDir, 'skills'), 'gsd-dev-preferences');
}
const skillFile = path.join(skillDir, 'SKILL.md');
if (fs.existsSync(skillFile)) return false;
// Symlink-escape guard: reject if any path component between targetDir and
// skillDir is a symlink that would redirect writes outside the config root.
if (hasExistingSymlinkBetween(path.resolve(targetDir), skillDir)) {
throw new Error(
`migrateLegacyDevPreferencesToSkill: skillDir "${skillDir}" contains a symlink escaping the install root "${targetDir}" — refusing to write`,
);
}
try {
fs.mkdirSync(skillDir, { recursive: true });
fs.writeFileSync(skillFile, saved.get('dev-preferences.md'), 'utf8');
@@ -6726,7 +6734,26 @@ const _stampNonClaudeRuntimeDefaults = runtimeArtifactConversion._stampNonClaude
* - agents: write as-is (files already carry their own `gsd-` prefix).
* For kimi-agents kind: recursively copy generated YAML/prompt files.
*/
function _copyStaged(stagedDir, destDir, kind) {
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`,
);
}
}
if (!fs.existsSync(stagedDir)) return;
fs.mkdirSync(destDir, { recursive: true });
@@ -7040,6 +7067,14 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) {
const kind = kindsByName.get(item.kind);
if (!kind) throw new Error(`Install plan returned unknown artifact kind: ${item.kind}`);
const dest = item.destDir;
// Symlink-escape guard: reject before mkdir if dest (or any component
// between configDir and dest) is a symlink pointing outside configDir.
// mkdirSync follows symlinks, so this must run BEFORE the mkdir call.
if (hasExistingSymlinkBetween(path.resolve(configDir), dest)) {
throw new Error(
`installRuntimeArtifacts: destDir "${dest}" contains a symlink escaping the install root "${configDir}" — refusing to create`,
);
}
fs.mkdirSync(dest, { recursive: true });
if (kind.kind === 'skills' && fs.existsSync(dest)) {
// Pre-prune: snapshot user-owned content before _removeGsdEntries wipes it,
@@ -7066,7 +7101,7 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) {
}
_removeGsdEntries(dest, kind);
_copyStaged(item.sourceDir, dest, kind);
_copyStaged(item.sourceDir, dest, kind, configDir);
// Restore user-owned dirs after the prune+copy
for (const [dirName, snap] of toPreserve) {
@@ -7076,7 +7111,7 @@ function installRuntimeArtifacts(runtime, configDir, scope, resolvedProfile) {
// For non-skills kinds (commands, agents): no user content to preserve;
// just prune stale gsd-* entries and copy new ones.
_removeGsdEntries(dest, kind);
_copyStaged(item.sourceDir, dest, kind);
_copyStaged(item.sourceDir, dest, kind, configDir);
}
}
} finally {
@@ -7140,7 +7175,14 @@ function installOpencodeFamilySkills(runtime, targetDir, rawCommandsDir, pathPre
? convertClaudeCommandToKiloSkill
: convertClaudeCommandToOpencodeSkill;
const dest = path.join(targetDir, skillsKindEntry.destSubpath);
const dest = assertDestWithinConfigHome(targetDir, skillsKindEntry.destSubpath);
// Symlink-escape guard: reject if any path component between targetDir and
// dest is a symlink that would redirect writes outside the config root.
if (hasExistingSymlinkBetween(path.resolve(targetDir), dest)) {
throw new Error(
`installOpencodeFamilySkills: destDir "${dest}" contains a symlink escaping the install root "${targetDir}" — refusing to write`,
);
}
fs.mkdirSync(dest, { recursive: true });
// Preserve user-owned GSD-prefixed skill dirs across the gsd-* prune.
@@ -12322,6 +12364,7 @@ module.exports = {
// runtimeArtifactConversion spread (#1559).
processAttribution,
applyRuntimeContentRewritesForCommandsInPlace,
_copyStaged,
};
// Main logic — only run when not loaded as a module for testing

30
gsd-core/bin/lib/runtime-artifact-install-plan.cjs
View File

@@ -9,6 +9,30 @@
// In .cts (CommonJS output) files, `require` is available as a global.
const _require = require;
const path = _require('node:path');
/**
* Asserts that `destSubpath` resolves to a path inside `configDir`.
*
* Rejects any path that escapes the configDir root (e.g. "../../etc") and any
* path containing a NUL byte. This is a security gate for Phase B of
* ADR-1239: third-party descriptors must never be able to write outside the
* designated config home directory.
*
* @param configDir - The root config directory (e.g. ~/.claude).
* @param destSubpath - The relative path declared by the runtime descriptor.
* @returns The resolved absolute path under configDir.
* @throws {Error} if destSubpath escapes configDir or contains a NUL byte.
*/
function assertDestWithinConfigHome(configDir, destSubpath) {
if (destSubpath.includes('\0')) {
throw new Error(`destSubpath "${destSubpath}" contains a NUL byte and is not valid`);
}
const root = path.resolve(configDir);
const resolved = path.resolve(configDir, destSubpath);
if (resolved === root || !resolved.startsWith(root + path.sep)) {
throw new Error(`destSubpath "${destSubpath}" must be a strict subpath of configHome "${configDir}" — not configHome itself or outside it (escapes configHome)`);
}
return resolved;
}
function errorMessage(err) {
if (err instanceof Error)
return err.message;
@@ -61,7 +85,7 @@ function createRuntimeArtifactInstallPlan(args) {
items.push({
kind: kind.kind,
sourceDir,
destDir: path.join(layout.configDir, kind.destSubpath),
destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath),
});
}
return { ok: true, plan: { items, cleanupDirs } };
@@ -70,8 +94,8 @@ function createRuntimeArtifactUninstallPlan(layout) {
return {
items: layout.kinds.map((kind) => ({
kind: kind.kind,
destDir: path.join(layout.configDir, kind.destSubpath),
destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath),
})),
};
}
module.exports = { createRuntimeArtifactInstallPlan, createRuntimeArtifactUninstallPlan };
module.exports = { assertDestWithinConfigHome, createRuntimeArtifactInstallPlan, createRuntimeArtifactUninstallPlan };

35
src/runtime-artifact-install-plan.cts
View File

@@ -87,6 +87,35 @@ interface CreateRuntimeArtifactInstallPlanArgs {
deps?: Dependencies;
}
/**
* Asserts that `destSubpath` resolves to a path inside `configDir`.
*
* Rejects any path that escapes the configDir root (e.g. "../../etc") and any
* path containing a NUL byte. This is a security gate for Phase B of
* ADR-1239: third-party descriptors must never be able to write outside the
* designated config home directory.
*
* @param configDir - The root config directory (e.g. ~/.claude).
* @param destSubpath - The relative path declared by the runtime descriptor.
* @returns The resolved absolute path under configDir.
* @throws {Error} if destSubpath escapes configDir or contains a NUL byte.
*/
function assertDestWithinConfigHome(configDir: string, destSubpath: string): string {
if (destSubpath.includes('\0')) {
throw new Error(
`destSubpath "${destSubpath}" contains a NUL byte and is not valid`,
);
}
const root = path.resolve(configDir);
const resolved = path.resolve(configDir, destSubpath);
if (resolved === root || !resolved.startsWith(root + path.sep)) {
throw new Error(
`destSubpath "${destSubpath}" must be a strict subpath of configHome "${configDir}" — not configHome itself or outside it (escapes configHome)`,
);
}
return resolved;
}
function errorMessage(err: unknown): string {
if (err instanceof Error) return err.message;
return String(err);
@@ -146,7 +175,7 @@ function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlan
items.push({
kind: kind.kind,
sourceDir,
destDir: path.join(layout.configDir, kind.destSubpath),
destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath),
});
}
@@ -157,9 +186,9 @@ function createRuntimeArtifactUninstallPlan(layout: Layout): UninstallPlan {
return {
items: layout.kinds.map((kind) => ({
kind: kind.kind,
destDir: path.join(layout.configDir, kind.destSubpath),
destDir: assertDestWithinConfigHome(layout.configDir, kind.destSubpath),
})),
};
}
export = { createRuntimeArtifactInstallPlan, createRuntimeArtifactUninstallPlan };
export = { assertDestWithinConfigHome, createRuntimeArtifactInstallPlan, createRuntimeArtifactUninstallPlan };

5
src/surface.cts
View File

@@ -45,6 +45,9 @@ import runtimeArtifactLayout = require('./runtime-artifact-layout.cjs');
const { findInstallSourceRoot } = runtimeArtifactLayout;
// eslint-disable-next-line @typescript-eslint/no-require-imports
import runtimeArtifactConversion = require('./runtime-artifact-conversion.cjs');
// eslint-disable-next-line @typescript-eslint/no-require-imports
import runtimeArtifactInstallPlan = require('./runtime-artifact-install-plan.cjs');
const { assertDestWithinConfigHome } = runtimeArtifactInstallPlan;
const SURFACE_FILE_NAME = '.gsd-surface.json';
@@ -341,7 +344,7 @@ function applySurface(runtimeConfigDir: string, layout: Layout, manifest: Map<st
tempDirsToClean.push(rewritten);
}
}
const dest = path.join(layout.configDir, kind.destSubpath);
const dest = assertDestWithinConfigHome(layout.configDir, kind.destSubpath);
_syncGsdDir(staged, dest, kind, skillManifest);
}
} finally {

783
tests/fix-1679-destsubpath-confinement.test.cjs Normal file
View File

@@ -0,0 +1,783 @@
'use strict';
/**
* Tests for ADR-1239 Phase B: destSubpath write-confinement security gate.
*
* Verifies that assertDestWithinConfigHome rejects escaping destSubpath values
* and that createRuntimeArtifactInstallPlan and createRuntimeArtifactUninstallPlan
* both reject them at plan-build time.
*
* Also covers:
* F3 - assertDestWithinConfigHome rejects destSubpath === configHome itself
* F4 - migrateLegacyDevPreferencesToSkill routes through the confinement gate
* F2 - write sites (installOpencodeFamilySkills) reject symlink-escaping destDir
*/
const { test, describe, beforeEach, afterEach } = 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 {
assertDestWithinConfigHome,
createRuntimeArtifactInstallPlan,
createRuntimeArtifactUninstallPlan,
} = require('../gsd-core/bin/lib/runtime-artifact-install-plan.cjs');
const {
migrateLegacyDevPreferencesToSkill,
installOpencodeFamilySkills,
installRuntimeArtifacts,
_copyStaged,
} = require('../bin/install.js');
const { createTempDir, cleanup } = require('./helpers.cjs');
// ---------------------------------------------------------------------------
// Unit tests for assertDestWithinConfigHome
// ---------------------------------------------------------------------------
describe('assertDestWithinConfigHome', () => {
let configDir;
beforeEach(() => {
configDir = createTempDir('gsd-confine-test-');
});
afterEach(() => {
cleanup(configDir);
});
// --- Rejection cases ---
test('rejects destSubpath "../../etc" that escapes configDir', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, '../../etc'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('escapes configHome'),
`expected "escapes configHome" in: ${err.message}`,
);
return true;
},
);
});
test('rejects destSubpath "../foo" that escapes configDir', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, '../foo'),
/escapes configHome/,
);
});
test('rejects destSubpath "a/../../b" that escapes configDir', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, 'a/../../b'),
/escapes configHome/,
);
});
test('rejects destSubpath containing a NUL byte', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, 'skills\0evil'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('NUL'),
`expected "NUL" in: ${err.message}`,
);
return true;
},
);
});
// --- F3: reject destSubpath that resolves to configHome itself ---
test('F3: rejects destSubpath "." that resolves to configHome itself', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, '.'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('not configHome itself') || err.message.includes('escapes configHome'),
`expected confinement error in: ${err.message}`,
);
return true;
},
);
});
test('F3: rejects destSubpath "a/.." that resolves to configHome itself', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, 'a/..'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('not configHome itself') || err.message.includes('escapes configHome'),
`expected confinement error in: ${err.message}`,
);
return true;
},
);
});
test('F3: rejects destSubpath "skills/../.." that resolves to configHome parent', () => {
assert.throws(
() => assertDestWithinConfigHome(configDir, 'skills/../..'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('not configHome itself') || err.message.includes('escapes configHome'),
`expected confinement error in: ${err.message}`,
);
return true;
},
);
});
// --- Accepted cases ---
test('accepts "skills" and returns path under configDir', () => {
const result = assertDestWithinConfigHome(configDir, 'skills');
assert.ok(
result.startsWith(path.resolve(configDir)),
`expected result to start with configDir (${path.resolve(configDir)}), got: ${result}`,
);
assert.strictEqual(result, path.join(path.resolve(configDir), 'skills'));
});
test('accepts "commands/gsd" and returns path under configDir', () => {
const result = assertDestWithinConfigHome(configDir, 'commands/gsd');
assert.ok(result.startsWith(path.resolve(configDir)));
assert.strictEqual(result, path.join(path.resolve(configDir), 'commands', 'gsd'));
});
test('accepts "./skills" and returns resolved path under configDir', () => {
const result = assertDestWithinConfigHome(configDir, './skills');
assert.ok(result.startsWith(path.resolve(configDir)));
assert.strictEqual(result, path.join(path.resolve(configDir), 'skills'));
});
test('does not match a sibling directory with a shared prefix', () => {
// configDir = /tmp/gsd-foo; a sibling like /tmp/gsd-foobar must NOT be accepted.
// The path.sep guard in the implementation prevents a startsWith match
// from crossing directory boundaries. We verify the happy-path: a valid
// nested subpath resolves to a path strictly under configDir (includes sep).
const result = assertDestWithinConfigHome(configDir, 'subdir/nested');
assert.ok(result.startsWith(path.resolve(configDir) + path.sep));
});
});
// ---------------------------------------------------------------------------
// Integration tests for createRuntimeArtifactInstallPlan
// ---------------------------------------------------------------------------
describe('createRuntimeArtifactInstallPlan destSubpath confinement', () => {
let configDir;
beforeEach(() => {
configDir = createTempDir('gsd-plan-confine-');
});
afterEach(() => {
cleanup(configDir);
});
function noopStage() {
return '/tmp/staged-noop';
}
function makeLayout(destSubpath) {
return {
runtime: 'claude',
configDir,
scope: 'global',
kinds: [
{
kind: 'skills',
destSubpath,
prefix: 'gsd-',
stage: noopStage,
},
],
};
}
test('rejects an escaping destSubpath ("../../escape") at plan-build time', () => {
const layout = makeLayout('../../escape');
assert.throws(
() => createRuntimeArtifactInstallPlan({
layout,
resolvedProfile: { name: 'core' },
deps: {
rewriteStagedSkillBodies: () => undefined,
rewriteStagedCommandBodies: () => undefined,
},
}),
(err) => {
assert.ok(err instanceof Error);
assert.ok(
err.message.includes('escapes'),
`expected "escapes" in: ${err.message}`,
);
return true;
},
);
});
test('normal destSubpath produces plan with destDir under configDir', () => {
const stagedDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-staged-'));
try {
const layout = {
runtime: 'claude',
configDir,
scope: 'global',
kinds: [
{
kind: 'skills',
destSubpath: 'skills',
prefix: 'gsd-',
stage: () => stagedDir,
},
],
};
const result = createRuntimeArtifactInstallPlan({
layout,
resolvedProfile: { name: 'core' },
deps: {
rewriteStagedSkillBodies: () => undefined,
rewriteStagedCommandBodies: () => undefined,
},
});
assert.strictEqual(result.ok, true, 'plan must succeed for normal destSubpath');
assert.strictEqual(result.plan.items.length, 1);
const destDir = result.plan.items[0].destDir;
assert.ok(
destDir.startsWith(path.resolve(configDir)),
`destDir (${destDir}) must be under configDir (${configDir})`,
);
} finally {
cleanup(stagedDir);
}
});
});
// ---------------------------------------------------------------------------
// Integration tests for createRuntimeArtifactUninstallPlan
// ---------------------------------------------------------------------------
describe('createRuntimeArtifactUninstallPlan destSubpath confinement', () => {
let configDir;
beforeEach(() => {
configDir = createTempDir('gsd-uninstall-confine-');
});
afterEach(() => {
cleanup(configDir);
});
function makeUninstallLayout(destSubpath) {
return {
runtime: 'claude',
configDir,
kinds: [
{
kind: 'skills',
destSubpath,
prefix: 'gsd-',
stage: () => '/tmp/staged-noop',
},
],
};
}
test('rejects an escaping destSubpath ("../../escape") at uninstall-plan-build time', () => {
const layout = makeUninstallLayout('../../escape');
assert.throws(
() => createRuntimeArtifactUninstallPlan(layout),
(err) => {
assert.ok(err instanceof Error);
assert.ok(
err.message.includes('escapes'),
`expected "escapes" in: ${err.message}`,
);
return true;
},
);
});
test('rejects destSubpath "../outside" at uninstall-plan-build time', () => {
const layout = makeUninstallLayout('../outside');
assert.throws(
() => createRuntimeArtifactUninstallPlan(layout),
/escapes/,
);
});
test('normal destSubpath produces uninstall plan with destDir under configDir', () => {
const layout = makeUninstallLayout('skills');
const plan = createRuntimeArtifactUninstallPlan(layout);
assert.strictEqual(plan.items.length, 1);
const destDir = plan.items[0].destDir;
assert.ok(
destDir.startsWith(path.resolve(configDir)),
`destDir (${destDir}) must be under configDir (${configDir})`,
);
assert.strictEqual(destDir, path.join(path.resolve(configDir), 'skills'));
});
test('normal nested destSubpath ("commands/gsd") produces uninstall plan with destDir under configDir', () => {
const layout = makeUninstallLayout('commands/gsd');
const plan = createRuntimeArtifactUninstallPlan(layout);
assert.strictEqual(plan.items.length, 1);
const destDir = plan.items[0].destDir;
assert.ok(
destDir.startsWith(path.resolve(configDir)),
`destDir (${destDir}) must be under configDir (${configDir})`,
);
assert.strictEqual(destDir, path.join(path.resolve(configDir), 'commands', 'gsd'));
});
});
// ---------------------------------------------------------------------------
// F4: migrateLegacyDevPreferencesToSkill must route through the confinement gate
// ---------------------------------------------------------------------------
describe('F4: migrateLegacyDevPreferencesToSkill confinement', () => {
let configDir;
let outsideDir;
beforeEach(() => {
configDir = createTempDir('gsd-f4-confine-');
outsideDir = createTempDir('gsd-f4-outside-');
});
afterEach(() => {
cleanup(configDir);
cleanup(outsideDir);
});
test('F4: migrateLegacyDevPreferencesToSkill throws when destSubpath resolves to configHome itself (via mocked layout with "." destSubpath)', () => {
// We cannot easily inject a bad destSubpath through the real layout resolver
// (it resolves to a real valid path). Instead we validate that the function
// uses assertDestWithinConfigHome by passing a runtime whose layout's
// skillsKindEntry.destSubpath, when joined with configDir, would escape — but
// since real layouts are always safe, we test the guard on a deliberately
// crafted saved map calling the real function and observing the path written
// is always within configDir for a real runtime.
//
// Real-layout sanity: verify 'opencode' produces a write inside configDir.
const savedLegacy = new Map([['dev-preferences.md', '# dev prefs\n']]);
// Real opencode layout — should succeed without throwing
assert.doesNotThrow(() => {
migrateLegacyDevPreferencesToSkill(configDir, savedLegacy, 'opencode', 'global');
}, 'migrateLegacyDevPreferencesToSkill with real opencode layout must not throw');
// Verify the written file is inside configDir
const written = [];
function findMd(dir) {
if (!fs.existsSync(dir)) return;
for (const e of fs.readdirSync(dir, { withFileTypes: true })) {
if (e.isDirectory()) findMd(path.join(dir, e.name));
else if (e.name.endsWith('.md')) written.push(path.join(dir, e.name));
}
}
findMd(configDir);
assert.ok(written.length > 0, 'at least one .md must have been written');
for (const f of written) {
assert.ok(
f.startsWith(path.resolve(configDir) + path.sep),
`written file ${f} must be inside configDir ${configDir}`,
);
}
});
test('F4: migrateLegacyDevPreferencesToSkill uses assertDestWithinConfigHome — path.join on configDir+destSubpath cannot escape via symlink in destSubpath string', () => {
// Validate that the guard (assertDestWithinConfigHome) would have caught a
// manipulated destSubpath value. We simulate by calling assertDestWithinConfigHome
// directly with a "."-equivalent subpath (F3 guard) to prove F4 now relies on it.
assert.throws(
() => assertDestWithinConfigHome(configDir, '.'),
(err) => {
assert.ok(err instanceof Error);
return true;
},
'assertDestWithinConfigHome must reject "." (used by F4 guard)',
);
});
});
// ---------------------------------------------------------------------------
// F2: write sites reject a symlink-escaping destDir
// ---------------------------------------------------------------------------
describe('F2: installOpencodeFamilySkills rejects symlink-escaping destDir', () => {
let configDir;
let outsideDir;
let symlinkTarget;
beforeEach(() => {
configDir = createTempDir('gsd-f2-config-');
outsideDir = createTempDir('gsd-f2-outside-');
// Create a symlink inside configDir pointing outside
symlinkTarget = path.join(configDir, 'skills');
fs.symlinkSync(outsideDir, symlinkTarget);
});
afterEach(() => {
// Remove symlink before cleanup to avoid errors
try { fs.unlinkSync(symlinkTarget); } catch { /* already gone */ }
cleanup(configDir);
cleanup(outsideDir);
});
test('F2: installOpencodeFamilySkills throws when skills/ is a symlink pointing outside configDir', () => {
// Create a minimal rawCommandsDir with one .md file
const rawDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-f2-raw-'));
try {
fs.writeFileSync(path.join(rawDir, 'help.md'), '# help\n', 'utf8');
assert.throws(
() => installOpencodeFamilySkills('opencode', configDir, rawDir, '~/.opencode/'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.toLowerCase().includes('symlink') ||
err.message.toLowerCase().includes('escap') ||
err.message.toLowerCase().includes('outside') ||
err.message.toLowerCase().includes('confinement'),
`expected symlink/escape error in: ${err.message}`,
);
return true;
},
);
// Verify nothing was written to outsideDir
const outsideFiles = fs.readdirSync(outsideDir);
assert.strictEqual(outsideFiles.length, 0, 'must not have written anything outside configDir');
} finally {
cleanup(rawDir);
}
});
});
// ---------------------------------------------------------------------------
// M1: _copyStaged defense-in-depth must also reject dest === configRoot
// ---------------------------------------------------------------------------
describe('M1: _copyStaged rejects dest equal to configRoot', () => {
let configDir;
let stagedDir;
beforeEach(() => {
configDir = createTempDir('gsd-m1-config-');
stagedDir = createTempDir('gsd-m1-staged-');
// Write a dummy file into stagedDir so _copyStaged has something to copy
fs.writeFileSync(path.join(stagedDir, 'help.md'), '# help\n', 'utf8');
});
afterEach(() => {
cleanup(configDir);
cleanup(stagedDir);
});
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.
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')),
`expected confinement error in: ${err.message}`,
);
return true;
},
);
});
test('M1: _copyStaged throws when destDir is outside configRoot', () => {
const outsideDir = createTempDir('gsd-m1-outside-');
try {
assert.throws(
() => _copyStaged(stagedDir, outsideDir, { kind: 'commands', destSubpath: 'commands', prefix: 'gsd-' }, configDir),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('_copyStaged'),
`expected _copyStaged error in: ${err.message}`,
);
return true;
},
);
} finally {
cleanup(outsideDir);
}
});
test('M1: _copyStaged accepts destDir strictly under configRoot', () => {
const destDir = path.join(configDir, 'commands', 'gsd');
fs.mkdirSync(destDir, { recursive: true });
// Should not throw — just copies (stagedDir has help.md, kind=commands)
assert.doesNotThrow(
() => _copyStaged(stagedDir, destDir, { kind: 'commands', destSubpath: 'commands/gsd', prefix: 'gsd-' }, configDir),
);
});
});
// ---------------------------------------------------------------------------
// L2: symlink guard BEFORE mkdirSync in installRuntimeArtifacts
// ---------------------------------------------------------------------------
describe('L2: installRuntimeArtifacts rejects symlink-escaping dest before mkdirSync', () => {
let configDir;
let outsideDir;
beforeEach(() => {
configDir = createTempDir('gsd-l2-config-');
outsideDir = createTempDir('gsd-l2-outside-');
// Create configDir/skills as a symlink pointing outside
fs.symlinkSync(outsideDir, path.join(configDir, 'skills'));
});
afterEach(() => {
// Remove symlink before cleanup to avoid crossing dir boundaries
try { fs.unlinkSync(path.join(configDir, 'skills')); } catch { /* already gone */ }
cleanup(configDir);
cleanup(outsideDir);
});
test('L2: installRuntimeArtifacts throws before creating dirs when skills/ is a symlink pointing outside', () => {
// Use the full profile shape (skills: '*') so staging short-circuits early
// and the symlink guard is the first thing that fires.
assert.throws(
() => installRuntimeArtifacts('opencode', configDir, 'global', { name: 'full', skills: '*', agents: new Set() }),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.toLowerCase().includes('symlink') ||
err.message.toLowerCase().includes('escap') ||
err.message.toLowerCase().includes('outside') ||
err.message.toLowerCase().includes('confinement') ||
err.message.toLowerCase().includes('install root'),
`expected symlink/escape error in: ${err.message}`,
);
return true;
},
);
// The symlink itself still exists but no new entries were created in outsideDir
const outsideEntries = fs.readdirSync(outsideDir);
assert.strictEqual(outsideEntries.length, 0, 'must not have created any dirs/files outside configDir');
});
});
// ---------------------------------------------------------------------------
// L1: symlink guard in migrateLegacyDevPreferencesToSkill
// ---------------------------------------------------------------------------
describe('L1: migrateLegacyDevPreferencesToSkill rejects symlink-escaping skillDir', () => {
let configDir;
let outsideDir;
beforeEach(() => {
configDir = createTempDir('gsd-l1-config-');
outsideDir = createTempDir('gsd-l1-outside-');
// Create configDir/skills as a symlink pointing outside
fs.symlinkSync(outsideDir, path.join(configDir, 'skills'));
});
afterEach(() => {
try { fs.unlinkSync(path.join(configDir, 'skills')); } catch { /* already gone */ }
cleanup(configDir);
cleanup(outsideDir);
});
test('L1: migrateLegacyDevPreferencesToSkill throws when skills/ is a symlink pointing outside', () => {
const saved = new Map([['dev-preferences.md', '# dev prefs\n']]);
assert.throws(
() => migrateLegacyDevPreferencesToSkill(configDir, saved, 'opencode', 'global'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.toLowerCase().includes('symlink') ||
err.message.toLowerCase().includes('escap') ||
err.message.toLowerCase().includes('outside') ||
err.message.toLowerCase().includes('install root'),
`expected symlink/escape error in: ${err.message}`,
);
return true;
},
);
// Nothing must have been written outside
const outsideFiles = fs.readdirSync(outsideDir);
assert.strictEqual(outsideFiles.length, 0, 'must not have written anything outside configDir');
});
});
// ---------------------------------------------------------------------------
// L3: relative configDir support
// ---------------------------------------------------------------------------
describe('L3: assertDestWithinConfigHome handles relative configDir', () => {
test('L3: throws when relative configDir + escaping destSubpath resolves outside', () => {
// path.resolve handles relative roots; '../../etc' from '.' would escape
assert.throws(
() => assertDestWithinConfigHome('.', '../../etc'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('escapes configHome') || err.message.includes('outside'),
`expected escape error in: ${err.message}`,
);
return true;
},
);
});
test('L3: throws when "." destSubpath resolves to the relative configDir itself', () => {
// '.' resolves to the same directory as the configDir — must be rejected (F3)
assert.throws(
() => assertDestWithinConfigHome('.', '.'),
/escapes configHome|not configHome itself/,
);
});
test('L3: accepts "skills" under relative "./somedir" and returns absolute path', () => {
const tmpBase = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-l3-'));
const relDir = path.relative(process.cwd(), tmpBase);
try {
const result = assertDestWithinConfigHome(relDir, 'skills');
const expectedBase = path.resolve(relDir);
assert.ok(
result.startsWith(expectedBase + path.sep),
`result (${result}) must be under resolved relDir (${expectedBase})`,
);
assert.strictEqual(result, path.join(expectedBase, 'skills'));
} finally {
fs.rmdirSync(tmpBase);
}
});
});
// ---------------------------------------------------------------------------
// N1: sibling-prefix NEGATIVE assertion
// ---------------------------------------------------------------------------
describe('N1: sibling directory with shared prefix is rejected', () => {
test('N1: rejects sibling path sharing a prefix with configDir', () => {
// /tmp/gsd-foobar is NOT inside /tmp/gsd-foo — must throw despite the
// startsWith prefix overlap at the string level (the sep-check prevents it).
assert.throws(
() => assertDestWithinConfigHome('/tmp/gsd-foo', '../gsd-foobar'),
(err) => {
assert.ok(err instanceof Error, 'must be an Error');
assert.ok(
err.message.includes('escapes configHome') || err.message.includes('outside'),
`expected confinement error in: ${err.message}`,
);
return true;
},
);
});
test('N1: accepts a true child subpath inside configDir', () => {
// 'bar' appended INSIDE /tmp/gsd-foo => the child path — accepted.
// Compute expected via path.resolve (the same primitive the helper uses) so
// the assertion is platform-portable: on Windows path.resolve prepends the
// cwd drive (C:\...) and uses backslashes, which a hardcoded posix literal /
// path.join (no drive) would not match (#1679 Windows-CI portability).
const root = path.resolve('/tmp/gsd-foo');
const result = assertDestWithinConfigHome('/tmp/gsd-foo', 'bar');
assert.strictEqual(result, path.resolve('/tmp/gsd-foo', 'bar'));
assert.ok(result.startsWith(root + path.sep));
});
test('N1: the accepted child does not imply the sibling is accepted', () => {
// Double-check: 'bar' inside is fine, but '../gsd-foobar' (the sibling) is not.
// 'bar' resolves to /tmp/gsd-foo/bar ✓
assert.doesNotThrow(() => assertDestWithinConfigHome('/tmp/gsd-foo', 'bar'));
// '../gsd-foobar' resolves to /tmp/gsd-foobar — NOT inside /tmp/gsd-foo
assert.throws(
() => assertDestWithinConfigHome('/tmp/gsd-foo', '../gsd-foobar'),
/escapes configHome/,
);
});
});
// ---------------------------------------------------------------------------
// N3: Windows-separator coverage (structural guard using path.win32)
// ---------------------------------------------------------------------------
describe('N3: Windows-separator confinement logic (path.win32 semantics)', () => {
/**
* Replicate the assertDestWithinConfigHome predicate using path.win32
* so we can test the sep-guard logic on any platform.
*
* This mirrors the implementation in runtime-artifact-install-plan.cjs
* but forces win32 path semantics.
*/
function assertDestWithinConfigHomeWin32(configDir, destSubpath) {
if (destSubpath.includes('\0')) {
throw new Error(`destSubpath "${destSubpath}" contains a NUL byte and is not valid`);
}
const root = path.win32.resolve(configDir);
const resolved = path.win32.resolve(configDir, destSubpath);
if (resolved === root || !resolved.startsWith(root + path.win32.sep)) {
throw new Error(
`destSubpath "${destSubpath}" must be a strict subpath of configHome "${configDir}" — not configHome itself or outside it (escapes configHome)`,
);
}
return resolved;
}
const winRoot = 'C:\\Users\\me\\.claude';
test('N3: rejects ..\\..\\Windows (Windows backslash traversal)', () => {
assert.throws(
() => assertDestWithinConfigHomeWin32(winRoot, '..\\..\\Windows'),
/escapes configHome/,
);
});
test('N3: rejects mixed ../..\\x traversal', () => {
assert.throws(
() => assertDestWithinConfigHomeWin32(winRoot, '../..\\x'),
/escapes configHome/,
);
});
test('N3: rejects "." that resolves to configHome itself', () => {
assert.throws(
() => assertDestWithinConfigHomeWin32(winRoot, '.'),
/escapes configHome/,
);
});
test('N3: accepts "skills" under Windows root', () => {
const result = assertDestWithinConfigHomeWin32(winRoot, 'skills');
assert.strictEqual(result, path.win32.join(winRoot, 'skills'));
assert.ok(result.startsWith(winRoot + path.win32.sep));
});
test('N3: accepts "commands\\gsd" (Windows nested path) under Windows root', () => {
const result = assertDestWithinConfigHomeWin32(winRoot, 'commands\\gsd');
assert.strictEqual(result, path.win32.join(winRoot, 'commands', 'gsd'));
assert.ok(result.startsWith(winRoot + path.win32.sep));
});
test('N3: rejects sibling C:\\Users\\me\\.claude-extra under win32 semantics', () => {
assert.throws(
() => assertDestWithinConfigHomeWin32(winRoot, '..\\.claude-extra'),
/escapes configHome/,
);
});
});