Communications: Accepted Risks
The known weaknesses in the communications platform (notifications, email, live data) that were found, understood, and deliberately not fixed, why each is accepted, and the pre-agreed condition under which "accepted" turns back into "fix it".
An accepted risk is not an unknown one. Each entry states what the weakness is, why shipping with it is the right call, and what would change the decision. Those trigger conditions are the part that matters most if you operate the comms platform, evaluate it, or pick up work on 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 to 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 are listed under Closed entries with what now holds. A future risk gets R8; no number is ever reused.
Summary
| # | Risk | Status | Exposure | Reversal trigger |
|---|---|---|---|---|
| 1 | Revoked membership keeps its websocket | Closed: revocation closes the socket | none | none |
| 2 | WS connect does not re-check app enablement | Closed: read and write use one check | 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 serves a real login | Residual: no compose bundle, DNS/TLS or real IdP | none |
| 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
POST /api/machine/comms/emit deliberately does not error on non-members: user_ids are filtered against the tenant's membership and non-members are silently dropped, so a caller who names a non-member still gets 202. That is the anti-oracle property, and the reason emit does not confirm or deny a user's existence with a status code. But the 202 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.
It is accepted because 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 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
Machine users are environment-scoped: one backend registers per (application, environment) and serves every enabled tenant. Comms emit and publish authorize the body's application_id by checking that the app belongs to the credential's environment's project and is enabled for the target tenant. 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 the documented cross-repo contract, not a divergence from it.
It is accepted because the project is the trust boundary the platform 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 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, such as 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 eventual fix needs no schema change, because the MachineUser row is already per-(application, environment): 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 release 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)
The communications.email.from address is validated for shape at write time. By default it is not checked against any list of domains the deployment is authorized to send as, so an administrator holding tenant.apps.configure can set a from on a domain with no SPF or DKIM alignment.
An optional operator allowlist narrows this. 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. 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, and 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 consequence is deliverability, not privilege: reading or writing the communications config block requires tenant.apps.configure, so an ordinary tenant member cannot set the from or the HTML layout template.
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
The outbox worker claims a row before sending it (a compare-and-swap on last_attempt_at). 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.
It is accepted because 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, such as 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 keep their original numbers and anchors so that references to them elsewhere still resolve. Each states what now holds.
1. Revocation closes the member's websockets (closed)
Revocation pushes. Removing a member from a tenant (removeUserFromTenant or deleteUser2Tenant) closes exactly the sockets held by that (user, tenant) pair, and deleting a tenant closes every socket for that tenant. The 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.
The close code is 4401, the contract's "identity absent/invalid" code, because its client behaviour is already right: a full reconnect after backoff, on which the connect-time membership check refuses the socket properly.
The max-lifetime sweeper (12 hours by default) stays as the backstop for anything that is not a membership revocation. The revocation hook cannot fail and cannot block the membership change, 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 checks Tenant2Application and communications.enabled (closed)
Both the websocket upgrade and the REST inbox require an enabled Tenant2Application and a resolved communications.enabled, through the same configuration resolution that emit and 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 the revocation close stays reserved for membership. The residual is bounded by the connect-time gate on the next reconnect plus the 12 h 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 cover the proxy hop (closed)
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. It checks that:
- 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 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 is a deployment, as opposed to the proxy hop:
- 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 small 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 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 claim, covered by the bundle's own CI to the extent a runner can prove it.