Skip to content

fix(gateway/weixin): correct aes_key encoding and upload_full_url CDN path - #7531

Closed
longsizhuo wants to merge 4 commits into
NousResearch:mainfrom
longsizhuo:fix/weixin-image-aes-key-encoding
Closed

fix(gateway/weixin): correct aes_key encoding and upload_full_url CDN path#7531
longsizhuo wants to merge 4 commits into
NousResearch:mainfrom
longsizhuo:fix/weixin-image-aes-key-encoding

Conversation

@longsizhuo

@longsizhuo longsizhuo commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes three bugs in the Weixin (iLink Bot) adapter's outbound media pipeline that caused bot-sent images to appear as a grey placeholder in the WeChat client — the image is delivered but cannot be opened or decrypted.

Root cause: image_item.media.aes_key was encoded as base64(raw_16_bytes), but the WeChat client's decode chain is:

base64_decode(aes_key)  →  32 ASCII hex chars  →  bytes.fromhex()  →  16-byte AES key

Sending base64(raw_bytes) produced 16 decoded bytes that couldn't be parsed as hex, so the client derived the wrong key and AES-128-ECB decryption failed silently, leaving the user with an unusable grey image.

Two follow-on bugs in the same _send_file() path were fixed at the same time because they blocked the image from ever reaching the client correctly:

  1. upload_full_url (returned by newer iLink API instances) requires HTTP POST, not PUT. The old code only tried PUT and got 404 from the CDN.
  2. The encrypted_query_param query argument embedded inside upload_full_url is an upload-auth token, not the download key. The real download key is returned by the CDN in the x-encrypted-param response header after a successful upload — the old code used the wrong value, which produced an encrypted reference the client could not resolve.

Related Issue

Fixes #7529

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

All changes are in gateway/platforms/weixin.py:

  • _send_file()aes_key encoding (root cause). Changed base64.b64encode(aes_key).decode("ascii")base64.b64encode(aes_key.hex().encode("ascii")).decode("ascii"), with a comment documenting the client's decode chain.
  • _send_file()upload_full_url CDN method. Replaced the single-PUT call with a POST-first, PUT-on-404 fallback loop so both current and legacy CDN endpoints work.
  • _send_file()encrypted_query_param source. The download key is now taken from the x-encrypted-param response header returned by the CDN after the ciphertext upload, not from the query string of upload_full_url. Falls back to filekey if the header is missing. A clear RuntimeError is raised if both methods fail.
  • send_image_file() — parameter name. Renamed pathimage_path to match the base adapter signature (gateway/platforms/base.py:1058). Accepts legacy path= kwarg for backwards compatibility, and returns SendResult(success=False, error=...) if neither is provided instead of letting Path("").read_bytes() raise IsADirectoryError deep in the stack.
  • _outbound_media_builder() — removed unused aes_key_hex kwarg plumbing (review feedback: it was passed to item_builder(...) but none of the lambdas read it).

How to Test

Reproducing the original bug (before this PR):

  1. Configure a Weixin account (hermes gateway setup, scan QR, confirm login).
  2. Start the gateway: hermes gateway.
  3. DM the bot from the WeChat mobile client and ask it to send an image (e.g., 拍一张照片 or any prompt that triggers a local image file response).
  4. Observe: the bot's reply contains an image bubble that renders as an opaque grey square; tapping it shows no preview.

Verifying the fix (with this PR):

  1. Repeat steps 1–3 on the branch.
  2. The image now renders correctly in the WeChat mobile client — tap-to-preview works, the full photo loads, and long-press / save works as expected.

Code-level sanity check of the aes_key encoding fix:

import base64, secrets
aes_key = secrets.token_bytes(16)
aes_key_b64 = base64.b64encode(aes_key.hex().encode("ascii")).decode("ascii")

# Simulate the WeChat client's decode chain:
decoded = base64.b64decode(aes_key_b64)       # 32 bytes (ASCII hex chars)
assert len(decoded) == 32
recovered = bytes.fromhex(decoded.decode("ascii"))
assert recovered == aes_key                   # Round-trip matches

The old base64.b64encode(aes_key).decode("ascii") produces 16 decoded bytes that are not valid ASCII hex, so bytes.fromhex(...) on the client side fails to recover the key.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(gateway/weixin): ...)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (the adapter's outbound media path)
  • I've run pytest tests/ -q and all tests pass — not yet re-run after the latest revert; will re-run before merge
  • I've added tests for my changes — not yet; planning to add unit tests mocking aiohttp.ClientSession to cover the POSTPUT fallback, the x-encrypted-param header read, and the base64(hex) aes_key encoding in a follow-up commit on this branch
  • I've tested on my platform: Ubuntu (Linux 6.8), against a live iLink Bot API account and the WeChat mobile client

Documentation & Housekeeping

  • I've updated relevant documentation — N/A (no public API change; website/docs/user-guide/messaging/weixin.md already describes the encrypted CDN pipeline at the correct level of abstraction)
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (no config changes)
  • I've updated CONTRIBUTING.md or AGENTS.md — N/A (no architectural/workflow change)
  • I've considered cross-platform impact — N/A (pure network/protocol code; no file-system, process, or terminal handling)
  • I've updated tool descriptions/schemas — N/A (no tool signature change; send_image_file(image_path=...) now matches the base adapter signature, which is a fix, not a breaking change)

Screenshots / Logs

Before: image bubble renders as a grey placeholder in the WeChat mobile client; tap-to-preview does nothing.
image

After: image renders correctly, preview and long-press actions work as expected.

Screenshots showing the before/after can be added on request; they contain personal chat context so they are not attached inline.

… path

Three bugs in the outbound media pipeline that caused images sent by the bot
to appear as a grey placeholder in the WeChat client:

1. aes_key encoding in image_item (root cause of grey box)
   The WeChat client expects aes_key as base64(hex_string_bytes), not
   base64(raw_bytes). Its decode chain is:
     base64_decode → 32 ASCII hex chars → fromhex() → 16-byte AES key
   Sending base64(raw_bytes) yielded 16 decoded bytes that the client
   couldn't interpret as hex, resulting in the wrong key and failed
   AES-128-ECB decryption.

   Before: base64.b64encode(aes_key)
   After:  base64.b64encode(aes_key.hex().encode("ascii"))

2. upload_full_url CDN path used PUT instead of POST
   Newer iLink API instances return upload_full_url instead of upload_param.
   The CDN endpoint behind upload_full_url returns 404 on PUT; POST succeeds.
   Fixed by trying POST first and falling back to PUT on 404.

3. Wrong encrypted_query_param source for upload_full_url path
   The encrypted_query_param embedded in upload_full_url is an upload-auth
   token, not the download key. The correct download key is the
   x-encrypted-param header returned by the CDN after a successful upload.

Also fix send_image_file() parameter name: the method was called with
image_path= but the parameter was declared as path=, causing a TypeError.

Also fix run_agent.py to read max_tokens from custom_providers per-model
config, so models configured under custom_providers can set their own
output token limit.

Fixes NousResearch#7529
Copilot AI review requested due to automatic review settings April 11, 2026 03:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Weixin/iLink outbound media uploads so bot-sent images decrypt/display correctly in WeChat, and extends agent configuration to support per-model max_tokens under custom_providers.

Changes:

  • Correct Weixin outbound media message construction (AES key encoding) and improve upload_full_url upload behavior (POST-first with PUT fallback; use CDN header for download key).
  • Fix Weixin adapter send_image_file() to accept image_path and legacy path.
  • Read max_tokens from custom_providers per-model config in run_agent.py.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
gateway/platforms/weixin.py Fixes outbound media upload path and AES key encoding for WeChat client compatibility; updates image send API surface.
run_agent.py Adds support for per-model max_tokens under custom_providers during agent initialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread run_agent.py Outdated
Comment thread gateway/platforms/weixin.py
Comment thread gateway/platforms/weixin.py Outdated
Comment thread gateway/platforms/weixin.py
…edback

- run_agent: decouple per-model max_tokens lookup from context_length
  guard so an explicit context_length no longer suppresses max_tokens
- gateway/weixin: return a clear SendResult error when send_image_file
  is called without image_path/path instead of failing later with
  IsADirectoryError on Path("").read_bytes()
- gateway/weixin: drop unused aes_key_hex kwarg from item_builder call
  (none of the media builder lambdas read it)
@poorld

poorld commented Apr 11, 2026

Copy link
Copy Markdown

good

The per-model max_tokens lookup in run_agent.py is unrelated to the
Weixin image fix. Restoring it to main's state so this PR stays focused
per CONTRIBUTING.md ("One logical change per PR").
@moonaries90

Copy link
Copy Markdown

Great catch on the aes_key encoding — the round-trip test in the PR description was exactly the smoking gun I was chasing in my own debug pass last night.

One thing I ran into while testing this locally: the same signature mismatch exists in send_document (not just send_image_file), so non-image file sends still fail on main even with the fixes in this PR.

Repro: ask the bot to send a .md / .txt / .pdf. The gateway's auto-forward code in base.py:1677 calls:

media_result = await self.send_document(
    chat_id=event.source.chat_id,
    file_path=media_path,
    metadata=_thread_metadata,
)

but WeixinAdapter.send_document declares the argument as path, so it raises:

TypeError: WeixinAdapter.send_document() got an unexpected keyword argument 'file_path'

The minimal fix mirrors what you already did for send_image_file:

async def send_document(
    self,
    chat_id: str,
    path: Optional[str] = None,
    caption: str = "",
    file_path: Optional[str] = None,
    metadata: Optional[Dict[str, Any]] = None,
) -> SendResult:
    if not self._session or not self._token:
        return SendResult(success=False, error="Not connected")
    target = path if path is not None else file_path
    if target is None:
        return SendResult(
            success=False,
            error="send_document requires 'file_path' (or legacy 'path')",
        )
    try:
        message_id = await self._send_file(chat_id, target, caption)
        return SendResult(success=True, message_id=message_id)
    except Exception as exc:
        logger.error(
            "[%s] send_document failed to=%s: %s",
            self.name, _safe_id(chat_id), exc,
        )
        return SendResult(success=False, error=str(exc))

Happy to either fold this into your PR or open a small follow-up after this one lands — whichever is easier for the maintainers.

The gateway's auto-forward code at gateway/platforms/base.py:1676 calls
send_document with `file_path=` kwarg, but WeixinAdapter.send_document
declared the argument as `path`, so any non-image/non-video file send
through the auto-forward path raised:

    TypeError: WeixinAdapter.send_document() got an unexpected keyword
    argument 'file_path'

Rename the primary argument to `file_path` to match the base class
(and every other platform adapter). Also accept `file_name`, `reply_to`,
and trailing `**kwargs` for full base-class parity. A legacy `path=`
kwarg is still honored through `**kwargs`, and empty calls now return
a clear SendResult error instead of ambiguously failing inside
`_send_file`.

Co-authored-by: moonaries90 <53324877+moonaries90@users.noreply.github.com>
@longsizhuo

Copy link
Copy Markdown
Contributor Author

Thanks @moonaries90 — nice catch. Verified locally that WeixinAdapter.send_document does raise TypeError: got an unexpected keyword argument 'file_path' when called via the auto-forward path in gateway/platforms/base.py:1676, and that every other platform adapter (base + 12 others) already uses file_path. So this is a strict Weixin-side inconsistency.

Folded the fix into this PR in ee3f32d1, with you as co-author. I took the signature a bit further than your snippet to fully match the base class — the new signature accepts file_path, file_name, reply_to, metadata, and trailing **kwargs, while still honoring a legacy path= kwarg for backwards compat. Empty calls now return a clear SendResult error instead of blowing up inside _send_file.

Checked that no internal caller passes path= as a kwarg (only positional or file_path=), so the rename is safe.

@bennywan

Copy link
Copy Markdown

Validated this against a real Weixin/iLink personal-account setup and a real WeChat client.

I reproduced three separate outbound attachment problems on current main:

  1. WeixinAdapter.send_document / send_image_file signature mismatch with the base gateway call sites

    • generic media delivery passes file_path / image_path
    • Weixin adapter still accepted path
    • this caused:
      TypeError: WeixinAdapter.send_document() got an unexpected keyword argument 'file_path'
  2. upload_full_url branch used the wrong HTTP method

    • PUT returned 404 from the CDN
    • POST succeeded
  3. aes_key encoding in outbound media payload was not compatible with the WeChat client

    • after fixing send/signature + upload method, the file bubble could be delivered
    • but the attachment stayed stuck downloading / could not open
    • after switching aes_key to base64(hex-string-bytes), download and open both worked correctly

After applying those fixes together, I verified end-to-end that a bot-sent document attachment:

  • is delivered to WeChat
  • downloads successfully
  • opens successfully in the client

So from a real-user validation standpoint, this PR fixes not only grey image / media issues, but also practical document attachment usability on Weixin.

@longsizhuo

longsizhuo commented Apr 12, 2026

Copy link
Copy Markdown
Contributor Author

@teknium1 Hey, just a heads-up on this one:

  1. CI is waiting on first-time contributor approval: the 3 workflows need a manual approve before they can run. Would appreciate it if you could unblock those when you get a chance.

  2. Independent validation: @bennywan tested end-to-end on a real iLink personal account and confirmed all three fixes work (signature mismatch, POST vs PUT, aes_key encoding). See his comment above.

  3. Related fix(weixin): update parameter names for image and document senders #8144: looks like @Astral-Yang also opened a PR addressing the parameter naming issue, which shows this is a real pain point for Weixin users. There's some overlap, happy to coordinate however works best for you.

No rush, just flagging so it doesn't get buried. Thanks!

teknium1 added a commit that referenced this pull request Apr 12, 2026
…essages

Four fixes for the Weixin/WeChat adapter, synthesized from the best
aspects of community PRs #8407, #8521, #8360, #7695, #8308, #8525,
#7531, #8144, #8251.

1. Streaming cursor (▉) stuck permanently — WeChat doesn't support
   message editing, so the cursor appended during streaming can never
   be removed.  Add SUPPORTS_MESSAGE_EDITING = False to WeixinAdapter
   and check it in gateway/run.py to use an empty cursor for non-edit
   platforms.  (Fixes #8307, #8326)

2. Media upload failures — two bugs in _send_file():
   a) upload_full_url path used PUT (404 on WeChat CDN); now uses POST.
   b) aes_key was base64(raw_bytes) but the iLink API expects
      base64(hex_string); images showed as grey boxes.  (Fixes #8352, #7529)
   Also: unified both upload paths into _upload_ciphertext(), preferring
   upload_full_url.  Added send_video/send_voice methods and voice_item
   media builder for audio/.silk files.  Added video_md5 field.

3. Markdown links stripped — WeChat can't render [text](url), so
   format_message() now converts them to 'text (url)' plaintext.
   Code blocks are preserved.  (Fixes #7617)

4. Blank message prevention — three guards:
   a) _split_text_for_weixin_delivery('') returns [] not ['']
   b) send() filters empty/whitespace chunks before _send_text_chunk
   c) _send_message() raises ValueError for empty text as safety net

Community credit: joei4cm (#8407), lyonDan (#8521), SKFDJKLDG (#8360),
tomqiaozc (#7695), joshleeeeee (#8308), luoxiao6645(#8525),
longsizhuo (#7531), Astral-Yang (#8144), QingWei-Li (#8251).
teknium1 added a commit that referenced this pull request Apr 12, 2026
…essages (#8665)

Four fixes for the Weixin/WeChat adapter, synthesized from the best
aspects of community PRs #8407, #8521, #8360, #7695, #8308, #8525,
#7531, #8144, #8251.

1. Streaming cursor (▉) stuck permanently — WeChat doesn't support
   message editing, so the cursor appended during streaming can never
   be removed.  Add SUPPORTS_MESSAGE_EDITING = False to WeixinAdapter
   and check it in gateway/run.py to use an empty cursor for non-edit
   platforms.  (Fixes #8307, #8326)

2. Media upload failures — two bugs in _send_file():
   a) upload_full_url path used PUT (404 on WeChat CDN); now uses POST.
   b) aes_key was base64(raw_bytes) but the iLink API expects
      base64(hex_string); images showed as grey boxes.  (Fixes #8352, #7529)
   Also: unified both upload paths into _upload_ciphertext(), preferring
   upload_full_url.  Added send_video/send_voice methods and voice_item
   media builder for audio/.silk files.  Added video_md5 field.

3. Markdown links stripped — WeChat can't render [text](url), so
   format_message() now converts them to 'text (url)' plaintext.
   Code blocks are preserved.  (Fixes #7617)

4. Blank message prevention — three guards:
   a) _split_text_for_weixin_delivery('') returns [] not ['']
   b) send() filters empty/whitespace chunks before _send_text_chunk
   c) _send_message() raises ValueError for empty text as safety net

Community credit: joei4cm (#8407), lyonDan (#8521), SKFDJKLDG (#8360),
tomqiaozc (#7695), joshleeeeee (#8308), luoxiao6645(#8525),
longsizhuo (#7531), Astral-Yang (#8144), QingWei-Li (#8251).
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #8665 which synthesizes the best fixes from ~25 community PRs into a single consolidated change. Your contribution (aes_key encoding + upload_full_url CDN path fix) was reviewed and informed the final implementation. Thank you @longsizhuo for your work on this!

@teknium1 teknium1 closed this Apr 12, 2026
aj-nt pushed a commit to aj-nt/hermes-agent that referenced this pull request May 1, 2026
…essages (NousResearch#8665)

Four fixes for the Weixin/WeChat adapter, synthesized from the best
aspects of community PRs NousResearch#8407, NousResearch#8521, NousResearch#8360, NousResearch#7695, NousResearch#8308, NousResearch#8525,
NousResearch#7531, NousResearch#8144, NousResearch#8251.

1. Streaming cursor (▉) stuck permanently — WeChat doesn't support
   message editing, so the cursor appended during streaming can never
   be removed.  Add SUPPORTS_MESSAGE_EDITING = False to WeixinAdapter
   and check it in gateway/run.py to use an empty cursor for non-edit
   platforms.  (Fixes NousResearch#8307, NousResearch#8326)

2. Media upload failures — two bugs in _send_file():
   a) upload_full_url path used PUT (404 on WeChat CDN); now uses POST.
   b) aes_key was base64(raw_bytes) but the iLink API expects
      base64(hex_string); images showed as grey boxes.  (Fixes NousResearch#8352, NousResearch#7529)
   Also: unified both upload paths into _upload_ciphertext(), preferring
   upload_full_url.  Added send_video/send_voice methods and voice_item
   media builder for audio/.silk files.  Added video_md5 field.

3. Markdown links stripped — WeChat can't render [text](url), so
   format_message() now converts them to 'text (url)' plaintext.
   Code blocks are preserved.  (Fixes NousResearch#7617)

4. Blank message prevention — three guards:
   a) _split_text_for_weixin_delivery('') returns [] not ['']
   b) send() filters empty/whitespace chunks before _send_text_chunk
   c) _send_message() raises ValueError for empty text as safety net

Community credit: joei4cm (NousResearch#8407), lyonDan (NousResearch#8521), SKFDJKLDG (NousResearch#8360),
tomqiaozc (NousResearch#7695), joshleeeeee (NousResearch#8308), luoxiao6645(NousResearch#8525),
longsizhuo (NousResearch#7531), Astral-Yang (NousResearch#8144), QingWei-Li (NousResearch#8251).
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…essages (NousResearch#8665)

Four fixes for the Weixin/WeChat adapter, synthesized from the best
aspects of community PRs NousResearch#8407, NousResearch#8521, NousResearch#8360, NousResearch#7695, NousResearch#8308, NousResearch#8525,
NousResearch#7531, NousResearch#8144, NousResearch#8251.

1. Streaming cursor (▉) stuck permanently — WeChat doesn't support
   message editing, so the cursor appended during streaming can never
   be removed.  Add SUPPORTS_MESSAGE_EDITING = False to WeixinAdapter
   and check it in gateway/run.py to use an empty cursor for non-edit
   platforms.  (Fixes NousResearch#8307, NousResearch#8326)

2. Media upload failures — two bugs in _send_file():
   a) upload_full_url path used PUT (404 on WeChat CDN); now uses POST.
   b) aes_key was base64(raw_bytes) but the iLink API expects
      base64(hex_string); images showed as grey boxes.  (Fixes NousResearch#8352, NousResearch#7529)
   Also: unified both upload paths into _upload_ciphertext(), preferring
   upload_full_url.  Added send_video/send_voice methods and voice_item
   media builder for audio/.silk files.  Added video_md5 field.

3. Markdown links stripped — WeChat can't render [text](url), so
   format_message() now converts them to 'text (url)' plaintext.
   Code blocks are preserved.  (Fixes NousResearch#7617)

4. Blank message prevention — three guards:
   a) _split_text_for_weixin_delivery('') returns [] not ['']
   b) send() filters empty/whitespace chunks before _send_text_chunk
   c) _send_message() raises ValueError for empty text as safety net

Community credit: joei4cm (NousResearch#8407), lyonDan (NousResearch#8521), SKFDJKLDG (NousResearch#8360),
tomqiaozc (NousResearch#7695), joshleeeeee (NousResearch#8308), luoxiao6645(NousResearch#8525),
longsizhuo (NousResearch#7531), Astral-Yang (NousResearch#8144), QingWei-Li (NousResearch#8251).
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…essages (NousResearch#8665)

Four fixes for the Weixin/WeChat adapter, synthesized from the best
aspects of community PRs NousResearch#8407, NousResearch#8521, NousResearch#8360, NousResearch#7695, NousResearch#8308, NousResearch#8525,
NousResearch#7531, NousResearch#8144, NousResearch#8251.

1. Streaming cursor (▉) stuck permanently — WeChat doesn't support
   message editing, so the cursor appended during streaming can never
   be removed.  Add SUPPORTS_MESSAGE_EDITING = False to WeixinAdapter
   and check it in gateway/run.py to use an empty cursor for non-edit
   platforms.  (Fixes NousResearch#8307, NousResearch#8326)

2. Media upload failures — two bugs in _send_file():
   a) upload_full_url path used PUT (404 on WeChat CDN); now uses POST.
   b) aes_key was base64(raw_bytes) but the iLink API expects
      base64(hex_string); images showed as grey boxes.  (Fixes NousResearch#8352, NousResearch#7529)
   Also: unified both upload paths into _upload_ciphertext(), preferring
   upload_full_url.  Added send_video/send_voice methods and voice_item
   media builder for audio/.silk files.  Added video_md5 field.

3. Markdown links stripped — WeChat can't render [text](url), so
   format_message() now converts them to 'text (url)' plaintext.
   Code blocks are preserved.  (Fixes NousResearch#7617)

4. Blank message prevention — three guards:
   a) _split_text_for_weixin_delivery('') returns [] not ['']
   b) send() filters empty/whitespace chunks before _send_text_chunk
   c) _send_message() raises ValueError for empty text as safety net

Community credit: joei4cm (NousResearch#8407), lyonDan (NousResearch#8521), SKFDJKLDG (NousResearch#8360),
tomqiaozc (NousResearch#7695), joshleeeeee (NousResearch#8308), luoxiao6645(NousResearch#8525),
longsizhuo (NousResearch#7531), Astral-Yang (NousResearch#8144), QingWei-Li (NousResearch#8251).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(weixin): outbound images show as grey box — aes_key must be base64(hex) not base64(raw)

6 participants