Skip to content
Merged
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
4 changes: 4 additions & 0 deletions docker/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ start_nginx() {
log "the engine did not publish its browser session cookie"
return 1
fi
# The name has to be the one the engine reads, and it derives that from the
# request's Host port -- which nginx forwards without one, so the engine keeps
# its own PAGE_PORT. Spelled here as that same port; test_docker_runtime.py
# holds the two sides together.
RAVEN_AUTO_LOGIN_COOKIE="raven_session_${PAGE_PORT}=${cookie}; Path=/; Max-Age=2592000; HttpOnly; SameSite=Strict"
fi
export RAVEN_AUTO_LOGIN_COOKIE
Expand Down
10 changes: 6 additions & 4 deletions docker/nginx/raven.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ server {

include /etc/nginx/proxy.conf;

# The engine answers only to an Origin naming its own loopback port
# (`_origin_ok` in raven/rpc/transports/ws.py), which no browser reaching
# this container through a published port or a hostname can send. Rewriting
# it here is what makes the proxy work at all.
# The engine accepts an Origin that names its own loopback port, or one that
# equals the request's Host on a loopback name (`_origin_ok` in
# raven/rpc/transports/ws.py). A browser reaching this container through a
# published port or a hostname sends neither -- behind nginx its Host is the
# hostname it published, and Origin names that host with the published port
# -- so rewriting Origin here is what makes the proxy work at all.
#
# What that gives up is a defence-in-depth layer, not the gate: the session
# cookie is SameSite=Strict, so a page on another site gets no cookie
Expand Down
22 changes: 14 additions & 8 deletions raven/cli/serve_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,9 +411,8 @@ async def _serve_main(port: int, open_browser: bool) -> None:
from raven.rpc.transports.ws import WsGateway, build_app, pick_port

register_console_feature()
# This coroutine binds a port and serves until it is killed, so no test
# runs it; what it schedules is pinned by source in
# tests/test_cli_serve_commands.py instead.
# Startup tests replace this background warm-up to keep provider
# initialization out of the serving probe.
warm_up_in_background() # pragma: no cover

# Declared before anything can emit a span. The page runs on the terminal's
Expand Down Expand Up @@ -486,11 +485,9 @@ async def _serve_main(port: int, open_browser: bool) -> None:
if stack.build_error is not None:
typer.echo(f"warning: agent loop failed to build ({stack.build_error.message}); chat turns will error")
if open_browser:
nonce = gateway.mint_nonce()
auth_url = f"{base_url}/auth#{nonce}"
auth_url = f"{base_url}/auth#{gateway.mint_nonce()}"
typer.echo(f"opening browser: {base_url}")
if not webbrowser.open(auth_url):
typer.echo(f"could not open a browser automatically; visit: {auth_url}")
_open(auth_url)

announcer = asyncio.create_task(_announce_updates(gateway.broadcast, stop))

Expand Down Expand Up @@ -1648,8 +1645,17 @@ def _run_foreground(port: int, dev: bool = False) -> None:


def _open(url: str) -> None:
"""Print the sign-in link, then hand it to a browser.

Printed every time, because ``webbrowser.open`` answers True whenever it ran
something: VS Code's ``browser.sh`` exits 0 whatever happened, and a machine
with no display hands the URL to a console browser that has no JavaScript
and so never redeems it. Printed first, because a console browser holds the
terminal until it is quit.
"""
typer.echo(f"sign in with this link (it works once): {url}")
if not webbrowser.open(url):
typer.echo(f"could not open a browser automatically; visit: {url}")
typer.echo("could not open a browser automatically; open the link above")


def register(app: typer.Typer) -> None:
Expand Down
96 changes: 81 additions & 15 deletions raven/rpc/transports/ws.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
from __future__ import annotations

import asyncio
import ipaddress
import json
import secrets
import time
from collections.abc import Callable
from pathlib import Path
from typing import Any
from urllib.parse import urlsplit

from aiohttp import WSCloseCode, WSMsgType, web
from loguru import logger
Expand All @@ -33,24 +35,40 @@
# the credential used after the one-time exchange.
_NONCE_TTL_S = 30 * 60.0

# The page shows whatever the exchange answered, so the gateway is what says
# why a sign-in was refused; the two refusals below need different remedies.
_AUTH_PAGE = """<!DOCTYPE html>
<html><head><meta charset="utf-8"><title>Raven</title></head>
<body style="font-family:system-ui;display:grid;place-items:center;height:100vh;margin:0">
<p id="s">Signing in to Raven...</p>
<script>
(async () => {
const s = document.getElementById('s');
const nonce = location.hash.slice(1);
history.replaceState(null, '', '/auth');
const r = await fetch('/auth/exchange', {
method: 'POST',
headers: {'Content-Type': 'application/json'},
body: JSON.stringify({nonce}),
});
if (r.ok) { location.replace('/'); }
else { document.getElementById('s').textContent = 'Sign-in link expired. Re-run: raven serve --open'; }
let r;
try {
r = await fetch('/auth/exchange', {
method: 'POST',
headers: {'Content-Type': 'application/json'},
body: JSON.stringify({nonce}),
});
} catch (e) {
s.textContent = 'Raven did not answer. If it has stopped, start it with: raven web';
return;
}
if (r.ok) { location.replace('/'); return; }
const why = (await r.text()).trim();
s.textContent = why || `Sign-in failed (HTTP ${r.status}). Run raven web for a new link.`;
})();
</script></body></html>"""

_REFUSED_ADDRESS = (
"Raven signs in a page opened at 127.0.0.1 or localhost, and this page is somewhere else. "
"Open the sign-in link at one of those addresses; reaching this machine through a port forward is fine."
)
_SPENT_LINK = "This sign-in link was already used or has expired. Run raven web for a new one."

_COOKIE_BASE = "raven_session"


Expand All @@ -73,6 +91,25 @@ def cookie_name(port: int) -> str:
return f"{_COOKIE_BASE}_{port}"


def _loopback_origin(origin: str) -> bool:
"""Whether a browser origin names this machine: ``localhost`` or a loopback IP.

Both parses raise on input a client other than a browser can send:
``urlsplit`` on an IPv6 bracket left open, ``ip_address`` on any name, which
is how the rebinding case lands here. Either one is a refusal, not an error.
"""
try:
hostname = urlsplit(origin).hostname
except ValueError:
return False
if hostname == "localhost":
return True
try:
return hostname is not None and ipaddress.ip_address(hostname).is_loopback
except ValueError:
return False


_COOKIE_MAX_AGE_S = 30 * 24 * 60 * 60
"""How long a browser keeps the session cookie.

Expand Down Expand Up @@ -176,20 +213,49 @@ def _authorized(self, request: web.Request) -> bool:
reachable here, but a credential check that short-circuits on the
first wrong byte is not worth keeping when the fix is free.
"""
cookie = request.cookies.get(cookie_name(self.port))
cookie = request.cookies.get(self._cookie_name(request))
if cookie is not None and secrets.compare_digest(cookie, self.session_cookie):
return True
header = request.headers.get("X-Raven-Token")
return header is not None and secrets.compare_digest(header, self.session_token)

def _cookie_name(self, request: web.Request) -> str:
"""The session cookie's name, for the port the browser reached.

That is the bound port unless a port forward picked another one, and two
machines whose gateways both bound 18792 must not share one jar entry when
one browser reaches them through different forwards. A Host with no port
-- the container's nginx forwards ``$host`` -- keeps the bound port, which
is the name that proxy's auto-login sets. So does a Host whose port does
not parse: this is read before any credential is checked, so a raise here
would answer an unauthenticated request with a 500 instead of a refusal.
"""
try:
port = urlsplit(f"//{request.headers.get('Host', '')}").port
except ValueError:
port = None
return cookie_name(port or self.port)

def _origin_ok(self, request: web.Request) -> bool:
"""Whether a browser request comes from this gateway's own page.

Beyond the bound port's own origin (also what the container's nginx
restates), a page may sit behind a port forward, which picks another
local port whenever the remote one is taken; its origin then names a
port this process never bound. It passes when it asks its own origin
(Origin equals Host) on a loopback name. The first half refuses a page
another local server shows: cookies and SameSite ignore the port, so
that page's requests arrive carrying this session's cookie. The second
refuses DNS rebinding, where a name resolving to 127.0.0.1 asks its own
origin and the two agree.
"""
origin = request.headers.get("Origin")
if origin is None:
return True
return origin in (
f"http://127.0.0.1:{self.port}",
f"http://localhost:{self.port}",
)
if origin in (f"http://127.0.0.1:{self.port}", f"http://localhost:{self.port}"):
return True
host = request.headers.get("Host", "")
return origin.lower() == f"http://{host.lower()}" and _loopback_origin(origin)

# ---- handlers ------------------------------------------------------------

Expand All @@ -214,7 +280,7 @@ async def handle_auth_page(self, request: web.Request) -> web.Response:

async def handle_auth_exchange(self, request: web.Request) -> web.Response:
if not self._origin_ok(request):
raise web.HTTPForbidden(reason="bad origin")
raise web.HTTPForbidden(reason="bad origin", text=_REFUSED_ADDRESS)
try:
body = await request.json()
except Exception:
Expand All @@ -226,10 +292,10 @@ async def handle_auth_exchange(self, request: web.Request) -> web.Response:
token_ok = isinstance(token, str) and secrets.compare_digest(token, self.session_token)
nonce_ok = isinstance(nonce, str) and self.burn_nonce(nonce)
if not (nonce_ok or token_ok):
raise web.HTTPForbidden(reason="invalid or used nonce")
raise web.HTTPForbidden(reason="invalid or used nonce", text=_SPENT_LINK)
resp = web.json_response({"ok": True})
resp.set_cookie(
cookie_name(self.port),
self._cookie_name(request),
self.session_cookie,
httponly=True,
samesite="Strict",
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/test_cli_upgrade_real_uv.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ def test_running_uv_tool_replaces_itself_in_custom_directories(tmp_path: Path, s
"PATH": str(external_tools) + os.pathsep + env["PATH"],
"UV_TOOL_DIR": str(tool_dir),
"UV_TOOL_BIN_DIR": str(bin_dir),
"RAVEN_HOME": str(tmp_path / "raven home"),
"RAVEN_UPGRADE_SOURCE": str(Path(__file__).parents[2]),
"RAVEN_UPGRADE_WHEEL": new_wheel.resolve().as_uri(),
}
Expand Down Expand Up @@ -207,6 +208,7 @@ def companion_present() -> bool:
env=env,
capture_output=True,
text=True,
errors="replace",
timeout=180,
)

Expand All @@ -215,7 +217,7 @@ def companion_present() -> bool:
assert "system certificate store" not in completed.stderr, completed.stderr
# From uv 0.11.9 to 0.12.15 the older name draws a deprecation warning even beside the new one.
assert "UV_NATIVE_TLS" not in completed.stderr, completed.stderr
assert "Raven upgraded: 1.0.0 -> 2.0.0" in completed.stdout
assert "Raven upgraded: 1.0.0 -> 2.0.0" in completed.stdout, completed.stderr
version = subprocess.run(
[str(executable), "--version"],
check=True,
Expand Down
Loading
Loading