mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(webapp): pending window and launch links keep the credential's digest, not the credential
The tickets a Webapp tab mints to open another window, and the one-use links hermes webapp opens, were stored with the browser credential itself next to them, and redeeming one compared that copy with the credential in use. Anything that could read the gateway's app state while a link was pending found the credential there, and the check was a fourth way of asking whether a grant outlived a sign-out. The store now records the credential's generation from hermes_cli.browser_credential. Redeeming a link hands over the credential through browser_credential.current_if(), which reads it once, so a sign-out landing between the check and the answer cannot pass on the new credential to a link minted under the old one. The sign-in binding's renewal uses the same call for the same reason.
This commit is contained in:
1 parent
0a4b903c52
commit
9514fc32e2
5 files changed
+41
-13
No files matched your search
@@ -47,7 +47,17 @@ def generation() -> str:
|
||||
|
||||
def is_current(recorded: Optional[str]) -> bool:
|
||||
"""Whether a grant that recorded generation *recorded* was made under the credential in use now."""
|
||||
return recorded is not None and hmac.compare_digest(recorded, generation())
|
||||
return current_if(recorded) is not None
|
||||
|
||||
|
||||
def current_if(recorded: Optional[str]) -> Optional[str]:
|
||||
"""The credential in use now when *recorded* is its generation, else None. For a grant that
|
||||
hands the credential over or binds a login to it: one read, so a rotation between the check
|
||||
and the use cannot pass on the credential that replaced the one the grant was made under."""
|
||||
credential = current()
|
||||
if recorded is None or not hmac.compare_digest(recorded, generation_of(credential)):
|
||||
return None
|
||||
return credential
|
||||
|
||||
|
||||
def on_rotate(name: str, revoker: Callable[[], int]) -> None:
|
||||
|
||||
@@ -61,19 +61,20 @@ async def host_terminal_ws(ws: WebSocket) -> None:
|
||||
|
||||
|
||||
def _mint_handoff_ticket(state, ttl: float, *, launch: bool) -> str:
|
||||
"""One-use ticket that ``/webapp/window-session`` exchanges for this start's session token."""
|
||||
from hermes_cli.web_server import _SESSION_TOKEN
|
||||
"""One-use ticket that ``/webapp/window-session`` exchanges for this start's session token. The
|
||||
store keeps ``(expiry, browser credential generation, launch)``, never the credential itself."""
|
||||
from hermes_cli import browser_credential
|
||||
|
||||
now = time.monotonic()
|
||||
with _ticket_lock:
|
||||
tickets = getattr(state, "webapp_window_tickets", {})
|
||||
tickets = {key: value for key, value in tickets.items()
|
||||
if value[0] > now and value[1] == _SESSION_TOKEN}
|
||||
if value[0] > now and browser_credential.is_current(value[1])}
|
||||
state.webapp_window_tickets = tickets
|
||||
if len(tickets) >= _MAX_WINDOW_TICKETS:
|
||||
raise HTTPException(429, "Too many pending Webapp windows; try again shortly")
|
||||
ticket = secrets.token_urlsafe(32)
|
||||
tickets[ticket] = (now + ttl, _SESSION_TOKEN, launch)
|
||||
tickets[ticket] = (now + ttl, browser_credential.generation(), launch)
|
||||
return ticket
|
||||
|
||||
|
||||
@@ -253,7 +254,7 @@ async def issue_audio_ticket(request: Request, socket: str = ""):
|
||||
|
||||
@router.post("/webapp/window-session")
|
||||
async def redeem_window_ticket(request: Request):
|
||||
from hermes_cli.web_server import _SESSION_TOKEN
|
||||
from hermes_cli import browser_credential
|
||||
|
||||
_require_private_webapp(request)
|
||||
# This endpoint deliberately sits outside /api: the child window or launch
|
||||
@@ -279,9 +280,10 @@ async def redeem_window_ticket(request: Request):
|
||||
entry = None
|
||||
elif entry is not None:
|
||||
del tickets[ticket]
|
||||
if entry is None or entry[0] <= time.monotonic() or entry[1] != _SESSION_TOKEN:
|
||||
token = browser_credential.current_if(entry[1]) if entry is not None and entry[0] > time.monotonic() else None
|
||||
if token is None:
|
||||
raise HTTPException(403, "Webapp window link expired or was already used; open a new window")
|
||||
return JSONResponse({"token": _SESSION_TOKEN}, headers={"Cache-Control": "no-store"})
|
||||
return JSONResponse({"token": token}, headers={"Cache-Control": "no-store"})
|
||||
|
||||
|
||||
_LAUNCH_SCRIPT = r"""
|
||||
|
||||
@@ -104,11 +104,8 @@ def bind(response: Response, request: Request) -> None:
|
||||
def renew(response: Response, request: Request) -> None:
|
||||
"""Keep the binding as long-lived as the session cookies a refresh just rotated, unless the
|
||||
credential it was checked against was replaced while the request ran."""
|
||||
# One read of the credential: binding the login to a second read could bind it to a credential
|
||||
# that replaced the one it was checked against.
|
||||
token = browser_credential.current()
|
||||
signed_in = sign_in_of(request)
|
||||
if signed_in is not None and hmac.compare_digest(signed_in, browser_credential.generation_of(token)):
|
||||
token = browser_credential.current_if(sign_in_of(request))
|
||||
if token is not None:
|
||||
_set(response, request, token)
|
||||
|
||||
|
||||
|
||||
@@ -9,13 +9,16 @@ from hermes_cli import browser_credential, web_server
|
||||
|
||||
|
||||
def test_a_generation_is_current_only_while_its_credential_is_in_use(monkeypatch):
|
||||
"""A grant hands over or binds to the credential only while its recorded generation is current."""
|
||||
monkeypatch.setattr(web_server, "_SESSION_TOKEN", "a" * 43)
|
||||
recorded = browser_credential.generation()
|
||||
assert recorded == browser_credential.generation_of("a" * 43) != "a" * 43
|
||||
assert browser_credential.is_current(recorded)
|
||||
assert not browser_credential.is_current(None)
|
||||
assert browser_credential.current_if(recorded) == "a" * 43
|
||||
monkeypatch.setattr(web_server, "_SESSION_TOKEN", "b" * 43)
|
||||
assert not browser_credential.is_current(recorded)
|
||||
assert browser_credential.current_if(recorded) is None
|
||||
|
||||
|
||||
def test_rotate_replaces_the_credential_then_runs_every_revoker(monkeypatch):
|
||||
|
||||
@@ -90,3 +90,19 @@ def test_child_handoff_is_bound_to_local_webapp_start(local_webapp, monkeypatch,
|
||||
headers={"Origin": "http://127.0.0.1", "X-Hermes-Window-Ticket": ticket}, follow_redirects=False)
|
||||
assert denied.status_code in (302, 401, 403, 404)
|
||||
assert server._SESSION_TOKEN not in denied.text
|
||||
|
||||
|
||||
def test_pending_handoffs_keep_the_credentials_generation_not_the_credential(local_webapp):
|
||||
"""Window and launch tickets waiting to be spent record the browser credential's generation
|
||||
(``hermes_cli.browser_credential``), never the credential itself, and still hand it over once."""
|
||||
from hermes_cli.web_routers import webapp
|
||||
|
||||
server, client = local_webapp
|
||||
token = server._SESSION_TOKEN
|
||||
window = client.post("/api/webapp/window-ticket", headers={"X-Hermes-Session-Token": token}).json()["ticket"]
|
||||
webapp.mint_launch_ticket(server.app.state)
|
||||
webapp.mint_printed_launch_ticket(server.app.state)
|
||||
stored = server.app.state.webapp_window_tickets
|
||||
assert len(stored) == 3 and token not in repr(stored)
|
||||
spent = client.post("/webapp/window-session", headers={"Origin": "http://127.0.0.1", "X-Hermes-Window-Ticket": window})
|
||||
assert spent.json()["token"] == token
|
||||
Reference in new issue
Block a user