fix: resolve intermediary variable detection in Java and Python engines

[Ayush-Patel-56]

Description

Resolves #8 by implementing recursive symbol tracing in the Java and Python detection engines.

The engine previously used shallow name-based matching for variables, which caused cryptographic rules to fail when intermediary variables were introduced (e.g., SeatInterface intermediary = s; Car c = new Car(intermediary);). This PR introduces a tracing mechanism that resolves variables back to their original initialization point.

Changes

• Recursive Tracing: Added traceSymbol to follow variable assignments through the AST.
• Deep Equivalence: Implemented areSymbolsEquivalent to compare symbols after tracing them to their source declaration.
• Engine Updates: Modified isInvocationOnVariable and isInitForVariable in both JavaDetectionEngine and PythonDetectionEngine to use deep symbol resolution.
• Regression Testing: Added Issue8IntermediaryVariableTest covering nested rule detection across assignment levels.

Verification

• Unit Tests: All 157 existing tests passed.
• Manual Verification: Confirmed via DetectionStoreLogger that the engine correctly associates children nodes through intermediary variables.
• Trace Proof: Logs confirm successful symbol resolution: Comparing symbols: s (traced to s) with intermediary (traced to s).

See attached screenshot for the successful test execution and detection hierarchy.

[n1ckl0sk0rtge]

@Ayush-Patel-56 Thanks for tackling this ❤️
The diagnosis is correct and the approach (replace name-equality with a recursive symbol trace) is reasonable. A few items below; flagging the bigger ones as required and one as a follow-up.

Required before merge

Correctness

• Python traceSymbol — first-usage-wins is non-deterministic. The loop returns on the first ASSIGNMENT_LHS encountered while iterating symbol.usages(). For a variable reassigned more than once (x = y; x = z; use(x)), which RHS is followed depends on iteration order rather than which assignment reaches the call site. The original name comparison was wrong but predictable; this is wrong and non-deterministic. At minimum, document the limitation in a comment; ideally pick the assignment lexically nearest the use site.
• C# engine not updated. CSharpDetectionEngine has the same shallow name().equals() pattern. If C# is out of scope intentionally, say so in the PR description — otherwise apply the same fix there.

Code quality

• Java duplicates an existing helper. JavaDetectionEngine.java:923 already has traceVariable(IdentifierTree) doing the same chain-of-initializers walk on IdentifierTree. The new traceSymbol(Symbol) is a near-clone. Refactor so there's one implementation (e.g., have traceVariable delegate to traceSymbol), rather than shipping two parallel walkers.
• Stray whitespace in PythonDetectionEngine.java:21. Blank line was changed to a line with a single space — unrelated to the fix. mvn spotless:apply should drop it.
• @Rule(key = "Issue8") on the test class isn't the convention here (see Issue16Test, Issue214Test, etc.). Please remove.
• Trailing-whitespace-only lines in Issue8IntermediaryVariableTest.java. Spotless will clean these up.
• Unused HeatedSeats rule. seatRules declares a rule for HeatedSeats but the test file never instantiates it. Drop the dead rule or add a HeatedSeats case.

Test coverage

The single new test covers the simplest case (intermediary = s as a constructor argument). For a change that introduces recursive resolution into both engines, please add:

• Chain length > 1 (a = s; b = a; new Car(b)).
• Method-invocation receiver path (isInvocationOnVariable) — currently only isInitForVariable via new Car(...) is exercised.
• At least one Python regression test. The Python engine path has no test coverage in this PR, and Python's usages()-based trace differs structurally from Java's declaration()-based one — they need independent verification.

Optional in this PR — please file a follow-up

• Recursion guard. Both traceSymbol methods recurse without a visited set or depth bound. Java is hard to construct a cycle in (compiler usually catches it); Python's a = b; b = a would loop forever, since the existing !rhsSymbol.equals(symbol) check only blocks the direct self-cycle. Real-world Python rarely produces this, so it's fine to defer — but please open a follow-up issue so it isn't lost. A small Set<Symbol> visited or a depth cap will close it cheaply.

[Ayush-Patel-56]

Addressed in the latest commit:

• Added chain-length > 1 test case (b → a → s) to the existing intermediary variable test
• Added Issue8MethodReceiverTest to cover the isInvocationOnVariable path (s.describe() where s traces through an intermediary)
• Added Python Issue8IntermediaryVariableTest covering Cipher(intermediary, ...) resolving through a variable chain
  Cleaned up the existing test - removed @Rule, HeatedSeats, and refactored traceVariable to delegate to traceSymbol

Will open a separate issue for the recursion guard. C# is intentionally out of scope as it doesn't have a traceSymbol equivalent yet.

[Ayush-Patel-56]

Issue created: #400

[Ayush-Patel-56]

Opened #401 to add a recursion guard to traceSymbol for the mutual assignment cycle case (a = b; b = a) that scenario is not covered by this PR but is a natural edge case of the same tracing logic.

[n1ckl0sk0rtge]

Re-reviewed the latest push. I built the branch, ran the tests, and then reverted the two engine files to check that the new tests really go red.

Good news first:

• The Java fix works. Both Java tests fail without the engine change (Expected size: 1 but was: 0) and pass with it. That is a proper red-to-green test. Thank you for the method-receiver case and the longer chain.
• No regressions. The branch is 47 commits behind main but merges cleanly. I test-merged and ran the full engine, java and python suites on the merged result: all green.

Two things still block merge. Both are inline:

1. The Python test does not test anything. It is green with the Python change removed, and the new code is never called during the test.
2. The Java trace ignores reassignment. That turns a missed detection into a wrong detection, which is worse for a CBOM.

One small note: please also rebase on main before merge, and mention in the PR description that C# is out of scope (see below for why).

FYI only — please do not add this to this PR

While checking the C# question I looked at the other two engines. Two separate gaps. I have opened an issue for each, so nothing here needs to grow this PR:

• C# → #511: CSharpDetectionEngine.java:420 and :439 still compare plain strings. This fix cannot be ported as-is, because CSharpSymbol holds only a name. The ANTLR grammar gives no declaration() and no usages() to walk. C# would need assignment tracking inside CSharpTreeConverter. So your "out of scope" call is right — the reason is architectural, and it is worth writing down.
• Go → #510: the opposite problem. GoDetectionEngine.isInvocationOnVariable and isInitForVariable both return false unconditionally (:555, :565), and run(traceSymbol, tree) ignores its traceSymbol argument. Go matches any call in the enclosing block with no variable check, so a child detection can come from an unrelated variable. 9 Go rule files use addDependingDetectionRules, so this is live today.

I confirmed both with a small repro before filing. The Go one is worse than it looks from the code: with two ECDSA keys in one function, one key raises no finding at all and the other is reported with the wrong curve.

[n1ckl0sk0rtge]

Reassignment is ignored, and that turns a miss into a wrong answer.

traceSymbol follows only the initializer of the declaration. Any later assignment to the same variable is invisible. So the trace can end at an object the variable no longer holds.

I ran this file against the rules from Issue8IntermediaryVariableTest:

LeatherSeats s = new LeatherSeats();
SeatInterface a = s;
a = new HeatedSeats();   // 'a' now holds HeatedSeats
Car c = new Car(a);

	child detections on new Car(a)
main today	0 — nothing found
this PR	1 — LeatherSeats, which is wrong

Today the engine stays quiet. With this change it reports the wrong algorithm. For a CBOM a wrong entry is worse than a missing one, because nobody can tell it is wrong.

This is the Java twin of the Python ordering problem I raised last time, so please treat both the same way. Minimum: a comment here saying reassignment is not tracked, like the one you added on the Python side. Better: if the variable has more than one assignment, stop and return the original symbol instead of guessing.

[n1ckl0sk0rtge]

This method is never executed by any test in the repo.

I put a System.err.println at the top of areSymbolsEquivalent and ran the new Python test. It prints nothing — zero calls.

I then ran the whole Python module (46 tests). The probe fires 8 times in total, all from existing key-agreement tests, and never with a chain that needs tracing:

5x areSymbolsEquivalent private_key vs private_key
1x areSymbolsEquivalent server_private_key vs server_private_key
1x areSymbolsEquivalent parameters vs server_private_key
1x areSymbolsEquivalent dh vs server_private_key

So traceSymbol on the Python side ships untested. Together with the ordering limit you documented in the note above, this is the part I am least comfortable merging. See my comment on the test file for what a real test needs.

[n1ckl0sk0rtge]

This test is green without the fix.

I reverted PythonDetectionEngine.java to main, rebuilt the engine module, and ran this test alone (mvn -pl python -am test -Dtest=Issue8*):

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

So it does not cover the change. The Python engine already handles this case through another path. I also tried wrapping the code in a function, in case module-level scope was the reason. Same result: still green, still no call into the new code.

Compare with the two Java tests. Both fail without the engine change. That is what a regression test should look like.

To make this test real it has to reach isInitForVariable / isInvocationOnVariable with a TraceSymbol that does not match by name. The example from #8 is a good start, because the engine must pick the right one of two candidates:

var = ec.ECDSA("SHA_256")
other_var = ec.ECDSA("SHA_512")   # decoy, must not be picked
intermediary_var = var
result = func1(intermediary_var)

Quickest way to know you got there: put a print in areSymbolsEquivalent, run the test, and check that it fires.