From b5024dfd218991f6bed491dd1acabd7f6cc6e259 Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Mon, 18 May 2026 17:12:03 +0200 Subject: [PATCH 1/2] fix(onboarding): complete app onboarding after first upload --- cli/src/init/command.ts | 22 +- ...complete_onboarding_after_first_upload.sql | 245 ++++++++++++++++++ tests/onboarding-first-upload.test.ts | 179 +++++++++++++ ...security-definer-execute-hardening.test.ts | 3 + 4 files changed, 437 insertions(+), 12 deletions(-) create mode 100644 supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql create mode 100644 tests/onboarding-first-upload.test.ts diff --git a/cli/src/init/command.ts b/cli/src/init/command.ts index 7f6026d46c..0b95997d5a 100644 --- a/cli/src/init/command.ts +++ b/cli/src/init/command.ts @@ -1623,16 +1623,7 @@ async function maybeReusePendingOnboardingApp( await syncPendingAppIdToCapacitorConfig(selectedAppId) } - const cleanupSpinner = pSpinner() - cleanupSpinner.start(`Preparing ${selectedAppId} for real onboarding`) - try { - await completePendingOnboardingApp(supabase, organization.gid, selectedAppId) - cleanupSpinner.stop('Pending onboarding app prepared ✅') - } - catch (error) { - cleanupSpinner.stop('Could not prepare pending onboarding app ❌') - throw error - } + await maybeCompletePendingOnboardingAppForLegacyBackend(supabase, organization, selectedAppId) await markStep(organization.gid, apikey, 'add-app', selectedAppId) @@ -1789,11 +1780,18 @@ async function checkPrerequisitesStep(orgId: string, apikey: string) { type ExistingAppConflictResolution = 'use-existing' | 'recreate' | 'choose-different' | 'not-owned' -async function completeExistingAppPendingOnboarding( +function shouldCompleteOnboardingInCliForLegacyBackend() { + return env.CAPGO_CLI_COMPLETE_ONBOARDING === 'true' +} + +async function maybeCompletePendingOnboardingAppForLegacyBackend( supabase: Awaited>, organization: Organization, appId: string, ) { + if (!shouldCompleteOnboardingInCliForLegacyBackend()) + return + const s = pSpinner() s.start(`Preparing existing app ${appId}`) try { @@ -1829,7 +1827,7 @@ async function resolveExistingAppConflict( if (useExistingApp === true) { if (existingApp.need_onboarding) - await completeExistingAppPendingOnboarding(supabase, organization, appId) + await maybeCompletePendingOnboardingAppForLegacyBackend(supabase, organization, appId) await saveAppIdToCapacitorConfig(appId) return 'use-existing' diff --git a/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql b/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql new file mode 100644 index 0000000000..870b8e18c9 --- /dev/null +++ b/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql @@ -0,0 +1,245 @@ +CREATE OR REPLACE FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid", "p_preserve_app_version_id" bigint) +RETURNS void +LANGUAGE "plpgsql" +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_app_id text; + v_owner_org uuid; + v_last_version text; + v_manifest_bundle_count bigint := 0; + v_channel_device_count bigint := 0; +BEGIN + SELECT app_id, owner_org + INTO v_app_id, v_owner_org + FROM public.apps + WHERE id = p_app_uuid; + + IF v_app_id IS NULL THEN + RETURN; + END IF; + + DELETE FROM public.channel_devices + WHERE app_id = v_app_id + AND ( + p_preserve_app_version_id IS NULL + OR NOT EXISTS ( + SELECT 1 + FROM public.channels + WHERE channels.id = channel_devices.channel_id + AND channels.version = p_preserve_app_version_id + ) + ); + + DELETE FROM public.deploy_history + WHERE app_id = v_app_id + AND ( + p_preserve_app_version_id IS NULL + OR version_id IS DISTINCT FROM p_preserve_app_version_id + ); + + DELETE FROM public.channels + WHERE app_id = v_app_id + AND ( + p_preserve_app_version_id IS NULL + OR version IS DISTINCT FROM p_preserve_app_version_id + ); + + DELETE FROM public.devices + WHERE app_id = v_app_id; + + DELETE FROM public.app_versions_meta + WHERE app_id = v_app_id + AND ( + p_preserve_app_version_id IS NULL + OR id IS DISTINCT FROM p_preserve_app_version_id + ); + + DELETE FROM public.daily_version + WHERE app_id = v_app_id; + + DELETE FROM public.daily_bandwidth + WHERE app_id = v_app_id; + + DELETE FROM public.daily_storage + WHERE app_id = v_app_id; + + DELETE FROM public.daily_mau + WHERE app_id = v_app_id; + + DELETE FROM public.daily_build_time + WHERE app_id = v_app_id; + + DELETE FROM public.build_requests + WHERE app_id = v_app_id; + + DELETE FROM public.app_versions + WHERE app_id = v_app_id + AND name NOT IN ('builtin', 'unknown') + AND ( + p_preserve_app_version_id IS NULL + OR id IS DISTINCT FROM p_preserve_app_version_id + ); + + INSERT INTO public.app_versions ( + owner_org, + deleted, + name, + app_id, + created_at + ) + VALUES + (v_owner_org, true, 'builtin', v_app_id, now()), + (v_owner_org, true, 'unknown', v_app_id, now()) + ON CONFLICT (name, app_id) DO UPDATE + SET + owner_org = EXCLUDED.owner_org, + deleted = true, + deleted_at = NULL, + checksum = NULL, + session_key = NULL, + r2_path = NULL, + link = NULL, + comment = NULL, + updated_at = now(); + + IF p_preserve_app_version_id IS NOT NULL THEN + SELECT name, CASE WHEN manifest_count > 0 THEN 1 ELSE 0 END + INTO v_last_version, v_manifest_bundle_count + FROM public.app_versions + WHERE id = p_preserve_app_version_id + AND app_id = v_app_id + AND deleted IS FALSE; + + SELECT COUNT(*)::bigint + INTO v_channel_device_count + FROM public.channel_devices + INNER JOIN public.channels + ON channels.id = channel_devices.channel_id + WHERE channel_devices.app_id = v_app_id + AND channels.version = p_preserve_app_version_id; + END IF; + + UPDATE public.apps + SET + channel_device_count = v_channel_device_count, + manifest_bundle_count = v_manifest_bundle_count, + last_version = v_last_version + WHERE id = p_app_uuid; + + IF v_owner_org IS NOT NULL THEN + DELETE FROM public.app_metrics_cache + WHERE org_id = v_owner_org; + END IF; +END; +$$; + +ALTER FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid", "p_preserve_app_version_id" bigint) OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid", "p_preserve_app_version_id" bigint) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid", "p_preserve_app_version_id" bigint) TO "service_role"; + +CREATE OR REPLACE FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid") +RETURNS void +LANGUAGE "plpgsql" +SECURITY DEFINER +SET search_path = '' +AS $$ +BEGIN + PERFORM public.clear_onboarding_app_data(p_app_uuid, NULL::bigint); +END; +$$; + +ALTER FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid") OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid") FROM PUBLIC; +GRANT EXECUTE ON FUNCTION "public"."clear_onboarding_app_data"("p_app_uuid" "uuid") TO "service_role"; + +CREATE OR REPLACE FUNCTION "public"."cleanup_onboarding_app_data_on_complete"() +RETURNS trigger +LANGUAGE "plpgsql" +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_preserve_setting text; + v_preserve_app_version_id bigint; +BEGIN + IF OLD.need_onboarding IS TRUE AND NEW.need_onboarding IS FALSE THEN + v_preserve_setting := current_setting('capgo.onboarding_preserve_app_version_id', true); + v_preserve_app_version_id := NULLIF(v_preserve_setting, '')::bigint; + + PERFORM public.clear_onboarding_app_data(NEW.id, v_preserve_app_version_id); + END IF; + + RETURN NEW; +END; +$$; + +ALTER FUNCTION "public"."cleanup_onboarding_app_data_on_complete"() OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."cleanup_onboarding_app_data_on_complete"() FROM PUBLIC; + +CREATE OR REPLACE FUNCTION "public"."complete_onboarding_after_first_upload"() +RETURNS trigger +LANGUAGE "plpgsql" +SECURITY DEFINER +SET search_path = '' +AS $$ +DECLARE + v_app_uuid uuid; +BEGIN + IF NEW.name IN ('builtin', 'unknown') THEN + RETURN NEW; + END IF; + + IF COALESCE(NEW.deleted, false) IS TRUE THEN + RETURN NEW; + END IF; + + IF NOT ( + (NEW.storage_provider = 'external' AND NULLIF(BTRIM(COALESCE(NEW.external_url, '')), '') IS NOT NULL) + OR (NEW.storage_provider <> 'r2-direct' AND NULLIF(BTRIM(COALESCE(NEW.r2_path, '')), '') IS NOT NULL) + ) THEN + RETURN NEW; + END IF; + + SELECT id + INTO v_app_uuid + FROM public.apps + WHERE app_id = NEW.app_id + AND owner_org = NEW.owner_org + AND need_onboarding IS TRUE + LIMIT 1; + + IF v_app_uuid IS NULL THEN + RETURN NEW; + END IF; + + PERFORM set_config('capgo.onboarding_preserve_app_version_id', NEW.id::text, true); + + UPDATE public.apps + SET need_onboarding = false + WHERE id = v_app_uuid + AND need_onboarding IS TRUE; + + RETURN NEW; +END; +$$; + +ALTER FUNCTION "public"."complete_onboarding_after_first_upload"() OWNER TO "postgres"; +REVOKE ALL ON FUNCTION "public"."complete_onboarding_after_first_upload"() FROM PUBLIC; + +DROP TRIGGER IF EXISTS "complete_onboarding_after_first_upload" ON "public"."app_versions"; + +CREATE TRIGGER "complete_onboarding_after_first_upload" +AFTER INSERT OR UPDATE OF "deleted", "external_url", "r2_path", "storage_provider", "name", "app_id", "owner_org" +ON "public"."app_versions" +FOR EACH ROW +WHEN ( + NEW.name NOT IN ('builtin', 'unknown') + AND COALESCE(NEW.deleted, false) IS FALSE + AND ( + (NEW.storage_provider = 'external' AND NULLIF(BTRIM(COALESCE(NEW.external_url, '')), '') IS NOT NULL) + OR (NEW.storage_provider <> 'r2-direct' AND NULLIF(BTRIM(COALESCE(NEW.r2_path, '')), '') IS NOT NULL) + ) +) +EXECUTE FUNCTION "public"."complete_onboarding_after_first_upload"(); diff --git a/tests/onboarding-first-upload.test.ts b/tests/onboarding-first-upload.test.ts new file mode 100644 index 0000000000..d82d45e7d1 --- /dev/null +++ b/tests/onboarding-first-upload.test.ts @@ -0,0 +1,179 @@ +import type { PoolClient } from 'pg' +import { randomUUID } from 'node:crypto' +import { Pool } from 'pg' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { ORG_ID, POSTGRES_URL, USER_ID } from './test-utils.ts' + +async function rollbackAndRelease(client: PoolClient) { + try { + await client.query('ROLLBACK') + } + finally { + client.release() + } +} + +describe('onboarding completion after first upload', () => { + let pool: Pool + + beforeAll(() => { + pool = new Pool({ + connectionString: POSTGRES_URL, + max: 3, + idleTimeoutMillis: 2000, + }) + }) + + afterAll(async () => { + await pool.end() + }) + + it.concurrent('waits for an r2-direct upload to finish before completing onboarding', async () => { + const client = await pool.connect() + const appId = `com.onboarding.first.${randomUUID().slice(0, 8)}` + const staleVersionName = `0.9.0-${randomUUID().slice(0, 8)}` + + try { + await client.query('BEGIN') + await client.query( + ` + INSERT INTO public.apps (app_id, name, icon_url, owner_org, need_onboarding) + VALUES ($1, $2, '', $3, true) + `, + [appId, `Onboarding First Upload ${appId}`, ORG_ID], + ) + + await client.query( + ` + INSERT INTO public.app_versions (app_id, name, owner_org, user_id, storage_provider) + VALUES ($1, $2, $3, $4, 'r2') + `, + [appId, staleVersionName, ORG_ID, USER_ID], + ) + + const inserted = await client.query<{ id: string }>( + ` + INSERT INTO public.app_versions (app_id, name, owner_org, user_id, storage_provider) + VALUES ($1, '1.0.0', $2, $3, 'r2-direct') + RETURNING id + `, + [appId, ORG_ID, USER_ID], + ) + const versionId = inserted.rows[0].id + + await client.query( + ` + UPDATE public.app_versions + SET r2_path = $2 + WHERE id = $1 + `, + [versionId, `orgs/${ORG_ID}/apps/${appId}/1.0.0.zip`], + ) + + const draftApp = await client.query<{ need_onboarding: boolean }>( + ` + SELECT need_onboarding + FROM public.apps + WHERE app_id = $1 + `, + [appId], + ) + expect(draftApp.rows[0].need_onboarding).toBe(true) + + await client.query( + ` + UPDATE public.app_versions + SET storage_provider = 'r2' + WHERE id = $1 + `, + [versionId], + ) + + const completedApp = await client.query<{ last_version: string, need_onboarding: boolean }>( + ` + SELECT need_onboarding, last_version + FROM public.apps + WHERE app_id = $1 + `, + [appId], + ) + expect(completedApp.rows[0]).toEqual({ + need_onboarding: false, + last_version: '1.0.0', + }) + + const versions = await client.query<{ name: string }>( + ` + SELECT name + FROM public.app_versions + WHERE app_id = $1 + ORDER BY name + `, + [appId], + ) + expect(versions.rows.map(row => row.name)).toEqual(['1.0.0', 'builtin', 'unknown']) + } + finally { + await rollbackAndRelease(client) + } + }) + + it.concurrent('completes onboarding immediately for an external bundle upload', async () => { + const client = await pool.connect() + const appId = `com.onboarding.external.${randomUUID().slice(0, 8)}` + + try { + await client.query('BEGIN') + await client.query( + ` + INSERT INTO public.apps (app_id, name, icon_url, owner_org, need_onboarding) + VALUES ($1, $2, '', $3, true) + `, + [appId, `Onboarding External Upload ${appId}`, ORG_ID], + ) + + await client.query( + ` + INSERT INTO public.app_versions ( + app_id, + name, + owner_org, + user_id, + storage_provider, + external_url, + checksum + ) + VALUES ($1, '1.0.0', $2, $3, 'external', 'https://example.com/bundle.zip', $4) + `, + [appId, ORG_ID, USER_ID, randomUUID().replaceAll('-', '')], + ) + + const completedApp = await client.query<{ last_version: string, need_onboarding: boolean }>( + ` + SELECT need_onboarding, last_version + FROM public.apps + WHERE app_id = $1 + `, + [appId], + ) + expect(completedApp.rows[0]).toEqual({ + need_onboarding: false, + last_version: '1.0.0', + }) + + const preservedVersion = await client.query<{ external_url: string }>( + ` + SELECT external_url + FROM public.app_versions + WHERE app_id = $1 + AND name = '1.0.0' + `, + [appId], + ) + expect(preservedVersion.rows).toEqual([{ external_url: 'https://example.com/bundle.zip' }]) + } + finally { + await rollbackAndRelease(client) + } + }) +}) diff --git a/tests/security-definer-execute-hardening.test.ts b/tests/security-definer-execute-hardening.test.ts index 513decfd5a..55cf189e34 100644 --- a/tests/security-definer-execute-hardening.test.ts +++ b/tests/security-definer-execute-hardening.test.ts @@ -24,7 +24,10 @@ const SERVICE_ONLY_PROCS = [ 'public.apikeys_strip_plain_key_for_hashed()', 'public.check_encrypted_bundle_on_insert()', 'public.check_org_hashed_key_enforcement(uuid, public.apikeys)', + 'public.clear_onboarding_app_data(uuid)', + 'public.clear_onboarding_app_data(uuid, bigint)', 'public.cleanup_onboarding_app_data_on_complete()', + 'public.complete_onboarding_after_first_upload()', 'public.delete_old_deleted_versions()', 'public.generate_org_user_stripe_info_on_org_create()', 'public.get_apikey()', From 7a99a476fc8eae10903db0e2769ef01832753b2f Mon Sep 17 00:00:00 2001 From: Martin Donadieu Date: Mon, 18 May 2026 17:47:01 +0200 Subject: [PATCH 2/2] fix(onboarding): recreate cleanup trigger --- ...0518131054_complete_onboarding_after_first_upload.sql | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql b/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql index 870b8e18c9..5321ad4871 100644 --- a/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql +++ b/supabase/migrations/20260518131054_complete_onboarding_after_first_upload.sql @@ -178,6 +178,15 @@ $$; ALTER FUNCTION "public"."cleanup_onboarding_app_data_on_complete"() OWNER TO "postgres"; REVOKE ALL ON FUNCTION "public"."cleanup_onboarding_app_data_on_complete"() FROM PUBLIC; +DROP TRIGGER IF EXISTS "cleanup_onboarding_app_data_on_complete" ON "public"."apps"; + +CREATE TRIGGER "cleanup_onboarding_app_data_on_complete" +AFTER UPDATE OF "need_onboarding" +ON "public"."apps" +FOR EACH ROW +WHEN (OLD.need_onboarding IS TRUE AND NEW.need_onboarding IS FALSE) +EXECUTE FUNCTION "public"."cleanup_onboarding_app_data_on_complete"(); + CREATE OR REPLACE FUNCTION "public"."complete_onboarding_after_first_upload"() RETURNS trigger LANGUAGE "plpgsql"