Skip to content

refactor(playground): derive buildHonoProxy mount prefix from label#394

Open
LissaGreense wants to merge 1 commit into
mainfrom
chore/buildhonoproxy-derive-prefix-from-label
Open

refactor(playground): derive buildHonoProxy mount prefix from label#394
LissaGreense wants to merge 1 commit into
mainfrom
chore/buildhonoproxy-derive-prefix-from-label

Conversation

@LissaGreense
Copy link
Copy Markdown
Contributor

Summary

buildHonoProxy(prefix, upstream, label) took both prefix and label where, by construction at both call sites, prefix === \"/api/\" + label:

.all(\"/api/daemon/*\", buildHonoProxy(\"/api/daemon\", DAEMON_URL, \"daemon\"))
.all(\"/api/tunnel/*\", buildHonoProxy(\"/api/tunnel\", TUNNEL_URL, \"tunnel\"))

The invariant was implicit and unenforced. A mismatched mount (e.g. buildHonoProxy(\"/foo\", url, \"daemon\")) would silently strip /foo from the request path while still stamping x-forwarded-prefix: /api/daemon on the upstream — surfacing downstream as wrong external URLs (OAuth callbacks, etc.) with nothing failing loudly at the mount point.

Drop the prefix parameter; derive it inside the function as /api/\${label}. Single source of truth, invariant enforced by construction.

Also swap replace(new RegExp(prefix), \"\") for a literal startsWith + slice so the prefix can't be reinterpreted as a regex pattern (no exploit today with hardcoded labels, but the regex shape is a general footgun).

Closes #316.

Scope

  • tools/agent-playground/src/lib/server/proxy.ts — signature change + regex → literal swap
  • tools/agent-playground/src/lib/server/static-app.ts — both call sites updated
  • tools/agent-playground/src/lib/server/proxy.hono.test.ts — existing 2 tests updated for new signature; added a third tunnel-label case that pins the x-forwarded-prefix / stripped-path single-source-of-truth property

Risks

  • Behavioral: none under the only two call sites — both used the /api/\${label} invariant already, so the derived prefix produces the identical strip + forward header.
  • Signature break: internal API only, no external consumers. The function is exported from one file and called from one other (and tests). `grep -rn buildHonoProxy` confirms.
  • Regex → startsWith: identical behavior on the production inputs ("/api/daemon", "/api/tunnel"). Strictly safer because metacharacters in the prefix can no longer alter matching.
  • Coordination: PR Move agent-playground → apps/studio-ui #293 is renaming `tools/agent-playground` → `apps/studio-ui`. Either order works — whichever lands first, the other rebases trivially (three small file edits, no semantic conflict).

Test plan

  • `npx vitest run src/lib/server/proxy.hono.test.ts` — 3/3 pass (existing 2 + new tunnel-label case)
  • `npx vitest run src/lib/server/` — 79 pass / 1 pre-existing unrelated flake in `mcp.test.ts` (Deno.openKv unavailable in vitest env, untouched by this diff)
  • `deno check` clean on proxy.ts + static-app.ts

buildHonoProxy took both `prefix` and `label`, where `prefix` was used
to strip the mount path off the incoming request and `label` was used
by executeProxyFetch to stamp `x-forwarded-prefix: /api/${label}`. By
construction the two always agreed (`prefix === "/api/" + label`) at
both call sites in static-app.ts, but the invariant was implicit and
unenforced. A mismatched mount (e.g. `buildHonoProxy("/foo", url,
"daemon")`) would silently send `x-forwarded-prefix: /api/daemon` to
the upstream while stripping `/foo` from the path — a latent footgun
surfacing downstream as wrong external URLs (OAuth callbacks, etc.).

Drop the `prefix` parameter; derive it inside the function as
`/api/${label}`. Single source of truth, invariant enforced by
construction. Also swap `replace(new RegExp(prefix), "")` for a
literal `startsWith + slice` so the prefix can't be reinterpreted as
a regex.

Updates both call sites in static-app.ts and the wrapper-specific
tests in proxy.hono.test.ts; adds a tunnel-label case that pins
the X-Forwarded-Prefix / stripped-path single-source-of-truth
property directly.

Closes #316
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.

buildHonoProxy: redundant prefix/label params can silently diverge

1 participant