fix(install): make one-shot retry() retry, and drop root grants on user files (#606)

retry() in one-shot-install.sh used `if ! "$@"; then status=$?`, where $? is
the status of the negation -- always 0. A failed command was never retried
and retry() reported success, so a failed `git clone` carried on until a
later check noticed the missing checkout. It now retries (3 attempts) and
returns the command's status. The two apt steps stay non-fatal: warning and
continuing is what they effectively did before, and making them fatal would
stop installs that work today. A clone that keeps failing stops the install,
as it already did, just sooner and with the one-shot's own error message.

Both installers granted the web user NOPASSWD root on display_controller.py,
start_display.sh and stop_display.sh. Those files are owned by the user after
Step 11's chown, so the grant let the web user rewrite them and run them as
root, and nothing ever ran them through sudo. Removed from both installers,
with a test that every project file granted as root is a root-owned
fix_perms helper.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-23 10:34:20 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 342e9164b8
commit e1ce7189f1
5 changed files with 102 additions and 25 deletions
-9
View File
@@ -111,13 +111,6 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *"
fi
# Required: python3, bash
# NOTE: display_controller.py/start_display.sh/stop_display.sh live at the
# project root, not under scripts/install/ (where this script lives) —
# must use PROJECT_ROOT here, not PROJECT_DIR.
echo "$WEB_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_ROOT/display_controller.py"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/start_display.sh"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/stop_display.sh"
echo ""
echo "# Allow web user to remove plugin directories via vetted helper script"
echo "# The helper validates that the target path resolves inside plugin-repos/ or plugins/"
@@ -155,8 +148,6 @@ echo "- Start/stop/restart the ledmatrix service"
echo "- Enable/disable the ledmatrix service"
echo "- Check service status"
echo "- View system logs via journalctl"
echo "- Run display_controller.py directly"
echo "- Execute start_display.sh and stop_display.sh"
echo "- Reboot and shutdown the system"
echo "- Remove plugin directories (for update/uninstall when root-owned files block deletion)"
echo "- Install plugin/base requirements.txt as root (so ledmatrix.service can see them)"
+22 -13
View File
@@ -65,15 +65,14 @@ retry() {
local delay_seconds=5
local status
while true; do
# Run command in a context that disables errexit so we can capture exit code
# This prevents errexit from triggering before status=$? runs
if ! "$@"; then
status=$?
else
status=0
fi
if [ $status -eq 0 ]; then
# The condition of an if doesn't trip errexit, and in the else branch
# $? is the command's own exit status. (This used to be `if ! "$@";
# then status=$?`, where $? is the status of the negation -- always 0 --
# so a failure never retried and was reported as success.)
if "$@"; then
return 0
else
status=$?
fi
if [ $attempt -ge $max_attempts ]; then
print_error "Command failed after $attempt attempts: $*"
@@ -259,22 +258,32 @@ main() {
# Update package list first. first_time_install.sh is told the lists are
# already fresh so it does not repeat this a minute later.
# A refresh that still fails after retries (say one unreachable mirror)
# only warns: that is what this step effectively did before retry()
# could report a failure, and making it fatal would stop installs that
# work today.
if [ "$EUID" -eq 0 ]; then
retry apt-get update -qq
retry apt-get update -qq || print_warning "apt-get update failed; continuing with the existing package lists"
else
retry sudo apt-get update -qq
retry sudo apt-get update -qq || print_warning "apt-get update failed; continuing with the existing package lists"
fi
export LEDMATRIX_APT_UPDATED=1
# Install git and curl (needed for cloning and the script itself)
if ! command -v git >/dev/null 2>&1 || ! command -v curl >/dev/null 2>&1; then
print_warning "git or curl not found, installing..."
# Not fatal here, for the same reason: without git the clone below
# fails and stops the install with its own error.
if [ "$EUID" -eq 0 ]; then
retry apt-get install -y git curl
retry apt-get install -y git curl || true
else
retry sudo apt-get install -y git curl
retry sudo apt-get install -y git curl || true
fi
if command -v git >/dev/null 2>&1 && command -v curl >/dev/null 2>&1; then
print_success "git and curl installed"
else
print_warning "Could not install git and curl"
fi
print_success "git and curl installed"
else
print_success "git and curl already installed"
fi