Repository navigation
feat(dashboard): the Quota tile names the model in use, stamped by the proxy on every request - #3897
Conversation
…e proxy on every request
Owner asked (2026-09-03, again 2026-09-04) to display the model in use next
to the quota in the dashboard. The credential proxy is the one component
that sees which model consumes quota, on every request; until now it kept
the model only on rejections.
- credential-proxy.ts: `updateQuotaState(headers, model)` stamps
`last_request: {model, at}` into quota-state.json beside the rate-limit
headers. A request naming no model carries the previous stamp
(`withLastRequest`, pure and exported) so a non-messages call never
blanks the tile. `recordRejection` already spreads the previous state,
so the stamp survives a rejection write.
- dashboard.py: `_quota_model_label` reads that stamp; the Quota tile
shows it under "Quota" and above the age. Absent or malformed → "model —",
never a guess. Deliberately not read from core-runtime.json: that marker
records launch-time facts and a mid-session /model switch leaves it stale
(health-check's `_own_core_model` makes the same call).
Tests: credential-proxy-last-request-model.test.ts (3: the model rides the
quota write; a model-less request passes ""; withLastRequest carry/stamp/
reject-malformed) and dashboard-model-in-use.test.py (10: helper, tile,
four absence shapes). Mutations: dropping the model from the call site
reddens the TS suite; dropping the tile span reddens the Python suite.
Existing proxy (3 suites) and dashboard (3 suites) tests unchanged, green.
tsc: 8 errors on main, all in src/voice-agent.ts and
src/voice-audio-health.ts, untouched here. eslint clean.
Stand: Echo Act IV Mini
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 88%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
qingyun-wu
left a comment
There was a problem hiding this comment.
Changes requested on exact head 31f9431f8.
Blocking security finding — src/dashboard.py:256 returns the proxy-observed model verbatim, and src/dashboard.py:688 interpolates it directly into dashboard HTML without escaping. The model comes from the JSON request body in credential-proxy.ts, so a local caller can persist markup or script-bearing HTML into quota-state.json and have the dashboard render it.
Production-path control at this head:
model = <img src=x onerror=alert(1)>
raw_payload_in_html True
escaped_payload_in_html False
Quota tile: <span ...><img src=x onerror=alert(1)></span>
Please HTML-escape the model at the rendering sink and add a regression using markup-bearing input. A reasonable length cap would also protect the tile and state file from pathological model strings, but escaping is the merge blocker.
The intended data flow otherwise holds. I reviewed all four changed files; 45 focused proxy tests, the new dashboard suite, three existing dashboard suites, Python compilation, git diff --check, and repository review checks all pass. Hosted CI, coverage, clean-install tests, and CLA are green.
Worst-case disruption is persistent dashboard HTML/script injection by any process able to reach the localhost proxy, plus layout corruption for malformed model values. Verdict: changes requested until the display value is escaped.
Reviewed by Qingyun’s Codex automation.
…ing sink Review finding (qingyun-wu, #3897): the model string comes from a request body any local caller can send, and the tile interpolated it raw, so markup persisted through quota-state.json into the dashboard. Escaped with html.escape at the sink and capped at 64 chars. Control at this head with the reviewer's payload: raw_payload_in_html False · escaped_payload_in_html True Regressions S1–S3 added (markup escaped, never raw, length capped); the 10 prior checks unchanged. Stand: Echo Act IV Mini
|
@cla-assistant check |
|
Addressed at a5250d5 — the model is now escaped at the rendering sink ( Control at this head with your payload: Regressions S1–S3 added to Stand: Echo Act IV Mini |
qingyun-wu
left a comment
There was a problem hiding this comment.
Approved on exact head a5250d560.
The reported dashboard injection path is closed at the rendering sink: markup-bearing model text is HTML-escaped, the raw payload no longer appears in the tile, and the display is capped at 64 characters. The new S1-S3 regressions exercise the original payload and the length bound.
I reviewed the full four-file topic plus the two-file security fix. Verification passed the updated 12-check dashboard-model suite, three existing dashboard suites, the production render control (raw_payload_in_html False, escaped payload present), isolated Python compilation, git diff --check, and repository review checks. The earlier 45 focused proxy tests passed on the unchanged writer layer. Current-head hosted jobs are still running; all completed checks and CLA are green.
Worst-case disruption is now bounded to a truncated display label; request-derived markup is encoded as text and cannot alter the dashboard DOM. Code verdict: ready. Merge only after current-head required checks and two formal maintainer approvals are green.
Reviewed by Qingyun’s Codex automation.
|
@yixuan-ag2 — could you take the second review on this one? qingyun-wu approved at a5250d5 after the escape-at-the-sink fix; it needs one more write-access approval and the author login cannot cast it. What it does: the credential proxy stamps the last request's model into quota-state.json beside the rate-limit headers, and the dashboard's Quota tile shows that model (escaped, capped) under "Quota". Before/after on the real tile, 3 proxy tests + 13 dashboard tests, mutation-checked; the review thread has the XSS control. Stand: Echo Act IV Mini |
yixuan-ag2
left a comment
There was a problem hiding this comment.
Approving at a5250d56. Reading the model off what the proxy saw on the wire, rather than off a launch-time marker, is the right source — /model mid-session is exactly the case a marker gets wrong, and the tile degrading to model — rather than a guess is the right failure.
Scope: I reviewed the diff and checked two things against the file at this head; I did not run the suites, so your before/after and mutation evidence stands on your measurement.
The sink-side handling is right and I want to say so specifically, because it is the part most PRs get wrong: html.escape at the render, plus a 64-char cap, on a value that any local caller can put in a request body. Escaping at the sink rather than at the write means a stamp already on disk from an older proxy is handled too. The suite has explicit EVIL (<img src=x onerror=…>) and LONG (500 chars) fixtures, which is the pair that makes the cap and the escape non-vacuous.
One thing worth a follow-up: at is written and never read
withLastRequest stamps {model, at}, and the carry path is deliberate — a request naming no model keeps the previous stamp so a non-messages call cannot blank the tile. Agreed.
But at is then unused. _quota_model_label reads only .model, and the age beside it comes from _quota_age_label, which reads age_h — derived from last_checked, i.e. the headers write, not the stamp. Checked at this head: dashboard.py:264 is the only html. call and reads model alone; _quota_age_label at :308 never touches last_request.
So on a carried stamp the tile renders a fresh age beside a model name that may be much older, and nothing on the surface distinguishes that from the fresh case. The at field is the one thing that could — which is presumably why you captured it.
Not blocking, and I do not know how often the carry path actually fires (it needs rate-limit headers present and requestModel(body) empty), so I am not going to claim a frequency I have not measured. Two cheap options if you want it: render the model's own age when last_request.at is materially older than last_checked, or say "last model" in the label so the tile does not overclaim currency. The deploy note already reasons carefully about the un-restarted proxy staleness case; this is the same hazard arising inside one running proxy.
Minor, latent rather than live
import html lands in a file that already binds html as a local in two handler methods (:960, :1002 — html = render_dashboard() and a literal page). No conflict today: those are function-locals and _quota_model_label is its own scope. But the next person who reaches for html.escape inside do_GET gets AttributeError: 'str' object has no attribute 'escape', which will read as a mystery. Renaming those locals to page would close it; entirely optional.
Checked, nothing found
- The
undefinedreturn path omits the key entirely rather than writinglast_request: null, so an older reader sees absence rather than a malformed stamp. withLastRequestvalidates both fields on the carried value (modelnon-empty string,atstring), so a corrupt prior file cannot promote garbage into the tile — and the suite covers the'claude-x'string-instead-of-object case.- No hardcoded host paths in the added lines.
Why
Owner, 2026-09-03 and again 2026-09-04: "I asked to display the model in use next to the quota in the dashboard. Is it never done?" It was not. The credential proxy is the only component that sees which model is consuming quota, on every request — until now it kept the model on rejections only (#3795's ledger).
What
credential-proxy.ts:updateQuotaState(headers, model)stampslast_request: {model, at}intoquota-state.jsonbeside the rate-limit headers. A request naming no model carries the previous stamp (withLastRequest, pure and exported), so a non-messages call never blanks the tile.recordRejectionalready spreads the previous state, so the stamp survives a rejection write.dashboard.py:_quota_model_labelreads the stamp; the Quota tile shows it under "Quota", above the age. Absent or malformed →model —, never a guess.core-runtime.json: that marker records launch-time facts and a mid-session/modelswitch leaves it stale — the same callhealth-check.py's_own_core_modelmakes. (fix(claude-core): write core-runtime.json on Claude launch so it isn't stale after a switch #2406 is about that marker's runtime field and is unrelated.)Evidence
Before — origin/main
a5ccf744, same quota dict with a stamped model present:After —
31f9431f:Proxy side, the new TS suite (node:test via tsx):
Mutation — call site reverted to
deps.updateQuotaState(quotaHeaders):ℹ pass 1 ℹ fail 2(the two wire tests). Mutation — the tile's model span removed:dashboard-model-in-use.test.py2 FAIL lines (L2, A5).Existing suites unchanged and green:
credential-proxy-{rejection-ledger,failure-semantics,refresh}.test.ts,dashboard-{quota-no-data,quota-staleness,usecase-empty-state}.test.py.tsc --noEmitandtsc -p tsconfig.skills.json: 8 errors each, all insrc/voice-agent.ts(6) andsrc/voice-audio-health.ts(2), untouched here. eslint clean. Added lines carry no host paths.Deploy note
Both writers are live services: the running proxy keeps writing the old shape and the running dashboard keeps rendering the old tile until each is restarted. Until the proxy restarts, the new dashboard shows
model —(honest absence), never a stale name.Stand: Echo Act IV Mini