FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(git): WAC preHandler 401/402/403 responses carry the git CORS headers — closes #548 by melvincarvalho · Pull Request #550 · JavaScriptSolidServer/JavaScriptSolidServer · GitHub

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

Filter by extension

Filter by extension .js  (3) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
10 changes: 6 additions & 4 deletions src/handlers/git.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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

Back | FazBrowse Home | New Git URL