feat: enhance security validation and caching behavior for requests with sensitive headers
This commit is contained in:
1 parent
69b27ea12a
commit
e705367524
4 files changed
+176
-23
No files matched your search
+30
-21
@@ -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
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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<typeof vi.fn>, put: ReturnType<typeof vi.fn> }} */
|
||||
let cacheDefault;
|
||||
|
||||
/** @type {ReturnType<typeof vi.fn>} */
|
||||
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');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user