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.
This commit is contained in:
mastertyko 2026-03-24 18:09:55 +01:00
parent 5a64da32d3
commit e9c8994174
2 changed files with 33 additions and 8 deletions

View file

@ -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>`;
}

View file

@ -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);
}