mirror of
https://github.com/anthropics/claude-code.git
synced 2026-02-19 04:27:33 -08:00
Compare commits
1 Commits
8c09097e8c
...
claude/sla
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0d15fae207 |
177
.claude/bash-permission-bug-analysis.md
Normal file
177
.claude/bash-permission-bug-analysis.md
Normal file
@@ -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.
|
||||||
@@ -1,5 +1,9 @@
|
|||||||
# Changelog
|
# Changelog
|
||||||
|
|
||||||
|
## Unreleased
|
||||||
|
|
||||||
|
- Fixed bash permission rules with unbalanced quotes being saved to settings, causing settings to fail validation on load
|
||||||
|
|
||||||
## 2.0.74
|
## 2.0.74
|
||||||
|
|
||||||
- Added LSP (Language Server Protocol) tool for code intelligence features like go-to-definition, find references, and hover documentation
|
- Added LSP (Language Server Protocol) tool for code intelligence features like go-to-definition, find references, and hover documentation
|
||||||
|
|||||||
Reference in New Issue
Block a user