SSE: POST /sse/subscribe does not check the caller may see what it subscribes to

[vincent-psarga]

The gap

POST /sse/subscribe (apps/api/src/app/sse/sse.controller.ts) takes an eventType and an arbitrary params array and passes both to sseService.subscribeUser without checking that the caller may see the thing params names. Any authenticated user who knows a space id can therefore subscribe to that space's channel, whatever organization it belongs to.

Two event types are affected today:

• SPACE_CONTENT_CHANGED — added in Refacto/use sse on packages page #518, scoped by space id;
• CHANGE_PROPOSAL_UPDATE — pre-existing.

What leaks

Activity, not content. Both events are deliberately thin — SPACE_CONTENT_CHANGED carries { organizationId, spaceId } and nothing else, precisely because a subscriber is not checked against the space it names, and every subscriber learns what changed by refetching through endpoints that do check. So a subscriber to a space they do not belong to learns that something changed in it, and when — not what, and not enough to read any of it.

That is a low ceiling, which is why #518 shipped rather than widening into this. It is still a timing side channel across organization boundaries, and it should not be the reason the payload has to stay thin forever.

What would close it

A membership check in the subscribe endpoint, per event type, before the subscription is registered — space-scoped types resolve params[0] as a space and check the caller belongs to it; unknown event types are refused rather than allowed by default. Both event types above gain it at once, so CHANGE_PROPOSAL_UPDATE subscribers need checking against the same rule.

Raised by Greptile on #518 (P2, security), and called out in that PR's own reviewer notes.

[cteyton]

(generated by an AI agent)

Hi @vincent-psarga, the context is clear, and a couple of clarifications would help the reviewer know exactly what to test:

1. What does a caller get now when a subscription is refused, and is DISTRIBUTION_STATUS_CHANGE in scope too? Something like: "A space the caller isn't a member of (another org's, or one in their own org they never joined) answers 404 'This does not exist, or you do not have access to it.', the same for an organization other than theirs on DISTRIBUTION_STATUS_CHANGE, and an unknown event type answers 400. The 403 in the 4ac2e812a message was replaced by e1e7254b2."
2. How should the reviewer test it? Something like:
   1. Set up two organizations, and in org A a space the user hasn't joined
   2. As a user of org A, POST /sse/subscribe with SPACE_CONTENT_CHANGED and a space id of org B, then the unjoined space → both refused with 404
   3. Same call on one of the user's own spaces → subscribed, and live refresh still works in the app
   4. An invented event type → 400
3. Which commits carry it? Looks like 5 commits, from 3ef6eb5b5 to b24ffa882, with 4ac2e812a as the main one. Is that all?

If you can add the answers to the description, the reviewer will find everything in one place. Thanks!

[vincent-psarga]

// Paste into the browser devtools console while logged in to Packmind.
// Uses the session cookie, so it drives exactly the path the frontend uses.
(async () => {
  const get = (p) => fetch(`/api/v0${p}`, { credentials: 'include' });
  const sub = (eventType, params, path = 'subscribe') =>
    fetch(`/api/v0/sse/${path}`, {
      method: 'POST',
      headers: { 'Content-Type': 'application/json' },
      credentials: 'include',
      body: JSON.stringify({ eventType, params }),
    });

  const me = await (await get('/auth/me')).json();
  const orgId = me.organization.id;
  const { spaces } = await (await get(`/organizations/${orgId}/spaces`)).json();
  const mySpaceId = spaces[0].id;
  const strangerId = '00000000-0000-4000-8000-000000000000';

  // 404 = the subject does not exist OR is not yours — one answer for both, so
  //       the caller cannot tell them apart (that was the side channel).
  // 400 = the request is wrong on its face: unknown event type, or params that
  //       do not name a subject in the shape the event type is scoped by.
  // 403 is deliberately never used here: it would confirm the subject exists.
  const cases = [
    ['a space I belong to',           'SPACE_CONTENT_CHANGED',      [mySpaceId],  200],
    ['a space I do not belong to',    'SPACE_CONTENT_CHANGED',      [strangerId], 404],
    ['change proposals, my space',    'CHANGE_PROPOSAL_UPDATE',     [mySpaceId],  200],
    ['change proposals, stranger',    'CHANGE_PROPOSAL_UPDATE',     [strangerId], 404],
    ['my own organization',           'DISTRIBUTION_STATUS_CHANGE', [orgId],      200],
    ['another organization',          'DISTRIBUTION_STATUS_CHANGE', [strangerId], 404],
    ['no organization named',         'DISTRIBUTION_STATUS_CHANGE', [],           400],
    ['a targeted event',              'USER_CONTEXT_CHANGE',        [],           200],
    ['an event type nobody publishes','NOTIFICATION',               [],           400],
    ['an invented event type',        'SOMETHING_INVENTED',         [mySpaceId],  400],
    ['a param that is not an id',     'SPACE_CONTENT_CHANGED',      ["'; drop table spaces; --"], 400],
  ];

  const rows = [];
  for (const [what, eventType, params, expected] of cases) {
    const status = (await sub(eventType, params)).status;
    rows.push({ what, eventType, expected, got: status, ok: status === expected ? '✅' : '❌' });
  }

  // Unsubscribe stays open, including for a subscription that was refused.
  const cleanup = (await sub('SPACE_CONTENT_CHANGED', [strangerId], 'unsubscribe')).status;
  rows.push({ what: 'unsubscribe after a refusal', eventType: '—', expected: 200, got: cleanup, ok: cleanup === 200 ? '✅' : '❌' });

  console.table(rows);
  console.log(rows.every((r) => r.ok === '✅') ? 'ALL PASS' : 'SOMETHING FAILED');
})();

A script to test the various subscribers

[quentinlebourles-packmind]

LGTM ✅

[cteyton]

LGTM ✅

Really ?
Damn we could not make it work this morning with @vincent-psarga :D
Did you get the full matrix in green?

[quentinlebourles-packmind]

Yup, really. Only 3 failling And if i said look good to me, its because the failing one are out of the scope of this ticket no?

[cteyton]

We'll wait for confirmation from @vincent-psarga but from what I understood, this matrix should be ALL green:

[quentinlebourles-packmind]

I've only got those 3 ^^

[cteyton]

Ok so it needs to be clarified

[quentinlebourles-packmind]

what eventType expected got ok
0 a space I belong to SPACE_CONTENT_CHANGED 200 200 ✅
1 a space I do not belong to SPACE_CONTENT_CHANGED 404 404 ✅
2 change proposals, my space CHANGE_PROPOSAL_UPDATE 200 200 ✅
3 change proposals, stranger CHANGE_PROPOSAL_UPDATE 404 404 ✅
4 my own organization DISTRIBUTION_STATUS_CHANGE 200 200 ✅
5 another organization DISTRIBUTION_STATUS_CHANGE 404 404 ✅
6 no organization named DISTRIBUTION_STATUS_CHANGE 400 400 ✅
7 a targeted event USER_CONTEXT_CHANGE 200 200 ✅
8 an event type nobody publishes NOTIFICATION 400 400 ✅
9 an invented event type SOMETHING_INVENTED 400 400 ✅
10 a param that is not an id SPACE_CONTENT_CHANGED 400 400 ✅
11 unsubscribe after a refusal — 200 200 ✅
ALL PASS debugger eval code:47:11