Repository navigation
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
The drop-sudo strategy runs this action again under sudo. It named the
interpreter as a bare "node", but sudo resets PATH to its own secure_path.
That path does not always hold the Node.js install that runs the action. On
a self-hosted runner the step then stops at once with:
sudo: node: command not found
Use process.execPath instead. It is the full path of the Node binary that
runs the action. The repository already does this in runCodexExec, and the
existing tests use it to launch the bundle.
This code is reachable from the "Drop sudo privilege, if appropriate" step
in action.yml. The Linux and macOS branches call drop-sudo without
--root-phase, so they enter this path.
No privilege boundary changes. The runner user already has sudo without a
password at this point, and ensurePasswordlessSudo asserts it.
Test:
- pnpm run check
- node --test test/*.test.mjs -- 146 tests, 143 pass, 0 fail
- test/dropSudoInterpreterPath.test.mjs is new. It fails on the old code.
- Rebuilt dist/main.js.
RainieLLM
force-pushed
the
fix/drop-sudo-absolute-node-path
branch
from
September 19, 2026 01:36
8776ac0 to
2145dc7
Compare
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.
Fixes #180
Summary
On Linux and macOS, the “Drop sudo privilege” step runs
drop-sudoagain undersudo. The command usednodeas the interpreter name. WhensudoreplacesPATHwith its ownsecure_path, that path may not contain the Node.js installation used by the action. On a self-hosted runner, the step can then fail with:The action now passes
process.execPathtosudo. This is the full path of the Node.js binary that already runs the action, so the second process uses the same interpreter. The rest of the command stays unchanged. The runner already has passwordless sudo at this point, so the change does not add or remove any privilege.Tests
The new test runs
drop-sudowith a Node.js path outside thesecure_pathconfigured forsudo. It checks that the privileged phase uses that path and keeps the remaining arguments unchanged.pnpm run checknode --test test/*.test.mjs— 146 tests, 143 pass, 3 skip, 0 faildist/main.js