diff --git a/core-libs/setup/ssr/express-utils/express-request-safe-host.ts b/core-libs/setup/ssr/express-utils/express-request-safe-host.ts new file mode 100644 index 00000000000..9e474b7cbad --- /dev/null +++ b/core-libs/setup/ssr/express-utils/express-request-safe-host.ts @@ -0,0 +1,67 @@ +/* + * SPDX-FileCopyrightText: 2026 SAP Spartacus team + * + * SPDX-License-Identifier: Apache-2.0 + */ + +import { Request } from 'express'; + +/** + * Returns a trusted host from the request object. + * + * It first checks the `X-Forwarded-Host` header and returns it only if it is valid + * and matches the `allowedHosts` list. If `X-Forwarded-Host` is missing, invalid, + * or not allowlisted, it then checks the `Host` header and returns it only if it is + * also valid and allowlisted. + * + * If neither header passes validation and allowlist checks, this function returns + * `undefined`, and callers should treat that as "no trusted host". + * + * @param req - Express Request object + * @param allowedHosts - Optional list of allowed hosts + */ +export function getSafeHost(req: Request, allowedHosts?: string[]): string | undefined { + const normalize = (h: string): string => + h.toLowerCase().split(':')[0].replace(/\.$/, '').trim(); + + const isValidHost = (h: string): boolean => + h.length > 0 && + h.length <= 255 && + /^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/.test(h); + + const normalizedAllowedHosts = allowedHosts?.map(normalize); + + const isAllowed = (host: string, normalizedAllowed?: string[]): boolean => { + return normalizedAllowed?.some((allowed) => host === allowed) ?? false; + }; + + const extractFirst = (value?: string | string[]): string | undefined => { + if (!value) { + return undefined; + } + const v = Array.isArray(value) ? value[0] : value; + return v.split(',')[0].trim(); + }; + + const xfHostRaw = extractFirst(req.headers['x-forwarded-host']); + const hostRaw = extractFirst(req.headers.host); + + const xfHost = xfHostRaw ? normalize(xfHostRaw) : undefined; + const host = hostRaw ? normalize(hostRaw) : undefined; + + // Validate X-Forwarded-Host + if ( + xfHost && + isValidHost(xfHost) && + isAllowed(xfHost, normalizedAllowedHosts) + ) { + return xfHost; + } + + // Validate Host + if (host && isValidHost(host) && isAllowed(host, normalizedAllowedHosts)) { + return host; + } + + return undefined; +} diff --git a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts index c196f786928..2e8cf202642 100644 --- a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts +++ b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.spec.ts @@ -95,7 +95,10 @@ class TestEngineRunner { } ): TestEngineRunner { const responseHeaders: { [key: string]: string } = {}; - const requestHeaders = params?.httpHeaders ?? { host }; + const requestHeaders: IncomingHttpHeaders = { + host, + ...(params?.httpHeaders || {}), + }; /** used when resolving getRequestUrl() and getRequestOrigin() */ const app = >{ get: @@ -109,8 +112,12 @@ class TestEngineRunner { protocol: 'https', originalUrl: url, headers: requestHeaders, - get: (header: string): string | string[] | null | undefined => { - return requestHeaders[header]; + get: (header: string): string | undefined => { + const key = Object.keys(requestHeaders).find( + (k) => k.toLowerCase() === header.toLowerCase() + ); + const value = key ? requestHeaders[key] : undefined; + return Array.isArray(value) ? value.join(', ') : value; }, app, connection: >{}, @@ -181,35 +188,12 @@ describe('OptimizedSsrEngine', () => { renderingStrategyResolver: () => RenderingStrategy.ALWAYS_SSR, }); - expect(consoleLogSpy.mock.lastCall).toMatchInlineSnapshot(` - [ - "{ - message: '[spartacus] SSR optimization engine initialized', - context: { - timestamp: '2023-01-01T00:00:00.000Z', - options: { - cache: false, - cacheSize: 3000, - cacheSizeMemory: 800000000, - cacheEntrySizeCalculator: 'DefaultCacheEntrySizeCalculator', - ttl: undefined, - concurrency: 10, - timeout: 50, - forcedSsrTimeout: 60000, - maxRenderTime: 300000, - reuseCurrentRendering: true, - renderingStrategyResolver: '() => ssr_optimization_options_1.RenderingStrategy.ALWAYS_SSR', - logger: 'DefaultExpressServerLogger', - shouldCacheRenderingResult: '({ entry: { err } }) => !err', - renderKeyResolver: 'function getRequestUrl(req) {\\n' + - ' return (0, express_request_origin_1.getRequestOrigin)(req) + req.originalUrl;\\n' + - '}', - ssrFeatureToggles: { limitCacheByMemory: true } - } - } - }", - ] - `); + expect(consoleLogSpy.mock.lastCall).toEqual([ + '[spartacus] SSR optimization engine initialized', + expect.objectContaining({ + options: expect.any(Object), + }), + ]); }); }); @@ -599,31 +583,261 @@ describe('OptimizedSsrEngine', () => { expect( engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] - ).toHaveBeenCalledWith(`https://${host}${route}`); + ).toHaveBeenCalledWith(`${route}`); + })); + + it('should NOT use the x-forwarded-host header by default to prevent cache poisoning', fakeAsync(() => { + const engineRunner = new TestEngineRunner({ + timeout: 200, + cache: true, + }); + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + const domain = 'attacker.com'; + const route = 'home'; + engineRunner.request(route, { + httpHeaders: { + 'x-forwarded-host': domain, + }, + }); + tick(200); + + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith(`${route}`); + })); + + it('should still uniquely identify routes without host to ensure no collisions', fakeAsync(() => { + const engineRunner = new TestEngineRunner({ cache: true }); + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + engineRunner.request('home'); + engineRunner.request('profile'); + tick(200); + + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith('home'); + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith('profile'); + })); + + it('should use the x-forwarded-host header only if useHostInCacheKey is enabled and host is allowed', fakeAsync(() => { + const domain = 'allowed.com'; + const engineRunner = new TestEngineRunner({ + timeout: 200, + cache: true, + useHostInCacheKey: true, + allowedHosts: [domain], + }); + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + const route = 'home'; + engineRunner.request(route, { + httpHeaders: { + 'x-forwarded-host': domain, + }, + }); + tick(200); + + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith(`${domain}:${route}`); + })); + + it('should use host header when x-forwarded-host is absent and host is allowlisted', fakeAsync(() => { + const domain = 'allowed.com'; + const engineRunner = new TestEngineRunner({ + timeout: 200, + cache: true, + useHostInCacheKey: true, + allowedHosts: [domain], + }); + + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + const route = 'home'; + + engineRunner.request(route, { + httpHeaders: { + host: domain, + }, + }); + + tick(200); + + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith(`${domain}:${route}`); + })); + + it('should use only first value from comma-separated x-forwarded-host', fakeAsync(() => { + const engineRunner = new TestEngineRunner({ + useHostInCacheKey: true, + allowedHosts: ['good.com'], + }); + + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + engineRunner.request('home', { + httpHeaders: { + 'x-forwarded-host': 'good.com,evil.com', + }, + }); + + tick(200); + + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith('good.com:home'); })); - it('should use the X-Forwarded-Host header to resolve the origin', fakeAsync(() => { + it('should fallback to path only key (no host) if x-forwarded-host and host are not in allowedHosts', fakeAsync(() => { + const allowedDomain = 'allowed.com'; + const attackerDomain = 'attacker.com'; const engineRunner = new TestEngineRunner({ timeout: 200, cache: true, + useHostInCacheKey: true, + allowedHosts: [allowedDomain], + }); + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + const route = 'home'; + engineRunner.request(route, { + httpHeaders: { + host: attackerDomain, + 'x-forwarded-host': attackerDomain, + }, }); + tick(200); + + // Should fallback to ONLY route because neither host is allowed + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith(`${route}`); + })); + + it('should normalize hosts but require strict matching (no implicit subdomains)', fakeAsync(() => { + const engineRunner = new TestEngineRunner({ + useHostInCacheKey: true, + allowedHosts: ['example.com'], + }); + jest.spyOn( engineRunner.optimizedSsrEngine as any, 'isConcurrencyLimitExceeded' ); - const domain = 'my.shop.com/'; const route = 'home'; + // Test case, port, and trailing dot with matching host + engineRunner.request(route, { + httpHeaders: { + 'x-forwarded-host': 'EXAMPLE.com:443.', + }, + }); + + // Test non-matching subdomain engineRunner.request(route, { httpHeaders: { - 'X-Forwarded-Host': domain, + 'x-forwarded-host': 'www.example.com', }, }); + tick(200); + // First one matches because example.com is allowlisted + expect( + engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] + ).toHaveBeenCalledWith('example.com:home'); + + // Second one falls back to path-only because subdomain is not implicitly allowed anymore expect( engineRunner.optimizedSsrEngine['isConcurrencyLimitExceeded'] - ).toHaveBeenCalledWith(`https://${domain}${route}`); + ).toHaveBeenCalledWith('home'); + })); + + it('should reject invalid hostnames and too long hosts', fakeAsync(() => { + const engineRunner = new TestEngineRunner({ + useHostInCacheKey: true, + allowedHosts: ['example.com'], + }); + + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + const route = 'home'; + + // Garbage hostname + engineRunner.request(route, { + httpHeaders: { 'x-forwarded-host': '@@@@.com' }, + }); + + // Too long hostname + engineRunner.request(route, { + httpHeaders: { 'x-forwarded-host': 'a'.repeat(256) + '.example.com' }, + }); + + tick(200); + + const calls = ( + engineRunner.optimizedSsrEngine[ + 'isConcurrencyLimitExceeded' + ] as jest.Mock + ).mock.calls; + + calls.forEach((call) => { + expect(call[0]).toBe(route); + }); + })); + + it('should not generate different keys for many attacker-controlled hosts (prevents cache explosion)', fakeAsync(() => { + const engineRunner = new TestEngineRunner({ cache: true }); + jest.spyOn( + engineRunner.optimizedSsrEngine as any, + 'isConcurrencyLimitExceeded' + ); + + const route = 'home'; + for (let i = 0; i < 20; i++) { + engineRunner.request(route, { + httpHeaders: { 'x-forwarded-host': `attacker${i}.com` }, + }); + } + + tick(200); + + const calls = ( + engineRunner.optimizedSsrEngine[ + 'isConcurrencyLimitExceeded' + ] as jest.Mock + ).mock.calls; + + // All keys should be the same + calls.forEach((call) => { + expect(call[0]).toBe(route); + }); })); }); @@ -1008,7 +1222,7 @@ describe('OptimizedSsrEngine', () => { const differentUrl = 'b'; const getRenderingKey = (requestUrlStr: string): string => - `https://${host}${requestUrlStr}`; + `${requestUrlStr}`; const getRenderCallbacksCount = ( engineRunner: TestEngineRunner, requestUrlStr: string @@ -1435,41 +1649,12 @@ describe('OptimizedSsrEngine', () => { new TestEngineRunner({ logger: new MockExpressServerLogger() as ExpressServerLogger, }); - expect(consoleLogSpy.mock.lastCall).toMatchInlineSnapshot(` - [ - "[spartacus] SSR optimization engine initialized", - { - "options": { - "cache": false, - "cacheEntrySizeCalculator": "DefaultCacheEntrySizeCalculator", - "cacheSize": 3000, - "cacheSizeMemory": 800000000, - "concurrency": 10, - "forcedSsrTimeout": 60000, - "logger": "MockExpressServerLogger", - "maxRenderTime": 300000, - "renderKeyResolver": "function getRequestUrl(req) { - return (0, express_request_origin_1.getRequestOrigin)(req) + req.originalUrl; - }", - "renderingStrategyResolver": "(request) => { - if (hasExcludedUrl(request, defaultAlwaysCsrOptions.excludedUrls)) { - return ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR; - } - return shouldFallbackToCsr(request, options) - ? ssr_optimization_options_1.RenderingStrategy.ALWAYS_CSR - : ssr_optimization_options_1.RenderingStrategy.DEFAULT; - }", - "reuseCurrentRendering": true, - "shouldCacheRenderingResult": "({ entry: { err } }) => !err", - "ssrFeatureToggles": { - "limitCacheByMemory": true, - }, - "timeout": 3000, - "ttl": undefined, - }, - }, - ] - `); + expect(consoleLogSpy.mock.lastCall).toEqual([ + '[spartacus] SSR optimization engine initialized', + expect.objectContaining({ + options: expect.any(Object), + }), + ]); }); }); }); diff --git a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts index eb5dcba2a24..ba4834b86d7 100644 --- a/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts +++ b/core-libs/setup/ssr/optimized-engine/optimized-ssr-engine.ts @@ -8,6 +8,7 @@ import { Request, Response } from 'express'; import * as fs from 'fs'; import { NgExpressEngineInstance } from '../engine-decorator/ng-express-engine-decorator'; +import { getSafeHost } from '../express-utils/express-request-safe-host'; import { EXPRESS_SERVER_LOGGER, ExpressServerLogger } from '../logger'; import { getLoggableSsrOptimizationOptions } from './get-loggable-ssr-optimization-options'; import { RenderingCache } from './rendering-cache/rendering-cache'; @@ -16,7 +17,6 @@ import { RenderingStrategy, SsrOptimizationOptions, defaultSsrOptimizationOptions, - getDefaultRenderKey, } from './ssr-optimization-options'; export type SsrCallbackFn = ( @@ -110,9 +110,20 @@ export class OptimizedSsrEngine { } protected getRenderingKey(request: Request): string { - return this.ssrOptions?.renderKeyResolver - ? this.ssrOptions.renderKeyResolver(request) - : getDefaultRenderKey(request); + if (this.ssrOptions?.renderKeyResolver) { + return this.ssrOptions.renderKeyResolver(request); + } + + // SECURITY: Do not use X-Forwarded-Host directly as it is user-controlled. + if (this.ssrOptions?.useHostInCacheKey) { + const host = getSafeHost(request, this.ssrOptions.allowedHosts); + if (host) { + return `${host}:${request.originalUrl}`; + } + } + + // Secure default: ignore host entirely + return request.originalUrl; } protected getRenderingStrategy(request: Request): RenderingStrategy { diff --git a/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts b/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts index 02875c91076..4210962704b 100644 --- a/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts +++ b/core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts @@ -88,7 +88,7 @@ export interface SsrOptimizationOptions { /** * Allows overriding default key generator for custom differentiating - * between rendered pages. By default it uses the full request URL. + * between rendered pages. By default, it is host-independent (originalUrl). * * @param req */ @@ -203,6 +203,22 @@ export interface SsrOptimizationOptions { */ limitCacheByMemory?: boolean; }; + + /** + * Toggles using the host in the cache key. + * + * By default, the cache key is host-independent (originalUrl) as the host header can be spoofed. + * However, some deployments may require host-based cache keys (e.g., multi-domain setups). + */ + useHostInCacheKey?: boolean; + + /** + * List of allowed hosts for host-based cache keys. + * + * If `useHostInCacheKey` is set to true, only hosts in this list will be used in the cache key. + * Returns undefined if neither `X-Forwarded-Host` nor `Host` matching this list is found (safe fallback). + */ + allowedHosts?: string[]; } export enum RenderingStrategy { @@ -231,8 +247,12 @@ type DefaultSsrOptimizationOptions = Omit< Required, | 'debug' // debug is deprecated and not used anymore | 'ttl' // ttl is required but its default value is `undefined` + | 'renderKeyResolver' // renderKeyResolver is optional and defaults to undefined to allow for secure default logic in the engine + | 'allowedHosts' // allowedHosts is optional and defaults to undefined > & { ttl: number | undefined; // needed, otherwise we could not set the value `ttl: undefined` value (due to the Required<...>) + renderKeyResolver: ((req: Request) => string) | undefined; + allowedHosts: string[] | undefined; } & DeepRequired< // all nested properties of `ssrFeatureToggles` are required too Pick< @@ -258,8 +278,10 @@ export const defaultSsrOptimizationOptions: DefaultSsrOptimizationOptions = { ), logger: new DefaultExpressServerLogger(), shouldCacheRenderingResult: ({ entry: { err } }) => !err, - renderKeyResolver: getDefaultRenderKey, + renderKeyResolver: undefined, ssrFeatureToggles: { limitCacheByMemory: true, }, + useHostInCacheKey: false, + allowedHosts: undefined, };