Aias00 commented on PR #7874:
URL: https://github.com/apache/shenyu/pull/7874#issuecomment-6285805608
Follow-up on the CodeQL point from my review — it is not cosmetic after all.
The `CodeQL` check has failed on this PR with a new alert:
```
shenyu-common/src/main/java/org/apache/shenyu/common/utils/RewriteHandleUtils.java:61
[failure] Regular expression injection
This regular expression is constructed from a [user-provided value].
1 new alert including 1 high severity security vulnerability
```
Line 61 is exactly the line that carries the inline suppression:
```java
Pattern.compile(regex + "|").matcher("").replaceAll(replace); //
lgtm[java/regex-injection]
```
So the inline comment is present on the alert's own line and the alert was
still raised, which means the suppression is not being honoured by this
repository's setup. Note also that the PR description says the suppression uses
`// codeql[java/regex-injection]`, while the code uses the older `// lgtm[...]`
spelling — that mismatch is the most likely place to start.
Two things I would try, in this order:
1. Use the current directive spelling and put it on its own line immediately
above the statement:
```java
// codeql[java/regex-injection]
Pattern.compile(regex + "|").matcher("").replaceAll(replace);
```
If the older `lgtm` form is still recognised by your local CodeQL CLI, that
would explain why it looked suppressed locally but the hosted analysis
disagrees.
2. If the alert still appears, do not contort the production code to satisfy
the analyser. The alert is a genuine false positive here — the value is only
compiled and matched against an empty string, and the `catch` blocks turn any
malformed pattern into a validation message rather than a request-path failure.
Merging with the alert and dismissing it as "false positive" with that
justification is the honest resolution, and the dismissal comment stays on the
alert timeline for audit. The catch is that the PR-level check will stay red
until the alert exists on a branch.
Everything else in this PR is still fine as far as I am concerned; my
approval stands on the code. This is only about getting the check green before
merge. The rest of the matrix (22 checks) is still running, so there is time to
sort this out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
