diff --git a/apps/api/src/cora/api/_agent_decision_signing.py b/apps/api/src/cora/api/_agent_decision_signing.py index a0a873f9bf1..cbc1cadaca6 100644 --- a/apps/api/src/cora/api/_agent_decision_signing.py +++ b/apps/api/src/cora/api/_agent_decision_signing.py @@ -2,17 +2,21 @@ The reactive agent subscribers (RunDebriefer, CautionDrafter) sign each agent-authored `DecisionRegistered` via their own `_maybe_sign` before appending, -per the signing design lock: `DecisionRegistered` is in `SIGNED_EVENT_TYPES`, so -an AI-agent row is signed at write time (an unsigned agent row would trip the -strict audit sweep, `signing.verify_stream(strict=True)` raising -`SignatureMissingError`). Human-actor rows from the operator-driven -`register_decision` slice stay unsigned by design. +per the signing design lock: `DecisionRegistered` is in `SIGNED_EVENT_TYPES`, and +the paths producing agent-attributed rows are exactly the ones the composition +root hands a `Signer`. Human-attributed rows from the operator-driven +`register_decision` slice stay unsigned by design, which is why an audit sweep +passes `verify_stream(must_be_signed=...)` and not a blanket flag: membership in +`SIGNED_EVENT_TYPES` means "signed IF a Signer-wired path produced it", so only +the caller can say which unsigned rows are findings. The proactive agent runtimes hosted at the composition root (`_run_initiator`, -`_run_supervisor`) also compose `DecisionRegistered` directly (they cannot use -`register_decision`, whose decider rejects AGENT principals), so they need the -same signing step. This module hoists it to one helper both import, rather than -each re-declaring a private `_maybe_sign`. Mirrors +`_run_supervisor`) also compose `DecisionRegistered` directly, so they need the +same signing step. They cannot route through `register_decision`: its decider +refuses rows ATTRIBUTED to an agent-kind Actor, and theirs are. Note that is a +guard on the Actor named by `decided_by`, not on the calling principal, whose +kind no decider ever sees. This module hoists the signing step to one helper both +import, rather than each re-declaring a private `_maybe_sign`. Mirrors `RunDebrieferSubscriber._maybe_sign` exactly. """ diff --git a/apps/api/src/cora/decision/features/register_decision/decider.py b/apps/api/src/cora/decision/features/register_decision/decider.py index 042c09a6627..ce883dd7f0a 100644 --- a/apps/api/src/cora/decision/features/register_decision/decider.py +++ b/apps/api/src/cora/decision/features/register_decision/decider.py @@ -37,11 +37,22 @@ `register_decision` is the operator-driven slice. Agent-emitted Decisions go through the subscriber path (CautionDrafter, RunDebriefer) so the Signer port can sign each row per -[[project_signed_events_design]] (SIGNED_EVENT_TYPES = -{DecisionRegistered}, discriminated at write time by -`actor.kind == AGENT`). The decider refuses `context.actor.kind -== AGENT` with `InvalidActorKindForDecisionError` so the slice -cannot become a signing-bypass route. +[[project_signed_events_design]]. + +The signing obligation is discriminated by EVENT TYPE, not by actor +kind. Every signing site gates on `signer is None or +new_event.event_type not in SIGNED_EVENT_TYPES` (= +{DecisionRegistered}); none of them reads `Actor.kind`, and +`tests/architecture/test_actor_kind_blindness.py` pins that. + +The decider refuses `context.actor.kind == AGENT` with +`InvalidActorKindForDecisionError` so this slice does not offer an +unsigned path for rows that the subscriber path would have signed. +The guard reads the kind of the Actor a Decision is ATTRIBUTED to +(`command.decided_by`), never the authenticated principal, which the +Authorize port structurally cannot observe. Note the guard rests on +honest attribution: `decided_by` is body-supplied and is not +reconciled against `principal_id`. ## VO trim semantics diff --git a/apps/api/src/cora/infrastructure/ports/federation/value_types.py b/apps/api/src/cora/infrastructure/ports/federation/value_types.py index 10b47834eb3..0f83251be81 100644 --- a/apps/api/src/cora/infrastructure/ports/federation/value_types.py +++ b/apps/api/src/cora/infrastructure/ports/federation/value_types.py @@ -95,9 +95,18 @@ class SignedOffBy: """Human Signed-off-by attribution; the only DCO arm that closes the chain. Per the Linux-kernel `coding-assistants.rst` invariant transplant, - AI actors cannot Signed-off-by. The federation decider enforces - that `actor_id` resolves to `Actor.kind == human`; an Agent - actor_id raises `DcoChainMissingHumanSignoffError`. + AI actors cannot Signed-off-by. + + NOT ENFORCED TODAY. Nothing checks that `actor_id` resolves to + `Actor.kind == human`: no decider reads the kind, and + `DcoChainMissingHumanSignoffError` does not exist. `check_dco_chain` + verifies only that the chain carries at least one `SignedOffBy` + entry, not that its actor is human, so an agent actor_id passes + today. The human-only rule is design intent per + [[project_federation_port_design]], not a shipped check. Building it + would add the first branch on `Actor.kind` outside the two carve-outs + enumerated in `tests/architecture/test_actor_kind_blindness.py`, which + is where the reason for it belongs. """ actor_id: UUID @@ -125,9 +134,13 @@ class CoDevelopedBy: """Collaborative attribution between TWO HUMAN actors. Mirrors the Linux kernel `CO-DEVELOPED-BY` convention. Both - `actor_id_a` and `actor_id_b` MUST resolve to `Actor.kind == - human`; Agent actor_ids raise `CoDevelopedByForbidsAgentError` - at the decider. + `actor_id_a` and `actor_id_b` are intended to resolve to + `Actor.kind == human`. + + NOT ENFORCED TODAY, same as `SignedOffBy`: no decider reads the + kind and `CoDevelopedByForbidsAgentError` does not exist. Agent + actor_ids pass. Design intent per + [[project_federation_port_design]], not a shipped check. """ actor_id_a: UUID diff --git a/apps/api/src/cora/infrastructure/published_artifact/_stages.py b/apps/api/src/cora/infrastructure/published_artifact/_stages.py index af9ccbf7f8f..bd4a96de3af 100644 --- a/apps/api/src/cora/infrastructure/published_artifact/_stages.py +++ b/apps/api/src/cora/infrastructure/published_artifact/_stages.py @@ -196,9 +196,12 @@ def check_dco_chain(artifact: PublishedArtifact) -> StageResult: Per project_federation_port_design.md: the Linux-kernel `coding-assistants.rst` invariant transplant requires at least - one human Signed-off-by. The decider enforces the deeper - invariant (actor_id resolves to Actor.kind == human) at - publish-time; this gate checks the chain shape at verify-time. + one human Signed-off-by. This gate checks the chain SHAPE only. + + The deeper invariant (actor_id resolves to Actor.kind == human) is + NOT enforced anywhere: no publish-time decider reads the kind. A + `SignedOffBy` naming an agent actor_id passes this gate. See the + `SignedOffBy` docstring in `ports/federation/value_types.py`. """ if not artifact.dco_chain: return StageResult( diff --git a/apps/api/src/cora/infrastructure/signing.py b/apps/api/src/cora/infrastructure/signing.py index 8042c1b4464..6c9e6768ec4 100644 --- a/apps/api/src/cora/infrastructure/signing.py +++ b/apps/api/src/cora/infrastructure/signing.py @@ -58,11 +58,16 @@ - `DecisionRegistered`: produced by Agents AND by humans. Per the design lock the signing requirement applies only to the Agent- - produced rows; the subscriber tier (CautionDrafter and - RunDebriefer today) checks `actor_id` against the Agent BC's - stable-id rule before invoking `Signer.sign`. Human-actor - `DecisionRegistered` rows from the operator-driven - `register_decision` slice stay unsigned. Both AI-agent + produced rows. That split is achieved by WIRING, not by a branch: + the subscriber tier (CautionDrafter and RunDebriefer today) is + handed a `Signer` and signs every row of a type in this set, while + the operator-driven `register_decision` slice is handed none, so + its human-attributed rows stay unsigned. No signing site reads + `Actor.kind`; `tests/architecture/test_actor_kind_blindness.py` + pins that. Membership here therefore means "signed IF a + Signer-wired path produced it", which is why an audit sweep needs + `verify_stream`'s `must_be_signed` predicate to say whether a given + unsigned row is a finding. Both AI-agent subscribers route through this single entry: CautionDrafter emits `DecisionRegistered` with `context="CautionProposal"` (the Caution aggregate itself is created later via the @@ -196,7 +201,7 @@ async def verify_stream( events: Sequence[StoredEvent], *, resolve_public_key: Callable[[str], Awaitable[bytes]], - strict: bool = False, + must_be_signed: Callable[[StoredEvent], Awaitable[bool]] | None = None, ) -> None: """Verify every signed event in a loaded stream. Raise on first failure. @@ -207,34 +212,53 @@ async def verify_stream( - When `event.signature_kid is not None`, calls `verify_signature` with the event's payload, signature, and kid. Raises `SignatureInvalidError` on the first row that fails. - - When `event.signature_kid is None` AND `event.event_type` is in - `SIGNED_EVENT_TYPES` AND `strict=True`, raises - `SignatureMissingError` (audit-mode "no signed event went - unsigned" check). - - When `event.signature_kid is None` and either the event type - is NOT in `SIGNED_EVENT_TYPES` or `strict=False`, the event is - skipped silently. Pre-rollout events and human-actor rows are - legitimately unsigned per the design lock's "AI-agent events - signed, human-actor events not" stance. - - The function is event-type-agnostic at the call signature; the - `SIGNED_EVENT_TYPES` check is the registry-lookup that determines - which rows must carry a signature. - - Kept as a standalone helper rather than an `EventStore.load` flag - so the port stays signing-unaware. Callers wanting opt-in - verification compose: `events, _ = await store.load(...); - await verify_stream(events, resolve_public_key=...)`. - - `strict=True` is the audit-sweep default; production read paths - that just need bytes (projection rebuilds, decider folds) leave it - `False` so they don't pay the verify cost per row. + - When `event.signature_kid is None`, the row is unsigned. Whether + that is a finding or a fact depends on who wrote it, and only the + caller knows: pass `must_be_signed` to say. It is awaited per + unsigned row and raises `SignatureMissingError` when it returns + True. Omit it (the default) and unsigned rows are skipped. + + ## Why the caller owns the obligation rule + + This took a `strict: bool` flag meaning "raise on any unsigned row whose + type is in `SIGNED_EVENT_TYPES`". Event type is the wrong predicate. + `DecisionRegistered` is in that set, but only the rows a `Signer`-wired + path produced carry a signature: an operator recording a human decision + through `register_decision` legitimately produces an unsigned + `DecisionRegistered`, and the old flag raised on it. The docstring said + so, a few lines above the code that did the opposite. + + Event type alone cannot express "should this row have been signed", + because the answer depends on the Actor the row is attributed to. Rather + than teach this module to load Actors and read their kind, which would + put an authorship branch inside signing infrastructure, the obligation + moves to the caller, who already has a store to read and a reason to + care. An audit sweep supplies something like: + + async def _must_be_signed(event: StoredEvent) -> bool: + if event.event_type not in SIGNED_EVENT_TYPES: + return False + actor = await load_actor(store, UUID(event.payload["decided_by"])) + return actor is not None and actor.kind is ActorKind.AGENT + + That predicate is exact in both directions: `register_decision` refuses + agent-attributed rows, so an agent-attributed `DecisionRegistered` can + only have come from a `Signer`-wired path. Reading a kind there is + evidence-checking, not authorization, and no shipped caller needs it yet. + + Kept as a standalone helper rather than an `EventStore.load` flag so the + port stays signing-unaware. Callers wanting opt-in verification compose: + `events, _ = await store.load(...); await verify_stream(events, + resolve_public_key=...)`. + + Production read paths that just need bytes (projection rebuilds, decider + folds) omit `must_be_signed` so they never pay the predicate per row. Raises: - `SignatureInvalidError`: a signed event's signature failed verification (tampering, key rotation drift, key compromise). - - `SignatureMissingError`: only with `strict=True`, an event - whose type is in SIGNED_EVENT_TYPES has no signature. + - `SignatureMissingError`: `must_be_signed` returned True for a row + carrying no signature. """ for event in events: if event.signature_kid is not None: @@ -249,7 +273,7 @@ async def verify_stream( kid=event.signature_kid, resolve_public_key=resolve_public_key, ) - elif strict and event.event_type in SIGNED_EVENT_TYPES: + elif must_be_signed is not None and await must_be_signed(event): raise SignatureMissingError(event.event_type) diff --git a/apps/api/tests/architecture/test_actor_kind_blindness.py b/apps/api/tests/architecture/test_actor_kind_blindness.py new file mode 100644 index 00000000000..6b87bacc1b3 --- /dev/null +++ b/apps/api/tests/architecture/test_actor_kind_blindness.py @@ -0,0 +1,295 @@ +"""Architecture fitness: no principal's authority varies by its own kind. + +An autonomous agent and a human operator are the same kind of actor to the +system: authority is decided over a bare principal, and nothing asks whether +that principal is a machine. That claim is load-bearing, so this test locks +it rather than leaving it to prose and discipline. + +The distinction the whole file turns on is SUBJECT kind vs CALLER kind. + + - CALLER kind is the kind of the authenticated principal making a request. + Nothing reads it, and Rule 2 makes that structural rather than merely + observed: `Authorize.authorize` cannot receive a kind, and `Policy` + carries no kind field, so the policy decision point cannot see it even + in principle. A caller-kind branch is what would falsify the claim. + - SUBJECT kind is the kind of an Actor a command NAMES: the Actor being + minted, the Actor a Decision is attributed to. Reading it decides what a + command means, not what its caller may do. Two such reads exist and both + are enumerated below. + +Sibling to `test_actor_kind_sync.py`, which pins the kind VOCABULARY across +DTO Literals and the SQL CHECK. This one pins the USAGE. + +## Rule 1: every `ActorKind` branch is an enumerated carve-out + +A comparison against `ActorKind`, or against one of its bare string values +on a `kind`-named operand, is a branch. Both that exist today read subject +kind, and each is enumerated with its reason in +`_ACTOR_KIND_BRANCH_ALLOWLIST`. The discovered set is asserted equal to the +allowlist in BOTH directions, so a new branch fails as drift-in and a +deleted branch fails as a stale entry. A ratchet would only catch the first. + +Enum definitions, type annotations, `ActorKind(payload["kind"])` decode +calls, constructor kwargs, wire Literals and projection columns need no +carve-out: they carry the value without deciding on it, so the rule never +sees them. + +## Rule 2: the Authorize port cannot observe kind + +The structural claim the others rest on. `Authorize.authorize` takes +`principal_id`, `command_name`, `conduit_id`, `surface_id`. Kind is not a +parameter, so the policy decision point cannot branch on it even in +principle, and `Policy` carries no kind field to branch with. Widening that +signature would falsify the claim silently, so the parameter set is pinned. + +## Rule 3: nothing that produces a signature reads kind + +Evidentiary obligation is kind-aware by design (an agent's identity is +key-backed; a human's is a session), but it is carried by the event +vocabulary and by which paths are wired with a `Signer`, never by a branch +inside signing code. + +The rule keys on files that CALL `.sign(...)`, not on files importing +`SIGNED_EVENT_TYPES`. The registry names which event types are signature +targets and is legitimately read by the verify side too, so importing it +does not make a file a signing site: `signing.py` DEFINES the set (and so +never imports it), while a read-side obligation predicate would import it +while signing nothing. Verification reading a subject's kind to decide +whether an absent signature is an anomaly is evidence-checking, not a gate, +and this rule deliberately leaves it free. + +Free of THIS rule, that is. Rule 1 still sees it and still wants it named in +the allowlist with a reason, because the allowlist is the ledger of every +place kind matters. The division is: Rule 1 says enumerate and justify; Rule +3 says that inside signing code no justification is accepted. +""" + +import ast +from pathlib import Path + +import pytest + +from tests.architecture.conftest import CORA_ROOT, tracked_python_files + +# Deliberate reads of a SUBJECT's kind. Each entry names why the branch does +# not partition principals by kind. Keyed by `::` rather than `path:line` so the entry does not go stale silently +# when unrelated lines move. +# +# A CALLER-kind branch does not belong here and is not allowlistable: it +# falsifies the claim outright. There are none, and Rule 2 keeps it that way +# structurally. A third SUBJECT-kind entry is worth a design look, not an +# automatic rewrite; moving a subject-kind read into Policy data would be a +# category error, since Policy governs what a CALLER may do. +_ACTOR_KIND_BRANCH_ALLOWLIST: dict[str, str] = { + "access/features/register_actor/decider.py::decide": ( + "Genesis guard on the minted subject, not on the caller. Reads `command.kind`, " + "the kind of the Actor being created, never the kind of the acting principal, " + "which this decider never sees. Routes agent-kind Actor genesis through " + "define_agent's cross-BC atomic write so the (Agent.id == Actor.id) lock holds. " + "The refusal is identical for every caller: no principal's authority, allowance, " + "obligation, or consequence changes by kind." + ), + "decision/features/register_decision/decider.py::decide": ( + "Attribution guard, not an authorization gate. Reads the kind of the Actor named " + "by `command.decided_by`, the attribution target; the authorization decision was " + "already made on `principal_id` by a port whose signature cannot carry kind (see " + "test_authorize_port_signature_omits_actor_kind). Refuses agent-attributed rows " + "on the unsigned operator route, so that route offers no unsigned path for rows " + "the Signer-wired subscriber path would have signed. Agents keep full capability " + "to record Decisions through that signed path, so this costs them no authority." + ), +} + +# The parameters the policy decision point is allowed to see. Kind is absent +# on purpose: the PDP cannot branch on what it never receives. +_AUTHORIZE_PARAMS: frozenset[str] = frozenset( + {"principal_id", "command_name", "conduit_id", "surface_id"} +) + +_ACTOR_KIND_VALUES: frozenset[str] = frozenset({"human", "agent", "service_account"}) + + +def _is_actor_kind_member(node: ast.expr) -> bool: + """`ActorKind.AGENT`, `ActorKind.HUMAN`, `ActorKind.SERVICE_ACCOUNT`.""" + return ( + isinstance(node, ast.Attribute) + and isinstance(node.value, ast.Name) + and node.value.id == "ActorKind" + ) + + +def _is_kind_operand(node: ast.expr) -> bool: + """An operand plausibly holding a kind: `actor.kind`, `kind`, `principal_kind`.""" + if isinstance(node, ast.Attribute): + return node.attr == "kind" or node.attr.endswith("_kind") + if isinstance(node, ast.Name): + return node.id == "kind" or node.id.endswith("_kind") + return False + + +def _is_actor_kind_constant(node: ast.expr) -> bool: + return isinstance(node, ast.Constant) and node.value in _ACTOR_KIND_VALUES + + +class _KindBranchVisitor(ast.NodeVisitor): + """Collect the qualnames of scopes that branch on an actor kind.""" + + def __init__(self) -> None: + self._scope: list[str] = [] + self.hits: set[str] = set() + + def _descend(self, name: str, node: ast.AST) -> None: + self._scope.append(name) + self.generic_visit(node) + self._scope.pop() + + def visit_FunctionDef(self, node: ast.FunctionDef) -> None: + self._descend(node.name, node) + + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: + self._descend(node.name, node) + + def visit_ClassDef(self, node: ast.ClassDef) -> None: + self._descend(node.name, node) + + def _record(self) -> None: + self.hits.add(".".join(self._scope) if self._scope else "") + + def visit_Compare(self, node: ast.Compare) -> None: + operands = [node.left, *node.comparators] + member_compare = any(_is_actor_kind_member(o) for o in operands) + value_compare = any(_is_kind_operand(o) for o in operands) and any( + _is_actor_kind_constant(o) for o in operands + ) + if member_compare or value_compare: + self._record() + self.generic_visit(node) + + def visit_Match(self, node: ast.Match) -> None: + # `match actor.kind: case ActorKind.AGENT:` is not a Compare node. + if _is_kind_operand(node.subject): + for case in node.cases: + if any( + _is_actor_kind_member(sub) or _is_actor_kind_constant(sub) + for sub in ast.walk(case.pattern) + if isinstance(sub, ast.expr) + ): + self._record() + break + self.generic_visit(node) + + +def _branch_sites() -> frozenset[str]: + """`::` for every actor-kind branch in `src/cora`.""" + found: set[str] = set() + for path in tracked_python_files(): + visitor = _KindBranchVisitor() + visitor.visit(ast.parse(path.read_text())) + rel = path.relative_to(CORA_ROOT).as_posix() + found.update(f"{rel}::{qualname}" for qualname in visitor.hits) + return frozenset(found) + + +def _authorize_protocol_params() -> frozenset[str]: + tree = ast.parse((CORA_ROOT / "infrastructure" / "ports" / "authorize.py").read_text()) + for node in ast.iter_child_nodes(tree): + if not (isinstance(node, ast.ClassDef) and node.name == "Authorize"): + continue + for member in node.body: + if isinstance(member, ast.AsyncFunctionDef | ast.FunctionDef) and ( + member.name == "authorize" + ): + args = member.args + declared = (*args.posonlyargs, *args.args, *args.kwonlyargs) + return frozenset(a.arg for a in declared if a.arg != "self") + raise AssertionError("Authorize Protocol declares no `authorize` method") + + +def _signing_sites() -> list[Path]: + """Files that PRODUCE a signature: they call `.sign(...)`. + + Keying on the call rather than on a `SIGNED_EVENT_TYPES` import matters in + both directions. The two subscriber sites take their `Signer` by + constructor and never import the port, so an import-keyed rule would miss + them; and `signing.py` defines the registry rather than importing it, so an + import-keyed rule would exempt the very file holding the verifier while + catching an innocent read-side predicate that imports the registry and + signs nothing. + """ + sites: set[Path] = set() + for path in tracked_python_files(): + text = path.read_text() + if ".sign(" not in text: + continue + for node in ast.walk(ast.parse(text)): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr == "sign" + ): + sites.add(path) + break + return sorted(sites) + + +@pytest.mark.architecture +def test_actor_kind_branch_outside_allowlist_is_rejected() -> None: + undocumented = sorted(_branch_sites() - frozenset(_ACTOR_KIND_BRANCH_ALLOWLIST)) + assert not undocumented, ( + "New branch(es) on Actor.kind in governance machinery:\n " + + "\n ".join(undocumented) + + "\n\nEvery gate decides over a bare principal; branching on kind partitions " + "principals into humans and machines and breaks that. If this branch genuinely " + "does not change any principal's authority, allowance, obligation, or " + "consequence, add it to `_ACTOR_KIND_BRANCH_ALLOWLIST` with a reason saying why. " + "Note the rule-of-three trigger recorded on the register_decision entry: a third " + "carve-out means the discriminator belongs in Policy data, not in machinery." + ) + + +@pytest.mark.architecture +def test_actor_kind_allowlist_entry_without_branch_is_rejected() -> None: + stale = sorted(frozenset(_ACTOR_KIND_BRANCH_ALLOWLIST) - _branch_sites()) + assert not stale, ( + "Allowlist entr(ies) with no matching branch in the tree:\n " + + "\n ".join(stale) + + "\n\nThe branch was removed or its enclosing function was renamed. Delete the " + "entry so the allowlist keeps reading as the true, complete list of exceptions." + ) + + +@pytest.mark.architecture +def test_authorize_port_signature_omits_actor_kind() -> None: + assert _authorize_protocol_params() == _AUTHORIZE_PARAMS, ( + f"Authorize.authorize parameters are {sorted(_authorize_protocol_params())}, " + f"expected {sorted(_AUTHORIZE_PARAMS)}.\n\n" + "The policy decision point cannot branch on what it never receives, and that is " + "the structural reason authorization is kind-blind rather than merely " + "disciplined. Widening this signature to carry an actor kind would make the " + "claim false. Narrowing it is fine; update this pin." + ) + + +@pytest.mark.architecture +@pytest.mark.parametrize( + "path", + _signing_sites(), + ids=lambda p: p.relative_to(CORA_ROOT).as_posix(), +) +def test_signing_site_does_not_read_actor_kind(path: Path) -> None: + reads = sorted( + f"{node.value.id}.{node.attr}" + for node in ast.walk(ast.parse(path.read_text())) + if isinstance(node, ast.Attribute) + and isinstance(node.value, ast.Name) + and node.value.id == "ActorKind" + ) + assert not reads, ( + f"{path.relative_to(CORA_ROOT).as_posix()} produces a signature and reads ActorKind:\n " + + "\n ".join(reads) + + "\n\nWhich rows carry a signature is settled by the event vocabulary " + "(SIGNED_EVENT_TYPES) and by which paths are wired with a Signer, never by a " + "branch inside signing code. Verification may read a subject's kind to decide " + "whether an absent signature is an anomaly; producing one may not." + ) diff --git a/apps/api/tests/unit/infrastructure/test_signing.py b/apps/api/tests/unit/infrastructure/test_signing.py index a85bc81f12b..fd57be8ffaa 100644 --- a/apps/api/tests/unit/infrastructure/test_signing.py +++ b/apps/api/tests/unit/infrastructure/test_signing.py @@ -455,6 +455,15 @@ def _stored( ) +async def _always_required(_event: StoredEvent) -> bool: + """Obligation rule that demands a signature on every unsigned row. + + Stands in for a real audit rule, which reads the Actor a row is + attributed to. Tests that care only about the raise path use this. + """ + return True + + @pytest.mark.unit @pytest.mark.asyncio async def test_verify_stream_accepts_empty_sequence() -> None: @@ -465,15 +474,15 @@ async def _resolver(kid: str) -> bytes: calls.append(kid) return b"\x00" * 32 - await verify_stream([], resolve_public_key=_resolver, strict=True) + await verify_stream([], resolve_public_key=_resolver, must_be_signed=_always_required) assert calls == [] @pytest.mark.unit @pytest.mark.asyncio -async def test_verify_stream_skips_unsigned_events_in_non_strict_mode() -> None: - """Default non-strict mode: unsigned events pass through silently - (legitimate pre-rollout + human-actor rows).""" +async def test_verify_stream_without_obligation_rule_skips_unsigned_events() -> None: + """No `must_be_signed`: unsigned rows pass through silently (pre-rollout + rows, and rows no Signer-wired path produced).""" events = [ _stored(event_type="DecisionRegistered"), # unsigned, in SIGNED_EVENT_TYPES _stored(event_type="RunStarted"), # unsigned, not in SIGNED_EVENT_TYPES @@ -490,33 +499,69 @@ async def _resolver(kid: str) -> bytes: @pytest.mark.unit @pytest.mark.asyncio -async def test_verify_stream_strict_mode_raises_on_unsigned_signed_event_type() -> None: - """Strict audit mode: an event with type in SIGNED_EVENT_TYPES that - lacks a signature is the canary for a misconfigured signer (no - signed event should ever land without a signature). Raises - SignatureMissingError so audit dashboards surface the gap.""" - events = [_stored(event_type="DecisionRegistered")] # in SIGNED_EVENT_TYPES, unsigned +async def test_verify_stream_raises_when_obligation_rule_requires_a_signature() -> None: + """An unsigned row the caller's rule says should have been signed is the + canary for a stripped signature or a misconfigured signer.""" + events = [_stored(event_type="DecisionRegistered")] async def _resolver(_kid: str) -> bytes: return b"\x00" * 32 with pytest.raises(SignatureMissingError) as exc_info: - await verify_stream(events, resolve_public_key=_resolver, strict=True) + await verify_stream(events, resolve_public_key=_resolver, must_be_signed=_always_required) assert exc_info.value.event_type == "DecisionRegistered" @pytest.mark.unit @pytest.mark.asyncio -async def test_verify_stream_strict_mode_passes_unsigned_non_signed_type() -> None: - """Strict mode is a per-event check; events whose type is NOT in - SIGNED_EVENT_TYPES pass through unsigned even with strict=True - (they're legitimately unsigned by design).""" - events = [_stored(event_type="RunStarted")] +async def test_verify_stream_honours_obligation_rule_declining_a_row() -> None: + """The regression the `strict: bool` flag could not express. + + A human-recorded `DecisionRegistered` is legitimately unsigned even though + its type is in SIGNED_EVENT_TYPES. The old flag keyed on event type alone + and raised on exactly this row; a caller-supplied rule declines it. + """ + events = [_stored(event_type="DecisionRegistered")] + seen: list[str] = [] async def _resolver(_kid: str) -> bytes: return b"\x00" * 32 - await verify_stream(events, resolve_public_key=_resolver, strict=True) + async def _only_agent_rows(event: StoredEvent) -> bool: + seen.append(event.event_type) + return False # stands in for "this row is attributed to a human" + + await verify_stream(events, resolve_public_key=_resolver, must_be_signed=_only_agent_rows) + assert seen == ["DecisionRegistered"] # the rule was consulted, and declined + + +@pytest.mark.unit +@pytest.mark.asyncio +async def test_verify_stream_does_not_consult_obligation_rule_for_signed_rows() -> None: + """A row carrying a signature is verified, never asked about.""" + signer = Ed25519PrivateKey.generate() + payload = {"x": 1} + signature = _sign_with_ed25519("DecisionRegistered", payload, signer) + pub = _public_bytes(signer) + events = [ + _stored( + event_type="DecisionRegistered", + payload=payload, + signature=signature, + signature_kid="kid-A", + ) + ] + consulted: list[str] = [] + + async def _resolver(_kid: str) -> bytes: + return pub + + async def _rule(event: StoredEvent) -> bool: + consulted.append(event.event_type) + return True + + await verify_stream(events, resolve_public_key=_resolver, must_be_signed=_rule) + assert consulted == [] @pytest.mark.unit @@ -584,27 +629,45 @@ async def _resolver(_kid: str) -> bytes: @pytest.mark.unit @pytest.mark.asyncio -async def test_verify_stream_mixed_signed_and_unsigned_in_strict_mode() -> None: - """Realistic audit replay: a stream contains both pre-rollout - unsigned events (event_type not in SIGNED_EVENT_TYPES) and newly - signed Agent-emitted events. Strict mode verifies the signed - ones and allows the unsigned non-signed-type rows.""" +async def test_verify_stream_audit_replay_over_a_realistic_mixed_stream() -> None: + """Realistic audit replay over every row shape a live stream carries. + + Pre-rollout rows whose type is not a signature target, a signed + agent-attributed Decision, and a human-attributed Decision that is + legitimately unsigned. That last row is the regression: its type IS in + SIGNED_EVENT_TYPES, so the old `strict: bool` flag raised on it even + though no Signer-wired path produced it and none should have. An + obligation rule that looks at attribution, as a real audit caller would, + tells the two Decisions apart. + """ signer = Ed25519PrivateKey.generate() - payload = {"x": 1} - signature = _sign_with_ed25519("DecisionRegistered", payload, signer) + agent_id = uuid4() + human_id = uuid4() + agent_payload = {"decided_by": str(agent_id), "choice": "NominalCompletion"} + signature = _sign_with_ed25519("DecisionRegistered", agent_payload, signer) pub = _public_bytes(signer) + + async def _agent_attributed(event: StoredEvent) -> bool: + if event.event_type not in SIGNED_EVENT_TYPES: + return False + return event.payload.get("decided_by") == str(agent_id) + events = [ - _stored(event_type="RunStarted"), # legitimately unsigned + _stored(event_type="RunStarted"), # not a signature target _stored( - event_type="DecisionRegistered", - payload=payload, + event_type="DecisionRegistered", # agent, signed + payload=agent_payload, signature=signature, signature_kid="kid-A", ), - _stored(event_type="RunCompleted"), # legitimately unsigned + _stored( + event_type="DecisionRegistered", # human, correctly unsigned + payload={"decided_by": str(human_id), "choice": "OperatorAbort"}, + ), + _stored(event_type="RunCompleted"), # not a signature target ] async def _resolver(_kid: str) -> bytes: return pub - await verify_stream(events, resolve_public_key=_resolver, strict=True) + await verify_stream(events, resolve_public_key=_resolver, must_be_signed=_agent_attributed)