From 13f209d4fd6cd763041a6e3fa42a11daec968af4 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:01:25 +0530 Subject: [PATCH] refactor(browser): dedupe auth-flow names and sentinel identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename the broker's TicketInvalid to ControllerTicketInvalid: the same exception name already exists in hermes_cli/dashboard_auth/ws_tickets.py and BOTH are caught in the same WS auth flow this feature touches — two unrelated same-named exception types in one blast radius invited a wrong except clause. - Import the 'server-internal' sentinel identity from its canonical definition (ws_tickets.INTERNAL_USER_ID/INTERNAL_PROVIDER) instead of re-declaring the strings; drift would have silently broken the internal-peer exclusion in _is_authenticated_identity. Surfaced during review of PR #85351. --- gateway/browser_control_broker.py | 12 ++++++------ gateway/platforms/api_server.py | 4 ++-- tests/gateway/test_browser_control_broker.py | 6 +++--- tui_gateway/methods_browser_control.py | 10 ++++------ 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/gateway/browser_control_broker.py b/gateway/browser_control_broker.py index c8a97fa8eb..2b32f0f1fc 100644 --- a/gateway/browser_control_broker.py +++ b/gateway/browser_control_broker.py @@ -22,7 +22,7 @@ Contract (each rule is exercised by tests/gateway/test_browser_control_broker.py (``secrets``-derived, >= 32 chars) plus an expiry derived from the injected clock; ``consume_ticket`` exchanges it exactly once for the :class:`ControllerScope` it was minted for, raising - :class:`TicketInvalid` for unknown, already-consumed, or expired values. + :class:`ControllerTicketInvalid` for unknown, already-consumed, or expired values. The ticket is the only cross-transport credential minted here; transports decide how to carry it. @@ -207,7 +207,7 @@ class BrowserControlError(Exception): """Base class for broker contract failures.""" -class TicketInvalid(BrowserControlError): +class ControllerTicketInvalid(BrowserControlError): """A registration ticket is unknown, already consumed, or expired.""" @@ -372,7 +372,7 @@ class BrowserControlBroker: def consume_ticket(self, value: str) -> ControllerScope: """Exchange a ticket for its scope, exactly once. - Raises :class:`TicketInvalid` for unknown, already-consumed, or + Raises :class:`ControllerTicketInvalid` for unknown, already-consumed, or expired tickets. The expiry check happens against the live clock at consume time, so a ticket that outlived its TTL can never be used. """ @@ -380,11 +380,11 @@ class BrowserControlBroker: with self._lock: record = self._tickets.get(value) if record is None: - raise TicketInvalid("unknown ticket") + raise ControllerTicketInvalid("unknown ticket") if record.consumed: - raise TicketInvalid("ticket already consumed") + raise ControllerTicketInvalid("ticket already consumed") if now > record.expires_at: - raise TicketInvalid("ticket expired") + raise ControllerTicketInvalid("ticket expired") record.consumed = True return record.scope diff --git a/gateway/platforms/api_server.py b/gateway/platforms/api_server.py index 9620ba6ff2..6f423534c4 100644 --- a/gateway/platforms/api_server.py +++ b/gateway/platforms/api_server.py @@ -138,7 +138,7 @@ from gateway.browser_control_broker import ( BROWSER_CONTROL_CAPABILITIES, BROWSER_CONTROL_DEVELOPER_CAPABILITIES, ControllerScope, - TicketInvalid, + ControllerTicketInvalid, browser_control_developer_mode, browser_control_protocol_supported, filter_browser_control_capabilities, @@ -3591,7 +3591,7 @@ class APIServerAdapter(BasePlatformAdapter): raise web.HTTPUnauthorized() try: scope = self._browser_control_broker.consume_ticket(ticket_value) - except TicketInvalid: + except ControllerTicketInvalid: raise web.HTTPUnauthorized() from None except Exception: logger.exception("browser-control WS ticket consumption failed") diff --git a/tests/gateway/test_browser_control_broker.py b/tests/gateway/test_browser_control_broker.py index 6f910f87e6..98affd2abd 100644 --- a/tests/gateway/test_browser_control_broker.py +++ b/tests/gateway/test_browser_control_broker.py @@ -7,7 +7,7 @@ from gateway.browser_control_broker import ( BrowserControlBroker, ControllerCancelled, ControllerScope, - TicketInvalid, + ControllerTicketInvalid, ) @@ -34,12 +34,12 @@ def test_registration_ticket_is_short_lived_single_use_and_identity_bound(): assert len(ticket.value) >= 32 assert ticket.expires_at == 130.0 assert broker.consume_ticket(ticket.value) == scope - with pytest.raises(TicketInvalid, match="unknown|consumed"): + with pytest.raises(ControllerTicketInvalid, match="unknown|consumed"): broker.consume_ticket(ticket.value) expired = broker.mint_ticket(scope) now[0] = 131.0 - with pytest.raises(TicketInvalid, match="expired"): + with pytest.raises(ControllerTicketInvalid, match="expired"): broker.consume_ticket(expired.value) diff --git a/tui_gateway/methods_browser_control.py b/tui_gateway/methods_browser_control.py index 65409ee69f..abd7afa951 100644 --- a/tui_gateway/methods_browser_control.py +++ b/tui_gateway/methods_browser_control.py @@ -41,6 +41,10 @@ from gateway.browser_control_broker import ( browser_control_protocol_supported, filter_browser_control_capabilities, ) +from hermes_cli.dashboard_auth.ws_tickets import ( + INTERNAL_PROVIDER as _INTERNAL_PROVIDER, + INTERNAL_USER_ID as _INTERNAL_USER_ID, +) from .method_ctx import HandlerRegistry @@ -57,12 +61,6 @@ _CLOUD_TRANSPORT_FAMILY = "cloud-ticket-ws" #: JSON-RPC error code for identity / session / flag denials (forbidden). _ERR_FORBIDDEN = 4403 -#: Identity recorded for server-spawned WS clients (see -#: ``hermes_cli.dashboard_auth.ws_tickets``) — never allowed to act as a -#: browser controller. -_INTERNAL_USER_ID = "server-internal" -_INTERNAL_PROVIDER = "server-internal" - def _is_authenticated_identity(identity: object) -> bool: """True for a server-minted, non-internal ``{user_id, provider}`` identity."""