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>

.github/workflows/claude-code-review.yml

@@ -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 }}'

CHANGELOG.md

@@ -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,28 @@ 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.
 
-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 +156,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 +208,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 +240,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 +271,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 +282,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 +300,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 +354,7 @@ 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):
+### 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 +392,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`,

docs/SPORTS_UNIFICATION.md

@@ -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
 ```
 

first_time_install.sh

@@ -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"

scripts/install/configure_web_sudo.sh

@@ -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 "--------------------------------"

src/__init__.py

@@ -4,5 +4,5 @@ LEDMatrix Display System
 Core source package for the LED Matrix Display project.
 """
 
-__version__ = "3.4.0"
+__version__ = "3.5.0"
 

src/base_classes/data_sources.py

@@ -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):

test/test_data_sources.py

@@ -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
 

test/test_sudoers_is_validated.py

@@ -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