gnodet-bot commented on code in PR #28295:
URL: https://github.com/apache/camel/pull/28295#discussion_r4348719897
##########
components/camel-ai/camel-ai-tool/src/main/java/org/apache/camel/component/ai/tool/AiToolExecutor.java:
##########
@@ -131,20 +149,26 @@ public static AiToolResult execute(AiToolSpec spec,
Map<String, Object> argument
// Set each argument as an exchange header so route expressions
(${header.city})
// and SQL bindings work naturally. Filter out Camel-internal names to
prevent
// header-namespace injection (CVE-2025-27636 family).
- for (Map.Entry<String, Object> entry : argsCopy.entrySet()) {
- String name = entry.getKey();
+ bodyArguments.keySet().removeIf(name -> {
String lower = name.toLowerCase(Locale.ROOT);
if (lower.startsWith("camel") ||
lower.startsWith("org.apache.camel.")) {
LOG.warn("Rejecting tool argument '{}' for tool '{}' --
argument names starting with "
+ "'Camel' or 'org.apache.camel.' are reserved for
internal use",
name, toolName);
- continue;
+ return true;
}
- exchange.getMessage().setHeader(name, entry.getValue());
- }
+ return false;
+ });
+ argsCopy.keySet().retainAll(bodyArguments.keySet());
Review Comment:
⚠️ **Potential silent loss of required arguments**
The execution flow is:
1. `argsCopy` = defensive copy of `arguments`
2. `bodyArguments = new HashMap<>(argsCopy)` — same keys initially
3. Undeclared keys filtered from `argsCopy` (required+declared params
survive)
4. Required-argument check runs against `argsCopy` ✅ — declared required
param passes
5. `bodyArguments.keySet().removeIf(name -> lower.startsWith("camel") ||
...)` — strips anything with a `camel`-prefixed name
6. **`argsCopy.keySet().retainAll(bodyArguments.keySet())`** — syncs
`argsCopy` down to `bodyArguments`
If a tool parameter is declared (survives step 3), required (passes step 4),
*and* its name starts with `"camel"` (case-insensitive) — e.g. `"camelCase"`,
`"camelHump"` — it passes validation but is silently removed from both
`argsCopy` (headers) and `bodyArguments` (body) before the route executes. The
route receives neither the header nor the body argument.
This is unlikely in practice since users rarely choose parameter names that
collide with the Camel reserved namespace, but it's a logical gap worth
documenting or guarding against. One option: run the camel-prefix strip on both
`bodyArguments` **and** `argsCopy` before the required-argument check, so
validation and stripping share the same view of the argument set.
--
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]
