Skip to content

Commit 72bb7e6

Browse files
waleedlatif1claude
waleedlatif1
and
claude
authored
fix(executor): skip Response block formatting for internal JWT callers (#3551)
* fix(executor): skip Response block formatting for internal JWT callers The workflow executor tool received `{error: true}` despite successful child workflow execution when the child had a Response block. This happened because `createHttpResponseFromBlock()` hijacked the response with raw user-defined data, and the executor's `transformResponse` expected the standard `{success, executionId, output, metadata}` wrapper. Fix: skip Response block formatting when `authType === INTERNAL_JWT` since Response blocks are designed for external API consumers, not internal workflow-to-workflow calls. Also extract `AuthType` constants from magic strings across all auth type comparisons in the codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(executor): add route-level tests for Response block auth gating Verify that internal JWT callers receive standard format while external callers (API key, session) get Response block formatting. Tests the server-side condition directly using workflowHasResponseBlock and createHttpResponseFromBlock with AuthType constants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(testing): add AuthType to all hybrid auth test mocks Route code now imports AuthType from @/lib/auth/hybrid, so test mocks must export it too. Added AuthTypeMock to @sim/testing and included it in all 15 test files that mock the hybrid auth module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4cb0f4a commit 72bb7e6

File tree

30 files changed

+284
-36
lines changed

30 files changed

+284
-36
lines changed

‎apps/sim/app/api/a2a/serve/[agentId]/route.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
isTerminalState,
1414
parseWorkflowSSEChunk,
1515
} from '@/lib/a2a/utils'
16-
import { type AuthResult, checkHybridAuth } from '@/lib/auth/hybrid'
16+
import { type AuthResult, AuthType, checkHybridAuth } from '@/lib/auth/hybrid'
1717
import { acquireLock, getRedisClient, releaseLock } from '@/lib/core/config/redis'
1818
import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
1919
import { SSE_HEADERS } from '@/lib/core/utils/sse'
@@ -242,9 +242,9 @@ export async function POST(request: NextRequest, { params }: { params: Promise<R
242242

243243
const { id, method, params: rpcParams } = body
244244
const requestApiKey = request.headers.get('X-API-Key')
245-
const apiKey = authenticatedAuthType === 'api_key' ? requestApiKey : null
245+
const apiKey = authenticatedAuthType === AuthType.API_KEY ? requestApiKey : null
246246
const isPersonalApiKeyCaller =
247-
authenticatedAuthType === 'api_key' && authenticatedApiKeyType === 'personal'
247+
authenticatedAuthType === AuthType.API_KEY && authenticatedApiKeyType === 'personal'
248248
const billedUserId = await getWorkspaceBilledAccountUserId(agent.workspaceId)
249249
if (!billedUserId) {
250250
logger.error('Unable to resolve workspace billed account for A2A execution', {

‎apps/sim/app/api/auth/oauth/credentials/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const { mockCheckSessionOrInternalAuth, mockLogger } = vi.hoisted(() => {
2424
})
2525

2626
vi.mock('@/lib/auth/hybrid', () => ({
27+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
2728
checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth,
2829
}))
2930

‎apps/sim/app/api/auth/oauth/token/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ vi.mock('@/lib/auth/credential-access', () => ({
5151
}))
5252

5353
vi.mock('@/lib/auth/hybrid', () => ({
54+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
5455
checkHybridAuth: vi.fn(),
5556
checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth,
5657
checkInternalAuth: vi.fn(),

‎apps/sim/app/api/auth/oauth/token/route.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import { z } from 'zod'
44
import { authorizeCredentialUse } from '@/lib/auth/credential-access'
5-
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
5+
import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
66
import { generateRequestId } from '@/lib/core/utils/request'
77
import { getCredential, getOAuthToken, refreshTokenIfNeeded } from '@/app/api/auth/oauth/utils'
88

@@ -72,7 +72,7 @@ export async function POST(request: NextRequest) {
7272
})
7373

7474
const auth = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
75-
if (!auth.success || auth.authType !== 'session' || !auth.userId) {
75+
if (!auth.success || auth.authType !== AuthType.SESSION || !auth.userId) {
7676
logger.warn(`[${requestId}] Unauthorized request for credentialAccountUserId path`, {
7777
success: auth.success,
7878
authType: auth.authType,
@@ -202,7 +202,7 @@ export async function GET(request: NextRequest) {
202202
credentialId,
203203
requireWorkflowIdForInternal: false,
204204
})
205-
if (!authz.ok || authz.authType !== 'session' || !authz.credentialOwnerUserId) {
205+
if (!authz.ok || authz.authType !== AuthType.SESSION || !authz.credentialOwnerUserId) {
206206
return NextResponse.json({ error: authz.error || 'Unauthorized' }, { status: 403 })
207207
}
208208

‎apps/sim/app/api/files/delete/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ vi.mock('@/lib/auth', () => ({
9191
}))
9292

9393
vi.mock('@/lib/auth/hybrid', () => ({
94+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
9495
checkHybridAuth: mocks.mockCheckHybridAuth,
9596
checkSessionOrInternalAuth: mocks.mockCheckSessionOrInternalAuth,
9697
checkInternalAuth: mocks.mockCheckInternalAuth,

‎apps/sim/app/api/files/parse/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ vi.mock('@/lib/auth', () => ({
106106
}))
107107

108108
vi.mock('@/lib/auth/hybrid', () => ({
109+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
109110
checkInternalAuth: mockCheckInternalAuth,
110111
checkHybridAuth: mockCheckHybridAuth,
111112
checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth,

‎apps/sim/app/api/files/serve/[...path]/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ vi.mock('fs/promises', () => ({
4949
}))
5050

5151
vi.mock('@/lib/auth/hybrid', () => ({
52+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
5253
checkSessionOrInternalAuth: mockCheckSessionOrInternalAuth,
5354
}))
5455

‎apps/sim/app/api/files/upload/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ vi.mock('@/lib/auth', () => ({
100100
}))
101101

102102
vi.mock('@/lib/auth/hybrid', () => ({
103+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
103104
checkHybridAuth: mocks.mockCheckHybridAuth,
104105
checkSessionOrInternalAuth: mocks.mockCheckSessionOrInternalAuth,
105106
checkInternalAuth: mocks.mockCheckInternalAuth,

‎apps/sim/app/api/function/execute/route.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ vi.mock('@/lib/execution/isolated-vm', () => ({
1818
}))
1919

2020
vi.mock('@/lib/auth/hybrid', () => ({
21+
AuthType: { SESSION: 'session', API_KEY: 'api_key', INTERNAL_JWT: 'internal_jwt' },
2122
checkInternalAuth: mockCheckInternalAuth,
2223
}))
2324

‎apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { randomUUID } from 'crypto'
22
import { createLogger } from '@sim/logger'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import { z } from 'zod'
5-
import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
5+
import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
66
import { SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants'
77
import { createTagDefinition, getTagDefinitions } from '@/lib/knowledge/tags/service'
88
import { checkKnowledgeBaseAccess } from '@/app/api/knowledge/utils'
@@ -25,7 +25,7 @@ export async function GET(req: NextRequest, { params }: { params: Promise<{ id:
2525
}
2626

2727
// For session auth, verify KB access. Internal JWT is trusted.
28-
if (auth.authType === 'session' && auth.userId) {
28+
if (auth.authType === AuthType.SESSION && auth.userId) {
2929
const accessCheck = await checkKnowledgeBaseAccess(knowledgeBaseId, auth.userId)
3030
if (!accessCheck.hasAccess) {
3131
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
@@ -62,7 +62,7 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
6262
}
6363

6464
// For session auth, verify KB access. Internal JWT is trusted.
65-
if (auth.authType === 'session' && auth.userId) {
65+
if (auth.authType === AuthType.SESSION && auth.userId) {
6666
const accessCheck = await checkKnowledgeBaseAccess(knowledgeBaseId, auth.userId)
6767
if (!accessCheck.hasAccess) {
6868
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })

0 commit comments

Comments
 (0)