mirror of
https://github.com/anthropics/claude-code.git
synced 2026-02-19 04:27:33 -08:00
Compare commits
2 Commits
claude/sla
...
claude/sla
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0d15fae207 | ||
|
|
5c92b97cc4 |
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
|
||||
|
||||
## 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
|
||||
|
||||
@@ -1,26 +1,18 @@
|
||||
---
|
||||
description: "Cancel active Ralph Wiggum loop"
|
||||
allowed-tools: ["Bash"]
|
||||
allowed-tools: ["Bash(test -f .claude/ralph-loop.local.md:*)", "Bash(rm .claude/ralph-loop.local.md)", "Read(.claude/ralph-loop.local.md)"]
|
||||
hide-from-slash-command-tool: "true"
|
||||
---
|
||||
|
||||
# Cancel Ralph
|
||||
|
||||
```!
|
||||
if [[ -f .claude/ralph-loop.local.md ]]; then
|
||||
ITERATION=$(grep '^iteration:' .claude/ralph-loop.local.md | sed 's/iteration: *//')
|
||||
echo "FOUND_LOOP=true"
|
||||
echo "ITERATION=$ITERATION"
|
||||
else
|
||||
echo "FOUND_LOOP=false"
|
||||
fi
|
||||
```
|
||||
To cancel the Ralph loop:
|
||||
|
||||
Check the output above:
|
||||
1. Check if `.claude/ralph-loop.local.md` exists using Bash: `test -f .claude/ralph-loop.local.md && echo "EXISTS" || echo "NOT_FOUND"`
|
||||
|
||||
1. **If FOUND_LOOP=false**:
|
||||
- Say "No active Ralph loop found."
|
||||
2. **If NOT_FOUND**: Say "No active Ralph loop found."
|
||||
|
||||
2. **If FOUND_LOOP=true**:
|
||||
- Use Bash: `rm .claude/ralph-loop.local.md`
|
||||
- Report: "Cancelled Ralph loop (was at iteration N)" where N is the ITERATION value from above.
|
||||
3. **If EXISTS**:
|
||||
- Read `.claude/ralph-loop.local.md` to get the current iteration number from the `iteration:` field
|
||||
- Remove the file using Bash: `rm .claude/ralph-loop.local.md`
|
||||
- Report: "Cancelled Ralph loop (was at iteration N)" where N is the iteration value
|
||||
|
||||
@@ -11,36 +11,6 @@ Execute the setup script to initialize the Ralph loop:
|
||||
|
||||
```!
|
||||
"${CLAUDE_PLUGIN_ROOT}/scripts/setup-ralph-loop.sh" $ARGUMENTS
|
||||
|
||||
# Extract and display completion promise if set
|
||||
if [ -f .claude/ralph-loop.local.md ]; then
|
||||
PROMISE=$(grep '^completion_promise:' .claude/ralph-loop.local.md | sed 's/completion_promise: *//' | sed 's/^"\(.*\)"$/\1/')
|
||||
if [ -n "$PROMISE" ] && [ "$PROMISE" != "null" ]; then
|
||||
echo ""
|
||||
echo "═══════════════════════════════════════════════════════════"
|
||||
echo "CRITICAL - Ralph Loop Completion Promise"
|
||||
echo "═══════════════════════════════════════════════════════════"
|
||||
echo ""
|
||||
echo "To complete this loop, output this EXACT text:"
|
||||
echo " <promise>$PROMISE</promise>"
|
||||
echo ""
|
||||
echo "STRICT REQUIREMENTS (DO NOT VIOLATE):"
|
||||
echo " ✓ Use <promise> XML tags EXACTLY as shown above"
|
||||
echo " ✓ The statement MUST be completely and unequivocally TRUE"
|
||||
echo " ✓ Do NOT output false statements to exit the loop"
|
||||
echo " ✓ Do NOT lie even if you think you should exit"
|
||||
echo ""
|
||||
echo "IMPORTANT - Do not circumvent the loop:"
|
||||
echo " Even if you believe you're stuck, the task is impossible,"
|
||||
echo " or you've been running too long - you MUST NOT output a"
|
||||
echo " false promise statement. The loop is designed to continue"
|
||||
echo " until the promise is GENUINELY TRUE. Trust the process."
|
||||
echo ""
|
||||
echo " If the loop should stop, the promise statement will become"
|
||||
echo " true naturally. Do not force it by lying."
|
||||
echo "═══════════════════════════════════════════════════════════"
|
||||
fi
|
||||
fi
|
||||
```
|
||||
|
||||
Please work on the task. When you try to exit, the Ralph loop will feed the SAME PROMPT back to you for the next iteration. You'll see your previous work in files and git history, allowing you to iterate and improve.
|
||||
|
||||
@@ -174,3 +174,30 @@ if [[ -n "$PROMPT" ]]; then
|
||||
echo ""
|
||||
echo "$PROMPT"
|
||||
fi
|
||||
|
||||
# Display completion promise requirements if set
|
||||
if [[ "$COMPLETION_PROMISE" != "null" ]]; then
|
||||
echo ""
|
||||
echo "═══════════════════════════════════════════════════════════"
|
||||
echo "CRITICAL - Ralph Loop Completion Promise"
|
||||
echo "═══════════════════════════════════════════════════════════"
|
||||
echo ""
|
||||
echo "To complete this loop, output this EXACT text:"
|
||||
echo " <promise>$COMPLETION_PROMISE</promise>"
|
||||
echo ""
|
||||
echo "STRICT REQUIREMENTS (DO NOT VIOLATE):"
|
||||
echo " ✓ Use <promise> XML tags EXACTLY as shown above"
|
||||
echo " ✓ The statement MUST be completely and unequivocally TRUE"
|
||||
echo " ✓ Do NOT output false statements to exit the loop"
|
||||
echo " ✓ Do NOT lie even if you think you should exit"
|
||||
echo ""
|
||||
echo "IMPORTANT - Do not circumvent the loop:"
|
||||
echo " Even if you believe you're stuck, the task is impossible,"
|
||||
echo " or you've been running too long - you MUST NOT output a"
|
||||
echo " false promise statement. The loop is designed to continue"
|
||||
echo " until the promise is GENUINELY TRUE. Trust the process."
|
||||
echo ""
|
||||
echo " If the loop should stop, the promise statement will become"
|
||||
echo " true naturally. Do not force it by lying."
|
||||
echo "═══════════════════════════════════════════════════════════"
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user