diff --git a/backend/__tests__/unit/routes/registry.list-agents-config.test.js b/backend/__tests__/unit/routes/registry.list-agents-config.test.js index cc548dba91..1c9eb02e51 100644 --- a/backend/__tests__/unit/routes/registry.list-agents-config.test.js +++ b/backend/__tests__/unit/routes/registry.list-agents-config.test.js @@ -30,6 +30,13 @@ jest.mock('../../../models/AgentProfile', () => ({ jest.mock('../../../models/AgentTemplate', () => ({ find: jest.fn(), })); +jest.mock('../../../models/AgentCredential', () => ({ + find: jest.fn().mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }) }), +})); +jest.mock('../../../models/Machine', () => ({ + find: jest.fn().mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }) }), +})); + jest.mock('../../../models/AgentEvent', () => ({ aggregate: jest.fn().mockResolvedValue([]), })); @@ -41,6 +48,12 @@ jest.mock('../../../models/User', () => ({ lean: jest.fn().mockResolvedValue([]), }), }), + // isGlobalAdminUser: the viewer's role, for the webhookUrl reveal. + findById: jest.fn().mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ role: 'user' }), + }), + }), })); // dmService.canViewPod is required at runtime via require(); the handler's @@ -176,6 +189,112 @@ describe('registry list pod agents config payload', () => { expect(res.body.agents[0].runtime).not.toHaveProperty('webhookSecret'); }); + // Cofounder's hardening ruling (2026-10-21): every pod member reads this + // roster, and a push webhook's URL can carry a credential in its query + // string, so only the installer and instance admins see it. + describe('a push webhook URL is shown only to its installer and instance admins', () => { + const SECRET_URL = 'https://agent.example.test/events?secret=s3cr3t'; + const seedWebhookAgent = (installedBy) => { + Pod.findById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ _id: 'pod-1', createdBy: 'someone-else', members: ['user-1', 'user-2'] }), + }); + AgentInstallation.getInstalledAgents.mockResolvedValue([{ + agentName: 'webhook-bot', + instanceId: 'default', + displayName: 'Webhook Bot', + status: 'active', + scopes: [], + usage: {}, + installedBy, + config: new Map(Object.entries({ runtime: { runtimeType: 'webhook', webhookUrl: SECRET_URL } })), + }]); + AgentRegistry.find.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }) }); + AgentProfile.find.mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }); + AgentTemplate.find.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }) }); + }; + const viewerRole = (role) => User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role }) }), + }); + + it('another member sees the runtime but never the URL', async () => { + seedWebhookAgent('user-2'); + viewerRole('user'); + const res = await request(app).get('/api/registry/pods/pod-1/agents'); + expect(res.status).toBe(200); + expect(res.body.agents[0].runtime).toEqual(expect.objectContaining({ runtimeType: 'webhook' })); + expect(res.body.agents[0].runtime).not.toHaveProperty('webhookUrl'); + expect(JSON.stringify(res.body)).not.toContain('s3cr3t'); + }); + + it.each(['map', 'object'])('keeps push connection semantics independent of URL visibility (%s config)', async (shape) => { + User.find.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([{ + _id: 'bot-1', username: 'webhook-bot-default', botMetadata: {}, agentRuntimeTokens: [], + }]) }) }); + viewerRole('user'); + const read = async (installer, url) => { + seedWebhookAgent(installer); + const [installation] = await AgentInstallation.getInstalledAgents(); + const config = { runtime: { runtimeType: 'webhook', ...(url && { webhookUrl: url }) } }; + installation.config = shape === 'map' ? new Map(Object.entries(config)) : config; + const res = await request(app).get('/api/registry/pods/pod-1/agents'); + expect(res.status).toBe(200); + return res.body.agents[0]; + }; + const installer = await read('user-1', SECRET_URL); + const peer = await read('user-2', SECRET_URL); + expect(installer.runtime.webhookUrl).toBe(SECRET_URL); + expect(peer.runtime).not.toHaveProperty('webhookUrl'); + expect(installer).not.toHaveProperty('connection'); + expect(peer).not.toHaveProperty('connection'); + // A poll-based webhook still derives connection health: prove the + // credential lookup was available, rather than taking its catch path. + const polling = await read('user-2', null); + expect(polling).toHaveProperty('connection.state'); + User.find.mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue([]) }) }); + }); + + it('a caller that does not opt in gets no URL (the builder fails closed)', () => { + // eslint-disable-next-line global-require + const { buildAgentInstallationPayload } = require('../../../routes/registry/helpers'); + const payload = buildAgentInstallationPayload({ + agentName: 'webhook-bot', instanceId: 'default', status: 'active', scopes: [], usage: {}, + config: new Map(Object.entries({ runtime: { runtimeType: 'webhook', webhookUrl: SECRET_URL } })), + }); + expect(payload.runtime).not.toHaveProperty('webhookUrl'); + expect(payload.runtime.runtimeType).toBe('webhook'); + }); + + it('the installer sees the URL', async () => { + seedWebhookAgent('user-1'); + const res = await request(app).get('/api/registry/pods/pod-1/agents'); + expect(res.body.agents[0].runtime.webhookUrl).toBe(SECRET_URL); + expect(User.findById).not.toHaveBeenCalled(); + }); + + it('an instance admin sees the URL', async () => { + seedWebhookAgent('user-2'); + viewerRole('admin'); + const res = await request(app).get('/api/registry/pods/pod-1/agents'); + expect(res.body.agents[0].runtime.webhookUrl).toBe(SECRET_URL); + }); + + it('the single-agent route withholds it the same way', async () => { + seedWebhookAgent('user-2'); + viewerRole('user'); + const [installation] = await AgentInstallation.getInstalledAgents(); + AgentInstallation.findOne = jest.fn().mockResolvedValue(installation); + AgentRegistry.findOne = jest.fn().mockReturnValue({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue(null) }) }); + AgentProfile.findOne = jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue(null) }); + const res = await request(app).get('/api/registry/pods/pod-1/agents/webhook-bot?instanceId=default'); + if (res.status === 200) { + expect(res.body.agent.runtime).not.toHaveProperty('webhookUrl'); + expect(JSON.stringify(res.body)).not.toContain('s3cr3t'); + } else { + throw new Error(`single-agent route answered ${res.status}: ${JSON.stringify(res.body).slice(0, 200)}`); + } + }); + }); + it('uses the chat principal label when scoped labels differ', async () => { Pod.findById.mockReturnValue({ lean: jest.fn().mockResolvedValue({ diff --git a/backend/routes/registry/helpers.ts b/backend/routes/registry/helpers.ts index e2cafa6b03..9baeaf5355 100644 --- a/backend/routes/registry/helpers.ts +++ b/backend/routes/registry/helpers.ts @@ -391,16 +391,28 @@ const buildAgentInstallationPayload = (installation: any, { lastActiveAt = null, user = null, lastMessage = null, + revealWebhookUrl = false, }: { profile?: any; iconUrl?: string; lastHeartbeatAt?: any; lastActiveAt?: any; user?: any; lastMessage?: { content?: any; createdAt?: any } | null; + /** + * Only the installer and instance admins see a push webhook's URL. Every + * pod member reads this payload, and a URL can carry a credential in its + * query string (`?secret=`), so it is withheld unless the caller opts in. + */ + revealWebhookUrl?: boolean; } = {}) => { if (!installation) return null; const normalizedConfig = normalizeConfigMap(installation.config); - const runtimeConfig = sanitizeRuntimeConfig( + const sanitizedRuntime = sanitizeRuntimeConfig( normalizedConfig?.runtime || installation.config?.runtime || null, installation.agentName, ); + const runtimeConfig = (() => { + if (revealWebhookUrl || !sanitizedRuntime || !('webhookUrl' in sanitizedRuntime)) return sanitizedRuntime; + const { webhookUrl: _withheld, ...rest } = sanitizedRuntime as Record; + return rest; + })(); // Settings may edit these model controls, and the daemon prefers an explicit // ADR-008 environment over the runtime overlay. Expose only these benign // scalar fields; never project environment secrets or tool declarations. diff --git a/backend/routes/registry/pod-agents.ts b/backend/routes/registry/pod-agents.ts index 3cc4612ebb..5eb3112cce 100644 --- a/backend/routes/registry/pod-agents.ts +++ b/backend/routes/registry/pod-agents.ts @@ -18,6 +18,7 @@ const { getUserId, isGlobalAdminUser, normalizeInstanceId, + normalizeConfigMap, normalizeDisplayName, resolveInstallation, buildAgentProfileId, @@ -329,6 +330,17 @@ podAgentsRouter.get('/pods/:podId/agents', auth, async (req: any, res: any) => { console.warn('[pod-agents] caller-spoke lookup failed:', (spokeErr as Error).message); } + // A push webhook's URL can carry a credential in its query string, so only + // its installer and instance admins see it (Cofounder's hardening ruling, + // 2026-10-21). The admin lookup runs only when the roster holds a URL the + // viewer did not install. + const viewerId = String(userId); + const installedByViewer = (i: any) => String(i.installedBy || '') === viewerId; + const needsAdminCheck = (installations as any[]).some((i: any) => ( + !installedByViewer(i) && Boolean(normalizeConfigMap(i.config)?.runtime?.webhookUrl) + )); + const viewerIsAdmin = needsAdminCheck ? await isGlobalAdminUser(userId) : false; + res.json({ callerSpoke, agents: installations.map((i: any) => { @@ -347,6 +359,7 @@ podAgentsRouter.get('/pods/:podId/agents', auth, async (req: any, res: any) => { lastActiveAt, user, lastMessage: (user && lastMessageByUserId.get(String(user._id))) || null, + revealWebhookUrl: viewerIsAdmin || installedByViewer(i), }); const runtimeType = payload?.runtime?.runtimeType; // A provisioned in-process seat needs no external connection. Its @@ -361,7 +374,7 @@ podAgentsRouter.get('/pods/:podId/agents', auth, async (req: any, res: any) => { || runtimeType === 'wrapper' || (runtimeType === 'webhook' && !payload?.runtime?.host); if (!connectionAvailable || !user || !hasRuntimeConnection - || payload?.runtime?.webhookUrl) return payload; + || normalizeConfigMap(i.config)?.runtime?.webhookUrl) return payload; const machineId = user.botMetadata?.machineId; return { ...payload, @@ -444,6 +457,9 @@ podAgentsRouter.get('/pods/:podId/agents/:name', auth, async (req: any, res: any agent: buildAgentInstallationPayload(installation, { profile, iconUrl: templateIcon || registryEntry?.iconUrl || '', + revealWebhookUrl: installedBy === String(userId) + || (Boolean(normalizeConfigMap(installation.config)?.runtime?.webhookUrl) + && await isGlobalAdminUser(userId)), }), }); } catch (error) { diff --git a/docs/architecture/WEBHOOK_RUNTIME.md b/docs/architecture/WEBHOOK_RUNTIME.md index 1a55140b9b..573aa077b9 100644 --- a/docs/architecture/WEBHOOK_RUNTIME.md +++ b/docs/architecture/WEBHOOK_RUNTIME.md @@ -65,6 +65,12 @@ your own machine, use `commonly agent connect`, which polls and forwards locally. A self-hosted instance whose receivers live on a private network can set `OUTBOUND_WEBHOOK_ALLOW_PRIVATE_ADDRESSES=true`. +**Never put a secret in the webhook URL** (no `?secret=`, `?token=` or key in +the path). Authenticate the delivery with `webhookSecret`, which signs each +request (below), and keep the URL itself non-secret. The pod's agent list +shows a webhook URL only to the agent's installer and instance admins, but a +URL still travels through logs and proxies that a header does not. + --- ## Receiving Events