fix(static): harden srvx/static - #233
Conversation
- Percent-decode the request pathname so filenames with spaces/unicode are reachable; malformed sequences fall through instead of crashing. Traversal checks run against the decoded path (raw and encoded `%2e%2e` blocked). - Deny any path segment starting with a dot (dotfiles like `.env`, `.env.local`, `.npmrc.bak`, `.git/...` and dot-segment traversal). - Resolve symlinks with `fs.realpath` and reject files that escape `dir`. - Compression correctness: only compress compressible MIME types (never re-encode png/jpg/woff/etc.); parse `Accept-Encoding` with exact tokens + q-values (`br;q=0` disabled, `abbr` no longer matches `br`); set `Vary: Accept-Encoding` on both compressed and identity variants. - Add `ETag` (size+mtime) and `Last-Modified`; handle `If-None-Match` / `If-Modified-Since` -> `304`; add a configurable conservative `Cache-Control`. - HEAD sends the same headers as GET with no body/compression work. - Add test/static.test.ts covering all of the above. Range/206 support is intentionally deferred. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughChangesStatic serving behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
pi0x
left a comment
pi0x
left a comment
There was a problem hiding this comment.
Adversarial review — fix(static): harden srvx/static (F31 + F32)
Verdict: APPROVE with comments. I could not construct a traversal or dotfile bypass, and every locked-scope spec point is implemented and tested. All 38 new tests pass; the full suite (953 passed / 35 skipped) is green; oxlint clean. One substantive usability finding (.well-known) and a few nits below — none is a security bypass or a spec violation, so this is a comment, not a block.
I checked out the PR, read src/static.ts + test/static.test.ts in context, and ran ~25 local attack probes (not committed) driving the middleware with raw origin-form targets that bypass Request's WHATWG normalization (i.e. the real Node-adapter wire path), which the shipped tests do not exercise since they build new Request(...).
Spec checklist (F31 + F32, locked scope)
- Percent-decoding —
decodeURIComponentadded (static.ts:177), dot-segment +startsWith(dir)checks kept.hello%20world.txt/caf%C3%A9.txtreachable. - Traversal safe — verified raw
../,%2e%2e,..%2f, double-encoded%252e%252e, mixedfoo%2f..%2f.., encoded backslash..%5c, and%00-prefixed all → 404, no secret leak. Two independent layers: the URL parser collapses/clamps literal..at root, and the middleware dot-check (static.ts:186) catches decoded encoded forms. Double-encoding is safe because a single decode leaves literal%2e/%2ftext (no real separators). - Null bytes / malformed percent —
decodeURIComponent("%ZZ")caught →next()(no 500).stat()on a null-byte path rejects (async), caught by.catch(()=>null)→ 404. No sync-throw 500. Confirmed. - Dotfile denylist — deliberate
segment.startsWith(".")blocks.env,.env.local,.npmrc.bak,.git/..., and dot-directories. (See finding 1 re: over-breadth.) - Symlinks —
realpathon both base and file; escape rejected (static.ts:212-222). Verified with a symlinked file and a symlinked directory pointing outsidedir→ both 404. - No re-compression —
isCompressible()gates it;.png/.jpg/.mp4/.zip/.woff2never encoded, andVarycorrectly omitted for them. -
accept-encodingparsing — real q-value parse;br;q=0→ no brotli (falls to gzip/identity);abbrno longer matchesbr;Vary: Accept-Encodingon both compressed and identity variants of compressible types. - Conditional / caching —
ETag(weak, size+mtime) +Last-Modified;If-None-Match(incl.*, multi-tag,W/normalization) precedesIf-Modified-Since; 304 strips body/Content-Length/Content-Encoding; saneCache-Controldefault,falseto disable. - HEAD — no read/compress; same headers, null body.
- Range/206 OUT — correctly absent. Nothing out-of-scope snuck in.
Findings
1. [Medium — usability/correctness] .well-known/ is blanket-denied. static.ts:186 — the dot-segment denylist rejects every segment starting with ., including .well-known. On the CLI's default serving path this breaks ACME HTTP-01 (/.well-known/acme-challenge/... → Let's Encrypt cert issuance), security.txt, apple-app-site-association, and assetlinks.json. Verified: GET /.well-known/acme-challenge → 404. The plan explicitly flagged .well-known as "SHOULD arguably be servable — check what they chose." The choice here is to deny it. Recommend allowlisting a leading .well-known/ segment (deny all other dotfiles), so the default static server can actually obtain a certificate. Not a security bug, but a real regression for a very common deployment.
2. [Low — caching semantics] ETag is shared across encodings. static.ts:237 — the gzip variant and the identity variant carry the same ETag (confirmed W/"4-…" identical). This is defensible: it is a weak validator (weak ETags denote semantic equivalence, and different content-encodings of the same bytes are semantically equivalent) and Vary: Accept-Encoding is set, so shared caches key correctly. Worth a one-line comment acknowledging the weak-ETag choice; no change required. (The plan's "gzip and identity must not share an ETag" targets strong ETags; using weak ones satisfies it in spirit.)
3. [Low] renderHTML bypasses conditional/caching and HEAD. static.ts:227-233 returns before ETag/Last-Modified/304/HEAD handling, so an .html file served through renderHTML gets no validators and is fully readFile'd even for HEAD. Pre-existing behavior shape; acceptable, but note it.
4. [Nit] .br → application/x-brotli (static.ts:67) is nonstandard; there is no registered type for a raw .br file. application/octet-stream (the fallback) would be more honest. Cosmetic.
5. [Nit] q-value regex rejects q=.5. static.ts:105 /^q=(\d+(?:\.\d+)?)$/ requires a leading digit, so q=.5 (seen in the wild, though technically ill-formed) is ignored and defaults to q=1. And *;q=0 still yields an identity response rather than 406 — pragmatic and safe, just noting the minor deviations.
Missing from locked scope
None. Every F31/F32 locked item is implemented and covered by a test.
Nice work — the security-critical core (decode-then-dot-check-then-containment, plus the realpath symlink guard) is correct and I couldn't get past it. Addressing finding 1 before it lands on the default CLI path would be my only real ask.
The dotfile denylist blocked every segment starting with a dot, which
also blocked `/.well-known/` -- silently breaking ACME/Let's Encrypt
renewal, `security.txt` and `assetlinks.json`, with no way to opt out
(the CLI passes only `dir`).
Exempt a *leading* `.well-known` segment from the denylist. Everything
below it is still checked, so `/.well-known/.env` and traversal out of
`/.well-known/` stay denied, and `.well-known` nested anywhere else is
not well-known and stays denied too.
Also skip the `.html`/`index.html` fallback under `/.well-known/`.
Well-known URIs are exact identifiers, and ACME challenge tokens are
extensionless -- `/.well-known/acme-challenge/<token>` would otherwise
resolve to `<token>.html` and 404, so the exemption alone did not fix
cert renewal.
Tests: `/.env` alone could not catch a denylist regression, because
`extname(".env")` is "" and the fallback looks for `.env.html` -- it
404s either way. Add `.env.local` / `.npmrc.bak` cases, which resolve to
real files and so are only stopped by the denylist. Verified by
mutation: removing the denylist, the exemption, or the fallback skip
each fail tests that previously passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/static.ts`:
- Around line 243-249: Update the renderHTML branch in the static request
handler so HEAD requests preserve the renderer’s headers/status but suppress the
returned response body, matching the existing HEAD handling around lines
299-302. Keep the rendered HTML body for non-HEAD requests.
- Around line 120-141: The encoding negotiation flow around negotiateEncoding
must distinguish “no compression selected” from identity being explicitly
disallowed. Track identity’s effective quality, including identity;q=0 and the
wildcard *;q=0, and propagate an explicit not-acceptable result so the request
returns 406 instead of falling back to an uncompressed 200, including when
compression negotiation is skipped for non-compressible files.
- Around line 281-296: Update the conditional-response logic around ifNoneMatch,
ifModifiedSince, and notModified to distinguish GET/HEAD from other methods. For
GET/HEAD, preserve the existing 304 handling; for non-GET/HEAD requests, a
matching If-None-Match must return 412, and If-Modified-Since must not
participate.
- Around line 175-199: Update the decoded path validation in the static request
handler before segment checks to reject any path containing a backslash,
including decoded %5c sequences, by returning next(). Ensure this validation
occurs before path joining or file resolution, while preserving the existing
.well-known exemption and dot-segment checks.
In `@test/static.test.ts`:
- Around line 136-139: Update the “raw ../ cannot escape dir” test to bypass
Request URL normalization by constructing a ServerRequest with url set directly
to "/../../outside/secret.txt" before invoking the static-serving path. Preserve
the existing response assertion against NEXT so the test verifies raw traversal
protection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e47a1cd9-f99c-4336-885e-bed56bd981d9
📒 Files selected for processing (3)
docs/1.guide/10.cli.mdsrc/static.tstest/static.test.ts
| 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; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file before reading the specific slice.
ast-grep outline src/static.ts --view expanded || true
printf '\n--- src/static.ts (selected lines) ---\n'
sed -n '1,220p' src/static.ts | cat -n
printf '\n--- Search for negotiateEncoding usages and 406 handling ---\n'
rg -n "negotiateEncoding|406|identity|Accept-Encoding|parseAcceptEncoding" src/static.ts src -SRepository: h3js/srvx
Length of output: 11252
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the parts that decide whether to serve compressed, identity, or 406.
sed -n '120,220p' src/static.ts | cat -n
sed -n '220,340p' src/static.ts | cat -nRepository: h3js/srvx
Length of output: 8567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
git ls-files src/static.ts
sed -n '1,360p' src/static.ts | cat -nRepository: 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:
- 1: https://datatracker.ietf.org/doc/html/rfc9110
- 2: https://www.rfc-editor.org/rfc/rfc9110.pdf
- 3: Recommended response behavior when identity encoding is unacceptable httpwg/http-core#980
- 4: https://groups.google.com/g/google-documents-list-api/c/D_1WAAP2hGY
- 5: Fix: Handle wildcard * in Accept-Encoding per RFC 9110 §12.5.3 tower-rs/tower-http#693
- 6: Accept-Encoding header parsing and interpretation aio-libs/aiohttp#8104
- 7: https://http.dev/accept-encoding
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the static handler section around the encoding decision and response selection.
nl -ba src/static.ts | sed -n '140,320p'Repository: h3js/srvx
Length of output: 187
Return 406 when identity is disallowed.
undefined still collapses “no acceptable encoding” into an identity 200, so identity;q=0 and *;q=0 can be served uncompressed here, including for non-compressible files where negotiation is skipped. Split that case out and return 406 Not Acceptable instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/static.ts` around lines 120 - 141, The encoding negotiation flow around
negotiateEncoding must distinguish “no compression selected” from identity being
explicitly disallowed. Track identity’s effective quality, including
identity;q=0 and the wildcard *;q=0, and propagate an explicit not-acceptable
result so the request returns 406 instead of falling back to an uncompressed
200, including when compression negotiation is skipped for non-compressible
files.
| 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(); | ||
| } | ||
| } |
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:
Node.js Windows path backslash separator path.win32 normalize decodeURIComponent URL pathname static file serving dotfile bypass
💡 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:
- 1: https://blog.openreplay.com/prevent-path-traversal-nodejs/
- 2: https://docs.cli.vulnetix.com/docs/sast-rules/vnx-node-010/
- 3: GHSA-wc8c-qw6v-h7f6
- 4: https://nodejs.org/docs/latest-v24.x/api/path.html
- 5: nodejs/node@42d5821873
- 6: nodejs/node@b0a4f16
- 7: path: match Windows reserved names by component boundary nodejs/node#61545
- 8: nodejs/node@9cc89f55f7
- 9: https://github.com/expressjs/serve-static
- 10: https://github.com/expressjs/serve-static/blob/v1.14.0/README.md
- 11: https://github.com/expressjs/serve-static/tree/refs/tags/v1.16.3
🏁 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 — %5c becomes \ after decodeURIComponent(), and path.split("/") won’t catch it before join() treats it as a separator on Windows. Return next() when the decoded path contains \, or normalize separators before the dot-segment check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/static.ts` around lines 175 - 199, Update the decoded path validation in
the static request handler before segment checks to reject any path containing a
backslash, including decoded %5c sequences, by returning next(). Ensure this
validation occurs before path joining or file resolution, while preserving the
existing .well-known exemption and dot-segment checks.
| if (options.renderHTML && fileExt === ".html") { | ||
| return options.renderHTML({ | ||
| html: await readFile(filePath, "utf8"), | ||
| filename: filePath, | ||
| request: req, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply HEAD body suppression to rendered HTML responses.
The early renderHTML return bypasses Lines 299-302, so a HEAD request can return the renderer’s body.
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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/static.ts` around lines 243 - 249, Update the renderHTML branch in the
static request handler so HEAD requests preserve the renderer’s headers/status
but suppress the returned response body, matching the existing HEAD handling
around lines 299-302. Keep the rendered HTML body for non-HEAD requests.
| 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 }); |
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 methods includes POST or another non-GET/HEAD method, this block still returns 304 for a matching If-None-Match. Those requests should use 412, and If-Modified-Since should be ignored unless the request is GET or HEAD.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/static.ts` around lines 281 - 296, Update the conditional-response logic
around ifNoneMatch, ifModifiedSince, and notModified to distinguish GET/HEAD
from other methods. For GET/HEAD, preserve the existing 304 handling; for
non-GET/HEAD requests, a matching If-None-Match must return 412, and
If-Modified-Since must not participate.
| test("raw ../ cannot escape dir", async () => { | ||
| const res = await req("/../../outside/secret.txt"); | ||
| expect(await res.text()).toBe(NEXT); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## test/static.test.ts (around the referenced lines)\n'
sed -n '110,170p' test/static.test.ts
printf '\n## locate serveStatic implementation and request handling\n'
fd -a 'static' . || true
rg -n "serveStatic|ServerRequest|Request\(" -S . --glob '!**/node_modules/**'Repository: h3js/srvx
Length of output: 20768
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## test/static.test.ts helper and traversal tests\n'
sed -n '1,95p' test/static.test.ts
sed -n '135,165p' test/static.test.ts
printf '\n## src/static.ts around serveStatic\n'
sed -n '1,260p' src/static.ts
printf '\n## request/url adapter bits that may normalize path segments\n'
sed -n '1,140p' src/adapters/_node/request.ts
sed -n '1,120p' src/adapters/_node/url.tsRepository: h3js/srvx
Length of output: 21369
Send the raw traversal target without Request normalization.
new Request("http://localhost/../../outside/secret.txt") shortens the path before serveStatic sees it, so this test can pass even if the raw ../ guard regresses. Build a ServerRequest with url: "/../../outside/secret.txt" here instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/static.test.ts` around lines 136 - 139, Update the “raw ../ cannot
escape dir” test to bypass Request URL normalization by constructing a
ServerRequest with url set directly to "/../../outside/secret.txt" before
invoking the static-serving path. Preserve the existing response assertion
against NEXT so the test verifies raw traversal protection.
2 participants
Hardens
srvx/static(on the CLI default path, previously untested):../and encoded%2e%2estay blocked..(covers.env,.env.local,.npmrc.bak,.git/..., and dot-segment traversal).fs.realpathand reject files that escapedir.Accept-Encodingby exact tokens + q-values (br;q=0disabled,abbrno longer matchesbr, identity fallback); setVary: Accept-Encodingon both compressed and identity variants.ETag(size+mtime) andLast-Modified; handleIf-None-Match/If-Modified-Since→304; add a configurable conservativeCache-Controldefault.test/static.test.tscovering traversal, decoding, MIME, encoding negotiation, HEAD, index/.htmlresolution, 404 fallthrough, dotfiles, symlink escape, conditional requests,Vary, and already-compressed types.Range/206 support is intentionally deferred to a follow-up.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Cache-Controlsettings.ETag,Last-Modified, and304 Not Modified.index.html, extensionless HTML paths, and.well-knownresources.Bug Fixes
HEADrequest handling and malformed URL processing.Documentation