diff --git a/.claude/bash-permission-bug-analysis.md b/.claude/bash-permission-bug-analysis.md new file mode 100644 index 00000000..69f17e02 --- /dev/null +++ b/.claude/bash-permission-bug-analysis.md @@ -0,0 +1,177 @@ +# Bash Permission Serialization Bug Analysis + +## Bug Description + +When Claude Code saves bash permission rules to `settings.local.json`, malformed commands with unbalanced quotes can be stored. The validation that detects malformed patterns only runs when **loading** settings, not when **saving** them. + +### Reproduction Steps +1. Have Claude run a bash command containing quotes (e.g., arithmetic with quoted strings) +2. When permission is requested, click "Always Allow" +3. A malformed permission rule may be saved to `settings.local.json` +4. On next Claude Code startup, the settings fail to load with error: + ``` + "Bash(250/0... + └ 029 ")": Unmatched " in Bash pattern. Ensure all quotes are properly paired + ``` + +### Example Malformed Rule +```json +{ + "permissions": { + "allow": ["Bash(250/0.029 \")"] + } +} +``` + +## Root Cause Analysis + +### Code Flow + +1. **Permission Check** (`OE0` function): + - Parses the bash command + - Creates suggestions via `UE0(command)` or `im5(prefix)` + - Returns suggestions to be shown in permission dialog + +2. **Suggestion Creation** (`UE0` function): + ```javascript + function UE0(A){ + return[{ + type:"addRules", + rules:[{toolName:M9.name, ruleContent:A}], + behavior:"allow", + destination:"localSettings" + }] + } + ``` + - Takes command string directly as `ruleContent` + - **No validation** that the command is well-formed + +3. **Saving Rules** (`_A1` function): + ```javascript + function _A1({ruleValues:A, ruleBehavior:Q}, B){ + let G = A.map(s5); // Serialize rules + // ... save to settings + nB(B, W); // No validation before save + } + ``` + +4. **Serialization** (`s5` function): + ```javascript + function s5(A){ + if(!A.ruleContent) return A.toolName; + let Q = HJ7(A.ruleContent); // Escape parens/backslashes + return `${A.toolName}(${Q})` + } + ``` + - Only escapes `\`, `(`, `)` + - Does **not** validate quotes are balanced + +5. **Validation** (only on load): + ```javascript + let X = ['"', "'"]; + for(let W of X) + if((J.match(new RegExp(W,"g"))||[]).length % 2 !== 0) + return { + valid: false, + error: `Unmatched ${W} in Bash pattern`, + suggestion: "Ensure all quotes are properly paired" + }; + ``` + +### The Gap + +Validation exists but is only called when **parsing** settings, not when **creating** permission rules. This allows: +- Malformed commands from the model to be saved +- Commands with shell parsing errors to be saved +- Truncated or incorrectly reconstructed commands to be saved + +## Proposed Fix + +### Option 1: Validate Before Saving (Recommended) + +Add validation in `_A1` before saving: + +```javascript +function _A1({ruleValues: A, ruleBehavior: Q}, B) { + if (A.length < 1) return true; + + // Validate each rule before saving + for (const rule of A) { + const serialized = s5(rule); + const validation = validatePermissionRule(serialized); + if (!validation.valid) { + console.error(`Invalid permission rule: ${validation.error}`); + return false; + } + } + + let G = A.map(s5); + // ... rest of function +} +``` + +### Option 2: Validate in UE0/im5 + +Add validation when creating suggestions: + +```javascript +function UE0(A) { + // Validate command has balanced quotes + const doubleQuotes = (A.match(/"/g) || []).length; + const singleQuotes = (A.match(/'/g) || []).length; + + if (doubleQuotes % 2 !== 0 || singleQuotes % 2 !== 0) { + // Don't create suggestion for malformed command + return []; + } + + return [{ + type: "addRules", + rules: [{toolName: M9.name, ruleContent: A}], + behavior: "allow", + destination: "localSettings" + }]; +} +``` + +### Option 3: Sanitize Command Before Creating Rule + +If the command has unbalanced quotes, try to fix or reject it: + +```javascript +function sanitizeCommandForRule(command) { + // Check for balanced quotes + const dq = (command.match(/"/g) || []).length; + const sq = (command.match(/'/g) || []).length; + + if (dq % 2 !== 0 || sq % 2 !== 0) { + // Log warning and return null to skip this rule + console.warn(`Skipping malformed command for permission rule: ${command}`); + return null; + } + + return command; +} +``` + +## Additional Observations + +1. The command parsing/reconstruction in `VJ7` is complex and may have edge cases where commands are not properly reconstructed + +2. The `KE0` function uses quote markers (`__DOUBLE_QUOTE__`, `__SINGLE_QUOTE__`) for parsing which may not be fully restored in all cases + +3. For piped commands, each segment is processed separately, increasing the chance of parsing errors + +## Recommended Fix Priority + +1. **Immediate**: Add validation before saving in `_A1` or `$v` functions +2. **Short-term**: Add better error handling when command parsing fails +3. **Long-term**: Review command reconstruction logic in `VJ7` for edge cases + +## Related CHANGELOG Entries + +- "Fixed permission rules incorrectly rejecting valid bash commands containing shell glob patterns" +- "Fixed Bash tool crashes caused by malformed shell syntax parsing" +- "Fixed security vulnerability in Bash tool permission checks" + +These suggest this area of the code has had issues before. diff --git a/CHANGELOG.md b/CHANGELOG.md index eb2d1256..b53d1b76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Fixed bash permission rules with unbalanced quotes being saved to settings, causing settings to fail validation on load + ## 2.0.74 - Added LSP (Language Server Protocol) tool for code intelligence features like go-to-definition, find references, and hover documentation