diff --git a/apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts b/apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts new file mode 100644 index 00000000000..23231c66581 --- /dev/null +++ b/apps/sim/app/api/knowledge/[id]/tag-definitions/route.test.ts @@ -0,0 +1,144 @@ +/** + * Tests for knowledge base tag definitions API route + * + * @vitest-environment node + */ +import { + createMockRequest, + hybridAuthMockFns, + knowledgeApiUtilsMock, + knowledgeApiUtilsMockFns, +} from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockGetTagDefinitions, mockCreateTagDefinition } = vi.hoisted(() => ({ + mockGetTagDefinitions: vi.fn(), + mockCreateTagDefinition: vi.fn(), +})) + +vi.mock('@/lib/knowledge/tags/service', () => ({ + getTagDefinitions: mockGetTagDefinitions, + createTagDefinition: mockCreateTagDefinition, +})) + +vi.mock('@/app/api/knowledge/utils', () => knowledgeApiUtilsMock) + +import { GET, POST } from '@/app/api/knowledge/[id]/tag-definitions/route' + +const KB_ID = 'kb-victim' +const TAG_DEFINITIONS = [ + { id: 'tag-def-1', tagSlot: 'tag1', displayName: 'Client Name', fieldType: 'text' }, +] as const +const CREATE_BODY = { tagSlot: 'tag1', displayName: 'Injected', fieldType: 'text' } as const + +const params = () => ({ params: Promise.resolve({ id: KB_ID }) }) + +const { mockCheckKnowledgeBaseAccess, mockCheckKnowledgeBaseWriteAccess } = knowledgeApiUtilsMockFns + +/** Stubs the auth result the route sees. Omit `userId` for a JWT with no acting user. */ +function authenticateAs(userId?: string, authType = 'internal_jwt') { + hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({ + success: true, + authType, + ...(userId ? { userId } : {}), + }) +} + +const granted = { hasAccess: true, knowledgeBase: { id: KB_ID, userId: 'user-1' } } as const + +describe('Knowledge Base Tag Definitions API Route', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetTagDefinitions.mockResolvedValue(TAG_DEFINITIONS) + mockCreateTagDefinition.mockResolvedValue({ id: 'tag-def-new' }) + }) + + describe('GET /api/knowledge/[id]/tag-definitions', () => { + it('returns tag definitions to a caller with read access', async () => { + authenticateAs('user-1', 'session') + mockCheckKnowledgeBaseAccess.mockResolvedValue(granted) + + const response = await GET(createMockRequest('GET'), params()) + + expect(response.status).toBe(200) + await expect(response.json()).resolves.toEqual({ success: true, data: TAG_DEFINITIONS }) + }) + + it('gates reads on read access, not write access', async () => { + authenticateAs('user-1', 'session') + mockCheckKnowledgeBaseAccess.mockResolvedValue(granted) + + await GET(createMockRequest('GET'), params()) + + expect(mockCheckKnowledgeBaseAccess).toHaveBeenCalledWith(KB_ID, 'user-1') + expect(mockCheckKnowledgeBaseWriteAccess).not.toHaveBeenCalled() + }) + + it('authorizes internal JWT callers instead of trusting them', async () => { + authenticateAs('attacker-1') + mockCheckKnowledgeBaseAccess.mockResolvedValue({ hasAccess: false }) + + const response = await GET(createMockRequest('GET'), params()) + + expect(response.status).toBe(403) + expect(mockCheckKnowledgeBaseAccess).toHaveBeenCalledWith(KB_ID, 'attacker-1') + expect(mockGetTagDefinitions).not.toHaveBeenCalled() + }) + + it('returns 404 for an unknown knowledge base', async () => { + authenticateAs('attacker-1') + mockCheckKnowledgeBaseAccess.mockResolvedValue({ hasAccess: false, notFound: true }) + + const response = await GET(createMockRequest('GET'), params()) + + expect(response.status).toBe(404) + expect(mockGetTagDefinitions).not.toHaveBeenCalled() + }) + + it('rejects a JWT that carries no acting user', async () => { + authenticateAs() + + const response = await GET(createMockRequest('GET'), params()) + + expect(response.status).toBe(401) + expect(mockCheckKnowledgeBaseAccess).not.toHaveBeenCalled() + expect(mockGetTagDefinitions).not.toHaveBeenCalled() + }) + }) + + describe('POST /api/knowledge/[id]/tag-definitions', () => { + it('creates a tag definition for a caller with write access', async () => { + authenticateAs('user-1', 'session') + mockCheckKnowledgeBaseWriteAccess.mockResolvedValue(granted) + + const response = await POST(createMockRequest('POST', CREATE_BODY), params()) + + expect(response.status).toBe(200) + expect(mockCreateTagDefinition).toHaveBeenCalledWith( + expect.objectContaining({ knowledgeBaseId: KB_ID, tagSlot: 'tag1' }), + expect.any(String) + ) + }) + + it('authorizes internal JWT callers instead of trusting them', async () => { + authenticateAs('attacker-1') + mockCheckKnowledgeBaseWriteAccess.mockResolvedValue({ hasAccess: false }) + + const response = await POST(createMockRequest('POST', CREATE_BODY), params()) + + expect(response.status).toBe(403) + expect(mockCheckKnowledgeBaseWriteAccess).toHaveBeenCalledWith(KB_ID, 'attacker-1') + expect(mockCreateTagDefinition).not.toHaveBeenCalled() + }) + + it('rejects a JWT that carries no acting user', async () => { + authenticateAs() + + const response = await POST(createMockRequest('POST', CREATE_BODY), params()) + + expect(response.status).toBe(401) + expect(mockCheckKnowledgeBaseWriteAccess).not.toHaveBeenCalled() + expect(mockCreateTagDefinition).not.toHaveBeenCalled() + }) + }) +}) diff --git a/apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts b/apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts index 8d8b1cc41be..811077245b3 100644 --- a/apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts +++ b/apps/sim/app/api/knowledge/[id]/tag-definitions/route.ts @@ -3,17 +3,16 @@ import { generateId } from '@sim/utils/id' import { type NextRequest, NextResponse } from 'next/server' import { createTagDefinitionContract } from '@/lib/api/contracts/knowledge' import { parseRequest } from '@/lib/api/server' -import { AuthType, checkSessionOrInternalAuth } from '@/lib/auth/hybrid' +import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { SUPPORTED_FIELD_TYPES } from '@/lib/knowledge/constants' import { createTagDefinition, getTagDefinitions } from '@/lib/knowledge/tags/service' -import { checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils' +import { checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils' export const dynamic = 'force-dynamic' const logger = createLogger('KnowledgeBaseTagDefinitionsAPI') -// GET /api/knowledge/[id]/tag-definitions - Get all tag definitions for a knowledge base export const GET = withRouteHandler( async (req: NextRequest, { params }: { params: Promise<{ id: string }> }) => { const requestId = generateId().slice(0, 8) @@ -23,19 +22,16 @@ export const GET = withRouteHandler( logger.info(`[${requestId}] Getting tag definitions for knowledge base ${knowledgeBaseId}`) const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) - if (!auth.success) { + if (!auth.success || !auth.userId) { return NextResponse.json({ error: auth.error || 'Unauthorized' }, { status: 401 }) } - // For session auth, verify KB access. Internal JWT is trusted. - if (auth.authType === AuthType.SESSION && auth.userId) { - const accessCheck = await checkKnowledgeBaseWriteAccess(knowledgeBaseId, auth.userId) - if (!accessCheck.hasAccess) { - return NextResponse.json( - { error: accessCheck.notFound ? 'Not found' : 'Forbidden' }, - { status: accessCheck.notFound ? 404 : 403 } - ) - } + const accessCheck = await checkKnowledgeBaseAccess(knowledgeBaseId, auth.userId) + if (!accessCheck.hasAccess) { + return NextResponse.json( + { error: accessCheck.notFound ? 'Not found' : 'Forbidden' }, + { status: accessCheck.notFound ? 404 : 403 } + ) } const tagDefinitions = await getTagDefinitions(knowledgeBaseId) @@ -55,7 +51,6 @@ export const GET = withRouteHandler( } ) -// POST /api/knowledge/[id]/tag-definitions - Create a new tag definition export const POST = withRouteHandler( async (req: NextRequest, context: { params: Promise<{ id: string }> }) => { const requestId = generateId().slice(0, 8) @@ -65,19 +60,16 @@ export const POST = withRouteHandler( logger.info(`[${requestId}] Creating tag definition for knowledge base ${knowledgeBaseId}`) const auth = await checkSessionOrInternalAuth(req, { requireWorkflowId: false }) - if (!auth.success) { + if (!auth.success || !auth.userId) { return NextResponse.json({ error: auth.error || 'Unauthorized' }, { status: 401 }) } - // For session auth, verify KB access. Internal JWT is trusted. - if (auth.authType === AuthType.SESSION && auth.userId) { - const accessCheck = await checkKnowledgeBaseWriteAccess(knowledgeBaseId, auth.userId) - if (!accessCheck.hasAccess) { - return NextResponse.json( - { error: accessCheck.notFound ? 'Not found' : 'Forbidden' }, - { status: accessCheck.notFound ? 404 : 403 } - ) - } + const accessCheck = await checkKnowledgeBaseWriteAccess(knowledgeBaseId, auth.userId) + if (!accessCheck.hasAccess) { + return NextResponse.json( + { error: accessCheck.notFound ? 'Not found' : 'Forbidden' }, + { status: accessCheck.notFound ? 404 : 403 } + ) } const parsed = await parseRequest(createTagDefinitionContract, req, context) diff --git a/apps/sim/tools/params.ts b/apps/sim/tools/params.ts index 75dc85c2a5e..ebccb601991 100644 --- a/apps/sim/tools/params.ts +++ b/apps/sim/tools/params.ts @@ -667,7 +667,7 @@ export async function createLLMToolSchema( } const propertySchema = buildParameterSchema(toolConfig.id, paramId, param) - const enrichedSchema = await enrichmentConfig.enrichSchema(dependencyValue) + const enrichedSchema = await enrichmentConfig.enrichSchema(dependencyValue, enrichmentContext) if (enrichedSchema) { safeAssign(propertySchema, enrichedSchema as Record) diff --git a/apps/sim/tools/schema-enrichers.test.ts b/apps/sim/tools/schema-enrichers.test.ts index 655507ab514..f7542177272 100644 --- a/apps/sim/tools/schema-enrichers.test.ts +++ b/apps/sim/tools/schema-enrichers.test.ts @@ -21,7 +21,7 @@ vi.mock('@/executor/utils/http', () => ({ extractAPIErrorMessage: mockExtractAPIErrorMessage, })) -import { enrichTableToolSchema } from '@/tools/schema-enrichers' +import { enrichKBTagsSchema, enrichTableToolSchema } from '@/tools/schema-enrichers' const ORIGINAL_SCHEMA = { type: 'object' as const, @@ -102,3 +102,41 @@ describe('enrichTableToolSchema', () => { ).rejects.toThrow('Workspace ID is required to enrich table tool schema for table-1') }) }) + +describe('enrichKBTagsSchema', () => { + beforeEach(() => { + vi.clearAllMocks() + mockBuildAuthHeaders.mockResolvedValue({ Authorization: 'Bearer internal-token' }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('fetches tag definitions as the acting user so the route can authorize them', async () => { + const mockFetch = vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + success: true, + data: [{ id: 'td-1', tagSlot: 'tag1', displayName: 'Client', fieldType: 'text' }], + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ) + ) + vi.stubGlobal('fetch', mockFetch) + + const result = await enrichKBTagsSchema('kb-1', { userId: 'user-1' }) + + expect(mockBuildAuthHeaders).toHaveBeenCalledWith('user-1') + expect(result?.properties).toEqual({ Client: { type: 'string', description: 'text tag' } }) + }) + + it('skips enrichment without an acting user rather than issuing an unauthorized request', async () => { + const mockFetch = vi.fn() + vi.stubGlobal('fetch', mockFetch) + + await expect(enrichKBTagsSchema('kb-1', {})).resolves.toBeNull() + expect(mockFetch).not.toHaveBeenCalled() + expect(mockBuildAuthHeaders).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/tools/schema-enrichers.ts b/apps/sim/tools/schema-enrichers.ts index a0132aa6da7..27f91fd50f7 100644 --- a/apps/sim/tools/schema-enrichers.ts +++ b/apps/sim/tools/schema-enrichers.ts @@ -114,13 +114,22 @@ function mapFieldTypeToSchemaType(fieldType: string): string { } /** - * Fetches tag definitions from knowledge base + * Fetches tag definitions from a knowledge base as the acting user, whose id the + * route requires to authorize the read. */ -async function fetchTagDefinitions(knowledgeBaseId: string): Promise { +async function fetchTagDefinitions( + knowledgeBaseId: string, + context: WorkflowToolExecutionContext +): Promise { + if (!context.userId) { + logger.warn(`Skipping tag definition enrichment for KB ${knowledgeBaseId}: no acting user`) + return [] + } + try { const { buildAuthHeaders, buildAPIUrl } = await import('@/executor/utils/http') - const headers = await buildAuthHeaders() + const headers = await buildAuthHeaders(context.userId) const url = buildAPIUrl(`/api/knowledge/${knowledgeBaseId}/tag-definitions`) logger.info(`Fetching tag definitions for KB ${knowledgeBaseId} from ${url.toString()}`) @@ -145,13 +154,16 @@ async function fetchTagDefinitions(knowledgeBaseId: string): Promise description?: string required?: string[] } | null> { - const tagDefinitions = await fetchTagDefinitions(knowledgeBaseId) + const tagDefinitions = await fetchTagDefinitions(knowledgeBaseId, context) if (tagDefinitions.length === 0) { return null @@ -181,12 +193,15 @@ export async function enrichKBTagsSchema(knowledgeBaseId: string): Promise<{ * Fetches KB tag definitions and builds a schema for tag filters. * Returns an array schema where each item is a filter with tagName and tagValue. */ -export async function enrichKBTagFiltersSchema(knowledgeBaseId: string): Promise<{ +export async function enrichKBTagFiltersSchema( + knowledgeBaseId: string, + context: WorkflowToolExecutionContext +): Promise<{ type: string items?: Record description?: string } | null> { - const tagDefinitions = await fetchTagDefinitions(knowledgeBaseId) + const tagDefinitions = await fetchTagDefinitions(knowledgeBaseId, context) if (tagDefinitions.length === 0) { return null diff --git a/apps/sim/tools/types.ts b/apps/sim/tools/types.ts index 94016b758ef..c9b15ee356c 100644 --- a/apps/sim/tools/types.ts +++ b/apps/sim/tools/types.ts @@ -309,7 +309,10 @@ interface SchemaEnrichmentConfig { /** The param ID that this enrichment depends on (e.g., 'knowledgeBaseId', 'workflowId') */ dependsOn: string /** Function to fetch and build dynamic schema based on the dependency value */ - enrichSchema: (dependencyValue: string) => Promise<{ + enrichSchema: ( + dependencyValue: string, + context: WorkflowToolExecutionContext + ) => Promise<{ type: string properties?: Record description?: string