fix(agent-os): reject supervisors registered above the deterministic trust root

[LHMQ878]

Fixes #3509

What was wrong

The level-0 determinism rule was enforced by a single exact comparison, if level == 0 and is_agent, in both TrustRoot.validate_supervisor and SupervisorHierarchy.validate_hierarchy. Anything not equal to 0 passed — including every negative level, which places an LLM agent above the deterministic trust root, and every non-integer level, because "0" == 0 is False in Python.

validate_hierarchy had no check that could see a negative level at all. The determinism loop only inspects supervisors whose level is exactly 0, and the gap scan walks range(1, max_level + 1), which never covers negatives. So:

h = SupervisorHierarchy(trust_root=root)
h.register_supervisor("trust-root", level=0, is_agent=False)
h.register_supervisor("safety-agent", level=1, is_agent=True)
h.register_supervisor("evil-agent", level=-1, is_agent=True)

h.validate_hierarchy()          # -> []   <- the documented "valid" signal
h.get_authority_chain({})[-1]   # -> 'evil-agent'

An empty violation list is the API's "valid" answer (docs/api-reference.md), while the agent occupies the slot the docstring reserves for the trust root as final authority. That contradicts the invariant docs/owasp-agentic-top10-mapping.md states under ASI10:

TrustRoot — a deterministic (non-LLM) policy authority at the top of the supervisor hierarchy that cannot be prompt-injected.

What changed

trust_root.py — validate_supervisor

• level must be a real int. bool is excluded explicitly: it is an int subclass and False == 0, so a bool level would otherwise be read as the root level.
• level < 0 is rejected before the determinism check, regardless of is_agent. There is no configuration under which a supervisor above the root is meaningful, so even a deterministic one there is a misconfiguration — this is not a case where an opt-out applies.

supervisor.py — validate_hierarchy

• A violation is emitted for every supervisor with a negative level, checked before the level-0 rules so the message names the supervisor that is actually there rather than the root that is missing. With a lone supervisor at level -1 the old code reported only Level 0 (root) has no registered supervisor, which points at the wrong thing.

Behaviour table (is_agent=True unless noted):

level	before	after
-1 / -100	True	False
-1, is_agent=False	True	False
"0" / "1" / 1.5 / [0]	True	False
True	True	False
0	False	False
0, is_agent=False	True	True
1 .. N	True	True

One previously accepted configuration does change: level=0.0 with is_agent=False was accepted by validate_supervisor on main and is now rejected, since isinstance(level, int) excludes floats. It cost nothing, because no float level could be consumed downstream anyway -- the gap scan computes range(1, max_level + 1), which raises TypeError on a float. Pinned in test_float_level_never_reached_a_working_hierarchy. Otherwise unchanged: level 0 deterministic, and agent-based supervisors at levels 1..N.

Compatibility

validate_supervisor becomes stricter, so a caller that today passes level as a string and relies on True will start getting False. That is the defect being fixed — a governance check returning "acceptable" for the input it exists to reject — and there are no in-repo callers passing a non-int level. register_supervisor is annotated level: int already, so the hierarchy side is a pure addition of a missing violation.

Verification

• tests/test_trust_root.py: 44 passed. With the two source files stashed, the same file is 15 failed / 29 passed — every new assertion fails on main and passes with the fix.
• Full tests/ suite (-p no:randomly --continue-on-collection-errors): 4488 passed / 257 failed, against 4464 / 257 on base — exactly +24, the new test count, with no new failures. (The 257 pre-existing failures and 3 collection errors are unrelated: modules importing agentmesh / agent_sre.)
• ruff check and ruff format --check on the three touched files: identical to base (base already reports I001 + F401 in the test file and would reformat trust_root.py; this branch reports the same and nothing more).
• cspell on all 147 added lines: clean.

Tests added

TestValidateSupervisorLevelInvariant (4 methods / 16 cases) — negative levels rejected regardless of is_agent; non-integer levels rejected; bool levels rejected with the reason stated; and a table asserting the four previously valid combinations still behave the same.

TestNegativeLevelIsAViolation (5 methods / 6 cases) — an agent above the root is reported; a deterministic supervisor above the root is also reported; the authority-chain consequence is pinned so the reason the violation matters is testable, not just asserted in a comment; a negative level is reported even with no level 0 present, where the gap scan is empty; and a valid 0/1 hierarchy still returns [].

Checklist

• Tests added that fail without the fix and pass with it
• ruff check / ruff format --check state unchanged relative to base
• No new failures in the full test suite

[azure-pipelines]

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

[github-actions]

🤖 AI Agent: security-scanner — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

No security issues found.

[github-actions]

🤖 AI Agent: test-generator — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Test coverage looks good. No gaps identified.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Docs Sync

• validate_supervisor in trust_root.py -- missing docstring
• README.md -- section on hierarchy validation needs update
• CHANGELOG.md -- missing entry for stricter validation rules and behavioral changes

[github-actions]

🤖 AI Agent: code-reviewer — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

TL;DR: 0 blockers, 0 warnings. Fix correctly addresses critical security gaps in trust root validation.

#	Sev	Issue	Where

No action items required. Clean change.

[github-actions]

🤖 AI Agent: contributor-guide — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Welcome, and thank you for your detailed contribution!

You did a great job thoroughly testing the changes to ensure compatibility and correctness.

Before merging, please verify that all new violation messages are consistent with existing error phrasing for clarity.

Refer to CONTRIBUTING.md for guidance.

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

AI-generated review output. Treat it as untrusted analysis and verify before acting.

API Compatibility

Severity	Change	Impact
High	validate_supervisor now rejects non-integer level values (e.g., strings, floats, booleans).	Code relying on non-integer level values will break, as these are now explicitly invalid.
High	validate_supervisor now rejects negative level values.	Supervisors with negative levels, previously allowed, will now be rejected.
High	validate_hierarchy now emits violations for supervisors with negative level values.	Hierarchies with supervisors at negative levels will now fail validation.

These changes introduce stricter validation rules, which may break existing code that relied on the previously lenient behavior.

[github-actions]

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.

[github-actions]

🔴 Contributor Check: HIGH

Check	Result
Profile	HIGH
Credential	LOW
Overall	HIGH

Automated check by AGT Contributor Check.

[Copilot]

Pull request overview

TL;DR: 1 blocker, 1 warning. Fix #1 and this ships.

This PR tightens agent-os supervisor hierarchy invariants by rejecting supervisors registered “above” the deterministic trust root (negative levels) and by hardening TrustRoot.validate_supervisor against type-confusion inputs; it also adds regression tests to lock the behavior in.

Changes:

• Enforce level as a real int (excluding bool) and reject level < 0 in TrustRoot.validate_supervisor.
• Emit a validate_hierarchy() violation for supervisors registered with negative levels.
• Add tests covering negative, non-integer, and boolean levels plus authority-chain implications.

#	Sev	Issue	Where
1	Block	validate_hierarchy() can raise TypeError on non-int level inputs (contract is “return violations list”)	agent_os/supervisor.py::validate_hierarchy
2	Warn	Docstring grammar typo (“reachable around”)	tests/test_trust_root.py

#1: Treat non-integer (and bool) supervisor levels as violations and filter them out of the gap-scan max_level computation so validation fails closed without crashing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
agent-governance-python/agent-os/src/agent_os/trust_root.py	Tightens supervisor level validation (type + non-negative) before applying the level-0 determinism rule.
agent-governance-python/agent-os/src/agent_os/supervisor.py	Adds negative-level detection to hierarchy validation (but needs type-safety to avoid runtime errors).
agent-governance-python/agent-os/tests/test_trust_root.py	Adds regression tests for negative/non-integer/bool levels and hierarchy violations.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

agent-governance-python/agent-os/tests/test_trust_root.py:69

• The docstring claims that any non-int level "skipped the determinism check", but that wasn’t true for all values in the parametrization (e.g., 0.0 == 0 and False == 0, so those cases did not bypass the old level == 0 check). Consider rewording to describe the actual failure mode (values not equal to 0, plus bool semantics) so the test rationale stays accurate.

    """A level that is not a real ``int`` skipped the determinism check.

    ``"0" == 0`` is False in Python, so a string level -- exactly what a config
    loader that skips coercion produces -- was never compared against the root
    at all rather than failing the comparison. ``bool`` is covered here too: it

[LHMQ878]

Rebased onto main at c38d90a to clear a conflict. The conflict was not cosmetic, so flagging what I had to decide.

main rewrote TrustRoot's constructor and flattened test_trust_root.py. The API is now TrustRoot(runtime) delegating to a native ACS runtime, where this branch was written against TrustRoot(policies=[GovernancePolicy(...)]); the test file went from class-grouped tests to four flat functions with a shared _root() helper and a _Runtime stub. My 114 test lines could not merge into that.

I took main's side wholesale — its validate_supervisor docstring, its file layout, its _root() helper — and re-added my tests as flat functions in that style rather than reintroducing the class grouping main had just removed. The two TestX classes became six module-level tests, and the two @pytest.fixture-based root fixtures were dropped in favour of calling _root() directly, which is what the surrounding tests now do. The non-integer and bool cases were merged into one parametrize since bool being an int subclass is the same point.

The source fix itself is unchanged — validate_supervisor rejects non-int levels and negative levels before the determinism check, and validate_hierarchy emits a violation per negative level.

A verification caveat I should state plainly: I could not run pytest tests/test_trust_root.py on this rebase. Main's new _Runtime stub imports agent_control_specification, which needs a compiled _native extension that is not built in my environment:

E   ImportError: cannot import name '_native' from partially initialized module
    'agent_control_specification' ... (most likely due to a circular import)

That is pre-existing and not caused by this branch — origin/main's own copy of the file fails to collect identically, which I checked by stashing my changes and re-running. CI has the built extension, so it will run there.

To not leave the logic unverified, I exercised every assertion in the new tests against the rebased source directly, stubbing only the runtime (which none of these code paths call):

ok   validate_supervisor level=-1 is_agent=True = False
ok   validate_supervisor level=-1 is_agent=False = False
...
ok   validate_supervisor non-int level='0' = False
ok   validate_supervisor non-int level=True = False
ok   validate_supervisor accepted level=0 is_agent=False = True
ok   validate_supervisor accepted level=0 is_agent=True = False
ok   agent above root reported = True
     violations: ["Supervisor 'above-root' has negative level -1; level 0 is the root and nothing may sit above it"]
ok   authority chain last = 'above-root'
ok   lone negative level=-7 = True
ok   valid hierarchy has no violations = []

FAILURES: 0

24 checks, all passing. ruff output on both changed files is identical to origin/main (the one I001 in the test file is main's own, from its reformat).

Happy to reshape anything if the flat style is not what you wanted here — main had only just landed it, so I followed it rather than assume.

[liamcrumm]

Reviewed. The escalation reproduces on main (level=-1 accepted, authority chain puts the agent above the trust root) and the fix holds for every malformed level I tried.

One correction for the PR description: it says previously accepted configurations are unchanged, but level=0.0 with is_agent=False was accepted on main and is now rejected, since isinstance(level, int) excludes floats. Fail-closed so nothing is exploitable, but a YAML or JSON round-trip that yields 0.0 will have a valid root rejected.

[LHMQ878]

You're right, and thanks for testing it rather than reading it — that claim in my description was wrong. Corrected above, and pinned in 93d8d849.

Confirmed the case you named, and it is broader than 0.0: every float level was accepted on main, not just the root one.

BASE (main), validate_supervisor:          FIXED:
  True   level=0.0, is_agent=False           False   <-- your case
  True   level=1.0, is_agent=True            False
  True   level=2.5, is_agent=True            False
  True   level=-1                            False   (the vulnerability)
  True   level='0'                           False
  True   level=True                          False

But a float never reached a working hierarchy

This is what decided me against loosening the check to isinstance(level, (int, float)) or coercing integral floats. Nothing downstream can consume a float level — the gap scan in validate_hierarchy computes range(1, max_level + 1):

$ # base commit c38d90ae, a lone level=0.0 root, is_agent=False
root at level=0.0 alone -> TypeError: 'float' object cannot be interpreted as an integer
$ # same on this branch
root at level=0.0 alone -> TypeError: 'float' object cannot be interpreted as an integer

So a config that got a float past validate_supervisor on main crashed at the next step rather than being governed. The change converts an unhandled TypeError into a False, which is a strictly better failure for the same input — it does not narrow the set of hierarchies that ever validated.

That reasoning is now a test rather than a comment, since the next reader will have exactly your doubt:

@pytest.mark.parametrize('level', [0.0, 1.0, 2.5])
def test_float_level_never_reached_a_working_hierarchy(level: float) -> None:
    hierarchy = SupervisorHierarchy(trust_root=_root())
    hierarchy.register_supervisor('root', level=level, is_agent=False)
    with pytest.raises(TypeError, match='float'):
        hierarchy.validate_hierarchy()

If you would rather a YAML 0.0 keep working, the right place is coercion at the config-loading boundary — int(level) when float(level).is_integer(), rejecting otherwise — not a wider type check in the invariant, since the invariant's job is to name the malformed input and range() will still refuse it. Happy to add that here or as a follow-up, whichever you prefer; I left it out because it is a different layer from the escalation this PR closes.

Verification caveat, same as I noted on #3508: tests/test_trust_root.py cannot be collected on my machine — ImportError: cannot import name '_native' from partially initialized module 'agent_control_specification', a compiled extension I do not have, and it fails identically on the base commit. So the new test body was exercised standalone against the real SupervisorHierarchy (3/3 parameters pass) rather than through the suite. ruff check --select E,F,W --ignore E501 clean.

Glad the level=-1 escalation reproduced for you.