From b176ce58e484ebfc277f5de5967a715db412be0c Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Mon, 9 Nov 2026 08:16:47 -0700 Subject: [PATCH] fix(api): scope the CVE summary by subscription_id Co-Authored-By: Claude Signed-off-by: Arpit Jain --- api/models/finding.py | 62 ++++++++++++++++++++++-------- api/routes/score.py | 15 +++++++- tests/test_clean_scan.py | 55 ++++++++++++++++++++++++++ tests/test_score_route_contract.py | 40 +++++++++++++++++++ 4 files changed, 153 insertions(+), 19 deletions(-) diff --git a/api/models/finding.py b/api/models/finding.py index 5c9c7602..9795ccdb 100644 --- a/api/models/finding.py +++ b/api/models/finding.py @@ -1559,26 +1559,54 @@ def get_score(self, subscription_id: Optional[str] = None) -> Dict[str, Any]: score = score_counts({severity: count for severity, count in rows}) return {"status": "OK", "score": score, "max_score": 100} - def get_cve_summary(self) -> Dict[str, Any]: - """Return high-level summary of CVE findings for the dashboard.""" + def get_cve_summary(self, subscription_id: Optional[str] = None) -> Dict[str, Any]: + """Return high-level summary of CVE findings for the dashboard. + + The "latest completed scan" lookup is a separate query from the + summary aggregate, matching get_score() and get_compliance_score(), + so the same subscription predicate can be applied to it. + + Args: + subscription_id: When given, scopes the "latest completed scan" + lookup to this subscription. In a shared-database deployment + that stores scans for more than one Azure subscription, + omitting this summarises whichever subscription scanned most + recently - not this caller's own data. Passed in by the route + (matching get_score) rather than read off the instance, which + never carries it. + """ conn = self._get_conn() with conn.cursor() as cur: - cur.execute(""" - SELECT - s.cve_enrichment_status, - COUNT(f.*) as total_findings, - COUNT(CASE WHEN f.exploit_available = TRUE THEN 1 END) as exploit_count, - MAX(f.cvss_score) as max_cvss_score, - AVG(f.cvss_score) as avg_cvss_score, - COUNT(CASE WHEN f.cvss_score >= 9.0 THEN 1 END) as critical_cve_count - FROM scans s - LEFT JOIN findings f ON s.scan_id = f.scan_id - WHERE s.scan_id = ( - SELECT scan_id FROM scans WHERE status = 'completed' ORDER BY started_at DESC LIMIT 1 + if subscription_id: + cur.execute( + "SELECT scan_id FROM scans " + "WHERE status = 'completed' AND subscription_id = %s " + "ORDER BY started_at DESC LIMIT 1", + (subscription_id,), ) - GROUP BY s.cve_enrichment_status - """) - row = cur.fetchone() + else: + cur.execute("SELECT scan_id FROM scans WHERE status = 'completed' ORDER BY started_at DESC LIMIT 1") + latest_scan = cur.fetchone() + + row = None + if latest_scan is not None: + cur.execute( + """ + SELECT + s.cve_enrichment_status, + COUNT(f.*) as total_findings, + COUNT(CASE WHEN f.exploit_available = TRUE THEN 1 END) as exploit_count, + MAX(f.cvss_score) as max_cvss_score, + AVG(f.cvss_score) as avg_cvss_score, + COUNT(CASE WHEN f.cvss_score >= 9.0 THEN 1 END) as critical_cve_count + FROM scans s + LEFT JOIN findings f ON s.scan_id = f.scan_id + WHERE s.scan_id = %s + GROUP BY s.cve_enrichment_status + """, + (latest_scan[0],), + ) + row = cur.fetchone() if not row: return { diff --git a/api/routes/score.py b/api/routes/score.py index 17ca36ee..c1c43abd 100644 --- a/api/routes/score.py +++ b/api/routes/score.py @@ -52,11 +52,22 @@ def get_score(): @score_bp.get("/api/score/cve-summary") def get_cve_summary(): - """Return high-level CVE summary for the dashboard.""" + """Return high-level CVE summary for the dashboard. + + Accepts the same optional ``?subscription_id=`` as GET /api/score. The + dashboard renders the score and this summary side by side, so both must + describe the same subscription. + """ try: + subscription_id = request.args.get("subscription_id") or os.environ.get("AZURE_SUBSCRIPTION_ID") + if subscription_id: + subscription_id = uuid_string(subscription_id, "subscription_id") + db = _get_db() - result = db.get_cve_summary() + result = db.get_cve_summary(subscription_id=subscription_id) return jsonify(result) + except ValidationError: + return jsonify({"error": VALIDATION_ERROR_MESSAGE}), 400 except Exception as exc: logger.error("Failed to fetch CVE summary: %s", exc) return jsonify({"error": "Failed to fetch CVE summary"}), 500 diff --git a/tests/test_clean_scan.py b/tests/test_clean_scan.py index 7a119345..eb5a77b8 100644 --- a/tests/test_clean_scan.py +++ b/tests/test_clean_scan.py @@ -400,3 +400,58 @@ def test_get_score_ignores_a_stray_subscription_id_attribute(): db.get_score() assert "subscription_id = %s" not in conn.cursor.return_value.execute.call_args[0][0] + + +# ── get_cve_summary subscription scoping ────────────────────────────────────── + + +def test_get_cve_summary_filters_latest_scan_by_subscription_id(): + """The CVE summary must honour the same scoping contract as get_score(). + + Both read the "latest completed scan" selector, and in a shared-database + deployment an unscoped selector returns whichever subscription scanned + most recently. get_score() was fixed for this; the CVE summary sitting + beside it on the dashboard was not. + """ + db = _db() + conn = MagicMock() + cur = _mock_score_cursor(None, []) + conn.cursor.return_value = cur + + with patch.object(db, "_get_conn", return_value=conn): + db.get_cve_summary(subscription_id="subscription-1") + + call = conn.cursor.return_value.execute.call_args + assert "subscription_id = %s" in call[0][0] + assert call[0][1] == ("subscription-1",) + + +def test_get_cve_summary_without_subscription_id_is_unscoped(): + """With no argument the SQL carries no subscription predicate and no + parameters, matching get_score()'s unscoped branch.""" + db = _db() + conn = MagicMock() + cur = _mock_score_cursor(None, []) + conn.cursor.return_value = cur + + with patch.object(db, "_get_conn", return_value=conn): + db.get_cve_summary() + + call = conn.cursor.return_value.execute.call_args + assert "subscription_id = %s" not in call[0][0] + assert len(call[0]) == 1 + + +def test_get_cve_summary_ignores_a_stray_subscription_id_attribute(): + """_db() sets db.subscription_id. Only the explicit argument may scope + the lookup, so an instance attribute cannot silently disagree with it.""" + db = _db() + assert db.subscription_id == "subscription-1" + conn = MagicMock() + cur = _mock_score_cursor(None, []) + conn.cursor.return_value = cur + + with patch.object(db, "_get_conn", return_value=conn): + db.get_cve_summary() + + assert "subscription_id = %s" not in conn.cursor.return_value.execute.call_args[0][0] diff --git a/tests/test_score_route_contract.py b/tests/test_score_route_contract.py index 73aea2a8..7736572d 100644 --- a/tests/test_score_route_contract.py +++ b/tests/test_score_route_contract.py @@ -84,3 +84,43 @@ def test_route_rejects_a_malformed_subscription_id(client, auth_headers, monkeyp assert resp.status_code == 400 db.get_score.assert_not_called() + + +def test_cve_summary_route_passes_subscription_id_through_to_the_model(client, auth_headers, monkeypatch): + """/api/score/cve-summary must scope on ?subscription_id= exactly as + /api/score does. The dashboard renders both side by side, so an unscoped + summary beside a scoped score describes two different subscriptions.""" + monkeypatch.delenv("AZURE_SUBSCRIPTION_ID", raising=False) + sub = "11111111-2222-3333-4444-555555555555" + db = MagicMock() + db.get_cve_summary.return_value = {"status": "OK", "total_findings": 3} + with patch.object(score_route, "_get_db", return_value=db): + resp = client.get(f"/api/score/cve-summary?subscription_id={sub}", headers=auth_headers) + + assert resp.status_code == 200 + db.get_cve_summary.assert_called_once_with(subscription_id=sub) + + +def test_cve_summary_route_falls_back_to_the_configured_default_subscription(client, auth_headers, monkeypatch): + """With no query parameter the deployment's own AZURE_SUBSCRIPTION_ID is + used, the same fallback /api/score applies.""" + sub = "11111111-2222-3333-4444-555555555555" + monkeypatch.setenv("AZURE_SUBSCRIPTION_ID", sub) + db = MagicMock() + db.get_cve_summary.return_value = {"status": "OK", "total_findings": 3} + with patch.object(score_route, "_get_db", return_value=db): + resp = client.get("/api/score/cve-summary", headers=auth_headers) + + assert resp.status_code == 200 + db.get_cve_summary.assert_called_once_with(subscription_id=sub) + + +def test_cve_summary_route_rejects_a_malformed_subscription_id(client, auth_headers, monkeypatch): + """A non-UUID subscription_id is a 400, not a silently unscoped summary.""" + monkeypatch.delenv("AZURE_SUBSCRIPTION_ID", raising=False) + db = MagicMock() + with patch.object(score_route, "_get_db", return_value=db): + resp = client.get("/api/score/cve-summary?subscription_id=not-a-uuid", headers=auth_headers) + + assert resp.status_code == 400 + db.get_cve_summary.assert_not_called()