* feat(#52): add agent_skills_security.trusted_global_roots allowlist Opt-in allowlist so a global: agent skill whose SKILL.md realpath resolves outside the default global skills base (e.g. ~/.claude/skills) is accepted when its real target lies under a user-declared trusted root. Default [] is byte-identical to prior behavior; the symlink-escape guard is preserved and simply re-applied against each declared root. - src/security.cts: loadTrustedGlobalRoots — tilde-expand (~ and ~/), reject project-relative and dangerously broad roots (filesystem/UNC root, homedir), realpath-canonicalize each root every run and drop non-existent ones. - src/init.cts: on base-check failure the guard consults the trusted roots (hoisted out of the loop); emits a stderr NOTE when a skill is accepted via a trusted root so the widened boundary is visible. - src/core.cts: thread agent_skills_security through loadConfig. - config-schema.manifest.json: allow the new key path. - docs/CONFIGURATION.md: document the option and its security model. - tests/agent-skills.test.cjs: unit + end-to-end CLI coverage (regression, feature, negative, broad-root hardening, stderr NOTE). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(#52): add changeset fragment for trusted_global_roots (#754) 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
5
.changeset/rapid-mice-bark.md
Normal file
5
.changeset/rapid-mice-bark.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Added
|
||||
pr: 754
|
||||
---
|
||||
**New `agent_skills_security.trusted_global_roots` config** — opt-in allowlist of trusted root directories so symlinked `global:` agent skills whose real path resolves outside the default skills dir (e.g. `~/.claude/skills`) are accepted; default `[]` is byte-identical and preserves the symlink-escape guard.
|
||||
@@ -115,6 +115,9 @@ GSD stores project settings in `.planning/config.json`. Created during `/gsd-new
|
||||
},
|
||||
"project_code": null,
|
||||
"agent_skills": {},
|
||||
"agent_skills_security": {
|
||||
"trusted_global_roots": []
|
||||
},
|
||||
"response_language": null,
|
||||
"features": {
|
||||
"thinking_partner": false,
|
||||
@@ -395,6 +398,7 @@ Inject custom skill files into GSD subagent prompts. Skills are read by agents a
|
||||
| Setting | Type | Default | Description |
|
||||
|---------|------|---------|-------------|
|
||||
| `agent_skills` | object | `{}` | Map of agent types to skill directory paths |
|
||||
| `agent_skills_security.trusted_global_roots` | array of strings | `[]` | Opt-in allowlist of additional trusted directories for `global:` skills. See [Trusted global skill roots](#trusted-global-skill-roots-agent_skills_securitytrusted_global_roots) |
|
||||
|
||||
### Configuration
|
||||
|
||||
@@ -454,6 +458,50 @@ gsd-tools query config-set agent_skills.gsd-executor '["skills/my-skill"]'
|
||||
|
||||
---
|
||||
|
||||
## Trusted Global Skill Roots (`agent_skills_security.trusted_global_roots`)
|
||||
|
||||
Widen the symlink-safety boundary for `global:` skills by declaring additional trusted root directories.
|
||||
|
||||
### Purpose
|
||||
|
||||
By default, a `global:<name>` skill whose `SKILL.md` real path (after resolving symlinks) escapes the runtime's global skills directory (e.g. `~/.claude/skills/`) is rejected as a symlink-escape. `agent_skills_security.trusted_global_roots` lets you declare additional trusted root directories so symlinked skills whose real target lives under one of them are accepted.
|
||||
|
||||
Common use case: a single source-of-truth skills directory elsewhere on disk (e.g. `~/shared/skills`) symlinked into `~/.claude/skills/` so `git pull` or `rsync` keeps a team's skills up to date without maintaining copies.
|
||||
|
||||
### Configuration
|
||||
|
||||
```json
|
||||
{
|
||||
"agent_skills_security": {
|
||||
"trusted_global_roots": [
|
||||
"~/shared/skills",
|
||||
"/opt/shared-skills"
|
||||
]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### How It Works
|
||||
|
||||
- **Default `[]`** — behavior is byte-identical to omitting the option entirely: only skills whose real `SKILL.md` path resolves inside the default global skills directory are accepted.
|
||||
- **Absolute or tilde-prefixed paths only.** Each entry must be an absolute path (`/opt/shared-skills`) or a `~`/`~/`-prefixed path (tilde expands to your home directory). Project-relative paths are rejected, so an untrusted repo's `.planning/config.json` cannot point trust at a directory inside itself.
|
||||
- **`realpathSync` at load time.** Each declared root is resolved with `realpathSync` on every run, so trust follows the real target and cannot silently drift if a root itself later becomes a symlink. Non-existent or unreadable roots are dropped without error.
|
||||
- **Dangerously broad roots are refused.** The filesystem root (`/`), drive or UNC roots, and your home directory itself cannot be declared as trusted roots — these would make the allowlist meaningless.
|
||||
- **Acceptance rule.** A skill is accepted if and only if its real `SKILL.md` path lies inside the default global skills directory OR inside one of the resolved trusted roots. Skills resolving outside all of these are still rejected.
|
||||
- **Audit note.** When a skill is accepted via a trusted root rather than the default global skills directory, a `[agent-skills] NOTE:` line is written to stderr so the widened boundary remains visible.
|
||||
|
||||
> **Security note:** `trusted_global_roots` is read from the project-local `.planning/config.json`. Only add roots you control and trust. Declaring a broad shared directory widens which symlinked global skills will load for every agent in this project.
|
||||
|
||||
### CLI
|
||||
|
||||
```bash
|
||||
gsd config-set agent_skills_security.trusted_global_roots '["~/shared/skills"]'
|
||||
```
|
||||
|
||||
Setting the parent object (`agent_skills_security`) directly is not supported; use the dot-notation leaf form shown above.
|
||||
|
||||
---
|
||||
|
||||
## Feature Flags
|
||||
|
||||
Toggle optional capabilities via the `features.*` config namespace. Feature flags default to `false` (disabled) — enabling a flag opts into new behavior without affecting existing workflows.
|
||||
|
||||
@@ -106,7 +106,8 @@
|
||||
"model_policy.budget",
|
||||
"model_policy.high",
|
||||
"model_policy.medium",
|
||||
"model_policy.low"
|
||||
"model_policy.low",
|
||||
"agent_skills_security.trusted_global_roots"
|
||||
],
|
||||
"runtimeStateKeys": [
|
||||
"workflow._auto_chain_active"
|
||||
|
||||
@@ -536,6 +536,7 @@ function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<
|
||||
effort: (parsed['effort']) || null,
|
||||
fast_mode: (parsed['fast_mode']) || null,
|
||||
agent_skills: (parsed['agent_skills']) || {},
|
||||
agent_skills_security: (parsed['agent_skills_security']) || null,
|
||||
manager: (parsed['manager']) || {},
|
||||
response_language: get('response_language') || null,
|
||||
claude_md_path: get('claude_md_path') || null,
|
||||
|
||||
23
src/init.cts
23
src/init.cts
@@ -21,7 +21,7 @@ import { stateExtractField } from './state-document.cjs';
|
||||
import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- commands.cjs is an export= CommonJS module
|
||||
import commandsMod = require('./commands.cjs');
|
||||
import { validatePath } from './security.cjs';
|
||||
import { validatePath, loadTrustedGlobalRoots } from './security.cjs';
|
||||
import { getGlobalSkillDir, getGlobalSkillDisplayPath, getGlobalSkillsBase } from './runtime-homes.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- frontmatter.cjs is an export= CommonJS module
|
||||
import frontmatterMod = require('./frontmatter.cjs');
|
||||
@@ -1870,6 +1870,12 @@ function buildAgentSkillsBlock(
|
||||
if (typeof skillPaths === 'string') skillPaths = [skillPaths];
|
||||
if (!Array.isArray(skillPaths) || skillPaths.length === 0) return '';
|
||||
|
||||
// Hoist trusted roots computation before the loop: loadTrustedGlobalRoots does
|
||||
// realpathSync I/O and should run at most once per call, not once per failing skill.
|
||||
// It returns [] cheaply when no roots are configured, so the realpath cost only
|
||||
// occurs when the caller has actually set trusted_global_roots.
|
||||
const trustedGlobalRoots = loadTrustedGlobalRoots(config);
|
||||
|
||||
const validPaths: { ref: string; display: string }[] = [];
|
||||
for (const skillPath of skillPaths) {
|
||||
if (typeof skillPath !== 'string') continue;
|
||||
@@ -1905,10 +1911,17 @@ function buildAgentSkillsBlock(
|
||||
}
|
||||
const pathCheck = validatePath(globalSkillMd, globalSkillsBase, { allowAbsolute: true }) as unknown as Record<string, unknown>;
|
||||
if (!pathCheck['safe']) {
|
||||
process.stderr.write(
|
||||
`[agent-skills] WARNING: Global skill "${skillName}" failed path check (symlink escape?) — skipping\n`,
|
||||
);
|
||||
continue;
|
||||
const acceptedViaTrustedRoot = trustedGlobalRoots.some((root) => {
|
||||
const rootCheck = validatePath(globalSkillMd, root, { allowAbsolute: true }) as unknown as Record<string, unknown>;
|
||||
return Boolean(rootCheck['safe']);
|
||||
});
|
||||
if (!acceptedViaTrustedRoot) {
|
||||
process.stderr.write(
|
||||
`[agent-skills] WARNING: Global skill "${skillName}" failed path check (symlink escape?) — skipping\n`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
process.stderr.write(`[agent-skills] NOTE: Global skill "${skillName}" accepted via trusted_global_roots (resolves outside the default skills dir)\n`);
|
||||
}
|
||||
validPaths.push({ ref: `${globalSkillDir}/SKILL.md`, display: displayPath });
|
||||
continue;
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
// ─── Path Traversal Prevention ──────────────────────────────────────────────
|
||||
@@ -75,6 +76,74 @@ export function validatePath(filePath: unknown, baseDir: unknown, opts: { allowA
|
||||
return { safe: true, resolved: resolvedPath };
|
||||
}
|
||||
|
||||
/**
|
||||
* Load the opt-in trusted global roots allowlist from config.
|
||||
*
|
||||
* Reads `config.agent_skills_security.trusted_global_roots` (an array of
|
||||
* path strings). Each entry is canonicalized via realpathSync: non-strings
|
||||
* are dropped, leading `~/` is expanded to `os.homedir()`, entries that are
|
||||
* not absolute after expansion are dropped (project-relative paths are
|
||||
* rejected as a security boundary), and entries that do not exist on disk are
|
||||
* dropped (a non-existent root is not trustworthy). The canonical realpath is
|
||||
* used for all subsequent checks and as the stored value — this closes the
|
||||
* case-insensitive bypass on macOS APFS (`/users/alice` vs `/Users/alice`)
|
||||
* and ensures trust doesn't drift across re-invocations if a root is
|
||||
* re-created at a different target. Results are de-duplicated by canonical path.
|
||||
*/
|
||||
export function loadTrustedGlobalRoots(config: unknown): string[] {
|
||||
const roots = (config as Record<string, unknown> | null | undefined)
|
||||
?.['agent_skills_security'] as Record<string, unknown> | undefined;
|
||||
const raw = roots?.['trusted_global_roots'];
|
||||
if (!Array.isArray(raw)) return [];
|
||||
|
||||
// Compute canonical homedir once for case-insensitive-safe comparison.
|
||||
let realHome: string;
|
||||
try {
|
||||
realHome = fs.realpathSync(os.homedir());
|
||||
} catch {
|
||||
realHome = os.homedir();
|
||||
}
|
||||
|
||||
const seen = new Set<string>();
|
||||
const result: string[] = [];
|
||||
for (const entry of raw) {
|
||||
if (typeof entry !== 'string') continue;
|
||||
let expanded: string;
|
||||
if (entry === '~') {
|
||||
expanded = os.homedir();
|
||||
} else if (entry.startsWith('~/')) {
|
||||
expanded = path.join(os.homedir(), entry.slice(2));
|
||||
} else {
|
||||
expanded = entry;
|
||||
}
|
||||
if (!path.isAbsolute(expanded)) continue; // reject project-relative
|
||||
|
||||
// Canonicalize: resolve symlinks and normalise case. If the path doesn't
|
||||
// exist or can't be read, skip it — a non-existent root is not trustworthy.
|
||||
let real: string;
|
||||
try {
|
||||
real = fs.realpathSync(expanded);
|
||||
} catch {
|
||||
continue; // non-existent or unreadable — skip
|
||||
}
|
||||
|
||||
// Reject dangerously broad roots: filesystem root (e.g. '/' or 'C:\' or UNC '\\server\share').
|
||||
// Normalize both sides by stripping trailing path separators before comparing so that
|
||||
// Windows UNC shares (where path.parse().root includes a trailing separator) are caught.
|
||||
const stripTrailingSep = (p: string): string => p.replace(/[\\/]+$/, '');
|
||||
if (stripTrailingSep(path.parse(real).root) === stripTrailingSep(real)) continue;
|
||||
// Reject homedir itself (canonical compare closes case-insensitive bypass).
|
||||
// Apply stripTrailingSep for robustness on platforms where realpathSync may
|
||||
// or may not include a trailing separator on the homedir path.
|
||||
if (stripTrailingSep(real) === stripTrailingSep(realHome)) continue;
|
||||
|
||||
if (seen.has(real)) continue;
|
||||
seen.add(real);
|
||||
result.push(real);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate a file path and throw on traversal attempt.
|
||||
* Convenience wrapper around validatePath for use in CLI commands.
|
||||
|
||||
@@ -12,9 +12,52 @@
|
||||
|
||||
const { test, describe, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const { spawnSync } = require('child_process');
|
||||
const fs = require('fs');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
const { runGsdTools, createTempProject, cleanup, TOOLS_PATH } = require('./helpers.cjs');
|
||||
const TEST_ENV_BASE = {
|
||||
GSD_SESSION_KEY: '',
|
||||
CODEX_THREAD_ID: '',
|
||||
CLAUDE_SESSION_ID: '',
|
||||
CLAUDE_CODE_SSE_PORT: '',
|
||||
OPENCODE_SESSION_ID: '',
|
||||
GEMINI_SESSION_ID: '',
|
||||
CURSOR_SESSION_ID: '',
|
||||
WINDSURF_SESSION_ID: '',
|
||||
TERM_SESSION_ID: '',
|
||||
WT_SESSION: '',
|
||||
TMUX_PANE: '',
|
||||
ZELLIJ_SESSION_NAME: '',
|
||||
TTY: '',
|
||||
SSH_TTY: '',
|
||||
};
|
||||
|
||||
/**
|
||||
* Run gsd-tools and capture BOTH stdout and stderr on success.
|
||||
* Returns { success, stdout, stderr }.
|
||||
*/
|
||||
function runGsdToolsWithStderr(args, cwd, env) {
|
||||
const childEnv = { ...process.env, ...TEST_ENV_BASE, ...(env || {}) };
|
||||
try {
|
||||
const result = spawnSync(process.execPath, [TOOLS_PATH, ...args], {
|
||||
cwd,
|
||||
encoding: 'utf-8',
|
||||
env: childEnv,
|
||||
});
|
||||
return {
|
||||
success: result.status === 0,
|
||||
stdout: (result.stdout || '').trim(),
|
||||
stderr: (result.stderr || '').trim(),
|
||||
exitCode: result.status,
|
||||
};
|
||||
} catch (err) {
|
||||
return { success: false, stdout: '', stderr: String(err), exitCode: 1 };
|
||||
}
|
||||
}
|
||||
|
||||
const { loadTrustedGlobalRoots, validatePath } = require('../gsd-core/bin/lib/security.cjs');
|
||||
|
||||
// ─── helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -343,3 +386,390 @@ describe('agent-skills global: prefix', () => {
|
||||
assert.strictEqual(r.ir.block, '', 'block must be empty for empty global: prefix');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── loadTrustedGlobalRoots unit tests (#52) ──────────────────────────────────
|
||||
|
||||
describe('loadTrustedGlobalRoots', () => {
|
||||
test('returns [] for undefined config', () => {
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(undefined), []);
|
||||
});
|
||||
|
||||
test('returns [] for null config', () => {
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(null), []);
|
||||
});
|
||||
|
||||
test('returns [] when agent_skills_security is absent', () => {
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots({}), []);
|
||||
});
|
||||
|
||||
test('returns [] when trusted_global_roots is absent', () => {
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots({ agent_skills_security: {} }), []);
|
||||
});
|
||||
|
||||
test('returns [] when trusted_global_roots is not an array', () => {
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots({ agent_skills_security: { trusted_global_roots: '/some/path' } }), []);
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots({ agent_skills_security: { trusted_global_roots: 42 } }), []);
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots({ agent_skills_security: { trusted_global_roots: true } }), []);
|
||||
});
|
||||
|
||||
test('drops non-string entries from the array', () => {
|
||||
// Use a real temp dir so realpathSync succeeds; non-strings are still dropped
|
||||
const realDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tgr-ns-'));
|
||||
try {
|
||||
const realPath = fs.realpathSync(realDir);
|
||||
const config = { agent_skills_security: { trusted_global_roots: [42, null, realDir, true] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), [realPath]);
|
||||
} finally {
|
||||
cleanup(realDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('drops project-relative (non-absolute) entries', () => {
|
||||
const config = { agent_skills_security: { trusted_global_roots: ['foo/bar', 'relative/path'] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), []);
|
||||
});
|
||||
|
||||
test('keeps absolute paths — real dirs are kept and canonicalized', () => {
|
||||
// Non-existent dirs are dropped; use real temp dirs and compare against realpaths
|
||||
const dir1 = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tgr-d1-'));
|
||||
const dir2 = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tgr-d2-'));
|
||||
try {
|
||||
const real1 = fs.realpathSync(dir1);
|
||||
const real2 = fs.realpathSync(dir2);
|
||||
const config = { agent_skills_security: { trusted_global_roots: [dir1, dir2] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), [real1, real2]);
|
||||
} finally {
|
||||
cleanup(dir1);
|
||||
cleanup(dir2);
|
||||
}
|
||||
});
|
||||
|
||||
test('expands leading ~/ to os.homedir() — kept only if the dir exists', () => {
|
||||
// Create a real subdir under os.tmpdir() and verify it is kept (canonical compare)
|
||||
// Note: we cannot reliably create a dir under os.homedir() in CI, so we verify
|
||||
// the expansion logic using a known-existing absolute path that happens to be
|
||||
// "within" homedir — the tilde expansion is exercised separately; this test
|
||||
// verifies the returned value equals the realpath of the expanded path.
|
||||
const subdir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tgr-tilde-'));
|
||||
try {
|
||||
const realSub = fs.realpathSync(subdir);
|
||||
// Pass a raw path (non-tilde) to verify realpath canonicalization at minimum
|
||||
const config = { agent_skills_security: { trusted_global_roots: [subdir] } };
|
||||
const result = loadTrustedGlobalRoots(config);
|
||||
assert.deepStrictEqual(result, [realSub], 'result must equal realpath of existing dir');
|
||||
} finally {
|
||||
cleanup(subdir);
|
||||
}
|
||||
});
|
||||
|
||||
test('non-existent absolute root is dropped (returns [])', () => {
|
||||
const config = { agent_skills_security: { trusted_global_roots: ['/nonexistent-gsd-root-12345xyz'] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), [], 'non-existent root must be dropped');
|
||||
});
|
||||
|
||||
test('trusted root that is a symlink is canonicalized to the link target', () => {
|
||||
const realTarget = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tgr-symtgt-'));
|
||||
const symlinkPath = path.join(os.tmpdir(), `gsd-tgr-symlink-${Date.now()}`);
|
||||
let symlinkCreated = false;
|
||||
try {
|
||||
try {
|
||||
fs.symlinkSync(realTarget, symlinkPath);
|
||||
symlinkCreated = true;
|
||||
} catch (err) {
|
||||
if (err.code === 'EPERM' || err.code === 'ENOSYS') {
|
||||
// symlinks not supported on this platform — skip
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
const realResolved = fs.realpathSync(realTarget);
|
||||
const config = { agent_skills_security: { trusted_global_roots: [symlinkPath] } };
|
||||
const result = loadTrustedGlobalRoots(config);
|
||||
assert.deepStrictEqual(result, [realResolved], 'symlink root must be canonicalized to the link target');
|
||||
} finally {
|
||||
cleanup(realTarget);
|
||||
if (symlinkCreated) {
|
||||
try { fs.unlinkSync(symlinkPath); } catch { /* ignore */ }
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('de-duplicates entries by canonical path', () => {
|
||||
// Both entries point to the same real dir — after canonicalization, only one is kept
|
||||
const realDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-tgr-dedup-'));
|
||||
try {
|
||||
const realPath = fs.realpathSync(realDir);
|
||||
const config = { agent_skills_security: { trusted_global_roots: [realDir, realDir, realPath] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), [realPath]);
|
||||
} finally {
|
||||
cleanup(realDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('expands ~/ before absolute check — non-existent ~/x is dropped after expansion', () => {
|
||||
// ~/x becomes an absolute path after expansion, but if ~/x does not exist it is
|
||||
// dropped by the realpathSync guard (non-existent root is not trustworthy).
|
||||
const expandedX = path.join(os.homedir(), 'x-gsd-nonexistent-12345');
|
||||
// Ensure it really doesn't exist
|
||||
if (fs.existsSync(expandedX)) {
|
||||
// Cannot test non-existence reliably — skip assertion
|
||||
return;
|
||||
}
|
||||
const config = { agent_skills_security: { trusted_global_roots: ['~/x-gsd-nonexistent-12345'] } };
|
||||
const result = loadTrustedGlobalRoots(config);
|
||||
assert.deepStrictEqual(result, [], 'non-existent ~/x must be dropped after expansion');
|
||||
});
|
||||
|
||||
test('expands bare ~ to os.homedir()', () => {
|
||||
// Bare ~ (exactly) must expand to homedir — mirrors runtime-homes.cts:28
|
||||
const config = { agent_skills_security: { trusted_global_roots: ['~'] } };
|
||||
const result = loadTrustedGlobalRoots(config);
|
||||
// ~ expands to homedir, which is then rejected as a dangerously broad root
|
||||
// So the result must be [] (rejected after expansion)
|
||||
assert.deepStrictEqual(result, [], 'bare ~ expands to homedir and is then rejected as too broad');
|
||||
});
|
||||
|
||||
test('rejects filesystem root /', () => {
|
||||
const config = { agent_skills_security: { trusted_global_roots: ['/'] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), [], 'filesystem root must be rejected');
|
||||
});
|
||||
|
||||
test('rejects os.homedir() itself', () => {
|
||||
const config = { agent_skills_security: { trusted_global_roots: [os.homedir()] } };
|
||||
assert.deepStrictEqual(loadTrustedGlobalRoots(config), [], 'homedir itself must be rejected as too broad');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── trusted_global_roots integration guard (#52) ─────────────────────────────
|
||||
//
|
||||
// NOTE: These tests validate the trusted-root bypass logic by directly calling
|
||||
// loadTrustedGlobalRoots + validatePath rather than invoking the full CLI
|
||||
// (which would require controlling the runtime HOME path in a way that also
|
||||
// triggers a symlink escape scenario through gsd-tools subprocess invocation).
|
||||
// Full end-to-end symlink testing would require OS-level symlink setup in tmp
|
||||
// dirs and a mechanism to redirect the runtime home path — coverage here is
|
||||
// sufficient to verify the core guard logic.
|
||||
|
||||
describe('trusted_global_roots guard logic', () => {
|
||||
let tmpDir;
|
||||
let externalDir;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-52-trusted-'));
|
||||
externalDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-52-external-'));
|
||||
// Create a skill file in externalDir
|
||||
fs.writeFileSync(path.join(externalDir, 'SKILL.md'), '# External\n');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
cleanup(externalDir);
|
||||
});
|
||||
|
||||
test('validatePath rejects skill outside globalSkillsBase (baseline — no trusted roots)', () => {
|
||||
const skillMd = path.join(externalDir, 'SKILL.md');
|
||||
const result = validatePath(skillMd, tmpDir, { allowAbsolute: true });
|
||||
assert.ok(!result.safe, 'skill outside base must be rejected by validatePath');
|
||||
});
|
||||
|
||||
test('with trusted root matching real target dir — validatePath accepts', () => {
|
||||
// Simulate the trusted-root fallback: skill is outside base but inside trusted root
|
||||
const skillMd = path.join(externalDir, 'SKILL.md');
|
||||
const baseCheck = validatePath(skillMd, tmpDir, { allowAbsolute: true });
|
||||
assert.ok(!baseCheck.safe, 'base check must fail (prerequisite)');
|
||||
|
||||
// Trusted root fallback: check against externalDir
|
||||
const config = { agent_skills_security: { trusted_global_roots: [externalDir] } };
|
||||
const trustedRoots = loadTrustedGlobalRoots(config);
|
||||
const acceptedViaTrustedRoot = trustedRoots.some((root) => {
|
||||
const rootCheck = validatePath(skillMd, root, { allowAbsolute: true });
|
||||
return rootCheck.safe;
|
||||
});
|
||||
assert.ok(acceptedViaTrustedRoot, 'skill must be accepted when within a trusted root');
|
||||
});
|
||||
|
||||
test('with unrelated trusted root — skill still rejected', () => {
|
||||
const skillMd = path.join(externalDir, 'SKILL.md');
|
||||
const unrelatedDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-52-unrelated-'));
|
||||
try {
|
||||
const config = { agent_skills_security: { trusted_global_roots: [unrelatedDir] } };
|
||||
const trustedRoots = loadTrustedGlobalRoots(config);
|
||||
const acceptedViaTrustedRoot = trustedRoots.some((root) => {
|
||||
const rootCheck = validatePath(skillMd, root, { allowAbsolute: true });
|
||||
return rootCheck.safe;
|
||||
});
|
||||
assert.ok(!acceptedViaTrustedRoot, 'skill must still be rejected when trusted root is unrelated');
|
||||
} finally {
|
||||
cleanup(unrelatedDir);
|
||||
}
|
||||
});
|
||||
|
||||
test('with empty trusted_global_roots array — skill still rejected (byte-identical to today)', () => {
|
||||
const skillMd = path.join(externalDir, 'SKILL.md');
|
||||
const config = { agent_skills_security: { trusted_global_roots: [] } };
|
||||
const trustedRoots = loadTrustedGlobalRoots(config);
|
||||
assert.strictEqual(trustedRoots.length, 0, 'no roots loaded');
|
||||
const acceptedViaTrustedRoot = trustedRoots.some((root) => {
|
||||
const rootCheck = validatePath(skillMd, root, { allowAbsolute: true });
|
||||
return rootCheck.safe;
|
||||
});
|
||||
assert.ok(!acceptedViaTrustedRoot, 'skill must be rejected when trusted roots is empty');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── trusted_global_roots e2e CLI tests (#52) ─────────────────────────────────
|
||||
//
|
||||
// These tests exercise the full CLI path (runAgentSkillsJson → gsd-tools →
|
||||
// loadConfig → agent-skills command) to verify that agent_skills_security is
|
||||
// properly threaded through the config pipeline. Symlinks are created so a
|
||||
// global: skill's realpath escapes the ~/.claude/skills/ base, requiring a
|
||||
// trusted root to be accepted.
|
||||
|
||||
describe('trusted_global_roots e2e CLI (#52)', () => {
|
||||
let tmpDir;
|
||||
let fakeHome;
|
||||
let globalSkillsDir;
|
||||
let sharedRoot;
|
||||
let symlinkSupported;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = createTempProject();
|
||||
fakeHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-52-e2e-home-'));
|
||||
globalSkillsDir = path.join(fakeHome, '.claude', 'skills');
|
||||
fs.mkdirSync(globalSkillsDir, { recursive: true });
|
||||
sharedRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-52-e2e-shared-'));
|
||||
|
||||
// Create the shared skill directory OUTSIDE fakeHome
|
||||
const sharedSkillDir = path.join(sharedRoot, 'shared-skill');
|
||||
fs.mkdirSync(sharedSkillDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(sharedSkillDir, 'SKILL.md'), '# Shared Skill\nContent from shared root.\n');
|
||||
|
||||
// Attempt to create a symlink inside globalSkillsDir pointing to the shared skill
|
||||
symlinkSupported = true;
|
||||
try {
|
||||
fs.symlinkSync(sharedSkillDir, path.join(globalSkillsDir, 'shared-skill'));
|
||||
} catch (err) {
|
||||
if (err.code === 'EPERM' || err.code === 'ENOSYS') {
|
||||
symlinkSupported = false;
|
||||
} else {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(tmpDir);
|
||||
cleanup(fakeHome);
|
||||
cleanup(sharedRoot);
|
||||
});
|
||||
|
||||
test('REGRESSION: symlinked-escape skill with NO agent_skills_security in config → block is empty', (t) => {
|
||||
if (!symlinkSupported) {
|
||||
t.skip('symlinks not supported on this platform');
|
||||
return;
|
||||
}
|
||||
// No agent_skills_security in config — symlink escape must be blocked
|
||||
writeConfig(tmpDir, {
|
||||
runtime: 'claude',
|
||||
agent_skills: { 'gsd-executor': ['global:shared-skill'] },
|
||||
});
|
||||
|
||||
const r = runAgentSkillsJson(
|
||||
['agent-skills', 'gsd-executor'], tmpDir, { HOME: fakeHome, USERPROFILE: fakeHome }
|
||||
);
|
||||
assert.ok(r.success, `Command failed: ${r.error}`);
|
||||
assert.strictEqual(r.ir.block, '', 'block must be empty when symlink escapes base and no trusted root configured');
|
||||
});
|
||||
|
||||
test('FEATURE: symlink escape with matching trusted_global_roots → block includes skill', (t) => {
|
||||
if (!symlinkSupported) {
|
||||
t.skip('symlinks not supported on this platform');
|
||||
return;
|
||||
}
|
||||
// Configure the sharedRoot as a trusted global root
|
||||
writeConfig(tmpDir, {
|
||||
runtime: 'claude',
|
||||
agent_skills: { 'gsd-executor': ['global:shared-skill'] },
|
||||
agent_skills_security: { trusted_global_roots: [sharedRoot] },
|
||||
});
|
||||
|
||||
const r = runAgentSkillsJson(
|
||||
['agent-skills', 'gsd-executor'], tmpDir, { HOME: fakeHome, USERPROFILE: fakeHome }
|
||||
);
|
||||
assert.ok(r.success, `Command failed: ${r.error}`);
|
||||
assert.ok(r.ir.block.includes('<agent_skills>'), `block must contain <agent_skills> tag, got: ${r.ir.block}`);
|
||||
assert.ok(r.ir.block.includes('shared-skill/SKILL.md'), `block must include the shared skill, got: ${r.ir.block}`);
|
||||
assert.ok(r.ir.skills_count >= 1, 'skills_count must be at least 1');
|
||||
});
|
||||
|
||||
test('FEATURE NOTE: accepted-via-trusted-root emits NOTE on stderr', (t) => {
|
||||
if (!symlinkSupported) {
|
||||
t.skip('symlinks not supported on this platform');
|
||||
return;
|
||||
}
|
||||
// Capture stderr using spawnSync (runGsdTools only captures stderr on failure)
|
||||
writeConfig(tmpDir, {
|
||||
runtime: 'claude',
|
||||
agent_skills: { 'gsd-executor': ['global:shared-skill'] },
|
||||
agent_skills_security: { trusted_global_roots: [sharedRoot] },
|
||||
});
|
||||
|
||||
const r = runGsdToolsWithStderr(
|
||||
['agent-skills', '--json', 'gsd-executor'],
|
||||
tmpDir,
|
||||
{ HOME: fakeHome, USERPROFILE: fakeHome }
|
||||
);
|
||||
assert.ok(r.success, `Command failed (exit ${r.exitCode}): ${r.stderr}`);
|
||||
// The NOTE must appear on stderr using only the skill name (no full paths)
|
||||
assert.ok(
|
||||
r.stderr.includes('[agent-skills] NOTE: Global skill "shared-skill" accepted via trusted_global_roots'),
|
||||
`stderr must contain the trusted-root NOTE, got: ${r.stderr}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('NEGATIVE: symlink escape with unrelated trusted root (existing dir) → block is empty', (t) => {
|
||||
if (!symlinkSupported) {
|
||||
t.skip('symlinks not supported on this platform');
|
||||
return;
|
||||
}
|
||||
// The unrelated dir MUST exist so it isn't dropped for the wrong reason (non-existence).
|
||||
// Rejection must be because it doesn't cover the shared skill location, not because
|
||||
// the dir is missing — otherwise the test would pass vacuously.
|
||||
const unrelatedRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-52-e2e-unrelated-'));
|
||||
// Verify the dir actually exists so the trusted root is loaded (not silently dropped)
|
||||
assert.ok(fs.existsSync(unrelatedRoot), 'unrelated root must exist so it enters the trusted roots list');
|
||||
try {
|
||||
writeConfig(tmpDir, {
|
||||
runtime: 'claude',
|
||||
agent_skills: { 'gsd-executor': ['global:shared-skill'] },
|
||||
agent_skills_security: { trusted_global_roots: [unrelatedRoot] },
|
||||
});
|
||||
|
||||
const r = runAgentSkillsJson(
|
||||
['agent-skills', 'gsd-executor'], tmpDir, { HOME: fakeHome, USERPROFILE: fakeHome }
|
||||
);
|
||||
assert.ok(r.success, `Command failed: ${r.error}`);
|
||||
assert.strictEqual(r.ir.block, '', 'block must be empty when trusted root does not cover the shared skill location');
|
||||
} finally {
|
||||
cleanup(unrelatedRoot);
|
||||
}
|
||||
});
|
||||
|
||||
test('HARDENING: trusted_global_roots: ["/"] → block is empty (broad root rejected)', (t) => {
|
||||
if (!symlinkSupported) {
|
||||
t.skip('symlinks not supported on this platform');
|
||||
return;
|
||||
}
|
||||
writeConfig(tmpDir, {
|
||||
runtime: 'claude',
|
||||
agent_skills: { 'gsd-executor': ['global:shared-skill'] },
|
||||
agent_skills_security: { trusted_global_roots: ['/'] },
|
||||
});
|
||||
|
||||
const r = runAgentSkillsJson(
|
||||
['agent-skills', 'gsd-executor'], tmpDir, { HOME: fakeHome, USERPROFILE: fakeHome }
|
||||
);
|
||||
assert.ok(r.success, `Command failed: ${r.error}`);
|
||||
assert.strictEqual(r.ir.block, '', 'block must be empty when "/" is the trusted root (rejected as too broad)');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user