From 2f321fb2aaa75031e92a4e72a77fb0e1eab539ea Mon Sep 17 00:00:00 2001 From: arcker <3090078+arcker@users.noreply.github.com> Date: Tue, 6 Jan 2026 13:27:27 +0100 Subject: [PATCH] Fix: Security allowlist not working in worktree mode (#646) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: Security allowlist not working in worktree mode This fixes three related bugs that prevented .auto-claude-allowlist from working in isolated workspace (worktree) mode: 1. Security hook reads from wrong directory - Hook used os.getcwd() which returns main project dir, not worktree - Added AUTO_CLAUDE_PROJECT_DIR env var set by agent on startup - Files: security/hooks.py, agents/coder.py, qa/loop.py 2. Security profile cache doesn't track allowlist changes - Cache only tracked .auto-claude-security.json mtime - Now also tracks .auto-claude-allowlist mtime - File: security/profile.py 3. Allowlist not copied to worktree - .env files were copied but not security config files - Now copies both .auto-claude-allowlist and .auto-claude-security.json - File: core/workspace/setup.py Impact: Custom commands (cargo, dotnet, etc.) were always blocked in worktree mode even with proper allowlist configuration. Tested on Windows with Rust project (cargo commands). * Address Gemini Code Assist review comments - hooks.py: Add input_data.get("cwd") back to priority chain (HIGH) - coder.py: Move import os to top of file (MEDIUM) - loop.py: Move import os to top of file (MEDIUM) - profile.py: Remove redundant exists() check, catch FileNotFoundError (MEDIUM) - setup.py: Refactor security files copying with loop (MEDIUM) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * Add clarifying comment for security file overwrite behavior Addresses CodeRabbit review comment explaining why security files always overwrite (unlike env files) - prevents security bypasses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * docs: Add security commands configuration guide Explains the security system for command validation: - How automatic stack detection works - When and how to use .auto-claude-allowlist - Troubleshooting common issues - Worktree mode behavior This helps users understand why commands may be blocked and how to properly configure custom commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * Add error handling for security file copy Addresses CodeRabbit review: wrap shutil.copy2 in try/except to provide clear error messages instead of crashing on permission or disk space issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * docs: Fix markdown formatting nitpicks - Add 'text' language specifier to ASCII diagram code block - Add 'text' language specifier to allowlist example - Add blank line before code fence in troubleshooting section Addresses CodeRabbit trivial review comments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * refactor: Use shared constants for security filenames and env var Addresses Auto Claude PR Review findings: MEDIUM: - setup.py: Use ProjectAnalyzer.PROFILE_FILENAME and StructureAnalyzer.CUSTOM_ALLOWLIST_FILENAME instead of magic strings - profile.py: Use StructureAnalyzer.CUSTOM_ALLOWLIST_FILENAME LOW: - Create security/constants.py with PROJECT_DIR_ENV_VAR - Use constant in hooks.py, coder.py, loop.py - Expand worktree documentation to explain overwrite behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * refactor: Centralize security filenames in constants.py Move ALLOWLIST_FILENAME and PROFILE_FILENAME to security/constants.py for better cohesion. All security-related constants are now in one place. - setup.py: Import from security.constants - profile.py: Import from .constants (same module) Addresses CodeRabbit review suggestion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 * style: Simplify exception handling (FileNotFoundError is subclass of OSError) * style: Fix import sorting order (ruff I001) * style: fix ruff formatting issues - Add blank line after import inside function (hooks.py) - Split global statements onto separate lines (profile.py) - Reformat long if condition with `and` at line start (profile.py) - Break long print_status line (setup.py) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Arcker Co-authored-by: Claude Opus 4.5 Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com> --- apps/backend/agents/coder.py | 6 ++ apps/backend/core/workspace/setup.py | 29 +++++ apps/backend/qa/loop.py | 6 ++ apps/backend/security/constants.py | 16 +++ apps/backend/security/hooks.py | 17 ++- apps/backend/security/profile.py | 57 +++++++--- shared_docs/SECURITY_COMMANDS.md | 154 +++++++++++++++++++++++++++ 7 files changed, 270 insertions(+), 15 deletions(-) create mode 100644 apps/backend/security/constants.py create mode 100644 shared_docs/SECURITY_COMMANDS.md diff --git a/apps/backend/agents/coder.py b/apps/backend/agents/coder.py index 04cd019c..863aef1c 100644 --- a/apps/backend/agents/coder.py +++ b/apps/backend/agents/coder.py @@ -7,6 +7,7 @@ Main autonomous agent loop that runs the coder agent to implement subtasks. import asyncio import logging +import os from pathlib import Path from core.client import create_client @@ -37,6 +38,7 @@ from prompt_generator import ( ) from prompts import is_first_run from recovery import RecoveryManager +from security.constants import PROJECT_DIR_ENV_VAR from task_logger import ( LogPhase, get_task_logger, @@ -90,6 +92,10 @@ async def run_autonomous_agent( verbose: Whether to show detailed output source_spec_dir: Original spec directory in main project (for syncing from worktree) """ + # Set environment variable for security hooks to find the correct project directory + # This is needed because os.getcwd() may return the wrong directory in worktree mode + os.environ[PROJECT_DIR_ENV_VAR] = str(project_dir.resolve()) + # Initialize recovery manager (handles memory persistence) recovery_manager = RecoveryManager(spec_dir, project_dir) diff --git a/apps/backend/core/workspace/setup.py b/apps/backend/core/workspace/setup.py index 171866f1..6ae33f43 100644 --- a/apps/backend/core/workspace/setup.py +++ b/apps/backend/core/workspace/setup.py @@ -13,6 +13,7 @@ import sys from pathlib import Path from merge import FileTimelineTracker +from security.constants import ALLOWLIST_FILENAME, PROFILE_FILENAME from ui import ( Icons, MenuOption, @@ -267,6 +268,34 @@ def setup_workspace( f"Environment files copied: {', '.join(copied_env_files)}", "success" ) + # Copy security configuration files if they exist + # Note: Unlike env files, security files always overwrite to ensure + # the worktree uses the same security rules as the main project. + # This prevents security bypasses through stale worktree configs. + security_files = [ + ALLOWLIST_FILENAME, + PROFILE_FILENAME, + ] + security_files_copied = [] + + for filename in security_files: + source_file = project_dir / filename + if source_file.is_file(): + target_file = worktree_info.path / filename + try: + shutil.copy2(source_file, target_file) + security_files_copied.append(filename) + except (OSError, PermissionError) as e: + debug_warning(MODULE, f"Failed to copy {filename}: {e}") + print_status( + f"Warning: Could not copy {filename} to worktree", "warning" + ) + + if security_files_copied: + print_status( + f"Security config copied: {', '.join(security_files_copied)}", "success" + ) + # Ensure .auto-claude/ is in the worktree's .gitignore # This is critical because the worktree inherits .gitignore from the base branch, # which may not have .auto-claude/ if that change wasn't committed/pushed. diff --git a/apps/backend/qa/loop.py b/apps/backend/qa/loop.py index ff830869..fcbc1c7f 100644 --- a/apps/backend/qa/loop.py +++ b/apps/backend/qa/loop.py @@ -6,6 +6,7 @@ Main QA loop that coordinates reviewer and fixer sessions until approval or max iterations. """ +import os import time as time_module from pathlib import Path @@ -22,6 +23,7 @@ from linear_updater import ( from phase_config import get_phase_model, get_phase_thinking_budget from phase_event import ExecutionPhase, emit_phase from progress import count_subtasks, is_build_complete +from security.constants import PROJECT_DIR_ENV_VAR from task_logger import ( LogPhase, get_task_logger, @@ -83,6 +85,10 @@ async def run_qa_validation_loop( Returns: True if QA approved, False otherwise """ + # Set environment variable for security hooks to find the correct project directory + # This is needed because os.getcwd() may return the wrong directory in worktree mode + os.environ[PROJECT_DIR_ENV_VAR] = str(project_dir.resolve()) + debug_section("qa_loop", "QA Validation Loop") debug( "qa_loop", diff --git a/apps/backend/security/constants.py b/apps/backend/security/constants.py new file mode 100644 index 00000000..3ddbca30 --- /dev/null +++ b/apps/backend/security/constants.py @@ -0,0 +1,16 @@ +""" +Security Constants +================== + +Shared constants for the security module. +""" + +# Environment variable name for the project directory +# Set by agents (coder.py, loop.py) at startup to ensure security hooks +# can find the correct project directory even in worktree mode. +PROJECT_DIR_ENV_VAR = "AUTO_CLAUDE_PROJECT_DIR" + +# Security configuration filenames +# These are the files that control which commands are allowed to run. +ALLOWLIST_FILENAME = ".auto-claude-allowlist" +PROFILE_FILENAME = ".auto-claude-security.json" diff --git a/apps/backend/security/hooks.py b/apps/backend/security/hooks.py index b606c17c..4bc7328d 100644 --- a/apps/backend/security/hooks.py +++ b/apps/backend/security/hooks.py @@ -65,8 +65,21 @@ async def bash_security_hook( if not command: return {} - # Get the working directory from input_data (SDK passes it there, not in context) - cwd = input_data.get("cwd") or os.getcwd() + # Get the working directory from context or use current directory + # Priority: + # 1. Environment variable PROJECT_DIR_ENV_VAR (set by agent on startup) + # 2. input_data cwd (passed by SDK in the tool call) + # 3. Context cwd (should be set by ClaudeSDKClient but sometimes isn't) + # 4. Current working directory (fallback, may be incorrect in worktree mode) + from .constants import PROJECT_DIR_ENV_VAR + + cwd = os.environ.get(PROJECT_DIR_ENV_VAR) + if not cwd: + cwd = input_data.get("cwd") + if not cwd and context and hasattr(context, "cwd"): + cwd = context.cwd + if not cwd: + cwd = os.getcwd() # Get or create security profile # Note: In actual use, spec_dir would be passed through context diff --git a/apps/backend/security/profile.py b/apps/backend/security/profile.py index da75cff1..a3087a65 100644 --- a/apps/backend/security/profile.py +++ b/apps/backend/security/profile.py @@ -9,11 +9,12 @@ Uses project_analyzer to create dynamic security profiles based on detected stac from pathlib import Path from project_analyzer import ( - ProjectAnalyzer, SecurityProfile, get_or_create_profile, ) +from .constants import ALLOWLIST_FILENAME, PROFILE_FILENAME + # ============================================================================= # GLOBAL STATE # ============================================================================= @@ -23,18 +24,33 @@ _cached_profile: SecurityProfile | None = None _cached_project_dir: Path | None = None _cached_spec_dir: Path | None = None # Track spec directory for cache key _cached_profile_mtime: float | None = None # Track file modification time +_cached_allowlist_mtime: float | None = None # Track allowlist modification time def _get_profile_path(project_dir: Path) -> Path: """Get the security profile file path for a project.""" - return project_dir / ProjectAnalyzer.PROFILE_FILENAME + return project_dir / PROFILE_FILENAME + + +def _get_allowlist_path(project_dir: Path) -> Path: + """Get the allowlist file path for a project.""" + return project_dir / ALLOWLIST_FILENAME def _get_profile_mtime(project_dir: Path) -> float | None: """Get the modification time of the security profile file, or None if not exists.""" profile_path = _get_profile_path(project_dir) try: - return profile_path.stat().st_mtime if profile_path.exists() else None + return profile_path.stat().st_mtime + except OSError: + return None + + +def _get_allowlist_mtime(project_dir: Path) -> float | None: + """Get the modification time of the allowlist file, or None if not exists.""" + allowlist_path = _get_allowlist_path(project_dir) + try: + return allowlist_path.stat().st_mtime except OSError: return None @@ -49,6 +65,7 @@ def get_security_profile( - The project directory changes - The security profile file is created (was None, now exists) - The security profile file is modified (mtime changed) + - The allowlist file is created, modified, or deleted Args: project_dir: Project root directory @@ -57,7 +74,11 @@ def get_security_profile( Returns: SecurityProfile for the project """ - global _cached_profile, _cached_project_dir, _cached_spec_dir, _cached_profile_mtime + global _cached_profile + global _cached_project_dir + global _cached_spec_dir + global _cached_profile_mtime + global _cached_allowlist_mtime project_dir = Path(project_dir).resolve() resolved_spec_dir = Path(spec_dir).resolve() if spec_dir else None @@ -68,30 +89,40 @@ def get_security_profile( and _cached_project_dir == project_dir and _cached_spec_dir == resolved_spec_dir ): - # Check if file has been created or modified since caching - current_mtime = _get_profile_mtime(project_dir) - # Cache is valid if: - # - Both are None (file never existed and still doesn't) - # - Both have same mtime (file unchanged) - if current_mtime == _cached_profile_mtime: + # Check if files have been created or modified since caching + current_profile_mtime = _get_profile_mtime(project_dir) + current_allowlist_mtime = _get_allowlist_mtime(project_dir) + + # Cache is valid if both mtimes are unchanged + if ( + current_profile_mtime == _cached_profile_mtime + and current_allowlist_mtime == _cached_allowlist_mtime + ): return _cached_profile - # File was created or modified - invalidate cache - # (This happens when analyzer creates the file after agent starts) + # File was created, modified, or deleted - invalidate cache + # (This happens when analyzer creates the file after agent starts, + # or when user adds/updates the allowlist) # Analyze and cache _cached_profile = get_or_create_profile(project_dir, spec_dir) _cached_project_dir = project_dir _cached_spec_dir = resolved_spec_dir _cached_profile_mtime = _get_profile_mtime(project_dir) + _cached_allowlist_mtime = _get_allowlist_mtime(project_dir) return _cached_profile def reset_profile_cache() -> None: """Reset the cached profile (useful for testing or re-analysis).""" - global _cached_profile, _cached_project_dir, _cached_spec_dir, _cached_profile_mtime + global _cached_profile + global _cached_project_dir + global _cached_spec_dir + global _cached_profile_mtime + global _cached_allowlist_mtime _cached_profile = None _cached_project_dir = None _cached_spec_dir = None _cached_profile_mtime = None + _cached_allowlist_mtime = None diff --git a/shared_docs/SECURITY_COMMANDS.md b/shared_docs/SECURITY_COMMANDS.md new file mode 100644 index 00000000..1cade744 --- /dev/null +++ b/shared_docs/SECURITY_COMMANDS.md @@ -0,0 +1,154 @@ +# Security Commands Configuration + +Auto Claude uses a dynamic security system that controls which shell commands the AI agent can execute. This prevents potentially dangerous operations while allowing legitimate development commands. + +## How It Works + +```text +┌─────────────────────────────────────────────────────────────┐ +│ Command Validation │ +├─────────────────────────────────────────────────────────────┤ +│ 1. Base Commands (always allowed) │ +│ └── ls, cat, grep, git, echo, etc. │ +│ │ +│ 2. Auto-Detected Stack Commands │ +│ └── Analyzer detects Cargo.toml → adds cargo, rustc │ +│ └── Analyzer detects package.json → adds npm, node │ +│ │ +│ 3. Custom Allowlist (manual additions) │ +│ └── .auto-claude-allowlist file │ +└─────────────────────────────────────────────────────────────┘ +``` + +## Automatic Detection + +When you start a task, Auto Claude analyzes your project and automatically allows commands based on detected technologies: + +| Detected File | Commands Added | +|---------------|----------------| +| `Cargo.toml` | `cargo`, `rustc`, `rustup`, `rustfmt`, `cargo-clippy`, etc. | +| `package.json` | `npm`, `node`, `npx` | +| `yarn.lock` | `yarn` | +| `pnpm-lock.yaml` | `pnpm` | +| `pyproject.toml` | `python`, `pip`, `poetry`, `uv` | +| `go.mod` | `go` | +| `*.csproj` / `*.sln` | `dotnet` | +| `pubspec.yaml` | `dart`, `flutter`, `pub` | +| `Dockerfile` | `docker` | +| `docker-compose.yml` | `docker-compose` | +| `Makefile` | `make` | + +The full detection logic is in `apps/backend/project/stack_detector.py`. + +### Generated Profile + +The analyzer saves its results to `.auto-claude-security.json` in your project root: + +```json +{ + "base_commands": ["ls", "cat", "grep", "..."], + "stack_commands": ["cargo", "rustc", "rustup"], + "detected_stack": { + "languages": ["rust"], + "package_managers": ["cargo"], + "frameworks": [], + "databases": [] + }, + "project_hash": "abc123...", + "created_at": "2024-01-15T10:30:00" +} +``` + +This file is auto-generated. Don't edit it manually - it will be overwritten. + +## Custom Allowlist + +For commands that aren't auto-detected, create a `.auto-claude-allowlist` file in your project root: + +```text +# .auto-claude-allowlist +# One command per line, no comments on same line + +# Custom build tools +bazel +buck + +# Project-specific scripts +./scripts/deploy.sh + +# Additional tools +ansible +terraform +``` + +### When to Use the Allowlist + +Use `.auto-claude-allowlist` when: + +- Your project uses uncommon build tools (Bazel, Buck, Pants, etc.) +- You have custom scripts that need to be executable +- Auto-detection doesn't recognize your stack +- You're using bleeding-edge tools not yet in the detection system + +### Format + +- One command per line +- Lines starting with `#` are ignored +- Empty lines are ignored +- Use the base command name only (e.g., `cargo`, not `cargo build`) + +## Troubleshooting + +### "Command X is not allowed" + +1. **Check if it should be auto-detected:** + - Does your project have the expected config file? (e.g., `Cargo.toml` for Rust) + - Run in project root, not a subdirectory + +2. **Add to allowlist:** + + ```bash + echo "your-command" >> .auto-claude-allowlist + ``` + +3. **Force re-analysis** (if detection seems wrong): + - Delete `.auto-claude-security.json` + - Restart the task + +### Allowlist Changes Not Taking Effect + +The security profile cache updates automatically when: +- `.auto-claude-allowlist` is modified (mtime changes) +- `.auto-claude-security.json` is modified + +No restart required - changes apply on the next command. + +### Worktree Mode + +When using isolated worktrees, security files are automatically copied from your main project on each worktree setup. + +**Important:** Unlike environment files (which are only copied if missing), security files **always overwrite** existing files in the worktree. This ensures the worktree uses the same security rules as the main project, preventing security bypasses through stale configurations. + +This means: +- Changes to allowlist in the main project are reflected in new worktrees +- You cannot have different security rules per worktree (by design) +- If you need to test with different commands, modify the main project's allowlist + +## Security Considerations + +The allowlist system exists to prevent: +- Accidental `rm -rf /` or similar destructive commands +- Execution of unknown binaries +- Network operations with unrestricted tools + +Only add commands you trust and understand. + +## Adding Support for New Technologies + +If you're using a technology that should be auto-detected, consider contributing: + +1. Add detection logic to `apps/backend/project/stack_detector.py` +2. Add commands to `apps/backend/project/command_registry/languages.py` +3. Submit a PR! + +See existing detectors for examples.