diff --git a/docs/env.md b/docs/env.md index 9821cab57..0a0c0585c 100644 --- a/docs/env.md +++ b/docs/env.md @@ -1677,7 +1677,20 @@ Some older surfaces are still compatibility-oriented: before it lands, so a concurrent writer's key is kept, never overwritten. The process-wide key cache holds only keychain material, so one store's key file never stands in for another store's key. `hack doctor` reports an unreadable or refused - store, a missing or refused store key, and a key that does not open the store. Explicit key + store, a missing or refused store key, and a key that does not open the store. + A store whose key is missing or wrong is an error only in a project that reads + it (its legacy env contract declares a store-backed value, or plaintext is kept + in the backend), and a warning elsewhere. The finding names the registered + projects that read the store, and offers two ways forward: restore the key, or + run `hack secrets retire-store`. That command moves the store into a new + directory `.retired-/` beside it, deletes and prints nothing, and says + how to move it back. It needs `--yes` without a terminal. It refuses a store that + another registered project's key still opens, and, unless `--allow-unchecked` is + passed, a store some registered projects could not be checked for. Because + `HACK_SECRETS_FILE_KEY` overrides key files and may be set in another shell for a + project that selects the store, the command cannot rule it out and also needs + `--allow-unchecked` as confirmation that none does. Doctor then lists + retired files as information. Explicit key provisioning also uses an atomic mode-`0600` replacement and refuses symlinked key paths or immediate parent directories. A key generated on first use is published only when no key file exists, so concurrent first users adopt the key diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 818c116ab..46880255c 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -2338,6 +2338,7 @@ hack secrets [options] | `hack secrets set [name]` | Store a secret | | `hack secrets get [name]` | Print a secret (exit 1 if missing) | | `hack secrets delete [name]` | Delete a stored secret | +| `hack secrets retire-store` | Move an encrypted secret store whose key is gone aside (reversible) | ### Options @@ -2427,6 +2428,37 @@ the deletion is refused with E_INTERACTIVE_REQUIRED unless --yes is passed. | `--help, -h` | Show help | | `--version, -v` | Show version | +## `hack secrets retire-store` + +Move an encrypted secret store whose key is gone aside (reversible) + +### Usage + +```bash +hack secrets retire-store [options] +``` + +For an encrypted_file (or cloud shim) store that hack doctor reports with a missing or wrong key. +Moves the store into a new directory .retired-/ beside it: nothing is deleted, read or printed, +and the key file is left alone. It prints how to put the store back. The store is the one this project +selects (--path or the current directory), or the global one outside a project. Refused for a store that +opens with the key in use, that another registered project's key still opens, or that is unreadable for +another reason. HACK_SECRETS_FILE_KEY set in another shell cannot be inspected, so retiring needs --allow-unchecked: +pass it once you have confirmed no shell for a project that selects the store sets it (it also covers registered +projects that could not be checked). Without a TTY (or with +HACK_NO_INTERACTIVE / --no-interactive) it is refused with E_INTERACTIVE_REQUIRED unless --yes is passed. + +### Options + +| Option | Description | +| --- | --- | +| `--path, -p ` | Run a project command against a repo path (overrides cwd search) | +| `--yes` | Confirm moving the store aside without prompting (required without a TTY or with HACK_NO_INTERACTIVE) | +| `--allow-unchecked` | Accept what cannot be checked: a HACK_SECRETS_FILE_KEY set in another shell for a project that selects the store, and registered projects that could not be checked. Retiring needs it; a key known to open the store still refuses | +| `--no-interactive` | Never prompt: apply documented defaults or fail with E_INTERACTIVE_REQUIRED (also via HACK_NO_INTERACTIVE=1) | +| `--help, -h` | Show help | +| `--version, -v` | Show version | + ## Integrations diff --git a/docs/versions/v5/guides/upgrade-from-v4.md b/docs/versions/v5/guides/upgrade-from-v4.md index a28c33f4e..a147619f0 100644 --- a/docs/versions/v5/guides/upgrade-from-v4.md +++ b/docs/versions/v5/guides/upgrade-from-v4.md @@ -374,6 +374,40 @@ an explicit `env get KEY` is a deliberate plaintext read. Follow that PR's migration notes when installing a release that contains it. Do not use the new output contract or assume the protection command exists in next.16. +#### An old global secret store whose key is gone + +A machine upgraded from v4 may still have an encrypted store such as +`~/.hack/secrets.enc.json` whose key file was lost. From next.17 (HACK-2169, +HACK-2177), Doctor reports that store as a `secret integrity` finding: + +- an **error** in a project whose legacy env contract reads the store, and a + **warning** in any other project; +- the finding names the registered projects that read the store. Without the + key, its entries cannot be counted or read. + +There are two ways forward. Do not provision a new key: it cannot open these +entries. + +1. **Restore the key.** Put the key file back where the finding says (mode + 600), from a backup or another machine, or set `HACK_SECRETS_FILE_KEY` to the + same key. +2. **Retire the store** when the key is gone for good: + `hack secrets retire-store`, run in a project that selects the store or + outside any project for the global one. Without a terminal, add `--yes`. + - It moves the store into a new directory `.retired-/` + beside it, keeping its name. It deletes nothing, reads and prints no + entry, and leaves the key file alone. It only retires a file that is a + Hack encrypted store, and refuses one that another registered project's + key still opens. If some registered projects could not be checked (for + example a worktree on an unmounted volume), it refuses unless you pass + `--allow-unchecked`. The same flag confirms that no shell running a + project that selects the store sets `HACK_SECRETS_FILE_KEY` to its key. + That variable overrides key files and the keychain and cannot be checked + from the command, so in practice retiring needs `--allow-unchecked`. + - It prints the `mv` command that puts the store back. + - Afterwards Doctor lists the retired file as information, and the project + check no longer fails. + Sources for next.16 behavior: [env list and get](https://github.com/hack-dance/hack/blob/b21c3b189841c1a3cbb8af2b76414fa347dc7057/src/commands/env.ts), [project key checks](https://github.com/hack-dance/hack/blob/b21c3b189841c1a3cbb8af2b76414fa347dc7057/src/lib/doctor-env-key.ts). diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index be5fd0769..25917cf69 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -3168,11 +3168,16 @@ async function checkSecretIntegrity({ message: "Every stored secret opens with the key in use", }; } + // Information alone (a retired store kept beside its old name) is ok. + let status: CheckResult["status"] = "ok"; + if (findings.some((finding) => finding.status === "error")) { + status = "error"; + } else if (findings.some((finding) => finding.status === "warn")) { + status = "warn"; + } return { name: SECRET_INTEGRITY_CHECK, - status: findings.some((finding) => finding.status === "error") - ? "error" - : "warn", + status, message: findings.map((finding) => finding.message).join("\n"), }; } @@ -5879,6 +5884,7 @@ async function renderRecoveryGuidance( const hasActions = guidance.temporaryBreakage.length > 0 || guidance.configurationRepair.length > 0 || + (guidance.choices?.length ?? 0) > 0 || guidance.followUp.length > 0; if (!hasActions) { diff --git a/src/commands/recovery-guidance.ts b/src/commands/recovery-guidance.ts index ec931a40d..ff34bce11 100644 --- a/src/commands/recovery-guidance.ts +++ b/src/commands/recovery-guidance.ts @@ -9,12 +9,31 @@ export type RecoveryCheckResult = { export type DoctorRecoveryGuidance = { readonly temporaryBreakage: readonly string[]; readonly configurationRepair: readonly string[]; + /** + * Alternatives the operator chooses between (one is enough), as plain + * text; for example the two ways forward for a secret store whose key is + * missing (HACK-2177). + */ + readonly choices?: readonly string[]; readonly followUp: readonly string[]; readonly verify: readonly string[]; readonly capture: readonly string[]; }; const FIX_CHECKS = new Set(["caddy local ca", "dns:hack", "dns:hack.gy"]); +/** The secret integrity finding for a store whose key is missing names this command. */ +const RETIRE_STORE_COMMAND = "hack secrets retire-store"; +/** The command as the finding names it, with the diagnosed project's `--path`. */ +const RETIRE_STORE_COMMAND_PATTERN = + /hack secrets retire-store(?: --path (?:'(?:[^']|'\\'')*'|[A-Za-z0-9_@%+=:,./-]+))?/u; + +/** The two ways forward for such a store, shown as one choice (HACK-2177). */ +function secretStoreChoices(command: string): readonly string[] { + return [ + "Restore the encrypted secret store's key: put the key file back where the secret integrity check says (mode 600), or set HACK_SECRETS_FILE_KEY to the same key.", + `Or, if the key is gone for good, retire the store: \`${command}\` moves it aside to a timestamped name beside it, deletes nothing, and prints how to put it back. It needs --allow-unchecked, your confirmation that no shell for a project that selects the store sets HACK_SECRETS_FILE_KEY to its key, which Hack cannot inspect.`, + ]; +} const SAFE_SHELL_ARG_PATTERN = /^[A-Za-z0-9_./:-]+$/u; const PROJECTS_PRUNE_COMMAND_PATTERN = /hack projects prune(?: --project [A-Za-z0-9_.-]+)?/u; @@ -61,6 +80,7 @@ export function buildDoctorRecoveryGuidance(input: { }): DoctorRecoveryGuidance { const temporaryBreakage: string[] = []; const configurationRepair: string[] = []; + const choices: string[] = []; const followUp: string[] = []; for (const result of input.results) { @@ -72,6 +92,21 @@ export function buildDoctorRecoveryGuidance(input: { continue; } + if ( + result.name === "secret integrity" && + result.message.includes(RETIRE_STORE_COMMAND) + ) { + const command = + result.message.match(RETIRE_STORE_COMMAND_PATTERN)?.[0] ?? + RETIRE_STORE_COMMAND; + for (const choice of secretStoreChoices(command)) { + pushUnique(choices, choice); + } + // Any other finding in the same check still needs a look. + followUp.push(`${result.name}: ${result.message}`); + continue; + } + if (result.message.includes("hack global up")) { pushUnique(temporaryBreakage, "hack global up"); continue; @@ -133,6 +168,7 @@ export function buildDoctorRecoveryGuidance(input: { return { temporaryBreakage, configurationRepair, + choices, followUp, verify: ["hack doctor"], capture: ["hack crash-capture --path "], @@ -150,6 +186,17 @@ export function scopeDoctorRecoveryGuidance(input: { configurationRepair: input.guidance.configurationRepair.map((command) => scopeRecoveryCommand({ command, projectRoot: input.projectRoot }) ), + // A retire command without --path acts on the current directory: scope + // it to the diagnosed project, as the other project commands are. + choices: (input.guidance.choices ?? []).map((choice) => + input.projectRoot?.trim() && + choice.includes(`\`${RETIRE_STORE_COMMAND}\``) + ? choice.replace( + `\`${RETIRE_STORE_COMMAND}\``, + `\`${RETIRE_STORE_COMMAND} --path ${quoteShellArg(input.projectRoot.trim())}\`` + ) + : choice + ), followUp: [...input.guidance.followUp], verify: input.guidance.verify.map((command) => scopeRecoveryCommand({ command, projectRoot: input.projectRoot }) @@ -188,6 +235,11 @@ export function buildRecoveryNextSteps(input: { nextSteps.push(`Configuration repair: \`${command}\`.`); } + const choices = scoped.choices ?? []; + if (choices.length > 0) { + nextSteps.push(`Choose one: ${choices.join(" ")}`); + } + for (const item of scoped.followUp) { nextSteps.push(`Manual follow-up: ${item}`); } @@ -244,6 +296,15 @@ export function buildRecoveryWorkflowLines(input: { stepNumber += 1; } + const choices = scoped.choices ?? []; + if (choices.length > 0) { + lines.push(`${stepNumber}. Choose one:`); + for (const choice of choices) { + lines.push(` - ${choice}`); + } + stepNumber += 1; + } + if (scoped.followUp.length > 0) { lines.push(`${stepNumber}. Investigate:`); for (const item of scoped.followUp) { diff --git a/src/commands/secrets.ts b/src/commands/secrets.ts index 9379b0c12..18f190a5a 100644 --- a/src/commands/secrets.ts +++ b/src/commands/secrets.ts @@ -1,12 +1,22 @@ +import { resolve } from "node:path"; + import { isCancel, password, text } from "@clack/prompts"; import type { CliContext, CommandArgs } from "../cli/command.ts"; import { defineCommand, defineOption, withHandler } from "../cli/command.ts"; +import { optPath } from "../cli/options.ts"; import { canPrompt, confirmSafe, requireInteractive, } from "../lib/interactivity.ts"; import { keychain } from "../lib/keychain.ts"; +import { quoteShellWord } from "../lib/logs.ts"; +import { findProjectContext } from "../lib/project.ts"; +import { + planSecretStoreRetire, + retireSecretStore, +} from "../lib/secret-store-retire.ts"; +import { escapeTerminalControls } from "../lib/terminal-text.ts"; import { logger } from "../ui/logger.ts"; const DEFAULT_SECRETS_SERVICE = "hack-cli" as const; @@ -28,8 +38,26 @@ const optYes = defineOption({ "Confirm the deletion without prompting (required without a TTY or with HACK_NO_INTERACTIVE)", } as const); +const optRetireYes = defineOption({ + name: "yes", + type: "boolean", + long: "--yes", + description: + "Confirm moving the store aside without prompting (required without a TTY or with HACK_NO_INTERACTIVE)", +} as const); + +const optAllowUnchecked = defineOption({ + name: "allowUnchecked", + type: "boolean", + long: "--allow-unchecked", + description: + "Accept what cannot be checked: a HACK_SECRETS_FILE_KEY set in another shell for a project that selects the store, and registered projects that could not be checked. Retiring needs it; a key known to open the store still refuses", +} as const); + const secretsOptions = [optService] as const; const deleteOptions = [optService, optYes] as const; +const retireStoreOptions = [optPath, optRetireYes, optAllowUnchecked] as const; +type RetireStoreArgs = CommandArgs; const secretsPositionals = [{ name: "name", required: false }] as const; type SecretsArgs = CommandArgs< @@ -72,6 +100,27 @@ const deleteSpec = defineCommand({ subcommands: [], } as const); +const retireStoreSpec = defineCommand({ + name: "retire-store", + summary: + "Move an encrypted secret store whose key is gone aside (reversible)", + group: "Secrets", + description: [ + "For an encrypted_file (or cloud shim) store that hack doctor reports with a missing or wrong key.", + "Moves the store into a new directory .retired-/ beside it: nothing is deleted, read or printed,", + "and the key file is left alone. It prints how to put the store back. The store is the one this project", + "selects (--path or the current directory), or the global one outside a project. Refused for a store that", + "opens with the key in use, that another registered project's key still opens, or that is unreadable for", + "another reason. HACK_SECRETS_FILE_KEY set in another shell cannot be inspected, so retiring needs --allow-unchecked:", + "pass it once you have confirmed no shell for a project that selects the store sets it (it also covers registered", + "projects that could not be checked). Without a TTY (or with", + "HACK_NO_INTERACTIVE / --no-interactive) it is refused with E_INTERACTIVE_REQUIRED unless --yes is passed.", + ].join("\n"), + options: retireStoreOptions, + positionals: [], + subcommands: [], +} as const); + export const secretsCommand = defineCommand({ name: "secrets", summary: "Manage secrets in OS keychain (Bun.secrets)", @@ -83,6 +132,7 @@ export const secretsCommand = defineCommand({ withHandler(setSpec, handleSecretsSet), withHandler(getSpec, handleSecretsGet), withHandler(deleteSpec, handleSecretsDelete), + withHandler(retireStoreSpec, handleSecretsRetireStore), ], } as const); @@ -216,3 +266,72 @@ async function handleSecretsDelete({ logger.success({ message: `Deleted secret "${name}" (${service})` }); return 0; } + +/** + * `hack secrets retire-store` (HACK-2177): the reversible way forward for an + * encrypted secret store whose key is gone. Confirmation works as for + * `delete`: `--yes`, a prompt (default no), or a closed refusal when the run + * cannot prompt, before anything is moved. + */ +async function handleSecretsRetireStore({ + args, +}: { + readonly ctx: CliContext; + readonly args: RetireStoreArgs; +}): Promise { + const startDir = args.options.path + ? resolve(process.cwd(), args.options.path) + : process.cwd(); + const project = await findProjectContext(startDir); + // Plan once: the confirmation names this exact target (its real path; the + // configured path is project input and may go through a symlinked + // directory), and the retirement refuses if the target changed since. + const allowUnchecked = args.options.allowUnchecked === true; + const planned = await planSecretStoreRetire({ + projectDir: project?.projectDir, + allowUnchecked, + }); + if (planned.kind === "refused") { + logger.error({ message: escapeTerminalControls(planned.message) }); + return 1; + } + const ok = + args.options.yes === true || + (await confirmSafe({ + // Paths are project input: show control characters, never send them. + message: `Move the encrypted secret store ${escapeTerminalControls(planned.plan.display)} aside? Its entries stay in the moved file, which can be put back.`, + initialValue: false, + nonInteractive: "fail", + hint: "Pass --yes to confirm (`hack secrets retire-store --yes`), or run it in a terminal.", + })); + if (!ok) { + return 0; + } + const outcome = await retireSecretStore({ + projectDir: project?.projectDir, + approved: planned.plan, + allowUnchecked, + }); + if (outcome.kind === "refused") { + logger.error({ message: escapeTerminalControls(outcome.message) }); + return 1; + } + logger.success({ + // Each line escaped on its own: the newlines between them are intended. + message: [ + outcome.configured === outcome.from + ? `Moved the encrypted secret store ${outcome.from} to ${outcome.to}.` + : `Moved the encrypted secret store ${outcome.from} (configured as ${outcome.configured}) to ${outcome.to}.`, + "Nothing was deleted, and no entry was read or printed. The key file was left alone.", + `To put it back (while no store is at ${outcome.from}): mv ${quoteShellWord(outcome.to)} ${quoteShellWord(outcome.from)}`, + ...(outcome.replacement === null + ? [] + : [ + `A different file appeared at ${outcome.from} while moving; it was kept at ${outcome.replacement}.`, + ]), + ] + .map(escapeTerminalControls) + .join("\n"), + }); + return 0; +} diff --git a/src/lib/doctor-secret-integrity.ts b/src/lib/doctor-secret-integrity.ts index 014873a8e..77784d7bc 100644 --- a/src/lib/doctor-secret-integrity.ts +++ b/src/lib/doctor-secret-integrity.ts @@ -1,14 +1,25 @@ -import { homedir } from "node:os"; import { isAbsolute, relative, sep } from "node:path"; import { PROJECT_ENV_KEY_FILENAME } from "../constants.ts"; +import { resolveUserHomeDir } from "./home-dir.ts"; +import { quoteShellWord } from "./logs.ts"; import { inspectProjectEnvLayerKeys, PROJECT_ENV_SECRET_KEY_ENV, type ProjectEnvLayerInspectionFailure, type ProjectEnvLayerKeyReport, } from "./project-env-config.ts"; -import { listFileSecretStoreEntries } from "./secret-store.ts"; +import { + isEncryptedStoreFile, + listFileSecretStoreEntries, + listRetiredEncryptedStores, +} from "./secret-store.ts"; +import { + listProjectsUsingSecretStore, + projectUsesSecretStore, + type SecretStoreUsers, +} from "./secret-store-retire.ts"; +import { escapeTerminalControls } from "./terminal-text.ts"; /** * `hack doctor`'s secret integrity findings (HACK-2169): stores and env @@ -16,10 +27,22 @@ import { listFileSecretStoreEntries } from "./secret-store.ts"; * value-free: paths, counts and fixed text only, never a value or key. */ export type SecretIntegrityFinding = { - readonly status: "warn" | "error"; + /** `info` is not a problem: it is shown with an `ok` result. */ + readonly status: "info" | "warn" | "error"; readonly message: string; }; +/** The command that moves a store whose key is gone aside (HACK-2177). */ +export const RETIRE_STORE_COMMAND = "hack secrets retire-store"; + +/** + * The retire command for the store this project selects: with `--path`, + * since it otherwise acts on the current directory's project. + */ +function retireStoreCommand(projectRoot: string): string { + return `${RETIRE_STORE_COMMAND} --path ${quoteShellWord(projectRoot)}`; +} + /** A path for display: project-relative inside it, `~/...` under home. */ function displayPath(opts: { readonly projectRoot: string; @@ -34,7 +57,7 @@ function displayPath(opts: { ) { return inside; } - const home = homedir(); + const home = resolveUserHomeDir(); const underHome = relative(home, opts.path); if (underHome && !underHome.startsWith("..") && !isAbsolute(underHome)) { return `~/${underHome}`; @@ -52,17 +75,161 @@ async function inspectSecretStore(opts: { projectName: opts.projectName, projectDir: opts.projectDir, }); + const retired = + listing.kind === "inactive" + ? [] + : await retiredStoreFindings({ ...opts, storePath: listing.storePath }); if (listing.kind !== "unreadable") { - return []; + return retired; } const store = displayPath({ ...opts, path: listing.storePath }); const key = displayPath({ ...opts, path: listing.keyPath }); - switch (listing.reason) { + if ( + listing.reason === "key_missing" || + listing.reason === "wrong_key_or_corrupt" + ) { + return [ + await keylessStoreFinding({ + ...opts, + storePath: listing.storePath, + store, + key, + missing: listing.reason === "key_missing", + }), + ...retired, + ]; + } + // The store is machine-wide: an error only in a project whose runtime + // reads it, a warning elsewhere (HACK-2177). + const usedHere = await projectUsesSecretStore({ + projectDir: opts.projectDir, + }); + return [ + ...storeFindings({ + reason: listing.reason, + store, + key, + storePath: listing.storePath, + keyPath: listing.keyPath, + }).map( + (finding): SecretIntegrityFinding => + usedHere || finding.status !== "error" + ? finding + : { + status: "warn", + message: `${finding.message} This project does not read it.`, + } + ), + ...retired, + ]; +} + +/** + * A store with entries that no available key opens (HACK-2177). An error + * only for a project whose runtime reads the store; for any other project + * the same machine-wide store is a warning. It says what is known without + * the key (entries cannot be counted), which registered projects read it, + * and the two ways forward: restore the key, or retire the store. + */ +async function keylessStoreFinding(opts: { + readonly projectRoot: string; + readonly projectDir: string; + readonly storePath: string; + readonly store: string; + readonly key: string; + readonly missing: boolean; +}): Promise { + const usedHere = await projectUsesSecretStore({ + projectDir: opts.projectDir, + }); + if (!(await isEncryptedStoreFile(opts.storePath))) { + // The configured path is project input and may name any file: never + // suggest retiring something that is not a Hack store. + return { + status: usedHere ? "error" : "warn", + message: `The configured encrypted secret store path ${opts.store} does not hold a Hack encrypted secret store (no store envelope), so Hack does not read, change or retire it. Check controlPlane.secrets.encryptedFile.path in the Hack config.`, + }; + } + const users = await listProjectsUsingSecretStore({ + storePath: opts.storePath, + }); + const state = opts.missing + ? `The encrypted secret store ${opts.store} has entries, but its key ${opts.key} is missing.` + : `The available key does not open the encrypted secret store ${opts.store}, or the store is damaged.`; + const usage = [ + usedHere + ? "This project's env contract reads it." + : "This project does not read it.", + describeStoreUsers(users), + ].join(" "); + return { + status: usedHere ? "error" : "warn", + message: `${state} Its entries cannot be counted or read without that key. ${usage} Choose one: restore the key (put the key file back at ${opts.key} with mode 600 from a backup or another machine, or set HACK_SECRETS_FILE_KEY to the same key); or, if the key is gone for good, run \`${retireStoreCommand(opts.projectRoot)}\`, which moves the store aside to a timestamped name beside it, deletes nothing, and prints how to put it back. Add --allow-unchecked once you have confirmed that no shell running a project that selects the store sets HACK_SECRETS_FILE_KEY to its key: that variable overrides key files and cannot be checked from here, so the command refuses without the flag. Do not provision a new key: it cannot open these entries.`, + }; +} + +/** + * Which registered projects read the store, saying plainly when the scan + * could not check every one (never "no project reads it" on a guess). + */ +function describeStoreUsers(users: SecretStoreUsers): string { + const named = + users.names.length > 0 + ? `Registered projects that read it: ${users.names.join(", ")}.` + : null; + if (!users.registryRead) { + return "The projects registry could not be read or is not valid, so which registered projects read it is unknown."; + } + if (users.unchecked > 0) { + const gap = `${users.unchecked} registered project(s) could not be checked (their Hack config or store file could not be read), so the list may be incomplete.`; + return named === null + ? `No checked registered project reads it, but ${gap}` + : `${named} ${gap}`; + } + return named ?? "No registered project reads it."; +} + +/** Retired copies kept beside the store: information, not a problem. */ +async function retiredStoreFindings(opts: { + readonly projectRoot: string; + readonly storePath: string; +}): Promise { + const store = displayPath({ ...opts, path: opts.storePath }); + return (await listRetiredEncryptedStores(opts.storePath)).map((path) => { + const shown = displayPath({ ...opts, path }); + // Absolute paths (a quoted `~` is not expanded), shell-quoted: a + // configured store path is project input and may hold a quote. + return { + status: "info", + message: `Retired secret store kept at ${shown}; Hack no longer reads it. To put it back while no store is at ${store}: mv ${quoteShellWord(path)} ${quoteShellWord(opts.storePath)}`, + }; + }); +} + +/** Findings for the remaining store states (unchanged from HACK-2169). */ +function storeFindings(opts: { + readonly reason: Exclude< + Extract< + Awaited>, + { kind: "unreadable" } + >["reason"], + "key_missing" | "wrong_key_or_corrupt" + >; + readonly store: string; + readonly key: string; + /** Absolute paths, shell-quoted wherever a command is printed. */ + readonly storePath: string; + readonly keyPath: string; +}): SecretIntegrityFinding[] { + const { store, key } = opts; + const storeArg = quoteShellWord(opts.storePath); + const keyArg = quoteShellWord(opts.keyPath); + switch (opts.reason) { case "store_unreadable": return [ { status: "error", - message: `The encrypted secret store ${store} exists but cannot be read; Hack will not read or change it. Make it a regular file this user owns and can read and write (chmod 600 ${store}), then re-run hack doctor.`, + message: `The encrypted secret store ${store} exists but cannot be read; Hack will not read or change it. Make it a regular file this user owns and can read and write (chmod 600 ${storeArg}), then re-run hack doctor.`, }, ]; case "store_refused": @@ -72,25 +239,25 @@ async function inspectSecretStore(opts: { message: `The encrypted secret store ${store} is a symlink, not a regular file, or too large, so Hack refuses to read or change it. Replace it with a regular file this user owns (for a symlink: copy the target's contents to ${store} with mode 600), then re-run hack doctor.`, }, ]; - case "key_missing": + case "key_access_failed": return [ { status: "error", - message: `The encrypted secret store ${store} has entries, but its key ${key} is missing. Restore that key file from a backup or another machine (mode 600), or set HACK_SECRETS_FILE_KEY to the same key. Do not provision a new key: it cannot open these entries.`, + message: `The key for ${store} could not be looked up: there is no key file at ${key}, and the OS keychain could not be asked (it is locked, denied access, or timed out). Unlock the keychain or allow access, then re-run hack doctor. Do not retire the store or provision a new key: the keychain may still hold its key.`, }, ]; - case "key_refused": + case "key_unreadable": return [ { status: "error", - message: `The key ${key} for ${store} is refused: it is a symlink, other users can read or write it, or git tracks it. Replace it with a regular, untracked file this user owns (chmod 600 ${key}; git rm --cached if tracked, then rotate it).`, + message: `The key ${key} for ${store} exists but cannot be read by this user. Fix its owner and permissions (chmod 600 ${keyArg}, owned by this user), then re-run hack doctor. Do not retire the store or provision a new key: the existing key may still open it.`, }, ]; - case "wrong_key_or_corrupt": + case "key_refused": return [ { status: "error", - message: `The available key does not open the encrypted secret store ${store}, or the store is damaged. Restore the key that encrypted it (or the store from a backup); do not provision a replacement key over it.`, + message: `The key ${key} for ${store} is refused: it is a symlink, other users can read or write it, or git tracks it. Replace it with a regular, untracked file this user owns (chmod 600 ${keyArg}; git rm --cached ${keyArg} if tracked, then rotate it).`, }, ]; case "incomplete_config": @@ -215,6 +382,13 @@ export async function inspectSecretIntegrity(opts: { ...(await inspectEnvLayers(opts)), ]; const rank = (finding: SecretIntegrityFinding) => - finding.status === "error" ? 0 : 1; - return findings.sort((a, b) => rank(a) - rank(b)); + ({ error: 0, warn: 1, info: 2 })[finding.status]; + // Messages carry project-configured paths: control characters are shown, + // never sent to the terminal (HACK-2177). + return findings + .sort((a, b) => rank(a) - rank(b)) + .map((finding) => ({ + ...finding, + message: escapeTerminalControls(finding.message), + })); } diff --git a/src/lib/projects-registry.ts b/src/lib/projects-registry.ts index efda30b62..90e416007 100644 --- a/src/lib/projects-registry.ts +++ b/src/lib/projects-registry.ts @@ -172,6 +172,61 @@ export async function readProjectsRegistry(opts?: { return out ?? { version: REGISTRY_VERSION, projects: [] }; } +/** + * Strict read for callers that must not take an unreadable or invalid + * registry for an empty one (HACK-2177): `absent` (no file), `valid`, or + * `invalid` (not JSON, or not a registry). A file that cannot be read + * throws {@link ProjectsRegistryReadError}, as {@link readProjectsRegistry}. + */ +export async function readProjectsRegistryStrict(opts?: { + readonly registryPath?: string; +}): Promise< + | { readonly kind: "absent" | "invalid" } + | { + readonly kind: "valid"; + readonly registry: ProjectsRegistry; + /** Project entries the registry holds that are not valid (skipped). */ + readonly rejected: number; + } +> { + const text = await readRegistryText(opts?.registryPath ?? getRegistryPath()); + if (text === null) { + return { kind: "absent" }; + } + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch { + return { kind: "invalid" }; + } + const registry = parseRegistry(parsed); + if (registry === null) { + return { kind: "invalid" }; + } + // The best-effort parser skips invalid project entries, and invalid + // worktree entries inside a valid project; count both, so a caller never + // presents a filtered list as complete. + const raw = isRecord(parsed) ? parsed.projects : undefined; + const items: unknown[] = Array.isArray(raw) ? raw : []; + let rejected = 0; + for (const item of items) { + const project = parseProject(item); + if (project === null) { + rejected += 1; + continue; + } + const rawWorktrees = isRecord(item) ? item.worktrees : undefined; + if (Array.isArray(rawWorktrees)) { + rejected += rawWorktrees.length - (project.worktrees?.length ?? 0); + } else if (rawWorktrees !== undefined) { + // A present worktrees field that is not an array hides any checkouts + // it held: unknown, not "no worktrees". + rejected += 1; + } + } + return { kind: "valid", registry, rejected }; +} + type RegistrationUpsertOptions = { readonly project: ProjectContext; readonly nowIso?: string; diff --git a/src/lib/secret-store-retire.ts b/src/lib/secret-store-retire.ts new file mode 100644 index 000000000..79a405388 --- /dev/null +++ b/src/lib/secret-store-retire.ts @@ -0,0 +1,667 @@ +import { lstat, realpath, stat } from "node:fs/promises"; +import { basename, resolve } from "node:path"; + +import { PROJECT_CONFIG_FILENAME } from "../constants.ts"; +import { readControlPlaneConfig } from "../control-plane/sdk/config.ts"; +import { resolveGlobalConfigPath } from "./config-paths.ts"; +import { readOptionalTextFile } from "./fs.ts"; +import { readHackEnvContract, readHackEnvRuntimeConfig } from "./hack-env.ts"; +import { + type RegisteredProject, + readProjectsRegistryStrict, +} from "./projects-registry.ts"; +import { + classifyEncryptedStoreAt, + isEncryptedStoreFile, + keychainKeyOpensStore, + keyFileOpensStore, + resolveFileSecretStorePaths, + retireEncryptedStoreFile, + SecretStoreRetireError, +} from "./secret-store.ts"; + +/** + * Retiring an encrypted secret store whose key is gone (HACK-2177): a + * deliberate, reversible way forward after `hack doctor` reports a store it + * cannot open. Retiring only renames the store file beside itself; it never + * deletes, decrypts or prints an entry. + */ + +/** Store states retiring is for: entries exist and the key that opens them does not. */ +const RETIRABLE_REASONS = new Set(["key_missing", "wrong_key_or_corrupt"]); + +/** + * Whether a project's runtime reads its secret store: its legacy env + * contract declares a store-backed (`source: "keychain"`) value, or its + * configuration keeps plaintext in the backend. The same condition the + * runtime uses before it opens the store. A contract that cannot be read + * counts as using it, since what it declares is unknown. + */ +export async function projectUsesSecretStore(opts: { + readonly projectDir: string; +}): Promise { + try { + const runtime = await readHackEnvRuntimeConfig(opts); + if (runtime.storePlaintextInBackend) { + return true; + } + const contract = await readHackEnvContract(opts); + if (contract.parseError !== undefined) { + return true; + } + return contract.contract.vars.some( + (contractVar) => contractVar.source === "keychain" + ); + } catch { + return true; + } +} + +/** + * Why the configuration that selects the store (the global config and, for + * a project, its `hack.config.json`) cannot be trusted, or null when it can. + * A file that exists but cannot be read is not absent, and one that does + * not parse contributes nothing, so either could silently select another + * store (for example the global one). Value-free: paths only. + */ +export async function secretStoreSelectionProblem(opts: { + readonly projectDir?: string; +}): Promise { + const paths = [ + resolveGlobalConfigPath(), + ...(opts.projectDir === undefined + ? [] + : [resolve(opts.projectDir, PROJECT_CONFIG_FILENAME)]), + ]; + for (const path of paths) { + if ((await readOptionalTextFile(path)).kind === "unreadable") { + return `The Hack config ${path} exists but cannot be read, so which secret store is selected is unknown.`; + } + } + const config = await readControlPlaneConfig({ projectDir: opts.projectDir }); + return config.parseError === undefined + ? null + : "The Hack config that selects the secret store does not parse, so which secret store is selected is unknown."; +} + +/** Registered projects that read a store, and whether every one was checked. */ +export type SecretStoreUsers = { + /** Names only, sorted. */ + readonly names: readonly string[]; + /** False when the registry could not be read or is not a valid registry. */ + readonly registryRead: boolean; + /** Registered projects whose store selection could not be checked. */ + readonly unchecked: number; +}; + +/** Registered checkouts that select one store file, and what could not be checked. */ +type StoreContexts = { + readonly contexts: readonly { + readonly name: string; + readonly projectDir: string; + readonly keyPath: string; + }[]; + readonly registryRead: boolean; + readonly unchecked: number; +}; + +/** + * Every registered checkout (each project's primary checkout and each of + * its linked worktrees) whose configuration selects the store file at + * `storePath`: the same configured path, or the same file (device and + * inode, so aliases through symlinked directories match). From + * configuration only. A checkout that is not there, whose config is unsure, + * or whose store identity cannot be read is counted as unchecked, never + * taken as "does not select it". + */ +async function storeContexts(opts: { + readonly storePath: string; +}): Promise { + let projects: readonly RegisteredProject[]; + // Registry entries that are not valid projects: unknown, so unchecked. + let rejected = 0; + try { + const read = await readProjectsRegistryStrict(); + if (read.kind === "invalid") { + return { contexts: [], registryRead: false, unchecked: 0 }; + } + projects = read.kind === "valid" ? read.registry.projects : []; + rejected = read.kind === "valid" ? read.rejected : 0; + } catch { + return { contexts: [], registryRead: false, unchecked: 0 }; + } + const target = await identity(opts.storePath); + const contexts: StoreContexts["contexts"][number][] = []; + let unchecked = rejected; + for (const checkout of projects.flatMap(registeredCheckouts)) { + const location = await checkoutStoreLocation(checkout.projectDir); + if (location === null) { + unchecked += 1; + continue; + } + if (!location.active) { + continue; + } + const same = await sameStoreFile({ + path: location.storePath, + opts, + target, + }); + if (same === "unknown") { + unchecked += 1; + } else if (same === "same") { + contexts.push({ ...checkout, keyPath: location.keyPath }); + } + } + return { contexts, registryRead: true, unchecked }; +} + +/** + * A checkout's store location from its configuration, or null when that + * cannot be established: the checkout is not there (an unmounted volume, a + * moved repo; absent files there are not "unconfigured"), or its selecting + * config is unsure. + */ +async function checkoutStoreLocation( + projectDir: string +): Promise> | null> { + if (!(await stat(projectDir).catch(() => null))?.isDirectory()) { + return null; + } + try { + if ((await secretStoreSelectionProblem({ projectDir })) !== null) { + return null; + } + return await resolveFileSecretStorePaths({ projectDir }); + } catch { + return null; + } +} + +/** + * Registered checkouts whose configuration selects the store at + * `storePath` and whose runtime reads it ({@link projectUsesSecretStore}). + * No store is opened. What could not be checked is counted, never taken as + * "does not read it". + */ +export async function listProjectsUsingSecretStore(opts: { + readonly storePath: string; +}): Promise { + const found = await storeContexts(opts); + const names = new Set(); + for (const context of found.contexts) { + if (await projectUsesSecretStore({ projectDir: context.projectDir })) { + names.add(context.name); + } + } + return { + names: [...names].sort(), + registryRead: found.registryRead, + unchecked: found.unchecked, + }; +} + +/** + * Why retiring this store would take it from another registered checkout + * that can still open it, or null when none can (HACK-2177). A checkout + * selecting the same store file with a different key file is asked whether + * its key opens the store (read-only, at the pinned real path). Incomplete + * coverage refuses unless `allowUnchecked`: the store may be another + * checkout's that could not be checked. + */ +async function otherOpenersRefusal(input: { + readonly canonical: string; + readonly keyPath: string; + readonly allowUnchecked: boolean; +}): Promise { + const found = await storeContexts({ storePath: input.canonical }); + const openers: string[] = []; + // Every key file that selects this store, this project's own included, + // read directly (no HACK_SECRETS_FILE_KEY override, no keychain), so a + // wrong override in this shell cannot hide a key file that works. + const keyFiles = new Map([[resolve(input.keyPath), []]]); + for (const context of found.contexts) { + const keyPath = resolve(context.keyPath); + keyFiles.set(keyPath, [...(keyFiles.get(keyPath) ?? []), context.name]); + } + const evidence = await keyEvidence({ + storePath: input.canonical, + keyFiles, + }); + openers.push(...evidence.openers); + const unknownKeys = evidence.unknown; + const envOnly = [...new Set(evidence.envOnly)].sort(); + if (openers.length > 0) { + return `The encrypted secret store at ${input.canonical} still opens with a configured key file (${[...new Set(openers)].sort().join(", ")}), so it is not retired; nothing was moved. Check HACK_SECRETS_FILE_KEY and this project's key configuration instead.`; + } + if (input.allowUnchecked) { + return null; + } + if (!found.registryRead) { + return `The projects registry could not be read or is not valid, so whether another project still opens ${input.canonical} is unknown; nothing was moved. Fix the registry, or pass --allow-unchecked to retire it anyway.`; + } + if (unknownKeys > 0) { + return `${unknownKeys} configured key(s) for ${input.canonical} (a key file, or the OS keychain) could not be read, so whether one still opens it is unknown; nothing was moved. Fix their permissions and run hack doctor, or pass --allow-unchecked to retire it anyway.`; + } + if (envOnly.length > 0) { + return `No configured key file or keychain entry opens ${input.canonical}, but HACK_SECRETS_FILE_KEY overrides both, and ${envOnly.join(", ")} may still open it with that variable set in another shell, which cannot be checked from here; nothing was moved. If you have confirmed none does, pass --allow-unchecked to retire it.`; + } + if (found.unchecked > 0) { + return `${found.unchecked} registered project(s) could not be checked (not available, or their Hack config or store file could not be read), so whether one still opens ${input.canonical} is unknown; nothing was moved. Run hack doctor to see them, or pass --allow-unchecked to retire it anyway.`; + } + return null; +} + +/** + * Which configured keys open the store, read independently of this shell: + * each key file directly, and the OS keychain (the runtime's fallback) once + * when any checkout has no key file. Returns who opens it, and how many + * keys could not be read (never taken as "does not open"). + */ +async function keyEvidence(input: { + readonly storePath: string; + readonly keyFiles: ReadonlyMap; +}): Promise<{ + readonly openers: readonly string[]; + readonly unknown: number; + /** + * Checkouts whose key file and keychain do not open the store. At + * runtime HACK_SECRETS_FILE_KEY overrides both, and it may be set in + * their own shell, which cannot be checked from here. + */ + readonly envOnly: readonly string[]; +}> { + const openers: string[] = []; + const keychainUsers: string[] = []; + const wrongKeyFile: string[] = []; + let keychainReader = false; + let unknown = 0; + for (const [keyPath, names] of input.keyFiles) { + const state = await keyFileOpensStore({ + storePath: input.storePath, + keyPath, + }); + if (state === "opens") { + openers.push(...(names.length > 0 ? names : [`its key file ${keyPath}`])); + } else if (state === "unknown") { + unknown += 1; + } else if (state === "absent") { + keychainReader = true; + keychainUsers.push(...names); + } else { + wrongKeyFile.push(...names); + } + } + if (!keychainReader) { + return { openers, unknown, envOnly: envOnlyNames(wrongKeyFile) }; + } + const keychainState = await keychainKeyOpensStore({ + storePath: input.storePath, + }); + if (keychainState === "opens") { + openers.push( + ...(keychainUsers.length > 0 + ? keychainUsers.map((name) => `${name}, through the OS keychain`) + : ["the OS keychain"]) + ); + return { openers, unknown, envOnly: [] }; + } + if (keychainState === "unknown") { + unknown += 1; + } + return { + openers, + unknown, + envOnly: envOnlyNames([...wrongKeyFile, ...keychainUsers]), + }; +} + +/** Names for the env-override refusal; "this project" when none is registered. */ +function envOnlyNames(names: readonly string[]): readonly string[] { + return names.length > 0 ? names : ["this project"]; +} + +/** A registered project's primary checkout and each of its linked worktrees. */ +function registeredCheckouts( + project: RegisteredProject +): { readonly name: string; readonly projectDir: string }[] { + return [ + { name: project.name, projectDir: project.projectDir }, + ...(project.worktrees ?? []).map((worktree) => ({ + name: `${project.name} (worktree ${basename(worktree.path)})`, + projectDir: resolve(worktree.path, project.projectDirName), + })), + ]; +} + +/** + * A file's device and inode: `absent` when nothing is there, `unknown` when + * that cannot be established (for example EACCES or EIO). + */ +async function identity( + path: string +): Promise< + { readonly dev: number; readonly ino: number } | "absent" | "unknown" +> { + try { + const info = await stat(path); + return { dev: info.dev, ino: info.ino }; + } catch (error: unknown) { + const code = + typeof error === "object" && error !== null && "code" in error + ? (error as { readonly code?: unknown }).code + : undefined; + return code === "ENOENT" || code === "ENOTDIR" ? "absent" : "unknown"; + } +} + +/** + * Whether a configured store path is the store: the same resolved path, or + * the same existing file (device and inode). `unknown` when a file's + * identity cannot be established, never taken as "different". + */ +async function sameStoreFile(input: { + readonly path: string; + readonly opts: { readonly storePath: string }; + readonly target: Awaited>; +}): Promise<"same" | "different" | "unknown"> { + if (resolve(input.path) === resolve(input.opts.storePath)) { + return "same"; + } + const candidate = await identity(input.path); + if (candidate === "absent" || input.target === "absent") { + return "different"; + } + if (candidate === "unknown" || input.target === "unknown") { + return "unknown"; + } + return candidate.dev === input.target.dev && + candidate.ino === input.target.ino + ? "same" + : "different"; +} + +/** The outcome of `hack secrets retire-store` (paths only, never values). */ +export type SecretStoreRetireOutcome = + | { + readonly kind: "retired"; + readonly from: string; + readonly to: string; + /** A replacement found at the store path while moving, kept too. */ + readonly replacement: string | null; + /** The configured store path; `from` is its real path. */ + readonly configured: string; + readonly keyPath: string; + } + | { readonly kind: "refused"; readonly message: string }; + +/** + * What retiring would act on, resolved once (HACK-2177): the configured + * store path (project input, possibly through a symlinked directory), its + * real path, and the key path. Shown before confirmation, and carried into + * {@link retireSecretStore}, which refuses if a fresh plan differs. + */ +export type SecretStoreRetirePlan = { + readonly configured: string; + readonly canonical: string; + readonly keyPath: string; + /** For the confirmation: the real path, and the configured one if it differs. */ + readonly display: string; +}; + +/** + * Plans retiring the store this context selects (a project, or the global + * configuration when `projectDir` is omitted). Refused, with the reason, + * when the selecting config is unsure, no file store is configured, its + * real path cannot be resolved, or the store at that real path is not one + * to retire (absent or empty, opens with the key in use, unreadable for a + * reason other than a missing or wrong key, or not a store envelope). + * Every store read uses the real path, never the configured alias. + */ +export async function planSecretStoreRetire(opts: { + readonly projectDir?: string; + /** Retire even when some registered checkouts could not be checked. */ + readonly allowUnchecked?: boolean; +}): Promise< + | { readonly kind: "plan"; readonly plan: SecretStoreRetirePlan } + | { readonly kind: "refused"; readonly message: string } +> { + const selection = await secretStoreSelectionProblem(opts); + if (selection !== null) { + return { + kind: "refused", + message: `${selection} Nothing was moved. Fix that config, then run hack doctor and retry.`, + }; + } + const location = await resolveFileSecretStorePaths(opts).catch(() => null); + if (location === null || !location.active) { + return { + kind: "refused", + message: + "No encrypted file secret store is configured here (the backend is the OS keychain); nothing was moved.", + }; + } + const leafRefusal = await symlinkedStoreRefusal(location.storePath); + if (leafRefusal !== null) { + return { kind: "refused", message: leafRefusal }; + } + const canonical = await realpath(location.storePath).catch(() => null); + if (canonical === null) { + return { + kind: "refused", + message: `There is no encrypted secret store with entries at ${location.storePath}; nothing was moved.`, + }; + } + const refusal = + (await eligibilityRefusal({ + storePath: canonical, + keyPath: location.keyPath, + })) ?? + (await otherOpenersRefusal({ + canonical, + keyPath: location.keyPath, + allowUnchecked: opts.allowUnchecked === true, + })); + if (refusal !== null) { + return { kind: "refused", message: refusal }; + } + return { + kind: "plan", + plan: { + configured: location.storePath, + canonical, + keyPath: location.keyPath, + display: + canonical === resolve(location.storePath) + ? canonical + : `${canonical} (configured as ${location.storePath})`, + }, + }; +} + +/** + * Retires the store this context selects ({@link retireEncryptedStoreFile} + * on its real path). With `approved` (the plan shown before confirmation), + * refuses unless a fresh plan names the same configured path, real path + * and key, so a target retargeted while the confirmation waited is not + * moved. Under the store's lock it checks again: the same config and + * paths, the configured path still leading to the same real path, and the + * store at that real path still one to retire. + */ +export async function retireSecretStore(opts: { + /** Unused: the store is the file, whichever project scope it holds. */ + readonly projectName?: string; + readonly projectDir?: string; + readonly now?: Date; + readonly approved?: SecretStoreRetirePlan; + /** Retire even when some registered checkouts could not be checked. */ + readonly allowUnchecked?: boolean; + /** Test seam: runs under the lock after the path checks, before eligibility. */ + readonly beforeEligibility?: () => Promise; +}): Promise { + const planned = await planSecretStoreRetire(opts); + if (planned.kind === "refused") { + return planned; + } + const { plan } = planned; + if (opts.approved !== undefined && !samePlan(opts.approved, plan)) { + return { + kind: "refused", + message: `The store to retire changed after it was confirmed (confirmed: ${opts.approved.display}; now: ${plan.display}); nothing was moved. Run the command again to confirm the current target.`, + }; + } + try { + const moved = await retireEncryptedStoreFile({ + storePath: plan.canonical, + keyPath: plan.keyPath, + now: opts.now, + recheck: async () => { + const changed = await lockedRecheck({ opts, plan }); + return changed === null + ? null + : `${changed} (It changed while retiring.)`; + }, + }); + return { + kind: "retired", + ...moved, + configured: plan.configured, + keyPath: plan.keyPath, + }; + } catch (error: unknown) { + if (error instanceof SecretStoreRetireError) { + return { kind: "refused", message: error.message }; + } + throw error; + } +} + +function samePlan(a: SecretStoreRetirePlan, b: SecretStoreRetirePlan): boolean { + return ( + a.configured === b.configured && + a.canonical === b.canonical && + a.keyPath === b.keyPath + ); +} + +/** + * Under the store's lock: the selecting config is still sure, still names + * the same store and key paths, the configured path still leads to the + * pinned real path, and the store at that real path (never the alias) is + * still one to retire. A refusal message, or null to go on. + */ +async function lockedRecheck(input: { + readonly opts: { + readonly projectDir?: string; + readonly allowUnchecked?: boolean; + readonly beforeEligibility?: () => Promise; + }; + readonly plan: SecretStoreRetirePlan; +}): Promise { + const { opts, plan } = input; + const unsure = await secretStoreSelectionProblem(opts); + if (unsure !== null) { + return `${unsure} Nothing was moved.`; + } + const location = await resolveFileSecretStorePaths(opts).catch(() => null); + if ( + location === null || + !location.active || + location.storePath !== plan.configured || + location.keyPath !== plan.keyPath + ) { + return "The secret store configuration changed; nothing was moved. Run hack doctor, then retry."; + } + const leafRefusal = await symlinkedStoreRefusal(plan.configured); + if (leafRefusal !== null) { + return leafRefusal; + } + if ((await realpath(plan.configured).catch(() => null)) !== plan.canonical) { + return `${plan.configured} no longer leads to ${plan.canonical}; nothing was moved.`; + } + await opts.beforeEligibility?.(); + return ( + (await eligibilityRefusal({ + storePath: plan.canonical, + keyPath: plan.keyPath, + })) ?? + (await otherOpenersRefusal({ + canonical: plan.canonical, + keyPath: plan.keyPath, + allowUnchecked: opts.allowUnchecked === true, + })) + ); +} + +/** + * Refuses a configured store path whose last component is a symlink (a + * symlinked directory on the way is fine). Runtime, doctor and the store's + * own reads refuse such a store; retiring its target would leave a + * dangling link at the configured path. + */ +async function symlinkedStoreRefusal( + configured: string +): Promise { + let leaf: Awaited>; + try { + leaf = await lstat(configured); + } catch (error: unknown) { + const code = + typeof error === "object" && error !== null && "code" in error + ? (error as { readonly code?: unknown }).code + : undefined; + // Absent is decided later (nothing to retire); anything else is unknown. + if (code === "ENOENT" || code === "ENOTDIR") { + return null; + } + return `The configured store path ${configured} could not be inspected (${String(code ?? "unknown")}), so whether it is a symlink is unknown; nothing was moved.`; + } + return leaf.isSymbolicLink() + ? `The configured store path ${configured} is itself a symlink, which Hack refuses as a store; nothing was moved. Replace it with the regular store file it points to, or point controlPlane.secrets.encryptedFile.path at that file.` + : null; +} + +/** + * Why the store at this (real) path is not one to retire, or null when it + * is: entries exist, the key that opens them is missing or wrong, and the + * file holds a store envelope. + */ +async function eligibilityRefusal(input: { + readonly storePath: string; + readonly keyPath: string; +}): Promise { + const state = await classifyEncryptedStoreAt(input); + if (state.kind === "empty") { + return `There is no encrypted secret store with entries at ${input.storePath}; nothing was moved.`; + } + if (state.kind === "opens") { + return `The encrypted secret store at ${input.storePath} opens with the key in use, so there is nothing to retire; nothing was moved.`; + } + if (!RETIRABLE_REASONS.has(state.reason)) { + return `The encrypted secret store at ${input.storePath} is not retired: it is not a missing or wrong key (${state.reason.replaceAll("_", " ")}). Run hack doctor for what to fix; nothing was moved.`; + } + return await notAStoreRefusal(input.storePath); +} + +/** + * What `hack secrets retire-store` would move, for its confirmation; null + * when there is nothing to retire here. {@link planSecretStoreRetire} + * carries the same target into the retirement. + */ +export async function describeRetireTarget(opts: { + readonly projectDir?: string; + readonly allowUnchecked?: boolean; +}): Promise { + const planned = await planSecretStoreRetire(opts); + return planned.kind === "plan" ? planned.plan.display : null; +} + +/** + * Refuses a configured store path that does not hold a Hack encrypted + * store envelope: the path is project input and may name any file. + */ +async function notAStoreRefusal(storePath: string): Promise { + return (await isEncryptedStoreFile(storePath)) + ? null + : `${storePath} is not a Hack encrypted secret store (it does not hold the store's encrypted envelope), so it is not retired; nothing was moved. Check controlPlane.secrets.encryptedFile.path in the Hack config.`; +} diff --git a/src/lib/secret-store.ts b/src/lib/secret-store.ts index 8853d2c35..84c5a4b35 100644 --- a/src/lib/secret-store.ts +++ b/src/lib/secret-store.ts @@ -3,9 +3,21 @@ import { createDecipheriv, createHash, randomBytes, + randomUUID, } from "node:crypto"; -import { lstat, realpath, stat } from "node:fs/promises"; -import { basename, dirname, resolve } from "node:path"; +import { + link, + lstat, + mkdir, + readdir, + readFile, + realpath, + rename, + rmdir, + stat, + unlink, +} from "node:fs/promises"; +import { basename, dirname, join, resolve } from "node:path"; import { CliUsageError } from "../cli/command.ts"; import type { CloudSecretProvider, @@ -18,6 +30,7 @@ import { envParseDiagnostic } from "./env-parse-diagnostic.ts"; import { ensureDir } from "./fs.ts"; import { resolveUserHomeDir } from "./home-dir.ts"; import { isKeychainDisabled, keychain } from "./keychain.ts"; +import { quoteShellWord } from "./logs.ts"; import { NativeKeychainTimeoutError } from "./native-keychain.ts"; import { decodeCanonicalBase64, @@ -109,6 +122,18 @@ export class SecretStoreKeyUnavailableError extends Error { } } +/** + * The OS keychain could not be asked for the encrypted_file key (access + * denied, locked, timed out, or the failure cooldown): whether the key + * exists is unknown, which is not the same as missing (HACK-2177). + */ +export class SecretStoreKeyAccessError extends SecretStoreKeyUnavailableError { + constructor(message: string) { + super(message); + this.name = "SecretStoreKeyAccessError"; + } +} + /** * The key file exists but is refused (HACK-2036): a link or non-regular * file, readable or writable by other users, or tracked by git. @@ -193,11 +218,452 @@ export type FileSecretStoreListing = { | "store_refused" | "key_missing" | "key_refused" + | "key_unreadable" + | "key_access_failed" | "wrong_key_or_corrupt" | "incomplete_config"; } ); +/** + * Where the encrypted file store (also the cloud shim's) for this context + * lives, from configuration only: nothing is read, opened or provisioned. + * `active` is false when the backend is the OS keychain. + */ +export async function resolveFileSecretStorePaths(input: { + /** Omitted: only the global configuration applies (no project). */ + readonly projectDir?: string; +}): Promise<{ + readonly active: boolean; + readonly storePath: string; + readonly keyPath: string; +}> { + const secretsConfig = ( + await readControlPlaneConfig({ projectDir: input.projectDir }) + ).config.secrets; + return { + active: + secretsConfig.backend === "encrypted_file" || + secretsConfig.backend === "cloud", + storePath: resolveConfiguredPath({ + path: secretsConfig.encryptedFile.path, + projectDir: input.projectDir, + }), + keyPath: resolveConfiguredPath({ + path: + secretsConfig.encryptedFile.keyPath ?? DEFAULT_ENCRYPTED_FILE_KEY_PATH, + projectDir: input.projectDir, + }), + }; +} + +/** Retired stores are kept beside the store as ``. */ +export const RETIRED_STORE_INFIX = ".retired-"; +const STAMP_SEPARATORS = /[-:]/g; +const STAMP_MILLISECONDS = /\.\d{3}Z$/; + +/** Thrown when a store cannot be moved aside; the message says why and what to do. */ +export class SecretStoreRetireError extends Error { + constructor(message: string) { + super(message); + this.name = "SecretStoreRetireError"; + } +} + +/** + * Moves an encrypted store file aside (HACK-2177), never deleting it or + * replacing a file this call did not create. Under the store's lock: + * + * 1. the store must be a regular file, and not the key file itself (same + * device and inode, so an alias or hard link of the key is refused); + * 2. `recheck` runs (eligibility, classified again under the lock); + * 3. a new directory `.retired-/` is created exclusively + * (mode 0700; refused if the name exists); + * 4. the store gets a second name `/` with a hard link, + * which never replaces an existing file: that is the archive; + * 5. whatever is at the store path at that instant is renamed into the + * new directory under an unpredictable, checked-free `.source-` + * name. When it is the archived file (same + * device and inode) that extra name is removed; when a writer outside + * the lock had replaced the store, the replacement stays there too. + * + * Every rename target, and the only name ever removed, is inside the + * directory this call created; nothing outside it is replaced or removed. + * This assumes no other process writes into that fresh private (0700) + * directory: one that does could plant a file at the unpredictable source + * name between its check and the rename, or swap that name between the + * same-file check and its removal. It is not a guarantee against such a + * process. + * On a failure before step 5 the directory is removed with rmdir, which + * refuses unless it is empty. Nothing is read or decrypted; the key file + * is not touched. + */ +export async function retireEncryptedStoreFile(input: { + readonly storePath: string; + /** The store's key file: a store that is the key file is not retired. */ + readonly keyPath?: string; + readonly now?: Date; + /** + * Runs under the store's lock just before the move: a refusal message + * when the store is no longer one to retire (its key was restored, or it + * was replaced meanwhile), null to go on. + */ + readonly recheck?: () => Promise; + /** + * Test seam: runs after the archive link, just before the store path is + * renamed, so a test can replace the store as a writer outside the lock + * would. + */ + readonly beforeMove?: () => Promise; + /** Test seam: runs right after the archive directory is created. */ + readonly afterDirectory?: (dir: string) => Promise; +}): Promise<{ + readonly from: string; + readonly to: string; + /** Set when a replacement was found at the store path and kept too. */ + readonly replacement: string | null; +}> { + const storePath = input.storePath; + return await withEncryptedStoreMutation({ + filePath: storePath, + run: async () => { + await assertRetirableStoreFile(input); + const changed = (await input.recheck?.()) ?? null; + if (changed !== null) { + throw new SecretStoreRetireError(changed); + } + const stamp = (input.now ?? new Date()) + .toISOString() + .replace(STAMP_SEPARATORS, "") + .replace(STAMP_MILLISECONDS, "Z"); + const dir = `${storePath}${RETIRED_STORE_INFIX}${stamp}`; + const to = join(dir, basename(storePath)); + await createRetireDirectory({ storePath, dir }); + await input.afterDirectory?.(dir); + const archived = await linkIntoRetireDirectory({ storePath, dir, to }); + await input.beforeMove?.(); + const replacement = await moveSourceIntoRetireDirectory({ + storePath, + dir, + archived, + }); + return { from: storePath, to, replacement }; + }, + }); +} + +/** The store is a regular file, and not its own key file (same device and inode). */ +async function assertRetirableStoreFile(input: { + readonly storePath: string; + readonly keyPath?: string; +}): Promise { + const info = await stat(input.storePath).catch(() => null); + const linkInfo = await lstat(input.storePath).catch(() => null); + if (info === null || linkInfo === null) { + throw new SecretStoreRetireError( + `There is no encrypted secret store at ${input.storePath}; nothing was moved.` + ); + } + if (!linkInfo.isFile()) { + throw new SecretStoreRetireError( + `The encrypted secret store at ${input.storePath} is a symlink or not a regular file; nothing was moved. Inspect it with ls -l, then move it aside by hand if you mean to.` + ); + } + const keyInfo = input.keyPath + ? await stat(input.keyPath).catch(() => null) + : null; + if ( + keyInfo !== null && + keyInfo.dev === info.dev && + keyInfo.ino === info.ino + ) { + throw new SecretStoreRetireError( + `The encrypted secret store at ${input.storePath} is its own key file (${input.keyPath}); retiring it would remove the key. Nothing was moved. Fix controlPlane.secrets.encryptedFile so the store and its key are different files.` + ); + } +} + +/** Creates the archive directory exclusively (0700); refuses if the name exists. */ +async function createRetireDirectory(input: { + readonly storePath: string; + readonly dir: string; +}): Promise { + try { + await mkdir(input.dir, { mode: 0o700 }); + } catch (error: unknown) { + throw new SecretStoreRetireError( + errorCode(error) === "EEXIST" + ? `${input.dir} already exists; nothing was moved. Retry in a second.` + : `Could not create ${input.dir} (${errorCode(error)}); nothing was moved. To retire the store by hand: mkdir ${quoteShellWord(input.dir)} && mv ${quoteShellWord(input.storePath)} ${quoteShellWord(input.dir)}/` + ); + } +} + +/** + * Links the store into the archive directory (a hard link never replaces an + * existing file) and returns the archived file's identity. On failure the + * directory is removed only if it is still empty (rmdir refuses otherwise). + */ +async function linkIntoRetireDirectory(input: { + readonly storePath: string; + readonly dir: string; + readonly to: string; +}): Promise<{ readonly dev: number; readonly ino: number }> { + try { + await link(input.storePath, input.to); + const archived = await lstat(input.to); + return { dev: archived.dev, ino: archived.ino }; + } catch (error: unknown) { + const code = errorCode(error); + if (code === "EEXIST") { + // Something appeared at the archive name: leave it and the directory + // as they are, and suggest no move that could overwrite it. + throw new SecretStoreRetireError( + `A file appeared at ${input.to} while retiring; it and ${input.storePath} were left as they are, and nothing was moved. Inspect ${quoteShellWord(input.dir)}, then retry (a new directory name is used).` + ); + } + // The directory is still empty unless something else wrote into it; + // rmdir refuses otherwise. + const removed = await rmdir(input.dir).then( + () => true, + () => false + ); + throw new SecretStoreRetireError( + removed + ? `Could not link ${input.storePath} into ${input.dir} (${code}; this filesystem may not support hard links); nothing was moved. To retire it by hand without overwriting anything: mkdir ${quoteShellWord(input.dir)} && mv -n ${quoteShellWord(input.storePath)} ${quoteShellWord(input.dir)}/` + : `Could not link ${input.storePath} into ${input.dir} (${code}), and ${input.dir} is no longer empty, so it was left as it is; nothing was moved. Inspect it before retrying.` + ); + } +} + +/** + * Renames whatever is at the store path into the archive directory, and + * removes that extra name only when it is the archived file itself. A + * replacement written meanwhile by a writer outside the lock is kept, and + * its path returned. Both names are inside the directory this call made. + */ +async function moveSourceIntoRetireDirectory(input: { + readonly storePath: string; + readonly dir: string; + readonly archived: { readonly dev: number; readonly ino: number }; +}): Promise { + // An unpredictable name in the directory this call created, checked to + // be free first: the rename never replaces a file planted there. + const source = join(input.dir, `.source-${randomUUID()}`); + if ((await lstat(source).catch(() => null)) !== null) { + throw new SecretStoreRetireError( + `The store was archived at ${join(input.dir, basename(input.storePath))}, but ${source} appeared meanwhile, so ${input.storePath} was left in place. Nothing was deleted; remove ${quoteShellWord(input.storePath)} by hand once you have checked it.` + ); + } + try { + await rename(input.storePath, source); + } catch (error: unknown) { + if (errorCode(error) === "ENOENT") { + // Already gone: the archive holds the store; nothing else to keep. + return null; + } + throw new SecretStoreRetireError( + `The store was archived at ${join(input.dir, basename(input.storePath))}, but ${input.storePath} could not be moved (${errorCode(error)}), so both names hold it. Nothing was deleted. Remove ${quoteShellWord(input.storePath)} by hand once you have checked it, or keep the archive.` + ); + } + const moved = await lstat(source).catch(() => null); + if ( + moved?.isFile() === true && + moved.dev === input.archived.dev && + moved.ino === input.archived.ino + ) { + await unlink(source); + return null; + } + return source; +} + +function errorCode(error: unknown): string { + return typeof error === "object" && error !== null && "code" in error + ? String((error as { readonly code?: unknown }).code) + : "unknown"; +} + +/** + * Classifies the store file at an explicit `storePath` with an explicit + * `keyPath`, read-only (no key is provisioned): `empty` (absent or empty), + * `opens` (the available key decrypts it), or `unreadable` with the same + * reasons as {@link listFileSecretStoreEntries}. For a caller that has + * pinned a real path and must not follow a configured alias again + * (HACK-2177). + */ +export async function classifyEncryptedStoreAt(input: { + readonly storePath: string; + readonly keyPath: string; +}): Promise< + | { readonly kind: "empty" } + | { readonly kind: "opens" } + | { + readonly kind: "unreadable"; + readonly reason: Extract< + FileSecretStoreListing, + { kind: "unreadable" } + >["reason"]; + } +> { + const read = await readEncryptedStoreText(input.storePath); + if (read.kind === "refused" || read.kind === "unreadable") { + return { kind: "unreadable", reason: STORE_READ_FAILURE_REASON[read.kind] }; + } + if (read.kind === "absent" || read.text.length === 0) { + return { kind: "empty" }; + } + const opened = await openListedStore({ ...input, text: read.text }); + return "reason" in opened + ? { kind: "unreadable", reason: opened.reason } + : { kind: "opens" }; +} + +/** + * Whether the key in the file at `keyPath` alone opens the store at + * `storePath` (HACK-2177): the key file is read directly, without the + * `HACK_SECRETS_FILE_KEY` override or the keychain, so a wrong override in + * the calling shell cannot hide a key file that works. `unknown` when the + * key file or the store exists but cannot be read; an absent or empty key + * file opens nothing. + */ +export async function keyFileOpensStore(input: { + readonly storePath: string; + readonly keyPath: string; +}): Promise<"opens" | "does_not_open" | "absent" | "unknown"> { + const store = await readEncryptedStoreText(input.storePath); + if (store.kind !== "text" || store.text.length === 0) { + return store.kind === "absent" ? "does_not_open" : "unknown"; + } + let key: string; + try { + key = + ( + await readReplaceablePrivateFile({ + path: input.keyPath, + directories: [], + refusal: () => new Error("not a regular key file"), + }) + )?.text.trim() ?? ""; + } catch { + return "unknown"; + } + if (key.length === 0) { + return "absent"; + } + return storeTextOpensUnder({ + storePath: input.storePath, + text: store.text, + key, + }); +} + +/** + * Whether the encrypted_file key in the OS keychain (the runtime's fallback + * when a project has no key file) opens the store (HACK-2177). Asked + * directly, without the key resolution cooldown or cache. `unknown` when + * the keychain cannot be asked; `does_not_open` when the fallback is turned + * off or holds no key. + */ +export async function keychainKeyOpensStore(input: { + readonly storePath: string; +}): Promise<"opens" | "does_not_open" | "unknown"> { + if (isKeychainDisabled()) { + return "does_not_open"; + } + const store = await readEncryptedStoreText(input.storePath); + if (store.kind !== "text" || store.text.length === 0) { + return store.kind === "absent" ? "does_not_open" : "unknown"; + } + let key: string; + try { + key = + ( + await keychain.get({ + service: ENCRYPTED_FILE_KEY_SERVICE, + name: ENCRYPTED_FILE_KEY_NAME, + }) + )?.trim() ?? ""; + } catch { + return "unknown"; + } + if (key.length === 0) { + return "does_not_open"; + } + return storeTextOpensUnder({ + storePath: input.storePath, + text: store.text, + key, + }); +} + +function storeTextOpensUnder(input: { + readonly storePath: string; + readonly text: string; + readonly key: string; +}): "opens" | "does_not_open" { + try { + decryptEncryptedEntriesText({ + filePath: input.storePath, + text: input.text, + encryptionKey: input.key, + }); + return "opens"; + } catch { + return "does_not_open"; + } +} + +/** + * Whether the file at `storePath` is a Hack encrypted secret store: a + * regular file (read without following a link) holding the store's JSON + * envelope. Nothing is decrypted. A configured store path is project + * input, so anything else (any other file) is never treated as a store to + * retire (HACK-2177). + */ +export async function isEncryptedStoreFile( + storePath: string +): Promise { + const read = await readEncryptedStoreText(storePath); + if (read.kind !== "text") { + return false; + } + try { + return parseEncryptedEnvelope(JSON.parse(read.text)) !== null; + } catch { + return false; + } +} + +/** + * Retired copies of `storePath`, oldest first: `/` for + * each `.retired-/` directory (and a bare file of that name, + * as an earlier build of this command left). + */ +export async function listRetiredEncryptedStores( + storePath: string +): Promise { + const prefix = `${basename(storePath)}${RETIRED_STORE_INFIX}`; + const entries = await readdir(dirname(storePath), { + withFileTypes: true, + }).catch(() => []); + const candidates = entries + .filter((entry) => entry.name.startsWith(prefix)) + .sort((a, b) => a.name.localeCompare(b.name)) + .map((entry) => { + const path = resolve(dirname(storePath), entry.name); + return entry.isDirectory() ? join(path, basename(storePath)) : path; + }); + // Only archives still there: a directory emptied by a restore is not one. + const present: string[] = []; + for (const path of candidates) { + if ((await lstat(path).catch(() => null))?.isFile() === true) { + present.push(path); + } + } + return present; +} + /** The listing's reason for each way a store file read can fail. */ const STORE_READ_FAILURE_REASON = { refused: "store_refused", @@ -212,7 +678,8 @@ const STORE_READ_FAILURE_REASON = { */ export async function listFileSecretStoreEntries(input: { readonly projectName: string; - readonly projectDir: string; + /** Omitted: only the global configuration applies (no project). */ + readonly projectDir?: string; }): Promise { const controlPlane = await readControlPlaneConfig({ projectDir: input.projectDir, @@ -302,7 +769,12 @@ async function openListedStore(input: { }): Promise< | { readonly entries: Record } | { - readonly reason: "key_missing" | "key_refused" | "wrong_key_or_corrupt"; + readonly reason: + | "key_missing" + | "key_refused" + | "key_unreadable" + | "key_access_failed" + | "wrong_key_or_corrupt"; } > { let key: string; @@ -315,12 +787,25 @@ async function openListedStore(input: { }) ).key; } catch (error: unknown) { - return { - reason: - error instanceof SecretStoreKeyRefusedError - ? "key_refused" - : "key_missing", - }; + if (error instanceof SecretStoreKeyRefusedError) { + return { reason: "key_refused" }; + } + // A key file that exists but cannot be read is not missing: fixing its + // permissions may recover the store, which must not be retired. + if (await keyFileUnreadable(input.keyPath)) { + return { reason: "key_unreadable" }; + } + // Missing only when every place the key could be has answered "none": + // no (or an empty) key file, and the keychain says no item or is turned + // off. A keychain that could not be asked, or any other failure, leaves + // it unknown (HACK-2177). + if ( + !(error instanceof SecretStoreKeyUnavailableError) || + error instanceof SecretStoreKeyAccessError + ) { + return { reason: "key_access_failed" }; + } + return { reason: "key_missing" }; } try { return { @@ -335,6 +820,20 @@ async function openListedStore(input: { } } +/** True when the key file exists but this user cannot read it. */ +async function keyFileUnreadable(keyPath: string): Promise { + try { + await readFile(keyPath); + return false; + } catch (error: unknown) { + const code = + typeof error === "object" && error !== null && "code" in error + ? (error as { readonly code?: unknown }).code + : undefined; + return code !== "ENOENT" && code !== "ENOTDIR"; + } +} + function decodeScopedKey(encoded: string): string | null { try { return decodeURIComponent(encoded); @@ -1066,7 +1565,7 @@ async function resolveEncryptedFileKeyMaterial(input?: { cachedKeychainEncryptedFileKey === null && nowMs < encryptedFileKeyFailureCooldownUntilMs ) { - throw new SecretStoreKeyUnavailableError( + throw new SecretStoreKeyAccessError( buildEncryptedBackendKeyRecoveryMessage({ keyPath: configuredKeyPath, storePath: input?.storePath, @@ -1162,7 +1661,7 @@ function keychainKeyAccessError(input: { if (input.error instanceof NativeKeychainTimeoutError) { return input.error; } - return new SecretStoreKeyUnavailableError( + return new SecretStoreKeyAccessError( buildEncryptedBackendKeyRecoveryMessage({ keyPath: input.keyPath, storePath: input.storePath, diff --git a/src/lib/terminal-text.ts b/src/lib/terminal-text.ts new file mode 100644 index 000000000..010db94dd --- /dev/null +++ b/src/lib/terminal-text.ts @@ -0,0 +1,17 @@ +/** + * Terminal-safe text for one line (HACK-2177). Paths from project + * configuration are input: a carriage return, a newline or an escape + * sequence in one could rewrite or forge what the terminal shows (for + * example erase the real target from a confirmation prompt). Every C0 + * control character (newline and tab included), DEL, and every C1 control + * character is shown visibly as `\xNN`. Multi-line output escapes each line + * on its own, then joins them. + */ +const TERMINAL_CONTROL = /[\u0000-\u001f\u007f-\u009f]/g; + +export function escapeTerminalControls(text: string): string { + return text.replace( + TERMINAL_CONTROL, + (char) => `\\x${char.charCodeAt(0).toString(16).padStart(2, "0")}` + ); +} diff --git a/tests/missing-project-error-code.test.ts b/tests/missing-project-error-code.test.ts index d5e96e630..7580eb3af 100644 --- a/tests/missing-project-error-code.test.ts +++ b/tests/missing-project-error-code.test.ts @@ -221,6 +221,11 @@ const COMMANDS: Readonly> = { "setup cursor": { kind: "not-a-project-command", reason: NOT_REQUIRED }, "setup claude": { kind: "not-a-project-command", reason: NOT_REQUIRED }, "setup codex": { kind: "not-a-project-command", reason: NOT_REQUIRED }, + // Outside a project it retires the global store (HACK-2177). + "secrets retire-store": { + kind: "not-a-project-command", + reason: NOT_REQUIRED, + }, "setup agents": { kind: "not-a-project-command", reason: NOT_REQUIRED }, "setup sync": { kind: "not-a-project-command", reason: NOT_REQUIRED }, "setup migrate": { kind: "not-a-project-command", reason: NOT_REQUIRED }, diff --git a/tests/secret-store-retire.test.ts b/tests/secret-store-retire.test.ts new file mode 100644 index 000000000..03e2025cc --- /dev/null +++ b/tests/secret-store-retire.test.ts @@ -0,0 +1,1519 @@ +import { afterEach, beforeEach, expect, spyOn, test } from "bun:test"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + realpath, + rename, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { + buildDoctorRecoveryGuidance, + buildRecoveryWorkflowLines, +} from "../src/commands/recovery-guidance.ts"; +import { + PROJECT_COMPOSE_FILENAME, + PROJECT_CONFIG_FILENAME, + PROJECT_ENV_CONTRACT_FILENAME, +} from "../src/constants.ts"; +import { inspectSecretIntegrity } from "../src/lib/doctor-secret-integrity.ts"; +import { keychain } from "../src/lib/keychain.ts"; +import { findProjectContext } from "../src/lib/project.ts"; +import { upsertProjectRegistration } from "../src/lib/projects-registry.ts"; +import { withProjectsRegistryLock } from "../src/lib/projects-registry-lock.ts"; +import { + resolveSecretStore, + retireEncryptedStoreFile, +} from "../src/lib/secret-store.ts"; +import { + describeRetireTarget, + planSecretStoreRetire, + retireSecretStore, +} from "../src/lib/secret-store-retire.ts"; +import { escapeTerminalControls } from "../src/lib/terminal-text.ts"; +import { restoreEnv } from "./helpers/env.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +/** + * HACK-2177: a global encrypted secret store whose key file is gone. Every + * case runs in a fake home (HOME, HACK_HOME and the global config inside a + * temp root); no real store, key, keychain or registry is touched, and + * nothing is deleted except the temp root itself. Values are synthetic. + */ + +const ENTRYPOINT = resolve(import.meta.dir, "..", "index.ts"); +const STORE_VALUE = ["synthetic", "store", "value", "6d1f"].join("-"); +const STORE_KEY = ["synthetic", "store", "key", "92ab"].join("-"); +const ISOLATED_ENV = [ + "HOME", + "HACK_HOME", + "HACK_GLOBAL_CONFIG_PATH", + "HACK_SECRETS_FILE_KEY", + "HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", + "HACK_ENV_SECRET_KEY", +] as const; +const saved = new Map(); +let root = ""; + +type Fixture = { + readonly home: string; + readonly hackHome: string; + readonly storePath: string; + readonly keyPath: string; + readonly reader: { + readonly projectRoot: string; + readonly projectDir: string; + }; + readonly bystander: { + readonly projectRoot: string; + readonly projectDir: string; + }; +}; + +beforeEach(async () => { + for (const key of ISOLATED_ENV) { + saved.set(key, process.env[key]); + } + root = await realpath(await mkdtemp(join(tmpdir(), "hack-retire-store-"))); +}); + +afterEach(async () => { + for (const key of ISOLATED_ENV) { + restoreEnv(key, saved.get(key)); + } + await rm(root, { recursive: true, force: true }); +}); + +async function createProject(opts: { + readonly name: string; + readonly readsStore: boolean; +}) { + const projectRoot = join(root, "repos", opts.name); + const projectDir = join(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + join(projectDir, PROJECT_COMPOSE_FILENAME), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + join(projectDir, PROJECT_CONFIG_FILENAME), + `${JSON.stringify({ name: opts.name, dev_host: `${opts.name}.hack` })}\n` + ); + if (opts.readsStore) { + // A legacy env contract with a store-backed value: runtime reads the store. + await writeFile( + join(projectDir, PROJECT_ENV_CONTRACT_FILENAME), + `${JSON.stringify({ version: 1, vars: [{ key: "TOKEN", source: "keychain" }] })}\n` + ); + } + const project = await findProjectContext(projectRoot); + if (!project) { + throw new Error(`fixture project ${opts.name} not found`); + } + await upsertProjectRegistration({ project }); + return { projectRoot, projectDir }; +} + +/** A fake home with a global store whose key file has been removed. */ +async function setUp(): Promise { + const home = join(root, "user-home"); + const hackHome = join(home, ".hack"); + await mkdir(hackHome, { recursive: true }); + const storePath = join(hackHome, "secrets.enc.json"); + const keyPath = join(hackHome, "secrets-file.key"); + process.env.HOME = home; + process.env.HACK_HOME = hackHome; + process.env.HACK_GLOBAL_CONFIG_PATH = join(hackHome, "hack.config.json"); + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + Reflect.deleteProperty(process.env, "HACK_SECRETS_FILE_KEY"); + Reflect.deleteProperty(process.env, "HACK_ENV_SECRET_KEY"); + await writeFile( + process.env.HACK_GLOBAL_CONFIG_PATH, + `${JSON.stringify({ + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath, keyPath }, + }, + }, + })}\n` + ); + const reader = await createProject({ name: "app-a", readsStore: true }); + const bystander = await createProject({ name: "app-b", readsStore: false }); + await writeFile(keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(keyPath, 0o600); + await ( + await resolveSecretStore({ + projectName: "app-a", + projectDir: reader.projectDir, + }) + ).set({ key: "TOKEN", value: STORE_VALUE }); + await rm(keyPath); + return { home, hackHome, storePath, keyPath, reader, bystander }; +} + +async function inspect( + project: Fixture["reader"], + projectName: string +): Promise>> { + return await inspectSecretIntegrity({ ...project, projectName }); +} + +function expectValueFree(text: string) { + expect(text).not.toContain(STORE_VALUE); + expect(text).not.toContain(STORE_KEY); +} + +async function runRetire(opts: { + readonly fixture: Fixture; + readonly path: string; + readonly yes: boolean; + readonly allowUnchecked?: boolean; +}) { + const child = Bun.spawn( + [ + process.execPath, + ENTRYPOINT, + "secrets", + "retire-store", + "--path", + opts.path, + ...(opts.yes ? ["--yes"] : []), + ...(opts.allowUnchecked ? ["--allow-unchecked"] : []), + ], + { + cwd: root, + env: privateCliEnv({ + root, + home: opts.fixture.home, + env: { + HACK_HOME: opts.fixture.hackHome, + HACK_GLOBAL_CONFIG_PATH: join( + opts.fixture.hackHome, + "hack.config.json" + ), + HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: "1", + HACK_NO_INTERACTIVE: "1", + }, + }), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + } + ); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + return { code, output: `${stdout}\n${stderr}` }; +} + +test("a store whose key is missing is an error only for a project that reads it", async () => { + const fixture = await setUp(); + const reader = await inspect(fixture.reader, "app-a"); + expect(reader).toHaveLength(1); + expect(reader[0]?.status).toBe("error"); + expect(reader[0]?.message).toContain( + "its key ~/.hack/secrets-file.key is missing" + ); + expect(reader[0]?.message).toContain("This project's env contract reads it."); + + const bystander = await inspect(fixture.bystander, "app-b"); + expect(bystander).toHaveLength(1); + expect(bystander[0]?.status).toBe("warn"); + const message = bystander[0]?.message ?? ""; + expect(message).toContain("This project does not read it."); + expect(message).toContain("Registered projects that read it: app-a."); + expect(message).toContain("cannot be counted or read without that key"); + expect(message).toContain("restore the key"); + expect(message).toContain("hack secrets retire-store"); + expect(message).toContain("Do not provision a new key"); + expectValueFree(message); + + // The recovery block names the two choices. + const lines = buildRecoveryWorkflowLines({ + guidance: buildDoctorRecoveryGuidance({ + results: [{ name: "secret integrity", status: "warn", message }], + }), + projectRoot: null, + }).join("\n"); + expect(lines).toContain("Choose one:"); + expect(lines).toContain("Restore the encrypted secret store's key"); + expect(lines).toContain( + `\`hack secrets retire-store --path ${fixture.bystander.projectRoot}\`` + ); +}); + +test("retire-store refuses without --yes when it cannot prompt, then retires, and moving the file back restores it", async () => { + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + + // No project has a key file: any may set HACK_SECRETS_FILE_KEY in another + // shell, which cannot be checked, so this refuses without the flag. + const envOnly = await runRetire({ + fixture, + path: fixture.bystander.projectRoot, + yes: true, + }); + expect(envOnly.code).not.toBe(0); + expect(envOnly.output).toContain("HACK_SECRETS_FILE_KEY"); + expect(envOnly.output).toContain("--allow-unchecked"); + expect(await readFile(fixture.storePath)).toEqual(before); + + // Non-interactive and no --yes: refused, nothing moved. + const refused = await runRetire({ + fixture, + path: fixture.bystander.projectRoot, + yes: false, + allowUnchecked: true, + }); + expect(refused.code).not.toBe(0); + expect(refused.output).toContain("E_INTERACTIVE_REQUIRED"); + expect(refused.output).toContain("--yes"); + expect(await readFile(fixture.storePath)).toEqual(before); + + // With --yes: moved aside, never deleted or read. + const retired = await runRetire({ + fixture, + path: fixture.bystander.projectRoot, + yes: true, + allowUnchecked: true, + }); + expect(retired.code).toBe(0); + expectValueFree(retired.output); + expect(await lstat(fixture.storePath).catch(() => null)).toBeNull(); + const kept = (await readdir(fixture.hackHome)).filter((name) => + name.startsWith("secrets.enc.json.retired-") + ); + expect(kept).toHaveLength(1); + // The archive is the store, under its own name, in a new directory. + const retiredPath = join(fixture.hackHome, kept[0] ?? "", "secrets.enc.json"); + expect(await readFile(retiredPath)).toEqual(before); + expect(retired.output).toContain(`to ${retiredPath}`); + expect(retired.output).toContain(`mv ${retiredPath} ${fixture.storePath}`); + expect(retired.output).toContain("Nothing was deleted"); + + // doctor afterwards: the retired file is information, not a problem. + for (const [project, name] of [ + [fixture.bystander, "app-b"], + [fixture.reader, "app-a"], + ] as const) { + const after = await inspect(project, name); + expect(after).toHaveLength(1); + expect(after[0]?.status).toBe("info"); + expect(after[0]?.message).toContain( + `Retired secret store kept at ~/.hack/${kept[0]}/secrets.enc.json` + ); + expect(after[0]?.message).toContain( + `mv ${retiredPath} ${fixture.storePath}` + ); + } + + // Putting it back by moving the file restores the original state. + await rename(retiredPath, fixture.storePath); + expect(await readFile(fixture.storePath)).toEqual(before); + const restored = await inspect(fixture.bystander, "app-b"); + expect(restored).toHaveLength(1); + expect(restored[0]?.status).toBe("warn"); + expect(restored[0]?.message).toContain("is missing"); + // With the key back, the store opens again: nothing was lost. + await writeFile(fixture.keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(fixture.keyPath, 0o600); + expect(await inspect(fixture.reader, "app-a")).toEqual([]); + expect( + await ( + await resolveSecretStore({ + projectName: "app-a", + projectDir: fixture.reader.projectDir, + }) + ).get({ key: "TOKEN" }) + ).toBe(STORE_VALUE); +}, 60_000); + +test("retire-store refuses a store that opens, an absent store, and a key that is refused", async () => { + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + await writeFile(fixture.keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(fixture.keyPath, 0o600); + const opens = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(opens).toMatchObject({ kind: "refused" }); + expect(opens.kind === "refused" ? opens.message : "").toContain( + "opens with the key in use" + ); + + // A refused key (others can read it) is not a missing key: fix it instead. + await chmod(fixture.keyPath, 0o644); + const refusedKey = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(refusedKey.kind === "refused" ? refusedKey.message : "").toContain( + "key refused" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + + // No store at all: nothing to move. + await rm(fixture.keyPath); + await rename(fixture.storePath, join(root, "elsewhere.enc.json")); + const absent = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(absent.kind === "refused" ? absent.message : "").toContain( + "There is no encrypted secret store with entries" + ); + expect(await readFile(join(root, "elsewhere.enc.json"))).toEqual(before); +}); + +test("retire re-checks the store under its lock: a key restored meanwhile is not retired", async () => { + // Codex review: eligibility was decided before the store's lock, so a + // store whose key came back in between was still moved aside. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + let retire: ReturnType | undefined; + await withProjectsRegistryLock({ + lockPath: `${fixture.storePath}.lock`, + label: "test store holder", + run: async () => { + retire = retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }); + await Bun.sleep(300); + // Still waiting for the lock: the key comes back now. + await writeFile(fixture.keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(fixture.keyPath, 0o600); + }, + }); + if (!retire) { + throw new Error("retire not started"); + } + const outcome = await retire; + expect(outcome.kind).toBe("refused"); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "opens with the key in use" + ); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "It changed while retiring" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + const retired = (await readdir(fixture.hackHome)).filter((name) => + name.includes(".retired-") + ); + expect(retired).toEqual([]); +}); + +test("the retire command doctor and the recovery block name targets the diagnosed project", async () => { + // Codex review: an unscoped command acts on the current directory, not + // the project doctor (or crash capture) diagnosed with --path. + const fixture = await setUp(); + const finding = (await inspect(fixture.bystander, "app-b"))[0]; + const command = `hack secrets retire-store --path ${fixture.bystander.projectRoot}`; + expect(finding?.message).toContain(command); + const guidance = buildDoctorRecoveryGuidance({ + results: [ + { + name: "secret integrity", + status: "warn", + message: finding?.message ?? "", + }, + ], + }); + expect( + buildRecoveryWorkflowLines({ guidance, projectRoot: null }).join("\n") + ).toContain(`\`${command}\``); + // A finding without --path (an older wording) is scoped to the project. + const unscoped = buildDoctorRecoveryGuidance({ + results: [ + { + name: "secret integrity", + status: "warn", + message: "Run hack secrets retire-store to move it aside.", + }, + ], + }); + expect( + buildRecoveryWorkflowLines({ + guidance: unscoped, + projectRoot: "/srv/some project", + }).join("\n") + ).toContain("`hack secrets retire-store --path '/srv/some project'`"); +}); + +test("printed restore commands quote a store path that holds a quote", async () => { + // Codex security review: a configured store path is project input; a + // quote in it must not end the shell literal of the printed mv command. + const fixture = await setUp(); + const hostile = join(fixture.hackHome, "store'; touch PWNED; #'.enc.json"); + await rename(fixture.storePath, hostile); + const configPath = join(fixture.hackHome, "hack.config.json"); + await writeFile( + configPath, + `${JSON.stringify({ + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: hostile, keyPath: fixture.keyPath }, + }, + }, + })}\n` + ); + const before = await readFile(hostile); + const outcome = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }); + expect(outcome.kind).toBe("retired"); + const info = (await inspect(fixture.bystander, "app-b")).find( + (finding) => finding.status === "info" + ); + const command = info?.message.split(": ").at(-1) ?? ""; + expect(command.startsWith("mv ")).toBe(true); + // Run exactly what was printed, in the fixture root. + const shell = Bun.spawn(["sh", "-c", command], { + cwd: root, + stdout: "ignore", + stderr: "pipe", + }); + expect(await shell.exited).toBe(0); + expect(await readFile(hostile)).toEqual(before); + expect(await lstat(join(root, "PWNED")).catch(() => null)).toBeNull(); + expect( + await lstat(join(fixture.hackHome, "PWNED")).catch(() => null) + ).toBeNull(); + // The guidance extracts the scoped retire command whole. + const lines = buildRecoveryWorkflowLines({ + guidance: buildDoctorRecoveryGuidance({ + results: [ + { + name: "secret integrity", + status: "warn", + message: + "Run hack secrets retire-store --path '/srv/it'\\''s here' now.", + }, + ], + }), + projectRoot: null, + }).join("\n"); + expect(lines).toContain( + "`hack secrets retire-store --path '/srv/it'\\''s here'`" + ); +}); + +test("a key file that exists but cannot be read is not missing: the store is not retired", async () => { + // Codex review: an unreadable key file was classified as missing, so a + // store that fixing the key's permissions would recover was retired. + if (process.getuid?.() === 0) { + return; + } + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + await writeFile(fixture.keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(fixture.keyPath, 0o000); + try { + const findings = await inspect(fixture.bystander, "app-b"); + expect(findings).toHaveLength(1); + // Machine-wide: a warning here, an error where the store is read. + expect(findings[0]?.status).toBe("warn"); + expect(findings[0]?.message).toContain("exists but cannot be read"); + expect(findings[0]?.message).toContain("This project does not read it."); + expect((await inspect(fixture.reader, "app-a"))[0]?.status).toBe("error"); + expect(findings[0]?.message).not.toContain("hack secrets retire-store"); + const outcome = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind).toBe("refused"); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "key unreadable" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + } finally { + await chmod(fixture.keyPath, 0o600); + } + // Readable again: the store opens; nothing was lost. + expect(await inspect(fixture.bystander, "app-b")).toEqual([]); +}); + +test("a store replaced at the moment of the move is archived, never deleted, and an existing archive is never replaced", async () => { + // Second-family review blocker: link then unlink removed a replacement a + // writer outside the lock put at the store path between the two steps. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const replacement = Buffer.from("synthetic-replacement-store-bytes\n"); + const outcome = await retireEncryptedStoreFile({ + storePath: fixture.storePath, + keyPath: fixture.keyPath, + now: new Date("2026-11-13T12:00:00Z"), + beforeMove: async () => { + // A writer that ignores the lock swaps in a new file (new inode). + const staged = `${fixture.storePath}.staged`; + await writeFile(staged, replacement); + await rename(staged, fixture.storePath); + }, + }); + const dir = `${fixture.storePath}.retired-20261113T120000Z`; + expect(outcome.to).toBe(join(dir, "secrets.enc.json")); + // Neither file is lost: the archive holds the store that was linked, and + // the replacement put at the store path meanwhile is kept beside it. + expect(await readFile(outcome.to)).toEqual(before); + expect(outcome.replacement?.startsWith(join(dir, ".source-"))).toBe(true); + expect(await readFile(outcome.replacement ?? "")).toEqual(replacement); + expect(await lstat(fixture.storePath).catch(() => null)).toBeNull(); + + // Without a replacement, the extra name is removed: only the archive. + const plain = join(fixture.hackHome, "plain.enc.json"); + await writeFile(plain, "synthetic-plain-store\n"); + const plainOutcome = await retireEncryptedStoreFile({ + storePath: plain, + now: new Date("2026-11-13T12:00:00Z"), + }); + expect(plainOutcome.replacement).toBeNull(); + expect(await readdir(`${plain}.retired-20261113T120000Z`)).toEqual([ + "plain.enc.json", + ]); + + // An archive name that already exists is never replaced. + const second = join(fixture.hackHome, "secrets.enc.json"); + await writeFile(second, "synthetic-second-store\n"); + await expect( + retireEncryptedStoreFile({ + storePath: second, + now: new Date("2026-11-13T12:00:00Z"), + }) + ).rejects.toThrow("already exists; nothing was moved"); + expect(await readFile(outcome.to)).toEqual(before); + expect(await readFile(outcome.replacement ?? "")).toEqual(replacement); + expect(await readFile(second, "utf8")).toBe("synthetic-second-store\n"); + + // A file planted at the archive name, inside the new directory, before + // the link: the link refuses, and nothing is removed or replaced. + const third = join(fixture.hackHome, "third.enc.json"); + await writeFile(third, "synthetic-third-store\n"); + let planted = ""; + await expect( + retireEncryptedStoreFile({ + storePath: third, + now: new Date("2026-11-13T12:00:00Z"), + afterDirectory: async (made) => { + planted = join(made, "third.enc.json"); + await writeFile(planted, "synthetic-planted\n"); + }, + }) + ).rejects.toThrow("left as they are"); + expect(await readFile(planted, "utf8")).toBe("synthetic-planted\n"); + expect(await readFile(third, "utf8")).toBe("synthetic-third-store\n"); + // That refusal suggests no move that could overwrite the planted file. + const collision = await retireEncryptedStoreFile({ + storePath: third, + now: new Date("2026-11-13T12:00:01Z"), + afterDirectory: async (made) => { + await writeFile(join(made, "third.enc.json"), "synthetic-planted\n"); + }, + }).catch((error: unknown) => String(error)); + expect(collision).not.toContain("mv "); + + // Any other link failure: the empty directory is removed, and the manual + // step makes a fresh one and moves without overwriting (mv -n). + const fourth = join(fixture.hackHome, "fourth.enc.json"); + await writeFile(fourth, "synthetic-fourth-store\n"); + const fourthDir = `${fourth}.retired-20261113T120000Z`; + const other = await retireEncryptedStoreFile({ + storePath: fourth, + now: new Date("2026-11-13T12:00:00Z"), + afterDirectory: async () => { + // The store vanishes, so the link fails with ENOENT, not EEXIST. + await rename(fourth, `${fourth}.away`); + }, + }).catch((error: unknown) => String(error)); + expect(other).toContain( + `mkdir ${fourthDir} && mv -n ${fourth} ${fourthDir}/` + ); + expect(await lstat(fourthDir).catch(() => null)).toBeNull(); +}); + +test("a store that is its own key file is not retired, through any spelling", async () => { + // Second-family review: path and keyPath naming the same file (directly + // or through a symlinked directory) made the key file retirable. + const fixture = await setUp(); + await writeFile(fixture.keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(fixture.keyPath, 0o600); + const alias = join(fixture.home, "key-alias"); + await symlink(fixture.hackHome, alias); + for (const path of [fixture.keyPath, join(alias, "secrets-file.key")]) { + await expect( + retireEncryptedStoreFile({ storePath: path, keyPath: fixture.keyPath }) + ).rejects.toThrow("is its own key file"); + expect(await readFile(fixture.keyPath, "utf8")).toBe(`${STORE_KEY}\n`); + } +}); + +test("retire refuses when the config that selects the store cannot be read or does not parse", async () => { + // Second-family review: an unreadable or malformed project config was + // taken as absent, so the global store could be retired instead. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const projectConfig = join(fixture.bystander.projectDir, "hack.config.json"); + if (process.getuid?.() !== 0) { + await chmod(projectConfig, 0o000); + try { + const outcome = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "exists but cannot be read" + ); + } finally { + await chmod(projectConfig, 0o644); + } + } + const projectText = await readFile(projectConfig, "utf8"); + await writeFile(projectConfig, "{ not json"); + const malformed = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(malformed.kind === "refused" ? malformed.message : "").toContain( + "does not parse" + ); + await writeFile(projectConfig, projectText); + expect(await readFile(fixture.storePath)).toEqual(before); +}); + +test("doctor says when the reader scan is incomplete, and finds readers through an alias", async () => { + // Second-family review: scan failures became "no registered project + // reads it", and a reader reaching the store through a symlinked + // directory was missed. + const fixture = await setUp(); + // app-a reaches the store through a symlinked directory instead. + const alias = join(fixture.home, "hack-alias"); + await symlink(fixture.hackHome, alias); + await writeFile( + join(fixture.reader.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-a", + dev_host: "app-a.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { + path: join(alias, "secrets.enc.json"), + keyPath: fixture.keyPath, + }, + }, + }, + })}\n` + ); + const aliased = await inspect(fixture.bystander, "app-b"); + expect(aliased[0]?.message).toContain( + "Registered projects that read it: app-a." + ); + + // A registered project whose config cannot be parsed is not "does not read it". + await writeFile(join(fixture.reader.projectDir, "hack.config.json"), "{ bad"); + const partial = await inspect(fixture.bystander, "app-b"); + expect(partial[0]?.message).toContain( + "No checked registered project reads it, but 1 registered project(s) could not be checked" + ); + expect(partial[0]?.message).not.toContain("No registered project reads it."); +}); + +test("a keychain that cannot be asked is not a missing key: the store is not retired", async () => { + // Codex review: with no key file and the keychain locked, denied or + // timed out, the key's existence is unknown, but it was classified as + // missing and the store could be retired. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "0"; + // The keychain answers "no such item": the key is missing (retirable). + const empty = spyOn(keychain, "get").mockImplementation(async () => null); + try { + const missing = await inspect(fixture.bystander, "app-b"); + expect(missing[0]?.message).toContain("is missing"); + expect(missing[0]?.message).toContain("hack secrets retire-store"); + } finally { + empty.mockRestore(); + } + // The keychain cannot be asked: unknown, not retirable. + const denied = spyOn(keychain, "get").mockImplementation(async () => { + throw new Error("synthetic keychain access denied"); + }); + try { + const unknown = await inspect(fixture.bystander, "app-b"); + expect(unknown).toHaveLength(1); + expect(unknown[0]?.message).toContain("could not be looked up"); + expect(unknown[0]?.message).not.toContain("hack secrets retire-store"); + // The failure cooldown that follows is unknown too, never missing. + const outcome = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "key access failed" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + } finally { + denied.mockRestore(); + // A successful resolution ends the module's failure cooldown, so no + // later test inherits it. + process.env.HACK_SECRETS_FILE_KEY = STORE_KEY; + await resolveSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + Reflect.deleteProperty(process.env, "HACK_SECRETS_FILE_KEY"); + } +}); + +test("a configured store path that is not a Hack store is never offered for retirement or retired", async () => { + // Codex security review: the path is project input; any non-empty file + // with no key counted as a keyless store, so a project could point it at + // a host file (such as an authorized_keys file) and have it moved aside. + const fixture = await setUp(); + const hostFile = join(fixture.home, ".ssh", "authorized_keys"); + await mkdir(join(fixture.home, ".ssh"), { recursive: true }); + await writeFile(hostFile, "synthetic-authorized-key-line\n"); + await writeFile( + join(fixture.hackHome, "hack.config.json"), + `${JSON.stringify({ + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: hostFile, keyPath: fixture.keyPath }, + }, + }, + })}\n` + ); + const findings = await inspect(fixture.bystander, "app-b"); + expect(findings).toHaveLength(1); + expect(findings[0]?.message).toContain( + "does not hold a Hack encrypted secret store" + ); + expect(findings[0]?.message).not.toContain("hack secrets retire-store"); + const outcome = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "is not a Hack encrypted secret store" + ); + expect(await readFile(hostFile, "utf8")).toBe( + "synthetic-authorized-key-line\n" + ); + expect( + (await readdir(join(fixture.home, ".ssh"))).filter((name) => + name.includes(".retired-") + ) + ).toEqual([]); +}); + +test("printed chmod and git rm guidance quotes a hostile key path", async () => { + // Codex security review: the key path is project input; the new + // key_unreadable guidance printed `chmod 600 ` unquoted. + if (process.getuid?.() === 0) { + return; + } + const fixture = await setUp(); + const hostileKey = join(fixture.hackHome, "key'; touch PWNED; #'"); + await writeFile(hostileKey, `${STORE_KEY}\n`, { mode: 0o600 }); + await writeFile( + join(fixture.hackHome, "hack.config.json"), + `${JSON.stringify({ + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: fixture.storePath, keyPath: hostileKey }, + }, + }, + })}\n` + ); + const quoted = `'${hostileKey.replaceAll("'", "'\\''")}'`; + await chmod(hostileKey, 0o000); + try { + const unreadable = (await inspect(fixture.reader, "app-a"))[0]; + expect(unreadable?.message).toContain(`chmod 600 ${quoted}`); + } finally { + await chmod(hostileKey, 0o600); + } + // Open to others: refused, with quoted chmod and git rm. + await chmod(hostileKey, 0o644); + const refused = (await inspect(fixture.reader, "app-a"))[0]; + expect(refused?.message).toContain(`chmod 600 ${quoted}`); + expect(refused?.message).toContain(`git rm --cached ${quoted}`); +}); + +test('a malformed registry, or a store identity that cannot be read, is never "no registered project reads it"', async () => { + // Second-family review: an invalid registry read as empty, and an + // identity lookup failure counted as "different", both produced the + // definitive no-readers wording. + const fixture = await setUp(); + const registry = join(fixture.hackHome, "projects.json"); + const text = await readFile(registry, "utf8"); + await writeFile(registry, "{ not a registry"); + try { + const malformed = (await inspect(fixture.bystander, "app-b"))[0]; + expect(malformed?.message).toContain( + "The projects registry could not be read or is not valid" + ); + expect(malformed?.message).not.toContain("No registered project reads it."); + } finally { + await writeFile(registry, text); + } + if (process.getuid?.() === 0) { + return; + } + // app-a reaches the store through an alias whose directory cannot be + // searched: its identity is unknown, so it counts as unchecked. + const hidden = join(fixture.home, "hidden"); + await mkdir(hidden); + await symlink(fixture.hackHome, join(hidden, "alias")); + await writeFile( + join(fixture.reader.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-a", + dev_host: "app-a.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { + path: join(hidden, "alias", "secrets.enc.json"), + keyPath: fixture.keyPath, + }, + }, + }, + })}\n` + ); + await chmod(hidden, 0o000); + try { + const unknown = (await inspect(fixture.bystander, "app-b"))[0]; + expect(unknown?.message).toContain("could not be checked"); + expect(unknown?.message).not.toContain("No registered project reads it."); + } finally { + await chmod(hidden, 0o755); + } +}); + +test("registry entries that are not valid projects make the reader scan incomplete", async () => { + // Second-family review: the strict read still skipped invalid project + // entries silently, so a filtered list read as complete. + const fixture = await setUp(); + const registry = join(fixture.hackHome, "projects.json"); + const text = await readFile(registry, "utf8"); + const parsed = JSON.parse(text) as { projects: unknown[] }; + // Mixed: the valid entries plus one that is not a project. + await writeFile( + registry, + JSON.stringify({ ...parsed, projects: [...parsed.projects, { id: 5 }] }) + ); + const mixed = (await inspect(fixture.bystander, "app-b"))[0]; + expect(mixed?.message).toContain("Registered projects that read it: app-a."); + expect(mixed?.message).toContain( + "1 registered project(s) could not be checked" + ); + // Only an invalid entry: never "No registered project reads it." + await writeFile( + registry, + JSON.stringify({ ...parsed, projects: [{ id: 5 }] }) + ); + const only = (await inspect(fixture.bystander, "app-b"))[0]; + expect(only?.message).toContain("could not be checked"); + expect(only?.message).not.toContain("No registered project reads it."); + await writeFile(registry, text); +}); + +test("a store reached through a symlinked directory is named and retired by its real path", async () => { + // Codex security review: lstat checked only the last component, so a + // repo-controlled symlinked directory could lead to another store while + // the prompt showed the alias. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const alias = join(fixture.bystander.projectRoot, "linked"); + await symlink(fixture.hackHome, alias); + const configured = join(alias, "secrets.enc.json"); + await writeFile( + join(fixture.bystander.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-b", + dev_host: "app-b.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: configured, keyPath: fixture.keyPath }, + }, + }, + })}\n` + ); + // The confirmation names the real file, and the configured alias. + expect( + await describeRetireTarget({ + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }) + ).toBe(`${fixture.storePath} (configured as ${configured})`); + const outcome = await retireSecretStore({ + projectName: "app-b", + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + now: new Date("2026-11-13T12:00:00Z"), + }); + expect(outcome.kind).toBe("retired"); + if (outcome.kind !== "retired") { + return; + } + // It acted on the real path: the archive sits beside the real store. + expect(outcome.from).toBe(fixture.storePath); + expect(outcome.configured).toBe(configured); + expect(outcome.to).toBe( + join(`${fixture.storePath}.retired-20261113T120000Z`, "secrets.enc.json") + ); + expect(await readFile(outcome.to)).toEqual(before); +}); + +/** + * Two keyless stores, A (the global store) and B (a copy elsewhere), and + * app-b configured through a repo-local symlinked directory that leads to + * A. Returns how to retarget that directory to B. + */ +async function aliasedStores(fixture: Fixture) { + const before = await readFile(fixture.storePath); + const otherDir = join(root, "other-store-dir"); + await mkdir(otherDir); + const storeB = join(otherDir, "secrets.enc.json"); + await writeFile(storeB, before); + const alias = join(fixture.bystander.projectRoot, "linked"); + await symlink(fixture.hackHome, alias); + await writeFile( + join(fixture.bystander.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-b", + dev_host: "app-b.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { + path: join(alias, "secrets.enc.json"), + keyPath: fixture.keyPath, + }, + }, + }, + })}\n` + ); + return { + before, + storeB, + retargetToB: async () => { + await rm(alias); + await symlink(otherDir, alias); + }, + }; +} + +test("a target retargeted after confirmation is not retired", async () => { + // Second-family review: the prompt resolved the real path for display, + // and the retirement resolved it again; a symlink retargeted while the + // prompt waited made it retire a store the user had not approved. + const fixture = await setUp(); + const { before, storeB, retargetToB } = await aliasedStores(fixture); + const planned = await planSecretStoreRetire({ + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }); + expect(planned.kind).toBe("plan"); + if (planned.kind !== "plan") { + return; + } + expect(planned.plan.canonical).toBe(fixture.storePath); + // While the confirmation waits, the repo's symlink is pointed at B. + await retargetToB(); + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + approved: planned.plan, + allowUnchecked: true, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "changed after it was confirmed" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + expect(await readFile(storeB)).toEqual(before); +}); + +test("the locked eligibility check reads the pinned store, not the alias", async () => { + // Second-family review: under the lock, eligibility was read through the + // alias after the path check; a retarget then validated B while A (whose + // key had just come back) was moved. + const fixture = await setUp(); + const { before, storeB, retargetToB } = await aliasedStores(fixture); + const planned = await planSecretStoreRetire({ + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }); + if (planned.kind !== "plan") { + throw new Error("expected a plan"); + } + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + approved: planned.plan, + allowUnchecked: true, + beforeEligibility: async () => { + // After the path check: A's key comes back, and the alias moves to B. + await writeFile(fixture.keyPath, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(fixture.keyPath, 0o600); + await retargetToB(); + }, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "opens with the key in use" + ); + // A, now recoverable, stays active; B is untouched. + expect(await readFile(fixture.storePath)).toEqual(before); + expect(await readFile(storeB)).toEqual(before); + expect( + (await readdir(fixture.hackHome)).filter((name) => + name.includes(".retired-") + ) + ).toEqual([]); +}); + +test("control characters in a configured store path are shown, never sent to the terminal", async () => { + // Codex security review: a symlinked directory named with CR and an + // erase-line sequence could rewrite the confirmation on screen. + expect(escapeTerminalControls("a\r\u001b[2Kb\nc\td\u009b")).toBe( + "a\\x0d\\x1b[2Kb\\x0ac\\x09d\\x9b" + ); + const fixture = await setUp(); + const alias = join(fixture.bystander.projectRoot, "lin\r\u001b[2Kked"); + await symlink(fixture.hackHome, alias); + await writeFile( + join(fixture.bystander.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-b", + dev_host: "app-b.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { + path: join(alias, "secrets.enc.json"), + keyPath: fixture.keyPath, + }, + }, + }, + })}\n` + ); + const raw = /[\u0000-\u0008\u000b-\u001f\u007f-\u009f]/; + // Doctor's finding names the path with its controls escaped. + const findings = await inspect(fixture.bystander, "app-b"); + for (const finding of findings) { + expect(raw.test(finding.message)).toBe(false); + } + // The real command's output (the moved paths) is escaped too. + const retired = await runRetire({ + fixture, + path: fixture.bystander.projectRoot, + yes: true, + allowUnchecked: true, + }); + expect(retired.code).toBe(0); + expect(retired.output).toContain("lin\\x0d\\x1b[2Kked"); + expect(raw.test(retired.output.replaceAll("\u001b[", "ESC["))).toBe(false); +}); + +test("a configured store path that is itself a symlink is not retired", async () => { + // Codex review: resolving a final-component symlink moved its target + // and left a dangling link at the configured path, which runtime and + // doctor refuse as a store. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const link = join(fixture.bystander.projectRoot, "store-link.enc.json"); + await symlink(fixture.storePath, link); + await writeFile( + join(fixture.bystander.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-b", + dev_host: "app-b.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: link, keyPath: fixture.keyPath }, + }, + }, + })}\n` + ); + const planned = await planSecretStoreRetire({ + projectDir: fixture.bystander.projectDir, + }); + expect(planned.kind === "refused" ? planned.message : "").toContain( + "is itself a symlink" + ); + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind).toBe("refused"); + expect(await readFile(fixture.storePath)).toEqual(before); + expect((await lstat(link)).isSymbolicLink()).toBe(true); +}); + +test("a registered worktree that reads the store is named, even when its primary checkout does not", async () => { + // Codex review: linked worktrees are registered under their project, not + // as projects; the scan checked only the primary checkout. + const fixture = await setUp(); + // A worktree of app-b whose env contract reads the store. + const worktreeRoot = join(root, "repos", "app-b-feature"); + const worktreeDir = join(worktreeRoot, ".hack"); + await mkdir(worktreeDir, { recursive: true }); + await writeFile( + join(worktreeDir, "hack.config.json"), + `${JSON.stringify({ name: "app-b", dev_host: "app-b.hack" })}\n` + ); + await writeFile( + join(worktreeDir, "hack.env.json"), + `${JSON.stringify({ version: 1, vars: [{ key: "TOKEN", source: "keychain" }] })}\n` + ); + const registry = join(fixture.hackHome, "projects.json"); + const parsed = JSON.parse(await readFile(registry, "utf8")) as { + projects: { name: string; worktrees?: unknown[] }[]; + }; + for (const project of parsed.projects) { + if (project.name === "app-b") { + project.worktrees = [ + { + path: worktreeRoot, + branch: "feature", + lastSeenAt: "2026-11-13T12:00:00.000Z", + }, + ]; + } + } + await writeFile(registry, JSON.stringify(parsed)); + // Only app-a's primary checkout and app-b's worktree read the store. + const finding = (await inspect(fixture.bystander, "app-b"))[0]; + expect(finding?.message).toContain( + "Registered projects that read it: app-a, app-b (worktree app-b-feature)." + ); + // A worktree whose config cannot be read is unchecked, not "no reader". + await writeFile(join(worktreeDir, "hack.config.json"), "{ bad"); + const partial = (await inspect(fixture.bystander, "app-b"))[0]; + expect(partial?.message).toContain( + "1 registered project(s) could not be checked" + ); +}); + +test("a registered checkout that is not available counts as unchecked", async () => { + // Codex review: a missing project directory (an unmounted volume) read + // as a checkout with no config and no contract, so a non-reader. + const fixture = await setUp(); + const registry = join(fixture.hackHome, "projects.json"); + const parsed = JSON.parse(await readFile(registry, "utf8")) as { + projects: { name: string; worktrees?: unknown[] }[]; + }; + for (const project of parsed.projects) { + if (project.name === "app-a") { + project.worktrees = [ + { + path: join(root, "unmounted", "app-a-feature"), + branch: "feature", + lastSeenAt: "2026-11-13T12:00:00.000Z", + }, + ]; + } + } + await writeFile(registry, JSON.stringify(parsed)); + const finding = (await inspect(fixture.bystander, "app-b"))[0]; + expect(finding?.message).toContain( + "Registered projects that read it: app-a." + ); + expect(finding?.message).toContain( + "1 registered project(s) could not be checked" + ); +}); + +test("a store another registered project's key still opens is not retired", async () => { + // Codex security review: retire asked only whether this project's key + // opens the store; another project with its own key still used it. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const appAKey = join(fixture.reader.projectRoot, ".hack-store.key"); + await writeFile(appAKey, `${STORE_KEY}\n`, { mode: 0o600 }); + await chmod(appAKey, 0o600); + await writeFile( + join(fixture.reader.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-a", + dev_host: "app-a.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: fixture.storePath, keyPath: appAKey }, + }, + }, + })}\n` + ); + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "still opens with a configured key file (app-a)" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + + // A wrong HACK_SECRETS_FILE_KEY in this shell does not hide app-a's key + // file: key files are read directly, not through the override. + process.env.HACK_SECRETS_FILE_KEY = "synthetic-wrong-override"; + try { + const masked = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(masked.kind === "refused" ? masked.message : "").toContain( + "still opens with a configured key file (app-a)" + ); + } finally { + Reflect.deleteProperty(process.env, "HACK_SECRETS_FILE_KEY"); + } + expect(await readFile(fixture.storePath)).toEqual(before); + + // app-a's key file cannot be read: not tested, so --allow-unchecked is + // needed rather than taking it as "does not open". + if (process.getuid?.() !== 0) { + await chmod(appAKey, 0o000); + try { + const unknown = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(unknown.kind === "refused" ? unknown.message : "").toContain( + "configured key(s)" + ); + expect(unknown.kind === "refused" ? unknown.message : "").toContain( + "--allow-unchecked" + ); + } finally { + await chmod(appAKey, 0o600); + } + expect(await readFile(fixture.storePath)).toEqual(before); + } +}); + +test("a project with a wrong key file may still open the store through HACK_SECRETS_FILE_KEY: not retired without --allow-unchecked", async () => { + // Codex security review: HACK_SECRETS_FILE_KEY overrides key files and the + // keychain at runtime, and may be set in another project's shell. A key + // file that does not open the store is no proof that project cannot. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const appAKey = join(fixture.reader.projectRoot, ".hack-store.key"); + await writeFile(appAKey, "synthetic-wrong-key-file\n", { mode: 0o600 }); + await chmod(appAKey, 0o600); + await writeFile( + join(fixture.reader.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-a", + dev_host: "app-a.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: fixture.storePath, keyPath: appAKey }, + }, + }, + })}\n` + ); + const refused = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + const message = refused.kind === "refused" ? refused.message : ""; + expect(message).toContain("HACK_SECRETS_FILE_KEY overrides both"); + expect(message).toContain("app-a"); + expect(message).toContain("--allow-unchecked"); + expect(await readFile(fixture.storePath)).toEqual(before); + const allowed = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }); + expect(allowed.kind).toBe("retired"); +}); + +test("retire refuses when registered projects could not be checked, unless --allow-unchecked", async () => { + // The store may be another checkout's that could not be checked. + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + const registry = join(fixture.hackHome, "projects.json"); + const parsed = JSON.parse(await readFile(registry, "utf8")) as { + projects: { name: string; worktrees?: unknown[] }[]; + }; + for (const project of parsed.projects) { + if (project.name === "app-a") { + project.worktrees = [ + { + path: join(root, "unmounted", "app-a-feature"), + branch: "feature", + lastSeenAt: "2026-11-13T12:00:00.000Z", + }, + ]; + } + } + await writeFile(registry, JSON.stringify(parsed)); + const refused = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(refused.kind === "refused" ? refused.message : "").toContain( + "--allow-unchecked" + ); + expect(await readFile(fixture.storePath)).toEqual(before); + const allowed = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + allowUnchecked: true, + }); + expect(allowed.kind).toBe("retired"); +}); + +test("a store another project opens through the OS keychain is not retired", async () => { + // Codex review: a project with no key file falls back to the keychain; + // an absent key file was taken as "does not open". + const fixture = await setUp(); + const before = await readFile(fixture.storePath); + // app-b has its own, wrong key file; app-a has none (keychain fallback). + const wrongKey = join(fixture.bystander.projectRoot, ".wrong.key"); + await writeFile(wrongKey, "synthetic-wrong-key\n", { mode: 0o600 }); + await chmod(wrongKey, 0o600); + await writeFile( + join(fixture.bystander.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-b", + dev_host: "app-b.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: fixture.storePath, keyPath: wrongKey }, + }, + }, + })}\n` + ); + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "0"; + const holds = spyOn(keychain, "get").mockImplementation( + async () => STORE_KEY + ); + try { + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "app-a, through the OS keychain" + ); + } finally { + holds.mockRestore(); + } + // A keychain that cannot be asked: unchecked, not "does not open". + const locked = spyOn(keychain, "get").mockImplementation(async () => { + throw new Error("synthetic keychain locked"); + }); + try { + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "--allow-unchecked" + ); + } finally { + locked.mockRestore(); + } + expect(await readFile(fixture.storePath)).toEqual(before); +}); + +test("an invalid worktree entry inside a valid project makes the reader scan incomplete", async () => { + // Codex review: the strict read counted rejected project entries but not + // rejected worktree entries, which were dropped silently. + const fixture = await setUp(); + const registry = join(fixture.hackHome, "projects.json"); + const parsed = JSON.parse(await readFile(registry, "utf8")) as { + projects: { name: string; worktrees?: unknown[] }[]; + }; + for (const project of parsed.projects) { + if (project.name === "app-a") { + // No lastSeenAt: the parser skips it. + project.worktrees = [{ path: join(root, "repos", "app-a-feature") }]; + } + } + await writeFile(registry, JSON.stringify(parsed)); + const finding = (await inspect(fixture.bystander, "app-b"))[0]; + expect(finding?.message).toContain( + "1 registered project(s) could not be checked" + ); + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "--allow-unchecked" + ); +}); + +test("a configured store path that cannot be inspected is refused, not taken as no symlink", async () => { + // Second-family review: an lstat failure other than "absent" fell + // through as "not a symlink". + const fixture = await setUp(); + const unreadable = join(fixture.hackHome, `${"x".repeat(300)}.enc.json`); + await writeFile( + join(fixture.bystander.projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "app-b", + dev_host: "app-b.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: unreadable, keyPath: fixture.keyPath }, + }, + }, + })}\n` + ); + const planned = await planSecretStoreRetire({ + projectDir: fixture.bystander.projectDir, + }); + expect(planned.kind === "refused" ? planned.message : "").toContain( + "could not be inspected (ENAMETOOLONG)" + ); +}); + +test("a registry worktrees field that is not an array makes the reader scan incomplete", async () => { + // Second-family review: a present but malformed worktrees container was + // read as "no worktrees", hiding any checkout it held. + const fixture = await setUp(); + const registry = join(fixture.hackHome, "projects.json"); + const text = await readFile(registry, "utf8"); + for (const malformed of [ + { + path: join(root, "repos", "app-a-feature"), + lastSeenAt: "2026-11-13T12:00:00.000Z", + }, + "not-a-list", + ]) { + const parsed = JSON.parse(text) as { + projects: { name: string; worktrees?: unknown }[]; + }; + for (const project of parsed.projects) { + if (project.name === "app-a") { + project.worktrees = malformed; + } + } + await writeFile(registry, JSON.stringify(parsed)); + const finding = (await inspect(fixture.bystander, "app-b"))[0]; + expect(finding?.message).toContain( + "1 registered project(s) could not be checked" + ); + const outcome = await retireSecretStore({ + projectDir: fixture.bystander.projectDir, + }); + expect(outcome.kind === "refused" ? outcome.message : "").toContain( + "--allow-unchecked" + ); + } +});