Skip to content

fix(dashboard): keep project names and session ids out of inline handlers - #641

Merged
lis186 merged 1 commit into
mainfrom
fix/dashboard-onclick-xss-pr
Sep 27, 2026
Merged

lis186 merged 1 commit into
mainfrom
fix/dashboard-onclick-xss-pr

Conversation

@lis186

@lis186 lis186 commented Sep 27, 2026

Copy link
Copy Markdown
Owner

摘要

修正 dashboard 的 DOM XSS。

  • 問題: 專案列和星號按鈕把專案名稱(來自 cwd 的目錄名)與 session id 直接拼進 inline onclick,而且只把 " 換成 "。名稱裡如果本來就有字面的 ",瀏覽器解碼屬性後就會跳出 JS 字串、執行任意程式碼。fix(store): record cwd for live Claude Code 2.1.283 proxy traffic #640 讓即時流量開始記錄 cwd 之後,這條路徑更容易被觸發。
  • 修正: 沿用 repo 既有的 data-* 寫法,把這些值移出 inline handler。
  • 稽核: 已檢查 public/ 所有 inline handler,其他地方都安全。

Details

  • Root cause: JSON.stringify(value).replace(/"/g, '"') escaped quotes but not &. The browser HTML-decodes an attribute before running it as JS, so a literal " in the data became a real " and closed the JS string.
  • Reproduced before the fix (53558f1) on both paths, using only a global counter as the payload effect:
    • proxy: a real directory named p");window.__ccxrayXss=…;(" used as the request cwd
    • import: synthetic transcripts, viewed under /?imported
    • Clicking the project row, the project star, and a session star with a crafted metadata.session_id each ran the payload.
  • Fix: follows the existing data-sid / data-resume convention. Values go into data-* attributes via escapeHtml, and handlers become fixed code reading this.dataset:
    • project row: data-project
    • renderStarBadge's 6 handlers (project and session stars, derived-star counts): data-level / data-star-id
    • keyboard-nav.js now reads data-project instead of parsing the row's onclick. This also fixes names that JSON.parse could not recover, which keyboard navigation used to skip.
  • Audit of public/ (full table in the review artifacts):
    • Every other inline handler interpolates only constants, numeric indices, ccxray-generated ids restricted to [0-9T-], 12-hex coreHashes, or server-derived agent keys restricted to [a-z0-9-].
    • There are no javascript: URLs, eval, new Function or setAttribute('on…'). An independent enumeration (85 literal handlers, 12 property assignments) found nothing missed.
  • Tests: a new test/dashboard-xss-e2e.test.js (puppeteer, same pattern as dashboard-codex-e2e). It covers the project row, project star, session star, derived-star count and its popover, and keyboard ↑/↓. Each asserts three things: the payload counter stays 0, selection and stars record the full original name (including /_api/stars), and state actually changes after the click. Every clicked element is asserted to exist first.

Verification

  • CCXRAY_HOME=$(mktemp -d) CCXRAY_EXPORT_DISABLE=1 npm test: 2652/2652. The real ~/.ccxray was untouched during the run.
  • Fail-on-old / pass-on-new: test/dashboard-xss-e2e.test.js with the pre-fix public/miller-columns.js and public/keyboard-nav.js fails 5 of 6; with the fix, 6/6 pass.
  • Browser smoke on an isolated --port 5602 server (temp CCXRAY_HOME and HOME), with headless Chrome via puppeteer (CDP):
    • project selection, session star, derived-star popover and keyboard navigation work
    • the payload counter stayed 0 and there were no page errors
  • Independent review (agentflow security-scan + acceptance, Claude-family reviewers): no high findings; Outcome, Minimality and Conformance all PASS.

Not verified:

  • Browser tooling: the smoke used puppeteer rather than the browser-harness tool named in CLAUDE.md. It is the same CDP transport.
  • Follow-up, not in this PR: messages.js dlScript is currently safe only because agent keys are restricted to [a-z0-9-]. It puts an escapeHtml'd value inside a single-quoted JS string, which would become injectable if that restriction were relaxed. Queued as a separate hardening change.

🤖 Generated with Claude Code

…lers

The project row and the star badge built their onclick source with
JSON.stringify(value).replace(/"/g, '"'). JSON.stringify does not escape
`&`, and the browser decodes the attribute before compiling it as JS, so a
literal `"` in the data became a real quote and escaped the string: a
directory named `p");alert(1);("` ran code when its project row or
star was clicked. Session ids are just as exposed, because metadata.session_id
is accepted verbatim. #640 made the path easier to reach by recording cwd for
live Claude traffic again.

Both values now travel in data-* attributes (escapeHtml'd), and the handlers
are fixed strings that read this.dataset, following the existing data-sid /
data-resume convention. keyboard-nav.js read project names back by parsing
the onclick source; it now reads data-project, which also fixes arrow-key
navigation skipping a project whose name JSON.parse could not recover.

An audit of every inline on*= handler under public/ found no other site that
splices a data-controlled string: the rest use constants, numeric indexes,
ccxray-generated entry ids, hex hashes, server-derived agent keys, or the
data-* pattern already. There are no javascript: URLs.

New puppeteer e2e (test/dashboard-xss-e2e.test.js) clicks the project row,
the project star, the session star and the derived-star chip for a payload
name and session id, and checks keyboard navigation. Old code: 5 of 6 fail;
new code: all pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lis186
lis186 merged commit 685321f into main Sep 27, 2026
3 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.

1 participant