diff --git a/eventing/agentdocs/DESIGN_PHASE0.md b/eventing/agentdocs/DESIGN_PHASE0.md index 956a47d8..1a63febf 100644 --- a/eventing/agentdocs/DESIGN_PHASE0.md +++ b/eventing/agentdocs/DESIGN_PHASE0.md @@ -107,7 +107,7 @@ def cmd_cont(args): if __name__ == "__main__": p = argparse.ArgumentParser() sub = p.add_subparsers(dest="op", required=True) - r = sub.add_parser("run"); r.add_argument("prompt"); r.add_argument("--max-turns", type=int, default=3); r.add_argument("--watch", action="store_true", default=True); r.set_defaults(fn=cmd_run) + r = sub.add_parser("run"); r.add_argument("prompt"); r.add_argument("--max-turns", type=int, default=3); r.add_argument("--watch", action=argparse.BooleanOptionalAction, default=True); r.set_defaults(fn=cmd_run) w = sub.add_parser("watch"); w.add_argument("correlationid"); w.set_defaults(fn=cmd_watch) c = sub.add_parser("cont"); c.add_argument("correlationid"); c.add_argument("prompt"); c.set_defaults(fn=cmd_cont) args = p.parse_args(); args.fn(args) diff --git a/eventing/agentdocs/DESIGN_PHASE2.md b/eventing/agentdocs/DESIGN_PHASE2.md index ca5354c3..a21dd080 100644 --- a/eventing/agentdocs/DESIGN_PHASE2.md +++ b/eventing/agentdocs/DESIGN_PHASE2.md @@ -386,6 +386,7 @@ unaffected until an operator opts in: | `ER_SIGNING_KEY_PATH` | empty | Seed EventRunner signs terminal responses with. | | `ER_SIGNING_KID` | empty | Names this runner's key. Needed once more than one runner is approved. | | `ER_REQUIRE_SIGNATURE` | `false` | Refuse unsigned or badly-signed requests. | +| `ER_VERIFY_KEY_PATH` | empty | Single public key requests are verified against. Empty falls back to the public half of `ER_SIGNING_KEY_PATH`'s seed. | | `ER_VERIFY_KEYSET_PATH` | empty | Approved-key set for requests. Empty falls back to `ER_VERIFY_KEY_PATH`'s single key. | Seed paths name **Secret** mounts and are env-only, never `config.toml`. Keyset diff --git a/eventing/agentdocs/DESIGN_PHASE3.md b/eventing/agentdocs/DESIGN_PHASE3.md index c958fd7b..4d0261b5 100644 --- a/eventing/agentdocs/DESIGN_PHASE3.md +++ b/eventing/agentdocs/DESIGN_PHASE3.md @@ -2,7 +2,7 @@ Status: draft (revision 1) — **partly implemented.** §9's steps 1-4 are in the tree: T1 (`shared/tenancy.py`), T2 (`TopicSet` threaded through both services), T3 (`Caller`, -the registry, `ce_userkey`), T4 (`SIGNED_ATTRS` += `userkey`, `depth`), T13 (`AgentSpec`) +the registry, `ce_userkey`), T4 (`SIGNED_ATTRS` += `userkey`, `depth`, `agent`), T13 (`AgentSpec`) and T22 (the Secret-vs-ConfigMap test). **T5 is also in** — per-user stores (`store_registry.py`), the global `correlationid → userkey` index (`owner_index.py`), and the `Minter` uniqueness check that replaces the startup seeding loop (§2.6). @@ -340,7 +340,7 @@ deployment worse than an unsigned one because an operator believes it is atteste response is written into and which ntfy topic it is announced on, so an unsigned, mutable `userkey` would let anything with write access to a topic file an event into another user's history. See §8.3 on the canonicalisation break that adding it causes, -and why it has to land with `depth` in one change rather than two. +and why it has to land with `depth` and `agent` in one change rather than three. **The trap.** It is tempting to salt the session derivation — `uuid5(NAMESPACE, userkey + "/" + corr)` — so two tenants can never collide in @@ -400,13 +400,14 @@ The brief's shape, with `{userid}` replaced by `{userkey}` for the reasons in § unchanged. The two topics this table adds have no Phase 2 predecessor to stay compatible with, so they are derived from the prefix instead: `{prefix}-events` and `{prefix}-dead`. -`{prefix}` defaults to the namespace (`kev1`), configurable as `EB_TOPIC_PREFIX`, -exactly as Phase 1 §5 prefixes the shared-broker topics today. It is validated as -`[a-zA-Z0-9-]{1,32}`: it is the one component of these names `_slug` never sees, and `.` -or `_` in it reproduces precisely the JMX metric collision §3.1's naming rules below warn -about. Longest realistic -name: `kev1-` + 2 + 1 + 24 + 1 + 8 + `-responses` = 51 characters, against Kafka's -249 — comfortable, and worth having checked rather than assumed. +`{prefix}` defaults to `kev1`, configurable as `EB_TOPIC_PREFIX`. It is not derived +from the namespace: a literal default keeps the names identical to Phase 1's shipped +`kev1-` topics regardless of the namespace the chart lands in, and it matches §8.1's +row. It is validated as `[a-zA-Z0-9-]{1,32}`: it is the one component of these names +`_slug` never sees, and `.` or `_` in it reproduces precisely the JMX metric collision +§3.1's naming rules below warn about. Longest realistic name: `kev1-` + 2 + 1 + 24 + +1 + 8 + `-responses` = 51 characters, against Kafka's 249 — comfortable, and worth +having checked rather than assumed. Two naming rules that bite if ignored: @@ -571,10 +572,27 @@ Three things this buys that are worth naming: the ownership check on transcript writes possible at all (§6.3). `ER_USERKEY` is the new required variable in multi-tenant mode, and the runner -**refuses to start without it** when `REQUEST_TOPIC` does not equal the bridge's -single-tenant default. A runner that stamps no `userkey` on its responses produces -events EventBridge cannot file, and a silent default here would route one user's -output into another user's store. +**refuses to start** on a mismatch between it and `REQUEST_TOPIC`, in either +direction: a per-user topic (`{prefix}-{userkey}-requests`, §3.1) the key does not +name, and — the inverse, which is just as wrong — a key set while `REQUEST_TOPIC` +is not that user's per-user topic. In the inverse direction the runner would +consume the shared topic while stamping `ce_userkey` on every response (the stamp +keys on the key's presence, not on tenancy mode), filing every response as one +tenant's. A runner that stamps no userkey on its responses produces events +EventBridge cannot file, and a silent default here would route one user's output +into another user's store. The key must also have `userkey()`'s shape +(`is_valid_userkey`): a typo'd key names nobody, and StoreRegistry counts every +stamped response `unattributed`. + +The match itself anchors on the key the runner holds rather than decomposing the +topic name: `{prefix}-{userkey}-{suffix}` is an ambiguous grammar — dashes are +legal in both the prefix and the slug inside `userkey` — so any split picks an +arbitrary answer (`prod-eu-gh-alice-…-requests` splits shortest-first and embeds +the wrong key), and a split-based check would refuse a correctly-configured +deployment with a dashed prefix. `topic_names_userkey` needs no split. The refusal +deliberately does not key on "REQUEST_TOPIC is not the single-tenant default" +either: single-tenant deployments rename the default freely (the shipped Phase 1 +configmap sets `kev1-requests`), and a renamed default is not a tenancy signal. ### 3.4 KEDA: one ScaledObject per user @@ -1948,8 +1966,8 @@ whose depth is `>= EB_TRIGGER_MAX_DEPTH` (default **3**) and records a `trigger.dropped` with reason `depth-exceeded`. `depth` joins `signing.SIGNED_ATTRS`, and it must: an unsigned counter can be reset to -zero in flight, which turns the whole control off. See §8.3 — this is the second -canonicalisation-breaking change in Phase 3 and it has to land together with `userkey`. +zero in flight, which turns the whole control off. See §8.3 — it is one of the three +canonicalisation-breaking attributes of Phase 3, and they land in one change. **2. Responses are not trigger-eligible by default.** `EB_TRIGGER_ON_RESPONSES=false`. Agent chaining — "when the triager finishes, run the fixer" — is a legitimate and @@ -2040,7 +2058,7 @@ upgrades and changes nothing behaves exactly as it did in Phase 2. | Variable | Default | Effect | |---|---|---| -| `ER_USERKEY` | empty | Which tenant this runner serves (§3.3). Stamped on every response as `ce_userkey`. Required once `REQUEST_TOPIC` is not the default. | +| `ER_USERKEY` | empty | Which tenant this runner serves (§3.3). Stamped on every response as `ce_userkey`. Must name the tenant `REQUEST_TOPIC` serves, in either direction: a per-user topic without the matching key refuses, and a key without its per-user topic refuses too. | | `ER_AGENT_DIR` | `/etc/rossoctl/agents` | Where baked `AgentSpec`s live (§5.2). | | `ER_AGENT_NAME` | `default` | Fallback agent when neither the trigger nor the registry names one. | | `ER_SKILL_SOURCES` | empty | Allowed source prefixes. **Empty means no fetching at all** (§5.4 gate 1). | @@ -2093,7 +2111,7 @@ The dependency order is real — several steps are unsafe before the one above t 3. **The registry and `Caller`** (§2.4, §2.5), still in `single` mode. `ce_userkey` is stamped on events but nothing routes on it — the Phase 2 §4.4 "audit before enforce" pattern, applied to tenancy. -4. **`SIGNED_ATTRS` += `userkey`, `depth`** (§8.3). One change, both services. +4. **`SIGNED_ATTRS` += `userkey`, `depth`, `agent`** (§8.3). One change, both services. 5. **Per-user stores** (§6.1) and **owner-scoped reads** (§6.2), behind `EB_TENANCY_MODE=multi`. Now `multi` is usable by one real tenant. 6. **Transcript auth** (§6.3). Must come before any second tenant exists: §6's opening @@ -2163,7 +2181,7 @@ In the spirit of Phase 2 §3, the things a demo of this must not claim: | **T1** | `shared/tenancy.py`: `userkey`, `_canonicalise`, `_slug`, `TopicSet`, `ntfy_topic`. Tests: collision cases, the GitHub case-folding agreement with `ghauth.is_allowed`, topic/DNS/ntfy legality over a corpus of adversarial identifiers. | — | | **T2** | Thread `TopicSet` through `kafka_out`, `kafka_in`, `RequestsMirror`, `GroupMirror`, `eventrunner/consume`. `single` mode only; zero behaviour change. | T1 | | **T3** | `Caller`, the registry loader, `auth.resolve` returning `userkey`; `ce.EXT_USERKEY`; stamp it on requests. | T1 | -| **T4** | `SIGNED_ATTRS` += `userkey`, `depth`, in one change, both services. | T3, T13 | +| **T4** | `SIGNED_ATTRS` += `userkey`, `depth`, `agent`, in one change, both services. | T3, T13 | | **T5** | `StoreRegistry` + per-user store files + the global `correlationid → userkey` index + lazy `Minter` seeding (§2.6). | T3 | | **T6** | `Consumer.ensure_subscribed` with the blocking assignment wait, `auto_offset_reset=earliest`, and submit-path ordering. Test the race explicitly: publish before subscribe must fail the test. | T2 | | **T7** | Owner-scoped reads: `404` not `403`, the group-list filter, and the single-tenant bypass. | T5 | diff --git a/eventing/agentdocs/IMPLEMENTATION_REPORT1.md b/eventing/agentdocs/IMPLEMENTATION_REPORT1.md index 8c9b768a..ff7839fc 100644 --- a/eventing/agentdocs/IMPLEMENTATION_REPORT1.md +++ b/eventing/agentdocs/IMPLEMENTATION_REPORT1.md @@ -172,7 +172,7 @@ Highlights, each re-measuring something the design had asserted: | Concurrency (T1.10) | ≤ partition count | peak **4** replicas for 3 correlations | | Image pull | 5.1 s cold (245 MB) | 454 ms – 2.45 s warm | | Rebalance floor (Gap A) | ~3 s | consistent with the 5.0 s total | -| Ed25519 sign / verify | not estimated | **222 ms / 227 ms** — see §6 | +| Ed25519 sign / verify | not estimated | **222 ms / 227 ms** — see §9 | --- diff --git a/eventing/agentdocs/KUBECON_NA_2026.md b/eventing/agentdocs/KUBECON_NA_2026.md index f0c22ff0..c4153460 100644 --- a/eventing/agentdocs/KUBECON_NA_2026.md +++ b/eventing/agentdocs/KUBECON_NA_2026.md @@ -774,5 +774,5 @@ been read rather than run. Confirm it before spending anything on either CNI opt | Date | Change | |---|---| | 2026-10-05 | Created. Captures the accepted abstract, status against it as of `main`, the SPIRE decision, the W1–W6 plan, and the 30-minute slide/demo shape | -| 2026-10-06 | M1 and M2 filed as #895 and #896; §5 gains a tracking table | | 2026-10-05 | Added §5, the M1-M4 MVP cut (~2 days) with sequencing, explicit cuts and the talk it supports. Corrected §4/W3: Kind's default CNI is kindnetd and does not enforce NetworkPolicy, so the `NetworkPolicy` MVP was unbuildable as written — three options tabulated, M3 recommends substituting the #888 asymmetry for the egress beat. §7's risk table and §8's observations updated; the 30-minute shape renumbered §5→§6, risks §6→§7, observations §7→§8 | +| 2026-10-06 | M1 and M2 filed as #895 and #896; §5 gains a tracking table | diff --git a/eventing/agentdocs/README_PHASE0.md b/eventing/agentdocs/README_PHASE0.md index 79dd0100..ec4071a1 100644 --- a/eventing/agentdocs/README_PHASE0.md +++ b/eventing/agentdocs/README_PHASE0.md @@ -675,7 +675,7 @@ Run the unit tests any time you want to reprove the DESIGN §4.5 contract withou ```bash .venv/bin/python -m pytest tests/ -q -# → .................... 20 passed in ~0.6s +# → .......................................................... 58 passed in 7.27s ``` The four tests in `tests/test_router.py` cover FIFO ordering per correlationid, cross-correlation parallelism, the concurrency cap, and slot re-arm after drain. diff --git a/eventing/agentdocs/README_PHASE1.md b/eventing/agentdocs/README_PHASE1.md index ade31fa8..78780b7b 100644 --- a/eventing/agentdocs/README_PHASE1.md +++ b/eventing/agentdocs/README_PHASE1.md @@ -694,9 +694,11 @@ Two limits worth stating plainly: `/continue` stay open by design — `/continue` because authenticating it would put a long-lived bearer token inside every ntfy notification; it relies on the unguessable `correlationid` as a capability URL instead. -- `submitter` is **not signed**, and Kafka is plaintext. The claim is "EventBridge - refuses unauthenticated submissions and records who it believes submitted this", - not "this event proves who submitted it." +- `submitter` is signed (it joined `signing.SIGNED_ATTRS` in Phase 2, DESIGN_PHASE2 + §4.2), and Kafka is still plaintext. The claim is "EventBridge refuses + unauthenticated submissions and records who it believes submitted this — and on a + signed request, the signature proves EventBridge asserted that name", not "this + event proves who typed it." Setting `EB_AUTH_TOKENS` will 401 the `/eventbridge` CLI, the three e2e scripts and the README `curl`s until they are taught to send the header. @@ -718,8 +720,10 @@ rm /tmp/seed.hex Signing is implemented in pure Python (`cryptography` is a C extension and banned), verified against the RFC 8032 test vectors. It is off by default partly -because the pure-Python scalar multiplication costs roughly 100 ms per -sign/verify, which is real per-event overhead — see `IMPLEMENTATION_REPORT1.md`. +because the pure-Python scalar multiplication is real per-event overhead: +IMPLEMENTATION_REPORT1.md §9 measured **222 ms to sign and 227 ms to verify** on +the reference laptop, and later hardware has measured inside a 190–230 ms band — +fine for the terminal event, wrong for every intermediate one. --- diff --git a/eventing/eventbridge/kafka_out.py b/eventing/eventbridge/kafka_out.py index 63d5b1a5..6fffbf83 100644 --- a/eventing/eventbridge/kafka_out.py +++ b/eventing/eventbridge/kafka_out.py @@ -15,7 +15,8 @@ class Producer: signed input — it cannot be swapped to impersonate another key. **Cost.** The Ed25519 here is pure Python (`cryptography` is a C extension and is - banned by §1.1) and takes ~150-200 ms per signature. `publish_request` is called + banned by §1.1) and takes 190-230 ms per signature (measured: 222 ms sign / 227 ms + verify, IMPLEMENTATION_REPORT1 §9). `publish_request` is called once per group member by `GroupService.submit_members`, in a loop, inside one HTTP request — so a 100-member batch spends ~20 s signing while the caller waits. That is accepted: a batch launch is an operator action, not a hot path. It is recorded diff --git a/eventing/eventrunner/config.py b/eventing/eventrunner/config.py index a6624916..4b07b4cd 100644 --- a/eventing/eventrunner/config.py +++ b/eventing/eventrunner/config.py @@ -6,6 +6,8 @@ from collections.abc import Mapping from dataclasses import dataclass +from shared import tenancy + # Env vars that carry an Anthropic credential. Their presence is what decides # whether we default to real `claude` or to mock mode — a container with no key # would otherwise spawn claude only to have it fail on auth, turning every @@ -109,10 +111,10 @@ class Cfg: # §3.3 — which tenant this runner serves. Stamped on every response as # `ce_userkey`, which is what lets EventBridge file the response in the right - # store. Empty is single-tenant mode (the default, = Phase 2). Required once - # REQUEST_TOPIC is not the default: a runner that stamps no userkey produces - # events EventBridge cannot attribute, and a silent default here would route one - # user's output into another user's store. `load` enforces that, loudly. + # store. Empty is single-tenant mode (the default, = Phase 2). Must agree with + # REQUEST_TOPIC in either direction — a per-user topic without this key, or + # this key without its per-user topic, is a deployment rendered wrong — and + # must have userkey()'s shape. `load` enforces all three, loudly. userkey: str = "" # §5.2 — where baked AgentSpecs live. A directory in the image, not a mount: @@ -173,17 +175,61 @@ def load() -> Cfg: # §3.3, §5.1-§5.2. cfg.userkey = (e("ER_USERKEY") or "").strip() + if cfg.userkey and not tenancy.is_valid_userkey(cfg.userkey): + # A key without tenancy.userkey()'s shape (--<8 hex>) could not + # have come from the registry, so it names nobody: EventBridge's + # StoreRegistry would count every stamped response `unattributed`. Catching + # the typo at boot is cheaper than finding it on /healthz after the work is + # lost — and the topic-name checks below cannot catch it, because a topic + # rendered from the same typo'd key anchors against it consistently. + raise SystemExit( + f"ER_USERKEY {cfg.userkey!r} does not have the shape tenancy.userkey() " + f"produces (--<8 hex>, e.g. gh-alice-a1b2c3d4). Check the " + f"user registry entry it was rendered from. See DESIGN_PHASE3.md §3.3.") cfg.agent_dir = e("ER_AGENT_DIR", cfg.agent_dir) cfg.agent_name = e("ER_AGENT_NAME", cfg.agent_name) - # A per-user runner is pointed at a per-user topic, so a non-default REQUEST_TOPIC - # with no ER_USERKEY means the deployment was rendered wrong. Refusing to start is - # the only safe direction: the alternative is responses EventBridge files into the - # `shared` store or drops, which looks exactly like an agent that never answered. - if cfg.request_topic != Cfg.request_topic and not cfg.userkey: + # A per-user runner is pointed at a per-user topic. Both directions of a mismatch + # between the two are a deployment rendered wrong, and both refuse to start: + # - a per-user REQUEST_TOPIC the configured ER_USERKEY does not name — the + # responses would be stamped for a tenant whose topic this is not; + # - ER_USERKEY set while REQUEST_TOPIC is NOT a per-user topic — the runner + # would consume the shared topic and stamp `ce_userkey` on every response + # (`emit.py` keys the stamp on userkey presence, not on tenancy mode), filing + # every response into one tenant's store. + # Refusing to start is the only safe direction: the alternative is responses + # EventBridge files into the `shared` store or another user's, which looks + # exactly like an agent that never answered. `topic_names_userkey` anchors on the + # key in hand rather than splitting the name — the `{prefix}-{userkey}-{suffix}` + # grammar is ambiguous when both prefix and slug contain dashes, so a split-based + # check false-positives on legal dashed prefixes (`prod-eu-…`). The per-user + # question itself is shape-only, and every valid split still means "per-user". + # A single-tenant deployment renaming the default (the shipped K8s configmap sets + # `kev1-requests`) is not a tenancy signal and still boots. + if tenancy.userkey_in_topic(cfg.request_topic) is not None \ + and not tenancy.topic_names_userkey(cfg.request_topic, cfg.userkey): + # Two distinct faults share this exit, and the message names which one: an + # unset ER_USERKEY (the runner stamps nothing) and one that disagrees with the + # topic (the runner stamps the wrong tenant). Saying "is required" for the + # second would be wrong — it was supplied, it just names someone else. The + # topic rides in the message because it carries the key it should name: the + # anchoring check does not extract one, and the exit line stays diagnosable + # without reading the chart. + problem = ("ER_USERKEY is required when REQUEST_TOPIC is a per-user topic" + if not cfg.userkey else + f"ER_USERKEY must match the tenant REQUEST_TOPIC serves, " + f"but names {cfg.userkey!r}") + raise SystemExit( + f"{problem} (got {cfg.request_topic!r}). A runner that stamps no " + f"userkey — or the wrong one — on its responses produces events " + f"EventBridge cannot attribute, or files them in another user's store. " + f"See DESIGN_PHASE3.md §3.3.") + if cfg.userkey and not tenancy.topic_names_userkey(cfg.request_topic, cfg.userkey): raise SystemExit( - f"ER_USERKEY is required when REQUEST_TOPIC is not {Cfg.request_topic!r} " - f"(got {cfg.request_topic!r}). A runner that stamps no userkey on its " - f"responses produces events EventBridge cannot attribute. " + f"ER_USERKEY is set but REQUEST_TOPIC {cfg.request_topic!r} is not this " + f"userkey's per-user topic. A runner configured this way consumes the " + f"shared topic while stamping every response as {cfg.userkey!r} — in " + f"multi-tenant mode that files other users' responses into one user's " + f"store. Set REQUEST_TOPIC to the per-user topic, or clear ER_USERKEY. " f"See DESIGN_PHASE3.md §3.3.") base = e("TMPDIR", "/tmp").rstrip("/") diff --git a/eventing/eventrunner/emit.py b/eventing/eventrunner/emit.py index 57735262..15ea071a 100644 --- a/eventing/eventrunner/emit.py +++ b/eventing/eventrunner/emit.py @@ -13,7 +13,8 @@ class Emitter: """§11: signs **terminal events only**, and what that does and does not prove. `emit()` runs for every `stdout` frame an agent produces, and the pure-Python - Ed25519 here costs ~150-200 ms per signature, so signing every frame would add + Ed25519 here costs 190-230 ms per signature (measured: 222 ms sign / 227 ms + verify, IMPLEMENTATION_REPORT1 §9), so signing every frame would add minutes to a chatty run. Terminal events are one per run, where the cost is invisible against an agent that already took seconds. diff --git a/eventing/shared/signing.py b/eventing/shared/signing.py index cfcf9103..98e6ed6c 100644 --- a/eventing/shared/signing.py +++ b/eventing/shared/signing.py @@ -89,7 +89,7 @@ def _scalar_mult(p: tuple[int, int], e: int) -> tuple[int, int]: strictly true: with `EB_SIGNING_KEY_PATH` set, EventBridge signs on the HTTP request path, so a caller who can time `POST /v0/agents` observes something correlated with the scalar. What still makes it acceptable is that the signal is - buried under a Kafka round trip and a ~150 ms pure-Python operation whose variance + buried under a Kafka round trip and a ~200 ms pure-Python operation whose variance dwarfs the leak, the seed never leaves the pod, and signing remains opt-in. It is a real if impractical weakness rather than a non-issue — do not promote this to a trust boundary that assumes constant time. @@ -464,7 +464,8 @@ def response_decision(event, ks, *, require: bool, **Unsigned non-terminal frames are accepted, and that is not a loophole being left open — it is the signing policy on the other side.** `emit()` signs terminal events only, because it runs for every `stdout` frame and a signature costs - ~150-200 ms; verifying all-or-nothing would rewrite every streamed frame of every + ~190-230 ms (measured: 222 ms sign / 227 ms verify, IMPLEMENTATION_REPORT1 §9); + verifying all-or-nothing would rewrite every streamed frame of every genuine run to `phase=error`. So an event that carries no signature AND is not terminal is passed through, while an unsigned **terminal** event is still refused — that is the one the transcript presents as the answer, and refusing it is the whole diff --git a/eventing/shared/tenancy.py b/eventing/shared/tenancy.py index 8d473f7b..658bfa9b 100644 --- a/eventing/shared/tenancy.py +++ b/eventing/shared/tenancy.py @@ -162,8 +162,19 @@ def userkey(issuer: str, userid: str) -> str: # # The slug is at most _SLUG_MAX characters, starts with an alphanumeric (`_slug` strips # leading dashes) and contains only `[a-z0-9-]`. -USERKEY_RE = re.compile( - rf"^[a-z]{{2}}-[a-z0-9][a-z0-9-]{{0,{_SLUG_MAX - 1}}}-[0-9a-f]{{{_DIGEST_HEX}}}$") +_USERKEY_CORE = rf"[a-z]{{2}}-[a-z0-9][a-z0-9-]{{0,{_SLUG_MAX - 1}}}-[0-9a-f]{{{_DIGEST_HEX}}}" +USERKEY_RE = re.compile(rf"^{_USERKEY_CORE}$") + +# TopicSet's four per-user suffixes (§3.1). One tuple because two recognisers need +# the same list: the shape regex below and `topic_names_userkey`'s anchored check. +_SUFFIXES = ("requests", "responses", "events", "dead") + +# A per-user topic name, `{prefix}-{userkey}-{suffix}` (§3.1): the prefix agrees with +# EventBridge's `EB_TOPIC_PREFIX` validation and the suffixes are `TopicSet`'s four. +# Used to recognise — not interpret — a per-user topic: see `userkey_in_topic` for why +# the split it captures cannot be trusted for identity. +PER_USER_TOPIC_RE = re.compile( + rf"^[a-zA-Z0-9-]{{1,32}}?-(?P{_USERKEY_CORE})-(?:{'|'.join(_SUFFIXES)})$") def is_valid_userkey(value: str | None) -> bool: @@ -192,6 +203,52 @@ def is_valid_userkey(value: str | None) -> bool: return bool(value) and bool(USERKEY_RE.fullmatch(value)) +def userkey_in_topic(topic: str | None) -> str | None: + """Whether a topic name is per-user, and a key it *might* embed — ambiguous by + construction, so only the yes/no answer is reliable. + + A per-user topic is `{prefix}-{userkey}-{suffix}` (§3.1) and both `prefix` + (`[a-zA-Z0-9-]{1,32}`, dashes allowed) and the slug inside `userkey` can contain + `-`, so the grammar has several valid splits and any one regex quantifier picks + one: non-greedy prefers the shortest prefix (`prod-eu-gh-alice-ed66acf0-requests` + yields `eu-gh-alice-ed66acf0`), greedy prefers the longest (`kev1-oi-alice-ex- + ample-com-1f806a21-requests` yields the slug-only tail). Both are ordinary + configurations. **The returned key must therefore never be compared for + identity** — use `topic_names_userkey`, which anchors on the key the caller + already holds and needs no split at all. This function's value is its yes/no + answer: every valid split still means "per-user". + + `None` is the answer for every single-tenant topic — including renamed defaults + like `kev1-requests` — because a renamed default is not a tenancy signal. + + False negatives are safe here: the runner's refusal is a deployment guard, not + the last line of defense — EventBridge's `StoreRegistry.for_event` is what + actually files or rejects a response, and it counts rather than guesses. + """ + if not topic: + return None + m = PER_USER_TOPIC_RE.fullmatch(topic) + return m.group("userkey") if m else None + + +def topic_names_userkey(topic: str | None, userkey: str | None) -> bool: + """Whether `topic` is this tenant's per-user topic. No split, so no ambiguity. + + The check the runner actually needs — "does this topic belong to a tenant other + than the one I am configured as?" — does not require decomposing the name, only + matching the key already in hand against the name's end: `{prefix}-{userkey}- + {suffix}` anchored on `-{userkey}-{suffix}` can be satisfied by exactly one + key, whatever dashes the prefix and slug contain. + + The caller supplies the key, so a typo'd or forged `ER_USERKEY` answers `False` + against a real per-user topic — which is the safe direction for a boot-time + guard: refuse, loudly, and let the operator read the error. + """ + if not topic or not userkey: + return False + return any(topic.endswith(f"-{userkey}-{s}") for s in _SUFFIXES) + + def ntfy_topic(prefix: str, userkey: str, secret: bytes) -> str: """An unguessable, stable, per-user ntfy topic that contains no identifier. diff --git a/eventing/tests/test_tenancy.py b/eventing/tests/test_tenancy.py index 947db03b..eb01c3e5 100644 --- a/eventing/tests/test_tenancy.py +++ b/eventing/tests/test_tenancy.py @@ -238,6 +238,92 @@ def test_single_mode_inbox_and_dead_are_derived_from_the_prefix(): assert ts.dead() == "kev1-dead" +# ---- userkey_in_topic ------------------------------------------------------ + +def test_userkey_in_topic_recognises_topicset_names_as_per_user(): + """`TopicSet`'s four per-user names are recognised as per-user. The captured key + is deliberately not asserted — see the ambiguity test below.""" + uk = T.userkey("github", "Mrsabath") + ts = T.TopicSet("kev1", tenancy=T.MULTI) + for method in (ts.requests, ts.responses, ts.events, ts.dead): + assert T.userkey_in_topic(method(uk)) is not None + assert T.topic_names_userkey(method(uk), uk) + + +@pytest.mark.parametrize("suffix", ["requests", "responses", "events", "dead"]) +def test_userkey_in_topic_recognises_each_suffix(suffix): + """Recognition only — the split's captured key is not asserted, per the + ambiguity test below.""" + uk = T.userkey("oidc", "alice@example.com") + assert T.userkey_in_topic(f"kev1-{uk}-{suffix}") is not None + assert T.topic_names_userkey(f"kev1-{uk}-{suffix}", uk) + + +def test_userkey_in_topic_returns_none_for_single_tenant_names(): + """A renamed default is not a tenancy signal — this is the case the runner's + old refusal got wrong, refusing to boot `k8s/base/configmap.yaml`'s + `REQUEST_TOPIC: kev1-requests`.""" + assert T.userkey_in_topic("requests") is None + assert T.userkey_in_topic("kev1-requests") is None + assert T.userkey_in_topic("kev1-kev1-requests") is None + assert T.userkey_in_topic("") is None + assert T.userkey_in_topic(None) is None + + +def test_userkey_in_topic_requires_a_per_user_suffix(): + uk = T.userkey("github", "Mrsabath") + assert T.userkey_in_topic(f"kev1-{uk}") is None + assert T.userkey_in_topic(f"kev1-{uk}-other") is None + + +def test_userkey_in_topic_rejects_names_that_are_not_userkey_products(): + """The embedded component must have the `userkey()` shape — anything else is a + coincidence of dashes, not a tenant.""" + assert T.userkey_in_topic("kev1-shared-requests") is None + assert T.userkey_in_topic("kev1-GH-MRSABATH-4C1D9E07-requests") is None + assert T.userkey_in_topic("kev1-gh-alice-notahex-requests") is None + + +def test_userkey_in_topic_split_is_ambiguous_and_only_the_yes_no_is_reliable(): + """Dashes are legal in both the prefix and the slug, so the split a regex + quantifier picks is arbitrary: `prod-eu-…` (dashed prefix) splits shortest-first + and embeds the wrong key, and a multi-dash slug splits the other way under a + greedy quantifier. Both answers are still 'per-user', which is the only claim + the caller may rely on — identity goes through `topic_names_userkey`.""" + # Non-greedy picks the shortest prefix, so the captured key is wrong here… + assert T.userkey_in_topic("prod-eu-gh-alice-ed66acf0-requests") == "eu-gh-alice-ed66acf0" + # …yet the same name, anchored on the real key, is recognised as that tenant's. + assert T.topic_names_userkey("prod-eu-gh-alice-ed66acf0-requests", "gh-alice-ed66acf0") + assert not T.topic_names_userkey("prod-eu-gh-alice-ed66acf0-requests", "eu-gh-xxxxxxx") + + +def test_topic_names_userkey_matches_by_suffix_anchor(): + uk = T.userkey("github", "Mrsabath") + for suffix in ("requests", "responses", "events", "dead"): + assert T.topic_names_userkey(f"kev1-{uk}-{suffix}", uk) + assert not T.topic_names_userkey(f"kev1-{uk}-{suffix}", "gh-someone-00000000") + + +def test_topic_names_userkey_rejects_shared_and_missing_names(): + uk = T.userkey("github", "Mrsabath") + assert not T.topic_names_userkey("kev1-requests", uk) # single-tenant rename + assert not T.topic_names_userkey("requests", uk) + assert not T.topic_names_userkey("", uk) + assert not T.topic_names_userkey(None, uk) + assert not T.topic_names_userkey("kev1-requests", "") # no key in hand + assert not T.topic_names_userkey("kev1-requests", None) + assert not T.topic_names_userkey(None, None) + + +def test_topic_names_userkey_needs_the_suffix_boundary(): + """`-{userkey}-requests` is the anchor; a name merely ending in the key, or + embedding it without the suffix, must not match.""" + uk = T.userkey("github", "Mrsabath") + assert not T.topic_names_userkey(f"kev1-{uk}", uk) + assert not T.topic_names_userkey(f"kev1-{uk}-other", uk) + assert not T.topic_names_userkey(f"prefix-{uk}-requests-extra", uk) + + # ---- ntfy_topic ------------------------------------------------------------ SECRET = b"a-test-ntfy-topic-secret" diff --git a/eventing/tests/test_topicset_wiring.py b/eventing/tests/test_topicset_wiring.py index ed3aec24..57498d09 100644 --- a/eventing/tests/test_topicset_wiring.py +++ b/eventing/tests/test_topicset_wiring.py @@ -175,7 +175,7 @@ def test_the_emitter_omits_the_userkey_in_single_mode(): assert ce.EXT_USERKEY not in _sent_event(em._prod).attrs -def test_the_runner_refuses_a_non_default_request_topic_without_a_userkey(monkeypatch): +def test_the_runner_refuses_a_per_user_topic_without_a_userkey(monkeypatch): """§3.3: a runner that stamps no userkey produces events EventBridge cannot attribute, and a silent default would route one user's output into another's store.""" from eventrunner import config as ercfg @@ -185,6 +185,82 @@ def test_the_runner_refuses_a_non_default_request_topic_without_a_userkey(monkey ercfg.load() +def test_the_runner_refuses_a_per_user_topic_with_a_mismatched_userkey(monkeypatch): + """The topic embeds one tenant; ER_USERKEY names another. Attributing the + responses to the named one would file them in the wrong store — refuse. + + The message must not say "is required": it *was* supplied, it just names + someone else, and an operator told a set variable is missing looks in the + wrong place. Both the supplied and the embedded key are named so the + rendering bug is diagnosable from the exit line alone.""" + from eventrunner import config as ercfg + monkeypatch.setenv("REQUEST_TOPIC", f"kev1-{UK}-requests") + monkeypatch.setenv("ER_USERKEY", "gh-alice-00000000") + with pytest.raises(SystemExit, match="ER_USERKEY must match") as exc: + ercfg.load() + msg = str(exc.value) + assert "is required" not in msg + assert "gh-alice-00000000" in msg # what the operator set + assert UK in msg # what the topic says it should be + + +def test_the_runner_accepts_a_per_user_topic_with_a_dashed_prefix(monkeypatch): + """`{prefix}-{userkey}-{suffix}` is an ambiguous grammar — dashes are legal in + both the prefix and the slug — so a split-based check false-positives on a + correctly-configured deployment with a dashed prefix: the non-greedy split of + `prod-eu-gh-alice-ed66acf0-requests` yields `eu-gh-alice-ed66acf0`. The guard + anchors on the key in hand instead (`topic_names_userkey`), which needs no + split and cannot be wrong about the one key it is asked about.""" + from eventrunner import config as ercfg + monkeypatch.setenv("REQUEST_TOPIC", "prod-eu-gh-alice-ed66acf0-requests") + monkeypatch.setenv("ER_USERKEY", "gh-alice-ed66acf0") + assert ercfg.load().userkey == "gh-alice-ed66acf0" + + +def test_the_runner_accepts_a_per_user_topic_with_a_multi_dash_slug(monkeypatch): + """The greedy fix's failure mode: slugs themselves contain dashes + (`oi-alice-example-com-9e8f7a6b` is userkey()'s own docstring example), so a + longest-prefix split is wrong the other way. The anchored check is indifferent.""" + from eventrunner import config as ercfg + monkeypatch.setenv("REQUEST_TOPIC", "kev1-oi-alice-ex-ample-com-1f806a21-requests") + monkeypatch.setenv("ER_USERKEY", "oi-alice-ex-ample-com-1f806a21") + assert ercfg.load().userkey == "oi-alice-ex-ample-com-1f806a21" + + +def test_the_runner_refuses_a_userkey_on_a_shared_topic(monkeypatch): + """The inverse misconfiguration: ER_USERKEY set while REQUEST_TOPIC is not that + user's per-user topic. `emit.py` keys its stamp on userkey presence, not tenancy + mode, so this runner would consume the shared topic and file every response as + one tenant's — the harm §3.3 names, from the other direction.""" + from eventrunner import config as ercfg + monkeypatch.setenv("REQUEST_TOPIC", "kev1-requests") + monkeypatch.setenv("ER_USERKEY", UK) + with pytest.raises(SystemExit, match="is not this userkey's per-user topic"): + ercfg.load() + + +def test_the_runner_refuses_an_invalid_userkey_shape(monkeypatch): + """A key without tenancy.userkey()'s shape names nobody — StoreRegistry counts + every stamped response `unattributed` — and a topic rendered from the same + typo'd key anchors against it consistently, so only a shape check catches it.""" + from eventrunner import config as ercfg + monkeypatch.setenv("REQUEST_TOPIC", f"kev1-{UK}-requests") + monkeypatch.setenv("ER_USERKEY", "gh-alice-typo") + with pytest.raises(SystemExit, match="does not have the shape"): + ercfg.load() + + +def test_the_runner_accepts_a_renamed_single_tenant_topic_without_a_userkey(monkeypatch): + """`k8s/base/configmap.yaml` ships `REQUEST_TOPIC: kev1-requests` for the + Phase 1 single-tenant deployment, with no ER_USERKEY. Renaming the default is + not a tenancy signal; the old refusal — any non-default topic — refused to boot + exactly this deployment.""" + from eventrunner import config as ercfg + monkeypatch.setenv("REQUEST_TOPIC", "kev1-requests") + monkeypatch.delenv("ER_USERKEY", raising=False) + assert ercfg.load().userkey == "" + + def test_the_runner_accepts_a_per_user_topic_with_a_userkey(monkeypatch): from eventrunner import config as ercfg monkeypatch.setenv("REQUEST_TOPIC", f"kev1-{UK}-requests")