diff --git a/packages/keyman/src/keyman.main.ts b/packages/keyman/src/keyman.main.ts index 0b4d38a..23a7752 100644 --- a/packages/keyman/src/keyman.main.ts +++ b/packages/keyman/src/keyman.main.ts @@ -44,6 +44,18 @@ export async function keyman() { fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); } + // Resolved on demand, because only generate and encrypt need a recipient, and + // remembered once it succeeds. Retried while it has not: creating the identity + // mid-session should not mean restarting. + let recipient: string | null = null; + const ageRecipient = async () => { + recipient ??= await extractAgePublicKey(paths.keyPath); + if (!recipient) { + console.error(` Create one with: age-keygen -o ${paths.keyPath}`); + } + return recipient; + }; + // Main loop - keep showing menu until user quits let running = true; while (running) { @@ -73,17 +85,20 @@ export async function keyman() { case 'copy': await copyKey(sshDir, paths.tmpDir); break; - case 'generate': - await generateKey(paths.tmpDir, paths.keysDir, extractAgePublicKey(paths.keyPath)!); + case 'generate': { + const pubkey = await ageRecipient(); + if (pubkey) { + await generateKey(paths.tmpDir, paths.keysDir, pubkey); + } break; - case 'encrypt': - await encryptKeys( - sshDir, - paths.vaultRoot, - paths.tmpDir, - extractAgePublicKey(paths.keyPath)! - ); + } + case 'encrypt': { + const pubkey = await ageRecipient(); + if (pubkey) { + await encryptKeys(sshDir, paths.vaultRoot, paths.tmpDir, pubkey); + } break; + } case 'decrypt': await decryptKeys(sshDir, paths.vaultRoot, paths.keyPath); break; diff --git a/packages/keyman/src/keyman.utils.ts b/packages/keyman/src/keyman.utils.ts index 0b14c80..5ff8b47 100644 --- a/packages/keyman/src/keyman.utils.ts +++ b/packages/keyman/src/keyman.utils.ts @@ -1,6 +1,14 @@ import fs from 'node:fs'; import { execa, type Options } from 'execa'; +/** A binary keyman needs is not installed — recoverable, unlike a tool refusing */ +export class ToolNotFoundError extends Error { + constructor(readonly binary: string) { + super(`\`${binary}\` was not found on PATH. Install it and try again.`); + this.name = 'ToolNotFoundError'; + } +} + /** * Runs one of the external binaries keyman depends on. * @@ -27,23 +35,57 @@ export async function runTool( } catch (error) { const failure = error as { code?: string; stderr?: string; shortMessage?: string }; if (failure.code === 'ENOENT') { - throw new Error(`\`${binary}\` was not found on PATH. Install it and try again.`); + throw new ToolNotFoundError(binary); } throw new Error(`\`${binary}\` failed: ${failure.stderr?.trim() || failure.shortMessage}`); } } /** - * Extracts the public key from an age key file. - * @param keyFilePath Path to the age key file. - * @returns The public key as a string, or null if not found. + * The age recipient a vault encrypts to, derived from its identity file. + * + * `age-keygen -y` derives the public key from the secret key, so it cannot + * disagree with it. The `# public key:` comment can: it is ordinary text that + * nothing re-checks, and a wrong one encrypts the vault to a recipient nobody + * holds the private half of. Verified — rewriting the comment does not change + * what `-y` reports. + * + * The comment stays as a fallback for a machine with no `age-keygen`, behind a + * warning that it is unverified. It is *not* a fallback for `age-keygen` + * refusing the file: that means age cannot read the identity, and trusting the + * comment then would encrypt to a recipient the vault could never decrypt with. + * + * @returns the recipient, or null with the reason already reported */ -export function extractAgePublicKey(keyFilePath: string): string | null { +export async function extractAgePublicKey(keyFilePath: string): Promise { if (!fs.existsSync(keyFilePath)) { console.error(`❌ ERROR: Age key file not found at ${keyFilePath}`); return null; } + try { + const { stdout } = await runTool('age-keygen', ['-y', keyFilePath]); + const derived = stdout.trim(); + if (derived.startsWith('age1')) { + return derived; + } + console.error(`❌ ERROR: age-keygen derived no public key from ${keyFilePath}`); + return null; + } catch (error) { + if (!(error instanceof ToolNotFoundError)) { + console.error(`❌ ERROR: ${error instanceof Error ? error.message : error}`); + return null; + } + console.warn( + `⚠️ age-keygen is not installed — reading the public key from the comment in ${keyFilePath}, unverified against the secret key.` + ); + } + + return publicKeyFromComment(keyFilePath); +} + +/** The `# public key:` line: a claim about the key rather than a derivation from it */ +function publicKeyFromComment(keyFilePath: string): string | null { try { const fileContents = fs.readFileSync(keyFilePath, 'utf-8'); const publicKeyMatch = fileContents.match(/^# public key:\s*(age1[^\s]+)/m); diff --git a/packages/keyman/tests/main.test.ts b/packages/keyman/tests/main.test.ts index 2519bad..ed01f6c 100644 --- a/packages/keyman/tests/main.test.ts +++ b/packages/keyman/tests/main.test.ts @@ -81,7 +81,7 @@ describe('keyman', () => { ageKeyFile: 'age.key', }); resolveConfigPaths.mockReturnValue(paths); - extractAgePublicKey.mockReturnValue('age1recipient'); + extractAgePublicKey.mockResolvedValue('age1recipient'); logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); @@ -197,6 +197,55 @@ describe('keyman', () => { ); }); + describe('without an age recipient', () => { + beforeEach(() => { + extractAgePublicKey.mockResolvedValue(null); + }); + + it.each([ + ['generate', generateKey], + ['encrypt', encryptKeys], + ])('refuses %s with a remedy instead of passing null to age', async (choice, operation) => { + menu([choice]); + + await keyman(); + + expect(operation).not.toHaveBeenCalled(); + const reported = errorSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + expect(reported).toContain(`age-keygen -o ${paths.keyPath}`); + // The whole point: the loop survives and quit is still reached. + expect(output()).toContain('Goodbye!'); + }); + + it('still allows the operations that need no recipient', async () => { + menu(['list', 'decrypt']); + + await keyman(); + + expect(listKeys).toHaveBeenCalled(); + expect(decryptKeys).toHaveBeenCalled(); + }); + + it('retries the lookup, so creating the identity mid-session works', async () => { + extractAgePublicKey.mockResolvedValueOnce(null).mockResolvedValueOnce('age1later'); + menu(['generate', 'generate']); + + await keyman(); + + expect(extractAgePublicKey).toHaveBeenCalledTimes(2); + expect(generateKey).toHaveBeenCalledTimes(1); + expect(generateKey).toHaveBeenCalledWith(paths.tmpDir, paths.keysDir, 'age1later'); + }); + }); + + it('resolves the recipient once for repeated operations', async () => { + menu(['generate', 'encrypt']); + + await keyman(); + + expect(extractAgePublicKey).toHaveBeenCalledTimes(1); + }); + it('keeps showing the menu until the user quits', async () => { menu(['list', 'copy', 'list']); diff --git a/packages/keyman/tests/tool.test.ts b/packages/keyman/tests/tool.test.ts new file mode 100644 index 0000000..c1318ca --- /dev/null +++ b/packages/keyman/tests/tool.test.ts @@ -0,0 +1,56 @@ +/** + * Tests for runTool. + * + * These spawn real processes rather than mocking execa. What runTool exists for + * is the shape of an execa failure — a mock would assert only what this test + * already assumes. It lives apart from utils.test.ts, which mocks execa to test + * the callers. + */ + +import { describe, expect, it } from 'vitest'; +import { runTool, ToolNotFoundError } from '../src/keyman.utils.js'; + +describe('runTool', () => { + it('returns stdout on success', async () => { + const result = await runTool('node', ['-e', 'process.stdout.write("hi")']); + + expect(result.stdout).toBe('hi'); + }); + + it('passes options through', async () => { + const result = await runTool('node', ['-e', 'process.stdout.write(process.env.PROBE ?? "")'], { + env: { PROBE: 'from-options' }, + }); + + expect(result.stdout).toBe('from-options'); + }); + + it('reports empty stdout when the output went elsewhere', async () => { + const result = await runTool('node', ['-e', 'process.stdout.write("hi")'], { + stdout: 'ignore', + }); + + expect(result.stdout).toBe(''); + }); + + it('turns a missing binary into an instruction rather than an ENOENT', async () => { + const failure = runTool('keyman-no-such-binary', []); + + await expect(failure).rejects.toThrow(ToolNotFoundError); + await expect(failure).rejects.toThrow( + '`keyman-no-such-binary` was not found on PATH. Install it and try again.' + ); + }); + + it('surfaces what the binary wrote to stderr', async () => { + await expect( + runTool('node', ['-e', 'process.stderr.write("no recipient\\n"); process.exit(1)']) + ).rejects.toThrow('`node` failed: no recipient'); + }); + + it('falls back to the command summary when stderr is empty', async () => { + await expect(runTool('node', ['-e', 'process.exit(3)'])).rejects.toThrow( + /`node` failed: .*exit code 3/ + ); + }); +}); diff --git a/packages/keyman/tests/utils.test.ts b/packages/keyman/tests/utils.test.ts index 14dff26..3fe201a 100644 --- a/packages/keyman/tests/utils.test.ts +++ b/packages/keyman/tests/utils.test.ts @@ -1,19 +1,30 @@ /** * Tests for extractAgePublicKey. * - * Runs against real files in a temp directory: the function is a thin wrapper - * around fs plus a regex, and faking fs would only test the fake. + * Real files in a temp directory, but a mocked execa: the recipient is now + * derived by spawning `age-keygen -y`, and the gate cannot depend on age being + * installed on the machine running it. runTool itself is tested against real + * processes in tool.test.ts. */ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { extractAgePublicKey, runTool } from '../src/keyman.utils.js'; + +const { execa } = vi.hoisted(() => ({ execa: vi.fn() })); + +vi.mock('execa', () => ({ execa })); + +import { extractAgePublicKey } from '../src/keyman.utils.js'; + +const DERIVED = 'age1derivedfromthesecretkey'; +const IN_COMMENT = 'age1fromthecomment'; describe('extractAgePublicKey', () => { let tmpDir: string; let errorSpy: ReturnType; + let warnSpy: ReturnType; const keyFile = (contents: string) => { const file = path.join(tmpDir, 'age.key'); @@ -21,9 +32,33 @@ describe('extractAgePublicKey', () => { return file; }; + /** A well-formed identity file, whose comment can be made to disagree */ + const identity = (comment = DERIVED) => + keyFile( + ['# created: 2026-01-01T00:00:00Z', `# public key: ${comment}`, 'AGE-SECRET-KEY-1QQQ'].join( + '\n' + ) + ); + + /** + * Makes age-keygen unavailable, the one case that falls back to the comment. + * + * Throws from an implementation rather than using mockRejectedValue: that + * builds its rejected promise when the mock is configured, and configuring it + * in a beforeEach leaves the rejection unhandled for a tick. + */ + const noAgeKeygen = () => { + execa.mockImplementation(async () => { + throw Object.assign(new Error('spawn age-keygen ENOENT'), { code: 'ENOENT' }); + }); + }; + beforeEach(() => { + vi.clearAllMocks(); tmpDir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'keyman-utils-'))); errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + execa.mockResolvedValue({ stdout: `${DERIVED}\n` }); }); afterEach(() => { @@ -31,88 +66,82 @@ describe('extractAgePublicKey', () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); - it('returns the public key from a standard age key file', () => { - const file = keyFile( - [ - '# created: 2026-01-01T00:00:00Z', - '# public key: age1abc123xyz', - 'AGE-SECRET-KEY-1QQQ', - ].join('\n') - ); + it('derives the recipient from the secret key with age-keygen', async () => { + const file = identity(); - expect(extractAgePublicKey(file)).toBe('age1abc123xyz'); + await expect(extractAgePublicKey(file)).resolves.toBe(DERIVED); + expect(execa).toHaveBeenCalledWith('age-keygen', ['-y', file]); + expect(warnSpy).not.toHaveBeenCalled(); }); - it('tolerates extra whitespace after the label', () => { - const file = keyFile('# public key: age1spaced\n'); + it('prefers the derived key over a comment that disagrees', async () => { + // §2.3: the comment is editable text, and this is what makes it not matter. + const file = identity('age1staleorforged'); - expect(extractAgePublicKey(file)).toBe('age1spaced'); + await expect(extractAgePublicKey(file)).resolves.toBe(DERIVED); }); - it('returns null and reports when the file does not exist', () => { + it('returns null and reports when the file does not exist', async () => { const missing = path.join(tmpDir, 'nope.key'); - expect(extractAgePublicKey(missing)).toBeNull(); + await expect(extractAgePublicKey(missing)).resolves.toBeNull(); expect(errorSpy.mock.calls[0][0]).toContain('Age key file not found'); + expect(execa).not.toHaveBeenCalled(); }); - it('returns null when the file has no public key line', () => { - const file = keyFile('AGE-SECRET-KEY-1QQQ\n'); + it('returns null when age-keygen refuses the file, without trusting the comment', async () => { + const file = identity(IN_COMMENT); + execa.mockRejectedValue( + Object.assign(new Error('failed'), { exitCode: 1, stderr: 'age-keygen: error: malformed' }) + ); - expect(extractAgePublicKey(file)).toBeNull(); - expect(errorSpy).not.toHaveBeenCalled(); + await expect(extractAgePublicKey(file)).resolves.toBeNull(); + expect(errorSpy.mock.calls[0][0]).toContain('malformed'); }); - it('ignores a key that is not on its own line', () => { - const file = keyFile('prefix # public key: age1inline\n'); + it('returns null when age-keygen prints something that is not a recipient', async () => { + const file = identity(); + execa.mockResolvedValue({ stdout: 'Public key: (none)\n' }); - expect(extractAgePublicKey(file)).toBeNull(); + await expect(extractAgePublicKey(file)).resolves.toBeNull(); + expect(errorSpy.mock.calls[0][0]).toContain('derived no public key'); }); - it('returns null and reports when the file cannot be read', () => { - const asDirectory = path.join(tmpDir, 'age.key'); - fs.mkdirSync(asDirectory); + describe('without age-keygen installed', () => { + beforeEach(noAgeKeygen); - expect(extractAgePublicKey(asDirectory)).toBeNull(); - expect(errorSpy.mock.calls[0][0]).toContain('Failed to read key file'); - }); -}); + it('falls back to the comment, warning that it is unverified', async () => { + const file = identity(IN_COMMENT); -/** - * These spawn real processes rather than mocking execa. The whole point of - * runTool is the shape of an execa failure, and a mock would only assert what - * this test already assumes. - */ -describe('runTool', () => { - it('returns the result on success', async () => { - const result = await runTool('node', ['-e', 'process.stdout.write("hi")']); - - expect(result.stdout).toBe('hi'); - }); - - it('passes options through', async () => { - const result = await runTool('node', ['-e', 'process.stdout.write(process.env.PROBE ?? "")'], { - env: { PROBE: 'from-options' }, + await expect(extractAgePublicKey(file)).resolves.toBe(IN_COMMENT); + expect(warnSpy.mock.calls[0][0]).toContain('unverified'); }); - expect(result.stdout).toBe('from-options'); - }); + it('tolerates extra whitespace after the label', async () => { + const file = keyFile('# public key: age1spaced\n'); - it('reports a missing binary as an instruction rather than an ENOENT', async () => { - await expect(runTool('keyman-no-such-binary', [])).rejects.toThrow( - '`keyman-no-such-binary` was not found on PATH. Install it and try again.' - ); - }); + await expect(extractAgePublicKey(file)).resolves.toBe('age1spaced'); + }); - it('surfaces what the binary wrote to stderr', async () => { - await expect( - runTool('node', ['-e', 'process.stderr.write("no recipient\\n"); process.exit(1)']) - ).rejects.toThrow('`node` failed: no recipient'); - }); + it('returns null when the file has no public key line', async () => { + const file = keyFile('AGE-SECRET-KEY-1QQQ\n'); - it('falls back to the command summary when stderr is empty', async () => { - await expect(runTool('node', ['-e', 'process.exit(3)'])).rejects.toThrow( - /`node` failed: .*exit code 3/ - ); + await expect(extractAgePublicKey(file)).resolves.toBeNull(); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('ignores a key that is not on its own line', async () => { + const file = keyFile('prefix # public key: age1inline\n'); + + await expect(extractAgePublicKey(file)).resolves.toBeNull(); + }); + + it('returns null and reports when the file cannot be read', async () => { + const asDirectory = path.join(tmpDir, 'age.key'); + fs.mkdirSync(asDirectory); + + await expect(extractAgePublicKey(asDirectory)).resolves.toBeNull(); + expect(errorSpy.mock.calls[0][0]).toContain('Failed to read key file'); + }); }); });