CAMEL-24371: camel-a2a - fix WebhookUrlValidator address classification and host matching - #25406
CAMEL-24371: camel-a2a - fix WebhookUrlValidator address classification and host matching#25406oscerd wants to merge 1 commit into
Conversation
…on and host matching WebhookUrlValidator classified webhook hosts in two places that did not agree. A host written as an IP literal was checked against a string prefix list, while a host reached through a name was classified with the InetAddress predicates. Those predicates do not cover the same ground: isSiteLocalAddress reports the deprecated fec0::/10 block and not the fc00::/7 unique local addresses that replaced it, so the same address was accepted or rejected depending on how it was written. The literal pre-check also prefix-matched the raw host string without establishing that the host was an IP literal, so any name beginning with fc or fd, such as fcm.googleapis.com, was rejected outright. Both paths now resolve the host and classify the resulting address with one shared raw-byte classifier. InetAddress.getByName already parses bracketed IPv6 literals without touching DNS, so the separate literal path is no longer needed. The classifier additionally recognises fc00::/7, IPv4-compatible IPv6, the NAT64 well-known prefix 64:ff9b::/96, 6to4 under 2002::/16 and the shared address space 100.64.0.0/10. NAT64 and 6to4 addresses are classified by the IPv4 address they embed, so a translation prefix carrying a globally routable address stays allowed. A package-private resolver seam lets the resolved-host path be tested without DNS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 28 compile-only — current: 9 all testedMaveniverse Scalpel detected 38 affected modules (current approach: 9).
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
The code changes to WebhookUrlValidator look solid — the single byte-based classifier is a clear improvement over the dual-path string+predicate approach, fixing real bugs where hostnames starting with "fc" or "fd" (like fcm.googleapis.com) were falsely rejected, and where fc00::/7 unique local addresses weren't caught by InetAddress.isSiteLocalAddress(). The HostResolver seam is clean for testing DNS-dependent code. Test coverage expansion from 13 to 30 tests is great, with proper AssertJ usage and conventions followed throughout.
One documentation concern: the upgrade guide entry is placed in a new camel-4x-upgrade-guide-4_23.adoc file, but main is currently at 4.22.0-SNAPSHOT (latest tag: camel-4.21.0). Changes merging to main now will ship with 4.22.0, so the behavioral changes should be appended to the existing camel-4x-upgrade-guide-4_22.adoc (which already has a camel-a2a section from CAMEL-23876). The new 4_23 file and its index entry in camel-4x-upgrade-guide.adoc are premature — users upgrading to 4.22 would miss this documentation.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Clean, well-tested security bug fix. The unified byte-based classifier is a clear improvement over the dual-path string+predicate approach, fixing real bugs (hostnames like fcm.googleapis.com falsely rejected, fc00::/7 not caught by isSiteLocalAddress()). Test expansion from 13 to 30 tests is thorough, covering each range in both literal and resolved form with proper boundary conditions. The HostResolver seam is a clean testability pattern.
Regarding the upgrade guide version: camel-4.22.0 was tagged today from the release branch, so the new camel-4x-upgrade-guide-4_23.adoc is the correct place for these behavioral changes — they will ship in 4.23.0, not 4.22.0.
This review covers project rules and conventions. It is not a replacement for specialized review tools such as CodeRabbit, Sourcery, or SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
3 participants
Description
WebhookUrlValidatorclassified webhook hosts in two places that did not agree with each other.A host written as an IP literal was checked against a string prefix list (
isPrivateIpv6), while a host reached through a name was classified with theInetAddresspredicates. Those two do not cover the same ground —Inet6Address.isSiteLocalAddress()reports the deprecatedfec0::/10block and not thefc00::/7unique local addresses that replaced it — so the same address was accepted or rejected depending on how it was written. Verified on JDK 21:fd00::1andfc00::1returnfalsefor all four predicates, whilefec0::1returnssite=true.The literal pre-check also prefix-matched the raw host string with
startsWith("fc")/startsWith("fd")without first establishing that the host was an IP literal, so any name beginning with those two characters was rejected outright.https://fcm.googleapis.com/webhookandhttps://fd-edge.example.com/webhookwere both refused.Changes
Both paths now resolve the host and classify the resulting address with a single shared raw-byte classifier.
InetAddress.getByName()already parses bracketed IPv6 literals without touching DNS, so the separate literal path is no longer needed and the two forms cannot drift apart again.The classifier recognises these in addition to the loopback / wildcard / link-local / site-local ranges already handled:
fc00::/7fd00::1::10.0.0.164:ff9b::/9664:ff9b::a00:12002::/162002:c0a8:101::1100.64.0.0/10100.64.0.1NAT64 and 6to4 addresses are classified by the IPv4 address they embed rather than blocked wholesale, so an IPv6-only deployment can still reach public webhook endpoints through a translation prefix.
IPv4-mapped IPv6 needs no new handling — the JDK returns an
Inet4Addressfor::ffff:x.x.x.x, so the existing IPv4 rules already applied.Host names are no longer classified by their spelling.
Notes for reviewers
[fd00::1]now reportsunique localinstead ofprivate/internal IPv6 ranges, and[::1]reportsloopbackinstead of the IPv6 message. Four existing test assertions were updated accordingly. No API signature changed.nonGlobalReasonchecks loopback even though a host reaching loopback directly is answered earlier (whereallowLocalcan let it through). That branch exists for the addresses embedding an IPv4 one —64:ff9b::7f00:1carries127.0.0.1— and is covered by a test.HostResolverseam lets the resolved-host path be tested without DNS and without installing a JVM-wideInetAddressResolverProvider, which would have affected every test in the module.Testing
WebhookUrlValidatorTestgoes from 13 to 30 tests, covering each range above in both literal and resolved form, the adjacent addresses just outside100.64.0.0/10, the NAT64/6to4 cases that must stay allowed, and the host names that were previously rejected for their spelling.camel-a2amodule suite: 519 tests, all passingmvn clean install -DskipTests): SUCCESS, no uncommitted regen artifactsDocumentation
Adds
camel-4x-upgrade-guide-4_23.adocwith a note on the newly rejected ranges and the relaxed host-name matching, and registers it in the upgrade guide index.Claude Code on behalf of @oscerd