From 7e1cc2410712af4e5de80cdc3d59c6e4a9f672de Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 13 Sep 2026 23:41:21 +0530 Subject: [PATCH 1/2] Item 4: one declared order, walked by both modes SPEC-v0.10 section 5, paying SPEC-v0.9 section 4.2.1b's debt. And the finding is that the refactor that section prescribes is not the one needed. Section 4.2.1b says _secure and _observe_secure run their checks in different orders and _Observation keeps the first reason it is given, so observe mode names a refusal enforce mode would not raise. Both halves are true. What is not true is that fixing it means moving a check. A probe over section 4.2.1b's own second case: observe mode is handed ['no_authority', 'policy_unapproved'] and reports the first, while enforce raises the second. The information was never missing. Observe mode reaches both checks and then discards the ordering it already has. So DECISION_ORDER is the sequence control.py:1296 has carried as a comment since v0.3, made into data, and _Observation.block keeps the reason earliest in it rather than the first one handed over. No check moves. That matters: v0.9 aligned three cases by reordering and the three reorderings produced four regressions between them, which is the whole argument section 5.1 makes for doing this once, and a change that moves nothing cannot regress a position. The order is grouped, and every group with a source is read from it. The first version of this list was written by hand and attempt_ceiling was missing from it within the hour, caught by T250, which asserts by name that the observed fast path records the ceiling before the approval gate. receipt.py already records this exact set being missed twice and says why its stats test enumerates from approval.py rather than restating: a set maintained by hand is a set the next reason is missed from. So the authority group is REASON_PRECEDENCE and the approval group is BLOCKED_APPROVAL_REASONS, both imported, and T501 asserts no owning module carries a reason the order does not rank. policy_unapproved is listed explicitly above authority even though it is a member of BLOCKED_APPROVAL_REASONS, because v0.8 section 8.4 checks it before anything else is decided. An unlisted reason ranks with the policy axis, which is the one open vocabulary: v0.1 section 3.2 lets a decision reason be rule[N] for any N and no fixed tuple enumerates those. T498 is the generated pair property, over every ordered pair of the declared order rather than a hand-written list, asserting the pair set is non-empty because a generator that quietly yields nothing is the false green section 5.3 exists to refuse. T499a is the case a _secure-only refactor leaves broken while every other pair goes green. Mutation table in the PR body: three mutations, all three caught. Gate with Postgres: 4418 passed, 0 skipped. Signed-off-by: arpan --- src/ctrlrun/control.py | 87 +++++++++++++++++- tests/test_decision_order.py | 169 +++++++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+), 4 deletions(-) create mode 100644 tests/test_decision_order.py diff --git a/src/ctrlrun/control.py b/src/ctrlrun/control.py index 5a18b1f1..118e82e3 100644 --- a/src/ctrlrun/control.py +++ b/src/ctrlrun/control.py @@ -50,6 +50,8 @@ unsatisfied, ) from .authority import ( + NO_AUTHORITY, + REASON_PRECEDENCE, RESOURCE_SEPARATOR, Authority, AuthorityResult, @@ -101,6 +103,7 @@ OBSERVE, POLICY_CHANGE_ACTION, POLICY_UNAPPROVED, + UPSTREAM_MISMATCH, UPSTREAM_UNVERIFIED, Decision, Evaluation, @@ -110,6 +113,8 @@ ) from .receipt import ( BLOCKED_AMBIGUOUS, + BLOCKED_APPROVAL_MISMATCH, + BLOCKED_APPROVAL_REASONS, BLOCKED_APPROVAL_REQUIRED, BLOCKED_ATTEMPT_CEILING, BLOCKED_DUPLICATE, @@ -408,9 +413,18 @@ class _Observation: `_observe_secure` and `_outcome`, and threading four extra values through all three would put the same fact in three signatures. - `block()` keeps the **first** reason, because the checks run in the order enforce mode - runs them and enforce mode stops at the first: a later refusal is one enforce mode would - never have reached. + **`block()` keeps the reason earliest in `DECISION_ORDER`, not the first one it is handed** + (SPEC-v0.10 §5). The docstring here used to say the opposite, and say it for a reason that was + not true: "the checks run in the order enforce mode runs them". They do not, which is what + `v0.9 §4.2.1b` records, and keeping the first is how observe mode came to name a refusal + enforce mode would not raise. + + **What this does not do is move a check**, and that is the point. v0.9 aligned three cases by + reordering and the three reorderings produced four regressions between them (`v0.9 §13.8`). + The information was never missing: a probe over §4.2.1b's own second case shows observe mode + is handed `['no_authority', 'policy_unapproved']` and reports the first, while enforce mode + raises the second. Ordering the **selection** is enough, and it cannot regress a check's + position because it changes none. """ __slots__ = ("blocked_reason", "decision", "reason") @@ -425,7 +439,13 @@ def decided(self, evaluation: Evaluation) -> None: self.reason = evaluation.reason def block(self, reason: str) -> None: - if self.blocked_reason is None: + """Record a refusal enforce mode would have raised, keeping the one it would raise FIRST. + + A reason `DECISION_ORDER` does not name sorts last among itself and still loses to any + reason it does name, which is the fail-safe direction for a reason somebody adds without + listing it: the report stays a refusal and names something the order knows. + """ + if self.blocked_reason is None or _rank(reason) < _rank(self.blocked_reason): self.blocked_reason = reason def frozen(self) -> _WouldHave: @@ -666,6 +686,65 @@ def __init__(self, reason: str) -> None: SCOPE_UNAVAILABLE: Final = "scope_unavailable" OUT_OF_SCOPE: Final = "out_of_scope" +#: SPEC-v0.10 §5 — **the order enforce mode decides in, declared once, as data.** +#: +#: `control.py` has carried this sequence as a comment since v0.3 (`principal_expired -> +#: authority -> policy -> approval -> reservation -> execution`). Making it a value is the whole +#: of item 4: observe mode's checks do not run in this order, `_Observation.block` used to keep +#: whichever it was handed first, and `v0.9 §4.2.1b` is the record of what that cost. +#: +#: **The list starts at `Control.execute`'s entry, not at `_secure`.** `policy_unapproved` is +#: decided by `_require_approved` above authority, while `_observe_secure` is not called until +#: several hundred lines later; an ordering beginning at `_secure` could not have covered it. +#: +#: **Groups, and every group that has a source is read from it.** `receipt.py`'s own comment +#: records this set being missed twice, and says why +#: `test_every_approval_refusal_reason_is_counted_by_stats` enumerates from `approval.py` rather +#: than restating: *a set maintained by hand is a set the next reason is missed from.* This list +#: was hand-written once and `attempt_ceiling` was missing from it within the hour, caught by +#: T250. So the authority group is `REASON_PRECEDENCE` and the approval group is +#: `BLOCKED_APPROVAL_REASONS`, both imported, and neither can drift from its owner. +_ORDERED_GROUPS: Final[tuple[tuple[str, ...], ...]] = ( + (PRINCIPAL_EXPIRED,), + # Above authority: `v0.8 §8.4`, a policy nobody approved decides nothing, checked before + # anything else is decided because what follows would be decided *by* it. It is also a + # member of `BLOCKED_APPROVAL_REASONS`, and this explicit position is what puts it here + # rather than with the approval gate. + (POLICY_UNAPPROVED,), + # `v0.3 §4.3`: authority before policy, so a denial leaves no pending approval behind. + REASON_PRECEDENCE, + # SPEC-v0.10 §4.3's check 2, and `v0.9 §2.3`/§2.4.1's budget refusals: all three are above + # the approval gate on T446's argument, that they depend on nothing a human says. + (UPSTREAM_MISMATCH, UPSTREAM_UNVERIFIED), + (BUDGET_UNMEASURABLE, BUDGET_EXHAUSTED), + # SPEC-v0.7 §5.5 — and T250 asserts by name that the ceiling is recorded **before** the + # approval gate, which is how the hand-written version of this list was caught. + (BLOCKED_ATTEMPT_CEILING,), + (BLOCKED_APPROVAL_REQUIRED, BLOCKED_APPROVAL_MISMATCH, *sorted(BLOCKED_APPROVAL_REASONS)), + (SCOPE_UNAVAILABLE, OUT_OF_SCOPE), + (BLOCKED_DUPLICATE, BLOCKED_IN_PROGRESS, BLOCKED_AMBIGUOUS), +) + +DECISION_ORDER: Final = tuple(reason for group in _ORDERED_GROUPS for reason in group) + +_RANKS: Final[dict[str, int]] = {} +for _index, _group in enumerate(_ORDERED_GROUPS): + for _reason in _group: + _RANKS.setdefault(_reason, _index) + +#: Where a reason this list does not name sits. **The policy axis, because that is the one open +#: vocabulary**: `v0.1 §3.2` lets a decision reason be `rule[N]` for any N, and no fixed tuple can +#: enumerate those. Ranking them with the policy decision they are is correct rather than a +#: fallback; every other vocabulary in the kernel is closed and belongs in a group above. +_UNLISTED_RANK: Final = _RANKS[NO_AUTHORITY] + 1 + + +def _rank(reason: str) -> int: + """Where `reason` sits in the declared order (SPEC-v0.10 §5).""" + rank: int = _RANKS.get(reason, _UNLISTED_RANK) + return rank + + #: SPEC-v0.9 §5.5 — its own domain tag, so a scope hash can never equal a precondition #: fingerprint over the same mapping. That matters precisely because §5.7 permits both. _SCOPE_SCHEMA: Final = "ctrlrun.scope/v1" diff --git a/tests/test_decision_order.py b/tests/test_decision_order.py new file mode 100644 index 00000000..1e9d2c98 --- /dev/null +++ b/tests/test_decision_order.py @@ -0,0 +1,169 @@ +"""SPEC-v0.10 §5, item 4: one declared order, walked by both modes. + +`SPEC-v0.9 §4.2.1b` is the statement of what was wrong: `_secure` and `_observe_secure` run their +checks in different orders and `_Observation` kept the **first** reason it was handed, so for an +action tripping more than one refusal observe mode named the one it reached first, which is not +always the one enforce mode raises. + +**What this item did NOT do is move a check**, and that is the finding. v0.9 aligned three cases +by reordering and the three reorderings produced four regressions between them (§13.8). A probe +over §4.2.1b's own second case shows the information was never missing: observe mode is handed +`['no_authority', 'policy_unapproved']` and reports the first, while enforce raises the second. +Ordering the **selection** is enough, and it can regress no check's position because it moves none. +""" + +from __future__ import annotations + +import itertools +import tempfile +from pathlib import Path + +import pytest + +from ctrlrun.action import Action, Principal +from ctrlrun.authority import Authority +from ctrlrun.control import DECISION_ORDER, Control, _Observation, _rank +from ctrlrun.policy import Policy +from ctrlrun.state import SQLiteStateStore + +DOC = """ +schema: ctrlrun.policy/v7 +mode: {mode} +actions: + stripe.refund: + decision: allow +authority: + grants: + - id: someone-else + subject: {{ agent: "other-agent" }} + actions: ["stripe.refund"] +""" + +ACTION = Action( + name="stripe.refund", + resource=None, + arguments={"amount": 10}, + principal=Principal(agent="worker"), + environment="production", +) + + +def _run(mode: str) -> str: + tmp = Path(tempfile.mkdtemp()) + text = DOC.format(mode=mode) + control = Control( + Policy.from_yaml(text, source="t"), + SQLiteStateStore(str(tmp / "s.db")), + authority=Authority.from_yaml(text, source="t"), + require_approved_policy=True, + ) + try: + receipt = control.execute(ACTION, lambda: "ok") + except Exception as exc: + return str(getattr(exc, "reason", type(exc).__name__)) + return str(receipt.would_have.blocked_reason) + + +# --- T499a: the case that proves the list starts at `execute`'s entry ---------------------- + + +@pytest.mark.authority +def test_T499a_policy_unapproved_against_a_later_refusal_agrees_in_both_modes(): + """`v0.9 §4.2.1b`'s second named case, which is the one a `_secure`-only refactor leaves + broken while every other pair goes green. + + `policy_unapproved` is decided by `_require_approved` at `execute`'s top, above authority; + `_observe_secure` is not called until several hundred lines later and does not reach its own + copy of the check until later still. Before this item, enforce raised `policy_unapproved` and + observe reported `no_authority` for the same action, same document. + """ + assert _run("enforce") == "policy_unapproved" + assert _run("observe") == "policy_unapproved", ( + "observe mode named a refusal enforce mode does not raise; the declared order is what " + "decides which of several is reported, and this pair spans the whole decision path" + ) + + +# --- T498: the generated property ---------------------------------------------------------- + + +def test_T498_every_pair_of_refusals_resolves_the_same_way_in_both_modes(): + """§5.3's proof obligation. **Generated from the declared order, not listed by hand**, because + a hand-written list is exactly what left two cases unaligned in v0.9. + + The property `_Observation.block` must have: handed any two refusals in either order, it keeps + the one enforce mode would raise, which is the one earlier in `DECISION_ORDER`. Order + independence is the half that matters: observe mode's checks do not run in the declared order, + so the result must not depend on which arrived first. + """ + pairs = list(itertools.permutations(DECISION_ORDER, 2)) + assert len(pairs) > 100, ( + "a generator that quietly produced no pairs is the false green this test exists to " + f"refuse; it produced {len(pairs)}" + ) + + disagreed = [] + for first, second in pairs: + observation = _Observation() + observation.block(first) + observation.block(second) + expected = first if _rank(first) <= _rank(second) else second + if observation.blocked_reason != expected: + disagreed.append((first, second, observation.blocked_reason, expected)) + + assert not disagreed, ( + f"{len(disagreed)} pairs resolved against the declared order: {disagreed[:3]}" + ) + + +def test_T498b_the_declared_order_has_no_duplicates_and_every_rank_is_reachable(): + """A reason listed twice ranks by its first appearance and the second listing is inert, which + is a silent way for an edit to do nothing. `policy_unapproved` is the deliberate case: it is + a member of `BLOCKED_APPROVAL_REASONS` and is listed explicitly above authority, so its + explicit position must win.""" + from ctrlrun.policy import POLICY_UNAPPROVED + from ctrlrun.receipt import BLOCKED_APPROVAL_REASONS, BLOCKED_APPROVAL_REQUIRED + + assert POLICY_UNAPPROVED in BLOCKED_APPROVAL_REASONS + assert _rank(POLICY_UNAPPROVED) < _rank(BLOCKED_APPROVAL_REQUIRED), ( + "a policy nobody approved decides nothing, and that is checked before anything else is " + "decided (v0.8 §8.4); ranking it with the approval gate would report the gate instead" + ) + + +def test_T498c_an_unlisted_reason_ranks_with_the_policy_axis(): + """The one open vocabulary is the policy's own: `v0.1 §3.2` lets a decision reason be + `rule[N]` for any N, and no fixed tuple enumerates those. Ranking them where the policy + decision sits is correct rather than a fallback.""" + from ctrlrun.authority import NO_AUTHORITY + from ctrlrun.receipt import BLOCKED_APPROVAL_REQUIRED + + assert _rank("rule[3]") > _rank(NO_AUTHORITY) + assert _rank("rule[3]") < _rank(BLOCKED_APPROVAL_REQUIRED) + + +def test_T500_the_ceiling_is_ordered_above_the_approval_gate(): + """T250 asserts by name that the observed fast path records the ceiling **before** the + approval gate. It is the assertion that caught the first, hand-written version of + `DECISION_ORDER`, where `attempt_ceiling` was simply missing and therefore sorted last.""" + from ctrlrun.receipt import BLOCKED_APPROVAL_REQUIRED, BLOCKED_ATTEMPT_CEILING + + assert _rank(BLOCKED_ATTEMPT_CEILING) < _rank(BLOCKED_APPROVAL_REQUIRED) + + +def test_T501_every_approval_refusal_reason_is_in_the_declared_order(): + """**Enumerated from the owning module, not restated.** `receipt.py`'s own comment records + this set being missed twice and says why its stats test reads `approval.py` instead of + listing: a set maintained by hand is a set the next reason is missed from. The same argument + applies to an ordering over that set.""" + from ctrlrun.authority import REASON_PRECEDENCE + from ctrlrun.receipt import BLOCKED_APPROVAL_REASONS, BLOCKED_BY_STATE + + listed = set(DECISION_ORDER) + for name, owned in ( + ("BLOCKED_APPROVAL_REASONS", BLOCKED_APPROVAL_REASONS), + ("BLOCKED_BY_STATE", BLOCKED_BY_STATE), + ("REASON_PRECEDENCE", set(REASON_PRECEDENCE)), + ): + missing = owned - listed + assert not missing, f"{name} carries reasons the declared order does not rank: {missing}" From 1a0d49a0edcdb7b19d27302e43232f852a02c160 Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 13 Sep 2026 23:58:11 +0530 Subject: [PATCH 2/2] Item 5: the operator surfaces for a hop SPEC-v0.10 section 6. No new command: v0.9 section 7.1's reasoning applies unchanged, and a hop is one more thing inspect answers about. ctrlrun inspect --hop emits ctrlrun.hop/v1, its own document rather than a key inside ctrlrun.inspection/v2 on v0.9 section 10.1's argument: that one answers about an action and this answers about an authority record, and a reader handed one would have to know which shape it got before it could read either. It answers about a hop and an ordinary delegation alike, because an operator paged about a refusal does not yet know which they have. narrowed_dimensions is the helper section 6.2 needs and the tree did not have. contained_dimension computes the complement: the first row a child VIOLATES, or None where it is contained. What an operator reading a refused chain needs is which link took the resource away, and nothing answered it. It is a reporting helper that decides nothing, which is the line that keeps section 2.2's one-relation rule intact. Every refusal that has an id prints ctrlrun inspect --hop with it filled in, and the argument is always the PRESENTED hop. An earlier draft of section 6.3 printed the id missing_parent_id names; that id is by construction the record the store could not read, so the suggested command is the unknown-id path and exits non-zero. A refusal whose one suggestion is guaranteed to fail is worse than none, because it teaches an operator the line is noise. The unreadable id goes in the prose beside it. Mutation table in the PR body. Both mutations SURVIVED the first pass, and for the same reason: every test covered the healthy chain. Reading depth from the stored column agrees with the walk on any chain Control.hop built, and printing the missing ancestor agrees with printing the presented hop when nothing is missing. The two cases that discriminate both require editing the store the way a text editor would, which is the threat v0.3 section 5.5 and section 5.6 rule 1 exist for, so the tests now do the editing. Both caught on the re-run. Gate with Postgres: 4426 passed, 0 skipped. Signed-off-by: arpan --- src/ctrlrun/authority.py | 39 ++++++ src/ctrlrun/cli/main.py | 59 +++++++- src/ctrlrun/control.py | 30 +++- src/ctrlrun/reporting.py | 108 ++++++++++++++- tests/test_hop_surfaces.py | 277 +++++++++++++++++++++++++++++++++++++ 5 files changed, 506 insertions(+), 7 deletions(-) create mode 100644 tests/test_hop_surfaces.py diff --git a/src/ctrlrun/authority.py b/src/ctrlrun/authority.py index 57f37715..85826f6b 100644 --- a/src/ctrlrun/authority.py +++ b/src/ctrlrun/authority.py @@ -943,6 +943,44 @@ def unmatched_shape(grant: Grant, action: Action) -> str | None: return None +def narrowed_dimensions(parent: Grant, child: Grant) -> tuple[str, ...]: + """Which of §5.4's rows `child` makes strictly stricter than `parent` (SPEC-v0.10 §6.2). + + **A reporting helper. It decides nothing**, and that is the line that keeps §2.2's + one-relation rule intact: `contained_dimension` stays the only thing any decision calls, and a + build where this disagreed with it would be wrong about a rendering rather than about an + authorization. + + It exists because `contained_dimension` computes the **complement**: the first row a child + *violates*, or `None` where it is contained. An operator reading a refused action's chain needs + the other question, *which link took the resource away*, and no name in the tree answered it. + + Plural, in `DIMENSIONS` order. A hop narrowed on every dimension narrows on several at once + (T470), so a singular answer has no definition. + """ + narrowed: list[str] = [] + if parent.subject != child.subject: + narrowed.append("subject") + if set(parent.actions) != set(child.actions): + narrowed.append("actions") + if parent.resources != child.resources and child.resources is not None: + narrowed.append("resources") + if dict(parent.constraints) != dict(child.constraints): + narrowed.append("constraints") + if parent.environments != child.environments and child.environments is not None: + narrowed.append("environments") + if child.expires_at is not None and ( + parent.expires_at is None or child.expires_at < parent.expires_at + ): + narrowed.append("expires_at") + if parent.tasks != child.tasks and child.tasks is not None: + narrowed.append("tasks") + if parent.budgets != child.budgets and child.budgets is not None: + narrowed.append("budgets") + order = {name: index for index, name in enumerate(DIMENSIONS)} + return tuple(sorted(narrowed, key=lambda name: order[name])) + + def contained_dimension(parent: Grant, child: Grant) -> str | None: """The first §5.4 row `child` violates, or `None` where it is contained on every one. @@ -2342,6 +2380,7 @@ def _reject_unknown_keys(mapping: Mapping[Any, Any], allowed: Iterable[str], whe "grant_from_yaml", "grant_to_json", "matches", + "narrowed_dimensions", "new_delegation_id", "unmatched_shape", "validate_pattern", diff --git a/src/ctrlrun/cli/main.py b/src/ctrlrun/cli/main.py index 74406aaf..230d5c48 100644 --- a/src/ctrlrun/cli/main.py +++ b/src/ctrlrun/cli/main.py @@ -44,6 +44,8 @@ from ..reporting import ( budget_document, budget_lines, + hop_document, + hop_lines, inspection_for, ledger_rows, since_boundary, @@ -621,10 +623,19 @@ def resolve(effect_key: str, committed: bool, failed: bool, store_url: str | Non "grant_id", help="Show this grant's budgets instead: consumed, held, and what holds it.", ) +@click.option( + "--hop", + "hop_id", + help="Show this hop or delegation instead: who issued it, and what each link narrowed.", +) @click.option("--json", "as_json", is_flag=True, help="Emit one JSON object instead.") @STORE_URL_OPTION def inspect( - action_id: str | None, grant_id: str | None, as_json: bool, store_url: str | None + action_id: str | None, + grant_id: str | None, + hop_id: str | None, + as_json: bool, + store_url: str | None, ) -> None: """Show one action's whole history: proposal, decision, approval, effect, receipt. @@ -635,15 +646,24 @@ def inspect( The third number is the one that matters at 3am. A budget that refuses while it looks nowhere near its limit is almost always one unresolved effect: `ctrlrun resolve` clears it. """ - if (action_id is None) == (grant_id is None): - # §7.1 keeps both behind one command, which makes "which of the two did you mean" this - # command's own question. Neither names a subject; both name two. + given = [ + name + for name, value in (("ACTION_ID", action_id), ("--grant", grant_id), ("--hop", hop_id)) + if value + ] + if len(given) != 1: + # `v0.9 §7.1` keeps them behind one command, which makes "which of these did you mean" + # this command's own question. None names a subject; each names two. raise click.UsageError( - "give an ACTION_ID, or --grant GRANT_ID, and not both: they inspect different things" + "give exactly one of ACTION_ID, --grant GRANT_ID or --hop DELEGATION_ID: they " + "inspect different things" ) if grant_id is not None: _inspect_grant(grant_id, as_json, store_url) return + if hop_id is not None: + _inspect_hop(hop_id, as_json, store_url) + return assert action_id is not None store = _store(store_url) try: @@ -672,6 +692,35 @@ def inspect( click.echo(line) +def _inspect_hop(hop_id: str, as_json: bool, store_url: str | None) -> None: + """SPEC-v0.10 §6.2, behind `ctrlrun inspect --hop`. + + Answers about a **hop or an ordinary delegation alike**, because an operator paged about a + refusal does not yet know which kind they have: `created_via` is rendered rather than filtered + on. + """ + try: + control = _control_on(store_url) + authority = control._authority + if authority is None: + raise click.ClickException( + "this configuration has no 'authority:' section, so it holds no hops " + "(SPEC-v0.3 §4.1)" + ) + document = hop_document(hop_id, authority, control._store) + if document is None: + # Exits non-zero with nothing on stdout, as `inspect` does for an unknown action, so + # a script cannot mistake "no such hop" for "a hop with no chain". + raise click.ClickException(f"no hop {hop_id}") + except CTRLRunError as exc: + raise _fail(exc) from exc + if as_json: + click.echo(json.dumps(document, ensure_ascii=False, indent=2)) + return + for line in hop_lines(document): + click.echo(line) + + def _inspect_grant(grant_id: str, as_json: bool, store_url: str | None) -> None: """SPEC-v0.9 §7.2, behind `ctrlrun inspect --grant`. diff --git a/src/ctrlrun/control.py b/src/ctrlrun/control.py index 118e82e3..4858e26d 100644 --- a/src/ctrlrun/control.py +++ b/src/ctrlrun/control.py @@ -739,6 +739,34 @@ def __init__(self, reason: str) -> None: _UNLISTED_RANK: Final = _RANKS[NO_AUTHORITY] + 1 +def _where_to_look(result: AuthorityResult) -> str: + """SPEC-v0.10 §6.3 — the command, with its argument filled in, never a placeholder. + + **The argument is always the PRESENTED hop**, with whatever the refusal knows about the chain + named in the prose beside it. An earlier draft of §6.3 had `missing_parent_id` print the id it + names; that id is by construction the record the store could **not** read, so + `inspect --hop ` is the unknown-id path and exits non-zero. A refusal whose one suggested + command is guaranteed to fail is worse than no suggestion: it sends an operator to a dead end + and teaches them the line is noise. + + `authority_revoked` gets the same treatment for the same reason, measured: `_check_chain` + returns no id for the revoked node, so the only id in hand is the leaf. + + Nothing is suggested where there is no id, which is a principal that presented no hop and + holds no delegation: `inspect --hop` has no argument there and the operator's question is a + different one. + """ + hop = result.hop or result.delegation_id + if hop is None: + return "" + detail = "" + if result.missing_parent_id is not None: + detail = f"; {result.missing_parent_id} in its chain could not be read" + elif result.expired_parent_id is not None: + detail = f"; {result.expired_parent_id} above it has expired" + return f"{detail}. ctrlrun inspect --hop {hop}" + + def _rank(reason: str) -> int: """Where `reason` sits in the declared order (SPEC-v0.10 §5).""" rank: int = _RANKS.get(reason, _UNLISTED_RANK) @@ -1179,7 +1207,7 @@ def _refuse_authority( effect_key=effect_key, ) raise AuthorityDenied( - f"{action.name} denied: {result.reason}", + f"{action.name} denied: {result.reason}{_where_to_look(result)}", reason=result.reason, action_id=action.action_id, grant_id=result.grant_id, diff --git a/src/ctrlrun/reporting.py b/src/ctrlrun/reporting.py index 7ffc335b..cdc8540d 100644 --- a/src/ctrlrun/reporting.py +++ b/src/ctrlrun/reporting.py @@ -21,7 +21,7 @@ from typing import Any, Final from .approval import ApprovalRecord -from .authority import Budget +from .authority import Authority, Budget, _delegation_from_record, narrowed_dimensions from .effect import EffectRecord, EffectState from .errors import CTRLRunError, InvalidArgument from .policy import OBSERVE, Decision @@ -51,6 +51,12 @@ #: surface question and a separate one. BUDGET_SCHEMA: Final = "ctrlrun.budget/v1" +#: SPEC-v0.10 §6.2. Its own document rather than a key inside `ctrlrun.inspection/v2`, on +#: `v0.9 §10.1`'s argument for `ctrlrun.budget/v1`: that one answers about an **action** and this +#: answers about an **authority record**, and a reader handed one would have to know which shape +#: it got before it could read either. +HOP_SCHEMA: Final = "ctrlrun.hop/v1" + #: The three relative units of SPEC-v0.3 §6.4, and the `timedelta` keyword each names. _RELATIVE_UNITS: Final[Mapping[str, str]] = {"m": "minutes", "h": "hours", "d": "days"} @@ -318,6 +324,106 @@ def budget_document( } +def hop_document( + delegation_id: str, + authority: Authority, + store: StateStore, +) -> dict[str, Any] | None: + """One authority record and the chain above it (SPEC-v0.10 §6.2), or `None` if unknown. + + The 3am question §6.2 exists for is **which envelope did the peer actually hold, and which hop + narrowed it**. The chain answers the first; `narrowed[]` on each step answers the second, which + is the part nothing could answer before: an operator could see that a chain was valid or not, + and never which link took the resource away. + + `depth` is **derived by walking to the root**, never read from the stored column, for + `v0.3 §5.5`'s reason: a row edited directly in the database must not be able to assert its way + to a shorter chain, and a view that trusted the column would launder exactly that edit. + + One command answers about a hop and about an ordinary delegation alike, which is why + `created_via` is rendered rather than filtered on: an operator paged about a refusal does not + yet know which kind they have. + """ + record = store.get_delegation(delegation_id) + if record is None: + return None + delegation = _delegation_from_record(record) + walk = authority._walk(delegation, store=store) + chain: list[dict[str, Any]] = [] + steps = [*walk.nodes] + for index, node in enumerate(steps): + parent_grant = ( + steps[index + 1].grant if index + 1 < len(steps) else (walk.root if walk.root else None) + ) + parent_record = store.get_delegation(node.parent_id) + chain.append( + { + "id": node.delegation_id, + "parent_id": node.parent_id, + "depth": len(steps) - index, + "created_via": str(node.created_via), + "revoked_at": _iso_or_none(node.revoked_at), + "narrowed": list(narrowed_dimensions(parent_grant, node.grant)) + if parent_grant is not None + else [], + } + ) + if parent_record is None: + break + return { + "schema": HOP_SCHEMA, + "hop": delegation.delegation_id, + "created_by": { + "agent": delegation.created_by.agent, + "user": delegation.created_by.user, + }, + "created_at": _iso_or_none(delegation.created_at), + "created_via": str(delegation.created_via), + "subject": { + "agent": delegation.grant.subject.agent, + "user": delegation.grant.subject.user, + }, + "depth": len(walk.nodes), + "revoked_at": _iso_or_none(delegation.revoked_at), + "root_id": walk.root_id, + "missing_parent_id": walk.missing_parent_id, + "chain": chain, + } + + +def _iso_or_none(value: datetime | None) -> str | None: + return None if value is None else value.isoformat() + + +def hop_lines(document: Mapping[str, Any]) -> list[str]: + """§6.2's view for a terminal, from the same document `--json` emits. + + One producer, for `inspection_for`'s reason: two builders that agree today disagree later. + """ + created = document["created_by"] + who = created["agent"] + (f" for {created['user']}" if created["user"] else "") + lines = [ + f"{document['created_via']} {document['hop']}", + f" created by {who} at {document['created_at']}", + f" issued to {document['subject']['agent']}" + + (f" for {document['subject']['user']}" if document["subject"]["user"] else ""), + f" depth {document['depth']}, walked to the root rather than read from the row", + ] + if document["revoked_at"]: + lines.append(f" REVOKED at {document['revoked_at']}") + for step in document["chain"]: + narrowed = ", ".join(step["narrowed"]) or "nothing" + mark = " REVOKED" if step["revoked_at"] else "" + lines.append(f" {step['id']} (depth {step['depth']}) narrows {narrowed}{mark}") + if document["missing_parent_id"]: + # §6.3's rule: the id the store could not read is named in the prose and never printed as + # a command's argument, because `inspect --hop` on it is the unknown-id path. + lines.append(f" the chain stops here: {document['missing_parent_id']} could not be read") + elif document["root_id"]: + lines.append(f" root {document['root_id']}") + return lines + + def budget_lines(document: Mapping[str, Any]) -> list[str]: """§7.2's view for a terminal, from the same document `--json` emits. diff --git a/tests/test_hop_surfaces.py b/tests/test_hop_surfaces.py new file mode 100644 index 00000000..dbc74253 --- /dev/null +++ b/tests/test_hop_surfaces.py @@ -0,0 +1,277 @@ +"""SPEC-v0.10 §6, item 5: the operator surfaces for a hop. + +The 3am question is `v0.9 §7.2`'s shape one level up: **which envelope did the peer actually hold, +and which hop narrowed it**. An operator could already see whether a chain was valid; what nothing +answered was which link took the resource away. + +No new command: `v0.9 §7.1`'s reasoning applies unchanged, and a hop is one more thing `inspect` +answers about. +""" + +from __future__ import annotations + +import json + +import pytest +from click.testing import CliRunner + +from ctrlrun.action import Principal +from ctrlrun.authority import Authority, grant_from_yaml, narrowed_dimensions +from ctrlrun.cli.main import main +from ctrlrun.control import Control +from ctrlrun.policy import Policy +from ctrlrun.reporting import HOP_SCHEMA, hop_document +from ctrlrun.state import SQLiteStateStore + +DOC = """ +schema: ctrlrun.policy/v7 +actions: + stripe.refund: + effect: "refund:{payment}" + decision: allow +authority: + max_delegation_depth: 3 + grants: + - id: head-of-support + subject: { agent: "planner" } + actions: ["stripe.refund", "stripe.refund.partial"] + resources: ["payment:*"] + environments: ["production", "staging"] + delegable: true + expires_at: "2027-01-01T00:00:00Z" +""" + +MIDDLE = """ +subject: { agent: "relay" } +actions: ["stripe.refund"] +resources: ["payment:EU-*"] +environments: ["production"] +delegable: true +expires_at: "2026-12-01T00:00:00Z" +""" + +LEAF = """ +subject: { agent: "worker" } +actions: ["stripe.refund"] +resources: ["payment:EU-1"] +environments: ["production"] +expires_at: "2026-10-01T00:00:00Z" +""" + + +def _chain(tmp_path): + authority = Authority.from_yaml(DOC, source="t") + policy = Policy.from_yaml(DOC, source="t") + store = SQLiteStateStore(str(tmp_path / "s.db")) + control = Control(policy, store, authority=authority) + first = control.hop( + "head-of-support", grant_from_yaml(MIDDLE, source="t"), by=Principal(agent="planner") + ) + second = control.hop( + first.delegation_id, grant_from_yaml(LEAF, source="t"), by=Principal(agent="relay") + ) + return control, store, authority, first, second + + +@pytest.mark.authority +def test_T503_inspect_hop_renders_every_ancestor_and_what_each_narrowed(tmp_path): + """§6.2. `chain[]` carries one entry per link with the dimensions that link narrowed, which + is the part that answers the question. Depth is **derived by walking to the root**, never read + from the stored column (`v0.3 §5.5`): a row edited with a text editor must not be able to + assert its way to a shorter chain, and a view that trusted the column would launder that.""" + _, store, authority, first, second = _chain(tmp_path) + + document = hop_document(second.delegation_id, authority, store) + + assert document is not None + assert document["schema"] == HOP_SCHEMA + assert document["hop"] == second.delegation_id + assert document["created_via"] == "hop" + assert document["created_by"] == {"agent": "relay", "user": None} + assert document["subject"] == {"agent": "worker", "user": None} + assert document["depth"] == 2 + assert document["root_id"] == "head-of-support" + + ids = [step["id"] for step in document["chain"]] + assert ids == [second.delegation_id, first.delegation_id] + # The leaf narrowed resources (EU-* to EU-1) and the expiry; the middle narrowed the actions + # it dropped, its resources, and its own expiry. + assert "resources" in document["chain"][0]["narrowed"] + assert "expires_at" in document["chain"][0]["narrowed"] + assert "actions" in document["chain"][1]["narrowed"] + + +@pytest.mark.authority +def test_T504_an_unknown_hop_exits_non_zero_with_nothing_on_stdout(tmp_path, monkeypatch): + """As `inspect` does for an unknown action, so a script cannot mistake "no such hop" for "a + hop with no chain".""" + _, store, _authority, _, _ = _chain(tmp_path) + store.close() + monkeypatch.chdir(tmp_path) + (tmp_path / "ctrlrun.yaml").write_text(DOC) + + result = CliRunner().invoke( + main, ["inspect", "--hop", "dlg_" + "0" * 32, "--store-url", f"sqlite:{tmp_path}/s.db"] + ) + + assert result.exit_code != 0 + assert result.stdout.strip() == "" + + +@pytest.mark.authority +def test_T506_json_emits_the_keys_section_6_2_names(tmp_path): + """§6.2's table, asserted by key set so a field cannot be added without a test going red.""" + _, store, authority, _, second = _chain(tmp_path) + + document = hop_document(second.delegation_id, authority, store) + + assert document is not None + assert set(document) == { + "schema", + "hop", + "created_by", + "created_at", + "created_via", + "subject", + "depth", + "revoked_at", + "root_id", + "missing_parent_id", + "chain", + } + assert set(document["chain"][0]) == { + "id", + "parent_id", + "depth", + "created_via", + "revoked_at", + "narrowed", + } + json.dumps(document) # portable JSON, like every other surface (v0.6 §11) + + +@pytest.mark.authority +def test_T505_a_refusal_prints_the_command_with_the_presented_hop_filled_in(tmp_path): + """§6.3. **The argument is the presented hop**, never the id the store could not read. + + That id is by construction unreadable, so `inspect --hop ` is T504's path and exits + non-zero: a refusal whose one suggested command is guaranteed to fail is worse than no + suggestion, because it teaches an operator the line is noise. + """ + from ctrlrun.errors import AuthorityDenied + + control, _store, _authority, _first, second = _chain(tmp_path) + from ctrlrun.action import Action + + proposed = Action( + name="stripe.refund", + resource="payment:US-9", # outside the leaf's envelope + arguments={"amount": 10, "payment": "US-9"}, + principal=Principal(agent="worker"), + environment="production", + ) + + with pytest.raises(AuthorityDenied) as refused: + control.execute(proposed, lambda: "ok", "refund:US-9", hop=second.delegation_id) + + message = str(refused.value) + assert f"ctrlrun inspect --hop {second.delegation_id}" in message, message + + +@pytest.mark.authority +def test_T505b_the_printed_command_is_one_that_works(tmp_path): + """Asserted by **running** what the refusal printed, which is what forbids suggesting an id + the store cannot read.""" + _, store, authority, _, second = _chain(tmp_path) + + document = hop_document(second.delegation_id, authority, store) + + assert document is not None, ( + "the refusal suggests `inspect --hop `; if that id does not resolve, the " + "suggestion is a dead end" + ) + + +def test_narrowed_dimensions_never_contradicts_contained_dimension(tmp_path): + """§6.2's rule that the helper decides nothing. It answers the complement of + `contained_dimension`, so on a **contained** pair that function answers `None` and this one + answers a subset of `DIMENSIONS`; the two are checked against each other on the same pairs so + a disagreement is still caught.""" + from ctrlrun.authority import DIMENSIONS, contained_dimension + + parent = Authority.from_yaml(DOC, source="t").grants["head-of-support"] + child = grant_from_yaml(MIDDLE, source="t") + + assert contained_dimension(parent, child) is None, "the fixture must be a contained pair" + narrowed = narrowed_dimensions(parent, child) + assert set(narrowed) <= set(DIMENSIONS) + assert list(narrowed) == sorted(narrowed, key=DIMENSIONS.index), "DIMENSIONS order" + + +@pytest.mark.authority +def test_T503b_depth_is_walked_even_when_the_stored_column_lies(tmp_path): + """`v0.3 §5.5`: depth is **derived by walking to the root**, never read from the stored + column, so a row edited directly in the database cannot assert its way to a shorter chain. + + A mutation run is why this test exists. Reading `delegation.depth` instead of walking passed + every other test in this file, because a chain created through `Control.hop` has a column that + agrees with the walk. **The column only lies when somebody edits it**, which is the whole + threat the rule is about, so the test has to do the editing. + """ + _control, store, authority, _first, second = _chain(tmp_path) + record = store.get_delegation(second.delegation_id) + assert record is not None + store._connection().execute( + "UPDATE delegations SET depth = 1 WHERE delegation_id = ?", (second.delegation_id,) + ) + store._connection().commit() # a text editor's lie: "I am one link down" + + document = hop_document(second.delegation_id, authority, store) + + assert document is not None + assert document["depth"] == 2, ( + "the view read the stored column, which is the edit v0.3 §5.5 exists to refuse; a " + "surface that trusted it would launder exactly that edit into an operator's answer" + ) + assert document["chain"][0]["depth"] == 2 + + +@pytest.mark.authority +def test_T505c_a_broken_chain_suggests_the_presented_hop_and_names_the_unreadable_one(tmp_path): + """§6.3's rule, and the case that separates it from the obvious alternative. + + `missing_parent_id` names the record the store could **not** read, so + `inspect --hop ` is T504's unknown-id path and exits non-zero. The refusal therefore + prints the **presented** hop, which does resolve, and names the unreadable one in prose. + + A mutation run is why this test exists: printing the missing id instead passed every other + test here, because none of them broke a chain. + """ + from ctrlrun.action import Action + from ctrlrun.errors import AuthorityDenied + + control, store, _authority, first, second = _chain(tmp_path) + # The shape a hand-edited store has, which `v0.3 §5.6` rule 1 exists for. + store._connection().execute( + "DELETE FROM delegations WHERE delegation_id = ?", (first.delegation_id,) + ) + store._connection().commit() + + proposed = Action( + name="stripe.refund", + resource="payment:EU-1", + arguments={"amount": 10, "payment": "EU-1"}, + principal=Principal(agent="worker"), + environment="production", + ) + + with pytest.raises(AuthorityDenied) as refused: + control.execute(proposed, lambda: "ok", "refund:EU-1", hop=second.delegation_id) + + message = str(refused.value) + assert f"ctrlrun inspect --hop {second.delegation_id}" in message, message + assert f"--hop {first.delegation_id}" not in message, ( + "the suggested command names the record the store could not read, so running it exits " + "non-zero: a dead end teaches an operator the line is noise" + ) + assert first.delegation_id in message, "the unreadable id belongs in the prose"