diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..b569087 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,44 @@ +name: 🧪 Tests + +on: + push: + branches: + - main + pull_request: + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + pytest: + name: 🐍 ${{ matrix.os }} · py${{ matrix.python }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + # Ubuntu only while this branch is based on a main that predates the + # Windows support in #57: the Windows lanes there exercise platform + # code this branch does not carry, and fail on main's known + # POSIX-only layers (os.killpg and friends). #57 brings the full + # ubuntu + windows matrix; on merge, its version of this file wins. + os: [ubuntu-latest] + python: ["3.10", "3.14"] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + + - name: 📦 Install with test dependencies + run: python -m pip install -e ".[test]" + + - name: 🧪 Run the suite + # The Windows symlink fixtures skip themselves when the runner lacks + # SeCreateSymbolicLinkPrivilege; those skips are expected and green. + run: python -m pytest -q diff --git a/src/lh_harness/adapters/claude_permissions.py b/src/lh_harness/adapters/claude_permissions.py index c41f903..75b611f 100644 --- a/src/lh_harness/adapters/claude_permissions.py +++ b/src/lh_harness/adapters/claude_permissions.py @@ -168,6 +168,7 @@ def workspace_snapshot_diff( deleted = sorted(before_paths - after_paths) changed: list[str] = [] type_changed: list[str] = [] + touched_dirs: list[str] = [] for path in sorted(before_paths & after_paths): old = before.records[path] new = after.records[path] @@ -175,8 +176,25 @@ def workspace_snapshot_diff( continue if old and new and old[0] != new[0]: type_changed.append(path) + elif old and new and old[0] == "dir" and old[:2] == new[:2]: + # Only the directory's mtime moved. Every child is manifested + # separately, so if none of them shows up in this diff the durable + # content is untouched -- what happened is a transient entry, most + # commonly a git lock file created and removed inside the window. + # Recorded (below) but not a mutation: invalidating audits over + # lock traffic teaches operators to ignore integrity failures. + touched_dirs.append(path) else: changed.append(path) + # A transient entry is only provably transient if the directory's real + # children are clean: when anything under a touched directory did change, + # the directory row rejoins the mutation list. + dirty = set(added) | set(deleted) | set(changed) | set(type_changed) + reinstated = [ + d for d in touched_dirs if any(entry.startswith(d + "/") for entry in dirty) + ] + changed = sorted([*changed, *reinstated]) + touched_dirs = [d for d in touched_dirs if d not in reinstated] return { "verifier_workspace_guard": True, "verifier_workspace_restore_on_mutation": True, @@ -194,6 +212,7 @@ def workspace_snapshot_diff( "deleted": len(deleted), "type_changed": len(type_changed), }, + "verifier_workspace_dir_mtime_only": touched_dirs[:100], "verifier_workspace_snapshot_errors": [*before.errors, *after.errors][:100], } diff --git a/src/lh_harness/cli.py b/src/lh_harness/cli.py index d78f70c..3f34648 100644 --- a/src/lh_harness/cli.py +++ b/src/lh_harness/cli.py @@ -531,6 +531,14 @@ def add_command(name: str, help_text: str) -> argparse.ArgumentParser: default=None, help="Extra directory to expose to the agent. May be repeated.", ) + run_parser.add_argument( + "--guard-exclude-git", + action=argparse.BooleanOptionalAction, + default=run_default("guard_exclude_git", False), + help="Deliberately drop .git from the auditor guard's snapshots. This is an " + "audit blind spot (hooks, refs, history become unwatched); meant for " + "workspaces where concurrent runs legitimately share one repository.", + ) run_parser.add_argument( "--guard-exclude-path", action="append", @@ -1606,6 +1614,16 @@ def _run_command(args: argparse.Namespace) -> int: except ValueError as exc: print(f"Cannot start run: {exc}", file=sys.stderr) return 2 + if getattr(args, "guard_exclude_git", False): + # The one exclusion the list form refuses, admitted only through its + # own named switch so the blind spot is a deliberate, visible line in + # the config -- and in this console record and every audit's metadata. + guard_exclude_paths = (*guard_exclude_paths, str(Path(workspace).resolve() / ".git")) + print( + "Guard: .git is EXCLUDED from audit snapshots by operator choice " + "(hooks/refs/history are unwatched).", + file=sys.stderr, + ) print(f"Run id: {run_id}") print(f"Run dir: {run_dir.resolve()}") @@ -1929,20 +1947,42 @@ def _resolve_guard_exclude_paths( f"guard exclude path would disable the read-only guard entirely: {item!r}" ) if ".git" in candidate.relative_to(workspace).parts: + # Exclusion switches off the witness, not the access: agents keep + # Bash over excluded paths, and unwatched .git means hooks, refs + # and history can be rewritten with no audit trace. Legitimate + # auditor git noise is already silenced via GIT_OPTIONAL_LOCKS=0. raise ValueError( - f"guard exclude path may not touch version-control state: {item!r}" + f"guard exclude path may not touch version-control state: {item!r} " + "(an unwatched .git would hide hook/history tampering from the audit; " + "if that is a deliberate trade-off, say so explicitly with " + "guard_exclude_git = true / --guard-exclude-git)" ) - clashing = next( + already_hidden = next( ( shielded for shielded in protected_paths - if shielded.is_relative_to(candidate) or candidate.is_relative_to(shielded) + if candidate == shielded or candidate.is_relative_to(shielded) ), None, ) + if already_hidden is not None: + # Harness-owned paths are never snapshotted in the first place, so + # listing one (or anything inside one) is redundancy, not a hole. + # Say so instead of failing the run over a harmless line. + print( + f"Note: guard exclude {item!r} is already skipped automatically " + f"(harness-owned: {already_hidden}); ignoring.", + file=sys.stderr, + ) + continue + clashing = next( + (shielded for shielded in protected_paths if shielded.is_relative_to(candidate)), + None, + ) if clashing is not None: raise ValueError( - f"guard exclude path may not cover harness state ({clashing}): {item!r}" + f"guard exclude path may not cover harness state and workspace " + f"content together ({clashing}): {item!r}" ) value = str(candidate) if value not in resolved: diff --git a/src/lh_harness/config.py b/src/lh_harness/config.py index 659d134..49e8bd8 100644 --- a/src/lh_harness/config.py +++ b/src/lh_harness/config.py @@ -40,6 +40,7 @@ "codex_mcp_config", "mcp_add_dirs", "guard_exclude_paths", + "guard_exclude_git", "max_rounds", "dashboard", "dashboard_port", @@ -92,6 +93,12 @@ # echoed at run start and recorded in each audited episode's metadata. # Passing --guard-exclude-path replaces this list rather than adding to it. guard_exclude_paths = [] +# Deliberately drop .git from audit snapshots. This is an audit blind spot -- +# hooks, refs and history become unwatched -- so it has its own named switch +# instead of hiding in the list above. Meant for workspaces where concurrent +# runs legitimately share one repository and every sibling commit would +# otherwise invalidate an open audit window. +# guard_exclude_git = true max_rounds = 25 dashboard = true @@ -212,6 +219,8 @@ def _flatten_run_table(run: dict[str, Any]) -> dict[str, Any]: if not isinstance(value, list) or not all(isinstance(item, str) and item for item in value): raise ProjectConfigError("run.mcp_add_dirs must be an array of non-empty strings") defaults["mcp_add_dir"] = list(value) + if "guard_exclude_git" in run: + defaults["guard_exclude_git"] = _boolean(run["guard_exclude_git"], "run.guard_exclude_git") if "guard_exclude_paths" in run: value = run["guard_exclude_paths"] if not isinstance(value, list) or not all(isinstance(item, str) and item for item in value): diff --git a/tests/test_guard_exclude_paths.py b/tests/test_guard_exclude_paths.py index 8887b5f..1f1a60b 100644 --- a/tests/test_guard_exclude_paths.py +++ b/tests/test_guard_exclude_paths.py @@ -70,17 +70,33 @@ def test_version_control_state_is_protected(tmp_path: Path, vcs_path: str) -> No _resolve_guard_exclude_paths([vcs_path], workspace=workspace, protected=()) -def test_harness_state_paths_are_protected(tmp_path: Path) -> None: +def test_redundant_harness_state_exclusion_is_a_note_not_an_error( + tmp_path: Path, capsys +) -> None: workspace = _workspace(tmp_path) run_dir = workspace / "runs" / "run-1" - # Excluding the harness path itself, or any parent that covers it, would - # hide the run's own control/state files from the guard. - for candidate in ("runs/run-1", "runs"): - with pytest.raises(ValueError, match="harness state"): - _resolve_guard_exclude_paths( - [candidate], workspace=workspace, protected=(run_dir,) - ) + # Harness-owned paths are never snapshotted anyway, so naming one (or + # anything inside one) must not fail the run -- operators reasonably list + # .lh-harness for completeness. + resolved = _resolve_guard_exclude_paths( + ["runs/run-1", "runs/run-1/logs", "target"], + workspace=workspace, + protected=(run_dir,), + ) + + assert resolved == (str(workspace / "target"),) + assert "already skipped automatically" in capsys.readouterr().err + + +def test_exclusion_covering_harness_state_and_more_is_rejected(tmp_path: Path) -> None: + workspace = _workspace(tmp_path) + run_dir = workspace / "runs" / "run-1" + + # "runs" holds run-1 *and* whatever else lands there: wider than the + # harness state it hides, so it is a real hole, not redundancy. + with pytest.raises(ValueError, match="harness state"): + _resolve_guard_exclude_paths(["runs"], workspace=workspace, protected=(run_dir,)) def test_sibling_of_harness_state_is_allowed(tmp_path: Path) -> None: @@ -206,3 +222,54 @@ def test_run_parses_repeatable_options_without_inheriting_the_config(monkeypatch # A second call in the same process must not accumulate. assert cli.main(["run", "--task=t", "--guard-exclude-path=cli-guard"]) == 0 assert captured[-1] == (["cli-guard"], ["cfg-dir"]) + + +# --- dir-mtime-only diffs and the explicit .git switch ----------------------- + + +def test_dir_mtime_only_change_is_a_note_not_a_mutation() -> None: + from lh_harness.adapters.claude_permissions import ( + WorkspaceSnapshot, + workspace_snapshot_diff, + ) + + before = WorkspaceSnapshot( + records={".git": ("dir", 0o755, 100), ".git/HEAD": ("file", 0o644, 23, 5, "x")}, + errors=(), + ) + after = WorkspaceSnapshot( + # Same children, the directory's own mtime moved: a transient entry + # (git lock) came and went. Must not read as tampering. + records={".git": ("dir", 0o755, 999), ".git/HEAD": ("file", 0o644, 23, 5, "x")}, + errors=(), + ) + diff = workspace_snapshot_diff(before, after) + assert diff["verifier_workspace_mutation_detected"] is False + assert diff["verifier_workspace_dir_mtime_only"] == [".git"] + + +def test_dir_mtime_change_with_dirty_children_stays_a_mutation() -> None: + from lh_harness.adapters.claude_permissions import ( + WorkspaceSnapshot, + workspace_snapshot_diff, + ) + + before = WorkspaceSnapshot( + records={".git": ("dir", 0o755, 100), ".git/HEAD": ("file", 0o644, 23, 5, "x")}, + errors=(), + ) + after = WorkspaceSnapshot( + records={".git": ("dir", 0o755, 999), ".git/HEAD": ("file", 0o644, 23, 9, "y")}, + errors=(), + ) + diff = workspace_snapshot_diff(before, after) + assert diff["verifier_workspace_mutation_detected"] is True + assert ".git" in diff["verifier_workspace_mutations"]["changed"] + assert ".git/HEAD" in diff["verifier_workspace_mutations"]["changed"] + assert diff["verifier_workspace_dir_mtime_only"] == [] + + +def test_listing_git_still_points_at_the_named_switch(tmp_path: Path) -> None: + workspace = _workspace(tmp_path) + with pytest.raises(ValueError, match="guard_exclude_git"): + _resolve_guard_exclude_paths([".git"], workspace=workspace, protected=())