fix(workspace): Reports tab renders through the shared display_hint renderers, never a raw dump (#2162) - #2173
Conversation
FR-5 enumerated Agent Detail and the Operations fleet tab while a third surface — the Workspace agent page — shipped a raw JSON dump to external clients. Enumerating the surfaces is load-bearing, so the list is corrected first: the omission is the hole the bug fell through. Also records the two things the surfaces deliberately do NOT share: the fallback is a bounded, humanised, redacted key-value summary (never a raw dump), and the Workspace passes allow-raw=false so no raw payload is reachable behind it. FR-5a records the client-facing row window as two optional query params on the EXISTING portal detail route rather than a second route a portal principal could not reach anyway (#2128 class). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…it whole (#2162) Two optional query params on the EXISTING portal detail route, not a second route. The operator row reader (#1537) is Depends(get_current_user), which a portal principal — a verified email with no users row — structurally cannot satisfy; cloning it on a client-facing prefix would mean a second hand-written gate beside the uniform-404 one, and that is how those two answers drift apart. The server decides tabularity from the REAL payload, so the client never guesses: display_hint is agent-authored and can disagree with what was filed, and a client-side prediction would need a 400 plus a recovery re-fetch. A non-tabular payload with rows_limit set comes back whole with no row_meta. rows_limit absent is byte-identical to today. Rate-limited after the roster gate: paging re-reads the whole (<=5 MiB) blob per request, so the route that cuts transfer raises reads — acceptable behind a JWT, an amplification primitive on a prefix a client can loop. Also fixes two stale "256 KB" comments; the cap has been 5 MiB since #1537. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mp them (#2162) The Reports tab shipped JSON.stringify(payload) to external clients. That is a disclosure defect, not only an ugly one: agent_page.py already refuses to expose an ask's `context` because free-form agent JSON has been a credential-leak surface (canary G-04), and `payload` is the same category. A typed renderer reads only the keys its hint declares, so routing the tab through the shared components/reports/ set strictly narrows what crosses. The fallback could not be reused as-is, because the shared fallback IS the raw JSON viewer. ReportSummary replaces it on EVERY surface — bounded (<=40 entries, truncated values, depth 1), humanised, with credential-shaped tokens redacted at value level anywhere in the string. Raw JSON moves one click behind a disclosure for operators; `allow-raw="false"` removes even that here. The prop names the POLICY rather than a mechanism, so it covers both a shape mismatch and an agent that deliberately set display_hint: "json" — a valid value in the MCP enum that a "fallback override" prop or slot would have silently left able to dump. shapeOk and all five payload.X keys stay in ReportRenderer.vue: extracting them is the natural refactor and it empties the set test_1535 regexes out of that exact file, breaking the cross-surface drift guard. Report state moves into the store (contract #21), and every await is generation-guarded. A reset cannot cancel an in-flight promise, so adding the reportsLoaded flag contract #15 requires would have turned today's TRANSIENT wrong-render into a permanent one — the new agent marked loaded-with-the-old- agent's-data for the life of the mount. The component additionally gates every read on the state belonging to the agent on screen, since the store outlives it and a fresh mount fires no props watcher. AC #4 was not free: three reused sites carried bare gray-500 meta text with no dark override, which the contract calls the floor and never meta text. Fixed, plus the set's only raw non-gray (bg-blue-500 -> bg-status-info-500), so the per-file non-gray count only shrinks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
) architecture.md: the renderer set is a THREE-surface contract, which widens the blast radius of the test_1535 pin — so the note that shapeOk must stay in ReportRenderer.vue lives beside it. The portal row window goes in the existing "Large payloads" bullet as two query params, not a second route. workspace-agent-page.md: a new "The Reports tab" section, placed above the #2161 prose. It records that rendering is PRESENTATION — exclusion stays projection-in-the-service, so this does not contradict the doc's own thesis — that the payload remains agent-authored untrusted content of the same class as asks.context, the honest residual a key-value summary leaves, the read amplification the window buys, and why the state moved into the store. agent-reports.md: three panel consumers, not two. Enumerating them is load-bearing — the list said "two" while the third dumped raw JSON at clients. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The finally block cleared its own reportId from _reportInFlight unconditionally, but resetAgentReports already empties that map — so a late finally from the previous agent's request deleted the marker a NEW request had just written, and the next expand issued a duplicate. The guard leaking through its own bookkeeping. Narrow (report ids are global uuids, so the same id under two agents is close to impossible) but the fix is two lines and the failure mode is exactly what the guard exists to prevent. Also pairs LoadFailed's title with a message that says what to DO rather than restating the title (contract #25). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eps JSON (#2162) AC #2 asks for a client fallback "deliberately stricter than the operator side, because the audience is an external client" — that is a request for a SPLIT, not for a stricter default everywhere. Making ReportSummary the fallback on all three surfaces erased exactly the distinction the criterion exists to create, and put an operator-visible behaviour change on a P1 client-facing bug fix. ReportRenderer now takes a `fallbackComponent` prop defaulting to ReportJson, so both operator panels render byte-identically to before; the Workspace passes ReportSummary. A raw dump is a FEATURE when you are debugging an agent's own output and a defect when the reader is that agent's customer. The override catches an agent-chosen `display_hint: "json"` as well as a shape mismatch, which is what the earlier allow-raw boolean was reaching for: `json` is a valid value in the MCP enum, so replacing only the mismatch path would leave an agent able to request a dump in front of a client. ReportSummary therefore loses its raw-JSON disclosure entirely — it is portal-only now, so there is nothing legitimate to disclose behind it, and a source guard pins that it never imports ReportJson. The AC #4 dark-ink fixes to ReportTable/ReportKpiTiles/ReportTimeline stay: theme correctness, no behaviour change, and they ratcheted fleet raw_nongray 810 -> 809. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ibe the superseded fallback design (#2162) Two review findings, both in the "the artefact says something the code does not" class. **1. `reportSummary.spec.js` claimed to pin an ordering it did not pin.** `safeText` redacts before it truncates, and the reason is real: truncating first can slice a credential-shaped token so the surviving head no longer matches, leaving a recognisable fragment on screen under a value that looks handled. The test asserting it used a `ghp_` token — and `\bghp_[A-Za-z0-9]+` needs exactly ONE character after the prefix, so the truncated head still matches and is still replaced. Verified by breaking `safeText` to truncate-then-redact during review: the suite stayed green. Replaced with a case that discriminates. `\bAKIA[A-Z0-9]{4}` needs FOUR characters, so a cut mid-token leaves a head the pattern no longer recognises — here `AKIAIO`, two characters of a real AWS key id. Re-broken against the new assertion: it fails, and passes again on restore. The original `ghp_` case is kept (it is still a valid behavioural assertion) but renamed to say what it actually proves. No source change: the ordering was already correct, only the proof was hollow. **2. Two docs still described the `allow-raw` design that review superseded.** The shipped shape is a `fallbackComponent` prop defaulting to `ReportJson`, with only the client-facing Workspace overriding it. `agent-reports.md` said the fallback IS `ReportSummary` "with ReportJson retained behind its disclosure" — describing a `<details>Show raw JSON</details>` that does not exist, inverting which surface gets which fallback, and contradicting the correct explanation four paragraphs below it in the same file. The architecture bullet was titled "one fallback" over a body whose whole point is that the fallback is per-surface. Both now say what the code does. This matters beyond tidiness: the split IS AC #2 (a client fallback deliberately stricter than the operator side), so a doc asserting a shared fallback is an invitation to "unify" it and re-open the disclosure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Resolve by running |
vybe
left a comment
There was a problem hiding this comment.
Validated via /validate-pr. Merges clean onto dev after #2171 and #2172; combined result verified locally (27 files / 550 frontend tests, 20/20 test_2162_portal_report_window.py, prod build, check:tokens all pass). Backend reviewed: ownership check precedes windowing so the uniform-404 (Invariant #8) survives, rows_offset/rows_limit bounded at both route and service layer, shallow copy leaves the source payload intact, rate limiter placed after _require_roster. Operator surfaces confirmed byte-identical (empty diff on both panels; fallbackComponent defaults null and resolves to ReportJson). No new endpoint/table/column, so Rule #9 dual-track correctly does not engage. Security scan clean — flagged token patterns are all fake fixtures (AWS's documented AKIAIOSFODNN7EXAMPLE, .test TLD, redaction-test strings).
Fixes #2162
This is a disclosure fix, not a cosmetic one
The Workspace Reports tab dumped an agent-authored payload key-for-key to an external client. That is the same class the sibling surface already refuses:
agent_page.pydeliberately withholds the free-formasks.contextbecause it "has been a credential-leak surface before (canary G-04)". Reports carry an equally free-form, equally agent-authored blob, and shipped it whole to the agent's customer.The tab now renders through the shared
display_hintrenderers, with a client-facing summary fallback instead of a<pre>.What this does NOT fix — stated precisely
Typed renderers narrow what crosses: each reads only its declared keys, so an unexpected sibling key is not rendered. The fallback bounds, humanises and redacts. Neither eliminates the class:
markdown/table/kpi/timelinereport still renders its values as authored.The universal control is a boundary-side scrub at the portal serialization edge, deliberately out of scope here and listed as the first follow-up. Please do not read this PR as closing the leak class.
The redactor is defense-in-depth on the fallback path only. It mirrors the prefix set in
canary/invariants/g04_*.pyand consumes the whole token. That is a limit, not protection — it does nothing for the four typed paths above.Operator surfaces are behaviourally unchanged
git diff origin/dev...HEAD -- ReportsPanel.vue ReportsPanelFleet.vueis empty.ReportRenderer's newfallbackComponentprop defaults tonulland resolvesprops.fallbackComponent || ReportJson, so operator call sites — which pass nothing — reduce exactly to pre-branch behaviour.The human-readable fallback is portal-scoped by design. AC #2 asks for a fallback "deliberately stricter than the operator side, because the audience is an external client." A raw dump is a feature when you are debugging your own agent and a defect when the reader is that agent's customer.
The portal override also covers an agent-chosen
display_hint: "json"— a valid value in the MCP tool's enum — not merely a shape mismatch. That closes a second route to a raw dump.The only operator-visible deltas
Three dark-ink meta-text pairs and one
bg-blue-500 → bg-status-info-500swap that resolves to the identical colour (status-infoaliasescolors.blue).The dark fix was necessary, not cosmetic. Measured on rendered DOM, those sites failed WCAG AA in dark:
Light is unchanged at 4.83.
Backend surface
Two optional query params (
rows_offset,rows_limit) on the existing portal detail route. No new route, gate,db.accessor, table or column ⇒ CLAUDE.md Rule #9 dual-track does not engage — no SQLite migration, no Alembic revision.Verified mechanically: nothing touched under
db/,migrations/,schema.py,tables.py,models.py,main.py; zero@router.changes; zeroDepends(changes in Python (the 6Depends(hits in the diff are all prose in docstrings and markdown explaining why the operator/rowsroute is JWT-gated).REPORT_ROWS_PAGE_MAXis imported frommodels.py:503where it already lives — never re-typed — so the two page sizes cannot drift with each side's tests pinning its own version.Auth and scoping are inherited from the existing route. The uniform-404 property (Invariant #8) survives:
report_detailreturnsNoneonrow["agent_name"] != agent_namebefore any windowing runs, so a foreign report id is still indistinguishable from a nonexistent one. Pinned bytest_missing_and_foreign_stay_indistinguishable_when_windowed.The route gains a rate limiter it did not previously have
services/rate_limiter.py::enforce, keyportal_report_detail:{email}:{agent_name}, 60/60s, placed after_require_rosterso an unvalidated path param can never mint unbounded limiter keys.The windowed read re-reads and re-parses the ≤5 MiB blob per page, so the route that exists to cut transfer raises reads. Amplification is therefore strictly better than pre-branch, which had no limit at all.
Residual, stated: fail-open on a Redis outage, and the key is per-agent, so a client rostered on N agents scales linearly.
Verification
verify-localPASS (--skip-agent; agent stages correctly skipped — nodocker/base-image/**change).<pre>count 0 in every case.rows_limitthe response is byte-for-byte back-compatible.<pre>, portal shows the summary.npm run check:tokens→ OK, "dark ink ladder holds in 13 swept files".vite buildsucceeds.Tests
cd src/frontend && npx vitest run→ 25 files / 428 tests passedpytest tests/unit/test_2162_portal_report_window.py test_1535_report_prompt_guidance.py test_918_report_endpoint.py -q→ 32 passed[XPASS(strict)]reds ondev, reproduced on a pristineorigin/devcheckout — not from this branch. The +20 over sibling branches is this branch's own new tests.The 14 backend tests are overwhelmingly non-happy-path: foreign report, missing/foreign indistinguishability, read-failure→404, limit clamping, negative offset, offset past end, non-mutation of the source payload, rate limiting, route param bounds, and FastAPI route buildability.
AC #4 is not machine-guarded — please read this bit
check:tokens'INK_LADDER_SWEPTcovers exactly 13 files and includes neithercomponents/reports/norcomponents/portal/. CI therefore structurally cannot see the theme work in this PR; it was verified by hand with the measured contrast above.Suggest adding both directories to the swept list so the fix is guarded rather than re-earned. They are clean now, so it should pass as-is.
Honest note on the raw-colour ratchet
This branch adds 21 raw-gray class occurrences (
ReportSummary.vue+14,PortalAgentPage.vue+4,ReportTable.vue+2,ReportKpiTiles.vue+1;ReportTimeline.vue−1 non-gray), which nudges the count of files exceedingraw-color-baseline.jsonfrom 19 → 20.That increase is the arithmetic of the dark-mode fix itself: repairing
text-gray-500totext-gray-500 dark:text-gray-400necessarily adds a second raw-gray occurrence. The scanner counts occurrences, while the design system's both-themes rule requires adding the dark variant — so the metric moves the wrong way for a change that strictly improves compliance.No conversion is available:
tailwind.config.jsdefines semantic tokens forstatus-*,state-*,brand-*,accent-*,action-*— semantic roles only. Neutral ink has no token (grayis deliberately kept as the raw palette), which is exactly whybg-blue-500 → bg-status-info-500was converted (info has a token) and the gray pairs were not (they have none).Rebase expectation
PortalAgentPage.vue,workspace-agent-page.md(## Filestable) andcore-agent.md§5.11 each have a second writer this wave (#2169). Verified disjoint — myPortalAgentPage.vuehunks are all at old line ≥192, #2169's Overview block is 103–191. Resolution is "take both sides".Follow-ups — listed, deliberately NOT filed
table/markdown/kpi/timeline.display_hint, so a prefix-resolved table never pages./rows400 dead-ends its own UI.scan-raw-colors.mjsis not CI-wired and its baseline is stale (generated 2026-07-31); measured today, 19 files on pristineorigin/devalready exceed it. It also has no self-compare mode — only--baseline(write) and--json— so the "may only shrink" ratchet in CLAUDE.md Rule Feature/process engine #10 is currently unenforced by anything.inheritAttrs: falseacross the set).🤖 Generated with Claude Code