Aias00 commented on PR #7868: URL: https://github.com/apache/shenyu/pull/7868#issuecomment-6355454362
CI note on head `290e21f5`: the `CodeQL` check is red with **1 new critical alert**: ``` Server-side request forgery shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/HttpUtils.java:379 ``` That is the `client.newCall(new Request.Builder().url(url).get().build()).execute()` inside the new `requestForValidatedResponse`. In substance this is a **false positive** — `url` is the very URL whose host was just validated by `UrlSecurityUtils.validatedDns`, the client pins the resolved addresses via `dns(validatedDns)`, and proxy + redirects are disabled, which is exactly the TOCTOU fix this PR is about. CodeQL simply cannot follow the pinning across the helper boundary. It still blocks merge because the check is required. Practical fix: suppress it in place with a justification, e.g. ```java // codeql[java/ssrf] URL is the one already validated by UrlSecurityUtils; DNS is pinned, proxy and redirects are disabled return client.newCall(new Request.Builder().url(url).get().build()).execute(); ``` (or a maintainer dismisses the alert as a false positive on the PR). My review verdict is unchanged — the pinning approach itself is correct. -- 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]
