libstore: Enforce Mach-O code-signature validity when bytes enter the store - #15638
libstore: Enforce Mach-O code-signature validity when bytes enter the store#15638ak2k wants to merge 3 commits into
Conversation
ad67af6 to
12dde40
Compare
12dde40 to
d5db6f1
Compare
|
To be clear, this not about placeholders, but scratch paths, right? |
d5db6f1 to
789046f
Compare
|
Yes, thank you for the catch. Amending the PR description, commit message, and |
789046f to
883e433
Compare
|
So I'll admit my current plan is to... just not have self references in the binary. How important is it to have paths like these? (For other-output references, the idea is that you imperatively registered outputs, so you could do this sort of thing manually with Nix helping. E.g. install lib output, get store path back, use it in bin output.) |
I doubt it: I expect this is an instance of the long-standing NixOS/nixpkgs#208951 bug, which is cursed and nondeterministic (fresh rebuilds tend to fix it and @zhaofengli found that it even depends on whether a machine has built the derivation before at all). Since (And TBH encoding Mach-O knowledge deep in Nix guts is a pretty awful layering violation, there are avenues to fix the rewriting issue that don't require that.) |
Makes sense, and seems like the vastly superior architecture move. I wonder if something like this PR offers a short-term bridge for the existing darwin breakage (nixpkgs#507531, nixpkgs#208951) in the interim? |
|
I don't see evidence that this PR fixes those issues. |
|
Thank you, @emilazy. I'll try to address those three: On whether this PR addresses the root cause of NixOS/nixpkgs#507531 / NixOS/nixpkgs#208951. You observed in nix-darwin#693 comment 38: "The What this PR addresses is that specific mechanism — scratch-path → final-path byte substitution inside Mach-O code pages already covered by This also explains the apparent nondeterminism that's made the bug hard to pin down. The trigger is hidden state: whether a sibling output happened to be in the store when the build started. On a fresh machine building an IA multi-output derivation cold, both outputs come out of the same builder run, neither is in the store yet, On the test methodology. I've added a non- On the layering violation. I agree — Mach-O parsing logic in Short of those, both #14999 and this PR fix the corruption from inside the daemon, via different mechanisms. #14999 (@andrewgazelka) shells out to |
I don’t think these correspond to the circumstances in which we see Hydra produce these broken outputs. It’s expected that local rebuilds of the derivations won’t have any issues, since it’s apparently nondeterministic and seemingly partially dependent on persistent system state of some kind. So building a broken‐in‐the‐cache derivation locally and seeing that it doesn’t exhibit the issue does not demonstrate that this PR fixes the issue; that’s already what we observe with no change. I know @zhaofengli had a somewhat reproducible test setup, but it was difficult to arrange. |
|
(For clarity: the sibling outputs thing is an interesting observation that I can imagine might have something to do with what we’re seeing here, but given the amount of times we see random stuff in |
Three darwin-only flake apps targeting aarch64-darwin: - ab-test (default): runs both halves of the A/B in one command. Three unpatched iterations to demonstrate the bug fires deterministically (bit-identical NAR hashes), then one patched iteration to demonstrate the fix. Prints a side-by-side comparison table and a final PASS/FAIL. - unpatched-test: just the bug. Sets up the trigger state, rebuilds via the system nix-daemon, asserts 1/2526 mismatch + codesign FAIL + SIGKILL. - patched-test: just the fix. Same trigger, rebuilds via a private daemon built from NixOS/nix#15638. Asserts 0/2526 mismatches + codesign PASS + fish runs. All three target the exact same store path (/nix/store/gngn7y9mn510mf1hkmr0l69qbpvxfbfh-fish-4.2.1) and the exact same nixpkgs revision (d96b37b). The only variable is the daemon. A recorded transcript of a passing ab-test run is in examples/ab-test-output.txt.
|
Thank you, @emilazy. One correction first, then to your three points. Correction: My statement that "a failed build that left half the outputs behind would reach the same code path" was wrong. Nix's
Local rebuilds with the specific trigger setup (sibling output already in the store, target output absent, substitution disabled) do reliably exhibit the issue on my machine. I just ran three consecutive iterations on Bit-identical between runs. Both produced
Seemingly nondeterministic in production, yes — but the trigger setup above is at least one state configuration under which it reproduces deterministically. As I read the bug I've verified, it's state-dependent rather than nondeterministic: it requires the sibling output to be in the store at the moment the build starts. Within that state, on this machine, it fires bit-identically. Outside that state (truly cold store, no sibling present), it doesn't trigger at all. Whether @zhaofengli's reproducible setup converges with this one or describes a distinct mechanism, I haven't traced yet; if there's a pointer to where it lives, I'd be glad to look and compare.
This isn't the shape of the PR body's A/B. The unpatched local rebuild does exhibit the issue under the trigger setup — the same If you'd like to test on your machine, I've packaged the described trigger setup as a flake at https://github.com/ak2k/nix-507531-repro: On If on your darwin machine the corruption reproduces on the unpatched daemon under this trigger, the A/B in the PR body holds and the patched daemon's fix applies under the same trigger. If it doesn't reproduce, that would point to a second state factor I haven't isolated here, distinct from the trigger above. Either way, the bug is independently visible: One epistemic caveat I should make: the observable here — a single SHA-256 page hash slot mismatch in a |
|
Thanks @emilazy — your parenthetical caught a real narrowness in how I'd been framing this. Two refinements after looking more carefully at the mechanism and at Hydra's source. 1. The trigger isn't specifically "sibling present"; it's "any output being built has
The sibling/self split is the same distinction @Ericson2314 drew from the architectural direction above — "just not have self references in the binary" — reached here from the source side. 2. The trigger state is not rare on Hydra; it's a consequence of per-output substitution. My earlier examples (
None of these require "failed build that left some outputs"; they're routine consequences of Hydra's per-output substitution design plus long-running worker store state. For a given staging-next rebuild, the probability that a multi-output drv has at least one output present when its sibling needs rebuilding seems high enough that I don't think it would need to be a rare coincidence to explain the observed rate. Quick pattern-check against the heavily-mentioned packages in the threads, verified in-situ on this machine via If you or @zhaofengli have a reproduction that doesn't fit this trigger, I'd want to look at it. |
Add gh-restricted, gh-private, gh-admin functions that switch GITHUB_TOKEN by reading tiered PATs from macOS Keychain. Defaults to restricted on shell startup; escalation gated by keychain password. Restricted uses automation.keychain-db (AI accessible). Private and admin use elevate-access.keychain-db (user unlock required). Centralizes token configuration in lib/user-config.nix under github.tokens with per-tier service + keychain attributes for DRY. Includes temporary direnv darwin overlay tracking NixOS/nix#6065: Mach-O signature corruption causes fish test SIGKILL. Remove when NixOS/nix#15638 lands.
* feat: tiered GitHub token context switching Add gh-restricted, gh-private, gh-admin functions that switch GITHUB_TOKEN by reading tiered PATs from macOS Keychain. Defaults to restricted on shell startup; escalation gated by keychain password. Restricted uses automation.keychain-db (AI accessible). Private and admin use elevate-access.keychain-db (user unlock required). Centralizes token configuration in lib/user-config.nix under github.tokens with per-tier service + keychain attributes for DRY. Includes temporary direnv darwin overlay tracking NixOS/nix#6065: Mach-O signature corruption causes fish test SIGKILL. Remove when NixOS/nix#15638 lands. * refactor: improve gh-token-switching error handling and cleanup - gh-token-switching.zsh: call security directly to distinguish missing entries from locked/access-denied/empty failures; route errors to stderr; add REQUIRES contract comment listing expected env vars - home.nix: unset _get_keychain_secret and _KC_AI_DB after init since the switching functions no longer need them at runtime - direnv-darwin-fix.nix: use lib.optionalAttrs instead of if/then/else
Adds five enriched fields to each slice record so downstream queries
can classify binaries by signature shape without re-walking the
binary:
* fat_variant: "thin" | "fat32" | "fat64" (distinguishing the two
fat formats that is_fat alone conflated).
* slots: the full SuperBlob directory as [(slot_type, blob_magic,
blob_length), ...]. Lets downstream code detect any blob class
without adding more convenience fields.
* alternate_cds: non-primary CodeDirectory details for dual-CD
SuperBlobs (pre-2016 SHA-1 + SHA-256 alternates). The existing
best-CD picker silently dropped the non-primary info.
* cms_blob_length: the CSMAGIC_BLOBWRAPPER payload length under
CS_SIGNATURESLOT (0x10000). 0 = absent, 8 = empty placeholder
that Apple's codesign leaves on adhoc binaries, >8 = real
PKCS#7 signature (Developer ID / App Store). Distinguishing
real from empty-wrapper is load-bearing for NixOS/nix#15638's
CMS-skip rule.
* has_entitlements / has_entitlements_der: convenience booleans
for slots 0x5 and 0x7, derivable from `slots` but exposed for
easy aggregation.
Adds a classify_signature() pass over each page_hash_mismatch slice that groups by SuperBlob shape using the scanner's enriched fields (cms_blob_length, has_entitlements, has_entitlements_der, linker_signed): * L linker-signed ad-hoc, no CMS slot * C2 codesign ad-hoc with empty 8 B CMS wrapper * B7-empty ad-hoc + Entitlements + empty CMS wrapper * B7-real embedded PKCS#7 chain (Developer ID / App Store) * unknown pre-enrichment scanner output The classification is the load-bearing input to NixOS/nix#15638's CMS-skip decision: classes L / C2 / B7-empty are fixable in-place, B7-real must be skipped with a warning because the PKCS#7 payload commits to the CodeDirectory's hash. Summary JSON now carries "page_hash_mismatch.by_signature_class", and failing.csv gains six enriched columns (fat_variant, cms_blob_length, has_entitlements, has_entitlements_der, n_alternate_cds, signature_class). Older scans without the enriched fields degrade gracefully to signature_class="unknown" + blank columns.
|
Thank you for the detailed investigation! This explains why just rebuilding fish from source locally didn't fix the problem: since the substituted version was already in the store, I was running into the exact situation of nix using a scratch path for the install prefix, breaking the resulting binary.
It seems to me that if Nix is going to be building with an alternative install prefix, then rewrite that install prefix in the resulting output, it should know how to properly do the rewrite, at least in common cases like a regular macOS binary. I agree that ideally Nix shouldn't be doing rewriting at all, but that's not the situation today. Moving forward, it seems like there are three main options from Nix's side of things:
For both 1 & 2, to prevent show stoppers like NixOS/nixpkgs#507531 from popping up sporadically, it seems prudent to have a test builder that always uses a scratch path to catch issues where Nix or a package aren't rewriting the path properly. |
|
Before talking about potential solutions, let’s first ensure that the diagnosis is correct. The hypothesis is that these code signing issues with executables built by Hydra are a result of hash rewriting – However, Hydra doesn’t use
The first issue with this is that the Rust Hydra queue runner rewrite has not yet been deployed, so the code references aren’t relevant here. It would seem a priori surprising to me for Hydra’s connection to S3 to be so unreliable, and at least if Nix itself is being used for substitution in the current Hydra codebase (I don’t know for sure), it would retry the download several times before giving up and building. We can check cases empirically, though. Let’s take NixOS/nixpkgs#507531 and NixOS/nixpkgs#511265, as recent examples of this issue that got a lot of attention. (It is true that all of
In the case of FFmpeg, it’s unhappy about one of the dylibs; all of them fail Given these store paths, we can look up the Hydra build logs for them:
(I’ve preserved these logs at https://gist.github.com/emilazy/78e5286ec92f1ec6fda6be7b23439ad2 for posterity.) If That does not seem to be the case for So it seems to me like for the The All of these appear verbatim in the build log, except for So something weird with partially‐present outputs is happening here, and this log does seem to be for a rebuild! If we do that exact rebuild, we get broken FFmpeg outputs, and indeed a rewritten I’m a bit surprised, but in this case it seems to be true: partial rebuilds of paths seem to be responsible for the issue with that FFmpeg build. Baking in Mach‐O knowledge into Nix could plausibly solve that case. But that doesn’t explain why it happened, and given that there doesn’t appear to be any similar indicator with the The first time a derivation is built, all its outputs are produced. My understanding was that they are then all unconditionally pushed to the cache, and that once a path is pushed to the cache it is never rewritten, which seems like it would make it hard for any partial rebuild of the outputs to result in any corruption of the cache, unless it leads to downstream issues with codesigning executables on the same builder. However, I am not confident in this assessment, and would need someone with more experience with the Hydra codebase and deployment to confirm. I would tend to think that, rather than chronic issues with substitution failures, it’s more likely to be Hydra doing weird things splitting builds of outputs across multiple machines and those running into race conditions (it’s certainly the case that we get duplicate builds on multiple builders running sometimes), or builds failing but still registering a partial set of outputs (due to lack of disk space, perhaps, which has happened increasingly commonly on our Hydra Darwin builders recently?), but these are just guesses. I think that before we discuss further about adding Mach‐O rewriting code to Nix or self‐references, we should make sure that the issues that have resulted in this PR and the attention it’s getting are going to actually be reliably fixed by it, and that there’s not a more immediate cause. While Certainly something weird is going on, and multiple outputs could very plausibly have something to do with it even in cases like |
|
Okay, so if you grab I’m still not sure about how this would interact with the cache behaviour, though. It can’t just be a local issue that persists to the point where things get uploaded to the cache, because then we’d see the fallback path in the log that gets uploaded. I guess we could observe both of these issues if we assume that store outputs and logs are only ever written to the cache once, but that any of them can fail to upload:
This still seems like a surprising sequence of events to be happening on such a regular basis, but it’s at least consistent. I am still very tempted to say that the disk space issues on Hydra builders could be contributing somehow to the increased prevalence of this issue recently, but I’m not quite sure how that would fit into the story. If this story is correct, then the least invasive way to address the immediate problems we’re seeing on Hydra is probably to not publish |
Wiring extension follow-up to NixOS/nix#15638 staged as a private branch. This commit retargets the verify workflow at it for empirical confirmation; once the run lands green we can decide whether to fold into the live PR.
Whether or not this issue is the cause of all binaries with invalid page hashes in the cache, it clearly is an issue that is reproducible and affects local builds as well.
If there's a quickish way to avoid Hydra poisoning the cache with such broken builds, that's definitely a good first step to fix things for most folks as soon as possible. That said, not all builds can be substituted (whether due to overrides, intentionally disabling substitution, or even using a different store path), and the problem situation can easily occur for local builds (e.g., if the user neglects to set If Hydra is adjusted so as to never build with a scratch path itself, I think it becomes especially important that building nixpkgs with scratch paths is tested an some interval or that |
Workaround missing outputs causing an incompatible rebuild that leads to SIGKILL on darwin. Ref: NixOS/nix#15638 (comment)
Sure, I agree, of course, now that I’ve verified that – before the research I did for my comment, it wasn’t clear to me that we actually had evidence that it was the cause of any binaries with issues in the cache. Disabling fallback paths seems like probably the correct way to go for now, given that they’re not used on sandboxed Linux, and making path rewriting work reliably on macOS in a way that works reasonably with the Nix model would require a non‐trivial amount of design work. |
|
Maybe this is an interesting datapoint: The behavior of macOS crashing such invalid-signature binaries also appears non-deterministic. I have two machines with the same But until today, this binary was running fine on both machines. Today, I updated one machine to macOS 27 Beta and now on that machine the same binary gets killed on launch with the known invalid page error. (It still works on the macOS 26 machine.) |
|
I went through the currently-failing population in
The opencode report is the bun family: opencode ships as a Bun single-executable, built during its nix build, and every cached arm64 Disabling fallback paths on macOS — the direction proposed above — turns the rewriting family into a hard error instead of silent corruption. In today's cache that family is small: one drv with direct fallback-path evidence (ffmpeg), plus five Haskell packages with the same For those 95, two non-exclusive paths. Per-package fixes in nixpkgs — re-sign after Per-drv classification data and scripts are in |
|
Based on the classification above, a concrete proposal. The population splits by who breaks the bytes: the daemon itself (hash rewriting), the package's own build tools (73 drvs), or upstream (22 drvs). The last two are nixpkgs's to fix, by re-signing in fixupPhase or rejecting broken vendored binaries at fetch time, and need nothing from Nix. The first is Nix-caused: the builder has already exited when For that family I'd reshape this PR in two steps: Step 1: detect and refuse. When a hash rewrite has modified a file carrying Step 2: repair, outside the daemon. Move the fix-up out of libstore into a small tool shipped with Nix, which the daemon execs as the build user at the detect point (the diff-hook pattern), failing closed to the step-1 error. The isolation is the same as running it in the builder's context: a malicious Mach-O exploiting the parser gets the build user, not root. Wired as a Does that shape address the libstore and attack-surface concerns? If so I'll reshape along these lines, as two commits here or as a follow-up, whichever is easier to review. |
|
Tbh this seems like a bit of a whack-a-mole trying to fix the symptoms, rather than the root cause. There can be many more issues that can arise from Franken-builds than just what has been observed here. Before introducing additional complexity we should try to attack the problem at its root – the bugs in hydra that lead frankenbuilds. The recent round of those has been caused by a combination of AWS S3 uploads not being properly retried + frankenbuilds that happened when retrying the builds with a partial upload/substitution. And if necessary, new safeguards to explicitly disallow interim paths from being used on the builders (as in a nix option) + bailing out when only some outputs could be substituted. |
|
@ak2k Thanks for the in-depth analysis. Regarding the |
nixpkgs b5aa0fbd forces a fresh aarch64-darwin rebuild of opencode whose build-time `--version` smoke test is SIGKILLed (exit 137) by the macOS kernel. Root cause is the Mach-O page-hash code-signing corruption from the still-unmerged nix daemon fix (NixOS/nix#15638): the daemon's RewritingSink rewrites store-path bytes inside an already linker-signed page, leaving a stale CodeDirectory digest that the kernel rejects with cs_invalid_page. Disable opencode here to unblock the nixpkgs bump until a patched daemon ships.
…atures When registering build outputs, the daemon sometimes rewrites store path hashes inside the output's files: when an output being built was already present in the store at build start, its scratch path is a synthesised fallback path that must be substituted with the final one after the builder exits, and content-addressed outputs need their self-references rewritten once the final hash is known. If the rewritten bytes sit inside a Mach-O binary carrying LC_CODE_SIGNATURE, the substitution invalidates the signature's page hashes, and the macOS kernel kills the binary with SIGKILL at first page-in. The corruption was silent: the build succeeds and the registered output is broken. This is the mechanism behind the recently reported darwin startup failures of fish (nixpkgs issue 507531) and one of the mechanisms behind Nix issue 6065. It has also reached cache.nixos.org — directly evidenced for ffmpeg, whose cached build log references fallback paths, and consistent with the stale page contents of several cached Haskell outputs. Before applying a rewrite, scan the output for regular files that both carry a Mach-O code signature and contain one of the hashes about to be substituted, and fail the build with an error naming the affected files and the already-present store paths whose deletion allows a clean rebuild. CMS-signed files (Developer ID) are called out separately since no re-signing without the original identity can ever fix those. The check also fires under --check, replacing the spurious "may not be deterministic" failure previously reported for signed binaries. The new macho-signature-rewrite-check setting controls the behaviour: refuse (default), warn (diagnose but register the broken output — the previous behaviour plus a diagnostic), ignore (previous behaviour). Detection is purely content-based, so cross-builds of darwin binaries on Linux are covered too; the Mach-O constants are vendored rather than taken from Apple headers for the same reason. Refusing by default is a deliberate behaviour change with a visible blast radius: --check / --rebuild of any signed darwin binary that was previously reported as spuriously non-deterministic now fails with this error instead, and content-addressed cold builds of self-referential signed Mach-O files fail loudly rather than registering silently broken outputs. This adds a Mach-O / code-signature parser to the daemon, over bytes produced by untrusted builders. It is deliberately detection-only: read-only, no writes, every read bounds-checked against the buffer, walk lengths capped (fat_arch and SuperBlob counts, file size — files over the limit are refused as unverifiable, not waved through), fat slices validated by offset and size, and unit-tested against malformed inputs. Repair — which needs substantially more parsing — is out of scope here and belongs outside the daemon's privileged context. Refusing at the rewrite is a finer-grained sibling of bailing out at build start when only some outputs could be substituted: the coarse form is simpler and catches non-signature rewrite damage too, but cannot cover content-addressed cold builds, where nothing is present at build start and the damaging rewrite is unconditional.
Detection alone (the previous commit) leaves the user of a multi-output rebuild with an error and a manual deletion step, and --check of a signed self-referential binary with no way to complete. This adds the repair: a new macho-signature-repair-hook setting, defaulting to the Nix-shipped tool `nix __fixup-macho` (registered like __build-remote), which recomputes exactly the stale signature page hashes in place. The repair is deterministic: only hash slots whose stored value disagrees with the page contents are rewritten, and every other byte — the linker-signed flag, the original page size, the identifier — is preserved, so the same input bytes always yield the same output bytes. That property is what --check and content-addressing require, and what re-signing with codesign(1) cannot provide (it switches page size and clears linker-signed). Both SHA-256 and SHA-1 CodeDirectories are recomputed when present, since the kernel validates every one at page-in. The daemon does not run the repair itself. At the detect point it chowns the affected files to the build user and execs the hook with that user's privileges (the diff-hook pattern) and a minimal environment; the complex parse of untrusted bytes thus happens outside the daemon's own context. A nonzero exit from the hook fails closed to the detection error. Setting macho-signature-repair-hook to an empty string disables repair entirely, restoring plain detect-and-refuse. The hook's exit status says it ran, not that the signatures are now valid: the tool skips what it cannot process (an unsupported CodeDirectory hash type, a malformed header), and a custom hook may do less than it claims. So after the repair the daemon re-invokes the hook with --check (same privileges) and registers the output only if every signature verifies — the hook contract, documented in the setting, is that --check modifies nothing and exits 0 when all signatures are valid, 2 when any is stale or cannot be verified. In check mode the tool counts a signature it cannot verify as a failure for the same reason: exit 0 promises all signatures are valid, and "could not parse" is not "valid". Not repairable, and still refused: CMS/Developer-ID signatures (the certificate chain commits to the directory hash; only the original identity can re-sign), files too large to have been inspected, and the self-reference rewrite of a content-addressed output — there the hashed pages contain the output's own path, which is itself a function of those pages, so no consistent signature value exists (issue 6065). The repair scope is thus exactly the damage the daemon's own rewrite causes; breakage introduced by build tools before registerOutputs is out of scope and stays visible. The hook runs between the rewrite and the metadata canonicalisation, so the NAR hash always covers the repaired bytes and ownership and permissions are restored over the hook's intermediate state. For fixed-output and impure derivations, whose outputs sit in a daemon-private 0700 temporary directory at this point, the directory is chowned along with the files; it is transient and deleted after registration. The functional tests assert the full matrix: default hook repairs (codesign --verify passes, output runs and prints the rewritten path), empty hook refuses, failing hook fails closed, a hook without --check support fails closed, a repairable-looking file whose signature the tool cannot process (unsupported hash type) is refused after the re-check rather than registered broken, --check completes with only the genuine LC_UUID nondeterminism, and a direct dual-oracle exercise of the tool (corrupt a signed byte, codesign rejects, repair, codesign accepts, byte content intact).
…d at rest The build-door check (the previous commits) covers damage the daemon itself causes when registering outputs. But broken signed binaries mostly reach users through substitution: the artifact was already broken where it was built — by the producing daemon, a build tool (bun, electron-builder, install_name_tool), or a broken upstream release — and the substituting machine registers it verbatim. This completes the check at the two remaining doors. Substitution: the new macho-signature-verify setting (default ignore) checks substituted paths in LocalStore::addToStore between restorePath and registerValidPath. A cheap in-daemon scan finds signed Mach-O files; the page-hash verification itself runs in a child process (the repair hook with --check) with the privileges of a build user acquired from the same pool as builds, falling back to the daemon's own uid in single-user mode. Modes: warn names the path and turns mystery SIGKILLs into a download-time diagnostic; refuse fails the substitution (falling back to a local build where possible); repair recomputes the stale hashes before registration, after which the path's NAR hash no longer matches the substituter's advertised one, so its signatures are dropped and it is registered unsigned. Content-addressed paths and CMS-signed files are never repaired and fall back to warn. A path is never accepted on evidence that doesn't exist: a Mach-O file too large to parse is refused under refuse (warned otherwise) rather than passed on the check child's silence, and after a repair the path only counts as repaired if a re-check comes back valid — either way the recorded NAR hash describes the bytes actually on disk, which a partial repair may have changed. Under refuse the path is never modified at all, unlike the build door, whose refuse mode repairs its own rewrite damage first; an operator who wants neither must also empty macho-signature-repair-hook (both settings document this). At rest: the new 'nix store fixup-macho' command repairs broken signatures in paths already registered. It never modifies files in place — with auto-optimise-store, a file may be hard-linked into other store paths, and an in-place write would corrupt every path sharing the inode. Instead the contents are copied, repaired in the copy, verified, swapped in, and the path's NAR hash updated in the database via the new LocalStore::replaceStorePath (which no existing primitive provides: verifyStore only fills in missing hashes and repairPath restores the original contents). The swap window is the same as repairPath's. Content-addressed paths are skipped; a copy whose signatures still fail the post-repair check is discarded, not swapped in. darwin gotcha encoded in replaceStorePath: renaming a read-only directory fails with EACCES on APFS, so owner-write is temporarily restored around the renames; timestamps and permissions are re-canonicalised after the swap. The functional tests manufacture a genuinely broken cached artifact (rewrite under warn with repair disabled, published to a file:// cache) and assert all four modes at the substitution door, the at-rest sweep including dry-run, idempotence, and batch resilience (a CMS path early in the batch does not prevent later repairs), the partial-repair outcome (one supported and one unsupported CodeDirectory: NAR hash updated, signatures dropped, path reported unrepaired, database consistent), and the unverifiable cases — an oversized Mach-O refused at the door and failed by the tool's own --check.
6130aad to
d2bbf5e
Compare
|
The PR has been restructured per the two-step proposal above; the body now describes the new shape. A summary of where it landed, since the thread has been quiet for a few weeks. The design in one paragraph: on macOS, a binary only works if its signature's page hashes match its contents, and this PR makes that a property Nix preserves when bytes enter a local store. It is checked where the NAR hash is already checked, repaired where permissions are already canonicalised, by one Nix-shipped tool that never runs as root. Three commits, one per enforcement point: the build door refuses rewrites that would break a signature (default on), a privilege-dropped hook repairs the daemon's own rewrite damage with the output registered only after the repaired signatures re-verify, and an opt-in substitution-time check covers binaries that were already broken where they were built. One invariant, enforced at each door bytes come through, rather than a patch per producer. On the diagnosis discussion, @emilazy's verification looks right to me: the recent fish/ffmpeg cache breakage is exactly the builds-retried-with-partial-outputs mechanism (ffmpeg's cached log shows the fallback path directly). The proposed Hydra-side fixes, never publishing narinfos until all outputs upload and rebuilding instead of giving up on substitutions, are worth doing and would cut most of the cache-side incidence. What they cannot close: a worker whose GC keeps On the bail-out shape, @xokdvium's "bail out when only some outputs could be substituted" and this PR's build door are the same safeguard at different granularity. The coarse form (at build start) is simpler, needs no Mach-O knowledge in Nix, and also catches non-signature rewrite damage. The fine form (at the rewrite) has a narrower error surface and reaches CA cold builds, which the coarse form cannot, since nothing is present at build start to bail on. The diff makes the trade concrete; I'm happy to implement the coarse form instead of, or alongside, the fine one if that's preferred. On the privilege concern, the repair never runs in the daemon: the hook is exec'd as a build user (the diff-hook pattern), and "repaired" is verified by a second privilege-dropped One disclosure and one wording note. The disclosure: an earlier revision of this branch could repair a content-addressed output and register a Review-wise this is ~2.6k lines, but the shape is three independent doors sharing one parser; each commit stands alone if splitting the review (or the PR) is easier. |
|
So a few things:
The issues in hydra are real, and there certainly things we can do in Nix to make easier to ban rewriting from happening. I don't think we'll be going with this approach though. It's too much of unreviewable code with a dubious design. Also please, can we dial it down a bit with the agents? These walls of text are surreal. |
|
I see that you admin-closed this. For context, I was trying to work toward what I thought was a consensus direction, and maybe I was trying to address too much in one PR. I hope the diagnostic contribution was at least useful. One question, mainly for the users on NixOS/nixpkgs#507531/NixOS/nixpkgs#208951 rather than for this PR: what is the path for people hit by the artifacts already in the cache? Today's scan shows 437 broken slices across 48 packages in the live channels, and as @mroi reported, macOS 27 beta kills binaries that macOS 26 still tolerated. Of those, 94% are ad-hoc-signed and mechanically repairable; the opt-in substitution check and repair in this PR were aimed at exactly that set. On the prevention side, the S3 retry fix (#15855) is merged, but I could not find a PR or tracking issue for the other safeguards mentioned (withholding narinfos until all outputs upload, or the bail-out itself). Is there a tracked plan for the full set of producer-side fixes, and do we know that set is complete? None of it helps anyone substituting what is already in the cache, and the bail-out also cannot reach content-addressed cold builds, which stay silently broken. Without a tracked plan for both halves, prevention and the existing cache, this stays broken for darwin users indefinitely, and the enforcement direction in macOS 27 makes that worse, not better. |
|
For future PRs, or just for posterity: assuming all producer problems are addressed (Hydra, bun, electron, the rest), a solution to this class seems to require covering:
|
Note
This PR was restructured in July 2026 following the discussion below. The original shape — the repair helper linked into libstore and run unconditionally by the daemon — is gone; the repair now runs exclusively in a privilege-dropped child process, wired as a hook setting with a Nix-shipped default payload, alongside a detect-and-refuse mode and an opt-in substitution-time check. Comments before this point reference the earlier shape. The repair mathematics (slot-only recompute, dual-CD, fat containers) is unchanged from what was verified against real cache.nixos.org binaries below.
Originally based on the direction of #14999 by @andrewgazelka; the comparison table below explains where the approaches differ and why.
Fixes the root cause of NixOS/nixpkgs#507531 (fish and the wider multi-output family): the daemon's rewrite damage is now repaired and re-verified before registration, so affected packages build correctly by default.
Addresses #6065 by converting it from silent corruption to a loud, actionable refusal; the full fix (CA hashing modulo signature slots) is out of scope and described under "CA endgame".
Motivation
On darwin,
DerivationBuilderImpl::registerOutputscallsRewritingSinkto substitute scratch-path bytes in build outputs. When an output being built was already in the store at build start, its scratch path is amakeFallbackPath-synthesised stand-in, andRewritingSinkrewrites those scratch-path bytes to the final path after the builder exits. The substitution is byte-level and has no knowledge of Mach-O code signatures, but Apple'sldad-hoc-signs every binary at link time with thelinker-signedflag set inLC_CODE_SIGNATURE. The signature covers the very bytes that were just rewritten, so one or more page hashes in theCodeDirectoryare stale after the rewrite. At first page-in, the macOS kernel SIGKILLs the process withcs_invalid_page.This is the root cause of NixOS/nixpkgs#507531 (fish on
nixpkgs-darwinfails to start) and one mechanism behind #6065 (open since 2022 against CA derivations). It has also reached cache.nixos.org — directly evidenced for ffmpeg, whose cached build log references fallback paths, and consistent with the stale page contents of several cached Haskell outputs. A mechanism classification of the full failing-cache population (101 derivations, 6 families) is on this thread; the hash-rewriting family is the one only Nix can fix.What this PR does now
Following the discussion here — @edolstra's rewrite-hook direction, @emilazy's disable-fallback-paths proposal, and @xokdvium's bail-out framing — the PR makes a valid Mach-O signature a property Nix preserves when bytes enter a local store: checked where the NAR hash is already checked, repaired where permissions are already canonicalised, by one tool that never runs as root. Three commits, one per enforcement point:
1. Build door: detect and refuse (
macho-signature-rewrite-check, defaultrefuse)Before applying a rewrite, the output is scanned for regular files that both carry a Mach-O code signature and contain one of the hashes about to be substituted. Under
refuse(the default) the build fails with an error naming the affected files and the already-present store paths whose deletion allows a clean rebuild;warnandignorerestore the previous behaviour with and without a diagnostic.This is @emilazy's "disable fallback paths on macOS" scoped precisely — the build only fails when the rewrite would actually break a signature — and @xokdvium's build-start bail-out at finer granularity (the coarse form is simpler and catches non-signature rewrite damage too, but cannot cover content-addressed cold builds, where nothing is present at build start and the damaging self-reference rewrite is unconditional; happy to add the coarse form as well if preferred).
--checktoo, replacing the spurious "may not be deterministic" failure previously reported for signed binaries.darwin-prefix on the settings.codeLimitbound) treated as unverifiable rather than waved through, fuzz-tested against malformed input and ASan/UBSan-clean on x86_64-linux. This is the small parser that has to live where the decision is made; everything that writes runs elsewhere:2. Build door: repair via a privilege-dropped hook (
macho-signature-repair-hook)Refusal alone leaves users of a multi-output rebuild with an error and a manual deletion step, and
--checkof a signed self-referential binary unable to complete. The new hook setting defaults to a Nix-shipped tool (nix __fixup-macho, registered like__build-remote) that recomputes exactly the stale page-hash slots in place. This is the rewrite-hook mechanism proposed above, with a default payload: the daemon chowns the affected files to the build user and execs the hook with that user's privileges (the diff-hook pattern) and a minimal environment, so the complex parse of untrusted bytes happens outside the daemon's context — and because the default payload ships with Nix rather than being declared by derivations, it also covers every already-published nixpkgs revision, which no derivation-declared hook can reach.--checkand content-addressing require, and what re-signing withcodesign(1)cannot provide (it switches page size and clearslinker-signed; see the fix(darwin): handle code signatures for CA derivations #14999 comparison below). Both SHA-256 and SHA-1 CodeDirectories are recomputed, since the kernel validates every one at page-in and dual-CD binaries exist in the cache.--check(same privileges) and registers the output only if every signature verifies. The tool's--checkcounts a signature it cannot verify — unsupported hash type, malformed CodeDirectory, oversized file — as a failure, because "could not parse" is not "valid". The--checkcontract (modifies nothing; exit 0 = valid, 2 = stale or unverifiable) is documented in the setting for custom hook authors.--check: all fail closed to the refusal.3. Substitution door + at rest (
macho-signature-verify,nix store fixup-macho)Broken signed binaries mostly reach users through substitution — broken where they were built, by whatever broke them (the producing daemon,
bun --compile, electron-builder,install_name_tool, or a broken upstream artifact; see the classification comment for the family breakdown).macho-signature-verify(defaultignore) checks substituted paths before registration:warnturns mystery SIGKILLs into a named download-time diagnostic;refusefails the substitution (falling back to a local build where possible);repairfixes the path before registration — its NAR hash then no longer matches the substituter's advertised one, so the substituter's signatures are dropped and the path is registered unsigned. The verification child runs with the privileges of a build user acquired from the same pool as builds; pool exhaustion fails the substitution rather than running the child as root.nix store fixup-macho [--dry-run]covers what is already inside a store. It never repairs in place —auto-optimise-storehard-links files across paths, and an in-place write would corrupt every sharing path — but copies, repairs, verifies, and swaps, updating the database through a newLocalStore::replaceStorePath(no existing primitive replaces a valid path's non-null NAR hash).Asymmetry worth knowing: under
refusethe substitution door never modifies a path, while the build door'srefuserepairs the daemon's own rewrite damage before deciding. An operator who wants no Nix-modified signed binaries at all must also emptymacho-signature-repair-hook(documented in both settings).What this deliberately does not do
autoSignDarwinBinariesHook, already used by 54 packages) and Hydra substitution fixes reduce how often the guards fire, and are worth pursuing — but only the consumer-side check makes the guarantee, and the Haskell family shows why: those packages use the signing hook and still break, because the daemon's rewrite runs after fixupPhase. Complementary tracks, not competing ones.Differences from #14999
darwin-codesign.cc)rewriteOutputlambda → both IA and CACodeDirectoryflagslinker-signedcleared bycodesign -s -codesignadds special slots (non-minimalCodeDirectory)codesign -P 4096)/usr/bin/codesignfrom the daemonEvidence
The repair engine is the same mathematics that was verified end-to-end before the restructure; the blocks below carry over and were re-validated against the current branch.
Empirical scope scan across the darwin channels
A scope scanner at ak2k/nix-507531-scope walks each darwin channel's cache and re-verifies every Mach-O slice's page hashes. As of 2026-07-04 it reports 436 failing slices across three channels (
nixpkgs-25.11-darwin55,nixpkgs-darwin121,nixpkgs-unstable260) in 47 distinct packages; live numbers in REPORT.md (auto-updated daily).The mechanism classification on this thread breaks that population into families. The hash-rewriting family is what the build door prevents and repairs; the other families (bun
--compileself-rewriting, electron fuse flips,install_name_tool, upstream-shipped breakage) are producer bugs that the build door does not touch — for those, the substitution door'swarn/refuse/repairis the user-facing mitigation at download time, regardless of which producer broke the binary. Developer-ID-signed files remain unrepairable by anyone but the signer, at every door.The failing list is a lower bound, not a census: packages matching the trigger variants pass the scan only because the worker that built the current cached version happened not to hit
scratchPath != finalPath.Real-binary validation with an independent oracle (re-run at the current head)
10 genuinely broken signed Mach-O files fetched from cache.nixos.org, spanning every failing signature class (thin linker-signed, fat32 single- and multi-arch, dual-CD SHA-1+SHA-256, 56-slot Bun single-file executables, Developer-ID CMS), run through
nix __fixup-machobuilt with ASan on x86_64-linux at this PR's head commit, and validated by a from-scratch Python page-hash verifier that clamps tocodeLimit, handles dual CDs, and shares no code with the engine:--checkagreed with the oracle before (exit 2) and after (exit 0) each repair.(An earlier
rcodesign-based pass false-failed the two Bun binaries — rcodesign reads pastcodeLimitinto Bun's embedded payload, a bug its own output flags; the independent recompute confirms 56 stale slots → 0.)Negative control: unpatched daemon (Nix 2.24.10 — has the bug)
Cold build:
codesign --verifyrc=0, binary runs — a cold build never triggers the bug. Delete one output of the multi-output reproducer, rebuild with the sibling present:codesign --verifyrc=1, running the binary rc=137 — SIGKILL by the macOS kernel,cs_invalid_pagein the system log. This is the corruption every mode of this PR either refuses, repairs, or (underwarn/ignore) at least names.Source-level trace of the trigger (Nix master
a37db9d24)makeFallbackPath(derivation-builder.cc, scratch-path selection inregisterOutputs).outputRewritesis populated per output wherescratchPath != finalPath.rewriteOutputlambda applies the map viaRewritingSinkto each output — both the input-addressed and content-addressed visitors pass through it, which is why the guard sits there.Two trigger variants, both covered: sibling-reference (fish:
bin/fishembeds the-docoutput's path in__TEXT,__const) and self-reference (zsh: three self-referential paths, no sibling refs). On Hydra the trigger state is a routine consequence of per-output substitution plus long-running worker store state — no failed build required.Testing
--checktriggers; CMS refusal; unsupported-hash-type (detection says repairable, the repair skips it, the re-check refuses — the case that distinguishes "the hook ran" from "the signatures verify"); partial repair (one supported + one unsupported CodeDirectory: NAR hash updated to match the bytes on disk, signatures dropped, path reported unrepaired); a custom hook that does not implement--check; oversized files (the fixture generates a sparse file at test time under a test-shrunk size bound — nothing large is checked in); fixed-output/impure temp-dir handling; and the at-rest sweep including dry-run, idempotence, and batch resilience.Known standing costs
macho-signature-verifyadds per-path work at substitution time proportional to the number and size of Mach-O files in the path; the defaultignorecosts nothing. The detection scan memory-maps files (falling back to a bounded read where mapping fails), so it does not hold file contents in the daemon's heap.