diff --git a/README.md b/README.md index 3b7f1d9..4a49d5e 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Then initialize the store once: ```bash $ quorumgit init store: /home/you/.quorumgit/quorumgit.db -migrations applied: ['001_core.sql', '002_approval_identities.sql', '004_approval_authority.sql', '005_content_governance.sql', '006_ref_updates.sql', '007_ref_update_scope_paths.sql', '008_operation_policies.sql', '009_approval_quorum.sql', '010_protected_ref_names.sql', '011_quorum_consumer_separation.sql'] +migrations applied: ['001_core.sql', '002_approval_identities.sql', '004_approval_authority.sql', '005_content_governance.sql', '006_ref_updates.sql', '007_ref_update_scope_paths.sql', '008_operation_policies.sql', '009_approval_quorum.sql', '010_protected_ref_names.sql', '011_quorum_consumer_separation.sql', '012_protected_fields.sql'] contract: ok ``` @@ -202,6 +202,15 @@ push → identify pusher → branch reservations → allowed ref namespace quorumgit repo protect-path myproject 'migrations/**' --agent lead quorumgit repo add other /path/to/other.git --protected-path 'infrastructure/**' ``` +- **Protected fields** (structured rules) govern one value inside JSON or TOML files rather than the whole file. A push needs a `protected_field_update` approval only when it changes that value; other edits to the same file are ordinary content. The value is named by a [JSON Pointer](https://www.rfc-editor.org/rfc/rfc6901) (`''` for the whole document), and the format comes from the glob's extension unless `--format` says otherwise: + + ```bash + quorumgit repo protect-field myproject config/state.json /max_depth --agent lead + quorumgit repo protect-field myproject 'services/*/settings.toml' /limits/replicas --agent lead + quorumgit repo protect-field myproject config/state /schema_version --format json --agent lead + ``` + + Values are compared per new commit, like paths: a commit changes a value when it differs from the value in every parent, so a clean merge changes nothing and a change reverted later in the same push still counts. Values compare by JSON type and content, so `1`, `1.0`, and `true` differ while reordered object keys do not. A value appearing or disappearing is a change, and content that cannot be parsed or read counts as a change (rules fail closed). - **Allowed ref namespaces.** Every repository accepts pushes to `refs/heads/` only. Tags, notes, and custom refs are refused until an operator opts in: `quorumgit repo allow-ref myproject refs/tags/ --agent lead` (`--remove` withdraws it). Branch claims, reservations, and scopes apply to `refs/heads/`; protected refs and protected paths apply to every allowed namespace. One push can need several approvals — a direct push to protected `main` that edits a protected path needs both `protected_ref_update` and `protected_path_update`. Ref-level rules compose too: force-updating a protected ref needs both `protected_ref_update` and `force_update`, while deleting one needs both `protected_ref_update` and `ref_delete`. They are all-or-nothing: the hook lists every missing approval at once, and consumes none unless all are usable. @@ -263,7 +272,7 @@ Rules that hold no matter what: - **Authority is checked when it is used, not only when it is granted.** Consumption re-derives eligible votes from current roles and policy, so raising the threshold or demoting an approver invalidates approvals that no longer meet it. Such an approval is reopened by the next vote on it, so eligible agents can bring it up to the new requirement instead of requesting it again. - **The repository owns the policy.** New repositories require one vote from an `operator`, and the requester may not vote. Change it with `quorumgit repo policy [--threshold ] [--role ]… [--requester-may-vote | --no-requester-may-vote] [--quorum | --no-quorum]`. `approve request` no longer accepts `--threshold`. - **Quorum mode scales the threshold with the approvers.** With `--quorum` (on the default or on one operation type's override), an operation needs **2/3 + 1 of the agents eligible to approve it** — agents holding an approving role, minus the requester (unless the policy lets it vote) and a takeover's beneficiary — and never fewer than `--threshold`. For example, with four operators a worker's request needs 3 of the 4, while an operator's own request is decided by the other three and needs all 3 of them. At consumption time, if the consumer also cast a yes vote, that vote is excluded and quorum is recomputed over the other eligible approvers; the consumer therefore never helps authorize itself. Because eligibility is counted when the approval is used, adding approvers raises the bar for approvals not yet used, just as raising `--threshold` does. -- **Policy can differ per operation type.** A repository's policy is its default; it may override the threshold, the approving roles, whether the requester may vote, or quorum mode for any of `protected_ref_update`, `force_update`, `ref_delete`, `out_of_scope_push`, `protected_path_update`, and `lease_takeover`. Fields an override leaves unset keep following the default, and `--inherit` removes the override. The effective policy is enforced by the same database triggers, and re-derived at consumption like any other policy change. +- **Policy can differ per operation type.** A repository's policy is its default; it may override the threshold, the approving roles, whether the requester may vote, or quorum mode for any of `protected_ref_update`, `force_update`, `ref_delete`, `out_of_scope_push`, `protected_path_update`, `protected_field_update`, and `lease_takeover`. Fields an override leaves unset keep following the default, and `--inherit` removes the override. The effective policy is enforced by the same database triggers, and re-derived at consumption like any other policy change. ```bash quorumgit repo policy myproject --operation force_update --threshold 2 --agent lead @@ -280,6 +289,8 @@ Registering a `reviewer` or `operator`, changing a role (`quorumgit agent role < > **Upgrading quorum consumer separation.** Migration `011_quorum_consumer_separation.sql` keeps fixed-threshold separation unchanged and makes quorum consumption exclude a consumer's own yes vote from both the counting votes and the quorum denominator. This prevents a small quorum from becoming impossible to execute while preserving independent authorization. > > **Upgrading to full protected ref names.** Protected refs are matched against the full names Git hands the hooks, so one registered as a short name (`--protected-ref main`) used to protect nothing. Migration `010_protected_ref_names.sql` rewrites short names to the branch they name (`main` becomes `refs/heads/main`); from then on pushes to those branches need their `protected_ref_update` approval, as intended. Check `quorumgit repo policy ` after running `quorumgit init`. +> +> **Upgrading to protected fields.** Migration `012_protected_fields.sql` adds no rules, so nothing changes until an operator adds one with `quorumgit repo protect-field`. > **Upgrading to per-operation policy.** Migration `008_operation_policies.sql` adds no overrides, so every operation keeps following the repository default until an operator sets one. @@ -305,6 +316,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `quorumgit repo policy [--operation [--inherit]] [--threshold ] [--role ]… [--[no-]requester-may-vote] [--[no-]quorum]` | Show or change a repository's default approval policy, or its override for one operation type (changes are operator only); also shows protected refs, protected paths, and allowed ref namespaces | | `quorumgit repo protect-ref [--remove]` | Require a `protected_ref_update` approval for every update of a ref, such as `refs/heads/main`; a bare name is taken as a branch (operator only) | | `quorumgit repo protect-path [--remove]` | Require an approval for any push changing matching paths (operator only) | +| `quorumgit repo protect-field [--format json\|toml] [--remove]` | Require an approval for any push changing one value inside matching JSON/TOML files (operator only) | | `quorumgit repo allow-ref [--remove]` | Allow pushes to a ref namespace such as `refs/tags/` (operator only) | | `quorumgit task add --repo --title [--objective ]` | Create a task | | `quorumgit task done [--note ] [--remove-worktree]` | Close a task — live claim holder only; releases the claim | @@ -325,7 +337,7 @@ The incumbent's checkout is not duplicated. When a claim supersedes an earlier c | `quorumgit hook install --repo ` | Install the pre-receive and reference-transaction hooks (hub model) | | `quorumgit audit [--entity ] [--entity-id ] [--limit ]` | Read the audit trail | -`repo list`, `agent list`, and `task list [--repo ]` enumerate what's registered. Commands that act as an agent (`claim`, `renew`, `release`, `task done`, `checkpoint`, `handoff create/accept/decline/cancel`, `approve request/vote`, `approve prepare`, and the operator actions `task reopen`, `agent add`, `agent role`, `repo policy`, `repo protect-ref`, `repo protect-path`, `repo allow-ref`) also take `--agent `, which overrides `QUORUMGIT_AGENT`. At least one `--scope` is required to claim. Exit codes: `0` success, `1` refused/violation/error, `2` usage error. +`repo list`, `agent list`, and `task list [--repo ]` enumerate what's registered. Commands that act as an agent (`claim`, `renew`, `release`, `task done`, `checkpoint`, `handoff create/accept/decline/cancel`, `approve request/vote`, `approve prepare`, and the operator actions `task reopen`, `agent add`, `agent role`, `repo policy`, `repo protect-ref`, `repo protect-path`, `repo protect-field`, `repo allow-ref`) also take `--agent `, which overrides `QUORUMGIT_AGENT`. At least one `--scope` is required to claim. Exit codes: `0` success, `1` refused/violation/error, `2` usage error. `quorumgit doctor` only checks worktree paths already recorded by QuorumGit. It reports: diff --git a/src/quorumgit/cli.py b/src/quorumgit/cli.py index bea966d..11f8149 100644 --- a/src/quorumgit/cli.py +++ b/src/quorumgit/cli.py @@ -228,6 +228,7 @@ def cmd_repo_policy(args, cfg) -> int: repo = registry.get_repository(conn, args.name) print(f"protected refs: {', '.join(repo['protected_refs']) or '-'}") print(f"protected paths: {', '.join(repo['protected_paths']) or '-'}") + print(f"protected fields: {_describe_fields(repo['protected_fields'])}") print(f"allowed ref namespaces: {', '.join(repo['allowed_ref_namespaces']) or '-'}") return 0 @@ -260,6 +261,28 @@ def cmd_repo_protect_path(args, cfg) -> int: return 0 +def _describe_fields(rules: list[dict]) -> str: + return ", ".join( + f"{rule['path_glob']}#{rule['pointer']} ({rule['format']})" for rule in rules + ) or "-" + + +def cmd_repo_protect_field(args, cfg) -> int: + with store.session(cfg) as conn: + rules = registry.set_protected_field( + conn, + args.name, + args.glob, + args.pointer, + actor=getattr(args, "agent", None) or cfg.agent, + fmt=args.format, + remove=args.remove, + ) + conn.commit() + print(f"protected fields for {args.name}: {_describe_fields(rules)}") + return 0 + + def cmd_repo_allow_ref(args, cfg) -> int: with store.session(cfg) as conn: prefixes = registry.set_allowed_ref_namespace( @@ -647,6 +670,8 @@ def cmd_approve_prepare(args, cfg) -> int: print(f" {operation['type']} {entry['hash']}") for path in operation.get("paths", []): print(f" {path}") + for field in operation.get("fields", []): + print(f" {field['path']}#{field['pointer']}") if "approval" in entry: approval = entry["approval"] print(f" approval {approval['id']} status={approval['status']} " @@ -789,6 +814,17 @@ def add(name, fn, configure=None, parent=None): sp.add_argument("--remove", action="store_true"), sp.add_argument("--agent", help="operator making the change"), ), parent=repo) + add("protect-field", cmd_repo_protect_field, lambda sp: ( + sp.add_argument("name"), + sp.add_argument("glob", help="files the rule applies to, e.g. config/*.json"), + sp.add_argument("pointer", + help="JSON Pointer to the protected value, e.g. " + "/limits/max_depth ('' for the whole document)"), + sp.add_argument("--format", choices=("json", "toml"), + help="file format (default: from the glob's extension)"), + sp.add_argument("--remove", action="store_true"), + sp.add_argument("--agent", help="operator making the change"), + ), parent=repo) add("allow-ref", cmd_repo_allow_ref, lambda sp: ( sp.add_argument("name"), sp.add_argument("prefix", help="ref namespace such as refs/tags/"), diff --git a/src/quorumgit/gate.py b/src/quorumgit/gate.py index 8b0833f..dfcb647 100644 --- a/src/quorumgit/gate.py +++ b/src/quorumgit/gate.py @@ -25,7 +25,7 @@ from pathlib import Path from typing import Any -from . import audit, git_objects +from . import audit, git_objects, structured from .canonical import stable_hash from .git_objects import is_zero, zero_oid_like from .registry import ( @@ -603,6 +603,7 @@ def governed_operations( git_dir: str | Path, paths: list[str], scope_paths: list[str], + field_changes: list[dict] | None = None, ) -> list[dict]: """Every approval one ref update needs, derived from what it carries. @@ -651,6 +652,10 @@ def governed_operations( protected = paths_within(paths, repo["protected_paths"]) if protected: operations.append({"type": "protected_path_update", **base, "paths": protected}) + if field_changes: + operations.append( + {"type": "protected_field_update", **base, "fields": field_changes} + ) return operations @@ -673,6 +678,10 @@ def _requirement_message(operation: dict, refusal: str | None) -> str: ) elif operation.get("paths"): message += f" Paths: {_describe_paths(operation['paths'])}." + elif operation.get("fields"): + message += " Fields: " + _describe_paths( + [f"{field['path']}#{field['pointer']}" for field in operation["fields"]] + ) + "." if refusal: message += f" {refusal}" return message @@ -714,14 +723,15 @@ def evaluate_ref_update( refname: str, paths: list[str] | None = None, scope_paths: list[str] | None = None, -) -> tuple[dict, list[tuple[dict, dict]], list[str], list[str]]: + field_changes: list[dict] | None = None, +) -> tuple[dict, list[tuple[dict, dict]], list[str], list[str], list[dict]]: """Apply every push rule to one ref update without changing any state. Returns (repository, [(approval, operation)] the update needs, paths, - scope_paths). Raises PushRejected. Runs in pre-receive and again at the + scope_paths, field_changes). 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 + pre-receive derived (and the field changes), 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. """ @@ -753,8 +763,14 @@ def evaluate_ref_update( ) paths = derived_paths if paths is None else paths scope_paths = derived_scope_paths if scope_paths is None else scope_paths + if field_changes is None: + known = git_objects.ref_tips(git_dir) + field_changes = structured.field_changes( + git_dir, oldrev, newrev, known, repo["protected_fields"] + ) operations = governed_operations( - conn, repo, refname, oldrev, newrev, git_dir, paths, scope_paths + conn, repo, refname, oldrev, newrev, git_dir, paths, scope_paths, + field_changes, ) except git_objects.GitObjectError as exc: raise PushRejected(f"Unable to inspect pushed objects: {exc}") from exc @@ -776,7 +792,7 @@ def evaluate_ref_update( "and request every approval this push needs." ) raise PushRejected(" ".join(missing)) - return repo, granted, paths, scope_paths + return repo, granted, paths, scope_paths, field_changes def check_ref_update( @@ -789,6 +805,7 @@ def check_ref_update( refname: str, paths: list[str] | None = None, scope_paths: list[str] | None = None, + field_changes: list[dict] | None = None, ) -> None: """pre-receive: validate one ref update and record it for its transaction. @@ -796,7 +813,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, scope_paths = evaluate_ref_update( + repo, granted, paths, scope_paths, field_changes = evaluate_ref_update( conn, repository, git_dir, @@ -806,15 +823,16 @@ def check_ref_update( refname, paths=paths, scope_paths=scope_paths, + field_changes=field_changes, ) operations = [operation for _, operation in granted] row = conn.execute( """ INSERT INTO ref_updates ( repository_id, refname, oldrev, newrev, pusher_agent_id, - paths, scope_paths, operations + paths, scope_paths, field_changes, operations ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) RETURNING id """, ( @@ -825,6 +843,7 @@ def check_ref_update( get_agent(conn, pusher)["id"], json_dumps(paths), json_dumps(scope_paths), + json_dumps(field_changes), json_dumps(operations), ), ).fetchone() @@ -904,6 +923,9 @@ def prepare_push( paths, scope_paths = update_paths( local_dir, hub_dir, repo, oldrev, newrev, known_tips=known ) + field_changes = structured.field_changes( + local_dir, oldrev, newrev, known, repo["protected_fields"] + ) refusals: list[str] = [] namespace = ref_namespace_refusal(repo, refname) @@ -926,7 +948,8 @@ def prepare_push( f"(claim {claim['id']}), not {pusher}." ) operations = governed_operations( - conn, repo, refname, oldrev, newrev, local_dir, paths, scope_paths + conn, repo, refname, oldrev, newrev, local_dir, paths, scope_paths, + field_changes, ) return { "repository": repo["name"], @@ -934,6 +957,7 @@ def prepare_push( "oldrev": oldrev, "newrev": newrev, "paths": paths, + "fields": field_changes, "claim": claim, "refusals": refusals, "operations": [ @@ -942,26 +966,31 @@ def prepare_push( } -def _derive_pushed_paths( +def _derive_pushed_content( conn: Connection, repository: str, git_dir: str, updates: list[tuple[str, str, str]], -) -> list[tuple[list[str], list[str]]]: - """(paths, scope_paths) for every update, read from Git alone. - - This is the expensive part of a push check (it reads every new commit), - so it runs before the store's write reservation is taken: other agents' - commands and pushes are not held up while a large push is inspected. - Commits are immutable, and the paths are recorded with the validation, - so the reference transaction re-checks the update against these same - paths under the ref locks. +) -> list[tuple[list[str], list[str], list[dict]]]: + """Paths, scope paths and protected fields for every update. + + Git-object inspection is the expensive part of a push check, so all of it + runs before the store's write reservation is taken. Commits are immutable, + and the derived content is recorded with the validation for replay under + the reference-transaction lock. """ repo = get_repository(conn, repository) + known = git_objects.ref_tips(git_dir) derived = [] for oldrev, newrev, _refname in updates: try: - derived.append(update_paths(git_dir, git_dir, repo, oldrev, newrev)) + paths, scope_paths = update_paths( + git_dir, git_dir, repo, oldrev, newrev, known_tips=known + ) + field_changes = structured.field_changes( + git_dir, oldrev, newrev, known, repo["protected_fields"] + ) + derived.append((paths, scope_paths, field_changes)) except git_objects.GitObjectError as exc: raise PushRejected(f"Unable to inspect pushed objects: {exc}") from exc return derived @@ -988,9 +1017,9 @@ def run_pre_receive( updates = _parse_updates(stdin_lines, hook="pre-receive") if not updates: raise PushRejected("No ref updates supplied on stdin.") - derived = _derive_pushed_paths(conn, repository, git_dir, updates) + derived = _derive_pushed_content(conn, repository, git_dir, updates) begin_immediate(conn) - for (oldrev, newrev, refname), (paths, scope_paths) in zip( + for (oldrev, newrev, refname), (paths, scope_paths, field_changes) in zip( updates, derived, strict=True ): check_ref_update( @@ -1003,6 +1032,7 @@ def run_pre_receive( refname, paths=paths, scope_paths=scope_paths, + field_changes=field_changes, ) # Without the transaction hook nothing would re-validate at update # time or consume approvals, so a missing one must reject the push. @@ -1058,7 +1088,8 @@ def _find_update( """ row = conn.execute( """ - SELECT u.id, u.oldrev, a.name, u.paths, u.approval_ids, u.scope_paths + SELECT u.id, u.oldrev, a.name, u.paths, u.approval_ids, u.scope_paths, + u.field_changes 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 ?) @@ -1086,6 +1117,8 @@ def _find_update( "approval_ids": json_loads(row[4], []), # NULL for an update validated before migration 007: recompute. "scope_paths": json_loads(row[5], None), + # NULL for an update validated before migration 012: recompute. + "field_changes": json_loads(row[6], None), } @@ -1144,7 +1177,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, @@ -1154,6 +1187,7 @@ def _prepare_updates( refname, paths=found["paths"], scope_paths=found["scope_paths"], + field_changes=found["field_changes"], ) 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 391b48c..6005f72 100644 --- a/src/quorumgit/git_objects.py +++ b/src/quorumgit/git_objects.py @@ -273,3 +273,22 @@ def changed_paths_against( sorted({path for commit in walk for path in by_commit[commit]}) for walk in walks ] + + +def commit_parents(git_dir: str | Path, commit: str) -> list[str]: + """The commit's parents, in order (none for a root commit).""" + out = _git(git_dir, "rev-list", "--parents", "-n", "1", commit) + return out.split()[1:] + + +def blob_at(git_dir: str | Path, commit: str, path: str) -> bytes | None: + """The contents of path in commit, or None when it holds no such file.""" + result = subprocess.run( + ["git", "--git-dir", str(git_dir), "cat-file", "blob", f"{commit}:{path}"], + capture_output=True, + check=False, + ) + if result.returncode != 0: + return None + return result.stdout + diff --git a/src/quorumgit/migrations/012_protected_fields.sql b/src/quorumgit/migrations/012_protected_fields.sql new file mode 100644 index 0000000..6e65251 --- /dev/null +++ b/src/quorumgit/migrations/012_protected_fields.sql @@ -0,0 +1,23 @@ +-- Structured protection rules: one value inside matching JSON or TOML files, +-- named by a JSON Pointer. A push needs a protected_field_update approval +-- only when it changes that value, not for every edit to the file. +-- +-- A validated ref update records the field changes pre-receive derived, like +-- its paths, so the reference-transaction `prepared` stage replays exactly +-- what was approved. NULL marks an update validated before this migration; +-- it is recomputed. + +CREATE TABLE protected_fields ( + id INTEGER PRIMARY KEY, + repository_id INTEGER NOT NULL REFERENCES repositories(id) ON DELETE CASCADE, + path_glob TEXT NOT NULL CHECK (path_glob <> ''), + format TEXT NOT NULL CHECK (format IN ('json', 'toml')), + pointer TEXT NOT NULL CHECK (pointer = '' OR pointer LIKE '/%'), + UNIQUE (repository_id, path_glob, pointer) +); + +-- quorumgit-statement +ALTER TABLE ref_updates ADD COLUMN field_changes TEXT CHECK ( + field_changes IS NULL + OR (json_valid(field_changes) AND json_type(field_changes) = 'array') +); diff --git a/src/quorumgit/registry.py b/src/quorumgit/registry.py index 7549f4c..b2e2322 100644 --- a/src/quorumgit/registry.py +++ b/src/quorumgit/registry.py @@ -75,6 +75,17 @@ def _protected_paths(conn: Connection, repository_id: int) -> list[str]: ] +def _protected_fields(conn: Connection, repository_id: int) -> list[dict]: + return [ + {"path_glob": row[0], "format": row[1], "pointer": row[2]} + for row in conn.execute( + "SELECT path_glob, format, pointer FROM protected_fields " + "WHERE repository_id = ? ORDER BY path_glob, pointer", + (repository_id,), + ).fetchall() + ] + + def _allowed_ref_namespaces(conn: Connection, repository_id: int) -> list[str]: return [ row[0] @@ -93,6 +104,7 @@ def _repository_dict(conn: Connection, row) -> dict: "path": row[2], "protected_refs": _protected_refs(conn, row[0]), "protected_paths": _protected_paths(conn, row[0]), + "protected_fields": _protected_fields(conn, row[0]), "allowed_ref_namespaces": _allowed_ref_namespaces(conn, row[0]), } @@ -274,6 +286,7 @@ def get_repository(conn: Connection, name: str) -> dict: "ref_delete", "out_of_scope_push", "protected_path_update", + "protected_field_update", "lease_takeover", ) @@ -650,6 +663,65 @@ def set_protected_path( return _protected_paths(conn, repo["id"]) +def set_protected_field( + conn: Connection, + repository: str, + glob: str, + pointer: str, + *, + actor: str | None, + fmt: str | None = None, + remove: bool = False, +) -> list[dict]: + """Add or remove a structured rule: one value inside matching files. + + ``fmt`` defaults to what the glob's extension implies (.json, .toml). + """ + from .structured import RuleError, infer_format, validate_format, validate_pointer + + begin_immediate(conn) + repo = get_repository(conn, repository) + require_operator(conn, actor, "Changing protected fields") + _validate_path_glob(glob) + try: + validate_pointer(pointer) + if not remove: + fmt = fmt or infer_format(glob) + if fmt is None: + raise RegistryError( + f"Cannot tell the format of {glob!r} from its extension; " + "pass --format json or --format toml." + ) + validate_format(fmt) + except RuleError as exc: + raise RegistryError(str(exc)) from exc + if remove: + cur = conn.execute( + "DELETE FROM protected_fields " + "WHERE repository_id = ? AND path_glob = ? AND pointer = ?", + (repo["id"], glob, pointer), + ) + else: + cur = conn.execute( + "INSERT INTO protected_fields (repository_id, path_glob, format, pointer) " + "VALUES (?, ?, ?, ?) " + "ON CONFLICT (repository_id, path_glob, pointer) DO UPDATE " + "SET format = excluded.format WHERE format <> excluded.format", + (repo["id"], glob, fmt, pointer), + ) + if cur.rowcount: + audit.record( + conn, + "repository.protected_field_removed" if remove + else "repository.protected_field_added", + "repository", + repo["id"], + agent=actor, + detail={"path_glob": glob, "pointer": pointer, "format": fmt}, + ) + return _protected_fields(conn, repo["id"]) + + def set_allowed_ref_namespace( conn: Connection, repository: str, diff --git a/src/quorumgit/store.py b/src/quorumgit/store.py index e7bf075..88bd637 100644 --- a/src/quorumgit/store.py +++ b/src/quorumgit/store.py @@ -57,6 +57,7 @@ "ref_updates", "operation_approval_policies", "operation_approval_roles", + "protected_fields", ) # Engine-level governance rules. A store missing any of these would still diff --git a/src/quorumgit/structured.py b/src/quorumgit/structured.py new file mode 100644 index 0000000..12f39fc --- /dev/null +++ b/src/quorumgit/structured.py @@ -0,0 +1,183 @@ +"""Structured protection rules: values inside JSON and TOML files. + +A protected path governs a whole file. A structured rule governs one value +inside matching files, named by a JSON Pointer (RFC 6901), so a push needs an +approval only when it changes that value; any other edit to the same file is +ordinary content. Like changed paths, field changes are derived from Git +objects alone, commit by commit over the commits new to the repository, so +the pre-receive hook and ``approve prepare`` agree on them. +""" + +from __future__ import annotations + +import json +import tomllib +from pathlib import Path +from typing import Any + +from . import git_objects +from .work import path_in_scopes + +FORMATS = ("json", "toml") + + +class RuleError(ValueError): + pass + + +class _Missing: + """No value: the file or the pointed-to member does not exist.""" + + def __repr__(self) -> str: + return "" + + +class _Unreadable: + """Content that cannot be parsed. Never equal to anything, itself + included, so an unparseable side always counts as a change.""" + + def __eq__(self, other: object) -> bool: + return False + + def __hash__(self) -> int: + return id(self) + + +MISSING = _Missing() + + +def infer_format(path_glob: str) -> str | None: + """The format a glob's file extension implies, if it implies one.""" + suffix = Path(path_glob).suffix.lower() + return {".json": "json", ".toml": "toml"}.get(suffix) + + +def validate_pointer(pointer: str) -> str: + """A JSON Pointer: empty (the whole document) or '/'-separated tokens.""" + if pointer and not pointer.startswith("/"): + raise RuleError( + f"Field pointer {pointer!r} must be empty or start with '/' " + "(a JSON Pointer such as /limits/max_depth)." + ) + body = pointer.replace("~0", "").replace("~1", "") + if "~" in body: + raise RuleError(f"Field pointer {pointer!r} has an invalid '~' escape.") + return pointer + + +def validate_format(fmt: str) -> str: + if fmt not in FORMATS: + raise RuleError( + f"Unknown structured format {fmt!r}; expected one of: {', '.join(FORMATS)}." + ) + return fmt + + +def _tokens(pointer: str) -> list[str]: + if not pointer: + return [] + return [ + token.replace("~1", "/").replace("~0", "~") + for token in pointer[1:].split("/") + ] + + +def resolve(document: Any, pointer: str) -> Any: + """The value a JSON Pointer names in a parsed document, or MISSING.""" + value = document + for token in _tokens(pointer): + if isinstance(value, dict): + if token not in value: + return MISSING + value = value[token] + elif isinstance(value, list): + if not token.isdigit() or (token != "0" and token.startswith("0")): + return MISSING + index = int(token) + if index >= len(value): + return MISSING + value = value[index] + else: + return MISSING + return value + + +def parse(raw: bytes, fmt: str) -> Any: + """Parse file contents, or return an unreadable marker.""" + try: + text = raw.decode("utf-8") + if fmt == "json": + return json.loads(text) + return tomllib.loads(text) + except (UnicodeDecodeError, ValueError): + return _Unreadable() + + +def canonical(value: Any) -> Any: + """A comparable form of a resolved value. + + Python's == treats 1, 1.0 and True as equal. Canonical JSON text keeps + those apart and ignores object key order. TOML dates render via str(). + """ + if value is MISSING or isinstance(value, _Unreadable): + return value + return json.dumps(value, sort_keys=True, ensure_ascii=False, default=str) + + +def _value(git_dir: str | Path, commit: str | None, path: str, rule: dict) -> Any: + if commit is None: + return MISSING + raw = git_objects.blob_at(git_dir, commit, path) + if raw is None: + return MISSING + document = parse(raw, rule["format"]) + if isinstance(document, _Unreadable): + return document + return canonical(resolve(document, rule["pointer"])) + + +def field_changes( + git_dir: str | Path, + oldrev: str, + newrev: str, + known_tips: list[str], + rules: list[dict], +) -> list[dict]: + """Protected values the update changes, as sorted {path, pointer} pairs. + + Only commits new to the repository are inspected (the same set as + git_objects.changed_paths with the same known tips). A commit changes a + value when it differs from the value in every parent, matching the + combined-diff rule for paths: a clean merge changes nothing of its own. + A file that cannot be parsed or read on either side counts as a change, + so a rule fails closed rather than open. + """ + if not rules or git_objects.is_zero(newrev): + return [] + known = list(known_tips) + if not git_objects.is_zero(oldrev): + known.append(oldrev) + changed: set[tuple[str, str]] = set() + for commit in git_objects.new_commits(git_dir, newrev, known): + paths = git_objects.commit_paths(git_dir, commit) + parents: list[str | None] = list(git_objects.commit_parents(git_dir, commit)) + if not parents: + parents = [None] + for path in paths: + for rule in rules: + if (path, rule["pointer"]) in changed: + continue + if not path_in_scopes(path, [rule["path_glob"]]): + continue + after = _value(git_dir, commit, path, rule) + before = [_value(git_dir, parent, path, rule) for parent in parents] + if after is MISSING and all(value is MISSING for value in before): + # The commit lists this path, so the file exists on at + # least one side; reading none means the name could not + # be read back (not valid UTF-8). Fail closed. + changed.add((path, rule["pointer"])) + elif all(after != value for value in before): + changed.add((path, rule["pointer"])) + return [ + {"path": path, "pointer": pointer} for path, pointer in sorted(changed) + ] diff --git a/tests/test_approval_authority.py b/tests/test_approval_authority.py index 438cafa..eecc6b1 100644 --- a/tests/test_approval_authority.py +++ b/tests/test_approval_authority.py @@ -368,6 +368,7 @@ def test_upgrade_leaves_no_agent_with_authority(tmp_path): "009_approval_quorum.sql", "010_protected_ref_names.sql", "011_quorum_consumer_separation.sql", + "012_protected_fields.sql", ] upgraded = store.connect(local) try: diff --git a/tests/test_authorization_binding.py b/tests/test_authorization_binding.py index 75a8879..e5778d4 100644 --- a/tests/test_authorization_binding.py +++ b/tests/test_authorization_binding.py @@ -46,6 +46,7 @@ def _apply_001(cfg: Config): "009_approval_quorum.sql", "010_protected_ref_names.sql", "011_quorum_consumer_separation.sql", + "012_protected_fields.sql", ] diff --git a/tests/test_protected_fields.py b/tests/test_protected_fields.py new file mode 100644 index 0000000..24b3678 --- /dev/null +++ b/tests/test_protected_fields.py @@ -0,0 +1,337 @@ +"""Structured protection rules: values inside JSON and TOML files. + +Derivation tests run against real Git repositories; hook tests are real +`git push`es through the installed hooks. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from quorumgit import cli, gate, git_objects, registry, structured +from tests.conftest import OPERATOR, approve, ensure_agent +from tests.test_gate import _push, _setup + +GIT_ENV = { + "GIT_AUTHOR_NAME": "t", + "GIT_AUTHOR_EMAIL": "t@localhost", + "GIT_COMMITTER_NAME": "t", + "GIT_COMMITTER_EMAIL": "t@localhost", +} + +STATE_RULE = {"path_glob": "config/state.json", "format": "json", "pointer": "/max_depth"} + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + text=True, + env={**os.environ, **GIT_ENV}, + ).stdout.strip() + + +def _write(repo: Path, path: str, content: str, message: str = "edit") -> str: + target = repo / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content) + _git(repo, "add", "-A") + _git(repo, "commit", "-m", message) + return _git(repo, "rev-parse", "HEAD") + + +def _state(**values) -> str: + return json.dumps(values, indent=2) + "\n" + + +@pytest.fixture() +def repo(tmp_path) -> Path: + path = tmp_path / "repo" + path.mkdir() + _git(path, "init", "-q", "-b", "main") + _write(path, "config/state.json", _state(max_depth=3, label="a"), "base") + return path + + +def _changes(repo: Path, old: str, new: str, rules=(STATE_RULE,)) -> list[dict]: + git_dir = git_objects.absolute_git_dir(repo) + return structured.field_changes(git_dir, old, new, [], list(rules)) + + +# ------------------------------------------------------------ rule semantics + + +def test_pointers_are_validated_and_resolved(): + assert structured.validate_pointer("") == "" + assert structured.validate_pointer("/a~1b/~0c") == "/a~1b/~0c" + for bad in ("max_depth", "/a~2b"): + with pytest.raises(structured.RuleError): + structured.validate_pointer(bad) + document = {"a/b": {"~c": [10, 20]}} + assert structured.resolve(document, "/a~1b/~0c/1") == 20 + assert structured.resolve(document, "/a~1b/~0c/2") is structured.MISSING + assert structured.resolve(document, "/a~1b/~0c/01") is structured.MISSING + assert structured.resolve(document, "") == document + + +def test_only_the_protected_value_counts(repo): + base = _git(repo, "rev-parse", "HEAD") + other = _write(repo, "config/state.json", _state(max_depth=3, label="b")) + assert _changes(repo, base, other) == [] + + changed = _write(repo, "config/state.json", _state(max_depth=4, label="b")) + assert _changes(repo, other, changed) == [ + {"path": "config/state.json", "pointer": "/max_depth"} + ] + # Measured over every new commit, not the net result: a change that is + # later reverted within the same push still counts. + reverted = _write(repo, "config/state.json", _state(max_depth=3, label="b")) + assert _changes(repo, other, reverted) == [ + {"path": "config/state.json", "pointer": "/max_depth"} + ] + + +def test_comparison_is_by_value_and_type(repo): + base = _git(repo, "rev-parse", "HEAD") + reordered = _write( + repo, "config/state.json", '{"label": "a", "max_depth": 3}\n' + ) + assert _changes(repo, base, reordered) == [] + retyped = _write(repo, "config/state.json", _state(max_depth=3.0, label="a")) + assert _changes(repo, reordered, retyped) != [] + boolean = _write(repo, "config/state.json", _state(max_depth=True, label="a")) + assert _changes(repo, retyped, boolean) != [] + + +def test_appearing_disappearing_and_unreadable_values_count(repo): + base = _git(repo, "rev-parse", "HEAD") + removed = _write(repo, "config/state.json", _state(label="a")) + assert _changes(repo, base, removed) != [] + broken = _write(repo, "config/state.json", "{not json") + assert _changes(repo, removed, broken) != [] + # Unreadable on both sides still fails closed. + still_broken = _write(repo, "config/state.json", "{still not json") + assert _changes(repo, broken, still_broken) != [] + _git(repo, "rm", "-q", "config/state.json") + _git(repo, "commit", "-q", "-m", "drop") + dropped = _git(repo, "rev-parse", "HEAD") + other = _write(repo, "README.md", "unrelated\n") + assert _changes(repo, dropped, other) == [] + + +@pytest.mark.skipif( + sys.platform != "linux", reason="needs a filesystem that accepts non-UTF-8 names" +) +def test_unreadable_file_names_fail_closed(repo): + base = _git(repo, "rev-parse", "HEAD") + with open(os.fsencode(repo) + b"/config/caf\xe9.json", "wb") as handle: + handle.write(b'{"max_depth": 1}\n') + _git(repo, "add", "-A") + _git(repo, "commit", "-q", "-m", "non-utf8 name") + head = _git(repo, "rev-parse", "HEAD") + rule = {**STATE_RULE, "path_glob": "config/*.json"} + assert _changes(repo, base, head, [rule]) == [ + {"path": "config/caf\\xe9.json", "pointer": "/max_depth"} + ] + + +def test_globs_and_toml(repo): + base = _git(repo, "rev-parse", "HEAD") + toml_rule = {"path_glob": "services/*/settings.toml", "format": "toml", + "pointer": "/limits/replicas"} + edited = _write( + repo, "services/api/settings.toml", "[limits]\nreplicas = 2\nmemory = 1\n" + ) + assert _changes(repo, base, edited, [toml_rule]) == [ + {"path": "services/api/settings.toml", "pointer": "/limits/replicas"} + ] + memory_only = _write( + repo, "services/api/settings.toml", "[limits]\nreplicas = 2\nmemory = 4\n" + ) + assert _changes(repo, edited, memory_only, [toml_rule]) == [] + + +def test_a_clean_merge_changes_nothing_of_its_own(repo): + _git(repo, "checkout", "-q", "-b", "feat") + feat = _write(repo, "src/app.py", "print(1)\n", "feature") + _git(repo, "checkout", "-q", "main") + main = _write(repo, "config/state.json", _state(max_depth=9, label="a")) + _git(repo, "checkout", "-q", "feat") + _git(repo, "merge", "-q", "--no-edit", "main") + merged = _git(repo, "rev-parse", "HEAD") + git_dir = git_objects.absolute_git_dir(repo) + # main's change is already known to the repository; the merge adds none. + assert structured.field_changes( + git_dir, feat, merged, [main, feat], [STATE_RULE] + ) == [] + + +# ------------------------------------------------------------- registration + + +def test_rules_are_operator_only_and_validated(conn, approval_repo): + ensure_agent(conn, OPERATOR, "operator") + ensure_agent(conn, "a-worker") + with pytest.raises(registry.RegistryError, match="requires an operator"): + registry.set_protected_field( + conn, approval_repo, "config/state.json", "/max_depth", actor="a-worker" + ) + with pytest.raises(registry.RegistryError, match="--format"): + registry.set_protected_field( + conn, approval_repo, "config/state", "/max_depth", actor=OPERATOR + ) + with pytest.raises(registry.RegistryError, match="JSON Pointer"): + registry.set_protected_field( + conn, approval_repo, "config/state.json", "max_depth", actor=OPERATOR + ) + rules = registry.set_protected_field( + conn, approval_repo, "config/state.json", "/max_depth", actor=OPERATOR + ) + assert rules == [STATE_RULE] + assert registry.set_protected_field( + conn, approval_repo, "config/state", "/max_depth", actor=OPERATOR, fmt="toml" + ) == [ + {"path_glob": "config/state", "format": "toml", "pointer": "/max_depth"}, + STATE_RULE, + ] + assert registry.set_protected_field( + conn, approval_repo, "config/state", "/max_depth", actor=OPERATOR, remove=True + ) == [STATE_RULE] + assert "protected_field_update" in registry.OPERATION_TYPES + + +# ------------------------------------------------------------------- pushes + + +def _protected_hub(committed_conn, tmp_path): + conn = committed_conn + repo_name, hub, clone, a, b = _setup(conn, tmp_path) + ensure_agent(conn, OPERATOR, "operator") + registry.set_protected_field( + conn, repo_name, "config/state.json", "/max_depth", actor=OPERATOR + ) + conn.commit() + return repo_name, hub, clone, a, b + + +def test_prederived_paths_can_recompute_missing_field_changes( + committed_conn, tmp_path +): + """Replay of a pre-field migration validation must recompute fields.""" + conn = committed_conn + repo_name, hub, _clone, a, _b = _protected_hub(conn, tmp_path) + git_dir = git_objects.absolute_git_dir(hub) + head = git_objects.ref_value(git_dir, "refs/heads/main") + + result = gate.evaluate_ref_update( + conn, + repo_name, + str(git_dir), + a, + head, + head, + "refs/heads/unprotected-replay", + paths=[], + scope_paths=[], + field_changes=None, + ) + assert result[2:] == ([], [], []) + + +def test_hook_governs_only_the_protected_value(committed_conn, tmp_path, cfg): + conn = committed_conn + repo_name, _hub, clone, a, _b = _protected_hub(conn, tmp_path) + _git(clone, "checkout", "-q", "-b", "feat/config") + _write(clone, "config/state.json", _state(max_depth=3, label="a")) + + # A new file carrying the protected value is itself a change to it. + first = _push(clone, a, "feat/config", cfg=cfg) + assert first.returncode != 0 + assert "protected_field_update" in first.stderr + assert "config/state.json#/max_depth" in first.stderr + + plan = gate.prepare_push(conn, repo_name, "feat/config", clone) + assert plan["fields"] == [{"path": "config/state.json", "pointer": "/max_depth"}] + entry = {e["operation"]["type"]: e for e in plan["operations"]}[ + "protected_field_update" + ] + assert entry["hash"] in first.stderr + approve(conn, entry["operation"], requested_by=a) + conn.commit() + accepted = _push(clone, a, "feat/config", cfg=cfg) + assert accepted.returncode == 0, accepted.stderr + assert gate.get_approval(conn, entry["hash"])["status"] == "consumed" + + # Other edits to the same file need nothing. + _write(clone, "config/state.json", _state(max_depth=3, label="b")) + other = _push(clone, a, "feat/config", cfg=cfg) + assert other.returncode == 0, other.stderr + + # Changing the value again needs a fresh approval. + _write(clone, "config/state.json", _state(max_depth=5, label="b")) + again = _push(clone, a, "feat/config", cfg=cfg) + assert again.returncode != 0 + assert "protected_field_update" in again.stderr + + +def test_per_operation_policy_applies_to_field_updates( + committed_conn, tmp_path, cfg +): + conn = committed_conn + repo_name, _hub, clone, a, _b = _protected_hub(conn, tmp_path) + ensure_agent(conn, "op-2", "operator") + registry.set_approval_policy( + conn, repo_name, actor=OPERATOR, + operation_type="protected_field_update", threshold=2, + ) + conn.commit() + _git(clone, "checkout", "-q", "-b", "feat/depth") + _write(clone, "config/state.json", _state(max_depth=7)) + plan = gate.prepare_push(conn, repo_name, "feat/depth", clone) + operation = plan["operations"][0]["operation"] + approval = gate.request_approval(conn, operation, requested_by=a) + assert approval["threshold"] == 2 + gate.vote(conn, approval["id"], OPERATOR, True) + conn.commit() + assert _push(clone, a, "feat/depth", cfg=cfg).returncode != 0 + gate.vote(conn, approval["id"], "op-2", True) + conn.commit() + pushed = _push(clone, a, "feat/depth", cfg=cfg) + assert pushed.returncode == 0, pushed.stderr + + +def test_cli_manages_and_shows_rules( + committed_conn, tmp_path, cfg, monkeypatch, capsys +): + conn = committed_conn + repo_name, _hub, clone, a, _b = _setup(conn, tmp_path) + ensure_agent(conn, OPERATOR, "operator") + conn.commit() + monkeypatch.setenv("QUORUMGIT_DATA_DIR", str(cfg.data_dir)) + monkeypatch.setenv("QUORUMGIT_AGENT", OPERATOR) + assert cli.main([ + "repo", "protect-field", repo_name, "config/*.json", "/max_depth", + ]) == 0 + assert "config/*.json#/max_depth (json)" in capsys.readouterr().out + assert cli.main(["repo", "policy", repo_name]) == 0 + assert "protected fields: config/*.json#/max_depth (json)" in ( + capsys.readouterr().out + ) + + _git(clone, "checkout", "-q", "-b", "feat/cli") + _write(clone, "config/limits.json", _state(max_depth=2)) + monkeypatch.setenv("QUORUMGIT_AGENT", a) + assert cli.main([ + "approve", "prepare", "--repo", repo_name, "--ref", "feat/cli", + "-C", str(clone), + ]) == 0 + out = capsys.readouterr().out + assert "protected_field_update" in out + assert " config/limits.json#/max_depth" in out diff --git a/tests/test_protected_refs.py b/tests/test_protected_refs.py index 2b0267b..51195af 100644 --- a/tests/test_protected_refs.py +++ b/tests/test_protected_refs.py @@ -177,6 +177,7 @@ def test_upgrade_expands_short_protected_ref_names(tmp_path): assert store.migrate(local) == [ "010_protected_ref_names.sql", "011_quorum_consumer_separation.sql", + "012_protected_fields.sql", ] upgraded = store.connect(local) try: