fix(v1): validate command args before the root check (green ShellCheck CI)#2
Merged
Conversation
…-root) install/uninstall/upgrade called need_root before parsing their arguments, so on a non-root runner (GitHub Actions) `install --bad` etc. exited with the "please run as root" error instead of the expected "unsupported argument" message — failing tests/unit.sh (the ShellCheck workflow) since v1.2.0. Parse and validate arguments first (as doctor already does); for upgrade also validate the URL before need_root so an unsafe --url is reported regardless of privilege. No behavior change when run as root. Verified: tests/unit.sh passes as both root and non-root; bash -n and shellcheck -S warning clean. Co-Authored-By: Claude Opus 4.8 (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
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.
Problem
The
ShellCheckworkflow's Unit tests step (bash tests/unit.sh) has been red since v1.2.0 because it runs as the non-rootrunneruser, while the v1 bash tool'sinstall/uninstall/upgradecommands calledneed_rootbefore parsing their arguments. So on a non-root runner:install --badexited with "please run as root" instead ofinstall: unsupported argument: --baduninstall --badupgrade --url http://… --yeshit the root error before the URL-safety check, soUpgrade URL is unsafe or invalidnever printedThe harness aborts on the first failing assertion, so only
installshowed in the CI log.Fix
Validate arguments before the privilege check — matching how
doctoralready works. Forupgrade, also runvalid_upgrade_urlbeforeneed_rootso an unsafe--urlis reported regardless of privilege. No behavior change when run as root.Verification
tests/unit.shpasses as both root and non-root (reproduced the CI non-root env with a throwaway user).bash -n temp-admin.sh tests/unit.shandshellcheck -S warning temp-admin.sh tests/unit.shclean.Scope: v1 bash maintenance line only; does not touch the v2 Go code.
🤖 Generated with Claude Code