Repository navigation
Python: [Bug]: A confidentiality violation whose approval binding cannot be built is ALLOWED, not blocked, when approval_on_violation=True #8761
Description
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.
- A tool declaring
{"source_integrity": "trusted", "confidentiality": "user_identity"}and no principals runs, tainting the context toUSER_IDENTITY. - A second tool declaring
max_allowed_confidentiality: "private"is called. - 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
- 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
exceptarm should block (or re-raise) rather than continue, independently ofblock_on_violation— which the caller cannot even set while approval is on. - The trigger is a silent contract change. A
USER_IDENTITYlabel without principals is no longer a label that enforces;_read_source_principalslogs a warning and returns{}. Anything that declaredconfidentiality: "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.
Metadata
Metadata
Labels
Type
Projects
- Status
Activity
github-actions commentedon Sep 25, 2026
👋 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_bindingraisesMiddlewareTermination; the exact sequence leaves the sink unexecuted. This suggests your process may be loading a different or modified module. Worth checkingagent_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 commentedon Sep 25, 2026
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_bindingis declaredNoReturnand ends withraise MiddlewareTermination("Unsafe policy approval binding")(security.py:2755-2764 in the 1.19.0 wheel). When_pending_recordraises, the except handler calls it, the raise propagates out of_process_in_scopebeforeapproved = ...is ever reached, and the pipeline's top-levelcontextlib.suppress(MiddlewareTermination)(_middleware.py:1238) returns the error result.call_nextis 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_recordpatched to raiseValueError("tool reads_user_identity principals must be a non-empty sequence")(the exact exception type the report cites)Observed:
Policy violation detectedis logged, thenMiddlewareTermination (Unsafe policy approval binding);call_nextnever ran;context.resultis theunsafe_approval_bindingerror 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
ValueErrorfrom_canonical_principalsis 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.leilei3167 commentedon Sep 26, 2026
Thanks for the detailed write-up — the fall-through when approval binding construction fails looks like a real safety bug. I am looking into a minimal fix that blocks instead of allowing the violating call when
approval_on_violation=True.Python: fail closed when approval binding is unavailable (microsoft#8761
antsok commentedon Sep 30, 2026
A correction, for the record. Yufeng He (@he-yufeng) and the triage bot were right: the fall-through I described does not happen at 1.19.0.
I re-ran the exact sequence against both
72325696(the pin in the report) and currentmain. In both,_block_unsafe_approval_bindingterminates, the result is theunsafe_approval_bindingrefusal, and the destination tool never runs. My mistake was in reading the result. Our label-tracking layer renders a refusal as a list ofContentitems, and I took that list for the tool's real output. The report quotes no(allowed)line because none was logged.What 1.19.0 actually changed for us is narrower: a
USER_IDENTITYlabel without principals is now refused instead of offered for approval. That's a behaviour change, not a leak.#8780 is still a good change, since it makes the permissive branch unreachable even if the terminating call ever returns. Thanks to everyone who dug into it.