Coerce session.steer/redirect text against non-string JSON - #79319
Closed
aydnOktay wants to merge 1 commit into
Closed
Coerce session.steer/redirect text against non-string JSON#79319aydnOktay wants to merge 1 commit into
aydnOktay wants to merge 1 commit into
Conversation
…ing JSON Inline RPCs called .strip() on raw params; list/int text raised AttributeError and could tear down the reader thread. Co-authored-by: Cursor <cursoragent@cursor.com>
alt-glitch
added
type/bug
later0day
added a commit
to later0day/hermes-agent
that referenced
this pull request
Aug 7, 2026
Upstream documents the "fail closed" rationale next to the relay branch in
_deliver_result, and the fork's diff removed that three-line comment. In
tandem the fork added a DingTalk-specific session_webhook fallback block
above the relay branch (lines 2298-2338). The webhook is keyed off the
job's ORIGIN — not the transport — so a relay-fronted DingTalk job whose
relay send failed would fall through into the webhook block and
re-deliver behind the relay's back, using a credential the connector does
not own. That is exactly the double-delivery the fail-closed branch
existed to prevent.
Fix:
1. Restore the fail-closed rationale comment above the relay branch.
2. Gate the DingTalk session_webhook fallback on
`not (transport is not None and transport.is_relay)` so it stays
available for standalone / native transports (where the process
legitimately owns the credential) but never re-delivers on a
relay-fronted target.
Guards (tests/cron/test_relay_delivery_fail_closed.py):
- test_failed_relay_delivery_does_not_retry_standalone
- test_raising_relay_delivery_does_not_retry_standalone
- test_relay_fronted_dingtalk_does_not_bypass_via_session_webhook
- test_native_dingtalk_still_uses_the_session_webhook_fallback
- test_relay_fail_closed_comment_documents_the_invariant
Two-way negative control: removing the comment triggers test 5; removing
the relay gate triggers test 3. Native (non-relay) DingTalk continues to
use the captured webhook exactly as before.
SECURITY.md §2.6 / NousResearch#79319 — no code-path fail-open on relay ownership.
Contributor
Author
|
Closing to concentrate review on a single linked product bug: #89979. |
Contributor
Author
|
Closing to concentrate review on a single linked product bug: #89979. |
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
2 participants
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.
Summary
session.steer/session.redirectrun inline (not in_LONG_HANDLERS); bare(params.get("text") or "").strip()AttributeErrors on list/inttextand can tear down the stdin/WS reader.str(... or "")before.strip(); same hardening forllm.oneshottemplate/task andhandoff.requestplatform."42", list text must not crash.