You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Python: Harden FIDES security state and policy enforcement - #9652
A private security review identified several cases where FIDES could lose or weaken information-flow state across transcript replay, MCP result retention, hidden argument validation, durable serialization, and invalid tool metadata. These gaps could cause policy decisions to use less restrictive integrity or confidentiality information than the data warranted.
This change hardens SecureAgentConfig and the FIDES middleware pipeline while preserving restored session state and approval-resume behavior. Context-provider message labeling remains separate follow-up work because context providers are trusted plugins with an existing documented trust boundary.
Description & Review Guide
What are the major changes?
Reconstruct missing FIDES state from caller-replayed and history-provider messages, using conservative labels for unstamped assistant/tool data while keeping restored state authoritative.
Retain MCP _meta.ifc in an independently bounded security carrier so generic Host-payload budget exhaustion cannot discard the label; malformed or unretainable supplied IFC metadata fails closed.
Split hidden-argument handling into policy preflight and post-authorization materialization so expanded values do not reach Pydantic validators or tool bodies before policy permits the call.
Serialize opaque FIDES state with ASCII escapes so lone Unicode surrogates round-trip through JSON and MessagePack session stores.
Parse FIDES tool metadata centrally and block unknown or mistyped values with an auditable invalid_policy violation.
Add regression coverage across core and AG-UI approval/replay paths and update the FIDES guidance.
What is the impact of these changes?
Transcript replay without persisted FIDES state now restores a conservative cumulative label instead of starting trusted/public.
Invalid security metadata that previously warned and continued now blocks execution.
Validators no longer run on expanded hidden values for calls rejected by FIDES.
Existing restored sessions and AG-UI approval resumes retain their authoritative state and behavior.
Non-history context-provider messages remain under the documented trusted-plugin model pending the follow-up design.
What do you want reviewers to focus on?
The boundary between reconstructing missing replay state and preserving restored session state.
Approval binding and argument-materialization ordering, including validator transformations and runtime kwargs.
The independent MCP IFC carrier, its size bound, and fail-closed handling.
Related Issue
N/A - security hardening based on a privately reported issue. Per SECURITY.md, vulnerability details are not tracked in a public GitHub issue; maintainer direction was explicitly confirmed before implementation. The context-provider labeling follow-up is tracked separately in #9645 and is intentionally not closed by this PR. No competing open PR was found.
AI Assistance
No material AI assistance was used.
This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.
AI assistance details: GitHub Copilot assisted with the security assessment, implementation, regression tests, documentation updates, and PR description.
Contribution Checklist
The code builds clean without any errors or warnings
All unit tests pass, and I have added new tests where possible
This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
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.
This PR hardens Python FIDES information-flow enforcement across transcript replay, MCP results, hidden arguments, and persisted session state. Non-history context-provider labeling remains a separate follow-up.
Changes:
Reconstruct missing replay security state while preserving restored labels.
Retain MCP security metadata independently and reject invalid tool policies.
Defer hidden-value validation until authorization, support surrogate-safe serialization, and add regression tests and guidance.
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)): 634139edf460 Model:gpt-5.6-sol
Overview
The PR adds conservative replay reconstruction, independently bounded MCP IFC retention, delayed hidden-argument materialization, ASCII-safe durable state, and centralized tool-policy parsing. Its strongest guardrails are fail-closed MCP labeling, identity-bound argument materialization, unconditional ContextVar cleanup, and targeted regression coverage. Two gaps remain: restored sessions stop conservatively labeling newly supplied assistant/tool input, and explicit null enum policies bypass the new invalid-policy path.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (1 high, 1 medium) across 1 file. Details are attached to the affected lines below.
When a user abandons an unanswered client-tool call from the backend snapshot, normalization inserts a framework-generated “Tool execution skipped” result without snapshot provenance. FIDES treats that synthetic result as new UNTRUSTED/PRIVATE data, tainting the restored session and blocking unrelated protected tools. Preserve trusted provenance for synthetic results derived from backend-owned calls without trusting actual client-supplied results, and add an abandoned-call continuation regression with FIDES enabled.
Addressed the current-head Copilot review’s previously missed synthetic skipped-result issue in 4b3fd9660. AG-UI now propagates backend-snapshot provenance only to framework-generated skipped results derived from backend-owned calls; real client tool results remain untrusted. Added an abandoned-call continuation regression with FIDES enabled.
Replaying an unchanged, authenticated hidden result as ordinary caller input still applies the UNTRUSTED fallback, even with restored session state. This taints an otherwise clean session and blocks unrelated protected tools although the payload remains hidden. Exempt authenticated-placeholder-only messages from the integrity fallback, retain conservative handling for modified or visible data, and add a caller-input regression.
Addressed the latest Copilot review’s previously missed authenticated-placeholder caller-input issue in a08f596e4. Replay reconstruction now skips the UNTRUSTED fallback only when the message contains authenticated session-owned hidden placeholders and no visible data; modified placeholders and visible/function-call content remain conservative. Added a caller-input regression that preserves TRUSTED/PRIVATE and allows an unrelated protected action.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Cross-package security-state and approval-lifecycle changes require final maintainer security review and confirmation of core and AG-UI regression validation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
documentationUsage: [Issues, PRs], Target: documentation in the code base and learn docspythonUsage: [Issues, PRs], Target: Python
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation & Context
A private security review identified several cases where FIDES could lose or weaken information-flow state across transcript replay, MCP result retention, hidden argument validation, durable serialization, and invalid tool metadata. These gaps could cause policy decisions to use less restrictive integrity or confidentiality information than the data warranted.
This change hardens
SecureAgentConfigand the FIDES middleware pipeline while preserving restored session state and approval-resume behavior. Context-provider message labeling remains separate follow-up work because context providers are trusted plugins with an existing documented trust boundary.Description & Review Guide
_meta.ifcin an independently bounded security carrier so generic Host-payload budget exhaustion cannot discard the label; malformed or unretainable supplied IFC metadata fails closed.invalid_policyviolation.Related Issue
N/A - security hardening based on a privately reported issue. Per
SECURITY.md, vulnerability details are not tracked in a public GitHub issue; maintainer direction was explicitly confirmed before implementation. The context-provider labeling follow-up is tracked separately in #9645 and is intentionally not closed by this PR. No competing open PR was found.AI Assistance
AI assistance details: GitHub Copilot assisted with the security assessment, implementation, regression tests, documentation updates, and PR description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.