fix(reports): asset-token format → fixed-length sig, no period collision

CI flake during 0.9.0's first publish run surfaced a real
production bug, not a test issue.

`mint_asset_token` was encoding `<payload> b"." <32-byte-hmac>`
and `verify_asset_token` recovered the two halves via
`decoded.rsplit(b".", 1)`. That works ONLY when the random HMAC
output contains no ASCII period byte (0x2E). With a 32-byte
SHA-256 output the per-token collision rate is

    P("`.` somewhere in sig") = 1 − (255/256)**32 ≈ 11.8 %

i.e. roughly one in nine asset-token verifications would silently
fail and the user would get 401 → reload → fresh token → eventually
work. Locally we got lucky for weeks; CI hit the bad bytes today.

Fix: drop the printable separator entirely. SHA-256 output is
always exactly `hashlib.sha256().digest_size` (32) bytes, so
`payload, sig = decoded[:-_SIG_LEN], decoded[-_SIG_LEN:]` is
unambiguous. The `_SIG_LEN` constant is derived from `hashlib`
rather than literal-32 so a hypothetical future SHA-512 swap
doesn't introduce a fresh off-by-31 bug.

Token format change is wire-incompatible. Tokens minted with the
old format (`.` separator) fail verify after the fix. Acceptable
because:
- TTL is 1 hour by default → at most one hour of in-flight tokens
  hit a 401, the user reloads, the freshly-minted post-fix token
  works.
- Asset tokens never persist server-side (no DB row, no cookie
  outside the iframe URL the page just minted), so there's no
  upgrade-path migration to write.
- Anyone bookmarking an iframe URL is already living dangerously
  vis-à-vis the 1-hour TTL.

Regression test (`test_round_trip_when_signature_contains_period_byte`)
sweeps `ttl_seconds` until the HMAC output contains a `.` byte
(typically within ~10 attempts), then mints + verifies. Pre-fix
that test fails deterministically; post-fix it passes
deterministically. 107 / 107 reports tests green locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: raffelino

backend/src/reports/asset_tokens.py

@@ -52,14 +52,26 @@ def _b64url_decode(s: str) -> bytes:
     return base64.urlsafe_b64decode((s + pad).encode("ascii"))
 
 
+# SHA-256 output is always 32 bytes — the suffix length is fixed,
+# so we don't need a printable separator between payload and
+# signature. The previous implementation used `.` as a separator
+# and `decoded.rsplit(b".", 1)` to recover payload + sig. ASCII
+# `0x2E` (period) is a valid byte in a random HMAC, so the sig
+# itself contained one with ~12% probability (1 − (255/256)**32),
+# at which point `rsplit` split at the wrong byte, the payload
+# parse failed, and verify returned False — silently. Fixed-length
+# slicing has no such collision class.
+_SIG_LEN = hashlib.sha256().digest_size  # 32
+
+
 def mint_asset_token(report_id: int, ttl_seconds: int = 3600) -> str:
     """Create a token that grants read access to report ``report_id``
     for the next ``ttl_seconds`` seconds.
     """
     expiry = int(time.time()) + ttl_seconds
     payload = f"{report_id}:{expiry}".encode("ascii")
     sig = hmac.new(_key(), payload, hashlib.sha256).digest()
-    return _b64url_encode(payload + b"." + sig)
+    return _b64url_encode(payload + sig)
 
 
 def verify_asset_token(token: str, report_id: int) -> bool:
@@ -74,11 +86,12 @@ def verify_asset_token(token: str, report_id: int) -> bool:
     except (ValueError, binascii.Error):
         return False
 
-    # Signature is the LAST 32 bytes (SHA-256 output). The payload is
-    # everything before the final `.` separator.
-    if b"." not in decoded:
+    if len(decoded) <= _SIG_LEN:
+        # Need at least one payload byte — `:` separator + at least
+        # one digit per side — but the conservative lower bound is
+        # "more than the signature itself".
         return False
-    payload, sig = decoded.rsplit(b".", 1)
+    payload, sig = decoded[:-_SIG_LEN], decoded[-_SIG_LEN:]
     expected = hmac.new(_key(), payload, hashlib.sha256).digest()
     if not hmac.compare_digest(sig, expected):
         return False

backend/tests/reports/test_asset_tokens.py

@@ -59,7 +59,61 @@ def test_rejects_tampered_payload(self):
     def test_rejects_empty_or_garbage(self):
         assert verify_asset_token("", report_id=1) is False
         assert verify_asset_token("not-base64-or-anything-meaningful!", report_id=1) is False
-        assert verify_asset_token("aGVsbG8=", report_id=1) is False  # valid b64, no '.'
+        assert verify_asset_token("aGVsbG8=", report_id=1) is False  # valid b64, too short
+
+    def test_round_trip_when_signature_contains_period_byte(self):
+        """Regression: previously the encoded form was `<payload> .
+        <sha256-sig>` and verify used `decoded.rsplit(b'.', 1)`.
+        Random SHA-256 output contains the ASCII period byte (0x2E)
+        with ~12 % probability per token (1 − (255/256)**32), at which
+        point rsplit landed in the *signature*, the payload-parse
+        choked on the trailing bytes, and verify returned False.
+        That manifested as a CI flake during 0.9.0's first release
+        run while local tests passed for weeks.
+
+        Force-mint a token whose HMAC contains a `.` byte by sweeping
+        the expiry second-by-second until we hit one. With 32 sig
+        bytes and ~12 % per-token rate we expect a hit within ~10
+        attempts; the loop guards against a 1-in-a-million miss with
+        a generous upper bound."""
+        from src.reports.asset_tokens import _b64url_decode, _key, _SIG_LEN
+        import hmac as _hmac, hashlib as _hashlib
+
+        seed_payload = None
+        for offset in range(2_000):
+            ts = int(time.time()) + offset
+            payload = f"7:{ts}".encode("ascii")
+            sig = _hmac.new(_key(), payload, _hashlib.sha256).digest()
+            if 0x2E in sig:  # `.`
+                seed_payload = (offset, sig)
+                break
+        assert seed_payload is not None, (
+            "couldn't find a `.`-containing signature in 2000 attempts — "
+            "either SECRET_KEY's broken or the universe ended"
+        )
+
+        # Mint a real token whose expiry produces that signature, then
+        # verify. The fixed-length slicing must extract the right
+        # payload/sig boundary regardless of period-byte placement.
+        offset, _expected_sig = seed_payload
+        # Patch time.time so mint produces the same payload + sig.
+        from unittest.mock import patch
+        target_ts = int(time.time())
+        with patch("src.reports.asset_tokens.time.time", return_value=float(target_ts)):
+            t = mint_asset_token(report_id=7, ttl_seconds=offset)
+        assert verify_asset_token(t, report_id=7) is True, (
+            "verify failed for a token whose signature contains `.` — "
+            "the rsplit-on-period bug is back"
+        )
+
+        # Plus a sanity-decode: the binary form of the token still
+        # starts with the literal payload bytes (`7:<ts+offset>`),
+        # which the regression-prone code split mid-signature.
+        decoded = _b64url_decode(t)
+        assert decoded.startswith(b"7:"), \
+            f"payload prefix corrupted: {decoded[:16]!r}"
+        # And the suffix is exactly _SIG_LEN bytes — the new format.
+        assert len(decoded) > _SIG_LEN
 
     # Note: TTL-via-sleep tests are flaky under the project's full
     # async-aware pytest config. `test_rejects_expired` already