From 36d0ab44df8c1f8f208a85366cd5e0f4abde4491 Mon Sep 17 00:00:00 2001 From: "posthog-eu[bot]" <226701856+posthog-eu[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 19:08:53 +0000 Subject: [PATCH] fix(upload): make outdated-CLI bundle upload failure actionable Old @capgo/cli (< MIN_CLI_VERSION) finalizes uploads by writing app_versions.manifest jsonb, which the check_encrypted_bundle_on_insert trigger blocks. The user saw only a raw Postgres P0001 that named an internal endpoint and gave no way to recover. - Rewrite the trigger's r2_direct_manifest_jsonb message to tell the user to update the CLI. The old CLI finalizes via direct PostgREST, so the trigger is the only chokepoint every upload path passes through; guard behavior and the pg_log reason are unchanged. - Enforce MIN_CLI_VERSION server-side on the presigned upload-link request so the doomed upload is rejected before any files transfer. TUS uploads still fail at the trigger, which now carries the same upgrade message. Generated-By: PostHog Desktop Task-Id: eb8ace10-e0bd-405f-b31f-a7e3efee2497 --- .../functions/_backend/private/upload_link.ts | 3 + .../functions/_backend/utils/cliMinVersion.ts | 38 +++ ...explain_r2_direct_manifest_cli_upgrade.sql | 289 ++++++++++++++++++ ...73_test_block_r2_direct_manifest_jsonb.sql | 12 +- tests/manifest-poison-guard.test.ts | 4 +- tests/private-error-cases.test.ts | 31 ++ 6 files changed, 369 insertions(+), 8 deletions(-) create mode 100644 supabase/migrations/20260914185856_explain_r2_direct_manifest_cli_upgrade.sql diff --git a/supabase/functions/_backend/private/upload_link.ts b/supabase/functions/_backend/private/upload_link.ts index 3815da6f8c..e76d62d04c 100644 --- a/supabase/functions/_backend/private/upload_link.ts +++ b/supabase/functions/_backend/private/upload_link.ts @@ -1,6 +1,7 @@ import type { MiddlewareKeyVariables } from '../utils/hono.ts' import type { Database } from '../utils/supabase.types.ts' import { Hono } from 'hono/tiny' +import { assertUploadCliVersionSupported } from '../utils/cliMinVersion.ts' import { parseBody, quickError, simpleError } from '../utils/hono.ts' import { middlewareKey } from '../utils/hono_middleware.ts' import { cloudlog } from '../utils/logging.ts' @@ -18,6 +19,8 @@ interface DataUpload { export const app = new Hono() app.post('/', middlewareKey(), async (c) => { + // Reject uploads from CLIs too old to finalize correctly before any files transfer. + assertUploadCliVersionSupported(c) const body = await parseBody(c) cloudlog({ requestId: c.get('requestId'), message: 'post upload link body', body }) const apikey = c.get('apikey') as Database['public']['Tables']['apikeys']['Row'] diff --git a/supabase/functions/_backend/utils/cliMinVersion.ts b/supabase/functions/_backend/utils/cliMinVersion.ts index 7b1f11aaf7..405355a1f8 100644 --- a/supabase/functions/_backend/utils/cliMinVersion.ts +++ b/supabase/functions/_backend/utils/cliMinVersion.ts @@ -1,3 +1,7 @@ +import type { Context } from 'hono' +import { canParse, lessThan, parse } from '@std/semver' +import { quickError } from './hono.ts' + /** * Minimum @capgo/cli version Capgo still supports. * @@ -14,3 +18,37 @@ export const MIN_CLI_VERSION = '8.42.3' export const MIN_CLI_VERSION_REASON = 'Oldest CLI version Capgo still tests against the current API. Older CLIs can fail on uploads, auth, or encryption.' + +// Parsed once at import. A malformed MIN_CLI_VERSION throws here at deploy time +// rather than silently disabling the gate on every request. +const MIN_CLI_SEMVER = parse(MIN_CLI_VERSION) + +/** + * Reject a bundle upload from a @capgo/cli older than MIN_CLI_VERSION. + * + * The floor is otherwise advisory: GET /private/config publishes it and the CLI + * enforces it, so a CLI too old to read that value never checks it. Such CLIs + * finalize uploads by writing app_versions.manifest jsonb, which the + * check_encrypted_bundle_on_insert trigger blocks after the files already + * reached R2. Calling this from the presigned upload-link request rejects that + * doomed upload up front, with a message the user can act on. + * + * This is a best-effort early exit, not the safety net. Uploads that skip the + * presigned link (TUS) are not gated here; they still fail at the trigger, which + * now carries the same upgrade message. Correctness comes from the trigger, so + * requests without a parseable `x-cli-version` header are left alone here to + * spare non-CLI API clients and self-hosted tooling. + */ +export function assertUploadCliVersionSupported(c: Context) { + const cliVersion = c.req.header('x-cli-version')?.trim() + if (!cliVersion || !canParse(cliVersion)) + return + if (!lessThan(parse(cliVersion), MIN_CLI_SEMVER)) + return + quickError( + 400, + 'cli_version_too_old', + `Your @capgo/cli (${cliVersion}) is too old to upload bundles. Update it: run npx @capgo/cli@latest, then upload again.`, + { minCliVersion: MIN_CLI_VERSION, cliVersion }, + ) +} diff --git a/supabase/migrations/20260914185856_explain_r2_direct_manifest_cli_upgrade.sql b/supabase/migrations/20260914185856_explain_r2_direct_manifest_cli_upgrade.sql new file mode 100644 index 0000000000..484d3f524e --- /dev/null +++ b/supabase/migrations/20260914185856_explain_r2_direct_manifest_cli_upgrade.sql @@ -0,0 +1,289 @@ +-- Make the r2_direct_manifest_jsonb guard message actionable for CLI users. +-- +-- The guard in check_encrypted_bundle_on_insert (20260826101500, +-- 20260908120000) blocks app_versions.manifest jsonb writes on in-progress +-- r2-direct uploads, including the r2-direct -> r2 finalize. @capgo/cli older +-- than MIN_CLI_VERSION (8.42.3) finalizes that way, so its uploads hit this +-- exception after the files already reached R2. The CLI prints the raw +-- exception text, which named an internal endpoint (POST /private/set_manifest) +-- and gave the user no way to recover. +-- +-- Only the raised message changes here: it now tells the user to update the +-- CLI, which is the real fix. The guard behavior, the pg_log reason +-- ('r2_direct_manifest_jsonb'), and every other branch stay identical to +-- 20260908120000. + +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_upload_complete boolean; + bundle_identity_locked boolean; + is_r2_direct_finalize boolean; + r2_direct_manifest_err constant text := + 'r2_direct_manifest_jsonb: Your @capgo/cli is too old to finish this upload. ' + || 'Update it: run npx @capgo/cli@latest, then upload again.'; +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_upload_complete := OLD.storage_provider IS DISTINCT FROM 'r2-direct'; + + IF bundle_upload_complete + 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_upload_complete' + )); + 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. + 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; + + -- GHSA-5rg9-rhwj-wj76: CLI/TUS creates r2-direct rows with checksum before + -- finalize. Lock identity fields after first set (checksum/session_key/ + -- key_id); still allow r2_path writes and the one-shot finalize + -- (r2-direct -> r2). Blank-checksum in-progress rows stay writable for + -- upload completion; channel linkage is not the freeze gate. + -- r2_path stays mutable while storage_provider = r2-direct (even when + -- channel-linked) so finalize can set the object key; only checksum, + -- session_key, and key_id are identity-locked here. + IF OLD.storage_provider = 'r2-direct' THEN + bundle_identity_locked := ( + NULLIF(BTRIM(COALESCE(OLD.checksum, '')), '') IS NOT NULL + OR NULLIF(BTRIM(COALESCE(OLD.session_key, '')), '') IS NOT NULL + OR NULLIF(BTRIM(COALESCE(OLD.key_id, '')), '') IS NOT NULL + ); + + is_r2_direct_finalize := ( + NEW.storage_provider = 'r2' + 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.checksum IS NOT DISTINCT FROM OLD.checksum + AND NEW.external_url IS NOT DISTINCT FROM OLD.external_url + AND NEW.native_packages IS NOT DISTINCT FROM OLD.native_packages + ); + + IF bundle_identity_locked + 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.checksum IS DISTINCT FROM OLD.checksum + OR NEW.external_url IS DISTINCT FROM OLD.external_url + OR NEW.native_packages IS DISTINCT FROM OLD.native_packages + OR ( + NEW.storage_provider IS DISTINCT FROM OLD.storage_provider + AND NOT is_r2_direct_finalize + ) + ) + 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_identity_locked' + )); + RAISE EXCEPTION '%', + 'bundle_identity_locked: Bundle identity fields cannot be changed ' + || 'after checksum, session_key, or key_id are first set during upload.'; + END IF; + 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; +$$; diff --git a/supabase/tests/73_test_block_r2_direct_manifest_jsonb.sql b/supabase/tests/73_test_block_r2_direct_manifest_jsonb.sql index f71971238a..4c902d1eb8 100644 --- a/supabase/tests/73_test_block_r2_direct_manifest_jsonb.sql +++ b/supabase/tests/73_test_block_r2_direct_manifest_jsonb.sql @@ -89,8 +89,8 @@ SELECT throws_ok( ) $sql$, 'P0001', - 'r2_direct_manifest_jsonb: Use POST /private/set_manifest for in-progress ' - 'r2-direct uploads instead of app_versions.manifest jsonb.', + 'r2_direct_manifest_jsonb: Your @capgo/cli is too old to finish this upload. ' + 'Update it: run npx @capgo/cli@latest, then upload again.', 'in-progress r2-direct cannot INSERT manifest jsonb' ); @@ -108,8 +108,8 @@ SELECT throws_ok( AND name = '1.0.0-in-progress' $sql$, 'P0001', - 'r2_direct_manifest_jsonb: Use POST /private/set_manifest for in-progress ' - 'r2-direct uploads instead of app_versions.manifest jsonb.', + 'r2_direct_manifest_jsonb: Your @capgo/cli is too old to finish this upload. ' + 'Update it: run npx @capgo/cli@latest, then upload again.', 'in-progress r2-direct cannot UPDATE manifest jsonb' ); @@ -130,8 +130,8 @@ SELECT throws_ok( AND name = '1.0.0-in-progress' $sql$, 'P0001', - 'r2_direct_manifest_jsonb: Use POST /private/set_manifest for in-progress ' - 'r2-direct uploads instead of app_versions.manifest jsonb.', + 'r2_direct_manifest_jsonb: Your @capgo/cli is too old to finish this upload. ' + 'Update it: run npx @capgo/cli@latest, then upload again.', 'r2-direct cannot set manifest jsonb while finalizing to r2' ); diff --git a/tests/manifest-poison-guard.test.ts b/tests/manifest-poison-guard.test.ts index 6be61b332e..cd8a229011 100644 --- a/tests/manifest-poison-guard.test.ts +++ b/tests/manifest-poison-guard.test.ts @@ -14,7 +14,7 @@ import { const APP_ID = 'com.demo.app' const R2_DIRECT_MANIFEST_ERR = 'r2_direct_manifest_jsonb' -const SET_MANIFEST_PATH = '/private/set_manifest' +const CLI_UPGRADE_HINT = '@capgo/cli@latest' function poisonManifestEntries(ownerOrg: string, versionName: string) { const prefix = `orgs/${ownerOrg}/apps/${APP_ID}/delta` @@ -85,7 +85,7 @@ describe('manifest poison guard', () => { expect(response.status).toBeGreaterThanOrEqual(400) const body = await response.text() expect(body).toContain(R2_DIRECT_MANIFEST_ERR) - expect(body).toContain(SET_MANIFEST_PATH) + expect(body).toContain(CLI_UPGRADE_HINT) const { data: versionRow, error: versionError } = await adminClient .from('app_versions') diff --git a/tests/private-error-cases.test.ts b/tests/private-error-cases.test.ts index a42e42b194..2ef92d0314 100644 --- a/tests/private-error-cases.test.ts +++ b/tests/private-error-cases.test.ts @@ -279,6 +279,37 @@ describe('[POST] /private/upload_link - Error Cases', () => { const data = await response.json() as { error: string } expect(data.error).toBe('error_version_not_found') }) + + it('rejects a @capgo/cli older than the minimum before any upload work', async () => { + const response = await fetch(getEndpointUrl('/private/upload_link'), { + method: 'POST', + headers: { ...headers, 'x-cli-version': '8.0.0' }, + body: JSON.stringify({ + app_id: APPNAME, + version: '1.0.0', + }), + }) + + expect(response.status).toBe(400) + const data = await response.json() as { error: string } + expect(data.error).toBe('cli_version_too_old') + }) + + it('does not gate requests without a parseable cli version header', async () => { + const response = await fetch(getEndpointUrl('/private/upload_link'), { + method: 'POST', + headers: { ...headers, 'x-cli-version': 'not-a-version' }, + body: JSON.stringify({ + app_id: APPNAME, + version: '1.0.0', + }), + }) + + // The gate is skipped, so the request falls through to the normal flow. + expect(response.status).toBe(404) + const data = await response.json() as { error: string } + expect(data.error).toBe('error_version_not_found') + }) }) describe('[POST] /private/download_link - Error Cases', () => {