diff --git a/conductor/code_styleguides/test_sandbox.md b/conductor/code_styleguides/test_sandbox.md new file mode 100644 index 00000000..00c3f084 --- /dev/null +++ b/conductor/code_styleguides/test_sandbox.md @@ -0,0 +1,170 @@ +# Test Sandbox Hardening — Hard Rule + +## TL;DR + +The Manual Slop test suite runs under a 4-layer sandbox that prevents any pytest invocation from writing files outside `./tests/`. The root-cause fix removes the historical `SLOP_CONFIG` env-var fallback in favor of an explicit `--config` CLI flag. Any test that needs a config file must point at one inside `./tests/artifacts/`. + +## The 4-Layer Model + +| Layer | Mechanism | Where | Default-on? | +|---|---|---|---| +| Layer 1 | Python runtime file-I/O guard (`sys.addaudithook`) | `tests/conftest.py:_sandbox_audit_hook` | Yes | +| Layer 2 | `isolate_workspace` autouse + `pyproject.toml --basetemp` | `tests/conftest.py` + `pyproject.toml` | Yes | +| Layer 3 | OS-level restricted-token PowerShell wrapper | `scripts/run_tests_sandboxed.ps1` | **Opt-in** | +| Layer 4 | Static audit script (CI gate) | `scripts/audit_test_sandbox_violations.py` | Yes (informational) / opt-in (`--strict`) | + +Layer 1 + Layer 2 + Layer 4 are file-presence-on = enabled (delete the relevant file to disable). Layer 3 requires explicit invocation. + +## The `--config` CLI Flag (replaces `SLOP_CONFIG`) + +The historical `SLOP_CONFIG` env var has been removed from `src/paths.py`. The CLI flag `--config ` is now the ONLY supported mechanism for overriding the default `/config.toml` location. + +### sloppy.py + +```bash +# Use the default /config.toml +uv run python sloppy.py + +# Override +uv run python sloppy.py --config /path/to/your/config.toml +``` + +`sloppy.py` calls `paths.set_config_override(Path(args.config).resolve())` AFTER `parse_args()` and BEFORE any `from src.gui_2 import App` import. This is the only way to override the config path in production. + +### tests/conftest.py + +`tests/conftest.py` parses `sys.argv` for `--config` at MODULE BODY (BEFORE any `src/` import). If `--config` is not passed, conftest auto-defaults to `tests/artifacts/_isolation_workspace_/config_overrides.toml` (which lives inside `./tests/artifacts/`, so the Layer 1 guard allows writes to it). + +```python +# Module body in tests/conftest.py (BEFORE any src/ import) +def _parse_config_arg(argv: list[str]) -> Path | None: + for i in range(1, len(argv)): + arg = argv[i] + if arg == "--config" and i + 1 < len(argv): + return Path(argv[i + 1]).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" + +from src import paths as _paths # noqa: E402 +_paths.set_config_override(_config_override_arg) +``` + +The fixture also auto-generates a placeholder `config_overrides.toml` (with `ai.provider`, `projects`, `gui.show_windows`) so src/ code that reads the config at startup does not crash. + +## The `--basetemp` Rule + +`pyproject.toml` sets `addopts = "--basetemp=tests/artifacts/_pytest_tmp"`. This redirects pytest's `tmp_path` and `tmp_path_factory` fixtures (which default to `%TEMP%\pytest-of-\` on Windows) into `./tests/artifacts/`. This is what allows the Layer 1 allowlist to be a single rule: "anything under `./tests/` is allowed." + +## Layer 1 Audit Hook Contract + +`tests/conftest.py:_sandbox_audit_hook` is a `sys.addaudithook` callback. It fires on every `open()` call. Behavior: + +- **Reads** (mode `r`, `rb`): pass through, no check +- **Writes** (mode contains `w`, `a`, `x`, `+`): check path +- **Allowed** if path resolves under: + - `/tests/` + - Path contains `.pytest_cache`, `__pycache__`, `.coverage`, `.slop_cache`, or `.ruff_cache` + - Original path string starts with `\\.\` (Windows device namespace) or `/dev/` (Unix device namespace) +- **Blocked** otherwise: raises `RuntimeError("TEST_SANDBOX_VIOLATION: attempted to write to ...")` + +**How to fix a violation:** +- Move the write under `/tests/` (use `tmp_path`, `tests/artifacts/_/`, etc.) +- For pytest internal files (cache, log): check if the path is in the allowlist; if not, open an issue to add it + +## Layer 2 Workspace Convention (`config_overrides.toml`) + +Tests that need a `config.toml` should use the auto-generated `tests/artifacts/_isolation_workspace_/config_overrides.toml`. The naming convention `config_overrides.toml` (instead of `config.toml`) signals that this file is an override for tests, not the production config. + +Tests CAN pass `--config /some/other/path.toml` explicitly; conftest will honor it. But the default is fine for most cases. + +## Layer 3 Opt-in OS-Level Wrapper + +`scripts/run_tests_sandboxed.ps1` is the Windows-only restricted-token + Job Object wrapper for paranoid users. It mirrors `scripts/tier2/run_tier2_sandboxed.ps1`: + +```bash +# Dry-run (no actual sandbox; just prints what would happen) +pwsh -File scripts/run_tests_sandboxed.ps1 -WhatIf + +# Run the full suite in the sandbox +pwsh -File scripts/run_tests_sandboxed.ps1 + +# Run a specific test path +pwsh -File scripts/run_tests_sandboxed.ps1 -TestPath tests/test_paths.py + +# Override config explicitly +pwsh -File scripts/run_tests_sandboxed.ps1 -ConfigPath /some/path/config.toml +``` + +The wrapper: +1. Acquires a restricted token via .NET DuplicateTokenEx +2. Sets cwd to `` +3. Invokes `uv run python -m pytest $TestPath --basetemp=tests/artifacts/_pytest_tmp [--config=...]` +4. Forwards pytest exit code + +## Layer 4 Static Audit + +`scripts/audit_test_sandbox_violations.py` scans `tests/test_*.py` for hardcoded paths that would corrupt user files: + +- `Path("manual_slop.toml")`, `Path("config.toml")`, `Path("credentials.toml")`, `Path("presets.toml")`, etc. +- `open("manual_slop.toml", "w")` and similar write-mode calls +- `Path("C:/projects/...")` and `Path("C:\\projects\\...")` +- `Path("tests/artifacts/...")` literal (violates workspace_paths.md; should use a fixture) +- `tempfile.mkdtemp()`, `tempfile.mkstemp()` (without `dir=`) + +Default mode (informational) exits 0 and lists violations. `--strict` mode (CI gate) exits 1 on any violation. + +```bash +# Informational +uv run python scripts/audit_test_sandbox_violations.py + +# CI gate +uv run python scripts/audit_test_sandbox_violations.py --strict +``` + +## Why This Rule Exists + +The user has lost "important sample data" multiple times over the past month because tests have written to `manual_slop.toml`, `manual_slop_history.toml`, `personas.toml`, `presets.toml`, `tool_presets.toml`, or `credentials.toml` at the top of the repo. The root cause was the silent `SLOP_CONFIG` env-var fallback in `src/paths.py` — any test could set the env var and have `paths.get_config_path()` return a project-root file. + +This track fixes that and adds defense in depth. + +## Forbidden Patterns (Hard Bans) + +### 1. `SLOP_CONFIG` env var + +Setting `SLOP_CONFIG` no longer affects `paths.get_config_path()`. Use `--config` instead. + +### 2. `tempfile.mkdtemp()` / `tempfile.mkstemp()` without `dir=` + +These default to `%TEMP%`, which the Layer 1 guard blocks. Use: +- `tempfile.mkdtemp(dir="tests/artifacts/")` (explicit under tests) +- `tmp_path` pytest fixture (resolves under `--basetemp`) +- `tmp_path_factory.mktemp("name")` (same) + +### 3. Writing to `/*.toml` or `/*.ini` + +The Layer 1 guard raises `TEST_SANDBOX_VIOLATION` on any write to a top-level TOML/INI file. Move the file under `tests/artifacts/`. + +### 4. `Path(__file__).parent.parent / "config.toml"` + +This pattern is a `..` traversal to the project root. Flagged by Layer 4 static audit. + +## Audit Enforcement + +- **Layer 4** runs as a pre-commit hook + CI gate (`--strict` mode) +- **Layer 1** fires at pytest runtime; cannot be bypassed without deleting `tests/conftest.py:_sandbox_audit_hook` +- **Layer 2** is enforced by `pyproject.toml` addopts; cannot be overridden per-invocation + +## See Also + +- `conductor/code_styleguides/workspace_paths.md` — the existing test-workspace rule (extended by this track) +- `conductor/code_styleguides/feature_flags.md` — file-presence = enabled convention +- `conductor/tech-stack.md` §"pyproject.toml pytest addopts" — dated note explaining `--basetemp` +- `scripts/audit_no_temp_writes.py` — pattern reference for Layer 4 audit +- `scripts/tier2/run_tier2_sandboxed.ps1` — pattern reference for Layer 3 wrapper +- `conductor/tracks/test_sandbox_hardening_20260619/` — this track's spec + plan + state +- `conductor/tracks/workspace_path_finalize_20260609/` — prior track that established `tests/artifacts/` workspace pattern \ No newline at end of file diff --git a/conductor/code_styleguides/workspace_paths.md b/conductor/code_styleguides/workspace_paths.md index 99e4e9e7..9ab92071 100644 --- a/conductor/code_styleguides/workspace_paths.md +++ b/conductor/code_styleguides/workspace_paths.md @@ -146,3 +146,4 @@ tests/artifacts/live_gui_workspace_20260609_201530 - `conductor/workflow.md` §"Process Anti-Patterns" #9 (this rule, added 2026-06-09) - `conductor/tracks/workspace_path_finalize_20260609/` — the track that established this rule - `docs/reports/rag_test_batch_failure_status_20260609_pm3.md` — the audit findings that led to the rule +- `conductor/code_styleguides/test_sandbox.md` — the 4-layer sandbox enforcement model (extends this rule with the `--config` CLI flag + Layer 1 audit hook; added 2026-06-19 per `test_sandbox_hardening_20260619`) diff --git a/docs/guide_testing.md b/docs/guide_testing.md index 7476f549..5bb051ac 100644 --- a/docs/guide_testing.md +++ b/docs/guide_testing.md @@ -53,12 +53,16 @@ The `tests/conftest.py` file defines 7 fixtures. They are listed below in the or **Purpose**: Give every test a fresh, isolated workspace so it cannot pollute the user's real `manual_slop.toml`, `presets.toml`, etc. **Mechanism**: -1. Creates a temp directory via `tmp_path_factory.mktemp("isolated_workspace")` -2. Writes a fresh `config.toml` to the temp dir -3. Sets `SLOP_CONFIG`, `SLOP_GLOBAL_PRESETS`, `SLOP_GLOBAL_TOOL_PRESETS`, `SLOP_GLOBAL_PERSONAS`, `SLOP_GLOBAL_WORKSPACE_PROFILES` env vars to point at the temp dir -4. The app reads these env vars on startup; the test sees an isolated world +1. Uses the module-level `_ISOLATION_WORKSPACE = Path(f"tests/artifacts/_isolation_workspace_{_RUN_ID}")` (created at conftest import time) +2. Writes a fresh `config_overrides.toml` to the workspace if it doesn't exist +3. Touches placeholder `presets.toml`, `tool_presets.toml`, `personas.toml`, `workspace_profiles.toml`, `credentials.toml`, `mcp_env.toml` +4. Sets `SLOP_GLOBAL_PRESETS`, `SLOP_GLOBAL_TOOL_PRESETS`, `SLOP_GLOBAL_PERSONAS`, `SLOP_GLOBAL_WORKSPACE_PROFILES`, `SLOP_CREDENTIALS`, `SLOP_MCP_ENV` env vars to point at the workspace files +5. The actual `config.toml` path comes from conftest module body (`_parse_config_arg` parses `--config` from `sys.argv`; auto-defaults to `_ISOLATION_WORKSPACE / "config_overrides.toml"` and calls `paths.set_config_override(...)` BEFORE any `src/` import) +6. The app reads these paths on startup; the test sees an isolated world -**Verification**: `python scripts/check_test_toml_paths.py` exits 0 (no test references real TOMLs). +**Migration history**: As of `test_sandbox_hardening_20260619` (2026-06-19), this fixture no longer uses `tmp_path_factory.mktemp` (which lives in `%TEMP%`) and no longer sets `SLOP_CONFIG` (which is now an unsupported env var). See [Sandbox Hardening](#sandbox-hardening-added-2026-06-19) below. + +**Verification**: `python scripts/check_test_toml_paths.py` and `python scripts/audit_test_sandbox_violations.py` both exit 0. #### `reset_paths` (line 95) @@ -161,6 +165,25 @@ def test_my_thing(live_gui): --- +## Sandbox Hardening (added 2026-06-19) + +Added in `test_sandbox_hardening_20260619` track. The test suite runs under a 4-layer sandbox that prevents any pytest invocation from writing files outside `./tests/`. The user has lost "important sample data" multiple times because tests have silently written to top-level `manual_slop.toml`, `config.toml`, `presets.toml`, etc. + +**The 4 layers:** + +| Layer | Mechanism | Default-on? | +|---|---|---| +| 1. Python runtime guard | `sys.addaudithook` in `tests/conftest.py:_sandbox_audit_hook` raises `RuntimeError("TEST_SANDBOX_VIOLATION")` on writes outside `./tests/` | Yes | +| 2. Workspace migration | `pyproject.toml --basetemp=tests/artifacts/_pytest_tmp` + `isolate_workspace` uses `_ISOLATION_WORKSPACE` under `tests/artifacts/` (no more `tmp_path_factory.mktemp`) | Yes | +| 3. OS-level wrapper | `scripts/run_tests_sandboxed.ps1` (Windows restricted-token + Job Object) | **Opt-in** | +| 4. Static audit | `scripts/audit_test_sandbox_violations.py` flags hardcoded paths + `tempfile.mkdtemp()` without `dir=` | Yes (informational) / opt-in (`--strict`) | + +**Root-cause fix**: `SLOP_CONFIG` env var is no longer consulted by `src/paths.py:get_config_path()`. The CLI flag `--config ` is the ONLY supported mechanism for overriding the default `/config.toml`. `sloppy.py` accepts `--config`. `tests/conftest.py` parses sys.argv at module body (BEFORE any src/ import) and auto-defaults to `tests/artifacts/_isolation_workspace_/config_overrides.toml`. + +**For full details see**: `conductor/code_styleguides/test_sandbox.md`. Regression tests in `tests/test_test_sandbox.py` cover all 4 layers. + +--- + ## Per-test Subprocess Resilience (2026-06-09) Added in `test_infrastructure_hardening_20260609` track. These three mechanisms address the "subprocess state pollution" and "controller state pollution" failure modes that caused batch regressions.