Python: [Bug]: A confidentiality violation whose approval binding cannot be built is ALLOWED, not blocked, when `approval_on_violation=True`

[antsok]

Description

In PolicyEnforcementFunctionMiddleware.process, a detected violation is resolved by this ladder (security.py, core 1.19.0):

binding: _PendingPolicyApproval | None = None
approved = False
if self.approval_on_violation:
    try:
        binding = self._pending_record(context, violations)
    except (TypeError, ValueError, OverflowError):
        self._block_unsafe_approval_binding(context, context_label=context_label)
    approved = self._matches_pending_approval(context, binding)

if approved:
    ...
elif binding is not None:
    self._request_policy_violation_approval(...)
elif self.block_on_violation:
    self._block_policy_violation(...)
else:
    logger.warning("WARNING: Tool '%s' policy violation(s) [%s] (allowed)", function_name, disclosed)

await call_next()

When _pending_record raises, _block_unsafe_approval_binding does not terminate the invocation, so control falls through with binding is None. approved is False, the elif binding is not None arm is skipped, and block_on_violation is False — forced so by the constructor, self.block_on_violation = block_on_violation if not approval_on_violation else False (line 2421). Execution therefore reaches the final else, logs "(allowed)", and runs await call_next(). The violating call executes.

The same violation with approval_on_violation=False is blocked correctly. So the safer-looking configuration — ask the human instead of refusing outright — is the one that can silently allow.

Reproduction

New in 1.19.0 (PR #8238, "Tighten security label enforcement"), a USER_IDENTITY label must carry a canonical principal set under agent_framework.security.principals. A tool that declares confidentiality: "user_identity" the pre-1.19 way produces a label without it; _canonical_principals then raises ValueError, which is exactly the exception the ladder above swallows.

1. A tool declaring {"source_integrity": "trusted", "confidentiality": "user_identity"} and no principals runs, tainting the context to USER_IDENTITY.
2. A second tool declaring max_allowed_confidentiality: "private" is called.
3. Enforcer configured with approval_on_violation=True.

Observed on core 1.19.0, with the violation correctly detected and logged:

WARNING agent_framework.security: Invalid USER_IDENTITY source principals for tool 'reads_user_identity': tool reads_user_identity principals must be a non-empty sequence
WARNING agent_framework.security: Policy violation detected
VIOLATION: {'type': 'confidentiality_violation', 'subtype': 'max_allowed_confidentiality',
            'function': 'microsoft_docs_fetch',
            'reason': 'Cannot write USER_IDENTITY data to PRIVATE destination (data exfiltration blocked)'}

…and the call then completing normally, returning a real result list. On core 1.18.0 the same sequence raised an approval request.

Two defects, separable

1. The fall-through is the real bug. Whatever the reason an approval binding cannot be built, "we could not ask the user" must not degrade to "so we allowed it". The except arm should block (or re-raise) rather than continue, independently of block_on_violation — which the caller cannot even set while approval is on.
2. The trigger is a silent contract change. A USER_IDENTITY label without principals is no longer a label that enforces; _read_source_principals logs a warning and returns {}. Anything that declared confidentiality: "user_identity" before 1.19.0 loses enforcement on upgrade, and the only signal is a WARNING line.

Code Sample

Error Messages / Stack Traces

Package Versions

agent-framework-core: 1.19.0

Python Version

No response

Additional Context

Suggested fix

In the except (TypeError, ValueError, OverflowError) arm, treat the failure as a block: call _block_policy_violation(...) (or have _block_unsafe_approval_binding raise) so the ladder cannot reach the permissive else. A defence that cannot obtain a decision should refuse, not proceed.

[github-actions]

👋 Hi Anton Sokolovskyi (@antsok) — thanks for the report!

Your report identifies agent-framework-core 1.19.0 and shows the missing-principals warning followed by a policy violation, but that release’s _block_unsafe_approval_binding raises MiddlewareTermination; the exact sequence leaves the sink unexecuted. This suggests your process may be loading a different or modified module. Worth checking agent_framework.security.__file__ and the installed function source from the same interpreter.

If you still believe there is a framework issue here, please reply with the exact import/class or method you are using, the version, and a minimal snippet that shows the failure. 🙏

[he-yufeng]

I can't reproduce the fall-through this report builds on, and I tried hard. In both current main and the published 1.19.0 wheel, _block_unsafe_approval_binding is declared NoReturn and ends with raise MiddlewareTermination("Unsafe policy approval binding") (security.py:2755-2764 in the 1.19.0 wheel). When _pending_record raises, the except handler calls it, the raise propagates out of _process_in_scope before approved = ... is ever reached, and the pipeline's top-level contextlib.suppress(MiddlewareTermination) (_middleware.py:1238) returns the error result. call_next is never invoked, so the final "(allowed)" branch is unreachable in this configuration.

Direct reproduction against current main (clone at 6f1522a):

• PolicyEnforcementFunctionMiddleware(approval_on_violation=True)
• _pending_record patched to raise ValueError("tool reads_user_identity principals must be a non-empty sequence") (the exact exception type the report cites)
• invocation label UNTRUSTED, so a real violation is detected

Observed: Policy violation detected is logged, then MiddlewareTermination (Unsafe policy approval binding); call_next never ran; context.result is the unsafe_approval_binding error dict.

So the ladder appears to block correctly in the configuration described. Could you share the full traceback (and the middleware list) from the failing run? One plausible alternative: the ValueError from _canonical_principals is raised earlier, while tainting the context on the first tool, in a different layer than the enforcer ladder, and the "(allowed)" line belongs to a different invocation. Happy to dig further with the actual logs.