Skip to content

Python: scope Secure MCP URL headers to origin - #8285

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 5 commits into
microsoft:mainfrom
eavanvalkenburg:automatic-disco
Sep 11, 2026
Merged

Python: scope Secure MCP URL headers to origin#8285
Eduard van Valkenburg (eavanvalkenburg) merged 5 commits into
microsoft:mainfrom
eavanvalkenburg:automatic-disco

Conversation

@eavanvalkenburg

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) commented Sep 11, 2026

Copy link
Copy Markdown
Member

Motivation & Context

URL-based SecureMCPToolProxy credentials must remain available for MCP initialization and discovery while staying scoped to the configured endpoint origin across redirects.

Description & Review Guide

  • What are the major changes? Add an origin-scoped fixed-header path to MCPStreamableHTTPTool, route URL-mode proxy headers through it, require a valid absolute HTTP(S) origin for scoped injection, and add httpx.MockTransport regression coverage.
  • What is the impact of these changes? Configured headers remain available for same-origin initialization, discovery, calls, and redirects, are removed when a redirect changes origin, and do not serialize concurrent tool calls.
  • What do you want reviewers to focus on? The separation between fixed headers and dynamically resolved per-call headers in the shared owner/origin-scoped request hook.

Related Issue

No related open issue.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: bf1fc8c4-eff7-42e7-9342-d385036223be

Copilot AI 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.

🟡 Changes recommended

Fixed headers now unnecessarily serialize concurrent MCP tool calls, causing a throughput regression.

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

Pull request overview

Scopes Secure MCP URL credentials to the configured HTTP(S) origin across initialization, discovery, requests, and redirects.

Changes:

  • Routes proxy headers through the scoped header provider.
  • Rejects invalid origins when injecting headers.
  • Adds redirect and validation regression tests.
File summaries
File Description
python/packages/core/agent_framework/_mcp.py Validates HTTP(S) origins.
python/packages/core/agent_framework/security.py Uses origin-scoped header injection.
python/packages/core/tests/test_security.py Tests redirects and invalid URLs.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

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

Comment thread python/packages/core/agent_framework/security.py Outdated

@github-actions github-actions 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.

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): 9236651ef265
Model: gpt-5.6-sol-fast

Overview

The change correctly reuses the MCP transport's owner-tagged, origin-aware request hook, preserving static credentials for initialization and same-origin redirects while stripping them on cross-origin redirects. URL validation now fails closed before the framework allocates an HTTP client, and the new tests exercise initialization, discovery, redirect loops, and malformed URLs. However, representing immutable proxy headers as a dynamic header_provider also opts every call into instance-wide serialization, regressing the framework's parallel tool-call behavior.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/core/agent_framework/security.py

Comment thread python/packages/core/agent_framework/security.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: bf1fc8c4-eff7-42e7-9342-d385036223be
@agent-framework-automation agent-framework-automation Bot added the documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs label Sep 11, 2026
Merged via the queue into microsoft:main with commit c47da07 Sep 11, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants