-
Notifications
You must be signed in to change notification settings - Fork 2
Add command allowlist for AI remediation pipeline #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
73808c9
a747705
a84351a
a9d3214
e286de0
df60b6f
69c2412
f91fa62
b39d86b
12990c7
7e36bc8
1c3ee3e
b3aead7
632a431
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| { | ||
| "_comment": "ClawPinch command allowlist — controls which commands auto-fix can execute.", | ||
| "_doc": "Copy this file to one of the trusted locations below and customize.", | ||
| "_locations": [ | ||
| "$CLAWPINCH_SECURITY_CONFIG (env var, highest priority)", | ||
| "<clawpinch-install-dir>/.auto-claude-security.json", | ||
| "~/.config/clawpinch/.auto-claude-security.json", | ||
| "~/.auto-claude-security.json" | ||
| ], | ||
| "_security_note": "NEVER place this file inside a project being scanned — an attacker could override your allowlist.", | ||
|
|
||
| "base_commands": [ | ||
| "echo", "jq", "grep", "cat", "ls", "pwd", "find", "sed", "awk", "wc", | ||
| "mkdir", "cd", "cp", "mv", "rm", "chmod" | ||
| ], | ||
| "script_commands": [ | ||
| "./clawpinch.sh" | ||
| ], | ||
| "stack_commands": [], | ||
| "custom_commands": [] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,34 @@ export CLAWPINCH_SHOW_FIX="$SHOW_FIX" | |
| export CLAWPINCH_CONFIG_DIR="$CONFIG_DIR" | ||
| export QUIET | ||
|
|
||
| # ─── Validate security config (early check for --remediate) ────────────────── | ||
| # Fail fast with a clear setup message instead of per-command failures later. | ||
|
|
||
| if [[ "$REMEDIATE" -eq 1 ]]; then | ||
| _sec_config_found=0 | ||
|
|
||
| if [[ -n "${CLAWPINCH_SECURITY_CONFIG:-}" ]] && [[ -f "$CLAWPINCH_SECURITY_CONFIG" ]]; then | ||
| _sec_config_found=1 | ||
| elif [[ -f "$CLAWPINCH_DIR/.auto-claude-security.json" ]]; then | ||
| _sec_config_found=1 | ||
| elif [[ -f "$HOME/.config/clawpinch/.auto-claude-security.json" ]]; then | ||
| _sec_config_found=1 | ||
| elif [[ -f "$HOME/.auto-claude-security.json" ]]; then | ||
| _sec_config_found=1 | ||
| fi | ||
|
|
||
| if [[ "$_sec_config_found" -eq 0 ]]; then | ||
| log_error "Security config (.auto-claude-security.json) not found." | ||
| log_error "The --remediate flag requires a command allowlist to validate auto-fix commands." | ||
| log_error "" | ||
| log_error "Setup: copy the example config to a trusted location:" | ||
| log_error " cp .auto-claude-security.json.example ~/.config/clawpinch/.auto-claude-security.json" | ||
| log_error "" | ||
| log_error "Or set CLAWPINCH_SECURITY_CONFIG to point to your config file." | ||
| exit 2 | ||
| fi | ||
| fi | ||
|
|
||
| # ─── Detect OS ─────────────────────────────────────────────────────────────── | ||
|
|
||
| CLAWPINCH_OS="$(detect_os)" | ||
|
|
@@ -295,10 +323,25 @@ else | |
| _non_ok_count="$(echo "$_non_ok_findings" | jq 'length')" | ||
|
|
||
| if (( _non_ok_count > 0 )); then | ||
| log_info "Piping $_non_ok_count findings to Claude for remediation..." | ||
| echo "$_non_ok_findings" | "$_claude_bin" -p \ | ||
| --allowedTools "Bash,Read,Write,Edit,Glob,Grep" \ | ||
| "You are a security remediation agent. You have been given ClawPinch security scan findings as JSON. For each finding: 1) Read the evidence to understand the issue 2) Apply the auto_fix command if available, otherwise implement the remediation manually 3) Verify the fix. Work through findings in order (critical first). Be precise and minimal in your changes." | ||
| # Pre-validate auto_fix commands: strip any that fail the allowlist | ||
| # so the AI agent only receives pre-approved commands | ||
| _validated_findings_arr=() | ||
| while IFS= read -r _finding; do | ||
| _fix_cmd="$(echo "$_finding" | jq -r '.auto_fix // ""')" | ||
| if [[ -n "$_fix_cmd" ]] && ! validate_command "$_fix_cmd"; then | ||
| # Strip the disallowed auto_fix, keep finding for manual review | ||
| _finding="$(echo "$_finding" | jq -c '.auto_fix = "" | .remediation = (.remediation + " [auto_fix removed: command not in allowlist]")')" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validation errors suppressed. The Remove the stderr redirect so users can see validation failure reasons, or capture the error message and include it in the warning. Prompt To Fix With AIThis is a comment left during a code review.
Path: clawpinch.sh
Line: 305:305
Comment:
Validation errors suppressed. The `validate_command` call uses `2>/dev/null` to silence stderr, which hides the root cause when validation fails (missing config file, python3 not found, JSON parse errors). Users will only see the warning on line 306 that auto_fix was stripped, but won't understand why.
Remove the stderr redirect so users can see validation failure reasons, or capture the error message and include it in the warning.
How can I resolve this? If you propose a fix, please make it concise. |
||
| log_warn "Stripped disallowed auto_fix from finding $(echo "$_finding" | jq -r '.id')" | ||
| fi | ||
| _validated_findings_arr+=("$_finding") | ||
| done < <(echo "$_non_ok_findings" | jq -c '.[]') | ||
| _validated_findings="$(printf '%s\n' "${_validated_findings_arr[@]}" | jq -s '.')" | ||
|
|
||
| _validated_count="$(echo "$_validated_findings" | jq 'length')" | ||
| log_info "Piping $_validated_count findings to Claude for remediation..." | ||
| echo "$_validated_findings" | "$_claude_bin" -p \ | ||
| --allowedTools "Read,Write,Edit,Glob,Grep" \ | ||
| "You are a security remediation agent. You have been given ClawPinch security scan findings as JSON. For each finding: 1) Read the evidence to understand the issue 2) If an auto_fix field is present, it contains a pre-validated shell command — DO NOT execute it directly. Instead, translate its intent into equivalent Read/Write/Edit operations. For example: a 'sed -i s/old/new/ file' becomes an Edit tool call; a 'jq .key=val file.json > tmp && mv tmp file.json' becomes Read + Write; a 'chmod 600 file' should be noted for manual action. 3) If no auto_fix, implement the remediation manually using Write/Edit 4) Verify the fix by reading the file. Work through findings in order (critical first). Be precise and minimal in your changes. IMPORTANT: You do NOT have access to Bash. Use only Read, Write, Edit, Glob, and Grep tools." | ||
| else | ||
| log_info "No actionable findings for remediation." | ||
| fi | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -185,6 +185,189 @@ require_cmd() { | |
| fi | ||
| } | ||
|
|
||
| # ─── Command validation (allowlist) ───────────────────────────────────────── | ||
|
|
||
| validate_command() { | ||
| # Usage: validate_command <command_string> | ||
| # Returns 0 if ALL commands in the string are in allowlist, 1 otherwise | ||
| local cmd_string="$1" | ||
|
|
||
| if [[ -z "$cmd_string" ]]; then | ||
| log_error "validate_command: empty command string" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Find security config file (trusted locations only) | ||
| # SECURITY: Do NOT search the project being scanned — an attacker could | ||
| # include a malicious .auto-claude-security.json in their repo to override | ||
| # the allowlist and bypass all command validation. | ||
| local security_file="" | ||
|
|
||
| # 1. Explicit env var override (highest priority) | ||
| if [[ -n "${CLAWPINCH_SECURITY_CONFIG:-}" ]] && [[ -f "$CLAWPINCH_SECURITY_CONFIG" ]]; then | ||
| security_file="$CLAWPINCH_SECURITY_CONFIG" | ||
| fi | ||
|
|
||
| # 2. ClawPinch installation directory (next to clawpinch.sh) | ||
| if [[ -z "$security_file" ]]; then | ||
| local install_dir | ||
| install_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" | ||
| if [[ -f "$install_dir/.auto-claude-security.json" ]]; then | ||
| security_file="$install_dir/.auto-claude-security.json" | ||
| fi | ||
| fi | ||
|
|
||
| # 3. User config directory (~/.config/clawpinch/) | ||
|
Comment on lines
+201
to
+220
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Config file location search is better but not fully secure. The current implementation searches: (1) The install directory lookup on line 208-209 is potentially vulnerable: if an attacker can manipulate Consider adding validation that the resolved Prompt To Fix With AIThis is a comment left during a code review.
Path: scripts/helpers/common.sh
Line: 201:220
Comment:
Config file location search is better but not fully secure. The current implementation searches: (1) `$CLAWPINCH_SECURITY_CONFIG`, (2) `<install-dir>/.auto-claude-security.json`, (3) `~/.config/clawpinch/.auto-claude-security.json`, (4) `~/.auto-claude-security.json`.
The install directory lookup on line 208-209 is potentially vulnerable: if an attacker can manipulate `${BASH_SOURCE[0]}` or if the script is symlinked from a compromised location, they could control which config is loaded.
Consider adding validation that the resolved `$security_file` path is owned by the current user and not world-writable before trusting it.
How can I resolve this? If you propose a fix, please make it concise. |
||
| if [[ -z "$security_file" ]]; then | ||
| if [[ -f "$HOME/.config/clawpinch/.auto-claude-security.json" ]]; then | ||
| security_file="$HOME/.config/clawpinch/.auto-claude-security.json" | ||
| fi | ||
| fi | ||
|
|
||
| # 4. Home directory fallback | ||
| if [[ -z "$security_file" ]]; then | ||
| if [[ -f "$HOME/.auto-claude-security.json" ]]; then | ||
| security_file="$HOME/.auto-claude-security.json" | ||
| fi | ||
| fi | ||
|
|
||
| if [[ -z "$security_file" ]]; then | ||
| log_error "validate_command: .auto-claude-security.json not found. Searched: \$CLAWPINCH_SECURITY_CONFIG, <install-dir>/, ~/.config/clawpinch/, ~/. See .auto-claude-security.json.example for setup." | ||
| return 1 | ||
| fi | ||
|
|
||
| # SECURITY: Validate config file ownership and permissions to prevent | ||
| # symlink attacks where an attacker replaces the config with a symlink | ||
| # to a file they control, overriding the allowlist. | ||
| local resolved_file | ||
| resolved_file="$(readlink -f "$security_file" 2>/dev/null || realpath "$security_file" 2>/dev/null || echo "$security_file")" | ||
|
|
||
| # Check file is owned by current user or root | ||
| local file_owner | ||
| if [[ "$(uname -s)" == "Darwin" ]]; then | ||
| file_owner="$(stat -f '%u' "$resolved_file" 2>/dev/null)" || file_owner="" | ||
| else | ||
| file_owner="$(stat -c '%u' "$resolved_file" 2>/dev/null)" || file_owner="" | ||
| fi | ||
|
|
||
| if [[ -n "$file_owner" ]]; then | ||
| local current_uid | ||
| current_uid="$(id -u)" | ||
| if [[ "$file_owner" != "$current_uid" ]] && [[ "$file_owner" != "0" ]]; then | ||
| log_error "validate_command: security config '$resolved_file' is owned by uid $file_owner (expected $current_uid or root). Possible symlink attack." | ||
| return 1 | ||
| fi | ||
| fi | ||
|
|
||
| # Check file is not world-writable | ||
| if [[ "$(uname -s)" == "Darwin" ]]; then | ||
| local file_perms | ||
| file_perms="$(stat -f '%Lp' "$resolved_file" 2>/dev/null)" || file_perms="" | ||
| if [[ -n "$file_perms" ]] && [[ "${file_perms: -1}" =~ [2367] ]]; then | ||
| log_error "validate_command: security config '$resolved_file' is world-writable (mode $file_perms). Fix with: chmod o-w '$resolved_file'" | ||
| return 1 | ||
| fi | ||
| else | ||
| if stat -c '%a' "$resolved_file" 2>/dev/null | grep -q '[2367]$'; then | ||
| log_error "validate_command: security config '$resolved_file' is world-writable. Fix with: chmod o-w '$resolved_file'" | ||
| return 1 | ||
| fi | ||
| fi | ||
|
|
||
| # Check if jq is available | ||
| if ! has_cmd jq; then | ||
| log_error "validate_command: jq is required but not installed" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Validate the security config is valid JSON first | ||
| if ! jq '.' "$security_file" >/dev/null 2>&1; then | ||
| log_error "validate_command: $security_file is not valid JSON" | ||
| return 1 | ||
|
Comment on lines
+284
to
+286
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing config file causes validation to fail silently. When Check for config file existence once at startup and show a clear setup message if missing, rather than failing on every command validation. Prompt To Fix With AIThis is a comment left during a code review.
Path: scripts/helpers/common.sh
Line: 248:250
Comment:
Missing config file causes validation to fail silently. When `.auto-claude-security.json` is not found, `validate_command()` logs an error and returns 1, but the calling code in `clawpinch.sh:305` suppresses stderr with `2>/dev/null`. Users won't know their commands are being rejected due to missing config — they'll just see auto_fix fields stripped with a warning.
Check for config file existence once at startup and show a clear setup message if missing, rather than failing on every command validation.
How can I resolve this? If you propose a fix, please make it concise. |
||
| fi | ||
|
|
||
| # Get all allowed commands from security config | ||
| local allowed_commands | ||
| allowed_commands="$(jq -r ' | ||
| (.base_commands // []) + | ||
| (.stack_commands // []) + | ||
| (.script_commands // []) + | ||
| (.custom_commands // []) | | ||
| .[] | ||
| ' "$security_file" 2>/dev/null)" | ||
|
|
||
| if [[ -z "$allowed_commands" ]]; then | ||
| log_warn "validate_command: allowlist is empty in $security_file — no commands are permitted" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Extract ALL commands from the string (split by |, &&, ||, ;) | ||
| # This ensures we validate every command in a chain | ||
| # Try to use Python script for proper quote-aware parsing | ||
| local script_dir | ||
| script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| local parse_script="$script_dir/parse_commands.py" | ||
|
|
||
| # Require Python parser — fail closed if unavailable (no insecure fallback) | ||
| if ! [[ -f "$parse_script" ]] || ! has_cmd python3; then | ||
| log_error "validate_command: python3 or parse_commands.py not available. Cannot securely validate command." | ||
| return 1 | ||
| fi | ||
|
|
||
| local base_commands_list | ||
| base_commands_list="$(python3 "$parse_script" "$cmd_string")" | ||
| if [[ $? -ne 0 || -z "$base_commands_list" ]]; then | ||
| log_error "validate_command: Python helper failed to parse command string." | ||
| return 1 | ||
| fi | ||
|
|
||
| # Check each base command | ||
| while IFS= read -r base_cmd; do | ||
| # Skip empty lines | ||
| [[ -z "$base_cmd" ]] && continue | ||
|
|
||
| # Skip flags/options (start with -) | ||
| [[ "$base_cmd" =~ ^- ]] && continue | ||
|
|
||
| # Strip surrounding quotes from command token before validation | ||
| # (shlex may return quoted tokens like "'cmd'" — strip to get bare command) | ||
| base_cmd="${base_cmd#\'}" | ||
| base_cmd="${base_cmd%\'}" | ||
| base_cmd="${base_cmd#\"}" | ||
| base_cmd="${base_cmd%\"}" | ||
| [[ -z "$base_cmd" ]] && continue | ||
|
|
||
| # Block interpreters with command execution flags (-c, -e) | ||
| # e.g., "bash -c 'rm -rf /'" — bash is in allowlist but -c allows arbitrary code | ||
| case "$base_cmd" in | ||
| bash|sh|zsh|python|python3|perl|ruby|node) | ||
| if [[ "$cmd_string" =~ [[:space:]]-[ce][[:space:]] ]] || [[ "$cmd_string" =~ [[:space:]]-[ce]$ ]]; then | ||
| log_error "validate_command: interpreter '$base_cmd' with -c or -e flag is not allowed" | ||
| return 1 | ||
| fi | ||
| ;; | ||
| esac | ||
|
|
||
| # Check allowlist first (allows script_commands like ./clawpinch.sh) | ||
| if grep -Fxq -- "$base_cmd" <<< "$allowed_commands"; then | ||
| continue | ||
| fi | ||
|
|
||
| # Block path-based commands not in allowlist (/bin/rm, ./malicious, ~/script) | ||
| if [[ "$base_cmd" =~ ^[/~\.] ]]; then | ||
| log_error "validate_command: path-based command '$base_cmd' is not in the allowlist" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Command not in allowlist | ||
| log_error "validate_command: '$base_cmd' is not in the allowlist" | ||
| return 1 | ||
| done <<< "$base_commands_list" | ||
|
|
||
| # All commands validated successfully | ||
| return 0 | ||
| } | ||
|
|
||
| # ─── OS detection ─────────────────────────────────────────────────────────── | ||
|
|
||
| detect_os() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
.auto-claude-security.jsonis missing,validate_command()will fail with error messages to stderr, but the script continues silently stripping all auto_fix commands. Users won't understand why auto-fix was removed from findings unless they notice the log_warn on line 306. Consider checking for config file existence before the loop and showing a clear setup message.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI