fix(bluebubbles): timing-safe webhook auth + fail closed when password unset

[Bartok9]

Problem

The BlueBubbles webhook handler (_handle_webhook) authenticates inbound
callers with a plain comparison:

token = (request.query.get("password") or ... or request.headers.get("x-bluebubbles-guid"))
if token != self.password:
    return web.json_response({"error": "unauthorized"}, status=401)

Two defects:

1. Fail-open when no password is configured. self.password defaults to
   extra.get("password") or os.getenv("BLUEBUBBLES_PASSWORD", "") — i.e. ""
   (or None if the adapter is mutated). When unset, a caller that also omits
   the token yields None != None → False (or "" != ""), so the request is
   accepted and dispatched unauthenticated. A misconfigured server silently
   accepts anonymous webhook posts.
2. Timing side channel. The plain != short-circuits on the first differing
   byte, leaking token length/content via response timing.

Root cause

The handler never special-cased the unconfigured-password state, and used !=
instead of a constant-time compare for a shared secret.

Fix

• Reject unconditionally (401) when self.password is falsy — fail closed.
• Compare with hmac.compare_digest(str(token or ""), str(self.password)) so a
  mismatch can't be recovered from response timing.

+19/-2 across 2 files (1 source, 1 test).

Tests (real output)

New regression class TestBlueBubblesWebhookAuth in
tests/gateway/test_bluebubbles.py. The fail-closed test FAILS without the fix
(auth bypassed → request dispatched, 400 downstream) and passes with it:

Without the fix:

>       assert response.status == 401
E       assert 400 == 401
FAILED tests/gateway/test_bluebubbles.py::TestBlueBubblesWebhookAuth::test_webhook_fails_closed_when_password_unconfigured
1 failed, 1 passed

With the fix:

tests/gateway/test_bluebubbles.py::TestBlueBubblesWebhookAuth::test_webhook_rejects_wrong_password PASSED
tests/gateway/test_bluebubbles.py::TestBlueBubblesWebhookAuth::test_webhook_fails_closed_when_password_unconfigured PASSED

Full adapter suite:

58 passed, 7 warnings in 1.95s

python -m py_compile clean on both changed files.

[alt-glitch]

This was generated by AI during triage.

Related: re-implements the fail-closed + timing-safe BlueBubbles webhook auth fix from the now-closed #36862 (and issue #36849). #6608 (open) addresses the same adapter via a different mechanism (a dedicated BLUEBUBBLES_WEBHOOK_SECRET with credential separation). Flagging the cluster so a reviewer can pick the canonical approach; this PR is the minimal fail-closed + hmac.compare_digest variant.

[tonydwb]

Code Review Summary

Verdict: Approved

Two security improvements for BlueBubbles webhook auth: (1) fail closed when password is unset (previously None != None = False let unauthenticated requests through), (2) constant-time comparison via hmac.compare_digest to prevent timing side channels. Includes regression tests for both fixes.

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused BlueBubbles authentication hardening. Current main still uses token != self.password in gateway/platforms/bluebubbles.py:876-884, so the constant-time comparison is a useful, narrowly scoped improvement.

Problems

• The fail-closed test does not represent a reachable live-webhook state. BlueBubblesAdapter.connect() returns when self.password is falsy at gateway/platforms/bluebubbles.py:240-244; the only aiohttp webhook binding is created later at gateway/platforms/bluebubbles.py:275-277. Directly assigning adapter.password = None in the test bypasses that lifecycle invariant.

Suggested changes

• Keep the hmac.compare_digest change, but replace the direct-mutation regression with a lifecycle assertion that an unconfigured adapter cannot connect. This matches the withdrawal rationale recorded on related #36862/#36849 while preserving the independent timing-hardening value.

Automated hermes-sweeper review.

[teknium1]

This mutation bypasses the lifecycle invariant: connect() rejects an empty password before binding _handle_webhook (gateway/platforms/bluebubbles.py:240-277). Please test that connection refusal instead; this does not reproduce a reachable live-endpoint bypass.

[Bartok9]

Thanks @teknium1 — agreed the direct adapter.password = None mutation bypasses the connect() lifecycle invariant (falsy password returns early at :240-244 before the webhook binds at :275-277), so it's not a reachable live-webhook state. Keeping the hmac.compare_digest hardening and replacing the fail-closed regression with a lifecycle assertion that an unconfigured adapter cannot connect. Pushing shortly.

[egilewski]

suggesting changes

gateway/platforms/bluebubbles.py:884 passes attacker-controlled credential text to hmac.compare_digest as str, but Python rejects non-ASCII string operands with TypeError. A percent-encoded Unicode query credential therefore reaches this new line and produces HTTP 500 instead of 401; a correctly configured Unicode password also regresses from an accepted request to 500. This is not an authentication bypass, but it introduces an unauthenticated exception path in the authentication boundary and breaks credentials that current main accepts.

Please compare UTF-8-encoded bytes while retaining compare_digest (rather than falling back to ordinary equality), and add regressions showing that a wrong Unicode query/header credential returns 401 and a correct configured Unicode credential remains accepted. If passwords are intended to be ASCII-only instead, enforce that explicitly at configuration time while still rejecting hostile Unicode input without an exception.

Security evidence:

• trust boundary: unauthenticated BlueBubbles query parameters and credential headers reach this comparison before payload parsing and message dispatch.
• source/sink/invariant: only the exact configured credential may cross into webhook processing; missing or invalid input must return 401 without raising, and an unconfigured adapter must not bind the route.
• current-main reproduction: a real aiohttp request returned 401 for a wrong Unicode credential and 200 for a correct configured Unicode credential.
• PR-head or patch-replay validation: the exact submitted head and a conflict-free replay onto current main returned 500 for both Unicode cases while preserving the expected ASCII 200/401 behavior.
• positive/negative cases: correct ASCII credentials were accepted, missing and wrong ASCII credentials were rejected, wrong Unicode credentials failed with 500, and correct Unicode credentials also failed with 500.
• residual bypass search: no authentication bypass was found; all five query/header aliases converge on the same comparison, and the empty-password lifecycle remains fail-closed.
• reviewer validation: source tracing, direct handler probes, an aiohttp route probe, and an independent GPT-5.6-Sol/xhigh review reproduced the same blocker.

Not checked:

• Full test suite
• Live BlueBubbles integration
• Timing benchmark
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

[Bartok9]

Thanks @egilewski — confirmed. hmac.compare_digest on str raises TypeError for non-ASCII operands, so Unicode query/header credentials (and configured non-ASCII passwords) became HTTP 500 instead of a clean 401/200.

Pushed a fix that UTF-8-encodes both sides before compare_digest (still constant-time; no fallback to ==), plus regressions:

• wrong Unicode credential → 401
• correct configured Unicode credential → accepted

TestBlueBubblesWebhookAuth: 4 passed locally.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #9219 for the current head's identical hmac.compare_digest replacement in the BlueBubbles webhook handler. The additional empty-password guard reprises closed #36862; #6608 remains a related, different credential-separation approach.

[Bartok9]

Thanks @alt-glitch — noted on the cluster with #9219 / closed #36862 / #6608.

Relative to open #9219 (timing-safe compare_digest only, with or "" on both sides), this head is intentionally broader:

1. UTF-8 bytes before compare_digest — str operands raise TypeError on non-ASCII, turning wrong/correct Unicode credentials into HTTP 500 (called out by @egilewski; fixed on head with regressions). fix(security): use hmac.compare_digest for BlueBubbles webhook token to prevent timing attacks #9219’s token or "" / self.password or "" path has the same str-vs-bytes issue for non-ASCII.
2. Lifecycle fail-closed — unconfigured adapter cannot connect() / bind the webhook (per @teknium1), not only a handler-level empty-password guard.
3. Regression coverage for wrong password, Unicode wrong/correct credentials, and connect refusal when password unset.

Happy for maintainers to pick a canonical approach (merge this, fold the UTF-8 + lifecycle pieces into #9219, or go the #6608 credential-separation route). No objection to closing this as duplicate if #9219 is extended to cover (1)–(3).