Cerrar el último hueco del instalador: la rama de sudo - #16
Merged
Merged
Conversation
The one thing in scripts/install.sh still taken on faith. The installer job put the binary in the runner's temp, which is writable, so install_bin always took its first branch and the sudo fallback was never executed. "It installs fine" said nothing about the path a member hits when /usr/local/bin needs root, which is the common case on a real machine. Two steps, on both platforms. The default path with no INSTALL_DIR, which is exactly what the one-liner on nan.builders runs. And a root-owned directory, so `install -d` fails as the runner user and the fallback has to fire. That second one asserts the output actually contains "retrying with sudo": without it the step would pass by never taking the branch, which is precisely the failure mode it exists to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by the new macOS job, the first time it ran, which is the whole argument
for having added it.
`version="$(get_latest_version)"` runs under `set -euo pipefail`. When the
GitHub API is rate limited there is no `tag_name` in the answer, so `grep`
exits 1, `pipefail` propagates it, and `set -e` kills the script at that line -
one before `require_version`, which is where the explanation lives. The message
written this morning to replace a confusing 404 had never once been printed.
What a member actually saw on a shared IP:
‣ fetching latest release...
<exit 1>
The pipeline ends in `|| true` now, and so does the call, because whether an
empty answer is fatal is the caller's decision and not the pipeline's.
Two more things while here. The lookup sends GITHUB_TOKEN or GH_TOKEN when one
is in the environment: the unauthenticated API allows 60 requests an hour per
IP, and anyone behind a shared address - an office, a CI runner, a tethered
phone - can be on the wrong side of that through no fault of their own. And
REPO is overridable, so a fork can install its own build and so this failure
path can be exercised against a repo that is not there, which is what CI now
does on both platforms.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 14, 2026
Merged
borjaperfra
added a commit
to helmcode/nan
that referenced
this pull request
Sep 14, 2026
The bash installer's rate-limit message was unreachable: the lookup runs under `set -euo pipefail`, so a grep finding no tag_name killed the script one line before the explanation. Anyone behind a shared IP got `fetching latest release...` and a bare exit. It also sends a token now when one is in the environment. helmcode/nan-cli#16. Co-authored-by: borjaperfra <borjaperfra@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
The one thing in
scripts/install.shstill taken on faith.The installer job put the binary in the runner's temp, which is writable — so
install_binalways took its first branch and thesudofallback was never executed. "It installs fine" said nothing about the path a member hits when/usr/local/binneeds root, which is the common case on a real machine.Two steps added, on both platforms:
INSTALL_DIR: exactly what the one-liner on nan.builders runs, thencommand -v nanandnan --version./opt/nan-sudo-test, soinstall -dfails as the runner user and the sudo branch has to fire. It then asserts the output actually containsretrying with sudo— without that check the step would pass by never taking the branch, which is precisely the failure mode it exists to catch — and runs the binary from where sudo put it.