This is an automated email from the ASF dual-hosted git repository. acosentino pushed a commit to branch CAMEL-23211 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 726ea6f1efd7f01bb84bbe92f25c244c38c0a124 Author: Andrea Cosentino <[email protected]> AuthorDate: Thu Mar 19 10:58:46 2026 +0100 CAMEL-23211 - Camel-Docling: Use secure temp file creation in camel-docling Replace insecure temp file creation in DoclingProducer with per-exchange UUID-named subdirectories under the system temp dir. Set POSIX 700 permissions on directories and POSIX 600 permissions on files when the platform supports it, preventing local attackers from pre-creating symlinks or monitoring temp files. Add createSecureTempDir() and createSecureTempFile() helper methods with automatic POSIX support detection via FileSystems. Replace the old single-file cleanup (registerTempFileCleanup) with directory-level cleanup (registerTempDirCleanup + deleteDirectoryRecursively) that removes the entire per-exchange subdirectory on exchange completion. Fix a pre-existing bug where the CLI output temp directory was only cleaned up when contentInBody=true (default is false), causing temp directory leaks on every CLI invocation. The finally block now always cleans up the output directory. Add DoclingSecureTempFileTest verifying per-exchange subdirectory isolation and POSIX permission enforcement. Update existing DoclingTempFileCleanupTest to scan for directories instead of files. Signed-off-by: Andrea Cosentino <[email protected]> --- .../camel/component/docling/DoclingProducer.java | 99 ++++++++++--- .../docling/DoclingSecureTempFileTest.java | 162 +++++++++++++++++++++ .../docling/DoclingTempFileCleanupTest.java | 31 ++-- 3 files changed, 261 insertions(+), 31 deletions(-) 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 437fa8f540cb..f3f71e7fafe2 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 @@ -21,16 +21,22 @@ import java.io.File; import java.io.IOException; import java.io.InputStreamReader; import java.net.URI; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.time.Duration; import java.util.ArrayList; import java.util.Base64; import java.util.Collection; +import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.ConcurrentHashMap; @@ -137,6 +143,18 @@ public class DoclingProducer extends DefaultProducer { */ private static final Set<String> PRODUCER_MANAGED_FLAGS = Set.of("--output", "-o"); + private static final boolean POSIX_SUPPORTED = FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); + + // Owner-only permissions: rwx for directories, rw for files + private static final Set<PosixFilePermission> DIR_PERMISSIONS_700 = EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE); + + private static final Set<PosixFilePermission> FILE_PERMISSIONS_600 = EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE); + private DoclingConfiguration configuration; private DoclingServeApi doclingServeApi; private ObjectMapper objectMapper; @@ -1643,9 +1661,10 @@ public class DoclingProducer extends DefaultProducer { return content; } else { // Treat as content to be written to a temp file - Path tempFile = Files.createTempFile("docling-", ".tmp"); + Path secureTempDir = createSecureTempDir(); + Path tempFile = createSecureTempFile(secureTempDir); Files.write(tempFile, content.getBytes()); - registerTempFileCleanup(exchange, tempFile); + registerTempDirCleanup(exchange, secureTempDir); validateFileSize(tempFile.toString()); return tempFile.toString(); } @@ -1653,9 +1672,10 @@ public class DoclingProducer extends DefaultProducer { if (content.length > configuration.getMaxFileSize()) { throw new IllegalArgumentException("File size exceeds maximum allowed size: " + configuration.getMaxFileSize()); } - Path tempFile = Files.createTempFile("docling-", ".tmp"); + Path secureTempDir = createSecureTempDir(); + Path tempFile = createSecureTempFile(secureTempDir); Files.write(tempFile, content); - registerTempFileCleanup(exchange, tempFile); + registerTempDirCleanup(exchange, secureTempDir); return tempFile.toString(); } else if (body instanceof File file) { validateFileSize(file.getAbsolutePath()); @@ -1665,20 +1685,63 @@ public class DoclingProducer extends DefaultProducer { throw new InvalidPayloadException(exchange, String.class); } - private void registerTempFileCleanup(Exchange exchange, Path tempFile) { + /** + * Creates a secure per-exchange subdirectory under the system temp dir with a UUID for uniqueness and restrictive + * POSIX permissions (700) when the platform supports it. + */ + private Path createSecureTempDir() throws IOException { + Path tempDir; + if (POSIX_SUPPORTED) { + FileAttribute<Set<PosixFilePermission>> dirAttr = PosixFilePermissions.asFileAttribute(DIR_PERMISSIONS_700); + tempDir = Files.createTempDirectory("docling-" + UUID.randomUUID() + "-", dirAttr); + } else { + tempDir = Files.createTempDirectory("docling-" + UUID.randomUUID() + "-"); + } + LOG.debug("Created secure temp directory: {}", tempDir); + return tempDir; + } + + /** + * Creates a temp file inside the given directory with restrictive POSIX permissions (600) when the platform + * supports it. + */ + private Path createSecureTempFile(Path parentDir) throws IOException { + Path tempFile; + if (POSIX_SUPPORTED) { + FileAttribute<Set<PosixFilePermission>> fileAttr = PosixFilePermissions.asFileAttribute(FILE_PERMISSIONS_600); + tempFile = Files.createTempFile(parentDir, "docling-", ".tmp", fileAttr); + } else { + tempFile = Files.createTempFile(parentDir, "docling-", ".tmp"); + } + return tempFile; + } + + /** + * Registers cleanup of an entire temp directory (and its contents) when the exchange completes. + */ + private void registerTempDirCleanup(Exchange exchange, Path tempDir) { exchange.getExchangeExtension().addOnCompletion(new SynchronizationAdapter() { @Override public void onDone(Exchange exchange) { - try { - Files.deleteIfExists(tempFile); - LOG.debug("Cleaned up temp file: {}", tempFile); - } catch (IOException e) { - LOG.warn("Failed to clean up temp file: {}", tempFile, e); - } + deleteDirectoryRecursively(tempDir); } }); } + private static void deleteDirectoryRecursively(Path dir) { + try { + if (Files.isDirectory(dir)) { + try (Stream<Path> entries = Files.list(dir)) { + entries.forEach(DoclingProducer::deleteDirectoryRecursively); + } + } + Files.deleteIfExists(dir); + LOG.debug("Cleaned up temp path: {}", dir); + } catch (IOException e) { + LOG.warn("Failed to clean up temp path: {}", dir, e); + } + } + private void validateFileSize(String filePath) throws IOException { Path path = Paths.get(filePath); if (Files.exists(path)) { @@ -1692,8 +1755,8 @@ public class DoclingProducer extends DefaultProducer { private String executeDoclingCommand(String inputPath, String outputFormat, Exchange exchange) throws Exception { LOG.debug("DoclingProducer executing Docling command for input: {} with format: {}", inputPath, outputFormat); - // Create temporary output directory - Path tempOutputDir = Files.createTempDirectory("docling-output"); + // Create secure temporary output directory with restrictive permissions + Path tempOutputDir = createSecureTempDir(); try { List<String> command = buildDoclingCommand(inputPath, outputFormat, exchange, tempOutputDir.toString()); @@ -1763,11 +1826,11 @@ public class DoclingProducer extends DefaultProducer { return result; } finally { - // Clean up temporary directory only if contentInBody is true - // (the file has already been read and deleted) - if (configuration.isContentInBody()) { - deleteDirectory(tempOutputDir); - } + // Always clean up the temporary output directory. When contentInBody is true, + // the file content has been read into the exchange body. When contentInBody is false, + // the output file has been moved to its final location. In both cases (and on failure), + // the temp directory is no longer needed. + deleteDirectory(tempOutputDir); } } diff --git a/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingSecureTempFileTest.java b/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingSecureTempFileTest.java new file mode 100644 index 000000000000..102a60919cdd --- /dev/null +++ b/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingSecureTempFileTest.java @@ -0,0 +1,162 @@ +/* + * 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.io.IOException; +import java.nio.file.DirectoryStream; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.apache.camel.CamelExecutionException; +import org.apache.camel.Exchange; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIf; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * Tests that temp files and directories created by DoclingProducer use secure per-exchange subdirectories with + * restrictive POSIX permissions. + */ +class DoclingSecureTempFileTest extends CamelTestSupport { + + @Test + void tempFilesAreCreatedInsidePerExchangeSubdirectory() throws Exception { + // During the exchange, intercept to check what's on disk + final List<Path> capturedDirs = new ArrayList<>(); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() { + from("direct:intercept-convert") + .process(exchange -> { + // Capture docling temp dirs that exist right now (mid-exchange) + capturedDirs.addAll(listDoclingTempDirs()); + }) + .to("docling:convert?operation=CONVERT_TO_MARKDOWN"); + } + }); + + // Snapshot before + List<Path> before = listDoclingTempDirs(); + + try { + template.requestBody("direct:intercept-convert", "Some text content"); + } catch (CamelExecutionException e) { + // Expected — docling binary not available + } + + // The temp file should have been inside a docling-UUID subdirectory, + // and after cleanup it should be gone. + List<Path> after = listDoclingTempDirs(); + List<Path> leaked = new ArrayList<>(after); + leaked.removeAll(before); + + assertTrue(leaked.isEmpty(), + "Temp directories leaked after exchange completion: " + leaked); + } + + @Test + @EnabledIf("isPosixSupported") + void tempDirectoryHasOwnerOnlyPermissions() throws Exception { + // We need to intercept the exchange mid-flight to check permissions + // before cleanup removes the directory. + final List<Set<PosixFilePermission>> capturedPermissions = new ArrayList<>(); + + context.addRoutes(new RouteBuilder() { + @Override + public void configure() { + from("direct:check-perms") + .to("docling:convert?operation=CONVERT_TO_MARKDOWN"); + } + }); + + List<Path> before = listDoclingTempDirs(); + + // Use a Synchronization to capture directory permissions before cleanup + Exchange exchange = createExchangeWithBody("Text content for permission test"); + exchange.getExchangeExtension().addOnCompletion( + new org.apache.camel.support.SynchronizationAdapter() { + @Override + public void onDone(Exchange exchange) { + // Check all docling dirs created during this exchange + try { + for (Path dir : listDoclingTempDirs()) { + if (!before.contains(dir) && Files.exists(dir)) { + capturedPermissions.add(Files.getPosixFilePermissions(dir)); + } + } + } catch (IOException e) { + // Ignore + } + } + + @Override + public int getOrder() { + // Run before the cleanup synchronization (lower order = earlier) + return HIGHEST - 1; + } + }); + + try { + template.send("direct:check-perms", exchange); + } catch (CamelExecutionException e) { + // Expected + } + + if (!capturedPermissions.isEmpty()) { + for (Set<PosixFilePermission> perms : capturedPermissions) { + // Should only have OWNER_READ, OWNER_WRITE, OWNER_EXECUTE + assertFalse(perms.contains(PosixFilePermission.GROUP_READ), "Group read should not be set"); + assertFalse(perms.contains(PosixFilePermission.GROUP_WRITE), "Group write should not be set"); + assertFalse(perms.contains(PosixFilePermission.OTHERS_READ), "Others read should not be set"); + assertFalse(perms.contains(PosixFilePermission.OTHERS_WRITE), "Others write should not be set"); + assertTrue(perms.contains(PosixFilePermission.OWNER_READ), "Owner read should be set"); + assertTrue(perms.contains(PosixFilePermission.OWNER_WRITE), "Owner write should be set"); + } + } + } + + private List<Path> listDoclingTempDirs() throws IOException { + List<Path> result = new ArrayList<>(); + Path tmpDir = Path.of(System.getProperty("java.io.tmpdir")); + try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpDir, "docling-*")) { + for (Path entry : stream) { + if (Files.isDirectory(entry)) { + result.add(entry); + } + } + } + return result; + } + + static boolean isPosixSupported() { + return FileSystems.getDefault().supportedFileAttributeViews().contains("posix"); + } + + @Override + public boolean isUseRouteBuilder() { + return false; + } +} diff --git a/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingTempFileCleanupTest.java b/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingTempFileCleanupTest.java index 69c0178e19ff..bf116ce7d74a 100644 --- a/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingTempFileCleanupTest.java +++ b/components/camel-ai/camel-docling/src/test/java/org/apache/camel/component/docling/DoclingTempFileCleanupTest.java @@ -35,15 +35,15 @@ import static org.junit.jupiter.api.Assertions.*; * processing completes. * * <p> - * Before the fix, temp files created for String content and byte[] bodies accumulated on disk indefinitely. After the - * fix, an {@code addOnCompletion} callback deletes them when the exchange finishes. + * Temp files are created inside per-exchange subdirectories under the system temp dir. The entire subdirectory is + * removed when the exchange finishes. */ class DoclingTempFileCleanupTest extends CamelTestSupport { @Test void tempFileFromStringContentIsCleanedUp() throws Exception { - // Snapshot temp files before - List<Path> before = listDoclingTempFiles(); + // Snapshot docling temp directories before + List<Path> before = listDoclingTempDirs(); // Send string content (not a path, not a URL) — this triggers temp file creation. // The docling CLI will fail (not installed), but the temp file cleanup @@ -54,18 +54,18 @@ class DoclingTempFileCleanupTest extends CamelTestSupport { // Expected — docling binary not available in test env } - // After exchange completes, temp files should have been cleaned up - List<Path> after = listDoclingTempFiles(); + // After exchange completes, temp directories should have been cleaned up + List<Path> after = listDoclingTempDirs(); List<Path> leaked = new ArrayList<>(after); leaked.removeAll(before); assertTrue(leaked.isEmpty(), - "Temp files leaked after exchange completion: " + leaked); + "Temp directories leaked after exchange completion: " + leaked); } @Test void tempFileFromByteArrayIsCleanedUp() throws Exception { - List<Path> before = listDoclingTempFiles(); + List<Path> before = listDoclingTempDirs(); try { template.requestBody("direct:convert", "Binary content for conversion".getBytes()); @@ -73,20 +73,25 @@ class DoclingTempFileCleanupTest extends CamelTestSupport { // Expected — docling binary not available in test env } - List<Path> after = listDoclingTempFiles(); + List<Path> after = listDoclingTempDirs(); List<Path> leaked = new ArrayList<>(after); leaked.removeAll(before); assertTrue(leaked.isEmpty(), - "Temp files leaked after exchange completion: " + leaked); + "Temp directories leaked after exchange completion: " + leaked); } - private List<Path> listDoclingTempFiles() throws IOException { + /** + * Lists docling temp directories (docling-UUID-*) in the system temp dir. + */ + private List<Path> listDoclingTempDirs() throws IOException { List<Path> result = new ArrayList<>(); Path tmpDir = Path.of(System.getProperty("java.io.tmpdir")); - try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpDir, "docling-*.tmp")) { + try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpDir, "docling-*")) { for (Path entry : stream) { - result.add(entry); + if (Files.isDirectory(entry)) { + result.add(entry); + } } } return result;
