Codeman/docs/web-tabs-fixes-plan.md

# Web tabs: two fixes (planned + implemented 2026-07-28)

Both found against the saved dashboard
`https://macminis-mac-mini.tailf80371.ts.net:4000` (Bio-Hacking-Dashboard).
Kept because the root-cause analysis of the second one is not obvious from the
resulting diff.

Status: **both implemented and verified end-to-end.** The one deliberate
non-change is recorded at the bottom.

---

## Bug 1: saved URLs could not be deleted from the Run dropdown

### What happened

The "Web / URL" section of the Run dropdown listed every saved dashboard as a
single clickable row whose only action was "open". Deleting required opening the
dashboard as a tab, clicking the tab's gear, then Delete in the modal, so a URL
you no longer wanted open at all could not be removed without first opening it.

### What shipped

- `renderWebviewMenuItems()` (`src/web/public/webview-tabs.js`) now renders each
  saved URL as a `.run-mode-row--web` flex row: the open button, a gear
  (`showWebviewModal`), and an `x` (`deleteWebviewById`). Nested buttons are
  invalid HTML, hence the wrapper rather than a button inside a button.
- `deleteWebview()` split into the modal entry point, the new row entry point
  `deleteWebviewById(id)`, and the shared `_confirmAndDeleteWebview(id)`.
- Both side buttons call `event.stopPropagation()` so the click does not also
  open the dashboard.
- The dropdown's outside-click handler (`session-ui.js`) closes when the click
  target is not inside `#runModeMenu`, and the row is gone by the time the delete
  resolves, so `deleteWebviewById` re-asserts `.active` on the menu. Verified in a
  browser: deleting one of several URLs leaves you looking at the rest of the list.
- CSS in `styles.css` (`.run-mode-row--web`, `.run-mode-row-btn`) plus a larger
  touch target in `mobile.css`. The side buttons are permanently visible rather
  than hover-revealed, because this menu is used on touch.

No server change: `DELETE /api/webviews/:id` already existed, owner-scoped, and
already revoked the capability and broadcast `WebviewChanged`.

---

## Bug 2: images did not load in a proxied dashboard

### Reproduction (before the fix)

```
CAP=<from POST /api/webviews/<id>/open>
# A) upstream direct                       -> 200 image/jpeg 118150
curl -sk "https://macminis-mac-mini.tailf80371.ts.net:4000/api/hero?slug=120-minutes-in-nature"
# B) through the proxy prefix              -> 200 image/jpeg 118150
curl -sk "https://localhost:3000/webview/$CAP/api/hero?slug=120-minutes-in-nature"
# C) what the browser ACTUALLY requested   -> 404 {"errorCode":"NOT_FOUND"}
curl -sk -H "Referer: https://localhost:3000/webview/$CAP/" \
     "https://localhost:3000/api/hero?slug=120-minutes-in-nature"
# D) same shape but NOT under /api         -> 200 (referer fallback rescues it)
curl -sk -H "Referer: https://localhost:3000/webview/$CAP/" "https://localhost:3000/styles.css"
```

The proxy itself was fine (B). The failure was entirely about which URL the
browser ended up requesting (C).

### Root cause

The dashboard builds its image markup at runtime with root-absolute URLs:
`c.innerHTML = '<img class="thumb" src="/api/hero?slug=...">'`, `img.src =
slideSrc(...)` returning `/api/slide?owner=...`, `/api/story`, `/api/video`, and a
nested `<iframe src="/api/preview?slug=...">`.

All three rewrite layers missed that shape:

1. `<base href="/webview/<cap>/">` only affects **relative** URLs. A root-absolute
   `/api/hero` ignores the base path and resolves against Codeman's origin.
2. `rewriteHtml()` only runs over the **initial HTML document**. This markup is
   created later by page script. (The static header `<img src="/api/logo">` DID
   work, having been rewritten at proxy time, which is why only the
   runtime-injected images were broken.)
3. `runtimeUrlShim()` patched only `fetch`, `XMLHttpRequest.open`, `WebSocket` and
   `EventSource`, so the dashboard's **data** loaded while its **pictures** did
   not.

The safety net was fenced off from `/api` in two places, both deliberate:
`server.ts`'s not-found handler returns the API-envelope 404 before reaching
`tryWebviewRefererFallback`, and `middleware/auth.ts` refuses the Referer-form
auth exemption for `/api/`, `/ws/`, `/q/`.

### What shipped

`runtimeUrlShim()` in `src/web/webview-proxy.ts` now also covers the DOM sinks, so
a root-absolute `/api/...` request is never emitted in the first place and neither
security fence had to move:

- `innerHTML` / `outerHTML` / `insertAdjacentHTML` (and `ShadowRoot.innerHTML`),
- `setAttribute` / `setAttributeNS`,
- the `src`/`srcset`/`href`/`poster`/`data`/`action` property setters on img,
  source, media, video poster, script, iframe, embed, track, link, anchor, area,
  object and form,
- a `MutationObserver` as a last net for any sink not patched above (it costs one
  wasted 404 per node, since the browser starts fetching on insert, so it is a net
  and not the mechanism).

Two details that mattered:

- Every rewrite routes through the existing idempotent `rw()` rather than a blind
  prefix concat. The first draft used the server-side regex shape and
  double-prefixed markup that was already proxied (a page re-injecting its own
  `outerHTML`); the jsdom test caught it.
- Everything stays inside `try`/`catch` and is marked `__cmrw`, so a double
  injection cannot wrap an already-wrapped setter, and nothing can throw into a
  page we do not control.

### Verification

- `test/webview-proxy.test.ts` gained a jsdom `runtimeUrlShim DOM sinks` block:
  innerHTML, insertAdjacentHTML, property setters, setAttribute, srcset candidate
  lists, the MutationObserver net via an unpatched sink
  (`createContextualFragment`), idempotence, re-injected markup, empty `src`, and
  the pass-throughs (relative, cross-origin, `#hash`, `data:`). 73 tests pass.
- End-to-end in a real browser against an isolated instance
  (`CODEMAN_INSTANCE=wvtest`, port 3151), with prod's old build as the negative
  control:

  | | before (prod, old build) | after (fixed) |
  | --- | --- | --- |
  | images found | 693 | 693 |
  | src under the proxy prefix | 0 | 693 |
  | in-viewport images decoded | 0 / 23 | 23 / 23 |
  | sample src | `/api/hero?slug=...` | `/webview/<cap>/api/hero?slug=...` |

  (The dashboard marks thumbs `loading="lazy"`, so only in-viewport images are
  ever fetched. All 27 proxied image responses returned 200.)

---

## Deliberately NOT done: widening the `/api` referer fallback

Considered as defense in depth, and skipped. It would have to change **both**
gates or it is useless on a password-protected instance, and the auth gate is the
security-sensitive one: auth runs in `onRequest`, before routing, so it cannot
tell a real Codeman API route from a 404. Dropping the `/api` fence would let a
page holding a capability forge a `Referer` and reach Codeman's **real** API
unauthenticated. Doing it safely means exempting only paths that match no
registered route (via Fastify's `hasRoute`/`findRoute`), which is a separate
change deserving its own review and its own cases in
`test/webview-auth-exemption.test.ts`.

The remaining gap this leaves is narrow and documented in `docs/web-tabs.md`: a
root-absolute `url(/img.png)` inside a stylesheet injected at runtime. Non-`/api`
ones are already rescued by the existing referer fallback.