test(relay): fail CI when a new send_message action skips the egress guard - #105731
benbarclay wants to merge 1 commit into
Conversation
…guard #99220 authorizes a relay destination with a guard call at each entry point that takes a `target` — today `send` and `react`/`unreact`. That shape has a failure mode no per-site test can see: a NEW action with a `target` and no guard call is a silent bypass, and every existing test still passes because none of them know it exists. This derives the action set from SEND_MESSAGE_SCHEMA — the tool's own declaration of what a model may ask for — and drives each one against an unattested relay destination, asserting both that the guard refused it AND that nothing reached the wire. A new action lands in neither classification list and fails the coverage test, which is the signal to decide whether it names a destination. Deliberately NOT a grep over source text: AGENTS.md bans tests that read .py files, and rightly — a regex passes when a call site is mis-wired and fails on a correct refactor. Everything here runs the real send_message_tool entry point and asserts on observed behaviour. Proven falsifiable, four mutations, all killed: - add "broadcast" to the schema enum → coverage test fails (the tripwire) - remove the send-lane guard → refusal test fails - remove the react-lane guard → refusal test fails - make the guard refuse everything → liveness control fails That last one matters: without it, deleting the tool body would satisfy every other assertion in the file. Two fixture notes worth keeping, both found by the assertions rather than by reading: the `send` lane resolves platform config BEFORE the guard, so a fixture that only stubs attestation makes `send` fail with "platform not configured" and never reach the check (the test caught itself passing for the wrong reason); and the read-only allow-list is proven, not trusted — `list` is driven and asserted to perform no outbound act. Follow-up to #99220. 409 passed across the relay + tool suites.
૮ >ﻌ< ა ci reviewran on b94e0eb — test(relay): fail CI when a new send_message action skips th ❌ Job failuresPython lints / Windows footguns (blocking) · View jobJob Python lints / Windows footguns (blocking) failed.
|
alt-glitch
added
type/test
|
Closing. Mutation-checked this against the pre-existing
The premise was also weaker than stated: A durable version would derive the action set from a dispatch table in No production code was touched by this PR. Guard coverage on main is unchanged (#99220, |
2 participants
Why
#99220 authorizes a relay destination with a guard call at each entry point that takes a
target— todayaction="send"andaction="react"/"unreact". That shape has a failure mode no per-site test can see:This is the class-of-defect risk the removed decline latch was partly meant to cover. A CI tripwire is the cheaper answer: it catches the missing guard at review time rather than inventing turn-lifetime state to catch it at runtime.
What is actually new here
I mutation-checked this against the pre-existing
test_send_message_relay_target_authz.pyto avoid claiming credit it already earns. Being precise:Only the bottom two rows are new coverage. The existing suite already protects the two guard sites that exist today; it cannot protect a site that does not exist yet.
How it works
The action set is derived from
SEND_MESSAGE_SCHEMA— the tool's own declaration of what a model may ask for — and each action is driven against an unattested relay destination, asserting the guard refused and nothing reached the wire. Two classification lists are hand-written; the action set is derived, so a newly declared action belongs to neither and fails.The obvious escape hatch is closed: silencing the tripwire by classifying a new sending action as read-only also fails, because read-only actions are driven and asserted to perform no outbound act. Verified by adding a real unguarded
broadcasthandler and misclassifying it —test_read_only_actions_really_do_not_reach_a_destination[broadcast]fails.Not a grep over source text.
AGENTS.mdbans tests that read.pyfiles, and the reasoning applies here exactly: a regex passes when a call site is mis-wired and fails on a correct refactor. Every assertion runs the realsend_message_toolentry point.Liveness
A guard test that refuses everything is worthless. Making the guard refuse even attested destinations fails
test_the_guard_admits_an_attested_destination— without that row, deleting the tool body would satisfy every other assertion in the file.One fixture note
The
sendlane resolves platform config before the guard, so a fixture that only stubbed attestation madesendfail with"Platform 'discord' is not configured"and never reach the check under test. The test caught itself passing for the wrong reason; the assertion requiring the refusal be the guard's is what exposed it.409 passed across the relay + tool suites via
scripts/run_tests.sh. Test-only; no production change.Follow-up to #99220.