fix(wecom): URL-decode aeskey and store inbound images with an image MIME
Review findings on #109139 (the "aeskey half is already on main" claim was wrong): `_decrypt_file_bytes` restored base64 padding but the key was never percent-decoded, so `%2F`/`%3D` keys failed to decrypt and `_cache_media` returned None. And `_store_media` received the CDN's `application/octet-stream` as the image MIME, which the gateway image classifier rejects even with the corrected `.png` name. Decode the key once at the payload boundary; for images forward the response MIME only when it is `image/*`, otherwise derive it from the resolved extension. Covered by one end-to-end test through `_cache_media` (red on the previous head).
This commit is contained in:
@@ -94,7 +94,8 @@ class WeComMediaMixin:
|
||||
url = str(media.get("url") or "").strip()
|
||||
if not url:
|
||||
return None
|
||||
aes_key = str(media.get("aeskey") or "").strip()
|
||||
# The key arrives URL-encoded in some payloads (%2F, %3D); base64-decode the real value.
|
||||
aes_key = unquote(str(media.get("aeskey") or "").strip())
|
||||
try:
|
||||
step = "download"
|
||||
raw, headers = await self._download_remote_bytes(url, max_bytes=ABSOLUTE_MAX_BYTES)
|
||||
@@ -105,7 +106,10 @@ class WeComMediaMixin:
|
||||
return None
|
||||
content_type = str(headers.get("content-type") or "").split(";", 1)[0].strip() or "application/octet-stream"
|
||||
ext = self._guess_extension(url, content_type, fallback=self._detect_image_ext(raw))
|
||||
return await self._store_media(kind, raw, ext, content_type, self._guess_filename(url, headers.get("content-disposition"), content_type), content_type, f" from {url}")
|
||||
# Images: never forward the CDN's generic octet-stream label — downstream classifiers
|
||||
# reject non-image MIMEs; derive it from the resolved extension instead.
|
||||
image_mime = content_type if content_type.startswith("image/") else ""
|
||||
return await self._store_media(kind, raw, ext, image_mime, self._guess_filename(url, headers.get("content-disposition"), content_type), content_type, f" from {url}")
|
||||
|
||||
async def _store_media(self, kind, raw, ext, image_mime, filename, doc_mime, origin) -> Optional[Tuple[str, str]]:
|
||||
"""Cache bytes as an image (``kind == "image"``) or a document; returns (path, mime)."""
|
||||
|
||||
@@ -43,6 +43,39 @@ class TestWeComInboundImageExtension:
|
||||
assert ext == ".jpg"
|
||||
assert WeComAdapter._guess_extension("https://x/y.png", "image/png", fallback=".jpg") == ".png"
|
||||
|
||||
def test_encoded_aeskey_and_octet_stream_image_cached_as_real_image(self, monkeypatch):
|
||||
"""End to end through `_cache_media`: a percent-encoded, unpadded `aeskey` decrypts, and an
|
||||
octet-stream-labelled PNG is stored with an image MIME, not application/octet-stream."""
|
||||
from urllib.parse import quote
|
||||
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
|
||||
from plugins.platforms.wecom import media as wecom_media
|
||||
from plugins.platforms.wecom.adapter import WeComAdapter
|
||||
|
||||
key = os.urandom(32)
|
||||
png = b"\x89PNG\r\n\x1a\n" + b"\x00" * 40
|
||||
pad = 16 - len(png) % 16
|
||||
enc = Cipher(algorithms.AES(key), modes.CBC(key[:16])).encryptor()
|
||||
encrypted = enc.update(png + bytes([pad]) * pad) + enc.finalize()
|
||||
encoded_key = quote(base64.b64encode(key).decode().rstrip("="), safe="")
|
||||
|
||||
adapter = WeComAdapter.__new__(WeComAdapter)
|
||||
stored = {}
|
||||
|
||||
async def _download(url, max_bytes):
|
||||
return encrypted, {"content-type": "application/octet-stream"}
|
||||
|
||||
async def _cache(raw, ext):
|
||||
stored["raw"] = raw
|
||||
return f"/tmp/img{ext}"
|
||||
|
||||
monkeypatch.setattr(adapter, "_download_remote_bytes", _download)
|
||||
monkeypatch.setattr(wecom_media, "cache_image_from_bytes_async", _cache)
|
||||
|
||||
result = asyncio.run(adapter._cache_media("image", {"url": "https://cdn/x", "aeskey": encoded_key}))
|
||||
|
||||
assert result == ("/tmp/img.png", "image/png")
|
||||
assert stored["raw"] == png
|
||||
|
||||
|
||||
class TestWeComAdapterAuthzScope:
|
||||
"""dm_policy/allowlist reads must honor the profile secret scope under
|
||||
|
||||
Reference in New Issue
Block a user