CAMEL-24298: add an optional allowedSchemes allow-list to the toD dynamic-URI EIP - #25315
Merged
Conversation
…amic-URI EIP The dynamic-URI EIP toD computes its recipient endpoint uri from a route-author expression at runtime, with no way to restrict which component schemes a dynamic recipient may resolve to. Add an optional allowedSchemes attribute (comma-separated component-scheme allow-list) on ToDynamicDefinition, wired through ToDynamicReifier into SendDynamicProcessor, which rejects a resolved recipient whose scheme is not in the list (independently of ignoreInvalidEndpoint). Default unset = any scheme allowed (no behavioural change). Useful for low-code / Kamelet deployments. wireTap extends ToDynamicDefinition and shares the reifier/processor path, so it inherits and enforces the same option. The 5 sibling dynamic-URI EIPs (recipientList, routingSlip, dynamicRouter, enrich, pollEnrich) can follow in later PRs once this pattern is reviewed. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
gnodet
reviewed
Aug 3, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Nice work on this defence-in-depth feature — the enforcement placement between prepareRecipient() and resolveEndpoint() is exactly right, and the deliberate bypass of ignoreInvalidEndpoint for disallowed schemes is a sound security decision. Two non-blocking suggestions below.
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
reviewed
Aug 3, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Nice feature — clean model/reifier/processor layering, correct placement of the security check (after prepareRecipient, before resolveEndpoint, outside the ignoreInvalidEndpoint catch), and sound decision to always hard-fail a disallowed scheme.
I agree with the two suggestions from the prior review (pre-parse allowedSchemes into a Set<String> to avoid per-exchange split(","), and add a multi-scheme test). Two additional non-blocking observations below.
This review does not replace specialized 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.
Claude Code on behalf of @davsclaus
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 551 tested, 26 compile-only — current: 548 all testedMaveniverse Scalpel detected 577 affected modules (current approach: 548).
|
…tests, YAML example) - SendDynamicProcessor: pre-parse allowedSchemes into a Set once in the setter, so the per-exchange check is a single Set.contains with no allocation on the hot path (gnodet). - Add a multiple-allowed-schemes test and a test proving a disallowed scheme is rejected even when ignoreInvalidEndpoint=true (gnodet, davsclaus). - toD EIP doc: add a YAML DSL example alongside the Java and XML ones (davsclaus). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
davsclaus
approved these changes
Aug 4, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Clean, well-layered implementation — correct enforcement placement (after prepareRecipient, before resolveEndpoint, bypassing ignoreInvalidEndpoint via exchange.setException() + return), pre-parsed Set<String> for hot-path efficiency, and solid test coverage including the security-critical ignoreInvalidEndpoint independence contract.
All 4 suggestions from prior reviews (pre-parse into Set, multi-scheme test, ignoreInvalidEndpoint test, YAML doc example) addressed in 92389a8.
Non-blocking observations:
- CI JDK 17/25 test builds still in progress — recommend waiting for green before merging.
- No explicit test for whitespace tolerance in the allow-list (e.g.
"mock, seda") — the code handles it viaallowed.trim()in the setter but it's untested. Very minor follow-up. - The
securitygroup label on the@Metadataannotation is a good precedent — worth maintaining as sibling EIPs get the same option.
This review does not replace specialized 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.
Claude Code on behalf of @davsclaus
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The dynamic-URI EIP
toDcomputes its recipient endpoint uri from a route-author expression at runtime (e.g.toD("${header.target}")), with no way to restrict which component schemes the recipient may resolve to. This adds an optionalallowedSchemesattribute (comma-separated component-scheme allow-list) onToDynamicDefinition, wired throughToDynamicReifierintoSendDynamicProcessor.A resolved recipient whose scheme is not in the list is rejected with a
ResolveEndpointFailedException— independently ofignoreInvalidEndpoint, so a disallowed scheme always hard-fails. Default unset = any scheme allowed, so there is no behavioural change. Useful for low-code / Kamelet deployments that want to constrain e.g..toD("${header.dest}")to a fixed set of components.Scope (MVP)
This is the
toD-only MVP of CAMEL-24298.wireTapextendsToDynamicDefinitionand shares the reifier/processor path, so it inherits and enforces the same option automatically. The 5 sibling dynamic-URI EIPs (recipientList,routingSlip,dynamicRouter,enrich,pollEnrich) can follow in separate PRs once this pattern is reviewed and accepted — so the JIRA stays open after this merges.Tests
ToDynamicAllowedSchemesTest: an allowed scheme is sent; a disallowed scheme is rejected withResolveEndpointFailedException; multiple allowed schemes (mock,seda) are honored; and a disallowed scheme is rejected even withignoreInvalidEndpoint=true. Full-reactormvn clean install -DskipTestsis green (model JSON, XML/YAML schemas, DSL writers/parsers/deserializers, and the toD EIP doc regenerated).Docs
Added an
allowedSchemessection to the toD EIP doc (Java, XML, and YAML examples).Backport
main only — new additive feature (default unrestricted), not a bug fix.
Partially addresses CAMEL-24298 (toD MVP; sibling EIPs to follow).
Claude Code on behalf of Andrea Cosentino (@oscerd)