fix(activity): gate both pod-scoped write routes on membership (#1300)

backend/__tests__/unit/routes/activity.write-membership.test.js

@@ -0,0 +1,158 @@
+const request = require('supertest');
+const express = require('express');
+
+// Both write routes on /api/activity landed a row into an arbitrary pod for any
+// authenticated caller. `/create` mattered most because `type` comes straight
+// off the body and `Activity.approval.status` defaults to 'pending', so the row
+// materialises exactly the two fields `getPendingApprovals` filters on — i.e. it
+// arrives in that pod's ADMINS' decision queue with attacker-chosen content.
+// Every case below asserts the write did not happen, not just the status code.
+
+const CALLER = 'caller-1';
+const OTHER = 'someone-else';
+
+const mockAuth = () => jest.doMock('../../../middleware/auth', () => (req, res, next) => {
+  req.userId = CALLER;
+  next();
+});
+
+const podFindById = jest.fn();
+const mockPod = (pod) => {
+  podFindById.mockImplementation(() => ({ select: () => ({ lean: async () => pod }) }));
+  jest.doMock('../../../models/Pod', () => ({ findById: podFindById }));
+};
+
+const buildApp = () => {
+  const app = express();
+  app.use(express.json());
+  app.use('/api/activity', require('../../../routes/activity'));
+  return app;
+};
+
+const setup = (pod) => {
+  mockAuth();
+  podFindById.mockReset();
+  mockPod(pod);
+  const create = jest.fn(async () => ({
+    _id: { toString: () => 'act-1' }, type: 'message', action: 'message', content: 'x', createdAt: new Date(),
+  }));
+  jest.doMock('../../../models/Activity', () => ({ create }));
+  jest.doMock('../../../models/User', () => ({
+    findById: jest.fn(() => ({ select: () => ({ lean: async () => ({ username: 'someone' }) }) })),
+  }));
+  const seedPodActivities = jest.fn(async () => ({ success: true, count: 4 }));
+  jest.doMock('../../../services/activityService', () => ({
+    seedPodActivities, isAgentUsername: jest.fn(() => false),
+  }));
+  return { app: buildApp(), create, seedPodActivities };
+};
+
+const body = (over = {}) => ({
+  type: 'message', action: 'message', content: 'hi', podId: 'pod-1', ...over,
+});
+
+describe('POST /api/activity/create — pod membership', () => {
+  afterEach(() => { jest.resetModules(); jest.clearAllMocks(); });
+
+  it('refuses a non-member and writes nothing', async () => {
+    const { app, create } = setup({ _id: 'pod-1', createdBy: OTHER, members: [OTHER] });
+    await request(app).post('/api/activity/create').send(body()).expect(403);
+    expect(create).not.toHaveBeenCalled();
+  });
+
+  it('allows a member', async () => {
+    const { app, create } = setup({ _id: 'pod-1', createdBy: OTHER, members: [OTHER, CALLER] });
+    await request(app).post('/api/activity/create').send(body()).expect(200);
+    expect(create).toHaveBeenCalledTimes(1);
+  });
+
+  it('allows the creator, who is not always listed in members', async () => {
+    const { app, create } = setup({ _id: 'pod-1', createdBy: CALLER, members: [] });
+    await request(app).post('/api/activity/create').send(body()).expect(200);
+    expect(create).toHaveBeenCalledTimes(1);
+  });
+
+  it('accepts a member listed as a populated subdocument', async () => {
+    const { app, create } = setup({ _id: 'pod-1', createdBy: OTHER, members: [{ _id: CALLER }] });
+    await request(app).post('/api/activity/create').send(body()).expect(200);
+    expect(create).toHaveBeenCalledTimes(1);
+  });
+
+  it('404s an unknown pod rather than writing an orphan row', async () => {
+    const { app, create } = setup(null);
+    await request(app).post('/api/activity/create').send(body()).expect(404);
+    expect(create).not.toHaveBeenCalled();
+  });
+});
+
+describe('POST /api/activity/create — the body\'s podId is untrusted input', () => {
+  afterEach(() => { jest.resetModules(); jest.clearAllMocks(); });
+
+  // `podId` arrives as `unknown`. A raw object reaching findById would be read
+  // as Mongo operators rather than as an id.
+  // Asserted on the ARGUMENT rather than the status: what the mocked findById
+  // returns for a malformed id is a property of the mock, but what the route
+  // hands it is the thing under test.
+  it('coerces the body podId to a string before it reaches a query', async () => {
+    const { app } = setup({ _id: 'pod-1', createdBy: CALLER, members: [CALLER] });
+    await request(app).post('/api/activity/create').send(body({ podId: { $ne: null } }));
+    expect(podFindById).toHaveBeenCalledTimes(1);
+    expect(typeof podFindById.mock.calls[0][0]).toBe('string');
+  });
+
+  it('passes the params podId to the seeder lookup as a string too', async () => {
+    const { app } = setup({ _id: 'pod-1', createdBy: CALLER, members: [CALLER] });
+    await request(app).post('/api/activity/seed/pod-1').send({}).expect(200);
+    expect(typeof podFindById.mock.calls[0][0]).toBe('string');
+  });
+
+  it('stores the resolved pod id, not the body\'s copy of it', async () => {
+    const { app, create } = setup({ _id: 'resolved-pod', createdBy: CALLER, members: [CALLER] });
+    await request(app).post('/api/activity/create').send(body()).expect(200);
+    expect(create.mock.calls[0][0].podId).toBe('resolved-pod');
+  });
+});
+
+describe('POST /api/activity/create — the approval_needed kind', () => {
+  afterEach(() => { jest.resetModules(); jest.clearAllMocks(); });
+
+  // Independent of membership: a member of the pod still cannot mint one here,
+  // because this is the generic client-facing create and the approval kind is
+  // what fills an admin decision queue.
+  it('refuses approval_needed even from a member', async () => {
+    const { app, create } = setup({ _id: 'pod-1', createdBy: CALLER, members: [CALLER] });
+    await request(app).post('/api/activity/create')
+      .send(body({ type: 'approval_needed' })).expect(400);
+    expect(create).not.toHaveBeenCalled();
+  });
+
+  it('positive control — the same member may create an ordinary kind', async () => {
+    const { app, create } = setup({ _id: 'pod-1', createdBy: CALLER, members: [CALLER] });
+    await request(app).post('/api/activity/create').send(body()).expect(200);
+    expect(create).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe('POST /api/activity/seed/:podId — pod membership', () => {
+  afterEach(() => { jest.resetModules(); jest.clearAllMocks(); });
+
+  // The seeder is the DESIGNED producer of approval_needed rows, so an ungated
+  // seed route is the same injection by another door.
+  it('refuses a non-member and never reaches the seeder', async () => {
+    const { app, seedPodActivities } = setup({ _id: 'pod-1', createdBy: OTHER, members: [OTHER] });
+    await request(app).post('/api/activity/seed/pod-1').send({}).expect(403);
+    expect(seedPodActivities).not.toHaveBeenCalled();
+  });
+
+  it('allows a member', async () => {
+    const { app, seedPodActivities } = setup({ _id: 'pod-1', createdBy: OTHER, members: [CALLER] });
+    await request(app).post('/api/activity/seed/pod-1').send({}).expect(200);
+    expect(seedPodActivities).toHaveBeenCalledTimes(1);
+  });
+
+  it('404s an unknown pod', async () => {
+    const { app, seedPodActivities } = setup(null);
+    await request(app).post('/api/activity/seed/pod-1').send({}).expect(404);
+    expect(seedPodActivities).not.toHaveBeenCalled();
+  });
+});

backend/routes/activity.ts

@@ -16,6 +16,10 @@ const User = require('../models/User');
 const ActivityService = require('../services/activityService');
 // eslint-disable-next-line global-require
 const getAuthenticatedUserId = require('../utils/getAuthenticatedUserId');
+// eslint-disable-next-line global-require
+const Pod = require('../models/Pod');
+// eslint-disable-next-line global-require
+const isPodMember = require('../utils/isPodMember');
 
 interface Req {
   query?: Record<string, string>;
@@ -209,6 +213,9 @@ router.post('/seed/:podId', auth, async (req: Req, res: Res) => {
   try {
     const { podId } = req.params || {};
     const userId = getAuthenticatedUserId(req);
+    const pod = await Pod.findById(String(podId)).select('members createdBy').lean();
+    if (!pod) return res.status(404).json({ error: 'Pod not found' });
+    if (!isPodMember(pod, userId)) return res.status(403).json({ error: 'Only pod members can seed activities' });
     const result = await ActivityService.seedPodActivities(podId, userId) as { success?: boolean; error?: string };
     if (!result.success) return res.status(400).json({ error: result.error });
     return res.json(result);
@@ -223,8 +230,23 @@ router.post('/create', auth, async (req: Req, res: Res) => {
     const { type, action, content, podId, target, agentMetadata } = (req.body || {}) as Record<string, unknown>;
     const userId = getAuthenticatedUserId(req);
     if (!type || !action || !podId) return res.status(400).json({ error: 'type, action, and podId are required' });
+    // `approval_needed` is what `getPendingApprovals` selects into a pod's
+    // admins' decision queue. This is the generic client-facing create, so it
+    // must not be able to mint one: the caller supplies no `approval` subdoc
+    // and does not need to, because `Activity.approval.status` defaults to
+    // 'pending' and Mongoose materialises exactly the two fields that filter
+    // selects on.
+    if (type === 'approval_needed') return res.status(400).json({ error: 'approval_needed activities cannot be created through this route' });
+    // `podId` arrives as `unknown` off the body, so it is coerced before it
+    // reaches a query: a raw object would otherwise be interpreted as Mongo
+    // operators rather than an id.
+    const pod = await Pod.findById(String(podId)).select('members createdBy').lean();
+    if (!pod) return res.status(404).json({ error: 'Pod not found' });
+    if (!isPodMember(pod, userId)) return res.status(403).json({ error: 'Only pod members can create activities in a pod' });
     const user = await User.findById(userId).select('username').lean() as { username?: string } | null;
-    const activity = await Activity.create({ type, actor: { id: userId, name: user?.username || 'Unknown', type: ActivityService.isAgentUsername(user?.username) ? 'agent' : 'human', verified: false }, action, content, podId, target, agentMetadata });
+    // Store the id of the pod that was actually resolved and authorised, not
+    // the body's copy of it.
+    const activity = await Activity.create({ type, actor: { id: userId, name: user?.username || 'Unknown', type: ActivityService.isAgentUsername(user?.username) ? 'agent' : 'human', verified: false }, action, content, podId: pod._id, target, agentMetadata });
     return res.json({ success: true, activity: { id: activity._id.toString(), type: activity.type, action: activity.action, content: activity.content, createdAt: activity.createdAt } });
   } catch (error) {
     console.error('Error creating activity:', error);

backend/routes/podInvites.ts

@@ -44,13 +44,8 @@ const inviteWriteRateLimit = rateLimit({
   handler: (_req: any, res: any) => res.status(429).json({ msg: 'rate limit exceeded: 20 invite writes per 60s' }),
 });
 
-const isPodMember = (pod: any, userId: string) => {
-  if (!pod || !userId) return false;
-  if (pod.createdBy?.toString?.() === userId.toString()) return true;
-  return (pod.members || []).some((m: any) => (
-    (m?._id?.toString?.() || m?.toString?.() || '') === userId.toString()
-  ));
-};
+// eslint-disable-next-line global-require
+const isPodMember = require('../utils/isPodMember');
 
 // POST /api/pods/:podId/invites — issue a fresh invite token. Caller must
 // be a member or creator. Body: { expiresInHours?, maxUses? } — both

backend/utils/isPodMember.ts

@@ -0,0 +1,18 @@
+// Membership predicate for pod-scoped WRITES. Deliberately strict: it does
+// not carry the admin bypass `DMService.canViewPod` has, because that bypass
+// exists for read observability and would make "only members can write here"
+// untrue for the one account most able to do damage by accident.
+//
+// The creator counts as a member — `Pod.members` does not always list them.
+const isPodMember = (pod: any, userId: unknown): boolean => {
+  if (!pod || !userId) return false;
+  const id = String(userId);
+  if (pod.createdBy?.toString?.() === id) return true;
+  return (pod.members || []).some((m: any) => (
+    (m?._id?.toString?.() || m?.toString?.() || '') === id
+  ));
+};
+
+module.exports = isPodMember;
+
+export {};