feat(dashboard): portfolio risk over time - #537
Merged
Conversation
The second aggregate behind the cockpit: a daily series of what opened, what closed, and what the portfolio is still carrying, over the caller's accessible projects. Two kinds of number share the series and they are not computed the same way. New and resolved counts are flows — the set difference between a scan's open exposures and the same project's previous succeeded scan. The naive portfolio version (today's total minus yesterday's) is wrong in a way that looks right: a CVE fixed in one project and a new one found in another cancel to zero, and the chart reports a quiet day on a day when two things happened. Critical and KEV counts are levels — the standing exposure on each day, anchored to each project's latest succeeded scan on or before it, carried forward on days nobody scanned. Every point also carries scan_count so the UI can say whether a level was measured or inherited; a quiet week is not a clean week. The set difference runs in the database. The previous scan per project comes from a LAG window function and each side is an anti-join against the other, so one row per scan crosses the wire rather than one per finding. Five queries regardless of portfolio or window size. That leaves "what counts as an open exposure" living in two modules, the duplicated-vocabulary trap of hardening rule #2. The closed-status tuple is imported from project_diff_service rather than restated, and test_dashboard_trends_parity runs both paths over the same rows. Its matrix is built from states where the two *can* disagree — the action queue's parity test learned that lesson expensively — and both halves were verified by mutation: reverting the exposure key fails five cases, dropping the status filter from the LAG fails six. Two defects found while building it, both fixed here: The release diff keyed exposures on (cve id, version string), so one CVE against two packages that share a version was one entry. A monorepo publishing in lockstep is the ordinary way that happens, and the second package silently vanished from both the introduced and the resolved list. The key is now the exposure's identity, (vulnerability_id, component_version_id). The window parameter was typed Literal[7, 30, 90]. A query string arrives as "7" and a literal of ints refuses to coerce it, so every explicit window was rejected with a 422 while the default kept working — a shape no test of the default can see. It is an IntEnum now, and the service still re-checks the set so a widened parameter cannot become an unbounded walk of scan history. Levels anchor on created_at, the clock dashboard_service already orders by, so the last point agrees with what /summary reports for today. Open means what the diff means by it, which is broader than the build gate's: a suppressed finding is not exposure the user is carrying, though it is still something policy will refuse to ship. The KEV level here can therefore sit below the action queue's KEV SLA bucket, deliberately. The widget is not here. This is the API only.
An independent review of the previous commit found two ways the chart would assert things that did not happen, plus one way it would fail outright on a large deployment. None were reachable by an attacker — the team-scope trace came back clean on all five queries — but all three would have shipped. **The predecessor ignored what a scan looked at.** The LAG partitioned by project alone, so a container scan (OS packages) followed by a source scan (language packages) compared two sets that share nothing: the whole first set read as resolved and the whole second as new, then the inverse the next day. A pull-request scan against a branch is the same defect in miniature. The partition is now (project, kind, ref) — the same line the retention model already draws by superseding only same-ref scans. **"No predecessor" did not mean "nothing came before".** A first scan contributed its whole open set as new, on the reasoning that onboarding a project is exposure entering the portfolio. But scan retention hard-deletes superseded scans after a grace period of days, so a project scanned daily by CI keeps only its last week on disk, and every 90-day window would have opened with a fabricated spike of hundreds of "new" findings for exposure that had been sitting there for months. A scan without a predecessor now contributes no flow at all. Nothing is lost: the exposure still enters the chart through the level series, which is where standing exposure belongs. On the local dataset this moved a 30-day window from 1,685 new / 121 resolved to 112 / 227 — the first pair was mostly fiction. **The level query round-tripped scan ids through Python**, one bind parameter each, so a portfolio of a few hundred projects scanned daily crossed asyncpg's 32767-parameter ceiling somewhere inside the 90-day window: a 500 that appears only on the largest deployments, and only for the widest window. The ids stay in SQL as a subquery now, which also removes a round trip. Two test defects the review found are worth more than the code fixes: The HTTP isolation test gave team A no project, so the service hit its empty-portfolio early return and never ran a query — deleting every project_id filter in the module would have left it green. Both teams now carry projects with different counts, so one execution proves the boundary. The parity matrix never placed a base scan outside the window, so bounding the LAG by start_ts — the exact mistake its docstring exists to prevent — would have passed every case. That is now an axis, alongside an interleaved container scan. Both new rules are verified by mutation: removing the lineage partition fails six tests, removing the no-predecessor rule fails six others. Also from the review, in the release diff this commit's predecessor touched: adding the purl to DiffVulnerability and to the sort key. Two Maven artefacts differing only by classifier share a name and a version, so after the key fix they serialized as two identical-looking rows in an order Postgres chose — non-idempotent output for anything rendering a diff into a PR comment. The levels now have an oracle too: a test pins the docstring's promise that the last point agrees with what /summary reports for today, including the tie-break when a project has two scans on the same day.
The action queue says what is waiting. This says whether the pile is growing — the question a report printed once cannot answer, because answering it needs history the tool has to have kept. Three panels, not one chart. Standing exposure (open critical, open KEV) and daily movement (new, resolved) are different measures on different scales, and putting them on one plot means two y-axes, which is the most reliable way to make a chart lie. Small multiples share the time axis and nothing else. Two decisions the data forced: **A flat line is ambiguous, so the panel says so.** Levels carry forward on days nobody scanned — a quiet week is not a clean week — and the footnote states in words that a flat line can mean nothing changed or nobody looked. When no scan landed in the whole window it says that instead, rather than drawing a confident line through nothing. **A failed load is not a clean portfolio.** Of the two mistakes this panel can make, drawing zeros when the truth is unknown is the one that says the risk is gone. The error state is explicit and has its own test, as the action queue's does. Direction carries the flow chart before colour does: new grows up from the baseline, resolved grows down, so the pair reads under any colour vision. The two hues are a validated pair (ΔE 8.6 under deuteranopia, both above the 3:1 non-text floor), the legend names both, and every value is also in a table view for readers the chart cannot reach. Each level series sits in its own panel, so identity never rests on telling two hues apart. Switching windows holds the previous series and dims it rather than dropping to skeletons — the content below this card would jump on every comparison. `--status-danger` gains the solid the flow bars paint with. index.css said the solids for warning, danger and info had been removed because nothing painted with them, and said to declare one again when something did; the design-system docs still listed all four, so this also closes half of a documentation drift and marks the other two honestly. The captured user-guide screenshot is not updated here. That capture waited on two containers that exist while their contents are still skeletons, so it could and did ship a page of grey blocks into the docs; it now waits on the loaded panels. Regenerating the guide's images belongs to the screenshot pass, not to this change.
The dashboard grew a second cockpit panel, so its baseline shifts and the rest re-render byte-identically under --update-snapshots=all. Captured on the Linux runner via the ui-gates dispatch, not locally. The a11y baseline is unchanged: the panel adds no violations, and the color-contrast drop on projects-list that appears locally does not reproduce on CI's seed — it was residue in the local database, not a fix.
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
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.
The action queue says what is waiting. This adds the other half of the cockpit: whether the pile is growing — the question a report printed once cannot answer, because answering it needs history the tool has to have kept.
What the series says
GET /v1/dashboard/trends?days=7|30|90returns one point per day carrying two kinds of number, which are not interchangeable:new_findings/resolved_findings) — the set difference between a scan's open exposures and those of the previous scan of the same lineage. The naive portfolio version, today's total minus yesterday's, is wrong in a way that looks right: a CVE fixed in one project and a new one found in another cancel to zero, and the chart reports a quiet day on a day when two things happened.critical_open/kev_open) — the standing exposure on each day, anchored to each project's latest succeeded scan on or before it, carried forward on days nobody scanned.Every point also carries
scan_count, which is what lets the UI tell the two apart honestly: a flat line can mean nothing changed or nobody looked.The set difference runs in the database — a
LAGwindow function for the predecessor, an anti-join each way — so one row per scan crosses the wire rather than one per finding. Five queries regardless of portfolio or window size.What the security review changed
The Producer-Reviewer rotation (CLAUDE.md §7) traced all five queries and found no team-scope leak. It found two ways the chart would assert things that did not happen, and one way it would fail outright at scale. All three are fixed in
d4173d24.The predecessor ignored what a scan looked at. The
LAGpartitioned by project alone, so a container scan (OS packages) followed by a source scan (language packages) compared two sets that share nothing: the whole first set read as resolved and the whole second as new, then the inverse the next day. A pull-request scan against a branch is the same defect in miniature. The partition is now(project, kind, ref)— the line the retention model already draws by superseding only same-ref scans."No predecessor" did not mean "nothing came before". A first scan contributed its whole open set as new. But
scan_retentionhard-deletes superseded scans after a grace period of days, so a project scanned daily by CI keeps only its last week on disk, and every 90-day window would have opened with a fabricated spike of hundreds of "new" findings for exposure that had been sitting there for months. A scan without a predecessor now contributes no flow. Nothing is lost — the exposure still enters through the level series, where standing exposure belongs. On the local dataset a 30-day window moved from 1,685 new / 121 resolved to 112 / 227; the first pair was mostly fiction.The level query round-tripped scan ids through Python, one bind parameter each, crossing asyncpg's 32767-parameter ceiling somewhere inside a 90-day window on a few hundred daily-scanned projects — a 500 visible only on the largest deployments, and only at the widest window. The ids stay in SQL as a subquery now.
Two test defects the review found are worth more than the code fixes. The HTTP isolation test gave team A no project, so the service hit its empty-portfolio early return and never ran a query — deleting every
project_idfilter in the module would have left it green. And the parity matrix never placed a base scan outside the window, so bounding theLAGbystart_ts, the exact mistake its docstring exists to prevent, would have passed every case.Parity, and how far it reaches
The flow rule now lives in two modules, which is the duplicated-vocabulary trap of hardening rule #2. The closed-status tuple is imported from
project_diff_servicerather than restated, andtest_dashboard_trends_parity.pyruns both paths over the same rows across six scenarios × four interference axes. The levels have their own oracle: a test pins the promise that the last point agrees with what/summaryreports for today, tie-break included.Both new rules are verified by mutation rather than assumed: removing the lineage partition fails six tests, removing the no-predecessor rule fails six others, reverting the exposure key fails five.
Two defects found on the way
project_diff_servicekeyed exposures on(cve id, version string), so one CVE against two packages sharing a version was one entry — a monorepo publishing in lockstep is the ordinary way that happens, and the second package silently vanished from both the introduced and the resolved list. The key is the exposure's identity now, andDiffVulnerabilitygains the purl, without which two Maven artefacts differing only by classifier render as one line repeated in a non-deterministic order.The
daysparameter was typedLiteral[7, 30, 90]. A query string arrives as"7"and a literal of ints refuses to coerce it, so every explicit window was rejected with a 422 while the default kept working — a shape no test of the default can see. It is anIntEnumnow, re-checked in the service so a widened parameter cannot become an unbounded walk of scan history.The panel
Three panels rather than one chart: levels and flows are different measures on different scales, and putting them on one plot means two y-axes. Direction carries the flow chart before colour does (new grows up, resolved down), the two hues are a validated pair (ΔE 8.6 under deuteranopia, both above the 3:1 non-text floor), and every value is also in a table view. Switching windows holds the previous series and dims it rather than dropping to skeletons.
--status-dangergains the solid the bars paint with —index.csssaid to declare one again when something painted with it, which also closes half of a drift where the design-system docs listed four solids for the one that existed.Gates
Visual baselines need recapturing on the Linux runner — the dashboard grew a panel. Doc screenshots are deliberately not regenerated here; that capture waited on containers that exist while their contents are skeletons, and it now waits on the loaded panels instead.
Not in this change
Triage does not survive a re-scan (
persist_trivy_findingswritesstatus="new"unconditionally), which this endpoint is the first surface to make visible as an oscillating chart, and which contradicts what the triage guide promises._accessible_project_idsdoes not narrow byapi_key_project_id— harmless while these routes are JWT-only, but it affects/summaryand/action-queuetoo and should be fixed once in the shared helper. Both need their own tickets.