Skip to content

Replace logname with id #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gvatsal60 merged 1 commit into from
Sep 28, 2026
Merged

Replace logname with id #128

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export DEBIAN_FRONTEND=noninteractive

ADJUSTED_ID="None"

NON_ROOT_USER=$(logname 2>/dev/null || echo "nobody")
NON_ROOT_USER=$(id -un 2>/dev/null || echo "nobody")

###################################################################################################
# Functions
Expand Down Expand Up @@ -236,7 +236,7 @@ update_brew() {

# Safely drop privileges back to the normal user to run brew commands
if [ "${NON_ROOT_USER}" != "nobody" ] && [ "${NON_ROOT_USER}" != "root" ]; then
if ! su - "${NON_ROOT_USER}" -s /bin/sh -c "${_brew_cmds}"; then
if ! su - "${NON_ROOT_USER}" -c "${_brew_cmds}"; then

@kilo-code-bot kilo-code-bot Bot Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Removing explicit -s /bin/sh may break for users with non-POSIX login shells

The su - "${NON_ROOT_USER}" -c "${_brew_cmds}" command now uses the user's login shell instead of explicitly invoking /bin/sh. The _brew_cmds variable contains POSIX shell syntax (eval, $(), &&, printf) which will fail in non-POSIX shells like fish or nushell.

The original su - "${NON_ROOT_USER}" -s /bin/sh -c "${_brew_cmds}" guaranteed POSIX compliance.

Consider restoring -s /bin/sh or using sh -c explicitly for compatibility.

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@gvatsal60 gvatsal60 Sep 28, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kilocode-bot raise a GitHub issue regarding this, but don't fix it here.

@kilo-code-bot kilo-code-bot Bot Sep 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GitHub issue has been created successfully: #129 — #129

This captures the warning about removing -s /bin/sh potentially breaking for users with non-POSIX login shells (like fish or nushell), as noted in the code review. The issue is now tracked separately from this PR.

print_err "Error: Brew commands failed for user ${NON_ROOT_USER}."
return
fi
Expand Down
Loading