diff --git a/scripts/lib/plugin-paths.js b/scripts/lib/plugin-paths.js index c012bd1c1..a99982340 100644 --- a/scripts/lib/plugin-paths.js +++ b/scripts/lib/plugin-paths.js @@ -33,7 +33,7 @@ export const PLUGIN_SCRIPTS_PATH = '/scripts'; // no variable bound to the loaded plugin root (CLAUDE_PLUGIN_ROOT is // hook-only). The plugin copy drops the rule and script runs go through // the normal Bash confirmation. -export const PROJECT_ALLOWED_TOOLS_LINE = ` - Bash(node ${CLAUDE_PROJECT_SCRIPTS_PATH}/*)\n`; +export const PROJECT_ALLOWED_TOOLS_LINE = ` - Bash(${CLAUDE_PROJECT_SCRIPTS_PATH}/impeccable *)\n`; // Setup step 1's second sentence names the project path as the fallback // when the runtime reports no base directory. A plugin install has no @@ -41,10 +41,10 @@ export const PROJECT_ALLOWED_TOOLS_LINE = ` - Bash(node ${CLAUDE_PROJECT_SCRIPT // fix), and every instruction in the plugin copy already carries the // token, so the sentence loses its fallback clause. const SETUP_FALLBACK_TEXT = - 'That base directory resolves every `node .claude/skills/impeccable/scripts/...` command in this skill and its references, ' + + 'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable ` command in this skill and its references, ' + 'and `.claude/skills/impeccable/scripts` is the fallback only when the runtime reports no base directory.'; const SETUP_PLUGIN_TEXT = - 'Every `node "/scripts/..."` command in this skill and its references resolves against that base directory.'; + 'Every `"/scripts/impeccable" ` command in this skill and its references resolves against that base directory.'; // Agent files are subagent system prompts: a spawned agent never loads // SKILL.md, so Setup's token is undefined in the one @@ -84,7 +84,11 @@ export function rewritePluginMarkdown(content) { // script argument, including the token-form commands SKILL.src.md // carries natively (Setup step 1). Runs after the path replacement so // one pattern covers both origins; already-quoted forms don't match. - .replace(/node \/scripts\/([^\s`"]+)/g, 'node "/scripts/$1"'); + .replace(/node \/scripts\/([^\s`"]+)/g, 'node "/scripts/$1"') + // The engine launcher is the command itself now (`/scripts/impeccable `, + // or `impeccable.cmd` on a Windows shell without sh), so the launcher path + // is what gets quoted; the verb and its arguments follow unquoted. + .replace(/(?\/scripts\/impeccable(\.cmd)?(?=[\s`])/g, '"/scripts/impeccable$1"'); } /** @@ -150,11 +154,11 @@ export function verifyPluginSkillRewrite(skillMdPath) { 'scripts/lib/plugin-paths.js (issue #523); update SETUP_FALLBACK_TEXT to the new wording.', ); } - if (content.includes('Bash(node ')) { + if (/Bash\((?:node |[^)]*scripts\/impeccable)/.test(content)) { throw new Error( - `Plugin rewrite drift: ${skillMdPath} still pre-approves a node script path. ` + + `Plugin rewrite drift: ${skillMdPath} still pre-approves an engine launcher or node script path. ` + "SKILL.src.md's allowed-tools entry no longer matches the removal in " + - 'scripts/lib/plugin-paths.js (issue #523); the plugin ships no node pre-approval.', + 'scripts/lib/plugin-paths.js (issue #523); the plugin ships no launcher pre-approval.', ); } if (content.includes(CLAUDE_PROJECT_SCRIPTS_PATH)) { diff --git a/tests/plugin-paths.test.js b/tests/plugin-paths.test.js index 9d5cdb66f..ad6212822 100644 --- a/tests/plugin-paths.test.js +++ b/tests/plugin-paths.test.js @@ -44,6 +44,16 @@ describe('rewritePluginMarkdown', () => { expect(output).toContain('node "/scripts/live-poll.mjs" --reply EVENT_ID done'); }); + test('quotes the engine launcher path and leaves the verb outside the quotes', () => { + const output = rewritePluginMarkdown( + 'Run `/scripts/impeccable context` once, then `/scripts/impeccable.cmd doctor --json`; ' + + 'already quoted: `"/scripts/impeccable" hooks on`.', + ); + expect(output).toContain('`"/scripts/impeccable" context`'); + expect(output).toContain('`"/scripts/impeccable.cmd" doctor --json`'); + expect(output).not.toContain('""'); + }); + test('quotes commands already in the skill-base-dir form without double-quoting', () => { // SKILL.src.md's Setup step 1 carries the token form natively; a base // directory with spaces splits an unquoted path before node sees it. @@ -60,15 +70,15 @@ describe('rewritePluginMarkdown', () => { const frontmatter = [ 'allowed-tools:', ' - Bash(npx impeccable *)', - ' - Bash(node .claude/skills/impeccable/scripts/*)', + ' - Bash(.claude/skills/impeccable/scripts/impeccable *)', '---', '', ].join('\n'); const output = rewritePluginMarkdown(frontmatter); - // The generic path rewrite alone would leave Bash(node /scripts/*), + // The generic path rewrite alone would leave Bash(/scripts/impeccable *), // a dead literal, and any wildcard replacement would auto-approve // same-shaped paths outside the plugin. The line must go entirely. - expect(output).not.toContain('Bash(node '); + expect(output).not.toContain('scripts/impeccable *'); expect(output).toContain(' - Bash(npx impeccable *)\n---'); }); @@ -76,12 +86,12 @@ describe('rewritePluginMarkdown', () => { const input = '1. Run `node /scripts/context.mjs` once per session, where `` is the ' + "loaded base directory the runtime reports for this skill; keep cwd at the user's project. " + - 'That base directory resolves every `node .claude/skills/impeccable/scripts/...` command in this skill ' + + 'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable ` command in this skill ' + 'and its references, and `.claude/skills/impeccable/scripts` is the fallback only when the runtime ' + 'reports no base directory. Pass a named source file or route as `--target `.'; const output = rewritePluginMarkdown(input); expect(output).toContain( - 'Every `node "/scripts/..."` command in this skill and its references resolves against that base directory.', + 'Every `"/scripts/impeccable" ` command in this skill and its references resolves against that base directory.', ); // The naive rewrite would keep the fallback clause and name the token as // its own fallback for when there is no base directory to resolve it. @@ -247,11 +257,11 @@ describe('verifyPluginSkillRewrite', () => { const goodSkill = [ 'allowed-tools:', - ' - Bash(node .claude/skills/impeccable/scripts/*)', + ' - Bash(.claude/skills/impeccable/scripts/impeccable *)', '', - '1. Run `node /scripts/context.mjs` once per session, where `` is the ' + + '1. Run `/scripts/impeccable context` once per session, where `` is the ' + "loaded base directory the runtime reports for this skill; keep cwd at the user's project. " + - 'That base directory resolves every `node .claude/skills/impeccable/scripts/...` command in this skill ' + + 'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable ` command in this skill ' + 'and its references, and `.claude/skills/impeccable/scripts` is the fallback only when the runtime ' + 'reports no base directory.', ].join('\n'); @@ -269,13 +279,25 @@ describe('verifyPluginSkillRewrite', () => { expect(() => verifyPluginSkillRewrite(p)).toThrow(/Setup step 1 fallback sentence/); }); - test('fails the build when a node pre-approval survives the removal', () => { + test('fails the build when a launcher pre-approval survives the removal', () => { const reworded = goodSkill.replace( - 'Bash(node .claude/skills/impeccable/scripts/*)', - 'Bash(node .claude/skills/impeccable/scripts/**)', + 'Bash(.claude/skills/impeccable/scripts/impeccable *)', + 'Bash(.claude/skills/impeccable/scripts/impeccable.cmd *)', ); const p = writeSkill(rewritePluginMarkdown(reworded)); - expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves a node script path/); + expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher/); + }); + + test('fails the build when a legacy node pre-approval survives', () => { + // The Node-era line is gone from SKILL.src.md, but a copy that still + // carries one must fail the same way as a surviving launcher line. + const p = writeSkill( + rewritePluginMarkdown(goodSkill).replace( + 'allowed-tools:\n', + 'allowed-tools:\n - Bash(node /scripts/*)\n', + ), + ); + expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher or node script path/); }); test('fails the build when the project-relative scripts path survives at all', () => {