feat(config): forward caller identity to HTTP backends, opt-in per server by luisaniceto-infra · Pull Request #110 · avelino/mcp

luisaniceto-infra · 2026-09-02T21:36:37Z

Closes #109.

O proxy autentica o caller e aplica a ACL, mas fala com o backend usando uma credencial de serviço resolvida no boot. Modelo certo pra Sentry/Slack/Databricks — o dado não tem dono por linha. Deixa de ser certo quando o dado é de alguém: aí toda escrita cai com o mesmo nome, e não falha alto — registra o dono errado, em silêncio.

"hub": {
  "url": "http://hub.internal/mcp",
  "headers": { "Authorization": "Bearer ${HUB_SERVICE_TOKEN}" },
  "forward_identity": { "header": "X-MCP-Subject" }
}

O subject vai por request, a partir da identidade que o dispatch já resolveu. roles_header é opcional e desligado por padrão — a maioria dos backends só precisa saber quem, e mandar roles que o backend não lê é exposição evitável.

Escopo, deliberado

Só HTTP. Backend stdio/CLI é processo do proxy; não existe canal por-request pra carregar header num stdin de vida longa.
Opt-in, nunca default. Backend que nunca pediu identidade não pode começar a receber uma num upgrade.
O header encaminhado ganha de uma anotação x-mcp-header com o mesmo nome, e a derivada do argumento é descartada, não anexada depois — header duplicado quem resolve é o receptor, e essa decisão não é nossa. Sem isso, uma tool declara um argumento mapeado pra X-MCP-Subject e o caller escolhe o próprio subject.
Caractere de controle no subject/roles recusa a chamada. Cair de volta na credencial compartilhada seria exatamente o "dono errado em silêncio" que a feature remove.
Um só check de injeção: is_tchar virou pub(crate) e o caminho novo usa o MESMO predicado do x-mcp-header. Duas cópias é como uma delas diverge.

A condição que o proxy não consegue verificar

O backend não pode ser alcançável por quem consiga setar esse header. Se for, o header é bypass total de autenticação — o caller simplesmente diz que é outra pessoa.

Na prática: o backend aceita só do endereço do proxy, por link privado. É por isso que a feature é opt-in por servidor em vez de ligada por padrão. Está escrito no doc do tipo, na referência de config e no código.

Testes

Seguindo o checklist do CONTRIBUTING pra mudança em header confiável — casos de negação em maior número que os de aceitação:

Construtor de headers (8): sem config → nada; backend stdio → nada; servidor desconhecido → nada; header default; nome custom + roles; CRLF no subject → recusa; caractere de controle em roles → recusa; quatro nomes de header malformados ("", com espaço, com :, com CRLF) → recusa.

Client (2): anotação x-mcp-header tentando sombrear X-MCP-Subject → a identidade encaminhada é a única que sai (escalação de privilégio); e sem identidade encaminhada o comportamento do x-mcp-header fica idêntico ao de antes (é aditivo).

cargo test  → 822 passed; 0 failed
cargo fmt --all --check  → limpo
cargo clippy --all-targets  → sem warning novo

Os 2 warnings de result_large_err que o clippy mostra já existiam na main (conferi com a árvore limpa) — não mexi neles pra não misturar refactor com feature.

Notas de revisão

identity_headers fica no ProxyServer e roda na fase 1 do dispatch, sob o mesmo lock que já resolve a rota: config e caller estão os dois em mão, e a fase 3 segue sem lock.
call_tool_raw continua com a assinatura de antes; a variante nova é call_tool_raw_with. Nada de churn em call site existente.
Um único ponto de compatibilidade: ServerConfig::Http ganhou campo, então as duas construções literais em src/output.rs (testes) ganharam forward_identity: None.

Se preferir outro nome pro campo (forwardIdentity, identity_forwarding) ou quiser o roles_header fora por enquanto, mudo — o desenho aguenta os dois.

…rver The proxy authenticates the caller and enforces the ACL, then talks to every backend with one credential resolved at startup. That is the right model when the backend's data has no per-row owner — Sentry, Slack, Databricks: the shared token IS the service account, and the proxy decides who may reach it. It stops being the right model when the backend's data is owned per user: an internal app catalogue, a notes service, anything that records an author. There a shared credential makes every write land under the same name, and it does not fail loudly — it records the wrong owner, silently. New per-server option on HTTP backends: "hub": { "url": "http://hub.internal/mcp", "headers": { "Authorization": "Bearer ${HUB_SERVICE_TOKEN}" }, "forward_identity": { "header": "X-MCP-Subject" } } The subject travels per request, from the identity the proxy already resolved in dispatch. `roles_header` is optional and off by default — most backends only need to know who, and sending roles a backend does not read is avoidable exposure. Scoped deliberately: - HTTP backends only. A stdio/CLI backend is a process the proxy owns, and there is no per-request channel to carry a header into a long-lived stdin pipe. - Opt-in per server, never on by default: a backend that never asked for identity must not start receiving one on upgrade. - The forwarded header WINS over a backend's own `x-mcp-header` annotation with the same name, and the argument-derived one is dropped rather than appended. Otherwise a tool could declare an argument mapped to `X-MCP-Subject` and let the caller pick their own subject — privilege escalation, tested. - A subject or role containing a control character REFUSES the call instead of falling back to the shared credential: falling back is the silent-wrong-owner outcome this feature exists to remove. - Header names are validated with the same `is_tchar` predicate the `x-mcp-header` path already uses (now `pub(crate)`) — two copies of a header-injection check is how one of them drifts. The safety condition the proxy cannot verify is documented on the type, in the config reference and in the code: the backend must not be reachable by anyone who could set the header themselves, or the header is a complete authentication bypass. That is why this is opt-in. Tests: 8 unit tests on the header builder (no config, stdio backend, unknown server, default and custom header names, roles, CRLF in subject, control char in roles, four malformed header names) and 2 client tests (shadowing refused, additive behaviour unchanged). Deny cases outnumber allow cases, per CONTRIBUTING's checklist for trusted-header changes. Full suite: 822 passing.

Copilot

🟡 Changes recommended

There are correctness/security-related issues around header shadowing semantics (and corresponding tests) plus doc/attribute attachment problems that should be fixed before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds an opt-in mechanism for HTTP backends to receive the authenticated caller’s identity as per-request HTTP headers, avoiding the “writes attributed to the shared service credential” problem for backends with per-user ownership semantics.

Changes:

Introduces forward_identity in ServerConfig::Http plus docs for configuration and safety constraints.
Computes per-request identity headers during dispatch (under the proxy lock) and forwards them on backend tools/call.
Adds a new McpClient::call_tool_raw_with path to send additional per-request headers, along with new tests.

File summaries

File	Description
src/serve/proxy.rs	Adds identity header construction/validation and unit tests; includes helper validators.
src/serve/dispatch.rs	Resolves identity headers in phase 1 and forwards them to backend calls.
src/client.rs	Adds `call_tool_raw_with` for extra per-request headers and tests for interaction with `x-mcp-header`.
src/config.rs	Adds `ForwardIdentity` type and `forward_identity` option on HTTP server configs.
src/output.rs	Updates HTTP `ServerConfig` test literals to include `forward_identity: None`.
docs/reference/config-file.md	Documents `forward_identity`, including security constraints and behavior notes.

Review details

Suppressed comments (2)

src/client.rs:2311

This test intends to ensure the argument-derived header is dropped, but it only inspects X-MCP-Subject and never asserts that the mirrored Mcp-Param-X-MCP-Subject header is absent. As written, it can pass even if the tool argument is still being forwarded via Mcp-Param-*.

        let headers = t.headers_for("tools/call");
        let subjects: Vec<&String> = headers
            .iter()
            .filter(|(name, _)| name.eq_ignore_ascii_case("x-mcp-subject"))
            .map(|(_, value)| value)

src/serve/proxy.rs:886

resolve_tool_call lost its #[allow(clippy::result_large_err)] and its doc comment due to the new identity_headers insertion. If clippy is still warning on this signature, the allow should live on resolve_tool_call itself (and the doc comment should be restored here).

    pub(crate) fn resolve_tool_call(
        &self,
        id: &Value,
        params: Option<Value>,
        identity: &AuthIdentity,

Files reviewed: 6/6 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        let reserved: Vec<String> = extra_headers
+            .iter()
+            .map(|(k, _)| k.to_ascii_lowercase())
+            .collect();
+        let mut headers = extra_headers.to_vec();
+        headers.extend(
+            from_params
+                .into_iter()
+                .filter(|(k, _)| !reserved.contains(&k.to_ascii_lowercase())),
+        );


+        if let Some(roles_header) = &cfg.roles_header {
+            out.push((
+                valid_header_name(roles_header)?,
+                valid_header_value(&identity.roles.join(","), "roles")?,
+            ));
+        }


@@ -842,6 +842,43 @@
    /// `arguments`. Returns Err(JsonRpcResponse) if the call should be
    /// rejected immediately.
    #[allow(clippy::result_large_err)]


+    /// Headers carrying the caller's identity for one backend call, or empty
+    /// when the server did not opt in via `forwardIdentity`.
+    ///


Four findings from review, all of them real. Two were mine to own. **1. Displaced doc comments took `#[allow]` with them — this is what broke CI.** Inserting `identity_headers` between `resolve_tool_call`'s doc block and the function moved BOTH the doc and `#[allow(clippy::result_large_err)]` onto the new function. `resolve_tool_call` lost its allow, so `cargo clippy -- -D warnings` failed on it. The same mistake sits in client.rs and review did not catch it: the MRTR doc block belonged to `call_tool_raw` and ended up on `call_tool_raw_with`, leaving `call_tool_raw` undocumented. Both restored. This also corrects the PR's claim that the two `result_large_err` errors pre-existed on main. Only ONE does — main's CI has been red since 2026-08-26 on `dispatch.rs` `client_or_connect`. The `proxy.rs` one was mine. Verified against main's own CI log, not a local run: the local run used `--all-targets` without `-D warnings`, so it printed them as warnings and I read both as inherited. **2. `roles_header` equal to `header` is now refused.** Two same-named headers with unrelated values leave the choice to the receiver, which could read the roles value as the subject. Refused for the same reason as a control character. Checked in `identity_headers` and not at load time because the existing config validation only warns at boot, and a warning would let the ambiguous call out. **3. The shadowing threat was overstated, and the test was vacuous.** `x-mcp-header` annotations always emit `Mcp-Param-{Name}`, so a tool annotating `X-MCP-Subject` produces `Mcp-Param-X-MCP-Subject` and cannot claim the identity header by name — the prefix isolates it with no help from the filter. There is no privilege escalation through that path, and the doc, the docs page and the PR body all claimed there was. `forwarded_identity_cannot_be_shadowed_by_x_mcp_header` asserted only that the forwarded subject was the value under `x-mcp-subject`, which was true for that structural reason — it passed with the filter deleted. Replaced by two tests: one documenting the prefix guarantee (and saying plainly that it does not exercise the filter), one exercising the only reachable collision, which is an operator configuring `header` under the `Mcp-Param-` prefix. **4. Doc said `forwardIdentity`; the field is `forward_identity`.** Fixed. Also translated the Portuguese comments and assertion messages this branch introduced — the rest of the codebase is in English and mixing the two is noise for a reader. Verified in a rust:1-slim container: cargo fmt --all --check clean cargo clippy -- -D warnings 1 error, the pre-existing dispatch.rs one cargo test 825 passed; 0 failed The three new tests were mutation-checked — removing the collision filter or disabling the roles_header check fails exactly them, and nothing else. The prefix-guarantee test keeps passing under those mutations, which is the point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

luisaniceto-infra · 2026-09-08T17:56:24Z

Os quatro achados procedem. Corrigidos em 41cd99c. Dois deles são meus de um jeito que vale dizer explicitamente, porque eu afirmei coisa errada no corpo desta PR.

1. A doc deslocada é a causa da falha do CI

O Copilot marcou como Low, mas é o que quebrou o build. Inserir identity_headers entre o bloco de doc do resolve_tool_call e a função levou junto o #[allow(clippy::result_large_err)] — então resolve_tool_call perdeu o allow e cargo clippy -- -D warnings passou a falhar nele.

E o mesmo erro está no client.rs, que a revisão não pegou: o bloco de doc sobre MRTR pertencia ao call_tool_raw e foi pro call_tool_raw_with, deixando o call_tool_raw sem documentação. Achei lendo o arquivo depois de entender o primeiro. Os dois voltaram pro lugar.

Retratação

Esta PR diz: "Os 2 warnings de result_large_err que o clippy mostra já existiam na main (conferi com a árvore limpa)". Isso está errado — só UM pré-existia.

O outro era meu, causado pelo allow deslocado. Conferi agora contra o log do CI da própria main, não contra minha máquina: a main está vermelha desde 26/08 (run 33011293588) e o log dela lista exatamente um erro, o do dispatch.rs. O meu run listava dois.

O que me enganou: rodei cargo clippy --all-targets sem -D warnings, então os dois saíram como warning e eu li os dois como herdados. Desculpa — você ia revisar uma PR cuja afirmação de baseline estava falsa.

Abri a #111 só pro erro pré-existente da main, separada porque é bug de CI anterior a esta feature e afeta todas as PRs. Com ela mergeada o CI daqui fica verde.

2. `roles_header` igual a `header` agora recusa

Procede, era lacuna real. Dois headers homônimos com valores sem relação deixam a escolha pro receptor, que pode ler o valor de roles como subject. Recusa, pela mesma razão do caractere de controle.

Fiz no identity_headers e não no load time porque a validação de config que existe (validate_server_names) só emite tracing::warn! no boot — um warning deixaria a chamada ambígua sair de qualquer jeito. Comparação case-insensitive, com teste pros dois casos.

3. O achado HIGH: mecanismo certo, consequência errada — e o problema real é meu

O Copilot está certo que meu filtro não pega Mcp-Param-X-MCP-Subject. Mas a conclusão de que isso "undermines the forwarded-identity-wins guarantee" não se sustenta, e verifiquei no código antes de mexer:

x_mcp_header::headers_for emite sempre Mcp-Param-{Name} (client.rs:925). Uma tool anotando X-MCP-Subject produz Mcp-Param-X-MCP-Subject — nome diferente. O backend que lê X-MCP-Subject como identidade nunca vê o valor do caller. O prefixo já isola estruturalmente; não existe a escalação.

O problema real é que esta PR e a doc afirmam que existe. Eu escrevi aqui: "uma tool declara um argumento mapeado pra X-MCP-Subject e o caller escolhe o próprio subject". Isso é falso — a tool não consegue produzir um X-MCP-Subject nu.

E pior: meu teste era vacuoso. forwarded_identity_cannot_be_shadowed_by_x_mcp_header filtrava headers cujo nome fosse exatamente x-mcp-subject e afirmava que só saía "ana". Verdade — mas verdade pelo prefixo, não pelo meu filtro. Ele passava com o filtro deletado.

Substituí por dois:

an_annotation_cannot_claim_the_identity_header_name — documenta a garantia do prefixo, e a doc dele diz explicitamente que passa sem o filtro.
extras_win_over_a_same_named_param_header — exercita a única colisão alcançável: o operador configurar header dentro do prefixo Mcp-Param-. Esse falha se o filtro sair.

Mantive o filtro por isso, e a doc agora diz exatamente o que ele protege em vez de vender segurança que o prefixo já dava.

4. `forwardIdentity` → `forward_identity`

Corrigido, na doc do Rust e na página de referência.

Verificação

Container rust:1-slim:


`cargo fmt --all --check`	limpo
`cargo clippy -- -D warnings`	1 erro, o pré-existente do `dispatch.rs` (#111)
`cargo test`	825 passed; 0 failed

Os três testes novos foram verificados por mutação — já entreguei um teste vazio nesta PR, não vou repetir:

mutação	falha
remover o filtro de colisão	`extras_win_over_a_same_named_param_header`
desligar a checagem de `roles_header`	`roles_header_equal_to_the_subject_header_is_refused` + `..._ignoring_case_...`

E o an_annotation_cannot_claim_the_identity_header_name continua passando sob as duas — que é o ponto dele.

Também traduzi pro inglês os comentários e mensagens em português que esta branch tinha introduzido; o resto do código é em inglês e misturar só atrapalha quem lê.

luisaniceto-infra · 2026-09-17T16:45:01Z

Obrigado por liberar o CI. O run de hoje ainda falha, mas por um erro que não é desta PR — vale registrar o que mudou:

run	erros no `cargo clippy -- -D warnings`
04/09	2 — `proxy.rs` (meu, `#[allow]` deslocado) + `dispatch.rs`
17/09	1 — só `dispatch.rs:783`

O meu foi corrigido em 41cd99c. O que sobrou é client_or_connect, que já falha na main desde 26/08 (run 33011293588) — esta branch só herda.

O job morre no passo do clippy, então o cargo test nem chega a rodar aqui. Localmente, em container rust:1-slim: 825 passed, 0 failed.

A #111 conserta exatamente esse erro e passou verde hoje (run 34260019894) — uma linha, o mesmo #[allow] que o resolve_tool_call já carrega.

Com a #111 na main, esta aqui fecha verde. Deixei separado de propósito: é bug de CI anterior a esta feature e afeta todas as PRs do repo, então misturar impediria reverter um sem o outro. Se preferir que eu traga a linha pra cá e feche a #111, faço na hora — é só dizer.

avelino requested review from avelino and a lite review from Copilot September 4, 2026 19:12

Copilot started reviewing on behalf of avelino September 4, 2026 19:13

View session

Copilot AI reviewed Sep 4, 2026

View reviewed changes

luisaniceto-infra mentioned this pull request Sep 8, 2026

fix(clippy): allow result_large_err on client_or_connect (unblocks CI on main) #111

Merged

avelino mentioned this pull request Sep 17, 2026

fix(serve): forward caller identity on every backend method #112

Merged

4 tasks done

avelino closed this Sep 17, 2026

Navigation Menu

Repository navigation

feat(config): forward caller identity to HTTP backends, opt-in per server - #110

feat(config): forward caller identity to HTTP backends, opt-in per server#110
luisaniceto-infra wants to merge 2 commits into
avelino:mainfrom
luisaniceto-infra:feat/forward-caller-identity

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Escopo, deliberado

A condição que o proxy não consegue verificar

Testes

Notas de revisão

Uh oh!

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

1. A doc deslocada é a causa da falha do CI

Retratação

2. roles_header igual a header agora recusa

3. O achado HIGH: mecanismo certo, consequência errada — e o problema real é meu

4. forwardIdentity → forward_identity

Verificação

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

2. `roles_header` igual a `header` agora recusa

4. `forwardIdentity` → `forward_identity`