diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 13988cbca1..bec95c95f9 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -147,7 +147,7 @@ export const serveCommand: CommandModule = { type: 'boolean', default: false, description: - 'Open the Web Shell in a browser once the daemon is listening. No-op with --no-web or in headless/CI/SSH environments.', + 'Open the Web Shell in a browser once the daemon is listening. With a token configured, the launch URL (token included) is handed to the browser launcher and is visible in the process list, so prefer opening the URL manually on multi-user hosts. No-op with --no-web, when the UI assets are absent, or in headless/CI/SSH environments.', }) .option('event-ring-size', { type: 'number', @@ -474,15 +474,48 @@ export const serveCommand: CommandModule = { ...(rateLimitRead !== undefined ? { rateLimitRead } : {}), ...(rateLimitWindowMs !== undefined ? { rateLimitWindowMs } : {}), }); - // Open the Web Shell in a browser once the listener is up. `handle.url` - // already reflects the resolved port (so `--port 0` works), and - // `shouldLaunchBrowser()` suppresses this in CI / SSH / headless. Best - // effort: a failed launch must not take down the daemon. - if (argv.open && argv.web && shouldLaunchBrowser()) { - const target = new URL(handle.url); - const browserToken = argv.token || process.env['QWEN_SERVER_TOKEN']; - if (browserToken) target.searchParams.set('token', browserToken); - await openBrowserSecurely(target.toString()); + // Open the Web Shell in a browser once the listener is up. Gated on + // `handle.webShellMounted` so we never point a browser at an API-only + // daemon (assets absent or --no-web). `shouldLaunchBrowser()` suppresses + // this in CI / SSH / headless. The whole block is wrapped in its own + // try/catch: openBrowserSecurely can throw (URL validation, unsupported + // platform), and the outer catch calls process.exit(1) — a failed + // browser launch must NOT take down the already-listening daemon. + if (argv.open && handle.webShellMounted && shouldLaunchBrowser()) { + try { + const target = new URL(handle.url); + // `handle.url` carries the raw bind host; a wildcard bind is not + // client-addressable, so send the browser to loopback instead. + if ( + target.hostname === '0.0.0.0' || + target.hostname === '::' || + target.hostname === '[::]' + ) { + target.hostname = '127.0.0.1'; + } + // Trim to match runQwenServe's own token normalization — otherwise a + // trailing newline from `$(cat token.txt)` makes the browser's token + // mismatch the server's trimmed value and 401 every API call. + const rawBrowserToken = + argv.token || process.env['QWEN_SERVER_TOKEN']; + const browserToken = + typeof rawBrowserToken === 'string' && + rawBrowserToken.trim().length > 0 + ? rawBrowserToken.trim() + : undefined; + if (browserToken) { + target.searchParams.set('token', browserToken); + writeStderrLine( + 'qwen serve: --open passes the token in the browser launch command ' + + '(visible via `ps` / /proc); on a multi-user host open the URL manually instead.', + ); + } + await openBrowserSecurely(target.toString()); + } catch (browserErr) { + writeStderrLine( + `qwen serve: failed to open browser: ${browserErr instanceof Error ? browserErr.message : String(browserErr)}`, + ); + } } } catch (err) { writeStderrLine( diff --git a/packages/cli/src/serve/runQwenServe.ts b/packages/cli/src/serve/runQwenServe.ts index 6a88c9fbbb..375b9c2fe6 100644 --- a/packages/cli/src/serve/runQwenServe.ts +++ b/packages/cli/src/serve/runQwenServe.ts @@ -347,6 +347,12 @@ export interface RunHandle { server: Server; url: string; bridge: AcpSessionBridge; + /** + * Whether the Web Shell UI was actually mounted (assets resolved and + * `serveWebShell !== false`). The `--open` launcher checks this so it never + * points a browser at an API-only daemon. + */ + webShellMounted: boolean; /** Resolves when the listener has fully closed and the bridge is drained. */ close(): Promise; } @@ -993,14 +999,33 @@ export async function runQwenServe( 'qwen serve: Web Shell assets not found; serving API only. ' + 'Build the web-shell workspace (npm run build) or pass --no-web to silence this.', ); - } else if (!isLoopbackBind(opts.hostname)) { - writeStderrLine( - 'qwen serve: Web Shell UI is served WITHOUT auth on a non-loopback ' + - 'bind (the static shell has no secrets; the API stays token-gated). ' + - 'Pass --no-web to disable the UI.', - ); + } else { + // Positive happy-path breadcrumb so operators can confirm the UI is live + // (the only other lines are negative-path warnings). + writeStderrLine(`qwen serve: Web Shell UI served from ${webShellDir}`); + if (!isLoopbackBind(opts.hostname)) { + writeStderrLine( + 'qwen serve: Web Shell UI is served WITHOUT auth on a non-loopback ' + + 'bind (the static shell has no secrets; the API stays token-gated). ' + + 'Pass --no-web to disable the UI.', + ); + // The shell HTML/JS loads (GET carries no Origin), but its same-origin + // POSTs (create session, prompt, permission vote) send an Origin the + // daemon's CORS wall rejects with 403 unless allow-listed — so without + // --allow-origin the UI is effectively read-only on a non-loopback + // bind. Front the daemon with a same-origin reverse proxy, or pass + // --allow-origin , to make mutations work. + if (!opts.allowOrigins || opts.allowOrigins.length === 0) { + writeStderrLine( + 'qwen serve: without --allow-origin the Web Shell is read-only on a ' + + 'non-loopback bind — same-origin POSTs are blocked by CORS (403). ' + + 'Pass --allow-origin or front it with a same-origin proxy.', + ); + } + } } } + const webShellMounted = !!webShellDir && opts.serveWebShell !== false; // Pass the already-canonical `boundWorkspace` into `createServeApp` // via `deps.boundWorkspace`. That field is the pre-canonicalized @@ -1227,6 +1252,7 @@ export async function runQwenServe( server, url, bridge, + webShellMounted, close: () => { // Idempotent: cache the in-flight (or settled) close promise so // overlapping calls (e.g. test harness + signal handler firing diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 60f21d6bd9..616b51e8a9 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { realpathSync, promises as fsp } from 'node:fs'; +import { existsSync, realpathSync, promises as fsp } from 'node:fs'; import type { ServerResponse } from 'node:http'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -19,6 +19,7 @@ import { resolvePromptDeadlineMs, } from './server.js'; import { runQwenServe, type RunHandle } from './runQwenServe.js'; +import { resolveWebShellDir, isDocumentNavigation } from './webShellStatic.js'; import { CONDITIONAL_SERVE_FEATURES, getAdvertisedServeFeatures, @@ -1703,6 +1704,68 @@ describe('createServeApp', () => { .set('Accept', 'text/html'); expect(res.text).not.toContain('
'); }); + + it('does not serve the shell for POST navigations (method guard)', async () => { + const app = createServeApp(baseOpts, undefined, { webShellDir }); + const res = await request(app) + .post('/session/abc') + .set('Host', host) + .set('Accept', 'text/html'); + expect(res.text).not.toContain('
'); + }); + + it('falls back to the shell on a sec-fetch navigation signal', async () => { + const app = createServeApp(baseOpts, undefined, { webShellDir }); + const res = await request(app) + .get('/session/deep') + .set('Host', host) + .set('Accept', '*/*') + .set('Sec-Fetch-Mode', 'navigate'); + expect(res.status).toBe(200); + expect(res.text).toContain('
'); + }); + + it('does not shadow /health on a browser navigation (Critical #1)', async () => { + // Non-loopback + requireAuth registers /health POST-auth. A browser + // navigation (Accept text/html) must fall THROUGH the SPA fallback to + // bearerAuth (401), not receive the shell. Without the /health guard the + // pre-auth fallback would return index.html instead. + const app = createServeApp( + { ...baseOpts, hostname: '0.0.0.0', token: 'secret', requireAuth: true }, + undefined, + { webShellDir }, + ); + const res = await request(app) + .get('/health') + .set('Host', '0.0.0.0:4170') + .set('Accept', 'text/html'); + expect(res.text).not.toContain('
'); + }); + + it('returns 500 when index.html is unreadable after mount', async () => { + const app = createServeApp(baseOpts, undefined, { webShellDir }); + await fsp.rm(path.join(webShellDir, 'index.html')); + const res = await request(app).get('/').set('Host', host); + expect(res.status).toBe(500); + }); + + it('isDocumentNavigation recognizes each navigation signal', () => { + const nav = (headers: Record) => + isDocumentNavigation({ headers } as never); + expect(nav({ 'sec-fetch-mode': 'navigate' })).toBe(true); + expect(nav({ 'sec-fetch-dest': 'document' })).toBe(true); + expect(nav({ accept: 'text/html,application/xhtml+xml' })).toBe(true); + expect(nav({ accept: 'application/json' })).toBe(false); + expect(nav({})).toBe(false); + }); + + it('resolveWebShellDir returns undefined or a dir with index.html + assets', () => { + const dir = resolveWebShellDir(); + if (dir !== undefined) { + expect(existsSync(path.join(dir, 'index.html'))).toBe(true); + expect(existsSync(path.join(dir, 'assets'))).toBe(true); + } + }); }); describe('GET /health', () => { diff --git a/packages/cli/src/serve/webShellStatic.ts b/packages/cli/src/serve/webShellStatic.ts index 14bf431445..106fced7fd 100644 --- a/packages/cli/src/serve/webShellStatic.ts +++ b/packages/cli/src/serve/webShellStatic.ts @@ -53,7 +53,16 @@ export function resolveWebShellDir(): string | undefined { path.resolve(selfDir, '..', '..', '..', 'web-shell', 'dist'), ]; for (const dir of candidates) { - if (existsSync(path.join(dir, 'index.html'))) return dir; + // Require BOTH index.html and assets/: a partial build (index.html + // without its hashed chunks) would otherwise pass and serve a shell + // whose every script/style 404s. copy_bundle_assets.js applies the same + // two-part check before copying. + if ( + existsSync(path.join(dir, 'index.html')) && + existsSync(path.join(dir, 'assets')) + ) { + return dir; + } } return undefined; } @@ -136,6 +145,13 @@ export function registerWebShell(app: Application, webShellDir: string): void { app.use((req: Request, res: Response, next: NextFunction) => { if (req.method !== 'GET' && req.method !== 'HEAD') return next(); if (!isDocumentNavigation(req)) return next(); + // Don't shadow the daemon's own browser-reachable endpoints. On + // non-loopback binds /health and /demo are registered AFTER bearerAuth + // (i.e. after this pre-auth fallback), so without this guard a browser + // navigation to them would receive index.html instead of their real + // response. API routes send Accept: application/json and already fail + // isDocumentNavigation, so they need no listing here. + if (req.path === '/health' || req.path === '/demo') return next(); sendIndex(res); }); }