diff --git a/docs/cli.md b/docs/cli.md index 93cd37236..070c64630 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -351,7 +351,9 @@ carriers follow only `HACK_NATIVE_STORAGE_TIMEOUT_MS`. Storage proofs that run while a native `up` or `restart` builds and starts workloads do not consume the start budget: their elapsed time extends the start deadline, and each proof stays bounded by its own carrier budget. Each Docker command inside a carrier -keeps its fixed 10–15 second cap. +is bounded by that carrier's remaining budget (reads at most 60 seconds), so a slow +helper start on a loaded host can use the budget rather than a fixed 15-second cap. +Helper cleanup keeps its own 10–60 second bound. The two variables bound different things: diff --git a/docs/env.md b/docs/env.md index a1ff9f68f..408d40f47 100644 --- a/docs/env.md +++ b/docs/env.md @@ -200,9 +200,12 @@ Use `--env=base` to bypass that default and read only `.hack/hack.env.default.ya command that accepts `--env`; an overlay file named after one of those spellings is never selected by name. Any other value must already be a canonical overlay name: lowercase letters and digits joined by single hyphens (`^[a-z0-9]+(-[a-z0-9]+)*$`, for example -`qa` or `qa-2`); surrounding whitespace is ignored. Hack never rewrites a requested name, -so `QA`, `q_a` or `X=1` is refused with a fixed `Invalid --env value.` message that does -not repeat the argument, before any project file is read or any child process starts. +`qa` or `qa-2`), at most 128 characters long; surrounding whitespace is ignored. Hack +never rewrites a requested name, so `QA`, `q_a`, `X=1` or a name longer than 128 +characters is refused with a fixed `Invalid --env value.` message (`E_USAGE` under +`--json`) that does not repeat the argument, before any project file is read or any +child process starts. The limit keeps every file name Hack derives from the overlay +name, such as the layer's lock, within the usual 255-byte file name limit. See the [command and delivery matrix](#command-and-delivery-matrix) for what each command does with the selection. @@ -653,7 +656,28 @@ message in the envelope: you cannot write (for example mode `0555`) both give this code. - `E_NO_SPACE`: the disk or the disk quota is full. - `E_PATH_CONFLICT`: a path is the wrong kind of entry, such as a file where Hack needs - a directory, or a directory where Hack needs a file. + a directory, a directory where Hack needs a file (including a `.hack/.gitignore` that + is a directory or a symlink), or a `.hack/.internal` that `hack env hydrate` finds to + be a symlink (dangling or not) or a file. A file name or path that is too long for + the filesystem is reported with this code too; use a shorter `--env` overlay name or + move the project to a shorter path. +- `E_LOCKED`: the project env key lock (`/.hack.secret.key.lock`, taken + by the first `--secret` value) stayed held for the whole 10-second wait. Nothing was + written. Another hack command may be creating the key; retry when it finishes. If no + other hack command is running, the lock is stale (for example its process ID now + belongs to an unrelated process, which Hack never treats as abandoned): remove the + lock file the message names, then retry. + +The first `--secret` value also works on a filesystem without hard links, such as ExFAT +or FAT on a USB drive. Hack normally publishes the key file and its locks with a hard +link, so they appear complete or not at all. Where that is not possible, it creates an +empty placeholder exclusively and renames the complete file over it, which keeps the +same rules: an existing file or symlink is never replaced or followed, and a reader +never sees a partial file. If Hack is killed between those two steps, an empty +`.hack.secret.key` or lock file is left behind. Hack refuses it rather than use or +replace it; remove it and run the command again. ExFAT and FAT do not store Unix +permissions, so on such a drive the key file is not limited to your user the way mode +`0600` limits it elsewhere. If you omit `key`, `value`, or `--service`, `hack env add` prompts for them interactively. Pass `--no-interactive` (or set `HACK_NO_INTERACTIVE=1`) for scripted/agent usage: the @@ -918,7 +942,7 @@ Write-path guarantees in a linked worktree (first secret added from any checkout - otherwise an existing shared key (git common dir) is reused - otherwise the primary checkout's key is adopted by copying it to the shared location, so sibling checkouts converge on one key - only then is a new key generated, and it is written to the shared location — never silently to the checkout -- concurrent first use from several checkouts converges on one key: when no key exists, creation runs under a cooperative lock at `/.hack.secret.key.lock`, and a key file is published only when complete, never empty or partial (Hack v4 writers do not take the lock) +- concurrent first use from several checkouts converges on one key: when no key exists, creation runs under a cooperative lock at `/.hack.secret.key.lock`, and a key file is published only when complete, never empty or partial (Hack v4 writers do not take the lock; on a filesystem without hard links, such as ExFAT, an empty placeholder is visible for a moment before the complete key replaces it) - if the shared location cannot be resolved (git unavailable) or written, Hack falls back to a checkout-local key and prints a loud divergence warning - `hack doctor` flags divergent `.hack.secret.key` contents across checkouts with the exact paths diff --git a/docs/performance.md b/docs/performance.md index 4953ff2f5..9957f9941 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -743,6 +743,25 @@ more changes that contract, not just its cost: Every command therefore still opens with a full proof. +**Host-side store checks (HACK-1440).** Each proof step also rechecks the private +store on the host: every held store directory gets an `lstat` identity, owner and mode +check, plus a canonical-path check. One xattr-witness `up` made about 3,100 such +rechecks. Two changes cut their file-system calls without dropping a check: + +- A held directory beneath another held directory in the same recheck proves both + paths canonical with one `realpath`, because a symlink in any prefix would show in + the deeper path's result. Every directory keeps its own `lstat` checks. +- A material-authority assertion opens and closes with a full store check, so the + generation load between them no longer adds two more. The generation directory's + own hold, recheck and manifest and document identity checks still run. + +Measured in-process (fake carrier, spied `node:fs/promises`) for the second `up` of +the same-birth xattr case on a macOS host with load average 60–196 on 18 CPUs, four +alternating trials each: `lstat` 18,834 → 13,452, `realpath` 12,821 → 3,685 and +`open` 4,542 → 3,944 calls per `up` (identical in every trial), with median system CPU +2.9 s → 1.9 s. An ordinary (USTAR) witness resume went from 1,298 / 922 / 280 calls to +1,118 / 282 / 260. Wall time on that host was dominated by load and is inconclusive. + **Revisit trigger.** Revisit this limit after the matched benchmark protocol (HACK-1169) is frozen and its first complete revision-1 cohort has run, using that cohort's measured warm startup and restart results. No proof-policy change may be made diff --git a/docs/reference/native-compose-commands.md b/docs/reference/native-compose-commands.md index f5fa69f01..a223c93ef 100644 --- a/docs/reference/native-compose-commands.md +++ b/docs/reference/native-compose-commands.md @@ -287,6 +287,21 @@ Hack does not kill a guessed guest PID or automatically stop the application on client cancellation. Explicit owned `down` stops the containing instance while retaining persistent data. +When the instance has enrolled persistent storage, saved `exec` proves that +storage before and after the guest command. The opening proof may reuse the +proof that command storage preparation just made under the same lease; the +closing proof after the command always runs fresh. Each proof has its own +`HACK_NATIVE_STORAGE_TIMEOUT_MS` carrier budget. The closing proof is +fail-closed: if it refuses (including a storage probe timeout, reported as +`E_COMPOSE_FAILED` with a `storage-*` stage), `exec` fails with that refusal even +though the guest command already ran and printed its output, and the command's +exit code is not reported. A storage result that cannot be proven after the +command must not look like a clean exec. The refusal keeps ownership and saved +state unchanged, and the next admission proves storage again. Retrying `exec` +runs the guest command again, so check its effects first when it is not +idempotent. A cancelled `exec` keeps its known exit code and starts no closing +proof. + `ps.pending` describes journaled engine mutations, not guest exec sessions. A cancelled `exec` can therefore have no pending engine operation even when its guest completion is unknown. An interrupted `run` retains its exact one-off and pending diff --git a/docs/reference/native-compose-storage-xattr-carrier.md b/docs/reference/native-compose-storage-xattr-carrier.md index 7a6cd9470..5e368cb42 100644 --- a/docs/reference/native-compose-storage-xattr-carrier.md +++ b/docs/reference/native-compose-storage-xattr-carrier.md @@ -415,11 +415,27 @@ Budgets are layered and all finite: the start deadline but capped in total at 16 × the storage budget or 1 hour, whichever is smaller. A guard proof that would start after the cap is spent is refused before it runs, and one that ends past it refuses as `storage-budget`. -- Inside a carrier every Docker probe and command keeps a fixed cap, clipped to - the carrier's remaining budget: 15 seconds for reads, target observation and - helper create/start, and 10 seconds for helper removal and cleanup reads, which - are not clipped. A recorded command's 15-second settlement budget follows its - exit. +- Inside a carrier every Docker probe and command is bounded by the carrier's + remaining budget (`nativeComposeStorageProbeTimeoutMs`), not by a fixed cap: + - Volume create and helper create/start get the whole remaining budget, so a + slow but healthy helper start on a loaded host completes while budget + remains, and a hung one times out exactly at the carrier's end. This holds + for recorded original commands too (the readonly `verify` proofs that + `exec` and `up` run): the child, its publication handshake and its identity + reads share that one bound. + - Reads and target observation get the remaining budget, at most 60 seconds + (the ownership probe's own ceiling). + - Helper removal and cleanup reads run even after the carrier's deadline, so + they are never clipped to it. Each has its own bound: the remaining budget + clamped to 10–60 seconds. A hung engine therefore cannot stall cleanup. + + Earlier fixed caps (15 seconds for reads, target observation and helper + create/start; 10 seconds for cleanup) refused healthy `docker start -ai` + helpers on loaded hosted runners as `storage-helper-start`/`probe-timeout` + while most of the 90-second budget remained (HACK-1369). A timeout that ends + before the carrier's deadline still reports `probe-timeout`; one that reaches + it reports `storage-budget`. A recorded command's 15-second settlement budget + follows its exit. - An unrecorded command (an identity read or a legacy root/seed command) waits at most three seconds for its process group to become absent after exit. Like the handshake and settlement budgets, this deadline is monotonic diff --git a/plugins/hack/skills/hack-diagnose/references/error-codes.md b/plugins/hack/skills/hack-diagnose/references/error-codes.md index 309ed4746..77c2fccec 100644 --- a/plugins/hack/skills/hack-diagnose/references/error-codes.md +++ b/plugins/hack/skills/hack-diagnose/references/error-codes.md @@ -26,10 +26,11 @@ stay within the request. | `E_PERMISSION` | A filesystem or runtime permission was denied, or a file Hack must write is on a read-only filesystem. | `hack doctor --json` | | `E_NO_SPACE` | A file Hack must write could not be written because the disk is full or the disk quota is exceeded. | `df -h ` | | `E_PATH_CONFLICT` | A path Hack must read or write is the wrong kind of entry: a file where a directory is needed, or a directory or special file where a regular file is needed. | `ls -ld ` | +| `E_LOCKED` | A lock Hack needs stayed held for the whole bounded wait, by another Hack command or by a stale lock Hack cannot prove abandoned; nothing guarded by it was written. | `ls -l ` | | `E_INTERACTIVE_REQUIRED` | The command needs an answer at a prompt but is running without one. | `hack --help` | | `E_USAGE` | Arguments or flags were invalid for the command. | `hack --help` | | `E_UNEXPECTED` | An unclassified failure; the message is the only detail. | `hack doctor --json` | --- -Content revision: `4c1343788d6a`. Generated by Hack; edit the canonical source and regenerate the plugin. +Content revision: `dc08a206e03d`. Generated by Hack; edit the canonical source and regenerate the plugin. diff --git a/src/agents/error-code-reference.ts b/src/agents/error-code-reference.ts index f7b6c1c6e..09550a489 100644 --- a/src/agents/error-code-reference.ts +++ b/src/agents/error-code-reference.ts @@ -102,6 +102,11 @@ const ERROR_CODE_GUIDES: Readonly> = { "A path Hack must read or write is the wrong kind of entry: a file where a directory is needed, or a directory or special file where a regular file is needed.", readFirst: "`ls -ld `", }, + E_LOCKED: { + meaning: + "A lock Hack needs stayed held for the whole bounded wait, by another Hack command or by a stale lock Hack cannot prove abandoned; nothing guarded by it was written.", + readFirst: "`ls -l `", + }, E_INTERACTIVE_REQUIRED: { meaning: "The command needs an answer at a prompt but is running without one.", diff --git a/src/cli/env-selection.ts b/src/cli/env-selection.ts index c88abf402..88baea5d8 100644 --- a/src/cli/env-selection.ts +++ b/src/cli/env-selection.ts @@ -1,4 +1,7 @@ -import { parseEnvConfigSelection } from "../lib/project.ts"; +import { + ENV_CONFIG_NAME_MAX_LENGTH, + parseEnvConfigSelection, +} from "../lib/project.ts"; import { CliUsageError } from "./command.ts"; /** @@ -6,8 +9,7 @@ import { CliUsageError } from "./command.ts"; * and never the argument, which may be a mistyped `KEY=value` or a pasted * value. */ -export const INVALID_ENV_SELECTION_MESSAGE = - "Invalid --env value. Overlay names must match ^[a-z0-9]+(-[a-z0-9]+)*$; use base, default or none for base layers only."; +export const INVALID_ENV_SELECTION_MESSAGE = `Invalid --env value. Overlay names must match ^[a-z0-9]+(-[a-z0-9]+)*$ and be at most ${ENV_CONFIG_NAME_MAX_LENGTH} characters; use base, default or none for base layers only.`; /** * Resolve a CLI `--env` value through the one shared selection grammar. @@ -16,7 +18,8 @@ export const INVALID_ENV_SELECTION_MESSAGE = * - `null`: base layers only. `base`, `default` and `none` (any case) and an * empty value bypass the configured default overlay. * - `string`: the overlay name, which must already be canonical - * (`ENV_CONFIG_NAME_PATTERN`); surrounding whitespace is ignored. + * (`ENV_CONFIG_NAME_PATTERN`) and at most `ENV_CONFIG_NAME_MAX_LENGTH` + * characters; surrounding whitespace is ignored. * * Anything else, including any value containing `=`, is refused with a fixed, * value-free usage error. A requested name is never slugged: slugging diff --git a/src/lib/cli-result.ts b/src/lib/cli-result.ts index 7b4df9a0c..c4a1076e4 100644 --- a/src/lib/cli-result.ts +++ b/src/lib/cli-result.ts @@ -35,6 +35,7 @@ export const HACK_ERROR_CODES = [ "E_PERMISSION", "E_NO_SPACE", "E_PATH_CONFLICT", + "E_LOCKED", "E_INTERACTIVE_REQUIRED", "E_USAGE", "E_UNEXPECTED", diff --git a/src/lib/compose-startup-budget.ts b/src/lib/compose-startup-budget.ts index dbba03961..c221caab7 100644 --- a/src/lib/compose-startup-budget.ts +++ b/src/lib/compose-startup-budget.ts @@ -79,9 +79,11 @@ export function resolveComposeImageTimeoutMs( * Budget of one native storage carrier (one fresh helper proof session), from * `HACK_NATIVE_STORAGE_TIMEOUT_MS`. It is separate from the start budget: * storage preparation, enrollment and every storage proof use it, never - * `HACK_COMPOSE_STARTUP_TIMEOUT_MS`. Each Docker probe inside a carrier keeps its - * own fixed 10-15 s cap, and an exhausted carrier refuses with the closed - * `storage-budget` effect reason. It grants nothing and relaxes no check. + * `HACK_COMPOSE_STARTUP_TIMEOUT_MS`. Each Docker probe inside a carrier is + * bounded by the carrier's remaining budget (cleanup keeps its own 10-60 s + * bound; see `nativeComposeStorageProbeTimeoutMs`), and an exhausted carrier + * refuses with the closed `storage-budget` effect reason. It grants nothing and + * relaxes no check. */ export function resolveNativeStorageTimeoutMs( opts: { diff --git a/src/lib/exclusive-publish.ts b/src/lib/exclusive-publish.ts new file mode 100644 index 000000000..c06042e0a --- /dev/null +++ b/src/lib/exclusive-publish.ts @@ -0,0 +1,149 @@ +import { constants, type Stats } from "node:fs"; +import { link, lstat, open, rename, unlink } from "node:fs/promises"; +import { isRecord } from "./guards.ts"; + +/** + * Exclusive publication of a complete file at a path that must not already + * exist, for filesystems with and without hard links. + * + * Ownership: the caller owns `source` (a fully written, fsynced sibling it + * created exclusively) and decides what an occupied `target` means. These + * helpers never follow a symlink at `target` and never replace an entry they + * did not create. + */ + +/** + * `link(2)` failures that mean the filesystem cannot make hard links: ExFAT + * and FAT report ENOTSUP on macOS and EPERM on Linux. A real permission + * problem also reports EPERM; the fallback then fails with its own + * permission error, so nothing is hidden. + */ +const HARD_LINK_UNSUPPORTED_CODES: ReadonlySet = new Set([ + "ENOTSUP", + "EOPNOTSUPP", + "EPERM", +]); + +/** Whether a `link(2)` error means the filesystem has no hard links. */ +export function isHardLinkUnsupportedError(error: unknown): boolean { + return ( + isRecord(error) && + typeof error.code === "string" && + HARD_LINK_UNSUPPORTED_CODES.has(error.code) + ); +} + +/** + * Publishes `source` at `target` only when no entry exists at `target`, and + * reports whether this call published it. `source` must be on the same + * filesystem (a sibling). + * + * Primary path: `link(source, target)`, which fails with EEXIST instead of + * replacing anything; `source` keeps its name. When the filesystem has no + * hard links ({@link isHardLinkUnsupportedError}), it falls back to + * {@link publishByExclusivePlaceholder}, after which `source` no longer + * exists. Callers that remove `source` afterwards must tolerate ENOENT. + * + * EEXIST resolves to false; every other error propagates unchanged. + */ +export async function publishFileExclusive(opts: { + readonly source: string; + readonly target: string; +}): Promise { + try { + await link(opts.source, opts.target); + return true; + } catch (error: unknown) { + if (hasCode(error, "EEXIST")) { + return false; + } + if (!isHardLinkUnsupportedError(error)) { + throw error; + } + } + return await publishByExclusivePlaceholder(opts); +} + +/** + * Publication without hard links: reserve `target` with an empty + * O_CREAT|O_EXCL|O_NOFOLLOW file (mode 0600), then rename the complete + * `source` over that reservation. + * + * Guarantees kept from the hard-link path: + * - no overwrite: the exclusive create fails with EEXIST on any existing + * entry, including a dangling symlink, and the rename replaces only the + * empty regular file this call created. It is checked again (still an + * empty regular file with the same identity) immediately before the + * rename; if anything else is there the call reports the path as occupied + * and leaves it alone. + * - no follow: neither O_EXCL|O_NOFOLLOW nor rename(2) follows a symlink at + * the final component. + * - readers see either nothing, the empty reservation, or the complete file, + * never a partial one. + * + * Difference: a process killed between the reservation and the rename + * leaves an empty file at `target`. Callers already treat an empty file as + * invalid, never as theirs, so it is refused rather than adopted or + * reclaimed; it must be removed by hand. A failed rename removes the + * reservation while it is still the empty file this call created. + */ +export async function publishByExclusivePlaceholder(opts: { + readonly source: string; + readonly target: string; +}): Promise { + let reserved: Stats; + try { + const placeholder = await open( + opts.target, + constants.O_CREAT | + constants.O_EXCL | + constants.O_NOFOLLOW | + constants.O_WRONLY, + 0o600 + ); + try { + reserved = await placeholder.stat(); + } finally { + await placeholder.close(); + } + } catch (error: unknown) { + if (hasCode(error, "EEXIST")) { + return false; + } + throw error; + } + if (!(await isOwnReservation({ path: opts.target, reserved }))) { + return false; + } + try { + await rename(opts.source, opts.target); + } catch (error: unknown) { + if (await isOwnReservation({ path: opts.target, reserved })) { + await unlink(opts.target).catch(() => undefined); + } + throw error; + } + return true; +} + +/** + * Whether `path` still names the empty regular file this call reserved. + * Size is checked as well as identity: some filesystems without hard links + * (ExFAT) give every empty file the same inode number. + */ +async function isOwnReservation(opts: { + readonly path: string; + readonly reserved: Stats; +}): Promise { + const current = await lstat(opts.path).catch(() => null); + return ( + current?.isFile() === true && + current.size === 0 && + current.dev === opts.reserved.dev && + current.ino === opts.reserved.ino + ); +} + +function hasCode(error: unknown, code: string): boolean { + return isRecord(error) && error.code === code; +} diff --git a/src/lib/fs.ts b/src/lib/fs.ts index 96bf4114b..6dc4698e0 100644 --- a/src/lib/fs.ts +++ b/src/lib/fs.ts @@ -40,12 +40,22 @@ export async function ensureDir(absoluteDir: string): Promise { * {@link ensureDir} for a write path whose failure the CLI reports: a * directory that cannot be created for one of the {@link FileWriteFailure} * reasons is refused with a closed {@link FileWriteRefusedError} that names - * the directory, never the raw errno text. Other failures propagate as is. + * the directory, never the raw errno text. An entry already at the path that + * is not a directory (a file, or a dangling symlink, which a recursive mkdir + * reports as EEXIST) is refused as `not_a_directory_entry`. Other failures + * propagate as is. */ export async function ensureDirForWrite(absoluteDir: string): Promise { try { await mkdir(absoluteDir, { recursive: true }); } catch (error: unknown) { + if (hasErrorCode(error, "EEXIST")) { + throw new FileWriteRefusedError({ + path: absoluteDir, + failure: "not_a_directory_entry", + operation: "create_directory", + }); + } throw closedFileWriteError({ error, path: absoluteDir, @@ -54,6 +64,56 @@ export async function ensureDirForWrite(absoluteDir: string): Promise { } } +/** + * {@link ensureDirForWrite} for a directory Hack creates files in and that + * must not redirect them: the final component must be a real directory. A + * symlink (dangling or not) or any other entry there is refused with a closed + * `not_a_directory_entry` {@link FileWriteRefusedError} (`E_PATH_CONFLICT`) + * before anything is created through it. Ancestors are not inspected. The + * check is repeated after a create, so an entry that races the mkdir is + * refused too; it is not a fence against a later swap. + */ +export async function ensureRealDirectoryForWrite( + absoluteDir: string +): Promise { + if (await isRealDirectoryOrAbsent(absoluteDir)) { + await ensureDirForWrite(absoluteDir); + if (await isRealDirectoryOrAbsent(absoluteDir)) { + return; + } + } + throw new FileWriteRefusedError({ + path: absoluteDir, + failure: "not_a_directory_entry", + operation: "create_directory", + }); +} + +/** False for a symlink or non-directory entry; lstat failures are closed. */ +async function isRealDirectoryOrAbsent(absoluteDir: string): Promise { + try { + return (await lstat(absoluteDir)).isDirectory(); + } catch (error: unknown) { + if (hasErrorCode(error, "ENOENT")) { + return true; + } + throw closedFileWriteError({ + error, + path: absoluteDir, + operation: "create_directory", + }); + } +} + +function hasErrorCode(error: unknown, code: string): boolean { + return ( + typeof error === "object" && + error !== null && + "code" in error && + (error as { readonly code: unknown }).code === code + ); +} + /** * Filesystem failures that the user, not Hack, must fix before a write can * succeed. Each maps from a fixed set of errno codes; any other error is @@ -64,7 +124,10 @@ export type FileWriteFailure = | "read_only_filesystem" | "no_space" | "not_a_directory" - | "is_a_directory"; + | "is_a_directory" + | "name_too_long" + /** A symlink or non-directory where a directory is needed; never an errno. */ + | "not_a_directory_entry"; const FILE_WRITE_FAILURE_CODES: Readonly> = { EACCES: "permission_denied", @@ -74,12 +137,14 @@ const FILE_WRITE_FAILURE_CODES: Readonly> = { EDQUOT: "no_space", ENOTDIR: "not_a_directory", EISDIR: "is_a_directory", + ENAMETOOLONG: "name_too_long", }; /** * The stable `--json` code of each {@link FileWriteFailure}. A read-only * filesystem is reported as a permission problem; a path of the wrong kind - * (a file where a directory is needed, or the reverse) as a path conflict. + * (a file where a directory is needed, or the reverse) or one too long for + * the filesystem as a path conflict. */ const FILE_WRITE_FAILURE_ERROR_CODES: Readonly< Record @@ -89,6 +154,8 @@ const FILE_WRITE_FAILURE_ERROR_CODES: Readonly< no_space: "E_NO_SPACE", not_a_directory: "E_PATH_CONFLICT", is_a_directory: "E_PATH_CONFLICT", + name_too_long: "E_PATH_CONFLICT", + not_a_directory_entry: "E_PATH_CONFLICT", }; /** The {@link FileWriteFailure} of a filesystem error, or null for any other error. */ @@ -114,6 +181,8 @@ const FILE_WRITE_FAILURE_REASONS: Readonly> = { no_space: "no space is left on the device or the disk quota is exceeded", not_a_directory: "a parent of that path is not a directory", is_a_directory: "that path is a directory", + name_too_long: "a file name or the path is too long for the filesystem", + not_a_directory_entry: "that path is a symlink or not a directory", }; const FILE_WRITE_PERMISSION_REMEDIES: Readonly< @@ -133,6 +202,9 @@ const FILE_WRITE_FAILURE_REMEDIES: Readonly< no_space: "Free some disk space or quota, then retry", not_a_directory: "Replace that path with a directory, then retry", is_a_directory: "Move that directory out of the way, then retry", + name_too_long: + "Use a shorter name or move the project to a shorter path, then retry", + not_a_directory_entry: "Replace it with a regular directory, then retry", }; /** The fixed remedy sentence for a failure; never names a path or an errno. */ @@ -436,6 +508,8 @@ const ATOMIC_WRITE_STAGING_REFUSALS: Readonly< no_space: "no_space", not_a_directory: "not_a_directory", is_a_directory: "not_regular_file", + name_too_long: "name_too_long", + not_a_directory_entry: "not_a_directory", }; /** The closed refusal for a classified staging failure, else `error` as is. */ @@ -462,7 +536,8 @@ export type AtomicWriteRefusal = | "directory_not_writable" | "read_only_filesystem" | "no_space" - | "not_a_directory"; + | "not_a_directory" + | "name_too_long"; const ATOMIC_WRITE_REFUSAL_MESSAGES: Readonly< Record @@ -478,13 +553,14 @@ const ATOMIC_WRITE_REFUSAL_MESSAGES: Readonly< read_only_filesystem: `${FILE_WRITE_FAILURE_REASONS.read_only_filesystem}. ${FILE_WRITE_FAILURE_REMEDIES.read_only_filesystem}`, no_space: `${FILE_WRITE_FAILURE_REASONS.no_space}. ${FILE_WRITE_FAILURE_REMEDIES.no_space}`, not_a_directory: `${FILE_WRITE_FAILURE_REASONS.not_a_directory}. ${FILE_WRITE_FAILURE_REMEDIES.not_a_directory}`, + name_too_long: `${FILE_WRITE_FAILURE_REASONS.name_too_long}. ${FILE_WRITE_FAILURE_REMEDIES.name_too_long}`, }; /** * The stable `--json` code of each {@link AtomicWriteRefusal}, consistent with * {@link FileWriteRefusedError}: access, ownership and read-only refusals are - * `E_PERMISSION`, a full disk is `E_NO_SPACE`, and a path of the wrong kind is - * `E_PATH_CONFLICT`. + * `E_PERMISSION`, a full disk is `E_NO_SPACE`, and a path of the wrong kind + * or one too long for the filesystem is `E_PATH_CONFLICT`. */ const ATOMIC_WRITE_REFUSAL_ERROR_CODES: Readonly< Record @@ -498,6 +574,7 @@ const ATOMIC_WRITE_REFUSAL_ERROR_CODES: Readonly< read_only_filesystem: "E_PERMISSION", no_space: "E_NO_SPACE", not_a_directory: "E_PATH_CONFLICT", + name_too_long: "E_PATH_CONFLICT", }; /** diff --git a/src/lib/native-compose-command-storage.ts b/src/lib/native-compose-command-storage.ts index 424141242..86b666a0f 100644 --- a/src/lib/native-compose-command-storage.ts +++ b/src/lib/native-compose-command-storage.ts @@ -357,9 +357,11 @@ export type NativeComposeCommandStorage = Awaited< /** Saved exec does not publish a ready receipt. The opening check directly * follows command storage preparation under the same lease, with only read-only * source and ownership checks between, so it may reuse that adjacent proof. The - * closing check after the process always performs a fresh proof. Cancellation - * preserves its known exit and starts no late helper; every future admission - * re-verifies content. */ + * closing check after the process always performs a fresh proof and is + * fail-closed: its refusal fails exec although the command already ran, and the + * command's exit code is not returned (docs/reference/native-compose-commands.md). + * Cancellation preserves its known exit and starts no late helper; every future + * admission re-verifies content. */ export async function runNativeComposeStorageVerifiedExec(opts: { readonly signal: AbortSignal; readonly assertSaved: ( diff --git a/src/lib/native-compose-generation.ts b/src/lib/native-compose-generation.ts index 5bf1e1874..8cd0edd4e 100644 --- a/src/lib/native-compose-generation.ts +++ b/src/lib/native-compose-generation.ts @@ -1562,14 +1562,27 @@ export async function openNativeComposeGenerationStore(opts: { readonly anchor: ProjectionAnchor; } >(); + /** + * `bracketed` is only for a caller that completed a full store `check()` + * earlier in the same proof step, with nothing but private reads since, + * and that runs another full `check()` after this load before any write, + * hook, engine or storage effect. Those caller checks then bracket the + * generation reads, so this load's own leading and trailing store checks + * would only repeat them. The generation directory's own hold and + * recheck, and every manifest and document identity check, always run + * (HACK-1440). + */ const load = async ( generationId: string, - expected?: GenerationAnchor + expected?: GenerationAnchor, + bracketed = false ): Promise => { if (!TOKEN.test(generationId)) { refuse(); } - await check(); + if (!bracketed) { + await check(); + } const generationRoot = join(generationsRoot, generationId); const held = await holdDirectory(generationRoot, true); try { @@ -1603,7 +1616,9 @@ export async function openNativeComposeGenerationStore(opts: { refuse(); } await recheckDirectories([held]); - await check(); + if (!bracketed) { + await check(); + } const generation = Object.freeze( Object.defineProperty( { @@ -1624,7 +1639,11 @@ export async function openNativeComposeGenerationStore(opts: { await held.file.close(); } }; - const verifyGeneration = async (generation: NativeComposeGeneration) => { + /** `bracketed` has the same narrow contract as in `load`. */ + const verifyGeneration = async ( + generation: NativeComposeGeneration, + bracketed = false + ) => { const manifest = known.get(generation); if (!manifest) { refuse(); @@ -1633,7 +1652,7 @@ export async function openNativeComposeGenerationStore(opts: { if (!anchor) { refuse(); } - const loaded = await load(generation.generationId, anchor); + const loaded = await load(generation.generationId, anchor, bracketed); if (JSON.stringify(known.get(loaded)) !== JSON.stringify(manifest)) { refuse(); } @@ -1988,7 +2007,9 @@ export async function openNativeComposeGenerationStore(opts: { ) { return refuse(); } - await verifyGeneration(selection.generation); + // Bracketed by the material assertion's leading and closing + // `lease.assertHeld()`, each of which runs a full store check. + await verifyGeneration(selection.generation, true); const anchor = knownAnchor(selection.generation); if ( !( diff --git a/src/lib/native-compose-private-state.ts b/src/lib/native-compose-private-state.ts index 2da0f32ca..1a6c99ac6 100644 --- a/src/lib/native-compose-private-state.ts +++ b/src/lib/native-compose-private-state.ts @@ -440,6 +440,18 @@ export async function holdDirectory( throw error; } } +/** + * Recheck that every held path still names its held, safe directory. + * + * Each directory gets its own `lstat` identity, owner and mode check. The + * canonical-path (`realpath`) check runs only for a directory that no other + * directory in the same batch lies beneath. Held paths are canonical when held, + * so `realpath(child) === child` at one instant proves that every proper prefix + * of `child`, including each held ancestor, was also canonical (no symlink + * component) at that instant: any symlink in a prefix would appear resolved + * in the child's result. Both forms prove canonicality at some instant inside + * this one recheck; neither was ever atomic with the `lstat` (HACK-1440). + */ export async function recheckDirectories( directories: readonly HeldDirectory[] ): Promise { @@ -450,12 +462,26 @@ export async function recheckDirectories( named.uid !== process.getuid?.() || (named.mode & 0o022) !== 0 || (held.private && (named.mode & 0o777) !== 0o700) || - (await realpath(held.path)) !== held.path + (!canonicalCoveredByDescendant(held, directories) && + (await realpath(held.path)) !== held.path) ) { refuse(); } } } +/** Whether another directory in this batch lies strictly beneath `held`. */ +function canonicalCoveredByDescendant( + held: HeldDirectory, + directories: readonly HeldDirectory[] +): boolean { + const prefix = held.path.endsWith("/") ? held.path : `${held.path}/`; + return directories.some( + (other) => + other !== held && + other.path.length > prefix.length && + other.path.startsWith(prefix) + ); +} export async function privateDirectory(path: string): Promise { try { await mkdir(path, { mode: 0o700 }); diff --git a/src/lib/native-compose-storage-witness-docker.ts b/src/lib/native-compose-storage-witness-docker.ts index 4c83a0d97..666ba8f8d 100644 --- a/src/lib/native-compose-storage-witness-docker.ts +++ b/src/lib/native-compose-storage-witness-docker.ts @@ -116,6 +116,50 @@ const CAPTURE_LIMIT = 4096; * uncertainty (HACK-1281). It stays finite and grants nothing. */ const SETTLEMENT_BUDGET_MS = 15_000; +/** + * Floor of every cleanup step of one carrier: cleanup reads after the helper + * exits and helper removal. Cleanup runs even after the carrier's own deadline + * has passed (it is never clipped to it), so this floor keeps a just-expired + * carrier able to remove its helper. + */ +export const NATIVE_STORAGE_CLEANUP_FLOOR_MS = 10_000; +/** + * Ceiling of one Docker read probe (`createNativeComposeProbe` accepts at most + * 60 s) and of every cleanup step, so a hung engine can stall neither a read + * nor cleanup beyond it. + */ +export const NATIVE_STORAGE_PROBE_CEILING_MS = 60_000; +/** + * Timeout of one Docker probe or command inside a storage carrier, from the + * carrier's remaining `HACK_NATIVE_STORAGE_TIMEOUT_MS` budget (HACK-1369). + * Earlier fixed 15 s/10 s caps refused a healthy but slow helper start on + * loaded hosts as `probe-timeout` while most of the 90 s budget remained. + * - `command` (volume create, helper create/start): the remaining budget, so a + * timeout still fires exactly at the carrier's end. + * - `read` (reads and target observation): the remaining budget, at most + * {@link NATIVE_STORAGE_PROBE_CEILING_MS}. + * - `cleanup` (post-exit reads and helper removal): its own bound, the remaining + * budget clamped to [{@link NATIVE_STORAGE_CLEANUP_FLOOR_MS}, + * {@link NATIVE_STORAGE_PROBE_CEILING_MS}], never clipped to the deadline. + * Every value is finite. A non-cleanup value at or below zero is refused by the + * probe or command before it spawns; it grants nothing and relaxes no check. + */ +export function nativeComposeStorageProbeTimeoutMs(opts: { + readonly kind: "read" | "command" | "cleanup"; + readonly deadline: number; + readonly now?: number; +}): number { + const remaining = opts.deadline - (opts.now ?? Date.now()); + if (opts.kind === "cleanup") { + return Math.min( + NATIVE_STORAGE_PROBE_CEILING_MS, + Math.max(NATIVE_STORAGE_CLEANUP_FLOOR_MS, remaining) + ); + } + return opts.kind === "read" + ? Math.min(NATIVE_STORAGE_PROBE_CEILING_MS, remaining) + : remaining; +} // A late/unconfirmed publication cannot release its original held directory FDs. const unsettledCommandFiles = new Set(); type Selection = { readonly name: string; readonly storage: string }; @@ -599,7 +643,10 @@ async function savedCarrierRecovery(opts: { await guard(); const result = await createNativeComposeProbe({ signal: context.signal, - timeoutMs: Math.min(15_000, context.deadline - Date.now()), + timeoutMs: nativeComposeStorageProbeTimeoutMs({ + kind: "read", + deadline: context.deadline, + }), })(args); await guard(); return result; @@ -1462,9 +1509,10 @@ export async function createNativeComposeDockerStorageXattrCarrier( lifetime(context); } const signal = cleanup ? undefined : context.signal; - const timeoutMs = cleanup - ? 10_000 - : Math.min(15_000, context.deadline - Date.now()); + const timeoutMs = nativeComposeStorageProbeTimeoutMs({ + kind: cleanup ? "cleanup" : "read", + deadline: context.deadline, + }); return nativeComposeStorageIdentityProbe({ probe: createNativeComposeProbe({ signal, timeoutMs }), signal, @@ -1687,7 +1735,10 @@ export async function createNativeComposeDockerStorageXattrCarrier( `io.hack.native-config.storage-provision=${nonce}`, selection.name, ], - timeoutMs: 15_000, + timeoutMs: nativeComposeStorageProbeTimeoutMs({ + kind: "command", + deadline: context.deadline, + }), beforeSpawn: () => { tool(); lifetime(context); @@ -1825,7 +1876,10 @@ export async function createNativeComposeDockerStorageXattrCarrier( program: files.program, }), ], - timeoutMs: 15_000, + timeoutMs: nativeComposeStorageProbeTimeoutMs({ + kind: "command", + deadline: context.deadline, + }), beforeSpawn: () => { tool(); lifetime(context); @@ -1947,7 +2001,10 @@ export async function createNativeComposeDockerStorageXattrCarrier( } : undefined, args: [docker, "start", "-ai", id], - timeoutMs: 15_000, + timeoutMs: nativeComposeStorageProbeTimeoutMs({ + kind: "command", + deadline: context.deadline, + }), beforeSpawn: () => { tool(); lifetime(context); @@ -2037,7 +2094,10 @@ export async function createNativeComposeDockerStorageXattrCarrier( : undefined, cleanup: true, args: [docker, "rm", id], - timeoutMs: 10_000, + timeoutMs: nativeComposeStorageProbeTimeoutMs({ + kind: "cleanup", + deadline: context.deadline, + }), beforeSpawn: tool, assertAdmitted: async () => { const finalCarrier = await inspectCarrier(true); diff --git a/src/lib/private-file.ts b/src/lib/private-file.ts index 1533f43cb..307034a52 100644 --- a/src/lib/private-file.ts +++ b/src/lib/private-file.ts @@ -2,7 +2,6 @@ import { randomUUID } from "node:crypto"; import { constants, type Stats } from "node:fs"; import { type FileHandle, - link, lstat, open, realpath, @@ -10,6 +9,7 @@ import { unlink, } from "node:fs/promises"; import { basename, dirname, join } from "node:path"; +import { publishFileExclusive } from "./exclusive-publish.ts"; import { closedFileWriteError } from "./fs.ts"; import { isRecord } from "./guards.ts"; @@ -420,10 +420,17 @@ export async function replacePrivateFileAtomic( * The complete contents are staged in a private (0600, fsynced) sibling and * published with link(2), which fails with EEXIST instead of replacing an * existing entry. Unlike {@link createPrivateFileExclusive}, readers never see - * an empty or partial file, so a concurrent loser can adopt the winner's file + * a partial file, so a concurrent loser can adopt the winner's file * immediately. An existing regular file returns false untouched; a symlink * (dangling or not), a non-regular entry, or a symlinked or replaced directory * from `directories` throws `refusal()`. The staging file is always removed. + * + * On a filesystem without hard links (ExFAT, FAT) the sibling is renamed over + * an exclusive, no-follow empty reservation instead (see + * `publishByExclusivePlaceholder`): an existing entry is still never replaced + * or followed, and readers see nothing, an empty file or the complete file. + * A process killed between the two steps leaves an empty file, which is + * refused, never adopted, until it is removed. */ export async function publishPrivateFileExclusive( opts: ReplaceablePrivateFileOptions & { readonly text: string } @@ -444,13 +451,11 @@ export async function publishPrivateFileExclusive( await file.writeFile(opts.text, "utf8"); await file.sync(); await recheckRealDirectories({ directories, refusal: opts.refusal }); - try { - await link(staging, opts.path); - published = true; - } catch (error: unknown) { - if (!(isRecord(error) && error.code === "EEXIST")) { - throw error; - } + published = await publishFileExclusive({ + source: staging, + target: opts.path, + }); + if (!published) { // Lost the race: classify what is there now (regular file or refusal). await observeReplaceableTarget(opts); } diff --git a/src/lib/project-env-config.ts b/src/lib/project-env-config.ts index 0c03f9cbd..7539c46b2 100644 --- a/src/lib/project-env-config.ts +++ b/src/lib/project-env-config.ts @@ -26,7 +26,7 @@ import { HACK_DIR_GITIGNORE_END_MARKER, HACK_DIR_GITIGNORE_ENTRIES, } from "../templates.ts"; -import { ClosedCliError } from "./cli-result.ts"; +import { ClosedCliError, HackCliError } from "./cli-result.ts"; import { parseDotEnv, serializeDotEnv } from "./env.ts"; import { withEnvLayerLock } from "./env-layer-lock.ts"; import { @@ -35,6 +35,7 @@ import { ensureDir, ensureDirForWrite, ensureGitignoreEntry, + ensureRealDirectoryForWrite, mergeManagedGitignoreBlock, pathExists, readAtomicWriteTargetText, @@ -79,7 +80,10 @@ import { assertLegacyProjectInputFamily, ProjectInputSelectionError, } from "./project-input-selection.ts"; -import { withProjectsRegistryLock } from "./projects-registry-lock.ts"; +import { + ProjectsRegistryLockTimeoutError, + withProjectsRegistryLock, +} from "./projects-registry-lock.ts"; import { isNativeManagedLocalTracked, resolvePrimaryLocalProjectDir, @@ -592,12 +596,13 @@ export const HACK_DIR_GITIGNORE_REFUSAL = /** * Refusal to read or write `.hack/.gitignore` through a symlink or a - * non-regular entry. Names no path or contents. Closed: the CLI prints its - * message as one line, never with a stack. + * non-regular entry (for example a directory). Names no path or contents. + * Closed with `E_PATH_CONFLICT`: the CLI prints it as one `CODE: message` + * line, never with a stack, and `--json` reports the same code. */ -export class HackDirGitignoreRefusalError extends ClosedCliError { +export class HackDirGitignoreRefusalError extends HackCliError { constructor() { - super(HACK_DIR_GITIGNORE_REFUSAL); + super({ code: "E_PATH_CONFLICT", message: HACK_DIR_GITIGNORE_REFUSAL }); this.name = "HackDirGitignoreRefusalError"; } } @@ -1489,10 +1494,68 @@ function nativeEnvValuesError(): Error { ); } -function nativeEnvMetadataError(): Error { - return new Error( +const NATIVE_ENV_DIAGNOSTIC_PROPERTY = "diagnosticCause"; +const NATIVE_ENV_DIAGNOSTIC_NAME = /^[A-Za-z][A-Za-z0-9]{0,63}$/; +const NATIVE_ENV_DIAGNOSTIC_CODE = /^[A-Z][A-Z0-9_]{0,63}$/; +const NATIVE_ENV_DIAGNOSTIC = + /^[A-Za-z][A-Za-z0-9]{0,63}(?::[A-Z][A-Z0-9_]{0,63})?$/; + +/** + * Closed metadata refusal. It keeps only a value-free diagnostic of what failed: + * the underlying error's class name and identifier-shaped code (for example + * `HackCliError:E_CONFIG_INVALID` or `Error:ENOENT`), never its message, paths, + * bytes or values. The diagnostic is a non-enumerable own property rather than + * `cause`, so serializers, reports and the public message stay unchanged. + */ +function nativeEnvMetadataError(diagnostic = "NativeEnvMetadataError"): Error { + const refusal = new Error( "Cannot resolve native managed env metadata: selected inputs are invalid, unreadable, unstable or oversized; values omitted." ); + Object.defineProperty(refusal, NATIVE_ENV_DIAGNOSTIC_PROPERTY, { + value: diagnostic, + enumerable: false, + }); + return refusal; +} + +/** Refuse with the cause's value-free diagnostic; a nested refusal keeps its own. */ +function nativeEnvMetadataRefusal(cause: unknown): Error { + return nativeEnvMetadataError( + nativeEnvMetadataDiagnostic(cause) ?? valueFreeErrorDiagnostic(cause) + ); +} + +function valueFreeErrorDiagnostic(error: unknown): string { + if (!(error instanceof Error)) { + return error === null ? "null" : typeof error; + } + const name = NATIVE_ENV_DIAGNOSTIC_NAME.test(error.name) + ? error.name + : "Error"; + const code: unknown = Reflect.get(error, "code"); + return typeof code === "string" && NATIVE_ENV_DIAGNOSTIC_CODE.test(code) + ? `${name}:${code}` + : name; +} + +/** + * Read the value-free diagnostic of a native metadata refusal, for tests and + * debugging. Returns undefined for any other error or a malformed diagnostic. + */ +export function nativeEnvMetadataDiagnostic( + error: unknown +): string | undefined { + if (!(error instanceof Error)) { + return undefined; + } + const diagnostic: unknown = Reflect.get( + error, + NATIVE_ENV_DIAGNOSTIC_PROPERTY + ); + return typeof diagnostic === "string" && + NATIVE_ENV_DIAGNOSTIC.test(diagnostic) + ? diagnostic + : undefined; } function validateNativeHostTargets( @@ -1832,7 +1895,7 @@ export async function resolveProjectEnvMetadataForNativeSelection( ) { throw error; } - throw nativeEnvMetadataError(); + throw nativeEnvMetadataRefusal(error); } } @@ -3337,6 +3400,7 @@ async function readExistingProjectEnvKeyFileText(opts: { /** * Takes the project's hydration lock: `.internal/env-hydrate.lock`, created * exclusively (no symlink follow, mode 0600) and holding the owner's PID. + * `.internal` itself must be a real directory (or absent, then created). * * An existing lock is never removed or taken over, because a second process * cannot prove the first one is gone without racing a third. It refuses as @@ -3350,7 +3414,9 @@ async function acquireProjectEnvHydrationLock(opts: { readonly lockPath: string; readonly refuse: ProjectEnvHydrationRefuse; }): Promise<() => Promise> { - await ensureDirForWrite(dirname(opts.lockPath)); + // A symlinked or non-directory `.internal` (including a dangling link) is + // refused as E_PATH_CONFLICT; the lock is never created through it. + await ensureRealDirectoryForWrite(dirname(opts.lockPath)); await assertLegacyProjectDirectory({ projectDir: opts.projectDir }); try { await createPrivateFileExclusive({ @@ -3987,9 +4053,10 @@ export async function ensureProjectEnvSecretKey(opts: { // git common dir serializes creation; under it the full lookup runs again // and adopts a key another checkout created meanwhile. let started = false; + const lockPath = `${sharedLocation.path}.lock`; try { return await withProjectsRegistryLock({ - lockPath: `${sharedLocation.path}.lock`, + lockPath, label: "project env key", run: async () => { started = true; @@ -4000,6 +4067,12 @@ export async function ensureProjectEnvSecretKey(opts: { if (started) { throw error; } + if (error instanceof ProjectsRegistryLockTimeoutError) { + throw new ProjectEnvKeyLockTimeoutError({ + lockPath, + recoveryBusy: error.recoveryBusy, + }); + } if (!isProjectEnvKeyLockUnavailable(error)) { // Any other filesystem failure the user must fix (a full disk or quota, // a non-directory on the way) is refused as the key it blocked, never @@ -4018,6 +4091,29 @@ export async function ensureProjectEnvSecretKey(opts: { } } +/** + * The project env key lock stayed held for the whole bounded wait: another + * hack command is creating the key, or a lock was left behind by a process + * whose PID is now reused (which is never reclaimed automatically), or an + * interrupted recovery left its guard. Closed (`E_LOCKED`) and value-free: it + * names the lock path and the remedy only. Nothing was written. + */ +export class ProjectEnvKeyLockTimeoutError extends HackCliError { + constructor(opts: { + readonly lockPath: string; + readonly recoveryBusy: boolean; + }) { + const stale = opts.recoveryBusy + ? `${opts.lockPath} and the directory ${opts.lockPath}.recovery` + : opts.lockPath; + super({ + code: "E_LOCKED", + message: `Timed out waiting for the project env key lock ${opts.lockPath}; nothing was written. Another hack command may be creating the key: retry when it finishes. If no other hack command is running, the lock is stale: remove ${stale}, then retry.`, + }); + this.name = "ProjectEnvKeyLockTimeoutError"; + } +} + function isProjectEnvKeyLockUnavailable(error: unknown): boolean { return ( isRecord(error) && diff --git a/src/lib/project.ts b/src/lib/project.ts index 29305f16b..a6b223413 100644 --- a/src/lib/project.ts +++ b/src/lib/project.ts @@ -135,6 +135,14 @@ export const ENV_BASE_SELECTION_ALIASES: ReadonlySet = new Set([ */ export const ENV_CONFIG_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; +/** + * Longest accepted overlay name. An overlay name becomes part of several file + * names in one directory; the longest is the env layer lock's staging name, + * `hack.env..local.yaml.lock..owner` (the name plus 68 bytes), + * which must fit the common 255-byte file name limit with room to spare. + */ +export const ENV_CONFIG_NAME_MAX_LENGTH = 128; + /** * Parse a requested overlay selection (CLI `--env`, daemon `env`). * @@ -142,7 +150,8 @@ export const ENV_CONFIG_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; * - `null`: base layers only (empty, or a base alias in any case). * - `string`: the overlay name, exactly as given apart from surrounding * whitespace. - * - `undefined` for a non-empty input: refused. A requested name is never + * - `undefined` for a non-empty input that is not a canonical overlay name + * or is longer than {@link ENV_CONFIG_NAME_MAX_LENGTH}: refused. A requested name is never * slugged, so `X=secret` or `QA` cannot silently become `xsecret` or `qa` * and then be printed or written into an overlay file name. Callers must * refuse with fixed text that does not include the input. @@ -163,7 +172,10 @@ export function parseEnvConfigSelection( return null; } - return ENV_CONFIG_NAME_PATTERN.test(trimmed) ? trimmed : undefined; + return trimmed.length <= ENV_CONFIG_NAME_MAX_LENGTH && + ENV_CONFIG_NAME_PATTERN.test(trimmed) + ? trimmed + : undefined; } export function sanitizeBranchSlug(input: string): string { diff --git a/src/lib/projects-registry-lock.ts b/src/lib/projects-registry-lock.ts index e7e70fe8a..97ad17112 100644 --- a/src/lib/projects-registry-lock.ts +++ b/src/lib/projects-registry-lock.ts @@ -2,7 +2,6 @@ import { randomUUID } from "node:crypto"; import { constants, type Stats } from "node:fs"; import { type FileHandle, - link, lstat, mkdir, open, @@ -11,6 +10,7 @@ import { unlink, } from "node:fs/promises"; import { setTimeout as delay } from "node:timers/promises"; +import { publishFileExclusive } from "./exclusive-publish.ts"; import { isRecord } from "./guards.ts"; const WAIT_MS = 10_000; @@ -20,11 +20,16 @@ const OWNER_PATTERN = /^([1-9]\d*)\n(?:[a-f0-9-]{36}\n)?$/; /** * The bounded wait for a cooperative lock ran out while another live writer * (or an interrupted recovery) held it. Nothing guarded by the lock ran. + * `recoveryBusy` is true when the recovery guard (`.recovery`) was held + * at the end of the wait, which only offline inspection can clear. */ export class ProjectsRegistryLockTimeoutError extends Error { - constructor(message: string) { + readonly recoveryBusy: boolean; + + constructor(message: string, opts: { readonly recoveryBusy?: boolean } = {}) { super(message); this.name = "ProjectsRegistryLockTimeoutError"; + this.recoveryBusy = opts.recoveryBusy ?? false; } } @@ -37,6 +42,11 @@ interface Owner { /** * Serialize cooperative registry writers. A fully written owner is published by * an exclusive hard link, so a killed publisher never leaves an empty lock. + * On a filesystem without hard links (ExFAT, FAT) the owner is published by an + * exclusive empty reservation that the complete owner is renamed over; see + * `publishByExclusivePlaceholder`. There, a publisher killed between the two + * steps leaves an empty lock, which is never reclaimed (it names no owner) and + * times out until it is removed by hand. * Age is not authority: only ESRCH permits reclamation, under a separate guard. * Live/reused/inaccessible PIDs and uncertain receipts are never reclaimed. * @@ -66,8 +76,9 @@ export async function withProjectsRegistryLock(opts: { await acquire({ ...opts, candidate }); try { // The descriptor stays open until release, preventing inode reuse while a - // caller still holds its receipt. Remove the staging name before running. - await unlink(candidate); + // caller still holds its receipt. Remove the staging name before running; + // the no-hard-link fallback has already renamed it into place. + await removeAbsentOkay(candidate); opts.signal?.throwIfAborted(); return await opts.run(); } finally { @@ -135,7 +146,8 @@ async function acquire(opts: { ? "; recovery guard is busy (interrupted recovery requires offline inspection)" : ""; throw new ProjectsRegistryLockTimeoutError( - `Timed out waiting for ${opts.label ?? "projects registry"} lock${recovery}` + `Timed out waiting for ${opts.label ?? "projects registry"} lock${recovery}`, + { recoveryBusy } ); } // Jitter prevents a burst of CLI clients from retrying in lockstep. @@ -146,15 +158,7 @@ async function acquire(opts: { } async function publish(candidate: string, path: string): Promise { - try { - await link(candidate, path); - return true; - } catch (error) { - if (hasCode(error, "EEXIST")) { - return false; - } - throw error; - } + return await publishFileExclusive({ source: candidate, target: path }); } function hasCode(error: unknown, code: string): boolean { diff --git a/tests/fixtures/staging-errno-preload.ts b/tests/fixtures/staging-errno-preload.ts index f3fec9555..902a56885 100644 --- a/tests/fixtures/staging-errno-preload.ts +++ b/tests/fixtures/staging-errno-preload.ts @@ -9,9 +9,10 @@ import * as fs from "node:fs/promises"; * would. No real disk is filled. * * - `HACK_TEST_ERRNO_STEP` picks the step (default `open`): `open`, `access`, - * `lstat` or `rename` of a matching path (the rename's source), `write` or - * `sync` on a handle opened at a matching path, or `bun-write` (`Bun.write`) - * to a matching path. + * `lstat` or `rename` of a matching path (the rename's source), `link` from + * a matching source (for example ENOTSUP, as on a filesystem without hard + * links such as ExFAT), `write` or `sync` on a handle opened at a matching + * path, or `bun-write` (`Bun.write`) to a matching path. * - `HACK_TEST_ERRNO_PATH` is a regular expression a path must match (default * `\.tmp$`, an atomic-write staging sibling). * @@ -28,6 +29,7 @@ const STEPS = [ "access", "lstat", "rename", + "link", "write", "sync", "bun-write", @@ -53,6 +55,7 @@ const realOpen = fs.open; const realAccess = fs.access; const realLstat = fs.lstat; const realRename = fs.rename; +const realLink = fs.link; type Handle = Awaited>; @@ -98,6 +101,12 @@ mock.module("node:fs/promises", () => ({ } return await realRename(...args); }, + link: async (...args: Parameters) => { + if (step === "link" && matches(args[0])) { + throw injected("link", String(args[0])); + } + return await realLink(...args); + }, })); if (step === "bun-write") { diff --git a/tests/native-compose-held-directory-recheck.test.ts b/tests/native-compose-held-directory-recheck.test.ts new file mode 100644 index 000000000..0574b03f2 --- /dev/null +++ b/tests/native-compose-held-directory-recheck.test.ts @@ -0,0 +1,96 @@ +import { afterEach, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import { + mkdir, + mkdtemp, + realpath, + rename, + rm, + symlink, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + type HeldDirectory, + holdDirectory, + recheckDirectories, +} from "../src/lib/native-compose-private-state.ts"; + +const roots: string[] = []; +const held: HeldDirectory[] = []; +afterEach(async () => { + await Promise.all(held.splice(0).map((directory) => directory.file.close())); + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })) + ); +}); +async function tree(...paths: readonly string[]) { + const root = await realpath(await mkdtemp(join(tmpdir(), "native-recheck-"))); + roots.push(root); + for (const path of paths) { + await mkdir(join(root, path), { recursive: true }); + } + return root; +} +async function hold(...paths: readonly string[]) { + const directories: HeldDirectory[] = []; + for (const path of paths) { + const directory = await holdDirectory(path, false); + held.push(directory); + directories.push(directory); + } + return directories; +} +async function realpathCalls(run: () => Promise) { + const original = fs.realpath; + let calls = 0; + const spy = spyOn(fs, "realpath").mockImplementation((( + ...args: Parameters + ) => { + calls++; + return original(...args); + }) as typeof original); + try { + await run(); + } finally { + spy.mockRestore(); + } + return calls; +} + +test("a held chain proves canonical paths with one realpath at its deepest directory", async () => { + const root = await tree("p/a/b/c", "p/a/bc"); + const chain = await hold( + join(root, "p/a"), + join(root, "p/a/b"), + join(root, "p/a/b/c") + ); + expect(await realpathCalls(() => recheckDirectories(chain))).toBe(1); + // A name that only shares a string prefix is not beneath its neighbour. + const prefixOnly = await hold(join(root, "p/a/b"), join(root, "p/a/bc")); + expect(await realpathCalls(() => recheckDirectories(prefixOnly))).toBe(2); +}); + +test("a symlink above a held chain still refuses through its deepest directory", async () => { + const root = await tree("p/a/b"); + const chain = await hold(join(root, "p/a"), join(root, "p/a/b")); + // Each held path still lstats to its held inode; only a canonical-path + // check can see the substituted symlink component above them. + await rename(join(root, "p"), join(root, "moved")); + await symlink(join(root, "moved"), join(root, "p")); + for (const batch of [chain, [chain[0] as HeldDirectory]]) { + await expect(recheckDirectories(batch)).rejects.toMatchObject({ + code: "E_NATIVE_COMPOSE_STATE", + }); + } +}); + +test("a held ancestor replaced by a symlink refuses", async () => { + const root = await tree("p/a/b"); + const chain = await hold(join(root, "p/a"), join(root, "p/a/b")); + await rename(join(root, "p/a"), join(root, "p/moved")); + await symlink(join(root, "p/moved"), join(root, "p/a")); + await expect(recheckDirectories(chain)).rejects.toMatchObject({ + code: "E_NATIVE_COMPOSE_STATE", + }); +}); diff --git a/tests/native-compose-material-authority.test.ts b/tests/native-compose-material-authority.test.ts index 52ea21441..e95203a7e 100644 --- a/tests/native-compose-material-authority.test.ts +++ b/tests/native-compose-material-authority.test.ts @@ -1,4 +1,5 @@ -import { afterEach, expect, test } from "bun:test"; +import { afterEach, expect, spyOn, test } from "bun:test"; +import * as fs from "node:fs/promises"; import { mkdir, mkdtemp, @@ -271,6 +272,89 @@ test.each([ }); }); +test.each([ + "receipt", + "document", +] as const)("a store directory swapped during the %s read of a generation assertion refuses at its closing check", async (point) => { + const store = await fixture(); + const generation = await store.withMutation(async (mutation) => { + const published = await publish(mutation); + await mutation.runEffect({ + generation: published, + operation: "up", + assertFresh: async () => {}, + assertOwned: async () => {}, + effect: async () => ({ outcome: "complete", value: 0 }), + }); + return published; + }); + const instanceRoot = dirname(receiptPath(store)); + const leases = join(instanceRoot, "leases"); + const target = + point === "receipt" + ? receiptPath(store) + : join( + instanceRoot, + "generations", + generation.generationId, + "compose.json" + ); + const checkoutRoot = store.identity.checkoutRoot; + const original = { open: fs.open, lstat: fs.lstat }; + let armed = false; + let swap = false; + let swapped = false; + let storeChecks = 0; + const openSpy = spyOn(fs, "open").mockImplementation((async ( + ...args: Parameters + ) => { + if (swap && !swapped && args[0] === target) { + swapped = true; + await rename(leases, `${leases}.moved`); + await mkdir(leases, { mode: 0o700 }); + } + return await original.open(...args); + }) as typeof original.open); + const lstatSpy = spyOn(fs, "lstat").mockImplementation((( + ...args: Parameters + ) => { + // Every full store check lstats the checkout root exactly once. + if (armed && args[0] === checkoutRoot) { + storeChecks++; + } + return original.lstat(...args); + }) as typeof original.lstat); + try { + await store.withMutation(async (mutation) => { + const select = { + authority: mutation.materialAuthority, + generation, + phase: "inspect" as const, + }; + // One assertion: the leading and closing lease checks bracket its + // generation load, which adds no store check of its own (HACK-1440). + armed = true; + await assertNativeComposeMaterialAuthority(select); + armed = false; + expect(storeChecks).toBe(2); + // A swap after the leading check is refused by the closing one. + swap = true; + await expect( + assertNativeComposeMaterialAuthority(select) + ).rejects.toMatchObject({ code: "E_NATIVE_COMPOSE_STATE" }); + swap = false; + expect(swapped).toBe(true); + await rm(leases, { recursive: true }); + await rename(`${leases}.moved`, leases); + const binding = await assertNativeComposeMaterialAuthority(select); + expect(binding.generationId).toBe(generation.generationId); + }); + } finally { + openSpy.mockRestore(); + lstatSpy.mockRestore(); + } +}); + test("saved mutation uses immutable generation despite missing authored source and cannot prepare", async () => { const initial = await fixture(); await initial.withMutation(async (mutation) => { diff --git a/tests/native-compose-storage-probe-budget.test.ts b/tests/native-compose-storage-probe-budget.test.ts new file mode 100644 index 000000000..08f76dd14 --- /dev/null +++ b/tests/native-compose-storage-probe-budget.test.ts @@ -0,0 +1,841 @@ +import { afterEach, beforeEach, expect, mock, spyOn, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { + chmod, + mkdir, + mkdtemp, + readFile, + realpath, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { basename, dirname, join } from "node:path"; +import { resolveNativeStorageTimeoutMs } from "../src/lib/compose-startup-budget.ts"; +import { runNativeComposeStorageVerifiedExec } from "../src/lib/native-compose-command-storage.ts"; +import { nativeComposeEffectRefusal } from "../src/lib/native-compose-effect-diagnostics.ts"; +import { + assertNativeComposeMaterialAuthority, + type NativeComposeGenerationStore, + type NativeComposeMaterialAuthority, + type NativeComposeMaterialBinding, + openNativeComposeGenerationStore, +} from "../src/lib/native-compose-generation.ts"; +import * as ownership from "../src/lib/native-compose-ownership.ts"; +import { + type HeldDirectory, + holdDirectory, +} from "../src/lib/native-compose-private-state.ts"; +import { + beginNativeComposeStorageCarrierIntent, + initializeNativeComposeStorageCarrierJournal, +} from "../src/lib/native-compose-storage-carrier-journal.ts"; +import { parseNativeComposeStorageCommandRecord } from "../src/lib/native-compose-storage-command-record.ts"; +import { + createNativeComposeDockerStorageXattrCarrier, + NATIVE_STORAGE_CLEANUP_FLOOR_MS, + NATIVE_STORAGE_PROBE_CEILING_MS, + nativeComposeStorageProbeTimeoutMs, +} from "../src/lib/native-compose-storage-witness-docker.ts"; +import { + NATIVE_STORAGE_DOCKER_ARTIFACT, + nativeComposeStorageDockerImageReference, +} from "../src/lib/native-compose-storage-witness-docker-artifact.ts"; +import { + NATIVE_STORAGE_CARRIER_FORMAT, + NATIVE_STORAGE_CARRIER_LABEL, +} from "../src/lib/native-compose-storage-witness-docker-policy.ts"; +import { + type NativeComposeStorageXattrInvocation, + nativeComposeStorageXattrCarrierPorts, +} from "../src/lib/native-compose-storage-witness-xattr-carrier.ts"; +import { + createNativeComposeStorageXattrMarker, + encodeNativeComposeStorageXattrResponse, + type NativeComposeStorageXattrRequest, + type NativeComposeStorageXattrResponse, + refuseNativeComposeStorageXattr, +} from "../src/lib/native-compose-storage-witness-xattr-codec.ts"; +import * as shell from "../src/lib/shell.ts"; + +// HACK-1369: storage probes are bounded by the carrier's remaining +// HACK_NATIVE_STORAGE_TIMEOUT_MS budget, not by fixed 15 s/10 s caps. The engine +// below is in-process: reads are answered by a stubbed probe, and each daemon +// command runs as a local /bin/sh child. Nothing reaches a Docker socket, the +// real HOME or the repository root; elapsed engine time is a virtual clock. +// Recorded (original-command) proofs also run the host's read-only process +// identity reads (`/bin/ps`, and `sysctl` on macOS) for their stopped children. + +const engineId = "engine-fixture"; +const selection = { name: "owned_data", storage: "data" }; +const createdAt = "2026-10-08T12:00:00Z"; +const helperRoot = { device: "1", inode: "42", uid: 70, gid: 70 }; +const carrierId = "e".repeat(64); +const invocationId = "a".repeat(32); +/** Virtual helper start duration: above the former fixed 15 s start cap. */ +const SLOW_START_MS = 20_000; +const roots: string[] = []; +const stores: NativeComposeGenerationStore[] = []; +const heldJournals: HeldDirectory[] = []; +const ENV_KEYS = [ + "PATH", + "HOME", + "HACK_HOME", + "DOCKER_CONFIG", + "DOCKER_HOST", + "DOCKER_CONTEXT", + "DOCKER_API_VERSION", + "DOCKER_TLS_VERIFY", + "DOCKER_CERT_PATH", + "HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK", +] as const; +let savedEnv: Record = {}; +let tripwire = ""; + +async function tempDirectory(prefix: string): Promise { + const path = await realpath(await mkdtemp(join(tmpdir(), prefix))); + roots.push(path); + return path; +} + +beforeEach(async () => { + savedEnv = Object.fromEntries(ENV_KEYS.map((key) => [key, process.env[key]])); + const home = await tempDirectory("native-storage-budget-home-"); + const bin = join(home, "bin"); + await mkdir(bin); + tripwire = join(home, "docker-executed"); + // A `docker` that only records that something executed it. Every engine read + // is stubbed and every daemon command is rewritten, so it must never run. + await writeFile( + join(bin, "docker"), + `#!/bin/sh\nprintf executed > '${tripwire}'\nexit 97\n` + ); + await chmod(join(bin, "docker"), 0o755); + process.env.PATH = `${bin}:/usr/bin:/bin`; + process.env.HOME = home; + process.env.HACK_HOME = join(home, "hack"); + process.env.DOCKER_CONFIG = join(home, "docker-config"); + // A TCP host with an explicit API version is never an admitted Unix socket, + // so engine-ID fences use the stubbed CLI probe, never a direct socket read. + process.env.DOCKER_HOST = "tcp://127.0.0.1:9"; + process.env.DOCKER_API_VERSION = "1.43"; + process.env.HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK = "1"; + for (const key of [ + "DOCKER_CONTEXT", + "DOCKER_TLS_VERIFY", + "DOCKER_CERT_PATH", + ]) { + delete process.env[key]; + } +}); + +afterEach(async () => { + mock.restore(); + for (const key of ENV_KEYS) { + const value = savedEnv[key]; + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + await Promise.all(stores.splice(0).map((store) => store.close())); + await Promise.all( + heldJournals.splice(0).map((directory) => directory.file.close()) + ); + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })) + ); + // A held directory this file failed to close would otherwise be closed by a + // later garbage collection and fail an unrelated file; collect here so a leak + // is reported against this file. + Bun.gc(true); + await new Promise((resolve) => setTimeout(resolve, 0)); +}); + +/** Runs `effect` inside one owned generation effect with its material authority. */ +async function withAuthority( + effect: (opts: { + readonly store: NativeComposeGenerationStore; + readonly authority: NativeComposeMaterialAuthority; + readonly binding: NativeComposeMaterialBinding; + }) => Promise +): Promise { + const root = await tempDirectory("native-storage-budget-project-"); + await mkdir(join(root, ".hack")); + await Bun.write( + join(root, ".hack/hack.project.json"), + JSON.stringify({ schema_version: 1, name: "storage" }) + ); + const store = await openNativeComposeGenerationStore({ + projectRoot: root, + instance: null, + }); + stores.push(store); + await store.withMutation(async (mutation) => { + const reservation = mutation.reserveGeneration(); + const labels = { + "io.hack.native-config.version": "1", + "io.hack.native-config.owner": store.identity.ownerToken, + "io.hack.native-config.instance": store.identity.composeProject, + }; + const generation = await mutation.publish({ + reservation, + profiles: [], + inputRevision: "a".repeat(64), + assertFresh: async () => {}, + composeJson: JSON.stringify({ + name: store.identity.composeProject, + services: { + app: { + image: "synthetic:1", + labels: { + ...labels, + "io.hack.native-config.generation": reservation.generationId, + "io.hack.native-config.workload": "service", + }, + }, + }, + volumes: { + data: { + name: selection.name, + labels: { + ...labels, + "io.hack.native-config.storage": selection.storage, + }, + }, + }, + }), + }); + await mutation.runEffect({ + generation, + operation: "up", + assertOwned: async () => {}, + assertFresh: async () => {}, + effect: async () => { + const authority = mutation.materialAuthority; + await effect({ + store, + authority, + binding: await assertNativeComposeMaterialAuthority({ + authority, + generation, + phase: "effect", + }), + }); + return { outcome: "complete", value: 0 }; + }, + }); + }); +} + +type Engine = { + readonly probeTimeouts: number[]; + readonly commands: { + readonly verb: string; + readonly timeoutMs: number; + readonly elapsed: number; + }[]; + /** Virtual time the engine has consumed so far. */ + elapsed: number; +}; + +/** + * One in-process engine holding the selected owned volume and, once created, + * the exact helper carrier. `startMs` is the helper start's virtual duration: a + * start whose granted timeout is shorter times out at that timeout, like a real + * `docker start -ai` would. + */ +function installEngine(opts: { + readonly binding: NativeComposeMaterialBinding; + readonly store: NativeComposeGenerationStore; + readonly input: () => NativeComposeStorageXattrInvocation; + readonly startMs: number; + readonly response: NativeComposeStorageXattrResponse; + readonly cleanupInspectMs?: number; +}): Engine { + const engine: Engine = { probeTimeouts: [], commands: [], elapsed: 0 }; + const { binding, store } = opts; + const now = Date.now(); + spyOn(Date, "now").mockImplementation(() => now + engine.elapsed); + const program = join( + store.identity.checkoutRoot, + ".hack", + ".internal", + "native-compose", + store.identity.instanceId, + "storage-carriers", + invocationId, + "helper.mjs" + ); + let carrierState: "absent" | "created" | "exited" = "absent"; + let cleanupInspected = false; + const carrierValue = () => { + const input = opts.input(); + return { + id: carrierId, + createdAt, + image: input.artifact.imageId, + configImage: nativeComposeStorageDockerImageReference(input.artifact), + user: `${input.uid}:${input.gid}`, + entrypoint: ["/usr/local/bin/bun"], + cmd: ["--no-env-file", "/hack-storage-witness-helper.mjs"], + openStdin: true, + tty: false, + labels: { + [NATIVE_STORAGE_CARRIER_LABEL]: input.invocationId, + "io.hack.storage-witness.owner": input.target.ownerToken, + "io.hack.storage-witness.generation": input.scope.generationId, + "io.hack.storage-witness.helper": input.artifact.helperHash, + }, + host: { + Privileged: false, + ReadonlyRootfs: true, + NetworkMode: "none", + Memory: 268_435_456, + NanoCpus: 1_000_000_000, + PidsLimit: 32, + CapDrop: ["ALL"], + CapAdd: null, + SecurityOpt: ["no-new-privileges:true"], + LogConfig: { Type: "none", Config: {} }, + RestartPolicy: { Name: "no" }, + Binds: null, + Tmpfs: null, + Devices: [], + DeviceRequests: null, + PortBindings: {}, + OomKillDisable: false, + Mounts: [ + { + Type: "bind", + Source: program, + Target: "/hack-storage-witness-helper.mjs", + ReadOnly: true, + BindOptions: { + NonRecursive: true, + Propagation: "rprivate", + CreateMountpoint: false, + }, + }, + { + Type: "volume", + Source: input.target.name, + Target: "/hack-storage-witness", + ...(input.readonly ? { ReadOnly: true } : {}), + VolumeOptions: { NoCopy: true }, + }, + ], + }, + mounts: [ + { + Type: "bind", + Source: program, + Destination: "/hack-storage-witness-helper.mjs", + RW: false, + Propagation: "rprivate", + }, + { + Type: "volume", + Name: input.target.name, + Source: input.target.mountpoint, + Destination: "/hack-storage-witness", + Driver: "local", + Mode: "", + RW: !input.readonly, + Propagation: "", + }, + ], + state: { + Status: carrierState, + Running: false, + Pid: 0, + ExitCode: 0, + Paused: false, + Restarting: false, + OOMKilled: false, + Dead: false, + Error: "", + }, + execIds: null, + }; + }; + const answer = (args: readonly string[]): string => { + const [noun, verb] = args; + if (noun === "info" && verb === "--format" && args.length === 3) { + return args[2] === "{{json .ID}}" + ? `${JSON.stringify(engineId)}\n` + : `${JSON.stringify({ id: engineId, os: "linux", arch: "arm64" })}\n`; + } + if (noun === "image" && verb === "inspect") { + return `${JSON.stringify({ + id: NATIVE_STORAGE_DOCKER_ARTIFACT.imageId, + os: "linux", + arch: "arm64", + volumes: null, + })}\n`; + } + if (noun === "volume" && verb === "ls") { + return `${JSON.stringify(selection.name)}\n`; + } + if (noun === "volume" && verb === "inspect") { + return `${JSON.stringify({ + name: selection.name, + createdAt, + driver: "local", + options: null, + mountpoint: `/var/lib/docker/volumes/${selection.name}/_data`, + project: binding.identity.composeProject, + instance: binding.identity.composeProject, + owner: binding.identity.ownerToken, + version: "1", + storage: selection.storage, + provision: null, + })}\n`; + } + // No workload holds the volume; the helper carrier is answered by its + // exact inspect only, and every inventory stays empty. + if (noun === "container" && verb === "ls") { + return ""; + } + if ( + noun === "container" && + verb === "inspect" && + args[3] === NATIVE_STORAGE_CARRIER_FORMAT && + args[4] === carrierId && + carrierState !== "absent" + ) { + if (carrierState === "exited" && !cleanupInspected) { + cleanupInspected = true; + engine.elapsed += opts.cleanupInspectMs ?? 0; + } + return `${JSON.stringify(carrierValue())}\n`; + } + throw new Error("Unexpected synthetic Docker request"); + }; + spyOn(ownership, "createNativeComposeProbe").mockImplementation((probe) => { + const timeoutMs = probe.timeoutMs ?? Number.NaN; + engine.probeTimeouts.push(timeoutMs); + if (!(Number.isSafeInteger(timeoutMs) && timeoutMs >= 1)) { + throw new Error("Unbounded synthetic probe"); + } + return (args) => Promise.resolve(answer(args)); + }); + const run = shell.run; + spyOn(shell, "run").mockImplementation((argv, options = {}) => { + // [/bin/sh, -c, wrapper, name, docker, ...daemon arguments]. A recorded + // command's wrapper stops itself before exec; the replacement child keeps + // that wrapper, so its publication handshake runs unchanged. + if (basename(argv[4] ?? "") !== "docker") { + // Host identity reads of recorded commands (`/bin/ps`, `sysctl`). + return run(argv, options); + } + const daemon = argv.slice(5); + const timeoutMs = options.timeoutMs ?? Number.NaN; + const verb = daemon[0] === "volume" ? "volume create" : (daemon[0] ?? ""); + engine.commands.push({ verb, timeoutMs, elapsed: engine.elapsed }); + // `effect` is the daemon-side change, applied when the child exits: a + // recorded command re-checks admission (the helper still `created`) after + // its child is spawned and before it resumes. + const child = ( + script: string, + step: { readonly granted?: number; readonly effect?: () => void } = {} + ) => + run([...argv.slice(0, 4), "/bin/sh", "-c", script], { + ...options, + timeoutMs: step.granted ?? timeoutMs, + onExit: (event) => { + step.effect?.(); + return options.onExit?.(event) ?? Promise.resolve(); + }, + }); + if (daemon[0] === "create") { + return child(`printf '%s\\n' ${carrierId}`, { + effect: () => { + carrierState = "created"; + }, + }); + } + if (daemon[0] === "start" && daemon[2] === carrierId) { + if (!(timeoutMs >= opts.startMs)) { + // The granted bound expires first: the engine is still starting the + // helper when the timeout fires, so the virtual clock reaches it. + engine.elapsed += Number.isFinite(timeoutMs) ? timeoutMs : 0; + return child("sleep 5", { granted: 20 }); + } + const response = encodeNativeComposeStorageXattrResponse(opts.response); + return child(`printf '%s' '${response.trimEnd()}'; printf '\\n'`, { + effect: () => { + engine.elapsed += opts.startMs; + carrierState = "exited"; + }, + }); + } + if (daemon[0] === "rm" && daemon[1] === carrierId) { + return child(`printf '%s\\n' ${carrierId}`, { + effect: () => { + carrierState = "absent"; + }, + }); + } + throw new Error("Unexpected synthetic Docker command"); + }); + return engine; +} + +function invocation(opts: { + readonly binding: NativeComposeMaterialBinding; + readonly target: NativeComposeStorageXattrInvocation["target"]; + readonly request: NativeComposeStorageXattrRequest; + readonly recordCreated?: NativeComposeStorageXattrInvocation["recordCreated"]; +}): NativeComposeStorageXattrInvocation { + const { binding, target, request } = opts; + return { + recordCreated: opts.recordCreated ?? (async () => {}), + invocationId, + artifact: NATIVE_STORAGE_DOCKER_ARTIFACT, + target, + readonly: true, + uid: 70, + gid: 70, + request, + scope: { + generationId: binding.generationId, + currentGenerationId: binding.currentGenerationId, + pendingGenerationId: binding.pendingGenerationId, + pendingToken: binding.pendingToken, + }, + }; +} + +type Proof = { + readonly engine: Engine; + readonly result: unknown; + readonly error: unknown; + /** Original-command record of a recorded proof, read after it returned. */ + readonly record: string | null; + /** A recorded proof's finish() retired its invocation directory. */ + readonly retired: boolean; +}; + +/** + * One readonly proof whose helper start takes {@link SLOW_START_MS}. The default + * is an unrecorded `root` request. `recorded` builds the carrier exactly as + * `exec` and `up` storage proofs do (`prepareNativeComposeCommandStorage`: + * `originalCommandRecords: true`) and sends a readonly `verify` request, so every + * helper command takes the recorded original-command branch. + */ +async function proveWithSlowStart(opts: { + readonly budgetMs: number; + readonly cleanupInspectMs?: number; + readonly recorded?: true; +}): Promise { + let outcome: Proof | null = null; + await withAuthority(async ({ store, authority, binding }) => { + let input: NativeComposeStorageXattrInvocation | null = null; + const marker = createNativeComposeStorageXattrMarker(); + const request: NativeComposeStorageXattrRequest = opts.recorded + ? { ...marker, operation: "verify", root: helperRoot } + : { kind: "directory-xattr", version: 1, operation: "root" }; + const engine = installEngine({ + binding, + store, + input: () => input ?? refuseNativeComposeStorageXattr(), + startMs: SLOW_START_MS, + response: opts.recorded + ? { + kind: "directory-xattr", + version: 1, + outcome: "verified", + root: helperRoot, + valueHex: marker.valueHex, + } + : { + kind: "directory-xattr", + version: 1, + outcome: "root", + root: helperRoot, + }, + cleanupInspectMs: opts.cleanupInspectMs, + }); + const handle = await createNativeComposeDockerStorageXattrCarrier({ + authority, + store, + engineId, + signal: new AbortController().signal, + deadline: Date.now() + opts.budgetMs, + ...(opts.recorded ? { originalCommandRecords: true as const } : {}), + }); + const ports = nativeComposeStorageXattrCarrierPorts(handle); + // Callers pass the carrier's own observed target, as storage proofs do. + const target = await ports.inspect(selection); + const base = invocation({ + binding, + target: target as NativeComposeStorageXattrInvocation["target"], + request, + }); + // A recorded proof journals its intent the way the witness owner does, so + // its completion can retire the invocation through finish(). + let journal: Awaited< + ReturnType + > | null = null; + if (opts.recorded) { + const directory = await holdDirectory( + await tempDirectory("native-storage-budget-journal-"), + true + ); + heldJournals.push(directory); + const bound = { + directory, + token: "b".repeat(32), + check: async () => {}, + }; + await initializeNativeComposeStorageCarrierJournal(bound); + const { recordCreated: _transportOnly, ...journalInput } = base; + journal = await beginNativeComposeStorageCarrierIntent({ + ...bound, + input: journalInput, + }); + } + input = journal ? { ...base, recordCreated: journal.recordCreated } : base; + let result: unknown = null; + let error: unknown = null; + try { + result = await ports.invoke(input); + } catch (caught) { + error = caught; + } + const recordPath = join( + store.identity.checkoutRoot, + ".hack", + ".internal", + "native-compose", + store.identity.instanceId, + "storage-carriers", + invocationId, + "commands.json" + ); + const record = + opts.recorded && existsSync(recordPath) + ? await readFile(recordPath, "utf8") + : null; + let retired = false; + if (journal && result !== null) { + // A recorded proof defers file retirement (and closing its held + // invocation directories) to finish() with the journal's completion. + const completion = await journal.complete({ id: carrierId, createdAt }); + await ports.finish?.({ input, completion }); + retired = !existsSync(dirname(recordPath)); + } + outcome = { engine, result, error, record, retired }; + }); + if (!outcome) { + throw new Error("Storage proof fixture did not run"); + } + return outcome; +} + +test("a helper start slower than the former 15 s cap completes within the default storage budget", async () => { + // The default per-carrier budget used by `exec` and `up` storage proofs. + const budgetMs = resolveNativeStorageTimeoutMs({ env: {} }); + expect(budgetMs).toBe(90_000); + const { engine, result, error } = await proveWithSlowStart({ budgetMs }); + expect(nativeComposeEffectRefusal(error)).toBeUndefined(); + expect(error).toBeNull(); + expect(result).toMatchObject({ + outcome: "complete", + exitCode: 0, + carrierId, + containersAfterCleanup: [], + }); + expect(engine.commands.map(({ verb }) => verb)).toEqual([ + "create", + "start", + "rm", + ]); + const [create, start, remove] = engine.commands; + // Create and start get the remaining carrier budget, never more. + for (const command of [create, start]) { + expect(command?.timeoutMs).toBeGreaterThan(SLOW_START_MS); + expect(command?.timeoutMs).toBeLessThanOrEqual(budgetMs); + } + // Removal keeps its own finite cleanup bound. + expect(remove?.timeoutMs).toBeGreaterThanOrEqual( + NATIVE_STORAGE_CLEANUP_FLOOR_MS + ); + expect(remove?.timeoutMs).toBeLessThanOrEqual( + NATIVE_STORAGE_PROBE_CEILING_MS + ); + expect(engine.probeTimeouts.length).toBeGreaterThan(0); + expect( + engine.probeTimeouts.every( + (value) => value >= 1 && value <= NATIVE_STORAGE_PROBE_CEILING_MS + ) + ).toBe(true); + expect(engine.elapsed).toBe(SLOW_START_MS); + expect(existsSync(tripwire)).toBe(false); +}, 30_000); + +// The exec/up path: prepareNativeComposeCommandStorage creates every carrier +// with `originalCommandRecords: true`, so readonly `verify` proofs run each +// helper command through the recorded original-command branch (publication, +// boundary deadline and settlement). That branch computes its own child bound, +// so it needs its own slow-start proof. +test("a recorded exec/up verify proof completes a helper start slower than the former 15 s cap", async () => { + const budgetMs = resolveNativeStorageTimeoutMs({ env: {} }); + expect(budgetMs).toBe(90_000); + const { engine, result, error, record, retired } = await proveWithSlowStart({ + budgetMs, + recorded: true, + }); + expect(nativeComposeEffectRefusal(error)).toBeUndefined(); + expect(error).toBeNull(); + expect(result).toMatchObject({ + outcome: "complete", + exitCode: 0, + carrierId, + containersAfterCleanup: [], + }); + expect(engine.commands.map(({ verb }) => verb)).toEqual([ + "create", + "start", + "rm", + ]); + const [create, start, remove] = engine.commands; + // The recorded child bound is the remaining carrier budget, never more. + for (const command of [create, start]) { + expect(command?.timeoutMs).toBeGreaterThan(SLOW_START_MS); + expect(command?.timeoutMs).toBeLessThanOrEqual(budgetMs); + } + expect(remove?.timeoutMs).toBeGreaterThan(0); + expect(remove?.timeoutMs).toBeLessThanOrEqual( + NATIVE_STORAGE_PROBE_CEILING_MS + ); + // Every helper command was published and settled as an original command. + expect(record).not.toBeNull(); + const { commands } = parseNativeComposeStorageCommandRecord(record ?? ""); + expect(commands.map(({ kind }) => kind)).toEqual([ + "create", + "start", + "remove", + ]); + for (const command of commands) { + expect(command.child).not.toBeNull(); + expect(command.settlement).toMatchObject({ + exitCode: 0, + timedOut: false, + cancelled: false, + }); + } + // The journal's completion retired the invocation files through finish(). + expect(retired).toBe(true); + expect(engine.elapsed).toBe(SLOW_START_MS); + expect(existsSync(tripwire)).toBe(false); +}, 30_000); + +test("a helper start that outlasts the whole storage budget still refuses at the budget's end", async () => { + const budgetMs = 12_000; + const { engine, result, error } = await proveWithSlowStart({ budgetMs }); + expect(result).toBeNull(); + // The start was granted only the carrier's remaining budget; its timeout + // lands at the carrier's end and names that exhausted budget. + expect(nativeComposeEffectRefusal(error)).toEqual({ + stage: "storage-helper-start", + reason: "storage-budget", + }); + expect(engine.commands.map(({ verb }) => verb)).toEqual(["create", "start"]); + const start = engine.commands[1]; + expect(start?.timeoutMs).toBeGreaterThan(0); + expect(start?.timeoutMs).toBeLessThanOrEqual(budgetMs); + expect(existsSync(tripwire)).toBe(false); +}, 30_000); + +test("cleanup crossing the storage deadline still removes the exited helper and refuses the proof", async () => { + const budgetMs = SLOW_START_MS + 1000; + const cleanupInspectMs = 2000; + const { engine, result, error } = await proveWithSlowStart({ + budgetMs, + cleanupInspectMs, + }); + expect(engine.commands.map(({ verb }) => verb)).toEqual([ + "create", + "start", + "rm", + ]); + const start = engine.commands[1]; + expect(start?.timeoutMs).toBeGreaterThan(SLOW_START_MS); + const remove = engine.commands[2]; + expect(remove?.elapsed).toBe(SLOW_START_MS + cleanupInspectMs); + expect(remove?.elapsed).toBeGreaterThan(budgetMs); + expect(remove?.timeoutMs).toBeGreaterThan(0); + expect(remove?.timeoutMs).toBe(NATIVE_STORAGE_CLEANUP_FLOOR_MS); + expect(remove?.timeoutMs).toBeLessThanOrEqual( + NATIVE_STORAGE_PROBE_CEILING_MS + ); + expect(result).toBeNull(); + expect(nativeComposeEffectRefusal(error)).toMatchObject({ + reason: "storage-budget", + }); + expect(existsSync(tripwire)).toBe(false); +}, 30_000); + +test("probe timeouts follow the remaining carrier budget with a finite cleanup bound", () => { + const now = 1_000_000; + const at = (remaining: number) => ({ deadline: now + remaining, now }); + // Commands get exactly the remaining budget, however large. + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "command", ...at(80_000) }) + ).toBe(80_000); + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "command", ...at(600_000) }) + ).toBe(600_000); + // Reads follow the budget but keep the probe's own 60 s ceiling. + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "read", ...at(40_000) }) + ).toBe(40_000); + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "read", ...at(90_000) }) + ).toBe(NATIVE_STORAGE_PROBE_CEILING_MS); + // An exhausted budget leaves a non-positive bound: the probe or command + // refuses before it spawns. + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "read", ...at(0) }) + ).toBeLessThanOrEqual(0); + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "command", ...at(-5) }) + ).toBeLessThanOrEqual(0); + // Cleanup has its own bound: a floor after the deadline, a ceiling before it. + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "cleanup", ...at(-30_000) }) + ).toBe(NATIVE_STORAGE_CLEANUP_FLOOR_MS); + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "cleanup", ...at(25_000) }) + ).toBe(25_000); + expect( + nativeComposeStorageProbeTimeoutMs({ kind: "cleanup", ...at(600_000) }) + ).toBe(NATIVE_STORAGE_PROBE_CEILING_MS); +}); + +// Documented exec contract: the closing proof runs after the guest command and +// is fail-closed. Its refusal (for example a storage probe timeout) fails exec +// even though the command already ran; the command's exit code is not reported. +test("a closing storage proof refusal fails saved exec after its command ran", async () => { + const order: string[] = []; + const refusal = new Error("closing storage proof refused"); + await expect( + runNativeComposeStorageVerifiedExec({ + signal: new AbortController().signal, + assertSaved: async (options) => { + order.push(options.adjacentProof ? "opening" : "closing"); + if (!options.adjacentProof) { + throw refusal; + } + }, + run: async () => { + order.push("command"); + return 0; + }, + }) + ).rejects.toBe(refusal); + expect(order).toEqual(["opening", "command", "closing"]); +}); diff --git a/tests/native-compose-storage-witness-xattr-owner.test.ts b/tests/native-compose-storage-witness-xattr-owner.test.ts index 125e9db6e..a6814a8fa 100644 --- a/tests/native-compose-storage-witness-xattr-owner.test.ts +++ b/tests/native-compose-storage-witness-xattr-owner.test.ts @@ -85,10 +85,10 @@ const artifact: NativeComposeStorageXattrArtifact = { * (publish, enroll, verify, retire) in process. They wait on no timer: the * fake carrier answers at once. Their cost is a fixed amount of durable store * file work, mostly system time: the second `up` of the same-birth case alone - * makes about 19k lstat, 13k realpath and 4.5k open calls, the same count on - * every run. That takes about 5-7 s on hosted CI, but 5-60 s or more on a - * host whose load is several times its CPU count, so this budget is a hang - * guard, not a speed assertion (HACK-1422). + * makes about 13.5k lstat, 3.7k realpath and 3.9k open calls, the same count + * on every run (19k, 13k and 4.5k before HACK-1440). It can still take tens + * of seconds on a host whose load is several times its CPU count, so this + * budget is a hang guard, not a speed assertion (HACK-1422). */ const STORE_CASE_BUDGET_MS = 180_000; /** diff --git a/tests/native-env-metadata.test.ts b/tests/native-env-metadata.test.ts index e66af84ce..5b91066b4 100644 --- a/tests/native-env-metadata.test.ts +++ b/tests/native-env-metadata.test.ts @@ -1,6 +1,14 @@ -import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + expect, + test, +} from "bun:test"; import { chmod, + cp, mkdir, mkdtemp, readdir, @@ -13,6 +21,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { acquireNativeManagedEnvFile } from "../src/lib/native-project-inputs.ts"; import { + nativeEnvMetadataDiagnostic, type ProjectEnvValuesByScope, resolveProjectEnvMetadataForNativeSelection, } from "../src/lib/project-env-config.ts"; @@ -46,8 +55,8 @@ afterEach(async () => { } await rm(root, { recursive: true, force: true }); }); -async function project(name = "primary") { - const projectRoot = join(root, name); +async function project(name = "primary", base = root) { + const projectRoot = join(base, name); await mkdir(join(projectRoot, ".hack"), { recursive: true }); // Selection is supplied by the compiler; this owner never parses project policy. await writeFile( @@ -72,60 +81,165 @@ async function layer( }) ); } +/** + * An unexpected refusal fails with its value-free diagnostic in the message, so a + * hosted failure names the underlying error class/code; refuses() unwraps the + * original closed refusal and asserts on it unchanged. + */ +class MetadataRefused extends Error { + readonly refusal: unknown; + constructor(refusal: unknown) { + super( + `native env metadata refused (diagnostic: ${nativeEnvMetadataDiagnostic(refusal) ?? "none"})` + ); + this.name = "MetadataRefused"; + this.refusal = refusal; + } +} +async function reportingDiagnostic(resolving: Promise): Promise { + try { + return await resolving; + } catch (error: unknown) { + throw new MetadataRefused(error); + } +} function metadata( projectRoot: string, overlay: string | null = null, inheritLocal = true, declaredWorkloadNames: readonly string[] = ["web", "job", "inactive"] ) { - return resolveProjectEnvMetadataForNativeSelection({ - projectRoot, - overlay, - inheritLocal, - declaredWorkloadNames, - }); + return reportingDiagnostic( + resolveProjectEnvMetadataForNativeSelection({ + projectRoot, + overlay, + inheritLocal, + declaredWorkloadNames, + }) + ); } +/** + * Fixture Git ignores the caller's global/system configuration, so a contributor's + * hooks, fsmonitor or signing setup cannot add processes or latency to a test. + */ async function git(projectRoot: string, args: string[]) { const child = Bun.spawn(["git", "-C", projectRoot, ...args], { + env: { + ...process.env, + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_CONFIG_NOSYSTEM: "1", + }, stdin: "ignore", stdout: "ignore", stderr: "ignore", }); expect(await child.exited).toBe(0); } -async function linked() { - const primary = await project(); - await git(primary, ["init", "--quiet", "-b", "main"]); - await git(primary, ["add", ".hack/hack.project.json"]); - await git(primary, [ - "-c", - "user.name=Fixture", - "-c", - "user.email=fixture@example.invalid", - "-c", - "commit.gpgsign=false", - "commit", - "--quiet", - "-m", - "fixture", +/** + * Git fixtures are built once per file and copied into each test's root. + * Creating a repository writes many files (init, index, objects, refs, worktree + * checkout); on a loaded runner those writes, not the product's read-only + * inspections, took seconds per test and hit bun's 5 s test budget (HACK-1362). + * A copy is filesystem-only and spawns no Git. + */ +let templates: string; +beforeAll(async () => { + templates = await realpath( + await mkdtemp(join(tmpdir(), "native-env-metadata-template-")) + ); + const plain = await project("primary", join(templates, "repository")); + const primary = await project("primary", join(templates, "linked")); + // No hook samples: fewer files to write now and to copy per test. + const init = ["init", "--quiet", "--template=", "-b", "main"]; + await Promise.all([ + git(plain, init), + (async () => { + await git(primary, init); + await git(primary, ["add", ".hack/hack.project.json"]); + await git(primary, [ + "-c", + "user.name=Fixture", + "-c", + "user.email=fixture@example.invalid", + "-c", + "commit.gpgsign=false", + // Never leave a detached background maintenance process behind the fixture. + "-c", + "maintenance.auto=false", + "commit", + "--quiet", + "-m", + "fixture", + ]); + await git(primary, [ + "worktree", + "add", + "--quiet", + "-b", + "fixture", + join(templates, "linked/linked"), + ]); + })(), ]); +}); +afterAll(async () => { + await rm(templates, { recursive: true, force: true }); +}); +async function copyTemplate(name: "repository" | "linked") { + await cp(join(templates, name), root, { + recursive: true, + verbatimSymlinks: true, + }); +} +/** An exact Git root with an empty index and no commit or checkout. */ +async function repository() { + await copyTemplate("repository"); + return join(root, "primary"); +} +/** + * A committed primary with one linked checkout. The two linkage files are the + * only absolute paths Git keeps, so the copy points them at this test's root. + */ +async function linked() { + await copyTemplate("linked"); + const primary = join(root, "primary"); const checkout = join(root, "linked"); - await git(primary, ["worktree", "add", "--quiet", "-b", "fixture", checkout]); + await writeFile( + join(checkout, ".git"), + `gitdir: ${join(primary, ".git/worktrees/linked")}\n` + ); + await writeFile( + join(primary, ".git/worktrees/linked/gitdir"), + `${join(checkout, ".git")}\n` + ); return { primary, checkout }; } -async function refuses(run: () => Promise) { +const VALUE_FREE_DIAGNOSTIC = /^[A-Za-z][A-Za-z0-9]*(?::[A-Z][A-Z0-9_]*)?$/; +async function refuses(run: () => Promise, diagnostic?: string) { + let refused: unknown = new Error("Expected refusal"); try { await run(); - throw new Error("Expected refusal"); } catch (error: unknown) { - expect(error).toBeInstanceOf(Error); - if (!(error instanceof Error)) { - throw error; - } - expect(error.message).toContain("values omitted"); - expect(error.message).not.toContain(SENTINEL); - expect(error.message).not.toContain(root); - expect(error.cause).toBeUndefined(); + refused = error instanceof MetadataRefused ? error.refusal : error; + } + expect(refused).toBeInstanceOf(Error); + if (!(refused instanceof Error)) { + throw refused; + } + expect(refused.message).toContain("values omitted"); + expect(refused.message).not.toContain(SENTINEL); + expect(refused.message).not.toContain(root); + expect(refused.cause).toBeUndefined(); + if ( + refused.message.startsWith("Cannot resolve native managed env metadata") + ) { + // Closed metadata refusals keep only a class/code diagnostic, never values or paths. + expect(nativeEnvMetadataDiagnostic(refused)).toMatch(VALUE_FREE_DIAGNOSTIC); + expect(Object.keys(refused)).toEqual([]); + expect(JSON.stringify(refused)).toBe("{}"); + } + if (diagnostic !== undefined) { + expect(nativeEnvMetadataDiagnostic(refused)).toBe(diagnostic); } } @@ -219,15 +333,12 @@ test("all six layers use live verified primary inputs with checkout-local preced await layer(checkout, "hack.env.qa.local.yaml", { global: { WIN: secure, CURRENT_OVERLAY: "" }, }); - expect((await metadata(checkout, "qa")).effectiveMetadata.web?.WIN).toEqual({ + const qa = await metadata(checkout, "qa"); + expect(qa.effectiveMetadata.web?.WIN).toEqual({ scope: "global", secret: true, }); - expect( - Object.keys( - (await metadata(checkout, "qa")).effectiveMetadata.web ?? {} - ).sort() - ).toEqual([ + expect(Object.keys(qa.effectiveMetadata.web ?? {}).sort()).toEqual([ "BASE", "CURRENT", "CURRENT_OVERLAY", @@ -280,11 +391,12 @@ test("nested native root never inherits its ancestor checkout's locals", async ( test("invalid exact-root Git linkage refuses instead of inheriting or falling back", async () => { const p = await project(); await writeFile(join(p, ".git"), `gitdir: ${SENTINEL}`); - await refuses(() => metadata(p)); + await refuses(() => metadata(p), "HackCliError:E_CONFIG_INVALID"); }); test("tracked local filename compatibility and non-Git environment hint remain supported", async () => { - const { primary } = await linked(); + // Tracking is index state at an exact Git root; no commit or linked checkout is needed. + const primary = await repository(); await layer( primary, "hack.env.local.yaml", @@ -316,6 +428,15 @@ test("tracked local filename compatibility and non-Git environment hint remain s }); }); +const SELECTED_INPUT_DIAGNOSTIC = { + symlink: "NativeProjectInputError:E_CONFIG_INVALID", + dangling: "NativeProjectInputError:E_CONFIG_INVALID", + directory: "NativeProjectInputError:E_CONFIG_INVALID", + unreadable: "NativeProjectInputError:E_CONFIG_INVALID", + oversized: "NativeProjectInputError:E_CONFIG_INVALID", + malformed: "SyntaxError", + utf8: "TypeError:ERR_ENCODING_INVALID_ENCODED_DATA", +} as const; for (const kind of [ "symlink", "dangling", @@ -344,7 +465,7 @@ for (const kind of [ await chmod(path, 0); } } - await refuses(() => metadata(p, "qa")); + await refuses(() => metadata(p, "qa"), SELECTED_INPUT_DIAGNOSTIC[kind]); }); } @@ -451,7 +572,7 @@ test("tracked-path inspection disables repository fsmonitor and ignores caller G }); test("cancellation interrupts and reaps an in-flight owned Git inspection", async () => { - const { primary } = await linked(); + const primary = await repository(); const bin = join(root, "bin"); const marker = join(root, "inspection-started"); await mkdir(bin); @@ -462,6 +583,7 @@ test("cancellation interrupts and reaps an in-flight owned Git inspection", asyn await chmod(join(bin, "git"), 0o700); process.env.PATH = bin; const controller = new AbortController(); + let settled = false; const pending = resolveProjectEnvMetadataForNativeSelection({ projectRoot: primary, overlay: null, @@ -472,12 +594,14 @@ test("cancellation interrupts and reaps an in-flight owned Git inspection", asyn () => null, (error: unknown) => error ); + pending.then(() => { + settled = true; + }); try { - for ( - let attempt = 0; - attempt < 100 && !(await Bun.file(marker).exists()); - attempt++ - ) { + // Wait for the owned child itself, not a fixed poll budget: on a loaded runner + // starting the inspection can outlast any fixed count of short sleeps. A call + // that settles without starting it fails the marker assertion below. + while (!(settled || (await Bun.file(marker).exists()))) { await Bun.sleep(5); } expect(await Bun.file(marker).exists()).toBe(true); @@ -521,20 +645,22 @@ function hostMetadata( readonly workloadNames?: readonly string[]; } = {} ) { - return resolveProjectEnvMetadataForNativeSelection({ - projectRoot, - overlay: opts.overlay ?? null, - inheritLocal: opts.inheritLocal ?? true, - declaredWorkloadNames: opts.declaredWorkloadNames ?? [ - "web", - "job", - "inactive", - ], - hostTargets: { - includeDefault: opts.includeDefault ?? true, - workloadNames: opts.workloadNames ?? ["web"], - }, - }); + return reportingDiagnostic( + resolveProjectEnvMetadataForNativeSelection({ + projectRoot, + overlay: opts.overlay ?? null, + inheritLocal: opts.inheritLocal ?? true, + declaredWorkloadNames: opts.declaredWorkloadNames ?? [ + "web", + "job", + "inactive", + ], + hostTargets: { + includeDefault: opts.includeDefault ?? true, + workloadNames: opts.workloadNames ?? ["web"], + }, + }) + ); } test("host selection is optional and preserves exact empty/default/requested workload presence", async () => { @@ -647,7 +773,7 @@ test("a host-named declared workload disables generic host injection even when n }); }); -test("host metadata observes all six real worktree layers and explicit base selection", async () => { +async function hostSixLayers() { const { primary, checkout } = await linked(); await layer(checkout, "hack.env.default.yaml", { host: { BASE: "", WIN: secure }, @@ -667,6 +793,11 @@ test("host metadata observes all six real worktree layers and explicit base sele await layer(checkout, "hack.env.qa.local.yaml", { global: { CURRENT_OVERLAY: "", WIN: "" }, }); + return checkout; +} + +test("host metadata observes all six real worktree layers and explicit base selection", async () => { + const checkout = await hostSixLayers(); const result = await hostMetadata(checkout, { overlay: "qa" }); expect(result.hostMetadata?.default?.WIN).toEqual({ scope: "global", @@ -691,12 +822,15 @@ test("host metadata observes all six real worktree layers and explicit base sele }); expect(base.hostMetadata?.default?.PRIMARY_OVERLAY).toBeUndefined(); expect(base.hostMetadata?.default?.CURRENT_OVERLAY).toBeUndefined(); - for (const exclusion of ["optout", "ci", "slim"]) { +}); + +for (const exclusion of ["optout", "ci", "slim"] as const) { + test(`host metadata ${exclusion} skips primary layers but retains checkout-local layers`, async () => { + const checkout = await hostSixLayers(); if (exclusion === "ci") { process.env.CI = "true"; } if (exclusion === "slim") { - Reflect.deleteProperty(process.env, "CI"); process.env.HACK_EXECUTION_MODE = "slim"; } const excluded = await hostMetadata(checkout, { @@ -709,8 +843,8 @@ test("host metadata observes all six real worktree layers and explicit base sele scope: "host", secret: false, }); - } -}); + }); +} test("unknown, duplicate, noncanonical and malformed host requests refuse without disclosure", async () => { const p = await project(); diff --git a/tests/project-env-write-refusal-gaps.test.ts b/tests/project-env-write-refusal-gaps.test.ts new file mode 100644 index 000000000..79e3e8a57 --- /dev/null +++ b/tests/project-env-write-refusal-gaps.test.ts @@ -0,0 +1,921 @@ +import { afterEach, expect, test } from "bun:test"; +import { + chmod, + lstat, + mkdir, + mkdtemp, + readdir, + readFile, + readlink, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { basename, join, resolve } from "node:path"; + +import { INVALID_ENV_SELECTION_MESSAGE } from "../src/cli/env-selection.ts"; +import { + PROJECT_COMPOSE_FILENAME, + PROJECT_CONFIG_FILENAME, +} from "../src/constants.ts"; +import { HackCliError, type HackErrorCode } from "../src/lib/cli-result.ts"; +import { + isHardLinkUnsupportedError, + publishByExclusivePlaceholder, +} from "../src/lib/exclusive-publish.ts"; +import { + type AtomicWriteRefusal, + AtomicWriteRefusedError, + ensureDirForWrite, + ensureRealDirectoryForWrite, + FileWriteRefusedError, +} from "../src/lib/fs.ts"; +import { + ENV_CONFIG_NAME_MAX_LENGTH, + parseEnvConfigSelection, +} from "../src/lib/project.ts"; +import { + HACK_DIR_GITIGNORE_REFUSAL, + HackDirGitignoreRefusalError, + ProjectEnvKeyLockTimeoutError, + resolveProjectEnvConfigPath, +} from "../src/lib/project-env-config.ts"; +import { + ProjectsRegistryLockTimeoutError, + withProjectsRegistryLock, +} from "../src/lib/projects-registry-lock.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +/** + * HACK-1377: env write refusals still raw after HACK-1330. + * + * 1. A filesystem without hard links (ExFAT, FAT) refused every env write + * with a raw ENOTSUP from the lock's and the key's `link(2)` publish; the + * publish now falls back to an exclusive reservation plus rename. + * 2. A dangling `.hack/.internal` symlink made hydrate print a raw EEXIST. + * 3. A very long `--env` name printed a raw ENAMETOOLONG with a staging path. + * 4. A stale project env key lock timed out with a stack. + * 5. A `.hack/.gitignore` directory printed a line with no code. + * 6. Mappings whose revert failed no test. + * + * Every value is a synthetic test string; every child gets a private HOME, + * HACK_HOME and Docker client config inside a temp directory, never the + * keychain; an env layer exists before any `--secret` set. + */ + +const REPO_ROOT = resolve(import.meta.dir, ".."); +const ENTRYPOINT = resolve(REPO_ROOT, "index.ts"); +const ERRNO_PRELOAD = resolve( + import.meta.dir, + "fixtures/staging-errno-preload.ts" +); +const KEPT_VALUE = "synthetic-kept-41c7"; +const NEW_VALUE = "synthetic-new-90ad"; +const LAYER = `version: 1\nenvironment: default\nsecretsprovider: project_key\nvalues:\n global:\n KEPT_SETTING: ${KEPT_VALUE}\n`; +const HYDRATE_INPUT = `NEW_SETTING=${NEW_VALUE}\n`; + +/** Permission bits do not bind root; permission-based cases skip under it. */ +const RUNS_AS_ROOT = (process.geteuid?.() ?? process.getuid?.() ?? -1) === 0; + +const tempDirs = new Set(); + +afterEach(async () => { + for (const tempDir of tempDirs) { + await rm(tempDir, { recursive: true, force: true }); + } + tempDirs.clear(); +}, 30_000); + +type Repo = { + readonly root: string; + readonly projectRoot: string; + readonly projectDir: string; + readonly tracked: string; +}; + +async function createRepo(): Promise { + const root = await mkdtemp(join(tmpdir(), "hack-env-gaps-")); + tempDirs.add(root); + const projectRoot = resolve(root, "repo"); + const projectDir = resolve(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + resolve(projectDir, PROJECT_COMPOSE_FILENAME), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + resolve(projectDir, PROJECT_CONFIG_FILENAME), + `${JSON.stringify({ name: "env-gaps", dev_host: "env-gaps.hack" }, null, 2)}\n` + ); + const tracked = resolveProjectEnvConfigPath({ projectDir, envName: null }); + await writeFile(tracked, LAYER); + return { root, projectRoot, projectDir, tracked }; +} + +type CliRun = { + readonly exitCode: number; + readonly stdout: string; + readonly stderr: string; +}; + +async function runCli(opts: { + readonly repo: Repo; + readonly args: readonly string[]; + readonly stdin?: string; + readonly logger?: string; + /** Injected errno: the preload fails one filesystem step of one path. */ + readonly inject?: { + readonly code: string; + readonly step: string; + readonly path: string; + }; +}): Promise { + const preload = opts.inject ? ["--preload", ERRNO_PRELOAD] : []; + const injected = opts.inject + ? { + HACK_TEST_STAGING_ERRNO: opts.inject.code, + HACK_TEST_ERRNO_STEP: opts.inject.step, + HACK_TEST_ERRNO_PATH: opts.inject.path, + } + : {}; + const proc = Bun.spawn( + [process.execPath, ...preload, ENTRYPOINT, ...opts.args], + { + cwd: opts.repo.projectRoot, + env: privateCliEnv({ + root: opts.repo.root, + env: { + HACK_HOME: join(opts.repo.root, "hack-home"), + HACK_GLOBAL_CONFIG_PATH: join(opts.repo.root, "hack-config.json"), + HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: "1", + HACK_LOGGER: opts.logger ?? "console", + HACK_NO_INTERACTIVE: "1", + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: "/dev/null", + ...injected, + }, + }), + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + } + ); + proc.stdin.write(opts.stdin ?? ""); + await proc.stdin.end(); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + return { exitCode, stdout, stderr }; +} + +async function runGit(repo: Repo, args: readonly string[]): Promise { + const proc = Bun.spawn(["git", ...args], { + cwd: repo.projectRoot, + env: privateCliEnv({ + root: repo.root, + env: { GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: "/dev/null" }, + }), + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([ + new Response(proc.stderr).text(), + proc.exited, + ]); + expect({ args, exitCode, stderr }).toEqual({ args, exitCode: 0, stderr: "" }); +} + +/** Output lines that carry text, ignoring the clack frame (`│`, `■`, ...). */ +function textLines(run: CliRun): string[] { + return `${run.stdout}\n${run.stderr}` + .split("\n") + .filter((line) => /[A-Za-z]/.test(line)); +} + +const RAW_ERRNO_CODES = [ + "ENOTSUP", + "EOPNOTSUPP", + "EPERM", + "EEXIST", + "ENAMETOOLONG", + "EISDIR", + "ENOENT", + "EROFS", +] as const; + +/** One closed line: no stack, no source path, no value, no errno, no staging name. */ +function expectOneLineRefusal(run: CliRun, expected: string): void { + expect(run.exitCode).toBe(1); + const lines = textLines(run); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain(expected); + expectNoRawDetail(run); +} + +function expectNoRawDetail(run: CliRun): void { + const output = `${run.stdout}${run.stderr}`; + expect(output).not.toMatch(/\bat\s.*\.ts:\d+/); + expect(output).not.toContain(REPO_ROOT); + expect(output).not.toContain(KEPT_VALUE); + expect(output).not.toContain(NEW_VALUE); + expect(output).not.toContain(".tmp"); + expect(output).not.toContain(".owner"); + for (const code of RAW_ERRNO_CODES) { + expect(output).not.toContain(code); + } + expect(output).not.toMatch(/\b(open|mkdir|rename|link) '/); +} + +/** Exit 0 with only the success line: no warning, error or raw detail. */ +function expectSuccess(run: CliRun): void { + expect({ exitCode: run.exitCode, stdout: run.stdout }).toEqual({ + exitCode: 0, + stdout: "", + }); + expect(run.stderr).toStartWith("SUCCESS: Updated "); + expect(textLines(run)).toHaveLength(1); + expectNoRawDetail(run); +} + +type JsonFailure = { readonly code: string; readonly message: string }; + +function jsonFailure(run: CliRun): JsonFailure { + expect(run.exitCode).toBe(1); + const envelope: unknown = JSON.parse(run.stdout); + expect(envelope).toMatchObject({ ok: false }); + return (envelope as { readonly error: JsonFailure }).error; +} + +function setArgs(repo: Repo, key: string, ...extra: string[]): string[] { + return [ + "env", + "set", + key, + "--value-stdin", + ...extra, + "--path", + repo.projectRoot, + ]; +} + +/** Names in `directory` that a failed or finished write must not leave. */ +async function leftovers(directory: string): Promise { + const names = await readdir(directory).catch(() => [] as string[]); + return names.filter( + (name) => + name.endsWith(".tmp") || name.endsWith(".owner") || name.endsWith(".lock") + ); +} + +// --------------------------------------------------------------------------- +// 1. No hard links: the exclusive publish falls back to reservation + rename. +// --------------------------------------------------------------------------- + +/** Every `link(2)` fails as it does on ExFAT (macOS) or FAT (Linux). */ +function noHardLinks(code: "ENOTSUP" | "EPERM") { + return { code, step: "link", path: "." } as const; +} + +for (const code of ["ENOTSUP", "EPERM"] as const) { + for (const git of [false, true]) { + test(`with link(2) failing ${code}, the first --secret ${git ? "in a git checkout " : ""}creates the key and later writes still lock`, async () => { + const repo = await createRepo(); + if (git) { + await runGit(repo, ["init", "-q"]); + } + const keyPath = resolve(repo.projectRoot, ".hack.secret.key"); + + const first = await runCli({ + repo, + args: setArgs(repo, "FIRST_SECRET", "--secret"), + stdin: `${NEW_VALUE}\n`, + inject: noHardLinks(code), + }); + expectSuccess(first); + expectNoRawDetail(first); + const keyText = (await readFile(keyPath, "utf8")).trim(); + expect(keyText.length).toBeGreaterThan(0); + expect((await lstat(keyPath)).mode & 0o777).toBe(0o600); + + // The lock was released (a second write would otherwise time out), the + // key was adopted, and nothing was left behind. + const second = await runCli({ + repo, + args: setArgs(repo, "SECOND_SECRET", "--secret"), + stdin: `${NEW_VALUE}\n`, + inject: noHardLinks(code), + }); + expect(second.exitCode).toBe(0); + expect((await readFile(keyPath, "utf8")).trim()).toBe(keyText); + const layer = await readFile(repo.tracked, "utf8"); + expect(layer).toContain("FIRST_SECRET"); + expect(layer).toContain("SECOND_SECRET"); + expect(layer).toContain(KEPT_VALUE); + expect(layer).not.toContain(NEW_VALUE); + expect(await leftovers(repo.projectRoot)).toEqual([]); + expect(await leftovers(repo.projectDir)).toEqual([]); + expect(await leftovers(resolve(repo.projectDir, ".internal"))).toEqual( + [] + ); + if (git) { + expect(await leftovers(resolve(repo.projectRoot, ".git"))).toEqual([]); + } + }, 60_000); + } +} + +test("with link(2) failing ENOTSUP, concurrent sets of different keys are all kept", async () => { + const repo = await createRepo(); + const keys = ["ALPHA_SETTING", "BRAVO_SETTING", "CHARLIE_SETTING"]; + const runs = await Promise.all( + keys.map( + async (key) => + await runCli({ + repo, + args: setArgs(repo, key), + stdin: `${NEW_VALUE}\n`, + inject: noHardLinks("ENOTSUP"), + }) + ) + ); + expect(runs.map((run) => run.exitCode)).toEqual([0, 0, 0]); + const layer = await readFile(repo.tracked, "utf8"); + for (const key of keys) { + expect(layer).toContain(key); + } + expect(layer).toContain(KEPT_VALUE); + expect(await leftovers(resolve(repo.projectDir, ".internal"))).toEqual([]); +}, 60_000); + +test("only link(2) errors that mean no hard links select the fallback", () => { + for (const code of ["ENOTSUP", "EOPNOTSUPP", "EPERM"]) { + expect(isHardLinkUnsupportedError({ code })).toBe(true); + } + for (const code of ["EEXIST", "EACCES", "EXDEV", "ENOENT", "EMLINK"]) { + expect(isHardLinkUnsupportedError({ code })).toBe(false); + } + expect(isHardLinkUnsupportedError("ENOTSUP")).toBe(false); + expect(isHardLinkUnsupportedError(null)).toBe(false); +}); + +async function scratch(): Promise { + const directory = await mkdtemp(join(tmpdir(), "hack-publish-")); + tempDirs.add(directory); + return directory; +} + +test("the reservation fallback publishes the complete file and consumes the source", async () => { + const directory = await scratch(); + const source = join(directory, ".target.staged.tmp"); + const target = join(directory, "target"); + await writeFile(source, "complete\n", { mode: 0o600 }); + + expect(await publishByExclusivePlaceholder({ source, target })).toBe(true); + expect(await readFile(target, "utf8")).toBe("complete\n"); + expect(await readdir(directory)).toEqual(["target"]); +}); + +test("the reservation fallback never replaces an existing file", async () => { + const directory = await scratch(); + const source = join(directory, ".target.staged.tmp"); + const target = join(directory, "target"); + await writeFile(source, "mine\n"); + await writeFile(target, "theirs\n"); + + expect(await publishByExclusivePlaceholder({ source, target })).toBe(false); + expect(await readFile(target, "utf8")).toBe("theirs\n"); + expect(await readFile(source, "utf8")).toBe("mine\n"); +}); + +test("the reservation fallback never follows or replaces a symlink", async () => { + const directory = await scratch(); + const source = join(directory, ".target.staged.tmp"); + const target = join(directory, "target"); + const elsewhere = join(directory, "elsewhere"); + await writeFile(source, "mine\n"); + await symlink(elsewhere, target); + + expect(await publishByExclusivePlaceholder({ source, target })).toBe(false); + expect(await readlink(target)).toBe(elsewhere); + expect(await readdir(directory)).not.toContain("elsewhere"); +}); + +test("a failed rename removes the reservation it created", async () => { + const directory = await scratch(); + const target = join(directory, "target"); + let caught: unknown; + try { + await publishByExclusivePlaceholder({ + source: join(directory, "missing-source"), + target, + }); + } catch (error: unknown) { + caught = error; + } + expect(caught).toMatchObject({ code: "ENOENT" }); + expect(await readdir(directory)).toEqual([]); +}); + +// Real ExFAT: opt-in on macOS, because it attaches and mounts a disk image. +// `HACK_TEST_EXFAT=1 bun test tests/project-env-write-refusal-gaps.test.ts` +const EXFAT_ENABLED = + process.platform === "darwin" && process.env.HACK_TEST_EXFAT === "1"; + +async function sh( + command: readonly string[] +): Promise<{ readonly exitCode: number; readonly stdout: string }> { + const proc = Bun.spawn([...command], { stdout: "pipe", stderr: "pipe" }); + const [stdout, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + proc.exited, + ]); + return { exitCode, stdout }; +} + +test.skipIf(!EXFAT_ENABLED)( + "on a real ExFAT volume the first --secret creates the key, in and out of git", + async () => { + const base = await mkdtemp(join(tmpdir(), "hack-exfat-")); + tempDirs.add(base); + const image = join(base, "volume.img"); + const mountPoint = join(base, "mnt"); + await mkdir(mountPoint); + expect((await sh(["mkfile", "-n", "64m", image])).exitCode).toBe(0); + const attached = await sh([ + "hdiutil", + "attach", + "-nomount", + "-imagekey", + "diskimage-class=CRawDiskImage", + image, + ]); + const device = attached.stdout.trim().split(/\s+/)[0] ?? ""; + expect(device).toMatch(/^\/dev\/disk\d+$/); + try { + expect( + (await sh(["newfs_exfat", "-v", "hackexfat", device])).exitCode + ).toBe(0); + expect( + (await sh(["diskutil", "mount", "-mountPoint", mountPoint, device])) + .exitCode + ).toBe(0); + const mounts = await sh(["mount"]); + expect(mounts.stdout).toMatch(new RegExp(`${device} on .*\\(exfat`)); + for (const git of [false, true]) { + const root = await mkdtemp(join(mountPoint, "case-")); + const projectRoot = join(root, "repo"); + const projectDir = join(projectRoot, ".hack"); + await mkdir(projectDir, { recursive: true }); + await writeFile( + join(projectDir, PROJECT_COMPOSE_FILENAME), + "services:\n api:\n image: alpine:3.20\n" + ); + await writeFile( + join(projectDir, PROJECT_CONFIG_FILENAME), + `${JSON.stringify({ name: "env-exfat", dev_host: "env-exfat.hack" })}\n` + ); + const tracked = resolveProjectEnvConfigPath({ + projectDir, + envName: null, + }); + await writeFile(tracked, LAYER); + const repo: Repo = { root, projectRoot, projectDir, tracked }; + if (git) { + await runGit(repo, ["init", "-q"]); + } + for (const key of ["FIRST_SECRET", "SECOND_SECRET"]) { + const run = await runCli({ + repo, + args: setArgs(repo, key, "--secret"), + stdin: `${NEW_VALUE}\n`, + }); + expectSuccess(run); + } + const keyText = ( + await readFile(join(projectRoot, ".hack.secret.key"), "utf8") + ).trim(); + expect(keyText.length).toBeGreaterThan(0); + expect(await readFile(tracked, "utf8")).toContain("SECOND_SECRET"); + expect(await leftovers(join(projectDir, ".internal"))).toEqual([]); + if (git) { + expect(await leftovers(join(projectRoot, ".git"))).toEqual([]); + } + } + } finally { + await sh(["diskutil", "unmount", "force", mountPoint]); + await sh(["hdiutil", "detach", "-force", device]); + } + }, + 120_000 +); + +// --------------------------------------------------------------------------- +// 2. A symlinked or non-directory `.hack/.internal` refuses hydrate closed. +// --------------------------------------------------------------------------- + +const INTERNAL_REFUSAL = (internal: string) => + `E_PATH_CONFLICT: Could not create directory ${internal}: that path is a symlink or not a directory. Replace it with a regular directory, then retry.`; + +for (const kind of ["dangling", "live"] as const) { + test(`hydrate with a ${kind} .hack/.internal symlink is a closed E_PATH_CONFLICT`, async () => { + const repo = await createRepo(); + const internal = resolve(repo.projectDir, ".internal"); + const elsewhere = resolve(repo.root, "elsewhere"); + if (kind === "live") { + await mkdir(elsewhere); + } + await symlink(elsewhere, internal); + + for (const logger of ["console", "clack", "gum"]) { + const run = await runCli({ + repo, + args: ["env", "hydrate", "--path", repo.projectRoot], + stdin: HYDRATE_INPUT, + logger, + }); + expectOneLineRefusal(run, INTERNAL_REFUSAL(internal)); + } + const json = await runCli({ + repo, + args: ["env", "hydrate", "--json", "--path", repo.projectRoot], + stdin: HYDRATE_INPUT, + }); + expect(jsonFailure(json)).toEqual({ + code: "E_PATH_CONFLICT", + message: INTERNAL_REFUSAL(internal).replace("E_PATH_CONFLICT: ", ""), + }); + // Nothing was created through the link, and the link is untouched. + expect(await readlink(internal)).toBe(elsewhere); + expect(await readdir(elsewhere).catch(() => null)).toEqual( + kind === "live" ? [] : null + ); + expect(await readFile(repo.tracked, "utf8")).toBe(LAYER); + }, 60_000); +} + +test("ensureDirForWrite refuses a dangling symlink or a file closed, not as EEXIST", async () => { + const directory = await scratch(); + const dangling = join(directory, "dangling"); + const file = join(directory, "file"); + await symlink(join(directory, "nowhere"), dangling); + await writeFile(file, "x"); + for (const path of [dangling, file]) { + let caught: unknown; + try { + await ensureDirForWrite(path); + } catch (error: unknown) { + caught = error; + } + expect(caught).toBeInstanceOf(FileWriteRefusedError); + expect(caught).toMatchObject({ + code: "E_PATH_CONFLICT", + failure: "not_a_directory_entry", + message: INTERNAL_REFUSAL(path).replace("E_PATH_CONFLICT: ", ""), + }); + } + // A real directory (new or existing) is fine. + await ensureRealDirectoryForWrite(join(directory, "fresh")); + await ensureRealDirectoryForWrite(join(directory, "fresh")); + expect((await lstat(join(directory, "fresh"))).isDirectory()).toBe(true); +}); + +// --------------------------------------------------------------------------- +// 3. A too-long `--env` name is a closed E_USAGE; ENAMETOOLONG is closed too. +// --------------------------------------------------------------------------- + +const LONGEST_NAME = "e".repeat(ENV_CONFIG_NAME_MAX_LENGTH); +const TOO_LONG_NAME = "e".repeat(215); + +test("overlay names are accepted up to the limit and refused beyond it", () => { + expect(ENV_CONFIG_NAME_MAX_LENGTH).toBe(128); + expect(parseEnvConfigSelection(LONGEST_NAME)).toBe(LONGEST_NAME); + expect(parseEnvConfigSelection(`${LONGEST_NAME}e`)).toBeUndefined(); + expect(parseEnvConfigSelection(TOO_LONG_NAME)).toBeUndefined(); + expect(INVALID_ENV_SELECTION_MESSAGE).toContain("at most 128 characters"); +}); + +test("set, unset and hydrate refuse a too-long --env before touching the project", async () => { + const repo = await createRepo(); + for (const args of [ + setArgs(repo, "NEW_SETTING", "--env", TOO_LONG_NAME), + ["env", "unset", "KEPT_SETTING", "--env", TOO_LONG_NAME], + ]) { + const run = await runCli({ repo, args, stdin: `${NEW_VALUE}\n` }); + expect(run.exitCode).toBe(1); + // A usage error found while parsing arguments also prints the help. + expect(run.stderr).toContain(`ERROR: ${INVALID_ENV_SELECTION_MESSAGE}`); + const output = `${run.stdout}${run.stderr}`; + for (const raw of ["ENAMETOOLONG", ".owner", "hack.env.e", NEW_VALUE]) { + expect(output).not.toContain(raw); + } + expect(output).not.toMatch(/\bat\s.*\.ts:\d+/); + expect(output).not.toContain(TOO_LONG_NAME); + } + const json = await runCli({ + repo, + args: [ + "env", + "hydrate", + "--json", + "--env", + TOO_LONG_NAME, + "--path", + repo.projectRoot, + ], + stdin: HYDRATE_INPUT, + }); + expect(jsonFailure(json)).toEqual({ + code: "E_USAGE", + message: INVALID_ENV_SELECTION_MESSAGE, + }); + expect((await readdir(repo.projectDir)).sort()).toEqual( + [ + PROJECT_COMPOSE_FILENAME, + PROJECT_CONFIG_FILENAME, + basename(repo.tracked), + ].sort() + ); +}, 60_000); + +test("an overlay name at the limit still writes its layer", async () => { + const repo = await createRepo(); + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SETTING", "--env", LONGEST_NAME), + stdin: `${NEW_VALUE}\n`, + }); + expectSuccess(run); + const overlay = resolveProjectEnvConfigPath({ + projectDir: repo.projectDir, + envName: LONGEST_NAME, + }); + expect(await readFile(overlay, "utf8")).toContain("NEW_SETTING"); +}, 60_000); + +const NAME_TOO_LONG_REASON = + "a file name or the path is too long for the filesystem. Use a shorter name or move the project to a shorter path, then retry."; + +test("an injected ENAMETOOLONG at the layer staging and at its lock is a closed E_PATH_CONFLICT", async () => { + const repo = await createRepo(); + const stagingPattern = "/\\.hack\\.env\\.default\\.yaml\\.[^/]+\\.tmp$"; + const lockOwnerPattern = + "/hack\\.env\\.default\\.yaml\\.lock\\.[^/]+\\.owner$"; + for (const [path, expected] of [ + [ + stagingPattern, + `E_PATH_CONFLICT: Refusing to replace ${repo.tracked}: ${NAME_TOO_LONG_REASON}`, + ], + [ + lockOwnerPattern, + `E_PATH_CONFLICT: Could not write ${repo.tracked}: ${NAME_TOO_LONG_REASON}`, + ], + ] as const) { + for (const args of [ + setArgs(repo, "NEW_SETTING"), + ["env", "unset", "KEPT_SETTING", "--path", repo.projectRoot], + ]) { + const run = await runCli({ + repo, + args, + stdin: `${NEW_VALUE}\n`, + inject: { code: "ENAMETOOLONG", step: "open", path }, + }); + expectOneLineRefusal(run, expected); + } + } + expect(await readFile(repo.tracked, "utf8")).toBe(LAYER); + expect(await leftovers(repo.projectDir)).toEqual([]); +}, 60_000); + +// --------------------------------------------------------------------------- +// 4. A stale project env key lock is a closed E_LOCKED naming the remedy. +// --------------------------------------------------------------------------- + +test("a key lock held by a live unrelated PID times out as a closed E_LOCKED", async () => { + const repo = await createRepo(); + await runGit(repo, ["init", "-q"]); + const lockPath = resolve( + await realpath(repo.projectRoot), + ".git", + ".hack.secret.key.lock" + ); + // This test runner is alive and is not the CLI child: a reused PID. + await writeFile(lockPath, `${process.pid}\n`, { mode: 0o600 }); + const expected = `E_LOCKED: Timed out waiting for the project env key lock ${lockPath}; nothing was written. Another hack command may be creating the key: retry when it finishes. If no other hack command is running, the lock is stale: remove ${lockPath}, then retry.`; + + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SECRET", "--secret"), + stdin: `${NEW_VALUE}\n`, + }); + expectOneLineRefusal(run, expected); + expect(await readFile(lockPath, "utf8")).toBe(`${process.pid}\n`); + expect((await readdir(repo.projectRoot)).sort()).toEqual([".git", ".hack"]); + expect(await readFile(repo.tracked, "utf8")).toBe(LAYER); +}, 60_000); + +test("a lock timeout reports a busy recovery guard, and the key refusal names it", async () => { + const directory = await scratch(); + const lockPath = join(directory, "resource.lock"); + // A dead owner (a PID far above any pid_max) with its recovery guard held. + await writeFile(lockPath, "2147483646\n", { mode: 0o600 }); + await mkdir(`${lockPath}.recovery`); + let caught: unknown; + try { + await withProjectsRegistryLock({ + lockPath, + timeoutMs: 50, + run: async () => "ran", + }); + } catch (error: unknown) { + caught = error; + } + expect(caught).toBeInstanceOf(ProjectsRegistryLockTimeoutError); + expect((caught as ProjectsRegistryLockTimeoutError).recoveryBusy).toBe(true); + + const plain = new ProjectEnvKeyLockTimeoutError({ + lockPath, + recoveryBusy: false, + }); + const busy = new ProjectEnvKeyLockTimeoutError({ + lockPath, + recoveryBusy: true, + }); + expect(plain).toBeInstanceOf(HackCliError); + expect(plain.code).toBe("E_LOCKED"); + expect(plain.message).toEndWith( + `the lock is stale: remove ${lockPath}, then retry.` + ); + expect(busy.message).toEndWith( + `the lock is stale: remove ${lockPath} and the directory ${lockPath}.recovery, then retry.` + ); +}); + +// --------------------------------------------------------------------------- +// 5. A `.hack/.gitignore` directory is E_PATH_CONFLICT, as docs/env.md says. +// --------------------------------------------------------------------------- + +test("a .hack/.gitignore directory is E_PATH_CONFLICT for set --local and hydrate --json", async () => { + const repo = await createRepo(); + const gitignore = resolve(repo.projectDir, ".gitignore"); + await mkdir(gitignore); + + for (const logger of ["console", "clack", "gum"]) { + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SETTING", "--local"), + stdin: `${NEW_VALUE}\n`, + logger, + }); + expectOneLineRefusal(run, `E_PATH_CONFLICT: ${HACK_DIR_GITIGNORE_REFUSAL}`); + } + const json = await runCli({ + repo, + args: ["env", "hydrate", "--json", "--path", repo.projectRoot], + stdin: HYDRATE_INPUT, + }); + expect(jsonFailure(json)).toEqual({ + code: "E_PATH_CONFLICT", + message: HACK_DIR_GITIGNORE_REFUSAL, + }); + expect((await lstat(gitignore)).isDirectory()).toBe(true); + expect(new HackDirGitignoreRefusalError().code).toBe("E_PATH_CONFLICT"); +}, 60_000); + +// --------------------------------------------------------------------------- +// 6. Mappings whose revert used to fail no test. +// --------------------------------------------------------------------------- + +const KEY_LOCK_OWNER_PATTERN = + "/\\.hack\\.secret\\.key\\.lock\\.[^/]+\\.owner$"; + +// The git dir cannot hold the key lock: the key is created unserialized, as +// before, instead of the lock failure being reported. +for (const code of ["ENOENT", "EPERM", "EROFS", "EACCES"] as const) { + test(`an injected ${code} at the key lock creates the key without the lock`, async () => { + const repo = await createRepo(); + await runGit(repo, ["init", "-q"]); + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SECRET", "--secret"), + stdin: `${NEW_VALUE}\n`, + inject: { code, step: "open", path: KEY_LOCK_OWNER_PATTERN }, + }); + expectSuccess(run); + const key = await readFile( + resolve(repo.projectRoot, ".hack.secret.key"), + "utf8" + ); + expect(key.trim().length).toBeGreaterThan(0); + expect(await readFile(repo.tracked, "utf8")).toContain("NEW_SECRET"); + expect(await leftovers(resolve(repo.projectRoot, ".git"))).toEqual([]); + }, 60_000); +} + +const ATOMIC_WRITE_REFUSALS: Readonly< + Record +> = { + unreadable: [ + "E_PERMISSION", + "it exists but cannot be read, so its contents cannot be kept", + ], + not_regular_file: ["E_PATH_CONFLICT", "it is not a regular file"], + not_writable: ["E_PERMISSION", "it is not writable"], + owner_not_preserved: [ + "E_PERMISSION", + "its owner and group cannot be preserved", + ], + group_not_preserved: [ + "E_PERMISSION", + "its group cannot be preserved because you are not a member of that group", + ], + directory_not_writable: [ + "E_PERMISSION", + "its directory is not writable. Make that directory writable by your user, then retry", + ], + read_only_filesystem: [ + "E_PERMISSION", + "the filesystem is read-only. Use a writable checkout or remount the filesystem read-write, then retry", + ], + no_space: [ + "E_NO_SPACE", + "no space is left on the device or the disk quota is exceeded. Free some disk space or quota, then retry", + ], + not_a_directory: [ + "E_PATH_CONFLICT", + "a parent of that path is not a directory. Replace that path with a directory, then retry", + ], + name_too_long: ["E_PATH_CONFLICT", NAME_TOO_LONG_REASON.replace(/\.$/, "")], +}; + +test("every atomic-write refusal has its fixed code and message", () => { + for (const [reason, [code, text]] of Object.entries(ATOMIC_WRITE_REFUSALS)) { + const refusal = new AtomicWriteRefusedError({ + path: "/synthetic/layer.yaml", + reason: reason as AtomicWriteRefusal, + }); + expect({ reason, code: refusal.code, message: refusal.message }).toEqual({ + reason, + code, + message: `Refusing to replace /synthetic/layer.yaml: ${text}.`, + }); + } +}); + +test("a directory at the layer path is a closed E_PATH_CONFLICT", async () => { + const repo = await createRepo(); + await rm(repo.tracked); + await mkdir(repo.tracked); + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SETTING"), + stdin: `${NEW_VALUE}\n`, + }); + expectOneLineRefusal( + run, + `E_PATH_CONFLICT: Refusing to replace ${repo.tracked}: it is not a regular file.` + ); +}, 60_000); + +test.skipIf(RUNS_AS_ROOT)( + "a read-only layer is a closed E_PERMISSION not_writable", + async () => { + const repo = await createRepo(); + await chmod(repo.tracked, 0o444); + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SETTING"), + stdin: `${NEW_VALUE}\n`, + }); + expectOneLineRefusal( + run, + `E_PERMISSION: Refusing to replace ${repo.tracked}: it is not writable.` + ); + expect(await readFile(repo.tracked, "utf8")).toBe(LAYER); + }, + 60_000 +); + +test("an injected EISDIR while staging the layer is a closed not_regular_file refusal", async () => { + const repo = await createRepo(); + const run = await runCli({ + repo, + args: setArgs(repo, "NEW_SETTING"), + stdin: `${NEW_VALUE}\n`, + inject: { + code: "EISDIR", + step: "open", + path: "/\\.hack\\.env\\.default\\.yaml\\.[^/]+\\.tmp$", + }, + }); + expectOneLineRefusal( + run, + `E_PATH_CONFLICT: Refusing to replace ${repo.tracked}: it is not a regular file.` + ); + expect(await readFile(repo.tracked, "utf8")).toBe(LAYER); + expect(await leftovers(repo.projectDir)).toEqual([]); +}, 60_000); diff --git a/tests/project-env-write-refusals.test.ts b/tests/project-env-write-refusals.test.ts index 18beaa8e0..013612b25 100644 --- a/tests/project-env-write-refusals.test.ts +++ b/tests/project-env-write-refusals.test.ts @@ -838,10 +838,12 @@ test.skipIf(RUNS_AS_ROOT)( ); // The CLI prints a closed error as one line; other code still narrows on -// these classes with instanceof, so they keep their names and messages. +// these classes with instanceof, so they keep their names and messages. The +// .hack/.gitignore refusal is a path of the wrong kind (HACK-1377). test("project key and .hack/.gitignore refusals are closed errors", () => { const gitignore = new HackDirGitignoreRefusalError(); - expect(gitignore).toBeInstanceOf(ClosedCliError); + expect(gitignore).toBeInstanceOf(HackCliError); + expect(gitignore.code).toBe("E_PATH_CONFLICT"); expect(gitignore).toBeInstanceOf(HackDirGitignoreRefusalError); expect(gitignore.name).toBe("HackDirGitignoreRefusalError"); expect(gitignore.message).toBe(HACK_DIR_GITIGNORE_REFUSAL); @@ -1221,6 +1223,7 @@ test("the write-failure classifier maps only its fixed errno set", () => { EDQUOT: ["no_space", "E_NO_SPACE"], ENOTDIR: ["not_a_directory", "E_PATH_CONFLICT"], EISDIR: ["is_a_directory", "E_PATH_CONFLICT"], + ENAMETOOLONG: ["name_too_long", "E_PATH_CONFLICT"], } as const; for (const [code, [failure, errorCode]] of Object.entries(cases)) { const raw = Object.assign(new Error(`${code}: raw, open '/x/.y.tmp'`), {