feat(serve): add /demo debug page for qwen serve daemon#4132
Merged
Conversation
Add a self-contained HTML debug page at GET /demo that provides a browser-based UI for exercising all daemon routes: session create/attach, prompt send/cancel, SSE event streaming, model switching, permission voting, and health/capabilities checks. Also add a same-origin exemption middleware (before the CORS deny layer) so browser fetch calls from the demo page pass through while external Origins remain blocked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
requested changes
May 14, 2026
wenshao
requested changes
May 14, 2026
wenshao
requested changes
May 14, 2026
- Fix XSS: build permission buttons with DOM APIs instead of innerHTML - Fix SSE: move currentEvent outside read loop for cross-chunk frames - Fix SSE: handle stream end (flush trailing buffer, update UI status) - Security: move /demo route behind hostAllowlist and bearerAuth guards - Security: add host.docker.internal to same-origin Origin allowlist - Add Auth Token input and include Authorization header in API/SSE calls - Add try/catch to /demo route handler with writeStderrLine logging - Check API result before removing permission card from UI - Add 7 tests for /demo route and Origin-stripping middleware
Collaborator
Author
|
@wenshao 感谢 review，所有反馈已在 85c9d42 中修复。逐条说明： Critical 修复
Suggestion 修复
全部 78 个测试通过。 |
wenshao
requested changes
May 14, 2026
wenshao
reviewed
May 14, 2026
wenshao
requested changes
May 14, 2026
Browsers cannot attach Authorization headers on address-bar navigation, so /demo behind bearerAuth was unreachable when --token was set. Move the /demo route after CORS + Host allowlist but before bearerAuth. The static HTML shell contains no secrets; all API/SSE routes remain bearer-protected and the in-page token input authenticates them.
jifeng
commented
May 15, 2026
jifeng
left a comment
Collaborator
Author
There was a problem hiding this comment.
Addressed the [Critical] /demo behind bearerAuth is unreachable issue in 92a77b9:
Fix: /demo is now registered AFTER CORS + Host allowlist but BEFORE bearerAuth. The static HTML shell contains no secrets — it only serves the page skeleton with the token input field. All daemon API/SSE routes (/session, /prompt, /events, etc.) remain behind bearerAuth, so the in-page token field authenticates subsequent calls.
Tests updated:
- Old "401 without token" test → now verifies
/demoreturns 200 even when--tokenis set (browser address-bar reachability) - New test confirms CORS + Host allowlist still guard
/demo(cross-origin requests get 403) - Removed the duplicate
/demoregistration that was afterbearerAuth
wenshao
requested changes
May 15, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Additional findings (not tied to a diff line):
packages/cli/src/serve/demo.ts— No structural guard preventing future secrets ingetDemoHtml(). The comment at server.ts:101 says "the page itself contains no secrets", but there is nothing preventing someone from embedding sensitive data in the HTML string later. Add aSECURITY:comment abovegetDemoHtml()noting this HTML is served via an unauthenticated endpoint.packages/cli/src/serve/demo.ts(checkHealth) — On non-loopback binds with--token, the demo page's initialcheckHealth()call gets 401 (non-loopback/healthis behindbearerAuth), showing a misleading "Disconnected" status before the user has a chance to enter the token.
When an API call returns 401 Unauthorized, highlight the Auth Token input field with a yellow border and display a hint message guiding the user to enter their bearer token. Applies to both API calls and SSE connections. The hint auto-dismisses after 6 seconds.
wenshao
requested changes
May 15, 2026
46 tasks
wenshao
requested changes
May 16, 2026
This was referenced May 16, 2026
- Loopback-gate /demo like /health: pre-auth on loopback, post-auth on non-loopback. Prevents unauthenticated access on public interfaces. - Add X-Frame-Options: DENY + CSP frame-ancestors to /demo response to prevent clickjacking via cross-origin iframe embedding. - Cache selfOrigins Set in Origin-stripping middleware (rebuild only when port changes) instead of allocating per-request. - Clear pendingPerms + reset currentAssistantBubble on session switch to prevent stale permission cards from the previous session. - Update tests: loopback vs non-loopback /demo auth, anti-clickjacking headers, rename CORS test, add localhost and [::1] origin coverage.
jifeng
commented
May 18, 2026
jifeng
left a comment
Collaborator
Author
There was a problem hiding this comment.
Addressed all round-2 CR feedback in fef5aba:
Critical fixes:
- Loopback-gate
/demo— Now mirrors the/healthpattern: loopback → beforebearerAuth, non-loopback → afterbearerAuth.--hostname 0.0.0.0 --token secretnow correctly returns 401 for unauthenticated/demorequests. - Clickjacking prevention —
/demoresponse now includesX-Frame-Options: DENYandContent-Security-Policy: ... frame-ancestors 'none'to block cross-origin iframe embedding. - Stale permissions on session switch —
createSession()now clearspendingPerms, resetscurrentAssistantBubble, and re-renders the permission panel.
Suggestions addressed:
4. Origin-stripping caching — selfOrigins Set is now lazily cached and only rebuilt when the port changes (mirrors hostAllowlist's pattern).
5. host.docker.internal trust boundary — Partially mitigated by the loopback gating; documented in existing comments.
6. Test updates — Loopback vs non-loopback /demo auth tests, anti-clickjacking header assertions, CORS test renamed, localhost and [::1] origin-stripping coverage added (83 tests total, all pass).
Collaborator
Author
Round-2 CR Replies (fef5aba)[Critical]
|
wenshao
reviewed
May 18, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Round-3 review (mimo-v2.5-pro) — fef5aba0c3
All round-2 CR feedback has been addressed. The implementation now correctly mirrors the /health loopback-gating pattern, includes anti-clickjacking headers (X-Frame-Options + CSP frame-ancestors), uses DOM APIs for permission buttons, and has comprehensive test coverage. No blocking issues remain.
Remaining suggestions (non-blocking):
-
addLogXSS viatag—demo.ts:208—tagandtagClassare concatenated intoinnerHTMLunescaped. If an SSE event type contained<img onerror=...>, it would execute. Suggest escaping viaescHtml()or switching to DOM APIs. -
SSE error path missing UI update —
demo.ts:335— Whenfetchreturns non-OK (e.g., 401), the code logs and returns early without callingenablePrompt(false)or updatingstatusDot/statusText. The normal disconnect path (line 372) correctly disables controls; the error path should too. -
enablePromptdoes not disablepromptInput—demo.ts:312— The sidebar textarea has its ownkeydown→sendPromptbinding but is not disabled byenablePrompt(false), so users can still submit prompts when controls should be off. -
textContent +=O(n²) —demo.ts:453— Each SSE chunk reads the full accumulated string, concatenates, and re-sets. For long responses with hundreds of chunks, this is quadratic. UsecreateTextNode+appendChildinstead. -
createSessiondoes not validatesessionId—demo.ts:295— If the server returns{ ok: true }withoutsessionId, the variable becomesundefinedand subsequent URL constructions produce/session/undefined/events. -
No concurrent-send guard —
demo.ts:497— The Send button stays enabled during prompt execution. Rapid clicks create interleaved SSE responses. -
_portparameter unused —demo.ts:12— The HTML template uses relative URLs (BASE = '') and never references_port. Either remove the parameter or use it. -
Permission card missing context —
demo.ts:510— Shows onlyreq.tool?.name+ request id. Omits the command/input/diff needed for informed allow/deny. -
Log container never prunes —
demo.ts:205— Long sessions grow DOM nodes without bound. Consider capping at ~1000 entries. -
Test gaps for
localhost/[::1]—server.test.ts:1518— Origin-stripping tests cover127.0.0.1andhost.docker.internalbut notlocalhostor[::1], which are distinct code paths.
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
wenshao
reviewed
May 18, 2026
# Conflicts: # packages/cli/src/serve/server.ts
wenshao
requested changes
May 18, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Test coverage gaps (no single diff line — listed here):
demoHandlertry/catch error path is untested. IfgetDemoHtml()ever throws, the 500 error response format is unverified.- CORS 403 and auth 401 test assertions only check status codes, not response bodies. Other tests in the same file assert
res.bodyfor equivalent middleware (e.g.,{ error: 'Request denied by CORS policy' }). - Host allowlist enforcement on
/demois untested. No test sends a non-allowlistedHostheader to verify the 403 rejection.
— DeepSeek/deepseek-v4-pro via Qwen Code /review
wenshao
requested changes
May 18, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
本地 159/159 server.test.ts 跑通，但 server.ts 与 main 上 #4250/#4247/#4251 有冲突需要 rebase。
更紧的是 L208 那条 [Critical] XSS —— 我对照 logEvent 调用点确认 tag 在 :408 和 :424 两处是 daemon-controlled（update.sessionUpdate.kind / event.type 都从 SSE 上行），所以恶意/被入侵 daemon 推一帧 {"sessionUpdate":{"kind":"<img src=x onerror=alert(1)>"}} 就在 demo 页面里跑任意 JS。本地 daemon 信任域虽然窄，但接错 URL / MITM / 攻击者控制的本地进程都成立。
下面贴出修法 + 标出两个 daemon-controlled 调用点。rebase 之后这条 + 04:29 review body 里 3 个 test gap 一起收掉就可以 approve。
- Replace innerHTML with DOM APIs in addLog() to prevent XSS - Add MAX_LOG_ENTRIES=500 pruning to prevent unbounded DOM growth - Add concurrent-send guard (promptInFlight) to prevent double submits - Show error feedback in Chat tab when sendPrompt or createSession fails - Disable prompt controls on SSE failure (both catch and non-OK paths) - Add toolCall context detail (command/input/path/diff) to permission cards
Collaborator
Author
CR Feedback Status — All RoundsBelow is the status of every review comment across all rounds. Fixes have been pushed in commits Round 1 (addressed in earlier commits)
Round 2 (addressed in
|
| Comment ID | Status | Summary |
|---|---|---|
| 3240390480 | ✅ Fixed | /demo behind bearerAuth unreachable → loopback-gated pre-auth (mirrors /health pattern) |
| 3240390487 | ✅ By design | Origin-stripping vs Host comparison — loopback-only origins are intentionally a small safe set |
| 3240390489 | ✅ Fixed (17e5360e3) |
addLog unbounded DOM growth → MAX_LOG_ENTRIES=500 pruning |
| 3240499582 | ✅ Fixed (17e5360e3) |
addLog innerHTML XSS → rewritten with DOM APIs |
| 3240499590 | ✅ Fixed (17e5360e3) |
createSession no error feedback → shows error in Chat + 401 token hint |
| 3240499595 | ✅ Fixed (66930a6c9) |
Test gaps → added localhost/[::1] origin tests, renamed CORS test |
Round 3 (addressed in 66930a6c9 and 17e5360e3)
| Comment ID | Status | Summary |
|---|---|---|
| 3243137149 | ✅ Fixed | createSession doesn't reset pendingPerms/currentAssistantBubble → added clearing |
| 3243137163 | ✅ Fixed (17e5360e3) |
SSE catch path missing enablePrompt(false) → added |
| 3243137169 | ✅ Fixed (17e5360e3) |
sendPrompt error feedback → shows error in Chat tab on failure |
| 3243137173 | ✅ Fixed (17e5360e3) |
SSE non-OK missing status/disable → updates dot/text + enablePrompt(false) |
| 3243137184 | ✅ Fixed (17e5360e3) |
No concurrent-send guard → added promptInFlight flag |
Round 4 (addressed in 66930a6c9 and 17e5360e3)
| Comment ID | Status | Summary |
|---|---|---|
| 3246264204 | ✅ Fixed | /demo unconditional pre-auth → uses exposeHealthPreAuth condition (same as /health) |
| 3246264213 | ✅ Fixed | Origin Set per-request allocation → lazy cached Set, rebuilt only on port change |
| 3246264216 | ✅ Fixed | host.docker.internal in selfOrigins → removed |
| 3246264218 | ✅ Fixed | Test binds 0.0.0.0 but supertest sends to loopback → test now uses hostname:'127.0.0.1' + token:'secret' to test non-loopback path properly |
| 3246264219 | ✅ Fixed | Misleading CORS test name → renamed to "is guarded by CORS (rejects cross-origin requests)" |
| 3246264222 | ✅ Fixed | Missing localhost/[::1] origin tests → added |
| 3246898296 | ✅ Fixed | createSession doesn't clear pendingPerms → added .clear() + renderPermissions() |
| 3251740250 | ✅ Fixed | Clickjacking → added X-Frame-Options: DENY + CSP frame-ancestors 'none' |
New in 17e5360e3
| Comment ID | Status | Summary |
|---|---|---|
| 3240390482 | ✅ Fixed | Permission card lacks toolCall context → now shows command/input/path/diff in a <pre> block |
All review comments have been addressed. Ready for re-review.
Keep both origin-stripping middleware (demo page) and new WorkspaceFileSystemFactory + createDefaultFsAuditEmit from #4250.
Keep demo.ts import alongside new workspaceMemory and workspaceAgents imports.
wenshao
approved these changes
May 18, 2026
Collaborator
Author
CI Failure AnalysisThe CI failures on this PR are pre-existing issues on the Lint (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a self-contained
/demodebug page to theqwen servedaemon. This page provides a browser-based UI for exercising all daemon routes without requiring any external tools or build steps.What's included
packages/cli/src/serve/demo.ts— inline HTML generator for the/demopagepackages/cli/src/serve/server.ts— registers the/demoroute and same-origin CORS workaroundFeatures of the demo page
/health,/health?deep=1, and/capabilities--token-protected deploymentsDesign notes
Originheader when it matches the daemon's own loopback address (includinghost.docker.internalfor Docker Desktop), allowing the demo page's API calls to pass through while maintaining the existingdenyBrowserOriginCorsprotection for non-loopback origins/demoroute is registered after CORS, Host allowlist, and bearer auth middleware, so it is protected by the same security guards as all other routesCR feedback addressed (commit
85c9d42)innerHTMLwith DOM APIs (createElement,textContent,dataset)currentEventscopewhileloop so frames split across chunks are handled correctly/demobypasses authhostAllowlistandbearerAuthmiddlewareAuthorization: Bearerheader inapi()and SSEfetchhost.docker.internal/demomissing error handlinggetDemoHtml(getPort())in try/catch withwriteStderrLineremovePermission()only called after checkingresult.ok; errors logged/demoroute (200/401/token) and Origin-stripping middlewareReviewer Test Plan
qwen serveand openhttp://localhost:<port>/demoin a browser--tokenprotected deployments: enter the token in the Auth Token field and verify API calls succeed202605141033.mp4