From 9b36d921e4a00d77855458057fa06a3ea431f252 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:58:15 -0700 Subject: [PATCH] fix(connectors): relay takes the pod write path's membership, so a departed creator cannot relay (TASK-161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `leavePod` filters `members` and never clears `createdBy`. The app's write path (`createMessage`, the socket post path) checks `pod.members` only and 401s that person; connector relay checked `utils/isPodMember`, whose first clause counts `createdBy` as a member. So a pod's creator who left it could not post in the pod from Commonly but could still relay into it from Telegram or Slack, still receive its messages in their private chat, and could still turn new gates on for it — under a comment in `integrations.ts` stating the invariant that breaks ("it must be a pod that owner can still write to"). Wren ruled option A: connector sites take the pod write path's membership, one definition. That definition is `isListedPodMember`, beside `isPodMember` in `utils/isPodMember.ts` — `pod.members` alone, named for the mechanism (`Pod.members` "does not always list" the creator, which is what the permissive clause answers, and never a ruling that leaving leaves membership intact). `connectorRelayPolicy` re-exports it and every connector site in the census reads it through that module; no connector site calls `isPodMember` any more: - `isRoutedPodTarget` — outbound relay for both bridges, the routed-quote-reply check, and decision-card delivery. - `slackBridgeService` / `telegramBridgeService` inbound authorship. - `decisionCardReply` (a card ruling written on behalf of a departed member) and `decisionCardReconcileService` (the closing line). - `routes/integrations.ts` — the connector's own pod, its active inbound destination, and every requested gate key. - `routes/installables.ts` — the install route, which is how a connector enters a pod and seeds that pod's gate, and the Slack bind confirm. - `installable/eventHandlers.ts` `activeHandlersForPod` — dropped the `pod.createdBy` union from `memberIds`, the selector half of the same rule. - `installable/installableReconciler.ts` `sweepOrphanedGates` — the half that closes the loop this PR would otherwise open: its own comment says it prunes gates "so the owner's gate list does not promise a pod they left", and the permissive predicate left it blind to the departed creator, whose ON switch this PR makes undeliverable (Vera 74663). Fixture fallout, disclosed rather than tidied: three route suites stubbed the whole membership module with `jest.fn(() => true)`, which is also why they were not exercising the write gate at all. They build real pods with the caller listed, so the stub is deleted rather than re-pointed, and they now run the predicate. `integrations.validation.test.js`'s mocked pod listed nobody and leaned on the same clause; it now lists its caller. `integrations.linkedUserId` had two 200 arms whose pods claimed `members: []` while asserting "pods they belong to" — those fixtures now list the owner, so they still mean what they say. Out of scope, measured: `activityService`, `decisionRequestService`, `routes/activity.ts` and `routes/podInvites.ts` keep `isPodMember` — they are the app/agent surfaces Vera measured as unreachable by the bypass (bots cannot hold a user JWT), not connector paths. --- .../unit/routes/installables.test.js | 20 ++++++++ .../integrations.discordTokenCopy.test.js | 1 - .../routes/integrations.linkedUserId.test.js | 39 +++++++++++++-- .../integrations.manifestStatus.test.js | 1 - .../routes/integrations.routingState.test.js | 1 - .../routes/integrations.validation.test.js | 34 ++++++++++++- .../routes/slackOAuth.installables.test.js | 19 +++++++ .../services/connectorRelayPolicy.test.js | 17 ++++++- .../decisionCardReconcileService.test.js | 47 ++++++++++++++++++ .../decisionCardReply.bridges.test.js | 14 ++++++ .../services/installableEventHandlers.test.js | 38 +++++++++----- .../installableInstallationService.test.js | 49 ++++++++++++++++++- .../unit/services/slackBridgeService.test.js | 28 +++++++++++ .../services/telegramBridgeService.test.js | 42 ++++++++++++++++ backend/routes/installables.ts | 9 ++-- backend/routes/integrations.ts | 22 +++++---- backend/services/connectorRelayPolicy.ts | 24 +++++++-- .../services/decisionCardReconcileService.ts | 5 +- backend/services/decisionCardReply.ts | 4 +- backend/services/installable/eventHandlers.ts | 16 +++--- .../installable/installableReconciler.ts | 12 +++-- backend/services/slackBridgeService.ts | 7 +-- backend/services/telegramBridgeService.ts | 7 +-- backend/utils/isPodMember.ts | 20 ++++++++ 24 files changed, 416 insertions(+), 60 deletions(-) diff --git a/backend/__tests__/unit/routes/installables.test.js b/backend/__tests__/unit/routes/installables.test.js index 1c4845e42..78e1e7e2a 100644 --- a/backend/__tests__/unit/routes/installables.test.js +++ b/backend/__tests__/unit/routes/installables.test.js @@ -125,6 +125,26 @@ describe('installable connector routes', () => { expect(installationService.install).not.toHaveBeenCalled(); }); + it('rejects a departed CREATOR before any install row is claimed', async () => { + // TASK-161: `leavePod` filters `members` and never clears `createdBy`, so + // this pod has no members and still names the caller as its creator. The + // install route used to admit them through the creator clause; installing + // seeds the pod's gate ON, so it must take the pod's write path instead. + Pod.findById.mockResolvedValue({ + _id: podId, + createdBy: { toString: () => '64b64c48c4f37a6b2f34c111' }, + members: [], + }); + + const res = await request(app) + .post('/api/installables/telegram/install') + .set(auth) + .send({ podId }); + + expect(res.status).toBe(403); + expect(installationService.install).not.toHaveBeenCalled(); + }); + it('rejects an invalid podId without querying a pod or claiming an install', async () => { const response = await request(app) .post('/api/installables/telegram/install') diff --git a/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js b/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js index e8ad1f12c..12c8ca5a8 100644 --- a/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js +++ b/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js @@ -29,7 +29,6 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({ listIntegrationsRateLimit: (_req, _res, next) => next(), })); jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn(() => true) })); -jest.mock('../../../utils/isPodMember', () => jest.fn(() => true)); jest.mock('jsonwebtoken', () => ({ sign: jest.fn(() => 't'), verify: jest.fn(), decode: jest.fn() })); // The create path initializes and connects the provider; only the row matters. jest.mock('../../../services/discordService', () => jest.fn().mockImplementation(() => ({ diff --git a/backend/__tests__/unit/routes/integrations.linkedUserId.test.js b/backend/__tests__/unit/routes/integrations.linkedUserId.test.js index f578e8e3e..b804df98e 100644 --- a/backend/__tests__/unit/routes/integrations.linkedUserId.test.js +++ b/backend/__tests__/unit/routes/integrations.linkedUserId.test.js @@ -220,7 +220,7 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => { it('checks every requested gate before writing any of them', async () => { Pod.findById - .mockResolvedValueOnce({ _id: allowedPodId, createdBy: 'user-1', members: [] }) + .mockResolvedValueOnce({ _id: allowedPodId, createdBy: 'someone-else', members: ['user-1'] }) .mockResolvedValueOnce({ _id: forbiddenPodId, createdBy: 'someone-else', members: [] }); const res = await request(app) @@ -239,7 +239,9 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => { }); it('allows the linked owner to write gates for pods they belong to', async () => { - Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'user-1', members: [] }); + // Listed in `members`, which is what "belong to" means — the fixture used to + // leave `members` empty and lean on `createdBy` (TASK-161). + Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'someone-else', members: ['user-1'] }); const res = await request(app) .patch(`/api/integrations/${userScopedIntegrationId}`) @@ -251,7 +253,7 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => { }); it('allows the linked owner to select a member pod as the active inbound destination', async () => { - Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'user-1', members: [] }); + Pod.findById.mockResolvedValue({ _id: allowedPodId, createdBy: 'someone-else', members: ['user-1'] }); const res = await request(app) .patch(`/api/integrations/${userScopedIntegrationId}`) @@ -262,6 +264,37 @@ describe('PATCH /api/integrations/:id — user-scoped connector gates', () => { expect(update.podId).toBe(allowedPodId); }); + it.each([ + ['created and then left', { _id: 'pod', createdBy: 'user-1', members: [] }], + ['never a member', { _id: 'pod', createdBy: 'someone-else', members: [] }], + ])('refuses a gate for a pod the linked owner %s', async (_label, podDoc) => { + // TASK-161: `leavePod` filters `members` and never clears `createdBy`, so + // the first cell is a creator who left. Both must refuse, or a connector + // could aim inbound messages at a pod its owner cannot post in. + Pod.findById.mockResolvedValue({ ...podDoc, _id: allowedPodId }); + + const res = await request(app) + .patch(`/api/integrations/${userScopedIntegrationId}`) + .send({ config: { gates: { [allowedPodId]: { enabled: true } } } }); + + expect(res.status).toBe(403); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + }); + + it.each([ + ['created and then left', { _id: 'pod', createdBy: 'user-1', members: [] }], + ['never a member', { _id: 'pod', createdBy: 'someone-else', members: [] }], + ])('refuses selecting a pod the linked owner %s as the active destination', async (_label, podDoc) => { + Pod.findById.mockResolvedValue({ ...podDoc, _id: allowedPodId }); + + const res = await request(app) + .patch(`/api/integrations/${userScopedIntegrationId}`) + .send({ podId: allowedPodId }); + + expect(res.status).toBe(403); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + }); + it('refuses selecting an active pod the linked owner is no longer a member of', async () => { Pod.findById.mockResolvedValue({ _id: forbiddenPodId, createdBy: 'someone-else', members: [] }); diff --git a/backend/__tests__/unit/routes/integrations.manifestStatus.test.js b/backend/__tests__/unit/routes/integrations.manifestStatus.test.js index 0194c20b6..2ab5bf596 100644 --- a/backend/__tests__/unit/routes/integrations.manifestStatus.test.js +++ b/backend/__tests__/unit/routes/integrations.manifestStatus.test.js @@ -21,7 +21,6 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({ listIntegrationsRateLimit: (_req, _res, next) => next(), })); jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn() })); -jest.mock('../../../utils/isPodMember', () => jest.fn(() => true)); jest.mock('jsonwebtoken', () => ({ sign: jest.fn(() => 't'), verify: jest.fn(), decode: jest.fn() })); const { MongoMemoryServer } = require('mongodb-memory-server'); diff --git a/backend/__tests__/unit/routes/integrations.routingState.test.js b/backend/__tests__/unit/routes/integrations.routingState.test.js index 57ea7c749..3b3ad552c 100644 --- a/backend/__tests__/unit/routes/integrations.routingState.test.js +++ b/backend/__tests__/unit/routes/integrations.routingState.test.js @@ -24,7 +24,6 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({ listIntegrationsRateLimit: (_req, _res, next) => next(), })); jest.mock('../../../services/dmService', () => ({ canViewPod: jest.fn() })); -jest.mock('../../../utils/isPodMember', () => jest.fn(() => true)); jest.mock('jsonwebtoken', () => ({ sign: jest.fn(() => 't'), verify: jest.fn(), decode: jest.fn() })); const { MongoMemoryServer } = require('mongodb-memory-server'); diff --git a/backend/__tests__/unit/routes/integrations.validation.test.js b/backend/__tests__/unit/routes/integrations.validation.test.js index c88faa82e..9804a0586 100644 --- a/backend/__tests__/unit/routes/integrations.validation.test.js +++ b/backend/__tests__/unit/routes/integrations.validation.test.js @@ -64,7 +64,13 @@ describe('integration manifest validation', () => { beforeEach(() => { jest.clearAllMocks(); - Pod.findById.mockResolvedValue({ _id: 'pod-1', createdBy: { toString: () => 'user-1' } }); + // The pod lists the caller: a real pod lists its creator, and the + // connector sites read `pod.members` alone (TASK-161). + Pod.findById.mockResolvedValue({ + _id: 'pod-1', + createdBy: { toString: () => 'user-1' }, + members: [{ toString: () => 'user-1' }], + }); User.findById.mockResolvedValue({ _id: 'user-1' }); consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); Integration.findOne.mockResolvedValue(null); @@ -209,4 +215,30 @@ describe('integration manifest validation', () => { expect(res.status).toBe(400); expect(res.body.missing).toEqual(expect.arrayContaining(['channelId'])); }); + + it('refuses to create a connection for a pod the caller created and then left', async () => { + // TASK-161, from Vera 74671: `:393` is the write gate, and the population it + // exists to refuse is exactly the departed creator — the one person the + // permissive predicate admitted. Every other create arm here lists the + // caller in `members`, so before this arm the site had no witness at all: + // `createdBy` merely identifies the owner and must not stand in for it. + Pod.findById.mockResolvedValue({ + _id: 'pod-1', + createdBy: { toString: () => 'user-1' }, + members: [], + }); + const before = Integration.__getLastInstance(); + + const res = await request(app) + .post('/api/integrations') + .send({ + podId: 'pod-1', + type: 'groupme', + config: { webhookListenerEnabled: true }, + }); + + expect(res.status).toBe(403); + expect(res.body.message).toBe('Access denied'); + expect(Integration.__getLastInstance()).toBe(before); + }); }); diff --git a/backend/__tests__/unit/routes/slackOAuth.installables.test.js b/backend/__tests__/unit/routes/slackOAuth.installables.test.js index cd1e4d4b5..8e12b5be1 100644 --- a/backend/__tests__/unit/routes/slackOAuth.installables.test.js +++ b/backend/__tests__/unit/routes/slackOAuth.installables.test.js @@ -271,6 +271,25 @@ describe('Slack installable OAuth routes', () => { expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[<Evil|pod>] connected'); }); + test('refuses to confirm a bind whose pod the caller created and left', async () => { + // TASK-161. `leavePod` filters `members` and never clears `createdBy`; the + // permissive predicate counted that as membership, so the confirm step — + // which stores the chat a connector will speak into — admitted a person the + // pod's own write path refuses. `slack_pod_access_denied` had no arm at all. + const pending = { + teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref', + expiresAt: new Date(Date.now() + 60_000), + }; + Integration.findOne.mockResolvedValue({ ...integration, config: { pendingBind: pending } }); + Pod.findById.mockResolvedValueOnce({ createdBy: String(ownerId), members: [] }); + + const response = await request(app).post('/api/installables/slack/confirm'); + + expect(response.status).toBe(403); + expect(response.body.code).toBe('slack_pod_access_denied'); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + }); + test('a reconnect clears the reason an earlier flip left on the row (wren 73838)', async () => { const pending = { teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref', diff --git a/backend/__tests__/unit/services/connectorRelayPolicy.test.js b/backend/__tests__/unit/services/connectorRelayPolicy.test.js index e04a9d50a..cb7ea0177 100644 --- a/backend/__tests__/unit/services/connectorRelayPolicy.test.js +++ b/backend/__tests__/unit/services/connectorRelayPolicy.test.js @@ -72,8 +72,21 @@ describe('connectorRelayPolicy — the routed-target conjunction', () => { expect(isRoutedPodTarget(target({ pod: undefined }))).toBe(false); }); - it('admits a pod whose creator is not listed in members', () => { - expect(isRoutedPodTarget(target({ pod: pod({ createdBy: 'user-1' }) }))).toBe(true); + it('refuses a pod whose creator is not listed in members', () => { + // TASK-161. `leavePod` filters `members` and never clears `createdBy`, so + // this cell IS "a creator who has left" — and the pod's own write path + // (`createMessage`) 401s them. The permissive `isPodMember` counted them as + // a member so that a pod whose `members` forgot its creator still + // authorises them; a connector must reach the same verdict as the write. + expect(isRoutedPodTarget(target({ pod: pod({ createdBy: 'user-1' }) }))).toBe(false); + }); + + it('still admits a creator who is also listed — the clause is gone, not the creator', () => { + // The complement, so the arm above cannot pass by refusing every pod that + // names a creator at all. + expect(isRoutedPodTarget(target({ + pod: pod({ createdBy: 'user-1', members: ['user-1'] }), + }))).toBe(true); }); it('refuses a missing user id rather than stringifying it into a match', () => { diff --git a/backend/__tests__/unit/services/decisionCardReconcileService.test.js b/backend/__tests__/unit/services/decisionCardReconcileService.test.js index 75eb27823..6bc349c68 100644 --- a/backend/__tests__/unit/services/decisionCardReconcileService.test.js +++ b/backend/__tests__/unit/services/decisionCardReconcileService.test.js @@ -301,6 +301,53 @@ describe('decision card closure fan-out', () => { expect((await Integration.findById(missingLink._id)).config.cards[0].closedAt).toEqual(expect.any(Date)); }); + test('sends no closing line to a creator who left, while a listed sibling still receives one', async () => { + // TASK-161, from Vera 74671: `canSendClosingLine` had no witness for the + // population the permissive predicate admitted. `createdBy` identifies the + // pod's creator; it is not membership, and this arm is the one that says so. + // The listed sibling is the control — without it a `not.toHaveBeenCalled` + // would be indistinguishable from an arm that never ran. + const thirdId = new mongoose.Types.ObjectId(); + Pod.findById.mockImplementation(() => chain({ createdBy: memberId, members: [ownerId, thirdId] })); + const origin = await Integration.create({ + podId, scope: 'user', type: 'telegram', createdBy: ownerId, isActive: true, status: 'connected', + config: { + liveRelay: true, linkedUserId: String(ownerId), chatType: 'private', chatId: 'origin', + gates: { [String(podId)]: { enabled: true, since: new Date() } }, + cards: [{ podMessageId: cardId, tgMessageId: '11', sentAt: new Date() }], + }, + }); + const leftCreator = await Integration.create({ + podId, scope: 'user', type: 'telegram', createdBy: memberId, isActive: true, status: 'connected', + config: { + liveRelay: true, linkedUserId: String(memberId), chatType: 'private', chatId: 'left-creator', + gates: { [String(podId)]: { enabled: true, since: new Date() } }, + cards: [{ podMessageId: cardId, tgMessageId: '12', sentAt: new Date() }], + }, + }); + const listed = await Integration.create({ + podId, scope: 'user', type: 'telegram', createdBy: thirdId, isActive: true, status: 'connected', + config: { + liveRelay: true, linkedUserId: String(thirdId), chatType: 'private', chatId: 'listed', + gates: { [String(podId)]: { enabled: true, since: new Date() } }, + cards: [{ podMessageId: cardId, tgMessageId: '13', sentAt: new Date() }], + }, + }); + + await fanoutDecisionClosure( + { _id: new mongoose.Types.ObjectId(), podId, messageId: cardId, ruling: { value: 'Now', byUsername: 'Sam' } }, + { via: 'workspace', integrationId: origin._id }, + ); + + expect(telegramSend.sendMessage).toHaveBeenCalledTimes(1); + expect(telegramSend.sendMessage).toHaveBeenCalledWith( + 'telegram-token', 'listed', '✓ Ruled by Sam: Now', { replyToMessageId: '13', plainText: true }, + ); + // Closure is unconditional (it is a receipt, not a delivery), so the + // departure shows up in the send and not in `closedAt`. + expect((await Integration.findById(leftCreator._id)).config.cards[0].closedAt).toEqual(expect.any(Date)); + }); + test('returns after durable closure while a sibling provider send is still pending', async () => { let releaseSend; const pendingSend = new Promise((resolve) => { releaseSend = resolve; }); diff --git a/backend/__tests__/unit/services/decisionCardReply.bridges.test.js b/backend/__tests__/unit/services/decisionCardReply.bridges.test.js index c86783881..79889b5c8 100644 --- a/backend/__tests__/unit/services/decisionCardReply.bridges.test.js +++ b/backend/__tests__/unit/services/decisionCardReply.bridges.test.js @@ -177,6 +177,20 @@ describe.each(['telegram', 'slack'])('%s decision reply', (provider) => { expect(confirmation()).not.toContain('secret'); }); + test('refuses a card reply from a pod creator who left, recording nothing', async () => { + // TASK-161, from Vera 74671: wren's ruling named this as witness (v) and the + // arm did not exist. The arm above departs through `members`; the population + // the permissive predicate admitted is the CREATOR who left — present in + // `createdBy`, absent from `members`. `assertOpen` is the other half of the + // claim: the refusal has to leave the standing ruling and the ledger alone. + Pod.findById.mockImplementation(() => chain({ name: 'Launch', members: [], createdBy: ownerId })); + await receive(); + expect(choose).not.toHaveBeenCalled(); + expect(messages).toHaveLength(0); + await assertOpen(); + expect(confirmation()).toContain("You're no longer in Launch"); + }); + test('409-ruled: loser text goes under ask, not winner; standing ruling and ledger stay unchanged', async () => { await receive(); const original = await ledger(); diff --git a/backend/__tests__/unit/services/installableEventHandlers.test.js b/backend/__tests__/unit/services/installableEventHandlers.test.js index 2d396057a..e19bb2514 100644 --- a/backend/__tests__/unit/services/installableEventHandlers.test.js +++ b/backend/__tests__/unit/services/installableEventHandlers.test.js @@ -455,24 +455,36 @@ describe('installable event dispatcher', () => { // reddens as well. describe('the outbound selector agrees with connectorRelayPolicy', () => { // The matrix the row names: member / non-member × gate on / off / absent. + // + // TASK-161 adds the CREATOR cell — a subject who is the pod's `createdBy` and + // is not listed in `members`, i.e. a creator who has left. It is the cell + // where the two sides used to agree on "yes": the selector unioned + // `pod.createdBy` into memberIds exactly as the permissive `isPodMember` + // did, so green meant the wrong answer rather than an untested one. Both + // sides now exclude it, and this asserts the exclusion rather than the + // agreement alone. const CELLS = [ - { label: 'member, gate on', member: true, gate: true }, - { label: 'member, gate off', member: true, gate: false }, - { label: 'member, gate absent', member: true, gate: null }, - { label: 'non-member, gate on', member: false, gate: true }, - { label: 'non-member, gate off', member: false, gate: false }, - { label: 'non-member, gate absent', member: false, gate: null }, + { label: 'member, gate on', membership: 'listed', gate: true }, + { label: 'member, gate off', membership: 'listed', gate: false }, + { label: 'member, gate absent', membership: 'listed', gate: null }, + { label: 'non-member, gate on', membership: 'absent', gate: true }, + { label: 'non-member, gate off', membership: 'absent', gate: false }, + { label: 'non-member, gate absent', membership: 'absent', gate: null }, + { label: 'creator not listed, gate on', membership: 'creator', gate: true }, + { label: 'creator not listed, gate off', membership: 'creator', gate: false }, + { label: 'creator not listed, gate absent', membership: 'creator', gate: null }, ]; it.each(CELLS)('$label: the selector and the predicate reach the same verdict', async ({ - member, gate, + membership, gate, }) => { const podId = freshId(); - const creatorId = freshId(); const ownerId = freshId(); // Installed while the owner IS a member, then removed for the non-member - // cells — the same shape the membership arm above uses, and the only order - // that leaves the row itself unchanged between cells. + // and creator cells — the only order that leaves the row itself unchanged + // between cells. `creatorId === ownerId` is the whole difference between + // the creator cell and the plain non-member one. + const creatorId = membership === 'creator' ? ownerId : freshId(); await createPod(podId, creatorId, [ownerId]); const installed = await install({ installableId: 'telegram', installedBy: ownerId, podId }); // `install` seeds the installed pod's gate ON, so the absent cell has to @@ -489,7 +501,9 @@ describe('installable event dispatcher', () => { { $set: { [`config.gates.${podId}.enabled`]: gate } }, ); } - if (!member) await Pod.updateOne({ _id: podId }, { $pull: { members: ownerId } }); + if (membership !== 'listed') { + await Pod.updateOne({ _id: podId }, { $pull: { members: ownerId } }); + } const row = await Integration.findById(installed.integration._id).lean(); const pod = await Pod.findById(podId).lean(); @@ -507,7 +521,7 @@ describe('installable event dispatcher', () => { // Both halves, then their conjunction, then the verdict all three must // reach — so agreement-by-both-saying-no cannot pass as agreement. - const expected = member && gate === true; + const expected = membership === 'listed' && gate === true; expect(isGatedPodTarget(row, podId)).toBe(gate === true); expect(selectorSays).toBe(expected); expect(predicateSays).toBe(expected); diff --git a/backend/__tests__/unit/services/installableInstallationService.test.js b/backend/__tests__/unit/services/installableInstallationService.test.js index 740223bbf..806831db2 100644 --- a/backend/__tests__/unit/services/installableInstallationService.test.js +++ b/backend/__tests__/unit/services/installableInstallationService.test.js @@ -690,7 +690,11 @@ describe('installable connector projection', () => { it('reconciles pause projections and prunes gates when the owner leaves a pod', async () => { const { userId, podId } = ids(); - await Pod.create({ _id: podId, name: 'Current pod', type: 'team', createdBy: userId, members: [] }); + // The owner is LISTED: this arm is about the stale pod's gate, and a pod + // created-but-unlisted is a different cell with its own arm below (TASK-161). + await Pod.create({ + _id: podId, name: 'Current pod', type: 'team', createdBy: 'someone-else', members: [userId], + }); const installed = await install({ installableId: 'telegram', installedBy: userId, podId }); const stalePodId = new mongoose.Types.ObjectId().toString(); const pausedAt = new Date(); @@ -722,6 +726,49 @@ describe('installable connector projection', () => { expect((await Integration.findById(installed.integration._id)).config.adminPause).toBeUndefined(); }); + it('prunes a NON-ACTIVE gate for a pod the owner created and then left', async () => { + // TASK-161, from Vera's 74663. The connecting gate is not the active pod: + // only `sweepOrphanedGates` prunes a secondary gate, so this cell cannot be + // satisfied by the active-pod sweep — which is why the first draft of this + // arm survived the reconciler being reverted. A pod the owner CREATED and + // left still names them in `createdBy`, which the permissive predicate read + // as membership, leaving the ON switch the Connectors page shows for a pod + // whose relay now refuses every message. + // + // Two halves make this site strict, and the mutation that witnesses it has to + // restore BOTH: the predicate, and the `.select('members')` beside it. Put the + // permissive predicate back on its own and it reads `pod.createdBy` off a + // query that no longer selects it, so the clause is inert and this arm stays + // green — measured, which is why the ledger's M6 is a two-edit mutation. + const { userId, podId } = ids(); + await Pod.create({ + _id: podId, name: 'Active pod', type: 'team', createdBy: new mongoose.Types.ObjectId(), members: [userId], + }); + const installed = await install({ installableId: 'telegram', installedBy: userId, podId }); + await Integration.updateOne( + { _id: installed.integration._id }, + { $set: { 'config.chatId': 'chat-1', 'config.chatType': 'private' } }, + ); + const leftPodId = new mongoose.Types.ObjectId().toString(); + await Pod.create({ + _id: leftPodId, name: 'Created then left', type: 'team', createdBy: userId, members: [userId], + }); + await Integration.updateOne( + { _id: installed.integration._id }, + { $set: { [`config.gates.${leftPodId}`]: { enabled: true, since: new Date() } } }, + ); + await Pod.updateOne({ _id: leftPodId }, { $pull: { members: userId } }); + expect(String((await Pod.findById(leftPodId).lean()).createdBy)).toBe(String(userId)); + + const reconciled = await sweep(new Date()); + const projection = await Integration.findById(installed.integration._id).lean(); + + expect(reconciled.prunedGates).toBe(1); + expect(projection.config.gates?.[leftPodId]).toBeUndefined(); + // The active pod's own gate stays: its owner is still listed there. + expect(projection.config.gates?.[podId]).toBeDefined(); + }); + it('clears the active pod with its orphaned gate and tells the linked chat why', async () => { const { userId, podId } = ids(); await Pod.create({ diff --git a/backend/__tests__/unit/services/slackBridgeService.test.js b/backend/__tests__/unit/services/slackBridgeService.test.js index 5c288396c..ea065358c 100644 --- a/backend/__tests__/unit/services/slackBridgeService.test.js +++ b/backend/__tests__/unit/services/slackBridgeService.test.js @@ -365,4 +365,32 @@ describe('Slack installable bridge', () => { 'D1', 'This connector has no active pod. Choose one in Commonly first.', ); }); + + test('refuses inbound authorship when the linked user created the active pod and left', async () => { + // TASK-161: same refusal, different cell. The arm above has no `createdBy`; + // here the linked user IS the pod's creator, which `leavePod` leaves behind + // — so the permissive predicate admitted exactly this message. + Pod.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ type: 'team', createdBy: 'user-1', members: [] }), + }), + }); + + await expect(relaySlackMessageToPod({ + integration: { + ...integration, + scope: 'user', + config: { ...integration.config, linkedUserId: 'user-1', slackUserId: 'U1' }, + }, + event: { text: 'hello', user: 'U1' }, + })).resolves.toEqual({ relayed: false }); + + expect(User.findById).not.toHaveBeenCalled(); + expect(connectorSecrets.get).toHaveBeenCalledWith('secret-ref'); + const api = SlackApi.mock.results[0].value; + expect(api.postMessage).toHaveBeenCalledWith( + 'D1', + 'This connector has no active pod. Choose one in Commonly first.', + ); + }); }); diff --git a/backend/__tests__/unit/services/telegramBridgeService.test.js b/backend/__tests__/unit/services/telegramBridgeService.test.js index eb9d97e37..8af8206b7 100644 --- a/backend/__tests__/unit/services/telegramBridgeService.test.js +++ b/backend/__tests__/unit/services/telegramBridgeService.test.js @@ -345,6 +345,48 @@ describe('telegramBridgeService — multi-pod routing', () => { expect(telegramSend.sendMessage.mock.calls[0][2]).toContain('Launch'); }); + it('refuses a quote-reply into a pod the linked user created and left', async () => { + // TASK-161. The arm above is a pod whose `members` no longer name the user; + // this is the same person as the pod's `createdBy`, which `leavePod` never + // clears. The permissive predicate treated that as membership, so this is + // the cell the row measured: relayed in, and the pod's own write path 401s. + Pod.findById.mockImplementation((id) => ({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue( + String(id) === GATED_POD + ? podDoc({ name: 'Launch', createdBy: 'user-1', members: [] }) + : podDoc(), + ), + }), + })); + const result = await inbound(userScoped(), { reply_to_message: { message_id: 101 } }); + + expect(result).toEqual({ relayed: false }); + expect(PGMessage.create).not.toHaveBeenCalled(); + expect(deliverMessageToAgents).not.toHaveBeenCalled(); + expect(telegramSend.sendMessage.mock.calls[0][2]).toContain('Launch'); + }); + + it('refuses an unquoted inbound message when the ACTIVE pod\'s creator left it', async () => { + // The routed-reply arm above exercises the policy conjunction; this one is + // the bridge's own membership read on the active pod, so a future edit that + // restores the permissive predicate at either site reddens something. + Pod.findById.mockImplementation(() => ({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue(podDoc({ createdBy: 'user-1', members: [] })), + }), + })); + const result = await inbound(userScoped(), {}); + + expect(result).toEqual({ relayed: false }); + expect(PGMessage.create).not.toHaveBeenCalled(); + expect(telegramSend.sendMessage).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + 'This connector has no active pod. Choose one in Commonly first.', + ); + }); + it('routes an entry with no podId as it always has — the active pod', async () => { await inbound(userScoped(), { reply_to_message: { message_id: 103 } }); diff --git a/backend/routes/installables.ts b/backend/routes/installables.ts index 65749772d..66565bdea 100644 --- a/backend/routes/installables.ts +++ b/backend/routes/installables.ts @@ -10,7 +10,7 @@ const Pod = require('../models/Pod'); const Integration = require('../models/Integration'); const InstallableInstallation = require('../models/InstallableInstallation'); // eslint-disable-next-line global-require -const isPodMember = require('../utils/isPodMember'); +const { isListedPodMember } = require('../services/connectorRelayPolicy'); // eslint-disable-next-line global-require const { randomSecret } = require('../utils/secret'); // eslint-disable-next-line global-require @@ -403,7 +403,7 @@ router.post('/slack/confirm', writeIntegrationsRateLimit, auth, async (req: Auth return slackError(res, 409, 'slack_bind_expired', 'Slack authorization expired. Start again.'); } const pod = await Pod.findById(owned.integration.podId); - if (!pod || !isPodMember(pod, userId)) { + if (!pod || !isListedPodMember(pod, userId)) { return slackError(res, 403, 'slack_pod_access_denied', 'You no longer have access to this pod.'); } const confirmed = await Integration.findOneAndUpdate( @@ -568,7 +568,10 @@ router.post('/:installableId/install', writeIntegrationsRateLimit, auth, async ( try { const pod = await Pod.findById(podId); - if (!pod || !isPodMember(pod, userId)) { + // The pod's write path, not the permissive predicate: installing a connector + // seeds that pod's gate ON, so a creator who left must not be able to write + // install state into a pod that would refuse their messages (TASK-161). + if (!pod || !isListedPodMember(pod, userId)) { return res.status(403).json({ error: 'Access denied' }); } const result = await install({ diff --git a/backend/routes/integrations.ts b/backend/routes/integrations.ts index 5fc1b4fe4..25a55b302 100644 --- a/backend/routes/integrations.ts +++ b/backend/routes/integrations.ts @@ -35,7 +35,7 @@ const { hash, randomSecret } = require('../utils/secret'); // eslint-disable-next-line global-require const { mintConnectCode } = require('../services/telegramConnectCode'); // eslint-disable-next-line global-require -const isPodMember = require('../utils/isPodMember'); +const { isListedPodMember } = require('../services/connectorRelayPolicy'); // eslint-disable-next-line global-require const { projectIntegrationForViewer, withoutConnectCode } = require('../models/integrationPublicConfig'); // eslint-disable-next-line global-require @@ -384,11 +384,13 @@ router.post('/', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: Res return res.status(400).json({ message: 'linkedUserId is derived from the authenticated caller and cannot be set' }); } // Membership gate: an integration relays a pod's content outward and - // authors content into it — a WRITE, so it takes the strict predicate - // (members + creator; no admin read-bypass — #1302's isPodMember, not - // DMService.canViewPod). Plain findById: unit mocks resolve a bare doc. + // authors content into it — a WRITE, so it takes the strict predicate the + // pod's own write path runs: `pod.members` alone, no creator bypass and no + // admin read-bypass (TASK-161; DMService.canViewPod's admin clause exists + // for read observability, and would make "only members can write here" + // untrue). Plain findById: unit mocks resolve a bare doc. const targetPod = await Pod.findById(String(podId)); - if (!targetPod || !isPodMember(targetPod, req.user?.id)) { + if (!targetPod || !isListedPodMember(targetPod, req.user?.id)) { return res.status(403).json({ message: 'Access denied' }); } const relay = readRelayFlags(stripServerOwnedConfig(config)); @@ -636,15 +638,15 @@ router.patch('/:id', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: return res.status(400).json({ message: 'adminPause is managed by an administrator' }); } // A user connector has one active inbound destination. Selecting it is - // an owner action, and it must be a pod that owner can still write to; - // otherwise a browser could redirect private inbound messages into a - // pod it merely knows the id of. + // an owner action, and it must be a pod that owner can still write to — + // the same strict membership `createMessage` checks, so a creator who + // left cannot aim private inbound messages at a pod that refuses them. if (podId !== undefined) { if (typeof podId !== 'string' || !isObjectIdKey(podId)) { return res.status(400).json({ message: 'podId must be a valid pod id' }); } const activePod = await Pod.findById(podId); - if (!activePod || !isPodMember(activePod, requesterId)) { + if (!activePod || !isListedPodMember(activePod, requesterId)) { return res.status(403).json({ message: 'Access denied' }); } } @@ -667,7 +669,7 @@ router.patch('/:id', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: // a mixed valid/invalid PATCH atomic: no valid gate is written first. for (const gatePodId of gatePodIds) { const gatePod = await Pod.findById(gatePodId); - if (!gatePod || !isPodMember(gatePod, requesterId)) { + if (!gatePod || !isListedPodMember(gatePod, requesterId)) { return res.status(403).json({ message: 'Access denied' }); } } diff --git a/backend/services/connectorRelayPolicy.ts b/backend/services/connectorRelayPolicy.ts index 96521eb5a..598d3533e 100644 --- a/backend/services/connectorRelayPolicy.ts +++ b/backend/services/connectorRelayPolicy.ts @@ -3,7 +3,13 @@ // about which pod messages are allowed to interrupt a person's attention surface, // or about which pods a connector may address at all. -const isPodMember = require('../utils/isPodMember'); +// The strict membership rule, re-exported here so every connector site reads one +// definition through this module. The permissive `isPodMember` is deliberately +// NOT imported any more: its creator clause is what let a departed creator keep +// relaying (TASK-161). Defined beside `isPodMember` in utils so the platform +// readers that need the same rule (PG chat, reactions) share this one home. +// eslint-disable-next-line @typescript-eslint/no-require-imports, global-require +const { isListedPodMember } = require('../utils/isPodMember'); export interface RelayPolicyIntegration { scope?: string; @@ -69,6 +75,11 @@ export const isGatedPodTarget = ( // failure this exists for: the gate says the connector is still subscribed, the // membership says the person it speaks for is still in the room. // +// The membership half is `isListedPodMember` — `pod.members` only, the check the +// pod's own write path runs — so a connector can never write where its owner +// would be refused. Before TASK-161 this read the permissive `isPodMember`, and a +// pod's creator who had left the pod still relayed in both directions. +// // KNOWN WINDOW, accepted: this is check-then-act. A gate switched off between // this call and the write still lets that one message through. Closing it means // making the write itself carry the condition (a conditional update or a @@ -88,11 +99,16 @@ export const isRoutedPodTarget = (opts: { const { integration, pod, podId, userId, } = opts; - // No separate user-id guard: `isPodMember` fails closed on a falsy id itself + // No separate user-id guard: the predicate fails closed on a falsy id itself // (measured — a guard here changed no arm, so it was removed rather than kept // unwitnessed). return isGatedPodTarget(integration, podId) - && isPodMember(pod, userId); + && isListedPodMember(pod, userId); }; -module.exports = { shouldEscalate, isGatedPodTarget, isRoutedPodTarget }; +module.exports = { + shouldEscalate, + isGatedPodTarget, + isRoutedPodTarget, + isListedPodMember, +}; diff --git a/backend/services/decisionCardReconcileService.ts b/backend/services/decisionCardReconcileService.ts index ad003a1f5..fd62f69c7 100644 --- a/backend/services/decisionCardReconcileService.ts +++ b/backend/services/decisionCardReconcileService.ts @@ -7,9 +7,8 @@ const DecisionRequest = require('../models/DecisionRequest'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const Pod = require('../models/Pod'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const isPodMember = require('../utils/isPodMember'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const { isGatedPodTarget } = require('./connectorRelayPolicy'); +const { isGatedPodTarget, isListedPodMember } = require('./connectorRelayPolicy'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const telegramSend = require('./telegramService'); const deliveryFailures = require('./connectorDeliveryFailureService'); @@ -92,7 +91,7 @@ const canSendClosingLine = ( const mutedUntil = integration.config.relayMutedUntil; if (mutedUntil && new Date(mutedUntil).getTime() > now.getTime()) return false; const linkedUserId = memberIdFor(integration); - if (!linkedUserId || !isPodMember(pod, linkedUserId)) return false; + if (!linkedUserId || !isListedPodMember(pod, linkedUserId)) return false; if (!integration.config.chatId) return false; if (integration.type === 'telegram') { return integration.config.chatType === 'private' && Boolean(process.env.TELEGRAM_BOT_TOKEN); diff --git a/backend/services/decisionCardReply.ts b/backend/services/decisionCardReply.ts index 6f5413b1c..acb30fd17 100644 --- a/backend/services/decisionCardReply.ts +++ b/backend/services/decisionCardReply.ts @@ -9,7 +9,7 @@ const Pod = require('../models/Pod'); // eslint-disable-next-line @typescript-eslint/no-require-imports const User = require('../models/User'); // eslint-disable-next-line @typescript-eslint/no-require-imports -const isPodMember = require('../utils/isPodMember'); +const { isListedPodMember } = require('./connectorRelayPolicy'); // eslint-disable-next-line @typescript-eslint/no-require-imports const verdicts = require('./channelVerdictService'); @@ -81,7 +81,7 @@ export const resolveDecisionCardReply = async (input: { + ` Open it in Commonly: ${link}`; // chooseDecision's already-ruled response precedes its membership guard. // Protect the late-reply writer and standing ruling from ex-members too. - if (!pod || !caller || caller.isBot || !isPodMember(pod, input.linkedUserId)) return answer(denied); + if (!pod || !caller || caller.isBot || !isListedPodMember(pod, input.linkedUserId)) return answer(denied); const close = async (): Promise => { try { diff --git a/backend/services/installable/eventHandlers.ts b/backend/services/installable/eventHandlers.ts index 2c4437a19..1b1991332 100644 --- a/backend/services/installable/eventHandlers.ts +++ b/backend/services/installable/eventHandlers.ts @@ -45,16 +45,20 @@ const activeHandlersForPod = async (podId: string, includeCardHolds = false): Pr // connector is private to its owner, so a stale gate can never relay after // that owner leaves even before the reconciler prunes the key. Legacy rows // retain their pod-scoped selection until they are explicitly migrated. + // + // `pod.members` ALONE: identity with the predicate the bridges apply + // (connectorRelayPolicy.isListedPodMember). This selector used to union + // `pod.createdBy` in, so the two agreed on admitting a departed creator — + // agreement on the wrong answer, which is why that cell is in the suite's + // matrix rather than left to this comment (TASK-161, from TASK-160's ruling). if (!Types.ObjectId.isValid(podId)) return []; - const pod = await Pod.findById(podId).select('createdBy members').lean() as { - createdBy?: unknown; + const pod = await Pod.findById(podId).select('members').lean() as { members?: unknown[]; } | null; if (!pod) return []; - const memberIds = Array.from(new Set([ - pod.createdBy, - ...(pod.members || []), - ].filter(Boolean).map((id) => String(id)))).map((id) => new Types.ObjectId(id)); + const memberIds = Array.from(new Set( + (pod.members || []).filter(Boolean).map((id) => String(id)), + )).map((id) => new Types.ObjectId(id)); if (!memberIds.length) return []; const gateEnabledPath = `config.gates.${String(podId)}.enabled`; // A decision card must record why it was deliberately withheld. Ordinary diff --git a/backend/services/installable/installableReconciler.ts b/backend/services/installable/installableReconciler.ts index c71c10f44..38dc11f60 100644 --- a/backend/services/installable/installableReconciler.ts +++ b/backend/services/installable/installableReconciler.ts @@ -25,7 +25,7 @@ const { allRefPaths, kindSpec, rowReferencesSecret } = require('../connectorSecr // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const connectorDeliveryFailures = require('../connectorDeliveryFailureService'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const isPodMember = require('../../utils/isPodMember'); +const { isListedPodMember } = require('../connectorRelayPolicy'); const ORPHAN_SECRET_GRACE_MS = 10 * 60_000; @@ -142,7 +142,11 @@ const sweepPausedInstallations = async (): Promise => { }; // Membership is authoritative on outbound selection, but prune obsolete keys -// here as well so the owner's gate list does not promise a pod they left. +// here as well so the owner's gate list does not promise a pod they left. That +// promise is exactly what a departed CREATOR breaks: `leavePod` filters `members` +// and keeps `createdBy`, so the permissive predicate kept this sweep blind to +// them, and after TASK-161 made relay strict the ON switch they were shown could +// never deliver. Same predicate as the relay, so the two cannot disagree here. const sweepOrphanedGates = async (): Promise => { const rows = await Integration.find({ scope: 'user', @@ -159,8 +163,8 @@ const sweepOrphanedGates = async (): Promise => { if (!gates || typeof gates !== 'object') continue; const unset: Record = {}; for (const podId of Object.keys(gates)) { - const pod = await Pod.findById(podId).select('createdBy members').lean(); - if (!pod || !isPodMember(pod, row.createdBy)) { + const pod = await Pod.findById(podId).select('members').lean(); + if (!pod || !isListedPodMember(pod, row.createdBy)) { unset[`config.gates.${podId}`] = 1; if (String(row.podId) === podId) unset.podId = 1; } diff --git a/backend/services/slackBridgeService.ts b/backend/services/slackBridgeService.ts index dbe107b09..e1b2a082f 100644 --- a/backend/services/slackBridgeService.ts +++ b/backend/services/slackBridgeService.ts @@ -5,12 +5,13 @@ const Integration = require('../models/Integration'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const Pod = require('../models/Pod'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const isPodMember = require('../utils/isPodMember'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const connectorSecrets = require('./connectorSecrets'); const deliveryFailures = require('./connectorDeliveryFailureService'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const { shouldEscalate, isGatedPodTarget, isRoutedPodTarget } = require('./connectorRelayPolicy'); +const { + shouldEscalate, isGatedPodTarget, isRoutedPodTarget, isListedPodMember, +} = require('./connectorRelayPolicy'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const channelVerdictService = require('./channelVerdictService'); import type { DecisionRelayCard } from './decisionCardRelay'; @@ -462,7 +463,7 @@ export const relaySlackMessageToPod = async (opts: { const socketConfig = require('../config/socket'); const pod = await PodModel.findById(podId).select('type createdBy members').lean(); - if (!pod || !isPodMember(pod, linkedUserId)) { + if (!pod || !isListedPodMember(pod, linkedUserId)) { console.warn('[slack-bridge] inbound dropped — linked user is no longer a pod member'); await replyNoActivePod(integration); return { relayed: false }; diff --git a/backend/services/telegramBridgeService.ts b/backend/services/telegramBridgeService.ts index 7c25f9de4..db728ef3a 100644 --- a/backend/services/telegramBridgeService.ts +++ b/backend/services/telegramBridgeService.ts @@ -21,12 +21,13 @@ // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const IntegrationModel = require('../models/Integration'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const isPodMember = require('../utils/isPodMember'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const telegramSend = require('./telegramService'); const deliveryFailures = require('./connectorDeliveryFailureService'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require -const { shouldEscalate, isGatedPodTarget, isRoutedPodTarget } = require('./connectorRelayPolicy'); +const { + shouldEscalate, isGatedPodTarget, isRoutedPodTarget, isListedPodMember, +} = require('./connectorRelayPolicy'); // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const channelVerdictService = require('./channelVerdictService'); import type { DecisionRelayCard } from './decisionCardRelay'; @@ -553,7 +554,7 @@ export const relayTelegramMessageToPod = async (opts: { const socketConfig = require('../config/socket'); const pod = await Pod.findById(podId).select('type createdBy members').lean(); - if (!pod || !isPodMember(pod, linkedUserId)) { + if (!pod || !isListedPodMember(pod, linkedUserId)) { console.warn('[tg-bridge] inbound dropped — linked user is no longer a pod member'); await replyNoActivePod(integration); return { relayed: false }; diff --git a/backend/utils/isPodMember.ts b/backend/utils/isPodMember.ts index 5e5b66c96..4ba4beab7 100644 --- a/backend/utils/isPodMember.ts +++ b/backend/utils/isPodMember.ts @@ -13,6 +13,26 @@ const isPodMember = (pod: any, userId: unknown): boolean => { )); }; +// The membership rule the pod's own write paths implement, and nothing else: +// `createMessage` (controllers/messageController.ts) and the socket post path +// (server.ts) both check `pod.members` alone. A connector that admits a writer +// those two refuse is more permissive than the pod it writes into, which is what +// TASK-161 measured: `createdBy` keeps the creator's id after `leavePod` filters +// `members`, so a departed creator was refused by the app and relayed anyway. +// +// Named for the mechanism, not for emphasis. The creator clause above answers +// "is this someone `Pod.members` forgot to list" — a false negative at creation +// time — and was never a ruling that leaving leaves membership intact. +const isListedPodMember = (pod: any, userId: unknown): boolean => { + if (!pod || !userId) return false; + const id = String(userId); + return (pod.members || []).some((m: any) => ( + (m?._id?.toString?.() || m?.toString?.() || '') === id + )); +}; + module.exports = isPodMember; +module.exports.isPodMember = isPodMember; +module.exports.isListedPodMember = isListedPodMember; export {};