Repository navigation
fix(agent-os): close three fail-open gaps from issue #3520 - #3524
MohammadHaroonAbuomar merged 2 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Welcome to the Agent Governance Toolkit! Thanks for your first pull request. |
🤖 AI Agent: docs-sync-checker — Docs Sync
Docs Sync
|
🤖 AI Agent: security-scanner — View details
No security issues found. |
🤖 AI Agent: contributor-guide — View details
Welcome, and thank you for contributing! Great job addressing multiple security gaps and including thorough regression tests. Before merging:
For guidance, please review CONTRIBUTING.md. |
🤖 AI Agent: test-generator — `agent-os/src/agent_os/policies/context_accumulation.py`
|
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
There was a problem hiding this comment.
Pull request overview
TL;DR: 0 blockers, 0 warnings. No issues found. Clean change.
Changes:
- Tightened ABAC geography enforcement to deny when a geography is required but missing/mismatched.
- Fixed context-action gating to consult aggregation-effective restrictions (including rule-implied restrictions) immediately.
- Prevented fail-open aggregation rules by rejecting empty
all_labels, with regression tests covering all three cases.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
agent-governance-python/agent-os/src/agent_os/policies/data_classification.py |
Fail-closed geography requirement when a required geography is configured. |
agent-governance-python/agent-os/src/agent_os/policies/context_accumulation.py |
Gate actions based on aggregation-effective restrictions and emit obligations from that same set. |
agent-governance-python/agent-os/src/agent_os/policies/context_aggregation.py |
Reject empty all_labels in AggregationRule to avoid match-all behavior defeating the backstop. |
agent-governance-python/agent-os/tests/test_data_classification.py |
Regression test for missing geography denial when geography is required. |
agent-governance-python/agent-os/tests/policies/test_context_accumulation.py |
Regression test ensuring rule-implied restrictions gate immediately. |
agent-governance-python/agent-os/tests/policies/test_context_aggregation.py |
Regression test ensuring empty all_labels is rejected. |
|
@microsoft-github-policy-service agree |
|
The To clear it, add |
|
#3523 fixes the same Could you drop gap #2 and keep gaps #1 (geography default) and #3 (empty Separately, the |
- Geography default fail-open: require explicit geography match when policy.required_geography is set; absent geography no longer bypasses. - Empty all_labels rejected: AggregationRule.__post_init__ raises ValueError when all_labels is empty, preventing silent backstop defeat. The restriction-gating gap (decide_next reading env.restrictions) is covered by microsoft#3523 and intentionally left out of this PR. Each fix ships with a regression test that would fail without the change, plus a security audit doc under docs/security/audits/. Signed-off-by: Jainil Gosalia <jainil.gosalia@gmail.com>
30c256d to
23fcf8a
Compare
github-actions
Bot
added
documentation
|
Status check: this is now approved with every check green, including the Security Audit gate that was blocking it on July 30. It is the only PR in the open queue in that state, so it just needs someone with merge rights to take it. MohammadHaroonAbuomar liamcrumm |
628b274
into
microsoft:main
) The 2026-07-31 fail-open closure audit doc merged in #3524 without a frontmatter block, but the strict frontmatter gate added in #3352 scans the whole tree (--root .). main therefore fails its own gate, and every open PR rebased onto main inherits the failure. Add the title/last_reviewed/owner block matching the sibling audit docs so `check_frontmatter.py --root . --strict` reports 0 findings. Signed-off-by: AlgoVoi <chopmob@gmail.com>
Closes #3520
Two fail-open security gaps where the Python reference was laxer than the sibling SDK ports:
1. Geography default fail-open (\data_classification.py)
Absent geography (empty string) no longer bypasses
equired_geography. Changed from \policy.required_geography and data_label.geography and ...\ to \policy.required_geography is not None and data_label.geography != ....
2. Empty \�ll_labels\ matches everything (\context_aggregation.py)
\AggregationRule.post_init\ raises \ValueError\ when \�ll_labels\ is empty, preventing a rule from matching every envelope and suppressing the escalation backstop.
The restriction-gating gap from #3520 (the \decide_next\ gate reading pre-existing \�nv.restrictions\ instead of the aggregation-effective set) is covered by #3523 and is intentionally not included here to avoid overlapping changes.
Both fixes ship with regression tests that fail without the change, plus a security audit doc (\docs/security/audits/2026-07-31-fail-open-closure-python-reference.md) to clear the Security Audit Required gate.