From 59868a9404b840ad974ce190c7b72cc77493e51f Mon Sep 17 00:00:00 2001 From: Wen Date: Wed, 14 Oct 2026 16:42:17 -0300 Subject: [PATCH] fix(ck): escape the skill dir for bash when SessionStart fills it in Signed-off-by: Wen (cherry picked from commit 00a03ab84fc7960143ecf7568cdf975e15b3de37) --- skills/ck/hooks/session-start.mjs | 5 ++++- tests/skills/ck-paths.test.js | 26 +++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/skills/ck/hooks/session-start.mjs b/skills/ck/hooks/session-start.mjs index 0ef9cd2fa..47d3623b7 100644 --- a/skills/ck/hooks/session-start.mjs +++ b/skills/ck/hooks/session-start.mjs @@ -89,8 +89,11 @@ function main() { // Load skill (always inject — now only ~50 lines) // The host does not expand ${CLAUDE_SKILL_DIR} in hook output, so fill it in here. + // SKILL.md uses it inside double-quoted bash, so escape the characters bash + // still interprets there; the callback keeps `$&` etc. in the path literal. + const shellSkillDir = SKILL_DIR.replace(/[\\$`"]/g, '\\$&'); const skill = existsSync(SKILL_FILE) - ? readFileSync(SKILL_FILE, 'utf8').replaceAll('${CLAUDE_SKILL_DIR}', SKILL_DIR) + ? readFileSync(SKILL_FILE, 'utf8').replaceAll('${CLAUDE_SKILL_DIR}', () => shellSkillDir) : ''; const projects = readJson(PROJECTS_FILE) || {}; diff --git a/tests/skills/ck-paths.test.js b/tests/skills/ck-paths.test.js index f9685e879..9ab5d88d4 100644 --- a/tests/skills/ck-paths.test.js +++ b/tests/skills/ck-paths.test.js @@ -78,7 +78,31 @@ async function runTests() { const { additionalContext } = JSON.parse(result.stdout); assert.ok(additionalContext.includes('# ck — Context Keeper'), 'SKILL.md was not injected'); assert.ok(!additionalContext.includes('${CLAUDE_SKILL_DIR}'), 'skill dir placeholder left unexpanded'); - assert.ok(additionalContext.includes(path.join(SKILL_DIR, 'commands')), 'skill dir not substituted'); + // Escaped for double-quoted bash (Windows backslashes are doubled). + const shellCommandsDir = path.join(SKILL_DIR, 'commands').replace(/[\\$`"]/g, '\\$&'); + assert.ok(additionalContext.includes(shellCommandsDir), 'skill dir not substituted'); + } finally { + fs.rmSync(ctx.root, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (process.platform === 'win32') { + console.log(' - skipped shell-special skill dir test on Windows'); + } else if (await test('hook-injected commands run from a skill dir with shell-special characters', async () => { + const ctx = tempEnv(); + try { + const skillDir = path.join(ctx.root, 'skills $USER $& `x` "q"', 'ck'); + fs.cpSync(SKILL_DIR, skillDir, { recursive: true }); + const result = spawnSync(process.execPath, [path.join(skillDir, 'hooks', 'session-start.mjs')], { + cwd: ctx.project, env: ctx.env, input: '{}', encoding: 'utf8', + }); + assert.strictEqual(result.status, 0, result.stderr); + const { additionalContext } = JSON.parse(result.stdout); + const listCommand = additionalContext.split('\n').find(line => line.includes('commands/list.mjs')); + assert.ok(listCommand, 'no list.mjs command injected'); + const list = spawnSync('bash', ['-c', listCommand], { cwd: ctx.project, env: ctx.env, encoding: 'utf8' }); + // No projects are registered, so list.mjs reaching its own message proves the path resolved. + assert.ok(list.stdout.includes('No projects registered'), `${listCommand}\n${list.stdout}${list.stderr}`); } finally { fs.rmSync(ctx.root, { recursive: true, force: true }); }