fix(gateway): allow source code/log extensions in MEDIA: delivery - #36060
Closed
Randimt wants to merge 1 commit into
Closed
fix(gateway): allow source code/log extensions in MEDIA: delivery#36060Randimt wants to merge 1 commit into
Randimt wants to merge 1 commit into
Conversation
The MEDIA: extension allowlist (MEDIA_DELIVERY_EXTS, follow-up to NousResearch#34517) covered docs/spreadsheets/archives/web output but not source code, build manifests, or logs. A response of 'MEDIA:/path/to/script.py' was silently dropped — extract_media() didn't match it, MEDIA_TAG_CLEANUP_RE left the tag in the body, and the file never reached the user. Same for .ts/.tsx, .go, .rs, .sh, .sol, .toml, .log, etc. — the very file types most likely to be shared in a chat workflow (code review, snippet sharing, log triage). - Extend MEDIA_DELIVERY_EXTS with source code (Python/JS/TS/shell/systems /JVM/scripting/Solidity/web), build metadata (.toml, .css), and .log. Sensitive single-purpose configs (.env, .conf, .ini) are deliberately excluded — the credential-path denylist is the runtime backstop, but keeping these out of the extension allowlist is the cheaper layer of defence against accidental delivery from non-denied locations. - Replace two duplicated extension regexes in gateway/run.py (history scan + post-run scan) with imports of _MEDIA_EXT_ALTERNATION from base.py. They were drifting from the source of truth that NousResearch#34517 introduced; pulling them through the shared constant closes the gap. - Regression tests: every new extension extracts via MEDIA: tag, and .env/.conf/.ini stay out of the allowlist (anti-regression for the credential-leak guardrail).
alt-glitch
added
type/bug
mxnstrexgl
approved these changes
Jun 1, 2026
mxnstrexgl
left a comment
mxnstrexgl
left a comment
There was a problem hiding this comment.
LGTM — automated review passed. No security, quality, or test coverage issues detected.
Contributor
|
Thanks for identifying the explicit Problems
Suggested changes
Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-message-delivery
teknium1
added a commit
that referenced
this pull request
Jul 16, 2026
…gress MEDIA: tags whose path had an unknown extension (.py, .log, .toml, .weirdext, ...) fell between both extraction passes: the anchored extension allowlist (MEDIA_TAG_CLEANUP_RE) did not match them, and the extension-less pass explicitly skipped any path that HAD a suffix. The file was never delivered even though the intended design (universal ingress/egress) says any non-credential file should ship. Widen _path_lacks_deliverable_extension() so the validated delivery pass (MEDIA_EXTENSIONLESS_TAG_RE + validate_media_delivery_path) covers every path the extension allowlist does not — unknown extensions and extension-less files alike. Security posture is unchanged: unknown-extension paths only deliver after full validation (exists, symlinks resolved, credential/system denylist, strict-mode allowlist+recency), and unvalidated tags stay visible in the text instead of being silently dropped. Known extensions keep their unconditional pre-existing behavior. Because extract_media, _strip_media_tag_directives (non-streaming dispatch), and strip_media_directives_for_display (streaming) all share the same two regexes + predicate, all delivery paths pick up the widened behavior with no per-site changes. Dispatch partition in gateway/run.py already routes non-image/video extensions through send_document. Closes the gap reported in PR #36060; supersedes the allowlist-append approach there (an extension allowlist can never enumerate every file type a user asks the agent to produce). Co-authored-by: Randimt <randimt@users.noreply.github.com>
Contributor
|
Merged via #65510 (rebase-merged onto current main, head commit 2fd36b1) — your extension-list idea was widened into universal validated egress — any MEDIA: path now delivers after passing the credential/system-path validation, so no allowlist maintenance is ever needed again. You're credited as Co-authored-by on the commit. Thanks for the contribution! |
exiao
added a commit
to exiao/hermes-agent
that referenced
this pull request
Jul 20, 2026
Third baseline failure (test_platform_base.py::TestUniversalMediaEgress x3). The egress redesign (2fd36b1, NousResearch#36060) intends unknown/code extensions to deliver ONLY via the validated pass (validate_media_delivery_path: exists on disk, safe root, not denylisted) so a prompt-injection MEDIA:/etc/x.py stays visible instead of silently exfiltrating. But the code still folded MEDIA_TAG_EXTRA_EXTS (.py/.log/.toml/...) into _MEDIA_TAG_EXT_ALTERNATION, the UNCONDITIONAL extract grammar — contradicting its own design comment and the egress tests. Point _MEDIA_TAG_EXT_ALTERNATION at MEDIA_DELIVERY_EXTS only, so code/config/log tags route through the validated pass. MEDIA_TAG_EXTRA_EXTS is retained for the separate producer-tool detector in run.py (_TOOL_MEDIA_RE). test_send_file_tool's expanded-ext test used nonexistent /tmp paths, which under the validated design correctly no longer extract. Fixed the fixture to create a real file in a monkeypatched safe root, exercising the true intended delivery path for every extension. Verified: TestUniversalMediaEgress 13/13, test_send_file_tool 85/85, full affected set 426 passed. The 5 unrelated red tests (SSRF, url_safety, cross_profile_guard) are pre-existing on clean live-config and untouched here. Approach A per Eric: egress design authoritative.
exiao
added a commit
to exiao/hermes-agent
that referenced
this pull request
Jul 20, 2026
…e shadow handlers) (#139) * fix(gateway): delete stale shadow slash-handlers + raise KANBAN_GUIDANCE ceiling Consolidates the live-config CI baseline repair (supersedes #136, #137, #138). 15 tests were failing on live-config across 4 modules. This fixes 12 via the correct root cause; 3 egress tests are a separate product decision (see patch note). 1. Stale shadow handlers (11 tests: test_fast_command x3, test_choice_picker x8). The June slash-command extraction moved _handle_reasoning_command and _handle_fast_command into GatewaySlashCommandsMixin with the new choice-picker logic, but left the OLD copies in the GatewayRunner class body in run.py. A method defined on the subclass overrides an inherited mixin method, so the stale copies shadowed the picker-enabled versions, making the choice picker unreachable and /fast --global unrecognized. Delete the two stale defs (166 lines) so the mixin versions activate. #137's re-implement-into-the-shadow approach was wrong; the shadow must be deleted. 2. KANBAN_GUIDANCE size ceiling (1 test). Upstream b5bd0ef added legitimate attachment guidance (5749 chars vs the <5500 cap). Raise cap to 6500. 219 tests pass across the affected modules; send_file's 57 expanded-ext tests still pass (unlike #138 which broke them). ruff clean. Patch note: ~/.hermes/plans/hermes-patches/live-config-baseline-consolidated.md * fix(gateway): route MEDIA-tag code/config exts through validated egress Third baseline failure (test_platform_base.py::TestUniversalMediaEgress x3). The egress redesign (2fd36b1, NousResearch#36060) intends unknown/code extensions to deliver ONLY via the validated pass (validate_media_delivery_path: exists on disk, safe root, not denylisted) so a prompt-injection MEDIA:/etc/x.py stays visible instead of silently exfiltrating. But the code still folded MEDIA_TAG_EXTRA_EXTS (.py/.log/.toml/...) into _MEDIA_TAG_EXT_ALTERNATION, the UNCONDITIONAL extract grammar — contradicting its own design comment and the egress tests. Point _MEDIA_TAG_EXT_ALTERNATION at MEDIA_DELIVERY_EXTS only, so code/config/log tags route through the validated pass. MEDIA_TAG_EXTRA_EXTS is retained for the separate producer-tool detector in run.py (_TOOL_MEDIA_RE). test_send_file_tool's expanded-ext test used nonexistent /tmp paths, which under the validated design correctly no longer extract. Fixed the fixture to create a real file in a monkeypatched safe root, exercising the true intended delivery path for every extension. Verified: TestUniversalMediaEgress 13/13, test_send_file_tool 85/85, full affected set 426 passed. The 5 unrelated red tests (SSRF, url_safety, cross_profile_guard) are pre-existing on clean live-config and untouched here. Approach A per Eric: egress design authoritative. * fix(gateway): honor effective model for fast mode
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…gress MEDIA: tags whose path had an unknown extension (.py, .log, .toml, .weirdext, ...) fell between both extraction passes: the anchored extension allowlist (MEDIA_TAG_CLEANUP_RE) did not match them, and the extension-less pass explicitly skipped any path that HAD a suffix. The file was never delivered even though the intended design (universal ingress/egress) says any non-credential file should ship. Widen _path_lacks_deliverable_extension() so the validated delivery pass (MEDIA_EXTENSIONLESS_TAG_RE + validate_media_delivery_path) covers every path the extension allowlist does not — unknown extensions and extension-less files alike. Security posture is unchanged: unknown-extension paths only deliver after full validation (exists, symlinks resolved, credential/system denylist, strict-mode allowlist+recency), and unvalidated tags stay visible in the text instead of being silently dropped. Known extensions keep their unconditional pre-existing behavior. Because extract_media, _strip_media_tag_directives (non-streaming dispatch), and strip_media_directives_for_display (streaming) all share the same two regexes + predicate, all delivery paths pick up the widened behavior with no per-site changes. Dispatch partition in gateway/run.py already routes non-image/video extensions through send_document. Closes the gap reported in PR NousResearch#36060; supersedes the allowlist-append approach there (an extension allowlist can never enumerate every file type a user asks the agent to produce). Co-authored-by: Randimt <randimt@users.noreply.github.com>
randlee
pushed a commit
to randlee/hermes-agent
that referenced
this pull request
Aug 11, 2026
…gress MEDIA: tags whose path had an unknown extension (.py, .log, .toml, .weirdext, ...) fell between both extraction passes: the anchored extension allowlist (MEDIA_TAG_CLEANUP_RE) did not match them, and the extension-less pass explicitly skipped any path that HAD a suffix. The file was never delivered even though the intended design (universal ingress/egress) says any non-credential file should ship. Widen _path_lacks_deliverable_extension() so the validated delivery pass (MEDIA_EXTENSIONLESS_TAG_RE + validate_media_delivery_path) covers every path the extension allowlist does not — unknown extensions and extension-less files alike. Security posture is unchanged: unknown-extension paths only deliver after full validation (exists, symlinks resolved, credential/system denylist, strict-mode allowlist+recency), and unvalidated tags stay visible in the text instead of being silently dropped. Known extensions keep their unconditional pre-existing behavior. Because extract_media, _strip_media_tag_directives (non-streaming dispatch), and strip_media_directives_for_display (streaming) all share the same two regexes + predicate, all delivery paths pick up the widened behavior with no per-site changes. Dispatch partition in gateway/run.py already routes non-image/video extensions through send_document. Closes the gap reported in PR NousResearch#36060; supersedes the allowlist-append approach there (an extension allowlist can never enumerate every file type a user asks the agent to produce). Co-authored-by: Randimt <randimt@users.noreply.github.com>
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
4 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
The
MEDIA:extension allowlist (MEDIA_DELIVERY_EXTS, introduced by #34844 / #34517) covered docs, spreadsheets, archives, and rendered web output but not source code, build manifests, or logs. A response ofMEDIA:/path/to/script.pywas silently dropped —extract_media()didn't match it,MEDIA_TAG_CLEANUP_REleft the tag in the body, and the file never reached the user. Same story for.ts/.tsx,.go,.rs,.sh,.sol,.toml,.log, etc. — exactly the file types most likely to be shared in a chat workflow (code review, snippet sharing, log triage).This is a follow-up to #34844 that closes the remaining gap.
Changes
Extend
MEDIA_DELIVERY_EXTSwith source code (Python / JS / TS / shell / systems / JVM / scripting / Solidity / web), build metadata (.toml,.css), and.log.Sensitive single-purpose configs (
.env,.conf,.ini) are deliberately excluded. The credential-path denylist in_media_delivery_denied_pathsis the runtime backstop, but keeping these out of the extension allowlist is the cheaper, earlier layer of defence — a malicious prompt-injection that emitsMEDIA:~/some/.envfrom a non-denied location is now ignored at the extractor instead of relying solely on the path check.Replace two duplicated extension regexes in
gateway/run.py(history scan + post-run scan) with imports of_MEDIA_EXT_ALTERNATIONfromgateway/platforms/base.py. They had been drifting from the source of truth that fix(gateway): unify MEDIA: extraction extension set + close the unknown-ext black hole (#34517) #34844 introduced; pulling them through the shared constant closes the gap and means future extension additions only need to land in one place.Test plan
Two new regression tests in
tests/gateway/test_platform_base.py::TestSourceCodeMediaDelivery:MEDIA:tag..env/.conf/.inistay out of the allowlist (anti-regression for the credential-leak guardrail).Adjacent gateway tests (
test_send_image_file.py,test_dingtalk.py) also still pass — no behaviour change on the existing allowed types.Manual verification
Tested live on a private Discord-connected gateway: a reply containing
MEDIA:/home/user/snippet.pynow delivers as a native file attachment. Before the patch, the same reply showed only the surrounding prose with the path silently stripped.