Repository navigation
Description
Describe the bug
cldk/analysis/commons/graphs.py::shortest_walks cannot return a witness for a flow that
flows_to_argument confirms exists. Same application, same source, same sink:
| accessor | depth=None |
goes through |
|---|---|---|
flows_to_argument |
True in 0.9 s |
reach |
taint / paths_between |
abandoned after 180 s | shortest_walks |
For a triage surface this is the worst failure shape: the caller is told a flow exists and then
cannot obtain the evidence for it. taint()'s whole purpose is to produce the witness that
flows_to_* only asserts.
Affects all four languages — shortest_walks backs paths_between and taint on Python,
Java, TypeScript and Go. Pre-existing (lifted to commons in leg 2.5b); not introduced by #444.
To Reproduce
Needs a real application; the committed fixtures have path distances around 3, which is why this
has never shown up in the suite.
# the analyzer's own eval corpus, pinned
git -C codeanalyzer-go checkout f8f6db8 # nats-server pin, see scripts/eval_l3.sh
EVAL_SRC=/tmp/eval bash scripts/eval_l3.sh --prepare nats-server
cango -i /tmp/eval/nats-server -a 4 --app-name nats-server --namespace nats-server -o /tmp/l4Then, over that payload:
b = GoCodeanalyzer(project_dir=None, cache_dir=..., analysis_level="system_dependency_graph",
application_name="nats-server", namespace="nats-server")
b.flows_to_argument(src, sink_sig, port, within=src_sig) # True, 0.9 s
b.taint(sources=[(src, src_sig)], sinks=[(port, sink_sig)]) # does not return
b.taint(sources=[(src, src_sig)], sinks=[(port, sink_sig)], depth=12) # 0.0 s, paths=0Expected behavior
taint / paths_between return the witness, in time comparable to the predicate that confirms
the same flow. A bound should be an optimization, not the only usable mode.
Root cause
The depth-first replay's only pruning is dist.get(d) == len(walked) + 1
(commons/graphs.py, in shortest_walks' inner walk). That keeps every step on a BFS layer
boundary, so each explored path has exactly the target length — but it never restricts the search
toward dst. The replay therefore enumerates shortest paths to every node at the target
distance, and only the ones that happen to end at dst are counted.
Measured on the nats-server pair above (1,693,330 indexed nodes, 6,761,391 adjacency pairs):
distance from src to dst |
42 hops |
| nodes at distance exactly 42 | 110,833 (dst is one of them) |
shortest paths src → distance 42 |
14,942,058,556,479,936 |
shortest paths src → dst |
736,848 |
| over-exploration factor | ~2 × 10¹⁰ |
The forward BFS that builds dist is fine — it finds dst at distance 42 in 1.1 s. The cost is
entirely in the replay.
Suggested fix
Standard bidirectional pruning: run a backward BFS from dst over the reversed adjacency to get
dist_bwd, then in the replay step only to d where dist_fwd[d] + dist_bwd[d] == target. That
restricts the DFS to nodes lying on some shortest src→dst path, which is exactly the set the
caller asked for.
Why this is not a small patch
Changing which walks are explored touches three published contracts, each of which needs
re-verification rather than assumption:
- "shortest, in order" —
max_pathsis documented as a prefix of a total order
(hop_sort_key), not an arbitrary subset. The pruned replay must emit the same prefix. - sanitizer cuts —
allow_edge/allow_nodeare applied insidesteps()so the breadth-first
pass measures the shortest satisfying distance. A backward BFS has to apply the same predicates,
or the two directions disagree about which graph they are searching and a sanitized route could
be pruned as unreachable, producing a false refutation. - both backends — the Neo4j side answers the same question with
allShortestPathsin Cypher
and is unaffected, so the parity tests must still pass afterwards. Divergence here is a
correctness regression, not a performance one.
Additional context
Found while evaluating #444 (Go L3/L4, PR for #436) against the analyzer's real-app corpus. That
evaluation also turned up two defects now fixed in #444, both invisible on the committed fixtures
for the same reason as this one:
_reachexpanded each node once per predecessor rather than once, so the frontier tracked path
count: 151,417,783 expansions at depth 8 against 670,450, and unbounded it did not terminate.
Fixed and lifted intocommons.graphs.reach, so all four languages share one walk.split_body_node_idre-sorted its whole candidate list on every call, making a bulkget_source
over a page's endpoints sort 5,955 ids per endpoint on nats-server.
A related scale note, relevant to how urgent this is: on nats-server an unbounded reach from a
typical port touches 1,631,237 of 1,693,330 nodes — 96% of the graph. The slices default to
DEFAULT_DEPTH = 5 and are unaffected, but paths_between, flows_to_* and taint default to
depth=None — correct for soundness, since a bound makes a False ambiguous, but it means the
default query on a large application is inherently whole-graph. The fix above does not change that;
it changes the replay from exploring 10¹⁶ paths to exploring the ones that can reach the sink.
Activity
docs: record the shared reach() walk in the language table