[jira] [Commented] (CAMEL-25886) camel-ftp/mina-sftp - resolve listed symbolic links for jailStartingDirectory

["shashank (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CAMEL-25886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18129475#comment-18129475 ] 

shashank commented on CAMEL-25886:
----------------------------------

Test for the expected behaviour: with jailStartingDirectory (default) the sftp consumer consumes the plain file and the link inside the starting directory, and skips the link to a file outside it. Goes in camel-ftp’s sftp integration tests.

> camel-ftp/mina-sftp - resolve listed symbolic links for jailStartingDirectory
> -----------------------------------------------------------------------------
>
>                 Key: CAMEL-25886
>                 URL: https://issues.apache.org/jira/browse/CAMEL-25886
>             Project: Camel
>          Issue Type: Improvement
>          Components: camel-ftp, camel-mina-sftp
>            Reporter: Andrea Cosentino
>            Assignee: Andrea Cosentino
>            Priority: Major
>             Fix For: 4.18.6, 4.22.3, 4.24.0
>
>         Attachments: SftpConsumerSymlinkJailStartingDirectoryIT.java
>
>
> The SFTP consumers ({{sftp}} in camel-ftp and {{mina-sftp}}) apply {{jailStartingDirectory}} to the path built from each listed name only: {{RemoteFileConsumer.isWithinStartingDirectory()}} (camel-ftp-common) compacts that path and compares it with the starting directory as text. The 4.23 upgrade guide note of CAMEL-25412 states that the remote file consumers keep this lexical check. A listed entry that the server reports as a symbolic link is therefore handled like any other entry:
> * {{SftpRemoteFile}} has no accessor for the link flag of the listing attributes, so {{SftpRemoteFileJCraft}} ({{ChannelSftp.LsEntry}}) and {{MinaSftpRemoteFile}} ({{SftpClient.DirEntry}}) do not expose it.
> * {{AbstractSftpConsumer.doPollDirectory()}} (camel-ftp-common, shared by both consumers) treats every entry that is not a directory as a file.
> * The retrieval ({{ChannelSftp.get()}} in {{SftpOperations}}, {{SftpClient.read()}} in {{MinaSftpOperations}}) is resolved by the server, which follows the link to its target wherever that is.
> camel-file resolves symbolic links for {{jailStartingDirectory}} since CAMEL-25412, and the ftp consumer ignores entries that the server lists as links ({{FtpConsumer}}, "Ignoring unsupported remote file type"). This change gives the SFTP consumers the same semantics as camel-file.
> h3. Behaviour after the change
> With {{jailStartingDirectory}} enabled (the default):
> * A listed entry that the server reports as a symbolic link is resolved on the server before it is accepted. If its target is outside the starting directory, the entry is skipped and logged by {{GenericFileConsumer.isValidFile()}} as for the existing lexical check (WARN the first time, DEBUG on later polls).
> * A link whose target cannot be resolved (a dangling link) is skipped.
> * Links that resolve inside the starting directory, and a starting directory that is itself a link, work as before, so {{MinaSftpSymlinkIT}} and the "Symbolic Links" section of the mina-sftp documentation stay valid.
> With {{jailStartingDirectory=false}} the consumers keep following every link, as today. No new option is added, so the endpoint metadata does not change.
> h3. Proposed change
> * {{SftpRemoteFile}} (camel-ftp-common): add {{default boolean isSymbolicLink()}} returning {{false}}, so {{SftpRemoteFileSingle}} and other implementations stay compatible. {{SftpRemoteFileJCraft}} returns {{entry.getAttrs().isLink()}} and {{MinaSftpRemoteFile}} returns {{entry.getAttributes().isSymbolicLink()}}.
> * A small interface in camel-ftp-common that resolves a link on the server, implemented by {{SftpOperations}} ({{ChannelSftp.realpath()}} and {{readlink()}}) and {{MinaSftpOperations}} ({{SftpClient.canonicalPath()}} and {{readLink()}}), instead of a new method on the public {{RemoteFileOperations}} interface, which {{FtpOperations}} implements too. It uses realpath. Where the server's realpath does not resolve links (it returns the link's own path, as Apache MINA SSHD does), it resolves the target itself with lstat and readlink, component by component, relative to the link's directory and up to a fixed number of links. The starting directory is resolved the same way, once per poll.
> * {{AbstractSftpConsumer}}: check an entry flagged as a link right after the existing lexical check, either through a protected overload of the {{GenericFileConsumer.isWithinStartingDirectory()}} hook that also receives the listed entry (defaulting to the String variant), or in {{AbstractSftpConsumer}} itself.
> * Only entries that the server reports as links are resolved, and no entry is required to be a regular file. CAMEL-1247 removed an earlier check that skipped the entries listed as links, because on Windows it skipped regular files too; the current {{SftpATTRS.isLink()}} compares the whole file type field.
> * camel-4.18.x has no camel-ftp-common: there {{SftpConsumer}} and {{MinaSftpConsumer}} each have their own polling loop, so the change goes into both, and the CAMEL-1247 comment in {{SftpConsumer}} is replaced.
> h3. Documentation
> * {{sftp-component.adoc}}: a "Symbolic links and the starting directory" section, as in {{file-component.adoc}}.
> * {{mina-sftp-component.adoc}}: amend "Symbolic Links": links resolving outside the starting directory are skipped unless {{jailStartingDirectory=false}}.
> * Regenerate the catalog copies of both pages.
> * Upgrade guide notes on main, since links to files outside the starting directory are no longer consumed: {{camel-4x-upgrade-guide-4_24.adoc}}, plus the "Upgrading from 4.22.2 to 4.22.3" section of the 4_22 guide and the "Upgrading from 4.18.5 to 4.18.6" section of the 4_18 guide. While there, add the CAMEL-25412 notes that main's 4_22 and 4_18 guides do not have yet.
> h3. Tests
> * {{RemoteFileConsumerStartingDirectoryJailTest}} (camel-ftp): with stubbed listing entries, a link resolving outside the starting directory is skipped and a link resolving inside is accepted.
> * New {{SftpConsumerJailStartingDirectorySymlinkIT}} (camel-ftp) and {{MinaSftpConsumerJailStartingDirectorySymlinkIT}} (camel-mina-sftp): a link resolving inside is consumed; a link resolving outside is skipped, and consumed when {{jailStartingDirectory=false}}; a dangling link is skipped. The embedded SFTP server (Apache MINA SSHD with its default settings) lists a link as a link, as OpenSSH does, so the tests run against it unchanged.
> * {{MinaSftpSymlinkIT}} stays green.
> h3. Notes
> * Whether a listing reports an entry as a link depends on the server. OpenSSH and Apache MINA SSHD (with its default settings) list a link with its own attributes, so the listing is enough there. A server that reports the target's attributes in its listings hides the link; recognising such entries too would need an LSTAT per listed entry, one more round trip per file and poll. The proposal relies on the listing attributes, and the documentation says so.
> * The check and the retrieval that follows are not atomic (an SFTP open follows links on the server), so the permissions and link settings of the SFTP server still matter.
> * Backport to camel-4.22.x and camel-4.18.x, as for CAMEL-25412.
> ----
> _Claude Code on behalf of [~acosentino]_



--
This message was sent by Atlassian Jira
(v8.20.10#820010)