Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
melvincarvalho merged 2 commits into from
May 6, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
097875e
url: strip all leading slashes — fixes 500 on //foo bot probes (#131)
melvincarvalho May 5, 2026
752269d
test: save/restore DATA_ROOT around urlToPath block to prevent test-o…
melvincarvalho May 6, 2026
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
11 changes: 7 additions & 4 deletions src/utils/url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ export function updateDataRoot() {
* @throws {Error} - If path traversal is detected
*/
export function urlToPath(urlPath) {
// Normalize: remove leading slash, decode URI
let normalized = urlPath.startsWith('/') ? urlPath.slice(1) : urlPath;
// Normalize: strip all leading slashes (#131 — `//foo` from bot probes
// would otherwise leave `/foo`, and path.resolve(root, '/foo') would
// treat the second arg as absolute, escape dataRoot, and trip the
// traversal guard with a 500 instead of resolving cleanly to a 404).
let normalized = urlPath.replace(/^\/+/, '');
normalized = decodeURIComponent(normalized);

// Security: remove path traversal attempts (multiple passes for ....// bypass)
Expand Down Expand Up @@ -54,8 +57,8 @@ export function urlToPath(urlPath) {
* @throws {Error} - If path traversal is detected
*/
export function urlToPathWithPod(urlPath, podName) {
// Normalize: remove leading slash, decode URI
let normalized = urlPath.startsWith('/') ? urlPath.slice(1) : urlPath;
// Normalize: strip all leading slashes (#131 — see urlToPath for context).
let normalized = urlPath.replace(/^\/+/, '');
normalized = decodeURIComponent(normalized);

// Security: remove path traversal attempts (multiple passes for ....// bypass)
Expand Down
58 changes: 56 additions & 2 deletions test/url.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
* Regression guard for #278 (single-user root-pod PUT → ENOTDIR).
*/

import { describe, it } from 'node:test';
import { describe, it, before, after } from 'node:test';
import assert from 'node:assert';
import { getPodName, getContentType } from '../src/utils/url.js';
import path from 'path';
import { getPodName, getContentType, urlToPath, urlToPathWithPod } from '../src/utils/url.js';

describe('getPodName', () => {
describe('subdomain mode', () => {
Expand Down Expand Up @@ -126,3 +127,56 @@ describe('getContentType', () => {
});
});
});

describe('urlToPath / urlToPathWithPod (#131 — leading-slash normalization)', () => {
// Bot probes hammer JSS with `//foo`, `///wp-admin/...`, etc. Without
// 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.

// Save/restore DATA_ROOT — other test suites mutate process.env.DATA_ROOT
// (via createServer's root option) and don't always restore it. Pinning
// to './data' keeps the assertions stable across run order.
let originalDataRoot;
before(() => {
originalDataRoot = process.env.DATA_ROOT;
delete process.env.DATA_ROOT; // forces getDataRoot() default of './data'
});
after(() => {
if (originalDataRoot === undefined) delete process.env.DATA_ROOT;
else process.env.DATA_ROOT = originalDataRoot;
});

const dataRoot = path.resolve('./data');

@melvincarvalho melvincarvalho May 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


describe('urlToPath', () => {
it('resolves a normal path inside dataRoot', () => {
assert.strictEqual(urlToPath('/alice/profile/card'), path.join(dataRoot, 'alice/profile/card'));
});

it('handles double leading slash without throwing (#131)', () => {
assert.strictEqual(urlToPath('//about.php'), path.join(dataRoot, 'about.php'));
});

it('handles many leading slashes (#131)', () => {
assert.strictEqual(urlToPath('////wp-admin/index.php'), path.join(dataRoot, 'wp-admin/index.php'));
});

it('still rejects real `..` traversal that escapes after normalization', () => {
// Security must be preserved: `/../etc/passwd` → strip leading slash →
// `../etc/passwd` → strip `..` → `/etc/passwd` (absolute residue) →
// path.resolve escapes dataRoot → guard fires.
assert.throws(() => urlToPath('/../etc/passwd'), /Path traversal/);
});
});

describe('urlToPathWithPod', () => {
it('resolves into the pod dir', () => {
assert.strictEqual(urlToPathWithPod('/profile/card', 'alice'), path.join(dataRoot, 'alice/profile/card'));
});

it('handles double leading slash (#131)', () => {
assert.strictEqual(urlToPathWithPod('//about.php', 'alice'), path.join(dataRoot, 'alice/about.php'));
});
});
});