Skip to content

fix: add error handling when sourcing profile scripts #91

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

Closed
wineee wants to merge 1 commit into from
Closed

fix: add error handling when sourcing profile scripts #91

Show file tree
Hide file tree
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
18 changes: 9 additions & 9 deletions data/scripts/Xsession
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@ case $SHELL in
rm -f $xsess_tmp
;;
*/fish)
[ -f /etc/profile ] && . /etc/profile
[ -f $HOME/.profile ] && . $HOME/.profile
[ -f /etc/profile ] && { . /etc/profile 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }

@sourcery-ai sourcery-ai Bot May 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Quoting file paths would make these conditionals more robust

Since these lines are already being changed, please quote the paths in the -f tests and when sourcing, to avoid issues if any path ever contains whitespace or glob characters. For example:

[ -f "/etc/profile" ] && { . "/etc/profile" 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }
[ -f "$HOME/.profile" ] && { . "$HOME/.profile" 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }

The same adjustment applies to the other similar conditionals in this file.

Suggested implementation:

  */fish)
    [ -f "/etc/profile" ] && { . "/etc/profile" 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }
    [ -f "$HOME/.profile" ] && { . "$HOME/.profile" 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }
    xsess_tmp=`mktemp /tmp/xsess-env-XXXXXX`
    $SHELL --login -c "/bin/sh -c 'export -p' > $xsess_tmp"
    . $xsess_tmp
    rm -f $xsess_tmp
    ;;
  *) # Plain sh, ksh, and anything we do not know.
    [ -f "/etc/profile" ] && { . "/etc/profile" 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }
    [ -f "$HOME/.profile" ] && { . "$HOME/.profile" 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }
    ;;

Search the rest of data/scripts/Xsession for other [ -f ... ] && . ... patterns (e.g. for other shells or configuration files) and apply the same quoting convention: quote the path in the -f test and in the . (source) command, e.g. [ -f "/path" ] && . "/path".

[ -f $HOME/.profile ] && { . $HOME/.profile 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }
Comment on lines +29 to +30
xsess_tmp=`mktemp /tmp/xsess-env-XXXXXX`
$SHELL --login -c "/bin/sh -c 'export -p' > $xsess_tmp"
. $xsess_tmp
rm -f $xsess_tmp
;;
*) # Plain sh, ksh, and anything we do not know.
[ -f /etc/profile ] && . /etc/profile
[ -f $HOME/.profile ] && . $HOME/.profile
[ -f /etc/profile ] && { . /etc/profile 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }
[ -f $HOME/.profile ] && { . $HOME/.profile 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }
Comment on lines +37 to +38
;;
esac

[ -f /etc/xprofile ] && . /etc/xprofile
[ -f /usr/local/etc/xprofile ] && . /usr/local/etc/xprofile
[ -f $HOME/.xprofile ] && . $HOME/.xprofile
[ -f /etc/xprofile ] && { . /etc/xprofile 2>/dev/null || echo "Warning: Failed to source /etc/xprofile" >&2; }
[ -f /usr/local/etc/xprofile ] && { . /usr/local/etc/xprofile 2>/dev/null || echo "Warning: Failed to source /usr/local/etc/xprofile" >&2; }
[ -f $HOME/.xprofile ] && { . $HOME/.xprofile 2>/dev/null || echo "Warning: Failed to source $HOME/.xprofile" >&2; }

# run all system xinitrc shell scripts.
if [ -d /etc/X11/xinit/xinitrc.d ]; then
Expand Down Expand Up @@ -81,10 +81,10 @@ fi
[ -f $HOME/.Xresources ] && xrdb -merge $HOME/.Xresources

if [ -f "$USERXSESSIONRC" ]; then
. "$USERXSESSIONRC"
{ . "$USERXSESSIONRC" 2>/dev/null || echo "Warning: Failed to source $USERXSESSIONRC" >&2; }
fi
if [ -f "$USERXSESSION" ]; then
. "$USERXSESSION"
{ . "$USERXSESSION" 2>/dev/null || echo "Warning: Failed to source $USERXSESSION" >&2; }
Comment on lines +84 to +87

@sourcery-ai sourcery-ai Bot May 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

question (bug_risk): Treating any non-zero exit from user session scripts as a failure may be too strict

Previously, these scripts’ exit codes were simply propagated. With the new || echo wrapper, any non-zero exit (including intentional early return/exit used by users) now emits a warning, which may introduce noisy logs or surprise users who rely on non-zero exits for control flow. Consider limiting warnings to specific failure modes (e.g., file-not-found, permission denied) or clarifying in documentation that user session scripts should exit 0 for expected early termination.

fi

if [ -z "$*" ]; then
Expand Down
8 changes: 4 additions & 4 deletions data/scripts/wayland-session
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,16 @@ case $SHELL in
rm -f $wlsess_tmp
;;
*/fish)
[ -f /etc/profile ] && . /etc/profile
[ -f $HOME/.profile ] && . $HOME/.profile
[ -f /etc/profile ] && { . /etc/profile 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }
[ -f $HOME/.profile ] && { . $HOME/.profile 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }
Comment on lines +33 to +34
xsess_tmp=`mktemp /tmp/xsess-env-XXXXXX`
$SHELL --login -c "/bin/sh -c 'export -p' > $xsess_tmp"
. $xsess_tmp
rm -f $xsess_tmp
;;
*) # Plain sh, ksh, and anything we do not know.
[ -f /etc/profile ] && . /etc/profile
[ -f $HOME/.profile ] && . $HOME/.profile
[ -f /etc/profile ] && { . /etc/profile 2>/dev/null || echo "Warning: Failed to source /etc/profile" >&2; }
[ -f $HOME/.profile ] && { . $HOME/.profile 2>/dev/null || echo "Warning: Failed to source $HOME/.profile" >&2; }
Comment on lines +41 to +42
;;
esac

Expand Down
Loading