[keyman] phase 3: derive the age recipient, and survive not having one
main.ts asserted the recipient non-null twice — extractAgePublicKey(...)! — and the type already said null was possible. With no age.key the vault encrypted to the string "null": execa stringifies it, age exits 1, and on the generate path that happens *after* ssh-keygen has written a plaintext private key into tmpDir, so the user is told the operation failed and left with a key on disk. Now the recipient is resolved once, remembered on success, and a null prints the remedy (age-keygen -o <path>) and returns to the menu. list, copy and decrypt still work without one. extractAgePublicKey now derives the public key with `age-keygen -y` instead of scraping the `# public key:` comment. The comment is ordinary text nothing re-checks; verified that rewriting it does not change what -y reports, so a stale or forged comment silently encrypted the vault to a recipient nobody holds the private half of. The comment survives as a fallback for a machine with no age-keygen, behind a warning that it is unverified — but not when age-keygen runs and refuses the file. That means age cannot read the identity, and trusting the comment there would encrypt to a recipient the vault could never decrypt with. runTool throws ToolNotFoundError for ENOENT so the two cases can be told apart. Its own tests move to tool.test.ts, which keeps real processes; utils.test.ts mocks execa, since the gate cannot require age installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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<typeof vi.spyOn>;
|
||||
let warnSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user