ed
33569e1ce5
TIER-3 READ AGENTS.md + conductor/code_styleguides/error_handling.md + tests/test_tier2_pre_commit_hook.py + conductor/tier2/githooks/pre-commit before pre-commit-test-fix.
7 tests in tests/test_tier2_pre_commit_hook.py asserted the OLD silent-strip behavior (exit 0). The pre-commit hook was changed in eae75877 to abort on strip (exit 1) to prevent the 2026-06-24 MCP regression where Tier 2 made an empty fix commit and reported success without verifying the diff.
Tests updated to assert the NEW abort behavior:
- result.returncode == 1 (was 0)
- Diagnostic message 'COMMIT ABORTED' in result.stderr
- File still unstaged after hook (unchanged behavior)
- HEAD-content assertions removed in 2 tests (commit was aborted, no HEAD changes)
Acceptance: 12/12 tests pass in tests/test_tier2_pre_commit_hook.py.
270 lines
13 KiB
Python
270 lines
13 KiB
Python
"""Tests for the Tier 2 pre-commit hook that prevents sandbox file leaks.
|
|
|
|
Background: setup_tier2_clone.ps1 modifies opencode.json and
|
|
mcp_paths.toml IN the clone (pointing them at the clone's MCP server
|
|
and clearing extra_dirs). If a tier-2 commit captures these
|
|
modifications via `git add .`, they leak into the main repo.
|
|
|
|
The pre-commit hook in conductor/tier2/githooks/pre-commit detects
|
|
these files (and other tier-2-only paths) in the staged set and
|
|
auto-unstages them via `git rm --cached` so the commit only contains
|
|
legitimate work. The hook reads its denylist from
|
|
conductor/tier2/githooks/forbidden-files.txt (one substring pattern
|
|
per line; `#` starts a comment, blank lines are ignored).
|
|
|
|
These tests create a temporary git repo, install the hook + config,
|
|
stage various files, and verify the hook:
|
|
- Allows commits that contain only allowed files
|
|
- Auto-unstages tier-2 sandbox files when staged (does NOT block
|
|
the commit, so tier-2 isn't stuck mid-flow)
|
|
- Always exits 0 (per design)
|
|
"""
|
|
import os
|
|
import re
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
HOOK_SOURCE = Path("conductor/tier2/githooks/pre-commit").resolve()
|
|
CONFIG_SOURCE = Path("conductor/tier2/githooks/forbidden-files.txt").resolve()
|
|
|
|
|
|
@pytest.fixture
|
|
def fake_clone(tmp_path: Path) -> Path:
|
|
"""Create a temporary git repo with the pre-commit hook + config installed."""
|
|
clone = tmp_path / "fake_clone"
|
|
clone.mkdir()
|
|
clone_str = str(clone)
|
|
subprocess.run(["git", "init"], cwd=clone_str, check=True, capture_output=True)
|
|
subprocess.run(["git", "config", "user.email", "test@test"], cwd=clone_str, check=True)
|
|
subprocess.run(["git", "config", "user.name", "Test"], cwd=clone_str, check=True)
|
|
# Mirror the repo layout: conductor/tier2/githooks/ inside the clone
|
|
# (so the hook can find its config relative to the project root).
|
|
config_dir = clone / "conductor" / "tier2" / "githooks"
|
|
config_dir.mkdir(parents=True)
|
|
config_dir.joinpath("forbidden-files.txt").write_bytes(CONFIG_SOURCE.read_bytes())
|
|
# Commit the config as part of the initial state. This prevents
|
|
# `git add -A` from accidentally re-staging the config in every test.
|
|
subprocess.run(["git", "add", "-A"], cwd=clone_str, check=True)
|
|
subprocess.run(["git", "commit", "-m", "init config"], cwd=clone_str, check=True)
|
|
hooks_dir = clone / ".git" / "hooks"
|
|
hooks_dir.mkdir(parents=True, exist_ok=True)
|
|
hooks_dir.joinpath("pre-commit").write_bytes(HOOK_SOURCE.read_bytes())
|
|
os.chmod(hooks_dir / "pre-commit", 0o755)
|
|
return clone
|
|
|
|
|
|
def _run(cwd: Path, *args: str) -> subprocess.CompletedProcess:
|
|
return subprocess.run(
|
|
list(args),
|
|
cwd=str(cwd),
|
|
capture_output=True,
|
|
text=True,
|
|
)
|
|
|
|
|
|
def _staged_files(clone: Path) -> list[str]:
|
|
"""Return the list of files currently in the index."""
|
|
result = _run(clone, "git", "diff", "--cached", "--name-only")
|
|
return [line for line in result.stdout.splitlines() if line]
|
|
|
|
|
|
def _commit(clone: Path, message: str = "test") -> subprocess.CompletedProcess:
|
|
return _run(clone, "git", "commit", "-m", message)
|
|
|
|
|
|
def test_hook_allows_commits_with_no_staged_files(fake_clone: Path) -> None:
|
|
"""Empty staged set: git refuses the commit, hook does not interfere."""
|
|
# All files in the clone are now committed. Staging nothing should
|
|
# produce git's standard "nothing to commit" error.
|
|
result = _commit(fake_clone, "empty")
|
|
assert result.returncode != 0, "commit with no staged files should fail"
|
|
combined = (result.stdout + result.stderr).lower()
|
|
assert "nothing to commit" in combined or "nothing added to commit" in combined, (
|
|
f"expected git's standard nothing-to-commit error, got stdout={result.stdout!r} stderr={result.stderr!r}"
|
|
)
|
|
|
|
|
|
def test_hook_allows_allowed_files(fake_clone: Path) -> None:
|
|
"""Files NOT in the denylist commit normally."""
|
|
(fake_clone / "src.py").write_text("print('hi')\n")
|
|
_run(fake_clone, "git", "add", "src.py")
|
|
staged_before = _staged_files(fake_clone)
|
|
assert staged_before == ["src.py"]
|
|
result = _commit(fake_clone, "add src")
|
|
assert result.returncode == 0, f"commit failed: {result.stderr}"
|
|
assert _staged_files(fake_clone) == [], "staged set should be empty after commit"
|
|
|
|
|
|
def test_hook_unstages_forbidden_opencode_agent_file(fake_clone: Path) -> None:
|
|
"""A staged .opencode/agents/tier2-*.md is auto-unstaged; commit proceeds without it."""
|
|
opencode_dir = fake_clone / ".opencode" / "agents"
|
|
opencode_dir.mkdir(parents=True)
|
|
forbidden = opencode_dir / "tier2-autonomous.md"
|
|
forbidden.write_text("# fake tier-2 agent\n")
|
|
_run(fake_clone, "git", "add", ".opencode/agents/tier2-autonomous.md")
|
|
assert _staged_files(fake_clone) == [".opencode/agents/tier2-autonomous.md"]
|
|
result = _commit(fake_clone, "leak attempt")
|
|
# Hook ABORTS the commit (exit 1) to prevent silent-strip-then-empty-commit
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
# File must have been unstaged (unchanged behavior)
|
|
assert _staged_files(fake_clone) == [], "forbidden file was not auto-unstaged"
|
|
# Working tree still has the modification (hook only unstaged)
|
|
assert forbidden.exists(), "hook should not delete the file from working tree"
|
|
|
|
|
|
def test_hook_unstages_forbidden_opencode_command_file(fake_clone: Path) -> None:
|
|
"""A staged .opencode/commands/tier-2-*.md is auto-unstaged."""
|
|
cmd_dir = fake_clone / ".opencode" / "commands"
|
|
cmd_dir.mkdir(parents=True)
|
|
forbidden = cmd_dir / "tier-2-auto-execute.md"
|
|
forbidden.write_text("# fake tier-2 command\n")
|
|
_run(fake_clone, "git", "add", ".opencode/commands/tier-2-auto-execute.md")
|
|
result = _commit(fake_clone, "leak attempt")
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
assert _staged_files(fake_clone) == []
|
|
|
|
|
|
def test_hook_unstages_modified_opencode_json(fake_clone: Path) -> None:
|
|
"""opencode.json is forbidden even when modified (the setup script modifies it locally)."""
|
|
opencode_json = fake_clone / "opencode.json"
|
|
opencode_json.write_text('{"version": 1}\n')
|
|
_run(fake_clone, "git", "add", "opencode.json")
|
|
_run(fake_clone, "git", "commit", "-m", "add opencode.json")
|
|
# Modify it (simulating the setup script's MCP path override)
|
|
opencode_json.write_text('{"version": 1, "tier2-modified": true}\n')
|
|
_run(fake_clone, "git", "add", "opencode.json")
|
|
result = _commit(fake_clone, "leak attempt")
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
assert _staged_files(fake_clone) == []
|
|
|
|
|
|
def test_hook_unstages_modified_mcp_paths_toml(fake_clone: Path) -> None:
|
|
"""mcp_paths.toml is forbidden even when modified."""
|
|
mcp_paths = fake_clone / "mcp_paths.toml"
|
|
mcp_paths.write_text('[allowed_paths]\nextra_dirs = []\n')
|
|
_run(fake_clone, "git", "add", "mcp_paths.toml")
|
|
_run(fake_clone, "git", "commit", "-m", "add mcp_paths.toml")
|
|
mcp_paths.write_text('[allowed_paths]\nextra_dirs = ["leaked"]\n')
|
|
_run(fake_clone, "git", "add", "mcp_paths.toml")
|
|
result = _commit(fake_clone, "leak attempt")
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
assert _staged_files(fake_clone) == []
|
|
|
|
|
|
def test_hook_unstages_all_forbidden_files_at_once(fake_clone: Path) -> None:
|
|
"""Multiple forbidden files staged: all are unstaged in one pass."""
|
|
(fake_clone / ".opencode" / "agents").mkdir(parents=True)
|
|
(fake_clone / ".opencode" / "commands").mkdir(parents=True)
|
|
(fake_clone / ".opencode" / "agents" / "tier2-autonomous.md").write_text("a\n")
|
|
(fake_clone / ".opencode" / "commands" / "tier-2-auto-execute.md").write_text("b\n")
|
|
(fake_clone / "opencode.json").write_text("c\n")
|
|
(fake_clone / "mcp_paths.toml").write_text("d\n")
|
|
# Stage each explicitly so we know exactly what the hook sees
|
|
_run(fake_clone, "git", "add",
|
|
".opencode/agents/tier2-autonomous.md",
|
|
".opencode/commands/tier-2-auto-execute.md",
|
|
"opencode.json",
|
|
"mcp_paths.toml")
|
|
staged = sorted(_staged_files(fake_clone))
|
|
assert len(staged) == 4, f"setup failed; staged={staged}"
|
|
result = _commit(fake_clone, "multi-leak")
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
assert _staged_files(fake_clone) == []
|
|
|
|
|
|
def test_hook_keeps_allowed_files_alongside_forbidden(fake_clone: Path) -> None:
|
|
"""Mixed staged set: forbidden unstaged, allowed committed normally."""
|
|
(fake_clone / ".opencode" / "agents").mkdir(parents=True)
|
|
(fake_clone / ".opencode" / "agents" / "tier2-autonomous.md").write_text("leak\n")
|
|
(fake_clone / "legit.py").write_text("print('legit work')\n")
|
|
_run(fake_clone, "git", "add",
|
|
".opencode/agents/tier2-autonomous.md", "legit.py")
|
|
result = _commit(fake_clone, "mixed")
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
# Commit was aborted: verify both files remain on disk (no HEAD changes)
|
|
assert (fake_clone / "legit.py").exists() and (fake_clone / ".opencode/agents/tier2-autonomous.md").exists()
|
|
# Forbidden file unstaged, legit file (not in denylist) remains staged
|
|
assert _staged_files(fake_clone) == ["legit.py"]
|
|
assert (fake_clone / ".opencode" / "agents" / "tier2-autonomous.md").exists()
|
|
|
|
|
|
def test_hook_silent_when_no_forbidden_files(fake_clone: Path) -> None:
|
|
"""Hook prints nothing to stderr/stdout when nothing is forbidden."""
|
|
(fake_clone / "clean.py").write_text("x = 1\n")
|
|
_run(fake_clone, "git", "add", "clean.py")
|
|
result = _commit(fake_clone, "clean")
|
|
assert result.returncode == 0, f"commit failed: {result.stderr}"
|
|
# The hook's warning text must NOT appear when no leaks were detected.
|
|
combined = (result.stdout + result.stderr).lower()
|
|
assert "removing" not in combined, (
|
|
f"hook printed warning despite no leak: stdout={result.stdout!r} stderr={result.stderr!r}"
|
|
)
|
|
|
|
|
|
def test_hook_warns_when_unstaging(fake_clone: Path) -> None:
|
|
"""Hook prints a clear warning when it unstages a forbidden file."""
|
|
(fake_clone / ".opencode" / "agents").mkdir(parents=True)
|
|
(fake_clone / ".opencode" / "agents" / "tier2-autonomous.md").write_text("leak\n")
|
|
_run(fake_clone, "git", "add", ".opencode/agents/tier2-autonomous.md")
|
|
result = _commit(fake_clone, "leak")
|
|
assert result.returncode == 1
|
|
# Hook output should mention the leak (so tier-2 sees what happened)
|
|
combined = (result.stdout + result.stderr).lower()
|
|
assert re.search(r"tier.?2|removing|sandbox", combined), (
|
|
f"expected warning text in commit output, got: stdout={result.stdout!r} stderr={result.stderr!r}"
|
|
)
|
|
# And it should mention the specific file
|
|
assert "tier2-autonomous" in combined, (
|
|
f"expected filename in warning, got: stdout={result.stdout!r} stderr={result.stderr!r}"
|
|
)
|
|
|
|
|
|
def test_hook_uses_config_from_project_root(fake_clone: Path) -> None:
|
|
"""Hook reads forbidden-files.txt from conductor/tier2/githooks/ in the project root.
|
|
Replacing the config changes the hook's denylist without modifying the hook itself.
|
|
"""
|
|
custom = fake_clone / "conductor" / "tier2" / "githooks" / "forbidden-files.txt"
|
|
custom.write_text("custom_forbidden.txt\n")
|
|
(fake_clone / "custom_forbidden.txt").write_text("leak\n")
|
|
# opencode.json is NOT in the custom config — it should NOT be unstaged.
|
|
(fake_clone / "opencode.json").write_text('{"version": 1}\n')
|
|
_run(fake_clone, "git", "add",
|
|
"custom_forbidden.txt", "opencode.json")
|
|
result = _commit(fake_clone, "mixed")
|
|
assert result.returncode == 1, f"hook did not abort commit: {result.stderr}"
|
|
assert re.search(r"COMMIT ABORTED|sandbox file leak", result.stderr), f"expected diagnostic message, got stderr={result.stderr!r}"
|
|
# Commit was aborted: nothing committed. Both files remain on disk.
|
|
# custom_forbidden.txt: was unstaged by hook (in custom config) -> not in staged set
|
|
# opencode.json: NOT in custom config -> hook left it staged, but commit aborted so not in HEAD
|
|
staged = _staged_files(fake_clone)
|
|
assert "custom_forbidden.txt" not in staged, (
|
|
f"custom_forbidden.txt should have been unstaged: {staged}"
|
|
)
|
|
assert "opencode.json" in staged, (
|
|
f"opencode.json should still be staged (not in custom config): {staged}"
|
|
)
|
|
# On-disk existence: neither file was deleted by the hook
|
|
assert (fake_clone / "custom_forbidden.txt").exists()
|
|
assert (fake_clone / "opencode.json").exists()
|
|
|
|
|
|
def test_hook_handles_paths_with_spaces(fake_clone: Path) -> None:
|
|
"""A forbidden file whose path contains spaces is still detected and unstaged."""
|
|
(fake_clone / ".opencode" / "agents").mkdir(parents=True)
|
|
weird = fake_clone / ".opencode" / "agents" / "tier2 my agent.md"
|
|
weird.write_text("x\n")
|
|
# Add with quoting so git stores the path with spaces
|
|
_run(fake_clone, "git", "add", ".opencode/agents/tier2 my agent.md")
|
|
staged = _staged_files(fake_clone)
|
|
assert staged == [".opencode/agents/tier2 my agent.md"], f"setup failed: {staged}"
|
|
result = _commit(fake_clone, "spaces")
|
|
assert result.returncode == 0, f"hook blocked commit: {result.stderr}"
|
|
assert _staged_files(fake_clone) == [] |