Communications — Accepted Risks
The Wave 6 communications platform shipped with seven known weaknesses that were found, understood, and deliberately not fixed. This page is the record of that decision, and of what has happened to it since.
A Wave 6 follow-up pass closed three of the seven (R1, R2, R5) and narrowed a fourth (R6). Three stand unchanged (R3, R4, R7). The follow-up also found three defects that were never accepted because nobody knew they were there; they are recorded in Defects the proxy E2E found, because they are the reason the R5 work mattered.
An accepted risk is not an unknown one. Each entry below states what the weakness is, why shipping with it was the right call, and — the part that matters most — what would change the decision. If you are evaluating Productify's comms platform, operating it, or picking up work on it, read the trigger conditions: they are the pre-agreed points at which "accepted" turns back into "fix it".
Nothing here is a substitute for the Access Model, which describes the controls that are enforced. For issues that are simply not done yet — as opposed to consciously accepted — see Known Issues.
Related reading
The comms platform itself is documented in Add Notifications to Your App and Live Data Updates.
Identifiers are permanent
R1–R7 are stable identifiers, not positions in a list. Changelogs and READMEs in the manager, proxy, deployments, cli and integration repositories cite them by number, so a closed entry keeps its number and its anchor rather than being deleted or renumbered — closed entries move to Closed entries and say what closed them. A future risk gets R8; no number is ever reused.
Summary
| # | Risk | Status | Exposure | Reversal trigger |
|---|---|---|---|---|
| 1 | Revoked membership keeps its websocket for up to 12h | Closed — revocation now pushes | — | — |
| 2 | WS connect does not re-check app enablement | Closed — read climbs the write ladder | Residual: no mid-connection close | Data frames carrying authoritative payload |
| 3 | recipient_count is a weak membership oracle | Accepted | Membership confirmation, audited and visible | An unaudited or invisible emit path |
| 4 | A machine credential reaches every app in its project | Accepted | Lateral movement within one project | Multi-team projects with distinct trust boundaries |
| 5 | The E2E does not cover the proxy hop | Closed — a real Caddy now serves a real login | Residual: no compose bundle, DNS/TLS or real IdP | — |
| 6 | No domain allowlist on the email from | Narrowed — optional allowlist, off by default | Unaligned sender domains where unconfigured | Shared-tenancy hosting, or a deliverability incident |
| 7 | A crash between outbox claim and send drops an email | Accepted | One lost notification email | A notification class where loss beats duplication |
Still accepted
3. recipient_count is a weak membership oracle
What it is. emit deliberately does not error on non-members: user_ids are filtered against user2tenant and non-members are silently dropped, so a caller who names a non-member still gets 202 — the anti-oracle property, and the reason emit does not confirm or deny a user's existence with a status code. But the response body carries recipient_count, and emitting to one uuid returns 1 for a member and 0 for a non-member. That is a one-bit membership oracle.
Why we accepted it. Every ingredient of an attack is already an authorization the attacker holds or a cost they pay:
- the caller must already hold a valid machine credential for that tenant and application — they are an authorized backend, not an outsider;
- they must know the victim's UUID a priori, which the platform does not hand out;
- every probe writes a real, audited notification that the victim receives and can see.
So it confirms a fact about a user whose UUID you already have, to a party already trusted to send that user notifications, in the noisiest possible way. Removing recipient_count would cost legitimate callers their only feedback on fan-out size — a genuinely useful number for a backend that just addressed all_tenant_members — to close a channel with no realistic attacker.
What would change the decision. An emit path that is neither audited nor visible to the recipient (a silent or draft-notification mode, a dry-run flag). That combination removes the cost of probing, and at that point recipient_count should be bucketed or dropped for the silent path.
4. A machine credential reaches every application in its project
What it is. Machine users are environment-scoped (Wave 3): one backend registers per (application, environment) and serves every enabled tenant. Comms emit/publish authorize the body's application_id by checking that the app belongs to the credential's environment's project. So a credential issued for application A can emit and publish as application B, provided B is in the same project and enabled for the target tenant.
This is exactly as specified in the Wave 6 cross-repo contract (§0) — it is the documented behavior, not a divergence from it.
Why we accepted it. The project is the trust boundary the Wave 3 model draws, and comms follows it rather than inventing a second, tighter one in one subsystem. Applications in a project are operated by the same team and deployed from the same place; the credential is a server-side secret held by a backend that the project's operators run. Tightening comms alone would make comms inconsistent with the rest of the machine API without actually narrowing the blast radius of a leaked credential, which reaches non-comms machine routes anyway.
What would change the decision. Projects that span trust boundaries — two teams, or a first-party and a third-party application, sharing one project. Delegated or customer-issued machine credentials would force it immediately.
The fix, when it comes. The MachineUser row is already per-(application, environment), so the tighter check is available without a schema change: require the body's application_id to match the credential's own application. It is a narrowing change and would break any backend currently relying on the project-wide reach, so it belongs in a wave with a deprecation window — and it should be applied across the machine API, not just to comms.
6. No domain allowlist on the email from address — narrowed
What it is. The communications.email.from address is validated for shape at write time. It was not checked against any list of domains the deployment is authorized to send as, so an administrator holding tenant.apps.configure could set a from on a domain with no SPF or DKIM alignment.
What changed. A follow-up added an optional operator allowlist (manager 8eea4fe). When comms.email_from_domains is set — environment variable PFY_COMMS_EMAIL_FROM_DOMAINS, comma-separated — a from address whose domain is not on the list is rejected at write time.
Matching is exact and case-insensitive. A subdomain does not match its parent: an allowlisted acme.com does not admit mail.acme.com. That is deliberate, because SPF and DKIM alignment is established per-domain — an aligned acme.com says nothing about a subdomain, and suffix matching would quietly admit notacme.com as well. An operator who wants a subdomain lists the subdomain.
What stays accepted. The allowlist is off by default, and unset means any parseable bare address — exactly the original behavior. That is deliberate operator policy rather than a gap: which domains a deployment may send as depends on its DNS, its ESP and its contracts, so the Manager takes the list rather than inventing one. Turning enforcement on by default would break every existing deployment on upgrade. A deployment that has not configured comms.email_from_domains carries the original risk in full, and the residual consequence is unchanged — deliverability, not privilege. The genuine escalation risk here, an ordinary tenant member setting both the from and the HTML layout template, was closed in Wave 6 by requiring tenant.apps.configure to read or write the communications config block.
What would change the decision. Shared-tenancy hosting where tenants do not trust each other. There, an empty list should be a misconfiguration that refuses to boot, not consent — the default would have to invert. A deliverability incident traced to an unaligned sender would also force the list to be populated, though not necessarily to become mandatory.
7. A crash between outbox claim and send loses an email
What it is. The outbox worker claims a row before sending it. If the process dies between the claim and the successful send, the row stays claimed and the message is never delivered. The failure mode is a missed email, not a duplicate one.
Why we accepted it. This is a chosen trade, not an unhandled case. Claiming after sending would invert it: a crash after the send but before the bookkeeping would redeliver, and duplicate notification emails are worse than a missing one. A missing email degrades to the notification the user already has — the bell in the app is authoritative, populated over the websocket and the inbox REST route, and the email is a nudge for a user who has not come to look. A duplicate email is user-visible noise with no recovery.
Claiming before sending is also what makes the worker safe to run in more than one process, which the alternative would not be.
What would change the decision. A notification class where a missed message is materially worse than a duplicate — security alerts, or anything with a compliance-driven delivery obligation. That calls for per-class delivery semantics and an explicit retry with idempotency keys, not for flipping the global trade.
Closed entries
These are kept, with their original numbers and anchors, so that references to them elsewhere still resolve and still mean what they meant.
1. A revoked membership keeps its websocket for up to 12 hours — closed
What it was. A comms websocket's identity is fixed at connect, and the membership re-check ran exactly once, at the upgrade. Nothing closed an already-open socket when the membership underwriting it went away, so a user removed from a tenant kept receiving that tenant's notifications and subscribed data topics until the max-lifetime sweeper retired the connection — 12 hours by default. The accepted exposure was that 12-hour tail, for a user who was connected at the moment of revocation.
What closed it (manager 890956d). Revocation now pushes. Hub.CloseForUser(user, tenant) closes exactly the sockets matching that pair, and Hub.CloseForTenant(tenant) closes a deleted tenant's sockets in one scan. The (user, tenant) pair, not the user, is the unit: one user may hold a socket per comms-enabled app times open tabs, and may legitimately be a member of several tenants, so closing by user alone would drop connections whose membership is untouched.
All three revocation paths are hooked — removeUserFromTenant (tenant admin), deleteUser2Tenant (operator) and deleteTenant, which is told about the tenant rather than an enumerated member list precisely so it cannot miss a membership created mid-delete.
The close code is 4401, the contract's existing "identity absent/invalid" code rather than a new one, because its client behavior was already right: a full reconnect after backoff, on which the connect-time membership check refuses the socket properly. No frontend or proxy change was required.
What remains. The max-lifetime sweeper stays as the backstop for anything that is not a membership revocation. The hook is total — no error, no context, no way to fail — because comms is optional at runtime and a membership that survived because the socket layer was unavailable would be far worse than a socket that lingers until the sweeper.
2. WS connect did not check Tenant2Application or communications.enabled — closed
What it was. The websocket upgrade authorized user2tenant membership only. It did not re-check that the application was enabled for that tenant (Tenant2Application), nor that communications.enabled resolved true for the (tenant, app) pair — both of which the machine-authed emit/publish writes check. The proxy-identity REST inbox omitted the same checks. This was accepted as a deliberate, documented read/write asymmetry that was unreachable in practice, since the app id is a path segment the rendered vhost writes and never a client choice.
It was unreachable, but it was not harmless: a tenant that had switched comms off, or whose entitlement to the application had been revoked, could still open a socket or poll the inbox and read back every notification historically emitted to it.
What closed it (manager 5f4564d). Both the websocket upgrade and the REST inbox now require an enabled Tenant2Application and a resolved communications.enabled, through the same comms.ResolveConfig call that emit/publish use — read and write climb one ladder, from one implementation. The websocket refuses with close code 4401. The REST inbox returns 404 for a missing or disabled entitlement and 403 {"error":"comms_disabled"} when the pair is entitled but comms is switched off.
Residual, still accepted. An already-open socket is not closed when comms is disabled, or when the Tenant2Application entitlement is revoked mid-connection. Neither of those is a membership loss — the user remains a member of the tenant, and every frame they receive is still re-read through their own scoped query — so Hub.CloseForUser stays reserved for revocation. The residual is bounded by the connect-time gate on the next reconnect plus the 12h sweeper.
What would change the decision. Data frames carrying authoritative payload content rather than change hints. At that point a lingering socket would be disclosing data rather than signalling that data moved, and disabling comms would need to close sockets as firmly as revoking membership does.
5. The end-to-end tests do not cover the proxy hop — closed
What it was. The Wave 6 comms E2E scenarios ran against a real Postgres, the real Manager binary and a real SMTP sink — but they injected the headers a proxy would inject and called the Manager directly. The rendered @comms and @comms_ws routes, and therefore the whole proxy layer, were never exercised. This was recorded as the weakest entry on the page: the Wave 6 X-Proxy-Auth defect had lived in precisely that untested layer, and the guard left behind was a golden-file assertion about rendered config text, not evidence that a request survives the hop.
What closed it (deployments d5f32c3, fc2428b, 29abb22). deployments/e2e/proxy.test.mjs drives a browser-equivalent OIDC login through a real Caddy, built with the proxy repo's productify_* plugin, serving the Manager's own rendered vhost — rendered by the Manager's own renderer, not by a fixture. Verified end to end:
- a logged-in browser is served its own inbox over the rendered route (
200); - the websocket upgrade crosses the proxy hop;
- a multi-tenant browser session lists its tenants, chooses one, and is served that tenant;
- a mutation test proves the coverage is real: removing
header_up X-Proxy-Authfrom the running config turns both comms routes into401.
The proxy scenarios are not optional and are never skipped — if the pieces are missing the harness exits 78 naming what is absent, because a run that quietly dropped the one boundary the other scenarios cannot reach would be worse than no run at all.
What remains uncovered. This is coverage of the proxy hop, not of a deployment. Still not exercised:
- the published compose bundle — the harness builds the Manager and the proxy from checkouts and needs the checkout's migrations, so the shipped image path is a different workflow;
- real DNS and TLS — the rendered vhosts are reached over plain HTTP on loopback ports;
- Pocket ID itself — the identity provider is a ~200-line test double (
deployments/e2e/lib/oidc.mjs), the one substituted component. What is real within it is the authorization-code flow, the RS256 signing key and the nonce check; what is a double is Pocket ID's own discovery document, login UI and user store.
So a request now provably survives the rendered hop with a real session. Whether the shipped bundle stands up on real DNS with the real IdP is a separate, still-untested claim.
Defects the proxy E2E found
Recorded here because they explain why R5 was worth closing: none of these was an accepted risk. All three were unknown, all three are fixed, and all three were invisible to every unit and golden-file test that existed.
1. The rendered Caddyfile never parsed (manager e4daca1). The renderer emitted the productify_before_auth, productify_esb_auth and productify directives without the with keyword their parsers require. A Caddyfile is all-or-nothing, so a real proxy receiving a real render rejected the whole config and kept its last-good one — every rendered vhost silently never took effect. Pre-existing since Wave 3.
2. Rendered routes could not authenticate a browser (manager 20f4d32, proxy 904a0ba). productify_oauth_validation — which maps caddy-security's X-Token-Subject to X-User-Id — was missing from the rendered comms chains and from the compound productify_protect snippet, so every rendered vhost's main application route was affected too. It fails open, so the omission produced no error anywhere; a browser session simply arrived without identity.
3. The multi-tenant browser chooser 403'd (proxy 74cedaa, af212e6, manager d5d44d8). productify_tenant_selector ran before authorize and identified callers only from the Authorization header — PAT-only. A browser portal session therefore had no identity at the selector, and the chooser refused the exact caller it exists to serve. Fixed by reordering the chain to sanitize → auth → authorize → tenant_selector → oauth_validation → … and teaching the selector to read X-Token-Subject. Reading that header is safe only because authorize has by then validated the session and productify_sanitize still strips forged inbound copies at the edge. A side effect worth noting: the tenant cookie's user binding and the live membership re-check now actually apply to browser sessions, where previously they were no-ops.
Harness hazard
The E2E harness itself has a known trap-and-port issue that can make a run look meaningful when it is not. It is a test-harness hazard rather than a product risk, so it is filed under Known Issues → E2E harness.