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;