mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-10 17:16:36 +00:00
Compare commits
6
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
87509befdb | ||
|
|
1fe7237799 | ||
|
|
ddf5f085a5 | ||
|
|
5baf983fe0 | ||
|
|
82f3a3a3e4 | ||
|
|
c1ce0b7b04 |
@@ -137,6 +137,13 @@ floor on the release that ships them):
|
||||
when the count is only known to the display service.
|
||||
- The Logs tab has a **Plugin errors** panel: per-plugin counts, repeating
|
||||
errors and a Clear button.
|
||||
- Credential redaction in exception text (`src/redaction.py`) takes time
|
||||
proportional to the text, not its square. Two patterns were quadratic: URL
|
||||
`user:password@`, on a long unbroken run of letters or digits (a hex digest,
|
||||
an ID), and `Authorization:` followed by a long run of whitespace. Either
|
||||
used to stall every thread of the display service for up to seconds each
|
||||
time the snapshot was published: about 0.5s for 20k characters of hex, 8s
|
||||
for 20k spaces. What gets redacted is unchanged.
|
||||
|
||||
### Removed
|
||||
|
||||
|
||||
@@ -157,5 +157,6 @@ For more, see the [Plugin Dependency Troubleshooting Guide](PLUGIN_DEPENDENCY_TR
|
||||
- Store installs: `src/plugin_system/store_manager.py` (`_install_dependencies`)
|
||||
- Root install helper: `src/common/permission_utils.py` (`install_requirements_file`), `scripts/fix_perms/safe_pip_install.sh`
|
||||
- Load-time installs: `src/plugin_system/plugin_loader.py` (`install_dependencies`)
|
||||
- Sudo rules: `scripts/install/configure_web_sudo.sh`
|
||||
- Sudo rules: `scripts/install/lib_sudoers.sh` (written by `first_time_install.sh`
|
||||
and `scripts/install/configure_web_sudo.sh`)
|
||||
- Manual installer: `scripts/install_plugin_dependencies.sh`
|
||||
|
||||
@@ -303,6 +303,76 @@ journalctl -u ledmatrix --since "-5min" --no-pager | grep -iE "px/s|px/frame"
|
||||
If a plugin logs its scroll config **twice** with different modes, the second
|
||||
line is what is running.
|
||||
|
||||
---
|
||||
|
||||
## A tear across the middle on fast scrolls
|
||||
|
||||
**Symptom:** while text scrolls, the top and bottom halves of the panel look
|
||||
shifted sideways against each other along a horizontal line at mid-height, and
|
||||
the shift grows with scroll speed. It shows most in Vegas mode at high speed.
|
||||
|
||||
**It is the panel's scan, not the software.** The measured panel, like most
|
||||
64-row panels, is multiplexed 1:32 (some panels of the same size scan
|
||||
differently, so check yours): it lights two rows at a time, one from each half
|
||||
(row 0 with row 32, row 1 with row 33, …), stepping down both halves together
|
||||
once per refresh. So row 31,
|
||||
the last row of the top half, lights almost a whole refresh period after row 32
|
||||
right below it. Your eye follows moving text, and moving content that lights at
|
||||
different times lands in different places, so the two rows meet with an offset
|
||||
of roughly
|
||||
|
||||
```
|
||||
offset ≈ scroll speed × refresh period
|
||||
```
|
||||
|
||||
Each frame already reaches the panel whole (`SwapOnVSync` swaps complete frames
|
||||
between refreshes), so there is nothing to fix in the render path; the shift is
|
||||
created inside a single refresh. Other panel heights show it too, at the point
|
||||
where their two scan halves meet.
|
||||
|
||||
On the 2×128×64 chain above, which refreshes at about 130 Hz flat out
|
||||
(7.7 ms per pass):
|
||||
|
||||
| scroll speed | offset at the midline |
|
||||
|---|---|
|
||||
| 50 px/s (Vegas default) | ~0.4 px |
|
||||
| 100 px/s | ~0.8 px |
|
||||
| 150 px/s | ~1.2 px, plainly visible |
|
||||
|
||||
### What changes it
|
||||
|
||||
Only a shorter scan period (a faster refresh) or a slower scroll. Measure what
|
||||
the panel actually achieves first. The library prints the rate with a carriage
|
||||
return and no newline, so read it from the raw journal:
|
||||
|
||||
```bash
|
||||
# set display.hardware.show_refresh_rate to true (web UI, Display tab), restart, then:
|
||||
journalctl -u ledmatrix --since "-1min" --no-pager -o cat --all | grep -a -oE "[0-9.]+Hz" | tail -5
|
||||
```
|
||||
|
||||
Turn it off again afterwards. Measured on that panel (Pi 4, single chain),
|
||||
changing one setting at a time from `pwm_bits: 7`, `gpio_slowdown: 3`:
|
||||
|
||||
| change | refresh, uncapped | notes |
|
||||
|---|---|---|
|
||||
| none | ~130 Hz | the ceiling for this wiring |
|
||||
| `pwm_bits: 6` | ~138 Hz | barely faster, and half the colour depth |
|
||||
| `gpio_slowdown: 2` | ~130 Hz | no faster, **and visible glitching**; keep 3 |
|
||||
| `limit_refresh_rate_hz: 0` | ~130 Hz | Vegas dropped from 100 to 72–95 fps as the refresh thread took more CPU |
|
||||
|
||||
None of these helps much, because the time goes into shifting each row's pixels
|
||||
out: a 2×128 chain pushes 256 pixels per row down one output. What does help is
|
||||
**fewer pixels per output**. On a bonnet with more than one output (the
|
||||
`regular` and `classic` mappings have 3; `adafruit-hat` has 1), put each panel
|
||||
on its own output and set `parallel` to the number of outputs used and
|
||||
`chain_length` to the panels per output, for example `parallel: 2`,
|
||||
`chain_length: 1` for two panels. Each refresh then shifts half the data, which
|
||||
should roughly double the refresh rate and halve the offset. That is a cable
|
||||
change, so measure again afterwards.
|
||||
|
||||
Short of rewiring, keep fast scrolls moderate: at the default 50 px/s the
|
||||
offset is under half a pixel.
|
||||
|
||||
## Rebuilding the binding
|
||||
|
||||
```bash
|
||||
|
||||
+58
-113
@@ -502,6 +502,41 @@ print_rgbmatrix_build_failure() {
|
||||
fi
|
||||
}
|
||||
|
||||
# Set WEB_SERVICE_USER to the account ledmatrix-web.service runs as, or "root"
|
||||
# when it cannot tell. Steps 3.1 and 11 choose plugin-directory ownership from
|
||||
# it. The logic was pasted three times, identically, and is kept verbatim here.
|
||||
# Note: install_web_service.sh and install_service.sh no longer contain the
|
||||
# "User=root" / "User=${ACTUAL_USER}" strings grepped for below (the units come
|
||||
# from systemd/*.service templates with User=__USER__), so until Step 8 has
|
||||
# installed the unit this yields "root".
|
||||
detect_web_service_user() {
|
||||
WEB_SERVICE_USER="root"
|
||||
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
|
||||
# Check actual installed service file (most accurate)
|
||||
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
|
||||
# Check install_web_service.sh (used by first_time_install.sh)
|
||||
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="root"
|
||||
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
|
||||
# Check template file (may have placeholder)
|
||||
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
|
||||
# If template has placeholder, check install script
|
||||
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
|
||||
# Check install_service.sh to see what user it uses
|
||||
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
# Web service will be installed by install_service.sh as ACTUAL_USER
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
}
|
||||
|
||||
echo ""
|
||||
echo "This script will perform the following steps:"
|
||||
echo "1. Check prerequisites (network, disk, memory) and install system dependencies"
|
||||
@@ -699,32 +734,7 @@ else
|
||||
fi
|
||||
|
||||
# Determine ownership based on web service user
|
||||
# Check if web service file exists and what user it runs as
|
||||
WEB_SERVICE_USER="root"
|
||||
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
|
||||
# Check actual installed service file (most accurate)
|
||||
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
|
||||
# Check install_web_service.sh (used by first_time_install.sh)
|
||||
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="root"
|
||||
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
|
||||
# Check template file (may have placeholder)
|
||||
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
|
||||
# If template has placeholder, check install script
|
||||
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
|
||||
# Check install_service.sh to see what user it uses
|
||||
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
# Web service will be installed by install_service.sh as ACTUAL_USER
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
detect_web_service_user
|
||||
|
||||
# If web service runs as ACTUAL_USER (not root), set ownership to ACTUAL_USER
|
||||
# so the web service can change permissions. Root service can still access via group (775).
|
||||
@@ -758,32 +768,7 @@ if [ ! -d "$PLUGIN_REPOS_DIR" ]; then
|
||||
fi
|
||||
|
||||
# Determine ownership based on web service user
|
||||
# Check if web service file exists and what user it runs as
|
||||
WEB_SERVICE_USER="root"
|
||||
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
|
||||
# Check actual installed service file (most accurate)
|
||||
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
|
||||
# Check install_web_service.sh (used by first_time_install.sh)
|
||||
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="root"
|
||||
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
|
||||
# Check template file (may have placeholder)
|
||||
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
|
||||
# If template has placeholder, check install script
|
||||
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
|
||||
# Check install_service.sh to see what user it uses
|
||||
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
# Web service will be installed by install_service.sh as ACTUAL_USER
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
detect_web_service_user
|
||||
|
||||
# If web service runs as ACTUAL_USER (not root), set ownership to ACTUAL_USER
|
||||
# so the web service can change permissions. Root service can still access via group (775).
|
||||
@@ -1516,51 +1501,31 @@ POWEROFF_PATH=$(which poweroff)
|
||||
BASH_PATH=$(which bash)
|
||||
JOURNALCTL_PATH=$(which journalctl 2>/dev/null || true)
|
||||
|
||||
# Create sudoers content
|
||||
cat > "$SUDOERS_TMP" << EOF
|
||||
# LED Matrix Web Interface passwordless sudo configuration
|
||||
# This allows the web interface user to run specific commands without a password
|
||||
|
||||
# Allow $ACTUAL_USER to run specific commands without a password for the LED Matrix web interface
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $REBOOT_PATH
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $POWEROFF_PATH
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_plugin_rm.sh *
|
||||
# Install a requirements.txt as root via vetted helper, so packages are visible
|
||||
# to root-run ledmatrix.service (not just the web interface's own user).
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_pip_install.sh *
|
||||
EOF
|
||||
if [ -n "$JOURNALCTL_PATH" ]; then
|
||||
cat >> "$SUDOERS_TMP" << EOF
|
||||
# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
|
||||
# when its output is a terminal. From that pager (less) a "!sh" is a root
|
||||
# shell -- the standard journalctl escalation. The web interface always passes
|
||||
# --no-pager, so nothing here needs it, but the rule cannot require a flag that
|
||||
# sits in the middle of the command line. NOEXEC stops the command executing
|
||||
# another program at all, which closes the hole without depending on wildcard
|
||||
# matching subtleties.
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *
|
||||
$ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
|
||||
EOF
|
||||
# The rules themselves live in scripts/install/lib_sudoers.sh, shared with
|
||||
# scripts/install/configure_web_sudo.sh so the two cannot drift apart again.
|
||||
# If it is missing (a damaged checkout), keep whatever is already installed
|
||||
# rather than failing the whole install; the gate below skips the install.
|
||||
SUDOERS_VALID=1
|
||||
SUDOERS_LIB="$PROJECT_ROOT_DIR/scripts/install/lib_sudoers.sh"
|
||||
if [ -f "$SUDOERS_LIB" ]; then
|
||||
# shellcheck source=scripts/install/lib_sudoers.sh
|
||||
. "$SUDOERS_LIB"
|
||||
web_sudoers_rules "$ACTUAL_USER" "$PROJECT_ROOT_DIR" "$SYSTEMCTL_PATH" "$BASH_PATH" \
|
||||
"$REBOOT_PATH" "$POWEROFF_PATH" "$JOURNALCTL_PATH" > "$SUDOERS_TMP"
|
||||
else
|
||||
SUDOERS_VALID=0
|
||||
echo "⚠ $SUDOERS_LIB not found; cannot generate the sudoers rules." >&2
|
||||
echo "⚠ Leaving $SUDOERS_FILE unchanged. The web interface cannot control" >&2
|
||||
echo " the display service until this is fixed." >&2
|
||||
fi
|
||||
|
||||
# Never install rules we have not parsed. A malformed drop-in in
|
||||
# /etc/sudoers.d makes sudo refuse every command for every user, which on a
|
||||
# headless Pi leaves no way in at all. If the rules do not parse, say so and
|
||||
# keep whatever is already installed.
|
||||
SUDOERS_VALID=1
|
||||
if command -v visudo >/dev/null 2>&1; then
|
||||
if [ "$SUDOERS_VALID" = "0" ]; then
|
||||
: # nothing was generated; already reported above
|
||||
elif command -v visudo >/dev/null 2>&1; then
|
||||
if ! visudo -c -f "$SUDOERS_TMP" >/dev/null 2>&1; then
|
||||
SUDOERS_VALID=0
|
||||
echo "⚠ The generated sudoers rules did not parse:" >&2
|
||||
@@ -1690,28 +1655,8 @@ fi
|
||||
|
||||
# Re-apply plugin directory permissions based on web service user
|
||||
echo "Re-applying plugin directory permissions..."
|
||||
# Determine web service user (check installed service, install scripts, or template)
|
||||
WEB_SERVICE_USER="root"
|
||||
if [ -f "/etc/systemd/system/ledmatrix-web.service" ]; then
|
||||
# Check actual installed service file (most accurate)
|
||||
WEB_SERVICE_USER=$(grep "^User=" /etc/systemd/system/ledmatrix-web.service | cut -d'=' -f2 || echo "root")
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh" ]; then
|
||||
# Check install_web_service.sh (used by first_time_install.sh)
|
||||
if grep -q "User=root" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="root"
|
||||
elif grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_web_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" ]; then
|
||||
WEB_SERVICE_USER=$(grep "^User=" "$PROJECT_ROOT_DIR/systemd/ledmatrix-web.service" | cut -d'=' -f2 || echo "root")
|
||||
if [ "$WEB_SERVICE_USER" = "__USER__" ] || [ -z "$WEB_SERVICE_USER" ]; then
|
||||
if [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
fi
|
||||
elif [ -f "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" ] && grep -q "User=\${ACTUAL_USER}" "$PROJECT_ROOT_DIR/scripts/install/install_service.sh"; then
|
||||
WEB_SERVICE_USER="$ACTUAL_USER"
|
||||
fi
|
||||
# Determine ownership based on web service user
|
||||
detect_web_service_user
|
||||
|
||||
# Set ownership based on web service user
|
||||
if [ "$WEB_SERVICE_USER" = "$ACTUAL_USER" ] || [ "$WEB_SERVICE_USER" != "root" ]; then
|
||||
|
||||
@@ -59,6 +59,16 @@ if [ ! -f "$SAFE_PIP_INSTALL_PATH" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# The rules are shared with first_time_install.sh (Step 10) so the two cannot
|
||||
# drift apart; add or remove a grant in lib_sudoers.sh, not here.
|
||||
SUDOERS_LIB="$PROJECT_DIR/lib_sudoers.sh"
|
||||
if [ ! -f "$SUDOERS_LIB" ]; then
|
||||
echo "Error: Sudoers rules library not found: $SUDOERS_LIB" >&2
|
||||
exit 1
|
||||
fi
|
||||
# shellcheck source=scripts/install/lib_sudoers.sh
|
||||
. "$SUDOERS_LIB"
|
||||
|
||||
echo "Command paths:"
|
||||
echo " Python: $PYTHON_PATH"
|
||||
echo " Systemctl: $SYSTEMCTL_PATH"
|
||||
@@ -72,56 +82,8 @@ echo " Safe pip install: $SAFE_PIP_INSTALL_PATH"
|
||||
# Create a temporary sudoers file
|
||||
TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
|
||||
|
||||
{
|
||||
echo "# LED Matrix Web Interface passwordless sudo configuration"
|
||||
echo "# This allows the web interface user to run specific commands without a password"
|
||||
echo ""
|
||||
echo "# Allow $WEB_USER to run specific commands without a password for the LED Matrix web interface"
|
||||
|
||||
# Optional: reboot/poweroff (non-critical — skip if not found)
|
||||
if [ -n "$REBOOT_PATH" ]; then
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $REBOOT_PATH"
|
||||
fi
|
||||
if [ -n "$POWEROFF_PATH" ]; then
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $POWEROFF_PATH"
|
||||
fi
|
||||
|
||||
# Required: systemctl
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service"
|
||||
|
||||
# Optional: journalctl (non-critical — skip if not found)
|
||||
#
|
||||
# NOEXEC, matching first_time_install.sh. These rules end in a wildcard and
|
||||
# journalctl starts a pager, so without it the caller can reach a shell:
|
||||
# less runs "!command" as the user the pager belongs to, which here is
|
||||
# root. NOEXEC stops the granted command executing anything of its own.
|
||||
if [ -n "$JOURNALCTL_PATH" ]; then
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *"
|
||||
fi
|
||||
|
||||
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/"
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_RM_PATH *"
|
||||
echo ""
|
||||
echo "# Allow web user to install a plugin's requirements.txt as root via vetted"
|
||||
echo "# helper script, so packages are visible to root-run ledmatrix.service"
|
||||
echo "# (not just the web interface's own user). The helper validates the target"
|
||||
echo "# is requirements.txt at the project root or under plugin-repos/ or plugins/."
|
||||
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_PATH *"
|
||||
} > "$TEMP_SUDOERS"
|
||||
web_sudoers_rules "$WEB_USER" "$PROJECT_ROOT" "$SYSTEMCTL_PATH" "$BASH_PATH" \
|
||||
"$REBOOT_PATH" "$POWEROFF_PATH" "$JOURNALCTL_PATH" > "$TEMP_SUDOERS"
|
||||
|
||||
# Never offer to install rules we have not parsed. A malformed drop-in in
|
||||
# /etc/sudoers.d makes sudo refuse every command for every user.
|
||||
|
||||
Executable
+76
@@ -0,0 +1,76 @@
|
||||
#!/bin/bash
|
||||
#
|
||||
# The web interface's passwordless-sudo allow-list, /etc/sudoers.d/ledmatrix_web.
|
||||
#
|
||||
# Sourced by first_time_install.sh (Step 10) and
|
||||
# scripts/install/configure_web_sudo.sh. Both used to carry their own copy of
|
||||
# these rules, and the copies drifted: one granted safe_pip_install.sh and the
|
||||
# other did not. Each caller still owns its own validate (visudo -c) / install /
|
||||
# confirm flow; this file only prints the rules.
|
||||
#
|
||||
# Add or remove a grant here and nowhere else.
|
||||
|
||||
# web_sudoers_rules WEB_USER PROJECT_ROOT SYSTEMCTL_PATH BASH_PATH REBOOT_PATH POWEROFF_PATH JOURNALCTL_PATH
|
||||
#
|
||||
# Print the ledmatrix_web sudoers rules to stdout.
|
||||
#
|
||||
# SYSTEMCTL_PATH and BASH_PATH are required, and the caller must make sure they
|
||||
# are not empty: `visudo -c` does not catch every such rule (with an empty
|
||||
# BASH_PATH the helper rules still parse, granting the script itself).
|
||||
# first_time_install.sh stops on a failed `which`; configure_web_sudo.sh checks
|
||||
# them before calling this.
|
||||
# REBOOT_PATH, POWEROFF_PATH and JOURNALCTL_PATH are optional: pass "" and
|
||||
# their rules are left out.
|
||||
web_sudoers_rules() {
|
||||
local WEB_USER="${1:-}"
|
||||
local PROJECT_ROOT="${2:-}"
|
||||
local SYSTEMCTL_PATH="${3:-}"
|
||||
local BASH_PATH="${4:-}"
|
||||
local REBOOT_PATH="${5:-}"
|
||||
local POWEROFF_PATH="${6:-}"
|
||||
local JOURNALCTL_PATH="${7:-}"
|
||||
|
||||
cat << EOF
|
||||
# LED Matrix Web Interface passwordless sudo configuration
|
||||
# This allows the web interface user to run specific commands without a password
|
||||
|
||||
# Allow $WEB_USER to run specific commands without a password for the LED Matrix web interface
|
||||
EOF
|
||||
if [ -n "$REBOOT_PATH" ]; then
|
||||
printf '%s\n' "$WEB_USER ALL=(ALL) NOPASSWD: $REBOOT_PATH"
|
||||
fi
|
||||
if [ -n "$POWEROFF_PATH" ]; then
|
||||
printf '%s\n' "$WEB_USER ALL=(ALL) NOPASSWD: $POWEROFF_PATH"
|
||||
fi
|
||||
cat << EOF
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH enable ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH disable ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH status ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh *
|
||||
# Install a requirements.txt as root via vetted helper, so packages are visible
|
||||
# to root-run ledmatrix.service (not just the web interface's own user).
|
||||
$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *
|
||||
EOF
|
||||
if [ -n "$JOURNALCTL_PATH" ]; then
|
||||
cat << EOF
|
||||
# NOEXEC, because these rules end in a wildcard and journalctl starts a pager
|
||||
# when its output is a terminal. From that pager (less) a "!sh" is a root
|
||||
# shell -- the standard journalctl escalation. The web interface always passes
|
||||
# --no-pager, so nothing here needs it, but the rule cannot require a flag that
|
||||
# sits in the middle of the command line. NOEXEC stops the command executing
|
||||
# another program at all, which closes the hole without depending on wildcard
|
||||
# matching subtleties.
|
||||
$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix.service *
|
||||
$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -u ledmatrix *
|
||||
$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
|
||||
EOF
|
||||
fi
|
||||
}
|
||||
@@ -26,6 +26,7 @@ from enum import Enum
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
import pytz
|
||||
from src.cache_manager import CacheManager
|
||||
from src.common.json_body import response_json
|
||||
from src.common.espn_dates import (
|
||||
RANGE_RETRY_SECONDS,
|
||||
_note_range_rejected,
|
||||
@@ -389,7 +390,7 @@ class BackgroundDataService:
|
||||
response.raise_for_status()
|
||||
else:
|
||||
response.raise_for_status()
|
||||
data = response.json()
|
||||
data = response_json(response)
|
||||
|
||||
# Validate data structure
|
||||
if not isinstance(data, dict):
|
||||
|
||||
Vendored
+43
@@ -7,6 +7,7 @@ Handles persistent disk-based caching with atomic writes and error recovery.
|
||||
import json
|
||||
import math
|
||||
import os
|
||||
import re
|
||||
import stat
|
||||
import time
|
||||
import tempfile
|
||||
@@ -98,6 +99,40 @@ def _replace_nonfinite(obj: Any) -> Any:
|
||||
# deleted. Both halves are covered by test/test_cache_nonfinite_floats.py.
|
||||
|
||||
|
||||
#: Enough of a record to hold its header: ``{"timestamp":<float>,"ttl":<n>,``.
|
||||
_HEAD_BYTES = 256
|
||||
|
||||
#: A record written with its header first (CacheManager.set does). Anything
|
||||
#: else -- older files with "data" first, records from other writers -- does not
|
||||
#: match and is parsed in full, as before.
|
||||
_HEAD_RE = re.compile(
|
||||
rb'\A\s*\{\s*"timestamp"\s*:\s*(-?[0-9][0-9.eE+-]*)\s*'
|
||||
rb'(?:,\s*"ttl"\s*:\s*(-?[0-9][0-9.eE+-]*))?\s*[,}]'
|
||||
)
|
||||
|
||||
|
||||
def _stale_from_head(head: bytes, max_age: Optional[int], now: float) -> bool:
|
||||
"""True when a record's header alone shows it has expired.
|
||||
|
||||
Mirrors the expiry rule in DiskCache.get: a per-entry ttl wins over the
|
||||
caller's max_age, and no limit at all means never stale. False whenever the
|
||||
header cannot be read, so the full parse decides as it always did.
|
||||
"""
|
||||
match = _HEAD_RE.match(head)
|
||||
if not match:
|
||||
return False
|
||||
try:
|
||||
timestamp = float(match.group(1))
|
||||
limit = max_age
|
||||
if match.group(2) is not None:
|
||||
ttl = float(match.group(2))
|
||||
if ttl >= 0:
|
||||
limit = ttl
|
||||
except ValueError:
|
||||
return False
|
||||
return limit is not None and (now - timestamp) > limit
|
||||
|
||||
|
||||
if orjson is not None:
|
||||
# Encoding the cache record dominated the background fetch worker: on a
|
||||
# Pi 4, stdlib json.dumps runs ~12ms per MB and holds the GIL for all of
|
||||
@@ -266,6 +301,14 @@ class DiskCache:
|
||||
try:
|
||||
with self._lock:
|
||||
with open(cache_path, 'rb') as f:
|
||||
# Decide staleness from the header before paying for the
|
||||
# parse. A stale read is the common case for the biggest
|
||||
# records (a season schedule is re-fetched when its cache
|
||||
# expires), and parsing 53MB to throw it away held the GIL
|
||||
# for ~1.8s -- a visible freeze on the panel.
|
||||
if _stale_from_head(f.read(_HEAD_BYTES), max_age, time.time()):
|
||||
return None
|
||||
f.seek(0)
|
||||
record = _loads(f.read())
|
||||
|
||||
# Determine record timestamp (prefer embedded, else file mtime)
|
||||
|
||||
@@ -522,8 +522,9 @@ class CacheManager:
|
||||
def update_cache(self, data_type: str, data: Dict[str, Any]) -> bool:
|
||||
"""Update cache with new data."""
|
||||
cache_data = {
|
||||
# Header first; see DiskCache's stale check.
|
||||
'timestamp': time.time(),
|
||||
'data': data,
|
||||
'timestamp': time.time()
|
||||
}
|
||||
return self.save_cache(data_type, cache_data)
|
||||
|
||||
@@ -556,12 +557,15 @@ class CacheManager:
|
||||
from the key and is only a fallback for entries that did not
|
||||
say. Omit it to keep that inferred behaviour.
|
||||
"""
|
||||
cache_data = {
|
||||
'data': data,
|
||||
'timestamp': time.time()
|
||||
}
|
||||
# timestamp and ttl before data, so they are the first bytes on disk:
|
||||
# DiskCache.get reads them from the head of the file and can call a
|
||||
# record stale without parsing it. That matters for the big ones -- a
|
||||
# whole MLB season is 53MB and ~1.8s of orjson.loads with the GIL held,
|
||||
# paid in full only to learn the record had expired.
|
||||
cache_data: Dict[str, Any] = {'timestamp': time.time()}
|
||||
if ttl is not None:
|
||||
cache_data['ttl'] = ttl
|
||||
cache_data['data'] = data
|
||||
self.save_cache(key, cache_data)
|
||||
|
||||
@deprecated("3.7.0")
|
||||
|
||||
@@ -39,6 +39,14 @@ from datetime import date, timedelta
|
||||
from functools import partial
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
|
||||
try:
|
||||
from src.common.json_body import response_json
|
||||
except ImportError:
|
||||
# Plugins bundle copies of this module for older cores, which predate
|
||||
# json_body; the stdlib parse is what those cores always used.
|
||||
def response_json(response: Any) -> Any:
|
||||
return response.json()
|
||||
|
||||
# Above this, ESPN returns a truncated list instead of an error. See module
|
||||
# docstring: 500 is the largest value measured to return complete data.
|
||||
ESPN_MAX_LIMIT = 500
|
||||
@@ -194,7 +202,7 @@ def _fetch_one_chunk(
|
||||
timeout=timeout,
|
||||
)
|
||||
response.raise_for_status()
|
||||
return response.json()
|
||||
return response_json(response)
|
||||
except Exception as exc: # noqa: BLE001 - see docstring
|
||||
if logger:
|
||||
logger.warning("ESPN chunk %s failed, skipping it: %s", chunk, exc)
|
||||
@@ -371,4 +379,4 @@ def fetch_espn_scoreboard(
|
||||
if data is not None:
|
||||
return data
|
||||
response.raise_for_status()
|
||||
return response.json()
|
||||
return response_json(response)
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
"""Parse an HTTP response body as JSON, with orjson when it is installed.
|
||||
|
||||
``requests``' ``response.json()`` uses the stdlib parser. For the payloads the
|
||||
sports plugins fetch -- a season schedule is tens of MB -- that runs ~1.7x
|
||||
slower than orjson on a Pi 4 (3.1s against 1.8s for the 53MB MLB season), and
|
||||
both hold the GIL for the whole parse, which freezes the display for as long.
|
||||
Nothing else changes: the result is the same Python objects.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
try:
|
||||
import orjson
|
||||
except ImportError: # optional dependency; see docs/SCROLL_PERFORMANCE.md
|
||||
orjson = None
|
||||
|
||||
|
||||
def response_json(response: Any) -> Any:
|
||||
"""``response.json()``, parsed by orjson when available."""
|
||||
body = getattr(response, "content", None)
|
||||
if orjson is None or not isinstance(body, (bytes, bytearray)):
|
||||
return response.json()
|
||||
try:
|
||||
return orjson.loads(body)
|
||||
except orjson.JSONDecodeError:
|
||||
# Let requests raise its usual error, with its usual message.
|
||||
return response.json()
|
||||
+16
-3
@@ -24,8 +24,13 @@ _REDACT_CREDENTIAL = re.compile(
|
||||
# silently leak the ones nobody thought of. Not covered by the generic pattern
|
||||
# above, whose value part stops at whitespace and so would keep the credential
|
||||
# once a space follows the scheme.
|
||||
#
|
||||
# The opening quote and the whitespace after it are one optional unit. Written
|
||||
# `\s*["\']?\s*`, a whitespace run with no quote in it could be split between
|
||||
# the two `\s*` in every possible way, and a header with no credential after
|
||||
# it tried them all: quadratic, 8s for 20k spaces.
|
||||
_REDACT_AUTH_HEADER = re.compile(
|
||||
r'((?:proxy-)?authorization["\']?\s*[=:]\s*["\']?\s*'
|
||||
r'((?:proxy-)?authorization["\']?\s*[=:]\s*(?:["\']\s*)?'
|
||||
r'(?:[A-Za-z][\w.+-]*[ \t]+)?)' # optional scheme name, kept
|
||||
r'([^\s,"\'<>}]+)', # the credential, redacted
|
||||
re.IGNORECASE,
|
||||
@@ -34,8 +39,16 @@ _REDACT_AUTH_HEADER = re.compile(
|
||||
# Credentials embedded in a URL: https://user:password@host. requests quotes
|
||||
# the full URL in its exceptions, so this is a realistic leak. The username is
|
||||
# kept -- it identifies which account failed without being the secret.
|
||||
_REDACT_URL_USERINFO = re.compile(r'([a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
|
||||
re.IGNORECASE)
|
||||
#
|
||||
# A match may only start where a run of scheme characters starts. Unanchored,
|
||||
# `[a-z][a-z0-9+.-]*://` was tried from every letter of a long run (a hex
|
||||
# digest, an ID, a blob of response body), each attempt reading to the end of
|
||||
# the run: quadratic, 1.6s for 20k characters, all of it holding the GIL.
|
||||
# Leading digits and `+.-` sit inside group 1 so the substitution puts them
|
||||
# back; the scheme proper still has to start with a letter.
|
||||
_REDACT_URL_USERINFO = re.compile(
|
||||
r'((?<![a-z0-9+.-])[0-9+.-]*[a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
|
||||
re.IGNORECASE)
|
||||
|
||||
|
||||
def redact_credentials(text: str) -> str:
|
||||
|
||||
@@ -320,5 +320,6 @@ def test_units_installers_and_updater_agree():
|
||||
assert (f'systemd/{unit}', f'/etc/systemd/system/{unit}') in StartupValidator._UNITS
|
||||
|
||||
# Triggering takes no privilege any more; no sudoers rule should linger.
|
||||
for sudoers in ('scripts/install/configure_web_sudo.sh', 'first_time_install.sh'):
|
||||
for sudoers in ('scripts/install/configure_web_sudo.sh', 'first_time_install.sh',
|
||||
'scripts/install/lib_sudoers.sh'):
|
||||
assert not re.search(r'NOPASSWD:.*update-verify', (ROOT / sudoers).read_text(encoding='utf-8')), sudoers
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
"""A stale cache record is recognised from its header, without parsing it.
|
||||
|
||||
The sports plugins cache whole season schedules -- 53MB for MLB, 18MB for NHL.
|
||||
When one expired, DiskCache.get parsed all of it (~1.8s of orjson.loads on a
|
||||
Pi 4, GIL held, the whole display frozen) only to find the timestamp too old
|
||||
and throw the result away. CacheManager.set now writes timestamp and ttl ahead
|
||||
of the data, and DiskCache.get reads them from the first bytes of the file.
|
||||
"""
|
||||
|
||||
import json
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from src.cache import disk_cache as disk_cache_module
|
||||
from src.cache.disk_cache import DiskCache, _stale_from_head
|
||||
from src.common import json_body
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def disk(tmp_path):
|
||||
return DiskCache(cache_dir=str(tmp_path))
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def parses(monkeypatch):
|
||||
"""Count full parses of cache files."""
|
||||
calls = []
|
||||
real = disk_cache_module._loads
|
||||
|
||||
def counting(raw):
|
||||
calls.append(len(raw))
|
||||
return real(raw)
|
||||
|
||||
monkeypatch.setattr(disk_cache_module, "_loads", counting)
|
||||
return calls
|
||||
|
||||
|
||||
def _header_first(age=0.0, ttl=None, events=100):
|
||||
record = {"timestamp": time.time() - age}
|
||||
if ttl is not None:
|
||||
record["ttl"] = ttl
|
||||
record["data"] = {"events": [{"id": n, "name": "x" * 50} for n in range(events)]}
|
||||
return record
|
||||
|
||||
|
||||
def test_cache_manager_writes_the_header_first(monkeypatch):
|
||||
from src.cache_manager import CacheManager
|
||||
written = {}
|
||||
manager = CacheManager.__new__(CacheManager)
|
||||
monkeypatch.setattr(manager, "save_cache",
|
||||
lambda key, record: written.update({key: record}),
|
||||
raising=False)
|
||||
CacheManager.set(manager, "k", {"events": []}, ttl=60)
|
||||
assert list(written["k"]) == ["timestamp", "ttl", "data"]
|
||||
CacheManager.set(manager, "k", {"events": []})
|
||||
assert list(written["k"]) == ["timestamp", "data"]
|
||||
|
||||
|
||||
def test_a_stale_record_is_not_parsed(disk, parses):
|
||||
disk.set("season", _header_first(age=600))
|
||||
assert disk.get("season", max_age=300) is None
|
||||
assert parses == []
|
||||
|
||||
|
||||
def test_a_fresh_record_is_parsed_and_returned(disk, parses):
|
||||
disk.set("season", _header_first(age=10))
|
||||
record = disk.get("season", max_age=300)
|
||||
assert record["data"]["events"][0]["id"] == 0
|
||||
assert len(parses) == 1
|
||||
|
||||
|
||||
def test_the_entry_ttl_wins_over_max_age(disk, parses):
|
||||
disk.set("long", _header_first(age=600, ttl=3600))
|
||||
assert disk.get("long", max_age=300) is not None # ttl says fresh
|
||||
disk.set("short", _header_first(age=60, ttl=30))
|
||||
parses.clear()
|
||||
assert disk.get("short", max_age=300) is None # ttl says stale
|
||||
assert parses == []
|
||||
|
||||
|
||||
def test_no_limit_means_never_stale(disk):
|
||||
disk.set("forever", _header_first(age=10 ** 7))
|
||||
assert disk.get("forever", max_age=None) is not None
|
||||
|
||||
|
||||
def test_older_files_with_data_first_still_work(disk, parses):
|
||||
# Records written before the header moved: parsed in full, as before.
|
||||
disk.set("legacy_fresh", {"data": {"v": 1}, "timestamp": time.time()})
|
||||
disk.set("legacy_stale", {"data": {"v": 1}, "timestamp": time.time() - 600})
|
||||
assert disk.get("legacy_fresh", max_age=300)["data"] == {"v": 1}
|
||||
assert disk.get("legacy_stale", max_age=300) is None
|
||||
assert len(parses) == 2
|
||||
|
||||
|
||||
@pytest.mark.parametrize("head, stale", [
|
||||
(b'{"timestamp":100.0,"data":{}}', True),
|
||||
(b'{"timestamp": 100.0, "ttl": 1000, "data": {}}', False), # stdlib spacing
|
||||
(b'{"timestamp":1e2,"ttl":5,"data":1}', True),
|
||||
(b'{"timestamp":100.0}', True),
|
||||
(b'{"data":{},"timestamp":100.0}', False), # unknown layout
|
||||
(b'{"timestamp":"100.0","data":{}}', False), # string: parse it
|
||||
(b'', False),
|
||||
])
|
||||
def test_reading_the_header(head, stale):
|
||||
assert _stale_from_head(head, 300, now=1000.0) is stale
|
||||
|
||||
|
||||
def test_response_json_prefers_orjson_and_falls_back():
|
||||
payload = {"events": [1, 2, 3]}
|
||||
response = SimpleNamespace(content=json.dumps(payload).encode(),
|
||||
json=lambda: pytest.fail("used the slow path"))
|
||||
if json_body.orjson is None:
|
||||
pytest.skip("orjson not installed")
|
||||
assert json_body.response_json(response) == payload
|
||||
# A response object without bytes content (a test double) still works.
|
||||
assert json_body.response_json(SimpleNamespace(json=lambda: payload)) == payload
|
||||
@@ -0,0 +1,110 @@
|
||||
"""redact_credentials must stay linear in the length of its input.
|
||||
|
||||
Regressions under test, both quadratic regexes in src/redaction.py:
|
||||
|
||||
- The URL-userinfo pattern (`scheme://user:password@`) could start a match at
|
||||
every letter of a run of scheme characters, and each attempt read to the end
|
||||
of the run looking for `://`: 1.6s for a 20k-character run.
|
||||
- The Authorization-header pattern had two `\\s*` separated only by an
|
||||
optional quote, so a header followed by whitespace and no credential tried
|
||||
every split of that whitespace between them: 8s for 20k spaces.
|
||||
|
||||
The display service redacts every message, stack trace and context value it
|
||||
publishes in the error snapshot, and re.sub holds the GIL throughout, so an
|
||||
exception quoting a hex digest or a long ID stalled the render loop with it.
|
||||
test_error_snapshot_cross_process.py's snapshot-size test spent 140s here.
|
||||
|
||||
The fixed patterns have to redact exactly what the old ones did.
|
||||
"""
|
||||
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from src.redaction import redact_credentials
|
||||
|
||||
# Each timed input took seconds before the fix and takes about a millisecond
|
||||
# after it; the bound leaves CI plenty of headroom while still failing on a
|
||||
# quadratic pattern.
|
||||
_TIME_LIMIT = 1.0
|
||||
|
||||
|
||||
def _timed(text):
|
||||
start = time.perf_counter()
|
||||
result = redact_credentials(text)
|
||||
return result, time.perf_counter() - start
|
||||
|
||||
|
||||
class TestUrlUserinfo:
|
||||
@pytest.mark.parametrize("text,expected", [
|
||||
("401 for https://user:hunter2@example.com/api",
|
||||
"401 for https://user:<redacted>@example.com/api"),
|
||||
("HTTPS://USER:HUNTER2@EXAMPLE.COM",
|
||||
"HTTPS://USER:<redacted>@EXAMPLE.COM"),
|
||||
("git+ssh://deploy:hunter2@host/repo",
|
||||
"git+ssh://deploy:<redacted>@host/repo"),
|
||||
# The scheme starts after digits or +.- in the same run. Those
|
||||
# characters must survive, and the password must still go.
|
||||
("1http://user:hunter2@host", "1http://user:<redacted>@host"),
|
||||
("+.-http://user:hunter2@host", "+.-http://user:<redacted>@host"),
|
||||
("a1+http://user:hunter2@host", "a1+http://user:<redacted>@host"),
|
||||
("see a://u:first@b and c://v:second@d",
|
||||
"see a://u:<redacted>@b and c://v:<redacted>@d"),
|
||||
])
|
||||
def test_password_is_redacted_and_the_rest_kept(self, text, expected):
|
||||
assert redact_credentials(text) == expected
|
||||
|
||||
def test_a_url_without_a_password_is_untouched(self):
|
||||
text = "GET https://user@example.com/path failed"
|
||||
assert redact_credentials(text) == text
|
||||
|
||||
|
||||
class TestAuthorizationHeader:
|
||||
@pytest.mark.parametrize("text,expected", [
|
||||
("Authorization: Bearer eyJ.SECRET.sig", "Authorization: Bearer <redacted>"),
|
||||
("Proxy-Authorization: Basic dXNlcg==", "Proxy-Authorization: Basic <redacted>"),
|
||||
("authorization: barecredential", "authorization: <redacted>"),
|
||||
# Whitespace and an opening quote around the value, in either order.
|
||||
('authorization=" Bearer tok"', 'authorization=" Bearer <redacted>"'),
|
||||
("authorization: ' tok'", "authorization: ' <redacted>'"),
|
||||
("authorization:\n\tBearer tok", "authorization:\n\tBearer <redacted>"),
|
||||
])
|
||||
def test_credential_is_redacted_and_the_rest_kept(self, text, expected):
|
||||
assert redact_credentials(text) == expected
|
||||
|
||||
@pytest.mark.parametrize("text", ["authorization: ", "authorization: , next"])
|
||||
def test_a_header_without_a_credential_is_untouched(self, text):
|
||||
assert redact_credentials(text) == text
|
||||
|
||||
|
||||
class TestLinearTime:
|
||||
@pytest.mark.parametrize("unit", ["x", "0123456789abcdef", "1a", "a+", "1"])
|
||||
def test_long_scheme_character_runs(self, unit):
|
||||
text = (unit * 50_000)[:50_000]
|
||||
result, elapsed = _timed(text)
|
||||
assert result == text
|
||||
assert elapsed < _TIME_LIMIT, f"{elapsed:.2f}s to redact {len(text)} chars of {unit!r}"
|
||||
|
||||
def test_a_credential_after_a_long_run_is_still_found(self):
|
||||
run = "ab12" * 10_000
|
||||
result, elapsed = _timed(f"{run} https://user:hunter2@example.com")
|
||||
assert result == f"{run} https://user:<redacted>@example.com"
|
||||
assert elapsed < _TIME_LIMIT
|
||||
|
||||
@pytest.mark.parametrize("header,whitespace", [
|
||||
("authorization:", " "),
|
||||
("Proxy-Authorization:", "\t"),
|
||||
("authorization=", "\n"),
|
||||
])
|
||||
def test_a_header_followed_by_long_whitespace(self, header, whitespace):
|
||||
text = header + whitespace * 20_000 + ","
|
||||
result, elapsed = _timed(text)
|
||||
assert result == text
|
||||
assert elapsed < _TIME_LIMIT, (
|
||||
f"{elapsed:.2f}s to redact {header!r} and {len(text) - len(header)} more chars")
|
||||
|
||||
def test_a_credential_after_long_whitespace_is_still_found(self):
|
||||
gap = " " * 20_000
|
||||
result, elapsed = _timed(f"authorization:{gap}Bearer tok")
|
||||
assert result == f"authorization:{gap}Bearer <redacted>"
|
||||
assert elapsed < _TIME_LIMIT
|
||||
@@ -37,6 +37,9 @@ ROOT = Path(__file__).resolve().parent.parent
|
||||
INSTALLERS = (
|
||||
ROOT / "first_time_install.sh",
|
||||
ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
|
||||
# The ledmatrix_web rules, which first_time_install.sh and
|
||||
# configure_web_sudo.sh both take from here.
|
||||
ROOT / "scripts" / "install" / "lib_sudoers.sh",
|
||||
)
|
||||
|
||||
#: Commands this change grants, each fully literal in the source.
|
||||
|
||||
@@ -62,33 +62,135 @@ def test_configure_web_sudo_validates_before_installing():
|
||||
assert validate < install, "the rules must be checked before they are installed"
|
||||
|
||||
|
||||
def _render_first_time_sudoers(project_root, user):
|
||||
"""Run the installer's own sudoers heredoc with realistic values."""
|
||||
def test_a_missing_rules_library_installs_nothing():
|
||||
"""If lib_sudoers.sh is missing, nothing is generated -- and an empty file
|
||||
would pass `visudo -c` -- so that branch must set the flag the install is
|
||||
gated on."""
|
||||
body = _read(FIRST_TIME)
|
||||
start = body.index("# Create sudoers content")
|
||||
missing = body.index('if [ -f "$SUDOERS_LIB" ]; then')
|
||||
flagged = body.index("SUDOERS_VALID=0", missing)
|
||||
validate = body.index('visudo -c -f "$SUDOERS_TMP"')
|
||||
install = body.index('cp "$SUDOERS_TMP" "$SUDOERS_FILE"')
|
||||
gate = body.rindex('if [ "$SUDOERS_VALID" = "0" ]; then', 0, install)
|
||||
assert missing < flagged < validate < gate < install
|
||||
|
||||
|
||||
def _step10_generation(body):
|
||||
"""first_time_install.sh's own Step 10 code that writes $SUDOERS_TMP."""
|
||||
start = body.index("# The rules themselves live in scripts/install/lib_sudoers.sh")
|
||||
end = body.index("# Never install rules we have not parsed.")
|
||||
block = body[start:end]
|
||||
out = os.path.join(project_root, "rendered")
|
||||
return body[start:end]
|
||||
|
||||
|
||||
def _run_step10_generation(project_root, user, out):
|
||||
"""Run the installer's Step 10 generation with realistic values.
|
||||
|
||||
Returns the SUDOERS_VALID it leaves behind."""
|
||||
script = "\n".join(
|
||||
[
|
||||
"set -euo pipefail",
|
||||
"set -Eeuo pipefail",
|
||||
f"ACTUAL_USER={user}",
|
||||
f"PROJECT_ROOT_DIR={project_root}",
|
||||
'SUDOERS_TMP="$(mktemp)"',
|
||||
"PYTHON_PATH=$(which python3)",
|
||||
f"PROJECT_ROOT_DIR='{project_root}'",
|
||||
f"SUDOERS_TMP='{out}'",
|
||||
"SUDOERS_FILE=/etc/sudoers.d/ledmatrix_web",
|
||||
"SYSTEMCTL_PATH=/usr/bin/systemctl",
|
||||
"REBOOT_PATH=/usr/sbin/reboot",
|
||||
"POWEROFF_PATH=/usr/sbin/poweroff",
|
||||
"BASH_PATH=$(which bash)",
|
||||
"JOURNALCTL_PATH=/usr/bin/journalctl",
|
||||
block,
|
||||
f'cp "$SUDOERS_TMP" {out}',
|
||||
_step10_generation(_read(FIRST_TIME)),
|
||||
'printf %s "$SUDOERS_VALID"',
|
||||
]
|
||||
)
|
||||
subprocess.run(["bash", "-c", script], check=True)
|
||||
return subprocess.run(
|
||||
["bash", "-c", script], check=True, capture_output=True, text=True
|
||||
).stdout
|
||||
|
||||
|
||||
def _render_first_time_sudoers(tmp, user):
|
||||
"""The rules first_time_install.sh generates, via the shared library."""
|
||||
out = os.path.join(tmp, "rendered")
|
||||
assert _run_step10_generation(REPO_ROOT, user, out) == "1"
|
||||
return out
|
||||
|
||||
|
||||
def _run_step10(tmp, project_root, visudo_ok, existing=None):
|
||||
"""Run all of Step 10 against a sudoers file in `tmp`, never /etc.
|
||||
|
||||
systemctl, reboot, poweroff, journalctl and visudo are stubs, so the
|
||||
outcome does not depend on the machine running the test."""
|
||||
body = _read(FIRST_TIME)
|
||||
step = body[body.index('CURRENT_STEP="Configure passwordless sudo access"'):
|
||||
body.index('CURRENT_STEP="Configure WiFi management permissions"')]
|
||||
target = os.path.join(tmp, "ledmatrix_web")
|
||||
real = 'SUDOERS_FILE="/etc/sudoers.d/ledmatrix_web"'
|
||||
assert step.count(real) == 1
|
||||
step = step.replace(real, f"SUDOERS_FILE='{target}'")
|
||||
stubs = os.path.join(tmp, "stubs")
|
||||
os.mkdir(stubs)
|
||||
for name, code in (("systemctl", 0), ("reboot", 0), ("poweroff", 0),
|
||||
("journalctl", 0), ("visudo", 0 if visudo_ok else 1)):
|
||||
path = os.path.join(stubs, name)
|
||||
with open(path, "w", encoding="utf-8") as handle:
|
||||
handle.write(f"#!/bin/sh\nexit {code}\n")
|
||||
os.chmod(path, 0o755)
|
||||
if existing is not None:
|
||||
with open(target, "w", encoding="utf-8") as handle:
|
||||
handle.write(existing)
|
||||
env = dict(os.environ, TMPDIR=tmp,
|
||||
PATH=os.pathsep.join([stubs, os.path.dirname(sys.executable),
|
||||
"/usr/bin", "/bin"]))
|
||||
script = "\n".join(["set -Eeuo pipefail", "ACTUAL_USER=ledmatrix",
|
||||
f"PROJECT_ROOT_DIR='{project_root}'", step])
|
||||
result = subprocess.run(["bash", "-c", script], env=env,
|
||||
capture_output=True, text=True)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
return target, stubs, result
|
||||
|
||||
|
||||
_POSIX_STEP10 = pytest.mark.skipif(
|
||||
sys.platform == "win32" or shutil.which("which") is None,
|
||||
reason="needs a POSIX bash and which")
|
||||
|
||||
|
||||
@_POSIX_STEP10
|
||||
def test_step10_installs_the_generated_rules():
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
target, stubs, _ = _run_step10(tmp, REPO_ROOT, visudo_ok=True)
|
||||
assert oct(os.stat(target).st_mode & 0o777) == "0o440"
|
||||
with open(target, encoding="utf-8") as handle:
|
||||
installed = handle.read()
|
||||
lib = os.path.join(REPO_ROOT, "scripts", "install", "lib_sudoers.sh")
|
||||
expected = subprocess.run(
|
||||
["bash", "-c", '. "$1"; web_sudoers_rules ledmatrix "$2" "$3/systemctl" '
|
||||
'"$(command -v bash)" "$3/reboot" "$3/poweroff" "$3/journalctl"',
|
||||
"_", lib, REPO_ROOT, stubs],
|
||||
check=True, capture_output=True, text=True,
|
||||
env=dict(os.environ, PATH=os.pathsep.join([stubs, "/usr/bin", "/bin"])),
|
||||
).stdout
|
||||
assert installed == expected
|
||||
assert not [f for f in os.listdir(tmp) if f.startswith("ledmatrix_web_sudoers.")]
|
||||
|
||||
|
||||
@_POSIX_STEP10
|
||||
def test_step10_without_the_library_keeps_the_existing_file():
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
target, _, result = _run_step10(tmp, tmp, visudo_ok=True, existing="keep\n")
|
||||
with open(target, encoding="utf-8") as handle:
|
||||
assert handle.read() == "keep\n"
|
||||
assert "lib_sudoers.sh not found" in result.stderr
|
||||
assert "Passwordless sudo access configured" not in result.stdout
|
||||
|
||||
|
||||
@_POSIX_STEP10
|
||||
def test_step10_keeps_the_existing_file_when_the_rules_do_not_parse():
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
target, _, result = _run_step10(tmp, REPO_ROOT, visudo_ok=False, existing="keep\n")
|
||||
with open(target, encoding="utf-8") as handle:
|
||||
assert handle.read() == "keep\n"
|
||||
assert "did not parse" in result.stderr
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="visudo is POSIX only")
|
||||
@pytest.mark.skipif(VISUDO is None, reason="visudo not installed")
|
||||
def test_the_rules_the_installer_emits_actually_parse():
|
||||
|
||||
@@ -30,10 +30,14 @@ ROOT = Path(__file__).resolve().parent.parent
|
||||
INSTALLERS = (
|
||||
ROOT / "first_time_install.sh",
|
||||
ROOT / "scripts" / "install" / "configure_wifi_permissions.sh",
|
||||
# Writes the same journalctl grants as first_time_install.sh. It was
|
||||
# missing here, and because of that this suite passed while three
|
||||
# ungranted wildcard rules sat in it.
|
||||
# Used to write its own copy of the journalctl grants. It was missing
|
||||
# here, and because of that this suite passed while three untagged
|
||||
# wildcard rules sat in it. Both it and first_time_install.sh now take
|
||||
# their rules from lib_sudoers.sh; they stay listed so a rule written
|
||||
# directly into either one is still checked.
|
||||
ROOT / "scripts" / "install" / "configure_web_sudo.sh",
|
||||
# The ledmatrix_web rules, shared by both installers.
|
||||
ROOT / "scripts" / "install" / "lib_sudoers.sh",
|
||||
)
|
||||
|
||||
#: Commands that will start another program of their own accord -- a pager, an
|
||||
|
||||
@@ -1,53 +1,189 @@
|
||||
"""The two installers that write /etc/sudoers.d/ledmatrix_web must agree.
|
||||
"""One generator writes /etc/sudoers.d/ledmatrix_web, and both installers use it.
|
||||
|
||||
first_time_install.sh (Step 10, a heredoc) and scripts/install/configure_web_sudo.sh
|
||||
(a block of echo lines) each generate the web user's sudo allow-list. They
|
||||
drifted: configure_web_sudo.sh granted scripts/fix_perms/safe_pip_install.sh
|
||||
but first_time_install.sh did not, so on a device set up only by the first-time
|
||||
first_time_install.sh (Step 10) and scripts/install/configure_web_sudo.sh each
|
||||
used to carry their own copy of the web user's sudo allow-list -- a heredoc in
|
||||
one, a block of echo lines in the other -- and the copies drifted:
|
||||
configure_web_sudo.sh granted scripts/fix_perms/safe_pip_install.sh but
|
||||
first_time_install.sh did not, so on a device set up only by the first-time
|
||||
installer permission_utils.install_requirements_file could not use the root
|
||||
wrapper and fell back to a user-level install that root-run ledmatrix.service
|
||||
may not see (and the auto-update rollback reported its reinstall as failed).
|
||||
|
||||
This compares the granted command sets after normalising the spellings that
|
||||
differ between the files but expand identically at install time:
|
||||
$WEB_USER/$ACTUAL_USER, $PROJECT_ROOT/$PROJECT_ROOT_DIR, and the helper-path
|
||||
variables configure_web_sudo.sh defines ($SAFE_RM_PATH, ...).
|
||||
The rules now live once, in web_sudoers_rules() in
|
||||
scripts/install/lib_sudoers.sh. What keeps them from drifting again:
|
||||
|
||||
* neither installer writes a rule line of its own, and each writes the
|
||||
generator's output to the very file it then validates and installs;
|
||||
* each passes its variables to the generator in the right positions -- checked
|
||||
by running the installer's own call line with distinct values;
|
||||
* the generator's grants are pinned to an explicit list below, so dropping,
|
||||
adding or re-pathing a grant is a deliberate edit to this file.
|
||||
|
||||
It also checks that every fix_perms helper granted via sudo is hardened to
|
||||
root:root in both scripts -- and, in first_time_install.sh, after Step 11's
|
||||
root:root in both installers -- and, in first_time_install.sh, after Step 11's
|
||||
project-wide chown to the user, which would otherwise undo it.
|
||||
"""
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
FIRST_TIME = ROOT / "first_time_install.sh"
|
||||
CONFIGURE = ROOT / "scripts" / "install" / "configure_web_sudo.sh"
|
||||
LIB = ROOT / "scripts" / "install" / "lib_sudoers.sh"
|
||||
|
||||
#: Grants that intentionally exist in only one installer, as normalised
|
||||
#: commands. There are none today; add one here with a reason rather than
|
||||
#: loosening the comparison.
|
||||
ONLY_IN_FIRST_TIME = frozenset()
|
||||
ONLY_IN_CONFIGURE = frozenset()
|
||||
#: Every grant web_sudoers_rules() writes, as (tags, command) with the
|
||||
#: generator's own variable names. Changing the allow-list means changing this.
|
||||
EXPECTED_GRANTS = frozenset({
|
||||
("NOPASSWD:", "$REBOOT_PATH"),
|
||||
("NOPASSWD:", "$POWEROFF_PATH"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH start ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH stop ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH restart ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH enable ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH disable ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH status ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH is-active ledmatrix"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH is-active ledmatrix.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH start ledmatrix-web.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH stop ledmatrix-web.service"),
|
||||
("NOPASSWD:", "$SYSTEMCTL_PATH restart ledmatrix-web.service"),
|
||||
("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh *"),
|
||||
("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *"),
|
||||
("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -u ledmatrix.service *"),
|
||||
("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -u ledmatrix *"),
|
||||
("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -t ledmatrix *"),
|
||||
})
|
||||
|
||||
#: The call each installer makes: its own names for the generator's arguments,
|
||||
#: in order, and the file it writes the rules to.
|
||||
CALLERS = {
|
||||
FIRST_TIME: (("$ACTUAL_USER", "$PROJECT_ROOT_DIR", "$SYSTEMCTL_PATH", "$BASH_PATH",
|
||||
"$REBOOT_PATH", "$POWEROFF_PATH", "$JOURNALCTL_PATH"), "$SUDOERS_TMP"),
|
||||
CONFIGURE: (("$WEB_USER", "$PROJECT_ROOT", "$SYSTEMCTL_PATH", "$BASH_PATH",
|
||||
"$REBOOT_PATH", "$POWEROFF_PATH", "$JOURNALCTL_PATH"), "$TEMP_SUDOERS"),
|
||||
}
|
||||
|
||||
RULE = re.compile(r'(\S+) ALL=\(ALL\) (NOPASSWD:(?:NOEXEC:)?)\s*(.*?)"?$')
|
||||
|
||||
|
||||
def _text(path):
|
||||
return path.read_text(encoding="utf-8", errors="replace")
|
||||
return path.read_text(encoding="utf-8", errors="replace").replace("\r\n", "\n")
|
||||
|
||||
|
||||
def _web_sudoers_section(path):
|
||||
"""The part of the script that writes the ledmatrix_web allow-list.
|
||||
def _generator_grants():
|
||||
"""{(tags, command)} for every rule line in lib_sudoers.sh."""
|
||||
grants = set()
|
||||
for line in _text(LIB).splitlines():
|
||||
m = RULE.search(line.strip())
|
||||
if m and m.group(1).endswith("$WEB_USER"):
|
||||
grants.add((m.group(2), " ".join(m.group(3).split())))
|
||||
return grants
|
||||
|
||||
first_time_install.sh also writes other files later (WiFi permissions are
|
||||
delegated to a separate script, but keep this robust against future
|
||||
additions), so restrict it to Step 10.
|
||||
"""
|
||||
|
||||
def _call(path):
|
||||
"""The installer's web_sudoers_rules statement, continuation lines joined."""
|
||||
text = _text(path)
|
||||
if path == FIRST_TIME:
|
||||
start = text.index('CURRENT_STEP="Configure passwordless sudo access"')
|
||||
end = text.index('CURRENT_STEP="Configure WiFi management permissions"')
|
||||
return text[start:end]
|
||||
return text
|
||||
calls = re.findall(r"^[ \t]*web_sudoers_rules\b(?:[^\n]*\\\n)*[^\n]*$", text, re.M)
|
||||
assert len(calls) == 1, f"{path.name}: expected one web_sudoers_rules call, found {calls}"
|
||||
return calls[0]
|
||||
|
||||
|
||||
def test_generator_grants_exactly_the_expected_rules():
|
||||
grants = _generator_grants()
|
||||
assert grants == EXPECTED_GRANTS, (
|
||||
f"lib_sudoers.sh grants changed:\n added: {sorted(grants - EXPECTED_GRANTS)}\n"
|
||||
f" removed: {sorted(EXPECTED_GRANTS - grants)}")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("installer", [FIRST_TIME, CONFIGURE], ids=lambda p: p.name)
|
||||
def test_installer_writes_no_rules_of_its_own(installer):
|
||||
"""A rule added to one installer only is how they drifted last time."""
|
||||
own = [line for line in _text(installer).splitlines()
|
||||
if "NOPASSWD" in line and not line.lstrip().startswith("#")]
|
||||
assert not own, f"{installer.name} writes sudoers rules itself: {own}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("installer", [FIRST_TIME, CONFIGURE], ids=lambda p: p.name)
|
||||
def test_installer_sources_the_generator_and_writes_what_it_validates(installer):
|
||||
text = _text(installer)
|
||||
assert "lib_sudoers.sh" in text, f"{installer.name} does not source lib_sudoers.sh"
|
||||
args, target = CALLERS[installer]
|
||||
call = _call(installer)
|
||||
words = call.replace("\\\n", " ").split()
|
||||
assert words[0] == "web_sudoers_rules"
|
||||
assert tuple(w.strip('"') for w in words[1:8]) == args, (
|
||||
f"{installer.name} passes the generator's arguments out of order: {call}")
|
||||
assert words[8:] == [">", f'"{target}"'], call
|
||||
# ...and that file is the one it runs visudo on.
|
||||
assert f'visudo -c -f "{target}"' in text
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32" or shutil.which("bash") is None,
|
||||
reason="needs a POSIX bash")
|
||||
@pytest.mark.parametrize("installer", [FIRST_TIME, CONFIGURE], ids=lambda p: p.name)
|
||||
def test_installer_call_renders_the_expected_rules(installer, tmp_path):
|
||||
"""Run the installer's own call line, with a distinct value per argument."""
|
||||
args, target = CALLERS[installer]
|
||||
values = {
|
||||
args[0]: "webuser", args[1]: "/srv/led root", args[2]: "/x/systemctl",
|
||||
args[3]: "/x/bash", args[4]: "/x/reboot", args[5]: "/x/poweroff",
|
||||
args[6]: "/x/journalctl", target: str(tmp_path / "out"),
|
||||
}
|
||||
assigns = "\n".join(f"{name[1:]}='{value}'" for name, value in values.items())
|
||||
script = f"set -euo pipefail\n. '{LIB}'\n{assigns}\n{_call(installer)}\n"
|
||||
subprocess.run(["bash", "-c", script], check=True)
|
||||
rendered = set()
|
||||
for line in (tmp_path / "out").read_text(encoding="utf-8").splitlines():
|
||||
m = RULE.match(line)
|
||||
if m:
|
||||
assert m.group(1) == "webuser", line
|
||||
rendered.add((m.group(2), m.group(3)))
|
||||
subst = {"$SYSTEMCTL_PATH": "/x/systemctl", "$BASH_PATH": "/x/bash",
|
||||
"$REBOOT_PATH": "/x/reboot", "$POWEROFF_PATH": "/x/poweroff",
|
||||
"$JOURNALCTL_PATH": "/x/journalctl", "$PROJECT_ROOT": "/srv/led root"}
|
||||
expected = set()
|
||||
for tags, command in EXPECTED_GRANTS:
|
||||
for var, value in subst.items():
|
||||
command = command.replace(var, value)
|
||||
expected.add((tags, command))
|
||||
assert rendered == expected
|
||||
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32" or shutil.which("bash") is None,
|
||||
reason="needs a POSIX bash")
|
||||
def test_optional_tools_are_left_out_when_absent(tmp_path):
|
||||
"""configure_web_sudo.sh passes "" for a missing reboot/poweroff/journalctl.
|
||||
|
||||
An empty path would otherwise leave `user ALL=(ALL) NOPASSWD: ` behind,
|
||||
which visudo rejects, and the whole file would not be installed.
|
||||
"""
|
||||
out = subprocess.run(
|
||||
["bash", "-c", f". '{LIB}'; web_sudoers_rules u /p /bin/systemctl /bin/bash '' '' ''"],
|
||||
check=True, capture_output=True, text=True).stdout
|
||||
rules = [line for line in out.splitlines() if RULE.match(line)]
|
||||
assert len(rules) == len(EXPECTED_GRANTS) - 5
|
||||
assert not [r for r in rules if r.rstrip().endswith("NOPASSWD:")]
|
||||
assert "journalctl" not in out
|
||||
|
||||
|
||||
def test_pip_install_helper_is_granted():
|
||||
wanted = ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *")
|
||||
assert wanted in _generator_grants()
|
||||
|
||||
|
||||
def _granted_helpers():
|
||||
helpers = set()
|
||||
for _, command in _generator_grants():
|
||||
m = re.search(r"scripts/fix_perms/([\w.-]+\.sh)", command)
|
||||
if m:
|
||||
helpers.add(m.group(1))
|
||||
assert helpers, "no fix_perms helper grant found; the parser matched nothing"
|
||||
return helpers
|
||||
|
||||
|
||||
def _variables(text):
|
||||
@@ -60,57 +196,9 @@ def _normalise(command, variables):
|
||||
for _ in range(3): # helper paths reference $PROJECT_ROOT
|
||||
command = re.sub(r"\$\{?([A-Z][A-Z0-9_]*)\}?",
|
||||
lambda m: variables.get(m.group(1), m.group(0)), command)
|
||||
command = command.replace("$PROJECT_ROOT_DIR", "$PROJECT_ROOT")
|
||||
return " ".join(command.split())
|
||||
|
||||
|
||||
def _grants(path):
|
||||
"""{(tags, command)} for every ledmatrix_web rule the script writes."""
|
||||
section = _web_sudoers_section(path)
|
||||
variables = _variables(_text(path))
|
||||
grants = set()
|
||||
for line in section.splitlines():
|
||||
m = re.search(r'\$(?:WEB_USER|ACTUAL_USER) ALL=\(ALL\) (NOPASSWD:(?:NOEXEC:)?)\s*(.*)$',
|
||||
line)
|
||||
if not m:
|
||||
continue
|
||||
command = m.group(2).rstrip().rstrip('"').rstrip()
|
||||
grants.add((m.group(1), _normalise(command, variables)))
|
||||
return grants
|
||||
|
||||
|
||||
def test_both_installers_generate_rules():
|
||||
# Guards against the parser silently matching nothing in either file.
|
||||
assert len(_grants(FIRST_TIME)) >= 15
|
||||
assert len(_grants(CONFIGURE)) >= 15
|
||||
|
||||
|
||||
def test_installers_grant_the_same_commands():
|
||||
first = _grants(FIRST_TIME)
|
||||
configure = _grants(CONFIGURE)
|
||||
only_first = {c for c in first - configure if c[1] not in ONLY_IN_FIRST_TIME}
|
||||
only_configure = {c for c in configure - first if c[1] not in ONLY_IN_CONFIGURE}
|
||||
assert not only_first and not only_configure, (
|
||||
"ledmatrix_web sudoers drift between installers:\n"
|
||||
f" only in first_time_install.sh: {sorted(only_first)}\n"
|
||||
f" only in configure_web_sudo.sh: {sorted(only_configure)}")
|
||||
|
||||
|
||||
def test_pip_install_helper_is_granted():
|
||||
wanted = ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *")
|
||||
assert wanted in _grants(FIRST_TIME)
|
||||
assert wanted in _grants(CONFIGURE)
|
||||
|
||||
|
||||
def _granted_helpers():
|
||||
helpers = set()
|
||||
for _, command in _grants(FIRST_TIME) | _grants(CONFIGURE):
|
||||
m = re.search(r"scripts/fix_perms/([\w.-]+\.sh)", command)
|
||||
if m:
|
||||
helpers.add(m.group(1))
|
||||
return helpers
|
||||
|
||||
|
||||
def test_every_granted_helper_is_hardened_in_configure_web_sudo():
|
||||
text = _text(CONFIGURE)
|
||||
variables = _variables(text)
|
||||
@@ -138,9 +226,8 @@ def test_no_grant_runs_a_file_the_web_user_can_edit():
|
||||
rule for it lets the web user rewrite the file and run it as root. The
|
||||
grants for display_controller.py, start_display.sh and stop_display.sh
|
||||
were exactly that, and nothing ever ran them through sudo."""
|
||||
for installer in (FIRST_TIME, CONFIGURE):
|
||||
for _, command in _grants(installer):
|
||||
for token in command.split():
|
||||
if token.startswith("$PROJECT_ROOT/"):
|
||||
assert token.startswith("$PROJECT_ROOT/scripts/fix_perms/"), (
|
||||
f"{installer.name} grants root on a user-owned file: {command}")
|
||||
for _, command in _generator_grants():
|
||||
for token in command.split():
|
||||
if token.startswith("$PROJECT_ROOT/"):
|
||||
assert token.startswith("$PROJECT_ROOT/scripts/fix_perms/"), (
|
||||
f"lib_sudoers.sh grants root on a user-owned file: {command}")
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Names two rarely-run api_v3 paths call must exist.
|
||||
|
||||
Both slipped through because nothing exercised them: the Pixlet editor's
|
||||
stop route only restarts the display after a SIGKILL, and the Starlark
|
||||
device-location resolver only builds a cache manager when the web app has
|
||||
not set one. Either raised NameError when it finally ran.
|
||||
"""
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from test._api_v3_test_helpers import api_v3_module # noqa: F401
|
||||
|
||||
|
||||
def test_the_editor_stop_route_can_restart_the_display():
|
||||
from web_interface.blueprints.api_v3 import starlark
|
||||
|
||||
assert callable(starlark._run_systemctl_command)
|
||||
|
||||
|
||||
def test_the_device_location_resolver_builds_without_a_cache_manager(api_v3_module):
|
||||
pkg = api_v3_module
|
||||
with patch.object(pkg.api_v3, 'cache_manager', None, create=True), \
|
||||
patch.object(pkg, '_starlark_device_location', None):
|
||||
resolver = pkg._get_starlark_device_location()
|
||||
assert resolver.cache_manager is None
|
||||
@@ -1,5 +1,6 @@
|
||||
"""Guards that every privileged systemctl call the web interface makes is
|
||||
covered by a passwordless-sudo grant in configure_web_sudo.sh.
|
||||
covered by a passwordless-sudo grant in scripts/install/lib_sudoers.sh, which
|
||||
both first_time_install.sh and configure_web_sudo.sh write the rules from.
|
||||
|
||||
The web interface runs headless (no TTY), so any `sudo` call that is not
|
||||
matched by a NOPASSWD rule in /etc/sudoers.d/ledmatrix_web falls back to a
|
||||
@@ -25,7 +26,7 @@ API_V3_PKG = PROJECT_ROOT / "web_interface" / "blueprints" / "api_v3"
|
||||
|
||||
def _api_v3_source() -> str:
|
||||
return "\n".join(p.read_text() for p in sorted(API_V3_PKG.glob("*.py")))
|
||||
SUDOERS_SCRIPT = PROJECT_ROOT / "scripts" / "install" / "configure_web_sudo.sh"
|
||||
SUDOERS_SCRIPT = PROJECT_ROOT / "scripts" / "install" / "lib_sudoers.sh"
|
||||
|
||||
|
||||
def _sudo_systemctl_calls(source: str) -> set[tuple[str, str]]:
|
||||
@@ -64,7 +65,7 @@ def test_every_sudo_systemctl_call_is_granted() -> None:
|
||||
uncovered = {c for c in calls if c not in rules}
|
||||
assert not uncovered, (
|
||||
"These sudo systemctl calls have no matching NOPASSWD grant in "
|
||||
"configure_web_sudo.sh; they will fail headless with "
|
||||
"lib_sudoers.sh; they will fail headless with "
|
||||
"'sudo: a terminal is required to read the password': "
|
||||
+ ", ".join(f"systemctl {v} {u}" for v, u in sorted(uncovered))
|
||||
)
|
||||
|
||||
@@ -1712,7 +1712,7 @@ def _get_starlark_device_location() -> DeviceLocationResolver:
|
||||
global _starlark_device_location
|
||||
if _starlark_device_location is None:
|
||||
_starlark_device_location = DeviceLocationResolver(
|
||||
getattr(api_v3, 'cache_manager', None) or _ensure_cache_manager(), logger)
|
||||
getattr(api_v3, 'cache_manager', None), logger)
|
||||
return _starlark_device_location
|
||||
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ from web_interface.blueprints.api_v3 import (
|
||||
_PIXLET_EDITOR_DEFAULT_TIMEOUT, _PIXLET_EDITOR_MAX_TIMEOUT,
|
||||
_PIXLET_EDITOR_SCRIPT, _PIXLET_EDITOR_STATE, _clear_pixlet_editor_state,
|
||||
_find_pixlet_binary, _install_star_file, _pixlet_editor_alive,
|
||||
_run_systemctl_command,
|
||||
_pixlet_editor_status, _read_pixlet_editor_state,
|
||||
_STARLARK_APPS_DIR, _standalone_render_starlark_app,
|
||||
_starlark_github_token, _starlark_manifest_lock,
|
||||
|
||||
Reference in New Issue
Block a user