diff --git a/ccw/frontend/src/components/hook/HookQuickTemplates.tsx b/ccw/frontend/src/components/hook/HookQuickTemplates.tsx index 26df4451..966098e6 100644 --- a/ccw/frontend/src/components/hook/HookQuickTemplates.tsx +++ b/ccw/frontend/src/components/hook/HookQuickTemplates.tsx @@ -274,7 +274,8 @@ function getCategoryName(category: TemplateCategory, formatMessage: ReturnType { await onInstallTemplate(templateId); diff --git a/ccw/frontend/src/components/hook/HookWizard.tsx b/ccw/frontend/src/components/hook/HookWizard.tsx index 88a74d66..09a73aeb 100644 --- a/ccw/frontend/src/components/hook/HookWizard.tsx +++ b/ccw/frontend/src/components/hook/HookWizard.tsx @@ -79,12 +79,6 @@ interface SkillContextConfig { // All templates use `ccw hook template exec --stdin` format // This avoids Windows Git Bash quote handling issues -interface HookTemplate { - event: string; - matcher: string; - timeout?: number; -} - // Template IDs that map to backend templates const TEMPLATE_IDS = { 'memory-update-queue': 'memory-auto-compress', @@ -106,72 +100,6 @@ const DANGER_OPTIONS = [ { id: 'permission-change', templateId: 'danger-permission-change', labelKey: 'cliHooks.wizards.dangerProtection.options.permissionChange', descKey: 'cliHooks.wizards.dangerProtection.options.permissionChangeDesc' }, ] as const; -// ========== convertToClaudeCodeFormat (ported from old hook-manager.js) ========== - -function convertToClaudeCodeFormat(hookData: { - command: string; - args: string[]; - matcher?: string; - timeout?: number; -}): Record { - let commandStr = hookData.command || ''; - - if (hookData.args && Array.isArray(hookData.args)) { - if (commandStr === 'bash' && hookData.args.length >= 2 && hookData.args[0] === '-c') { - const script = hookData.args[1]; - const escapedScript = script.replace(/'/g, "'\\''"); - commandStr = `bash -c '${escapedScript}'`; - if (hookData.args.length > 2) { - const additionalArgs = hookData.args.slice(2).map(arg => - arg.includes(' ') && !arg.startsWith('"') && !arg.startsWith("'") - ? `"${arg.replace(/"/g, '\\"')}"` - : arg - ); - commandStr += ' ' + additionalArgs.join(' '); - } - } else if (commandStr === 'node' && hookData.args.length >= 2 && hookData.args[0] === '-e') { - const script = hookData.args[1]; - const isWindows = typeof navigator !== 'undefined' && navigator.userAgent.includes('Win'); - if (isWindows) { - const escapedScript = script.replace(/"/g, '\\"'); - commandStr = `node -e "${escapedScript}"`; - } else { - const escapedScript = script.replace(/'/g, "'\\''"); - commandStr = `node -e '${escapedScript}'`; - } - if (hookData.args.length > 2) { - const additionalArgs = hookData.args.slice(2).map(arg => - arg.includes(' ') && !arg.startsWith('"') && !arg.startsWith("'") - ? `"${arg.replace(/"/g, '\\"')}"` - : arg - ); - commandStr += ' ' + additionalArgs.join(' '); - } - } else { - const quotedArgs = hookData.args.map(arg => - arg.includes(' ') && !arg.startsWith('"') && !arg.startsWith("'") - ? `"${arg.replace(/"/g, '\\"')}"` - : arg - ); - commandStr = `${commandStr} ${quotedArgs.join(' ')}`.trim(); - } - } - - const converted: Record = { - hooks: [{ - type: 'command', - command: commandStr, - ...(hookData.timeout ? { timeout: Math.ceil(hookData.timeout / 1000) } : {}), - }], - }; - - if (hookData.matcher) { - converted.matcher = hookData.matcher; - } - - return converted; -} - // ========== Wizard Definitions ========== const WIZARD_METADATA = { diff --git a/ccw/frontend/src/pages/HookManagerPage.tsx b/ccw/frontend/src/pages/HookManagerPage.tsx index 89e5551f..f64591d5 100644 --- a/ccw/frontend/src/pages/HookManagerPage.tsx +++ b/ccw/frontend/src/pages/HookManagerPage.tsx @@ -28,7 +28,6 @@ import { Card } from '@/components/ui/Card'; import { Badge } from '@/components/ui/Badge'; import { HookCard, HookFormDialog, HookQuickTemplates, HookWizard, type HookCardData, type HookFormData, type HookTriggerType, HOOK_TEMPLATES, type WizardType } from '@/components/hook'; import { useHooks, useToggleHook } from '@/hooks'; -import { installHookTemplate } from '@/lib/api'; import { cn } from '@/lib/utils'; // ========== Types ========== diff --git a/ccw/src/commands/hook.ts b/ccw/src/commands/hook.ts index 9fd3ae3b..abe3c252 100644 --- a/ccw/src/commands/hook.ts +++ b/ccw/src/commands/hook.ts @@ -745,8 +745,8 @@ async function templateAction(subcommand: string, args: string[], options: HookO listTemplatesByCategory, executeTemplate, installTemplateToSettings, - type HookInputData, } = await import('../core/hooks/hook-templates.js'); + type HookInputData = import('../core/hooks/hook-templates.js').HookInputData; switch (subcommand) { case 'list': { diff --git a/ccw/src/core/hooks/hook-templates.ts b/ccw/src/core/hooks/hook-templates.ts index a6fad16a..c49bc6ce 100644 --- a/ccw/src/core/hooks/hook-templates.ts +++ b/ccw/src/core/hooks/hook-templates.ts @@ -12,7 +12,7 @@ import { spawnSync } from 'child_process'; import { existsSync, readFileSync, writeFileSync } from 'fs'; -import { join } from 'path'; +import { join, resolve, basename } from 'path'; import { homedir } from 'os'; // ============================================================================ @@ -79,22 +79,24 @@ export interface HookOutput { // ============================================================================ /** - * Send notification to dashboard via HTTP + * Send notification to dashboard via HTTP (using native fetch) */ function notifyDashboard(type: string, payload: Record): void { - const data = JSON.stringify({ + const data = { type, ...payload, project: process.env.CLAUDE_PROJECT_DIR || process.cwd(), timestamp: Date.now(), - }); + }; - spawnSync('curl', [ - '-s', '-X', 'POST', - '-H', 'Content-Type: application/json', - '-d', data, - 'http://localhost:3456/api/hook' - ], { stdio: 'inherit', shell: true }); + // Use native fetch (Node.js 18+) to avoid shell command injection + fetch('http://localhost:3456/api/hook', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(data), + }).catch(() => { + // Silently ignore errors - dashboard may not be running + }); } /** @@ -144,6 +146,80 @@ function isDangerousGitCommand(cmd: string): boolean { return patterns.some(p => p.test(cmd)); } +/** + * Safely extract string value from unknown input + */ +function getStringInput(value: unknown): string { + if (typeof value === 'string') { + return value; + } + return ''; +} + +/** + * Validate file path to prevent command injection + * Returns null if path is invalid, otherwise returns the sanitized path + */ +function validateFilePath(filePath: string): string | null { + if (!filePath || typeof filePath !== 'string') { + return null; + } + + // Check for dangerous characters that could be used for command injection + const dangerousPatterns = /[;&|`$(){}[\]<>!\\]/; + if (dangerousPatterns.test(filePath)) { + return null; + } + + // Check for path traversal attempts + if (filePath.includes('..')) { + return null; + } + + // Check for null bytes + if (filePath.includes('\0')) { + return null; + } + + return filePath; +} + +/** + * Safe spawnSync wrapper that avoids shell: true to prevent command injection + * On Windows, this uses .cmd extension for npm/npx commands + */ +function safeSpawnSync(command: string, args: string[]): { stdout: string; stderr: string; status: number | null } { + // Use spawnSync without shell to avoid command injection + // Note: On Windows, npx/npm/git may need .cmd extension + const isWindows = process.platform === 'win32'; + const execCommand = isWindows && !command.endsWith('.cmd') && ['npx', 'npm', 'git', 'ccw'].includes(command) + ? `${command}.cmd` + : command; + + return spawnSync(execCommand, args, { + stdio: ['inherit', 'pipe', 'pipe'], + shell: false, + encoding: 'utf8', + cwd: process.cwd(), + }); +} + +/** + * Safe spawnSync with inherited stdio (for tools that need interactive output) + */ +function safeSpawnSyncInherit(command: string, args: string[]): void { + const isWindows = process.platform === 'win32'; + const execCommand = isWindows && !command.endsWith('.cmd') && ['npx', 'npm', 'git', 'ccw'].includes(command) + ? `${command}.cmd` + : command; + + spawnSync(execCommand, args, { + stdio: 'inherit', + shell: false, + cwd: process.cwd(), + }); +} + /** * Check if file is in protected system paths */ @@ -187,8 +263,8 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ trigger: 'PostToolUse', matcher: 'Write|Edit', execute: (data) => { - const file = (data.tool_input?.file_path as string) || ''; - if (/workflow-session\.json$|session-metadata\.json$/.test(file)) { + const file = getStringInput(data.tool_input?.file_path); + if (file && /workflow-session\.json$|session-metadata\.json$/.test(file)) { try { if (existsSync(file)) { const content = readFileSync(file, 'utf8'); @@ -239,9 +315,10 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ trigger: 'PostToolUse', matcher: 'Write|Edit', execute: (data) => { - const file = (data.tool_input?.file_path as string) || ''; + const rawFile = getStringInput(data.tool_input?.file_path); + const file = validateFilePath(rawFile); if (file) { - spawnSync('npx', ['prettier', '--write', file], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('npx', ['prettier', '--write', file]); } return { exitCode: 0 }; } @@ -254,9 +331,10 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ trigger: 'PostToolUse', matcher: 'Write|Edit', execute: (data) => { - const file = (data.tool_input?.file_path as string) || ''; + const rawFile = getStringInput(data.tool_input?.file_path); + const file = validateFilePath(rawFile); if (file) { - spawnSync('npx', ['eslint', '--fix', file], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('npx', ['eslint', '--fix', file]); } return { exitCode: 0 }; } @@ -268,7 +346,7 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ category: 'automation', trigger: 'Stop', execute: () => { - spawnSync('git', ['add', '-u'], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('git', ['add', '-u']); return { exitCode: 0 }; } }, @@ -282,8 +360,8 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ trigger: 'PreToolUse', matcher: 'Write|Edit', execute: (data) => { - const file = (data.tool_input?.file_path as string) || ''; - if (isSensitiveFile(file)) { + const file = getStringInput(data.tool_input?.file_path); + if (file && isSensitiveFile(file)) { return { exitCode: 2, stderr: `Blocked: modifying sensitive file ${file}`, @@ -324,9 +402,9 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ trigger: 'PreToolUse', matcher: 'Write|Edit', execute: (data) => { - const file = (data.tool_input?.file_path as string) || ''; + const file = getStringInput(data.tool_input?.file_path); const protectedPatterns = /\.env|\.git\/|package-lock\.json|yarn\.lock|\.credentials|secrets|id_rsa|\.pem$|\.key$/i; - if (protectedPatterns.test(file)) { + if (file && protectedPatterns.test(file)) { return { exitCode: 2, jsonOutput: { @@ -431,8 +509,8 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ }; } } else { - const file = (data.tool_input?.file_path as string) || ''; - if (isSystemPath(file)) { + const file = getStringInput(data.tool_input?.file_path); + if (file && isSystemPath(file)) { return { exitCode: 2, jsonOutput: { @@ -483,7 +561,7 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ trigger: 'PostToolUse', matcher: 'Write|Edit', execute: (data) => { - const file = (data.tool_input?.file_path as string) || ''; + const file = getStringInput(data.tool_input?.file_path); if (file) { notifyDashboard('FILE_MODIFIED', { file }); } @@ -512,7 +590,7 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ category: 'utility', trigger: 'Stop', execute: () => { - spawnSync('ccw', ['memory', 'consolidate', '--threshold', '50'], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('ccw', ['memory', 'consolidate', '--threshold', '50']); return { exitCode: 0 }; } }, @@ -523,7 +601,7 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ category: 'utility', trigger: 'SessionStart', execute: () => { - spawnSync('ccw', ['memory', 'preview', '--include-native'], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('ccw', ['memory', 'preview', '--include-native']); return { exitCode: 0 }; } }, @@ -534,7 +612,7 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ category: 'utility', trigger: 'SessionStart', execute: () => { - spawnSync('ccw', ['memory', 'status'], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('ccw', ['memory', 'status']); return { exitCode: 0 }; } }, @@ -545,7 +623,7 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ category: 'utility', trigger: 'Stop', execute: () => { - spawnSync('ccw', ['core-memory', 'extract', '--max-sessions', '10'], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('ccw', ['core-memory', 'extract', '--max-sessions', '10']); return { exitCode: 0 }; } }, @@ -556,14 +634,11 @@ export const HOOK_TEMPLATES: HookTemplate[] = [ category: 'utility', trigger: 'Stop', execute: () => { - const result = spawnSync('ccw', ['core-memory', 'extract', '--json'], { - encoding: 'utf8', - shell: true - }); + const result = safeSpawnSync('ccw', ['core-memory', 'extract', '--json']); try { const d = JSON.parse(result.stdout); if (d && d.total_stage1 >= 5) { - spawnSync('ccw', ['core-memory', 'consolidate'], { stdio: 'inherit', shell: true }); + safeSpawnSyncInherit('ccw', ['core-memory', 'consolidate']); } } catch { // Ignore parse errors @@ -681,8 +756,8 @@ export function installTemplateToSettings( } // Check if already installed - const triggerHooks = hooks[template.trigger]; - const alreadyInstalled = triggerHooks.some((h: Record) => + const triggerHooks = hooks[template.trigger] as Array>; + const alreadyInstalled = triggerHooks.some((h) => h._templateId === templateId ); diff --git a/ccw/src/scripts/migrate-hook-templates.ts b/ccw/src/scripts/migrate-hook-templates.ts index 260a72d5..3ede51f7 100644 --- a/ccw/src/scripts/migrate-hook-templates.ts +++ b/ccw/src/scripts/migrate-hook-templates.ts @@ -16,6 +16,7 @@ import { join } from 'path'; interface OldHookEntry { matcher?: string; command?: string; + _templateId?: string; hooks?: Array<{ type?: string; command?: string; @@ -29,56 +30,45 @@ interface Settings { // Command patterns that indicate old-style inline scripts const OLD_PATTERNS = [ - // Bash inline with jq /bash\s+-c.*jq/, - // Node inline with complex scripts /node\s+-e.*child_process/, /node\s+-e.*spawnSync/, - // Long inline commands /command.*node -e ".*\{.*\}.*"/, ]; // Mapping from old patterns to new template IDs const MIGRATION_MAP: Record = { - // Danger protection patterns 'danger-bash-confirm': 'danger-bash-confirm', 'danger-file-protection': 'danger-file-protection', 'danger-git-destructive': 'danger-git-destructive', 'danger-network-confirm': 'danger-network-confirm', 'danger-system-paths': 'danger-system-paths', 'danger-permission-change': 'danger-permission-change', - // Memory patterns 'memory-update-queue': 'memory-auto-compress', 'memory-v2-extract': 'memory-v2-extract', - // Notification patterns 'session-start-notify': 'session-start-notify', 'stop-notify': 'stop-notify', 'session-state-watch': 'session-state-watch', - // Automation patterns 'auto-format-on-write': 'auto-format-on-write', 'auto-lint-on-write': 'auto-lint-on-write', 'block-sensitive-files': 'block-sensitive-files', 'git-auto-stage': 'git-auto-stage', - // Utility patterns + 'post-edit-index': 'post-edit-index', 'memory-preview-extract': 'memory-preview-extract', 'memory-status-check': 'memory-status-check', - 'post-edit-index': 'post-edit-index', }; function detectTemplateFromCommand(command: string): string | null { - // Check for explicit template ID patterns for (const [pattern, templateId] of Object.entries(MIGRATION_MAP)) { if (command.includes(pattern)) { return templateId; } } - // Check for jq usage in bash (indicates old-style danger detection) if (command.includes('jq -r') && command.includes('DANGEROUS_PATTERNS')) { return 'danger-bash-confirm'; } - // Check for curl to localhost:3456 (dashboard notification) if (command.includes('localhost:3456/api/hook')) { if (command.includes('SESSION_CREATED')) return 'session-start-notify'; if (command.includes('TASK_COMPLETED')) return 'stop-notify'; @@ -86,22 +76,18 @@ function detectTemplateFromCommand(command: string): string | null { if (command.includes('SESSION_STATE_CHANGED')) return 'session-state-watch'; } - // Check for prettier if (command.includes('prettier --write')) { return 'auto-format-on-write'; } - // Check for eslint if (command.includes('eslint --fix')) { return 'auto-lint-on-write'; } - // Check for git add if (command.includes('git add -u')) { return 'git-auto-stage'; } - // Check for sensitive file patterns if (command.includes('.env') && command.includes('credential')) { return 'block-sensitive-files'; } @@ -156,9 +142,8 @@ function migrateSettings(settings: Settings, dryRun: boolean): Settings { const migratedEntry = migrateHookEntry(entry, trigger); newEntries.push(migratedEntry); } else { - // Check if already using template approach if (entry.hooks?.[0]?.command?.includes('ccw hook template')) { - console.log(` āœ“ Already using template: ${entry._templateId || 'unknown'}`); + console.log(` āœ“ Already using template: ${(entry as OldHookEntry & { _templateId?: string })._templateId || 'unknown'}`); } newEntries.push(entry); } @@ -179,7 +164,6 @@ async function main(): Promise { if (settingsIndex >= 0 && args[settingsIndex + 1]) { settingsPath = args[settingsIndex + 1]; } else { - // Default to project settings settingsPath = join(process.cwd(), '.claude', 'settings.json'); } @@ -208,12 +192,10 @@ async function main(): Promise { console.log('\nšŸ“„ Migrated settings (dry run):'); console.log(JSON.stringify(migrated, null, 2)); } else { - // Backup original const backupPath = `${settingsPath}.backup-${Date.now()}`; writeFileSync(backupPath, JSON.stringify(settings, null, 2)); console.log(`\nšŸ’¾ Backup saved to: ${backupPath}`); - // Write migrated writeFileSync(settingsPath, JSON.stringify(migrated, null, 2)); console.log(`\nāœ… Settings migrated successfully!`); }