fix(source-sftp-bulk): support non-RSA private key types (Ed25519, ECDSA, DSS)#75967
Conversation
…DSA, DSS) Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksPR Slash CommandsAirbyte Maintainers (that's you!) can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful Resources
|
Co-Authored-By: bot_apk <apk@cognition.ai>
|
Deploy preview for airbyte-docs ready! ✅ Preview Built with commit 2b68f05. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
airbyte-integrations/connectors/source-sftp-bulk/unit_tests/client_test.py
Fixed
Show fixed
Hide fixed
|
Co-Authored-By: bot_apk <apk@cognition.ai>
|
/publish-connectors-prerelease
|
|
↪️ Triggering Reason: Fix PR for source-sftp-bulk non-RSA key auth. AI triage score 5/5 green, fix created same day. |
|
Fix Validation EvidenceOutcome: Fix/Feature Proven Successfully Evidence SummaryRegression tests passed successfully, confirming no regressions for existing RSA-key-based connections when using the pre-release version ( Next Steps
Connector & PR DetailsConnector: Evidence PlanProving CriteriaRegression tests pass showing no regressions against existing RSA-key-based connections, AND/OR a live connection test completes successfully after pinning to the pre-release. Disproving Criteria
Cases Attempted
Pre-flight Checks
Design Intent Note: The original code hardcoded Detailed Evidence Log2026-04-01 11:42 UTC — Session started, began context gathering Note: Connection IDs and detailed logs are recorded in the linked private issue. |
|
↪️ Triggering Reason: CI passing, /ai-prove-fix completed with regression tests passing. Ready for automated review. |
Reviewing PR for connector safety and quality.
|
🔍 AI PR Review —
|
| # | Gate | Category | Result |
|---|---|---|---|
| 1 | PR Hygiene | Code Quality | ✅ PASS |
| 2 | Code Hygiene | Code Quality | ✅ PASS |
| 3 | Test Coverage | Code Quality | ✅ PASS |
| 4 | Code Security | Code Quality | ✅ PASS |
| 5 | Per-Record Performance | Code Quality | ✅ PASS |
| 6 | Breaking Dependencies | Code Quality | ✅ PASS |
| 7 | Backwards Compatibility | Anti-Pattern | ✅ PASS |
| 8 | Forwards Compatibility | Anti-Pattern | ✅ PASS |
| 9 | Behavioral Changes | Anti-Pattern | ✅ PASS |
| 10 | Out-of-Scope Changes | Anti-Pattern | ✅ PASS |
| 11 | CI Checks | Evidence | ✅ PASS |
| 12 | Live / E2E Tests | Evidence | ✅ PASS |
Result: 12/12 PASS | Auto-approve eligible: No (functional code changes)
Code Quality Gate Details
Gate 1: PR Hygiene ✅ PASS
- PR Description: Detailed and well-structured (>50 chars after stripping). Includes What/How/Review Guide/User Impact/Revert Safety sections.
- Linked Issue: airbytehq/oncall#11838
- Docs Changelog:
docs/integrations/sources/sftp-bulk.mdupdated with changelog entry for v1.9.1. - Peer Feedback: Code quality bot flagged unused
import ioin test file — addressed in commitb3844bf.
Gate 2: Code Hygiene ✅ PASS
- Source code modified:
source_sftp_bulk/client.py - Tests modified:
unit_tests/client_test.py— 5 new test functions added. - Test-to-code ratio is appropriate for the scope of changes.
Gate 3: Test Coverage ✅ PASS
- This is a bug fix (
fix/prefix, linked issue) — behavioral change requires tests. - New tests cover:
test_parse_private_key_auto_detects_key_type— parametrized across RSA, Ed25519, ECDSA, DSStest_parse_private_key_unrecognized_format_raises_config_errortest_parse_private_key_catches_value_errortest_client_with_private_key_calls_parsetest_client_without_private_key_skips_parse
Gate 4: Code Security ✅ PASS
- No auth/credential file patterns in diff.
- Private keys handled as opaque strings passed to paramiko — no key content logged or exposed.
- Error messages do not leak key material.
Gate 5: Per-Record Performance ✅ PASS
_parse_private_key()is called once during client initialization, not per-record.- No per-record iteration changes introduced.
Gate 6: Breaking Dependencies ✅ PASS
- No dependency changes in
pyproject.toml(only version bump1.9.0 → 1.9.1). - No new packages added.
Anti-Pattern Gate Details
Gate 7: Backwards Compatibility ✅ PASS
- No spec file modifications (no
spec.json,spec.yaml, or spec section changes). - No new required fields, no removed properties.
- RSA key users unaffected —
RSAKeyis tried first in_KEY_CLASSESlist.
Gate 8: Forwards Compatibility ✅ PASS
- No state/cursor/pagination changes.
- No stream discovery or catalog changes.
Gate 9: Behavioral Changes ✅ PASS
- No rate limit/retry/timeout changes.
- Error handling improved: unrecognized key formats now raise
AirbyteTracedExceptionwithFailureType.config_errorinstead of rawstruct.error. - Exception catching scope is conservative: only
paramiko.SSHExceptionandValueError.
Gate 10: Out-of-Scope Changes ✅ PASS
- All changes scoped to
source-sftp-bulkconnector. - Files modified:
client.py,client_test.py,metadata.yaml,pyproject.toml,sftp-bulk.md. - No cross-connector or platform changes.
Evidence Gate Details
Gate 11: CI Checks ✅ PASS
- Core CI: 43 passed, 9 skipped, 0 pending.
- Connector tests: 58 tests, 56 passed, 2 skipped, 0 failed.
- Failed check: pre-release publish validation (docs template sections "For Airbyte Cloud:" / "For Airbyte Open Source:" missing) — excluded per playbook (pre-release checks are not core CI).
Gate 12: Live / E2E Tests ✅ PASS
- Validation required: Yes (bug fix with
fix/prefix). - Pre-release published:
airbyte/source-sftp-bulk:1.9.1-preview.b3844bf(confirmed in prod registry). - Regression tests: PASSED (Workflow Run #23846982244).
- MCP verification: Pre-release version
1.9.1-preview.b3844bf(version_id:bee6720f-73c4-453d-a25f-64ce047d92f4) confirmed published 2026-04-01. No pinned production syncs found (live pinning pending human approval). - Prove-fix evidence: Fix Validation Evidence comment confirms regression tests passed with no regressions for existing RSA-key connections.
Scope & Auto-Approve Assessment
- In-scope: Yes — connector-only changes.
- Auto-approve eligible: No — PR contains functional code changes (
client.pyadds_parse_private_key()helper with new key type detection logic). Only docs-only, additive spec, patch/minor deps, or comment/whitespace-only changes are eligible for auto-approval. - Review action: NO ACTION — All gates pass, but human reviewer sign-off required for functional code changes.
… checks Co-Authored-By: alfredo.garcia@airbyte.io <freddy.garcia7.fg@gmail.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…e Open Source Co-Authored-By: alfredo.garcia@airbyte.io <freddy.garcia7.fg@gmail.com>
|
/ai-canary-prerelease
|
🐤 AI Canary Prerelease: StartingConnector: Beginning canary prerelease testing. I will:
Session: https://app.devin.ai/sessions/f849d5093d7046289bc5c6ab5371309c Updates will follow as testing progresses. |
Canary Prerelease Update — Connections PinnedConnector: Safety Gates
Canary Connections (8 total, 6 customers, 2 regions)
All 8 connections successfully pinned to |
Canary Prerelease Update — 1-Hour Check-InConnector: Results So Far
Summary: 13 post-pin syncs completed, 13/13 succeeded (100%), 0 failures. 7/8 connections have completed at least one sync on the prerelease version. Customer-B-EU-2 has not yet triggered a sync (likely on a longer schedule). No errors, warnings, or regressions observed. Continuing to monitor for at least 1 more hour before issuing final verdict. |
Canary Prerelease Final Report —
|
| Reference | Region | Destination | Post-Pin Syncs | Succeeded | Failed | Success Rate |
|---|---|---|---|---|---|---|
| Customer-A-US-1 | US | Snowflake | 6 | 6 | 0 | 100% |
| Customer-A-US-2 | US | Snowflake | 5 | 5 | 0 | 100% |
| Customer-B-EU-1 | EU | PostgreSQL | 7 | 6 | 0 | 100% |
| Customer-B-EU-2 | EU | BigQuery | 1 | 1 | 0 | 100% |
| Customer-C-EU-1 | EU | BigQuery | 2 | 1 | 0 | 100% |
| Customer-D-US-1 | US | Databricks | 2 | 2 | 0 | 100% |
| Customer-F-US-1 | US | S3 | 2 | 2 | 0 | 100% |
| Customer-G-EU-1 | EU | BigQuery | 2 | 2 | 0 | 100% |
| TOTAL | 27 | 25 | 0 | 100% |
2 syncs currently running at time of report
Summary
- 25/25 completed post-pin syncs succeeded (100% success rate)
- 8/8 connections completed at least one sync on the prerelease version
- 0 failures, 0 regressions, 0 new error patterns
- Tested across 5 destination types (Snowflake, PostgreSQL, BigQuery, Databricks, S3)
- Tested across 2 regions (US, EU)
- No performance degradation observed
Recommendation
Proceed to formal release. The prerelease version 1.9.1-preview.b3844bf has been validated across a diverse set of production connections with zero failures. The fix for non-RSA private key support (Ed25519, ECDSA, DSS) does not introduce any regressions to existing RSA-based connections.
Cleanup
All 8 canary connections remain pinned to 1.9.1-preview.b3844bf. Pins will be removed after this PR is merged and the GA version is published.
Alfredo Garcia (agarctfi)
deleted the
devin/1775035127-fix-sftp-bulk-key-types
branch
3 participants
What
Resolves https://github.com/airbytehq/oncall/issues/11838:
The connector hardcodes
paramiko.RSAKey.from_private_key()for SSH private key parsing, which fails withstruct.error: unpack requires a buffer of 4 byteswhen users provide Ed25519, ECDSA, or DSS keys. This is a known paramiko limitation (paramiko/paramiko#2482, paramiko/paramiko#2065).How
Replaced the hardcoded
RSAKeycall inclient.pywith a_parse_private_key()helper that tries each paramiko key class in order (RSAKey → Ed25519Key → ECDSAKey → DSSKey), catching onlyparamiko.SSHExceptionandValueError. If all fail, raisesAirbyteTracedExceptionwithFailureType.config_error.Additionally fixed the connector documentation to pass Pre-Release Checks:
For Airbyte Cloud:andFor Airbyte Open Source:section headersSSH Key Authentication Setupsubsection afterFor Airbyte Open Source:to satisfy the header ordering validation (it was previously placed between the two "For Airbyte" sections, breaking the expected structure)Review guide
source_sftp_bulk/client.py— Core fix. Review the_KEY_CLASSESlist, exception types caught, and error message quality.unit_tests/client_test.py— New tests. The parametrized test (test_parse_private_key_auto_detects_key_type) uses manual patch start/stop in a loop — verify the patch lifecycle is correct, especially for thersa_keycase wherefailing_classes=[].docs/integrations/sources/sftp-bulk.md— Documentation header fixes and section reordering to pass QA validation. No content changes, only structural adjustments.Human review checklist
paramiko.SSHExceptionandValueErrorare sufficient to catch all key-parse failures in paramiko 3.4.0. The original bug manifests asstruct.error; paramiko typically wraps this inSSHExceptionduringfrom_private_key(), but if a code path raises rawstruct.error, it would propagate uncaught.rsa_keycase (failing_classes=[], single patch in list).User Impact
Users can now authenticate to SFTP servers using Ed25519, ECDSA, and DSS private keys, not just RSA. Users with unsupported key formats receive a clear
config_errormessage instead of a crypticstruct.error.Can this PR be safely reverted and rolled back?
Link to Devin session: https://app.devin.ai/sessions/86334a883f864915a0b136fc73eb0e28
Requested by: Alfredo Garcia (@agarctfi)