ansiblings/packages/keyman/docs/PLAN.md

keyman remediation plan

Turns AUDIT.md into sequenced work. Each phase is one commit, independently landable, gate-green on its own. Section references (§) are to AUDIT.md.

Ordering is by blast radius per unit of risk, not by severity: the error boundary comes first because it makes every later phase's failure mode legible, and the config threading comes late because it is the only phase that rewrites existing test assertions.

Verified before planning

Four things the fixes depend on, checked by running them rather than assumed — two of them changed the prescription:

Check	Result	Consequence
ssh-keygen with -N omitted	Prompts Enter passphrase … (empty for no passphrase) and confirms	§2.2 fix works: omit -N, inherit stdio, keyman never holds the passphrase
ssh-keygen -y -f <encrypted key>	Prompts for the passphrase	§1.6 fix cannot be a silent spawn — needs stdio: 'inherit' and a skip path
@inquirer/core from keyman	ERR_MODULE_NOT_FOUND — transitive via inquirer, not a direct dep	Detect ExitPromptError by error.name, never by import
z.strictObject in zod 4.4.3	Available; reports unrecognized_keys with a keys array	§3.5 has a hard-failure option, though the plan prefers a warning

What is not a breaking change

Per §4.3, src/index.ts exports only keyman, loadConfig, resolveConfigPaths and the update module. encryptKeys, decryptKeys, generateKey, listKeys, copyKey and extractAgePublicKey are not on the public surface, so every signature change below is internal. Phases 2–6 are not semver-breaking.

The one user-visible behaviour change is Phase 5 — see Migration.

Gate discipline

lint:ci → typecheck → test:coverage runs on pre-push and in CI. Two standing constraints:

• Every phase lands its tests with its fix. No phase may leave a red gate, so there is no "write the failing tests first" commit.
• keyman.cli.ts is excluded from coverage (vitest.config.ts:18). Per CLAUDE.md, adding logic to those files means moving it somewhere covered — which is why Phase 1 extracts argument parsing into a new module rather than growing cli.ts.

Per-phase verification is pnpm --filter @bitsquare/keyman run test; the full gate (pnpm run lint:ci && pnpm run typecheck && pnpm run test:coverage) before each push.

---

Phase 1 — Error boundary, --help, argument validation

Closes §3.1, §3.2, and the second half of §1.2 (the crash dump).

First because it is pure addition, touches no operation module, and converts every latent throw in phases 2–6 from a stack dump into a line of text. The ExitPromptError half is independently worth shipping: today Ctrl-C at any prompt produces a crash dump.

New file src/keyman.args.ts (covered by the gate, unlike cli.ts):

• parseArgs(argv: string[]): ParsedArgs — supports --flag value and --flag=value, rejects a flag consumed as another flag's value, rejects unknown flags, and validates --channel against 'latest' | 'next' | 'main' so §3.2's false "Could not reach " cannot happen.
• helpText(): string — flags, both subcommands, and the four KEYMAN_* variables. This is the text Phase 9 keeps in step with the README.

src/keyman.cli.ts stays wiring: dispatch on the parse result, and

try {
  await keyman();
} catch (error) {
  if ((error as { name?: string }).name === 'ExitPromptError') {
    console.log('\n👋 Goodbye!\n');
    process.exit(0);
  }
  console.error(`❌ ${error instanceof Error ? error.message : error}`);
  process.exit(1);
}

error.name, not instanceof — @inquirer/core is not a direct dependency and does not resolve from this package.

Tests — new tests/args.test.ts: each rejection, both flag forms, the channel whitelist, and that helpText() names every flag parseArgs accepts (so the two cannot drift).

Done when keyman --help prints usage and exits 0 without loading config or prompting; keyman --bogus errors; Ctrl-C prints Goodbye and exits 0.

---

Phase 2 — Guards and error handling in encrypt/decrypt

Closes §1.2 (first half), §1.5, and §1.10's statSync.

• encrypt.ts:13,16 and decrypt.ts:8 — existsSync guard, falling through to the "⚠️ No …" message each function already has but cannot currently reach.
• main.ts:40-41 — create keysDir alongside vaultRoot and tmpDir. Use {recursive: true, mode: 0o700} now, so Phase 4 does not have to revisit it.
• Wrap the age spawns in both functions. An ENOENT on the binary gets its own message ("age was not found on PATH") — it is the one hard external requirement and currently the least legible failure.
• list.ts:80 — readdirSync(dir, {withFileTypes: true}) instead of statSync per entry, which also drops N stat calls and fixes the broken-symlink throw.
• Delete the debug logging while in these files: encrypt.ts:18-19 and decrypt.ts:10 (§1.10). That also closes DOCS-AUDIT.md §6.4's open bullet.

Tests — the cases the current suite structurally cannot have, because every beforeEach pre-creates the directories: encrypt with no ~/.ssh, encrypt with no tmp, decrypt with no <vault>/keys, each asserting the warning and no throw. Plus list with a dangling symlink in the keys directory.

Note on coverage. encrypt.ts and decrypt.ts are at 100 % lines and branches today. The number will not move; the tests are the point.

Done when a first run against an empty vault can reach every menu entry and return to the menu.

---

Phase 3 — Resolve the age recipient once, and derive it properly

Closes §1.3 and §2.3, and makes CLAUDE.md's age-keygen claim true (§5.5).

Two changes that belong together because both are about the recipient:

1. utils.ts — derive, don't scrape. extractAgePublicKey currently regexes # public key: out of a comment (utils.ts:16) and trusts it. Replace with age-keygen -y <keyPath>, which derives the public key from the private key and cannot disagree with it. Keep the comment parse as a fallback for when age-keygen is absent, behind a warning that the recipient is unverified. The function becomes async.
2. main.ts:73,80 — delete both !. Resolve the recipient once before the switch, and treat null as recoverable: print the remedy (age-keygen -o <keyPath>) and break back to the menu. This is the whole of §1.3 — the type already said null was possible.

Sequencing matters inside the phase: fix the call site first. Without it, a missing key file still reaches age -r null, and the generate path still leaves a plaintext private key in tmpDir after telling the user the operation failed.

Tests — utils.test.ts gains the age-keygen -y path with execa mocked, the fallback-with-warning path, and the both-unavailable path. main.test.ts gains: missing recipient → neither generateKey nor encryptKeys is called, a remedy is printed, and the menu loop continues.

Done when keyman against a vault with no age.key reaches the menu, refuses generate and encrypt with a remedy, and still offers list and decrypt.

---

Phase 4 — Decrypt: stop overwriting, stop the 0644 window

Closes §1.4 and §2.1. The highest-value phase — §1.4 is the only finding that destroys data the user did not ask to touch.

• Collision check before any decryption. Both output paths, both modes. Prompt per collision, defaulting to skip; ~/.ssh deserves the friction more than vault/tmp does, but the check is the same code.
• Replace the shell-outs (decrypt.ts:49-50) with fs.copyFileSync and fs.chmodSync. Three spawns per key become one, it works on Windows, and it removes a cp that overwrites unconditionally.
• Close the permission window. Verified: age -o creates the file 0644 and mkdirSync creates vault/tmp as 0755, so a plaintext key is world-readable for the duration of two process spawns — and stays 0644 if the chmod fails. fs.chmodSync immediately after age resolves; mode: 0o700 on the directory (already done in Phase 2); create ~/.ssh 0700 if absent.

Test rework — the fiddliest in the plan. decrypt.test.ts asserts on the mocked spawns: argsOf('cp') (:98,111) and argsOf('chmod') (:99,112) both disappear, and execa is mocked with a bare mockResolvedValue (:57) that writes no output file. Once copyFileSync is real it needs a real file, so the mock must write to its -o argument the way encrypt.test.ts:51-54 already does. Assert the on-disk result and mode instead of the argv — a better test than the one it replaces, since it checks the outcome rather than the mechanism. execa call counts also change (:123: six spawns → two).

Done when decrypting onto an existing key requires a confirmation, and the decrypted key is never observable at anything but 0600.

---

Phase 5 — Thread keysDir and tmpDir through encrypt and decrypt

Closes §1.1 and the DOCS-AUDIT.md entry in §5.1.

Mechanical, but it is the one phase that rewrites assertions that pass today, so it stays its own commit with nothing else in it.

• encryptKeys(sshDir, keysDir, tmpDir, pubkey) — drop vaultDir, delete the hardcoded path.join(vaultDir, 'keys') (encrypt.ts:38).
• decryptKeys(sshDir, keysDir, tmpDir, ageKey) — drop vaultDir, delete the hardcoded joins at decrypt.ts:7,39,43.
• main.ts:76,84 — pass paths.keysDir and paths.tmpDir. Neither function has any remaining use for vaultRoot, so the parameter goes rather than becoming a second source of truth.

Assertions to change — all four, named so the diff is reviewable:

Location	Today	After
main.test.ts:164-175	"encrypts keys into the vault root", asserts paths.vaultRoot	asserts paths.keysDir, paths.tmpDir
main.test.ts:177-187	asserts paths.vaultRoot	asserts paths.keysDir, paths.tmpDir
encrypt.test.ts:95,115,128-129	path.join(vaultDir, 'keys', …)	path.join(keysDir, …)
decrypt.test.ts:53,89	keyDir = path.join(vaultDir, 'keys')	keysDir passed in directly

New test, the one that would have caught this: a config with keysDir: 'encrypted' and tmpDir: 'plain', encrypt a key, then list it, and assert the listing shows it in [Vault]. That round trip fails today and is the regression worth owning.

Also in this commit: move the DOCS-AUDIT.md:826-827 bullet out of checked and accurate and point it at this finding. It was verified against keyman.encrypt.ts — the file that ignores the config — which is precisely how §1.1 stayed invisible.

---

Phase 6 — Generate: passphrase off argv, and .pub recovery

Closes §2.2, §1.6, and §1.10's leftover-directory bullet.

Passphrase (§2.2). Verified: omitting -N makes ssh-keygen prompt and confirm. So delete keyman's own password prompt (generate.ts:26-33), omit -N, and spawn with stdio: 'inherit'. The passphrase never enters keyman's memory and never reaches argv — strictly better than routing it more carefully, and it deletes code. generate.test.ts:78-87 loses -N/'pw' from the expected argv and the password-prompt case goes away.

Missing .pub (§1.6). The selection list is built from private keys only, so an orphan private key is offered and copyFileSync throws after age has written the .age file — leaving a vault entry with no public key and killing the rest of the batch. Fix in two parts:

• Derive it with ssh-keygen -y -f <key> when the sibling is absent. Verified that this prompts for a passphrase on an encrypted key, so it needs stdio: 'inherit' and a clean skip when the user cannot or will not supply it — not a silent spawn whose stdout is captured.
• Wrap the loop body in encrypt.ts:36-48 per key, so one bad key costs one key. Report the failures at the end rather than dying at the first.

Leftover directory. generate.ts:65 creates <keysDir>/<name>/ before age runs at :68. Move the mkdirSync after age succeeds.

---

Phase 7 — Config: warn on typos, decide on the dead machinery

Closes §3.5, §3.4, and asks for a decision on §3.3.

Typos (§3.5). Verified: {"vaultroot": "…"} is silently stripped by z.object. Warn per file rather than failing — diff Object.keys(rawConfig) against the schema keys plus resolution inside the existing per-file loop (config.ts:210-227), where the filename is in hand. That names the offending file, which z.strictObject cannot do from the merged result, and it preserves the module's documented posture of degrading to defaults rather than throwing. (z.strictObject is available in zod 4.4.3 and reports unrecognized_keys with a keys array, if a hard failure is preferred later.)

getConfigPaths (§3.4). Add it to the --print-config JSON as configFiles. The function is currently exercised only by its own test, and --print-config currently cannot answer which files were read — that exists only as unstructured stderr from loadConfig. One change fixes both.

Decision needed — the resolution machinery (§3.3). Roughly 45 lines (config.ts:23-30,115-157) that cannot affect a valid config, because every schema property is a string and both strategies return childValue for primitives. config.test.ts:212 "honours an explicit override strategy" passes either way.

• Recommended: delete it, along with the doc comment at :186-194 that advertises it. keyman.config.ts goes from 267 lines to roughly 220, and the config file stops documenting a knob that does nothing.
• Alternative: keep it as the shape a future array- or object-valued option would need — but then say so in a comment, because today it reads as functional, and make config.test.ts:212 assert something that distinguishes the two strategies (which requires a non-string property to exist first).

Deleting is the smaller lie. It also diverges from nopy, where the machinery is load-bearing — worth a line in CLAUDE.md so the divergence reads as deliberate.

---

Phase 8 — Portability and the gaps that make keys invisible

Closes §1.7, §1.8, §1.9, §2.4. Independent of each other; split if any grows.

• Clipboard (§1.9). pbcopy / wl-copy / xclip / clip.exe by platform, falling back to printing the key to stdout so the operation is never a dead end. Delete the comment at copy.ts:46-48 that admits the shortcut.
• Home directory (§1.7). os.userInfo() for the current user; for a named user, look the home directory up rather than assuming /home/<user> — wrong on the one platform the tool currently supports. Check existsSync and say so, instead of feeding a nonexistent path into a readdir. main.test.ts:198-204 changes.
• Non-id_* keys (§1.8). Relax the filters (copy.ts:9, encrypt.ts:14,17, list.ts:23,51) to any private key with a recognisable header, or at minimum print a count of the keys that were skipped and why. Today a key named deploy_ed25519 is simply absent from the menu — and pre-existing keys are the population a key manager is adopted to take over. decrypt.ts:9 reconstructs id_${dir} from the folder name, so the vault layout has the assumption baked in; relaxing discovery means storing the real filename per key, which is the largest single item in this plan. Size it before committing to it — a skipped-key count is a tenth of the work and closes most of the surprise.
• Plaintext hygiene (§2.4). A "🧹 Clear decrypted keys" menu entry, and write a .gitignore next to the vault on first run covering age.key and tmp/ — which README.md:25-26 currently tells the user to do by hand. This is the cheapest guard against the exact failure the tool exists to prevent.

---

Phase 9 — Documentation

Closes §5.2, §5.3, §5.4, §5.5. Last, so it documents what the code now does.

• README.md — the only shipped document (package.json:37-41 ships dist, README.md, LICENSE). Currently describes four of six menu entries, invents key rotation, and mentions none of self-update, --print-config, --version, --help, or the four KEYMAN_* variables. README.PUBLISH.md has all of it and never reaches a reader on the registry. Reuse Phase 1's helpText() as the source for the CLI section so the two cannot drift.
• README.md:46-86 — the configuration and vault-layout sections, now that Phase 5 makes keysDir/tmpDir real, plus the migration note below.
• README.md:11 — drop "Support for key rotation" unless Phase 10 lands first.
• README.md:33 — stop telling the user to shell out to ssh-keygen; the Generate operation exists.
• CLAUDE.md — age-keygen becomes true in Phase 3; note that cp/chmod are gone (Phase 4) and record the resolution divergence from nopy (Phase 7).
• AUDIT.md — mark findings closed, keeping their text as the record, the way DOCS-AUDIT.md does.

---

Phase 10 — Key rotation (decision required)

§3.6. README.md:11 has advertised it since before this audit; grep -rn "rotat" packages/keyman/src/ returns nothing.

Unlike the rest of this plan it is a feature, not a repair, and it is the one item that could reasonably be dropped instead. Recommendation: build it — rotation is the operation that makes a key vault worth having, and the pieces all exist by Phase 6 (generate under a new name, encrypt, keep the old key until the replacement is deployed, then shred). Sketch:

1. Pick an existing vault key.
2. Generate a replacement into tmpDir under a versioned name.
3. Encrypt it alongside the current one — never replacing it.
4. Report both public keys, so the new one can be deployed before the old one goes.
5. A separate "retire" step that removes the superseded key once the user confirms.

Steps 3 and 4 are the whole value: a rotation that atomically replaces the key is a rotation that locks you out of the host you were rotating for. If this is deferred, delete the README claim in Phase 9 instead.

---

Migration

Phase 5 is the only user-visible change. Anyone with a custom keysDir or tmpDir currently has a split vault — generate and list on the configured directory, encrypt and decrypt on <vaultRoot>/keys and <vaultRoot>/tmp. After Phase 5 all five agree on the configured directory, so anything written by encrypt before the upgrade needs moving:

mv <vaultRoot>/keys/* <vaultRoot>/<keysDir>/

Nobody on the defaults is affected, since the two halves coincide there. The README gets this as a note, and it is worth a line in the release notes for whichever version carries Phase 5.

Rollout

Per CLAUDE.md: bump packages/keyman/package.json, land on main, then tag keyman-v<version>.

• Snapshots come free. Every push to main publishes <version>-main.<run>.g<sha> to Gitea under the main dist-tag, so each phase is installable for testing without a release. pnpm run try:snapshot installs one into a throwaway project.
• Suggested cut points. After Phase 4 as 0.6.0 — error boundary, guards, recipient handling and the data-loss fix, which is the set worth getting to users first. After Phase 9 as 0.7.0, carrying the Phase 5 migration note.
• keyman can reach npmjs. It has no workspace:* dependencies (execa, inquirer, semver, zod only), so scripts/linked-deps.mjs has nothing to block on — unlike nopy, which CLAUDE.md records as gated behind nopy-cubes shipping. keyman has never been published to npmjs; 0.6.0 could be the first, and versions being 0.x.y rather than 1.0.0-alphaN means the latest dist-tag will now actually move.
• pnpm publish, never npm publish — no workspace: ranges here, but the rule is repo-wide and scripts/verify-pack.mjs enforces it in both workflows.

Sequencing at a glance

1  cli boundary + --help + args      §3.1 §3.2 §1.2(half)   isolated, pure addition
2  guards in encrypt/decrypt/list    §1.2 §1.5 §1.10         new tests only
3  age recipient, once and derived   §1.3 §2.3 §5.5          signature → async
4  decrypt: no clobber, no 0644      §1.4 §2.1               reworks decrypt.test.ts
   ── cut 0.6.0 ──
5  thread keysDir/tmpDir             §1.1 §5.1               rewrites 4 assertions
6  generate: -N gone, .pub recovery  §2.2 §1.6 §1.10         reworks generate.test.ts
7  config: warn, prune, print        §3.5 §3.4 §3.3*         *decision
8  portability + hygiene             §1.7 §1.8 §1.9 §2.4     §1.8 needs sizing
9  documentation                     §5.2 §5.3 §5.4 §5.5     README is the shipped one
   ── cut 0.7.0 ──
10 rotation                          §3.6*                   *decision: build or delete

Phases 1–6 are repairs and want to land in order. 7 and 8 are independent of each other and of 5–6. 9 depends on everything before it. 10 is optional and gates one line of Phase 9.

Open decisions

Neither blocks Phase 1. Both change scope where they land:

1. §3.3, at Phase 7 — delete the inert resolution machinery (recommended, −45 lines) or keep it as future shape with a comment saying so.
2. §3.6, at Phase 10 — build rotation (recommended) or delete the README claim in Phase 9.