mirror of
https://github.com/jcreek/LivingDexTracker.git
synced 2026-09-14 17:42:17 +00:00
fix(export): restrict provider endpoint overrides to loopback test servers
The endpoint overrides are read through `$env/dynamic/private`, so they are evaluated per request in production, not baked in at build time. That made a single injected environment variable enough to redirect the authorization-code and refresh-token POSTs - which carry the OAuth client secret and the user's refresh token - to an arbitrary host, and to redirect the user's authorize hop to an arbitrary URL. Overrides are now ignored unless ALLOW_PROVIDER_ENDPOINT_OVERRIDES is exactly "true" and the value is a loopback URL. `npm run test:bdd` sets the flag; nothing else should. resolveProviderEndpoints is pure so the refusals are unit tested, including near-miss hosts such as http://127.0.0.1.example. Also drops the unused `pokedex` parameter from buildCsv rather than silencing it with `void`, and the dead hasGigantamaxed field from its fallback record.
This commit is contained in:
+5
-1
@@ -8,7 +8,11 @@ GOOGLE_OAUTH_CLIENT_SECRET="your-google-client-secret"
|
||||
DROPBOX_OAUTH_CLIENT_ID="your-dropbox-client-id"
|
||||
DROPBOX_OAUTH_CLIENT_SECRET="your-dropbox-client-secret"
|
||||
|
||||
# Optional endpoint overrides for deterministic local provider tests.
|
||||
# Endpoint overrides for deterministic local provider tests. They are ignored unless
|
||||
# ALLOW_PROVIDER_ENDPOINT_OVERRIDES is exactly "true", and only loopback URLs are accepted -
|
||||
# these endpoints receive the OAuth client secret and refresh token, so never set this in a
|
||||
# deployed environment. `npm run test:bdd` sets it for you.
|
||||
ALLOW_PROVIDER_ENDPOINT_OVERRIDES=""
|
||||
GOOGLE_OAUTH_AUTHORIZE_URL=""
|
||||
GOOGLE_OAUTH_TOKEN_URL=""
|
||||
GOOGLE_DRIVE_API_URL=""
|
||||
|
||||
+1
-1
@@ -28,7 +28,7 @@
|
||||
"test:integration": "node scripts/run-with-local-supabase.mjs vitest run --config vitest.integration.config.mts",
|
||||
"test:bdd:generate": "bddgen",
|
||||
"test:bdd:inner": "npm run build-inject-manifest && npm run test:bdd:generate && playwright test --project=chromium",
|
||||
"test:bdd": "GOOGLE_OAUTH_CLIENT_ID=mock-google GOOGLE_OAUTH_CLIENT_SECRET=mock-google-secret DROPBOX_OAUTH_CLIENT_ID=mock-dropbox DROPBOX_OAUTH_CLIENT_SECRET=mock-dropbox-secret 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 GOOGLE_DRIVE_UPLOAD_URL=http://127.0.0.1:4199/google/upload DROPBOX_OAUTH_AUTHORIZE_URL=http://127.0.0.1:4199/dropbox/authorize DROPBOX_OAUTH_TOKEN_URL=http://127.0.0.1:4199/dropbox/token DROPBOX_UPLOAD_URL=http://127.0.0.1:4199/dropbox/files/upload node scripts/run-with-local-supabase.mjs npm run test:bdd:inner",
|
||||
"test:bdd": "ALLOW_PROVIDER_ENDPOINT_OVERRIDES=true GOOGLE_OAUTH_CLIENT_ID=mock-google GOOGLE_OAUTH_CLIENT_SECRET=mock-google-secret DROPBOX_OAUTH_CLIENT_ID=mock-dropbox DROPBOX_OAUTH_CLIENT_SECRET=mock-dropbox-secret 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 GOOGLE_DRIVE_UPLOAD_URL=http://127.0.0.1:4199/google/upload DROPBOX_OAUTH_AUTHORIZE_URL=http://127.0.0.1:4199/dropbox/authorize DROPBOX_OAUTH_TOKEN_URL=http://127.0.0.1:4199/dropbox/token DROPBOX_UPLOAD_URL=http://127.0.0.1:4199/dropbox/files/upload node scripts/run-with-local-supabase.mjs npm run test:bdd:inner",
|
||||
"test:build:generate-static": "npm run build-generate-sw && GENERATE_SW=true vitest run --config vitest.build.config.mts",
|
||||
"test:build:generate-node": "npm run build-generate-sw-node && NODE_ADAPTER=true GENERATE_SW=true vitest run --config vitest.build.config.mts",
|
||||
"test:build:inject-static": "npm run build-inject-manifest && vitest run --config vitest.build.config.mts",
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import type { CombinedData } from '$lib/models/CombinedData';
|
||||
import type { Pokedex } from '$lib/models/Pokedex';
|
||||
|
||||
export function csvEscape(value: unknown): string {
|
||||
if (value === null || value === undefined) return '';
|
||||
@@ -15,8 +14,7 @@ export function sanitizeFileName(name: string, fallback: string): string {
|
||||
return safe.endsWith('.csv') ? safe : `${safe}.csv`;
|
||||
}
|
||||
|
||||
export function buildCsv(pokedex: Pokedex, combinedData: CombinedData[]): string {
|
||||
void pokedex;
|
||||
export function buildCsv(combinedData: CombinedData[]): string {
|
||||
const headers = [
|
||||
'pokemonId',
|
||||
'pokedexNumber',
|
||||
@@ -35,7 +33,6 @@ export function buildCsv(pokedex: Pokedex, combinedData: CombinedData[]): string
|
||||
caught: false,
|
||||
haveToEvolve: false,
|
||||
inHome: false,
|
||||
hasGigantamaxed: false,
|
||||
personalNotes: ''
|
||||
};
|
||||
lines.push(
|
||||
|
||||
@@ -398,7 +398,7 @@ export async function exportPokedexIfConfigured(
|
||||
pokedex.gameScope || '',
|
||||
dexScopes
|
||||
);
|
||||
const csv = buildCsv(pokedex, combinedData);
|
||||
const csv = buildCsv(combinedData);
|
||||
|
||||
const failures: ExportFailure[] = [];
|
||||
let successes = 0;
|
||||
|
||||
@@ -1,18 +1,70 @@
|
||||
import { getEnv } from '$lib/utils/env';
|
||||
|
||||
export function getProviderEndpoints() {
|
||||
const env = getEnv();
|
||||
export type ProviderEndpoints = {
|
||||
google: { authorize: string; token: string; driveApi: string; driveUpload: string };
|
||||
dropbox: { authorize: string; token: string; upload: string };
|
||||
};
|
||||
|
||||
const DEFAULTS: ProviderEndpoints = {
|
||||
google: {
|
||||
authorize: 'https://accounts.google.com/o/oauth2/v2/auth',
|
||||
token: 'https://oauth2.googleapis.com/token',
|
||||
driveApi: 'https://www.googleapis.com/drive/v3',
|
||||
driveUpload: 'https://www.googleapis.com/upload/drive/v3'
|
||||
},
|
||||
dropbox: {
|
||||
authorize: 'https://www.dropbox.com/oauth2/authorize',
|
||||
token: 'https://api.dropbox.com/oauth2/token',
|
||||
upload: 'https://content.dropboxapi.com/2/files/upload'
|
||||
}
|
||||
};
|
||||
|
||||
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
|
||||
* read at runtime (`$env/dynamic/private`) and a single injected variable would otherwise be
|
||||
* enough to redirect those credentials to an arbitrary host:
|
||||
*
|
||||
* 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 {
|
||||
const url = new URL(value);
|
||||
return (
|
||||
(url.protocol === 'http:' || url.protocol === 'https:') && LOOPBACK_HOSTS.has(url.hostname)
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function resolveProviderEndpoints(
|
||||
env: Record<string, string | undefined>
|
||||
): ProviderEndpoints {
|
||||
const overridesAllowed = env.ALLOW_PROVIDER_ENDPOINT_OVERRIDES === 'true';
|
||||
const pick = (override: string | undefined, fallback: string) =>
|
||||
overridesAllowed && override && isLocalOverride(override) ? override : fallback;
|
||||
|
||||
return {
|
||||
google: {
|
||||
authorize: env.GOOGLE_OAUTH_AUTHORIZE_URL || 'https://accounts.google.com/o/oauth2/v2/auth',
|
||||
token: env.GOOGLE_OAUTH_TOKEN_URL || 'https://oauth2.googleapis.com/token',
|
||||
driveApi: env.GOOGLE_DRIVE_API_URL || 'https://www.googleapis.com/drive/v3',
|
||||
driveUpload: env.GOOGLE_DRIVE_UPLOAD_URL || 'https://www.googleapis.com/upload/drive/v3'
|
||||
authorize: pick(env.GOOGLE_OAUTH_AUTHORIZE_URL, DEFAULTS.google.authorize),
|
||||
token: pick(env.GOOGLE_OAUTH_TOKEN_URL, DEFAULTS.google.token),
|
||||
driveApi: pick(env.GOOGLE_DRIVE_API_URL, DEFAULTS.google.driveApi),
|
||||
driveUpload: pick(env.GOOGLE_DRIVE_UPLOAD_URL, DEFAULTS.google.driveUpload)
|
||||
},
|
||||
dropbox: {
|
||||
authorize: env.DROPBOX_OAUTH_AUTHORIZE_URL || 'https://www.dropbox.com/oauth2/authorize',
|
||||
token: env.DROPBOX_OAUTH_TOKEN_URL || 'https://api.dropbox.com/oauth2/token',
|
||||
upload: env.DROPBOX_UPLOAD_URL || 'https://content.dropboxapi.com/2/files/upload'
|
||||
authorize: pick(env.DROPBOX_OAUTH_AUTHORIZE_URL, DEFAULTS.dropbox.authorize),
|
||||
token: pick(env.DROPBOX_OAUTH_TOKEN_URL, DEFAULTS.dropbox.token),
|
||||
upload: pick(env.DROPBOX_UPLOAD_URL, DEFAULTS.dropbox.upload)
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
export function getProviderEndpoints(): ProviderEndpoints {
|
||||
return resolveProviderEndpoints(getEnv());
|
||||
}
|
||||
|
||||
export const PROVIDER_ENDPOINT_DEFAULTS = DEFAULTS;
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
/**
|
||||
* Stands in for `$env/dynamic/private` so modules that reach for runtime env can be unit
|
||||
* tested. Vitest maps the virtual module here; see vitest.config.mts.
|
||||
*/
|
||||
export const env: Record<string, string | undefined> = process.env;
|
||||
@@ -4,7 +4,7 @@ import {
|
||||
csvEscape,
|
||||
sanitizeFileName,
|
||||
shouldRefreshToken
|
||||
} from '../../src/lib/services/PokedexExportFormatting';
|
||||
} from '$lib/services/PokedexExportFormatting';
|
||||
|
||||
describe('Pokédex export formatting', () => {
|
||||
it.each([
|
||||
@@ -26,35 +26,32 @@ describe('Pokédex export formatting', () => {
|
||||
});
|
||||
|
||||
it('builds a stable, escaped CSV with defaults for missing catch records', () => {
|
||||
const csv = buildCsv(
|
||||
{ _id: 'dex-1', name: 'Test' } as never,
|
||||
[
|
||||
{
|
||||
pokedexEntry: {
|
||||
_id: '25',
|
||||
pokedexNumber: 25,
|
||||
pokemon: 'Pikachu',
|
||||
form: null
|
||||
},
|
||||
catchRecord: {
|
||||
caught: true,
|
||||
haveToEvolve: false,
|
||||
inHome: true,
|
||||
hasGigantamaxed: false,
|
||||
personalNotes: 'Comma, and "quote"'
|
||||
}
|
||||
const csv = buildCsv([
|
||||
{
|
||||
pokedexEntry: {
|
||||
_id: '25',
|
||||
pokedexNumber: 25,
|
||||
pokemon: 'Pikachu',
|
||||
form: null
|
||||
},
|
||||
{
|
||||
pokedexEntry: {
|
||||
_id: '26',
|
||||
pokedexNumber: 26,
|
||||
pokemon: 'Raichu',
|
||||
form: 'Alolan'
|
||||
},
|
||||
catchRecord: null
|
||||
catchRecord: {
|
||||
caught: true,
|
||||
haveToEvolve: false,
|
||||
inHome: true,
|
||||
hasGigantamaxed: false,
|
||||
personalNotes: 'Comma, and "quote"'
|
||||
}
|
||||
] as never
|
||||
);
|
||||
},
|
||||
{
|
||||
pokedexEntry: {
|
||||
_id: '26',
|
||||
pokedexNumber: 26,
|
||||
pokemon: 'Raichu',
|
||||
form: 'Alolan'
|
||||
},
|
||||
catchRecord: null
|
||||
}
|
||||
] as never);
|
||||
expect(csv.split('\r\n')).toEqual([
|
||||
'pokemonId,pokedexNumber,pokemon,form,caught,haveToEvolve,inHome,personalNotes',
|
||||
'25,25,Pikachu,,true,false,true,"Comma, and ""quote"""',
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
PROVIDER_ENDPOINT_DEFAULTS,
|
||||
resolveProviderEndpoints
|
||||
} from '$lib/services/providerEndpoints';
|
||||
|
||||
const localOverrides = {
|
||||
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',
|
||||
GOOGLE_DRIVE_UPLOAD_URL: 'http://127.0.0.1:4199/google/upload',
|
||||
DROPBOX_OAUTH_AUTHORIZE_URL: 'http://127.0.0.1:4199/dropbox/authorize',
|
||||
DROPBOX_OAUTH_TOKEN_URL: 'http://127.0.0.1:4199/dropbox/token',
|
||||
DROPBOX_UPLOAD_URL: 'http://127.0.0.1:4199/dropbox/files/upload'
|
||||
};
|
||||
|
||||
describe('provider endpoints', () => {
|
||||
it('uses the real provider endpoints when nothing is configured', () => {
|
||||
expect(resolveProviderEndpoints({})).toEqual(PROVIDER_ENDPOINT_DEFAULTS);
|
||||
});
|
||||
|
||||
it('ignores overrides unless they are explicitly allowed', () => {
|
||||
// The exfiltration case: these variables carry the client secret and refresh token, and
|
||||
// the env is read per request in production.
|
||||
expect(resolveProviderEndpoints(localOverrides)).toEqual(PROVIDER_ENDPOINT_DEFAULTS);
|
||||
expect(
|
||||
resolveProviderEndpoints({ ...localOverrides, ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'false' })
|
||||
).toEqual(PROVIDER_ENDPOINT_DEFAULTS);
|
||||
expect(
|
||||
resolveProviderEndpoints({ ...localOverrides, ALLOW_PROVIDER_ENDPOINT_OVERRIDES: '1' })
|
||||
).toEqual(PROVIDER_ENDPOINT_DEFAULTS);
|
||||
});
|
||||
|
||||
it('applies allowed loopback overrides', () => {
|
||||
expect(
|
||||
resolveProviderEndpoints({ ...localOverrides, ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'true' })
|
||||
).toEqual({
|
||||
google: {
|
||||
authorize: localOverrides.GOOGLE_OAUTH_AUTHORIZE_URL,
|
||||
token: localOverrides.GOOGLE_OAUTH_TOKEN_URL,
|
||||
driveApi: localOverrides.GOOGLE_DRIVE_API_URL,
|
||||
driveUpload: localOverrides.GOOGLE_DRIVE_UPLOAD_URL
|
||||
},
|
||||
dropbox: {
|
||||
authorize: localOverrides.DROPBOX_OAUTH_AUTHORIZE_URL,
|
||||
token: localOverrides.DROPBOX_OAUTH_TOKEN_URL,
|
||||
upload: localOverrides.DROPBOX_UPLOAD_URL
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
'https://attacker.example/token',
|
||||
'http://127.0.0.1.attacker.example/token',
|
||||
'http://[::2]/token',
|
||||
'file:///etc/passwd',
|
||||
'not-a-url',
|
||||
''
|
||||
])('refuses the non-loopback override %j even when overrides are allowed', (value) => {
|
||||
const endpoints = resolveProviderEndpoints({
|
||||
ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'true',
|
||||
GOOGLE_OAUTH_TOKEN_URL: value,
|
||||
DROPBOX_OAUTH_TOKEN_URL: value
|
||||
});
|
||||
expect(endpoints.google.token).toBe(PROVIDER_ENDPOINT_DEFAULTS.google.token);
|
||||
expect(endpoints.dropbox.token).toBe(PROVIDER_ENDPOINT_DEFAULTS.dropbox.token);
|
||||
});
|
||||
|
||||
it('accepts localhost as well as 127.0.0.1', () => {
|
||||
expect(
|
||||
resolveProviderEndpoints({
|
||||
ALLOW_PROVIDER_ENDPOINT_OVERRIDES: 'true',
|
||||
GOOGLE_OAUTH_TOKEN_URL: 'http://localhost:4199/google/token'
|
||||
}).google.token
|
||||
).toBe('http://localhost:4199/google/token');
|
||||
});
|
||||
});
|
||||
+2
-1
@@ -4,7 +4,8 @@ import { fileURLToPath } from 'node:url';
|
||||
export default defineConfig({
|
||||
resolve: {
|
||||
alias: {
|
||||
$lib: fileURLToPath(new URL('./src/lib', import.meta.url))
|
||||
$lib: fileURLToPath(new URL('./src/lib', import.meta.url)),
|
||||
'$env/dynamic/private': fileURLToPath(new URL('./tests/support/envStub.ts', import.meta.url))
|
||||
}
|
||||
},
|
||||
test: {
|
||||
|
||||
Reference in New Issue
Block a user