diff --git a/README.md b/README.md index 0f20f3e..f2d1be9 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ Then initialize the store once: ```bash $ quorumgit init store: /home/you/.quorumgit/quorumgit.db -migrations applied: ['001_core.sql', '002_approval_identities.sql'] +migrations applied: ['001_core.sql', '002_approval_identities.sql', '003_approver_policy.sql'] contract: ok ``` @@ -161,13 +161,17 @@ git push origin feat/x # rejected — unidentified 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. +**Scopes are enforced on the push, not just the claim.** When the owner pushes a claimed branch, the hook lists the paths changed by the commits the push introduces (commits not yet reachable from any ref on the hub) and checks each against the claim's declared scopes. A push that touches anything outside them is rejected as an `out_of_scope_update` naming the stray paths, and lands only with an approval bound to that exact update — the operation includes the sorted `out_of_scope_paths`, so the approver sees what they are signing off on. Renames count as a deletion plus an addition, so moving a file *into* scope still charges its old location. Merging already-published history (for example, `main`) into the branch is not charged; edits made inside a merge commit — conflict resolutions or extra changes — are. Unclaimed branches are not scope-checked, and neither is the local model, which has no hook. + +Scope globs match repository-relative paths: `**` spans any number of directories, `*` and `?` stay within one path segment, and a glob without wildcards names a single file or a whole directory (`docs` covers `docs/guide.md`). + 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 `. 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. ## Protected operations and approvals -Updates to protected refs, force pushes, ref deletions, and lease takeovers all require an approval. An approval is bound by SHA-256 hash to the **exact operation payload** (canonical JSON) — approving one push authorizes that push at those exact revisions, and nothing else. +Updates to protected refs, force pushes, ref deletions, pushes that change paths outside the pusher's claimed scopes, and lease takeovers all require an approval. An approval is bound by SHA-256 hash to the **exact operation payload** (canonical JSON) — approving one push authorizes that push at those exact revisions, and nothing else. The flow, driven by the rejection messages themselves: @@ -177,9 +181,12 @@ The flow, driven by the rejection messages themselves: # bound to this exact update (hash sha256:ab12…). # 2. Any registered agent requests that exact operation, and an approver votes -# on the returned approval instance ID. The approver must be registered too: +# on the returned approval instance ID. The approver must be registered too. +# --push derives the operation from the commit in your clone, so nobody +# has to write the JSON by hand: quorumgit agent add operator -quorumgit approve request '{"type":"protected_ref_update","repository":"myproject","refname":"refs/heads/main","oldrev":"","newrev":""}' +quorumgit approve request --repo myproject --push main --from ./my-clone --agent agent-one +# operation: {"newrev": "", "oldrev": "", "refname": "refs/heads/main", "repository": "myproject", "type": "protected_ref_update"} # approval 17 hash=sha256:ab12… status=pending quorumgit approve vote 17 --agent operator @@ -190,11 +197,31 @@ quorumgit approve vote 17 --agent operator Rules that hold no matter what: - **An approval never bypasses branch reservations.** A claimed branch still accepts pushes only from its claiming agent; a branch frozen by an open handoff accepts none at all. The hook checks reservations *before* approvals. -- **Denial has precedence and is terminal.** One `--deny` vote denies the approval; later yes votes are refused. +- **Denial has precedence and is terminal.** One `--deny` vote denies the approval; later yes votes are refused. An approved approval that has not been used yet still accepts votes, so a deny revokes it before use. - **Consumption is single-use under concurrency.** Two simultaneous pushes racing for one approval produce exactly one accepted push — the loser is rejected, not silently allowed. - **Votes bind to one approval instance.** A delayed vote for an older denied or consumed instance cannot decide a newer request with the same operation hash. Requesters, voters, consumers, and pushers must name registered agents. - **A consumed approval is spent, not blacklisted.** Consumption moves the approval to a terminal `consumed` state (with `consumed_at`) rather than reusing `denied`, and only one *live* (`pending` or `approved`) approval may exist per operation hash. The same operation can therefore be requested and approved again later as a new approval instance — which matters for takeovers, whose payload is stable and legitimately repeatable. What is never possible is one approval authorizing twice. -- The default threshold is 1; `--threshold` sets a higher one. "Operator" is a convention, not a role: **any registered agent can vote, including the agent that requested the approval or is making the push.** Keeping approvals in human hands depends on how agent identities are used, not on anything QuorumGit enforces (see the threat model). +- **By default the policy is open:** the threshold is 1 (`--threshold` sets a higher one) and any registered agent can vote, including the agent that requested the approval or is making the push. A repository can tighten this — see below. + +### Approval policy per repository + +Two optional settings restrict who can approve operations on a repository (operations name it in their `repository` field): + +```bash +quorumgit repo approver add myproject alice # build an approver roster +quorumgit repo approver add myproject bob +quorumgit repo approver add myproject carol +quorumgit repo policy myproject --separate-duties +quorumgit repo policy myproject # show the current policy +``` + +- **Approver roster.** Once a repository has approvers, only they can vote on its approvals, and the threshold becomes a **2/3 + 1 quorum** of the roster (1 of 1, 2 of 2, 3 of 3, 3 of 4, 4 of 5, …). `--threshold` on a request can raise that bar but not lower it. +- **Separation of duties.** With `--separate-duties`, the agent that requested an approval cannot vote on it, and the agent that *uses* it — the pusher, or the agent taking over a lease — cannot count its own yes vote. With a roster, the quorum is counted over the approvers eligible for that particular approval, so these exclusions never make approval impossible while other approvers exist; a sole approver, however, can never approve their own operation. +- **Quorum is re-checked when an approval is used,** against the roster and policy at that moment. Removing an approver withdraws their vote from every approval not yet used; if that drops an approval below quorum, the push or takeover is refused until more eligible approvers vote. + +Every roster and policy change is audited. Without a roster or separation of duties, the open policy above applies — "operator" is then a convention, not a role, and keeping approvals in human hands depends on how agent identities are used (see the threat model). + +`approve request --push` runs the hook's own derivation against the hub, with your clone's object store attached read-only (the way Git exposes incoming objects to a pre-receive hook). It fetches and writes nothing, reads the old revision from the hub's current ref, resolves `--rev` (default `HEAD`) in `--from`, and produces the same operation the hook will compute — protected update, force push, deletion (`--delete`), or out-of-scope update with its path list — for the agent named by `--pusher` (default: the requester). It applies the same branch reservations as the hook, so a push the hook would refuse outright is refused here too. If the hub's ref moves before you push, the operation no longer matches: derive and request again. Takeovers follow the same pattern: claiming a task someone else holds (`claim --takeover`) prints the takeover operation to approve. Its payload includes the incumbent claim ID, so an unused approval cannot displace a later claim by the same agent. The takeover is atomic — the incumbent is released, the replacement claim created, and the approval consumed in one transaction, or none of it happens. A refused takeover leaves the incumbent untouched and the approval unconsumed. @@ -207,6 +234,8 @@ Takeovers follow the same pattern: claiming a task someone else holds (`claim [--protected-ref ]…` | Register a repository | +| `quorumgit repo approver add\|remove ` / `repo approver list ` | Manage a repository's approver roster | +| `quorumgit repo policy [--separate-duties \| --no-separate-duties]` | Show or change a repository's approval policy | | `quorumgit agent add ` | Register an agent identity | | `quorumgit task add --repo --title [--objective ]` | Create a task | | `quorumgit claim --branch --scope … [--no-worktree] [--takeover] [--override-overlap] [--lease-hours ]` | Claim a task | @@ -219,6 +248,7 @@ Takeovers follow the same pattern: claiming a task someone else holds (`claim ` | Cancel — creator only; removes the retained worktree, and is refused if it has uncommitted changes | | `quorumgit handoff list [--status ] / show ` | Inspect handoffs | | `quorumgit approve request [--threshold ]` | Open an approval for an exact operation | +| `quorumgit approve request --repo --push (--from [--rev ] \| --delete) [--pusher ]` | Derive the operation a push will need, before pushing, and open an approval for it (prints nothing to approve if the push is ungoverned) | | `quorumgit approve vote [--deny]` | Vote on one approval instance | | `quorumgit approve hash ` | Compute an operation's hash | | `quorumgit hook install --repo ` | Install the pre-receive hook (hub model) | @@ -259,7 +289,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. -- Approval hashes provide exact-payload binding and tamper-evidence, not approver authentication. Any registered agent may vote, including the requester or pusher, so an agent can approve its own protected operation. +- Approval hashes provide exact-payload binding and tamper-evidence, not approver authentication. Approver rosters and separation of duties restrict *which asserted identities* may approve, but because identity is asserted, a process that can claim to be an approver can vote as one. These are the correct trade-offs for preventing well-intentioned agents from colliding on one machine. They are not Byzantine fault tolerance, and this document will not pretend otherwise. diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index ffea5b4..18c1cbd 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -119,6 +119,56 @@ def cmd_repo_list(args, cfg) -> int: return 0 +def cmd_repo_approver_add(args, cfg) -> int: + with store.connect(cfg) as conn: + registry.add_approver(conn, args.repo, args.agent_name) + conn.commit() + print(f"{args.agent_name} is now an approver for {args.repo}.") + return 0 + + +def cmd_repo_approver_remove(args, cfg) -> int: + with store.connect(cfg) as conn: + registry.remove_approver(conn, args.repo, args.agent_name) + conn.commit() + print(f"{args.agent_name} is no longer an approver for {args.repo}.") + return 0 + + +def cmd_repo_approver_list(args, cfg) -> int: + with store.connect(cfg) as conn: + for name in registry.get_repository(conn, args.repo)["approvers"]: + print(name) + return 0 + + +def cmd_repo_policy(args, cfg) -> int: + with store.connect(cfg) as conn: + if args.separate_duties is not None: + registry.set_separate_duties(conn, args.repo, args.separate_duties) + conn.commit() + repo = registry.get_repository(conn, args.repo) + approvers = repo["approvers"] + if approvers: + print(f"approvers: {', '.join(approvers)}") + print( + f"quorum: {gate.quorum_threshold(len(approvers))} of " + f"{len(approvers)} (2/3 + 1)" + ) + else: + print("approvers: any registered agent") + print("quorum: the threshold given with each request (default 1)") + if repo["separate_duties"]: + print("separate duties: on — the requester's vote is refused and the " + "agent using an approval cannot count its own vote") + if approvers: + print(" (quorum is counted over the approvers eligible for each " + "approval)") + else: + print("separate duties: off") + return 0 + + def cmd_agent_add(args, cfg) -> int: with store.connect(cfg) as conn: agent_id = registry.add_agent(conn, args.name) @@ -357,10 +407,37 @@ def _operation_from_args(args) -> dict: def cmd_approve_request(args, cfg) -> int: + requester = _agent(args, cfg) + if (args.operation is None) == (args.push is None): + print("Give either an operation JSON object or --push .", + file=sys.stderr) + return 2 with store.connect(cfg) as conn: + if args.push is not None: + if not args.repo: + print("--push requires --repo .", file=sys.stderr) + return 2 + if args.delete == bool(args.source): + print("--push requires exactly one of --from or --delete.", + file=sys.stderr) + return 2 + operation = gate.derive_push_operation( + conn, + args.repo, + args.push, + args.pusher or requester, + source=args.source, + rev=args.rev, + delete=args.delete, + ) + if operation is None: + print("This push needs no approval; nothing requested.") + return 0 + print(f"operation: {json.dumps(operation, sort_keys=True)}") + else: + operation = _operation_from_args(args) approval = gate.request_approval( - conn, _operation_from_args(args), requested_by=_agent(args, cfg), - threshold=args.threshold, + conn, operation, requested_by=requester, threshold=args.threshold, ) conn.commit() print( @@ -445,6 +522,26 @@ def add(name, fn, configure=None, parent=None): sp.add_argument("--protected-ref", action="append", default=[]), ), parent=repo) add("list", cmd_repo_list, parent=repo) + approver = repo.add_parser("approver").add_subparsers(dest="action", required=True) + add("add", cmd_repo_approver_add, lambda sp: ( + sp.add_argument("repo"), + sp.add_argument("agent_name"), + ), parent=approver) + add("remove", cmd_repo_approver_remove, lambda sp: ( + sp.add_argument("repo"), + sp.add_argument("agent_name"), + ), parent=approver) + add("list", cmd_repo_approver_list, + lambda sp: sp.add_argument("repo"), parent=approver) + add("policy", cmd_repo_policy, lambda sp: ( + sp.add_argument("repo"), + sp.add_argument("--separate-duties", dest="separate_duties", + action="store_true", default=None, + help="forbid agents from approving their own requests, " + "pushes, or takeovers"), + sp.add_argument("--no-separate-duties", dest="separate_duties", + action="store_false"), + ), parent=repo) agent = sub.add_parser("agent").add_subparsers(dest="sub", required=True) add("add", cmd_agent_add, lambda sp: sp.add_argument("name"), parent=agent) @@ -525,7 +622,22 @@ def add(name, fn, configure=None, parent=None): ap = sub.add_parser("approve").add_subparsers(dest="sub", required=True) add("request", cmd_approve_request, lambda sp: ( - sp.add_argument("operation", help="operation JSON object"), + sp.add_argument("operation", nargs="?", help="operation JSON object"), + sp.add_argument("--push", metavar="REF", + help="derive the operation for pushing this ref " + "(branch name or full refname) instead of " + "passing JSON"), + sp.add_argument("--repo", help="registered repository (with --push)"), + sp.add_argument("--from", dest="source", metavar="CLONE", + help="repository holding the commit to push " + "(with --push)"), + sp.add_argument("--rev", default="HEAD", + help="commit to push, resolved in --from " + "(default HEAD)"), + sp.add_argument("--delete", action="store_true", + help="derive a deletion of the ref (with --push)"), + sp.add_argument("--pusher", + help="agent that will push (default: the requester)"), sp.add_argument("--agent"), sp.add_argument("--threshold", type=int, default=1), ), parent=ap) diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index 948d99d..32635a9 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -15,13 +15,15 @@ from pathlib import Path from typing import Any -from . import audit +from . import audit, work from .canonical import stable_hash from .registry import ( RegistryError, + approvers_for_repository, assert_repository_identity_unique, get_agent, get_repository, + git_common_dir, ) from .store import Connection, begin_immediate, json_dumps, json_loads from .work import live_claim_for_branch, open_handoff_for_branch @@ -89,6 +91,95 @@ def _approval_dict(row) -> dict: } +def quorum_threshold(approver_count: int) -> int: + """Integer-stable 2/3 + 1 quorum over an approver roster.""" + return (2 * approver_count) // 3 + 1 + + +def approval_policy(conn: Connection, repository: str) -> dict: + """The approval policy governing operations on a repository. + + Operations naming an unregistered repository, or a repository with no + roster and separation of duties off, keep the open policy: any registered + agent may vote and the requested threshold applies. + """ + row = conn.execute( + "SELECT id, separate_duties FROM repositories WHERE name = ?", + (repository,), + ).fetchone() + if row is None: + return {"approvers": [], "separate_duties": False} + return { + "approvers": approvers_for_repository(conn, row[0]), + "separate_duties": bool(row[1]), + } + + +def required_threshold( + requested: int, policy: dict, excluded: set[str] | frozenset[str] = frozenset() +) -> int: + """A roster sets a 2/3 + 1 floor; a request may only raise it. + + The quorum is taken over the approvers eligible to vote on this approval, + so separation of duties never makes approval arithmetically impossible + for a roster that still has other members. + """ + if policy["approvers"]: + eligible = set(policy["approvers"]) - set(excluded) + return max(requested, quorum_threshold(len(eligible))) + return requested + + +def _requester(conn: Connection, approval_id: int) -> str | None: + row = conn.execute( + """ + SELECT a.name FROM approvals ap + JOIN agents a ON a.id = ap.requested_by_agent_id + WHERE ap.id = ? + """, + (approval_id,), + ).fetchone() + return row[0] if row else None + + +def _excluded( + conn: Connection, approval: dict, policy: dict, consumer: str | None = None +) -> set[str]: + """Agents whose approval does not count under separation of duties.""" + if not policy["separate_duties"]: + return set() + requester = _requester(conn, approval["id"]) + return {name for name in (requester, consumer) if name} + + +def _tally( + conn: Connection, + approval: dict, + policy: dict, + excluded: set[str], +) -> tuple[list[str], list[str]]: + """Yes votes that count under the current policy, and all deny votes. + + Recomputed on every decision and again at consumption, so removing an + approver withdraws their vote and separation of duties discounts the + consuming agent's own approval. + """ + rows = conn.execute( + "SELECT voter, vote FROM votes WHERE approval_id = ? ORDER BY voter", + (approval["id"],), + ).fetchall() + roster = set(policy["approvers"]) + yes = [ + voter + for voter, choice in rows + if choice == 1 + and (not roster or voter in roster) + and voter not in excluded + ] + no = [voter for voter, choice in rows if choice == 0] + return yes, no + + def request_approval( conn: Connection, operation: dict[str, Any], @@ -174,14 +265,26 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: """Record a vote against one explicit approval instance atomically. BEGIN IMMEDIATE serializes competing voters before either reads the current - approval state. Denial has precedence and terminal states remain final. + approval state. Denial has precedence and terminal states remain final. An + approved but unused approval still accepts votes, so a deny revokes it + before use and extra yes votes can restore quorum after a roster change. """ begin_immediate(conn) voter_row = get_agent(conn, voter) approval = get_approval_by_id(conn, approval_id) - if approval["status"] != "pending": + if approval["status"] not in ("pending", "approved"): raise GateError(f"Approval {approval_id} is already {approval['status']}.") - threshold = approval["threshold"] + repository = str(approval["operation"].get("repository")) + policy = approval_policy(conn, repository) + if policy["approvers"] and voter not in policy["approvers"]: + raise GateError( + f"Agent {voter} is not an approver for repository {repository}." + ) + if policy["separate_duties"] and voter == _requester(conn, approval["id"]): + raise GateError( + f"Agent {voter} requested approval {approval_id}; separation of " + f"duties on repository {repository} forbids voting on it." + ) conn.execute( """ INSERT INTO votes (approval_id, voter, vote, voter_agent_id) @@ -201,24 +304,18 @@ def vote(conn: Connection, approval_id: int, voter: str, approve: bool) -> dict: detail={"vote": approve, "hash": approval["operation_hash"]}, ) - counts = conn.execute( - "SELECT count(*) FILTER (WHERE vote = 1), " - "count(*) FILTER (WHERE vote = 0) " - "FROM votes WHERE approval_id = ?", - (approval["id"],), - ).fetchone() - assert counts is not None - yes, no = counts - if no > 0: + excluded = _excluded(conn, approval, policy) + yes, no = _tally(conn, approval, policy, excluded) + if no: new_status = "denied" - elif yes >= threshold: + elif len(yes) >= required_threshold(approval["threshold"], policy, excluded): new_status = "approved" else: new_status = "pending" - if new_status != "pending": + if new_status != approval["status"]: conn.execute( "UPDATE approvals SET status = ?, decided_at = unixepoch() " - "WHERE id = ? AND status = 'pending'", + "WHERE id = ? AND status IN ('pending', 'approved')", (new_status, approval["id"]), ) audit.record( @@ -267,6 +364,19 @@ def consume_approval( f"Approval {op_hash} is not consumable (status " f"{approval['status']}); it may already be used." ) + policy = approval_policy(conn, str(operation["repository"])) + excluded = _excluded(conn, approval, policy, consumer=agent) + yes, _no = _tally(conn, approval, policy, excluded) + required = required_threshold(approval["threshold"], policy, excluded) + if len(yes) < required: + detail = "" + if policy["separate_duties"]: + detail = f"; {agent}'s own vote and the requester's do not count" + raise GateError( + f"Approval {approval_id} does not satisfy the current approval " + f"policy for {operation['repository']}: {len(yes)} eligible yes " + f"vote(s), {required} required{detail}." + ) cur = conn.execute( "UPDATE approvals SET status = 'consumed', consumed_at = unixepoch(), " "consumed_by_agent_id = ? " @@ -298,7 +408,9 @@ def _is_zero(oid: str) -> bool: return set(oid) == {"0"} -def _is_fast_forward(git_dir: str, oldrev: str, newrev: str) -> bool: +def _is_fast_forward( + git_dir: str, oldrev: str, newrev: str, env: dict[str, str] | None = None +) -> bool: result = subprocess.run( [ "git", @@ -311,12 +423,62 @@ def _is_fast_forward(git_dir: str, oldrev: str, newrev: str) -> bool: ], capture_output=True, check=False, + env=env, ) if result.returncode not in (0, 1): raise PushRejected("Unable to determine fast-forward status.") return result.returncode == 0 +def _new_commit_paths( + git_dir: str, newrev: str, env: dict[str, str] | None = None +) -> list[str]: + """Paths changed by the commits this push introduces. + + Only commits not yet reachable from any existing ref are inspected, so a + branch created from (or merged with) already-published history is not + charged for that history. Merge commits use Git's combined diff, which + reports only paths the merge result changes relative to every parent: + conflict resolutions and edits made inside the merge itself. Renames are + split into a deletion and an addition so both paths are checked. + """ + commits = subprocess.run( + ["git", "--git-dir", git_dir, "rev-list", newrev, "--not", "--all"], + capture_output=True, + check=False, + env=env, + ) + if commits.returncode != 0: + raise PushRejected("Unable to list the commits introduced by this push.") + if not commits.stdout.strip(): + return [] + result = subprocess.run( + [ + "git", + "--git-dir", + git_dir, + "diff-tree", + "--stdin", + "-r", + "-c", + "-z", + "--root", + "--no-renames", + "--no-commit-id", + "--name-only", + ], + input=commits.stdout, + capture_output=True, + check=False, + env=env, + ) + if result.returncode != 0: + raise PushRejected("Unable to determine the paths changed by this push.") + return sorted( + {item.decode("utf-8") for item in result.stdout.split(b"\0") if item} + ) + + def _invoking_git_common_dir(git_dir: str) -> Path: result = subprocess.run( ["git", "--git-dir", git_dir, "rev-parse", "--git-common-dir"], @@ -353,18 +515,29 @@ def _verify_repository_binding( return repo -def check_ref_update( +def governed_operation( conn: Connection, - repository: str, + repo: dict, git_dir: str, pusher: str | None, oldrev: str, newrev: str, refname: str, -) -> None: - """Enforce governance for one ref update. Raises PushRejected.""" - repo = _verify_repository_binding(conn, repository, git_dir) + env: dict[str, str] | None = None, +) -> dict[str, Any] | None: + """The approval-gated operation a ref update amounts to, if any. + + Raises PushRejected when branch reservations forbid the update outright + (approvals never override them). Returns None for an ungoverned update. + This is the single derivation shared by the pre-receive hook and + `approve request --push`, so a request built before pushing binds to the + exact payload the hook recomputes. `env` lets the offline derivation see + the pusher's not-yet-pushed objects through an object alternate. + """ + repository = repo["name"] branch = refname.removeprefix("refs/heads/") + deletion = _is_zero(newrev) + out_of_scope: list[str] = [] if refname.startswith("refs/heads/"): pending = open_handoff_for_branch(conn, repo["id"], branch) @@ -380,30 +553,154 @@ def check_ref_update( f"(claim {claim['id']}); pusher is " f"{pusher or 'unidentified — set QUORUMGIT_AGENT'}." ) + if claim and not deletion: + # Declared scopes are enforced on what the push actually changes, + # not only on what the claim said it would change. + out_of_scope = work.paths_outside_scopes( + _new_commit_paths(git_dir, newrev, env), + work.claim_scopes(conn, claim["id"]), + ) protected = refname in repo["protected_refs"] - deletion = _is_zero(newrev) forced = ( not deletion and not _is_zero(oldrev) - and not _is_fast_forward(git_dir, oldrev, newrev) + and not _is_fast_forward(git_dir, oldrev, newrev, env) + ) + + if not (protected or deletion or forced or out_of_scope): + return None + if protected: + op_type = "protected_ref_update" + elif deletion: + op_type = "ref_delete" + elif forced: + op_type = "force_update" + else: + op_type = "out_of_scope_update" + operation: dict[str, Any] = { + "type": op_type, + "repository": repository, + "refname": refname, + "oldrev": oldrev, + "newrev": newrev, + } + if out_of_scope: + operation["out_of_scope_paths"] = out_of_scope + return operation + + +def _git_out(args: list[str], what: str, env: dict[str, str] | None = None) -> str: + result = subprocess.run( + ["git", *args], capture_output=True, text=True, check=False, env=env ) + if result.returncode != 0: + detail = (result.stderr or result.stdout).strip() + raise GateError(f"{what}{': ' + detail if detail else ''}") + return result.stdout.strip() + - if protected or deletion or forced: - operation = { - "type": "protected_ref_update" - if protected - else ("ref_delete" if deletion else "force_update"), - "repository": repository, - "refname": refname, - "oldrev": oldrev, - "newrev": newrev, +def derive_push_operation( + conn: Connection, + repository: str, + refname: str, + pusher: str, + *, + source: str | Path | None = None, + rev: str = "HEAD", + delete: bool = False, +) -> dict[str, Any] | None: + """Derive, before pushing, the exact operation the hook will require. + + The hub's current value of the ref is the old revision; the new revision + is resolved in the pusher's own repository (`source`). Nothing is fetched + or written: the hub is inspected with the source's object store attached + as a read-only alternate, the same way Git exposes incoming objects to a + real pre-receive hook, and the hook's own derivation is reused. Returns + None when the push needs no approval. + """ + if not refname.startswith("refs/"): + refname = f"refs/heads/{refname}" + repo = get_repository(conn, repository) + get_agent(conn, pusher) + hub_git_dir = str(assert_repository_identity_unique(conn, repo)) + current = subprocess.run( + ["git", "--git-dir", hub_git_dir, "rev-parse", "--verify", "--quiet", + f"{refname}^{{commit}}"], + capture_output=True, + text=True, + check=False, + ) + exists = current.returncode == 0 + + env: dict[str, str] | None = None + if delete: + if not exists: + raise GateError(f"{refname} does not exist in {repository}; nothing to delete.") + oldrev = current.stdout.strip() + newrev = "0" * len(oldrev) + else: + if source is None: + raise GateError("Deriving a push needs --from .") + source_path = Path(source).resolve() + newrev = _git_out( + ["-C", str(source_path), "rev-parse", "--verify", f"{rev}^{{commit}}"], + f"Cannot resolve {rev!r} to a commit in {source_path}", + ) + try: + source_common = git_common_dir(source_path) + except RegistryError as exc: + raise GateError(str(exc)) from exc + env = { + **os.environ, + "GIT_ALTERNATE_OBJECT_DIRECTORIES": str(source_common / "objects"), } + oldrev = current.stdout.strip() if exists else "0" * len(newrev) + if exists and oldrev == newrev: + raise GateError(f"{refname} is already at {newrev}; nothing to push.") + return governed_operation( + conn, repo, hub_git_dir, pusher, oldrev, newrev, refname, env=env + ) + + +def check_ref_update( + conn: Connection, + repository: str, + git_dir: str, + pusher: str | None, + oldrev: str, + newrev: str, + refname: str, +) -> None: + """Enforce governance for one ref update. Raises PushRejected.""" + repo = _verify_repository_binding(conn, repository, git_dir) + operation = governed_operation( + conn, repo, git_dir, pusher, oldrev, newrev, refname + ) + + if operation is not None: approval = approved_instance(conn, operation) if approval is None: + scope_note = "" + out_of_scope = operation.get("out_of_scope_paths", []) + if out_of_scope: + shown = ", ".join(out_of_scope[:10]) + more = len(out_of_scope) - 10 + if more > 0: + shown += f", … (+{more} more)" + scope_note = ( + f" It changes paths outside the pusher's claimed scopes: " + f"{shown}." + ) + if operation["type"] == "ref_delete": + how = f"--push {refname} --delete" + else: + how = f"--push {refname} --from --rev {newrev}" raise PushRejected( f"{operation['type']} on {refname} requires an approval " f"bound to this exact update (hash {operation_hash(operation)})." + f"{scope_note} Request it with: quorumgit approve request " + f"--repo {repository} {how}" ) assert pusher is not None consume_approval(conn, approval["id"], operation, agent=pusher) diff --git a/src/quorumgit/migrations/003_approver_policy.sql b/src/quorumgit/migrations/003_approver_policy.sql new file mode 100644 index 0000000..db4a291 --- /dev/null +++ b/src/quorumgit/migrations/003_approver_policy.sql @@ -0,0 +1,16 @@ +-- Per-repository approval policy: an optional approver roster whose size sets +-- a 2/3 + 1 quorum, and an optional separation-of-duties rule that stops an +-- agent approving its own request or its own push/takeover. + +CREATE TABLE repository_approvers ( + id INTEGER PRIMARY KEY, + repository_id INTEGER NOT NULL REFERENCES repositories(id) ON DELETE CASCADE, + agent_id INTEGER NOT NULL REFERENCES agents(id), + created_at INTEGER NOT NULL DEFAULT (unixepoch()), + UNIQUE (repository_id, agent_id) +); + +-- quorumgit-statement +ALTER TABLE repositories + ADD COLUMN separate_duties INTEGER NOT NULL DEFAULT 0 + CHECK (separate_duties IN (0, 1)); diff --git a/src/quorumgit/registry.py b/src/quorumgit/registry.py index c135695..f0ae783 100644 --- a/src/quorumgit/registry.py +++ b/src/quorumgit/registry.py @@ -13,6 +13,21 @@ class RegistryError(RuntimeError): pass +def approvers_for_repository(conn: Connection, repository_id: int) -> list[str]: + return [ + row[0] + for row in conn.execute( + """ + SELECT a.name + FROM repository_approvers ra JOIN agents a ON a.id = ra.agent_id + WHERE ra.repository_id = ? + ORDER BY a.name + """, + (repository_id,), + ).fetchall() + ] + + def _protected_refs(conn: Connection, repository_id: int) -> list[str]: return [ row[0] @@ -153,7 +168,7 @@ def add_repository( def get_repository(conn: Connection, name: str) -> dict: row = conn.execute( - "SELECT id, name, path FROM repositories WHERE name = ?", + "SELECT id, name, path, separate_duties FROM repositories WHERE name = ?", (name,), ).fetchone() if row is None: @@ -163,6 +178,8 @@ def get_repository(conn: Connection, name: str) -> dict: "name": row[1], "path": row[2], "protected_refs": _protected_refs(conn, row[0]), + "approvers": approvers_for_repository(conn, row[0]), + "separate_duties": bool(row[3]), } @@ -203,3 +220,73 @@ def get_agent(conn: Connection, name: str) -> dict: def list_agents(conn: Connection) -> list[dict]: rows = conn.execute("SELECT id, name FROM agents ORDER BY name").fetchall() return [{"id": r[0], "name": r[1]} for r in rows] + + +# ------------------------------------------------------------ approval policy + + +def add_approver(conn: Connection, repository: str, agent: str) -> None: + """Add a registered agent to a repository's approver roster.""" + begin_immediate(conn) + repo = get_repository(conn, repository) + member = get_agent(conn, agent) + if agent in repo["approvers"]: + raise RegistryError( + f"Agent {agent} is already an approver for repository {repository}." + ) + conn.execute( + "INSERT INTO repository_approvers (repository_id, agent_id) VALUES (?, ?)", + (repo["id"], member["id"]), + ) + audit.record( + conn, + "repository.approver_added", + "repository", + repo["id"], + agent=agent, + detail={"repository": repository, "approver": agent}, + ) + + +def remove_approver(conn: Connection, repository: str, agent: str) -> None: + """Remove an agent from a repository's approver roster. + + Votes the agent already cast stay on record but stop counting: quorum is + re-evaluated against the current roster whenever an approval is used. + """ + begin_immediate(conn) + repo = get_repository(conn, repository) + member = get_agent(conn, agent) + cur = conn.execute( + "DELETE FROM repository_approvers WHERE repository_id = ? AND agent_id = ?", + (repo["id"], member["id"]), + ) + if cur.rowcount != 1: + raise RegistryError( + f"Agent {agent} is not an approver for repository {repository}." + ) + audit.record( + conn, + "repository.approver_removed", + "repository", + repo["id"], + agent=agent, + detail={"repository": repository, "approver": agent}, + ) + + +def set_separate_duties(conn: Connection, repository: str, enabled: bool) -> None: + """Toggle the rule that an agent may not approve its own operations.""" + begin_immediate(conn) + repo = get_repository(conn, repository) + conn.execute( + "UPDATE repositories SET separate_duties = ? WHERE id = ?", + (1 if enabled else 0, repo["id"]), + ) + audit.record( + conn, + "repository.policy_changed", + "repository", + repo["id"], + detail={"repository": repository, "separate_duties": enabled}, + ) diff --git a/src/quorumgit/work.py b/src/quorumgit/work.py index 8df438b..3a7717a 100644 --- a/src/quorumgit/work.py +++ b/src/quorumgit/work.py @@ -8,6 +8,7 @@ from __future__ import annotations import math +import re import subprocess from typing import Any @@ -318,6 +319,81 @@ def scopes_overlap(a: str, b: str) -> bool: return pa.startswith(pb) or pb.startswith(pa) +def _segment_regex(segment: str) -> str: + """Regex for one path segment: `*` and `?` never cross a `/`.""" + out: list[str] = [] + i = 0 + while i < len(segment): + ch = segment[i] + if ch == "*": + out.append("[^/]*") + elif ch == "?": + out.append("[^/]") + elif ch == "[": + end = segment.find("]", i + 2 if segment[i + 1 : i + 2] in ("!", "]") else i + 1) + if end == -1: + out.append(re.escape(ch)) + else: + body = segment[i + 1 : end] + if body.startswith("!"): + body = "^" + body[1:] + out.append("[" + body.replace("\\", "\\\\") + "]") + i = end + else: + out.append(re.escape(ch)) + i += 1 + return "".join(out) + + +def _scope_regex(glob: str) -> re.Pattern[str]: + parts = glob.split("/") + pieces: list[str] = [] + for index, part in enumerate(parts): + last = index == len(parts) - 1 + if part == "**": + pieces.append(".*" if last else "(?:[^/]+/)*") + else: + pieces.append(_segment_regex(part) + ("" if last else "/")) + return re.compile("".join(pieces)) + + +def scope_matches(glob: str, path: str) -> bool: + """True if a repository-relative path falls inside a declared scope. + + `**` spans any number of directories, `*` and `?` stay within one path + segment, and a glob without wildcards names a file or a whole directory. + """ + glob = glob.strip().removeprefix("./").lstrip("/") + path = path.removeprefix("./").lstrip("/") + if not glob: + return False + if _glob_prefix(glob) == glob: + literal = glob.rstrip("/") + return path == literal or path.startswith(literal + "/") + return _scope_regex(glob).fullmatch(path) is not None + + +def paths_outside_scopes(paths: list[str], scope_globs: list[str]) -> list[str]: + """The sorted subset of paths that no declared scope covers.""" + return sorted( + { + path + for path in paths + if not any(scope_matches(glob, path) for glob in scope_globs) + } + ) + + +def claim_scopes(conn: Connection, claim_id: int) -> list[str]: + return [ + row[0] + for row in conn.execute( + "SELECT path_glob FROM scopes WHERE claim_id = ? ORDER BY id", + (claim_id,), + ).fetchall() + ] + + def verify_commit( repo_path: str, commit_oid: str, branch: str | None = None ) -> None: diff --git a/tests/test_approval_policy.py b/tests/test_approval_policy.py new file mode 100644 index 0000000..54c027f --- /dev/null +++ b/tests/test_approval_policy.py @@ -0,0 +1,267 @@ +"""Approver rosters, 2/3 + 1 quorum, and separation of duties.""" + +from __future__ import annotations + +import os +import subprocess +import sys +import uuid + +import pytest + +from quorumgit import audit, gate, registry +from tests.test_gate import _commit, _push, _setup + + +def _names(*roles: str) -> list[str]: + suffix = uuid.uuid4().hex[:8] + return [f"{role}-{suffix}" for role in roles] + + +def _repo(conn, git_repo, *approvers: str, separate_duties: bool = False) -> str: + name = f"policy-{uuid.uuid4().hex[:8]}" + registry.add_repository(conn, name, git_repo) + for approver in approvers: + registry.add_approver(conn, name, approver) + if separate_duties: + registry.set_separate_duties(conn, name, True) + return name + + +def _agents(conn, *names: str) -> None: + for name in names: + registry.add_agent(conn, name) + + +def _op(repo: str) -> dict: + return {"type": "protected_ref_update", "repository": repo, "n": uuid.uuid4().hex} + + +@pytest.mark.parametrize( + ("members", "threshold"), + [(1, 1), (2, 2), (3, 3), (4, 3), (5, 4), (6, 5), (7, 5), (9, 7)], +) +def test_quorum_threshold_is_two_thirds_plus_one(members, threshold): + assert gate.quorum_threshold(members) == threshold + + +def test_roster_sets_quorum_and_excludes_outsiders(conn, git_repo): + a1, a2, a3, a4, outsider = _names("a1", "a2", "a3", "a4", "outsider") + _agents(conn, a1, a2, a3, a4, outsider) + repo = _repo(conn, git_repo, a1, a2, a3, a4) + + approval = gate.request_approval(conn, _op(repo), requested_by=outsider) + with pytest.raises(gate.GateError, match="not an approver"): + gate.vote(conn, approval["id"], outsider, True) + assert gate.vote(conn, approval["id"], a1, True)["status"] == "pending" + assert gate.vote(conn, approval["id"], a2, True)["status"] == "pending" + assert gate.vote(conn, approval["id"], a3, True)["status"] == "approved" + + +def test_request_threshold_can_raise_but_not_lower_quorum(conn, git_repo): + a1, a2, a3 = _names("a1", "a2", "a3") + _agents(conn, a1, a2, a3) + repo = _repo(conn, git_repo, a1, a2) + + low = gate.request_approval(conn, _op(repo), requested_by=a1, threshold=1) + assert gate.vote(conn, low["id"], a1, True)["status"] == "pending" + assert gate.vote(conn, low["id"], a2, True)["status"] == "approved" + + registry.add_approver(conn, repo, a3) + high = gate.request_approval(conn, _op(repo), requested_by=a1, threshold=3) + gate.vote(conn, high["id"], a1, True) + assert gate.vote(conn, high["id"], a2, True)["status"] == "pending" + assert gate.vote(conn, high["id"], a3, True)["status"] == "approved" + + +def test_removed_approver_vote_stops_counting_at_use(conn, git_repo): + a1, a2, a3, pusher = _names("a1", "a2", "a3", "pusher") + _agents(conn, a1, a2, a3, pusher) + repo = _repo(conn, git_repo, a1, a2) + op = _op(repo) + approval = gate.request_approval(conn, op, requested_by=pusher) + gate.vote(conn, approval["id"], a1, True) + assert gate.vote(conn, approval["id"], a2, True)["status"] == "approved" + + registry.remove_approver(conn, repo, a2) + registry.add_approver(conn, repo, a3) + with pytest.raises(gate.GateError, match="1 eligible yes vote"): + gate.consume_approval(conn, approval["id"], op, agent=pusher) + + # The approval is still live: a current approver can restore quorum. + gate.vote(conn, approval["id"], a3, True) + gate.consume_approval(conn, approval["id"], op, agent=pusher) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_deny_revokes_an_unused_approval(conn, git_repo): + a1, a2 = _names("a1", "a2") + _agents(conn, a1, a2) + repo = _repo(conn, git_repo) + op = _op(repo) + approval = gate.request_approval(conn, op, requested_by=a1) + assert gate.vote(conn, approval["id"], a1, True)["status"] == "approved" + assert gate.vote(conn, approval["id"], a2, False)["status"] == "denied" + with pytest.raises(gate.GateError, match="not consumable"): + gate.consume_approval(conn, approval["id"], op, agent=a1) + + +def test_separate_duties_blocks_requester_vote(conn, git_repo): + requester, other = _names("requester", "other") + _agents(conn, requester, other) + repo = _repo(conn, git_repo, separate_duties=True) + approval = gate.request_approval(conn, _op(repo), requested_by=requester) + with pytest.raises(gate.GateError, match="separation of duties"): + gate.vote(conn, approval["id"], requester, True) + assert gate.vote(conn, approval["id"], other, True)["status"] == "approved" + + +def test_separate_duties_discounts_consumers_own_vote(conn, git_repo): + requester, pusher, other = _names("requester", "pusher", "other") + _agents(conn, requester, pusher, other) + repo = _repo(conn, git_repo, separate_duties=True) + op = _op(repo) + approval = gate.request_approval(conn, op, requested_by=requester) + assert gate.vote(conn, approval["id"], pusher, True)["status"] == "approved" + + with pytest.raises(gate.GateError, match="own vote"): + gate.consume_approval(conn, approval["id"], op, agent=pusher) + # Any agent whose own vote is not the deciding one may use it. + gate.consume_approval(conn, approval["id"], op, agent=other) + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_open_policy_is_unchanged_without_roster(conn, git_repo): + solo = _names("solo")[0] + _agents(conn, solo) + repo = _repo(conn, git_repo) + op = _op(repo) + approval = gate.request_approval(conn, op, requested_by=solo) + assert gate.vote(conn, approval["id"], solo, True)["status"] == "approved" + gate.consume_approval(conn, approval["id"], op, agent=solo) + + +def test_policy_changes_are_audited(conn, git_repo): + member = _names("member")[0] + _agents(conn, member) + repo = _repo(conn, git_repo, member, separate_duties=True) + repo_id = registry.get_repository(conn, repo)["id"] + registry.remove_approver(conn, repo, member) + kinds = [ + e["event_type"] + for e in audit.events(conn, entity="repository", entity_id=repo_id) + ] + assert "repository.approver_added" in kinds + assert "repository.approver_removed" in kinds + assert "repository.policy_changed" in kinds + + +def test_roster_rejects_duplicates_and_unknown_members(conn, git_repo): + member = _names("member")[0] + _agents(conn, member) + repo = _repo(conn, git_repo, member) + with pytest.raises(registry.RegistryError, match="already an approver"): + registry.add_approver(conn, repo, member) + with pytest.raises(registry.RegistryError, match="not registered"): + registry.add_approver(conn, repo, "nobody-" + uuid.uuid4().hex) + registry.remove_approver(conn, repo, member) + with pytest.raises(registry.RegistryError, match="not an approver"): + registry.remove_approver(conn, repo, member) + + +def test_separate_duties_quorum_counts_only_eligible_approvers(conn, git_repo): + """Excluding the requester and consumer never makes quorum impossible.""" + a, b, c, pusher = _names("a", "b", "c", "pusher") + _agents(conn, a, b, c, pusher) + repo = _repo(conn, git_repo, a, b, c, separate_duties=True) + op = _op(repo) + # b requests: a and c are eligible, so quorum is 2 of 2. + approval = gate.request_approval(conn, op, requested_by=b) + assert gate.vote(conn, approval["id"], a, True)["status"] == "pending" + assert gate.vote(conn, approval["id"], c, True)["status"] == "approved" + # a uses it: only c is eligible, c approved, so quorum (1 of 1) holds. + gate.consume_approval(conn, approval["id"], op, agent=a) + + +def test_sole_approver_cannot_approve_own_push(conn, git_repo): + solo = _names("solo")[0] + _agents(conn, solo) + repo = _repo(conn, git_repo, solo, separate_duties=True) + op = _op(repo) + approval = gate.request_approval(conn, op, requested_by=solo) + with pytest.raises(gate.GateError, match="separation of duties"): + gate.vote(conn, approval["id"], solo, True) + + +def test_hub_pusher_cannot_approve_own_push_under_separate_duties( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, hub, clone, a, b = _setup(conn, tmp_path) + registry.set_separate_duties(conn, repo_name, True) + reviewer = _names("reviewer")[0] + _agents(conn, reviewer) + conn.commit() + + oldrev = subprocess.run( + ["git", "--git-dir", str(hub), "rev-parse", "refs/heads/main"], + capture_output=True, text=True, check=True, + ).stdout.strip() + _commit(clone, "release.txt") + newrev = subprocess.run( + ["git", "-C", str(clone), "rev-parse", "HEAD"], + capture_output=True, text=True, check=True, + ).stdout.strip() + op = { + "type": "protected_ref_update", + "repository": repo_name, + "refname": "refs/heads/main", + "oldrev": oldrev, + "newrev": newrev, + } + approval = gate.request_approval(conn, op, requested_by=b) + assert gate.vote(conn, approval["id"], a, True)["status"] == "approved" + conn.commit() + + own = _push(clone, a, "main", cfg=cfg) + assert own.returncode != 0 + assert "own vote" in own.stderr + + gate.vote(conn, approval["id"], reviewer, True) + conn.commit() + reviewed = _push(clone, a, "main", cfg=cfg) + assert reviewed.returncode == 0, reviewed.stderr + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + + +def test_cli_manages_roster_and_policy(committed_conn, git_repo, cfg): + member, other = _names("member", "other") + _agents(committed_conn, member, other) + repo = _repo(committed_conn, git_repo) + committed_conn.commit() + + def cli(*args): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(cfg.data_dir)} + return subprocess.run( + [sys.executable, "-m", "quorumgit", *args], + capture_output=True, text=True, env=env, check=False, + ) + + assert cli("repo", "approver", "add", repo, member).returncode == 0 + assert cli("repo", "approver", "add", repo, other).returncode == 0 + listed = cli("repo", "approver", "list", repo) + assert listed.stdout.split() == sorted([member, other]) + + policy = cli("repo", "policy", repo, "--separate-duties") + assert policy.returncode == 0, policy.stderr + assert "quorum: 2 of 2" in policy.stdout + assert "separate duties: on" in policy.stdout + + assert cli("repo", "approver", "remove", repo, other).returncode == 0 + policy = cli("repo", "policy", repo, "--no-separate-duties") + assert "quorum: 1 of 1" in policy.stdout + assert "separate duties: off" in policy.stdout + + duplicate = cli("repo", "approver", "add", repo, member) + assert duplicate.returncode == 1 + assert "already an approver" in duplicate.stderr diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index 1c38650..6b998cc 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -23,6 +23,9 @@ def _agents(conn, *names): conn.execute("INSERT INTO agents (name) VALUES (?) ON CONFLICT DO NOTHING", (name,)) +UPGRADES_AFTER_001 = ["002_approval_identities.sql", "003_approver_policy.sql"] + + def _apply_001(cfg: Config): conn = store.open_connection(cfg) conn.execute( @@ -186,13 +189,13 @@ def test_contract_rejects_001_only_store_until_all_migrations_apply(tmp_path): conn.commit() conn.close() - message = "Missing required migrations: ['002_approval_identities.sql']" + message = f"Missing required migrations: {UPGRADES_AFTER_001}" with pytest.raises(store.ContractViolation, match=re.escape(message)): store.verify_contract(cfg) with pytest.raises(store.ContractViolation, match=re.escape(message)): store.connect(cfg) - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADES_AFTER_001 store.verify_contract(cfg) upgraded = store.connect(cfg) upgraded.close() @@ -209,7 +212,7 @@ def test_identity_migration_preserves_and_backfills_history(tmp_path): conn.commit() conn.close() - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADES_AFTER_001 migrated = store.connect(cfg) try: requester = migrated.execute( @@ -249,7 +252,7 @@ def test_migration_does_not_backfill_requester_registered_after_request(tmp_path conn.commit() conn.close() - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADES_AFTER_001 migrated = store.connect(cfg) try: row = migrated.execute( @@ -281,7 +284,7 @@ def test_migration_does_not_backfill_voter_registered_after_vote(tmp_path): conn.commit() conn.close() - assert store.migrate(cfg) == ["002_approval_identities.sql"] + assert store.migrate(cfg) == UPGRADES_AFTER_001 migrated = store.connect(cfg) try: approval = migrated.execute( diff --git a/tests/test_push_request.py b/tests/test_push_request.py new file mode 100644 index 0000000..295f9a8 --- /dev/null +++ b/tests/test_push_request.py @@ -0,0 +1,202 @@ +"""`approve request --push` derives the exact operation the hook requires.""" + +from __future__ import annotations + +import os +import re +import subprocess +import sys + +import pytest + +from quorumgit import gate, work +from tests.test_gate import _commit, _push, _register_agents, _setup + +GIT_ENV = { + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@localhost", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@localhost", +} + + +def _git(repo, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), *args], + check=True, capture_output=True, text=True, env=GIT_ENV, + ).stdout.strip() + + +def _hub_has(hub, oid: str) -> bool: + return subprocess.run( + ["git", "--git-dir", str(hub), "cat-file", "-e", f"{oid}^{{commit}}"], + capture_output=True, check=False, + ).returncode == 0 + + +def _approve(conn, operation) -> int: + _register_agents(conn, "operator") + approval = gate.request_approval(conn, operation, requested_by="operator") + gate.vote(conn, approval["id"], "operator", True) + conn.commit() + return approval["id"] + + +def _cli(cfg, *args, agent=None): + env = {**os.environ, "QUORUMGIT_DATA_DIR": str(cfg.data_dir)} + env.pop("QUORUMGIT_AGENT", None) + if agent: + env["QUORUMGIT_AGENT"] = agent + return subprocess.run( + [sys.executable, "-m", "quorumgit", *args], + capture_output=True, text=True, env=env, check=False, + ) + + +def test_derived_protected_push_matches_hook_and_lands(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "release.txt") + newrev = _git(clone, "rev-parse", "HEAD") + + derived = gate.derive_push_operation(conn, repo_name, "main", a, source=clone) + assert derived is not None + assert derived == { + "type": "protected_ref_update", + "repository": repo_name, + "refname": "refs/heads/main", + "oldrev": _git(clone, "rev-parse", "origin/main"), + "newrev": newrev, + } + assert not _hub_has(hub, newrev), "derivation must not write to the hub" + + rejected = _push(clone, a, "main", cfg=cfg) + assert rejected.returncode != 0 + assert gate.operation_hash(derived) in rejected.stderr + assert "approve request" in rejected.stderr + + _approve(conn, derived) + assert _push(clone, a, "main", cfg=cfg).returncode == 0 + + +def test_derived_out_of_scope_push_matches_hook(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, "src/ok.py", branch="feat/scoped") + _commit(clone, "docs/stray.md") + + derived = gate.derive_push_operation( + conn, repo_name, "feat/scoped", a, source=clone + ) + assert derived is not None + assert derived["type"] == "out_of_scope_update" + assert derived["out_of_scope_paths"] == ["docs/stray.md"] + assert derived["oldrev"] == "0" * 40 + + rejected = _push(clone, a, "feat/scoped", cfg=cfg) + assert gate.operation_hash(derived) in rejected.stderr + _approve(conn, derived) + assert _push(clone, a, "feat/scoped", cfg=cfg).returncode == 0 + + +def test_derived_force_push_and_delete(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "one.txt", branch="topic") + assert _push(clone, a, "topic", cfg=cfg).returncode == 0 + published = _git(clone, "rev-parse", "HEAD") + + _git(clone, "reset", "--hard", "HEAD~1") + _commit(clone, "rewritten.txt") + forced = gate.derive_push_operation(conn, repo_name, "topic", a, source=clone) + assert forced is not None + assert forced["type"] == "force_update" + assert forced["oldrev"] == published + _approve(conn, forced) + assert _push(clone, a, "+topic", cfg=cfg).returncode == 0 + + deletion = gate.derive_push_operation(conn, repo_name, "topic", a, delete=True) + assert deletion is not None + assert deletion["type"] == "ref_delete" + assert deletion["newrev"] == "0" * 40 + assert deletion["oldrev"] == _git(clone, "rev-parse", "HEAD") + _approve(conn, deletion) + assert _push(clone, a, ":topic", cfg=cfg).returncode == 0 + assert subprocess.run( + ["git", "--git-dir", str(hub), "rev-parse", "--verify", "refs/heads/topic"], + capture_output=True, check=False, + ).returncode != 0 + + +def test_ungoverned_push_derives_nothing(committed_conn, tmp_path): + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + _commit(clone, "free.txt", branch="free") + assert gate.derive_push_operation(conn, repo_name, "free", a, source=clone) is None + + +def test_derivation_enforces_branch_reservations(committed_conn, tmp_path): + conn = committed_conn + repo_name, _hub, clone, a, b = _setup(conn, tmp_path) + task = work.create_task(conn, repo_name, "owned") + work.claim_task(conn, task, a, branch="feat/owned", scope_globs=["src/**"]) + conn.commit() + _commit(clone, "src/x.py", branch="feat/owned") + with pytest.raises(gate.PushRejected, match="claimed by"): + gate.derive_push_operation(conn, repo_name, "feat/owned", b, source=clone) + + +def test_derivation_rejects_bad_inputs(committed_conn, tmp_path): + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + with pytest.raises(gate.GateError, match="nothing to push"): + gate.derive_push_operation(conn, repo_name, "main", a, source=clone) + with pytest.raises(gate.GateError, match="Cannot resolve"): + gate.derive_push_operation( + conn, repo_name, "main", a, source=clone, rev="no-such-rev" + ) + with pytest.raises(gate.GateError, match="nothing to delete"): + gate.derive_push_operation(conn, repo_name, "absent", a, delete=True) + + +def test_cli_push_request_end_to_end(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + _register_agents(conn, "operator") + conn.commit() + _commit(clone, "cli.txt") + + requested = _cli( + cfg, "approve", "request", "--repo", repo_name, "--push", "main", + "--from", str(clone), "--pusher", a, agent="operator", + ) + assert requested.returncode == 0, requested.stderr + assert '"type": "protected_ref_update"' in requested.stdout + match = re.search(r"approval (\d+) hash=", requested.stdout) + assert match + voted = _cli(cfg, "approve", "vote", match.group(1), agent="operator") + assert "status=approved" in voted.stdout + + assert _push(clone, a, "main", cfg=cfg).returncode == 0 + + _commit(clone, "free.txt", branch="free") + nothing = _cli( + cfg, "approve", "request", "--repo", repo_name, "--push", "free", + "--from", str(clone), agent=a, + ) + assert nothing.returncode == 0 + assert "needs no approval" in nothing.stdout + + +def test_cli_push_request_usage_errors(cfg): + both = _cli(cfg, "approve", "request", "{}", "--push", "main", agent="x") + assert both.returncode == 2 + neither = _cli(cfg, "approve", "request", agent="x") + assert neither.returncode == 2 + no_repo = _cli(cfg, "approve", "request", "--push", "main", "--from", ".", + agent="x") + assert no_repo.returncode == 2 diff --git a/tests/test_scope_enforcement.py b/tests/test_scope_enforcement.py new file mode 100644 index 0000000..1c3f035 --- /dev/null +++ b/tests/test_scope_enforcement.py @@ -0,0 +1,198 @@ +"""Claim scopes are enforced on what a push actually changes.""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +from quorumgit import gate, work +from tests.test_gate import _commit, _push, _register_agents, _setup + +GIT_ENV = { + **os.environ, + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@localhost", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@localhost", +} + + +def _git(clone: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(clone), *args], + check=True, + capture_output=True, + text=True, + env=GIT_ENV, + ).stdout.strip() + + +def _claim(conn, repo_name, agent, branch, *scopes): + task = work.create_task(conn, repo_name, f"work on {branch}") + work.claim_task(conn, task, agent, branch=branch, scope_globs=list(scopes)) + conn.commit() + + +@pytest.mark.parametrize( + ("glob", "path", "expected"), + [ + ("src/**", "src/app.py", True), + ("src/**", "src/deep/nested/mod.py", True), + ("src/**", "srcfoo/app.py", False), + ("src/**", "docs/guide.md", False), + ("src/*.py", "src/app.py", True), + ("src/*.py", "src/pkg/app.py", False), + ("**/*.md", "README.md", True), + ("**/*.md", "docs/deep/guide.md", True), + ("src/**/test_*.py", "src/test_x.py", True), + ("src/**/test_*.py", "src/a/b/test_x.py", True), + ("src/**/test_*.py", "src/a/b/x.py", False), + ("docs", "docs/guide.md", True), + ("docs/", "docs/guide.md", True), + ("docs", "docs", True), + ("docs", "docsite/index.md", False), + ("README.md", "README.md", True), + ("./src/**", "src/app.py", True), + ("src/[ab].py", "src/a.py", True), + ("src/[!ab].py", "src/a.py", False), + ("src/?.py", "src/a.py", True), + ("src/?.py", "src/ab.py", False), + ("**", "anything/at/all", True), + ], +) +def test_scope_matches(glob, path, expected): + assert work.scope_matches(glob, path) is expected + + +def test_paths_outside_scopes_is_sorted_and_unique(): + assert work.paths_outside_scopes( + ["docs/b.md", "src/a.py", "docs/a.md", "docs/b.md"], ["src/**"] + ) == ["docs/a.md", "docs/b.md"] + + +def test_in_scope_push_is_accepted(committed_conn, tmp_path, cfg): + repo_name, _hub, clone, a, _b = _setup(committed_conn, tmp_path) + _claim(committed_conn, repo_name, a, "feat/in", "src/**") + _commit(clone, "src/in_scope.py", branch="feat/in") + result = _push(clone, a, "feat/in", cfg=cfg) + assert result.returncode == 0, result.stderr + + +def test_out_of_scope_push_requires_exact_approval(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, hub, clone, a, _b = _setup(conn, tmp_path) + _claim(conn, repo_name, a, "feat/out", "src/**") + _commit(clone, "src/ok.py", branch="feat/out") + _commit(clone, "docs/stray.md") + + rejected = _push(clone, a, "feat/out", cfg=cfg) + assert rejected.returncode != 0 + assert "out_of_scope_update" in rejected.stderr + assert "docs/stray.md" in rejected.stderr + assert "src/ok.py" not in rejected.stderr + + op = { + "type": "out_of_scope_update", + "repository": repo_name, + "refname": "refs/heads/feat/out", + "oldrev": "0" * 40, + "newrev": _git(clone, "rev-parse", "HEAD"), + "out_of_scope_paths": ["docs/stray.md"], + } + assert gate.operation_hash(op) in rejected.stderr + _register_agents(conn, "operator") + approval = gate.request_approval(conn, op, requested_by="operator") + gate.vote(conn, approval["id"], "operator", True) + conn.commit() + + accepted = _push(clone, a, "feat/out", cfg=cfg) + assert accepted.returncode == 0, accepted.stderr + assert gate.get_approval_by_id(conn, approval["id"])["status"] == "consumed" + hub_tip = subprocess.run( + ["git", "--git-dir", str(hub), "rev-parse", "refs/heads/feat/out"], + capture_output=True, + text=True, + check=True, + ).stdout.strip() + assert hub_tip == op["newrev"] + + +def test_follow_up_push_is_checked_against_its_own_commits( + committed_conn, tmp_path, cfg +): + repo_name, _hub, clone, a, _b = _setup(committed_conn, tmp_path) + _claim(committed_conn, repo_name, a, "feat/next", "src/**") + _commit(clone, "src/first.py", branch="feat/next") + assert _push(clone, a, "feat/next", cfg=cfg).returncode == 0 + + _commit(clone, "src/second.py") + assert _push(clone, a, "feat/next", cfg=cfg).returncode == 0 + + _commit(clone, "README.md") + stray = _push(clone, a, "feat/next", cfg=cfg) + assert stray.returncode != 0 + assert "README.md" in stray.stderr + + +def test_rename_out_of_scope_is_detected(committed_conn, tmp_path, cfg): + """Moving a file into scope still deletes it from outside the scope.""" + repo_name, _hub, clone, a, _b = _setup(committed_conn, tmp_path) + _claim(committed_conn, repo_name, a, "feat/move", "src/**") + _git(clone, "checkout", "-B", "feat/move") + _git(clone, "mv", "docs/guide.md", "src/guide.md") + _git(clone, "commit", "-m", "move guide into src") + + result = _push(clone, a, "feat/move", cfg=cfg) + assert result.returncode != 0 + assert "docs/guide.md" in result.stderr + + +def test_merging_published_history_is_not_charged(committed_conn, tmp_path, cfg): + """Bringing in main's already-pushed changes is not an out-of-scope edit.""" + conn = committed_conn + repo_name, _hub, clone, a, b = _setup(conn, tmp_path) + _claim(conn, repo_name, a, "feat/merge", "src/**") + _commit(clone, "src/feature.py", branch="feat/merge") + assert _push(clone, a, "feat/merge", cfg=cfg).returncode == 0 + + # Another agent publishes docs work on an unclaimed branch. + _git(clone, "checkout", "-B", "docs-work", "main") + _commit(clone, "docs/published.md") + assert _push(clone, b, "docs-work", cfg=cfg).returncode == 0 + + _git(clone, "checkout", "feat/merge") + _git(clone, "merge", "--no-edit", "docs-work") + result = _push(clone, a, "feat/merge", cfg=cfg) + assert result.returncode == 0, result.stderr + + +def test_edits_hidden_in_a_merge_commit_are_charged(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, _hub, clone, a, b = _setup(conn, tmp_path) + _claim(conn, repo_name, a, "feat/evil", "src/**") + _commit(clone, "src/feature.py", branch="feat/evil") + assert _push(clone, a, "feat/evil", cfg=cfg).returncode == 0 + + _git(clone, "checkout", "-B", "docs-work", "main") + _commit(clone, "docs/published.md") + assert _push(clone, b, "docs-work", cfg=cfg).returncode == 0 + + _git(clone, "checkout", "feat/evil") + _git(clone, "merge", "--no-commit", "--no-ff", "docs-work") + (clone / "docs" / "sneaky.md").write_text("added inside the merge\n") + _git(clone, "add", "-A") + _git(clone, "commit", "--no-edit") + result = _push(clone, a, "feat/evil", cfg=cfg) + assert result.returncode != 0 + assert "docs/sneaky.md" in result.stderr + assert "docs/published.md" not in result.stderr + + +def test_unclaimed_branch_is_not_scope_checked(committed_conn, tmp_path, cfg): + _repo_name, _hub, clone, _a, b = _setup(committed_conn, tmp_path) + _commit(clone, "docs/anything.md", branch="free") + result = _push(clone, b, "free", cfg=cfg) + assert result.returncode == 0, result.stderr