fix(dev): complete the contributor Workbench HMR loop through an explicit loopback dev-origin allowlist (#572 §3) - #581
Merged
Conversation
…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 detectedLatest commit: ad25740 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
ScriptedAlchemy
enabled auto-merge (squash)
September 5, 2026 02:23
…-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
This was referenced Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:3000proxying/apito a foreground server onhttp://127.0.0.1:3100— could not open a Workbench session:GET /api/project/sessioncame back withorigin: http://127.0.0.1:3100, soForegroundRouteClientrefused its own bootstrap (AB8003, client-side) and the page stayed on the connection gate.POSTcarriedOrigin: http://localhost:3000(Rsbuild'schangeOriginrewrites onlyHost), 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
Originrewrite.ForegroundServerOptions.workbenchDevOrigins/startDevServer({ workbenchDevOrigins })/agent-bundle dev --workbench-dev-origin <origin>(repeatable). Each value must be a barehttp(s)origin onlocalhost,127.0.0.1or[::1](no path, query, credentials,ws:, non-loopback host); anything else is refused at construction (ForegroundServerErrorAB8000, before the coordinator starts). The CLI andstartDevServerforward the option only when non-empty;[]/absent means the default guard.#assertBrowserOriginnow backs the three browser routes that used to carry the inline check (session bootstrap, mutations via#assertMutationSession, event stream via#assertEventSession): accept whenOrigin ∈ {this.url} ∪ allowlist, or whenOriginis absent andSec-Fetch-Site: same-origin. With an empty allowlist this is the pre-change predicate, term for term. The Agent API/mcporigin rule (validateOriginHeader+origin === this.url) is untouched.devOrigins: [...](sorted, deduplicated) only when the allowlist is non-empty;originstays the foreground URL. The Workbench client accepts the session whenlocation.originequalsbody.originor is listed in a well-formeddevOrigins(non-empty array of serialized origins; anything else isAB8019). The exact-keys check still rejects any other extra field.packages/workbench/rsbuild.config.ts: the env-gated proxy branch addsserver.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
Originguard 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 canfetchthe foreground URL. Two consequences shaped the fix: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 throughhttp://localhost:3000and arrive looking same-origin. Keeping the browser's realOriginintact means the foreground still decides per origin, and the e2e proves it: through the proxy,Origin: http://127.0.0.1:65500→ 403AB8003, noOriginat all → 403AB8003, the listed origin → 200.devOriginsis 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-Sitehandling, does not touch the session token / cookie requirement (AB8004still 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 (sandboxhostOrigin,frame-ancestorsCSP), and the docs say to open the foreground URL for those pages.Proof
Automated —
packages/workbench/tests/contributor-hmr.e2e.test.ts(integration pool): reserves a port onlocalhost, starts the foreground withworkbenchDevOrigins: [devOrigin], starts a real Rsbuild dev server from the documentedcreateWorkbenchConfig(foreground.url)(proxy +strictPortretained; only host/port/open/printUrlspinned), waits for the first compile, then in Chrome at 1440×900:Foreground server connected,Current build(so the cookie-authenticated SSE stream connected through the proxy too);POST http://localhost:<port>/api/project/rebuild→ 200 with requestOrigin= dev origin, no alert, artifactactive, no page errors;Plus unit coverage: foreground-server matrix (default refuses the dev origin on all three routes; listed origin admitted on all three; nine malformed values →
AB8000before the coordinator starts; loopback spellings accepted, advertised sorted/deduplicated;[]= default), client bootstrap matrix (admit listed, reject unlisted, reject when nodevOrigins, six malformeddevOrigins→AB8019), CLI forwarding +--help,startDevServerforwarding only when non-empty,rsbuild.configproxy/strictPort and no-server-block-without-env.Manual acceptance (1440×900), exact documented commands from
examples/skills-starter:Playwright, Chromium, viewport 1440×900,
http://localhost:3000/overview:Negative acceptance — Terminal A restarted without the flag: the page stays on
Foreground connection unavailable(Workbench request failed with HTTP 200.— the client'sAB8003refusal of the mismatched bootstrap origin), andGET /api/project/sessionthrough the proxy withOrigin: http://localhost:3000→ 403AB8003.--workbench-dev-origin ftp://localhost:3000→agent-bundle devexits 1:Foreground server Workbench dev origins must be loopback http(s) origins such as http://localhost:3000.Per-finding resolution (#572 §3)
mcp-route-client.ts:592,foreground-server.ts:804-827)createRsbuild+pluginReactconfigtests/support/workbench-fixture-config.ts(createWorkbenchFixtureConfig({ distRoot, entry }),mode: 'production'pinned so no manualprocess.env.NODE_ENVdefine). All twelve rewired, copies deleted;workbench-browser-modules.ts(single remaining importer, one dead export) folded in and deleted; the vestigialextensionAliasfor the removed vendored adapter dropped (19/19 without it). Also oneavailablePort(host)(agent-bundle/tests/support/available-port.ts) replaces the two pre-existing copies plus the one the e2e would have added.guide/development/workbench.mdx(en+zh): two-terminal loop,http://localhost:3000↔http://127.0.0.1:3100,--portoverride,strictPort, what is proxied (/apiincl. 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-originrow),reference/security.mdx(threat-model paragraph),reference/runtime-environment.mdx(AGENT_BUNDLE_WORKBENCH_API_PROXYrow),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.tspackages/workbench/src/mcp/mcp-route-client.ts,packages/workbench/rsbuild.config.tsagent-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.tswebsite/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-bundlepatch)Avoided per the brief:
packages/workbench/tests/mcp-app-real.e2e.test.ts(#565) andpackages/workbench/src/mcp/mcp-page.tsx.origin/mainmerged 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 31packages/workbench/tests/*files inintegrationTestFiles+cli.test.ts+dev-workbench.test.ts✓ (32 files, 162 tests, 0 failed;contributor-hmr.e2ere-run alone ✓) ·pnpm docs:site:build✓ (language parity OK).Notes / follow-ups (not in this PR)
ForegroundServerErroras anAB5000diagnostic carrying theAB8000message (pre-existingdiagnosticsFor); the docs say "exits before serving" for the CLI andAB8000only forstartDevServer.ProjectClientError.message(Workbench request failed with HTTP 200.) and not itscode; surfacing the code there would make the no-flag failure self-explanatory. Pre-existing, separate change.Self-review
Reviewer:
change-risk-reviewerwas 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: ageneralPurposereviewer on the same model (GPT-5.6 Sol, medium), read-only, againstgit diff origin/main...HEAD, asked specifically whether the origin change weakens the default guard.Findings (5 questions, 0 risks):
#assertBrowserOriginare equivalent with an empty allowlist, including the missing-Origin+Sec-Fetch-Site: same-originpath; the empty-list bootstrap body preserves the original key order and bytes (foreground-server.ts:780-786vs baseline:746).isLoopbackBrowserOriginrequires a canonical serializedhttp(s)origin with host exactlylocalhost/127.0.0.1/::1(foreground-server.ts:53-63); the e2e proves the proxy forwardsOriginunchanged 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/xall rejected;http://localhost,https://localhost:3000,http://[::1]:3000accepted.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).workbench-fixture-config.ts:61-75); all new helpers have importers; the e2e's pool routing matches the header rules inrstest.integration-tests.ts:1-12.foreground-server.ts:427-446); the documented UI strings matchproject-client.ts:242-245andmain.tsx:1341-1344; en/zh heading counts match; the changeset names onlyagent-bundle,patch.Disposition: nothing to fix; no re-run needed. Codex's automatic PR review also completed on
beb23e9with no threads.