fix: harden permissions matching and alias caching (#1027)

This commit is contained in:
Charles Packer
2026-02-18 19:49:53 -08:00
committed by GitHub
parent 92d2955035
commit dc25ce5573
18 changed files with 472 additions and 127 deletions

View File

@@ -11678,6 +11678,12 @@ Plan file path: ${planFilePath}`;
allowPersistence={ allowPersistence={
currentApprovalContext?.allowPersistence ?? true currentApprovalContext?.allowPersistence ?? true
} }
defaultScope={
currentApprovalContext?.defaultScope === "user"
? "session"
: (currentApprovalContext?.defaultScope ??
"project")
}
showPreview={showApprovalPreview} showPreview={showApprovalPreview}
/> />
) : ln.kind === "user" ? ( ) : ln.kind === "user" ? (
@@ -11757,6 +11763,11 @@ Plan file path: ${planFilePath}`;
allowPersistence={ allowPersistence={
currentApprovalContext?.allowPersistence ?? true currentApprovalContext?.allowPersistence ?? true
} }
defaultScope={
currentApprovalContext?.defaultScope === "user"
? "session"
: (currentApprovalContext?.defaultScope ?? "project")
}
showPreview={showApprovalPreview} showPreview={showApprovalPreview}
/> />
</Box> </Box>

View File

@@ -68,6 +68,7 @@ type Props = {
approveAlwaysText?: string; approveAlwaysText?: string;
allowPersistence?: boolean; allowPersistence?: boolean;
showPreview?: boolean; showPreview?: boolean;
defaultScope?: "project" | "session";
// Special handlers for ExitPlanMode // Special handlers for ExitPlanMode
onPlanApprove?: (acceptEdits: boolean) => void; onPlanApprove?: (acceptEdits: boolean) => void;
@@ -215,6 +216,7 @@ export const ApprovalSwitch = memo(
precomputedDiff, precomputedDiff,
allDiffs, allDiffs,
showPreview = true, showPreview = true,
defaultScope = "project",
}: Props) => { }: Props) => {
const toolName = approval.toolName; const toolName = approval.toolName;
@@ -251,6 +253,7 @@ export const ApprovalSwitch = memo(
isFocused={isFocused} isFocused={isFocused}
approveAlwaysText={approveAlwaysText} approveAlwaysText={approveAlwaysText}
allowPersistence={allowPersistence} allowPersistence={allowPersistence}
defaultScope={defaultScope}
showPreview={showPreview} showPreview={showPreview}
/> />
); );
@@ -271,6 +274,7 @@ export const ApprovalSwitch = memo(
isFocused={isFocused} isFocused={isFocused}
approveAlwaysText={approveAlwaysText} approveAlwaysText={approveAlwaysText}
allowPersistence={allowPersistence} allowPersistence={allowPersistence}
defaultScope={defaultScope}
showPreview={showPreview} showPreview={showPreview}
/> />
); );
@@ -340,6 +344,7 @@ export const ApprovalSwitch = memo(
isFocused={isFocused} isFocused={isFocused}
approveAlwaysText={approveAlwaysText} approveAlwaysText={approveAlwaysText}
allowPersistence={allowPersistence} allowPersistence={allowPersistence}
defaultScope={defaultScope}
showPreview={showPreview} showPreview={showPreview}
/> />
); );

View File

@@ -22,6 +22,7 @@ type Props = {
approveAlwaysText?: string; approveAlwaysText?: string;
allowPersistence?: boolean; allowPersistence?: boolean;
showPreview?: boolean; showPreview?: boolean;
defaultScope?: "project" | "session";
}; };
// Horizontal line character for Claude Code style // Horizontal line character for Claude Code style
@@ -44,6 +45,7 @@ export const InlineBashApproval = memo(
approveAlwaysText, approveAlwaysText,
allowPersistence = true, allowPersistence = true,
showPreview = true, showPreview = true,
defaultScope = "project",
}: Props) => { }: Props) => {
const [selectedOption, setSelectedOption] = useState(0); const [selectedOption, setSelectedOption] = useState(0);
const { const {
@@ -107,7 +109,7 @@ export const InlineBashApproval = memo(
if (selectedOption === 0) { if (selectedOption === 0) {
onApprove(); onApprove();
} else if (selectedOption === 1 && allowPersistence) { } else if (selectedOption === 1 && allowPersistence) {
onApproveAlways("project"); onApproveAlways(defaultScope);
} }
return; return;
} }
@@ -123,7 +125,7 @@ export const InlineBashApproval = memo(
return; return;
} }
if (input === "2" && allowPersistence) { if (input === "2" && allowPersistence) {
onApproveAlways("project"); onApproveAlways(defaultScope);
return; return;
} }
}, },

View File

@@ -45,6 +45,7 @@ type Props = {
approveAlwaysText?: string; approveAlwaysText?: string;
allowPersistence?: boolean; allowPersistence?: boolean;
showPreview?: boolean; showPreview?: boolean;
defaultScope?: "project" | "session";
}; };
// Horizontal line characters for Claude Code style // Horizontal line characters for Claude Code style
@@ -160,6 +161,7 @@ export const InlineFileEditApproval = memo(
approveAlwaysText, approveAlwaysText,
allowPersistence = true, allowPersistence = true,
showPreview = true, showPreview = true,
defaultScope = "project",
}: Props) => { }: Props) => {
const [selectedOption, setSelectedOption] = useState(0); const [selectedOption, setSelectedOption] = useState(0);
const { const {
@@ -268,7 +270,7 @@ export const InlineFileEditApproval = memo(
onApprove(diffsToPass.size > 0 ? diffsToPass : undefined); onApprove(diffsToPass.size > 0 ? diffsToPass : undefined);
} else if (selectedOption === 1 && allowPersistence) { } else if (selectedOption === 1 && allowPersistence) {
onApproveAlways( onApproveAlways(
"project", defaultScope,
diffsToPass.size > 0 ? diffsToPass : undefined, diffsToPass.size > 0 ? diffsToPass : undefined,
); );
} }
@@ -286,7 +288,7 @@ export const InlineFileEditApproval = memo(
} }
if (input === "2" && allowPersistence) { if (input === "2" && allowPersistence) {
onApproveAlways( onApproveAlways(
"project", defaultScope,
diffsToPass.size > 0 ? diffsToPass : undefined, diffsToPass.size > 0 ? diffsToPass : undefined,
); );
return; return;

View File

@@ -17,6 +17,7 @@ type Props = {
approveAlwaysText?: string; approveAlwaysText?: string;
allowPersistence?: boolean; allowPersistence?: boolean;
showPreview?: boolean; showPreview?: boolean;
defaultScope?: "project" | "session";
}; };
// Horizontal line character for Claude Code style // Horizontal line character for Claude Code style
@@ -58,6 +59,7 @@ export const InlineGenericApproval = memo(
approveAlwaysText, approveAlwaysText,
allowPersistence = true, allowPersistence = true,
showPreview = true, showPreview = true,
defaultScope = "project",
}: Props) => { }: Props) => {
const [selectedOption, setSelectedOption] = useState(0); const [selectedOption, setSelectedOption] = useState(0);
const { const {
@@ -121,7 +123,7 @@ export const InlineGenericApproval = memo(
if (selectedOption === 0) { if (selectedOption === 0) {
onApprove(); onApprove();
} else if (selectedOption === 1 && allowPersistence) { } else if (selectedOption === 1 && allowPersistence) {
onApproveAlways("project"); onApproveAlways(defaultScope);
} }
return; return;
} }
@@ -136,7 +138,7 @@ export const InlineGenericApproval = memo(
return; return;
} }
if (input === "2" && allowPersistence) { if (input === "2" && allowPersistence) {
onApproveAlways("project"); onApproveAlways(defaultScope);
return; return;
} }
}, },

View File

@@ -3,6 +3,7 @@
import { homedir } from "node:os"; import { homedir } from "node:os";
import { dirname, relative, resolve, win32 } from "node:path"; import { dirname, relative, resolve, win32 } from "node:path";
import { canonicalToolName, isFileToolName } from "./canonical";
export interface ApprovalContext { export interface ApprovalContext {
// What rule should be saved if user clicks "approve always" // What rule should be saved if user clicks "approve always"
@@ -98,29 +99,30 @@ export function analyzeApprovalContext(
toolArgs: ToolArgs, toolArgs: ToolArgs,
workingDirectory: string, workingDirectory: string,
): ApprovalContext { ): ApprovalContext {
const canonicalTool = canonicalToolName(toolName);
const resolveFilePath = () => { const resolveFilePath = () => {
const candidate = const candidate =
toolArgs.file_path ?? toolArgs.path ?? toolArgs.notebook_path ?? ""; toolArgs.file_path ?? toolArgs.path ?? toolArgs.notebook_path ?? "";
return typeof candidate === "string" ? candidate : ""; return typeof candidate === "string" ? candidate : "";
}; };
switch (toolName) { switch (canonicalTool) {
case "Read": case "Read":
case "read_file":
return analyzeReadApproval(resolveFilePath(), workingDirectory); return analyzeReadApproval(resolveFilePath(), workingDirectory);
case "Write": case "Write":
return analyzeWriteApproval(resolveFilePath(), workingDirectory); return analyzeWriteApproval(resolveFilePath(), workingDirectory);
case "Edit": case "Edit":
case "MultiEdit":
return analyzeEditApproval(resolveFilePath(), workingDirectory); return analyzeEditApproval(resolveFilePath(), workingDirectory);
case "Bash": case "Bash":
case "shell":
case "shell_command":
return analyzeBashApproval( return analyzeBashApproval(
typeof toolArgs.command === "string" ? toolArgs.command : "", typeof toolArgs.command === "string"
? toolArgs.command
: Array.isArray(toolArgs.command)
? toolArgs.command.join(" ")
: "",
workingDirectory, workingDirectory,
); );
@@ -131,9 +133,8 @@ export function analyzeApprovalContext(
case "Glob": case "Glob":
case "Grep": case "Grep":
case "grep_files":
return analyzeSearchApproval( return analyzeSearchApproval(
toolName, canonicalTool,
typeof toolArgs.path === "string" ? toolArgs.path : workingDirectory, typeof toolArgs.path === "string" ? toolArgs.path : workingDirectory,
workingDirectory, workingDirectory,
); );
@@ -150,7 +151,7 @@ export function analyzeApprovalContext(
}; };
default: default:
return analyzeDefaultApproval(toolName); return analyzeDefaultApproval(canonicalTool, toolArgs, workingDirectory);
} }
} }
@@ -280,6 +281,7 @@ const SAFE_READONLY_COMMANDS = [
"diff", "diff",
"file", "file",
"stat", "stat",
"curl",
]; ];
// Commands that should never be auto-approved // Commands that should never be auto-approved
@@ -751,7 +753,41 @@ function analyzeSearchApproval(
/** /**
* Default approval for unknown tools * 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 { return {
recommendedRule: toolName, recommendedRule: toolName,
ruleDescription: `${toolName} operations`, ruleDescription: `${toolName} operations`,

View File

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

View File

@@ -4,6 +4,7 @@
import { resolve } from "node:path"; import { resolve } from "node:path";
import { getCurrentAgentId } from "../agent/context"; import { getCurrentAgentId } from "../agent/context";
import { runPermissionRequestHooks } from "../hooks"; import { runPermissionRequestHooks } from "../hooks";
import { canonicalToolName, isShellToolName } from "./canonical";
import { cliPermissions } from "./cli"; import { cliPermissions } from "./cli";
import { import {
matchesBashPattern, matchesBashPattern,
@@ -22,30 +23,7 @@ import type {
/** /**
* Tools that don't require approval within working directory * Tools that don't require approval within working directory
*/ */
const WORKING_DIRECTORY_TOOLS = [ const WORKING_DIRECTORY_TOOLS = ["Read", "Glob", "Grep", "ListDir"];
// 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 READ_ONLY_SHELL_TOOLS = new Set([ const READ_ONLY_SHELL_TOOLS = new Set([
"Bash", "Bash",
"shell", "shell",
@@ -83,8 +61,9 @@ export function checkPermission(
permissions: PermissionRules, permissions: PermissionRules,
workingDirectory: string = process.cwd(), workingDirectory: string = process.cwd(),
): PermissionCheckResult { ): PermissionCheckResult {
const canonicalTool = canonicalToolName(toolName);
// Build permission query string // Build permission query string
const query = buildPermissionQuery(toolName, toolArgs); const query = buildPermissionQuery(canonicalTool, toolArgs);
// Get session rules // Get session rules
const sessionRules = sessionPermissions.getRules(); 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); const shellCommand = extractShellCommand(toolArgs);
if (shellCommand && isReadOnlyShellCommand(shellCommand)) { if (shellCommand && isReadOnlyShellCommand(shellCommand)) {
return { return {
@@ -180,7 +159,7 @@ export function checkPermission(
} }
// After checking CLI overrides, check if Read/Glob/Grep within working directory // 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); const filePath = extractFilePath(toolArgs);
if ( if (
filePath && filePath &&
@@ -300,25 +279,7 @@ function buildPermissionQuery(toolName: string, toolArgs: ToolArgs): string {
case "Glob": case "Glob":
case "Grep": case "Grep":
// Codex file tools // Codex file tools
case "read_file": case "ListDir": {
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": {
const filePath = extractFilePath(toolArgs); const filePath = extractFilePath(toolArgs);
return filePath ? `${toolName}(${filePath})` : toolName; return filePath ? `${toolName}(${filePath})` : toolName;
} }
@@ -330,7 +291,9 @@ function buildPermissionQuery(toolName: string, toolArgs: ToolArgs): string {
return `Bash(${command})`; return `Bash(${command})`;
} }
case "shell": case "shell":
case "shell_command": { case "shell_command":
case "run_shell_command":
case "RunShellCommand": {
const command = const command =
typeof toolArgs.command === "string" typeof toolArgs.command === "string"
? toolArgs.command ? toolArgs.command
@@ -357,34 +320,7 @@ function extractShellCommand(toolArgs: ToolArgs): string | string[] | null {
/** /**
* File tools that use glob matching for permissions * File tools that use glob matching for permissions
*/ */
const FILE_TOOLS = [ const FILE_TOOLS = ["Read", "Write", "Edit", "Glob", "Grep", "ListDir"];
// 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",
];
/** /**
* Check if query matches a permission pattern * Check if query matches a permission pattern
@@ -395,22 +331,19 @@ function matchesPattern(
pattern: string, pattern: string,
workingDirectory: string, workingDirectory: string,
): boolean { ): boolean {
const canonicalTool = canonicalToolName(toolName);
// File tools use glob matching // File tools use glob matching
if (FILE_TOOLS.includes(toolName)) { if (FILE_TOOLS.includes(canonicalTool)) {
return matchesFilePattern(query, pattern, workingDirectory); return matchesFilePattern(query, pattern, workingDirectory);
} }
// Bash uses prefix matching // Bash uses prefix matching
if ( if (canonicalTool === "Bash") {
toolName === "Bash" ||
toolName === "shell" ||
toolName === "shell_command"
) {
return matchesBashPattern(query, pattern); return matchesBashPattern(query, pattern);
} }
// Other tools use simple name matching // Other tools use simple name matching
return matchesToolPattern(toolName, pattern); return matchesToolPattern(canonicalTool, pattern);
} }
/** /**

View File

@@ -2,6 +2,13 @@
// CLI-level permission overrides from command-line flags // CLI-level permission overrides from command-line flags
// These take precedence over settings.json but not over enterprise managed policies // 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. * CLI permission overrides that are set via --allowedTools and --disallowedTools flags.
* These rules override settings.json permissions for the current session. * These rules override settings.json permissions for the current session.
@@ -77,24 +84,27 @@ class CliPermissions {
* - Tool patterns with parentheses stay as-is * - Tool patterns with parentheses stay as-is
*/ */
private normalizePattern(pattern: string): string { private normalizePattern(pattern: string): string {
const trimmed = pattern.trim();
// If pattern has parentheses, keep as-is // If pattern has parentheses, keep as-is
if (pattern.includes("(")) { if (trimmed.includes("(")) {
return pattern; return normalizePermissionRule(trimmed);
} }
// Bash without parentheses needs wildcard to match all commands const canonicalTool = canonicalToolName(trimmed);
if (pattern === "Bash") {
// Bash/shell aliases without parentheses need wildcard to match all commands
if (isShellToolName(canonicalTool)) {
return "Bash(:*)"; return "Bash(:*)";
} }
// File tools need wildcard to match all files // File tools need wildcard to match all files
const fileTools = ["Read", "Write", "Edit", "Glob", "Grep"]; if (isFileToolName(canonicalTool)) {
if (fileTools.includes(pattern)) { return `${canonicalTool}(**)`;
return `${pattern}(**)`;
} }
// All other bare tool names stay as-is // All other bare tool names stay as-is
return pattern; return canonicalTool;
} }
/** /**

View File

@@ -4,6 +4,10 @@ import { exists, readFile, writeFile } from "../utils/fs.js";
import { homedir } from "node:os"; import { homedir } from "node:os";
import { join } from "node:path"; import { join } from "node:path";
import {
normalizePermissionRule,
permissionRulesEquivalent,
} from "./rule-normalization";
import type { PermissionRules } from "./types"; import type { PermissionRules } from "./types";
type SettingsFile = { type SettingsFile = {
@@ -69,13 +73,13 @@ function mergePermissions(
source: PermissionRules, source: PermissionRules,
): void { ): void {
if (source.allow) { if (source.allow) {
target.allow = [...(target.allow || []), ...source.allow]; target.allow = mergeRuleList(target.allow, source.allow);
} }
if (source.deny) { if (source.deny) {
target.deny = [...(target.deny || []), ...source.deny]; target.deny = mergeRuleList(target.deny, source.deny);
} }
if (source.ask) { if (source.ask) {
target.ask = [...(target.ask || []), ...source.ask]; target.ask = mergeRuleList(target.ask, source.ask);
} }
if (source.additionalDirectories) { if (source.additionalDirectories) {
target.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 * Save a permission rule to a specific scope
*/ */
@@ -131,9 +148,15 @@ export async function savePermissionRule(
settings.permissions[ruleType] = []; settings.permissions[ruleType] = [];
} }
// Add rule if not already present const normalizedRule = normalizePermissionRule(rule);
if (!settings.permissions[ruleType].includes(rule)) {
settings.permissions[ruleType].push(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 // Save settings

View File

@@ -3,6 +3,7 @@
import { resolve } from "node:path"; import { resolve } from "node:path";
import { minimatch } from "minimatch"; import { minimatch } from "minimatch";
import { canonicalToolName } from "./canonical";
/** /**
* Normalize path separators to forward slashes for consistent glob matching. * Normalize path separators to forward slashes for consistent glob matching.
@@ -115,21 +116,25 @@ export function matchesFilePattern(
): boolean { ): boolean {
// Extract tool name and file path from query // Extract tool name and file path from query
// Format: "ToolName(filePath)" // Format: "ToolName(filePath)"
const queryMatch = query.match(/^([^(]+)\((.+)\)$/); const queryMatch = query.match(/^([^(]+)\(([\s\S]+)\)$/);
if (!queryMatch || !queryMatch[1] || !queryMatch[2]) { if (!queryMatch || !queryMatch[1] || !queryMatch[2]) {
return false; return false;
} }
const queryTool = queryMatch[1]; const queryTool = canonicalToolName(queryMatch[1]);
// Normalize path separators for cross-platform compatibility // Normalize path separators for cross-platform compatibility
const filePath = normalizePath(queryMatch[2]); const filePath = normalizePath(queryMatch[2]);
// Extract tool name and glob pattern from permission rule // Extract tool name and glob pattern from permission rule
// Format: "ToolName(pattern)" // Format: "ToolName(pattern)"
const patternMatch = pattern.match(/^([^(]+)\((.+)\)$/); const patternMatch = pattern.match(/^([^(]+)\(([\s\S]+)\)$/);
if (!patternMatch || !patternMatch[1] || !patternMatch[2]) { 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; return false;
} }
const patternTool = patternMatch[1];
// Normalize path separators for cross-platform compatibility // Normalize path separators for cross-platform compatibility
let globPattern = normalizePath(patternMatch[2]); let globPattern = normalizePath(patternMatch[2]);
@@ -220,22 +225,36 @@ function extractActualCommand(command: string): string {
export function matchesBashPattern(query: string, pattern: string): boolean { export function matchesBashPattern(query: string, pattern: string): boolean {
// Extract the command from query // Extract the command from query
// Format: "Bash(actual command)" or "Bash()" // Format: "Tool(actual command)" or "Tool()"
const queryMatch = query.match(/^Bash\((.*)\)$/); const queryMatch = query.match(/^([^(]+)\(([\s\S]*)\)$/);
if (!queryMatch || queryMatch[1] === undefined) { if (
!queryMatch ||
queryMatch[1] === undefined ||
queryMatch[2] === undefined
) {
return false; 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 // Extract actual command by stripping cd prefixes from compound commands
const command = extractActualCommand(rawCommand); const command = extractActualCommand(rawCommand);
// Extract the command pattern from permission rule // Extract the command pattern from permission rule
// Format: "Bash(command pattern)" or "Bash()" // Format: "Tool(command pattern)" or "Tool()"
const patternMatch = pattern.match(/^Bash\((.*)\)$/); const patternMatch = pattern.match(/^([^(]+)\(([\s\S]*)\)$/);
if (!patternMatch || patternMatch[1] === undefined) { if (
!patternMatch ||
patternMatch[1] === undefined ||
patternMatch[2] === undefined
) {
return canonicalToolName(pattern) === "Bash";
}
if (canonicalToolName(patternMatch[1]) !== "Bash") {
return false; return false;
} }
const commandPattern = patternMatch[1]; const commandPattern = patternMatch[2];
// Check for wildcard suffix // Check for wildcard suffix
if (commandPattern.endsWith(":*")) { if (commandPattern.endsWith(":*")) {
@@ -260,19 +279,25 @@ export function matchesBashPattern(query: string, pattern: string): boolean {
* @param pattern - The permission pattern * @param pattern - The permission pattern
*/ */
export function matchesToolPattern(toolName: string, pattern: string): boolean { export function matchesToolPattern(toolName: string, pattern: string): boolean {
const canonicalTool = canonicalToolName(toolName);
// Wildcard matches everything // Wildcard matches everything
if (pattern === "*") { if (pattern === "*") {
return true; return true;
} }
if (canonicalToolName(pattern) === canonicalTool) {
return true;
}
// Check for tool name match (with or without parens) // Check for tool name match (with or without parens)
if (pattern === toolName || pattern === `${toolName}()`) { if (pattern === canonicalTool || pattern === `${canonicalTool}()`) {
return true; return true;
} }
// Check for tool name prefix (e.g., "WebFetch(...)") // Check for tool name prefix (e.g., "WebFetch(...)")
if (pattern.startsWith(`${toolName}(`)) { const patternToolMatch = pattern.match(/^([^(]+)\(/);
return true; if (patternToolMatch?.[1]) {
return canonicalToolName(patternToolMatch[1]) === canonicalTool;
} }
return false; return false;

View File

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

View File

@@ -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.recommendedRule).toBe("Bash(npm run lint:*)");
expect(context.approveAlwaysText).toContain("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:*)");
});

View File

@@ -641,3 +641,39 @@ test("Tool with alternative path parameter (Glob uses 'path' not 'file_path')",
expect(result.decision).toBe("allow"); 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");
});

View File

@@ -399,3 +399,19 @@ test("Precedence: CLI allowedTools > settings allow", () => {
expect(dockerResult.decision).toBe("allow"); expect(dockerResult.decision).toBe("allow");
expect(dockerResult.matchedRule).toBe("Bash(docker:*)"); 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(**)"]);
});

View File

@@ -368,3 +368,59 @@ test("Saving local settings creates .gitignore if missing", async () => {
const content = await gitignoreFile.text(); const content = await gitignoreFile.text();
expect(content).toContain(".letta/settings.local.json"); 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);
});

View File

@@ -419,3 +419,10 @@ test("File pattern: extended UNC pattern matches UNC query path", () => {
), ),
).toBe(true); ).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);
});

View File

@@ -469,3 +469,23 @@ test("plan mode - remembers and restores previous mode", () => {
// Once we leave plan mode, the remembered mode is consumed. // Once we leave plan mode, the remembered mode is consumed.
expect(permissionMode.getModeBeforePlan()).toBe(null); 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");
});