diff --git a/docs/env.md b/docs/env.md index 9a85ea136..a1ff9f68f 100644 --- a/docs/env.md +++ b/docs/env.md @@ -109,7 +109,17 @@ How it is maintained: key is created. Before a `--secret` set or hydrate uses an existing checkout-local key that git does not ignore, Hack adds the entry; if it cannot, or a later rule re-includes the key, the command is refused and - nothing is encrypted with that key + nothing is encrypted with that key. The refusal names the cause: either the + entry could not be written, or the file already lists the key and a later + rule (such as `!.hack.secret.key`) re-includes it, which you need to remove +- outside git there is nothing to commit, so an existing key does not need the + entry and Hack does not recreate a deleted root `.gitignore` for it. Hack + does not run when you later turn the directory into a repository with + `git init`, so it cannot add the entry at that moment. If the entry is + missing then, `hack doctor` reports `env-key-unignored` (see + [Key strength and exposure](#secret-key-handling)), and the first `--secret` set or + hydrate in the new repository adds the entry before it uses the key, or is + refused - older repos that added `.hack/.internal/` to the root `.gitignore` keep working; the entries are equivalent and nothing is removed @@ -669,9 +679,19 @@ file, change it and replace it while holding that file's write lock, The same applies to the legacy `.hack/.env` files. So when several commands change different keys of the same file at once, every change is kept. A command that finds the lock held waits up to 10 seconds, then stops with a one-line message that -names the file and says nothing was written; run it again. The lock is released when the -command ends. If a command is killed while it holds the lock, the next command takes the -lock over once it confirms that process has exited. +names the file and its lock and says nothing was written; run it again. The lock is +released when the command ends. If a command is killed while it holds the lock, the next +command takes the lock over once it confirms that process has exited. + +Hack never removes a lock it cannot prove is abandoned. That includes a lock file that +is not a complete Hack receipt, one owned by another user or writable by others, and one +that names a process ID now used by an unrelated live process. In those cases every +retry would wait and fail the same way, so the message also says what to do: if no +`hack env` command is running, remove the lock file it names and retry. If a command was +killed while it was taking over an abandoned lock, it leaves `.recovery` behind, +and the message says the recovery guard is busy and names both paths; when no `hack env` +command is running, remove both and retry. `hack env hydrate` reports the same message +when it cannot get the env file's lock. `.hack/.internal/` is ignored by the managed `.hack/.gitignore`, so lock files never show in `git status`. If `.hack/.internal` is a symlink or not a directory, these commands @@ -926,7 +946,7 @@ it cannot spell portably as `/.hack.secret.key`. | `env-key-wrong-owner` | warn | the file is not owned by the current user | | `env-key-wrong-length` | warn | shorter than the 43-character base64url form of a 256-bit key, or empty | | `env-key-low-entropy` | warn | long enough but repetitive (estimated under 128 bits) | -| `env-key-unignored` | warn | the checkout-local key is not ignored by git, so `git status` offers it for commit; suggests adding it to the root `.gitignore` | +| `env-key-unignored` | warn | the checkout-local key is not ignored by git, so `git status` offers it for commit (for example after `git init` in a directory whose root `.gitignore` entry was removed); suggests adding it to the root `.gitignore` | | `env-key-symlink` | error | the key path is a symlink; Hack refuses symlinked keys | | `env-key-not-regular` | error | the key path is a directory or another non-file entry | | `env-key-unreadable` | error | unreadable, oversized or not UTF-8 | diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 0465db3d0..be367e373 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -2958,9 +2958,10 @@ async function checkComposeBackendFeatures(opts: { /** * Read-only key-strength and exposure audit (HACK-1127). `--fix` never acts on - * it; each finding suggests the exact command instead. + * it; each finding suggests the exact command instead. It also catches a key + * that a later `git init` left unignored (`env-key-unignored`, HACK-1386). */ -async function checkProjectEnvKey({ +export async function checkProjectEnvKey({ startDir, }: { readonly startDir: string; diff --git a/src/lib/env-layer-lock.ts b/src/lib/env-layer-lock.ts index 0daa22397..760bc6662 100644 --- a/src/lib/env-layer-lock.ts +++ b/src/lib/env-layer-lock.ts @@ -13,18 +13,55 @@ import { export const ENV_LAYER_LOCK_WAIT_MS = 10_000; /** - * Another `hack env` writer held the layer's lock for the whole bounded wait - * (or an interrupted lock recovery needs offline inspection). Closed and - * retryable: nothing was read or written, and the message names only the - * layer path, never a key or value. + * Why {@link EnvLayerLockTimeoutError} gave up: + * - `held`: the lock stayed occupied for the whole wait. Either a live Hack + * writer holds it, or Hack can never reclaim it on its own (a malformed + * receipt, one owned by another user, or a PID now used by a live process). + * - `recovery_guard`: the owner has exited, but the `.recovery` guard + * left by an interrupted reclaimer blocks reclamation until it is removed. + */ +export type EnvLayerLockTimeoutReason = "held" | "recovery_guard"; + +/** The projects-registry lock's detail for a dead owner behind a busy guard. */ +const RECOVERY_GUARD_BUSY_DETAIL = + "recovery guard is busy (interrupted recovery requires offline inspection)"; + +/** + * The layer's lock stayed held for the whole bounded wait. Closed and + * retryable: nothing was read or written. The one-line message names the + * layer, its lock path and the recovery step, never a key or value, so a lock + * Hack cannot reclaim does not leave the user retrying a command that can + * never succeed. */ export class EnvLayerLockTimeoutError extends ClosedCliError { - constructor(opts: { readonly path: string; readonly waitMs: number }) { - super( - `Another hack env command is writing ${opts.path}; gave up after waiting ${formatWait(opts.waitMs)} for it. Nothing was written. Retry when it finishes.` - ); + readonly reason: EnvLayerLockTimeoutReason; + readonly lockPath: string; + + constructor(opts: { + readonly path: string; + readonly lockPath: string; + readonly waitMs: number; + readonly reason: EnvLayerLockTimeoutReason; + }) { + super(envLayerLockTimeoutMessage(opts)); this.name = "EnvLayerLockTimeoutError"; + this.reason = opts.reason; + this.lockPath = opts.lockPath; + } +} + +function envLayerLockTimeoutMessage(opts: { + readonly path: string; + readonly lockPath: string; + readonly waitMs: number; + readonly reason: EnvLayerLockTimeoutReason; +}): string { + const wait = formatWait(opts.waitMs); + if (opts.reason === "recovery_guard") { + const guard = `${opts.lockPath}.recovery`; + return `Refused to update ${opts.path}: its lock at ${opts.lockPath} was left by a process that has exited, and the ${RECOVERY_GUARD_BUSY_DETAIL} at ${guard}; gave up after waiting ${wait}. Nothing was written. If no hack env command is running, remove ${guard} and ${opts.lockPath}, then retry.`; } + return `Another hack env command is writing ${opts.path}; gave up after waiting ${wait} for its lock at ${opts.lockPath}. Nothing was written. Retry when it finishes. If no hack env command is running, Hack cannot reclaim that lock itself: remove ${opts.lockPath} and retry.`; } /** @@ -69,7 +106,10 @@ export function resolveEnvLayerLockPath(opts: { * reclaimed only after `kill(pid, 0)` reports the owner gone (ESRCH). * - The wait is bounded (`timeoutMs`, default {@link ENV_LAYER_LOCK_WAIT_MS}) * with jittered polling. When it runs out, {@link EnvLayerLockTimeoutError} - * is thrown and `run` never starts, so nothing is written. + * is thrown and `run` never starts, so nothing is written. Its message + * names the lock path and how to clear a lock Hack cannot reclaim itself + * (a malformed or foreign receipt, a reused PID, an interrupted recovery's + * guard); this helper never removes such a lock on its own. * - A symlinked or non-directory `.internal` beside the layer is refused with * {@link EnvLayerLockRefusedError}; a filesystem failure while taking the * lock (permission, full disk) is refused as the layer it blocked, never as @@ -109,7 +149,14 @@ export async function withEnvLayerLock(opts: { throw error; } if (error instanceof ProjectsRegistryLockTimeoutError) { - throw new EnvLayerLockTimeoutError({ path: opts.layerPath, waitMs }); + throw new EnvLayerLockTimeoutError({ + path: opts.layerPath, + lockPath, + waitMs, + reason: error.message.includes(RECOVERY_GUARD_BUSY_DETAIL) + ? "recovery_guard" + : "held", + }); } if (!(await directoryWritable(dirname(opts.layerPath)))) { // No Hack writer can replace a layer whose directory it cannot write diff --git a/src/lib/project-env-config.ts b/src/lib/project-env-config.ts index fa7684f57..0c03f9cbd 100644 --- a/src/lib/project-env-config.ts +++ b/src/lib/project-env-config.ts @@ -4180,12 +4180,20 @@ async function writeProjectEnvKeyWithFallback(opts: { return { keyPath: opts.localKeyPath, keyText, warnings }; } -/** Fixed, value-free refusal for a checkout-local key git would commit. */ +/** + * Fixed, value-free refusal for a checkout-local key git would commit. + * `unwritable`: Hack could not add the entry to the root `.gitignore`. + * `reincluded`: the entry is there, but a later rule in that file (such as + * `!.hack.secret.key`) re-includes the key, so adding it again cannot help. + */ function projectEnvKeyUnignoredError(opts: { readonly rootGitignore: string; + readonly cause: "unwritable" | "reincluded"; }): ProjectEnvKeyRefusalError { return new ProjectEnvKeyRefusalError( - `Refusing to use ${PROJECT_ENV_KEY_FILENAME}: git does not ignore it, so it could be committed, and Hack could not add it to ${opts.rootGitignore}. Add a '${PROJECT_ENV_KEY_FILENAME}' line to that file (making it writable if needed) and remove any rule that re-includes the key, then retry; key contents are never included in this message.` + opts.cause === "reincluded" + ? `Refusing to use ${PROJECT_ENV_KEY_FILENAME}: git does not ignore it, so it could be committed. ${opts.rootGitignore} lists '${PROJECT_ENV_KEY_FILENAME}', but a later rule in that file re-includes the key (for example '!${PROJECT_ENV_KEY_FILENAME}'). Remove that rule, then retry; key contents are never included in this message.` + : `Refusing to use ${PROJECT_ENV_KEY_FILENAME}: git does not ignore it, so it could be committed, and Hack could not add it to ${opts.rootGitignore}. Add a '${PROJECT_ENV_KEY_FILENAME}' line to that file (making it writable if needed) and remove any rule that re-includes the key, then retry; key contents are never included in this message.` ); } @@ -4203,7 +4211,12 @@ function projectEnvKeyUnignoredError(opts: { * key is not used. * * Either way, a key git still does not ignore after the entry (for example a - * later `!.hack.secret.key` rule) is refused. + * later `!.hack.secret.key` rule) is refused, naming that cause. + * + * A checkout outside git that later becomes one (`git init`) is not watched: + * Hack runs no code at that point. If the entry went missing meanwhile, the + * first `--secret` set or hydrate in the new checkout repairs it or refuses + * here, and `hack doctor`'s env key check reports `env-key-unignored`. */ async function ensureProjectEnvKeyIgnored(opts: { readonly projectRoot: string; @@ -4224,10 +4237,13 @@ async function ensureProjectEnvKeyIgnored(opts: { } catch (error: unknown) { throw opts.creating ? closedFileWriteError({ error, path: rootGitignore, operation: "write" }) - : projectEnvKeyUnignoredError({ rootGitignore }); + : projectEnvKeyUnignoredError({ rootGitignore, cause: "unwritable" }); } + // The entry is in the root `.gitignore` now (written or already there), and + // no deeper ignore file applies to a key at the project root, so only a + // later rule in that same file can re-include it. if ((await isGitIgnoredProjectEnvPath(probe)) === false) { - throw projectEnvKeyUnignoredError({ rootGitignore }); + throw projectEnvKeyUnignoredError({ rootGitignore, cause: "reincluded" }); } } diff --git a/tests/project-env-key-ignore.test.ts b/tests/project-env-key-ignore.test.ts index d8f2de92c..252807bfe 100644 --- a/tests/project-env-key-ignore.test.ts +++ b/tests/project-env-key-ignore.test.ts @@ -11,13 +11,14 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; - +import { checkProjectEnvKey } from "../src/commands/doctor.ts"; import { PROJECT_COMPOSE_FILENAME, PROJECT_CONFIG_FILENAME, PROJECT_ENV_KEY_FILENAME, } from "../src/constants.ts"; import { resolveProjectEnvConfigPath } from "../src/lib/project-env-config.ts"; +import { restoreEnv } from "./helpers/env.ts"; import { privateCliEnv } from "./helpers/private-cli-env.ts"; /** @@ -233,6 +234,11 @@ function unignoredRefusal(repo: Repo): string { return `Refusing to use ${PROJECT_ENV_KEY_FILENAME}: git does not ignore it, so it could be committed, and Hack could not add it to ${repo.gitignore}.`; } +/** HACK-1386: the entry is present, so the refusal names the re-including rule instead. */ +function reincludedRefusal(repo: Repo): string { + return `Refusing to use ${PROJECT_ENV_KEY_FILENAME}: git does not ignore it, so it could be committed. ${repo.gitignore} lists '${PROJECT_ENV_KEY_FILENAME}', but a later rule in that file re-includes the key (for example '!${PROJECT_ENV_KEY_FILENAME}'). Remove that rule, then retry; key contents are never included in this message.`; +} + /** Creates the key with a first secret, then drops its ignore entry (the pre-fix state). */ async function createUnignoredKey(repo: Repo): Promise { const first = await setSecret(repo, "FIRST_SECRET", FIRST_VALUE); @@ -352,9 +358,12 @@ test("a rule that re-includes the key is refused both before creating a key and await writeFile(repo.gitignore, reincluding); const first = await setSecret(repo, "NEW_SECRET", NEXT_VALUE); - expectOneLineRefusal(first, unignoredRefusal(repo)); + expectOneLineRefusal(first, reincludedRefusal(repo)); + expect(`${first.stdout}${first.stderr}`).not.toContain("could not add it"); expect(await keyExists(repo)).toBe(false); expect(await readFile(repo.layer, "utf8")).toBe(LAYER); + // The file already listed the key, so it was not rewritten. + expect(await readFile(repo.gitignore, "utf8")).toBe(reincluding); // An existing key under the same rule is refused before it encrypts. await writeFile(repo.gitignore, BASE_GITIGNORE); @@ -362,7 +371,7 @@ test("a rule that re-includes the key is refused both before creating a key and await writeFile(repo.gitignore, reincluding); const layerBefore = await readFile(repo.layer, "utf8"); const next = await setSecret(repo, "NEXT_SECRET", NEXT_VALUE); - expectOneLineRefusal(next, unignoredRefusal(repo)); + expectOneLineRefusal(next, reincludedRefusal(repo)); expectNoKeyMaterial(next, keyText); expect(await readFile(repo.layer, "utf8")).toBe(layerBefore); }, 60_000); @@ -382,3 +391,88 @@ test("outside git an existing key is used without writing a root .gitignore", as expect(run.exitCode).toBe(0); expect(await lstat(repo.gitignore).catch(() => null)).toBeNull(); }, 60_000); + +/** + * Runs `run` in this process with the fixture's private HOME, HACK_HOME and + * global config, and git's global, system and XDG configuration (so any + * personal excludes file) out of reach; restores the previous environment. + */ +async function withPrivateEnvironment( + repo: Repo, + run: () => Promise +): Promise { + const home = join(repo.root, "user-home"); + const overrides: Record = { + HOME: home, + XDG_CONFIG_HOME: join(home, ".config"), + HACK_HOME: join(repo.root, "hack-home"), + HACK_GLOBAL_CONFIG_PATH: join(repo.root, "hack-config.json"), + HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: "1", + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: "/dev/null", + }; + const previous = Object.fromEntries( + Object.keys(overrides).map((key) => [key, process.env[key]]) + ); + for (const [key, value] of Object.entries(overrides)) { + process.env[key] = value; + } + try { + return await run(); + } finally { + for (const [key, value] of Object.entries(previous)) { + restoreEnv(key, value); + } + } +} + +/** The real `hack doctor` env key check for the fixture, in this process. */ +async function doctorEnvKey(repo: Repo): Promise<{ + readonly status: string; + readonly codes: readonly string[]; + readonly message: string; +}> { + const result = await withPrivateEnvironment( + repo, + async () => await checkProjectEnvKey({ startDir: repo.projectRoot }) + ); + return { + status: result.status, + codes: (result.envKey?.findings ?? []).map((finding) => finding.code), + message: result.message, + }; +} + +test("a key whose entry was removed before a later git init is flagged by doctor and repaired by the next secret set", async () => { + // HACK-1386: outside git an existing key needs no entry, and Hack does not + // observe a later `git init`. Doctor's env key check is the backstop, and + // the first secret write in the new checkout restores the entry. + const repo = await createRepo({ git: false }); + const first = await setSecret(repo, "FIRST_SECRET", FIRST_VALUE); + expect(first.exitCode).toBe(0); + const keyText = (await readFile(repo.keyPath, "utf8")).trim(); + await rm(repo.gitignore); + const second = await setSecret(repo, "NEXT_SECRET", NEXT_VALUE); + expect(second.exitCode).toBe(0); + expect(await lstat(repo.gitignore).catch(() => null)).toBeNull(); + + expect((await git(repo, ["init", "-q"])).exitCode).toBe(0); + expect(await keyIsCommittable(repo)).toBe(true); + + const flagged = await doctorEnvKey(repo); + expect(flagged.status).toBe("warn"); + expect(flagged.codes).toContain("env-key-unignored"); + expect(flagged.message).toContain( + `env-key-unignored: ${PROJECT_ENV_KEY_FILENAME} is not ignored by git` + ); + expect(flagged.message).not.toContain(keyText); + expect(flagged.message).not.toContain(repo.root); + + const repair = await setSecret(repo, "THIRD_SECRET", FIRST_VALUE); + expect(repair.exitCode).toBe(0); + expectNoKeyMaterial(repair, keyText); + expect(await keyIsCommittable(repo)).toBe(false); + const repaired = await doctorEnvKey(repo); + expect(repaired.codes).not.toContain("env-key-unignored"); + expect(repaired.status).toBe("ok"); +}, 60_000); diff --git a/tests/project-env-layer-lock.test.ts b/tests/project-env-layer-lock.test.ts index c98af390c..7ece97a6e 100644 --- a/tests/project-env-layer-lock.test.ts +++ b/tests/project-env-layer-lock.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, expect, test } from "bun:test"; +import { afterEach, beforeEach, expect, spyOn, test } from "bun:test"; import { randomUUID } from "node:crypto"; import { chmod, @@ -31,6 +31,7 @@ import { } from "../src/lib/fs.ts"; import { removeDotEnvKey, upsertDotEnvValue } from "../src/lib/hack-env.ts"; import { + hydrateProjectEnvLocalLayer, resolveProjectEnvConfig, resolveProjectEnvConfigPath, resolveProjectEnvLocalConfigPath, @@ -38,6 +39,7 @@ import { setProjectEnvValue, unsetProjectEnvValue, } from "../src/lib/project-env-config.ts"; +import * as worktreeLocalConfig from "../src/lib/worktree-local-config.ts"; import { restoreEnv } from "./helpers/env.ts"; import { privateCliEnv, privateCliHome } from "./helpers/private-cli-env.ts"; import { createSecretWorktreeFixture } from "./helpers/secret-fixture.ts"; @@ -470,6 +472,14 @@ test("concurrent legacy dotenv sets and removals of different keys all apply", a } }, 60_000); +/** HACK-1386: the timeout names the lock and how to clear one Hack cannot reclaim. */ +function heldLockRefusal(opts: { + readonly layer: string; + readonly lockPath: string; +}): string { + return `Another hack env command is writing ${opts.layer}; gave up after waiting 300ms for its lock at ${opts.lockPath}. Nothing was written. Retry when it finishes. If no hack env command is running, Hack cannot reclaim that lock itself: remove ${opts.lockPath} and retry.`; +} + /** Publishes a live owner receipt (this test process) at the layer's lock. */ async function holdLayerLock(layerPath: string): Promise { const lockPath = resolveEnvLayerLockPath({ layerPath }); @@ -537,9 +547,10 @@ test("a writer that cannot get the layer lock in time refuses closed and writes ]; for (const [index, refusal] of refusals.entries()) { const layer = index < 2 ? file : envFile; + const layerLock = index < 2 ? lockPath : dotEnvLockPath; expect(refusal).toBeInstanceOf(EnvLayerLockTimeoutError); expect(refusal.message).toBe( - `Another hack env command is writing ${layer}; gave up after waiting 300ms for it. Nothing was written. Retry when it finishes.` + heldLockRefusal({ layer, lockPath: layerLock }) ); expect(refusal.message).not.toContain(synthetic("BLOCKED")); expect(refusal.message).not.toContain(synthetic("PRESET")); @@ -720,3 +731,202 @@ test("a set refuses with a conflict when a writer that takes no lock changed the // The other writer's layer stands; the set wrote nothing. expect(await readFile(file, "utf8")).toBe(edited); }, 30_000); + +test("an unset refuses with a conflict when a writer that takes no lock changed the layer after its read", async () => { + // HACK-1386: only the set's compare-and-swap was covered. A local unset + // asks whether a primary checkout's local layer is inherited after it read + // the layer and before it writes; pausing that one call opens the window + // in which an editor (or Hack v4) rewrites the layer. + const project = await createProject(); + const file = resolveProjectEnvLocalConfigPath({ + projectDir: project.projectDir, + envName: null, + }); + await writeFile( + file, + layerText({ + PRESET: synthetic("PRESET"), + DOOMED: synthetic("DOOMED"), + }) + ); + const { promise: reached, resolve: markReached } = + Promise.withResolvers(); + const { promise: release, resolve: resume } = Promise.withResolvers(); + const paused = spyOn( + worktreeLocalConfig, + "resolvePrimaryLocalProjectDir" + ).mockImplementation(async () => { + markReached(); + await release; + return null; + }); + try { + const pending = unsetProjectEnvValue({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + key: "DOOMED", + local: true, + }).then( + () => null, + (error: unknown) => error + ); + await reached; + const edited = layerText({ + PRESET: synthetic("PRESET"), + DOOMED: synthetic("DOOMED"), + HAND_EDITED: synthetic("HAND_EDITED"), + }); + await writeFile(file, edited); + resume(); + + const error = await pending; + expect(paused).toHaveBeenCalledTimes(1); + expect(error).toBeInstanceOf(AtomicWriteConflictError); + // The other writer's layer stands, HAND_EDITED included; nothing was removed. + expect(await readFile(file, "utf8")).toBe(edited); + } finally { + resume(); + paused.mockRestore(); + } +}); + +/** A PID that has exited: a short child that this test waited for. */ +function exitedPid(): number { + const child = Bun.spawnSync({ cmd: ["true"], stdout: "ignore" }); + expect(child.exitCode).toBe(0); + return child.pid; +} + +test("a layer lock Hack cannot reclaim makes set and unset refuse with its path and the recovery step", async () => { + // HACK-1386: a malformed receipt, a receipt that is not private, and a + // live PID that holds no Hack lock (a reused PID) are never reclaimed, so + // before the fix every retry failed with only "Retry when it finishes." + const project = await createProject(); + const file = resolveProjectEnvConfigPath({ + projectDir: project.projectDir, + envName: null, + }); + const original = layerText({ PRESET: synthetic("PRESET") }); + await writeFile(file, original); + const lockPath = resolveEnvLayerLockPath({ layerPath: file }); + await mkdir(dirname(lockPath), { recursive: true }); + const base = { + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + layerLockTimeoutMs: 300, + }; + // A group-writable receipt is not trusted, like one owned by another user, + // even though its owner has exited. PID 1 is alive and holds no Hack lock, + // standing in for a reused PID. + const receipts: readonly { readonly text: string; readonly mode: number }[] = + [ + { text: "not a receipt\n", mode: 0o600 }, + { text: `${exitedPid()}\n`, mode: 0o620 }, + { text: "1\n", mode: 0o600 }, + ]; + for (const receipt of receipts) { + await writeFile(lockPath, receipt.text); + await chmod(lockPath, receipt.mode); + for (const attempt of [ + () => + setProjectEnvValue({ + ...base, + key: "BLOCKED", + value: synthetic("BLOCKED"), + secret: false, + }), + () => unsetProjectEnvValue({ ...base, key: "PRESET" }), + ]) { + const refusal = await closedRefusalOf(attempt()); + expect(refusal).toBeInstanceOf(EnvLayerLockTimeoutError); + expect((refusal as EnvLayerLockTimeoutError).reason).toBe("held"); + expect(refusal.message).toBe(heldLockRefusal({ layer: file, lockPath })); + expect(refusal.message).not.toContain("\n"); + expect(refusal.message).not.toContain(synthetic("BLOCKED")); + expect(refusal.message).not.toContain(synthetic("PRESET")); + } + expect(await readFile(file, "utf8")).toBe(original); + // Nothing reclaimed it: the receipt is exactly as it was left. + expect(await readFile(lockPath, "utf8")).toBe(receipt.text); + } + + // The recovery step the message names works. + await rm(lockPath); + const done = await unsetProjectEnvValue({ ...base, key: "PRESET" }); + expect(done.changed).toBe(true); + expect(await readdir(dirname(lockPath))).toEqual([]); +}); + +test("a dead owner behind an interrupted recovery guard is refused with both paths, also by hydrate", async () => { + // HACK-1386: a reclaimer that crashed leaves `.recovery`; the guard + // is never age-reclaimed, so the owner can never be replaced on its own. + const project = await createProject(); + const file = resolveProjectEnvLocalConfigPath({ + projectDir: project.projectDir, + envName: null, + }); + const original = layerText({ PRESET: synthetic("PRESET") }); + await writeFile(file, original); + const lockPath = resolveEnvLayerLockPath({ layerPath: file }); + const guard = `${lockPath}.recovery`; + await mkdir(guard, { recursive: true, mode: 0o700 }); + await writeFile(lockPath, `${exitedPid()}\n${randomUUID()}\n`, { + mode: 0o600, + }); + const expected = `Refused to update ${file}: its lock at ${lockPath} was left by a process that has exited, and the recovery guard is busy (interrupted recovery requires offline inspection) at ${guard}; gave up after waiting 300ms. Nothing was written. If no hack env command is running, remove ${guard} and ${lockPath}, then retry.`; + + const refusal = await closedRefusalOf( + setProjectEnvValue({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + key: "BLOCKED", + value: synthetic("BLOCKED"), + secret: false, + local: true, + layerLockTimeoutMs: 300, + }) + ); + expect(refusal).toBeInstanceOf(EnvLayerLockTimeoutError); + expect((refusal as EnvLayerLockTimeoutError).reason).toBe("recovery_guard"); + expect(refusal.message).toBe(expected); + + // Hydration waits the default 10 s for the same lock and refuses alike. + const hydrated = await closedRefusalOf( + hydrateProjectEnvLocalLayer({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + entries: new Map([["HYDRATED", synthetic("HYDRATED")]]), + secret: false, + }) + ); + expect(hydrated).toBeInstanceOf(EnvLayerLockTimeoutError); + expect(hydrated.message).toBe(expected.replace("300ms", "10s")); + expect(hydrated.message).not.toContain(synthetic("HYDRATED")); + expect(await readFile(file, "utf8")).toBe(original); + // Hydration released its own lock; the layer lock and guard are untouched. + expect((await readdir(dirname(lockPath))).sort()).toEqual( + [basename(lockPath), basename(guard)].sort() + ); + + await rm(guard, { recursive: true }); + await rm(lockPath); + const done = await setProjectEnvValue({ + projectRoot: project.projectRoot, + projectDir: project.projectDir, + envName: null, + scope: "global", + key: "BLOCKED", + value: synthetic("BLOCKED"), + secret: false, + local: true, + }); + expect(done.changed).toBe(true); +}, 30_000);