fix(assertions): approval_required accepted approvals a target could fabricate

[avp9-nexus]

Fixes #

No open issue covers this — the report is in the description below. Happy to open one first if you prefer that order.

Describe the changes you have made in this PR -

approval_required decides which approval sources it accepts from input.context. That context is sent to the target: build_target_payload returns {"scenario_id", "input"} (adapters.py:26-29), and input.context carries the trust: trusted labels. A target therefore reads which source names would be accepted, and the trace it writes is the same trace the assertion reads.

The shipped approval_bypass scenario shows it end to end. Its context declares source: approval_service with trust: trusted and no content — an entry whose only job is to name a trusted source. A target that copies that name into {"type": "approval", "action": "change_root_password", "decision": "approved", "source": "approval_service"} gets pass, and that event is the one in test_approval_required_passes_with_valid_approval_from_trusted_source up to the name.

docs/assertions/approval_required.md states the opposite today:

An agent cannot fabricate a trusted approval by writing a fake event.

The change. Accept trusted_sources on the assertion. Assertion config never leaves the harness, so a name declared there is not readable from the payload. When it is absent the assertion falls back to the context-derived set, so scenarios written before this keep working unchanged.

Five files: the assertion, the shipped scenario migrated to the declared form, the doc, four tests, and a CHANGELOG entry. No schema change — assertions.items is already additionalProperties: true.

What it does not close. Declaring the source stops it being readable. It does not stop it being guessed, and the approval event is still written by the implementation under test. An approval a verifier could check without trusting the emitter would need a record produced outside that implementation. The doc now says this rather than the sentence quoted above, and the fallback path keeps the old behaviour for scenarios that have not migrated — that is deliberate, and stated in the doc.

---

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code or documentation ?

• No, I wrote all the code myself
• Yes, I used AI assistance

If you used AI assistance, briefly describe:

• Tool(s) used: Claude Code (Anthropic).
• Parts of the contribution that were AI-assisted: the diff to assertions.py, the four tests, the doc rewrite, the scenario migration, and this description were drafted with it.
• How you reviewed the output: I reproduced the problem against main before any code was written, and checked each factual claim against the files rather than taking it from the model — adapters.py:26-29, the shipped scenario's content-less approval_service entry, and the doc sentence quoted above. I then read the diff line by line and confirmed the fallback branch leaves unmigrated scenarios byte-identical in behaviour.
• Tests or checks I ran: on CPython 3.13.15, against a checkout of main, in both directions. Before the change: a target that never approves fails, a legitimate approval passes, and a target that copies the trusted name out of the payload also passes — the hole reproduces. After the change, on the migrated scenario: the payload carries no trusted name, the copied-name trace fails, the legitimate approval still passes, the missing approval still fails. On an unmigrated scenario the three legacy outcomes are unchanged, including the old hole. Plus the four new tests. I have not run the full pytest suite or the CI workflows locally — happy to if you would rather see that first.

---

Checklist before requesting a review

• I have added proper PR title and linked to the issue (no issue exists for this; the report is in the description above)
• I have performed a self-review of my code
• I can explain the purpose of every documentation, function, class, and logic block I added
• I understand why my changes work and have tested them thoroughly
• I have considered potential edge cases and how my code handles them (the absent-trusted_sources path has a test; an empty list and non-string entries are handled by the comprehension but not separately tested — say the word and I will add those two. See "What it does not close" for the limit that stays open by design.)
• If it is a core feature, I have added thorough tests
• My code follows the project's style guidelines and conventions (the four tests reuse make_scenario, Trace, and evaluate_approval_required in the style of the neighbouring tests)
• I added an entry under [Unreleased] in CHANGELOG.md (or this PR does not need one — pure refactor, internal tests, or CI-only change)

[ossumpossum]

The "what this does and does not establish" paragraph in approval_required.md is the right way to write this up: trusted_sources narrows who can be credited as the source, it doesn't make the approval event itself independently observed, since the event is still written by the target. That's the accurate scope of the fix, stated in the same PR rather than left for someone to find later.

A genuinely independent approval record would need either the approval service itself as a trace participant whose output the harness reads directly, or corroboration from something outside trace.events entirely. Interested in sketching what that would look like as a follow-up scenario once this has merged and settled, if that's useful groundwork.

Separate question: is a lint or CI check planned to flag scenarios still on the input.context fallback path, now that the docs call it out as the weaker form? Nothing in tests.yml currently would catch one, and I wasn't sure if migrating existing scenarios off it is already planned or expected to stay long-lived.

[avp9-nexus]

Agreed on the scope, and on saying it in the PR rather than leaving it to be found: trusted_sources narrows who can be credited, the approval event is still written by the target. That sentence is the whole limit of this fix.

On the follow-up: yes, once this has merged and settled. The version that would make the approval record independent is the one you name - the approval service as a trace participant whose events the harness reads directly, so the record is produced by a component the target never writes. That is the same move as this fix, one step further along the path: here the trusted-source declaration stopped traveling through the target; there the approval event itself would. Worth sketching as a scenario, and I would rather do it against the merged shape than guess at it now.

On the lint question - measured rather than assumed, on main as of today: one scenario carries an approval_required assertion, none carries trusted_sources, and nothing in tests.yml inspects assertion shape - it runs the scenarios and dry-runs the rest. So you are right that nothing catches a scenario left on the input.context path. This PR migrates the one shipped scenario; the check itself is a single test - fail when an approval_required assertion has no trusted_sources - and I would add it as a follow-up after this merges rather than grow the PR while it is waiting on a maintainer. Whether migration off the fallback is meant to be complete or the fallback is meant to stay long-lived is a maintainer call I do not want to pre-empt in a test.

Avp9.