Bind caller-supplied names as query parameters instead of concatenating

[dkayiwa]

Summary

An audit for SQL string concatenation turned up two native/JDBC queries that concatenated a caller-supplied string directly into the SQL text instead of binding it. Every other value in these queries is already bound or type-safe, so these were the two places a caller-supplied string reached the SQL as literal text.

Change

• HibernateProgramWorkflowDAO.getPatientProgramAttributeByAttributeName: bind attributeName as a named parameter (:attributeName) so it is always handled as a literal attribute-type name. The patient-id list stays inlined since those elements are Integers.
• FilterUtil.getGlobalPropertyValue(String): replace the concatenated global_property lookup with a PreparedStatement that binds the property name. This runs before the Spring context is up (setup/upgrade wizard) via a raw JDBC connection, so it does not go through the usual service layer. Not exploitable today since the only caller passes a hardcoded constant, but the signature accepts an arbitrary string.

There is no behaviour change for legitimate attribute or property names.

Tests

• ProgramWorkflowServiceTest#getPatientProgramAttributeByAttributeName_shouldMatchTheAttributeNameLiterally: stores a ProgramId attribute on an existing enrollment and checks that a real name resolves the stored value, while a name containing quote characters is compared literally and resolves no attribute type.
• FilterUtilTest#getGlobalPropertyValue_shouldMatchThePropertyNameLiterally: a real property name resolves its stored value, while a name carrying SQL control characters is matched literally and returns null. The payload was chosen so it resolved a single row (returning a value) on the old concatenated query, so the test fails on the pre-change code and passes on the fix.

ProgramWorkflowServiceTest and FilterUtilTest pass and spotless is clean.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.66%. Comparing base (d8fe136) to head (1661217).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #6335      +/-   ##
============================================
+ Coverage     59.47%   59.66%   +0.18%     
- Complexity     9541     9573      +32     
============================================
  Files           731      731              
  Lines         38323    38403      +80     
  Branches       5587     5598      +11     
============================================
+ Hits          22794    22912     +118     
+ Misses        13489    13447      -42     
- Partials       2040     2044       +4     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dkayiwa]

@claude review

[ibacher]

Have we run an audit for any other similar string concatentations? I recall seeing a few.

[dkayiwa]

Yeah, I went through api and web looking for concatenated strings going into createNativeQuery/createSQLQuery/createQuery. This DAO method was the only place where a caller-supplied string actually ended up in the SQL as a literal.

The other concatenation spots are fine: some inline integers only, HibernatePatientDAO.getDuplicatePatientsByAttributes whitelists the column names against the entity fields, a few use hardcoded table/column names, and the rest either concatenate parameter names or just toggle clauses on booleans while keeping the values bound.

The one exception was FilterUtil.getGlobalPropertyValue(String), which built raw JDBC SQL by concatenating the property name. It wasn't exploitable since the only caller passes a hardcoded constant, but the signature takes any string and it runs before the app context is up, so I've folded a fix into this PR that binds it via a PreparedStatement, plus a regression test.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud