Skip to content

/graphql/ws has no per-client connection cap, idle timeout, or rate limit #422

Description

@PierrunoYT

/graphql/ws is mounted after the optional_signature layer (server.rs:67), so it is anonymous by design. Nothing bounds how many connections one client may hold, and each subscriber holds a broadcast::Receiver for the life of the connection.

What is and is not at issue

The visibility reasoning is sound and documented at graphql/subscription.rs:15-23: the resolver has no caller identity, so its safety rests entirely on the write side only publishing announce-gated events. That is a clearly stated single-point invariant and this issue does not dispute it.

What is missing is the resource half. The route has:

  • no per-client connection cap
  • no idle timeout
  • no rate limit (it sits outside every rate_limit_by_ip layer in build_router)

So the ceiling on concurrent anonymous subscriptions is whatever the process and the platform allow, and each one costs a receiver, a task, and a socket.

Severity

Low. The blast radius is the node's own connection and memory budget, an operator can cap connections at the edge, and nothing crosses a trust boundary. What earns it an issue is that every other anon-reachable route in build_router did get a brake — push, create, /ipfs/{cid}, /sync/trigger, /sync/notify — with the reasoning written down each time. This one looks like it was missed rather than decided.

Note that taskEvents on the same endpoint is separately covered by #329 for what it streams; this is about how many streams there can be, not what is in them.

Fix direction

Cap concurrent subscriptions per resolved client key. PerCallerConcurrency in rate_limit.rs is already the right shape — RAII permit, key removed at zero, reject-before-insert at the map cap — and holding a permit for the connection's lifetime is exactly its intended use.

Add an idle timeout so an abandoned connection releases its permit without waiting on TCP.

client_key resolution for a WebSocket upgrade needs checking; the per-IP middleware runs on the upgrade request, so ConnectInfo should be available at the point the permit is taken.

Validation status

Verified by reading server.rs and graphql/subscription.rs. Not verified by opening connections against a running node, so the practical ceiling is unmeasured.


Found during an external audit pass. Duplicate-checked against open and closed issues; no existing coverage found, but the search used a limited keyword set.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind:securityVulnerability fix or hardeningsev:lowCosmetic, cleanup, or nice-to-havesubsystem:apiNode REST API request/response surface

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions