Repository navigation
fix(tui): preserve npm binary executable modes - #8330
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe workflows package npm directories into a tarball that preserves executable modes. Stable and nightly publish jobs extract the tarball before publishing, with tests covering permissions, traversal protection, and workflow configuration. Changesnpm artifact mode preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant BuildWorkflow
participant ArtifactStorage
participant PublishWorkflow
participant package_npm_artifact.py
participant npm
BuildWorkflow->>package_npm_artifact.py: create npm-packages.tar.gz
BuildWorkflow->>ArtifactStorage: upload archive
PublishWorkflow->>ArtifactStorage: download archive
PublishWorkflow->>package_npm_artifact.py: extract archive into dist
PublishWorkflow->>npm: publish restored packages
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge; the archive round-trip correctly preserves executable modes and the path-safety guards are sound for a trusted build pipeline. The core fix is well-scoped: tarball creation uses recursive add with mode preservation, extraction applies filter=tar (preserves execute bits, strips setuid/setgid), and verify_executables post-extraction confirms the critical binaries are actually executable before publish proceeds. Path traversal, zip-bomb, and duplicate-entry checks are all present. Workflow wiring is covered by tests. The only observation is a create/extract asymmetry with symlinks, which is not a current concern given the simple package structure. cmux-tui/dist/scripts/package_npm_artifact.py — see the note about symlink asymmetry between create and extract paths. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Build as cmux-tui-build-package
participant Store as GitHub Artifact Store
participant Publish as publish job
participant NPM as npmjs.com
Build->>Build: "cargo build → dist/npm-packages/"
Build->>Build: "package_npm_artifact.py create (tar → npm-packages.tar.gz)"
Build->>Store: "upload npm-packages.tar.gz"
Note over Store: "executable bits preserved in tarball"
Publish->>Store: "download npm-packages.tar.gz → dist/"
Publish->>Publish: "package_npm_artifact.py extract (validate + filter=tar)"
Publish->>Publish: "verify_executables() post-extract"
Publish->>NPM: "npm publish with provenance"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Build as cmux-tui-build-package
participant Store as GitHub Artifact Store
participant Publish as publish job
participant NPM as npmjs.com
Build->>Build: "cargo build → dist/npm-packages/"
Build->>Build: "package_npm_artifact.py create (tar → npm-packages.tar.gz)"
Build->>Store: "upload npm-packages.tar.gz"
Note over Store: "executable bits preserved in tarball"
Publish->>Store: "download npm-packages.tar.gz → dist/"
Publish->>Publish: "package_npm_artifact.py extract (validate + filter=tar)"
Publish->>Publish: "verify_executables() post-extract"
Publish->>NPM: "npm publish with provenance"
Reviews (2): Last reviewed commit: "test(tui): run npm artifact regression i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@tests/test_tui_npm_package_artifact.py`:
- Around line 48-53: Replace the manual os.walk-based cleanup with shutil.rmtree
for the packages directory, adding the shutil import if needed. Preserve
deletion of the entire directory tree and its root while removing the
now-unnecessary traversal and individual unlink/rmdir calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a51f8cc4-557a-4adf-bc97-ef741257ad48
⛔ Files ignored due to path filters (1)
cmux-tui/dist/scripts/package_npm_artifact.pyis excluded by!**/dist/**
📒 Files selected for processing (4)
.github/workflows/cmux-tui-build-package.yml.github/workflows/cmux-tui-nightly.yml.github/workflows/tui-publish-npm.ymltests/test_tui_npm_package_artifact.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13721be80b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
npx cmux@0.9.1fails withEACCESbecause GitHub artifact transfer removes executable bits after package smoke verification.This wraps generated npm package directories in a validated tar archive before artifact upload, restores it in stable and nightly publish jobs, and rejects unsafe archive entries. The same helper verifies all four platform binaries plus the launcher remain executable.
Regression history:
ed2529b64cadds the failing archive round-trip test.97de7f447aadds the fix.Verified:
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Preserves npm binary executable modes across artifact transfer to fix EACCES when running
npx cmux@0.9.1. Builds now tardist/npm-packages, and publish jobs extract it safely beforenpm publish.cmux-tui/dist/scripts/package_npm_artifact.pyto create/extractnpm-packages.tar.gz, enforce safe extraction (no absolute/.. paths, entry/size limits), and verify executables.dist/to restore modes; nightly checks out the immutable source ref before extraction..github/workflows/ci.yml.Written for commit 6ffc42d. Summary will update on new commits.
Summary by CodeRabbit