fix: harden review findings and Netlify install

.env.example

@@ -9,9 +9,9 @@ DROPBOX_OAUTH_CLIENT_ID="your-dropbox-client-id"
 DROPBOX_OAUTH_CLIENT_SECRET="your-dropbox-client-secret"
 
 # Endpoint overrides for deterministic local provider tests. They are ignored unless
-# ALLOW_PROVIDER_ENDPOINT_OVERRIDES is exactly "true", and only loopback URLs are accepted -
+# ALLOW_PROVIDER_ENDPOINT_OVERRIDES is exactly "true", the local BDD stack variables are present,
-# these endpoints receive the OAuth client secret and refresh token, so never set this in a
+# and every URL is loopback. These endpoints receive OAuth secrets, so never set this in a deployed
-# deployed environment. `npm run test:bdd` sets it for you.
+# environment. `npm run test:bdd` supplies the complete test context.
 ALLOW_PROVIDER_ENDPOINT_OVERRIDES=""
 GOOGLE_OAUTH_AUTHORIZE_URL=""
 GOOGLE_OAUTH_TOKEN_URL=""

README.md

@@ -75,9 +75,10 @@ reviewed with product stakeholders, but does not skip it. Missing or ambiguous s
 
 Google Drive and Dropbox scenarios use a local provider server (`scripts/mock-provider-server.mjs`)
 and never contact real provider accounts. The endpoint overrides that point at it are refused unless
-`ALLOW_PROVIDER_ENDPOINT_OVERRIDES=true` **and** the override is a loopback URL - these endpoints
+`ALLOW_PROVIDER_ENDPOINT_OVERRIDES=true`, the local test-stack/service-role variables are present,
-receive the OAuth client secret and refresh token, so they must not be redirectable in a deployed
+and the override is a loopback URL. These endpoints receive the OAuth client secret and refresh
-environment. `npm run test:bdd` sets the flag; nothing else should.
+token, so they must not be redirectable in a deployed environment. `npm run test:bdd` supplies the
+complete test context.
 
 The mock's recorded requests, refresh counter and fail-uploads switch are reset before every scenario
 by an auto fixture in `tests/bdd/fixtures.ts`. That reset is also why the suite runs with a single

src/lib/services/providerEndpoints.ts

@@ -23,12 +23,10 @@ const LOOPBACK_HOSTS = new Set(['127.0.0.1', 'localhost', '[::1]']);
 
 /**
  * These endpoints receive the OAuth client secret and the user's refresh token, so an override
- * is only ever a local test seam - never a deployment knob. Two guards, because the env is
+ * is only ever a local test seam - never a deployment knob. The guards require the explicit
- * read at runtime (`$env/dynamic/private`) and a single injected variable would otherwise be
+ * override flag, the BDD service-role context, and a loopback test stack. Each override must also
- * enough to redirect those credentials to an arbitrary host:
+ * be loopback, so a single injected variable cannot redirect credentials to an arbitrary host.
- *
+ * Values are read at runtime from `$env/dynamic/private`.
- *  1. overrides are ignored unless ALLOW_PROVIDER_ENDPOINT_OVERRIDES is exactly "true", and
- *  2. even then, only loopback URLs are accepted.
  */
 function isLocalOverride(value: string): boolean {
 	try {
@@ -44,7 +42,11 @@ function isLocalOverride(value: string): boolean {
 export function resolveProviderEndpoints(
 	env: Record<string, string | undefined>
 ): ProviderEndpoints {
-	const overridesAllowed = env.ALLOW_PROVIDER_ENDPOINT_OVERRIDES === 'true';
+	const overridesAllowed =
+		env.ALLOW_PROVIDER_ENDPOINT_OVERRIDES === 'true' &&
+		!!env.E2E_SERVICE_ROLE_KEY &&
+		!!env.TEST_SUPABASE_URL &&
+		isLocalOverride(env.TEST_SUPABASE_URL);
 	const pick = (override: string | undefined, fallback: string) =>
 		overridesAllowed && override && isLocalOverride(override) ? override : fallback;
 

src/lib/stores/offlineSync.ts

@@ -122,6 +122,7 @@ export function startOfflineSync(getUserId: () => string | null): () => void {
 					)
 				)
 			);
+			if (getUserId() !== userId) throw new Error('Session changed during offline synchronization');
 			const result = await workerMessage({
 				type: 'SYNC_OFFLINE_SNAPSHOT',
 				snapshot,

src/routes/+layout.ts

@@ -5,15 +5,35 @@ import type { CookieSerializeOptions } from 'cookie';
 
 export const load: LayoutLoad = async ({ fetch, data, depends }) => {
 	depends('supabase:auth');
-	const recoveryIntent =
+	let recoveryExchangeSucceeded = false;
-		isBrowser() &&
+	let hashRecoveryCallback = false;
-		window.location.pathname === '/reset-password' &&
+	let codeRecoveryCallback = false;
-		(new URLSearchParams(window.location.hash.slice(1)).get('type') === 'recovery' ||
+	if (isBrowser() && window.location.pathname === '/reset-password') {
-			new URL(window.location.href).searchParams.has('code'));
+		const hash = new URLSearchParams(window.location.hash.slice(1));
+		hashRecoveryCallback =
+			hash.get('type') === 'recovery' && hash.has('access_token') && hash.has('refresh_token');
+		codeRecoveryCallback = new URL(window.location.href).searchParams.has('code');
+	}
+	const authFetch: typeof fetch = async (input, init) => {
+		const response = await fetch(input, init);
+		if (codeRecoveryCallback && response.ok) {
+			const requestUrl = new URL(
+				typeof input === 'string' || input instanceof URL ? input : input.url,
+				window.location.origin
+			);
+			if (
+				requestUrl.pathname.endsWith('/auth/v1/token') &&
+				requestUrl.searchParams.get('grant_type') === 'pkce'
+			) {
+				recoveryExchangeSucceeded = true;
+			}
+		}
+		return response;
+	};
 
 	const supabase = createBrowserClient(PUBLIC_SUPABASE_URL, PUBLIC_SUPABASE_ANON_KEY, {
 		global: {
-			fetch
+			fetch: authFetch
 		},
 		cookies: {
 			get(key: string) {
@@ -44,5 +64,9 @@ export const load: LayoutLoad = async ({ fetch, data, depends }) => {
 		data: { session }
 	} = await supabase.auth.getSession();
 
-	return { supabase, session, recoveryIntent };
+	return {
+		supabase,
+		session,
+		recoveryIntent: !!session && (hashRecoveryCallback || recoveryExchangeSucceeded)
+	};
 };

src/routes/reset-password/+page.svelte

@@ -108,10 +108,7 @@
 
 			successMessage = 'Password updated successfully! Redirecting to sign in...';
 			sessionStorage.removeItem(recoveryMarkerKey);
-			setTimeout(async () => {
+			setTimeout(() => void finishPasswordReset(), 1_000);
-				await supabase.auth.signOut();
-				await goto('/signin');
-			}, 1_000);
 		} catch (err) {
 			console.error('Update password error:', err);
 			errorMessage = 'An unexpected error occurred. Please try again.';
@@ -120,6 +117,20 @@
 		}
 	}
 
+	async function finishPasswordReset() {
+		try {
+			const { error } = await supabase.auth.signOut();
+			if (error) {
+				errorMessage = `Password updated, but sign out failed: ${error.message}`;
+				return;
+			}
+			await goto('/signin');
+		} catch (error) {
+			console.error('Sign out after password reset failed:', error);
+			errorMessage = 'Password updated, but sign out failed. Please try again.';
+		}
+	}
+
 	function handleKeyPress(event: KeyboardEvent) {
 		if (event.key === 'Enter') {
 			updatePassword();

static/offline-worker.js

@@ -3,6 +3,7 @@ const OFFLINE_META_CACHE = `${OFFLINE_CACHE_PREFIX}meta-v1`;
 const OFFLINE_META_URL = '/__offline/current';
 let offlineEpoch = 0;
 let offlineOperation = Promise.resolve();
+let claimedUserId = null;
 
 function queueOfflineOperation(operation) {
 	const result = offlineOperation.then(operation, operation);
@@ -63,6 +64,7 @@ async function cacheArtwork(cache, urls) {
 self.addEventListener('message', (event) => {
 	const reply = (value) => event.ports[0]?.postMessage(value);
 	if (event.data?.type === 'CLEAR_OFFLINE_DATA') {
+		claimedUserId = null;
 		offlineEpoch++;
 		event.waitUntil(
 			queueOfflineOperation(async () => {
@@ -75,10 +77,14 @@ self.addEventListener('message', (event) => {
 		return;
 	}
 	if (event.data?.type === 'CLAIM_OFFLINE_USER') {
+		if (typeof event.data.userId !== 'string') {
+			reply({ ok: false, error: 'Invalid offline cache owner' });
+			return;
+		}
+		claimedUserId = event.data.userId;
 		offlineEpoch++;
 		event.waitUntil(
 			queueOfflineOperation(async () => {
-				if (typeof event.data.userId !== 'string') throw new Error('Invalid offline cache owner');
 				const meta = await currentOfflineMeta();
 				if (meta?.userId && meta.userId !== event.data.userId) {
 					await clearOfflineData();
@@ -91,6 +97,7 @@ self.addEventListener('message', (event) => {
 	}
 	if (event.data?.type !== 'SYNC_OFFLINE_SNAPSHOT') return;
 	const syncEpoch = offlineEpoch;
+	const syncUserId = claimedUserId;
 
 	event.waitUntil(
 		queueOfflineOperation(async () => {
@@ -101,6 +108,9 @@ self.addEventListener('message', (event) => {
 				if (!snapshot || snapshot.version !== 1 || typeof snapshot.userId !== 'string') {
 					throw new Error('Unsupported offline snapshot');
 				}
+				if (!syncUserId || snapshot.userId !== syncUserId) {
+					throw new Error('Offline snapshot owner did not match the claimed account');
+				}
 				const previousMeta = await currentOfflineMeta();
 				if (previousMeta?.userId && previousMeta.userId !== snapshot.userId)
 					await clearOfflineData();

tests/bdd/globalTeardown.ts

@@ -1,9 +1,14 @@
+import { requireLoopbackUrl } from '../support/loopback';
+
 /**
  * Deletes the users each BDD run provisions, so repeated local runs do not need a full
  * `supabase db reset` to stay clean. Pokédexes, catch records and integrations follow via the
  * schema's cascades. Only the suite's own synthetic addresses are touched.
  */
-const SUPABASE_URL = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321';
+const SUPABASE_URL = requireLoopbackUrl(
+	process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321',
+	'TEST_SUPABASE_URL'
+);
 const OWNED_EMAIL = /^(bdd|other|integration)-.*@example\.test$/;
 
 type AdminUser = { id: string; email?: string };

tests/bdd/steps/account.steps.ts

@@ -104,7 +104,7 @@ When('I request a password reset', async ({ page, state }) => {
 });
 
 When('I visit the password recovery page directly', async ({ page }) => {
-	await page.goto('/reset-password');
+	await page.goto('/reset-password?code=arbitrary-code');
 });
 
 When('I enter two different replacement passwords', async ({ page, state }) => {

tests/bdd/steps/backup.steps.ts

@@ -1,10 +1,14 @@
 import { createBdd } from 'playwright-bdd';
 import { test, expect } from '../fixtures';
 import { createDexThroughUi, firstPokemon, openFirstPokemon } from '../support/app';
+import { requireLoopbackUrl } from '../../support/loopback';
 
 const { Given, When, Then } = createBdd(test);
 const MOCK_URL = process.env.MOCK_PROVIDER_URL ?? 'http://127.0.0.1:4199';
-const SUPABASE_URL = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321';
+const SUPABASE_URL = requireLoopbackUrl(
+	process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321',
+	'TEST_SUPABASE_URL'
+);
 
 type Provider = 'google_drive' | 'dropbox';
 

tests/bdd/steps/pokedex-lifecycle.steps.ts

@@ -1,6 +1,7 @@
 import { createBdd } from 'playwright-bdd';
 import { test, expect } from '../fixtures';
 import { createDexThroughUi, deleteAllPokedexes } from '../support/app';
+import { requireLoopbackUrl } from '../../support/loopback';
 
 const { Given, When, Then } = createBdd(test);
 
@@ -37,7 +38,10 @@ Given('I have a Shiny Dex named {string}', async ({ page, state }, name: string)
 });
 
 Given('another trainer has a Pokédex', async ({ state }) => {
-	const url = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321';
+	const url = requireLoopbackUrl(
+		process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321',
+		'TEST_SUPABASE_URL'
+	);
 	const key = process.env.E2E_SERVICE_ROLE_KEY;
 	if (!key) throw new Error('E2E_SERVICE_ROLE_KEY is required');
 	const headers = {

tests/bdd/steps/progress.steps.ts

@@ -8,6 +8,20 @@ async function ensurePokemonModal(page: Parameters<typeof firstPokemon>[0]) {
 	if ((await page.getByRole('dialog').count()) === 0) await openFirstPokemon(page);
 }
 
+async function persistCatchChange(
+	page: Parameters<typeof firstPokemon>[0],
+	action: () => Promise<void>
+) {
+	const persistence = page.waitForResponse(
+		(response) =>
+			response.request().method() === 'POST' &&
+			new URL(response.url()).pathname.endsWith('/catch-records')
+	);
+	await action();
+	const response = await persistence;
+	expect(response.ok(), `catch-record persistence failed: ${response.status()}`).toBe(true);
+}
+
 async function settleAndReload(page: Parameters<typeof firstPokemon>[0]) {
 	await expect(page.getByText(/Saving…/)).toHaveCount(0, { timeout: 15_000 });
 	await page.reload({ waitUntil: 'networkidle' });
@@ -21,7 +35,7 @@ When('I mark the first Pokémon as caught', async ({ page }) => {
 		.getByText('Caught:', { exact: true })
 		.locator('..')
 		.getByRole('checkbox');
-	await checkbox.check();
+	await persistCatchChange(page, () => checkbox.check());
 });
 
 When('I mark the first Pokémon as needing evolution', async ({ page }) => {
@@ -31,23 +45,25 @@ When('I mark the first Pokémon as needing evolution', async ({ page }) => {
 		.getByText('Needs to evolve:', { exact: true })
 		.locator('..')
 		.getByRole('checkbox');
-	await checkbox.check();
+	await persistCatchChange(page, () => checkbox.check());
 });
 
 When('I mark the first Pokémon as in HOME', async ({ page }) => {
 	await ensurePokemonModal(page);
-	await page
+	const checkbox = page
 		.getByRole('dialog')
 		.getByText('In Home:', { exact: true })
 		.locator('..')
-		.getByRole('checkbox')
+		.getByRole('checkbox');
-		.check();
+	await persistCatchChange(page, () => checkbox.check());
 });
 
 When('I add the note {string} to the first Pokémon', async ({ page }, note: string) => {
 	await ensurePokemonModal(page);
-	await page.getByRole('dialog').getByLabel('Notes:').fill(note);
+	await persistCatchChange(page, async () => {
-	await page.getByRole('dialog').getByLabel('Notes:').blur();
+		await page.getByRole('dialog').getByLabel('Notes:').fill(note);
+		await page.getByRole('dialog').getByLabel('Notes:').blur();
+	});
 });
 
 function boxContainer(page: Parameters<typeof firstPokemon>[0], box: number) {
@@ -60,7 +76,9 @@ function boxContainer(page: Parameters<typeof firstPokemon>[0], box: number) {
 When('I mark box {int} as caught', async ({ page }, box: number) => {
 	const container = boxContainer(page, box);
 	await container.getByRole('button', { name: 'Open bulk actions menu' }).click();
-	await container.getByRole('button', { name: 'Mark box as Caught' }).click();
+	await persistCatchChange(page, () =>
+		container.getByRole('button', { name: 'Mark box as Caught' }).click()
+	);
 });
 
 When('I filter to Pokémon that are not caught', async ({ page, state }) => {

tests/bdd/steps/pwa.steps.ts

@@ -40,6 +40,10 @@ When('I open the built application', async ({ page, state }) => {
 	recordLegacyWorkerRequest(page, state);
 	await page.goto('/');
 	await waitForServiceWorker(page);
+	if (!(await page.evaluate(() => !!navigator.serviceWorker.controller))) {
+		await page.reload();
+		await waitForServiceWorker(page);
+	}
 });
 
 Given('I have opened the built application online', async ({ page, state }) => {
@@ -79,12 +83,13 @@ Given('my offline copy is synchronized', async ({ page, state }) => {
 		const metaResponse = await (
 			await caches.open('livingdex-offline-meta-v1')
 		).match('/__offline/current');
-		if (!metaResponse) return '';
+		if (!metaResponse) throw new Error('Offline snapshot metadata was not cached');
 		const meta = await metaResponse.json();
 		const snapshotResponse = await (
 			await caches.open(meta.dataCache)
 		).match(`/__offline/snapshot/${encodeURIComponent(meta.userId)}`);
-		return snapshotResponse ? await snapshotResponse.text() : '';
+		if (!snapshotResponse) throw new Error('Offline snapshot payload was not cached');
+		return snapshotResponse.text();
 	});
 	expect(serializedSnapshot).not.toMatch(/access_token|refresh_token/i);
 });
@@ -104,7 +109,9 @@ When('I go offline and then return online', async ({ page }) => {
 });
 
 Then('a service worker controls the page', async ({ page }) => {
-	expect(await waitForServiceWorker(page)).toMatch(/\/(?:sw|prompt-sw)\.js$/);
+	await expect
+		.poll(() => page.evaluate(() => navigator.serviceWorker.controller?.scriptURL ?? null))
+		.toMatch(/\/(?:sw|prompt-sw)\.js$/);
 	expect(
 		await page.evaluate(() =>
 			navigator.serviceWorker.getRegistrations().then((items) => items.length)

tests/bdd/support/app.ts

@@ -1,7 +1,11 @@
 import type { Page } from '@playwright/test';
 import type { ScenarioState } from '../fixtures';
+import { requireLoopbackUrl } from '../../support/loopback';
 
-const SUPABASE_URL = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321';
+const SUPABASE_URL = requireLoopbackUrl(
+	process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321',
+	'TEST_SUPABASE_URL'
+);
 const SERVICE_ROLE_KEY = process.env.E2E_SERVICE_ROLE_KEY;
 
 function requireServiceRoleKey(): string {

tests/integration/databaseIntegrity.integration.test.ts

@@ -1,7 +1,11 @@
 import { createClient } from '@supabase/supabase-js';
 import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+import { requireLoopbackUrl } from '../support/loopback';
 
-const url = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321';
+const url = requireLoopbackUrl(
+	process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321',
+	'TEST_SUPABASE_URL'
+);
 const anonKey = process.env.TEST_SUPABASE_ANON_KEY;
 const serviceKey = process.env.E2E_SERVICE_ROLE_KEY ?? process.env.SUPABASE_SERVICE_ROLE_KEY;
 

tests/support/loopback.ts

@@ -0,0 +1,15 @@
+const LOOPBACK_HOSTS = new Set(['127.0.0.1', 'localhost', '[::1]']);
+
+export function isLoopbackUrl(value: string): boolean {
+	try {
+		return LOOPBACK_HOSTS.has(new URL(value).hostname);
+	} catch {
+		return false;
+	}
+}
+
+export function requireLoopbackUrl(value: string, label: string): string {
+	if (!isLoopbackUrl(value))
+		throw new Error(`Refusing to send credentials to non-loopback ${label}`);
+	return value;
+}

tests/unit/providerEndpoints.test.ts

@@ -5,6 +5,8 @@ import {
 } from '$lib/services/providerEndpoints';
 
 const localOverrides = {
+	TEST_SUPABASE_URL: 'http://127.0.0.1:54321',
+	E2E_SERVICE_ROLE_KEY: 'local-test-service-key',
 	GOOGLE_OAUTH_AUTHORIZE_URL: 'http://127.0.0.1:4199/google/authorize',
 	GOOGLE_OAUTH_TOKEN_URL: 'http://127.0.0.1:4199/google/token',
 	GOOGLE_DRIVE_API_URL: 'http://127.0.0.1:4199/google/drive',
@@ -49,6 +51,19 @@ describe('provider endpoints', () => {
 		});
 	});
 
+	it('refuses HTTP overrides without a verified local test stack', () => {
+		const { TEST_SUPABASE_URL, E2E_SERVICE_ROLE_KEY, ...deployedOverrides } = localOverrides;
+		expect(TEST_SUPABASE_URL).toBeTruthy();
+		expect(E2E_SERVICE_ROLE_KEY).toBeTruthy();
+		expect(
+			resolveProviderEndpoints({
+				...deployedOverrides,
+				NODE_ENV: 'production',
+				ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'true'
+			})
+		).toEqual(PROVIDER_ENDPOINT_DEFAULTS);
+	});
+
 	it.each([
 		'https://attacker.example/token',
 		'http://127.0.0.1.attacker.example/token',
@@ -58,6 +73,8 @@ describe('provider endpoints', () => {
 		''
 	])('refuses the non-loopback override %j even when overrides are allowed', (value) => {
 		const endpoints = resolveProviderEndpoints({
+			TEST_SUPABASE_URL: localOverrides.TEST_SUPABASE_URL,
+			E2E_SERVICE_ROLE_KEY: localOverrides.E2E_SERVICE_ROLE_KEY,
 			ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'true',
 			GOOGLE_OAUTH_TOKEN_URL: value,
 			DROPBOX_OAUTH_TOKEN_URL: value
@@ -69,6 +86,8 @@ describe('provider endpoints', () => {
 	it('accepts localhost as well as 127.0.0.1', () => {
 		expect(
 			resolveProviderEndpoints({
+				TEST_SUPABASE_URL: localOverrides.TEST_SUPABASE_URL,
+				E2E_SERVICE_ROLE_KEY: localOverrides.E2E_SERVICE_ROLE_KEY,
 				ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'true',
 				GOOGLE_OAUTH_TOKEN_URL: 'http://localhost:4199/google/token'
 			}).google.token