Compare commits

...
5 Commits
Author SHA1 Message Date
Claude 734adfba7d docs(changelog): record #602 in the 3.5.0 section
#602 merged into main after #601, the same way #600 merged during it, and
also touched no CHANGELOG. So the section was still a commit short of what
v3.5.0 will actually ship.

It gets its own "Installers" subsection rather than a line under "Small
fixes": a malformed drop-in in /etc/sudoers.d makes sudo refuse every command
for every user, which on a headless Pi is unrecoverable over SSH. That is not
a small fix, and someone reading the release notes to decide whether to update
should see it.

Written from the commit: what both installers did, what `visudo -c` now gates,
and the fixed /tmp path that mktemp replaced.

`scripts/check_release_version.py v3.5.0` still passes, and this branch is
rebased onto 967f3a05 so the section now covers every commit since v3.4.0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqzd6Nz2bQJp5K7DD5dS4X
2026-09-21 20:17:52 +00:00
Claude 58aff7b7f0 docs(changelog): record #600 in the 3.5.0 section
#600 merged into main while the release PR was open, so the 3.5.0 section
went in without it. Nothing in that PR touched the CHANGELOG, and no check
covers "everything merged since the last tag is written down", so tagging
v3.5.0 as main stands would ship the standings-endpoint fix undocumented.

The entry goes under Sports data, next to the other ESPN fetch changes, and
is written from the commit: what the old order did, why a college league's
200 defeated the 404 fallback, and what is now treated as routine.

No version change: 3.5.0 is not tagged yet, so this belongs in that section
rather than a new one. `scripts/check_release_version.py v3.5.0` still passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqzd6Nz2bQJp5K7DD5dS4X
2026-09-21 20:17:28 +00:00
claude[bot]andClaude 967f3a0567 fix(install): parse the sudoers rules before installing them (#602)
Both installers generated the ledmatrix_web rules and copied them straight
into /etc/sudoers.d without ever parsing them. Every rule is built from
`which` lookups, so an empty or surprising path produces a malformed
drop-in -- and a malformed file in /etc/sudoers.d makes sudo refuse every
command for every user. On a headless Pi that is unrecoverable over SSH.

first_time_install.sh now runs `visudo -c` on the generated file and, if it
does not parse, prints what visudo said and leaves the installed file
untouched rather than replacing it with a broken one. configure_web_sudo.sh
does the same before it offers the rules for confirmation.

first_time_install.sh also built the file at a fixed /tmp path as root;
mktemp now picks the name.

test/test_sudoers_is_validated.py renders the installer's own sudoers
heredoc and checks the result with visudo -- the check neither installer
had -- and asserts the install stays gated on it.


Claude-Session: https://claude.ai/code/session_01Dby94z9PV3zVM25fqGNXTt

Co-authored-by: Claude <noreply@anthropic.com>
2026-09-21 16:08:29 -04:00
claude[bot]andClaude Opus 5 21c8a54f68 chore: prepare the 3.5.0 release (#601)
* chore: prepare the 3.5.0 release

Turns the CHANGELOG's Unreleased section into `## 3.5.0` and bumps
`src.__version__`, the value plugin `ledmatrix_min_version` floors compare
against. No behaviour change; nothing outside the CHANGELOG, `src/__init__.py`
and one docs line is touched.

The staged entries are reshaped into the `### ` subsections every released
section already uses, and the "new modules a plugin may import via `src.*`"
block moves to the top as the plugin-facing summary, the same shape as 3.4.0.
Its floor, written as "the release that ships this" while it was staged, is now
3.5.0, and `docs/SPORTS_UNIFICATION.md` says 3.5.0 for `sports_helpers.py`
instead of "(unreleased)".

Four merged changes had never been written down. They are added under the
subsection each belongs to, from the commits and their measurements:

- the idle back-off clamped to the next kickoff (#599)
- concurrent ESPN date chunks (#596)
- the three web routes that consulted plugin manifests before anything had
  discovered plugins, one of which wrote a plugin API key to config.json in
  plain text (#594)
- the cache permission fix and its systemd unit changes (#593), which get
  their own subsection

No tag and no release: `scripts/check_release_version.py v3.5.0` passes, so
tagging is a separate, deliberate step.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqzd6Nz2bQJp5K7DD5dS4X

* ci: let Claude Code Review run on PRs the Claude app opens

The review action refuses a workflow whose actor is a GitHub App unless the
app is named in `allowed_bots`, which this workflow never set:

  Actor is a GitHub App: claude[bot]
  Actor type: Bot
  Action failed with error: Workflow initiated by non-human actor: claude
  (type: Bot). Add bot to allowed_bots list or use '*' to allow all bots.

It aborts about two seconds in, before the diff is read, so the check is red
on every such PR and re-running cannot help: the actor does not change. Until
now no PR here had a bot author, so nothing tripped it.

`'claude'` rather than `'*'`: the action lowercases each entry and strips a
trailing `[bot]` before comparing it to the actor
(`isAllowedBot` in `src/github/validation/actor.ts`), so this admits
`claude[bot]` and no other app. `'*'` would admit any app that can trigger a
workflow here, with a prompt it controls — the action's own docs warn about
that on public repositories, and this one is public.

The write-permission check already allowed the app; `checkHumanActor` was the
only gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rqzd6Nz2bQJp5K7DD5dS4X

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-09-21 16:08:05 -04:00
ChuckandClaude Opus 5 cf02538d2e fix(sports): ask the endpoint the league actually publishes for standings (#600)
* fix(sports): ask the endpoint the league actually publishes for standings

ESPNDataSource.fetch_standings tried /standings first regardless of league
and fell back to /rankings only on a 404. College leagues answer /standings
with a 200 that carries no poll, so the fallback never fired and the poll
came back empty every time. Nothing failed; the rank badge simply never
appeared, and anything keyed off rankings quietly did nothing.

Endpoints are now ordered by whether the league publishes a poll, a 200
that lacks the key counts as a miss so a league answering both still ends
up with whichever one carries the poll, and only a 404 is treated as
routine -- it is how a league says it has none. A connection error, a
timeout or an unparseable body is logged as an error again.

This is the implementation the football, baseball and hockey boards already
ship; core was the last copy still on the old one. Verified against live
ESPN: mens-college-basketball returns a populated rankings key where it
previously returned nothing, and nba still resolves from /standings alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

* fix(standings): stop the endpoint handler from swallowing its own bugs

Addresses both CodeRabbit findings on #600.

The handler caught `Exception`, so an AttributeError or TypeError raised
while *inspecting* the payload was indistinguishable from an endpoint that
failed. The loop would move on and, if the other endpoint had nothing
either, return {} -- silently dropping rankings for a league that has them.
That is the precise failure this function was written to fix, so the
handler was able to reintroduce it.

Only the request is guarded now. `requests.RequestException` covers the
transport failures and `ValueError` covers a body that will not parse;
payload inspection happens after the handler, where a bug surfaces instead
of being logged as a missing poll. A non-dict payload is treated as a miss
explicitly rather than by tripping over `.get`.

Tests: the fallback paths had no coverage -- the old single-endpoint code
would have passed the suite unchanged. Added order assertions for both
league kinds, a 200-without-a-poll fall-through, 404 and non-404 recovery,
a non-object payload, and a guard proving a bug is no longer swallowed.

`test_fetch_standings_returns_empty_on_error` faked a transport failure
with a bare `Exception`, which only passed because the handler caught
everything. It now raises ConnectionError, which is what actually happens.

Verified by mutation: restoring standings-first fails 5 tests, restoring
the catch-all fails the bug-not-swallowed guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-21 16:07:51 -04:00
9 changed files with 458 additions and 76 deletions
+6
View File
@@ -36,6 +36,12 @@ jobs:
uses: anthropics/claude-code-action@v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
# Review PRs opened by the Claude GitHub App. Without this the action
# aborts before reading the diff ("Workflow initiated by non-human
# actor"), so every such PR shows this check red. Named rather than
# '*': the allow-list is matched against the triggering actor, so
# this admits claude[bot] alone and no other app.
allowed_bots: 'claude'
plugin_marketplaces: 'https://github.com/anthropics/claude-code.git'
plugins: 'code-review@claude-code-plugins'
prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}'
+123 -35
View File
@@ -19,7 +19,32 @@ accepts both, but the store flags the old spelling as deprecated
## Unreleased
Config saves and plugin config preparation:
## 3.5.0
New modules a plugin may import via `src.*` (floor on 3.5.0):
- `src/common/sports_helpers.py` — the helpers the scoreboards' `sports.py`
carry byte-identical copies of: `clamp_window`, `clamp_seconds`,
`logo_needs_refresh`, `spread_weighted_order` (+ `MIN_WINDOW_DAYS`,
`MAX_WINDOW_DAYS`), and `SportsHelpersMixin` with `_mode_customization`,
`_setting_int`, `_reset_dwell_on_reentry`, `_next_switch_index`,
`_spread_weighted_order`, `_odds_color`, `_upcoming_date_and_time_text` under
the plugins' names and signatures, plus the `_favorite_key` override point.
Constructor-free; keeps lazy state on its host (see the module docstring,
which also gives the host contract).
A new module rather than more methods on `sports_shared`: a plugin that
deletes a copy and leans on an older module having grown the method fails at
runtime with `AttributeError`, which no load-time check sees, while a missing
module fails at load. Nothing in core uses it yet.
- `test/test_common_is_hardware_free.py` — `src/common` must import without
`rgbmatrix` and never import `src.base_classes`, `src.display_manager` or
`src.plugin_system` at module level.
- `src/common/espn_dates.py` — `fetch_espn_scoreboard`,
`fetch_espn_date_chunks`, `espn_date_chunks`, `clamp_espn_limit`,
`ESPN_MAX_LIMIT`: fetch an ESPN scoreboard date range now that ESPN rejects
ranges (see Sports data below). Plugins bundle a copy of it.
### Config saves and plugin config preparation
- A JSON `POST /api/v3/config/main` changes only the keys it sends. The MQTT
bridge's brightness slider used to turn off `disable_hardware_pulsing`,
@@ -65,31 +90,7 @@ Config saves and plugin config preparation:
with any core config section are flagged: the last private copies of the
core-key list now use `src/core_config_keys.py`.
New module a plugin may import via `src.*` (floor on the release that ships
this):
- `src/common/sports_helpers.py` — the helpers the scoreboards' `sports.py`
carry byte-identical copies of: `clamp_window`, `clamp_seconds`,
`logo_needs_refresh`, `spread_weighted_order` (+ `MIN_WINDOW_DAYS`,
`MAX_WINDOW_DAYS`), and `SportsHelpersMixin` with `_mode_customization`,
`_setting_int`, `_reset_dwell_on_reentry`, `_next_switch_index`,
`_spread_weighted_order`, `_odds_color`, `_upcoming_date_and_time_text` under
the plugins' names and signatures, plus the `_favorite_key` override point.
Constructor-free; keeps lazy state on its host (see the module docstring,
which also gives the host contract).
A new module rather than more methods on `sports_shared`: a plugin that
deletes a copy and leans on an older module having grown the method fails at
runtime with `AttributeError`, which no load-time check sees, while a missing
module fails at load. Nothing in core uses it yet.
- `test/test_common_is_hardware_free.py` — `src/common` must import without
`rgbmatrix` and never import `src.base_classes`, `src.display_manager` or
`src.plugin_system` at module level.
- `src/common/espn_dates.py` — `fetch_espn_scoreboard`,
`fetch_espn_date_chunks`, `espn_date_chunks`, `clamp_espn_limit`,
`ESPN_MAX_LIMIT`: fetch an ESPN scoreboard date range now that ESPN rejects
ranges (see Sports data below). Plugins bundle a copy of it.
Sports data:
### Sports data
- Since 2026-09-15 ESPN answers `dates=YYYYMMDD-YYYYMMDD` scoreboard queries
with `400 Bad Request` for every sport, so season schedules, the weeks window
@@ -104,8 +105,41 @@ Sports data:
- `BackgroundDataService.handles_espn_date_ranges` is `True`. Plugins check it
to decide whether to submit a season range to the service or fetch it
themselves on an older core.
- A league with no live games no longer backs its poll off past the next
kickoff. The escalation counted empty looks and nothing else, so a league
three hours before kickoff was indistinguishable from one out of season and
both reached `live_idle_max_interval`: measured gaps of up to 928 seconds,
and a rig that sat for a quarter of an hour with eight NFL games in progress
without noticing any of them. The wait is now clamped so it cannot run past
the earliest start still ahead, which the live fetch already downloads, so
it costs no extra request. Just after a kickoff the live cadence is held for
a grace window, because a provider that has not yet flipped the status would
otherwise read as another empty check and escalate the back-off again.
- ESPN date chunks are fetched six at a time (`ESPN_CHUNK_WORKERS`) in two
passes: months and edge days first, then the days of any month that came
back at the cap. A cold college-baseball season is about 130 requests, and
they went out one at a time; March and April measured on a Pi 4 (63
requests, 3101 events) went from 11.2s to 1.6s. Merged events still follow
`espn_date_chunks` order, so the payload does not depend on which request
won the race, and a capped month's payload is dropped before its days are
fetched, which keeps the peak memory of a four-capped-month fetch to about
16 MB over the sequential path rather than 43 MB — `docs/LOW_MEMORY_BOARDS.md`
puts a 1 GB Pi 3B+ at under 200 MB of headroom.
- `ESPNDataSource.fetch_standings` asks each league the endpoint that league
actually publishes. It tried `/standings` first whatever the league and fell
back to `/rankings` only on a 404, but college leagues answer `/standings`
with a 200 that carries no poll, so the fallback never fired: the rank badge
simply never appeared and anything keyed off rankings quietly did nothing.
Endpoints are now ordered by whether the league publishes a poll, and a 200
that lacks the key counts as a miss, so a league answering both still ends up
with whichever carries the poll. Only a 404 is routine — that is how a league
says it has none; a connection error, a timeout or an unparseable body is
logged as an error again, and a bug raised while inspecting the payload is no
longer swallowed as a missing poll. This is the implementation the football,
baseball and hockey boards already ship; core was the last copy on the old
one.
Scrolling:
### Scrolling
- **Scoreboard scroll speed no longer changes with the General tab's "Scroll
Frame Rate" (`target_fps`).** Scoreboards on `src.common.sports_scroll`
@@ -135,7 +169,7 @@ Scrolling:
`scroll_delay` are described as the speed clamp they are rather than frame
stepping. Scoreboard `scroll_delay` is documented as ignored for pacing.
Web interface:
### Web interface
- The plugin settings form honours `"x-display": "hidden"` in config schemas:
the property gets no control at any depth (top level, nested objects, array
@@ -187,8 +221,19 @@ Web interface:
request that gets no HTTP answer (e.g. the web service restarting mid-run) is
re-sent with backoff instead of being counted as failed and skipped — that is
how a disabled plugin with an update waiting was silently left out.
- Three routes consulted the web process's plugin manifests without
discovering plugins first, so they misbehaved from every `ledmatrix-web`
restart until something else ran a discovery — in practice until someone
opened the dashboard, measured at over three minutes on one rig.
`POST /display/on-demand/start` and `POST /plugins/toggle` answered 404
"Plugin not found", and `POST /config/main` did not recognise a plugin
section, so it skipped secret separation and wrote the plugin's API key to
`config.json` in plain text instead of `config_secrets.json`. The routes now
discover when nothing has been discovered yet, and rescan once when a
specific plugin id (or, for on-demand by mode, a mode) is not found, so a
plugin installed since the last scan is found too.
Security (request paths and inline handlers, siblings of #561):
### Security (request paths and inline handlers, siblings of #561)
- `POST /api/v3/plugins/assets/upload`, `GET .../assets/list` and
`POST .../assets/delete` validate `plugin_id` with `src/common/path_safety`
@@ -208,7 +253,7 @@ Security (request paths and inline handlers, siblings of #561):
- The uploaded-images list escapes each file's original name, path and ids; a
name like `<img src=x onerror=...>.png` was inserted as markup.
Display hardware settings the library refuses:
### Display hardware settings the library refuses
- The rgbmatrix library answers several settings with no matrix or `abort()`
rather than an error, on every board, so the display service crash-looped
@@ -239,7 +284,7 @@ Display hardware settings the library refuses:
values as the defaults; the "code default" values they listed never apply,
because config migration fills missing keys from the template.
Plugin system:
### Plugin system
- A plugin no longer starts with a schema warning and a degraded flag because
config.json still holds a boolean where its schema now has an object with an
@@ -250,7 +295,7 @@ Plugin system:
is written at load; the next save of that plugin's settings stores the object.
Other type mismatches still warn.
Core:
### Core
- `ConfigManager.load_config()` no longer raises on a host without the POSIX
ownership APIs. The self-heal that chgrp's `config_secrets.json` to the
@@ -268,7 +313,37 @@ Core:
ownership step is now skipped where `os.chown` is missing. No behaviour
change on the Pi.
Automatic updates and Update Code:
### Cache permissions
- The web interface can read what the display service caches again.
`ledmatrix-web.service` carried `CacheDirectory=ledmatrix`, and systemd
re-owns `/var/cache/ledmatrix` and its contents to the unit's `User=`
whenever the directory's owner differs, which erased the `root:ledmatrix`
setgid layout the installers set up: every file the root display service
wrote afterwards was `root:root` 0660 and unreadable by the web interface
(392 unreadable files on one rig, with display status, on-demand state and
plugin health empty). Since #547 the web unit is rendered from its template
on every install, so every fresh install hit this.
`DiskCache.set` now gives each file the directory's group (when that
directory is group-writable) and 0660 on the open descriptor before the
rename, independent of setgid, which also closes a window where a fresh
file was visible as mkstemp's 0600. `DiskCache.share_existing_files`
repairs files an older version left behind, once per process, through
`O_NOFOLLOW` descriptors, skipping hard links and other users' files.
Existing installs only ever receive `git pull`, so that repair is the fix
for them; new installs also drop `CacheDirectory=` and
`CacheDirectoryMode=` from the web unit.
- `install_web_service.sh` replaces an existing cache directory's group
whenever the installing user is not in it. It used to replace only root's,
so a `root:ledmatrix` directory belonging to a user outside that group was
left alone and everything root wrote there stayed unreadable.
- `/display/on-demand/status` and the current-display status read the display
service's keys with `memory_ttl=0`, as every other cross-process reader
already does. They served the first copy the web process had read for the
full 120s `max_age`, so on-demand reported "active" for over 100 seconds
after the file on disk said "idle".
### Automatic updates and Update Code
- An update that changes `web_interface/requirements.txt` is no longer rolled
back on every auto-updating device. `safe_pip_install.sh` allowed only the
@@ -292,7 +367,20 @@ Automatic updates and Update Code:
budget, so a rollback finishes inside the unit's 30-minute limit instead of
being killed mid-way.
Small fixes (update-all, plugin system settings, scripts):
### Installers
- The generated `ledmatrix_web` sudoers rules are parsed before they are
installed. Both installers built the drop-in from `which` lookups and copied
it into `/etc/sudoers.d` without ever checking it, and a malformed file there
makes sudo refuse every command for every user — on a headless Pi, that is
unrecoverable over SSH. `first_time_install.sh` now runs `visudo -c` on the
generated file and, if it does not parse, prints what visudo said and leaves
the installed file untouched instead of replacing it with a broken one;
`configure_web_sudo.sh` does the same before offering the rules for
confirmation. `first_time_install.sh` also built that file at a fixed `/tmp`
path as root; `mktemp` now picks the name.
### Small fixes (update-all, plugin system settings, scripts)
- **Check & Update All** counts a plugin that had nothing to update as
"already up to date" instead of "updated". ZIP-installed monorepo plugins
@@ -330,7 +418,7 @@ Small fixes (update-all, plugin system settings, scripts):
check `web_interface/blueprints/api_v3/`, which became a package, instead of
reporting `api_v3.py` as missing.
Docs and developer tools:
### Docs and developer tools
- `docs/REST_API_REFERENCE.md` rechecked against every handler: request
fields that made documented calls fail (`repo_url`, `action_id`/`params`,
+1 -1
View File
@@ -81,7 +81,7 @@ src/common/
sports_scroll.py SportsScrollDisplay / …Manager — scroll orchestration
(content building stays in the plugins)
sports_helpers.py clamp/logo/rotation free functions + SportsHelpersMixin
(unreleased) — the helpers byte-identical in the
(3.5.0) — the helpers byte-identical in the
plugins' sports.py, and the _favorite_key seam
```
+34 -10
View File
@@ -1504,6 +1504,9 @@ echo "------------------------------------------------"
# Create sudoers configuration for the web interface
echo "Creating sudoers configuration..."
SUDOERS_FILE="/etc/sudoers.d/ledmatrix_web"
# A predictable name in a world-writable directory is a symlink target;
# root writes the rules here, so let mktemp pick the name.
SUDOERS_TMP=$(mktemp "${TMPDIR:-/tmp}/ledmatrix_web_sudoers.XXXXXX")
# Get command paths
PYTHON_PATH=$(which python3)
@@ -1514,7 +1517,7 @@ BASH_PATH=$(which bash)
JOURNALCTL_PATH=$(which journalctl 2>/dev/null || true)
# Create sudoers content
cat > /tmp/ledmatrix_web_sudoers << EOF
cat > "$SUDOERS_TMP" << EOF
# LED Matrix Web Interface passwordless sudo configuration
# This allows the web interface user to run specific commands without a password
@@ -1541,7 +1544,7 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/
$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_pip_install.sh *
EOF
if [ -n "$JOURNALCTL_PATH" ]; then
cat >> /tmp/ledmatrix_web_sudoers << EOF
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
@@ -1555,17 +1558,38 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *
EOF
fi
if [ -f "$SUDOERS_FILE" ] && cmp -s /tmp/ledmatrix_web_sudoers "$SUDOERS_FILE"; then
echo "Sudoers configuration already up to date"
rm /tmp/ledmatrix_web_sudoers
# 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 ! visudo -c -f "$SUDOERS_TMP" >/dev/null 2>&1; then
SUDOERS_VALID=0
echo "⚠ The generated sudoers rules did not parse:" >&2
visudo -c -f "$SUDOERS_TMP" >&2 || true
echo "⚠ Leaving $SUDOERS_FILE unchanged. The web interface cannot control" >&2
echo " the display service until this is fixed." >&2
fi
else
echo "Installing/updating sudoers configuration..."
cp /tmp/ledmatrix_web_sudoers "$SUDOERS_FILE"
chmod 440 "$SUDOERS_FILE"
rm /tmp/ledmatrix_web_sudoers
echo "⚠ visudo not found; installing the sudoers rules unvalidated"
fi
echo "✓ Passwordless sudo access configured"
if [ "$SUDOERS_VALID" = "0" ]; then
rm -f "$SUDOERS_TMP"
elif [ -f "$SUDOERS_FILE" ] && cmp -s "$SUDOERS_TMP" "$SUDOERS_FILE"; then
echo "Sudoers configuration already up to date"
rm -f "$SUDOERS_TMP"
else
echo "Installing/updating sudoers configuration..."
cp "$SUDOERS_TMP" "$SUDOERS_FILE"
chmod 440 "$SUDOERS_FILE"
rm -f "$SUDOERS_TMP"
fi
if [ "$SUDOERS_VALID" = "1" ]; then
echo "✓ Passwordless sudo access configured"
fi
echo ""
CURRENT_STEP="Configure WiFi management permissions"
+13
View File
@@ -130,6 +130,19 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_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.
if command -v visudo >/dev/null 2>&1; then
if ! visudo -c -f "$TEMP_SUDOERS" >/dev/null 2>&1; then
echo ""
echo "✗ The generated sudoers rules did not parse:" >&2
visudo -c -f "$TEMP_SUDOERS" >&2 || true
echo "Nothing was changed." >&2
rm -f "$TEMP_SUDOERS"
exit 1
fi
fi
echo ""
echo "Generated sudoers configuration:"
echo "--------------------------------"
+1 -1
View File
@@ -4,5 +4,5 @@ LEDMatrix Display System
Core source package for the LED Matrix Display project.
"""
__version__ = "3.4.0"
__version__ = "3.5.0"
+61 -28
View File
@@ -114,35 +114,68 @@ class ESPNDataSource(DataSource):
return []
def fetch_standings(self, sport: str, league: str) -> Dict:
"""Fetch standings from ESPN API."""
# Try standings endpoint first (for professional leagues like NFL, NBA, etc.)
try:
url = f"{self.base_url}/{sport}/{league}/standings"
response = self.session.get(url, headers=self.get_headers(), timeout=15)
response.raise_for_status()
data = response.json()
self.logger.debug(f"Fetched standings for {sport}/{league}")
"""Fetch standings, or the poll for leagues that have one.
Order matters and used to be wrong. College leagues publish a poll at
/rankings and a records table at /standings; professional leagues have
only /standings. The old code tried /standings first and fell back to
/rankings only on a 404 -- but college /standings answers 200, so the
fallback never fired and college rankings came back empty forever.
Nothing failed; the AP rank badge simply never appeared, and anything
else keyed off rankings quietly did nothing.
A 200 that lacks the key is treated as a miss, so a league answering
both endpoints still ends up with whichever one actually carries a poll.
"""
league_name = (league or "").lower()
wants_poll = "college" in league_name or "ncaa" in league_name
endpoints = ["rankings", "standings"] if wants_poll else ["standings", "rankings"]
for endpoint in endpoints:
url = f"{self.base_url}/{sport}/{league}/{endpoint}"
# Only the request is guarded. Inspecting the payload happens
# below, outside the handler, so that a bug in this method cannot
# be mistaken for an endpoint that failed -- that mistake would
# silently drop rankings for a league that has them, which is the
# exact failure this function was written to fix.
try:
response = self.session.get(
url, headers=self.get_headers(), timeout=15
)
response.raise_for_status()
data = response.json()
except (requests.RequestException, ValueError) as e:
status = getattr(getattr(e, "response", None), "status_code", None)
# Only a 404 is routine -- it is how a league says "no poll
# here". Everything else is worth an error, and `status is
# None` covers the ones that matter most: ConnectionError,
# Timeout, a body that would not parse. Silencing those left a
# board that could not reach ESPN with one debug line, and the
# ranked filter running on an empty table.
if status != 404:
self.logger.error(
f"Error fetching {endpoint} from ESPN for "
f"{sport}/{league}: {e}"
)
continue
if not isinstance(data, dict):
# A list or a bare string is not something the callers can
# read. Treat it as a miss so the other endpoint still gets a
# turn, but say so -- this means ESPN changed shape.
self.logger.error(
f"Unexpected {endpoint} payload for {sport}/{league}: "
f"got {type(data).__name__}, expected an object"
)
continue
if endpoint == "rankings" and not data.get("rankings"):
continue
self.logger.debug(f"Fetched {endpoint} for {sport}/{league}")
return data
except Exception as e:
# If standings doesn't exist, try rankings (for college sports)
if hasattr(e, 'response') and hasattr(e.response, 'status_code') and e.response.status_code == 404:
try:
url = f"{self.base_url}/{sport}/{league}/rankings"
response = self.session.get(url, headers=self.get_headers(), timeout=15)
response.raise_for_status()
data = response.json()
self.logger.debug(f"Fetched rankings for {sport}/{league}")
return data
except Exception:
# Both endpoints failed - standings/rankings may not be available for this sport/league
self.logger.debug(f"Standings/rankings not available for {sport}/{league} from ESPN API")
return {}
else:
# Non-404 error - log at debug level since standings are optional
self.logger.debug(f"Error fetching standings from ESPN for {sport}/{league}: {e}")
return {}
self.logger.debug(
f"Standings/rankings not available for {sport}/{league} from ESPN API"
)
return {}
class MLBAPIDataSource(DataSource):
+99 -1
View File
@@ -92,10 +92,108 @@ class TestESPNDataSource:
assert result == payload
def test_fetch_standings_returns_empty_on_error(self):
with patch.object(self.source.session, "get", side_effect=Exception("error")):
# A transport failure is a RequestException, not a bare Exception.
# The old stand-in passed only because the handler caught everything,
# including bugs in the method under test.
with patch.object(self.source.session, "get",
side_effect=requests.ConnectionError("error")):
result = self.source.fetch_standings("football", "nfl")
assert result == {}
# ------------------------------------------------------------------
# fetch_standings endpoint selection
#
# College leagues publish a poll at /rankings and a records table at
# /standings; professional leagues have only /standings. Probing them in
# the wrong order still returns 200 -- just without a poll in it -- so
# nothing failed and the rank badge simply never appeared. Order is the
# behaviour here, so these tests assert it directly.
# ------------------------------------------------------------------
@staticmethod
def _requested_endpoints(mock_get):
"""The endpoint names probed, in the order they were requested."""
return [call.args[0].rsplit("/", 1)[-1] for call in mock_get.call_args_list]
def test_professional_league_asks_standings_first(self):
payload = {"standings": []}
with patch.object(self.source.session, "get",
return_value=_mock_response(payload)) as mock_get:
result = self.source.fetch_standings("football", "nfl")
assert result == payload
assert self._requested_endpoints(mock_get) == ["standings"]
def test_college_league_asks_rankings_first(self):
poll = {"rankings": [{"name": "AP Top 25"}]}
with patch.object(self.source.session, "get",
return_value=_mock_response(poll)) as mock_get:
result = self.source.fetch_standings("football", "college-football")
assert result == poll
assert self._requested_endpoints(mock_get) == ["rankings"]
def test_rankings_200_without_a_poll_falls_through_to_standings(self):
"""A 200 is not the same as an answer.
This is the case the old code could not see: the endpoint responded,
so nothing raised, but the body carried no poll.
"""
empty_poll = _mock_response({"rankings": []})
table = _mock_response({"standings": [{"entries": []}]})
with patch.object(self.source.session, "get",
side_effect=[empty_poll, table]) as mock_get:
result = self.source.fetch_standings(
"basketball", "mens-college-basketball")
assert result == {"standings": [{"entries": []}]}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_404_on_the_first_endpoint_falls_through_quietly(self):
missing = _mock_response({}, status_code=404)
table = _mock_response({"standings": []})
with patch.object(self.source.session, "get",
side_effect=[missing, table]) as mock_get:
result = self.source.fetch_standings("baseball", "college-baseball")
assert result == {"standings": []}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_recovers_from_a_non_404_failure_on_the_first_endpoint(self):
table = _mock_response({"standings": [{"entries": []}]})
with patch.object(self.source.session, "get",
side_effect=[requests.ConnectionError("reset"), table]) as mock_get:
result = self.source.fetch_standings("football", "college-football")
assert result == {"standings": [{"entries": []}]}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_both_endpoints_failing_returns_empty(self):
with patch.object(self.source.session, "get",
side_effect=requests.ConnectionError("down")) as mock_get:
result = self.source.fetch_standings("football", "nfl")
assert result == {}
assert self._requested_endpoints(mock_get) == ["standings", "rankings"]
def test_a_non_object_payload_is_treated_as_a_miss(self):
odd = _mock_response(["not", "an", "object"])
table = _mock_response({"standings": []})
with patch.object(self.source.session, "get",
side_effect=[odd, table]) as mock_get:
result = self.source.fetch_standings("football", "college-football")
assert result == {"standings": []}
assert self._requested_endpoints(mock_get) == ["rankings", "standings"]
def test_a_bug_in_this_method_is_not_swallowed_as_a_failed_endpoint(self):
"""The guard for the narrowed handler.
An error raised while reading the payload used to be caught by the
endpoint handler and reported as 'no poll here', which would silently
drop rankings for a league that has them. It must surface instead.
"""
boom = Mock(spec=requests.Response)
boom.status_code = 200
boom.raise_for_status = Mock()
boom.json.side_effect = TypeError("a bug, not a network failure")
with patch.object(self.source.session, "get", return_value=boom):
with pytest.raises(TypeError):
self.source.fetch_standings("football", "nfl")
def test_base_url_set_correctly(self):
assert "espn.com" in self.source.base_url
+120
View File
@@ -0,0 +1,120 @@
"""The generated sudoers rules must parse before they reach /etc/sudoers.d.
A malformed drop-in there makes sudo refuse every command for every user. On a
headless Pi that is unrecoverable without pulling the SD card, so both
installers run `visudo -c` on the file they generated before installing it.
The render test also gives us the check neither installer had: that the rules
they actually emit are valid sudoers syntax on a real Linux box.
"""
import os
import shutil
import subprocess
import sys
import tempfile
import pytest
REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
FIRST_TIME = os.path.join(REPO_ROOT, "first_time_install.sh")
CONFIGURE = os.path.join(REPO_ROOT, "scripts", "install", "configure_web_sudo.sh")
VISUDO = shutil.which("visudo") or (
"/usr/sbin/visudo" if os.path.exists("/usr/sbin/visudo") else None
)
def _read(path):
with open(path, "r", encoding="utf-8") as handle:
return handle.read()
def test_first_time_install_validates_before_installing():
body = _read(FIRST_TIME)
assert 'visudo -c -f "$SUDOERS_TMP"' in body
install = body.index('cp "$SUDOERS_TMP" "$SUDOERS_FILE"')
validate = body.index('visudo -c -f "$SUDOERS_TMP"')
assert validate < install, "the rules must be checked before they are installed"
def test_the_install_is_gated_on_the_check():
"""Checking and then installing anyway would be worse than not checking."""
body = _read(FIRST_TIME)
assert "SUDOERS_VALID=0" in body
gate = body.index('if [ "$SUDOERS_VALID" = "0" ]')
install = body.index('cp "$SUDOERS_TMP" "$SUDOERS_FILE"')
assert gate < install
def test_first_time_install_does_not_use_a_predictable_temp_file():
body = _read(FIRST_TIME)
assert "mktemp" in body
assert "> /tmp/ledmatrix_web_sudoers" not in body
assert ">> /tmp/ledmatrix_web_sudoers" not in body
def test_configure_web_sudo_validates_before_installing():
body = _read(CONFIGURE)
assert 'visudo -c -f "$TEMP_SUDOERS"' in body
install = body.index('cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web')
validate = body.index('visudo -c -f "$TEMP_SUDOERS"')
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."""
body = _read(FIRST_TIME)
start = body.index("# Create sudoers content")
end = body.index("# Never install rules we have not parsed.")
block = body[start:end]
out = os.path.join(project_root, "rendered")
script = "\n".join(
[
"set -euo pipefail",
f"ACTUAL_USER={user}",
f"PROJECT_ROOT_DIR={project_root}",
'SUDOERS_TMP="$(mktemp)"',
"PYTHON_PATH=$(which python3)",
"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}',
]
)
subprocess.run(["bash", "-c", script], check=True)
return out
@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():
with tempfile.TemporaryDirectory() as tmp:
rendered = _render_first_time_sudoers(tmp, "ledmatrix")
os.chmod(rendered, 0o440)
result = subprocess.run(
[VISUDO, "-c", "-f", rendered], capture_output=True, text=True
)
assert result.returncode == 0, result.stdout + 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_a_broken_rule_is_caught_rather_than_installed():
"""The guard is only worth having if visudo rejects what it should."""
with tempfile.TemporaryDirectory() as tmp:
rendered = _render_first_time_sudoers(tmp, "ledmatrix")
with open(rendered, "r", encoding="utf-8") as handle:
good = handle.read()
broken = os.path.join(tmp, "broken")
with open(broken, "w", encoding="utf-8") as handle:
# An empty command path is what an unset $BASH_PATH would produce.
handle.write(good + "\nledmatrix ALL=(ALL) NOPASSWD:\n")
os.chmod(broken, 0o440)
result = subprocess.run(
[VISUDO, "-c", "-f", broken], capture_output=True, text=True
)
assert result.returncode != 0