Up to standards ✅

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

Code Review

Overview

This PR resolves 18 open CodeQL alerts across three categories: GitHub Actions workflow over-permissions (15 alerts), Python ReDoS vulnerabilities (2 alerts), and a Java path-injection issue in Neo4jImporter (1 alert). The changes are focused, well-scoped, and the PR description clearly explains each trade-off.

GitHub Actions Workflow Permissions

Positive. All four workflow files correctly adopt the pattern of a restrictive top-level default (contents: read) with per-job elevations only where the job's actions require them. The job-level checks: write additions for dorny/test-reporter jobs follow least-privilege correctly, and mvn-deploy.yml only needs contents: read because deploy credentials travel via secrets, not GITHUB_TOKEN.

Python ReDoS Fixes

Positive. The fix is technically correct. Removing the s (DOTALL) flag and replacing .* with [^\n]* eliminates the ambiguity that caused catastrophic backtracking: with [^\n]*\n, the newline delimiter cannot be consumed by the body alternation, so the NFA has no exponential branch to explore. Output is byte-identical on valid YAML input.

Minor note. The original \s* matched any Unicode whitespace (including \r, \f, etc.); the replacement [ \t]* matches only horizontal ASCII whitespace. For YAML compose files fetched from a well-known URL this is fine in practice, but worth noting in case the tool ever processes CRLF content from Windows hosts.

Java Path Injection Fix (Neo4jImporter)

Positive overall. Replacing the blocklist with a strict allow-list is the right direction - blocklists require enumerating every bad character, allow-lists are safe by construction. CodeQL rightly recognizes matcher(label).matches() as a DirectoryCharactersGuard, so the alert clears cleanly.

Bug: error message does not match the new behaviour (medium)

The exception message at line 692 still says "Invalid label: must not contain path separators or '..'", which describes the old blocklist semantics. The new allow-list rejects far more than just path separators - it also rejects labels containing Unicode letters, accented characters, dots, colons, brackets, etc. A user with a label like Nodo or MyType.v2 will see an error blaming "path separators" when neither is present, which is confusing.

Suggested replacement:

throw new ImportException("Invalid label '" + label + "': only ASCII letters, digits, underscore, hyphen and space are allowed");

The test assertions at lines 333 and 352 in Neo4jImporterIT use hasMessageContaining("path separators") and will need updating to match the new message text.

Minor: regex character class readability

In Pattern.compile("[A-Za-z0-9_ -]+"), the hyphen at the end of the class (before ]) is treated as a literal, which is correct. However, _ - can look like a range from space to something, and some tooling flags trailing hyphens inconsistently. The comment above mentions hyphen explicitly, which helps. An unambiguous alternative: [-A-Za-z0-9_ ]+ or [A-Za-z0-9_\\- ]+.

Missing test: the stricter allow-list behaviour is not covered

The two existing tests (rejectNodeLabelWithPathTraversal, rejectEdgeLabelWithPathTraversal) use labels like ../../etc/evil, which both the old blocklist and new allow-list reject. There is no test for a label that the old code accepted but the new allow-list rejects - for example a label with a Unicode character or a dot (MyType.v2). Adding one regression test for this case would verify the allow-list tightening and lock in the expected exception message.

Summary

The path-injection fix and workflow permissions are the right approach. The main item to address is aligning the exception message (and test assertion) with what the allow-list actually enforces.

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.35%. Comparing base (3f6c078) to head (5e9b2ce).

@@             Coverage Diff              @@
##               main    #4388      +/-   ##
============================================
+ Coverage     64.32%   64.35%   +0.02%     
- Complexity      424      431       +7     
============================================
  Files          1647     1647              
  Lines        127835   127836       +1     
  Branches      27403    27403              
============================================
+ Hits          82233    82263      +30     
+ Misses        34046    34020      -26     
+ Partials      11556    11553       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Code Review

Overview

This PR resolves 18 open CodeQL alerts across three categories: workflow permissions (15), Python ReDoS (2), and Java path-injection (1). It also bundles an undocumented behavioral change to the Python vector embedding API. The security fixes are well-motivated and technically sound; one issue requires attention before merging.

Workflow Permissions (.github/workflows/*.yml) - Approved

The layered permission model is correct:

• Top-level contents: read establishes a restrictive default that caps every job.
• Per-job elevations (checks: write, security-events: write, pull-requests: write) are scoped precisely to the jobs that need them.
• The pre-existing studio-e2e job that already had contents: read simply gains checks: write — no regression there.

Python ReDoS Fix (11_vector_index_build.py, 12_vector_search.py) - Approved

The root cause is correctly identified: (?ms) lets . swallow \n, making (?:.*\n)*? ambiguous because the \n delimiter can also be consumed by ., forcing the engine into exponential backtracking on adversarial input.

The fix is the right approach:

• \s* → [ \t]* (whitespace without newlines)
• .* → [^\n]* (match characters up to, but not including, newline)
• Drop the (?s) (DOTALL) flag entirely

Each [^\n]*\n token is now unambiguous — there is exactly one way to match it — eliminating the backtracking. The PR description's claim that output is byte-identical on valid input is correct.

Java Path-Injection Fix (Neo4jImporter.java) - Minor issue

The allow-list approach ([A-Za-z0-9_ -]+) is strictly more secure than the deny-list it replaces and is the correct fix for CodeQL's DirectoryCharactersGuard recognizer. Compiling the pattern as a static final constant is good practice.

Issue: error message no longer matches the validation constraint.

The existing error message — "Invalid label: must not contain path separators or '..'" — described the old deny-list. The new allow-list rejects far more characters: any period (.), any non-ASCII character, @, #, etc. A user whose label is "Person.Employee" or "Ärzte" will get a message that doesn't explain why their label was rejected.

Suggested fix:

throw new ImportException("Invalid label '" + label + "': only ASCII letters, digits, underscore, hyphen and space are allowed");

Minor: empty string behaviour changed. The old code accepted empty strings (condition required at least one forbidden character); [A-Za-z0-9_ -]+ rejects them (+ requires ≥1 character). This is almost certainly the right behaviour, but worth noting.

Note on the character class: [A-Za-z0-9_ -] — in Java regex, - immediately before ] is a literal hyphen, so the class is A-Z | a-z | 0-9 | _ | space | -. This is correct.

Python Vector API change (vector.py, test_vector.py) - Needs documentation

This change is not mentioned in the PR description but is a breaking behavioral change:

The _uses_reverse_score_sort() helper is removed entirely; _sort_results now always sorts ascending. The nearest-neighbor ranking is unchanged (nearest item is always first), but the numeric score values returned to callers change completely for EUCLIDEAN queries. Any user code comparing or thresholding these scores will break silently.

This belongs in the PR description with an explicit "breaking change" callout and should probably appear in the project's changelog / release notes. If this is intentional, please add a note to the description explaining the motivation (the code comment says the engine already returns distances natively, so the old conversion was superfluous — that rationale should be visible at the PR level).

Summary