Python: keep MCP labels subordinate to local FIDES policy - #8187
Python: keep MCP labels subordinate to local FIDES policy#8187Eduard van Valkenburg (eavanvalkenburg) wants to merge 6 commits into
Conversation
Keep resolved-input confidentiality on transformed and embedded-label results, publish authoritative inspect/quarantine labels, and avoid unwrapping ordinary response-shaped JSON. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve the stacked PR2/PR3 boundary so hidden arguments are expanded once, retain their combined label for policy, and preserve their confidentiality on transformed results.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use middleware provenance for quarantine unwrapping, require explicit label fields, fail unknown quarantine inputs closed, and scope trusted inspect errors to clean parser-marked invocations.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Bound FIDES policy approvals per session with FIFO and TTL expiry, clean authenticated non-grants by occurrence, and persist visible replacement approvals so stale grants require a safe second approval.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| } | ||
| # Sentinel is consumed. | ||
| assert "_meta" not in contents[0].additional_properties | ||
| for local_integrity in IntegrityLabel: |
| # Sentinel is consumed. | ||
| assert "_meta" not in contents[0].additional_properties | ||
| for local_integrity in IntegrityLabel: | ||
| for local_confidentiality in ConfidentialityLabel: |
| assert "_meta" not in contents[0].additional_properties | ||
| for local_integrity in IntegrityLabel: | ||
| for local_confidentiality in ConfidentialityLabel: | ||
| for remote_integrity in IntegrityLabel: |
| for local_integrity in IntegrityLabel: | ||
| for local_confidentiality in ConfidentialityLabel: | ||
| for remote_integrity in IntegrityLabel: | ||
| for remote_confidentiality in ConfidentialityLabel: |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 40c862e916f4
Model: gpt-5.6-sol-fast
Overview
The review found 2 verified inline finding(s).
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/security.py
| server_meta = props.pop(_MCP_RESULT_META_KEY, None) | ||
| dynamic = _label_from_mcp_meta(server_meta) if server_meta else None | ||
| label = dynamic or static_label | ||
| label = combine_labels(static_label, dynamic) if dynamic is not None else static_label |
There was a problem hiding this comment.
static_label is captured only when the wrapper is first installed, but refresh_labels() can later update this tool's source_integrity after its annotations or local policy become stricter. Because the wrapper marker prevents replacement, a trusted _meta.ifc value is still combined with the stale label and can leave the result TRUSTED even though the refreshed policy is UNTRUSTED. Please read the current locally derived label at invocation time or update the wrapper's captured policy during refresh.
| server_meta = props.pop(_MCP_RESULT_META_KEY, None) | ||
| dynamic = _label_from_mcp_meta(server_meta) if server_meta else None | ||
| label = dynamic or static_label | ||
| label = combine_labels(static_label, dynamic) if dynamic is not None else static_label |
There was a problem hiding this comment.
security.py:3859 treats the invocation-only max_allowed_confidentiality ceiling as output confidentiality, so security.py:3832 upgrades a server-declared PUBLIC result to PRIVATE, tainting the session and blocking later PUBLIC sinks; a safe fix must preserve locally declared output confidentiality against remote declassification.
Motivation & Context
MCP servers control result
_meta.ifcand ToolAnnotations, so treating either as security authority lets an untrusted server replace or relax integrity and confidentiality derived from local FIDES policy. Remote metadata must remain useful for adding restrictions without allowing a server to promote trust, declassify content, or authorize tainted input.Description & Review Guide
openWorldHint=FalseandreadOnlyHint=Trueas non-authoritative hints, and adds regression coverage for built-in/custom parsers, structured and task results, malicious annotations, and the full integrity/confidentiality matrix.accepts_untrusted; explicit local per-tool annotation overrides remain the authority seam.Related Issue
Related to #7455. Depends on #8142 and is the new top layer above it in native stack 8143; this is a distinct MCP ingestion-authority fix.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.