From 5ecc178ee9698cee57619105ec32e41cd5f5322a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 01:07:42 -0700 Subject: [PATCH] fix(copilot): clamp the legacy int4 size when materializing a chat upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `materialize_file(operation: 'save')` wrote the HEADed object size straight into `workspace_files.size`, which is still `integer NOT NULL`. Since the `size_bytes` widening (0289), a mothership chat attachment may be up to MAX_WORKSPACE_FILE_SIZE (5 GiB): `upload-session/service.ts` gives `mothership_attachment` that ceiling, and `finalizers.ts` already dual-writes the row as `size = 2147483647, size_bytes = `. Saving such an upload then re-read the true size from `headObject` and issued `SET size = 3221225472` against int4. Postgres raises 22003; the retry filter matches only 23505, so it rethrows, the transaction rolls back and the tool returns `success: false` with no way for the user to complete the save. No corruption — int4 overflow errors, it never truncates — but the file can never be saved. Every other `workspace_files` size writer already pairs `toLegacyWorkspaceFileSize(bytes)` with `sizeBytes: bytes` (metadata.ts x4, workspace-file-manager.ts:243/1706, finalizers.ts:367). This call site was simply missed when the widening landed; the fix converges it with the other six rather than inventing a third shape. Storage accounting keeps using the exact `verifiedSize`, so quota and usage are unaffected. The size source itself also had to widen. `head?.size ?? row.size` fell back to the clamped int4 column, and since this change now writes `sizeBytes` too, that fallback would overwrite an exact `size_bytes` with the clamp — the object is gone, so nothing could recover it, and the row would look internally consistent afterwards. The fallback is live whenever `hasCloudStorage()` is false, since the early return at the HEAD miss is cloud-only. Reading `row.sizeBytes ?? row.size` is the same coalescing shape the readers already use (workspace-file-manager.ts:227, finalizers.ts:399, metadata.ts:46), and the row comes from a full `select()` so the column is present. The clamp is derived once next to `verifiedSize` rather than inline in the update because the value is loop-invariant. Two sibling writers were examined and deliberately left alone. `copy-files.ts` reads `task.size` out of the int4 column itself, so it is arithmetically incapable of overflow, and its missing `sizeBytes` is unreachable behind the 100 MB fork download cap. `workspace-file-manager.ts:963` takes a caller-supplied size, but its insert branch needs an orphaned storage object with no `workspace_files` row, and converting loose external input from a DB error into a JS throw deserves its own review rather than a release patch; it is the next instance of this bug and should be filed as a follow-up. Both new tests were proven red against the unfixed code: the clamp test fails with "expected 3221225472 to be 2147483647", the fallback test with "expected undefined to be 3221225472". --- .../tools/handlers/materialize-file.test.ts | 55 +++++++++++++++++++ .../tools/handlers/materialize-file.ts | 11 +++- 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts index 69848850802..65af6a518cb 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts @@ -155,6 +155,9 @@ const STORAGE_CONTEXT = { customStorageLimitGB: null, } +const POSTGRES_INT4_MAX = 2_147_483_647 +const OVERSIZED_BYTES = 3 * 1024 * 1024 * 1024 + const mothershipRow = { id: 'file-1', key: 'mothership/file-1', @@ -339,6 +342,58 @@ describe('executeMaterializeFile - save storage transition', () => { ) }) + it('writes an int4-representable legacy size and the exact byte count above the int4 ceiling', async () => { + // A row above the int4 ceiling must not be written raw to `size`: Postgres + // raises 22003 and the save becomes unrecoverable. + mockHeadObject.mockResolvedValue({ size: OVERSIZED_BYTES, contentType: 'text/plain' }) + + const result = await executeMaterializeFile( + { fileNames: ['report.txt'], operation: 'save' }, + context + ) + + expect(result.success).toBe(true) + const [updateSet] = dbChainMockFns.set.mock.calls.at(-1) as [Record] + expect(updateSet.size).toBe(POSTGRES_INT4_MAX) + expect(updateSet.sizeBytes).toBe(OVERSIZED_BYTES) + expect(mockCheckStorageQuotaForBillingContext).toHaveBeenCalledWith( + STORAGE_CONTEXT, + OVERSIZED_BYTES + ) + expect(mockIncrementStorageUsageForBillingContextInTx).toHaveBeenCalledWith( + expect.anything(), + STORAGE_CONTEXT, + OVERSIZED_BYTES + ) + }) + + it('falls back to the exact stored byte count, not the clamped legacy size', async () => { + // Without cloud storage a missing object does not short-circuit, so the row is + // the only size source — and its `size` is already clamped. + mockHeadObject.mockResolvedValue(null) + mockHasCloudStorage.mockReturnValue(false) + mockFindUpload.mockResolvedValue({ + ...mothershipRow, + size: POSTGRES_INT4_MAX, + sizeBytes: OVERSIZED_BYTES, + }) + + const result = await executeMaterializeFile( + { fileNames: ['report.txt'], operation: 'save' }, + context + ) + + expect(result.success).toBe(true) + const [updateSet] = dbChainMockFns.set.mock.calls.at(-1) as [Record] + expect(updateSet.sizeBytes).toBe(OVERSIZED_BYTES) + expect(updateSet.size).toBe(POSTGRES_INT4_MAX) + expect(mockIncrementStorageUsageForBillingContextInTx).toHaveBeenCalledWith( + expect.anything(), + STORAGE_CONTEXT, + OVERSIZED_BYTES + ) + }) + it('materializes with an available root-level copy name', async () => { mockFindUpload.mockResolvedValueOnce({ ...mothershipRow, diff --git a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts index 25dc2a51685..d6b5e2dece6 100644 --- a/apps/sim/lib/copilot/tools/handlers/materialize-file.ts +++ b/apps/sim/lib/copilot/tools/handlers/materialize-file.ts @@ -36,6 +36,7 @@ import { } from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { getBoundWorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { hasCloudStorage, headObject } from '@/lib/uploads/core/storage-service' +import { toLegacyWorkspaceFileSize } from '@/lib/uploads/shared/types' import { isArchiveFileName } from '@/lib/uploads/utils/file-utils' import { parseWorkflowJson } from '@/lib/workflows/operations/import-export' import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils' @@ -109,7 +110,12 @@ async function executeSave( if (!head && hasCloudStorage()) { return { success: false, error: `Upload object not found: "${fileName}".` } } - const verifiedSize = head?.size ?? row.size + /** + * The true byte count can exceed the legacy int4 `size` column, so read the exact + * `sizeBytes` first and clamp back down for the legacy projection. + */ + const verifiedSize = head?.size ?? row.sizeBytes ?? row.size + const legacySize = toLegacyWorkspaceFileSize(verifiedSize) const billingContext = await resolveStorageBillingContext(workspaceId) const quotaCheck = await checkStorageQuotaForBillingContext(billingContext, verifiedSize) if (!quotaCheck.allowed) { @@ -145,7 +151,8 @@ async function executeSave( messageId: null, originalName: materializedName, displayName: materializedName, - size: verifiedSize, + size: legacySize, + sizeBytes: verifiedSize, }) .where( and(