Skip to content

fix(web-ui): serve /ui on Windows by comparing against the platform separator - #322

Merged
saucam merged 3 commits into
mainfrom
fix/web-ui-windows-path-separator
Sep 5, 2026
Merged

fix(web-ui): serve /ui on Windows by comparing against the platform separator#322
saucam merged 3 commits into
mainfrom
fix/web-ui-windows-path-separator

Conversation

@saucam

@saucam saucam commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #299.

The bug

The path-traversal guard in src/frontends/web-ui/index.ts tested containment with a hardcoded /:

if (target !== DIST && !target.startsWith(`${DIST}/`))

normalize() emits the platform separator, so on Windows every legitimate asset resolves to C:\...\web\dist\index.html — which does not start with C:\...\web\dist/. The guard fired on valid files and the whole UI returned 403 Forbidden.

What the guard was actually doing

Worth recording, because it changes how you read this diff: that 403 branch is unreachable over HTTP. The WHATWG URL parser resolves .. segments (and folds \ to / for http) before handleFetch ever sees the path:

http://h/ui/../../../etc/passwd  ->  pathname "/etc/passwd"
http://h/ui/..\..\x              ->  pathname "/x"

Both fall out of the /ui/ prefix check entirely. So the guard's only observable effect in production was this Windows false positive. It is still correct defense-in-depth, so this fixes it rather than dropping it.

The fix

The reporter's one-line sep fix is right, but no test on a POSIX CI machine can verify it. So the mapping moves into a pure resolveUiAsset(distRoot, urlPath, pathFlavor) — the precedent buildBootScript already set in this module. The path flavor is injectable, so path.win32 reproduces the Windows behaviour from Linux and the regression is covered where CI actually runs. handleFetch drops to three lines.

Tests

src/tests/web-ui-asset-path.test.ts, 14 cases across both flavors: legitimate nested assets, the mount root with and without a trailing slash, SPA deep links, .. traversal, and a sibling directory that merely shares the root's string prefix (web/dist-secrets).

Confirmed to catch the regression — reverting the guard to ${distRoot}/ fails four win32 cases; restoring ${distRoot}${p.sep} passes.

Verification

Full suite 2443 pass / 0 fail; typecheck and biome clean.

End to end against a live local-mode daemon (bun src/cli.ts start --local):

Request Result
/ui, /ui/ 200 text/html
/ui/assets/index-*.js 200 text/javascript, 498706b
/ui/assets/index-*.css 200 text/css, 52480b
/ui/sessions/abc (SPA deep link) 200 text/html
/ui/assets/nope.js 404
4 traversal payloads, incl. %2f-encoded never escaped dist

Then the real UI path: harvested the injected __CODEOID_LOCAL_TOKEN__ from the served HTML, opened the WebSocket, auth.ok, session.list.result → 4 sessions.

Not verified on Windows hardware — I have no Windows host here. The win32 coverage is path.win32 semantics, which is precisely where the bug lived; @yl-dev-tmtc confirmed the equivalent sep fix resolves it on a real Windows machine.

Thanks to @yl-dev-tmtc for reporting this with the cause and the fix already identified.

🤖 Generated with Claude Code

saucam and others added 3 commits September 5, 2026 23:31
…eparator

The path-traversal guard in the web-ui frontend tested containment with a
hardcoded `/`:

    if (target !== DIST && !target.startsWith(`${DIST}/`))

`normalize()` emits the PLATFORM separator, so on Windows every legitimate
asset resolves to `C:\...\web\dist\index.html`, which does not start with
`C:\...\web\dist/`. The guard fired on valid files and the entire UI returned
403 Forbidden. Fixes #299.

Worth recording what the guard was actually doing: that 403 branch is
unreachable over HTTP. The WHATWG URL parser resolves `..` segments (and folds
`\` to `/` for http) before `handleFetch` sees the path, so `/ui/../../etc/passwd`
arrives as pathname `/etc/passwd` and falls out of the `/ui/` prefix check
entirely. The guard's only observable effect in production was this Windows
false positive. It is still correct defense-in-depth, so it is fixed rather
than dropped.

Rather than only swapping in `sep` — which no test on a POSIX CI machine can
verify — the mapping moves into a pure `resolveUiAsset(distRoot, urlPath,
pathFlavor)`, following the precedent already set by `buildBootScript` in this
module. The path flavor is injectable, so `path.win32` reproduces the Windows
behaviour from Linux and the regression is covered where it actually runs.

Tests assert both flavors: legitimate nested assets, the mount root with and
without a trailing slash, SPA deep links, `..` traversal, and a sibling
directory that merely shares the root's string prefix (`web/dist-secrets`).
Confirmed to catch the regression — reverting the guard to `${distRoot}/`
fails four win32 cases; restoring `${distRoot}${p.sep}` passes.

Verified end to end against a live local-mode daemon: `/ui` and `/ui/` 200,
both hashed bundle assets 200, SPA deep link 200, missing asset 404, four
traversal payloads (including `%2f`-encoded) never escaping dist, and the
injected local token driving a real WebSocket through auth.ok to a session
list.

Reported by @yl-dev-tmtc, who also identified the cause and the `sep` fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@saucam
saucam merged commit b5d5150 into main Sep 5, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web UI returns 403 Forbidden on Windows due to path separator check

2 participants