Reject a non-string target_agent instead of raising TypeError - #107
Open
LibertasSpZ wants to merge 1 commit into
Open
Reject a non-string target_agent instead of raising TypeError#107LibertasSpZ wants to merge 1 commit into
target_agent instead of raising TypeError#107LibertasSpZ wants to merge 1 commit into
Conversation
extract_handoff checks the handoff target with `target not in
ALLOWED_TARGETS`, which hashes the value. The blob is parsed out of the
orchestrator's own text output, which is downstream of untrusted-document
readers, so whoever controls a processed document controls the JSON type
of every field in it — and a JSON array or object is unhashable:
>>> extract_handoff('{"type":"handoff_request",'
... '"target_agent":["reg-monitor"],"payload":{}}')
TypeError: unhashable type: 'list'
run() calls extract_handoff inside the stream loop with no handler, so
this stops the orchestrator rather than logging a rejected handoff — a
denial of service reachable from document text, through the gate that
exists to stop document text. Every other field is type-checked by
jsonschema, but target_agent is read before that validation runs.
The isinstance check goes first so membership is only ever attempted on a
string. Rejection now takes the normal path and is audited under the
existing target_not_allowlisted reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
target_agent instead of raising TypeError
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
extract_handoffchecks the handoff target against the allowlist withSet membership hashes the value. The blob is parsed out of the orchestrator's own text output, which is downstream of untrusted-document readers — the threat model the module docstring already sets out — so whoever controls a processed document controls the JSON type of every field in it, and a JSON array or object is unhashable:
run()callsextract_handoffinside the stream loop with no exception handling, so this stops the orchestrator rather than logging a rejected handoff — a denial of service reachable from document text, through the gate that exists to stop document text. It fails stopped rather than open, so it is not an allowlist bypass.Every other field is type-checked by
jsonschemaa few lines further down.target_agentis read before that validation runs, which is why it is the only field with this exposure.Fix
One line, plus a comment so the
isinstanceis not tidied away later:Rejection now takes the normal path and is audited under the existing
target_not_allowlistedreason — no new code path, no new reason string.Verified against the patched script:
["reg-monitor"]and{"slug": "reg-monitor"}are now rejected and logged, while"reg-monitor"still approves and"nope",nulland7reject as before.Relationship to #106
That PR marks this defect
xfail(strict=True)attests/test_orchestrate_handoff.py::test_an_unhashable_target_is_rejected_rather_than_raising,asserting the behaviour this change introduces. If this lands first, the marker has to be dropped in the same merge or the suite goes red on
XPASS(strict). Happy to add that removal to this PR if you would prefer to take the fix ahead of the tests.