diff --git a/docs/env.md b/docs/env.md index f8d50dc28..91f31f63b 100644 --- a/docs/env.md +++ b/docs/env.md @@ -933,6 +933,22 @@ copies remain until migration cleanup or explicit removal; encrypting a YAML ent does not erase those copies. `doctor --migrate-env-config` prints the remaining plaintext key names and a `hack env protect` command after migration. +Migration cleanup keeps content and file-identity receipts from its source +inventory. After confirmation, it takes the source writer locks, checks those +receipts and rechecks current YAML delivery and Compose references before +removing anything. A legacy store and its key use their separate owner locks, +including the key lock shared by every store using that path. +Changed sources or delivery refuse cleanup with reconciliation +advice; migration writes already completed remain. Confirmed removals before a +failure are named. Implicit cleanup warns and lets the original command continue +even if a lock times out, a key is unavailable or a file is unreadable. +Once YAML layers exist, rerunning migration may not import changed legacy +values. Keep the copies and keys, reconcile chosen values with `env set KEY +--value-stdin` (use `--local` for machine-local values, add `--secret` for secrets +and preserve scope/overlay), check +delivery, then remove only copies confirmed obsolete. Editors and older writers +that do not take Hack's locks can still race a filesystem operation. + ### Concurrent writes to one env file Hack env commands that change an env file never silently lose another command's change. @@ -1529,6 +1545,8 @@ only, so it runs even when Docker is not reachable; the other `--migrate-env-con repairs still need Docker. As with any migration, `hack doctor` then offers to remove the legacy `.hack/.env*` files that Compose no longer references (a non-interactive run accepts for untracked files, and for git-tracked ones only with `--accept-tracked-env-changes`). +An untracked legacy store key joins its tracked store's confirmation batch, so +declining tracked cleanup keeps both files and the store remains readable. Files Compose still names as `env_file` are offered only when the Compose repair removed those references. Without `hack.env.json`, a legacy file that git tracks, or one whose status git cannot report in a git checkout, is never offered, because the migrated @@ -1667,6 +1685,7 @@ Some older surfaces are still compatibility-oriented: bound to a path, project or variable name beyond the scoped key inside the encrypted store plaintext - the daemon env GET response identifies the format. For YAML projects it returns names, scopes, encryption flags and availability without decrypting or presenting legacy state. Legacy projects keep the dotenv-shaped metadata response. Legacy set/unset mutations refuse YAML projects until those endpoints use canonical YAML mutation helpers; use the CLI instead. +- legacy daemon unset reports HTTP 500 with `secret_delete_unconfirmed` and `secretDeleted: null` if backend deletion throws, including an unknown outcome. It preserves `dotenvChanged` for the completed dotenv update; this operation is not atomic across both stores. A failure does not confirm that a backend value is absent. If you are writing new docs or new project setup flows, document the YAML overlay model first and treat `.hack/hack.env.json` as legacy migration context. diff --git a/docs/security/env-secrets-threat-model.md b/docs/security/env-secrets-threat-model.md index 146a709b7..4b7ce1c76 100644 --- a/docs/security/env-secrets-threat-model.md +++ b/docs/security/env-secrets-threat-model.md @@ -290,34 +290,27 @@ planned branch or a passing documentation check is not a mitigation. session is followed until Hack's next record; ancestors of `.hack/.internal/` are not checked for other writers; staged multiplexer payloads are removed by the next staging, not by an independent reaper. -17. **Open: secret-store and key integrity (HACK-2169).** In - `src/lib/secret-store.ts`, an unreadable encrypted store can be treated as empty - by `readEncryptedEntries`; a later mutation can replace it with only the new - entry. The process-global key cache is not bound to a store/key path, so a - cached key can be reused for another store whose key is absent. Explicit key - provisioning can replace an existing key without authenticating and re-encrypting - the store under the new one. Separately, project env add/set can create a file - key despite a working environment-only project key; file precedence then hides - that environment key from reads (`src/lib/project-env-config.ts`, - `src/commands/env.ts`). These are integrity and recoverability gaps, not safe - rotation paths. Keep them open until the corresponding fixes merge. -18. **Open: local-layer symlinks, ignores and modes (HACK-2165).** Hydration checks - whether the logical local path is tracked before resolving an allowed - in-project symlink; the writer can then replace its tracked shared target. - Ordinary local add/set and hydrate can warn about failed ignore setup and - still write, and named local layers can already be tracked - (`src/lib/project-env-config.ts`, `src/lib/project-env-layer-path.ts`, - `src/lib/fs.ts`). Existing local-layer modes are preserved. Legacy dotenv - inspection also follows symlinks through `readTextFile`, including links - outside the project (`src/lib/hack-env.ts`); mutation checks do not protect - that read path. Keep these gaps open until the hygiene fixes merge; an - in-project target or local filename is not proof of private storage. -19. **Open: migration cleanup recheck (HACK-2171).** Cleanup removes the current - path after an earlier migration/consent decision without rechecking the - selected file's current bytes and identity under a lock - (`removeLegacyProjectEnvArtifacts` in `src/lib/project-env-config.ts`, - cleanup callers in `src/commands/doctor.ts`). Edits made between that decision - and removal can be lost. Keep this open until cleanup revalidation merges. +17. **Narrowed: secret-store and key integrity (HACK-2169, #766).** Unreadable or + malformed stores refuse mutation instead of becoming an empty store. Configured + file-key lookup does not borrow a cached key from another path; provisioning + refuses to replace a different existing key. An environment-only project key + is retained without creating a shadowing file key. Shared key-writer locks + serialize provisioning and cleanup. These are preservation checks, not a + rotation or recovery mechanism for keys already lost. +18. **Narrowed: local-layer symlinks and Git privacy (HACK-2165, #762).** + Machine-local writes refuse linked or tracked destinations, unknown tracking + inside a checkout and ineffective ignore rules. Legacy dotenv inspection uses + no-follow reads. Local naming and ignores still do not prove private historical + permissions or erase earlier copies; inspect Doctor's findings and retain the + remaining filesystem-race limits described above. +19. **Mitigated: stale migration cleanup decisions (HACK-2171).** Cleanup carries + content and file-identity receipts from before migration, then rechecks them + and current scoped delivery under writer locks after consent. Each unlink + rechecks its target. Refusals preserve remaining files and report confirmed + removals; automatic cleanup warns and continues the original command. Writers + outside Hack's cooperative locks can still race the final unlink. The daemon + also reports unconfirmed secret deletion as HTTP 500 `partial_failure`, + retaining the completed dotenv result instead of claiming success. ### Plaintext on disk outside materialization diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 9c104b0d1..8908312ff 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -4529,6 +4529,7 @@ async function maybeMigrateProjectEnvConfig(opts: { projectDir: project.projectDir, projectName, paths: cleanupCandidates, + context: migrated.cleanupContext, acceptTrackedChanges: opts.acceptTrackedChanges, }); await printMigrationProtectionHint(project); @@ -4575,6 +4576,7 @@ async function offerDoctorEnvCleanup(opts: { readonly projectDir: string; readonly projectName: string; readonly paths: readonly string[]; + readonly context?: LegacyProjectEnvMigration["cleanupContext"]; readonly acceptTrackedChanges: boolean; }): Promise { const untracked: string[] = []; @@ -4589,6 +4591,17 @@ async function offerDoctorEnvCleanup(opts: { ).push(path); } + // A key cannot be removed while its tracked store still exists. Include an + // untracked key in that store's later consent batch, keeping the dependency + // intact whether tracked cleanup is accepted, declined or refused. + const store = opts.context?.store; + if (store && tracked.includes(store.path)) { + const keyIndex = untracked.indexOf(store.keyPath); + if (keyIndex !== -1) { + tracked.push(...untracked.splice(keyIndex, 1)); + } + } + if (untracked.length > 0) { note( [ @@ -4609,6 +4622,7 @@ async function offerDoctorEnvCleanup(opts: { projectDir: opts.projectDir, projectName: opts.projectName, paths: untracked, + context: opts.context, }); } } @@ -4616,7 +4630,7 @@ async function offerDoctorEnvCleanup(opts: { if (tracked.length > 0) { note( [ - "These legacy env files are tracked by git and can now be removed. Teammates share them, so commit the removal together with the new env layers:", + "These legacy env files are tracked by git, or are keys kept with a tracked store, and can now be removed. Teammates share the tracked files, so commit their removal together with the new env layers:", ...tracked.map((path) => `- ${path}`), ].join("\n"), "env cleanup" @@ -4632,6 +4646,7 @@ async function offerDoctorEnvCleanup(opts: { projectDir: opts.projectDir, projectName: opts.projectName, paths: tracked, + context: opts.context, }); } else { note( @@ -4652,10 +4667,12 @@ async function removeDoctorEnvCleanup(opts: { readonly projectDir: string; readonly projectName: string; readonly paths: readonly string[]; + readonly context?: LegacyProjectEnvMigration["cleanupContext"]; }): Promise { const removed = await removeLegacyProjectEnvArtifacts({ paths: opts.paths, projectRoot: opts.projectRoot, + context: opts.context, }); if (removed.length > 0) { note(`Removed ${removed.join(", ")}`, "env cleanup"); diff --git a/src/daemon/routes/env.ts b/src/daemon/routes/env.ts index 6aa8034ce..cd5d40465 100644 --- a/src/daemon/routes/env.ts +++ b/src/daemon/routes/env.ts @@ -525,7 +525,19 @@ async function handleUnsetEnv(opts: { }), }); } catch { - secretDeleted = null; + return jsonResponse( + { + status: "partial_failure", + error: "secret_delete_unconfirmed", + message: + "The dotenv update completed, but secret deletion could not be confirmed. Inspect backend state and retry; values omitted.", + dotenvChanged: dotenvResult.changed, + secretDeleted: null, + backend: secretStore.descriptor.backend, + location: secretStore.descriptor.location, + }, + 500 + ); } return jsonResponse( diff --git a/src/lib/legacy-env-cleanup-receipt.ts b/src/lib/legacy-env-cleanup-receipt.ts new file mode 100644 index 000000000..a5512cb1e --- /dev/null +++ b/src/lib/legacy-env-cleanup-receipt.ts @@ -0,0 +1,64 @@ +import { createHash } from "node:crypto"; +import { constants } from "node:fs"; +import { lstat, open } from "node:fs/promises"; + +/** In-memory deletion authority tied to file identity and contents; never source text. */ +export type LegacyEnvCleanupReceipt = { + readonly path: string; + readonly identity: string; + readonly digest: string; +}; + +/** Missing, linked, changing or unreadable files have no deletion authority. */ +export async function legacyEnvCleanupReceipt( + path: string +): Promise { + try { + const file = await open( + path, + constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK + ); + try { + const before = await file.stat({ bigint: true }); + if (!before.isFile() || before.size > 32n * 1024n * 1024n) { + return null; + } + const bytes = Buffer.alloc(Number(before.size) + 1); + const { bytesRead } = await file.read(bytes, 0, bytes.length, 0); + if (BigInt(bytesRead) !== before.size) { + return null; + } + const digest = createHash("sha256") + .update(bytes.subarray(0, bytesRead)) + .digest("hex"); + const after = await file.stat({ bigint: true }); + const named = await lstat(path, { bigint: true }); + const identity = (entry: typeof before) => + [entry.dev, entry.ino, entry.size, entry.mtimeNs, entry.ctimeNs].join( + ":" + ); + if ( + named.isSymbolicLink() || + identity(before) !== identity(after) || + identity(after) !== identity(named) + ) { + return null; + } + return { path, identity: identity(after), digest }; + } finally { + await file.close(); + } + } catch { + return null; + } +} + +export async function legacyEnvCleanupReceiptMatches( + expected: LegacyEnvCleanupReceipt +): Promise { + const current = await legacyEnvCleanupReceipt(expected.path); + return ( + current?.identity === expected.identity && + current.digest === expected.digest + ); +} diff --git a/src/lib/project-env-config.ts b/src/lib/project-env-config.ts index 06acf7317..03f74bc2f 100644 --- a/src/lib/project-env-config.ts +++ b/src/lib/project-env-config.ts @@ -80,6 +80,11 @@ import { resolveHackEnv, } from "./hack-env.ts"; import { resolveUserHomeDir } from "./home-dir.ts"; +import { + type LegacyEnvCleanupReceipt, + legacyEnvCleanupReceipt, + legacyEnvCleanupReceiptMatches, +} from "./legacy-env-cleanup-receipt.ts"; import { quoteShellWord } from "./logs.ts"; import { isOwnedLegacyAdoptionManagedEnvAdmission, @@ -123,6 +128,8 @@ import { import { type FileSecretStoreListing, listFileSecretStoreEntries, + withEncryptedFileKeyLock, + withEncryptedStoreMutation, } from "./secret-store.ts"; import { hasPrimaryLocalInputs, @@ -6113,6 +6120,9 @@ async function ensureLegacyMigrationLocalLayersIgnored(opts: { /** Outcome of {@link migrateLegacyProjectEnv}. */ export type LegacyProjectEnvMigration = { + /** Ephemeral authority from the source inventory, carried across confirmation prompts. */ + readonly cleanupContext?: LegacyEnvCleanupContext; + readonly cleanupStore?: { readonly path: string; readonly keyPath: string }; readonly wroteFiles: readonly string[]; readonly migratedOverlays: readonly string[]; readonly movedPlainValues: readonly { @@ -6133,6 +6143,16 @@ export type LegacyProjectEnvMigration = { readonly keptSecretStorePointer?: KeptSecretStorePointer | null; }; +export type LegacyEnvCleanupContext = { + readonly projectDir: string; + readonly projectName: string; + readonly serviceNames: readonly string[]; + readonly overlayNames: readonly string[]; + readonly contract: HackEnvContract; + readonly receipts: readonly LegacyEnvCleanupReceipt[]; + readonly store?: { readonly path: string; readonly keyPath: string }; +}; + /** * An encrypted secret store pointer (`controlPlane.secrets.encryptedFile` and * `backend: encrypted_file`) the migration left in `configPath` because it @@ -6225,7 +6245,48 @@ export function describeRetainedLegacyEnvFile(opts: { * callers in commands/project.ts, commands/doctor.ts, commands/session.ts, * and commands/env.ts (`maybeMigrateLegacyProjectEnv`). */ -export async function migrateLegacyProjectEnv(opts: { +export async function migrateLegacyProjectEnv( + opts: Parameters[0] +): Promise { + await assertLegacyProjectDirectory({ projectDir: opts.projectDir }); + await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); + const planned = await planLegacyProjectConfigChanges(opts); + const names = await discoverLegacyOverlayNames({ + projectDir: opts.projectDir, + includeSymlinks: false, + }); + const paths = new Set([ + resolve(opts.projectDir, PROJECT_ENV_CONTRACT_FILENAME), + resolve(opts.projectDir, PROJECT_ENV_FILENAME), + ...names.map((name) => + resolve(opts.projectDir, `${PROJECT_ENV_FILENAME}.${name}`) + ), + ...planned.cleanupCandidates, + ]); + const receipts: LegacyEnvCleanupReceipt[] = []; + for (const path of paths) { + const receipt = await legacyEnvCleanupReceipt(path); + if (receipt) { + receipts.push(receipt); + } + } + const contract = await readHackEnvContract({ projectDir: opts.projectDir }); + const migrated = await migrateLegacyProjectEnvWithSources(opts); + return { + ...migrated, + cleanupContext: { + projectDir: opts.projectDir, + projectName: opts.projectName, + serviceNames: opts.serviceNames, + overlayNames: migrated.migratedOverlays, + contract: contract.contract, + receipts, + store: migrated.cleanupStore, + }, + }; +} + +async function migrateLegacyProjectEnvWithSources(opts: { readonly projectRoot: string; readonly projectDir: string; readonly projectName: string; @@ -6566,6 +6627,10 @@ export async function migrateLegacyProjectEnv(opts: { legacyDetected: true, updatedProjectConfig: configCleanup.changed, cleanupCandidates: cleanupCandidates.allowed, + cleanupStore: { + path: secretStoreListing.storePath, + keyPath: secretStoreListing.keyPath, + }, blockedCleanupCandidates: cleanupCandidates.blocked, retainedEnvFiles: [ ...cleanupCandidates.retainedEnvFiles, @@ -6852,14 +6917,25 @@ export async function finishImplicitLegacyEnvMigration(opts: { resolve(opts.projectDir, `${PROJECT_ENV_FILENAME}.${overlay}`) ), ]); - const removable = await filterUntrackedLegacyCleanupCandidates({ - projectRoot: opts.projectRoot, - paths: migrated.cleanupCandidates.filter((path) => dotenvFiles.has(path)), - }); - const removed = await removeLegacyProjectEnvArtifacts({ - projectRoot: opts.projectRoot, - paths: removable, - }); + let removed: readonly string[] = []; + try { + const removable = await filterUntrackedLegacyCleanupCandidates({ + projectRoot: opts.projectRoot, + paths: migrated.cleanupCandidates.filter((path) => dotenvFiles.has(path)), + }); + removed = await removeLegacyProjectEnvArtifacts({ + projectRoot: opts.projectRoot, + paths: removable, + context: migrated.cleanupContext, + }); + } catch (error) { + const failure = + error instanceof LegacyEnvCleanupError + ? error + : new LegacyEnvCleanupError({ reason: "unverified" }); + removed = failure.removedPaths; + lines.push(`Warning: kept remaining legacy env files. ${failure.message}`); + } if (removed.length > 0) { lines.push( `Removed ${removed.map(shown).join(", ")}: env layers now hold every value ${removed.length === 1 ? "it" : "they"} set.` @@ -9368,9 +9444,7 @@ async function assertLegacyCleanupDoesNotRemoveLiveInputs(opts: { }); for (const path of opts.paths) { if (await legacyCleanupIsLiveInput({ path, live })) { - throw new ClosedCliError( - "Refusing legacy env cleanup: a target is a current project env key, configuration or YAML layer. Keep live env inputs; migrated values still need them (values omitted)." - ); + throw new LegacyEnvCleanupError({ reason: "live_input" }); } } } @@ -9378,23 +9452,285 @@ async function assertLegacyCleanupDoesNotRemoveLiveInputs(opts: { export async function removeLegacyProjectEnvArtifacts(opts: { readonly projectRoot: string; readonly paths: readonly string[]; + readonly context?: LegacyEnvCleanupContext; }): Promise { await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); - await assertLegacyCleanupDoesNotRemoveLiveInputs(opts); const removed: string[] = []; - for (const path of opts.paths) { + try { + return await removeLegacyProjectEnvArtifactsUnderLocks({ + ...opts, + removed, + }); + } catch (error) { + throw new LegacyEnvCleanupError({ + reason: + error instanceof LegacyEnvCleanupError ? error.reason : "unverified", + removedPaths: removed, + }); + } +} + +async function removeLegacyProjectEnvArtifactsUnderLocks(opts: { + readonly projectRoot: string; + readonly paths: readonly string[]; + readonly context?: LegacyEnvCleanupContext; + readonly removed: string[]; +}): Promise { + await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); + await assertLegacyCleanupDoesNotRemoveLiveInputs(opts); + if (opts.paths.length === 0) { + return []; + } + const context = opts.context; + if (!context) { + throw legacyCleanupChanged(); + } + // Take the same source and destination locks as their writers. Sorted order + // avoids competing cleanup calls taking these locks in opposite orders. + const layers = new Set([ + ...opts.paths, + ...context.receipts.map((receipt) => receipt.path), + ]); + // Key writers have their own shared .lock (HACK-2169). Take it as well + // as the store lock; env-layer locks would create .internal beside the key. + if (context.store && layers.has(context.store.keyPath)) { + layers.add(context.store.path); + } + for (const envName of [null, ...context.overlayNames]) { + layers.add( + resolveProjectEnvConfigPath({ projectDir: context.projectDir, envName }) + ); + layers.add( + resolveProjectEnvLocalConfigPath({ + projectDir: context.projectDir, + envName, + }) + ); + const metadata = await resolveProjectEnvMetadata({ + ...opts, + ...context, + envName, + }); + for (const file of Object.values(metadata?.files ?? {})) { + if (typeof file === "string") { + layers.add(file); + } + } + } + const paths = [...layers].sort(); + const locked = async (index: number): Promise => { + const path = paths[index]; + if (path === undefined) { + await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); + await assertLegacyCleanupDoesNotRemoveLiveInputs(opts); + await assertLegacyCleanupReceipts({ paths: opts.paths, context }); + await assertLegacyCleanupDelivery({ ...opts, context }); + // Keep a store readable if unlinking it fails: its key is always last. + const targets = [ + ...opts.paths.filter((path) => path !== context.store?.keyPath), + ...opts.paths.filter((path) => path === context.store?.keyPath), + ]; + for (const target of targets) { + if (!(await pathExists(target))) { + continue; + } + const receipt = context.receipts.find((entry) => entry.path === target); + if (!(receipt && (await legacyEnvCleanupReceiptMatches(receipt)))) { + throw legacyCleanupChanged(); + } + await assertLegacyCleanupDoesNotRemoveLiveInputs({ + ...opts, + paths: [target], + }); + await rm(target, { recursive: false, force: false }); + opts.removed.push(target); + } + return opts.removed; + } + const run = () => locked(index + 1); + if (path === context.store?.keyPath) { + return await withEncryptedFileKeyLock({ keyPath: path, run }); + } + return path === context.store?.path + ? await withEncryptedStoreMutation({ filePath: path, run }) + : await withEnvLayerLock({ layerPath: path, run }); + }; + return await locked(0); +} + +const LEGACY_CLEANUP_RECOVERY = + "Keep the remaining legacy copies and existing keys. Once YAML layers exist, rerunning migration may not import changed legacy values. Reconcile chosen values with hack env set KEY --value-stdin (use --local for machine-local values, add --secret for secrets and preserve --env/--service), check delivery, then remove only copies confirmed obsolete. Values omitted."; + +/** A value-free cleanup outcome, including every removal confirmed before failure. */ +export class LegacyEnvCleanupError extends ClosedCliError { + readonly removedPaths: readonly string[]; + readonly reason: "changed" | "live_input" | "unverified"; + + constructor(opts: { + readonly reason: "changed" | "live_input" | "unverified"; + readonly removedPaths?: readonly string[]; + }) { + const removedPaths = [...(opts.removedPaths ?? [])]; + const reasons = { + changed: + "Refusing legacy env cleanup: source inventory or current delivery changed.", + live_input: + "Refusing legacy env cleanup: a target is a current project env key, configuration or YAML layer.", + unverified: + "Legacy env cleanup could not be confirmed; a writer may still hold a lock, or an input may be unreadable.", + }; + const outcome = + removedPaths.length > 0 + ? `Confirmed removals: ${removedPaths.map((path) => JSON.stringify(path)).join(", ")}.` + : "No removals were confirmed."; + super(`${reasons[opts.reason]} ${outcome} ${LEGACY_CLEANUP_RECOVERY}`); + this.removedPaths = removedPaths; + this.reason = opts.reason; + } +} + +function legacyCleanupChanged(): LegacyEnvCleanupError { + return new LegacyEnvCleanupError({ reason: "changed" }); +} + +async function assertLegacyCleanupReceipts(opts: { + readonly paths: readonly string[]; + readonly context: LegacyEnvCleanupContext; +}): Promise { + for (const path of new Set([ + ...opts.paths, + ...opts.context.receipts.map((entry) => entry.path), + ])) { if (!(await pathExists(path))) { continue; } - await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); - await assertLegacyCleanupDoesNotRemoveLiveInputs({ - ...opts, - paths: [path], + const receipt = opts.context.receipts.find((entry) => entry.path === path); + if (!(receipt && (await legacyEnvCleanupReceiptMatches(receipt)))) { + throw legacyCleanupChanged(); + } + } +} + +/** Re-evaluate delivery and Compose references after prompts, while writer locks are held. */ +async function assertLegacyCleanupDelivery(opts: { + readonly projectRoot: string; + readonly paths: readonly string[]; + readonly context: LegacyEnvCleanupContext; +}): Promise { + const context = { + ...opts.context, + serviceNames: await discoverComposeServiceNames({ + composeFile: resolve(opts.context.projectDir, PROJECT_COMPOSE_FILENAME), + }), + }; + const resolved = new Map(); + const preserves = async ( + key: string, + value: string, + envName: string | null + ) => { + let modern = resolved.get(envName); + if (!modern) { + modern = + (await resolveProjectEnvConfig({ + ...context, + projectRoot: opts.projectRoot, + envName, + })) ?? undefined; + if (!modern) { + throw legacyCleanupChanged(); + } + resolved.set(envName, modern); + } + const scopes = context.contract.vars.find((entry) => entry.key === key) + ?.services ?? ["global"]; + for (const scope of scopes.length > 0 ? scopes : ["global"]) { + let actual: string | undefined; + try { + actual = selectProjectEnvValues({ + resolved: modern, + scopeName: scope.trim() || "global", + })[key]; + } catch { + throw legacyCleanupChanged(); + } + if (actual !== value) { + throw legacyCleanupChanged(); + } + } + }; + const references = await inspectLegacyComposeEnvFileReferences({ + projectDir: context.projectDir, + composeFile: resolve(context.projectDir, PROJECT_COMPOSE_FILENAME), + }); + const contractSelected = opts.paths.includes( + resolve(context.projectDir, PROJECT_ENV_CONTRACT_FILENAME) + ); + for (const envName of [null, ...context.overlayNames]) { + const path = resolveEnvFilePath({ + projectDir: context.projectDir, + envName, }); - await rm(path, { recursive: false, force: true }); - removed.push(path); + if (opts.paths.includes(path) && (await pathExists(path))) { + if (references.some((entry) => entry.resolvedPath === path)) { + throw legacyCleanupChanged(); + } + for (const [key, value] of Object.entries( + await readLegacyDotenvCleanupEntries({ path }) + )) { + await preserves(key, value, envName); + } + } + if (contractSelected || (envName === null && opts.paths.includes(path))) { + const legacy = await resolveHackEnv({ ...context, envName }); + if (contractSelected) { + for (const entry of legacy.values) { + if (entry.value !== null) { + await preserves(entry.key, entry.value, envName); + } + } + } + if (envName === null && opts.paths.includes(path)) { + const interpolated = await composeInterpolatedServiceOnlyKeys({ + projectDir: context.projectDir, + resolvedValues: legacy.values, + }); + const entries = await readLegacyDotenvCleanupEntries({ path }); + if (Object.keys(entries).some((key) => interpolated.has(key))) { + throw legacyCleanupChanged(); + } + } + } + } + if ( + !( + context.store && + opts.paths.some( + (path) => + path === context.store?.path || path === context.store?.keyPath + ) + ) + ) { + return; + } + if (!(await pathExists(context.store.path))) { + return; + } + if (!opts.paths.includes(context.store.path)) { + throw legacyCleanupChanged(); + } + const listing = await listFileSecretStoreEntries(context); + if ( + listing.storePath !== context.store.path || + listing.kind !== "listed" || + listing.foreignEntryCount > 0 + ) { + throw legacyCleanupChanged(); + } + for (const [storeKey, value] of listing.ownEntries) { + const parsed = parseStoreOverlayKey(storeKey); + await preserves(parsed?.key ?? storeKey, value, parsed?.envName ?? null); } - return removed; } /** diff --git a/src/lib/secret-store.ts b/src/lib/secret-store.ts index 5e7b46db7..8853d2c35 100644 --- a/src/lib/secret-store.ts +++ b/src/lib/secret-store.ts @@ -685,7 +685,7 @@ function createEncryptedScopedStore(input: { * lock, so a concurrent writer's update is never lost. Reads take no lock: * writes replace the file atomically, so a reader sees a whole envelope. */ -async function withEncryptedStoreMutation(input: { +export async function withEncryptedStoreMutation(input: { readonly filePath: string; readonly run: () => Promise; }): Promise { @@ -844,9 +844,10 @@ async function provisionEnvKey(input: { * provisioning, keychain persistence and first-use bootstrap (HACK-2169), * so no writer can land between another's check and write. Key writers * take no store lock while holding it, and store writers resolve their key - * before taking the store lock, so the two locks never nest. + * before taking the store lock. Migration cleanup can hold both locks in + * sorted path order while rechecking receipts before deletion. */ -async function withEncryptedFileKeyLock(input: { +export async function withEncryptedFileKeyLock(input: { readonly keyPath: string; readonly run: () => Promise; }): Promise { diff --git a/tests/daemon-env.test.ts b/tests/daemon-env.test.ts index 9cf622731..f81fb8066 100644 --- a/tests/daemon-env.test.ts +++ b/tests/daemon-env.test.ts @@ -341,6 +341,22 @@ describe("handleEnvRoutes", () => { }); test("POST /v1/env/unset clears .hack/.env and shows as missing", async () => { + // A successful unset requires a backend that can confirm deletion/absence; + // an unavailable host keychain must now report an unconfirmed outcome. + process.env.HACK_SECRETS_FILE_KEY = "synthetic-plain-unset-key"; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + await writeFile( + resolve(repoRoot, ".hack", PROJECT_CONFIG_FILENAME), + JSON.stringify({ + name: "env-test", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: join(tempDir, "unset-store.json") }, + }, + }, + }) + ); const setReq = mockRequest({ method: "POST", path: "/v1/env/set", @@ -378,6 +394,49 @@ describe("handleEnvRoutes", () => { expect(foo?.hasValue).toBe(false); }); + test("unset reports an unconfirmed backend deletion and preserves its completed dotenv result", async () => { + const sentinel = "synthetic-delete-error-value"; + process.env.HACK_SECRETS_FILE_KEY = "synthetic-delete-error-key"; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + const storePath = join(tempDir, "broken-store.json"); + await writeFile(storePath, `{"version": ${sentinel}}`); + await writeFile( + join(repoRoot, ".hack", PROJECT_CONFIG_FILENAME), + JSON.stringify({ + name: "env-test", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath }, + }, + }, + }) + ); + await writeFile( + join(repoRoot, ".hack/.env"), + `FOO=${sentinel}\nKEEP=preserved\n` + ); + const req = mockRequest({ + method: "POST", + path: "/v1/env/unset", + body: { project: "env-test", key: "FOO" }, + }); + const response = await handleEnvRoutes({ req, url: new URL(req.url) }); + expect(response?.status).toBe(500); + const text = await response?.text(); + expect(text).not.toContain(sentinel); + expect(JSON.parse(text ?? "null")).toMatchObject({ + status: "partial_failure", + error: "secret_delete_unconfirmed", + dotenvChanged: true, + secretDeleted: null, + }); + expect(await readFile(join(repoRoot, ".hack/.env"), "utf8")).toBe( + "KEEP=preserved\n" + ); + expect(await readFile(storePath, "utf8")).toContain(sentinel); + }); + test("secret set/unset uses configured encrypted_file backend", async () => { process.env.HACK_SECRETS_FILE_KEY = "daemon-env-secret-key"; await writeTextFileIfChanged( diff --git a/tests/doctor-fix-noninteractive.test.ts b/tests/doctor-fix-noninteractive.test.ts index 7873f1ca8..518e0d3cd 100644 --- a/tests/doctor-fix-noninteractive.test.ts +++ b/tests/doctor-fix-noninteractive.test.ts @@ -34,6 +34,7 @@ import { } from "../src/lib/compose-managed-env.ts"; import { resetNoInteractiveFlagForTests } from "../src/lib/interactivity.ts"; import { restoreEnv } from "./helpers/env.ts"; +import { runFixtureGit } from "./helpers/fixture-git.ts"; import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts"; /** @@ -1262,79 +1263,92 @@ test("doctor keeps an encrypted store with an undeclared entry and its key file, } }, 30_000); -test("doctor removes a fully delivered encrypted store, its key file and its pointer (HACK-1715)", async () => { - const projectRoot = await createProjectWithStaleDaemonPid(); - const projectDir = resolve(projectRoot, ".hack"); - const saved = { - fileKey: process.env.HACK_SECRETS_FILE_KEY, - fallback: process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK, - }; - Reflect.deleteProperty(process.env, "HACK_SECRETS_FILE_KEY"); - process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; - try { - const configPath = resolve(projectDir, "hack.config.json"); - await writeFile( - configPath, - JSON.stringify({ - name: "store-delivered", - controlPlane: { - secrets: { - backend: "encrypted_file", - encryptedFile: { path: "secrets.enc.json", keyPath: "store.key" }, +for (const trackedStore of [false, true]) { + test(`doctor removes a fully delivered encrypted store (${trackedStore ? "tracked store, untracked key" : "untracked"}), its key file and its pointer (HACK-1715)`, async () => { + const projectRoot = await createProjectWithStaleDaemonPid(); + const projectDir = resolve(projectRoot, ".hack"); + const saved = { + fileKey: process.env.HACK_SECRETS_FILE_KEY, + fallback: process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK, + }; + Reflect.deleteProperty(process.env, "HACK_SECRETS_FILE_KEY"); + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + try { + const configPath = resolve(projectDir, "hack.config.json"); + await writeFile( + configPath, + JSON.stringify({ + name: "store-delivered", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: "secrets.enc.json", keyPath: "store.key" }, + }, }, - }, - }) - ); - await writeFile( - resolve(projectDir, "hack.env.json"), - JSON.stringify({ - version: 1, - vars: [{ key: "SEC", source: "keychain" }], - }) - ); - const storePath = resolve(projectRoot, "secrets.enc.json"); - const keyPath = resolve(projectRoot, "store.key"); - await writeFile( - keyPath, - `${["synthetic", "store", "key", "62cd"].join("-")}\n`, - { mode: 0o600 } - ); - // Private, as Hack writes it: a key others can read is refused. - await chmod(keyPath, 0o600); - const declared = ["synthetic", "declared", "62cd"].join("-"); - const { resolveSecretStore } = await import("../src/lib/secret-store.ts"); - const store = await resolveSecretStore({ - projectName: "store-delivered", - projectDir, - }); - await store.set({ key: "SEC", value: declared }); + }) + ); + await writeFile( + resolve(projectDir, "hack.env.json"), + JSON.stringify({ + version: 1, + vars: [{ key: "SEC", source: "keychain" }], + }) + ); + const storePath = resolve(projectRoot, "secrets.enc.json"); + const keyPath = resolve(projectRoot, "store.key"); + await writeFile( + keyPath, + `${["synthetic", "store", "key", "62cd"].join("-")}\n`, + { mode: 0o600 } + ); + // Private, as Hack writes it: a key others can read is refused. + await chmod(keyPath, 0o600); + const declared = ["synthetic", "declared", "62cd"].join("-"); + const { resolveSecretStore } = await import("../src/lib/secret-store.ts"); + const store = await resolveSecretStore({ + projectName: "store-delivered", + projectDir, + }); + await store.set({ key: "SEC", value: declared }); + + if (trackedStore) { + for (const args of [ + ["init", "--quiet"], + ["add", "secrets.enc.json"], + ["commit", "--quiet", "-m", "fixture"], + ]) { + const result = await runFixtureGit({ cwd: projectRoot, args }); + expect(result.exitCode, result.stderr).toBe(0); + } + } - const { runCli } = await import("../src/cli/run.ts"); - expect( - await runCli([ - "doctor", - "--migrate-env-config", - "--no-interactive", - "--accept-tracked-env-changes", - "--path", - projectRoot, - ]) - ).toBe(0); + const { runCli } = await import("../src/cli/run.ts"); + expect( + await runCli([ + "doctor", + "--migrate-env-config", + "--no-interactive", + "--accept-tracked-env-changes", + "--path", + projectRoot, + ]) + ).toBe(0); - expect(existsSync(storePath)).toBe(false); - expect(existsSync(keyPath)).toBe(false); - const config = JSON.parse(await readFile(configPath, "utf8")) as { - readonly controlPlane?: unknown; - }; - expect(config.controlPlane).toBeUndefined(); - const notes = noteCalls.join("\n"); - expect(notes).toContain( - "Removed the encrypted secret store pointer from hack.config.json" - ); - expect(notes).not.toContain("Kept the encrypted secret store pointer"); - expect(notes).not.toContain(declared); - } finally { - restoreEnv("HACK_SECRETS_FILE_KEY", saved.fileKey); - restoreEnv("HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", saved.fallback); - } -}, 30_000); + expect(existsSync(storePath)).toBe(false); + expect(existsSync(keyPath)).toBe(false); + const config = JSON.parse(await readFile(configPath, "utf8")) as { + readonly controlPlane?: unknown; + }; + expect(config.controlPlane).toBeUndefined(); + const notes = noteCalls.join("\n"); + expect(notes).toContain( + "Removed the encrypted secret store pointer from hack.config.json" + ); + expect(notes).not.toContain("Kept the encrypted secret store pointer"); + expect(notes).not.toContain(declared); + } finally { + restoreEnv("HACK_SECRETS_FILE_KEY", saved.fileKey); + restoreEnv("HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", saved.fallback); + } + }, 30_000); +} diff --git a/tests/env-migration-live-inputs.test.ts b/tests/env-migration-live-inputs.test.ts index 0d1fb6721..1c8850caf 100644 --- a/tests/env-migration-live-inputs.test.ts +++ b/tests/env-migration-live-inputs.test.ts @@ -103,6 +103,7 @@ for (const configuredKeyPath of [".hack.secret.key", "link/.hack.secret.key"]) { await removeLegacyProjectEnvArtifacts({ projectRoot: options.projectRoot, paths: migrated.cleanupCandidates, + context: migrated.cleanupContext, }); const resolved = await resolveProjectEnvConfig({ ...options, @@ -208,12 +209,15 @@ test("an unrelated .dev file does not block cleanup in a .hack project", async ( ); const ordinary = join(options.projectRoot, "legacy-artifact"); await writeFile(ordinary, "fixture\n"); - expect( - await removeLegacyProjectEnvArtifacts({ - projectRoot: options.projectRoot, - paths: [ordinary], - }) - ).toEqual([ordinary]); + const candidates = await collectLegacyProjectEnvCleanupCandidates({ + ...options, + overlayNames: [], + materialize: false, + deliveredValues: new Map(), + configCleanupCandidates: [ordinary], + composeEnvFileReferences: [], + }); + expect(candidates.allowed).toEqual([ordinary]); expect(await readFile(join(options.projectRoot, ".dev"), "utf8")).toBe( "unrelated fixture file\n" ); diff --git a/tests/legacy-env-cleanup.test.ts b/tests/legacy-env-cleanup.test.ts new file mode 100644 index 000000000..f0cf4266f --- /dev/null +++ b/tests/legacy-env-cleanup.test.ts @@ -0,0 +1,567 @@ +import { afterEach, expect, test } from "bun:test"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readFile, + rename, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { withEnvLayerLock } from "../src/lib/env-layer-lock.ts"; +import { + finishImplicitLegacyEnvMigration, + LegacyEnvCleanupError, + migrateLegacyProjectEnv, + removeLegacyProjectEnvArtifacts, + repairLegacyComposeEnvFileReferences, + setProjectEnvValue, +} from "../src/lib/project-env-config.ts"; +import { + resolveSecretStore, + withEncryptedFileKeyLock, +} from "../src/lib/secret-store.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +const SENTINEL = "synthetic-cleanup-value"; +const roots: string[] = []; +afterEach(async () => { + for (const root of roots.splice(0)) { + await rm(root, { recursive: true, force: true }); + } +}); + +async function fixture() { + const projectRoot = await mkdtemp(join(tmpdir(), "hack-cleanup-")); + roots.push(projectRoot); + const projectDir = join(projectRoot, ".hack"); + await mkdir(projectDir); + await writeFile(join(projectDir, "hack.config.json"), '{"name":"fixture"}\n'); + await writeFile( + join(projectDir, "docker-compose.yml"), + "services:\n app:\n image: alpine:3.22\n env_file:\n - .env\n" + ); + const path = join(projectDir, ".env"); + await writeFile(path, `TOKEN=${SENTINEL}\n`); + return { + projectRoot, + projectDir, + projectName: "fixture", + serviceNames: ["app"], + path, + }; +} + +async function migrate(options: Awaited>) { + const migrated = await migrateLegacyProjectEnv({ + ...options, + materialize: false, + migrateUndeclaredDotenv: true, + composeRepairFollows: true, + }); + await repairLegacyComposeEnvFileReferences({ + composeFile: join(options.projectDir, "docker-compose.yml"), + projectDir: options.projectDir, + }); + return { + projectRoot: options.projectRoot, + paths: [options.path], + context: migrated.cleanupContext, + migrated, + }; +} + +async function refused( + options: Parameters[0] +) { + const error = await removeLegacyProjectEnvArtifacts(options).then( + () => null, + (caught: unknown) => caught + ); + expect(error).toBeInstanceOf(Error); + expect(String(error)).toContain( + "Reconcile chosen values with hack env set KEY --value-stdin" + ); + expect(String(error)).toContain("--local for machine-local values"); + expect(String(error)).not.toContain(SENTINEL); +} + +test("unchanged sources are removed only after current delivery and Compose references are checked", async () => { + const options = await fixture(); + const cleanup = await migrate(options); + expect(await removeLegacyProjectEnvArtifacts(cleanup)).toEqual([ + options.path, + ]); + expect(await Bun.file(options.path).exists()).toBe(false); +}); + +test("implicit cleanup reports a real writer lock timeout without failing the command", async () => { + const options = await fixture(); + await writeFile( + join(options.projectDir, "docker-compose.yml"), + "services:\n app:\n image: alpine:3.22\n" + ); + await writeFile( + join(options.projectDir, "hack.env.json"), + JSON.stringify({ + version: 1, + vars: [{ key: "TOKEN", source: "plain_env", required: false }], + }) + ); + const migrated = await migrateLegacyProjectEnv({ + ...options, + materialize: false, + }); + expect(migrated.cleanupCandidates).toContain(options.path); + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const writer = withEnvLayerLock({ + layerPath: options.path, + run: async () => { + entered.resolve(); + await release.promise; + }, + }); + await entered.promise; + try { + const report = await finishImplicitLegacyEnvMigration({ + ...options, + migrated, + }); + expect(report.removed).toEqual([]); + expect(report.lines.join("\n")).toContain("writer may still hold a lock"); + expect(report.lines.join("\n")).not.toContain(SENTINEL); + expect(await Bun.file(options.path).exists()).toBe(true); + } finally { + release.resolve(); + await writer; + } +}, 20_000); + +async function withMigratedStore( + run: (options: Awaited>) => Promise +) { + const savedKey = process.env.HACK_SECRETS_FILE_KEY; + const savedFallback = process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK; + process.env.HACK_SECRETS_FILE_KEY = "synthetic-cleanup-encryption-key"; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + try { + await run(await migratedStore()); + } finally { + restoreEnv("HACK_SECRETS_FILE_KEY", savedKey); + restoreEnv("HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", savedFallback); + } +} + +async function migratedStore() { + const options = await fixture(); + const storePath = join(options.projectRoot, "legacy-store.json"); + const keyPath = join(options.projectRoot, "legacy-store.key"); + const contractPath = join(options.projectDir, "hack.env.json"); + await writeFile(keyPath, "synthetic-cleanup-encryption-key\n"); + await writeFile( + join(options.projectDir, "hack.config.json"), + JSON.stringify({ + name: "fixture", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath, keyPath }, + }, + }, + }) + ); + await writeFile( + contractPath, + JSON.stringify({ + version: 1, + vars: [{ key: "TOKEN", source: "keychain", required: false }], + }) + ); + const store = await resolveSecretStore(options); + await store.set({ key: "TOKEN", value: SENTINEL }); + const cleanup = await migrate(options); + return { ...options, ...cleanup, storePath, keyPath, contractPath }; +} + +test("a changed YAML store value keeps the unchanged encrypted store and key", async () => { + await withMigratedStore(async (options) => { + const bytes = await readFile(options.storePath, "utf8"); + await setProjectEnvValue({ + ...options, + envName: null, + scope: "global", + key: "TOKEN", + value: `${SENTINEL}-changed`, + secret: true, + local: true, + }); + await refused({ ...options, paths: [options.storePath, options.keyPath] }); + expect(await readFile(options.storePath, "utf8")).toBe(bytes); + expect(await Bun.file(options.keyPath).exists()).toBe(true); + }); +}); + +test("cleanup refuses a key-only batch while its store remains", async () => { + await withMigratedStore(async (options) => { + const bytes = await readFile(options.storePath, "utf8"); + const key = await readFile(options.keyPath, "utf8"); + await refused({ ...options, paths: [options.keyPath] }); + expect(await readFile(options.storePath, "utf8")).toBe(bytes); + expect(await readFile(options.keyPath, "utf8")).toBe(key); + }); +}); + +test.skipIf(process.getuid?.() === 0)( + "an unreadable store is kept with its key", + async () => { + await withMigratedStore(async (options) => { + await chmod(options.storePath, 0o000); + try { + await expect(readFile(options.storePath)).rejects.toThrow(); + await refused({ + ...options, + paths: [options.storePath, options.keyPath], + }); + expect(await Bun.file(options.keyPath).exists()).toBe(true); + } finally { + await chmod(options.storePath, 0o600); + } + expect(await Bun.file(options.storePath).exists()).toBe(true); + }); + } +); + +test.skipIf(process.getuid?.() === 0)( + "partial cleanup reports confirmed removals and retains the key", + async () => { + await withMigratedStore(async (options) => { + await chmod(options.projectDir, 0o500); + try { + const error = await removeLegacyProjectEnvArtifacts({ + ...options, + paths: [options.storePath, options.contractPath, options.keyPath], + }).catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(LegacyEnvCleanupError); + if (!(error instanceof LegacyEnvCleanupError)) { + throw new Error("Expected a safe cleanup outcome"); + } + expect(error.removedPaths).toEqual([options.storePath]); + expect(error.message).toContain(JSON.stringify(options.storePath)); + expect(error.message).not.toContain(SENTINEL); + expect(await Bun.file(options.storePath).exists()).toBe(false); + expect(await Bun.file(options.contractPath).exists()).toBe(true); + expect(await Bun.file(options.keyPath).exists()).toBe(true); + } finally { + await chmod(options.projectDir, 0o700); + } + }); + } +); + +test("cleanup uses store/key owner locks without a root .internal directory", async () => { + await withMigratedStore(async (options) => { + await expect( + lstat(join(options.projectRoot, ".internal")) + ).rejects.toMatchObject({ code: "ENOENT" }); + expect( + await removeLegacyProjectEnvArtifacts({ + ...options, + paths: [options.keyPath, options.storePath], + }) + ).toEqual([options.storePath, options.keyPath]); + await expect( + lstat(join(options.projectRoot, ".internal")) + ).rejects.toMatchObject({ code: "ENOENT" }); + }); +}); + +test("cleanup waits for the key writer and preserves its completed change", async () => { + await withMigratedStore(async (options) => { + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const keyText = "synthetic-concurrent-key-change\n"; + const writer = withEncryptedFileKeyLock({ + keyPath: options.keyPath, + run: async () => { + entered.resolve(); + await release.promise; + await writeFile(options.keyPath, keyText); + }, + }); + await entered.promise; + let settled = false; + const removal = removeLegacyProjectEnvArtifacts({ + ...options, + paths: [options.storePath, options.keyPath], + }).then( + () => { + settled = true; + return null; + }, + (error: unknown) => { + settled = true; + return error; + } + ); + try { + await Bun.sleep(200); + expect(settled).toBe(false); + } finally { + release.resolve(); + await writer; + const error = await removal; + expect(error).toBeInstanceOf(LegacyEnvCleanupError); + expect(String(error)).not.toContain(keyText.trim()); + } + expect(await readFile(options.keyPath, "utf8")).toBe(keyText); + expect(await Bun.file(options.storePath).exists()).toBe(true); + }); +}); + +test("cleanup preserves a source updated while confirmation was pending", async () => { + const options = await fixture(); + const cleanup = await migrate(options); + const current = `TOKEN=${SENTINEL}-new\nADDED=${SENTINEL}\n`; + await writeFile(options.path, current); + await refused(cleanup); + expect(await readFile(options.path, "utf8")).toBe(current); +}); + +test("an equal-content replacement has a new identity and is kept", async () => { + const options = await fixture(); + const cleanup = await migrate(options); + const current = await readFile(options.path, "utf8"); + await writeFile(`${options.path}.replacement`, current); + await rename(`${options.path}.replacement`, options.path); + await refused(cleanup); + expect(await readFile(options.path, "utf8")).toBe(current); +}); + +test("implicit migration also carries the source receipt into cleanup", async () => { + const options = await fixture(); + await writeFile( + join(options.projectDir, "docker-compose.yml"), + "services:\n app:\n image: alpine:3.22\n" + ); + await writeFile( + join(options.projectDir, "hack.env.json"), + JSON.stringify({ + version: 1, + vars: [{ key: "TOKEN", source: "plain_env", required: false }], + }) + ); + const migrated = await migrateLegacyProjectEnv({ + ...options, + materialize: false, + }); + expect(migrated.cleanupCandidates).toContain(options.path); + await writeFile(options.path, `TOKEN=${SENTINEL}-new\n`); + const report = await finishImplicitLegacyEnvMigration({ + ...options, + migrated, + }); + expect(report.removed).toEqual([]); + expect(report.lines.join("\n")).toContain("kept remaining legacy env files"); + expect(report.lines.join("\n")).not.toContain(SENTINEL); + expect(await readFile(options.path, "utf8")).toBe(`TOKEN=${SENTINEL}-new\n`); +}); + +test("cleanup rechecks current YAML delivery after confirmation", async () => { + const options = await fixture(); + const cleanup = await migrate(options); + await setProjectEnvValue({ + ...options, + envName: null, + scope: "global", + key: "TOKEN", + value: `${SENTINEL}-changed`, + secret: false, + local: true, + }); + await refused(cleanup); + expect(await readFile(options.path, "utf8")).toBe(`TOKEN=${SENTINEL}\n`); +}); + +async function scopedFixture() { + const options = await fixture(); + await writeFile( + join(options.projectDir, "hack.env.json"), + JSON.stringify({ + version: 1, + vars: [ + { + key: "TOKEN", + source: "plain_env", + required: false, + services: ["app"], + }, + ], + }) + ); + return options; +} + +test("cleanup rechecks a service-scoped value after confirmation", async () => { + const options = await scopedFixture(); + const cleanup = await migrate(options); + await setProjectEnvValue({ + ...options, + envName: null, + scope: "app", + key: "TOKEN", + value: `${SENTINEL}-changed`, + secret: false, + local: true, + }); + await refused(cleanup); + expect(await readFile(options.path, "utf8")).toBe(`TOKEN=${SENTINEL}\n`); +}); + +test("a removed contract service gets a fixed cleanup refusal", async () => { + const options = await scopedFixture(); + const cleanup = await migrate(options); + await writeFile( + join(options.projectDir, "docker-compose.yml"), + "services:\n other:\n image: alpine:3.22\n" + ); + await writeFile( + join(options.projectDir, "hack.env.local.yaml"), + "version: 1\nsecretsprovider: project_key\nvalues:\n global: {}\n" + ); + await refused(cleanup); + expect(await Bun.file(options.path).exists()).toBe(true); +}); + +test("cleanup keeps a source when Compose re-adds its env_file reference", async () => { + const options = await fixture(); + const cleanup = await migrate(options); + await writeFile( + join(options.projectDir, "docker-compose.yml"), + "services:\n app:\n image: alpine:3.22\n env_file: .env\n" + ); + await refused(cleanup); + expect(await Bun.file(options.path).exists()).toBe(true); +}); + +test("cleanup keeps a scoped key newly used for Compose interpolation", async () => { + const options = await scopedFixture(); + const cleanup = await migrate(options); + await writeFile( + join(options.projectDir, "docker-compose.yml"), + 'services:\n app:\n image: alpine:3.22\n environment:\n OTHER: "${TOKEN}"\n' + ); + await refused(cleanup); + expect(await Bun.file(options.path).exists()).toBe(true); +}); + +test("a second cleanup confirmation works after the first removed untracked sources", async () => { + const options = await fixture(); + const contractPath = join(options.projectDir, "hack.env.json"); + await writeFile( + contractPath, + JSON.stringify({ + version: 1, + vars: [{ key: "TOKEN", source: "plain_env", required: false }], + }) + ); + const cleanup = await migrate(options); + expect(await removeLegacyProjectEnvArtifacts(cleanup)).toEqual([ + options.path, + ]); + expect( + await removeLegacyProjectEnvArtifacts({ ...cleanup, paths: [contractPath] }) + ).toEqual([contractPath]); +}); + +test("cleanup waits for the dotenv writer lock and rechecks the completed write", async () => { + const options = await fixture(); + const cleanup = await migrate(options); + const entered = Promise.withResolvers(); + const release = Promise.withResolvers(); + const writer = withEnvLayerLock({ + layerPath: options.path, + run: async () => { + entered.resolve(); + await release.promise; + await writeFile(options.path, `TOKEN=${SENTINEL}-concurrent\n`); + }, + }); + await entered.promise; + let settled = false; + const removal = refused(cleanup).finally(() => { + settled = true; + }); + try { + await Bun.sleep(30); + expect(settled).toBe(false); + } finally { + release.resolve(); + await writer; + await removal; + } + expect(await readFile(options.path, "utf8")).toContain( + `${SENTINEL}-concurrent` + ); +}); + +test("a stale encrypted-store inventory preserves both store and key", async () => { + const options = await fixture(); + const savedKey = process.env.HACK_SECRETS_FILE_KEY; + const savedFallback = process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK; + process.env.HACK_SECRETS_FILE_KEY = "synthetic-cleanup-encryption-key"; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + try { + const storePath = join(options.projectRoot, "legacy-store.json"); + const keyPath = join(options.projectRoot, "legacy-store.key"); + await writeFile(keyPath, "synthetic-cleanup-encryption-key\n"); + await writeFile( + join(options.projectDir, "hack.config.json"), + JSON.stringify({ + name: "fixture", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { path: storePath, keyPath }, + }, + }, + }) + ); + await writeFile( + join(options.projectDir, "hack.env.json"), + JSON.stringify({ + version: 1, + vars: [{ key: "TOKEN", source: "keychain", required: false }], + }) + ); + const store = await resolveSecretStore(options); + await store.set({ key: "TOKEN", value: SENTINEL }); + const migrated = await migrateLegacyProjectEnv({ + ...options, + materialize: false, + composeRepairFollows: true, + }); + expect(migrated.cleanupCandidates).toContain(storePath); + expect(migrated.cleanupCandidates).toContain(keyPath); + await repairLegacyComposeEnvFileReferences({ + composeFile: join(options.projectDir, "docker-compose.yml"), + projectDir: options.projectDir, + }); + await store.set({ key: "ADDED", value: `${SENTINEL}-new` }); + const bytes = await readFile(storePath, "utf8"); + await refused({ + projectRoot: options.projectRoot, + paths: [storePath, keyPath], + context: migrated.cleanupContext, + }); + expect(await readFile(storePath, "utf8")).toBe(bytes); + expect(await Bun.file(keyPath).exists()).toBe(true); + expect(await store.get({ key: "ADDED" })).toBe(`${SENTINEL}-new`); + } finally { + restoreEnv("HACK_SECRETS_FILE_KEY", savedKey); + restoreEnv("HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", savedFallback); + } +}); diff --git a/tests/project-env-config.test.ts b/tests/project-env-config.test.ts index 8c3e2decf..204cd5b44 100644 --- a/tests/project-env-config.test.ts +++ b/tests/project-env-config.test.ts @@ -1630,6 +1630,7 @@ test("migrateLegacyProjectEnv converts legacy base and overlay values into new c const removed = await removeLegacyProjectEnvArtifacts({ projectRoot: repo.projectRoot, paths: [storePath], + context: migrated.cleanupContext, }); expect( await dropRemovedLegacySecretStorePointer({