microsoft / microsoft/agent-governance-toolkit

Declarative negative operators (`ne`, `not_in`) fail open on missing fields, bypassing deny rules

Open
#3,297 2 comments 0 reactions 0 assignees View on GitHub
stale
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.