Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: openssh key parsing issue#41369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
NilanshBansal wants to merge1 commit intorelease
base:release
Choose a base branch
Loading
fromfix/openssh-key-parsing

Conversation

@NilanshBansal
Copy link
Contributor

@NilanshBansalNilanshBansal commentedNov 7, 2025•
edited by github-actionsbot
Loading

Description

Tip

Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).

Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.

Fixes #Issue Number
or
FixesIssue URL

Warning

If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.

Automation

/ok-to-test tags="@tag.Datasource"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run:https://github.com/appsmithorg/appsmith/actions/runs/19168276106
Commit:d156b8d
Cypress dashboard.
Tags:@tag.Datasource
Spec:

Fri, 07 Nov 2025 13:06:53 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features
    • SSH connections now support RSA PKCS#1 private key format in addition to existing PKCS#8 and OpenSSH formats. Users can utilize a wider variety of private keys for SSH authentication without requiring manual conversion, as the system automatically handles all necessary format conversions transparently.

@NilanshBansal
Copy link
ContributorAuthor

/build-deploy-preview skip-tests=true

github-actions[bot] reacted with rocket emojigithub-actions[bot] reacted with eyes emoji

@github-actionsgithub-actionsbot added the BugSomething isn't working labelNov 7, 2025
@NilanshBansalNilanshBansal added ok-to-testRequired label for CI and removed BugSomething isn't working labelsNov 7, 2025
@github-actions
Copy link

Deploying Your Preview:https://github.com/appsmithorg/appsmith/actions/runs/19168275199.
Workflow:On demand build Docker image and deploy preview.
skip-tests:true.
env: ``.
PR: 41369.
recreate: .

@github-actionsgithub-actionsbot added the BugSomething isn't working labelNov 7, 2025
@coderabbitai
Copy link
Contributor

coderabbitaibot commentedNov 7, 2025•
edited
Loading

Walkthrough

The PR extends SSH key handling to support RSA PKCS#1 private keys by converting them to PKCS#8 format. A new helper method handles the ASN.1 conversion transparently. Tests are refactored to use BouncyCastleProvider with dynamic RSA key generation.

Changes

Cohort / File(s)Summary
SSH Key Format Conversion
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/helpers/SSHUtils.java		AddedconvertRsaPkcs1ToPkcs8 helper method to convert RSA PKCS#1 keys to PKCS#8 format. Reworked key type detection to handle PKCS#8, PKCS#1, OpenSSH, and PEM formats via distinct code paths. Introduced ASN.1 and Base64 imports to support PrivateKeyInfo reconstruction with rsaEncryption OID.
SSH Key Format Tests
app/server/appsmith-interfaces/src/test/java/com/appsmith/external/helpers/SSHUtilsTest.java		Replaced hard-coded OpenSSH key test with BouncyCastleProvider-based setup. Added dynamic RSA key pair generation and tests for PKCS#8 and PKCS#1→PKCS#8 conversion flows. Introduced private helper methods for key generation and PEM formatting utilities.

Sequence Diagram

sequenceDiagram    participant Client    participant SSHUtils    participant KeyDetection    participant PKCS1Converter    participant PKCS8KeyFile    Client->>SSHUtils: parseKey(keyContent)    SSHUtils->>KeyDetection: Detect key format        alt PKCS#8 Format        KeyDetection-->>PKCS8KeyFile: Feed directly    else PKCS#1 Format (RSA)        KeyDetection->>PKCS1Converter: convertRsaPkcs1ToPkcs8(keyContent)        Note over PKCS1Converter: Parse ASN.1, build PrivateKeyInfo,<br/>encode to PKCS#8 PEM        PKCS1Converter-->>KeyDetection: PKCS#8 PEM string        KeyDetection->>PKCS8KeyFile: Feed converted key    else OpenSSH/Other        KeyDetection-->>PKCS8KeyFile: Handle via existing path    end        PKCS8KeyFile-->>Client: KeyFile object
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • ASN.1 and PrivateKeyInfo reconstruction: Verify correctness of OID handling and ASN.1 encoding logic inconvertRsaPkcs1ToPkcs8
  • Key format detection paths: Ensure all branches handle edge cases and error conditions appropriately
  • Security implications: Confirm cryptographic operations follow best practices and key material is handled securely
  • Test coverage: Validate that dynamic RSA key generation covers representative key sizes and formats

Poem

🔑 PKCS#1 meets PKCS#8 today,
ASN.1 shows the format way,
Keys convert with grace and care,
Stronger SSH keychain everywhere! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.You can run@coderabbitai generate docstrings to improve docstring coverage.
Description check⚠️ WarningPull request description is incomplete; missing issue reference, motivation, context, and technical details about the SSH key parsing fix.Fill in the issue number/URL, add TL;DR explaining the PKCS#1 to PKCS#8 conversion logic, describe the motivation for supporting RSA PKCS#1 keys, and specify any testing details.
✅ Passed checks (1 passed)
Check nameStatusExplanation
Title check✅ PassedThe title 'fix: openssh key parsing issue' clearly summarizes the main change—adding RSA PKCS#1 support to SSH key handling.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branchfix/openssh-key-parsing

Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment@coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

Deploy-Preview-URL:https://ce-41369.dp.appsmith.com

@github-actions
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions
Copy link

This PR has been closed because of inactivity.

@NilanshBansal
Copy link
ContributorAuthor

/build-deploy-preview skip-tests=true recreate=true

github-actions[bot] reacted with rocket emojigithub-actions[bot] reacted with eyes emoji

@github-actions
Copy link

Deploying Your Preview:https://github.com/appsmithorg/appsmith/actions/runs/19702877303.
Workflow:On demand build Docker image and deploy preview.
skip-tests:true.
env: ``.
PR: 41369.
recreate: true.

@github-actions
Copy link

Failed server tests

  • com.appsmith.server.helpers.UserUtilsTest#makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

BugSomething isn't workingok-to-testRequired label for CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@NilanshBansal
[8]ページ先頭
©2009-2025 Movatter.jp