oscerd opened a new pull request, #25730: URL: https://github.com/apache/camel/pull/25730
# CAMEL-24487: contain remote-file consumer operations within the configured directory ## What The remote-file consumers build the absolute remote path by concatenating the configured directory with the file name reported in the server directory listing, and then use that result as the operand for the subsequent retrieve, delete and rename operations. No lexical normalization or containment check was applied at any point between the listing and those operations, so a listing entry that is not a single path segment could resolve outside the configured directory. This is inconsistent with two paths that already contain the resolved path via `FileUtil.compactPath` plus a path-boundary check when `jailStartingDirectory` is enabled: - the file producer (`GenericFileProducer`) - the `localWorkDirectory` download path (CAMEL-23765, made boundary-aware in CAMEL-23868) This change extends the same treatment to the remote operand. ## How - `GenericFileHelper` gains a separator-aware `isWithinDirectory(target, dir, separator)` overload. The existing two-argument version hardcoded `File.separatorChar`, which is wrong for remote paths — those always use `/` regardless of the platform Camel runs on. The two-arg version now delegates. The overload also treats a target that still resolves upwards after compaction as not contained, so the session root is a boundary even when no directory is configured. - `GenericFileConsumer` gains a `isWithinStartingDirectory(String)` strategy, consulted from `isValidFile` when `jailStartingDirectory` is enabled. The base implementation returns `true`: names obtained from a local directory listing are always single path segments, so there is no boundary to enforce and local behaviour is unchanged. - `RemoteFileConsumer` overrides it for `camel-ftp` (ftp, ftps, sftp), `camel-mina-sftp` and `camel-azure-files`; `SmbConsumer` overrides it for `camel-smb`. Both compact the resolved path with `/` and check it still resolves inside the directory being polled. Two behavioural choices worth review attention: 1. **A file resolving outside is skipped with a warning, not thrown.** CAMEL-23765 threw a `GenericFileOperationFailedException`, but that governed a single download. Here a throw would abort the whole poll and take the legitimate entries in the same listing with it. 2. **The check runs after the existing match filters, not before.** SFTP listings contain `.` and `..` entries, and `poll/..` compacts to an empty path. Running the check first would log a warning on every poll; the hidden-file rule in `isMatched` already excludes those entries. ## Compatibility Gated on the existing `jailStartingDirectory` option (default `true`), so this is consistent with the producer and can be turned off with `jailStartingDirectory=false`. Ordinary listings are unaffected — a listed name is normally a single path segment, and a `../` that still resolves back inside the polled directory remains accepted. Two configurations can newly see files skipped: a server that reports names navigating above the polled directory, and a `fileName` expression (used with `useList=false`) that navigates above it. Both are covered by the upgrade-guide note. ## Testing - `GenericFileHelperTest` — two new methods covering the separator-aware overload and paths resolving outside the directory. - `RemoteFileConsumerStartingDirectoryJailTest` (new) — FTP and SFTP endpoints, accepted and rejected paths, plus the session-root case. No network required. - Regression: 397 `org.apache.camel.component.file.*` tests in `core/camel-core`, 78 in `camel-ftp`, 21 in `camel-file` — all green. - `camel-file`, `camel-ftp-common`, `camel-ftp`, `camel-smb`, `camel-mina-sftp` and `camel-azure-files` all build; full reactor `mvn clean install -DskipTests` is green. Note on assertion style: the project prefers AssertJ, but it is not a test dependency in `camel-file`, `camel-ftp`, `camel-ftp-common` or `camel-smb`. These tests use JUnit assertions to match the surrounding files rather than add a dependency for assertion style alone. ## Docs Upgrade-guide entry added to `camel-4x-upgrade-guide-4_23.adoc`. --- _Claude Code on behalf of oscerd_ -- 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]
