oscerd commented on code in PR #28214:
URL: https://github.com/apache/camel/pull/28214#discussion_r4375781482
##########
components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/SftpOperations.java:
##########
@@ -939,6 +939,42 @@ public SftpRemoteFile[] listFiles(String path) throws
GenericFileOperationFailed
}
}
+ @Override
+ public String getHomeDirectory() throws
GenericFileOperationFailedException {
+ lock.lock();
+ try {
+ return channel.getHome();
+ } catch (SftpException e) {
+ throw new GenericFileOperationFailedException("Cannot get the home
directory", e);
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ @Override
+ public boolean isSymbolicLink(String path) throws
GenericFileOperationFailedException {
+ lock.lock();
+ try {
+ return channel.lstat(path).isLink();
Review Comment:
Fixed in 026d4214. Both JSch link operations now quote the literal path
before the call: `SftpOperations.isSymbolicLink()`/`readSymbolicLink()` escape
`\`, `*` and `?` (mirroring JSch's own `Util.quote`), so
`ChannelSftp.lstat()`/`readlink()` act on the literal path instead of a glob.
Only the jail resolver calls these two methods.
Added a real-server regression IT to both
`SftpConsumerJailStartingDirectorySymlinkIT` and
`MinaSftpConsumerJailStartingDirectorySymlinkIT`
(`skipsLinkWhoseTargetStepsThroughBackslashNamedLinkOutsideStartingDirectory`)
— your exact scenario: a `pivotx` directory inside, a distinct `pivot\x` link
pointing outside, and `latest.txt` → `pivot\x/report.txt`. Without the quoting
the sftp test consumes the outside file; the mina-sftp probe passes either way,
as that client already uses literal paths.
The quoting covers `*` and `?` through the same code path. If you'd like
dedicated `*`/`?` and inside-resolving ITs as well, I'm happy to add them in a
follow-up — the glob scenarios need careful embedded-server setup to stay
non-vacuous, so this first regression test reproduces the backslash case from
the report.
_Claude Code on behalf of oscerd_
##########
components/camel-ftp-common/src/main/java/org/apache/camel/component/file/remote/AbstractSftpConsumer.java:
##########
@@ -247,6 +260,104 @@ protected boolean
isMatched(Supplier<GenericFile<SftpRemoteFile>> file, String d
return false;
}
+ @Override
+ protected boolean
isWithinStartingDirectory(Supplier<GenericFile<SftpRemoteFile>> file, String
absoluteFilePath) {
+ if (!super.isWithinStartingDirectory(file, absoluteFilePath)) {
+ return false;
+ }
+ GenericFile<SftpRemoteFile> remoteFile = file.get();
+ if (remoteFile.getFile() == null ||
!remoteFile.getFile().isSymbolicLink()) {
+ return true;
+ }
+ // the server follows a symbolic link when the file is retrieved, so
the target of a listed link must be within
+ // the starting directory too
+ if (!(operations instanceof SftpSymbolicLinkOperations links)) {
+ LOG.debug("Cannot resolve symbolic link: {} as the operations
cannot read symbolic links", absoluteFilePath);
+ return false;
+ }
+ try {
+ String startingDirectory = resolvedStartingDirectory;
+ if (startingDirectory == null) {
+ startingDirectory = resolveSymbolicLinks(links, null,
endpointPath);
+ resolvedStartingDirectory = startingDirectory;
+ }
+ // the path of the file below the starting directory is resolved
from the resolved starting directory
+ String relativeFilePath = remoteFile.getRelativeFilePath();
+ String target = relativeFilePath != null
+ ? resolveSymbolicLinks(links, startingDirectory,
relativeFilePath)
+ : resolveSymbolicLinks(links, null,
remoteFile.getAbsoluteFilePath());
+ return GenericFileHelper.isWithinDirectory(target,
startingDirectory, '/');
+ } catch (GenericFileOperationFailedException e) {
Review Comment:
Acknowledged — a worthwhile distinction. Today a transport error during
resolution is treated as "skip", which is safe but indistinguishable from
"outside". I've kept this round focused on the blocking JSch literal-path fix
and will address the "cannot resolve vs outside" logging (and whether a
transport error should fail the poll) as a follow-up, so it gets its own review
rather than riding on the security fix.
_Claude Code on behalf of oscerd_
##########
components/camel-mina-sftp/src/main/docs/mina-sftp-component.adoc:
##########
@@ -571,6 +571,15 @@ IMPORTANT: The `chmod` and `chmodDirectory` options only
work on POSIX-compatibl
The consumer follows symbolic links to their target files. The producer writes
to symlink targets.
Review Comment:
Good point. I'll fold this wording tweak into the doc follow-up together
with the `jailStartingDirectory` option-description update (point 5 of your
2026-10-20 review), since both regenerate the catalog copies — bundling them
keeps it one doc/regen changeset instead of touching the generated pages twice.
_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]
