Fix: Security allowlist not working in worktree mode (#646)

* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Arcker <Arcker@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
2026-01-06 13:27:27 +01:00
parent df57fbf8bc
commit 2f321fb2aa
7 changed files with 270 additions and 15 deletions
@@ -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)

@@ -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.
@@ -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",
@@ -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"
@@ -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
@@ -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
@@ -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.