url: strip all leading slashes — fixes 500 on //foo bot probes (#131) by melvincarvalho · Pull Request #360 · JavaScriptSolidServer/JavaScriptSolidServer · GitHub

Closes #131.

Problem

URLs with multiple leading slashes (typical bot probes — `//about.php`, `//wp-admin/...`, `//cgi-bin/...`) returned 500 Internal Server Error instead of 404. On production, this was 100% of 5xx traffic (~2,130/week on melvin.me), inflating monitoring noise without representing real errors.

Reproduce against any current JSS:

```
$ curl -s https://solid.social//about.php
{"statusCode":500,"error":"Internal Server Error","message":"Path traversal detected"}
```

Root cause

In `src/utils/url.js`:

```js
let normalized = urlPath.startsWith('/') ? urlPath.slice(1) : urlPath;
```

`slice(1)` only strips one leading slash. With input `//about.php`:

After slice → `/about.php`
After `..` strip → `/about.php` (unchanged)
`path.resolve(dataRoot, "/about.php")` → `/about.php` (absolute argument wins)
Resolved path doesn't start with `dataRoot` → throws `"Path traversal detected"`

The traversal guard fires not because traversal occurred, but because the absolute residue escaped `dataRoot` accidentally.

Fix

Replace `slice(1)` with `replace(/^\/+/, '')` in both `urlToPath` and `urlToPathWithPod`. Strips any number of leading slashes; `//about.php` → `about.php` → resolves to `dataRoot/about.php` → file doesn't exist → existing 404 path takes over.

Security

Real `..` traversal is still rejected. The fix only changes what happens to leading slashes — internal `..` segments still strip down to absolute residues that the guard catches. Tested explicitly:

```js
it('still rejects real `..` traversal that escapes after normalization', () => {
assert.throws(() => urlToPath('/../etc/passwd'), /Path traversal/);
});
```

Test plan

6 new unit tests in `test/url.test.js` covering:

Normal path resolves inside dataRoot (regression baseline)
`//about.php` resolves cleanly (Double-slash URLs return 500 instead of 404 #131)
`////wp-admin/index.php` (many slashes) resolves cleanly (Double-slash URLs return 500 instead of 404 #131)
Real `..` traversal still throws (security baseline)
Pod-aware variant (`urlToPathWithPod`) handles double slash too
All 26 url tests pass; no regression in conneg / data-island / container suites

Deploy / verify

After merge + 0.0.167 publish + solid.social upgrade:

```
$ curl -s -o /dev/null -w "%{http_code}\n" https://solid.social//about.php
404
```

`urlPath.slice(1)` only stripped one leading slash, so `//about.php` became `/about.php`, which `path.resolve(dataRoot, ...)` then treated as absolute, escaped dataRoot, and tripped the path-traversal guard with a 500 "Path traversal detected" instead of resolving cleanly to a 404. 100% of 5xx in production logs (~2,130/week on melvin.me) were these double-slash bot probes — `//about.php`, `//cgi-bin/...`, etc. Replacing slice(1) with replace(/^\\/+/, '') normalizes any number of leading slashes; the path then resolves to dataRoot/foo, the file doesn't exist, and the existing 404 path takes over. Security guard preserved: paths with internal '..' that escape after normalization (e.g. /../etc/passwd → ../etc/passwd → /etc/passwd absolute residue) still trip the traversal guard, tested explicitly.

Pull request overview

This PR fixes an edge case in URL-to-filesystem-path normalization where multiple leading slashes (e.g. //about.php) could cause path.resolve() to treat the URL path as absolute, triggering the path traversal guard and returning 500 instead of the expected 404.

Changes:

Strip all leading slashes in urlToPath() and urlToPathWithPod() using replace(/^\/+/, '') to prevent accidental absolute-path resolution.
Add unit tests covering double-/multi-slash inputs and confirming .. traversal is still rejected.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/utils/url.js	Adjusts URL path normalization to remove multiple leading slashes before path.resolve().
test/url.test.js	Adds regression tests for //... bot-probe paths and traversal guard behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Addressed in 752269d — save/restore process.env.DATA_ROOT in before/after for the new describe block. Verified with DATA_ROOT=/tmp/whatever node --test test/url.test.js — tests still pass under deliberately dirty env. Same pattern as test/idp-change-password.test.js (round 1 of PR #355).

…rder flakiness (PR #360 round 1) Caught by Copilot: other test suites mutate process.env.DATA_ROOT via createServer's root option without always restoring it. The new #131 block hardcoded path.resolve('./data') in its assertions, so a preceding suite leaving DATA_ROOT set would make these tests fail under different run orders. Same pattern as test/idp-change-password.test.js — save in before(), restore in after().

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

…ckend (#373) (#374) * git handler: collapse multi-slash in URL before forwarding to http-backend (#373) git http-backend rejects PATH_INFO with `//` as "aliased" and JSS was forwarding the urlPath unchanged, returning a 500 instead of routing to the repo. Same shape as #131 (LDP path); the fix in #360 only covered src/utils/url.js. The git handler builds urlPath independently and never got the same treatment. Verified with a local boot test: GET /repo.git/info/refs, /repo.git//info/refs, and ///repo.git//info/refs all return 200 with application/x-git-upload-pack-advertisement. Frontends (jss.live/git/) producing `${repo}/info/refs` from a trailing-slash repo URL, and bot probes hitting ///wp-admin/..., both now resolve cleanly. * git handler: also catch malformed percent-encoding (don't 500) Adjacent to the multi-slash fix: decodeURIComponent throws URIError on malformed inputs like `%g1`, truncated `%E0%`, or a lone `%` — common in bot traffic. Fastify surfaces the throw as 500. Wrap in try/catch and return 400. Verified locally: malformed `%g1`, `%E0%`, and lone `%` all now return HTTP 400 (was 500); good URLs (single + double slash) still return 200. Per Copilot review on #374. * git handler: drop dead URIError catch, add CORS to remaining 4xx paths Per Copilot follow-up review on #374: - Drops the try/catch around decodeURIComponent. Empirically verified that current Fastify rejects every malformed percent-encoding (`%g1`, truncated `%E0%`, `%C3%28`, `%FF%FE`, `%80`) with 400 FST_ERR_BAD_URL at the URL parser, before this handler runs. The catch was dead code. Comment now records that finding. - Extracts setGitCorsHeaders() helper and applies it to the reachable 4xx returns (extractRepoPath 400, traversal 403, missing-repo 404). Without these headers, browser git clients (e.g. jss.live/git/) see a generic CORS/network error instead of the actual status — undermining the CORS work in #371. Note: Fastify's own 400 (FST_ERR_BAD_URL) still doesn't carry CORS headers, since it short-circuits before our handler. That's a broader issue across all routes, separate from #373. * git handler: GIT_CORS_HEADERS const, drop dead null-check Per Copilot pass on #374: - Centralizes CORS header values in GIT_CORS_HEADERS const. Both the setGitCorsHeaders() Fastify-reply helper and the streaming success path's reply.raw.setHeader loop iterate over the same map. Future updates (allowed methods/headers) live in one place — drift risk gone. - Drops the unreachable `if (!repoRelative)` check. extractRepoPath always returns a non-empty string ('.' for root); JSDoc updated to match the actual contract. Regression set still passes: 200 single, 200 double-slash, 404 missing repo with CORS, OPTIONS preflight with CORS.

melvincarvalho requested a review from Copilot May 5, 2026 23:25

Copilot started reviewing on behalf of melvincarvalho May 5, 2026 23:26 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

melvincarvalho requested a review from Copilot May 6, 2026 07:05

Copilot started reviewing on behalf of melvincarvalho May 6, 2026 07:06 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

melvincarvalho merged commit 469f21c into gh-pages May 6, 2026
4 checks passed

melvincarvalho deleted the issue-131-double-slash-404 branch May 6, 2026 08:22

This was referenced May 8, 2026

git handler: 500 on double-slash in URL (PATH_INFO not normalized) #373

Closed

git handler: collapse multi-slash in URL before forwarding to http-backend (#373) #374

Merged

+                // multi-slash stripping these used to escape dataRoot via path.resolve
+                // (which treats `/foo` as absolute) and 500 with "Path traversal detected"
+                // instead of the expected 404.
+                const dataRoot = path.resolve('./data');

url: strip all leading slashes — fixes 500 on //foo bot probes (#131) by melvincarvalho · Pull Request #360 · JavaScriptSolidServer/JavaScriptSolidServer · GitHub

Uh oh!

url: strip all leading slashes — fixes 500 on //foo bot probes (#131) - #360

url: strip all leading slashes — fixes 500 on //foo bot probes (#131)#360
melvincarvalho merged 2 commits into
gh-pagesfrom
issue-131-double-slash-404

melvincarvalho commented May 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

melvincarvalho May 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

url: strip all leading slashes — fixes 500 on //foo bot probes (#131) by melvincarvalho · Pull Request #360 · JavaScriptSolidServer/JavaScriptSolidServer · GitHub

Uh oh!

url: strip all leading slashes — fixes 500 on //foo bot probes (#131) - #360

Conversation

melvincarvalho commented May 5, 2026

Problem

Root cause

Fix

Security

Test plan

Deploy / verify

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

melvincarvalho May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants