diff --git a/web/cloudflare-worker/src/csp.ts b/web/cloudflare-worker/src/csp.ts index d418c1ff00f4..5cc62c20acd8 100644 --- a/web/cloudflare-worker/src/csp.ts +++ b/web/cloudflare-worker/src/csp.ts @@ -23,6 +23,9 @@ type DevelopmentOrigins = { */ const DEVELOPMENT_OLDDOT_ORIGIN = 'https://www.expensify.com.dev'; +/** PNG exports fetch uploaded avatars directly from the local S3 mock. */ +const DEVELOPMENT_S3_ORIGIN = 'https://expensify.com.dev:7070'; + /** Inline scripts allowed without a nonce. Tests check that every such script in `web/index.html` is listed. */ const INLINE_SCRIPT_HASHES = [ 'sha256-hK550RmP3t+9myNCpVty39wCs9CDUTXjWUP30rrXvD0=', @@ -91,6 +94,8 @@ function buildCSP({fontSrcHosts = [], frameSrcHosts = [], scriptSrcHashes = [], 'https://cdn.expensify.com', 'https://cdn.ketchjs.com', 'https://connect.facebook.net', + // PNG exports fetch uploaded avatars rather than only displaying them. + 'https://d1wpcgnaa73g0y.cloudfront.net', 'https://edge.fullstory.com', 'https://events.mapbox.com', 'https://global.ketchcdn.com', @@ -192,7 +197,7 @@ function getCSP(environment: Env['ENVIRONMENT'] | DevelopmentEnv['ENVIRONMENT'], fontSrcHosts: ["'self'"], frameSrcHosts: backendOrigins, scriptSrcHashes: INLINE_SCRIPT_HASHES, - connectSrcHosts: [...backendOrigins, ...(developmentOrigins ? [developmentOrigins.hmrOrigin] : [])], + connectSrcHosts: [...backendOrigins, DEVELOPMENT_S3_ORIGIN, ...(developmentOrigins ? [developmentOrigins.hmrOrigin] : [])], // 'self' covers attachments the Rsbuild proxy serves from the app origin. mediaSrcHosts: ["'self'", ...backendOrigins], }); diff --git a/web/cloudflare-worker/tests/csp.test.ts b/web/cloudflare-worker/tests/csp.test.ts new file mode 100644 index 000000000000..a7a460c73ae7 --- /dev/null +++ b/web/cloudflare-worker/tests/csp.test.ts @@ -0,0 +1,45 @@ +/** PNG exports fetch avatars, so the document CSP must allow more than displaying them. */ +import assert from 'node:assert/strict'; +import {after, before, describe, test} from 'node:test'; + +import createWorkerHarness from './createWorkerHarness.ts'; + +for (const environment of ['staging', 'production'] as const) { + describe(`${environment} avatar export CSP`, () => { + const {server, cleanup} = createWorkerHarness(environment); + + before(async () => { + await server.listen(); + }); + + after(async () => { + await cleanup(); + }); + + test('allows uploaded-avatar fetches without allowing every CloudFront origin', async () => { + // Given a browser opening the app in this environment + // When the Worker serves the document's policy + const response = await server.fetch('/', {headers: {'MF-Sec-Fetch-Mode': 'navigate'}}); + const csp = response.headers.get('Content-Security-Policy') ?? ''; + const connectSources = csp + .split('; ') + .find((directive) => directive.startsWith('connect-src ')) + ?.split(' ') + .slice(1); + const scriptSources = csp + .split('; ') + .find((directive) => directive.startsWith('script-src ')) + ?.split(' ') + .slice(1); + + // Then exports can fetch uploaded avatars without allowing unrelated CloudFront hosts, local mocks or scripts + assert.ok(connectSources); + assert.ok(connectSources.includes('https://d1wpcgnaa73g0y.cloudfront.net')); + assert.ok(!connectSources.includes('https://*.cloudfront.net')); + assert.ok(!connectSources.includes('*')); + assert.ok(!connectSources.includes('https://expensify.com.dev:7070')); + assert.ok(scriptSources); + assert.ok(!scriptSources.includes('https://d1wpcgnaa73g0y.cloudfront.net')); + }); + }); +} diff --git a/web/cloudflare-worker/tests/development.test.ts b/web/cloudflare-worker/tests/development.test.ts index b380922dd929..b4d48bd910e7 100644 --- a/web/cloudflare-worker/tests/development.test.ts +++ b/web/cloudflare-worker/tests/development.test.ts @@ -166,6 +166,19 @@ describe('live development worker', () => { assert.doesNotMatch(csp, /script-src[^;]*'unsafe-inline'|connect-src \*/); }); + test('allows mock S3 avatar fetches only through connect-src', async () => { + // Given uploaded avatars served by the local S3 mock rather than CloudFront + const mockS3Origin = 'https://expensify.com.dev:7070'; + // When the Worker serves the development policy used by PNG exports + const csp = (await server.fetch('/')).headers.get('Content-Security-Policy') ?? ''; + const getSources = (directive: string) => new RegExp(`(?:^|; )${directive} ([^;]*)`).exec(csp)?.[1].split(' ') ?? []; + // Then exports can fetch those avatars without allowing scripts, frames or media from the mock + assert.ok(getSources('connect-src').includes(mockS3Origin)); + for (const directive of ['script-src', 'frame-src', 'media-src']) { + assert.ok(!getSources(directive).includes(mockS3Origin)); + } + }); + test('adds only configured backends and the OldDot origin the app hardcodes', async () => { // Given a backend from .env, and the app's hardcoded development OldDot URL (CONST.INTERNAL_DEV_EXPENSIFY_URL) // When the Worker serves a page