Skip to content

Build native modules for Mac - #3979

Open
Dreamsorcerer wants to merge 7 commits into
mainfrom
mac-nix
Open

Build native modules for Mac#3979
Dreamsorcerer wants to merge 7 commits into
mainfrom
mac-nix

Conversation

@Dreamsorcerer

@Dreamsorcerer Dreamsorcerer commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds an Apple-silicon macOS Cachix publishing path, separate platform publication markers, and native-module provisioning for self-hosted macOS tests. The new macOS publisher exposes its Cachix publishing credential to a mutable third-party action reference and must be fixed before merging.

Confidence Score: 3/5

Not safe to merge until the secret-bearing Cachix action is pinned to an immutable commit reference.

The confirmed credential-exposure issue affects the new macOS publishing configuration.

Files Needing Attention: .github/workflows/ci.yml

Security Review

The new macOS Cachix publisher passes CACHIX_AUTH_TOKEN to cachix/cachix-action@v17. Because v17 is mutable, a retargeted action reference could run unreviewed code with access to the publishing credential.

T-Rex T-Rex Logs

What T-Rex did

  • The macOS-related job plumbing was added to HEAD, introducing four mutable action refs (checkout, cache, cachix-action, and cache/save) and token wiring for CACHIX_AUTH_TOKEN when needs-build-macos is true.
  • The harness demonstrated that cachix-build-macos: failure still allowed a Linux matrix job to be scheduled, because the if condition uses !cancelled() and GitHub Actions does not enforce a success gate in that scenario.
  • T-Rex produced a proof for a posted P1 finding, linking to the reviewer comment detailing the finding.
  • T-Rex produced a second proof for the posted P1 finding, adding supporting artifacts as evidence.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Mutable Cachix action receives the publishing token

    • Bug
      • The newly added macOS Cachix publisher job runs cachix/cachix-action@v17 and passes ${{ secrets.CACHIX_AUTH_TOKEN }} through its authToken input. v17 is a mutable major-version tag, so a later retarget of that tag can execute changed third-party action code with the publisher token.
    • Cause
      • The workflow pins the Cachix action to the mutable v17 tag instead of an immutable full commit SHA while wiring a secret into the action.
    • Fix
      • Pin cachix/cachix-action to a verified full 40-character commit SHA (with an audit comment/version label if desired) before passing CACHIX_AUTH_TOKEN. The same new job also introduces mutable refs for actions/checkout@v7, actions/cache@v6, and actions/cache/save@v6; pin these newly added refs to immutable SHAs as well.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "Build native modules for Mac" | Re-trigger Greptile

Comment thread .github/workflows/ci.yml
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...imos/experimental/memory/test_rust_recorder_e2e.py 0.00% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3979      +/-   ##
==========================================
- Coverage   78.21%   78.20%   -0.01%     
==========================================
  Files        1428     1428              
  Lines      135077   135078       +1     
  Branches    11708    11709       +1     
==========================================
- Hits       105647   105644       -3     
- Misses      26110    26112       +2     
- Partials     3320     3322       +2     
Components Coverage Δ
Tests 94.71% <0.00%> (-0.01%) ⬇️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.53% <0.00%> (-0.01%) ⬇️
OS-ubuntu-latest 75.10% <0.00%> (-0.01%) ⬇️
Py-3.10 75.09% <0.00%> (-0.01%) ⬇️
Py-3.11 75.09% <0.00%> (-0.01%) ⬇️
Py-3.12 75.09% <0.00%> (+<0.01%) ⬆️
Py-3.13 75.09% <0.00%> (-0.01%) ⬇️
Py-3.14 75.10% <0.00%> (-0.01%) ⬇️
Py-3.14t 75.09% <0.00%> (-0.01%) ⬇️
SelfHosted-Large 30.76% <0.00%> (-0.01%) ⬇️
SelfHosted-Linux 35.57% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...imos/experimental/memory/test_rust_recorder_e2e.py 90.68% <0.00%> (-0.94%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 8, 2026
Base automatically changed from nix-cache to main September 10, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant