From 523f700bcda0c159a0949e5ebbbb192ab9a5f96e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 29 Sep 2026 13:28:23 +0000 Subject: [PATCH] Enforce claim scopes at push time Scopes were only compared against other claims when a claim was granted; the hook never looked at what a push actually changed, so an agent could claim src/** and push edits anywhere. The pre-receive hook now lists the paths changed by the commits a push introduces (rev-list newrev --not --all, combined diff for merges, renames split) and requires an approval bound to the exact update, including the sorted out_of_scope_paths, when the owner of a claimed branch touches anything outside its scopes. This restores the content-aware check the predecessor Codex_Kernel gate performed on incoming objects, applied to QuorumGit's claim scopes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Mc44WsAZx9hzZdWEihDk5a --- README.md | 6 +- src/quorumgit/gate.py | 86 ++++++++++++-- src/quorumgit/work.py | 76 ++++++++++++ tests/test_scope_enforcement.py | 198 ++++++++++++++++++++++++++++++++ 4 files changed, 358 insertions(+), 8 deletions(-) create mode 100644 tests/test_scope_enforcement.py diff --git a/README.md b/README.md index 0f20f3e..6f1b66b 100644 --- a/README.md +++ b/README.md @@ -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: diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index 948d99d..b0ff661 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -15,7 +15,7 @@ from pathlib import Path from typing import Any -from . import audit +from . import audit, work from .canonical import stable_hash from .registry import ( RegistryError, @@ -317,6 +317,51 @@ def _is_fast_forward(git_dir: str, oldrev: str, newrev: str) -> bool: return result.returncode == 0 +def _new_commit_paths(git_dir: str, newrev: str) -> 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, + ) + 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, + ) + 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"], @@ -365,6 +410,8 @@ def check_ref_update( """Enforce governance for one ref update. Raises PushRejected.""" repo = _verify_repository_binding(conn, repository, git_dir) 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 +427,55 @@ 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), + 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) ) - if protected or deletion or forced: - operation = { - "type": "protected_ref_update" - if protected - else ("ref_delete" if deletion else "force_update"), + if protected or deletion or forced or out_of_scope: + 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 approval = approved_instance(conn, operation) if approval is None: + scope_note = "" + 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}." + ) raise PushRejected( f"{operation['type']} on {refname} requires an approval " f"bound to this exact update (hash {operation_hash(operation)})." + f"{scope_note}" ) assert pusher is not None consume_approval(conn, approval["id"], operation, agent=pusher) 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_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