diff --git a/docs/env.md b/docs/env.md index b7fd428a9..3873ccade 100644 --- a/docs/env.md +++ b/docs/env.md @@ -608,10 +608,11 @@ per-checkout values you don't want to commit. With no env layers yet, `hack env set --local` writes legacy `.hack/.env` (or `.hack/.env.`), including when a `hack.env.json` contract exists. Use `hack env add --local` to migrate first and then write the machine-local layer. -Contract migration writes `plain_env` values from legacy dotenv files (or the shell) -to the gitignored machine-local layer, never a tracked layer. It reports each moved -key and destination path, without values. It refuses a local destination already -tracked by Git. Values declared as `keychain` migrate to tracked layers as ciphertext. +Contract migration writes `plain_env` values from legacy dotenv files to the gitignored +machine-local layer, never a tracked layer, and refuses a `plain_env` value that comes only +from your shell. It reports each moved key and destination path, without values. It refuses +a local destination already tracked by Git. Values declared as `keychain` migrate to tracked +layers as ciphertext. Migration requires effective ignore protection before writing plaintext. It refuses an outdated read-only `.hack/.gitignore` or a local destination that Git does not ignore. Without `hack.env.json`, `set --local` still writes `.hack/.env`; if a contract is @@ -1218,6 +1219,44 @@ files, when: that resolves to a readable file inside `.hack` is migrated like a regular one; cleanup removes only the link, never its target. +With `hack.env.json`, the migration refuses (`E_CONFIG_INVALID`) when a `plain_env` value comes only +from your shell, because it would otherwise be saved to a plaintext file you didn't ask for. This applies to commands +that migrate implicitly, such as `hack env exec`. Simply skipping the value isn't enough, because +env layers don't read the contract, so services would stop receiving it. The error names the key and +never the value, and gives the commands for the scope the contract declares: + +1. Run one command without the variable, so the migration runs: `env -u KEY hack env exec -- true`. + Add a `-u` for each key it names. +2. Set it for the declared scope: `hack env set KEY --service --local --value-stdin`, or + without `--service` for a key the contract doesn't scope. `--local` alone delivers it to every + service, so keep `--service` for a scoped key. +3. If an overlay file also sets the key, the migration moves that value to the overlay's + machine-local layer, which outranks the local base, so the overlay keeps it. To change it: + `hack env set KEY --env --local --value-stdin`, with `--service` as above. + The error prints a command only for a name the CLI accepts. It names an overlay file `--env` + can't select, such as `.env.My_Staging` or `.env.default`, or a service `--service` can't. Fix + those before step 1, because env layers don't read the contract once the project migrates. +4. Keep it exported if Compose interpolation, or a host command run without that service's scope, + reads a key you set with `--service`, because a service value doesn't reach them. A value set + without `--service` does reach them, and overrides your shell. + +The refusal comes before Hack provisions any secret-store key. If plaintext values are mirrored to a +secret backend whose key isn't available, Hack can't tell whether a key your shell sets is also in +the backend, so it refuses that key the same way. Make the backend's key available and retry. If the +backend has nothing stored yet and `HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK` isn't set, you can run +these steps instead, and the migration creates its key; set the key only if the backend doesn't hold +it. Any other backend failure, such as a corrupt store, stops the migration with that error. + +With `hack.env.json`, the migration also refuses (`E_CONFIG_INVALID`), writing no env layer, an +overlay named `default`, `local` or `.local`, in any case, that sets a value of its own, because +it would replace the base layer or write a machine-local file, and a contract service that isn't a +valid env scope when a value would be written for it, because no read could parse that layer. A +legacy `.env.default` or `.env.local` refuses even when it sets no value of its own, and so does a +different-case spelling of either, through the destination check. A +secret backend's key may already have been created by then. The migration also refuses, writing nothing +itself, a `hack.env.json` it can't open or read as a contract, and it doesn't quote the file. Earlier +`hack doctor` steps, such as creating `.hack/.gitignore`, can still have run. + With `hack.env.json`, the migration discovers only regular `.hack/.env.` files, so a symlinked overlay is not migrated. It refuses (`E_PATH_CONFLICT`) when `.hack/.env` or a configured default overlay is a symlink, wherever it points, because another file's values, diff --git a/docs/security/env-secrets-threat-model.md b/docs/security/env-secrets-threat-model.md index 27a81c8da..a319b591c 100644 --- a/docs/security/env-secrets-threat-model.md +++ b/docs/security/env-secrets-threat-model.md @@ -72,6 +72,7 @@ refer to `hack-dance/hack`. | Threat | Mitigation on `next` | | --- | --- | +| A repository's contract names a variable exported in the developer's shell, so the migration saves it to disk. | `migrateLegacyProjectEnv` refuses, naming the key only, when a `plain_env` value resolves from the process environment (HACK-1688). A value in a regular `.hack/.env` migrates to the gitignored machine-local layer, never a tracked one (HACK-1712). | | A repository plants `.hack.secret.key` as a symlink (dangling or not) so that Hack reads or writes a file it should not. | Key reads go through `readBoundedPrivateFile` (`lstat`, `O_NOFOLLOW`, descriptor and path identity checks, 1 MiB bound, fatal UTF-8). New keys are created with `createPrivateFileExclusive` (`O_CREAT|O_EXCL|O_NOFOLLOW`, mode 0600, `fchmod`, `fsync`). Both are in `src/lib/private-file.ts`, used by `resolveProjectEnvKey`, `ensureProjectEnvSecretKey` and `resolveInheritedProjectEnvKey` in `src/lib/project-env-config.ts` (#288). | | A symlinked `.hack/.env`, overlay file or `.hack` directory redirects a write, or a reader sees a partial file. | `src/lib/private-env-file.ts` (`readPrivateEnvFile`, `writePrivateEnvFileIfChanged`) and `replacePrivateFileAtomic` in `src/lib/private-file.ts`: a private sibling is written and fsynced, then renamed over the target; symlinks and non-regular entries are refused with one fixed message (#302). Used by `src/lib/hack-env.ts` and `materializeProjectEnv`. | | A world-readable env file or store. | `.hack/.env`, overlays and the `encrypted_file` store are written 0600, and a same-content file with group or other access is tightened (#302). The `hack doctor` env key check reports `env-key-weak-permissions` and `env-key-wrong-owner` (#309). | diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 7b292a1c2..f5b2f027b 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -119,7 +119,10 @@ import { import { resolveInstalledGrafanaHost } from "../lib/global-service-host.ts"; import { getString, isRecord } from "../lib/guards.ts"; import { resolveHackInvocation } from "../lib/hack-cli.ts"; -import { inspectHackEnvOverlayWarnings } from "../lib/hack-env.ts"; +import { + inspectHackEnvOverlayWarnings, + readHackEnvContract, +} from "../lib/hack-env.ts"; import { canPrompt, confirmSafe } from "../lib/interactivity.ts"; import { findHackLocalCaCertPath } from "../lib/local-ca.ts"; import { noteBrowserTargetsForLocalNetworkHint } from "../lib/local-network-hint.ts"; @@ -2674,14 +2677,23 @@ async function checkProjectEnvMode({ } const contractPath = resolve(ctx.projectDir, PROJECT_ENV_CONTRACT_FILENAME); - const contractExists = await pathExists(contractPath); - if (!contractExists) { + const contract = await readHackEnvContract({ projectDir: ctx.projectDir }); + if (!contract.exists) { return { name: "env mode", status: "warn", message: `Missing ${contractPath} and no .hack/hack.env.default.yaml present`, }; } + // The migration refuses this contract, so don't send the user to it. The parser's + // message isn't printed, because it can quote the file. + if (contract.parseError !== undefined) { + return { + name: "env mode", + status: "warn", + message: `${contractPath} can't be read or parsed. Correct it or remove it, then run \`hack doctor --migrate-env-config\`.`, + }; + } return { name: "env mode", @@ -3720,6 +3732,33 @@ function buildEnvMigrationNote(opts: { return notes.join("\n"); } +/** Describes the env migration and asks whether to run it. */ +async function confirmProjectEnvMigration(opts: { + readonly projectDir: string; + readonly contractPath: string; + readonly hasContract: boolean; +}): Promise { + const { projectDir, contractPath, hasContract } = opts; + note( + [ + "Detected legacy project env files.", + hasContract + ? `- source: ${contractPath} and current local env state` + : `- source: ${resolve(projectDir, PROJECT_ENV_FILENAME)} and its overlays (no ${PROJECT_ENV_CONTRACT_FILENAME}; secret-backend values are not migrated)`, + hasContract + ? `- target: ${resolve(projectDir, "hack.env.default.yaml")}` + : `- target: values go to the gitignored ${resolve(projectDir, "hack.env.local.yaml")} (and hack.env..local.yaml); ${resolve(projectDir, "hack.env.default.yaml")} is written with no values`, + "- runtime: direct env injection by default; .hack/.env becomes compatibility-only", + ].join("\n"), + "env migration" + ); + return await doctorConfirm({ + message: + "Migrate project env from .hack/hack.env.json/.hack/.env to hack.env.*.yaml now?", + initialValue: true, + }); +} + async function maybeMigrateProjectEnvConfig(opts: { readonly startDir: string; }): Promise { @@ -3761,7 +3800,10 @@ async function maybeMigrateProjectEnvConfig(opts: { project.projectDir, PROJECT_ENV_CONTRACT_FILENAME ); - const hasContract = await pathExists(contractPath); + const contract = await readHackEnvContract({ + projectDir: project.projectDir, + }); + const hasContract = contract.exists; const cfg = await readProjectConfig(project); const projectName = ( @@ -3772,25 +3814,15 @@ async function maybeMigrateProjectEnvConfig(opts: { const serviceNames = await discoverComposeServiceNames({ composeFile: project.composeFile, }); - note( - [ - "Detected legacy project env files.", - hasContract - ? `- source: ${contractPath} and current local env state` - : `- source: ${resolve(project.projectDir, PROJECT_ENV_FILENAME)} and its overlays (no ${PROJECT_ENV_CONTRACT_FILENAME}; secret-backend values are not migrated)`, - hasContract - ? `- target: ${resolve(project.projectDir, "hack.env.default.yaml")}` - : `- target: values go to the gitignored ${resolve(project.projectDir, "hack.env.local.yaml")} (and hack.env..local.yaml); ${resolve(project.projectDir, "hack.env.default.yaml")} is written with no values`, - "- runtime: direct env injection by default; .hack/.env becomes compatibility-only", - ].join("\n"), - "env migration" - ); - const shouldMigrate = await doctorConfirm({ - message: - "Migrate project env from .hack/hack.env.json/.hack/.env to hack.env.*.yaml now?", - initialValue: true, - }); - if (!shouldMigrate) { + // The migration refuses this contract, so don't describe it or ask first. + if ( + contract.parseError === undefined && + !(await confirmProjectEnvMigration({ + projectDir: project.projectDir, + contractPath, + hasContract, + })) + ) { return; } @@ -4079,11 +4111,13 @@ async function buildEnvMigrationPlanStep(opts: { return "Skip env migration because this project already uses hack.env.*.yaml."; } - const contractPath = resolve( - opts.project.projectDir, - PROJECT_ENV_CONTRACT_FILENAME - ); - if (await pathExists(contractPath)) { + const contract = await readHackEnvContract({ + projectDir: opts.project.projectDir, + }); + if (contract.parseError !== undefined) { + return "Refuse env migration (E_CONFIG_INVALID) because .hack/hack.env.json can't be read or parsed."; + } + if (contract.exists) { return "Prompt to migrate legacy env config (.hack/hack.env.json) to hack.env.*.yaml."; } diff --git a/src/lib/hack-env.ts b/src/lib/hack-env.ts index ad920c22f..8e52373a2 100644 --- a/src/lib/hack-env.ts +++ b/src/lib/hack-env.ts @@ -7,7 +7,7 @@ import { import { readControlPlaneConfig } from "../control-plane/sdk/config.ts"; import { parseDotEnv, serializeDotEnv } from "./env.ts"; import { withEnvLayerLock } from "./env-layer-lock.ts"; -import { readTextFile } from "./fs.ts"; +import { readOptionalTextFile, readTextFile } from "./fs.ts"; import { ensureHackGitInfoExclude } from "./git-info-exclude.ts"; import { getString, isRecord } from "./guards.ts"; import { @@ -19,6 +19,7 @@ import type { SecretStoreDescriptor } from "./secret-store.ts"; import { resolveSecretStore, resolveSecretStoreDescriptor, + SecretStoreKeyUnavailableError, } from "./secret-store.ts"; export const HACK_ENV_VERSION = 1 as const; @@ -187,14 +188,27 @@ export type HackEnvStorageSummary = { readonly portableState: HackEnvPortableStateSummary; }; +const CONTRACT_UNREADABLE = "the file can't be read"; + export async function readHackEnvContract(opts: { readonly projectDir: string; }): Promise { const path = resolve(opts.projectDir, PROJECT_ENV_CONTRACT_FILENAME); - const text = await readTextFile(path); - if (text === null) { + const read = await readOptionalTextFile(path); + if (read.kind === "absent") { return { path, exists: false, contract: defaultHackEnvContract() }; } + // A contract that can't be opened isn't a project without one: callers that took it + // for absent created layers that bypass .hack/.env, or ignored its scopes. + if (read.kind === "unreadable") { + return { + path, + exists: true, + contract: defaultHackEnvContract(), + parseError: CONTRACT_UNREADABLE, + }; + } + const { text } = read; let parsed: unknown; try { @@ -274,6 +288,136 @@ export async function resolveHackEnvValue(opts: { return state.value; } +export type ProcessOnlyContractValue = { + readonly key: string; + readonly services: readonly string[] | null; + /** Overlays that supply the key themselves, so they don't take the process value. */ + readonly overlaysWithOwnValue: readonly string[]; + /** + * The process sets the key, and a mirrored backend that could also hold it has no + * key available, so whether the base takes it from the process is unknown. A + * caller that must not write a shell value has to treat this as if it did. + */ + readonly undetermined: boolean; +}; + +/** + * Contract values the base env may take from this process's environment. It never + * generates or copies key material. A key the process doesn't set, or that the base + * file sets, can't come from the process, so only a key the process sets alone + * reads the backend, and only when plaintext values are mirrored there. Only a + * missing backend key counts as undetermined: any other failure, such as a corrupt + * store or an incomplete config, propagates, so nothing is written either way. + */ +export async function findProcessOnlyContractValues(opts: { + readonly projectDir: string; + readonly projectName: string; + readonly overlayNames: readonly string[]; +}): Promise { + const { contract } = await readHackEnvContract({ + projectDir: opts.projectDir, + }); + const baseText = await readTextFile( + resolve(opts.projectDir, PROJECT_ENV_FILENAME) + ); + const baseDotenv = baseText === null ? {} : parseDotEnv(baseText); + const candidates = contract.vars.filter( + (contractVar) => + contractVar.source !== "keychain" && + isPresentEnvValue(process.env[contractVar.key]) && + !isPresentEnvValue(baseDotenv[contractVar.key]) + ); + if (candidates.length === 0) { + return []; + } + const { storePlaintextInBackend } = await readHackEnvRuntimeConfig({ + projectDir: opts.projectDir, + }); + const overlays: { + readonly name: string; + readonly dotenv: Readonly>; + }[] = []; + for (const name of opts.overlayNames) { + const text = await readTextFile( + resolveEnvFilePath({ projectDir: opts.projectDir, envName: name }) + ); + overlays.push({ name, dotenv: text === null ? {} : parseDotEnv(text) }); + } + + let secretStore: { + readonly get: (input: { readonly key: string }) => Promise; + } = { get: async () => null }; + if (storePlaintextInBackend) { + try { + secretStore = await resolveSecretStore({ + projectName: opts.projectName, + projectDir: opts.projectDir, + allowKeyProvisioning: false, + }); + } catch (error: unknown) { + if (!(error instanceof SecretStoreKeyUnavailableError)) { + throw error; + } + return candidates.map((contractVar) => ({ + key: contractVar.key, + services: contractVar.services, + // A backend value can't be seen here, but an overlay's file still can. + overlaysWithOwnValue: overlays + .filter((overlay) => + isPresentEnvValue(overlay.dotenv[contractVar.key]) + ) + .map((overlay) => overlay.name), + undetermined: true, + })); + } + } + const resolvedFrom = async (input: { + readonly contractVar: HackEnvVar; + readonly envName: string | null; + readonly overlayDotenv: Readonly>; + }) => + ( + await resolveHackEnvValueState({ + contractVar: input.contractVar, + storePlaintextInBackend, + effectiveEnvName: input.envName, + baseDotenv, + overlayDotenv: input.overlayDotenv, + secretStore, + }) + ).resolvedFrom; + + const found: ProcessOnlyContractValue[] = []; + for (const contractVar of candidates) { + const fromBase = await resolvedFrom({ + contractVar, + envName: null, + overlayDotenv: {}, + }); + if (fromBase !== "process") { + continue; + } + const overlaysWithOwnValue: string[] = []; + for (const overlay of overlays) { + const fromOverlay = await resolvedFrom({ + contractVar, + envName: overlay.name, + overlayDotenv: overlay.dotenv, + }); + if (fromOverlay !== "process") { + overlaysWithOwnValue.push(overlay.name); + } + } + found.push({ + key: contractVar.key, + services: contractVar.services, + overlaysWithOwnValue, + undetermined: false, + }); + } + return found; +} + export async function resolveHackEnv(opts: { readonly projectDir: string; readonly projectName: string; @@ -752,12 +896,18 @@ function buildHackEnvMutationSourceMismatchMessage(input: { return `Env key "${input.key}" is declared as source "plain_env" in ${input.contractPath}. Use "hack env set ${input.key}=…" or update the contract source before writing. Hack will not write this key through the secret backend until the contract source changes.`; } +function describeContractProblem(parseError: string): string { + return parseError === CONTRACT_UNREADABLE + ? "can't be read" + : `is malformed (${parseError})`; +} + function buildHackEnvMalformedContractMessage(input: { readonly key: string; readonly contractPath: string; readonly parseError: string; }): string { - return `Env contract ${input.contractPath} is malformed (${input.parseError}). Fix or remove the contract file before writing "${input.key}". Hack will not fall back to the default contract while mutation-source validation cannot trust the declared storage rules.`; + return `Env contract ${input.contractPath} ${describeContractProblem(input.parseError)}. Fix or remove the contract file before writing "${input.key}". Hack will not fall back to the default contract while mutation-source validation cannot trust the declared storage rules.`; } function describePortableState(input: { diff --git a/src/lib/logs.ts b/src/lib/logs.ts index e44423667..5303643ae 100644 --- a/src/lib/logs.ts +++ b/src/lib/logs.ts @@ -361,7 +361,7 @@ function trimmedWords(values: readonly string[] | undefined): string[] { const SHELL_SAFE_WORD = /^[A-Za-z0-9_@%+=:,./-]+$/; -function quoteShellWord(value: string): string { +export function quoteShellWord(value: string): string { if (SHELL_SAFE_WORD.test(value)) { return value; } diff --git a/src/lib/project-env-config.ts b/src/lib/project-env-config.ts index 5b347ebd7..b48c8dc28 100644 --- a/src/lib/project-env-config.ts +++ b/src/lib/project-env-config.ts @@ -61,11 +61,13 @@ import { } from "./git-worktree.ts"; import { getRecord, getString, isRecord } from "./guards.ts"; import { - type HackEnvResolveResult, + findProcessOnlyContractValues, + type ProcessOnlyContractValue, readHackEnvContract, resolveEnvFilePath, resolveHackEnv, } from "./hack-env.ts"; +import { quoteShellWord } from "./logs.ts"; import { isOwnedLegacyAdoptionManagedEnvAdmission, type LegacyAdoptionManagedEnvAdmission, @@ -5139,6 +5141,15 @@ export async function migrateLegacyProjectEnv(opts: { await assertLegacyProjectDirectory({ projectDir: opts.projectDir }); await assertLegacyProjectInputFamily({ projectRoot: opts.projectRoot }); const contract = await readHackEnvContract({ projectDir: opts.projectDir }); + // Read as an empty contract, it would migrate nothing and offer .env and the contract + // for cleanup. The parser's message isn't repeated, because it can quote the file. + if (contract.exists && contract.parseError !== undefined) { + throw new LegacyDotenvMigrationRefusalError({ + code: "E_CONFIG_INVALID", + reason: + "hack.env.json can't be read as an env contract, so the migration can't tell which values to keep. Correct it, or remove it if this project doesn't use a contract, then retry.", + }); + } if (!contract.exists) { if (opts.migrateUndeclaredDotenv === true) { return await migrateUndeclaredLegacyDotenv(opts); @@ -5188,11 +5199,35 @@ export async function migrateLegacyProjectEnv(opts: { overlayNames, }); + // Before resolveHackEnv, which can provision secret-store key material. + assertNoShellValuesInContractMigration({ + values: await findProcessOnlyContractValues({ + projectDir: opts.projectDir, + projectName: opts.projectName, + overlayNames, + }), + }); const baseResolved = await resolveHackEnv({ projectDir: opts.projectDir, projectName: opts.projectName, envName: null, }); + const overlaysResolved: { + readonly name: string; + readonly values: Awaited>["values"]; + }[] = []; + for (const name of overlayNames) { + const overlay = await resolveHackEnv({ + projectDir: opts.projectDir, + projectName: opts.projectName, + envName: name, + }); + overlaysResolved.push({ name, values: overlay.values }); + } + assertContractMigrationLayersAreWritable({ + baseValues: baseResolved.values, + overlays: overlaysResolved, + }); if (overlayNames.includes("default")) { throw new ClosedCliError( @@ -5209,29 +5244,16 @@ export async function migrateLegacyProjectEnv(opts: { projectDir: opts.projectDir, overlayNames, }); - const resolvedOverlays: { name: string; resolved: HackEnvResolveResult }[] = - []; - for (const name of overlayNames) { - resolvedOverlays.push({ - name, - resolved: await resolveHackEnv({ - projectDir: opts.projectDir, - projectName: opts.projectName, - envName: name, - }), - }); - } const keyInfo = baseResolved.values.some( (value) => value.source === "keychain" ) ? await ensureProjectEnvSecretKey({ projectRoot: opts.projectRoot }) : null; if ( - [baseResolved, ...resolvedOverlays.map((overlay) => overlay.resolved)].some( - (resolved) => - resolved.values.some( - (value) => value.source === "plain_env" && value.value !== null - ) + [baseResolved, ...overlaysResolved].some((resolved) => + resolved.values.some( + (value) => value.source === "plain_env" && value.value !== null + ) ) ) { await ensureLegacyMigrationLocalLayersIgnored({ @@ -5323,16 +5345,13 @@ export async function migrateLegacyProjectEnv(opts: { wroteFiles.push(defaultPath); } - for (const { - name: overlayName, - resolved: overlayResolved, - } of resolvedOverlays) { + for (const { name: overlayName, values: overlayValues } of overlaysResolved) { deliveredValues.set( resolve(opts.projectDir, `${PROJECT_ENV_FILENAME}.${overlayName}`), - new Map(overlayResolved.values.map((entry) => [entry.key, entry.value])) + new Map(overlayValues.map((entry) => [entry.key, entry.value])) ); const hasPlainOverlayEntries = await migratePlainValues({ - resolvedValues: overlayResolved.values, + resolvedValues: overlayValues, envName: overlayName, baseValues: new Map( baseResolved.values.map((value) => [value.key, value.value]) @@ -5341,7 +5360,7 @@ export async function migrateLegacyProjectEnv(opts: { const overlayConfig = buildMigratedProjectEnvConfig({ environment: overlayName, values: buildMigratedValues({ - resolvedValues: overlayResolved.values.filter( + resolvedValues: overlayValues.filter( (entry) => entry.source === "keychain" ), baseValues: new Map( @@ -5424,10 +5443,12 @@ export class LegacyDotenvMigrationRefusalError extends HackCliError { constructor(opts: { readonly code: "E_CONFIG_INVALID" | "E_PATH_CONFLICT" | "E_PERMISSION"; readonly reason: string; + /** Replaces the closing sentence, for a refusal that can follow a key being created. */ + readonly closing?: string; }) { super({ code: opts.code, - message: `Refusing to migrate the legacy .hack/.env files: ${opts.reason} Nothing was written, and the legacy files are kept.`, + message: `Refusing to migrate the legacy .hack/.env files: ${opts.reason} ${opts.closing ?? "Nothing was written, and the legacy files are kept."}`, }); this.name = "LegacyDotenvMigrationRefusalError"; } @@ -6415,6 +6436,217 @@ export async function canMigrateCheckoutLegacyDotenv(opts: { ); } +/** + * Refuses when a contract value comes only from this process's environment. A + * repository's contract can name any variable, and this migration saves plain + * values to disk, so a shell credential would land in a plaintext file nobody asked + * for. Skipping it isn't enough: env layers don't read the contract, so a service that received it + * would silently stop. + * + * Checking the base covers every overlay. An overlay falls back to the process only + * when its own sources and the base's lack the key, and then the base does too. + * + * The recovery keeps each key's declared scope, because `--local` alone would + * deliver it to every service. An overlay file's own value moves to that overlay's + * machine-local layer, which outranks the local base, so the overlay command only + * changes it later. Names from the repository are quoted, because these commands get + * pasted. Only a name the CLI accepts unchanged gets a command: `--env base`, for + * example, selects no overlay, so following it would replace the base value. Any other + * name is reported. + * + * It also refuses when a mirrored backend's key isn't available, because the shell + * value might be the only one. + */ +function assertNoShellValuesInContractMigration(opts: { + readonly values: readonly ProcessOnlyContractValue[]; +}): void { + if (opts.values.length === 0) { + return; + } + const named = opts.values.filter((value) => + PROJECT_ENV_KEY_PATTERN.test(value.key) + ); + const malformed = opts.values.length - named.length; + const parts = [ + ...named.map((value) => JSON.stringify(value.key)).sort(), + ...(malformed > 0 + ? [countLabel({ count: malformed, noun: "malformed key" })] + : []), + ]; + const undetermined = opts.values.some((value) => value.undetermined); + const sentences = [ + undetermined + ? `the contract declares ${parts.join(", ")} as plain_env, your shell sets it, and the secret backend's key isn't available, so the value may come only from your shell, and migrating could save it to a plaintext file on this machine without you asking. No key was created. Make the backend's key available and retry. If the backend has nothing stored yet and HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK isn't set, the steps below also work, and the migration then creates its key.` + : `the contract declares ${parts.join(", ")} as plain_env, and the value comes only from your shell, so migrating would save it to a plaintext file on this machine without you asking.`, + ]; + if (malformed > 0) { + sentences.push( + "Correct or remove a malformed key in hack.env.json, and unset it in your shell." + ); + } + if (named.length > 0) { + const unset = named + .map((value) => `-u ${value.key}`) + .sort() + .join(" "); + const setCommand = (input: { + readonly key: string; + readonly envName?: string; + readonly service?: string; + }) => + [ + `hack env set ${input.key}`, + ...(input.envName ? [`--env ${quoteShellWord(input.envName)}`] : []), + ...(input.service + ? [`--service ${quoteShellWord(input.service)}`] + : []), + "--local --value-stdin", + ].join(" "); + const isUsableOverlay = (name: string) => + parseEnvConfigSelection(name) === name && + !isReservedLegacyOverlayName(name.toLowerCase()); + const isUsableService = (service: string) => + isValidProjectEnvScopeName({ scopeName: service }); + const scopesOf = (value: ProcessOnlyContractValue) => + value.services && value.services.length > 0 + ? value.services.filter(isUsableService).map((service) => ({ service })) + : [{}]; + const baseCommands = named + .flatMap((value) => + scopesOf(value).map((scope) => setCommand({ key: value.key, ...scope })) + ) + .sort(); + const overlayCommands = named + .flatMap((value) => + value.overlaysWithOwnValue + .filter(isUsableOverlay) + .flatMap((envName) => + scopesOf(value).map((scope) => + setCommand({ key: value.key, envName, ...scope }) + ) + ) + ) + .sort(); + const unusableOverlays = named + .flatMap((value) => value.overlaysWithOwnValue) + .filter((name) => !isUsableOverlay(name)) + .map((name) => JSON.stringify(`.env.${name}`)); + const unusableServices = named + .flatMap((value) => value.services ?? []) + .filter((service) => !isUsableService(service)) + .map((service) => JSON.stringify(service)); + // First, because the layers don't read the contract or these files once it migrates. + if (unusableOverlays.length > 0) { + sentences.push( + `First, \`--env\` can't select the overlay file ${[...new Set(unusableOverlays)].sort().join(", ")}, so no command is given for it. Rename it to a lowercase name such as .env.staging and update any env_file or env.defaultOverlay that names it, or remove it.` + ); + } + if (unusableServices.length > 0) { + sentences.push( + `First, \`--service\` can't select the service ${[...new Set(unusableServices)].sort().join(", ")} in hack.env.json, so no command is given for it. Correct the name there, renaming the Compose service if it must match.` + ); + } + sentences.push( + `Unset it for one command so the migration runs (\`env ${unset} hack env exec -- true\`). Services then stop receiving it from the shell${ + baseCommands.length > 0 + ? `, so set it again with the scope the contract declares${undetermined ? ", if the backend doesn't hold it" : ""}: ${baseCommands.join("; ")}` + : "" + }.` + ); + if (overlayCommands.length > 0) { + sentences.push( + `The migration moves each overlay file's own value to that overlay's machine-local layer, so it keeps it. To change it later: ${overlayCommands.join("; ")}.` + ); + } + sentences.push( + "A value set with --service doesn't reach Compose interpolation or a host command run without that service's scope, so keep it exported if they read it. A value set without --service reaches them and overrides your shell." + ); + } + throw new LegacyDotenvMigrationRefusalError({ + code: "E_CONFIG_INVALID", + reason: sentences.join(" "), + }); +} + +type ResolvedContractMigrationValue = { + readonly key: string; + readonly value: string | null; + readonly services: readonly string[] | null; +}; + +/** + * Resolving values can create a mirrored backend's key before these refusals, so they + * don't claim nothing at all was written. + */ +const CONTRACT_LAYER_REFUSAL_CLOSING = + "No env layer was written, and the legacy files are kept. A secret backend's key may have been created."; + +/** + * Refuses what this migration can't write as a working layer, before it writes + * anything, because it has no rollback and a retry finds layers and does nothing. + * An overlay named `default` would land on the base layer, and `local` on the + * machine-local one. A scope a layer can't hold would leave a layer no read can parse. + * Only an entry that would be written counts, so a `.local` overlay that adds + * nothing to the base, or a scope with no value, still migrates. `.env.default` and + * `.env.local` are refused separately, whatever they hold, and a case variant of + * either by the destination preflight. Names only, JSON-quoted. + */ +function assertContractMigrationLayersAreWritable(opts: { + readonly baseValues: readonly ResolvedContractMigrationValue[]; + readonly overlays: readonly { + readonly name: string; + readonly values: readonly ResolvedContractMigrationValue[]; + }[]; +}): void { + const base = new Map( + opts.baseValues.map((value) => [value.key, value.value]) + ); + const addedBy = (values: readonly ResolvedContractMigrationValue[]) => + values.filter( + (value) => value.value !== null && base.get(value.key) !== value.value + ); + const reserved = opts.overlays + .filter( + (overlay) => + // Lowercased: on a case-insensitive filesystem .env.Default lands on the same layer. + isReservedLegacyOverlayName(overlay.name.toLowerCase()) && + addedBy(overlay.values).length > 0 + ) + .map((overlay) => JSON.stringify(`.env.${overlay.name}`)) + .sort(); + if (reserved.length > 0) { + throw new LegacyDotenvMigrationRefusalError({ + code: "E_CONFIG_INVALID", + reason: `the overlay file ${reserved.join(", ")} uses a name reserved for Hack's own layers, so migrating it would replace the base layer or write a machine-local file. Rename it to a lowercase name such as .env.staging and update any env_file or env.defaultOverlay that names it, or remove it, then retry.`, + closing: CONTRACT_LAYER_REFUSAL_CLOSING, + }); + } + const written = [ + ...opts.baseValues.filter((value) => value.value !== null), + ...opts.overlays.flatMap((overlay) => addedBy(overlay.values)), + ]; + const invalidScopes = [ + ...new Set( + written + .flatMap((value) => value.services ?? []) + .filter( + (service) => + service.trim().length > 0 && + !isValidProjectEnvScopeName({ scopeName: service }) + ) + ), + ] + .map((service) => JSON.stringify(service)) + .sort(); + if (invalidScopes.length > 0) { + throw new LegacyDotenvMigrationRefusalError({ + code: "E_CONFIG_INVALID", + reason: `hack.env.json declares the service ${invalidScopes.join(", ")}, which isn't a valid env scope, so no read could parse the layer the migration would write. Use lowercase letters, numbers, dots, dashes or underscores, renaming the Compose service if it must match, then retry.`, + closing: CONTRACT_LAYER_REFUSAL_CLOSING, + }); + } +} + /** * Refuses a legacy `.hack/.env` or overlay that is a symlink at all, dangling or * not, for a migration that publishes values to tracked layers. diff --git a/src/lib/secret-store.ts b/src/lib/secret-store.ts index c353f5246..04bb9b471 100644 --- a/src/lib/secret-store.ts +++ b/src/lib/secret-store.ts @@ -86,6 +86,14 @@ export type SecretStore = { readonly delete: (input: { readonly key: string }) => Promise; }; +/** An encrypted backend's key isn't available, and this read didn't create one. */ +export class SecretStoreKeyUnavailableError extends Error { + constructor(message: string) { + super(message); + this.name = "SecretStoreKeyUnavailableError"; + } +} + /** * Resolve the active secret backend for a project context. * @@ -604,7 +612,7 @@ async function resolveEncryptedFileKeyMaterial(input?: { if ( isTruthyEnvFlag(process.env[DISABLE_ENCRYPTED_FILE_KEYCHAIN_FALLBACK_ENV]) ) { - throw new Error( + throw new SecretStoreKeyUnavailableError( buildEncryptedBackendKeyRecoveryMessage({ keyPath: configuredKeyPath, storePath: input?.storePath, @@ -664,7 +672,7 @@ async function resolveEncryptedFileKeyMaterial(input?: { const nowMs = Date.now(); if (nowMs < encryptedFileKeyFailureCooldownUntilMs) { - throw new Error( + throw new SecretStoreKeyUnavailableError( buildEncryptedBackendKeyRecoveryMessage({ keyPath: configuredKeyPath, storePath: input?.storePath, @@ -704,7 +712,7 @@ async function resolveEncryptedFileKeyMaterial(input?: { } if (configuredKeyPath || !allowKeyProvisioning) { - throw new Error( + throw new SecretStoreKeyUnavailableError( buildEncryptedBackendKeyRecoveryMessage({ keyPath: configuredKeyPath, storePath: input?.storePath, @@ -755,7 +763,7 @@ function keychainKeyAccessError(input: { if (input.error instanceof NativeKeychainTimeoutError) { return input.error; } - return new Error( + return new SecretStoreKeyUnavailableError( buildEncryptedBackendKeyRecoveryMessage({ keyPath: input.keyPath, storePath: input.storePath, diff --git a/tests/doctor-command.test.ts b/tests/doctor-command.test.ts index 9a3ebe414..7c747509d 100644 --- a/tests/doctor-command.test.ts +++ b/tests/doctor-command.test.ts @@ -5,6 +5,7 @@ import { mkdtemp, readdir, rm, + symlink, writeFile, } from "node:fs/promises"; import { tmpdir } from "node:os"; @@ -169,6 +170,32 @@ test("doctor rejects mutating repair flags combined with json", () => { ).not.toThrow(); }); +for (const form of ["dangling link", "malformed"] as const) { + test(`doctor remediation plan says a ${form} contract refuses, and doesn't offer the migration`, async () => { + const root = await createDoctorTestProject({ legacy: true }); + try { + const contract = resolve(root, ".hack", "hack.env.json"); + await rm(contract); + if (form === "dangling link") { + await symlink(resolve(root, "contract-that-is-gone"), contract); + } else { + await writeFile(contract, "{not json\n"); + } + + const lines = await buildDoctorRemediationPlanLines({ + startDir: root, + migrateEnvConfig: true, + }); + + expect(lines.at(-1)).toBe( + "3. Refuse env migration (E_CONFIG_INVALID) because .hack/hack.env.json can't be read or parsed." + ); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +} + test("doctor remediation plan mentions env migration when requested for a legacy project", async () => { const root = await createDoctorTestProject({ legacy: true }); try { diff --git a/tests/env-contract-no-key-provisioning.test.ts b/tests/env-contract-no-key-provisioning.test.ts new file mode 100644 index 000000000..0c8e489b4 --- /dev/null +++ b/tests/env-contract-no-key-provisioning.test.ts @@ -0,0 +1,307 @@ +import { afterEach, expect, test } from "bun:test"; +import { + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + realpath, + rm, + stat, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, relative, resolve } from "node:path"; +import { fixtureChildren } from "./helpers/fixture-children.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +/* + * HACK-1688: the check for shell-only contract values reads a mirrored backend + * before the migration refuses, and that read must not create key material. + * These runs leave HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK unset: with it set, the + * read stops before any provisioning code, so it can't show this. The worker stubs + * Bun.secrets, so the OS keychain isn't reached. + */ + +const WORKER = resolve( + import.meta.dir, + "fixtures", + "contract-migration-keychain-worker.ts" +); +const SHELL_KEY = "NO_PROVISION_SHELL_KEY"; +const SHELL_VALUE = ["synthetic", "shell", "1688"].join("-"); +const UNAVAILABLE = "the secret backend's key isn't available"; + +const children = fixtureChildren("env-contract-no-key-provisioning"); +const roots: string[] = []; + +afterEach(async () => { + await children.stopAll(); + for (const root of roots.splice(0)) { + await rm(root, { recursive: true, force: true }); + } +}); + +async function snapshot(root: string): Promise> { + const out: Record = {}; + for (const entry of await readdir(root, { + withFileTypes: true, + recursive: true, + })) { + const path = join(entry.parentPath, entry.name); + out[relative(root, path)] = entry.isFile() + ? `${(await stat(path)).mode.toString(8)}:${(await readFile(path)).toString("base64")}` + : "not-a-file"; + } + return out; +} + +async function expectRefusedWithoutProvisioning(opts: { + readonly keychain: "empty" | "key" | "throw"; + readonly configuredKeyPath: boolean; + readonly unavailable: boolean; +}): Promise { + const root = await realpath( + await mkdtemp(join(tmpdir(), "hack-no-provision-")) + ); + roots.push(root); + const projectRoot = resolve(root, "repo"); + const projectDir = resolve(projectRoot, ".hack"); + const keyPath = resolve(projectRoot, "secrets-file.key"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + resolve(projectDir, "docker-compose.yml"), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + resolve(projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "contract-keychain", + dev_host: "contract-keychain.hack", + controlPlane: { + secrets: { + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + ...(opts.configuredKeyPath ? { keyPath } : {}), + }, + }, + }, + })}\n` + ); + await writeFile( + resolve(projectDir, "hack.env.json"), + `${JSON.stringify({ + version: 1, + vars: [{ key: SHELL_KEY, source: "plain_env", required: false }], + })}\n` + ); + const before = await snapshot(projectRoot); + + const run = await children.run({ + cmd: [process.execPath, WORKER], + cwd: projectRoot, + env: privateCliEnv({ + root, + env: { + [SHELL_KEY]: SHELL_VALUE, + HACK_GLOBAL_CONFIG_PATH: join(root, "hack-config.json"), + HACK_HOME: join(root, "hack-home"), + HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: undefined, + HACK_SECRETS_FILE_KEY: undefined, + HACK_TEST_KEYCHAIN: opts.keychain, + HACK_TEST_PROJECT_ROOT: projectRoot, + }, + }), + }); + + expect(run.stderr).toBe(""); + expect(run.exitCode).toBe(0); + const result = JSON.parse(run.stdout) as { + readonly message: string | null; + readonly calls: { + readonly get: number; + readonly set: number; + readonly delete: number; + }; + }; + expect(result.message).toContain(SHELL_KEY); + expect(result.message).not.toContain(SHELL_VALUE); + expect(result.message?.includes(UNAVAILABLE)).toBe(opts.unavailable); + expect(await snapshot(projectRoot)).toEqual(before); + expect(await lstat(keyPath).catch(() => null)).toBeNull(); + // Anywhere under the fixture, which holds the private HOME and the default key path. + const everything = await readdir(root, { recursive: true }); + expect( + everything.filter((path) => path.endsWith("secrets-file.key")) + ).toEqual([]); + // The keychain branch ran, so this input reached the code past the disable flag. + expect(result.calls.get).toBeGreaterThan(0); + expect(result.calls.set).toBe(0); + expect(result.calls.delete).toBe(0); +} + +test("a configured key path and an empty keychain create no key before the refusal", async () => { + await expectRefusedWithoutProvisioning({ + keychain: "empty", + configuredKeyPath: true, + unavailable: true, + }); +}, 45_000); + +test("a key in the keychain isn't copied to a configured key path before the refusal", async () => { + await expectRefusedWithoutProvisioning({ + keychain: "key", + configuredKeyPath: true, + unavailable: false, + }); +}, 45_000); + +test("the default key path and an empty keychain create no key before the refusal", async () => { + await expectRefusedWithoutProvisioning({ + keychain: "empty", + configuredKeyPath: false, + unavailable: true, + }); +}, 45_000); + +test("a failed keychain read refuses as unavailable and creates no key", async () => { + await expectRefusedWithoutProvisioning({ + keychain: "throw", + configuredKeyPath: true, + unavailable: true, + }); +}, 45_000); + +/** One worker run with no shell value, or a stored backend, or the keychain backend. */ +async function runWorker(opts: { + readonly keychain: "empty" | "key" | "throw"; + readonly shell: boolean; + readonly storePresent: boolean; + readonly backend?: "encrypted_file" | "keychain"; +}) { + const root = await realpath( + await mkdtemp(join(tmpdir(), "hack-no-provision-")) + ); + roots.push(root); + const projectRoot = resolve(root, "repo"); + const projectDir = resolve(projectRoot, ".hack"); + const keyPath = resolve(projectRoot, "secrets-file.key"); + const storePath = resolve(projectRoot, "secrets.enc.json"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + resolve(projectDir, "docker-compose.yml"), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + resolve(projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "contract-keychain", + dev_host: "contract-keychain.hack", + controlPlane: { + secrets: + opts.backend === "keychain" + ? { backend: "keychain", storePlaintextInBackend: true } + : { + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { path: storePath, keyPath }, + }, + }, + })}\n` + ); + await writeFile( + resolve(projectDir, "hack.env.json"), + `${JSON.stringify({ + version: 1, + vars: [{ key: SHELL_KEY, source: "plain_env", required: false }], + })}\n` + ); + if (opts.storePresent) { + await writeFile(storePath, '{"version":1,"entries":{"x":"synthetic"}}\n'); + } + const before = await snapshot(projectRoot); + const run = await children.run({ + cmd: [process.execPath, WORKER], + cwd: projectRoot, + env: privateCliEnv({ + root, + env: { + ...(opts.shell ? { [SHELL_KEY]: SHELL_VALUE } : {}), + HACK_GLOBAL_CONFIG_PATH: join(root, "hack-config.json"), + HACK_HOME: join(root, "hack-home"), + HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: undefined, + HACK_SECRETS_FILE_KEY: undefined, + HACK_TEST_KEYCHAIN: opts.keychain, + HACK_TEST_PROJECT_ROOT: projectRoot, + }, + }), + }); + expect(run.stderr).toBe(""); + expect(run.exitCode).toBe(0); + return { + result: JSON.parse(run.stdout) as { + readonly message: string | null; + readonly calls: { + readonly get: number; + readonly set: number; + readonly delete: number; + }; + }, + before, + after: await snapshot(projectRoot), + keyFile: await lstat(keyPath).catch(() => null), + }; +} + +test("with nothing stored, the printed step 1 migrates", async () => { + const { result, after, keyFile } = await runWorker({ + keychain: "empty", + shell: false, + storePresent: false, + }); + expect(result.message).toBeNull(); + // The docs say the migration creates the key when nothing is stored yet. + expect(keyFile).not.toBeNull(); + expect(Object.keys(after)).toContain(".hack/hack.env.default.yaml"); +}, 45_000); + +test("a store with no key refuses a shell value, and leaves the store and key path alone", async () => { + const { result, before, after, keyFile } = await runWorker({ + keychain: "empty", + shell: true, + storePresent: true, + }); + expect(result.message).toContain(UNAVAILABLE); + expect(result.message).not.toContain(SHELL_VALUE); + expect(after).toEqual(before); + expect(keyFile).toBeNull(); + expect(result.calls.set).toBe(0); +}, 45_000); + +test("a store with no key stops step 1 too, with no layer and no key", async () => { + const { result, before, after, keyFile } = await runWorker({ + keychain: "empty", + shell: false, + storePresent: true, + }); + expect(result.message).toContain("Missing encrypted backend key"); + expect(after).toEqual(before); + expect(keyFile).toBeNull(); + expect(result.calls.get).toBeGreaterThan(0); + expect(result.calls.set).toBe(0); +}, 45_000); + +test("a keychain backend read failure stops the migration with its own error", async () => { + const { result, before, after } = await runWorker({ + keychain: "throw", + shell: true, + storePresent: false, + backend: "keychain", + }); + expect(result.message).toContain("stub keychain failure"); + expect(result.message).not.toContain(SHELL_VALUE); + expect(after).toEqual(before); + expect(result.calls.set).toBe(0); +}, 45_000); diff --git a/tests/env-contract-shell-values.test.ts b/tests/env-contract-shell-values.test.ts new file mode 100644 index 000000000..b097d28c9 --- /dev/null +++ b/tests/env-contract-shell-values.test.ts @@ -0,0 +1,1044 @@ +import { afterEach, expect, test } from "bun:test"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + rename, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, relative, resolve } from "node:path"; + +import { findProcessOnlyContractValues } from "../src/lib/hack-env.ts"; +import { + LegacyDotenvMigrationRefusalError, + migrateLegacyProjectEnv, + resolveProjectEnvConfig, + selectProjectEnvValues, +} from "../src/lib/project-env-config.ts"; +import { resolveSecretStore } from "../src/lib/secret-store.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +const SHELL_KEY = "HACK_1688_SYNTHETIC_TOKEN"; +const SHELL_VALUE = ["synthetic", "shell", "1688"].join("-"); +const DOTENV_VALUE = ["synthetic", "dotenv", "1688"].join("-"); +const OTHER_SHELL_KEY = "HACK_1688_OTHER_SYNTHETIC_TOKEN"; +const ENTRYPOINT = resolve(import.meta.dir, "..", "index.ts"); +const tempDirs = new Set(); + +afterEach(async () => { + delete process.env[SHELL_KEY]; + delete process.env[OTHER_SHELL_KEY]; + Reflect.deleteProperty(process.env, "HACK_SECRETS_FILE_KEY"); + Reflect.deleteProperty(process.env, "HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK"); + for (const dir of tempDirs) { + await rm(dir, { recursive: true, force: true }); + } + tempDirs.clear(); +}); + +type Repo = { + readonly root: string; + readonly projectRoot: string; + readonly projectDir: string; +}; + +async function createContractRepo(opts: { + readonly services?: readonly string[]; + readonly files?: Readonly>; + readonly otherVars?: readonly Record[]; + readonly secrets?: (projectRoot: string) => Record; +}): Promise { + const root = await mkdtemp(join(tmpdir(), "hack-env-contract-shell-")); + tempDirs.add(root); + const projectRoot = resolve(root, "repo"); + const projectDir = resolve(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + resolve(projectDir, "docker-compose.yml"), + "services:\n api:\n image: alpine:3.20\n worker:\n image: alpine:3.20\n" + ); + await writeFile( + resolve(projectDir, "hack.config.json"), + `${JSON.stringify({ + name: "contract-shell", + dev_host: "contract-shell.hack", + ...(opts.secrets + ? { controlPlane: { secrets: opts.secrets(projectRoot) } } + : {}), + })}\n` + ); + await writeFile( + resolve(projectDir, "hack.env.json"), + `${JSON.stringify({ + version: 1, + vars: [ + { + key: SHELL_KEY, + source: "plain_env", + required: false, + ...(opts.services ? { services: opts.services } : {}), + }, + ...(opts.otherVars ?? []), + ], + })}\n` + ); + for (const [name, text] of Object.entries(opts.files ?? {})) { + await writeFile(resolve(projectDir, name), text); + } + return { root, projectRoot, projectDir }; +} + +function migrate(repo: Repo) { + return migrateLegacyProjectEnv({ + projectRoot: repo.projectRoot, + projectDir: repo.projectDir, + projectName: "contract-shell", + serviceNames: ["api", "worker"], + materialize: false, + }); +} + +async function layerTexts(repo: Repo): Promise { + const names = (await readdir(repo.projectDir)).filter( + (name) => name.startsWith("hack.env.") && name.endsWith(".yaml") + ); + const texts = await Promise.all( + names.map((name) => readFile(resolve(repo.projectDir, name), "utf8")) + ); + return texts.join("\n"); +} + +/** Every file under the fixture root, with its bytes and mode, so a partial write can't pass. */ +async function snapshotTree( + repo: Repo +): Promise>> { + const entries = await readdir(repo.projectRoot, { + withFileTypes: true, + recursive: true, + }); + const snapshot: Record = {}; + for (const entry of entries) { + const path = join(entry.parentPath, entry.name); + const label = relative(repo.projectRoot, path); + snapshot[label] = entry.isFile() + ? `${(await stat(path)).mode.toString(8)}:${(await readFile(path)).toString("base64")}` + : "not-a-file"; + } + return snapshot; +} + +async function expectShellValueRefused(repo: Repo): Promise { + const before = await snapshotTree(repo); + const error = await migrate(repo).catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(LegacyDotenvMigrationRefusalError); + const { message } = error as Error; + expect(message).toContain(SHELL_KEY); + expect(message).not.toContain(SHELL_VALUE); + expect(await snapshotTree(repo)).toEqual(before); + return message; +} + +async function runHack(opts: { + readonly repo: Repo; + readonly args: readonly string[]; + readonly shellValue?: string; + readonly stdin?: string; + readonly env?: Readonly>; +}) { + const proc = Bun.spawn([process.execPath, ENTRYPOINT, ...opts.args], { + cwd: opts.repo.projectRoot, + env: privateCliEnv({ + root: opts.repo.root, + env: { + HACK_GLOBAL_CONFIG_PATH: join(opts.repo.root, "hack-config.json"), + HACK_HOME: join(opts.repo.root, "hack-home"), + HACK_DAEMON_AUTOSTART_DISABLE: "1", + HACK_NO_INTERACTIVE: "1", + HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: "1", + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: "/dev/null", + ...(opts.shellValue === undefined + ? {} + : { [SHELL_KEY]: opts.shellValue }), + ...opts.env, + }, + }), + stdin: opts.stdin === undefined ? "ignore" : new Blob([opts.stdin]), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + return { exitCode, output: `${stdout}${stderr}` }; +} + +async function valueFor(repo: Repo, service: string, envName?: string) { + const resolved = await resolveProjectEnvConfig({ + projectRoot: repo.projectRoot, + projectDir: repo.projectDir, + ...(envName ? { envName } : {}), + serviceNames: ["api", "worker"], + }); + if (!resolved) { + throw new Error("expected env layers"); + } + return selectProjectEnvValues({ resolved, scopeName: service })[SHELL_KEY]; +} + +test("a shell-only plain_env value never reaches a layer, and the migration refuses", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + await expectShellValueRefused(await createContractRepo({})); +}); + +test("a service-scoped shell-only plain_env value is refused the same way", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ services: ["api"] }) + ); + expect(message).toContain(`hack env set ${SHELL_KEY} --service api --local`); + expect(message).not.toContain(`hack env set ${SHELL_KEY} --local`); +}); + +test("a key set only in an overlay file is refused too, because the base still takes it from the shell", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ + files: { ".env.staging": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }) + ); + expect(message).toContain( + `hack env set ${SHELL_KEY} --env staging --local --value-stdin` + ); + expect(message).not.toContain(DOTENV_VALUE); +}); + +test("an overlay that leaves the key to the shell gets no overlay command", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ files: { ".env.staging": "OTHER=1\n" } }) + ); + expect(message).not.toContain("--env staging"); +}); + +for (const services of [undefined, ["api"]] as const) { + const scope = services ? "a service-scoped key" : "an unscoped key"; + test(`the printed recovery keeps an overlay's own value for ${scope}`, async () => { + const repo = await createContractRepo({ + ...(services ? { services } : {}), + files: { ".env.staging": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const serviceFlags = services ? ["--service", "api"] : []; + const set = (input: { + readonly flags: readonly string[]; + readonly value: string; + }) => + runHack({ + repo, + args: [ + "env", + "set", + SHELL_KEY, + ...input.flags, + ...serviceFlags, + "--local", + "--value-stdin", + "--path", + repo.projectRoot, + ], + stdin: `${input.value}\n`, + }); + + const refused = await runHack({ + repo, + args: ["env", "exec", "--path", repo.projectRoot, "--", "true"], + shellValue: SHELL_VALUE, + }); + expect(refused.exitCode).not.toBe(0); + const migrated = await runHack({ + repo, + args: ["env", "exec", "--path", repo.projectRoot, "--", "true"], + }); + expect(migrated.exitCode).toBe(0); + expect((await set({ flags: [], value: SHELL_VALUE })).exitCode).toBe(0); + // The migration moved staging's own value to its machine-local layer, which + // outranks the local base above, so setting the base doesn't replace it. + expect(await valueFor(repo, "api", "staging")).toBe(DOTENV_VALUE); + expect( + (await set({ flags: ["--env", "staging"], value: DOTENV_VALUE })).exitCode + ).toBe(0); + + expect(await valueFor(repo, "api")).toBe(SHELL_VALUE); + expect(await valueFor(repo, "api", "staging")).toBe(DOTENV_VALUE); + if (services) { + expect(await valueFor(repo, "worker", "staging")).toBeUndefined(); + } + }, 60_000); +} + +test("a service name that isn't safe to paste is reported, and gets no command", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ services: ["a b$(touch pwned)"] }) + ); + expect(message).not.toContain("--service 'a b$(touch pwned)'"); + expect(message).not.toContain("--service a b"); + expect(message).toContain(JSON.stringify("a b$(touch pwned)")); +}); + +test("the printed unset names every key it refuses", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env[OTHER_SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ + otherVars: [ + { key: OTHER_SHELL_KEY, source: "plain_env", required: false }, + ], + }) + ); + expect(message).toContain( + `env -u ${OTHER_SHELL_KEY} -u ${SHELL_KEY} hack env exec -- true` + ); +}); + +test("a keychain var doesn't set up a store or write a key before the refusal", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + const repo = await createContractRepo({ + otherVars: [{ key: "SERVICE_TOKEN", source: "keychain", required: false }], + secrets: (projectRoot) => ({ + backend: "encrypted_file", + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + await expectShellValueRefused(repo); + expect( + await lstat(resolve(repo.projectRoot, "secrets-file.key")).catch(() => null) + ).toBeNull(); +}); + +test("a mirrored backend whose read works doesn't write a key file before the refusal", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + process.env.HACK_SECRETS_FILE_KEY = ["synthetic", "file", "1688"].join("-"); + const repo = await createContractRepo({ + secrets: (projectRoot) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + await expectShellValueRefused(repo); + expect( + await lstat(resolve(repo.projectRoot, "secrets-file.key")).catch(() => null) + ).toBeNull(); +}); + +test("a mirrored backend whose key isn't available refuses a shell key, and creates no key", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + const repo = await createContractRepo({ + secrets: (projectRoot) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + const message = await expectShellValueRefused(repo); + expect(message).toContain("the secret backend's key isn't available"); + expect( + await lstat(resolve(repo.projectRoot, "secrets-file.key")).catch(() => null) + ).toBeNull(); +}); + +test("an undetermined key still gets the overlay command, so recovery can't replace that overlay's value", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + const repo = await createContractRepo({ + files: { ".env.staging": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + secrets: (projectRoot) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + const message = await expectShellValueRefused(repo); + expect(message).toContain("the secret backend's key isn't available"); + expect(message).toContain( + `hack env set ${SHELL_KEY} --env staging --local --value-stdin` + ); + expect(message).not.toContain(DOTENV_VALUE); +}); + +test("a corrupt mirrored store stops the migration with its own error, not a shell-value refusal", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_FILE_KEY = ["synthetic", "file", "1688"].join("-"); + const repo = await createContractRepo({ + secrets: (projectRoot) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + await writeFile(resolve(repo.projectRoot, "secrets.enc.json"), "{not json\n"); + const before = await snapshotTree(repo); + + const error = await migrate(repo).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(Error); + expect(error).not.toBeInstanceOf(LegacyDotenvMigrationRefusalError); + expect((error as Error).message).toContain( + "Invalid encrypted secret store JSON" + ); + expect((error as Error).message).not.toContain(SHELL_VALUE); + expect(await snapshotTree(repo)).toEqual(before); +}); + +test("with the key unset in the shell, no shell-only value is found", async () => { + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + const repo = await createContractRepo({ + secrets: (projectRoot) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + expect( + await findProcessOnlyContractValues({ + projectDir: repo.projectDir, + projectName: "contract-shell", + overlayNames: [], + }) + ).toEqual([]); +}); + +test("a dotenv value leaves no shell-only value", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + const repo = await createContractRepo({ + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + secrets: (projectRoot) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + expect( + await findProcessOnlyContractValues({ + projectDir: repo.projectDir, + projectName: "contract-shell", + overlayNames: [], + }) + ).toEqual([]); +}); + +test("an encrypted value still migrates beside a dotenv value, and a shell value stays out", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + process.env.HACK_SECRETS_FILE_KEY = ["synthetic", "file", "1688"].join("-"); + const secret = ["synthetic", "encrypted", "1688"].join("-"); + const repo = await createContractRepo({ + otherVars: [ + { + key: "SERVICE_TOKEN", + source: "keychain", + required: false, + services: ["api"], + }, + ], + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + secrets: (projectRoot) => ({ + backend: "encrypted_file", + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, + }), + }); + const store = await resolveSecretStore({ + projectName: "contract-shell", + projectDir: repo.projectDir, + }); + await store.set({ key: "SERVICE_TOKEN", value: secret }); + + await migrate(repo); + + const layers = await layerTexts(repo); + expect(layers).toContain(DOTENV_VALUE); + expect(layers).not.toContain(SHELL_VALUE); + expect(layers).not.toContain(secret); + const resolved = await resolveProjectEnvConfig({ + projectRoot: repo.projectRoot, + projectDir: repo.projectDir, + serviceNames: ["api", "worker"], + }); + expect(resolved?.serviceEnv.api?.SERVICE_TOKEN).toBe(secret); +}); + +test("a contract mixing a dotenv key and a shell key writes nothing at all", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + await expectShellValueRefused( + await createContractRepo({ + otherVars: [{ key: "FROM_DOTENV", source: "plain_env", required: false }], + files: { ".env": `FROM_DOTENV=${DOTENV_VALUE}\n` }, + }) + ); +}); + +test("an overlay with no shell-origin key adds no refusal: the key in the base file wins over the shell", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const repo = await createContractRepo({ + files: { + ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n`, + ".env.staging": "OTHER=1\n", + }, + }); + await migrate(repo); + expect(await layerTexts(repo)).not.toContain(SHELL_VALUE); +}); + +test("implicit env exec refuses with the shell value set, then the printed recovery keeps the contract's scope", async () => { + const repo = await createContractRepo({ services: ["api"] }); + const before = await snapshotTree(repo); + + const refused = await runHack({ + repo, + args: ["env", "exec", "--path", repo.projectRoot, "--", "true"], + shellValue: SHELL_VALUE, + }); + expect(refused.exitCode).not.toBe(0); + expect(refused.output).toContain("E_CONFIG_INVALID"); + expect(refused.output).toContain(SHELL_KEY); + expect(refused.output).not.toContain(SHELL_VALUE); + expect(await snapshotTree(repo)).toEqual(before); + + // Step 1: one command without the variable lets the migration run. + const migrated = await runHack({ + repo, + args: ["env", "exec", "--path", repo.projectRoot, "--", "true"], + }); + expect(migrated.exitCode).toBe(0); + expect(await layerTexts(repo)).not.toContain(SHELL_VALUE); + expect(await valueFor(repo, "api")).toBeUndefined(); + + // Step 2: the scoped command delivers it to the declared service only. + const set = await runHack({ + repo, + args: [ + "env", + "set", + SHELL_KEY, + "--service", + "api", + "--local", + "--value-stdin", + "--path", + repo.projectRoot, + ], + stdin: `${DOTENV_VALUE}\n`, + }); + expect(set.exitCode).toBe(0); + expect(set.output).not.toContain(DOTENV_VALUE); + expect(await valueFor(repo, "api")).toBe(DOTENV_VALUE); + expect(await valueFor(repo, "worker")).toBeUndefined(); + + // A shell value still reaches a host command, and no longer blocks anything. + const hostValue = await runHack({ + repo, + args: [ + "env", + "exec", + "--path", + repo.projectRoot, + "--", + "/usr/bin/printenv", + SHELL_KEY, + ], + shellValue: SHELL_VALUE, + }); + expect(hostValue.exitCode).toBe(0); + expect(hostValue.output).toContain(SHELL_VALUE); + expect(await layerTexts(repo)).not.toContain(SHELL_VALUE); +}, 60_000); + +test("a dotenv value still migrates as documented, and a same-named shell value stays out", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const repo = await createContractRepo({ + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const migrated = await migrate(repo); + expect(migrated.legacyDetected).toBe(true); + const layers = await layerTexts(repo); + expect(layers).toContain(DOTENV_VALUE); + expect(layers).not.toContain(SHELL_VALUE); +}); + +test("with the key unset in the shell, the migration proceeds and writes no value for it", async () => { + const repo = await createContractRepo({}); + const migrated = await migrate(repo); + expect(migrated.legacyDetected).toBe(true); + expect(await layerTexts(repo)).not.toContain(SHELL_KEY); + expect( + await lstat(resolve(repo.projectDir, "hack.env.default.yaml")).catch( + () => null + ) + ).not.toBeNull(); +}); + +const MIRRORED = (projectRoot: string) => ({ + backend: "encrypted_file", + storePlaintextInBackend: true, + encryptedFile: { + path: resolve(projectRoot, "secrets.enc.json"), + keyPath: resolve(projectRoot, "secrets-file.key"), + }, +}); + +for (const services of [undefined, ["api"]] as const) { + const scope = services ? "a service-scoped key" : "an unscoped key"; + test(`an undetermined ${scope} recovers through the printed steps once the key is available`, async () => { + const repo = await createContractRepo({ + ...(services ? { services } : {}), + files: { ".env.staging": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + secrets: MIRRORED, + }); + const before = await snapshotTree(repo); + const exec = ["env", "exec", "--path", repo.projectRoot, "--", "true"]; + + const refused = await runHack({ + repo, + args: exec, + shellValue: SHELL_VALUE, + }); + expect(refused.exitCode).not.toBe(0); + expect(refused.output).toContain( + "the secret backend's key isn't available" + ); + expect(refused.output).not.toContain(SHELL_VALUE); + expect(await snapshotTree(repo)).toEqual(before); + + // Step 1 with the key still missing stops, and writes no layer. + const stillMissing = await runHack({ repo, args: exec }); + expect(stillMissing.exitCode).not.toBe(0); + expect(await snapshotTree(repo)).toEqual(before); + + const withKey = { + HACK_SECRETS_FILE_KEY: ["synthetic", "file", "1688"].join("-"), + }; + const serviceFlags = services ? ["--service", "api"] : []; + const set = (flags: readonly string[], value: string) => + runHack({ + repo, + args: [ + "env", + "set", + SHELL_KEY, + ...flags, + ...serviceFlags, + "--local", + "--value-stdin", + "--path", + repo.projectRoot, + ], + stdin: `${value}\n`, + env: withKey, + }); + expect((await runHack({ repo, args: exec, env: withKey })).exitCode).toBe( + 0 + ); + expect((await set([], SHELL_VALUE)).exitCode).toBe(0); + expect((await set(["--env", "staging"], DOTENV_VALUE)).exitCode).toBe(0); + + expect(await valueFor(repo, "api")).toBe(SHELL_VALUE); + expect(await valueFor(repo, "api", "staging")).toBe(DOTENV_VALUE); + expect(await layerTextsTrackedOnly(repo)).not.toContain(SHELL_VALUE); + }, 90_000); +} + +/** Text of the tracked layers: the machine-local ones are meant to hold the value. */ +async function layerTextsTrackedOnly(repo: Repo): Promise { + const names = (await readdir(repo.projectDir)).filter( + (name) => + name.startsWith("hack.env.") && + name.endsWith(".yaml") && + !name.endsWith(".local.yaml") && + name !== "hack.env.local.yaml" + ); + const texts = await Promise.all( + names.map((name) => readFile(resolve(repo.projectDir, name), "utf8")) + ); + return texts.join("\n"); +} + +for (const [file, flag] of [ + [".env.My_Staging", "--env"], + [".env.local", "--env"], + [".env.foo.bar", "--env"], + [".env.default", "--env"], + [".env.base", "--env"], +] as const) { + test(`an overlay ${file} gets no command that --env can't select`, async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const repo = await createContractRepo({ + files: { [file]: `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const message = await expectShellValueRefused(repo); + expect(message).not.toContain(`${flag} ${file.slice(".env.".length)}`); + expect(message).toContain(JSON.stringify(file)); + expect(message).not.toContain(DOTENV_VALUE); + }); +} + +for (const service of ["Api", "a b"]) { + test(`a contract service ${JSON.stringify(service)} gets no command --service would refuse`, async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const repo = await createContractRepo({ services: [service] }); + const message = await expectShellValueRefused(repo); + expect(message).not.toContain(`hack env set ${SHELL_KEY} --service`); + expect(message).toContain(JSON.stringify(service)); + }); +} + +test("every command the refusal prints is accepted, for the names --env and --service take", async () => { + const repo = await createContractRepo({ + services: ["api"], + files: { ".env.staging": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const exec = ["env", "exec", "--path", repo.projectRoot, "--", "true"]; + const refused = await runHack({ repo, args: exec, shellValue: SHELL_VALUE }); + expect(refused.exitCode).not.toBe(0); + const printed = [ + ...refused.output.matchAll(/hack env set [^;.`\n]+--local --value-stdin/g), + ].map((match) => match[0]); + expect(printed).toHaveLength(2); + expect((await runHack({ repo, args: exec })).exitCode).toBe(0); + for (const command of printed) { + const words = command.split(" ").slice(1); + const run = await runHack({ + repo, + args: [...words, "--path", repo.projectRoot], + stdin: `${SHELL_VALUE}\n`, + }); + expect(run.exitCode).toBe(0); + } +}, 60_000); + +const RESERVED_NAME = "reserved for Hack's own layers"; + +for (const file of [ + ".env.default", + ".env.Default", + ".env.local", + ".env.LOCAL", + ".env.staging.local", +]) { + test(`step 1 refuses an overlay ${file} before any write, and a retry after renaming migrates it`, async () => { + const repo = await createContractRepo({ + files: { + ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n`, + [file]: `${SHELL_KEY}=overlay-value\n`, + ".env.staging": `${SHELL_KEY}=staging-value\n`, + }, + }); + const before = await snapshotTree(repo); + + const error = await migrate(repo).catch((caught: unknown) => caught); + + expect((error as Error).message).toContain(RESERVED_NAME); + expect((error as Error).message).toContain("No env layer was written"); + expect((error as Error).message).not.toContain("Nothing was written"); + expect((error as Error).message).toContain(JSON.stringify(file)); + expect((error as Error).message).not.toContain("overlay-value"); + expect(await snapshotTree(repo)).toEqual(before); + + await rename( + resolve(repo.projectDir, file), + resolve(repo.projectDir, ".env.renamed") + ); + await migrate(repo); + expect(await valueFor(repo, "api")).toBe(DOTENV_VALUE); + expect(await valueFor(repo, "api", "staging")).toBe("staging-value"); + expect(await valueFor(repo, "api", "renamed")).toBe("overlay-value"); + }); +} + +test("an overlay .env.default refuses even when it adds nothing to the base", async () => { + const repo = await createContractRepo({ + files: { + ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n`, + ".env.default": `${SHELL_KEY}=${DOTENV_VALUE}\n`, + }, + }); + const before = await snapshotTree(repo); + + const error = await migrate(repo).catch((caught: unknown) => caught); + + expect((error as Error).message).toContain(".env.default would overwrite"); + expect((error as Error).message).not.toContain(DOTENV_VALUE); + expect(await snapshotTree(repo)).toEqual(before); +}); + +test("step 1 refuses a contract scope a layer can't hold, before any write", async () => { + const repo = await createContractRepo({ + services: ["Api"], + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const before = await snapshotTree(repo); + + const error = await migrate(repo).catch((caught: unknown) => caught); + + expect((error as Error).message).toContain(JSON.stringify("Api")); + expect((error as Error).message).toContain("isn't a valid env scope"); + expect((error as Error).message).toContain("No env layer was written"); + expect((error as Error).message).not.toContain(DOTENV_VALUE); + expect(await snapshotTree(repo)).toEqual(before); +}); + +test("a contract scope a layer can't hold doesn't stop a migration with no value for it", async () => { + const repo = await createContractRepo({ services: ["Api"] }); + const migrated = await migrate(repo); + expect(migrated.legacyDetected).toBe(true); + expect(await valueFor(repo, "api")).toBeUndefined(); +}); + +test("a service list with usable and unusable names prints a command for the usable one", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ services: ["api", "Api"] }) + ); + expect(message).toContain(`hack env set ${SHELL_KEY} --service api --local`); + expect(message).not.toContain(`hack env set ${SHELL_KEY} --service Api`); + expect(message).toContain(JSON.stringify("Api")); +}); + +test("names to fix come before the step that migrates, because a migration can't be redone", async () => { + process.env[SHELL_KEY] = SHELL_VALUE; + const message = await expectShellValueRefused( + await createContractRepo({ + services: ["Api"], + files: { ".env.My_Staging": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }) + ); + const unsetAt = message.indexOf("Unset it for one command"); + expect(unsetAt).toBeGreaterThan(-1); + for (const name of [".env.My_Staging", "Api"]) { + const at = message.indexOf(JSON.stringify(name)); + expect(at).toBeGreaterThan(-1); + expect(at).toBeLessThan(unsetAt); + } +}); + +for (const [shape, text] of [ + ["malformed", "{not json\n"], + ["wrong-shape", '{"version":99}\n'], +] as const) { + test(`a ${shape} hack.env.json refuses before any write, without quoting the file`, async () => { + const repo = await createContractRepo({ + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\nOTHER=only-here\n` }, + }); + await writeFile(resolve(repo.projectDir, "hack.env.json"), text); + const before = await snapshotTree(repo); + + const error = await migrate(repo).catch((caught: unknown) => caught); + + expect((error as Error).message).toContain("hack.env.json can't be read"); + expect((error as Error).message).not.toContain("not json"); + expect((error as Error).message).not.toContain("only-here"); + expect(await snapshotTree(repo)).toEqual(before); + }); +} + +for (const kind of ["mode 000", "directory", "dangling link"] as const) { + test.skipIf(kind === "mode 000" && process.geteuid?.() === 0)( + `a hack.env.json that can't be opened (${kind}) refuses, and isn't read as no contract`, + async () => { + const repo = await createContractRepo({ + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const contract = resolve(repo.projectDir, "hack.env.json"); + if (kind === "mode 000") { + await chmod(contract, 0o000); + } else { + await rm(contract); + if (kind === "directory") { + await mkdir(contract); + } else { + await symlink(resolve(repo.root, "contract-that-is-gone"), contract); + } + } + const names = async () => (await readdir(repo.projectDir)).sort(); + const before = await names(); + + const error = await migrateLegacyProjectEnv({ + projectRoot: repo.projectRoot, + projectDir: repo.projectDir, + projectName: "contract-shell", + serviceNames: ["api", "worker"], + materialize: false, + migrateUndeclaredDotenv: true, + }) + .catch((caught: unknown) => caught) + .finally(async () => { + if (kind === "mode 000") { + await chmod(contract, 0o600); + } + }); + + expect((error as Error).message).toContain("hack.env.json can't be read"); + expect((error as Error).message).not.toContain(DOTENV_VALUE); + expect(await names()).toEqual(before); + } + ); +} + +test("env set says a contract it can't open can't be read, not that it's malformed", async () => { + const repo = await createContractRepo({ files: {} }); + const contract = resolve(repo.projectDir, "hack.env.json"); + await rm(contract); + await symlink(resolve(repo.root, "contract-that-is-gone"), contract); + + const run = await runHack({ + repo, + args: [ + "env", + "set", + "NEW_SETTING", + "--value-stdin", + "--path", + repo.projectRoot, + ], + stdin: "new-value\n", + }); + + expect(run.exitCode).not.toBe(0); + expect(run.output).toContain("hack.env.json can't be read"); + expect(run.output).not.toContain("is malformed"); + expect(run.output).not.toContain("new-value"); +}, 60_000); + +test("doctor refuses a contract it can't open, without offering the migration first", async () => { + const repo = await createContractRepo({ + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const contract = resolve(repo.projectDir, "hack.env.json"); + await rm(contract); + await symlink(resolve(repo.root, "contract-that-is-gone"), contract); + const tools = resolve(repo.root, "doctor-tools"); + await mkdir(tools); + await writeFile(join(tools, "hack"), "#!/bin/sh\nexit 0\n", { mode: 0o700 }); + await writeFile(join(tools, "docker"), "#!/bin/sh\nexit 1\n", { + mode: 0o700, + }); + await writeFile(join(tools, "mutagen"), "#!/bin/sh\nexit 0\n", { + mode: 0o700, + }); + + const run = await runHack({ + repo, + args: [ + "doctor", + "--migrate-env-config", + "--no-interactive", + "--path", + repo.projectRoot, + ], + env: { + HACK_LOGGER: "console", + HACK_MUTAGEN_PATH: join(tools, "mutagen"), + PATH: `${tools}:/usr/bin:/bin`, + }, + }); + + expect(run.output).toContain("hack.env.json can't be read"); + // The env mode check also says "can't be read", so this proves the migration ran. + expect(run.output).toContain("Refusing to migrate"); + expect(run.output).toContain("can't be read or parsed"); + expect(run.output).not.toContain( + "and no .hack/hack.env.default.yaml present" + ); + expect(run.output).not.toContain("Legacy env format detected"); + expect(run.output).not.toContain("Detected legacy project env files"); + const entries = await readdir(repo.projectDir); + expect(entries).toContain("hack.env.json"); + expect(entries).not.toContain("hack.env.default.yaml"); + expect(await readFile(resolve(repo.projectDir, ".env"), "utf8")).toContain( + DOTENV_VALUE + ); + expect(run.output).not.toContain(DOTENV_VALUE); +}, 90_000); + +for (const kind of ["mode 000", "directory", "dangling link"] as const) { + test.skipIf(kind === "mode 000" && process.geteuid?.() === 0)( + `an implicit migration refuses a hack.env.json that can't be opened (${kind}), before any layer`, + async () => { + const repo = await createContractRepo({ + files: { ".env": `${SHELL_KEY}=${DOTENV_VALUE}\n` }, + }); + const contract = resolve(repo.projectDir, "hack.env.json"); + if (kind === "mode 000") { + await chmod(contract, 0o000); + } else { + await rm(contract); + if (kind === "directory") { + await mkdir(contract); + } else { + await symlink(resolve(repo.root, "contract-that-is-gone"), contract); + } + } + const before = (await readdir(repo.projectDir)).sort(); + + const run = await runHack({ + repo, + args: [ + "env", + "add", + "NEW_SETTING", + "--local", + "--value-stdin", + "--path", + repo.projectRoot, + ], + stdin: "new-value\n", + }); + if (kind === "mode 000") { + await chmod(contract, 0o600); + } + + expect(run.exitCode).not.toBe(0); + expect(run.output).toContain("hack.env.json can't be read"); + expect(run.output).not.toContain(DOTENV_VALUE); + expect(run.output).not.toContain("new-value"); + expect((await readdir(repo.projectDir)).sort()).toEqual(before); + }, + 60_000 + ); +} diff --git a/tests/env-legacy-service-scope.test.ts b/tests/env-legacy-service-scope.test.ts index 09a85ed41..b04adebc9 100644 --- a/tests/env-legacy-service-scope.test.ts +++ b/tests/env-legacy-service-scope.test.ts @@ -4357,7 +4357,7 @@ test("outside git, setting that local layer's environment to default, as the ref expect(resolved?.hostTargetEnv.global?.KEPT_SETTING).toBe(KEPT_VALUE); }); -test("in git, a tracked local layer still in the working tree gets the tracked-layer refusal and its remedy", async () => { +test("in git, a tracked local layer still in the working tree gets the tracked-layer refusal", async () => { const repo = await createLegacyRepo(); await git({ repo, args: ["init", "-q"] }); const localPath = resolve(repo.projectDir, "hack.env.local.yaml"); diff --git a/tests/env-set-local-contract.test.ts b/tests/env-set-local-contract.test.ts index 26a5905ba..62f5de65e 100644 --- a/tests/env-set-local-contract.test.ts +++ b/tests/env-set-local-contract.test.ts @@ -406,6 +406,28 @@ for (const lateContract of [false, true]) { ); } +test( + "doctor refuses a local overlay file even when it adds no value of its own", + async () => { + const project = await createProject({ contract: true }); + await git(project, ["init", "--quiet"]); + const text = `LOCAL_KEY=${VALUE}\n`; + await writeFile(join(project.projectDir, ".env"), text); + await writeFile(join(project.projectDir, ".env.local"), text); + const run = await runDoctor(project); + expect(run.exitCode).toBe(1); + expect(run.output).not.toContain(VALUE); + expect(run.output).toContain(".env.local would collide"); + expect(await readFile(join(project.projectDir, ".env.local"), "utf8")).toBe( + text + ); + await expect( + readFile(join(project.projectDir, "hack.env.default.yaml"), "utf8") + ).rejects.toMatchObject({ code: "ENOENT" }); + }, + CLI_TEST_TIMEOUT_MS +); + test( "doctor refuses the reserved local overlay without exposing or losing plaintext", async () => { @@ -419,7 +441,7 @@ test( const run = await runDoctor(project); expect(run.exitCode).toBe(1); expect(run.output).not.toContain(VALUE); - expect(run.output).toContain(".env.local would collide"); + expect(run.output).toContain('".env.local" uses a name reserved'); expect(await readFile(join(project.projectDir, ".env"), "utf8")).toContain( VALUE ); @@ -480,7 +502,7 @@ test.each(["staging.local", "staging.LOCAL"])( const run = await runDoctor(project); expect(run.exitCode).toBe(1); expect(run.output).not.toContain(VALUE); - expect(run.output).toContain("env layer destination paths collide"); + expect(run.output).toContain(`".env.${overlayName}" uses a name reserved`); for (const [name, text] of Object.entries(sources)) { expect(await readFile(join(project.projectDir, name), "utf8")).toBe(text); } diff --git a/tests/fixtures/contract-migration-keychain-worker.ts b/tests/fixtures/contract-migration-keychain-worker.ts new file mode 100644 index 000000000..62f2d19cb --- /dev/null +++ b/tests/fixtures/contract-migration-keychain-worker.ts @@ -0,0 +1,60 @@ +import { spyOn } from "bun:test"; +import { secrets } from "bun"; + +/** + * Runs one contract migration with `Bun.secrets` stubbed, so no test reaches the + * OS keychain, and prints `{ message, calls }` as JSON. `HACK_TEST_KEYCHAIN` picks + * the keychain: `empty`, `key` or `throw`. One process per scenario, because + * secret-store.ts caches key material and a failure cooldown at module level. + */ +const PREFLIGHT_SERVICE = "contract-migration-keychain-preflight"; +const PREFLIGHT_ANSWER = "stub-active"; +const keychain = process.env.HACK_TEST_KEYCHAIN; +const projectRoot = process.env.HACK_TEST_PROJECT_ROOT; +if (!(keychain && projectRoot)) { + throw new Error("Missing HACK_TEST_KEYCHAIN or HACK_TEST_PROJECT_ROOT"); +} +const calls = { get: 0, set: 0, delete: 0 }; + +spyOn(secrets, "get").mockImplementation(async (opts) => { + if (opts.service === PREFLIGHT_SERVICE) { + return PREFLIGHT_ANSWER; + } + calls.get += 1; + if (keychain === "throw") { + throw new Error("stub keychain failure"); + } + return keychain === "key" ? "stub-keychain-key" : null; +}); +spyOn(secrets, "set").mockImplementation(async () => { + calls.set += 1; +}); +spyOn(secrets, "delete").mockImplementation(async () => { + calls.delete += 1; + return false; +}); + +// Fails before any migration runs if this reference to `secrets` isn't stubbed. The +// test's `calls.get` check shows afterwards that secret-store.ts used the stub too. +const preflight = await secrets.get({ + service: PREFLIGHT_SERVICE, + name: "preflight", +}); +if (preflight !== PREFLIGHT_ANSWER) { + throw new Error("Bun.secrets stub is not active"); +} + +const { migrateLegacyProjectEnv } = await import( + "../../src/lib/project-env-config.ts" +); +const message = await migrateLegacyProjectEnv({ + projectRoot, + projectDir: `${projectRoot}/.hack`, + projectName: "contract-keychain", + serviceNames: ["api"], + materialize: false, +}).then( + () => null, + (error: unknown) => (error instanceof Error ? error.message : String(error)) +); +console.log(JSON.stringify({ message, calls }));