Repository navigation
fix(native): reject Hack ownership label namespaces in runtime-core's label review - #465
Merged
Merged
Conversation
… label review runtime-core's own Compose label review rejected only the caddy, traefik, hack and com.docker.compose prefixes, so a service label in io.hack-local.*, io.hack.native-config.*, io.hack.native-file.* or io.hack.storage-witness.* passed the candidate review if the TypeScript classifier was bypassed. Add a Rust mirror of HACK_OWNERSHIP_LABEL_NAMESPACES and reject those namespaces (and their bare roots) in the review, never exempted by a routing or dependency cache declaration. A TypeScript parity test keeps the mirror identical to the TypeScript source of truth. Refs HACK-1409 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
roodboi
marked this pull request as ready for review
October 24, 2026 19:26
roodboi
added a commit
that referenced
this pull request
Oct 24, 2026
…ches and restore owned listeners on timeout (#497) ## Summary Fix unit G103: the HACK-1455 follow-ups after #469 (storage diagnostics), #464 (global-state guard) and #465 (label review). Diagnostics and test infrastructure only: no carrier check, deadline, retention or recovery rule changes. ## Root causes and fixes ### #469: storage diagnostics 1. **Binding-check call sites were unpinned.** The only test of `assertNativeComposeStorageBindingUnchanged` called the helper directly, and nothing drove the real Docker carrier with a binding that changed mid-step. So any call site could go back to `if (!same(before, x)) return refuse();` unnoticed. New `tests/native-compose-storage-binding- call-sites.test.ts` runs the real `createNativeComposeDockerStorageXattrCarrier` in process on the synthetic daemon from `tests/helpers/native-compose-storage-docker.ts`. It drives an original readonly verify (inspect, journaled invoke, deferred finish) and a cold provision. Only the saved material binding is a stand-in: `assertNativeComposeMaterialAuthority` is spied, and the chosen read returns the same binding with another receipt hash. The table names the read that reaches each call site and the stage it must report. The baseline case pins the total read count (53 for verify, 4 for provision), so a change to the read order fails there first. The case runs with the synthetic `docker` first on `PATH` and an isolated Docker client config, so the engine-ID admission keeps the synthetic CLI and never dials a real socket. `PATH` and the `DOCKER_*` selectors are restored in `finally`, and a timed-out case's carrier is aborted in `afterEach`. 2. **The owner-creation `refuseFrom` was untested.** New `tests/native-compose- storage-command-record.test.ts` cases make the creation check refuse before the record is written (check 1) and after it is read back (check 2). Each asserts the issued boundary and its post-deadline reason. The carrier table's read 6 covers the same path end to end. 3. **The helper's stage was not pinned.** The helper unit test now runs with two stages (`storage-helper- start`, `storage-provision`). The carrier table also checks five different stages. 4. **Remaining untagged mismatches.** - New `assertNativeComposeStorageCarrierStateUnchanged` replaces `same(repeated.state, stopped.state)` and `same(finalCarrier.state, stopped.state)`. A stopped carrier whose state changed is the same instability as a changed creation time or policy, so it reports `storage-helper-policy` / `helper-policy-stability`. - The finalizer's binding check now uses `assertNativeComposeStorageBindingUnchanged` at `storage-helper-cleanup` (the carrier's own cleanup stage). The owner-confirmed check still runs first, as before. - Both reasons are outside `deadlineReasons`, so they never read `storage-budget` past the deadline. The reference docs say so. - A new `fault-carrier-state-changed` synthetic-daemon fault returns another `FinishedAt` on the Nth policy inspection. 5. **The xattr-owner guard storage proof case** now uses the file's `STORE_CASE_BUDGET_MS` instead of Bun's 5 s default. G99 / HACK-1439 has no open PR or remote branch yet, so there was nothing to merge. ### #464: global-state guard 6. **The refused-teardown case could bypass the fixture gate.** It now asserts that it holds the gate (`fixtureActive`, `!fixtureUncertain`). As a plain `boundedTest` it fails. 7. **`restoreProcessListeners`' docstring claimed removal had no effect.** It said that removing a leftover listener "cannot let unsettled work reach the host". That is wrong for cleanup handlers: a leftover `SIGINT`/`SIGTERM` handler that kills a child on Ctrl-C stops running, and the child can outlive an interrupted run. **Decision:** keep report-and-restore to the baseline. The guard does not call the handler or wait for what it owns. It cannot know what the handler owns or whether running it now is safe, and running another file's handler at an arbitrary point is the cross-file interference the restore exists to prevent. The leak report names the file and fails the run, so the fix belongs in that file's teardown. The docstring and the verify skill now say this. 8. **`withOwnedProcessListeners` restored only when its callback settled.** On a timeout the callback never settles, so other code's listeners stayed removed. Its restore is now idempotent and registered three ways: - the callback's own `finally`; - `onTestFinished`, which Bun runs after a timeout; - a process-wide set that the guard preload empties before it compares the test's state. Without this, the guard (whose `afterEach` runs before `onTestFinished`) would blame the timed-out test. A saved `once` listener now comes back as `once`. Bun does not expose `once`: `listeners()` and `rawListeners()` both return the bare function. So the new `tests/helpers/process-once- listeners.ts` wraps the registration methods of `process` and remembers the last registration kind; the preload installs it before any file loads. The guard's file-boundary `restoreProcessListeners` uses the same registry for missing listeners. ### #465: label review 9. `hack_ownership_label_namespaces_block_enrollment` now covers the dependency-cache declaration form. Without an ownership key that form reports no owner label; with each of the four ownership keys it still reports one and refuses enrollment. ## Mutation proof Each mutant was applied to the source and run against its own focused test with a scratch driver; the file was restored byte for byte afterwards. The kill reason was checked in each log; for the binding call sites it is `private-state` becoming `unclassified` (and `storage-budget` once late). | Mutant | Killed by | |---|---| | Each of the 12 binding call sites, one at a time, back to the untagged `same()` + `refuse()` form: observe, provision (latest, admission, after), owner check, admitted, create admission, final, start admission, cleanup authority, remove admission, finalizer | the matching row of the carrier table (verify read 2; provision reads 2, 3, 4; verify reads 6 and 12, 8, 9, 21, 22, 34, 35, 49) | | `repeated` state check back to `if (!same(...)) return refuse()` | state row, inspection 7 | | `finalCarrier` state check back to the untagged form | state rows, inspections 8 and 10 | | Owner creation `return refuseFrom(error)` becomes `return refuse()` | both new command-record cases, plus carrier read 6 | | Helper hard-codes `"storage-helper-start"` | the `storage-provision` row of the helper test | | Guard storage proof case back on Bun's default timeout | that case timed out at 5 s on this host at load average 50-60 (load-dependent, see gaps) | | Refused-teardown case `gatedTest` becomes `boundedTest` | the case itself (gate assertion) | | No `onTestFinished` restore | timed-out nested suite without the preload | | Preload does not call `restoreUnsettledOwnedProcessListeners` | timed-out nested suite with the preload (the guard blames the timed-out test) | | `withOwnedProcessListeners` re-adds every listener as persistent | all 4 listener tests | | Guard `restoreProcessListeners` re-adds every listener as persistent | the extended file-boundary restore test | | runtime-core: ownership labels exempt under a dependency cache | `hack_ownership_label_namespaces_block_enrollment` (owner-label assertion of the new form) | ## Evidence - Bun 1.4.2 (`mise x bun@1.4.2`), `bun install --frozen-lockfile`; no `bun.lock` change. - Root `bun run check`: privacy-check ok, 34 warnings, all existing complexity warnings (`invoke` 59 -> 57). `bun run --cwd packages/cli typecheck`: 0 errors. - gitleaks 8.30.1 (`gitleaks dir` over the `git log -p origin/next..HEAD` export): no leaks. - Rust (cargo +1.97.1): `cargo fmt --check`, `clippy --all-targets -D warnings` and `project_contract hack_ownership_label_namespaces_block_enrollment` all pass. The TS parity scan `tests/hack-ownership-labels.test.ts` passes (3). - Focused files, run serially on a shared Mac at load average 20-60: - new carrier call-site file: 17 pass (285 s at load ~55; each case 4-40 s) - command record: 34 pass - docker witness: 90 pass - listener and guard files: 12 pass - routing gate cases: 4 pass - Loki SIGINT cases: 3 pass - guard storage proof case: pass - policy stability CLI: 4 pass; the fifth hit its 120 s command kill at load ~60 before any helper inspection, then passed alone at load ~20 - Trial merge with the G99 branch `wf/g99-test-reliability-under-load` (HACK-1439; not merged, no PR yet): clean. That branch sets `setDefaultTimeout(STORE_CASE_BUDGET_MS)` for the whole xattr-owner file, which agrees with the explicit budget here. - Full suite: hosted CI at the exact PR head a8f6c35 (run 40129069973) passed `test` (shards 1-4), docker-e2e (shards 1-4), Linux and macOS candidate core, the Linux and macOS-arm config compilers, runtime state models, linux-process-lifetime, runtime-images and secret-scan. On the hosted runner, the 19 new carrier and listener cases took 0.06-0.9 s each. - **One failure: `Config compiler (macos-15-intel)`.** `tests/native-compose-file- owner.test.ts`, "cancellation during the last durable up arm …", hit its explicit 5000 ms timeout (5312 ms). That file and test are not changed here. In the last green `next` run (40125320831) the same case took 3071 ms, and its `down` sibling took 5418 ms under a 30 s budget. It is not on the known-flake list, so it was not rerun; it is reported as a finding. ## Gaps - The carrier table selects reads by their order. The baseline case pins the totals (53 and 4), so a change to the order fails loudly, but the table must then be re-derived (the test header says so). - The binding stand-in is a spied `assertNativeComposeMaterialAuthority`. No real receipt publication happens between two carrier reads, and no CLI fault changes the saved binding mid- proof. - The new carrier file adds 18 real-carrier runs: under 1 s each on hosted CI, but 4-40 s each on a heavily loaded Mac. - The intel config-compiler job is red for an unchanged test's 5 s budget (see evidence). It needs a separate fix or a rerun decision. - The guard-proof budget mutant is killed only on a loaded host. A timeout value has no deterministic mutation proof. - The `once` registry wraps `process.on`, `once`, `addListener`, `prependListener` and `prependOnceListener` for the test process. A function registered for one event both ways at once is read as its last registration kind. Without the root preload (for example `packages/db`), `once` listeners added before the helper first loads come back persistent. - When the preload's restore is removed, the guard's `afterEach` error also stops Bun from running that test's `onTestFinished`. So in this repository the preload restore is the one that matters; `onTestFinished` covers runs without the preload. - Item 7 is a documented decision (report and restore; never call or await a leftover handler), not a behavior change. Nothing beyond the docstring can be tested. Refs HACK-1455 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs HACK-1409. Follow-up to #404 (HACK-1337).
Root cause
runtime-core's own Compose label review (
external_route_or_owner_labelinpackages/runtime-core/src/project/compose/service.rs) rejected only thecaddy,traefik,hackandcom.docker.composeprefixes. A service label in one of Hack's ownership namespaces (io.hack-local.*,io.hack.native-config.*,io.hack.native-file.*,io.hack.storage-witness.*) passed the candidate review. Only the TypeScript classifier from #404 stopped it. If that classifier were bypassed, an authored project could carry ownership evidence into the candidate.Fix
packages/runtime-core/src/project/compose/ownership_labels.rsholds a Rust mirror ofHACK_OWNERSHIP_LABEL_NAMESPACESand anis_hack_ownership_labelpredicate. The predicate matches keys inside a namespace and also the bare root without its trailing dot (for exampleio.hack-local).src/lib/hack-ownership-labels.ts). Rust cannot import it without a build script or a runtime JSON parse, so a parity test keeps the two lists identical instead.tests/hack-ownership-labels.test.tsreads the Rust array (ignoring comment lines) and checks its declared length, its entries and their order against the TypeScript list. The Rust file is also added to the scan's restatement set, so the mirror cannot count as a "use" of a namespace.Network and volume
labelsare already refused by runtime-core asunsupported_field, so service labels are the only path the review has to cover.Part 2 (writers spelling keys inline): left as is, with reasons
io.hack*key sites across 37 TypeScript and Rust files. Converting them would be a broad edit across native-config, route-owner, storage-witness and runtime graph code, which is outside a defence-in-depth follow-up.${NS}owner) would be invisible to that scan. The "namespace is used" test would then fail, or would need to be weakened.The parity scan already catches an undeclared key, and with this PR it also catches drift between the TypeScript list and the Rust mirror.
Tests and mutation evidence
All mutation scripts are in the unit's scratchpad. Each mutation was reverted, and the clean baseline passes.
Rust integration test
hack_ownership_label_namespaces_block_enrollment(tests/project_contract.rs): for one key in each namespace, in map form, list form, and alongside a validcaddyroute, the plan reportsexternal_route_or_owner_label, is not enrollment-compatible, keeps the value out of the plan, andenrollfails withincompatible_compose. Labels that only resemble the namespaces stay reviewable.io.hack.storage-witness.from the Rust constant. The integration test and the unit test both fail.Rust unit tests in
ownership_labels.rscover the namespaces and their bare roots, plus close misses (io.hacker.owner,io.hack-localx.owner,io.hack.storage,io.hack.custom, a key that only contains a namespace, upper case).TypeScript parity test
runtime-core's label review rejects exactly the declared ownership namespaces:Local gates
cargo +1.97.1throughbun run check:local(fmt and clippy-D warnings, all targets): green.bun run test:local(cargo +1.97.1) at this head: 21 test binaries, 1205 passed, 0 failed, 65 ignored.bun run check(including privacy:check): green, with only the existing warnings.bun run --cwd packages/cli typecheck: 0 errors.bun test tests/hack-ownership-labels.test.ts tests/native-compose-backend-features.test.tsgives 18 pass.git log -p origin/next..HEADexport: no leaks.Remaining gaps
testand Candidate core on macOS and ubuntu) at this exact head is the full-suite evidence. Check the PR checks.🤖 Generated with Claude Code