zbendhiba commented on PR #24473:
URL: https://github.com/apache/camel/pull/24473#issuecomment-4915554107
> ### Discussion: passing tool arguments as an exchange variable instead of
headers
> This PR changes how tool arguments reach the route compared to
`camel-langchain4j-tools`: instead of one header per argument, everything is
wrapped in a single `AiToolArguments` object under the `AiTool.TOOL_ARGUMENTS`
exchange variable. We think this is the right call and want to make the
reasoning and consequences explicit before steps 2–5 build on it:
>
> **Behavioral changes**
>
> 1. **Behavioral break in steps 2–5**: every existing tool route reading
`${header.city}` or using sql `:#param` binding will fail at runtime, not at
startup. The upgrade guide entries promised in the roadmap need to spell out
the migration pattern very explicitly.
> 2. **Lost binding convenience**: the documented replacement patterns
should be either simple OGNL (`:#${variable.AiToolArguments.parameters[id]}`),
or a one-line processor extracting arguments into route-author-named headers
(safe, since the names are then trusted).
>
> **Why variables are safer than headers**
>
> * LLM output is untrusted input under Camel's threat model, and headers
are exactly the surface behind the
[CVE-2025-27636](https://github.com/advisories/GHSA-2c2h-2855-mf97)
header-injection family (the `allowedParams` filter in
`LangChain4jToolsProducer` is a hardening patch over this very risk).
> * Headers propagate downstream: a tool route doing `.to("http:...")` or
`.to("jms:...")` forwards LLM-controlled values as HTTP headers / JMS
properties under default filter strategies. Variables are never auto-propagated
and never drive component dispatch.
> * Wrapping all arguments under one constant key means even argument
_names_ can't pollute any namespace.
>
> **Open question**: keep the single wrapper variable (most conservative),
or additionally set one variable per _declared_ argument (`${variable.city}`)
for ergonomics? Declared names are route-author-chosen, so per-argument
variables would not reopen the injection surface.
>
> _Claude Code on behalf of @Croway_
>
> @zbendhiba @orpiske @davsclaus @oscerd this is something worth discussing,
from a user perspective the header approach is more elegant, `${header.city}`
or using sql `:#param` compared to
`:#${variable.AiToolArguments.parameters[id]}`
I assume this change. Keep in mind that this is internal to components using
the camel ai tool. I've already started implementing for camel langchain4j
agent and camel spring ai chat components. There is 0 impact for users. there's
no reason to propagate headers as those are the internal LLM calls.
I didn't spent time on langchain4j-tools internal. I marked in the spec this
component as to deprecate.
--
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]
