diff --git a/src/index.js b/src/index.js index aded411..3e9f0b4 100644 --- a/src/index.js +++ b/src/index.js @@ -117,6 +117,11 @@ async function handleRequest(request, env, ctx) { const targetUrl = `${config.PLATFORMS[platform]}${finalTargetPath}${url.search}`; const authorization = request.headers.get('Authorization'); + const hasSensitiveHeaders = Boolean( + authorization || + request.headers.get('Cookie') || + request.headers.get('Proxy-Authorization') + ); // Check if this is a Git operation const isGit = isGitRequest(request, url); @@ -138,7 +143,15 @@ async function handleRequest(request, env, ctx) { ? /** @type {any} */ (caches).default // eslint-disable-line jsdoc/reject-any-type : null; - if (cache && !isGit && !isGitLFS && !isDocker && !isAI && !isHF) { + if ( + cache && + !isGit && + !isGitLFS && + !isDocker && + !isAI && + !isHF && + !hasSensitiveHeaders + ) { try { // For Range requests, try cache match first const cacheKey = new Request(targetUrl, { @@ -304,24 +317,6 @@ async function handleRequest(request, env, ctx) { } } else if (rangeResponse.ok) { contentLength = rangeResponse.headers.get('Content-Length'); - if (!contentLength) { - const sizeLimit = 50 * 1024 * 1024; - const contentLengthHint = rangeResponse.headers.get('Content-Length'); - if ( - !contentLengthHint || - parseInt(contentLengthHint, 10) < sizeLimit - ) { - try { - const arrayBuffer = await rangeResponse.arrayBuffer(); - contentLength = arrayBuffer.byteLength.toString(); - } catch (error) { - console.warn( - 'Could not buffer response to get Content-Length:', - error - ); - } - } - } } if (contentLength) { @@ -543,8 +538,20 @@ async function handleRequest(request, env, ctx) { const headers = new Headers(response.headers); - if (!isGit && !isDocker) { - headers.set('Cache-Control', `public, max-age=${config.CACHE_DURATION}`); + if (!isGit && !isGitLFS && !isDocker && !isAI && !isHF) { + if (hasSensitiveHeaders) { + headers.set('Cache-Control', 'private, no-store'); + const existingVary = headers.get('Vary'); + headers.set( + 'Vary', + existingVary + ? `${existingVary}, Authorization, Cookie` + : 'Authorization, Cookie' + ); + } else { + headers.set('Cache-Control', `public, max-age=${config.CACHE_DURATION}`); + } + headers.set('X-Content-Type-Options', 'nosniff'); headers.set('Accept-Ranges', 'bytes'); @@ -574,6 +581,8 @@ async function handleRequest(request, env, ctx) { !isGitLFS && !isDocker && !isAI && + !isHF && + !hasSensitiveHeaders && request.method === 'GET' && response.ok && response.status === 200 diff --git a/src/utils/validation.js b/src/utils/validation.js index 3932721..783a723 100644 --- a/src/utils/validation.js +++ b/src/utils/validation.js @@ -27,6 +27,59 @@ import { isAIInferenceRequest } from '../protocols/ai.js'; import { isGitLFSRequest, isGitRequest } from '../protocols/git.js'; import { isHuggingFaceAPIRequest } from '../protocols/huggingface.js'; +/** + * Best-effort decode for security validation. + * + * URL.pathname may keep some reserved characters percent-encoded (e.g. %2F). + * We decode a couple of times to catch traversal attempts like %2e%2e%2f. + * @param {string} pathname + * @returns {{ok: true, value: string} | {ok: false}} Decoded path result + */ +function decodePathnameForValidation(pathname) { + let decoded = pathname; + for (let i = 0; i < 2; i++) { + if (!/%[0-9a-fA-F]{2}/.test(decoded)) { + break; + } + try { + decoded = decodeURIComponent(decoded); + } catch { + return { ok: false }; + } + } + return { ok: true, value: decoded }; +} + +/** + * Detects directory traversal patterns in a URL path. + * @param {string} pathname + * @returns {boolean} True if traversal is detected + */ +function hasPathTraversal(pathname) { + const decodedResult = decodePathnameForValidation(pathname); + if (!decodedResult.ok) { + return true; + } + + const decoded = decodedResult.value.replace(/\\/g, '/'); + return /(^|\/)\.\.(\/|$)/.test(decoded); +} + +/** + * Checks for ASCII control characters. + * @param {string} value + * @returns {boolean} True if ASCII control chars are present + */ +function hasAsciiControlChars(value) { + for (let i = 0; i < value.length; i++) { + const code = value.charCodeAt(i); + if (code <= 31 || code === 127) { + return true; + } + } + return false; +} + /** * Detects if a request is a container registry operation (Docker/OCI). * @@ -111,5 +164,25 @@ export function validateRequest(request, url, config = CONFIG) { return { valid: false, error: 'Path too long', status: 414 }; } + // Reject obvious traversal in the raw URL path (before URL normalization). + // Some runtimes normalize `..` segments when parsing URL.pathname. + const rawPathname = request.url.startsWith(url.origin) + ? request.url.slice(url.origin.length).split('?')[0].split('#')[0].replace(/\\/g, '/') + : url.pathname; + + if (/(^|\/)\.\.(\/|$)/.test(rawPathname)) { + return { valid: false, error: 'Invalid path', status: 400 }; + } + + // Reject control characters and directory traversal attempts. + // This protects both our routing logic and upstream requests. + if (hasAsciiControlChars(url.pathname)) { + return { valid: false, error: 'Invalid path', status: 400 }; + } + + if (hasPathTraversal(url.pathname)) { + return { valid: false, error: 'Invalid path', status: 400 }; + } + return { valid: true }; } diff --git a/test/features/security.test.js b/test/features/security.test.js index 2c4c8c6..4b1f86e 100644 --- a/test/features/security.test.js +++ b/test/features/security.test.js @@ -96,8 +96,13 @@ describe('Security Features', () => { method: 'HEAD', redirect: 'manual' // Don't follow redirects }); - // Should either reject with 400, redirect (302/301), or safely handle the path - expect([400, 404, 500, 302, 301]).toContain(response.status); + // Some runtimes normalize plain `..` segments before the Worker sees them. + // Encoded traversal should still be rejected. + if (/%[0-9a-fA-F]{2}/.test(path)) { + expect(response.status).toBe(400); + } else { + expect(response.status).not.toBe(500); + } } }, 30000); diff --git a/test/unit/cache-privacy.test.js b/test/unit/cache-privacy.test.js new file mode 100644 index 0000000..46fc55f --- /dev/null +++ b/test/unit/cache-privacy.test.js @@ -0,0 +1,66 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import worker from '../../src/index.js'; + +describe('Cache Privacy', () => { + /** @type {{ match: ReturnType, put: ReturnType }} */ + let cacheDefault; + + /** @type {ReturnType} */ + let fetchStub; + + beforeEach(() => { + cacheDefault = { + match: vi.fn(async () => null), + put: vi.fn(async () => undefined) + }; + + vi.stubGlobal('caches', { default: cacheDefault }); + + fetchStub = vi.fn(async () => { + return new Response('ok', { + status: 200, + headers: { + 'Content-Type': 'text/plain' + } + }); + }); + vi.stubGlobal('fetch', fetchStub); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('should not use Cache API for requests with Authorization', async () => { + const request = new Request('https://example.com/gh/test/repo/file.txt', { + method: 'GET', + headers: { + Authorization: 'Bearer test-token' + } + }); + + const ctx = { waitUntil: () => {}, passThroughOnException: () => {} }; + const response = await worker.fetch(request, {}, ctx); + + expect(response.status).toBe(200); + expect(cacheDefault.match).not.toHaveBeenCalled(); + expect(cacheDefault.put).not.toHaveBeenCalled(); + expect(response.headers.get('Cache-Control')).toBe('private, no-store'); + }); + + it('should use Cache API for non-authenticated GET requests', async () => { + 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.match).toHaveBeenCalled(); + expect(fetchStub).toHaveBeenCalled(); + expect(response.headers.get('Cache-Control') || '').toContain('public'); + }); +});