Skip to content

Validate downloaded paths stay within the destination directory in SFTPHook.retrieve_directory#67985

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:fix-sftp-retrieve-directory-path-containment
Jun 4, 2026
Merged

Validate downloaded paths stay within the destination directory in SFTPHook.retrieve_directory#67985
potiuk merged 1 commit into
apache:mainfrom
potiuk:fix-sftp-retrieve-directory-path-containment

Conversation

@potiuk

@potiuk potiuk commented Jun 4, 2026

Copy link
Copy Markdown
Member

SFTPHook.retrieve_directory and retrieve_directory_concurrently build each
local destination path by joining the local directory with a path derived from
directory-entry names returned by the remote SFTP server. Because those names
can contain .. components, the recursive download could write outside the
configured local destination directory.

This adds a containment check (_validate_within_directory) that resolves each
computed local path and refuses to write when it falls outside the destination
directory, applied to both the serial and concurrent retrieval paths.

Tests

  • Unit test for the containment helper (in-bounds passes, escape rejected)
  • Unit test that retrieve_directory raises when get_tree_map yields a
    traversing entry, and nothing is written outside the destination
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Opus 4.8 (1M context)

Generated-by: Claude Opus 4.8 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions

…TPHook.retrieve_directory

SFTPHook.retrieve_directory and retrieve_directory_concurrently build each
local destination path by joining the local directory with a path derived
from directory-entry names returned by the remote SFTP server. Because those
names can contain ".." components, the recursive download could write outside
the configured local destination directory.

Add a containment check (_validate_within_directory) that resolves each
computed local path and refuses to write when it falls outside the
destination directory, applied to both the serial and concurrent retrieval
paths.

Generated-by: Claude Opus 4.8 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
@potiuk potiuk merged commit 16547df into apache:main Jun 4, 2026
96 checks passed
@potiuk potiuk deleted the fix-sftp-retrieve-directory-path-containment branch June 4, 2026 13:33
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