harden(#4023): apply Opus security findings — verify-side name-pattern gate + require handler when auth enabled
Opus independent security review concurred SAFE and surfaced 2 LOW defense-in-depth items, both applied: (1) verify_profile_cookie_value now validates the profile name against _PROFILE_ID_RE itself (not only in get_profile_cookie) so a future second caller can't return an unvalidated name; (2) build_profile_cookie raises when auth is enabled and no handler is passed, so a future call site can't silently emit an unsigned (session-unbound) profile cookie. +3 regression tests.
This commit is contained in:
@@ -498,6 +498,12 @@ def verify_profile_cookie_value(cookie_value: str, session_cookie_value: str | N
|
||||
token = _session_token_from_cookie_value(session_cookie_value)
|
||||
if not profile_name or not token or not sig:
|
||||
return None
|
||||
# Defense-in-depth: validate the profile-name pattern here too, not only in
|
||||
# get_profile_cookie(), so any future caller of this verifier can't return an
|
||||
# unvalidated name. (#4023 Opus hardening.)
|
||||
from api.profiles import _PROFILE_ID_RE
|
||||
if profile_name != 'default' and not _PROFILE_ID_RE.fullmatch(profile_name):
|
||||
return None
|
||||
expected = hmac.new(
|
||||
_signing_key(),
|
||||
f"profile:{token}:{profile_name}".encode(),
|
||||
|
||||
@@ -561,6 +561,17 @@ def build_profile_cookie(name: str, handler=None) -> str:
|
||||
cookie = _hc.SimpleCookie()
|
||||
cookie_name = get_profile_cookie_name()
|
||||
value = name
|
||||
# Guard against a future call site silently emitting an UNSIGNED profile
|
||||
# cookie while auth is enabled (which a client could then... not forge, but
|
||||
# it would weaken the binding). If auth is on we require a handler so the
|
||||
# cookie is bound to the session. (#4023 Opus hardening.)
|
||||
try:
|
||||
from api.auth import is_auth_enabled
|
||||
_auth_on = is_auth_enabled()
|
||||
except Exception:
|
||||
_auth_on = False
|
||||
if _auth_on and handler is None:
|
||||
raise RuntimeError("build_profile_cookie requires a request handler when auth is enabled (to bind the profile cookie to the session)")
|
||||
if handler is not None:
|
||||
try:
|
||||
from api.auth import is_auth_enabled, parse_cookie, sign_profile_cookie_value
|
||||
|
||||
@@ -184,6 +184,41 @@ class TestProfileCookieHelpers:
|
||||
)
|
||||
assert get_profile_cookie(handler) == 'writer'
|
||||
|
||||
def test_verify_profile_cookie_rejects_invalid_name_pattern(self, monkeypatch):
|
||||
"""Defense-in-depth (#4023 Opus hardening): even a correctly-HMAC-signed
|
||||
cookie whose profile name fails _PROFILE_ID_RE must be rejected by the
|
||||
verifier itself, so a future caller can't skip the pattern gate."""
|
||||
from api.auth import sign_profile_cookie_value, verify_profile_cookie_value
|
||||
|
||||
session_cookie = 'session-token.session-sig'
|
||||
monkeypatch.setattr('api.auth.verify_session', lambda cookie: cookie == session_cookie)
|
||||
# Sign a hostile name (would never come from a real switch, but proves the
|
||||
# verifier validates the name even when the signature is valid).
|
||||
signed = sign_profile_cookie_value('../etc', session_cookie)
|
||||
assert verify_profile_cookie_value(signed, session_cookie) is None
|
||||
# And a normal name still round-trips.
|
||||
ok = sign_profile_cookie_value('alice', session_cookie)
|
||||
assert verify_profile_cookie_value(ok, session_cookie) == 'alice'
|
||||
|
||||
def test_build_profile_cookie_requires_handler_when_auth_enabled(self, monkeypatch):
|
||||
"""Defense-in-depth (#4023 Opus hardening): a future call site that forgets
|
||||
to pass the handler while auth is enabled must NOT silently emit an
|
||||
unsigned profile cookie — it raises instead."""
|
||||
from api.helpers import build_profile_cookie
|
||||
|
||||
monkeypatch.setattr('api.auth.is_auth_enabled', lambda: True)
|
||||
with pytest.raises(RuntimeError):
|
||||
build_profile_cookie('alice') # no handler
|
||||
|
||||
def test_build_profile_cookie_allows_no_handler_when_auth_disabled(self, monkeypatch):
|
||||
"""No-auth mode keeps the legacy plain-name cookie with no handler."""
|
||||
from api.helpers import build_profile_cookie
|
||||
|
||||
monkeypatch.delenv('WEBUI_PROFILE_COOKIE_NAME', raising=False)
|
||||
monkeypatch.setattr('api.auth.is_auth_enabled', lambda: False)
|
||||
s = build_profile_cookie('alice')
|
||||
assert 'hermes_profile=alice' in s
|
||||
|
||||
def test_configured_profile_cookie_ignores_default_cookie_name(self, monkeypatch):
|
||||
from api.helpers import get_profile_cookie
|
||||
|
||||
|
||||
Reference in New Issue
Block a user