Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions docs/env.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,12 @@ How it is maintained:
- the entries live inside a marked, hack-managed block; anything you add
outside the markers is preserved on every rewrite
- the root `.gitignore` still gets a `.hack.secret.key` entry when the secret
key is first generated (the key lives at the repo root, not under `.hack/`)
key is first generated (the key lives at the repo root, not under `.hack/`).
Hack writes that entry before the key, so if the entry cannot be written, no
key is created. Before a `--secret` set or hydrate uses an existing
checkout-local key that git does not ignore, Hack adds the entry; if it
cannot, or a later rule re-includes the key, the command is refused and
nothing is encrypted with that key
- older repos that added `.hack/.internal/` to the root `.gitignore` keep
working; the entries are equivalent and nothing is removed

Expand Down Expand Up @@ -649,6 +654,34 @@ hack env unset SERVICE_TOKEN --service api --env qa
hack env unset API_BASE_URL --local
```

### Concurrent writes to one env file

Hack env commands that change an env file never silently lose another command's change.
`hack env set`, `hack env add`, `hack env unset` and `hack env hydrate` each read the
file, change it and replace it while holding that file's write lock,
`.hack/.internal/<file name>.lock` (for example `.hack/.internal/hack.env.yaml.lock`).
The same applies to the legacy `.hack/.env` files. So when several commands change
different keys of the same file at once, every change is kept. A command
that finds the lock held waits up to 10 seconds, then stops with a one-line message that
names the file and says nothing was written; run it again. The lock is released when the
command ends. If a command is killed while it holds the lock, the next command takes the
lock over once it confirms that process has exited.

`.hack/.internal/` is ignored by the managed `.hack/.gitignore`, so lock files never show
in `git status`. If `.hack/.internal` is a symlink or not a directory, these commands
refuse instead of creating the lock through it. If you cannot write the directory that
holds the env file (for example a read-only checkout), no lock is taken: no command can
replace the file there, so a command that changes nothing still succeeds and one that
would change the file is refused as before.

Programs that do not take this lock, such as an editor or Hack v4, are not kept out. For
`hack.env*.yaml` files, `hack env set`, `add`, `unset` and `hydrate` also check that the
file still has the contents they read just before replacing it. If it changed, the
command writes nothing and stops with a retryable conflict ("The file changed while it
was being replaced; nothing was written."), or, for `hydrate`, a `concurrent_write`
refusal. That check narrows the window in which such a program's write can be
overwritten, but cannot close it.

### Hydrate machine-local values in one step

`hack env hydrate` imports many values into the machine-local layer at once, for example
Expand Down Expand Up @@ -716,15 +749,17 @@ Hydration refuses, without changing the layer, when:
re-run with `--secret`, or remove the key with `hack env unset <key> --local` first
- another `hack env hydrate` for the same project is running, or an interrupted one left
a stale lock (see below)
- the local layer changed between hydration's read and its rename, for example from a
concurrent `hack env set --local`
- the local layer changed between hydration's read and its rename, by a program that
does not take the env file's write lock (for example an editor or Hack v4)
- hydration waited 10 seconds for the env file's write lock without getting it

Concurrent hydrations are serialized by `.hack/.internal/env-hydrate.lock`, which holds
the owner's process ID. A hydration that is killed (for example with SIGKILL) cannot
remove it; the next hydration then reports the stale lock and its path instead of taking
it over. Remove the file once no hydration is running. `hack env set --local` does not
take this lock. Hydration checks the layer again just before its rename, which narrows
the window in which such a write could be overwritten but cannot close it.
take this lock, but hydration also holds the local file's write lock (see
[Concurrent writes to one env file](#concurrent-writes-to-one-env-file)), so a
concurrent `hack env set --local` waits for the hydration and both changes are kept.

Materialize a compatibility `.hack/.env`:

Expand Down Expand Up @@ -885,6 +920,7 @@ it cannot spell portably as `<unprintable-path>/.hack.secret.key`.
| `env-key-wrong-owner` | warn | the file is not owned by the current user |
| `env-key-wrong-length` | warn | shorter than the 43-character base64url form of a 256-bit key, or empty |
| `env-key-low-entropy` | warn | long enough but repetitive (estimated under 128 bits) |
| `env-key-unignored` | warn | the checkout-local key is not ignored by git, so `git status` offers it for commit; suggests adding it to the root `.gitignore` |
| `env-key-symlink` | error | the key path is a symlink; Hack refuses symlinked keys |
| `env-key-not-regular` | error | the key path is a directory or another non-file entry |
| `env-key-unreadable` | error | unreadable, oversized or not UTF-8 |
Expand Down
11 changes: 8 additions & 3 deletions docs/reference/application-benchmark-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,13 @@ A different or missing value is refused (`artifact_identity_mismatch`,
frozen correctness check (`workload_correctness_weakened`). A revision may add
correctness checks. The sample counts are floors at their frozen values: a protocol
that plans fewer than 10 lifecycle runs or 60 tail samples per lane is refused, even
where the program floor or the p95 bound would allow fewer. Every number in the protocol or a summary must be
within ±(2⁵³ − 1), the interoperable JSON range; a larger value is refused as malformed.
where the program floor or the p95 bound would allow fewer. Every number anywhere in
the protocol or a summary, including a field the checker does not otherwise read, must
be finite and within ±(2⁵³ − 1), the interoperable JSON range. Any other number,
including the `NaN` and `Infinity` that Python's JSON parser accepts, is refused as
`protocol_number_out_of_range` or `summary_number_out_of_range`, alongside any code
for the field that holds it. A mean is the correctly rounded sum divided by the
count, so a mean at an exclusive ceiling gets the same verdict on every Python version.

These are acceptance thresholds, not measured achievements.

Expand All @@ -315,7 +320,7 @@ record. It exits 1 when any verdict fails or is inconclusive. It exits 2, withou
judging, when the protocol or summary is invalid: for example a summary for another
cache class or scale point, a candidate marked `not_applicable`, a `not_applicable`
record that carries samples, a negative sample, a planned count that is not a positive
integer, or a value of the wrong type. Diagnostics are closed codes and never repeat
integer, a number outside ±(2⁵³ − 1), or a value of the wrong type. Diagnostics are closed codes and never repeat
input values.

A summary has this shape. `samples` lists only admitted samples; `flagged` and `failed`
Expand Down
13 changes: 13 additions & 0 deletions docs/reference/ax-target-result-contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ fixed text that never repeats caller input, paths or registry values.
| `path_not_found` | `E_PROJECT_NOT_FOUND` | `project` | `fix_selector` |
| `no_project_context` | `E_PROJECT_NOT_FOUND` | `project` | `fix_selector` |
| `project_inputs_unsupported` | `E_CONFIG_INVALID` | `project` | `fix_project` |
| `adoption_state_unsafe` | `E_CONFIG_INVALID` | `project` | `fix_project` |
| `config_parse_failed` | `E_CONFIG_PARSE` | `project` | `fix_project` |
| `compose_parse_failed` | `E_CONFIG_PARSE` | `project` | `fix_project` |
| `detached_checkout` | `E_USAGE` | `instance` | `fix_selector` |
Expand All @@ -163,6 +164,16 @@ read permission, or a directory where a config file belongs) is reported as
filesystem error). The OS message is never surfaced, because it contains the
absolute path.

The shared project input guards that every project command runs are mapped
too. Native or conflicting project files are `project_inputs_unsupported`. A
legacy adoption state that is unparseable, interrupted, or cannot be checked
(for example a project directory the caller cannot search) is
`adoption_state_unsafe`. A non-directory where the project directory belongs is
`input_unreadable`. When the guard cannot check whether a project input exists
(for example a `.dev` file, or a `.dev` directory the caller cannot search,
beside a valid `.hack`), the refusal is `input_permission_denied` (`EACCES`,
`EPERM`) or `input_unreadable` (any other fault), like a read fault.

`retry` is advice for the caller. Like suggested actions and application logs,
it carries no execution authority.

Expand Down Expand Up @@ -208,6 +219,8 @@ WU21). The inspector itself only produces snapshots.
| Two registrations owning one checkout | `ambiguous_project` | same | paired fixture |
| Malformed Hack config or Compose file, oversized input | `config_parse_failed`, `compose_parse_failed`, `generation_input_too_large` | same | paired fixture |
| Unreadable input (directory in place of a file, no read permission) | `input_unreadable`, `input_permission_denied` | same | paired fixture (permission case skipped as root) |
| Input guard refusals (native/legacy conflict, unsafe adoption state, unsearchable or non-directory project dir) | `project_inputs_unsupported`, `adoption_state_unsafe`, `input_unreadable` | same | paired fixture (unsearchable case skipped as root) |
| Uncheckable input presence (`.dev` file or unsearchable `.dev` beside a valid `.hack`) | `input_unreadable`, `input_permission_denied` | same | paired fixture and domain test (unsearchable case skipped as root) |
| Detached linked worktree, no project, missing path | closed refusals | same | paired fixture |
| Unknown service | `service_not_found` | same | paired fixture |
| Config changed during read | `generation_changed_during_read` | same code path | domain test |
Expand Down
2 changes: 1 addition & 1 deletion docs/security/env-secrets-threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ refer to `hack-dance/hack`.

| Threat | Mitigation on `next` |
| --- | --- |
| The key is committed. | The root `.gitignore` gets a `.hack.secret.key` entry when the key is created. Encryption refuses a git-tracked key, including an inherited primary key, before it is copied (#288). `hack doctor` reports `env-key-tracked` at any depth (#309) and the "generated files" check can untrack a root key with `--fix`. |
| The key is committed. | The root `.gitignore` gets a `.hack.secret.key` entry before the key is created, so a failed entry write leaves no key; an existing checkout-local key git does not ignore is repaired or refused before a `--secret` write uses it, and `hack doctor` reports it as `env-key-unignored` (HACK-1375). Encryption refuses a git-tracked key, including an inherited primary key, before it is copied (#288). `hack doctor` reports `env-key-tracked` at any depth (#309) and the "generated files" check can untrack a root key with `--fix`. |
| A key copy ships in build output or a static site. | `hack doctor` reports `env-key-public-path` for copies under `public/`, `static/`, `dist/`, `build/`, `out/`, `www/`, `_site/`, `.next/` and `.output/` (#309). |
| A weak, short or repetitive key. | `env-key-wrong-length` and `env-key-low-entropy` (#309). Generated keys are 32 random bytes in base64url. |
| Materialized `.hack/.env` or local overrides are committed. | Hack owns a committed `.hack/.gitignore` block, and `hack doctor` flags tracked generated files (see [ignore rules](../env.md#ignore-rules-committed-hackgitignore)). |
Expand Down
39 changes: 37 additions & 2 deletions scripts/check-application-benchmark-protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
Revision-1 identities (artifacts, host, lane kinds and backends, candidate capacity,
workload pins, rotated lane order) are exact; budgets and sample counts are floors at
their frozen values, which a revision may tighten.
Every number must lie within MAX_JSON_NUMBER.
Every number anywhere in the protocol or a summary must be finite and lie within
MAX_JSON_NUMBER, including fields the checker does not otherwise read.
Diagnostics are closed codes from DIAGNOSTICS; they never echo input values or paths.
A budget verdict is one of VERDICTS. A statistic is judged only from complete,
sufficient, admitted samples: missing, short, flagged or unaccounted samples are
Expand Down Expand Up @@ -197,8 +198,10 @@
"statistic_unknown",
"observer_policy_missing",
"known_limit_malformed",
"protocol_number_out_of_range",
"summary_unreadable",
"summary_not_object",
"summary_number_out_of_range",
"summary_revision_mismatch",
"summary_workload_unknown",
"summary_cache_class_mismatch",
Expand Down Expand Up @@ -313,6 +316,24 @@ def _number(value):
and -MAX_JSON_NUMBER <= value <= MAX_JSON_NUMBER


def _numbers_in_range(value):
"""True when every number anywhere in a JSON value satisfies _number.

Booleans are not numbers and object keys are strings, so neither is checked. The
walk is iterative, so a deeply nested value cannot raise RecursionError.
"""
pending = [value]
while pending:
node = pending.pop()
if isinstance(node, dict):
pending.extend(node.values())
elif isinstance(node, list):
pending.extend(node)
elif isinstance(node, (int, float)) and not isinstance(node, bool) and not _number(node):
return False
return True


def _same(value, frozen):
"""Exact JSON equality: unlike ==, 4.0 is not 4 and true is not 1."""
return json.dumps(value, sort_keys=True) == json.dumps(frozen, sort_keys=True)
Expand Down Expand Up @@ -554,6 +575,8 @@ def validate_protocol(doc):
if not isinstance(doc, dict):
return ["protocol_not_object"]
found = set()
if not _numbers_in_range(doc):
found.add("protocol_number_out_of_range")
if doc.get("protocol") != PROTOCOL_ID:
found.add("protocol_id_mismatch")
if not _count(doc.get("revision")) or doc["revision"] != PROTOCOL_REVISION:
Expand Down Expand Up @@ -647,6 +670,8 @@ def validate_summary(protocol, summary):
if workload is None:
return ["summary_workload_unknown"], None
found = set()
if not _numbers_in_range(summary):
found.add("summary_number_out_of_range")
# Budgets are frozen for the warm class only; other classes are reported separately.
if summary.get("cache_class") != protocol["cache_classes"]["budget_class"]:
found.add("summary_cache_class_mismatch")
Expand Down Expand Up @@ -679,12 +704,22 @@ def validate_summary(protocol, summary):
return sorted(found), workload


def window_mean(samples):
"""The correctly rounded sum divided by the count.

Built-in sum() rounds after every addition, and Python 3.12 changed how, so a mean
at an exclusive ceiling (ten samples of 0.01) could land just below it on one
interpreter and on it on another. math.fsum gives the same verdict everywhere.
"""
return math.fsum(samples) / len(samples)


def _statistic(samples, statistic):
if statistic == "median":
return median(samples)
if statistic == "p95":
return nearest_rank(samples, P95_QUANTILE)
return sum(samples) / len(samples)
return window_mean(samples)


def _sample_floor(protocol, statistic):
Expand Down
21 changes: 18 additions & 3 deletions src/backends/native-project-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ const RUN = /^[a-f0-9]{32}$/;
const MAX_OUTPUT = 16 * 1024 * 1024;
const MAX_INPUT = 256 * 1024;

/**
* Exit-relative bound on draining the owned child's stdout and stderr.
*
* The native runtime writes its closed error code (and any readiness line)
* before it exits, so those bytes are already in the pipes when the exit is
* observed; with no other writer, EOF follows immediately and the drain ends
* there. This bound only ends a drain that EOF cannot end: a descendant that
* inherited a pipe and keeps it open. It is measured from the observed exit,
* not from spawn, and is deliberately generous so that a heavily
* oversubscribed host still schedules the pipe readers before it expires.
* A 250 ms bound lost the closed code under load (HACK-1350).
*/
export const NATIVE_EXIT_DRAIN_BOUND_MS = 5000;

export type NativeExitDiagnostic = {
readonly exitCode: number;
readonly nativeCode?: string;
Expand Down Expand Up @@ -317,14 +331,15 @@ async function serveGraphProcess(
}
);
// Descendants may inherit a pipe after the owned receiver exits. Its exit,
// not unrelated writers' EOF, bounds draining; final graph inspection remains
// the caller's authority for cleanup, regardless of a successful exit code.
// not unrelated writers' EOF, bounds draining (see NATIVE_EXIT_DRAIN_BOUND_MS);
// final graph inspection remains the caller's authority for cleanup,
// regardless of a successful exit code.
const drain = new AbortController();
const owner = new AbortController();
let drainTimer: ReturnType<typeof setTimeout> | undefined;
void child.exited.then(() => {
owner.abort();
drainTimer = setTimeout(() => drain.abort(), 250);
drainTimer = setTimeout(() => drain.abort(), NATIVE_EXIT_DRAIN_BOUND_MS);
});
const failureCodes = readNativeFailureCodes(child.stderr, drain.signal);
const failureCode = failureCodes.then((failure) => failure?.code);
Expand Down
2 changes: 2 additions & 0 deletions src/commands/doctor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ import {
import {
describeEnvKeyInspection,
type EnvKeyInspection,
gitIsLocalKeyIgnored,
gitListTrackedKeyPaths,
inspectProjectEnvKey,
resolveEnvKeyCandidates,
Expand Down Expand Up @@ -2990,6 +2991,7 @@ async function checkEnvKeyAtRoot(opts: {
env: process.env,
uid: typeof process.getuid === "function" ? process.getuid() : null,
listTrackedKeyPaths: gitListTrackedKeyPaths,
isLocalKeyIgnored: gitIsLocalKeyIgnored,
});
return envKeyCheckResult(inspection);
}
Expand Down
Loading
Loading