Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/handlers/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
11 changes: 10 additions & 1 deletion src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions test/git-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down