From 070be392f5817e92de7c8ee9c0ea27b3eeb2ff9f Mon Sep 17 00:00:00 2001 From: Ahmed Abushagur Date: Wed, 6 May 2026 17:01:11 -0700 Subject: [PATCH] fix(ssh): auto-repair stale pub that does not pair with local priv (#3395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ssh): verify pub/priv keypair before registering with cloud providers When a local SSH .pub file doesn't actually pair with the corresponding .priv (e.g. .pub copied from another machine, regenerated mid-flow, or edited by hand), spawn would still register the .pub with the cloud provider's key store. The registration check passes by fingerprint, the droplet boots with that key in authorized_keys, and SSH then fails with "Permission denied (publickey)" because the local .priv can't prove ownership of the registered .pub. This produced the silent failure mode where users saw "SSH key 'id_ed25519' already registered with DigitalOcean" immediately followed by 33 "Permission denied" retries. Adds verifyKeyPair() which derives the public key from the private key via `ssh-keygen -y -P "" -f priv` and compares it (key type + base64, ignoring the comment field) to the .pub file. discoverSshKeys() now filters out mismatched pairs with a clear warning naming the offending file, and silently skips passphrase-protected or otherwise unverifiable keys (BatchMode SSH can't use them anyway). Bumps CLI to 1.0.37. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(ssh): auto-repair stale .pub instead of skipping mismatched pair When the local .pub doesn't derive from the matching .priv (stale copy from another machine, etc.), the priv is still authoritative — any .pub that doesn't derive from it is wrong by definition. Previously spawn printed a warning and skipped the pair; now it backs up the stale .pub as .pub.spawn-backup- and rewrites the .pub from the derived key. The next launch uses the correct pub end-to-end, so the droplet boots with a public key that actually pairs with the local priv and SSH handshake succeeds instead of failing 33 times with "Permission denied (publickey)". Passphrase-protected keys (ssh-keygen -y cannot derive without the passphrase) are still skipped silently — nothing to repair with. Bumps CLI to 1.0.38. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: SPA --- packages/cli/package.json | 2 +- .../cli/src/__tests__/ssh-keys-cov.test.ts | 30 ++- packages/cli/src/__tests__/ssh-keys.test.ts | 196 ++++++++++++++++-- packages/cli/src/shared/ssh-keys.ts | 138 +++++++++++- 4 files changed, 344 insertions(+), 22 deletions(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index d7b76af0..b2661d53 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openrouter/spawn", - "version": "1.0.36", + "version": "1.0.38", "type": "module", "bin": { "spawn": "cli.js" diff --git a/packages/cli/src/__tests__/ssh-keys-cov.test.ts b/packages/cli/src/__tests__/ssh-keys-cov.test.ts index e5cca7cf..59ee64d1 100644 --- a/packages/cli/src/__tests__/ssh-keys-cov.test.ts +++ b/packages/cli/src/__tests__/ssh-keys-cov.test.ts @@ -123,7 +123,7 @@ describe("generateSshKey race recovery", () => { }); describe("discoverSshKeys with unknown key type", () => { - it("labels key as UNKNOWN when ssh-keygen fails", () => { + it("labels key as UNKNOWN when ssh-keygen -lf fails (after verification passes)", () => { const sshDir = join(tmpDir, ".ssh"); mkdirSync(sshDir, { recursive: true, @@ -134,8 +134,11 @@ describe("discoverSshKeys with unknown key type", () => { }); writeFileSync(join(sshDir, "id_custom.pub"), "some-key AAAA fake\n"); - // ssh-keygen throws - const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation(() => { + // verify (`-y`) succeeds with matching pub; getKeyType (`-lf`) throws + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation((args: string[]) => { + if (args[1] === "-y") { + return makeSyncResult("some-key AAAA fake\n"); + } throw new Error("command not found"); }); @@ -145,7 +148,7 @@ describe("discoverSshKeys with unknown key type", () => { expect(keys[0].type).toBe("UNKNOWN"); }); - it("labels key as UNKNOWN when ssh-keygen output has no parenthesized type", () => { + it("labels key as UNKNOWN when ssh-keygen -lf output has no parenthesized type", () => { const sshDir = join(tmpDir, ".ssh"); mkdirSync(sshDir, { recursive: true, @@ -156,9 +159,12 @@ describe("discoverSshKeys with unknown key type", () => { }); writeFileSync(join(sshDir, "id_weird.pub"), "weird-key AAAA fake\n"); - const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue( - makeSyncResult("256 SHA256:abc user@host"), // no (TYPE) suffix - ); + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation((args: string[]) => { + if (args[1] === "-y") { + return makeSyncResult("weird-key AAAA fake\n"); + } + return makeSyncResult("256 SHA256:abc user@host"); // no (TYPE) suffix + }); const keys = discoverSshKeys(); spawnSpy.mockRestore(); @@ -210,6 +216,16 @@ describe("discoverSshKeys sorting", () => { const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation((args: string[]) => { const path = String(args[args.length - 1]); + if (args[1] === "-y") { + // verify (`-y`) call: return the matching pub file contents from disk + if (path.endsWith("id_ed25519")) { + return makeSyncResult("ssh-ed25519 AAAA\n"); + } + if (path.endsWith("id_rsa")) { + return makeSyncResult("ssh-rsa AAAA\n"); + } + return makeSyncResult("ecdsa-sha2 AAAA\n"); + } if (path.includes("ed25519")) { return makeSyncResult("256 SHA256:x (ED25519)"); } diff --git a/packages/cli/src/__tests__/ssh-keys.test.ts b/packages/cli/src/__tests__/ssh-keys.test.ts index 9e7c32bc..a5ef38bc 100644 --- a/packages/cli/src/__tests__/ssh-keys.test.ts +++ b/packages/cli/src/__tests__/ssh-keys.test.ts @@ -6,7 +6,7 @@ */ import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test"; -import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { tryCatch } from "@openrouter/spawn-shared"; import { mockClackPrompts } from "./test-helpers"; @@ -18,9 +18,16 @@ mockClackPrompts({ // ── Import after @clack/prompts mock ──────────────────────────────────────── -const { discoverSshKeys, generateSshKey, getSshFingerprint, ensureSshKeys, getSshKeyOpts, _resetCache } = await import( - "../shared/ssh-keys" -); +const { + discoverSshKeys, + generateSshKey, + getSshFingerprint, + ensureSshKeys, + getSshKeyOpts, + verifyKeyPair, + repairPubFromPriv, + _resetCache, +} = await import("../shared/ssh-keys"); // ─── Temp dir helpers ─────────────────────────────────────────────────────── @@ -124,6 +131,46 @@ function sshKeygenMd5Result(): Bun.SyncSubprocess<"pipe", "pipe"> { return makeSyncResult("256 MD5:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99 user@host (ED25519)"); } +/** + * Smart mock for Bun.spawnSync that handles all three ssh-keygen invocations + * used by ssh-keys.ts: + * - `ssh-keygen -y -P "" -f ` (verifyKeyPair) — returns the contents + * of the corresponding .pub file from disk so verification reports "match" + * - `ssh-keygen -lf ` (getKeyType) — returns lf output for the given + * keyType (default ED25519, or RSA if pub path contains "rsa") + * - `ssh-keygen -lf -E md5` (getSshFingerprint) — returns MD5 output + * + * Pass `mismatch: true` to make verifyKeyPair return "mismatch" instead. + */ +function smartSshKeygenMock(opts: { mismatch?: boolean } = {}): (args: string[]) => Bun.SyncSubprocess<"pipe", "pipe"> { + return (args: string[]) => { + if (args[1] === "-y") { + const privPath = args[args.length - 1]; + const pubPath = `${privPath}.pub`; + if (opts.mismatch) { + return makeSyncResult("ssh-ed25519 AAAADIFFERENT spawn\n"); + } + const pubText = unwrapOrEmpty(() => readFileSync(pubPath, "utf-8")); + return makeSyncResult(pubText); + } + if (args.includes("-E") && args[args.indexOf("-E") + 1] === "md5") { + return sshKeygenMd5Result(); + } + if (args[1] === "-lf") { + const pubPath = args[2]; + const type = pubPath.includes("rsa") ? "RSA" : "ED25519"; + return sshKeygenLfResult(type); + } + // Default: empty success + return makeSyncResult(""); + }; +} + +function unwrapOrEmpty(fn: () => T): T | "" { + const r = tryCatch(fn); + return r.ok ? r.data : ""; +} + /** * Build a mock spawnSync result that simulates successful ssh-keygen key generation. * Also writes the expected output files so existsSync checks pass. @@ -180,7 +227,7 @@ describe("discoverSshKeys", () => { it("discovers a single key pair", () => { createFakeKeyPair("id_ed25519", "ed25519"); - const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue(sshKeygenLfResult("ED25519")); + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation(smartSshKeygenMock()); const keys = discoverSshKeys(); spawnSpy.mockRestore(); expect(keys).toHaveLength(1); @@ -189,6 +236,57 @@ describe("discoverSshKeys", () => { expect(keys[0].privPath).toContain("id_ed25519"); expect(keys[0].pubPath).toContain("id_ed25519.pub"); }); + + it("auto-repairs pairs whose .pub does not match the local private key", () => { + const { pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + const staleContents = readFileSync(pubPath, "utf-8"); + const derivedContents = "ssh-ed25519 AAAADERIVED spawn\n"; + + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation((args: string[]) => { + if (args[1] === "-y") { + // ssh-keygen -y derives the *correct* pub from the priv + return makeSyncResult(derivedContents); + } + if (args.includes("-E") && args[args.indexOf("-E") + 1] === "md5") { + return sshKeygenMd5Result(); + } + return sshKeygenLfResult("ED25519"); + }); + + const keys = discoverSshKeys(); + spawnSpy.mockRestore(); + + // Pair is returned, not skipped + expect(keys).toHaveLength(1); + expect(keys[0].name).toBe("id_ed25519"); + expect(keys[0].pubPath).toBe(pubPath); + + // .pub has been rewritten with the derived contents + expect(readFileSync(pubPath, "utf-8")).toBe(derivedContents); + + // The stale contents are preserved in a backup file + const sshDir = join(tmpDir, ".ssh"); + const files = readdirSync(sshDir); + const backup = files.find((f) => f.startsWith("id_ed25519.pub.spawn-backup-")); + expect(backup).toBeDefined(); + if (backup) { + expect(readFileSync(join(sshDir, backup), "utf-8")).toBe(staleContents); + } + }); + + it("skips pairs that ssh-keygen cannot derive (e.g. passphrase-protected)", () => { + createFakeKeyPair("id_ed25519", "ed25519"); + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation((args: string[]) => { + if (args[1] === "-y") { + // Simulate ssh-keygen -y failing (e.g. passphrase prompt rejected) + return makeSyncResult("", 1); + } + return sshKeygenLfResult("ED25519"); + }); + const keys = discoverSshKeys(); + spawnSpy.mockRestore(); + expect(keys).toEqual([]); + }); }); // ─── generateSshKey ───────────────────────────────────────────────────────── @@ -279,7 +377,7 @@ describe("ensureSshKeys", () => { it("uses single key silently when only one is found", async () => { createFakeKeyPair("id_rsa", "rsa"); - const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue(sshKeygenLfResult("RSA")); + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation(smartSshKeygenMock()); const keys = await ensureSshKeys(); spawnSpy.mockRestore(); expect(keys).toHaveLength(1); @@ -290,11 +388,7 @@ describe("ensureSshKeys", () => { createFakeKeyPair("id_ed25519", "ed25519"); createFakeKeyPair("id_rsa", "rsa"); - const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation((args: string[]) => { - const pubPath = args[args.length - 1]; - const type = pubPath.includes("ed25519") ? "ED25519" : "RSA"; - return sshKeygenLfResult(type); - }); + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation(smartSshKeygenMock()); const keys = await ensureSshKeys(); spawnSpy.mockRestore(); @@ -305,7 +399,7 @@ describe("ensureSshKeys", () => { it("caches results across calls", async () => { createFakeKeyPair("id_ed25519", "ed25519"); - const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue(sshKeygenLfResult("ED25519")); + const spawnSpy = spyOn(Bun, "spawnSync").mockImplementation(smartSshKeygenMock()); const keys1 = await ensureSshKeys(); const keys2 = await ensureSshKeys(); @@ -314,6 +408,84 @@ describe("ensureSshKeys", () => { }); }); +// ─── verifyKeyPair ────────────────────────────────────────────────────────── + +describe("verifyKeyPair", () => { + it("returns 'match' when the derived public key equals the .pub file (ignoring comment)", () => { + const { privPath, pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + // Same key core as createFakeKeyPair writes, with a different comment field + const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue( + makeSyncResult("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFake different-comment\n"), + ); + const result = verifyKeyPair(privPath, pubPath); + spawnSpy.mockRestore(); + expect(result).toBe("match"); + }); + + it("returns 'mismatch' when the derived public key differs from the .pub file", () => { + const { privPath, pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue( + makeSyncResult("ssh-ed25519 AAAACOMPLETELYDIFFERENT spawn\n"), + ); + const result = verifyKeyPair(privPath, pubPath); + spawnSpy.mockRestore(); + expect(result).toBe("mismatch"); + }); + + it("returns 'unverifiable' when ssh-keygen exits non-zero (e.g. passphrase)", () => { + const { privPath, pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue(makeSyncResult("", 1)); + const result = verifyKeyPair(privPath, pubPath); + spawnSpy.mockRestore(); + expect(result).toBe("unverifiable"); + }); + + it("returns 'unverifiable' when the .pub file is missing or empty", () => { + const { privPath, pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + rmSync(pubPath); + const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue( + makeSyncResult("ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIFake spawn\n"), + ); + const result = verifyKeyPair(privPath, pubPath); + spawnSpy.mockRestore(); + expect(result).toBe("unverifiable"); + }); +}); + +// ─── repairPubFromPriv ────────────────────────────────────────────────────── + +describe("repairPubFromPriv", () => { + it("rewrites the .pub from the derived key and backs up the original", () => { + const { privPath, pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + const stale = readFileSync(pubPath, "utf-8"); + const derived = "ssh-ed25519 AAAADERIVEDCONTENT spawn\n"; + const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue(makeSyncResult(derived)); + + const backupPath = repairPubFromPriv(privPath, pubPath); + spawnSpy.mockRestore(); + + expect(backupPath).not.toBeNull(); + expect(backupPath).toContain(".spawn-backup-"); + expect(readFileSync(pubPath, "utf-8")).toBe(derived); + if (backupPath) { + expect(readFileSync(backupPath, "utf-8")).toBe(stale); + } + }); + + it("returns null when the private key cannot be derived (e.g. passphrase)", () => { + const { privPath, pubPath } = createFakeKeyPair("id_ed25519", "ed25519"); + const stale = readFileSync(pubPath, "utf-8"); + const spawnSpy = spyOn(Bun, "spawnSync").mockReturnValue(makeSyncResult("", 1)); + + const backupPath = repairPubFromPriv(privPath, pubPath); + spawnSpy.mockRestore(); + + expect(backupPath).toBeNull(); + // .pub is untouched, no backup created + expect(readFileSync(pubPath, "utf-8")).toBe(stale); + }); +}); + // ─── getSshKeyOpts ────────────────────────────────────────────────────────── describe("getSshKeyOpts", () => { diff --git a/packages/cli/src/shared/ssh-keys.ts b/packages/cli/src/shared/ssh-keys.ts index 14209602..6230b14d 100644 --- a/packages/cli/src/shared/ssh-keys.ts +++ b/packages/cli/src/shared/ssh-keys.ts @@ -1,9 +1,9 @@ // shared/ssh-keys.ts — SSH key discovery, selection, and generation -import { existsSync, mkdirSync, readdirSync } from "node:fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { getSshDir } from "./paths.js"; import { isFileError, tryCatch, tryCatchIf, unwrapOr } from "./result.js"; -import { logInfo, logStep } from "./ui.js"; +import { logInfo, logStep, logWarn } from "./ui.js"; // ─── Types ────────────────────────────────────────────────────────────────── @@ -52,6 +52,30 @@ export function discoverSshKeys(): SshKeyPair[] { continue; } + // Auto-repair pairs whose public key doesn't pair with the private key. + // Without this, the stale .pub gets registered with the cloud provider and + // SSH fails with "Permission denied (publickey)" because the local .priv + // can't prove ownership of that .pub. The .priv is authoritative — any .pub + // that doesn't derive from it is wrong by definition, so we back up the + // stale file and rewrite .pub from the derived key. + // + // Passphrase-protected and otherwise unverifiable keys are skipped silently + // — BatchMode SSH can't use them without an active ssh-agent anyway. + const verification = verifyKeyPair(privPath, pubPath); + if (verification === "mismatch") { + const repaired = repairPubFromPriv(privPath, pubPath); + if (!repaired) { + logWarn( + `SSH key '${baseName}' skipped: ${pubPath} does not pair with the local private key and could not be repaired automatically.`, + ); + continue; + } + logInfo(`Repaired ${pubPath} (stale public key replaced; original saved as ${repaired}).`); + // fall through — pair is now valid + } else if (verification === "unverifiable") { + continue; + } + // Extract key type via ssh-keygen const keyType = getKeyType(pubPath); pairs.push({ @@ -84,6 +108,116 @@ export function discoverSshKeys(): SshKeyPair[] { return pairs; } +/** + * Read the first two whitespace-separated fields ("type base64") from an OpenSSH + * public key string, ignoring trailing comment. Returns "" if the input is empty + * or malformed. + */ +function pubKeyCore(text: string): string { + const trimmed = text.trim(); + if (!trimmed) { + return ""; + } + const parts = trimmed.split(/\s+/); + if (parts.length < 2) { + return ""; + } + return `${parts[0]} ${parts[1]}`; +} + +/** + * Derive the public key text from a private key via `ssh-keygen -y`. + * Returns the raw stdout (e.g. `"ssh-ed25519 AAAA... comment\n"`) on success, + * or "" when the private key is passphrase-protected, corrupt, or missing. + */ +function derivePubFromPriv(privPath: string): string { + return unwrapOr( + tryCatch(() => { + const result = Bun.spawnSync( + [ + "ssh-keygen", + "-y", + "-P", + "", + "-f", + privPath, + ], + { + stdio: [ + "ignore", + "pipe", + "pipe", + ], + }, + ); + if (result.exitCode !== 0) { + return ""; + } + return new TextDecoder().decode(result.stdout); + }), + "", + ); +} + +/** + * Verify that a private/public keypair on disk are actually paired: + * derive the public key from the private key and compare to the `.pub`. + * + * Returns: + * - "match" — derived public matches `.pub` + * - "mismatch" — files exist but do NOT pair (silent-failure source) + * - "unverifiable" — passphrase-protected, corrupt, or otherwise can't derive + * (skip silently — spawn's BatchMode SSH can't use these + * anyway unless the user has them in ssh-agent) + */ +export function verifyKeyPair(privPath: string, pubPath: string): "match" | "mismatch" | "unverifiable" { + const derivedCore = pubKeyCore(derivePubFromPriv(privPath)); + if (!derivedCore) { + return "unverifiable"; + } + + const pubText = unwrapOr( + tryCatchIf(isFileError, () => readFileSync(pubPath, "utf-8")), + "", + ); + const pubCore = pubKeyCore(pubText); + if (!pubCore) { + return "unverifiable"; + } + + return derivedCore === pubCore ? "match" : "mismatch"; +} + +/** + * Repair a stale `.pub` file by rewriting it from the matching private key. + * + * The original `.pub` is preserved as `.spawn-backup-` so + * the user can inspect what was replaced. Returns the backup path on success, + * or null if the private key couldn't be read (passphrase-protected, etc.) or + * the filesystem write failed. + * + * Safe because the `.priv` is authoritative: any `.pub` that doesn't derive + * from it is wrong by definition. + */ +export function repairPubFromPriv(privPath: string, pubPath: string): string | null { + const derived = derivePubFromPriv(privPath); + if (!pubKeyCore(derived)) { + return null; + } + + const backupPath = `${pubPath}.spawn-backup-${Date.now()}`; + const result = tryCatchIf(isFileError, () => { + renameSync(pubPath, backupPath); + writeFileSync(pubPath, derived, { + mode: 0o644, + }); + }); + if (!result.ok) { + return null; + } + return backupPath; +} + /** Extract the key type from a public key file using ssh-keygen. */ function getKeyType(pubPath: string): string { return unwrapOr(