mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(dashboard): accept HA ingress prefix paths
Allow mainstream reverse-proxy path mounts to keep their X-Forwarded-Prefix when Home Assistant Supervisor ingress already consumes nearly the old 64-character budget. Keep validation bounded and keep rejected non-empty prefixes diagnosable with a deduplicated warning. Constraint: HA Supervisor ingress prefixes are 63 chars before add-on subpaths, so the old 64-char cap dropped valid dashboard deployments. Rejected: remove the length cap entirely | a bounded header budget is still a conservative validation guard. Confidence: high Scope-risk: narrow Directive: Keep prefix validation centralized in hermes_cli.dashboard_auth.prefix so auth routes, cookies, and SPA asset rewriting agree. Tested: python probe for the 73-char HA ingress prefix; scripts/run_tests.sh tests/hermes_cli/test_dashboard_auth_prefix.py -q; .venv/bin/python -m pytest tests/hermes_cli/test_web_server.py -k 'spa_assets_are_read_as_utf8' -q; python -m ruff check hermes_cli/dashboard_auth/prefix.py tests/hermes_cli/test_dashboard_auth_prefix.py; git diff --check Not-tested: full test suite
This commit is contained in:
parent
3b5c645433
commit
ef79ad014d
2 changed files with 88 additions and 1 deletions
|
|
@ -26,6 +26,11 @@ from typing import Optional
|
|||
|
||||
_log = logging.getLogger(__name__)
|
||||
|
||||
# Home Assistant Supervisor ingress prefixes are already 63 chars before
|
||||
# deployments add their own sub-path. Keep a bounded header budget, but leave
|
||||
# room for mainstream reverse-proxy path mounts.
|
||||
_MAX_PREFIX_LENGTH = 256
|
||||
|
||||
# Characters that, if present in a public_url or prefix value, indicate
|
||||
# either a typo or a header-injection attempt. Reject the whole value
|
||||
# rather than try to sanitise — the operator can fix their config.
|
||||
|
|
@ -37,6 +42,7 @@ _REJECT_CHARS = frozenset(('"', "'", "<", ">", " ", "\n", "\r", "\t"))
|
|||
# misconfigured deploy. Keyed on the raw value too, so changing the
|
||||
# config and reloading surfaces a fresh warning.
|
||||
_warned_malformed_public_urls: set = set()
|
||||
_warned_malformed_prefixes: set = set()
|
||||
|
||||
|
||||
def _warn_if_malformed(source: str, raw: str) -> None:
|
||||
|
|
@ -72,6 +78,23 @@ def _warn_if_malformed(source: str, raw: str) -> None:
|
|||
)
|
||||
|
||||
|
||||
def _warn_if_malformed_prefix(raw: Optional[str], reason: str) -> None:
|
||||
"""Warn once when a non-empty X-Forwarded-Prefix value is rejected."""
|
||||
cleaned = raw.strip() if raw else ""
|
||||
if not cleaned:
|
||||
return
|
||||
key = (cleaned, reason)
|
||||
if key in _warned_malformed_prefixes:
|
||||
return
|
||||
_warned_malformed_prefixes.add(key)
|
||||
_log.warning(
|
||||
"X-Forwarded-Prefix header %r was ignored because %s. "
|
||||
"Dashboard URLs will be generated without a reverse-proxy path prefix.",
|
||||
cleaned,
|
||||
reason,
|
||||
)
|
||||
|
||||
|
||||
def normalise_prefix(raw: Optional[str]) -> str:
|
||||
"""Normalise an X-Forwarded-Prefix header value.
|
||||
|
||||
|
|
@ -94,8 +117,16 @@ def normalise_prefix(raw: Optional[str]) -> str:
|
|||
or ".." in p
|
||||
or any(c in p for c in _REJECT_CHARS)
|
||||
):
|
||||
_warn_if_malformed_prefix(
|
||||
raw,
|
||||
"it contains a disallowed character or path sequence",
|
||||
)
|
||||
return ""
|
||||
if len(p) > 64:
|
||||
if len(p) > _MAX_PREFIX_LENGTH:
|
||||
_warn_if_malformed_prefix(
|
||||
raw,
|
||||
f"it is longer than {_MAX_PREFIX_LENGTH} characters",
|
||||
)
|
||||
return ""
|
||||
return p
|
||||
|
||||
|
|
|
|||
|
|
@ -30,15 +30,24 @@ those rules surfaces before a Mission Control deploy.
|
|||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
|
||||
import pytest
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from hermes_cli import web_server
|
||||
from hermes_cli.dashboard_auth import clear_providers, register_provider
|
||||
from hermes_cli.dashboard_auth import prefix as prefix_mod
|
||||
from tests.hermes_cli.conftest_dashboard_auth import StubAuthProvider
|
||||
|
||||
|
||||
HA_INGRESS_DASHBOARD_PREFIX = (
|
||||
"/api/hassio_ingress/8AbCdEfGhIjKlMnOpQrStUvWxYz0123456789AbCdEf"
|
||||
"/dashboard"
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def gated_app_proxied():
|
||||
"""web_server.app configured for gated mode with proxy_headers + a
|
||||
|
|
@ -92,6 +101,53 @@ def gated_app_direct():
|
|||
web_server.app.state.auth_required = prev_required
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# X-Forwarded-Prefix normalisation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestForwardedPrefixNormalisation:
|
||||
def test_home_assistant_ingress_prefix_with_subpath_is_accepted(
|
||||
self, caplog
|
||||
):
|
||||
"""Home Assistant Supervisor ingress prefixes are 63 chars before
|
||||
add-ons append their own mount path. They must survive validation so
|
||||
the SPA receives the correct __HERMES_BASE_PATH__ and asset prefix."""
|
||||
prefix_mod._warned_malformed_prefixes.clear()
|
||||
assert len(HA_INGRESS_DASHBOARD_PREFIX) > 64
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger=prefix_mod.__name__):
|
||||
result = prefix_mod.normalise_prefix(HA_INGRESS_DASHBOARD_PREFIX)
|
||||
|
||||
assert result == HA_INGRESS_DASHBOARD_PREFIX
|
||||
assert not [
|
||||
r for r in caplog.records
|
||||
if r.levelno == logging.WARNING
|
||||
and "X-Forwarded-Prefix" in r.getMessage()
|
||||
]
|
||||
|
||||
def test_overlong_prefix_is_rejected_with_deduplicated_warning(
|
||||
self, caplog
|
||||
):
|
||||
"""Keep a bounded header budget, but make rejected non-empty
|
||||
prefixes diagnosable instead of silently producing root-relative
|
||||
dashboard URLs."""
|
||||
prefix_mod._warned_malformed_prefixes.clear()
|
||||
too_long = "/" + ("a" * 257)
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger=prefix_mod.__name__):
|
||||
for _ in range(3):
|
||||
assert prefix_mod.normalise_prefix(too_long) == ""
|
||||
|
||||
warnings = [
|
||||
r for r in caplog.records
|
||||
if r.levelno == logging.WARNING
|
||||
and "X-Forwarded-Prefix" in r.getMessage()
|
||||
]
|
||||
assert len(warnings) == 1
|
||||
assert "longer than 256 characters" in warnings[0].getMessage()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Gate middleware: Location: header and 401 envelope respect prefix
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue