fix: add .ics, .vcf, .vcs to MEDIA_DELIVERY_EXTS and gateway regex - #43871
fix: add .ics, .vcf, .vcs to MEDIA_DELIVERY_EXTS and gateway regex#43871sxhorschi wants to merge 1 commit into
Conversation
Calendar (.ics) and contact (.vcf, .vcs) files were missing from the allowed file extensions list, causing MEDIA: tags with these extensions to be silently ignored and never delivered as attachments. Fixes two locations: - gateway/platforms/base.py: MEDIA_DELIVERY_EXTS tuple (source of truth) - gateway/run.py: _TOOL_MEDIA_RE regex (mirrors base.py for tool results) Discovered when trying to send a .ics calendar file via Slack.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good
- Small file extension addition: Adds .ics, .vcf, .vcs to MEDIA_DELIVERY_EXTS and gateway regex.
- Correct file type handling: These are standard calendar/contact file formats that should be delivered as media.
- No security concerns: Simple file extension list addition.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the shared gateway extraction path. The reported gap is still present on current main: MEDIA_DELIVERY_EXTS at gateway/platforms/base.py:1455-1472 omits all three extensions, and _TOOL_MEDIA_RE at gateway/run.py:1061-1066 also stops at apk|ipa.
Problems
- The changed
_TOOL_MEDIA_REline remains a second hardcoded allowlist. It already differs fromMEDIA_DELIVERY_EXTS(for example, current main allows.md,.json,.yaml,.svg, and.htmlingateway/platforms/base.py:1455-1472, but the tool regex does not). This preserves the drift class that the shared source-of-truth refactor in781604ce4addressed for the primary extraction paths. - The PR adds no regression tests for the three new extensions.
Suggested changes
- Derive the tool-result matcher from the shared delivery set, or add parity coverage.
- Add focused extraction/matcher tests and reconcile the overlapping
.icsMIME/test work in open PR #38455.
This is an automated hermes-sweeper review.
| r'mp4|mov|avi|mkv|webm|ogg|opus|mp3|wav|m4a|' | ||
| r'flac|epub|pdf|zip|rar|7z|docx?|xlsx?|pptx?|' | ||
| r'txt|csv|apk|ipa))', | ||
| r'txt|csv|apk|ipa|ics|vcf|vcs))', |
There was a problem hiding this comment.
This remains a separate hardcoded list from MEDIA_DELIVERY_EXTS. Current main already has delivery extensions absent from this matcher; please derive this matcher from the shared set or add a parity invariant so another extension addition cannot drift again.
|
Closing as resolved on main: PR #65510 replaced the extension-allowlist approach with universal validated egress — .ics/.vcf/.vcs (and any other extension) now deliver via MEDIA: after passing validate_media_delivery_path. Verified live on main for all three. Thanks for the contribution! |
Problem
Calendar files (.ics) and contact files (.vcf, .vcs) were missing from the allowed file extensions list in
MEDIA_DELIVERY_EXTS. This causedMEDIA:/path/to/file.icstags in agent responses to be silently ignored and never delivered as file attachments on any platform.Fix
Added
.ics,.vcf, and.vcsto two locations:gateway/platforms/base.py:MEDIA_DELIVERY_EXTStuple — the source of truth that drivesMEDIA_TAG_CLEANUP_REgateway/run.py:_TOOL_MEDIA_REregex — mirrors the allowed extensions for tool result scanningHow it was discovered
A user asked Hermes to create an iCalendar (.ics) file with university course dates and send it via Slack. The
MEDIA:tag was emitted correctly in the response but never delivered — after debugging, the root cause was the missing extension inMEDIA_DELIVERY_EXTS.