Add SSH key generation and deployment features #234
Reference in New Issue
Block a user
Delete Branch "main"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Features Added
SSH Key Generation
SSH Key Deployment
Technical Improvements
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Walkthrough
Adds SSH key pair support across backend, UI, and types: database schema gains private/public key fields; backend routes add key parsing, detection, generation, validation, and deploy-to-host endpoints; SSH connection code prefers privateKey; new SSH key utilities module; UI updates for key generation/detection and deployment; API wrappers added.
Changes
src/backend/database/db/schema.tsprivate_key,public_key,detected_key_type; retainskeywith backward-compatibility note.src/backend/utils/ssh-key-utils.tsparseSSHKey,parsePublicKey,detectKeyType,getFriendlyKeyTypeName,validateKeyPair.src/backend/database/routes/credentials.tssrc/backend/ssh/terminal.ts,src/backend/ssh/tunnel.tscredential.privateKeyovercredential.keywhen setting SSH connect config key.src/types/index.tspublicKey?: stringtoCredentialandCredentialData.src/ui/Desktop/Apps/Credentials/CredentialEditor.tsx,.../unified_key_section.tsxpublicKey.src/ui/Desktop/Apps/Credentials/CredentialsManager.tsxsrc/ui/main-axios.tsSequence Diagram(s)
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
@coderabbitai generate docstringsto improve docstring coverage.✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment
@coderabbitai helpto get the list of available commands and usage tips.Actionable comments posted: 16
🧹 Nitpick comments (16)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between
0f75cd4d16anda674073ec8.⛔ Files ignored due to path filters (2)
src/locales/en/translation.jsonis excluded by!**/*.jsonsrc/locales/zh/translation.jsonis excluded by!**/*.json📒 Files selected for processing (10)
src/backend/database/db/schema.ts(1 hunks)src/backend/database/routes/credentials.ts(7 hunks)src/backend/ssh/terminal.ts(1 hunks)src/backend/ssh/tunnel.ts(2 hunks)src/backend/utils/ssh-key-utils.ts(1 hunks)src/types/index.ts(2 hunks)src/ui/Desktop/Apps/Credentials/CredentialEditor.tsx(10 hunks)src/ui/Desktop/Apps/Credentials/CredentialsManager.tsx(6 hunks)src/ui/main-axios.ts(9 hunks)unified_key_section.tsx(1 hunks)🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
Files:
src/backend/ssh/terminal.tssrc/types/index.tssrc/ui/Desktop/Apps/Credentials/CredentialsManager.tsxsrc/backend/ssh/tunnel.tssrc/backend/utils/ssh-key-utils.tssrc/backend/database/db/schema.tssrc/backend/database/routes/credentials.tsunified_key_section.tsxsrc/ui/main-axios.tssrc/ui/Desktop/Apps/Credentials/CredentialEditor.tsx**/backend/**/*.{ts,js}
⚙️ CodeRabbit configuration file
Files:
src/backend/ssh/terminal.tssrc/backend/ssh/tunnel.tssrc/backend/utils/ssh-key-utils.tssrc/backend/database/db/schema.tssrc/backend/database/routes/credentials.ts**/types/**/*.{ts,js}
⚙️ CodeRabbit configuration file
Files:
src/types/index.ts**/main-axios.ts
⚙️ CodeRabbit configuration file
Files:
src/ui/main-axios.ts🧬 Code graph analysis (6)
src/ui/Desktop/Apps/Credentials/CredentialsManager.tsx (2)
src/backend/utils/ssh-key-utils.ts (1)
src/backend/database/routes/credentials.ts (2)
unified_key_section.tsx (1)
src/ui/main-axios.ts (1)
src/ui/Desktop/Apps/Credentials/CredentialEditor.tsx (3)
🪛 Gitleaks (8.27.2)
src/backend/utils/ssh-key-utils.ts
[high] 10-31: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 64-67: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 70-75: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 79-148: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
src/backend/database/routes/credentials.ts
[high] 986-1000: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🔇 Additional comments (9)
@@ -142,3 +144,4 @@keyType: text("key_type"),detectedKeyType: text("detected_key_type"),usageCount: integer("usage_count").notNull().default(0),lastUsed: text("last_used"),🛠️ Refactor suggestion
⚠️ Potential issue
Encrypt private keys at rest; reconsider TEXT length hints.
privateKeyin plaintext is a high‑risk posture. Encrypt at rest (e.g., libsodium or AES‑GCM with app‑level KEK) and ensure strict access controls.publicKeyto avoid truncation risk from long comments.Also add a migration to backfill
privateKeyfromkeyand then phase outkey.📝 Committable suggestion
@@ -8,0 +34,4 @@options.passphrase = passphrase;options.cipher = 'aes128-cbc'; // Default cipher for encrypted private keys}🛠️ Refactor suggestion
⚠️ Potential issue
Weak/default crypto options in key generation.
Also consider exposing ECDSA curves via options.curve instead of bits for clarity.
Also applies to: 38-50
@@ -664,0 +941,4 @@try {console.log("Using Node.js crypto to generate public key from private key...");console.log("Private key length:", privateKey.length);console.log("Private key first 100 chars:", privateKey.substring(0, 100));🛠️ Refactor suggestion
⚠️ Potential issue
Secret leakage: verbose logs print key material and lengths. Remove before release.
Logging “first 100 chars” and similar leaks private/public keys. Use structured logs without secrets.
Apply redaction:
Repeat across all key-handling endpoints. Gate any remaining diagnostics behind a DEBUG flag and never log key contents.
Also applies to: 799-835, 837-879, 891-899, 941-969, 1039-1046, 1055-1066, 1084-1097, 1106-1110
@@ -664,0 +1264,4 @@success: false,error: "Credential ID and target host ID are required"});}💡 Verification agent
🧩 Analysis chain
Address Gitleaks finding (private-key).
The flagged lines are due to debug logs printing key material. Removing/redacting them resolves the alert.
Run to confirm no key contents are logged:
🏁 Script executed:
Length of output: 2283
Remove/redact debug logs that print SSH private/public key material
Gitleaks flagged console logs that expose key contents. Remove or redact any logs that print keys, substrings, or raw key content; log only presence/status or a hashed/truncated indicator.
Replace with non-sensitive logs (e.g., privateKeyProvided: true, generatedPublicKey: true) or redact values (e.g., "[REDACTED]" or a short hash). Re-run scans after changes.
🧰 Tools
🪛 Gitleaks (8.27.2)
[high] 986-1000: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⚠️ Potential issue
generate-public-key may return PEM; enforce SSH format or fail.
The UI and PR promise SSH-format output. If ssh2 conversion fails, returning PEM breaks deployment. Prefer returning 400 with guidance rather than a PEM fallback.
🧰 Tools
🪛 Gitleaks (8.27.2)
[high] 986-1000: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🤖 Prompt for AI Agents
@@ -664,0 +1345,4 @@} else if (cred.authType === 'key') {hostConfig.privateKey = cred.privateKey || cred.key; // Try both fieldshostConfig.keyPassword = cred.keyPassword;}🛠️ Refactor suggestion
⚠️ Potential issue
Command injection risk and fragile grep in deployment.
Apply:
Recommended: replace shell appends with SFTP append to ~/.ssh/authorized_keys to eliminate shell injection entirely.
Also applies to: 1191-1200, 1176-1189
🤖 Prompt for AI Agents
🛠️ Refactor suggestion
Add connection timeouts to execs and cleanly handle stream errors.
Wrap exec calls with error and stderr handlers; ensure reject on non-zero and capture stderr for diagnostics.
⚠️ Potential issue
Authorization bypass: deploy-to-host lacks user scoping for both credential and host. Blocker.
Endpoint does not restrict credential/host to the authenticated user. An attacker could deploy keys to another user’s hosts via ID enumeration.
Apply scoping:
Also validate that any resolved hostCredential (when host uses credential) belongs to the same user.
Also applies to: 1317-1363, 1364-1396
@@ -0,0 +245,4 @@console.log('First 100 chars:', privateKeyData?.substring(0, 100) || 'undefined');console.log('ssh2Utils available:', typeof ssh2Utils);console.log('parseKey function available:', typeof ssh2Utils?.parseKey);⚠️ Potential issue
Do not log key material (even partial).
Printing the first 100 chars of private/public keys (and derived key data) is a severe secret‑leak risk.
Apply this sanitization:
If you need diagnostics, gate redacted logs behind a dedicated DEBUG flag and never print raw key content.
Also applies to: 369-372, 431-434, 488-490
🤖 Prompt for AI Agents
@@ -0,0 +288,4 @@} else {publicKey = `${keyType} ${base64Data}`;}⚠️ Potential issue
Bug: Misencoding public key —
getPublicSSH()already returns OpenSSH text.You’re base64‑encoding the OpenSSH public key again, producing invalid keys.
Apply this fix:
📝 Committable suggestion
🤖 Prompt for AI Agents
@@ -0,0 +333,4 @@success: keyType !== 'unknown'};} catch (error) {console.error('Exception during SSH key parsing:', error);⚠️ Potential issue
Don’t mark fallback heuristics as success.
If ssh2 parsing fails, heuristics should not set
success: true. Returnsuccess: falseunless we positively parsed and/or generated a public key.📝 Committable suggestion
🤖 Prompt for AI Agents
@@ -0,0 +513,4 @@privateKeyType: privateKeyInfo.keyType,publicKeyType: publicKeyInfo.keyType,error: 'Unable to verify key pair match, but key types are compatible'};⚠️ Potential issue
Validation must be definitive — don’t return true without a match.
Currently returns
isValid: truewhen types match but key matching is unverified. That can accept mismatched pairs.📝 Committable suggestion
🤖 Prompt for AI Agents
@@ -206,0 +314,4 @@};return keyTypeMap[keyType] || keyType;};🛠️ Refactor suggestion
Key type detection should react to keyPassword changes.
Trigger detection when keyPassword changes if a key is present.
Note: capture values via form.getValues() to avoid extra renders.
🤖 Prompt for AI Agents
@@ -93,0 +130,4 @@toast.error("Only SSH key-based credentials can be deployed");return;}if (!credential.publicKey) {🛠️ Refactor suggestion
Type safety for hosts; avoid any[].
Use SSHHost[] for availableHosts and number for selectedHostId where possible. This reduces parseInt churn and runtime errors.
And when calling:
Also applies to: 100-107
🤖 Prompt for AI Agents
🛠️ Refactor suggestion
Add response types for new credential/key APIs.
Avoid Promise; define small interfaces (e.g., { success: boolean; keyType?: string; publicKey?: string; error?: string }).
@@ -0,0 +88,4 @@: 'text-green-600'}`}>{getFriendlyKeyTypeName(detectedKeyType)}</span>🛠️ Refactor suggestion
Avoid importing backend-only utils in the frontend; inline or move key-type map to a shared UI module.
getFriendlyKeyTypeName lives under backend/utils; bundlers may pull server-only deps. Create a small UI helper or shared constants and import from there.
Proposed UI helper:
Also applies to: 205-210
🤖 Prompt for AI Agents