fix(security): block r2-direct manifest jsonb poison path

.github/workflows/coderabbit-bot-trigger.yml

@@ -11,6 +11,8 @@ concurrency:
   group: coderabbit-bot-trigger-${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: false
 
+# pull-requests: write is required: GITHUB_TOKEN gets 403 on issues.createComment
+# for PRs when only pull-requests: read is granted (see run 32975845301).
 permissions:
   issues: write
   pull-requests: write
@@ -36,7 +38,9 @@ jobs:
             }
 
             for (const pr of prs) {
-              const headSha = pr.head.sha
+              const headSha = context.eventName === 'push'
+                ? context.sha
+                : pr.head.sha
               const reviews = await github.paginate(github.rest.pulls.listReviews, {
                 owner,
                 repo,
@@ -46,11 +50,12 @@ jobs:
                 .filter(review => review.user?.login === 'coderabbitai[bot]')
                 .pop()
 
+              // Skip only when CodeRabbit already approved this SHA (COMMENTED does not clear CHANGES_REQUESTED).
               if (
                 latestCodeRabbitReview?.commit_id === headSha
-                && latestCodeRabbitReview.state !== 'CHANGES_REQUESTED'
+                && latestCodeRabbitReview.state === 'APPROVED'
               ) {
-                console.log(`CodeRabbit already reviewed PR #${pr.number} at ${headSha}`)
+                console.log(`CodeRabbit already approved PR #${pr.number} at ${headSha}`)
                 continue
               }
 
@@ -60,13 +65,22 @@ jobs:
                 repo,
                 issue_number: pr.number,
               })
-              if (comments.some(comment =>
+              const alreadyTriggered = comments.some(comment =>
                 comment.user?.login === 'github-actions[bot]'
                 && comment.body?.includes(marker)
-              )) {
-                console.log(`CodeRabbit trigger already posted for PR #${pr.number}`)
+              )
+              // Skip only when a prior trigger already produced an approval on headSha.
+              if (
+                alreadyTriggered
+                && latestCodeRabbitReview?.commit_id === headSha
+                && latestCodeRabbitReview.state === 'APPROVED'
+              ) {
+                console.log(`CodeRabbit trigger already completed for PR #${pr.number} at ${headSha}`)
                 continue
               }
+              if (alreadyTriggered) {
+                console.log(`Retrying CodeRabbit trigger for PR #${pr.number}; prior trigger had no review on ${headSha}`)
+              }
 
               await github.rest.issues.createComment({
                 owner,

supabase/functions/_backend/triggers/on_version_update.ts

@@ -259,10 +259,12 @@ async function updateIt(c: Context, record: Database['public']['Tables']['app_ve
     }
   }
 
-  // Handle manifest entries (reload when the queue payload omitted the jsonb column)
-  const recordWithManifest = await ensureVersionManifest(c, record)
-  if (recordWithManifest.manifest)
-    await handleManifest(c, recordWithManifest)
+  // In-progress r2-direct uploads must use POST /private/set_manifest instead.
+  if (record.storage_provider !== 'r2-direct') {
+    const recordWithManifest = await ensureVersionManifest(c, record)
+    if (recordWithManifest.manifest)
+      await handleManifest(c, recordWithManifest)
+  }
 
   return c.json(BRES)
 }

supabase/migrations/20260826101500_block_r2_direct_manifest_jsonb_writes.sql

@@ -0,0 +1,267 @@
+-- Block PostgREST writes to app_versions.manifest while a bundle is still
+-- in-progress (storage_provider = r2-direct). Legitimate delta uploads use
+-- POST /private/set_manifest, which inserts into public.manifest directly.
+--
+-- Execution profile for app_version_manifest_jsonb_unmigrated:
+-- - Called from check_encrypted_bundle_on_insert on public.app_versions
+--   INSERT/UPDATE when manifest jsonb is cleared or compared (at most once
+--   per affected row).
+-- - Roles: service_role only; not exposed to anon/authenticated PostgREST.
+-- - Frequency: console/API app_versions writes; not plugin /updates hot path.
+-- - Cardinality: p_manifest is bounded by bundle file count (thousands of
+--   entries at most); each entry probes public.manifest via app_version_id.
+-- - Indexes: idx_manifest_app_version_id on (app_version_id); per-entry
+--   s3_path/file_hash filter on the index-scanned row set.
+-- - Worst case: Nested Loop from unnest(p_manifest) to Index Scan on
+--   idx_manifest_app_version_id with s3_path/file_hash filters. Bounded by
+--   manifest entry count, not table cardinality.
+
+CREATE OR REPLACE FUNCTION public.app_version_manifest_jsonb_unmigrated(
+  p_version_id bigint,
+  p_manifest public.manifest_entry[]
+)
+RETURNS boolean
+LANGUAGE sql
+STABLE
+SET search_path = ''
+AS $$
+  SELECT EXISTS (
+    SELECT 1
+    FROM pg_catalog.unnest(p_manifest) AS entry(file_name, s3_path, file_hash)
+    WHERE NOT EXISTS (
+      SELECT 1
+      FROM public.manifest AS m
+      WHERE m.app_version_id = p_version_id
+        AND m.s3_path = entry.s3_path
+        AND m.file_hash = entry.file_hash
+    )
+  );
+$$;
+
+ALTER FUNCTION public.app_version_manifest_jsonb_unmigrated(
+  bigint, public.manifest_entry[]
+) OWNER TO postgres;
+REVOKE ALL ON FUNCTION public.app_version_manifest_jsonb_unmigrated(
+  bigint, public.manifest_entry[]
+) FROM PUBLIC;
+GRANT ALL ON FUNCTION public.app_version_manifest_jsonb_unmigrated(
+  bigint, public.manifest_entry[]
+) TO service_role;
+
+CREATE OR REPLACE FUNCTION "public"."check_encrypted_bundle_on_insert"() RETURNS "trigger"
+    LANGUAGE "plpgsql" SECURITY DEFINER
+    SET "search_path" TO ''
+    AS $$
+DECLARE
+  org_id uuid;
+  org_enforcing boolean;
+  org_required_key varchar(21);
+  bundle_is_encrypted boolean;
+  bundle_key_id varchar(20);
+  bundle_was_ready boolean;
+  r2_direct_manifest_err constant text :=
+    'r2_direct_manifest_jsonb: Use POST /private/set_manifest for in-progress '
+    || 'r2-direct uploads instead of app_versions.manifest jsonb.';
+BEGIN
+  IF TG_OP = 'INSERT'
+    AND NEW.storage_provider = 'r2-direct'
+    AND NEW.manifest IS NOT NULL
+  THEN
+    PERFORM public.pg_log('deny: BUNDLE_CONTENT_LOCKED_TRIGGER',
+      pg_catalog.jsonb_build_object(
+        'org_id', NEW.owner_org,
+        'app_id', NEW.app_id,
+        'version_name', NEW.name,
+        'user_id', NEW.user_id,
+        'old_storage_provider', NULL,
+        'new_storage_provider', NEW.storage_provider,
+        'reason', 'r2_direct_manifest_jsonb'
+      ));
+    RAISE EXCEPTION '%', r2_direct_manifest_err;
+  END IF;
+
+  IF TG_OP = 'UPDATE' THEN
+    IF pg_catalog.current_setting('capgo.reclaim_manifest_null', true) = 'on'
+      AND NEW.manifest IS NULL
+      AND OLD.manifest IS NOT NULL
+      AND NEW.native_packages IS NOT DISTINCT FROM OLD.native_packages
+      AND NEW.name IS NOT DISTINCT FROM OLD.name
+      AND NEW.app_id IS NOT DISTINCT FROM OLD.app_id
+      AND NEW.session_key IS NOT DISTINCT FROM OLD.session_key
+      AND NEW.key_id IS NOT DISTINCT FROM OLD.key_id
+      AND NEW.storage_provider IS NOT DISTINCT FROM OLD.storage_provider
+      AND NEW.r2_path IS NOT DISTINCT FROM OLD.r2_path
+      AND NEW.external_url IS NOT DISTINCT FROM OLD.external_url
+      AND NEW.checksum IS NOT DISTINCT FROM OLD.checksum
+    THEN
+      RETURN NEW;
+    END IF;
+
+    IF NEW.manifest IS NULL
+      AND OLD.manifest IS NOT NULL
+      AND public.app_version_manifest_jsonb_unmigrated(OLD.id, OLD.manifest)
+    THEN
+      RAISE EXCEPTION '%',
+        'bundle_manifest_not_migrated: Cannot clear app_versions.manifest '
+        || 'until every entry exists in public.manifest.';
+    END IF;
+
+    bundle_was_ready := OLD.storage_provider IS DISTINCT FROM 'r2-direct';
+
+    IF bundle_was_ready
+      AND (
+        NEW.name IS DISTINCT FROM OLD.name
+        OR NEW.app_id IS DISTINCT FROM OLD.app_id
+        OR NEW.session_key IS DISTINCT FROM OLD.session_key
+        OR NEW.key_id IS DISTINCT FROM OLD.key_id
+        OR NEW.storage_provider IS DISTINCT FROM OLD.storage_provider
+        OR NEW.r2_path IS DISTINCT FROM OLD.r2_path
+        OR NEW.external_url IS DISTINCT FROM OLD.external_url
+        OR NEW.checksum IS DISTINCT FROM OLD.checksum
+        OR (NEW.manifest IS DISTINCT FROM OLD.manifest AND NEW.manifest IS NOT NULL)
+        OR (
+          NEW.manifest IS NULL
+          AND OLD.manifest IS NOT NULL
+          AND public.app_version_manifest_jsonb_unmigrated(OLD.id, OLD.manifest)
+        )
+        OR NEW.native_packages IS DISTINCT FROM OLD.native_packages
+      )
+    THEN
+      PERFORM public.pg_log('deny: BUNDLE_CONTENT_LOCKED_TRIGGER',
+        pg_catalog.jsonb_build_object(
+          'org_id', OLD.owner_org,
+          'app_id', OLD.app_id,
+          'version_name', OLD.name,
+          'user_id', OLD.user_id,
+          'old_storage_provider', OLD.storage_provider,
+          'new_storage_provider', NEW.storage_provider,
+          'reason', 'bundle_ready'
+        ));
+      RAISE EXCEPTION '%',
+        'bundle_already_ready: Bundle content cannot be changed '
+        || 'after upload is complete. Upload a new bundle instead.';
+    END IF;
+
+    -- In-progress r2-direct uploads must use POST /private/set_manifest.
+    -- Block any non-null manifest jsonb write, including r2-direct -> r2 finalize
+    -- requests that try to smuggle manifest rows through on_version_update.
+    IF OLD.storage_provider = 'r2-direct'
+      AND NEW.manifest IS DISTINCT FROM OLD.manifest
+      AND NEW.manifest IS NOT NULL
+    THEN
+      PERFORM public.pg_log('deny: BUNDLE_CONTENT_LOCKED_TRIGGER',
+        pg_catalog.jsonb_build_object(
+          'org_id', OLD.owner_org,
+          'app_id', OLD.app_id,
+          'version_name', OLD.name,
+          'user_id', OLD.user_id,
+          'old_storage_provider', OLD.storage_provider,
+          'new_storage_provider', NEW.storage_provider,
+          'reason', 'r2_direct_manifest_jsonb'
+        ));
+      RAISE EXCEPTION '%', r2_direct_manifest_err;
+    END IF;
+  END IF;
+
+  IF TG_OP = 'UPDATE'
+    AND NEW.session_key IS NOT DISTINCT FROM OLD.session_key
+    AND NEW.key_id IS NOT DISTINCT FROM OLD.key_id
+    AND NEW.name IS NOT DISTINCT FROM OLD.name
+    AND NEW.app_id IS NOT DISTINCT FROM OLD.app_id
+    AND NEW.storage_provider IS NOT DISTINCT FROM OLD.storage_provider
+    AND NEW.r2_path IS NOT DISTINCT FROM OLD.r2_path
+    AND NEW.external_url IS NOT DISTINCT FROM OLD.external_url
+    AND NEW.checksum IS NOT DISTINCT FROM OLD.checksum
+    AND NEW.native_packages IS NOT DISTINCT FROM OLD.native_packages
+    AND (
+      NEW.manifest IS NOT DISTINCT FROM OLD.manifest
+      OR (
+        NEW.manifest IS NULL
+        AND OLD.manifest IS NOT NULL
+        AND NOT public.app_version_manifest_jsonb_unmigrated(OLD.id, OLD.manifest)
+      )
+    )
+  THEN
+    RETURN NEW;
+  END IF;
+
+  SELECT apps.owner_org INTO org_id
+  FROM public.apps
+  WHERE apps.app_id = NEW.app_id;
+
+  IF org_id IS NULL THEN
+    org_id := NEW.owner_org;
+  END IF;
+
+  IF org_id IS NULL THEN
+    RETURN NEW;
+  END IF;
+
+  SELECT enforce_encrypted_bundles, required_encryption_key
+  INTO org_enforcing, org_required_key
+  FROM public.orgs
+  WHERE id = org_id;
+
+  IF org_enforcing IS NULL OR org_enforcing = false THEN
+    RETURN NEW;
+  END IF;
+
+  bundle_is_encrypted := public.is_bundle_encrypted(NEW.session_key);
+  bundle_key_id := NULLIF(pg_catalog.btrim(NEW.key_id), '')::varchar(20);
+
+  IF NOT bundle_is_encrypted THEN
+    PERFORM public.pg_log('deny: ORG_REQUIRES_ENCRYPTED_BUNDLES_TRIGGER',
+      pg_catalog.jsonb_build_object(
+        'org_id', org_id,
+        'app_id', NEW.app_id,
+        'version_name', NEW.name,
+        'user_id', NEW.user_id,
+        'reason', 'not_encrypted'
+      ));
+    RAISE EXCEPTION '%',
+      'encryption_required: This organization requires all bundles to be '
+      || 'encrypted. Please upload an encrypted bundle with a session_key.';
+  END IF;
+
+  IF org_required_key IS NOT NULL AND org_required_key <> '' THEN
+    IF bundle_key_id IS NULL THEN
+      PERFORM public.pg_log('deny: ORG_REQUIRES_SPECIFIC_ENCRYPTION_KEY_TRIGGER',
+        pg_catalog.jsonb_build_object(
+          'org_id', org_id,
+          'app_id', NEW.app_id,
+          'version_name', NEW.name,
+          'user_id', NEW.user_id,
+          'required_key', org_required_key,
+          'bundle_key_id', bundle_key_id,
+          'reason', 'missing_key_id'
+        ));
+      RAISE EXCEPTION '%',
+        'encryption_key_required: This organization requires bundles to be '
+        || 'encrypted with a specific key. The uploaded bundle does not have '
+        || 'a key_id.';
+    END IF;
+
+    IF NOT (
+      bundle_key_id = pg_catalog.left(org_required_key, 20)
+      OR pg_catalog.left(bundle_key_id, pg_catalog.length(org_required_key)) = org_required_key
+    ) THEN
+      PERFORM public.pg_log('deny: ORG_REQUIRES_SPECIFIC_ENCRYPTION_KEY_TRIGGER',
+        pg_catalog.jsonb_build_object(
+          'org_id', org_id,
+          'app_id', NEW.app_id,
+          'version_name', NEW.name,
+          'user_id', NEW.user_id,
+          'required_key', org_required_key,
+          'bundle_key_id', bundle_key_id,
+          'reason', 'key_mismatch'
+        ));
+      RAISE EXCEPTION '%',
+        'encryption_key_mismatch: This organization requires bundles to be '
+        || 'encrypted with a specific key. The uploaded bundle was encrypted '
+        || 'with a different key.';
+    END IF;
+  END IF;
+
+  RETURN NEW;
+END;
+$$;