diff --git a/backend/__tests__/service/agentsRuntime.crossAgent.test.js b/backend/__tests__/service/agentsRuntime.crossAgent.test.js index 8cd0a156c5..716bb0ceae 100644 --- a/backend/__tests__/service/agentsRuntime.crossAgent.test.js +++ b/backend/__tests__/service/agentsRuntime.crossAgent.test.js @@ -17,6 +17,7 @@ const express = require('express'); const request = require('supertest'); const jwt = require('jsonwebtoken'); +const mongoose = require('mongoose'); const { setupMongoDb, closeMongoDb } = require('../utils/testUtils'); @@ -25,6 +26,7 @@ const Pod = require('../../models/Pod'); const { AgentRegistry, AgentInstallation } = require('../../models/AgentRegistry'); const AgentMemory = require('../../models/AgentMemory'); const AgentAsk = require('../../models/AgentAsk'); +const AgentMessageService = require('../../services/agentMessageService'); // Stub the event service before requiring routes. jest.mock('../../services/agentEventService', () => ({ @@ -219,6 +221,102 @@ describe('Cross-agent HTTP routes (ADR-003 Phase 4)', () => { }); }); + describe('POST /agent-dm authorization', () => { + const openDm = (target, originPodId) => request(app) + .post('/api/agents/runtime/agent-dm') + .set('Authorization', `Bearer ${aliceToken}`) + .send({ target, ...(originPodId ? { originPodId: String(originPodId) } : {}) }); + + const outsider = async () => { + const agentName = `outsider-${new mongoose.Types.ObjectId().toString().slice(-8)}`; + const user = await User.create({ + username: agentName, + email: `${agentName}@agents.commonly.local`, + password: 'password123', + isBot: true, + botMetadata: { agentName, instanceId: 'default' }, + }); + return { agentName, user }; + }; + + it('keeps the co-pod agent-name path and posts a notice in its authorized origin', async () => { + const notice = jest.spyOn(AgentMessageService, 'postMessage').mockResolvedValue({}); + try { + const res = await openDm({ agentName: 'bob' }, pod._id); + expect(res.status).toBe(200); + expect(res.body.room.type).toBe('agent-dm'); + expect(notice).toHaveBeenCalledWith(expect.objectContaining({ + agentName: 'commonly-bot', podId: String(pod._id), + })); + } finally { + notice.mockRestore(); + } + }); + + it('keeps an authorized alias as a target name when the agents share a pod', async () => { + pod.contacts.set('peer', { agentName: 'bob', instanceId: 'default' }); + await pod.save(); + const res = await openDm({ alias: 'peer' }, pod._id); + expect(res.status).toBe(200); + expect(res.body.room.type).toBe('agent-dm'); + }); + + it('refuses an alias pinned in another pod before reading its contacts or posting a notice', async () => { + const { agentName, user } = await outsider(); + const foreign = await Pod.create({ + name: 'Foreign pin', type: 'team', createdBy: adminUser._id, + members: [adminUser._id, user._id], + contacts: { reach: { agentName, instanceId: 'default' } }, + }); + const notice = jest.spyOn(AgentMessageService, 'postMessage').mockResolvedValue({}); + const originRead = jest.spyOn(Pod, 'findById'); + try { + const res = await openDm({ alias: 'reach' }, foreign._id); + expect(res.status).toBe(403); + expect(originRead).not.toHaveBeenCalledWith(String(foreign._id)); + expect(await Pod.countDocuments({ type: 'agent-dm', members: { $all: [user._id] } })).toBe(0); + expect(notice).not.toHaveBeenCalled(); + } finally { + originRead.mockRestore(); + notice.mockRestore(); + } + }); + + it('refuses a caller-owned pod pin when the target shares no pod', async () => { + const { agentName, user } = await outsider(); + const alice = await User.findOne({ 'botMetadata.agentName': 'alice', isBot: true }); + const ownPod = await Pod.create({ + name: 'Caller pin', type: 'team', createdBy: alice._id, + members: [alice._id], contacts: { reach: { agentName, instanceId: 'default' } }, + }); + await AgentInstallation.create({ + agentName: 'alice', instanceId: 'default', podId: ownPod._id, + installedBy: adminUser._id, status: 'active', version: '1.0.0', + }); + const res = await openDm({ alias: 'reach' }, ownPod._id); + expect(res.status).toBe(403); + expect(res.body.rule).toBe('sharePod'); + expect(await Pod.countDocuments({ type: 'agent-dm', members: { $all: [user._id] } })).toBe(0); + }); + + it('does not send a notice to a foreign origin even when the target shares a pod', async () => { + const foreign = await Pod.create({ + name: 'Foreign notice', type: 'team', createdBy: adminUser._id, + members: [adminUser._id], + }); + const roomsBefore = await Pod.countDocuments({ type: 'agent-dm' }); + const notice = jest.spyOn(AgentMessageService, 'postMessage').mockResolvedValue({}); + try { + const res = await openDm({ agentName: 'bob' }, foreign._id); + expect(res.status).toBe(403); + expect(await Pod.countDocuments({ type: 'agent-dm' })).toBe(roomsBefore); + expect(notice).not.toHaveBeenCalled(); + } finally { + notice.mockRestore(); + } + }); + }); + // ----------------------------------------------------------------------- // // GET /memory/shared // // ----------------------------------------------------------------------- // diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index 6e4d8887e7..df7fbd5e18 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -1070,6 +1070,18 @@ router.post('/agent-dm', phase4RateLimit, agentRuntimeAuth, async (req: any, res if (!target || typeof target !== 'object') { return res.status(400).json({ message: 'target is required' }); } + const originPodIdSafe = typeof originPodId === 'string' ? originPodId.trim() : ''; + if (originPodId !== undefined && !originPodIdSafe) { + return res.status(400).json({ message: 'originPodId must be a pod id' }); + } + // The runtime-auth middleware has already resolved this token's active + // installations. An origin is a place this agent works, not a pod named + // by the request body; checking it here also protects alias lookup below. + if (originPodIdSafe && !(req.agentInstallations || []).some( + (entry: any) => String(entry.podId).toLowerCase() === originPodIdSafe.toLowerCase(), + )) { + return res.status(403).json({ message: 'Agent is not installed in origin pod' }); + } // Resolve the target User row. Accept three shapes: // { agentName, instanceId? } → look up the bot User row directly. @@ -1142,12 +1154,8 @@ router.post('/agent-dm', phase4RateLimit, agentRuntimeAuth, async (req: any, res // priority but only when an originPodId is given. const alias = String(target.alias).trim().toLowerCase(); let bound: { agentName: string; instanceId: string } | null = null; - // Coerce to string before findById so a malicious request body of - // `{ originPodId: { $gt: '' } }` can't slip through Mongoose's - // implicit object-as-query interpretation. CodeQL flagged this. - const originPodIdStr = originPodId ? String(originPodId) : ''; - if (originPodIdStr) { - const originPod = await Pod.findById(originPodIdStr).select('contacts members').lean(); + if (originPodIdSafe) { + const originPod = await Pod.findById(originPodIdSafe).select('contacts').lean(); const fromPod = originPod?.contacts?.[alias]; if (fromPod && fromPod.agentName) bound = { agentName: fromPod.agentName, instanceId: fromPod.instanceId || 'default' }; } @@ -1180,22 +1188,15 @@ router.post('/agent-dm', phase4RateLimit, agentRuntimeAuth, async (req: any, res return res.status(400).json({ message: 'Cannot DM yourself' }); } - // §3.7 co-pod-member rule. Bypass when an admin has pinned the - // target via originPodId's pod.contacts (admin-binding carve-out). - let authorizedByPodBinding = false; - const originPodIdSafe = originPodId ? String(originPodId) : ''; - if (originPodIdSafe && target.alias) { - const originPod = await Pod.findById(originPodIdSafe).select('contacts').lean(); - authorizedByPodBinding = !!originPod?.contacts?.[String(target.alias).toLowerCase()]; - } - if (!authorizedByPodBinding) { - const shared = await DMService.sharePod(callerAgentUser._id, targetUser._id); - if (!shared) { - return res.status(403).json({ - message: 'No shared pod with target — refused per co-pod-member rule', - rule: 'sharePod', - }); - } + // Contacts choose a target; they do not grant that target access to the + // caller. Every path, including aliases pinned by a pod creator, needs a + // shared pod before it can create a DM or install the peer into it. + const shared = await DMService.sharePod(callerAgentUser._id, targetUser._id); + if (!shared) { + return res.status(403).json({ + message: 'No shared pod with target — refused per co-pod-member rule', + rule: 'sharePod', + }); } // Build member metadata for both sides so AgentInstallation upserts diff --git a/docs/plans/agent-collaboration-surfaces.md b/docs/plans/agent-collaboration-surfaces.md index bf7b6c637e..52ba8a9583 100644 --- a/docs/plans/agent-collaboration-surfaces.md +++ b/docs/plans/agent-collaboration-surfaces.md @@ -344,8 +344,10 @@ tests at the service boundary. autoJoin gate. Single source of truth for the rule. 6. **Endpoint**: `POST /api/agents/runtime/agent-dm` - body: `{ target: { agentName, instanceId } | { userId } | { alias }, originPodId? }` - - Resolves target via §3.2, runs §3.7 check, creates DM, - drops event in `originPodId` if provided. + - Resolves target via §3.2, runs §3.7 for every target shape + (a contact pin is resolution, not DM authority), and creates the DM. + `originPodId` must be an active installation of the caller before + its contacts are read or a DM-started event is posted there. - 403 with structured reason if §3.7 fails. 7. **Sidebar (V2)**: list `agent-dm` rows under their own group; visibility per §3.7 (any user sharing a pod with either