Repository navigation
test_python_neo4j_backend.py writes to whatever CLDK_TEST_NEO4J_URI points at, and never cleans up #324
Description
Problem
tests/analysis/python/test_python_neo4j_backend.py is presented as a read-only parity test, but its
backends fixture writes a whole application into the target database:
opts = AnalysisOptions(
input=proj,
emit=EmitTarget.NEO4J,
app_name=APP_NAME,
rebuild_analysis=True,
neo4j_uri=NEO4J_URI, # <- $CLDK_TEST_NEO4J_URI
...
)
with Codeanalyzer(opts) as az:
emit_neo4j(az.analyze(), opts)(tests/analysis/python/test_python_neo4j_backend.py:145-165)
Two things make this a trap rather than a documented cost:
- The write is invisible to inspection. There is no literal
CREATE/MERGE/SET/DELETE
anywhere in the file — the write goes through a library call. Anyone auditing the test suite for
destructive operations by searching for Cypher keywords will conclude the file is read-only. It is not. - There is no teardown. The fixture yields and then only calls
neo.close(). The emitted
application (APP_NAME) stays in the database after the run.
Impact
Anyone who has CLDK_TEST_NEO4J_URI exported — which is exactly the setup used to run the live
end-to-end tests against a real analysis — and then runs the python test directory will silently
inject a second application into that database.
This already happened during leg 1.5 development: a run against the integration database (holding a
~50-minute analysis of a real project) added a second application, which broke three live tests whose
expectation queries count nodes without scoping to an application. Repair required identifying and
deleting the injected nodes by hand.
Suggested fixes (any one helps; the first two are the important ones)
- Isolate the target. Have this file require a separate env var (e.g.
CLDK_TEST_NEO4J_WRITE_URI)
so pointing at a read-only integration database is not sufficient to trigger a write. Skip the module
when that var is unset. - Tear down. Delete the emitted application in fixture teardown, scoped by
app_name, so a run
leaves the database as it found it. - Label the hazard. Say plainly in the module docstring and in any test README that this file
MUTATES the target database. The current docstring says the harness "loads the graph out of band"
and then "queries it read-only", which reads as if the whole file is read-only. - Consider a
pytestmarker (e.g.@pytest.mark.destructive) deselected by default, so the write
requires an explicit opt-in.
Related
The mirror-image defect exists on the read side: tests/analysis/python/test_e2e_neo4j_live.py has
three expectation queries that count nodes without scoping to an application, which is why a second
application turned them red instead of leaving them alone. Worth fixing together.
Definition of done
- Running the full python test directory with only the read-only integration env vars set performs
no writes to that database. - If the write path is invoked deliberately, the database is left as it was found.
- The hazard is stated in the module docstring, not merely implied.
Caveats
- Changing the env var this file keys off will break any existing CI job that runs it; check for one
before renaming.
Activity
rahlk commentedon Sep 5, 2026
Worse than first reported: unsetting the env vars does not disable the write, it redirects it
The original report assumed
CLDK_TEST_NEO4J_URIhad to be set for the write to reach a real database.That is wrong. Lines 46-48:
With the env unset the module does not skip by construction. It attempts
verify_connectivity()against
localhost:7687with the defaultneo4j/neo4jcredentials, and on success its fixture runsemit_neo4j(...)and writes thecldk_py_parityapplication there.Port 7687 is the Neo4j default, so anything a developer has bound or forwarded to it becomes the target
— including an SSH port-forward to a remote or shared Neo4j. On one developer machine during leg 1.5,
7687 was held by an
sshprocess in LISTEN while the local test container was deliberately on 7688.Under those conditions "unset the variables to be safe" sends the write off-box rather than preventing it.
This raises the severity: the hazard is not merely "a developer who opted in loses their integration
database", it is "a test run with no configuration at all can write to whatever answers on the default
port with default credentials".
Additional requirement for the fix
Beyond the fixes already listed, the module must skip when it is not explicitly configured. It should
not fall back to a default URI or default credentials at all — no env var, no run. An opt-in write test
that defaults to writing somewhere is the whole problem:
Silent connection to a default endpoint is also worth auditing for elsewhere in the suite.
test(python): make the emitting parity module impossible to run by ac…
test(python): purge what the emitter created, not what its wipe reach…
rahlk commentedon Sep 6, 2026
Resolved in #323 (merged to
release/2.0asb5a18b4, shipped in2.0.0-rc.2):test_python_neo4j_backend.pynow skips unlessCLDK_TEST_NEO4J_WRITE_URI/_WRITE_USER/_WRITE_PASSWORDare set (no default URI or credentials —a47fabb), its docstring leads with the fact that it writes, and a two-statement teardown removes what the emitter created, anchored on the application name and thecan://id prefix (ca5ed1f; the 664 shared package/decorator nodes are deliberately left, documented). The mirror-image defect — unscoped expectation counts intest_e2e_neo4j_live.py— was fixed in the same leg. Running the python test directory with only the read-only integration variables set performs no writes.docs(spec): correct §2 — Python's value traversals are scoped by id, …