Skip to content

CAMEL-23765: remote-file consumers - contain localWorkDirectory downloads within the work directory#24180

Merged
oscerd merged 2 commits into
mainfrom
fix/CAMEL-23765
Jun 24, 2026
Merged

CAMEL-23765: remote-file consumers - contain localWorkDirectory downloads within the work directory#24180
oscerd merged 2 commits into
mainfrom
fix/CAMEL-23765

Conversation

@oscerd

@oscerd oscerd commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Fixes CAMEL-23765.

Problem

When localWorkDirectory is enabled, the remote-file consumers build the local work file path from the remote file name (target.getRelativeFilePath()) without ensuring the result stays within the configured work directory:

File local = new File(normalizePath(localWorkDirectory));
String relativeName = target.getRelativeFilePath();   // untrusted remote name
local = new File(local, relativeName);                // "../" escapes the work dir
local.mkdirs(); // ... writes outside localWorkDirectory

A remote file name containing ../ sequences could therefore resolve to a path outside the work directory (arbitrary local file write) — unlike the file producer, which already jails writes via FileUtil.compactPath + startsWith when jailStartingDirectory is enabled. Per the security model, path traversal in file/FTP consumers is in scope.

Change

  • New shared GenericFileHelper.jailToLocalWorkDirectory(target, workDir) (camel-file) — throws GenericFileOperationFailedException if compactPath(target) does not startsWith compactPath(workDir), mirroring the producer's jailedCheck.
  • Applied (to both the in-progress temp file and the final file), before mkdirs, in the localWorkDirectory download path of:
    • FtpOperations + SftpOperations (camel-ftp)
    • MinaSftpOperations (camel-mina-sftp)
    • FilesOperations (camel-azure-files)
    • SmbOperations (camel-smb)
  • Reuses the existing jailStartingDirectory option (default true, inherited by all file endpoints) — secure by default, consistent with the producer, opt-out via jailStartingDirectory=false. No new config surface.
  • The jailStartingDirectory option is now label = "common" (was "producer") with an expanded description, since it now governs consumer-side localWorkDirectory downloads in addition to producer writes. Catalog/component metadata and endpoint-dsl factories are regenerated accordingly for all file-based components (file, ftp, ftps, sftp, smb, azure-files, mina-sftp, scp).

Tests

  • New GenericFileHelperTest — verifies files within the work directory (incl. ../ that still resolves inside) are allowed, and escaping paths are rejected.
  • Existing localWorkDirectory ITs (camel-smb SmbConsumerLocalWorkDirectoryIT, camel-mina-sftp feature IT) cover legitimate-download regression.
  • Full reactor build (mvn clean install -DskipTests, 1874 modules) green. The only generated-file changes are the jailStartingDirectory producer → common relabel (catalog + component JSON + endpoint-dsl factories for the file components); no other drift.

Documentation

  • camel-4x-upgrade-guide-4_21.adoc — note added for the remote-file consumers.

Compatibility / backport

Default-secure with an opt-out (jailStartingDirectory=false), so suitable for backport to camel-4.18.x and camel-4.14.x (per the Jira fixVersions: 4.14.8, 4.18.3, 4.21.0). Matching 4_18/4_14 guide entries will be added on main with the backports.


Claude Code on behalf of Andrea Cosentino

…oads within the work directory

When localWorkDirectory was enabled, the remote-file consumers built the local
work file path from the remote file name (target.getRelativeFilePath()) without
ensuring the result stayed within the configured work directory. A remote file
name containing ../ sequences could therefore resolve to a path outside the work
directory (arbitrary local file write), unlike the file producer which already
jails writes via FileUtil.compactPath + startsWith when jailStartingDirectory is
enabled.

This adds a shared GenericFileHelper.jailToLocalWorkDirectory containment check,
mirroring the producer, and applies it (for both the in-progress temp file and the
final file) in the localWorkDirectory download path of FtpOperations, SftpOperations,
MinaSftpOperations (camel-mina-sftp), FilesOperations (camel-azure-files) and
SmbOperations (camel-smb). The check reuses the existing jailStartingDirectory option
(default true), so it is secure by default and can be disabled with
jailStartingDirectory=false. A remote file resolving outside the work directory is
rejected with a GenericFileOperationFailedException.

Adds GenericFileHelperTest and a 4.21 upgrade-guide note.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@oscerd oscerd requested review from davsclaus and gnodet June 22, 2026 12:11
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid, well-scoped security fix. The jailToLocalWorkDirectory check mirrors the existing GenericFileProducer.jailedCheck pattern, covers all five retrieveFileToFileInLocalWorkDirectory implementations, and is secure by default via the existing jailStartingDirectory option. No blocking issues found — two minor observations below.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • catalog/camel-catalog
  • components/camel-azure/camel-azure-files
  • components/camel-file
  • components/camel-ftp
  • components/camel-jsch
  • components/camel-mina-sftp
  • components/camel-smb
  • docs
  • dsl/camel-endpointdsl

ℹ️ Dependent modules were not tested because the total number of affected modules exceeded the threshold (50). Use the test-dependents label to force testing all dependents.

Build reactor — dependencies compiled but only changed modules were tested (9 modules)
  • Camel :: Azure :: Files
  • Camel :: Catalog :: Camel Catalog
  • Camel :: Docs
  • Camel :: Endpoint DSL
  • Camel :: FTP
  • Camel :: File
  • Camel :: Jsch
  • Camel :: MINA SFTP
  • Camel :: SMB

⚙️ View full build and test results

…B jail ordering)

Addresses the two non-blocking review observations from @davsclaus:

- GenericFileEndpoint: jailStartingDirectory is now label="common" (was "producer")
  with an expanded description covering consumer localWorkDirectory downloads, since
  the option now governs both producer writes and consumer downloads. Regenerated the
  catalog, component metadata and endpoint-dsl factories for all file-based components
  (file, ftp, ftps, sftp, smb, azure-files, mina-sftp, scp).
- SmbOperations: moved the localWorkDirectory jail check before directory creation to
  match FtpOperations/SftpOperations/MinaSftpOperations/FilesOperations, preserving
  SMB's existing mkdirs() on the base work directory.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@oscerd oscerd merged commit 1da66d1 into main Jun 24, 2026
6 checks passed
oscerd added a commit that referenced this pull request Jun 24, 2026
…localWorkDirectory downloads within the work directory (#24219)

CAMEL-23765: remote-file consumers - contain localWorkDirectory downloads within the work directory

Backport of #24180 to camel-4.18.x.

When localWorkDirectory was enabled, the remote-file consumers built the local
work file path from the remote file name without ensuring the result stayed
within the configured work directory, so a remote file name containing ../
sequences could resolve to a path outside it. A shared
GenericFileHelper.jailToLocalWorkDirectory containment check (compactPath +
startsWith, mirroring the file producer's jail) is now applied to both the
in-progress temp file and the final file, before mkdirs, in FtpOperations,
SftpOperations, MinaSftpOperations, FilesOperations and SmbOperations. It reuses
the existing jailStartingDirectory option (default true).

Source-only backport: the main PR's jailStartingDirectory producer->common label
change and its regenerated metadata are intentionally omitted to keep the
maintenance-branch change minimal; the upgrade-guide entry lives on main.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
oscerd added a commit that referenced this pull request Jun 24, 2026
…localWorkDirectory downloads within the work directory (#24220)

CAMEL-23765: remote-file consumers - contain localWorkDirectory downloads within the work directory

Backport of #24180 to camel-4.14.x.

When localWorkDirectory was enabled, the remote-file consumers built the local
work file path from the remote file name without ensuring the result stayed
within the configured work directory, so a remote file name containing ../
sequences could resolve to a path outside it. A shared
GenericFileHelper.jailToLocalWorkDirectory containment check (compactPath +
startsWith, mirroring the file producer's jail) is now applied to both the
in-progress temp file and the final file, before mkdirs, in FtpOperations,
SftpOperations, FilesOperations and SmbOperations. It reuses the existing
jailStartingDirectory option (default true).

Note: camel-mina-sftp does not exist on camel-4.14.x (added later), so its
MinaSftpOperations change from the main PR is not applicable here. Source-only
backport: the jailStartingDirectory producer->common label change and its
regenerated metadata are intentionally omitted; the upgrade-guide entry lives on
main.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants