From e491a23a77f5b43d33baf60e2e44a1fab91a25e9 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 18 Apr 2026 11:21:19 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20fix:=20enforce=20REMOTE=5FAGENT?= =?UTF-8?q?=20ACL=20on=20handoff=20sub-agents=20for=20API=20routes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the second P1 codex finding on #12740: the OpenAI-compat `/v1/chat/completions` and Open Responses `/v1/responses` routes gate the primary agent on `REMOTE_AGENT` (via `createCheckRemoteAgentAccess`), but `discoverConnectedAgents` was checking handoff sub-agents against the looser in-app `AGENT` resource type. That allowed a remote caller who could reach the orchestrator but had only in-app visibility on a sub-agent to invoke it via the API — bypassing the remote-sharing boundary. Adds an optional `resourceType` param to `discoverConnectedAgents` (defaulting to `AGENT` for the chat UI path) and passes `ResourceType.REMOTE_AGENT` from both API controllers so every discovered sub-agent clears the same sharing boundary enforced at route entry. --- api/server/controllers/agents/openai.js | 4 ++++ api/server/controllers/agents/responses.js | 4 ++++ packages/api/src/agents/discovery.spec.ts | 28 ++++++++++++++++++++++ packages/api/src/agents/discovery.ts | 11 ++++++++- 4 files changed, 46 insertions(+), 1 deletion(-) diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 12e47672f4..91e2b98617 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -278,6 +278,10 @@ const OpenAIChatCompletionController = async (req, res) => { requestFiles: [], conversationId, parentMessageId, + // The route enforces REMOTE_AGENT on the primary; every discovered + // sub-agent must clear the same sharing boundary, not the looser + // in-app AGENT one. + resourceType: ResourceType.REMOTE_AGENT, }, { getAgent: db.getAgent, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 45c3fa2249..f6530aebb0 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -416,6 +416,10 @@ const createResponse = async (req, res) => { requestFiles: [], conversationId, parentMessageId, + // The route enforces REMOTE_AGENT on the primary; every discovered + // sub-agent must clear the same sharing boundary, not the looser + // in-app AGENT one. + resourceType: ResourceType.REMOTE_AGENT, }, { getAgent: db.getAgent, diff --git a/packages/api/src/agents/discovery.spec.ts b/packages/api/src/agents/discovery.spec.ts index 5cc811d551..f178cc72f7 100644 --- a/packages/api/src/agents/discovery.spec.ts +++ b/packages/api/src/agents/discovery.spec.ts @@ -202,6 +202,34 @@ describe('discoverConnectedAgents', () => { expect(result.edges.map((e) => e.to)).toEqual(['B']); }); + it('passes the configured resourceType (e.g. REMOTE_AGENT) to checkPermission', async () => { + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + resourceType: 'REMOTE_AGENT', + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(checkPermission).toHaveBeenCalledWith( + expect.objectContaining({ resourceType: 'REMOTE_AGENT' }), + ); + }); + it('drops edges whose `from` references a skipped agent (bidirectional graph)', async () => { // Orchestrator A with bidirectional handoffs A<->B. B is inaccessible, // so `A -> B` and `B -> A` must both be filtered — otherwise `B -> A` diff --git a/packages/api/src/agents/discovery.ts b/packages/api/src/agents/discovery.ts index 9aef0ee9b6..9ad9d94161 100644 --- a/packages/api/src/agents/discovery.ts +++ b/packages/api/src/agents/discovery.ts @@ -54,6 +54,14 @@ export interface DiscoverConnectedAgentsParams { requestFiles?: InitializeAgentParams['requestFiles']; conversationId?: string | null; parentMessageId?: string | null; + /** + * ResourceType to check each sub-agent's access against. Defaults to + * `AGENT` for the in-app chat flow. Callers whose entry-point gates on + * a different resource type (e.g. the OpenAI-compat controllers gate on + * `REMOTE_AGENT`) must pass the matching resource type so sub-agents + * don't bypass the same sharing boundary enforced at the route. + */ + resourceType?: string; } export interface DiscoverConnectedAgentsDeps { @@ -117,6 +125,7 @@ export async function discoverConnectedAgents( requestFiles, conversationId, parentMessageId, + resourceType = ResourceType.AGENT, } = params; const { @@ -162,7 +171,7 @@ export async function discoverConnectedAgents( const hasAccess = await checkPermission({ userId, role: req.user?.role, - resourceType: ResourceType.AGENT, + resourceType, resourceId: agent._id, requiredPermission: PermissionBits.VIEW, });