From 006184456a4039342dc7facdfb30acfd1911f22f Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Mon, 23 Mar 2026 18:46:44 +0100 Subject: [PATCH 1/3] fix(gsd): use explicit parameter syntax in skill activation prompts The skill activation block used positional-looking syntax `Call Skill('name')` which caused LLMs (especially non-Anthropic models) to pass `{name: "..."}` instead of the required `{skill: "..."}` parameter. This triggered tool validation failures and stuck dispatch loops in auto-mode. Change the prompt template to `Call Skill({ skill: 'name' })` which makes the parameter name explicit and matches the Skill tool schema. Update all 4 affected test assertions to match the new format. Closes #2224 --- src/resources/extensions/gsd/auto-prompts.ts | 4 +++- .../extensions/gsd/tests/skill-activation.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/resources/extensions/gsd/auto-prompts.ts b/src/resources/extensions/gsd/auto-prompts.ts index d683102dc..102aebb63 100644 --- a/src/resources/extensions/gsd/auto-prompts.ts +++ b/src/resources/extensions/gsd/auto-prompts.ts @@ -421,7 +421,9 @@ function resolvePreferredSkillNames( function formatSkillActivationBlock(skillNames: string[]): string { if (skillNames.length === 0) return ""; - const calls = skillNames.map(name => `Call Skill('${name}')`).join('. '); + // Use explicit parameter syntax so LLMs pass { skill: "..." } instead of { name: "..." }. + // Positional-looking `Skill('name')` caused validation failures — see #2224. + const calls = skillNames.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..312c078bf 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); } From 5a64da32d336b015713082f139511af50d059987 Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Mon, 23 Mar 2026 20:17:01 +0100 Subject: [PATCH 2/3] review: clarify comment wording, add special-character test Address review feedback: - Update comment to clarify that the function-call-like syntax led LLMs to infer a positional parameter name (not 'positional-looking') - Add test documenting current behavior when skill names contain special characters (quotes, apostrophes) --- src/resources/extensions/gsd/auto-prompts.ts | 3 ++- .../gsd/tests/skill-activation.test.ts | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/resources/extensions/gsd/auto-prompts.ts b/src/resources/extensions/gsd/auto-prompts.ts index 102aebb63..f06bca4da 100644 --- a/src/resources/extensions/gsd/auto-prompts.ts +++ b/src/resources/extensions/gsd/auto-prompts.ts @@ -422,7 +422,8 @@ function resolvePreferredSkillNames( function formatSkillActivationBlock(skillNames: string[]): string { if (skillNames.length === 0) return ""; // Use explicit parameter syntax so LLMs pass { skill: "..." } instead of { name: "..." }. - // Positional-looking `Skill('name')` caused validation failures — see #2224. + // The function-call-like syntax `Skill('name')` led LLMs to infer a positional + // parameter name, causing tool validation failures — see #2224. const calls = skillNames.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 312c078bf..064b68f5c 100644 --- a/src/resources/extensions/gsd/tests/skill-activation.test.ts +++ b/src/resources/extensions/gsd/tests/skill-activation.test.ts @@ -191,3 +191,23 @@ test("buildSkillActivationBlock does not activate skills from extraContext or ta cleanup(base); } }); + +test("buildSkillActivationBlock handles skill names with special characters safely", () => { + const base = makeTempBase(); + try { + // Skill names come from directory names — test that quotes/braces don't break the template + writeSkill(base, "my-skill's", "Skill with apostrophe in name."); + loadOnlyTestSkills(base); + + const result = buildBlock(base, {}, { + always_use_skills: ["my-skill's"], + }); + + // The skill name is interpolated as-is — this documents current behavior. + // A future guard (e.g. /^[a-z0-9-]+$/) could reject such names. + assert.match(result, /skill_activation/); + assert.match(result, /my-skill's/); + } finally { + cleanup(base); + } +}); From e9c89941743de29fc912b2c64d125fa29f03ab75 Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:09:55 +0100 Subject: [PATCH 3/3] fix: add SAFE_SKILL_NAME guard to reject prompt-injection via crafted skill names Adds /^[a-z0-9][a-z0-9-]*$/ validation in formatSkillActivationBlock() so that skill names containing quotes, braces, or other special characters are silently filtered out before interpolation into the prompt string. Addresses the prompt injection surface noted by @trek-e in PR review. Updates the special-character test to verify rejection instead of passthrough. --- src/resources/extensions/gsd/auto-prompts.ts | 9 ++++-- .../gsd/tests/skill-activation.test.ts | 32 +++++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/resources/extensions/gsd/auto-prompts.ts b/src/resources/extensions/gsd/auto-prompts.ts index f06bca4da..e8136371d 100644 --- a/src/resources/extensions/gsd/auto-prompts.ts +++ b/src/resources/extensions/gsd/auto-prompts.ts @@ -419,12 +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 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 = skillNames.map(name => `Call Skill({ skill: '${name}' })`).join('. '); + 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 064b68f5c..f02310935 100644 --- a/src/resources/extensions/gsd/tests/skill-activation.test.ts +++ b/src/resources/extensions/gsd/tests/skill-activation.test.ts @@ -192,10 +192,11 @@ test("buildSkillActivationBlock does not activate skills from extraContext or ta } }); -test("buildSkillActivationBlock handles skill names with special characters safely", () => { +test("buildSkillActivationBlock rejects skill names with special characters", () => { const base = makeTempBase(); try { - // Skill names come from directory names — test that quotes/braces don't break the template + // 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); @@ -203,10 +204,29 @@ test("buildSkillActivationBlock handles skill names with special characters safe always_use_skills: ["my-skill's"], }); - // The skill name is interpolated as-is — this documents current behavior. - // A future guard (e.g. /^[a-z0-9-]+$/) could reject such names. - assert.match(result, /skill_activation/); - assert.match(result, /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); }