From deb26558117f592c6c207df41d6d1e10eb3fbdef Mon Sep 17 00:00:00 2001 From: arpan Date: Thu, 3 Sep 2026 19:54:34 +0530 Subject: [PATCH 1/2] item 3: policy effect: and resource: templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SPEC-v0.2 §3, acceptance test T16. `schema: ctrlrun.policy/v2` adds `effect:`, `resource:` and `mcp:` to an action entry, because the gateway (item 6) has no decorator to carry an effect template and the policy file has to. v0.2 loads both schemas. A v2 key in a v1 document is a PolicyError naming the key and the required schema: v0.1 would ignore the template and execute with no duplicate protection at all, so a document that depends on one has to say so in a way v0.1 refuses. Templates are validated at load, not at first traffic — the same reason @protect checks its own at decoration time — and `mcp.not_executed_on_error` must be a bool, defaulting to the fail-closed value. Precedence (§3.2): where both the decorator and the policy declare a template, the decorator's is in force. The policy is a deployment artefact and the decorator is a statement in the code that will run; substituting the policy's would change an effect identity without changing a line of the program. The mismatch is a warning naming both, once per decorated function, emitted at decoration time when `control=` was passed — so an operator adding templates for the gateway's sake hears about it at import and is not broken by it. The decorator now resolves its Control before building the Action, since the policy may be the only place a template is declared and `resource` is part of the canonical form. ARCHITECTURE §6 records the one new cross-module import: policy.py takes the template grammar from effect.py to validate at load, and still knows nothing of effect state. A second copy of a security-critical grammar is worse. One v0.1 test used `ctrlrun.policy/v2` as its example of an unsupported schema. The rule under test is unchanged, so the example moves to v3. --- docs/ARCHITECTURE.md | 9 +- src/ctrlrun/control.py | 71 ++++++++++++++- src/ctrlrun/policy.py | 154 ++++++++++++++++++++++++++++++-- tests/test_policy.py | 178 ++++++++++++++++++++++++++++++++++++- tests/test_protect.py | 196 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 594 insertions(+), 14 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1ca08996..c5508839 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -135,7 +135,7 @@ Pragmas: `journal_mode=WAL`, `busy_timeout=5000`, `synchronous=NORMAL`. | Module | Owns | Must not know about | |---|---|---| | `action.py` | model, canonicalization, hash | policy, storage | -| `policy.py` | YAML → rules → Decision | approvals, effects | +| `policy.py` | YAML → rules → Decision; `effect:`/`resource:` templates | approvals, effect *state* | | `approval.py` | request/grant/consume, providers | executors | | `effect.py` | key templating, state enum, transition rules | SQLite | | `state.py` | `StateStore` protocol + SQLite/in-memory impls | policy, decorator | @@ -145,6 +145,13 @@ Pragmas: `journal_mode=WAL`, `busy_timeout=5000`, `synchronous=NORMAL`. Dependencies point downward only. `Control` is the only module that composes the others. +One exception, added in v0.2 and worth stating rather than discovering: `policy.py` imports the +template grammar (`template_placeholders`) from `effect.py`, because SPEC-v0.2 §3.1 requires an +`effect:` / `resource:` template to be validated when the policy loads and the grammar is +security-critical enough that a second copy of it is worse than the import. Policy still knows +nothing of effect state — no records, no transitions, no reservations — and `effect.py` does not +import `policy.py`, so there is no cycle. + ## 7. What changes after v0.1 (and what doesn't) Stable from v0.1 onward: the four public concepts, the action canonical form (versioned), the effect state machine, fail-closed defaults, the executor outcome mapping. diff --git a/src/ctrlrun/control.py b/src/ctrlrun/control.py index 57ec892a..d216851e 100644 --- a/src/ctrlrun/control.py +++ b/src/ctrlrun/control.py @@ -876,6 +876,12 @@ def decorator(func: Callable[P, R]) -> Callable[P, R]: signature = inspect.signature(func) _reject_variadic(signature, name) dangling: list[bool] = [] + compared: list[bool] = [] + if control is not None: + # SPEC-v0.2 §3.2 — the warning lands when the function first resolves its + # Control, which is here when one was passed: an operator who added templates to + # a policy hears about the disagreement at import, not on the first agent run. + _templates_in_force(control, name, effect, resource, compared) @functools.wraps(func) def wrapper(*args: P.args, **kwargs: P.kwargs) -> R: @@ -892,17 +898,27 @@ def wrapper(*args: P.args, **kwargs: P.kwargs) -> R: ) bound = signature.bind(*args, **kwargs) bound.apply_defaults() + # SPEC-v0.2 §3.2 — the Control resolves before the Action, because the policy + # may be the only place a template is declared and `resource` is part of the + # canonical form. The decorator's value wins where both exist. + resolved = control if control is not None else _default_control() + effect_template, resource_template = _templates_in_force( + resolved, name, effect, resource, compared + ) action = Action( name=name, arguments=dict(bound.arguments), principal=invocation.principal, # SPEC-v0.1 §5.1 — `resource` is part of the canonical form, so it resolves # before the Action exists; the effect template resolves against the Action. - resource=None if resource is None else resolve_resource(resource, bound.arguments), + resource=( + None + if resource_template is None + else resolve_resource(resource_template, bound.arguments) + ), environment=invocation.environment, ) - resolved = control if control is not None else _default_control() - effect_key = resolved._resolve_effect(action, effect) + effect_key = resolved._resolve_effect(action, effect_template) if reconcile is not None and effect_key is None and not dangling: # SPEC-v0.2 §2.1 — not a decoration-time error, because the effect template # may come from the policy and that is not loaded yet. A hook with no key to @@ -954,6 +970,55 @@ def executor() -> R: return decorator +def _templates_in_force( + resolved: Control, + name: str, + effect: str | None, + resource: str | None, + compared: list[bool], +) -> tuple[str | None, str | None]: + """The `effect` and `resource` templates this action will use (SPEC-v0.2 §3.2). + + The decorator's value wins where both it and the policy declare one. The policy is a + deployment artefact and the decorator is a statement in the code that will run; when + they disagree, the code is what executes, and silently substituting the policy's + template would change an effect identity without changing a line of the program. + + `compared` is the once-per-decorated-function latch for the warning, because a warning + that repeats per call is a warning nobody reads. + """ + from_policy_effect = resolved.policy.effect_template(name) + from_policy_resource = resolved.policy.resource_template(name) + if not compared: + compared.append(True) + _warn_template_mismatch(name, "effect", effect, from_policy_effect) + _warn_template_mismatch(name, "resource", resource, from_policy_resource) + return ( + effect if effect is not None else from_policy_effect, + resource if resource is not None else from_policy_resource, + ) + + +def _warn_template_mismatch( + name: str, kwarg: str, decorated: str | None, from_policy: str | None +) -> None: + """Name the action, both templates, and which one is in force (SPEC-v0.2 §3.2). + + A warning and not an error: an operator adding templates to a policy for the gateway's + sake must not break a running decorator-based deployment. + """ + if decorated is None or from_policy is None or decorated == from_policy: + return + _LOG.warning( + "%s: %s= is %r in the decorator and %r in the policy; the decorator's is in force, " + "because that is the code that will run. Remove one of them.", + name, + kwarg, + decorated, + from_policy, + ) + + def _reconciler(reconcile: object, reconcile_eagerly: object, where: str) -> _Reconciler: """Validate the pair, wherever one is offered, and hand back this attempt's budget. diff --git a/src/ctrlrun/policy.py b/src/ctrlrun/policy.py index 286194b6..c84dcf6d 100644 --- a/src/ctrlrun/policy.py +++ b/src/ctrlrun/policy.py @@ -1,4 +1,12 @@ -"""Policy loading and rule evaluation to a Decision. Build-list item 2; SPEC-v0.1 §3.""" +"""Policy loading and rule evaluation to a Decision. Build-list item 2; SPEC-v0.1 §3. + +SPEC-v0.2 §3 adds per-action `effect:` and `resource:` templates, because the gateway has no +decorator to carry one. That is the only reason this module imports the template grammar from +`effect.py`: it validates the syntax at load time, so a typo fails at startup rather than +raising `EffectKeyError` at the moment an agent tries to move money. It stays ignorant of +effect *state* — records, transitions and reservations are none of policy's business +(ARCHITECTURE §6). +""" from __future__ import annotations @@ -15,9 +23,21 @@ import yaml from .action import Action -from .errors import PolicyError +from .effect import template_placeholders +from .errors import InvalidArgument, PolicyError +#: The schema `ctrlrun init` writes, and the one every v0.1 file declares. POLICY_SCHEMA: Final = "ctrlrun.policy/v1" + +#: SPEC-v0.2 §3.1 — required by any document using `effect:`, `resource:` or `mcp:`. A v2 +#: file fails to load on v0.1, correctly: v0.1 would ignore the effect template and execute +#: with no duplicate protection at all. The schema string is the only thing standing between +#: those two outcomes, so it is not optional and not inferred. +POLICY_SCHEMA_V2: Final = "ctrlrun.policy/v2" + +#: Both, newest last, for the message an unknown schema produces. +SUPPORTED_SCHEMAS: Final = (POLICY_SCHEMA, POLICY_SCHEMA_V2) + CONFIG_ENV_VAR: Final = "CTRLRUN_CONFIG" DEFAULT_POLICY_FILENAME: Final = "ctrlrun.yaml" @@ -39,9 +59,16 @@ _OPERATORS_BY_LENGTH: Final = tuple(sorted(_OPERATORS, key=len, reverse=True)) _TOP_LEVEL_KEYS: Final = frozenset({"schema", "actions"}) -_ENTRY_KEYS: Final = frozenset({"decision", "rules"}) _RULE_KEYS: Final = frozenset({"when", "decision"}) +#: SPEC-v0.2 §3.1 — the keys `ctrlrun.policy/v2` adds to an action entry. The gateway has no +#: decorator to carry an effect template, so the policy file has to. +_V2_ENTRY_KEYS: Final = frozenset({"effect", "resource", "mcp"}) +_ENTRY_KEYS: Final = frozenset({"decision", "rules"}) | _V2_ENTRY_KEYS + +#: And the closed key set of the `mcp` mapping, which is one key wide. +_MCP_KEYS: Final = frozenset({"not_executed_on_error"}) + #: Names of `Action` fields (SPEC-v0.1 §2.1), which a condition cannot address: conditions #: see the action's *arguments* and nothing else (§3.2). Writing one reads like it scopes a #: rule — `when: { environment_eq: production }` — and matches nothing, so it is refused at @@ -156,10 +183,29 @@ def matches(self, action_name: str, arguments: Mapping[str, Any]) -> bool: return all(condition.matches(action_name, arguments) for condition in self.conditions) +@dataclass(frozen=True) +class McpOptions: + """Per-tool assertions an operator makes about their upstream (SPEC-v0.2 §3.1, §6.8). + + `not_executed_on_error` is the `NotExecuted` of v0.1 §5.5 in YAML: the claim that this + tool reports errors only *before* acting. It defaults to the fail-closed value, so an + `isError: true` the operator has said nothing about is an `AMBIGUOUS` outcome. + """ + + not_executed_on_error: bool = False + + +#: The options an action with no `mcp:` mapping gets. Shared, because it is immutable. +_DEFAULT_MCP_OPTIONS: Final = McpOptions() + + @dataclass(frozen=True) class _ActionPolicy: decision: Decision | None rules: tuple[_Rule, ...] + effect: str | None = None + resource: str | None = None + mcp: McpOptions = _DEFAULT_MCP_OPTIONS def evaluate(self, action_name: str, arguments: Mapping[str, Any]) -> Evaluation: if self.decision is not None: @@ -180,6 +226,7 @@ class Policy: actions: Mapping[str, _ActionPolicy] source: str + schema: str = POLICY_SCHEMA @classmethod def from_file(cls, path: str | os.PathLike[str] | None = None) -> Policy: @@ -209,9 +256,10 @@ def _from_document(cls, document: object, source: str) -> Policy: ) # SPEC: §3.4 — an absent `schema` key is an unknown schema, never "assume v1". schema = document.get("schema") - if schema != POLICY_SCHEMA: + if schema not in SUPPORTED_SCHEMAS: raise PolicyError( - f"{source}: unknown policy schema {schema!r}, expected {POLICY_SCHEMA!r}" + f"{source}: unknown policy schema {schema!r}, expected one of " + f"{', '.join(repr(known) for known in SUPPORTED_SCHEMAS)}" ) # SPEC: §3.1 — key sets are closed, so a typo such as `action:` fails at load # instead of silently denying everything at runtime. @@ -227,8 +275,27 @@ def _from_document(cls, document: object, source: str) -> Policy: for name, entry in entries.items(): if not isinstance(name, str) or not name: raise PolicyError(f"{source}: action names must be non-empty strings, got {name!r}") - actions[name] = _parse_entry(entry, f"{source}: action {name!r}") - return cls(actions=MappingProxyType(actions), source=source) + actions[name] = _parse_entry(entry, f"{source}: action {name!r}", str(schema)) + return cls(actions=MappingProxyType(actions), source=source, schema=str(schema)) + + def effect_template(self, action_name: str) -> str | None: + """This action's `effect:` template, or `None` (SPEC-v0.2 §3.1, §11). + + `None` means the action has no logical effect and gets no reservation — the + documented escape hatch of v0.1 §5.1, and the right answer for a read. + """ + entry = self.actions.get(action_name) + return None if entry is None else entry.effect + + def resource_template(self, action_name: str) -> str | None: + """This action's `resource:` template, or `None` (SPEC-v0.2 §3.1, §11).""" + entry = self.actions.get(action_name) + return None if entry is None else entry.resource + + def mcp_options(self, action_name: str) -> McpOptions: + """This action's `mcp:` options, defaulting to the fail-closed ones (§3.1, §11).""" + entry = self.actions.get(action_name) + return _DEFAULT_MCP_OPTIONS if entry is None else entry.mcp def evaluate(self, action: Action) -> Evaluation: """Decide an action. No side effects; an unlisted action is denied (SPEC-v0.1 §3.4).""" @@ -258,19 +325,30 @@ def _reject_unknown_keys(mapping: Mapping[Any, Any], allowed: Iterable[str], whe ) -def _parse_entry(entry: object, where: str) -> _ActionPolicy: +def _parse_entry(entry: object, where: str, schema: str) -> _ActionPolicy: if not isinstance(entry, Mapping): raise PolicyError( f"{where}: entry must be a mapping with 'decision' or 'rules', got {_type_name(entry)}" ) _reject_unknown_keys(entry, _ENTRY_KEYS, where) + _reject_v2_keys_under_v1(entry, where, schema) has_decision = "decision" in entry has_rules = "rules" in entry if has_decision == has_rules: raise PolicyError(f"{where}: entry must have exactly one of 'decision' or 'rules'") + effect = _parse_template(entry.get("effect"), "effect", where) + resource = _parse_template(entry.get("resource"), "resource", where) + mcp = _parse_mcp(entry.get("mcp"), where) + if has_decision: - return _ActionPolicy(decision=_parse_decision(entry["decision"], where), rules=()) + return _ActionPolicy( + decision=_parse_decision(entry["decision"], where), + rules=(), + effect=effect, + resource=resource, + mcp=mcp, + ) rules = entry["rules"] if not isinstance(rules, list) or not rules: @@ -280,9 +358,67 @@ def _parse_entry(entry: object, where: str) -> _ActionPolicy: rules=tuple( _parse_rule(rule, f"{where} rule[{index}]") for index, rule in enumerate(rules) ), + effect=effect, + resource=resource, + mcp=mcp, ) +def _reject_v2_keys_under_v1(entry: Mapping[Any, Any], where: str, schema: str) -> None: + """A v2 key in a v1 document is a load error naming the key and the schema (§3.1). + + Not a warning, and not silently honoured. A v0.1 kernel reading this file would ignore + the effect template and execute with no duplicate protection at all, so a document that + depends on one has to say so in a way v0.1 refuses. + """ + if schema != POLICY_SCHEMA: + return + used = sorted(key for key in entry if key in _V2_ENTRY_KEYS) + if used: + raise PolicyError( + f"{where}: {', '.join(repr(key) for key in used)} needs " + f"'schema: {POLICY_SCHEMA_V2}'; this document declares {POLICY_SCHEMA!r}, and " + "a v0.1 reader would ignore the template and execute with no duplicate protection" + ) + + +def _parse_template(value: object, kwarg: str, where: str) -> str | None: + """Validate an `effect:` / `resource:` template at load time (SPEC-v0.2 §3.1). + + A malformed template is a `PolicyError` here, not an `EffectKeyError` when an agent + first reaches the action: the same reason `@protect` checks its templates at decoration + time (v0.1 §5.1). The grammar is `effect.py`'s, so the two paths cannot diverge. + """ + if value is None: + return None + if not isinstance(value, str): + raise PolicyError( + f"{where}: {kwarg!r} must be a template string, got {_type_name(value)}" + ) + try: + template_placeholders(value) + except InvalidArgument as exc: + raise PolicyError(f"{where}: {kwarg!r}: {exc}") from exc + return value + + +def _parse_mcp(value: object, where: str) -> McpOptions: + if value is None: + return _DEFAULT_MCP_OPTIONS + if not isinstance(value, Mapping): + raise PolicyError(f"{where}: 'mcp' must be a mapping, got {_type_name(value)}") + _reject_unknown_keys(value, _MCP_KEYS, f"{where}: mcp") + claimed = value.get("not_executed_on_error", False) + # SPEC-v0.2 §3.1 — a bool, and `1` is not one. This is an assertion about a remote that + # CTRLRun cannot check (§6.8), so it is made deliberately or not at all. + if not isinstance(claimed, bool): + raise PolicyError( + f"{where}: mcp: 'not_executed_on_error' must be true or false, " + f"got {_type_name(claimed)}" + ) + return McpOptions(not_executed_on_error=claimed) + + def _parse_decision(value: object, where: str) -> Decision: allowed = ", ".join(member.value for member in Decision) if not isinstance(value, str): diff --git a/tests/test_policy.py b/tests/test_policy.py index 7be4ef52..c853a263 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -605,8 +605,11 @@ def test_disallowed_operand_type_is_a_policy_error(condition: str, operand: str) "text", [ pytest.param("actions:\n customer.read:\n decision: allow\n", id="missing-schema"), + # `ctrlrun.policy/v2` stood here until SPEC-v0.2 §3.1 made it a schema v0.2 loads. + # The rule under test is unchanged — an unsupported schema is refused — so the + # example moves forward to one that is still unsupported. pytest.param( - "schema: ctrlrun.policy/v2\nactions:\n customer.read:\n decision: allow\n", + "schema: ctrlrun.policy/v3\nactions:\n customer.read:\n decision: allow\n", id="future-schema", ), pytest.param( @@ -736,3 +739,176 @@ def test_the_shipped_example_policy_loads_and_evaluates() -> None: assert policy.evaluate(_action(name="customer.read")).decision is Decision.ALLOW assert policy.evaluate(_action(name="iam.grant_admin")).decision is Decision.DENY assert policy.evaluate(_action(name="k8s.delete_namespace")).decision is Decision.APPROVE + + +# --- T16: per-action effect: and resource: templates (SPEC-v0.2 §3) -------------------- + +V2_POLICY = """ +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + effect: "refund:{payment_id}" + resource: "payment:{payment_id}" + mcp: + not_executed_on_error: true + rules: + - when: { amount_gte: 0, amount_lte: 50000 } + decision: allow + - decision: approve + customer.read: + decision: allow +""" + + +def test_T16_a_v2_document_loads_and_exposes_its_templates(): + policy = Policy.from_yaml(V2_POLICY) + + assert policy.effect_template("stripe.refund") == "refund:{payment_id}" + assert policy.resource_template("stripe.refund") == "payment:{payment_id}" + assert policy.mcp_options("stripe.refund").not_executed_on_error is True + + +def test_T16_an_action_without_the_new_keys_has_no_templates(): + """§3.2's documented escape hatch: no `effect:` means no logical effect, and no + reservation. Right for reads, and the reason it is `None` rather than an error.""" + policy = Policy.from_yaml(V2_POLICY) + + assert policy.effect_template("customer.read") is None + assert policy.resource_template("customer.read") is None + assert policy.mcp_options("customer.read").not_executed_on_error is False + + +def test_T16_an_unknown_action_has_no_templates_either(): + policy = Policy.from_yaml(V2_POLICY) + + assert policy.effect_template("nothing.declared") is None + assert policy.resource_template("nothing.declared") is None + assert policy.mcp_options("nothing.declared").not_executed_on_error is False + + +def test_T16_a_v1_document_still_loads_unchanged(): + """A v1 file keeps loading exactly as it did, which is the compatibility half of §3.1.""" + policy = Policy.from_yaml(REFUND_POLICY) + + assert policy.evaluate(_action(arguments={"amount": 400})).decision is Decision.ALLOW + assert policy.effect_template("stripe.refund") is None + + +@pytest.mark.parametrize("key", ["effect", "resource", "mcp"]) +def test_T16_a_key_added_by_v2_is_a_PolicyError_in_a_v1_document(key): + """v0.1 would ignore the template and execute with no duplicate protection at all, so + the schema string is the only thing standing between those outcomes (§3.1).""" + value = '"refund:{payment_id}"' if key != "mcp" else "{ not_executed_on_error: true }" + document = f""" +schema: ctrlrun.policy/v1 +actions: + stripe.refund: + {key}: {value} + decision: allow +""" + with pytest.raises(PolicyError) as raised: + Policy.from_yaml(document) + + assert key in str(raised.value) + assert "ctrlrun.policy/v2" in str(raised.value) + + +@pytest.mark.parametrize("key", ["effect", "resource"]) +@pytest.mark.parametrize("template", ["refund:{payment_id", "refund:{}", "refund:{1st}", ""]) +def test_T16_a_malformed_template_is_a_PolicyError_at_load(key, template): + """At load, not an `EffectKeyError` at runtime (§3.1): a typo must not wait for traffic.""" + document = f""" +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + {key}: "{template}" + decision: allow +""" + with pytest.raises(PolicyError): + Policy.from_yaml(document) + + +@pytest.mark.parametrize("key", ["effect", "resource"]) +def test_T16_a_template_that_is_not_a_string_is_a_PolicyError(key): + document = f""" +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + {key}: 17 + decision: allow +""" + with pytest.raises(PolicyError): + Policy.from_yaml(document) + + +@pytest.mark.parametrize("value", ["yes-please", "1", "null", "[]", "{}"]) +def test_T16_not_executed_on_error_must_be_a_bool(value): + """An operator's claim about their upstream, so it is a claim or it is nothing (§3.1).""" + document = f""" +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + mcp: + not_executed_on_error: {value} + decision: allow +""" + with pytest.raises(PolicyError): + Policy.from_yaml(document) + + +def test_T16_not_executed_on_error_defaults_to_the_fail_closed_value(): + document = """ +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + mcp: {} + decision: allow +""" + assert Policy.from_yaml(document).mcp_options("stripe.refund").not_executed_on_error is False + + +def test_T16_an_unknown_key_under_mcp_is_a_PolicyError(): + document = """ +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + mcp: + not_executed_on_errors: true + decision: allow +""" + with pytest.raises(PolicyError) as raised: + Policy.from_yaml(document) + + assert "not_executed_on_errors" in str(raised.value) + + +def test_T16_mcp_must_be_a_mapping(): + document = """ +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + mcp: true + decision: allow +""" + with pytest.raises(PolicyError): + Policy.from_yaml(document) + + +def test_T16_an_unknown_key_in_an_entry_is_still_a_PolicyError(): + document = """ +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + effekt: "refund:{payment_id}" + decision: allow +""" + with pytest.raises(PolicyError): + Policy.from_yaml(document) + + +def test_T16_an_unknown_schema_names_both_supported_ones(): + with pytest.raises(PolicyError) as raised: + Policy.from_yaml("schema: ctrlrun.policy/v3\nactions: {}\n") + + assert "ctrlrun.policy/v1" in str(raised.value) + assert "ctrlrun.policy/v2" in str(raised.value) diff --git a/tests/test_protect.py b/tests/test_protect.py index 56ad1ca6..e156a68a 100644 --- a/tests/test_protect.py +++ b/tests/test_protect.py @@ -741,3 +741,199 @@ def test_in_memory_store_assigns_event_ids(store): def test_receipt_is_exported_from_the_package(): assert Receipt.__module__ == "ctrlrun.receipt" + + +# --- T16: the decorator wins a template mismatch (SPEC-v0.2 §3.2) ---------------------- + +TEMPLATED_POLICY = """ +schema: ctrlrun.policy/v2 +actions: + stripe.refund: + effect: "refund:{payment_id}" + resource: "payment:{payment_id}" + decision: allow +""" + + +def _templated_control(store=None): + return Control(Policy.from_yaml(TEMPLATED_POLICY), store or InMemoryStateStore()) + + +def test_T16_the_policy_supplies_the_templates_the_decorator_omits(): + control = _templated_control() + + @protect("stripe.refund", control=control) + def refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + with context(agent="refund-agent"): + refund(payment_id="txn_1", amount=200) + + receipt = control.store.receipts()[-1] + assert receipt.effect_key == "refund:txn_1" + assert receipt.resource == "payment:txn_1" + + +def test_T16_a_decorator_and_a_policy_template_produce_the_same_action_hash(): + """The gateway has no decorator (§3.2), so the two paths have to agree exactly.""" + from_policy = _templated_control() + from_decorator = Control( + Policy.from_yaml(TEMPLATED_POLICY.replace("ctrlrun.policy/v2", "ctrlrun.policy/v1") + .replace(' effect: "refund:{payment_id}"\n', "") + .replace(' resource: "payment:{payment_id}"\n', "")), + InMemoryStateStore(), + ) + + @protect("stripe.refund", control=from_policy) + def policy_refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + @protect( + "stripe.refund", + effect="refund:{payment_id}", + resource="payment:{payment_id}", + control=from_decorator, + ) + def decorated_refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + with context(agent="refund-agent"): + policy_refund(payment_id="txn_1", amount=200) + decorated_refund(payment_id="txn_1", amount=200) + + left = from_policy.store.receipts()[-1] + right = from_decorator.store.receipts()[-1] + assert left.action_hash == right.action_hash + assert left.effect_key == right.effect_key == "refund:txn_1" + assert left.resource == right.resource == "payment:txn_1" + + +def test_T16_a_gateway_built_action_hashes_the_same_as_the_decorator_call(): + """§6.6 builds its Action from the policy's templates; this is that construction.""" + from ctrlrun.effect import resolve_effect_key, resolve_resource + + control = _templated_control() + policy = control.policy + arguments = {"payment_id": "txn_1", "amount": 200} + + gateway_action = Action( + name="stripe.refund", + arguments=arguments, + principal=Principal(agent="refund-agent"), + resource=resolve_resource(policy.resource_template("stripe.refund"), arguments), + ) + gateway_key = resolve_effect_key(policy.effect_template("stripe.refund"), gateway_action) + + @protect("stripe.refund", control=control) + def refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + with context(agent="refund-agent"): + refund(payment_id="txn_1", amount=200) + + receipt = control.store.receipts()[-1] + assert receipt.action_hash == gateway_action.action_hash + assert receipt.effect_key == gateway_key + + +@pytest.mark.parametrize( + ("kwarg", "decorated", "expected_key", "expected_resource"), + [ + ("effect", "rf:{payment_id}", "rf:txn_1", "payment:txn_1"), + ("resource", "card:{payment_id}", "refund:txn_1", "card:txn_1"), + ], +) +def test_T16_the_decorator_wins_a_mismatch(kwarg, decorated, expected_key, expected_resource): + """The code that will run is what executes; substituting the policy's template would + change an effect identity without changing a line of the program (§3.2).""" + control = _templated_control() + + @protect("stripe.refund", control=control, **{kwarg: decorated}) + def refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + with context(agent="refund-agent"): + refund(payment_id="txn_1", amount=200) + + receipt = control.store.receipts()[-1] + assert receipt.effect_key == expected_key + assert receipt.resource == expected_resource + + +def test_T16_a_mismatch_warns_once_naming_both_templates(caplog): + control = _templated_control() + + with caplog.at_level(logging.WARNING, logger="ctrlrun"): + + @protect("stripe.refund", effect="rf:{payment_id}", control=control) + def refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + with context(agent="refund-agent"): + refund(payment_id="txn_1", amount=200) + refund(payment_id="txn_2", amount=200) + + warnings = [ + record.getMessage() + for record in caplog.records + if "rf:{payment_id}" in record.getMessage() + ] + assert len(warnings) == 1 + assert "refund:{payment_id}" in warnings[0] + assert "stripe.refund" in warnings[0] + + +def test_T16_the_mismatch_warning_is_emitted_at_decoration_time_with_an_explicit_control(caplog): + """§3.2 — when `control=` is passed the Control resolves at decoration, so the warning + lands at import rather than waiting for the first agent run to surface it.""" + control = _templated_control() + + with caplog.at_level(logging.WARNING, logger="ctrlrun"): + + @protect("stripe.refund", effect="rf:{payment_id}", control=control) + def refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + assert [record for record in caplog.records if "rf:{payment_id}" in record.getMessage()] + + +def test_T16_matching_templates_do_not_warn(caplog): + control = _templated_control() + + with caplog.at_level(logging.WARNING, logger="ctrlrun"): + + @protect( + "stripe.refund", + effect="refund:{payment_id}", + resource="payment:{payment_id}", + control=control, + ) + def refund(payment_id: str, amount: int) -> str: + return f"re_{payment_id}" + + with context(agent="refund-agent"): + refund(payment_id="txn_1", amount=200) + + assert not [record for record in caplog.records if "in force" in record.getMessage()] + + +def test_T16_an_action_with_no_effect_in_policy_gets_no_reservation(): + """§3.2's escape hatch, and it is the right answer for a read.""" + control = _templated_control() + + @protect("customer.read", control=control) + def read(customer_id: str) -> str: + return "ok" + + document = TEMPLATED_POLICY + " customer.read:\n decision: allow\n" + control = Control(Policy.from_yaml(document), InMemoryStateStore()) + + @protect("customer.read", control=control) + def read_again(customer_id: str) -> str: + return "ok" + + with context(agent="support-agent"): + read_again(customer_id="cus_1") + + assert control.store.list_effects() == () + assert control.store.receipts()[-1].effect_key is None From e11f7d6523580e09b09177413d0145cb7237e528 Mon Sep 17 00:00:00 2001 From: arpan Date: Thu, 3 Sep 2026 19:54:51 +0530 Subject: [PATCH 2/2] style: ruff format --- src/ctrlrun/policy.py | 4 +--- tests/test_protect.py | 12 ++++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ctrlrun/policy.py b/src/ctrlrun/policy.py index c84dcf6d..f9fe5b7b 100644 --- a/src/ctrlrun/policy.py +++ b/src/ctrlrun/policy.py @@ -392,9 +392,7 @@ def _parse_template(value: object, kwarg: str, where: str) -> str | None: if value is None: return None if not isinstance(value, str): - raise PolicyError( - f"{where}: {kwarg!r} must be a template string, got {_type_name(value)}" - ) + raise PolicyError(f"{where}: {kwarg!r} must be a template string, got {_type_name(value)}") try: template_placeholders(value) except InvalidArgument as exc: diff --git a/tests/test_protect.py b/tests/test_protect.py index e156a68a..171a1f85 100644 --- a/tests/test_protect.py +++ b/tests/test_protect.py @@ -778,9 +778,11 @@ def test_T16_a_decorator_and_a_policy_template_produce_the_same_action_hash(): """The gateway has no decorator (§3.2), so the two paths have to agree exactly.""" from_policy = _templated_control() from_decorator = Control( - Policy.from_yaml(TEMPLATED_POLICY.replace("ctrlrun.policy/v2", "ctrlrun.policy/v1") - .replace(' effect: "refund:{payment_id}"\n', "") - .replace(' resource: "payment:{payment_id}"\n', "")), + Policy.from_yaml( + TEMPLATED_POLICY.replace("ctrlrun.policy/v2", "ctrlrun.policy/v1") + .replace(' effect: "refund:{payment_id}"\n', "") + .replace(' resource: "payment:{payment_id}"\n', "") + ), InMemoryStateStore(), ) @@ -874,9 +876,7 @@ def refund(payment_id: str, amount: int) -> str: refund(payment_id="txn_2", amount=200) warnings = [ - record.getMessage() - for record in caplog.records - if "rf:{payment_id}" in record.getMessage() + record.getMessage() for record in caplog.records if "rf:{payment_id}" in record.getMessage() ] assert len(warnings) == 1 assert "refund:{payment_id}" in warnings[0]