fix(ssh): repair mangled PEM private keys before parsing#1147
Conversation
A valid PEM key whose framing was damaged in transit — newlines collapsed to spaces, turned into literal "\n", or lines indented — fails ssh2's parser with "Unsupported key format" even though the key material is intact. This commonly happens when a key is copy/pasted through a field or app that strips line breaks. (follow-up to #1139) When parsing fails, rebuild clean PEM framing from the BEGIN/END markers (which survive newline loss) and the base64 body, then retry through the existing parse and PKCS#8 conversion paths. The body is preserved byte-for-byte and a repaired key is only used if it re-validates, so this can never produce a different or invalid key. Encrypted legacy PEM (Proc-Type/DEK-Info) and truncated keys are left untouched. Refs #1139 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa8f8bfc11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (repaired && repaired !== privateKey) { | ||
| const reparsed = sshUtils.parseKey(repaired, passphrase); | ||
| if (reparsed && !(reparsed instanceof Error)) { | ||
| return { privateKey: repaired, passphrase, converted: true }; | ||
| } |
There was a problem hiding this comment.
Preserve repaired encrypted OpenSSH keys before prompting
For an encrypted OpenSSH key whose line breaks became literal \n, this path repairs the PEM but then discards it because ssh2.parseKey(repaired, undefined) fails without a passphrase, and the non-PKCS#8 fallback returns the original mangled text. In the first-party auth flow, that same literal-\n form is not reliably detected as encrypted before resolveKeyForAuth is called, so the user never gets a passphrase retry with the repaired key even though normalizePrivateKeyForSsh2(mangled, passphrase) would work. Please keep or surface the repaired candidate for encrypted non-PKCS#8 keys instead of falling back to the unrepaired input when the only missing piece is a passphrase.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified. The repaired candidate was indeed being discarded, but the reason there's no passphrase retry is one layer up: isKeyEncrypted() couldn't read the cipher from the mangled OpenSSH blob — the literal \n corrupts the base64 decode of the openssh-key-v1 header — so the key was routed to the unencrypted branch and never prompted. Surfacing the repaired candidate alone wouldn't help, since resolveKeyForAuth re-validates without a passphrase and drops it.
Fixed in 81e02ca by repairing the PEM framing before reading the cipher name in isKeyEncrypted, so the key is detected as encrypted and reaches the passphrase prompt — where normalizePrivateKeyForSsh2(key, passphrase) already repairs and validates it (exactly the path you noted would work). Added a regression test: a literal-\n encrypted OpenSSH key now triggers exactly one prompt and parses with the passphrase.
A mangled encrypted OpenSSH key (line breaks flattened to literal "\n") was not recognized as encrypted: the literal escapes corrupt the base64 decode used to read the cipher name, so isKeyEncrypted() returned false and preparePrivateKeyForAuth routed the key to the unencrypted branch with no passphrase prompt — and the repaired candidate was discarded because it can't parse without one. Repair the PEM framing before reading the OpenSSH cipher name, so such keys are detected as encrypted and reach the passphrase prompt, where normalizePrivateKeyForSsh2(key, passphrase) already repairs and validates them. Addresses Codex review feedback on #1147. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Problem
A valid PEM private key whose text framing was damaged before it reached netcatty — newlines collapsed to spaces, turned into literal
\n, or every line indented — fails with:ssh2 needs a real newline right after the
-----BEGIN ... -----header and proper line structure; lose that and it rejects the key even though the key bytes are intact. This commonly happens when a key is copy/pasted through a chat box, web form, or JSON string that strips or escapes line breaks.This is the second of the two plausible causes behind #1139 (the first, PKCS#8, was #1146). netcatty itself doesn't mangle keys — verified there's no internal newline stripping — so the damage is upstream and the right fix is to be liberal in what we accept.
Fix
Extends
privateKeyNormalizer.cjs: when parsing fails,repairMalformedPem()rebuilds clean PEM framing from theBEGIN/ENDmarkers (which survive newline loss) plus the base64 body, then retries through the existing parse + PKCS#8 paths.Safety:
Proc-Type/DEK-Infolines inside the block) and truncated keys untouched.Tests
privateKeyNormalizer.test.cjsadds: collapsed-to-spaces, literal\n, indented, and collapsed-PKCS#8 (repair + convert) all become parseable; truncated and encrypted-legacy-PEM are left unchanged. FullsshAuthHelper+ normalizer suites pass (39 tests); eslint clean.Refs #1139
🤖 Generated with Claude Code