Merge remote-tracking branch 'origin/staging' into fix/agiloft-connector

Author: mzxchandra

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

@@ -20,6 +20,10 @@ const {
   mockIsUsingCloudStorage,
   mockDownloadCopilotFile,
   mockInferContextFromKey,
+  mockParseWorkspaceFileKey,
+  mockAuthenticateWorkspaceFile,
+  mockReadWorkspaceFileContentByKey,
+  mockResolveServableDocBytes,
   mockGetContentType,
   mockFindLocalFile,
   mockCreateFileResponse,
@@ -40,6 +44,10 @@ const {
     mockIsUsingCloudStorage: vi.fn(),
     mockDownloadCopilotFile: vi.fn(),
     mockInferContextFromKey: vi.fn(),
+    mockParseWorkspaceFileKey: vi.fn(),
+    mockAuthenticateWorkspaceFile: vi.fn(),
+    mockReadWorkspaceFileContentByKey: vi.fn(),
+    mockResolveServableDocBytes: vi.fn(),
     mockGetContentType: vi.fn(),
     mockFindLocalFile: vi.fn(),
     mockCreateFileResponse: vi.fn(),
@@ -82,7 +90,19 @@ vi.mock('@/lib/execution/sandbox/run-task', () => ({
 }))
 
 vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
-  parseWorkspaceFileKey: vi.fn().mockReturnValue(undefined),
+  parseWorkspaceFileKey: mockParseWorkspaceFileKey,
+}))
+
+vi.mock('@/lib/workspace-files/api', () => ({
+  internalWorkspaceFileServeAuth: { authenticate: mockAuthenticateWorkspaceFile },
+}))
+
+vi.mock('@/lib/workspace-files/application/read-workspace-file-content-by-key', () => ({
+  readWorkspaceFileContentByKey: { execute: mockReadWorkspaceFileContentByKey },
+}))
+
+vi.mock('@/lib/copilot/tools/server/files/doc-compile', () => ({
+  resolveServableDocBytes: mockResolveServableDocBytes,
 }))
 
 vi.mock('@/app/api/files/utils', () => ({
@@ -109,7 +129,27 @@ describe('File Serve API Route', () => {
     mockReadFile.mockResolvedValue(Buffer.from('test content'))
     mockIsUsingCloudStorage.mockReturnValue(false)
     storageServiceMockFns.mockHasCloudStorage.mockReturnValue(true)
-    mockInferContextFromKey.mockReturnValue('workspace')
+    mockInferContextFromKey.mockReturnValue('mothership')
+    mockParseWorkspaceFileKey.mockReturnValue(undefined)
+    mockAuthenticateWorkspaceFile.mockResolvedValue({
+      kind: 'session',
+      userId: 'test-user-id',
+      sessionId: 'session-1',
+    })
+    mockReadWorkspaceFileContentByKey.mockResolvedValue({
+      file: {
+        id: 'file-1',
+        workspaceId: 'test-workspace-id',
+        name: 'report.pdf',
+      },
+      content: Buffer.from('generated source'),
+    })
+    mockResolveServableDocBytes.mockImplementation(
+      async ({ rawBuffer, fileName }: { rawBuffer: Buffer; fileName: string }) => ({
+        buffer: rawBuffer,
+        contentType: mockGetContentType(fileName),
+      })
+    )
     mockGetContentType.mockReturnValue('text/plain')
     mockFindLocalFile.mockReturnValue('/test/uploads/test-file.txt')
     mockCreateFileResponse.mockImplementation(
@@ -181,8 +221,59 @@ describe('File Serve API Route', () => {
 
     expect(storageServiceMockFns.mockDownloadFile).toHaveBeenCalledWith({
       key: 'workspace/test-workspace-id/1234567890-image.png',
-      context: 'workspace',
+      context: 'mothership',
+    })
+  })
+
+  it('serves a workspace document through the authorized use case and preserves the Principal', async () => {
+    const principal = {
+      kind: 'delegated' as const,
+      serviceId: 'executor' as const,
+      subjectUserId: 'test-user-id',
+      workspaceId: 'test-workspace-id',
+      delegationId: 'delegation-1',
+      audience: 'sim:workspace-files',
+      issuedAt: new Date('2026-08-01T00:00:00Z'),
+      expiresAt: new Date('2026-08-01T01:00:00Z'),
+      delegationContext: {
+        kind: 'workflow_execution' as const,
+        workflowId: 'workflow-1',
+      },
+    }
+    mockInferContextFromKey.mockReturnValue('workspace')
+    mockParseWorkspaceFileKey.mockReturnValue('test-workspace-id')
+    mockAuthenticateWorkspaceFile.mockResolvedValue(principal)
+    mockResolveServableDocBytes.mockResolvedValue({
+      buffer: Buffer.from('%PDF-compiled'),
+      contentType: 'application/pdf',
     })
+
+    const req = new NextRequest(
+      'http://localhost:3000/api/files/serve/workspace/test-workspace-id/report.pdf'
+    )
+    const response = await GET(req, {
+      params: Promise.resolve({
+        path: ['workspace', 'test-workspace-id', 'report.pdf'],
+      }),
+    })
+
+    expect(response.status).toBe(200)
+    expect(mockReadWorkspaceFileContentByKey).toHaveBeenCalledWith({
+      principal,
+      input: {
+        key: 'workspace/test-workspace-id/report.pdf',
+        assertedWorkspaceId: 'test-workspace-id',
+      },
+      request: req,
+    })
+    expect(mockResolveServableDocBytes).toHaveBeenCalledWith(
+      expect.objectContaining({
+        workspaceId: 'test-workspace-id',
+        filePrincipal: principal,
+      })
+    )
+    expect(hybridAuthMockFns.mockCheckSessionOrInternalAuth).not.toHaveBeenCalled()
+    expect(mockVerifyFileAccess).not.toHaveBeenCalled()
   })
 
   it('should return 404 when file not found', async () => {

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

@@ -1,18 +1,26 @@
 import { readFile } from 'fs/promises'
+import { type Principal, requirePrincipalSubjectUserId } from '@sim/auth/principal'
 import { createLogger } from '@sim/logger'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
 import { fileServeParamsSchema, fileServeQuerySchema } from '@/lib/api/contracts/storage-transfer'
+import {
+  concealCrossTenantResourceError,
+  InternalUnauthenticatedError,
+} from '@/lib/api/server/routes'
 import { checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
 import { resolveServableDocBytes } from '@/lib/copilot/tools/server/files/doc-compile'
 import { DocCompileUserError } from '@/lib/copilot/tools/server/files/doc-compile-error'
+import { asOrchestrationError } from '@/lib/core/orchestration/types'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { CopilotFiles, isUsingCloudStorage } from '@/lib/uploads'
 import type { StorageContext } from '@/lib/uploads/config'
 import { parseWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
 import { downloadFile } from '@/lib/uploads/core/storage-service'
 import { resolveServableImageBytes } from '@/lib/uploads/server/image-derivative'
 import { inferContextFromKey } from '@/lib/uploads/utils/file-utils'
+import { internalWorkspaceFileServeAuth } from '@/lib/workspace-files/api'
+import { readWorkspaceFileContentByKey } from '@/lib/workspace-files/application/read-workspace-file-content-by-key'
 import { verifyFileAccess } from '@/app/api/files/authorization'
 import {
   createErrorResponse,
@@ -66,9 +74,11 @@ async function resolveServableBytes(params: {
   workspaceId: string | undefined
   options: ServeOptions
   ownerKey: string | undefined
+  filePrincipal?: Principal
   signal: AbortSignal | undefined
 }): Promise<{ buffer: Buffer; contentType: string }> {
-  const { buffer, filename, storageKey, workspaceId, options, ownerKey, signal } = params
+  const { buffer, filename, storageKey, workspaceId, options, ownerKey, filePrincipal, signal } =
+    params
   if (options.raw) return { buffer, contentType: getContentType(filename) }
 
   if (options.preview) {
@@ -82,6 +92,7 @@ async function resolveServableBytes(params: {
     rawBuffer: buffer,
     fileName: filename,
     workspaceId,
+    filePrincipal,
     ownerKey,
     signal,
   })
@@ -154,6 +165,23 @@ export const GET = withRouteHandler(
         return await handleLocalFilePublic(fullPath)
       }
 
+      const storageContext = inferContextFromKey(cloudKey)
+      const workspacePrincipal =
+        storageContext === 'workspace'
+          ? await internalWorkspaceFileServeAuth.authenticate(request, { path })
+          : undefined
+      const legacyAuthResult = workspacePrincipal
+        ? undefined
+        : await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
+
+      if (legacyAuthResult && (!legacyAuthResult.success || !legacyAuthResult.userId)) {
+        logger.warn('Unauthorized file access attempt', {
+          path,
+          error: legacyAuthResult.error || 'Missing userId',
+        })
+        return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
+      }
+
       const query = fileServeQuerySchema.parse({
         raw: request.nextUrl.searchParams.get('raw'),
         preview: request.nextUrl.searchParams.get('preview'),
@@ -165,24 +193,24 @@ export const GET = withRouteHandler(
         versioned: query.v != null,
       }
 
-      const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
-
-      if (!authResult.success || !authResult.userId) {
-        logger.warn('Unauthorized file access attempt', {
-          path,
-          error: authResult.error || 'Missing userId',
-        })
-        return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
+      if (workspacePrincipal) {
+        return await handleWorkspaceFile(cloudKey, workspacePrincipal, options, request)
       }
 
-      const userId = authResult.userId
+      const userId = legacyAuthResult?.userId
+      if (!userId) throw new Error('Authenticated file serve request is missing a user ID')
 
       if (isUsingCloudStorage()) {
         return await handleCloudProxy(cloudKey, userId, options, request.signal)
       }
 
       return await handleLocalFile(cloudKey, userId, options, request.signal)
     } catch (error) {
+      if (error instanceof InternalUnauthenticatedError) {
+        logger.warn('Unauthorized file access attempt', { error: error.message })
+        return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
+      }
+
       // An in-progress/incomplete doc source fails to compile — this is expected
       // mid-generation, not a server fault. Return 409 (not 500) so it isn't an
       // alarming error; the client re-fetches once the doc finishes (the serve
@@ -194,6 +222,15 @@ export const GET = withRouteHandler(
         return NextResponse.json({ error: 'Document is still being generated' }, { status: 409 })
       }
 
+      const orchestrationError = asOrchestrationError(
+        concealCrossTenantResourceError(error, 'File not found')
+      )
+      if (orchestrationError?.code === 'not_found') {
+        const notFound = new FileNotFoundError('File not found')
+        logServeFailure('Error serving file:', notFound)
+        return createErrorResponse(notFound)
+      }
+
       logServeFailure('Error serving file:', error)
 
       if (error instanceof FileNotFoundError) {
@@ -205,6 +242,45 @@ export const GET = withRouteHandler(
   }
 )
 
+async function handleWorkspaceFile(
+  key: string,
+  principal: Principal,
+  options: ServeOptions,
+  request: NextRequest
+): Promise<NextResponse> {
+  const workspaceId = getWorkspaceIdForCompile(key)
+  if (!workspaceId) throw new FileNotFoundError(`File not found: ${key}`)
+
+  const { file, content } = await readWorkspaceFileContentByKey.execute({
+    principal,
+    input: { key, assertedWorkspaceId: workspaceId },
+    request,
+  })
+  const ownerKey = `user:${requirePrincipalSubjectUserId(principal)}`
+  const resolved = await resolveServableBytes({
+    buffer: content,
+    filename: file.name,
+    storageKey: key,
+    workspaceId,
+    options,
+    ownerKey,
+    filePrincipal: principal,
+    signal: request.signal,
+  })
+
+  logger.info('Workspace file served', {
+    fileId: file.id,
+    workspaceId,
+    size: resolved.buffer.length,
+  })
+  return createFileResponse({
+    buffer: resolved.buffer,
+    contentType: resolved.contentType,
+    filename: file.name,
+    cacheControl: resolveServeCacheControl(options.versioned, 'workspace'),
+  })
+}
+
 async function handleLocalFile(
   filename: string,
   userId: string,

apps/sim/app/api/v1/auth.test.ts

@@ -0,0 +1,86 @@
+/**
+ * @vitest-environment node
+ */
+
+import { NextRequest } from 'next/server'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const mocks = vi.hoisted(() => ({
+  authenticateApiKey: vi.fn(),
+  updateLastUsed: vi.fn(),
+}))
+
+vi.mock('@/lib/core/config/env-flags', () => ({ isAuthDisabled: false }))
+vi.mock('@/lib/api-key/service', () => ({
+  authenticateApiKeyFromHeader: mocks.authenticateApiKey,
+  updateApiKeyLastUsed: mocks.updateLastUsed,
+}))
+
+import { authenticateV1Request } from '@/app/api/v1/auth'
+
+describe('v1 API key authentication', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('constructs a personal API-key Principal from canonical key identity', async () => {
+    mocks.authenticateApiKey.mockResolvedValue({
+      success: true,
+      userId: 'user-1',
+      keyId: 'key-1',
+      keyType: 'personal',
+    })
+
+    await expect(
+      authenticateV1Request(
+        new NextRequest('http://localhost/api/v1/files', {
+          headers: { 'x-api-key': 'secret' },
+        })
+      )
+    ).resolves.toMatchObject({
+      authenticated: true,
+      principal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-1' },
+    })
+  })
+
+  it('constructs a workspace API-key Principal without borrowing the creator identity', async () => {
+    mocks.authenticateApiKey.mockResolvedValue({
+      success: true,
+      userId: 'creator-1',
+      keyId: 'key-1',
+      keyType: 'workspace',
+      workspaceId: 'workspace-1',
+    })
+
+    const result = await authenticateV1Request(
+      new NextRequest('http://localhost/api/v1/files', {
+        headers: { 'x-api-key': 'secret' },
+      })
+    )
+
+    expect(result.principal).toEqual({
+      kind: 'workspace_api_key',
+      workspaceId: 'workspace-1',
+      keyId: 'key-1',
+    })
+    expect(result.principal).not.toHaveProperty('userId')
+  })
+
+  it('fails closed when authenticated key identity is incomplete', async () => {
+    mocks.authenticateApiKey.mockResolvedValue({
+      success: true,
+      userId: 'creator-1',
+      keyId: 'key-1',
+      keyType: 'workspace',
+    })
+
+    await expect(
+      authenticateV1Request(
+        new NextRequest('http://localhost/api/v1/files', {
+          headers: { 'x-api-key': 'secret' },
+        })
+      )
+    ).resolves.toEqual({ authenticated: false, error: 'Authentication failed' })
+    expect(mocks.updateLastUsed).not.toHaveBeenCalled()
+  })
+})