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
7 changes: 7 additions & 0 deletions .changes/planning/fix-view-bridge-hardening-6109a1c4.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- Harden the session bridge: the watcher exits after 40 consecutive empty waits (exit 4) so an orphaned watcher no longer keeps the server alive, the view bridge gates the wait and lease-release endpoints with a watcher-only token that the page never receives, and U+2028 and U+2029 are escaped in output lines. The README now states the single-user-workstation limit.
7 changes: 7 additions & 0 deletions .changes/review/fix-view-bridge-hardening-6109b2d5.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- Harden the view bridge: the watcher exits after 40 consecutive empty waits (exit 4) so an orphaned watcher no longer keeps the server alive, the wait and lease-release endpoints need a watcher-only token that the page never receives, and U+2028 and U+2029 are escaped in output lines.
7 changes: 7 additions & 0 deletions .changes/work-items/fix-view-bridge-hardening-6109c3e6.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- Harden the triage board's view bridge: the watcher exits after 40 consecutive empty waits (exit 4) so an orphaned watcher no longer keeps the server alive, the wait and lease-release endpoints need a watcher-only token that the page never receives, and U+2028 and U+2029 are escaped in output lines.
33 changes: 25 additions & 8 deletions lib/session-bridge/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ subclass sets `max_body` and `page_csp` and adds routes in `route_get(url, query
sent, must match.
- A per-run token from `secrets.token_urlsafe(32)` rides only in the `X-<Name>-Token` header; every
POST and `/api/wait` needs it. Every POST must be `application/json`, 1 to `max_body` bytes, a
JSON object. `watch.sh` hands the header to curl on stdin (`-H @-`), so the token is in no
JSON object. An app may set a separate `watch_token` on its hub: it then alone gates `/api/wait` and
the lease release, is written to the env file as `WATCH_TOKEN` and to the session json as
`watchToken`, and is never served to the page (without one, the page token gates them too).
`watch.sh` hands the header to curl on stdin (`-H @-`), so the token is in no
process's argv, which other local users can read on Linux without `hidepid` and on macOS.
- On Windows the listener refuses address reuse and binds with `SO_EXCLUSIVEADDRUSE`, so no other
local process can bind the same port and receive requests that carry the token.
Expand All @@ -73,8 +76,16 @@ name for `ensure-running`, `stop` and `lease [--release]`. On events it prints o
`dataDir` and `next` (the re-arm command, `bash '<here>/wake.sh' '<data_dir>'`), stores the seq in
`.watch-seq` and the replayed seq in `.watch-replay`, and exits 0. It exits 2 when curl, the conf
or the env file is missing, the token is rejected, or the server stays unreachable, and 3 when
another watcher holds the lease, its lease was released, or the server was stopped. The watcher id
is `WATCH_ID`, else `CLAUDE_CODE_SESSION_ID`, else `<hostname>-<parent pid>`.
another watcher holds the lease, its lease was released, or the server was stopped. It exits 4 after
`WAIT_EMPTY` consecutive waits with no event (default 40, about an hour at the default 90 s wait),
printing the re-arm command: a watcher orphaned by a crashed session ends, and the server expires.
A live session re-arms it. There is no parent-pid check, since the parent pid is 1 for a Claude Code
Bash shell on Windows. The watcher id is `WATCH_ID`, else `CLAUDE_CODE_SESSION_ID`, else
`<hostname>-<parent pid>`. It sends `WATCH_TOKEN` from the env file, else `TOKEN`.

Output lines (the `/api/wait` body and the channel server's messages) are `json.dumps` with
`ensure_ascii=False`, then U+2028 and U+2029 are escaped (`dumps_line`), so no reader splits a line
on them.

## The client half

Expand Down Expand Up @@ -102,9 +113,14 @@ carries: an action a page asks for passes the same confirm or permission gate it
prints `{url, origin, page, watch}`.
2. The adopter builds its page with `--connect <origin>` into `page`. The builder adds
`connect-src <origin>` to the page's policy and nothing else (rendered-views rule 3).
3. The reader opens `url`. The runtime fetches the token from `GET /api/token`, which answers only
a request with `Sec-Fetch-Site: same-origin` and a matching `Host` and `Origin`. The token never
enters the page's markup, so a saved or published copy holds none.
3. The reader opens `url`. The runtime fetches the page token from `GET /api/token`, which answers
only a request with `Sec-Fetch-Site: same-origin` and a matching `Host` and `Origin`. The token
never enters the page's markup, so a saved or published copy holds none. Those checks stop a
browser page from another origin; they do not stop a non-browser local client such as curl, which
can send the same headers, so any local process can fetch the page token. The bridge assumes a
single-user workstation: another OS user on the same machine can reach the loopback port. The
page token opens `POST /api/action` only. `/api/wait` and the lease release need the watcher
token, which lives in the 0600 session env file and is never served by `/api/token`.
4. `POST /api/action` takes `{action, picked, choices, notes}`: `action` and every choice are builder
keys (`^[a-z0-9-]{1,32}$`), `picked` holds builder row ids, and `notes` holds the reader's text,
at most 20 entries of 4000 characters. Any other field or shape is a 400.
Expand All @@ -122,7 +138,8 @@ clears the watcher lease.
The server also ends on its own once no watcher wait has been in flight for `IDLE_SECONDS` (600, the
lease timeout; `--idle-seconds` overrides it), and clears its session files. The watcher is a
background task of the session that armed it, so a session that ends stops polling and its token
stops working about 600 seconds after its last wait. A session that leaves the watcher down that long
stops working about 600 seconds after its last wait. (A watcher orphaned by a crash is not that
case: it exits itself after `WAIT_EMPTY` empty waits, see The watcher.) A session that leaves the watcher down that long
loses the server too: `watch` then exits 2 asking for `ensure-running`, which starts a server with a
new token on the same port, and the reader reloads the page.

Expand All @@ -132,7 +149,7 @@ new token on the same port, and the reader reloads the page.
|---|---|
| The page sends only reader input and builder ids | `view-runtime.js` sends picked row ids, option keys and textarea text; `view_bridge.py` refuses anything else |
| Page fields reach the session as data, never as the user's message | `DATA_NOTE` on every wait answer, delivered as background-task output (Delivery as data) |
| An unguessable per-session token authenticates every message | `secrets.token_urlsafe(32)` per server run, required on every POST and wait, handed only to same-origin page script, gone when the server stops; the server stops by itself about 600 s after the session's watcher last waited, so the token expires with the session |
| An unguessable per-session token authenticates every message | `secrets.token_urlsafe(32)` per server run, required on every POST (the page token) and wait (the separate watcher token, never served to the page), handed only to same-origin page script (a browser-only check: a local process can fetch it), gone when the server stops; the server stops by itself about 600 s after the session's watcher last waited, so the token expires with the session |
| No message triggers a gated action without its gate | The bridge and the view app store and deliver; nothing in either acts on an event |

## The channels adapter
Expand Down
58 changes: 45 additions & 13 deletions lib/session-bridge/session_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ def token_header(name):
return f"X-{name[:1].upper()}{name[1:]}-Token"


def dumps_line(value):
"""json.dumps for a line a reader splits on line separators: U+2028 and U+2029 stay escaped.

ensure_ascii=False keeps other non-ASCII text readable; those two are valid raw in JSON
(RFC 8259 section 7) but end a line in some readers.
"""
return (
json.dumps(value, ensure_ascii=False)
.replace("
", "\\u2028")
.replace("
", "\\u2029")
)


def replace_into(tmp, path):
"""os.replace with retry: Windows refuses while a reader holds the target open."""
for _ in range(20):
Expand Down Expand Up @@ -171,6 +184,9 @@ def __init__(self, port, data_dir):
self.url = f"http://127.0.0.1:{port}/"
self.dir = Path(data_dir).resolve()
self.token = secrets.token_urlsafe(32)
# Gates /api/wait and the lease release. None means the page token also gates them; an
# app whose page is untrusted script sets its own, which is never served to the page.
self._watch_token = None
# Differs on every start; the page tells a restart by it.
self.instance = secrets.token_hex(6)
self.cond = threading.Condition()
Expand All @@ -183,6 +199,14 @@ def __init__(self, port, data_dir):
# The one watcher allowed: {watcher, since, last, inflight, pid}. In memory, so a restart frees it.
self.lease = None

@property
def watch_token(self):
return self._watch_token or self.token

@watch_token.setter
def watch_token(self, value):
self._watch_token = value

@staticmethod
def lease_live(lease, now, timeout):
"""A lease holds while its watcher has a wait in flight or its last wait ended within timeout."""
Expand Down Expand Up @@ -397,7 +421,7 @@ def send(self, code, body, ctype="application/json", csp=None):
raw = (
body
if isinstance(body, bytes)
else json.dumps(body, ensure_ascii=False).encode("utf-8")
else dumps_line(body).encode("utf-8")
)
if code >= 400:
# An unread request body must not become the next request.
Expand Down Expand Up @@ -443,6 +467,11 @@ def token_ok(self):
given = self.headers.get(token_header(self.hub.name)) or ""
return secrets.compare_digest(given, self.hub.token)

def watch_token_ok(self):
"""The watcher's token, which gates /api/wait and the lease release."""
given = self.headers.get(token_header(self.hub.name)) or ""
return secrets.compare_digest(given, self.hub.watch_token)

def do_GET(self):
if not self.origin_ok():
return self.send(403, {"error": "bad host or origin"})
Expand Down Expand Up @@ -471,7 +500,7 @@ def route_get(self, url, query):
self.send(404, {"error": "not found"})

def long_poll(self, query):
if not self.token_ok():
if not self.watch_token_ok():
return self.send(403, {"error": "token required"})
try:
after = (query.get("after") or ["0"])[0]
Expand Down Expand Up @@ -561,7 +590,8 @@ def do_POST(self):
if not self.origin_ok():
return self.send(403, {"error": "bad host or origin"})
url = urlparse(self.path)
if not self.token_ok():
gate = self.watch_token_ok if url.path == "/api/lease" else self.token_ok
if not gate():
return self.send(403, {"error": "token required"})
if (
not self.headers.get("Content-Type", "")
Expand Down Expand Up @@ -647,12 +677,14 @@ def start(handler, make_hub, port, nonce=""):
"nonce": nonce,
"startedAt": now_iso(),
}
env = f"PID={pid}\nPORT={port}\nTOKEN={hub.token}\n"
# Only an app's own watcher token is written; without one, readers fall back to the page token.
if hub._watch_token:
session["watchToken"] = hub._watch_token
env += f"WATCH_TOKEN={hub._watch_token}\n"
json_name, env_name = session_files(hub.name)
write_private(hub.dir / json_name, json.dumps(session, indent=2) + "\n")
write_private(
hub.dir / env_name,
f"PID={pid}\nPORT={port}\nTOKEN={hub.token}\nNONCE={nonce}\n",
)
write_private(hub.dir / env_name, f"{env}NONCE={nonce}\n")
return httpd, hub


Expand Down Expand Up @@ -830,7 +862,7 @@ def release_lease(s, name):
body=json.dumps({"action": "release"}),
headers={
"Content-Type": "application/json",
token_header(name): s["token"],
token_header(name): s.get("watchToken") or s["token"],
},
)
resp = conn.getresponse()
Expand Down Expand Up @@ -1240,11 +1272,11 @@ def __init__(self, data_dir, name, control_cmd, ring, watcher):
self.last_deliver = 0.0

def session(self):
"""(port, token) from the data dir's session env file, or None."""
"""(port, watcher token) from the data dir's session env file, or None."""
try:
text = (self.dir / session_files(self.name)[1]).read_text(encoding="utf-8")
env = dict(line.split("=", 1) for line in text.splitlines() if "=" in line)
return int(env["PORT"]), env["TOKEN"]
return int(env["PORT"]), env.get("WATCH_TOKEN") or env["TOKEN"]
except (OSError, KeyError, ValueError):
return None

Expand Down Expand Up @@ -1292,7 +1324,7 @@ def release(self):
s = self.session()
if s is not None:
try:
release_lease({"port": s[0], "token": s[1]}, self.name)
release_lease({"port": s[0], "watchToken": s[1]}, self.name)
except (OSError, http.client.HTTPException):
pass

Expand Down Expand Up @@ -1473,7 +1505,7 @@ def __init__(self, here, out):

def send(self, msg):
with self.lock:
self.out.write(json.dumps(msg, ensure_ascii=False) + "\n")
self.out.write(dumps_line(msg) + "\n")
self.out.flush()

def ring(self, relay, text, **meta):
Expand Down Expand Up @@ -1523,7 +1555,7 @@ def tool_watch(self, args):
return f"watching {d}: a channel event announces new page events; read them with the events tool"

def tool_events(self, args):
return json.dumps(self.relay_for(args).take(), ensure_ascii=False)
return dumps_line(self.relay_for(args).take())

def tool_unwatch(self, args):
relay = self.relay_for(args)
Expand Down
52 changes: 52 additions & 0 deletions lib/session-bridge/test_session_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,34 @@ def test_names_follow_the_app_name(self):
self.assertEqual(sb.token_header("interview"), "X-Interview-Token")


class TestLineSeparators(BridgeCase):
def test_a_wait_answer_escapes_u2028_and_u2029_but_keeps_other_unicode(self):
text = "a
b
c é"
self.hub.append(text)
conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=TIMEOUT)
try:
conn.request(
"GET",
"/api/wait?after=handled&timeout=1",
headers={self.header: self.hub.token},
)
raw = conn.getresponse().read().decode("utf-8")
finally:
conn.close()
self.assertNotIn("
", raw)
self.assertNotIn("
", raw)
self.assertIn("\\u2028", raw)
self.assertIn("\\u2029", raw)
self.assertIn("é", raw)
self.assertEqual(json.loads(raw)["events"][0]["text"], text)

def test_dumps_line_round_trips(self):
value = {"t": "x
y
z"}
line = sb.dumps_line(value)
self.assertEqual(len(line.splitlines()), 1)
self.assertEqual(json.loads(line), value)


class TestSessionFiles(BridgeCase):
def test_session_files_record_port_pid_and_token(self):
s = json.loads((self.dir / ".toy-session.json").read_text(encoding="utf-8"))
Expand Down Expand Up @@ -472,6 +500,21 @@ def test_a_held_lease_exits_3_naming_the_holder(self):
self.assertIn("holds this toy's lease: session other,", r.stderr)
t.join(TIMEOUT)

def test_an_orphaned_watcher_exits_4_after_n_empty_waits(self):
sb.set_wait_timeout(self.dir, "toy", 1)
r = self.run_script("watch.sh", str(self.dir), env={"WAIT_EMPTY": "2"})
self.assertEqual(r.returncode, 4, r.stderr)
self.assertEqual(r.stdout, "")
self.assertIn("2 consecutive waits", r.stderr)
self.assertIn(f"bash '{self.bin}/wake.sh'", r.stderr.replace("\\", "/"))

def test_an_event_before_the_limit_is_delivered(self):
sb.set_wait_timeout(self.dir, "toy", 1)
threading.Timer(0.3, self.hub.append, ["late"]).start()
r = self.run_script("watch.sh", str(self.dir), env={"WAIT_EMPTY": "5"})
self.assertEqual(r.returncode, 0, r.stderr)
self.assertEqual(json.loads(r.stdout)["events"][0]["text"], "late")

def test_no_env_file_names_the_control_script(self):
r = self.run_script("watch.sh", str(self.tmp))
self.assertEqual(r.returncode, 2)
Expand Down Expand Up @@ -1063,6 +1106,15 @@ def test_a_hostile_holder_never_reaches_the_ring(self):
self.assertNotIn(piece, dumped)
self.assertNotIn(piece, stderr)

def test_a_message_with_u2028_and_u2029_is_written_escaped(self):
self.server.send({"jsonrpc": "2.0", "params": {"content": "a
b
c é"}})
written = self.out.getvalue()
self.assertNotIn("
", written)
self.assertNotIn("
", written)
self.assertIn("é", written)
(line,) = written.splitlines()
self.assertEqual(json.loads(line)["params"]["content"], "a
b
c é")

def test_a_held_lease_rings_fixed_text_and_logs_a_plain_holder(self):
conflict = sb.Conflict({"error": "lease held", "holder": "other-session.1"})
with unittest.mock.patch.object(self.relay, "wait", side_effect=conflict):
Expand Down
Loading
Loading