Skip to content

Commit cd1b2be

Browse files
committed
fix(files): guard dimension writes by content key so a stale PATCH can't persist
Ties the dimensions write to the storage key the client measured. The key is regenerated on every content replacement, so an in-flight PATCH measured against superseded bytes is rejected at the DB (WHERE key = measured key) instead of persisting the old aspect ratio for new content. Closes the last stale-ordering window Greptile flagged — the write is now content-version-conditioned, not just corrected on the next render.
1 parent ba8507c commit cd1b2be

5 files changed

Lines changed: 34 additions & 18 deletions

File tree

apps/sim/app/api/workspaces/[id]/files/[fileId]/dimensions/route.test.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
1616

1717
const WS = '7727ef3f-8cf6-4686-b063-2bb006a10785'
1818
const FILE = 'wf_abc123'
19+
const KEY = 'workspace/7727ef3f/screenshot.png'
1920

2021
import { PATCH } from '@/app/api/workspaces/[id]/files/[fileId]/dimensions/route'
2122

@@ -37,43 +38,45 @@ describe('PATCH /api/workspaces/[id]/files/[fileId]/dimensions', () => {
3738
mockUpdateWorkspaceFileDimensions.mockResolvedValue(true)
3839
})
3940

40-
it('stores dimensions for a writer', async () => {
41-
const res = await PATCH(buildRequest({ width: 1600, height: 900 }), routeContext)
41+
it('stores dimensions for a writer, keyed to the content version', async () => {
42+
const res = await PATCH(buildRequest({ key: KEY, width: 1600, height: 900 }), routeContext)
4243
expect(res.status).toBe(200)
4344
expect(await res.json()).toEqual({ success: true })
4445
expect(mockUpdateWorkspaceFileDimensions).toHaveBeenCalledWith(WS, FILE, {
46+
key: KEY,
4547
width: 1600,
4648
height: 900,
4749
})
4850
})
4951

5052
it('allows an admin', async () => {
5153
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('admin')
52-
const res = await PATCH(buildRequest({ width: 10, height: 20 }), routeContext)
54+
const res = await PATCH(buildRequest({ key: KEY, width: 10, height: 20 }), routeContext)
5355
expect(res.status).toBe(200)
5456
expect(mockUpdateWorkspaceFileDimensions).toHaveBeenCalledOnce()
5557
})
5658

5759
it('rejects an unauthenticated caller before touching the DB', async () => {
5860
authMockFns.mockGetSession.mockResolvedValue(null)
59-
const res = await PATCH(buildRequest({ width: 10, height: 10 }), routeContext)
61+
const res = await PATCH(buildRequest({ key: KEY, width: 10, height: 10 }), routeContext)
6062
expect(res.status).toBe(401)
6163
expect(mockUpdateWorkspaceFileDimensions).not.toHaveBeenCalled()
6264
})
6365

6466
it('rejects a read-only member (backfill requires write)', async () => {
6567
permissionsMockFns.mockGetUserEntityPermissions.mockResolvedValue('read')
66-
const res = await PATCH(buildRequest({ width: 10, height: 10 }), routeContext)
68+
const res = await PATCH(buildRequest({ key: KEY, width: 10, height: 10 }), routeContext)
6769
expect(res.status).toBe(403)
6870
expect(mockUpdateWorkspaceFileDimensions).not.toHaveBeenCalled()
6971
})
7072

71-
it('rejects non-positive / non-integer dimensions', async () => {
73+
it('rejects a missing key or non-positive / non-integer dimensions', async () => {
7274
for (const body of [
73-
{ width: 0, height: 10 },
74-
{ width: 10, height: -5 },
75-
{ width: 10.5, height: 10 },
76-
{ width: 10 },
75+
{ width: 10, height: 10 }, // missing key
76+
{ key: KEY, width: 0, height: 10 },
77+
{ key: KEY, width: 10, height: -5 },
78+
{ key: KEY, width: 10.5, height: 10 },
79+
{ key: KEY, width: 10 },
7780
]) {
7881
const res = await PATCH(buildRequest(body), routeContext)
7982
expect(res.status).toBe(400)

apps/sim/app/api/workspaces/[id]/files/[fileId]/dimensions/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ export const PATCH = withRouteHandler(
2727
const parsed = await parseRequest(updateWorkspaceFileDimensionsContract, request, context)
2828
if (!parsed.success) return parsed.response
2929
const { id: workspaceId, fileId } = parsed.data.params
30-
const { width, height } = parsed.data.body
30+
const { key, width, height } = parsed.data.body
3131

3232
const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId)
3333
if (permission !== 'admin' && permission !== 'write') {
3434
return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
3535
}
3636

3737
try {
38-
await updateWorkspaceFileDimensions(workspaceId, fileId, { width, height })
38+
await updateWorkspaceFileDimensions(workspaceId, fileId, { key, width, height })
3939
return NextResponse.json({ success: true as const })
4040
} catch (error) {
4141
logger.error('Failed to backfill workspace file dimensions', {

apps/sim/hooks/queries/workspace-files.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ export function useWorkspaceImageDimensionsAdapter(workspaceId: string): ImageDi
164164
)
165165
void requestJson(updateWorkspaceFileDimensionsContract, {
166166
params: { id: workspaceId, fileId: record.id },
167-
body: dimensions,
167+
// Send the key we measured against; the server rejects the write if the row's content (key) has
168+
// since changed, so a stale in-flight PATCH for replaced bytes can't persist the old size.
169+
body: { key: record.key, ...dimensions },
168170
}).catch(() => {})
169171
},
170172
}

apps/sim/lib/api/contracts/workspace-files.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ export const updateWorkspaceFileContentBodySchema = z.object({
6060
const IMAGE_DIMENSION_MAX = 100_000
6161

6262
export const updateWorkspaceFileDimensionsBodySchema = z.object({
63+
/**
64+
* The storage key the client measured. The write commits only if the row still has this key — a
65+
* content-version guard: the key changes on every content replacement, so a stale in-flight write for
66+
* superseded bytes is rejected rather than persisting the old aspect ratio.
67+
*/
68+
key: z.string().min(1, 'key is required'),
6369
width: z.number().int().positive().max(IMAGE_DIMENSION_MAX),
6470
height: z.number().int().positive().max(IMAGE_DIMENSION_MAX),
6571
})

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -864,15 +864,19 @@ async function mapSingleWorkspaceFileRecord(
864864
/**
865865
* Store an image file's intrinsic pixel dimensions (a pure rendering hint used to reserve layout space
866866
* before the image loads). The client reports the browser's own EXIF-corrected `naturalWidth/Height`, and
867-
* only when it differs from what's stored, so this overwrites rather than backfilling once — a stale
868-
* value (e.g. left over after the file's content was replaced) self-corrects on the next view instead of
869-
* sticking behind a `width IS NULL` guard. Does NOT touch `updatedAt` — dimensions are not content and
870-
* must not cache-bust the served image bytes. Returns whether a live row was written.
867+
* only when it differs from what's stored, so this overwrites rather than backfilling once — a stale value
868+
* self-corrects on the next view instead of sticking behind a `width IS NULL` guard.
869+
*
870+
* `key` is a content-version guard: the write commits only if the row still has the storage key the
871+
* client measured. The key is regenerated on every content replacement, so an in-flight write measured
872+
* against superseded bytes is rejected here rather than persisting the old aspect ratio for new content.
873+
* Does NOT touch `updatedAt` — dimensions are not content and must not cache-bust the served image bytes.
874+
* Returns whether a live row was written.
871875
*/
872876
export async function updateWorkspaceFileDimensions(
873877
workspaceId: string,
874878
fileId: string,
875-
dimensions: { width: number; height: number }
879+
dimensions: { key: string; width: number; height: number }
876880
): Promise<boolean> {
877881
const updated = await db
878882
.update(workspaceFiles)
@@ -881,6 +885,7 @@ export async function updateWorkspaceFileDimensions(
881885
and(
882886
eq(workspaceFiles.id, fileId),
883887
eq(workspaceFiles.workspaceId, workspaceId),
888+
eq(workspaceFiles.key, dimensions.key),
884889
isNull(workspaceFiles.deletedAt)
885890
)
886891
)

0 commit comments

Comments
 (0)