diff --git a/crates/aisix-core/src/models/passthrough_route.rs b/crates/aisix-core/src/models/passthrough_route.rs index 350bee4a5..5e9f9593c 100644 --- a/crates/aisix-core/src/models/passthrough_route.rs +++ b/crates/aisix-core/src/models/passthrough_route.rs @@ -23,11 +23,23 @@ 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 `/` 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. + /// 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, + /// 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 +302,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 +564,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..973508e2d 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`. 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 e9676fe1f..7e4af06c5 100644 --- a/tests/e2e/src/cases/passthrough-route-e2e.test.ts +++ b/tests/e2e/src/cases/passthrough-route-e2e.test.ts @@ -302,6 +302,80 @@ describe("passthrough-route e2e: explicit routes, BYO credentials, 410 tombstone expect(noKeyBody.error?.message).not.toContain("Authorization"); }); + test("a prefixed host route claims a reserved namespace and mirrors the 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). + // + // 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" }, + }); + upstreams.push(upstream); + + await seed.createPassthroughRoute({ + name: "ptr-mirror-mcp", + hosts: ["agent-upstream.example.com"], + path_prefix: "/mcp", + target_url: upstream.baseUrl, + auth_mode: "header_key", + auth_header_name: "x-aisix-api-key", + credential_mode: "forward_client", + }); + + 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 }), + }); + + await waitConfigPropagation(async () => { + try { + const r = await call(); + const ok = r.statusCode === 200; + await r.body.text(); + return ok; + } catch { + return false; + } + }); + + const baseline = upstream.receivedRequests.length; + const res = await call(); + 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) => { if (!etcdReachable || !app || !seed) { ctx.skip();