From b17ce95ddb019f924a5fa9a9f2ffa0e6965a75e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 23:41:41 +0000 Subject: [PATCH 1/3] Bind push authorization to Git's reference transaction pre-receive consumed approvals and committed before Git moved the ref, so an update Git refused afterwards (another hook, a lost ref lock) still spent its approvals, and a claim release, takeover, or handoff in that gap let a push land on authority that no longer held. - pre-receive (#18's checks, unchanged) now records each validated ref update in ref_updates (migration 006) with its derived paths and governed operations, and consumes nothing. - A managed reference-transaction hook re-validates every recorded update at `prepared`, while Git holds the ref locks and can still abort, and consumes the approvals there; any failure aborts the update. It reuses the paths pre-receive derived, so operation hashes match what was approved even after an earlier ref of the same push made the commits known. `committed`/`aborted` record the outcome; an abort after `prepared` restores the consumed approvals unless a newer live instance exists. - Only unidentified transactions pass through as local maintenance; an identified one without a fresh validation, or with the store missing, fails closed. Symbolic refs such as HEAD are left to their target line. - pre-receive refuses pushes when the transaction hook is missing, modified, or not executable; hook install writes both hooks, inspects both before writing either, and requires Git 2.28. - doctor reconciles updates stuck in `prepared` from the ref itself and never guesses when the ref has moved or is still locked. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp --- README.md | 22 +- src/quorumgit/cli.py | 74 +- src/quorumgit/gate.py | 674 ++++++++++++++++--- src/quorumgit/migrations/006_ref_updates.sql | 41 ++ src/quorumgit/store.py | 1 + tests/test_approval_authority.py | 1 + tests/test_authorization_binding.py | 4 +- tests/test_transaction_boundary.py | 643 ++++++++++++++++++ 8 files changed, 1349 insertions(+), 111 deletions(-) create mode 100644 src/quorumgit/migrations/006_ref_updates.sql create mode 100644 tests/test_transaction_boundary.py diff --git a/README.md b/README.md index d4a87a8..d7f0bdb 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ QuorumGit prevents coding agents (or humans) working the same codebase from step agent-one ──┐ agent-two ──┼──► quorumgit CLI ──► local SQLite file (claims, leases, approvals, audit) reviewer ──┘ │ - └──► git pre-receive hook (push enforcement) + └──► git pre-receive + reference-transaction hooks (push enforcement) ``` ## The problem it solves @@ -43,7 +43,7 @@ Everything an agent does — claim, renew, checkpoint, hand off, release — wri ## Installation -Requirements: **Python 3.11–3.14**, **Git 2.17 or newer** (the first release with `git worktree remove`). There is no database to install, no server to run, and no extension to provision: the store is a single SQLite 3 database file under `QUORUMGIT_DATA_DIR`, accessed through Python's standard-library [`sqlite3`](https://docs.python.org/3/library/sqlite3.html) module. QuorumGit has no third-party runtime dependencies. The Python interpreter must be linked against SQLite 3.38 or newer (every supported CPython release from python.org, Homebrew, and current Linux distributions is); `quorumgit` refuses to open the store otherwise. +Requirements: **Python 3.11–3.14**, **Git 2.28 or newer** (the first release with the `reference-transaction` hook that hub push enforcement relies on; the local model alone needs only 2.17, for `git worktree remove`). There is no database to install, no server to run, and no extension to provision: the store is a single SQLite 3 database file under `QUORUMGIT_DATA_DIR`, accessed through Python's standard-library [`sqlite3`](https://docs.python.org/3/library/sqlite3.html) module. QuorumGit has no third-party runtime dependencies. The Python interpreter must be linked against SQLite 3.38 or newer (every supported CPython release from python.org, Homebrew, and current Linux distributions is); `quorumgit` refuses to open the store otherwise. > **Platform note.** Linux, macOS, and Windows are supported on every Python from 3.11 to 3.14. @@ -151,7 +151,7 @@ Where the registered repository sits determines how enforcement works. Pick one **Local model** — the registered repo is a working checkout on the same machine. Claims create isolated worktrees; agents commit directly in them. Coordination is entirely claim-based; no pushes, no hook needed. This is the default and the simplest setup — the quick start above is the local model. -**Hub model** — the registered repo is a **bare** hub that agents push to from their own clones. Enforcement moves to a pre-receive hook: +**Hub model** — the registered repo is a **bare** hub that agents push to from their own clones. Enforcement moves to a pair of Git hooks, `pre-receive` and `reference-transaction`: ```bash git clone --bare /path/to/source myproject.git @@ -174,6 +174,8 @@ Owning the branch is necessary but not sufficient: when the branch has a live cl The hook learns who is pushing from `QUORUMGIT_AGENT` in its own environment, which Git passes through only when the remote is a **local path** (as `origin` is above). Over SSH or HTTP the variable does not reach the hook, so every push is rejected as unidentified. The hook also opens the store under its own `QUORUMGIT_DATA_DIR`, so pushers must use the same data directory as the rest of the deployment. +**Authorization is bound to the ref update itself.** `pre-receive` checks the push and records each exact ref update it accepted — with the paths it derived and the approvals it needs — but spends nothing. Git's `reference-transaction` hook then runs at the `prepared` stage, after Git has locked the refs and while it can still abort, re-checks every rule against the store's current state using the paths `pre-receive` recorded, and only then consumes the approvals. If ownership changed after `pre-receive` (a release, takeover, or handoff), the update is aborted. If Git refuses the update after `pre-receive` (another hook, a lost ref lock), no approval is spent; if Git aborts after `prepared`, the consumed approvals are restored. The `committed`/`aborted` outcome of every governed update is recorded. `pre-receive` refuses pushes when the transaction hook is missing, modified, or not executable. A transaction that carries an agent identity must match a fresh `pre-receive` validation (and a store that has vanished fails it closed); only unidentified ref transactions — local maintenance in the hub — pass through. + Accepting a handoff in the hub model prints `worktree: (none — create manually)`: there is no managed worktree to transfer, so the successor fetches the branch into its own clone and continues from the reported commit. Checkpoints in the hub model take an explicit `--commit ` (any commit spec Git resolves — a full or abbreviated OID or a ref — recorded as the full OID). Continuation points are **verified, not trusted**: the commit must exist in the registered repository, and when the claimed branch exists, be reachable from it. A typo'd or fabricated OID is rejected. @@ -245,7 +247,7 @@ QUORUMGIT_AGENT=agent-one quorumgit approve request '{"type":"protected_ref_upda quorumgit approve vote 17 --agent lead # 3. The same push now lands. Pushing it again — or replaying the approval — is rejected: -# the approval was consumed atomically when the operation was accepted. +# the approval was consumed while Git held the ref lock, as the update committed. ``` Rules that hold no matter what: @@ -277,7 +279,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c |---|---| | `quorumgit init` | Start/provision the store (idempotent) | | `quorumgit status` | Store health, contract check, row counts | -| `quorumgit doctor [--repair]` | Detect and conservatively reconcile recorded managed-worktree drift | +| `quorumgit doctor [--repair]` | Detect and conservatively reconcile managed-worktree drift and stuck ref updates | | `quorumgit destroy --yes` | Delete the database file (managed worktrees under `QUORUMGIT_DATA_DIR/worktrees` are left in place) | | `quorumgit repo add [--protected-ref ]… [--protected-path ]…` | Register a repository | | `quorumgit agent add [--role worker\|reviewer\|operator]` | Register an agent identity (non-workers need an operator once one exists) | @@ -300,7 +302,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `quorumgit approve vote [--deny]` | Vote on one approval instance | | `quorumgit approve prepare --repo --ref [--new \| --delete] [-C ] [--request] [--json]` | Show every approval a push would need, with the hook's exact hashes; `--request` opens them | | `quorumgit approve hash ` | Compute an operation's hash | -| `quorumgit hook install --repo ` | Install the pre-receive hook (hub model) | +| `quorumgit hook install --repo ` | Install the pre-receive and reference-transaction hooks (hub model) | | `quorumgit audit [--entity ] [--entity-id ] [--limit ]` | Read the audit trail | `repo list`, `agent list`, and `task list [--repo ]` enumerate what's registered. Commands that act as an agent (`claim`, `renew`, `release`, `task done`, `checkpoint`, `handoff create/accept/decline/cancel`, `approve request/vote`, `approve prepare`, and the operator actions `agent add`, `agent role`, `repo policy`, `repo protect-path`, `repo allow-ref`) also take `--agent `, which overrides `QUORUMGIT_AGENT`. At least one `--scope` is required to claim. Exit codes: `0` success, `1` refused/violation/error, `2` usage error. @@ -312,7 +314,11 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c - `unexpectedly_present` — recorded as removed, but the directory exists again. `--repair` removes it. - `repository_mismatch`, `branch_mismatch`, `detached_head`, `unverifiable_checkout` — the checkout at the recorded path no longer matches the recorded repository and branch. These are never repaired automatically; inspect them by hand. -Repairs use ordinary non-forced `git worktree remove`, so dirty worktrees are reported instead of destroyed. `doctor` exits non-zero while any finding remains unresolved. +Repairs use ordinary non-forced `git worktree remove`, so dirty worktrees are reported instead of destroyed. + +`doctor` also reports governed ref updates stuck in `prepared` for more than five minutes — ones whose `committed`/`aborted` hook never recorded an outcome (a killed process, an unreachable store). It reads the outcome from the ref itself: at the update's new value (or gone, for a deletion) means Git committed; still at the old value means Git aborted, and `--repair` restores the approvals consumed for it. A ref that has since moved elsewhere, or is still locked by Git, is reported for manual inspection and never guessed at. + +`doctor` exits non-zero while any finding remains unresolved. ## Configuration @@ -337,7 +343,7 @@ QuorumGit v1 coordinates **cooperating agents**; the adversary is *accident, not - Agent identity is asserted (`QUORUMGIT_AGENT`), not cryptographically authenticated. Any local process can claim to be any agent. - The trust root is write access to the database and the filesystem. An actor with either can bypass governance. -- The pre-receive hook governs `git push` only. Direct ref manipulation inside a repository bypasses it. +- The hooks govern `git push` only. Direct ref manipulation inside a repository bypasses them. - Content is governed when it first reaches the hub. Scopes are checked only on claimed branches, so changes pushed to an unclaimed branch (subject to protected paths) and later merged into a claimed one are not re-attributed to the claim. - Approval hashes provide exact-payload binding and tamper-evidence, not approver authentication. - Roles separate duties between cooperating agents. Because identity is asserted, a process willing to impersonate an operator can still do so; role separation stops an agent from authorizing its own work by accident or by default, not a determined impersonator. diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index 9a59be7..db187ad 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -81,28 +81,43 @@ def cmd_destroy(args, cfg) -> int: def cmd_doctor(args, cfg) -> int: - """Inspect and optionally reconcile recorded managed-worktree drift.""" + """Inspect and optionally reconcile worktree drift and stuck ref updates.""" with store.session(cfg) as conn: findings = trees.doctor_worktrees(conn, repair=args.repair) + ref_findings = gate.doctor_ref_updates(conn, repair=args.repair) if args.repair: conn.commit() - if not findings: - print("worktrees: ok") - return 0 unresolved = False - for finding in findings: - state = "repaired" if finding.get("repaired") else "detected" + + def state_of(finding: dict) -> str: + nonlocal unresolved if finding.get("error"): - state = f"repair failed: {finding['error']}" - unresolved = True - elif not finding.get("repaired"): unresolved = True + prefix = "repair failed" if args.repair else "unresolvable" + return f"{prefix}: {finding['error']}" + if finding.get("repaired"): + return "repaired" + unresolved = True + return "detected" + + for finding in findings: print( f"worktree {finding['worktree_id']} claim {finding['claim_id']}: " - f"{finding['issue']} — {state} — {finding['path']}" + f"{finding['issue']} — {state_of(finding)} — {finding['path']}" + ) + for finding in ref_findings: + outcome = f" (git {finding['outcome']})" if finding.get("outcome") else "" + print( + f"ref update {finding['ref_update_id']} {finding['repository']} " + f"{finding['refname']}: {finding['issue']}{outcome} — {state_of(finding)}" ) - if args.repair and not unresolved: - print("worktrees: reconciled") + if not findings: + print("worktrees: ok") + if not ref_findings: + print("ref updates: ok") + if not unresolved: + if args.repair and (findings or ref_findings): + print("worktrees: reconciled") return 0 if not args.repair: print("re-run with `quorumgit doctor --repair` to reconcile safe cases.") @@ -579,8 +594,11 @@ def cmd_approve_hash(args, cfg) -> int: def cmd_hook_install(args, cfg) -> int: with store.session(cfg) as conn: path = gate.install_hook(conn, args.repo) + repo_path = registry.get_repository(conn, args.repo)["path"] conn.commit() print(f"pre-receive hook installed: {path}") + print("reference-transaction hook installed: " + f"{gate._effective_hook(repo_path, 'reference-transaction')}") return 0 @@ -589,6 +607,34 @@ def cmd_hook_pre_receive(args, cfg) -> int: return gate.run_pre_receive(conn, args.repo, sys.stdin) +def cmd_hook_reference_transaction(args, cfg) -> int: + if not store.database_path(cfg).exists(): + # Without a store, only unidentified local ref maintenance may pass. + # Every push is identified (pre-receive requires it), so a store that + # vanished after pre-receive must not wave its update through. + if args.state == "prepared" and cfg.agent: + print( + "[quorumgit] REJECTED: the store is missing; cannot re-validate " + f"this update by {cfg.agent}.", + file=sys.stderr, + ) + return 1 + return 0 + try: + conn = store.connect(cfg) + except store.StoreError as exc: + label = "REJECTED" if args.state == "prepared" else f"WARNING ({args.state})" + print(f"[quorumgit] {label}: {exc}", file=sys.stderr) + return 1 + try: + with conn: + return gate.run_reference_transaction( + conn, args.repo, args.state, sys.stdin + ) + finally: + conn.close() + + # -------------------------------------------------------------------- audit @@ -786,6 +832,10 @@ def add(name, fn, configure=None, parent=None): lambda sp: sp.add_argument("--repo", required=True), parent=hook) add("pre-receive", cmd_hook_pre_receive, lambda sp: sp.add_argument("--repo", required=True), parent=hook) + add("reference-transaction", cmd_hook_reference_transaction, lambda sp: ( + sp.add_argument("--repo", required=True), + sp.add_argument("state"), + ), parent=hook) add("audit", cmd_audit, lambda sp: ( sp.add_argument("--entity"), diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index c6ff86d..781f22e 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -14,7 +14,9 @@ from __future__ import annotations import os +import re import shlex +import sqlite3 import subprocess import sys import tempfile @@ -32,6 +34,7 @@ get_agent, get_repository, git_common_dir, + path_scoped_git_env, ) from .store import Connection, begin_immediate, json_dumps, json_loads from .work import ( @@ -570,7 +573,7 @@ def _requirement_message(operation: dict, refusal: str | None) -> str: return message -def check_ref_update( +def evaluate_ref_update( conn: Connection, repository: str, git_dir: str, @@ -578,8 +581,17 @@ def check_ref_update( oldrev: str, newrev: str, refname: str, -) -> None: - """Enforce governance for one ref update. Raises PushRejected.""" + paths: list[str] | None = None, +) -> tuple[dict, list[tuple[dict, dict]], list[str]]: + """Apply every push rule to one ref update without changing any state. + + Returns (repository, [(approval, operation)] the update needs, paths). + Raises PushRejected. Runs in pre-receive and again at the reference + transaction's `prepared` stage, against the state current while Git holds + the ref locks. The second run passes the paths pre-receive derived, so + the operations (and their hashes) are the ones that were approved even + when an earlier ref of the same push has since made its commits known. + """ repo = _verify_repository_binding(conn, repository, git_dir) refusal = ref_namespace_refusal(repo, refname) if refusal is not None: @@ -602,57 +614,88 @@ def check_ref_update( ) try: - paths = git_objects.changed_paths( - git_dir, oldrev, newrev, git_objects.ref_tips(git_dir) - ) + if paths is None: + paths = git_objects.changed_paths( + git_dir, oldrev, newrev, git_objects.ref_tips(git_dir) + ) operations = governed_operations( conn, repo, refname, oldrev, newrev, git_dir, paths ) except git_objects.GitObjectError as exc: raise PushRejected(f"Unable to inspect pushed objects: {exc}") from exc - if operations: - granted: list[tuple[dict, dict]] = [] - missing: list[str] = [] - for operation in operations: - approval = approved_instance(conn, operation, consumer=pusher) - if approval is None: - missing.append(_requirement_message( - operation, approval_refusal(conn, operation, consumer=pusher) - )) - else: - granted.append((approval, operation)) - if missing: - if len(operations) > 1: - missing.append( - "Run `quorumgit approve prepare` from your clone to list " - "and request every approval this push needs." - ) - raise PushRejected(" ".join(missing)) - assert pusher is not None - for approval, operation in granted: - consume_approval(conn, approval["id"], operation, agent=pusher) - audit.record( - conn, - "gate.protected_update_allowed", - "repository", - repo["id"], - agent=pusher, - detail=operation, + granted: list[tuple[dict, dict]] = [] + missing: list[str] = [] + for operation in operations: + approval = approved_instance(conn, operation, consumer=pusher) + if approval is None: + missing.append(_requirement_message( + operation, approval_refusal(conn, operation, consumer=pusher) + )) + else: + granted.append((approval, operation)) + if missing: + if len(operations) > 1: + missing.append( + "Run `quorumgit approve prepare` from your clone to list " + "and request every approval this push needs." ) - return + raise PushRejected(" ".join(missing)) + return repo, granted, paths + +def check_ref_update( + conn: Connection, + repository: str, + git_dir: str, + pusher: str, + oldrev: str, + newrev: str, + refname: str, +) -> None: + """pre-receive: validate one ref update and record it for its transaction. + + Nothing is consumed here. Git may still refuse the update after + pre-receive (another hook, a lost ref lock), so approvals are spent only + once Git has locked the ref, in the reference-transaction hook. + """ + repo, granted, paths = evaluate_ref_update( + conn, repository, git_dir, pusher, oldrev, newrev, refname + ) + operations = [operation for _, operation in granted] + row = conn.execute( + """ + INSERT INTO ref_updates ( + repository_id, refname, oldrev, newrev, pusher_agent_id, + paths, operations + ) + VALUES (?, ?, ?, ?, ?, ?, ?) + RETURNING id + """, + ( + repo["id"], + refname, + oldrev, + newrev, + get_agent(conn, pusher)["id"], + json_dumps(paths), + json_dumps(operations), + ), + ).fetchone() + assert row is not None audit.record( conn, - "gate.update_allowed", - "repository", - repo["id"], + "gate.update_validated", + "ref_update", + row[0], agent=pusher, detail={ "refname": refname, "oldrev": oldrev, "newrev": newrev, "paths": len(paths), + "operations": operations, + "approval_ids": [approval["id"] for approval, _ in granted], }, ) @@ -780,6 +823,9 @@ def run_pre_receive( check_ref_update(conn, repository, git_dir, pusher, *parts) if not saw_update: raise PushRejected("No ref updates supplied on stdin.") + # Without the transaction hook nothing would re-validate at update + # time or consume approvals, so a missing one must reject the push. + _require_reference_transaction_hook(conn, repository) except Exception as exc: # noqa: BLE001 — fail closed on anything conn.rollback() print(f"[quorumgit] REJECTED: {exc}", file=sys.stderr) @@ -789,10 +835,399 @@ def run_pre_receive( return 0 -def _effective_pre_receive_hook(repository_path: str | Path) -> Path: +# ------------------------------------------------------ reference transaction + +TRANSACTION_STATES = ("prepared", "committed", "aborted") +# pre-receive and the reference transaction run seconds apart. A validation +# older than this is stale (Git refused the update after pre-receive) and is +# never matched against a later transaction. +VALIDATION_TTL_SECONDS = 300 +# A prepared update normally resolves within milliseconds, when Git commits or +# aborts the locked transaction. One older than this lost its outcome hook +# (killed process, unreachable store) and doctor reconciles it from the ref. +PREPARED_STUCK_AFTER_SECONDS = VALIDATION_TTL_SECONDS + + +def _parse_updates(stdin_lines: Iterable[str]) -> list[tuple[str, str, str]]: + updates = [] + for line in stdin_lines: + parts = line.split() + if not parts: + continue + if len(parts) != 3: + raise PushRejected(f"Malformed reference-transaction input: {line!r}") + updates.append((parts[0], parts[1], parts[2])) + return updates + + +def _find_update( + conn: Connection, + repository_id: int, + status: str, + oldrev: str, + newrev: str, + refname: str, +) -> dict | None: + """The newest recorded update matching one transaction line. + + Git reports the zero OID as the old value when a transaction does not + check it, so a zero old value matches any recorded one. + """ + row = conn.execute( + """ + SELECT u.id, u.oldrev, a.name, u.paths, u.approval_ids + FROM ref_updates u JOIN agents a ON a.id = u.pusher_agent_id + WHERE u.repository_id = ? AND u.status = ? AND u.refname = ? + AND u.newrev = ? AND (u.oldrev = ? OR ?) + AND u.created_at >= unixepoch() - ? + ORDER BY u.id DESC + LIMIT 1 + """, + ( + repository_id, + status, + refname, + newrev, + oldrev, + 1 if is_zero(oldrev) else 0, + VALIDATION_TTL_SECONDS, + ), + ).fetchone() + if row is None: + return None + return { + "id": row[0], + "oldrev": row[1], + "pusher": row[2], + "paths": json_loads(row[3], []), + "approval_ids": json_loads(row[4], []), + } + + +def _prepare_updates( + conn: Connection, + repository: str, + git_dir: str, + pusher: str | None, + updates: list[tuple[str, str, str]], +) -> None: + """Re-validate and authorize validated updates while Git holds the locks. + + Only an unidentified transaction with no matching validation is local ref + maintenance. An identified one (every push carries QUORUMGIT_AGENT, which + pre-receive requires) must match a fresh validation, or it is rejected: + an expired or missing record never downgrades a push to pass-through. + """ + repo = get_repository(conn, repository) + governed = [] + # Classify without the write reservation, so pass-through maintenance + # never waits on (or deadlocks with) a command holding the store. + for oldrev, newrev, refname in updates: + if not refname.startswith("refs/"): + # A symbolic ref such as HEAD is listed alongside the ref it + # points to; that ref appears as its own line and is governed + # there. Pushes only ever name refs under refs/. + continue + if _find_update(conn, repo["id"], "validated", oldrev, newrev, refname): + governed.append((oldrev, newrev, refname)) + continue + if pusher is not None: + raise PushRejected( + f"Update of {refname} to {newrev} by {pusher} has no current " + "pre-receive validation; push it again." + ) + stray = conn.execute( + "SELECT id FROM ref_updates WHERE repository_id = ? " + "AND refname = ? AND status = 'validated' " + "AND created_at >= unixepoch() - ? LIMIT 1", + (repo["id"], refname, VALIDATION_TTL_SECONDS), + ).fetchone() + if stray is not None: + raise PushRejected( + f"Update of {refname} to {newrev} does not match the update " + "pre-receive validated." + ) + if not governed: + return + begin_immediate(conn) + for oldrev, newrev, refname in governed: + found = _find_update(conn, repo["id"], "validated", oldrev, newrev, refname) + if found is None: + raise PushRejected(f"Update of {refname} changed while being prepared.") + if pusher != found["pusher"]: + raise PushRejected( + f"Update of {refname} was validated for {found['pusher']}, " + f"not {pusher or 'an unidentified pusher'}." + ) + _, granted, _ = evaluate_ref_update( + conn, + repository, + git_dir, + pusher, + found["oldrev"], + newrev, + refname, + paths=found["paths"], + ) + assert pusher is not None + for approval, operation in granted: + consume_approval(conn, approval["id"], operation, agent=pusher) + approval_ids = [approval["id"] for approval, _ in granted] + cur = conn.execute( + "UPDATE ref_updates SET status = 'prepared', approval_ids = ? " + "WHERE id = ? AND status = 'validated'", + (json_dumps(approval_ids), found["id"]), + ) + if cur.rowcount != 1: + raise PushRejected(f"Update of {refname} changed while being prepared.") + audit.record( + conn, + "gate.update_prepared", + "ref_update", + found["id"], + agent=pusher, + detail={ + "refname": refname, + "oldrev": found["oldrev"], + "newrev": newrev, + "approval_ids": approval_ids, + }, + ) + + +def _restore_approval(conn: Connection, update_id: int, approval_id: int) -> None: + """Return an approval consumed for an update Git then aborted.""" + try: + cur = conn.execute( + "UPDATE approvals SET status = 'approved', consumed_at = NULL, " + "consumed_by_agent_id = NULL WHERE id = ? AND status = 'consumed'", + (approval_id,), + ) + except sqlite3.IntegrityError as exc: + # A newer live instance for the same operation now exists; the old + # one stays consumed rather than creating two live approvals. + audit.record( + conn, + "approval.restore_skipped", + "approval", + approval_id, + detail={"ref_update_id": update_id, "reason": str(exc)}, + ) + return + if cur.rowcount == 1: + audit.record( + conn, + "approval.restored", + "approval", + approval_id, + detail={"ref_update_id": update_id}, + ) + + +def _resolve_updates( + conn: Connection, + repository: str, + state: str, + updates: list[tuple[str, str, str]], +) -> None: + begin_immediate(conn) + repo = get_repository(conn, repository) + statuses = ("prepared",) if state == "committed" else ("prepared", "validated") + for oldrev, newrev, refname in updates: + for status in statuses: + found = _find_update(conn, repo["id"], status, oldrev, newrev, refname) + if found is None: + continue + conn.execute( + "UPDATE ref_updates SET status = ?, resolved_at = unixepoch() " + "WHERE id = ? AND status = ?", + (state, found["id"], status), + ) + if state == "aborted": + for approval_id in found["approval_ids"]: + _restore_approval(conn, found["id"], approval_id) + audit.record( + conn, + f"gate.update_{state}", + "ref_update", + found["id"], + agent=found["pusher"], + detail={"refname": refname, "newrev": newrev}, + ) + break + + +def run_reference_transaction( + conn: Connection, repository: str, state: str, stdin_lines: Iterable[str] +) -> int: + """Hook entry point for Git's reference-transaction hook. + + `prepared` is the authoritative gate: its failure makes Git abort the + transaction, so it fails closed. Git ignores the exit status of + `committed` and `aborted`; they only record the outcome. + """ + if state not in TRANSACTION_STATES: + return 0 + git_dir = os.environ.get("GIT_DIR") or "." + pusher = os.environ.get("QUORUMGIT_AGENT") or None + try: + updates = _parse_updates(stdin_lines) + if state == "prepared": + _prepare_updates(conn, repository, git_dir, pusher, updates) + else: + _resolve_updates(conn, repository, state, updates) + except Exception as exc: # noqa: BLE001 — fail closed on anything + conn.rollback() + label = "REJECTED" if state == "prepared" else f"WARNING ({state})" + print(f"[quorumgit] {label}: {exc}", file=sys.stderr) + return 1 + conn.commit() + return 0 + + +# ------------------------------------------------- doctor: stuck ref updates + + +def _current_ref(repository_path: str, refname: str) -> str | None: + """The ref's object ID in the repository, or None when it does not exist.""" + result = subprocess.run( + ["git", "-C", repository_path, "rev-parse", "--verify", "--quiet", refname], + capture_output=True, + text=True, + env=path_scoped_git_env(), + check=False, + ) + if result.returncode == 0: + return result.stdout.strip().lower() + if result.returncode == 1: + return None + detail = (result.stderr or result.stdout).strip() + raise GateError(f"Unable to read {refname} in {repository_path}: {detail}") + + +def _ref_is_locked(repository_path: str, refname: str) -> bool: + """True while Git holds the files-backend lock for the ref.""" + result = subprocess.run( + ["git", "-C", repository_path, "rev-parse", "--git-path", f"{refname}.lock"], + capture_output=True, + text=True, + env=path_scoped_git_env(), + check=False, + ) + if result.returncode != 0: + return False + lock = Path(result.stdout.strip()) + if not lock.is_absolute(): + lock = Path(repository_path) / lock + return lock.exists() + + +def doctor_ref_updates(conn: Connection, repair: bool = False) -> list[dict]: + """Report and conservatively reconcile ref updates stuck in `prepared`. + + The outcome is read from the ref itself: at the new value (or gone, for + a deletion) means Git committed; still at the old value (or absent, for a + creation) means Git aborted, so the consumed approvals are restored. A ref + that has since moved elsewhere, or is still locked, is reported for manual + inspection and never guessed at. + """ + if repair: + begin_immediate(conn) + rows = conn.execute( + """ + SELECT u.id, u.refname, u.oldrev, u.newrev, u.approval_ids, r.name, r.path + FROM ref_updates u JOIN repositories r ON r.id = u.repository_id + WHERE u.status = 'prepared' AND u.created_at < unixepoch() - ? + ORDER BY u.id + """, + (PREPARED_STUCK_AFTER_SECONDS,), + ).fetchall() + findings: list[dict] = [] + for update_id, refname, oldrev, newrev, approval_ids, repo_name, repo_path in rows: + finding: dict[str, Any] = { + "ref_update_id": update_id, + "repository": repo_name, + "refname": refname, + "issue": "stuck_prepared", + "outcome": None, + "repaired": False, + } + findings.append(finding) + try: + if _ref_is_locked(repo_path, refname): + finding["error"] = "ref is still locked by Git; retry once it is released" + continue + current = _current_ref(repo_path, refname) + except GateError as exc: + finding["error"] = str(exc) + continue + new_value = None if is_zero(newrev) else newrev + old_value = None if is_zero(oldrev) else oldrev + if current == new_value: + finding["outcome"] = "committed" + elif current == old_value: + finding["outcome"] = "aborted" + else: + finding["error"] = ( + f"{refname} has since moved to {current or 'deletion'}; the outcome " + "cannot be proven from the ref. Inspect it manually." + ) + continue + if not repair: + continue + cur = conn.execute( + "UPDATE ref_updates SET status = ?, resolved_at = unixepoch() " + "WHERE id = ? AND status = 'prepared'", + (finding["outcome"], update_id), + ) + if cur.rowcount != 1: + finding["error"] = "ref update changed while being reconciled" + continue + if finding["outcome"] == "aborted": + for approval_id in json_loads(approval_ids, []): + _restore_approval(conn, update_id, approval_id) + audit.record( + conn, + f"gate.update_reconciled_{finding['outcome']}", + "ref_update", + update_id, + detail={"refname": refname, "current": current}, + ) + finding["repaired"] = True + return findings + + +# ------------------------------------------------------------ hook install + +HOOK_NAMES = ("pre-receive", "reference-transaction") +REFERENCE_TRANSACTION_MARKER = "# quorumgit-managed-reference-transaction v1" +# The reference-transaction hook (with its prepared/committed/aborted states). +MIN_GIT_VERSION = (2, 28) + + +def _git_version() -> tuple[int, ...]: + result = subprocess.run( + ["git", "version"], capture_output=True, text=True, check=False + ) + match = re.search(r"(\d+)\.(\d+)", result.stdout) + if result.returncode != 0 or match is None: + raise GateError("Unable to determine the Git version.") + return (int(match.group(1)), int(match.group(2))) + + +def _require_git_version() -> None: + version = _git_version() + if version < MIN_GIT_VERSION: + raise GateError( + f"Git {'.'.join(map(str, version))} lacks the reference-transaction " + f"hook; QuorumGit needs Git {'.'.join(map(str, MIN_GIT_VERSION))} or newer." + ) + + +def _effective_hook(repository_path: str | Path, name: str) -> Path: repo_path = Path(repository_path).resolve() result = subprocess.run( - ["git", "-C", str(repo_path), "rev-parse", "--git-path", "hooks/pre-receive"], + ["git", "-C", str(repo_path), "rev-parse", "--git-path", f"hooks/{name}"], capture_output=True, text=True, check=False, @@ -803,13 +1238,17 @@ def _effective_pre_receive_hook(repository_path: str | Path) -> Path: raise GateError(f"Unable to resolve Git hook path for {repo_path}{suffix}") raw = result.stdout.strip() if not raw: - raise GateError(f"Git returned no pre-receive hook path for {repo_path}") + raise GateError(f"Git returned no {name} hook path for {repo_path}") hook_path = Path(raw) if not hook_path.is_absolute(): hook_path = repo_path / hook_path return hook_path.resolve() +def _effective_pre_receive_hook(repository_path: str | Path) -> Path: + return _effective_hook(repository_path, "pre-receive") + + def _hook_script(repository: str, executable: str | None = None) -> str: python = executable or sys.executable return ( @@ -820,6 +1259,22 @@ def _hook_script(repository: str, executable: str | None = None) -> str: ) +def _reference_transaction_script( + repository: str, executable: str | None = None +) -> str: + python = executable or sys.executable + return ( + "#!/bin/sh\n" + f"{REFERENCE_TRANSACTION_MARKER}\n" + 'case "$1" in\n' + "prepared|committed|aborted) ;;\n" + "*) cat >/dev/null; exit 0 ;;\n" + "esac\n" + f"exec {shlex.quote(python)} -m quorumgit hook reference-transaction " + f'--repo {shlex.quote(repository)} "$1"\n' + ) + + def _legacy_hook_script(repository: str) -> str: """Exact pre-PR6 hook shape, recognized only for safe in-place upgrade.""" return ( @@ -829,6 +1284,36 @@ def _legacy_hook_script(repository: str) -> str: ) +def _hook_plan(repository: str, name: str) -> tuple[str, str, tuple[str, ...]]: + """(expected content, ownership marker, upgradeable legacy contents).""" + if name == "pre-receive": + return _hook_script(repository), HOOK_MARKER, (_legacy_hook_script(repository),) + return _reference_transaction_script(repository), REFERENCE_TRANSACTION_MARKER, () + + +def _require_reference_transaction_hook(conn: Connection, repository: str) -> None: + _require_git_version() + repo = get_repository(conn, repository) + hook_path = _effective_hook(repo["path"], "reference-transaction") + expected, _, _ = _hook_plan(repository, "reference-transaction") + try: + installed = hook_path.read_text(encoding="utf-8") + except (OSError, UnicodeError): + installed = None + if installed != expected: + raise PushRejected( + f"The QuorumGit reference-transaction hook is missing or modified at " + f"{hook_path}; run `quorumgit hook install --repo {repository}`." + ) + # Git silently ignores a hook that is not an executable regular file. + if not hook_path.is_file() or not os.access(hook_path, os.X_OK): + raise PushRejected( + f"The QuorumGit reference-transaction hook at {hook_path} is not " + f"executable, so Git would skip it; run `quorumgit hook install " + f"--repo {repository}`." + ) + + def _write_hook_atomically(hook_path: Path, content: str) -> None: hook_path.parent.mkdir(parents=True, exist_ok=True) fd, tmp_name = tempfile.mkstemp( @@ -851,63 +1336,72 @@ def _write_hook_atomically(hook_path: Path, content: str) -> None: pass +def _inspect_hook( + hook_path: Path, expected: str, marker: str, legacy: tuple[str, ...] +) -> str: + """Decide what installing over `hook_path` means; refuse foreign content.""" + if not hook_path.exists(): + return "installed" + try: + existing = hook_path.read_text(encoding="utf-8") + except (OSError, UnicodeError) as exc: + raise GateError(f"Cannot inspect existing hook {hook_path}: {exc}") from exc + if existing == expected: + return "verified" + if existing in legacy: + return "upgraded" + if marker in existing.splitlines()[:3]: + raise GateError( + f"Existing QuorumGit-managed hook differs at {hook_path}; " + "refusing to overwrite it." + ) + raise GateError( + f"Existing hook at {hook_path} is not owned by QuorumGit; refusing to " + "overwrite or silently chain it." + ) + + def install_hook(conn: Connection, repository: str) -> Path: - """Safely install QuorumGit at Git's effective pre-receive hook path. + """Safely install QuorumGit's pre-receive and reference-transaction hooks. - Existing unrelated hooks are never overwritten or silently chained. An + Both go at Git's effective hook paths. Existing unrelated hooks are never + overwritten or silently chained, and every existing hook is inspected + before anything is written, so a refusal leaves both paths untouched. An exact current QuorumGit hook is idempotent; an exact legacy QuorumGit hook - for the same repository is upgraded in place. Any other existing content - is refused so operators must make coexistence explicit. + for the same repository is upgraded in place. Returns the pre-receive path. """ begin_immediate(conn) + _require_git_version() repo = get_repository(conn, repository) common_dir = assert_repository_identity_unique(conn, repo) - hook_path = _effective_pre_receive_hook(repo["path"]) - expected = _hook_script(repository) - action = "installed" - if hook_path.exists(): - try: - existing = hook_path.read_text(encoding="utf-8") - except (OSError, UnicodeError) as exc: - raise GateError( - f"Cannot inspect existing pre-receive hook {hook_path}: {exc}" - ) from exc - if existing == expected: + plans = [] + for name in HOOK_NAMES: + expected, marker, legacy = _hook_plan(repository, name) + hook_path = _effective_hook(repo["path"], name) + plans.append( + (name, hook_path, expected, _inspect_hook(hook_path, expected, marker, legacy)) + ) + + for name, hook_path, expected, action in plans: + if action == "verified": hook_path.chmod(0o755) - action = "verified" - elif existing == _legacy_hook_script(repository): - _write_hook_atomically(hook_path, expected) - action = "upgraded" - elif HOOK_MARKER in existing.splitlines()[:3]: - raise GateError( - f"Existing QuorumGit-managed pre-receive hook differs at " - f"{hook_path}; refusing to overwrite it." - ) else: - raise GateError( - f"Existing pre-receive hook at {hook_path} is not owned by " - "QuorumGit; refusing to overwrite or silently chain it." - ) - else: - _write_hook_atomically(hook_path, expected) - - effective = _effective_pre_receive_hook(repo["path"]) - if effective != hook_path or not hook_path.exists(): - raise GateError( - f"Installed hook is not Git's effective pre-receive hook: {hook_path}" + _write_hook_atomically(hook_path, expected) + effective = _effective_hook(repo["path"], name) + if effective != hook_path or not hook_path.exists(): + raise GateError(f"Installed hook is not Git's effective {name} hook: {hook_path}") + if hook_path.read_text(encoding="utf-8") != expected: + raise GateError(f"Installed {name} hook failed verification: {hook_path}") + audit.record( + conn, + f"hook.{action}", + "repository", + repo["id"], + detail={ + "hook": name, + "path": str(hook_path), + "git_common_dir": str(common_dir), + }, ) - if hook_path.read_text(encoding="utf-8") != expected: - raise GateError(f"Installed pre-receive hook failed verification: {hook_path}") - - audit.record( - conn, - f"hook.{action}", - "repository", - repo["id"], - detail={ - "path": str(hook_path), - "git_common_dir": str(common_dir), - }, - ) - return hook_path + return plans[0][1] diff --git a/src/quorumgit/migrations/006_ref_updates.sql b/src/quorumgit/migrations/006_ref_updates.sql new file mode 100644 index 0000000..baecfc6 --- /dev/null +++ b/src/quorumgit/migrations/006_ref_updates.sql @@ -0,0 +1,41 @@ +-- Bind a push's pre-receive validation to Git's own reference transaction. +-- pre-receive records each exact ref update it validated, with the paths it +-- derived and the governed operations it found. The reference-transaction +-- hook re-validates it and consumes the approvals while Git holds the ref +-- locks ("prepared"), then records whether Git committed or aborted the +-- update. Approvals are therefore never spent on an update that did not +-- happen, and ownership is re-checked at the last abortable moment. + +CREATE TABLE ref_updates ( + id INTEGER PRIMARY KEY, + repository_id INTEGER NOT NULL REFERENCES repositories(id), + refname TEXT NOT NULL CHECK (refname <> ''), + oldrev TEXT NOT NULL CHECK ( + length(oldrev) BETWEEN 40 AND 64 + AND oldrev NOT GLOB '*[^0-9a-f]*' + ), + newrev TEXT NOT NULL CHECK ( + length(newrev) BETWEEN 40 AND 64 + AND newrev NOT GLOB '*[^0-9a-f]*' + ), + pusher_agent_id INTEGER NOT NULL REFERENCES agents(id), + paths TEXT NOT NULL DEFAULT '[]' CHECK ( + json_valid(paths) AND json_type(paths) = 'array' + ), + operations TEXT NOT NULL DEFAULT '[]' CHECK ( + json_valid(operations) AND json_type(operations) = 'array' + ), + approval_ids TEXT NOT NULL DEFAULT '[]' CHECK ( + json_valid(approval_ids) AND json_type(approval_ids) = 'array' + ), + status TEXT NOT NULL DEFAULT 'validated' + CHECK (status IN ('validated', 'prepared', 'committed', 'aborted')), + created_at INTEGER NOT NULL DEFAULT (unixepoch()), + resolved_at INTEGER, + CHECK (status IN ('validated', 'prepared') OR resolved_at IS NOT NULL) +); + +-- quorumgit-statement +CREATE INDEX ref_updates_in_flight + ON ref_updates(repository_id, refname, newrev) + WHERE status IN ('validated', 'prepared'); diff --git a/src/quorumgit/store.py b/src/quorumgit/store.py index 7b05c28..6773a02 100644 --- a/src/quorumgit/store.py +++ b/src/quorumgit/store.py @@ -54,6 +54,7 @@ "repository_approval_roles", "protected_paths", "allowed_ref_namespaces", + "ref_updates", ) # Engine-level governance rules. A store missing any of these would still diff --git a/tests/test_approval_authority.py b/tests/test_approval_authority.py index f60fa7b..a5d49dc 100644 --- a/tests/test_approval_authority.py +++ b/tests/test_approval_authority.py @@ -362,6 +362,7 @@ def test_upgrade_leaves_no_agent_with_authority(tmp_path): assert store.migrate(local) == [ "004_approval_authority.sql", "005_content_governance.sql", + "006_ref_updates.sql", ] upgraded = store.connect(local) try: diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index 5f5523b..10ede0e 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -40,6 +40,7 @@ def _apply_001(cfg: Config): "002_approval_identities.sql", "004_approval_authority.sql", "005_content_governance.sql", + "006_ref_updates.sql", ] @@ -207,7 +208,8 @@ def test_contract_rejects_001_only_store_until_all_migrations_apply(tmp_path): message = ( "Missing required migrations: " "['002_approval_identities.sql', '004_approval_authority.sql', " - "'005_content_governance.sql']" + "'005_content_governance.sql', " + "'006_ref_updates.sql']" ) with pytest.raises(store.ContractViolation, match=re.escape(message)): store.verify_contract(cfg) diff --git a/tests/test_transaction_boundary.py b/tests/test_transaction_boundary.py new file mode 100644 index 0000000..9afe192 --- /dev/null +++ b/tests/test_transaction_boundary.py @@ -0,0 +1,643 @@ +"""The push authorization boundary. + +pre-receive validates and records each ref update but spends nothing. The +reference-transaction hook re-validates it and consumes any approval at the +`prepared` stage, while Git holds the ref locks and can still abort, then +records whether Git committed or aborted. doctor reconciles updates whose +outcome was never recorded. +""" + +from __future__ import annotations + +import os +import shlex +import subprocess +import sys +from pathlib import Path + +import pytest + +from quorumgit import gate, registry, work +from tests.conftest import approve +from tests.test_gate import _commit, _push, _setup + + +def _rev(git_args: list[str], ref: str) -> str: + return subprocess.run( + ["git", *git_args, "rev-parse", ref], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +def _approve_main(conn, repo_name: str, hub: Path, clone: Path) -> tuple[dict, dict]: + """Approve the clone's pending update of main: requested, then an operator votes.""" + op = { + "type": "protected_ref_update", + "repository": repo_name, + "refname": "refs/heads/main", + "oldrev": _rev(["--git-dir", str(hub)], "refs/heads/main"), + "newrev": _rev(["-C", str(clone)], "HEAD"), + } + approval = approve(conn, op, requested_by="requester") + conn.commit() + return op, approval + + +def _install_update_hook(hub: Path, python_body: str) -> Path: + """An extra `update` hook: runs after pre-receive, before the ref moves.""" + script = hub / "hooks" / "extra_update.py" + script.write_text(python_body, encoding="utf-8") + hook = hub / "hooks" / "update" + hook.write_text( + "#!/bin/sh\n" + f"exec {shlex.quote(sys.executable)} {shlex.quote(str(script))} \"$@\"\n", + encoding="utf-8", + ) + hook.chmod(0o755) + return hook + + +def _ref_update_statuses(conn, refname: str, newrev: str) -> list[str]: + conn.commit() + return [ + row[0] + for row in conn.execute( + "SELECT status FROM ref_updates WHERE refname = ? AND newrev = ? " + "ORDER BY id", + (refname, newrev), + ).fetchall() + ] + + +# ----------------------------------------------------- transaction boundary + + +def test_approval_survives_an_update_git_refuses_after_pre_receive( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/approved.py") + op, approval = _approve_main(conn, repo_name, hub, clone) + + refusing = _install_update_hook(hub, "import sys\nsys.exit(1)\n") + refused = _push(clone, a, "main", cfg=cfg) + assert refused.returncode != 0 + assert _rev(["--git-dir", str(hub)], "refs/heads/main") == op["oldrev"] + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == ["validated"] + + refusing.unlink() + accepted = _push(clone, a, "main", cfg=cfg) + assert accepted.returncode == 0, accepted.stderr + assert _rev(["--git-dir", str(hub)], "refs/heads/main") == op["newrev"] + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == [ + "validated", + "committed", + ] + + +def test_ownership_change_after_pre_receive_aborts_the_update( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, hub, clone, a, b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "contested") + claim, _, _ = work.claim_task(conn, task, a, branch="feat/race", scope_globs=["src/**"]) + conn.commit() + _commit(clone, "src/first.py", branch="feat/race") + first = _push(clone, a, "feat/race", cfg=cfg) + assert first.returncode == 0, first.stderr + before = _rev(["--git-dir", str(hub)], "refs/heads/feat/race") + + # Between pre-receive and the ref update, a's claim is released and b + # claims the branch. pre-receive saw a as owner; the update must not land. + _install_update_hook( + hub, + "from quorumgit import config, store, work\n" + "cfg = config.load()\n" + "conn = store.connect(cfg)\n" + f"work.release_claim(conn, {claim}, agent={a!r})\n" + f"work.claim_task(conn, {task}, {b!r}, branch='feat/race', scope_globs=['src/**'])\n" + "conn.commit()\n" + "conn.close()\n", + ) + _commit(clone, "src/second.py", branch="feat/race") + raced = _push(clone, a, "feat/race", cfg=cfg) + assert raced.returncode != 0 + assert _rev(["--git-dir", str(hub)], "refs/heads/feat/race") == before + newrev = _rev(["-C", str(clone)], "HEAD") + assert _ref_update_statuses(conn, "refs/heads/feat/race", newrev) == ["aborted"] + rejected = conn.execute( + "SELECT count(*) FROM audit_events WHERE event_type = 'gate.update_prepared' " + "AND json_extract(detail, '$.newrev') = ?", + (newrev,), + ).fetchone() + assert rejected is not None and rejected[0] == 0 + + +def test_aborted_transaction_restores_the_consumed_approval( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/restore.py") + # Make the new commit's objects available in the hub without moving main. + staged = _push(clone, a, "HEAD:refs/heads/staging", cfg=cfg) + assert staged.returncode == 0, staged.stderr + op, approval = _approve_main(conn, repo_name, hub, clone) + + # receive-pack runs hooks from inside the repository with GIT_DIR=".". + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + line = [f"{op['oldrev']} {op['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + assert gate.run_reference_transaction(conn, repo_name, "aborted", line) == 0 + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == ["aborted"] + restored = conn.execute( + "SELECT count(*) FROM audit_events WHERE event_type = 'approval.restored' " + "AND entity_id = ?", + (approval["id"],), + ).fetchone() + assert restored is not None and restored[0] == 1 + + +def test_abort_does_not_restore_over_a_newer_live_approval( + committed_conn, tmp_path, cfg, monkeypatch +): + """Restoring must never leave two live approvals for one operation.""" + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/conflict.py") + staged = _push(clone, a, "HEAD:refs/heads/staging", cfg=cfg) + assert staged.returncode == 0, staged.stderr + op, first = _approve_main(conn, repo_name, hub, clone) + + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + line = [f"{op['oldrev']} {op['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + assert gate.get_approval_by_id(conn, first["id"])["status"] == "consumed" + + # While the transaction is in flight the same operation is requested + # again; the consumed instance is not live, so this opens a new one. + second = gate.request_approval(conn, op, requested_by="requester") + conn.commit() + assert second["id"] != first["id"] and second["status"] == "pending" + + assert gate.run_reference_transaction(conn, repo_name, "aborted", line) == 0 + assert gate.get_approval_by_id(conn, first["id"])["status"] == "consumed" + assert gate.get_approval_by_id(conn, second["id"])["status"] == "pending" + live = conn.execute( + "SELECT count(*) FROM approvals WHERE operation_hash = ? " + "AND status IN ('pending', 'approved')", + (gate.operation_hash(op),), + ).fetchone() + assert live is not None and live[0] == 1 + assert _ref_update_statuses(conn, "refs/heads/main", op["newrev"]) == ["aborted"] + events = { + row[0] + for row in conn.execute( + "SELECT event_type FROM audit_events WHERE entity = 'approval' " + "AND entity_id = ?", + (first["id"],), + ).fetchall() + } + assert "approval.restore_skipped" in events + assert "approval.restored" not in events + + +def test_prepared_rejects_an_update_pre_receive_did_not_validate( + committed_conn, tmp_path, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "docs/one.md", branch="feat/swap") + _commit(clone, "docs/two.md", branch="feat/swap") + validated = _rev(["-C", str(clone)], "HEAD") + swapped = _rev(["-C", str(clone)], "HEAD~1") + + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + zero = "0" * 40 + conn.execute( + "INSERT INTO ref_updates (repository_id, refname, oldrev, newrev, " + "pusher_agent_id) VALUES (?, 'refs/heads/feat/swap', ?, ?, ?)", + ( + registry.get_repository(conn, repo_name)["id"], + zero, + validated, + registry.get_agent(conn, a)["id"], + ), + ) + conn.commit() + line = [f"{zero} {swapped} refs/heads/feat/swap\n"] + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 1 + + +def test_push_requires_the_reference_transaction_hook(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + (hub / "hooks" / "reference-transaction").unlink() + _commit(clone, "docs/free.md", branch="feat/nohook") + rejected = _push(clone, a, "feat/nohook", cfg=cfg) + assert rejected.returncode != 0 + assert "reference-transaction hook is missing" in rejected.stderr + + +def test_local_ref_maintenance_in_the_hub_passes_through( + committed_conn, tmp_path, cfg +): + _repo_name, hub, _clone, _a, _b = _setup(committed_conn, tmp_path) + head = _rev(["--git-dir", str(hub)], "refs/heads/main") + for data_dir in (cfg.data_dir, tmp_path / "no-store-here"): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(data_dir)} + env.pop("QUORUMGIT_AGENT", None) + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + result = subprocess.run( + ["git", "--git-dir", str(hub), "update-ref", + f"refs/heads/local-{data_dir.name}", head], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode == 0, result.stderr + + +def test_install_refuses_foreign_reference_transaction_hook_before_writing( + conn, tmp_path +): + from tests.test_hook_integrity import _hub_with_clone + + hub, _clone = _hub_with_clone(tmp_path, "foreign-rt") + repo_name = f"foreign-rt-{tmp_path.name[-8:]}" + registry.add_repository(conn, repo_name, hub) + foreign = hub / "hooks" / "reference-transaction" + foreign.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + foreign.chmod(0o755) + + with pytest.raises(gate.GateError, match="not owned by QuorumGit"): + gate.install_hook(conn, repo_name) + assert not (hub / "hooks" / "pre-receive").exists() + assert foreign.read_text(encoding="utf-8") == "#!/bin/sh\nexit 0\n" + + +def test_install_writes_both_hooks(conn, tmp_path): + from tests.test_hook_integrity import _hub_with_clone + + hub, _clone = _hub_with_clone(tmp_path, "both-hooks") + repo_name = f"both-{tmp_path.name[-8:]}" + registry.add_repository(conn, repo_name, hub) + gate.install_hook(conn, repo_name) + rt = (hub / "hooks" / "reference-transaction").read_text(encoding="utf-8") + assert rt == gate._reference_transaction_script(repo_name) + assert gate.REFERENCE_TRANSACTION_MARKER in rt + + +# ----------------------------------------------------- doctor: stuck updates + + +def _stuck_prepared_update(conn, tmp_path, cfg, monkeypatch): + """A protected update that reached `prepared` but never heard its outcome.""" + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "src/stuck.py") + staged = _push(clone, a, "HEAD:refs/heads/staging", cfg=cfg) + assert staged.returncode == 0, staged.stderr + op, approval = _approve_main(conn, repo_name, hub, clone) + + with monkeypatch.context() as scoped: + scoped.chdir(hub) + scoped.setenv("GIT_DIR", ".") + scoped.setenv("QUORUMGIT_AGENT", a) + line = [f"{op['oldrev']} {op['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + update_id = conn.execute( + "SELECT id FROM ref_updates WHERE status = 'prepared' AND newrev = ?", + (op["newrev"],), + ).fetchone()[0] + conn.execute( + "UPDATE ref_updates SET created_at = created_at - ? WHERE id = ?", + (gate.PREPARED_STUCK_AFTER_SECONDS + 60, update_id), + ) + conn.commit() + return hub, op, approval, update_id + + +def _finding(conn, update_id: int, repair: bool = False) -> dict: + findings = [ + f for f in gate.doctor_ref_updates(conn, repair=repair) + if f["ref_update_id"] == update_id + ] + assert len(findings) == 1, findings + return findings[0] + + +def _status(conn, update_id: int) -> str: + return conn.execute( + "SELECT status FROM ref_updates WHERE id = ?", (update_id,) + ).fetchone()[0] + + +def _move_ref(hub: Path, tmp_path: Path, refname: str, value: str) -> None: + # No store at this data dir, so QuorumGit's hook treats this as local + # maintenance and records nothing. + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(tmp_path / "no-store")} + env.pop("QUORUMGIT_AGENT", None) + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + subprocess.run( + ["git", "--git-dir", str(hub), "update-ref", refname, value], + check=True, + capture_output=True, + env=env, + ) + + +def test_doctor_reconciles_a_stuck_update_git_committed( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + hub, op, approval, update_id = _stuck_prepared_update(conn, tmp_path, cfg, monkeypatch) + _move_ref(hub, tmp_path, "refs/heads/main", op["newrev"]) + + detected = _finding(conn, update_id) + assert detected["outcome"] == "committed" and not detected["repaired"] + assert _status(conn, update_id) == "prepared" + + repaired = _finding(conn, update_id, repair=True) + conn.commit() + assert repaired["repaired"] and "error" not in repaired + assert _status(conn, update_id) == "committed" + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_doctor_reconciles_a_stuck_update_git_aborted( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + _hub, _op, approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + repaired = _finding(conn, update_id, repair=True) + conn.commit() + assert repaired["outcome"] == "aborted" and repaired["repaired"] + assert _status(conn, update_id) == "aborted" + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + events = conn.execute( + "SELECT count(*) FROM audit_events WHERE entity = 'ref_update' " + "AND entity_id = ? AND event_type = 'gate.update_reconciled_aborted'", + (update_id,), + ).fetchone() + assert events is not None and events[0] == 1 + + +def test_doctor_refuses_to_guess_when_the_ref_has_moved_on( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + hub, op, approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + # A later commit that is neither the update's old nor its new value. + pusher = conn.execute( + "SELECT a.name FROM ref_updates u JOIN agents a ON a.id = u.pusher_agent_id " + "WHERE u.id = ?", + (update_id,), + ).fetchone()[0] + clone = tmp_path / "clone" + _commit(clone, "src/later.py") + later = _push(clone, pusher, "HEAD:refs/heads/later", cfg=cfg) + assert later.returncode == 0, later.stderr + elsewhere = _rev(["-C", str(clone)], "HEAD") + assert elsewhere not in (op["oldrev"], op["newrev"]) + _move_ref(hub, tmp_path, "refs/heads/main", elsewhere) + + finding = _finding(conn, update_id, repair=True) + conn.commit() + assert not finding["repaired"] and "moved" in finding["error"] + assert _status(conn, update_id) == "prepared" + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_doctor_leaves_a_locked_ref_alone(committed_conn, tmp_path, cfg, monkeypatch): + conn = committed_conn + hub, _op, _approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + lock = hub / "refs" / "heads" / "main.lock" + lock.parent.mkdir(parents=True, exist_ok=True) + lock.write_text("", encoding="utf-8") + + finding = _finding(conn, update_id, repair=True) + conn.commit() + assert not finding["repaired"] and "locked" in finding["error"] + assert _status(conn, update_id) == "prepared" + + +def test_doctor_ignores_a_recent_prepared_update( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + _hub, _op, _approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + conn.execute( + "UPDATE ref_updates SET created_at = unixepoch() WHERE id = ?", (update_id,) + ) + conn.commit() + assert all( + f["ref_update_id"] != update_id for f in gate.doctor_ref_updates(conn) + ) + + +def test_doctor_cli_reports_stuck_updates(committed_conn, tmp_path, cfg, monkeypatch): + from tests.test_cli_hub import _cli + + conn = committed_conn + _hub, _op, _approval, update_id = _stuck_prepared_update( + conn, tmp_path, cfg, monkeypatch + ) + result = _cli(cfg, "doctor") + assert result.returncode == 1 + assert f"ref update {update_id} " in result.stdout + assert "stuck_prepared (git aborted) — detected" in result.stdout + + +# ------------------------------------------- review: fail-closed transactions + + +@pytest.mark.skipif(os.name == "nt", reason="Windows has no executable bit") +def test_push_requires_an_executable_transaction_hook(committed_conn, tmp_path, cfg): + conn = committed_conn + _repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + (hub / "hooks" / "reference-transaction").chmod(0o644) + _commit(clone, "docs/noexec.md", branch="feat/noexec") + rejected = _push(clone, a, "feat/noexec", cfg=cfg) + assert rejected.returncode != 0 + assert "not executable" in rejected.stderr + + +def test_identified_transaction_needs_a_fresh_validation( + committed_conn, tmp_path, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "docs/slow.md", branch="feat/slow") + newrev = _rev(["-C", str(clone)], "HEAD") + zero = "0" * 40 + conn.execute( + "INSERT INTO ref_updates (repository_id, refname, oldrev, newrev, " + "pusher_agent_id, created_at) VALUES (?, 'refs/heads/feat/slow', ?, ?, ?, " + "unixepoch() - ?)", + ( + registry.get_repository(conn, repo_name)["id"], + zero, + newrev, + registry.get_agent(conn, a)["id"], + gate.VALIDATION_TTL_SECONDS + 60, + ), + ) + conn.commit() + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + line = [f"{zero} {newrev} refs/heads/feat/slow\n"] + + # A push delayed past the validation window is rejected, not waved through. + monkeypatch.setenv("QUORUMGIT_AGENT", a) + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 1 + other = [f"{zero} {newrev} refs/heads/feat/never-validated\n"] + assert gate.run_reference_transaction(conn, repo_name, "prepared", other) == 1 + + # Only unidentified transactions are local maintenance. + monkeypatch.delenv("QUORUMGIT_AGENT") + assert gate.run_reference_transaction(conn, repo_name, "prepared", other) == 0 + + +def test_identified_local_ref_change_in_the_hub_is_rejected( + committed_conn, tmp_path, cfg +): + _repo_name, hub, _clone, a, _b = _setup(committed_conn, tmp_path) + head = _rev(["--git-dir", str(hub)], "refs/heads/main") + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(cfg.data_dir), "QUORUMGIT_AGENT": a} + env["PATH"] = str(Path(sys.executable).parent) + os.pathsep + env["PATH"] + result = subprocess.run( + ["git", "--git-dir", str(hub), "update-ref", "refs/heads/sneaky", head], + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode != 0 + assert "no current pre-receive validation" in result.stderr + + +@pytest.mark.parametrize( + ("agent", "state", "code"), + [("someone", "prepared", 1), (None, "prepared", 0), ("someone", "committed", 0)], +) +def test_missing_store_fails_closed_only_for_identified_prepare( + tmp_path, agent, state, code +): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(tmp_path / "vanished")} + env.pop("QUORUMGIT_AGENT", None) + if agent: + env["QUORUMGIT_AGENT"] = agent + result = subprocess.run( + [sys.executable, "-m", "quorumgit", "hook", "reference-transaction", + "--repo", "any", state], + input=f"{'0' * 40} {'1' * 40} refs/heads/x\n", + capture_output=True, + text=True, + env=env, + check=False, + ) + assert result.returncode == code, result.stderr + + +# --------------------------------------- content governance at the boundary + + +def _approve_planned(conn, repo_name: str, ref: str, clone: Path, pusher: str) -> list[dict]: + """Approve every operation `approve prepare` says pushing HEAD to ref needs.""" + plan = gate.prepare_push(conn, repo_name, ref, clone, pusher=pusher) + approvals = [ + approve(conn, entry["operation"], requested_by="requester") + for entry in plan["operations"] + ] + conn.commit() + return approvals + + +def test_out_of_scope_approval_is_spent_only_when_the_ref_moves( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "scoped") + work.claim_task(conn, task, a, branch="feat/scoped", scope_globs=["src/**"]) + conn.commit() + _commit(clone, "docs/outside.md", branch="feat/scoped") + + approvals = _approve_planned(conn, repo_name, "feat/scoped", clone, a) + assert [ap["operation"]["type"] for ap in approvals] == ["out_of_scope_push"] + + refusing = _install_update_hook(hub, "import sys\nsys.exit(1)\n") + refused = _push(clone, a, "feat/scoped", cfg=cfg) + assert refused.returncode != 0 + assert all( + gate.get_approval_by_id(conn, ap["id"])["status"] == "approved" + for ap in approvals + ) + + refusing.unlink() + accepted = _push(clone, a, "feat/scoped", cfg=cfg) + assert accepted.returncode == 0, accepted.stderr + assert all( + gate.get_approval_by_id(conn, ap["id"])["status"] == "consumed" + for ap in approvals + ) + + +def test_prepared_checks_the_paths_pre_receive_derived(committed_conn, tmp_path, cfg): + """A shared commit made known by an earlier ref still needs its approval.""" + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "scoped") + work.claim_task(conn, task, a, branch="feat/scoped", scope_globs=["src/**"]) + conn.commit() + _commit(clone, "docs/shared.md", branch="feat/scoped") + subprocess.run( + ["git", "-C", str(clone), "branch", "-f", "feat/base", "HEAD"], + check=True, + capture_output=True, + ) + approvals = _approve_planned(conn, repo_name, "feat/scoped", clone, a) + assert [ap["operation"]["type"] for ap in approvals] == ["out_of_scope_push"] + + # feat/base is updated first; once it lands, the shared commit is known + # and recomputing feat/scoped's paths would find nothing to govern. + pushed = _push(clone, a, "feat/base", "feat/scoped", cfg=cfg) + assert pushed.returncode == 0, pushed.stderr + assert _rev(["--git-dir", str(hub)], "refs/heads/feat/scoped") == _rev( + ["-C", str(clone)], "HEAD" + ) + assert all( + gate.get_approval_by_id(conn, ap["id"])["status"] == "consumed" + for ap in approvals + ) From 8c80511615996fdc2b94c9a5046faf80e4dc651f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 23:45:10 +0000 Subject: [PATCH 2/3] Compare schema SQL as tokens so formatting never fails the contract Review on #28: the schema fingerprint only ignored whitespace around brackets and commas, so an object recreated with lowercase keywords or different operator spacing (role<>'' versus role <> '') was reported as altered and every command refused the store. SQL is now compared as a token sequence: keyword and bare-name case, whitespace, and comments are ignored, while quoted literals and quoted identifiers stay exact. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp --- src/quorumgit/store.py | 31 +++++++++++++++++++++++++++++-- tests/test_hardening.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/quorumgit/store.py b/src/quorumgit/store.py index 6773a02..7266600 100644 --- a/src/quorumgit/store.py +++ b/src/quorumgit/store.py @@ -343,11 +343,38 @@ def migrate(target: Config | str | Path) -> list[str]: # ------------------------------------------------------------ contract check +# One SQL token: a quoted literal or identifier (kept exactly), a comment +# (dropped), a word or number, a multi-character operator, or any other +# single character. Whitespace between tokens is insignificant. +_SQL_TOKEN = re.compile( + r"""'(?:[^']|'')*'""" + r'|"(?:[^"]|"")*"' + r"|`(?:[^`]|``)*`" + r"|\[[^\]]*\]" + r"|--[^\n]*" + r"|/\*.*?\*/" + r"|\w+" + r"|<>|!=|<=|>=|==|\|\||<<|>>" + r"|\S", + re.DOTALL, +) + + def _normalized_sql(sql: str | None) -> str | None: - """Schema SQL with formatting-only whitespace removed.""" + """Schema SQL as a token sequence, ignoring formatting. + + Keyword and bare-name case, whitespace, and comments do not change what + SQLite builds, so they are normalized away; quoted literals and quoted + identifiers are compared exactly. + """ if sql is None: return None - return re.sub(r" ?([(),;]) ?", r"\1", " ".join(sql.split())) + tokens = [] + for token in _SQL_TOKEN.findall(sql): + if token.startswith(("--", "/*")): + continue + tokens.append(token if token[0] in "'\"`[" else token.lower()) + return " ".join(tokens) def _schema_objects(conn: Connection) -> dict[tuple[str, str], tuple[str, str | None]]: diff --git a/tests/test_hardening.py b/tests/test_hardening.py index a2e307a..f635f25 100644 --- a/tests/test_hardening.py +++ b/tests/test_hardening.py @@ -137,6 +137,42 @@ def test_contract_tolerates_formatting_only_schema_differences(fresh_store): store.verify_contract(fresh_store) +def test_contract_tolerates_keyword_case_and_operator_spacing(fresh_store): + _tamper( + fresh_store, + "DROP INDEX approvals_one_live_per_operation", + "create unique index approvals_one_live_per_operation on approvals" + "(operation_hash) where status in ('pending','approved') -- rebuilt", + ) + store.verify_contract(fresh_store) + + +def test_contract_still_rejects_a_changed_literal(fresh_store): + _tamper( + fresh_store, + "DROP INDEX approvals_one_live_per_operation", + "CREATE UNIQUE INDEX approvals_one_live_per_operation ON approvals" + "(operation_hash) WHERE status IN ('pending', 'APPROVED')", + ) + with pytest.raises( + store.ContractViolation, match="altered index approvals_one_live_per_operation" + ): + store.verify_contract(fresh_store) + + +@pytest.mark.parametrize( + ("a", "b", "same"), + [ + ("CHECK (role <> '')", "check(role<>'')", True), + ("x IN ('a', 'b')", "x in ('a','b') /* c */", True), + ("x = 'Worker'", "x = 'worker'", False), + ('CREATE TABLE "T"(a)', 'create table "t"(a)', False), + ], +) +def test_schema_sql_normalization(a, b, same): + assert (store._normalized_sql(a) == store._normalized_sql(b)) is same + + # ------------------------------------------------------ repository identity From 5ae3063f8386b364296fb4c2fe396b06d4267174 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 00:49:25 +0000 Subject: [PATCH 3/3] Port three transaction-boundary regression cases from the #10 handover From the session that rebuilt this boundary on #10's branch (44a6b5f): - A force push refused by receive.denyNonFastForwards, which Git checks after pre-receive, keeps its approval and the retry uses it. On main the approval is consumed and the branch does not move. - An update needing two approvals (protected ref and protected path) consumes both at `prepared`, records both in approval_ids, and restores both on `aborted`. - A non-atomic push of main and a claimed branch whose commits include main's still matches the out-of-scope operation that was approved. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp --- tests/test_transaction_boundary.py | 132 ++++++++++++++++++++++++++++- 1 file changed, 130 insertions(+), 2 deletions(-) diff --git a/tests/test_transaction_boundary.py b/tests/test_transaction_boundary.py index 9afe192..ae32255 100644 --- a/tests/test_transaction_boundary.py +++ b/tests/test_transaction_boundary.py @@ -17,8 +17,8 @@ import pytest -from quorumgit import gate, registry, work -from tests.conftest import approve +from quorumgit import gate, registry, store, work +from tests.conftest import OPERATOR, approve, ensure_agent from tests.test_gate import _commit, _push, _setup @@ -641,3 +641,131 @@ def test_prepared_checks_the_paths_pre_receive_derived(committed_conn, tmp_path, gate.get_approval_by_id(conn, ap["id"])["status"] == "consumed" for ap in approvals ) + + +# ---------------------------------------------- ported from the #10 handover +# Regression cases written by the session that rebuilt this boundary on the +# #10 branch (44a6b5f), adapted to this PR's `approval_ids` column. + + +def test_approval_survives_a_non_fast_forward_git_denies_by_config( + committed_conn, tmp_path, cfg +): + """`git init --shared` hubs set receive.denyNonFastForwards, which Git + enforces after pre-receive; the force-push approval must not be spent.""" + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "first.py", branch="feat/rewrite") + assert _push(clone, a, "feat/rewrite", cfg=cfg).returncode == 0 + oldrev = _rev(["--git-dir", str(hub)], "refs/heads/feat/rewrite") + subprocess.run( + ["git", "-C", str(clone), "reset", "--hard", "HEAD~1"], + check=True, capture_output=True, + ) + _commit(clone, "second.py", branch="feat/rewrite") + op = { + "type": "force_update", + "repository": repo_name, + "refname": "refs/heads/feat/rewrite", + "oldrev": oldrev, + "newrev": _rev(["-C", str(clone)], "HEAD"), + } + approval = approve(conn, op, requested_by="requester") + conn.commit() + + subprocess.run( + ["git", "--git-dir", str(hub), "config", "receive.denyNonFastForwards", "true"], + check=True, + ) + denied = _push(clone, a, "+feat/rewrite", cfg=cfg) + assert denied.returncode != 0 + assert "non-fast-forward" in denied.stderr + assert _rev(["--git-dir", str(hub)], "refs/heads/feat/rewrite") == oldrev + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "approved" + + subprocess.run( + ["git", "--git-dir", str(hub), "config", "--unset", "receive.denyNonFastForwards"], + check=True, + ) + forced = _push(clone, a, "+feat/rewrite", cfg=cfg) + assert forced.returncode == 0, forced.stderr + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_every_approval_of_one_update_is_consumed_and_restored_together( + committed_conn, tmp_path, cfg, monkeypatch +): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + ensure_agent(conn, OPERATOR, "operator") + registry.set_protected_path(conn, repo_name, "deploy.yml", actor=OPERATOR) + conn.commit() + _commit(clone, "deploy.yml") + # Bring the objects into the hub without a governed ref update. + subprocess.run( + ["git", "--git-dir", str(hub), "fetch", str(clone), "HEAD"], + check=True, capture_output=True, + ) + oldrev = _rev(["--git-dir", str(hub)], "refs/heads/main") + plan = gate.prepare_push(conn, repo_name, "main", clone, pusher=a) + required = {entry["operation"]["type"]: entry["operation"] for entry in plan["operations"]} + assert set(required) == {"protected_ref_update", "protected_path_update"} + approvals = [ + approve(conn, operation, requested_by="requester") + for operation in required.values() + ] + conn.commit() + + monkeypatch.chdir(hub) + monkeypatch.setenv("GIT_DIR", ".") + monkeypatch.setenv("QUORUMGIT_AGENT", a) + line = [f"{oldrev} {plan['newrev']} refs/heads/main\n"] + assert gate.run_pre_receive(conn, repo_name, line) == 0 + + def statuses() -> set[str]: + return { + gate.get_approval_by_id(conn, approval["id"])["status"] + for approval in approvals + } + + assert statuses() == {"approved"} + + assert gate.run_reference_transaction(conn, repo_name, "prepared", line) == 0 + assert statuses() == {"consumed"} + row = conn.execute( + "SELECT approval_ids FROM ref_updates WHERE refname = 'refs/heads/main' " + "AND newrev = ? AND status = 'prepared'", + (plan["newrev"],), + ).fetchone() + assert row is not None + assert sorted(store.json_loads(row[0], [])) == sorted(ap["id"] for ap in approvals) + + assert gate.run_reference_transaction(conn, repo_name, "aborted", line) == 0 + assert statuses() == {"approved"} + + +def test_prepared_uses_the_paths_pre_receive_derived(committed_conn, tmp_path, cfg): + """In a non-atomic push each ref commits separately. When main lands + first, a later ref's commits no longer look new at `prepared`; the + operations must still be the ones pre-receive derived and approved.""" + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "ordered refs") + work.claim_task(conn, task, a, branch="work/x", scope_globs=["src/**"]) + conn.commit() + _commit(clone, "prod.tf") # on main, outside the claim's scope + _commit(clone, "schema.sql", branch="work/x") # also outside it + plans = [ + gate.prepare_push(conn, repo_name, ref, clone, ref, pusher=a) + for ref in ("main", "work/x") + ] + operations = [entry["operation"] for plan in plans for entry in plan["operations"]] + out_of_scope = next(op for op in operations if op["type"] == "out_of_scope_push") + assert out_of_scope["paths"] == ["prod.tf", "schema.sql"] + approvals = [approve(conn, op, requested_by="requester") for op in operations] + conn.commit() + + pushed = _push(clone, a, "main", "work/x", cfg=cfg) + assert pushed.returncode == 0, pushed.stderr + for approval in approvals: + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed"