fix(authz): the PG chat path reads Mongo membership, not its own mirror (TASK-162)

backend/__tests__/service/pgMessages.test.js

@@ -13,6 +13,8 @@ jest.mock('../../models/User', () => ({
   })),
 }));
 
+jest.mock('../../models/Pod', () => ({ findById: jest.fn() }));
+
 // Mock PG models
 jest.mock('../../models/pg/Pod', () => ({
   findById: jest.fn(),
@@ -35,10 +37,24 @@ jest.mock('../../services/agentMentionService', () => {
   };
 });
 
+const MongoPod = require('../../models/Pod');
 const PGPod = require('../../models/pg/Pod');
 const PGMessage = require('../../models/pg/Message');
 const AgentMentionService = require('../../services/agentMentionService');
 
+// TASK-162: Mongo `members` decides access, so an arm that expects 200 names
+// the caller in the pod Mongo returns. `PGPod.isMember` is still mocked in these
+// arms on purpose — a live PG row that says "member" must not be enough.
+const podListing = (...memberIds) => {
+  MongoPod.findById.mockReturnValue({
+    select: jest.fn().mockReturnValue({
+      lean: jest.fn().mockResolvedValue(memberIds === null || memberIds[0] === null
+        ? null
+        : { members: memberIds }),
+    }),
+  });
+};
+
 let app;
 
 beforeAll(() => {
@@ -53,9 +69,10 @@ afterEach(() => {
 });
 
 describe('PostgreSQL Message Routes', () => {
-  it('retrieves messages when user is member', async () => {
+  it('retrieves messages for a member of the pod, decided by Mongo membership', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
-    PGPod.isMember.mockResolvedValue(true);
+    PGPod.isMember.mockResolvedValue(true); // a live PG row is not the reason
+    podListing('user1');
     PGMessage.findByPodId.mockResolvedValue([{ id: 1, content: 'Hello' }]);
     const token = generateTestToken('user1');
 
@@ -64,13 +81,35 @@ describe('PostgreSQL Message Routes', () => {
       .set('Authorization', `Bearer ${token}`)
       .expect(200);
 
-    expect(PGPod.isMember).toHaveBeenCalledWith('pod1', 'user1');
+    expect(MongoPod.findById).toHaveBeenCalledWith('pod1');
+    expect(PGPod.isMember).not.toHaveBeenCalled();
     expect(res.body[0].content).toBe('Hello');
   });
 
+  // TASK-162's witness at the route tier: a live PG row that says "member" for
+  // a caller Mongo no longer lists. `PGPod.isMember` returns true in this arm,
+  // so an arm that only leaves the pod cannot see the defect — the SURVIVOR row
+  // is what discriminates the read-time check from a mirror-on-leave fix.
+  it('refuses a post whose PG pod_members row survived a leave', async () => {
+    PGPod.findById.mockResolvedValue({ id: 'pod1' });
+    PGPod.isMember.mockResolvedValue(true);
+    podListing(); // Mongo membership is gone
+    const token = generateTestToken('user1');
+
+    const res = await request(app)
+      .post('/api/pg/messages/pod1')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ content: 'still here?' })
+      .expect(401);
+
+    expect(res.body.msg).toMatch(/Not authorized/);
+    expect(PGMessage.create).not.toHaveBeenCalled();
+  });
+
   it('returns 401 if user is not a member', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
     PGPod.isMember.mockResolvedValue(false);
+    podListing();
     const token = generateTestToken('user1');
 
     const res = await request(app)
@@ -84,6 +123,7 @@ describe('PostgreSQL Message Routes', () => {
   it('creates a message successfully', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
     PGPod.isMember.mockResolvedValue(true);
+    podListing('user1');
     PGMessage.create.mockResolvedValue({ id: 1 });
     PGMessage.findById.mockResolvedValue({ id: 1, content: 'Hi there' });
     const token = generateTestToken('user1');
@@ -112,6 +152,7 @@ describe('PostgreSQL Message Routes', () => {
     };
     PGPod.findById.mockResolvedValue({ id: 'pod1', type: 'chat' });
     PGPod.isMember.mockResolvedValue(true);
+    podListing('user1');
     PGMessage.create.mockResolvedValue({ id: message.id });
     PGMessage.findById.mockResolvedValue(message);
     const token = generateTestToken('user1');
@@ -136,6 +177,7 @@ describe('PostgreSQL Message Routes', () => {
     };
     PGPod.findById.mockResolvedValue({ id: 'pod1', type: 'chat' });
     PGPod.isMember.mockResolvedValue(true);
+    podListing('user1');
     PGMessage.create.mockResolvedValue(persistedMessage);
     PGMessage.findById.mockResolvedValue(null);
     const token = generateTestToken('user1');
@@ -168,6 +210,7 @@ describe('PostgreSQL Message Routes', () => {
     };
     PGPod.findById.mockResolvedValue({ id: 'pod1', type: 'agent-room' });
     PGPod.isMember.mockResolvedValue(true);
+    podListing('user1');
     PGMessage.create.mockResolvedValue({ id: message.id });
     PGMessage.findById.mockResolvedValue(message);
     const token = generateTestToken('user1');
@@ -187,6 +230,7 @@ describe('PostgreSQL Message Routes', () => {
   it('rejects message creation for non-members', async () => {
     PGPod.findById.mockResolvedValue({ id: 'pod1' });
     PGPod.isMember.mockResolvedValue(false);
+    podListing();
     const token = generateTestToken('user1');
 
     const res = await request(app)

backend/__tests__/unit/controllers/pgMessageController.test.js

@@ -1,14 +1,29 @@
 const controller = require('../../../controllers/pgMessageController');
 const PGPod = require('../../../models/pg/Pod');
 const PGMessage = require('../../../models/pg/Message');
+const MongoPod = require('../../../models/Pod');
 const AgentMentionService = require('../../../services/agentMentionService');
 const { AgentInstallation } = require('../../../models/AgentRegistry');
 
 jest.mock('../../../models/pg/Pod');
 jest.mock('../../../models/pg/Message');
+jest.mock('../../../models/Pod');
 jest.mock('../../../services/agentMentionService');
 jest.mock('../../../models/AgentRegistry');
 
+// Mongo membership is the decision since TASK-162, so every arm that expects a
+// status other than 404 has to say what the pod's `members` holds. `null` is a
+// pod Mongo does not have (the orphan-row class), not an empty member list.
+const mongoPod = (members) => {
+  MongoPod.findById.mockReturnValue({
+    select: jest.fn().mockReturnValue({
+      lean: jest.fn().mockResolvedValue(members === null ? null : { members }),
+    }),
+  });
+};
+
+const jsonRes = () => ({ status: jest.fn().mockReturnThis(), json: jest.fn() });
+
 describe('pgMessageController', () => {
   afterEach(() => jest.clearAllMocks());
 
@@ -21,21 +36,22 @@ describe('pgMessageController', () => {
 
   it('getMessages returns 404 if pod not found', async () => {
     PGPod.findById.mockResolvedValue(null);
+    mongoPod(null);
     const req = {
       params: { podId: 'p1' },
       query: {},
       userId: 'u1',
       user: { id: 'u1' },
     };
-    const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
+    const res = jsonRes();
     await controller.getMessages(req, res);
     expect(res.status).toHaveBeenCalledWith(404);
   });
 
   it('returns mention delivery feedback for a legacy PG message post', async () => {
     const message = { id: 'm1', content: 'hello @recorder', userId: { username: 'sam' } };
     PGPod.findById.mockResolvedValue({ type: 'chat' });
-    PGPod.isMember.mockResolvedValue(true);
+    mongoPod(['u1']);
     PGMessage.create.mockResolvedValue({ id: 'm1', content: 'hello @recorder' });
     PGMessage.findById.mockResolvedValue(message);
     AgentMentionService.isAutoRoutedDmPod.mockReturnValue(false);
@@ -51,7 +67,7 @@ describe('pgMessageController', () => {
       userId: 'u1',
       user: { id: 'u1', username: 'sam' },
     };
-    const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
+    const res = jsonRes();
 
     await controller.createMessage(req, res);
 
@@ -68,4 +84,119 @@ describe('pgMessageController', () => {
       },
     });
   });
+
+  // TASK-162. The defect was a stale positive: the PG `pod_members` row was
+  // checked first and concluded membership, so a row that outlived the
+  // membership granted write access. A "leave then post is 401" arm cannot see
+  // that — it passes while the row is absent. The arm that discriminates is the
+  // SURVIVOR: the ghost row still present, Mongo membership gone.
+  it('refuses a post from a member whose PG row survived their departure', async () => {
+    PGPod.findById.mockResolvedValue({ type: 'chat' }); // the ghost row is there
+    PGPod.isMember.mockResolvedValue(true); // and would still say yes
+    mongoPod([]); // Mongo is the truth, and this caller is not in it
+    const req = {
+      params: { podId: 'p1' },
+      body: { content: 'still here?' },
+      userId: 'u1',
+      user: { id: 'u1', username: 'sam' },
+    };
+    const res = jsonRes();
+
+    await controller.createMessage(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(401);
+    expect(PGMessage.create).not.toHaveBeenCalled();
+    expect(PGPod.isMember).not.toHaveBeenCalled();
+  });
+
+  it('refuses a read from the same stale row, so the ghost does not leak history', async () => {
+    PGPod.findById.mockResolvedValue({ type: 'chat' });
+    PGPod.isMember.mockResolvedValue(true);
+    mongoPod([]);
+    const req = {
+      params: { podId: 'p1' },
+      query: {},
+      userId: 'u1',
+      user: { id: 'u1' },
+    };
+    const res = jsonRes();
+
+    await controller.getMessages(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(401);
+    expect(PGMessage.findByPodId).not.toHaveBeenCalled();
+  });
+
+  it('refuses a departed CREATOR whose PG row is present', async () => {
+    // 36 of the 77 ghost rows are the pod's own creator (Vera 74648), which is
+    // the population where the creator clause and the stale row reinforce each
+    // other. `createdBy` is not membership: it says who made the pod, not who is
+    // in it.
+    PGPod.findById.mockResolvedValue({ type: 'chat' });
+    PGPod.isMember.mockResolvedValue(true);
+    mongoPod([]);
+    MongoPod.findById.mockReturnValue({
+      select: jest.fn().mockReturnValue({
+        lean: jest.fn().mockResolvedValue({ createdBy: 'u1', members: [] }),
+      }),
+    });
+    const req = {
+      params: { podId: 'p1' },
+      body: { content: 'hello' },
+      userId: 'u1',
+      user: { id: 'u1', username: 'sam' },
+    };
+    const res = jsonRes();
+
+    await controller.createMessage(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(401);
+    expect(PGMessage.create).not.toHaveBeenCalled();
+  });
+
+  it('refuses a post into a pod Mongo no longer has, however old its PG row is', async () => {
+    // The 140-row orphan class: the PG pod row exists, Mongo's does not, so
+    // there is no membership list left to be in.
+    PGPod.findById.mockResolvedValue({ type: 'chat' });
+    mongoPod(null);
+    const req = {
+      params: { podId: 'p1' },
+      body: { content: 'hello' },
+      userId: 'u1',
+      user: { id: 'u1', username: 'sam' },
+    };
+    const res = jsonRes();
+
+    await controller.createMessage(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(401);
+    expect(PGMessage.create).not.toHaveBeenCalled();
+  });
+
+  it('admits a listed member who has no PG row at all — the lazily-synced mirror must not refuse', async () => {
+    // The inverse direction, and the reason Mongo decides rather than the
+    // mirror: community auto-join and other join paths write Mongo only, so an
+    // absent PG row is not evidence against membership.
+    PGPod.findById.mockResolvedValue({ type: 'chat' });
+    PGMessage.create.mockResolvedValue({ id: 'm2', content: 'hi' });
+    PGMessage.findById.mockResolvedValue({ id: 'm2', content: 'hi' });
+    AgentMentionService.isAutoRoutedDmPod.mockReturnValue(false);
+    AgentMentionService.enqueueMentions.mockResolvedValue({ enqueued: [], implicit: [], woken: [] });
+    AgentInstallation.countDocuments.mockResolvedValue(0);
+    mongoPod(['u1']);
+    const req = {
+      params: { podId: 'p1' },
+      body: { content: 'hi' },
+      userId: 'u1',
+      user: { id: 'u1', username: 'sam' },
+    };
+    const res = jsonRes();
+
+    await controller.createMessage(req, res);
+
+    expect(PGMessage.create).toHaveBeenCalledWith('p1', 'u1', 'hi');
+    // The mirror is warmed for the PG listing surfaces, and that write decides
+    // nothing.
+    expect(PGPod.addMember).toHaveBeenCalledWith('p1', 'u1');
+  });
 });