feat(mcp): support custom request headers for MCP connectors - #5582
feat(mcp): support custom request headers for MCP connectors#5582adhyaay-karnwal wants to merge 3 commits into
Conversation
…nc#5465) Add the ability to attach custom HTTP headers (key-value pairs) to MCP server connectors, enabling authentication with static API keys, bearer tokens, or any custom header for servers that don't support OAuth. - Add `headers` JSONB column to `mcp_servers` table - Extend `McpServerRecord` with `headers: HashMap<String, String>` field - Build `StreamableHttpClientTransportConfig` with custom_headers in connect() - Update AddServerRequest, UpdateServerRequest, ServerResponse API types - Persist/load headers in PgServerRepo with JSONB serialization - Frontend: Add key-value header editor in Add Server dialog - Frontend: Add "Configure Headers" dialog on connected server rows - Frontend: Highlight header icon when custom headers are configured - Update all generated SDK types (orval + hey-api)
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds optional custom HTTP headers to MCP server records. Headers are stored as JSONB, loaded and saved through the repository, exposed by add and update APIs, and applied to both authenticated and unauthenticated transports. The settings interface supports adding headers during server creation and editing headers for existing servers through a configuration dialog. Invalid header names and values are ignored during transport configuration. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/features/settings/Integrations.tsx (1)
122-133: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not force OAuth for static-header server setup.
When a server uses an API key or bearer header instead of OAuth,
startAuthcan fail after the server is saved. The UI then reports a failed authorization for a valid header-authenticated server.Let the user select OAuth explicitly, or skip automatic OAuth for the static-header connection path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/features/settings/Integrations.tsx` around lines 122 - 133, Update handleSubmit so the successful addMutation flow does not unconditionally call startAuth for static-header authentication; invoke OAuth only when the user explicitly selected OAuth, while preserving reset and props.onOpenChange for valid header-authenticated servers.crates/mcp_client/src/outbound/oauth.rs (1)
280-286: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve configured headers after OAuth exchange.
The add flow first saves
body.headers. This record then setsheadersto an empty map.PgServerRepo::saveusesheaders = EXCLUDED.headerson conflict, so a successful OAuth exchange deletes the configured static credential.Load and retain the existing headers, or carry them through
PendingAuth, before saving this record.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/mcp_client/src/outbound/oauth.rs` around lines 280 - 286, Update the OAuth exchange record construction around McpServerRecord so configured headers from the initial add flow are retained instead of replaced with an empty map. Carry the headers through PendingAuth or load them from the existing server record before invoking PgServerRepo::save, and assign them to the record’s headers field during the OAuth completion path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/mcp_client/src/outbound/pg_server_repo.rs`:
- Around line 76-98: Update the server persistence flow around the headers_json
construction in the repository method to encrypt each custom header value with
the existing self.encrypt() mechanism before serializing and storing JSONB. In
the McpServerRecord materialization path, decrypt those values before returning
the record, while preserving header names, non-secret structure, and existing
error propagation.
---
Outside diff comments:
In `@apps/web/src/features/settings/Integrations.tsx`:
- Around line 122-133: Update handleSubmit so the successful addMutation flow
does not unconditionally call startAuth for static-header authentication; invoke
OAuth only when the user explicitly selected OAuth, while preserving reset and
props.onOpenChange for valid header-authenticated servers.
In `@crates/mcp_client/src/outbound/oauth.rs`:
- Around line 280-286: Update the OAuth exchange record construction around
McpServerRecord so configured headers from the initial add flow are retained
instead of replaced with an empty map. Carry the headers through PendingAuth or
load them from the existing server record before invoking PgServerRepo::save,
and assign them to the record’s headers field during the OAuth completion path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 73d08e7c-cf75-456c-9e1b-6aa2c8535c34
⛔ Files ignored due to path filters (4)
apps/web/src/lib/service-clients/service-cognition/generated/schemas/addServerRequest.tsis excluded by!**/generated/**,!apps/web/src/lib/service-clients/**/generated/**apps/web/src/lib/service-clients/service-cognition/generated/schemas/serverResponse.tsis excluded by!**/generated/**,!apps/web/src/lib/service-clients/**/generated/**apps/web/src/lib/service-clients/service-cognition/generated/schemas/updateServerRequest.tsis excluded by!**/generated/**,!apps/web/src/lib/service-clients/**/generated/**packages/sdk/generated/cognition/types.gen.tsis excluded by!**/generated/**,!**/*.gen.ts
📒 Files selected for processing (10)
apps/web/src/features/settings/Integrations.tsxcrates/macro_db_client/migrations/20260811000000_add_mcp_servers_headers.down.sqlcrates/macro_db_client/migrations/20260811000000_add_mcp_servers_headers.up.sqlcrates/mcp_client/Cargo.tomlcrates/mcp_client/src/domain/models/server.rscrates/mcp_client/src/domain/service/persisting_credential_store/test.rscrates/mcp_client/src/inbound/axum_router.rscrates/mcp_client/src/outbound/oauth.rscrates/mcp_client/src/outbound/pg_server_repo.rscrates/onboarding/src/domain/service/test.rs
| let headers_json = serde_json::to_value(&record.headers) | ||
| .map_err(|e| sqlx::Error::Protocol(e.to_string()))?; | ||
|
|
||
| // Never clobber stored credentials with NULL on conflict: re-adding | ||
| // an existing server (e.g. via the Add Server dialog) must not wipe | ||
| // a valid OAuth grant. | ||
| sqlx::query!( | ||
| r#" | ||
| INSERT INTO mcp_servers (user_id, url, server_name, credentials, enabled) | ||
| VALUES ($1, $2, $3, $4, $5) | ||
| INSERT INTO mcp_servers (user_id, url, server_name, credentials, enabled, headers) | ||
| VALUES ($1, $2, $3, $4, $5, $6) | ||
| ON CONFLICT (user_id, url) DO UPDATE | ||
| SET server_name = EXCLUDED.server_name, | ||
| credentials = COALESCE(EXCLUDED.credentials, mcp_servers.credentials), | ||
| enabled = EXCLUDED.enabled, | ||
| headers = EXCLUDED.headers, | ||
| updated_at = NOW() | ||
| "#, | ||
| record.user_id.as_ref(), | ||
| record.url, | ||
| record.server_name, | ||
| encrypted.as_deref(), | ||
| record.enabled, | ||
| &headers_json, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Encrypt custom header values before persistence.
Custom headers can contain bearer tokens and API keys. This code stores them as plaintext JSONB, while OAuth credentials use self.encrypt(). A direct database or backup read can expose these credentials.
Encrypt header values before writing them. Decrypt them only when materializing McpServerRecord.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/mcp_client/src/outbound/pg_server_repo.rs` around lines 76 - 98,
Update the server persistence flow around the headers_json construction in the
repository method to encrypt each custom header value with the existing
self.encrypt() mechanism before serializing and storing JSONB. In the
McpServerRecord materialization path, decrypt those values before returning the
record, while preserving header names, non-secret structure, and existing error
propagation.
synoet
left a comment
synoet
left a comment
There was a problem hiding this comment.
Very Nice! Seems mostly sensible, one comment on how we store the headers. I'll let @ehayes2000 look closer.
| r#" | ||
| INSERT INTO mcp_servers (user_id, url, server_name, credentials, enabled) | ||
| VALUES ($1, $2, $3, $4, $5) | ||
| INSERT INTO mcp_servers (user_id, url, server_name, credentials, enabled, headers) | ||
| VALUES ($1, $2, $3, $4, $5, $6) | ||
| ON CONFLICT (user_id, url) DO UPDATE | ||
| SET server_name = EXCLUDED.server_name, | ||
| credentials = COALESCE(EXCLUDED.credentials, mcp_servers.credentials), | ||
| enabled = EXCLUDED.enabled, | ||
| headers = EXCLUDED.headers, | ||
| updated_at = NOW() |
There was a problem hiding this comment.
Given that a common use case will be to store credentials, storing this in plain text might not be great.
ehayes2000
left a comment
ehayes2000
left a comment
There was a problem hiding this comment.
Overall looks very strong. It looks like we're saving header-value pairs in plaintext JSONB. This should be encrypted following the storage strategy of MCP tokens.
Nice work
…auth, preserve headers on exchange - Encrypt custom header values with AES-256-GCM (matching credential storage strategy) by switching the `headers` column from JSONB to BYTEA - Don't start OAuth after adding a server when static headers are present; API-key / bearer-token servers authenticate directly via headers - Load and preserve previously-configured headers during the OAuth exchange so the completion flow doesn't wipe them with an empty map
|
Thanks for the feedback, fixing |
|
@adhyaay-karnwal appreciate you working on this! |
There was a problem hiding this comment.
@adhyaay-karnwal, I'm very thankful for your work on this PR. It means a lot to me since I initially requested #5465.
Since I requested the feature, I feel some responsibility to lend a helping hand. I had my agent review this PR and here's its researched response. I hope it's helpful.
Please chew the meat and spit out the bones. Anything that's not helpful or blocking on here can be ignored!
Agent summary
Custom headers as the model match #5465. Encryption at rest (same AES-GCM path as OAuth creds) is the right storage call. OAuth complete now keeps prior headers. Good work.
Must fix before merge:
- Header-only servers still look unauthenticated in the UI (
authenticatedmeans OAuth credentials only). The toolset connectsenabledservers, so runtime can still try headers, but the row shows Connect, hides the enable toggle, and has no checkmark. For #5465, "I attached a key" must look connected. - Re-add can wipe stored headers (
headers = EXCLUDED.headerswhile credentials useCOALESCE). - Run
just prepare_dband commit.sqlx(CS-50 / AGENTS.md). Prefersqlx migrate addfor the migration name (do not invent…000000).
Strong recommendations (not hard blockers):
- Prefer not to return full header values in
ServerResponse(XSS, extensions, screenshare, client logs). Claude's remote MCP does not show values again after save. Owner-retrievable secrets are also a valid model. Keys + mask, or write-once, is a good middle path. - Do not skip OAuth only because any header exists if Macro wants gateway headers + user OAuth (Claude supports both;
Authorizationstays OAuth-owned). For pure static-key servers, skipping OAuth is fine. - Validate header names and values on Add/Update instead of silent drop at transport build.
- Password-style input for values; avoid
createEffectprops→state sync (FE-09 / FE-10).
See inline comments.
Code Reviewer bot, sent on behalf of Cam.
| @@ -218,6 +228,7 @@ impl ServerResponse { | |||
| server_name: record.server_name.clone(), | |||
| enabled: record.enabled, | |||
| authenticated: record.credentials.is_some(), | |||
There was a problem hiding this comment.
issue (blocking, ux): authenticated still means "has OAuth credentials".
authenticated: record.credentials.is_some(),After Add with headers, the row has no checkmark. Connect stays. The enable toggle stays hidden.
The toolset connects enabled servers, and Add sets enabled: true, so runtime can still try to connect with headers. The gap is the product UI for #5465: attaching a key must look connected.
suggestion: Treat a server as connected in the UI when OAuth credentials exist, or when usable static headers exist. Keep a manual Connect action for OAuth. Let the user toggle enable for header-only servers.
Code Reviewer bot, sent on behalf of Cam.
| SET server_name = EXCLUDED.server_name, | ||
| credentials = COALESCE(EXCLUDED.credentials, mcp_servers.credentials), | ||
| enabled = EXCLUDED.enabled, | ||
| headers = EXCLUDED.headers, |
There was a problem hiding this comment.
issue (blocking): Re-add can wipe stored headers.
credentials = COALESCE(EXCLUDED.credentials, mcp_servers.credentials),
headers = EXCLUDED.headers,Credentials stay when the new row has none. Headers do not. An Add without headers replaces stored secrets with empty (None when the map is empty).
suggestion: Match the credentials pattern. Update headers only when the request sets them (or use COALESCE when the new value is null).
Code Reviewer bot, sent on behalf of Cam.
| @@ -0,0 +1,2 @@ | |||
| ALTER TABLE mcp_servers | |||
| ADD COLUMN IF NOT EXISTS headers BYTEA; | |||
There was a problem hiding this comment.
chore (blocking): Migration timestamp looks hand-made, and .sqlx is missing.
AGENTS.md: use sqlx migrate add for the name. Do not invent …000000. Recent neighbors use wall-clock stamps like 20260810154320.
CS-50 / AGENTS.md: after SQL changes, run just prepare_db from the repo root and commit the .sqlx cache. The PR notes this for post-merge. Put the cache in this PR so CI and offline builds work.
Code Reviewer bot, sent on behalf of Cam.
| server_name: record.server_name.clone(), | ||
| enabled: record.enabled, | ||
| authenticated: record.credentials.is_some(), | ||
| headers: record.headers.clone(), |
There was a problem hiding this comment.
suggestion (security): Prefer not to return full header values in ServerResponse.
At-rest encryption is good. List and get still decrypt and send every value to the client. XSS, extensions, screen shares, and client logs can then see tokens.
Claude's remote MCP stores header values and does not show them again after save (docs). Owner-retrievable secrets are also a valid model for user-pasted connector creds.
A strong middle path: return header names (and optional masks) only. Accept a full replace map (or set/delete patch) on update. Keep omitted secrets. Show •••••• in the UI when a value exists.
Code Reviewer bot, sent on behalf of Cam.
| startAuth(n, u); | ||
| // Only start OAuth when the server has no static headers — | ||
| // API-key / bearer-token servers authenticate via headers directly. | ||
| if (!headersObject()) { |
There was a problem hiding this comment.
suggestion: Do not skip OAuth only because any header exists.
if (!headersObject()) {
startAuth(n, u);
}For pure static-key servers, skipping auto-OAuth is reasonable. Claude also allows request headers in addition to OAuth on one connection (Authorization stays OAuth-owned) (docs).
If Macro needs gateway routing headers plus user OAuth, the current heuristic blocks that path. Prefer an explicit auth mode (oauth | static_headers | both), or skip auto-OAuth only for auth-shaped headers. Keep a manual Connect control.
Code Reviewer bot, sent on behalf of Cam.
| let custom_headers: HashMap<HeaderName, HeaderValue> = self | ||
| .headers | ||
| .iter() | ||
| .filter_map(|(k, v)| { |
There was a problem hiding this comment.
suggestion: Validate header names and values on Add and Update.
filter_map in transport_config drops bad HeaderName / HeaderValue entries with no error. A typo can save and then fail to auth with little signal.
Prefer a clear API error on write. Keep a last-resort filter only if a bad row somehow reaches connect.
Code Reviewer bot, sent on behalf of Cam.
| >(initialHeaders()); | ||
|
|
||
| // Reset when server changes | ||
| createEffect(() => { |
There was a problem hiding this comment.
suggestion (non-blocking): Avoid createEffect to sync props into local state.
FE-10: use createEffect only for external systems. FE-09: derive, do not sync.
Reset pairs when the dialog opens through dialog lifecycle, a key on the dialog, or state owned by the parent.
Code Reviewer bot, sent on behalf of Cam.
| /> | ||
| <span class="text-ink-muted text-xs">:</span> | ||
| <input | ||
| type="text" |
There was a problem hiding this comment.
nitpick (non-blocking, security): Use a password-style input for header values.
Tokens show in plain text today. Prefer type="password" or a reveal toggle.
Code Reviewer bot, sent on behalf of Cam.
| .map_err(|e| anyhow::anyhow!("invalid user_id in pending context: {e}"))? | ||
| .into_owned(); | ||
|
|
||
| // Preserve any custom headers that were configured before the OAuth |
There was a problem hiding this comment.
praise: OAuth complete keeps prior headers.
Load-then-save here stops the exchange from clearing headers that Add already stored. Good fix.
Code Reviewer bot, sent on behalf of Cam.
|
|
||
| impl McpServerRecord { | ||
| /// Build the transport config for this server, including any custom headers. | ||
| fn transport_config(&self) -> StreamableHttpClientTransportConfig { |
There was a problem hiding this comment.
praise: Headers sit on the transport config for both auth paths.
OAuth and no-credential connect both call transport_config(). That matches "send on every request." Free-form headers also match peers like Cursor's remote MCP headers map (docs).
Code Reviewer bot, sent on behalf of Cam.
|
Will take a look at these today! |
…ly servers as connected, validate headers (macro-inc#5582) - add_server loads the existing row so re-adding never wipes configured headers, and the response reflects the stored OAuth credentials - add/update validate header names and values, returning 400 on malformed input - settings UI treats a server as connected when it has OAuth creds or static headers (checkmark + enable toggle), while keeping a manual OAuth connect - skip auto-OAuth only for auth-shaped headers (Authorization / X-Api-Key) - header values use a password input with a reveal toggle - remove createEffect props->state sync in the headers dialog (keyed remount) - rename migration to a wall-clock timestamp
Review round addressed —
|
|
@adhyaay-karnwal thanks! Good work! I'll now defer to @ehayes2000 for review |
4 participants
Closes #5465
What
Adds the ability to attach custom HTTP headers (key-value pairs) to MCP server connectors, enabling authentication with static API keys, bearer tokens, or any custom headers for servers that don't support OAuth.
Changes
Backend
headers JSONBcolumn tomcp_serversMcpServerRecordgainsheaders: HashMap<String, String>field, serialized as JSONB, deserialized on loadtransport_config()buildsStreamableHttpClientTransportConfigwithcustom_headers(), filtering invalid names/values gracefullyAddServerRequest,UpdateServerRequest,ServerResponse) all includeheadersPgServerRepopersists & loads headers alongside encrypted credentialsFrontend
Screenshots
Add Server dialog with custom headers
(screenshot showing the "Custom Headers" section with key-value inputs)
Configure Headers dialog on existing server
(screenshot showing the headers config dialog on a connected server row)
Post-merge
nix develop --command just prepare_dbto update SQLx offline cache