From 40834da9f0fbc8904962466df1c69d94ce587f5a Mon Sep 17 00:00:00 2001 From: Seranged <80223622+Seranged@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:19:08 +0100 Subject: [PATCH] fix: keep VPN usage as audit metadata Continue address screening for VPN users and record unavailable VPN measurements as unknown. Preserve country restrictions and fail-closed address screening. --- composables/useAddressScreen.ts | 10 +---- docs/architecture.md | 2 +- services/screening.ts | 2 +- services/vpn.ts | 17 +++++---- tests/composables/useAddressScreen.test.ts | 16 ++++---- tests/services/screening.test.ts | 10 +++-- tests/services/vpn.test.ts | 43 +++++++++++++++++++--- 7 files changed, 64 insertions(+), 36 deletions(-) diff --git a/composables/useAddressScreen.ts b/composables/useAddressScreen.ts index 5fe2c3840..cf17f3006 100644 --- a/composables/useAddressScreen.ts +++ b/composables/useAddressScreen.ts @@ -50,15 +50,7 @@ export const useAddressScreen = () => { const vpnIsUsed = await detectVpn() if (gen !== screeningGeneration) return false - // A positive local signal is independently blocking. A clean or failed - // remote address-screen response must never erase it. - if (vpnIsUsed) { - await disconnect() - if (gen !== screeningGeneration) return false - showBlockedModal(address) - return true - } - + // VPN usage is audit metadata; only the address-screening verdict gates access. const isRestricted = await screenAddress(address, vpnIsUsed) if (gen !== screeningGeneration) return false diff --git a/docs/architecture.md b/docs/architecture.md index fdb1f5c5a..2a6e8ddad 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -357,7 +357,7 @@ The app includes a built-in per-IP rate limiter as a defense-in-depth measure. D - **Tenderly simulate**: 10 requests - **Address screening**: 10 requests -**Wallet screening fail-closed**: `server/api/internal/screen-address.post.ts` proxies address checks to the data-v3 compliance API (configured via `ADDRESS_SCREENING_URI` + `ADDRESS_SCREENING_API_KEY`; the shared upstream logic lives in `server/utils/screening.ts`). With BOTH env vars unset the deployment is treated as having no screening provider (a fork, typically) and every address passes — except in production (`DOPPLER_ENVIRONMENT=prd`, the same convention the CORS/geo/rate middleware use), where an absent configuration is a failed secret injection rather than an opt-out and screening fails closed. Once either var is set, the path is fail-closed: partial configuration, a non-https `ADDRESS_SCREENING_URI` (plain http is tolerated for localhost/127.0.0.1 only — the restricted key must not travel without TLS), upstream errors, timeouts, redirects, address-mismatched or malformed verdicts all return `addressIsSuspicious: true`. A strict client `vpnIsUsed: true` is an additional positive signal; client false or malformed values cannot clear trusted edge VPN headers, and the client also blocks its own positive local verdict without waiting for a remote clean result. Operators of screened deployments must therefore set both vars — and be aware that removing both silently disables screening, production refuses to run unscreened, and non-production monitoring can watch the screening-disabled log line. The route is also consumed cross-origin by first-party `*.euler.finance` SPAs that have no server of their own — `server/middleware/cors.ts` carries a CORS exception scoped to exactly this path, so no other internal route is exposed to sibling apps. It deliberately stays under `/api/internal/` (not `/api/public/`): the consumers are our own apps, and the public prefix would advertise it to external integrators. Because of these external first-party consumers, changes to this route's request/response contract must stay backward-compatible. +**Wallet screening fail-closed**: `server/api/internal/screen-address.post.ts` proxies address checks to the data-v3 compliance API (configured via `ADDRESS_SCREENING_URI` + `ADDRESS_SCREENING_API_KEY`; the shared upstream logic lives in `server/utils/screening.ts`). With BOTH env vars unset the deployment is treated as having no screening provider (a fork, typically) and every address passes — except in production (`DOPPLER_ENVIRONMENT=prd`, the same convention the CORS/geo/rate middleware use), where an absent configuration is a failed secret injection rather than an opt-out and screening fails closed. Once either var is set, the path is fail-closed: partial configuration, a non-https `ADDRESS_SCREENING_URI` (plain http is tolerated for localhost/127.0.0.1 only — the restricted key must not travel without TLS), upstream errors, timeouts, redirects, address-mismatched or malformed verdicts all return `addressIsSuspicious: true`. VPN usage is audit metadata and does not gate wallet access: the client always continues to address screening, including when VPN usage is detected or the VPN probe fails. The client reports missing, invalid, failed, or unsupported VPN measurements as `null`. A strict client `vpnIsUsed: true` contributes positive audit evidence; client false or malformed values cannot clear trusted edge VPN headers. Operators of screened deployments must therefore set both vars — and be aware that removing both silently disables screening, production refuses to run unscreened, and non-production monitoring can watch the screening-disabled log line. The route is also consumed cross-origin by first-party `*.euler.finance` SPAs that have no server of their own — `server/middleware/cors.ts` carries a CORS exception scoped to exactly this path, so no other internal route is exposed to sibling apps. It deliberately stays under `/api/internal/` (not `/api/public/`): the consumers are our own apps, and the public prefix would advertise it to external integrators. Because of these external first-party consumers, changes to this route's request/response contract must stay backward-compatible. **Important**: This is a best-effort safeguard, not a security boundary. It catches accidental abuse (e.g. a client stuck in a retry loop) but will not stop a determined attacker. Known limitations: diff --git a/services/screening.ts b/services/screening.ts index 760fe0201..55d9325c0 100644 --- a/services/screening.ts +++ b/services/screening.ts @@ -2,7 +2,7 @@ import { WALLET_SCREENING_TIMEOUT_MS } from '~/entities/tuning-constants' export async function screenAddress( address: string, - vpnIsUsed: boolean, + vpnIsUsed: boolean | null, ): Promise { if (!address) return false diff --git a/services/vpn.ts b/services/vpn.ts index 4169c2291..c6f29062f 100644 --- a/services/vpn.ts +++ b/services/vpn.ts @@ -1,20 +1,20 @@ import { CACHE_TTL_5MIN_MS, WALLET_SCREENING_TIMEOUT_MS } from '~/entities/tuning-constants' -let cached: { value: boolean, timestamp: number } | null = null +let cached: { value: boolean | null, timestamp: number } | null = null // Whether the deployment's edge provider measures VPN usage at all, // injected by server/plugins/app-config.ts. When absent or false (edges // without VPN evidence, forks, static deploys) probing would only produce -// noise — the server derives its verdict from edge request headers, and only -// a strict client `true` can add to it (never clear it). +// noise. VPN evidence is audit metadata, not an access verdict; the server +// combines positive client evidence with its own edge request headers. function edgeProvidesVpnEvidence(): boolean { if (typeof window === 'undefined') return false return window.__APP_CONFIG__?.vpnDetection === true } -export async function detectVpn(): Promise { +export async function detectVpn(): Promise { if (!edgeProvidesVpnEvidence()) { - return false + return null } if (cached !== null && Date.now() - cached.timestamp < CACHE_TTL_5MIN_MS) { @@ -26,11 +26,12 @@ export async function detectVpn(): Promise { try { const resp = await fetch(window.location.origin, { method: 'HEAD', signal: controller.signal }) - const header = resp.headers.get('x-is-vpn') - cached = { value: header === 'true', timestamp: Date.now() } + const header = resp.ok ? resp.headers.get('x-is-vpn')?.trim().toLowerCase() : null + const value = header === 'true' ? true : header === 'false' ? false : null + cached = { value, timestamp: Date.now() } } catch { - cached = { value: true, timestamp: Date.now() } + cached = { value: null, timestamp: Date.now() } } finally { clearTimeout(timeout) diff --git a/tests/composables/useAddressScreen.test.ts b/tests/composables/useAddressScreen.test.ts index 9bffdc34e..3f9e8763f 100644 --- a/tests/composables/useAddressScreen.test.ts +++ b/tests/composables/useAddressScreen.test.ts @@ -80,8 +80,8 @@ describe('useAddressScreen', () => { expect(screening.isAddressScreened(USER)).toBe(true) }) - it('disconnects restricted addresses without marking them screened', async () => { - mocks.detectVpn.mockResolvedValue(false) + it.each([true, false, null])('disconnects restricted addresses regardless of VPN evidence (%s)', async (vpnIsUsed) => { + mocks.detectVpn.mockResolvedValue(vpnIsUsed) mocks.screenAddress.mockResolvedValue(true) const screening = useAddressScreen() @@ -92,17 +92,17 @@ describe('useAddressScreen', () => { expect(screening.isAddressScreened(USER)).toBe(false) }) - it('blocks a positive local VPN verdict without letting remote screening clear it', async () => { - mocks.detectVpn.mockResolvedValue(true) + it.each([true, false, null])('allows a screened address regardless of VPN evidence (%s)', async (vpnIsUsed) => { + mocks.detectVpn.mockResolvedValue(vpnIsUsed) mocks.screenAddress.mockResolvedValue(false) const screening = useAddressScreen() await screening.screenConnectedAddress(USER) - expect(mocks.screenAddress).not.toHaveBeenCalled() - expect(mocks.disconnect).toHaveBeenCalledTimes(1) - expect(mocks.modalOpen).toHaveBeenCalledTimes(1) - expect(screening.isAddressScreened(USER)).toBe(false) + expect(mocks.screenAddress).toHaveBeenCalledWith(USER, vpnIsUsed) + expect(mocks.disconnect).not.toHaveBeenCalled() + expect(mocks.modalOpen).not.toHaveBeenCalled() + expect(screening.isAddressScreened(USER)).toBe(true) }) it('invalidates a pending verdict when screening state is reset', async () => { diff --git a/tests/services/screening.test.ts b/tests/services/screening.test.ts index 3cbf9e4fd..c88619a20 100644 --- a/tests/services/screening.test.ts +++ b/tests/services/screening.test.ts @@ -10,10 +10,14 @@ describe('screenAddress', () => { vi.unstubAllGlobals() }) - it('allows only an explicit false suspicious verdict', async () => { - vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ addressIsSuspicious: false }), { status: 200 }))) + it.each([true, false, null])('forwards VPN audit evidence (%s) and allows an explicit clean verdict', async (vpnIsUsed) => { + const fetchMock = vi.fn(async () => new Response(JSON.stringify({ addressIsSuspicious: false }), { status: 200 })) + vi.stubGlobal('fetch', fetchMock) - await expect(screenAddress(USER, false)).resolves.toBe(false) + await expect(screenAddress(USER, vpnIsUsed)).resolves.toBe(false) + expect(fetchMock).toHaveBeenCalledWith('/api/internal/screen-address', expect.objectContaining({ + body: JSON.stringify({ address: USER, vpnIsUsed }), + })) }) it('fails closed for non-ok responses and malformed success bodies', async () => { diff --git a/tests/services/vpn.test.ts b/tests/services/vpn.test.ts index fba481e6c..6d78fada6 100644 --- a/tests/services/vpn.test.ts +++ b/tests/services/vpn.test.ts @@ -18,17 +18,48 @@ describe('detectVpn', () => { vi.unstubAllGlobals() }) - it('reads the VPN edge header', async () => { + it.each([ + ['true', true], + ['false', false], + [' TRUE ', true], + [' FALSE ', false], + ['', null], + ['unknown', null], + ] as const)('records header %j as %s', async (header, expected) => { stubWindow(true) vi.stubGlobal('fetch', vi.fn(async () => new Response(null, { - headers: { 'x-is-vpn': 'true' }, + headers: { 'x-is-vpn': header }, status: 200, }))) - await expect(detectVpn()).resolves.toBe(true) + await expect(detectVpn()).resolves.toBe(expected) + }) + + it('records a missing header as unknown', async () => { + stubWindow(true) + vi.stubGlobal('fetch', vi.fn(async () => new Response(null, { status: 200 }))) + + await expect(detectVpn()).resolves.toBeNull() + }) + + it('records an unsuccessful HTTP response as unknown even with a VPN header', async () => { + stubWindow(true) + vi.stubGlobal('fetch', vi.fn(async () => new Response(null, { + headers: { 'x-is-vpn': 'true' }, + status: 503, + }))) + + await expect(detectVpn()).resolves.toBeNull() + }) + + it('records a network failure as unknown', async () => { + stubWindow(true) + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) + + await expect(detectVpn()).resolves.toBeNull() }) - it('fails closed when VPN detection stalls', async () => { + it('records a timeout as unknown', async () => { vi.useFakeTimers() stubWindow(true) vi.stubGlobal('fetch', vi.fn((_url: string, init?: RequestInit) => @@ -41,7 +72,7 @@ describe('detectVpn', () => { await vi.advanceTimersByTimeAsync(WALLET_SCREENING_TIMEOUT_MS) - await expect(promise).resolves.toBe(true) + await expect(promise).resolves.toBeNull() }) it('skips the probe entirely when the edge provides no VPN evidence', async () => { @@ -50,7 +81,7 @@ describe('detectVpn', () => { for (const vpnDetection of [false, undefined] as const) { stubWindow(vpnDetection) - await expect(detectVpn()).resolves.toBe(false) + await expect(detectVpn()).resolves.toBeNull() } expect(fetchMock).not.toHaveBeenCalled() })