tags fixes

Author: icecrasher321

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

@@ -11,6 +11,7 @@ import {
   createOrUpdateTagDefinitionsBulk,
   deleteAllTagDefinitions,
   getDocumentTagDefinitions,
+  KnowledgeTagProvenanceConflictError,
 } from '@/lib/knowledge/tags/service'
 import type { BulkTagDefinitionsData } from '@/lib/knowledge/tags/types'
 import { checkDocumentAccess, checkDocumentWriteAccess } from '@/app/api/knowledge/utils'
@@ -198,6 +199,9 @@ export const DELETE = withRouteHandler(
         data: { deleted: deletedCount },
       })
     } catch (error) {
+      if (error instanceof KnowledgeTagProvenanceConflictError) {
+        return NextResponse.json({ error: error.message }, { status: 409 })
+      }
       logger.error(`[${requestId}] Error with tag definitions operation`, error)
       return NextResponse.json({ error: 'Failed to process tag definitions' }, { status: 500 })
     }

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

@@ -5,7 +5,10 @@ import { deleteTagDefinitionContract } from '@/lib/api/contracts/knowledge'
 import { parseRequest } from '@/lib/api/server'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
-import { deleteTagDefinition } from '@/lib/knowledge/tags/service'
+import {
+  deleteTagDefinition,
+  KnowledgeTagProvenanceConflictError,
+} from '@/lib/knowledge/tags/service'
 import { checkKnowledgeBaseWriteAccess } from '@/app/api/knowledge/utils'
 
 export const dynamic = 'force-dynamic'
@@ -45,6 +48,9 @@ export const DELETE = withRouteHandler(
         message: `Tag definition "${deletedTag.displayName}" deleted successfully`,
       })
     } catch (error) {
+      if (error instanceof KnowledgeTagProvenanceConflictError) {
+        return NextResponse.json({ error: error.message }, { status: 409 })
+      }
       logger.error(`[${requestId}] Error deleting tag definition`, error)
       return NextResponse.json({ error: 'Failed to delete tag definition' }, { status: 500 })
     }

apps/sim/app/api/knowledge/secret-provenance.test.ts

@@ -34,7 +34,7 @@ function createHeaderlessRequest(payload: Record<string, unknown>): NextRequest
 }
 
 describe('knowledge write secret provenance', () => {
-  it('does not track durable provenance for a headerless external chunk write', () => {
+  it('classifies a headerless external chunk write as exact-empty', () => {
     const payload = { content: 'manual content' }
 
     const result = resolveKnowledgeWriteSecretProvenance({
@@ -46,13 +46,15 @@ describe('knowledge write secret provenance', () => {
       selectionKeys: ['chunk-content'],
     })
 
-    expect(result).toEqual({ success: true })
+    expect(result).toEqual({
+      success: true,
+      provenances: [{ status: 'exact', entries: [] }],
+    })
   })
 
-  it('does not track durable provenance for a headerless external document write', () => {
+  it('classifies a headerless external document write as exact-empty', () => {
     const payload = {
       filename: 'manual.txt',
-      documentTagsData: JSON.stringify([{ tagName: 'source', tagValue: 'manual' }]),
     }
 
     const result = resolveKnowledgeDocumentWriteSecretProvenance({
@@ -64,7 +66,16 @@ describe('knowledge write secret provenance', () => {
       documents: [payload],
     })
 
-    expect(result).toEqual({ success: true })
+    expect(result).toEqual({
+      success: true,
+      provenances: [
+        {
+          filename: { status: 'exact', entries: [] },
+          content: { status: 'exact', entries: [] },
+          tags: [],
+        },
+      ],
+    })
   })
 
   it('does not track durable provenance for a legacy headerless internal write', () => {

apps/sim/app/api/knowledge/secret-provenance.ts

@@ -4,6 +4,7 @@ import {
   createDurableSecretProvenanceRegistry,
   type DurableSecretProvenance,
   durableSecretProvenanceFromPrivateBundle,
+  EXACT_EMPTY_DURABLE_SECRET_PROVENANCE,
 } from '@/lib/execution/durable-secret-provenance'
 import {
   inspectPrivateSecretProvenanceRequest,
@@ -48,7 +49,12 @@ export function resolveKnowledgeWriteSecretProvenance(options: {
   const { request } = options
   const inspection = inspectPrivateSecretProvenanceRequest(request.headers, options.payload)
   if (inspection.status === 'unsupported') {
-    return { success: true }
+    return options.authType === AuthType.INTERNAL_JWT
+      ? { success: true }
+      : {
+          success: true,
+          provenances: options.selectionKeys.map(() => EXACT_EMPTY_DURABLE_SECRET_PROVENANCE),
+        }
   }
   if (inspection.status !== 'verified' || options.authType !== AuthType.INTERNAL_JWT) {
     return { success: false, response: invalidKnowledgeProvenanceResponse() }

apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts

@@ -7,12 +7,12 @@ const {
   mockDownloadFileFromUrl,
   mockGenerateInternalToken,
   mockGetInternalApiBaseUrl,
-  mockIsModelSafeWorkspaceFileKey,
+  mockParseBuffer,
 } = vi.hoisted(() => ({
   mockDownloadFileFromUrl: vi.fn(),
   mockGenerateInternalToken: vi.fn(),
   mockGetInternalApiBaseUrl: vi.fn(),
-  mockIsModelSafeWorkspaceFileKey: vi.fn(),
+  mockParseBuffer: vi.fn(),
 }))
 
 vi.mock('@/lib/auth/internal', () => ({
@@ -24,10 +24,8 @@ vi.mock('@/lib/core/utils/urls', async (importOriginal) => ({
   getInternalApiBaseUrl: mockGetInternalApiBaseUrl,
 }))
 
-vi.mock('@/lib/uploads/contexts/workspace/workspace-file-secret-provenance', () => ({
-  isModelSafeWorkspaceFileKey: mockIsModelSafeWorkspaceFileKey,
-  MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE:
-    'File cannot be sent to a model because its secret provenance is unavailable',
+vi.mock('@/lib/file-parsers', () => ({
+  parseBuffer: mockParseBuffer,
 }))
 
 vi.mock('@/lib/uploads/utils/file-utils.server', () => ({
@@ -38,6 +36,7 @@ import { env } from '@/lib/core/config/env'
 import { RESOLVED_SECRET_PROVENANCE_FIELD } from '@/lib/execution/private-tool-metadata'
 import { processDocument } from '@/lib/knowledge/documents/document-processor'
 import { runWithKnowledgeModelInputProvenance } from '@/lib/knowledge/model-input-provenance'
+import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
 
 describe('knowledge document model-input provenance', () => {
   beforeEach(() => {
@@ -57,27 +56,38 @@ describe('knowledge document model-input provenance', () => {
     vi.unstubAllGlobals()
   })
 
-  it('rejects an unsafe workspace file before downloading or parsing its bytes', async () => {
-    mockIsModelSafeWorkspaceFileKey.mockResolvedValue(false)
+  it('parses tracked workspace-file bytes locally without treating parsing as model egress', async () => {
+    const registry = new ResolvedSecretTraceRegistry([
+      { name: 'TOKEN', plaintext: 'tracked-secret', encryptedValue: 'encrypted-token' },
+    ])
+    registry.recordResolved('TOKEN', 'tracked-secret')
+    mockDownloadFileFromUrl.mockResolvedValue(
+      Buffer.from('Locally parsed content containing tracked-secret.')
+    )
+    mockParseBuffer.mockResolvedValue({
+      content: 'Locally parsed content containing tracked-secret.',
+      metadata: {},
+    })
+    const fetchMock = vi.fn()
+    vi.stubGlobal('fetch', fetchMock)
 
-    await expect(
+    const processed = await runWithKnowledgeModelInputProvenance(registry, () =>
       processDocument(
-        '/api/files/serve/workspace/workspace-1/opaque.txt?context=workspace',
-        'opaque.txt',
+        '/api/files/serve/workspace/workspace-1/tracked.txt?context=workspace',
+        'tracked.txt',
         'text/plain',
         1024,
         200,
-        100,
+        1,
         'user-1',
         'workspace-1'
       )
-    ).rejects.toThrow('secret provenance is unavailable')
-
-    expect(mockIsModelSafeWorkspaceFileKey).toHaveBeenCalledWith(
-      'workspace/workspace-1/opaque.txt',
-      { workspaceId: 'workspace-1' }
     )
-    expect(mockDownloadFileFromUrl).not.toHaveBeenCalled()
+
+    expect(processed.metadata.processingMethod).toBe('file-parser')
+    expect(processed.chunks.map((chunk) => chunk.text).join('\n')).toContain('tracked-secret')
+    expect(mockDownloadFileFromUrl).toHaveBeenCalledOnce()
+    expect(fetchMock).not.toHaveBeenCalled()
   })
 
   it('rejects secret-bearing opaque document bytes before external OCR', async () => {

apps/sim/lib/knowledge/documents/document-processor.ts

@@ -25,11 +25,7 @@ import {
   getKnowledgeOpaqueModelInputRegistry,
 } from '@/lib/knowledge/model-input-provenance'
 import { StorageService } from '@/lib/uploads'
-import {
-  isModelSafeWorkspaceFileKey,
-  MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE,
-} from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
-import { extractStorageKey, isInternalFileUrl } from '@/lib/uploads/utils/file-utils'
+import { isInternalFileUrl } from '@/lib/uploads/utils/file-utils'
 import { downloadFileFromUrl } from '@/lib/uploads/utils/file-utils.server'
 import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
 import { mistralParserTool } from '@/tools/mistral/parser'
@@ -58,20 +54,6 @@ type OCRPage = {
 
 const MISTRAL_MAX_PAGES = 1000
 
-async function assertDocumentFileModelSafe(
-  fileUrl: string,
-  workspaceId?: string | null
-): Promise<void> {
-  if (!isInternalFileUrl(fileUrl)) return
-
-  const path = new URL(fileUrl, 'http://localhost').pathname
-  const key = extractStorageKey(path)
-  const safe = await isModelSafeWorkspaceFileKey(key, workspaceId ? { workspaceId } : {})
-  if (!safe) {
-    throw new Error(MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE)
-  }
-}
-
 async function getPdfPageCount(buffer: Buffer): Promise<number> {
   try {
     const { getDocumentProxy } = await import('unpdf')
@@ -209,7 +191,6 @@ export async function processDocument(
   logger.info('Processing document', { mimeType })
 
   try {
-    await assertDocumentFileModelSafe(fileUrl, workspaceId)
     const parseResult = await parseDocument(fileUrl, filename, mimeType, userId, workspaceId)
     const { content, processingMethod } = parseResult
     const cloudUrl = 'cloudUrl' in parseResult ? parseResult.cloudUrl : undefined

apps/sim/lib/knowledge/secret-provenance.ts

@@ -166,41 +166,10 @@ export function rebindKnowledgeDocumentSecretProvenance(
   provenance: DurableSecretProvenance,
   previousSource: KnowledgeDocumentSourceValue,
   nextSource: KnowledgeDocumentSourceValue
-): DurableSecretProvenance {
-  return rebindKnowledgeDocumentSecretProvenanceFields(
-    provenance,
-    previousSource,
-    nextSource,
-    new Set()
-  )
-}
-
-/** Rebinds unchanged source fields while dropping provenance for metadata fields being cleared. */
-export function rebindKnowledgeDocumentSecretProvenanceAfterMetadataClear(
-  provenance: DurableSecretProvenance,
-  previousSource: KnowledgeDocumentSourceValue,
-  nextSource: KnowledgeDocumentSourceValue,
-  clearedFields: ReadonlySet<KnowledgeDocumentMetadataField>
-): DurableSecretProvenance {
-  return rebindKnowledgeDocumentSecretProvenanceFields(
-    provenance,
-    previousSource,
-    nextSource,
-    clearedFields
-  )
-}
-
-function rebindKnowledgeDocumentSecretProvenanceFields(
-  provenance: DurableSecretProvenance,
-  previousSource: KnowledgeDocumentSourceValue,
-  nextSource: KnowledgeDocumentSourceValue,
-  excludedFields: ReadonlySet<KnowledgeDocumentMetadataField>
 ): DurableSecretProvenance {
   if (provenance.status === 'unknown') return provenance
   if (provenance.entries.some((entry) => !entry.sourceValueHash)) return { status: 'unknown' }
-  const rebound = KNOWLEDGE_DOCUMENT_METADATA_FIELDS.filter(
-    (field) => !excludedFields.has(field)
-  ).map((field) =>
+  const rebound = KNOWLEDGE_DOCUMENT_METADATA_FIELDS.map((field) =>
     bindKnowledgeDocumentFieldSecretProvenance(
       filterDurableSecretProvenanceBySourceValues(provenance, [
         createKnowledgeDocumentFieldBinding(field, previousSource[field]),

apps/sim/lib/knowledge/tags/secret-provenance-delete.test.ts

@@ -0,0 +1,77 @@
+/**
+ * @vitest-environment node
+ */
+import { document, embedding, knowledgeBase, knowledgeBaseTagDefinitions } from '@sim/db/schema'
+import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import {
+  deleteAllTagDefinitions,
+  deleteTagDefinition,
+  KnowledgeTagProvenanceConflictError,
+} from '@/lib/knowledge/tags/service'
+
+const KNOWLEDGE_BASE_ID = 'knowledge-base-1'
+const TAG_DEFINITION = {
+  id: 'tag-definition-1',
+  knowledgeBaseId: KNOWLEDGE_BASE_ID,
+  tagSlot: 'tag1',
+  displayName: 'Classification',
+}
+
+function queueSingleTagDeletion(conflict: boolean): void {
+  queueTableRows(knowledgeBase, [{ id: KNOWLEDGE_BASE_ID }])
+  queueTableRows(knowledgeBaseTagDefinitions, [TAG_DEFINITION])
+  queueTableRows(document, conflict ? [{ id: 'document-1' }] : [])
+}
+
+describe('knowledge tag deletion provenance', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+  })
+
+  it('uses one bounded transaction and preserves the established bulk cleanup path', async () => {
+    queueSingleTagDeletion(false)
+
+    await expect(
+      deleteTagDefinition(KNOWLEDGE_BASE_ID, TAG_DEFINITION.id, 'request-1')
+    ).resolves.toEqual({ tagSlot: 'tag1', displayName: 'Classification' })
+
+    expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1)
+    expect(dbChainMockFns.execute).toHaveBeenCalledTimes(1)
+    expect(JSON.stringify(dbChainMockFns.execute.mock.calls[0]?.[0])).toContain('statement_timeout')
+
+    const selectedTables = dbChainMockFns.from.mock.calls.map(([table]) => table)
+    expect(selectedTables).toEqual([knowledgeBase, knowledgeBaseTagDefinitions, document])
+    expect(dbChainMockFns.for).toHaveBeenCalledWith('update', { of: document })
+    expect(dbChainMockFns.update.mock.calls.map(([table]) => table)).toEqual([embedding, document])
+    expect(dbChainMockFns.set).toHaveBeenCalledWith({ tag1: null })
+    expect(dbChainMockFns.delete).toHaveBeenCalledWith(knowledgeBaseTagDefinitions)
+  })
+
+  it('rejects uncertain or nonempty tracked provenance before mutating data', async () => {
+    queueSingleTagDeletion(true)
+
+    await expect(
+      deleteTagDefinition(KNOWLEDGE_BASE_ID, TAG_DEFINITION.id, 'request-1')
+    ).rejects.toBeInstanceOf(KnowledgeTagProvenanceConflictError)
+
+    expect(dbChainMockFns.update).not.toHaveBeenCalled()
+    expect(dbChainMockFns.delete).not.toHaveBeenCalled()
+  })
+
+  it('clears every bounded tag slot through the same guarded mutation path', async () => {
+    queueTableRows(knowledgeBase, [{ id: KNOWLEDGE_BASE_ID }])
+    queueTableRows(knowledgeBaseTagDefinitions, [
+      { id: 'tag-definition-1', tagSlot: 'tag1' },
+      { id: 'tag-definition-2', tagSlot: 'number1' },
+    ])
+    queueTableRows(document, [])
+
+    await expect(deleteAllTagDefinitions(KNOWLEDGE_BASE_ID, 'request-1')).resolves.toBe(2)
+
+    expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1)
+    expect(dbChainMockFns.execute).toHaveBeenCalledTimes(1)
+    expect(dbChainMockFns.set).toHaveBeenCalledWith({ tag1: null, number1: null })
+  })
+})