fix: add worktree isolation warnings to prevent agent escape (ACS-394) (#1495)
* fix: add worktree isolation warnings to prevent agent escape (ACS-394) Agents were escaping isolated worktrees by using `cd` to navigate to parent project paths, causing git commits to go to the wrong branch and files to be created/modified in the wrong location. This fix adds: - detect_worktree_mode() function to detect worktree isolation - Worktree isolation warning section injected into agent prompts - Explicit forbidden parent path display to prevent `cd` commands - Comprehensive test coverage (11 tests) for Unix/Windows paths Modified files: - apps/backend/prompts_pkg/prompt_generator.py - apps/backend/prompts/coder.md - apps/backend/prompts/qa_fixer.md New files: - tests/test_prompt_generator.py Refs: ACS-394 * refactor: simplify worktree detection and address PR review comments - Normalize path separators to forward slashes for consistent matching - Use split with maxsplit=1 instead of chained split calls - Extract path patterns to named constants for clarity - Remove unused pytest import from test file Addresses review comments from: - GitHub Advanced Security: unused import - Gemini code-assist: simplify path splitting logic All 11 tests continue to pass. * refactor: address Auto Claude PR review findings - Remove redundant sys.path.insert (conftest.py already handles this) - Remove hardcoded directory examples from worktree warning - Update test to verify warning content without specific paths - Simplify worktree isolation warning to be project-agnostic Addresses Auto Claude PR Review findings: - [LOW] Redundant sys.path.insert - removed - [LOW] Hardcoded directory examples - made generic All 11 tests pass. * refactor: remove unused project_dir parameter from detect_worktree_mode The project_dir parameter was accepted but never used in the function body. Worktree detection only examines spec_dir to detect worktree path patterns. Updated: - Function signature to remove unused parameter - Docstring to remove parameter documentation - Call site in generate_environment_context() - All 6 test calls Addresses CodeRabbitAI review comment. All 11 tests pass. --------- Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com> Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
This commit is contained in:
StillKnotKnown
@@ -84,6 +84,65 @@ git add [verified-path]
|
||||
|
||||
---
|
||||
|
||||
## 🚨 CRITICAL: WORKTREE ISOLATION 🚨
|
||||
|
||||
**You may be in an ISOLATED GIT WORKTREE environment.**
|
||||
|
||||
Check the "YOUR ENVIRONMENT" section at the top of this prompt. If you see an
|
||||
**"ISOLATED WORKTREE - CRITICAL"** section, you are in a worktree.
|
||||
|
||||
### What is a Worktree?
|
||||
|
||||
A worktree is a **complete copy of the project** isolated from the main project.
|
||||
This allows safe development without affecting the main branch.
|
||||
|
||||
### Worktree Rules (CRITICAL)
|
||||
|
||||
**If you are in a worktree, the environment section will show:**
|
||||
|
||||
* **YOUR LOCATION:** The path to your isolated worktree
|
||||
* **FORBIDDEN:** The parent project path you must NEVER `cd` to
|
||||
|
||||
**CRITICAL RULES:**
|
||||
* **NEVER** `cd` to the forbidden parent path
|
||||
* **NEVER** use `cd ../..` to escape the worktree
|
||||
* **STAY** within your working directory at all times
|
||||
* **ALL** file operations use paths relative to your current location
|
||||
|
||||
### Why This Matters
|
||||
|
||||
Escaping the worktree causes:
|
||||
* ❌ Git commits going to the wrong branch
|
||||
* ❌ Files created/modified in the wrong location
|
||||
* ❌ Breaking worktree isolation guarantees
|
||||
* ❌ Losing the safety of isolated development
|
||||
|
||||
### How to Stay Safe
|
||||
|
||||
**Before ANY `cd` command:**
|
||||
|
||||
```bash
|
||||
# 1. Check where you are
|
||||
pwd
|
||||
|
||||
# 2. Verify the target is within your worktree
|
||||
# If pwd shows: /path/to/.auto-claude/worktrees/tasks/spec-name/
|
||||
# Then: cd ./apps/backend ✅ SAFE
|
||||
# But: cd /path/to/parent/project ❌ FORBIDDEN - ESCAPES ISOLATION
|
||||
|
||||
# 3. When in doubt, don't use cd at all
|
||||
# Use relative paths from your current directory instead
|
||||
git add ./apps/backend/file.py # Works from anywhere in worktree
|
||||
```
|
||||
|
||||
### The Golden Rule in Worktrees
|
||||
|
||||
**If you're in a worktree, pretend the parent project doesn't exist.**
|
||||
|
||||
Everything you need is in your worktree, accessible via relative paths.
|
||||
|
||||
---
|
||||
|
||||
## STEP 1: GET YOUR BEARINGS (MANDATORY)
|
||||
|
||||
First, check your environment. The prompt should tell you your working directory and spec location.
|
||||
|
||||
@@ -142,6 +142,65 @@ git add [verified-path]
|
||||
|
||||
---
|
||||
|
||||
## 🚨 CRITICAL: WORKTREE ISOLATION 🚨
|
||||
|
||||
**You may be in an ISOLATED GIT WORKTREE environment.**
|
||||
|
||||
Check the "YOUR ENVIRONMENT" section at the top of this prompt. If you see an
|
||||
**"ISOLATED WORKTREE - CRITICAL"** section, you are in a worktree.
|
||||
|
||||
### What is a Worktree?
|
||||
|
||||
A worktree is a **complete copy of the project** isolated from the main project.
|
||||
This allows safe development without affecting the main branch.
|
||||
|
||||
### Worktree Rules (CRITICAL)
|
||||
|
||||
**If you are in a worktree, the environment section will show:**
|
||||
|
||||
* **YOUR LOCATION:** The path to your isolated worktree
|
||||
* **FORBIDDEN:** The parent project path you must NEVER `cd` to
|
||||
|
||||
**CRITICAL RULES:**
|
||||
* **NEVER** `cd` to the forbidden parent path
|
||||
* **NEVER** use `cd ../..` to escape the worktree
|
||||
* **STAY** within your working directory at all times
|
||||
* **ALL** file operations use paths relative to your current location
|
||||
|
||||
### Why This Matters
|
||||
|
||||
Escaping the worktree causes:
|
||||
* ❌ Git commits going to the wrong branch
|
||||
* ❌ Files created/modified in the wrong location
|
||||
* ❌ Breaking worktree isolation guarantees
|
||||
* ❌ Losing the safety of isolated development
|
||||
|
||||
### How to Stay Safe
|
||||
|
||||
**Before ANY `cd` command:**
|
||||
|
||||
```bash
|
||||
# 1. Check where you are
|
||||
pwd
|
||||
|
||||
# 2. Verify the target is within your worktree
|
||||
# If pwd shows: /path/to/.auto-claude/worktrees/tasks/spec-name/
|
||||
# Then: cd ./apps/backend ✅ SAFE
|
||||
# But: cd /path/to/parent/project ❌ FORBIDDEN - ESCAPES ISOLATION
|
||||
|
||||
# 3. When in doubt, don't use cd at all
|
||||
# Use relative paths from your current directory instead
|
||||
git add ./apps/backend/file.py # Works from anywhere in worktree
|
||||
```
|
||||
|
||||
### The Golden Rule in Worktrees
|
||||
|
||||
**If you're in a worktree, pretend the parent project doesn't exist.**
|
||||
|
||||
Everything you need is in your worktree, accessible via relative paths.
|
||||
|
||||
---
|
||||
|
||||
## PHASE 3: FIX ISSUES ONE BY ONE
|
||||
|
||||
For each issue in the fix request:
|
||||
|
||||
@@ -16,6 +16,37 @@ import json
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def detect_worktree_mode(spec_dir: Path) -> tuple[bool, str | None]:
|
||||
"""
|
||||
Detect if running in isolated worktree mode.
|
||||
|
||||
Args:
|
||||
spec_dir: Absolute path to spec directory
|
||||
|
||||
Returns:
|
||||
(is_worktree, forbidden_parent_path) tuple:
|
||||
- is_worktree: True if running in a worktree
|
||||
- forbidden_parent_path: The parent project path to forbid, or None
|
||||
"""
|
||||
# Check if spec_dir contains worktree path patterns
|
||||
# Normalize path separators to forward slashes for consistent matching
|
||||
spec_str = str(spec_dir).replace("\\", "/")
|
||||
|
||||
# New worktree location: .auto-claude/worktrees/tasks/{spec-name}/
|
||||
new_worktree_marker = "/.auto-claude/worktrees/tasks/"
|
||||
if new_worktree_marker in spec_str:
|
||||
parent_path = spec_str.split(new_worktree_marker, 1)[0]
|
||||
return True, parent_path
|
||||
|
||||
# Legacy worktree location: .worktrees/{spec-name}/
|
||||
legacy_worktree_marker = "/.worktrees/"
|
||||
if legacy_worktree_marker in spec_str:
|
||||
parent_path = spec_str.split(legacy_worktree_marker, 1)[0]
|
||||
return True, parent_path
|
||||
|
||||
return False, None
|
||||
|
||||
|
||||
def get_relative_spec_path(spec_dir: Path, project_dir: Path) -> str:
|
||||
"""
|
||||
Get the spec directory path relative to the project/working directory.
|
||||
@@ -54,7 +85,11 @@ def generate_environment_context(project_dir: Path, spec_dir: Path) -> str:
|
||||
"""
|
||||
relative_spec = get_relative_spec_path(spec_dir, project_dir)
|
||||
|
||||
return f"""## YOUR ENVIRONMENT
|
||||
# Detect worktree mode and get forbidden parent path
|
||||
is_worktree, forbidden_parent = detect_worktree_mode(spec_dir)
|
||||
|
||||
# Build the environment context
|
||||
context = f"""## YOUR ENVIRONMENT
|
||||
|
||||
**Working Directory:** `{project_dir}`
|
||||
**Spec Location:** `{relative_spec}/`
|
||||
@@ -77,6 +112,34 @@ coder prompt for detailed examples.
|
||||
|
||||
"""
|
||||
|
||||
# Add worktree isolation warning if in worktree mode
|
||||
if is_worktree and forbidden_parent:
|
||||
context += f"""## 🚨 ISOLATED WORKTREE - CRITICAL
|
||||
|
||||
You are in an **ISOLATED GIT WORKTREE** - a complete copy of the project.
|
||||
|
||||
**YOUR LOCATION:** `{project_dir}`
|
||||
**FORBIDDEN:** Do NOT use `cd {forbidden_parent}` or `cd ../..` - this **ESCAPES ISOLATION**
|
||||
|
||||
All project files exist HERE via relative paths from your working directory.
|
||||
|
||||
**CRITICAL RULES:**
|
||||
* **NEVER** `cd {forbidden_parent}` or any path traversal outside your worktree
|
||||
* **STAY** within your working directory at all times
|
||||
* **ALL** file operations use paths relative to your current location
|
||||
* Before any `cd` command, run `pwd` and verify the target is within your worktree
|
||||
|
||||
**VIOLATION WARNING:** Escaping the worktree will cause:
|
||||
* Git commits going to wrong branch
|
||||
* Files created/modified in the wrong location
|
||||
* Breaking worktree isolation guarantees
|
||||
|
||||
---
|
||||
|
||||
"""
|
||||
|
||||
return context
|
||||
|
||||
|
||||
def generate_subtask_prompt(
|
||||
spec_dir: Path,
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
"""
|
||||
Tests for prompt_generator module functions.
|
||||
|
||||
Tests for worktree detection and environment context generation.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
# Note: sys.path manipulation is handled by conftest.py line 46
|
||||
|
||||
from prompts_pkg.prompt_generator import detect_worktree_mode, generate_environment_context
|
||||
|
||||
|
||||
class TestDetectWorktreeMode:
|
||||
"""Tests for detect_worktree_mode function."""
|
||||
|
||||
def test_new_worktree_unix_path(self):
|
||||
"""Test detection of new worktree location on Unix."""
|
||||
# New worktree: /project/.auto-claude/worktrees/tasks/spec-name/.auto-claude/specs/spec/
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
||||
|
||||
is_worktree, forbidden = detect_worktree_mode(spec_dir)
|
||||
|
||||
assert is_worktree is True
|
||||
assert forbidden == "/opt/dev/project"
|
||||
|
||||
def test_new_worktree_windows_path(self):
|
||||
"""Test detection of new worktree location on Windows."""
|
||||
# Windows path with backslashes
|
||||
spec_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit/.auto-claude/specs/009-audit")
|
||||
project_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit")
|
||||
|
||||
is_worktree, forbidden = detect_worktree_mode(spec_dir)
|
||||
|
||||
assert is_worktree is True
|
||||
assert forbidden == "E:/projects/x"
|
||||
|
||||
def test_legacy_worktree_unix_path(self):
|
||||
"""Test detection of legacy worktree location on Unix."""
|
||||
# Legacy worktree: /project/.worktrees/spec-name/.auto-claude/specs/spec/
|
||||
spec_dir = Path("/opt/dev/project/.worktrees/001-feature/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project/.worktrees/001-feature")
|
||||
|
||||
is_worktree, forbidden = detect_worktree_mode(spec_dir)
|
||||
|
||||
assert is_worktree is True
|
||||
assert forbidden == "/opt/dev/project"
|
||||
|
||||
def test_legacy_worktree_windows_path(self):
|
||||
"""Test detection of legacy worktree location on Windows."""
|
||||
spec_dir = Path("C:/projects/x/.worktrees/009-audit/.auto-claude/specs/009-audit")
|
||||
project_dir = Path("C:/projects/x/.worktrees/009-audit")
|
||||
|
||||
is_worktree, forbidden = detect_worktree_mode(spec_dir)
|
||||
|
||||
assert is_worktree is True
|
||||
assert forbidden == "C:/projects/x"
|
||||
|
||||
def test_not_in_worktree(self):
|
||||
"""Test when not in a worktree (direct mode)."""
|
||||
# Direct mode: /project/.auto-claude/specs/spec/
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project")
|
||||
|
||||
is_worktree, forbidden = detect_worktree_mode(spec_dir)
|
||||
|
||||
assert is_worktree is False
|
||||
assert forbidden is None
|
||||
|
||||
def test_deeply_nested_worktree(self):
|
||||
"""Test worktree detection with deeply nested spec directory."""
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/009-very-long-spec-name-for-testing/.auto-claude/specs/009-very-long-spec-name-for-testing")
|
||||
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/009-very-long-spec-name-for-testing")
|
||||
|
||||
is_worktree, forbidden = detect_worktree_mode(spec_dir)
|
||||
|
||||
assert is_worktree is True
|
||||
assert forbidden == "/opt/dev/project"
|
||||
|
||||
|
||||
class TestGenerateEnvironmentContext:
|
||||
"""Tests for generate_environment_context function."""
|
||||
|
||||
def test_context_includes_worktree_warning(self):
|
||||
"""Test that worktree isolation warning is included when in worktree."""
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
||||
|
||||
context = generate_environment_context(project_dir, spec_dir)
|
||||
|
||||
# Verify worktree warning is present
|
||||
assert "ISOLATED WORKTREE - CRITICAL" in context
|
||||
assert "FORBIDDEN:" in context
|
||||
assert "/opt/dev/project" in context
|
||||
assert "ESCAPES ISOLATION" in context
|
||||
|
||||
def test_context_no_worktree_warning_in_direct_mode(self):
|
||||
"""Test that worktree warning is NOT included in direct mode."""
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project")
|
||||
|
||||
context = generate_environment_context(project_dir, spec_dir)
|
||||
|
||||
# Verify worktree warning is NOT present
|
||||
assert "ISOLATED WORKTREE - CRITICAL" not in context
|
||||
assert "FORBIDDEN:" not in context
|
||||
|
||||
def test_context_includes_basic_environment(self):
|
||||
"""Test that basic environment information is always included."""
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project")
|
||||
|
||||
context = generate_environment_context(project_dir, spec_dir)
|
||||
|
||||
# Verify basic sections
|
||||
assert "## YOUR ENVIRONMENT" in context
|
||||
assert "**Working Directory:**" in context
|
||||
assert "**Spec Location:**" in context
|
||||
assert "implementation_plan.json" in context
|
||||
assert "PATH CONFUSION PREVENTION" in context
|
||||
|
||||
def test_context_windows_worktree(self):
|
||||
"""Test worktree warning with Windows paths (from ticket ACS-394)."""
|
||||
# This is the exact scenario from the bug report
|
||||
spec_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit/.auto-claude/specs/009-audit")
|
||||
project_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit")
|
||||
|
||||
context = generate_environment_context(project_dir, spec_dir)
|
||||
|
||||
# Verify worktree warning includes the Windows path
|
||||
assert "ISOLATED WORKTREE - CRITICAL" in context
|
||||
assert "E:/projects/x" in context
|
||||
assert "cd E:/projects/x" in context or '"cd E:/projects/x"' in context
|
||||
|
||||
def test_context_forbidden_path_examples(self):
|
||||
"""Test that forbidden path is shown and critical rules are included."""
|
||||
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature")
|
||||
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
||||
|
||||
context = generate_environment_context(project_dir, spec_dir)
|
||||
|
||||
# Verify forbidden parent path is shown
|
||||
assert "FORBIDDEN:" in context
|
||||
assert "/opt/dev/project" in context # The parent project that is forbidden
|
||||
|
||||
# Verify CRITICAL RULES section exists
|
||||
assert "**CRITICAL RULES:**" in context
|
||||
assert "**NEVER**" in context # Explicit prohibition
|
||||
|
||||
# Verify violation warning explains consequences
|
||||
assert "**VIOLATION WARNING:**" in context
|
||||
assert "Git commits going to wrong branch" in context
|
||||
Reference in New Issue
Block a user