From 52a87697f0e45002650d1f53b6ea517048357f01 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 26 Oct 2026 17:39:26 -0500 Subject: [PATCH] Respect all authorized caller tool grants in connection tests Use the full caller access decision rather than requiring a profile, preserve credential identity checks for all auth kinds, and cover allowed grants and explicit denials. Co-Authored-By: Paperclip --- .../connection-management.integration.test.ts | 19 +++++++++++++----- server/src/services/tool-access-policy.ts | 20 +++++-------------- server/src/services/tool-gateway.ts | 4 ---- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/server/src/__tests__/connection-management.integration.test.ts b/server/src/__tests__/connection-management.integration.test.ts index 2d18dbe40f6..977d4fd840d 100644 --- a/server/src/__tests__/connection-management.integration.test.ts +++ b/server/src/__tests__/connection-management.integration.test.ts @@ -231,7 +231,7 @@ describe("connection management with production default agents", () => { expect(bindings.map(binding => binding.targetId)).toEqual([f.agentId]); }); - it.each(["none", "other-agent", "other-agent-no-access", "other-agent-caller-ask", "other-agent-caller-rate", "other-agent-shared-rate", "other-agent-credential", "other-agent-caller-revoked", "private-status", "agent", "user", "cancelled", "project-block", "rate-limit", "agent-rate-approval", "authorized-policy-edit"] as const)("keeps delegated test policy and approval continuation bounded (%s)", async revocation => { + it.each(["none", "other-agent", "other-agent-no-access", "other-agent-allow-policy", "other-agent-explicit-grant", "other-agent-caller-ask", "other-agent-caller-rate", "other-agent-shared-rate", "other-agent-credential", "other-agent-credential-none", "other-agent-caller-revoked", "private-status", "agent", "user", "cancelled", "project-block", "rate-limit", "agent-rate-approval", "authorized-policy-edit"] as const)("keeps delegated test policy and approval continuation bounded (%s)", async revocation => { const f = await fixture(); let targetAgentId = f.agentId; if (revocation.startsWith("other-agent")) { @@ -266,16 +266,25 @@ describe("connection management with production default agents", () => { await server.db.update(toolConnections).set({ healthStatus: "ok" }).where(eq(toolConnections.id, created.body.id)); const [entry] = await server.db.insert(toolCatalogEntries).values({ companyId: f.companyId, applicationId: created.body.applicationId, connectionId: created.body.id, entryKind: "tool", name: "Fixture probe", toolName: "fixture_probe", inputSchema: { type: "object", properties: {} }, riskLevel: "read", isReadOnly: true, status: "active", versionHash: "fixture-v1" }).returning(); - expect(await f.request("POST", `/companies/${f.companyId}/tools/apps/${created.body.id}/finish`, { enabledCatalogEntryIds: [entry.id], access: { agentIds: revocation.startsWith("other-agent") && revocation !== "other-agent-no-access" ? [f.agentId, targetAgentId] : [targetAgentId] } })).toMatchObject({ status: 200 }); + expect(await f.request("POST", `/companies/${f.companyId}/tools/apps/${created.body.id}/finish`, { enabledCatalogEntryIds: [entry.id], access: { agentIds: revocation.startsWith("other-agent") && !["other-agent-no-access", "other-agent-allow-policy", "other-agent-explicit-grant"].includes(revocation) ? [f.agentId, targetAgentId] : [targetAgentId] } })).toMatchObject({ status: 200 }); const path = `/tool-connections/${created.body.id}/test-calls`; const body = { agentId: targetAgentId, runContext: { heartbeatRunId: randomUUID(), issueId: randomUUID(), projectId: randomUUID() }, toolName: "fixture_probe", parameters: {} }; if (revocation === "other-agent-no-access") { - for (const token of [f.key.token, f.jwt]) expect(await f.request("POST", path, body, token)).toMatchObject({ status: 403, body: { reasonCode: "test_caller_connection_unavailable" } }); + await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "Caller cannot use this tool", policyType: "block", selectors: { connectionId: created.body.id, agentId: f.agentId }, priority: 0 }); + for (const token of [f.key.token, f.jwt]) expect(await f.request("POST", path, body, token)).toMatchObject({ status: 200, body: { decision: "off", error: { reasonCode: "deny_policy_block" } } }); expect(calls).toBe(0); return; } - if (revocation === "other-agent-credential") { - await server.db.update(toolConnections).set({ authKind: "api_key", credentialPolicy: "per_agent" }).where(eq(toolConnections.id, created.body.id)); + if (revocation === "other-agent-allow-policy" || revocation === "other-agent-explicit-grant") { + if (revocation === "other-agent-allow-policy") await server.db.insert(toolPolicies).values({ companyId: f.companyId, name: "Caller authorized by policy", policyType: "allow", selectors: { connectionId: created.body.id, agentId: f.agentId }, priority: 0 }); + else await server.db.update(principalPermissionGrants).set({ scope: { connectionId: created.body.id } }).where(and(eq(principalPermissionGrants.principalId, f.agentId), eq(principalPermissionGrants.permissionKey, "tools:use"))); + const result = await f.request("POST", path, body, f.jwt); + expect(result, JSON.stringify(result)).toMatchObject({ status: 200, body: { decision: "allowed", result: { content: "Fixture result" } } }); + expect(calls).toBe(1); + return; + } + if (revocation === "other-agent-credential" || revocation === "other-agent-credential-none") { + await server.db.update(toolConnections).set({ authKind: revocation === "other-agent-credential-none" ? "none" : "api_key", credentialPolicy: "per_agent" }).where(eq(toolConnections.id, created.body.id)); await server.db.insert(connectionGrants).values([f.agentId, targetAgentId].map(subjectAgentId => ({ companyId: f.companyId, connectionId: created.body.id, kind: "agent" as const, subjectAgentId, status: "active" }))); expect(await f.request("POST", path, body, f.jwt)).toMatchObject({ status: 403, body: { reasonCode: "test_credential_identity_mismatch" } }); expect(calls).toBe(0); diff --git a/server/src/services/tool-access-policy.ts b/server/src/services/tool-access-policy.ts index b16c33ecebd..94a5f9ff0cb 100644 --- a/server/src/services/tool-access-policy.ts +++ b/server/src/services/tool-access-policy.ts @@ -1238,19 +1238,6 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr return { profiles: activeProfiles, entries }; } - function profileAllowsTool(profileState: Awaited>, ctx: ToolAccessContext) { - return profileState.profiles.some(profile => { - const entries = profileState.entries.filter(entry => entry.profileId === profile.id && profileEntryMatches(entry, ctx)); - return !entries.some(entry => entry.effect === "exclude") - && (profile.defaultAction === "allow" || entries.some(entry => entry.effect === "include")); - }); - } - - async function hasProfileAccess(input: ToolAccessDecisionInput) { - const loaded = await loadContext(input); - return loaded.ok && profileAllowsTool(await effectiveProfiles(loaded.ctx), loaded.ctx); - } - async function explicitGrant(ctx: ToolAccessContext, cache?: ToolAccessDecisionCache): Promise { const principalType = ctx.actorType === "agent" ? "agent" : ctx.actorType === "user" ? "user" : null; const principalId = ctx.actorType === "agent" ? ctx.agentId : ctx.actorId; @@ -1454,7 +1441,11 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr ? await admittedRateLimitAdmissions(ctx, options.admittedInvocationId) : []; const profileState = await effectiveProfiles(ctx, cache); const effectiveProfileIds = profileState.profiles.map((profile) => profile.id); - const permittedByProfile = profileAllowsTool(profileState, ctx); + const permittedByProfile = profileState.profiles.some((profile) => { + const matchingEntries = profileState.entries.filter((entry) => entry.profileId === profile.id && profileEntryMatches(entry, ctx)); + return !matchingEntries.some((entry) => entry.effect === "exclude") + && (profile.defaultAction === "allow" || matchingEntries.some((entry) => entry.effect === "include")); + }); const policies = await cachedRead(cache, `enabled-policies:${ctx.companyId}`, async () => db.select().from(toolPolicies).where(and(eq(toolPolicies.companyId, ctx.companyId), eq(toolPolicies.enabled, true))).orderBy(asc(toolPolicies.priority), asc(toolPolicies.createdAt))); for (const policy of policies) { @@ -2182,7 +2173,6 @@ export function toolAccessPolicyService(db: Db, options: { toolActionSigningSecr return { decide, - hasProfileAccess, writeAudit, recordInvocation, summarizeAndRedact, diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index adc802e7ce1..edcbd47418a 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -7434,14 +7434,10 @@ export function createToolGatewayService( async function assertDelegatedTestCredentialAccess(session: ToolGatewaySession, tool: ToolGatewayDescriptor) { const caller = delegatedTestCallerSession(session); if (!caller || !tool.connectionId) return; - if (!(await policyService.hasProfileAccess(policyInputForTool({ session: caller, tool })))) { - throw new ToolGatewayHttpError(403, "Connection is not available in the acting agent's profiles", "test_caller_connection_unavailable"); - } const [connection] = await db.select().from(toolConnections).where(and( eq(toolConnections.companyId, session.companyId), eq(toolConnections.id, tool.connectionId), )); if (!connection) throw new ToolGatewayHttpError(404, "Tool connection not found", "connection_not_found"); - if (connection.authKind === "none") return; // Resolve consent metadata before any vault read. Testing another agent // cannot select that agent's dedicated credential or a different identity. const callerGrant = await resolveConnectionGrant(caller, connection, false);