Potential compatibility issue: Removing explicit `-s /bin/sh` from su command may break for users with non-POSIX login shells
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- shell
- Domain
- operating-systems
Research direction
Locate the _brew_cmds variable and the su invocation in the current script, then compare them with the change discussed in PR #128. Verify the command path with a non-POSIX login shell and a POSIX shell; done means the privilege-drop command explicitly runs the POSIX shell syntax reliably for both user configurations.
Written by the indexing model from the issue text.
Description
Issue Description
In PR #128, the su command used to drop privileges for running Homebrew commands was simplified by removing the explicit shell specification (-s /bin/sh). This change may break compatibility for users with non-POSIX login shells (e.g., fish, nushell, csh, tcsh).
Technical Details
The _brew_cmds variable contains POSIX shell syntax including:
eval- $() command substitution
&&logical ANDprintfwith format strings
When su - ${NON_ROOT_USER} -c ${_brew_cmds} is used without -s /bin/sh, it invokes the user's login shell. If that shell is non-POSIX (like fish or nushell), the POSIX syntax in _brew_cmds will fail.
Original Code (with explicit shell)
su - ${NON_ROOT_USER} -s /bin/sh -c ${_brew_cmds}
Current Code (without explicit shell)
su - ${NON_ROOT_USER} -c ${_brew_cmds}
Recommended Fix
Restore -s /bin/sh to guarantee POSIX compliance, or use sh -c explicitly.
Context
This was flagged in the code review for PR #128 by the Kilo Code bot. The review comment can be found at: https://github.com/gvatsal60/Linux-All-In-One-Update-Script/pull/128#discussion_r4118722195
Labels: bug, compatibility, shell
PR Signature to include in the PR description:
If you create a pull request or merge request, include the following signature at the end of the PR/MR description:
Built for gvatsal60 by Kilo
- Dominant language
- Shell
- Stars
- 5
- Forks
- 1
- Avg merge
- 25m
- Merged PRs (30d)
- 7
Getting set up
Starts the project's dev container in your browser, under your own GitHub account.
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from gvatsal60/Linux-All-In-One-Update-Script
-
Support for opensuse linuxPossibly taken @gvatsal60 claimed this 1 day ago. Openenhancement good first issue
Difficulty 3/5 1-2 days Newbie friendliness 35/100
gvatsal60/Linux-All-In-One-Update-Script#30 · 1 assignee ·
All issues in gvatsal60/Linux-All-In-One-Update-Script
Similar issues
-
package-update
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
oSoWoSo/vOid_Community_repOsitory#207 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
alunduil/alunduil-chezmoi#815 ·
Maintainers usually reply within 1 day
-
good first issue new package
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
wimpysworld/deb-get#2035 ·
Maintainers usually reply within 1 day
-
automation documentation
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 92/100
termux/termux-packages#32085 ·
Maintainers usually reply within 1 day