test: git handler HTTP contract coverage + DATA_ROOT pollution fix — closes #375#549
Merged
Merged
Conversation
Comment on lines
+68
to
+77
| function gitAsync(args, cwd) { | ||
| return new Promise((resolve) => { | ||
| const child = spawn('git', args, { cwd, stdio: ['ignore', 'pipe', 'pipe'] }); | ||
| let stdout = ''; | ||
| let stderr = ''; | ||
| child.stdout.on('data', (d) => { stdout += d; }); | ||
| child.stderr.on('data', (d) => { stderr += d; }); | ||
| child.on('close', (status) => resolve({ status, stdout, stderr })); | ||
| }); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Done in 26719b9. gitAsync now (a) resolves with status: -1 + a 'spawn failed' stderr on the child error event, so a missing git binary fails the assertion instead of hanging to the runner timeout, and (b) sets GIT_TERMINAL_PROMPT=0 so an unexpected credential challenge fails fast rather than wedging on a prompt (stdin is 'ignore'). 8/8 still passing.
Comment on lines
+5
to
6
| A minimal, fast, JSON-LD native Solid server for the agentic web. | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Good catch — that was a branching mistake on my side: this branch was cut from the #547 docs branch instead of gh-pages, so the README/package.json commit leaked into this PR's diff. Fixed by merging #547 first (it was that content's real home) and rebasing this branch onto the updated gh-pages — the docs commit dropped as empty. The PR is now genuinely test-only: the diff is exactly test/git-handler.test.js (+) and test/git-auto-init.test.js (+10), matching the description.
… auto-init suite (#375) src/handlers/git.js had zero automated coverage — #371 (CORS) and #373 (multi-slash) were verified by hand. New test/git-handler.test.js pins the HTTP contract with 8 cases: - OPTIONS preflight → 200 + canonical git CORS headers (#371) - single/double/triple-slash info/refs advertise → 200 (#373) - missing repo → 404 WITH CORS headers (#374) - path traversal (percent-encoded so fetch doesn't normalize it away) → blocked 403/404, never 200/500, with CORS - unauthenticated push advertise → 401 + WWW-Authenticate on a non-public server. Deliberately does NOT assert CORS: the WAC preHandler's 401 path doesn't set the git CORS headers today — that gap is now tracked as #548; the test asserts what is true. - end-to-end push: real `git push HEAD:main` via async spawn → exit 0 → pushed file served as a static resource (receive-pack POST through http-backend + updateInstead extraction). The push MUST use async spawn — spawnSync blocks the event loop and the in-process server can never respond (deadlock). Also fixes a latent cross-test pollution bug in test/git-auto-init.test.js: the unauthenticated-ACL test boots a second server with root './test-data-git-auto-init-authcheck', and createServer({ root }) mutates process.env.DATA_ROOT globally without restoring it. Every test after it silently ran against the WRONG data root — handleGit auto-inited public/apps/penny inside the authcheck dir, the suite cleanup missed it, and the orphaned directory kept reappearing after every full test run. Snapshot/restore the env in the test's finally block (same pollution class as the #543 review finding). The authcheck dir no longer survives a run. Full suite: 934/934 passing. Closes #375.
melvincarvalho
force-pushed
the
issue-375-git-handler-tests
branch
from
June 10, 2026 16:32
bfe4cb9 to
26719b9
Compare
melvincarvalho
added a commit
that referenced
this pull request
Jun 10, 2026
…ders (#548) (#550) 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.
melvincarvalho
added a commit
that referenced
this pull request
Jun 11, 2026
Eight PRs merged since 0.0.206 — IdP hardening, protocol conformance, and the de-Googled-phone (#46) arc: IdP / auth - #558 passkey login degrades cleanly on stale WebViews / insecure contexts instead of crashing — drops a redundant browser crypto.randomUUID and guards both ceremonies on secure-context + WebAuthn (closes #556) - #554 IdP login-form error now survives the POST→redirect→GET cycle (rode in a non-persisted field that Interaction.save() dropped); failed sign-ins show the error instead of a silent re-render (closes #514) - #551 RFC 9207 'iss' authorization-response param normalized to match the discovery issuer, so strict OIDC clients (solid-oidc) complete sign-in (closes #524) Tunnel - #555 opt-in, per-tunnel credential passthrough (Cookie / Authorization / Set-Cookie) so authenticated access works through a tunnel; the relay's own IdP session cookies are isolated from the tunnel client (closes #530) Content negotiation / git - #553 HEAD now mirrors GET's negotiated Content-Type / Content-Length / Cache-Control for files (RFC 9110 §9.3.2 parity) (closes #552) - #550 git WAC preHandler 401/402/403 responses carry the git CORS headers, so browser git clients see the status, not a CORS error (closes #548) - #549 first HTTP-contract coverage for the git handler + fixes a DATA_ROOT test-pollution bug (closes #375) Docs / metadata - #547 README tagline + npm keywords surface the agentic positioning (closes #406)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #375. Test-only — zero production code touched.
New:
test/git-handler.test.js(8 cases)Pins the HTTP-level contract of
src/handlers/git.js, which previously had no automated coverage (#371 and #373 were hand-verified with curl):Git-Protocol)info/refsadvertise → 200 +application/x-git-upload-pack-advertisement//info/refs→ 200///info/refs→ 200fetchcan't normalize it client-side) → 403/404, never 200/500, with CORSWWW-Authenticate(non-public server)git push HEAD:main→ exit 0 → pushed file served as a static resourceupdateInsteadextraction — previously never exercised by any testTwo deliberate scoping notes, both documented in code comments:
Bonus: root-caused and fixed the recurring
test-data-git-auto-init-authcheck/orphanThe unauthenticated-ACL test in
test/git-auto-init.test.jsboots a second server withroot: './test-data-git-auto-init-authcheck'.createServer({ root })mutatesprocess.env.DATA_ROOTglobally and the test never restored it — so every test after it in the file silently ran against the wrong data root:handleGitre-auto-initedpublic/apps/pennyinside the authcheck dir, suite cleanup only removed the mainDATA_DIR, and the orphaned directory reappeared after every full test run (it's been manually deleted from the worktree at least twice).Fix: snapshot/restore
DATA_ROOTin the test'sfinally(same pollution class as the #543 review finding). Verified: the authcheck dir no longer survives a fullnpm test.Verification
git-auto-init.test.js: 7/7 passing, now against the correct data roottest-data-git-auto-init-authcheck/left behindRefs
git-protocolheader in CORS preflight responses #371, git handler: 500 on double-slash in URL (PATH_INFO not normalized) #373, git handler: collapse multi-slash in URL before forwarding to http-backend (#373) #374 — the fixes this coverage pins