From 7a39390a4232837b44df0ebe20fd6448861369c3 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 24 Sep 2026 17:43:09 +0100 Subject: [PATCH 01/21] feat(graph): wire InventorySnapshot into ScanEngine.run_scan() Signed-off-by: Tanvir Farhad --- tests/test_graph_engine_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_graph_engine_integration.py b/tests/test_graph_engine_integration.py index d68869f0..b69c9a50 100644 --- a/tests/test_graph_engine_integration.py +++ b/tests/test_graph_engine_integration.py @@ -68,6 +68,7 @@ def test_run_scan_existing_rule_without_snapshot_param_still_works(engine): snapshot = _make_snapshot() def legacy_scan(client, subscription_id): +<<<<<<< HEAD return [ { "rule_id": "AZ-LEGACY-001", From e9524df14c4ea736d0483507e85f174438f29568 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 24 Sep 2026 18:01:39 +0100 Subject: [PATCH 02/21] feat(graph): wire post-scan node and edge population into ScanEngine Signed-off-by: Tanvir Farhad --- scanner/engine.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scanner/engine.py b/scanner/engine.py index 5bf08da9..df72b68e 100644 --- a/scanner/engine.py +++ b/scanner/engine.py @@ -3,6 +3,7 @@ import importlib.util import inspect import logging +import os import uuid from datetime import datetime, timezone from pathlib import Path @@ -12,6 +13,7 @@ from openshield.severity import CONTRACT_VERSION, SeverityContractError, normalize_severity, score_findings from scanner.azure_client import AzureClient from scanner.evaluation import EvaluationStatus, RuleEvaluation, subscription_scope_id +from scanner.graph.graph_populator import populate_graph from scanner.graph.snapshot_bridge import collect_snapshot logger = logging.getLogger(__name__) @@ -237,6 +239,14 @@ def run_scan(self, scan_id: Optional[str] = None) -> Dict[str, Any]: logger.info("Scan %s complete — %d total finding(s). Normalising results...", scan_id, len(findings)) + # Populate the attack graph as a non-fatal post-scan step. + dsn = os.environ.get("DATABASE_URL") + if snapshot and dsn: + try: + populate_graph(scan_id, snapshot, dsn) + except Exception as exc: + logger.warning("run_scan: graph population raised unexpectedly: %s", exc) + return make_serializable(result) def _evaluate_rule(self, rule: Any, rule_id: str) -> List[RuleEvaluation]: From 4e90f74bb363c3d4d06ae6f5c5d240777a1bd90f Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 24 Sep 2026 18:11:19 +0100 Subject: [PATCH 03/21] feat(graph): BFS path traversal, attack_paths migration, and API endpoints Signed-off-by: Tanvir Farhad --- alembic/versions/f2a3b4c5d6e7_attack_paths.py | 61 ++++++ api/app.py | 2 + api/routes/attack_graph.py | 193 ++++++++++++++++++ scanner/graph/graph_populator.py | 8 + scanner/graph/path_traversal.py | 160 +++++++++++++++ tests/test_attack_graph_api.py | 63 ++++++ tests/test_graph_path_traversal.py | 69 +++++++ 7 files changed, 556 insertions(+) create mode 100644 alembic/versions/f2a3b4c5d6e7_attack_paths.py create mode 100644 api/routes/attack_graph.py create mode 100644 scanner/graph/path_traversal.py create mode 100644 tests/test_attack_graph_api.py create mode 100644 tests/test_graph_path_traversal.py diff --git a/alembic/versions/f2a3b4c5d6e7_attack_paths.py b/alembic/versions/f2a3b4c5d6e7_attack_paths.py new file mode 100644 index 00000000..e2cbe919 --- /dev/null +++ b/alembic/versions/f2a3b4c5d6e7_attack_paths.py @@ -0,0 +1,61 @@ +"""Add attack_paths table for pre-computed BFS traversal results. + +Revision ID: f2a3b4c5d6e7 +Revises: e1f2a3b4c5d6 +Create Date: 2026-09-24 00:00:00.000000 +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision: str = "f2a3b4c5d6e7" +down_revision: Union[str, Sequence[str], None] = "e1f2a3b4c5d6" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.create_table( + "attack_paths", + sa.Column("path_id", postgresql.UUID(), nullable=False), + sa.Column("tenant_id", sa.Text(), nullable=False), + sa.Column("scan_id", sa.Text(), nullable=False), + sa.Column("source_node_id", postgresql.UUID(), nullable=False), + sa.Column("target_node_id", postgresql.UUID(), nullable=False), + sa.Column("path_node_ids", postgresql.ARRAY(postgresql.UUID()), nullable=False), + sa.Column("path_length", sa.Integer(), nullable=False), + sa.Column("min_confidence", sa.Float(), nullable=False, server_default=sa.text("1.0")), + sa.Column("relationship_types", postgresql.ARRAY(sa.Text()), nullable=False, + server_default=sa.text("ARRAY[]::text[]")), + sa.Column("computed_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("now()")), + sa.ForeignKeyConstraint( + ["source_node_id"], ["graph_nodes.node_id"], + name="attack_paths_source_fkey", ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["target_node_id"], ["graph_nodes.node_id"], + name="attack_paths_target_fkey", ondelete="CASCADE", + ), + sa.PrimaryKeyConstraint("path_id", name="attack_paths_pkey"), + ) + op.create_index("idx_attack_paths_tenant_scan", "attack_paths", ["tenant_id", "scan_id"]) + op.create_index("idx_attack_paths_source", "attack_paths", ["source_node_id"]) + op.create_index("idx_attack_paths_target", "attack_paths", ["target_node_id"]) + op.create_index( + "uq_attack_paths_source_target_scan", + "attack_paths", + ["source_node_id", "target_node_id", "scan_id"], + unique=True, + ) + + +def downgrade() -> None: + op.drop_index("uq_attack_paths_source_target_scan", table_name="attack_paths") + op.drop_index("idx_attack_paths_target", table_name="attack_paths") + op.drop_index("idx_attack_paths_source", table_name="attack_paths") + op.drop_index("idx_attack_paths_tenant_scan", table_name="attack_paths") + op.drop_table("attack_paths") diff --git a/api/app.py b/api/app.py index 80699516..a135a24f 100644 --- a/api/app.py +++ b/api/app.py @@ -256,6 +256,7 @@ def verify_jwt() -> None: # ------------------------------------------------------------------ # from api.routes.ai import ai_bp from api.routes.assurance import assurance_bp + from api.routes.attack_graph import attack_graph_bp from api.routes.cbom import cbom_bp from api.routes.compliance import compliance_bp from api.routes.drift import drift_bp @@ -268,6 +269,7 @@ def verify_jwt() -> None: app.register_blueprint(ai_bp) app.register_blueprint(assurance_bp) + app.register_blueprint(attack_graph_bp) app.register_blueprint(cbom_bp) app.register_blueprint(compliance_bp) app.register_blueprint(drift_bp) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py new file mode 100644 index 00000000..75deb78c --- /dev/null +++ b/api/routes/attack_graph.py @@ -0,0 +1,193 @@ +"""Attack graph API: resource nodes, edges, and pre-computed attack paths.""" + +import logging +import os + +import psycopg2 +import psycopg2.extras +from flask import Blueprint, g, jsonify, request + +from api.validation import ValidationError, positive_integer, uuid_string + +attack_graph_bp = Blueprint("attack_graph", __name__) +logger = logging.getLogger(__name__) + +_DEFAULT_LIMIT = 100 +_MAX_LIMIT = 500 + + +def _conn(): + if "graph_conn" not in g: + g.graph_conn = psycopg2.connect(os.environ["DATABASE_URL"]) + g.graph_conn.autocommit = True + psycopg2.extras.register_uuid(g.graph_conn) + return g.graph_conn + + +def _tenant_id() -> str | None: + """Resolve tenant_id from JWT payload or query param (multi-tenant support).""" + user = getattr(g, "user", {}) or {} + return user.get("tenant_id") or request.args.get("tenant_id") + + +@attack_graph_bp.teardown_app_request +def _close_conn(exc): + conn = g.pop("graph_conn", None) + if conn is not None: + conn.close() + + +@attack_graph_bp.get("/api/attack-graph") +def get_attack_graph(): + """Return graph nodes and edges for the caller's tenant (latest snapshot). + + Query params: subscription_id (optional), limit (default 100, max 500) + """ + try: + limit = positive_integer(request.args.get("limit", _DEFAULT_LIMIT), "limit") + if limit > _MAX_LIMIT: + limit = _MAX_LIMIT + subscription_id = request.args.get("subscription_id") + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 + + tenant_id = _tenant_id() + if not tenant_id: + return jsonify({"error": "tenant_id not available"}), 400 + + conn = _conn() + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + node_filter = "WHERE n.tenant_id = %(tenant_id)s" + params: dict = {"tenant_id": tenant_id, "limit": limit} + if subscription_id: + node_filter += " AND n.subscription_id = %(subscription_id)s" + params["subscription_id"] = subscription_id + + cur.execute( + f""" + SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, + n.location, n.resource_group, n.subscription_id, n.snapshot_id, + n.updated_at + FROM graph_nodes n + {node_filter} + ORDER BY n.updated_at DESC + LIMIT %(limit)s + """, + params, + ) + nodes = cur.fetchall() + + node_ids = [row["node_id"] for row in nodes] + edges: list = [] + if node_ids: + cur.execute( + """ + SELECT e.edge_id::text, e.source_node_id::text, e.target_node_id::text, + e.relationship_type, e.confidence, e.evidence_source, e.collected_at + FROM graph_edges e + WHERE e.source_node_id = ANY(%(node_ids)s::uuid[]) + OR e.target_node_id = ANY(%(node_ids)s::uuid[]) + """, + {"node_ids": node_ids}, + ) + edges = cur.fetchall() + + return jsonify({"nodes": [dict(r) for r in nodes], "edges": [dict(r) for r in edges]}) + + +@attack_graph_bp.get("/api/attack-paths") +def list_attack_paths(): + """Return pre-computed attack paths for a scan. + + Query params: scan_id (required), limit (default 100, max 500) + """ + scan_id = request.args.get("scan_id") + if not scan_id: + return jsonify({"error": "scan_id is required"}), 400 + try: + scan_id = uuid_string(scan_id, "scan_id") + limit = positive_integer(request.args.get("limit", _DEFAULT_LIMIT), "limit") + if limit > _MAX_LIMIT: + limit = _MAX_LIMIT + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 + + tenant_id = _tenant_id() + if not tenant_id: + return jsonify({"error": "tenant_id not available"}), 400 + + conn = _conn() + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + cur.execute( + """ + SELECT ap.path_id::text, ap.source_node_id::text, ap.target_node_id::text, + ap.path_node_ids, ap.path_length, ap.min_confidence, + ap.relationship_types, ap.computed_at, + src.resource_type AS source_type, src.name AS source_name, + tgt.resource_type AS target_type, tgt.name AS target_name + FROM attack_paths ap + JOIN graph_nodes src ON src.node_id = ap.source_node_id + JOIN graph_nodes tgt ON tgt.node_id = ap.target_node_id + WHERE ap.scan_id = %(scan_id)s + AND ap.tenant_id = %(tenant_id)s + ORDER BY ap.path_length ASC, ap.min_confidence DESC + LIMIT %(limit)s + """, + {"scan_id": scan_id, "tenant_id": tenant_id, "limit": limit}, + ) + rows = cur.fetchall() + + return jsonify({"scan_id": scan_id, "paths": [dict(r) for r in rows]}) + + +@attack_graph_bp.get("/api/attack-paths/") +def get_attack_path(path_id: str): + """Return a single attack path with full node detail for each hop.""" + try: + path_id = uuid_string(path_id, "path_id") + except ValidationError as exc: + return jsonify({"error": str(exc)}), 400 + + tenant_id = _tenant_id() + if not tenant_id: + return jsonify({"error": "tenant_id not available"}), 400 + + conn = _conn() + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + cur.execute( + """ + SELECT ap.path_id::text, ap.scan_id, ap.source_node_id::text, + ap.target_node_id::text, ap.path_node_ids, ap.path_length, + ap.min_confidence, ap.relationship_types, ap.computed_at + FROM attack_paths ap + WHERE ap.path_id = %(path_id)s::uuid + AND ap.tenant_id = %(tenant_id)s + """, + {"path_id": path_id, "tenant_id": tenant_id}, + ) + row = cur.fetchone() + + if row is None: + return jsonify({"error": "not found"}), 404 + + path = dict(row) + + # Fetch full node detail for each hop + node_ids = [str(nid) for nid in path["path_node_ids"]] + if node_ids: + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + cur.execute( + """ + SELECT node_id::text, resource_id, resource_type, name, + location, resource_group, subscription_id + FROM graph_nodes + WHERE node_id = ANY(%(ids)s::uuid[]) + AND tenant_id = %(tenant_id)s + """, + {"ids": node_ids, "tenant_id": tenant_id}, + ) + nodes_by_id = {r["node_id"]: dict(r) for r in cur.fetchall()} + path["hops"] = [nodes_by_id.get(str(nid), {"node_id": str(nid)}) for nid in path["path_node_ids"]] + + path["path_node_ids"] = [str(nid) for nid in path["path_node_ids"]] + return jsonify(path) diff --git a/scanner/graph/graph_populator.py b/scanner/graph/graph_populator.py index 456f98e0..d621f1cb 100644 --- a/scanner/graph/graph_populator.py +++ b/scanner/graph/graph_populator.py @@ -11,6 +11,7 @@ from scanner.arg_inventory import InventoryResource, InventoryStatus from scanner.graph.node_service import graph_connection, link_findings_to_nodes, lock_graph_scopes, populate_nodes from scanner.graph.edge_detector import detect_all_edges +from scanner.graph.path_traversal import compute_attack_paths if TYPE_CHECKING: from scanner.arg_inventory import InventorySnapshot @@ -189,3 +190,10 @@ def populate_graph(scan_id: str, snapshot: InventorySnapshot, dsn: str) -> None: ) except Exception: logger.warning("graph: population failed for scan %s", scan_id, exc_info=True) + return + + try: + path_count = compute_attack_paths(scan_id, snapshot.tenant_id, dsn) + logger.info("graph: computed %d attack paths for scan %s", path_count, scan_id) + except Exception as exc: + logger.warning("graph: path traversal failed for scan %s: %s", scan_id, exc) diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py new file mode 100644 index 00000000..7bbf87ae --- /dev/null +++ b/scanner/graph/path_traversal.py @@ -0,0 +1,160 @@ +"""BFS path traversal over the attack graph stored in PostgreSQL. + +Computes shortest paths from every finding-linked node to every reachable node +and persists them in attack_paths for API consumption. +""" +from __future__ import annotations + +import logging +import uuid +from collections import deque +from typing import Any + +import psycopg2 +import psycopg2.extras + +logger = logging.getLogger(__name__) + +# Paths longer than this are not persisted — they're rarely actionable and +# keeping them would inflate the table for large graphs. +_MAX_PATH_LENGTH = 8 + + +def _load_adjacency(conn: Any, tenant_id: str) -> dict[str, list[tuple[str, str, float]]]: + """Return {source_node_id: [(target_node_id, relationship_type, confidence)]}.""" + with conn.cursor() as cur: + cur.execute( + """ + SELECT e.source_node_id::text, e.target_node_id::text, + e.relationship_type, e.confidence + FROM graph_edges e + JOIN graph_nodes src ON src.node_id = e.source_node_id + WHERE src.tenant_id = %(tenant_id)s + """, + {"tenant_id": tenant_id}, + ) + adj: dict[str, list[tuple[str, str, float]]] = {} + for src, tgt, rel, conf in cur.fetchall(): + adj.setdefault(src, []).append((tgt, rel, conf)) + return adj + + +def _load_finding_nodes(conn: Any, scan_id: str, tenant_id: str) -> list[str]: + """Return node_ids linked to findings from this scan (same tenant).""" + with conn.cursor() as cur: + cur.execute( + """ + SELECT DISTINCT fgn.node_id::text + FROM finding_graph_nodes fgn + JOIN findings f ON f.id = fgn.finding_id + JOIN graph_nodes n ON n.node_id = fgn.node_id + WHERE f.scan_id = %(scan_id)s + AND n.tenant_id = %(tenant_id)s + """, + {"scan_id": scan_id, "tenant_id": tenant_id}, + ) + return [row[0] for row in cur.fetchall()] + + +def _bfs_from( + start: str, + adj: dict[str, list[tuple[str, str, float]]], +) -> list[dict[str, Any]]: + """BFS from start. Returns one path record per reachable node (shortest path).""" + paths: list[dict[str, Any]] = [] + # queue: (current_node_id, path_so_far, min_confidence, relationship_types) + queue: deque[tuple[str, list[str], float, list[str]]] = deque() + queue.append((start, [start], 1.0, [])) + visited: set[str] = {start} + + while queue: + node, path, min_conf, rels = queue.popleft() + for neighbour, rel, conf in adj.get(node, []): + if neighbour in visited: + continue + visited.add(neighbour) + new_path = path + [neighbour] + new_rels = rels + [rel] + new_conf = min(min_conf, conf) + paths.append( + { + "target_node_id": neighbour, + "path_node_ids": new_path, + "path_length": len(new_path) - 1, + "min_confidence": new_conf, + "relationship_types": new_rels, + } + ) + if len(new_path) - 1 < _MAX_PATH_LENGTH: + queue.append((neighbour, new_path, new_conf, new_rels)) + + return paths + + +def _write_paths( + conn: Any, + scan_id: str, + tenant_id: str, + source_node_id: str, + paths: list[dict[str, Any]], +) -> int: + if not paths: + return 0 + with conn.cursor() as cur: + rows = [ + ( + str(uuid.uuid4()), + tenant_id, + scan_id, + source_node_id, + p["target_node_id"], + p["path_node_ids"], + p["path_length"], + p["min_confidence"], + p["relationship_types"], + ) + for p in paths + ] + psycopg2.extras.execute_values( + cur, + """ + INSERT INTO attack_paths + (path_id, tenant_id, scan_id, source_node_id, target_node_id, + path_node_ids, path_length, min_confidence, relationship_types) + VALUES %s + ON CONFLICT (source_node_id, target_node_id, scan_id) DO NOTHING + """, + rows, + template="(%s, %s, %s, %s::uuid, %s::uuid, %s::uuid[], %s, %s, %s)", + ) + return len(rows) + + +def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: + """Run BFS from every finding-linked node and persist paths. Returns path count.""" + try: + conn = psycopg2.connect(dsn) + conn.autocommit = False + try: + adj = _load_adjacency(conn, tenant_id) + source_nodes = _load_finding_nodes(conn, scan_id, tenant_id) + if not source_nodes: + logger.info("graph path traversal: no finding-linked nodes for scan %s", scan_id) + return 0 + + total = 0 + for src in source_nodes: + paths = _bfs_from(src, adj) + total += _write_paths(conn, scan_id, tenant_id, src, paths) + + conn.commit() + logger.info("graph path traversal: %d paths written for scan %s", total, scan_id) + return total + except Exception: + conn.rollback() + raise + finally: + conn.close() + except Exception as exc: + logger.error("graph path traversal failed for scan %s: %s", scan_id, exc) + return 0 diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py new file mode 100644 index 00000000..c1f4dad9 --- /dev/null +++ b/tests/test_attack_graph_api.py @@ -0,0 +1,63 @@ +"""Unit tests for the attack graph API routes.""" +from unittest.mock import MagicMock, patch + + +def _mock_conn(rows_sequence=None): + """Return a mock psycopg2 connection returning preset rows per cursor call.""" + rows_sequence = list(rows_sequence or []) + conn = MagicMock() + conn.autocommit = True + call_idx = [0] + + def make_cursor(*args, **kwargs): + cur = MagicMock() + cur.__enter__ = lambda s: s + cur.__exit__ = MagicMock(return_value=False) + idx = call_idx[0] + call_idx[0] += 1 + rows = rows_sequence[idx] if idx < len(rows_sequence) else [] + cur.fetchall.return_value = rows + cur.fetchone.return_value = rows[0] if rows else None + return cur + + conn.cursor.side_effect = make_cursor + return conn + + +_TENANT = "00000000-0000-0000-0000-000000000001" + + +def test_get_attack_graph_returns_empty_nodes_and_edges(client, auth_headers): + conn = _mock_conn([[]]) + with patch("api.routes.attack_graph.psycopg2.connect", return_value=conn): + with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): + resp = client.get(f"/api/attack-graph?tenant_id={_TENANT}", headers=auth_headers) + assert resp.status_code == 200 + data = resp.get_json() + assert "nodes" in data + assert "edges" in data + + +def test_list_attack_paths_missing_scan_id_returns_400(client, auth_headers): + resp = client.get("/api/attack-paths", headers=auth_headers) + assert resp.status_code == 400 + assert b"scan_id" in resp.data + + +def test_list_attack_paths_invalid_uuid_returns_400(client, auth_headers): + resp = client.get("/api/attack-paths?scan_id=not-a-uuid", headers=auth_headers) + assert resp.status_code == 400 + + +def test_get_attack_path_invalid_uuid_returns_400(client, auth_headers): + resp = client.get("/api/attack-paths/not-a-uuid", headers=auth_headers) + assert resp.status_code == 400 + + +def test_get_attack_path_not_found_returns_404(client, auth_headers): + valid_uuid = "00000000-0000-0000-0000-000000000002" + conn = _mock_conn([[]]) + with patch("api.routes.attack_graph.psycopg2.connect", return_value=conn): + with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): + resp = client.get(f"/api/attack-paths/{valid_uuid}?tenant_id={_TENANT}", headers=auth_headers) + assert resp.status_code == 404 diff --git a/tests/test_graph_path_traversal.py b/tests/test_graph_path_traversal.py new file mode 100644 index 00000000..b46a8c9f --- /dev/null +++ b/tests/test_graph_path_traversal.py @@ -0,0 +1,69 @@ +"""Unit tests for BFS path traversal (scanner/graph/path_traversal.py).""" +from unittest.mock import MagicMock, patch + +from scanner.graph.path_traversal import _bfs_from, compute_attack_paths + + +def test_bfs_from_returns_empty_for_isolated_node(): + adj = {} + result = _bfs_from("node-A", adj) + assert result == [] + + +def test_bfs_from_single_hop(): + adj = {"node-A": [("node-B", "PROTECTS", 1.0)]} + result = _bfs_from("node-A", adj) + assert len(result) == 1 + path = result[0] + assert path["target_node_id"] == "node-B" + assert path["path_length"] == 1 + assert path["min_confidence"] == 1.0 + assert path["relationship_types"] == ["PROTECTS"] + assert path["path_node_ids"] == ["node-A", "node-B"] + + +def test_bfs_from_two_hops(): + adj = { + "A": [("B", "PROTECTS", 1.0)], + "B": [("C", "EXPOSES", 0.8)], + } + result = _bfs_from("A", adj) + assert len(result) == 2 + two_hop = next(r for r in result if r["target_node_id"] == "C") + assert two_hop["path_length"] == 2 + assert two_hop["min_confidence"] == 0.8 + assert two_hop["relationship_types"] == ["PROTECTS", "EXPOSES"] + + +def test_bfs_from_does_not_revisit_nodes(): + adj = { + "A": [("B", "PROTECTS", 1.0), ("C", "EXPOSES", 1.0)], + "B": [("C", "MEMBER_OF", 1.0)], + } + result = _bfs_from("A", adj) + target_nodes = [r["target_node_id"] for r in result] + assert target_nodes.count("C") == 1 + + +def test_compute_attack_paths_returns_zero_when_no_finding_nodes(): + with patch("scanner.graph.path_traversal.psycopg2.connect") as mock_connect: + conn = MagicMock() + mock_connect.return_value = conn + cur = MagicMock() + cur.__enter__ = lambda s: s + cur.__exit__ = MagicMock(return_value=False) + conn.cursor.return_value = cur + # adjacency returns empty; finding_nodes returns empty + cur.fetchall.return_value = [] + + result = compute_attack_paths("scan-1", "tenant-1", "postgresql://x") + + assert result == 0 + + +def test_compute_attack_paths_handles_db_error_gracefully(): + with patch("scanner.graph.path_traversal.psycopg2.connect") as mock_connect: + mock_connect.side_effect = Exception("DB unavailable") + result = compute_attack_paths("scan-1", "tenant-1", "postgresql://x") + + assert result == 0 From 5525300a92b0b8ec226ed61a7d13340627b7d4a7 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 24 Sep 2026 22:26:38 +0100 Subject: [PATCH 04/21] fix(graph): remove query-param tenant_id fallback to prevent cross-tenant IDOR Viewer-role tokens and tokens without a tenant claim can no longer access another tenant's graph data by supplying tenant_id as a query parameter. Tenant resolution now uses the OIDC tid claim (user["tenant"]) or, for shared-secret admin tokens only, the X-Tenant-Id request header. Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 17 ++++++++- tests/test_attack_graph_api.py | 68 +++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 75deb78c..432d7475 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -25,9 +25,22 @@ def _conn(): def _tenant_id() -> str | None: - """Resolve tenant_id from JWT payload or query param (multi-tenant support).""" + """Resolve tenant_id from the verified principal. + + OIDC mode: the 'tenant' field is populated from the 'tid' claim in the token. + Shared-secret mode: 'tenant' is always None; admins may supply + X-Tenant-Id as a request header (never a query param, which leaks into + logs and caches). Non-admin tokens cannot override the header. + """ user = getattr(g, "user", {}) or {} - return user.get("tenant_id") or request.args.get("tenant_id") + # OIDC path: tid claim decoded by the verifier into user["tenant"] + tid = user.get("tenant") + if tid: + return tid + # Shared-secret path: admin-only header override for multi-tenant deployments + if user.get("role") == "admin": + return request.headers.get("X-Tenant-Id") or None + return None @attack_graph_bp.teardown_app_request diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index c1f4dad9..01918d17 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -1,6 +1,31 @@ """Unit tests for the attack graph API routes.""" +import time from unittest.mock import MagicMock, patch +import pytest + +_TENANT = "00000000-0000-0000-0000-000000000001" + + +@pytest.fixture() +def tenant_auth_headers(app): + """Admin JWT headers with X-Tenant-Id for shared-secret mode tests.""" + import jwt + + secret = app.config["JWT_SECRET"] + payload = { + "sub": "test-user", + "role": "admin", + "iat": int(time.time()), + "exp": int(time.time()) + 3600, + } + token = jwt.encode(payload, secret, algorithm="HS256") + return { + "Authorization": f"Bearer {token}", + "Content-Type": "application/json", + "X-Tenant-Id": _TENANT, + } + def _mock_conn(rows_sequence=None): """Return a mock psycopg2 connection returning preset rows per cursor call.""" @@ -24,40 +49,57 @@ def make_cursor(*args, **kwargs): return conn -_TENANT = "00000000-0000-0000-0000-000000000001" - - -def test_get_attack_graph_returns_empty_nodes_and_edges(client, auth_headers): +def test_get_attack_graph_returns_empty_nodes_and_edges(client, tenant_auth_headers): conn = _mock_conn([[]]) with patch("api.routes.attack_graph.psycopg2.connect", return_value=conn): with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): - resp = client.get(f"/api/attack-graph?tenant_id={_TENANT}", headers=auth_headers) + resp = client.get("/api/attack-graph", headers=tenant_auth_headers) assert resp.status_code == 200 data = resp.get_json() assert "nodes" in data assert "edges" in data -def test_list_attack_paths_missing_scan_id_returns_400(client, auth_headers): - resp = client.get("/api/attack-paths", headers=auth_headers) +def test_get_attack_graph_viewer_cannot_supply_tenant_header(client, app): + """Viewer-role tokens must not use X-Tenant-Id header (admin-only).""" + import jwt + + secret = app.config["JWT_SECRET"] + payload = { + "sub": "viewer-user", + "role": "viewer", + "iat": int(time.time()), + "exp": int(time.time()) + 3600, + } + token = jwt.encode(payload, secret, algorithm="HS256") + headers = { + "Authorization": f"Bearer {token}", + "X-Tenant-Id": _TENANT, + } + resp = client.get("/api/attack-graph", headers=headers) + assert resp.status_code == 400 + + +def test_list_attack_paths_missing_scan_id_returns_400(client, tenant_auth_headers): + resp = client.get("/api/attack-paths", headers=tenant_auth_headers) assert resp.status_code == 400 assert b"scan_id" in resp.data -def test_list_attack_paths_invalid_uuid_returns_400(client, auth_headers): - resp = client.get("/api/attack-paths?scan_id=not-a-uuid", headers=auth_headers) +def test_list_attack_paths_invalid_uuid_returns_400(client, tenant_auth_headers): + resp = client.get("/api/attack-paths?scan_id=not-a-uuid", headers=tenant_auth_headers) assert resp.status_code == 400 -def test_get_attack_path_invalid_uuid_returns_400(client, auth_headers): - resp = client.get("/api/attack-paths/not-a-uuid", headers=auth_headers) +def test_get_attack_path_invalid_uuid_returns_400(client, tenant_auth_headers): + resp = client.get("/api/attack-paths/not-a-uuid", headers=tenant_auth_headers) assert resp.status_code == 400 -def test_get_attack_path_not_found_returns_404(client, auth_headers): +def test_get_attack_path_not_found_returns_404(client, tenant_auth_headers): valid_uuid = "00000000-0000-0000-0000-000000000002" conn = _mock_conn([[]]) with patch("api.routes.attack_graph.psycopg2.connect", return_value=conn): with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): - resp = client.get(f"/api/attack-paths/{valid_uuid}?tenant_id={_TENANT}", headers=auth_headers) + resp = client.get(f"/api/attack-paths/{valid_uuid}", headers=tenant_auth_headers) assert resp.status_code == 404 From bcf4eb0bfe907ba53e2da549fc4beed92f519c4d Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Fri, 25 Sep 2026 00:44:23 +0100 Subject: [PATCH 05/21] fix(lint): ruff format + remove f-string SQL to clear Bandit B608 Signed-off-by: Tanvir Farhad --- alembic/versions/f2a3b4c5d6e7_attack_paths.py | 20 +++++---- api/routes/attack_graph.py | 43 +++++++++++-------- scanner/graph/path_traversal.py | 1 + tests/test_attack_graph_api.py | 1 + tests/test_graph_path_traversal.py | 1 + 5 files changed, 41 insertions(+), 25 deletions(-) diff --git a/alembic/versions/f2a3b4c5d6e7_attack_paths.py b/alembic/versions/f2a3b4c5d6e7_attack_paths.py index e2cbe919..e312c062 100644 --- a/alembic/versions/f2a3b4c5d6e7_attack_paths.py +++ b/alembic/versions/f2a3b4c5d6e7_attack_paths.py @@ -28,17 +28,21 @@ def upgrade() -> None: sa.Column("path_node_ids", postgresql.ARRAY(postgresql.UUID()), nullable=False), sa.Column("path_length", sa.Integer(), nullable=False), sa.Column("min_confidence", sa.Float(), nullable=False, server_default=sa.text("1.0")), - sa.Column("relationship_types", postgresql.ARRAY(sa.Text()), nullable=False, - server_default=sa.text("ARRAY[]::text[]")), - sa.Column("computed_at", sa.DateTime(timezone=True), nullable=False, - server_default=sa.text("now()")), + sa.Column( + "relationship_types", postgresql.ARRAY(sa.Text()), nullable=False, server_default=sa.text("ARRAY[]::text[]") + ), + sa.Column("computed_at", sa.DateTime(timezone=True), nullable=False, server_default=sa.text("now()")), sa.ForeignKeyConstraint( - ["source_node_id"], ["graph_nodes.node_id"], - name="attack_paths_source_fkey", ondelete="CASCADE", + ["source_node_id"], + ["graph_nodes.node_id"], + name="attack_paths_source_fkey", + ondelete="CASCADE", ), sa.ForeignKeyConstraint( - ["target_node_id"], ["graph_nodes.node_id"], - name="attack_paths_target_fkey", ondelete="CASCADE", + ["target_node_id"], + ["graph_nodes.node_id"], + name="attack_paths_target_fkey", + ondelete="CASCADE", ), sa.PrimaryKeyConstraint("path_id", name="attack_paths_pkey"), ) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 432d7475..3969fdcd 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -70,24 +70,33 @@ def get_attack_graph(): conn = _conn() with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: - node_filter = "WHERE n.tenant_id = %(tenant_id)s" - params: dict = {"tenant_id": tenant_id, "limit": limit} if subscription_id: - node_filter += " AND n.subscription_id = %(subscription_id)s" - params["subscription_id"] = subscription_id - - cur.execute( - f""" - SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, - n.location, n.resource_group, n.subscription_id, n.snapshot_id, - n.updated_at - FROM graph_nodes n - {node_filter} - ORDER BY n.updated_at DESC - LIMIT %(limit)s - """, - params, - ) + cur.execute( + """ + SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, + n.location, n.resource_group, n.subscription_id, n.snapshot_id, + n.updated_at + FROM graph_nodes n + WHERE n.tenant_id = %(tenant_id)s + AND n.subscription_id = %(subscription_id)s + ORDER BY n.updated_at DESC + LIMIT %(limit)s + """, + {"tenant_id": tenant_id, "subscription_id": subscription_id, "limit": limit}, + ) + else: + cur.execute( + """ + SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, + n.location, n.resource_group, n.subscription_id, n.snapshot_id, + n.updated_at + FROM graph_nodes n + WHERE n.tenant_id = %(tenant_id)s + ORDER BY n.updated_at DESC + LIMIT %(limit)s + """, + {"tenant_id": tenant_id, "limit": limit}, + ) nodes = cur.fetchall() node_ids = [row["node_id"] for row in nodes] diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index 7bbf87ae..442176f3 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -3,6 +3,7 @@ Computes shortest paths from every finding-linked node to every reachable node and persists them in attack_paths for API consumption. """ + from __future__ import annotations import logging diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index 01918d17..dd1ae07e 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -1,4 +1,5 @@ """Unit tests for the attack graph API routes.""" + import time from unittest.mock import MagicMock, patch diff --git a/tests/test_graph_path_traversal.py b/tests/test_graph_path_traversal.py index b46a8c9f..7f3b8e06 100644 --- a/tests/test_graph_path_traversal.py +++ b/tests/test_graph_path_traversal.py @@ -1,4 +1,5 @@ """Unit tests for BFS path traversal (scanner/graph/path_traversal.py).""" + from unittest.mock import MagicMock, patch from scanner.graph.path_traversal import _bfs_from, compute_attack_paths From a3dc77613c2b9343863e540b96aac48373924587 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 27 Sep 2026 11:37:42 +0100 Subject: [PATCH 06/21] fix(graph): bidirectional BFS traversal and AND filter for attack-graph edge query Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 2 +- scanner/graph/path_traversal.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 3969fdcd..4289c0e7 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -108,7 +108,7 @@ def get_attack_graph(): e.relationship_type, e.confidence, e.evidence_source, e.collected_at FROM graph_edges e WHERE e.source_node_id = ANY(%(node_ids)s::uuid[]) - OR e.target_node_id = ANY(%(node_ids)s::uuid[]) + AND e.target_node_id = ANY(%(node_ids)s::uuid[]) """, {"node_ids": node_ids}, ) diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index 442176f3..f2f298d4 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -37,6 +37,10 @@ def _load_adjacency(conn: Any, tenant_id: str) -> dict[str, list[tuple[str, str, adj: dict[str, list[tuple[str, str, float]]] = {} for src, tgt, rel, conf in cur.fetchall(): adj.setdefault(src, []).append((tgt, rel, conf)) + # Add reverse direction so BFS from a finding can reach nodes that + # point TO it (e.g. PublicIP -EXPOSES-> VM: starting from the flagged + # VM can now reach the PublicIP that exposes it). + adj.setdefault(tgt, []).append((src, rel + "_REV", conf)) return adj From 18421081de14e5d55706f4b1821d4250950018fc Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 27 Sep 2026 23:11:47 +0100 Subject: [PATCH 07/21] fix(graph): remove double populate_graph, scope BFS reversal, fix path count, use DatabaseManager - Remove populate_graph call from ScanEngine.run_scan(); worker.py is the authoritative post-save call site. Running it inside run_scan() fires before findings are persisted and duplicates the population on every scan. - Restrict _REV adjacency edges to EXPOSES and HAS_IDENTITY only. Reversing MEMBER_OF would connect any two VMs sharing a subnet through the subnet node, producing spurious lateral-movement paths with no real attack vector. - _write_paths: return cur.rowcount instead of len(rows). ON CONFLICT DO NOTHING silently drops duplicate insertions; len(rows) overcounts whereas rowcount reflects actual rows written. - Replace raw psycopg2.connect in attack_graph routes with DatabaseManager to use the shared connection pool and match the pattern used by all other routes. - Remove unused MagicMock import (ruff F401). Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 27 +++++++++++++-------------- scanner/engine.py | 9 --------- scanner/graph/path_traversal.py | 13 ++++++++----- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 4289c0e7..ec97b200 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -3,10 +3,10 @@ import logging import os -import psycopg2 import psycopg2.extras from flask import Blueprint, g, jsonify, request +from api.models.finding import DatabaseManager from api.validation import ValidationError, positive_integer, uuid_string attack_graph_bp = Blueprint("attack_graph", __name__) @@ -16,12 +16,11 @@ _MAX_LIMIT = 500 -def _conn(): - if "graph_conn" not in g: - g.graph_conn = psycopg2.connect(os.environ["DATABASE_URL"]) - g.graph_conn.autocommit = True - psycopg2.extras.register_uuid(g.graph_conn) - return g.graph_conn +def _get_db() -> DatabaseManager: + if "db" not in g: + g.db = DatabaseManager(os.environ["DATABASE_URL"]) + g.db.connect() + return g.db def _tenant_id() -> str | None: @@ -44,10 +43,10 @@ def _tenant_id() -> str | None: @attack_graph_bp.teardown_app_request -def _close_conn(exc): - conn = g.pop("graph_conn", None) - if conn is not None: - conn.close() +def _close_db(exc): + db = g.pop("db", None) + if db is not None: + db.close() @attack_graph_bp.get("/api/attack-graph") @@ -68,7 +67,7 @@ def get_attack_graph(): if not tenant_id: return jsonify({"error": "tenant_id not available"}), 400 - conn = _conn() + conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: if subscription_id: cur.execute( @@ -138,7 +137,7 @@ def list_attack_paths(): if not tenant_id: return jsonify({"error": "tenant_id not available"}), 400 - conn = _conn() + conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: cur.execute( """ @@ -174,7 +173,7 @@ def get_attack_path(path_id: str): if not tenant_id: return jsonify({"error": "tenant_id not available"}), 400 - conn = _conn() + conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: cur.execute( """ diff --git a/scanner/engine.py b/scanner/engine.py index df72b68e..4a88b4f9 100644 --- a/scanner/engine.py +++ b/scanner/engine.py @@ -13,7 +13,6 @@ from openshield.severity import CONTRACT_VERSION, SeverityContractError, normalize_severity, score_findings from scanner.azure_client import AzureClient from scanner.evaluation import EvaluationStatus, RuleEvaluation, subscription_scope_id -from scanner.graph.graph_populator import populate_graph from scanner.graph.snapshot_bridge import collect_snapshot logger = logging.getLogger(__name__) @@ -239,14 +238,6 @@ def run_scan(self, scan_id: Optional[str] = None) -> Dict[str, Any]: logger.info("Scan %s complete — %d total finding(s). Normalising results...", scan_id, len(findings)) - # Populate the attack graph as a non-fatal post-scan step. - dsn = os.environ.get("DATABASE_URL") - if snapshot and dsn: - try: - populate_graph(scan_id, snapshot, dsn) - except Exception as exc: - logger.warning("run_scan: graph population raised unexpectedly: %s", exc) - return make_serializable(result) def _evaluate_rule(self, rule: Any, rule_id: str) -> List[RuleEvaluation]: diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index f2f298d4..9355ae1e 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -34,13 +34,16 @@ def _load_adjacency(conn: Any, tenant_id: str) -> dict[str, list[tuple[str, str, """, {"tenant_id": tenant_id}, ) + # Only traverse in reverse for relationships where the reverse direction + # is semantically meaningful. MEMBER_OF and PROTECTS are not reversed: + # reversing MEMBER_OF would let any two VMs in the same subnet reach + # each other via the subnet node, producing spurious lateral-movement paths. + _REVERSE_RELS = {"EXPOSES", "HAS_IDENTITY"} adj: dict[str, list[tuple[str, str, float]]] = {} for src, tgt, rel, conf in cur.fetchall(): adj.setdefault(src, []).append((tgt, rel, conf)) - # Add reverse direction so BFS from a finding can reach nodes that - # point TO it (e.g. PublicIP -EXPOSES-> VM: starting from the flagged - # VM can now reach the PublicIP that exposes it). - adj.setdefault(tgt, []).append((src, rel + "_REV", conf)) + if rel in _REVERSE_RELS: + adj.setdefault(tgt, []).append((src, rel + "_REV", conf)) return adj @@ -132,7 +135,7 @@ def _write_paths( rows, template="(%s, %s, %s, %s::uuid, %s::uuid, %s::uuid[], %s, %s, %s)", ) - return len(rows) + return cur.rowcount def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: From 55a90b0392a5ba9dee3ce7dbf2eeb0fbfdb8a11c Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 28 Sep 2026 00:09:35 +0100 Subject: [PATCH 08/21] fix(graph): add subnet-bridging test, return 403 for shared-secret callers, document retention - tests/test_graph_path_traversal.py: add test asserting MEMBER_OF edges are not reversed (two VMs in one subnet must not be reachable from each other through the subnet node). Add test for _load_adjacency verifying EXPOSES gets a _REV edge but MEMBER_OF does not. - api/routes/attack_graph.py: return 403 instead of 400 when tenant_id is absent in shared-secret mode. The request is well-formed; the auth method is insufficient. Error message now explains OIDC is required. - docs/api-reference.md: document the OIDC-only requirement for attack graph endpoints and note the attack_paths retention follow-up (issue #333). Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 15 +++++++++--- docs/api-reference.md | 14 +++++++++++ tests/test_graph_path_traversal.py | 39 +++++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index ec97b200..1d653c27 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -65,7 +65,10 @@ def get_attack_graph(): tenant_id = _tenant_id() if not tenant_id: - return jsonify({"error": "tenant_id not available"}), 400 + # Shared-secret callers have no tenant claim; OIDC is required for + # tenant-scoped graph endpoints. Return 403 (not 400) since the + # request is well-formed but the auth method is insufficient. + return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: @@ -135,7 +138,10 @@ def list_attack_paths(): tenant_id = _tenant_id() if not tenant_id: - return jsonify({"error": "tenant_id not available"}), 400 + # Shared-secret callers have no tenant claim; OIDC is required for + # tenant-scoped graph endpoints. Return 403 (not 400) since the + # request is well-formed but the auth method is insufficient. + return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: @@ -171,7 +177,10 @@ def get_attack_path(path_id: str): tenant_id = _tenant_id() if not tenant_id: - return jsonify({"error": "tenant_id not available"}), 400 + # Shared-secret callers have no tenant claim; OIDC is required for + # tenant-scoped graph endpoints. Return 403 (not 400) since the + # request is well-formed but the auth method is insufficient. + return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: diff --git a/docs/api-reference.md b/docs/api-reference.md index 7192e140..c60526e3 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -697,3 +697,17 @@ The following endpoints are called by the frontend but have no backend implement | Endpoint | Used by | Status | |---|---|---| | `GET /api/monitoring` | Monitoring page — score trend chart, category distribution | Deferred. Score and findings data come from `GET /api/score` and `GET /api/findings` instead. | + +--- + +## Attack graph endpoints + +`GET /api/attack-graph`, `GET /api/attack-paths`, and `GET /api/attack-paths/` expose the attack graph computed from the most recent scan for the caller's tenant. + +### Authentication requirement + +These endpoints are **OIDC-only**. In `shared_secret` mode no tenant claim is present in the token, so the endpoints return `403 {"error": "tenant_id not available; OIDC authentication required"}`. This is intentional: the graph is strictly scoped per tenant and there is no safe way to infer a tenant from a shared-secret token. + +### Attack-path retention + +Attack paths are written per scan and are not automatically pruned. In long-running deployments, old scan paths accumulate in `attack_paths`. Retention policy (e.g. keep only the N most recent scans per tenant) is tracked in issue #333 and will be addressed in a follow-up. diff --git a/tests/test_graph_path_traversal.py b/tests/test_graph_path_traversal.py index 7f3b8e06..969cf899 100644 --- a/tests/test_graph_path_traversal.py +++ b/tests/test_graph_path_traversal.py @@ -2,7 +2,7 @@ from unittest.mock import MagicMock, patch -from scanner.graph.path_traversal import _bfs_from, compute_attack_paths +from scanner.graph.path_traversal import _bfs_from, _load_adjacency, compute_attack_paths def test_bfs_from_returns_empty_for_isolated_node(): @@ -46,6 +46,43 @@ def test_bfs_from_does_not_revisit_nodes(): assert target_nodes.count("C") == 1 +def test_member_of_not_reversed_prevents_subnet_bridging(): + # Two VMs sharing a subnet. MEMBER_OF is forward-only (VM -> subnet), + # so BFS from vm-a must not reach vm-b through the subnet. + adj = { + "vm-a": [("subnet-1", "MEMBER_OF", 0.9)], + "vm-b": [("subnet-1", "MEMBER_OF", 0.9)], + # subnet-1 has no outgoing edges: MEMBER_OF is not reversed + } + result = _bfs_from("vm-a", adj) + target_ids = {r["target_node_id"] for r in result} + assert "vm-b" not in target_ids + assert "subnet-1" in target_ids # forward hop is still present + + +def test_load_adjacency_reverses_exposes_not_member_of(): + conn = MagicMock() + cur = MagicMock() + cur.__enter__ = lambda s: s + cur.__exit__ = MagicMock(return_value=False) + conn.cursor.return_value = cur + # One EXPOSES edge and one MEMBER_OF edge + cur.fetchall.return_value = [ + ("pub-ip", "vm-1", "EXPOSES", 1.0), + ("vm-1", "subnet-1", "MEMBER_OF", 0.9), + ] + + adj = _load_adjacency(conn, "tenant-x") + + # EXPOSES should have a reverse entry (vm-1 -> pub-ip via EXPOSES_REV) + rev_exposes = [n for n, rel, _ in adj.get("vm-1", []) if rel == "EXPOSES_REV"] + assert rev_exposes, "EXPOSES_REV edge missing from vm-1" + + # MEMBER_OF should NOT have a reverse entry (subnet-1 should not point to vm-1) + rev_member = [n for n, rel, _ in adj.get("subnet-1", []) if rel == "MEMBER_OF_REV"] + assert not rev_member, "MEMBER_OF_REV edge should not exist (spurious bridging)" + + def test_compute_attack_paths_returns_zero_when_no_finding_nodes(): with patch("scanner.graph.path_traversal.psycopg2.connect") as mock_connect: conn = MagicMock() From 3f44cbdc78b48835085602b1967c62463652e3df Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Mon, 28 Sep 2026 01:18:26 +0100 Subject: [PATCH 09/21] fix(lint+test): remove unused os import, update 400->403 assertion, fix stacked admission test - scanner/engine.py: remove 'import os' now unused after populate_graph was removed from run_scan(). - tests/test_attack_graph_api.py: update viewer-token test to expect 403 (auth method insufficient) instead of 400 (bad request) to match the updated response for shared-secret callers without a tenant claim. - tests/test_scan_admission_migration_postgres.py: resolve _HEAD dynamically via ScriptDirectory.get_current_head() so stacked migration tests do not pin a hardcoded revision. Signed-off-by: Tanvir Farhad --- scanner/engine.py | 1 - tests/test_attack_graph_api.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/scanner/engine.py b/scanner/engine.py index 4a88b4f9..5bf08da9 100644 --- a/scanner/engine.py +++ b/scanner/engine.py @@ -3,7 +3,6 @@ import importlib.util import inspect import logging -import os import uuid from datetime import datetime, timezone from pathlib import Path diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index dd1ae07e..bff52966 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -78,7 +78,7 @@ def test_get_attack_graph_viewer_cannot_supply_tenant_header(client, app): "X-Tenant-Id": _TENANT, } resp = client.get("/api/attack-graph", headers=headers) - assert resp.status_code == 400 + assert resp.status_code == 403 def test_list_attack_paths_missing_scan_id_returns_400(client, tenant_auth_headers): From 6ef0c9af379f366001e9f4fa73c49bc72fac1f5a Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 1 Oct 2026 00:39:58 +0100 Subject: [PATCH 10/21] fix(graph): return attempted path count instead of rowcount from execute_values psycopg2 execute_values returns -1 for rowcount when ON CONFLICT DO NOTHING is used, making compute_attack_paths always report 0 paths written to logs. Return len(rows) (paths attempted) instead so the log line is meaningful. Signed-off-by: Tanvir Farhad --- scanner/graph/path_traversal.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index 9355ae1e..9501e634 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -135,7 +135,9 @@ def _write_paths( rows, template="(%s, %s, %s, %s::uuid, %s::uuid, %s::uuid[], %s, %s, %s)", ) - return cur.rowcount + # execute_values returns -1 for rowcount with ON CONFLICT DO NOTHING; + # return the number of rows attempted instead. + return len(rows) def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: From 8929d3e3d7847c310b305729b3ca26424bb1c796 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 4 Oct 2026 02:44:22 +0100 Subject: [PATCH 11/21] fix(graph): fix rowcount from execute_values, add path retention, wrap DB exceptions in routes Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 204 +++++++++++++++++--------------- scanner/graph/path_traversal.py | 33 +++++- 2 files changed, 138 insertions(+), 99 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 1d653c27..d7774b2c 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -70,51 +70,55 @@ def get_attack_graph(): # request is well-formed but the auth method is insufficient. return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 - conn = _get_db().conn - with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: - if subscription_id: - cur.execute( - """ - SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, - n.location, n.resource_group, n.subscription_id, n.snapshot_id, - n.updated_at - FROM graph_nodes n - WHERE n.tenant_id = %(tenant_id)s - AND n.subscription_id = %(subscription_id)s - ORDER BY n.updated_at DESC - LIMIT %(limit)s - """, - {"tenant_id": tenant_id, "subscription_id": subscription_id, "limit": limit}, - ) - else: - cur.execute( - """ - SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, - n.location, n.resource_group, n.subscription_id, n.snapshot_id, - n.updated_at - FROM graph_nodes n - WHERE n.tenant_id = %(tenant_id)s - ORDER BY n.updated_at DESC - LIMIT %(limit)s - """, - {"tenant_id": tenant_id, "limit": limit}, - ) - nodes = cur.fetchall() - - node_ids = [row["node_id"] for row in nodes] - edges: list = [] - if node_ids: - cur.execute( - """ - SELECT e.edge_id::text, e.source_node_id::text, e.target_node_id::text, - e.relationship_type, e.confidence, e.evidence_source, e.collected_at - FROM graph_edges e - WHERE e.source_node_id = ANY(%(node_ids)s::uuid[]) - AND e.target_node_id = ANY(%(node_ids)s::uuid[]) - """, - {"node_ids": node_ids}, - ) - edges = cur.fetchall() + try: + conn = _get_db().conn + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + if subscription_id: + cur.execute( + """ + SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, + n.location, n.resource_group, n.subscription_id, n.snapshot_id, + n.updated_at + FROM graph_nodes n + WHERE n.tenant_id = %(tenant_id)s + AND n.subscription_id = %(subscription_id)s + ORDER BY n.updated_at DESC + LIMIT %(limit)s + """, + {"tenant_id": tenant_id, "subscription_id": subscription_id, "limit": limit}, + ) + else: + cur.execute( + """ + SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, + n.location, n.resource_group, n.subscription_id, n.snapshot_id, + n.updated_at + FROM graph_nodes n + WHERE n.tenant_id = %(tenant_id)s + ORDER BY n.updated_at DESC + LIMIT %(limit)s + """, + {"tenant_id": tenant_id, "limit": limit}, + ) + nodes = cur.fetchall() + + node_ids = [row["node_id"] for row in nodes] + edges: list = [] + if node_ids: + cur.execute( + """ + SELECT e.edge_id::text, e.source_node_id::text, e.target_node_id::text, + e.relationship_type, e.confidence, e.evidence_source, e.collected_at + FROM graph_edges e + WHERE e.source_node_id = ANY(%(node_ids)s::uuid[]) + AND e.target_node_id = ANY(%(node_ids)s::uuid[]) + """, + {"node_ids": node_ids}, + ) + edges = cur.fetchall() + except Exception: + logger.exception("get_attack_graph failed for tenant %s", tenant_id) + return jsonify({"error": "internal server error"}), 500 return jsonify({"nodes": [dict(r) for r in nodes], "edges": [dict(r) for r in edges]}) @@ -143,26 +147,30 @@ def list_attack_paths(): # request is well-formed but the auth method is insufficient. return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 - conn = _get_db().conn - with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: - cur.execute( - """ - SELECT ap.path_id::text, ap.source_node_id::text, ap.target_node_id::text, - ap.path_node_ids, ap.path_length, ap.min_confidence, - ap.relationship_types, ap.computed_at, - src.resource_type AS source_type, src.name AS source_name, - tgt.resource_type AS target_type, tgt.name AS target_name - FROM attack_paths ap - JOIN graph_nodes src ON src.node_id = ap.source_node_id - JOIN graph_nodes tgt ON tgt.node_id = ap.target_node_id - WHERE ap.scan_id = %(scan_id)s - AND ap.tenant_id = %(tenant_id)s - ORDER BY ap.path_length ASC, ap.min_confidence DESC - LIMIT %(limit)s - """, - {"scan_id": scan_id, "tenant_id": tenant_id, "limit": limit}, - ) - rows = cur.fetchall() + try: + conn = _get_db().conn + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + cur.execute( + """ + SELECT ap.path_id::text, ap.source_node_id::text, ap.target_node_id::text, + ap.path_node_ids, ap.path_length, ap.min_confidence, + ap.relationship_types, ap.computed_at, + src.resource_type AS source_type, src.name AS source_name, + tgt.resource_type AS target_type, tgt.name AS target_name + FROM attack_paths ap + JOIN graph_nodes src ON src.node_id = ap.source_node_id + JOIN graph_nodes tgt ON tgt.node_id = ap.target_node_id + WHERE ap.scan_id = %(scan_id)s + AND ap.tenant_id = %(tenant_id)s + ORDER BY ap.path_length ASC, ap.min_confidence DESC + LIMIT %(limit)s + """, + {"scan_id": scan_id, "tenant_id": tenant_id, "limit": limit}, + ) + rows = cur.fetchall() + except Exception: + logger.exception("list_attack_paths failed for scan %s", scan_id) + return jsonify({"error": "internal server error"}), 500 return jsonify({"scan_id": scan_id, "paths": [dict(r) for r in rows]}) @@ -182,42 +190,46 @@ def get_attack_path(path_id: str): # request is well-formed but the auth method is insufficient. return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 - conn = _get_db().conn - with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: - cur.execute( - """ - SELECT ap.path_id::text, ap.scan_id, ap.source_node_id::text, - ap.target_node_id::text, ap.path_node_ids, ap.path_length, - ap.min_confidence, ap.relationship_types, ap.computed_at - FROM attack_paths ap - WHERE ap.path_id = %(path_id)s::uuid - AND ap.tenant_id = %(tenant_id)s - """, - {"path_id": path_id, "tenant_id": tenant_id}, - ) - row = cur.fetchone() - - if row is None: - return jsonify({"error": "not found"}), 404 - - path = dict(row) - - # Fetch full node detail for each hop - node_ids = [str(nid) for nid in path["path_node_ids"]] - if node_ids: + try: + conn = _get_db().conn with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: cur.execute( """ - SELECT node_id::text, resource_id, resource_type, name, - location, resource_group, subscription_id - FROM graph_nodes - WHERE node_id = ANY(%(ids)s::uuid[]) - AND tenant_id = %(tenant_id)s + SELECT ap.path_id::text, ap.scan_id, ap.source_node_id::text, + ap.target_node_id::text, ap.path_node_ids, ap.path_length, + ap.min_confidence, ap.relationship_types, ap.computed_at + FROM attack_paths ap + WHERE ap.path_id = %(path_id)s::uuid + AND ap.tenant_id = %(tenant_id)s """, - {"ids": node_ids, "tenant_id": tenant_id}, + {"path_id": path_id, "tenant_id": tenant_id}, ) - nodes_by_id = {r["node_id"]: dict(r) for r in cur.fetchall()} - path["hops"] = [nodes_by_id.get(str(nid), {"node_id": str(nid)}) for nid in path["path_node_ids"]] + row = cur.fetchone() + + if row is None: + return jsonify({"error": "not found"}), 404 + + path = dict(row) + + # Fetch full node detail for each hop + node_ids = [str(nid) for nid in path["path_node_ids"]] + if node_ids: + with conn.cursor(cursor_factory=psycopg2.extras.RealDictCursor) as cur: + cur.execute( + """ + SELECT node_id::text, resource_id, resource_type, name, + location, resource_group, subscription_id + FROM graph_nodes + WHERE node_id = ANY(%(ids)s::uuid[]) + AND tenant_id = %(tenant_id)s + """, + {"ids": node_ids, "tenant_id": tenant_id}, + ) + nodes_by_id = {r["node_id"]: dict(r) for r in cur.fetchall()} + path["hops"] = [nodes_by_id.get(str(nid), {"node_id": str(nid)}) for nid in path["path_node_ids"]] + except Exception: + logger.exception("get_attack_path failed for path_id %s", path_id) + return jsonify({"error": "internal server error"}), 500 path["path_node_ids"] = [str(nid) for nid in path["path_node_ids"]] return jsonify(path) diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index 9501e634..8660241d 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -99,6 +99,30 @@ def _bfs_from( return paths +def _delete_stale_paths(conn: Any, scan_id: str, tenant_id: str) -> None: + """Delete attack paths from previous scans for the same subscription, keeping only the current scan.""" + with conn.cursor() as cur: + cur.execute( + """ + DELETE FROM attack_paths + WHERE tenant_id = %(tenant_id)s + AND scan_id != %(scan_id)s + AND scan_id IN ( + SELECT DISTINCT ap2.scan_id + FROM attack_paths ap2 + JOIN findings f ON f.scan_id = ap2.scan_id + WHERE ap2.tenant_id = %(tenant_id)s + AND f.subscription_id = ( + SELECT subscription_id FROM findings + WHERE scan_id = %(scan_id)s + LIMIT 1 + ) + ) + """, + {"tenant_id": tenant_id, "scan_id": scan_id}, + ) + + def _write_paths( conn: Any, scan_id: str, @@ -135,9 +159,10 @@ def _write_paths( rows, template="(%s, %s, %s, %s::uuid, %s::uuid, %s::uuid[], %s, %s, %s)", ) - # execute_values returns -1 for rowcount with ON CONFLICT DO NOTHING; - # return the number of rows attempted instead. - return len(rows) + inserted = cur.rowcount + # cur.rowcount reflects actual inserts after ON CONFLICT DO NOTHING; + # fall back to attempted count if the driver reports -1. + return inserted if inserted >= 0 else len(rows) def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: @@ -146,10 +171,12 @@ def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: conn = psycopg2.connect(dsn) conn.autocommit = False try: + _delete_stale_paths(conn, scan_id, tenant_id) adj = _load_adjacency(conn, tenant_id) source_nodes = _load_finding_nodes(conn, scan_id, tenant_id) if not source_nodes: logger.info("graph path traversal: no finding-linked nodes for scan %s", scan_id) + conn.commit() return 0 total = 0 From 901b2a7138410ced244257ddff5a37db8555f0bc Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 4 Oct 2026 11:55:45 +0100 Subject: [PATCH 12/21] fix(tests): remove leftover merge-conflict marker in integration test Signed-off-by: Tanvir Farhad --- tests/test_graph_engine_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_graph_engine_integration.py b/tests/test_graph_engine_integration.py index b69c9a50..d68869f0 100644 --- a/tests/test_graph_engine_integration.py +++ b/tests/test_graph_engine_integration.py @@ -68,7 +68,6 @@ def test_run_scan_existing_rule_without_snapshot_param_still_works(engine): snapshot = _make_snapshot() def legacy_scan(client, subscription_id): -<<<<<<< HEAD return [ { "rule_id": "AZ-LEGACY-001", From cb4c40d314c9d13bafd1768a91c4bbbac62d6bbd Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 4 Oct 2026 14:26:37 +0100 Subject: [PATCH 13/21] fix: correct teardown scope, int coerce limit param, module-level _REVERSE_RELS, safe logger.error Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 6 +++--- scanner/graph/path_traversal.py | 10 +++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index d7774b2c..76f3db0c 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -42,7 +42,7 @@ def _tenant_id() -> str | None: return None -@attack_graph_bp.teardown_app_request +@attack_graph_bp.teardown_request def _close_db(exc): db = g.pop("db", None) if db is not None: @@ -56,7 +56,7 @@ def get_attack_graph(): Query params: subscription_id (optional), limit (default 100, max 500) """ try: - limit = positive_integer(request.args.get("limit", _DEFAULT_LIMIT), "limit") + limit = positive_integer(int(request.args.get("limit", _DEFAULT_LIMIT)), "limit") if limit > _MAX_LIMIT: limit = _MAX_LIMIT subscription_id = request.args.get("subscription_id") @@ -134,7 +134,7 @@ def list_attack_paths(): return jsonify({"error": "scan_id is required"}), 400 try: scan_id = uuid_string(scan_id, "scan_id") - limit = positive_integer(request.args.get("limit", _DEFAULT_LIMIT), "limit") + limit = positive_integer(int(request.args.get("limit", _DEFAULT_LIMIT)), "limit") if limit > _MAX_LIMIT: limit = _MAX_LIMIT except ValidationError as exc: diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index 8660241d..0e8c1e4f 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -20,6 +20,11 @@ # keeping them would inflate the table for large graphs. _MAX_PATH_LENGTH = 8 +# Relationships whose edges are also traversed in reverse during BFS. +# MEMBER_OF and PROTECTS are excluded: reversing MEMBER_OF would let any two +# VMs in the same subnet reach each other via the subnet node. +_REVERSE_RELS: frozenset[str] = frozenset({"EXPOSES", "HAS_IDENTITY"}) + def _load_adjacency(conn: Any, tenant_id: str) -> dict[str, list[tuple[str, str, float]]]: """Return {source_node_id: [(target_node_id, relationship_type, confidence)]}.""" @@ -38,7 +43,6 @@ def _load_adjacency(conn: Any, tenant_id: str) -> dict[str, list[tuple[str, str, # is semantically meaningful. MEMBER_OF and PROTECTS are not reversed: # reversing MEMBER_OF would let any two VMs in the same subnet reach # each other via the subnet node, producing spurious lateral-movement paths. - _REVERSE_RELS = {"EXPOSES", "HAS_IDENTITY"} adj: dict[str, list[tuple[str, str, float]]] = {} for src, tgt, rel, conf in cur.fetchall(): adj.setdefault(src, []).append((tgt, rel, conf)) @@ -192,6 +196,6 @@ def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: raise finally: conn.close() - except Exception as exc: - logger.error("graph path traversal failed for scan %s: %s", scan_id, exc) + except Exception: + logger.error("graph path traversal failed for scan %s", scan_id, exc_info=True) return 0 From 00a8fdbcb6806e53b571a5d36ddd6561b4957701 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 8 Oct 2026 14:10:27 +0100 Subject: [PATCH 14/21] fix(graph-api): reject malformed limits and describe admin tenant fallback Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 6 ++--- docs/api-reference.md | 48 +++++++++++++++++----------------- tests/test_attack_graph_api.py | 24 +++++++++++++++++ 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 76f3db0c..66a3f266 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -60,7 +60,7 @@ def get_attack_graph(): if limit > _MAX_LIMIT: limit = _MAX_LIMIT subscription_id = request.args.get("subscription_id") - except ValidationError as exc: + except (ValidationError, ValueError) as exc: return jsonify({"error": str(exc)}), 400 tenant_id = _tenant_id() @@ -137,7 +137,7 @@ def list_attack_paths(): limit = positive_integer(int(request.args.get("limit", _DEFAULT_LIMIT)), "limit") if limit > _MAX_LIMIT: limit = _MAX_LIMIT - except ValidationError as exc: + except (ValidationError, ValueError) as exc: return jsonify({"error": str(exc)}), 400 tenant_id = _tenant_id() @@ -180,7 +180,7 @@ def get_attack_path(path_id: str): """Return a single attack path with full node detail for each hop.""" try: path_id = uuid_string(path_id, "path_id") - except ValidationError as exc: + except (ValidationError, ValueError) as exc: return jsonify({"error": str(exc)}), 400 tenant_id = _tenant_id() diff --git a/docs/api-reference.md b/docs/api-reference.md index c60526e3..9a7b2464 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -7,7 +7,7 @@ routes become public only when the deliberate demo-mode setting is enabled. ## Authentication -`/`, `/health`, `/ready`, and `/metrics` are always public. All other routes — including all `/api/*` GET endpoints — require an `Authorization: Bearer ` header, verified by `api/auth.py` according to `OPENSHIELD_AUTH_MODE`: +`/`, `/health`, `/ready`, and `/metrics` are always public. All other routes — including all `/api/*` GET endpoints — require an `Authorization: Bearer ` header, verified by `api/auth.py` according to `OPENSHIELD_AUTH_MODE`: | Mode | Verification | Role source | Intended for | |---|---|---|---| @@ -27,7 +27,7 @@ The dashboard never embeds a token (see [authentication and containment](securit ### Subscription authorization -`POST /api/scans/trigger` also checks `subscription_id` against `OPENSHIELD_AUTHORIZED_SUBSCRIPTIONS`, a comma-separated allowlist. A valid `operator`/`admin` token can otherwise trigger a scan against *any* subscription_id — role alone doesn't say which subscription a caller is entitled to. Left unset, every subscription_id is accepted (matches historical behavior); the API logs a loud startup warning when it's unset. This is a single-tenant containment boundary, not a substitute for real per-tenant authorization — see issue #294 for the remaining tenant-ownership scope this is a stopgap for. +`POST /api/scans/trigger` also checks `subscription_id` against `OPENSHIELD_AUTHORIZED_SUBSCRIPTIONS`, a comma-separated allowlist. A valid `operator`/`admin` token can otherwise trigger a scan against *any* subscription_id — role alone doesn't say which subscription a caller is entitled to. Left unset, every subscription_id is accepted (matches historical behavior); the API logs a loud startup warning when it's unset. This is a single-tenant containment boundary, not a substitute for real per-tenant authorization — see issue #294 for the remaining tenant-ownership scope this is a stopgap for. ## Input limits @@ -185,7 +185,7 @@ Example response: Returns the details and current status of a specific scan. -Path parameters: `scan_id` — UUID of the scan. +Path parameters: `scan_id` — UUID of the scan. Example response: @@ -208,7 +208,7 @@ Example response: Admits an asynchronous scan against the configured subscription. Execution happens in a background worker process; the response returns as soon as the scan is durably recorded. -Request body (optional — falls back to `AZURE_SUBSCRIPTION_ID`): +Request body (optional — falls back to `AZURE_SUBSCRIPTION_ID`): ```json { @@ -221,7 +221,7 @@ Request body (optional — falls back to `AZURE_SUBSCRIPTION_ID`): Admission is serialized per subscription and enforced by the database, so concurrent and replayed triggers converge on one logical scan rather than creating duplicates: - **At most one active scan per subscription.** While a `pending` or `running` scan exists, a further trigger returns that existing scan instead of queueing another. -- **`Idempotency-Key` (optional request header, 1–200 characters).** A repeat of the same key for the same subscription returns the original scan. The key is scoped to the subscription; the same key under a different subscription is a different request. A trigger carries no request input other than `subscription_id`, so a key that resolves to an existing scan is always a replay of the same logical request and there is no changed-payload conflict to report. +- **`Idempotency-Key` (optional request header, 1–200 characters).** A repeat of the same key for the same subscription returns the original scan. The key is scoped to the subscription; the same key under a different subscription is a different request. A trigger carries no request input other than `subscription_id`, so a key that resolves to an existing scan is always a replay of the same logical request and there is no changed-payload conflict to report. - **`OPENSHIELD_MAX_SCANS_PER_SUBSCRIPTION_PER_HOUR`** adds an optional hourly admission quota. Unset or `0` (the default) applies no time-window limit; the one-active-scan rule still applies. ### Responses @@ -229,8 +229,8 @@ Admission is serialized per subscription and enforced by the database, so concur | Status | When | Body | | --- | --- | --- | | `202 Accepted` | A new scan was admitted and queued. | `scan_id`, `status: "pending"`, `message` | -| `200 OK` | The request resolved to an existing logical scan — an `Idempotency-Key` replay of the same request, or a trigger while a scan is already active for the subscription. | `scan_id`, `status` (the existing scan's `pending`/`running`), `message: "Existing logical scan returned."` | -| `400 Bad Request` | Malformed body, invalid `subscription_id`, missing subscription, or an `Idempotency-Key` outside 1–200 characters. | `error` | +| `200 OK` | The request resolved to an existing logical scan — an `Idempotency-Key` replay of the same request, or a trigger while a scan is already active for the subscription. | `scan_id`, `status` (the existing scan's `pending`/`running`), `message: "Existing logical scan returned."` | +| `400 Bad Request` | Malformed body, invalid `subscription_id`, missing subscription, or an `Idempotency-Key` outside 1–200 characters. | `error` | | `403 Forbidden` | `subscription_id` is not on the `OPENSHIELD_AUTHORIZED_SUBSCRIPTIONS` allowlist. | `error` | | `429 Too Many Requests` | The configured hourly quota for this subscription is exhausted. | `error: "Scan quota exceeded for this subscription."` | @@ -266,7 +266,7 @@ Missing subscription response (`400`): ## POST /api/scans/<scan_id>/enrich -Queues durable CVE enrichment for a completed scan's findings. Enrichment runs as a database-backed job claimed by the background worker — the request never owns a thread, so the work survives an API restart. +Queues durable CVE enrichment for a completed scan's findings. Enrichment runs as a database-backed job claimed by the background worker — the request never owns a thread, so the work survives an API restart. There is **never more than one enrichment job per scan**. Repeat calls are safe: they report the state of the single job rather than creating another. @@ -280,11 +280,11 @@ Every response carries `scan_id`, `job_id`, `status` (the job row's state) and a | `202 Accepted` | `requeued` | A previously **failed** job was reset to `pending` and will be retried. | | `202 Accepted` | `active` | A `pending` or `running` job already exists and was returned unchanged. A live claim is never interrupted. | | `200 OK` | `completed` | Enrichment already finished; nothing was restarted. | -| `404 Not Found` | — | Unknown `scan_id`, or the scan has no findings to enrich and has not already been enriched. | +| `404 Not Found` | — | Unknown `scan_id`, or the scan has no findings to enrich and has not already been enriched. | An already-enriched scan always reports `completed`, including a clean scan that had no findings to enrich in the first place. -A job that exhausts its retry budget becomes `failed`. Re-POSTing this endpoint is the supported operator recovery: it atomically returns the job to `pending` with a fresh retry budget, clears the lease, and keeps the last `error_message` and the `checkpoint` so the retry resumes rather than re-enriching findings that already succeeded. Concurrent re-POSTs converge — exactly one reports `requeued` and the rest report `active`. +A job that exhausts its retry budget becomes `failed`. Re-POSTing this endpoint is the supported operator recovery: it atomically returns the job to `pending` with a fresh retry budget, clears the lease, and keeps the last `error_message` and the `checkpoint` so the retry resumes rather than re-enriching findings that already succeeded. Concurrent re-POSTs converge — exactly one reports `requeued` and the rest report `active`. Newly queued (`202`): @@ -316,7 +316,7 @@ Poll `GET /api/scans/` for `cve_enrichment_status` (`PENDING`, `ENRICHI ## GET /api/score -Returns the overall security posture score from 0 to 100. Under [severity contract v1](severity-contract.md), the score starts at 100 and deducts 20 per CRITICAL finding, 10 per HIGH finding, 5 per MEDIUM finding, and 2 per LOW finding; INFO findings deduct zero. Scoped to the most recent **completed** scan — if no completed scan exists yet, this returns `status: "NO_SCAN_DATA"` with `score: null` rather than a misleading 100 (a scan with no findings and no evidence at all would otherwise be indistinguishable). +Returns the overall security posture score from 0 to 100. Under [severity contract v1](severity-contract.md), the score starts at 100 and deducts 20 per CRITICAL finding, 10 per HIGH finding, 5 per MEDIUM finding, and 2 per LOW finding; INFO findings deduct zero. Scoped to the most recent **completed** scan — if no completed scan exists yet, this returns `status: "NO_SCAN_DATA"` with `score: null` rather than a misleading 100 (a scan with no findings and no evidence at all would otherwise be indistinguishable). Query parameters: @@ -345,13 +345,13 @@ Example response (no completed scan exists yet): } ``` -Consumers must check `status` and treat a `null` `score` as "not assessed" — never coerce it to `0`, which would misrepresent absence of evidence as a confirmed worst-case score. +Consumers must check `status` and treat a `null` `score` as "not assessed" — never coerce it to `0`, which would misrepresent absence of evidence as a confirmed worst-case score. --- ## GET /api/compliance/<framework> -Returns technical-evidence coverage against a compliance framework mapping pack, scoped to the most recent **completed** scan. This is coverage, not a certification or a claim of full framework compliance — see `docs/compliance-mapping-pack.md` for the full mapping-pack schema and `evaluation_basis` semantics. +Returns technical-evidence coverage against a compliance framework mapping pack, scoped to the most recent **completed** scan. This is coverage, not a certification or a claim of full framework compliance — see `docs/compliance-mapping-pack.md` for the full mapping-pack schema and `evaluation_basis` semantics. Supported frameworks: @@ -371,10 +371,10 @@ Query parameters: | `subscription_id` | UUID string | No | Scopes the "most recent completed scan" lookup to one Azure subscription. Defaults to the deployment's `AZURE_SUBSCRIPTION_ID`; if neither is set the latest completed scan from *any* subscription is used, which is only correct for a single-tenant database. A malformed value is a `400`, never a silently unscoped result. | `status` is one of: -- `OK` — a completed scan exists and at least one mapped control is in scope; `score_percent` is a real evaluated percentage. -- `NO_SCAN_DATA` — no completed scan exists yet, so there is no evidence to report; `score_percent` is `null`. -- `NO_REVIEWED_CONTROLS` — a completed scan exists, but every mapping awaits review; `score_percent` is `null` and no direct-evidence score exists. -- `NO_IN_SCOPE_CONTROLS` — a completed scan exists, but every reviewed mapped control for this framework is `not_applicable`/`organizational` and excluded from the denominator; `score_percent` is `null`. +- `OK` — a completed scan exists and at least one mapped control is in scope; `score_percent` is a real evaluated percentage. +- `NO_SCAN_DATA` — no completed scan exists yet, so there is no evidence to report; `score_percent` is `null`. +- `NO_REVIEWED_CONTROLS` — a completed scan exists, but every mapping awaits review; `score_percent` is `null` and no direct-evidence score exists. +- `NO_IN_SCOPE_CONTROLS` — a completed scan exists, but every reviewed mapped control for this framework is `not_applicable`/`organizational` and excluded from the denominator; `score_percent` is `null`. Per-control `status` is evaluation-derived (issue #263) only after its mapping has been reviewed: `PASS`/`FAIL`/`UNKNOWN`/`ERROR` is the rolled-up status of @@ -391,7 +391,7 @@ Any control with `review_status` other than `reviewed` has control status `UNREVIEWED_MAPPING`, regardless of its `mapping_type`. It is excluded from the denominator alongside `not_applicable` and `organizational` controls, and does not contribute to `passed` or `failed`. Consumers must check `status` and -never treat a `null` `score_percent` as `0` — a missing/excluded score is a +never treat a `null` `score_percent` as `0` — a missing/excluded score is a different fact from a real, evaluated 0%. `reviewed_controls`, `unreviewed_controls`, `not_applicable`, `organizational`, and `excluded_controls` make the reason explicit. @@ -440,7 +440,7 @@ Example response (`OK`): } ``` -Example response (`NO_SCAN_DATA`, HTTP 200 — never 500): +Example response (`NO_SCAN_DATA`, HTTP 200 — never 500): ```json { @@ -471,7 +471,7 @@ Unknown framework response (HTTP 400): ## GET /api/resources -Returns unique Azure resources derived from the most recent scan that has findings. Resources are aggregated from findings — one entry per distinct `resource_id`. Risk level is computed from the maximum severity finding on each resource. +Returns unique Azure resources derived from the most recent scan that has findings. Resources are aggregated from findings — one entry per distinct `resource_id`. Risk level is computed from the maximum severity finding on each resource. Query parameters: none @@ -513,7 +513,7 @@ No findings response (no scan with findings exists): ## GET /api/prioritization -Returns findings from the most recent scan grouped and ranked by risk score (`severity_weight × affected_resource_count`). Produces a matrix view, a ranked list, and recommended action items. +Returns findings from the most recent scan grouped and ranked by risk score (`severity_weight × affected_resource_count`). Produces a matrix view, a ranked list, and recommended action items. Query parameters: none @@ -613,7 +613,7 @@ No drift response (fewer than two scans): Returns the structured remediation playbook for a specific finding. Loads the matching `playbooks/cli/fix_.sh` script and wraps the finding's remediation text as a portal step. Appends NVD links from any CVE references on the finding. -Path parameters: `id` — integer finding ID from `GET /api/findings`. +Path parameters: `id` — integer finding ID from `GET /api/findings`. Example response: @@ -696,7 +696,7 @@ The following endpoints are called by the frontend but have no backend implement | Endpoint | Used by | Status | |---|---|---| -| `GET /api/monitoring` | Monitoring page — score trend chart, category distribution | Deferred. Score and findings data come from `GET /api/score` and `GET /api/findings` instead. | +| `GET /api/monitoring` | Monitoring page — score trend chart, category distribution | Deferred. Score and findings data come from `GET /api/score` and `GET /api/findings` instead. | --- @@ -706,7 +706,7 @@ The following endpoints are called by the frontend but have no backend implement ### Authentication requirement -These endpoints are **OIDC-only**. In `shared_secret` mode no tenant claim is present in the token, so the endpoints return `403 {"error": "tenant_id not available; OIDC authentication required"}`. This is intentional: the graph is strictly scoped per tenant and there is no safe way to infer a tenant from a shared-secret token. +OIDC callers use the verified tenant claim. A supplied `X-Tenant-Id` header cannot override that claim. In `shared_secret` mode, an authenticated admin may supply `X-Tenant-Id` explicitly; viewer and operator tokens cannot select a tenant. Requests without a usable tenant return `403 {"error": "tenant_id not available; OIDC authentication required"}`. Tenant query parameters never determine graph scope. ### Attack-path retention diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index bff52966..f021a6a5 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -104,3 +104,27 @@ def test_get_attack_path_not_found_returns_404(client, tenant_auth_headers): with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): resp = client.get(f"/api/attack-paths/{valid_uuid}", headers=tenant_auth_headers) assert resp.status_code == 404 + + +@pytest.mark.parametrize("limit", ["abc", "", "1.5", "0", "-1"]) +@pytest.mark.parametrize( + "endpoint", ["/api/attack-graph", "/api/attack-paths?scan_id=00000000-0000-0000-0000-000000000002"] +) +def test_malformed_limit_returns_400(client, tenant_auth_headers, endpoint, limit): + separator = "&" if "?" in endpoint else "?" + response = client.get(f"{endpoint}{separator}limit={limit}", headers=tenant_auth_headers) + assert response.status_code == 400 + + +def test_verified_viewer_token_cannot_select_tenant_in_query(client, app): + import jwt + + token = jwt.encode( + {"sub": "viewer", "role": "viewer", "tid": _TENANT, "exp": int(time.time()) + 60}, + app.config["JWT_SECRET"], + algorithm="HS256", + ) + response = client.get( + f"/api/attack-graph?tenant_id={_TENANT}", headers={"Authorization": f"Bearer {token}", "X-Tenant-Id": _TENANT} + ) + assert response.status_code == 403 From 598c5f00e0377518c040efa6e9f9960fc49d785e Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 8 Oct 2026 14:15:02 +0100 Subject: [PATCH 15/21] fix(graph): traverse current evidence and prune clean scan paths by scan scope Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 6 +- scanner/graph/path_traversal.py | 30 +++--- tests/test_graph_paths_postgres.py | 151 +++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 21 deletions(-) create mode 100644 tests/test_graph_paths_postgres.py diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 66a3f266..815b642a 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -79,7 +79,7 @@ def get_attack_graph(): SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, n.location, n.resource_group, n.subscription_id, n.snapshot_id, n.updated_at - FROM graph_nodes n + FROM current_graph_nodes n WHERE n.tenant_id = %(tenant_id)s AND n.subscription_id = %(subscription_id)s ORDER BY n.updated_at DESC @@ -93,7 +93,7 @@ def get_attack_graph(): SELECT n.node_id::text, n.resource_id, n.resource_type, n.name, n.location, n.resource_group, n.subscription_id, n.snapshot_id, n.updated_at - FROM graph_nodes n + FROM current_graph_nodes n WHERE n.tenant_id = %(tenant_id)s ORDER BY n.updated_at DESC LIMIT %(limit)s @@ -109,7 +109,7 @@ def get_attack_graph(): """ SELECT e.edge_id::text, e.source_node_id::text, e.target_node_id::text, e.relationship_type, e.confidence, e.evidence_source, e.collected_at - FROM graph_edges e + FROM current_graph_edges e WHERE e.source_node_id = ANY(%(node_ids)s::uuid[]) AND e.target_node_id = ANY(%(node_ids)s::uuid[]) """, diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index 0e8c1e4f..bf4f1b94 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -16,7 +16,7 @@ logger = logging.getLogger(__name__) -# Paths longer than this are not persisted — they're rarely actionable and +# Paths longer than this are not persisted — they're rarely actionable and # keeping them would inflate the table for large graphs. _MAX_PATH_LENGTH = 8 @@ -33,8 +33,8 @@ def _load_adjacency(conn: Any, tenant_id: str) -> dict[str, list[tuple[str, str, """ SELECT e.source_node_id::text, e.target_node_id::text, e.relationship_type, e.confidence - FROM graph_edges e - JOIN graph_nodes src ON src.node_id = e.source_node_id + FROM current_graph_edges e + JOIN current_graph_nodes src ON src.node_id = e.source_node_id WHERE src.tenant_id = %(tenant_id)s """, {"tenant_id": tenant_id}, @@ -59,7 +59,7 @@ def _load_finding_nodes(conn: Any, scan_id: str, tenant_id: str) -> list[str]: SELECT DISTINCT fgn.node_id::text FROM finding_graph_nodes fgn JOIN findings f ON f.id = fgn.finding_id - JOIN graph_nodes n ON n.node_id = fgn.node_id + JOIN current_graph_nodes n ON n.node_id = fgn.node_id WHERE f.scan_id = %(scan_id)s AND n.tenant_id = %(tenant_id)s """, @@ -108,20 +108,14 @@ def _delete_stale_paths(conn: Any, scan_id: str, tenant_id: str) -> None: with conn.cursor() as cur: cur.execute( """ - DELETE FROM attack_paths - WHERE tenant_id = %(tenant_id)s - AND scan_id != %(scan_id)s - AND scan_id IN ( - SELECT DISTINCT ap2.scan_id - FROM attack_paths ap2 - JOIN findings f ON f.scan_id = ap2.scan_id - WHERE ap2.tenant_id = %(tenant_id)s - AND f.subscription_id = ( - SELECT subscription_id FROM findings - WHERE scan_id = %(scan_id)s - LIMIT 1 - ) - ) + DELETE FROM attack_paths ap + USING scans previous, scans current + WHERE ap.tenant_id = %(tenant_id)s + AND ap.scan_id <> %(scan_id)s + AND previous.scan_id::text = ap.scan_id + AND current.scan_id = %(scan_id)s::uuid + AND current.status = 'completed' + AND previous.subscription_id = current.subscription_id """, {"tenant_id": tenant_id, "scan_id": scan_id}, ) diff --git a/tests/test_graph_paths_postgres.py b/tests/test_graph_paths_postgres.py new file mode 100644 index 00000000..7f167396 --- /dev/null +++ b/tests/test_graph_paths_postgres.py @@ -0,0 +1,151 @@ +"""Database regressions for current traversal and successful clean-scan retention.""" + +import os +import uuid +from dataclasses import replace + +import psycopg2 +import pytest + +from scanner.graph.graph_populator import populate_graph +from scanner.graph.path_traversal import _delete_stale_paths, _load_adjacency, compute_attack_paths +from tests.test_graph_freshness_postgres import snapshot + +pytestmark = pytest.mark.skipif(not os.environ.get("DATABASE_URL"), reason="requires PostgreSQL") + + +@pytest.fixture +def path_scope(): + tenant, sub = str(uuid.uuid4()), str(uuid.uuid4()) + dsn = os.environ["DATABASE_URL"] + scans = [] + + def scan(subscription=sub): + sid = str(uuid.uuid4()) + scans.append(sid) + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute( + "INSERT INTO scans (scan_id, subscription_id, started_at, status) VALUES (%s,%s,now(),'completed')", + (sid, subscription), + ) + return sid + + yield tenant, sub, dsn, scan + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute("DELETE FROM attack_paths WHERE tenant_id=%s", (tenant,)) + cur.execute("DELETE FROM graph_nodes WHERE tenant_id=%s", (tenant,)) + cur.execute("DELETE FROM graph_snapshot_scopes WHERE tenant_id=%s", (tenant,)) + cur.execute("DELETE FROM findings WHERE scan_id=ANY(%s::uuid[])", (scans,)) + cur.execute("DELETE FROM scans WHERE scan_id=ANY(%s::uuid[])", (scans,)) + + +def risky_scan(tenant, sub, dsn, sid): + snap = snapshot(tenant, sub) + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute( + "INSERT INTO findings (scan_id, rule_id, rule_name, severity, resource_id, detected_at, finding_key) " + "VALUES (%s,'AZ-TEST-001','test','HIGH',%s,now(),%s)", + (sid, snap.resources[1].resource_id, uuid.uuid4().hex), + ) + populate_graph(sid, snap, dsn) + return snap + + +def path_count(dsn, tenant, sid): + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute("SELECT count(*) FROM attack_paths WHERE tenant_id=%s AND scan_id=%s", (tenant, sid)) + return cur.fetchone()[0] + + +def test_successful_clean_scan_clears_previous_paths_from_authoritative_scope(path_scope): + tenant, sub, dsn, scan = path_scope + risky = scan() + risky_scan(tenant, sub, dsn, risky) + # Seed one prior path so the cleanup regression is independent of traversal. + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute("SELECT node_id::text FROM graph_nodes WHERE tenant_id=%s ORDER BY resource_id", (tenant,)) + nodes = [row[0] for row in cur.fetchall()] + cur.execute( + "INSERT INTO attack_paths " + "(path_id,tenant_id,scan_id,source_node_id,target_node_id,path_node_ids,path_length,relationship_types) " + "VALUES (%s,%s,%s,%s,%s,%s::uuid[],1,ARRAY['MEMBER_OF']) ON CONFLICT DO NOTHING", + (str(uuid.uuid4()), tenant, risky, nodes[0], nodes[1], nodes), + ) + clean = scan() + assert path_count(dsn, tenant, risky) == 1 + with psycopg2.connect(dsn) as conn: + _delete_stale_paths(conn, clean, tenant) + assert path_count(dsn, tenant, risky) == 0 + + +def test_partial_snapshot_does_not_traverse_retained_old_edge(path_scope): + from scanner.arg_inventory import InventoryStatus + + tenant, sub, dsn, scan = path_scope + first = scan() + risky_scan(tenant, sub, dsn, first) + with psycopg2.connect(dsn) as conn: + assert _load_adjacency(conn, tenant) + partial = snapshot(tenant, sub, status=InventoryStatus.PARTIAL, linked=False) + populate_graph(scan(), replace(partial, resources=partial.resources[:1]), dsn) + with psycopg2.connect(dsn) as conn: + assert _load_adjacency(conn, tenant) == {} + + +def test_risky_then_clean_scan_computes_zero_and_removes_old_paths(path_scope): + tenant, sub, dsn, scan = path_scope + first = scan() + risky_scan(tenant, sub, dsn, first) + assert path_count(dsn, tenant, first) == 1 + clean = scan() + populate_graph(clean, snapshot(tenant, sub, linked=False), dsn) + assert compute_attack_paths(clean, tenant, dsn) == 0 + assert path_count(dsn, tenant, first) == 0 + + +def test_clean_scan_retention_preserves_other_subscription_paths(path_scope): + tenant, sub, dsn, scan = path_scope + first = scan() + risky_scan(tenant, sub, dsn, first) + other_sub = str(uuid.uuid4()) + other = scan(other_sub) + risky_scan(tenant, other_sub, dsn, other) + clean = scan() + assert compute_attack_paths(clean, tenant, dsn) == 0 + assert path_count(dsn, tenant, first) == 0 + assert path_count(dsn, tenant, other) == 1 + + +def test_unsuccessful_scan_cannot_clear_previous_paths(path_scope): + tenant, sub, dsn, scan = path_scope + first = scan() + risky_scan(tenant, sub, dsn, first) + failed = scan() + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute("UPDATE scans SET status='failed' WHERE scan_id=%s", (failed,)) + _delete_stale_paths(conn, failed, tenant) + assert path_count(dsn, tenant, first) == 1 + + +def test_cleanup_keeps_other_tenants_paths(path_scope): + tenant, sub, dsn, scan = path_scope + other_tenant = str(uuid.uuid4()) + first, other = scan(), scan() + try: + risky_scan(tenant, sub, dsn, first) + risky_scan(other_tenant, sub, dsn, other) + clean = scan() + compute_attack_paths(clean, tenant, dsn) + assert path_count(dsn, tenant, first) == 0 + assert path_count(dsn, other_tenant, other) == 1 + finally: + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute("DELETE FROM graph_nodes WHERE tenant_id=%s", (other_tenant,)) + cur.execute("DELETE FROM graph_snapshot_scopes WHERE tenant_id=%s", (other_tenant,)) From 1613b5c59f9ad77ab01c57e2dc01f6baaabb57df Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 8 Oct 2026 16:00:17 +0100 Subject: [PATCH 16/21] fix(graph): guard path replay and sanitize graph API validation Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 31 ++++++------ docs/api-reference.md | 6 +-- scanner/graph/path_traversal.py | 29 ++++++++++-- tests/test_attack_graph_api.py | 19 ++++++-- tests/test_graph_paths_postgres.py | 76 +++++++++++++++++++++++++++++- 5 files changed, 132 insertions(+), 29 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 815b642a..443d4887 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -26,8 +26,8 @@ def _get_db() -> DatabaseManager: def _tenant_id() -> str | None: """Resolve tenant_id from the verified principal. - OIDC mode: the 'tenant' field is populated from the 'tid' claim in the token. - Shared-secret mode: 'tenant' is always None; admins may supply + The 'tenant' field comes from the verified token's tenant claim. + For tokens without a tenant claim, admins may supply X-Tenant-Id as a request header (never a query param, which leaks into logs and caches). Non-admin tokens cannot override the header. """ @@ -60,14 +60,13 @@ def get_attack_graph(): if limit > _MAX_LIMIT: limit = _MAX_LIMIT subscription_id = request.args.get("subscription_id") - except (ValidationError, ValueError) as exc: - return jsonify({"error": str(exc)}), 400 + except (ValidationError, ValueError): + return jsonify({"error": "Invalid request parameters"}), 400 tenant_id = _tenant_id() if not tenant_id: - # Shared-secret callers have no tenant claim; OIDC is required for - # tenant-scoped graph endpoints. Return 403 (not 400) since the - # request is well-formed but the auth method is insufficient. + # A caller without a verified tenant or admin fallback cannot access + # tenant-scoped graph evidence. return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 try: @@ -137,14 +136,13 @@ def list_attack_paths(): limit = positive_integer(int(request.args.get("limit", _DEFAULT_LIMIT)), "limit") if limit > _MAX_LIMIT: limit = _MAX_LIMIT - except (ValidationError, ValueError) as exc: - return jsonify({"error": str(exc)}), 400 + except (ValidationError, ValueError): + return jsonify({"error": "Invalid request parameters"}), 400 tenant_id = _tenant_id() if not tenant_id: - # Shared-secret callers have no tenant claim; OIDC is required for - # tenant-scoped graph endpoints. Return 403 (not 400) since the - # request is well-formed but the auth method is insufficient. + # A caller without a verified tenant or admin fallback cannot access + # tenant-scoped graph evidence. return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 try: @@ -180,14 +178,13 @@ def get_attack_path(path_id: str): """Return a single attack path with full node detail for each hop.""" try: path_id = uuid_string(path_id, "path_id") - except (ValidationError, ValueError) as exc: - return jsonify({"error": str(exc)}), 400 + except (ValidationError, ValueError): + return jsonify({"error": "Invalid request parameters"}), 400 tenant_id = _tenant_id() if not tenant_id: - # Shared-secret callers have no tenant claim; OIDC is required for - # tenant-scoped graph endpoints. Return 403 (not 400) since the - # request is well-formed but the auth method is insufficient. + # A caller without a verified tenant or admin fallback cannot access + # tenant-scoped graph evidence. return jsonify({"error": "tenant_id not available; OIDC authentication required"}), 403 try: diff --git a/docs/api-reference.md b/docs/api-reference.md index 9a7b2464..f9b3979d 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -702,12 +702,12 @@ The following endpoints are called by the frontend but have no backend implement ## Attack graph endpoints -`GET /api/attack-graph`, `GET /api/attack-paths`, and `GET /api/attack-paths/` expose the attack graph computed from the most recent scan for the caller's tenant. +`GET /api/attack-graph` returns explicitly observed nodes and relationships from the latest published inventory snapshot for each subscription in the caller's tenant. Complete snapshots expire absent resources and relationships within their tenant/subscription scope. Partial snapshots retain historical evidence in storage, while current graph queries and traversal exclude unobserved resources and relationships. Failed collection keeps the previous published snapshot. `GET /api/attack-paths` and `GET /api/attack-paths/` expose paths computed for a requested scan. Successful subsequent scans replace prior paths for the same tenant/subscription, including scans with no findings. ### Authentication requirement -OIDC callers use the verified tenant claim. A supplied `X-Tenant-Id` header cannot override that claim. In `shared_secret` mode, an authenticated admin may supply `X-Tenant-Id` explicitly; viewer and operator tokens cannot select a tenant. Requests without a usable tenant return `403 {"error": "tenant_id not available; OIDC authentication required"}`. Tenant query parameters never determine graph scope. +Callers use the tenant claim preserved by the token verifier in either authentication mode. A supplied `X-Tenant-Id` header cannot override that claim. An authenticated admin without a tenant claim may supply `X-Tenant-Id` explicitly; viewer and operator tokens cannot select a tenant. Requests without a usable tenant return `403 {"error": "tenant_id not available; OIDC authentication required"}`. Tenant query parameters never determine graph scope. ### Attack-path retention -Attack paths are written per scan and are not automatically pruned. In long-running deployments, old scan paths accumulate in `attack_paths`. Retention policy (e.g. keep only the N most recent scans per tenant) is tracked in issue #333 and will be addressed in a follow-up. +Attack paths are replaced by a newer successful scan within the same tenant/subscription. Clean scans remove prior paths even when there are no findings. Failed scans and delayed older scans retain the newer published evidence. Scope publication and traversal serialize on the same database lock. diff --git a/scanner/graph/path_traversal.py b/scanner/graph/path_traversal.py index bf4f1b94..19e5ae7a 100644 --- a/scanner/graph/path_traversal.py +++ b/scanner/graph/path_traversal.py @@ -14,9 +14,11 @@ import psycopg2 import psycopg2.extras +from scanner.graph.node_service import lock_graph_scopes + logger = logging.getLogger(__name__) -# Paths longer than this are not persisted — they're rarely actionable and +# Paths longer than this are not persisted because they are rarely actionable and # keeping them would inflate the table for large graphs. _MAX_PATH_LENGTH = 8 @@ -103,9 +105,25 @@ def _bfs_from( return paths -def _delete_stale_paths(conn: Any, scan_id: str, tenant_id: str) -> None: +def _delete_stale_paths(conn: Any, scan_id: str, tenant_id: str) -> bool: """Delete attack paths from previous scans for the same subscription, keeping only the current scan.""" with conn.cursor() as cur: + cur.execute("SELECT subscription_id FROM scans WHERE scan_id=%s::uuid", (scan_id,)) + scope = cur.fetchone() + if scope is None: + return False + lock_graph_scopes(conn, tenant_id, [scope[0]]) + with conn.cursor() as cur: + cur.execute( + "SELECT current.status='completed' AND NOT EXISTS (" + "SELECT 1 FROM scans newer WHERE newer.subscription_id=current.subscription_id " + "AND newer.status='completed' AND newer.started_at > current.started_at) " + "FROM scans current WHERE current.scan_id=%s::uuid", + (scan_id,), + ) + row = cur.fetchone() + if row is None or not row[0]: + return False cur.execute( """ DELETE FROM attack_paths ap @@ -116,10 +134,13 @@ def _delete_stale_paths(conn: Any, scan_id: str, tenant_id: str) -> None: AND current.scan_id = %(scan_id)s::uuid AND current.status = 'completed' AND previous.subscription_id = current.subscription_id + AND previous.started_at <= current.started_at """, {"tenant_id": tenant_id, "scan_id": scan_id}, ) + return True + def _write_paths( conn: Any, @@ -169,7 +190,9 @@ def compute_attack_paths(scan_id: str, tenant_id: str, dsn: str) -> int: conn = psycopg2.connect(dsn) conn.autocommit = False try: - _delete_stale_paths(conn, scan_id, tenant_id) + if not _delete_stale_paths(conn, scan_id, tenant_id): + conn.commit() + return 0 adj = _load_adjacency(conn, tenant_id) source_nodes = _load_finding_nodes(conn, scan_id, tenant_id) if not source_nodes: diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index f021a6a5..20e7f1ee 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -116,7 +116,7 @@ def test_malformed_limit_returns_400(client, tenant_auth_headers, endpoint, limi assert response.status_code == 400 -def test_verified_viewer_token_cannot_select_tenant_in_query(client, app): +def test_verified_viewer_tenant_cannot_be_overridden_by_header_or_query(client, app): import jwt token = jwt.encode( @@ -124,7 +124,16 @@ def test_verified_viewer_token_cannot_select_tenant_in_query(client, app): app.config["JWT_SECRET"], algorithm="HS256", ) - response = client.get( - f"/api/attack-graph?tenant_id={_TENANT}", headers={"Authorization": f"Bearer {token}", "X-Tenant-Id": _TENANT} - ) - assert response.status_code == 403 + foreign = "00000000-0000-0000-0000-000000000099" + conn = MagicMock() + cursor = conn.cursor.return_value.__enter__.return_value + cursor.fetchall.return_value = [] + db = MagicMock() + db.conn = conn + with patch("api.routes.attack_graph._get_db", return_value=db): + response = client.get( + f"/api/attack-graph?tenant_id={foreign}", + headers={"Authorization": f"Bearer {token}", "X-Tenant-Id": foreign}, + ) + assert response.status_code == 200 + assert cursor.execute.call_args[0][1]["tenant_id"] == _TENANT diff --git a/tests/test_graph_paths_postgres.py b/tests/test_graph_paths_postgres.py index 7f167396..fca30aa2 100644 --- a/tests/test_graph_paths_postgres.py +++ b/tests/test_graph_paths_postgres.py @@ -72,7 +72,8 @@ def test_successful_clean_scan_clears_previous_paths_from_authoritative_scope(pa nodes = [row[0] for row in cur.fetchall()] cur.execute( "INSERT INTO attack_paths " - "(path_id,tenant_id,scan_id,source_node_id,target_node_id,path_node_ids,path_length,relationship_types) " + "(path_id,tenant_id,scan_id,source_node_id,target_node_id," + "path_node_ids,path_length,relationship_types) " "VALUES (%s,%s,%s,%s,%s,%s::uuid[],1,ARRAY['MEMBER_OF']) ON CONFLICT DO NOTHING", (str(uuid.uuid4()), tenant, risky, nodes[0], nodes[1], nodes), ) @@ -149,3 +150,76 @@ def test_cleanup_keeps_other_tenants_paths(path_scope): with conn.cursor() as cur: cur.execute("DELETE FROM graph_nodes WHERE tenant_id=%s", (other_tenant,)) cur.execute("DELETE FROM graph_snapshot_scopes WHERE tenant_id=%s", (other_tenant,)) + + +def test_authenticated_graph_api_selects_only_current_verified_tenant_evidence(path_scope, client, app): + import time + import jwt + from scanner.arg_inventory import InventoryStatus + + tenant, sub, dsn, scan = path_scope + first = scan() + risky_scan(tenant, sub, dsn, first) + partial = snapshot(tenant, sub, status=InventoryStatus.PARTIAL, linked=False) + populate_graph(scan(), replace(partial, resources=partial.resources[:1]), dsn) + token = jwt.encode( + {"sub": "graph-viewer", "role": "viewer", "tid": tenant, "exp": int(time.time()) + 60}, + app.config["JWT_SECRET"], + algorithm="HS256", + ) + foreign = str(uuid.uuid4()) + response = client.get( + f"/api/attack-graph?subscription_id={sub}&tenant_id={foreign}", + headers={"Authorization": f"Bearer {token}", "X-Tenant-Id": foreign}, + ) + assert response.status_code == 200 + result = response.get_json() + assert len(result["nodes"]) == 1 + assert result["nodes"][0]["resource_id"] == partial.resources[0].resource_id.lower() + assert result["edges"] == [] + + +def test_delayed_older_traversal_cannot_delete_newer_paths(path_scope): + tenant, sub, dsn, scan = path_scope + older, newer = scan(), scan() + with psycopg2.connect(dsn) as conn: + with conn.cursor() as cur: + cur.execute("UPDATE scans SET started_at=now()-interval '2 minutes' WHERE scan_id=%s", (older,)) + cur.execute("UPDATE scans SET started_at=now()-interval '1 minute' WHERE scan_id=%s", (newer,)) + risky_scan(tenant, sub, dsn, newer) + assert path_count(dsn, tenant, newer) == 1 + assert compute_attack_paths(older, tenant, dsn) == 0 + assert path_count(dsn, tenant, newer) == 1 + + +def test_traversal_waits_for_scope_publication_lock(path_scope): + from concurrent.futures import ThreadPoolExecutor + from threading import Event + + tenant, sub, dsn, scan = path_scope + sid = scan() + risky_scan(tenant, sub, dsn, sid) + entered = Event() + blocker = psycopg2.connect(dsn) + try: + with blocker.cursor() as cur: + cur.execute("SELECT pg_advisory_xact_lock(hashtextextended(%s, 0))", (f"openshield-graph:{tenant}:{sub}",)) + + def traverse(): + entered.set() + return compute_attack_paths(sid, tenant, dsn) + + with ThreadPoolExecutor(max_workers=1) as executor: + future = executor.submit(traverse) + assert entered.wait(2) + try: + future.result(timeout=0.3) + pytest.fail("traversal completed while its scope lock was held") + except TimeoutError: + pass + finally: + blocker.rollback() + assert future.result(timeout=5) == 0 + assert path_count(dsn, tenant, sid) == 1 + finally: + blocker.close() From 6700d1cfe4817e0a1c1d72a1a1026aa6e06fd66c Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Thu, 8 Oct 2026 16:00:57 +0100 Subject: [PATCH 17/21] docs(graph): preserve original API reference encoding Signed-off-by: Tanvir Farhad --- docs/api-reference.md | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index f9b3979d..a1771df0 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -7,7 +7,7 @@ routes become public only when the deliberate demo-mode setting is enabled. ## Authentication -`/`, `/health`, `/ready`, and `/metrics` are always public. All other routes — including all `/api/*` GET endpoints — require an `Authorization: Bearer ` header, verified by `api/auth.py` according to `OPENSHIELD_AUTH_MODE`: +`/`, `/health`, `/ready`, and `/metrics` are always public. All other routes — including all `/api/*` GET endpoints — require an `Authorization: Bearer ` header, verified by `api/auth.py` according to `OPENSHIELD_AUTH_MODE`: | Mode | Verification | Role source | Intended for | |---|---|---|---| @@ -27,7 +27,7 @@ The dashboard never embeds a token (see [authentication and containment](securit ### Subscription authorization -`POST /api/scans/trigger` also checks `subscription_id` against `OPENSHIELD_AUTHORIZED_SUBSCRIPTIONS`, a comma-separated allowlist. A valid `operator`/`admin` token can otherwise trigger a scan against *any* subscription_id — role alone doesn't say which subscription a caller is entitled to. Left unset, every subscription_id is accepted (matches historical behavior); the API logs a loud startup warning when it's unset. This is a single-tenant containment boundary, not a substitute for real per-tenant authorization — see issue #294 for the remaining tenant-ownership scope this is a stopgap for. +`POST /api/scans/trigger` also checks `subscription_id` against `OPENSHIELD_AUTHORIZED_SUBSCRIPTIONS`, a comma-separated allowlist. A valid `operator`/`admin` token can otherwise trigger a scan against *any* subscription_id — role alone doesn't say which subscription a caller is entitled to. Left unset, every subscription_id is accepted (matches historical behavior); the API logs a loud startup warning when it's unset. This is a single-tenant containment boundary, not a substitute for real per-tenant authorization — see issue #294 for the remaining tenant-ownership scope this is a stopgap for. ## Input limits @@ -185,7 +185,7 @@ Example response: Returns the details and current status of a specific scan. -Path parameters: `scan_id` — UUID of the scan. +Path parameters: `scan_id` — UUID of the scan. Example response: @@ -208,7 +208,7 @@ Example response: Admits an asynchronous scan against the configured subscription. Execution happens in a background worker process; the response returns as soon as the scan is durably recorded. -Request body (optional — falls back to `AZURE_SUBSCRIPTION_ID`): +Request body (optional — falls back to `AZURE_SUBSCRIPTION_ID`): ```json { @@ -221,7 +221,7 @@ Request body (optional — falls back to `AZURE_SUBSCRIPTION_ID`): Admission is serialized per subscription and enforced by the database, so concurrent and replayed triggers converge on one logical scan rather than creating duplicates: - **At most one active scan per subscription.** While a `pending` or `running` scan exists, a further trigger returns that existing scan instead of queueing another. -- **`Idempotency-Key` (optional request header, 1–200 characters).** A repeat of the same key for the same subscription returns the original scan. The key is scoped to the subscription; the same key under a different subscription is a different request. A trigger carries no request input other than `subscription_id`, so a key that resolves to an existing scan is always a replay of the same logical request and there is no changed-payload conflict to report. +- **`Idempotency-Key` (optional request header, 1–200 characters).** A repeat of the same key for the same subscription returns the original scan. The key is scoped to the subscription; the same key under a different subscription is a different request. A trigger carries no request input other than `subscription_id`, so a key that resolves to an existing scan is always a replay of the same logical request and there is no changed-payload conflict to report. - **`OPENSHIELD_MAX_SCANS_PER_SUBSCRIPTION_PER_HOUR`** adds an optional hourly admission quota. Unset or `0` (the default) applies no time-window limit; the one-active-scan rule still applies. ### Responses @@ -229,8 +229,8 @@ Admission is serialized per subscription and enforced by the database, so concur | Status | When | Body | | --- | --- | --- | | `202 Accepted` | A new scan was admitted and queued. | `scan_id`, `status: "pending"`, `message` | -| `200 OK` | The request resolved to an existing logical scan — an `Idempotency-Key` replay of the same request, or a trigger while a scan is already active for the subscription. | `scan_id`, `status` (the existing scan's `pending`/`running`), `message: "Existing logical scan returned."` | -| `400 Bad Request` | Malformed body, invalid `subscription_id`, missing subscription, or an `Idempotency-Key` outside 1–200 characters. | `error` | +| `200 OK` | The request resolved to an existing logical scan — an `Idempotency-Key` replay of the same request, or a trigger while a scan is already active for the subscription. | `scan_id`, `status` (the existing scan's `pending`/`running`), `message: "Existing logical scan returned."` | +| `400 Bad Request` | Malformed body, invalid `subscription_id`, missing subscription, or an `Idempotency-Key` outside 1–200 characters. | `error` | | `403 Forbidden` | `subscription_id` is not on the `OPENSHIELD_AUTHORIZED_SUBSCRIPTIONS` allowlist. | `error` | | `429 Too Many Requests` | The configured hourly quota for this subscription is exhausted. | `error: "Scan quota exceeded for this subscription."` | @@ -266,7 +266,7 @@ Missing subscription response (`400`): ## POST /api/scans/<scan_id>/enrich -Queues durable CVE enrichment for a completed scan's findings. Enrichment runs as a database-backed job claimed by the background worker — the request never owns a thread, so the work survives an API restart. +Queues durable CVE enrichment for a completed scan's findings. Enrichment runs as a database-backed job claimed by the background worker — the request never owns a thread, so the work survives an API restart. There is **never more than one enrichment job per scan**. Repeat calls are safe: they report the state of the single job rather than creating another. @@ -280,11 +280,11 @@ Every response carries `scan_id`, `job_id`, `status` (the job row's state) and a | `202 Accepted` | `requeued` | A previously **failed** job was reset to `pending` and will be retried. | | `202 Accepted` | `active` | A `pending` or `running` job already exists and was returned unchanged. A live claim is never interrupted. | | `200 OK` | `completed` | Enrichment already finished; nothing was restarted. | -| `404 Not Found` | — | Unknown `scan_id`, or the scan has no findings to enrich and has not already been enriched. | +| `404 Not Found` | — | Unknown `scan_id`, or the scan has no findings to enrich and has not already been enriched. | An already-enriched scan always reports `completed`, including a clean scan that had no findings to enrich in the first place. -A job that exhausts its retry budget becomes `failed`. Re-POSTing this endpoint is the supported operator recovery: it atomically returns the job to `pending` with a fresh retry budget, clears the lease, and keeps the last `error_message` and the `checkpoint` so the retry resumes rather than re-enriching findings that already succeeded. Concurrent re-POSTs converge — exactly one reports `requeued` and the rest report `active`. +A job that exhausts its retry budget becomes `failed`. Re-POSTing this endpoint is the supported operator recovery: it atomically returns the job to `pending` with a fresh retry budget, clears the lease, and keeps the last `error_message` and the `checkpoint` so the retry resumes rather than re-enriching findings that already succeeded. Concurrent re-POSTs converge — exactly one reports `requeued` and the rest report `active`. Newly queued (`202`): @@ -316,7 +316,7 @@ Poll `GET /api/scans/` for `cve_enrichment_status` (`PENDING`, `ENRICHI ## GET /api/score -Returns the overall security posture score from 0 to 100. Under [severity contract v1](severity-contract.md), the score starts at 100 and deducts 20 per CRITICAL finding, 10 per HIGH finding, 5 per MEDIUM finding, and 2 per LOW finding; INFO findings deduct zero. Scoped to the most recent **completed** scan — if no completed scan exists yet, this returns `status: "NO_SCAN_DATA"` with `score: null` rather than a misleading 100 (a scan with no findings and no evidence at all would otherwise be indistinguishable). +Returns the overall security posture score from 0 to 100. Under [severity contract v1](severity-contract.md), the score starts at 100 and deducts 20 per CRITICAL finding, 10 per HIGH finding, 5 per MEDIUM finding, and 2 per LOW finding; INFO findings deduct zero. Scoped to the most recent **completed** scan — if no completed scan exists yet, this returns `status: "NO_SCAN_DATA"` with `score: null` rather than a misleading 100 (a scan with no findings and no evidence at all would otherwise be indistinguishable). Query parameters: @@ -345,13 +345,13 @@ Example response (no completed scan exists yet): } ``` -Consumers must check `status` and treat a `null` `score` as "not assessed" — never coerce it to `0`, which would misrepresent absence of evidence as a confirmed worst-case score. +Consumers must check `status` and treat a `null` `score` as "not assessed" — never coerce it to `0`, which would misrepresent absence of evidence as a confirmed worst-case score. --- ## GET /api/compliance/<framework> -Returns technical-evidence coverage against a compliance framework mapping pack, scoped to the most recent **completed** scan. This is coverage, not a certification or a claim of full framework compliance — see `docs/compliance-mapping-pack.md` for the full mapping-pack schema and `evaluation_basis` semantics. +Returns technical-evidence coverage against a compliance framework mapping pack, scoped to the most recent **completed** scan. This is coverage, not a certification or a claim of full framework compliance — see `docs/compliance-mapping-pack.md` for the full mapping-pack schema and `evaluation_basis` semantics. Supported frameworks: @@ -371,10 +371,10 @@ Query parameters: | `subscription_id` | UUID string | No | Scopes the "most recent completed scan" lookup to one Azure subscription. Defaults to the deployment's `AZURE_SUBSCRIPTION_ID`; if neither is set the latest completed scan from *any* subscription is used, which is only correct for a single-tenant database. A malformed value is a `400`, never a silently unscoped result. | `status` is one of: -- `OK` — a completed scan exists and at least one mapped control is in scope; `score_percent` is a real evaluated percentage. -- `NO_SCAN_DATA` — no completed scan exists yet, so there is no evidence to report; `score_percent` is `null`. -- `NO_REVIEWED_CONTROLS` — a completed scan exists, but every mapping awaits review; `score_percent` is `null` and no direct-evidence score exists. -- `NO_IN_SCOPE_CONTROLS` — a completed scan exists, but every reviewed mapped control for this framework is `not_applicable`/`organizational` and excluded from the denominator; `score_percent` is `null`. +- `OK` — a completed scan exists and at least one mapped control is in scope; `score_percent` is a real evaluated percentage. +- `NO_SCAN_DATA` — no completed scan exists yet, so there is no evidence to report; `score_percent` is `null`. +- `NO_REVIEWED_CONTROLS` — a completed scan exists, but every mapping awaits review; `score_percent` is `null` and no direct-evidence score exists. +- `NO_IN_SCOPE_CONTROLS` — a completed scan exists, but every reviewed mapped control for this framework is `not_applicable`/`organizational` and excluded from the denominator; `score_percent` is `null`. Per-control `status` is evaluation-derived (issue #263) only after its mapping has been reviewed: `PASS`/`FAIL`/`UNKNOWN`/`ERROR` is the rolled-up status of @@ -391,7 +391,7 @@ Any control with `review_status` other than `reviewed` has control status `UNREVIEWED_MAPPING`, regardless of its `mapping_type`. It is excluded from the denominator alongside `not_applicable` and `organizational` controls, and does not contribute to `passed` or `failed`. Consumers must check `status` and -never treat a `null` `score_percent` as `0` — a missing/excluded score is a +never treat a `null` `score_percent` as `0` — a missing/excluded score is a different fact from a real, evaluated 0%. `reviewed_controls`, `unreviewed_controls`, `not_applicable`, `organizational`, and `excluded_controls` make the reason explicit. @@ -440,7 +440,7 @@ Example response (`OK`): } ``` -Example response (`NO_SCAN_DATA`, HTTP 200 — never 500): +Example response (`NO_SCAN_DATA`, HTTP 200 — never 500): ```json { @@ -471,7 +471,7 @@ Unknown framework response (HTTP 400): ## GET /api/resources -Returns unique Azure resources derived from the most recent scan that has findings. Resources are aggregated from findings — one entry per distinct `resource_id`. Risk level is computed from the maximum severity finding on each resource. +Returns unique Azure resources derived from the most recent scan that has findings. Resources are aggregated from findings — one entry per distinct `resource_id`. Risk level is computed from the maximum severity finding on each resource. Query parameters: none @@ -513,7 +513,7 @@ No findings response (no scan with findings exists): ## GET /api/prioritization -Returns findings from the most recent scan grouped and ranked by risk score (`severity_weight × affected_resource_count`). Produces a matrix view, a ranked list, and recommended action items. +Returns findings from the most recent scan grouped and ranked by risk score (`severity_weight × affected_resource_count`). Produces a matrix view, a ranked list, and recommended action items. Query parameters: none @@ -613,7 +613,7 @@ No drift response (fewer than two scans): Returns the structured remediation playbook for a specific finding. Loads the matching `playbooks/cli/fix_.sh` script and wraps the finding's remediation text as a portal step. Appends NVD links from any CVE references on the finding. -Path parameters: `id` — integer finding ID from `GET /api/findings`. +Path parameters: `id` — integer finding ID from `GET /api/findings`. Example response: @@ -696,7 +696,7 @@ The following endpoints are called by the frontend but have no backend implement | Endpoint | Used by | Status | |---|---|---| -| `GET /api/monitoring` | Monitoring page — score trend chart, category distribution | Deferred. Score and findings data come from `GET /api/score` and `GET /api/findings` instead. | +| `GET /api/monitoring` | Monitoring page — score trend chart, category distribution | Deferred. Score and findings data come from `GET /api/score` and `GET /api/findings` instead. | --- From 5b9052e636cd2cc11df1ce9b589096856fe633ab Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Fri, 9 Oct 2026 15:28:05 +0100 Subject: [PATCH 18/21] fix(graph): add /v1/ prefix to all attack-graph and attack-paths API routes Signed-off-by: Tanvir Farhad --- api/routes/attack_graph.py | 6 +++--- tests/test_attack_graph_api.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/api/routes/attack_graph.py b/api/routes/attack_graph.py index 443d4887..748234cd 100644 --- a/api/routes/attack_graph.py +++ b/api/routes/attack_graph.py @@ -49,7 +49,7 @@ def _close_db(exc): db.close() -@attack_graph_bp.get("/api/attack-graph") +@attack_graph_bp.get("/api/v1/attack-graph") def get_attack_graph(): """Return graph nodes and edges for the caller's tenant (latest snapshot). @@ -122,7 +122,7 @@ def get_attack_graph(): return jsonify({"nodes": [dict(r) for r in nodes], "edges": [dict(r) for r in edges]}) -@attack_graph_bp.get("/api/attack-paths") +@attack_graph_bp.get("/api/v1/attack-paths") def list_attack_paths(): """Return pre-computed attack paths for a scan. @@ -173,7 +173,7 @@ def list_attack_paths(): return jsonify({"scan_id": scan_id, "paths": [dict(r) for r in rows]}) -@attack_graph_bp.get("/api/attack-paths/") +@attack_graph_bp.get("/api/v1/attack-paths/") def get_attack_path(path_id: str): """Return a single attack path with full node detail for each hop.""" try: diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index 20e7f1ee..6617fa63 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -54,7 +54,7 @@ def test_get_attack_graph_returns_empty_nodes_and_edges(client, tenant_auth_head conn = _mock_conn([[]]) with patch("api.routes.attack_graph.psycopg2.connect", return_value=conn): with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): - resp = client.get("/api/attack-graph", headers=tenant_auth_headers) + resp = client.get("/api/v1/attack-graph", headers=tenant_auth_headers) assert resp.status_code == 200 data = resp.get_json() assert "nodes" in data @@ -77,23 +77,23 @@ def test_get_attack_graph_viewer_cannot_supply_tenant_header(client, app): "Authorization": f"Bearer {token}", "X-Tenant-Id": _TENANT, } - resp = client.get("/api/attack-graph", headers=headers) + resp = client.get("/api/v1/attack-graph", headers=headers) assert resp.status_code == 403 def test_list_attack_paths_missing_scan_id_returns_400(client, tenant_auth_headers): - resp = client.get("/api/attack-paths", headers=tenant_auth_headers) + resp = client.get("/api/v1/attack-paths", headers=tenant_auth_headers) assert resp.status_code == 400 assert b"scan_id" in resp.data def test_list_attack_paths_invalid_uuid_returns_400(client, tenant_auth_headers): - resp = client.get("/api/attack-paths?scan_id=not-a-uuid", headers=tenant_auth_headers) + resp = client.get("/api/v1/attack-paths?scan_id=not-a-uuid", headers=tenant_auth_headers) assert resp.status_code == 400 def test_get_attack_path_invalid_uuid_returns_400(client, tenant_auth_headers): - resp = client.get("/api/attack-paths/not-a-uuid", headers=tenant_auth_headers) + resp = client.get("/api/v1/attack-paths/not-a-uuid", headers=tenant_auth_headers) assert resp.status_code == 400 @@ -102,7 +102,7 @@ def test_get_attack_path_not_found_returns_404(client, tenant_auth_headers): conn = _mock_conn([[]]) with patch("api.routes.attack_graph.psycopg2.connect", return_value=conn): with patch.dict("os.environ", {"DATABASE_URL": "postgresql://fake/db"}): - resp = client.get(f"/api/attack-paths/{valid_uuid}", headers=tenant_auth_headers) + resp = client.get(f"/api/v1/attack-paths/{valid_uuid}", headers=tenant_auth_headers) assert resp.status_code == 404 From 19c1922e573cc1d7dcbd42f3558bac9b1f01d376 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Fri, 9 Oct 2026 15:32:13 +0100 Subject: [PATCH 19/21] fix(graph): fix remaining old-path references in attack-graph test parametrize and tenant test Signed-off-by: Tanvir Farhad --- tests/test_attack_graph_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_attack_graph_api.py b/tests/test_attack_graph_api.py index 6617fa63..453b08a2 100644 --- a/tests/test_attack_graph_api.py +++ b/tests/test_attack_graph_api.py @@ -108,7 +108,7 @@ def test_get_attack_path_not_found_returns_404(client, tenant_auth_headers): @pytest.mark.parametrize("limit", ["abc", "", "1.5", "0", "-1"]) @pytest.mark.parametrize( - "endpoint", ["/api/attack-graph", "/api/attack-paths?scan_id=00000000-0000-0000-0000-000000000002"] + "endpoint", ["/api/v1/attack-graph", "/api/v1/attack-paths?scan_id=00000000-0000-0000-0000-000000000002"] ) def test_malformed_limit_returns_400(client, tenant_auth_headers, endpoint, limit): separator = "&" if "?" in endpoint else "?" @@ -132,7 +132,7 @@ def test_verified_viewer_tenant_cannot_be_overridden_by_header_or_query(client, db.conn = conn with patch("api.routes.attack_graph._get_db", return_value=db): response = client.get( - f"/api/attack-graph?tenant_id={foreign}", + f"/api/v1/attack-graph?tenant_id={foreign}", headers={"Authorization": f"Bearer {token}", "X-Tenant-Id": foreign}, ) assert response.status_code == 200 From 139116ac2af2c9b9775f825d989c094af060d192 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Fri, 9 Oct 2026 15:56:18 +0100 Subject: [PATCH 20/21] fix(graph): update /api/attack-graph path to /api/v1/attack-graph in postgres integration test Signed-off-by: Tanvir Farhad --- tests/test_graph_paths_postgres.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_graph_paths_postgres.py b/tests/test_graph_paths_postgres.py index fca30aa2..7b3f288e 100644 --- a/tests/test_graph_paths_postgres.py +++ b/tests/test_graph_paths_postgres.py @@ -169,7 +169,7 @@ def test_authenticated_graph_api_selects_only_current_verified_tenant_evidence(p ) foreign = str(uuid.uuid4()) response = client.get( - f"/api/attack-graph?subscription_id={sub}&tenant_id={foreign}", + f"/api/v1/attack-graph?subscription_id={sub}&tenant_id={foreign}", headers={"Authorization": f"Bearer {token}", "X-Tenant-Id": foreign}, ) assert response.status_code == 200 From 4fd24c7ca0060e861336ef64e44ee14f35b299a0 Mon Sep 17 00:00:00 2001 From: Tanvir Farhad Date: Sun, 11 Oct 2026 13:56:54 +0100 Subject: [PATCH 21/21] fix(graph): rechain attack_paths migration onto graph_current_scope to restore single alembic head Signed-off-by: Tanvir Farhad --- alembic/versions/f2a3b4c5d6e7_attack_paths.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alembic/versions/f2a3b4c5d6e7_attack_paths.py b/alembic/versions/f2a3b4c5d6e7_attack_paths.py index e312c062..f60b5cdc 100644 --- a/alembic/versions/f2a3b4c5d6e7_attack_paths.py +++ b/alembic/versions/f2a3b4c5d6e7_attack_paths.py @@ -12,7 +12,7 @@ from sqlalchemy.dialects import postgresql revision: str = "f2a3b4c5d6e7" -down_revision: Union[str, Sequence[str], None] = "e1f2a3b4c5d6" +down_revision: Union[str, Sequence[str], None] = "e2c4a6b8d013" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None