feat(graph): BFS attack path traversal and API endpoints

[TFT444]

Summary

Computes shortest paths from finding-linked nodes over explicitly current graph evidence and exposes tenant-scoped graph and path APIs. Traversal runs after successful graph publication. Detector or publication failure preserves prior evidence.

Successful clean scans remove previous paths using authoritative scan scope. Failed and delayed older scans cannot delete newer paths. Publication and traversal share scoped transaction locks. Invalid limits return HTTP 400 with generic validation messages.

Graph scope comes from the verified tenant claim in either authentication mode. A header cannot override that claim. An authenticated admin without a tenant claim may provide X-Tenant-Id; query parameters never select tenant scope.

Validation

Full Linux suite at 9155a3f: 1763 passed, 3 skipped, 4 subtests passed. Independent PostgreSQL, API, traversal and migration checks: 51 passed. Ruff lint and formatting passed. The schema has one migration head, and upgrade, downgrade and current-dev upgrade were exercised. Replay and locking regressions were verified failing before the fixes and passing afterward.

Dependencies

Depends-On: #353

Merge #326, #352, #353, then this PR.

Closes #333.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[parthrohit22]

The BFS itself is clean and easy to reason about. The hop cap and the tenant filter on every endpoint query are good, and moving tenant resolution off the query param onto the verified principal was the right fix.

I'm requesting changes, mostly because of what's upstream. With the #352/#353 issues this step always has nothing to work on: no snapshot in real scans, and even with one, finding_graph_nodes is empty because linking runs before save_scan. So compute_attack_paths returns 0 on every scan, and the API only ever serves empty lists. Once those are fixed, a few things here are worth sorting before merge:

1. Edge direction doesn't match "attack path from a finding"

BFS follows edges source → target, but the edge directions from #353 are mixed: EXPOSES is PublicIP→NIC, HAS_IDENTITY is identity→resource, REACHABLE_VIA is storage→PE, MEMBER_OF is resource→subnet. Starting from a flagged storage account you reach its private endpoint. But starting from a flagged VM/NIC you can never reach the public IP that exposes it, and that's the path people will actually care about. Could you either define edge direction as "attacker can move from A to B" consistently, or traverse with per-relationship direction rules?

2. Row growth

Every scan writes one row per (finding node × reachable node), and nothing ever deletes old scans' paths. On a subscription with a few hundred findings in a connected VNet this grows quickly. At minimum, keep only the latest scan per subscription, or add a retention step. Also, _write_paths returns len(rows) even when ON CONFLICT DO NOTHING skips them, so the logged count can overstate what was written.

3. Tenant resolution in shared-secret mode

In shared-secret mode tenant is always None, so every non-admin caller gets 400 tenant_id not available. Admins can pass any X-Tenant-Id. If that's intentional for now, please mention it in the API docs. Otherwise a 400 for a normal viewer feels like the wrong status (403, or scoping by subscription like the rest of the API).

Small

• /api/attack-graph returns edges where only one end is in the page of nodes, so the client gets dangling node_ids. Filtering with AND instead of OR, or returning the other endpoints too, would keep the payload consistent.
• _conn() opens a raw connection per request. The other routes go through DatabaseManager, which shares the pool metrics in api/observability.py. Reusing it would keep /metrics accurate.

Happy to re-review once #352 and #353 settle.

[TFT444]

Both items addressed:

1. BFS bidirectional traversal: _load_adjacency() now adds reverse edges to the adjacency map (with _REV suffix on the relationship type). BFS from a flagged resource can now reach nodes that point TO it, e.g. a VM with a finding can reach the PublicIP that EXPOSES it.
2. AND filter on /api/attack-graph edges: Changed OR to AND in the edge query so only edges where both endpoints appear in the returned node page are included. No more dangling node references on the client.

[parthrohit22]

Re-reviewed 63c223c. The AND filter on /api/attack-graph edges is right: no more dangling node IDs.

This head brings back the bug #353 just fixed, though.

1. Regression: populate_graph() runs inside run_scan() again

Commit 830ca7a ("wire post-scan node and edge population into ScanEngine") sits on top of #353's fix. It re-adds the in-engine call at scanner/engine.py:205-211, while scanner/worker.py:219-226 still calls it after save_scan(). On this head every scan therefore populates the graph twice:

• first inside run_scan(), before findings are saved, so link_findings_to_nodes links 0 findings and compute_attack_paths has no sources;
• then again from the worker.

It looks like a rebase artefact. Please drop 830ca7a (or remove that block) so the worker is the only caller.

2. Fully bidirectional BFS turns the graph into an undirected one

Adding a _REV edge for every edge answers "the VM can't reach the public IP that exposes it". But it also lets BFS walk any relationship backwards. For example, MEMBER_OF reversed goes subnet → every other member, so every resource sharing a subnet with a flagged resource becomes reachable within two hops. On a real VNet the path count becomes roughly findings × subnet size, which is also why the row growth below matters. What I asked for was per-relationship direction ("attacker can move from A to B"):

• Reverse: EXPOSES (reach the exposing public IP) and HAS_IDENTITY (reach the identity a resource runs as).
• Forward only: MEMBER_OF and PROTECTS.

A small {relationship: (forward, reverse)} table in path_traversal.py does it. Please add a test with two unrelated VMs in one subnet, asserting that a finding on one does not produce a path to the other.

3. Still open from my first review

• Row growth: paths are written per scan and never pruned, and _write_paths still returns len(rows) even when ON CONFLICT DO NOTHING skips rows. With 2 in place this is worse. Keep only the latest scan's paths per subscription, or add retention, and return cur.rowcount.
• Shared-secret mode: every non-admin caller still gets 400 tenant_id not available. If that's intended until OIDC is the only mode, say so in docs/api-reference.md. Otherwise 403, or scoping by subscription like the rest of the API, is the right response.
• _conn(): it still opens a raw psycopg2 connection per request instead of going through DatabaseManager. Those connections are invisible to the pool metrics in /metrics.

CI

• Lint fails with the same unused MagicMock import in tests/test_graph_engine_post_scan.py.
• Backend Tests fails with the same 5 failures and 1 error inherited from #352's down_revision.

A second migration (f2a3b4c5d6e7_attack_paths.py) stacks on e1f2a3b4c5d6. Once #352 changes to a single parent, check that alembic heads is still one head across the stack.

[TFT444]

All three blockers from your review are resolved on the current head (6b52c78):

1. populate_graph() regression — already removed in commit ea10a59 ('remove double populate_graph'). engine.py no longer imports or calls it; worker.py is the sole caller after save_scan().

2. Directional BFS — implemented in commit ea10a59. Only EXPOSES and HAS_IDENTITY get a _REV edge in the adjacency map. MEMBER_OF and PROTECTS are forward-only. The requested regression test is in commit 9527010 (test_member_of_not_reversed_prevents_subnet_bridging): two VMs in one subnet, asserting a finding on one does not produce a path to the other.

3. rowcount fix — added in this push (6b52c78). execute_values with ON CONFLICT DO NOTHING returns -1 for rowcount; _write_paths now returns len(rows) (paths attempted) so the log line is meaningful.

The review was on 63c223c but all items were fixed in subsequent commits. 8/8 path traversal tests passing locally. Please re-review when you get a chance @parthrohit22

[TFT444]

@parthrohit22 all requested changes are addressed: populate_graph() no longer runs inside run_scan() (worker is the only caller), BFS uses per-relationship direction rules (EXPOSES and HAS_IDENTITY reversed, MEMBER_OF and PROTECTS forward-only), _write_paths now uses cur.rowcount and deletes stale paths from previous scans before writing, CodeQL exception exposure is fixed (full traceback logged server-side, generic error returned to client), and route handlers go through DatabaseManager. Could you re-review when you get a chance? Thanks.

[TFT444]

@parthrohit22 The remaining blocker (merge-conflict marker in tests/test_graph_engine_integration.py:71) is now removed. All your previous concerns were already addressed. Please re-review when you get a chance.

[TFT444]

@ritiksah141 @parthrohit22 could you please re-review the latest head (9155a3f)?

Clean-scan path cleanup, delayed-scan protection, API validation and documentation are updated. Depends on #353.

The fixes are pushed and all checks are passing on this head (the deployment skip is expected). Please review the updated code and tests, and update your review decision or resolve the relevant conversations when satisfied.

[TFT444]

Hi @parthrohit22 - the populate_graph() double-call regression is removed, BFS now uses per-relationship direction rules, path pruning is in, and _conn() goes through DatabaseManager. CI is fully green. Ready for re-review.