From 5a21bac224e05c021a4ffe0e0ba5514269b97653 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 07:58:23 +0200 Subject: [PATCH 01/18] =?UTF-8?q?cors-proxy:=20Phase=201=20=E2=80=94=20WAC?= =?UTF-8?q?-gated=20proxy=20for=20browser=20apps=20(#378)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New optional feature behind --cors-proxy. Browser apps hosted on a JSS pod can now fetch arbitrary upstream URLs that don't return CORS headers themselves — the pod takes the request, validates the upstream URL against the existing SSRF guard, fetches it, and streams the response back with permissive CORS headers attached. The git-specific framing in #377 is now a special case: jss.live/git/ can route through /proxy?url=https://github.com/.../info/refs instead of needing a third-party CORS proxy or per-host server-side glue. ## Auth model WAC. The /proxy resource is checked through the standard authorize() pipeline, so pod owners control access by writing an .acl on /proxy (or inheriting from /.acl) — anything from "owner-only" to "foaf:Agent for any authenticated user" to fully public. ## SSRF validateExternalUrl() runs on the user-supplied URL AND on every redirect target. Required: fetch's automatic redirect following bypasses the initial guard at hop 2 (an attacker hosts a 302 to 169.254.169.254 or any private IP). Manual redirect with re-validation closes that. Redirects are limited to --cors-proxy-max-redirects (default 5), and 301/302/303 force the next hop to GET (per HTTP semantics) while 307/308 preserve the original method and body. ## Streaming + caps Upstream body is streamed via Readable.fromWeb piped through a Transform that counts bytes. Hitting --cors-proxy-max-bytes (default 50 MB) errors the Transform, which surfaces as a stream error to the caller — pack files, large images, etc. won't blow memory. Per-request --cors-proxy-timeout-ms (default 30 s) is enforced via AbortController. ## Headers Forward (request → upstream): a fixed allowlist that includes Authorization, Git-Protocol, Content-Type, Accept*, Range, conditional headers, User-Agent. Stripped: Cookie, Origin, Host, Referer. Strip (upstream → response): Content-Encoding (Node fetch already decoded), Transfer-Encoding, Connection, Set-Cookie. CORS headers (GIT_CORS_HEADERS-style PROXY_CORS_HEADERS const) get appended on every response, including 4xx. ## Verified locally - /proxy without ?url → 400 - /proxy?url=http://192.168.1.1/ → 400 (SSRF guard) - /proxy?url=not-a-url → 400 (URL parse) - /proxy?url=https://example.com/ → 200 with HTML body and CORS headers - POST /proxy?url=https://httpbin.org/post with body — body forwarded - /proxy?url=https://httpbin.org/redirect/2 → 200 (redirects followed) - /proxy?url=https://httpbin.org/redirect-to?url=http://192.168.1.1/ → 400 (redirect target re-validated, hop 2 blocked) - --cors-proxy-max-bytes 100 → stream errors after 99 bytes - Without --public, anonymous request → 401 (WAC) Phase 2 (--pay integration: per-request debit, free-tier rate limit, 402↔401 mapping for git clients) deferred. Tracked in #378. --- bin/jss.js | 10 ++ src/config.js | 11 ++ src/handlers/cors-proxy.js | 237 +++++++++++++++++++++++++++++++++++++ src/server.js | 35 ++++++ 4 files changed, 293 insertions(+) create mode 100644 src/handlers/cors-proxy.js diff --git a/bin/jss.js b/bin/jss.js index 0d86907a..ffd5a53a 100755 --- a/bin/jss.js +++ b/bin/jss.js @@ -105,6 +105,11 @@ program .option('--mashlib-version ', 'Mashlib version for CDN mode (default: 2.0.0)') .option('--git', 'Enable Git HTTP backend (clone/push support)') .option('--no-git', 'Disable Git HTTP backend') + .option('--cors-proxy', 'Enable CORS proxy at /proxy?url=... for browser apps (WAC-gated)') + .option('--no-cors-proxy', 'Disable CORS proxy') + .option('--cors-proxy-max-bytes ', 'CORS proxy upstream response size cap (default 50MB)', parseInt) + .option('--cors-proxy-timeout-ms ', 'CORS proxy upstream request timeout (default 30s)', parseInt) + .option('--cors-proxy-max-redirects ', 'CORS proxy max redirect hops, each re-validated (default 5)', parseInt) .option('--nostr', 'Enable Nostr relay') .option('--no-nostr', 'Disable Nostr relay') .option('--nostr-path ', 'Nostr relay WebSocket path (default: /relay)') @@ -191,6 +196,10 @@ program mashlibVersion: config.mashlibVersion, mashlibModule: config.mashlibModule, git: config.git, + corsProxy: config.corsProxy, + corsProxyMaxBytes: config.corsProxyMaxBytes, + corsProxyTimeoutMs: config.corsProxyTimeoutMs, + corsProxyMaxRedirects: config.corsProxyMaxRedirects, nostr: config.nostr, nostrPath: config.nostrPath, nostrMaxEvents: config.nostrMaxEvents, @@ -242,6 +251,7 @@ program } if (config.mashlibModule) console.log(` Mashlib module: ${config.mashlibModule}`); if (config.git) console.log(' Git: enabled (clone/push support)'); + if (config.corsProxy) console.log(' CORS proxy: enabled (/proxy?url=..., WAC-gated)'); if (config.nostr) console.log(` Nostr: enabled (${config.nostrPath})`); if (config.webrtc) console.log(` WebRTC: enabled (${config.webrtcPath || '/.webrtc'})`); if (config.terminal) console.log(' Terminal: enabled (/.terminal)'); diff --git a/src/config.js b/src/config.js index 23b087be..b4e3f73e 100644 --- a/src/config.js +++ b/src/config.js @@ -46,6 +46,13 @@ export const defaults = { // Git HTTP backend git: false, + // CORS proxy (#378) — pod-hosted, WAC-gated proxy for browser apps to + // fetch arbitrary upstreams that don't return CORS headers. + corsProxy: false, + corsProxyMaxBytes: 50 * 1024 * 1024, // 50 MB ceiling on upstream response size + corsProxyTimeoutMs: 30_000, // 30 s overall request deadline + corsProxyMaxRedirects: 5, // each redirect re-validated for SSRF + // Nostr relay nostr: false, nostrPath: '/relay', @@ -147,6 +154,10 @@ const envMap = { JSS_MASHLIB_VERSION: 'mashlibVersion', JSS_MASHLIB_MODULE: 'mashlibModule', JSS_GIT: 'git', + JSS_CORS_PROXY: 'corsProxy', + JSS_CORS_PROXY_MAX_BYTES: 'corsProxyMaxBytes', + JSS_CORS_PROXY_TIMEOUT_MS: 'corsProxyTimeoutMs', + JSS_CORS_PROXY_MAX_REDIRECTS: 'corsProxyMaxRedirects', JSS_NOSTR: 'nostr', JSS_NOSTR_PATH: 'nostrPath', JSS_NOSTR_MAX_EVENTS: 'nostrMaxEvents', diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js new file mode 100644 index 00000000..0cc007da --- /dev/null +++ b/src/handlers/cors-proxy.js @@ -0,0 +1,237 @@ +/** + * CORS proxy handler — fetches an arbitrary upstream URL on behalf of a + * browser-side caller and returns the response with CORS headers, so + * pod-hosted apps can talk to non-CORS-friendly origins (#378). + * + * URL shape: `/proxy?url=` — query-param keeps the path a + * plain Solid resource for WAC purposes (acl can sit at /proxy or /.acl + * and inherit normally). + * + * Auth: WAC. The standard /proxy resource is checked by server.js's + * authorize() pipeline before this handler runs; pod owner controls + * access by writing an .acl on /proxy. + * + * SSRF: validateExternalUrl() runs on the user-supplied URL AND on every + * redirect target. Manual redirect following is non-negotiable — letting + * fetch() follow redirects automatically would let an attacker host a + * 302 to 169.254.169.254 and bypass the initial guard. + * + * Body limits: streamed, with a configurable max-bytes ceiling enforced + * during streaming. Timeout via AbortController. + */ + +import { validateExternalUrl } from '../utils/ssrf.js'; +import { Readable, Transform } from 'stream'; + +// CORS headers applied to every proxy response. Same shape/source-of-truth +// pattern as GIT_CORS_HEADERS in src/handlers/git.js (#374). +const PROXY_CORS_HEADERS = { + 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Methods': 'GET, POST, HEAD, OPTIONS', + 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Git-Protocol, Accept, Accept-Encoding, Accept-Language', + 'Access-Control-Expose-Headers': 'Content-Type, Content-Length, ETag, Last-Modified, Link, Location, WWW-Authenticate', +}; + +function setProxyCorsHeaders(reply) { + for (const [k, v] of Object.entries(PROXY_CORS_HEADERS)) { + reply.header(k, v); + } +} + +// Headers we forward from the caller to the upstream. Anything not on +// this list is dropped — origin/cookie/host/referer would either confuse +// upstream auth or leak the proxying pod's identity. +const FORWARD_REQUEST_HEADERS = new Set([ + 'accept', + 'accept-encoding', + 'accept-language', + 'authorization', + 'content-type', + 'content-length', + 'git-protocol', + 'if-match', + 'if-none-match', + 'if-modified-since', + 'range', + 'user-agent', +]); + +// Headers we strip from the upstream response before returning to the +// caller. Encoding-related ones are removed because Node's fetch already +// transparently decodes; passing through Content-Encoding would +// double-decompress in the browser. +const STRIP_RESPONSE_HEADERS = new Set([ + 'content-encoding', + 'transfer-encoding', + 'connection', + 'keep-alive', + // Strip set-cookie — we never want to forward upstream cookies into + // our origin's domain (would let upstream set cookies on the pod). + 'set-cookie', +]); + +const DEFAULT_MAX_REDIRECTS = 5; +const DEFAULT_TIMEOUT_MS = 30_000; +const DEFAULT_MAX_BYTES = 50 * 1024 * 1024; // 50 MB +const ALLOWED_METHODS = new Set(['GET', 'POST', 'HEAD', 'OPTIONS']); + +function pickRequestHeaders(reqHeaders) { + const out = {}; + for (const [k, v] of Object.entries(reqHeaders)) { + if (v == null) continue; + if (FORWARD_REQUEST_HEADERS.has(k.toLowerCase())) { + out[k] = v; + } + } + return out; +} + +function copyResponseHeaders(reply, fetchResponse) { + for (const [k, v] of fetchResponse.headers) { + if (STRIP_RESPONSE_HEADERS.has(k.toLowerCase())) continue; + reply.header(k, v); + } +} + +/** + * Stream the upstream body to the caller, enforcing a byte cap mid-stream. + * Uses Node 18+ Readable.fromWeb to bridge the fetch ReadableStream into + * a Node Readable, then a Transform passthrough counts bytes and errors + * the pipeline if maxBytes is exceeded. + */ +function streamWithCap(reply, fetchResponse, maxBytes, abortController) { + if (!fetchResponse.body) { + return reply.send(); + } + + let bytesSeen = 0; + const counter = new Transform({ + transform(chunk, _encoding, callback) { + bytesSeen += chunk.length; + if (bytesSeen > maxBytes) { + abortController.abort(); + return callback(new Error(`Upstream response exceeded ${maxBytes} bytes`)); + } + callback(null, chunk); + } + }); + + Readable.fromWeb(fetchResponse.body).on('error', (err) => counter.destroy(err)).pipe(counter); + return reply.send(counter); +} + +/** + * Handle a CORS proxy request. + * + * @param {FastifyRequest} request + * @param {FastifyReply} reply + * @param {object} options + * @param {number} [options.maxBytes] + * @param {number} [options.timeoutMs] + * @param {number} [options.maxRedirects] + */ +export async function handleCorsProxy(request, reply, options = {}) { + setProxyCorsHeaders(reply); + + // CORS preflight — short-circuit without fetching. + if (request.method === 'OPTIONS') { + return reply.code(204).send(); + } + + if (!ALLOWED_METHODS.has(request.method)) { + return reply.code(405).send({ error: 'Method not allowed', method: request.method }); + } + + const targetUrl = request.query?.url; + if (!targetUrl || typeof targetUrl !== 'string') { + return reply.code(400).send({ error: 'Missing required query parameter: url' }); + } + + const maxBytes = options.maxBytes ?? DEFAULT_MAX_BYTES; + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const maxRedirects = options.maxRedirects ?? DEFAULT_MAX_REDIRECTS; + + // Validate the initial URL, then follow redirects manually re-validating + // each hop's Location header. Required because fetch's automatic redirect + // following bypasses our SSRF guard at the second hop. + let currentUrl = targetUrl; + let redirectsLeft = maxRedirects; + + // Body for non-GET/HEAD methods. Read once up front; we may need it on + // the first hop and (for 307/308) on redirected hops. + let body = null; + if (request.method !== 'GET' && request.method !== 'HEAD') { + body = request.rawBody ?? request.body; + if (typeof body === 'object' && !(body instanceof Buffer) && body !== null) { + body = JSON.stringify(body); + } + } + + while (true) { + const validation = await validateExternalUrl(currentUrl, { + requireHttps: false, // allow http:// — pod operator can lock down via .acl scope + blockPrivateIPs: true, + resolveDNS: true, + }); + if (!validation.valid) { + return reply.code(400).send({ error: 'Invalid upstream URL', detail: validation.error }); + } + + const abortController = new AbortController(); + const timeoutId = setTimeout(() => abortController.abort(), timeoutMs); + + let upstream; + try { + upstream = await fetch(currentUrl, { + method: request.method, + headers: pickRequestHeaders(request.headers), + body, + redirect: 'manual', + signal: abortController.signal, + }); + } catch (err) { + clearTimeout(timeoutId); + if (err.name === 'AbortError') { + return reply.code(504).send({ error: 'Upstream timeout' }); + } + return reply.code(502).send({ error: 'Upstream fetch failed', detail: err.message }); + } + clearTimeout(timeoutId); + + // Manual redirect handling: 301/302/303 → GET, 307/308 → preserve method + if ([301, 302, 303, 307, 308].includes(upstream.status)) { + const location = upstream.headers.get('location'); + if (!location) { + return reply.code(502).send({ error: 'Upstream redirect with no Location header' }); + } + if (--redirectsLeft < 0) { + return reply.code(502).send({ error: `Exceeded ${maxRedirects} redirects` }); + } + // Resolve relative redirects against the previous URL. + currentUrl = new URL(location, currentUrl).toString(); + // 301/302/303 force the next request to GET with no body. + if ([301, 302, 303].includes(upstream.status)) { + body = null; + } + // Drain the redirect body to free the connection; we never return it. + try { await upstream.body?.cancel(); } catch { /* ignore */ } + continue; + } + + // Final response — stream it back. + reply.code(upstream.status); + copyResponseHeaders(reply, upstream); + setProxyCorsHeaders(reply); // reapply in case copyResponseHeaders set conflicting CORS values + + return streamWithCap(reply, upstream, maxBytes, abortController); + } +} + +/** + * Match a request URL against the proxy route. + * Used by server.js to decide whether the proxy preHandler should fire + * and whether the standard WAC hook should skip. + */ +export function isCorsProxyRequest(urlPath) { + return urlPath === '/proxy' || urlPath.startsWith('/proxy?'); +} diff --git a/src/server.js b/src/server.js index 696dfad6..67dcc591 100644 --- a/src/server.js +++ b/src/server.js @@ -12,6 +12,7 @@ import { notificationsPlugin } from './notifications/index.js'; import { startFileWatcher } from './notifications/events.js'; import { idpPlugin } from './idp/index.js'; import { isGitRequest, isGitWriteOperation, handleGit } from './handlers/git.js'; +import { handleCorsProxy, isCorsProxyRequest } from './handlers/cors-proxy.js'; import { AccessMode } from './wac/parser.js'; import { registerNostrRelay } from './nostr/relay.js'; import { createPayHandler, isPayRequest } from './handlers/pay.js'; @@ -71,6 +72,11 @@ export function createServer(options = {}) { const mashlibVersion = options.mashlibVersion ?? '2.0.0'; // Git HTTP backend is OFF by default - enables clone/push via git protocol const gitEnabled = options.git ?? false; + // CORS proxy (#378) — OFF by default + const corsProxyEnabled = options.corsProxy ?? false; + const corsProxyMaxBytes = options.corsProxyMaxBytes ?? 50 * 1024 * 1024; + const corsProxyTimeoutMs = options.corsProxyTimeoutMs ?? 30_000; + const corsProxyMaxRedirects = options.corsProxyMaxRedirects ?? 5; // Nostr relay is OFF by default const nostrEnabled = options.nostr ?? false; const nostrPath = options.nostrPath ?? '/relay'; @@ -439,6 +445,34 @@ export function createServer(options = {}) { fastify.addHook('preHandler', createPayHandler({ cost: payCost, mempoolUrl: payMempoolUrl, payAddress, payToken, payRate, payChains })); } + // CORS proxy (#378) — WAC-gated. Standard authorize() path runs against + // /proxy as a virtual resource; pod owner controls access by writing an + // .acl on /proxy (or inheriting from /.acl). OPTIONS preflight returns + // 204 directly without auth so browser CORS checks succeed before sign-in. + if (corsProxyEnabled) { + fastify.addHook('preHandler', async (request, reply) => { + const urlPath = request.url.split('?')[0]; + if (!isCorsProxyRequest(urlPath)) { + return; + } + + const { authorized, webId, wacAllow, authError } = await authorize(request, reply, { requiredMode: AccessMode.READ }); + request.webId = webId; + request.wacAllow = wacAllow; + + if (request.method !== 'OPTIONS' && !authorized) { + reply.header('WAC-Allow', wacAllow); + return handleUnauthorized(request, reply, webId !== null, wacAllow, authError); + } + + return handleCorsProxy(request, reply, { + maxBytes: corsProxyMaxBytes, + timeoutMs: corsProxyTimeoutMs, + maxRedirects: corsProxyMaxRedirects, + }); + }); + } + // Authorization hook - check WAC permissions // Skip for pod creation endpoint (needs special handling) fastify.addHook('preHandler', async (request, reply) => { @@ -460,6 +494,7 @@ export function createServer(options = {}) { request.url.startsWith('/.well-known/') || (nostrEnabled && request.url.startsWith(nostrPath)) || (gitEnabled && isGitRequest(request.url)) || + (corsProxyEnabled && isCorsProxyRequest(request.url.split('?')[0])) || (activitypubEnabled && apPaths.some(p => request.url === p || request.url.startsWith(p + '?'))) || isProfileAP || request.url.startsWith('/storage/') || From e1d35821076e312fee8bd787384e595385d5f275 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 08:30:09 +0200 Subject: [PATCH 02/18] cors-proxy: Copilot review fixes (security + correctness) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the 8-comment review pass on #379: Security - Stop forwarding the request's Authorization header upstream by default. The browser uses Authorization to authenticate to *this* pod (WAC, Solid-OIDC bearer/DPoP); silently leaking that token to an arbitrary upstream URL was a real credential-leak shape. Added an opt-in X-Upstream-Authorization header — clients that genuinely need to send credentials upstream (GitHub PAT, etc.) supply them under that name and the proxy renames to Authorization on the way out. Correctness - 301/302/303 now actually switch to GET on the next hop. Previously only the body was cleared; the loop kept using request.method. Also drops Content-Length/Content-Type from forwarded headers when switching to GET, per RFC 7231. - new URL(location, currentUrl) wrapped in try/catch — malformed upstream Location header now returns 502 with a clear message instead of crashing into 500. CORS / browser UX - DPoP added to Access-Control-Allow-Headers (Solid-OIDC clients preflight DPoP). - WAC-Allow added to Access-Control-Expose-Headers so browser auth UX can read the pod's policy. - /proxy WAC 401 now applies setProxyCorsHeaders before the response, so browser sees the actual 401 instead of a generic CORS failure (same shape as the #374 fix on the git handler). Config - corsProxy added to BOOLEAN_KEYS so JSS_CORS_PROXY=true is parsed as a real boolean. corsProxyMaxBytes / corsProxyTimeoutMs / corsProxyMaxRedirects added to the numeric-key list so env-var values become ints. Verified end-to-end: - Authorization: Bearer pod-token → upstream sees (absent) - X-Upstream-Authorization: … → upstream sees Authorization: … - POST→302→GET httpbin → upstream method=GET, body empty - DPoP visible in Allow-Headers - WAC-Allow visible in Expose-Headers - 401 from /proxy carries proxy CORS headers --- src/config.js | 9 ++++- src/handlers/cors-proxy.js | 76 +++++++++++++++++++++++++++++++------- src/server.js | 6 ++- 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/src/config.js b/src/config.js index b4e3f73e..b2cd0ad9 100644 --- a/src/config.js +++ b/src/config.js @@ -219,6 +219,7 @@ const BOOLEAN_KEYS = new Set([ 'mashlib', 'mashlibCdn', 'git', + 'corsProxy', 'nostr', 'webrtc', 'terminal', @@ -254,7 +255,13 @@ function parseEnvValue(value, key) { } // Numeric values for known numeric keys - if ((key === 'port' || key === 'nostrMaxEvents' || key === 'payCost' || key === 'payRate') && !isNaN(value)) { + if ((key === 'port' || + key === 'nostrMaxEvents' || + key === 'payCost' || + key === 'payRate' || + key === 'corsProxyMaxBytes' || + key === 'corsProxyTimeoutMs' || + key === 'corsProxyMaxRedirects') && !isNaN(value)) { return parseInt(value, 10); } diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 0cc007da..0e6d23ec 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -25,14 +25,24 @@ import { Readable, Transform } from 'stream'; // CORS headers applied to every proxy response. Same shape/source-of-truth // pattern as GIT_CORS_HEADERS in src/handlers/git.js (#374). -const PROXY_CORS_HEADERS = { +// +// Allow-Headers includes: +// - Authorization, DPoP — for WAC + Solid-OIDC auth to *this* pod +// - X-Upstream-Authorization — opt-in upstream credential (renamed to +// Authorization on the way out, see pickRequestHeaders) +// - Git-Protocol — for browser-side smart-HTTP v2 clients +// - Accept*, Content-Type — standard fetch headers +// +// Expose-Headers includes WAC-Allow so browser clients can render auth +// UX based on the pod's policy, plus the usual content/etag/location set. +export const PROXY_CORS_HEADERS = { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, HEAD, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Git-Protocol, Accept, Accept-Encoding, Accept-Language', - 'Access-Control-Expose-Headers': 'Content-Type, Content-Length, ETag, Last-Modified, Link, Location, WWW-Authenticate', + 'Access-Control-Allow-Headers': 'Content-Type, Authorization, DPoP, X-Upstream-Authorization, Git-Protocol, Accept, Accept-Encoding, Accept-Language', + 'Access-Control-Expose-Headers': 'Content-Type, Content-Length, ETag, Last-Modified, Link, Location, WWW-Authenticate, WAC-Allow', }; -function setProxyCorsHeaders(reply) { +export function setProxyCorsHeaders(reply) { for (const [k, v] of Object.entries(PROXY_CORS_HEADERS)) { reply.header(k, v); } @@ -41,11 +51,21 @@ function setProxyCorsHeaders(reply) { // Headers we forward from the caller to the upstream. Anything not on // this list is dropped — origin/cookie/host/referer would either confuse // upstream auth or leak the proxying pod's identity. +// +// Authorization is deliberately NOT forwarded by default: the browser +// uses it to authenticate to *this* pod (WAC, Solid-OIDC bearer/DPoP), +// and silently leaking that token to an arbitrary upstream is a security +// hole. Callers who genuinely need to send credentials to the upstream +// (e.g. a GitHub PAT for a private repo) opt in via the +// X-Upstream-Authorization header — pickRequestHeaders renames that to +// Authorization on the way out. +// +// DPoP is similarly not forwarded — it's bound to this pod's URL and +// would be rejected by any upstream anyway. const FORWARD_REQUEST_HEADERS = new Set([ 'accept', 'accept-encoding', 'accept-language', - 'authorization', 'content-type', 'content-length', 'git-protocol', @@ -56,6 +76,8 @@ const FORWARD_REQUEST_HEADERS = new Set([ 'user-agent', ]); +const UPSTREAM_AUTH_HEADER = 'x-upstream-authorization'; + // Headers we strip from the upstream response before returning to the // caller. Encoding-related ones are removed because Node's fetch already // transparently decodes; passing through Content-Encoding would @@ -79,8 +101,15 @@ function pickRequestHeaders(reqHeaders) { const out = {}; for (const [k, v] of Object.entries(reqHeaders)) { if (v == null) continue; - if (FORWARD_REQUEST_HEADERS.has(k.toLowerCase())) { + const lower = k.toLowerCase(); + if (FORWARD_REQUEST_HEADERS.has(lower)) { out[k] = v; + } else if (lower === UPSTREAM_AUTH_HEADER) { + // Opt-in upstream credential: client supplies the token under + // X-Upstream-Authorization, we relay it as Authorization upstream. + // Pod's own Authorization (used to auth to /proxy) never leaves + // the pod — see FORWARD_REQUEST_HEADERS. + out['Authorization'] = v; } } return out; @@ -155,18 +184,24 @@ export async function handleCorsProxy(request, reply, options = {}) { // each hop's Location header. Required because fetch's automatic redirect // following bypasses our SSRF guard at the second hop. let currentUrl = targetUrl; + let currentMethod = request.method; let redirectsLeft = maxRedirects; // Body for non-GET/HEAD methods. Read once up front; we may need it on // the first hop and (for 307/308) on redirected hops. let body = null; - if (request.method !== 'GET' && request.method !== 'HEAD') { + if (currentMethod !== 'GET' && currentMethod !== 'HEAD') { body = request.rawBody ?? request.body; if (typeof body === 'object' && !(body instanceof Buffer) && body !== null) { body = JSON.stringify(body); } } + // Forwarded headers are computed once (not per-hop) — Content-Length + // gets dropped when we switch to GET on 301/302/303 to avoid sending + // a stale length for an absent body. + const forwardHeaders = pickRequestHeaders(request.headers); + while (true) { const validation = await validateExternalUrl(currentUrl, { requireHttps: false, // allow http:// — pod operator can lock down via .acl scope @@ -183,9 +218,9 @@ export async function handleCorsProxy(request, reply, options = {}) { let upstream; try { upstream = await fetch(currentUrl, { - method: request.method, - headers: pickRequestHeaders(request.headers), - body, + method: currentMethod, + headers: forwardHeaders, + body: (currentMethod === 'GET' || currentMethod === 'HEAD') ? undefined : body, redirect: 'manual', signal: abortController.signal, }); @@ -198,7 +233,8 @@ export async function handleCorsProxy(request, reply, options = {}) { } clearTimeout(timeoutId); - // Manual redirect handling: 301/302/303 → GET, 307/308 → preserve method + // Manual redirect handling: 301/302/303 → GET (per HTTP semantics), + // 307/308 → preserve method and body. if ([301, 302, 303, 307, 308].includes(upstream.status)) { const location = upstream.headers.get('location'); if (!location) { @@ -207,11 +243,23 @@ export async function handleCorsProxy(request, reply, options = {}) { if (--redirectsLeft < 0) { return reply.code(502).send({ error: `Exceeded ${maxRedirects} redirects` }); } - // Resolve relative redirects against the previous URL. - currentUrl = new URL(location, currentUrl).toString(); - // 301/302/303 force the next request to GET with no body. + // Resolve relative redirects against the previous URL. Malformed + // Location values throw — bubble that up as a 502 instead of 500. + try { + currentUrl = new URL(location, currentUrl).toString(); + } catch { + return reply.code(502).send({ error: 'Upstream redirect with malformed Location', location }); + } if ([301, 302, 303].includes(upstream.status)) { + // Method/body change is mandated by RFC 7231: redirected request + // becomes GET with no body. Also drop content-length / content-type + // so we don't send headers that no longer match the body. + currentMethod = 'GET'; body = null; + delete forwardHeaders['content-length']; + delete forwardHeaders['Content-Length']; + delete forwardHeaders['content-type']; + delete forwardHeaders['Content-Type']; } // Drain the redirect body to free the connection; we never return it. try { await upstream.body?.cancel(); } catch { /* ignore */ } diff --git a/src/server.js b/src/server.js index 67dcc591..ffb7f6c0 100644 --- a/src/server.js +++ b/src/server.js @@ -12,7 +12,7 @@ import { notificationsPlugin } from './notifications/index.js'; import { startFileWatcher } from './notifications/events.js'; import { idpPlugin } from './idp/index.js'; import { isGitRequest, isGitWriteOperation, handleGit } from './handlers/git.js'; -import { handleCorsProxy, isCorsProxyRequest } from './handlers/cors-proxy.js'; +import { handleCorsProxy, isCorsProxyRequest, setProxyCorsHeaders } from './handlers/cors-proxy.js'; import { AccessMode } from './wac/parser.js'; import { registerNostrRelay } from './nostr/relay.js'; import { createPayHandler, isPayRequest } from './handlers/pay.js'; @@ -461,6 +461,10 @@ export function createServer(options = {}) { request.wacAllow = wacAllow; if (request.method !== 'OPTIONS' && !authorized) { + // Apply proxy CORS headers BEFORE handleUnauthorized so the 401/403 + // is readable by browser clients (without these the browser surfaces + // the response as a generic CORS failure — same shape as #374). + setProxyCorsHeaders(reply); reply.header('WAC-Allow', wacAllow); return handleUnauthorized(request, reply, webId !== null, wacAllow, authError); } From 5f982d79d5262e05fd975b4142a63c834654fbf5 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 09:20:05 +0200 Subject: [PATCH 03/18] =?UTF-8?q?cors-proxy:=20tighten=20Copilot=20review?= =?UTF-8?q?=20pass=202=20=E2=80=94=20timeout=20scope,=20port=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per the second Copilot review on #379: - Make the timeout's actual scope explicit in code, comments, and the CLI/config description: the deadline applies to the headers-received phase. Once Fastify has sent the 200 status to the client we can't rewrite to a 504, and trying to destroy the upstream pipe mid-stream doesn't reliably terminate the response (verified empirically). The honest fix for slow-streaming uploads needs a different shape (idle timeout that closes the socket without trying to flip the status) — filing as a follow-up. - Mid-stream errors (byte cap, fetch error) now end the response with a truncated body rather than throwing, so they don't surface as ERR_HTTP_HEADERS_SENT. - streamWithCap renamed to streamUpstream (no longer holds the timeout). Port allowlisting was mentioned in #378's framing but never wired into validateExternalUrl or this handler. Updating the PR body to remove the claim; revisit if a real use case for restricting upstream ports emerges (we have WAC + SSRF as the actual auth/safety primitives). Verified locally still passes the regression set: - fast → 200 - slow headers (delay/5, timeout=2s) → 504 at ~2s - example.com → 200 - zero uncaught exceptions in log --- src/config.js | 2 +- src/handlers/cors-proxy.js | 29 +++++++++++++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/config.js b/src/config.js index b2cd0ad9..301e56f5 100644 --- a/src/config.js +++ b/src/config.js @@ -50,7 +50,7 @@ export const defaults = { // fetch arbitrary upstreams that don't return CORS headers. corsProxy: false, corsProxyMaxBytes: 50 * 1024 * 1024, // 50 MB ceiling on upstream response size - corsProxyTimeoutMs: 30_000, // 30 s overall request deadline + corsProxyTimeoutMs: 30_000, // 30 s deadline for upstream to send headers (504 if exceeded). Body streaming is not currently capped; see follow-up. corsProxyMaxRedirects: 5, // each redirect re-validated for SSRF // Nostr relay diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 0e6d23ec..4d1b830a 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -125,10 +125,18 @@ function copyResponseHeaders(reply, fetchResponse) { /** * Stream the upstream body to the caller, enforcing a byte cap mid-stream. * Uses Node 18+ Readable.fromWeb to bridge the fetch ReadableStream into - * a Node Readable, then a Transform passthrough counts bytes and errors - * the pipeline if maxBytes is exceeded. + * a Node Readable, then a Transform passthrough counts bytes and ends + * the stream cleanly if maxBytes is exceeded. + * + * Note on the timeout: once status/headers are sent we can't change the + * status code (Fastify throws ERR_HTTP_HEADERS_SENT), so the deadline + * applies to the headers-received phase only. A slow-streaming upstream + * past the deadline is *not* killed mid-stream in Phase 1 — see the + * follow-up tracked in #378 / the open issue list. Mid-stream errors + * (byte cap, fetch error) terminate the response with a truncated body + * rather than a thrown error. */ -function streamWithCap(reply, fetchResponse, maxBytes, abortController) { +function streamUpstream(reply, fetchResponse, maxBytes, abortController) { if (!fetchResponse.body) { return reply.send(); } @@ -139,13 +147,17 @@ function streamWithCap(reply, fetchResponse, maxBytes, abortController) { bytesSeen += chunk.length; if (bytesSeen > maxBytes) { abortController.abort(); - return callback(new Error(`Upstream response exceeded ${maxBytes} bytes`)); + this.push(null); + return callback(); } callback(null, chunk); } }); - Readable.fromWeb(fetchResponse.body).on('error', (err) => counter.destroy(err)).pipe(counter); + Readable.fromWeb(fetchResponse.body) + .on('error', () => counter.end()) + .pipe(counter); + return reply.send(counter); } @@ -231,6 +243,11 @@ export async function handleCorsProxy(request, reply, options = {}) { } return reply.code(502).send({ error: 'Upstream fetch failed', detail: err.message }); } + // Headers received — clear the deadline. Streaming-phase timeouts + // are a known Phase 1 limitation: once Fastify has sent headers we + // can't change the status code, and the destroy-source-during-pipe + // pattern doesn't reliably terminate the response. Tracked as a + // follow-up. clearTimeout(timeoutId); // Manual redirect handling: 301/302/303 → GET (per HTTP semantics), @@ -271,7 +288,7 @@ export async function handleCorsProxy(request, reply, options = {}) { copyResponseHeaders(reply, upstream); setProxyCorsHeaders(reply); // reapply in case copyResponseHeaders set conflicting CORS values - return streamWithCap(reply, upstream, maxBytes, abortController); + return streamUpstream(reply, upstream, maxBytes, abortController); } } From 04130f0996bc70dd6fabe0a6cfd289848e919bfd Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 09:32:53 +0200 Subject: [PATCH 04/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=203?= =?UTF-8?q?=20=E2=80=94=20strip=20Content-Length,=20expand=20Allow-Headers?= =?UTF-8?q?,=20override=20Allow-Credentials?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes: 1. Strip upstream Content-Length from the response. We strip Content-Encoding (Node's fetch already decoded), and the byte cap may truncate mid-stream — in both cases the upstream length is wrong and clients hit ERR_CONTENT_LENGTH_MISMATCH or hang waiting for bytes that never come. Node sends actual length or chunked encoding automatically. 2. Add If-Match / If-None-Match / If-Modified-Since / Range / User-Agent to PROXY_CORS_HEADERS Allow-Headers. We already forward these via FORWARD_REQUEST_HEADERS, but if the browser sets one and they're not in the preflight allowlist the request is rejected before reaching us. Allow-Headers now mirrors FORWARD_REQUEST_HEADERS. 3. Set Access-Control-Allow-Credentials: 'false' explicitly. The server's global CORS hook sets it to 'true' for normal pod routes, but combining 'true' with Allow-Origin: * is a CORS spec violation that browsers reject (credentialed mode requires a specific origin). For an anonymous-readable proxy with Cookie stripped, non-credentialed mode is the correct shape — and 'false' overrides the global hook cleanly. Verified locally: HEAD request to /proxy?url=https://example.com/ shows all three changes in the response headers. --- src/handlers/cors-proxy.js | 40 +++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 4d1b830a..5b283e3f 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -26,20 +26,42 @@ import { Readable, Transform } from 'stream'; // CORS headers applied to every proxy response. Same shape/source-of-truth // pattern as GIT_CORS_HEADERS in src/handlers/git.js (#374). // -// Allow-Headers includes: +// Allow-Headers must list every request header the proxy actually +// forwards (see FORWARD_REQUEST_HEADERS) — otherwise browsers reject +// preflight before the request reaches us. Plus: // - Authorization, DPoP — for WAC + Solid-OIDC auth to *this* pod // - X-Upstream-Authorization — opt-in upstream credential (renamed to // Authorization on the way out, see pickRequestHeaders) -// - Git-Protocol — for browser-side smart-HTTP v2 clients -// - Accept*, Content-Type — standard fetch headers // // Expose-Headers includes WAC-Allow so browser clients can render auth // UX based on the pod's policy, plus the usual content/etag/location set. +// +// Allow-Credentials is set explicitly to 'false' to override the +// server-wide global CORS hook (which uses 'true' for the rest of the +// pod). Combining 'true' with `Allow-Origin: *` is a CORS spec +// violation that browsers reject — and an anonymous-readable proxy +// doesn't have a use case for credentialed cross-origin anyway (Cookie +// is stripped before forwarding). export const PROXY_CORS_HEADERS = { 'Access-Control-Allow-Origin': '*', + 'Access-Control-Allow-Credentials': 'false', 'Access-Control-Allow-Methods': 'GET, POST, HEAD, OPTIONS', - 'Access-Control-Allow-Headers': 'Content-Type, Authorization, DPoP, X-Upstream-Authorization, Git-Protocol, Accept, Accept-Encoding, Accept-Language', - 'Access-Control-Expose-Headers': 'Content-Type, Content-Length, ETag, Last-Modified, Link, Location, WWW-Authenticate, WAC-Allow', + 'Access-Control-Allow-Headers': [ + 'Content-Type', + 'Authorization', + 'DPoP', + 'X-Upstream-Authorization', + 'Git-Protocol', + 'Accept', + 'Accept-Encoding', + 'Accept-Language', + 'If-Match', + 'If-None-Match', + 'If-Modified-Since', + 'Range', + 'User-Agent', + ].join(', '), + 'Access-Control-Expose-Headers': 'Content-Type, ETag, Last-Modified, Link, Location, WWW-Authenticate, WAC-Allow', }; export function setProxyCorsHeaders(reply) { @@ -82,8 +104,16 @@ const UPSTREAM_AUTH_HEADER = 'x-upstream-authorization'; // caller. Encoding-related ones are removed because Node's fetch already // transparently decodes; passing through Content-Encoding would // double-decompress in the browser. +// +// Content-Length is stripped because (a) we strip Content-Encoding so +// the body length may differ from the upstream-declared length after +// decompression, and (b) we may truncate mid-stream when maxBytes is +// exceeded. Forwarding the upstream Content-Length in either case +// causes ERR_CONTENT_LENGTH_MISMATCH or hangs in the client. Node sends +// the actual length (or chunked) automatically. const STRIP_RESPONSE_HEADERS = new Set([ 'content-encoding', + 'content-length', 'transfer-encoding', 'connection', 'keep-alive', From 93af554d5c7d32f2066cdcf275ea11f52b571e85 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 14:57:43 +0200 Subject: [PATCH 05/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=204?= =?UTF-8?q?=20=E2=80=94=20per-method=20ACL=20mode,=20numeric=20guards,=20c?= =?UTF-8?q?omment=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes per the latest review: 1. /proxy preHandler no longer hard-codes requiredMode: AccessMode.READ for every method. authorize() now derives the mode from the request method via getRequiredMode() — GET/HEAD need Read on /proxy, POST needs Append/Write. Pod owners can grant these separately so an ACL that allows browse-only access doesn't accidentally permit side-effecting POSTs through to upstream. 2. corsProxy* numeric options validated as finite positive numbers in server.js. Previously the ?? fallback only caught null/undefined; a non-numeric env var (JSS_CORS_PROXY_MAX_BYTES=banana) or JSON config value would have flowed through as a string and silently disabled the safety limit (bytesSeen > "banana" is always false). New positiveInt() helper falls back to the documented default for any non-positive-finite-number input. corsProxy itself is now `=== true` to reject string-truthy "false" values from misconfiguration. 3. config.js comment for corsProxyTimeoutMs clarified: the timeout doesn't apply during body streaming, but body *size* is capped by corsProxyMaxBytes. Earlier wording suggested no body-side cap at all, which was misleading. Verified locally: - Boot clean with JSS_CORS_PROXY_MAX_BYTES=banana (falls back to 50MB) - Anonymous GET and POST both 401 against non-public pod (mode now derived per-method by authorize()) --- src/config.js | 2 +- src/server.js | 24 ++++++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/config.js b/src/config.js index 301e56f5..a0d2225d 100644 --- a/src/config.js +++ b/src/config.js @@ -50,7 +50,7 @@ export const defaults = { // fetch arbitrary upstreams that don't return CORS headers. corsProxy: false, corsProxyMaxBytes: 50 * 1024 * 1024, // 50 MB ceiling on upstream response size - corsProxyTimeoutMs: 30_000, // 30 s deadline for upstream to send headers (504 if exceeded). Body streaming is not currently capped; see follow-up. + corsProxyTimeoutMs: 30_000, // 30 s deadline for upstream to send headers (504 if exceeded). The timeout does not apply during body streaming — body size is capped by corsProxyMaxBytes; see follow-up for streaming-phase timeout. corsProxyMaxRedirects: 5, // each redirect re-validated for SSRF // Nostr relay diff --git a/src/server.js b/src/server.js index ffb7f6c0..d9a6982c 100644 --- a/src/server.js +++ b/src/server.js @@ -72,11 +72,18 @@ export function createServer(options = {}) { const mashlibVersion = options.mashlibVersion ?? '2.0.0'; // Git HTTP backend is OFF by default - enables clone/push via git protocol const gitEnabled = options.git ?? false; - // CORS proxy (#378) — OFF by default - const corsProxyEnabled = options.corsProxy ?? false; - const corsProxyMaxBytes = options.corsProxyMaxBytes ?? 50 * 1024 * 1024; - const corsProxyTimeoutMs = options.corsProxyTimeoutMs ?? 30_000; - const corsProxyMaxRedirects = options.corsProxyMaxRedirects ?? 5; + // CORS proxy (#378) — OFF by default. Numeric settings get the + // sane-default fallback if the env var or config file supplies a + // non-finite/non-positive value (e.g. JSS_CORS_PROXY_MAX_BYTES=banana + // would otherwise leave the cap as the string "banana", making + // `bytesSeen > "banana"` always false and silently disabling the + // safety limit). + const positiveInt = (v, fallback) => + (typeof v === 'number' && Number.isFinite(v) && v > 0) ? v : fallback; + const corsProxyEnabled = options.corsProxy === true; + const corsProxyMaxBytes = positiveInt(options.corsProxyMaxBytes, 50 * 1024 * 1024); + const corsProxyTimeoutMs = positiveInt(options.corsProxyTimeoutMs, 30_000); + const corsProxyMaxRedirects = positiveInt(options.corsProxyMaxRedirects, 5); // Nostr relay is OFF by default const nostrEnabled = options.nostr ?? false; const nostrPath = options.nostrPath ?? '/relay'; @@ -456,7 +463,12 @@ export function createServer(options = {}) { return; } - const { authorized, webId, wacAllow, authError } = await authorize(request, reply, { requiredMode: AccessMode.READ }); + // Don't override requiredMode — let authorize() derive it from the + // request method via getRequiredMode(). GET/HEAD need READ on the + // /proxy resource, POST needs APPEND/WRITE — pod owners can grant + // these separately via ACL modes (e.g. acl:Read for browse-only, + // acl:Append/Write for proxying side-effecting POSTs upstream). + const { authorized, webId, wacAllow, authError } = await authorize(request, reply); request.webId = webId; request.wacAllow = wacAllow; From 3208a296e8e9eae031d41e80fc20e267b96d30ca Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 16:13:21 +0200 Subject: [PATCH 06/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=205?= =?UTF-8?q?=20=E2=80=94=20paymentRequired=20pass-through,=20drop=20forward?= =?UTF-8?q?ed=20content-length?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: 1. /proxy preHandler now handles paymentRequired from authorize(), same shape as the git hook. Pod owners who attach a PaymentCondition to the /proxy ACL get a 402 with the payment challenge instead of 401, matching every other WAC-gated endpoint. 2. content-length removed from FORWARD_REQUEST_HEADERS. We may re-stringify Fastify-parsed JSON bodies before the upstream fetch, so the caller's declared length doesn't match what we send. Node's fetch sets Content-Length automatically based on the actual body. (Browsers don't need it in Allow-Headers — Content-Length is CORS-safelisted on simple requests.) Verified locally: - POST text body 34 bytes → upstream sees content-length: 34 - POST JSON {"x":42,"y":"hi"} → upstream sees content-length: 17 (matches the re-stringified body, not what curl sent) --- src/handlers/cors-proxy.js | 7 ++++++- src/server.js | 10 +++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 5b283e3f..de2f0ad4 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -84,12 +84,17 @@ export function setProxyCorsHeaders(reply) { // // DPoP is similarly not forwarded — it's bound to this pod's URL and // would be rejected by any upstream anyway. +// Note: Content-Length is *not* forwarded. We may transform the body +// (Fastify-parsed JSON gets re-stringified before the upstream fetch), +// so the caller's declared length may not match what we send. Node's +// fetch sets Content-Length automatically based on the actual body. +// (Browsers send Content-Length on simple requests without needing it +// in Access-Control-Allow-Headers, since it's CORS-safelisted.) const FORWARD_REQUEST_HEADERS = new Set([ 'accept', 'accept-encoding', 'accept-language', 'content-type', - 'content-length', 'git-protocol', 'if-match', 'if-none-match', diff --git a/src/server.js b/src/server.js index d9a6982c..b4399afa 100644 --- a/src/server.js +++ b/src/server.js @@ -468,10 +468,18 @@ export function createServer(options = {}) { // /proxy resource, POST needs APPEND/WRITE — pod owners can grant // these separately via ACL modes (e.g. acl:Read for browse-only, // acl:Append/Write for proxying side-effecting POSTs upstream). - const { authorized, webId, wacAllow, authError } = await authorize(request, reply); + const { authorized, webId, wacAllow, authError, paymentRequired } = await authorize(request, reply); request.webId = webId; request.wacAllow = wacAllow; + // ACL with a PaymentCondition surfaces as 402 here — mirrors the + // git handler at src/server.js:418 and the standard WAC hook so + // payment-gated /proxy ACLs behave consistently. + if (paymentRequired) { + setProxyCorsHeaders(reply); + return reply.code(402).send({ type: 'PaymentRequired', ...paymentRequired }); + } + if (request.method !== 'OPTIONS' && !authorized) { // Apply proxy CORS headers BEFORE handleUnauthorized so the 401/403 // is readable by browser clients (without these the browser surfaces From c1a469c498a325531777d07dd8a1da21a34423d8 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 16:42:10 +0200 Subject: [PATCH 07/18] cors-proxy: emit partial slice at byte cap instead of dropping whole chunk Per Copilot review pass 6: The Transform's previous behavior was "if this chunk pushes us over the cap, drop the entire chunk and end the stream." For a small maxBytes (e.g. 100) the first incoming chunk (typically a full TLS record, ~16KB) would always exceed the cap, and the client received an empty body despite the cap being well above zero. Fixed by emitting the partial slice up to the cap, then push(null) + abort. Verified locally: --cors-proxy-max-bytes 100 against example.com now returns 100 bytes (the leading "..." HTML), not zero. --- src/handlers/cors-proxy.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index de2f0ad4..4a8cec76 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -176,16 +176,29 @@ function streamUpstream(reply, fetchResponse, maxBytes, abortController) { return reply.send(); } + // Partial-slice emission at the cap: if a single chunk would push us + // over, emit only the bytes up to the cap, then end the stream. The + // earlier "drop the whole chunk" version meant a small maxBytes (e.g. + // 100 against a typical TLS record-sized chunk) returned an empty + // response, even though the cap was much larger than zero. let bytesSeen = 0; const counter = new Transform({ transform(chunk, _encoding, callback) { - bytesSeen += chunk.length; - if (bytesSeen > maxBytes) { - abortController.abort(); - this.push(null); + const remaining = maxBytes - bytesSeen; + if (remaining <= 0) { + // Already at cap; quietly drop subsequent chunks until upstream EOF. return callback(); } - callback(null, chunk); + if (chunk.length <= remaining) { + bytesSeen += chunk.length; + return callback(null, chunk); + } + // Emit partial slice up to the cap, then end cleanly. + bytesSeen += remaining; + this.push(chunk.slice(0, remaining)); + this.push(null); + abortController.abort(); + return callback(); } }); From 0f51b608a7bd381e0d5425dd7c2344c0e30692ed Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 16:52:43 +0200 Subject: [PATCH 08/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=207?= =?UTF-8?q?=20=E2=80=94=20virtual-resource=20WAC=20+=20byte-cap=20boundary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more: 1. authorize() has a "non-existent resource + write method → check parent container" fallback in src/auth/middleware.js:101. /proxy is a virtual endpoint with no backing storage, so POST /proxy was getting authorized against the parent (/) instead of /proxy itself. That's too permissive: an ACL granting acl:Read on / would let any Solid client POST through the proxy. Added a `skipParentForMissing` option to authorize() that disables the parent-fallback. The cors-proxy preHandler now passes it. WAC now correctly checks /proxy directly regardless of storage state. 2. The byte-cap Transform's exact-boundary case (chunk.length === remaining) used to pass the chunk through unchanged and continue reading. Subsequent chunks fell into the `remaining <= 0` drop branch but the stream stayed open — slow-streaming clients could hang and the upstream kept sending bytes we silently discarded. Fixed by collapsing both "fills exactly" and "would exceed" branches into a single "emit slice, end stream, abort" path. Verified locally: - cap=100 against example.com → 200 with 100 bytes, ~0.1 s - POST anonymous on a non-public pod → 401 (was previously evaluated against /, now against /proxy) --- src/auth/middleware.js | 6 +++++- src/handlers/cors-proxy.js | 27 +++++++++++++++++---------- src/server.js | 9 ++++++++- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/auth/middleware.js b/src/auth/middleware.js index 88007dda..535553ad 100644 --- a/src/auth/middleware.js +++ b/src/auth/middleware.js @@ -98,7 +98,7 @@ export async function authorize(request, reply, options = {}) { let checkUrl = resourceUrl; let checkIsContainer = isContainer; - if (!resourceExists && (method === 'PUT' || method === 'POST' || method === 'PATCH')) { + if (!resourceExists && (method === 'PUT' || method === 'POST' || method === 'PATCH') && !options.skipParentForMissing) { // Check write permission on parent container const parentPath = getParentPath(storagePath); checkPath = parentPath; @@ -107,6 +107,10 @@ export async function authorize(request, reply, options = {}) { checkUrl = buildResourceUrl(request, parentUrlPath); checkIsContainer = true; } + // skipParentForMissing: callers with virtual endpoints (e.g. /proxy in + // #378) want WAC checked against the URL path itself even when no + // backing storage exists. Without this opt-out, POST /proxy on a + // single-user pod gets authorized against /, which is too permissive. // Check WAC permissions const { allowed, wacAllow, paymentRequired, paid, balance, currency } = await checkAccess({ diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 4a8cec76..791ed20c 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -176,26 +176,33 @@ function streamUpstream(reply, fetchResponse, maxBytes, abortController) { return reply.send(); } - // Partial-slice emission at the cap: if a single chunk would push us - // over, emit only the bytes up to the cap, then end the stream. The - // earlier "drop the whole chunk" version meant a small maxBytes (e.g. - // 100 against a typical TLS record-sized chunk) returned an empty - // response, even though the cap was much larger than zero. + // Byte cap with two boundary cases handled: + // - chunk smaller than remaining: pass through, keep counting + // - chunk meets or exceeds remaining: emit just enough to fill the + // cap and end the stream right then. Previously a chunk that hit + // the cap *exactly* (chunk.length === remaining) was passed + // through without ending — subsequent chunks fell into the + // "remaining <= 0" branch and were dropped silently while the + // stream stayed open, which can hang slow-streaming clients. let bytesSeen = 0; const counter = new Transform({ transform(chunk, _encoding, callback) { const remaining = maxBytes - bytesSeen; if (remaining <= 0) { - // Already at cap; quietly drop subsequent chunks until upstream EOF. + // Defensive: shouldn't happen because we end on first overflow, + // but if a chunk arrives after we've started shutting down, + // drop it. return callback(); } - if (chunk.length <= remaining) { + if (chunk.length < remaining) { bytesSeen += chunk.length; return callback(null, chunk); } - // Emit partial slice up to the cap, then end cleanly. - bytesSeen += remaining; - this.push(chunk.slice(0, remaining)); + // chunk fills or exceeds the cap — emit (up to) `remaining` bytes + // and end the stream right now. + const slice = chunk.length === remaining ? chunk : chunk.slice(0, remaining); + bytesSeen += slice.length; + this.push(slice); this.push(null); abortController.abort(); return callback(); diff --git a/src/server.js b/src/server.js index b4399afa..bd5c75e3 100644 --- a/src/server.js +++ b/src/server.js @@ -468,7 +468,14 @@ export function createServer(options = {}) { // /proxy resource, POST needs APPEND/WRITE — pod owners can grant // these separately via ACL modes (e.g. acl:Read for browse-only, // acl:Append/Write for proxying side-effecting POSTs upstream). - const { authorized, webId, wacAllow, authError, paymentRequired } = await authorize(request, reply); + // + // skipParentForMissing prevents authorize()'s "non-existent resource + + // write method → check parent container" fallback from kicking in. + // /proxy is a virtual endpoint with no backing storage, so the + // fallback would route POST authorization to / (the root) instead + // of /proxy — too permissive. With this flag, authorize() checks + // ACLs against /proxy directly regardless of storage existence. + const { authorized, webId, wacAllow, authError, paymentRequired } = await authorize(request, reply, { skipParentForMissing: true }); request.webId = webId; request.wacAllow = wacAllow; From 22f0b89afba3d7d9139871f9ce98ceb869639ebf Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 17:05:45 +0200 Subject: [PATCH 09/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=208?= =?UTF-8?q?=20=E2=80=94=20strip=20Authorization=20on=20cross-origin=20redi?= =?UTF-8?q?rect,=20JSDoc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes: 1. SECURITY: strip Authorization on cross-origin redirects. forwardHeaders is computed once and reused across hops. If the client opted in to upstream credentials via X-Upstream-Authorization, that token (renamed to Authorization) was being sent unchanged to every redirect target. A redirect from github.com → evil.com would leak the user's GitHub PAT to the attacker. Now we recompute the target origin per hop and delete Authorization when it differs from the initial origin — matches curl/browser/well-behaved-client behavior. Auth still rides along on same-origin redirects. 2. authorize() JSDoc updated to document skipParentForMissing — the new option that lets virtual endpoints (like /proxy) opt out of the non-existent-resource → check-parent fallback. Includes the why (without it, POST /proxy authorizes against / instead of /proxy). 3. PR description updated separately via the REST API to remove a stale "forwards Authorization" claim (gh pr edit --body was silently no-op'ing for me). The current header policy section explicitly states Authorization is NOT forwarded by default and describes the X-Upstream-Authorization opt-in. --- src/auth/middleware.js | 12 ++++++++++-- src/handlers/cors-proxy.js | 29 ++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/auth/middleware.js b/src/auth/middleware.js index 535553ad..519058b0 100644 --- a/src/auth/middleware.js +++ b/src/auth/middleware.js @@ -48,8 +48,16 @@ export function buildResourceUrl(request, urlPath) { * @param {object} request - Fastify request * @param {object} reply - Fastify reply * @param {object} options - Optional settings - * @param {string} options.requiredMode - Override the required access mode (e.g., 'Write' for git push) - * @returns {Promise<{authorized: boolean, webId: string|null, wacAllow: string, authError: string|null}>} + * @param {string} [options.requiredMode] - Override the required access mode + * (e.g., 'Write' for git push). Defaults to getRequiredMode(method). + * @param {boolean} [options.skipParentForMissing] - When true, skip the + * "non-existent resource + write method → check parent container" + * fallback. Used by virtual endpoints (e.g. `/proxy` in #378) that have + * no backing storage but still want WAC checked against the URL itself. + * Without this flag, POST/PUT/PATCH on a missing resource is authorized + * against the parent (e.g. `/proxy` falls back to `/`), which is too + * permissive for endpoints whose ACL is meant to live at that path. + * @returns {Promise<{authorized: boolean, webId: string|null, wacAllow: string, authError: string|null, paymentRequired?: object}>} */ export async function authorize(request, reply, options = {}) { const urlPath = request.url.split('?')[0]; diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 791ed20c..855501dc 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -264,10 +264,16 @@ export async function handleCorsProxy(request, reply, options = {}) { } } - // Forwarded headers are computed once (not per-hop) — Content-Length - // gets dropped when we switch to GET on 301/302/303 to avoid sending - // a stale length for an absent body. + // Forwarded headers are computed once and reused across redirect hops, + // EXCEPT Authorization. If the client opted in via X-Upstream-Authorization, + // its credential is now in forwardHeaders.Authorization. We must strip + // it on cross-origin redirects: a redirect to evil.com would otherwise + // leak the user's upstream token (e.g. a GitHub PAT) to whoever the + // attacker controls. See cross-origin redirect handling below. const forwardHeaders = pickRequestHeaders(request.headers); + const initialOrigin = (() => { + try { return new URL(currentUrl).origin; } catch { return null; } + })(); while (true) { const validation = await validateExternalUrl(currentUrl, { @@ -333,6 +339,23 @@ export async function handleCorsProxy(request, reply, options = {}) { delete forwardHeaders['content-type']; delete forwardHeaders['Content-Type']; } + // Cross-origin redirect: strip Authorization to prevent leaking + // the X-Upstream-Authorization-derived credential to a different + // origin. Curl, browsers, and well-behaved HTTP clients all do + // this. The opt-in upstream auth is bound to the origin the + // client requested. + try { + const nextOrigin = new URL(currentUrl).origin; + if (initialOrigin && nextOrigin !== initialOrigin) { + delete forwardHeaders['authorization']; + delete forwardHeaders['Authorization']; + } + } catch { + // currentUrl was just validated above, so this should never + // throw; if it somehow does, drop Authorization to fail safe. + delete forwardHeaders['authorization']; + delete forwardHeaders['Authorization']; + } // Drain the redirect body to free the connection; we never return it. try { await upstream.body?.cancel(); } catch { /* ignore */ } continue; From c860022675aca5d7f55c57c094c55a662609e317 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 17:50:59 +0200 Subject: [PATCH 10/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=209?= =?UTF-8?q?=20=E2=80=94=20WAC-Allow=20on=20402,=20Allow=20on=20405,=20drop?= =?UTF-8?q?=20dead=20branch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small fixes: 1. 402 PaymentRequired response in the proxy preHandler now sets WAC-Allow before sending. Matches the global WAC hook and 401/403 paths; consistent header surface for browser clients (which can already read it via Access-Control-Expose-Headers). 2. 405 Method Not Allowed now carries an Allow header listing the supported methods (GET, POST, HEAD, OPTIONS). RFC 9110 requires this; without it, well-behaved clients can't discover what's acceptable. 3. isCorsProxyRequest() simplified to `urlPath === '/proxy'`. The `startsWith('/proxy?')` branch was dead code — every callsite already passes `request.url.split('?')[0]`. JSDoc updated to document that contract. Verified: PUT /proxy → 405 with `allow: GET, POST, HEAD, OPTIONS` GET /proxy?url=… on public pod → 200 GET /proxy on non-public → 401 with WAC-Allow + full proxy CORS headers --- src/handlers/cors-proxy.js | 9 ++++++--- src/server.js | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 855501dc..b06c913e 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -235,6 +235,8 @@ export async function handleCorsProxy(request, reply, options = {}) { } if (!ALLOWED_METHODS.has(request.method)) { + // HTTP requires 405 to carry an Allow header listing supported methods. + reply.header('Allow', [...ALLOWED_METHODS].join(', ')); return reply.code(405).send({ error: 'Method not allowed', method: request.method }); } @@ -371,10 +373,11 @@ export async function handleCorsProxy(request, reply, options = {}) { } /** - * Match a request URL against the proxy route. + * Match a request URL *path* (not full URL) against the proxy route. * Used by server.js to decide whether the proxy preHandler should fire - * and whether the standard WAC hook should skip. + * and whether the standard WAC hook should skip. Callers must pass the + * already-split path component (e.g. `request.url.split('?')[0]`). */ export function isCorsProxyRequest(urlPath) { - return urlPath === '/proxy' || urlPath.startsWith('/proxy?'); + return urlPath === '/proxy'; } diff --git a/src/server.js b/src/server.js index bd5c75e3..113f2bad 100644 --- a/src/server.js +++ b/src/server.js @@ -484,6 +484,7 @@ export function createServer(options = {}) { // payment-gated /proxy ACLs behave consistently. if (paymentRequired) { setProxyCorsHeaders(reply); + reply.header('WAC-Allow', wacAllow); return reply.code(402).send({ type: 'PaymentRequired', ...paymentRequired }); } From 46ae2f47571e5963d0b2d035627c4f00d952c32e Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 17:57:49 +0200 Subject: [PATCH 11/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?0=20=E2=80=94=20JSDoc=20completeness,=20comment=20accuracy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both documentation polish, no behavioral changes: 1. authorize() JSDoc return type now lists every field the function actually returns (paid, balance, currency in addition to the already-documented paymentRequired). Callers in server.js use these for paid-access headers. 2. cors-proxy.js comment that suggested "Fastify-parsed JSON gets re-stringified" was misleading. JSS registers a wildcard parseAs:'buffer' parser at server.js:190, so request bodies arrive as Buffer and pass through unchanged. The defensive JSON.stringify branch remains for safety, but the comment now reflects that it's a fallback rather than the common path. --- src/auth/middleware.js | 11 ++++++++++- src/handlers/cors-proxy.js | 9 ++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/auth/middleware.js b/src/auth/middleware.js index 519058b0..2f40d927 100644 --- a/src/auth/middleware.js +++ b/src/auth/middleware.js @@ -57,7 +57,16 @@ export function buildResourceUrl(request, urlPath) { * Without this flag, POST/PUT/PATCH on a missing resource is authorized * against the parent (e.g. `/proxy` falls back to `/`), which is too * permissive for endpoints whose ACL is meant to live at that path. - * @returns {Promise<{authorized: boolean, webId: string|null, wacAllow: string, authError: string|null, paymentRequired?: object}>} + * @returns {Promise<{ + * authorized: boolean, + * webId: string|null, + * wacAllow: string, + * authError: string|null, + * paymentRequired?: object, + * paid?: boolean, + * balance?: number, + * currency?: string + * }>} */ export async function authorize(request, reply, options = {}) { const urlPath = request.url.split('?')[0]; diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index b06c913e..46edf444 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -84,9 +84,12 @@ export function setProxyCorsHeaders(reply) { // // DPoP is similarly not forwarded — it's bound to this pod's URL and // would be rejected by any upstream anyway. -// Note: Content-Length is *not* forwarded. We may transform the body -// (Fastify-parsed JSON gets re-stringified before the upstream fetch), -// so the caller's declared length may not match what we send. Node's +// Note: Content-Length is *not* forwarded. JSS registers a wildcard +// parseAs:'buffer' content parser (server.js), so request bodies arrive +// as a Buffer that we pass through to fetch unchanged — but we keep a +// defensive JSON.stringify branch downstream in case anything ever +// changes that and a parsed object lands here. Either way, the +// caller-declared length isn't reliable for the upstream call. Node's // fetch sets Content-Length automatically based on the actual body. // (Browsers send Content-Length on simple requests without needing it // in Access-Control-Allow-Headers, since it's CORS-safelisted.) From c6b80e8d53b1468043d43e91769080c869442811 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 18:08:49 +0200 Subject: [PATCH 12/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?1=20=E2=80=94=20surface=20paid-access=20headers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the standard WAC hook (server.js:564-569) for the /proxy auth path: when checkAccess() returns paid (the cost) plus balance/currency, set X-Cost / X-Balance / X-Pay-Currency on the response. Without this, a successful paid ACL access debits the ledger silently and the browser side can't render charge UI for /proxy traffic. Note: the path-based multi-pod gap raised in the same review pass (/proxy is a single global endpoint, not per-pod //proxy) is filed as #382 — too big a design change for Phase 1. --- src/server.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/server.js b/src/server.js index 113f2bad..b88f542a 100644 --- a/src/server.js +++ b/src/server.js @@ -475,10 +475,23 @@ export function createServer(options = {}) { // fallback would route POST authorization to / (the root) instead // of /proxy — too permissive. With this flag, authorize() checks // ACLs against /proxy directly regardless of storage existence. - const { authorized, webId, wacAllow, authError, paymentRequired } = await authorize(request, reply, { skipParentForMissing: true }); + const { authorized, webId, wacAllow, authError, paymentRequired, paid, balance, currency } = + await authorize(request, reply, { skipParentForMissing: true }); request.webId = webId; request.wacAllow = wacAllow; + // Surface paid-access bookkeeping the same way the standard WAC + // hook does (lines 564-569 below). When a /proxy ACL uses a + // PaymentCondition and the caller has sufficient balance, + // checkAccess() returns paid (the cost), balance, and currency — + // browser-side renders charge UI off these. Without this, ledger + // debit happens silently. + if (paid !== undefined) { + reply.header('X-Cost', String(paid)); + reply.header('X-Balance', String(balance)); + if (currency) reply.header('X-Pay-Currency', currency); + } + // ACL with a PaymentCondition surfaces as 402 here — mirrors the // git handler at src/server.js:418 and the standard WAC hook so // payment-gated /proxy ACLs behave consistently. From 00a7e3d0d505755c0390811db76e5dfb7a5a1dbb Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 21:14:30 +0200 Subject: [PATCH 13/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?2=20=E2=80=94=20short-circuit=20OPTIONS,=20JSDoc=20paid=20type?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: 1. OPTIONS preflight now bypasses authorize() entirely and goes straight to the handler (which already returns 204 + proxy CORS headers). authorize() does have its own OPTIONS short-circuit, but routing the preflight through there at all is unnecessary risk: future changes to authorize()/checkAccess() could end up debiting a ledger or evaluating a PaymentCondition on a preflight, which is never what we want. Defense-in-depth. 2. authorize() JSDoc had `paid?: boolean` but checkAccess() returns the cost as a number (src/wac/checker.js:189), and callers stringify it for the X-Cost header. Updated to `paid?: number` with a note explaining the runtime contract. Verified: OPTIONS /proxy on non-public pod → 204 with full proxy CORS (no auth) GET /proxy on non-public pod → 401 (unchanged) --- src/auth/middleware.js | 5 ++++- src/server.js | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/auth/middleware.js b/src/auth/middleware.js index 2f40d927..71e4e462 100644 --- a/src/auth/middleware.js +++ b/src/auth/middleware.js @@ -63,10 +63,13 @@ export function buildResourceUrl(request, urlPath) { * wacAllow: string, * authError: string|null, * paymentRequired?: object, - * paid?: boolean, + * paid?: number, * balance?: number, * currency?: string * }>} + * `paid` is the cost actually debited (number, not boolean) — see + * checkAccess() in src/wac/checker.js:189; callers stringify it for + * the X-Cost response header. */ export async function authorize(request, reply, options = {}) { const urlPath = request.url.split('?')[0]; diff --git a/src/server.js b/src/server.js index b88f542a..4773279f 100644 --- a/src/server.js +++ b/src/server.js @@ -463,6 +463,20 @@ export function createServer(options = {}) { return; } + // OPTIONS preflight short-circuits to the handler (which returns + // 204 + proxy CORS headers) without going through authorize() at + // all. authorize() does have its own OPTIONS short-circuit, but + // routing through here keeps the preflight off the auth/payment + // path entirely — preflights must never debit ledgers or evaluate + // PaymentConditions. + if (request.method === 'OPTIONS') { + return handleCorsProxy(request, reply, { + maxBytes: corsProxyMaxBytes, + timeoutMs: corsProxyTimeoutMs, + maxRedirects: corsProxyMaxRedirects, + }); + } + // Don't override requiredMode — let authorize() derive it from the // request method via getRequiredMode(). GET/HEAD need READ on the // /proxy resource, POST needs APPEND/WRITE — pod owners can grant From 451509fd53b1a6221aafaed3999797b125a0e482 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 21:30:46 +0200 Subject: [PATCH 14/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?3=20=E2=80=94=20expose=20payment=20headers,=20WAC-Allow=20on=20?= =?UTF-8?q?success?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two consistency fixes to match the global WAC hook's response-header contract: 1. Add X-Cost / X-Balance / X-Pay-Currency to PROXY_CORS_HEADERS Access-Control-Expose-Headers. Pass 11 wired the proxy preHandler to *set* these headers, but without exposing them browser-side readers can't access them cross-origin. Aligns with the global getCorsHeaders() in src/ldp/headers.js. 2. Set WAC-Allow on the success path. Previously only the 401/403/402 branches did, so successful /proxy responses lost the header. The global WAC hook sets it for every response — the proxy hook now matches. Verified: GET /proxy?url=… on a public pod returns 200 with both \`wac-allow\` and \`access-control-expose-headers: …, X-Cost, X-Balance, X-Pay-Currency\`. --- src/handlers/cors-proxy.js | 2 +- src/server.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 46edf444..560c376d 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -61,7 +61,7 @@ export const PROXY_CORS_HEADERS = { 'Range', 'User-Agent', ].join(', '), - 'Access-Control-Expose-Headers': 'Content-Type, ETag, Last-Modified, Link, Location, WWW-Authenticate, WAC-Allow', + 'Access-Control-Expose-Headers': 'Content-Type, ETag, Last-Modified, Link, Location, WWW-Authenticate, WAC-Allow, X-Cost, X-Balance, X-Pay-Currency', }; export function setProxyCorsHeaders(reply) { diff --git a/src/server.js b/src/server.js index 4773279f..de6ab9c8 100644 --- a/src/server.js +++ b/src/server.js @@ -506,6 +506,12 @@ export function createServer(options = {}) { if (currency) reply.header('X-Pay-Currency', currency); } + // Set WAC-Allow on success too, matching the global WAC hook + // (line 562 area). Browser clients read it via Expose-Headers + // to render auth UX. Without this, only 401/403/402 responses + // carry WAC-Allow, which is inconsistent. + reply.header('WAC-Allow', wacAllow); + // ACL with a PaymentCondition surfaces as 402 here — mirrors the // git handler at src/server.js:418 and the standard WAC hook so // payment-gated /proxy ACLs behave consistently. From 361e56325a704c1411a8e0412374da267f615c0d Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 22:08:28 +0200 Subject: [PATCH 15/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?4=20=E2=80=94=20strip=20pod-authoritative=20headers,=20CSP=20sa?= =?UTF-8?q?ndbox?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real security fixes: 1. Pod-authoritative response headers (WAC-Allow, X-Cost, X-Balance, X-Pay-Currency) added to STRIP_RESPONSE_HEADERS. copyResponseHeaders would otherwise blindly forward an upstream's spoofed values and override our local ones. WAC-Allow describes *this* pod's ACL decision; the X-* payment headers describe a debit on this pod's ledger — neither is the upstream's to set. 2. Content-Security-Policy: sandbox + X-Content-Type-Options: nosniff added to setProxyCorsHeaders. Without these, navigating a browser to `/proxy?url=https://evil/page.html` would render attacker HTML in this pod's origin, with full access to other pod resources (cookies, localStorage, fetch). CSP sandbox neutralizes scripts/ plugins/navigation when the response is rendered as a document; nosniff blocks MIME-confusion tricks. fetch()-based callers are unaffected — they consume the bytes regardless. Verified: - 200 + 400 responses both carry `content-security-policy: sandbox` and `x-content-type-options: nosniff` - Upstream sending `WAC-Allow: evil` and `X-Cost: 999` via httpbin /response-headers — JSS strips them and our `wac-allow: public=…` survives, no x-cost in the response. --- src/handlers/cors-proxy.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 560c376d..87841a46 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -68,6 +68,16 @@ export function setProxyCorsHeaders(reply) { for (const [k, v] of Object.entries(PROXY_CORS_HEADERS)) { reply.header(k, v); } + // Hardening against the proxy being used as an XSS vector: if a user + // navigates a browser to /proxy?url=https://evil/page.html, the + // upstream HTML would otherwise execute in this pod's origin and + // could exfiltrate other pod resources. CSP `sandbox` neutralizes + // scripts/plugins/navigation when rendered as a document; nosniff + // prevents MIME-sniffing tricks. fetch()-based callers are + // unaffected (they consume the raw bytes regardless of these + // response-side defenses). + reply.header('Content-Security-Policy', 'sandbox'); + reply.header('X-Content-Type-Options', 'nosniff'); } // Headers we forward from the caller to the upstream. Anything not on @@ -119,6 +129,12 @@ const UPSTREAM_AUTH_HEADER = 'x-upstream-authorization'; // exceeded. Forwarding the upstream Content-Length in either case // causes ERR_CONTENT_LENGTH_MISMATCH or hangs in the client. Node sends // the actual length (or chunked) automatically. +// +// WAC-Allow / X-Cost / X-Balance / X-Pay-Currency are stripped because +// they describe *this* pod's ACL decision; if the upstream sets them +// they'd override our local values and let the upstream spoof auth or +// payment state. server.js re-applies our values after the upstream +// headers are copied in. const STRIP_RESPONSE_HEADERS = new Set([ 'content-encoding', 'content-length', @@ -128,6 +144,11 @@ const STRIP_RESPONSE_HEADERS = new Set([ // Strip set-cookie — we never want to forward upstream cookies into // our origin's domain (would let upstream set cookies on the pod). 'set-cookie', + // Pod-authoritative headers: never accept upstream values for these. + 'wac-allow', + 'x-cost', + 'x-balance', + 'x-pay-currency', ]); const DEFAULT_MAX_REDIRECTS = 5; From 8d7f883818eeebfd0ec32a4f9fda6dc0a32d3a4b Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 22:37:17 +0200 Subject: [PATCH 16/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?5=20=E2=80=94=20strip=20Content-Range/Accept-Ranges=20from=20up?= =?UTF-8?q?stream?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the byte-cap Transform truncates an upstream 206 mid-stream, the forwarded Content-Range header still reflects the upstream's full range and no longer matches the bytes the client receives. Caller trusts the header → mis-handles the truncated body. Stripping both Content-Range and Accept-Ranges on the response side (via STRIP_RESPONSE_HEADERS) means the caller can't trust a stale range and just consumes whatever bytes actually arrived. Range still helps in the request direction — upstream sends fewer bytes when the caller sets it. Byte-accurate range proxying when the cap is smaller than the requested range is a Phase 2 concern. --- src/handlers/cors-proxy.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 87841a46..6667acdb 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -149,6 +149,15 @@ const STRIP_RESPONSE_HEADERS = new Set([ 'x-cost', 'x-balance', 'x-pay-currency', + // Range/206 mismatch: we may truncate the body at maxBytes, in which + // case the upstream's Content-Range no longer matches the bytes the + // client gets. Stripping both Content-Range and Accept-Ranges means + // callers don't trust a stale range — they receive whatever bytes + // actually streamed and can re-request with a smaller window if + // needed. Phase 2 could revisit if someone needs byte-accurate range + // proxying. + 'content-range', + 'accept-ranges', ]); const DEFAULT_MAX_REDIRECTS = 5; From 8395d6000b1122834861fb88415b0614eae9261e Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 23:10:59 +0200 Subject: [PATCH 17/18] =?UTF-8?q?cors-proxy:=20Copilot=20review=20pass=201?= =?UTF-8?q?6=20=E2=80=94=20preserve=20HEAD=20across=20redirects,=20fix=20d?= =?UTF-8?q?otfile=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real fixes: 1. Dotfile guard (server.js:402) was splitting the full URL on '/' before stripping the query string, so for /proxy?url=https://example .com/.git/config the segments included '.git' and the request was rejected with 403 — even though the dotfile is in the *upstream* URL, not the pod's path. The guard is about this pod's filesystem, not arbitrary URL contents. Fixed by splitting on '?' first. 2. Redirect handler in cors-proxy.js was unconditionally turning 301/302/303 responses into GET on the next hop, including for HEAD requests. That'd stream a body back on what was originally a body-less HEAD. Per fetch/RFC 7231 semantics, only POST (and other non-GET/non-HEAD) should be downgraded to GET on these redirects; HEAD must stay HEAD. Verified: - /proxy?url=…example.com/.git/config → 404 (upstream's response), no longer 403 from our guard - HEAD /proxy?url=…/redirect-to?...status_code=302 → 200 with empty body, method preserved across the redirect - GET redirect still works → 200 - Direct /.git/config → 403 (pod-internal dotfile guard intact) --- src/handlers/cors-proxy.js | 24 +++++++++++++++--------- src/server.js | 8 +++++++- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index 6667acdb..b52c3001 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -364,15 +364,21 @@ export async function handleCorsProxy(request, reply, options = {}) { return reply.code(502).send({ error: 'Upstream redirect with malformed Location', location }); } if ([301, 302, 303].includes(upstream.status)) { - // Method/body change is mandated by RFC 7231: redirected request - // becomes GET with no body. Also drop content-length / content-type - // so we don't send headers that no longer match the body. - currentMethod = 'GET'; - body = null; - delete forwardHeaders['content-length']; - delete forwardHeaders['Content-Length']; - delete forwardHeaders['content-type']; - delete forwardHeaders['Content-Type']; + // Per fetch / RFC 7231 redirect semantics: + // - 303: any non-GET/HEAD becomes GET with no body + // - 301/302: POST → GET with no body (legacy quirk; spec + // technically says preserve method, but every real client + // downgrades POST to GET); HEAD stays HEAD; GET stays GET + // HEAD must never be turned into GET — it'd cause an + // unexpected body to stream on what was a body-less request. + if (currentMethod !== 'GET' && currentMethod !== 'HEAD') { + currentMethod = 'GET'; + body = null; + delete forwardHeaders['content-length']; + delete forwardHeaders['Content-Length']; + delete forwardHeaders['content-type']; + delete forwardHeaders['Content-Type']; + } } // Cross-origin redirect: strip Authorization to prevent leaking // the X-Upstream-Authorization-derived credential to a different diff --git a/src/server.js b/src/server.js index de6ab9c8..93a3b5b2 100644 --- a/src/server.js +++ b/src/server.js @@ -399,7 +399,13 @@ export function createServer(options = {}) { return; } - const segments = request.url.split('/').map(s => s.split('?')[0]); // Remove query strings + // Only inspect the path component — splitting the full URL on '/' + // would catch dot-prefixed segments inside query-string values + // (e.g. /proxy?url=https://example.com/.git/config), rejecting + // legitimate proxy requests for upstream URLs that happen to + // contain dotfile-like path segments. The dotfile guard is about + // *this* pod's filesystem, not what the URL looks like. + const segments = request.url.split('?')[0].split('/'); const hasForbiddenDotfile = segments.some(seg => seg.startsWith('.') && seg.length > 1 && From 6ec53f8f2a8a76a66600dcde7b3eb33f53067b32 Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Fri, 8 May 2026 23:16:44 +0200 Subject: [PATCH 18/18] =?UTF-8?q?cors-proxy:=20review=20pass=2017=20?= =?UTF-8?q?=E2=80=94=20fix=20misleading=20STRIP=5FRESPONSE=5FHEADERS=20com?= =?UTF-8?q?ment=20about=20WAC-Allow=20ordering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment said server.js "re-applies" the local values after upstream headers are copied. Actual flow: server.js sets WAC-Allow / X-Cost / X-Balance / X-Pay-Currency in the proxy preHandler *before* handleCorsProxy runs, and stripping them on the upstream side ensures copyResponseHeaders can't overwrite. Updated wording to describe the actual order so future maintainers (human or LLM) get accurate provenance. --- src/handlers/cors-proxy.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/handlers/cors-proxy.js b/src/handlers/cors-proxy.js index b52c3001..676f1e61 100644 --- a/src/handlers/cors-proxy.js +++ b/src/handlers/cors-proxy.js @@ -131,10 +131,11 @@ const UPSTREAM_AUTH_HEADER = 'x-upstream-authorization'; // the actual length (or chunked) automatically. // // WAC-Allow / X-Cost / X-Balance / X-Pay-Currency are stripped because -// they describe *this* pod's ACL decision; if the upstream sets them -// they'd override our local values and let the upstream spoof auth or -// payment state. server.js re-applies our values after the upstream -// headers are copied in. +// they describe *this* pod's ACL decision and ledger debit; an upstream +// must not be able to spoof them. server.js sets these headers *before* +// calling handleCorsProxy (in the proxy preHandler, after authorize()), +// and stripping them here ensures copyResponseHeaders can never +// overwrite the local values when iterating upstream headers. const STRIP_RESPONSE_HEADERS = new Set([ 'content-encoding', 'content-length',