CAMEL-24241: Apply the tool argument allowlist to tools declaring no parameters - #25094
Conversation
…parameters AiToolExecutor gated the undeclared-argument filter on !spec.getParameterDefs().isEmpty(), so a tool that declares no parameters skipped filtering entirely and every LLM-supplied argument became an exchange header. A tool declaring no parameters accepts none: buildJsonSchemaFromDefs emits "additionalProperties": false unconditionally, so the schema such a tool advertises already forbids every property. This re-introduced the bypass that CAMEL-23621 (e9c4541, "remove backwards-compatibility bypass") deliberately removed from the sibling producers, which still carry the strict form. LLM output is attacker-influenceable through prompt injection, and tool routes commonly forward headers to downstream components. The Camel-internal prefix rejection still applied, so the CVE-2025-27636 header-injection family was not reachable; what was unfiltered is arbitrary non-Camel header names. Add a regression test asserting that arguments sent to a zero-parameter tool do not reach the exchange headers. testCamelPrefixedArgumentsRejectedFromHeaders used the noParams tool and relied on the bypass to get its non-Camel argument through. Point it at a new prefixTool that declares those names, so the Camel-prefixed arguments survive the allowlist and actually reach the prefix check the test exists to verify -- otherwise it would pass even if that check were deleted. No upgrade guide entry: camel-ai-tool is new in 4.22 (CAMEL-23382) and the permissive behaviour has never been released. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
davsclaus
left a comment
There was a problem hiding this comment.
Clean, well-crafted hardening fix. The bypass regression is confirmed against e9c4541a91ce (CAMEL-23621), and the fix aligns the shared executor with the sibling producers that already carry the strict form.
One minor convention nit on test visibility (inline below), otherwise LGTM.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
gnodet
left a comment
There was a problem hiding this comment.
Clean, minimal, and well-reasoned security hardening fix. Removes the !spec.getParameterDefs().isEmpty() bypass so that zero-parameter tools correctly reject all LLM-hallucinated arguments instead of letting them pass through as exchange headers.
AiToolSpec.getParameterDefs() is guaranteed non-null (returns Map.of() if constructor argument is null), so no NPE risk. The fix is consistent with the strict enforcement already applied in LangChain4jToolsProducer and SpringAiToolsEndpoint (commit e9c4541). The additionalProperties: false schema emission confirms server-side enforcement is the correct complement.
Test coverage is thorough and the existing test was carefully adjusted. The component is new in 4.22 with no release tags, so no upgrade guide is needed.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
|
🌟 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: 11 tested, 29 compile-only — current: 11 all testedMaveniverse Scalpel detected 40 affected modules (current approach: 11).
|
The problem
AiToolExecutorgates the undeclared-argument filter on the parameter map being non-empty:So a tool that declares no parameters skips filtering entirely, and every argument the model sends becomes an exchange header.
A tool declaring no parameters accepts none.
AiToolParameterHelper.buildJsonSchemaFromDefsemits"additionalProperties": falseunconditionally, so the schema such a tool advertises already forbids every property — which is exactly the rationale the filter's own comment cites.This also re-introduced a bypass that was removed on purpose. Commit
e9c4541a91ce("CAMEL-23621: remove backwards-compatibility bypass") stripped the identicalisEmpty()guard from three places:LangChain4jToolsProducerandSpringAiToolsEndpointstill carry the strict form; only the newer shared executor regressed it.Severity
Hardening rather than a CVE. The Camel-internal prefix rejection (
camel/org.apache.camel., case-insensitive) still applies, so the CVE-2025-27636 header-injection family is not reachable through this path. What is unfiltered is arbitrary non-Camel header names originating from model output — and LLM output is attacker-influenceable via prompt injection, while tool routes commonly forward headers to downstream components (HTTP producers, SQL named parameters).The fix
Drop the
&& !spec.getParameterDefs().isEmpty()condition, so an empty declaration means "no arguments accepted" instead of "all arguments accepted".Tests
New:
testExecuteIgnoresUndeclaredArgumentsForToolWithoutParameters— arguments sent to a zero-parameter tool must not reach the exchange headers. Verified it fails without the fix (expected: null but was: "should-be-ignored") and passes with it.Adjusted:
testCamelPrefixedArgumentsRejectedFromHeadersused thenoParamstool and depended on the bypass to get its non-Camel argument through, so the fix broke it. Simply flipping that assertion toisNull()would have made the test vacuous — all four arguments would then be dropped by the allowlist, and it would pass even if the Camel-prefix check were deleted. Instead it now points at a newprefixToolthat declares those names, so the Camel-prefixed arguments survive the allowlist and genuinely exercise the prefix check.One caveat recorded in a comment: a dotted name such as
org.apache.camel.hackcannot be declared through the endpoint URI (parseParameterMetadatasplits the key on the first dot), so that one is dropped as undeclared rather than by the prefix check.Full
AiToolExecutorTestsuite: 13 tests, green.Upgrade guide
Not required —
camel-ai-toolis new in 4.22 (CAMEL-23382) and the permissive behaviour has never appeared in a release.Claude Code on behalf of @oscerd