diff --git a/backend/__tests__/unit/controllers/podController.test.js b/backend/__tests__/unit/controllers/podController.test.js index d1b058f0..00ac233e 100644 --- a/backend/__tests__/unit/controllers/podController.test.js +++ b/backend/__tests__/unit/controllers/podController.test.js @@ -259,9 +259,14 @@ describe('podController', () => { expect(res.json).toHaveBeenCalledWith([myPod]); }); - it('getPodsByType returns ALL agent-rooms when caller is a global admin', async () => { + it('getPodsByType filters agent-room to caller membership even for global admins', async () => { + // Regression: admins used to bypass membership and see every private DM + // in the instance in their sidebar. That leaked other users' rooms and + // produced the "Talk to → can't post" UX bug. Admins now see only + // their own personal pods on this listing; moderation goes via a + // dedicated admin tool, not this generic endpoint. const otherPod = { _id: 'p1', type: 'agent-room', members: [{ _id: 'agent-id' }, { _id: 'someone-else' }] }; - const myPod = { _id: 'p2', type: 'agent-room', members: [{ _id: 'agent-id' }, { _id: 'me' }] }; + const myPod = { _id: 'p2', type: 'agent-room', members: [{ _id: 'agent-id' }, { _id: 'admin-id' }] }; const sort = jest.fn().mockResolvedValue([otherPod, myPod]); const populateSecond = jest.fn(() => ({ sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); @@ -273,14 +278,32 @@ describe('podController', () => { const req = { params: { type: 'agent-room' }, userId: 'admin-id', user: {} }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getPodsByType(req, res); - // Admin sees both pods, not just their own. - expect(res.json).toHaveBeenCalledWith([otherPod, myPod]); + expect(res.json).toHaveBeenCalledWith([myPod]); }); - it('getAllPods returns ALL agent-rooms when caller is a global admin (parity with getPodsByType)', async () => { - const otherPod = { _id: 'p1', type: 'agent-room', members: [{ _id: 'agent-id' }, { _id: 'someone-else' }] }; - const myPod = { _id: 'p2', type: 'agent-room', members: [{ _id: 'agent-id' }, { _id: 'me' }] }; - // getAllPods has an extra .populate() chain (parentPod) — match it. + it('getAllPods default scope=mine filters to caller membership even for admins', async () => { + const otherPod = { _id: 'p1', type: 'chat', members: [{ _id: 'someone-else' }] }; + const myPod = { _id: 'p2', type: 'chat', members: [{ _id: 'admin-id' }] }; + const sort = jest.fn().mockResolvedValue([otherPod, myPod]); + const populateThird = jest.fn(() => ({ sort })); + const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); + const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); + Pod.find.mockReturnValue({ populate: populateFirst }); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'admin' }) }), + }); + + const req = { query: {}, userId: 'admin-id', user: {} }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + await podController.getAllPods(req, res); + // Default scope=mine: admin is filtered to their own pods, NOT every + // chat pod in the instance. + expect(res.json).toHaveBeenCalledWith([myPod]); + }); + + it('getAllPods scope=all returns everything for admins (explicit moderation view)', async () => { + const otherPod = { _id: 'p1', type: 'chat', members: [{ _id: 'someone-else' }] }; + const myPod = { _id: 'p2', type: 'chat', members: [{ _id: 'admin-id' }] }; const sort = jest.fn().mockResolvedValue([otherPod, myPod]); const populateThird = jest.fn(() => ({ sort })); const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); @@ -290,9 +313,65 @@ describe('podController', () => { select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'admin' }) }), }); - const req = { query: { type: 'agent-room' }, userId: 'admin-id', user: {} }; + const req = { query: { scope: 'all' }, userId: 'admin-id', user: {} }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getAllPods(req, res); expect(res.json).toHaveBeenCalledWith([otherPod, myPod]); }); + + it('getAllPods scope=all is silently downgraded to scope=mine for non-admins', async () => { + const otherPod = { _id: 'p1', type: 'chat', members: [{ _id: 'someone-else' }] }; + const myPod = { _id: 'p2', type: 'chat', members: [{ _id: 'me' }] }; + const sort = jest.fn().mockResolvedValue([otherPod, myPod]); + const populateThird = jest.fn(() => ({ sort })); + const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); + const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); + Pod.find.mockReturnValue({ populate: populateFirst }); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'user' }) }), + }); + + const req = { query: { scope: 'all' }, userId: 'me', user: {} }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + await podController.getAllPods(req, res); + expect(res.json).toHaveBeenCalledWith([myPod]); + }); + + it('getPodById returns 404 for personal pod types when caller is not a member', async () => { + const pod = { + _id: 'agent-room-1', + type: 'agent-room', + members: [{ _id: 'agent-id' }, { _id: 'someone-else' }], + }; + Pod.findById.mockReturnValue({ + populate: jest.fn().mockReturnValue({ + populate: jest.fn().mockReturnValue({ + populate: jest.fn().mockResolvedValue(pod), + }), + }), + }); + const req = { params: { id: 'agent-room-1' }, userId: 'me', user: {} }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + await podController.getPodById(req, res); + expect(res.status).toHaveBeenCalledWith(404); + }); + + it('getPodById returns the pod for personal pod types when caller IS a member', async () => { + const pod = { + _id: 'agent-room-1', + type: 'agent-room', + members: [{ _id: 'agent-id' }, { _id: 'me' }], + }; + Pod.findById.mockReturnValue({ + populate: jest.fn().mockReturnValue({ + populate: jest.fn().mockReturnValue({ + populate: jest.fn().mockResolvedValue(pod), + }), + }), + }); + const req = { params: { id: 'agent-room-1' }, userId: 'me', user: {} }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + await podController.getPodById(req, res); + expect(res.json).toHaveBeenCalledWith(pod); + }); }); diff --git a/backend/controllers/podController.ts b/backend/controllers/podController.ts index fad673c5..f7c873bf 100644 --- a/backend/controllers/podController.ts +++ b/backend/controllers/podController.ts @@ -172,19 +172,21 @@ exports.getAllPods = async (req: any, res: any) => { // a multi-tenant or shared dev instance and broke isolation for // demos / test accounts. // - // To opt back into the legacy "list everything I have access to read" - // behavior — useful for admin tooling, marketplace browsing, and - // the pod-discovery surface — pass `?scope=all`. That path still - // requires admin or explicit join-policy. + // To opt into the cross-instance "everything I can audit" view — + // useful for admin tooling — pass `?scope=all`. That path is admin- + // only; non-admins are silently downgraded to `scope=mine`. // - // Global admins bypass membership filtering so they can audit every - // pod in the instance — same moderation surface as before for the - // 1:1 invariant on agent-rooms (ADR-001 §3.10). + // Admins do NOT bypass membership on the default sidebar listing: + // their own pod list must be their own pods, otherwise every + // private DM in the instance leaks into their sidebar (which made + // xcjsam see — and try to post into — sam-demo's agent-rooms). const scope = String(req.query?.scope || 'mine').toLowerCase(); const isPersonal = type === 'agent-admin' || type === 'agent-room' || type === 'agent-dm'; - if (req.userId && (isPersonal || scope === 'mine')) { - const isAdmin = await isGlobalAdminRequest(req); - if (!isAdmin) { + if (req.userId) { + const wantsAll = scope === 'all'; + const isAdmin = wantsAll ? await isGlobalAdminRequest(req) : false; + const filterToMine = isPersonal || !wantsAll || !isAdmin; + if (filterToMine) { const uid = String(req.userId); pods = pods.filter((p: any) => p.members.some((m: any) => String(m._id || m) === uid)); } @@ -245,14 +247,13 @@ exports.getPodsByType = async (req: any, res: any) => { .populate('members', 'username profilePicture isBot') .sort({ updatedAt: -1 }); - // Same admin-bypass rule as getAllPods — see comment there. - if ((type === 'agent-admin' || type === 'agent-room') && req.userId) { - const isAdmin = await isGlobalAdminRequest(req); - if (!isAdmin) { - const uid = String(req.userId); - const memberPods = pods.filter((p: any) => p.members.some((m: any) => String(m._id || m) === uid)); - return res.json(memberPods); - } + // Personal pod types (agent-admin, agent-room, agent-dm) are always + // filtered to caller membership — admins included. The instance-wide + // moderation view is a separate admin tool, not this typed listing. + if ((type === 'agent-admin' || type === 'agent-room' || type === 'agent-dm') && req.userId) { + const uid = String(req.userId); + const memberPods = pods.filter((p: any) => p.members.some((m: any) => String(m._id || m) === uid)); + return res.json(memberPods); } return res.json(pods); @@ -288,6 +289,22 @@ exports.getPodById = async (req: any, res: any) => { .json({ error: 'Pod not found or is not of specified type' }); } + // Personal pod types are private 1:1/N:1 surfaces. Only members can + // read them — direct ID lookups by non-members 404 to avoid leaking + // existence and to keep admins from accidentally landing in another + // user's DM (the sidebar bug that surfaced this fix). Admins can + // still moderate via dedicated admin tooling, not this generic GET. + const personalTypes = new Set(['agent-room', 'agent-dm', 'agent-admin']); + if (personalTypes.has(pod.type) && req.userId) { + const uid = String(req.userId); + const isMember = (pod.members || []).some( + (m: any) => String(m?._id || m) === uid, + ); + if (!isMember) { + return res.status(404).json({ error: 'Pod not found' }); + } + } + return res.json(pod); } catch (err: any) { console.error('Error in getPodById:', err.message);