CHANGELOG.md

@@ -78,6 +78,19 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **git_tools._get_base_env() TOCTOU Race Condition** (#7619): Fixed a
+  Time-Of-Check-To-Time-Of-Use race condition in `git_tools._get_base_env()`
+  where two concurrent threads could both observe `_BASE_ENV is None`, both
+  snapshot `os.environ`, and write potentially different snapshots. The fix
+  adds a module-level `_BASE_ENV_LOCK: threading.Lock` and replaces the bare
+  `if _BASE_ENV is None` assignment with double-checked locking: the outer
+  check keeps the warm-cache path lock-free; the inner check inside
+  `with _BASE_ENV_LOCK` prevents duplicate initialisation on the very first
+  concurrent call. Three new BDD scenarios in `features/git_tools.feature`
+  (with step definitions in
+  `features/steps/git_tools_thread_safety_steps.py`) verify caching identity,
+  content correctness, and thread safety under 20 concurrent threads.
+
 - **`create_llm()` raises `Unsupported provider type: openrouter`** (#10948): Fixed
   `ProviderRegistry._create_provider_llm()` missing an `OPENROUTER` branch, which
   caused `agents actor run openrouter/<model>` to fail with `ValueError`. Added a
@@ -87,14 +100,6 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   `default_headers` kwarg with automatic string coercion for non-string keys/values.
 
 - **LoadingThrobber Widget Restored** (#6357): Restored `LoadingThrobber` widget
-  (`src/cleveragents/tui/widgets/throbber.py`) and its Robot Framework integration
-  tests (`robot/tui_throbber.robot`) that were missing from master. Also restored
-  supporting modules `src/cleveragents/tui/quotes.py` and
-  `src/cleveragents/tui/data/throbber_quotes.txt`. Fixed the `Throbber Rejects
-  Invalid Styles` integration test by using `Fix Python Indentation` to correctly
-  reconstruct indentation stripped by Robot Framework's `Catenate` keyword.
-  Narrowed exception handling in `throbber.py` to specific types
-  (`ImportError`, `AttributeError`) per coding standards.
 
 - **Built-in actors v3 YAML format** (#10883): Fixed `agents actor run` failing for
   built-in actors (e.g., `openai/gpt-4`, `anthropic/claude-3-opus`) due to missing

CONTRIBUTORS.md

@@ -25,4 +25,7 @@ Below are some of the specific details of various contributions.
 * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.
 * 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 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.
+* HAL 9000 has contributed the git_tools TOCTOU race condition fix (PR #8255 / issue #7619): eliminated the Time-Of-Check-To-Time-Of-Use race in `_get_base_env()` by adding double-checked locking with a module-level `threading.Lock`, preventing concurrent threads from writing conflicting environment snapshots.
 * HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration.

features/git_tools.feature

@@ -267,3 +267,19 @@ Feature: Built-in Git Tools
     Given a tool registry
     When I register all git tools
     Then all registered git tools should be read-only
+
+  # ---- Thread Safety (_get_base_env TOCTOU fix) ----
+
+  Scenario: _get_base_env returns the same dict object on repeated calls
+    When I call _get_base_env twice
+    Then both calls should return the same dict object
+
+  Scenario: _get_base_env returns a dict containing git override keys
+    When I call _get_base_env
+    Then the result should contain key "GIT_PAGER"
+    And the result should contain key "NO_COLOR"
+    And the result should contain key "GIT_TERMINAL_PROMPT"
+
+  Scenario: _get_base_env is safe under concurrent initialisation
+    When I call _get_base_env concurrently from 20 threads
+    Then all threads should receive the same dict object

features/steps/git_tools_thread_safety_steps.py

@@ -0,0 +1,106 @@
+"""Step definitions for _get_base_env thread-safety (TOCTOU fix) scenarios.
+
+These steps exercise the double-checked locking pattern introduced in
+``git_tools._get_base_env()`` to eliminate the TOCTOU race condition
+where two concurrent threads could both observe ``_BASE_ENV is None``
+and write potentially different snapshots.
+"""
+
+from __future__ import annotations
+
+import threading
+from typing import Any
+
+import cleveragents.tool.builtins.git_tools as _git_tools_mod
+from behave import then, when
+from cleveragents.tool.builtins.git_tools import _get_base_env
+
+__all__: list[str] = []
+
+
+# ---------------------------------------------------------------------------
+# Whens
+# ---------------------------------------------------------------------------
+
+
+@when("I call _get_base_env twice")
+def step_when_call_get_base_env_twice(context: Any) -> None:
+    # Reset the module-level cache so the test is independent of call order.
+    original = _git_tools_mod._BASE_ENV
+    _git_tools_mod._BASE_ENV = None
+    try:
+        context.base_env_first = _get_base_env()
+        context.base_env_second = _get_base_env()
+    finally:
+        _git_tools_mod._BASE_ENV = original
+
+
+@when("I call _get_base_env")
+def step_when_call_get_base_env(context: Any) -> None:
+    original = _git_tools_mod._BASE_ENV
+    _git_tools_mod._BASE_ENV = None
+    try:
+        context.base_env_result = _get_base_env()
+    finally:
+        _git_tools_mod._BASE_ENV = original
+
+
+@when("I call _get_base_env concurrently from 20 threads")
+def step_when_call_get_base_env_concurrent(context: Any) -> None:
+    n_threads = 20
+    results: list[dict[str, str] | None] = [None] * n_threads
+    barrier = threading.Barrier(n_threads)
+
+    # Reset the module-level cache so all threads race on first initialisation.
+    original = _git_tools_mod._BASE_ENV
+    _git_tools_mod._BASE_ENV = None
+
+    def worker(idx: int) -> None:
+        barrier.wait()  # synchronise all threads to maximise race probability
+        results[idx] = _get_base_env()
+
+    threads = [threading.Thread(target=worker, args=(i,)) for i in range(n_threads)]
+    try:
+        for t in threads:
+            t.start()
+        for t in threads:
+            t.join(timeout=10)
+    finally:
+        _git_tools_mod._BASE_ENV = original
+
+    context.concurrent_results = results
+
+
+# ---------------------------------------------------------------------------
+# Thens
+# ---------------------------------------------------------------------------
+
+
+@then("both calls should return the same dict object")
+def step_then_same_dict_object(context: Any) -> None:
+    assert context.base_env_first is context.base_env_second, (
+        "_get_base_env() returned different dict objects on repeated calls; "
+        "caching is broken"
+    )
+
+
+@then('the result should contain key "{key}"')
+def step_then_result_contains_key(context: Any, key: str) -> None:
+    assert key in context.base_env_result, (
+        f"Expected key '{key}' in _get_base_env() result, "
+        f"got keys: {list(context.base_env_result.keys())}"
+    )
+
+
+@then("all threads should receive the same dict object")
+def step_then_all_threads_same_object(context: Any) -> None:
+    results = context.concurrent_results
+    assert all(r is not None for r in results), (
+        "Some threads did not receive a result from _get_base_env()"
+    )
+    first = results[0]
+    for i, r in enumerate(results[1:], start=1):
+        assert r is first, (
+            f"Thread {i} received a different dict object than thread 0; "
+            "double-checked locking is broken"
+        )

src/cleveragents/tool/builtins/git_tools.py

@@ -37,6 +37,7 @@ from __future__ import annotations
 
 import os
 import subprocess
+import threading
 from pathlib import Path
 from typing import Any
 
@@ -65,16 +66,30 @@ _GIT_ENV: dict[str, str] = {
 # Cached merge of ``os.environ`` with ``_GIT_ENV``, populated on first use.
 _BASE_ENV: dict[str, str] | None = None
 
+# Lock that serialises the one-time initialisation of ``_BASE_ENV``.
+# The outer ``if _BASE_ENV is None`` check keeps the warm-cache path
+# lock-free; the inner check inside the ``with`` block prevents duplicate
+# initialisation when two threads race on the very first call.
+_BASE_ENV_LOCK: threading.Lock = threading.Lock()
+
 
 def _get_base_env() -> dict[str, str]:
     """Return a cached merge of the process environment with git overrides.
 
     The snapshot is taken on first call and reused thereafter, avoiding a
     full ``os.environ`` copy on every git invocation.
+
+    Thread safety: double-checked locking eliminates the TOCTOU race where
+    two concurrent threads could both observe ``_BASE_ENV is None``, both
+    snapshot ``os.environ``, and write potentially different snapshots.
+    The outer check keeps the warm-cache path lock-free; the inner check
+    inside the lock prevents duplicate initialisation on the first call.
     """
     global _BASE_ENV
     if _BASE_ENV is None:
-        _BASE_ENV = {**os.environ, **_GIT_ENV}
+        with _BASE_ENV_LOCK:
+            if _BASE_ENV is None:
+                _BASE_ENV = {**os.environ, **_GIT_ENV}
     return _BASE_ENV