Skip to content

Commit 768dc63

Browse files
authored
Fix release workflow GPG secret names to match JFalkorDB pattern (#11)
* Fix release workflow to use correct GPG secret names matching JFalkorDB pattern * Fix critical CodeRabbitAI issues - Fix ORDER BY for count/exists queries (don't add ORDER BY to aggregation queries) - Fix regex injection vulnerability by properly escaping user input with Pattern.quote() - Add parameter bounds checking to prevent ArrayIndexOutOfBoundsException - Fix ID semantics: only set internal IDs, never overwrite external @id properties These changes address security and correctness issues identified by CodeRabbitAI in PR #10.
1 parent 965882d commit 768dc63

File tree

3 files changed

+38
-18
lines changed

3 files changed

+38
-18
lines changed

.github/workflows/release.yml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ jobs:
2626
distribution: 'temurin'
2727
cache: maven
2828
server-id: central
29-
server-username: CENTRAL_USERNAME
30-
server-password: CENTRAL_TOKEN
29+
server-username: MAVEN_CENTRAL_USERNAME
30+
server-password: MAVEN_CENTRAL_TOKEN
3131

3232
- name: Cache Maven dependencies
3333
uses: actions/cache@v4
@@ -45,7 +45,8 @@ jobs:
4545
- name: Get version from tag
4646
id: get_version
4747
run: |
48-
realversion="${GITHUB_REF/refs\/tags\/v/}"
48+
realversion="${GITHUB_REF/refs\/tags\/}"
49+
realversion="${realversion//v/}"
4950
echo "VERSION=$realversion" >> $GITHUB_OUTPUT
5051
5152
- name: Update version in POM
@@ -56,8 +57,8 @@ jobs:
5657
if: github.event_name == 'release'
5758
uses: crazy-max/ghaction-import-gpg@v6
5859
with:
59-
gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }}
60-
passphrase: ${{ secrets.GPG_PASSPHRASE }}
60+
gpg_private_key: ${{ secrets.OSSH_GPG_SECRET_KEY }}
61+
passphrase: ${{ secrets.OSSH_GPG_SECRET_KEY_PASSWORD }}
6162

6263
- name: Start FalkorDB
6364
run: |
@@ -79,9 +80,9 @@ jobs:
7980
--batch-mode \
8081
clean deploy -P release
8182
env:
82-
CENTRAL_USERNAME: ${{ secrets.CENTRAL_USERNAME }}
83-
CENTRAL_TOKEN: ${{ secrets.CENTRAL_TOKEN }}
84-
MAVEN_GPG_PASSPHRASE: ${{ secrets.GPG_PASSPHRASE }}
83+
MAVEN_CENTRAL_USERNAME: ${{ secrets.CENTRAL_USERNAME }}
84+
MAVEN_CENTRAL_TOKEN: ${{ secrets.CENTRAL_TOKEN }}
85+
MAVEN_GPG_PASSPHRASE: ${{ secrets.OSSH_GPG_SECRET_KEY_PASSWORD }}
8586

8687
- name: Build artifacts for workflow dispatch
8788
if: github.event_name == 'workflow_dispatch'

src/main/java/org/springframework/data/falkordb/core/mapping/DefaultFalkorDBEntityConverter.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -656,9 +656,10 @@ private Object saveEntityAndGetId(Object entity, FalkorDBPersistentEntity<?> per
656656
});
657657

658658
if (nodeId != null) {
659-
// Update the entity's ID property if it exists and is writable
659+
// Update the entity's ID property only if it's an internal FalkorDB ID
660+
// Never overwrite external @Id properties with internal node IDs
660661
FalkorDBPersistentProperty idProperty = persistentEntity.getIdProperty();
661-
if (idProperty != null && !idProperty.isImmutable()) {
662+
if (idProperty != null && !idProperty.isImmutable() && idProperty.isInternalIdProperty()) {
662663
accessor.setProperty(idProperty, nodeId);
663664
}
664665

src/main/java/org/springframework/data/falkordb/repository/query/DerivedCypherQueryGenerator.java

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,9 @@ else if (this.partTree.isDelete()) {
130130
cypher.append(" RETURN n");
131131
}
132132

133-
// Add ORDER BY clause
134-
if (sort.isSorted() && !this.partTree.isDelete()) {
133+
// Add ORDER BY clause (only for regular SELECT queries, not for count/exists/delete)
134+
if (sort.isSorted() && !this.partTree.isDelete() && !this.partTree.isCountProjection()
135+
&& !this.partTree.isExistsProjection()) {
135136
cypher.append(" ORDER BY ");
136137
boolean first = true;
137138
for (Sort.Order order : sort) {
@@ -168,6 +169,13 @@ private void appendCondition(StringBuilder cypher, Part part, Map<String, Object
168169

169170
String property = part.getProperty().toDotPath();
170171
String paramName = "param" + parameterIndex.getAndIncrement();
172+
int currentParamIndex = parameterIndex.get() - 1;
173+
174+
// Validate parameter index bounds
175+
if (currentParamIndex >= parameters.length) {
176+
throw new IllegalArgumentException("Parameter index " + currentParamIndex
177+
+ " out of bounds for parameters array of length " + parameters.length);
178+
}
171179

172180
switch (part.getType()) {
173181
case SIMPLE_PROPERTY:
@@ -196,26 +204,36 @@ private void appendCondition(StringBuilder cypher, Part part, Map<String, Object
196204
break;
197205
case LIKE:
198206
cypher.append("n.").append(property).append(" =~ $").append(paramName);
199-
// Convert SQL LIKE to regex pattern
207+
// Convert SQL LIKE to regex pattern with proper escaping
200208
String likeValue = String.valueOf(parameters[parameterIndex.get() - 1]);
201-
String regexPattern = "(?i)" + likeValue.replace("%", ".*").replace("_", ".");
209+
// Split by wildcards, escape each part, then reassemble
210+
String regexPattern = likeValue.replace("%", "\u0000").replace("_", "\u0001");
211+
regexPattern = java.util.regex.Pattern.quote(regexPattern);
212+
regexPattern = regexPattern.replace("\\Q\u0000\\E", ".*").replace("\\Q\u0001\\E", ".");
213+
regexPattern = "(?i)" + regexPattern;
202214
queryParameters.put(paramName, regexPattern);
203215
break;
204216
case STARTING_WITH:
205217
cypher.append("n.").append(property).append(" =~ $").append(paramName);
206-
queryParameters.put(paramName, "(?i)" + parameters[parameterIndex.get() - 1] + ".*");
218+
String startValue = String.valueOf(parameters[parameterIndex.get() - 1]);
219+
queryParameters.put(paramName, "(?i)" + java.util.regex.Pattern.quote(startValue) + ".*");
207220
break;
208221
case ENDING_WITH:
209222
cypher.append("n.").append(property).append(" =~ $").append(paramName);
210-
queryParameters.put(paramName, "(?i).*" + parameters[parameterIndex.get() - 1]);
223+
String endValue = String.valueOf(parameters[parameterIndex.get() - 1]);
224+
queryParameters.put(paramName, "(?i).*" + java.util.regex.Pattern.quote(endValue));
211225
break;
212226
case CONTAINING:
213227
cypher.append("n.").append(property).append(" =~ $").append(paramName);
214-
queryParameters.put(paramName, "(?i).*" + parameters[parameterIndex.get() - 1] + ".*");
228+
String containValue = String.valueOf(parameters[parameterIndex.get() - 1]);
229+
queryParameters.put(paramName,
230+
"(?i).*" + java.util.regex.Pattern.quote(containValue) + ".*");
215231
break;
216232
case NOT_CONTAINING:
217233
cypher.append("NOT n.").append(property).append(" =~ $").append(paramName);
218-
queryParameters.put(paramName, "(?i).*" + parameters[parameterIndex.get() - 1] + ".*");
234+
String notContainValue = String.valueOf(parameters[parameterIndex.get() - 1]);
235+
queryParameters.put(paramName,
236+
"(?i).*" + java.util.regex.Pattern.quote(notContainValue) + ".*");
219237
break;
220238
case IS_NULL:
221239
cypher.append("n.").append(property).append(" IS NULL");

0 commit comments

Comments
 (0)