[yangjj-iso]

Fixes #7595. Swagger import, MCP import and connection testing rejected public HTTP services on 8080/8081/9090/9091. They now permit these application ports by default and share a configurable port policy. An empty shenyu.swagger.allowed-ports permits any non-denied port; denied-ports replaces the infrastructure-port defaults and takes precedence. Configured ports outside 1–65535 fail binding. The application configuration documents the policy. Resolve each request destination once, validate every returned address and pass an immutable, hostname-restricted DNS result into the actual OkHttp request. The dedicated document fetch disables proxies and both redirect types and uses an isolated zero-idle connection pool so another client's connection cannot bypass the validated resolution. Preserve the original hostname for HTTP Host and HTTPS certificate/SNI handling. Private, loopback, link-local, unspecified, multicast, IPv6 unique-local and shared-address destinations remain blocked even when a port is allowed; mixed public/private DNS responses fail closed. The general HttpUtils methods retain their existing internal-upstream and redirect behavior. Keep the existing two-argument programmatic service constructor with default policy; Spring injects the configured policy. Validation failures retain IllegalArgumentException and connection testing still returns false. Body-size limits and response closure remain in place. A neighboring file-helper test now uses URL.toURI() so its resource lookup works in checkout paths containing spaces. Validation on Microsoft OpenJDK 17.0.18, through the Maven wrapper: - Before the fix, all four new application-port cases failed against the original implementation; the existing parser-confusion regression passed. - After the fix, 101 focused tests passed with zero failures, errors or skips. Coverage includes all three service paths, default/configured port policy, configuration binding, private and alternate IP forms, mixed DNS answers, IPv6 ULA/multicast, immutable DNS snapshots and resolver failures. - Real HTTP transport tests use a local server and a recording socket factory: they verify the selected address stays pinned despite a changing resolver, an existing unvalidated pooled connection is not reused, the Host header is preserved, inherited proxies are bypassed and a redirect never reaches its internal target. - Existing Swagger controller, MCP registration, response-size and generic HttpUtils tests pass. - Checkstyle, Apache RAT and git diff --check pass. sh ./mvnw -B -ntp -pl shenyu-admin -am test apache-rat:check \ -Dtest=UrlSecurityUtilsTest,ValidatedHttpRequestTest,SwaggerImportPropertiesTest,SwaggerImportServiceTest,SwaggerImportServiceImplTest,SwaggerImportControllerTest,HttpUtilsTest \ -Dsurefire.failIfNoSpecifiedTests=false -DskipRemoteResources=true \ -Djacoco.skip=true -Dmaven.javadoc.skip=true No live external DNS-rebinding service or Docker E2E environment was used. DNS changes and transport destination selection are tested deterministically without external network access; full-repository checks remain with CI. Review follow-up (October 20): explicit 16-byte IPv4-mapped IPv6 DNS answers now use the same IPv4 range checks. The 13 new internal-address cases failed before the fix; all 120 focused tests pass afterward on Java 17, including six public mapped-address boundary cases and the real HTTP pinning/proxy/redirect tests. Checkstyle, affected-reactor RAT and git diff --check pass. The standard Java resolver normally collapses mapped literals to IPv4; the new cases also cover resolvers that preserve Inet6Address.

The previous CodeQL analysis job succeeded, while its separate code-scanning check reported an SSRF alert at HttpUtils.requestForValidatedResponse. The production call sites validate the same URL, pin every DNS answer, isolate the connection pool, disable proxies and reject redirects. The alert remains subject to maintainer triage; this local test result does not establish a green CodeQL check. A fresh scan is triggered by the follow-up commit.

• Read the contribution guidelines. - [x] Add regression coverage. - [ ] Full-repository clean install with all tests (affected-reactor validation is recorded above).

CI follow-up on 2026-10-22: merged current master (be34dd1); all 114 Swagger properties/service/URL-security/HTTP utility regressions pass. The old CodeQL check reports SSRF at HttpUtils.requestForValidatedResponse. The request path validates every address, pins resolution, disables redirects/proxies and isolates the connection pool; existing tests exercise these controls. The alert has not been suppressed or dismissed. Reading full alert 87 is denied to this contributor, so final alert classification requires maintainer access or the new CodeQL run.

Shared CI fixture corrections: CryptorResponsePluginTest now checks the post-#7903 configuration error for both encrypt/decrypt when a field is absent, including absence of the original user data. SDK tests wait up to 60 seconds for usable registered routes before exercising assertions. ZooKeeper registration tests keep their declared backend ports reachable so the real health checker cannot race registry-removal assertions. Validation: 16 cryptor unit cases and 16 Admin discovery/health cases pass; the HTTP and SDK integration test reactors compile with Checkstyle. Full Docker integration execution is not claimed: the local Linux Docker engine is unavailable. Remote check results are tracked separately.

[Aias00]

Review: #7868 fix(admin): allow application ports and pin Swagger import DNS

Reviewed the SSRF hardening in the Swagger import / HTTP utilities path.

What's good

• Strong SSRF protection in HttpUtilsConfiguration / SwaggerImportProperties / requestForValidatedResponse:
  • Resolves the host once and pins the DNS answers so a mid-request re-resolution cannot redirect to an internal address.
  • Disables redirects and proxies, and uses an isolated connection pool so the import path cannot be coerced into unintended routing.
  • Address validation covers IPv4 and IPv6, including CGNAT 100.64/10, reserved 240/4, 0.0.0.0, link-local and multicast ranges.
• Backed by real HttpServer integration tests rather than mocks, which is the right level of coverage for security-sensitive code.

Non-blocking suggestions

• Consider how IPv4-mapped IPv6 addresses (::ffff:a.b.c.d) are handled by the address checks — they may take the IPv6 branch and bypass the IPv4-range checks. If that matters for your threat model, normalize/normalize-and-check both representations.
• The build (~3s) and CodeQL (~5s) failures, plus the e2e-case matrix failures, in the latest run are infra/network flakiness: CodeQL at 5s is a setup failure (it never ran), and e2e-case jobs are network/registry dependent and are not affected by an admin-only change. A re-run should pass.

CI note

The decisive pr_build job is not in the failure list for this PR, i.e. the admin compile + unit tests passed. The red jobs are unrelated/flaky.

Verdict

APPROVE — well-designed SSRF hardening with appropriate tests.

[Aias00]

Re-review: Approved

Re-approving following commit ccc6903c39 (fix(admin): validate IPv4 ranges in mapped IPv6 DNS answers).

Verification:

• UrlSecurityUtils.java now correctly detects IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) via isIpv4MappedAddress and validates the embedded IPv4 bytes against internal address ranges.
• Comprehensive parameterized unit tests in UrlSecurityUtilsTest cover both blocked internal ranges and allowed public ranges for IPv4-mapped addresses.
• pr_build passed (the lone CodeQL check failure is the known runner setup issue).

SSRF protection and DNS pinning are verified. Approved!

[Aias00]

Re-review after 290e21f5: DNS pinning is the right mechanism, and the deny-list change is documented

This is a meaningful improvement over the previous InetAddress.getAllByName check, because it closes the TOCTOU window: the addresses that were validated are the addresses the request connects to.

Verified:

• validatedDns parses with HttpUrl.parse (the same parser OkHttp uses), checks the port policy, resolves through the supplied resolver, rejects the URL if any answer is internal, then returns a Dns that serves an immutable copy and throws UnknownHostException for any other hostname. Pinning to the validated hostname as well is the detail that makes this safe.
• requestForValidatedResponse isolates the connection: newBuilder() (so the configured connect/read/write timeouts carry over - I checked, HttpUtils sets all three), Proxy.NO_PROXY, a fresh ConnectionPool(0, 1s), and both redirect flags off. Without all four, a pooled connection or a 302 could reach a destination that was never validated.
• isInternalAddress is stricter than the old regex list: it now covers 0.0.0.0/8, 100.64/10 (CGNAT), >=224, IPv4-mapped IPv6 via isIpv4MappedAddress, and fc00::/7 - which Java does not report as site-local, so the old code was blind to unique-local IPv6.
• Removing 8080/8081/9090/9091 from the default deny list is the point of the issue, and it is documented in both SwaggerImportProperties and application.yml. Internal addresses are still rejected by the resolution check, so a public :8080 is the only newly reachable case.
• SwaggerImportProperties validates the port range at bind time, so a typo fails the Admin startup instead of silently allowing nothing (or everything).

Non-blocking:

1. validateHostForSSRF(host, port) now builds http://host:port and delegates. That breaks for an IPv6 literal host (::1), because HttpUrl.Builder.host needs brackets - the old code accepted the string. If any caller can pass an IPv6 literal, normalise it ([..]) or keep a literal check.
2. isInternalIpv4Address does not cover 192.0.0.0/24, 198.18.0.0/15 (benchmarking) or 192.0.2.0/24 (documentation). Low risk, but they are free to add alongside 100.64/10.
3. Disabling redirects changes behaviour for sources that serve http -> https or -> /swagger/v3/api-docs. That is intentional and documented, but it belongs in RELEASE-NOTES.md as a behaviour change, not only in the properties javadoc.
4. Same shared-CI-commit issue as the sibling PRs (CryptorResponsePluginTest, SdkHttpIntegratedTest, Zookeeper...). Rebase after #7970 lands.

Tests are the right ones: ValidatedHttpRequestTest for the pinning, UrlSecurityUtilsTest for the address classes, SwaggerImportPropertiesTest for the port policy.

[Aias00]

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.

// 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.

[Aias00]

Merged. Heads-up for a follow-up: because the CodeQL alert was not suppressed in this PR, it will surface on master as a new critical SSRF finding in the security tab. It is a false positive (the URL is already validated and the resolved addresses are pinned), so it is worth a small follow-up that adds the // codeql[java/ssrf] justification comment, or a dismissal on the alert itself.