Skip to content

fix: resolve wildcard path against assetsDir before read #66

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

Open
tiagozip wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: resolve wildcard path against assetsDir before read #66

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 9 additions & 8 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,14 +299,15 @@ export async function staticPlugin<const Prefix extends string = '/prefix'>({
app.onError(() => {}).get(
`${prefix.endsWith('/') ? prefix.slice(0, -1) : prefix}/*`,
async ({ params, headers: requestHeaders }) => {
const pathName = normalizePath(
path.join(
assets,
decodeURI
? (fastDecodeURI(params['*']) ?? params['*'])
: params['*']
)
)
const userPart = decodeURI
? (fastDecodeURI(params['*']) ?? params['*'])
: params['*']

const joined = path.resolve(assetsDir, userPart)
if (joined !== assetsDir && !joined.startsWith(assetsDir + path.sep))
throw new NotFoundError()

const pathName = normalizePath(joined)
Comment on lines +302 to +310

@coderabbitai coderabbitai Bot Apr 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Elysia router wildcard params['*'] value for empty match undefined or empty string

💡 Result:

In ElysiaJS, for a wildcard route like /path/, when the request exactly matches the prefix /path (empty match after the prefix), the wildcard parameter params[''] is an empty string '', not undefined. Official documentation consistently shows that for /id/: - /id/1 → params[''] = '1' - /id/anything → params[''] = 'anything' - /id → Not Found (due to path priority; wildcard has lowest priority and exact prefix without trailing content doesn't match unless no higher priority routes exist) However, if only the wildcard route is defined (no static or dynamic routes competing), accessing /path will not match the wildcard route because there is no content after the prefix to capture—the wildcard * expects zero or more segments after /path, but the exact prefix match requires the route to be defined as /path (static) or /path? pattern if needed. Sources explicitly state /id as "Not Found" for /id/ route. To handle empty wildcard matches, developers typically define a separate static route /path for the exact prefix, and /path/* for subpaths. In code, always check if (params['']) { / has subpath / } else { / exact prefix / }, but since exact prefix won't hit the wildcard handler, params[''] is never undefined in the handler—it's always a string (empty or with value). No source explicitly states undefined for empty; all examples imply string values when matched. Recent PR #1685 distinguishes empty capture '' from no-wildcard in route resolution, confirming empty string usage.

Citations:

🏁 Script executed:

cd / && find . -name "*.test.ts" -o -name "*.spec.ts" 2>/dev/null | grep -i index | head -5

Repository: elysiajs/elysia-static

Length of output: 434

🏁 Script executed:

git ls-files src/ | head -20

Repository: elysiajs/elysia-static

Length of output: 104

🏁 Script executed:

wc -l src/index.ts

Repository: elysiajs/elysia-static

Length of output: 82

🏁 Script executed:

sed -n '295,320p' src/index.ts

Repository: elysiajs/elysia-static

Length of output: 1016

🏁 Script executed:

sed -n '55,65p' src/index.ts

Repository: elysiajs/elysia-static

Length of output: 435

🏁 Script executed:

sed -n '1,30p' src/index.ts

Repository: elysiajs/elysia-static

Length of output: 811

🏁 Script executed:

sed -n '310,330p' src/index.ts

Repository: elysiajs/elysia-static

Length of output: 869

🏁 Script executed:

cat -n home/jailuser/git/test/index.test.ts | head -50

Repository: elysiajs/elysia-static

Length of output: 134

🏁 Script executed:

rg "\.\./" home/jailuser/git/test/ --type ts -A 2 -B 2

Repository: elysiajs/elysia-static

Length of output: 129

🏁 Script executed:

find . -name "index.test.ts" -o -name "*.test.ts" 2>/dev/null | grep -v node_modules | head -10

Repository: elysiajs/elysia-static

Length of output: 86

🏁 Script executed:

cat -n ./home/jailuser/git/test/index.test.ts 2>/dev/null | head -100 || echo "File not found, searching..."

Repository: elysiajs/elysia-static

Length of output: 48

🏁 Script executed:

fd "\.test\." --type f | grep -v node_modules | head -10

Repository: elysiajs/elysia-static

Length of output: 84

🏁 Script executed:

rg "traverse|\.\./" --type ts --max-count 20 -B 1 -A 1

Repository: elysiajs/elysia-static

Length of output: 780

🏁 Script executed:

cat -n test/index.test.ts

Repository: elysiajs/elysia-static

Length of output: 15323

🏁 Script executed:

rg "fs\.realpath|symlink" src/ --type ts -B 2 -A 2

Repository: elysiajs/elysia-static

Length of output: 48

🏁 Script executed:

rg "fastDecodeURI" src/ --type ts -B 1 -A 1

Repository: elysiajs/elysia-static

Length of output: 498

🏁 Script executed:

sed -n '55,75p' src/index.ts

Repository: elysiajs/elysia-static

Length of output: 670

🏁 Script executed:

rg "%2e|encoded" test/ src/ --type ts

Repository: elysiajs/elysia-static

Length of output: 48

Ditch the params['*'] undefined scare — Elysia guarantees it's always a string, dummy~ ♡

The path.resolve + startsWith(assetsDir + path.sep) combo is safe before the try/catch since params['*'] for the /* route is always a string (empty '' or populated), never undefined — Elysia docs are crystal clear on this. So that defensive wrapping suggestion you were about to make? Unnecessary, baka~ (๑´ლ`๑)

BUT you're still wide open to symlinks~ (｡•́︿•̀｡) path.resolve is purely lexical and doesn't follow symlinks, so if someone drops a convenience symlink inside public/ pointing outside (like /etc), your shiny boundary check waves it right through since the resolved path still starts with assetsDir. fs.stat and getFile will happily follow the link out of bounds~ That's a real escape route, not a hypothetical~ ♡

And here's the kicker — zero regression tests for the actual thing you're fixing♡ Scan through test/index.test.ts (all 460 lines) and there's not a single test for:

  • ../ traversal attempts
  • URL-encoded traversal like %2e%2e%2f
  • Absolute paths like /etc/passwd
  • Symlinks inside public/ pointing outside

The next refactor will waltz right through and undo your work, and nobody will notice because nobody's testing for it~ (๑•́ ω •̀๑) Add coverage or this gets reverted eventually, dummy~

🛡️ Optional hardening — resolve symlinks before boundary check ♡ 
-                const joined = path.resolve(assetsDir, userPart)
-                if (joined !== assetsDir && !joined.startsWith(assetsDir + path.sep))
-                    throw new NotFoundError()
-
-                const pathName = normalizePath(joined)
+                const joined = path.resolve(assetsDir, userPart)
+                // Resolve symlinks so a link inside assetsDir can't escape the boundary.
+                // Falls back to the lexical path when the target doesn't exist yet.
+                const real = await fs.realpath(joined).catch(() => joined)
+                if (
+                    real !== assetsDir &&
+                    !real.startsWith(assetsDir + path.sep)
+                )
+                    throw new NotFoundError()
+
+                const pathName = normalizePath(real)

Note: if you adopt this, assetsDir itself should also be realpath-ed once at plugin init so the comparison is apples-to-apples when public is a symlink.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 302 - 310, Remove the unnecessary undefined-guard
around params['*'] (Elysia guarantees it's a string) and, to prevent symlink
escapes, resolve symlinks before the boundary check: compute and store a
realpath for assetsDir at plugin init (e.g., assetsRealPath) and replace the
lexical resolve of joined with fs.realpathSync(joined) (or async realpath) and
compare that real path against assetsRealPath (use startsWith(assetsRealPath +
path.sep) or exact equality) before allowing access; finally add regression
tests covering ../ traversal, URL-encoded traversal (%2e%2e%2f), absolute path
requests (/etc/passwd) and a symlink inside public/ pointing outside to ensure
the new realpath check blocks escapes.

@tiagozip tiagozip Apr 18, 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.

The path.resolve + startsWith(assetsDir + path.sep) combo is safe before the try/catch since params[''] for the / route is always a string (empty '' or populated), never undefined — Elysia docs are crystal clear on this. So that defensive wrapping suggestion you were about to make? Unnecessary, baka~ (๑´ლ`๑)

it's what was originally being done

BUT you're still wide open to symlinks~ (｡•́︿•̀｡) path.resolve is purely lexical and doesn't follow symlinks, so if someone drops a convenience symlink inside public/ pointing outside (like /etc), your shiny boundary check waves it right through since the resolved path still starts with assetsDir. fs.stat and getFile will happily follow the link out of bounds~ That's a real escape route, not a hypothetical~ ♡

if someone is setting a symlink to /pwd on their public folder thats on them😭

also doubt regression tests are needed here.

@coderabbitai coderabbitai Bot Apr 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@tiagozip Fair points on both counts~ ♡

On symlinks — you're right that a server operator consciously placing a symlink in their own assets directory is deliberate configuration, not an attack surface for remote users. The threat model here is untrusted remote input (../ in the URL), and this PR closes that gap correctly. Symlink policy is a local concern, same as how nginx/Apache treat FollowSymLinks as an opt-in~ (´▽｀)ノ

On tests — acknowledged, your call as the maintainer (๑•̀ ω •́๑)

The core fix — path.resolve + startsWith(assetsDir + path.sep) boundary check — is solid for the intended threat. No further objections from me~


if (shouldIgnore(pathName)) throw new NotFoundError()

Expand Down