feat(policies): additive structured policy-check contract#1594
Merged
imran-siddique merged 8 commits intoApr 30, 2026
Merged
Conversation
github-actions
Bot
added
documentation
🤖 AI Agent: docs-sync-checker — Docs SyncDocs Sync
Please ensure these documentation updates are made. |
🤖 AI Agent: breaking-change-detector — API CompatibilityAPI CompatibilityNo breaking changes detected. |
🤖 AI Agent: security-scanner — View detailsNo security issues found. |
🤖 AI Agent: code-reviewer — View detailsTL;DR: 0 blockers, 1 warning. Solid foundation for structured policy-check contract; minor follow-up suggested.
Action items: None. Warnings:
|
🤖 AI Agent: test-generator — `agent_os/policies/decision.py`
|
PR Review Summary
Verdict: ❌ Changes needed |
eltoncarr-ms
changed the title
23 tasks
eltoncarr-ms
force-pushed
the
dev/eltonc/agt-unify-policy-decisions-foundation
branch
from
78a0415 to
6edec5f
Compare
1 task
imran-siddique
requested changes
Apr 29, 2026
Member
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
TL;DR: 2 blockers, 3 warnings. Fix #1 and #2 and this ships.
| # | Sev | Issue | Where |
|---|---|---|---|
| 1 | Block | Async path silently bypasses subclass overrides of legacy pre_execute/post_execute |
base.py async_pre_execute_check |
| 2 | Block | **result.audit_entry spread in from_check_result can overwrite explicit detail keys |
exceptions.py:68 |
| 3 | Warn | PolicyCheckResult defaults allowed=True (fail-open by construction) |
decision.py:51 |
| 4 | Warn | Full user input passed to deny_blocked_pattern_input, one redact_user_text=False from leaking |
base.py pre_execute_check |
| 5 | Warn | ADR index jumps 0009 to 0011, missing ADR-0010 | docs/adr/index.md |
#1: Before this PR, async_pre_execute dispatched through self.pre_execute(ctx, input_data), respecting subclass overrides. Now it calls self.pre_execute_check() directly, bypassing any adapter that overrides only legacy pre_execute. In a governance framework, a silently skipped policy check is a policy bypass. Fix: delegate through the legacy method or check for subclass override before calling the new path.
#2: **result.audit_entry is spread after explicit keys like category, detail. A crafted audit_entry with {"category": "benign"} would silently overwrite the real violation category. Fix: spread audit_entry first, or namespace under "audit" sub-key.
Warnings are fine as follow-up PRs. Test coverage (120 new tests) and the decision_factory sanitization pattern are excellent. No internal/confidential content in the ADR or security audit doc.
Adds an additive structured policy-check contract for integration-layer governance, addressing the policy-internals leak surfaces enumerated in .plans/agt-unify-policy-decisions.md (PR (a)+(b) foundation). * New module `agent_os.policies.decision` with `ViolationCategory` enum and `PolicyCheckResult` Pydantic model (`to_legacy_tuple`/`to_public_dict` serializers). * New module `agent_os.policies.decision_factory` — single source of truth for denial result construction; sanitized public-message templates keyed by category. * `PolicyViolationError.from_check_result` classmethod (additive); legacy `(message, error_code, details)` constructor preserved verbatim. `str(e)` is the sanitized public_message; `e.details['detail']` retains audit fidelity. * `BaseIntegration` gains `pre_execute_check` / `post_execute_check` (sync+async) returning `PolicyCheckResult`. Legacy tuple methods reimplemented as thin wrappers; reason strings byte-identical (AC-5). * `AsyncGovernedWrapper` and `PolicyInterceptor` migrated internally to `*_check` variants. External adapter API unchanged. * ADR docs/adr/0011-additive-policy-check-contract.md with Host Migration Guide (AC-15, AC-16). 0009 RATS unaffected. * AGENTS.md updated with one-paragraph opt-in snippet (T13). Tests: 120 new (test_policy_check_result_no_leak, test_policy_violation_error_safety, test_legacy_constructors, test_pre_execute_check_contract, test_public_api_surface). Baseline 257 still green. Zero new ruff or mypy findings on touched files. Refs microsoft#1574 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds pytest, mypy, isort, pyupgrade, Pydantic, Docstrings, classmethod, xfail, Zava.
Removes a host name from the migration example (uses generic phrasing) and drops local OneDrive file:/// links from the references section. Also removes the now-unused dictionary entry.
Restore the original ordering of pre-existing imports; the additive PR only inserts the single .decision line and two __all__ entries. The earlier reorder was an isort auto-fix not required by CI lint rules (which use --select E,F,W only).
Reverts the global .cspell-repo-terms.txt additions. Reword classmethod and xfail in ADR 0011 (PR-introduced terms). Add an in-file cspell:ignore directive to AGENTS.md for pre-existing technical terms (pytest, mypy, isort, pyupgrade, Pydantic, Docstrings) that were inherited debt surfaced by editing the file.
Restore classmethod and xfail in ADR 0011 (their precise meaning matters). Drop the cspell:ignore directive from AGENTS.md and instead track all 8 surfaced terms in .cspell-repo-terms.txt: classmethod, Docstrings, isort, mypy, Pydantic, pytest, pyupgrade, xfail.
…ntract Satisfies scripts/ci/security-audit-required.sh gate for capability paths touched in agent_os/policies/. Documents threat model impact (information leakage reduced; no new powers; no policy-bypass surface), mitigations, and the 120-test coverage matrix. Refs microsoft#1574
Removes the smoketest filename reference that triggered cspell. The local smoke script is not part of this PR.
eltoncarr-ms
force-pushed
the
dev/eltonc/agt-unify-policy-decisions-foundation
branch
from
a530bc0 to
ee2753f
Compare
imran-siddique
approved these changes
Apr 30, 2026
imran-siddique
approved these changes
Apr 30, 2026
Member
imran-siddique
left a comment
imran-siddique
left a comment
There was a problem hiding this comment.
Approving - will address blockers in follow-up PRs.
eltoncarr-ms
deleted the
dev/eltonc/agt-unify-policy-decisions-foundation
branch
imran-siddique
pushed a commit
to imran-siddique/agent-governance-toolkit
that referenced
this pull request
May 4, 2026
…1594) * feat(policies): additive PolicyCheckResult + decision factories Adds an additive structured policy-check contract for integration-layer governance, addressing the policy-internals leak surfaces enumerated in .plans/agt-unify-policy-decisions.md (PR (a)+(b) foundation). * New module `agent_os.policies.decision` with `ViolationCategory` enum and `PolicyCheckResult` Pydantic model (`to_legacy_tuple`/`to_public_dict` serializers). * New module `agent_os.policies.decision_factory` — single source of truth for denial result construction; sanitized public-message templates keyed by category. * `PolicyViolationError.from_check_result` classmethod (additive); legacy `(message, error_code, details)` constructor preserved verbatim. `str(e)` is the sanitized public_message; `e.details['detail']` retains audit fidelity. * `BaseIntegration` gains `pre_execute_check` / `post_execute_check` (sync+async) returning `PolicyCheckResult`. Legacy tuple methods reimplemented as thin wrappers; reason strings byte-identical (AC-5). * `AsyncGovernedWrapper` and `PolicyInterceptor` migrated internally to `*_check` variants. External adapter API unchanged. * ADR docs/adr/0011-additive-policy-check-contract.md with Host Migration Guide (AC-15, AC-16). 0009 RATS unaffected. * AGENTS.md updated with one-paragraph opt-in snippet (T13). Tests: 120 new (test_policy_check_result_no_leak, test_policy_violation_error_safety, test_legacy_constructors, test_pre_execute_check_contract, test_public_api_surface). Baseline 257 still green. Zero new ruff or mypy findings on touched files. Refs microsoft#1574 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(spell): add foundation PR terms to cspell dictionary Adds pytest, mypy, isort, pyupgrade, Pydantic, Docstrings, classmethod, xfail, Zava. * docs(adr): remove personal/internal refs from ADR 0011 Removes a host name from the migration example (uses generic phrasing) and drops local OneDrive file:/// links from the references section. Also removes the now-unused dictionary entry. * chore(policies): minimize __init__.py churn Restore the original ordering of pre-existing imports; the additive PR only inserts the single .decision line and two __all__ entries. The earlier reorder was an isort auto-fix not required by CI lint rules (which use --select E,F,W only). * chore(spell): scope spell-check fix to PR-introduced terms Reverts the global .cspell-repo-terms.txt additions. Reword classmethod and xfail in ADR 0011 (PR-introduced terms). Add an in-file cspell:ignore directive to AGENTS.md for pre-existing technical terms (pytest, mypy, isort, pyupgrade, Pydantic, Docstrings) that were inherited debt surfaced by editing the file. * chore(spell): account for terms in repo dictionary Restore classmethod and xfail in ADR 0011 (their precise meaning matters). Drop the cspell:ignore directive from AGENTS.md and instead track all 8 surfaced terms in .cspell-repo-terms.txt: classmethod, Docstrings, isort, mypy, Pydantic, pytest, pyupgrade, xfail. * docs(security-audit): add audit doc for additive PolicyCheckResult contract Satisfies scripts/ci/security-audit-required.sh gate for capability paths touched in agent_os/policies/. Documents threat model impact (information leakage reduced; no new powers; no policy-bypass surface), mitigations, and the 120-test coverage matrix. Refs microsoft#1574 * docs(security-audit): drop reference to local smoke script Removes the smoketest filename reference that triggered cspell. The local smoke script is not part of this PR. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a structured policy-check contract for integration-layer governance, addressing policy-internals leak surfaces. This is a purely additive foundation: ΓÇö no existing public API changes, no behavior change for existing callers.
Why: Adapter denial sites currently leak policy internals (raw regex, allow-list contents, limit numbers) into user-visible error text, and hosts have no programmatic way to dispatch on violation category without substring-matching free-form English. This PR introduces the structured contract that fixes both — without changing any existing public API.
Delivery plan: Foundation is split across a PR stack :
xfailallowlistChanges
agent_os.policies.decisionwithViolationCategoryenum andPolicyCheckResultPydantic model (to_legacy_tuple/to_public_dictserializers).agent_os.policies.decision_factoryΓÇö single source of truth for denial result construction; sanitized public-message templates keyed by category.PolicyViolationError.from_check_resultclassmethod (additive); legacy(message, error_code, details)constructor preserved verbatim.str(e)is the sanitized public_message;e.details["detail"]retains audit fidelity.BaseIntegrationgainspre_execute_check/post_execute_check(sync + async) returningPolicyCheckResult. Legacy tuple methods reimplemented as thin wrappers; reason strings byte-identical.AsyncGovernedWrapperandPolicyInterceptormigrated internally to*_checkvariants. External adapter API unchanged.docs/adr/0011-additive-policy-check-contract.mdwith Host Migration Guide.agent-governance-python/agent-os/AGENTS.mdupdated with one-paragraph opt-in snippet.Backward compatibility
Follow-ups (separate PRs, tracked in plan)
from_check_result(...)to remove user-visible regex / allow-list / limit interpolation.Type of Change
Package(s) Affected
Checklist
Attribution & Prior Art
Prior art / related projects (if any):
None.
AI & IP Disclosure