microsoft / microsoft/agent-governance-toolkit
Declarative negative operators (`ne`, `not_in`) fail open on missing fields, bypassing deny rules
- Dominant language
- Python
- Stars
- 6.3k
- Forks
- 1.1k
- Avg merge
- 5d 11h
- Merged PRs (30d)
- 142
Description
## Summary
In the declarative rule compiler (`agt-policies`), **negative comparison operators (`ne`, `not_in`) fail open when the referenced field is absent**. A deny rule built on a negative operator does not fire when the field it checks is missing, so the action is allowed. This lets a caller bypass such a rule by simply omitting the field.
## Impact
Negative operators are the natural way to express allowlist-style deny rules, which are exactly the security-sensitive ones:
- **Content-hash pinning**: `deny if tool_call.content_hash ne "sha256:REGISTERED"`. A tool call with **no** `content_hash` is allowed, so an unsigned or tampered call bypasses integrity pinning by dropping the field.
- **Destination/region allowlists**: `deny if tool_call.args.region not_in [...]`. A call that omits `region` is allowed.
The gate is defeated by omission, which is trivial for a caller to do.
## Root cause
`_rego_op_clause` in `agent-governance-python/agt-policies/src/agt/manifest_resolution/build.py` emits a `_v != null` guard before every comparison, and field accessors resolve a missing field to `null` (`_rego_field_accessor`, the `object.get(..., null)` sentinel). For `ne`:
```rego
_v := object.get(object.get(input.snapshot, "tool_call", null), "content_hash", null)
_v != null
_v != "sha256:REGISTERED"
```
When the field is absent, `_v == null`, so `_v != null` is false, the rule body fails, and evaluation falls through to `default verdict := allow`.
The `_v != null` guard is a deliberate "fail closed on missing field" choice (see the comment in `_rego_field_accessor`). That is correct for **positive** operators (`eq`, `gt`, `lt`, `in`), where a missing field should not satisfy a positive assertion. But it is applied uniformly, and for **negative** operators it inverts the intended safety: a missing field logically *is* "not equal to X" / "not in the set", which for a deny rule is exactly the case that should fire.
## Reproduction
OPA 0.70. Run from repo root with `PYTHONPATH=agent-governance-python/agt-policies/src`.
```bash
REGO=$(python3 -c 'from agt.manifest_resolution.build import _render_rego; print(_render_rego([{"name":"deny_bad_hash","condition":{"field":"tool_call.content_hash","operator":"ne","value":"sha256:REGISTERED"},"action":"deny","message":"hash mismatch"}]))')
echo '{"snapshot":{"tool_call":{"name":"exec"}}}' | opa eval -I -d <(echo "$REGO") 'data.agt.legacy.verdict.decision' -f raw
```
| Operator | Field state | Expected | Actual |
|----------|-------------|----------|--------|
| `ne` | value matches | allow | allow |
| `ne` | value differs | deny | deny |
| `ne` | **field missing** | **deny** | **allow** |
| `not_in` | value in set | allow | allow |
| `not_in` | value not in set | deny | deny |
| `not_in` | **field missing** | **deny** | **allow** |
## Suggested fix
For negative operators (`ne`, `not_in`), treat an absent field as **satisfying** the condition so a deny rule fires. Concretely, drop or invert the `_v != null` guard for these operators (e.g. `ne` becomes "field is missing OR `_v != value`"). More generally, ensure deny rules fail closed on missing fields rather than inheriting the positive-operator guard.
## Scope
Affects the declarative `governance.yaml` -> Rego compiler path (`data.agt.legacy.verdict`). It does not affect the ACS-direct host-dispatcher path, where the decision logic is written directly.
Contributor guide
Research direction
Start in agent-governance-python/agt-policies/src/agt/manifest_resolution/build.py, reading _rego_op_clause, _rego_field_accessor, and _render_rego. Run the OPA 0.70 reproduction from the issue, then add regression coverage for missing fields with ne and not_in. Done means those cases deny while the matching, differing, in, and out-of-set cases retain their expected verdicts.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- compilers, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100