fix(agent-mesh): support contains/startswith/endswith in PolicyRule condition DSL by karimad · Pull Request #3924 · microsoft/agent-governance-toolkit

Karim Mehalebi (karimad) · 2026-09-10T09:11:11Z

Related Issue

None yet — opening as draft directly against the reproduced bug; happy to file an issue first if preferred.

Problem & Solution

Problem: PolicyRule._eval_expression (agent-mesh/src/agentmesh/governance/policy.py) only recognizes ==, !=, in [...], numeric comparisons, and bare boolean-attribute conditions. Any condition written with contains/startswith/endswith falls through every regex branch and hits the final return False — a silent no-match, not an evaluation error, so it never triggers the existing fail-closed exception handling (the V27 comment at line ~159). A deny rule written with one of these operators quietly never fires, with no warning at lint or policy-load time.

Reproduced end-to-end through govern(): a policy rule action.tool startswith 'delete-' never matched, so a delete-* tool call that should have been denied was allowed.

Solution: Add contains, startswith, endswith as recognized operators, following the existing re.match-per-operator dispatch pattern. Deliberately plain string operators, not regex — this repo already has open issues about ReDoS surface in other regex-based scanners, so I didn't want to add another one. Both new operators are type-safe against non-string field values (return False rather than raising).

Impact on Your Work

I'm building a small AGT-governed MCP setup (gateway + browser-tool bridge) and hit this while writing a policy rule with startswith. The rule silently never matched — no error, no warning — which is a dangerous failure mode for a deny rule specifically.

Timeline

None.

Alternatives Considered

Considered adding a general regex/matches operator instead, but scoped this PR to the three plain-string operators to avoid ReDoS surface. Happy to open a follow-up for matches with bounded/timeout-protected regex if that's wanted.

Update — federation.py parity + hardening (self-review pass)

While preparing this PR I noticed federation.py's OrgPolicyRule/_eval_expression is a hand-copied sibling of PolicyRule._eval_expression, used for cross-org trust-agreement rules. It had the same missing-operator gap this PR fixes (only ==/bare-boolean), plus a fail-open exception handler (the opposite of PolicyRule's fail-closed V27 pattern) and no DoS depth/length guards. None of this came from a GitHub review comment — I found it myself during a self-review pass over the sibling evaluator, before requesting human review — flagging the provenance here since there's no PR comment thread to point a reviewer to. Fixed in two follow-up commits with matching tests in test_org_policy_rule_string_operators.py.

Known pre-existing gap, not introduced or fixed by this PR: both evaluators use re.match (not re.fullmatch), so trailing garbage after a valid condition is silently ignored (e.g. "action.type == 'export' ) drop table" matches as if only action.type == 'export' were written). Not filed as a separate issue yet — flagging here for visibility.

Type of Change

Bug fix (non-breaking change that fixes an issue)

Package(s) Affected

agent-mesh

Testing

Unit Testing

Added agent-governance-python/agent-mesh/tests/test_policy_rule_string_operators.py: contains/startswith/endswith match and no-match cases, type-safety against non-string field values, composition with existing and/or, and a regression check that ==/!=/in/numeric/boolean operators are unaffected.

Manual Testing

Reproduced the bug and verified the fix end-to-end through the real govern() call path (not just the unit level): installed the published agent-governance-toolkit[full] package in a scratch venv, confirmed a startswith deny rule silently allowed a delete-* action, patched in this fix, reran, confirmed GovernanceDenied now fires. Also reran against an existing ==-based policy (matching what I ship in my own gateway) to confirm no regression.

Update: the monorepo's full pytest was initially blocked locally by a Rust toolchain mismatch (rustup's stable pinned at 1.85.0; agt-policies's native agent-control-specification extension needs rustc >=1.86-1.89 for several transitive crates). Fixed with rustup update stable (-> 1.98.1), after which pip install -e ".[dev]" on agent-governance-toolkit-core built cleanly and the full agent-mesh suite ran: 3572 passed, 147 skipped, 65 failed. All 65 failures are pre-existing and unrelated to this change — missing optional extras not pulled in by [dev] (prometheus_client, opentelemetry, websockets: test_prometheus_exporter.py, test_prometheus_metrics.py, test_otel_bootstrap.py, test_otel_observability.py, test_websocket_transport.py) plus one flaky Path.stat-monkeypatch test unrelated to condition evaluation (tests/engine_api/test_policy_registry.py::TestMalformedTolerance::test_file_with_unreadable_metadata_is_skipped). Nothing in test_policy.py, test_policy_composition.py, test_policy_schema.py, test_governance.py, test_policy_rate_limit.py, or test_conflict_resolution.py failed.

Checklist

I have linked a related issue above, or completed "Problem & Solution", "Impact on Your Work", and "Alternatives Considered"
My code follows the project style guidelines (ruff check) — not yet run locally, will verify before marking ready for review
I have added tests that prove my fix/feature works
All new and existing tests pass (pytest) — full agent-mesh suite now runs locally (Rust toolchain mismatch fixed, see Manual Testing); 3572 passed / 147 skipped / 65 pre-existing unrelated failures, 0 failures related to this change
I have updated documentation as needed — updated the docstring listing supported condition examples; no separate docs page found for this DSL
I have signed the Microsoft CLA

Attribution & Prior Art

This contribution does not contain code copied or derived from other projects without attribution
Any external projects that inspired this design are credited in code comments or documentation
If this PR implements functionality similar to an existing open-source project, I have listed it below

Prior art / related projects (if any):
None.

AI Assistance

I can explain every meaningful change in this PR: what it does, why, and what tradeoffs were considered
I have run tests and verification appropriate for this change
No part of this PR was autonomously submitted by an AI agent without my review
I have not used AI to generate review comments on others' PRs

If AI tools materially shaped this change, briefly note what was used:
Claude Code was used throughout — to investigate the DSL gap, implement the operators, write and run the tests, and reproduce/verify the fix end-to-end via govern() in a scratch venv. Leaving the boxes above unchecked until I've personally re-verified each attestation before marking this ready for review.

IP, Patents, and Licensing

This contribution does not implement patent-pending or patent-encumbered techniques
This contribution does not require an NDA or licensing agreement to understand or use
Any AI tools used have terms compatible with the MIT License

azure-pipelines · 2026-09-10T09:11:21Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

github-actions · 2026-09-10T09:12:14Z

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

azure-pipelines · 2026-09-10T15:03:03Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

MohammadHaroonAbuomar

DCO: none of the 6 non-merge commits (64b84e0, 05cc247, 1ad17b5, 0d16e19, 6e14dbb, 5dc975b) carries a Signed-off-by trailer; the fork-gated DCO check will fail once approved — please git rebase --signoff.

, microsoft#3916, microsoft#3924, microsoft#3853, advance watermarks

Carlos Hernandez (carloshvp)

Reviewed current head 45447a0, which already contains current main. The focused policy/federation matrix passes 134 tests and the prior empty-operand, type, depth, and DCO findings are addressed. Adversarial evaluation still found the three fail-closed/parser gaps noted inline. Separately, the final unsupported-syntax fallback remains action-insensitive: action.path containz '..' silently disables a deny rule, the same failure class that motivated this PR; please reject unsupported expressions at load or make that fallback action-aware. The two new test files also need the configured formatter applied.

Karim Mehalebi (karimad) · 2026-09-14T08:46:06Z

Reviewed current head 45447a0, which already contains current main. The focused policy/federation matrix passes 134 tests and the prior empty-operand, type, depth, and DCO findings are addressed. Adversarial evaluation still found the three fail-closed/parser gaps noted inline. Separately, the final unsupported-syntax fallback remains action-insensitive: action.path containz '..' silently disables a deny rule, the same failure class that motivated this PR; please reject unsupported expressions at load or make that fallback action-aware. The two new test files also need the configured formatter applied.

thanks for the review fixed all comment Carlos Hernandez (@carloshvp)

Karim Mehalebi (karimad) · 2026-09-14T08:46:51Z

DCO: none of the 6 non-merge commits (64b84e0, 05cc247, 1ad17b5, 0d16e19, 6e14dbb, 5dc975b) carries a Signed-off-by trailer; the fork-gated DCO check will fail once approved — please git rebase --signoff.

fixed the commit

Carlos Hernandez (carloshvp)

Re-reviewed exact head 281cf7d7, which already contains current main 1a896e70. The previous absent/null inequality, malformed numeric, trailing-token, unknown-operator, and test-formatting examples are addressed. The six focused policy/federation/governance suites pass all 158 tests, and the squashed commit has a DCO signoff.

Three remaining malformed-input cases are documented inline. Independent regression probes reproduce them on both rule evaluators, with each also producing allowed=True through PolicyEngine using an applicable agents=["*"] policy. The 21-case negative matrix fails all 21 expectations on this head. Baseline comparison matters: numeric and non-string inequality weaknesses already exist in PolicyRule on main; this PR exposes those operators in OrgPolicyRule and should not carry those weaknesses into the new support. Mismatched-quote string expressions are newly accepted in both evaluators.

Validation scope: focused tests and independent adversarial probes, not the full monorepo suite. Ruff reports the same 55 lint findings as main under the package configuration; both new test files are formatted. The two source files still need formatting, including added logging calls. Current reported GitHub checks pass, but that does not establish that fork-gated package workflows ran. Requesting changes for the three inline findings.

MohammadHaroonAbuomar

agent-governance-python/agent-mesh/src/agentmesh/governance/policy.py:161 and federation.py:100 pass self.condition to _eval_expression unstripped, and the new $ anchors (policy.py:250,267,284,300; federation.py:1105,1121,1137,1152) now send a condition with trailing whitespace to the unrecognized-syntax fallback. Probed: deny action.cost > 100 (trailing space) with cost=50 -> True on both engines (denies everything, WARNING) where main -> False; allow action.cost > 100\t with cost=150 -> False where main PolicyRule -> True; same for action.path contains 'x' . Fix: add expr = expr.strip() as the first line of PolicyRule._eval_expression (policy.py:181) and of federation _eval_expression (federation.py:1021), or use \s*$ in the anchored regexes; add a trailing-whitespace case to both test files.
agent-governance-python/agent-mesh/src/agentmesh/governance/policy.py:216,223,239-240 and federation.py:1067-1069,1076-1078,1094-1096 — the ==, != and in regexes are still unanchored, so trailing garbage after a valid prefix is accepted for these operators (the != and in branches are new to federation in this PR). Probed on both engines: allow action.path != 'x' JUNK on path 'safe/file' -> True; allow action.path in ['safe/file'] JUNK -> True; allow action.path == 'safe/file' JUNK -> True (PolicyEngine allowed=True). Fix: append $ to those three regexes in both evaluators (after the strip from the first ask) and add trailing-token negatives for them to both test files.

Karim Mehalebi (karimad) · 2026-09-15T08:29:13Z

agent-governance-python/agent-mesh/src/agentmesh/governance/policy.py:161 and federation.py:100 pass self.condition to _eval_expression unstripped, and the new $ anchors (policy.py:250,267,284,300; federation.py:1105,1121,1137,1152) now send a condition with trailing whitespace to the unrecognized-syntax fallback. Probed: deny action.cost > 100 (trailing space) with cost=50 -> True on both engines (denies everything, WARNING) where main -> False; allow action.cost > 100\t with cost=150 -> False where main PolicyRule -> True; same for action.path contains 'x' . Fix: add expr = expr.strip() as the first line of PolicyRule._eval_expression (policy.py:181) and of federation _eval_expression (federation.py:1021), or use \s*$ in the anchored regexes; add a trailing-whitespace case to both test files.

agent-governance-python/agent-mesh/src/agentmesh/governance/policy.py:216,223,239-240 and federation.py:1067-1069,1076-1078,1094-1096 — the ==, != and in regexes are still unanchored, so trailing garbage after a valid prefix is accepted for these operators (the != and in branches are new to federation in this PR). Probed on both engines: allow action.path != 'x' JUNK on path 'safe/file' -> True; allow action.path in ['safe/file'] JUNK -> True; allow action.path == 'safe/file' JUNK -> True (PolicyEngine allowed=True). Fix: append $ to those three regexes in both evaluators (after the strip from the first ask) and add trailing-token negatives for them to both test files.

added a $ to all regex
trimmed the str before evaluation

…pty audience, docs None-deref, group-path limit Four more real gaps from MohammadHaroonAbuomar's follow-up review: key_ops gate ran before the try/except and used a bare "verify" not in key_ops check: a non-list value (123, true) raised TypeError instead of failing closed, and a malformed string like "noverify" passed the check via substring match ("verify" IS a substring of "noverify"), letting an encryption-only key verify a signature anyway. Now requires isinstance(key_ops, list) before the membership check, so any non-list shape fails closed the same way the rest of _verify_signature already does for a malformed JWKS entry. Parametrized test covers all three shapes (int, bool, malformed string). nbf was checked asymmetrically with exp: a non-numeric nbf ("9999999999") was silently treated as absent instead of failing closed like a non-numeric exp already does. Made the two checks match. TrustedEndpoint(audience="") would match a token whose own aud is also "" - silently trusting a token that asserts no audience at all - and audience=[] can never intersect anything, rejecting every token with no signal the field is misconfigured rather than intentionally locking down. Added a field_validator rejecting both at construction. docs/identity.md's worked example said "None if verification fails" but then dereferenced identity unconditionally two lines later - AttributeError on any rejected token, not the PermissionError a reader would expect. Added the missing check and a test that runs the doc's example through both branches (success and rejection). The as_policy_kwargs() dict-shape fix from the previous round doesn't fully close the gap it was meant to: PolicyRule's bare-attribute matcher still can't address a Keycloak-shaped "/engineering" group path or a hyphenated role like "default-roles-company" (it splits on "." and requires \w+ segments), so those still never match - silently, not with an error. Rather than adding a list-membership operator to the shared policy DSL (a larger, cross-cutting change tracked separately in microsoft#3924), documented the exact name grammar on as_policy_kwargs() and in docs/identity.md, and added a test pinning both failure directions (an allow rule against a group path silently denies; a deny rule against a hyphenated role silently lets the call through) so the limit stays visible instead of reappearing as a surprise. Also added OAEP/footgun/noverify to .cspell-repo-terms.txt for words introduced by this round's own comments and tests - verified against the actual CI gate (scripts/ci/changed_lines.py --base origin/main --mode added-lines piped into cspell@8.17.3 --config .cspell.json), exits 0. 87/87 tests pass (test_external_jwks.py + test_govern.py). Signed-off-by: Fernando Marino <fernando.marino85@gmail.com>

…l-closed semantics PolicyRule._eval_expression only recognized ==, !=, in [...], numeric comparisons, and bare boolean-attribute conditions. A condition written with contains/startswith/endswith fell through every branch to a silent `return False` -- a no-match, not an evaluation error -- so a deny rule using one of these operators would quietly never fire. Adds contains/startswith/endswith to PolicyRule (policy.py) and brings federation.py's OrgPolicyRule._eval_expression to genuine parity with it (previously only == and bare-boolean), plus !=, in, and numeric comparisons. Every guard/edge-case in the evaluator is now action-aware instead of unconditionally fail-open or fail-closed: a non-allow rule (deny/warn/ require_approval) treats the trip as a MATCH so its action still takes effect, while an allow rule treats it as NO-MATCH so it can't grant access on bad input. This applies to: - A non-string value on contains/startswith/endswith. - A missing (None) field on != and on numeric comparisons -- absence is no longer treated as evidence of inequality, and no longer silently coerced to 0 for numeric comparisons. - A malformed/non-numeric value on numeric comparisons. - An oversized expression (>2000 chars) or excessive nesting depth (>20), guarding against pathological/adversarial input -- federation conditions can originate from a partner org's trust agreement, a higher-privilege attack surface than a same-org policy. - Unrecognized condition syntax (typo'd operator, malformed expression), which previously fell through to an unconditional `return False`. - An exception raised mid-evaluation (the existing V27 fail-closed pattern), now also fixed for allow rules, which previously matched on error and incorrectly granted access. Each of these now logs a WARNING instead of staying silent or logging at debug level, so a rule that stops working is visible in logs rather than silently disabled. Also closes an empty-operand regex bug: `action.path contains ''` (or startswith/endswith '') previously matched every string under default-deny since the operand pattern allowed a zero-length literal. The three new operators' regexes now require a non-empty operand and a full match to end-of-expression, so trailing garbage after a valid prefix (e.g. `contains 'x' JUNK`) is no longer silently accepted. Tests cover both engines' new operators, all of the above action-aware edge cases for both deny and allow rules, and the exception/depth/ length fail paths. The two new test files are ruff-formatted. Signed-off-by: karimad <kmehaleb@gmail.com>

… and quote-mismatch bypasses Second round of review findings on the condition-DSL operators: - Trailing whitespace (e.g. a trailing space/tab from YAML authoring) was pushed into the unrecognized-syntax fallback by the new end anchors, silently flipping the result relative to main for both deny and allow rules. Both evaluators now strip() the expression before matching. - The ==, !=, and in [...] regexes were still unanchored, so trailing garbage after a valid prefix (e.g. `!= 'x' JUNK`) was accepted. Anchored with $ to match contains/startswith/endswith/numeric. - Numeric comparisons accepted non-finite values: float("NaN") and float("inf") parse successfully, but every ordered comparison against them is False, which silently failed open a deny rule fed non-finite evidence. Both evaluators now reject non-finite actual/ target values with the existing action-aware fallback. - != treated any non-string value (list, dict, bool, number) as evidence of inequality against a string literal, since non-string values always compare unequal to a string. Now requires the actual value to be a string, same as contains/startswith/endswith. - contains/startswith/endswith accepted mismatched quote delimiters (e.g. `contains 'safe"`) because the opening and closing quote characters were matched independently. Now captured and matched via backreference so the closing quote must equal the opening one. - Removed unreachable dead code (`return False` after an unconditional `return match`) and corrected a stale docstring claiming evaluation errors unconditionally return False. Both source files are now ruff-formatted in the regions this PR touches (the _eval_expression functions), and the two test files remain fully ruff-formatted. Regression tests added for each case above, action-aware for both deny and allow rules. Signed-off-by: karimad <kmehaleb@gmail.com>

The == and != regexes still used independent quote character classes (['"]([^'"]+)['"]$) after the contains/startswith/endswith operators were already fixed to require matching delimiters. A mismatched literal like action.path == 'safe/file" parsed as a valid operand, letting an allow rule fire on malformed policy text. Fixed by capturing the opening quote and requiring the same character to close, matching the pattern already used for contains/startswith/ endswith, in both PolicyRule (policy.py) and OrgPolicyRule (federation.py). Extended test_string_operators_require_matching_quote_delimiters in both test files to cover startswith/endswith/==/!= mismatched-quote negatives, not just contains. Signed-off-by: karimad <kmehaleb@gmail.com>

MohammadHaroonAbuomar

Verified at 21fabd8: == and != now require matching quote delimiters via a backreference on both PolicyRule and OrgPolicyRule, with the mixed-delimiter negatives covered in both test files. The ask's two probes plus 31 more shapes fail closed; earlier fixes (trailing whitespace, anchored operators, NaN and infinity handling, Carlos's five items) all hold.

…pty audience, docs None-deref, group-path limit Four more real gaps from MohammadHaroonAbuomar's follow-up review: key_ops gate ran before the try/except and used a bare "verify" not in key_ops check: a non-list value (123, true) raised TypeError instead of failing closed, and a malformed string like "noverify" passed the check via substring match ("verify" IS a substring of "noverify"), letting an encryption-only key verify a signature anyway. Now requires isinstance(key_ops, list) before the membership check, so any non-list shape fails closed the same way the rest of _verify_signature already does for a malformed JWKS entry. Parametrized test covers all three shapes (int, bool, malformed string). nbf was checked asymmetrically with exp: a non-numeric nbf ("9999999999") was silently treated as absent instead of failing closed like a non-numeric exp already does. Made the two checks match. TrustedEndpoint(audience="") would match a token whose own aud is also "" - silently trusting a token that asserts no audience at all - and audience=[] can never intersect anything, rejecting every token with no signal the field is misconfigured rather than intentionally locking down. Added a field_validator rejecting both at construction. docs/identity.md's worked example said "None if verification fails" but then dereferenced identity unconditionally two lines later - AttributeError on any rejected token, not the PermissionError a reader would expect. Added the missing check and a test that runs the doc's example through both branches (success and rejection). The as_policy_kwargs() dict-shape fix from the previous round doesn't fully close the gap it was meant to: PolicyRule's bare-attribute matcher still can't address a Keycloak-shaped "/engineering" group path or a hyphenated role like "default-roles-company" (it splits on "." and requires \w+ segments), so those still never match - silently, not with an error. Rather than adding a list-membership operator to the shared policy DSL (a larger, cross-cutting change tracked separately in microsoft#3924), documented the exact name grammar on as_policy_kwargs() and in docs/identity.md, and added a test pinning both failure directions (an allow rule against a group path silently denies; a deny rule against a hyphenated role silently lets the call through) so the limit stays visible instead of reappearing as a surprise. Also added OAEP/footgun/noverify to .cspell-repo-terms.txt for words introduced by this round's own comments and tests - verified against the actual CI gate (scripts/ci/changed_lines.py --base origin/main --mode added-lines piped into cspell@8.17.3 --config .cspell.json), exits 0. 87/87 tests pass (test_external_jwks.py + test_govern.py). Signed-off-by: Fernando Marino <fernando.marino85@gmail.com>

…t, security audit doc Rebased onto main (post-microsoft#3924) surfaced a stale assumption in test_as_policy_kwargs_group_paths_and_hyphenated_roles_never_match: policy.py's unrecognized-condition fallback now fails closed for any non-allow rule, so the deny-rule half of the pinned scenario denies instead of letting the call through. Updated the test and the as_policy_kwargs()/docs/identity.md documentation to describe the current (still-limited, but no-longer-fail-open) behavior accurately. Also adds the missing repo-standard license header to external_jwks.py and its test file, and a security-audit doc under docs/security/audits/ covering the identity-surface changes across all three review rounds, per the CI gate and MohammadHaroonAbuomar's review. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com>

Review caught that agent_os (force-included into this same wheel) imports agent_control_specification directly in 4 places: providers.py, cmd_validate.py, _native_adapter_runtime.py, and openai_agents_sdk.py. That package previously arrived only transitively via agt-policies's own pin, which the prior commit removed from the base dependency set - trading the original unresolvable-install failure for a quieter ImportError at runtime the moment agent_os actually exercised one of those paths. Fix: declare agent-control-specification>=0.3.1b0,<0.4.0 directly in agent-governance-toolkit-core's own dependencies, matching the range agt-policies 5.0.0 already resolved to before microsoft#3939. Verified in a clean venv: all four previously-broken agent_os modules import cleanly, and cmd_validate.py's _validate_manifest() actually runs end-to-end (not just import-checked). govern()'s contains/startswith/endswith path (microsoft#3924) still passes both the deny and allow cases. One related, pre-existing gap surfaced during this verification and is NOT fixed here (out of scope for a packaging PR): _native_adapter_runtime .py's _session_for() imports HostSession from agent_control_specification, which genuinely does not exist in any published release yet (checked 0.3.1b1's exports directly). That call path was already broken before this PR - it depends on ACS 0.4.0b0's API regardless of how the agt-policies/agent-control-specification pins are arranged - so it's unaffected by this change either way. Also: moved the new 'migrate' extra out from under the '--- Bundles ---' header (it's a single-package extra like redis/django, not a bundle), and added a CHANGELOG.md [Unreleased]/Fixed entry. Signed-off-by: karimad <kmehaleb@gmail.com>

agent-governance-toolkit-core's dependencies pin agt-policies>=5.1.0,<6.0, which in turn pins agent-control-specification>=0.4.0b0,<0.5.0. Neither version is published to PyPI (PyPI tops out at agt-policies 5.0.0 and agent-control-specification 0.3.1b1), so a plain 'pip install agent-governance-toolkit-core' or '[full]' cannot resolve. agt-policies backs only the v4-to-ACS manifest migration CLI ('agt migrate'). Nothing in agentmesh.governance (govern(), GovernanceDenied, policy.py/PolicyEngine) imports agt or agent_control_specification - confirmed by grepping agent-mesh/src/agentmesh/ for both. The base governance runtime does not need this dependency at all. Moves it to a new 'migrate' extra instead, matching the existing optional-dependencies pattern for other CLI/framework-specific pieces (mcp, redis, django, etc.). Verified: a clean venv can now 'pip install agent-governance-toolkit-core[full]' and exercise govern()'s contains/startswith/endswith operators (microsoft#3924) end-to-end without agt-policies or agent-control-specification installed at all. Signed-off-by: karimad <kmehaleb@gmail.com>

Review caught that agent_os (force-included into this same wheel) imports agent_control_specification directly in 4 places: providers.py, cmd_validate.py, _native_adapter_runtime.py, and openai_agents_sdk.py. That package previously arrived only transitively via agt-policies's own pin, which the prior commit removed from the base dependency set - trading the original unresolvable-install failure for a quieter ImportError at runtime the moment agent_os actually exercised one of those paths. Fix: declare agent-control-specification>=0.3.1b0,<0.4.0 directly in agent-governance-toolkit-core's own dependencies, matching the range agt-policies 5.0.0 already resolved to before microsoft#3939. Verified in a clean venv: all four previously-broken agent_os modules import cleanly, and cmd_validate.py's _validate_manifest() actually runs end-to-end (not just import-checked). govern()'s contains/startswith/endswith path (microsoft#3924) still passes both the deny and allow cases. One related, pre-existing gap surfaced during this verification and is NOT fixed here (out of scope for a packaging PR): _native_adapter_runtime .py's _session_for() imports HostSession from agent_control_specification, which genuinely does not exist in any published release yet (checked 0.3.1b1's exports directly). That call path was already broken before this PR - it depends on ACS 0.4.0b0's API regardless of how the agt-policies/agent-control-specification pins are arranged - so it's unaffected by this change either way. Also: moved the new 'migrate' extra out from under the '--- Bundles ---' header (it's a single-package extra like redis/django, not a bundle), and added a CHANGELOG.md [Unreleased]/Fixed entry. Signed-off-by: karimad <kmehaleb@gmail.com>

…l loudly Addresses review from MohammadHaroonAbuomar on microsoft#4020: - string_operators.py was missing its MIT license header (scripts/check_license_headers.py --fix). - README's pip-install path doesn't work today: contains/startswith/endswith landed in microsoft#3924 (merged 2026-09-15) but no agent-governance-toolkit-core release includes it yet (latest on PyPI is 5.0.0, 2026-08-03) — following the old instructions silently reproduces the exact bug this example demonstrates. Documented that gap and gave a working PYTHONPATH-based run path from a checkout. - string_operators.py only printed results for the reader to eyeball; on a stale install every check prints False with nothing to signal the mismatch. Replaced with a check() helper that asserts each expected value, so a stale/pre-fix install now fails loudly with an AssertionError instead of silently demonstrating the bug it's supposed to show was fixed. Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com>

…te role/group claims (#3956) * feat: identity - support RS256/ES256 in ExternalJWKSProvider, propagate role/group claims Fixes #3954. ExternalJWKSProvider could only verify Ed25519-signed tokens (ADR-0007's original agent-to-agent federation scheme), so it never worked against a standard OIDC provider (Keycloak, Okta, etc.) whose default signing key is RS256, not Ed25519 - confirmed against a real Keycloak realm's actual production JWKS. _verify_signature now dispatches on the JWK's own kty/crv (never the JWT header's unverified alg, to avoid algorithm-confusion) to add RS256 and ES256 alongside the existing Ed25519 path. Separately, ExternalIdentity carried no role/group information at all, so even a successfully verified external token had no path into govern()'s policy context - FederationPolicy/TrustedEndpoint gain configurable (Keycloak-defaulted) dotted-path claim extraction, and ExternalIdentity.as_policy_kwargs() bridges the result into a governed call (safe(**identity.as_policy_kwargs(), ...)), matching this codebase's kwargs-only, no-hidden-magic design. Also replaces docs/identity.md's dead OIDC/SAML section (referencing a non-existent agentmesh.enterprise.OIDCProvider) with a real, runnable example using this provider. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address review — audience/nbf checks, role-order bug, JWK use, and four other gaps MohammadHaroonAbuomar's review found several real gaps, each fixed here: Audience (aud) and not-before (nbf) - the critical one. A verified RS256 signature only proves the issuer minted the token, not that it was minted for this verifier: without an audience check, a token issued for any other client the issuer trusts (e.g. one lifted from a browser SPA) verified identically to one this integration actually requested. Added TrustedEndpoint.audience (str or list[str]); when set, the token's own aud (also str-or-list per RFC 7519 §4.1.3) must intersect it, checked via the new _audience_satisfied helper. Also added the missing nbf check next to the existing exp check. Both are documented on TrustedEndpoint and in docs/identity.md, including that leaving audience unset accepts a token minted for any client. caller_role order-dependence. as_policy_kwargs() picked roles[0] as a single "primary" role, but role order in a verified token is whatever the issuer happened to serialize - a YAML rule keyed on caller_role gave different decisions for the same role set depending on iteration order, so a deny-by-role rule was bypassable just by how roles sorted that request. Removed caller_role. caller_roles/caller_groups are now dicts ({"admin": True, ...}) instead of lists: GovernedCallable passes a dict kwarg through to the policy context as-is, and PolicyRule._eval_expression's bare-boolean-attribute branch evaluates caller_roles.<role> as a plain, order-independent truthiness check - the YAML DSL has no list-membership operator, so a list value gave policies no usable signal at all. New tests cover order-independence directly and end to end through govern(), per the linked issue #3954. JWK use/key_ops ignored. A realm's JWKS can publish encryption keys (use="enc", e.g. RSA-OAEP) alongside signing keys; _verify_signature now rejects use not in (None, "sig") and key_ops missing "verify" before dispatching on kty, per RFC 7517 §4.2/4.3. except (..., ValueError) missed TypeError - a JWKS entry with a non-string numeric member (n: 12345, e: null) raised out of rsa.RSAPublicNumbers/verify() instead of failing closed like every other malformed-key case. Added TypeError to the tuple. role_claim_path="" was being treated as unset (`or` folds "" and None together) rather than "disable extraction for this endpoint" - switched to an explicit `is not None` check. resource_access.<client_id>.roles can't be expressed as a dotted string when the client id itself contains a dot - splitting on "." can't tell that dot from the path separator. role_claim_path/ group_claim_path now also accept a pre-split list[str] of literal segments. Docstring/docs: the class docstring named only Ed25519; updated to name all three supported algorithms and that everything else (PS256, RS512, ES384, ...) is rejected, not silently skipped. docs/identity.md's worked example didn't import govern, and read_doc didn't accept the kwargs as_policy_kwargs() spreads into it (TypeError on the very call the doc shows) - fixed both and pinned the corrected example with a test that runs it verbatim. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address second review — key_ops fail-open, nbf/exp asymmetry, empty audience, docs None-deref, group-path limit Four more real gaps from MohammadHaroonAbuomar's follow-up review: key_ops gate ran before the try/except and used a bare "verify" not in key_ops check: a non-list value (123, true) raised TypeError instead of failing closed, and a malformed string like "noverify" passed the check via substring match ("verify" IS a substring of "noverify"), letting an encryption-only key verify a signature anyway. Now requires isinstance(key_ops, list) before the membership check, so any non-list shape fails closed the same way the rest of _verify_signature already does for a malformed JWKS entry. Parametrized test covers all three shapes (int, bool, malformed string). nbf was checked asymmetrically with exp: a non-numeric nbf ("9999999999") was silently treated as absent instead of failing closed like a non-numeric exp already does. Made the two checks match. TrustedEndpoint(audience="") would match a token whose own aud is also "" - silently trusting a token that asserts no audience at all - and audience=[] can never intersect anything, rejecting every token with no signal the field is misconfigured rather than intentionally locking down. Added a field_validator rejecting both at construction. docs/identity.md's worked example said "None if verification fails" but then dereferenced identity unconditionally two lines later - AttributeError on any rejected token, not the PermissionError a reader would expect. Added the missing check and a test that runs the doc's example through both branches (success and rejection). The as_policy_kwargs() dict-shape fix from the previous round doesn't fully close the gap it was meant to: PolicyRule's bare-attribute matcher still can't address a Keycloak-shaped "/engineering" group path or a hyphenated role like "default-roles-company" (it splits on "." and requires \w+ segments), so those still never match - silently, not with an error. Rather than adding a list-membership operator to the shared policy DSL (a larger, cross-cutting change tracked separately in #3924), documented the exact name grammar on as_policy_kwargs() and in docs/identity.md, and added a test pinning both failure directions (an allow rule against a group path silently denies; a deny rule against a hyphenated role silently lets the call through) so the limit stays visible instead of reappearing as a surprise. Also added OAEP/footgun/noverify to .cspell-repo-terms.txt for words introduced by this round's own comments and tests - verified against the actual CI gate (scripts/ci/changed_lines.py --base origin/main --mode added-lines piped into cspell@8.17.3 --config .cspell.json), exits 0. 87/87 tests pass (test_external_jwks.py + test_govern.py). Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: reject audience lists containing an empty-string member MohammadHaroonAbuomar's third review round on the empty-audience guard: len(v) == 0 only catches "" and [], not a list that contains an empty-string member ([""], ["", "x"]) - that slips through and still matches a token whose own aud claim is "". Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address third review — license headers, stale DSL-limitation test, security audit doc Rebased onto main (post-#3924) surfaced a stale assumption in test_as_policy_kwargs_group_paths_and_hyphenated_roles_never_match: policy.py's unrecognized-condition fallback now fails closed for any non-allow rule, so the deny-rule half of the pinned scenario denies instead of letting the call through. Updated the test and the as_policy_kwargs()/docs/identity.md documentation to describe the current (still-limited, but no-longer-fail-open) behavior accurately. Also adds the missing repo-standard license header to external_jwks.py and its test file, and a security-audit doc under docs/security/audits/ covering the identity-surface changes across all three review rounds, per the CI gate and MohammadHaroonAbuomar's review. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address fourth review — cspell misses and stale commit ref in security audit doc The audit doc itself failed the Spell Check job: the reviewer's name written out ("MohammadHaroonAbuomar"), and "footguns"/"unaddressable" weren't in the wordlist. Reworded rather than adding to the wordlist, and fixed a stale pre-rebase commit hash (d48413a -> db2172c) plus the round-3 fix-commit reference and the cspell-status claim, both left inaccurate by the rebase. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> --------- Signed-off-by: Fernando Marino <fernando.marino85@gmail.com>

PolicyRule's condition DSL gained contains/startswith/endswith operators in microsoft#3924, fixing a bug where those operators silently fell through to False (a deny rule using them never fired, with no warning). Adds examples/policy-condition-operators/ demonstrating the three operators and their fail-closed/fail-open semantics, since no existing example covered them. string_operators.py asserts each expected value rather than just printing it, so a pre-microsoft#3924 install fails loudly with an AssertionError instead of silently reproducing the bug it's meant to demonstrate. The operators aren't in any released agent-governance-toolkit-core yet (latest on PyPI is 5.0.0, predating microsoft#3924's 2026-09-15 merge), so the README documents that gap and gives a working checkout-based run path (PYTHONPATH into agent-mesh/agent-hypervisor/agt-policies, plus pydantic[email]/pyyaml/cryptography/httpx/python-dateutil - verified in a genuinely clean venv) rather than a pip-install path that would silently reproduce the fixed bug. Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com>

… installs (#4017) * fix(core): move agt-policies to an opt-in extra, not a base dependency agent-governance-toolkit-core's dependencies pin agt-policies>=5.1.0,<6.0, which in turn pins agent-control-specification>=0.4.0b0,<0.5.0. Neither version is published to PyPI (PyPI tops out at agt-policies 5.0.0 and agent-control-specification 0.3.1b1), so a plain 'pip install agent-governance-toolkit-core' or '[full]' cannot resolve. agt-policies backs only the v4-to-ACS manifest migration CLI ('agt migrate'). Nothing in agentmesh.governance (govern(), GovernanceDenied, policy.py/PolicyEngine) imports agt or agent_control_specification - confirmed by grepping agent-mesh/src/agentmesh/ for both. The base governance runtime does not need this dependency at all. Moves it to a new 'migrate' extra instead, matching the existing optional-dependencies pattern for other CLI/framework-specific pieces (mcp, redis, django, etc.). Verified: a clean venv can now 'pip install agent-governance-toolkit-core[full]' and exercise govern()'s contains/startswith/endswith operators (#3924) end-to-end without agt-policies or agent-control-specification installed at all. Signed-off-by: karimad <kmehaleb@gmail.com> * trim comment on the migrate extra Signed-off-by: karimad <kmehaleb@gmail.com> * fix(core): give agent-control-specification its own base dependency Review caught that agent_os (force-included into this same wheel) imports agent_control_specification directly in 4 places: providers.py, cmd_validate.py, _native_adapter_runtime.py, and openai_agents_sdk.py. That package previously arrived only transitively via agt-policies's own pin, which the prior commit removed from the base dependency set - trading the original unresolvable-install failure for a quieter ImportError at runtime the moment agent_os actually exercised one of those paths. Fix: declare agent-control-specification>=0.3.1b0,<0.4.0 directly in agent-governance-toolkit-core's own dependencies, matching the range agt-policies 5.0.0 already resolved to before #3939. Verified in a clean venv: all four previously-broken agent_os modules import cleanly, and cmd_validate.py's _validate_manifest() actually runs end-to-end (not just import-checked). govern()'s contains/startswith/endswith path (#3924) still passes both the deny and allow cases. One related, pre-existing gap surfaced during this verification and is NOT fixed here (out of scope for a packaging PR): _native_adapter_runtime .py's _session_for() imports HostSession from agent_control_specification, which genuinely does not exist in any published release yet (checked 0.3.1b1's exports directly). That call path was already broken before this PR - it depends on ACS 0.4.0b0's API regardless of how the agt-policies/agent-control-specification pins are arranged - so it's unaffected by this change either way. Also: moved the new 'migrate' extra out from under the '--- Bundles ---' header (it's a single-package extra like redis/django, not a bundle), and added a CHANGELOG.md [Unreleased]/Fixed entry. Signed-off-by: karimad <kmehaleb@gmail.com> * shorten comments Signed-off-by: karimad <kmehaleb@gmail.com> * docs: disclose still-unresolvable migrate-extra pin, file tracking issue The migrate extra's agt-policies>=5.1.0,<6.0 pin predates this PR and is unchanged by it. It's still unresolvable on its own today (agt-policies 5.1.0 isn't published), and will conflict with this PR's new base ACS pin once it is (agt-policies would then require ACS>=0.4.0b0, base requires <0.4.0). Documented in the CHANGELOG and the extra itself, tracked in #4019 so it isn't rediscovered fresh. Signed-off-by: Karim Mehalebi <kmehalebi@egencia.com> * docs: tell agt migrate users they now need the migrate extra Signed-off-by: Karim Mehalebi <kmehalebi@egencia.com> * fix(core): pin agent-control-specification to 0.4.0b0 range agent_os requires Reviewer found 0.3.1b1 lacks HostSession and rejects every in-repo manifest, breaking the adapter runtime and agent-os validate despite the install itself resolving. Update the base pin and changelog entry to match. Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com> --------- Signed-off-by: karimad <kmehaleb@gmail.com> Signed-off-by: Karim Mehalebi <kmehalebi@egencia.com> Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com> Co-authored-by: Karim Mehalebi <kmehalebi@egencia.com>

PolicyRule's condition DSL gained contains/startswith/endswith operators in #3924, fixing a bug where those operators silently fell through to False (a deny rule using them never fired, with no warning). Adds examples/policy-condition-operators/ demonstrating the three operators and their fail-closed/fail-open semantics, since no existing example covered them. string_operators.py asserts each expected value rather than just printing it, so a pre-#3924 install fails loudly with an AssertionError instead of silently reproducing the bug it's meant to demonstrate. The operators aren't in any released agent-governance-toolkit-core yet (latest on PyPI is 5.0.0, predating #3924's 2026-09-15 merge), so the README documents that gap and gives a working checkout-based run path (PYTHONPATH into agent-mesh/agent-hypervisor/agt-policies, plus pydantic[email]/pyyaml/cryptography/httpx/python-dateutil - verified in a genuinely clean venv) rather than a pip-install path that would silently reproduce the fixed bug. Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com>

…ondition DSL (microsoft#3924) * fix(agent-mesh): condition-DSL string operators with action-aware fail-closed semantics PolicyRule._eval_expression only recognized ==, !=, in [...], numeric comparisons, and bare boolean-attribute conditions. A condition written with contains/startswith/endswith fell through every branch to a silent `return False` -- a no-match, not an evaluation error -- so a deny rule using one of these operators would quietly never fire. Adds contains/startswith/endswith to PolicyRule (policy.py) and brings federation.py's OrgPolicyRule._eval_expression to genuine parity with it (previously only == and bare-boolean), plus !=, in, and numeric comparisons. Every guard/edge-case in the evaluator is now action-aware instead of unconditionally fail-open or fail-closed: a non-allow rule (deny/warn/ require_approval) treats the trip as a MATCH so its action still takes effect, while an allow rule treats it as NO-MATCH so it can't grant access on bad input. This applies to: - A non-string value on contains/startswith/endswith. - A missing (None) field on != and on numeric comparisons -- absence is no longer treated as evidence of inequality, and no longer silently coerced to 0 for numeric comparisons. - A malformed/non-numeric value on numeric comparisons. - An oversized expression (>2000 chars) or excessive nesting depth (>20), guarding against pathological/adversarial input -- federation conditions can originate from a partner org's trust agreement, a higher-privilege attack surface than a same-org policy. - Unrecognized condition syntax (typo'd operator, malformed expression), which previously fell through to an unconditional `return False`. - An exception raised mid-evaluation (the existing V27 fail-closed pattern), now also fixed for allow rules, which previously matched on error and incorrectly granted access. Each of these now logs a WARNING instead of staying silent or logging at debug level, so a rule that stops working is visible in logs rather than silently disabled. Also closes an empty-operand regex bug: `action.path contains ''` (or startswith/endswith '') previously matched every string under default-deny since the operand pattern allowed a zero-length literal. The three new operators' regexes now require a non-empty operand and a full match to end-of-expression, so trailing garbage after a valid prefix (e.g. `contains 'x' JUNK`) is no longer silently accepted. Tests cover both engines' new operators, all of the above action-aware edge cases for both deny and allow rules, and the exception/depth/ length fail paths. The two new test files are ruff-formatted. Signed-off-by: karimad <kmehaleb@gmail.com> * fix(agent-mesh): harden condition-DSL against whitespace, non-finite, and quote-mismatch bypasses Second round of review findings on the condition-DSL operators: - Trailing whitespace (e.g. a trailing space/tab from YAML authoring) was pushed into the unrecognized-syntax fallback by the new end anchors, silently flipping the result relative to main for both deny and allow rules. Both evaluators now strip() the expression before matching. - The ==, !=, and in [...] regexes were still unanchored, so trailing garbage after a valid prefix (e.g. `!= 'x' JUNK`) was accepted. Anchored with $ to match contains/startswith/endswith/numeric. - Numeric comparisons accepted non-finite values: float("NaN") and float("inf") parse successfully, but every ordered comparison against them is False, which silently failed open a deny rule fed non-finite evidence. Both evaluators now reject non-finite actual/ target values with the existing action-aware fallback. - != treated any non-string value (list, dict, bool, number) as evidence of inequality against a string literal, since non-string values always compare unequal to a string. Now requires the actual value to be a string, same as contains/startswith/endswith. - contains/startswith/endswith accepted mismatched quote delimiters (e.g. `contains 'safe"`) because the opening and closing quote characters were matched independently. Now captured and matched via backreference so the closing quote must equal the opening one. - Removed unreachable dead code (`return False` after an unconditional `return match`) and corrected a stale docstring claiming evaluation errors unconditionally return False. Both source files are now ruff-formatted in the regions this PR touches (the _eval_expression functions), and the two test files remain fully ruff-formatted. Regression tests added for each case above, action-aware for both deny and allow rules. Signed-off-by: karimad <kmehaleb@gmail.com> * fix(agent-mesh): anchor quote backreference on == and != operators The == and != regexes still used independent quote character classes (['"]([^'"]+)['"]$) after the contains/startswith/endswith operators were already fixed to require matching delimiters. A mismatched literal like action.path == 'safe/file" parsed as a valid operand, letting an allow rule fire on malformed policy text. Fixed by capturing the opening quote and requiring the same character to close, matching the pattern already used for contains/startswith/ endswith, in both PolicyRule (policy.py) and OrgPolicyRule (federation.py). Extended test_string_operators_require_matching_quote_delimiters in both test files to cover startswith/endswith/==/!= mismatched-quote negatives, not just contains. Signed-off-by: karimad <kmehaleb@gmail.com> --------- Signed-off-by: karimad <kmehaleb@gmail.com> Co-authored-by: MohammadHaroonAbuomar <40180927+MohammadHaroonAbuomar@users.noreply.github.com> Signed-off-by: yuvrajsingh2428 <offcyuvi2428@gmail.com>

…te role/group claims (microsoft#3956) * feat: identity - support RS256/ES256 in ExternalJWKSProvider, propagate role/group claims Fixes microsoft#3954. ExternalJWKSProvider could only verify Ed25519-signed tokens (ADR-0007's original agent-to-agent federation scheme), so it never worked against a standard OIDC provider (Keycloak, Okta, etc.) whose default signing key is RS256, not Ed25519 - confirmed against a real Keycloak realm's actual production JWKS. _verify_signature now dispatches on the JWK's own kty/crv (never the JWT header's unverified alg, to avoid algorithm-confusion) to add RS256 and ES256 alongside the existing Ed25519 path. Separately, ExternalIdentity carried no role/group information at all, so even a successfully verified external token had no path into govern()'s policy context - FederationPolicy/TrustedEndpoint gain configurable (Keycloak-defaulted) dotted-path claim extraction, and ExternalIdentity.as_policy_kwargs() bridges the result into a governed call (safe(**identity.as_policy_kwargs(), ...)), matching this codebase's kwargs-only, no-hidden-magic design. Also replaces docs/identity.md's dead OIDC/SAML section (referencing a non-existent agentmesh.enterprise.OIDCProvider) with a real, runnable example using this provider. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address review — audience/nbf checks, role-order bug, JWK use, and four other gaps MohammadHaroonAbuomar's review found several real gaps, each fixed here: Audience (aud) and not-before (nbf) - the critical one. A verified RS256 signature only proves the issuer minted the token, not that it was minted for this verifier: without an audience check, a token issued for any other client the issuer trusts (e.g. one lifted from a browser SPA) verified identically to one this integration actually requested. Added TrustedEndpoint.audience (str or list[str]); when set, the token's own aud (also str-or-list per RFC 7519 §4.1.3) must intersect it, checked via the new _audience_satisfied helper. Also added the missing nbf check next to the existing exp check. Both are documented on TrustedEndpoint and in docs/identity.md, including that leaving audience unset accepts a token minted for any client. caller_role order-dependence. as_policy_kwargs() picked roles[0] as a single "primary" role, but role order in a verified token is whatever the issuer happened to serialize - a YAML rule keyed on caller_role gave different decisions for the same role set depending on iteration order, so a deny-by-role rule was bypassable just by how roles sorted that request. Removed caller_role. caller_roles/caller_groups are now dicts ({"admin": True, ...}) instead of lists: GovernedCallable passes a dict kwarg through to the policy context as-is, and PolicyRule._eval_expression's bare-boolean-attribute branch evaluates caller_roles.<role> as a plain, order-independent truthiness check - the YAML DSL has no list-membership operator, so a list value gave policies no usable signal at all. New tests cover order-independence directly and end to end through govern(), per the linked issue microsoft#3954. JWK use/key_ops ignored. A realm's JWKS can publish encryption keys (use="enc", e.g. RSA-OAEP) alongside signing keys; _verify_signature now rejects use not in (None, "sig") and key_ops missing "verify" before dispatching on kty, per RFC 7517 §4.2/4.3. except (..., ValueError) missed TypeError - a JWKS entry with a non-string numeric member (n: 12345, e: null) raised out of rsa.RSAPublicNumbers/verify() instead of failing closed like every other malformed-key case. Added TypeError to the tuple. role_claim_path="" was being treated as unset (`or` folds "" and None together) rather than "disable extraction for this endpoint" - switched to an explicit `is not None` check. resource_access.<client_id>.roles can't be expressed as a dotted string when the client id itself contains a dot - splitting on "." can't tell that dot from the path separator. role_claim_path/ group_claim_path now also accept a pre-split list[str] of literal segments. Docstring/docs: the class docstring named only Ed25519; updated to name all three supported algorithms and that everything else (PS256, RS512, ES384, ...) is rejected, not silently skipped. docs/identity.md's worked example didn't import govern, and read_doc didn't accept the kwargs as_policy_kwargs() spreads into it (TypeError on the very call the doc shows) - fixed both and pinned the corrected example with a test that runs it verbatim. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address second review — key_ops fail-open, nbf/exp asymmetry, empty audience, docs None-deref, group-path limit Four more real gaps from MohammadHaroonAbuomar's follow-up review: key_ops gate ran before the try/except and used a bare "verify" not in key_ops check: a non-list value (123, true) raised TypeError instead of failing closed, and a malformed string like "noverify" passed the check via substring match ("verify" IS a substring of "noverify"), letting an encryption-only key verify a signature anyway. Now requires isinstance(key_ops, list) before the membership check, so any non-list shape fails closed the same way the rest of _verify_signature already does for a malformed JWKS entry. Parametrized test covers all three shapes (int, bool, malformed string). nbf was checked asymmetrically with exp: a non-numeric nbf ("9999999999") was silently treated as absent instead of failing closed like a non-numeric exp already does. Made the two checks match. TrustedEndpoint(audience="") would match a token whose own aud is also "" - silently trusting a token that asserts no audience at all - and audience=[] can never intersect anything, rejecting every token with no signal the field is misconfigured rather than intentionally locking down. Added a field_validator rejecting both at construction. docs/identity.md's worked example said "None if verification fails" but then dereferenced identity unconditionally two lines later - AttributeError on any rejected token, not the PermissionError a reader would expect. Added the missing check and a test that runs the doc's example through both branches (success and rejection). The as_policy_kwargs() dict-shape fix from the previous round doesn't fully close the gap it was meant to: PolicyRule's bare-attribute matcher still can't address a Keycloak-shaped "/engineering" group path or a hyphenated role like "default-roles-company" (it splits on "." and requires \w+ segments), so those still never match - silently, not with an error. Rather than adding a list-membership operator to the shared policy DSL (a larger, cross-cutting change tracked separately in microsoft#3924), documented the exact name grammar on as_policy_kwargs() and in docs/identity.md, and added a test pinning both failure directions (an allow rule against a group path silently denies; a deny rule against a hyphenated role silently lets the call through) so the limit stays visible instead of reappearing as a surprise. Also added OAEP/footgun/noverify to .cspell-repo-terms.txt for words introduced by this round's own comments and tests - verified against the actual CI gate (scripts/ci/changed_lines.py --base origin/main --mode added-lines piped into cspell@8.17.3 --config .cspell.json), exits 0. 87/87 tests pass (test_external_jwks.py + test_govern.py). Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: reject audience lists containing an empty-string member MohammadHaroonAbuomar's third review round on the empty-audience guard: len(v) == 0 only catches "" and [], not a list that contains an empty-string member ([""], ["", "x"]) - that slips through and still matches a token whose own aud claim is "". Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address third review — license headers, stale DSL-limitation test, security audit doc Rebased onto main (post-microsoft#3924) surfaced a stale assumption in test_as_policy_kwargs_group_paths_and_hyphenated_roles_never_match: policy.py's unrecognized-condition fallback now fails closed for any non-allow rule, so the deny-rule half of the pinned scenario denies instead of letting the call through. Updated the test and the as_policy_kwargs()/docs/identity.md documentation to describe the current (still-limited, but no-longer-fail-open) behavior accurately. Also adds the missing repo-standard license header to external_jwks.py and its test file, and a security-audit doc under docs/security/audits/ covering the identity-surface changes across all three review rounds, per the CI gate and MohammadHaroonAbuomar's review. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> * fix: address fourth review — cspell misses and stale commit ref in security audit doc The audit doc itself failed the Spell Check job: the reviewer's name written out ("MohammadHaroonAbuomar"), and "footguns"/"unaddressable" weren't in the wordlist. Reworded rather than adding to the wordlist, and fixed a stale pre-rebase commit hash (d48413a -> db2172c) plus the round-3 fix-commit reference and the cspell-status claim, both left inaccurate by the rebase. Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> --------- Signed-off-by: Fernando Marino <fernando.marino85@gmail.com> Signed-off-by: yuvrajsingh2428 <offcyuvi2428@gmail.com>

… installs (microsoft#4017) * fix(core): move agt-policies to an opt-in extra, not a base dependency agent-governance-toolkit-core's dependencies pin agt-policies>=5.1.0,<6.0, which in turn pins agent-control-specification>=0.4.0b0,<0.5.0. Neither version is published to PyPI (PyPI tops out at agt-policies 5.0.0 and agent-control-specification 0.3.1b1), so a plain 'pip install agent-governance-toolkit-core' or '[full]' cannot resolve. agt-policies backs only the v4-to-ACS manifest migration CLI ('agt migrate'). Nothing in agentmesh.governance (govern(), GovernanceDenied, policy.py/PolicyEngine) imports agt or agent_control_specification - confirmed by grepping agent-mesh/src/agentmesh/ for both. The base governance runtime does not need this dependency at all. Moves it to a new 'migrate' extra instead, matching the existing optional-dependencies pattern for other CLI/framework-specific pieces (mcp, redis, django, etc.). Verified: a clean venv can now 'pip install agent-governance-toolkit-core[full]' and exercise govern()'s contains/startswith/endswith operators (microsoft#3924) end-to-end without agt-policies or agent-control-specification installed at all. Signed-off-by: karimad <kmehaleb@gmail.com> * trim comment on the migrate extra Signed-off-by: karimad <kmehaleb@gmail.com> * fix(core): give agent-control-specification its own base dependency Review caught that agent_os (force-included into this same wheel) imports agent_control_specification directly in 4 places: providers.py, cmd_validate.py, _native_adapter_runtime.py, and openai_agents_sdk.py. That package previously arrived only transitively via agt-policies's own pin, which the prior commit removed from the base dependency set - trading the original unresolvable-install failure for a quieter ImportError at runtime the moment agent_os actually exercised one of those paths. Fix: declare agent-control-specification>=0.3.1b0,<0.4.0 directly in agent-governance-toolkit-core's own dependencies, matching the range agt-policies 5.0.0 already resolved to before microsoft#3939. Verified in a clean venv: all four previously-broken agent_os modules import cleanly, and cmd_validate.py's _validate_manifest() actually runs end-to-end (not just import-checked). govern()'s contains/startswith/endswith path (microsoft#3924) still passes both the deny and allow cases. One related, pre-existing gap surfaced during this verification and is NOT fixed here (out of scope for a packaging PR): _native_adapter_runtime .py's _session_for() imports HostSession from agent_control_specification, which genuinely does not exist in any published release yet (checked 0.3.1b1's exports directly). That call path was already broken before this PR - it depends on ACS 0.4.0b0's API regardless of how the agt-policies/agent-control-specification pins are arranged - so it's unaffected by this change either way. Also: moved the new 'migrate' extra out from under the '--- Bundles ---' header (it's a single-package extra like redis/django, not a bundle), and added a CHANGELOG.md [Unreleased]/Fixed entry. Signed-off-by: karimad <kmehaleb@gmail.com> * shorten comments Signed-off-by: karimad <kmehaleb@gmail.com> * docs: disclose still-unresolvable migrate-extra pin, file tracking issue The migrate extra's agt-policies>=5.1.0,<6.0 pin predates this PR and is unchanged by it. It's still unresolvable on its own today (agt-policies 5.1.0 isn't published), and will conflict with this PR's new base ACS pin once it is (agt-policies would then require ACS>=0.4.0b0, base requires <0.4.0). Documented in the CHANGELOG and the extra itself, tracked in microsoft#4019 so it isn't rediscovered fresh. Signed-off-by: Karim Mehalebi <kmehalebi@egencia.com> * docs: tell agt migrate users they now need the migrate extra Signed-off-by: Karim Mehalebi <kmehalebi@egencia.com> * fix(core): pin agent-control-specification to 0.4.0b0 range agent_os requires Reviewer found 0.3.1b1 lacks HostSession and rejects every in-repo manifest, breaking the adapter runtime and agent-os validate despite the install itself resolving. Update the base pin and changelog entry to match. Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com> --------- Signed-off-by: karimad <kmehaleb@gmail.com> Signed-off-by: Karim Mehalebi <kmehalebi@egencia.com> Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com> Co-authored-by: Karim Mehalebi <kmehalebi@egencia.com> Signed-off-by: yuvrajsingh2428 <offcyuvi2428@gmail.com>

…rosoft#4020) PolicyRule's condition DSL gained contains/startswith/endswith operators in microsoft#3924, fixing a bug where those operators silently fell through to False (a deny rule using them never fired, with no warning). Adds examples/policy-condition-operators/ demonstrating the three operators and their fail-closed/fail-open semantics, since no existing example covered them. string_operators.py asserts each expected value rather than just printing it, so a pre-microsoft#3924 install fails loudly with an AssertionError instead of silently reproducing the bug it's meant to demonstrate. The operators aren't in any released agent-governance-toolkit-core yet (latest on PyPI is 5.0.0, predating microsoft#3924's 2026-09-15 merge), so the README documents that gap and gives a working checkout-based run path (PYTHONPATH into agent-mesh/agent-hypervisor/agt-policies, plus pydantic[email]/pyyaml/cryptography/httpx/python-dateutil - verified in a genuinely clean venv) rather than a pip-install path that would silently reproduce the fixed bug. Signed-off-by: Karim Mehalebi <kmehaleb@gmail.com> Signed-off-by: yuvrajsingh2428 <offcyuvi2428@gmail.com>

github-actions Bot added the size/M Medium PR (< 200 lines) label Sep 10, 2026

github-actions Bot added tests agent-mesh agent-mesh package size/L Large PR (< 500 lines) and removed size/M Medium PR (< 200 lines) labels Sep 10, 2026

Karim Mehalebi (karimad) marked this pull request as ready for review September 10, 2026 15:02

Karim Mehalebi (karimad) requested review from MohammadHaroonAbuomar and liamcrumm as code owners September 10, 2026 15:02

MohammadHaroonAbuomar requested changes Sep 11, 2026

View reviewed changes

Karim Mehalebi (karimad) force-pushed the fix/policy-condition-string-operators branch 2 times, most recently from 04f648a to 18c0101 Compare September 11, 2026 17:18

Karim Mehalebi (karimad) requested a review from MohammadHaroonAbuomar September 11, 2026 17:21

San-Hsien (SanHsien) added a commit to SanHsien/agent-governance-toolkit that referenced this pull request Sep 12, 2026

feat(upstream): sync upstream/main through 0533cea, adopt microsoft#3913

6a8457f

, microsoft#3916, microsoft#3924, microsoft#3853, advance watermarks

Carlos Hernandez (carloshvp) suggested changes Sep 12, 2026

View reviewed changes

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/federation.py

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/federation.py

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/federation.py Outdated

Karim Mehalebi (karimad) requested a review from Prayag (prayagupa) as a code owner September 13, 2026 17:34

Karim Mehalebi (karimad) force-pushed the fix/policy-condition-string-operators branch from a11a858 to 281cf7d Compare September 14, 2026 08:27

github-actions Bot added size/XL Extra large PR (500+ lines) and removed size/L Large PR (< 500 lines) labels Sep 14, 2026

Karim Mehalebi (karimad) requested a review from Carlos Hernandez (carloshvp) September 14, 2026 08:46

Carlos Hernandez (carloshvp) suggested changes Sep 14, 2026

View reviewed changes

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/federation.py

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/federation.py

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/federation.py Outdated

MohammadHaroonAbuomar requested changes Sep 14, 2026

View reviewed changes

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/policy.py Outdated

MohammadHaroonAbuomar mentioned this pull request Sep 14, 2026

feat: identity - support RS256/ES256 in ExternalJWKSProvider, propagate role/group claims #3956

Merged

16 of 47 tasks

Karim Mehalebi (karimad) force-pushed the fix/policy-condition-string-operators branch from 236baab to 01fb893 Compare September 15, 2026 08:10

Karim Mehalebi (karimad) requested a review from MohammadHaroonAbuomar September 15, 2026 08:29

Karim Mehalebi (karimad) requested a review from Carlos Hernandez (carloshvp) September 15, 2026 08:29

MohammadHaroonAbuomar requested changes Sep 15, 2026

View reviewed changes

Comment thread agent-governance-python/agent-mesh/src/agentmesh/governance/policy.py Outdated

Karim Mehalebi (karimad) added 3 commits September 15, 2026 17:48

Karim Mehalebi (karimad) force-pushed the fix/policy-condition-string-operators branch from 9b54271 to 21fabd8 Compare September 15, 2026 15:49

Karim Mehalebi (karimad) requested a review from MohammadHaroonAbuomar September 15, 2026 15:50

MohammadHaroonAbuomar approved these changes Sep 15, 2026

View reviewed changes

Karim Mehalebi (karimad) and others added 2 commits September 15, 2026 18:59

Merge branch 'main' into fix/policy-condition-string-operators

5590a87

Merge branch 'main' into fix/policy-condition-string-operators

ce57258

MohammadHaroonAbuomar merged commit b1a9f73 into microsoft:main Sep 15, 2026
124 checks passed

Karim Mehalebi (karimad) deleted the fix/policy-condition-string-operators branch September 15, 2026 17:55

Karim Mehalebi (karimad) mentioned this pull request Sep 17, 2026

fix(core): fix dependency pins blocking agent-governance-toolkit-core installs #4017

Merged

18 tasks done

Karim Mehalebi (karimad) mentioned this pull request Sep 17, 2026

docs(examples): add policy condition-DSL string-operator example #4020

Merged

15 of 18 tasks

Conversation

Karim Mehalebi (karimad) commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue

Problem & Solution

Impact on Your Work

Timeline

Alternatives Considered

Update — federation.py parity + hardening (self-review pass)

Type of Change

Package(s) Affected

Testing

Unit Testing

Manual Testing

Checklist

Attribution & Prior Art

AI Assistance

IP, Patents, and Licensing

Uh oh!

azure-pipelines Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

PR Review Summary

Uh oh!

azure-pipelines Bot commented Sep 10, 2026

Uh oh!

MohammadHaroonAbuomar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Carlos Hernandez (carloshvp) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Karim Mehalebi (karimad) commented Sep 14, 2026

Uh oh!

Karim Mehalebi (karimad) commented Sep 14, 2026

Uh oh!

Carlos Hernandez (carloshvp) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MohammadHaroonAbuomar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Karim Mehalebi (karimad) commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MohammadHaroonAbuomar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Karim Mehalebi (karimad) commented Sep 10, 2026 •

edited

Loading

Karim Mehalebi (karimad) commented Sep 15, 2026 •

edited

Loading