From 921a409bf1c434405eafa447dcc024ccb5fa5ddd Mon Sep 17 00:00:00 2001 From: Xi Xu Date: Tue, 5 May 2026 16:30:30 +0800 Subject: [PATCH] fix(cache): harden cache policy guards --- src/config/index.js | 13 +- src/response/finalize-response.js | 8 +- src/upstream/cache-policy.js | 282 ++++++++++++++++++++++++++++- test/unit/cache-privacy.test.js | 53 ++++++ test/unit/pipeline-modules.test.js | 84 +++++++++ test/unit/utils.test.js | 11 ++ 6 files changed, 442 insertions(+), 9 deletions(-) diff --git a/src/config/index.js b/src/config/index.js index 9434f80..8ab1a00 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -18,6 +18,17 @@ import { PLATFORMS } from './platform-catalog.js'; +/** + * Parses an environment value as a positive integer. + * @param {unknown} value Environment value. + * @param {number} fallback Fallback used for missing or invalid values. + * @returns {number} Parsed positive integer or fallback. + */ +function parsePositiveInteger(value, fallback) { + const parsed = parseInt(String(value), 10); + return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback; +} + /** * Security-related configuration options for request validation and CORS. * @typedef {object} SecurityConfig @@ -149,7 +160,7 @@ export function createConfig(env = {}) { TIMEOUT_SECONDS: parseInt(String(env.TIMEOUT_SECONDS), 10) || 30, MAX_RETRIES: parseInt(String(env.MAX_RETRIES), 10) || 3, RETRY_DELAY_MS: parseInt(String(env.RETRY_DELAY_MS), 10) || 1000, - CACHE_DURATION: parseInt(String(env.CACHE_DURATION), 10) || 300, // 5 minutes + CACHE_DURATION: parsePositiveInteger(env.CACHE_DURATION, 300), // 5 minutes SECURITY: { ALLOWED_METHODS: allowedMethods.length ? allowedMethods : ['GET', 'HEAD'], ALLOWED_ORIGINS: allowedOrigins.length ? allowedOrigins : ['*'], diff --git a/src/response/finalize-response.js b/src/response/finalize-response.js index fb3d48d..b0349a2 100644 --- a/src/response/finalize-response.js +++ b/src/response/finalize-response.js @@ -21,7 +21,7 @@ import { rewriteTextResponse, shouldRewriteTextResponse } from '../utils/rewrite.js'; -import { resolveCachePolicy } from '../upstream/cache-policy.js'; +import { resolveCachePolicy, resolveResponseCachePolicy } from '../upstream/cache-policy.js'; import { addSecurityHeaders, createErrorResponse } from '../utils/security.js'; /** @@ -143,7 +143,7 @@ async function finalizeSuccessfulResponse({ headers.set('Content-Length', String(rewrittenContentLength)); } - const cachePolicy = resolveCachePolicy({ + const requestCachePolicy = resolveCachePolicy({ canUseCache, config, effectivePath, @@ -154,6 +154,10 @@ async function finalizeSuccessfulResponse({ requestContext, targetUrl }); + const cachePolicy = resolveResponseCachePolicy({ + basePolicy: requestCachePolicy, + response + }); if (!isGit && !isGitLFS && !isDocker && !isAI && !isHF) { headers.set('Cache-Control', cachePolicy.cacheControl); diff --git a/src/upstream/cache-policy.js b/src/upstream/cache-policy.js index e6e7a78..2053f3c 100644 --- a/src/upstream/cache-policy.js +++ b/src/upstream/cache-policy.js @@ -23,16 +23,187 @@ const MUTABLE_EDGE_TTL_SECONDS = 300; const IMMUTABLE_EDGE_TTL_SECONDS = 86400; const IMMUTABLE_BROWSER_TTL_SECONDS = 3600; -const IMMUTABLE_ARTIFACT_PATTERN = +const GITHUB_RELEASE_ARTIFACT_PATTERN = /\.(?:tgz|whl|jar|zip|gem|crate|deb|rpm|nupkg|tar\.gz|tar\.bz2|tar\.xz)(?:$|[?#])/i; +const MAVEN_ARTIFACT_PATTERN = /\.(?:jar|pom|war|aar|module)(?:$|[?#])/i; +const PYPI_FILE_ARTIFACT_PATTERN = /\.(?:whl|zip|tar\.gz|tar\.bz2|tar\.xz)(?:$|[?#])/i; +const MOVING_VERSION_ALIASES = new Set([ + 'current', + 'dev', + 'edge', + 'latest', + 'main', + 'master', + 'nightly', + 'snapshot', + 'stable' +]); /** - * Checks whether a request path points to versioned or content-addressed package artifacts. + * Extracts a pathname from either a request path or an absolute URL. * @param {string} value Request path or target URL. + * @returns {string} URL pathname without query or fragment. + */ +function getPathname(value) { + try { + return new URL(value).pathname; + } catch { + return value.split(/[?#]/, 1)[0] || ''; + } +} + +/** + * Extracts the last path segment from a pathname. + * @param {string} pathname URL or request pathname. + * @returns {string} Last segment without percent encoding. + */ +function getBasename(pathname) { + const basename = pathname.split('/').filter(Boolean).at(-1) || ''; + try { + return decodeURIComponent(basename); + } catch { + return basename; + } +} + +/** + * Checks whether a segment contains an explicit version instead of a moving alias. + * @param {string} value Path segment or file stem. + * @returns {boolean} True when the value contains a version-like token. + */ +function hasVersionLikeToken(value) { + const normalized = value.toLowerCase(); + if (MOVING_VERSION_ALIASES.has(normalized)) { + return false; + } + + return /(?:^|[-_.])v?\d/.test(normalized); +} + +/** + * Reads the tag segment from a GitHub releases/download path. + * @param {string} pathname URL or request pathname. + * @returns {string | null} Release tag segment when present. + */ +function getGithubReleaseDownloadTag(pathname) { + const segments = pathname.split('/').filter(Boolean); + const releasesIndex = segments.findIndex( + (segment, index) => segment === 'releases' && segments[index + 1] === 'download' + ); + + if (releasesIndex === -1) { + return null; + } + + const tag = segments[releasesIndex + 2]; + if (!tag) { + return null; + } + + try { + return decodeURIComponent(tag); + } catch { + return tag; + } +} + +/** + * Checks whether a GitHub request targets a release asset instead of a branch archive. + * @param {string} effectivePath Normalized request path. + * @param {string} targetPath Upstream target path. + * @returns {boolean} True for release asset downloads. + */ +function isGithubReleaseArtifact(effectivePath, targetPath) { + const effectiveTag = getGithubReleaseDownloadTag(effectivePath); + const targetTag = getGithubReleaseDownloadTag(targetPath); + + return ( + (effectivePath.includes('/releases/download/') || targetPath.includes('/releases/download/')) && + (GITHUB_RELEASE_ARTIFACT_PATTERN.test(effectivePath) || + GITHUB_RELEASE_ARTIFACT_PATTERN.test(targetPath)) && + ((effectiveTag !== null && hasVersionLikeToken(effectiveTag)) || + (targetTag !== null && hasVersionLikeToken(targetTag))) + ); +} + +/** + * Checks whether an npm path points to a package tarball. + * @param {string} pathname URL or request pathname. + * @returns {boolean} True for npm tarball requests. + */ +function isNpmTarballPath(pathname) { + return /^\/(?:npm\/)?(?:@[^/]+\/)?[^/]+\/-\/[^/]+\.tgz$/i.test(pathname); +} + +/** + * Checks whether a Maven path contains a non-SNAPSHOT versioned artifact. + * @param {string} pathname URL or request pathname. + * @returns {boolean} True for Maven release artifacts. + */ +function isMavenReleaseArtifactPath(pathname) { + const segments = pathname.split('/').filter(Boolean); + if (segments.length < 4 || !MAVEN_ARTIFACT_PATTERN.test(pathname)) { + return false; + } + + const version = segments.at(-2) || ''; + return /\d/.test(version) && !version.toUpperCase().includes('SNAPSHOT'); +} + +/** + * Checks whether a PyPI file path points to a versioned distribution artifact. + * @param {string} pathname URL or request pathname. + * @returns {boolean} True for versioned PyPI file artifacts. + */ +function isVersionedPyPIFilePath(pathname) { + if (!PYPI_FILE_ARTIFACT_PATTERN.test(pathname)) { + return false; + } + + const basename = getBasename(pathname); + const stem = basename.replace(/\.(?:whl|zip|tar\.gz|tar\.bz2|tar\.xz)$/i, ''); + + return hasVersionLikeToken(stem); +} + +/** + * Checks whether a request points to an explicitly immutable package or release artifact. + * @param {string} platform Platform key. + * @param {string} effectivePath Normalized request path. + * @param {string} targetUrl Upstream target URL. * @returns {boolean} True when the resource can use long-lived immutable caching. */ -export function isImmutableArtifactPath(value) { - return IMMUTABLE_ARTIFACT_PATTERN.test(value); +export function isImmutableArtifactRequest(platform, effectivePath, targetUrl) { + const effectivePathname = getPathname(effectivePath); + const targetPathname = getPathname(targetUrl); + + if (platform === 'npm') { + return isNpmTarballPath(effectivePathname) || isNpmTarballPath(targetPathname); + } + + if (platform === 'pypi-files') { + return isVersionedPyPIFilePath(effectivePathname) || isVersionedPyPIFilePath(targetPathname); + } + + if (platform === 'maven') { + return ( + isMavenReleaseArtifactPath(effectivePathname) || isMavenReleaseArtifactPath(targetPathname) + ); + } + + if (platform === 'crates') { + return effectivePathname.endsWith('.crate') || targetPathname.endsWith('.crate'); + } + + if (platform === 'rubygems') { + return effectivePathname.endsWith('.gem') || targetPathname.endsWith('.gem'); + } + + if (platform === 'gh') { + return isGithubReleaseArtifact(effectivePathname, targetPathname); + } + + return false; } /** @@ -41,7 +212,7 @@ export function isImmutableArtifactPath(value) { * @returns {boolean} True when npm response rewriting can bind content to request origin. */ function isNpmMetadataPath(effectivePath) { - return effectivePath.startsWith('/npm/') && !isImmutableArtifactPath(effectivePath); + return effectivePath.startsWith('/npm/') && !isImmutableArtifactRequest('npm', effectivePath, ''); } /** @@ -175,7 +346,7 @@ export function resolveCachePolicy({ }; } - if (isImmutableArtifactPath(effectivePath) || isImmutableArtifactPath(targetUrl)) { + if (isImmutableArtifactRequest(platform, effectivePath, targetUrl)) { return { allowCacheApi: request.method === 'GET', allowFetchCache: true, @@ -217,3 +388,102 @@ export function resolveCachePolicy({ varyByOrigin: shouldVaryCacheByOrigin(platform, effectivePath) }; } + +/** + * Checks whether a Cache-Control response header contains a directive. + * @param {string} cacheControl Cache-Control header value. + * @param {string} directive Directive name. + * @returns {boolean} True when the directive is present. + */ +function hasCacheControlDirective(cacheControl, directive) { + return cacheControl + .split(',') + .map(part => part.trim().toLowerCase().split('=', 1)[0]) + .includes(directive); +} + +/** + * Safely reads a response header from standard or test double header objects. + * @param {Headers} headers Response headers. + * @param {string} name Header name. + * @returns {string | null} Header value when available. + */ +function getHeaderValue(headers, name) { + try { + return typeof headers.get === 'function' ? headers.get(name) : null; + } catch { + return null; + } +} + +/** + * Safely checks header presence from standard or test double header objects. + * @param {Headers} headers Response headers. + * @param {string} name Header name. + * @returns {boolean} True when the header is present. + */ +function hasHeaderValue(headers, name) { + try { + if (typeof headers.has === 'function') { + return headers.has(name); + } + } catch { + return false; + } + + return getHeaderValue(headers, name) !== null; +} + +/** + * Applies upstream response privacy directives to a request-level cache policy. + * @param {{ + * basePolicy: ReturnType, + * response: Response + * }} options + * @returns {ReturnType} Response-aware cache policy. + */ +export function resolveResponseCachePolicy({ basePolicy, response }) { + if (basePolicy.mode === 'private') { + return { + ...basePolicy, + allowCacheApi: false, + browserTtl: 0, + cacheControl: 'private, no-store', + edgeTtl: 0 + }; + } + + const upstreamCacheControl = getHeaderValue(response.headers, 'Cache-Control') || ''; + const hasPrivateDirective = hasCacheControlDirective(upstreamCacheControl, 'private'); + const hasNoStoreDirective = hasCacheControlDirective(upstreamCacheControl, 'no-store'); + const hasNoCacheDirective = hasCacheControlDirective(upstreamCacheControl, 'no-cache'); + const hasSetCookie = hasHeaderValue(response.headers, 'Set-Cookie'); + const hasVaryStar = getHeaderValue(response.headers, 'Vary') + ?.split(',') + .map(value => value.trim()) + .includes('*'); + + if (hasSetCookie || hasPrivateDirective) { + return { + ...basePolicy, + allowCacheApi: false, + browserTtl: 0, + cacheControl: 'private, no-store', + edgeTtl: 0, + mode: 'private' + }; + } + + if (hasNoStoreDirective || hasNoCacheDirective || hasVaryStar) { + return { + ...basePolicy, + allowCacheApi: false, + browserTtl: 0, + cacheControl: 'no-store', + edgeTtl: 0, + mode: 'bypass' + }; + } + + return basePolicy; +} diff --git a/test/unit/cache-privacy.test.js b/test/unit/cache-privacy.test.js index 336d221..3f4fd8d 100644 --- a/test/unit/cache-privacy.test.js +++ b/test/unit/cache-privacy.test.js @@ -84,4 +84,57 @@ describe('Cache Privacy', () => { expect(fetchStub).toHaveBeenCalled(); expect(response.headers.get('Cache-Control') || '').toContain('public'); }); + + it('should not cache upstream responses with Set-Cookie', async () => { + fetchStub.mockResolvedValueOnce( + new Response('private data', { + status: 200, + headers: { + 'Content-Type': 'text/plain', + 'Set-Cookie': 'session=upstream-secret' + } + }) + ); + + const request = new Request('https://example.com/gh/test/repo/file.txt', { + method: 'GET' + }); + + const ctx = { waitUntil: () => {}, passThroughOnException: () => {} }; + const response = await worker.fetch(request, {}, ctx); + + expect(response.status).toBe(200); + expect(cacheDefault.put).not.toHaveBeenCalled(); + expect(response.headers.get('Cache-Control')).toBe('private, no-store'); + }); + + it.each([ + ['private', 'private, max-age=60', 'private, no-store'], + ['no-store', 'public, no-store, max-age=60', 'no-store'], + ['no-cache', 'public, no-cache, max-age=60', 'no-store'] + ])( + 'should not publish-cache upstream %s responses', + async (_name, upstreamCacheControl, expected) => { + fetchStub.mockResolvedValueOnce( + new Response('uncacheable data', { + status: 200, + headers: { + 'Cache-Control': upstreamCacheControl, + 'Content-Type': 'text/plain' + } + }) + ); + + const request = new Request('https://example.com/gh/test/repo/file.txt', { + method: 'GET' + }); + + const ctx = { waitUntil: () => {}, passThroughOnException: () => {} }; + const response = await worker.fetch(request, {}, ctx); + + expect(response.status).toBe(200); + expect(cacheDefault.put).not.toHaveBeenCalled(); + expect(response.headers.get('Cache-Control')).toBe(expected); + } + ); }); diff --git a/test/unit/pipeline-modules.test.js b/test/unit/pipeline-modules.test.js index e254e09..821cb17 100644 --- a/test/unit/pipeline-modules.test.js +++ b/test/unit/pipeline-modules.test.js @@ -134,11 +134,25 @@ describe('Pipeline modules', () => { requestUrl: 'https://example.com/pypi/files/packages/py3/r/requests/requests-2.31.0-py3-none-any.whl' }, + { + cacheTargetUrl: + 'https://files.pythonhosted.org/packages/source/r/requests/requests-2.31.0.tar.gz', + effectivePath: '/pypi/files/packages/source/r/requests/requests-2.31.0.tar.gz', + platform: 'pypi-files', + requestUrl: + 'https://example.com/pypi/files/packages/source/r/requests/requests-2.31.0.tar.gz' + }, { cacheTargetUrl: 'https://repo1.maven.org/maven2/org/example/demo/1.0.0/demo-1.0.0.jar', effectivePath: '/maven/maven2/org/example/demo/1.0.0/demo-1.0.0.jar', platform: 'maven', requestUrl: 'https://example.com/maven/maven2/org/example/demo/1.0.0/demo-1.0.0.jar' + }, + { + cacheTargetUrl: 'https://github.com/user/repo/releases/download/v1.2.3/file.tar.gz', + effectivePath: '/gh/user/repo/releases/download/v1.2.3/file.tar.gz', + platform: 'gh', + requestUrl: 'https://example.com/gh/user/repo/releases/download/v1.2.3/file.tar.gz' } ]; @@ -175,6 +189,76 @@ describe('Pipeline modules', () => { } }); + it('does not treat mutable branch archives as immutable artifacts', async () => { + const archiveCases = [ + { + cacheTargetUrl: 'https://github.com/user/repo/archive/refs/heads/main.zip', + effectivePath: '/gh/user/repo/archive/refs/heads/main.zip', + platform: 'gh', + requestUrl: 'https://example.com/gh/user/repo/archive/refs/heads/main.zip' + }, + { + cacheTargetUrl: + 'https://github.com/Homebrew/homebrew-cask/archive/refs/heads/master.tar.gz', + effectivePath: '/homebrew/homebrew-cask.git/archive/refs/heads/master.tar.gz', + platform: 'homebrew', + requestUrl: + 'https://example.com/homebrew/homebrew-cask.git/archive/refs/heads/master.tar.gz' + }, + { + cacheTargetUrl: 'https://github.com/user/repo/releases/download/latest/file.zip', + effectivePath: '/gh/user/repo/releases/download/latest/file.zip', + platform: 'gh', + requestUrl: 'https://example.com/gh/user/repo/releases/download/latest/file.zip' + }, + { + cacheTargetUrl: 'https://files.pythonhosted.org/packages/source/p/pkg/latest.tar.gz', + effectivePath: '/pypi/files/packages/source/p/pkg/latest.tar.gz', + platform: 'pypi-files', + requestUrl: 'https://example.com/pypi/files/packages/source/p/pkg/latest.tar.gz' + }, + { + cacheTargetUrl: + 'https://files.pythonhosted.org/packages/py3/p/pkg/pkg-latest-py3-none-any.whl', + effectivePath: '/pypi/files/packages/py3/p/pkg/pkg-latest-py3-none-any.whl', + platform: 'pypi-files', + requestUrl: 'https://example.com/pypi/files/packages/py3/p/pkg/pkg-latest-py3-none-any.whl' + } + ]; + + for (const archiveCase of archiveCases) { + const request = new Request(archiveCase.requestUrl); + const requestContext = createRequestContext(request, {}); + + const response = await finalizeResponse({ + cache: null, + cacheTargetUrl: archiveCase.cacheTargetUrl, + canUseCache: true, + config: CONFIG, + ctx: /** @type {ExecutionContext} */ ({ waitUntil() {}, passThroughOnException() {} }), + effectivePath: archiveCase.effectivePath, + hasSensitiveHeaders: false, + monitor: new PerformanceMonitor(), + platform: archiveCase.platform, + request, + requestContext, + response: new Response('archive-data', { + status: 200, + headers: { + 'Content-Type': 'application/octet-stream', + 'Content-Length': '12' + } + }), + responseGeneratedLocally: false, + url: new URL(request.url) + }); + + expect(response.headers.get('Cache-Control')).toBe( + 'public, max-age=0, s-maxage=300, must-revalidate' + ); + } + }); + it('varies npm metadata cache keys by request origin after rewriting', () => { const targetA = resolveTarget( new URL('https://mirror-a.example/npm/pkg'), diff --git a/test/unit/utils.test.js b/test/unit/utils.test.js index f946772..b37bf05 100644 --- a/test/unit/utils.test.js +++ b/test/unit/utils.test.js @@ -11,6 +11,17 @@ import { import { getAllowedMethods, isDockerRequest, validateRequest } from '../../src/utils/validation.js'; describe('Utility Functions', () => { + describe('createConfig', () => { + it.each([ + ['-1', 300], + ['0', 300], + ['abc', 300], + ['60', 60] + ])('should parse CACHE_DURATION=%s as %i', (value, expected) => { + expect(createConfig({ CACHE_DURATION: value }).CACHE_DURATION).toBe(expected); + }); + }); + describe('isGitRequest', () => { it('should identify Git info/refs requests', () => { const request = new Request('https://example.com/repo.git/info/refs');