diff --git a/README.md b/README.md index 426c606..2201808 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,9 @@ push → identify pusher → branch reservations → allowed ref namespace → ALLOW, or REQUIRE the matching approval(s) ``` -- **Changed paths** are the paths introduced by commits that are new to the repository — reachable from the pushed revision and from no existing ref. Content already in the hub was governed when it first arrived, so merging `main` into a feature branch does not attribute `main`'s changes to the pusher; a merge contributes only the paths it changes relative to *every* parent (conflict resolutions and hand edits). Root commits are compared with the empty tree. Deletions carry no content. +- **Changed paths** are the paths introduced by the commits a push brings in; a merge contributes only the paths it changes relative to *every* parent (conflict resolutions and hand edits), root commits are compared with the empty tree, and deletions carry no content. Which commits count as "brought in" depends on the check: + - For **protected paths**, commits reachable from no existing ref. Every push to every ref is checked against protected paths, so content already in the hub was governed when it first arrived. + - For a **claim's scopes**, commits not reachable from the branch's previous value, the hub's default branch (`HEAD`), or a protected ref. Merging `main` into a feature branch therefore does not attribute `main`'s changes to the pusher, but merging another agent's branch does, and so does pushing a commit to an unclaimed branch first and then to the claimed one: unclaimed branches carry no scopes, so their content is not treated as already governed. - **Out-of-scope changes need an approval, not a new claim.** When a claimed branch receives changes to paths none of its scopes cover, the push requires an `out_of_scope_push` approval naming the claim and exactly those paths. Scope globs use `**` (any number of directories), `*` and `?` (within one path segment), and `[...]`; a glob with no wildcard, or ending in `/`, covers a file or everything beneath a directory. Branches with no live claim carry no scopes, so only the other checks apply to them. - **Protected paths** need a `protected_path_update` approval whenever a push changes them, on any ref and whoever owns the branch: @@ -345,7 +347,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 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. +- A claim's scopes are measured against the mainline: the default branch and the protected refs. Content that reached the mainline is not charged to a claim that later merges it, so if the default branch is not protected, a change pushed there directly (subject to protected paths) is never checked against any claim. Protect the default branch. - 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/gate.py b/src/quorumgit/gate.py index 781f22e..0722984 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -501,11 +501,15 @@ def governed_operations( newrev: str, git_dir: str | Path, paths: list[str], + scope_paths: list[str], ) -> list[dict]: """Every approval one ref update needs, derived from what it carries. - ``paths`` are the paths the update brings in (git_objects.changed_paths). - Ref-level governance (protected ref, force push, deletion) comes first, + ``paths`` are the paths the update brings into the repository, measured + against every existing ref; they are checked against protected paths. + ``scope_paths`` are measured against the mainline only + (git_objects.mainline_tips), so work first pushed to another branch still + counts against the claim that brings it in. Ref-level governance (protected ref, force push, deletion) comes first, then content governance: paths outside the scopes of the claim on the branch, and paths under the repository's protected paths. Each operation binds the exact revisions, so its hash is reproducible by anyone who can @@ -531,11 +535,11 @@ def governed_operations( elif forced: operations.append({"type": "force_update", **base}) - if refname.startswith("refs/heads/") and paths: + if refname.startswith("refs/heads/") and scope_paths: branch = refname.removeprefix("refs/heads/") claim = live_claim_for_branch(conn, repo["id"], branch) if claim is not None: - outside = paths_outside(paths, claim_scopes(conn, claim["id"])) + outside = paths_outside(scope_paths, claim_scopes(conn, claim["id"])) if outside: operations.append({ "type": "out_of_scope_push", @@ -582,15 +586,17 @@ def evaluate_ref_update( newrev: str, refname: str, paths: list[str] | None = None, -) -> tuple[dict, list[tuple[dict, dict]], list[str]]: + scope_paths: list[str] | None = None, +) -> tuple[dict, list[tuple[dict, dict]], list[str], 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. + Returns (repository, [(approval, operation)] the update needs, paths, + scope_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 both path lists + 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 or moved the mainline. """ repo = _verify_repository_binding(conn, repository, git_dir) refusal = ref_namespace_refusal(repo, refname) @@ -618,8 +624,15 @@ def evaluate_ref_update( paths = git_objects.changed_paths( git_dir, oldrev, newrev, git_objects.ref_tips(git_dir) ) + if scope_paths is None: + scope_paths = git_objects.changed_paths( + git_dir, + oldrev, + newrev, + git_objects.mainline_tips(git_dir, repo["protected_refs"]), + ) operations = governed_operations( - conn, repo, refname, oldrev, newrev, git_dir, paths + conn, repo, refname, oldrev, newrev, git_dir, paths, scope_paths ) except git_objects.GitObjectError as exc: raise PushRejected(f"Unable to inspect pushed objects: {exc}") from exc @@ -641,7 +654,7 @@ def evaluate_ref_update( "and request every approval this push needs." ) raise PushRejected(" ".join(missing)) - return repo, granted, paths + return repo, granted, paths, scope_paths def check_ref_update( @@ -659,7 +672,7 @@ def check_ref_update( 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( + repo, granted, paths, scope_paths = evaluate_ref_update( conn, repository, git_dir, pusher, oldrev, newrev, refname ) operations = [operation for _, operation in granted] @@ -667,9 +680,9 @@ def check_ref_update( """ INSERT INTO ref_updates ( repository_id, refname, oldrev, newrev, pusher_agent_id, - paths, operations + paths, scope_paths, operations ) - VALUES (?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) RETURNING id """, ( @@ -679,6 +692,7 @@ def check_ref_update( newrev, get_agent(conn, pusher)["id"], json_dumps(paths), + json_dumps(scope_paths), json_dumps(operations), ), ).fetchone() @@ -755,6 +769,13 @@ def prepare_push( "matches what the hook will see." ) paths = git_objects.changed_paths(local_dir, oldrev, newrev, known) + # Mainline tips are hub ref tips, so the check above covers them too. + scope_paths = git_objects.changed_paths( + local_dir, + oldrev, + newrev, + git_objects.mainline_tips(hub_dir, repo["protected_refs"]), + ) refusals: list[str] = [] namespace = ref_namespace_refusal(repo, refname) @@ -777,7 +798,7 @@ def prepare_push( f"(claim {claim['id']}), not {pusher}." ) operations = governed_operations( - conn, repo, refname, oldrev, newrev, local_dir, paths + conn, repo, refname, oldrev, newrev, local_dir, paths, scope_paths ) return { "repository": repo["name"], @@ -875,7 +896,7 @@ def _find_update( """ row = conn.execute( """ - SELECT u.id, u.oldrev, a.name, u.paths, u.approval_ids + SELECT u.id, u.oldrev, a.name, u.paths, u.approval_ids, u.scope_paths 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 ?) @@ -901,6 +922,8 @@ def _find_update( "pusher": row[2], "paths": json_loads(row[3], []), "approval_ids": json_loads(row[4], []), + # NULL for an update validated before migration 007: recompute. + "scope_paths": json_loads(row[5], None), } @@ -959,7 +982,7 @@ def _prepare_updates( f"Update of {refname} was validated for {found['pusher']}, " f"not {pusher or 'an unidentified pusher'}." ) - _, granted, _ = evaluate_ref_update( + _, granted, _, _ = evaluate_ref_update( conn, repository, git_dir, @@ -968,6 +991,7 @@ def _prepare_updates( newrev, refname, paths=found["paths"], + scope_paths=found["scope_paths"], ) assert pusher is not None for approval, operation in granted: diff --git a/src/quorumgit/git_objects.py b/src/quorumgit/git_objects.py index 53eea4f..afaf8a6 100644 --- a/src/quorumgit/git_objects.py +++ b/src/quorumgit/git_objects.py @@ -158,6 +158,33 @@ def ref_tips(git_dir: str | Path) -> list[str]: return sorted(tips) +def _commit_at(git_dir: str | Path, ref: str) -> str | None: + """The commit a ref names (tags peeled), or None if it names none.""" + _refuse_option(ref) + result = subprocess.run( + ["git", "--git-dir", str(git_dir), "rev-parse", "--verify", "--quiet", + f"{ref}^{{commit}}"], + capture_output=True, + text=True, + check=False, + ) + oid = result.stdout.strip() + return oid if result.returncode == 0 and oid else None + + +def mainline_tips(git_dir: str | Path, protected_refs: list[str]) -> list[str]: + """Commits the default branch (HEAD) and the protected refs point at. + + This is the shared baseline a claimed branch is measured against. Other + branches are someone's work in progress, and nothing checks the scopes of + an unclaimed one, so content taken from them counts against the claim that + brings it in. A ref that is missing or names no commit is skipped, which + only shrinks the baseline and makes the scope check stricter. + """ + tips = {oid for ref in ("HEAD", *protected_refs) if (oid := _commit_at(git_dir, ref))} + return sorted(tips) + + def new_commits( git_dir: str | Path, newrev: str, known_tips: list[str] ) -> list[str]: @@ -185,12 +212,14 @@ def commit_paths(git_dir: str | Path, commit: str) -> list[str]: def changed_paths( git_dir: str | Path, oldrev: str, newrev: str, known_tips: list[str] ) -> list[str]: - """Paths this update brings into the repository, sorted and de-duplicated. - - Content already reachable from an existing ref was governed when it first - arrived, so only commits new to the repository are inspected. Merging - another branch in therefore does not re-attribute that branch's changes - to the pusher. A deletion carries no content. + """Paths the commits reachable from newrev but not from oldrev or + known_tips introduce, sorted and de-duplicated. + + What counts as already known depends on the check. Protected paths are + checked on every push to every ref, so content reachable from any + existing ref (ref_tips) was governed when it arrived. Claim scopes are + checked only on claimed branches, so they are measured against the + mainline alone (mainline_tips). A deletion carries no content. """ if is_zero(newrev): return [] diff --git a/src/quorumgit/migrations/007_ref_update_scope_paths.sql b/src/quorumgit/migrations/007_ref_update_scope_paths.sql new file mode 100644 index 0000000..8291bce --- /dev/null +++ b/src/quorumgit/migrations/007_ref_update_scope_paths.sql @@ -0,0 +1,10 @@ +-- Record the claim-scope path list alongside the protected-path list. +-- Claim scopes are measured against the mainline (the default branch and the +-- protected refs), protected paths against every ref, so a validated update +-- carries both and the reference-transaction `prepared` stage replays both. +-- NULL marks an update validated before this migration; it is recomputed. + +ALTER TABLE ref_updates ADD COLUMN scope_paths TEXT CHECK ( + scope_paths IS NULL + OR (json_valid(scope_paths) AND json_type(scope_paths) = 'array') +); diff --git a/tests/test_approval_authority.py b/tests/test_approval_authority.py index a5d49dc..cb4692d 100644 --- a/tests/test_approval_authority.py +++ b/tests/test_approval_authority.py @@ -363,6 +363,7 @@ def test_upgrade_leaves_no_agent_with_authority(tmp_path): "004_approval_authority.sql", "005_content_governance.sql", "006_ref_updates.sql", + "007_ref_update_scope_paths.sql", ] upgraded = store.connect(local) try: diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index 10ede0e..3577252 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -41,6 +41,7 @@ def _apply_001(cfg: Config): "004_approval_authority.sql", "005_content_governance.sql", "006_ref_updates.sql", + "007_ref_update_scope_paths.sql", ] @@ -209,7 +210,7 @@ def test_contract_rejects_001_only_store_until_all_migrations_apply(tmp_path): "Missing required migrations: " "['002_approval_identities.sql', '004_approval_authority.sql', " "'005_content_governance.sql', " - "'006_ref_updates.sql']" + "'006_ref_updates.sql', '007_ref_update_scope_paths.sql']" ) with pytest.raises(store.ContractViolation, match=re.escape(message)): store.verify_contract(cfg) diff --git a/tests/test_content_governance.py b/tests/test_content_governance.py index 099dc25..d85d059 100644 --- a/tests/test_content_governance.py +++ b/tests/test_content_governance.py @@ -191,6 +191,67 @@ def test_merging_mainline_into_a_claimed_branch_stays_in_scope( assert merged.returncode == 0, merged.stderr +def test_staging_work_on_another_branch_does_not_escape_the_claim( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + _claimed_branch(conn, repo_name, a, "feat/docs", ["docs/**"]) + _git(clone, "checkout", "-b", "feat/docs") + _edit(clone, "src/auth.py", message="stray") + + direct = _push(clone, a, "feat/docs", cfg=cfg) + assert direct.returncode != 0 + assert "out_of_scope_push" in direct.stderr + + # Unclaimed branches carry no scopes, so this is accepted... + staged = _push(clone, a, "HEAD:refs/heads/scratch", cfg=cfg) + assert staged.returncode == 0, staged.stderr + # ...but the same commit still counts when it reaches the claimed branch. + plan = gate.prepare_push(conn, repo_name, "feat/docs", clone, pusher=a) + required = _required(plan) + assert set(required) == {"out_of_scope_push"} + assert required["out_of_scope_push"]["operation"]["paths"] == ["src/auth.py"] + bypass = _push(clone, a, "feat/docs", cfg=cfg) + assert bypass.returncode != 0 + assert "src/auth.py" in bypass.stderr + # The planner derives exactly the hash the hook demanded. + assert required["out_of_scope_push"]["hash"] in bypass.stderr + + +def test_new_claimed_branch_is_measured_from_the_default_branch( + committed_conn, tmp_path, cfg +): + """Without protected refs, the hub's default branch is still the mainline, + so a new claimed branch is charged only for its own commits.""" + conn = committed_conn + suffix = uuid.uuid4().hex[:8] + hub = tmp_path / "open-hub.git" + seed = tmp_path / "open-seed" + seed.mkdir() + _git(seed, "init", "-b", "main") + _edit(seed, "infra/prod.tf", "README.md", message="history outside the scope") + _git(tmp_path, "clone", "--bare", str(seed), str(hub)) + repo_name = f"open-{suffix}" + registry.add_repository(conn, repo_name, hub) + agent = f"agent-{suffix}" + registry.add_agent(conn, agent) + gate.install_hook(conn, repo_name) + conn.commit() + assert registry.get_repository(conn, repo_name)["protected_refs"] == [] + + _claimed_branch(conn, repo_name, agent, "feat/api", ["src/api/**"]) + clone = tmp_path / "open-clone" + _git(tmp_path, "clone", str(hub), str(clone)) + _git(clone, "checkout", "-b", "feat/api") + _edit(clone, "src/api/routes.py", message="feature") + + plan = gate.prepare_push(conn, repo_name, "feat/api", clone, pusher=agent) + assert plan["operations"] == [] + pushed = _push(clone, agent, "feat/api", cfg=cfg) + assert pushed.returncode == 0, pushed.stderr + + # ----------------------------------------------------------- protected paths diff --git a/tests/test_scope_enforcement.py b/tests/test_scope_enforcement.py index e8d77af..b542fed 100644 --- a/tests/test_scope_enforcement.py +++ b/tests/test_scope_enforcement.py @@ -146,8 +146,8 @@ def test_hook_holds_claimed_branch_to_its_scopes(committed_conn, tmp_path, cfg): assert "outside claim" in outside.stderr assert "README.md" in outside.stderr - # Undo the out-of-scope commit; merging another branch's accepted work in - # does not attribute that branch's files to the claim holder. + # Undo the out-of-scope commit. Another agent's branch is not mainline: + # merging its work in brings its files into this claim, so they count. _git(clone, "reset", "--hard", "HEAD~1") _commit(clone, "docs/elsewhere.md", branch="feat/base") base = _push(clone, b, "feat/base", cfg=cfg) @@ -155,7 +155,9 @@ def test_hook_holds_claimed_branch_to_its_scopes(committed_conn, tmp_path, cfg): _git(clone, "checkout", "feat/scoped") _git(clone, "merge", "--no-edit", "feat/base") merged = _push(clone, a, "feat/scoped", cfg=cfg) - assert merged.returncode == 0, merged.stderr + assert merged.returncode != 0 + assert "out_of_scope_push" in merged.stderr + assert "docs/elsewhere.md" in merged.stderr def test_hook_scope_check_covers_new_claimed_branches(committed_conn, tmp_path, cfg):