Follow-up from the merge-turn review of #325 (batch 3). Not attacker-controlled, so kept out of the batch; filed so it is fixed in the normal flow.
Problem
spp_dci_server/middleware/signature.py (verify_bearer_token, around lines 381-388) now rejects a non-ASCII client token with 401 before hmac.compare_digest. The other operand is still unguarded: the accepted tokens come from the dci.api_tokens config parameter, split and strip()ped, and strip() removes only leading/trailing whitespace.
If an operator pastes a token containing a non-ASCII character (an NBSP or a smart quote from a runbook, a typo), compare_digest(str, str) raises TypeError and every request to all seven bearer routes returns 500, including requests with a valid token, until the parameter is corrected. That defeats #325's stated invariant ("no TypeError from the compare").
Fix
One line in the accepted-token filter: if t.strip() and t.strip().isascii(), plus a warning log naming the config key (not the value) when a configured token is skipped, and a unit test with a non-ASCII configured token asserting 401 for a wrong client token and 200 for a valid ASCII one.
Also noted in the same review (pre-existing, optional in the same PR)
authorization.startswith("Bearer ") is case-sensitive; RFC 6750 makes the scheme case-insensitive and spp_api_v2/middleware/auth.py lowercases. Align.
- The unauthenticated caller can distinguish
err.auth.no_tokens_configured from err.auth.invalid_token, disclosing whether tokens are configured. Consider a single error code.
_logger.warning per bad token is unbounded (log flood); consider rate-limiting or info.
- Tests call the coroutine with a Python
str; no HTTP-level test proves the 401 body end to end. Repo has TestClient infra in fastapi/tests/common.py.
Follow-up from the merge-turn review of #325 (batch 3). Not attacker-controlled, so kept out of the batch; filed so it is fixed in the normal flow.
Problem
spp_dci_server/middleware/signature.py(verify_bearer_token, around lines 381-388) now rejects a non-ASCII client token with 401 beforehmac.compare_digest. The other operand is still unguarded: the accepted tokens come from thedci.api_tokensconfig parameter, split andstrip()ped, andstrip()removes only leading/trailing whitespace.If an operator pastes a token containing a non-ASCII character (an NBSP or a smart quote from a runbook, a typo),
compare_digest(str, str)raisesTypeErrorand every request to all seven bearer routes returns 500, including requests with a valid token, until the parameter is corrected. That defeats #325's stated invariant ("no TypeError from the compare").Fix
One line in the accepted-token filter:
if t.strip() and t.strip().isascii(), plus a warning log naming the config key (not the value) when a configured token is skipped, and a unit test with a non-ASCII configured token asserting 401 for a wrong client token and 200 for a valid ASCII one.Also noted in the same review (pre-existing, optional in the same PR)
authorization.startswith("Bearer ")is case-sensitive; RFC 6750 makes the scheme case-insensitive andspp_api_v2/middleware/auth.pylowercases. Align.err.auth.no_tokens_configuredfromerr.auth.invalid_token, disclosing whether tokens are configured. Consider a single error code._logger.warningper bad token is unbounded (log flood); consider rate-limiting orinfo.str; no HTTP-level test proves the 401 body end to end. Repo has TestClient infra infastapi/tests/common.py.