diff --git a/rein_aharness/repository_transaction.py b/rein_aharness/repository_transaction.py index bfc77b3..c913eb4 100644 --- a/rein_aharness/repository_transaction.py +++ b/rein_aharness/repository_transaction.py @@ -8,9 +8,11 @@ boundary is still under owner review. from __future__ import annotations import fcntl +import fnmatch import hashlib import json import os +import stat import subprocess import uuid from dataclasses import dataclass @@ -50,6 +52,49 @@ class DirtyRepositoryError(RepositoryTransactionError): ) +class RepositoryAcceptanceError(RepositoryTransactionError): + """The post-run repository state does not satisfy its explicit policy.""" + + def __init__(self, code: str, detail: str) -> None: + self.code = _bounded_text(code, 80) + self.detail = _bounded_text(detail, 300) + super().__init__(f"repository acceptance failed: {self.code}: {self.detail}") + + +@dataclass(frozen=True) +class RepositoryAcceptancePolicy: + """Bounded local-commit policy supplied by a future governed run grant.""" + + allowed_paths: tuple[str, ...] + min_commits: int = 1 + max_commits: int = 1 + max_evidence_paths: int = 100 + + def __post_init__(self) -> None: + if not 1 <= self.min_commits <= self.max_commits <= 32: + raise ValueError("commit bounds must satisfy 1 <= min <= max <= 32") + if not 0 <= self.max_evidence_paths <= 200: + raise ValueError("max_evidence_paths must be between 0 and 200") + if len(self.allowed_paths) > 100: + raise ValueError("allowed_paths must contain at most 100 patterns") + for pattern in self.allowed_paths: + _validate_path_pattern(pattern) + + @property + def policy_id(self) -> str: + payload = json.dumps( + { + "allowed_paths": self.allowed_paths, + "min_commits": self.min_commits, + "max_commits": self.max_commits, + "max_evidence_paths": self.max_evidence_paths, + }, + sort_keys=True, + separators=(",", ":"), + ) + return _digest(payload)[:32] + + @dataclass(frozen=True) class RepositoryBaseline: """Exact local facts captured while the canonical repository lock is held.""" @@ -67,6 +112,7 @@ class RepositoryBaseline: upstream_ref: str | None upstream_oid: str | None remote_refs: tuple[tuple[str, str], ...] + protected_git_metadata_digest: str @property def detached(self) -> bool: @@ -90,6 +136,37 @@ class RepositoryBaseline: "upstream_oid": self.upstream_oid, "remote_ref_count": len(self.remote_refs), "remote_refs_digest": _digest(remote_payload), + "protected_git_metadata_digest": self.protected_git_metadata_digest, + } + + +@dataclass(frozen=True) +class RepositoryAcceptance: + """Validated, bounded evidence for one accepted local repository result.""" + + policy_id: str + head: str + branch: str | None + commits: tuple[str, ...] + changed_paths: tuple[str, ...] + max_evidence_paths: int + + def evidence(self) -> dict[str, Any]: + visible_paths = self.changed_paths[: self.max_evidence_paths] + return { + "policy_id": self.policy_id, + "accepted": True, + "head": self.head, + "branch": self.branch, + "commit_count": len(self.commits), + "commits": list(self.commits), + "changed_path_count": len(self.changed_paths), + "changed_paths": list(visible_paths), + "changed_paths_truncated": len(visible_paths) < len(self.changed_paths), + "changed_paths_digest": _digest("\0".join(self.changed_paths)), + "clean_post_state": True, + "remote_refs_unchanged": True, + "protected_git_metadata_unchanged": True, } @@ -116,6 +193,7 @@ class RepositoryTransaction: self.require_clean = require_clean self.transaction_id = uuid.uuid4().hex self.baseline: RepositoryBaseline | None = None + self.acceptance: RepositoryAcceptance | None = None self.lock_path: Path | None = None self._lock_file: TextIO | None = None @@ -209,11 +287,125 @@ class RepositoryTransaction: """Return the bounded transaction and baseline result envelope.""" if self.baseline is None: raise RepositoryTransactionError("repository baseline is not captured") - return { + evidence = { "transaction_id": self.transaction_id, "correlation_id": self.correlation_id, "baseline": self.baseline.evidence(), } + if self.acceptance is not None: + evidence["acceptance"] = self.acceptance.evidence() + return evidence + + def validate_acceptance( + self, + policy: RepositoryAcceptancePolicy, + ) -> RepositoryAcceptance: + """Validate the current checkout without changing it or releasing its lock.""" + if not self.locked or self.baseline is None: + raise RepositoryTransactionError( + "repository acceptance requires an active transaction" + ) + + baseline = self.baseline + post = _capture_baseline( + baseline.repo_root, + baseline.git_common_dir, + baseline.repo_id, + ) + if post.branch != baseline.branch: + raise RepositoryAcceptanceError( + "branch-changed", + f"expected={baseline.branch or 'detached'} actual={post.branch or 'detached'}", + ) + if post.head == baseline.head: + raise RepositoryAcceptanceError("head-unchanged", "no new commit to accept") + + ancestry = _run_git( + baseline.repo_root, + "merge-base", + "--is-ancestor", + baseline.head, + post.head, + ) + if ancestry.returncode == 1: + raise RepositoryAcceptanceError( + "non-descendant-head", + "post-run HEAD does not descend from the transaction baseline", + ) + if ancestry.returncode != 0: + reason = (ancestry.stderr or ancestry.stdout or "Git ancestry error").strip() + raise GitRepositoryError(f"cannot validate commit ancestry: {reason[:300]}") + + commit_lines = _git( + baseline.repo_root, + "rev-list", + "--reverse", + f"{baseline.head}..{post.head}", + ).splitlines() + commits = tuple(line for line in commit_lines if line) + if not policy.min_commits <= len(commits) <= policy.max_commits: + raise RepositoryAcceptanceError( + "commit-count", + f"expected={policy.min_commits}..{policy.max_commits} actual={len(commits)}", + ) + + paths_result = _run_git( + baseline.repo_root, + "diff", + "--name-only", + "-z", + baseline.head, + post.head, + "--", + ) + if paths_result.returncode != 0: + reason = (paths_result.stderr or paths_result.stdout or "Git diff error").strip() + raise GitRepositoryError(f"cannot determine changed paths: {reason[:300]}") + changed_paths = tuple(path for path in paths_result.stdout.split("\0") if path) + if not changed_paths: + raise RepositoryAcceptanceError( + "no-changed-paths", + "accepted commits do not change a repository path", + ) + invalid_paths = tuple( + path for path in changed_paths if not _path_allowed(path, policy.allowed_paths) + ) + if invalid_paths: + visible = ",".join(invalid_paths[:5]) + raise RepositoryAcceptanceError( + "path-not-granted", + f"count={len(invalid_paths)} paths={visible[:220]}", + ) + + if not post.clean: + raise RepositoryAcceptanceError( + "dirty-post-state", + f"entries={post.dirty_entries} status_digest={post.status_digest}", + ) + if post.remote_refs != baseline.remote_refs: + raise RepositoryAcceptanceError( + "remote-refs-changed", + "local remote-tracking refs moved during the transaction", + ) + if ( + post.protected_git_metadata_digest + != baseline.protected_git_metadata_digest + ): + raise RepositoryAcceptanceError( + "git-metadata-changed", + "protected Git config, hooks, or info metadata changed", + ) + + acceptance = RepositoryAcceptance( + policy_id=policy.policy_id, + head=post.head, + branch=post.branch, + commits=commits, + changed_paths=changed_paths, + max_evidence_paths=policy.max_evidence_paths, + ) + self.acceptance = acceptance + return acceptance def _write_metadata(self, value: dict[str, Any]) -> None: if self._lock_file is None: @@ -301,9 +493,68 @@ def _capture_baseline(repo_root: Path, common_dir: Path, repo_id: str) -> Reposi upstream_ref=upstream_ref, upstream_oid=upstream_oid, remote_refs=tuple(remote_refs), + protected_git_metadata_digest=_protected_git_metadata_digest( + repo_root, + common_dir, + ), ) +def _protected_git_metadata_digest(repo_root: Path, common_dir: Path) -> str: + raw_git_dir = Path(_git(repo_root, "rev-parse", "--git-dir")) + git_dir = ( + (repo_root / raw_git_dir).resolve() + if not raw_git_dir.is_absolute() + else raw_git_dir.resolve() + ) + candidates = ( + ("common/config", common_dir / "config"), + ("common/hooks", common_dir / "hooks"), + ("common/info/attributes", common_dir / "info" / "attributes"), + ("common/info/exclude", common_dir / "info" / "exclude"), + ("worktree/config.worktree", git_dir / "config.worktree"), + ) + digest = hashlib.sha256() + for label, path in candidates: + _hash_metadata_path(digest, label, path) + return digest.hexdigest() + + +def _hash_metadata_path(digest: Any, label: str, path: Path) -> None: + digest.update(label.encode("utf-8")) + digest.update(b"\0") + if not path.exists() and not path.is_symlink(): + digest.update(b"missing\0") + return + + paths = [path] + if path.is_dir(): + paths.extend(sorted(path.rglob("*"), key=lambda item: item.as_posix())) + for item in paths: + relative = "." if item == path else item.relative_to(path).as_posix() + try: + item_stat = item.lstat() + mode = stat.S_IMODE(item_stat.st_mode) + digest.update(f"{relative}\0{mode:o}\0".encode("utf-8")) + if item.is_symlink(): + digest.update(b"symlink\0") + digest.update(os.readlink(item).encode("utf-8", errors="surrogateescape")) + elif item.is_file(): + digest.update(b"file\0") + with item.open("rb") as handle: + for chunk in iter(lambda: handle.read(65536), b""): + digest.update(chunk) + elif item.is_dir(): + digest.update(b"dir\0") + else: + digest.update(b"other\0") + digest.update(b"\0") + except OSError as exc: + raise GitRepositoryError( + f"cannot hash protected Git metadata: {label}: {exc}" + ) from exc + + def _run_git(repo: Path, *args: str) -> subprocess.CompletedProcess[str]: try: return subprocess.run( @@ -349,6 +600,56 @@ def _digest(value: str) -> str: return hashlib.sha256(value.encode("utf-8", errors="replace")).hexdigest() +def _validate_path_pattern(pattern: str) -> None: + if not isinstance(pattern, str) or not pattern or len(pattern) > 256: + raise ValueError("path patterns must be non-empty strings of at most 256 characters") + normalized = pattern.rstrip("/") + parts = tuple(normalized.split("/")) + if ( + pattern.startswith("/") + or "\\" in pattern + or "\0" in pattern + or any(part in {"", ".", ".."} for part in parts) + ): + raise ValueError(f"path pattern must be repository-relative: {pattern!r}") + if parts and parts[0] == ".git": + raise ValueError("path patterns cannot grant protected .git metadata") + + +def _path_allowed(path: str, patterns: tuple[str, ...]) -> bool: + normalized = path + parts = tuple(normalized.split("/")) + if ( + normalized.startswith("/") + or "\\" in normalized + or any(part in {"", ".", ".."} for part in parts) + or (parts and parts[0] == ".git") + ): + return False + for raw_pattern in patterns: + if raw_pattern.endswith("/") and normalized.startswith(raw_pattern): + return True + if _match_path_parts(parts, tuple(raw_pattern.split("/"))): + return True + return False + + +def _match_path_parts(path_parts: tuple[str, ...], pattern_parts: tuple[str, ...]) -> bool: + if not pattern_parts: + return not path_parts + pattern = pattern_parts[0] + if pattern == "**": + return _match_path_parts(path_parts, pattern_parts[1:]) or bool( + path_parts + and _match_path_parts(path_parts[1:], pattern_parts) + ) + return bool( + path_parts + and fnmatch.fnmatchcase(path_parts[0], pattern) + and _match_path_parts(path_parts[1:], pattern_parts[1:]) + ) + + def _bounded_text(value: str, limit: int) -> str: cleaned = " ".join(str(value).split()) return cleaned[:limit] diff --git a/tests/test_repository_transaction.py b/tests/test_repository_transaction.py index d55e046..b507d17 100644 --- a/tests/test_repository_transaction.py +++ b/tests/test_repository_transaction.py @@ -12,6 +12,8 @@ import pytest from rein_aharness.repository_transaction import ( DirtyRepositoryError, GitRepositoryError, + RepositoryAcceptanceError, + RepositoryAcceptancePolicy, RepositoryBusyError, RepositoryTransaction, ) @@ -215,6 +217,250 @@ def test_busy_error_exposes_bounded_owner_metadata(tmp_path: Path) -> None: assert len(excinfo.value.owner["correlation_id"]) == 200 +def test_acceptance_proves_one_descendant_commit_on_granted_paths(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + policy = RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/result.md", "accepted\n") + accepted = tx.validate_acceptance(policy) + + assert len(accepted.commits) == 1 + assert accepted.changed_paths == ("docs/result.md",) + evidence = tx.evidence()["acceptance"] + assert evidence["accepted"] is True + assert evidence["policy_id"] == policy.policy_id + assert evidence["changed_paths"] == ["docs/result.md"] + assert evidence["clean_post_state"] is True + assert evidence["remote_refs_unchanged"] is True + + +def test_acceptance_rejects_ungranted_extra_path(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "UNRELATED.md", "not granted\n") + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + ) + + assert excinfo.value.code == "path-not-granted" + assert "UNRELATED.md" in excinfo.value.detail + + +def test_acceptance_rejects_more_commits_than_policy_allows(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/one.md", "one\n") + _commit(repo, "docs/two.md", "two\n") + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + ) + + assert excinfo.value.code == "commit-count" + assert "actual=2" in excinfo.value.detail + + +def test_acceptance_rejects_dirty_post_state(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/result.md", "committed\n") + (repo / "docs" / "result.md").write_text("left dirty\n", encoding="utf-8") + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + ) + + assert excinfo.value.code == "dirty-post-state" + + +def test_acceptance_rejects_remote_tracking_ref_movement(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + subprocess.run( + ["git", "update-ref", "refs/remotes/origin/main", "HEAD"], + cwd=repo, + check=True, + ) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/result.md", "committed\n") + subprocess.run( + ["git", "update-ref", "refs/remotes/origin/main", "HEAD"], + cwd=repo, + check=True, + ) + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + ) + + assert excinfo.value.code == "remote-refs-changed" + + +@pytest.mark.parametrize("metadata_kind", ["config", "hook"]) +def test_acceptance_rejects_protected_git_metadata_changes( + tmp_path: Path, + metadata_kind: str, +) -> None: + repo = _make_repo(tmp_path) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/result.md", "committed\n") + if metadata_kind == "config": + subprocess.run( + ["git", "config", "rein-aharness.test", "changed"], + cwd=repo, + check=True, + ) + else: + hook = repo / ".git" / "hooks" / "pre-commit" + hook.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + ) + + assert excinfo.value.code == "git-metadata-changed" + + +def test_acceptance_rejects_non_descendant_head(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + branch = subprocess.run( + ["git", "branch", "--show-current"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + subprocess.run(["git", "checkout", "--orphan", "unrelated"], cwd=repo, check=True) + subprocess.run(["git", "rm", "-qf", "README.md"], cwd=repo, check=True) + (repo / "docs").mkdir() + (repo / "docs" / "result.md").write_text("unrelated\n", encoding="utf-8") + subprocess.run(["git", "add", "."], cwd=repo, check=True) + subprocess.run( + [ + "git", + "-c", + "user.email=test@example.invalid", + "-c", + "user.name=test", + "commit", + "-qm", + "unrelated", + ], + cwd=repo, + check=True, + ) + subprocess.run(["git", "checkout", "-qB", branch], cwd=repo, check=True) + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + ) + + assert excinfo.value.code == "non-descendant-head" + + +def test_acceptance_rejects_branch_change_and_unchanged_head(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + policy = RepositoryAcceptancePolicy(allowed_paths=("docs/",)) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance(policy) + assert excinfo.value.code == "head-unchanged" + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + subprocess.run(["git", "checkout", "-qb", "other"], cwd=repo, check=True) + _commit(repo, "docs/result.md", "committed\n") + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance(policy) + assert excinfo.value.code == "branch-changed" + + +def test_acceptance_evidence_bounds_changed_path_list(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + policy = RepositoryAcceptancePolicy( + allowed_paths=("docs/",), + max_evidence_paths=2, + ) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/a.md", "a\n", extra={"docs/b.md": "b\n", "docs/c.md": "c\n"}) + evidence = tx.validate_acceptance(policy).evidence() + + assert evidence["changed_path_count"] == 3 + assert len(evidence["changed_paths"]) == 2 + assert evidence["changed_paths_truncated"] is True + assert len(evidence["changed_paths_digest"]) == 64 + + +def test_acceptance_path_glob_does_not_cross_directory_boundaries(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/nested/result.md", "nested\n") + with pytest.raises(RepositoryAcceptanceError) as excinfo: + tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/*.md",)) + ) + assert excinfo.value.code == "path-not-granted" + + +def test_acceptance_double_star_path_glob_allows_nested_paths(tmp_path: Path) -> None: + repo = _make_repo(tmp_path) + + with RepositoryTransaction(repo, state_dir=tmp_path / "state") as tx: + _commit(repo, "docs/nested/result.md", "nested\n") + accepted = tx.validate_acceptance( + RepositoryAcceptancePolicy(allowed_paths=("docs/**/*.md",)) + ) + assert accepted.changed_paths == ("docs/nested/result.md",) + + +@pytest.mark.parametrize( + "pattern", + ["/absolute", "../escape", ".git/config", "docs\\windows"], +) +def test_acceptance_policy_rejects_unsafe_path_patterns(pattern: str) -> None: + with pytest.raises(ValueError, match="path pattern"): + RepositoryAcceptancePolicy(allowed_paths=(pattern,)) + + +def _commit( + repo: Path, + relative_path: str, + content: str, + *, + extra: dict[str, str] | None = None, +) -> None: + files = {relative_path: content, **(extra or {})} + for name, value in files.items(): + path = repo / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(value, encoding="utf-8") + subprocess.run(["git", "add", "."], cwd=repo, check=True) + subprocess.run( + [ + "git", + "-c", + "user.email=test@example.invalid", + "-c", + "user.name=test", + "commit", + "-qm", + "task result", + ], + cwd=repo, + check=True, + ) + + def _status(repo: Path) -> str: return subprocess.run( ["git", "status", "--porcelain=v2", "--untracked-files=all"], diff --git a/workplans/HARNESS-WP-0003-governed-runtime-integrity.md b/workplans/HARNESS-WP-0003-governed-runtime-integrity.md index e2720b1..87aca2b 100644 --- a/workplans/HARNESS-WP-0003-governed-runtime-integrity.md +++ b/workplans/HARNESS-WP-0003-governed-runtime-integrity.md @@ -159,9 +159,9 @@ and non-repository refusal. The primitive is intentionally not wired into `runner.py`, legacy approaches, or the claim loop while ADR-002 still awaits Activity Core, sand-boxer, and -llm-connect acknowledgements. Lease-loss cancellation, moved-`HEAD` acceptance, -timeout/signal integration, and result-close reconciliation remain outstanding; -T02 therefore remains `wait`. +llm-connect acknowledgements. Controlled moved-`HEAD` acceptance is prepared +under T03 below; lease-loss cancellation, timeout/signal integration, and +result-close reconciliation remain outstanding. T02 therefore remains `wait`. ## Verify accepted commits and reconcile metrics/reporting @@ -199,6 +199,28 @@ leaves the checkout in its declared state; metrics and required completion evidence survive a temporary Hub/API outage without duplicating the workload commit; and docs no longer claim checks that the code does not perform. +### Preparation — 2026-08-23 + +Extended the production-inert transaction core with an explicit +`RepositoryAcceptancePolicy` and read-only post-run validator. It now requires +same-branch descendant commits within declared bounds, repository-relative path +patterns, a clean index/worktree, unchanged local remote-tracking refs, and an +unchanged digest of protected Git config, hooks, and info metadata. Accepted +evidence contains bounded commit/path lists plus policy, path-set, baseline, and +post-state digests; it retains no prompt or provider output. + +Sixteen additional adversarial cases bring the focused transaction suite to 26 +tests. They cover a valid one-commit result, unrelated history, excess commits, +ungranted paths, glob boundary semantics, dirty post-state, branch movement, +unchanged `HEAD`, remote-ref movement, Git config/hook changes, and bounded path +evidence. The full suite passes 123 tests with the existing optional +`glas_harness` contract skip. + +No current `TaskSpec` or instance-manifest field supplies an authoritative path +grant, so the validator remains deliberately unwired. Defining that versioned +grant surface, metrics atomicity, and required close-evidence reconciliation +remain outstanding; T03 remains `wait`. + ## Remove tenant logic from the shared runtime ```task