Skip to content

Python's Neo4j scope audit judges statements, not variables — the check that found 16 TypeScript leaks has never run against Python #363

Description

@rahlk

Problem

TypeScript and Java judge every Cypher statement per bound variable — each node the statement binds must carry the application scope. Python still judges per statement: _scope_kind returns "prefix" if the statement contains any prefix predicate, whatever the rest of it binds.

def _scope_kind(statement: str) -> str | None:
    if _INTROSPECTION.match(statement): return "introspection"
    if _is_scoped(statement): return "prefix"          # ← the whole statement, not each variable
    ...

The per-variable audit is what found 16 scope leaks in TypeScript, two of them path enumerators whose endpoints were scoped and whose interiors were not. It has never been run against Python.

It has already missed one

PyNeo4jBackend._REACHES binds three node positions and scoped two of them:

MATCH (a:PyCallable {signature:$a}) WHERE a.id STARTS WITH $prefix
MATCH (a) ((x:PyCallable)-[:PY_CALLS]->(y:PyCallable)
           WHERE x.id STARTS WITH $prefix){1,} (m:PyCallable)
WITH DISTINCT m WHERE m.signature = $b RETURN count(m) > 0 AS ok

Consecutive repetitions bind x to the previous y, so predicating x covers the anchor and every interior node — but not the last one, which is only ever a y. That node is m, and it was free to belong to another application. The statement carried a prefix, so the per-statement audit called it scoped.

Found by hand while working #356, fixed there, and pinned by a targeted test. The general case is this issue.

Scope boundary

In scope: port the per-variable audit to the Python suite and fix whatever it reports.

Out of scope: the _REACHES fix itself (#356). The exemptions TypeScript and Java already record for containment-only walks — those are reasoned, and Python's equivalents should be judged on the same basis rather than assumed.

Goals

  • Every node variable a PyNeo4jBackend statement binds is judged individually, in the idiom test_typescript_neo4j_multi_application_scope.py already uses
  • Statements keyed only by an application-stamped id keep their documented exemption, each naming its reason
  • Every leak the audit reports is either fixed or exempted with a stated cause — none silently filtered out
  • Consider lifting the audit helper into tests/analysis/commons/ rather than a third copy; three near-identical implementations is how the allShortestPaths filter bug happened in the first place

Caveats and known risks

  • Expect this to find more than one. TypeScript's port found 16. A large report is the audit working, not a reason to weaken it.
  • Adding a predicate to a hop is not free, and adding it to both endpoints of a quantified pattern is actively harmful — measured 623.9s against 3.2s on a 5,000-node cyclic graph for the same answer (reaches() with no depth bound does not terminate on a large call graph: the quantified path pattern enumerates trails, it does not prune #356). Scope every node once; never twice.
  • Some Python statements are keyed purely by can:// id and may legitimately omit the predicate. That exemption is already documented in the per-statement audit; carry the reasoning across rather than re-deriving it.

Definition of done

  • Python's audit judges the same thing TypeScript's and Java's do, and every variable it reports unscoped is either predicated or exempted with a reason a reviewer can check.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions