address comments

Author: icecrasher321

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

@@ -2089,6 +2089,50 @@ describe('Function Execute API Route', () => {
       expect((await response.json()).__resolvedSecretNames).toEqual([])
     })
 
+    it('conservatively reports only compiled secrets when bounded output classification is exceeded', async () => {
+      const result = Array.from({ length: 100_001 }, () => 'ordinary')
+      mockExecuteInIsolatedVM.mockResolvedValueOnce({ result, stdout: '' })
+
+      const response = await POST(
+        createMockRequest(
+          'POST',
+          {
+            code: 'const key = {{API_KEY}}; return params.items',
+            params: { items: result },
+            envVars: { API_KEY: 'secret-value', UNUSED: 'x' },
+          },
+          { 'x-sim-request-private-tool-metadata': 'resolved-secret-names-v1' }
+        )
+      )
+      const data = await response.json()
+
+      expect(response.status).toBe(200)
+      expect(response.headers.get('x-sim-private-tool-metadata')).toBe('resolved-secret-names-v1')
+      expect(data.output.result).toHaveLength(100_001)
+      expect(data.output.result[0]).toBe('ordinary')
+      expect(data.__resolvedSecretNames).toEqual(['API_KEY'])
+    })
+
+    it('conservatively reports a compiled secret whose value exceeds matcher capacity', async () => {
+      mockExecuteInIsolatedVM.mockResolvedValueOnce({ result: 'ordinary', stdout: '' })
+
+      const response = await POST(
+        createMockRequest(
+          'POST',
+          {
+            code: 'const key = {{OVERSIZED_SECRET}}; return "ordinary"',
+            envVars: { OVERSIZED_SECRET: 's'.repeat(64 * 1024 + 1), UNUSED: 'x' },
+          },
+          { 'x-sim-request-private-tool-metadata': 'resolved-secret-names-v1' }
+        )
+      )
+      const data = await response.json()
+
+      expect(response.status).toBe(200)
+      expect(data.output.result).toBe('ordinary')
+      expect(data.__resolvedSecretNames).toEqual(['OVERSIZED_SECRET'])
+    })
+
     it('tracks only compiled names when configured secrets share the same value', async () => {
       mockExecuteInIsolatedVM.mockResolvedValueOnce({ result: 'true', stdout: '' })
       const oneResponse = await POST(

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

@@ -113,8 +113,6 @@ const TAG_PATTERN = createReferencePattern()
 const E2B_JS_WRAPPER_LINES = 3
 const E2B_PYTHON_WRAPPER_LINES = 1
 const MAX_SANDBOX_OUTPUT_FILES = 20
-const MAX_PRIVATE_RESOLVED_SECRET_NAMES = 10_000
-const MAX_PRIVATE_RESOLVED_SECRET_NAMES_BYTES = 1024 * 1024
 const MAX_PRIVATE_FILE_SECRET_MATCH_EVENTS = 1_000_000
 const SANDBOX_RUNTIME_PAYLOAD_PATH_ENV = '__SIM_RUNTIME_PAYLOAD_PATH'
 
@@ -984,7 +982,6 @@ interface FunctionRouteExecutionContext {
   resolvedSecretNames: Set<string>
   includePrivateResolvedSecretNames: boolean
   privateResolvedSecretNamesMetadataType?: ResolvedSecretNamesMetadataType
-  outputProvenanceComplete: boolean
   outputSecretMatcher?: ResolvedSecretMatcher
   outputSecretNamesByScanLiteral: Map<string, string[]>
   outputSecretPlaintextsByName: Map<string, string>
@@ -1193,7 +1190,10 @@ function activateOutputSecretProvenance(
   body: unknown,
   context: FunctionRouteExecutionContext
 ): void {
-  if (!context.outputSecretMatcher) return
+  if (!context.outputSecretMatcher) {
+    activateCompiledSecretProvenance(context)
+    return
+  }
 
   const matchedPlaintexts = new Set<string>()
   const projection = projectResolvedSecretContent(
@@ -1205,7 +1205,7 @@ function activateOutputSecretProvenance(
     }
   )
   if (!projection.safe) {
-    context.outputProvenanceComplete = false
+    activateCompiledSecretProvenance(context)
     return
   }
   for (const plaintext of matchedPlaintexts) {
@@ -1215,6 +1215,17 @@ function activateOutputSecretProvenance(
   }
 }
 
+/**
+ * Conservatively activates only secrets whose placeholders were compiled for this invocation.
+ * This fallback is used when the bounded output classifier cannot inspect a result; it never
+ * considers configured-but-unused environment values and never mutates the functional result.
+ */
+function activateCompiledSecretProvenance(context: FunctionRouteExecutionContext): void {
+  for (const name of context.outputSecretPlaintextsByName.keys()) {
+    context.resolvedSecretNames.add(name)
+  }
+}
+
 /**
  * True when this execution compiled a secret placeholder or received a mounted file with verified
  * secret provenance. Ordinary mounts without a provenance envelope are user data, not evidence that
@@ -1267,7 +1278,6 @@ async function getOutputFileSecretProvenance(
       MAX_PRIVATE_FILE_SECRET_MATCH_EVENTS
     )
   } catch {
-    context.outputProvenanceComplete = false
     return { status: 'unknown' }
   }
 
@@ -1292,17 +1302,8 @@ async function getOutputFileSecretProvenance(
   }
 }
 
-function getPrivateResolvedSecretNames(context: FunctionRouteExecutionContext): string[] | null {
-  if (!context.outputProvenanceComplete) return null
-  if (context.resolvedSecretNames.size > MAX_PRIVATE_RESOLVED_SECRET_NAMES) return null
-
-  const names = Array.from(context.resolvedSecretNames).sort()
-  let bytes = 0
-  for (const name of names) {
-    bytes += Buffer.byteLength(name, 'utf8')
-    if (bytes > MAX_PRIVATE_RESOLVED_SECRET_NAMES_BYTES) return null
-  }
-  return names
+function getPrivateResolvedSecretNames(context: FunctionRouteExecutionContext): string[] {
+  return Array.from(context.resolvedSecretNames).sort()
 }
 
 async function appendResolvedSecretNames(
@@ -1326,21 +1327,17 @@ async function appendPrivateResolvedSecretNames(
 ): Promise<NextResponse> {
   if (!names || !metadataType) return response
 
-  try {
-    const body = (await response.clone().json()) as Record<string, unknown>
-    const headers = new Headers(response.headers)
-    headers.delete('content-length')
-    headers.set(PRIVATE_TOOL_METADATA_RESPONSE_HEADER, metadataType)
-    return NextResponse.json(
-      {
-        ...body,
-        [RESOLVED_SECRET_NAMES_FIELD]: names,
-      },
-      { status: response.status, statusText: response.statusText, headers }
-    )
-  } catch {
-    return response
-  }
+  const body = (await response.json()) as Record<string, unknown>
+  const headers = new Headers(response.headers)
+  headers.delete('content-length')
+  headers.set(PRIVATE_TOOL_METADATA_RESPONSE_HEADER, metadataType)
+  return NextResponse.json(
+    {
+      ...body,
+      [RESOLVED_SECRET_NAMES_FIELD]: names,
+    },
+    { status: response.status, statusText: response.statusText, headers }
+  )
 }
 
 /**
@@ -2025,7 +2022,6 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       resolvedSecretNames: new Set<string>(),
       includePrivateResolvedSecretNames,
       privateResolvedSecretNamesMetadataType,
-      outputProvenanceComplete: true,
       outputSecretNamesByScanLiteral: new Map(),
       outputSecretPlaintextsByName: new Map(),
       mountedFileSecretProvenanceScanner,
@@ -2078,7 +2074,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
           }))
         )
       } catch {
-        routeContext.outputProvenanceComplete = false
+        activateCompiledSecretProvenance(routeContext)
       }
     }
     resolvedCode = compilation.code

apps/sim/app/api/mcp/tools/execute/route.test.ts

@@ -9,17 +9,11 @@ const {
   mockDiscoverServerTools,
   mockExecuteTool,
   mockGetExecutionTimeout,
-  mockReadResponseToBufferWithLimit,
 } = vi.hoisted(() => ({
   mockCapExecutionTimeoutMs: vi.fn((_policy: number, requested?: number) => requested ?? 0),
   mockDiscoverServerTools: vi.fn(),
   mockExecuteTool: vi.fn(),
   mockGetExecutionTimeout: vi.fn(() => 0),
-  mockReadResponseToBufferWithLimit: vi.fn(),
-}))
-
-vi.mock('@/lib/core/utils/stream-limits', () => ({
-  readResponseToBufferWithLimit: mockReadResponseToBufferWithLimit,
 }))
 
 vi.mock('@/lib/mcp/middleware', () => ({
@@ -102,9 +96,6 @@ describe('MCP tool execution private secret provenance', () => {
     vi.clearAllMocks()
     mockDiscoverServerTools.mockResolvedValue([{ name: 'example_tool', inputSchema: {} }])
     mockExecuteTool.mockResolvedValue({ content: [{ type: 'text', text: 'ok' }] })
-    mockReadResponseToBufferWithLimit.mockImplementation(async (response: Response) =>
-      Buffer.from(await response.arrayBuffer())
-    )
   })
 
   it('returns provenance activated by this MCP transport call', async () => {
@@ -160,10 +151,10 @@ describe('MCP tool execution private secret provenance', () => {
     expect(mockExecuteTool.mock.calls[0]?.[5]).toBeUndefined()
   })
 
-  it('preserves the functional response when private provenance cannot be attached', async () => {
-    mockReadResponseToBufferWithLimit.mockRejectedValueOnce(new Error('Response exceeds limit'))
+  it('attaches private provenance without imposing a second functional response limit', async () => {
+    const largeText = 'x'.repeat(10 * 1024 * 1024 + 1)
     mockExecuteTool.mockResolvedValueOnce({
-      content: [{ type: 'text', text: 'unchanged' }],
+      content: [{ type: 'text', text: largeText }],
     })
     const request = createRequest({
       'x-sim-request-private-tool-metadata': 'resolved-secret-provenance-v1',
@@ -174,8 +165,15 @@ describe('MCP tool execution private secret provenance', () => {
 
     expect(response.status).toBe(200)
     expect(response.ok).toBe(true)
-    expect(response.headers.has('x-sim-private-tool-metadata')).toBe(false)
-    expect(body).not.toHaveProperty('__resolvedSecretTraceProvenance')
+    expect(response.headers.get('x-sim-private-tool-metadata')).toBe(
+      'resolved-secret-provenance-v1'
+    )
+    expect(body.__resolvedSecretTraceProvenance).toEqual({
+      version: 1,
+      complete: true,
+      entries: [],
+      scope: { userId: 'user-1', workspaceId: 'workspace-1' },
+    })
     expect(body).toMatchObject({
       success: true,
       data: {
@@ -185,7 +183,7 @@ describe('MCP tool execution private secret provenance', () => {
     })
     expect(
       (body.data as { output: { content: Array<{ text?: unknown }> } }).output.content[0]?.text
-    ).toBe('unchanged')
+    ).toBe(largeText)
   })
 
   it('uses the remaining workflow deadline for trusted internal tool calls', async () => {

apps/sim/app/api/mcp/tools/execute/route.ts

@@ -11,7 +11,6 @@ import {
 } from '@/lib/billing/core/billing-attribution'
 import { capExecutionTimeoutMs, getExecutionTimeout } from '@/lib/core/execution-limits'
 import type { SubscriptionPlan } from '@/lib/core/rate-limiter/types'
-import { readResponseToBufferWithLimit } from '@/lib/core/utils/stream-limits'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { SIM_VIA_HEADER } from '@/lib/execution/call-chain'
 import { parseRemainingExecutionDeadlineMs } from '@/lib/execution/execution-deadline-header'
@@ -45,7 +44,6 @@ import {
 } from '@/executor/utils/resolved-secret-trace-registry'
 
 const logger = createLogger('McpToolExecutionAPI')
-const MAX_PRIVATE_MCP_RESPONSE_BYTES = 10 * 1024 * 1024
 
 export const dynamic = 'force-dynamic'
 
@@ -72,21 +70,11 @@ async function attachPrivateProvenance(
   response: NextResponse,
   provenance: ResolvedSecretTraceProvenanceAccumulator
 ): Promise<NextResponse> {
-  let payload: Record<string, unknown>
-  try {
-    const body = await readResponseToBufferWithLimit(response.clone(), {
-      maxBytes: MAX_PRIVATE_MCP_RESPONSE_BYTES,
-      label: 'MCP private metadata response',
-      allowNoBodyFallback: true,
-    })
-    const parsed: unknown = JSON.parse(body.toString('utf8'))
-    if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
-      throw new Error('MCP response is not a JSON object')
-    }
-    payload = parsed as Record<string, unknown>
-  } catch {
-    return response
+  const parsed: unknown = await response.json()
+  if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
+    throw new Error('MCP response is not a JSON object')
   }
+  const payload = parsed as Record<string, unknown>
 
   const headers = new Headers(response.headers)
   headers.delete('content-length')

apps/sim/ee/workspace-forking/lib/copy/cleanup-failed.test.ts

@@ -1,7 +1,13 @@
 /**
  * @vitest-environment node
  */
-import { knowledgeBase, workflow, workflowBlocks, workflowDeploymentVersion } from '@sim/db/schema'
+import {
+  document,
+  knowledgeBase,
+  workflow,
+  workflowBlocks,
+  workflowDeploymentVersion,
+} from '@sim/db/schema'
 import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
 import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
 
@@ -341,6 +347,75 @@ describe('cleanup-failed', () => {
       expect(deletes()[0].table).toBe(knowledgeBase)
     })
 
+    it('keeps a failed copied knowledge base when it contains a non-fork document', async () => {
+      queueTableRows(knowledgeBase, [{ id: 'failed-kb' }])
+
+      const cleaned = await clearFailedForkResourceReferences({
+        childWorkspaceId: 'child-ws',
+        failures: [{ kind: 'knowledge-base', childId: 'failed-kb', documentChildIds: [] }],
+        requestId: 'test',
+      })
+
+      expect(cleaned).toEqual({ cleared: 0, clearingFailed: false })
+      expect(dbChainMockFns.update).not.toHaveBeenCalled()
+      expect(dbChainMockFns.delete).not.toHaveBeenCalled()
+      expect(mockInvalidateDeployedStateCache).not.toHaveBeenCalled()
+    })
+
+    it('clears only failed fork-document references when a user document keeps the KB alive', async () => {
+      queueTableRows(knowledgeBase, [{ id: 'failed-kb' }])
+      queueTableRows(workflow, [{ id: 'wf-1' }])
+      queueTableRows(workflowBlocks, [
+        {
+          ...draftBlockRow('failed-kb'),
+          subBlocks: {
+            ...draftBlockRow('failed-kb').subBlocks,
+            documentId: {
+              id: 'documentId',
+              type: 'document-selector',
+              value: 'fork_document_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
+            },
+          },
+        },
+      ])
+
+      const cleaned = await clearFailedForkResourceReferences({
+        childWorkspaceId: 'child-ws',
+        failures: [
+          {
+            kind: 'knowledge-base',
+            childId: 'failed-kb',
+            documentChildIds: ['fork_document_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'],
+          },
+        ],
+        requestId: 'test',
+      })
+
+      expect(cleaned).toEqual({ cleared: 1, clearingFailed: false })
+      const cleared = updates()[0].values.subBlocks as Record<string, { value: unknown }>
+      expect(cleared.knowledgeBaseId.value).toBe('failed-kb')
+      expect(cleared.documentId.value).toBe('')
+      expect(deletes().map(({ table }) => table)).toEqual([document])
+    })
+
+    it('guards the final KB delete against a non-fork document inserted during cleanup', async () => {
+      queueTableRows(workflow, [])
+
+      await clearFailedForkResourceReferences({
+        childWorkspaceId: 'child-ws',
+        failures: [{ kind: 'knowledge-base', childId: 'failed-kb', documentChildIds: [] }],
+        requestId: 'test',
+      })
+
+      const deletePredicate = dbChainMockFns.where.mock.calls.at(-1)?.[0]
+      expect(deletePredicate).toEqual(
+        expect.objectContaining({
+          type: 'and',
+          conditions: expect.arrayContaining([expect.objectContaining({ type: 'notExists' })]),
+        })
+      )
+    })
+
     it('sweeps a deployed target version even when no draft referenced the failed id', async () => {
       // Draft is clean (other-kb), but a deployed target version still points at the dropped
       // placeholder - the deployed-target scope (not draft divergence) catches it.