diff --git a/conductor/tracks/test_sandbox_hardening_20260619/plan.md b/conductor/tracks/test_sandbox_hardening_20260619/plan.md new file mode 100644 index 00000000..63639f79 --- /dev/null +++ b/conductor/tracks/test_sandbox_hardening_20260619/plan.md @@ -0,0 +1,741 @@ +# Track Implementation Plan: Test Sandbox Hardening (2026-06-19) + +> **For Tier 3 workers:** This plan is executed task-by-task per `conductor/workflow.md`. Each task has WHERE / WHAT / HOW / SAFETY / COMMIT / GIT NOTE fields. Use the spec at `conductor/tracks/test_sandbox_hardening_20260619/spec.md` as the authoritative reference for FR/NFR/VC details. + +**Goal:** Make any `pytest` or `run_tests_batched.py` invocation provably incapable of writing files outside `./tests/` at the Python layer (default-on) and at the OS layer (opt-in via `scripts/run_tests_sandboxed.ps1`), by replacing the silent `SLOP_CONFIG` env-var fallback with an explicit `--config` CLI flag and adding a runtime file-I/O guard. + +**Architecture:** 5-part fix — (1) `src/paths.py` removes the env-var fallback; (2) sloppy.py + conftest.py parse `--config` and call `paths.set_config_override()`; (3) `sys.addaudithook` blocks writes outside `./tests/`; (4) pytest's `--basetemp` + conftest's `isolate_workspace` migrated under `./tests/`; (5) opt-in Windows restricted-token wrapper. Tests are TDD (red → green → commit). + +**Tech Stack:** Python 3.11+, `sys.addaudithook`, `pytest` 9.0+, PowerShell 7+, existing `tomli_w`, `tomllib`. + +**Reference files:** +- Spec: `conductor/tracks/test_sandbox_hardening_20260619/spec.md` +- Existing pattern: `scripts/audit_no_temp_writes.py` (audit script), `scripts/tier2/run_tier2_sandboxed.ps1` (PowerShell wrapper), `scripts/check_test_toml_paths.py` (existing static audit). + +--- + +## Phase 1: Investigation + Baseline + +**Focus:** Capture current pass count + audit src/ for `get_config_path()` callers so FR2 changes are transparent. + +- [ ] **Task 1.1:** Capture baseline pass count. + - **WHERE:** None (read-only audit). + - **WHAT:** Run the full test suite and record results. + - **HOW:** `uv run python scripts/run_tests_batched.py --tiers 1,2,3,4,5,6,7,8,9,10,11 > tests/artifacts/_baseline_pre_sandbox.txt 2>&1` + - **SAFETY:** Capture pass count + skip count + duration to `tests/artifacts/_baseline_pre_sandbox_summary.txt`. Do NOT modify any source file. + - **COMMIT:** None (audit-only). + - **GIT NOTE:** None. + +- [ ] **Task 1.2:** Audit `src/` for `get_config_path()` callers. + - **WHERE:** `src/` (grep audit). + - **WHAT:** Find every call site of `paths.get_config_path()` and `models._load_config_from_disk()` / `models._save_config_to_disk()`. The FR2 change (removing env-var fallback) must be transparent to all of them. + - **HOW:** + ```bash + grep -rn "get_config_path\|_load_config_from_disk\|_save_config_to_disk" src/ > tests/artifacts/_get_config_path_callers.txt + cat tests/artifacts/_get_config_path_callers.txt | wc -l # record count + ``` + - **SAFETY:** Expected ~10-20 call sites. All must be transparent because FR2's default (`/config.toml`) matches the current silent fallback behavior. + - **COMMIT:** None. + - **GIT NOTE:** None. + +- [ ] **Task 1.3:** Phase 1 verification. + - **WHERE:** None. + - **WHAT:** Confirm baseline + audit files exist + no source changes since session start. + - **HOW:** `ls tests/artifacts/_baseline_pre_sandbox* tests/artifacts/_get_config_path_callers.txt; git status --short | wc -l` + - **SAFETY:** Phase 1 is READ-ONLY; `git status` must show 0 modified source files. + - **COMMIT:** None. + - **GIT NOTE:** None. + +--- + +## Phase 2: FR4 Static Audit (LOW RISK — ship first) + +**Focus:** Write the static audit script that flags test files with hardcoded paths or `tempfile.mkdtemp()` without `dir=`. CI gate (default informational, `--strict` exits 1). + +- [ ] **Task 2.1:** Write `scripts/audit_test_sandbox_violations.py`. + - **WHERE:** Create `scripts/audit_test_sandbox_violations.py`. + - **WHAT:** Mirror `scripts/check_test_toml_paths.py` structure (compiled regexes + `find_violations(root_dir)` + `main()` with `--strict`). + - **HOW:** Patterns: + ```python + TOML_BASENAMES = r"manual_slop|config|credentials|presets|personas|tool_presets|workspace_profiles|project|manualslop_layout|manualslop_history|manualslop_history" + PATTERNS = [ + re.compile(rf'Path\(["\'](?:{TOML_BASENAMES})\.toml["\']'), + re.compile(rf'Path\(["\'](?:{TOML_BASENAMES})\.ini["\']'), + re.compile(rf'open\(["\'](?:{TOML_BASENAMES})\.toml["\'], ["\']w["\']'), + re.compile(r'Path\(["\']C:[/\\]+projects'), + re.compile(r'Path\(["\']tests/artifacts/'), + re.compile(r"tempfile\.mk(dt|st)emp\("), # bare calls without dir= + ] + EXCLUDE_DIRS = {"artifacts", "logs", "__pycache__"} + ``` + Plus a `find_violations(tests_dir)` that scans `tests/test_*.py` and returns `list[tuple[Path, int, str]]`. Plus `main()` with `--strict` (exit 1 on any violation; default exit 0 with report). + - **SAFETY:** Audit is INFORMATIONAL by default (exits 0). `--strict` exits 1 only on violations. Per `conductor/code_styleguides/audit-script-conventions.md` (if exists) or `audit_no_temp_writes.py` precedent. + - **COMMIT:** `chore(audit): add scripts/audit_test_sandbox_violations.py + tests for FR4 (Phase 2)` + - **GIT NOTE:** "Phase 2: static audit script + 3 regression tests for FR4 (hardcoded paths, clean test, tempfile.mkdtemp without dir=). Audit default informational, --strict exits 1." + +- [ ] **Task 2.2:** Write tests 5, 6, 10 in `tests/test_test_sandbox.py`. + - **WHERE:** Create `tests/test_test_sandbox.py`. + - **WHAT:** Three tests for the audit script. Imports + test signatures use 1-space indentation per `conductor/workflow.md`. + - **HOW:** + ```python + import subprocess, sys + from pathlib import Path + + def test_audit_flags_known_bad_pattern() -> None: + bad = Path("tests/artifacts/_audit_test_bad.py") + bad.parent.mkdir(parents=True, exist_ok=True) + bad.write_text('Path("manual_slop.toml").write_text("x")\n', encoding="utf-8") + result = subprocess.run([sys.executable, "scripts/audit_test_sandbox_violations.py", "--strict"], + capture_output=True, text=True) + assert result.returncode == 1, f"Expected exit 1, got {result.returncode}" + bad.unlink() + + def test_audit_passes_clean_test() -> None: + good = Path("tests/artifacts/_audit_test_good.py") + good.parent.mkdir(parents=True, exist_ok=True) + good.write_text("def test_x(tmp_path): tmp_path.joinpath('foo').write_text('x')\n", encoding="utf-8") + result = subprocess.run([sys.executable, "scripts/audit_test_sandbox_violations.py", "--strict"], + capture_output=True, text=True) + assert result.returncode == 0, f"Expected exit 0, got {result.returncode}: {result.stdout}" + good.unlink() + + def test_audit_flags_tempfile_mkdtemp_without_tests_dir() -> None: + bad = Path("tests/artifacts/_audit_test_tempfile.py") + bad.parent.mkdir(parents=True, exist_ok=True) + bad.write_text("import tempfile\ndef test_x(): tempfile.mkdtemp()\n", encoding="utf-8") + result = subprocess.run([sys.executable, "scripts/audit_test_sandbox_violations.py", "--strict"], + capture_output=True, text=True) + assert result.returncode == 1, f"Expected exit 1, got {result.returncode}" + bad.unlink() + ``` + - **SAFETY:** Tests must clean up their temp files even on failure (use `try/finally` or pytest fixture cleanup). + - **COMMIT:** Same as 2.1 (combined commit). + - **GIT NOTE:** Same as 2.1. + +- [ ] **Task 2.3:** Run Phase 2 tests to verify. + - **WHERE:** None. + - **WHAT:** Run the 3 new tests + manually invoke the audit script with a known-bad fixture file. + - **HOW:** `uv run python -m pytest tests/test_test_sandbox.py -v -k "audit_"` + - **SAFETY:** All 3 must pass. If any fail, debug and fix before committing. + - **COMMIT:** Same as 2.1. + - **GIT NOTE:** Same as 2.1. + +--- + +## Phase 3: FR1 Python Guard (HIGH RISK — must be precise) + +**Focus:** Implement `sys.addaudithook` to block all Python writes outside `./tests/` with `RuntimeError("TEST_SANDBOX_VIOLATION")`. + +- [ ] **Task 3.1:** Write `_enforce_test_sandbox` autouse fixture in `tests/conftest.py`. + - **WHERE:** Modify `tests/conftest.py` — add new fixture near `isolate_workspace` at line ~258. + - **WHAT:** Install `sys.addaudithook` for `open` (write modes), `os.mkdir`, `os.makedirs`, `shutil.rmtree`, `tempfile.mkdtemp`, `tempfile.mkstemp`. Allowlist = anything under `/tests/`. Block everything else. + - **HOW:** (Insert before the existing `isolate_workspace` fixture): + ```python + _SANDBOX_ALLOWLIST_PREFIXES: tuple[str, ...] = () # initialized in pytest_configure + + def _sandbox_audit_hook(event: str, args: tuple[object, ...]) -> None: + """sys.addaudithook target. Blocks writes outside ./tests/.""" + if event == "open": + path_obj, mode, *_ = args + if not isinstance(path_obj, (str, bytes, os.PathLike)): + return + if isinstance(mode, str) and not any(m in mode for m in ("w", "a", "x", "+")): + return + try: + resolved = Path(os.fspath(path_obj)).resolve() + except (OSError, ValueError): + return + if not _is_under_tests(resolved): + raise RuntimeError( + f"TEST_SANDBOX_VIOLATION: attempted to write to {resolved} " + f"(outside /tests/). Use tmp_path or fixture-provided paths." + ) + + def _is_under_tests(path: Path) -> bool: + for prefix in _SANDBOX_ALLOWLIST_PREFIXES: + try: + path.relative_to(prefix) + return True + except ValueError: + pass + return False + + @pytest.fixture(autouse=True) + def _enforce_test_sandbox() -> Generator[None, None, None]: + """Default-on runtime guard. Installed in pytest_configure.""" + yield # No-op; hook is installed at session start. + + def pytest_configure(config: object) -> None: + global _SANDBOX_ALLOWLIST_PREFIXES + project_root = Path(__file__).resolve().parent.parent + _SANDBOX_ALLOWLIST_PREFIXES = ( + str(project_root / "tests"), + str(Path("tests/artifacts/_pytest_tmp").resolve()), + str(Path("tests/artifacts/_isolation_workspace").resolve()), + ) + sys.addaudithook(_sandbox_audit_hook) + _check_required_test_dependencies() # existing call + + def pytest_unconfigure(config: object) -> None: + # Note: sys.addaudithook is permanent for the process; no removal API. + # The hook stays active until process exit (pytest is the only Python here). + pass + ``` + **IMPORTANT:** The existing `pytest_configure` at conftest.py:140 must be MERGED with the new one (don't create two definitions). + - **SAFETY:** The hook ONLY blocks write modes. Reads pass through. `.pytest_cache`, `__pycache__`, `.coverage` live under `./tests/` or project_root — verify with a quick test run before committing. + - **COMMIT:** `feat(tests): add _enforce_test_sandbox autouse fixture for FR1 (Phase 3)` + - **GIT NOTE:** "Phase 3: Python sys.addaudithook runtime guard. Blocks writes outside ./tests/ with TEST_SANDBOX_VIOLATION RuntimeError. Reads unaffected. Layer 1 of 4 enforcement stack." + +- [ ] **Task 3.2:** Write tests 1-4 in `tests/test_test_sandbox.py`. + - **WHERE:** Add to existing `tests/test_test_sandbox.py` (created in Phase 2). + - **WHAT:** Four tests verifying guard behavior. + - **HOW:** + ```python + def test_sandbox_blocks_writes_outside_tests_dir() -> None: + bad_path = Path(__file__).resolve().parent.parent / "manual_slop.toml" + with pytest.raises(RuntimeError, match="TEST_SANDBOX_VIOLATION"): + bad_path.write_text("corrupt", encoding="utf-8") + + def test_sandbox_allows_writes_inside_tests_dir(tmp_path) -> None: + (tmp_path / "foo.txt").write_text("ok", encoding="utf-8") + assert (tmp_path / "foo.txt").read_text(encoding="utf-8") == "ok" + + def test_sandbox_allows_writes_inside_tests_artifacts() -> None: + p = Path("tests/artifacts/_sandbox_test_allows/foo.txt") + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text("ok", encoding="utf-8") + assert p.read_text(encoding="utf-8") == "ok" + p.unlink() + + def test_sandbox_does_not_block_reads() -> None: + pyproject = Path(__file__).resolve().parent.parent / "pyproject.toml" + content = pyproject.read_text(encoding="utf-8") + assert "[tool.pytest.ini_options]" in content + ``` + - **SAFETY:** Test 1 is expected to RAISE; pytest.raises catches it. Tests 2-3 must SUCCEED. Test 4 must SUCCEED (read-only). + - **COMMIT:** Same as 3.1 (combined). + - **GIT NOTE:** Same as 3.1. + +- [ ] **Task 3.3:** Run full Tier-1 unit suite to verify no regression. + - **WHERE:** None. + - **WHAT:** Confirm the guard doesn't break any Tier-1 test that legitimately writes within `./tests/`. + - **HOW:** `uv run python -m pytest tests/ --collect-only -q | head -50` (just verify collection works). Then `uv run python scripts/run_tests_batched.py --tiers 1 --timeout 120` + - **SAFETY:** Tier-1 may have tests that write to `tmp_path` (which now resolves under `./tests/artifacts/_pytest_tmp`). If any test fails, the guard's allowlist needs expansion. Document and add to allowlist only after careful review (the test should already be using `tmp_path`). + - **COMMIT:** Same as 3.1. + - **GIT NOTE:** Same as 3.1. + +--- + +## Phase 4: FR2 Root-Cause Fix (--config CLI flag — MOST IMPORTANT) + +**Focus:** Replace the silent `SLOP_CONFIG` env-var fallback in `src/paths.py` with an explicit `set_config_override()` module-level setter, called from CLI parsers in `sloppy.py` and `tests/conftest.py`. This is THE fix for the user's data-loss pain. + +- [ ] **Task 4.1:** Refactor `src/paths.py` to remove the env-var fallback. + - **WHERE:** Modify `src/paths.py:42-46` (the `get_config_path()` function). + - **WHAT:** Remove `os.environ.get("SLOP_CONFIG", ...)` lookup. Add module-level `_CONFIG_OVERRIDE: Path | None = None` and `set_config_override(path: Path | None) -> None` function. + - **HOW:** + ```python + _CONFIG_OVERRIDE: Path | None = None + + def set_config_override(path: Path | None) -> None: + """Set the active config.toml path. None = use default. + CLI flag is the ONLY way to override. No env var fallback. + [C: sloppy.py:main, tests/conftest.py:_setup_test_paths]""" + global _CONFIG_OVERRIDE + _CONFIG_OVERRIDE = path + _RESOLVED.clear() + + def get_config_path() -> Path: + """Returns the active config.toml. If override is set, returns it. + Otherwise returns the default /config.toml. + [C: src/app_controller.py:AppController.load_config, + src/app_controller.py:AppController.init_state, + src/models.py:_load_config_from_disk]""" + if _CONFIG_OVERRIDE is not None: + return _CONFIG_OVERRIDE + root_dir = Path(__file__).resolve().parent.parent + return root_dir / "config.toml" + ``` + - **SAFETY:** The default behavior (no override) returns the same path as the previous env-var fallback when `SLOP_CONFIG` was unset. This is the SAME path the desktop GUI currently uses. So sloppy.py without `--config` works unchanged. + - **COMMIT:** `fix(paths): remove SLOP_CONFIG env-var fallback from get_config_path() (Phase 4, FR2 root-cause)` + - **GIT NOTE:** "Phase 4 task 4.1: root-cause fix for data loss. src/paths.py no longer silently falls back to /config.toml via SLOP_CONFIG env var. New API: paths.set_config_override(path). Default behavior unchanged when no override is set." + +- [ ] **Task 4.2:** Remove diagnostic stderr line from `src/models.py:193`. + - **WHERE:** Modify `src/models.py:193` (in `_save_config_to_disk`). + - **WHAT:** Delete the `sys.stderr.write(f"[DEBUG] Saving config. Theme: {config.get('theme')}\n"); sys.stderr.flush()` line. Per `AGENTS.md` "No Diagnostic Noise in Production" rule. + - **HOW:** Delete the two lines. + - **SAFETY:** This is a pure removal of diagnostic noise. No behavior change for normal operation. If any test depends on this stderr output, it should be removed too (check `tests/` for `capsys` fixtures matching this output). + - **COMMIT:** Same as 4.1 (combined commit "src cleanup for FR2"). + - **GIT NOTE:** Same as 4.1. + +- [ ] **Task 4.3:** Add `--config` argparse to `sloppy.py`. + - **WHERE:** Modify `sloppy.py` — the argparse setup (find the existing `ArgumentParser` block). + - **WHAT:** Add `--config ` flag. Call `paths.set_config_override(args.config)` BEFORE any `src/` import. + - **HOW:** + ```python + parser.add_argument("--config", type=str, default=None, + help="Path to config.toml (default: /config.toml)") + # ... parse args ... + if args.config: + from src import paths + paths.set_config_override(Path(args.config).resolve()) + # THEN import the rest: + from src.gui_2 import App # existing import below + ``` + - **SAFETY:** The `set_config_override` call must happen BEFORE `from src.gui_2 import App` because that import chain eventually imports paths and may trigger `get_config_path()`. + - **COMMIT:** `feat(sloppy): add --config CLI flag for config.toml override (Phase 4, FR2)` + - **GIT NOTE:** "Phase 4 task 4.3: sloppy.py accepts --config . Sets paths.set_config_override() before any src/ import. Default behavior unchanged." + +- [ ] **Task 4.4:** Update `tests/conftest.py` to parse `--config` at module body. + - **WHERE:** Modify `tests/conftest.py` — INSERT NEW CODE at the TOP of the file (before the existing `import pytest` line, around line 14). + - **WHAT:** Parse `sys.argv` for `--config` at module body BEFORE any `src/` import. Auto-default to `tests/artifacts/_isolation_workspace_/config_overrides.toml`. Also register with pytest via `pytest_addoption`. + - **HOW:** + ```python + # === STAGE 1: Parse --config from sys.argv BEFORE any src/ import === + import sys as _sys + from pathlib import Path as _Path + + _RUN_ID = datetime.datetime.now().strftime("%Y%m%d_%H%M%S") + _ISOLATION_WORKSPACE = _Path(f"tests/artifacts/_isolation_workspace_{_RUN_ID}") + _ISOLATION_WORKSPACE.mkdir(parents=True, exist_ok=True) + + def _parse_config_arg(argv: list[str]) -> _Path | None: + for i, arg in enumerate(argv[1:]): + if arg == "--config" and i + 1 < len(argv) - 1: + return _Path(argv[i + 2]).resolve() + if arg.startswith("--config="): + return _Path(arg.split("=", 1)[1]).resolve() + return None + + _config_override_arg = _parse_config_arg(_sys.argv) + if _config_override_arg is None: + _config_override_arg = _ISOLATION_WORKSPACE / "config_overrides.toml" + + # Set override BEFORE any src/ import + from src import paths as _paths # noqa: E402 + _paths.set_config_override(_config_override_arg) + + # Register --config with pytest so it doesn't warn about unknown flag + def pytest_addoption(parser): + parser.addoption("--config", action="store", default=None, + help="Manual Slop: override config.toml path for tests") + ``` + **IMPORTANT:** This block must be inserted BEFORE `from src.app_controller import AppController` (line 64) and BEFORE any other `src/` imports. Also DELETE the `from src.gui_2 import App` line at line ~250 (move it after the new fixture insertion point to keep imports tidy). + - **SAFETY:** The sys.argv parse happens at conftest module import time, BEFORE pytest's argparse. The auto-generated `_config_override_arg` lives inside `./tests/artifacts/`, which the Layer 1 guard will allowlist. Tests that explicitly pass `--config /some/path` get that override. Tests without `--config` get the auto-sandbox. + - **COMMIT:** `feat(tests): parse --config CLI flag in conftest.py module body (Phase 4, FR2)` + - **GIT NOTE:** "Phase 4 task 4.4: conftest.py parses sys.argv for --config BEFORE any src/ import. Auto-defaults to tests/artifacts/_isolation_workspace_/config_overrides.toml. registers via pytest_addoption so pytest doesn't warn." + +- [ ] **Task 4.5:** Write tests 11, 12, 13 in `tests/test_test_sandbox.py`. + - **WHERE:** Add to existing `tests/test_test_sandbox.py`. + - **WHAT:** Three tests for the `--config` CLI flag behavior. + - **HOW:** + ```python + def test_config_override_via_cli_flag(tmp_path) -> None: + config_path = tmp_path / "my_config.toml" + config_path.write_text("[ai]\nprovider='gemini'\n", encoding="utf-8") + from src import paths + original = paths._CONFIG_OVERRIDE + try: + paths.set_config_override(config_path) + assert paths.get_config_path() == config_path + finally: + paths.set_config_override(original) + + def test_paths_get_config_path_no_env_fallback(monkeypatch) -> None: + monkeypatch.delenv("SLOP_CONFIG", raising=False) + from src import paths + original = paths._CONFIG_OVERRIDE + try: + paths.set_config_override(None) + root = Path(__file__).resolve().parent.parent + assert paths.get_config_path() == root / "config.toml" + finally: + paths.set_config_override(original) + + def test_sloppy_py_parses_config_flag() -> None: + import ast + sloppy = Path(__file__).resolve().parent.parent / "sloppy.py" + tree = ast.parse(sloppy.read_text(encoding="utf-8")) + found_config = False + for node in ast.walk(tree): + if isinstance(node, ast.arg) and node.arg == "config": + found_config = True + assert found_config, "sloppy.py must have a --config argparse argument" + ``` + - **SAFETY:** Tests manipulate `paths._CONFIG_OVERRIDE` directly (private API but necessary for testing). Always restore in `finally` block. + - **COMMIT:** `test(sandbox): add regression tests for --config CLI flag (Phase 4)` + - **GIT NOTE:** "Phase 4 task 4.5: 3 regression tests for FR2 (--config CLI flag, no env var fallback, sloppy.py argparse)." + +- [ ] **Task 4.6:** Phase 4 verification — run a broad smoke test. + - **WHERE:** None. + - **WHAT:** Confirm sloppy.py (production) still launches with default config + tests still work with --config. + - **HOW:** + ```bash + # Production: sloppy.py without --config uses default + python sloppy.py --help # should NOT raise; --config appears in help + + # Tests: conftest auto-defaults to ./tests/artifacts/.../config_overrides.toml + uv run python -m pytest tests/test_test_sandbox.py::test_config_override_via_cli_flag -v + uv run python -m pytest tests/test_paths.py -v # existing tests still work + ``` + - **SAFETY:** If sloppy.py crashes at import, the `--config` ordering is wrong. If existing tests fail, the new default breaks something — debug before committing. + - **COMMIT:** None (this is verification, not a code change). + - **GIT NOTE:** None. + +--- + +## Phase 5: FR3 isolate_workspace + basetemp migration + +**Focus:** Move the `isolate_workspace` workspace off `%TEMP%` to `./tests/artifacts/_isolation_workspace_/`. Add `addopts = "--basetemp=..."` to pyproject.toml. Update tech-stack.md note. + +- [ ] **Task 5.1:** Refactor `isolate_workspace` in `tests/conftest.py`. + - **WHERE:** Modify `tests/conftest.py:259-281` (the existing `isolate_workspace` autouse). + - **WHAT:** Replace `tmp_path_factory.mktemp("isolated_workspace")` with `Path("tests/artifacts/_isolation_workspace") / _RUN_ID`. Add `SLOP_CREDENTIALS` + `SLOP_MCP_ENV` env vars. Auto-generate placeholder TOML files. + - **HOW:** + ```python + @pytest.fixture(autouse=True) + def isolate_workspace(monkeypatch) -> Generator[None, None, None]: + """Autouse fixture to isolate tests from the active user workspace. + Workspace lives under tests/artifacts/ per workspace_paths.md.""" + test_workspace = _ISOLATION_WORKSPACE # defined in conftest module body + test_workspace.mkdir(parents=True, exist_ok=True) + + # Generate placeholder TOML files + config_content = { + "ai": {"provider": "gemini", "model": "gemini-2.5-flash-lite"}, + "projects": {"paths": [], "active": ""}, + "gui": {"show_windows": {}}, + } + with open(test_workspace / "config_overrides.toml", "wb") as f: + tomli_w.dump(config_content, f) + for name in ("credentials.toml", "mcp_env.toml", "presets.toml", + "tool_presets.toml", "personas.toml", "workspace_profiles.toml"): + (test_workspace / name).touch() + + monkeypatch.setenv("SLOP_CREDENTIALS", str(test_workspace / "credentials.toml")) + monkeypatch.setenv("SLOP_MCP_ENV", str(test_workspace / "mcp_env.toml")) + monkeypatch.setenv("SLOP_GLOBAL_PRESETS", str(test_workspace / "presets.toml")) + monkeypatch.setenv("SLOP_GLOBAL_TOOL_PRESETS", str(test_workspace / "tool_presets.toml")) + monkeypatch.setenv("SLOP_GLOBAL_PERSONAS", str(test_workspace / "personas.toml")) + monkeypatch.setenv("SLOP_GLOBAL_WORKSPACE_PROFILES", str(test_workspace / "workspace_profiles.toml")) + yield + ``` + **Note:** The `tmp_path_factory` parameter is REMOVED from this fixture. Tests that legitimately need it should request it directly (`def test_x(tmp_path): ...`). + - **SAFETY:** All env vars point INSIDE the isolation workspace, which is inside `./tests/artifacts/`. The Layer 1 guard allows this. No test should break UNLESS it was relying on the previous `%TEMP%` path. + - **COMMIT:** `refactor(tests): migrate isolate_workspace off tmp_path_factory to tests/artifacts/ (Phase 5, FR3)` + - **GIT NOTE:** "Phase 5 task 5.1: isolate_workspace fixture now creates tests/artifacts/_isolation_workspace_/. Adds SLOP_CREDENTIALS + SLOP_MCP_ENV env vars (previously only set in live_gui fixture). Per workspace_paths.md styleguide." + +- [ ] **Task 5.2:** Add `addopts` to `pyproject.toml`. + - **WHERE:** Modify `pyproject.toml` — add to `[tool.pytest.ini_options]` section. + - **WHAT:** Add `addopts = "--basetemp=tests/artifacts/_pytest_tmp"` so pytest's `tmp_path` factory uses a path under `./tests/`. + - **HOW:** Insert: + ```toml + [tool.pytest.ini_options] + addopts = "--basetemp=tests/artifacts/_pytest_tmp" + markers = [ + ... + ] + ``` + - **SAFETY:** The basetemp directory is auto-created by pytest. `.gitignore` already has `tests/artifacts/` so it's gitignored. + - **COMMIT:** `chore(pyproject): add --basetemp=tests/artifacts/_pytest_tmp addopts (Phase 5, FR3)` + - **GIT NOTE:** "Phase 5 task 5.2: pyproject.toml pytest addopts sets --basetemp to ./tests/artifacts/_pytest_tmp so all pytest tmp_path fixtures live under ./tests/." + +- [ ] **Task 5.3:** Defensive `_tmp_path_factory._basetemp` check in `conftest.py:pytest_configure`. + - **WHERE:** Add to existing `pytest_configure` in `tests/conftest.py` (the one merged in Task 3.1). + - **WHAT:** If `config._tmp_path_factory._basetemp` resolves outside `./tests/`, override to `./tests/artifacts/_pytest_tmp`. + - **HOW:** + ```python + project_root = Path(__file__).resolve().parent.parent + basetemp = getattr(config, "_tmp_path_factory", None) + if basetemp is not None: + current = Path(str(basetemp._basetemp)).resolve() + if not str(current).startswith(str(project_root / "tests")): + basetemp._basetemp = str(project_root / "tests" / "artifacts" / "_pytest_tmp") + ``` + - **SAFETY:** Uses private API `_tmp_path_factory._basetemp` — if pytest version changes, this breaks. The `addopts` in Task 5.2 is the primary mechanism; this is defensive. + - **COMMIT:** Same as 5.2 (combined). + - **GIT NOTE:** Same as 5.2. + +- [ ] **Task 5.4:** Add dated note to `conductor/tech-stack.md`. + - **WHERE:** Modify `conductor/tech-stack.md` — append a dated note to the pytest section. + - **WHAT:** Explain the `--basetemp` choice and reference `workspace_paths.md`. + - **HOW:** + ```markdown + ## pyproject.toml pytest addopts (added 2026-06-19, per test_sandbox_hardening_20260619) + + `[tool.pytest.ini_options].addopts = "--basetemp=tests/artifacts/_pytest_tmp"`. + + **Rationale:** Per `conductor/code_styleguides/workspace_paths.md`, ALL test + infrastructure paths must live under `./tests/`. pytest's `tmp_path` and + `tmp_path_factory` fixtures default to `%TEMP%\pytest-of-\` on + Windows. This `addopts` redirects them under `./tests/` so the Layer 1 + runtime guard's allowlist (also `./tests/`) can be a single rule. + ``` + - **SAFETY:** Pure documentation change. + - **COMMIT:** `docs(tech-stack): note --basetemp addopts rationale (Phase 5, FR3)` + - **GIT NOTE:** Same as 5.2. + +- [ ] **Task 5.5:** Write tests 7, 8, 9 in `tests/test_test_sandbox.py`. + - **WHERE:** Add to existing `tests/test_test_sandbox.py`. + - **WHAT:** Three tests verifying pyproject.toml, isolate_workspace, and AppController invariant. + - **HOW:** + ```python + def test_pyproject_toml_basetemp_is_under_tests() -> None: + pyproject = Path(__file__).resolve().parent.parent / "pyproject.toml" + text = pyproject.read_text(encoding="utf-8") + assert "--basetemp=tests/artifacts/_pytest_tmp" in text + + def test_isolate_workspace_does_not_use_tmp_path_factory_for_infra() -> None: + import ast + conftest = Path(__file__).resolve().parent / "conftest.py" + tree = ast.parse(conftest.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef) and node.name == "isolate_workspace": + src = ast.unparse(node) + assert "tmp_path_factory.mktemp" not in src, ( + "isolate_workspace must not use tmp_path_factory.mktemp; " + "use Path('tests/artifacts/_isolation_workspace') / _RUN_ID" + ) + return + raise AssertionError("isolate_workspace fixture not found in conftest.py") + + def test_appcontroller_init_does_not_load_config() -> None: + import ast + app_controller = Path(__file__).resolve().parent.parent / "src" / "app_controller.py" + tree = ast.parse(app_controller.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef) and node.name == "__init__": + src = ast.unparse(node) + assert "init_state()" not in src, ( + "AppController.__init__ must not call init_state() " + "(this would trigger config reads before fixtures apply)" + ) + assert "load_config()" not in src, ( + "AppController.__init__ must not call load_config() " + "(this would trigger config reads before fixtures apply)" + ) + return + raise AssertionError("AppController.__init__ not found") + ``` + - **SAFETY:** These tests are static AST checks; they parse source files. They fail loud if invariants break. The `init_state()` invariant test is critical per FR2 audit. + - **COMMIT:** `test(sandbox): add regression tests for FR3 invariants (Phase 5)` + - **GIT NOTE:** "Phase 5 task 5.5: 3 regression tests for FR3 (pyproject basetemp, isolate_workspace no tmp_path_factory, AppController.__init__ invariant)." + +- [ ] **Task 5.6:** Phase 5 verification — run Tier-2 + Tier-3 to confirm no regression. + - **WHERE:** None. + - **WHAT:** Verify the basetemp migration + isolate_workspace migration don't break existing tests. + - **HOW:** `uv run python scripts/run_tests_batched.py --tiers 2,3 --timeout 180` + - **SAFETY:** If tests fail, check whether they were using `tmp_path` (which now resolves under `./tests/`) or hardcoded paths to `%TEMP%` (which the Layer 1 guard now blocks). Audit the failing test, don't disable the guard. + - **COMMIT:** None. + - **GIT NOTE:** None. + +--- + +## Phase 6: FR5 PowerShell Wrapper (OPT-IN) + +**Focus:** Write `scripts/run_tests_sandboxed.ps1` (Windows-only, opt-in) that wraps pytest in a Windows restricted token + Job Object. + +- [ ] **Task 6.1:** Write `scripts/run_tests_sandboxed.ps1`. + - **WHERE:** Create `scripts/run_tests_sandboxed.ps1`. + - **WHAT:** Mirror `scripts/tier2/run_tier2_sandboxed.ps1` structure (100 lines). Replace OpenCode launch with pytest launch. + - **HOW:** Tier 3 worker MUST read `scripts/tier2/run_tier2_sandboxed.ps1` end-to-end first (per writing-plans skill "Read Reference Implementation COMPLETELY"), then copy its Add-Type / Job Object / token-acquisition blocks verbatim. Only the LAST step (the actual process launch) differs. Full template: + ```powershell + # scripts/run_tests_sandboxed.ps1 + <# + .SYNOPSIS + Run pytest in a Windows restricted-token sandbox. + .DESCRIPTION + Acquires a Windows restricted token (drops dangerous privileges), + wraps pytest in a Job Object, and runs the test suite. The test + workspace is forced under ./tests/ via the --config and --basetemp + flags (handled by the conftest.py autouse fixtures). The Tier 2 + clone at is the only directory pytest can read/write + for tests; everything outside ./tests/ is blocked by the Layer 1 + Python guard PLUS the restricted-token enforcement. + .NOTES + Requires Windows + PowerShell 7+ + admin privileges for full + restricted-token acquisition. The -WhatIf mode is a no-op dry-run + (exits 0 without acquiring a token). + .LINK + scripts/tier2/run_tier2_sandboxed.ps1 (template) + conductor/tracks/test_sandbox_hardening_20260619/spec.md (FR5) + #> + [CmdletBinding()] + param( + [switch]$WhatIf, + [string]$TestPath = "tests/", + [string]$ConfigPath = "" # empty = conftest.py auto-defaults to config_overrides.toml + ) + + $ErrorActionPreference = "Stop" + $ProjectRoot = (Resolve-Path "$PSScriptRoot/..").Path + + if ($WhatIf) { + Write-Host "[SANDBOX-WHATIF] Would run pytest in restricted token at $ProjectRoot" + Write-Host "[SANDBOX-WHATIF] TestPath: $TestPath" + Write-Host "[SANDBOX-WHATIF] ConfigPath: $($ConfigPath) (empty = conftest.py auto-defaults)" + exit 0 + } + + # === BEGIN: copy Add-Type / token / Job Object blocks from === + # === scripts/tier2/run_tier2_sandboxed.ps1 lines 30-95 verbatim === + # (See reference script for the full restricted-token + Job Object setup.) + + # === END: tier2 clone blocks === + + # Invoke pytest under restricted token with sandbox flags. + # The --basetemp flag ensures pytest's tmp dirs live under ./tests/. + # The --config flag points to a config_overrides.toml inside ./tests/ + # (or empty = conftest.py auto-defaults). + $argList = @( + "run", "python", "-m", "pytest", $TestPath, + "--basetemp=tests/artifacts/_pytest_tmp" + ) + if ($ConfigPath -ne "") { $argList += "--config=$ConfigPath" } + + Push-Location $ProjectRoot + try { + & uv @argList + } finally { + Pop-Location + } + ``` + The Add-Type / token / Job Object blocks MUST be copied verbatim from `scripts/tier2/run_tier2_sandboxed.ps1` lines 30-95 (or wherever the equivalent code lives in the latest version of that script — Tier 3 worker should re-read the source). Only the LAST block (the actual invocation) is new. + - **SAFETY:** `-WhatIf` mode is a no-op (exits 0). Full PowerShell restricted-token wrapper requires admin privileges on Windows; document this in the script header. The script is OPT-IN — users continue to use `uv run pytest` or `uv run python scripts/run_tests_batched.py` for normal test runs. + - **COMMIT:** `feat(scripts): add scripts/run_tests_sandboxed.ps1 (Phase 6, FR5 opt-in)` + - **GIT NOTE:** "Phase 6 task 6.1: PowerShell wrapper for Windows restricted-token + Job Object pytest sandbox. Mirrors run_tier2_sandboxed.ps1 structure (Add-Type + token + Job Object blocks copied verbatim). Only the invocation differs (pytest instead of OpenCode). -WhatIf mode for dry-run. OPT-IN." + +- [ ] **Task 6.2:** Write a smoke test for `-WhatIf` mode. + - **WHERE:** Add to `tests/test_test_sandbox.py` (as test 14). + - **WHAT:** Verify `pwsh -File scripts/run_tests_sandboxed.ps1 -WhatIf` exits 0. + - **HOW:** + ```python + @pytest.mark.skipif(os.name != "nt", reason="Windows-only sandbox wrapper") + def test_run_tests_sandboxed_whatif() -> None: + result = subprocess.run( + ["pwsh", "-File", "scripts/run_tests_sandboxed.ps1", "-WhatIf"], + capture_output=True, text=True, + ) + assert result.returncode == 0, f"Expected exit 0, got {result.returncode}: {result.stderr}" + ``` + - **SAFETY:** Skipped on non-Windows per `conductor/workflow.md` Skip-Marker Policy (legitimate opt-in integration test, requires Windows + pwsh). + - **COMMIT:** Same as 6.1. + - **GIT NOTE:** Same as 6.1. + +--- + +## Phase 7: FR7 Documentation + +**Focus:** Document the 4-layer enforcement model + `--config` CLI flag convention + `config_overrides.toml` naming. + +- [ ] **Task 7.1:** Create `conductor/code_styleguides/test_sandbox.md`. + - **WHERE:** Create `conductor/code_styleguides/test_sandbox.md`. + - **WHAT:** Styleguide document covering: the `--config` CLI flag, `config_overrides.toml` convention, 4-layer enforcement model, `--basetemp` rule, Layer 1 audit hook contract, opt-in `run_tests_sandboxed.ps1`, audit script. + - **HOW:** Use elements-of-style:writing-clearly-and-concisely (the existing styleguides in `conductor/code_styleguides/` are good templates). Sections: TL;DR; The 4-Layer Model; `--config` CLI Flag (replaces SLOP_CONFIG); `--basetemp` Rule; Layer 1 Audit Hook Contract; Static Audit; OS-Level Wrapper; Test Workspace Convention (`config_overrides.toml`); See Also. + - **SAFETY:** Documentation only. Reference actual file:line locations from the spec. + - **COMMIT:** `docs(styleguide): add test_sandbox.md (Phase 7, FR7)` + - **GIT NOTE:** "Phase 7 task 7.1: new styleguide test_sandbox.md documents the 4-layer enforcement model, --config CLI flag, config_overrides.toml convention, --basetemp rule." + +- [ ] **Task 7.2:** Update `conductor/code_styleguides/workspace_paths.md`. + - **WHERE:** Append a section to the existing file. + - **WHAT:** Mention the `SLOP_CONFIG → --config` migration + `pytest --basetemp` addopts. + - **HOW:** Add a "2026-06-19 Update" section at the bottom. + - **SAFETY:** Documentation only. + - **COMMIT:** Same as 7.1. + - **GIT NOTE:** Same as 7.1. + +- [ ] **Task 7.3:** Add `Sandbox Hardening` section to `docs/guide_testing.md`. + - **WHERE:** Modify `docs/guide_testing.md` — add a new section. + - **WHAT:** Cross-reference to `test_sandbox.md` + summary of the 4 layers. + - **HOW:** Append the section. + - **SAFETY:** Documentation only. + - **COMMIT:** Same as 7.1. + - **GIT NOTE:** Same as 7.1. + +--- + +## Phase 8: Full Suite Verification + +**Focus:** Run the full 11-tier suite and confirm no regression vs. the `1288 passed + 4 xdist-skipped` baseline. + +- [ ] **Task 8.1:** Run full test suite. + - **WHERE:** None. + - **WHAT:** Run all 11 tiers and capture results. + - **HOW:** `uv run python scripts/run_tests_batched.py --tiers 1,2,3,4,5,6,7,8,9,10,11 > tests/artifacts/_full_suite_post_sandbox.txt 2>&1` + - **SAFETY:** If regression vs. baseline (1288 + 4), STOP and report to user. Do not commit a broken suite. Per `conductor/workflow.md` Phase Completion Verification protocol. + - **COMMIT:** None (verification). + - **GIT NOTE:** None. + +- [ ] **Task 8.2:** Commit verification report. + - **WHERE:** None (commit the baseline diff comparison). + - **WHAT:** Stage `tests/artifacts/_full_suite_post_sandbox.txt` as a verification artifact. + - **HOW:** `git add tests/artifacts/_full_suite_post_sandbox.txt; git commit -m "conductor(checkpoint): Phase 8 - full suite green, no regression vs. baseline 1288+4"` + - **SAFETY:** If regression occurred in 8.1, fix forward or roll back per `conductor/workflow.md` Per-Task Decision Protocol. + - **COMMIT:** As above. + - **GIT NOTE:** "Phase 8 checkpoint: full 11-tier suite passed. No regression vs. pre-track baseline (1288 + 4). Test sandbox hardening is operational." + +--- + +## Phase 9: End-of-Track Report + +**Focus:** Write the completion report following the precedent set by `TRACK_COMPLETION_tier2_autonomous_sandbox_20260616.md`. Update state.toml to `completed`. + +- [ ] **Task 9.1:** Write `docs/reports/TRACK_COMPLETION_test_sandbox_hardening_20260619.md`. + - **WHERE:** Create `docs/reports/TRACK_COMPLETION_test_sandbox_hardening_20260619.md`. + - **WHAT:** Track completion report with: scope (files added/modified), pass-rate baseline + post, deferred items, lessons learned, follow-up tracks (other SLOP_* env vars), user review gate. + - **HOW:** Mirror the structure of `docs/reports/TRACK_COMPLETION_tier2_autonomous_sandbox_20260616.md`. + - **SAFETY:** Pure documentation. + - **COMMIT:** `docs(reports): TRACK_COMPLETION_test_sandbox_hardening_20260619 (Phase 9)` + - **GIT NOTE:** "Phase 9: track completion report. 9 phases shipped. 4-layer test sandbox enforcement operational. Deferred: convert other SLOP_* env vars to CLI flags (separate mess, separate tracks)." + +- [ ] **Task 9.2:** Update `state.toml` and commit. + - **WHERE:** Modify `conductor/tracks/test_sandbox_hardening_20260619/state.toml`. + - **WHAT:** Set `status = "completed"`, `current_phase = "complete"`. + - **HOW:** + ```toml + [meta] + status = "completed" + current_phase = "complete" + last_updated = "2026-06-19" + ``` + - **SAFETY:** Pure metadata. + - **COMMIT:** `conductor(state): mark test_sandbox_hardening_20260619 complete` + - **GIT NOTE:** "Phase 9 final: state.toml marked complete. Track ships." + +--- + +## Summary + +| Phase | Tasks | Key output | Risk | +|-------|-------|------------|------| +| 1: Investigation | 3 | Baseline pass count + audit of get_config_path() callers | None (read-only) | +| 2: FR4 Static audit | 3 | `scripts/audit_test_sandbox_violations.py` + 3 tests | Low | +| 3: FR1 Python guard | 3 | `_enforce_test_sandbox` fixture + 4 tests | High (can break tests) | +| 4: FR2 Root-cause fix | 6 | `set_config_override()` + `--config` CLI flag + 3 tests | High (root-cause) | +| 5: FR3 Isolation migration | 6 | `isolate_workspace` + `--basetemp` + tech-stack.md + 3 tests | Medium | +| 6: FR5 PowerShell | 2 | `scripts/run_tests_sandboxed.ps1` + smoke test | Low (opt-in) | +| 7: FR7 Documentation | 3 | `test_sandbox.md` + updates | None | +| 8: Verification | 2 | 11-tier pass count + checkpoint commit | Verification only | +| 9: Report | 2 | `TRACK_COMPLETION_*` + state.toml `completed` | None | + +**Total: 30 tasks across 9 phases, ~11 atomic commits.** + +**TDD per phase:** Red (write failing test) → Green (minimal impl) → Verify → Commit. + +**Per-task discipline:** WHERE / WHAT / HOW / SAFETY / COMMIT / GIT NOTE per `conductor/workflow.md` Tier 1 rules. + +**Hard bans:** No `git restore`, `git checkout`, `git reset`. No day estimates in commit messages or git notes. No diagnostic noise in `src/*.py`. No new `@pytest.mark.skip` markers except the one for `test_run_tests_sandboxed_whatif` (Windows-only, legitimate per `conductor/workflow.md` Skip-Marker Policy). + +**Rollback:** Each phase is a separate commit. If any phase breaks, `git revert` the phase's commit(s) without affecting the others. + +--- + +## Handoff to Tier 2 + +This plan is executed by a Tier 2 Tech Lead via the standard `conductor/workflow.md` Task Workflow: +1. Activate `mma-orchestrator` skill. +2. For each task: read context, write code, run tests, commit per `git commit` line, attach git note. +3. After each phase: phase completion verification + checkpoint. +4. After Phase 9: track complete; user reviews merge per `conductor/workflow.md` "Review and merge workflow". + +Tier 3 workers (via `scripts/mma_exec.py --role tier3-worker`) handle individual tasks with surgical prompts. The Tier 2 Tech Lead reviews each commit before moving to the next task.