Repository navigation
Python: Harden FIDES core policy foundation - #9875
Eduard van Valkenburg (eavanvalkenburg) wants to merge 2 commits into
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
documentation
Code coverage overviewLanguages: Python Python / code-coverage/python
View coverage of the 3 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
There was a problem hiding this comment.
🟡 Changes recommended
Critical authorization gaps omit runtime kwargs from checks, materialize hidden values before policy approval, and permit validator-transformed arguments to execute without renewed authorization.
3 open issues
What changed in this PR
This PR establishes the core Python FIDES hardening layer, separating policy preflight from hidden-value materialization ahead of later transport and replay work.
Changes:
- Centralizes fail-closed tool metadata parsing and audits invalid declarations.
- Adds detached argument preflight and invocation-boundary authority checks.
- Makes durable state surrogate-safe and adds regression tests and developer guidance.
| File | Description |
|---|---|
python/samples/02-agents/security/FIDES_DEVELOPER_GUIDE.md |
Documents authorization ordering and strict metadata validation. |
python/packages/core/tests/test_security.py |
Adds policy, hidden-value, runtime-mutation, retry, and serialization regressions. |
python/packages/core/agent_framework/security.py |
Implements policy parsing, detached preflight, authority checks, and ASCII-escaped state. |
python/packages/core/agent_framework/_tools.py |
Adds a materialization hook to tool invocation. |
python/packages/core/agent_framework/_middleware.py |
Tracks active pipelines and materializes arguments before the final handler. |
Review effort: Balanced
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): 6e36e0bcee9d
Model: gpt-5.6-sol
Overview
The PR centralizes FIDES metadata parsing, separates hidden-value preflight from materialization, and adds final-handler and retry checks backed by focused regressions. Its task-local materialization state is reset reliably, and recognized invalid metadata values now fail closed. Two residual gaps remain: misspelled security keys can still remove a declared restriction, and unsupported runtime authority is omitted from mutation binding.
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.
Affected areas: python/packages/core/agent_framework/security.py
GitHub Actions
Inactive
GitHub Actions
Inactive
27c6d64 to
b04ad3b
Compare
GitHub Actions
Inactive
| args = cast(Any, context.arguments) | ||
| if isinstance(args, Mapping) and arg_name in args: | ||
| return cast(Any, args[arg_name]), "arguments" | ||
| return _top_level_argument_value_from(context.arguments, context.kwargs, arg_name) |
There was a problem hiding this comment.
why do we need to pass this to a new function? can't we just update this one?
|
|
||
| def _top_level_argument_keys(context: FunctionInvocationContext) -> set[str]: | ||
| """Union of top level keys in context.arguments and context.kwargs.""" | ||
| return _top_level_argument_keys_from(context.arguments, context.kwargs) |
There was a problem hiding this comment.
same here
Motivation & Context
A private security review identified cases where FIDES could validate or execute a tool call with weaker authority than
the hidden values and runtime arguments inspected by policy. Invalid tool metadata could also fall back to permissive
defaults, and opaque session state containing lone Unicode surrogates could fail durable serialization.
This first replacement layer establishes the core FIDES policy foundation for later transport and replay layers. It
keeps policy checks ahead of hidden-value materialization and validator execution, binds authorization to exact
materialized arguments and runtime kwargs, and preserves JSON-safe session state.
Description & Review Guide
invalid_policy.policy middleware authorizes the call.
isolation and direct tool invocation.
test_security.pyand update the FIDES developer guidance.cd python && uv run --directory packages/core pytest tests/test_security.py -q -k 'invalid_tool_policy_is_blocked or hidden_surrogate_round_trips_in_file_session_store or all_reference_forms_resolve_and_block_untrusted_arguments or blocked_hidden_value_never_reaches_argument_validator or later_policy_blocks_before_hidden_value_validation or equal_value_reference_substitution_fails_closed or runtime_kwarg_mutation_after_approval_fails_closed or runtime_session_substitution_after_approval_fails_closed or callable_policy_blocks_before_hidden_value_validation or hidden_argument_retry_rematerializes_from_original_reference or retry_after_policy_revalidates_materialized_runtime_authority'cd python && uv run --directory packages/core pytest tests/test_security.py -qcd python && uv run poe syntax -P corecd python && uv run poe pyright -P corecd python && uv run poe test-typing -P corecd python && uv run poe test -P coresecurity.pyand the minimal final-handler hooks in_middleware.pyand_tools.py.continuation, AG-UI transport, and MCP result/carrier handling. Those remain later replacement layers.
Related Issue
N/A - security hardening based on a privately reported issue. Per
SECURITY.md, vulnerability details are not trackedin a public GitHub issue; maintainer direction was explicitly confirmed before implementation. This draft is layer 1
of the replacement stack for #9652 and isolates only the core policy foundation rather than competing with the full
mixed-scope implementation.
AI Assistance
AI assistance details: GitHub Copilot assisted with adapting the implementation, regression tests, documentation, and
this 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.