Repository navigation
security: enforce PIC bridge schema constraints - #203
Conversation
Reviewed by running it, after the merge, on main at 4785fa7. Parity is complete and I could not break it: 46 cases — each of the eleven required fields dropped, a wrong type substituted into each, the two profile constants and the root object violated, and hand-written boundaries for minLength, minItems, uniqueItems, minimum and the decision enum — produced zero cases where the schema says no and the code says yes. One of the 46 is classified in a way that reads against the docstrings. decision is enum: ["allow", "deny"] in the schema and a != "allow" comparison in the code, so "banana", 1, None and {"a": 1} all raise AuthorizationDenied: the signed decision is not allow. It fails closed, so this is not a hole — but IntentBridgeError is documented as “malformed… or cannot be evaluated” and AuthorizationDenied as “the signed decision is not an authorization to execute”, and a value outside the enum is the former. A membership check before the comparison moves them to the class that describes them. The part I would act on is the tests. Removing each of the five guards this PR adds, one at a time, and running the whole suite: guard removed suite Three of the five can be deleted and CI stays green. Two causes. Both pytest.raises are bare IntentBridgeError, and AuthorizationDenied and AuthorizationMismatch subclass it, so a case passes on any rejection — with the item-emptiness guard gone, ("tools", [""]) still raises, as AuthorizationMismatch: executed tool is outside the authorized tool scope. And the scalar cases run at now=0 while the fixture’s authorized_at is 100, so authorization is not yet valid catches four of the five before the guard they name is reached. minItems additionally has no case: the four scope parameters cover the duplicate and empty-string halves, not the empty array. Three changes fix all of it, and I ran them: match= on both pytest.raises, the scalar cases at now=150, and ("tools", []) and ("impacts", []) added. That is 731 passed, ruff and mypy clean, and every one of the five guards then goes red when removed — including the three that are currently free to delete. One piece of context for why this file carries more weight than most: agentrust_trace.intent_bridge is in all but has no caller inside this repository and no conformance coverage in agentrust-trace-tests, so tests/test_intent_bridge.py is the only thing checking it. Happy to open the test-only PR if useful. |
…uard is gone (#204) #203 added five runtime guards and nine parametrised cases for them. Three of the five guards can be deleted with the whole suite still green: guard removed suite the three _nonempty_string calls 729 passed or not value (the minItems half) 729 passed item non-emptiness (the minLength half) 729 passed authorization[field] < 0 1 failed the duplicate check 2 failed Two causes. Both pytest.raises are bare IntentBridgeError, and AuthorizationDenied and AuthorizationMismatch subclass it, so a case passes on any rejection: with the item-emptiness guard removed, ("tools", [""]) still raises, as AuthorizationMismatch "executed tool is outside the authorized tool scope". And the scalar cases run at now=0 while the fixture's authorized_at is 100, so "authorization is not yet valid" catches four of the five before the guard they name is reached. minItems has no case at all: the four scope parameters cover the duplicate and empty-string halves, not the empty array. Three changes, no src change. Add match= to both pytest.raises so a case can only pass on the rejection it names. Move the scalar cases to now=150, inside the fixture's validity window. Add ("tools", []) and ("impacts", []). With this, removing any one of the five guards turns the file red. Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
Security hardening prepared with reproducible evidence and tests. Please review and run required CI.