diff --git a/docs/diagrams/live-input-cleanup.mmd b/docs/diagrams/live-input-cleanup.mmd new file mode 100644 index 000000000..d52d42c81 --- /dev/null +++ b/docs/diagrams/live-input-cleanup.mmd @@ -0,0 +1,7 @@ +flowchart TD + P[Stale payload] --> R[Resolve current inputs] + F[Doctor finding] --> S[Inspected and current inputs] + R --> C{Removal allowed?} + S --> C + C -->|unknown, live, retired| K[Keep payload] + C -->|confirmed unused| D[Remove owned payload] diff --git a/docs/diagrams/live-input-cleanup.svg b/docs/diagrams/live-input-cleanup.svg new file mode 100644 index 000000000..cbe7ca48d --- /dev/null +++ b/docs/diagrams/live-input-cleanup.svg @@ -0,0 +1,66 @@ +unknown, live, retiredconfirmed unusedStale payloadResolve current inputsDoctor findingInspected and currentinputsRemoval allowed?Keep payloadRemove owned payload \ No newline at end of file diff --git a/docs/security/env-secrets-threat-model.md b/docs/security/env-secrets-threat-model.md index e6baf2536..6f379007f 100644 --- a/docs/security/env-secrets-threat-model.md +++ b/docs/security/env-secrets-threat-model.md @@ -43,7 +43,7 @@ such a process racing its checks. | `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. | | Local YAML layers | `.hack/hack.env.local.yaml` and `.hack/hack.env..local.yaml` | May hold plaintext; these are local-only, not safe to commit. New local files use 0600; an existing file keeps its mode. | -| Multiplexer input | `$TMPDIR/hack-mux-env-*/env.json` (the OS temp directory when TMPDIR is unset) | A private plaintext env payload, consumed and removed by the pane wrapper or owned cleanup; it exists on disk before that cleanup. A payload older than 10 minutes is refused and removed by the next staging (HACK-2170). | +| Multiplexer input | `$TMPDIR/hack-mux-env-*/env.json` (the OS temp directory when TMPDIR is unset) | A private plaintext env payload, consumed and removed by the pane wrapper or owned cleanup; it exists on disk before that cleanup. A payload older than 10 minutes is refused and removed by the next staging unless its `env.json` or directory is a path current configuration reads, or that configuration cannot be read; a directory still holding any other file is not removed (HACK-2170, HACK-2181). | | Background command output | `/commands//stdout` and `stderr` | Output can contain injected values. Redaction is partial and delayed; withholding a read does not erase disk bytes. | | Background command arguments | `/commands//meta.json` and `launch.json` | Automatic env maps are omitted, but caller argv is retained, including with metadata-only capture. Expanded values in argv are not redacted. | | Lifecycle output | `.hack/.internal/lifecycle/.log` (and `.before-private-*` kept history), and a FIFO in a private `${TMPDIR:-/tmp}/hack-lifecycle.*` directory | Hooks and persistent processes can emit injected values, stored verbatim. The log is private and owner-checked; remaining gaps are listed in gap 16 (HACK-2170). | @@ -116,7 +116,7 @@ HACK-2163 hardens three boundaries found in the env audit: | --- | --- | | A repository's contract names a variable exported in the developer's shell, so the migration saves it to disk. | `migrateLegacyProjectEnv` refuses, naming the key only, when a `plain_env` value resolves from the process environment (HACK-1688). A value in a regular `.hack/.env` migrates to the gitignored machine-local layer, never a tracked one (HACK-1712). | | A repository plants `.hack.secret.key` as a symlink (dangling or not) so that Hack reads or writes a file it should not. | Key reads go through `readBoundedPrivateFile` (`lstat`, `O_NOFOLLOW`, descriptor and path identity checks, 1 MiB bound, fatal UTF-8). New keys are created with `createPrivateFileExclusive` (`O_CREAT|O_EXCL|O_NOFOLLOW`, mode 0600, `fchmod`, `fsync`). Both are in `src/lib/private-file.ts`, used by `resolveProjectEnvKey`, `ensureProjectEnvSecretKey` and `resolveInheritedProjectEnvKey` in `src/lib/project-env-config.ts` (#288). | -| A symlinked `.hack/.env`, overlay file or `.hack` directory redirects a write, or a reader sees a partial file. | `src/lib/private-env-file.ts` (`readPrivateEnvFile`, `writePrivateEnvFileIfChanged`) and `replacePrivateFileAtomic` in `src/lib/private-file.ts`: a private staging file is written and fsynced, then renamed over the target; symlinks and non-regular entries are refused with one fixed message (#302). Used by mutation paths in `src/lib/hack-env.ts` and `materializeProjectEnv`. Dotenv reads in `src/lib/hack-env.ts` use the no-follow private-file reader (#762; gap 18). Staging copies of these files and of keys live in `.hack-staging/` next to the target, a private (0700) directory whose `.gitignore` of `*` keeps git from listing a copy a crash leaves. Writers never remove anything there, since a file configured at a staging-shaped path could be live; `hack doctor --fix` removes copies older than 10 minutes and never a path current resolution reads: configured stores and keys, the shared key and the project inputs legacy cleanup also keeps (`projectEnvLiveInputPaths`), each as given and resolved; the same holds for stale terminal-session payloads. Just before each unlink the configuration is resolved again and the target must be live in neither it nor the inspected one; if either cannot be resolved nothing is removed. The automatic pre-staging mux sweep (HACK-2170) does not consult configuration (`src/lib/private-staging.ts`, `src/lib/secrets-hygiene.ts`, HACK-2165). | +| 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 staging file is written and fsynced, then renamed over the target; symlinks and non-regular entries are refused with one fixed message (#302). Used by mutation paths in `src/lib/hack-env.ts` and `materializeProjectEnv`. Dotenv reads in `src/lib/hack-env.ts` use the no-follow private-file reader (#762; gap 18). Staging copies of these files and of keys live in `.hack-staging/` next to the target, a private (0700) directory whose `.gitignore` of `*` keeps git from listing a copy a crash leaves. Writers never remove anything there, since a file configured at a staging-shaped path could be live; `hack doctor --fix` removes copies older than 10 minutes and never a path current resolution reads: configured stores and keys, the shared key and the project inputs legacy cleanup also keeps (`projectEnvLiveInputPaths`), each as given and resolved; the same holds for stale terminal-session payloads. Just before each unlink the configuration is resolved again and the target must be live in neither it nor the inspected one; if either cannot be resolved nothing is removed. The automatic pre-staging mux sweep (HACK-2170) makes the same decision through the same code (`removalAllowed` in `src/lib/live-inputs.ts`): before each removal it resolves the configuration from the current directory, keeps a payload whose `env.json` or directory is a path current resolution reads, lies in a retired store or cannot be resolved, and removes nothing when that configuration cannot be read; inside a native project, whose legacy project discovery refuses, it therefore removes nothing and `hack doctor --fix` remains the cleanup (`src/lib/private-staging.ts`, `src/lib/secrets-hygiene.ts`, `src/mux/private-environment.ts`, HACK-2165, HACK-2181). | | 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). Machine-local layers are created 0600, and a write, even one that changes nothing, drops the group and other bits of an existing one; commands that read layers warn about a loose one (HACK-2165). The `hack doctor` env key check reports `env-key-weak-permissions` and `env-key-wrong-owner` (#309), and its "secrets hygiene" section reports loose, linked, tracked or unignored secret-bearing files, staging leftovers, stale terminal-session payloads and a store whose key is missing; `--fix` tightens modes, adds ignore entries and removes Hack's own leftovers, never a store, key or user file (`src/lib/secrets-hygiene.ts`, HACK-2165). | | 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 the generated override does not persist resolved values (command output can still contain them) (`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. The doctor check, every removal and the aside sweep apply the writer's no-follow check to `.hack` and `.hack/.internal` first (and recheck them before each move or delete): a symlinked or non-directory `.hack` or `.hack/.internal`, or a symlinked or non-regular override, is a closed, value-free refusal, and nothing behind it is listed, read, moved or deleted (HACK-1316). 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` (with or without `--stop`) and its preview refuse a literal override before preparation, because an adoption pins the file's bytes and inode for its life. Recovery acts on an existing preparation, so the store refuses instead: publication (`--recover --stop`) and switch completion (`--recover`) refuse a generation bound to a literal override before any original moves, and name `hack config adopt --recover --rollback` (HACK-1318). Rollback stays allowed. 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 `HACK_ENV_*` references, shared for global scope and distinct for service overrides, 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-managed.test.ts`, `tests/native-compose-adoption-generation-recovery.test.ts` and the sentinel scan `tests/env-secret-sentinel-scan.test.ts` (HACK-1273, HACK-1318). | | 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). | @@ -344,10 +344,18 @@ with mode 0600; the pane wrapper consumes and removes that payload (`src/mux/private-environment.ts`). During its lifetime, the same user and a backup or temp-file collector can read it. A payload older than 10 minutes is refused by the wrapper, and each new staging sweeps this user's stale private -payloads (an ownership-aware check, not a blanket temp-directory deletion). A -payload left by a crash stays until the next staging; there is no independent -reaper, but `hack doctor` reports it and `hack doctor --fix` removes it -(HACK-2165). +payloads (an ownership-aware check, not a blanket temp-directory deletion), +keeping a payload whose `env.json` or directory is a path current +configuration reads; if the configuration cannot be read that sweep removes +nothing (HACK-2181). This includes the global configuration file itself, even +when `HACK_GLOBAL_CONFIG_PATH` points inside a payload-shaped directory. +The shared guard's decision is shown below. + +![Live-input removal guard](../diagrams/live-input-cleanup.svg) + +A payload left by a crash stays until +the next staging; there is no independent reaper, but `hack doctor` reports it +and `hack doctor --fix` removes it (HACK-2165). Background output reaches `/commands//stdout` and `stderr` before the supervisor scans it (`src/lib/background-command/journal.ts`, diff --git a/docs/versions/v5/guides/parallel-work.md b/docs/versions/v5/guides/parallel-work.md index 1220de9d9..e9de6ebec 100644 --- a/docs/versions/v5/guides/parallel-work.md +++ b/docs/versions/v5/guides/parallel-work.md @@ -1,5 +1,9 @@ # Work on another branch +Stale terminal-session cleanup preserves configuration and environment inputs +that current resolution reads. When configuration cannot be verified, automatic +cleanup skips removal rather than guessing which files are unused. + Start from a configured repository. Read [Worktrees and watchers](../../guides/development-runtime-safety.md) before choosing shared or isolated files. diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 18364c642..dfc5ef237 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -177,6 +177,7 @@ import { } from "../lib/interactivity.ts"; import { resolveInternalExtraHosts } from "../lib/internal-extra-hosts.ts"; import { createLinuxTrustHost } from "../lib/linux-trust-host.ts"; +import { resolveLiveInputScope } from "../lib/live-inputs.ts"; import { findHackLocalCaCertPath } from "../lib/local-ca.ts"; import { noteBrowserTargetsForLocalNetworkHint } from "../lib/local-network-hint.ts"; import { @@ -258,7 +259,6 @@ import { removeLegacyProjectEnvArtifacts, repairLegacyComposeEnvFileReferences, resolveProjectEnvConfig, - resolveProjectEnvSharedKeyPath, type TrackedEnvMigrationChange, } from "../lib/project-env-config.ts"; import { @@ -275,7 +275,6 @@ import { readProjectsRegistry } from "../lib/projects-registry.ts"; import { resolveProxyBindAddress } from "../lib/proxy-bind.ts"; import { inspectProxyLoopback } from "../lib/proxy-loopback.ts"; import { readRuntimeProjects } from "../lib/runtime-projects.ts"; -import { resolveFileSecretStorePaths } from "../lib/secret-store.ts"; import { applySecretsHygieneFixes, inspectSecretsHygiene, @@ -3276,45 +3275,10 @@ export async function resolveSecretsHygieneScope(opts: { readonly startDir: string; readonly strict?: boolean; }): Promise { - const strict = opts.strict === true; - const tolerate = (promise: Promise): Promise => - strict ? promise : promise.catch(() => null); - const project = await tolerate(findDoctorLegacyProjectContext(opts.startDir)); - const stores = new Map(); - for (const projectDir of [undefined, project?.projectDir]) { - const resolved = await tolerate( - resolveFileSecretStorePaths({ projectDir, strict }) - ); - const paths = resolved - ? { storePath: resolved.storePath, keyPath: resolved.keyPath } - : null; - const key = paths ? `${paths.storePath}\0${paths.keyPath}` : null; - if (!(paths && key) || stores.has(key)) { - continue; - } - // Paths only a project's configuration chose (a cloned repository's - // may name any file of the user's) are fixed only inside the checkout - // or the Hack home. - stores.set( - key, - projectDir && project - ? { ...paths, fixRoots: [project.projectRoot, resolveGlobalHackDir()] } - : paths - ); - } - return { - project: project - ? { projectRoot: project.projectRoot, projectDir: project.projectDir } - : null, - sharedKeyPath: project - ? await tolerate( - resolveProjectEnvSharedKeyPath({ projectRoot: project.projectRoot }) - ) - : null, - stores: [...stores.values()], - storeKeyFromEnvironment: - (process.env.HACK_SECRETS_FILE_KEY ?? "").trim().length > 0, - }; + return await resolveLiveInputScope({ + ...opts, + findProject: findDoctorLegacyProjectContext, + }); } /** diff --git a/src/lib/live-inputs.ts b/src/lib/live-inputs.ts new file mode 100644 index 000000000..6b28eb16a --- /dev/null +++ b/src/lib/live-inputs.ts @@ -0,0 +1,220 @@ +import { realpath } from "node:fs/promises"; +import { join, resolve, sep } from "node:path"; +import { PROJECT_ENV_KEY_FILENAME } from "../constants.ts"; +import { + resolveGlobalConfigPath, + resolveGlobalHackDir, +} from "./config-paths.ts"; +import { + projectEnvLiveInputPaths, + resolveProjectEnvSharedKeyPath, +} from "./project-env-config.ts"; +import { + RETIRED_STORE_INFIX, + resolveFileSecretStorePaths, +} from "./secret-store.ts"; +import type { SecretsHygieneScope } from "./secrets-hygiene.ts"; + +/** + * The files current configuration reads (HACK-2165, HACK-2181), shared by + * every path that removes files on its own: `hack doctor --fix` and the + * automatic sweep of stale terminal-session payloads before each staging. + * Both decide through {@link removalAllowed}; a configuration that cannot + * be read refuses every removal. + */ + +/** + * `path` resolved, null when it (or a directory on the way) does not exist, + * or "unknown" for any other failure: only confirmed absence lets a caller + * look at the parent instead (HACK-2165). + */ +export async function realpathOrMissing( + path: string +): Promise { + try { + return await realpath(path); + } catch (error: unknown) { + const code = + typeof error === "object" && error !== null && "code" in error + ? (error as { readonly code?: unknown }).code + : undefined; + return code === "ENOENT" || code === "ENOTDIR" ? null : "unknown"; + } +} + +/** Whether a path component is a retired-store name (`.retired-…`). */ +export function inRetiredStore(path: string): boolean { + return path.split(sep).some((part) => part.includes(RETIRED_STORE_INFIX)); +} + +/** + * Every path current resolution reads (HACK-2165): the global configuration, + * configured stores and + * keys, the shared key, and the project's env inputs (its key, the primary + * worktree's key, `hack.config.json` and every YAML layer; see + * `projectEnvLiveInputPaths`), each as given and resolved. "unknown" when the + * project's inputs cannot be inspected: then nothing counts as removable. + */ +export async function livePaths( + scope: SecretsHygieneScope +): Promise | "unknown"> { + const paths = [ + ...(scope.globalConfigPath ? [scope.globalConfigPath] : []), + ...scope.stores.flatMap((store) => [store.storePath, store.keyPath]), + ...(scope.sharedKeyPath ? [scope.sharedKeyPath] : []), + ]; + if (scope.project) { + paths.push(join(scope.project.projectRoot, PROJECT_ENV_KEY_FILENAME)); + try { + paths.push(...(await projectEnvLiveInputPaths(scope.project))); + } catch { + return "unknown"; + } + } + const live = new Set(); + for (const path of paths) { + live.add(resolve(path)); + // A missing input has no resolved spelling; one that cannot be resolved + // for another reason leaves the set unknown, never smaller. + const real = await realpathOrMissing(path); + if (real === "unknown") { + return "unknown"; + } + if (real !== null) { + live.add(real); + } + } + return live; +} + +/** + * Whether resolution reads `path`, by its own spelling or resolved. A path, + * not an inode: removing another link to a live file never removes the name + * resolution reads. + */ +export async function isLive(opts: { + readonly path: string; + readonly live: ReadonlySet | "unknown"; +}): Promise { + if (opts.live === "unknown" || opts.live.has(resolve(opts.path))) { + return true; + } + // Only a missing target has no resolved spelling; one that cannot be + // resolved for another reason might be live (or retired), so it is. + const real = await realpathOrMissing(opts.path); + if (real === "unknown") { + return true; + } + if (real !== null && opts.live.has(real)) { + return true; + } + // A retired store (HACK-2177) is kept on purpose: it may hold the only + // copy of values its key still opens. Nothing in or under one is removed. + return [resolve(opts.path), real].some( + (spelling) => spelling !== null && inRetiredStore(spelling) + ); +} + +/** + * What removing a stale terminal-session payload can remove: its `env.json` + * and then, only when empty, its directory. + */ +export function muxRemovalTargets(root: string): readonly string[] { + return [join(root, "env.json"), root]; +} + +/** + * The one decision every automatic or `--fix` removal makes just before it + * acts: each target is live in none of `scopes` (current configuration, + * plus the inspected one for `--fix`), is not in a retired store, and has a + * resolved path that is known. Any failure to resolve a scope refuses. + */ +export async function removalAllowed(opts: { + readonly targets: readonly string[]; + readonly scopes: readonly (() => Promise)[]; +}): Promise { + const live: (ReadonlySet | "unknown")[] = []; + for (const scope of opts.scopes) { + const resolved = await scope().catch(() => null); + if (resolved === null) { + return false; + } + live.push(await livePaths(resolved).catch(() => "unknown" as const)); + } + for (const path of opts.targets) { + // Whatever the scopes say, a retired store is never removed, and neither + // is a target whose resolved path (so its membership) is unknown. + const real = await realpathOrMissing(path); + if ( + real === "unknown" || + inRetiredStore(resolve(path)) || + (real !== null && inRetiredStore(real)) + ) { + return false; + } + for (const set of live) { + if (await isLive({ path, live: set })) { + return false; + } + } + } + return true; +} + +/** + * The configuration a removal must respect, from `startDir`: the project + * (found by `findProject`), its shared key, and the encrypted-file stores the + * global and project configuration name. Reads configuration and git only. + * A report tolerates what it cannot read; with `strict` (used to authorize a + * removal) any discovery, read or parse failure rejects instead, so an + * unknown configuration never looks like one that names nothing. + */ +export async function resolveLiveInputScope(opts: { + readonly startDir: string; + readonly strict?: boolean; + readonly findProject: (startDir: string) => Promise<{ + readonly projectRoot: string; + readonly projectDir: string; + } | null>; +}): Promise { + const strict = opts.strict === true; + const tolerate = (promise: Promise): Promise => + strict ? promise : promise.catch(() => null); + const project = await tolerate(opts.findProject(opts.startDir)); + const stores = new Map(); + for (const projectDir of [undefined, project?.projectDir]) { + const resolved = await tolerate( + resolveFileSecretStorePaths({ projectDir, strict }) + ); + const paths = resolved + ? { storePath: resolved.storePath, keyPath: resolved.keyPath } + : null; + const key = paths ? `${paths.storePath}\0${paths.keyPath}` : null; + if (!(paths && key) || stores.has(key)) { + continue; + } + // Paths only a project's configuration chose (a cloned repository's + // may name any file of the user's) are fixed only inside the checkout + // or the Hack home. + stores.set( + key, + projectDir && project + ? { ...paths, fixRoots: [project.projectRoot, resolveGlobalHackDir()] } + : paths + ); + } + return { + globalConfigPath: resolve(resolveGlobalConfigPath()), + project: project + ? { projectRoot: project.projectRoot, projectDir: project.projectDir } + : null, + sharedKeyPath: project + ? await tolerate( + resolveProjectEnvSharedKeyPath({ projectRoot: project.projectRoot }) + ) + : null, + stores: [...stores.values()], + storeKeyFromEnvironment: + (process.env.HACK_SECRETS_FILE_KEY ?? "").trim().length > 0, + }; +} diff --git a/src/lib/secrets-hygiene.ts b/src/lib/secrets-hygiene.ts index a5147daab..0096e54da 100644 --- a/src/lib/secrets-hygiene.ts +++ b/src/lib/secrets-hygiene.ts @@ -24,6 +24,13 @@ import { } from "./env-layer-modes.ts"; import { gitignoreLinePattern, writeTextFileAtomicIfChanged } from "./fs.ts"; import { isKeychainDisabled } from "./keychain.ts"; +import { + isLive, + livePaths, + muxRemovalTargets, + realpathOrMissing, + removalAllowed, +} from "./live-inputs.ts"; import { findStagingLeftovers, legacyStagingTargetName, @@ -34,9 +41,7 @@ import { isGitIgnoredProjectEnvPath, isGitTrackedOrUnknownProjectPath, isInsideGitCheckoutMarker, - projectEnvLiveInputPaths, } from "./project-env-config.ts"; -import { RETIRED_STORE_INFIX } from "./secret-store.ts"; /** * `hack doctor`'s "secrets hygiene" section (HACK-2165): files at rest that @@ -136,6 +141,8 @@ export type SecretsHygieneFinding = { /** One file or directory the inspection covers, labelled for messages. */ export type SecretsHygieneScope = { + /** Global configuration file read while resolving the current inputs. */ + readonly globalConfigPath?: string; /** The project's root and its `.hack` directory, or null for none. */ readonly project: { readonly projectRoot: string; @@ -638,25 +645,6 @@ async function containingWorkTree( : "unknown"; } -/** - * `path` resolved, null when it (or a directory on the way) does not exist, - * or "unknown" for any other failure: only confirmed absence lets a caller - * look at the parent instead (HACK-2165). - */ -async function realpathOrMissing( - path: string -): Promise { - try { - return await realpath(path); - } catch (error: unknown) { - const code = - typeof error === "object" && error !== null && "code" in error - ? (error as { readonly code?: unknown }).code - : undefined; - return code === "ENOENT" || code === "ENOTDIR" ? null : "unknown"; - } -} - /** * The resolved nearest directory of `path` that exists (git runs there); null * when a directory on the way cannot be inspected. @@ -1023,7 +1011,7 @@ export async function inspectSecretsHygiene( const fix = finding.fix; if (fix?.kind !== "remove_leftover" && fix?.kind !== "remove_mux") { kept.push(finding); - } else if (!(await isLive({ path: removalTarget(fix), live }))) { + } else if (!(await anyLive({ paths: removalTargets(fix), live }))) { kept.push({ ...finding, fix: { ...fix, scope } }); } } @@ -1031,81 +1019,6 @@ export async function inspectSecretsHygiene( return await Promise.all(findings.map(withGuard)); } -/** - * Files the configuration uses (stores, keys, the project and shared keys), - * as given and resolved. - */ -/** - * Every path current resolution reads (HACK-2165): configured stores and - * keys, the shared key, and the project's env inputs (its key, the primary - * worktree's key, `hack.config.json` and every YAML layer; see - * `projectEnvLiveInputPaths`), each as given and resolved. "unknown" when the - * project's inputs cannot be inspected: then nothing counts as removable. - */ -async function livePaths( - scope: SecretsHygieneScope -): Promise | "unknown"> { - const paths = [ - ...scope.stores.flatMap((store) => [store.storePath, store.keyPath]), - ...(scope.sharedKeyPath ? [scope.sharedKeyPath] : []), - ]; - if (scope.project) { - paths.push(join(scope.project.projectRoot, PROJECT_ENV_KEY_FILENAME)); - try { - paths.push(...(await projectEnvLiveInputPaths(scope.project))); - } catch { - return "unknown"; - } - } - const live = new Set(); - for (const path of paths) { - live.add(resolve(path)); - // A missing input has no resolved spelling; one that cannot be resolved - // for another reason leaves the set unknown, never smaller. - const real = await realpathOrMissing(path); - if (real === "unknown") { - return "unknown"; - } - if (real !== null) { - live.add(real); - } - } - return live; -} - -/** - * Whether resolution reads `path`, by its own spelling or resolved. A path, - * not an inode: removing another link to a live file never removes the name - * resolution reads. - */ -async function isLive(opts: { - readonly path: string; - readonly live: ReadonlySet | "unknown"; -}): Promise { - if (opts.live === "unknown" || opts.live.has(resolve(opts.path))) { - return true; - } - // Only a missing target has no resolved spelling; one that cannot be - // resolved for another reason might be live (or retired), so it is. - const real = await realpathOrMissing(opts.path); - if (real === "unknown") { - return true; - } - if (real !== null && opts.live.has(real)) { - return true; - } - // A retired store (HACK-2177) is kept on purpose: it may hold the only - // copy of values its key still opens. Nothing in or under one is removed. - return [resolve(opts.path), real].some( - (spelling) => spelling !== null && inRetiredStore(spelling) - ); -} - -/** Whether a path component is a retired-store name (`.retired-…`). */ -function inRetiredStore(path: string): boolean { - return path.split(sep).some((part) => part.includes(RETIRED_STORE_INFIX)); -} - /** The directory a fix changes. */ function fixDirectory(fix: SecretsHygieneFix): string { switch (fix.kind) { @@ -1285,20 +1198,33 @@ export async function applySecretsHygieneFixes(opts: { return { applied, skipped }; } -/** The file a removal unlinks. */ -function removalTarget( +/** What a removal can remove (see `muxRemovalTargets`). */ +function removalTargets( fix: Extract -): string { +): readonly string[] { return fix.kind === "remove_leftover" - ? fix.leftover.path - : join(fix.stale.root, "env.json"); + ? [fix.leftover.path] + : muxRemovalTargets(fix.stale.root); +} + +async function anyLive(opts: { + readonly paths: readonly string[]; + readonly live: ReadonlySet | "unknown"; +}): Promise { + for (const path of opts.paths) { + if (await isLive({ path, live: opts.live })) { + return true; + } + } + return false; } /** - * Whether a removal may still go ahead: its target is live in neither the + * Whether a removal may still go ahead: its targets are live in neither the * inspection's scope nor the current one (resolved again here, so a * configuration edit since inspection counts). Any failure to resolve - * refuses the removal. + * refuses the removal. The automatic payload sweep makes the same decision + * through the same function (`removalAllowed`, HACK-2181). */ async function removalStillSafe(opts: { readonly fix: Extract< @@ -1307,34 +1233,14 @@ async function removalStillSafe(opts: { >; readonly currentScope?: () => Promise; }): Promise { - const scopes: SecretsHygieneScope[] = []; - if (opts.fix.scope) { - scopes.push(opts.fix.scope); - } - if (opts.currentScope) { - const current = await opts.currentScope().catch(() => null); - if (current === null) { - return false; - } - scopes.push(current); - } - const path = removalTarget(opts.fix); - // Whatever the scopes say, a retired store is never removed, and neither - // is a target whose resolved path (so its membership) is unknown. - const real = await realpathOrMissing(path); - if ( - real === "unknown" || - inRetiredStore(resolve(path)) || - (real !== null && inRetiredStore(real)) - ) { - return false; - } - for (const scope of scopes) { - if (await isLive({ path, live: await livePaths(scope) })) { - return false; - } - } - return true; + const inspected = opts.fix.scope; + return await removalAllowed({ + targets: removalTargets(opts.fix), + scopes: [ + ...(inspected ? [() => Promise.resolve(inspected)] : []), + ...(opts.currentScope ? [opts.currentScope] : []), + ], + }); } /** Applies one fix; false when it no longer applies. */ diff --git a/src/mux/private-environment.ts b/src/mux/private-environment.ts index 61f8b334c..b511a2a8c 100644 --- a/src/mux/private-environment.ts +++ b/src/mux/private-environment.ts @@ -11,6 +11,13 @@ import { import { constants as osConstants, tmpdir } from "node:os"; import { basename, dirname, isAbsolute, join } from "node:path"; import { childEnvironment } from "../lib/child-environment.ts"; +import { + muxRemovalTargets, + removalAllowed, + resolveLiveInputScope, +} from "../lib/live-inputs.ts"; +import { findProjectContext } from "../lib/project.ts"; +import type { SecretsHygieneScope } from "../lib/secrets-hygiene.ts"; const KEY = /^[A-Za-z_][A-Za-z0-9_]*$/; const PREFIX = "hack-mux-env-"; @@ -26,19 +33,28 @@ export const MUX_ENVIRONMENT_TTL_MS = 10 * 60 * 1000; // file's: both are set within moments of each other at staging. A payload // whose mtime is in the future (clock skew) never counts as expired. +/** Test seam: told the parent directory of every sweep this process runs. */ +export const sweepObserver: { onSweep?: (parent: string) => void } = {}; + /** * Remove this user's staged payloads older than the TTL from `parent` * (normally the temp directory). Only `hack-mux-env-*` directories owned by * this user with private modes are touched, and only their `env.json` when - * it is a regular single-link file of this user. Failures are skipped: this - * is cleanup, never a reason to refuse a new session. + * it is a regular single-link file of this user. Before each removal the + * configuration is resolved (from the current directory, strictly) and the + * same live-input check `hack doctor --fix` uses must allow it + * (`removalAllowed`, HACK-2181) for the exact targets, the payload's + * `env.json` and the directory: a payload is kept when either is a path + * current resolution reads, and nothing is removed when the configuration + * cannot be read. Any other file inside keeps the directory, since only an + * empty one is removed. Failures are skipped: this is cleanup, never a + * reason to refuse a new session. */ -/** Test seam: told the parent directory of every sweep this process runs. */ -export const sweepObserver: { onSweep?: (parent: string) => void } = {}; - export async function sweepStaleMuxEnvironments(opts: { readonly parent?: string; readonly now?: number; + /** The configuration to respect; resolved from the current directory by default. */ + readonly scope?: () => Promise; }): Promise { const parent = opts.parent ?? tmpdir(); sweepObserver.onSweep?.(parent); @@ -60,6 +76,14 @@ export async function sweepStaleMuxEnvironments(opts: { ) { continue; } + if ( + !(await removalAllowed({ + targets: muxRemovalTargets(root), + scopes: [opts.scope ?? currentLiveInputScope], + })) + ) { + continue; + } const payload = join(root, "env.json"); const file = await lstat(payload).catch(() => null); if (file) { @@ -75,6 +99,14 @@ export async function sweepStaleMuxEnvironments(opts: { } } +function currentLiveInputScope(): Promise { + return resolveLiveInputScope({ + startDir: process.cwd(), + strict: true, + findProject: findProjectContext, + }); +} + function isMissing(error: unknown): boolean { return error instanceof Error && "code" in error && error.code === "ENOENT"; } diff --git a/tests/mux-sweep-live-inputs.test.ts b/tests/mux-sweep-live-inputs.test.ts new file mode 100644 index 000000000..a82a1557e --- /dev/null +++ b/tests/mux-sweep-live-inputs.test.ts @@ -0,0 +1,169 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + lstat, + mkdir, + mkdtemp, + readFile, + realpath, + rm, + utimes, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { resolveLiveInputScope } from "../src/lib/live-inputs.ts"; +import { findProjectContext } from "../src/lib/project.ts"; +import { + MUX_ENVIRONMENT_TTL_MS, + sweepStaleMuxEnvironments, +} from "../src/mux/private-environment.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +// --------------------------------------------------------------------------- +// HACK-2181: the automatic sweep before a new terminal-session staging never +// removes a file current configuration reads, even inside a directory that +// looks exactly like a stale payload. Real files and a real configuration. +// --------------------------------------------------------------------------- + +let root = ""; +let saved: { config?: string; home?: string } = {}; + +beforeEach(async () => { + root = await realpath(await mkdtemp(join(tmpdir(), "hack-sweep-live-"))); + saved = { + config: process.env.HACK_GLOBAL_CONFIG_PATH, + home: process.env.HACK_HOME, + }; + process.env.HACK_HOME = join(root, "hack-home"); + process.env.HACK_GLOBAL_CONFIG_PATH = join(root, "hack.config.json"); +}); + +afterEach(async () => { + restoreEnv("HACK_GLOBAL_CONFIG_PATH", saved.config); + restoreEnv("HACK_HOME", saved.home); + await rm(root, { recursive: true, force: true }); +}); + +/** A private directory that looks like a payload left by a session that never ran. */ +async function stalePayload(parent: string, name: string): Promise { + const dir = join(parent, `hack-mux-env-${name}`); + await mkdir(dir, { mode: 0o700 }); + await writeFile(join(dir, "env.json"), "{}", { mode: 0o600 }); + const old = new Date(Date.now() - MUX_ENVIRONMENT_TTL_MS - 60_000); + await utimes(join(dir, "env.json"), old, old); + await utimes(dir, old, old); + return dir; +} + +const scopeFrom = (startDir: string) => () => + resolveLiveInputScope({ + startDir, + strict: true, + findProject: findProjectContext, + }); + +test("a configured store key inside a payload-like directory is never removed; an ordinary stale payload is", async () => { + const parent = join(root, "tmp"); + await mkdir(parent); + const holdsKey = await stalePayload(parent, "configured"); + const ordinary = await stalePayload(parent, "ordinary"); + // The global configuration names that directory's env.json as the key of + // the encrypted-file store. + await writeFile( + join(root, "hack.config.json"), + JSON.stringify({ + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { + path: join(root, "store.json"), + keyPath: join(holdsKey, "env.json"), + }, + }, + }, + }) + ); + await sweepStaleMuxEnvironments({ parent, scope: scopeFrom(root) }); + expect(await readFile(join(holdsKey, "env.json"), "utf8")).toBe("{}"); + expect(await lstat(ordinary).catch(() => null)).toBeNull(); +}); + +test("when the configuration cannot be read, the automatic sweep removes nothing", async () => { + const parent = join(root, "tmp"); + await mkdir(parent); + const stale = await stalePayload(parent, "kept"); + await writeFile(join(root, "hack.config.json"), "{ not json"); + await sweepStaleMuxEnvironments({ parent, scope: scopeFrom(root) }); + expect((await lstat(join(stale, "env.json"))).isFile()).toBe(true); + // Same when resolving throws for any other reason. + await sweepStaleMuxEnvironments({ + parent, + scope: async () => { + throw new Error("unreadable"); + }, + }); + expect((await lstat(join(stale, "env.json"))).isFile()).toBe(true); +}); + +test("the global configuration itself is a live input when stored in a stale payload", async () => { + const parent = join(root, "tmp"); + await mkdir(parent); + const configured = await stalePayload(parent, "global-config"); + const ordinary = await stalePayload(parent, "ordinary"); + process.env.HACK_GLOBAL_CONFIG_PATH = join(configured, "env.json"); + await sweepStaleMuxEnvironments({ parent, scope: scopeFrom(root) }); + expect(await readFile(join(configured, "env.json"), "utf8")).toBe("{}"); + expect(await lstat(ordinary).catch(() => null)).toBeNull(); +}); + +/** A project whose own configuration may name a store key; cwd moves into it. */ +async function inProject(config: string, run: () => Promise): Promise { + const project = join(root, "project"); + await mkdir(join(project, ".hack"), { recursive: true }); + await writeFile( + join(project, ".hack", "docker-compose.yml"), + "services: {}\n" + ); + await writeFile(join(project, ".hack", "hack.config.json"), config); + const cwd = process.cwd(); + process.chdir(project); + try { + return await run(); + } finally { + process.chdir(cwd); + } +} + +test("by default the sweep resolves the configuration from the current directory", async () => { + const parent = join(root, "tmp"); + await mkdir(parent); + const holdsKey = await stalePayload(parent, "configured"); + const ordinary = await stalePayload(parent, "ordinary"); + // Only the project's own configuration (found from the current directory) + // names the key: no injected scope, no global setting. + await inProject( + JSON.stringify({ + name: "sweepdefault", + controlPlane: { + secrets: { + backend: "encrypted_file", + encryptedFile: { + path: join(root, "store.json"), + keyPath: join(holdsKey, "env.json"), + }, + }, + }, + }), + () => sweepStaleMuxEnvironments({ parent }) + ); + expect(await readFile(join(holdsKey, "env.json"), "utf8")).toBe("{}"); + expect(await lstat(ordinary).catch(() => null)).toBeNull(); +}); + +test("by default an unparsable project configuration removes nothing", async () => { + const parent = join(root, "tmp"); + await mkdir(parent); + const stale = await stalePayload(parent, "kept"); + await inProject("{ not json", () => sweepStaleMuxEnvironments({ parent })); + expect((await lstat(join(stale, "env.json"))).isFile()).toBe(true); +});