fix(build): verify native release binaries are portable#1256
Conversation
ensure-native-binary.ts accepted any staged package binary whose version matched, including binaries dynamically linked against libraries that only exist on the build machine. A Nix-built macOS binary linking /nix/store/.../libiconv.2.dylib shipped this way and crashed for users without Nix with a missing dynamic library error. Add an isPortableBinary() gate before accepting the staged binary: Linux binaries must be fully static (ldd, mirroring the release CI check), macOS binaries may only link system dylibs under /usr/lib or /System/Library (otool -L), and Windows is not checked because MSVC builds link only system DLLs by default. A version-matched but non-portable binary now fails loudly instead of being silently accepted or silently replaced by a local cargo build.
The darwin package relies on install_name_tool to rewrite the Nix store libiconv reference to /usr/lib/libiconv.2.dylib (#1249), but install_name_tool -change is a silent no-op when the old path does not match, and nothing verified the final binary. A regression (for example a new Nix-linked dylib) would ship unnoticed and crash for users without /nix/store. Assert in postInstall that every linked dylib lives under /usr/lib or /System/Library, printing the offending entries otherwise. The assertion runs on every nix build and via nix flake check, which already builds the ccusage package as a check. Verified both ways on darwin: the current build passes, and removing the install_name_tool line makes the build fail with the offending /nix/store path in the log.
ccusage-static targets musl specifically so the published Linux binaries run without any dynamic libraries, but the derivation never verified the result; only a separate CI action step did. Check in postInstall that the binary has no PT_INTERP program header, which would mean it requests a dynamic loader and fails on end-user machines. READELF comes from the cross bintools wrapper, with a plain readelf fallback. This complements the existing ldd verification in build-linux-native-package, moving the primary gate into nix build itself so local builds and any future workflows are covered too.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
ccusage-guide | fb1127f | Jun 10 2026, 09:36 PM |
The darwin-x64 package is built with plain cargo on a GitHub macOS runner, so the portability assertion added to the Nix derivation does not cover it and no other gate inspects it before staging. Fail the build action if otool -L reports any dylib outside /usr/lib or /System/Library, mirroring the assertion in package.nix. The check logic was exercised locally on macOS: a /usr/lib-linked binary passes and a Nix-store-linked binary fails with the offending path printed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/ccusage/scripts/ensure-native-binary.ts (1)
56-61: ⚡ Quick winConsider using
as constfor the literal array.The
systemDylibPrefixesarray is never modified and represents a fixed set of path prefixes. Applyingas constmakes it a readonly tuple, preventing accidental mutation and improving type inference.As per coding guidelines, TypeScript files should use
as const satisfiesfor typed literals.♻️ Suggested refactor
-const systemDylibPrefixes = ['/usr/lib/', '/System/Library/']; +const systemDylibPrefixes = ['/usr/lib/', '/System/Library/'] as const;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ccusage/scripts/ensure-native-binary.ts` around lines 56 - 61, The array systemDylibPrefixes is a fixed literal and should be made an immutable typed literal; change its declaration to use "as const" together with a "satisfies" clause (e.g., make it a readonly tuple that satisfies the expected string-array type) so it becomes readonly and improves type inference for references to systemDylibPrefixes; update any consumers that expect a mutable string[] to accept a readonly string[] or convert via Array.from(...) where necessary.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/ccusage/scripts/ensure-native-binary.ts`:
- Around line 56-61: The array systemDylibPrefixes is a fixed literal and should
be made an immutable typed literal; change its declaration to use "as const"
together with a "satisfies" clause (e.g., make it a readonly tuple that
satisfies the expected string-array type) so it becomes readonly and improves
type inference for references to systemDylibPrefixes; update any consumers that
expect a mutable string[] to accept a readonly string[] or convert via
Array.from(...) where necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba410b67-fdc9-489d-aa15-f3547d9b9ca2
📒 Files selected for processing (3)
apps/ccusage/scripts/ensure-native-binary.tsnix/static-package.nixpackage.nix
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.nix">
<violation number="1" location="package.nix:54">
P1: The dylib portability check is fail-open: if the inspection pipeline errors, the `if` condition just evaluates false and the build continues without enforcing portability.</violation>
</file>
<file name="nix/static-package.nix">
<violation number="1" location="nix/static-package.nix:62">
P1: The PT_INTERP verification is fail-open: an error running `readelf` can skip the check and allow the build to pass without validating static linkage.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| # End-user machines have no /nix/store, so any dylib outside the macOS | ||
| # system paths would crash the published binary with a missing dynamic | ||
| # library error. grep prints the offending entries when it matches. | ||
| if otool -L $out/bin/ccusage | tail -n +2 | awk '{print $1}' | grep -Ev '^(/usr/lib/|/System/Library/)'; then |
There was a problem hiding this comment.
P1: The dylib portability check is fail-open: if the inspection pipeline errors, the if condition just evaluates false and the build continues without enforcing portability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.nix, line 54:
<comment>The dylib portability check is fail-open: if the inspection pipeline errors, the `if` condition just evaluates false and the build continues without enforcing portability.</comment>
<file context>
@@ -48,6 +48,13 @@ craneLib.buildPackage (
+ # End-user machines have no /nix/store, so any dylib outside the macOS
+ # system paths would crash the published binary with a missing dynamic
+ # library error. grep prints the offending entries when it matches.
+ if otool -L $out/bin/ccusage | tail -n +2 | awk '{print $1}' | grep -Ev '^(/usr/lib/|/System/Library/)'; then
+ echo "error: ccusage links dylibs that do not exist on end-user machines" >&2
+ exit 1
</file context>
| # so it would not run on end-user machines without the build-time | ||
| # loader path. READELF is exported by the cross bintools wrapper. | ||
| postInstall = '' | ||
| if "''${READELF:-readelf}" -l $out/bin/ccusage | grep -q INTERP; then |
There was a problem hiding this comment.
P1: The PT_INTERP verification is fail-open: an error running readelf can skip the check and allow the build to pass without validating static linkage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At nix/static-package.nix, line 62:
<comment>The PT_INTERP verification is fail-open: an error running `readelf` can skip the check and allow the build to pass without validating static linkage.</comment>
<file context>
@@ -55,6 +55,15 @@ in
+ # so it would not run on end-user machines without the build-time
+ # loader path. READELF is exported by the cross bintools wrapper.
+ postInstall = ''
+ if "''${READELF:-readelf}" -l $out/bin/ccusage | grep -q INTERP; then
+ echo "error: ccusage-static is not statically linked" >&2
+ exit 1
</file context>
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runner startupExecution setup measures any pre-benchmark package materialization used by the execution benchmark. Bunx temp cache measures one
Cached bunx execution performanceRuns the same large fixture through Fixtures: Claude
Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact sizeLower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
* fix(nix): drop unused libiconv from the Darwin build ccusage links no iconv symbols (verified: `nm -u` shows none on both arm64 and x64, no iconv crate in Cargo.lock), but libiconv was listed in the Darwin buildInputs, so the linker recorded /usr/lib/libiconv.2.dylib as an unused load-time dependency. When the Darwin package was built with Nix that path was the /nix/store one, which dyld can't resolve on non-Nix Macs — the crash behind #1251 (worked around in #1256 by rewriting the install name). Remove libiconv from buildInputs so the binary links only system dylibs and the install-name rewrite is no longer needed. The portability gate (otool, reject non-system dylibs) stays to catch any future stray /nix dependency. * fix(nix): strip the unused libiconv dependency with dead_strip_dylibs Removing libiconv from buildInputs was not enough: the nixpkgs Darwin stdenv injects -liconv regardless, so the arm64 binary still recorded the unused /nix/store libiconv dylib (the otool gate caught it). ccusage references no iconv symbols, so add -Wl,-dead_strip_dylibs to the Darwin link, which drops dylib load commands with no used symbols. The arm64 binary now links only /usr/lib/libSystem.B.dylib (verified), so the install-name rewrite stays gone. (Linux ships a fully static musl binary with no dynamic deps, so no equivalent is needed there.) * fix(nix): fall back to libiconv install_name_tool rewrite when dead_strip_dylibs doesn't strip it --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
1 participant
Summary
v20.0.10 shipped a macOS binary that was dynamically linked against
/nix/store/.../libiconv.2.dylib, which crashed for every user without Nix with a missing dynamic library error. #1249 fixed the linkage withinstall_name_tool, but nothing guarded against the same class of regression. This PR adds verification at three layers so a non-portable binary can no longer ship silently.What Changed
package.nix: the darwin build now fails inpostInstallif any linked dylib lives outside/usr/libor/System/Library, printing the offending paths.install_name_tool -changeis a silent no-op when the old path does not match, so the rewrite alone is not a guarantee. Runs on everynix build .#ccusageand vianix flake check(which builds the package as a check).nix/static-package.nix:ccusage-staticnow fails inpostInstallif the binary has aPT_INTERPprogram header, i.e. it is not actually static. This moves the primary gate into the build itself, complementing the existinglddstep in the CI action.apps/ccusage/scripts/ensure-native-binary.ts: staged package binaries are only accepted if they are portable (Linux: fully static vialdd; macOS: only system dylibs viaotool -L; Windows: skipped). A version-matched but non-portable binary throws instead of being silently accepted.Testing
nix build .#ccusagepasses andotool -Lshows only/usr/libdylibs.install_name_toolline makes the build fail with the offending/nix/storepath in the log.ensure-native-binary.tsverified in both directions: accepts the fixed staged binary (exit 0) and throws on a version-matched Nix-linked binary.nix eval .#packages.aarch64-linux.ccusage-static.drvPathconfirms the derivation evaluates. The release CI build exercises it, with the existinglddaction step as a second net.just fmtandjust typecheckpass.Related Issues
Follow-up hardening to #1249.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Add build, CI, and packaging checks to ensure native release binaries are portable. macOS links only system dylibs, Linux is fully static, and non‑portable staged binaries are rejected to prevent regressions like the Nix‑linked
libiconvcrash.package.nix(darwin): fail if any dylib outside/usr/libor/System/Library; print offenders.nix/static-package.nix(linux-static): fail ifPT_INTERPexists (not fully static).apps/ccusage/scripts/ensure-native-binary.ts: only accept portable staged binaries (Linux static; macOS system dylibs)..github/actions/build-macos-cargo-native-package: fail macOS x64 cargo build if non‑system dylibs; show offenders.Written for commit fb1127f. Summary will update on new commits.
Summary by CodeRabbit