Block deleted bundle reads from cache

supabase/functions/_backend/files/files.ts

@@ -317,6 +317,61 @@ async function saveBandwidthUsage(c: Context, fileSize: number | null | undefine
   }
 }
 
+async function assertReadableAppScopedAttachment(c: Context, fileId: unknown): Promise<void> {
+  const scopedPath = parseAppScopedAttachmentPath(fileId)
+  if (scopedPath?.kind === 'invalid_scoped') {
+    quickError(404, 'not_found', 'Not found')
+  }
+  if (!scopedPath) {
+    return
+  }
+
+  // Attachment reads must use the primary to avoid replica lag serving deleted-app files.
+  const pgClient = getPgClient(c, false)
+  const drizzleClient = getDrizzleClient(pgClient)
+
+  try {
+    const app = await getAppByAppIdPg(c, scopedPath.app_id, drizzleClient)
+    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
+            )
+            OR EXISTS (
+              SELECT 1
+              FROM public.deleted_app_version_manifest_paths dmp
+              WHERE dmp.app_version_id = public.app_versions.id
+                AND dmp.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)
+  }
+}
+
 async function getSupabaseStorageResponse(c: Context, fileId: string): Promise<Response> {
   const { data: signedUrlData, error: signedUrlError } = await supabaseAdmin(c).storage.from('capgo').createSignedUrl(fileId, 60)
 
@@ -376,9 +431,9 @@ async function getSupabaseStorageResponse(c: Context, fileId: string): Promise<R
 
 async function getHandler(c: Context): Promise<Response> {
   const fileId = c.get('fileId')
-  // It is imperative that files are read without any database read to avoid bottlenecks and keep file downloads highly available, especially under heavy load.
-  // This was designed that way, and access to a file that is going to be deleted is not important compared to download availability.
-  // Do not add DB or R2 checks before serving the file; if the file is missing in R2, a 404 is expected.
+  await assertReadableAppScopedAttachment(c, fileId)
+  // Keep normal reads cache-first after the deletion guard. The guard must run
+  // before cache/R2 so stale deleted app-scoped attachments cannot be restored.
 
   cloudlog({ requestId: c.get('requestId'), message: 'getHandler files', fileId })
   if (getRuntimeKey() !== 'workerd') {

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

supabase/functions/_backend/triggers/on_version_update.ts

@@ -208,6 +208,29 @@ async function deleteManifest(c: Context, record: Database['public']['Tables']['
 
     if (manifestEntries && manifestEntries.length > 0) {
       const manifestCount = manifestEntries.length
+      const ownerOrg = await resolveOwnerOrg(c, record)
+      if (ownerOrg) {
+        const tombstonePgClient = getPgClient(c, false)
+        try {
+          await tombstonePgClient.query(
+            `
+              INSERT INTO public.deleted_app_version_manifest_paths
+                (app_version_id, owner_org, app_id, s3_path)
+              SELECT $1, $2, $3, m.s3_path
+              FROM public.manifest m
+              WHERE m.app_version_id = $1
+              ON CONFLICT (app_version_id, s3_path) DO NOTHING
+            `,
+            [record.id, ownerOrg, record.app_id],
+          )
+        }
+        catch (error) {
+          cloudlog({ requestId: c.get('requestId'), message: 'error storing deleted manifest path tombstones', error, id: record.id })
+        }
+        finally {
+          await closeClient(c, tombstonePgClient)
+        }
+      }
 
       // Delete each file from S3
       const promisesDeleteS3 = []

supabase/migrations/20260511012637_add_app_versions_deleted_r2_path_index.sql

@@ -0,0 +1,18 @@
+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");
+
+CREATE TABLE IF NOT EXISTS "public"."deleted_app_version_manifest_paths" (
+  "app_version_id" bigint NOT NULL REFERENCES "public"."app_versions" ("id") ON DELETE CASCADE,
+  "owner_org" uuid NOT NULL,
+  "app_id" character varying NOT NULL,
+  "s3_path" character varying NOT NULL,
+  "created_at" timestamp with time zone DEFAULT now() NOT NULL,
+  PRIMARY KEY ("app_version_id", "s3_path")
+);
+
+CREATE INDEX IF NOT EXISTS "idx_deleted_app_version_manifest_paths_lookup"
+ON "public"."deleted_app_version_manifest_paths" USING "btree" ("owner_org", "app_id", "s3_path");

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<typeof import('../supabase/functions/_backend/utils/hono.ts')>()
+  return {
+    ...actual,
+    middlewareAuth: async (c: any, next: () => Promise<void>) => {
+      c.set('authorization', 'Bearer test-token')
+      c.set('auth', { userId: 'test-user' })
+      await next()
+    },
+    useCors: async (_c: any, next: () => Promise<void>) => {
+      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()
+  })
+})