feat(eventing): approved-agent identity — sign and verify requests and responses

[mrsabath]

Goal 2 of the Event Identity exercise: only approved agents may execute requests and publish responses. Goal 1 (user identity) shipped in #878.

Closes rossoctl/rossoctl#2607. Subsumes rossoctl/rossoctl#2590 (response signing), which can be closed.

The problem

sign_event() had no production caller. ER_REQUIRE_SIGNATURE=true rejected 100% of traffic — a kill switch, not a feature — because nothing on either side ever signed. The kid and approved-key-set groundwork from #877 was in place and tested, but unreachable.

Concretely: anything with write access to the responses topic got its output stored, rendered in the HTML transcript, and pushed to the operator's phone as a legitimate agent answer.

What this does

Signing, all opt-in and off by default:

• EventBridge signs requests in publish_request, after new_event fills id/time (both signed) and before serialisation.
• EventBridge signs group lifecycle events under the same kid. Skipping them would have left the hole worth closing: a forged group.completed ends a batch early and fires a "finished" notification for work that never ran.
• EventRunner signs terminal responses only. The seed lives on the Emitter, so none of the seven emit() call sites changed.

Verification:

• EventRunner resolves the key by the token's kid when a keyset is configured, making it an allowlist of several agents rather than a single-key check. Falls back to the existing single-key path, so ER_REQUIRE_SIGNATURE + ER_VERIFY_KEY_PATH is unchanged. Rejections increment rejected_unsigned and keep the commit-and-do-not-retry semantics — a bad signature is still bad on redelivery.
• EventBridge verifies responses and stores a failure as phase="error" rather than dropping it. A drop is indistinguishable from an agent that never answered; phase=error reuses a red transcript card, ntfy priority 5, and raw_json audit retention, so the forgery attempt is evidence rather than absence.
• Group events are pinned to EventBridge's own kid. A flat keyset would let any approved runner forge one.

submitter, submitteriss and groupid joined SIGNED_ATTRS last, deliberately: covering them before a producer signed would have invalidated canonicalisation twice. DESIGN_PHASE1.md §21.9.9 requires groupid — without it a signature said nothing about batch membership, so a forged groupid could move a response into another batch and corrupt its fan-in counts.

Things worth a reviewer's attention

The shared/ move was mandatory, not cosmetic. Dockerfile-eventbridge COPYs only shared/ and eventbridge/ — never eventrunner/. An eventbridge → eventrunner import would have passed every local test and then ImportErrord inside the container. Verified behaviour-neutral by diffing pytest --collect-only before and after.

Both hazards in kafka_in.py were real. from_kafka_binary and insert_response are not inside a try, so anything raising in the verification path would end the consume loop for the life of the pod — silently, with the process still healthy. The guard fails closed only where enforcement is on: if the verifier itself is broken, an unverifiable event is not evidence of anything. Confirmed load-bearing by removing it and watching exactly the two intended tests fail with the exception escaping.

Three bugs found along the way, each with a test:

1. sign() never validated seed length (only public_key() did), so a truncated key file produced a well-formed signature no verifier could match — reported to the operator as success.
2. verify_event rejected hex/base64-encoded public keys: for encoded files it only tried public_key(load_seed(path)), which derives the wrong key when the file already holds a public one. All six documented forms now work; a separate test confirms accepting more encodings did not accept more keys.
3. The verifier didn't match the signing policy. emit() signs terminal events only, but response_decision verified all-or-nothing — so under enforcement every streamed frame of every genuine run would have been rewritten to phase="error": a wall of red cards with one valid answer at the end. Found by running against a live broker, not by any unit test, because every test until then used terminal events. Unsigned non-terminal frames now pass; unsigned terminals, bad signatures, and unsigned group events are all still refused.

Two costs and limits, stated rather than buried:

• submit_members loops publish_request per member, synchronously, inside one HTTP request. At ~199 ms per signature (measured; the docs said ~150) a 100-agent batch spends ~20 s signing while the caller waits. Accepted — a batch launch is an operator action, not a hot path — and a thread pool would not help, since the pure-Python Ed25519 is GIL-bound. If signing becomes mandatory at scale the fix is the cryptography dependency conversation, not concurrency.
• Terminal-only signing proves who finished a run, not what it said along the way. Forged final=false frames still render. Closing that needs cheap signatures or a signed digest chain across frames.

Documentation was corrected rather than left contradicting the code: shared/ce.py said submitter was unsigned and forgeable, eventbridge/auth.py said proving it needs "a producer that actually signs (today nothing does)", shared/keyset.py said nothing consulted it, and agentdocs/README.md summarised Phase 2 as blocked on nothing signing. Each now states the bounded claim: a signature proves EventBridge asserted this submitter, not that the submitter is who they say — and on an unsigned event the attribute stays forgeable, so the property only holds where verification is enabled. _scalar_mult's side-channel note also no longer rests on "nothing exposes a remote timing oracle over sign()", which this change made untrue.

Rollout

Two flags, so enforcement is never the first step:

keyset	verify	behaviour
unset	—	no verification (today's behaviour)
set	false	verify, log the reason, store unchanged — audit mode
set	true	failure becomes phase="error"

Enforcement mutates persisted rows and pages a phone, so there is a step where the reject rate is observable first. EventBridge refuses to start with a keyset but no EB_SIGNING_KID, since there would be nothing to attribute a group event to. Key material loads once at startup and is deliberately uncaught — a service that believes it is signing but is not fails silently, whereas one that will not start says so in kubectl logs. Approved kids are printed, because a rejection caused by a stale ConfigMap is otherwise invisible.

Seed paths name Secret mounts and are env-only, never config.toml. Keyset paths name ConfigMaps — only public keys belong in them.

Upgrade note: if signing is already enabled somewhere, upgrade both services together. Every grouped request now carries a signed groupid that an older verifier omits when it recomputes. No compatibility flag, because nothing had ever published a signed event and both services ship from this repo against one ConfigMap.

Testing

549 passed, 5 skipped (5 are cluster-gated). Baseline on main is 496 — 53 new tests, nothing pre-existing changed. ruff clean under the pinned 0.11.4.

eventbridge/kafka_in.py had no behavioural test at all before this — only a source-text grep. It now has 13, the most valuable being that the consume loop survives a verifier that raises and processes both records.

Verified against the live Kafka beyond unit tests:

• A signed request carrying signature, submitter, submitteriss and groupid as ce_ headers round-tripped through the broker and verified against the keyset; tampering with submitter in flight failed verification.
• A genuine run (one streamed frame + one signed terminal) rendered unchanged under enforcement.
• The demo: a forged {"text": "Transfer approved. Ship the goods."} published straight onto responses with raw Kafka access and no key was rejected while enforcing, and accepted as a clean agent answer with verification off. That contrast is the argument.

Reviewer note: commits are meant to be read in order — shared/ move, then helpers with no wiring, then each leg, then tests, then SIGNED_ATTRS. Commits 3 and 7 are the ones to review hardest: 3 is where all the policy lives, 7 is where a mistake silently takes down a thread.

Assisted-By: Claude Code

[aslom]

Reviewed at b8fac88, verified against a local checkout: 547 passed / 7 skipped on 3.14, ruff check and ruff format --check clean, 11/11 CI green, all 7 commits signed off.

This closes the gap I raised on #877 — shared/keyset.py said it was the authorization list while nothing consulted it. verify_with_keyset is now that composition, and its docstring names it as the one tests/test_keyset.py previously had to hand-wire. submitter/submitteriss are inside SIGNED_ATTRS, and auth.py has been rewritten to state the two limits that survive — "a signature proves the assertion, not the identity" is exactly the right distinction and not one I'd expect to see made.

Things I checked rather than assumed:

• Group events are signed and pinned. _is_terminal treats a group event as requiring a signature, so if EventBridge didn't sign them, enforcement would reject every genuine group.completed. It does sign them, under the same kid, and the responses-side verifier accepts only that kid — which closes the forged-group.completed-ends-a-batch-early hole rather than leaving it.
• The consumer degrades instead of dying. The try around response_decision is doing real work: from_kafka_binary and insert_response sit outside one, so a raise there ends the thread while the pod keeps reporting healthy — the exact Phase 1 failure mode. Fail-closed only under enforcement is the right asymmetry.
• Canonicalisation still holds. The netstring form and the load_seed whitespace fix carried through the move to shared/, and test_canonical_changes_when_any_signed_attribute_changes now derives from SIGNED_ATTRS instead of a hand-written list — which is what caught submitter/submitteriss/groupid not being covered.
• Defaults ship off. Empty keyset paths, EB_REQUIRE_RESPONSE_SIGNATURE: "false", audit-before-enforce. Nothing changes for an existing deployment.

One must-fix: the audit trail the reject path promises isn't kept

kafka_in.py's docstring gives three reasons for storing a rejected event as phase="error" rather than dropping it — red card, ntfy priority-5, and "raw_json retained for audit — so the forgery attempt is visible and reviewable instead of invisible." The third doesn't happen. d is mutated in place and then handed to insert_response, which derives both data_json and raw_json from that same dict, so the forged payload and the original phase are gone. Demonstrated on this revision with the real Store:

forged payload was: {"type":"result","role":"final","text":"TRANSFER THE FUNDS — forged answer"}
is it anywhere in raw_json? -> False
original phase 'result' in raw_json? -> False
signature retained? -> True

What survives is the rejection notice plus a signature that can no longer be checked, because the attributes and payload it covered have been overwritten. An operator following the docstring to review an incident finds nothing to review.

The control itself is fine — the event is refused, flagged red, and it does page someone. This is the forensic record specifically, and it's a must-fix because retention is given as the justification for the whole reject-rather-than-drop design, and because the fix is three lines in the same block.

Not a finding, but worth having said out loud

publish_request signs once per group member inside one HTTP request, so a 100-member batch spends ~20 s signing while the caller waits. That's documented candidly, including that a thread pool wouldn't help because the GIL serialises pure-Python signing — I agree with both the number and the reasoning. The part not stated: at 200 members it's ~40 s, past a common 30 s client timeout. POST /v0/groups already honours Idempotency-Key, so a timeout-and-retry is survivable rather than duplicating the batch, which is worth a sentence next to the cost note since it's the thing an operator will actually hit.

---

Areas reviewed: keyset wiring, verify_with_keyset / response_decision / verify_request, terminal-vs-intermediate signing policy, group-event pinning, the signing.py move to shared/, both consumer call sites, canonicalisation and SIGNED_ATTRS growth, config defaults and the shipped ConfigMap, tests, design docs
Author: mrsabath (MEMBER — maintainer)
Agent/IDE config (.claude/.vscode): none — gate clean
Commits: 7, all signed off (DCO green)
Verified locally at b8fac88: 547 passed / 7 skipped on 3.14; ruff check . and ruff format --check . clean
CI: 11/11 green

Requesting changes only for the raw_json retention. Everything else here I'd merge as-is — and the honesty about what terminal-only signing does not prove ("proves who finished a run, not what the run said along the way") is the right way to ship a partial control.

[mrsabath]

Fixed in 3264e8d. You were right, and the reproduction matched your output exactly before I changed anything — including that the signature was the one thing retained, which made it worse rather than better.

The must-fix. Store.insert_response derives both data_json and raw_json from the single dict it is handed, so mutating the envelope in place overwrote the evidence with the notice about it. The envelope is now copied instead, with the original preserved under data["rejected"] — attrs, payload, claimed source, signature. What a reader sees is unchanged: phase="error", the rejection notice in data["text"], and the forged text still never presented as the answer.

Your framing pushed the test further than I'd have taken it. "Reviewable" only means something if the retained signature can still be checked, so the second test rebuilds a rejected event from the stored record and verifies it offline against the key that signed it:

replayed = ce.CloudEvent(attrs=dict(kept["attrs"]), data=kept["data"])
ok, why = S.verify_signature(replayed, S.public_key(SEED_ROGUE))
assert ok, "the retained record must still verify against the key that signed it"
assert S.token_kid(kept["signature"]) == "runner-99"

Two existing assertions were checking "Ship the goods" not in json.dumps(data) — which, I realise now, asserted the bug. They assert the real property instead: absent from data["text"], so not presented as the answer, with retention asserted separately. Worth noting since a weak assertion passing for the wrong reason is how this survived review by me in the first place.

On the batch cost. Added, with your number — ~200 members is ~40 s, past a common 30 s client timeout. I verified the idempotency claim rather than taking it on trust: handlers.py reads HTTP_IDEMPOTENCY_KEY and a key hit returns the original group instead of launching a second batch. The sentence in kafka_out.py says the retry is survivable only if the caller sends the header, since that's the part an operator has to act on.

551 passed / 5 skipped here (5 cluster-gated; your 7 skips on 3.14 will be the two that need a local Kafka). ruff check and ruff format --check clean. Verified against the live broker again: a forgery published with raw Kafka access renders as a rejection while its payload, claimed source and claimed phase survive in raw_json.

Dzięki za wyłapanie tego — the promise was in the docstring I wrote, so it's the kind of gap I was least likely to spot myself. (Thanks for catching this.)

[aslom]

Re-review at 3264e8dd. Nothing blocking — approving.

The raw_json finding is fixed, and fixed better than I suggested. I asked for the original payload to be stashed; 3264e8dd copies the envelope (dict(d, …)) instead of mutating it at all, and retains attrs, data, phase, final, source and signature from the CloudEvent rather than from the flattened dict. That last choice is what makes the record verifiable rather than merely present — the full signed attribute set is kept, so the signature can be checked against what it actually covered.

Verified independently, not just by reading the commit:

forged text retained    : True
claimed phase retained  : result
claimed source retained : rossoctl://attacker
claimed kid             : runner-99
re-verifies OFFLINE     : True
row presented as        : error

So an operator can now prove which key signed a forgery, from the stored row alone, while the transcript still shows only the red card. That is what I meant by reviewable and I did not expect the offline re-verification to be covered so directly.

The accompanying test does the same thing — rebuilds the event from data["rejected"]["attrs"] and verifies it against the rogue key. That is the right assertion: a test checking only that the forged text is present would pass on a record too incomplete to verify.

The docstring is also correctly rewritten rather than patched. The false "raw_json retained for audit" line is gone, replaced by an explanation of why in-place rewriting would have destroyed the evidence — which is more useful to the next reader than the claim ever was.

Also picked up the non-blocking note from my last review: kafka_out.py now records that ~200 members is ~40 s, past a common 30 s client timeout, and that Idempotency-Key makes the retry return the original batch rather than launching a second one — with the caveat that it only helps if the caller sends the header. That caveat is the part that matters and I had not spelled it out.

Verified at 3264e8dd: 549 passed / 7 skipped on 3.14 (up from 547 — the two new forensic tests), ruff check . and ruff format --check . clean, 11/11 CI green, all 8 commits signed off, no .claude/.vscode changes.

Nothing further from me. My earlier thread on kafka_in.py shows as outdated because the lines moved under the fix; it can be resolved.

---

For the record, the three eventing PRs each closed the previous one's gap: #877's keyset that nothing consulted became #879's verify_with_keyset; #878's unsigned submitter became an entry in SIGNED_ATTRS; and the audit record that the docstring promised is now one an incident can actually be reconstructed from. The remaining limit is stated plainly in emit.py — terminal-only signing proves who finished a run, not what it said along the way — and leaving that written down rather than implied is the right way to ship a partial control.