Skip to content

Python: keep MCP labels subordinate to local FIDES policy - #8187

Draft
Eduard van Valkenburg (eavanvalkenburg) wants to merge 6 commits into
python-fides-approval-bindingfrom
bookish-invention
Draft

Python: keep MCP labels subordinate to local FIDES policy#8187
Eduard van Valkenburg (eavanvalkenburg) wants to merge 6 commits into
python-fides-approval-bindingfrom
bookish-invention

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

MCP servers control result _meta.ifc and 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

  • What are the major changes? Combines MCP result labels with locally derived labels using the existing most-restrictive FIDES semantics, treats openWorldHint=False and readOnlyHint=True as non-authoritative hints, and adds regression coverage for built-in/custom parsers, structured and task results, malicious annotations, and the full integrity/confidentiality matrix.
  • What is the impact of these changes? MCP metadata can still lower integrity or raise confidentiality, but cannot raise integrity, lower confidentiality, remove local confidentiality caps, or set accepts_untrusted; explicit local per-tool annotation overrides remain the authority seam.
  • What do you want reviewers to focus on? Please focus on the restriction-only merge at MCP ingestion and preservation of existing parser, host-payload, task-result, and local provenance behavior.

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

  • 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 the title prefix in sync automatically.

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:

@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)): 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

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.

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

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.

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.

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

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant