diff --git a/src/cli/App.tsx b/src/cli/App.tsx index a8dd90a..271f8f1 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -11678,6 +11678,12 @@ Plan file path: ${planFilePath}`; allowPersistence={ currentApprovalContext?.allowPersistence ?? true } + defaultScope={ + currentApprovalContext?.defaultScope === "user" + ? "session" + : (currentApprovalContext?.defaultScope ?? + "project") + } showPreview={showApprovalPreview} /> ) : ln.kind === "user" ? ( @@ -11757,6 +11763,11 @@ Plan file path: ${planFilePath}`; allowPersistence={ currentApprovalContext?.allowPersistence ?? true } + defaultScope={ + currentApprovalContext?.defaultScope === "user" + ? "session" + : (currentApprovalContext?.defaultScope ?? "project") + } showPreview={showApprovalPreview} /> diff --git a/src/cli/components/ApprovalSwitch.tsx b/src/cli/components/ApprovalSwitch.tsx index 0bffa8c..576192b 100644 --- a/src/cli/components/ApprovalSwitch.tsx +++ b/src/cli/components/ApprovalSwitch.tsx @@ -68,6 +68,7 @@ type Props = { approveAlwaysText?: string; allowPersistence?: boolean; showPreview?: boolean; + defaultScope?: "project" | "session"; // Special handlers for ExitPlanMode onPlanApprove?: (acceptEdits: boolean) => void; @@ -215,6 +216,7 @@ export const ApprovalSwitch = memo( precomputedDiff, allDiffs, showPreview = true, + defaultScope = "project", }: Props) => { const toolName = approval.toolName; @@ -251,6 +253,7 @@ export const ApprovalSwitch = memo( isFocused={isFocused} approveAlwaysText={approveAlwaysText} allowPersistence={allowPersistence} + defaultScope={defaultScope} showPreview={showPreview} /> ); @@ -271,6 +274,7 @@ export const ApprovalSwitch = memo( isFocused={isFocused} approveAlwaysText={approveAlwaysText} allowPersistence={allowPersistence} + defaultScope={defaultScope} showPreview={showPreview} /> ); @@ -340,6 +344,7 @@ export const ApprovalSwitch = memo( isFocused={isFocused} approveAlwaysText={approveAlwaysText} allowPersistence={allowPersistence} + defaultScope={defaultScope} showPreview={showPreview} /> ); diff --git a/src/cli/components/InlineBashApproval.tsx b/src/cli/components/InlineBashApproval.tsx index bf8ee6c..6198008 100644 --- a/src/cli/components/InlineBashApproval.tsx +++ b/src/cli/components/InlineBashApproval.tsx @@ -22,6 +22,7 @@ type Props = { approveAlwaysText?: string; allowPersistence?: boolean; showPreview?: boolean; + defaultScope?: "project" | "session"; }; // Horizontal line character for Claude Code style @@ -44,6 +45,7 @@ export const InlineBashApproval = memo( approveAlwaysText, allowPersistence = true, showPreview = true, + defaultScope = "project", }: Props) => { const [selectedOption, setSelectedOption] = useState(0); const { @@ -107,7 +109,7 @@ export const InlineBashApproval = memo( if (selectedOption === 0) { onApprove(); } else if (selectedOption === 1 && allowPersistence) { - onApproveAlways("project"); + onApproveAlways(defaultScope); } return; } @@ -123,7 +125,7 @@ export const InlineBashApproval = memo( return; } if (input === "2" && allowPersistence) { - onApproveAlways("project"); + onApproveAlways(defaultScope); return; } }, diff --git a/src/cli/components/InlineFileEditApproval.tsx b/src/cli/components/InlineFileEditApproval.tsx index 9b6d7d5..530136b 100644 --- a/src/cli/components/InlineFileEditApproval.tsx +++ b/src/cli/components/InlineFileEditApproval.tsx @@ -45,6 +45,7 @@ type Props = { approveAlwaysText?: string; allowPersistence?: boolean; showPreview?: boolean; + defaultScope?: "project" | "session"; }; // Horizontal line characters for Claude Code style @@ -160,6 +161,7 @@ export const InlineFileEditApproval = memo( approveAlwaysText, allowPersistence = true, showPreview = true, + defaultScope = "project", }: Props) => { const [selectedOption, setSelectedOption] = useState(0); const { @@ -268,7 +270,7 @@ export const InlineFileEditApproval = memo( onApprove(diffsToPass.size > 0 ? diffsToPass : undefined); } else if (selectedOption === 1 && allowPersistence) { onApproveAlways( - "project", + defaultScope, diffsToPass.size > 0 ? diffsToPass : undefined, ); } @@ -286,7 +288,7 @@ export const InlineFileEditApproval = memo( } if (input === "2" && allowPersistence) { onApproveAlways( - "project", + defaultScope, diffsToPass.size > 0 ? diffsToPass : undefined, ); return; diff --git a/src/cli/components/InlineGenericApproval.tsx b/src/cli/components/InlineGenericApproval.tsx index 9e51eb6..21b12f5 100644 --- a/src/cli/components/InlineGenericApproval.tsx +++ b/src/cli/components/InlineGenericApproval.tsx @@ -17,6 +17,7 @@ type Props = { approveAlwaysText?: string; allowPersistence?: boolean; showPreview?: boolean; + defaultScope?: "project" | "session"; }; // Horizontal line character for Claude Code style @@ -58,6 +59,7 @@ export const InlineGenericApproval = memo( approveAlwaysText, allowPersistence = true, showPreview = true, + defaultScope = "project", }: Props) => { const [selectedOption, setSelectedOption] = useState(0); const { @@ -121,7 +123,7 @@ export const InlineGenericApproval = memo( if (selectedOption === 0) { onApprove(); } else if (selectedOption === 1 && allowPersistence) { - onApproveAlways("project"); + onApproveAlways(defaultScope); } return; } @@ -136,7 +138,7 @@ export const InlineGenericApproval = memo( return; } if (input === "2" && allowPersistence) { - onApproveAlways("project"); + onApproveAlways(defaultScope); return; } }, diff --git a/src/permissions/analyzer.ts b/src/permissions/analyzer.ts index 48a2c5a..b364d20 100644 --- a/src/permissions/analyzer.ts +++ b/src/permissions/analyzer.ts @@ -3,6 +3,7 @@ import { homedir } from "node:os"; import { dirname, relative, resolve, win32 } from "node:path"; +import { canonicalToolName, isFileToolName } from "./canonical"; export interface ApprovalContext { // What rule should be saved if user clicks "approve always" @@ -98,29 +99,30 @@ export function analyzeApprovalContext( toolArgs: ToolArgs, workingDirectory: string, ): ApprovalContext { + const canonicalTool = canonicalToolName(toolName); const resolveFilePath = () => { const candidate = toolArgs.file_path ?? toolArgs.path ?? toolArgs.notebook_path ?? ""; return typeof candidate === "string" ? candidate : ""; }; - switch (toolName) { + switch (canonicalTool) { case "Read": - case "read_file": return analyzeReadApproval(resolveFilePath(), workingDirectory); case "Write": return analyzeWriteApproval(resolveFilePath(), workingDirectory); case "Edit": - case "MultiEdit": return analyzeEditApproval(resolveFilePath(), workingDirectory); case "Bash": - case "shell": - case "shell_command": return analyzeBashApproval( - typeof toolArgs.command === "string" ? toolArgs.command : "", + typeof toolArgs.command === "string" + ? toolArgs.command + : Array.isArray(toolArgs.command) + ? toolArgs.command.join(" ") + : "", workingDirectory, ); @@ -131,9 +133,8 @@ export function analyzeApprovalContext( case "Glob": case "Grep": - case "grep_files": return analyzeSearchApproval( - toolName, + canonicalTool, typeof toolArgs.path === "string" ? toolArgs.path : workingDirectory, workingDirectory, ); @@ -150,7 +151,7 @@ export function analyzeApprovalContext( }; default: - return analyzeDefaultApproval(toolName); + return analyzeDefaultApproval(canonicalTool, toolArgs, workingDirectory); } } @@ -280,6 +281,7 @@ const SAFE_READONLY_COMMANDS = [ "diff", "file", "stat", + "curl", ]; // Commands that should never be auto-approved @@ -751,7 +753,41 @@ function analyzeSearchApproval( /** * Default approval for unknown tools */ -function analyzeDefaultApproval(toolName: string): ApprovalContext { +function analyzeDefaultApproval( + toolName: string, + toolArgs: ToolArgs, + workingDir: string, +): ApprovalContext { + if (isFileToolName(toolName)) { + const candidate = + toolArgs.file_path ?? toolArgs.path ?? toolArgs.notebook_path ?? ""; + const filePath = typeof candidate === "string" ? candidate : ""; + if (filePath.trim().length > 0) { + const absolutePath = resolvePathForContext(workingDir, filePath); + if (!isPathWithinDirectory(absolutePath, workingDir)) { + const dirPath = dirnameForContext(absolutePath); + const displayPath = formatDisplayPath(dirPath); + return { + recommendedRule: `${toolName}(${formatAbsoluteRulePath(dirPath)}/**)`, + ruleDescription: `${toolName} in ${displayPath}/`, + approveAlwaysText: `Yes, allow ${toolName} in ${displayPath}/ in this project`, + defaultScope: "project", + allowPersistence: true, + safetyLevel: "moderate", + }; + } + } + + return { + recommendedRule: `${toolName}(**)`, + ruleDescription: `${toolName} operations`, + approveAlwaysText: `Yes, allow ${toolName} operations during this session`, + defaultScope: "session", + allowPersistence: true, + safetyLevel: "moderate", + }; + } + return { recommendedRule: toolName, ruleDescription: `${toolName} operations`, diff --git a/src/permissions/canonical.ts b/src/permissions/canonical.ts new file mode 100644 index 0000000..f4ab658 --- /dev/null +++ b/src/permissions/canonical.ts @@ -0,0 +1,96 @@ +const SHELL_TOOL_NAMES = new Set([ + "Bash", + "shell", + "Shell", + "shell_command", + "ShellCommand", + "run_shell_command", + "RunShellCommand", +]); + +const READ_TOOL_NAMES = new Set([ + "Read", + "read_file", + "ReadFile", + "read_file_gemini", + "ReadFileGemini", +]); + +const WRITE_TOOL_NAMES = new Set([ + "Write", + "write_file", + "WriteFile", + "write_file_gemini", + "WriteFileGemini", +]); + +const EDIT_TOOL_NAMES = new Set([ + "Edit", + "MultiEdit", + "NotebookEdit", + "replace", + "Replace", +]); + +const GLOB_TOOL_NAMES = new Set(["Glob", "glob_gemini", "GlobGemini"]); + +const GREP_TOOL_NAMES = new Set([ + "Grep", + "grep_files", + "GrepFiles", + "search_file_content", + "SearchFileContent", +]); + +const LIST_TOOL_NAMES = new Set([ + "list_dir", + "ListDir", + "list_directory", + "ListDirectory", + "LS", +]); + +const TASK_TOOL_NAMES = new Set(["Task", "task"]); + +const FILE_TOOL_FAMILIES = new Set([ + "Read", + "Write", + "Edit", + "Glob", + "Grep", + "ListDir", +]); + +export function canonicalToolName(toolName: string): string { + if (SHELL_TOOL_NAMES.has(toolName)) return "Bash"; + if (READ_TOOL_NAMES.has(toolName)) return "Read"; + if (WRITE_TOOL_NAMES.has(toolName)) return "Write"; + if (EDIT_TOOL_NAMES.has(toolName)) return "Edit"; + if (GLOB_TOOL_NAMES.has(toolName)) return "Glob"; + if (GREP_TOOL_NAMES.has(toolName)) return "Grep"; + if (LIST_TOOL_NAMES.has(toolName)) return "ListDir"; + if (TASK_TOOL_NAMES.has(toolName)) return "Task"; + return toolName; +} + +export function isShellToolName(toolName: string): boolean { + return canonicalToolName(toolName) === "Bash"; +} + +export function isFileToolName(toolName: string): boolean { + return FILE_TOOL_FAMILIES.has(canonicalToolName(toolName)); +} + +export function canonicalizePathLike(value: string): string { + let normalized = value.replace(/\\/g, "/").trim(); + + if (/^\/+[a-zA-Z]:\//.test(normalized)) { + normalized = normalized.replace(/^\/+/, ""); + } + + if (/^[a-zA-Z]:\//.test(normalized)) { + normalized = `${normalized[0]?.toUpperCase() ?? ""}${normalized.slice(1)}`; + } + + return normalized; +} diff --git a/src/permissions/checker.ts b/src/permissions/checker.ts index 8886981..6e63c90 100644 --- a/src/permissions/checker.ts +++ b/src/permissions/checker.ts @@ -4,6 +4,7 @@ import { resolve } from "node:path"; import { getCurrentAgentId } from "../agent/context"; import { runPermissionRequestHooks } from "../hooks"; +import { canonicalToolName, isShellToolName } from "./canonical"; import { cliPermissions } from "./cli"; import { matchesBashPattern, @@ -22,30 +23,7 @@ import type { /** * Tools that don't require approval within working directory */ -const WORKING_DIRECTORY_TOOLS = [ - // Default/Anthropic toolset - "Read", - "Glob", - "Grep", - // Codex toolset - "read_file", - "ReadFile", - "list_dir", - "ListDir", - "grep_files", - "GrepFiles", - // Gemini toolset - "read_file_gemini", - "ReadFileGemini", - "glob_gemini", - "GlobGemini", - "list_directory", - "ListDirectory", - "search_file_content", - "SearchFileContent", - "read_many_files", - "ReadManyFiles", -]; +const WORKING_DIRECTORY_TOOLS = ["Read", "Glob", "Grep", "ListDir"]; const READ_ONLY_SHELL_TOOLS = new Set([ "Bash", "shell", @@ -83,8 +61,9 @@ export function checkPermission( permissions: PermissionRules, workingDirectory: string = process.cwd(), ): PermissionCheckResult { + const canonicalTool = canonicalToolName(toolName); // Build permission query string - const query = buildPermissionQuery(toolName, toolArgs); + const query = buildPermissionQuery(canonicalTool, toolArgs); // Get session rules const sessionRules = sessionPermissions.getRules(); @@ -155,7 +134,7 @@ export function checkPermission( }; } - if (READ_ONLY_SHELL_TOOLS.has(toolName)) { + if (READ_ONLY_SHELL_TOOLS.has(toolName) || isShellToolName(canonicalTool)) { const shellCommand = extractShellCommand(toolArgs); if (shellCommand && isReadOnlyShellCommand(shellCommand)) { return { @@ -180,7 +159,7 @@ export function checkPermission( } // After checking CLI overrides, check if Read/Glob/Grep within working directory - if (WORKING_DIRECTORY_TOOLS.includes(toolName)) { + if (WORKING_DIRECTORY_TOOLS.includes(canonicalTool)) { const filePath = extractFilePath(toolArgs); if ( filePath && @@ -300,25 +279,7 @@ function buildPermissionQuery(toolName: string, toolArgs: ToolArgs): string { case "Glob": case "Grep": // Codex file tools - case "read_file": - case "ReadFile": - case "list_dir": - case "ListDir": - case "grep_files": - case "GrepFiles": - // Gemini file tools - case "read_file_gemini": - case "ReadFileGemini": - case "write_file_gemini": - case "WriteFileGemini": - case "glob_gemini": - case "GlobGemini": - case "list_directory": - case "ListDirectory": - case "search_file_content": - case "SearchFileContent": - case "read_many_files": - case "ReadManyFiles": { + case "ListDir": { const filePath = extractFilePath(toolArgs); return filePath ? `${toolName}(${filePath})` : toolName; } @@ -330,7 +291,9 @@ function buildPermissionQuery(toolName: string, toolArgs: ToolArgs): string { return `Bash(${command})`; } case "shell": - case "shell_command": { + case "shell_command": + case "run_shell_command": + case "RunShellCommand": { const command = typeof toolArgs.command === "string" ? toolArgs.command @@ -357,34 +320,7 @@ function extractShellCommand(toolArgs: ToolArgs): string | string[] | null { /** * File tools that use glob matching for permissions */ -const FILE_TOOLS = [ - // Default/Anthropic toolset - "Read", - "Write", - "Edit", - "Glob", - "Grep", - // Codex toolset - "read_file", - "ReadFile", - "list_dir", - "ListDir", - "grep_files", - "GrepFiles", - // Gemini toolset - "read_file_gemini", - "ReadFileGemini", - "write_file_gemini", - "WriteFileGemini", - "glob_gemini", - "GlobGemini", - "list_directory", - "ListDirectory", - "search_file_content", - "SearchFileContent", - "read_many_files", - "ReadManyFiles", -]; +const FILE_TOOLS = ["Read", "Write", "Edit", "Glob", "Grep", "ListDir"]; /** * Check if query matches a permission pattern @@ -395,22 +331,19 @@ function matchesPattern( pattern: string, workingDirectory: string, ): boolean { + const canonicalTool = canonicalToolName(toolName); // File tools use glob matching - if (FILE_TOOLS.includes(toolName)) { + if (FILE_TOOLS.includes(canonicalTool)) { return matchesFilePattern(query, pattern, workingDirectory); } // Bash uses prefix matching - if ( - toolName === "Bash" || - toolName === "shell" || - toolName === "shell_command" - ) { + if (canonicalTool === "Bash") { return matchesBashPattern(query, pattern); } // Other tools use simple name matching - return matchesToolPattern(toolName, pattern); + return matchesToolPattern(canonicalTool, pattern); } /** diff --git a/src/permissions/cli.ts b/src/permissions/cli.ts index 1222b1b..3f9badf 100644 --- a/src/permissions/cli.ts +++ b/src/permissions/cli.ts @@ -2,6 +2,13 @@ // CLI-level permission overrides from command-line flags // These take precedence over settings.json but not over enterprise managed policies +import { + canonicalToolName, + isFileToolName, + isShellToolName, +} from "./canonical"; +import { normalizePermissionRule } from "./rule-normalization"; + /** * CLI permission overrides that are set via --allowedTools and --disallowedTools flags. * These rules override settings.json permissions for the current session. @@ -77,24 +84,27 @@ class CliPermissions { * - Tool patterns with parentheses stay as-is */ private normalizePattern(pattern: string): string { + const trimmed = pattern.trim(); + // If pattern has parentheses, keep as-is - if (pattern.includes("(")) { - return pattern; + if (trimmed.includes("(")) { + return normalizePermissionRule(trimmed); } - // Bash without parentheses needs wildcard to match all commands - if (pattern === "Bash") { + const canonicalTool = canonicalToolName(trimmed); + + // Bash/shell aliases without parentheses need wildcard to match all commands + if (isShellToolName(canonicalTool)) { return "Bash(:*)"; } // File tools need wildcard to match all files - const fileTools = ["Read", "Write", "Edit", "Glob", "Grep"]; - if (fileTools.includes(pattern)) { - return `${pattern}(**)`; + if (isFileToolName(canonicalTool)) { + return `${canonicalTool}(**)`; } // All other bare tool names stay as-is - return pattern; + return canonicalTool; } /** diff --git a/src/permissions/loader.ts b/src/permissions/loader.ts index fbd7fb5..f38f9d1 100644 --- a/src/permissions/loader.ts +++ b/src/permissions/loader.ts @@ -4,6 +4,10 @@ import { exists, readFile, writeFile } from "../utils/fs.js"; import { homedir } from "node:os"; import { join } from "node:path"; +import { + normalizePermissionRule, + permissionRulesEquivalent, +} from "./rule-normalization"; import type { PermissionRules } from "./types"; type SettingsFile = { @@ -69,13 +73,13 @@ function mergePermissions( source: PermissionRules, ): void { if (source.allow) { - target.allow = [...(target.allow || []), ...source.allow]; + target.allow = mergeRuleList(target.allow, source.allow); } if (source.deny) { - target.deny = [...(target.deny || []), ...source.deny]; + target.deny = mergeRuleList(target.deny, source.deny); } if (source.ask) { - target.ask = [...(target.ask || []), ...source.ask]; + target.ask = mergeRuleList(target.ask, source.ask); } if (source.additionalDirectories) { target.additionalDirectories = [ @@ -85,6 +89,19 @@ function mergePermissions( } } +function mergeRuleList( + existing: string[] | undefined, + incoming: string[], +): string[] { + const merged = [...(existing || [])]; + for (const rule of incoming) { + if (!merged.some((current) => permissionRulesEquivalent(current, rule))) { + merged.push(rule); + } + } + return merged; +} + /** * Save a permission rule to a specific scope */ @@ -131,9 +148,15 @@ export async function savePermissionRule( settings.permissions[ruleType] = []; } - // Add rule if not already present - if (!settings.permissions[ruleType].includes(rule)) { - settings.permissions[ruleType].push(rule); + const normalizedRule = normalizePermissionRule(rule); + + // Add rule if not already present (canonicalized comparison for alias/path variants) + if ( + !settings.permissions[ruleType].some((existingRule) => + permissionRulesEquivalent(existingRule, normalizedRule), + ) + ) { + settings.permissions[ruleType].push(normalizedRule); } // Save settings diff --git a/src/permissions/matcher.ts b/src/permissions/matcher.ts index 2edc967..f05f2e9 100644 --- a/src/permissions/matcher.ts +++ b/src/permissions/matcher.ts @@ -3,6 +3,7 @@ import { resolve } from "node:path"; import { minimatch } from "minimatch"; +import { canonicalToolName } from "./canonical"; /** * Normalize path separators to forward slashes for consistent glob matching. @@ -115,21 +116,25 @@ export function matchesFilePattern( ): boolean { // Extract tool name and file path from query // Format: "ToolName(filePath)" - const queryMatch = query.match(/^([^(]+)\((.+)\)$/); + const queryMatch = query.match(/^([^(]+)\(([\s\S]+)\)$/); if (!queryMatch || !queryMatch[1] || !queryMatch[2]) { return false; } - const queryTool = queryMatch[1]; + const queryTool = canonicalToolName(queryMatch[1]); // Normalize path separators for cross-platform compatibility const filePath = normalizePath(queryMatch[2]); // Extract tool name and glob pattern from permission rule // Format: "ToolName(pattern)" - const patternMatch = pattern.match(/^([^(]+)\((.+)\)$/); + const patternMatch = pattern.match(/^([^(]+)\(([\s\S]+)\)$/); if (!patternMatch || !patternMatch[1] || !patternMatch[2]) { + // Legacy fallback: allow bare tool names (for rules saved before param suffixes were added) + return canonicalToolName(pattern) === queryTool; + } + const patternTool = canonicalToolName(patternMatch[1]); + if (!patternTool) { return false; } - const patternTool = patternMatch[1]; // Normalize path separators for cross-platform compatibility let globPattern = normalizePath(patternMatch[2]); @@ -220,22 +225,36 @@ function extractActualCommand(command: string): string { export function matchesBashPattern(query: string, pattern: string): boolean { // Extract the command from query - // Format: "Bash(actual command)" or "Bash()" - const queryMatch = query.match(/^Bash\((.*)\)$/); - if (!queryMatch || queryMatch[1] === undefined) { + // Format: "Tool(actual command)" or "Tool()" + const queryMatch = query.match(/^([^(]+)\(([\s\S]*)\)$/); + if ( + !queryMatch || + queryMatch[1] === undefined || + queryMatch[2] === undefined + ) { return false; } - const rawCommand = queryMatch[1]; + if (canonicalToolName(queryMatch[1]) !== "Bash") { + return false; + } + const rawCommand = queryMatch[2]; // Extract actual command by stripping cd prefixes from compound commands const command = extractActualCommand(rawCommand); // Extract the command pattern from permission rule - // Format: "Bash(command pattern)" or "Bash()" - const patternMatch = pattern.match(/^Bash\((.*)\)$/); - if (!patternMatch || patternMatch[1] === undefined) { + // Format: "Tool(command pattern)" or "Tool()" + const patternMatch = pattern.match(/^([^(]+)\(([\s\S]*)\)$/); + if ( + !patternMatch || + patternMatch[1] === undefined || + patternMatch[2] === undefined + ) { + return canonicalToolName(pattern) === "Bash"; + } + if (canonicalToolName(patternMatch[1]) !== "Bash") { return false; } - const commandPattern = patternMatch[1]; + const commandPattern = patternMatch[2]; // Check for wildcard suffix if (commandPattern.endsWith(":*")) { @@ -260,19 +279,25 @@ export function matchesBashPattern(query: string, pattern: string): boolean { * @param pattern - The permission pattern */ export function matchesToolPattern(toolName: string, pattern: string): boolean { + const canonicalTool = canonicalToolName(toolName); // Wildcard matches everything if (pattern === "*") { return true; } + if (canonicalToolName(pattern) === canonicalTool) { + return true; + } + // Check for tool name match (with or without parens) - if (pattern === toolName || pattern === `${toolName}()`) { + if (pattern === canonicalTool || pattern === `${canonicalTool}()`) { return true; } // Check for tool name prefix (e.g., "WebFetch(...)") - if (pattern.startsWith(`${toolName}(`)) { - return true; + const patternToolMatch = pattern.match(/^([^(]+)\(/); + if (patternToolMatch?.[1]) { + return canonicalToolName(patternToolMatch[1]) === canonicalTool; } return false; diff --git a/src/permissions/rule-normalization.ts b/src/permissions/rule-normalization.ts new file mode 100644 index 0000000..7cab693 --- /dev/null +++ b/src/permissions/rule-normalization.ts @@ -0,0 +1,44 @@ +import { + canonicalizePathLike, + canonicalToolName, + isFileToolName, + isShellToolName, +} from "./canonical"; + +function splitRule(rule: string): { tool: string; payload: string | null } { + const match = rule.trim().match(/^([^(]+)(?:\(([\s\S]*)\))?$/); + if (!match?.[1]) { + return { tool: rule.trim(), payload: null }; + } + + return { + tool: match[1].trim(), + payload: match[2] !== undefined ? match[2] : null, + }; +} + +export function normalizePermissionRule(rule: string): string { + const { tool, payload } = splitRule(rule); + const canonicalTool = canonicalToolName(tool); + + if (payload === null) { + return canonicalTool; + } + + if (isShellToolName(canonicalTool)) { + return `Bash(${payload.trim()})`; + } + + if (isFileToolName(canonicalTool)) { + return `${canonicalTool}(${canonicalizePathLike(payload)})`; + } + + return `${canonicalTool}(${payload.trim()})`; +} + +export function permissionRulesEquivalent( + left: string, + right: string, +): boolean { + return normalizePermissionRule(left) === normalizePermissionRule(right); +} diff --git a/src/tests/permissions-analyzer.test.ts b/src/tests/permissions-analyzer.test.ts index dc8f9cd..5d76197 100644 --- a/src/tests/permissions-analyzer.test.ts +++ b/src/tests/permissions-analyzer.test.ts @@ -564,3 +564,24 @@ test("Very long non-git commands should generate prefix-based wildcards", () => expect(context.recommendedRule).toBe("Bash(npm run lint:*)"); expect(context.approveAlwaysText).toContain("npm run lint"); }); + +test("WriteFileGemini uses write-family wildcard rule", () => { + const context = analyzeApprovalContext( + "WriteFileGemini", + { file_path: "src/main.ts", content: "console.log('hi');" }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toBe("Write(**)"); + expect(context.defaultScope).toBe("session"); +}); + +test("run_shell_command is analyzed as Bash", () => { + const context = analyzeApprovalContext( + "run_shell_command", + { command: "curl -s http://localhost:4321/intro" }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toBe("Bash(curl:*)"); +}); diff --git a/src/tests/permissions-checker.test.ts b/src/tests/permissions-checker.test.ts index 35f730b..575f828 100644 --- a/src/tests/permissions-checker.test.ts +++ b/src/tests/permissions-checker.test.ts @@ -641,3 +641,39 @@ test("Tool with alternative path parameter (Glob uses 'path' not 'file_path')", expect(result.decision).toBe("allow"); }); + +test("Shell alias tools match Bash permission patterns", () => { + const permissions: PermissionRules = { + allow: ["Bash(curl:*)"], + deny: [], + ask: [], + }; + + const result = checkPermission( + "run_shell_command", + { command: "curl -s http://localhost:4321/health" }, + permissions, + "/Users/test/project", + ); + + expect(result.decision).toBe("allow"); + expect(result.matchedRule).toBe("Bash(curl:*)"); +}); + +test("Legacy bare WriteFileGemini rule still matches write invocations", () => { + const permissions: PermissionRules = { + allow: ["WriteFileGemini"], + deny: [], + ask: [], + }; + + const result = checkPermission( + "WriteFileGemini", + { file_path: "src/main.ts", content: "console.log('x');" }, + permissions, + "/Users/test/project", + ); + + expect(result.decision).toBe("allow"); + expect(result.matchedRule).toBe("WriteFileGemini"); +}); diff --git a/src/tests/permissions-cli.test.ts b/src/tests/permissions-cli.test.ts index eb4242f..9992a06 100644 --- a/src/tests/permissions-cli.test.ts +++ b/src/tests/permissions-cli.test.ts @@ -399,3 +399,19 @@ test("Precedence: CLI allowedTools > settings allow", () => { expect(dockerResult.decision).toBe("allow"); expect(dockerResult.matchedRule).toBe("Bash(docker:*)"); }); + +test("CLI allowedTools normalizes shell aliases to Bash wildcard", () => { + cliPermissions.clear(); + cliPermissions.setAllowedTools("run_shell_command"); + + const tools = cliPermissions.getAllowedTools(); + expect(tools).toEqual(["Bash(:*)"]); +}); + +test("CLI allowedTools normalizes file alias family", () => { + cliPermissions.clear(); + cliPermissions.setAllowedTools("WriteFileGemini"); + + const tools = cliPermissions.getAllowedTools(); + expect(tools).toEqual(["Write(**)"]); +}); diff --git a/src/tests/permissions-loader.test.ts b/src/tests/permissions-loader.test.ts index ef3701f..6b92aba 100644 --- a/src/tests/permissions-loader.test.ts +++ b/src/tests/permissions-loader.test.ts @@ -368,3 +368,59 @@ test("Saving local settings creates .gitignore if missing", async () => { const content = await gitignoreFile.text(); expect(content).toContain(".letta/settings.local.json"); }); + +test("Save permission dedupes canonical shell aliases", async () => { + const projectDir = join(testDir, "project"); + await savePermissionRule( + "run_shell_command(curl -s http://localhost:4321/intro)", + "allow", + "project", + projectDir, + ); + await savePermissionRule( + "Bash(curl -s http://localhost:4321/intro)", + "allow", + "project", + projectDir, + ); + + const settingsPath = join(projectDir, ".letta", "settings.json"); + const file = Bun.file(settingsPath); + const settings = await file.json(); + + expect(settings.permissions.allow).toContain( + "Bash(curl -s http://localhost:4321/intro)", + ); + expect( + settings.permissions.allow.filter( + (r: string) => r === "Bash(curl -s http://localhost:4321/intro)", + ), + ).toHaveLength(1); +}); + +test("Save permission dedupes slash variants for file patterns", async () => { + const projectDir = join(testDir, "project"); + await savePermissionRule( + "Edit(.skills\\skilled-mcp\\**)", + "allow", + "project", + projectDir, + ); + await savePermissionRule( + "Edit(.skills/skilled-mcp/**)", + "allow", + "project", + projectDir, + ); + + const settingsPath = join(projectDir, ".letta", "settings.json"); + const file = Bun.file(settingsPath); + const settings = await file.json(); + + expect(settings.permissions.allow).toContain("Edit(.skills/skilled-mcp/**)"); + expect( + settings.permissions.allow.filter( + (r: string) => r === "Edit(.skills/skilled-mcp/**)", + ), + ).toHaveLength(1); +}); diff --git a/src/tests/permissions-matcher.test.ts b/src/tests/permissions-matcher.test.ts index 6dbb3dd..88df031 100644 --- a/src/tests/permissions-matcher.test.ts +++ b/src/tests/permissions-matcher.test.ts @@ -419,3 +419,10 @@ test("File pattern: extended UNC pattern matches UNC query path", () => { ), ).toBe(true); }); + +test("Bash pattern: multiline command rules match", () => { + const pattern = `Bash(curl -s http://localhost:4321/intro 2>/dev/null | grep -o\n'class="[^"]*"' | sort -u:*)`; + const query = `Bash(curl -s http://localhost:4321/intro 2>/dev/null | grep -o\n'class="[^"]*"' | sort -u | head -20)`; + + expect(matchesBashPattern(query, pattern)).toBe(true); +}); diff --git a/src/tests/permissions-mode.test.ts b/src/tests/permissions-mode.test.ts index f973d5a..afa3149 100644 --- a/src/tests/permissions-mode.test.ts +++ b/src/tests/permissions-mode.test.ts @@ -469,3 +469,23 @@ test("plan mode - remembers and restores previous mode", () => { // Once we leave plan mode, the remembered mode is consumed. expect(permissionMode.getModeBeforePlan()).toBe(null); }); + +test("plan mode - allows read_file_gemini", () => { + permissionMode.setMode("plan"); + + const permissions: PermissionRules = { + allow: [], + deny: [], + ask: [], + }; + + const result = checkPermission( + "read_file_gemini", + { file_path: "/tmp/test.txt" }, + permissions, + "/Users/test/project", + ); + + expect(result.decision).toBe("allow"); + expect(result.matchedRule).toBe("plan mode"); +});