Skip to content

Commit 2cf9178

Browse files
committed
fix(db): fail closed on missing file sizes
1 parent 1ffc614 commit 2cf9178

4 files changed

Lines changed: 51 additions & 11 deletions

File tree

apps/sim/background/cleanup-soft-deletes.test.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ describe('cleanup soft deletes', () => {
121121
key: 'workspace/ws-1/file-failed',
122122
workspaceId: 'ws-1',
123123
context: 'workspace',
124-
size: 11,
124+
sizeBytes: 11,
125125
},
126126
])
127127
mockDeleteFiles.mockResolvedValueOnce({
@@ -148,14 +148,14 @@ describe('cleanup soft deletes', () => {
148148
key: 'workspace/ws-1/file-deleted',
149149
workspaceId: 'ws-1',
150150
context: 'workspace',
151-
size: 7,
151+
sizeBytes: 7,
152152
},
153153
{
154154
id: 'file-restored',
155155
key: 'workspace/ws-1/file-restored',
156156
workspaceId: 'ws-1',
157157
context: 'workspace',
158-
size: 13,
158+
sizeBytes: 13,
159159
},
160160
])
161161
mockDeleteFiles.mockResolvedValueOnce({ deleted: 2, failed: [] })
@@ -184,7 +184,7 @@ describe('cleanup soft deletes', () => {
184184
key: 'mothership/chat-file',
185185
workspaceId: 'ws-1',
186186
context: 'mothership',
187-
size: 17,
187+
sizeBytes: 17,
188188
},
189189
])
190190
mockDeleteFiles.mockResolvedValueOnce({ deleted: 1, failed: [] })
@@ -198,6 +198,26 @@ describe('cleanup soft deletes', () => {
198198
expect(mockDecrementStorageUsageForBillingContextInTx).not.toHaveBeenCalled()
199199
})
200200

201+
it('fails before deleting storage when canonical size metadata is missing', async () => {
202+
mockSelectRowsByIdChunks
203+
.mockResolvedValueOnce([])
204+
.mockResolvedValueOnce([])
205+
.mockResolvedValueOnce([
206+
{
207+
id: 'file-missing-size',
208+
key: 'workspace/ws-1/file-missing-size',
209+
workspaceId: 'ws-1',
210+
context: 'workspace',
211+
sizeBytes: null,
212+
},
213+
])
214+
215+
await expect(runCleanupSoftDeletes(basePayload)).rejects.toThrow(
216+
'Workspace file is missing canonical size_bytes metadata'
217+
)
218+
expect(mockDeleteFiles).not.toHaveBeenCalled()
219+
})
220+
201221
it('hard-deletes retained documents before deleting an expired knowledge base', async () => {
202222
mockChunkedBatchDelete.mockImplementationOnce(
203223
async (options: {

apps/sim/background/cleanup-soft-deletes.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import type { StorageContext } from '@/lib/uploads'
3535
import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
3636
import { allocateUniqueWorkspaceFileName } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
3737
import { deleteFileMetadata } from '@/lib/uploads/server/metadata'
38+
import { getWorkspaceFileSize } from '@/lib/uploads/shared/types'
3839
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
3940

4041
const logger = createLogger('CleanupSoftDeletes')
@@ -113,7 +114,7 @@ async function selectExpiredWorkspaceFiles(
113114
key: workspaceFiles.key,
114115
workspaceId: workspaceFiles.workspaceId,
115116
context: workspaceFiles.context,
116-
size: sql<number>`${workspaceFiles.sizeBytes}`.mapWith(Number),
117+
sizeBytes: workspaceFiles.sizeBytes,
117118
})
118119
.from(workspaceFiles)
119120
.where(
@@ -134,7 +135,7 @@ async function selectExpiredWorkspaceFiles(
134135
key: r.key,
135136
workspaceId: r.workspaceId,
136137
context: r.context as StorageContext,
137-
size: r.size,
138+
size: getWorkspaceFileSize(r),
138139
})),
139140
}
140141
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import {
4949
} from '@/ee/workspace-forking/lib/copy/storage-quota'
5050
import { ForkError } from '@/ee/workspace-forking/lib/lineage/authz'
5151

52-
function makeExecutor(total: number | string) {
52+
function makeExecutor(total: number | string | null) {
5353
const execute = vi.fn((_query: unknown) => Promise.resolve([{ total }]))
5454
return { executor: { execute } as unknown as DbOrTx, execute }
5555
}
@@ -71,6 +71,8 @@ describe('sumForkCopyBytes', () => {
7171
const compiled = outerQuery.toSQL()
7272
expect(compiled.sql).toBe('SELECT (? + ?)::bigint AS total')
7373
const [fileBytes, kbBytes] = compiled.params
74+
expect(fileBytes.toSQL().sql).toContain('count(*) FILTER')
75+
expect(fileBytes.toSQL().sql).toContain('IS NULL')
7476
expect(fileBytes.toSQL().params).toContainEqual({
7577
type: 'and',
7678
conditions: [
@@ -101,6 +103,17 @@ describe('sumForkCopyBytes', () => {
101103
expect(bytes).toBe(1024)
102104
})
103105

106+
it('fails closed when a selected workspace file lacks canonical size metadata', async () => {
107+
const { executor } = makeExecutor(null)
108+
109+
await expect(
110+
sumForkCopyBytes(executor, 'src-ws', { fileIds: ['wf-missing-size'] })
111+
).rejects.toMatchObject({
112+
message: 'Storage calculation is temporarily unavailable',
113+
statusCode: 503,
114+
})
115+
})
116+
104117
it('runs no query for an empty selection', async () => {
105118
const { executor, execute } = makeExecutor(0)
106119

apps/sim/ee/workspace-forking/lib/copy/storage-quota.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,11 @@ export async function sumForkCopyBytes(
4747
const fileBytes =
4848
fileSelectors.length === 0
4949
? sql<number>`0`
50-
: sql<number>`(
51-
SELECT coalesce(sum(${workspaceFiles.sizeBytes}), 0)
50+
: sql<number | null>`(
51+
SELECT CASE
52+
WHEN count(*) FILTER (WHERE ${workspaceFiles.sizeBytes} IS NULL) > 0 THEN NULL
53+
ELSE coalesce(sum(${workspaceFiles.sizeBytes}), 0)
54+
END
5255
FROM ${workspaceFiles}
5356
WHERE ${and(
5457
fileSelectors.length === 1 ? fileSelectors[0] : or(...fileSelectors),
@@ -74,10 +77,13 @@ export async function sumForkCopyBytes(
7477
isNotNull(document.storageKey)
7578
)}
7679
)`
77-
const [row] = await executor.execute<{ total: number | string }>(
80+
const [row] = await executor.execute<{ total: number | string | null }>(
7881
sql`SELECT (${fileBytes} + ${kbBytes})::bigint AS total`
7982
)
80-
return Number(row?.total ?? 0)
83+
if (row?.total == null) {
84+
throw new ForkError('Storage calculation is temporarily unavailable', 503)
85+
}
86+
return Number(row.total)
8187
}
8288

8389
type ForkCreationPayerPolicy = Pick<

0 commit comments

Comments
 (0)