diff --git a/.changeset/sturdy-sloths-roar.md b/.changeset/sturdy-sloths-roar.md new file mode 100644 index 000000000..b99e87298 --- /dev/null +++ b/.changeset/sturdy-sloths-roar.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3918 +--- +**Global Claude installs now load agent-file `@`-includes** — agent files in a global Claude install (e.g. the planner) carried `@`-includes that silently loaded nothing, so guidance those agents were supposed to read — including the untrusted-input boundary — was absent from their context; those includes now resolve on `~/`. Also fixes a related path-rewrite bug where a `--config-dir` name extending `.claude` (e.g. `.claude-work`) doubled its own suffix in agent-file paths. (#3719) diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index ce439a89c..08d8293dc 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -2949,7 +2949,13 @@ function restoreClaudeGlobalAtRefTilde(content, pathPrefix) { if (typeof pathPrefix !== 'string' || !pathPrefix.startsWith('$HOME')) return content; const tildeEquivalent = '~' + pathPrefix.slice('$HOME'.length); const atRefRe = new RegExp(`(? `@${tildeEquivalent}`); } /** @@ -3410,8 +3416,34 @@ function applyAgentPathRewrites(content: string, runtime: string, pathPrefix: st const normalizedPathPrefix = pathPrefix.replace(/\/$/, ''); content = content.replace(/~\/\.claude\//g, pathPrefix); content = content.replace(/\$HOME\/\.claude\//g, pathPrefix); - content = content.replace(/~\/\.claude\b/g, normalizedPathPrefix); - content = content.replace(/\$HOME\/\.claude\b/g, normalizedPathPrefix); + // #3719 review (MAJOR): a bare `\b` is satisfied by ANY non-word character, + // including '-' — for a --config-dir whose name EXTENDS '.claude' (e.g. + // '.claude-work'), this re-matched the '.claude' PREFIX of the emitted + // '.claude-work' path and corrupted it to '.claude-work-work'. Guard with + // the SAME negative-lookahead convention already used at + // copyWithPathReplacement's call site (bin/install.js:7729-7730). + content = content.replace(/~\/\.claude(?![\w-])/g, normalizedPathPrefix); + content = content.replace(/\$HOME\/\.claude(?![\w-])/g, normalizedPathPrefix); + // #3719: the THIRD emit path that needed this restore. #3133 added it to the + // skill/command pipeline (`_applyRuntimeRewrites` case 'claude') and #3544 to + // bin/install.js's spec-tree copy; the agents pipeline never got it, so every + // `@~/.claude/...` include in a global Claude install shipped as `@$HOME/...` + // and resolved to NOTHING (27 of 34 emitted agents, 103 lines). + // + // Guarded on `claude` for the same reason the sibling call site lives inside + // `case 'claude'`: the helper self-guards only on the `$HOME` PREFIX, and every + // runtime's global prefix is a `$HOME` form (`$HOME/.cursor/`, ...), so an + // unguarded call would rewrite `@`-refs for runtimes whose resolver has no + // documented `~` expansion at all. + // + // Passed the NORMALIZED prefix, not `pathPrefix`. The two word-boundary + // replaces above emit the trailing-slash-free form, and the helper's regex is + // anchored to the exact prefix string it is handed — so `restore(pathPrefix)` + // fixes `@$HOME/.claude/x` and leaves a bare `@$HOME/.claude` broken. The + // normalized form is a PREFIX of both, so one call covers both. + if (runtime === 'claude') { + content = restoreClaudeGlobalAtRefTilde(content, normalizedPathPrefix); + } return content; } diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index dd8fd6ff2..0ec45478b 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -38,6 +38,7 @@ const { installerEnv, stripAnsi, runMinimalInstall, + walk, } = require('./helpers/install-shared.cjs'); const { @@ -7482,3 +7483,126 @@ describe('#2875: user-artifact-staging — call-site integration (C7 anti-inertn assert.equal(entries.length, 0, 'the orphan is discarded once recovered — not left for a third run to find again'); }); }); + +// --------------------------------------------------------------------------- +// #3719: applyAgentPathRewrites (install-profiles.cts:964, the agents/ +// staging pipeline's pre-converter path-rewrite step) never calls +// restoreClaudeGlobalAtRefTilde — the same restore #3133/#3544 wired into +// the skill-staging and gsd-core/ spec-tree emit paths. A real global Claude +// install therefore ships every `agents/gsd-*.md` `@`-include as +// `@$HOME/.claude/...`, which Claude Code does not expand: the include +// silently loads nothing (planner guidance, the untrusted-input boundary, +// the agent-skills bootstrap, the mandatory initial read). +// +// Uses the real spawned installer (runMinimalInstall, `bin/install.js` +// subprocess) so a green run here proves the fix reaches the actual emit +// path, not just the pure function tested in tests/path-replacement.test.cjs. +// --------------------------------------------------------------------------- +describe('#3719: real global Claude install — agents/*.md @-refs must resolve on tilde', () => { + let claudeGlobal; + let projectLocal; + + before(() => { + claudeGlobal = runMinimalInstall({ runtime: 'claude', scope: 'global' }); + projectLocal = runMinimalInstall({ runtime: 'claude', scope: 'local' }); + }); + + after(() => { + cleanup(claudeGlobal.root); + cleanup(projectLocal.root); + }); + + // A live `@`-include is BARE markdown. An occurrence inside an inline-code span is + // PROSE ABOUT one — `gsd-core/CHANGELOG.md` ships the #3133/#3544 entries, which + // quote `@$HOME/...` verbatim while describing the very defect this row guards. + // + // Dropping the line-start anchor (correct: it hid 48 mid-line refs) made those + // entries visible to this scan, so the row went red on a CORRECT tree. The fix is + // to strip inline-code spans, NOT to restore the anchor — restoring it would trade + // a false positive for the false negative that let this bug ship in the first place. + const INLINE_CODE_SPAN_RE = /`[^`]*`/g; + function collectAtHomeLines(rootDir) { + const failures = []; + for (const file of walk(rootDir)) { + if (!file.endsWith('.md')) continue; + const content = fs.readFileSync(file, 'utf8'); + for (const line of splitLines(content)) { + if (/@\$HOME\//.test(line.replace(INLINE_CODE_SPAN_RE, ''))) failures.push({ file, line }); + } + } + return failures; + } + + test('row 3 [RED] — a real global Claude install emits ZERO @$HOME/ lines across agents/*.md', () => { + const agentsDir = path.join(claudeGlobal.configDir, 'agents'); + assert.ok(fs.existsSync(agentsDir), `expected ${agentsDir} to exist`); + const failures = []; + for (const file of walk(agentsDir)) { + if (!file.endsWith('.md')) continue; + const content = fs.readFileSync(file, 'utf8'); + for (const line of splitLines(content)) { + if (/@\$HOME\//.test(line.replace(INLINE_CODE_SPAN_RE, ''))) failures.push(`${path.relative(agentsDir, file)}: ${line}`); + } + } + assert.deepStrictEqual(failures, [], `agents/*.md files with a broken @$HOME/ include:\n${failures.join('\n')}`); + }); + + test('row 4 [RED, non-vacuity] — that same install DOES emit @~/ includes in agents/*.md', () => { + const agentsDir = path.join(claudeGlobal.configDir, 'agents'); + const planner = fs.readFileSync(path.join(agentsDir, 'gsd-planner.md'), 'utf8'); + const tildeLines = splitLines(planner).filter((l) => l.startsWith('@~/')); + assert.ok( + tildeLines.length > 0, + `expected at least one @~/ line in gsd-planner.md (proves the file is not empty/absent), got: ${JSON.stringify(splitLines(planner).filter((l) => l.startsWith('@')))}`, + ); + assert.ok( + tildeLines.some((l) => l.includes('mandatory-initial-read.md')), + `expected the mandatory-initial-read.md @-include specifically to survive on tilde, got: ${JSON.stringify(tildeLines)}`, + ); + }); + + test('row 5 [CONTROL] — skills and workflows emit paths still emit @~/ (no #3133/#3544 regression)', () => { + const gsdCoreDir = path.join(claudeGlobal.configDir, 'gsd-core'); + assert.ok(fs.existsSync(gsdCoreDir), `expected ${gsdCoreDir} to exist`); + const failures = collectAtHomeLines(gsdCoreDir); + assert.deepStrictEqual(failures, [], `#3544 regression — @$HOME/ lines under gsd-core/:\n${failures.map(f => `${f.file}: ${f.line}`).join('\n')}`); + const skillsDir = path.join(claudeGlobal.configDir, 'skills'); + assert.ok(fs.existsSync(skillsDir), `expected ${skillsDir} to exist`); + let sawTilde = false; + for (const file of walk(skillsDir)) { + if (!file.endsWith('.md')) continue; + if (splitLines(fs.readFileSync(file, 'utf8')).some((l) => l.startsWith('@~/'))) sawTilde = true; + } + assert.ok(sawTilde, 'expected at least one skills/ SKILL.md to still carry an @~/ include (#3133 not regressed)'); + }); + + // THE KEY ROW: walk the ENTIRE emitted tree — not an enumerated list of known + // paths — so a new emit path added later is covered automatically by landing + // in the same tree. Reports every offending file so a future failure is + // diagnosable without re-deriving the failing path by hand. + test('row 6 [RED, PARITY] — no file anywhere in the emitted global Claude install tree contains @$HOME/', () => { + const failures = collectAtHomeLines(claudeGlobal.configDir); + assert.deepStrictEqual( + failures, + [], + `found @$HOME/ lines under the entire emitted tree (${claudeGlobal.configDir}):\n` + + failures.map((f) => ` ${path.relative(claudeGlobal.configDir, f.file)}: ${f.line}`).join('\n'), + ); + }); + + test('row 8 [CONTROL] — a project-scoped (non-global) Claude install is unaffected', () => { + const agentsDir = path.join(projectLocal.configDir, 'agents'); + assert.ok(fs.existsSync(agentsDir), `expected ${agentsDir} to exist`); + // Local install's pathPrefix is absolute (not $HOME-form) — restoreClaudeGlobalAtRefTilde + // is a documented no-op for it, and there must be no @$HOME/ or bare @~/.claude leak either. + const failures = []; + for (const file of walk(agentsDir)) { + if (!file.endsWith('.md')) continue; + const content = fs.readFileSync(file, 'utf8'); + for (const line of splitLines(content)) { + if (/^@\$HOME\//.test(line) || /^@~\/\.claude\//.test(line)) failures.push(`${path.relative(agentsDir, file)}: ${line}`); + } + } + assert.deepStrictEqual(failures, [], `local install must not leak @$HOME/ or @~/.claude/:\n${failures.join('\n')}`); + }); +}); diff --git a/tests/path-replacement.test.cjs b/tests/path-replacement.test.cjs index b58a147dd..cf3232b8c 100644 --- a/tests/path-replacement.test.cjs +++ b/tests/path-replacement.test.cjs @@ -650,6 +650,53 @@ describe('#3544: _restoreClaudeGlobalAtRefTilde (spec-tree @-ref restore, unit)' }); }); +// ──────────────────────────────────────────────────────────────────────── +// #3719 GAP 2: restoreClaudeGlobalAtRefTilde used to build the substitution +// with a STRING passed as the 2nd arg to .replace() — `content.replace(re, +// '@' + tildeEquivalent)`. When tildeEquivalent (derived from an +// operator-controlled --config-dir pathPrefix) contains the literal sequence +// `$&` or `` $` ``, the JS engine treats those as REPLACEMENT PATTERNS, not +// literal text: `$&` re-inserts the whole match (duplicating the matched +// `@$HOME/...` span back into the output) and `` $` `` inserts the substring +// before the match (which is empty at position 0, silently dropping the +// literal text that followed it). Measured pre-fix: +// prefix "$HOME/.cl$&ude/" -> "@~/.cl@$HOME/.cl$&ude/ude/x" (corrupted, duplicated) +// prefix "$HOME/.cl$`ude/" -> "@~/.clude/x" (corrupted, text dropped) +// Fixed by passing a FUNCTION as the 2nd arg (`() => '@' + tildeEquivalent`), +// which the engine never pattern-scans. These pathological prefixes are built +// via string concatenation / String.fromCharCode, not template literals, so an +// editor normalizing a literal backtick-dollar sequence can't silently defang +// the fixture. +// ──────────────────────────────────────────────────────────────────────── +describe('#3719 GAP 2: _restoreClaudeGlobalAtRefTilde is immune to $&/$` in pathPrefix', () => { + const { _restoreClaudeGlobalAtRefTilde } = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); + + test('a pathPrefix containing a literal "$&" round-trips to the plain tilde form (not duplicated)', () => { + const dollarAmp = '$' + '&'; + const prefix = '$HOME/.cl' + dollarAmp + 'ude/'; + const src = '@' + prefix + 'x'; + const out = _restoreClaudeGlobalAtRefTilde(src, prefix); + assert.strictEqual(out, '@~/.cl' + dollarAmp + 'ude/x', + 'a literal $& in pathPrefix must not re-insert the whole match into the output'); + }); + + test('a pathPrefix containing a literal "$`" (backtick-dollar) round-trips to the plain tilde form (not dropped)', () => { + const dollarBacktick = '$' + String.fromCharCode(96); + const prefix = '$HOME/.cl' + dollarBacktick + 'ude/'; + const src = '@' + prefix + 'x'; + const out = _restoreClaudeGlobalAtRefTilde(src, prefix); + assert.strictEqual(out, '@~/.cl' + dollarBacktick + 'ude/x', + 'a literal $` in pathPrefix must not be treated as a "text before match" replacement pattern'); + }); + + test('an ordinary pathPrefix (no special replacement chars) is unaffected by the function-replacement fix', () => { + const prefix = '$HOME/.claude/'; + const src = '@' + prefix + 'x'; + const out = _restoreClaudeGlobalAtRefTilde(src, prefix); + assert.strictEqual(out, '@~/.claude/x'); + }); +}); + // Verification boundary (#3544): this suite proves the emitted @-line takes // the `~` STRING form Claude Code documents as expanding. It cannot prove // the host actually resolves it — no automated test can spawn a live Claude @@ -827,3 +874,142 @@ describe('#3544: spawned installer — gsd-core/ spec tree @-refs resolve on til ); }); }); + +// ──────────────────────────────────────────────────────────────────────── +// #3719: applyAgentPathRewrites (the agents/ staging pipeline's pre-converter +// path-rewrite step, install-profiles.cts:964) does the 4 base ~/.claude/ and +// $HOME/.claude/ substitutions but NEVER calls restoreClaudeGlobalAtRefTilde +// — the SAME restore #3133 wired into `_applyRuntimeRewrites`'s 'claude' case +// and #3544 wired into the gsd-core/ spec-tree emit path. Consequence: every +// `agents/gsd-*.md` `@`-include (mandatory-initial-read.md, the +// untrusted-input boundary, the agent-skills bootstrap, etc.) ships as +// `@$HOME/.claude/...` in a global Claude install, which Claude Code does not +// expand — the include silently loads nothing. Unlike #3133/#3544, this is a +// MISSING call, not a wrong regex; fixing it must not touch the ordinary +// $HOME/.claude/ substitutions non-@-anchored text still needs (#1284). +// ──────────────────────────────────────────────────────────────────────── +describe('#3719: applyAgentPathRewrites must restore @-anchored $HOME refs to tilde', () => { + const { applyAgentPathRewrites } = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); + const globalHomePrefix = '$HOME/.claude/'; + + test('row 1 [RED] — an @-include through the agents path emits @~/… not @$HOME/…', () => { + const src = '@~/.claude/gsd-core/references/mandatory-initial-read.md\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, '@~/.claude/gsd-core/references/mandatory-initial-read.md\n'); + }); + + // #3719 review (MINOR 3): the original control string contained no `@` at + // all, so `restoreClaudeGlobalAtRefTilde`'s `atRefRe` (which only matches + // an `@`-anchored sequence) could never match it — the row passed even + // with the quote-lookbehind deleted entirely, proving nothing about + // quote-awareness. A real control needs an `@`-anchored $HOME reference + // INSIDE quotes: `~` does not expand inside double-quoted shell strings + // (#1284), so a quoted `@~/.claude/x` must stay on `@$HOME/.claude/x` + // after the restore pass, not be rewritten back to `@~/`. Confirmed this + // row fails (asserts the wrong string) if the lookbehind + // `(? { + const src = 'echo "@~/.claude/x"\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, 'echo "@$HOME/.claude/x"\n'); + }); + + test('row 7 [CONTROL] — a noPathRewrite runtime (copilot) is untouched', () => { + const src = '@~/.claude/gsd-core/references/mandatory-initial-read.md\n'; + const out = applyAgentPathRewrites(src, 'copilot', globalHomePrefix); + assert.strictEqual(out, src, 'noPathRewrite runtimes must be returned unchanged (no substitutions at all)'); + }); + + // #3719 review (MINOR 2): row 7 (copilot) returns early at the + // `noPathRewrite` check and never reaches the `if (runtime === 'claude')` + // restore guard at all — zero coverage of the guard itself. Cursor and + // Kilo are NOT noPathRewrite, so they exercise the base `.claude/` -> + // pathPrefix substitution AND then hit the guard, which must suppress the + // `@~` restore because they are not `claude`. + test('row 10 [CONTROL] — cursor (non-claude, non-noPathRewrite) keeps @$HOME/.cursor/…, does not restore to tilde', () => { + const src = '@~/.claude/gsd-core/references/mandatory-initial-read.md\n'; + const out = applyAgentPathRewrites(src, 'cursor', '$HOME/.cursor/'); + assert.strictEqual(out, '@$HOME/.cursor/gsd-core/references/mandatory-initial-read.md\n', + 'cursor must keep the @$HOME/.cursor/ form — the runtime guard must suppress the claude-only tilde restore'); + }); + + test('row 11 [CONTROL] — kilo (non-claude, non-noPathRewrite) keeps @$HOME/.kilo/…, does not restore to tilde', () => { + const src = '@~/.claude/gsd-core/references/mandatory-initial-read.md\n'; + const out = applyAgentPathRewrites(src, 'kilo', '$HOME/.kilo/'); + assert.strictEqual(out, '@$HOME/.kilo/gsd-core/references/mandatory-initial-read.md\n', + 'kilo must keep the @$HOME/.kilo/ form — the runtime guard must suppress the claude-only tilde restore'); + }); + + // ── boundary rows ── + test('boundary — leading @~/.claude/x @-include', () => { + const src = '@~/.claude/x\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, '@~/.claude/x\n'); + }); + + test('boundary — mid-sentence @~/.claude/x @-include', () => { + const src = 'See @~/.claude/x for details\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, 'See @~/.claude/x for details\n'); + }); + + test('boundary — ~/.claude/x with NO @ prefix must stay on $HOME (not an @-include)', () => { + const src = 'See ~/.claude/x for details\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, 'See $HOME/.claude/x for details\n'); + }); + + test('boundary — @~/.claude with no trailing slash (word-boundary variant)', () => { + const src = '@~/.claude\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, '@~/.claude\n'); + }); + + test('boundary — @$HOME/.claude/x already present in source is restored, not double-rewritten', () => { + const src = '@$HOME/.claude/x\n'; + const out = applyAgentPathRewrites(src, 'claude', globalHomePrefix); + assert.strictEqual(out, '@~/.claude/x\n'); + }); + + // #3719 GAP 1: `-work-work` doubling guard. The MAJOR fix changed the two + // `~/.claude` / `$HOME/.claude` replaces in this function from a bare `\b` + // to `(?![\w-])`, because `\b` is satisfied by ANY non-word character — + // including '-'. For a --config-dir whose name EXTENDS '.claude' (e.g. + // '.claude-work'), the bare-`\b` regex re-matched the '.claude' PREFIX of + // the already-substituted '.claude-work' path and corrupted it to + // '.claude-work-work'. Security measured 120 doubled paths before the fix + // and 0 after. Nothing else in this file installs under a config dir that + // extends '.claude', so without this block the fix ships unpinned. + describe('-work-work doubling guard (#3719 MAJOR: \\b -> (?![\\w-]))', () => { + const extendedPrefix = '$HOME/.claude-work/'; + + test('@-include: @~/.claude/x.md -> @~/.claude-work/x.md (not .claude-work-work)', () => { + const src = '@~/.claude/x.md'; + const out = applyAgentPathRewrites(src, 'claude', extendedPrefix); + assert.strictEqual(out, '@~/.claude-work/x.md'); + }); + + test('no-trailing-slash variant: @~/.claude -> @~/.claude-work', () => { + const src = '@~/.claude'; + const out = applyAgentPathRewrites(src, 'claude', extendedPrefix); + assert.strictEqual(out, '@~/.claude-work'); + }); + + test('prose (non-@-anchored): See ~/.claude/p.md -> See $HOME/.claude-work/p.md', () => { + const src = 'See ~/.claude/p.md'; + const out = applyAgentPathRewrites(src, 'claude', extendedPrefix); + assert.strictEqual(out, 'See $HOME/.claude-work/p.md'); + }); + + // Different extension character class (word char 'x' immediately after + // '.claude', vs. '-' above) — confirms the lookahead's `[\w-]` class + // covers both. + test('.claudex prefix (word-char extension): @~/.claude/x.md -> @~/.claudex/x.md', () => { + const src = '@~/.claude/x.md'; + const out = applyAgentPathRewrites(src, 'claude', '$HOME/.claudex/'); + assert.strictEqual(out, '@~/.claudex/x.md'); + }); + }); +});