From 61306b292aa31eee7a598de026d085f4c8d6d75d Mon Sep 17 00:00:00 2001 From: Melvin Carvalho Date: Wed, 10 Jun 2026 18:55:44 +0200 Subject: [PATCH] fix(git): WAC preHandler 401/402/403 responses carry the git CORS headers (#548) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 401/403 for an unauthorized git operation is emitted by the WAC preHandler in src/server.js — before handleGit runs — and that path set WAC-Allow / WWW-Authenticate but not the git CORS headers. A browser-based git client (e.g. jss.live/git/) hitting an auth-gated repo therefore saw a generic CORS/network error instead of the 401 auth challenge — the exact failure mode #371 fixed for handleGit's own paths. Fix: export setGitCorsHeaders from src/handlers/git.js (it was module-private) and apply it on the preHandler's early returns — the 402 paymentRequired branch and the 401/403 unauthorized branch. The GIT_CORS_HEADERS doc comment is updated to name the new caller so it stays the single source of truth. The 402 branch gets the same one-line helper call as the tested 401/403 path; a dedicated 402 test would need a pay-gated git fixture, which is disproportionate to an identical mechanical call — the 401 test pins the helper's output. Test: the deliberately-scoped 401 test from #549 (which asserted WWW-Authenticate but NOT CORS, with a pointer comment at this issue) now asserts the full contract via assertGitCors. Test title, inline comment, and the file docstring all updated to match the new behaviour. Full suite: 934/934 passing. Closes #548. --- src/handlers/git.js | 10 ++++++---- src/server.js | 11 ++++++++++- test/git-handler.test.js | 17 +++++++++-------- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/handlers/git.js b/src/handlers/git.js index 3573bcae..03fbb468 100644 --- a/src/handlers/git.js +++ b/src/handlers/git.js @@ -167,16 +167,18 @@ function tryAutoInitRepo(repoAbs, log) { // CORS headers for git responses. Single source of truth — used by the // success path (Fastify reply on the OPTIONS preflight, raw stream on -// http-backend output) and by every 4xx early-return. Without these, -// browser-based git clients (e.g. jss.live/git/) see a generic -// CORS/network error instead of the actual status, undermining #371. +// http-backend output), by every 4xx early-return in this module, and +// by the WAC preHandler's 401/402/403 returns in src/server.js (#548). +// Without these, browser-based git clients (e.g. jss.live/git/) see a +// generic CORS/network error instead of the actual status, +// undermining #371. const GIT_CORS_HEADERS = { 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'GET, POST, OPTIONS', 'Access-Control-Allow-Headers': 'Content-Type, Authorization, Git-Protocol', }; -function setGitCorsHeaders(reply) { +export function setGitCorsHeaders(reply) { for (const [k, v] of Object.entries(GIT_CORS_HEADERS)) { reply.header(k, v); } diff --git a/src/server.js b/src/server.js index 2a0e00b3..1044e979 100644 --- a/src/server.js +++ b/src/server.js @@ -17,7 +17,7 @@ import { idpPlugin } from './idp/index.js'; // below so non-IdP deployments don't pull in the IdP accounts module // (bcryptjs etc.) just to register Fastify routes. The same lazy-load // pattern is used in src/auth/nostr.js for the NIP-98 verifier. -import { isGitRequest, isGitWriteOperation, handleGit } from './handlers/git.js'; +import { isGitRequest, isGitWriteOperation, handleGit, setGitCorsHeaders } from './handlers/git.js'; import { handleCorsProxy, isCorsProxyRequest, setProxyCorsHeaders } from './handlers/cors-proxy.js'; import { AccessMode } from './wac/parser.js'; import { registerNostrRelay } from './nostr/relay.js'; @@ -533,11 +533,20 @@ export function createServer(options = {}) { request.wacAllow = wacAllow; if (paymentRequired) { + // Git CORS headers on the early return — same reasoning as the + // 401/403 below: a browser git client must see the 402, not a + // generic CORS/network error. See #548 / #371. + setGitCorsHeaders(reply); return reply.code(402).send({ type: 'PaymentRequired', ...paymentRequired }); } if (!authorized) { const message = needsWrite ? 'Write access required for push' : 'Read access required for clone'; + // Without the git CORS headers, browser-based git clients (e.g. + // jss.live/git/) hitting an auth-gated repo saw a generic CORS + // error instead of this 401/403 — the same failure mode #371 + // fixed inside handleGit. See #548. + setGitCorsHeaders(reply); reply.header('WAC-Allow', wacAllow); if (!webId) { // No authentication - request Basic auth for git clients diff --git a/test/git-handler.test.js b/test/git-handler.test.js index 4ecbd430..d5741879 100644 --- a/test/git-handler.test.js +++ b/test/git-handler.test.js @@ -13,10 +13,10 @@ * them a browser git client sees a CORS error, not the 404) * - path traversal attempts are blocked (never 200, never 500) * with CORS headers - * - unauthenticated push advertise → 401 + WWW-Authenticate - * (NOTE: this 401 is emitted by the server.js WAC preHandler, - * which does NOT currently set the git CORS headers — that gap is - * tracked separately; the test asserts what is true today) + * - unauthenticated push advertise → 401 + WWW-Authenticate + the + * git CORS headers (the preHandler's 401/403 path lacked them + * until #548; this 401 is emitted by the server.js WAC + * preHandler, not by handleGit) * - a real end-to-end `git push` succeeds and the pushed file is * served as a static resource (receive-pack POST through * http-backend + updateInstead extraction) @@ -179,7 +179,7 @@ describe('git handler HTTP contract (#375)', () => { assertGitCors(res, 'traversal block'); }); - it('unauthenticated push advertise returns 401 with WWW-Authenticate (non-public server)', async () => { + it('unauthenticated push advertise returns 401 with WWW-Authenticate and git CORS headers (non-public server, #548)', async () => { // Second server WITHOUT public:true so the WAC preHandler gates // the push. Snapshot/restore DATA_ROOT — createServer({ root }) // mutates it process-wide and the main suite's server reads it @@ -198,9 +198,10 @@ describe('git handler HTTP contract (#375)', () => { `unauthenticated push advertise must be 401, got ${res.status}`); assert.ok(res.headers.get('www-authenticate'), 'WWW-Authenticate must be present so git CLI clients prompt for credentials'); - // Deliberately NOT asserting CORS here: the WAC preHandler's - // 401 path does not currently set the git CORS headers (tracked - // as a follow-up). Assert truth, not aspiration. + // #548: the WAC preHandler's 401/403 path must carry the git + // CORS headers, or browser git clients see a generic CORS error + // instead of the auth challenge. + assertGitCors(res, 'unauthenticated push 401'); } finally { await authServer.close(); if (originalDataRoot === undefined) delete process.env.DATA_ROOT;