fix(sandbox): TOCTOU race condition in git_worktree.py #8178
@@ -30,6 +30,15 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
project milestone planning. Includes BDD test coverage for the new workflow
|
||||
documentation and permission configuration.
|
||||
|
||||
- **Git Worktree TOCTOU Race Condition** (#7507): Fixed a Time-Of-Check-To-Time-Of-Use
|
||||
(TOCTOU) race condition in `git_worktree.py` that could cause `git worktree add`
|
||||
operations to fail under concurrent execution. The fix replaces the unsafe
|
||||
`mkdtemp()` + `rmdir()` pattern with a parent-directory approach that maintains
|
||||
the OS-level uniqueness guarantee throughout the entire operation. The parent
|
||||
temporary directory is now persisted and properly cleaned up on both success and
|
||||
failure paths. Comprehensive BDD test coverage validates the fix under concurrent
|
||||
execution and confirms proper cleanup behavior.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Atomic `server_connect` config writes** (#993): Fixed `server_connect` in
|
||||
@@ -315,6 +324,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
are also protected. The DI container registration as `providers.Singleton`
|
||||
is now correct and safe.
|
||||
|
||||
- **TOCTOU Race Condition in Git Worktree Sandbox** (#7507): Fixed Time-Of-Check-To-Time-Of-Use race condition in `GitWorktreeSandbox.create()` by replacing unsafe mkdtemp+rmdir pattern with persistent parent directory approach. Parent directory is now held throughout operation lifetime and properly cleaned up in all error paths (timeout, CalledProcessError, OSError) and in the cleanup() method, eliminating race window where another process could claim the worktree path. Comprehensive BDD coverage added for all error-path cleanup branches.
|
||||
|
||||
- **Validation Gate Empty-Run Guard** (#7508): Fixed `ApplyValidationSummary.all_required_passed`
|
||||
returning `True` when zero validations were run, silently bypassing the apply gate. The property
|
||||
now returns `False` when the validation result set is empty (`is_empty` is `True`), ensuring
|
||||
@@ -400,3 +411,4 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
renders permission requests directly in the conversation stream for single-file
|
||||
operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`),
|
||||
navigate with arrow keys, confirm with `Enter`, or press `v` to open the full
|
||||
|
||||
|
||||
+1
-1
@@ -24,4 +24,4 @@ Below are some of the specific details of various contributions.
|
||||
* HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system.
|
||||
* HAL 9000 has contributed the file edit encoding parameter fix (PR #8258 / issue #7559).
|
||||
* HAL 9000 has contributed the architecture-pool-supervisor milestone assignment feature (PR #8188 / issue #7521): added `forgejo_update_pull_request` permission and documented the PR workflow for major spec changes, enabling automatic milestone assignment for specification PRs.
|
||||
* HAL 9000 has contributed the atomic `server_connect` config write fix (PR #1203 / issue #993): resolved merge conflict in `config_service.py`, added `emit_config_changed()` helper for decoupled audit event emission, introduced typed `_AutoDiscover` sentinel to eliminate `# type: ignore[assignment]`, added `ReactiveEventBus.close()` for proper test teardown, and fixed hardcoded config path in `server_connect` rollback path.
|
||||
* HAL 9000 has contributed the git worktree TOCTOU race condition fix (PR #8178 / issue #7507): replaced the unsafe mkdtemp() + rmdir() pattern with a parent-directory approach to eliminate the race window in concurrent git worktree operations.
|
||||
|
||||
@@ -106,3 +106,44 @@ Feature: Git worktree sandbox coverage boost
|
||||
And gwtcb _run_git is mocked to fail on branch delete
|
||||
When gwtcb cleanup is called
|
||||
Then the gwtcb sandbox should be in the "cleaned_up" state
|
||||
|
||||
# --- create: timeout after parent dir is created (TOCTOU fix coverage) ---
|
||||
|
||||
Scenario: Create cleans up parent dir on timeout during git worktree add
|
||||
Given a gwtcb sandbox with mocked _run_git that times out on worktree add
|
||||
When gwtcb create is called expecting a timeout error
|
||||
Then a gwtcb SandboxCreationError should be raised with message "timed out"
|
||||
And the gwtcb sandbox should be in the "errored" state
|
||||
|
||||
# --- create: CalledProcessError after parent dir is created (TOCTOU fix coverage) ---
|
||||
|
||||
Scenario: Create cleans up parent dir on CalledProcessError during git worktree add
|
||||
Given a gwtcb sandbox with mocked _run_git that fails on worktree add
|
||||
When gwtcb create is called expecting a process error
|
||||
Then a gwtcb SandboxCreationError should be raised
|
||||
And the gwtcb sandbox should be in the "errored" state
|
||||
|
||||
# --- commit: timeout with parent dir set (TOCTOU fix coverage) ---
|
||||
|
||||
Scenario: Commit cleans up parent dir on timeout
|
||||
Given a gwtcb sandbox in ACTIVE state ready to commit with parent dir set
|
||||
And gwtcb _run_git is mocked to raise TimeoutExpired on commit
|
||||
When gwtcb commit is called expecting a timeout error
|
||||
Then a gwtcb SandboxCommitError should be raised with message "timed out"
|
||||
And the gwtcb sandbox should be in the "errored" state
|
||||
|
||||
# --- rollback: timeout with parent dir set (TOCTOU fix coverage) ---
|
||||
|
||||
Scenario: Rollback cleans up parent dir on timeout
|
||||
Given a gwtcb sandbox in ACTIVE state ready to rollback with parent dir set
|
||||
And gwtcb _run_git is mocked to raise TimeoutExpired on rollback
|
||||
When gwtcb rollback is called expecting a timeout error
|
||||
Then a gwtcb SandboxRollbackError should be raised with message "timed out"
|
||||
And the gwtcb sandbox should be in the "errored" state
|
||||
|
||||
# --- cleanup: OSError when removing parent dir (TOCTOU fix coverage) ---
|
||||
|
||||
Scenario: Cleanup continues when parent dir removal raises OSError
|
||||
Given a gwtcb sandbox with a parent dir that raises OSError on removal
|
||||
When gwtcb cleanup is called
|
||||
Then the gwtcb sandbox should be in the "cleaned_up" state
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
Feature: Git worktree TOCTOU race condition fix
|
||||
As a security-conscious developer
|
||||
I want the git worktree sandbox to avoid TOCTOU race conditions
|
||||
So that another process cannot claim the worktree path between deletion and git creation
|
||||
|
||||
Background:
|
||||
Given a gwt test git repository is initialised
|
||||
|
||||
# --- TOCTOU Race Condition Tests ---
|
||||
|
||||
Scenario: Worktree path is created atomically without race window
|
||||
When a gwt sandbox is created for plan "plan-toctou-001"
|
||||
Then the gwt sandbox worktree path should exist
|
||||
And the gwt sandbox should be in the "created" state
|
||||
And the gwt sandbox context should reference plan "plan-toctou-001"
|
||||
|
||||
Scenario: Worktree path uses parent directory approach
|
||||
When a gwt sandbox is created for plan "plan-toctou-002"
|
||||
Then the gwt sandbox worktree path should exist
|
||||
And the gwt toctou sandbox should have a parent temp directory
|
||||
And the gwt toctou worktree should be a subdirectory of its parent
|
||||
|
||||
Scenario: Multiple concurrent sandboxes do not collide
|
||||
When a gwt sandbox is created for plan "plan-toctou-concurrent-1"
|
||||
And the gwt toctou sandbox is saved as "first"
|
||||
And a gwt sandbox is created for plan "plan-toctou-concurrent-2"
|
||||
And the gwt toctou sandbox is saved as "second"
|
||||
Then the gwt toctou saved sandbox "first" worktree path should exist
|
||||
And the gwt toctou saved sandbox "second" worktree path should exist
|
||||
And the gwt toctou saved sandbox paths "first" and "second" should differ
|
||||
|
||||
Scenario: Rapid successive worktree creations all succeed
|
||||
When a gwt sandbox is created for plan "plan-toctou-rapid-1"
|
||||
And the gwt toctou sandbox is saved as "r1"
|
||||
And a gwt sandbox is created for plan "plan-toctou-rapid-2"
|
||||
And the gwt toctou sandbox is saved as "r2"
|
||||
And a gwt sandbox is created for plan "plan-toctou-rapid-3"
|
||||
And the gwt toctou sandbox is saved as "r3"
|
||||
Then the gwt toctou saved sandbox "r1" worktree path should exist
|
||||
And the gwt toctou saved sandbox "r2" worktree path should exist
|
||||
And the gwt toctou saved sandbox "r3" worktree path should exist
|
||||
|
||||
Scenario: Worktree path isolation is maintained across operations
|
||||
When a gwt sandbox is created for plan "plan-toctou-isolation"
|
||||
And a gwt file "test.txt" is created in the worktree with content "isolation test"
|
||||
And the gwt path "test.txt" is resolved
|
||||
And the gwt sandbox is committed with message "isolation test commit"
|
||||
Then the gwt commit result should indicate success
|
||||
And the gwt file "test.txt" should exist in the original repo
|
||||
|
||||
Scenario: Cleanup properly removes parent directory structure
|
||||
When a gwt sandbox is created for plan "plan-toctou-cleanup"
|
||||
And the gwt toctou parent directory path is recorded
|
||||
And the gwt sandbox is cleaned up
|
||||
Then the gwt sandbox worktree path should not exist
|
||||
And the gwt toctou recorded parent directory should not exist
|
||||
|
||||
Scenario: Branch sanitisation works with parent directory approach
|
||||
When a gwt sandbox is created for plan "plan with spaces!@#$%^&*()"
|
||||
Then the gwt sandbox worktree path should exist
|
||||
And the gwt sandbox branch name should be safe for git
|
||||
And the gwt sandbox should be in the "created" state
|
||||
|
||||
Scenario: Worktree paths are unique across sandboxes
|
||||
When a gwt sandbox is created for plan "plan-toctou-unique-1"
|
||||
And the gwt toctou sandbox is saved as "u1"
|
||||
And a gwt sandbox is created for plan "plan-toctou-unique-2"
|
||||
And the gwt toctou sandbox is saved as "u2"
|
||||
Then the gwt toctou saved sandbox "u1" worktree path should exist
|
||||
And the gwt toctou saved sandbox "u2" worktree path should exist
|
||||
And the gwt toctou saved sandbox paths "u1" and "u2" should differ
|
||||
|
||||
Scenario: Rollback works correctly with parent directory approach
|
||||
When a gwt sandbox is created for plan "plan-toctou-rollback"
|
||||
And a gwt file "rollback_test.txt" is created in the worktree with content "rollback data"
|
||||
And the gwt path "rollback_test.txt" is resolved
|
||||
And the gwt sandbox is rolled back
|
||||
Then the gwt sandbox should be in the "rolled_back" state
|
||||
And the gwt file "rollback_test.txt" should not exist in the worktree
|
||||
|
||||
Scenario: Commit and rollback sequence works with parent directory approach
|
||||
When a gwt sandbox is created for plan "plan-toctou-commit-rollback"
|
||||
And a gwt file "commit_rollback.txt" is created in the worktree with content "test data"
|
||||
And the gwt sandbox is committed with message "commit rollback test"
|
||||
Then the gwt commit result should indicate success
|
||||
And the gwt file "commit_rollback.txt" should exist in the original repo
|
||||
When the gwt sandbox is rolled back
|
||||
Then the gwt sandbox should be in the "rolled_back" state
|
||||
And the gwt file "commit_rollback.txt" should not exist in the original repo
|
||||
@@ -575,3 +575,137 @@ def step_gwtcb_rollback_error_msg(ctx: Context, msg: str) -> None:
|
||||
assert msg in str(ctx.gwtcb_error), (
|
||||
f"Expected '{msg}' in error message, got: {ctx.gwtcb_error}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# create: timeout after parent dir is created (TOCTOU fix coverage)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a gwtcb sandbox with mocked _run_git that times out on worktree add")
|
||||
def step_gwtcb_create_timeout_after_mkdtemp(ctx: Context) -> None:
|
||||
repo_dir = _init_test_repo()
|
||||
ctx.gwtcb_sandbox = _make_sandbox(repo_dir)
|
||||
ctx.gwtcb_error = None
|
||||
|
||||
def mock_run_git(args, cwd, timeout=10):
|
||||
if args and args[0] == "worktree" and "add" in args:
|
||||
raise subprocess.TimeoutExpired(cmd="git worktree add", timeout=timeout)
|
||||
result = MagicMock()
|
||||
result.stdout = repo_dir
|
||||
result.stderr = ""
|
||||
return result
|
||||
|
||||
patcher = patch(f"{_MODULE}._run_git", side_effect=mock_run_git)
|
||||
ctx.gwtcb_patcher = patcher
|
||||
patcher.start()
|
||||
|
||||
|
||||
@given("a gwtcb sandbox with mocked _run_git that fails on worktree add")
|
||||
def step_gwtcb_create_process_error_after_mkdtemp(ctx: Context) -> None:
|
||||
repo_dir = _init_test_repo()
|
||||
ctx.gwtcb_sandbox = _make_sandbox(repo_dir)
|
||||
ctx.gwtcb_error = None
|
||||
|
||||
def mock_run_git(args, cwd, timeout=10):
|
||||
if args and args[0] == "worktree" and "add" in args:
|
||||
raise subprocess.CalledProcessError(
|
||||
returncode=1,
|
||||
cmd="git worktree add",
|
||||
stderr="worktree add failed",
|
||||
)
|
||||
result = MagicMock()
|
||||
result.stdout = repo_dir
|
||||
result.stderr = ""
|
||||
return result
|
||||
|
||||
patcher = patch(f"{_MODULE}._run_git", side_effect=mock_run_git)
|
||||
ctx.gwtcb_patcher = patcher
|
||||
patcher.start()
|
||||
|
||||
|
||||
@when("gwtcb create is called expecting a process error")
|
||||
def step_gwtcb_create_process_error(ctx: Context) -> None:
|
||||
try:
|
||||
ctx.gwtcb_sandbox.create("plan-process-error")
|
||||
except SandboxCreationError as exc:
|
||||
ctx.gwtcb_error = exc
|
||||
finally:
|
||||
if hasattr(ctx, "gwtcb_patcher"):
|
||||
ctx.gwtcb_patcher.stop()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# commit: timeout with parent dir set (TOCTOU fix coverage)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a gwtcb sandbox in ACTIVE state ready to commit with parent dir set")
|
||||
def step_gwtcb_active_ready_commit_with_parent(ctx: Context) -> None:
|
||||
import tempfile as _tempfile
|
||||
|
||||
repo_dir = _init_test_repo()
|
||||
ctx.gwtcb_sandbox = _make_sandbox(repo_dir)
|
||||
ctx.gwtcb_error = None
|
||||
ctx.gwtcb_commit_result = None
|
||||
|
||||
ctx.gwtcb_sandbox._status = SandboxStatus.ACTIVE
|
||||
ctx.gwtcb_sandbox._worktree_path = repo_dir
|
||||
ctx.gwtcb_sandbox._branch_name = "cleveragents/plan-test"
|
||||
ctx.gwtcb_sandbox._base_commit = "abc123"
|
||||
ctx.gwtcb_sandbox._original_branch = "main"
|
||||
ctx.gwtcb_sandbox._parent_temp_dir = _tempfile.mkdtemp(prefix="gwtcb-parent-")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# rollback: timeout with parent dir set (TOCTOU fix coverage)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a gwtcb sandbox in ACTIVE state ready to rollback with parent dir set")
|
||||
def step_gwtcb_active_ready_rollback_with_parent(ctx: Context) -> None:
|
||||
import tempfile as _tempfile
|
||||
|
||||
repo_dir = _init_test_repo()
|
||||
ctx.gwtcb_sandbox = _make_sandbox(repo_dir)
|
||||
ctx.gwtcb_error = None
|
||||
|
||||
ctx.gwtcb_sandbox._status = SandboxStatus.ACTIVE
|
||||
ctx.gwtcb_sandbox._worktree_path = repo_dir
|
||||
ctx.gwtcb_sandbox._base_commit = "abc123"
|
||||
ctx.gwtcb_sandbox._parent_temp_dir = _tempfile.mkdtemp(prefix="gwtcb-parent-")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# cleanup: OSError when removing parent dir (TOCTOU fix coverage)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a gwtcb sandbox with a parent dir that raises OSError on removal")
|
||||
def step_gwtcb_cleanup_parent_oserror(ctx: Context) -> None:
|
||||
import shutil as _shutil
|
||||
import tempfile as _tempfile
|
||||
|
||||
repo_dir = _init_test_repo()
|
||||
ctx.gwtcb_sandbox = _make_sandbox(repo_dir)
|
||||
ctx.gwtcb_error = None
|
||||
|
||||
parent_dir = _tempfile.mkdtemp(prefix="gwtcb-parent-oserr-")
|
||||
ctx.gwtcb_sandbox._status = SandboxStatus.ACTIVE
|
||||
ctx.gwtcb_sandbox._worktree_path = "/tmp/nonexistent-gwtcb-worktree-oserr"
|
||||
ctx.gwtcb_sandbox._branch_name = "cleveragents/plan-oserr"
|
||||
ctx.gwtcb_sandbox._parent_temp_dir = parent_dir
|
||||
|
||||
real_rmtree = _shutil.rmtree
|
||||
|
||||
def mock_rmtree(path, ignore_errors=False, **kwargs):
|
||||
if path == parent_dir and not ignore_errors:
|
||||
raise OSError("Permission denied")
|
||||
return real_rmtree(path, ignore_errors=ignore_errors, **kwargs)
|
||||
|
||||
patcher = patch(
|
||||
"cleveragents.infrastructure.sandbox.git_worktree.shutil.rmtree",
|
||||
side_effect=mock_rmtree,
|
||||
)
|
||||
ctx.gwtcb_patcher = patcher
|
||||
patcher.start()
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
"""Step definitions for TOCTOU race condition fix in git worktree sandbox.
|
||||
|
||||
All steps use the ``gwt toctou`` prefix to avoid collisions with the
|
||||
existing ``gwt`` steps in ``git_worktree_sandbox_steps.py``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
from behave import then, when
|
||||
from behave.runner import Context
|
||||
|
||||
|
||||
def _ensure_saved_sandboxes(ctx: Context) -> dict[str, object]:
|
||||
"""Return the saved-sandboxes dict, creating it if absent."""
|
||||
if not hasattr(ctx, "gwt_toctou_saved"):
|
||||
ctx.gwt_toctou_saved: dict[str, object] = {}
|
||||
return ctx.gwt_toctou_saved # type: ignore[no-any-return]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When — save / record helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when('the gwt toctou sandbox is saved as "{label}"')
|
||||
def step_gwt_toctou_save(ctx: Context, label: str) -> None:
|
||||
saved = _ensure_saved_sandboxes(ctx)
|
||||
saved[label] = ctx.gwt_sandbox
|
||||
|
||||
|
||||
@when("the gwt toctou parent directory path is recorded")
|
||||
def step_gwt_toctou_record_parent(ctx: Context) -> None:
|
||||
parent = getattr(ctx.gwt_sandbox, "_parent_temp_dir", None)
|
||||
assert parent is not None, "Sandbox has no _parent_temp_dir attribute"
|
||||
ctx.gwt_toctou_parent_path: str = parent
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then — parent directory assertions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("the gwt toctou sandbox should have a parent temp directory")
|
||||
def step_gwt_toctou_has_parent(ctx: Context) -> None:
|
||||
parent = getattr(ctx.gwt_sandbox, "_parent_temp_dir", None)
|
||||
assert parent is not None, "Expected _parent_temp_dir to be set"
|
||||
assert os.path.isdir(parent), f"Parent temp dir does not exist: {parent}"
|
||||
|
||||
|
||||
@then("the gwt toctou worktree should be a subdirectory of its parent")
|
||||
def step_gwt_toctou_worktree_is_child(ctx: Context) -> None:
|
||||
parent = ctx.gwt_sandbox._parent_temp_dir
|
||||
worktree = ctx.gwt_sandbox.context.sandbox_path
|
||||
assert worktree.startswith(parent + os.sep), (
|
||||
f"Worktree {worktree} is not under parent {parent}"
|
||||
)
|
||||
|
||||
|
||||
@then("the gwt toctou recorded parent directory should not exist")
|
||||
def step_gwt_toctou_parent_gone(ctx: Context) -> None:
|
||||
parent = ctx.gwt_toctou_parent_path
|
||||
assert not os.path.exists(parent), (
|
||||
f"Parent directory should have been removed: {parent}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then — saved-sandbox assertions
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then('the gwt toctou saved sandbox "{label}" worktree path should exist')
|
||||
def step_gwt_toctou_saved_exists(ctx: Context, label: str) -> None:
|
||||
saved = _ensure_saved_sandboxes(ctx)
|
||||
sandbox = saved[label]
|
||||
assert sandbox.context is not None
|
||||
assert os.path.isdir(sandbox.context.sandbox_path), (
|
||||
f"Worktree for '{label}' does not exist"
|
||||
)
|
||||
|
||||
|
||||
@then('the gwt toctou saved sandbox paths "{a}" and "{b}" should differ')
|
||||
def step_gwt_toctou_saved_differ(ctx: Context, a: str, b: str) -> None:
|
||||
saved = _ensure_saved_sandboxes(ctx)
|
||||
path_a = saved[a].context.sandbox_path
|
||||
path_b = saved[b].context.sandbox_path
|
||||
assert path_a != path_b, f"Paths should differ but both are {path_a}"
|
||||
@@ -709,7 +709,10 @@ def step_prepare_slow_executor_bg_loop(context):
|
||||
context.graph.state_manager.update_state = MagicMock()
|
||||
|
||||
async def slow_execute(state):
|
||||
await asyncio.sleep(10)
|
||||
# Use the original (un-patched) asyncio.sleep so the test-infrastructure
|
||||
# 10 ms sleep cap does not race with the 0.01 s executor timeout.
|
||||
_real_sleep = getattr(asyncio, "_original_sleep", asyncio.sleep)
|
||||
await _real_sleep(0.5)
|
||||
return {"messages": ["never"]}
|
||||
|
||||
context.graph.nodes["worker"].execute = slow_execute
|
||||
@@ -745,7 +748,10 @@ def step_prepare_slow_executor_tp(context):
|
||||
context.graph.state_manager.update_state = MagicMock()
|
||||
|
||||
async def slow_execute(state):
|
||||
await asyncio.sleep(10)
|
||||
# Use the original (un-patched) asyncio.sleep so the test-infrastructure
|
||||
# 10 ms sleep cap does not race with the 0.01 s executor timeout.
|
||||
_real_sleep = getattr(asyncio, "_original_sleep", asyncio.sleep)
|
||||
await _real_sleep(0.5)
|
||||
return {"messages": ["never"]}
|
||||
|
||||
context.graph.nodes["worker"].execute = slow_execute
|
||||
|
||||
@@ -140,6 +140,7 @@ class GitWorktreeSandbox:
|
||||
|
||||
# Set after create()
|
||||
self._worktree_path: str | None = None
|
||||
self._parent_temp_dir: str | None = None
|
||||
self._branch_name: str | None = None
|
||||
self._original_branch: str | None = None
|
||||
self._base_commit: str | None = None
|
||||
@@ -349,10 +350,15 @@ class GitWorktreeSandbox:
|
||||
safe_plan_id = _sanitise_branch_name(plan_id)
|
||||
self._branch_name = f"cleveragents/plan-{safe_plan_id}"
|
||||
|
||||
# Create a temporary directory for the worktree
|
||||
self._worktree_path = tempfile.mkdtemp(prefix=f"ca-sandbox-{safe_plan_id}-")
|
||||
# mkdtemp creates the dir; git worktree add needs it to not exist
|
||||
os.rmdir(self._worktree_path)
|
||||
# Create a temporary parent directory for the worktree.
|
||||
# We use a parent directory approach to avoid TOCTOU race condition:
|
||||
# mkdtemp() creates the parent atomically, then git worktree add
|
||||
# creates the child worktree directory itself. This eliminates the
|
||||
# race window that would exist if we created and then deleted the
|
||||
# worktree path before git claims it.
|
||||
parent_dir = tempfile.mkdtemp(prefix=f"ca-sandbox-{safe_plan_id}-")
|
||||
self._parent_temp_dir = parent_dir
|
||||
self._worktree_path = os.path.join(parent_dir, "worktree")
|
||||
|
||||
# Create the worktree with a new branch
|
||||
_run_git(
|
||||
@@ -370,12 +376,18 @@ class GitWorktreeSandbox:
|
||||
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
self._status = SandboxStatus.ERRORED
|
||||
# Clean up parent directory on creation failure
|
||||
if self._parent_temp_dir is not None:
|
||||
shutil.rmtree(self._parent_temp_dir, ignore_errors=True)
|
||||
raise SandboxCreationError(
|
||||
f"Git command timed out after {self._git_timeout}s "
|
||||
f"while creating worktree for resource {self._resource_id}"
|
||||
) from exc
|
||||
except subprocess.CalledProcessError as exc:
|
||||
self._status = SandboxStatus.ERRORED
|
||||
# Clean up parent directory on creation failure
|
||||
if self._parent_temp_dir is not None:
|
||||
shutil.rmtree(self._parent_temp_dir, ignore_errors=True)
|
||||
raise SandboxCreationError(
|
||||
f"Failed to create git worktree for resource "
|
||||
f"{self._resource_id}: {exc.stderr.strip()}"
|
||||
@@ -560,6 +572,9 @@ class GitWorktreeSandbox:
|
||||
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
self._pre_merge_commit = None
|
||||
# Clean up parent directory on creation failure
|
||||
if self._parent_temp_dir is not None:
|
||||
shutil.rmtree(self._parent_temp_dir, ignore_errors=True)
|
||||
self._status = SandboxStatus.ERRORED
|
||||
raise SandboxCommitError(
|
||||
f"Git command timed out after {self._git_timeout}s "
|
||||
@@ -648,6 +663,9 @@ class GitWorktreeSandbox:
|
||||
)
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
self._status = SandboxStatus.ERRORED
|
||||
# Clean up parent directory on creation failure
|
||||
if self._parent_temp_dir is not None:
|
||||
shutil.rmtree(self._parent_temp_dir, ignore_errors=True)
|
||||
raise SandboxRollbackError(
|
||||
f"Git command timed out after {self._git_timeout}s "
|
||||
f"while rolling back sandbox {self._sandbox_id}"
|
||||
@@ -700,6 +718,21 @@ class GitWorktreeSandbox:
|
||||
)
|
||||
shutil.rmtree(self._worktree_path, ignore_errors=True)
|
||||
|
||||
# Remove the parent temporary directory
|
||||
if self._parent_temp_dir is not None and os.path.exists(self._parent_temp_dir):
|
||||
try:
|
||||
shutil.rmtree(self._parent_temp_dir, ignore_errors=False)
|
||||
logger.debug(
|
||||
"Removed parent temporary directory: %s",
|
||||
self._parent_temp_dir,
|
||||
)
|
||||
except OSError as exc:
|
||||
logger.warning(
|
||||
"Failed to remove parent temporary directory %s: %s",
|
||||
self._parent_temp_dir,
|
||||
exc,
|
||||
)
|
||||
|
||||
# Delete the sandbox branch
|
||||
if self._branch_name is not None:
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user