Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 50 additions & 8 deletions plugins/platforms/matrix/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,52 @@ def _normalize_matrix_bang_command(text: str) -> str:
return f"/{resolved}{match.group(2) or ''}"


# Auth errcodes that genuinely require re-authentication (never retried).
_MATRIX_PERMANENT_ERRCODES = frozenset(
{"m_unknown_token", "m_missing_token", "m_forbidden"}
)
# Leading HTTP status on error strings formatted as ``"<status>: <body>"``.
_MATRIX_LEADING_STATUS_RE = re.compile(r"^\s*(\d{3})\b")


def _is_permanent_matrix_auth_error(exc: object) -> bool:
"""Return True only for genuine auth failures that must stop the sync loop.

A transient homeserver outage surfaces as a 5xx whose body may be an HTML
error page (Umbrel's app-proxy returns one). Naive substring checks like
``"403" in str(exc)`` false-positive on digits embedded in that HTML (an SVG
coordinate such as ``40.4302`` contains ``403``), which previously stopped
the sync loop permanently on a passing blip. Classify on the real HTTP
status / errcode instead, and retry everything that is not 401/403.
"""
# Prefer structured attributes when the exception carries them.
errcode = getattr(exc, "errcode", None)
if isinstance(errcode, str) and errcode.strip().lower() in _MATRIX_PERMANENT_ERRCODES:
return True
for attr in ("http_status", "status", "status_code", "code"):
val = getattr(exc, attr, None)
if isinstance(val, int):
return val in (401, 403)

# Fall back to parsing a leading status code off the string form
# (e.g. ``"502: <!DOCTYPE html>..."``). A known status is authoritative:
# only 401/403 are permanent; 5xx/429/etc. must retry regardless of body.
text = str(exc)
m = _MATRIX_LEADING_STATUS_RE.match(text)
if m:
return int(m.group(1)) in (401, 403)

# No status available: trust only whole-word auth errcodes/keywords found in
# a bounded prefix, so a large HTML body cannot smuggle a false positive.
head = text[:200].lower()
return bool(
re.search(
r"\b(m_unknown_token|m_missing_token|m_forbidden|unauthorized|forbidden)\b",
head,
)
)


class _MatrixHtmlSanitizer(HTMLParser):
"""Allowlist sanitizer for Matrix-compatible formatted HTML."""

Expand Down Expand Up @@ -2323,14 +2369,10 @@ async def _sync_loop(self) -> None:
except Exception as exc:
if self._closing:
return
# Detect permanent auth/permission failures.
err_str = str(exc).lower()
if (
"401" in err_str
or "403" in err_str
or "unauthorized" in err_str
or "forbidden" in err_str
):
# Detect permanent auth/permission failures. Transient 5xx
# outages (e.g. a homeserver restart returning a 502 HTML page)
# must be retried, not treated as fatal.
if _is_permanent_matrix_auth_error(exc):
logger.error(
"Matrix: permanent auth error: %s — stopping sync", exc
)
Expand Down
54 changes: 54 additions & 0 deletions tests/gateway/test_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -5276,3 +5276,57 @@ async def test_flag_clears_when_second_connect_resolves_device_id(self):
assert None not in _verify_call.args[0]["@bot:example.org"]

await adapter.disconnect()


class TestMatrixPermanentAuthClassifier:
"""Only genuine 401/403 auth failures may stop the sync loop.

A transient homeserver outage surfaces as a 5xx whose body may be an HTML
error page. The Umbrel app-proxy returns one whose embedded SVG contains the
coordinate ``40.4302`` — the substring ``403``. The old ``"403" in str(exc)``
check false-positived on that and permanently halted Matrix sync on a passing
blip. The classifier must retry any status that is not 401/403.
"""

# Trimmed excerpt of the actual Umbrel 502 body that caused the outage;
# the SVG path coordinate 40.4302 contains the substring "403".
_REAL_502 = (
'502: <!DOCTYPE html><svg><path d="M17.4517 40.4302C12.7214 40.4302'
' 9.82339 41.8182 7.98048 44.0001"/></svg>'
)

def _fn(self):
from plugins.platforms.matrix.adapter import _is_permanent_matrix_auth_error

return _is_permanent_matrix_auth_error

def test_transient_502_html_body_is_retried(self):
# The exact failure mode: a 502 whose HTML body embeds "403".
assert self._fn()(Exception(self._REAL_502)) is False

@pytest.mark.parametrize("status", [500, 502, 503, 504, 429])
def test_server_errors_are_retried(self, status):
assert self._fn()(Exception(f"{status}: upstream unavailable")) is False

def test_connection_errors_are_retried(self):
fn = self._fn()
assert fn(Exception("[Errno 104] Connection reset by peer")) is False
assert fn(Exception("Server disconnected")) is False

@pytest.mark.parametrize("status", [401, 403])
def test_real_auth_status_stops_sync(self, status):
assert self._fn()(Exception(f"{status}: nope")) is True

def test_http_status_attribute_beats_body_digits(self):
# A structured 502 whose message text also contains "403" must retry.
exc = Exception("body 40.4302")
exc.http_status = 502
assert self._fn()(exc) is False

def test_errcode_unknown_token_stops_sync(self):
exc = Exception("sync failed")
exc.errcode = "M_UNKNOWN_TOKEN"
assert self._fn()(exc) is True

def test_bare_auth_keyword_without_status_stops_sync(self):
assert self._fn()(Exception("Unauthorized")) is True