fix(files): stop saving fetched URLs to Files and look names up by the unique index

.agents/skills/memory-load-check/SKILL.md

@@ -45,7 +45,7 @@ Read these when doing a deeper pass:
   - dispatch concrete chunks (`workspaceIds`, retention, label) instead of one giant scope
   - prefer Trigger.dev queue/concurrency keys when available
   - execute inline fallback chunks sequentially, not with unbounded `Promise.all`
-- File parse pattern in `apps/sim/lib/internal/file/parser.ts` and `apps/sim/lib/uploads/contexts/workspace/fetch-external-url.ts`
+- File parse pattern in `apps/sim/lib/internal/file/parser.ts` and `apps/sim/lib/uploads/utils/fetch-external-url.server.ts`
   - cap downloads and parsed output separately
   - preserve partial results when a later item exceeds the cap
   - never read untrusted response bodies without a byte cap

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

@@ -14,6 +14,7 @@ import {
   workspaceFileSecretProvenanceMock,
   workspaceFileSecretProvenanceMockFns,
 } from '@sim/testing/mocks/workspace-file-secret-provenance.mock'
+import { generateShortId } from '@sim/utils/id'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 /** The `workspace_files` columns {@link fileRows} enforces its unique indexes on. */
@@ -36,7 +37,7 @@ interface WorkspaceFileRow {
  */
 const { fileRows, allocateFromFileRows } = vi.hoisted(() => {
   const fileRows: WorkspaceFileRow[] = []
-  const withCopySuffix = (name: string, n: number) => {
+  const withCopySuffix = (name: string, n: number | string) => {
     const lastDot = name.lastIndexOf('.')
     return lastDot > 0 && lastDot < name.length - 1
       ? `${name.slice(0, lastDot)} (${n})${name.slice(lastDot)}`
@@ -63,11 +64,11 @@ const { fileRows, allocateFromFileRows } = vi.hoisted(() => {
             row.originalName === name
         )
       if (!taken(baseName)) return baseName
-      for (let n = 1; n <= 1000; n++) {
+      for (let n = 1; n <= 20; n++) {
         const candidate = withCopySuffix(baseName, n)
         if (!taken(candidate)) return candidate
       }
-      throw new Error(`A file named "${baseName}" already exists in this workspace`)
+      return withCopySuffix(baseName, generateShortId(8))
     },
   }
 })

apps/sim/lib/internal/file/parser.test.ts

@@ -27,10 +27,7 @@ import {
 } from '@sim/testing/mocks/uploads-execution.mock'
 import { uploadsMetadataMock } from '@sim/testing/mocks/uploads-metadata.mock'
 import { uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock'
-import {
-  workspaceFileManagerMock,
-  workspaceFileManagerMockFns,
-} from '@sim/testing/mocks/workspace-file-manager.mock'
+import { workspaceFileManagerMock } from '@sim/testing/mocks/workspace-file-manager.mock'
 import {
   workspaceFileSecretProvenanceMock,
   workspaceFileSecretProvenanceMockFns,
@@ -181,21 +178,8 @@ vi.mock('fs/promises', () => ({
 }))
 
 const { mockGetStorageProvider, mockIsUsingCloudStorage } = uploadsMockFns
-const { mockUploadWorkspaceFile } = workspaceFileManagerMockFns
 const { mockGetBoundWorkspaceFileSecretProvenance } = workspaceFileSecretProvenanceMockFns
 
-mockUploadWorkspaceFile.mockImplementation(
-  async (workspaceId: string, _userId: string, _buffer: Buffer, fileName: string) => ({
-    id: 'wf_test',
-    name: fileName,
-    size: 0,
-    type: 'application/octet-stream',
-    url: `/api/files/serve/${workspaceId}/${fileName}`,
-    key: `${workspaceId}/${fileName}`,
-    context: 'workspace',
-  })
-)
-
 import { fileParseBodySchema } from '@/lib/api/contracts/storage-transfer'
 import { executeFileParserOperation } from '@/lib/internal/file/parser'
 import { createWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal'
@@ -658,7 +642,6 @@ describe('file parser operation', () => {
         })
       )
     mockIsSupportedFileType.mockReturnValue(false)
-    permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('write')
 
     const req = createMockRequest('POST', {
       filePath: [
@@ -686,7 +669,6 @@ describe('file parser operation', () => {
       '203.0.113.10',
       expect.any(Object)
     )
-    expect(mockUploadWorkspaceFile).toHaveBeenCalledTimes(2)
     expect(storageServiceMockFns.mockDownloadFile).not.toHaveBeenCalled()
   })
 

apps/sim/lib/internal/file/parser.ts

@@ -35,16 +35,16 @@ import {
 } from '@/lib/internal/file/operations'
 import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
 import { uploadExecutionFile } from '@/lib/uploads/contexts/execution'
-import {
-  ExternalUrlValidationError,
-  fetchExternalUrlToWorkspace,
-} from '@/lib/uploads/contexts/workspace'
 import {
   getBoundWorkspaceFileSecretProvenance,
   type WorkspaceFileSecretProvenance,
 } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance'
 import { UPLOAD_DIR_SERVER } from '@/lib/uploads/core/setup.server'
 import { isWorkspaceScopedContext } from '@/lib/uploads/shared/types'
+import {
+  ExternalUrlValidationError,
+  fetchExternalUrl,
+} from '@/lib/uploads/utils/fetch-external-url.server'
 import {
   extractCleanFilename,
   extractStorageKey,
@@ -512,7 +512,7 @@ function assertParsedContentWithinLimit(content: string, maxBytes?: number): str
  * Validate file path for security - prevents null byte injection and path traversal attacks.
  *
  * External URLs (`http`/`https`) are fetched over HTTP — with SSRF protection applied
- * downstream in `fetchExternalUrlToWorkspace` (DNS resolution + private/reserved IP blocking)
+ * downstream in `fetchExternalUrl` (DNS resolution + private/reserved IP blocking)
  * — and are never resolved against the filesystem, so `..`/`~` are legal URL content and must
  * not be rejected. Providers such as Slack routinely emit slugs containing a literal `...`.
  *
@@ -560,8 +560,8 @@ function validateFilePath(filePath: string): { isValid: boolean; error?: string
  *
  * Always fetches the URL fresh — there is no filename-based dedup. Distinct URLs
  * commonly share a path tail (e.g. every Slack clipboard paste is `image.png`),
- * so keying a cache by filename returns stale bytes. `fetchExternalUrlToWorkspace`
- * delegates to `uploadWorkspaceFile`, which suffix-disambiguates collisions on save.
+ * so keying a cache by filename returns stale bytes. The fetched bytes are never saved
+ * to workspace Files; with an execution context they are kept as an execution file only.
  *
  * URLs for our workspace and execution storage resolve through the authorized canonical
  * read path, keeping stored provenance bound to the same bytes the parser reads.
@@ -647,11 +647,8 @@ async function handleExternalUrl(
       )
     }
 
-    const { filename, buffer, mimeType } = await fetchExternalUrlToWorkspace({
+    const { filename, buffer, mimeType } = await fetchExternalUrl({
       url,
-      userId,
-      workspaceId: workspaceId || undefined,
-      saveToWorkspace: Boolean(workspaceId),
       headers,
       signal,
       maxDownloadBytes,

apps/sim/lib/uploads/archive.test.ts

@@ -114,7 +114,7 @@ function craftCentralDirectory(records: number, extraPerRecord: number): Buffer
   return buffer
 }
 
-/** Mirrors `allocateUniqueWorkspaceFileName`'s " (n)" suffixing. */
+/** Numbered " (n)" suffixing in the style of `allocateUniqueWorkspaceFileName`'s first candidates. */
 function allocateUniqueName(folderKey: string, name: string): string {
   const dot = name.lastIndexOf('.')
   const base = dot > 0 ? name.slice(0, dot) : name

apps/sim/lib/uploads/contexts/workspace/__integration__/file-names.integration.ts

@@ -0,0 +1,197 @@
+/** Real PostgreSQL name allocation and name lookups for workspace files, plus the URL fetch path. */
+import { mkdtempSync } from 'node:fs'
+import { rm } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import path from 'node:path'
+import { db, dbFor } from '@sim/db'
+import { organization, user, workspace, workspaceFiles } from '@sim/db/schema'
+import { generateId } from '@sim/utils/id'
+import { and, eq, inArray, isNull, sql } from 'drizzle-orm'
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
+
+const fixtureStorage = vi.hoisted(() => ({ root: '' }))
+vi.mock('@/lib/uploads/core/setup.server', () => ({
+  get UPLOAD_DIR_SERVER() {
+    return fixtureStorage.root
+  },
+}))
+
+import { fileParseBodySchema } from '@/lib/api/contracts/storage-transfer'
+import * as inputValidation from '@/lib/core/security/input-validation.server'
+import { executeFileParserOperation } from '@/lib/internal/file/parser'
+import {
+  createKnowledgeAclFixtureIds,
+  seedKnowledgeAclFixture,
+} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
+import {
+  createWorkspaceFileFolder,
+  fileNameExistsInWorkspaceFolder,
+  workspaceFileNameFolderCondition,
+} from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
+import {
+  getWorkspaceFileByName,
+  uploadWorkspaceFile,
+} from '@/lib/uploads/contexts/workspace/workspace-file-manager'
+import { createWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal'
+
+describe('workspace file names in PostgreSQL', () => {
+  const fixtures: ReturnType<typeof createKnowledgeAclFixtureIds>[] = []
+
+  beforeAll(() => {
+    fixtureStorage.root = mkdtempSync(path.join(tmpdir(), 'sim-file-names-'))
+  })
+
+  afterAll(async () => {
+    vi.restoreAllMocks()
+    for (const ids of fixtures) {
+      await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))
+      await db.delete(organization).where(eq(organization.id, ids.organizationId))
+      await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId]))
+    }
+    await rm(fixtureStorage.root, { recursive: true, force: true })
+    await Promise.all([db.$client.end(), dbFor('cleanup').$client.end()])
+  })
+
+  async function seedWorkspace() {
+    const ids = createKnowledgeAclFixtureIds()
+    fixtures.push(ids)
+    await seedKnowledgeAclFixture(ids)
+    return ids
+  }
+
+  function upload(workspaceId: string, userId: string, name: string, folderId?: string | null) {
+    return uploadWorkspaceFile(workspaceId, userId, Buffer.from(name), name, 'text/plain', {
+      folderId,
+      notifyWorkspaceChange: false,
+    })
+  }
+
+  async function parseExternalUrl(executionId?: string) {
+    const fixture = await seedWorkspace()
+    const url = 'https://example.com/page.txt'
+    vi.spyOn(inputValidation, 'validateUrlWithDNS').mockResolvedValue({
+      isValid: true,
+      resolvedIP: '203.0.113.10',
+      originalHostname: new URL(url).hostname,
+    })
+    vi.spyOn(inputValidation, 'secureFetchWithPinnedIP').mockImplementation(async () => {
+      const response = new Response('fetched page body')
+      return {
+        ok: response.ok,
+        status: response.status,
+        statusText: response.statusText,
+        headers: new inputValidation.SecureFetchHeaders({ 'content-type': 'text/plain' }),
+        body: response.body,
+        text: () => response.text(),
+        json: () => response.json(),
+        arrayBuffer: () => response.arrayBuffer(),
+      }
+    })
+
+    const response = await executeFileParserOperation(
+      fileParseBodySchema.parse({ filePath: url, workspaceId: fixture.workspaceId }),
+      {
+        principal: createWorkspaceFileDelegatedPrincipal({
+          serviceId: 'executor',
+          subjectUserId: fixture.aliceId,
+          workspaceId: fixture.workspaceId,
+          delegationId: generateId(),
+          executionId,
+        }),
+        workspaceId: fixture.workspaceId,
+        workflowId: generateId(),
+        executionId,
+        attributedUserId: fixture.aliceId,
+        fileAccessUserId: fixture.aliceId,
+      }
+    )
+    const body = await response.json()
+
+    expect(response.status).toBe(200)
+    expect(body.output.content).toContain('fetched page body')
+    return db
+      .select({ context: workspaceFiles.context })
+      .from(workspaceFiles)
+      .where(eq(workspaceFiles.workspaceId, fixture.workspaceId))
+  }
+
+  it('parses an external URL without saving a copy to workspace Files', async () => {
+    expect(await parseExternalUrl()).toEqual([])
+  })
+
+  it('keeps an external URL parsed during an execution as an execution file only', async () => {
+    expect(await parseExternalUrl(generateId())).toEqual([{ context: 'execution' }])
+  })
+
+  it('scopes name lookups to root or folder through the unique name index', async () => {
+    const fixture = await seedWorkspace()
+    const folder = await createWorkspaceFileFolder({
+      workspaceId: fixture.workspaceId,
+      userId: fixture.aliceId,
+      name: 'Reports',
+    })
+    const rootFile = await upload(fixture.workspaceId, fixture.aliceId, 'root.txt')
+    const folderFile = await upload(fixture.workspaceId, fixture.aliceId, 'nested.txt', folder.id)
+
+    expect(await fileNameExistsInWorkspaceFolder(fixture.workspaceId, 'root.txt', null)).toBe(true)
+    expect(await fileNameExistsInWorkspaceFolder(fixture.workspaceId, 'root.txt', folder.id)).toBe(
+      false
+    )
+    expect(await fileNameExistsInWorkspaceFolder(fixture.workspaceId, 'nested.txt', null)).toBe(
+      false
+    )
+    expect(
+      await fileNameExistsInWorkspaceFolder(fixture.workspaceId, 'nested.txt', folder.id)
+    ).toBe(true)
+    expect((await getWorkspaceFileByName(fixture.workspaceId, 'root.txt'))?.id).toBe(rootFile.id)
+    expect(
+      await getWorkspaceFileByName(fixture.workspaceId, 'root.txt', { folderId: folder.id })
+    ).toBeNull()
+    expect(
+      (await getWorkspaceFileByName(fixture.workspaceId, 'nested.txt', { folderId: folder.id }))?.id
+    ).toBe(folderFile.id)
+    expect(await getWorkspaceFileByName(fixture.workspaceId, 'nested.txt')).toBeNull()
+
+    await db.execute(sql`
+      INSERT INTO ${workspaceFiles} (id, key, user_id, workspace_id, folder_id, context, original_name, content_type)
+      SELECT 'wf_pad_' || n || '_' || ${fixture.workspaceId}, 'pad/' || n || '/' || ${fixture.workspaceId},
+        ${fixture.aliceId}, ${fixture.workspaceId}, CASE WHEN n % 2 = 0 THEN ${folder.id} END,
+        'workspace', 'pad-' || n || '.txt', 'text/plain'
+      FROM generate_series(1, 2000) AS n`)
+    await db.execute(sql`ANALYZE ${workspaceFiles}`)
+
+    for (const folderId of [null, folder.id]) {
+      const plan = await db.execute(
+        sql`EXPLAIN (FORMAT JSON) SELECT id FROM ${workspaceFiles} WHERE ${and(
+          eq(workspaceFiles.workspaceId, fixture.workspaceId),
+          eq(workspaceFiles.originalName, 'root.txt'),
+          eq(workspaceFiles.context, 'workspace'),
+          workspaceFileNameFolderCondition(folderId),
+          isNull(workspaceFiles.deletedAt)
+        )}`
+      )
+      const scan = JSON.stringify(plan[0]['QUERY PLAN'])
+      expect(scan).toContain('workspace_files_workspace_folder_name_active_unique')
+      expect(scan).toMatch(/"Index Cond":"[^"]*COALESCE\(folder_id/)
+    }
+  })
+
+  it('falls back to a short-id suffix after 20 numbered copies, including under concurrency', async () => {
+    const fixture = await seedWorkspace()
+    await upload(fixture.workspaceId, fixture.aliceId, 'page.html')
+    for (let n = 1; n <= 20; n++) {
+      await upload(fixture.workspaceId, fixture.aliceId, `page (${n}).html`)
+    }
+
+    const next = await upload(fixture.workspaceId, fixture.aliceId, 'page.html')
+    const concurrent = await Promise.all(
+      Array.from({ length: 8 }, () => upload(fixture.workspaceId, fixture.aliceId, 'page.html'))
+    )
+
+    const shortIdSuffixed = /^page \([A-Za-z0-9_-]{8}\)\.html$/
+    expect(next.name).toMatch(shortIdSuffixed)
+    const names = concurrent.map((file) => file.name)
+    for (const name of names) expect(name).toMatch(shortIdSuffixed)
+    expect(new Set([next.name, ...names]).size).toBe(names.length + 1)
+  })
+})