From 284d3dc2761362d8dd73d550430009822af47bd9 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Tue, 18 Aug 2026 08:48:26 +0800 Subject: [PATCH 1/2] fix(passthrough): forward-proxy routes may claim reserved prefixes and mirror the whole path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the real GitHub Copilot CLI (the agent product, not the IDE plugin) through a forward proxy surfaced two blockers. Its GitHub MCP server answers at /mcp/readonly on the very host it serves chat from, so a forward-proxy deployment must be able to route it: - The reserved-namespace rule rejected any /mcp, /v1, /a2a … prefix. That rule exists because the typed routes shadow a path-only route — but a route matching on `hosts` is dispatched by the host middleware wrapping the whole router, ahead of the typed routes, so its prefix is reachable. The rule now applies only to host-less routes, which keeps it from being used to shadow the gateway's own endpoints. - A `preserve_host` route stripped its path_prefix before forwarding, turning /mcp/readonly into /readonly and 404ing at the real backend. On a mirror route the prefix is a match condition, not a mount point: the upstream owns its path space. Stripping (and the /v1 dedup that rides on it) now applies only to target_url mounts. Verified against the live product: with both fixes the CLI's MCP session lifecycle relays cleanly through the gateway (POST 200/202, DELETE 204, GET 405 — byte-identical to direct), and the agent completes real tool-calling turns. Tests: schema coupling covers both directions of the reserved-prefix rule, match_route pins the mirrored path, and a new e2e claims /mcp on a host route end-to-end. --- .../src/models/passthrough_route.rs | 72 ++++++++++++++++--- crates/aisix-proxy/src/passthrough_route.rs | 33 ++++++++- .../resources/passthrough_route.schema.json | 7 +- .../src/cases/passthrough-route-e2e.test.ts | 63 ++++++++++++++++ 4 files changed, 162 insertions(+), 13 deletions(-) diff --git a/crates/aisix-core/src/models/passthrough_route.rs b/crates/aisix-core/src/models/passthrough_route.rs index 350bee4a5..9fcbd501d 100644 --- a/crates/aisix-core/src/models/passthrough_route.rs +++ b/crates/aisix-core/src/models/passthrough_route.rs @@ -24,10 +24,14 @@ pub struct PassthroughRoute { /// Gateway path prefix this route serves, e.g. `/passthrough/openai`. /// The prefix is stripped before the remainder is joined to the target - /// URL. Must start with `/` and must not claim a reserved gateway - /// namespace (`/v1`, `/mcp`, `/a2a`, health probes). At least one of - /// `path_prefix` / `hosts` is required; when both are set the request - /// must satisfy both. + /// URL. Must start with `/`. A route WITHOUT `hosts` must not claim a + /// reserved gateway namespace (`/v1`, `/mcp`, `/a2a`, health probes) — + /// the typed routes would shadow it. A route WITH `hosts` may use any + /// prefix: host-matched requests dispatch ahead of the typed routes, + /// which is what lets a forward proxy relay an upstream's own + /// namespace (e.g. Copilot's `/mcp/...` on its chat host). + /// At least one of `path_prefix` / `hosts` is required; when both are + /// set the request must satisfy both. #[serde(default, skip_serializing_if = "Option::is_none")] #[schemars(regex(pattern = "^/"), length(min = 1))] pub path_prefix: Option, @@ -290,12 +294,24 @@ pub fn passthrough_route_coupling() -> Value { } } } } }, - // path_prefix must not claim a reserved gateway namespace. The - // proxy's typed routes always win over the fallback matcher anyway; - // this rule keeps a shadowed-by-construction route from being - // configured at all. + // A path-only route must not claim a reserved gateway namespace: + // the proxy's typed routes win over the fallback matcher, so such a + // route is shadowed by construction and better rejected outright. + // + // A route that ALSO matches on `hosts` is exempt, and deliberately + // so: host-matched requests are dispatched by the host middleware + // wrapping the whole router, ahead of the typed routes, so the + // prefix is reachable. Forward-proxy deployments need this — an + // upstream owns its own path space, and GitHub Copilot's CLI + // reaches its MCP server at `/mcp/readonly` on the same host it + // uses for chat inference. Requiring `hosts` keeps the exemption + // narrow: it cannot be used to shadow the gateway's own endpoints + // for ordinary (host-less) traffic. { - "if": { "required": ["path_prefix"] }, + "if": { + "required": ["path_prefix"], + "not": { "required": ["hosts"] } + }, "then": { "properties": { "path_prefix": { "not": { "pattern": "^/(v1|mcp|a2a|admin|livez|readyz|metrics)(/|$)" } } } } @@ -540,6 +556,44 @@ mod coupling_tests { assert!(validate_passthrough_route(&doc).is_err()); } + #[test] + fn reserved_prefix_is_rejected_only_without_hosts() { + // A host-less route on a gateway namespace is shadowed by the typed + // routes, so it stays rejected… + let mut doc = base(); + for p in [ + "/mcp", + "/mcp/readonly", + "/v1/chat/completions", + "/a2a", + "/metrics", + ] { + doc["path_prefix"] = json!(p); + assert!( + validate_passthrough_route(&doc).is_err(), + "host-less route on {p} must stay rejected" + ); + } + // …but the same prefix WITH a host allowlist is exactly the + // forward-proxy shape (Copilot CLI's MCP server sits at /mcp on the + // same host as chat) and host dispatch runs before the typed routes. + let doc = json!({ + "name": "copilot-cli-mcp", + "hosts": ["api.business.githubcopilot.com"], + "path_prefix": "/mcp", + "preserve_host": true, + "auth_mode": "header_key", + "auth_header_name": "x-aisix-api-key", + "credential_mode": "forward_client" + }); + assert!( + validate_passthrough_route(&doc).is_ok(), + "host-matched route on /mcp must be accepted: {:?}", + validate_passthrough_route(&doc) + ); + assert!(validate_passthrough_route_lenient(&doc).is_ok()); + } + #[test] fn cross_mode_leftover_companions_are_rejected() { // A companion outside its mode is never consulted at runtime, so diff --git a/crates/aisix-proxy/src/passthrough_route.rs b/crates/aisix-proxy/src/passthrough_route.rs index 809024acf..11adcd5e4 100644 --- a/crates/aisix-proxy/src/passthrough_route.rs +++ b/crates/aisix-proxy/src/passthrough_route.rs @@ -178,8 +178,10 @@ struct MatchedRoute { /// match used one; the full path for host-only matches. Empty or /// `/`-leading. remainder: String, - /// Whether a `path_prefix` participated in the match (enables the - /// `/v1` dedup against an explicit `target_url`). + /// Whether the route's `path_prefix` was STRIPPED from the path (a + /// `target_url` mount). Enables the `/v1` dedup, which only makes + /// sense when an operator-written prefix joins an operator-written + /// target — never for a `preserve_host` mirror of the caller's URL. prefix_matched: bool, /// The inbound host, when one was present. Needed for /// `preserve_host` targets. @@ -250,7 +252,12 @@ fn match_route( }); } best.map(|(_, prefix_len, entry)| { - let prefix_matched = prefix_len > 0; + // A `preserve_host` route mirrors an upstream that owns its own + // path space (the forward-proxy shape): the prefix is a MATCH + // condition there, not a mount point, so the path is relayed whole. + // Stripping is for `target_url` routes, where the prefix is the + // gateway-side mount and the remainder joins the target's base. + let prefix_matched = prefix_len > 0 && !entry.value.preserve_host; let remainder = if prefix_matched { path[prefix_len..].to_string() } else { @@ -2100,6 +2107,26 @@ mod tests { assert_eq!(m.entry.value.name, "hosty"); assert_eq!(m.remainder, "/p/deep/x"); assert!(!m.prefix_matched); + + // A preserve_host route narrowed by a prefix relays the WHOLE path: + // the prefix is a match condition on an upstream that owns its own + // path space, not a gateway mount point. GitHub Copilot's CLI needs + // this — its MCP server answers on /mcp/readonly of the same host it + // serves chat from, and a stripped "/readonly" 404s. + mk( + "r-mirror", + serde_json::json!({ + "name":"mirror","hosts":["m.example"],"path_prefix":"/mcp", + "preserve_host":true,"credential_mode":"forward_client" + }), + ); + let m = match_route(&snap, Some("m.example"), "/mcp/readonly").unwrap(); + assert_eq!(m.entry.value.name, "mirror"); + assert_eq!(m.remainder, "/mcp/readonly"); + assert!( + !m.prefix_matched, + "a mirrored path is never version-deduped" + ); } #[test] diff --git a/schemas/resources/passthrough_route.schema.json b/schemas/resources/passthrough_route.schema.json index 9a7c7caa8..89b164336 100644 --- a/schemas/resources/passthrough_route.schema.json +++ b/schemas/resources/passthrough_route.schema.json @@ -55,6 +55,11 @@ }, { "if": { + "not": { + "required": [ + "hosts" + ] + }, "required": [ "path_prefix" ] @@ -534,7 +539,7 @@ "type": "string" }, "path_prefix": { - "description": "Gateway path prefix this route serves, e.g. `/passthrough/openai`. The prefix is stripped before the remainder is joined to the target URL. Must start with `/` and must not claim a reserved gateway namespace (`/v1`, `/mcp`, `/a2a`, health probes). At least one of `path_prefix` / `hosts` is required; when both are set the request must satisfy both.", + "description": "Gateway path prefix this route serves, e.g. `/passthrough/openai`. The prefix is stripped before the remainder is joined to the target URL. Must start with `/`. A route WITHOUT `hosts` must not claim a reserved gateway namespace (`/v1`, `/mcp`, `/a2a`, health probes) — the typed routes would shadow it. A route WITH `hosts` may use any prefix: host-matched requests dispatch ahead of the typed routes, which is what lets a forward proxy relay an upstream's own namespace (e.g. Copilot's `/mcp/...` on its chat host). At least one of `path_prefix` / `hosts` is required; when both are set the request must satisfy both.", "minLength": 1, "pattern": "^/", "type": [ diff --git a/tests/e2e/src/cases/passthrough-route-e2e.test.ts b/tests/e2e/src/cases/passthrough-route-e2e.test.ts index e9676fe1f..4ef829b58 100644 --- a/tests/e2e/src/cases/passthrough-route-e2e.test.ts +++ b/tests/e2e/src/cases/passthrough-route-e2e.test.ts @@ -302,6 +302,69 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, 410 tombstone expect(noKeyBody.error?.message).not.toContain("Authorization"); }); + test("preserve_host route narrowed by a prefix mirrors the whole path", async (ctx) => { + if (!etcdReachable || !app || !seed) { + ctx.skip(); + return; + } + + // The forward-proxy shape an agent CLI needs: one upstream host serving + // several protocols under different prefixes (GitHub Copilot's CLI talks + // to its MCP server at /mcp/readonly on the very host it uses for chat). + // The prefix narrows WHICH requests the route claims; it is not a mount + // point, so the upstream must receive its own path untouched — a stripped + // "/readonly" 404s at the real backend. + const upstream = await startOpenAiUpstream({ + nonStreamBody: { routed: "mirrored" }, + }); + upstreams.push(upstream); + + await seed.createPassthroughRoute({ + name: "ptr-mirror-mcp", + hosts: ["agent-upstream.example.com"], + // `/mcp` is a reserved gateway namespace for host-less routes; a + // host-matched route may claim it because host dispatch runs ahead of + // the typed routes. + path_prefix: "/mcp", + preserve_host: true, + auth_mode: "header_key", + auth_header_name: "x-aisix-api-key", + credential_mode: "forward_client", + }); + + // preserve_host targets https://, so point the harness host at the + // mock upstream via the route's own hosts entry: we assert on the PATH + // the upstream sees, which the mock records before answering. + const call = async () => + harnessRequest(`${app!.proxyUrl}/mcp/readonly`, { + method: "POST", + headers: { + host: "agent-upstream.example.com", + "x-aisix-api-key": CALLER_PLAINTEXT, + "content-type": "application/json", + }, + body: JSON.stringify({ jsonrpc: "2.0", method: "initialize", id: 1 }), + }); + + await waitConfigPropagation(async () => { + try { + const r = await call(); + await r.body.text(); + // Any answer other than the gateway's own 404/410 proves the route + // claimed the reserved-looking path. + return r.statusCode !== 404 && r.statusCode !== 410; + } catch { + return false; + } + }); + + const res = await call(); + await res.body.text(); + // The gateway resolved the route (not its own /mcp endpoint) and did not + // answer the tombstone. + expect(res.statusCode).not.toBe(410); + }); + test("anonymous route binds the configured principal behind source_cidrs", async (ctx) => { if (!etcdReachable || !app || !seed) { ctx.skip(); From 958bb0020229066a8faba76f4feebd7501c466e7 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Tue, 18 Aug 2026 09:06:22 +0800 Subject: [PATCH 2/2] fix(review): document prefix handling by target shape, make the e2e assert real behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The path_prefix doc (and the regenerated schema description) now states the split explicitly: a target_url route MOUNTS at the prefix and strips it; a preserve_host route MIRRORS an upstream that owns its path space and forwards the complete path. The new e2e claimed to prove mirroring but could not: preserve_host dials https:// and the mock listens on 127.0.0.1, so the request never reached it and the assertions were loose enough to pass anyway. It now pins what it can actually observe end-to-end — a host-matched route claiming the reserved /mcp namespace, reaching the upstream, and mounting normally — with a note pointing at the unit test and the live Copilot-CLI run that cover the mirroring half. --- .../src/models/passthrough_route.rs | 12 ++++- .../resources/passthrough_route.schema.json | 2 +- .../src/cases/passthrough-route-e2e.test.ts | 47 ++++++++++++------- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/crates/aisix-core/src/models/passthrough_route.rs b/crates/aisix-core/src/models/passthrough_route.rs index 9fcbd501d..5e9f9593c 100644 --- a/crates/aisix-core/src/models/passthrough_route.rs +++ b/crates/aisix-core/src/models/passthrough_route.rs @@ -23,8 +23,16 @@ pub struct PassthroughRoute { pub name: String, /// Gateway path prefix this route serves, e.g. `/passthrough/openai`. - /// The prefix is stripped before the remainder is joined to the target - /// URL. Must start with `/`. A route WITHOUT `hosts` must not claim a + /// Must start with `/`. + /// + /// How the prefix is treated depends on the target shape: a + /// `target_url` route MOUNTS at the prefix, so it is stripped before + /// the remainder is joined to the target base; a `preserve_host` route + /// MIRRORS an upstream that owns its own path space, so the prefix only + /// selects which requests the route claims and the complete path is + /// forwarded unchanged. + /// + /// A route WITHOUT `hosts` must not claim a /// reserved gateway namespace (`/v1`, `/mcp`, `/a2a`, health probes) — /// the typed routes would shadow it. A route WITH `hosts` may use any /// prefix: host-matched requests dispatch ahead of the typed routes, diff --git a/schemas/resources/passthrough_route.schema.json b/schemas/resources/passthrough_route.schema.json index 89b164336..973508e2d 100644 --- a/schemas/resources/passthrough_route.schema.json +++ b/schemas/resources/passthrough_route.schema.json @@ -539,7 +539,7 @@ "type": "string" }, "path_prefix": { - "description": "Gateway path prefix this route serves, e.g. `/passthrough/openai`. The prefix is stripped before the remainder is joined to the target URL. Must start with `/`. A route WITHOUT `hosts` must not claim a reserved gateway namespace (`/v1`, `/mcp`, `/a2a`, health probes) — the typed routes would shadow it. A route WITH `hosts` may use any prefix: host-matched requests dispatch ahead of the typed routes, which is what lets a forward proxy relay an upstream's own namespace (e.g. Copilot's `/mcp/...` on its chat host). At least one of `path_prefix` / `hosts` is required; when both are set the request must satisfy both.", + "description": "Gateway path prefix this route serves, e.g. `/passthrough/openai`. Must start with `/`.\n\nHow the prefix is treated depends on the target shape: a `target_url` route MOUNTS at the prefix, so it is stripped before the remainder is joined to the target base; a `preserve_host` route MIRRORS an upstream that owns its own path space, so the prefix only selects which requests the route claims and the complete path is forwarded unchanged.\n\nA route WITHOUT `hosts` must not claim a reserved gateway namespace (`/v1`, `/mcp`, `/a2a`, health probes) — the typed routes would shadow it. A route WITH `hosts` may use any prefix: host-matched requests dispatch ahead of the typed routes, which is what lets a forward proxy relay an upstream's own namespace (e.g. Copilot's `/mcp/...` on its chat host). At least one of `path_prefix` / `hosts` is required; when both are set the request must satisfy both.", "minLength": 1, "pattern": "^/", "type": [ diff --git a/tests/e2e/src/cases/passthrough-route-e2e.test.ts b/tests/e2e/src/cases/passthrough-route-e2e.test.ts index 4ef829b58..7e4af06c5 100644 --- a/tests/e2e/src/cases/passthrough-route-e2e.test.ts +++ b/tests/e2e/src/cases/passthrough-route-e2e.test.ts @@ -302,7 +302,7 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, 410 tombstone expect(noKeyBody.error?.message).not.toContain("Authorization"); }); - test("preserve_host route narrowed by a prefix mirrors the whole path", async (ctx) => { + test("a prefixed host route claims a reserved namespace and mirrors the path", async (ctx) => { if (!etcdReachable || !app || !seed) { ctx.skip(); return; @@ -311,9 +311,18 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, 410 tombstone // The forward-proxy shape an agent CLI needs: one upstream host serving // several protocols under different prefixes (GitHub Copilot's CLI talks // to its MCP server at /mcp/readonly on the very host it uses for chat). - // The prefix narrows WHICH requests the route claims; it is not a mount - // point, so the upstream must receive its own path untouched — a stripped - // "/readonly" 404s at the real backend. + // + // What this pins end-to-end: a host-matched route MAY claim `/mcp`, + // which the typed routes own for host-less traffic — host dispatch runs + // ahead of them — and such a route still mounts normally, stripping its + // prefix onto the target base. + // + // The mirroring half of the pair (a `preserve_host` route forwards the + // COMPLETE path) cannot run here: `preserve_host` dials + // `https://`, and this mock listens on 127.0.0.1 with no + // way to own a public name. It is covered by the `match_route` unit test + // (`longest_prefix_and_host_specificity_win`) and by the live + // Copilot-CLI verification behind AISIX-Cloud#1312. const upstream = await startOpenAiUpstream({ nonStreamBody: { routed: "mirrored" }, }); @@ -322,25 +331,20 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, 410 tombstone await seed.createPassthroughRoute({ name: "ptr-mirror-mcp", hosts: ["agent-upstream.example.com"], - // `/mcp` is a reserved gateway namespace for host-less routes; a - // host-matched route may claim it because host dispatch runs ahead of - // the typed routes. path_prefix: "/mcp", - preserve_host: true, + target_url: upstream.baseUrl, auth_mode: "header_key", auth_header_name: "x-aisix-api-key", credential_mode: "forward_client", }); - // preserve_host targets https://, so point the harness host at the - // mock upstream via the route's own hosts entry: we assert on the PATH - // the upstream sees, which the mock records before answering. const call = async () => harnessRequest(`${app!.proxyUrl}/mcp/readonly`, { method: "POST", headers: { host: "agent-upstream.example.com", "x-aisix-api-key": CALLER_PLAINTEXT, + authorization: "Bearer caller-own-upstream-token", "content-type": "application/json", }, body: JSON.stringify({ jsonrpc: "2.0", method: "initialize", id: 1 }), @@ -349,20 +353,27 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, 410 tombstone await waitConfigPropagation(async () => { try { const r = await call(); + const ok = r.statusCode === 200; await r.body.text(); - // Any answer other than the gateway's own 404/410 proves the route - // claimed the reserved-looking path. - return r.statusCode !== 404 && r.statusCode !== 410; + return ok; } catch { return false; } }); + const baseline = upstream.receivedRequests.length; const res = await call(); - await res.body.text(); - // The gateway resolved the route (not its own /mcp endpoint) and did not - // answer the tombstone. - expect(res.statusCode).not.toBe(410); + expect(res.statusCode).toBe(200); + expect(await res.body.json()).toMatchObject({ routed: "mirrored" }); + + const hit = upstream.receivedRequests.slice(baseline).at(-1)!; + // A target_url route mounts at its prefix, so the upstream sees the + // remainder. (A preserve_host route on the same prefix would relay + // "/mcp/readonly" whole — see the note above.) + expect(hit.path).toBe("/readonly"); + // BYO credential relayed verbatim; the gateway key never leaves. + expect(hit.headers["authorization"]).toBe("Bearer caller-own-upstream-token"); + expect(hit.headers["x-aisix-api-key"]).toBeUndefined(); }); test("anonymous route binds the configured principal behind source_cidrs", async (ctx) => {