Merge branch 'main' into claude/cache-stale-check

CHANGELOG.md

@@ -120,6 +120,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
 

docs/SCROLL_PERFORMANCE.md

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

src/redaction.py

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

test/test_redaction.py

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

test/web_interface/test_api_v3_names_resolve.py

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

web_interface/blueprints/api_v3/__init__.py

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

web_interface/blueprints/api_v3/starlark.py

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