Skip to content

fix(linker): drop com.apple.quarantine from materialized native binaries - #601

Merged
colinhacks merged 5 commits into
mainfrom
quarantine-strip
Jul 29, 2026
Merged

fix(linker): drop com.apple.quarantine from materialized native binaries#601
colinhacks merged 5 commits into
mainfrom
quarantine-strip

Conversation

@colinhacks

@colinhacks colinhacks commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Closes #575

On macOS, com.apple.quarantine reaches a materialized package two ways. The issue describes the first; the second is why the fix is not where the title says.

  1. Source propagationclonefile(2)/copyfile(3) carry the source's xattrs, so a quarantined CAS blob poisons every package materialized from it.
  2. Creation-time stamping — a process that inherited quarantine flags (a terminal inside a Gatekeeper-enabled app) has the kernel stamp every inode it creates, whatever the source was.

Measured against a verifiably clean store blob under a quarantine-enabled process: the clonefile and copyfile targets still come out quarantined; only a hardlink escapes, having created no new inode. So stripping the store entry would fix only the hardlink strategy, and macOS defaults to ReflinkAuto (clonefile). Opting the process out isn't possible either — the flags are monotonic, and the path-exclusion SPI returns EPERM.

The strip therefore lands on the materialized target, and on cache hits as well as fresh materializations: the global virtual store is on by default and outlives rm -rf node_modules, so an entry written by a pre-fix build would otherwise stay quarantined forever — the exact state the issue reports.

Notes

  • Filter is the union of the executable bit and .node/.dylib/.so. Neither alone suffices: npm ships loadable modules mode 0644, while native CLI binaries (bin/esbuild, biome) are 0755 with no extension. Both halves catch a real file below.
  • Uses removexattr(2) + XATTR_NOFOLLOW, not a spawned xattr(1). Driven off the package index, so it also covers the whole-dir clonefile branch that fills a package without visiting files.
  • Warm-path cost is one removexattr per guarded file: 19 files on a 20-package project, ~0.7ms against a ~229ms warm install.
  • macOS-only. The filter and its test sit outside the macOS module so aube's ubuntu CI leg exercises them; the behavioural xattr tests are macOS-scoped and were verified locally.

Not covered (documented in the module)

An install that relinks nothing (the frozen fast path) strips nothing; removing node_modules invalidates it. Files written into a package after linking — node-gyp output and side-effects-cache restores — are invisible to an index-driven pass. The side-effects cache is a second persistent cache with the same propagation shape and is worth its own issue.

Verification

Differential against v0.2.10 on an isolated store.

Scenario pre-fix fixed
A — quarantined store, clean installer 3/4 quarantined 0/3
B — clean store, quarantine-enabled installer 4/4 0/3
C — poisoned virtual store, rm -rf node_modules, reinstall 3/4 0/3

Per-file in A: @esbuild/darwin-arm64/bin/esbuild and esbuild/bin/esbuild (Mach-O executables) and lightningcss.darwin-arm64.node (Mach-O dylib) all go quarantined → clean. Plus 5 unit tests and clippy --all-targets -D warnings clean on both the macOS and non-macOS cfg paths.

On macOS, `com.apple.quarantine` reaches a materialized package by two
independent routes:

1. Source propagation — `clonefile(2)` and `copyfile(3)` carry the
   source's xattrs, so one quarantined CAS blob poisons every package
   materialized from it, and `rm -rf node_modules` never clears it
   because the store copy is the source.
2. Creation-time stamping — a process that inherited quarantine flags
   (a terminal embedded in a Gatekeeper-enabled app such as a GUI Git
   client) has the kernel stamp every inode it creates, whatever the
   source was.

Gatekeeper then refuses the ad-hoc-signed native binaries npm ships:
loadable modules fail to load, and quarantined executables are killed
on exec.

Strip the attribute from the materialized target, once per package,
after the last step that can create a file in it. The target is the
only seam that closes both routes: stripping the CAS entry would help
only the hardlink strategy, and macOS defaults to clonefile, whose
target is re-stamped by route 2 even from a verifiably clean blob.
Opting the process out is not available — quarantine flags are
monotonic, and the path-exclusion SPI returns EPERM.

Removing quarantine after integrity verification is safe for the same
reason Homebrew does it after checksum verification: the tarball was
verified against the lockfile hash before any file reached the CAS.
Only com.apple.quarantine is removed; other xattrs are preserved.

The file filter is the union of the executable bit and the loadable
module extensions. Neither signal suffices alone: npm ships .node and
.dylib mode 0644, while native CLI binaries (bin/esbuild, biome) are
0755 with no extension. Seeding a store and reinstalling leaves one of
each quarantined. The predicate selects ~1.3% of files in a full store
(~1 per package) and runs only on the cold path.

Uses removexattr(2) with XATTR_NOFOLLOW rather than spawning xattr(1),
and is driven off the package index so it also covers the whole-dir
clonefile branch, which fills a package without visiting files
individually.

macOS-only, compiled out elsewhere. The union filter and its test live
outside the macOS module so aube's ubuntu CI leg exercises them; the
behavioural xattr tests are macOS-scoped.
Copilot AI review requested due to automatic review settings July 29, 2026 04:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nub Ready Ready Preview, Comment Jul 29, 2026 5:27am

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The cold-path fix is sound and well-argued, but it never reaches a global virtual store that is already quarantined — which is the default store location under nub and the exact state a user hit by #575 is in.

Reviewed changes — a macOS-only pass that drops com.apple.quarantine from the native binaries of each freshly materialized package, landing the strip on the materialized target rather than the CAS store entry.

  • Add the quarantine moduleremovexattr(2) with XATTR_NOFOLLOW, driven off the PackageIndex so the whole-directory clonefile branch is covered without a traversal, and only com.apple.quarantine is touched.
  • Filter on the union of the executable bit and .node/.dylib/.sois_gatekeeper_guarded sits outside the macOS module so aube's ubuntu leg compiles and tests the portable half.
  • Wire two per-package seams — the isolated linker in materialize_into and the hoisted linker in materialize_hoisted_node, both immediately after the patch pass.
  • Register a warning codeWARN_AUBE_QUARANTINE_STRIP_FAILED in aube-codes plus docs/error-codes.data.json, emitted at most once per process behind an AtomicBool latch.
  • Add five tests — four macOS-scoped behavioural tests (strip, xattr preservation, missing entry, symlink non-follow) and one portable filter test.

I independently verified the FFI contract against libc 0.2.186's Apple bindings and Apple's man pages: argument counts and order for removexattr/setxattr/listxattr all match, ENOATTR and XATTR_NOFOLLOW are both exposed for Apple targets, and libc is correctly a macOS-only target dep of aube-linker. Cold-path coverage is also complete — every file-creating path (link_file_fresh in both linkers, both try_clonedir_fill branches, build_tree's store tree tier used only as a clone source) is either followed by the strip or produces no observable quarantined file.

⚠️ An already-quarantined global virtual store is never re-stripped

The strip only runs when a package is actually materialized. Every entry point short-circuits on an existing entry before reaching it:

Path Guard
GVS entry materialize.rs:247if pkg_nm_dir.exists() { return Ok(()) }
Per-project .aube entry materialize.rs:410if final_entry.exists() { return Ok(()) }
Isolated link steps link.rs:245, link.rs:1003if !aube_entry.exists()

Under nub the global virtual store is on by default for a plain nub installpm_engine/mod.rs:2330 forces it off only for nub ci / project-local locality, and aube disables it under CI, on a next/nuxt/parcel trigger, with injected deps, or for dlx. That store lives outside the project, so a user in exactly the state #575 describes runs rm -rf node_modules, reinstalls on the fixed binary, and gets the same quarantined .node back from the reused GVS entry.

The PR's verification ran against an isolated store, so it exercised the cold path only — the warm path is untested and, as written, unfixed.

Technical details
# An already-quarantined global virtual store is never re-stripped

## Affected sites
- `vendor/aube/crates/aube-linker/src/materialize.rs:247``ensure_in_virtual_store_with_subdir` returns `Ok(())` on `pkg_nm_dir.exists()`, before `materialize_into` (and therefore before the new strip) is reached.
- `vendor/aube/crates/aube-linker/src/materialize.rs:410` — same shape in `ensure_in_aube_dir` on `final_entry.exists()`.
- `vendor/aube/crates/aube-linker/src/link.rs:245`, `:1003``if !aube_entry.exists()` gates the `materialize_into` call; the `else` branch only bumps `packages_cached`.
- `crates/nub-cli/src/pm_engine/mod.rs:2330-2332` — nub pushes `enableGlobalVirtualStore=false` only when `store_locality == VirtualStoreLocality::ProjectLocal`, so a default `nub install` on macOS materializes into the machine-global store that `rm -rf node_modules` does not clear.

## Required outcome
- A user already affected by #575 must have a documented, discoverable recovery path, or an automatic one. Today neither exists: nothing in the PR, the warning text, or the docs tells them the global virtual store has to be purged.
- If the decision is "automatic", the remediation must not regress the warm-install hot path, and must account for the frozen fast path (`install/frozen.rs`) short-circuiting before the linker runs at all — in which case a cache-hit strip would never execute either.

## Suggested approach
Two directions, in rough order of cost:

1. Document it. State in the fix's user-facing surface that an install predating the fix leaves the global virtual store poisoned, and give the purge command. Cheapest, but leaves the reporter's own repro path broken.
2. Strip on the cache-hit path. The index is already in scope at both early returns, so `strip_from_native_binaries(&pkg_nm_dir, index)` before the `return Ok(())` is a two-line change. Cost is ~1 `removexattr` per package on the warm path (the doc measures the predicate at ~1.3% of files). Note this still does nothing when the frozen fast path skips linking, so it is not a complete answer on its own.

## Open questions for the human
- Is "the cold path is fixed, existing caches are not" an acceptable resolution for #575, or does the issue need the warm path too?
- If remediation is in scope, does it belong in the linker at the cache-hit seams, or as a one-shot store migration (a version-stamped sweep of the GVS) so the warm hot path stays untouched?

ℹ️ Native binaries written after materialization are outside the strip

The strip is a single index-driven pass, and the index is the immutable file list from the tarball. Two later writers put native binaries into an already-linked package directory:

  • Approved dependency buildsnode-gyp rebuild and native postinstalls emit build/Release/*.node that was never in the index, created by a child process that inherited the same quarantine flags route 2 describes.
  • The side-effects cacheside_effects_cache.rs:118 restores a cached build tree with copy_dir(..., HardlinkOrCopy); the hardlink shares the cache entry's inode and its xattrs, and the fs::copy fallback at :430 goes through fcopyfile with COPYFILE_ALL on macOS, which carries xattrs across. sideEffectsCache defaults to true.

Notably finalize.rs:314-325 already documents the underlying shape of this — a build can turn a JS launcher into a native binary, and a cache restore recreates the package directory with the already-native bin. This class is narrower than the prebuilt-binary case the PR fixes and sits outside #575's literal scope, so it reads as a follow-up rather than something to fold in here.

Technical details
# Native binaries written after materialization are outside the strip

## Affected sites
- `vendor/aube/crates/aube/src/commands/install/side_effects_cache.rs:118``restore_if_available` calls `copy_dir(&self.path, package_dir, CopyMode::HardlinkOrCopy)` into an already-linked package dir.
- `vendor/aube/crates/aube/src/commands/install/side_effects_cache.rs:423-433``CopyMode::HardlinkOrCopy` does `std::fs::hard_link` (shares the cache inode + its xattrs) then falls back to `std::fs::copy`, which on macOS is `fcopyfile` with `COPYFILE_ALL` and copies xattrs.
- `vendor/aube/crates/aube/src/commands/install/finalize.rs:305-325` — the dep-lifecycle phase runs after the link phase; its own comment records that a build can replace a JS bin with a native one, and that a `sideEffectsCache` restore recreates the package dir.
- `vendor/aube/crates/aube-linker/src/quarantine.rs` (new, lines 124-139 of the added file) — `strip_from_native_binaries` iterates the `PackageIndex`, so any file not in the tarball's index is invisible to it by construction.

## Required outcome
- A native addon that is compiled during install, or restored from the side-effects cache, should end up unquarantined on macOS for the same reason a prebuilt one does.
- If that is deferred, the deferral should be explicit so the next reader does not assume the linker seam already covers build output.

## Suggested approach
The side-effects-cache half is the cheaper and more valuable one: it is a second persistent cache with exactly the route-1 propagation problem the module doc describes for the CAS, and a poisoned entry re-poisons every future restore. A strip over the restored tree in `restore_if_available` (or a strip when the cache entry is written) closes it. The freshly-compiled half needs a post-lifecycle pass and is a larger change.

## Open questions for the human
- Follow-up issue, or in scope for this PR?

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread vendor/aube/crates/aube-linker/src/materialize.rs Outdated
…zations

The strip only ran when a package was actually materialized. Every
linker entry point short-circuits on an existing virtual-store entry
before reaching it (materialize.rs:247 and :410, link.rs:245 and
:1000), and the global virtual store is on by default and lives
outside the project — so an entry written by a pre-fix build survived
`rm -rf node_modules` and was re-linked, still quarantined, forever.
That is exactly the state #575 reports, so the cold-path fix alone did
not resolve the issue for anyone already affected.

Reproduced: poison the virtual store, `rm -rf node_modules`, reinstall
— 3 of 4 native binaries came back quarantined. With the strip on the
cache-hit path, 0 of 3.

Strip at all four cache-hit seams, where the package index is already
in scope. This holds one invariant — every package the linker touches
has its native binaries stripped, whether or not this run created them
— rather than a one-shot migration marker, which would have to choose
between sweeping the whole store and marking it swept when only the
current project's packages were ever visited.

Cost is one removexattr per Gatekeeper-guarded file on the warm path.
Measured on a 20-package project: 19 guarded files, ~0.7ms against a
~229ms warm install, below run-to-run noise.

Also documents two boundaries this seam does not cover: an install that
relinks nothing (the frozen fast path) strips nothing, and files
written into a package after linking — node-gyp output and
side-effects-cache restores — are invisible to an index-driven pass.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The warm-path strip closes the poisoned-store hole from the previous review; two doc/coverage nits inline.

Reviewed changes — the new commit extends the quarantine strip from fresh materializations to cached-entry hits, which is what makes a reinstall on an already-poisoned global virtual store actually heal.

  • Strip on cache hits as well as fresh materializations — four cached-entry short-circuits now strip before returning: the global-virtual-store hit in ensure_in_virtual_store_with_subdir, the per-project .aube hit in ensure_in_aube_dir, and the two local-dependency hits in link.rs.
  • Add strip_cached_entry — a warm-path wrapper that reconstructs <entry>/node_modules/<name>, the layout materialize_into writes, with a matching no-op stub on non-macOS.
  • Document the warm-path rationale and the pass's boundaries — new module-doc sections on why both seams strip, why that beats a one-shot migration marker, and what the index-driven pass cannot reach.

I verified the path composition at all three strip_cached_entry call sites against materialize_into's base_dir/<subdir>/node_modules/<pkg.name> layout — all three match, scoped names included. I also chased whether mutating the machine-global store on a warm hit could break anything downstream, and it cannot: the store's own freshness checks key on file size, content hash, and executable only, the lthash self-check is over the content-hash graph, the modulesCacheMaxAge sweep keys on directory mtime, and no store entry is ever made read-only or immutable. removexattr is idempotent, so concurrent installs sharing the store are safe too.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread vendor/aube/crates/aube-linker/src/quarantine.rs Outdated
Comment thread vendor/aube/crates/aube-linker/src/quarantine.rs
…ayout

Review follow-ups, all in comments and tests — no behaviour change.

The strip's placement comment claimed it precedes the caller's atomic
rename into the virtual store. That holds for two of the seven callers;
the other five pass `&aube_dir, &aube_dir` and write the entry in place.
The placement is still right, so drop the wrong justification rather
than the comment.

The module doc claimed every package the linker touches gets stripped.
It does not: `EntryState::Fresh` in link.rs and
`ensure_shared_local_in_global_store`, plus the `aube_entry.exists()`
checks the install fast path relies on, all return earlier, and they
deliberately never load the package index — stripping there would cost
a `load_index` per package on the warm path, more than the strip it
enables. State the real reach ("every package this run links"), and
record the consequence: a repeat install with node_modules intact
strips nothing new, so healing a poisoned store goes through the
remove-and-reinstall path.

`strip_cached_entry` hardcodes `<entry>/node_modules/<name>` with
nothing coupling it to materialize_into, and a best-effort strip turns
a drifted path into a silent no-op on the seam that heals poisoned
stores. Add a test pinning the composition, scoped names included;
verified it fails when the join is broken.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No behavioural change in this delta — three doc/coverage-accuracy nits inline.

Reviewed changes — the new commit retracts two claims the prior review flagged as overstated and adds a test pinning the warm-path entry layout.

  • Drop the atomic-rename clause from the cold-path strip comment — the placement rationale in materialize_into no longer asserts an ordering that only two of its seven callers provide.
  • Narrow the warm-path invariant and expand ## Known boundaries — the module doc now scopes reach to "every package this run links" and names the per-package warm short-circuits plus the remove-and-reinstall heal, instead of claiming coverage of every package the linker touches.
  • Add strip_cached_entry_resolves_the_virtual_store_entry_layout — a macOS-scoped test asserting the warm wrapper resolves into the package subdirectory, scoped names included.

I re-read both retracted claims against the code and resolved their threads. I also checked the new boundary rationale against each site it covers: the four link.rs short-circuits do lazy-load the index after their guard, so the cost argument holds there, but ensure_shared_local_in_global_store already takes index as a parameter — flagged inline.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread vendor/aube/crates/aube-linker/src/quarantine.rs Outdated
Comment thread vendor/aube/crates/aube-linker/src/quarantine.rs Outdated
Comment thread vendor/aube/crates/aube-linker/src/materialize.rs
…properly

Three review follow-ups.

`ensure_shared_local_in_global_store` returns early on
`EntryState::Fresh`, and the previous commit justified not stripping
there with the cost argument that covers the `link.rs` short-circuits.
That argument does not apply: this function takes `index` as a
parameter, so the strip is free. Strip the global entry the local
symlink resolves to, and scope the doc's cost rationale to the sites
where it actually holds.

The cache-hit comment in `ensure_in_virtual_store_with_subdir` still
claimed the invariant "every package the linker touches, cold or warm"
that the module doc retracted last commit. Reword to match.

The layout test built `<entry>/node_modules/<name>` itself and asserted
`strip_cached_entry` found it, which pins the function against a copy of
its own join. A change to `materialize_into` — the drift direction the
doc names — left it green while the warm strip became a silent no-op.
Replace it with an end-to-end test that materializes through
`ensure_in_aube_dir`, poisons the result, and re-enters so the
`final_entry.exists()` branch strips it, asserting `packages_cached`
to prove the warm path was the one taken. Verified by drifting
`materialize_into`'s layout: the new test fails, the old one passed.
The unit test stays, retitled to the scoped-name composition it covers.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ The free site from the last review is closed and the warm seam now has a real end-to-end test — one doc-accuracy nit inline.

Reviewed changes — the new commit covers the one warm short-circuit that already had the package index in hand, and replaces the self-referential layout test with an end-to-end one.

  • Strip the shared-local cache hitensure_shared_local_in_global_store now strips the global entry on EntryState::Fresh, covering git and remote-tarball deps in global-virtual-store mode at no extra syscall.
  • Pin the warm seam end to end — a new macOS test materializes a real package carrying a .node, poisons the materialized file, re-enters ensure_in_aube_dir on the cache-hit branch, and asserts it comes back clean; the older unit test is rescoped to scoped-name composition.
  • Correct the two remaining doc claims — the ## Known boundaries bullet no longer groups the now-covered site with the link.rs guards, and the global-virtual-store hit comment drops the retracted invariant wording.

I verified the new call site's path composition: global_entry is virtual_store/<virtual_store_subdir(dep_path)> and ensure_in_virtual_store_with_subdir places the package at <global_entry>/node_modules/<pkg.name>, so strip_cached_entry lands correctly, and EntryState::Fresh requires the local link to resolve, so the directory exists. I also re-swept for warm short-circuits the boundary bullet doesn't name: the two lost-rename-race returns are safe (the winner strips its own entry before the rename, and the loser adopts the winner's), hoisted.rs:85 is a path recompute rather than a linker seam, and one disk-materialize return does not match the documented rationale — flagged inline. The layout the new test pins is additionally pinned portably at tests.rs:2186, so a materializer drift is caught on aube's ubuntu leg too.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread vendor/aube/crates/aube-linker/src/quarantine.rs
…lize hit

The disk-materialize branch places a package twice: the shared-store
copy at the canonical subdir, then an ejected project-local dir. When
the local dir is already correct it returns early — after
`ensure_in_virtual_store_with_subdir` has stripped the store copy, and
with `index` still in hand. So one branch healed the store copy and
left the project-local one quarantined, which is the copy resolution
actually reaches in this mode.

Strip it there too. The remaining `link.rs` warm returns still skip the
strip for the documented reason: they return before `owned_index` is
loaded, so covering them would cost a `load_index` per package.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — the new commit closes the last warm short-circuit the previous review flagged, so the disk-materialize branch no longer heals one of its two placements and leaves the other quarantined.

  • Strip the ejected project-local copy on the disk-materialize hit — the local_is_real_dir return in link_all now calls strip_cached_entry(&local_aube_entry, …), matching the ensure_in_virtual_store_with_subdir call above it that already strips the shared-store copy.

I verified the path composition at the new site: local_aube_entry is aube_dir/<aube_dir_entry_name(dep_path)>, and this branch's materialize_into(&aube_dir, &aube_dir, …, apply_hashes = false) places the package at <local_aube_entry>/node_modules/<pkg.name> — exactly what strip_cached_entry reconstructs. The local_is_real_dir guard proves the entry is a real directory rather than a symlink, and a package subdirectory that isn't there degrades to the tolerated ENOENT. The index is the one already resolved at link.rs:390-406, so this costs no load_index.

I also re-checked the ## Known boundaries bullet the prior thread was about, and it is now accurate as written: every remaining warm return it excuses on load-index cost really does precede the index load — the EntryState::Fresh returns at link.rs:479 and :1074 sit above the owned_index block at :492-508, the non-GVS aube_entry.exists() returns at :573 and :1140 do the same, and the both-placements-correct return at :385-388 is above the branch's own index resolution. The only asymmetry left inside the disk-materialize branch is the cost-motivated one the doc describes.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@colinhacks
colinhacks merged commit f009510 into main Jul 29, 2026
52 checks passed
colinhacks added a commit that referenced this pull request Jul 29, 2026
…f-upgrades (#608)

* fix(install): clear quarantine on side-effects-cache restores and self-upgrades

Two macOS surfaces the linker fix in #601 could not reach, both writing
files that are then loaded or executed.

The side-effects cache restores a cached build tree into an
already-linked package, by hardlink (which shares the cache entry's
inode and its xattrs) or by `fs::copy` (`fcopyfile` with
`COPYFILE_ALL`, which both carries xattrs and mints a new inode a
quarantine-enabled process gets stamped). The payload is locally
compiled node-gyp output, so it is ad-hoc signed at best and Gatekeeper
refuses it outright. The strip runs after the restore rather than when
the entry is saved, because the new-inode stamping happens whatever the
source looked like.

`nub upgrade` swaps in a binary that `codesign` reports as
`adhoc, linker-signed` with no Team ID, and macOS kills a quarantined
ad-hoc-signed executable on exec. `curl` and `tar` run as children of
nub, so an upgrade from a terminal that inherited quarantine flags
installs a nub that cannot start. The same seam covers the self-shim's
provision-then-exec path, whose binary has the same provenance. Both
archives are checksum-verified before use, so clearing the attribute is
the trade Homebrew makes for a verified download.

Adds a walk-driven `strip_quarantine_from_tree` for callers with no
package index, and narrows the shared filter to take an `executable`
bool so the index- and walk-driven paths cannot drift apart on the
union rule.

Deliberately excluded: Node dist provisioning. Official Node is
Developer ID signed with a hardened runtime, so Gatekeeper verifies it
rather than blocking, and the other extractor in that crate handles JS
bundles. No failure was demonstrable there, so it stays unchanged.

* fix(install): cover the build that creates a cache entry, and nub's own addon

Three review follow-ups, two of them surfaces the first commit left open.

The side-effects strip only ran on RESTORE, so the install that actually
compiled the addon left it poisoned and only a later install healed it —
and `sideEffectsCache` defaults on, making the miss path the first
install of any package with a native build. Worse, the entry was saved
from the poisoned tree, seeding every future restore. Strip after the
dep-hook loop and before the save, so the built package and the saved
entry are both clean.

nub's own embedded runtime addon is a third write-then-load surface.
`addons/nub-native.node` is ad-hoc signed (`codesign`: `adhoc,
linker-signed`, no Team ID), inflated in-process from the in-binary
blob, and `dlopen`ed by `transform-core.mjs`, which swallows a load
failure into a null handle it then calls unguarded — so a quarantined
addon is a hard failure for every TypeScript transpile. It runs on the
warm path too: `VERIFIED_ENTRYPOINTS` compares content hashes and an
xattr does not change bytes, so a poisoned dir verifies clean forever
and the self-heal re-extract never fires. One syscall per process,
behind the existing `OnceLock`.

The upgrade warning said `nub upgrade:` on a seam that also serves the
self-shim's provision-then-exec, so it now says `nub:`.

Moves the syscall wrapper into `nub-core::quarantine` rather than adding
a third copy — the upgrade path and the runtime cache share it, with
tests using the crate's own temp-dir convention.

* fix(install): move the build-output strip off the async worker, drop a false claim

The strip after the dep-hook loop ran inline in the async task while
every other filesystem step in that closure — `restore_if_available`,
`break_cas_hardlinks`, `save` — goes through `spawn_blocking`. It walks
the whole package dir, and a node-gyp `build/` tree can hold thousands
of files, so it blocked a runtime worker for the duration. Wrapped, and
best-effort: a join error is ignored rather than failing a build that
otherwise succeeded.

Its comment also claimed the strip left the cache entry saved below
clean. It does not: `save` copies with `CopyMode::Copy`, so `fs::copy`
mints inodes the kernel stamps again whatever the source looked like —
route 2 in aube-linker's own module doc. The restore-side strip is what
covers that and stays load-bearing; the two halves are not redundant.
Nothing changes behaviourally, but as written the claim invited a reader
to delete the wrong one.

The restore-side strip needs no such move — `restore_if_available` is
already called inside a `spawn_blocking`.
@colinhacks

Copy link
Copy Markdown
Contributor Author

Shipped in v0.7.0: https://github.com/nubjs/nub/releases/tag/v0.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CAS store should strip com.apple.quarantine after integrity verification

2 participants