fix(knowledge): apply knowledge-base access checks consistently across auth types

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()
+    })
+  })
+})

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)

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<string, unknown>)

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()
+  })
+})

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<TagDefinition[]> {
+async function fetchTagDefinitions(
+  knowledgeBaseId: string,
+  context: WorkflowToolExecutionContext
+): Promise<TagDefinition[]> {
+  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<TagDefiniti
  * Fetches KB tag definitions and builds a schema for LLM consumption.
  * Returns an object schema where each property is a tag the LLM can set.
  */
-export async function enrichKBTagsSchema(knowledgeBaseId: string): Promise<{
+export async function enrichKBTagsSchema(
+  knowledgeBaseId: string,
+  context: WorkflowToolExecutionContext
+): Promise<{
   type: string
   properties?: Record<string, { type: string; description?: string }>
   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<string, unknown>
   description?: string
 } | null> {
-  const tagDefinitions = await fetchTagDefinitions(knowledgeBaseId)
+  const tagDefinitions = await fetchTagDefinitions(knowledgeBaseId, context)
 
   if (tagDefinitions.length === 0) {
     return null

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<string, { type: string; description?: string }>
     description?: string