diff --git a/docs/architecture.md b/docs/architecture.md index eedd166c..2b20b9ba 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -113,8 +113,8 @@ Projects can declare shareable env overlays and safely inject secrets into compo At runtime, hack resolves the selected overlay and injects the effective env directly into compose and host-side command flows. For service-scoped values, hack still generates `.hack/.internal/compose.env.override.yml` so compose services get the right per-service environment -without requiring `.hack/.env` to exist. That file holds resolved plaintext, so it is written like -`.hack/.env`: atomically, mode `0600`, and never through a symlink. +without requiring `.hack/.env` to exist. That file holds only `${HACK_ENV_...}` references; the values +reach `docker compose` through its process environment, so no decrypted value rests on disk. See `docs/env.md` for the current env model and migration notes. diff --git a/docs/env.md b/docs/env.md index 35af7494..c70347f6 100644 --- a/docs/env.md +++ b/docs/env.md @@ -196,6 +196,37 @@ Hack reads the canonical YAML files and injects the resolved env directly into: That means `.hack/.env` is no longer the primary runtime source of truth. +On the Compose backend, Hack still generates `.hack/.internal/compose.env.override.yml` +so each service gets its own environment. The file holds no values: each entry is a +`${HACK_ENV__}` reference, and Hack passes the resolved values to +`docker compose` through its process environment. Nothing Hack writes there contains a +decrypted value, whether `up` succeeds, fails or is interrupted. The file is written +with mode 0600, and Hack refuses to write through a symlink at that path. A value +reaches the container literally, `$` included. + +Only the `docker compose` invocations that read the override (`up`, `run`, `exec`, +`ps` and the dependency cache initialization) receive the `HACK_ENV_*` values. The other +`docker` commands Hack runs around Compose, such as `docker inspect`, `docker logs` and +`docker events` for dependency cache progress and reuse, get the same environment +without them. + +Because the file holds only references, a `docker compose -f +.hack/.internal/compose.env.override.yml ...` command run outside Hack (a script, an IDE +integration or a test harness) gets a blank value for every managed variable, with only a +per-variable warning from Compose. Use `hack run` or `hack exec`, or supply the same +`HACK_ENV_*` variables to that command. + +Every service gets its own `HACK_ENV_*` copy of each value, including values that come +from the global scope, so the Compose client environment grows with services times keys. +The platform limits apply to it: about 1 MiB of arguments and environment in total on +macOS, and on Linux also 128 KiB (`MAX_ARG_STRLEN`) for a single `NAME=value` string. A +larger single value, such as a big JSON credential, makes the Compose start fail with +`E2BIG`. + +When you upgrade from an earlier version, read the two subsections at the end of this +section: managed values that contain `$` now reach containers differently, and an +override written by the earlier version can still hold values. + `hack env materialize` is manual by design. Use it only when you need a compatibility file for an external tool that expects `.env` on disk. Hack writes `.hack/.env` and `.hack/.env.` (from `materialize`, legacy @@ -216,6 +247,53 @@ That means Hack prefers host-usable values when it can: Use `--target compose` if you explicitly want the container-oriented compose view instead. +### Upgrading: managed values that contain `$` + +Earlier versions wrote each value into the override literally, so Compose interpolated +a `$` inside a managed value: `$VAR` and `${VAR}` became the host or Compose value of +`VAR` (blank when unset), and `$$` became `$`. Now Compose substitutes each reference +once and does not interpolate the value, so the container receives the value exactly as +stored, `$` included. If a managed value contains `$`: + +- the container sees a different value after the upgrade, and the next `hack up` + recreates that service's containers because their configuration changed; +- a value that relied on interpolation (for example `${HOME}/data`) now reaches the + container unexpanded; store the expanded value instead; +- a value that escaped `$` as `$$` now reaches the container as `$$`; store a single `$`. + +Values without `$` reach containers unchanged, and their containers are not recreated. + +### Overrides from earlier versions + +An override written by an earlier Hack version can still hold literal values. The next +`hack up`, `hack restart`, `hack run` or `hack exec` replaces it or, when it renders no +override, removes it. `hack down` removes it before its lifecycle hooks, the remote target +or `docker compose down` run, so a failed hook, a remote target or a failed Compose down +does not keep it. `hack doctor` reports such a file in its "compose env at rest" check +(counts only, never values), and `hack doctor --fix` removes it. + +A legacy adoption (`hack config adopt`) pins the override's exact bytes and file +identity for as long as the adoption exists, and adopted commands never rewrite it. So: + +- `hack config adopt` and `hack config adopt --dry-run` refuse a literal override. Run + `hack up` once so Hack rewrites it with references, then adopt. +- An adoption made by an earlier version over a literal override keeps that file. Hack + never removes it while a prepared, active or pending adoption exists, because removal + would make the adoption refuse to load. `hack doctor` reports it with this next step. + To drop the values, stop the adopted containers (`hack down`), run + `hack config adopt --rollback`, run `hack up` so Hack rewrites the file with references, + then adopt again with `hack config adopt --stop`. + +A removal first moves the file aside, then checks what it moved. If a concurrent Hack +command had just replaced it with a value-free file, that file is put back rather than +deleted. Between the move and the put-back the path is briefly absent, so a Compose +command started by that concurrent Hack command at that moment can fail to find it; run +the command again. If Hack is killed between the move and the delete, the moved file +(`.hack/.internal/.compose.env.override.yml..remove`) can hold values. `hack doctor` +reports such files by count, and the next removal, `hack doctor --fix`, or `hack up`, +`hack restart`, `hack run` or `hack exec` deletes each one once it is more than a minute +old. A newer one may belong to a removal that is still running, so it is left alone. + ## Choosing the right execution surface - `hack host exec` / `hack host shell`: run on your host machine with Hack-resolved env injected. diff --git a/docs/security/env-secrets-threat-model.md b/docs/security/env-secrets-threat-model.md index 9e968dcd..9551ee2e 100644 --- a/docs/security/env-secrets-threat-model.md +++ b/docs/security/env-secrets-threat-model.md @@ -32,7 +32,7 @@ node code is listed under [open gaps](#open-gaps) so that it is not lost. | `encrypted_file` store and its key | Store file chosen in config; key from `HACK_SECRETS_FILE_KEY`, `~/.hack/secrets-file.key`, or the keychain service `hack-secrets-backend` | Legacy backend values, including the `cloud` shim. | | `keychain` store | macOS keychain through `Bun.secrets`, one service per project | Legacy backend values. | | Values from an external manager (for example 1Password) | The manager's vault, until the user pipes a value into Hack or injects `HACK_ENV_SECRET_KEY` into a process | Hack has no first-party 1Password integration. Custody passes to Hack's stores at the moment of the pipe or injection. | -| Decrypted values in flight | Process environments of Compose containers, host commands, lifecycle processes and session shells; `.hack/.env`; `.hack/.internal/compose.env.override.yml` | The resolved plaintext, after decryption. | +| Decrypted values in flight | Process environments of the `docker compose` client, Compose containers, host commands, lifecycle processes and session shells; `.hack/.env` | The resolved plaintext, after decryption. `.hack/.internal/compose.env.override.yml` holds only `${HACK_ENV_...}` references (HACK-1273). | ## Trust boundaries @@ -75,10 +75,10 @@ refer to `hack-dance/hack`. | 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). | +| The Compose backend leaves decrypted values at rest in `.hack/.internal/compose.env.override.yml` after `up`, `down`, a failed `up` or an interrupted one. | The override holds only `${HACK_ENV__}` references; each value reaches `docker compose` through its process environment, so no Compose command writes a value to disk, whatever way it ends (`renderComposeEnvReferenceOverride` in `src/lib/compose-managed-env.ts`). The file keeps its path because Compose records it in each container's `config_files` label. It is written with `replacePrivateFileAtomic` (mode 0600, symlink refused), and a literal value is refused before any write (`writeComposeEnvOverride` in `src/lib/compose-env-at-rest.ts`). A file left by an earlier version is replaced by the next Compose command that renders the override, removed by one that renders none and by `hack down` (before its hooks, remote target and Compose down, so none of them can skip it), and reported by the `hack doctor` "compose env at rest" check by counts only; `hack doctor --fix` removes it. Removal moves the file aside atomically and classifies what it moved, so a value-free file written concurrently is put back rather than deleted (the path is absent for that moment; see gap 4). An aside left by a crash between the move and the delete is reported by the doctor check by count and removed by a later removal, `hack doctor --fix` or a Compose command that renders the override, once it is more than a minute old. Legacy adoption gives the reference values only to its Compose config-hash probe at preview and preparation; a saved recheck runs without values and binds each container's config-hash label to the hash proven at preparation. `hack config adopt` and its preview refuse a literal override, because an adoption pins the file's bytes and inode for its life. While a prepared, active or pending adoption exists, no removal touches the file (removal would make the adoption unloadable); an adoption that an earlier version made over a literal file therefore keeps its values at rest until `hack config adopt --rollback`, and the doctor check names that next step (see gap 4). The per-service `HACK_ENV_*` values go only to `docker compose` invocations that read the override; the `docker inspect`, `logs` and `events` children for dependency cache progress and reuse get the environment without them (`withoutComposeEnvReferences`). Proven with a stub `docker` by `tests/compose-env-at-rest-cli.test.ts`, `tests/compose-env-at-rest.test.ts`, `tests/compose-env-reference-least-privilege.test.ts`, `tests/doctor-fix-noninteractive.test.ts`, `tests/native-compose-adoption-generation.test.ts` and the sentinel scan `tests/env-secret-sentinel-scan.test.ts` (HACK-1273). | | Two writers of the `encrypted_file` store lose an update. | An in-process queue plus a cooperative `.lock` using the owner protocol from `src/lib/projects-registry-lock.ts`; a symlinked store path is refused for writes (`src/lib/secret-store.ts`, #302). | | Concurrent first users of the `encrypted_file` store each generate a key; the last key file wins and the store, already encrypted under another key, can no longer be decrypted. | The generated key is published with `publishPrivateFileExclusive` (`src/lib/private-file.ts`): a fsynced 0600 sibling is linked into place only when no key file exists, so losers adopt the published key. A store found non-empty after the first key read re-reads the key file before the keychain fallback. Proven across eight worker processes by `tests/secret-key-provisioning-convergence.test.ts`. | | Concurrent first secrets in the primary checkout and a linked worktree create two different project keys (checkout-local and shared), so values do not decrypt across checkouts; or a reader refuses a key file that another process is still publishing. | When no key exists, `ensureProjectEnvSecretKey` (`src/lib/project-env-config.ts`) creates it under a cooperative lock at `/.hack.secret.key.lock` and repeats the lookup under the lock. Key files are published complete with `publishPrivateFileExclusive`, and a key read that overlaps a publication is retried for about a quarter second while the entry is a regular file. Proven by `tests/secret-key-provisioning-convergence.test.ts`. Hack v4 writers do not take the lock. | -| Resolved plaintext for Compose lands in a world-readable file or is redirected through a symlink. | The modern projection `.hack/.internal/compose.env.override.yml` is written with `writePrivateEnvFileIfChanged` (0600, atomic, symlink refusal) in `resolveModernComposeEnvOverrides` (`src/commands/project.ts`). The legacy dotenv path writes `${KEY}` interpolations only, not values. | ### Malformed or hostile ciphertext @@ -96,7 +96,7 @@ refer to `hack-dance/hack`. | A key copy ships in build output or a static site. | `hack doctor` reports `env-key-public-path` for copies under `public/`, `static/`, `dist/`, `build/`, `out/`, `www/`, `_site/`, `.next/` and `.output/` (#309). | | A weak, short or repetitive key. | `env-key-wrong-length` and `env-key-low-entropy` (#309). Generated keys are 32 random bytes in base64url. | | Materialized `.hack/.env` or local overrides are committed. | Hack owns a committed `.hack/.gitignore` block, and `hack doctor` flags tracked generated files (see [ignore rules](../env.md#ignore-rules-committed-hackgitignore)). | -| A secret leaks into a public artifact through a path nobody listed. | `tests/env-secret-sentinel-scan.test.ts` runs in the CI `test` job. It gives a primary checkout and a linked worktree random synthetic secrets through `env add --secret --value-stdin` (shared and `--local`), runs materialize, exec, up (Compose generation), env apply, logs, list, explain and doctor, then commits everything with `git add -A`. It requires zero occurrences of any value or the project key, raw or hex/base64/base64url encoded, in CLI output, recorded child argv, tracked files, every Git object, HACK_HOME, the private HOME and its caches, and generated Compose and runtime files. Plaintext may exist only in `.hack/.env` and `.hack/.internal/compose.env.override.yml`, and key text only in key files; each must be git-ignored, untracked and mode 0600. Values may reach a child's environment only for the exec'd command and `docker compose`. Positive controls prove delivery and that the scanner finds planted encodings in files and Git objects. Docker is a recording shim; no engine, keychain or installed Hack is reached. | +| A secret leaks into a public artifact through a path nobody listed. | `tests/env-secret-sentinel-scan.test.ts` runs in the CI `test` job. It gives a primary checkout and a linked worktree random synthetic secrets through `env add --secret --value-stdin` (shared and `--local`), runs materialize, exec, up (Compose generation), env apply, logs, list, explain and doctor, then commits everything with `git add -A`. It requires zero occurrences of any value or the project key, raw or hex/base64/base64url encoded, in CLI output, recorded child argv, tracked files, every Git object, HACK_HOME, the private HOME and its caches, and generated Compose and runtime files. Plaintext may exist only in `.hack/.env`, and key text only in key files; each must be git-ignored, untracked and mode 0600. `.hack/.internal/compose.env.override.yml` must hold `${HACK_ENV_...}` references and no value, and must also be private and ignored. Values may reach a child's environment only for the exec'd command and `docker compose`. Positive controls prove delivery and that the scanner finds planted encodings in files and Git objects. Docker is a recording shim; no engine, keychain or installed Hack is reached. | | A dependency vulnerability ships in Hack. | `bun install --frozen-lockfile`, and the read-only `scripts/dependabot-inventory.ts` lists open alerts (#309). Hack does not sandbox project dependencies that run under `hack host exec`; they receive the injected env by design. | ### Agent and MCP exfiltration @@ -125,13 +125,33 @@ These gaps are known and not closed on `next`. Each one needs its own change. 1Password stay only in the expected custody: the keychain item, the encrypted YAML or store, and the target process environment. The CLI tests disable the keychain fallback (`HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK=1`) and never call 1Password. -4. **Plaintext materializations stay plaintext.** `.hack/.env` and - `.hack/.internal/compose.env.override.yml` are now both private (0600, atomic, - symlink refusal) and git-ignored, and the sentinel scan enforces that. They still - hold decrypted values by design, so backups, file sync and anything running as the - user can read them. Other generated files under `.hack/.internal/` and - `.hack/.branch/` are written with the process umask; the scan shows they hold no - values today. +4. **Plaintext materializations and the Compose client environment.** `.hack/.env` + is private (0600, atomic, symlink refusal) and git-ignored, and the sentinel scan + enforces that. It still holds decrypted values by design, so backups, file sync and + anything running as the user can read it. `.hack/.internal/compose.env.override.yml` + no longer holds values (HACK-1273), but the `docker compose` client process now + carries one `HACK_ENV_*` variable per service and key. Like every injected env, it is + readable by the same user (for example `ps eww`) for the life of that process. Each + service gets its own copy of each value, so the size grows with services times keys. + A project whose resolved values exceed the platform's argument and environment limit + (about 1 MiB in total on macOS; on Linux also 128 KiB, `MAX_ARG_STRLEN`, for one + `NAME=value` string such as a large PEM bundle) cannot start through Compose; the + start fails with `E2BIG`. Existing containers keep their values in `docker inspect` + (gap 9). A direct `docker compose -f` run of the override outside Hack gets blank + values. Legacy adoption resolves the values at preview and preparation and checks + each container's config-hash label against Compose's hash computed with them; a + saved recheck, which has no key or values, compares the label with the hash saved + at preparation while the Compose copy, the generated override bytes and the managed + env revision stay pinned. Removing an earlier version's literal override moves it + aside first; if a concurrent Hack command had just written a value-free file there, + that file is put back, but for the moment between the move and the put-back the path + is absent, so a concurrent Compose command reading it then fails and must be + retried. An aside left by a crash in that step stays until a later removal finds + it stale; `hack doctor` reports it by count. An adoption that an earlier Hack + version made over a literal override keeps that file, values included, until it is + rolled back: the adoption pins the file, so Hack refuses to remove it and the doctor + check names rollback, `hack up` and adopting again as the way to drop the values. Other generated files under `.hack/.internal/` and `.hack/.branch/` are + written with the process umask; the scan shows they hold no values today. 5. **YAML env file writes.** `writeProjectEnvConfigFile` in `src/lib/project-env-config.ts` also uses `writeTextFileIfChanged`, so `.hack/hack.env.*.yaml` writes follow a symlink at the file. These files hold diff --git a/docs/security/env-secrets.md b/docs/security/env-secrets.md index 1ce7ddaa..78717d63 100644 --- a/docs/security/env-secrets.md +++ b/docs/security/env-secrets.md @@ -62,7 +62,13 @@ job) enforces the artifact policy above for one primary checkout and one linked worktree: after env set, materialize, exec, Compose generation, env apply, logs and doctor and a careless `git add -A` commit, no value or key appears in tracked files, Git objects, output, child argv, HACK_HOME or caches; plaintext exists only in the -private, ignored, mode-0600 `.hack/.env` and `.hack/.internal/compose.env.override.yml`. +private, ignored, mode-0600 `.hack/.env`. The generated +`.hack/.internal/compose.env.override.yml` must hold `${HACK_ENV_...}` references and no +value (HACK-1273), and must also be private and ignored. Values reach a child process +environment only for the exec'd command and the `docker compose` client. The +per-service `HACK_ENV_*` values go only to `docker compose` invocations that read the +override; the `docker inspect`, `logs` and `events` children Hack starts around Compose +get the same environment without them. Its limits are listed in the threat model. ## Key writes and rotation diff --git a/src/commands/config-adopt.ts b/src/commands/config-adopt.ts index 31098089..19bc4af4 100644 --- a/src/commands/config-adopt.ts +++ b/src/commands/config-adopt.ts @@ -14,7 +14,11 @@ import { type LegacyComposeAdoptedGenerationStore, openLegacyComposeAdoptedGenerationStore, } from "../lib/native-compose-adoption-generation.ts"; -import { previewLegacyComposeAdoption } from "../lib/native-compose-adoption-preview.ts"; +import { + LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE, + legacyAdoptionEnvOverrideHoldsValues, + previewLegacyComposeAdoption, +} from "../lib/native-compose-adoption-preview.ts"; import { legacyComposePublicationRefusal } from "../lib/native-compose-adoption-publication-diagnostics.ts"; import { legacyComposeRetainedOrdered } from "../lib/native-compose-adoption-readiness.ts"; import { runLegacyComposeRetainedRoutingOperation } from "../lib/native-compose-adoption-routing-execution.ts"; @@ -68,6 +72,22 @@ const spec = defineCommand({ subcommands: [], } as const); +/** + * Adoption pins the generated env override's bytes and inode for the life of + * the adoption, so binding a literal file from an earlier version would keep + * decrypted values on disk (HACK-1273). Fixed text; values omitted. + */ +const LITERAL_ENV_OVERRIDE_REFUSAL = + "Refused adoption: .hack/.internal/compose.env.override.yml holds literal env values written by an earlier Hack version, and adoption would keep them on disk. Run 'hack up' once so Hack rewrites it with references only, then retry 'hack config adopt'. Values omitted."; + +/** The one refusal whose fixed guidance survives the handler's redaction. */ +class LiteralEnvOverrideRefusal extends HackCliError { + constructor() { + super({ code: "E_CONFIG_INVALID", message: LITERAL_ENV_OVERRIDE_REFUSAL }); + this.name = "LiteralEnvOverrideRefusal"; + } +} + async function dryRun( projectRoot: string, signal: AbortSignal, @@ -90,6 +110,13 @@ async function dryRun( `${field.document} ${JSON.stringify(field.pointer || "/")} (${field.line}:${field.column}): ${field.status} ${field.code}\n` ); } + if ( + report.fields.some( + (field) => field.code === LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE + ) + ) { + process.stdout.write(`${LITERAL_ENV_OVERRIDE_REFUSAL}\n`); + } } return report.complete ? 0 : 1; } @@ -174,13 +201,23 @@ async function apply(opts: { readonly stop?: boolean; readonly branch?: string; }) { - const saved = opts.rollback || opts.recover, - store = await openLegacyComposeAdoptedGenerationStore({ + const saved = opts.rollback || opts.recover; + // A new preparation would pin a literal override for the life of the + // adoption. Recovery and rollback act on an existing generation's proof. + if ( + !saved && + (await legacyAdoptionEnvOverrideHoldsValues({ projectRoot: opts.projectRoot, - requestedBranch: opts.branch, - mode: saved ? "saved" : "prepare", - signal: opts.signal, - }); + })) + ) { + throw new LiteralEnvOverrideRefusal(); + } + const store = await openLegacyComposeAdoptedGenerationStore({ + projectRoot: opts.projectRoot, + requestedBranch: opts.branch, + mode: saved ? "saved" : "prepare", + signal: opts.signal, + }); try { if (opts.recover && !opts.stop) { await store.recoverInterruptedLock(); @@ -252,6 +289,9 @@ export const configAdoptCommand = withHandler(spec, async ({ ctx, args }) => { branch: args.options.branch, }); } catch (error: unknown) { + if (error instanceof LiteralEnvOverrideRefusal) { + throw error; + } const diagnostic = legacyComposePublicationRefusal(error); throw new HackCliError({ code: "E_CONFIG_INVALID", diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index a0c88484..2d52557d 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -1,6 +1,6 @@ import { lookup } from "node:dns/promises"; import { homedir } from "node:os"; -import { dirname, resolve } from "node:path"; +import { dirname, join, resolve } from "node:path"; import { note, spinner } from "@clack/prompts"; import { YAML } from "bun"; import { HACK_AGENT_INTEGRATION_CONTENT_REVISION } from "../agents/integration-revision.ts"; @@ -57,6 +57,13 @@ import { resolveGlobalCaddyIp, } from "../lib/caddy-hosts.ts"; import { emitCliResult, okResult } from "../lib/cli-result.ts"; +import { + ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP, + composeEnvAtRestRepairs, + describeComposeEnvAtRest, + inspectComposeEnvAtRest, + removePlaintextComposeEnvOverride, +} from "../lib/compose-env-at-rest.ts"; import { resolveGlobalConfigPath, resolveGlobalHackDir, @@ -117,6 +124,7 @@ import { type NativeCaDoctorDependencies, repairNativeCaTrust, } from "../lib/native-ca-doctor.ts"; +import { selectLegacyComposeAdoptedRoot } from "../lib/native-compose-adoption-command.ts"; import { describeHackLocalRoute, HACK_LOCAL_ROLLBACK_COMMAND, @@ -896,6 +904,11 @@ const handleDoctor: CommandHandlerFor = async ({ } ) ); + results.push( + await runCheck(s, COMPOSE_ENV_AT_REST_CHECK, () => + checkComposeEnvOverrideAtRest({ startDir }) + ) + ); if ( results.some( (result) => result.name === "docker daemon" && result.status === "ok" @@ -3053,6 +3066,70 @@ async function checkTrackedGeneratedFiles({ }; } +const COMPOSE_ENV_AT_REST_CHECK = "compose env at rest"; + +/** + * Warns when `.hack/.internal/compose.env.override.yml` holds literal env + * values (an earlier Hack version wrote decrypted values there) or cannot be + * verified, and when an interrupted removal left an aside that may hold them. + * Counts only; never a key or value. `hack doctor --fix` removes them, except + * an override that a saved adoption pins (the message gives the next step). + */ +export async function checkComposeEnvOverrideAtRest({ + startDir, +}: { + readonly startDir: string; +}): Promise { + const name = COMPOSE_ENV_AT_REST_CHECK; + const ctx = await findComposeEnvAtRestProject({ startDir }); + if (!ctx) { + return { + name, + status: "warn", + message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, + }; + } + const report = await inspectComposeEnvAtRest({ projectDir: ctx.projectDir }); + return { + name, + ...describeComposeEnvAtRest({ + report, + projectDirName: ctx.projectDirName, + }), + }; +} + +/** + * The project whose generated Compose env override doctor inspects. A legacy + * adoption makes ordinary discovery refuse the project (its inputs become + * native), yet the adoption can pin an earlier version's literal override, so + * an active or pending adoption root is selected explicitly. + */ +async function findComposeEnvAtRestProject(opts: { + readonly startDir: string; +}): Promise<{ + readonly projectDir: string; + readonly projectDirName: string; +} | null> { + try { + const ctx = await findProjectContext(opts.startDir); + if (ctx) { + return ctx; + } + } catch { + // Adopted or refused inputs: fall through to the adoption selector. + } + const adopted = await selectLegacyComposeAdoptedRoot({ + cwd: opts.startDir, + }).catch(() => null); + return adopted + ? { + projectDir: join(adopted, HACK_PROJECT_DIR_PRIMARY), + projectDirName: HACK_PROJECT_DIR_PRIMARY, + } + : null; +} + async function checkWorktreeInstanceCollisions({ startDir, }: { @@ -3278,6 +3355,7 @@ async function runDoctorFix(opts: { await maybeInstallMutagenForDoctorFix(); await maybeUntrackGeneratedFiles({ startDir: opts.startDir }); + await maybeRemovePlaintextComposeEnvOverride({ startDir: opts.startDir }); await maybeRepairHackd(); const dockerOk = await dockerInfoOk(); @@ -3625,6 +3703,22 @@ export async function buildDoctorRemediationPlanLines(opts: { ); } + const composeEnvAtRest = project + ? composeEnvAtRestRepairs( + await inspectComposeEnvAtRest({ projectDir: project.projectDir }) + ) + : null; + if (composeEnvAtRest?.override) { + steps.push( + `Remove ${project?.projectDirName}/.internal/compose.env.override.yml, which may hold env values at rest; Compose commands regenerate it with references only.` + ); + } + if (composeEnvAtRest?.asides) { + steps.push( + `Remove leftover ${project?.projectDirName}/.internal/.compose.env.override.yml..remove files from an interrupted removal, which may hold env values.` + ); + } + if (opts.migrateEnvConfig) { steps.push(await buildEnvMigrationPlanStep({ project })); } @@ -3721,6 +3815,101 @@ async function maybeUntrackGeneratedFiles(opts: { note(notes.join("\n"), "generated files"); } +/** + * `hack doctor --fix` step: removes a generated Compose env override that holds + * (or may hold) literal env values, and stale asides that an interrupted + * removal left. Hack-owned and regenerated value-free by the next Compose + * command, so it is a safe generated-file repair. The file is re-inspected + * immediately before removal; a value-free file is kept, and so is one that a + * saved adoption pins (removing it would break the adoption). + */ +async function maybeRemovePlaintextComposeEnvOverride(opts: { + readonly startDir: string; +}): Promise { + const project = await findComposeEnvAtRestProject(opts); + if (!project) { + return; + } + const report = await inspectComposeEnvAtRest({ + projectDir: project.projectDir, + }); + const file = `${project.projectDirName}/.internal/compose.env.override.yml`; + const repairs = composeEnvAtRestRepairs(report); + if (report.adoptionBound) { + note( + `Kept ${file}: a saved 'hack config adopt' generation pins it, and removing it would break the adoption (values omitted). ${ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP}`, + "compose env at rest" + ); + } + if (!(repairs.override || repairs.asides)) { + return; + } + const targets = [ + ...(repairs.override ? [file] : []), + ...(repairs.asides + ? [ + `leftover ${project.projectDirName}/.internal/.compose.env.override.yml..remove files`, + ] + : []), + ].join(" and "); + const ok = await doctorConfirm({ + message: `Remove ${targets}? They may hold env values at rest; Compose commands regenerate the override with references only.`, + initialValue: true, + }); + if (!ok) { + return; + } + try { + const removal = await removePlaintextComposeEnvOverride({ + projectDir: project.projectDir, + includeUnverifiable: true, + }); + note( + composeEnvAtRestFixLines({ + removal, + file, + overrideRequested: repairs.override, + }).join("\n"), + "compose env at rest" + ); + } catch { + note( + `Failed to remove ${targets}; remove them manually (values omitted).`, + "compose env at rest" + ); + } +} + +/** Value-free outcome lines for the `--fix` step above. */ +function composeEnvAtRestFixLines(opts: { + readonly removal: Awaited< + ReturnType + >; + readonly file: string; + readonly overrideRequested: boolean; +}): string[] { + const { removal, file } = opts; + const lines = + removal.asidesRemoved > 0 + ? [ + `Removed ${removal.asidesRemoved} leftover removal file(s) (values omitted).`, + ] + : []; + if (!opts.overrideRequested) { + return lines; + } + if (removal.removed) { + lines.push(`Removed ${file} (values omitted).`); + } else if (removal.adoptionBound) { + lines.push( + `Kept ${file}: a saved 'hack config adopt' generation now pins it (values omitted).` + ); + } else { + lines.push(`Left ${file} in place: it no longer holds env values.`); + } + return lines; +} + async function maybeRepairProjectLifecycleSessions(opts: { readonly startDir: string; }): Promise { diff --git a/src/commands/project.ts b/src/commands/project.ts index 7512381f..ddc405d6 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -151,8 +151,12 @@ import { HackCliError, okResult, } from "../lib/cli-result.ts"; +import { + removePlaintextComposeEnvOverride, + writeComposeEnvOverride, +} from "../lib/compose-env-at-rest.ts"; import { withComposeServiceEnvFingerprints } from "../lib/compose-env-fingerprint.ts"; -import { renderManagedComposeEnvOverride } from "../lib/compose-managed-env.ts"; +import { renderComposeEnvReferenceOverride } from "../lib/compose-managed-env.ts"; import { resolveComposeProjectName } from "../lib/compose-project-name.ts"; import { type ComposeStartupBudgets, @@ -228,7 +232,6 @@ import { resolvePreferredOpenHost, } from "../lib/open-host.ts"; import { openUrl } from "../lib/os.ts"; -import { writePrivateEnvFileIfChanged } from "../lib/private-env-file.ts"; import { defaultProjectSlugFromPath, findProjectContext, @@ -1196,6 +1199,33 @@ function isEnvVarRelevantToServices(opts: { return opts.varServices.some((svc) => serviceSet.has(svc)); } +/** + * Removes an env override that still holds literal values (written by an + * earlier Hack version). Best effort: a failure is reported without values and + * never fails the command; `hack doctor` reports what remains. A file that a + * saved adoption pins is kept (removing it would break the adoption). + */ +async function scrubPlaintextComposeEnvOverride(opts: { + readonly projectDir: string; +}): Promise { + try { + const result = await removePlaintextComposeEnvOverride({ + projectDir: opts.projectDir, + }); + if (result.adoptionBound) { + logger.warn({ + message: + "Kept .hack/.internal/compose.env.override.yml, which holds env values from an earlier Hack version, because a saved 'hack config adopt' generation pins it (values omitted). Run 'hack doctor' for the supported way to drop them.", + }); + } + } catch { + logger.warn({ + message: + "Could not remove .hack/.internal/compose.env.override.yml, which holds env values from an earlier Hack version (values omitted). Run 'hack doctor --fix'.", + }); + } +} + async function maybePromptLegacyProjectEnvMigration(opts: { readonly project: Awaited>; readonly projectName: string; @@ -1293,12 +1323,17 @@ export async function resolveModernComposeEnvOverrides(opts: { }); } - const text = renderManagedComposeEnvOverride({ + // Values travel only through the Compose process env; the file on disk holds + // `${NAME}` references (see src/lib/compose-env-at-rest.ts). + const override = renderComposeEnvReferenceOverride({ targetServices: opts.targetServices, globalEnv: modern.globalEnv, serviceEnv: modern.serviceEnv, }); - if (text === null) { + if (override === null) { + await scrubPlaintextComposeEnvOverride({ + projectDir: opts.project.projectDir, + }); return { composeFiles: [], env: modern.globalEnv, @@ -1309,17 +1344,13 @@ export async function resolveModernComposeEnvOverrides(opts: { } await ensureHackDirGitignore({ projectDir: opts.project.projectDir }); - const overridePath = resolve( - opts.project.projectDir, - ".internal", - "compose.env.override.yml" - ); - // The modern projection holds resolved plaintext, including decrypted - // secrets: write it like .hack/.env (0600, atomic, symlink refusal). - await writePrivateEnvFileIfChanged({ envFile: overridePath, text }); + const overridePath = await writeComposeEnvOverride({ + projectDir: opts.project.projectDir, + text: override.text, + }); return { composeFiles: [overridePath], - env: modern.globalEnv, + env: { ...modern.globalEnv, ...override.env }, lifecycleEnv, preflightEnvByService, effectiveEnvName: modern.selection.effectiveEnv, @@ -1419,6 +1450,9 @@ async function resolveComposeEnvOverrides(opts: { ); if (resolved.contract.vars.length === 0) { + await scrubPlaintextComposeEnvOverride({ + projectDir: opts.project.projectDir, + }); return { composeFiles: [], env: {}, @@ -1461,6 +1495,9 @@ async function resolveComposeEnvOverrides(opts: { } if (Object.keys(overrideServices).length === 0) { + await scrubPlaintextComposeEnvOverride({ + projectDir: opts.project.projectDir, + }); return { composeFiles: [], env: resolved.envForCompose, @@ -1474,10 +1511,10 @@ async function resolveComposeEnvOverrides(opts: { const yaml = YAML.stringify(override, null, 2); const text = ensureTrailingNewline(cleanupYaml(yaml)); await ensureHackDirGitignore({ projectDir: opts.project.projectDir }); - const overrideDir = resolve(opts.project.projectDir, ".internal"); - await ensureDir(overrideDir); - const overridePath = resolve(overrideDir, "compose.env.override.yml"); - await writeTextFileIfChanged(overridePath, text); + const overridePath = await writeComposeEnvOverride({ + projectDir: opts.project.projectDir, + text, + }); return { composeFiles: [overridePath], @@ -5947,6 +5984,10 @@ async function runDownCommand({ ); } } + // An override written by an earlier Hack version can hold decrypted values. + // Down never reads it, so it is removed first: a failed lifecycle hook, a + // remote target or a failed Compose down cannot leave it behind. + await scrubPlaintextComposeEnvOverride({ projectDir: project.projectDir }); const remoteDownCode = await runRemoteLifecycleCommand({ project, @@ -7736,6 +7777,7 @@ async function handleExec({ profiles, effectiveEnvName: envOverrides.effectiveEnvName, service, + env: envOverrides.env, }); if (!execReady.ok) { throw new CliUsageError(execReady.message); @@ -7831,6 +7873,12 @@ async function resolveExecTargetReady(opts: { readonly project: Awaited>; readonly effectiveEnvName: string | null; readonly service: string; + /** + * The Compose env, including the `HACK_ENV_*` values the env override + * references. `ps` does not use them, but without them Compose warns once + * per unset reference. + */ + readonly env?: Record; }): Promise< { readonly ok: true } | { readonly ok: false; readonly message: string } > { @@ -7852,6 +7900,7 @@ async function resolveExecTargetReady(opts: { composeProject: opts.composeProject, profiles: opts.profiles, cwd: dirname(opts.project.composeFile), + env: opts.env, }); if (psResult.exitCode !== 0) { return { diff --git a/src/lib/compose-env-at-rest.ts b/src/lib/compose-env-at-rest.ts new file mode 100644 index 00000000..fd6bbb05 --- /dev/null +++ b/src/lib/compose-env-at-rest.ts @@ -0,0 +1,426 @@ +import { randomUUID } from "node:crypto"; +import { link, lstat, mkdir, readdir, rename, unlink } from "node:fs/promises"; +import { basename, dirname, join, resolve } from "node:path"; +import { HACK_PROJECT_DIR_PRIMARY } from "../constants.ts"; +import { + type ComposeEnvOverrideContent, + classifyComposeEnvOverrideText, +} from "./compose-managed-env.ts"; +import { isRecord } from "./guards.ts"; +import { inspectLegacyComposeAdoptionSelection } from "./native-compose-adoption-marker.ts"; +import { + type ReplaceablePrivateFileOptions, + readReplaceablePrivateFile, + replacePrivateFileAtomic, +} from "./private-file.ts"; + +/** + * At-rest ownership of `.hack/.internal/compose.env.override.yml`. + * + * Invariant: Hack writes this file with environment values that are only + * `${NAME}` Compose references (see `renderComposeEnvReferenceOverride`). The + * resolved values reach Compose through its process environment, so no + * decrypted value is written to disk by any Compose command, including a + * failed, interrupted or killed one. The path stays stable because Compose + * records it in each container's `config_files` label. + * + * Files written by earlier Hack versions can still hold literal values. A + * Compose command that renders the override replaces such a file, one that + * renders none removes it, `hack down` removes it, and `hack doctor` reports + * it with an authorized `--fix` removal. A value-free file is never removed + * here, even when a concurrent Compose command replaces the inspected file + * between inspection and removal: it is put back. It is absent only between + * the move aside and the put-back, so a Compose process that opens it in that + * moment fails and must be retried. Nothing here returns or logs a value from + * the file. + * + * Exception: a saved `hack config adopt` generation pins the override's bytes + * and inode in its integrity proof, and adopted commands never write it. While + * such a generation exists (prepared, active or pending), nothing here removes + * the file, because removal would make the adoption unloadable; the supported + * way to drop its values is rollback, `hack up`, then adopting again. New + * adoptions refuse a literal override (`hack config adopt`). + */ +export const COMPOSE_ENV_OVERRIDE_FILENAME = "compose.env.override.yml"; + +const REFUSAL = + "Refused the generated Compose env override: it or .hack/.internal is a symlink or not a regular file. Remove it and retry; values omitted."; + +/** Absolute path of the generated env override for a project directory. */ +export function composeEnvOverridePath(opts: { + readonly projectDir: string; +}): string { + return resolve(opts.projectDir, ".internal", COMPOSE_ENV_OVERRIDE_FILENAME); +} + +function fileOptions(path: string): ReplaceablePrivateFileOptions { + return { + path, + directories: [dirname(path)], + refusal: () => new Error(REFUSAL), + }; +} + +/** + * Writes value-free override text atomically with mode 0600 when the content + * or the permissions differ. `text` must come from + * `renderComposeEnvReferenceOverride`; a literal value is refused before any + * write so a caller bug cannot put a value at rest. + */ +export async function writeComposeEnvOverride(opts: { + readonly projectDir: string; + readonly text: string; + /** Clock for the stale-aside age check; tests only, no authority. */ + readonly now?: () => number; +}): Promise { + if (classifyComposeEnvOverrideText(opts.text).kind !== "references") { + throw new Error( + "Refused to write a Compose env override that holds literal values; values omitted." + ); + } + const path = composeEnvOverridePath(opts); + await mkdir(dirname(path), { recursive: true }); + const options = fileOptions(path); + const existing = await readReplaceablePrivateFile(options); + if (existing?.text !== opts.text || (existing.mode & 0o077) !== 0) { + await replacePrivateFileAtomic({ ...options, text: opts.text }); + } + // An aside left by a crashed removal can hold values, so a rendering command + // sweeps stale ones too, once the directory passed the symlink checks above. + // Best effort: `hack doctor` reports what remains. + await removeStaleAsides({ path, now: opts.now ?? Date.now }).catch(() => 0); + return path; +} + +/** At-rest state of the generated override; counts only, never values. */ +export type ComposeEnvOverrideAtRest = + | { readonly kind: "absent"; readonly path: string } + | { readonly kind: "unreadable"; readonly path: string } + | (ComposeEnvOverrideContent & { readonly path: string }); + +/** + * Reports what the generated override holds without following a symlink. A + * symlink or other non-regular entry is `unverifiable`. + */ +export async function inspectComposeEnvOverrideAtRest(opts: { + readonly projectDir: string; +}): Promise { + const path = composeEnvOverridePath(opts); + return { ...(await inspectEntry(path)), path }; +} + +async function inspectEntry( + path: string +): Promise< + | { readonly kind: "absent" } + | { readonly kind: "unreadable" } + | ComposeEnvOverrideContent +> { + let current: Awaited>; + try { + current = await readReplaceablePrivateFile(fileOptions(path)); + } catch (error: unknown) { + if (error instanceof Error && error.message === REFUSAL) { + const info = await lstat(path).catch(() => null); + return info ? { kind: "unverifiable" } : { kind: "absent" }; + } + return { kind: "unreadable" }; + } + if (current === null) { + return { kind: "absent" }; + } + return classifyComposeEnvOverrideText(current.text); +} + +/** Private sibling that holds an entry only between its move and its unlink. */ +const ASIDE_PATTERN = /^\.compose\.env\.override\.yml\.[0-9a-f-]{36}\.remove$/; +/** An aside is stale (its owner crashed) once its rename is this old. */ +const STALE_ASIDE_MS = 60_000; + +/** + * Removes the override only while it still holds (or may hold) literal + * values: `plaintext` always, `unverifiable` when `includeUnverifiable` is set. + * Returns the state that was acted on. + * + * Race safety: the entry is first moved aside with one atomic rename, and the + * moved entry itself is classified again. If a concurrent Compose command had + * already replaced the inspected file with a value-free one, that file is put + * back with a no-clobber link (a still newer file at the path wins), so a + * value-free override is never removed. An aside left by a crash between the + * move and the unlink is removed by a later call once it is stale. + */ +export async function removePlaintextComposeEnvOverride(opts: { + readonly projectDir: string; + readonly includeUnverifiable?: boolean; + /** Race-test observation point between inspection and the move; no authority. */ + readonly afterInspect?: () => Promise; + /** Clock for the stale-aside age check; tests only, no authority. */ + readonly now?: () => number; +}): Promise<{ + readonly removed: boolean; + readonly state: ComposeEnvOverrideAtRest; + /** True when a saved adoption may pin the file, so it was kept. */ + readonly adoptionBound: boolean; + /** Stale asides of crashed removals that this call deleted. */ + readonly asidesRemoved: number; +}> { + const path = composeEnvOverridePath(opts); + const asidesRemoved = await removeStaleAsides({ + path, + now: opts.now ?? Date.now, + }); + const eligible = (state: { readonly kind: string }) => + state.kind === "plaintext" || + (state.kind === "unverifiable" && opts.includeUnverifiable === true); + const state = await inspectComposeEnvOverrideAtRest(opts); + const kept = { removed: false, adoptionBound: false, asidesRemoved }; + if (!eligible(state)) { + return { ...kept, state }; + } + if (await composeEnvOverrideAdoptionBound(opts)) { + return { ...kept, state, adoptionBound: true }; + } + await opts.afterInspect?.(); + const info = await lstat(path).catch(() => null); + if (!info || info.isDirectory()) { + return { ...kept, state }; + } + const aside = join( + dirname(path), + `.${COMPOSE_ENV_OVERRIDE_FILENAME}.${randomUUID()}.remove` + ); + try { + await rename(path, aside); + } catch (error: unknown) { + if (isRecord(error) && error.code === "ENOENT") { + return { ...kept, state: { kind: "absent", path } }; + } + throw error; + } + const movedInfo = await lstat(aside); + const moved = await inspectEntry(aside); + if (movedInfo.isDirectory() || !eligible(moved)) { + await restoreAside({ aside, path, regular: movedInfo.isFile() }); + return { ...kept, state: { ...moved, path } }; + } + await unlinkIfPresent(aside); + return { ...kept, removed: true, state }; +} + +/** + * True when a saved `hack config adopt` generation may pin the override: its + * private state exists and is not rolled back. Prepared, active and pending + * generations bind the file's bytes and inode in their integrity proof, so + * removing or replacing it makes the adoption refuse to load. An adoption + * state that cannot be read safely counts as bound. Reads the adoption + * receipt status only; never a key or value. + */ +export async function composeEnvOverrideAdoptionBound(opts: { + readonly projectDir: string; +}): Promise { + const projectDir = resolve(opts.projectDir); + if (basename(projectDir) !== HACK_PROJECT_DIR_PRIMARY) { + return false; + } + try { + const status = await inspectLegacyComposeAdoptionSelection({ + projectRoot: dirname(projectDir), + }); + return status !== null && status !== "rolled-back"; + } catch { + return true; + } +} + +/** Puts a moved entry back unless a newer one already holds the path. */ +async function restoreAside(opts: { + readonly aside: string; + readonly path: string; + readonly regular: boolean; +}): Promise { + if (opts.regular) { + try { + await link(opts.aside, opts.path); + } catch (error: unknown) { + if ( + !(isRecord(error) && ["EEXIST", "ENOENT"].includes(String(error.code))) + ) { + throw error; + } + } + await unlinkIfPresent(opts.aside); + return; + } + if (!(await lstat(opts.path).catch(() => null))) { + await rename(opts.aside, opts.path); + } +} + +/** + * Removes stale regular-file asides; they are never a current override. The + * rename that created an aside updated its ctime, so a fresh one may still + * belong to a concurrent call and is left alone. + */ +async function removeStaleAsides(opts: { + readonly path: string; + readonly now: () => number; +}): Promise { + const asides = await listAsides(opts); + for (const aside of asides.stale) { + await unlinkIfPresent(aside); + } + return asides.stale.length; +} + +/** Regular-file asides of crashed or in-flight removals, split by age. */ +async function listAsides(opts: { + readonly path: string; + readonly now: () => number; +}): Promise<{ readonly stale: string[]; readonly fresh: string[] }> { + const directory = dirname(opts.path); + const names = await readdir(directory).catch(() => []); + const stale: string[] = []; + const fresh: string[] = []; + for (const name of names) { + if (!ASIDE_PATTERN.test(name)) { + continue; + } + const aside = join(directory, name); + const info = await lstat(aside).catch(() => null); + if (info?.isFile()) { + (opts.now() - info.ctimeMs > STALE_ASIDE_MS ? stale : fresh).push(aside); + } + } + return { stale, fresh }; +} + +async function unlinkIfPresent(path: string): Promise { + try { + await unlink(path); + } catch (error: unknown) { + if (!(isRecord(error) && error.code === "ENOENT")) { + throw error; + } + } +} + +/** + * Everything `hack doctor` checks about Compose env at rest. Counts only; + * never a key or value. + */ +export type ComposeEnvAtRestReport = { + readonly override: ComposeEnvOverrideAtRest; + /** A saved adoption may pin the override; see {@link composeEnvOverrideAdoptionBound}. */ + readonly adoptionBound: boolean; + /** Asides left by interrupted removals; each may hold literal values. */ + readonly asides: { readonly stale: number; readonly fresh: number }; +}; + +/** Read-only inspection for the doctor check and its `--fix` plan. */ +export async function inspectComposeEnvAtRest(opts: { + readonly projectDir: string; + /** Clock for the stale-aside age check; tests only, no authority. */ + readonly now?: () => number; +}): Promise { + const override = await inspectComposeEnvOverrideAtRest(opts); + const asides = await listAsides({ + path: override.path, + now: opts.now ?? Date.now, + }); + return { + override, + adoptionBound: + composeEnvOverrideMayHoldValues(override) && + (await composeEnvOverrideAdoptionBound(opts)), + asides: { stale: asides.stale.length, fresh: asides.fresh.length }, + }; +} + +function composeEnvOverrideMayHoldValues( + state: ComposeEnvOverrideAtRest +): boolean { + return state.kind === "plaintext" || state.kind === "unverifiable"; +} + +/** + * What `hack doctor --fix` may remove: the override while it may hold values + * and no saved adoption pins it, and stale asides. A fresh aside may belong to + * a removal still in progress and is left to that removal. + */ +export function composeEnvAtRestRepairs(report: ComposeEnvAtRestReport): { + readonly override: boolean; + readonly asides: boolean; +} { + return { + override: + composeEnvOverrideMayHoldValues(report.override) && !report.adoptionBound, + asides: report.asides.stale > 0, + }; +} + +/** Supported way to drop values that a saved adoption pins; see the module doc. */ +export const ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP = + "To drop them, stop the adopted containers ('hack down'), run 'hack config adopt --rollback', run 'hack up' so Hack rewrites the file with references only, then adopt again with 'hack config adopt --stop'."; + +/** + * Doctor status and message. Names the project-relative path and counts only; + * never a key or value. + */ +export function describeComposeEnvAtRest(opts: { + readonly report: ComposeEnvAtRestReport; + readonly projectDirName: string; +}): { readonly status: "ok" | "warn"; readonly message: string } { + const file = `${opts.projectDirName}/.internal/${COMPOSE_ENV_OVERRIDE_FILENAME}`; + const described = describeOverride({ + state: opts.report.override, + file, + adoptionBound: opts.report.adoptionBound, + }); + const asides = opts.report.asides.stale + opts.report.asides.fresh; + if (asides === 0) { + return described; + } + const leftover = `${asides} leftover removal file(s) (${opts.projectDirName}/.internal/.${COMPOSE_ENV_OVERRIDE_FILENAME}..remove) from an interrupted removal may hold env values (values omitted). Run 'hack doctor --fix' to remove those older than a minute.`; + return { + status: "warn", + message: + described.status === "ok" ? leftover : `${described.message} ${leftover}`, + }; +} + +function describeOverride(opts: { + readonly state: ComposeEnvOverrideAtRest; + readonly file: string; + readonly adoptionBound: boolean; +}): { readonly status: "ok" | "warn"; readonly message: string } { + const { state, file } = opts; + const next = opts.adoptionBound + ? `A saved 'hack config adopt' generation pins this file, so 'hack doctor --fix' keeps it: removing it would break the adoption. ${ADOPTED_COMPOSE_ENV_OVERRIDE_NEXT_STEP}` + : "Run 'hack doctor --fix' to remove it; Compose commands now write references only."; + switch (state.kind) { + case "absent": + return { status: "ok", message: "No generated Compose env override" }; + case "references": + return { + status: "ok", + message: `${file} holds env references only (no values at rest)`, + }; + case "plaintext": + return { + status: "warn", + message: `${file} holds ${state.values} literal env value(s) for ${state.services} service(s), written by an earlier Hack version (values omitted). ${next}`, + }; + case "unverifiable": + return { + status: "warn", + message: `${file} is not a Compose override Hack can verify and may hold env values (values omitted). ${next}`, + }; + case "unreadable": + return { + status: "warn", + message: `${file} could not be read, so Hack cannot confirm it holds no env values. Check its owner and permissions.`, + }; + default: + return state satisfies never; + } +} diff --git a/src/lib/compose-managed-env.ts b/src/lib/compose-managed-env.ts index ca6f8d79..8890c966 100644 --- a/src/lib/compose-managed-env.ts +++ b/src/lib/compose-managed-env.ts @@ -1,11 +1,15 @@ +import { createHash } from "node:crypto"; import { YAML } from "bun"; const EMPTY_MAPPING_SPACE = /: \n/g; /** - * Pure projection shared by the legacy writer and strict adoption qualification. + * Literal projection that Hack versions before the reference writer kept at + * `.hack/.internal/compose.env.override.yml`. No writer emits it now; strict + * adoption qualification still recognizes such files, and + * {@link renderComposeEnvReferenceOverride} is the form written today. * Input and output contain private runtime values: never log or serialize them - * into reports or public receipts. Target order and key order are preserved so + * into reports or public receipts, and never write them to disk. Target order and key order are preserved so * existing generated bytes can be checked exactly. This performs no I/O. */ export function renderManagedComposeEnvOverride(opts: { @@ -34,3 +38,216 @@ export function renderManagedComposeEnvOverride(opts: { ); return text.endsWith("\n") ? text : `${text}\n`; } + +/** Prefix of the interpolation variables that carry managed env values to Compose. */ +export const COMPOSE_ENV_REFERENCE_PREFIX = "HACK_ENV_"; + +const ENV_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/; +const COMPOSE_REFERENCE_PATTERN = /^\$\{[A-Za-z_][A-Za-z0-9_]*\}$/; + +/** + * Deterministic Compose interpolation variable for one service's key. + * + * The name depends only on the service and key, never on the value, so the + * rendered override is value-free and stays byte-stable while values change. + * The digest keeps names distinct across services that share a key and keeps + * arbitrary service names inside the variable grammar. + */ +export function composeEnvReferenceName(opts: { + readonly service: string; + readonly key: string; +}): string { + const digest = createHash("sha256") + .update(`${opts.service}\0${opts.key}`) + .digest("hex") + .slice(0, 12) + .toUpperCase(); + return ENV_NAME_PATTERN.test(opts.key) + ? `${COMPOSE_ENV_REFERENCE_PREFIX}${digest}_${opts.key}` + : `${COMPOSE_ENV_REFERENCE_PREFIX}${digest}`; +} + +const COMPOSE_ENV_REFERENCE_NAME = /^HACK_ENV_[0-9A-F]{12}(?:_|$)/; + +/** True for a name {@link composeEnvReferenceName} can produce. */ +export function isComposeEnvReferenceName(name: string): boolean { + return COMPOSE_ENV_REFERENCE_NAME.test(name); +} + +/** + * Copy of `env` without the `HACK_ENV_*` reference values. Only a + * `docker compose` invocation that reads the generated override needs them; + * every other child (for example `docker inspect` or `docker events`) gets + * this copy, so the private per-service values reach no process that cannot + * use them. + */ +export function withoutComposeEnvReferences( + env: Readonly> | undefined +): Record | undefined { + if (!env) { + return env; + } + const kept: Record = {}; + for (const [key, value] of Object.entries(env)) { + if (!isComposeEnvReferenceName(key)) { + kept[key] = value; + } + } + return kept; +} + +/** + * Value-free managed override plus the private values it references. + * + * `text` is safe to keep on disk: every environment value is a single + * `${NAME}` reference. `env` maps each NAME to its private value and must be + * delivered only through the Compose process environment; never persist, log + * or serialize it. Compose substitutes each reference once, so a value that + * contains `$` reaches the container literally. + */ +export type ComposeEnvReferenceOverride = { + readonly text: string; + readonly env: Readonly>; +}; + +/** + * Same service/key selection and order as {@link renderManagedComposeEnvOverride}, + * with every value replaced by a Compose interpolation reference. Pure; no I/O. + */ +export function renderComposeEnvReferenceOverride(opts: { + readonly targetServices: readonly string[]; + readonly globalEnv: Readonly>; + readonly serviceEnv: Readonly< + Record>> + >; +}): ComposeEnvReferenceOverride | null { + const services: Record< + string, + { readonly environment: Readonly> } + > = {}; + const env: Record = {}; + for (const service of opts.targetServices) { + const values = opts.serviceEnv[service] ?? opts.globalEnv; + const environment: Record = {}; + for (const [key, value] of Object.entries(values)) { + const name = composeEnvReferenceName({ service, key }); + environment[key] = `\${${name}}`; + env[name] = value; + } + if (Object.keys(environment).length > 0) { + services[service] = { environment }; + } + } + if (Object.keys(services).length === 0) { + return null; + } + const text = YAML.stringify({ services }, null, 2).replaceAll( + EMPTY_MAPPING_SPACE, + ":\n" + ); + return { text: text.endsWith("\n") ? text : `${text}\n`, env }; +} + +/** + * What a generated Compose env override holds at rest. Counts only; never a + * key or value. + * - `references`: every environment value is a bare key or one `${NAME}` + * reference, so the file holds no env value. + * - `plaintext`: at least one environment value is a literal. + * - `unverifiable`: the text is not a Compose services document Hack can + * check, so it may hold literal values. + */ +export type ComposeEnvOverrideContent = + | { readonly kind: "references" } + | { + readonly kind: "plaintext"; + readonly services: number; + readonly values: number; + } + | { readonly kind: "unverifiable" }; + +function countLiteralEnvironmentValues(environment: unknown): number | null { + if (environment === undefined || environment === null) { + return 0; + } + if (Array.isArray(environment)) { + let literals = 0; + for (const item of environment) { + if (typeof item !== "string") { + literals += 1; + continue; + } + const separator = item.indexOf("="); + if ( + separator >= 0 && + !COMPOSE_REFERENCE_PATTERN.test(item.slice(separator + 1)) + ) { + literals += 1; + } + } + return literals; + } + if (typeof environment !== "object") { + return null; + } + let literals = 0; + for (const value of Object.values(environment)) { + if (value === null) { + continue; + } + if (typeof value !== "string" || !COMPOSE_REFERENCE_PATTERN.test(value)) { + literals += 1; + } + } + return literals; +} + +/** + * Classifies generated override text without returning any of it. Pure. + */ +export function classifyComposeEnvOverrideText( + text: string +): ComposeEnvOverrideContent { + let parsed: unknown; + try { + parsed = YAML.parse(text); + } catch { + return { kind: "unverifiable" }; + } + if (parsed === null || parsed === undefined) { + return { kind: "references" }; + } + if (typeof parsed !== "object" || Array.isArray(parsed)) { + return { kind: "unverifiable" }; + } + const services = (parsed as Record).services; + if (services === undefined || services === null) { + return { kind: "references" }; + } + if (typeof services !== "object" || Array.isArray(services)) { + return { kind: "unverifiable" }; + } + let plaintextServices = 0; + let values = 0; + for (const service of Object.values(services)) { + if (service === null) { + continue; + } + if (typeof service !== "object" || Array.isArray(service)) { + return { kind: "unverifiable" }; + } + const literals = countLiteralEnvironmentValues( + (service as Record).environment + ); + if (literals === null) { + return { kind: "unverifiable" }; + } + if (literals > 0) { + plaintextServices += 1; + values += literals; + } + } + return values > 0 + ? { kind: "plaintext", services: plaintextServices, values } + : { kind: "references" }; +} diff --git a/src/lib/dependency-startup-progress.ts b/src/lib/dependency-startup-progress.ts index 0c2080d9..5f5a585d 100644 --- a/src/lib/dependency-startup-progress.ts +++ b/src/lib/dependency-startup-progress.ts @@ -1,3 +1,4 @@ +import { withoutComposeEnvReferences } from "./compose-managed-env.ts"; import { isRecord } from "./guards.ts"; import { type ProcessIdentityRow, @@ -113,8 +114,14 @@ export async function withDependencyStartupProgress(opts: { /* Progress must not replace startup outcome. */ } } + // These children are `docker inspect/logs/events`, not Compose: they never + // read the generated env override, so its private `HACK_ENV_*` values stay + // with the Compose invocation (HACK-1273). const env: Record = {}; - for (const [key, value] of Object.entries({ ...process.env, ...opts.env })) { + for (const [key, value] of Object.entries({ + ...process.env, + ...withoutComposeEnvReferences(opts.env), + })) { if (value !== undefined) { env[key] = value; } diff --git a/src/lib/native-compose-adoption-binding.ts b/src/lib/native-compose-adoption-binding.ts index cbf7bb24..100f2b24 100644 --- a/src/lib/native-compose-adoption-binding.ts +++ b/src/lib/native-compose-adoption-binding.ts @@ -373,7 +373,12 @@ export type LegacyComposeVerifiedBinding = LegacyComposeVerifiedBindingBase & type ProjectedPreparation = Pick< Awaited>, - "candidate" | "composeFiles" | "metadata" | "projectionProof" | "localFields" + | "candidate" + | "composeFiles" + | "composeEnv" + | "metadata" + | "projectionProof" + | "localFields" >; /** Canonical ordered owner paths only; observations cannot introduce caller-selected override authority. */ @@ -1482,13 +1487,17 @@ async function acquireBinding( retainedRouting: intent.routing !== undefined, }); const resolved = await projection.resolve({ signal }); - projected = Object.freeze({ + const prepared = { candidate: resolved.candidate, composeFiles: resolved.composeFiles, + composeEnv: resolved.composeEnv, metadata: resolved.metadata, projectionProof: resolved.projectionProof, localFields: resolved.localFields, - }); + }; + // Private interpolation values for the runtime config-hash probe only. + Object.defineProperty(prepared, "composeEnv", { enumerable: false }); + projected = Object.freeze(prepared); } const layoutSupported = async (selectedSignal?: AbortSignal) => { if (branch) { diff --git a/src/lib/native-compose-adoption-generation.ts b/src/lib/native-compose-adoption-generation.ts index 677a4471..ebe9652d 100644 --- a/src/lib/native-compose-adoption-generation.ts +++ b/src/lib/native-compose-adoption-generation.ts @@ -1269,6 +1269,9 @@ async function readInputs( : await inspectLegacyComposeRuntimeConfig({ binding: observed, composeFile: join(generationRoot, "legacy-compose.yml"), + // Saved reads deliver no env values; a value-free env override is + // bound to the preparation-time hashes instead. + ...(projection ? { saved: meta.runtimeConfig } : {}), signal: ctx.signal, timeoutMs: ctx.timeoutMs, }); @@ -1701,6 +1704,7 @@ async function prepare( runtimeConfig: await inspectLegacyComposeRuntimeConfig({ binding: acquired.binding, composeFile: join(generationRoot, "legacy-compose.yml"), + privateInterpolationEnv: acquired.projection?.composeEnv, signal: ctx.signal, timeoutMs: ctx.timeoutMs, }), diff --git a/src/lib/native-compose-adoption-preview.ts b/src/lib/native-compose-adoption-preview.ts index bdc6154a..a820992c 100644 --- a/src/lib/native-compose-adoption-preview.ts +++ b/src/lib/native-compose-adoption-preview.ts @@ -1,3 +1,6 @@ +import { join } from "node:path"; +import { HACK_PROJECT_DIR_PRIMARY } from "../constants.ts"; +import { inspectComposeEnvOverrideAtRest } from "./compose-env-at-rest.ts"; import { acquireLegacyComposeAdoptionPreparationBinding } from "./native-compose-adoption-binding.ts"; import { admitLegacyComposeCandidate } from "./native-compose-adoption-compiler.ts"; import { @@ -33,6 +36,25 @@ function report(value: T): Readonly { freezeImportValue(value); return Object.freeze(value); } +/** Preview field code for a generated env override that holds literal values. */ +export const LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE = + "generated_env_override_holds_values"; + +/** + * True when `.hack/.internal/compose.env.override.yml` holds literal env + * values written by an earlier Hack version. Adoption pins the file's bytes and + * inode for the life of the adoption, so it must not bind such a file + * (HACK-1273); `hack up` rewrites it with references first. Counts only. + */ +export async function legacyAdoptionEnvOverrideHoldsValues(opts: { + readonly projectRoot: string; +}): Promise { + const state = await inspectComposeEnvOverrideAtRest({ + projectDir: join(opts.projectRoot, HACK_PROJECT_DIR_PRIMARY), + }); + return state.kind === "plaintext"; +} + /** Read-only exact existing-data preview; source values, resource names and digests never enter this report. */ export async function previewLegacyComposeAdoption(input: { readonly projectRoot: string; @@ -43,6 +65,13 @@ export async function previewLegacyComposeAdoption(input: { const opts = { ...input }; let fields: readonly ImportField[] = []; try { + if (await legacyAdoptionEnvOverrideHoldsValues(opts)) { + return report({ + complete: false, + adoption: "not_performed", + fields: [refused(LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE)], + }); + } const owner = await acquireLegacyComposeAdoptionPreparationBinding(opts), acquired = await owner.resolvePreparationInputs(opts); let mapper = mapLegacyNativeStorageAdoption; @@ -107,6 +136,7 @@ export async function previewLegacyComposeAdoption(input: { await inspectLegacyComposeRuntimeConfig({ binding: acquired.binding, composeFile: acquired.binding.composeFile, + privateInterpolationEnv: acquired.projection?.composeEnv, signal: opts.signal, }); const states = await inspectLegacyComposeContainerStates({ diff --git a/src/lib/native-compose-adoption-projection.ts b/src/lib/native-compose-adoption-projection.ts index 2c12ed55..396d86f5 100644 --- a/src/lib/native-compose-adoption-projection.ts +++ b/src/lib/native-compose-adoption-projection.ts @@ -3,7 +3,10 @@ import type { Stats } from "node:fs"; import { lstat, opendir } from "node:fs/promises"; import { join, resolve } from "node:path"; import { DEFAULT_PROJECT_TLD } from "../constants.ts"; -import { renderManagedComposeEnvOverride } from "./compose-managed-env.ts"; +import { + renderComposeEnvReferenceOverride, + renderManagedComposeEnvOverride, +} from "./compose-managed-env.ts"; import { isRecord } from "./guards.ts"; import { legacyComposeAdoptionCandidateSupported, @@ -784,18 +787,33 @@ export class LegacyComposeAdoptionProjection { ) { refuse(); } - const text = renderManagedComposeEnvOverride({ + const projection = { targetServices: context.serviceNames, globalEnv: values.globalEnv, serviceEnv: values.workloadEnv, - }); + }; + // Current writers keep only `${NAME}` references at rest; files from + // earlier versions hold the literal projection. Either must match exactly. + // `hack config adopt` and its preview refuse a literal file before this + // point (HACK-1273), so a new adoption never pins values on disk. The + // literal match remains so tests can model a generation that an earlier + // version prepared over such a file. + const reference = renderComposeEnvReferenceOverride(projection); + const referenceText = reference?.text ?? null; + const literalText = renderManagedComposeEnvOverride(projection); const [runtime, env] = context.generated.files; + const envText = env?.text ?? null; if ( - (env?.text ?? null) !== text || + (envText !== referenceText && envText !== literalText) || (runtime && runtime.text !== context.runtimeText) ) { refuse(); } + // A reference-form file resolves only with these values in the Compose + // environment; the engine computed each container's config hash with + // them. Private: delivered to the hash probe only, never serialized. + const composeEnv: Readonly> = + reference && envText === reference.text ? reference.env : {}; const candidate = projectRuntimeFallbacks(context.candidate, runtime); const primary = privatePrimaryProof( await context.admission.resolvePrivatePrimaryProof() @@ -810,6 +828,7 @@ export class LegacyComposeAdoptionProjection { ), ]), globalEnv: values.globalEnv, + composeEnv, metadata: context.env.metadata, localFields: context.admission.localFields, projectionProof: { diff --git a/src/lib/native-compose-adoption-runtime.ts b/src/lib/native-compose-adoption-runtime.ts index e8187885..c26bed33 100644 --- a/src/lib/native-compose-adoption-runtime.ts +++ b/src/lib/native-compose-adoption-runtime.ts @@ -40,6 +40,8 @@ function row(text: string) { return parsed; } +const ENV_OVERRIDE_SUFFIX = "/.hack/.internal/compose.env.override.yml"; + /** * Private ordered-source fidelity check. The strict importer and generated-source * owner have already refused interpolation, env files and unknown @@ -47,11 +49,28 @@ function row(text: string) { * and a null env file prevent Compose from discovering alternate inputs. Only * configuration hashes and engine-created hash labels are read, never container * environment/image values. This read-only observation grants no effect authority. + * + * The generated env override keeps only `${HACK_ENV_*}` references at rest + * (HACK-1273), while each container's config-hash label was computed by Compose + * with the resolved values. A fresh check therefore needs those values: + * `privateInterpolationEnv` carries them into the Compose child environment + * only, and they are never returned, logged or serialized. + * + * A saved-generation recheck runs without key or value delivery. When the + * bound files include the env override and no values are supplied, `saved` + * (the runtime config proven fresh at preparation) is required instead: each + * retained container's engine label must still equal its saved hash. The + * caller's saved owners separately pin the Compose copy, the generated + * override bytes and the managed env revision, so an unchanged label means the + * container still runs the configuration proven at preparation. */ export async function inspectLegacyComposeRuntimeConfig(opts: { readonly binding: LegacyComposeVerifiedBinding; readonly composeFile: string; readonly composeFiles?: readonly string[]; + readonly privateInterpolationEnv?: Readonly>; + /** Saved manifest proof; validated here, so the raw saved value is accepted. */ + readonly saved?: unknown; readonly signal?: AbortSignal; readonly timeoutMs?: number; }): Promise< @@ -61,7 +80,11 @@ export async function inspectLegacyComposeRuntimeConfig(opts: { readonly hash: string; }[] > { - const probe = createNativeComposeProbe(opts); + const probe = createNativeComposeProbe({ + signal: opts.signal, + timeoutMs: opts.timeoutMs, + privateInterpolationEnv: opts.privateInterpolationEnv, + }); const expectedFiles = [ opts.composeFile, ...(opts.binding.binding_version === 2 || @@ -78,7 +101,54 @@ export async function inspectLegacyComposeRuntimeConfig(opts: { ) { refuse(); } - const output = await probe([ + const savedOnly = + opts.saved !== undefined && + opts.privateInterpolationEnv === undefined && + expectedFiles.some((file) => file.endsWith(ENV_OVERRIDE_SUFFIX)); + const hashes = savedOnly + ? savedHashes({ binding: opts.binding, saved: opts.saved }) + : await composeHashes({ + probe, + binding: opts.binding, + files: expectedFiles, + }); + const result: RuntimeConfig[] = []; + for (const container of opts.binding.containers) { + if (!ID.test(container.id)) { + refuse(); + } + const inspected = row( + await probe([ + "container", + "inspect", + "--format", + '{"id":{{json .Id}},"hash":{{json (index .Config.Labels "com.docker.compose.config-hash")}}}', + container.id, + ]) + ); + const hash = hashes.get(container.service); + if ( + !hash || + Object.keys(inspected).sort().join() !== "hash,id" || + inspected.id !== container.id || + inspected.hash !== hash + ) { + refuse(); + } + result.push( + Object.freeze({ id: container.id, service: container.service, hash }) + ); + } + return Object.freeze(result); +} + +/** Compose's own per-service hash of the exact ordered files. */ +async function composeHashes(opts: { + readonly probe: (args: readonly string[]) => Promise; + readonly binding: LegacyComposeVerifiedBinding; + readonly files: readonly string[]; +}): Promise> { + const output = await opts.probe([ "compose", "--project-name", opts.binding.composeProject, @@ -88,7 +158,7 @@ export async function inspectLegacyComposeRuntimeConfig(opts: { "/dev/null", "--profile", "*", - ...expectedFiles.flatMap((file) => ["--file", file]), + ...opts.files.flatMap((file) => ["--file", file]), "config", "--no-env-resolution", "--hash", @@ -107,34 +177,42 @@ export async function inspectLegacyComposeRuntimeConfig(opts: { if (hashes.size !== opts.binding.containers.length) { refuse(); } - const result: RuntimeConfig[] = []; - for (const container of opts.binding.containers) { - if (!ID.test(container.id)) { - refuse(); - } - const inspected = row( - await probe([ - "container", - "inspect", - "--format", - '{"id":{{json .Id}},"hash":{{json (index .Config.Labels "com.docker.compose.config-hash")}}}', - container.id, - ]) - ); - const hash = hashes.get(container.service); + return hashes; +} + +/** The saved per-container proof, exactly aligned with the retained binding. */ +function savedHashes(opts: { + readonly binding: LegacyComposeVerifiedBinding; + readonly saved: unknown; +}): Map { + if ( + !( + Array.isArray(opts.saved) && + opts.saved.length === opts.binding.containers.length + ) + ) { + refuse(); + } + const hashes = new Map(); + for (const [index, container] of opts.binding.containers.entries()) { + const saved: unknown = opts.saved[index]; if ( - !hash || - Object.keys(inspected).sort().join() !== "hash,id" || - inspected.id !== container.id || - inspected.hash !== hash + !( + saved && + isRecord(saved) && + Object.keys(saved).sort().join() === "hash,id,service" && + saved.id === container.id && + saved.service === container.service && + typeof saved.hash === "string" && + ID.test(saved.hash) + ) || + hashes.has(container.service) ) { refuse(); } - result.push( - Object.freeze({ id: container.id, service: container.service, hash }) - ); + hashes.set(container.service, saved.hash); } - return Object.freeze(result); + return hashes; } /** Minimal state observation for owned stopped adoption and retained-ID lifecycle postconditions. */ diff --git a/src/lib/native-compose-ownership.ts b/src/lib/native-compose-ownership.ts index 7359f1ed..bc573645 100644 --- a/src/lib/native-compose-ownership.ts +++ b/src/lib/native-compose-ownership.ts @@ -1379,7 +1379,14 @@ function batches(resources: readonly Inventory[]): Inventory[][] { * an acquisition whose aggregate deadline or output allowance has expired. */ export function createNativeComposeProbe( - opts: Pick + opts: Pick & { + /** + * Private Compose interpolation values (the `HACK_ENV_*` references of a + * value-free env override) added to the child environment only. Never + * logged, returned or serialized. + */ + readonly privateInterpolationEnv?: Readonly>; + } ): (args: readonly string[]) => Promise { const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; if ( @@ -1399,7 +1406,7 @@ export function createNativeComposeProbe( if (!binary) { probeRefused("E_NATIVE_COMPOSE_PROBE", "operation"); } - const environment = { ...process.env }; + const environment = { ...process.env, ...opts.privateInterpolationEnv }; return async (args) => { checkInterruption(opts.signal, false); const remainingTime = deadline - Date.now(); diff --git a/src/lib/run-dependency-cache.ts b/src/lib/run-dependency-cache.ts index 2ff51c94..b5374d3a 100644 --- a/src/lib/run-dependency-cache.ts +++ b/src/lib/run-dependency-cache.ts @@ -1,3 +1,4 @@ +import { withoutComposeEnvReferences } from "./compose-managed-env.ts"; import { isRecord } from "./guards.ts"; import { exec } from "./shell.ts"; @@ -27,7 +28,13 @@ export async function canReuseRunningDependencyCache(opts: { try { const result = await exec( ["docker", "inspect", "--format", INSPECT_FORMAT, opts.containerId], - { stdin: "ignore", timeoutMs: 15_000, cwd: opts.cwd, env: opts.env } + { + stdin: "ignore", + timeoutMs: 15_000, + cwd: opts.cwd, + // `docker inspect` never reads the generated env override. + env: withoutComposeEnvReferences(opts.env), + } ); if (result.exitCode !== 0) { return false; diff --git a/tests/compose-env-at-rest-cli.test.ts b/tests/compose-env-at-rest-cli.test.ts new file mode 100644 index 00000000..26e98154 --- /dev/null +++ b/tests/compose-env-at-rest-cli.test.ts @@ -0,0 +1,415 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + realpath, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { PROJECT_ENV_KEY_FILENAME } from "../src/constants.ts"; +import { renderManagedComposeEnvOverride } from "../src/lib/compose-managed-env.ts"; +import { + setProjectEnvValue, + unsetProjectEnvValue, +} from "../src/lib/project-env-config.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +/** + * HACK-1273: the Compose backend must not leave decrypted env values at rest + * in `.hack/.internal/compose.env.override.yml`, whatever way `up` ends. A + * stub `docker` stands in for the engine; it records what Compose would read + * (the override bytes and its process env) so the test can prove the values + * were still delivered. Every value is a synthetic canary. + */ +const CANARY = "g5-synthetic-canary"; +const SECRET_VALUE = `${CANARY}-secret-value`; +const PLAIN_VALUE = `${CANARY}-web-only`; +const CLI = resolve(import.meta.dir, "..", "index.ts"); + +let root: string; +let projectRoot: string; +let capture: string; +let overridePath: string; + +const STUB = [ + "#!/bin/sh", + 'printf "%s\\n" "$*" >> "$STUB_CAPTURE/docker-args.log"', + 'if [ "$1" = "compose" ]; then', + ' case " $* " in', + ' *" up "*)', + ' prev=""', + ' for arg in "$@"; do', + ' if [ "$prev" = "-f" ]; then', + ' case "$arg" in', + ' *compose.env.override.yml) cp "$arg" "$STUB_CAPTURE/override-at-up.yml" ;;', + " esac", + " fi", + ' prev="$arg"', + " done", + // Delivery evidence only: the synthetic values Compose would interpolate. + ' env > "$STUB_CAPTURE/compose-process-env.txt"', + ' if [ "${STUB_BLOCK_UP:-}" = "1" ]; then', + ' parent="$PPID"', + ' : > "$STUB_CAPTURE/up-started"', + ' while kill -0 "$parent" 2>/dev/null; do sleep 0.1; done', + " exit 143", + " fi", + ' if [ "${STUB_FAIL_UP:-}" = "1" ]; then exit 42; fi', + " exit 0", + " ;;", + ' *" ps "*)', + ' printf "%s\\n" \'{"Service":"web","State":"running","ExitCode":0}\' \'{"Service":"worker","State":"running","ExitCode":0}\'', + " exit 0", + " ;;", + " *) exit 0 ;;", + " esac", + "fi", + "exit 0", + "", +].join("\n"); + +beforeEach(async () => { + root = await realpath(await mkdtemp(join(tmpdir(), "hack-env-at-rest-"))); + projectRoot = join(root, "project"); + capture = join(root, "capture"); + const hackDir = join(projectRoot, ".hack"); + overridePath = join(hackDir, ".internal", "compose.env.override.yml"); + await mkdir(hackDir, { recursive: true }); + await mkdir(capture); + await mkdir(join(root, "bin")); + await writeFile(join(root, "bin", "docker"), STUB); + await chmod(join(root, "bin", "docker"), 0o755); + await writeFile( + join(hackDir, "hack.config.json"), + `${JSON.stringify({ + name: "envatrest", + dev_host: "envatrest.hack", + internal: { dns: false, tls: false }, + logs: { follow_backend: "compose", snapshot_backend: "compose" }, + })}\n` + ); + await writeFile( + join(hackDir, "docker-compose.yml"), + [ + "services:", + " web:", + " image: alpine:3.22", + " worker:", + " image: alpine:3.22", + "", + ].join("\n") + ); + await writeFile(join(projectRoot, PROJECT_ENV_KEY_FILENAME), "synthetic-key"); + await setProjectEnvValue({ + projectRoot, + projectDir: hackDir, + envName: null, + scope: "global", + key: "SECRET_TOKEN", + value: SECRET_VALUE, + secret: true, + }); + await setProjectEnvValue({ + projectRoot, + projectDir: hackDir, + envName: null, + scope: "web", + key: "WEB_ONLY", + value: PLAIN_VALUE, + secret: false, + }); +}); + +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +function cliEnv(extra?: Readonly>) { + return privateCliEnv({ + root, + env: { + // Stub docker plus base system tools only: no host mutagen or brew. + PATH: `${join(root, "bin")}:/usr/bin:/bin`, + STUB_CAPTURE: capture, + HACK_NO_INTERACTIVE: "1", + ...extra, + }, + }); +} + +async function runCli(opts: { + readonly args: readonly string[]; + readonly env?: Readonly>; +}): Promise<{ exitCode: number; stdout: string; stderr: string }> { + const child = Bun.spawn([process.execPath, CLI, ...opts.args], { + cwd: projectRoot, + env: cliEnv(opts.env), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + const [exitCode, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]); + return { exitCode, stdout, stderr }; +} + +/** Seeds the literal file an earlier Hack version left behind. */ +async function seedEarlierVersionPlaintext(): Promise { + await mkdir(join(projectRoot, ".hack", ".internal"), { recursive: true }); + const text = renderManagedComposeEnvOverride({ + targetServices: ["web", "worker"], + globalEnv: { SECRET_TOKEN: SECRET_VALUE }, + serviceEnv: { + web: { SECRET_TOKEN: SECRET_VALUE, WEB_ONLY: PLAIN_VALUE }, + worker: { SECRET_TOKEN: SECRET_VALUE }, + }, + }); + await writeFile(overridePath, text ?? "", { mode: 0o600 }); + expect(await readFile(overridePath, "utf8")).toContain(CANARY); +} + +/** No canary at rest: the override is absent or value-free, 0600 and ours. */ +async function expectNoPlaintextAtRest(): Promise { + const info = await lstat(overridePath).catch(() => null); + if (info === null) { + return; + } + expect(info.isFile()).toBe(true); + expect(info.mode & 0o777).toBe(0o600); + expect(info.uid).toBe(process.getuid?.() ?? info.uid); + const text = await readFile(overridePath, "utf8"); + expect(text).not.toContain(CANARY); + expect(text).toContain("${HACK_ENV_"); +} + +function expectNoCanary(result: { + readonly stdout: string; + readonly stderr: string; +}): void { + expect(result.stdout).not.toContain(CANARY); + expect(result.stderr).not.toContain(CANARY); +} + +/** + * Fixture files holding a canary, except the stub's delivery evidence. The + * authored env config holds the non-secret value by design (the secret is + * stored encrypted there), so only the secret is searched in it. + */ +async function filesHoldingCanary(dir: string): Promise { + const authored = join(projectRoot, ".hack", "hack.env.default.yaml"); + const hits: string[] = []; + for (const entry of await readdir(dir, { withFileTypes: true })) { + const path = join(dir, entry.name); + if (entry.isDirectory()) { + hits.push(...(await filesHoldingCanary(path))); + continue; + } + if (!entry.isFile() || path === join(capture, "compose-process-env.txt")) { + continue; + } + const text = await readFile(path, "utf8").catch(() => ""); + if (text.includes(path === authored ? SECRET_VALUE : CANARY)) { + hits.push(path.slice(root.length + 1)); + } + } + return hits; +} + +test("up delivers values through the Compose process env and keeps only references at rest; down keeps nothing literal", async () => { + const up = await runCli({ args: ["up", "--detach", "--json"] }); + expectNoCanary(up); + expect(up.exitCode).toBe(0); + + const atUp = await readFile(join(capture, "override-at-up.yml"), "utf8"); + expect(atUp).not.toContain(CANARY); + expect(atUp).toMatch( + /SECRET_TOKEN: "\$\{HACK_ENV_[0-9A-F]{12}_SECRET_TOKEN\}"/ + ); + expect(atUp).toMatch(/WEB_ONLY: "\$\{HACK_ENV_[0-9A-F]{12}_WEB_ONLY\}"/); + const delivered = await readFile( + join(capture, "compose-process-env.txt"), + "utf8" + ); + expect(delivered).toMatch( + new RegExp(`^HACK_ENV_[0-9A-F]{12}_SECRET_TOKEN=${SECRET_VALUE}$`, "m") + ); + expect(delivered).toMatch( + new RegExp(`^HACK_ENV_[0-9A-F]{12}_WEB_ONLY=${PLAIN_VALUE}$`, "m") + ); + await expectNoPlaintextAtRest(); + expect(await lstat(overridePath).then((info) => info.isFile())).toBe(true); + + const down = await runCli({ args: ["down", "--json"] }); + expectNoCanary(down); + expect(down.exitCode).toBe(0); + await expectNoPlaintextAtRest(); + + const args = await readFile(join(capture, "docker-args.log"), "utf8"); + expect(args).not.toContain(CANARY); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("down removes a literal override left by an earlier version", async () => { + await seedEarlierVersionPlaintext(); + const down = await runCli({ args: ["down", "--json"] }); + expectNoCanary(down); + expect(down.exitCode).toBe(0); + expect(await lstat(overridePath).catch(() => null)).toBeNull(); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("down removes a literal override even when its down-before hook fails", async () => { + const configPath = join(projectRoot, ".hack", "hack.config.json"); + const config = JSON.parse(await readFile(configPath, "utf8")); + await writeFile( + configPath, + `${JSON.stringify({ ...config, lifecycle: { down: { before: ["exit 17"] } } })}\n` + ); + await seedEarlierVersionPlaintext(); + const down = await runCli({ args: ["down", "--json"] }); + expectNoCanary(down); + expect(down.exitCode).toBe(17); + expect(await lstat(overridePath).catch(() => null)).toBeNull(); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("up that renders no override removes a literal one (env config without values)", async () => { + const hackDir = join(projectRoot, ".hack"); + for (const [scope, key] of [ + ["global", "SECRET_TOKEN"], + ["web", "WEB_ONLY"], + ] as const) { + await unsetProjectEnvValue({ + projectRoot, + projectDir: hackDir, + envName: null, + scope, + key, + }); + } + await seedEarlierVersionPlaintext(); + const up = await runCli({ args: ["up", "--detach", "--json"] }); + expectNoCanary(up); + expect(up.exitCode).toBe(0); + expect(await lstat(overridePath).catch(() => null)).toBeNull(); + expect( + await readFile(join(capture, "docker-args.log"), "utf8") + ).not.toContain("compose.env.override.yml"); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("up that renders no override removes a literal one (no env config)", async () => { + await rm(join(projectRoot, ".hack", "hack.env.default.yaml")); + await seedEarlierVersionPlaintext(); + const up = await runCli({ args: ["up", "--detach", "--json"] }); + expectNoCanary(up); + expect(up.exitCode).toBe(0); + expect(await lstat(overridePath).catch(() => null)).toBeNull(); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("up that renders no override removes a literal one (legacy contract, no value for these services)", async () => { + // Legacy `hack.env.json` path: the contract has a var, but only for a + // service this stack does not run, so no service gets an override. + const hackDir = join(projectRoot, ".hack"); + await rm(join(hackDir, "hack.env.default.yaml")); + await writeFile( + join(hackDir, "hack.env.json"), + `${JSON.stringify({ + version: 1, + vars: [{ key: "OTHER_ONLY", source: "plain_env", services: ["other"] }], + })}\n` + ); + await writeFile(join(hackDir, ".env"), "OTHER_ONLY=synthetic-other\n", { + mode: 0o600, + }); + await seedEarlierVersionPlaintext(); + const up = await runCli({ args: ["up", "--detach", "--json"] }); + expectNoCanary(up); + expect(up.exitCode).toBe(0); + expect(await lstat(overridePath).catch(() => null)).toBeNull(); + expect( + await readFile(join(capture, "docker-args.log"), "utf8") + ).not.toContain("compose.env.override.yml"); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("a failed up leaves no literal override, even over an earlier version's file", async () => { + await seedEarlierVersionPlaintext(); + const up = await runCli({ + args: ["up", "--detach", "--json"], + env: { STUB_FAIL_UP: "1" }, + }); + expectNoCanary(up); + expect(up.exitCode).not.toBe(0); + expect( + await readFile(join(capture, "override-at-up.yml"), "utf8") + ).not.toContain(CANARY); + await expectNoPlaintextAtRest(); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("an up interrupted by SIGTERM during Compose startup leaves no literal override", async () => { + await seedEarlierVersionPlaintext(); + const child = Bun.spawn([process.execPath, CLI, "up", "--detach", "--json"], { + cwd: projectRoot, + env: cliEnv({ STUB_BLOCK_UP: "1" }), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + const started = join(capture, "up-started"); + const deadline = Date.now() + 30_000; + while ( + (await lstat(started).catch(() => null)) === null && + Date.now() < deadline + ) { + await Bun.sleep(50); + } + expect(await lstat(started).catch(() => null)).not.toBeNull(); + // Compose is reading the override right now: it must already be value-free. + await expectNoPlaintextAtRest(); + child.kill("SIGTERM"); + const [exitCode, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]); + expectNoCanary({ stdout, stderr }); + expect(exitCode).not.toBe(0); + await expectNoPlaintextAtRest(); + expect(await filesHoldingCanary(root)).toEqual([]); +}, 60_000); + +test("doctor reports a literal override by counts only and leaves removal to --fix", async () => { + await seedEarlierVersionPlaintext(); + const doctor = await runCli({ args: ["doctor", "--json"] }); + expectNoCanary(doctor); + const report = JSON.parse(doctor.stdout) as { + readonly data?: { + readonly checks?: readonly { + readonly id: string; + readonly status: string; + readonly detail: string; + }[]; + }; + }; + const check = report.data?.checks?.find( + (entry) => entry.id === "compose env at rest" + ); + expect(check?.status).toBe("warn"); + expect(check?.detail).toContain( + ".hack/.internal/compose.env.override.yml holds 3 literal env value(s) for 2 service(s)" + ); + expect(check?.detail).toContain("hack doctor --fix"); + expect(await readFile(overridePath, "utf8")).toContain(CANARY); +}, 120_000); diff --git a/tests/compose-env-at-rest.test.ts b/tests/compose-env-at-rest.test.ts new file mode 100644 index 00000000..ab76f122 --- /dev/null +++ b/tests/compose-env-at-rest.test.ts @@ -0,0 +1,501 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { YAML } from "bun"; +import { buildDoctorRemediationPlanLines } from "../src/commands/doctor.ts"; +import { + composeEnvAtRestRepairs, + composeEnvOverridePath, + describeComposeEnvAtRest, + inspectComposeEnvAtRest, + inspectComposeEnvOverrideAtRest, + removePlaintextComposeEnvOverride, + writeComposeEnvOverride, +} from "../src/lib/compose-env-at-rest.ts"; +import { + classifyComposeEnvOverrideText, + composeEnvReferenceName, + renderComposeEnvReferenceOverride, + renderManagedComposeEnvOverride, +} from "../src/lib/compose-managed-env.ts"; + +const CANARY = "g5-unit-canary"; +let root: string; +let projectDir: string; + +beforeEach(async () => { + root = await realpath(await mkdtemp(join(tmpdir(), "hack-env-at-rest-"))); + projectDir = join(root, ".hack"); + await mkdir(projectDir); +}); + +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +const serviceEnv = { + web: { SECRET: `${CANARY}-web`, DOLLAR: `${CANARY}-$HOME-$$`, EMPTY: "" }, + worker: { SECRET: `${CANARY}-worker` }, +}; + +test("reference projection keeps selection and order, holds no value and maps every reference", () => { + const rendered = renderComposeEnvReferenceOverride({ + targetServices: ["worker", "web", "fallback"], + globalEnv: { GLOBAL: `${CANARY}-global` }, + serviceEnv, + }); + if (!rendered) { + throw new Error("expected a reference projection"); + } + expect(rendered.text).not.toContain(CANARY); + const parsed = YAML.parse(rendered.text) as { + services: Record }>; + }; + expect(Object.keys(parsed.services)).toEqual(["worker", "web", "fallback"]); + expect(Object.keys(parsed.services.web?.environment ?? {})).toEqual([ + "SECRET", + "DOLLAR", + "EMPTY", + ]); + for (const [service, values] of Object.entries({ + ...serviceEnv, + fallback: { GLOBAL: `${CANARY}-global` }, + })) { + for (const [key, value] of Object.entries(values)) { + const name = composeEnvReferenceName({ service, key }); + expect(name).toMatch(/^HACK_ENV_[0-9A-F]{12}_[A-Z_]+$/); + expect(parsed.services[service]?.environment[key]).toBe(`\${${name}}`); + // The literal value, `$` included, travels only through the env map. + expect(rendered.env[name]).toBe(value); + } + } + expect(classifyComposeEnvOverrideText(rendered.text)).toEqual({ + kind: "references", + }); +}); + +test("reference text does not depend on values and names do not collide across services", () => { + const first = renderComposeEnvReferenceOverride({ + targetServices: ["web", "worker"], + globalEnv: {}, + serviceEnv, + }); + const second = renderComposeEnvReferenceOverride({ + targetServices: ["web", "worker"], + globalEnv: {}, + serviceEnv: { + web: { SECRET: "other", DOLLAR: "other", EMPTY: "x" }, + worker: { SECRET: "other" }, + }, + }); + expect(first?.text).toBe(second?.text ?? ""); + expect(composeEnvReferenceName({ service: "web", key: "SECRET" })).not.toBe( + composeEnvReferenceName({ service: "worker", key: "SECRET" }) + ); + expect(composeEnvReferenceName({ service: "web", key: "a.b" })).toMatch( + /^HACK_ENV_[0-9A-F]{12}$/ + ); + expect( + renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv: {}, + }) + ).toBeNull(); +}); + +test("classifier counts literal values without returning them", () => { + const literal = renderManagedComposeEnvOverride({ + targetServices: ["web", "worker"], + globalEnv: {}, + serviceEnv, + }); + const result = classifyComposeEnvOverrideText(literal ?? ""); + expect(result).toEqual({ kind: "plaintext", services: 2, values: 4 }); + expect(JSON.stringify(result)).not.toContain(CANARY); + expect( + classifyComposeEnvOverrideText( + "services:\n web:\n environment:\n - A=${A}\n - B\n - C=literal\n" + ) + ).toEqual({ kind: "plaintext", services: 1, values: 1 }); + expect( + classifyComposeEnvOverrideText( + 'services:\n web:\n environment:\n A: "${A:-fallback}"\n B: 42\n C: null\n' + ) + ).toEqual({ kind: "plaintext", services: 1, values: 2 }); + expect(classifyComposeEnvOverrideText("services: [\n")).toEqual({ + kind: "unverifiable", + }); + expect(classifyComposeEnvOverrideText("services:\n web: 3\n")).toEqual({ + kind: "unverifiable", + }); + expect(classifyComposeEnvOverrideText("")).toEqual({ kind: "references" }); +}); + +test("writer leaves a value-free 0600 file and tightens an existing loose mode", async () => { + const rendered = renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }); + const path = composeEnvOverridePath({ projectDir }); + await mkdir(join(projectDir, ".internal")); + await writeFile(path, rendered?.text ?? ""); + await chmod(path, 0o644); + expect( + await writeComposeEnvOverride({ projectDir, text: rendered?.text ?? "" }) + ).toBe(path); + const info = await lstat(path); + expect(info.mode & 0o777).toBe(0o600); + expect(info.uid).toBe(process.getuid?.() ?? info.uid); + expect(await readFile(path, "utf8")).toBe(rendered?.text ?? ""); +}); + +test("writer refuses literal text and a symlinked path without writing or naming a value", async () => { + const literal = renderManagedComposeEnvOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }); + const refusal = await writeComposeEnvOverride({ + projectDir, + text: literal ?? "", + }).catch((error: unknown) => error); + expect(refusal).toBeInstanceOf(Error); + expect(String(refusal)).not.toContain(CANARY); + expect( + await lstat(composeEnvOverridePath({ projectDir })).catch(() => null) + ).toBeNull(); + + const target = join(root, "elsewhere.yml"); + await writeFile(target, "outside\n"); + await mkdir(join(projectDir, ".internal"), { recursive: true }); + await symlink(target, composeEnvOverridePath({ projectDir })); + const rendered = renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }); + await expect( + writeComposeEnvOverride({ projectDir, text: rendered?.text ?? "" }) + ).rejects.toThrow("values omitted"); + expect(await readFile(target, "utf8")).toBe("outside\n"); +}); + +test("inspection and removal act only on literal or unverifiable overrides", async () => { + expect(await inspectComposeEnvOverrideAtRest({ projectDir })).toEqual({ + kind: "absent", + path: composeEnvOverridePath({ projectDir }), + }); + const path = composeEnvOverridePath({ projectDir }); + await mkdir(join(projectDir, ".internal")); + + const rendered = renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }); + await writeComposeEnvOverride({ projectDir, text: rendered?.text ?? "" }); + const kept = await removePlaintextComposeEnvOverride({ + projectDir, + includeUnverifiable: true, + }); + expect(kept.removed).toBe(false); + expect(kept.state.kind).toBe("references"); + expect(await lstat(path).catch(() => null)).not.toBeNull(); + + await writeFile( + path, + renderManagedComposeEnvOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }) ?? "" + ); + const report = await inspectComposeEnvAtRest({ projectDir }); + expect(report.adoptionBound).toBe(false); + expect(composeEnvAtRestRepairs(report)).toEqual({ + override: true, + asides: false, + }); + const described = describeComposeEnvAtRest({ + report, + projectDirName: ".hack", + }); + expect(described.status).toBe("warn"); + expect(described.message).toContain( + ".hack/.internal/compose.env.override.yml holds 3 literal env value(s) for 1 service(s)" + ); + expect(described.message).not.toContain(CANARY); + const removed = await removePlaintextComposeEnvOverride({ projectDir }); + expect(removed.removed).toBe(true); + expect(await lstat(path).catch(() => null)).toBeNull(); + + // A symlink is never followed: it is unverifiable, removed only on request, + // and its target is untouched. + const target = join(root, "target.yml"); + await writeFile(target, `A: ${CANARY}\n`); + await symlink(target, path); + const linked = await removePlaintextComposeEnvOverride({ projectDir }); + expect(linked).toEqual({ + removed: false, + state: { kind: "unverifiable", path }, + adoptionBound: false, + asidesRemoved: 0, + }); + expect( + ( + await removePlaintextComposeEnvOverride({ + projectDir, + includeUnverifiable: true, + }) + ).removed + ).toBe(true); + expect(await lstat(path).catch(() => null)).toBeNull(); + expect(await readFile(target, "utf8")).toContain(CANARY); +}); + +test("removal keeps a value-free override that replaced the inspected literal file", async () => { + const path = composeEnvOverridePath({ projectDir }); + await mkdir(join(projectDir, ".internal")); + const projection = { targetServices: ["web"], globalEnv: {}, serviceEnv }; + await writeFile(path, renderManagedComposeEnvOverride(projection) ?? ""); + const references = renderComposeEnvReferenceOverride(projection)?.text ?? ""; + // A concurrent Compose command rewrites the override after the scrub saw + // the literal file and before it removes anything. + const raced = await removePlaintextComposeEnvOverride({ + projectDir, + afterInspect: async () => { + await writeComposeEnvOverride({ projectDir, text: references }); + }, + }); + expect(raced.removed).toBe(false); + expect(raced.state.kind).toBe("references"); + expect(await readFile(path, "utf8")).toBe(references); + expect((await lstat(path)).mode & 0o777).toBe(0o600); + expect( + (await readdir(join(projectDir, ".internal"))).filter((name) => + name.endsWith(".remove") + ) + ).toEqual([]); + expect(JSON.stringify(raced)).not.toContain(CANARY); +}); + +test("removal sweeps an aside left by a crashed removal once it is stale", async () => { + const internal = join(projectDir, ".internal"); + await mkdir(internal); + const literal = + renderManagedComposeEnvOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }) ?? ""; + // A removal that crashed between its move and its unlink left this behind. + const aside = + ".compose.env.override.yml.0f8c2a4e-1b3d-4c5e-8f9a-0b1c2d3e4f5a.remove"; + await writeFile(join(internal, aside), literal, { mode: 0o600 }); + // Not an aside of this module: wrong suffix shape, or not a regular file. + const unrelated = ".compose.env.override.yml.backup.remove"; + await writeFile(join(internal, unrelated), "kept\n"); + const directory = + ".compose.env.override.yml.11111111-2222-3333-4444-555555555555.remove"; + await mkdir(join(internal, directory)); + + // A fresh aside may still belong to a concurrent removal: it is kept. + const fresh = await removePlaintextComposeEnvOverride({ projectDir }); + expect(fresh.state.kind).toBe("absent"); + expect((await readdir(internal)).sort()).toEqual( + [aside, directory, unrelated].sort() + ); + + const stale = await removePlaintextComposeEnvOverride({ + projectDir, + now: () => Date.now() + 61_000, + }); + expect(stale).toEqual({ + removed: false, + state: { kind: "absent", path: composeEnvOverridePath({ projectDir }) }, + adoptionBound: false, + asidesRemoved: 1, + }); + expect((await readdir(internal)).sort()).toEqual( + [directory, unrelated].sort() + ); +}); + +test("doctor --fix plan offers removal only while a literal override is at rest", async () => { + await writeFile( + join(projectDir, "docker-compose.yml"), + "services:\n web:\n image: alpine:3.22\n" + ); + const step = ".hack/.internal/compose.env.override.yml, which may hold"; + const plan = async () => + ( + await buildDoctorRemediationPlanLines({ + startDir: root, + migrateEnvConfig: false, + }) + ).join("\n"); + expect(await plan()).not.toContain(step); + await mkdir(join(projectDir, ".internal")); + await writeFile( + composeEnvOverridePath({ projectDir }), + renderManagedComposeEnvOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }) ?? "" + ); + const withLiteral = await plan(); + expect(withLiteral).toContain(step); + expect(withLiteral).not.toContain(CANARY); +}); + +test("doctor descriptions are ok for absent and reference-only files", () => { + const path = "/unused"; + const describe = ( + override: Parameters< + typeof describeComposeEnvAtRest + >[0]["report"]["override"] + ) => + describeComposeEnvAtRest({ + report: { + override, + adoptionBound: false, + asides: { stale: 0, fresh: 0 }, + }, + projectDirName: ".hack", + }); + expect(describe({ kind: "absent", path }).status).toBe("ok"); + expect(describe({ kind: "references", path }).status).toBe("ok"); + expect(describe({ kind: "unverifiable", path }).message).toContain( + "hack doctor --fix" + ); +}); + +const ASIDE = + ".compose.env.override.yml.0f8c2a4e-1b3d-4c5e-8f9a-0b1c2d3e4f5a.remove"; + +function literalText(): string { + return ( + renderManagedComposeEnvOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + }) ?? "" + ); +} + +test("a rendering write sweeps stale removal asides and keeps fresh ones", async () => { + const internal = join(projectDir, ".internal"); + await mkdir(internal); + await writeFile(join(internal, ASIDE), literalText(), { mode: 0o600 }); + const text = + renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + })?.text ?? ""; + // A fresh aside may still belong to a removal in progress. + await writeComposeEnvOverride({ projectDir, text }); + expect(await readdir(internal)).toContain(ASIDE); + await writeComposeEnvOverride({ + projectDir, + text, + now: () => Date.now() + 61_000, + }); + expect(await readdir(internal)).toEqual(["compose.env.override.yml"]); +}); + +test("doctor reports leftover removal asides by count and --fix removes only stale ones", async () => { + await writeFile( + join(projectDir, "docker-compose.yml"), + "services:\n web:\n image: alpine:3.22\n" + ); + const internal = join(projectDir, ".internal"); + await mkdir(internal); + await writeFile(join(internal, ASIDE), literalText(), { mode: 0o600 }); + + const fresh = await inspectComposeEnvAtRest({ projectDir }); + expect(fresh.override.kind).toBe("absent"); + expect(fresh.asides).toEqual({ stale: 0, fresh: 1 }); + expect(composeEnvAtRestRepairs(fresh)).toEqual({ + override: false, + asides: false, + }); + const described = describeComposeEnvAtRest({ + report: fresh, + projectDirName: ".hack", + }); + expect(described.status).toBe("warn"); + expect(described.message).toContain("1 leftover removal file(s)"); + expect(described.message).not.toContain(CANARY); + expect(described.message).not.toContain(ASIDE); + + const stale = await inspectComposeEnvAtRest({ + projectDir, + now: () => Date.now() + 61_000, + }); + expect(stale.asides).toEqual({ stale: 1, fresh: 0 }); + expect(composeEnvAtRestRepairs(stale).asides).toBe(true); + const removed = await removePlaintextComposeEnvOverride({ + projectDir, + includeUnverifiable: true, + now: () => Date.now() + 61_000, + }); + expect(removed.asidesRemoved).toBe(1); + expect(await readdir(internal)).toEqual([]); + expect( + describeComposeEnvAtRest({ + report: await inspectComposeEnvAtRest({ projectDir }), + projectDirName: ".hack", + }).status + ).toBe("ok"); +}); + +test("an override that an adoption state may pin is kept by removal and reported by doctor", async () => { + const internal = join(projectDir, ".internal"); + await mkdir(internal); + const path = composeEnvOverridePath({ projectDir }); + await writeFile(path, literalText(), { mode: 0o600 }); + // Adoption state Hack cannot read safely counts as pinning the override. + const adoption = join(internal, "legacy-compose-adoption-v1"); + await mkdir(adoption); + + const kept = await removePlaintextComposeEnvOverride({ + projectDir, + includeUnverifiable: true, + }); + expect(kept.removed).toBe(false); + expect(kept.adoptionBound).toBe(true); + expect(await readFile(path, "utf8")).toBe(literalText()); + const report = await inspectComposeEnvAtRest({ projectDir }); + expect(report.adoptionBound).toBe(true); + expect(composeEnvAtRestRepairs(report).override).toBe(false); + const described = describeComposeEnvAtRest({ + report, + projectDirName: ".hack", + }); + expect(described.status).toBe("warn"); + expect(described.message).toContain("pins this file"); + expect(described.message).toContain("hack config adopt --rollback"); + expect(described.message).not.toContain(CANARY); + + // Without the adoption state the same file is removed. + await rm(adoption, { recursive: true }); + const removed = await removePlaintextComposeEnvOverride({ projectDir }); + expect(removed).toMatchObject({ removed: true, adoptionBound: false }); + expect(await lstat(path).catch(() => null)).toBeNull(); +}); diff --git a/tests/compose-env-reference-least-privilege.test.ts b/tests/compose-env-reference-least-privilege.test.ts new file mode 100644 index 00000000..7c6da084 --- /dev/null +++ b/tests/compose-env-reference-least-privilege.test.ts @@ -0,0 +1,118 @@ +import { afterEach, expect, test } from "bun:test"; +import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { withDependencyStartupProgress } from "../src/lib/dependency-startup-progress.ts"; +import { canReuseRunningDependencyCache } from "../src/lib/run-dependency-cache.ts"; + +/** + * HACK-1273: the `HACK_ENV_*` values that the generated Compose env override + * references belong to `docker compose` invocations that read that file. The + * other docker children Hack starts around Compose must not receive them. + */ +const CANARY = "g5-least-privilege-canary"; +const ID = "c".repeat(64); +const dirs: string[] = []; + +afterEach(async () => { + for (const dir of dirs.splice(0)) { + await rm(dir, { recursive: true, force: true }); + } +}); + +type Recorded = { + readonly args: readonly string[]; + readonly env: Readonly>; +}; + +/** A `docker` stand-in that records each invocation's argv and environment. */ +async function recordingDocker(): Promise<{ + readonly dir: string; + readonly calls: () => Promise; +}> { + const dir = await mkdtemp(join(tmpdir(), "hack-env-least-privilege-")); + dirs.push(dir); + const record = join(dir, "calls.jsonl"); + const docker = join(dir, "docker"); + await writeFile( + docker, + `#!${process.execPath} +import {appendFileSync} from 'node:fs'; +appendFileSync(${JSON.stringify(record)}, JSON.stringify({args: process.argv.slice(2), env: process.env}) + '\\n'); +if (process.argv[2] === 'inspect' && process.argv.includes(${JSON.stringify(ID)})) { + console.log(JSON.stringify({id: ${JSON.stringify(ID)}, running: true, project: 'fixture', service: 'deps', mounts: [{Type: 'volume', Name: 'cache'}]})); +} +process.exit(0); +` + ); + await chmod(docker, 0o700); + return { + dir, + calls: async () => + (await readFile(record, "utf8").catch(() => "")) + .split("\n") + .filter(Boolean) + .map((line) => JSON.parse(line) as Recorded), + }; +} + +function composeEnv(dir: string): Record { + return { + PATH: `${dir}:/usr/bin:/bin`, + GLOBAL_FLAG: "kept", + HACK_ENV_0123456789AB_SECRET: CANARY, + HACK_ENV_0123456789AB: CANARY, + }; +} + +function expectNoReferenceValues(calls: readonly Recorded[]): void { + expect(calls.length).toBeGreaterThan(0); + for (const call of calls) { + expect(call.env.GLOBAL_FLAG).toBe("kept"); + expect( + Object.keys(call.env).filter((key) => key.startsWith("HACK_ENV_")) + ).toEqual([]); + expect(JSON.stringify(call.env)).not.toContain(CANARY); + } +} + +test("dependency progress observers get the Compose env without its HACK_ENV_ reference values", async () => { + const docker = await recordingDocker(); + const result = await withDependencyStartupProgress({ + services: ["deps"], + project: "fixture", + cwd: docker.dir, + env: composeEnv(docker.dir), + onPhase: () => undefined, + run: async () => { + for (let attempt = 0; attempt < 200; attempt += 1) { + if ((await docker.calls()).length > 0) { + break; + } + await Bun.sleep(25); + } + return 7; + }, + }); + expect(result).toBe(7); + const calls = await docker.calls(); + expect(calls.some((call) => call.args[0] === "events")).toBe(true); + expectNoReferenceValues(calls); +}); + +test("the dependency cache reuse probe gets the Compose env without its HACK_ENV_ reference values", async () => { + const docker = await recordingDocker(); + expect( + await canReuseRunningDependencyCache({ + containerId: ID, + composeProject: "fixture", + service: "deps", + volumeNames: ["cache"], + cwd: docker.dir, + env: composeEnv(docker.dir), + }) + ).toBe(true); + const calls = await docker.calls(); + expect(calls.map((call) => call.args[0])).toEqual(["inspect"]); + expectNoReferenceValues(calls); +}); diff --git a/tests/doctor-fix-noninteractive.test.ts b/tests/doctor-fix-noninteractive.test.ts index 5a102808..37024d9b 100644 --- a/tests/doctor-fix-noninteractive.test.ts +++ b/tests/doctor-fix-noninteractive.test.ts @@ -6,7 +6,14 @@ import { expect, test, } from "bun:test"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { + lstat, + mkdir, + mkdtemp, + readFile, + rm, + writeFile, +} from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; @@ -15,6 +22,10 @@ import { GLOBAL_DAEMON_PID_FILENAME, GLOBAL_HACK_DIR_NAME, } from "../src/constants.ts"; +import { + renderComposeEnvReferenceOverride, + renderManagedComposeEnvOverride, +} from "../src/lib/compose-managed-env.ts"; import { resetNoInteractiveFlagForTests } from "../src/lib/interactivity.ts"; import { restoreEnv } from "./helpers/env.ts"; import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts"; @@ -246,3 +257,38 @@ test("doctor --fix never calls the raw clack confirm prompt under HACK_NO_INTERA // non-interactive branch instead of the raw clack prompt. expect(code).toBe(0); }); + +test("doctor --fix removes a literal Compose env override and keeps a reference-only one (HACK-1273)", async () => { + const projectRoot = await createProjectWithStaleDaemonPid(); + const canary = "doctor-fix-synthetic-canary"; + const projection = { + targetServices: ["api"], + globalEnv: {}, + serviceEnv: { api: { TOKEN: canary } }, + }; + const internal = resolve(projectRoot, ".hack", ".internal"); + const path = resolve(internal, "compose.env.override.yml"); + await mkdir(internal, { recursive: true }); + await writeFile(path, renderManagedComposeEnvOverride(projection) ?? "", { + mode: 0o600, + }); + + const { runCli } = await import("../src/cli/run.ts"); + const fix = () => + runCli(["doctor", "--fix", "--no-interactive", "--path", projectRoot]); + expect(await fix()).toBe(0); + expect(await lstat(path).catch(() => null)).toBeNull(); + expect(noteCalls.join("\n")).toContain( + "Removed .hack/.internal/compose.env.override.yml (values omitted)." + ); + + const references = renderComposeEnvReferenceOverride(projection)?.text ?? ""; + await writeFile(path, references, { mode: 0o600 }); + noteCalls.length = 0; + expect(await fix()).toBe(0); + expect(await readFile(path, "utf8")).toBe(references); + for (const message of noteCalls) { + expect(message).not.toContain(canary); + } + // Two full doctor --fix runs; the default 5 s budget is too tight on CI. +}, 30_000); diff --git a/tests/e2e/scenarios/native-compose-adoption-managed-inputs.ts b/tests/e2e/scenarios/native-compose-adoption-managed-inputs.ts index 452dc283..edd580ab 100644 --- a/tests/e2e/scenarios/native-compose-adoption-managed-inputs.ts +++ b/tests/e2e/scenarios/native-compose-adoption-managed-inputs.ts @@ -4,7 +4,7 @@ import { resolveModernComposeEnvOverrides, resolveRuntimeHostMetadataOverride, } from "../../../src/commands/project.ts"; -import { renderManagedComposeEnvOverride } from "../../../src/lib/compose-managed-env.ts"; +import { renderComposeEnvReferenceOverride } from "../../../src/lib/compose-managed-env.ts"; import { defaultProjectSlugFromPath, findProjectContextAtRoot, @@ -31,6 +31,8 @@ const LAYERS = [ "hack.env.qa.local.yaml", ] as const; const NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +/** Synthetic Compose process env per fixture root, as Hack supplies it. */ +const composeEnvByRoot = new Map>>(); function refuse(): never { throw new Error( "Adoption managed fixture source check failed; values omitted." @@ -48,6 +50,22 @@ export function managedAdoptionFixtureComposeFiles( ]); } +/** + * The Compose process env Hack supplies with the value-free env override + * (HACK-1273): the override holds only `${HACK_ENV_*}` references, so a raw + * `docker compose --file` start of the original stack needs these values or + * every managed variable reaches the containers blank. Synthetic values only. + */ +export function managedAdoptionFixtureComposeEnv( + root: string +): Readonly> { + const env = composeEnvByRoot.get(root); + if (!env) { + refuse(); + } + return env; +} + /** Direct existing writers use the same isolated home/policy as the later compiled CLI. */ async function withFixtureHome(opts: { readonly hackHome: string; @@ -241,17 +259,23 @@ async function writeGenerated(instance: Checkout) { refuse(); } const expectedRuntime = buildRuntimeHostMetadataOverride(runtimeOpts); - const expectedEnv = renderManagedComposeEnvOverride({ + // The writer keeps only `${NAME}` references at rest (HACK-1273). + const expectedEnv = renderComposeEnvReferenceOverride({ targetServices: ["db", "worker"], globalEnv: managed.env, serviceEnv: managed.preflightEnvByService, }); if ( + !expectedEnv || (await readFile(runtime, "utf8")) !== expectedRuntime || - (await readFile(files[2] ?? "", "utf8")) !== expectedEnv + (await readFile(files[2] ?? "", "utf8")) !== expectedEnv.text || + Object.entries(expectedEnv.env).some( + ([name, value]) => managed.env[name] !== value + ) ) { refuse(); } + composeEnvByRoot.set(instance.root, Object.freeze({ ...managed.env })); } /** Private byte/inode oracle covers selected raw layers, shared primary locals and original generated files throughout adoption. */ diff --git a/tests/e2e/scenarios/native-compose-adoption-worktrees.ts b/tests/e2e/scenarios/native-compose-adoption-worktrees.ts index 019ea119..c0bc1c5a 100644 --- a/tests/e2e/scenarios/native-compose-adoption-worktrees.ts +++ b/tests/e2e/scenarios/native-compose-adoption-worktrees.ts @@ -67,6 +67,7 @@ import { } from "./native-compose-adoption-local-inputs.ts"; import { MANAGED_FIXTURE_VALUE, + managedAdoptionFixtureComposeEnv, managedAdoptionFixtureComposeFiles, managedAdoptionFixtureEnvAssertion, managedAdoptionFixtureSourceSnapshot, @@ -2329,6 +2330,11 @@ export async function bootstrapOriginal(h: FixtureRuntime, instance: Instance) { ...(instance.basicBuild ? ["--no-build"] : []), ], cwd: fixtureRoot, + // The generated env override holds references only; Hack delivers the + // values through the Compose process env, so the original start does too. + ...(instance.sourceMode === "canonical-generated" + ? { env: managedAdoptionFixtureComposeEnv(instance.root) } + : {}), timeoutMs: TIMEOUT, }); if (instance.basicBuild) { diff --git a/tests/env-secret-sentinel-scan.test.ts b/tests/env-secret-sentinel-scan.test.ts index 3a3af311..fcf2d1e0 100644 --- a/tests/env-secret-sentinel-scan.test.ts +++ b/tests/env-secret-sentinel-scan.test.ts @@ -36,13 +36,16 @@ import { * argv, tracked files, any Git object of the fixture repository, or any other * file on disk (HACK_HOME, the private HOME and its caches, generated Compose * and runtime files, host-command records and logs). The only exceptions are - * the private-by-design plaintext materializations (`.hack/.env` and - * `.hack/.internal/compose.env.override.yml`) and the key files themselves, - * each of which must be git-ignored, untracked and mode 0600. + * the private-by-design plaintext materialization `.hack/.env` and the key + * files themselves, each of which must be git-ignored, untracked and mode + * 0600. The generated `.hack/.internal/compose.env.override.yml` holds only + * `${HACK_ENV_...}` references (HACK-1273): it gets no exception, and it must + * also be private, ignored and untracked. * * Positive controls prove the scan is not vacuous: every sentinel reaches the - * exec'd command, the encrypted env file is committed, and each allowed - * plaintext artifact actually contains a sentinel. + * exec'd command and the `docker compose` client environment, the encrypted + * env file is committed, each allowed plaintext artifact actually contains a + * sentinel, and the Compose env override exists with references. * * Isolation: private HOME/HACK_HOME/Docker client config (privateCliEnv), a * `docker` shim first on PATH that only records its argv and fails, so no real @@ -404,19 +407,15 @@ async function allSecrets(): Promise> { return secrets; } -/** Plaintext is allowed only in these documented private materializations. */ -const PRIVATE_PLAINTEXT_NAMES = new Set([".env", "compose.env.override.yml"]); - +/** + * Plaintext is allowed only in the documented private materialization + * `.hack/.env`. The Compose env override is value-free since HACK-1273. + */ function isPrivatePlaintextArtifact(path: string): boolean { const { primaryRoot, linkedRoot } = requireFixture(); - return [primaryRoot, linkedRoot].some((checkout) => { - const inside = relative(join(checkout, ".hack"), path); - return ( - !inside.startsWith("..") && - PRIVATE_PLAINTEXT_NAMES.has(basename(path)) && - (inside === ".env" || inside.startsWith(`.internal${sep}`)) - ); - }); + return [primaryRoot, linkedRoot].some( + (checkout) => path === join(checkout, ".hack", ".env") + ); } function readSpawnRecords(text: string): SpawnRecord[] { @@ -595,28 +594,34 @@ test("plaintext exists on disk only in private, ignored, 0600 materializations", } expect(unexpected).toEqual([]); - // Positive control: both checkouts materialized both private artifacts. + // Positive control: both checkouts materialized the private artifact. const { primaryRoot, linkedRoot } = requireFixture(); - const expectedArtifacts = [primaryRoot, linkedRoot].flatMap((checkout) => [ - join(checkout, ".hack", ".env"), - join(checkout, ".hack", ".internal", "compose.env.override.yml"), - ]); - for (const path of expectedArtifacts) { + for (const checkout of [primaryRoot, linkedRoot]) { + const path = join(checkout, ".hack", ".env"); expect(allowed).toContain(path); expect( findHits(await readFile(path, "utf8"), valueNeedles).length ).toBeGreaterThan(0); } + // Positive control: Compose generation wrote the env override, and it holds + // references only (any value in it is in `unexpected` above). + const envOverrides = [primaryRoot, linkedRoot].map((checkout) => + join(checkout, ".hack", ".internal", "compose.env.override.yml") + ); + for (const path of envOverrides) { + expect(allowed).not.toContain(path); + expect(await readFile(path, "utf8")).toContain("${HACK_ENV_"); + } - // Every plaintext artifact and key file is private and ignored. - for (const path of [...allowed, ...keyFiles]) { + // Every plaintext artifact, env override and key file is private and ignored. + for (const path of [...allowed, ...envOverrides, ...keyFiles]) { expect({ path: relative(root, path), mode: (await lstat(path)).mode & 0o777, }).toEqual({ path: relative(root, path), mode: 0o600 }); } for (const checkout of [primaryRoot, linkedRoot]) { - const inCheckout = [...allowed, ...keyFiles] + const inCheckout = [...allowed, ...envOverrides, ...keyFiles] .filter((path) => !relative(checkout, path).startsWith("..")) .map((path) => relative(checkout, path)); const ignored = runFixtureGit({ @@ -636,6 +641,7 @@ test("secret values reach child environments only for runtime delivery", async ( const shimDir = join(root, "shim"); const unexpected: string[] = []; let execDeliveries = 0; + let composeDeliveries = 0; for (const record of records) { const argv = Array.isArray(record.argv) ? record.argv.map((part) => String(part)) @@ -659,10 +665,13 @@ test("secret values reach child environments only for runtime delivery", async ( continue; } if (basename(program) === "docker" && argv[1] === "compose") { + composeDeliveries += 1; continue; } unexpected.push(`${record.via} ${basename(program)} ${argv[1] ?? ""}`); } expect(unexpected).toEqual([]); expect(execDeliveries).toBe(2); + // The value-free override resolves only through the Compose client env. + expect(composeDeliveries).toBeGreaterThan(0); }); diff --git a/tests/native-compose-adoption-generation.test.ts b/tests/native-compose-adoption-generation.test.ts index da07a4e7..7a71067a 100644 --- a/tests/native-compose-adoption-generation.test.ts +++ b/tests/native-compose-adoption-generation.test.ts @@ -15,12 +15,22 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { checkComposeEnvOverrideAtRest } from "../src/commands/doctor.ts"; import { resolveModernComposeEnvOverrides, resolveRuntimeHostMetadataOverride, } from "../src/commands/project.ts"; import { HackCliError } from "../src/lib/cli-result.ts"; -import { renderManagedComposeEnvOverride } from "../src/lib/compose-managed-env.ts"; +import { + composeEnvAtRestRepairs, + describeComposeEnvAtRest, + inspectComposeEnvAtRest, + removePlaintextComposeEnvOverride, +} from "../src/lib/compose-env-at-rest.ts"; +import { + renderComposeEnvReferenceOverride, + renderManagedComposeEnvOverride, +} from "../src/lib/compose-managed-env.ts"; import { acquireLegacyComposeAdoptionPreparationBinding } from "../src/lib/native-compose-adoption-binding.ts"; import { tryLegacyComposeAdoptedCommand } from "../src/lib/native-compose-adoption-command.ts"; import { @@ -32,7 +42,10 @@ import { openLegacyComposeAdoptedGenerationStore, } from "../src/lib/native-compose-adoption-generation.ts"; import { inspectLegacyComposeAdoptionSelection } from "../src/lib/native-compose-adoption-marker.ts"; -import { previewLegacyComposeAdoption } from "../src/lib/native-compose-adoption-preview.ts"; +import { + LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE, + previewLegacyComposeAdoption, +} from "../src/lib/native-compose-adoption-preview.ts"; import { defaultProjectSlugFromPath, findProjectContextAtRoot, @@ -58,6 +71,28 @@ const ID = "a".repeat(64); const NETWORK = "b".repeat(64); const VOLUME = "original_private_database"; const CREATED = "2026-01-01T01:02:03Z"; +/** + * Synthetic engine config hash over every `${NAME}` reference in the selected + * Compose files, resolved from the Compose process env (blank when unset), as + * the engine does when it labels a container. The stub engine below computes + * the same digest from its own argv and environment. + */ +async function composeReferenceHash(opts: { + readonly files: readonly string[]; + readonly env: Readonly>; +}): Promise { + const names: string[] = []; + for (const file of opts.files) { + for (const match of (await readFile(file, "utf8")).matchAll( + /\$\{([A-Za-z_][A-Za-z0-9_]*)\}/g + )) { + names.push(match[1] ?? ""); + } + } + return new Bun.CryptoHasher("sha256") + .update(JSON.stringify(names.map((name) => [name, opts.env[name] ?? ""]))) + .digest("hex"); +} type Fixture = { engine: string; container: Record[]; @@ -65,6 +100,8 @@ type Fixture = { network: Record[]; mode?: string; configHash?: string; + /** Hash Compose references like the engine: with the values in the Compose env. */ + envConfigHash?: boolean; running?: boolean; mutationFailure?: boolean; states?: Record; @@ -216,7 +253,11 @@ if (kind === 'container' && ['start','restart','stop'].includes(action)) { writeFileSync(root+'/fixture.json',JSON.stringify(fixture));process.exit(fixture.mutationFailure ? 7 : 0); } if (kind === 'container' && ['exec','logs'].includes(action)) { process.exit(0); } -if (kind === "compose") { for (const container of fixture.container) console.log(container.service + ' ' + 'd'.repeat(64)); process.exit(0); } +if (kind === "compose") { + let hash = 'd'.repeat(64); + if (fixture.envConfigHash) { const names = args.flatMap((arg, index) => args[index - 1] === '--file' ? [...readFileSync(arg, 'utf8').matchAll(/\\$\\{([A-Za-z_][A-Za-z0-9_]*)\\}/g)].map((match) => match[1]) : []); hash = new Bun.CryptoHasher('sha256').update(JSON.stringify(names.map((name) => [name, process.env[name] ?? '']))).digest('hex'); } + for (const container of fixture.container) console.log(container.service + ' ' + hash); process.exit(0); +} if (!(kind === "info" && action === "--format") && (!['container','volume','network'].includes(kind) || !['ls','inspect'].includes(action) || !args.includes('--format'))) {writeFileSync(root + "/mutation", "unauthorized command");process.exit(99);} if (fixture.mode === "fail") {console.error(${JSON.stringify(CANARY)});process.exit(29);} if (fixture.mode === "malformed") {console.log(${JSON.stringify(CANARY)});process.exit(0);} @@ -1869,7 +1910,7 @@ boundedTest.skipIf(!process.env.HACK_TEST_OLD_ADOPTION_BINARY)( 30_000 ); -async function managedGenerated() { +async function managedGenerated(form: "literal" | "reference" = "literal") { const composeText = await readFile( join(projectRoot, ".hack/docker-compose.yml"), "utf8" @@ -1890,11 +1931,15 @@ async function managedGenerated() { aliasHost: null, composeProject: "fixture", }); - const env = renderManagedComposeEnvOverride({ + const projection = { targetServices: ["db"], globalEnv: { MANAGED: CANARY, EMPTY: "" }, serviceEnv: { db: { MANAGED: CANARY, EMPTY: "" } }, - }); + }; + const env = + form === "reference" + ? renderComposeEnvReferenceOverride(projection)?.text + : renderManagedComposeEnvOverride(projection); if (!(runtime && env)) { throw new Error("Synthetic projections missing"); } @@ -1959,9 +2004,14 @@ test("v3 saved adoption consumes canonical original generated sources without en const binary = await managedEnvCompilerFixture( join(root, "managed-compiler") ); + // `hack config adopt` and its preview now refuse an earlier version's + // literal override (HACK-1273); the store below models a generation that an + // earlier version prepared over it. expect( - (await previewLegacyComposeAdoption({ projectRoot, binary })).complete - ).toBe(true); + (await previewLegacyComposeAdoption({ projectRoot, binary })).fields.map( + (field) => field.code + ) + ).toEqual([LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE]); const store = await openLegacyComposeAdoptedGenerationStore({ projectRoot }); try { const generation = await store.prepare({ binary }); @@ -2020,6 +2070,240 @@ test("v3 saved adoption consumes canonical original generated sources without en } }); +test("an adoption bound to an earlier version's literal env override keeps it loadable until rollback frees it (HACK-1273)", async () => { + Reflect.deleteProperty(process.env, "CI"); + Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE"); + await trackedManagedCheckout(); + const files = await managedGenerated(); + const override = files[2] ?? ""; + const projectDir = join(projectRoot, ".hack"); + await writeFile(join(projectRoot, ".hack.secret.key"), "synthetic-v3-key"); + await setProjectEnvValue({ + projectRoot, + projectDir, + envName: null, + scope: "global", + key: "MANAGED", + value: CANARY, + secret: true, + }); + const binary = await managedEnvCompilerFixture( + join(root, "managed-compiler") + ); + // Models an adoption that an earlier version made over its literal file. + const store = await openLegacyComposeAdoptedGenerationStore({ projectRoot }); + try { + const generation = await store.prepare({ binary }); + await store.publish({ generation, binary }); + } finally { + await store.close(); + } + const pinned = await lstat(override); + + // What `hack doctor --fix` and the down and no-override scrubs run. + const kept = await removePlaintextComposeEnvOverride({ + projectDir, + includeUnverifiable: true, + }); + expect(kept).toMatchObject({ removed: false, adoptionBound: true }); + expect(kept.state.kind).toBe("plaintext"); + expect((await lstat(override)).ino).toBe(pinned.ino); + expect(await readFile(override, "utf8")).toContain(CANARY); + const report = await inspectComposeEnvAtRest({ projectDir }); + expect(report.adoptionBound).toBe(true); + expect(composeEnvAtRestRepairs(report)).toEqual({ + override: false, + asides: false, + }); + const described = describeComposeEnvAtRest({ + report, + projectDirName: ".hack", + }); + expect(described.status).toBe("warn"); + expect(described.message).toContain("hack config adopt --rollback"); + expect(JSON.stringify([kept, report, described])).not.toContain(CANARY); + // The doctor check reaches the adopted project, whose inputs ordinary + // discovery now refuses, and names the supported next step. + const checked = await checkComposeEnvOverrideAtRest({ + startDir: projectRoot, + }); + expect(checked.status).toBe("warn"); + expect(checked.message).toContain("pins this file"); + expect(checked.message).toContain("hack config adopt --rollback"); + expect(checked.message).not.toContain(CANARY); + + // The adoption still loads with key-free saved reads, then rolls back. + await rm(join(projectRoot, ".hack.secret.key")); + const savedStore = await openLegacyComposeAdoptedGenerationStore({ + projectRoot, + mode: "saved", + }); + try { + expect(await savedStore.loadActive()).toBeTruthy(); + await savedStore.rollback(); + } finally { + await savedStore.close(); + } + expect(await inspectLegacyComposeAdoptionSelection({ projectRoot })).toBe( + "rolled-back" + ); + // Rolled back, nothing pins the file: the same removal now takes it. + expect( + await removePlaintextComposeEnvOverride({ + projectDir, + includeUnverifiable: true, + }) + ).toMatchObject({ removed: true, adoptionBound: false }); + expect(await lstat(override).catch(() => null)).toBeNull(); +}); + +test("CLI adoption and its preview refuse a literal env override before preparation or engine probes (HACK-1273)", async () => { + await managedGenerated(); + for (const flags of [ + [], + ["--dry-run"], + ["--stop"], + ["--dry-run", "--json"], + ]) { + const child = Bun.spawn( + [ + process.execPath, + join(import.meta.dir, "../index.ts"), + "config", + "adopt", + "--path", + projectRoot, + ...flags, + ], + { + env: privateCliEnv({ + root, + env: { HACK_HOME: join(root, "isolated-home") }, + }), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + } + ); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + expect(code).not.toBe(0); + const output = stdout + stderr; + expect(output).toContain( + flags.includes("--json") + ? LEGACY_ADOPTION_LITERAL_ENV_OVERRIDE_CODE + : "Run 'hack up' once so Hack rewrites it with references only" + ); + expect(output).not.toContain(CANARY); + expect(await Bun.file(join(root, "commands")).exists()).toBe(false); + expect(await Bun.file(stateRoot()).exists()).toBe(false); + } +}); + +test("v3 value-free env override binds the engine config hash computed with the values (HACK-1273)", async () => { + Reflect.deleteProperty(process.env, "CI"); + Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE"); + await trackedManagedCheckout(); + // Distinct from CANARY, which the stub engine also uses as its engine ID. + const value = "synthetic-private-reference-env-value"; + const files = await managedGenerated("reference"); + expect(await readFile(files[2] ?? "", "utf8")).not.toContain(CANARY); + await writeFile(join(projectRoot, ".hack.secret.key"), "synthetic-v3-key"); + await setProjectEnvValue({ + projectRoot, + projectDir: join(projectRoot, ".hack"), + envName: null, + scope: "global", + key: "MANAGED", + value, + secret: true, + }); + const reference = renderComposeEnvReferenceOverride({ + targetServices: ["db"], + globalEnv: { MANAGED: value, EMPTY: "" }, + serviceEnv: { db: { MANAGED: value, EMPTY: "" } }, + }); + if (!reference) { + throw new Error("Synthetic reference projection missing"); + } + // The label Compose writes when Hack starts the stack with the values in + // its environment, and the hash the same files give without them. + const label = await composeReferenceHash({ files, env: reference.env }); + const blank = await composeReferenceHash({ files, env: {} }); + expect(label).not.toBe(blank); + fixture.envConfigHash = true; + fixture.configHash = label; + await save(); + const binary = await managedEnvCompilerFixture( + join(root, "managed-compiler") + ); + expect( + (await previewLegacyComposeAdoption({ projectRoot, binary })).complete + ).toBe(true); + const store = await openLegacyComposeAdoptedGenerationStore({ projectRoot }); + try { + const generation = await store.prepare({ binary }); + expect(generation.report.adoption_generation_version).toBe(3); + const metaText = await readFile( + await artifactPath("manifest.json"), + "utf8" + ); + const meta = JSON.parse(metaText); + expect(meta.binding.composeFiles).toEqual(files); + expect(meta.runtimeConfig).toEqual([ + { id: ID, service: "db", hash: label }, + ]); + for (const text of [metaText, JSON.stringify(generation)]) { + expect(text).not.toContain(value); + expect(text).not.toContain("HACK_ENV_"); + } + await store.publish({ generation, binary }); + await store.close(); + // Saved reads run without key or value delivery. + await rm(join(projectRoot, ".hack.secret.key")); + await mkdir(join(projectRoot, ".hack.secret.key")); + const savedStore = await openLegacyComposeAdoptedGenerationStore({ + projectRoot, + mode: "saved", + }); + try { + const active = await savedStore.loadActive(); + if (!active) { + throw new Error("Expected active adopted generation"); + } + let effects = 0; + await savedStore.withLease({ + generation: active, + run: async (input) => { + effects++; + expect(input.binding.containers[0]?.id).toBe(ID); + }, + }); + expect(effects).toBe(1); + // A recreated container carries a different engine label. + fixture.configHash = "e".repeat(64); + await save(); + await refusal( + savedStore.withLease({ + generation: active, + run: async () => { + effects++; + }, + }) + ); + expect(effects).toBe(1); + } finally { + await savedStore.close(); + } + expect(await readFile(join(root, "commands"), "utf8")).not.toContain(value); + } finally { + await store.close(); + } +}); + test("v3 raw managed drift fences saved execution and preserves the original stopped IDs", async () => { Reflect.deleteProperty(process.env, "CI"); Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE"); @@ -2058,7 +2342,8 @@ test("v4 typed locals preserve generated bytes, exact original IDs and key-free Reflect.deleteProperty(process.env, "CI"); Reflect.deleteProperty(process.env, "HACK_EXECUTION_MODE"); await trackedManagedCheckout(); - const files = await managedGenerated(); + // A fresh adoption binds the reference form; preview refuses a literal one. + const files = await managedGenerated("reference"); const generatedBytes = await Promise.all( files.map((path) => readFile(path, "utf8")) ); diff --git a/tests/native-compose-adoption-managed-fixture.test.ts b/tests/native-compose-adoption-managed-fixture.test.ts index ae48fb51..45146950 100644 --- a/tests/native-compose-adoption-managed-fixture.test.ts +++ b/tests/native-compose-adoption-managed-fixture.test.ts @@ -131,10 +131,16 @@ test("managed fixture uses real writer owners with isolated home, exact private MANAGED_FIXTURE_VALUE ); expect(JSON.stringify(resolved)).toBe("{}"); + expect(resolved.globalEnv.NC04_SECRET).toBe( + `${MANAGED_FIXTURE_VALUE}-${instance.name}` + ); + // HACK-1273: the generated override keeps only `${NAME}` references at + // rest; the private value is delivered through the resolved env. const path = resolved.composeFiles[2] ?? ""; const text = await readFile(path, "utf8"); - expect(text).toContain(`${MANAGED_FIXTURE_VALUE}-${instance.name}`); - generated.push(text); + expect(text).not.toContain(MANAGED_FIXTURE_VALUE); + expect(text).toContain('NC04_SECRET: "${HACK_ENV_'); + generated.push(resolved.globalEnv.NC04_SECRET ?? ""); const before = await managedAdoptionFixtureSourceSnapshot({ ...selected, instance, diff --git a/tests/native-compose-adoption-projection.test.ts b/tests/native-compose-adoption-projection.test.ts index e24c51dc..e25ab984 100644 --- a/tests/native-compose-adoption-projection.test.ts +++ b/tests/native-compose-adoption-projection.test.ts @@ -15,7 +15,10 @@ import { resolveRuntimeHostMetadataOverride, } from "../src/commands/project.ts"; import { PROJECT_ENV_KEY_FILENAME } from "../src/constants.ts"; -import { renderManagedComposeEnvOverride } from "../src/lib/compose-managed-env.ts"; +import { + renderComposeEnvReferenceOverride, + renderManagedComposeEnvOverride, +} from "../src/lib/compose-managed-env.ts"; import { hasLegacyComposeGeneratedSources, LegacyComposeAdoptionProjection, @@ -149,7 +152,15 @@ test("real canonical legacy writers produce exactly the privately qualified frag root, ".hack/.internal/compose.runtime.override.yml" ); - const expectedEnv = await readFile(envPath, "utf8"); + // The current writer keeps only `${NAME}` references at rest. + const expectedEnv = renderComposeEnvReferenceOverride({ + targetServices: ["web", "worker"], + globalEnv: { PRIVATE: CANARY, EMPTY: "" }, + serviceEnv: { + web: { PRIVATE: CANARY, EMPTY: "" }, + worker: { PRIVATE: CANARY, EMPTY: "" }, + }, + })?.text; const expectedRuntime = await readFile(runtimePath, "utf8"); await rm(envPath); await rm(runtimePath); @@ -176,13 +187,36 @@ test("real canonical legacy writers produce exactly the privately qualified frag expect(runtime).toBe(runtimePath); expect(env?.composeFiles).toEqual([envPath]); expect(await readFile(runtimePath, "utf8")).toBe(expectedRuntime); - expect(await readFile(envPath, "utf8")).toBe(expectedEnv); + expect(await readFile(envPath, "utf8")).toBe(expectedEnv ?? ""); + expect(await readFile(envPath, "utf8")).not.toContain(CANARY); + expect(Object.values(env?.env ?? {})).toContain(CANARY); const projection = await LegacyComposeAdoptionProjection.acquire({ source: await source(), }); expect(JSON.stringify(await projection.resolve())).toBe("{}"); }); +test("reference-form generated env still refuses a dropped managed key", async () => { + await layer(); + await generated(); + const reference = renderComposeEnvReferenceOverride({ + targetServices: ["web", "worker"], + globalEnv: { PRIVATE: CANARY }, + serviceEnv: { + web: { PRIVATE: CANARY }, + worker: { PRIVATE: CANARY }, + }, + }); + await writeFile( + join(root, ".hack/.internal/compose.env.override.yml"), + reference?.text ?? "" + ); + const projection = await LegacyComposeAdoptionProjection.acquire({ + source: await source(), + }); + await refuses(() => projection.resolve()); +}); + test("projection binds exact ordered generated bytes and keeps managed values outside candidate/report", async () => { await layer(); await generated(); diff --git a/tests/project-exec-command.test.ts b/tests/project-exec-command.test.ts index b106dae6..b76b4e61 100644 --- a/tests/project-exec-command.test.ts +++ b/tests/project-exec-command.test.ts @@ -20,6 +20,7 @@ const execCalls: Array<{ readonly cmdArgs: readonly string[]; }> = []; const psJsonByComposeProject = new Map(); +const psEnvs: Array | undefined> = []; const tempDirs = new Set(); const originalHackHome = process.env.HACK_HOME; @@ -31,11 +32,17 @@ const runtimeBackendMock = await registerScopedModuleMock({ name: "compose", up: async () => 0, down: async () => 0, - psJson: async (opts: { readonly composeFiles: readonly string[] }) => ({ - exitCode: 0, - stdout: psJsonByComposeProject.get(opts.composeFiles[0] ?? "") ?? "", - stderr: "", - }), + psJson: async (opts: { + readonly composeFiles: readonly string[]; + readonly env?: Record; + }) => { + psEnvs.push(opts.env ? { ...opts.env } : undefined); + return { + exitCode: 0, + stdout: psJsonByComposeProject.get(opts.composeFiles[0] ?? "") ?? "", + stderr: "", + }; + }, ps: async () => 0, run: async () => 0, exec: async (opts: { @@ -66,6 +73,7 @@ beforeAll(() => { afterEach(async () => { execCalls.length = 0; + psEnvs.length = 0; psJsonByComposeProject.clear(); for (const tempDir of tempDirs) { await rm(tempDir, { recursive: true, force: true }); @@ -139,10 +147,32 @@ test("exec reaches the running service while registry registration is locked", a expect(await Bun.file(resolve(registryRoot, "projects.json")).exists()).toBe( false ); - expect(execCalls[0]?.env).toEqual({ + const execEnv = Object.entries(execCalls[0]?.env ?? {}); + expect( + Object.fromEntries(execEnv.filter(([key]) => !key.startsWith("HACK_ENV_"))) + ).toEqual({ GLOBAL_FLAG: "qa", SHARED_KEY: "base", }); + // HACK-1273: the api override references these through the Compose + // process env instead of holding the values on disk. + expect( + execEnv + .filter(([key]) => key.startsWith("HACK_ENV_")) + .map( + ([key, value]) => + `${key.replace(/^HACK_ENV_[0-9A-F]{12}_/, "")}=${value}` + ) + .sort() + ).toEqual([ + "GLOBAL_FLAG=qa", + "PORT=3000", + "SERVICE_TOKEN=overlay-secret", + "SHARED_KEY=base", + ]); + // The readiness `compose ps` reads the same override, so it gets the same + // env; without it Compose warns once per unset reference. + expect(psEnvs).toEqual([execCalls[0]?.env]); expect(execCalls[0]?.workdir).toBe("/app"); expect(execCalls[0]?.cmdArgs).toEqual(["bun", "test"]); expect(execCalls[0]?.composeFiles.length).toBeGreaterThanOrEqual(2); diff --git a/tests/project-run-command.test.ts b/tests/project-run-command.test.ts index 5b148b3d..fbf52553 100644 --- a/tests/project-run-command.test.ts +++ b/tests/project-run-command.test.ts @@ -2,6 +2,7 @@ import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { YAML } from "bun"; import { CLI_SPEC } from "../src/cli/spec.ts"; import { @@ -200,7 +201,12 @@ test("run applies modern env overlays to service-specific compose overrides", as const call = runCalls[0]; expect(call).toBeDefined(); - expect(call?.env).toEqual({ + const callEnv = call?.env ?? {}; + expect( + Object.fromEntries( + Object.entries(callEnv).filter(([key]) => !key.startsWith("HACK_ENV_")) + ) + ).toEqual({ GLOBAL_FLAG: "qa", SHARED_KEY: "base", }); @@ -226,12 +232,27 @@ test("run applies modern env overlays to service-specific compose overrides", as throw new Error("Missing env override compose file."); } + // HACK-1273: the override holds `${NAME}` references only; Compose resolves + // each one from the env passed to it. const overrideText = await readFile(overridePath, "utf8"); - expect(overrideText).toContain("api:"); - expect(overrideText).toContain("GLOBAL_FLAG: qa"); - expect(overrideText).toContain("SHARED_KEY: base"); - expect(overrideText).toContain('PORT: "4000"'); - expect(overrideText).toContain("SERVICE_TOKEN: overlay-secret"); + expect(overrideText).not.toContain("overlay-secret"); + const override = YAML.parse(overrideText) as { + readonly services: Record< + string, + { readonly environment: Record } + >; + }; + const delivered = Object.fromEntries( + Object.entries(override.services.api?.environment ?? {}).map( + ([key, reference]) => [key, callEnv[reference.slice(2, -1)]] + ) + ); + expect(delivered).toEqual({ + GLOBAL_FLAG: "qa", + SHARED_KEY: "base", + PORT: "4000", + SERVICE_TOKEN: "overlay-secret", + }); expectNoEnvScopeWarnings(); }); diff --git a/tests/project-up-startup-state.test.ts b/tests/project-up-startup-state.test.ts index c7148f5f..0ba52c81 100644 --- a/tests/project-up-startup-state.test.ts +++ b/tests/project-up-startup-state.test.ts @@ -28,6 +28,21 @@ const psRows: string[] = []; const errorMessages: string[] = []; const warnMessages: string[] = []; const upComposeContents: string[] = []; +/** + * The global env Compose receives. The per-service `HACK_ENV_*` values that the + * value-free env override references (HACK-1273) are covered by + * tests/compose-env-at-rest-cli.test.ts. + */ +function withoutComposeEnvReferences( + env: Readonly> | undefined +): Readonly> | undefined { + return env + ? Object.fromEntries( + Object.entries(env).filter(([key]) => !key.startsWith("HACK_ENV_")) + ) + : env; +} + const upEnvs: Array> | undefined> = []; const psEnvs: Array> | undefined> = []; const upDetachSelections: Array = []; @@ -89,7 +104,7 @@ const runtimeBackendMock = await registerScopedModuleMock({ ) ).join("\n") ); - upEnvs.push(opts.env); + upEnvs.push(withoutComposeEnvReferences(opts.env)); upServiceSelections.push(opts.services); upStartupTimeouts.push(opts.startupTimeoutMs); upImageTimeouts.push(opts.imageTimeoutMs); @@ -106,7 +121,7 @@ const runtimeBackendMock = await registerScopedModuleMock({ psJson: async (opts: { readonly env?: Readonly>; }) => { - psEnvs.push(opts.env); + psEnvs.push(withoutComposeEnvReferences(opts.env)); return { exitCode: 0, stdout: psRows.join("\n"),