From 5917ad6f3d8e9f5c60de837d10624230fd699675 Mon Sep 17 00:00:00 2001 From: SkyZeroZx <73321943+SkyZeroZx@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:42:03 -0500 Subject: [PATCH] fix(service-worker): Strips sensitive headers on cross-origin redirects Removes `Authorization`, `Cookie`, and `Proxy-Authorization` headers when a request is redirected to a different origin. This aligns with the Fetch API's redirect algorithm to prevent sensitive information from being sent to third-party origins. (cherry picked from commit 423a1094047909675efe21f63cc732b544920709) --- packages/service-worker/worker/src/adapter.ts | 2 +- packages/service-worker/worker/src/assets.ts | 20 +++++++++-- .../service-worker/worker/test/happy_spec.ts | 35 ++++++++++++++++-- .../service-worker/worker/testing/fetch.ts | 36 +++++++++---------- .../service-worker/worker/testing/scope.ts | 7 ++-- 5 files changed, 71 insertions(+), 29 deletions(-) diff --git a/packages/service-worker/worker/src/adapter.ts b/packages/service-worker/worker/src/adapter.ts index 63505b1691c1..8b7aa256a6b3 100644 --- a/packages/service-worker/worker/src/adapter.ts +++ b/packages/service-worker/worker/src/adapter.ts @@ -51,7 +51,7 @@ export class Adapter { /** * Wrapper around the `Headers` constructor. */ - newHeaders(headers: {[name: string]: string}): Headers { + newHeaders(headers: HeadersInit): Headers { return new Headers(headers); } diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index 54fb4f66dc60..cec901050b97 100644 --- a/packages/service-worker/worker/src/assets.ts +++ b/packages/service-worker/worker/src/assets.ts @@ -502,7 +502,9 @@ export abstract class AssetGroup { * metadata that are known to be safe. * * Currently, headers, redirect policy, an explicit `credentials: 'omit'`, and the HTTP cache - * mode are preserved. + * mode are preserved. On cross-origin redirects, sensitive headers are removed. This includes + * `Authorization`, as required by the Fetch redirect algorithm, and forbidden request headers + * that could contain credentials. * * NOTE: * `credentials: 'same-origin'` and `credentials: 'include'` are intentionally not preserved. @@ -515,9 +517,21 @@ export abstract class AssetGroup { * Investigate preserving more metadata. See, also, discussion on preserving `mode`: * https://github.com/angular/angular/issues/41931#issuecomment-1227601347. */ - private newRequestWithMetadata(url: string, options: RequestInit): Request { + private newRequestWithMetadata(url: string, options: Request): Request { + let headers = options.headers; + const parsedUrl = this.adapter.parseUrl(url, this.adapter.origin); + + const hasHeaders = headers.keys().next().done !== true; + + if (hasHeaders && parsedUrl.origin !== this.adapter.origin) { + headers = this.adapter.newHeaders(options.headers); + headers.delete('Authorization'); + headers.delete('Proxy-Authorization'); + headers.delete('Cookie'); + } + const init: RequestInit = { - headers: options.headers, + headers, redirect: options.redirect, }; diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts index 09b94ab4349e..2af08b1b8be5 100644 --- a/packages/service-worker/worker/test/happy_spec.ts +++ b/packages/service-worker/worker/test/happy_spec.ts @@ -135,7 +135,7 @@ import {envIsSupported} from '../testing/utils'; name: 'other', installMode: 'lazy', updateMode: 'lazy', - urls: ['/baz.txt', '/qux.txt', '/lazy/redirected.txt'], + urls: ['/baz.txt', '/qux.txt', '/lazy/redirected.txt', '/lazy/cross-origin-redirected.txt'], patterns: [], cacheQueryOptions: {ignoreVary: true}, }, @@ -220,6 +220,11 @@ import {envIsSupported} from '../testing/utils'; .withStaticFiles(dist) .withRedirect('/redirected.txt', '/redirect-target.txt') .withRedirect('/lazy/redirected.txt', '/lazy/redirect-target.txt') + .withRedirect( + '/lazy/cross-origin-redirected.txt', + 'https://example.com/lazy/redirect-target.txt', + ) + .withRedirect('https://example.com/lazy/redirect-target.txt', '/lazy/redirect-target.txt') .withError('/error.txt'); const server = serverBuilderBase.withManifest(manifest).build(); @@ -1684,7 +1689,10 @@ import {envIsSupported} from '../testing/utils'; // Request a redirected, lazy-cached asset (so that it is fetched from the network) and // provide headers. const reqInit = { - headers: {SomeHeader: 'SomeValue'}, + headers: { + Authorization: 'Bearer secret', + SomeHeader: 'SomeValue', + }, }; expect(await makeRequest(scope, '/lazy/redirected.txt', undefined, reqInit)).toBe( 'this was a redirect too', @@ -1692,6 +1700,29 @@ import {envIsSupported} from '../testing/utils'; // Verify that the headers were passed through to the network. const [redirectReq] = server.getRequestsFor('/lazy/redirect-target.txt'); + expect(redirectReq.headers.get('Authorization')).toBe('Bearer secret'); + expect(redirectReq.headers.get('SomeHeader')).toBe('SomeValue'); + }); + + it('does not pass sensitive headers through to a different origin', async () => { + const reqInit = { + headers: { + Authorization: 'Bearer secret', + Cookie: 'session=secret', + 'Proxy-Authorization': 'Basic secret', + SomeHeader: 'SomeValue', + }, + }; + expect( + await makeRequest(scope, '/lazy/cross-origin-redirected.txt', undefined, reqInit), + ).toBe('this was a redirect too'); + + const [redirectReq] = server.getRequestsFor( + 'https://example.com/lazy/redirect-target.txt', + ); + expect(redirectReq.headers.get('Authorization')).toBeNull(); + expect(redirectReq.headers.get('Cookie')).toBeNull(); + expect(redirectReq.headers.get('Proxy-Authorization')).toBeNull(); expect(redirectReq.headers.get('SomeHeader')).toBe('SomeValue'); }); diff --git a/packages/service-worker/worker/testing/fetch.ts b/packages/service-worker/worker/testing/fetch.ts index 1f815436aa18..3e5997ba744c 100644 --- a/packages/service-worker/worker/testing/fetch.ts +++ b/packages/service-worker/worker/testing/fetch.ts @@ -59,6 +59,20 @@ export class MockBody implements Body { export class MockHeaders implements Headers { map = new Map(); + constructor(headers?: HeadersInit) { + if (headers === undefined) { + return; + } + + if (Array.isArray(headers)) { + headers.forEach(([name, value]) => this.set(name, value)); + } else if (headers instanceof MockHeaders || headers instanceof Headers) { + headers.forEach((value, name) => this.set(name, value)); + } else { + Object.entries(headers).forEach(([name, value]) => this.set(name, value)); + } + } + [Symbol.iterator]() { return this.map[Symbol.iterator](); } @@ -131,15 +145,8 @@ export class MockRequest extends MockBody implements Request { throw 'Not implemented'; } this.url = input; - const headers = init.headers as {[key: string]: string}; - if (headers !== undefined) { - if (headers instanceof MockHeaders) { - this.headers = headers; - } else { - Object.keys(headers).forEach((header) => { - this.headers.set(header, headers[header]); - }); - } + if (init.headers !== undefined) { + this.headers = new MockHeaders(init.headers); } if (init.cache !== undefined) { this.cache = init.cache; @@ -195,15 +202,8 @@ export class MockResponse extends MockBody implements Response { super(typeof body === 'string' ? body : null); this.status = init.status !== undefined ? init.status : 200; this.statusText = init.statusText !== undefined ? init.statusText : 'OK'; - const headers = init.headers as {[key: string]: string}; - if (headers !== undefined) { - if (headers instanceof MockHeaders) { - this.headers = headers; - } else { - Object.keys(headers).forEach((header) => { - this.headers.set(header, headers[header]); - }); - } + if (init.headers !== undefined) { + this.headers = new MockHeaders(init.headers); } if (init.type !== undefined) { this.type = init.type; diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts index ec8ea7c51df4..5ce267607596 100644 --- a/packages/service-worker/worker/testing/scope.ts +++ b/packages/service-worker/worker/testing/scope.ts @@ -181,11 +181,8 @@ export class SwTestHarnessImpl return new MockResponse(body, init); } - override newHeaders(headers: {[name: string]: string}): Headers { - return Object.keys(headers).reduce((mock, name) => { - mock.set(name, headers[name]); - return mock; - }, new MockHeaders()); + override newHeaders(headers: HeadersInit): Headers { + return new MockHeaders(headers); } async skipWaiting(): Promise {