Skip to content

feat(mcp): support custom HTTP headers for native agents - #1510

Open
fisherivco wants to merge 1 commit into
openabdev:mainfrom
fisherivco:feat/mcp-http-headers
Open

feat(mcp): support custom HTTP headers for native agents#1510
fisherivco wants to merge 1 commit into
openabdev:mainfrom
fisherivco:feat/mcp-http-headers

Conversation

@fisherivco

Copy link
Copy Markdown

What problem does this solve?

Native openab-agent backends can discover remote Streamable HTTP MCP servers, but they cannot currently attach the custom HTTP headers required by authenticated servers. This forces native agents to use a separate credential-bearing CLI path even when the same MCP server is already available to other OpenAB agent backends.

This change lets native agents declare secret-resolved, per-server HTTP headers in mcp.json and sends them through the existing rmcp transport.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1540933586973233264

Review Contract

Goal

Allow native openab-agent backends to connect to authenticated remote Streamable HTTP MCP servers by configuring per-server custom headers whose values can use OpenAB's existing ${env:VAR} resolution.

Non-goals

  • Add a new secret store, credential broker, proxy, or sidecar.
  • Change OAuth discovery, token storage, or refresh behavior.
  • Add server-specific configuration or tools.
  • Change stdio MCP transport behavior.
  • Migrate any deployment or credential as part of this PR.

Accepted Residual Risks

  • Header values are resolved when configuration is loaded, so changing an environment variable requires the agent process to reload its MCP configuration. Recovery is to restart the agent with the corrected environment.
  • Literal header values remain valid configuration for compatibility. Documentation uses environment references and recommends keeping credentials out of config files.
  • The anonymous HTTP path has an end-to-end wire test. The OAuth path is compile-checked through the same StreamableHttpClientTransportConfig::custom_headers field but does not add a second end-to-end OAuth-plus-custom-header fixture in this PR. Existing OAuth tests and the full workspace suite remain green; a dedicated OAuth wire fixture is listed as a follow-up.
  • A custom Authorization header combined with oauth is rejected instead of defining ambiguous precedence. Users can choose either OAuth or a custom Authorization header for that server.

Acceptance Criteria

  • Existing HTTP MCP entries without headers continue to deserialize unchanged.
  • Header values support the existing ${env:VAR} resolution and fail closed when a referenced variable is absent.
  • Header names and values are validated before a connection is attempted, without exposing secret values in errors.
  • Custom headers are sent by the anonymous Streamable HTTP transport.
  • OAuth and anonymous HTTP transports both receive the resolved header map.
  • Ambiguous OAuth plus custom Authorization configuration is rejected.
  • Native-agent MCP documentation describes the public configuration shape and credential-handling guidance.
  • Required upstream CI passes on the submitted commit.

Follow-ups

  • Add a dedicated end-to-end OAuth token plus custom-header wire fixture if maintainers want transport-level coverage beyond the shared typed configuration path.
  • Broader secret-provider abstractions remain separate from this narrowly scoped transport capability.

At a Glance

mcp.json headers
       |
       v
existing ${env:VAR} resolver
       |
       v
validated sensitive HeaderName/HeaderValue map
       |
       v
rmcp Streamable HTTP custom_headers
       |
       v
authenticated remote MCP server

Prior Art & Industry Research

OpenClaw:

OpenClaw accepts per-server HTTP headers, resolves environment-backed values before connection, and attaches the resulting map to its remote MCP transport:

Hermes Agent:

Hermes Agent exposes a generic headers map for HTTP MCP servers and supports environment substitution in its MCP configuration:

Other references (optional):

Proposed Solution

  • Add a backward-compatible headers map to ServerConfig::Http.
  • Reuse OpenAB's existing recursive environment resolver for header values.
  • Parse names and values into http::HeaderName and http::HeaderValue, reject normalized duplicates, and mark values sensitive for debug output.
  • Reject custom Authorization when OAuth is configured so authentication precedence is explicit.
  • Pass the validated map into rmcp's custom_headers field for both anonymous and OAuth Streamable HTTP transports.
  • Document the configuration in the native-agent MCP reference and alignment documents.

Example:

{
  "mcpServers": {
    "remote": {
      "type": "http",
      "url": "https://mcp.example.com/mcp",
      "headers": {
        "X-API-Key": "${env:REMOTE_MCP_API_KEY}"
      }
    }
  }
}

Why this approach?

The underlying rmcp client already supports custom headers, and OpenAB already owns environment interpolation and MCP configuration loading. Connecting those existing capabilities keeps authentication policy at the remote server, preserves the current native-agent architecture, and avoids introducing a second control plane.

Typed validation makes malformed configuration fail before network activity. The OAuth conflict rule avoids silently sending two competing authorization mechanisms.

Alternatives Considered

  • Credential-bearing CLI commands: useful as a temporary operational fallback, but they do not make authenticated MCP capabilities available to native agents and require a separate credential path.
  • A local proxy or sidecar: can inject headers, but adds deployment, lifecycle, and failure-surface complexity for a capability already present in rmcp.
  • A single auth_header field: does not cover authenticated servers that use non-Authorization headers or require more than one header.
  • Server-specific configuration: would couple OpenAB to one MCP implementation instead of providing the generic HTTP capability.

Validation

  • cargo check passes — pending required upstream CI on the submitted commit.
  • cargo test -p openab-mcp passes: 226 passed, 0 failed.
  • cargo test --workspace -- --test-threads=1 passes.
  • cargo clippy clean — pending required upstream CI on the submitted commit.
  • Standalone native-agent compatibility: cargo test passes with the native openab-agent lock (rmcp 1.7.0), including 62 default and 11 ignored tests.
  • Standalone native-agent release build: cargo build --release passes.
  • Manual wire test: a local Streamable HTTP MCP server rejected requests without X-API-Key and accepted the native client configured with the environment-resolved header.
  • git diff --check passes.

Independent review completed with PASS on the exact submitted diff:

  • Review thread: at3-20260826-openab-native-http-headers-chi-show
  • Reviewed diff SHA-256: 5d89f9e133e25c27f7cea761e4758006e1717b29506bf94c2e076b83b8def87a
  • Reviewed commit: f91a0e85de7f259a960ce55eacf51c008aec298e

@chaodu-agent

Copy link
Copy Markdown
Collaborator

/review

@chaodu-obk

This comment has been minimized.

@chaodu-obk chaodu-obk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

CHANGES REQUESTED ⚠️ - Custom credential headers can cross redirect boundaries, invalid header configuration leaves misleading status, and required CI is failing.

Consolidated review: #1510 (comment)

None => {
let transport = StreamableHttpClientTransport::from_uri(url.as_str());
let cfg = StreamableHttpClientTransportConfig::with_uri(url.as_str())
.custom_headers(headers);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 F1 - Prevent credential disclosure through redirects

This config is passed to rmcp 1.7/1.8, whose default reqwest client follows redirects. Reqwest strips standard auth/cookie headers across origins but not arbitrary credentials such as X-API-Key, so a redirect can replay this custom secret to another origin. The OAuth client constructed above has the same default redirect policy.

Requested change: use an MCP reqwest client with redirects disabled (or a rigorously tested same-origin-only policy) for both anonymous and OAuth paths, and add a two-server regression test proving the redirect target never receives the custom header.

ServerConfig::Http { url, .. } => DialPlan::Dial(Dial::Http { url, client: None }),
} => DialPlan::OauthHttp {
url,
headers: parse_http_headers(name, headers, true)?,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 F2 - Record deterministic header configuration failures in status

This ? returns before handle.status becomes Connecting and before the common dial-failure branch. Invalid header configuration can therefore leave mcp status reporting Disconnected rather than Failed.

Requested change: route these redacted configuration errors through ServerStatus::Failed without charging the transport circuit breaker, and assert status for each rejected-header test.

@chaodu-obk

chaodu-obk Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Important

CHANGES REQUESTED ⚠️ - Credential-bearing custom headers remain exposed to redirect replay, configuration failures are not consistently surfaced, and required CI is not green.

What This PR Does

This PR adds per-server custom HTTP headers to native Streamable HTTP MCP configuration. Header values use the existing ${env:VAR} resolver, are parsed into typed sensitive values, and are passed to rmcp for anonymous and OAuth-backed transports.

How It Works

ServerConfig::Http gains a default-empty headers map. Connection planning resolves environment placeholders, parses header names and values, rejects case-normalized duplicates and OAuth plus custom Authorization, and supplies the resulting map to StreamableHttpClientTransportConfig::custom_headers.

Findings

# Severity Finding Location
F1 🔴 Critical Custom credential headers can be replayed to a cross-origin redirect target. crates/openab-mcp/src/mcp/runtime.rs:604-617, :2088-2105
F2 🟡 Important Local header-validation errors return without recording ServerStatus::Failed. crates/openab-mcp/src/mcp/runtime.rs:1442-1458
F3 🟡 Important rmcp-owned headers are accepted locally, then fail late with weaker context and breaker impact. crates/openab-mcp/src/mcp/runtime.rs:1938-1964
F4 🟡 Important mcp validate does not reject malformed literal header names or values before runtime connection. crates/openab-mcp/src/mcp/config.rs:470-480
F5 🟡 Important Documentation omits that HTTP header names are case-insensitive and case-variant duplicates are rejected. openab-agent/docs/mcp.md
F6 🟡 Important Required CI is failing because cargo clippy exited with code 101. CI job 98321412132
F7 🟢 Praise Environment resolution, value-safe errors, typed validation, OAuth conflict handling, and anonymous wire coverage are strong. config.rs, runtime.rs, docs
Finding Details

🔴 F1: Prevent credential disclosure through redirects

The workspace locks rmcp 1.8.0 and the standalone agent locks rmcp 1.7.0. Their default reqwest clients follow redirects. Reqwest removes a fixed set of standard sensitive headers when an origin changes, but it does not remove arbitrary credentials such as X-API-Key. HeaderValue::set_sensitive(true) affects formatting, not redirect forwarding. Both the anonymous transport and the OAuth-backed MCP client are affected.

Requested change: disable redirects for credential-bearing MCP clients, or implement and test a strict same-origin policy that never forwards custom credentials across origins. Add a two-server regression test for anonymous and OAuth-backed client construction.

🟡 F2: Record deterministic configuration failures

parse_http_headers(...)? can return before the handle reaches Connecting and before the common dial-error branch. Invalid names, values, duplicates, and OAuth conflicts may therefore leave status as Disconnected instead of Failed.

Requested change: record a redacted ServerStatus::Failed for deterministic header configuration errors without charging the transport circuit breaker. Assert status in every rejected-header test.

🟡 F3: Reject transport-owned headers locally

The local validator only handles generic syntax, duplicates, and OAuth plus Authorization. rmcp owns headers including Accept, MCP-Session-Id, and Last-Event-ID; accepting them locally defers failure to rmcp request construction, loses OpenAB server context, and can count deterministic configuration errors as transport failures. MCP-Protocol-Version should remain SDK-owned and must have an explicit policy.

Requested change: align local validation with the pinned rmcp reserved-header contract, preserve server and header context, and add regression tests. Avoid a drifting hand-maintained denylist if upstream exposes a reusable validator.

🟡 F4: Validate literal header configuration before connection

McpConfig::validate currently validates OAuth configuration only. A malformed literal header can pass config validation and fail much later on first connection.

Requested change: share header validation between boot/config validation and resolved connect-time validation. Literal names and values should fail early; unresolved value placeholders may be deferred until resolution.

🟡 F5: Document case-insensitive header names

JSON keys are case-sensitive, but HTTP header names are not. A user can write both X-Key and x-key and receive a duplicate-header error without the docs explaining why.

Requested change: document case-insensitive names and case-variant duplicate rejection, and clarify that interpolation applies to values rather than header names.

🟡 F6: Restore required CI

The required check job failed in cargo clippy with exit code 101 on this SHA.

Requested change: fix the lint failure and rerun required CI successfully.

Inline Thread Disposition

Thread Response Resolution
Redirect credential replay Confirmed by pinned rmcp and reqwest behavior; remediation and regression criteria are in F1. Open - code is unchanged, so resolving would be incorrect.
Invalid-header status Confirmed in dial-plan construction; status and breaker expectations are in F2. Open - code is unchanged, so resolving would be incorrect.

Addressing All Review Feedback

Concern Disposition
Redirect forwarding of custom credentials Accepted as blocking - F1.
Late rejection of rmcp-owned headers Accepted as blocking - F3.
Missing boot-time validation Accepted as blocking - F4.
Case-insensitive duplicate behavior is undocumented Accepted as blocking - F5.
mcp list --resolve prints resolved values Accepted as an explicit opt-in behavior - the command already warns that output may contain secrets. Follow-up hardening could make redaction the default and require a separate show-secrets flag.
Derived Debug and Serialize can expose raw config values in future sinks Accepted as defense-in-depth - no new implicit sink was identified in this diff; a redacted wrapper remains a worthwhile follow-up.
Literal credential values remain allowed Accepted residual risk - keep environment-backed examples and warnings; a future policy may warn on literals.
Header ordering, boolean validation context, and duplicated transport setup Non-blocking maintainability follow-ups.
Anonymous transport wire coverage Positive - the real wire test is useful, but redirect and OAuth-backed regression coverage are still required by F1.
Scope and simplicity Positive - the implementation reuses the existing resolver and rmcp transport instead of adding a new credential subsystem.

Validation

  • Exact head reviewed: f91a0e85de7f259a960ce55eacf51c008aec298e
  • Declared base and merge-base: 8661f3f80d9f2273f153f79b672453c8207b1db4
  • Diff: 5 files, +328/-34
  • git diff --check: passed locally
  • Local Rust tests: not rerun because this review environment has no Cargo toolchain
  • Upstream checks: most jobs passed; required cargo clippy failed
What's Good (🟢)
  • Header-less HTTP configuration remains compatible through serde(default).
  • Header values are omitted from validation errors and typed values are marked sensitive.
  • Case-insensitive duplicates and OAuth plus custom Authorization fail closed.
  • The anonymous path includes a real wire-level test.
  • Documentation consistently recommends environment-backed credentials.
  • The feature remains narrowly scoped and reuses existing architecture.

5. Three Reasons We Might Not Need This PR

  1. OAuth covers some authenticated servers - Generic headers expand credential-handling surface for integrations outside that path.
  2. A safer upstream rmcp version may remove local policy code - A dependency upgrade could supply redirect-safe defaults, but both lockfiles must be verified.
  3. A credential broker would centralize rotation and policy - Environment-backed static headers require process reload and distribute secret handling across agents.

@chaodu-obk chaodu-obk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

CHANGES REQUESTED ⚠️ - Redirect credential replay, configuration validation/status gaps, reserved-header handling, documentation, and required CI still need changes.

Consolidated review: #1510 (comment)

@chaodu-agent

Copy link
Copy Markdown
Collaborator

Let me take over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants