Skip to content

fix(dev): complete the contributor Workbench HMR loop through an explicit loopback dev-origin allowlist (#572 §3) - #581

Merged
ScriptedAlchemy merged 7 commits into
mainfrom
fix/572-workbench-hmr-proxy
Sep 5, 2026
Merged

fix(dev): complete the contributor Workbench HMR loop through an explicit loopback dev-origin allowlist (#572 §3)#581
ScriptedAlchemy merged 7 commits into
mainfrom
fix/572-workbench-hmr-proxy

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Closes the Workbench section (§3) of #572. Out of scope here: the MCP Apps compiler path (§1–2), docsite buildCache, version bumps.

Problem

The documented contributor HMR loop — an Rsbuild dev server on http://localhost:3000 proxying /api to a foreground server on http://127.0.0.1:3100 — could not open a Workbench session:

  • GET /api/project/session came back with origin: http://127.0.0.1:3100, so ForegroundRouteClient refused its own bootstrap (AB8003, client-side) and the page stayed on the connection gate.
  • Every POST carried Origin: http://localhost:3000 (Rsbuild's changeOrigin rewrites only Host), so the foreground refused every mutation and the event stream (AB8003, server-side).

Design

An explicit, loopback-only, off-by-default allowlist on the foreground server — not a proxy-side Origin rewrite.

  • ForegroundServerOptions.workbenchDevOrigins / startDevServer({ workbenchDevOrigins }) / agent-bundle dev --workbench-dev-origin <origin> (repeatable). Each value must be a bare http(s) origin on localhost, 127.0.0.1 or [::1] (no path, query, credentials, ws:, non-loopback host); anything else is refused at construction (ForegroundServerError AB8000, before the coordinator starts). The CLI and startDevServer forward the option only when non-empty; []/absent means the default guard.
  • One #assertBrowserOrigin now backs the three browser routes that used to carry the inline check (session bootstrap, mutations via #assertMutationSession, event stream via #assertEventSession): accept when Origin ∈ {this.url} ∪ allowlist, or when Origin is absent and Sec-Fetch-Site: same-origin. With an empty allowlist this is the pre-change predicate, term for term. The Agent API /mcp origin rule (validateOriginHeader + origin === this.url) is untouched.
  • The bootstrap body gains devOrigins: [...] (sorted, deduplicated) only when the allowlist is non-empty; origin stays the foreground URL. The Workbench client accepts the session when location.origin equals body.origin or is listed in a well-formed devOrigins (non-empty array of serialized origins; anything else is AB8019). The exact-keys check still rejects any other extra field.
  • packages/workbench/rsbuild.config.ts: the env-gated proxy branch adds server.strictPort: true, so a busy port 3000 fails loudly instead of Rsbuild silently moving the UI to a port that is not allowlisted. The proxy still does no header rewriting.

Threat model (why this shape, per reference/security.mdx)

The Origin guard protects the loopback foreground server from browser-mediated request forgery by any other origin, including other loopback origins: MCP App sandbox iframes run third-party plugin code on their own loopback ports, and a page on any local port can fetch the foreground URL. Two consequences shaped the fix:

  1. The proxy must not rewrite Origin. Rsbuild's dev server admits every loopback origin through its default CORS policy, so a proxy that stamped the foreground's own origin onto forwarded requests would let any local page launder a request through http://localhost:3000 and arrive looking same-origin. Keeping the browser's real Origin intact means the foreground still decides per origin, and the e2e proves it: through the proxy, Origin: http://127.0.0.1:65500 → 403 AB8003, no Origin at all → 403 AB8003, the listed origin → 200.
  2. The allowlist names exactly the contributor's own Rsbuild origin, and only when the operator says so. It is never derived from an env var the shell might carry around, never inferred from the request, never on by default. Non-loopback values are rejected at startup so the flag cannot be turned into a remote allowlist. devOrigins is disclosed in the bootstrap body only when non-empty, so the default response is byte-identical to before.

What the allowlist does not do: it does not widen Sec-Fetch-Site handling, does not touch the session token / cookie requirement (AB8004 still applies to the listed origin, tested), and does not cover MCP App preview or runtime client-surface iframes — those stay bound to the foreground origin (sandbox hostOrigin, frame-ancestors CSP), and the docs say to open the foreground URL for those pages.

Proof

Automatedpackages/workbench/tests/contributor-hmr.e2e.test.ts (integration pool): reserves a port on localhost, starts the foreground with workbenchDevOrigins: [devOrigin], starts a real Rsbuild dev server from the documented createWorkbenchConfig(foreground.url) (proxy + strictPort retained; only host/port/open/printUrls pinned), waits for the first compile, then in Chrome at 1440×900:

  1. bootstrap through the proxy — Overview renders on the dev origin, no connection gate, Foreground server connected, Current build (so the cookie-authenticated SSE stream connected through the proxy too);
  2. one mutation — clicks Rebuild, asserts POST http://localhost:<port>/api/project/rebuild → 200 with request Origin = dev origin, no alert, artifact active, no page errors;
  3. no laundering — the three Node probes above.

Plus unit coverage: foreground-server matrix (default refuses the dev origin on all three routes; listed origin admitted on all three; nine malformed values → AB8000 before the coordinator starts; loopback spellings accepted, advertised sorted/deduplicated; [] = default), client bootstrap matrix (admit listed, reject unlisted, reject when no devOrigins, six malformed devOriginsAB8019), CLI forwarding + --help, startDevServer forwarding only when non-empty, rsbuild.config proxy/strictPort and no-server-block-without-env.

Manual acceptance (1440×900), exact documented commands from examples/skills-starter:

# Terminal A
node ../../packages/agent-bundle/bin/agent-bundle.js dev --root . --port 3100 --no-open \
  --workbench-dev-origin http://localhost:3000
Development workbench at http://127.0.0.1:3100
# Terminal B
AGENT_BUNDLE_WORKBENCH_API_PROXY=http://127.0.0.1:3100 pnpm --filter agent-bundle-workbench dev
  ➜  Local:    http://localhost:3000/
ready   built in 0.45s

Playwright, Chromium, viewport 1440×900, http://localhost:3000/overview:

page.url() = http://localhost:3000/overview
connection gate headings: 0
status: Foreground server connected
build health: Current build
POST /api/project/rebuild through the proxy -> 200; request Origin header = http://localhost:3000
alerts after rebuild: 0
GET /api/project/session via proxy with Origin http://127.0.0.1:65500 -> 403 {"diagnostic":{"code":"AB8003",...}}
GET /api/project/session via proxy with Origin <none>                -> 403 {"diagnostic":{"code":"AB8003",...}}
GET /api/project/session via proxy with Origin http://localhost:3000 -> 200 {"devOrigins":["http://localhost:3000"],"origin":"http://127.0.0.1:3100"}
page errors: []

Negative acceptance — Terminal A restarted without the flag: the page stays on Foreground connection unavailable (Workbench request failed with HTTP 200. — the client's AB8003 refusal of the mismatched bootstrap origin), and GET /api/project/session through the proxy with Origin: http://localhost:3000 → 403 AB8003. --workbench-dev-origin ftp://localhost:3000agent-bundle dev exits 1: Foreground server Workbench dev origins must be loopback http(s) origins such as http://localhost:3000.

Per-finding resolution (#572 §3)

Finding Resolution
P1 — HMR loop cannot complete a session (mcp-route-client.ts:592, foreground-server.ts:804-827) Explicit loopback allowlist above; automated e2e + manual transcript.
P2 — twelve browser tests fork createRsbuild+pluginReact config One tests/support/workbench-fixture-config.ts (createWorkbenchFixtureConfig({ distRoot, entry }), mode: 'production' pinned so no manual process.env.NODE_ENV define). All twelve rewired, copies deleted; workbench-browser-modules.ts (single remaining importer, one dead export) folded in and deleted; the vestigial extensionAlias for the removed vendored adapter dropped (19/19 without it). Also one availablePort(host) (agent-bundle/tests/support/available-port.ts) replaces the two pre-existing copies plus the one the e2e would have added.
P2 — HMR docs omit URL/port/proxy scope guide/development/workbench.mdx (en+zh): two-terminal loop, http://localhost:3000http://127.0.0.1:3100, --port override, strictPort, what is proxied (/api incl. SSE; not HTML/JS/CSS/HMR socket; iframes stay on the foreground origin), the origin rule, what happens without the flag. reference/cli.mdx (--workbench-dev-origin row), reference/security.mdx (threat-model paragraph), reference/runtime-environment.mdx (AGENT_BUNDLE_WORKBENCH_API_PROXY row), contributing/index.mdx (new section), packages/agent-bundle/README.md.

Files

  • packages/agent-bundle/src/dev/foreground-server.ts, src/dev/workbench-server.ts, src/cli.ts
  • packages/workbench/src/mcp/mcp-route-client.ts, packages/workbench/rsbuild.config.ts
  • tests: agent-bundle/tests/{dev-server,dev-workbench,cli,dev-workbench-packaging}.test.ts, agent-bundle/tests/support/available-port.ts (new), workbench/tests/contributor-hmr.e2e.test.ts (new), workbench/tests/support/workbench-fixture-config.ts (new), workbench/tests/support/{packed-release-harness,workbench-browser-modules(deleted)}.ts, workbench/tests/{mcp-route-client,rsbuild-workbench,packed-release.e2e}.test.ts, twelve rewired browser tests, rstest.integration-tests.ts
  • docs: website/docs/{en,zh}/{guide/development/workbench,reference/cli,reference/security,reference/runtime-environment,contributing/index}.mdx, packages/agent-bundle/README.md
  • .changeset/572-workbench-hmr-proxy.md (agent-bundle patch)

Avoided per the brief: packages/workbench/tests/mcp-app-real.e2e.test.ts (#565) and packages/workbench/src/mcp/mcp-page.tsx. origin/main merged before opening.

Gates (merged state)

pnpm typecheck ✓ · pnpm lint ✓ (0/0) · pnpm test:unit ✓ (3630, includes the foreground-server and route-client suites) · integration pool subset: all 31 packages/workbench/tests/* files in integrationTestFiles + cli.test.ts + dev-workbench.test.ts ✓ (32 files, 162 tests, 0 failed; contributor-hmr.e2e re-run alone ✓) · pnpm docs:site:build ✓ (language parity OK).

Notes / follow-ups (not in this PR)

  • The CLI wraps a ForegroundServerError as an AB5000 diagnostic carrying the AB8000 message (pre-existing diagnosticsFor); the docs say "exits before serving" for the CLI and AB8000 only for startDevServer.
  • The connection gate renders ProjectClientError.message (Workbench request failed with HTTP 200.) and not its code; surfacing the code there would make the no-flag failure self-explanatory. Pre-existing, separate change.

Self-review

Reviewer: change-risk-reviewer was attempted first (GPT-5.6 Sol, medium) but is blocked in this environment — it depends on the TraceDecay MCP daemon, whose socket is down, and it cannot run shell commands. Fallback per AGENTS.md: a generalPurpose reviewer on the same model (GPT-5.6 Sol, medium), read-only, against git diff origin/main...HEAD, asked specifically whether the origin change weakens the default guard.

Findings (5 questions, 0 risks):

  1. Default guard unchanged? — No finding. Old inline predicates and the new #assertBrowserOrigin are equivalent with an empty allowlist, including the missing-Origin + Sec-Fetch-Site: same-origin path; the empty-list bootstrap body preserves the original key order and bytes (foreground-server.ts:780-786 vs baseline :746).
  2. Any non-listed origin admitted when configured? — No finding. isLoopbackBrowserOrigin requires a canonical serialized http(s) origin with host exactly localhost / 127.0.0.1 / ::1 (foreground-server.ts:53-63); the e2e proves the proxy forwards Origin unchanged and manufactures no same-origin provenance (contributor-hmr.e2e.test.ts:141-166). Author spot-check of the validator: http://LOCALHOST:3000, http://localhost:80, http://127.1:3000, http://[0:0:0:0:0:0:0:1]:3000, http://localhost.:3000, http://127.0.0.2:3000, http://localhost:3000#x, null, file://localhost/x all rejected; http://localhost, https://localhost:3000, http://[::1]:3000 accepted.
  3. Behaviour change for existing callers / session contract? — No finding. Option is optional and omitted when empty (workbench-server.ts:927-929, cli.ts:752); the sole session-body parser accepts exactly the two key sets (mcp-route-client.ts:594-607).
  4. Test-support consolidation? — No finding. Fixture profile keeps the required aliases/output and selects production behaviour explicitly (workbench-fixture-config.ts:61-75); all new helpers have importers; the e2e's pool routing matches the header rules in rstest.integration-tests.ts:1-12.
  5. Docs / changeset vs source? — No finding. AB8000 validation runs before the listener starts (foreground-server.ts:427-446); the documented UI strings match project-client.ts:242-245 and main.tsx:1341-1344; en/zh heading counts match; the changeset names only agent-bundle, patch.

Disposition: nothing to fix; no re-run needed. Codex's automatic PR review also completed on beb23e9 with no threads.

…ack dev-origin allowlist (#572 §3)

The documented Workbench HMR loop (Rsbuild dev server on its own port proxying
/api to the foreground server) could not open a session: the bootstrap body's
`origin` never matched the page origin (AB8003 client-side) and every POST
carried the Rsbuild port as `Origin` (AB8003 server-side; `changeOrigin`
rewrites only Host).

- foreground-server: `workbenchDevOrigins` — an explicit, loopback-only
  (`localhost` / `127.0.0.1` / `[::1]`, bare http(s) origin) allowlist, never
  set by default, validated at construction (AB8000). Bootstrap, mutation and
  event-stream routes accept `Origin` ∈ {own URL} ∪ allowlist; the bootstrap
  body discloses the list as `devOrigins` only when non-empty. The Agent API
  `/mcp` origin rule and the missing-Origin/Sec-Fetch-Site rule are unchanged.
- mcp-route-client: accept the session when the page origin equals the
  foreground origin or is listed in a well-formed `devOrigins`.
- cli / startDevServer: `--workbench-dev-origin <origin>` (repeatable) /
  `workbenchDevOrigins`, forwarded only when non-empty.
- rsbuild.config: `server.strictPort: true` in the proxy branch so a busy
  port fails loudly instead of silently moving the UI off the allowlisted one.
- tests: foreground-server Origin matrix, client bootstrap matrix, CLI and
  startDevServer forwarding, and `contributor-hmr.e2e.test.ts` — a real
  Rsbuild dev server with the documented proxy config completing bootstrap,
  the SSE stream and a `POST /api/project/rebuild` through the proxy, plus
  no-laundering probes.
- test support: one `createWorkbenchFixtureConfig` replaces twelve forked
  `createRsbuild`+`pluginReact` configs; one `availablePort(host)` replaces
  three copies.
- docs (en+zh): URL, ports, proxy scope, origin rule, threat model.
@changeset-bot

changeset-bot Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ad25740

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
agent-bundle Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-05T02:05:51.587237Z beb23e9 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pkg-pr-new

pkg-pr-new Bot commented Sep 5, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle@581
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@581
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/rsc-markdown-stream@581
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@581

commit: ad25740

@ScriptedAlchemy
ScriptedAlchemy enabled auto-merge (squash) September 5, 2026 02:23
ScriptedAlchemy and others added 4 commits September 4, 2026 19:41
…-proxy

# Conflicts:
#	packages/workbench/tests/mcp-route-client.test.ts
#	packages/workbench/tests/support/workbench-browser-modules.ts
…-proxy

# Conflicts:
#	packages/workbench/tests/comparisons-page-client-scope-browser.test.ts
#	packages/workbench/tests/discovery-atoms-disposal.test.ts
#	packages/workbench/tests/lifecycles-page.browser.test.tsx
#	packages/workbench/tests/mcp-app-frame.test.ts
#	packages/workbench/tests/mcp-app-preview-browser.test.ts
#	packages/workbench/tests/mcp-json-input.test.ts
#	packages/workbench/tests/mcp-page-app-browser.test.ts
#	packages/workbench/tests/route-editor-atoms-disposal.test.ts
#	packages/workbench/tests/runtime-consent-dialog.test.ts
#	packages/workbench/tests/runtime-document-atoms-disposal.test.ts
#	packages/workbench/tests/runtime-inspector.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant