diff --git a/docs/env.md b/docs/env.md index 3df3e3b48..a9de84aa4 100644 --- a/docs/env.md +++ b/docs/env.md @@ -345,6 +345,15 @@ reports such files by count, and the next removal, `hack doctor --fix`, or `hack `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. +Every step that touches the override first checks, without following a symlink, that +`.hack` and `.hack/.internal` are real directories and that the override is a regular +file; a step that moves or deletes something checks the directories again first. If +`.hack`, `.hack/.internal` or the override is a symlink or not a regular file or +directory, a Compose command that renders the override refuses with a fixed message, +and `hack doctor` reports a fixed refusal that names no value. Neither `hack doctor --fix`, +a removal nor the sweep of leftover `.remove` files lists, reads, moves or deletes +anything behind the link. Replace the link with a regular directory or file. + ## 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/reference/native-compose-adoption.md b/docs/reference/native-compose-adoption.md index ad16a7108..38adccfb8 100644 --- a/docs/reference/native-compose-adoption.md +++ b/docs/reference/native-compose-adoption.md @@ -423,6 +423,20 @@ TTY/stdin forwarding, deadlines and process-group cleanup use the existing shared runner. The store's AbortSignal is a cooperative admission/recheck control; it does not independently terminate a caller-owned engine callback. +`hack doctor` selects an active or pending adoption before ordinary discovery, which +would refuse the project as native input. It reports the adopted project, the env key, +tracked generated files and Compose env values at rest. Each other project check +(runtime and lifecycle hygiene, Compose networks and backend features, `DEV_HOST`, env +mode, materialization and overlay warnings, worktree keys and instances, the +`.hack/.gitignore` check, Caddy hosts) +reads inputs that the adoption owns, so doctor lists it as not checked instead of +failing. A pending adoption is a warning that asks for explicit recovery. The +`--fix` plan says that it leaves the adoption's inputs, retained containers and state +alone. For the project it only removes tracked generated files from the Git index +(the files stay on disk) and keeps an env override that the adoption pins. It leaves +`.hack/.gitignore` unchanged; a committed `.hack.secret.key` is also added to the +root `.gitignore`. Global repairs still run. + ## Rollback and interruption recovery ### Original file grants: private version 8 diff --git a/docs/security/env-secrets-threat-model.md b/docs/security/env-secrets-threat-model.md index 58b401486..5ebacde6d 100644 --- a/docs/security/env-secrets-threat-model.md +++ b/docs/security/env-secrets-threat-model.md @@ -75,7 +75,7 @@ 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` (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 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, HACK-1318). | +| 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. 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 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, 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). | | 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. | diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 5ecac5b39..d6770d32b 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -69,6 +69,12 @@ import { resolveGlobalHackDir, } from "../lib/config-paths.ts"; import { resolveDefaultDomain } from "../lib/default-domain.ts"; +import { + adoptedProjectCheckResult, + adoptedProjectNotApplicableResults, + type DoctorAdoptedProject, + findDoctorAdoptedProject, +} from "../lib/doctor-adopted-project.ts"; import { checkBrowserLocalNetwork, parseBrowserNetworkOptions, @@ -529,10 +535,26 @@ async function maybeRunNativeMappingRecovery( return null; } +/** + * The adopted project doctor reports, or null after ordinary discovery's + * native-input refusal passed. An adopted project's inputs belong to its + * adoption, so that discovery would refuse it as native input; doctor selects + * it first (HACK-1311). + */ +async function preflightDoctorProject(opts: { + readonly startDir: string; +}): Promise { + const adopted = await findDoctorAdoptedProject(opts); + if (!adopted) { + await assertLegacyProjectDiscovery(opts); + } + return adopted; +} + const handleDoctor: CommandHandlerFor = async ({ args, }): Promise => { - await assertLegacyProjectDiscovery({ + const adopted = await preflightDoctorProject({ startDir: resolveDoctorStartDir(args.options.path), }); const mappingResult = await maybeRunNativeMappingRecovery(args); @@ -832,13 +854,13 @@ const handleDoctor: CommandHandlerFor = async ({ ? resolve(process.cwd(), args.options.path) : process.cwd(); const projectCtx = await runCheck(s, "project", () => - checkProject({ startDir }) + checkProject({ startDir, adopted }) ); results.push(projectCtx); const agentIntegrationCheck = await runCheck( s, "agent integrations", - () => checkAgentIntegrations({ startDir }), + () => checkAgentIntegrations({ startDir, adopted }), { timeoutMs: 5000 } ); results.push( @@ -853,133 +875,21 @@ const handleDoctor: CommandHandlerFor = async ({ ); results.push( await runCheck(s, "sessions mux", () => - checkSessionsMuxConfig({ startDir }) + checkSessionsMuxConfig({ startDir, adopted }) ) ); - if (projectCtx.status === "ok") { - if ( - results.some( - (result) => result.name === "docker daemon" && result.status === "ok" - ) - ) { - results.push( - await runCheck(s, "runtime hygiene", () => - checkProjectRuntimeHygiene({ startDir }) - ) - ); - } - results.push( - await runCheck(s, "lifecycle hygiene", () => - checkProjectLifecycleHygiene({ startDir }) - ) - ); - results.push( - await runCheck(s, "compose networks", () => - checkComposeNetworkHygiene({ startDir }) - ) - ); - results.push( - await runCheck(s, COMPOSE_BACKEND_CHECK, () => - checkComposeBackendFeatures({ startDir, backend: "compose" }) - ) - ); - results.push( - await runCheck(s, "DEV_HOST", () => checkDevHost({ startDir })) - ); - results.push( - await runCheck(s, "env mode", () => checkProjectEnvMode({ startDir })) - ); - results.push( - await runCheck(s, "env materialization", () => - checkProjectEnvMaterialization({ startDir }) - ) - ); - results.push( - await runCheck(s, "env overlay warnings", () => - checkProjectEnvOverlayWarnings({ startDir }) - ) - ); - results.push( - await runCheck( - s, - "worktree keys", - () => checkWorktreeSecretKeys({ startDir }), - { - timeoutMs: 5000, - } - ) - ); - results.push( - await runCheck(s, ENV_KEY_CHECK, () => checkProjectEnvKey({ startDir }), { - timeoutMs: 5000, - }) - ); - results.push( - await runCheck( - s, - "generated files", - () => checkTrackedGeneratedFiles({ startDir }), - { - timeoutMs: 5000, - } - ) - ); - results.push( - await runCheck( - s, - HACK_GITIGNORE_CHECK, - () => checkHackDirGitignore({ startDir }), - { - timeoutMs: 5000, - } - ) - ); - results.push( - await runCheck(s, COMPOSE_ENV_AT_REST_CHECK, () => - checkComposeEnvOverrideAtRest({ startDir }) - ) - ); - if ( - results.some( + results.push( + ...(await runProjectChecks({ + progress: s, + startDir, + adopted, + projectFound: projectCtx.status === "ok", + dockerOk: results.some( (result) => result.name === "docker daemon" && result.status === "ok" - ) - ) { - results.push( - await runCheck( - s, - "worktree instances", - () => checkWorktreeInstanceCollisions({ startDir }), - { - timeoutMs: 5000, - } - ) - ); - } - results.push( - await runCheck( - s, - "caddy hosts", - () => checkCaddyHostMapping({ startDir }), - { - timeoutMs: 2000, - } - ) - ); - } else { - results.push({ - name: "DEV_HOST", - status: "warn", - message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, - durationMs: 0, - }); - results.push({ - name: "env mode", - status: "warn", - message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, - durationMs: 0, - }); - } + ), + })) + ); s.stop(`Doctor checks complete (${results.length} checks)`); @@ -1532,8 +1442,10 @@ function muxInstallCommand(opts: { async function checkSessionsMuxConfig(opts: { readonly startDir: string; + readonly adopted?: DoctorAdoptedProject | null; }): Promise { - const project = await findProjectContext(opts.startDir); + // An adopted project's config belongs to the adoption: global settings only. + const project = opts.adopted ? null : await findProjectContext(opts.startDir); const mode = await resolveSessionsMuxMode({ project }); const tmuxPath = findExecutableInPath("tmux"); const zellijPath = findExecutableInPath("zellij"); @@ -1797,6 +1709,7 @@ async function checkDaemonStatus(): Promise { async function checkAgentIntegrations(opts: { readonly startDir: string; + readonly adopted?: DoctorAdoptedProject | null; }): Promise { try { return await checkAgentIntegrationsUnsafe(opts); @@ -1812,8 +1725,9 @@ async function checkAgentIntegrations(opts: { async function checkAgentIntegrationsUnsafe(opts: { readonly startDir: string; + readonly adopted?: DoctorAdoptedProject | null; }): Promise { - const project = await findProjectContext(opts.startDir); + const project = opts.adopted ?? (await findProjectContext(opts.startDir)); const report = await inspectDoctorAgentIntegrations({ projectRoot: project?.projectRoot ?? null, }); @@ -2549,9 +2463,14 @@ async function checkProxyPortForwarding(): Promise { async function checkProject({ startDir, + adopted, }: { readonly startDir: string; + readonly adopted: DoctorAdoptedProject | null; }): Promise { + if (adopted) { + return adoptedProjectCheckResult(adopted); + } const ctx = await findProjectContext(startDir); if (ctx) { return { @@ -3069,10 +2988,17 @@ async function checkProjectEnvKey({ message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, }; } + return await checkEnvKeyAtRoot({ projectRoot: ctx.projectRoot }); +} + +/** The env key audit for a resolved project root (legacy or adopted). */ +async function checkEnvKeyAtRoot(opts: { + readonly projectRoot: string; +}): Promise { const inspection = await inspectProjectEnvKey({ - projectRoot: ctx.projectRoot, + projectRoot: opts.projectRoot, candidates: await resolveEnvKeyCandidates({ - projectRoot: ctx.projectRoot, + projectRoot: opts.projectRoot, }), env: process.env, uid: typeof process.getuid === "function" ? process.getuid() : null, @@ -3095,16 +3021,23 @@ async function checkTrackedGeneratedFiles({ }: { readonly startDir: string; }): Promise { - const name = "generated files"; const ctx = await findProjectContext(startDir); if (!ctx) { return { - name, + name: "generated files", status: "warn", message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, }; } + return await checkTrackedGeneratedFilesAt(ctx); +} +/** The tracked generated-files check for a resolved project (legacy or adopted). */ +async function checkTrackedGeneratedFilesAt(ctx: { + readonly projectRoot: string; + readonly projectDirName: string; +}): Promise { + const name = "generated files"; const inspection = await inspectTrackedGeneratedFiles({ projectRoot: ctx.projectRoot, projectDirName: ctx.projectDirName, @@ -3270,6 +3203,187 @@ async function findComposeEnvAtRestProject(opts: { : null; } +/** + * Project-scoped checks after the project check. An adopted project gets its + * own set (HACK-1311); a legacy project the full set; no project two skips. + */ +async function runProjectChecks(opts: { + readonly progress: DoctorProgress; + readonly startDir: string; + readonly adopted: DoctorAdoptedProject | null; + readonly projectFound: boolean; + readonly dockerOk: boolean; +}): Promise { + const { progress: s, startDir, adopted, dockerOk } = opts; + if (adopted) { + return await runAdoptedProjectChecks({ progress: s, adopted, startDir }); + } + const results: TimedCheckResult[] = []; + if (!opts.projectFound) { + results.push({ + name: "DEV_HOST", + status: "warn", + message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, + durationMs: 0, + }); + results.push({ + name: "env mode", + status: "warn", + message: `Skipped (no ${HACK_PROJECT_DIR_PRIMARY}/ found)`, + durationMs: 0, + }); + return results; + } + if (dockerOk) { + results.push( + await runCheck(s, "runtime hygiene", () => + checkProjectRuntimeHygiene({ startDir }) + ) + ); + } + results.push( + await runCheck(s, "lifecycle hygiene", () => + checkProjectLifecycleHygiene({ startDir }) + ) + ); + results.push( + await runCheck(s, "compose networks", () => + checkComposeNetworkHygiene({ startDir }) + ) + ); + results.push( + await runCheck(s, COMPOSE_BACKEND_CHECK, () => + checkComposeBackendFeatures({ startDir, backend: "compose" }) + ) + ); + results.push(await runCheck(s, "DEV_HOST", () => checkDevHost({ startDir }))); + results.push( + await runCheck(s, "env mode", () => checkProjectEnvMode({ startDir })) + ); + results.push( + await runCheck(s, "env materialization", () => + checkProjectEnvMaterialization({ startDir }) + ) + ); + results.push( + await runCheck(s, "env overlay warnings", () => + checkProjectEnvOverlayWarnings({ startDir }) + ) + ); + results.push( + await runCheck( + s, + "worktree keys", + () => checkWorktreeSecretKeys({ startDir }), + { + timeoutMs: 5000, + } + ) + ); + results.push( + await runCheck(s, ENV_KEY_CHECK, () => checkProjectEnvKey({ startDir }), { + timeoutMs: 5000, + }) + ); + results.push( + await runCheck( + s, + "generated files", + () => checkTrackedGeneratedFiles({ startDir }), + { + timeoutMs: 5000, + } + ) + ); + results.push( + await runCheck( + s, + HACK_GITIGNORE_CHECK, + () => checkHackDirGitignore({ startDir }), + { + timeoutMs: 5000, + } + ) + ); + results.push( + await runCheck(s, COMPOSE_ENV_AT_REST_CHECK, () => + checkComposeEnvOverrideAtRest({ startDir }) + ) + ); + if (dockerOk) { + results.push( + await runCheck( + s, + "worktree instances", + () => checkWorktreeInstanceCollisions({ startDir }), + { + timeoutMs: 5000, + } + ) + ); + } + results.push( + await runCheck( + s, + "caddy hosts", + () => checkCaddyHostMapping({ startDir }), + { + timeoutMs: 2000, + } + ) + ); + return results; +} + +/** + * Project checks for an adopted Compose project: those that do not read the + * inputs the adoption owns, then a not-applicable entry for each other + * project check (HACK-1311). The env-at-rest check keeps its own adopted-root + * selection. + */ +async function runAdoptedProjectChecks(opts: { + readonly progress: DoctorProgress; + readonly adopted: DoctorAdoptedProject; + readonly startDir: string; +}): Promise { + const { progress, adopted } = opts; + return [ + await runCheck( + progress, + ENV_KEY_CHECK, + () => checkEnvKeyAtRoot({ projectRoot: adopted.projectRoot }), + { timeoutMs: 5000 } + ), + await runCheck( + progress, + "generated files", + () => checkTrackedGeneratedFilesAt(adopted), + { timeoutMs: 5000 } + ), + await runCheck(progress, COMPOSE_ENV_AT_REST_CHECK, () => + checkComposeEnvOverrideAtRest({ startDir: opts.startDir }) + ), + ...adoptedProjectNotApplicableResults().map((result) => ({ + ...result, + durationMs: 0, + })), + ]; +} + +/** + * The legacy project context for a `--fix` step, or null when there is none + * or the project is adopted: an adoption owns its project's Compose inputs, + * env config and runtime, so those steps leave it alone. + */ +async function findDoctorLegacyProjectContext( + startDir: string +): Promise>> { + if (await findDoctorAdoptedProject({ startDir })) { + return null; + } + return await findProjectContext(startDir); +} + async function checkWorktreeInstanceCollisions({ startDir, }: { @@ -3544,7 +3658,7 @@ async function runDoctorFix(opts: { async function maybeRepairIncompleteRuntime(opts: { readonly startDir: string; }): Promise { - const project = await findProjectContext(opts.startDir); + const project = await findDoctorLegacyProjectContext(opts.startDir); if (!project) { return; } @@ -3605,7 +3719,7 @@ function renderSkippedDoctorFixSteps(): void { async function maybeMigrateProjectEnvConfig(opts: { readonly startDir: string; }): Promise { - const project = await findProjectContext(opts.startDir); + const project = await findDoctorLegacyProjectContext(opts.startDir); if (!project) { return; } @@ -3826,21 +3940,31 @@ export async function buildDoctorRemediationPlanLines(opts: { "Review and repair global Docker networks, CoreDNS, CA, host TLS env, daemon drift, and agent integration freshness where needed.", ]; - const project = await findProjectContext(opts.startDir); + const adopted = await findDoctorAdoptedProject({ startDir: opts.startDir }); + const project = adopted ? null : await findProjectContext(opts.startDir); + // Generated files and env values at rest are checked for either kind. + const target = adopted ?? project; + if (adopted) { + steps.push( + "Leave the adopted Compose project's inputs, retained containers and adoption state to 'hack config adopt'; --fix changes none of them." + ); + } if (project) { steps.push( "Reconcile lifecycle sessions and remove only ownership-proven sessions whose Compose instance is absent." ); } - const trackedGenerated = project + const trackedGenerated = target ? await inspectTrackedGeneratedFiles({ - projectRoot: project.projectRoot, - projectDirName: project.projectDirName, + projectRoot: target.projectRoot, + projectDirName: target.projectDirName, }) : null; if (trackedGenerated && trackedGenerated.trackedPaths.length > 0) { steps.push( - `Untrack machine-local generated files from git (git rm --cached; files stay on disk) and ensure the committed ${project?.projectDirName}/.gitignore: ${trackedGenerated.trackedPaths.join(", ")}` + adopted + ? `Untrack machine-local generated files from git (git rm --cached; files stay on disk) and leave the adopted project's ${adopted.projectDirName}/.gitignore unchanged: ${trackedGenerated.trackedPaths.join(", ")}` + : `Untrack machine-local generated files from git (git rm --cached; files stay on disk) and ensure the committed ${target?.projectDirName}/.gitignore: ${trackedGenerated.trackedPaths.join(", ")}` ); } const hackGitignore = project @@ -3854,24 +3978,28 @@ export async function buildDoctorRemediationPlanLines(opts: { ); } - const composeEnvAtRest = project + const composeEnvAtRest = target ? composeEnvAtRestRepairs( - await inspectComposeEnvAtRest({ projectDir: project.projectDir }) + await inspectComposeEnvAtRest({ projectDir: target.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.` + `Remove ${target?.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.` + `Remove leftover ${target?.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 })); + steps.push( + adopted + ? "Skip env migration because an adopted Compose project's env config belongs to 'hack config adopt'." + : await buildEnvMigrationPlanStep({ project }) + ); } return steps.map((step, index) => `${index + 1}. ${step}`); @@ -3911,7 +4039,11 @@ async function buildEnvMigrationPlanStep(opts: { async function maybeUntrackGeneratedFiles(opts: { readonly startDir: string; }): Promise { - const project = await findProjectContext(opts.startDir); + // An adopted project gets the git-index change and the root key entry only: + // its `.hack/` belongs to the adoption, and the legacy `.hack/.gitignore` + // writer refuses it as native input (HACK-1311). + const adopted = await findDoctorAdoptedProject({ startDir: opts.startDir }); + const project = adopted ?? (await findProjectContext(opts.startDir)); if (!project) { return; } @@ -3944,13 +4076,21 @@ async function maybeUntrackGeneratedFiles(opts: { return; } - const ignoreEnsured = await ensureHackDirGitignoreForDoctor({ project }); - const notes = [ - ignoreEnsured - ? `Removed ${inspection.trackedPaths.length} path(s) from the git index and ensured ${project.projectDirName}/.gitignore.` - : `Removed ${inspection.trackedPaths.length} path(s) from the git index; ${project.projectDirName}/.gitignore was left alone.`, - "Commit the removal to stop sharing these files.", - ]; + const notes: string[] = []; + if (adopted) { + notes.push( + `Removed ${inspection.trackedPaths.length} path(s) from the git index. Left ${project.projectDirName}/.gitignore unchanged: an adopted Compose project's ${project.projectDirName}/ belongs to 'hack config adopt'.`, + "Commit the removal to stop sharing these files, and keep them out of later commits." + ); + } else { + const ignoreEnsured = await ensureHackDirGitignoreForDoctor({ project }); + notes.push( + ignoreEnsured + ? `Removed ${inspection.trackedPaths.length} path(s) from the git index and ensured ${project.projectDirName}/.gitignore.` + : `Removed ${inspection.trackedPaths.length} path(s) from the git index; ${project.projectDirName}/.gitignore was left alone.`, + "Commit the removal to stop sharing these files." + ); + } if (inspection.secretKeyTracked) { // The nested .hack/.gitignore cannot cover the repo-root key file; without // the root entry the just-untracked key shows up as untracked and gets @@ -3978,7 +4118,8 @@ async function maybeUntrackGeneratedFiles(opts: { async function maybeEnsureHackDirGitignore(opts: { readonly startDir: string; }): Promise { - const project = await findProjectContext(opts.startDir); + // An adopted project's .hack/ belongs to `hack config adopt`: leave it alone. + const project = await findDoctorLegacyProjectContext(opts.startDir); if (!project) { return; } @@ -4125,6 +4266,10 @@ function composeEnvAtRestFixLines(opts: { lines.push( `Kept ${file}: a saved 'hack config adopt' generation now pins it (values omitted).` ); + } else if (removal.state.kind === "refused") { + lines.push( + `Left ${file} in place: it or its directory is now a symlink or not a regular file or directory (values omitted).` + ); } else { lines.push(`Left ${file} in place: it no longer holds env values.`); } @@ -4134,7 +4279,7 @@ function composeEnvAtRestFixLines(opts: { async function maybeRepairProjectLifecycleSessions(opts: { readonly startDir: string; }): Promise { - const project = await findProjectContext(opts.startDir); + const project = await findDoctorLegacyProjectContext(opts.startDir); if (!project) { return; } diff --git a/src/lib/compose-env-at-rest.ts b/src/lib/compose-env-at-rest.ts index fd6bbb056..4ae75a2ce 100644 --- a/src/lib/compose-env-at-rest.ts +++ b/src/lib/compose-env-at-rest.ts @@ -44,7 +44,7 @@ import { 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."; + "Refused the generated Compose env override: it, .hack or .hack/.internal is a symlink or not a regular file or directory. Replace it with a regular file or directory and retry; values omitted."; /** Absolute path of the generated env override for a project directory. */ export function composeEnvOverridePath(opts: { @@ -53,14 +53,80 @@ export function composeEnvOverridePath(opts: { return resolve(opts.projectDir, ".internal", COMPOSE_ENV_OVERRIDE_FILENAME); } +/** + * The project directory (`.hack`) and its `.internal` child. Every read, + * write, removal and aside sweep requires both to be real directories, checked + * without following a symlink, so no operation here reaches outside the + * project through a linked directory. + */ +function overrideDirectories(path: string): readonly string[] { + const internal = dirname(path); + return [dirname(internal), internal]; +} + function fileOptions(path: string): ReplaceablePrivateFileOptions { return { path, - directories: [dirname(path)], + directories: overrideDirectories(path), refusal: () => new Error(REFUSAL), }; } +type HeldDirectory = { + readonly path: string; + readonly dev: number; + readonly ino: number; +}; + +/** + * No-follow ownership check of the override's directories, the same one the + * writer applies before it writes: `absent` when either is missing, `refused` + * when either is a symlink or not a directory, else their identities so a + * later step can confirm they were not swapped. Other filesystem errors + * propagate. + */ +async function holdOverrideDirectories( + path: string +): Promise< + | { readonly kind: "absent" | "refused" } + | { readonly kind: "real"; readonly directories: readonly HeldDirectory[] } +> { + const directories: HeldDirectory[] = []; + for (const directory of overrideDirectories(path)) { + let info: Awaited>; + try { + info = await lstat(directory); + } catch (error: unknown) { + if (isRecord(error) && error.code === "ENOENT") { + return { kind: "absent" }; + } + throw error; + } + if (!info.isDirectory()) { + return { kind: "refused" }; + } + directories.push({ path: directory, dev: info.dev, ino: info.ino }); + } + return { kind: "real", directories }; +} + +/** True while every held directory is still the same real directory. */ +async function sameDirectories( + directories: readonly HeldDirectory[] +): Promise { + for (const held of directories) { + const current = await lstat(held.path).catch(() => null); + if ( + !current?.isDirectory() || + current.dev !== held.dev || + current.ino !== held.ino + ) { + return false; + } + } + return true; +} + /** * Writes value-free override text atomically with mode 0600 when the content * or the permissions differ. `text` must come from @@ -79,6 +145,11 @@ export async function writeComposeEnvOverride(opts: { ); } const path = composeEnvOverridePath(opts); + // Refuse a linked `.hack` or `.internal` before mkdir can create anything + // through it; the read and replace below check both again. + if ((await holdOverrideDirectories(path)).kind === "refused") { + throw new Error(REFUSAL); + } await mkdir(dirname(path), { recursive: true }); const options = fileOptions(path); const existing = await readReplaceablePrivateFile(options); @@ -86,21 +157,29 @@ export async function writeComposeEnvOverride(opts: { 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); + // sweeps stale ones too, once the directories passed the symlink checks + // above. Best effort: `hack doctor` reports what remains. + await removeStaleAsidesIfOwned({ path, now: opts.now ?? Date.now }).catch( + () => 0 + ); return path; } -/** At-rest state of the generated override; counts only, never values. */ +/** + * At-rest state of the generated override; counts only, never values. + * `refused`: the override, `.hack` or `.hack/.internal` is a symlink or not a + * regular file or directory. Nothing behind it is read, and nothing here + * removes or replaces it. + */ export type ComposeEnvOverrideAtRest = | { readonly kind: "absent"; readonly path: string } | { readonly kind: "unreadable"; readonly path: string } + | { readonly kind: "refused"; 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`. + * Reports what the generated override holds without following a symlink at + * the file, `.hack` or `.hack/.internal`; any of those is `refused`. */ export async function inspectComposeEnvOverrideAtRest(opts: { readonly projectDir: string; @@ -112,17 +191,33 @@ export async function inspectComposeEnvOverrideAtRest(opts: { async function inspectEntry( path: string ): Promise< - | { readonly kind: "absent" } - | { readonly kind: "unreadable" } + | { readonly kind: "absent" | "unreadable" | "refused" } | ComposeEnvOverrideContent > { + const held = await holdOverrideDirectories(path).catch(() => null); + if (!held) { + return { kind: "unreadable" }; + } + if (held.kind !== "real") { + return { kind: held.kind }; + } let current: Awaited>; try { current = await readReplaceablePrivateFile(fileOptions(path)); } catch (error: unknown) { if (error instanceof Error && error.message === REFUSAL) { + // Classify only through the directories held above: a swapped one is + // refused, never looked through. + if (!(await sameDirectories(held.directories))) { + return { kind: "refused" }; + } const info = await lstat(path).catch(() => null); - return info ? { kind: "unverifiable" } : { kind: "absent" }; + if (!info) { + return { kind: "absent" }; + } + // A regular file that changed during the read may hold values; a + // symlink or other entry is refused. + return info.isFile() ? { kind: "unverifiable" } : { kind: "refused" }; } return { kind: "unreadable" }; } @@ -142,6 +237,11 @@ const STALE_ASIDE_MS = 60_000; * values: `plaintext` always, `unverifiable` when `includeUnverifiable` is set. * Returns the state that was acted on. * + * Ownership: nothing is swept, moved or removed unless `.hack` and + * `.hack/.internal` are real directories (no-follow, as the writer checks), + * and they are checked again before each move and unlink. A symlinked or + * non-regular override is `refused` and left in place. + * * 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 @@ -165,8 +265,18 @@ export async function removePlaintextComposeEnvOverride(opts: { readonly asidesRemoved: number; }> { const path = composeEnvOverridePath(opts); + const held = await holdOverrideDirectories(path); + if (held.kind !== "real") { + return { + removed: false, + adoptionBound: false, + asidesRemoved: 0, + state: { kind: held.kind, path }, + }; + } const asidesRemoved = await removeStaleAsides({ path, + directories: held.directories, now: opts.now ?? Date.now, }); const eligible = (state: { readonly kind: string }) => @@ -185,6 +295,9 @@ export async function removePlaintextComposeEnvOverride(opts: { if (!info || info.isDirectory()) { return { ...kept, state }; } + if (!(await sameDirectories(held.directories))) { + return { ...kept, state: { kind: "refused", path } }; + } const aside = join( dirname(path), `.${COMPOSE_ENV_OVERRIDE_FILENAME}.${randomUUID()}.remove` @@ -263,22 +376,48 @@ async function restoreAside(opts: { */ async function removeStaleAsides(opts: { readonly path: string; + readonly directories: readonly HeldDirectory[]; readonly now: () => number; }): Promise { const asides = await listAsides(opts); for (const aside of asides.stale) { + if (!(await sameDirectories(opts.directories))) { + throw new Error(REFUSAL); + } await unlinkIfPresent(aside); } return asides.stale.length; } -/** Regular-file asides of crashed or in-flight removals, split by age. */ +/** Sweeps stale asides only while both directories are real; else none. */ +async function removeStaleAsidesIfOwned(opts: { + readonly path: string; + readonly now: () => number; +}): Promise { + const held = await holdOverrideDirectories(opts.path); + return held.kind === "real" + ? await removeStaleAsides({ ...opts, directories: held.directories }) + : 0; +} + +/** + * Regular-file asides of crashed or in-flight removals, split by age. Lists + * only the held `.internal` directory: a swap before or during the listing is + * refused rather than listed through. + */ async function listAsides(opts: { readonly path: string; + readonly directories: readonly HeldDirectory[]; readonly now: () => number; }): Promise<{ readonly stale: string[]; readonly fresh: string[] }> { const directory = dirname(opts.path); + if (!(await sameDirectories(opts.directories))) { + throw new Error(REFUSAL); + } const names = await readdir(directory).catch(() => []); + if (!(await sameDirectories(opts.directories))) { + throw new Error(REFUSAL); + } const stale: string[] = []; const fresh: string[] = []; for (const name of names) { @@ -323,10 +462,16 @@ export async function inspectComposeEnvAtRest(opts: { readonly now?: () => number; }): Promise { const override = await inspectComposeEnvOverrideAtRest(opts); - const asides = await listAsides({ - path: override.path, - now: opts.now ?? Date.now, - }); + // Asides are listed only inside real directories, never through a link. + const held = await holdOverrideDirectories(override.path).catch(() => null); + const asides = + held?.kind === "real" + ? await listAsides({ + path: override.path, + directories: held.directories, + now: opts.now ?? Date.now, + }) + : { stale: [], fresh: [] }; return { override, adoptionBound: @@ -374,6 +519,7 @@ export function describeComposeEnvAtRest(opts: { const described = describeOverride({ state: opts.report.override, file, + projectDirName: opts.projectDirName, adoptionBound: opts.report.adoptionBound, }); const asides = opts.report.asides.stale + opts.report.asides.fresh; @@ -391,6 +537,7 @@ export function describeComposeEnvAtRest(opts: { function describeOverride(opts: { readonly state: ComposeEnvOverrideAtRest; readonly file: string; + readonly projectDirName: string; readonly adoptionBound: boolean; }): { readonly status: "ok" | "warn"; readonly message: string } { const { state, file } = opts; @@ -420,6 +567,11 @@ function describeOverride(opts: { status: "warn", message: `${file} could not be read, so Hack cannot confirm it holds no env values. Check its owner and permissions.`, }; + case "refused": + return { + status: "warn", + message: `Refused to inspect ${file}: it, ${opts.projectDirName} or ${opts.projectDirName}/.internal is a symlink or not a regular file or directory. Hack does not follow or change it, and 'hack doctor --fix' leaves it in place (values omitted). Replace it with a regular file or directory.`, + }; default: return state satisfies never; } diff --git a/src/lib/doctor-adopted-project.ts b/src/lib/doctor-adopted-project.ts new file mode 100644 index 000000000..84d36a08d --- /dev/null +++ b/src/lib/doctor-adopted-project.ts @@ -0,0 +1,107 @@ +import { join } from "node:path"; +import { HACK_PROJECT_DIR_PRIMARY } from "../constants.ts"; +import { COMPOSE_BACKEND_CHECK } from "./doctor-compose-backend.ts"; +import { selectLegacyComposeAdoptedRoot } from "./native-compose-adoption-command.ts"; +import { inspectLegacyComposeAdoptionSelection } from "./native-compose-adoption-marker.ts"; + +/** + * An adopted (`hack config adopt`) Compose project as `hack doctor` sees it. + * + * Ownership: an active or pending adoption owns the project's authored Compose + * inputs (they live in its saved generation), so ordinary project discovery + * refuses the project. Doctor selects it explicitly, runs only the checks that + * do not read those inputs (env key, tracked generated files, env values at + * rest), and reports every other project check as not applicable. Doctor and + * `--fix` never read, move or rewrite the adoption's inputs or state. + */ +export type DoctorAdoptedProject = { + readonly projectRoot: string; + readonly projectDir: string; + readonly projectDirName: typeof HACK_PROJECT_DIR_PRIMARY; + /** `pending`: an adoption operation is in progress or was interrupted. */ + readonly state: "active" | "pending"; +}; + +/** + * The adopted project that owns `startDir`, or null when discovery from there + * reaches no active or pending adoption (or a nearer legacy project). Unsafe + * adoption state throws the adoption selector's closed, value-free refusal. + */ +export async function findDoctorAdoptedProject(opts: { + readonly startDir: string; +}): Promise { + const projectRoot = await selectLegacyComposeAdoptedRoot({ + cwd: opts.startDir, + }); + if (!projectRoot) { + return null; + } + const state = await inspectLegacyComposeAdoptionSelection({ projectRoot }); + if (state !== "active" && state !== "pending") { + // Rolled back since the selection: ordinary discovery owns it again. + return null; + } + return { + projectRoot, + projectDir: join(projectRoot, HACK_PROJECT_DIR_PRIMARY), + projectDirName: HACK_PROJECT_DIR_PRIMARY, + state, + }; +} + +/** Doctor's `project` check for an adopted project. Closed and value-free. */ +export function adoptedProjectCheckResult(project: DoctorAdoptedProject): { + readonly name: "project"; + readonly status: "ok" | "warn"; + readonly message: string; +} { + if (project.state === "pending") { + return { + name: "project", + status: "warn", + message: `Adopted Compose project at ${project.projectDir} has a pending or interrupted adoption operation. Use explicit adoption recovery before other project commands; values omitted.`, + }; + } + return { + name: "project", + status: "ok", + message: `Adopted Compose project at ${project.projectDir} ('hack config adopt'). Its Compose inputs belong to the adoption; adopted commands support retained-container start, restart, stop, ps, logs and exec.`, + }; +} + +/** + * Project checks that read authored Compose inputs, project config or env + * config, or the project's own Compose instance. For an adopted project those + * inputs belong to the adoption, so each is reported as not applicable + * instead of failing on ordinary discovery's refusal. + */ +export const ADOPTED_PROJECT_NOT_APPLICABLE_CHECKS = [ + "runtime hygiene", + "lifecycle hygiene", + "compose networks", + COMPOSE_BACKEND_CHECK, + "DEV_HOST", + "env mode", + "env materialization", + "env overlay warnings", + "worktree keys", + "worktree instances", + "hack gitignore", + "caddy hosts", +] as const; + +export const ADOPTED_PROJECT_NOT_APPLICABLE_MESSAGE = + "Not checked: this adopted Compose project's inputs belong to 'hack config adopt'."; + +/** Not-applicable results for {@link ADOPTED_PROJECT_NOT_APPLICABLE_CHECKS}. */ +export function adoptedProjectNotApplicableResults(): readonly { + readonly name: string; + readonly status: "ok"; + readonly message: string; +}[] { + return ADOPTED_PROJECT_NOT_APPLICABLE_CHECKS.map((name) => ({ + name, + status: "ok", + message: ADOPTED_PROJECT_NOT_APPLICABLE_MESSAGE, + })); +} diff --git a/tests/compose-env-at-rest.test.ts b/tests/compose-env-at-rest.test.ts index ab76f122a..1e5e91548 100644 --- a/tests/compose-env-at-rest.test.ts +++ b/tests/compose-env-at-rest.test.ts @@ -243,27 +243,25 @@ test("inspection and removal act only on literal or unverifiable overrides", asy 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. + // A symlink is never followed and never removed, even on request: it is + // refused (HACK-1316), and both it and its target are left in place. 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( - ( + for (const includeUnverifiable of [false, true]) { + expect( await removePlaintextComposeEnvOverride({ projectDir, - includeUnverifiable: true, + includeUnverifiable, }) - ).removed - ).toBe(true); - expect(await lstat(path).catch(() => null)).toBeNull(); + ).toEqual({ + removed: false, + state: { kind: "refused", path }, + adoptionBound: false, + asidesRemoved: 0, + }); + } + expect((await lstat(path)).isSymbolicLink()).toBe(true); expect(await readFile(target, "utf8")).toContain(CANARY); }); @@ -499,3 +497,130 @@ test("an override that an adoption state may pin is kept by removal and reported expect(removed).toMatchObject({ removed: true, adoptionBound: false }); expect(await lstat(path).catch(() => null)).toBeNull(); }); + +/** Names and contents of every entry in a directory outside the project. */ +async function snapshotDirectory( + directory: string +): Promise> { + const names = (await readdir(directory)).sort(); + return Object.fromEntries( + await Promise.all( + names.map(async (name) => [ + name, + await readFile(join(directory, name), "utf8"), + ]) + ) + ); +} + +/** + * HACK-1316: a symlinked `.hack` or `.hack/.internal` points outside the + * project, at a directory that holds a file named like the override (not one + * Hack can verify, the reported case) and a stale-aged aside. Inspection, the + * `--fix` removal, the aside sweep and the writer all refuse it, closed and + * value-free, and nothing behind the link is listed, read or changed. + */ +for (const linked of [".hack", ".hack/.internal"] as const) { + test(`a symlinked ${linked} is refused by inspection, removal, the sweep and the writer, and nothing behind it changes (HACK-1316)`, async () => { + const behind = join(root, "outside", ".internal"); + await mkdir(behind, { recursive: true }); + await writeFile(join(behind, "compose.env.override.yml"), `${CANARY}\n`); + await writeFile(join(behind, ASIDE), literalText()); + let dir = projectDir; + if (linked === ".hack") { + await mkdir(join(root, "linked-root")); + dir = join(root, "linked-root", ".hack"); + await symlink(join(root, "outside"), dir); + } else { + await symlink(behind, join(projectDir, ".internal")); + } + const before = await snapshotDirectory(behind); + // Old enough that a sweep through the link would take the aside. + const later = () => Date.now() + 61_000; + const path = composeEnvOverridePath({ projectDir: dir }); + + const report = await inspectComposeEnvAtRest({ + projectDir: dir, + now: later, + }); + expect(report).toEqual({ + override: { kind: "refused", path }, + adoptionBound: false, + asides: { stale: 0, fresh: 0 }, + }); + expect(composeEnvAtRestRepairs(report)).toEqual({ + override: false, + asides: false, + }); + const described = describeComposeEnvAtRest({ + report, + projectDirName: ".hack", + }); + expect(described).toEqual({ + status: "warn", + message: + "Refused to inspect .hack/.internal/compose.env.override.yml: it, .hack or .hack/.internal is a symlink or not a regular file or directory. Hack does not follow or change it, and 'hack doctor --fix' leaves it in place (values omitted). Replace it with a regular file or directory.", + }); + + expect( + await removePlaintextComposeEnvOverride({ + projectDir: dir, + includeUnverifiable: true, + now: later, + }) + ).toEqual({ + removed: false, + state: { kind: "refused", path }, + adoptionBound: false, + asidesRemoved: 0, + }); + const references = + renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + })?.text ?? ""; + const refusal = await writeComposeEnvOverride({ + projectDir: dir, + text: references, + now: later, + }).catch((error: unknown) => error); + expect(String(refusal)).toBe( + "Error: Refused the generated Compose env override: it, .hack or .hack/.internal is a symlink or not a regular file or directory. Replace it with a regular file or directory and retry; values omitted." + ); + + expect(await snapshotDirectory(behind)).toEqual(before); + expect(JSON.stringify([report, described, String(refusal)])).not.toContain( + CANARY + ); + expect(described.message).not.toContain(root); + }); +} + +/** + * HACK-1316: the writer refuses a symlinked `.hack` before its `mkdir`, so a + * link to an outside directory without `.internal` gains nothing; the later + * read and replace checks alone would refuse only after creating it. + */ +test("the writer refuses a symlinked .hack before creating .internal behind it (HACK-1316)", async () => { + const outside = join(root, "outside"); + await mkdir(outside); + await mkdir(join(root, "linked-root")); + const dir = join(root, "linked-root", ".hack"); + await symlink(outside, dir); + const references = + renderComposeEnvReferenceOverride({ + targetServices: ["web"], + globalEnv: {}, + serviceEnv, + })?.text ?? ""; + + const refusal = await writeComposeEnvOverride({ + projectDir: dir, + text: references, + }).catch((error: unknown) => error); + expect(String(refusal)).toBe( + "Error: Refused the generated Compose env override: it, .hack or .hack/.internal is a symlink or not a regular file or directory. Replace it with a regular file or directory and retry; values omitted." + ); + expect(await readdir(outside)).toEqual([]); +}); diff --git a/tests/doctor-adopted-project.test.ts b/tests/doctor-adopted-project.test.ts new file mode 100644 index 000000000..9021332d8 --- /dev/null +++ b/tests/doctor-adopted-project.test.ts @@ -0,0 +1,165 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdir, readFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { buildDoctorRemediationPlanLines } from "../src/commands/doctor.ts"; +import { + ADOPTED_PROJECT_NOT_APPLICABLE_CHECKS, + ADOPTED_PROJECT_NOT_APPLICABLE_MESSAGE, +} from "../src/lib/doctor-adopted-project.ts"; +import { inspectLegacyComposeAdoptionSelection } from "../src/lib/native-compose-adoption-marker.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; +import { + retainedSourceBindFixture, + SOURCE_BIND_CANARY, +} from "./helpers/retained-source-bind-adoption.ts"; + +/** + * HACK-1311: `hack doctor` in an adopted (`hack config adopt`) project. The + * adoption owns the project's Compose inputs, so ordinary discovery refuses + * the project as native input; doctor must select it explicitly, report it + * with closed messages, and plan `--fix` without throwing. The fixture is the + * synthetic retained source-bind adoption (no real engine). + */ +let h: Awaited>; +beforeEach(async () => { + h = await retainedSourceBindFixture(); +}); +afterEach(async () => { + await h.cleanup(); +}); + +/** Prepares and publishes the adoption: the project is now adopted. */ +async function adopt(): Promise { + const store = await h.store(); + try { + const generation = await store.prepare({ binary: h.compiler }); + await store.publish({ generation, binary: h.compiler }); + } finally { + await store.close(); + } + expect( + await inspectLegacyComposeAdoptionSelection({ projectRoot: h.root }) + ).toBe("active"); +} + +const receiptPath = () => + join(h.root, ".hack/.internal/legacy-compose-adoption-v1/receipt.json"); + +/** Read-only `doctor --json` in a private HOME and HACK_HOME, stub engine only. */ +async function doctorJson(): Promise<{ + readonly exit: number; + readonly stdout: string; + readonly stderr: string; +}> { + const cli = resolve(import.meta.dir, "..", "index.ts"); + const state = join(h.outer, "state"); + await mkdir(state, { recursive: true }); + const child = Bun.spawn( + [process.execPath, cli, "doctor", "--path", h.root, "--json"], + { + cwd: h.root, + env: privateCliEnv({ + root: h.outer, + env: { + HACK_HOME: state, + HACK_GLOBAL_CONFIG_PATH: join(state, "hack.config.json"), + HACK_DAEMON_DISABLE: "1", + HACK_NO_INTERACTIVE: "1", + HACK_LOGGER: "console", + XDG_CACHE_HOME: join(h.outer, "cache"), + PATH: `${h.outer}:/usr/bin:/bin`, + }, + }), + stdout: "pipe", + stderr: "pipe", + } + ); + const [exit, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]); + return { exit, stdout, stderr }; +} + +type JsonCheck = { + readonly id: string; + readonly status: string; + readonly detail: string; + readonly durationMs: number; +}; + +test("doctor reports an adopted project with closed messages instead of refusing it as native input (HACK-1311)", async () => { + await adopt(); + const receipt = await readFile(receiptPath(), "utf8"); + const run = await doctorJson(); + expect(run.stdout + run.stderr).not.toContain("E_NATIVE_PROJECT"); + expect(run.stdout + run.stderr).not.toContain(SOURCE_BIND_CANARY); + const parsed: unknown = JSON.parse(run.stdout); + expect(parsed).toMatchObject({ ok: true }); + const checks = (parsed as { data: { checks: JsonCheck[] } }).data.checks; + const byId = new Map(checks.map((check) => [check.id, check])); + + expect(byId.get("project")).toEqual({ + id: "project", + status: "ok", + detail: `Adopted Compose project at ${join(h.root, ".hack")} ('hack config adopt'). Its Compose inputs belong to the adoption; adopted commands support retained-container start, restart, stop, ps, logs and exec.`, + durationMs: expect.any(Number), + }); + // Checks that do not read the adoption's inputs still run. + expect(byId.get("env key")?.status).toBe("ok"); + expect(byId.get("generated files")?.status).toBe("ok"); + expect(byId.get("compose env at rest")).toMatchObject({ + status: "ok", + detail: "No generated Compose env override", + }); + // The adoption owns .hack/, so its committed .gitignore is not audited. + expect(byId.get("hack gitignore")).toMatchObject({ + status: "ok", + detail: ADOPTED_PROJECT_NOT_APPLICABLE_MESSAGE, + }); + // The rest are reported as not applicable, never as discovery failures. + for (const name of ADOPTED_PROJECT_NOT_APPLICABLE_CHECKS) { + expect(byId.get(name)).toMatchObject({ + status: "ok", + detail: ADOPTED_PROJECT_NOT_APPLICABLE_MESSAGE, + }); + } + for (const name of [ + "project", + "agent integrations", + "sessions mux", + "env key", + "generated files", + "compose env at rest", + ...ADOPTED_PROJECT_NOT_APPLICABLE_CHECKS, + ]) { + expect(byId.get(name)?.status).not.toBe("error"); + } + // Read-only: the adoption is unchanged and no retained container moved. + expect(await readFile(receiptPath(), "utf8")).toBe(receipt); + expect( + (await h.commands()).filter( + (args) => + args[0] === "container" && ["start", "stop"].includes(args[1] ?? "") + ) + ).toEqual([]); +}, 120_000); + +test("the doctor --fix plan for an adopted project says what it leaves alone instead of throwing (HACK-1311)", async () => { + await adopt(); + const receipt = await readFile(receiptPath(), "utf8"); + // From a subdirectory too: selection walks up to the adopted root. + for (const startDir of [h.root, join(h.root, "source")]) { + const plan = await buildDoctorRemediationPlanLines({ + startDir, + migrateEnvConfig: true, + }); + expect(plan).toEqual([ + "1. Review and repair global Docker networks, CoreDNS, CA, host TLS env, daemon drift, and agent integration freshness where needed.", + "2. Leave the adopted Compose project's inputs, retained containers and adoption state to 'hack config adopt'; --fix changes none of them.", + "3. Skip env migration because an adopted Compose project's env config belongs to 'hack config adopt'.", + ]); + } + expect(await readFile(receiptPath(), "utf8")).toBe(receipt); +}, 60_000); diff --git a/tests/doctor-fix-adopted-project.test.ts b/tests/doctor-fix-adopted-project.test.ts new file mode 100644 index 000000000..4fd31e8d6 --- /dev/null +++ b/tests/doctor-fix-adopted-project.test.ts @@ -0,0 +1,299 @@ +import { afterAll, afterEach, beforeEach, expect, test } from "bun:test"; +import { randomBytes } from "node:crypto"; +import { lstat, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { resetNoInteractiveFlagForTests } from "../src/lib/interactivity.ts"; +import { inspectLegacyComposeAdoptionSelection } from "../src/lib/native-compose-adoption-marker.ts"; +import { restoreEnv } from "./helpers/env.ts"; +import { + retainedSourceBindFixture, + SOURCE_BIND_CANARY, +} from "./helpers/retained-source-bind-adoption.ts"; +import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts"; + +/** + * HACK-1311: `hack doctor --fix --no-interactive` in an adopted + * (`hack config adopt`) project whose generated files are tracked by Git. Every + * `--fix` step must leave the adoption alone instead of reaching a legacy + * writer that refuses the project as native input: the untrack step changes + * the Git index (and the root key entry) but not the adoption's + * `.hack/.gitignore`, and the Docker-backed project steps and env migration + * skip the project. The fixture is the synthetic retained source-bind + * adoption; Docker, DNS, prompts and child commands are mocked, and Git runs + * only against the fixture checkout with no global or system config. + */ + +const runCalls: string[][] = []; +const noteCalls: string[] = []; + +/** Real `git` for the fixture checkout only; never through the mocked shell. */ +async function gitExec(cmd: readonly string[]): Promise<{ + readonly exitCode: number; + readonly stdout: string; + readonly stderr: string; +}> { + const child = Bun.spawn(["/usr/bin/git", ...cmd.slice(1)], { + env: gitEnv(), + 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 }; +} + +let home = ""; +function gitEnv(): Record { + return { + PATH: "/usr/bin:/bin", + HOME: home, + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_CONFIG_NOSYSTEM: "1", + }; +} + +const mocks = await Promise.all([ + registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "node:dns/promises", + overrides: { + lookup: async () => { + throw new Error("Fixture DNS is not configured"); + }, + }, + }), + registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "@clack/prompts", + overrides: { + confirm: async () => { + throw new Error( + "confirm() must not be called under HACK_NO_INTERACTIVE=1" + ); + }, + isCancel: () => false, + note: (message: string) => { + noteCalls.push(message); + }, + spinner: () => ({ start: () => {}, stop: () => {} }), + }, + }), + registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "../src/lib/shell.ts", + overrides: { + exec: async (cmd: readonly string[]) => { + if (cmd[0] === "git") { + return await gitExec(cmd); + } + // Docker is reachable, so the Docker-backed --fix steps run too. + if (cmd[0] === "docker" && cmd[1] === "info") { + return { exitCode: 0, stdout: "", stderr: "" }; + } + if ( + cmd[0] === "docker" && + cmd[1] === "network" && + cmd[2] === "inspect" + ) { + return { exitCode: 0, stdout: "[]", stderr: "" }; + } + return { exitCode: 1, stdout: "", stderr: "" }; + }, + execOrThrow: async () => ({ exitCode: 0, stdout: "", stderr: "" }), + run: async (cmd: readonly string[]) => { + runCalls.push([...cmd]); + return 0; + }, + findExecutableInPath: (name?: string) => + name === "hack" || name === "bun" || name === "docker" + ? `/usr/local/bin/${name}` + : null, + CommandError: class CommandError extends Error {}, + }, + }), + registerScopedModuleMock({ + importerPath: import.meta.path, + specifier: "../src/lib/os.ts", + overrides: { + isMac: () => true, + isLinux: () => false, + openUrl: async () => 0, + }, + }), +]); + +const ENV_KEYS = [ + "HOME", + "HACK_HOME", + "HACK_GLOBAL_CONFIG_PATH", + "HACK_NO_INTERACTIVE", + "HACK_MUTAGEN_PATH", + "HACK_LOGGER", +] as const; +let saved: Record = {}; +let h: Awaited>; + +beforeEach(async () => { + saved = Object.fromEntries(ENV_KEYS.map((key) => [key, process.env[key]])); + home = await mkdtemp(join(tmpdir(), "hack-doctor-fix-adopted-")); + h = await retainedSourceBindFixture(); + runCalls.length = 0; + noteCalls.length = 0; +}); + +afterEach(async () => { + for (const mock of mocks) { + mock.deactivate(); + } + resetNoInteractiveFlagForTests(); + for (const key of ENV_KEYS) { + restoreEnv(key, saved[key]); + } + await h.cleanup(); + await rm(home, { recursive: true, force: true }); +}); + +afterAll(() => { + for (const mock of mocks) { + mock.deactivate(); + } +}); + +function git(args: readonly string[]): string { + const result = Bun.spawnSync(["/usr/bin/git", "-C", h.root, ...args], { + env: gitEnv(), + }); + if (result.exitCode !== 0) { + throw new Error(`git ${args[0]} failed in the fixture; output omitted.`); + } + return result.stdout.toString(); +} + +const receiptPath = () => + join(h.root, ".hack/.internal/legacy-compose-adoption-v1/receipt.json"); + +/** + * A committed project env key (the reported case) and a committed generated + * env state file. The key is also an `env key` error found before `--fix` + * runs, so that doctor run still exits 1 after completing every step. + */ +const CASES = [ + { + tracked: ".hack.secret.key", + content: () => `${randomBytes(32).toString("base64url")}\n`, + exit: 1, + rootGitignore: "# project env key\n.hack.secret.key\n", + lastNotes: [ + "Re-run: hack doctor", + "Fix the errors above, then rerun: hack doctor", + ], + }, + { + tracked: ".hack/.env.state.json", + content: () => "{}\n", + exit: 0, + rootGitignore: null, + lastNotes: ["Re-run: hack doctor"], + }, +] as const; + +for (const testCase of CASES) { + test(`doctor --fix in an adopted project untracks a committed ${testCase.tracked} and completes without the native-input refusal (HACK-1311)`, async () => { + await runAdoptedFixCase(testCase); + }, 120_000); +} + +async function runAdoptedFixCase(testCase: (typeof CASES)[number]) { + // A real Git checkout with a committed generated file. + await rm(join(h.root, ".git"), { recursive: true }); + git(["init", "-q"]); + const content = testCase.content(); + const trackedPath = join(h.root, testCase.tracked); + await writeFile(trackedPath, content, { mode: 0o600 }); + git(["add", "-f", testCase.tracked]); + + const store = await h.store(); + try { + const generation = await store.prepare({ binary: h.compiler }); + await store.publish({ generation, binary: h.compiler }); + } finally { + await store.close(); + } + expect( + await inspectLegacyComposeAdoptionSelection({ projectRoot: h.root }) + ).toBe("active"); + const receipt = await readFile(receiptPath(), "utf8"); + const engineCommands = (await h.commands()).length; + + process.env.HOME = home; + process.env.HACK_HOME = join(home, "hack-home"); + Reflect.deleteProperty(process.env, "HACK_GLOBAL_CONFIG_PATH"); + process.env.HACK_NO_INTERACTIVE = "1"; + process.env.HACK_MUTAGEN_PATH = "/usr/local/bin/mutagen"; + process.env.HACK_LOGGER = "console"; + for (const mock of mocks) { + mock.activate(); + } + const { runCli } = await import("../src/cli/run.ts"); + const code = await runCli([ + "doctor", + "--fix", + "--no-interactive", + "--migrate-env-config", + "--path", + h.root, + ]); + for (const mock of mocks) { + mock.deactivate(); + } + + const notes = noteCalls.join("\n"); + expect(notes).not.toContain("E_NATIVE_PROJECT"); + expect(notes).not.toContain(SOURCE_BIND_CANARY); + if (testCase.tracked === ".hack.secret.key") { + expect(notes).not.toContain(content.trim()); + } + expect(code).toBe(testCase.exit); + // The plan and the step name the adopted project's Git-index-only change. + expect(notes).toContain( + `Untrack machine-local generated files from git (git rm --cached; files stay on disk) and leave the adopted project's .hack/.gitignore unchanged: ${testCase.tracked}` + ); + expect(notes).toContain( + "Removed 1 path(s) from the git index. Left .hack/.gitignore unchanged: an adopted Compose project's .hack/ belongs to 'hack config adopt'." + ); + expect(notes).toContain( + "Skip env migration because an adopted Compose project's env config belongs to 'hack config adopt'." + ); + // Every step after the untrack ran: the closing notes follow the last one. + expect(noteCalls.slice(-testCase.lastNotes.length)).toEqual([ + ...testCase.lastNotes, + ]); + + // Index only: the file stays on disk, untracked; the key is ignored at the + // root, and the adoption's .hack/.gitignore is not created. + expect(git(["ls-files"])).toBe(""); + expect(await readFile(trackedPath, "utf8")).toBe(content); + expect( + await readFile(join(h.root, ".gitignore"), "utf8").catch(() => null) + ).toBe(testCase.rootGitignore); + expect(await lstat(join(h.root, ".hack/.gitignore")).catch(() => null)).toBe( + null + ); + + // The adoption is unchanged and no retained container moved. + expect( + await inspectLegacyComposeAdoptionSelection({ projectRoot: h.root }) + ).toBe("active"); + expect(await readFile(receiptPath(), "utf8")).toBe(receipt); + expect((await h.commands()).length).toBe(engineCommands); + expect(await lstat(join(h.outer, "unexpected")).catch(() => null)).toBe(null); + // No project command ran for the adopted project; only global repairs. + for (const call of runCalls) { + expect(call.join(" ")).not.toContain(h.root); + } +} diff --git a/tests/doctor-fix-noninteractive.test.ts b/tests/doctor-fix-noninteractive.test.ts index ce97bcb2a..6b17a1589 100644 --- a/tests/doctor-fix-noninteractive.test.ts +++ b/tests/doctor-fix-noninteractive.test.ts @@ -450,3 +450,32 @@ test("doctor --fix removes a literal Compose env override and keeps a reference- } // Two full doctor --fix runs; the default 5 s budget is too tight on CI. }, 30_000); + +test("doctor --fix leaves a file behind a symlinked .hack/.internal untouched (HACK-1316)", async () => { + const projectRoot = await createProjectWithStaleDaemonPid(); + if (!tempDir) { + throw new Error("tempDir not set"); + } + const canary = "doctor-fix-outside-canary"; + // An outside directory that happens to hold a file named like the override, + // which is not a Compose override Hack can verify (the reported case). + const outside = resolve(tempDir, "outside"); + await mkdir(outside); + const override = resolve(outside, "compose.env.override.yml"); + await writeFile(override, `${canary}\n`); + const internal = resolve(projectRoot, ".hack", ".internal"); + await symlink(outside, internal); + + const { runCli } = await import("../src/cli/run.ts"); + expect( + await runCli(["doctor", "--fix", "--no-interactive", "--path", projectRoot]) + ).toBe(0); + expect(await readFile(override, "utf8")).toBe(`${canary}\n`); + expect(await readdir(outside)).toEqual(["compose.env.override.yml"]); + expect((await lstat(internal)).isSymbolicLink()).toBe(true); + const notes = noteCalls.join("\n"); + // Neither planned nor performed. + expect(notes).not.toContain(".hack/.internal/compose.env.override.yml"); + expect(notes).not.toContain(canary); + expect(notes).not.toContain(outside); +}, 30_000);