Compare commits

...
6 Commits
Author SHA1 Message Date
Chuck 87509befdb Merge branch 'main' into claude/phase2-sports-twins 2026-09-24 15:51:23 -04:00
ChuckandClaude Opus 5.5 1fe7237799 refactor(install): generate the web sudoers rules in one place (#622)
* refactor(install): generate the web sudoers rules in one place

/etc/sudoers.d/ledmatrix_web was written by two copies of the same
allow-list: a heredoc in first_time_install.sh Step 10 and a block of
echo lines in scripts/install/configure_web_sudo.sh. They drifted before
(safe_pip_install.sh was granted by one only), and a test existed just
to catch that.

Both now call web_sudoers_rules() from the new
scripts/install/lib_sudoers.sh and keep their own validate (visudo -c),
install and confirm flows.

- first_time_install.sh output is byte-for-byte unchanged, so a device
  re-running the installer gets "already up to date". If the library is
  missing, Step 10 keeps the installed file and carries on, the same way
  it handles rules that fail visudo (an empty file would pass visudo).
- configure_web_sudo.sh now writes the installer's layout: same 18 rules,
  different comments and order. It still leaves out reboot, poweroff and
  journalctl when they are missing; the library does that for both.

The drift test now pins the generator's grants, checks that neither
installer writes rules of its own, and runs each installer's call line
to check the argument order. Tests that read the rule text now read the
library.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* refactor(install): detect the web service user in one function

first_time_install.sh pasted the same WEB_SERVICE_USER detection block
three times (Step 3.1's fallback, the plugin-repos setup and Step 11).
The copies were identical apart from comments; they now call
detect_web_service_user(), whose body is that block unchanged.

Behaviour is the same: the function sets the same global and always
returns 0, as the inline if-chain did. Checked on Linux against all
three original copies across 13 layouts (installed unit with and without
User=, the repo as shipped, each grep branch, template placeholders).

The comment notes that the install_web_service.sh / install_service.sh
greps no longer match anything, so until Step 8 installs the unit the
result is "root". That behaviour is left as it was.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-24 15:51:14 -04:00
ChuckandClaude Opus 5.5 ddf5f085a5 perf(cache): tell a stale record from its header instead of parsing it (#633)
The sports plugins cache whole season schedules: 53MB for MLB, 18MB for
NHL, 17MB for NCAA baseball. On a Pi 4, orjson.loads of the MLB file
takes ~1.8s with the GIL held, and every thread in the display service
waits -- the stall watchdog caught the render thread frozen 0.5-1.3s with
the interpreter itself blocked, right on these reads. When a season record
expired, DiskCache.get paid that whole parse 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 256 bytes of the file, applying
the same rule as before (a per-entry ttl wins over max_age; no limit
means never stale). A record that is stale is refused without being
parsed. Files in the old layout, and records from other writers, don't
match the header and are parsed in full as before.

Also: ESPN responses in the background data service and espn_dates are
parsed with orjson when it is installed (src/common/json_body.py). The
stdlib parser behind response.json() takes 3.1s on the MLB season
against orjson's 1.8s, both with the GIL held. espn_dates imports it with
a fallback, since plugins bundle copies of that module for older cores.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-24 15:50:50 -04:00
ChuckandClaude Opus 5.5 5baf983fe0 docs(scroll): explain the tear across the middle on fast scrolls (#620)
* docs(scroll): explain the tear across the middle on fast scrolls

A 1:32-multiplexed 64-row panel lights row 31 almost a whole refresh after
row 32, so fast scrolls show a sideways offset at mid-height of about
speed x refresh period. Documents the cause, how to read the real refresh
rate (show_refresh_rate prints with a carriage return), what was measured on
a single-chain 2x128x64 Pi 4 (pwm_bits, gpio_slowdown and an uncapped
refresh barely help; gpio_slowdown 2 glitches), and the fix that does help:
fewer pixels per output via parallel chains.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs(scroll): limit the 1:32 row-pair explanation to panels that scan that way

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-24 15:50:33 -04:00
ChuckandClaude Opus 5.5 82f3a3a3e4 fix(redaction): make credential redaction linear, not quadratic (#631)
* fix(redaction): make URL-userinfo redaction linear, not quadratic

_REDACT_URL_USERINFO 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 `://`. On a long unbroken run of letters or digits (a hex digest, an
ID, part of a response body) that is quadratic: 1.6s for 20k characters.

The display service redacts every message, stack trace and context value
it publishes in the error snapshot, holding the aggregator lock, and
re.sub holds the GIL for the whole call, so one such exception stalled
every thread, render loop included (~0.5s measured for 20k chars of hex).
It also made test_snapshot_stays_small the slowest test in the suite by
far: 142s of a 383s run, 139s of it in this one regex.

A match may now only start where a run of scheme characters starts
(negative lookbehind). Leading digits and `+.-` are captured in group 1
so the substitution restores them, and the scheme still has to start
with a letter, so what gets redacted is unchanged: old and new output
were identical on 300k fuzzed inputs. 20k chars now take ~0.5ms, 200k
~6ms, and test_snapshot_stays_small takes 0.8s.

test/test_redaction.py pins the exact output for schemes that begin after
digits or `+.-`, and bounds 50k-character runs at 1s; against the old
pattern those timing tests fail at 3-11s each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KMXdS2S4NXTJ8ET96GymhK

* fix(redaction): make Authorization-header redaction linear too

_REDACT_AUTH_HEADER matched the value's opening as `\s*["\']?\s*`: two
`\s*` separated only by an optional quote. With no quote, a whitespace
run could be split between them in every possible way, and when no
credential followed (end of text, or `,` `"` `<` ...) the engine tried
them all before giving up: quadratic, 8s for `authorization:` and 20k
spaces, 17s with `Proxy-Authorization:` (tried again at the inner
`authorization`). Same stall as the URL pattern: re.sub holds the GIL,
and the display service redacts everything it publishes.

The quote and the whitespace after it are now one optional unit,
`\s*(?:["\']\s*)?`, which matches the same strings with only one way to
split them. Output is identical to the old pattern on 300k fuzzed
inputs; 20k spaces now take ~1.6ms. A scan of all three redaction
patterns over prefix/run/suffix shapes finds none left that scales
superlinearly.

test/test_redaction.py pins exact output for quoted, tabbed, multi-line
and credential-less headers, and bounds header + 20k whitespace at 1s;
against the previous pattern those fail at 8-17s each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KMXdS2S4NXTJ8ET96GymhK

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-09-24 15:49:51 -04:00
ChuckandClaude Opus 5.5 c1ce0b7b04 fix(web): two api_v3 paths called names that no longer exist (#625)
The Pixlet editor stop route restarts the display after a SIGKILL with
_run_systemctl_command, which starlark.py never imported (since #554). The
Starlark device-location resolver fell back to _ensure_cache_manager, which
#609 deleted; the resolver already accepts no cache manager. Both raised
NameError on the rare path that reaches them. pyflakes finds no other
undefined names in src/ or web_interface/.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-09-24 15:49:29 -04:00
23 changed files with 887 additions and 276 deletions
+7
View File
@@ -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
+2 -1
View File
@@ -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`
+70
View File
@@ -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
View File
@@ -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
+12 -50
View File
@@ -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.
+76
View File
@@ -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
}
+2 -1
View File
@@ -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):
+43
View File
@@ -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)
+9 -5
View File
@@ -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")
+10 -2
View File
@@ -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)
+29
View File
@@ -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
View File
@@ -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:
+2 -1
View File
@@ -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
+118
View File
@@ -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
+110
View File
@@ -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
+3
View File
@@ -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.
+114 -12
View File
@@ -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():
+7 -3
View File
@@ -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
+168 -81
View File
@@ -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))
)
+1 -1
View File
@@ -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,