This is an automated email from the ASF dual-hosted git repository. acosentino pushed a commit to branch task-36-1 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 6e3c608847e75c1f685131422a4d291afd56d3fc Author: Andrea Cosentino <[email protected]> AuthorDate: Sun Mar 1 13:23:17 2026 +0100 CAMEL-23107 - camel-docling - Add CLI command validation in buildDoclingCommand Signed-off-by: Andrea Cosentino <[email protected]> --- .../camel/component/docling/DoclingProducer.java | 38 +++++ .../component/docling/DoclingCliInjectionTest.java | 171 +++++++++++++++++++++ 2 files changed, 209 insertions(+) diff --git a/components/camel-ai/camel-docling/src/main/java/org/apache/camel/component/docling/DoclingProducer.java b/components/camel-ai/camel-docling/src/main/java/org/apache/camel/component/docling/DoclingProducer.java index 8eeeb8a28e9c..114040f2e244 100644 --- a/components/camel-ai/camel-docling/src/main/java/org/apache/camel/component/docling/DoclingProducer.java +++ b/components/camel-ai/camel-docling/src/main/java/org/apache/camel/component/docling/DoclingProducer.java @@ -1752,11 +1752,49 @@ public class DoclingProducer extends DefaultProducer { @SuppressWarnings("unchecked") List<String> customArgs = exchange.getIn().getHeader(DoclingHeaders.CUSTOM_ARGUMENTS, List.class); if (customArgs != null && !customArgs.isEmpty()) { + validateCustomArguments(customArgs); LOG.debug("Adding custom Docling arguments: {}", customArgs); command.addAll(customArgs); } } + /** + * Validates custom CLI arguments to prevent injection of flags that could write to arbitrary directories or + * override security-critical options managed by the producer. + */ + private void validateCustomArguments(List<String> customArgs) { + // Flags that control output location — the producer manages the output directory + // and allowing these to be overridden via headers would bypass that control. + List<String> blockedFlags = List.of("--output", "-o"); + + for (int i = 0; i < customArgs.size(); i++) { + String arg = customArgs.get(i); + + if (arg == null) { + throw new IllegalArgumentException("Custom argument at index " + i + " is null"); + } + + // Block flags that override output directory + String argLower = arg.toLowerCase(); + for (String blocked : blockedFlags) { + if (argLower.equals(blocked) || argLower.startsWith(blocked + "=")) { + throw new IllegalArgumentException( + "Custom argument '" + blocked + + "' is not allowed because the output directory is managed by the producer. " + + "Use the " + DoclingHeaders.OUTPUT_FILE_PATH + + " header or endpoint configuration instead."); + } + } + + // Reject arguments containing path traversal sequences + if (arg.contains("../") || arg.contains("..\\")) { + throw new IllegalArgumentException( + "Custom argument at index " + i + + " contains path traversal sequence and is rejected for security reasons"); + } + } + } + private void addOutputFormatArguments(List<String> command, String outputFormat) { if (outputFormat != null && !outputFormat.isEmpty()) { command.add("--to"); diff --git a/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingCliInjectionTest.java b/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingCliInjectionTest.java new file mode 100644 index 000000000000..5a51ca8744bb --- /dev/null +++ b/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingCliInjectionTest.java @@ -0,0 +1,171 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.docling; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.apache.camel.CamelExecutionException; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests that custom CLI arguments injected via the {@link DoclingHeaders#CUSTOM_ARGUMENTS} header are validated to + * prevent overriding the output directory managed by the producer. + * + * <p> + * Before this fix, passing {@code --output /arbitrary/path} via the header would silently override the producer-managed + * output directory, potentially writing files to unintended locations. After the fix, such arguments are rejected with + * an {@link IllegalArgumentException}. + */ +class DoclingCliInjectionTest extends CamelTestSupport { + + @TempDir + Path tempDir; + + @Test + void customArgsWithOutputFlagAreRejected() throws Exception { + Path inputFile = createInputFile(); + + // Before the fix, this --output flag would have been passed directly to + // the docling process, overriding the producer-managed output directory. + // After the fix, it is rejected before the process is ever started. + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBodyAndHeaders("direct:cli-convert", + inputFile.toString(), + java.util.Map.of(DoclingHeaders.CUSTOM_ARGUMENTS, List.of("--output", "/tmp/attacker-dir"))); + }); + + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("--output")); + assertTrue(ex.getCause().getMessage().contains("not allowed")); + } + + @Test + void customArgsWithOutputEqualsFormAreRejected() throws Exception { + Path inputFile = createInputFile(); + + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBodyAndHeaders("direct:cli-convert", + inputFile.toString(), + java.util.Map.of(DoclingHeaders.CUSTOM_ARGUMENTS, List.of("--output=/tmp/attacker-dir"))); + }); + + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("--output")); + } + + @Test + void customArgsWithShortOutputFlagAreRejected() throws Exception { + Path inputFile = createInputFile(); + + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBodyAndHeaders("direct:cli-convert", + inputFile.toString(), + java.util.Map.of(DoclingHeaders.CUSTOM_ARGUMENTS, List.of("-o", "/tmp/attacker-dir"))); + }); + + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("-o")); + } + + @Test + void customArgsWithPathTraversalAreRejected() throws Exception { + Path inputFile = createInputFile(); + + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBodyAndHeaders("direct:cli-convert", + inputFile.toString(), + java.util.Map.of(DoclingHeaders.CUSTOM_ARGUMENTS, List.of("--some-flag", "../../etc/passwd"))); + }); + + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("path traversal")); + } + + @Test + void customArgsWithNullEntryAreRejected() throws Exception { + Path inputFile = createInputFile(); + List<String> argsWithNull = new java.util.ArrayList<>(); + argsWithNull.add(null); + + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBodyAndHeaders("direct:cli-convert", + inputFile.toString(), + java.util.Map.of(DoclingHeaders.CUSTOM_ARGUMENTS, argsWithNull)); + }); + + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("null")); + } + + @Test + void safeCustomArgsPassValidation() throws Exception { + Path inputFile = createInputFile(); + + // Safe arguments should pass validation and proceed to process execution. + // The process itself will fail (docling binary not installed in test env), + // but the error should NOT be IllegalArgumentException — that proves + // the validation passed and execution moved on to ProcessBuilder. + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBodyAndHeaders("direct:cli-convert", + inputFile.toString(), + java.util.Map.of(DoclingHeaders.CUSTOM_ARGUMENTS, List.of("--verbose", "--table-mode", "fast"))); + }); + + // The failure should be from process execution, not from argument validation + assertFalse(ex.getCause() instanceof IllegalArgumentException, + "Safe custom arguments should pass validation; failure should come from process execution, not argument validation"); + } + + @Test + void noCustomArgsIsAllowed() throws Exception { + Path inputFile = createInputFile(); + + // With no custom arguments at all, validation is skipped entirely. + // The process will still fail (no docling binary), but not from validation. + CamelExecutionException ex = assertThrows(CamelExecutionException.class, () -> { + template.requestBody("direct:cli-convert", inputFile.toString()); + }); + + assertFalse(ex.getCause() instanceof IllegalArgumentException, + "No custom arguments should not trigger argument validation"); + } + + private Path createInputFile() throws Exception { + Path file = tempDir.resolve("test-input.txt"); + Files.writeString(file, "test content"); + return file; + } + + @Override + protected RouteBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + // CLI mode (useDoclingServe=false is the default) + from("direct:cli-convert") + .to("docling:convert?operation=CONVERT_TO_MARKDOWN"); + } + }; + } +}
