Python: Tighten security label enforcement - #8238
Python: Tighten security label enforcement#8238Eduard van Valkenburg (eavanvalkenburg) wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Public API validation, sensitive metadata logging, and approval-resume integration coverage need correction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Tightens Python security-label authority, identity-principal enforcement, approval binding, and variable expansion limits.
Changes:
- Restricts embedded labels unless framework-authoritative.
- Enforces canonical tenant/user principals across flows and approvals.
- Adds bounded transitive variable expansion and security regressions.
File summaries
| File | Description |
|---|---|
python/packages/core/agent_framework/security.py |
Implements security enforcement changes. |
python/packages/core/tests/test_security.py |
Adds principal, approval, label, expansion, and isolation tests. |
python/packages/hosting-a2a/tests/hosting_a2a/test_conversion.py |
Verifies remote A2A metadata cannot upgrade integrity. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): f183929bf5e1
Model: gpt-5.6-sol-fast
Overview
The change establishes strong authority boundaries through process-local label stamps, canonical tenant/user principal sets, principal-bound approval records, and bounded transitive variable expansion. Tests cover forged labels, principal drift, cross-session isolation, malformed principal state, cycles, and traversal limits. Two Medium compatibility gaps remain: the new public helper does not honor its advertised input types, and existing documented USER_IDENTITY configurations are blocked without corresponding migration guidance.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/security.py
f183929 to
9addf5d
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
9addf5d to
1779db9
Compare
Motivation & Context
Security labels and identity-scoped data must preserve locally established trust and ownership boundaries across tool results, approvals, sessions, and remote content conversion.
Description & Review Guide
USER_IDENTITYflows and approvals to canonical tenant/user principals, bound transitive variable expansion with cycle detection, and add a runnableuser_identity_security_example.pyshowing authenticated principal setup with allowed and blocked destinations.PUBLICandPRIVATEranking remains unchanged. The security README and developer guide now document canonical principal metadata and restriction-only mixed-trust labeling. Full core, hosting-A2A, sample lint/type, and markdown code checks pass.Related Issue
No related open issue.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.