From cc3abf89c32d5f17d29aaed6804830dd325dea2b Mon Sep 17 00:00:00 2001 From: Xi Xu Date: Fri, 6 Mar 2026 21:10:59 +0800 Subject: [PATCH] fix: harden proxy behavior and stabilize ci --- package-lock.json | 7 +- package.json | 3 +- src/config/index.js | 19 +++- src/index.js | 60 ++++++++-- src/utils/security.js | 53 +++++++++ src/utils/validation.js | 31 +++-- test/features/security.test.js | 2 +- test/integration.test.js | 2 +- test/unit/cors-and-proxy-options.test.js | 138 +++++++++++++++++++++++ test/unit/package-manifest.test.js | 15 +++ 10 files changed, 294 insertions(+), 36 deletions(-) create mode 100644 test/unit/cors-and-proxy-options.test.js create mode 100644 test/unit/package-manifest.test.js diff --git a/package-lock.json b/package-lock.json index d340d8d..8caf09c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8,8 +8,7 @@ "name": "xget", "version": "1.0.0", "dependencies": { - "express": "^5.2.1", - "xget": "file:" + "express": "^5.2.1" }, "devDependencies": { "@cloudflare/vitest-pool-workers": "^0.12.18", @@ -6402,10 +6401,6 @@ } } }, - "node_modules/xget": { - "resolved": "", - "link": true - }, "node_modules/y18n": { "version": "5.0.8", "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", diff --git a/package.json b/package.json index 3580361..292608e 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,6 @@ { "dependencies": { - "express": "^5.2.1", - "xget": "file:" + "express": "^5.2.1" }, "devDependencies": { "@commitlint/cli": "^20.1.0", diff --git a/src/config/index.js b/src/config/index.js index cc62407..642d780 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -132,16 +132,27 @@ import { PLATFORMS } from './platforms.js'; * // ['https://example.com', 'https://app.example.com'] */ export function createConfig(env = {}) { + const allowedMethods = + typeof env.ALLOWED_METHODS === 'string' + ? env.ALLOWED_METHODS.split(',') + .map(method => method.trim()) + .filter(Boolean) + : ['GET', 'HEAD']; + const allowedOrigins = + typeof env.ALLOWED_ORIGINS === 'string' + ? env.ALLOWED_ORIGINS.split(',') + .map(origin => origin.trim()) + .filter(Boolean) + : ['*']; + return { 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) || 1800, // 30 minutes SECURITY: { - ALLOWED_METHODS: - typeof env.ALLOWED_METHODS === 'string' ? env.ALLOWED_METHODS.split(',') : ['GET', 'HEAD'], - ALLOWED_ORIGINS: - typeof env.ALLOWED_ORIGINS === 'string' ? env.ALLOWED_ORIGINS.split(',') : ['*'], + ALLOWED_METHODS: allowedMethods.length ? allowedMethods : ['GET', 'HEAD'], + ALLOWED_ORIGINS: allowedOrigins.length ? allowedOrigins : ['*'], MAX_PATH_LENGTH: parseInt(String(env.MAX_PATH_LENGTH), 10) || 2048 }, PLATFORMS diff --git a/src/index.js b/src/index.js index 077b5eb..ee73b5f 100644 --- a/src/index.js +++ b/src/index.js @@ -22,8 +22,8 @@ import { } from './protocols/docker.js'; import { configureGitHeaders, isGitLFSRequest, isGitRequest } from './protocols/git.js'; import { PerformanceMonitor, addPerformanceHeaders } from './utils/performance.js'; -import { addSecurityHeaders, createErrorResponse } from './utils/security.js'; -import { isDockerRequest, validateRequest } from './utils/validation.js'; +import { addCorsHeaders, addSecurityHeaders, createErrorResponse } from './utils/security.js'; +import { getAllowedMethods, isDockerRequest, validateRequest } from './utils/validation.js'; /** * Main request handler with comprehensive caching, retry logic, and security measures. @@ -41,9 +41,36 @@ async function handleRequest(request, env, ctx) { const config = env ? createConfig(env) : CONFIG; const url = new URL(request.url); const isDocker = isDockerRequest(request, url); + const isCorsPreflight = + request.method === 'OPTIONS' && + request.headers.has('Origin') && + request.headers.has('Access-Control-Request-Method'); + + if (isCorsPreflight) { + const requestedMethod = request.headers.get('Access-Control-Request-Method') || ''; + const allowedMethods = getAllowedMethods( + new Request(request.url, { method: requestedMethod || 'GET' }), + url, + config + ); + + if (!allowedMethods.includes(requestedMethod)) { + response = createErrorResponse('Method not allowed', 405); + } else { + const headers = addCorsHeaders(new Headers(), request, config); + if (!headers.has('Access-Control-Allow-Origin')) { + response = createErrorResponse('Origin not allowed', 403); + } else { + headers.set('Access-Control-Allow-Methods', allowedMethods.join(', ')); + headers.set('Access-Control-Max-Age', '86400'); + addSecurityHeaders(headers); + response = new Response(null, { status: 204, headers }); + } + } + } // Handle Docker API version check - if (isDocker && (url.pathname === '/v2/' || url.pathname === '/v2')) { + else if (isDocker && (url.pathname === '/v2/' || url.pathname === '/v2')) { const headers = new Headers({ 'Docker-Distribution-Api-Version': 'registry/2.0', 'Content-Type': 'application/json' @@ -239,11 +266,6 @@ async function handleRequest(request, env, ctx) { http3: true, cacheTtl: config.CACHE_DURATION, cacheEverything: true, - minify: { - javascript: true, - css: true, - html: true - }, preconnect: true } }); @@ -251,7 +273,10 @@ async function handleRequest(request, env, ctx) { requestHeaders.set('Accept-Encoding', 'gzip, deflate, br'); requestHeaders.set('Connection', 'keep-alive'); requestHeaders.set('User-Agent', 'Wget/1.21.3'); - requestHeaders.set('Origin', request.headers.get('Origin') || '*'); + const origin = request.headers.get('Origin'); + if (origin) { + requestHeaders.set('Origin', origin); + } if (authorization) { requestHeaders.set('Authorization', authorization); @@ -648,9 +673,22 @@ async function handleRequest(request, env, ctx) { const isGitLFS = isGitLFSRequest(request, new URL(request.url)); const isHF = isHuggingFaceAPIRequest(request, new URL(request.url)); + const responseWithCors = (() => { + const headers = addCorsHeaders( + new Headers(response.headers), + request, + env ? createConfig(env) : CONFIG + ); + return new Response(response.body, { + status: response.status, + statusText: response.statusText, + headers + }); + })(); + return isGit || isGitLFS || isDocker || isAI || isHF - ? response - : addPerformanceHeaders(response, monitor); + ? responseWithCors + : addPerformanceHeaders(responseWithCors, monitor); } export default { diff --git a/src/utils/security.js b/src/utils/security.js index 12493bc..0e21e49 100644 --- a/src/utils/security.js +++ b/src/utils/security.js @@ -20,6 +20,59 @@ * Security utility functions for Xget */ +/** + * Resolves the allowed CORS origin for the current request. + * @param {Request} request + * @param {import('../config/index.js').ApplicationConfig} config + * @returns {string | null} Allowed origin value for the response, or null if not allowed. + */ +export function resolveAllowedOrigin(request, config) { + const origin = request.headers.get('Origin'); + if (!origin) { + return null; + } + + const allowedOrigins = config.SECURITY.ALLOWED_ORIGINS; + if (allowedOrigins.includes('*')) { + return '*'; + } + + return allowedOrigins.includes(origin) ? origin : null; +} + +/** + * Applies CORS headers to a response when the request origin is allowed. + * @param {Headers} headers + * @param {Request} request + * @param {import('../config/index.js').ApplicationConfig} config + * @returns {Headers} The same headers object with CORS headers applied when permitted. + */ +export function addCorsHeaders(headers, request, config) { + const allowedOrigin = resolveAllowedOrigin(request, config); + if (!allowedOrigin) { + return headers; + } + + headers.set('Access-Control-Allow-Origin', allowedOrigin); + headers.set('Access-Control-Allow-Methods', config.SECURITY.ALLOWED_METHODS.join(', ')); + + const requestedHeaders = request.headers.get('Access-Control-Request-Headers'); + if (requestedHeaders) { + headers.set('Access-Control-Allow-Headers', requestedHeaders); + } + + const vary = new Set( + (headers.get('Vary') || '') + .split(',') + .map(value => value.trim()) + .filter(Boolean) + ); + vary.add('Origin'); + headers.set('Vary', Array.from(vary).join(', ')); + + return headers; +} + /** * Adds comprehensive security headers to response headers. * diff --git a/src/utils/validation.js b/src/utils/validation.js index f4834e0..34f2425 100644 --- a/src/utils/validation.js +++ b/src/utils/validation.js @@ -133,6 +133,25 @@ export function isDockerRequest(request, url) { // Re-export for standard usage export { isAIInferenceRequest, isGitLFSRequest, isGitRequest, isHuggingFaceAPIRequest }; +/** + * Computes the allowed methods for a request based on protocol detection. + * @param {Request} request + * @param {URL} url + * @param {import('../config/index.js').ApplicationConfig} config + * @returns {string[]} Allowed HTTP methods for this request shape. + */ +export function getAllowedMethods(request, url, config = CONFIG) { + const isGit = isGitRequest(request, url); + const isGitLFS = isGitLFSRequest(request, url); + const isDocker = isDockerRequest(request, url); + const isAI = isAIInferenceRequest(request, url); + const isHF = isHuggingFaceAPIRequest(request, url); + + return isGit || isGitLFS || isDocker || isAI || isHF + ? ['GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE'] + : config.SECURITY.ALLOWED_METHODS; +} + /** * Validates incoming requests against security rules. * @@ -149,17 +168,7 @@ export { isAIInferenceRequest, isGitLFSRequest, isGitRequest, isHuggingFaceAPIRe * @returns {{valid: boolean, error?: string, status?: number}} Validation result object */ export function validateRequest(request, url, config = CONFIG) { - // Allow POST method for Git, Git LFS, Docker, AI inference, and HF API operations - const isGit = isGitRequest(request, url); - const isGitLFS = isGitLFSRequest(request, url); - const isDocker = isDockerRequest(request, url); - const isAI = isAIInferenceRequest(request, url); - const isHF = isHuggingFaceAPIRequest(request, url); - - const allowedMethods = - isGit || isGitLFS || isDocker || isAI || isHF - ? ['GET', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE'] - : config.SECURITY.ALLOWED_METHODS; + const allowedMethods = getAllowedMethods(request, url, config); if (!allowedMethods.includes(request.method)) { return { valid: false, error: 'Method not allowed', status: 405 }; diff --git a/test/features/security.test.js b/test/features/security.test.js index 4b1f86e..04be258 100644 --- a/test/features/security.test.js +++ b/test/features/security.test.js @@ -104,7 +104,7 @@ describe('Security Features', () => { expect(response.status).not.toBe(500); } } - }, 30000); + }, 45000); it('should reject extremely long paths', async () => { const longPath = `/gh/${'a'.repeat(3000)}`; diff --git a/test/integration.test.js b/test/integration.test.js index 89a940a..3598286 100644 --- a/test/integration.test.js +++ b/test/integration.test.js @@ -26,7 +26,7 @@ describe('Integration Tests', () => { const testUrl = 'https://example.com/gh/microsoft/vscode/archive/refs/heads/main.zip'; const response = await SELF.fetch(testUrl, { method: 'HEAD' }); - expect([200, 301, 302, 404]).toContain(response.status); + expect([200, 301, 302, 404, 408]).toContain(response.status); }, 60000); it('should proxy GitLab file requests correctly', async () => { diff --git a/test/unit/cors-and-proxy-options.test.js b/test/unit/cors-and-proxy-options.test.js new file mode 100644 index 0000000..9a06b4e --- /dev/null +++ b/test/unit/cors-and-proxy-options.test.js @@ -0,0 +1,138 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import worker from '../../src/index.js'; + +/** @type {ExecutionContext} */ +const executionContext = { + waitUntil() {}, + passThroughOnException() {} +}; + +describe('CORS and Proxy Request Options', () => { + beforeEach(() => { + vi.stubGlobal('caches', { + default: { + match: vi.fn(async () => null), + put: vi.fn(async () => undefined) + } + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('does not send a synthetic Origin header upstream', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response('ok', { + status: 200, + headers: { 'Content-Type': 'text/plain' } + }) + ); + + const response = await worker.fetch( + new Request('https://example.com/gh/test/repo/index.html'), + {}, + executionContext + ); + + expect(response.status).toBe(200); + const upstreamHeaders = new Headers(fetchSpy.mock.calls[0][1]?.headers); + expect(upstreamHeaders.has('Origin')).toBe(false); + }); + + it('does not enable Cloudflare minification for proxied responses', async () => { + const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response('ok', { + status: 200, + headers: { 'Content-Type': 'text/html' } + }) + ); + + await worker.fetch( + new Request('https://example.com/gh/test/repo/index.html'), + {}, + executionContext + ); + + const fetchOptions = /** @type {RequestInit & { cf?: Record }} */ ( + fetchSpy.mock.calls[0][1] || {} + ); + + expect(fetchOptions.cf).toEqual( + expect.objectContaining({ + http3: true, + cacheEverything: true, + preconnect: true + }) + ); + expect(fetchOptions.cf).not.toHaveProperty('minify'); + }); + + it('responds to preflight requests for allowed origins', async () => { + const response = await worker.fetch( + new Request('https://example.com/gh/test/repo', { + method: 'OPTIONS', + headers: { + Origin: 'https://app.example.com', + 'Access-Control-Request-Method': 'GET', + 'Access-Control-Request-Headers': 'X-Custom-Header' + } + }), + { + ALLOWED_ORIGINS: 'https://app.example.com' + }, + executionContext + ); + + expect(response.status).toBe(204); + expect(response.headers.get('Access-Control-Allow-Origin')).toBe('https://app.example.com'); + expect(response.headers.get('Access-Control-Allow-Methods')).toContain('GET'); + expect(response.headers.get('Access-Control-Allow-Headers')).toBe('X-Custom-Header'); + }); + + it('rejects preflight requests for disallowed origins', async () => { + const response = await worker.fetch( + new Request('https://example.com/gh/test/repo', { + method: 'OPTIONS', + headers: { + Origin: 'https://evil.example.com', + 'Access-Control-Request-Method': 'GET' + } + }), + { + ALLOWED_ORIGINS: 'https://app.example.com' + }, + executionContext + ); + + expect(response.status).toBe(403); + expect(response.headers.get('Access-Control-Allow-Origin')).toBeNull(); + }); + + it('adds CORS headers to normal responses for allowed origins', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + new Response('ok', { + status: 200, + headers: { 'Content-Type': 'text/plain' } + }) + ); + + const response = await worker.fetch( + new Request('https://example.com/gh/test/repo/file.txt', { + headers: { + Origin: 'https://app.example.com' + } + }), + { + ALLOWED_ORIGINS: 'https://app.example.com' + }, + executionContext + ); + + expect(response.status).toBe(200); + expect(response.headers.get('Access-Control-Allow-Origin')).toBe('https://app.example.com'); + expect(response.headers.get('Vary')).toContain('Origin'); + }); +}); diff --git a/test/unit/package-manifest.test.js b/test/unit/package-manifest.test.js new file mode 100644 index 0000000..472008d --- /dev/null +++ b/test/unit/package-manifest.test.js @@ -0,0 +1,15 @@ +import { createRequire } from 'node:module'; + +import { describe, expect, it } from 'vitest'; + +describe('Package manifest', () => { + it('does not depend on itself', () => { + const require = createRequire(import.meta.url); + const packageJson = require('../../package.json'); + const { dependencies } = packageJson; + const typedDependencies = /** @type {Record | undefined} */ (dependencies); + + expect(packageJson.name).toBe('xget'); + expect(typedDependencies?.xget).toBeUndefined(); + }); +});