From 690befd715ddc7c628330fbff0257799f6e8d131 Mon Sep 17 00:00:00 2001 From: moe Date: Sun, 10 May 2026 21:07:50 -0400 Subject: [PATCH 1/6] Block deleted bundle reads from cache Deleted bundle objects can still have a cached response at the edge. Check the app_versions deletion state for the exact app-scoped r2_path before serving cached attachment bytes, so a soft-deleted bundle cannot be read or restored from cache. Co-authored-by: Codex --- supabase/functions/_backend/files/files.ts | 17 ++++++++ tests/files-app-read-guard.unit.test.ts | 47 +++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 333ab10860..7bd25f46a4 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -356,6 +356,23 @@ async function assertReadableAppScopedAttachment(c: Context, fileId: unknown): P if (!app || app.owner_org !== scopedPath.owner_org) { quickError(404, 'not_found', 'Not found') } + + const deletedBundlePath = await pgClient.query<{ id: number }>( + ` + SELECT id + FROM public.app_versions + WHERE owner_org = $1 + AND app_id = $2 + AND r2_path = $3 + AND COALESCE(deleted, false) = true + LIMIT 1 + `, + [scopedPath.owner_org, scopedPath.app_id, fileId], + ) + + if (deletedBundlePath.rows.length > 0) { + quickError(404, 'not_found', 'Not found') + } } finally { await closeClient(c, pgClient) diff --git a/tests/files-app-read-guard.unit.test.ts b/tests/files-app-read-guard.unit.test.ts index 0b063dc938..641e5601c8 100644 --- a/tests/files-app-read-guard.unit.test.ts +++ b/tests/files-app-read-guard.unit.test.ts @@ -1,7 +1,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const getAppByAppIdPgMock = vi.fn() -const getPgClientMock = vi.fn(() => ({})) +const pgClientMock = { + query: vi.fn(), +} +const getPgClientMock = vi.fn(() => pgClientMock) vi.mock('hono/adapter', async (importOriginal) => { const actual = await importOriginal() @@ -32,6 +35,7 @@ describe('files app-scoped read guard', () => { beforeEach(() => { vi.resetModules() vi.clearAllMocks() + pgClientMock.query.mockResolvedValue({ rows: [] }) }) it('returns 404 for deleted app-scoped files before serving cached content', async () => { @@ -103,4 +107,45 @@ describe('files app-scoped read guard', () => { expect(bucketPut).not.toHaveBeenCalled() expect(getAppByAppIdPgMock).not.toHaveBeenCalled() }) + + it('returns 404 for soft-deleted bundle paths before serving cached content', async () => { + getAppByAppIdPgMock.mockResolvedValue({ app_id: 'test-app', owner_org: 'test-org' }) + pgClientMock.query.mockResolvedValue({ rows: [{ id: 123 }] }) + + const bucketPut = vi.fn() + globalThis.caches = { + default: { + match: async () => new Response('cached deleted bundle bytes', { + headers: { + 'content-type': 'application/zip', + }, + }), + put: async () => { }, + }, + } as any + + const { app: files } = await import('../supabase/functions/_backend/files/files.ts') + const { createAllCatch, createHono } = await import('../supabase/functions/_backend/utils/hono.ts') + const { version } = await import('../supabase/functions/_backend/utils/version.ts') + + const appGlobal = createHono('files', version) + appGlobal.route('/', files) + createAllCatch(appGlobal, 'files') + + const filePath = 'orgs/test-org/apps/test-app/1.0.0.zip' + const response = await appGlobal.fetch( + new Request(`http://localhost/read/attachments/${filePath}`), + { + ATTACHMENT_BUCKET: { put: bucketPut }, + }, + { waitUntil: () => { } } as any, + ) + + expect(response.status).toBe(404) + expect(pgClientMock.query).toHaveBeenCalledWith( + expect.stringContaining('FROM public.app_versions'), + ['test-org', 'test-app', filePath], + ) + expect(bucketPut).not.toHaveBeenCalled() + }) }) From 2fb2ada9a5fa84d9e8084deeae05acc989bd679a Mon Sep 17 00:00:00 2001 From: moe Date: Sun, 10 May 2026 21:19:17 -0400 Subject: [PATCH 2/6] Fix local read proxy test mock Keep the local read proxy unit test aligned with the app-scoped attachment guard's database lookup by returning an empty app_versions result from the mocked pg client. Co-authored-by: Codex --- tests/files-local-read-proxy.unit.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/files-local-read-proxy.unit.test.ts b/tests/files-local-read-proxy.unit.test.ts index dd3d7ce052..aad3da47a1 100644 --- a/tests/files-local-read-proxy.unit.test.ts +++ b/tests/files-local-read-proxy.unit.test.ts @@ -2,7 +2,10 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' const createSignedUrlMock = vi.fn() const getAppByAppIdPgMock = vi.fn() -const getPgClientMock = vi.fn(() => ({})) +const pgClientMock = { + query: vi.fn(), +} +const getPgClientMock = vi.fn(() => pgClientMock) const storageFromMock = vi.fn(() => ({ createSignedUrl: createSignedUrlMock, })) @@ -46,6 +49,7 @@ describe('files local read proxy', () => { beforeEach(() => { vi.resetModules() vi.clearAllMocks() + pgClientMock.query.mockResolvedValue({ rows: [] }) globalThis.fetch = originalFetch }) From 135d2df67dc86f3b0b6c4c4b31cf9457effffef4 Mon Sep 17 00:00:00 2001 From: moe Date: Sun, 10 May 2026 21:28:06 -0400 Subject: [PATCH 3/6] Add deleted bundle path index Create a partial app_versions index for deleted R2 paths and cover the non-deleted cached bundle path in the app-scoped read guard tests. Co-authored-by: Codex --- ...add_app_versions_deleted_r2_path_index.sql | 3 ++ tests/files-app-read-guard.unit.test.ts | 42 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql diff --git a/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql b/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql new file mode 100644 index 0000000000..ecc88e2748 --- /dev/null +++ b/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql @@ -0,0 +1,3 @@ +CREATE INDEX IF NOT EXISTS "idx_app_versions_deleted_r2_path" +ON "public"."app_versions" USING "btree" ("owner_org", "app_id", "r2_path") +WHERE ("deleted" = true); diff --git a/tests/files-app-read-guard.unit.test.ts b/tests/files-app-read-guard.unit.test.ts index 641e5601c8..87447fa897 100644 --- a/tests/files-app-read-guard.unit.test.ts +++ b/tests/files-app-read-guard.unit.test.ts @@ -148,4 +148,46 @@ describe('files app-scoped read guard', () => { ) expect(bucketPut).not.toHaveBeenCalled() }) + + it('serves cached content for non-deleted bundle paths', async () => { + getAppByAppIdPgMock.mockResolvedValue({ app_id: 'test-app', owner_org: 'test-org' }) + pgClientMock.query.mockResolvedValue({ rows: [] }) + + const bucketPut = vi.fn() + globalThis.caches = { + default: { + match: async () => new Response('cached bundle bytes', { + headers: { + 'content-type': 'application/zip', + }, + }), + put: async () => { }, + }, + } as any + + const { app: files } = await import('../supabase/functions/_backend/files/files.ts') + const { createAllCatch, createHono } = await import('../supabase/functions/_backend/utils/hono.ts') + const { version } = await import('../supabase/functions/_backend/utils/version.ts') + + const appGlobal = createHono('files', version) + appGlobal.route('/', files) + createAllCatch(appGlobal, 'files') + + const filePath = 'orgs/test-org/apps/test-app/1.0.0.zip' + const response = await appGlobal.fetch( + new Request(`http://localhost/read/attachments/${filePath}`), + { + ATTACHMENT_BUCKET: { put: bucketPut }, + }, + { waitUntil: () => { } } as any, + ) + + expect(response.status).toBe(200) + expect(await response.text()).toBe('cached bundle bytes') + expect(pgClientMock.query).toHaveBeenCalledWith( + expect.stringContaining('FROM public.app_versions'), + ['test-org', 'test-app', filePath], + ) + expect(bucketPut).not.toHaveBeenCalled() + }) }) From 46b689aa3da0fe6f14f83bdc7e51695bf6a816c1 Mon Sep 17 00:00:00 2001 From: moe Date: Sun, 10 May 2026 21:40:04 -0400 Subject: [PATCH 4/6] Cover deleted manifest bundle reads Extend the deleted bundle guard to manifest asset paths, keep private download links from resolving deleted bundles, and add focused regression coverage for both paths. Co-authored-by: Codex --- supabase/functions/_backend/files/files.ts | 11 ++- .../_backend/private/download_link.ts | 1 + .../download-link-deleted-bundle.unit.test.ts | 87 +++++++++++++++++++ tests/files-app-read-guard.unit.test.ts | 41 +++++++++ 4 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 tests/download-link-deleted-bundle.unit.test.ts diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 7bd25f46a4..5eff447fdd 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -363,8 +363,15 @@ async function assertReadableAppScopedAttachment(c: Context, fileId: unknown): P FROM public.app_versions WHERE owner_org = $1 AND app_id = $2 - AND r2_path = $3 - AND COALESCE(deleted, false) = true + AND deleted = true + AND ( + r2_path = $3 + OR EXISTS ( + SELECT 1 + FROM unnest(manifest) AS manifest_entry + WHERE manifest_entry.s3_path = $3 + ) + ) LIMIT 1 `, [scopedPath.owner_org, scopedPath.app_id, fileId], diff --git a/supabase/functions/_backend/private/download_link.ts b/supabase/functions/_backend/private/download_link.ts index 82642206f9..81ecc17720 100644 --- a/supabase/functions/_backend/private/download_link.ts +++ b/supabase/functions/_backend/private/download_link.ts @@ -44,6 +44,7 @@ app.post('/', middlewareAuth, async (c) => { .select('*, owner_org ( created_by )') .eq('app_id', body.app_id) .eq('id', body.id) + .eq('deleted', false) .single() const ownerOrg = bundle?.owner_org.created_by diff --git a/tests/download-link-deleted-bundle.unit.test.ts b/tests/download-link-deleted-bundle.unit.test.ts new file mode 100644 index 0000000000..8d977afe12 --- /dev/null +++ b/tests/download-link-deleted-bundle.unit.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAllCatch, createHono } from '../supabase/functions/_backend/utils/hono.ts' +import { version } from '../supabase/functions/_backend/utils/version.ts' + +const eqMock = vi.fn() +const singleMock = vi.fn() +const getBundleUrlMock = vi.fn() + +vi.mock('../supabase/functions/_backend/utils/discord.ts', () => ({ + sendDiscordAlert500: () => Promise.resolve(), + sendDiscordAlert: () => Promise.resolve(), +})) + +vi.mock('../supabase/functions/_backend/utils/downloadUrl.ts', () => ({ + getBundleUrl: getBundleUrlMock, + getManifestUrl: vi.fn(() => []), +})) + +vi.mock('../supabase/functions/_backend/utils/rbac.ts', () => ({ + checkPermission: vi.fn(() => Promise.resolve(true)), +})) + +vi.mock('../supabase/functions/_backend/utils/supabase.ts', () => ({ + supabaseClient: vi.fn(() => ({ + from: vi.fn(() => ({ + select: vi.fn(() => ({ + eq: eqMock, + })), + })), + })), +})) + +vi.mock('../supabase/functions/_backend/utils/hono.ts', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + middlewareAuth: async (c: any, next: () => Promise) => { + c.set('authorization', 'Bearer test-token') + c.set('auth', { userId: 'test-user' }) + await next() + }, + useCors: async (_c: any, next: () => Promise) => { + await next() + }, + } +}) + +describe('private download_link deleted bundle guard', () => { + it('filters deleted bundles before creating download URLs', async () => { + vi.resetModules() + vi.clearAllMocks() + + const query = { + eq: eqMock, + single: singleMock, + } + eqMock.mockReturnValue(query) + singleMock.mockResolvedValue({ + data: null, + error: { message: 'no rows' }, + }) + + const { app: downloadLink } = await import('../supabase/functions/_backend/private/download_link.ts') + const appGlobal = createHono('private', version) + appGlobal.route('/download_link', downloadLink) + createAllCatch(appGlobal, 'private') + + const response = await appGlobal.fetch( + new Request('http://localhost/download_link', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + app_id: 'test-app', + id: 123, + storage_provider: 'r2', + }), + }), + ) + + const body = await response.json() as { error: string } + + expect(response.status).toBe(400) + expect(body.error).toBe('cannot_get_bundle') + expect(eqMock).toHaveBeenCalledWith('deleted', false) + expect(getBundleUrlMock).not.toHaveBeenCalled() + }) +}) diff --git a/tests/files-app-read-guard.unit.test.ts b/tests/files-app-read-guard.unit.test.ts index 87447fa897..2e778a2cc0 100644 --- a/tests/files-app-read-guard.unit.test.ts +++ b/tests/files-app-read-guard.unit.test.ts @@ -149,6 +149,47 @@ describe('files app-scoped read guard', () => { expect(bucketPut).not.toHaveBeenCalled() }) + it('returns 404 for soft-deleted manifest asset paths before serving cached content', async () => { + getAppByAppIdPgMock.mockResolvedValue({ app_id: 'test-app', owner_org: 'test-org' }) + pgClientMock.query.mockResolvedValue({ rows: [{ id: 123 }] }) + + const bucketPut = vi.fn() + globalThis.caches = { + default: { + match: async () => new Response('cached deleted manifest bytes', { + headers: { + 'content-type': 'text/javascript', + }, + }), + put: async () => { }, + }, + } as any + + const { app: files } = await import('../supabase/functions/_backend/files/files.ts') + const { createAllCatch, createHono } = await import('../supabase/functions/_backend/utils/hono.ts') + const { version } = await import('../supabase/functions/_backend/utils/version.ts') + + const appGlobal = createHono('files', version) + appGlobal.route('/', files) + createAllCatch(appGlobal, 'files') + + const filePath = 'orgs/test-org/apps/test-app/assets/main.js' + const response = await appGlobal.fetch( + new Request(new URL(filePath, 'http://localhost/read/attachments/')), + { + ATTACHMENT_BUCKET: { put: bucketPut }, + }, + { waitUntil: () => { } } as any, + ) + + expect(response.status).toBe(404) + expect(pgClientMock.query).toHaveBeenCalledWith( + expect.stringContaining('unnest(manifest)'), + ['test-org', 'test-app', filePath], + ) + expect(bucketPut).not.toHaveBeenCalled() + }) + it('serves cached content for non-deleted bundle paths', async () => { getAppByAppIdPgMock.mockResolvedValue({ app_id: 'test-app', owner_org: 'test-org' }) pgClientMock.query.mockResolvedValue({ rows: [] }) From cec23dc7d18eeda662f9ed3a9c02e5ed55e09f5b Mon Sep 17 00:00:00 2001 From: moe Date: Sun, 10 May 2026 21:53:39 -0400 Subject: [PATCH 5/6] Avoid module reset in download link test Keep the deleted-bundle download link regression test scoped to local mocks so it does not reset modules during the wider backend suite. Co-authored-by: Codex --- tests/download-link-deleted-bundle.unit.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/download-link-deleted-bundle.unit.test.ts b/tests/download-link-deleted-bundle.unit.test.ts index 8d977afe12..003ec8af18 100644 --- a/tests/download-link-deleted-bundle.unit.test.ts +++ b/tests/download-link-deleted-bundle.unit.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { createAllCatch, createHono } from '../supabase/functions/_backend/utils/hono.ts' import { version } from '../supabase/functions/_backend/utils/version.ts' @@ -46,10 +46,11 @@ vi.mock('../supabase/functions/_backend/utils/hono.ts', async (importOriginal) = }) describe('private download_link deleted bundle guard', () => { - it('filters deleted bundles before creating download URLs', async () => { - vi.resetModules() + beforeEach(() => { vi.clearAllMocks() + }) + it('filters deleted bundles before creating download URLs', async () => { const query = { eq: eqMock, single: singleMock, From 0374e6c80371ba6e74a25baa25f5d841035d8968 Mon Sep 17 00:00:00 2001 From: moe Date: Sun, 10 May 2026 22:14:53 -0400 Subject: [PATCH 6/6] Use persisted manifest rows in deleted bundle guard Also adds the manifest lookup index and updates the read-guard regression to cover persisted manifest assets.\n\nCo-authored-by: Codex --- supabase/functions/_backend/files/files.ts | 5 +++-- ...20260511012637_add_app_versions_deleted_r2_path_index.sql | 3 +++ tests/files-app-read-guard.unit.test.ts | 5 +++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 5eff447fdd..51a3eaff2c 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -368,8 +368,9 @@ async function assertReadableAppScopedAttachment(c: Context, fileId: unknown): P r2_path = $3 OR EXISTS ( SELECT 1 - FROM unnest(manifest) AS manifest_entry - WHERE manifest_entry.s3_path = $3 + FROM public.manifest m + WHERE m.app_version_id = public.app_versions.id + AND m.s3_path = $3 ) ) LIMIT 1 diff --git a/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql b/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql index ecc88e2748..0bc53e77d0 100644 --- a/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql +++ b/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql @@ -1,3 +1,6 @@ CREATE INDEX IF NOT EXISTS "idx_app_versions_deleted_r2_path" ON "public"."app_versions" USING "btree" ("owner_org", "app_id", "r2_path") WHERE ("deleted" = true); + +CREATE INDEX IF NOT EXISTS "idx_manifest_app_version_id_s3_path" +ON "public"."manifest" USING "btree" ("app_version_id", "s3_path"); diff --git a/tests/files-app-read-guard.unit.test.ts b/tests/files-app-read-guard.unit.test.ts index 2e778a2cc0..4359bc24ab 100644 --- a/tests/files-app-read-guard.unit.test.ts +++ b/tests/files-app-read-guard.unit.test.ts @@ -149,7 +149,7 @@ describe('files app-scoped read guard', () => { expect(bucketPut).not.toHaveBeenCalled() }) - it('returns 404 for soft-deleted manifest asset paths before serving cached content', async () => { + it('returns 404 for soft-deleted persisted manifest asset paths before serving cached content', async () => { getAppByAppIdPgMock.mockResolvedValue({ app_id: 'test-app', owner_org: 'test-org' }) pgClientMock.query.mockResolvedValue({ rows: [{ id: 123 }] }) @@ -184,9 +184,10 @@ describe('files app-scoped read guard', () => { expect(response.status).toBe(404) expect(pgClientMock.query).toHaveBeenCalledWith( - expect.stringContaining('unnest(manifest)'), + expect.stringContaining('FROM public.manifest'), ['test-org', 'test-app', filePath], ) + expect(pgClientMock.query.mock.calls[0]?.[0]).not.toContain('unnest(manifest)') expect(bucketPut).not.toHaveBeenCalled() })