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] 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 `<skill_activation>${calls}.</skill_activation>`;
 }
 
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);
   }