[keyman] phase 4: decrypt stops destroying keys and stops the 0644 window

Verified before the fix: `age -d -o <existing>` overwrites without a word
("PRECIOUS EXISTING KEY" became "secret"), and the old `cp` for the public
key did the same. Decrypting a vault entry on top of a newer working key
in ~/.ssh destroyed it with no prompt, no backup and no mention. It is the
only finding in the audit that loses data the user never asked to touch.

Every collision — private and public, both output modes — is now settled
before anything is written, so the questions are asked about files that
still exist. Default is to keep what is there.

cp and chmod are gone. Three spawns per key become one, it works where
those binaries do not, and the chmod happens in-process immediately after
age returns: age creates its output 0644 regardless of umask, so a
plaintext private key was world-readable for the length of two spawns and
stayed 0644 whenever the chmod itself failed. ~/.ssh is created 0700 when
absent rather than assumed.

decrypt.test.ts stops asserting on which binaries were spawned. The age
stand-in now writes its -o file at 0644 the way age does, and the tests
assert the bytes and the mode on disk — the outcome rather than the
mechanism.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

packages/keyman/src/keyman.decrypt.ts

@@ -1,9 +1,18 @@
 import fs from 'node:fs';
 import path from 'node:path';
-import { execa } from 'execa';
 import inquirer from 'inquirer';
 import { runTool } from './keyman.utils.js';
 
+const LOCAL_MODE = 'Local (vault/tmp)';
+
+interface DecryptPlan {
+  key: string;
+  encryptedKey: string;
+  publicKey: string;
+  privateKeyOut: string;
+  publicKeyOut: string;
+}
+
 export async function decryptKeys(sshDir: string, vaultDir: string, ageKey: string) {
   const keyDir = path.join(vaultDir, 'keys');
   // Guarded: nothing creates the keys directory until the first encrypt, so on a
@@ -28,27 +37,71 @@ export async function decryptKeys(sshDir: string, vaultDir: string, ageKey: stri
       type: 'list',
       name: 'decryptMode',
       message: 'Choose decryption location:',
-      choices: ['Local (vault/tmp)', 'SSH (~/.ssh)'],
+      choices: [LOCAL_MODE, 'SSH (~/.ssh)'],
     },
   ]);
 
-  for (const key of selectedKeys) {
-    const encryptedKey = path.join(keyDir, key, `id_${key}.age`);
-    const publicKey = path.join(keyDir, key, `id_${key}.pub`);
-    const privateKeyOut =
-      decryptMode === 'Local (vault/tmp)'
-        ? path.join(vaultDir, 'tmp', `id_${key}`)
-        : path.join(sshDir, `id_${key}`);
-    const publicKeyOut =
-      decryptMode === 'Local (vault/tmp)'
-        ? path.join(vaultDir, 'tmp', `id_${key}.pub`)
-        : path.join(sshDir, `id_${key}.pub`);
+  const outDir = decryptMode === LOCAL_MODE ? path.join(vaultDir, 'tmp') : sshDir;
 
-    // Decrypt key
-    await runTool('age', ['-d', '-i', ageKey, '-o', privateKeyOut, encryptedKey]);
+  const plans: DecryptPlan[] = selectedKeys.map((key: string) => ({
+    key,
+    encryptedKey: path.join(keyDir, key, `id_${key}.age`),
+    publicKey: path.join(keyDir, key, `id_${key}.pub`),
+    privateKeyOut: path.join(outDir, `id_${key}`),
+    publicKeyOut: path.join(outDir, `id_${key}.pub`),
+  }));
 
-    await execa('cp', [publicKey, publicKeyOut]);
-    await execa('chmod', ['600', privateKeyOut]);
-    console.log(`✅ Decrypted: ${privateKeyOut}`);
+  // Every collision is settled before anything is written. `age -d -o` and the
+  // old `cp` both overwrote silently, so decrypting a vault key on top of a
+  // newer working key destroyed it with no prompt and no copy — and the user is
+  // answering these questions about files that still exist.
+  const approved: DecryptPlan[] = [];
+  for (const plan of plans) {
+    const existing = [plan.privateKeyOut, plan.publicKeyOut].filter((file) => fs.existsSync(file));
+
+    if (existing.length === 0) {
+      approved.push(plan);
+      continue;
+    }
+
+    const { overwrite } = await inquirer.prompt<{ overwrite: boolean }>([
+      {
+        type: 'confirm',
+        name: 'overwrite',
+        message: `${existing.join(', ')} already present. Overwrite?`,
+        default: false,
+      },
+    ]);
+
+    if (overwrite) {
+      approved.push(plan);
+    } else {
+      console.log(`⏭️  Skipped ${plan.key} — kept what was already there.`);
+    }
+  }
+
+  if (approved.length === 0) {
+    return;
+  }
+
+  // 0700: ~/.ssh may not exist yet, and it is about to hold a private key.
+  fs.mkdirSync(outDir, { recursive: true, mode: 0o700 });
+
+  for (const plan of approved) {
+    await runTool('age', ['-d', '-i', ageKey, '-o', plan.privateKeyOut, plan.encryptedKey]);
+    // Immediately, and in-process: age creates its output 0644 regardless of
+    // umask, so this used to be a world-readable private key for the length of
+    // two process spawns — and stayed 0644 whenever the chmod itself failed.
+    fs.chmodSync(plan.privateKeyOut, 0o600);
+
+    if (fs.existsSync(plan.publicKey)) {
+      fs.copyFileSync(plan.publicKey, plan.publicKeyOut);
+    } else {
+      console.log(
+        `⚠️  ${plan.key} has no public key in the vault; only the private key was written.`
+      );
+    }
+
+    console.log(`✅ Decrypted: ${plan.privateKeyOut}`);
   }
 }

packages/keyman/tests/decrypt.test.ts

@@ -1,8 +1,10 @@
 /**
  * Tests for decryptKeys.
  *
- * age, cp and chmod are all mocked; the assertions cover which keys are
- * offered and exactly where each decrypted key is written.
+ * Only `age` is mocked, and its stand-in writes the output file the way age
+ * would: the copy and the chmod are now real fs calls, so the assertions are on
+ * what ends up on disk and at what mode rather than on which binaries were
+ * spawned.
  */
 
 import fs from 'node:fs';
@@ -25,6 +27,7 @@ describe('decryptKeys', () => {
   let sshDir: string;
   let vaultDir: string;
   let keyDir: string;
+  let tmpDir: string;
   let logSpy: ReturnType<typeof vi.spyOn>;
 
   const AGE_KEY = '/vault/age.key';
@@ -33,11 +36,18 @@ describe('decryptKeys', () => {
   const vaultKey = (name: string) => {
     const dir = path.join(keyDir, name);
     fs.mkdirSync(dir, { recursive: true });
-    fs.writeFileSync(path.join(dir, `id_${name}.age`), 'ENCRYPTED');
-    fs.writeFileSync(path.join(dir, `id_${name}.pub`), 'PUBLIC');
+    fs.writeFileSync(path.join(dir, `id_${name}.age`), `ENCRYPTED ${name}`);
+    fs.writeFileSync(path.join(dir, `id_${name}.pub`), `PUBLIC ${name}`);
   };
 
-  const choices = () => prompt.mock.calls.at(-1)?.[0][0].choices as string[];
+  /** Answers the selection prompt, then every overwrite confirmation. */
+  const answers = (selectedKeys: string[], decryptMode = LOCAL, overwrite = false) => {
+    prompt.mockImplementation(async (questions: { name: string }[]) =>
+      questions[0].name === 'selectedKeys' ? { selectedKeys, decryptMode } : { overwrite }
+    );
+  };
+
+  const choices = () => prompt.mock.calls[0]?.[0][0].choices as string[];
 
   const argsOf = (binary: string) =>
     execa.mock.calls.find((c) => c[0] === binary)?.[1] as string[] | undefined;
@@ -45,16 +55,26 @@ describe('decryptKeys', () => {
   const messages = (spy: ReturnType<typeof vi.spyOn>) =>
     spy.mock.calls.map((c) => c.join(' ')).join('\n');
 
+  const modeOf = (file: string) => fs.statSync(file).mode & 0o777;
+
   beforeEach(() => {
     vi.clearAllMocks();
     root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'keyman-decrypt-')));
     sshDir = path.join(root, '.ssh');
     vaultDir = path.join(root, 'vault');
     keyDir = path.join(vaultDir, 'keys');
+    tmpDir = path.join(vaultDir, 'tmp');
     fs.mkdirSync(keyDir, { recursive: true });
     fs.mkdirSync(sshDir, { recursive: true });
     logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
-    execa.mockResolvedValue({ exitCode: 0 });
+
+    // Stand in for `age -d`: write the plaintext to -o, 0644 as age does.
+    execa.mockImplementation(async (_binary: string, args: string[]) => {
+      const out = args[args.indexOf('-o') + 1];
+      fs.mkdirSync(path.dirname(out), { recursive: true });
+      fs.writeFileSync(out, 'PLAINTEXT', { mode: 0o644 });
+      return { exitCode: 0 };
+    });
   });
 
   afterEach(() => {
@@ -78,8 +98,10 @@ describe('decryptKeys', () => {
 
   it('reports a missing age binary rather than an ENOENT', async () => {
     vaultKey('prod');
-    prompt.mockResolvedValue({ selectedKeys: ['prod'], decryptMode: LOCAL });
-    execa.mockRejectedValue(Object.assign(new Error('spawn age ENOENT'), { code: 'ENOENT' }));
+    answers(['prod']);
+    execa.mockImplementation(async () => {
+      throw Object.assign(new Error('spawn age ENOENT'), { code: 'ENOENT' });
+    });
 
     await expect(decryptKeys(sshDir, vaultDir, AGE_KEY)).rejects.toThrow(
       '`age` was not found on PATH'
@@ -90,7 +112,7 @@ describe('decryptKeys', () => {
     vaultKey('prod');
     fs.mkdirSync(path.join(keyDir, 'empty'), { recursive: true });
     fs.writeFileSync(path.join(keyDir, 'README.md'), '');
-    prompt.mockResolvedValue({ selectedKeys: [], decryptMode: LOCAL });
+    answers([]);
 
     await decryptKeys(sshDir, vaultDir, AGE_KEY);
 
@@ -99,11 +121,11 @@ describe('decryptKeys', () => {
 
   it('decrypts into the vault tmp directory', async () => {
     vaultKey('prod');
-    prompt.mockResolvedValue({ selectedKeys: ['prod'], decryptMode: LOCAL });
+    answers(['prod']);
 
     await decryptKeys(sshDir, vaultDir, AGE_KEY);
 
-    const out = path.join(vaultDir, 'tmp', 'id_prod');
+    const out = path.join(tmpDir, 'id_prod');
     expect(argsOf('age')).toEqual([
       '-d',
       '-i',
@@ -112,40 +134,149 @@ describe('decryptKeys', () => {
       out,
       path.join(keyDir, 'prod', 'id_prod.age'),
     ]);
-    expect(argsOf('cp')).toEqual([path.join(keyDir, 'prod', 'id_prod.pub'), `${out}.pub`]);
-    expect(argsOf('chmod')).toEqual(['600', out]);
+    expect(fs.readFileSync(out, 'utf-8')).toBe('PLAINTEXT');
+    expect(fs.readFileSync(`${out}.pub`, 'utf-8')).toBe('PUBLIC prod');
     expect(messages(logSpy)).toContain(`Decrypted: ${out}`);
   });
 
   it('decrypts into the .ssh directory when asked', async () => {
     vaultKey('prod');
-    prompt.mockResolvedValue({ selectedKeys: ['prod'], decryptMode: SSH });
+    answers(['prod'], SSH);
 
     await decryptKeys(sshDir, vaultDir, AGE_KEY);
 
     const out = path.join(sshDir, 'id_prod');
     expect(argsOf('age')?.[4]).toBe(out);
-    expect(argsOf('cp')?.[1]).toBe(`${out}.pub`);
-    expect(argsOf('chmod')).toEqual(['600', out]);
+    expect(fs.readFileSync(`${out}.pub`, 'utf-8')).toBe('PUBLIC prod');
   });
 
-  it('decrypts every selected key', async () => {
+  it('creates the .ssh directory when it does not exist, private to the owner', async () => {
+    fs.rmSync(sshDir, { recursive: true });
     vaultKey('prod');
-    vaultKey('stage');
-    prompt.mockResolvedValue({ selectedKeys: ['prod', 'stage'], decryptMode: LOCAL });
+    answers(['prod'], SSH);
 
     await decryptKeys(sshDir, vaultDir, AGE_KEY);
 
-    // age, cp and chmod for each of the two keys.
-    expect(execa).toHaveBeenCalledTimes(6);
+    expect(modeOf(sshDir)).toBe(0o700);
+    expect(fs.existsSync(path.join(sshDir, 'id_prod'))).toBe(true);
+  });
+
+  it('leaves the private key at 0600, never observable at what age wrote', async () => {
+    vaultKey('prod');
+    answers(['prod']);
+
+    await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+    expect(modeOf(path.join(tmpDir, 'id_prod'))).toBe(0o600);
+  });
+
+  it('decrypts every selected key with one spawn each', async () => {
+    vaultKey('prod');
+    vaultKey('stage');
+    answers(['prod', 'stage']);
+
+    await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+    // One age per key: the cp and chmod spawns are gone.
+    expect(execa).toHaveBeenCalledTimes(2);
+    expect(execa.mock.calls.every((c) => c[0] === 'age')).toBe(true);
   });
 
   it('does nothing when the selection is empty', async () => {
     vaultKey('prod');
-    prompt.mockResolvedValue({ selectedKeys: [], decryptMode: LOCAL });
+    answers([]);
 
     await decryptKeys(sshDir, vaultDir, AGE_KEY);
 
     expect(execa).not.toHaveBeenCalled();
   });
+
+  it('writes the private key even when the vault entry has no public key', async () => {
+    vaultKey('prod');
+    fs.rmSync(path.join(keyDir, 'prod', 'id_prod.pub'));
+    answers(['prod']);
+
+    await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+    expect(fs.existsSync(path.join(tmpDir, 'id_prod'))).toBe(true);
+    expect(messages(logSpy)).toContain('has no public key in the vault');
+  });
+
+  describe('when the target already exists', () => {
+    const existing = (dir: string, name = 'id_prod') => {
+      fs.mkdirSync(dir, { recursive: true });
+      fs.writeFileSync(path.join(dir, name), 'PRECIOUS EXISTING KEY');
+      return path.join(dir, name);
+    };
+
+    it('keeps the existing key by default', async () => {
+      vaultKey('prod');
+      const target = existing(tmpDir);
+      answers(['prod']);
+
+      await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+      expect(fs.readFileSync(target, 'utf-8')).toBe('PRECIOUS EXISTING KEY');
+      expect(execa).not.toHaveBeenCalled();
+      expect(messages(logSpy)).toContain('Skipped prod');
+    });
+
+    it('asks before overwriting, defaulting to no', async () => {
+      vaultKey('prod');
+      existing(tmpDir);
+      answers(['prod']);
+
+      await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+      const confirm = prompt.mock.calls.at(-1)?.[0][0];
+      expect(confirm).toMatchObject({ type: 'confirm', default: false });
+      expect(confirm.message).toContain(path.join(tmpDir, 'id_prod'));
+    });
+
+    it('overwrites once confirmed', async () => {
+      vaultKey('prod');
+      const target = existing(tmpDir);
+      answers(['prod'], LOCAL, true);
+
+      await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+      expect(fs.readFileSync(target, 'utf-8')).toBe('PLAINTEXT');
+    });
+
+    it('asks about an existing public key too', async () => {
+      vaultKey('prod');
+      existing(tmpDir, 'id_prod.pub');
+      answers(['prod']);
+
+      await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+      expect(execa).not.toHaveBeenCalled();
+      expect(prompt.mock.calls.at(-1)?.[0][0].message).toContain('id_prod.pub');
+    });
+
+    it('protects a key in .ssh the same way', async () => {
+      vaultKey('prod');
+      const target = existing(sshDir);
+      answers(['prod'], SSH);
+
+      await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+      expect(fs.readFileSync(target, 'utf-8')).toBe('PRECIOUS EXISTING KEY');
+    });
+
+    it('settles every collision before decrypting anything', async () => {
+      vaultKey('prod');
+      vaultKey('stage');
+      existing(tmpDir);
+      answers(['prod', 'stage']);
+
+      await decryptKeys(sshDir, vaultDir, AGE_KEY);
+
+      // stage is written, prod is kept — and the question about prod was asked
+      // before either was touched.
+      expect(fs.existsSync(path.join(tmpDir, 'id_stage'))).toBe(true);
+      expect(fs.readFileSync(path.join(tmpDir, 'id_prod'), 'utf-8')).toBe('PRECIOUS EXISTING KEY');
+      expect(execa).toHaveBeenCalledTimes(1);
+    });
+  });
 });