[
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)
