From 6f7b3bb08b7e9c352f3b7006e3f0ec4be7179f63 Mon Sep 17 00:00:00 2001 From: cpacker Date: Sun, 26 Oct 2025 19:12:55 -0700 Subject: [PATCH] fix: generate smart wildcard patterns for complex bash commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When users approve long piped/chained commands (e.g., cd /path && git diff | head -100), the system now generates intelligent wildcard patterns instead of exact matches. This allows similar commands to be cached properly. Changes: - Parse complex commands containing &&, |, or ; to extract significant patterns - Generate wildcards like Bash(cd /path && git diff:*) for git commands - Generate wildcards like Bash(npm run lint:*) for npm commands - Add tests for long command pattern generation Fixes the issue where approving git diff | head would not also allow git diff | tail 👾 Generated with [Letta Code](https://letta.com) Co-Authored-By: Letta --- src/permissions/analyzer.ts | 75 ++++++++++++++++++++++++++ src/tests/permissions-analyzer.test.ts | 42 +++++++++++++++ src/tests/permissions-checker.test.ts | 41 ++++++++++++++ 3 files changed, 158 insertions(+) diff --git a/src/permissions/analyzer.ts b/src/permissions/analyzer.ts index 4436218..8cc5511 100644 --- a/src/permissions/analyzer.ts +++ b/src/permissions/analyzer.ts @@ -306,6 +306,81 @@ function analyzeBashApproval( }; } + // Handle complex piped/chained commands (cd /path && git diff | head) + // Extract the most significant command and generate a wildcard pattern + if ( + command.includes("&&") || + command.includes("|") || + command.includes(";") + ) { + // Split on these delimiters and analyze each part + const segments = command.split(/\s*(?:&&|\||;)\s*/); + + for (const segment of segments) { + const segmentParts = segment.trim().split(/\s+/); + const segmentBase = segmentParts[0]; + const segmentArg = segmentParts[1] || ""; + + // Check if this segment is git command + if (segmentBase === "git") { + const gitSubcommand = segmentArg; + const safeGitCommands = ["status", "diff", "log", "show", "branch"]; + const writeGitCommands = ["push", "pull", "fetch", "commit", "add"]; + + if ( + safeGitCommands.includes(gitSubcommand) || + writeGitCommands.includes(gitSubcommand) + ) { + // Generate wildcard pattern that includes the leading commands + // e.g., "cd /path && git diff:*" + const beforeGit = command.substring(0, command.indexOf("git")); + const pattern = `${beforeGit}git ${gitSubcommand}:*`; + + return { + recommendedRule: `Bash(${pattern})`, + ruleDescription: `'${beforeGit}git ${gitSubcommand}' commands`, + approveAlwaysText: `Yes, and don't ask again for '${beforeGit}git ${gitSubcommand}' commands in this project`, + defaultScope: "project", + allowPersistence: true, + safetyLevel: safeGitCommands.includes(gitSubcommand) + ? "safe" + : "moderate", + }; + } + } + + // Check if this segment is npm/bun/yarn/pnpm + if (["npm", "bun", "yarn", "pnpm"].includes(segmentBase)) { + const subcommand = segmentArg; + const thirdPart = segmentParts[2]; + + if (subcommand === "run" && thirdPart) { + const fullCommand = `${segmentBase} ${subcommand} ${thirdPart}`; + return { + recommendedRule: `Bash(${fullCommand}:*)`, + ruleDescription: `'${fullCommand}' commands`, + approveAlwaysText: `Yes, and don't ask again for '${fullCommand}' commands in this project`, + defaultScope: "project", + allowPersistence: true, + safetyLevel: "safe", + }; + } + + if (subcommand) { + const fullCommand = `${segmentBase} ${subcommand}`; + return { + recommendedRule: `Bash(${fullCommand}:*)`, + ruleDescription: `'${fullCommand}' commands`, + approveAlwaysText: `Yes, and don't ask again for '${fullCommand}' commands in this project`, + defaultScope: "project", + allowPersistence: true, + safetyLevel: "safe", + }; + } + } + } + } + // Default: allow this specific command only const displayCommand = command.length > 40 ? `${command.slice(0, 40)}...` : command; diff --git a/src/tests/permissions-analyzer.test.ts b/src/tests/permissions-analyzer.test.ts index 4a62f72..8f6d597 100644 --- a/src/tests/permissions-analyzer.test.ts +++ b/src/tests/permissions-analyzer.test.ts @@ -353,3 +353,45 @@ test("Unknown tool suggests session-only", () => { expect(context.defaultScope).toBe("session"); expect(context.safetyLevel).toBe("moderate"); }); + +// ============================================================================ +// Long Command Bugs +// ============================================================================ + +test("Long complex bash commands should generate smart wildcard patterns", () => { + // Bug: When command is >40 chars, analyzer saves exact match instead of wildcard + // Example: "cd /path && git diff file.ts | head -100" + // Should generate: "Bash(cd /path && git diff:*)" to also match "... | tail -30" + + const longCommand = + "cd /Users/test/project && git diff src/file.ts | head -100"; + + const context = analyzeApprovalContext( + "Bash", + { command: longCommand }, + "/Users/test/project", + ); + + // Should extract "git diff" pattern, not save full command + expect(context.recommendedRule).toBe( + "Bash(cd /Users/test/project && git diff:*)", + ); + // Button text should reflect the wildcard pattern + expect(context.approveAlwaysText).not.toContain("..."); +}); + +test("Very long non-git commands should generate prefix-based wildcards", () => { + // For commands that don't match known patterns (npm, git, etc) + // we should still be smarter than exact match + const longCommand = "cd /Users/test/project && npm run lint 2>&1 | tail -20"; + + const context = analyzeApprovalContext( + "Bash", + { command: longCommand }, + "/Users/test/project", + ); + + // Should generate wildcard for "npm run lint" + expect(context.recommendedRule).toBe("Bash(npm run lint:*)"); + expect(context.approveAlwaysText).toContain("npm run lint"); +}); diff --git a/src/tests/permissions-checker.test.ts b/src/tests/permissions-checker.test.ts index 8534d30..2ea17c3 100644 --- a/src/tests/permissions-checker.test.ts +++ b/src/tests/permissions-checker.test.ts @@ -44,6 +44,47 @@ test("Glob within working directory is auto-allowed", () => { expect(result.decision).toBe("allow"); }); +// ============================================================================ +// Long Command Caching Tests +// ============================================================================ + +test("Long bash commands should use wildcard patterns, not exact match", () => { + // This is the bug: when you approve a long command like + // "cd /path && git diff file.ts | head -100" + // it should also match + // "cd /path && git diff file.ts | tail -30" + // But currently it saves an exact match instead of a wildcard + + const longCommand1 = + "cd /Users/test/project && git diff src/file.ts | head -100"; + const longCommand2 = + "cd /Users/test/project && git diff src/file.ts | tail -30"; + + // After approving the first command with a wildcard pattern + const permissions: PermissionRules = { + allow: ["Bash(cd /Users/test/project && git diff:*)"], + deny: [], + ask: [], + }; + + // Both should match + const result1 = checkPermission( + "Bash", + { command: longCommand1 }, + permissions, + "/Users/test/project", + ); + expect(result1.decision).toBe("allow"); + + const result2 = checkPermission( + "Bash", + { command: longCommand2 }, + permissions, + "/Users/test/project", + ); + expect(result2.decision).toBe("allow"); +}); + test("Grep within working directory is auto-allowed", () => { const result = checkPermission( "Grep",