Skip to content

Fix encrypted private keys in async SFTP hook - #70858

Merged
potiuk merged 1 commit into
apache:mainfrom
fallintoplace:fix/sftp-private-key-passphrase
Aug 1, 2026
Merged

Fix encrypted private keys in async SFTP hook#70858
potiuk merged 1 commit into
apache:mainfrom
fallintoplace:fix/sftp-private-key-passphrase

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

The async SFTP hook only read the legacy passphrase connection extra because its condition always evaluated to that key. As a result, encrypted private keys configured with the documented private_key_passphrase extra could not be imported.

This accepts the documented extra while preserving the existing passphrase behavior and precedence, matching the async SSH hook. The regression test verifies that the configured passphrase is passed when importing a private key.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

The documented connection extra was ignored, causing authentication failures when encrypted private keys were used through asynchronous SFTP connections.

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch — ("passphrase" or "private_key_passphrase") in extra_options collapses to "passphrase" in extra_options before the membership test runs, so the documented private_key_passphrase extra was silently ignored and encrypted keys configured that way could not be imported.

Good call matching SSHHook._parse_extras, which already handles this exactly the same way (providers/ssh/.../ssh.py:613-615) — same precedence, same empty-string fallback. Reusing the proven shape rather than inventing a new one is the right move.

I checked that switching the fixture to private_key_passphrase doesn't drop coverage of the legacy key — it's still exercised in several other fixtures in that file.


Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

@potiuk
potiuk merged commit cdedaf3 into apache:main Aug 1, 2026
83 checks passed
@boring-cyborg

boring-cyborg Bot commented Aug 1, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

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