Skip to content

Python: fix FIDES session isolation and runtime integration gaps - #7528

Open
lerelerele wants to merge 1 commit into
microsoft:mainfrom
lerelerele:codex/fides-7455-fixes
Open

Python: fix FIDES session isolation and runtime integration gaps#7528
lerelerele wants to merge 1 commit into
microsoft:mainfrom
lerelerele:codex/fides-7455-fixes

Conversation

@lerelerele

Copy link
Copy Markdown

I have implemented the A1 to C3 items from this issue locally and I would like to open a PR for it. Flagging it here first, per the repo's suggested workflow, since it is a sizeable change and it touches behavior.

What it covers

The core change is architectural: conversation scoped FIDES state moves out of shared middleware attributes and into AgentSession.state, so a middleware or Agent instance reused across concurrent conversations can no longer leak context labels, audit records or pending approvals between them.

  • A1: context label, audit log, pending approvals and counters live in AgentSession.state; variable stores are keyed per session.
  • A2: a blocked call now returns a correlated function_result instead of terminating the loop with no content, so the model can explain the refusal or choose another action. Approval requests still pause for the user.
  • A3: audit records carry a per run turn and a per call call_index instead of the constant -1.
  • B1: tool_labels configuration for tools the application did not construct, such as harness and MCP tools.
  • B2: approval request additional_properties propagate into the reconstructed function call.
  • B3: standing approval rules and auto approval callbacks can no longer approve a FIDES policy violation.
  • B4: public build_function_call_content() extension point, with the former private method kept as an alias.
  • B5: deny_untrusted_tools, where deny takes precedence over allow.
  • C1: enable_quarantine=False, so labels and policy enforcement can be used without quarantine tooling.
  • C2: the quarantine client is bound to the current async context instead of a process global slot.
  • C3: MCP read only tools receive a PUBLIC confidentiality cap, since their arguments still leave the process, with a granular mark_read_tools_as_sinks opt out that leaves the write side gate untouched.

Middleware order is unchanged, and direct middleware calls made without an AgentSession keep working through explicit fallbacks.

Observable behavior changes

Every symbol this touches is marked @experimental(feature_id=ExperimentalFeature.FIDES), so I have not treated this as a breaking change, but I want these four called out before anyone reviews the diff:

  1. A blocked call now carries exception, so it participates in max_consecutive_errors_per_request. With the default limit, three consecutive blocked calls disable tools for the remainder of that request. This is a deliberate circuit breaker against retry loops.
  2. MCP tools annotated readOnlyHint=True now get max_allowed_confidentiality=PUBLIC by default.
  3. set_quarantine_client() is context local rather than process global, and the SecureAgentConfig constructor no longer registers the client. before_run() binds it, and it never clears a client another caller installed.
  4. The public accessors take an optional session. Without one they resolve the last session seen in the current async context.

If you would rather treat any of these as breaking, say so and I will follow the process for that instead.

Two additions this issue did not ask for

I would like agreement on these before opening the PR, since new APIs are supposed to be discussed first:

  • max_audit_log_entries (default 1000, None for unlimited). Once the audit log lives in AgentSession.state it is serialized to durable storage, so an unbounded log would grow the session payload without limit.
  • begin_turn(session). A3 asks for a real turn number, and without a run boundary hook the counter can only be per tool call, which is not what the audit field means.

What it does not cover

This does not address #7466. It does not make SecureAgentConfig the default, does not add it to samples, and does not change the public documentation or learning path.

Verification

Python 3.12. 21 new regression tests in python/packages/core/tests/test_fides_7455_regressions.py, including one that asserts a blocked call leaves no unanswered function call in the invocation loop. Full core unit suite: 3926 passed, 18 skipped, 2 xfailed. Ruff format, Ruff check and Pyright are clean on the changed files.

Open questions for maintainers

  1. The session.state["_fides"] key names and shape, since they become part of the durable session contract.
  2. Whether variable content stores should persist through an official AgentSession mechanism rather than a session keyed store on the middleware.
  3. Whether B3 should be a hard block, as implemented, or a configurable policy that lets a host callback decide.
  4. Whether the MCP argument cap should also be configurable per server, in addition to the two global flags.

Happy to split this into smaller PRs per group if you prefer to review each fix independently.

FIDES kept conversation scoped security state on shared middleware
instances, so a single Agent or middleware instance serving more than
one conversation could leak context labels, audit records and pending
approvals across conversation boundaries. A blocked tool call also
terminated the invocation loop without a visible result, which could
reach the user as an empty response.

Move conversation scoped state into AgentSession.state and address the
A1 to C3 items from microsoft#7455:

- A1: context label, audit log, pending approvals and counters live in
  AgentSession.state; variable stores are keyed per session.
- A2: a blocked call returns a correlated function_result instead of
  terminating with no content, so the model can explain the refusal or
  choose another action. Approval requests still pause for the user.
- A3: audit records carry a per run turn and a per call call_index
  instead of the constant -1.
- B1: new tool_labels configuration for tools the application did not
  construct, such as harness and MCP tools.
- B2: approval request additional_properties propagate into the
  reconstructed function call.
- B3: standing approval rules and auto approval callbacks can no longer
  approve a FIDES policy violation.
- B4: new public build_function_call_content() extension point; the
  former private method remains as an alias.
- B5: new deny_untrusted_tools; deny takes precedence over allow.
- C1: new enable_quarantine=False so labels and policy enforcement can
  be used without quarantine tooling.
- C2: the quarantine client is bound to the current async context
  instead of a process global slot.
- C3: MCP read only tools receive a PUBLIC confidentiality cap, since
  their arguments still leave the process, with a granular
  mark_read_tools_as_sinks opt out.

Also hardens durable state: label metadata is sanitized before it
reaches session storage, and the audit log is capped per session with a
configurable limit.

Middleware order is unchanged, and direct middleware calls made without
an AgentSession keep working through explicit fallbacks.

Adds 21 regression tests in tests/test_fides_7455_regressions.py.
Copilot AI lite review requested due to automatic review settings August 5, 2026 11:33
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Aug 5, 2026
@lerelerele

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

2 participants