[leilei3167]

Motivation & Context

When PolicyEnforcementFunctionMiddleware is configured with approval_on_violation=True, a detected policy violation must not execute if an approval binding cannot be established. If _pending_record fails and _block_unsafe_approval_binding does not terminate the middleware chain, the ladder falls through with binding is None and block_on_violation=False, reaching the permissive allow path.

Closes #8761

Description & Review Guide

• What are the major changes?
  • Treat unresolved approval_on_violation flows as a block (same as block_on_violation) instead of logging (allowed) and calling call_next().
  • Add regression tests covering the legacy USER_IDENTITY source without principals plus a fail-closed test when approval binding construction is unsafe.
• What is the impact of these changes?
  • Violating tool calls are blocked when approval cannot be requested or bound; safer default for approval_on_violation=True.
• What do you want reviewers to focus on?
  • Whether blocking via _block_policy_violation is the right surface when approval binding fails, versus only raising from _block_unsafe_approval_binding.

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.

Evidence

• Old red (without ladder fix, _block_unsafe_approval_binding no-op): pytest packages/core/tests/test_security.py::TestPolicyEnforcementMiddleware::test_approval_on_violation_fail_closed_when_binding_record_fails fails with tool execution allowed.
• New green: pytest packages/core/tests/test_security.py::TestPolicyEnforcementMiddleware::test_legacy_user_identity_violation_with_approval_does_not_execute_tool packages/core/tests/test_security.py::TestPolicyEnforcementMiddleware::test_approval_on_violation_fail_closed_when_binding_record_fails

[Copilot]

Copilot review overview

🟡 Changes recommended

The fallback can still be bypassed by an AttributeError when an unavailable binding is matched against an existing pending approval.

Review effort: Balanced
Findings: 1

Open (1)

• Guard pending approval matching when binding construction fails · New

What changed in this PR

Adds fail-closed handling when policy-violation approval binding cannot be established.

Changes:

• Blocks unresolved approval flows instead of allowing tool execution.
• Adds regression tests for unsafe and legacy identity bindings.

File	Description
python/​packages/​core/​agent_framework/​security.py	Adds approval-mode fallback blocking.
python/​packages/​core/​tests/​test_security.py	Adds fail-closed regression coverage.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[leilei3167]

Evan Mattson (@moonbox3) the call site now raises MiddlewareFailure if _block_unsafe_approval_binding() returns. Could you take another look at 5c54273 when you have a moment?

[Eduard van Valkenburg (eavanvalkenburg)]

leilei3167 Looks good to me.