feat: HTTP proxy primitive for bundles#128
Merged
mgoldsborough merged 4 commits intomainfrom Apr 29, 2026
Merged
Conversation
Bundles can declare _meta["ai.nimblebrain/http-proxy"] in their manifest to expose a local HTTP server (e.g. astro dev, a notebook kernel) to the user's browser through a same-origin route at /v1/apps/<bundle>/<mount>/*. - HttpProxyConfig type + plumbing through LocalBundleMeta and BundleInstance - extractHttpProxy validates target (http/https), mount (single segment, not reserved), and logs a warning on malformed declarations - Workspace.allowHttpProxy flag (default true) — set false to disable all proxy routes in a workspace - proxyRoutes streams HTTP in both directions via Bun's native fetch, strips hop-by-hop headers, and adds standard X-Forwarded-* headers - Bundles receive NB_PROXY_PREFIX env so they can self-configure their upstream server to match the public URL prefix WebSocket upgrade handling is declared in the manifest shape but not yet wired — follow-up. Without it, HMR falls back to full reloads.
The original cut of the http-proxy primitive routed via
`/v1/apps/<bundle>/<mount>/*` and resolved the workspace from an
`X-Workspace-Id` header. That breaks browser iframe loads — the iframe
can't set custom headers, so every preview load 400'd.
Move the workspace into the URL: `/v1/ws/<wsId>/apps/<bundle>/<mount>/*`.
Validate the wsId against `WORKSPACE_ID_RE`, look up the workspace, and
verify membership against the authenticated identity inside the handler.
Mount the proxy route BEFORE other sub-apps in `app.ts` because their
`.use("*")` middleware would otherwise intercept `/v1/ws/*` requests
before our handler runs.
Hardening pass while in here:
- Restrict `target` in `extractHttpProxy` to loopback hosts only
(127.0.0.1, ::1, localhost). The proxy exists for bundles to expose
their own loopback servers; allowing any host turns it into an SSRF
gadget against cloud metadata / RFC1918 / external endpoints with
user credentials attached.
- Strip `Authorization`, `Cookie`, `X-Workspace-Id` from forwarded
requests so the bundle's loopback server can't read user creds.
- Strip `Set-Cookie`, `X-Frame-Options`, `Content-Security-Policy`,
`Content-Encoding`, `Content-Length` from upstream responses. The
encoding/length strip is required because Bun's fetch transparently
decompresses gzip — re-emitting `content-encoding: gzip` makes the
browser try to gunzip already-decoded bytes (corrupted render).
- Set `X-Frame-Options: SAMEORIGIN` on proxy responses and change the
security-headers middleware to honor route-level intent (default
`DENY` only when no value is already set).
- Only attach `duplex: "half"` when forwarding a body — Bun rejects it
for bodyless GET/HEAD.
Bundle-side wiring (`startup.ts`):
- Inject `NB_WORKSPACE_ID` into bundle env so bundles know which
workspace they're spawned for (already known to the platform; just
surfacing it).
- `NB_PROXY_PREFIX` now includes the workspace segment to match the
new route shape (`/v1/ws/<wsId>/apps/<name>/<mount>`). Bundles use
this as their `astro --base` (or equivalent) so absolute URLs in
the rendered page line up with the public path.
- Inject `NB_PUBLIC_ORIGIN` (operator-set, falls back to first
`ALLOWED_ORIGINS` entry) so bundle UI resources can declare the
platform origin in `_meta.ui.csp.{frameDomains,connectDomains}`
without the bundle having to guess.
Logging in the proxy route is now structured (`[proxy] METHOD PATH →
status (reason)`) instead of ad-hoc `console.error` calls, so the
common failure modes (invalid wsId, not a member, mount not declared,
upstream unreachable) are easy to grep in dev.
Trust model is documented at the top of `proxy.ts`: same-origin code
running in the user's session, treat http-proxy bundles like browser
extensions, the operator vouches for the code. The kill switch
(`Workspace.allowHttpProxy = false`) and per-workspace membership check
remain.
3 tasks
… gaps QA feedback on PR #128 surfaced six issues. None were exploitable on the current code path (the trust model assumes vouched-for bundles), but several were defenses-in-comment-only or tomorrow-bugs. X-Forwarded-For The original code wrote `XFF` to itself (`set(name, get(name) ?? "")`), which was dead code AND let a browser-supplied `X-Forwarded-For` value flow to the bundle's loopback server unchanged. Bundles run on loopback and don't need XFF; if they did, browser-supplied is unverifiable. Add `x-forwarded-for` to `REQUEST_HEADERS_STRIPPED` and delete the self-assignment. Content-encoding handling The previous response strip removed `content-encoding` and `content-length` unconditionally on the assumption that "Bun's fetch transparently decompresses gzipped responses." That assumption holds today for gzip and probably deflate, but couples our correctness to Bun's per-encoding behavior — silently corrupts the body the day Bun adds `br` or `zstd` and we don't update the strip set. Cleaner architecture: strip `Accept-Encoding` from the forwarded request so upstream returns identity-encoded bodies. Identity in, identity out, no encoding state to track. Loopback bandwidth cost is negligible. Drop the response-side `content-encoding` / `content-length` strips. Trust-model header Two gaps the QA review noted, both now in the comment: - `XFF` stripping is documented as a defense. - Dev auth mode (no IdentityProvider configured) leaves request identity undefined and the membership check is a no-op. This matches every other workspace-scoped route on the platform, but the previous header read as if membership was always enforced. Reserved-mount list `RESERVED_PROXY_MOUNTS` (`resources`, `tools`, `mcp`, `events`) was a guard against collision with `/v1/apps/<bundle>/<segment>` from the pre-hardening route shape. The current shape is `/v1/ws/<wsId>/apps/<bundle>/<mount>/*` and never collides — the list guards nothing today. Delete it. If a future route shape collides, re-add with a comment naming the collision. Stale path comment in `types.ts` Doc comments on `HttpProxyConfig.{target, mount}` referred to the old `/v1/apps/<bundle>/<mount>/*` shape. Updated to current. CHANGELOG Added an `[Unreleased] → Added` entry — new env vars (`NB_WORKSPACE_ID`, `NB_PROXY_PREFIX`, `NB_PUBLIC_ORIGIN`), new `Workspace.allowHttpProxy` field, and the route shape are all operator-relevant. Test coverage for these defenses lands in the next commit.
Adds the test coverage that should have shipped with the http-proxy
primitive. Closes the QA-flagged gap of "200-line security-critical
handler with zero unit tests."
Unit (test/unit/bundles/extract-http-proxy.test.ts) — 16 cases against
`extractHttpProxy` covering every rejection branch:
- meta absent / not an object / null / non-string types
- target missing / mount missing / target unparseable
- non-http(s) protocols (file:, ftp:, javascript:)
- SSRF attempts: example.com, 169.254.169.254 (AWS metadata),
RFC1918 (10.0.0.1, 172.16.0.1, 192.168.1.1), 0.0.0.0
- All three loopback hostnames accepted (127.0.0.1, ::1, localhost)
- Loopback check is case-insensitive
- Mount with embedded slashes rejected
- Mount normalization (leading/trailing slash trim, empty after trim)
- websocket defaults to false; only strict `true` enables it
- Original target string preserved verbatim
Integration (test/integration/api/proxy-route.test.ts) — one test per
defense in the trust-model header, plus header-behavior coverage:
- 400 invalid wsId (path-traversal guard via WORKSPACE_ID_RE)
- 400 unknown workspace
- 403 authenticated identity not a member of workspace
- 403 workspace.allowHttpProxy === false (kill switch)
- 404 no bundle instance for workspace
- 404 bundle exists but mount doesn't match its declaration
- 502 upstream unreachable
- Forwards full path + query string verbatim
- Forwards request method (DELETE) and body (POST)
- Strips Authorization, Cookie, X-Workspace-Id, X-Forwarded-For from
forward; sets Accept-Encoding: identity (proves the strip-and-set
pair works against Bun's fetch default-injection)
- Sets X-Forwarded-Host and X-Forwarded-Proto on forward
- Strips Set-Cookie, X-Frame-Options, CSP from upstream response
- Sets X-Frame-Options: SAMEORIGIN on success
- Preserves upstream status codes and body bytes
The integration test reaches into `lifecycle.instances` via `as any` to
register a fake `BundleInstance` with an `HttpProxyConfig` pointing at
a `Bun.serve` test upstream. Lighter than spawning a real bundle
subprocess and isolates the test surface to exactly the proxy route's
contract.
Source change in this commit: `Accept-Encoding: identity` is now SET
explicitly after the strip loop, not just stripped. The strip alone
isn't sufficient — Bun's outbound `fetch` auto-injects an
Accept-Encoding default when the header is absent, which would
re-introduce the encoding-mismatch class of bug. Test discovered this
behavior; trust-model and inline comments updated to reflect the
final mechanism.
Contributor
Author
|
QA feedback addressed in two follow-up commits:
Notable architectural decisions per the review thread: X-Forwarded-For — added to `REQUEST_HEADERS_STRIPPED` rather than rewritten. Browser-supplied XFF is unverifiable; bundles run on loopback and don't need it. If a future use case needs it, we'll set it from a server-side source. Content-encoding — moved from response-side strip to request-side `Accept-Encoding: identity`. The previous strip relied on "Bun's fetch decompresses gzip" — true today but couples our correctness to Bun's per-encoding behavior. Identity in, identity out, no encoding state to track. Discovered via the integration test that simply stripping isn't enough: Bun's outbound `fetch` auto-injects `Accept-Encoding: gzip, deflate, br, zstd` when absent. We now SET `identity` explicitly to override that default. Updated the trust-model comment to reflect the actual mechanism. Reserved-mount list — deleted. With workspace-in-URL, `/v1/ws//apps//resources` doesn't collide with `/v1/resources`, so the guard had no purpose. If a future route shape collides, we'll re-add with a comment naming the collision. Dev-mode bypass — left as-is (consistent with every other workspace-scoped route on the platform), but documented in the trust-model header so operators don't assume membership is always enforced. CHANGELOG — added under `[Unreleased] → Added` with the operator-relevant bits (route shape, env vars, kill switch). `bun run verify` clean: 2141 unit + 213 web + integration + 17 smoke pass, 0 fail. |
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
1 participant
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.
Summary
Adds an opt-in same-origin HTTP proxy so a bundle can expose its own loopback HTTP server (e.g.
astro preview, a Jupyter kernel) inside the platform UI via/v1/ws/<wsId>/apps/<bundle>/<mount>/*. The bundle declares_meta["ai.nimblebrain/http-proxy"]in its manifest; the platform proxies browser requests to its loopback server with credentials stripped, X-Frame-Options set to SAMEORIGIN, and per-workspace kill switch.Driven by
synapse-apps/synapse-astro-editor, which uses it to embed a built Astro site in the editor preview iframe — but the primitive is generic.Two commits
eb07bee— initial route + manifest plumbingd6c18e1— workspace-in-URL refactor + hardening pass (header strip, gzip handling, X-Frame default, loopback restriction, NB_PUBLIC_ORIGIN env injection, Hono ordering fix)Trust model
Documented at the top of
src/api/routes/proxy.ts:allow-scripts allow-same-origin. Treat these bundles like browser extensions.Workspace.allowHttpProxy = falsekill switch.Why workspace in the URL
Browser iframe loads can't set custom headers, so the original
X-Workspace-Idheader pattern 400'd every preview load. Workspace ID lives in the URL path now, validated withWORKSPACE_ID_REand membership-checked per request inside the handler. Proxy route is mounted before other sub-apps inapp.tsbecause their.use("*", requireWorkspace)middleware would otherwise intercept/v1/ws/*.Bundle env
Bundles get three new env vars at spawn:
NB_WORKSPACE_ID— which workspace this instance is forNB_PROXY_PREFIX—/v1/ws/<wsId>/apps/<name>/<mount>for use asastro --baseor equivalentNB_PUBLIC_ORIGIN— operator-set platform origin so bundles can declare it in_meta.ui.csp.{frameDomains,connectDomains}on their UI resourcesTest plan
bun run verify— 418 unit/integration + 17 smoke pass, 0 failsynapse-astro-editor: bundle loads target site (Bayze) with full styling in editor preview iframe; edits trigger rebuild; preview reflects changestargetis rejected with warning at manifest parseallowHttpProxy: falseon workspace returns 403