diff --git a/supabase/functions/_backend/files/files.ts b/supabase/functions/_backend/files/files.ts index 333ab10860..51a3eaff2c 100644 --- a/supabase/functions/_backend/files/files.ts +++ b/supabase/functions/_backend/files/files.ts @@ -356,6 +356,31 @@ 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 deleted = true + AND ( + r2_path = $3 + OR EXISTS ( + SELECT 1 + FROM public.manifest m + WHERE m.app_version_id = public.app_versions.id + AND m.s3_path = $3 + ) + ) + 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/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/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..0bc53e77d0 --- /dev/null +++ b/supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql @@ -0,0 +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/download-link-deleted-bundle.unit.test.ts b/tests/download-link-deleted-bundle.unit.test.ts new file mode 100644 index 0000000000..003ec8af18 --- /dev/null +++ b/tests/download-link-deleted-bundle.unit.test.ts @@ -0,0 +1,88 @@ +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' + +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', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('filters deleted bundles before creating download URLs', async () => { + 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 0b063dc938..4359bc24ab 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,129 @@ 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() + }) + + 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 }] }) + + 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('FROM public.manifest'), + ['test-org', 'test-app', filePath], + ) + expect(pgClientMock.query.mock.calls[0]?.[0]).not.toContain('unnest(manifest)') + 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() + }) }) 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 })