Skip to content

stream:subscribe has no ownership check: any client can join any stream room #520

Description

@Xhristin3

Problem

stream:subscribe lets any authenticated socket join any stream's room with no ownership or visibility check. StreamsGateway.handleSubscribe (api/src/gateways/streams.gateway.ts) verifies only that the client is authenticated:

@SubscribeMessage("stream:subscribe")
handleSubscribe(client: AuthenticatedSocket, payload: { streamId?: string | number } = {}) {
  if (!client.data?.userId) {
    return { ok: false, error: "unauthenticated" }
  }
  ...
  const room = this.roomFor(payload.streamId)
  void client.join(room)
  return { ok: true, room }
}

Once joined, the socket receives every broadcast to stream:<id> — including stream:started/stream:stopped/stream:error for streams it does not own and that are private (the default per CreateStreamDto and the streams_visibility migration). The same userId-based check is absent from the notification room path, but user rooms are scoped by the server (user:<id>), so the exposure is the stream rooms.

Today the leak is latent because the emit helpers have no production callers (see the realtime-emits issue), but the moment status broadcasts are wired — or any future payload is broadcast to a stream room — any authenticated user can observe another user's private stream lifecycle. The platform's visibility model (packages/types/src/stream.ts: private streams are owner-only) is enforced on the REST layer by StreamOwnershipGuard; the socket layer has no equivalent.

Root cause

// api/src/gateways/streams.gateway.ts — handleSubscribe
if (payload.streamId === undefined || payload.streamId === null) {
  return { ok: false, error: "streamId required" }
}
const room = this.roomFor(payload.streamId)
void client.join(room)          // ← no ownership / visibility check before joining

Why this is architecturally hard

  1. The ownership primitive already exists — StreamOwnershipService.ownsStream() (api/src/common/guards/stream-ownership.service.ts) powers StreamOwnershipGuard — but handleSubscribe is synchronous today and ownership is a database round trip. The handler must become async (or pre-load ownership at connection time), and the failure mode must be defined: reject the join ({ ok: false, error: "forbidden" }) versus silently not joining.
  2. Visibility semantics for rooms need a decision: private streams are owner-only, but should any authenticated user be able to subscribe to a public stream's status room? The REST layer allows authenticated users to list public streams but not read a single public stream (GET /streams/:id is ownership-guarded). The socket rule should be chosen deliberately and documented, and ideally pinned in the contract suite (tests/contracts) so it cannot drift.
  3. The gateway must not become a second source of truth for the visibility model. The clean design reuses StreamOwnershipService and the same "public or owner" predicate the list endpoint uses (api/src/streams/repository/streams-db.repository.ts listPaginated), so a visibility change is fixed in one place.

Acceptance criteria

Contract

  • stream:subscribe for a stream the client does not own and that is not public returns { ok: false, error: "forbidden" } and does not join the room.
  • A client that owns the stream, and a client subscribing to a public stream (per the visibility rule chosen in the design), receive the room broadcasts after subscribing.
  • stream:unsubscribe behaviour is unchanged for rooms the client successfully joined.

Tests

  • Gateway tests cover: owner subscribes and receives broadcasts; non-owner private stream is rejected; non-owner public stream follows the chosen rule; unauthenticated subscribe is still rejected.
  • The ownership lookup reuses StreamOwnershipService (assert via the service being exercised, not re-implemented in the gateway).

Documentation

  • The StreamsGateway JSDoc documents the subscription visibility rule for private and public streams.

Out of scope

Broadcasting status events (the realtime-emits issue), and notification-room changes.

Getting started

Real files in scope: api/src/gateways/streams.gateway.ts, api/src/common/guards/stream-ownership.service.ts, api/src/gateways/streams.gateway.spec.ts, api/src/streams/repository/streams-db.repository.ts (visibility predicate reference).

Verify with:

cd api && npm run typecheck && npm test

Good first files to read: api/src/gateways/streams.gateway.ts (handleSubscribe), api/src/common/guards/stream-ownership.service.ts.

Activity

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

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third CampaignbugSomething isn't workingsecuritySecurity related issueswebsocketWebSocket / real-time features

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions