fix(ssh-tunnel): support ed25519 and ECDSA keys, not just RSA (#24180) by rusackas · Pull Request #40139 · apache/superset

rusackas · 2026-05-15T01:33:27Z

SUMMARY

SSHManager.create_tunnel (in superset/extensions/ssh.py) unconditionally called RSAKey.from_private_key regardless of key type. Users who pasted an ed25519 (or ECDSA) private key got an opaque paramiko.ssh_exception.SSHException: unpack requires a buffer of 4 bytes and the tunnel never opened. ed25519 has been the OpenSSH default since 2018, so this hits anyone with a recent ssh-keygen config.

This PR:

Adds _load_private_key() — a polymorphic loader that tries Ed25519Key → ECDSAKey → RSAKey in order and returns the first that parses cleanly. Modern types are tried first because RSA's parser is the most permissive and would happily mis-accept a non-RSA buffer.
Replaces the hardcoded RSAKey.from_private_key call with _load_private_key(...).
Surfaces a single, actionable error if no loader succeeds: an SSHException whose message lists every type attempted — much better than the original four-bytes-buffer error.
Locks in regression tests (added in this PR's earlier commit):
- test_create_tunnel_accepts_ed25519_private_key — the new path
- test_create_tunnel_accepts_rsa_private_key_unchanged — backward-compat for RSA

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — backend key-loader change. Verified locally:

ed25519 OK: Ed25519Key
rsa OK: RSAKey
invalid raised: SSHException

TESTING INSTRUCTIONS

pytest tests/unit_tests/extensions/ssh_test.py::test_create_tunnel_accepts_ed25519_private_key -v
pytest tests/unit_tests/extensions/ssh_test.py::test_create_tunnel_accepts_rsa_private_key_unchanged -v

ADDITIONAL INFORMATION

Has associated issue: closes SSH Only Accepts RSA Keys #24180
Required feature flags: SSH_TUNNELING
Changes UI
Includes DB Migration
Introduces new feature or API
Removes existing feature or API

🤖 Generated with Claude Code

bito-code-review · 2026-05-15T01:33:36Z

Code Review Agent Run #c2a919

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: ae4eac2..ae4eac2
- tests/unit_tests/extensions/ssh_test.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

codecov · 2026-05-15T01:37:39Z

Codecov Report

❌ Patch coverage is 21.42857% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.14%. Comparing base (f187a8e) to head (5a6cdd8).
⚠️ Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/extensions/ssh.py	21.42%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #40139      +/-   ##
==========================================
- Coverage   64.14%   64.14%   -0.01%     
==========================================
  Files        2592     2592              
  Lines      138841   138855      +14     
  Branches    32201    32202       +1     
==========================================
+ Hits        89064    89070       +6     
- Misses      48245    48253       +8     
  Partials     1532     1532

Flag	Coverage Δ
hive	`39.32% <21.42%> (+<0.01%)`	⬆️
mysql	`58.85% <21.42%> (-0.01%)`	⬇️
postgres	`58.93% <21.42%> (-0.01%)`	⬇️
presto	`41.00% <21.42%> (+<0.01%)`	⬆️
python	`60.49% <21.42%> (-0.01%)`	⬇️
sqlite	`58.57% <21.42%> (-0.01%)`	⬇️
unit	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bito-code-review · 2026-05-15T01:40:09Z

The flagged issue is correct: the new ed25519 regression test will fail deterministically because SSHManager.create_tunnel hard-codes RSA key parsing, causing an 'unpack requires 4 bytes' error when trying to parse ed25519 keys. To resolve, update the key loading logic to use paramiko's polymorphic loader instead of RSAKey.

superset/extensions/ssh.py

import io

# In SSHManager.create_tunnel, replace the RSA-specific line:
# Original: RSAKey.from_private_key(private_key_file, ssh_tunnel.private_key_password)
# Fixed:
paramiko.PKey.from_private_key(io.StringIO(private_key), password=ssh_tunnel.private_key_password)

bito-code-review · 2026-05-15T14:01:42Z

Code Review Agent Run #186373

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: ae4eac2..4d29641
- superset/extensions/ssh.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

aminghadersohi

Review: SSH key type support

The fix is correct, secure, and backwards-compatible — _load_private_key cleanly solves the ed25519/ECDSA bug without touching any auth boundaries and without leaking key material in error paths. RSA regression test is a nice touch.

Three things worth addressing before LGTM (details in inline comments):

raise ... from last_exc — one-word fix, restores traceback chain
PasswordRequiredException should be re-raised rather than absorbed by the except SSHException clause — it's a distinct, actionable signal ("key requires passphrase") that gets masked as "Unable to parse"
ECDSA test missing — third declared key type, distinct paramiko code path

Nits inline as well.

aminghadersohi

left comments

aminghadersohi

Review feedback addressed in the follow-up commit (8dbc525):

PasswordRequiredException is now re-raised immediately instead of being absorbed by the except SSHException loop — passphrase errors surface as actionable PasswordRequiredException rather than a generic "Unable to parse" message
Exception chaining added: raise SSHException(...) from last_exc preserves the full traceback for Sentry/debugging
last_exc type narrowed to SSHException | None
ECDSA test added (test_create_tunnel_accepts_ecdsa_private_key, NIST P-256) — all three types in _SSH_KEY_TYPES are now exercised
Inline RSA import moved to module level (consistent with ed25519 import style)

All pre-commit hooks pass. LGTM.

Closes #24180 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

`SSHManager.create_tunnel` unconditionally called `RSAKey.from_private_key` regardless of key type, so users who pasted a modern key (ed25519, ECDSA) got an opaque `paramiko.ssh_exception.SSHException: unpack requires a buffer of 4 bytes` and the tunnel never opened. paramiko does not expose a polymorphic `PKey.from_private_key`; each key class only accepts its own format. Add `_load_private_key` that tries Ed25519Key → ECDSAKey → RSAKey in order and returns the first that parses cleanly. Modern types are tried first because RSA's parser is the most permissive and would happily accept a non-RSA buffer with the same vague error otherwise. If none of the loaders succeed, surface a single `SSHException` whose message lists every type that was attempted — much more actionable than the original 4-bytes-buffer error. Companion regression tests (added in this PR's earlier commit) cover both the new ed25519 path and the backward-compatible RSA path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Re-raise PasswordRequiredException immediately instead of absorbing it into the try-next-type loop, so callers see an actionable "key requires passphrase" error rather than "Unable to parse" - Add `from last_exc` to the final raise to preserve the exception chain for traceback tools (Sentry et al.) - Narrow last_exc type annotation to SSHException | None - Add test_create_tunnel_accepts_ecdsa_private_key (NIST P-256) to cover the third entry in _SSH_KEY_TYPES - Move RSA and ECDSA crypto imports to module level (rm inline import)

netlify · 2026-05-21T20:50:12Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`8dbc525`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a0f6f736b1e44000829c161
😎 Deploy Preview	https://deploy-preview-40139--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

rusackas · 2026-05-22T17:20:18Z

Just need a code owner stamp here, if @villebro @geido @eschutho @kgabryje see fit :D

bito-code-review · 2026-05-23T04:27:23Z

Code Review Agent Run #831652

Actionable Suggestions - 0

Review Details

Files reviewed - 2 · Commit Range: da76c68..5a6cdd8
- superset/extensions/ssh.py
- tests/unit_tests/extensions/ssh_test.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

pull-request-size Bot added the size/L label May 15, 2026

codeant-ai-for-open-source Bot reviewed May 15, 2026

View reviewed changes

Comment thread tests/unit_tests/extensions/ssh_test.py

rusackas requested review from eschutho, geido, kgabryje, michael-s-molina and villebro as code owners May 15, 2026 07:15

rusackas changed the title ~~test(ssh-tunnel): assert ed25519 SSH keys load for tunnel setup (#24180)~~ fix(ssh-tunnel): support ed25519 and ECDSA keys, not just RSA (#24180) May 15, 2026

rusackas requested a review from sadpandajoe May 15, 2026 07:15

github-actions Bot added the preset-io label May 15, 2026

rusackas requested review from aminghadersohi and dpgaspar May 18, 2026 14:09

aminghadersohi requested changes May 20, 2026

View reviewed changes

Comment thread superset/extensions/ssh.py Outdated

Comment thread superset/extensions/ssh.py

Comment thread superset/extensions/ssh.py

Comment thread superset/extensions/ssh.py

Comment thread tests/unit_tests/extensions/ssh_test.py

Comment thread tests/unit_tests/extensions/ssh_test.py

aminghadersohi self-requested a review May 20, 2026 22:19

aminghadersohi reviewed May 20, 2026

View reviewed changes

aminghadersohi approved these changes May 21, 2026

View reviewed changes

claude and others added 3 commits May 21, 2026 20:48

test(ssh-tunnel): assert ed25519 SSH keys load for tunnel setup

da76c68

Closes #24180 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

aminghadersohi force-pushed the tdd/issue-24180-ssh-ed25519 branch from 8dbc525 to 5a6cdd8 Compare May 21, 2026 20:48

Conversation

rusackas commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #c2a919

Uh oh!

codecov Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

bito-code-review Bot commented May 15, 2026

Uh oh!

bito-code-review Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #186373

Uh oh!

aminghadersohi left a comment

Choose a reason for hiding this comment

Review: SSH key type support

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aminghadersohi left a comment

Choose a reason for hiding this comment

Uh oh!

aminghadersohi left a comment

Choose a reason for hiding this comment

Uh oh!

netlify Bot commented May 21, 2026

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

rusackas commented May 22, 2026

Uh oh!

bito-code-review Bot commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #831652

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rusackas commented May 15, 2026 •

edited

Loading

bito-code-review Bot commented May 15, 2026 •

edited

Loading

codecov Bot commented May 15, 2026 •

edited

Loading

bito-code-review Bot commented May 15, 2026 •

edited

Loading

bito-code-review Bot commented May 23, 2026 •

edited

Loading