fix(static): harden srvx/static #233
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,7 @@ import type { ServerMiddleware } from "./types.ts"; | |
| import type { Transform } from "node:stream"; | ||
|
|
||
| import { extname, join, resolve, sep } from "node:path"; | ||
| import { readFile, stat, realpath } from "node:fs/promises"; | ||
| import { createReadStream, ReadStream } from "node:fs"; | ||
| import { FastResponse } from "srvx"; | ||
| import { createGzip, createBrotliCompress } from "node:zlib"; | ||
|
|
@@ -19,6 +19,15 @@ export interface ServeStaticOptions { | |
| */ | ||
| methods?: string[]; | ||
|
|
||
| /** | ||
| * Value for the `Cache-Control` response header. | ||
| * | ||
| * Defaults to a conservative `"public, max-age=0, must-revalidate"` which | ||
| * lets clients cache but forces revalidation (via `ETag`/`Last-Modified`) | ||
| * on every request. Set to `false` to omit the header entirely. | ||
| */ | ||
| cacheControl?: string | false; | ||
|
|
||
| /** | ||
| * A function to modify the HTML content before serving it. | ||
| */ | ||
|
|
@@ -39,73 +48,266 @@ const COMMON_MIME_TYPES: Record<string, string> = { | |
| ".json": "application/json", | ||
| ".txt": "text/plain", | ||
| ".xml": "application/xml", | ||
| ".wasm": "application/wasm", | ||
| ".gif": "image/gif", | ||
| ".ico": "image/vnd.microsoft.icon", | ||
| ".jpeg": "image/jpeg", | ||
| ".jpg": "image/jpeg", | ||
| ".png": "image/png", | ||
| ".svg": "image/svg+xml", | ||
| ".webp": "image/webp", | ||
| ".avif": "image/avif", | ||
| ".woff": "font/woff", | ||
| ".woff2": "font/woff2", | ||
| ".mp3": "audio/mpeg", | ||
| ".mp4": "video/mp4", | ||
| ".webm": "video/webm", | ||
| ".zip": "application/zip", | ||
| ".gz": "application/gzip", | ||
| ".br": "application/x-brotli", | ||
| ".pdf": "application/pdf", | ||
| }; | ||
|
|
||
| /** | ||
| * Whether a MIME type benefits from compression. Already-compressed binary | ||
| * formats (images, video, audio, archives, fonts) are excluded so we never | ||
| * waste CPU re-encoding them. | ||
| */ | ||
| function isCompressible(mimeType: string): boolean { | ||
| const type = mimeType.split(";", 1)[0].trim(); | ||
| return ( | ||
| type.startsWith("text/") || | ||
| type === "application/json" || | ||
| type === "application/xml" || | ||
| type === "application/javascript" || | ||
| type === "application/wasm" || | ||
| type === "image/svg+xml" || | ||
| type.endsWith("+json") || | ||
| type.endsWith("+xml") | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Parse an `Accept-Encoding` header into a `token -> q-value` map, honoring | ||
| * q-values (so `br;q=0` disables brotli) and only matching exact tokens (so a | ||
| * value like `abbr` never matches `br`). | ||
| */ | ||
| function parseAcceptEncoding(header: string): Map<string, number> { | ||
| const map = new Map<string, number>(); | ||
| for (const part of header.split(",")) { | ||
| const [token, ...params] = part.trim().split(";"); | ||
| const name = token.trim().toLowerCase(); | ||
| if (!name) { | ||
| continue; | ||
| } | ||
| let q = 1; | ||
| for (const param of params) { | ||
| const match = /^q=(\d+(?:\.\d+)?)$/.exec(param.trim()); | ||
| if (match) { | ||
| q = Number.parseFloat(match[1]); | ||
| } | ||
| } | ||
| map.set(name, q); | ||
| } | ||
| return map; | ||
| } | ||
|
|
||
| /** | ||
| * Negotiate a content encoding from an `Accept-Encoding` header, preferring | ||
| * brotli, then gzip, and falling back to identity (`undefined`) when neither is | ||
| * acceptable (q=0 / absent). | ||
| */ | ||
| function negotiateEncoding(header: string): "br" | "gzip" | undefined { | ||
| if (!header) { | ||
| return undefined; | ||
| } | ||
| const map = parseAcceptEncoding(header); | ||
| const star = map.get("*"); | ||
| const qOf = (name: string): number => { | ||
| const direct = map.get(name); | ||
| if (direct !== undefined) { | ||
| return direct; | ||
| } | ||
| return star ?? 0; | ||
| }; | ||
| const brQ = qOf("br"); | ||
| const gzipQ = qOf("gzip"); | ||
| if (brQ > 0 && brQ >= gzipQ) { | ||
| return "br"; | ||
| } | ||
| if (gzipQ > 0) { | ||
| return "gzip"; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** Weak `ETag` comparison (RFC 9110): ignore any leading `W/`. */ | ||
| function etagMatches(ifNoneMatch: string, etag: string): boolean { | ||
| if (ifNoneMatch.trim() === "*") { | ||
| return true; | ||
| } | ||
| const normalize = (tag: string) => tag.trim().replace(/^W\//, ""); | ||
| const target = normalize(etag); | ||
| return ifNoneMatch.split(",").some((tag) => normalize(tag) === target); | ||
| } | ||
|
|
||
| export const serveStatic = (options: ServeStaticOptions): ServerMiddleware => { | ||
| const dir = resolve(options.dir) + sep; | ||
| const methods = new Set((options.methods || ["GET", "HEAD"]).map((m) => m.toUpperCase())); | ||
| const cacheControl = | ||
| options.cacheControl === undefined | ||
| ? "public, max-age=0, must-revalidate" | ||
| : options.cacheControl; | ||
|
|
||
| // Real (symlink-resolved) base directory, resolved lazily and cached. Used to | ||
| // reject files that escape `dir` through a symlink. | ||
| let realDir: string | undefined; | ||
|
|
||
| return async (req, next) => { | ||
| if (!methods.has(req.method)) { | ||
| return next(); | ||
| } | ||
| const isHead = req.method === "HEAD"; | ||
| const url = (req._url ??= new FastURL(req.url)); | ||
|
|
||
| // Percent-decode the pathname so on-disk names with spaces/unicode are | ||
| // reachable. Malformed sequences must not crash; fall through to `next()`. | ||
| let path: string; | ||
| try { | ||
| path = decodeURIComponent(url.pathname.slice(1).replace(/\/$/, "")); | ||
| } catch { | ||
| return next(); | ||
| } | ||
|
|
||
| // Deny any path segment starting with a dot. This is a deliberate denylist | ||
| // that blocks dotfiles (`.env`, `.env.local`, `.npmrc.bak`, `.git/...`) and | ||
| // dot-segment traversal (`.` / `..`, including once-encoded `%2e` forms | ||
| // which are now decoded), so secrets and parent dirs are never served. | ||
| // | ||
| // A leading `.well-known` (RFC 8615) is the single exemption: it is a | ||
| // registered, public-by-design namespace (ACME challenges, `security.txt`, | ||
| // `assetlinks.json`) that must stay reachable. Only the first segment is | ||
| // exempt, so everything below it is still denied (`/.well-known/.env`), and | ||
| // `.well-known` nested anywhere else (`/sub/.well-known/...`) is not | ||
| // well-known at all and stays denied too. | ||
| const segments = path.split("/"); | ||
| const isWellKnown = segments[0] === ".well-known"; | ||
| for (let i = isWellKnown ? 1 : 0; i < segments.length; i++) { | ||
| if (segments[i].startsWith(".")) { | ||
| return next(); | ||
| } | ||
| } | ||
|
Comment on lines
+175
to
+199
There was a problem hiding this comment. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Map the relevant file and nearby symbols first.
ast-grep outline src/static.ts --view expanded || true
echo
echo "=== src/static.ts (relevant range) ==="
sed -n '1,260p' src/static.ts
echo
echo "=== Search for path resolution / separator handling in src ==="
rg -n "decodeURIComponent|split\\(\"/\"\\)|startsWith\\(\"\\.\"\\)|path\\.|fs\\.|join\\(|resolve\\(|normalize\\(|sep|backslash|well-known" srcRepository: h3js/srvx Length of output: 15973 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Probe Node's path semantics relevant to the claim.
node - <<'JS'
const path = require('path');
const samples = [
'sub%5c.well-known%5cnope.txt',
'sub\\.well-known\\nope.txt',
'.well-known\\nope.txt',
'a/b',
'a\\b',
];
for (const s of samples) {
console.log(`INPUT: ${s}`);
console.log(' posix.split("/"):', s.split('/'));
console.log(' win32.normalize:', path.win32.normalize(s));
console.log(' win32.isAbsolute:', path.win32.isAbsolute(s));
console.log(' win32.parse:', path.win32.parse(s));
console.log(' win32.resolve("C:\\\\base", s):', path.win32.resolve('C:\\base', s));
console.log();
}
JSRepository: h3js/srvx Length of output: 1562 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the exact file-resolution path in src/static.ts if it uses path joins.
python3 - <<'PY'
from pathlib import Path
p = Path('src/static.ts')
text = p.read_text()
for needle in ['path =', 'segments =', 'fs.', 'createReadStream', 'sendFile', 'join(', 'resolve(', 'normalize(', 'URL(', 'pathname']:
idx = text.find(needle)
if idx != -1:
start = max(0, idx - 250)
end = min(len(text), idx + 700)
print(f"\n=== around {needle!r} ===")
print(text[start:end])
PYRepository: h3js/srvx Length of output: 7950 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== src/static.ts lines 1-260 ==="
cat -n src/static.ts | sed -n '1,260p'Repository: h3js/srvx Length of output: 10496 🌐 Web query:
💡 Result: In Node.js, security vulnerabilities such as dotfile bypass or path traversal can occur when static file servers inconsistently normalize paths or decode URL components [1][2][3]. Path Normalization and Backslashes The path.normalize and path.win32.normalize methods resolve '..' and '.' segments and replace multiple, sequential path separators with the platform-specific separator ('' on Windows) [4]. However, these methods are utilities for path cleaning, not security tools [1][2]. They do not constrain the resulting path to a specific directory [2]. On Windows, vulnerabilities have historically occurred where naive path manipulation or inconsistent normalization allowed attackers to escape intended directories using platform-specific behaviors or reserved device names [5][6][7][8]. URL Decoding and Pathname Handling Vulnerabilities often arise from the order of operations between URL decoding (decodeURIComponent) and path normalization [1][3]. If a raw URL pathname is decoded before it is validated or normalized, an attacker can use URL-encoded sequences (e.g., %2e%2e%2f for../) to bypass security filters that only check for literal '..' strings [1][2]. Conversely, if normalization happens before decoding, malicious characters may be hidden within encoded sequences, bypassing checks designed to block dotfiles or restricted paths [3]. Dotfile Bypass and Static Serving In middleware like express.static or serve-static, the dotfiles option (which can be set to 'deny', 'ignore', or 'allow') relies on checking the path for segments beginning with a '.' [9][10][11]. If the path is not correctly decoded or normalized before this check is performed, an attacker might bypass the restriction [3]. For example, if a system uses a different normalization logic than the security filter, or if the filter fails to account for how the underlying file system interprets encoded characters, access to sensitive files (e.g.,.env) can be granted [1][3]. Recommended Best Practices 1. Never trust user input as a direct file path [1]. 2. Always resolve the final path using path.resolve against a fixed, intended base directory [1][2]. 3. After resolution, verify the resulting path starts with the base directory path (including the path separator) to prevent traversal [1][2]. 4. Decode URL parameters only once and perform all security checks (like dotfile restrictions) on the fully resolved, decoded path [1]. 5. Prefer higher-level abstractions like res.sendFile with an explicit root option, which is designed to constrain access to the specified root automatically [2]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== src/_url.ts ==="
cat -n src/_url.ts
echo
echo "=== any backslash / pathname normalization in request URL handling ==="
rg -n "backslashes|\\\\|pathname|URL|FastURL|new URL" src/_url.ts src/adapters/_node/url.ts src/adapters/_node/request.ts src/static.tsRepository: h3js/srvx Length of output: 13273 Reject decoded backslashes before segment validation. src/static.ts:175-199 — 🤖 Prompt for AI Agents |
||
|
|
||
| let paths: string[]; | ||
| if (path === "") { | ||
| paths = ["index.html"]; | ||
| } else if (isWellKnown || extname(path) !== "") { | ||
| // Well-known URIs are exact identifiers, so the `.html`/`index.html` | ||
| // fallback must not apply below `/.well-known/`: ACME challenge tokens | ||
| // (`/.well-known/acme-challenge/<token>`) are extensionless and would | ||
| // otherwise resolve to `<token>.html` and 404, silently breaking cert | ||
| // renewal. | ||
| paths = [path]; | ||
| } else { | ||
| paths = [`${path}.html`, `${path}/index.html`]; | ||
| } | ||
|
|
||
| for (const candidate of paths) { | ||
| const filePath = join(dir, candidate); | ||
| // Defense-in-depth: `join` normalization must not escape `dir`. | ||
| if (!filePath.startsWith(dir)) { | ||
| continue; | ||
| } | ||
| const fileStat = await stat(filePath).catch(() => null); | ||
| if (!fileStat?.isFile()) { | ||
| continue; | ||
| } | ||
|
|
||
| // Symlink escape: resolve the real path and ensure it stays inside the | ||
| // real base directory before serving. | ||
| try { | ||
| if (realDir === undefined) { | ||
| realDir = (await realpath(resolve(options.dir))) + sep; | ||
| } | ||
| const realFile = await realpath(filePath); | ||
| if (realFile !== realDir.slice(0, -1) && !realFile.startsWith(realDir)) { | ||
| continue; | ||
| } | ||
| } catch { | ||
| continue; | ||
| } | ||
|
|
||
| const fileExt = extname(filePath); | ||
| const contentType = COMMON_MIME_TYPES[fileExt] || "application/octet-stream"; | ||
|
|
||
| if (options.renderHTML && fileExt === ".html") { | ||
| return options.renderHTML({ | ||
| html: await readFile(filePath, "utf8"), | ||
| filename: filePath, | ||
| request: req, | ||
| }); | ||
| } | ||
|
Comment on lines
+243
to
+249
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Apply HEAD body suppression to rendered HTML responses. The early Proposed fix if (options.renderHTML && fileExt === ".html") {
- return options.renderHTML({
+ const response = await options.renderHTML({
html: await readFile(filePath, "utf8"),
filename: filePath,
request: req,
});
+ if (!isHead) {
+ return response;
+ }
+ return new FastResponse(null, {
+ status: response.status,
+ statusText: response.statusText,
+ headers: response.headers,
+ });
}Also applies to: 299-302 🤖 Prompt for AI Agents |
||
|
|
||
| // Validators for conditional requests and caching. | ||
| const mtime = fileStat.mtime; | ||
| const etag = `W/"${fileStat.size.toString(16)}-${mtime.getTime().toString(16)}"`; | ||
| const lastModified = mtime.toUTCString(); | ||
|
|
||
| const headers: Record<string, string> = { | ||
| "Content-Type": contentType, | ||
| "Content-Length": fileStat.size.toString(), | ||
| ETag: etag, | ||
| "Last-Modified": lastModified, | ||
| }; | ||
| if (cacheControl) { | ||
| headers["Cache-Control"] = cacheControl; | ||
| } | ||
|
|
||
| // Compression negotiation (only for compressible types). `Vary` is set on | ||
| // both the compressed and the identity variant so caches key correctly. | ||
| let encoding: "br" | "gzip" | undefined; | ||
| if (isCompressible(contentType)) { | ||
| headers["Vary"] = "Accept-Encoding"; | ||
| encoding = negotiateEncoding(req.headers.get("accept-encoding") || ""); | ||
| if (encoding) { | ||
| headers["Content-Encoding"] = encoding; | ||
| // Compressed length is unknown ahead of time. | ||
| delete headers["Content-Length"]; | ||
| } | ||
| } | ||
|
|
||
| // Conditional requests: `If-None-Match` takes precedence over | ||
| // `If-Modified-Since` (RFC 9110). Respond `304` with no body. | ||
| const ifNoneMatch = req.headers.get("if-none-match"); | ||
| const ifModifiedSince = req.headers.get("if-modified-since"); | ||
| let notModified = false; | ||
| if (ifNoneMatch) { | ||
| notModified = etagMatches(ifNoneMatch, etag); | ||
| } else if (ifModifiedSince) { | ||
| const since = Date.parse(ifModifiedSince); | ||
| // Compare at second resolution (HTTP dates have no sub-second part). | ||
| if (!Number.isNaN(since) && Math.floor(mtime.getTime() / 1000) * 1000 <= since) { | ||
| notModified = true; | ||
| } | ||
| } | ||
| if (notModified) { | ||
| delete headers["Content-Length"]; | ||
| delete headers["Content-Encoding"]; | ||
| return new FastResponse(null, { status: 304, headers }); | ||
|
Comment on lines
+281
to
+296
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby logic.
ast-grep outline src/static.ts --view expanded || true
wc -l src/static.ts
sed -n '240,340p' src/static.tsRepository: h3js/srvx Length of output: 3531 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect method handling and the serveStatic control flow around the conditional-response block.
sed -n '154,314p' src/static.tsRepository: h3js/srvx Length of output: 6294 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find any explicit method checks or method allowlists in src/static.ts.
rg -n 'method|methods|GET|HEAD|POST|If-None-Match|If-Modified-Since|412|304' src/static.tsRepository: h3js/srvx Length of output: 703 Gate conditional responses by method When 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| // HEAD: send the same headers a GET would, with no body work. | ||
| if (isHead) { | ||
| return new FastResponse(null, { headers }); | ||
| } | ||
|
|
||
| let stream: ReadStream | Transform = createReadStream(filePath); | ||
| if (encoding === "br") { | ||
| stream = stream.pipe(createBrotliCompress()); | ||
| } else if (encoding === "gzip") { | ||
| stream = stream.pipe(createGzip()); | ||
| } | ||
| return new FastResponse(stream as any, { headers }); | ||
| } | ||
| return next(); | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: h3js/srvx
Length of output: 11252
🏁 Script executed:
Repository: h3js/srvx
Length of output: 8567
🏁 Script executed:
Repository: h3js/srvx
Length of output: 12887
🌐 Web query:
RFC 9110 Accept-Encoding identity q=0 *;q=0 no acceptable representation 406💡 Result:
Under RFC 9110, the Accept-Encoding header field is used by a client to specify the content codings that are acceptable in the response [1][2]. The identity content coding, which represents the original, uncompressed data, is always considered acceptable by default unless it is specifically excluded [3][4]. A client can explicitly exclude the identity coding by including identity;q=0 in the Accept-Encoding header [3]. Similarly, if a client includes the wildcard *;q=0, this also excludes all content codings, including identity, unless a specific, higher q-value is provided for identity (e.g., *;q=0, identity;q=1) [3][5][6]. When a client explicitly excludes all available content codings—for instance, by sending Accept-Encoding: identity;q=0 when the server cannot provide any compressed representations—the server is unable to provide an acceptable representation [3][7][5]. In such cases, the server should respond with a 406 (Not Acceptable) status code to indicate that no suitable representation could be produced [3][1][7]. Essentially, sending identity;q=0 or *;q=0 (without enabling identity) serves as a signal that the client cannot handle uncompressed content, effectively making "uncompressed" an unacceptable state [3][5]. If the server's only available options are also unacceptable to the client based on these preferences, the 406 response is the semantically correct way to communicate this mismatch [3][5].
Citations:
🏁 Script executed:
Repository: h3js/srvx
Length of output: 187
Return 406 when identity is disallowed.
undefinedstill collapses “no acceptable encoding” into an identity200, soidentity;q=0and*;q=0can be served uncompressed here, including for non-compressible files where negotiation is skipped. Split that case out and return406 Not Acceptableinstead.🤖 Prompt for AI Agents