diff --git a/src/resources/extensions/gsd/auto-prompts.ts b/src/resources/extensions/gsd/auto-prompts.ts index b710154f0..876e68cb8 100644 --- a/src/resources/extensions/gsd/auto-prompts.ts +++ b/src/resources/extensions/gsd/auto-prompts.ts @@ -419,9 +419,17 @@ function resolvePreferredSkillNames( .map(skill => normalizeSkillReference(skill.name)); } +/** Skill names must be lowercase alphanumeric with hyphens — reject anything else + * to prevent prompt injection via crafted directory names. */ +const SAFE_SKILL_NAME = /^[a-z0-9][a-z0-9-]*$/; + function formatSkillActivationBlock(skillNames: string[]): string { - if (skillNames.length === 0) return ""; - const calls = skillNames.map(name => `Call Skill('${name}')`).join('. '); + const safe = skillNames.filter(name => SAFE_SKILL_NAME.test(name)); + if (safe.length === 0) return ""; + // Use explicit parameter syntax so LLMs pass { skill: "..." } instead of { name: "..." }. + // The function-call-like syntax `Skill('name')` led LLMs to infer a positional + // parameter name, causing tool validation failures — see #2224. + const calls = safe.map(name => `Call Skill({ skill: '${name}' })`).join('. '); return `${calls}.`; } diff --git a/src/resources/extensions/gsd/tests/skill-activation.test.ts b/src/resources/extensions/gsd/tests/skill-activation.test.ts index 673e8911c..f02310935 100644 --- a/src/resources/extensions/gsd/tests/skill-activation.test.ts +++ b/src/resources/extensions/gsd/tests/skill-activation.test.ts @@ -75,7 +75,7 @@ test("buildSkillActivationBlock activates skills via prefer_skills when context prefer_skills: ["react"], }); - assert.match(result, /Call Skill\('react'\)/); + assert.match(result, /Call Skill\(\{ skill: 'react' \}\)/); assert.doesNotMatch(result, /swiftui/); } finally { cleanup(base); @@ -92,7 +92,7 @@ test("buildSkillActivationBlock includes always_use_skills from preferences usin always_use_skills: ["swift-testing"], }); - assert.equal(result, "Call Skill('swift-testing')."); + assert.equal(result, "Call Skill({ skill: 'swift-testing' })."); } finally { cleanup(base); } @@ -120,8 +120,8 @@ test("buildSkillActivationBlock includes skill_rules matches and task-plan skill skill_rules: [{ when: "prisma database schema", use: ["prisma"] }], }); - assert.match(result, /Call Skill\('accessibility'\)/); - assert.match(result, /Call Skill\('prisma'\)/); + assert.match(result, /Call Skill\(\{ skill: 'accessibility' \}\)/); + assert.match(result, /Call Skill\(\{ skill: 'prisma' \}\)/); } finally { cleanup(base); } @@ -191,3 +191,43 @@ test("buildSkillActivationBlock does not activate skills from extraContext or ta cleanup(base); } }); + +test("buildSkillActivationBlock rejects skill names with special characters", () => { + const base = makeTempBase(); + try { + // Skill names with quotes, braces, or other non-alphanumeric characters are + // rejected by the SAFE_SKILL_NAME guard to prevent prompt injection. + writeSkill(base, "my-skill's", "Skill with apostrophe in name."); + loadOnlyTestSkills(base); + + const result = buildBlock(base, {}, { + always_use_skills: ["my-skill's"], + }); + + // Unsafe skill name is filtered out — empty result + assert.equal(result, ""); + } finally { + cleanup(base); + } +}); + +test("buildSkillActivationBlock allows valid skill names and rejects invalid ones", () => { + const base = makeTempBase(); + try { + writeSkill(base, "react", "React skill."); + writeSkill(base, "bad'name", "Injection attempt."); + writeSkill(base, "good-skill-2", "Another valid skill."); + loadOnlyTestSkills(base); + + const result = buildBlock(base, {}, { + always_use_skills: ["react", "bad'name", "good-skill-2"], + }); + + assert.match(result, /skill_activation/); + assert.match(result, /Call Skill\(\{ skill: 'react' \}\)/); + assert.match(result, /Call Skill\(\{ skill: 'good-skill-2' \}\)/); + assert.doesNotMatch(result, /bad'name/); + } finally { + cleanup(base); + } +});