From 9149336b7b613666f8b920ad55bf6bd355aecfe2 Mon Sep 17 00:00:00 2001 From: Maidong <408097061@qq.com> Date: Sat, 2 May 2026 00:10:01 +0800 Subject: [PATCH 1/4] feat(weixin): add image sending support via CDN upload --- packages/channels/weixin/src/WeixinAdapter.ts | 86 ++++++++++-- packages/channels/weixin/src/api.ts | 98 +++++++++++++ packages/channels/weixin/src/media.test.ts | 51 +++++++ packages/channels/weixin/src/media.ts | 15 +- packages/channels/weixin/src/send.test.ts | 132 +++++++++++++++++- packages/channels/weixin/src/send.ts | 79 ++++++++++- 6 files changed, 448 insertions(+), 13 deletions(-) diff --git a/packages/channels/weixin/src/WeixinAdapter.ts b/packages/channels/weixin/src/WeixinAdapter.ts index 561a366f415..a357454c93c 100644 --- a/packages/channels/weixin/src/WeixinAdapter.ts +++ b/packages/channels/weixin/src/WeixinAdapter.ts @@ -17,7 +17,7 @@ import type { import { loadAccount, DEFAULT_BASE_URL } from './accounts.js'; import { startPollLoop, getContextToken } from './monitor.js'; import type { CdnRef, FileCdnRef } from './monitor.js'; -import { sendText } from './send.js'; +import { sendText, sendImage } from './send.js'; import { downloadAndDecrypt } from './media.js'; import { getConfig, sendTyping } from './api.js'; import { TypingStatus } from './types.js'; @@ -43,6 +43,22 @@ export class WeixinChannel extends ChannelBase { } async connect(): Promise { + // Default channel instructions: tell the AI it can send images via WeChat + if (!this.config.instructions) { + this.config.instructions = [ + '## WeChat Channel Capabilities', + '', + 'You are communicating with users through WeChat. You CAN send images to the user.', + 'To send an image, include it in your response using this EXACT format:', + '[IMAGE: 图片文件的完整绝对路径]', + '', + 'Example: 如果你想发送 /tmp/cat.png 给用户, 在回复中写 [IMAGE: /tmp/cat.png]', + 'This marker will be automatically removed from the text and the image will be uploaded and sent.', + 'You can include multiple [IMAGE: ...] markers in one response.', + '', + 'Users can also send you images, which you can see and analyze.', + ].join('\n'); + } const account = loadAccount(); if (!account) { throw new Error( @@ -153,18 +169,72 @@ export class WeixinChannel extends ChannelBase { } } + // Always remind the AI about image-sending capability on every message + const IMAGE_INSTRUCTION = + '[WeChat Channel] 你可以通过微信发送图片。在回复中使用 [IMAGE: 文件绝对路径] 发送图片,例如 [IMAGE: /tmp/cat.png]。标记会被自动移除。'; + envelope.text = `${IMAGE_INSTRUCTION}\n\n${envelope.text}`; + await super.handleInbound(envelope); } - async sendMessage(chatId: string, text: string): Promise { + async sendMessage( + chatId: string, + text: string, + imagePaths?: string[], + ): Promise { const contextToken = getContextToken(chatId) || ''; - await sendText({ - to: chatId, - text, - baseUrl: this.baseUrl, - token: this.token, - contextToken, + + // Parse [IMAGE: /path/to/file.png] markers from text + const imageRegex = /\[IMAGE:\s*([^\]]+)\]/gi; + const parsedImages: string[] = []; + let cleanedText = text.replace(imageRegex, (_, path: string) => { + parsedImages.push(path.trim()); + return ''; }); + + // Merge with any imagePaths from the ACP pipeline + const allImages = [...(imagePaths || []), ...parsedImages]; + + // Clean up double blank lines left by removed markers + cleanedText = cleanedText.replace(/\n{3,}/g, '\n\n').trim(); + + // Send text first if non-empty + if (cleanedText) { + await sendText({ + to: chatId, + text: cleanedText, + baseUrl: this.baseUrl, + token: this.token, + contextToken, + }); + } + + // Send images + if (allImages.length) { + for (const imagePath of allImages) { + try { + await sendImage({ + to: chatId, + imagePath, + baseUrl: this.baseUrl, + token: this.token, + contextToken, + }); + } catch (err) { + const errMsg = err instanceof Error ? err.message : String(err); + process.stderr.write( + `[Weixin:${this.name}] Failed to send image ${imagePath}: ${errMsg}\n`, + ); + await sendText({ + to: chatId, + text: `图片发送失败: ${errMsg}`, + baseUrl: this.baseUrl, + token: this.token, + contextToken, + }); + } + } + } } disconnect(): void { diff --git a/packages/channels/weixin/src/api.ts b/packages/channels/weixin/src/api.ts index 93ccbf4b6d4..3148e11e197 100644 --- a/packages/channels/weixin/src/api.ts +++ b/packages/channels/weixin/src/api.ts @@ -141,3 +141,101 @@ export async function sendTyping( const body: SendTypingReq = { ...req, base_info: baseInfo() }; return post(baseUrl, '/ilink/bot/sendtyping', body, token); } + +interface GetUploadUrlReq { + filekey: string; + media_type: number; + to_user_id: string; + rawsize: number; + rawfilemd5: string; + filesize: number; + no_need_thumb: boolean; + aeskey: string; + base_info: BaseInfo; +} + +interface GetUploadUrlResp { + ret?: number; + errmsg?: string; + upload_full_url?: string; + upload_param?: string; + thumb_upload_param?: string; +} + +/** + * Request an upload URL and CDN credentials for media. + * @param aeskeyHex 16-byte AES key as 32-char hex string (e.g. "00112233445566778899aabbccddeeff") + * @returns Either the full CDN upload URL or the upload_param string + */ +export async function getUploadUrl( + baseUrl: string, + token: string, + toUserId: string, + filekey: string, + rawsize: number, + rawfilemd5: string, + encryptedSize: number, + aeskeyHex: string, +): Promise { + const body: GetUploadUrlReq = { + filekey, + media_type: 1, + to_user_id: toUserId, + rawsize, + rawfilemd5, + filesize: encryptedSize, + no_need_thumb: true, + aeskey: aeskeyHex, + base_info: baseInfo(), + }; + const resp = await post( + baseUrl, + '/ilink/bot/getuploadurl', + body, + token, + ); + + // upload_full_url: CDN upload URL with all params embedded + if (resp.upload_full_url) { + return resp.upload_full_url; + } + + // upload_param: CDN upload params only (must construct URL with filekey) + if (resp.upload_param) { + return resp.upload_param; + } + + throw new Error( + `getuploadurl failed: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`, + ); +} + +/** Upload encrypted media to CDN. + * If urlOrParam is a full URL, use it directly. + * If it's just a param, construct the URL. */ +export async function uploadToCdn( + urlOrParam: string, + filekey: string, + encryptedData: Buffer, +): Promise { + const url = urlOrParam.startsWith('http') + ? urlOrParam + : `https://novac2c.cdn.weixin.qq.com/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`; + + const resp = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/octet-stream' }, + body: encryptedData, + }); + if (!resp.ok) { + throw new Error(`CDN upload failed: HTTP ${resp.status}`); + } + // Extract x-encrypted-param from response header + const encryptParam = resp.headers.get('x-encrypted-param'); + if (!encryptParam) { + throw new Error( + 'CDN upload succeeded but missing x-encrypted-param header', + ); + } + return encryptParam; +} diff --git a/packages/channels/weixin/src/media.test.ts b/packages/channels/weixin/src/media.test.ts index 745c0155497..28b42467a9a 100644 --- a/packages/channels/weixin/src/media.test.ts +++ b/packages/channels/weixin/src/media.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest'; import { createDecipheriv, createCipheriv } from 'node:crypto'; +import { encryptAesEcb, computeMd5 } from './media.js'; /** * Test the AES key parsing and decryption logic used in media.ts. @@ -89,3 +90,53 @@ describe('Weixin media crypto', () => { }); }); }); + +describe('encryptAesEcb', () => { + it('encrypts data deterministically', () => { + const key = Buffer.alloc(16, 0xab); + const plaintext = Buffer.from('test data for encryption'); + const ciphertext1 = encryptAesEcb(plaintext, key); + const ciphertext2 = encryptAesEcb(plaintext, key); + expect(ciphertext1).toEqual(ciphertext2); + }); + + it('encrypts then decrypts round-trip', () => { + const key = Buffer.alloc(16, 0x42); + const plaintext = Buffer.from('Hello, WeChat media upload!'); + + const ciphertext = encryptAesEcb(plaintext, key); + const decipher = createDecipheriv('aes-128-ecb', key, null); + const decrypted = Buffer.concat([ + decipher.update(ciphertext), + decipher.final(), + ]); + expect(decrypted.toString()).toBe(plaintext.toString()); + }); + + it('handles empty plaintext', () => { + const key = Buffer.alloc(16, 0x01); + const ciphertext = encryptAesEcb(Buffer.alloc(0), key); + // ECB with empty input produces empty output (no padding block needed + // when input is exactly 0 bytes — behavior varies by implementation) + // At minimum the result should be decryptable + const decipher = createDecipheriv('aes-128-ecb', key, null); + const decrypted = Buffer.concat([ + decipher.update(ciphertext), + decipher.final(), + ]); + expect(decrypted.length).toBe(0); + }); +}); + +describe('computeMd5', () => { + it('computes expected MD5', () => { + const data = Buffer.from('hello world'); + expect(computeMd5(data)).toBe('5eb63bbbe01eeed093cb22bb8f5acdc3'); + }); + + it('computes MD5 of empty buffer', () => { + expect(computeMd5(Buffer.alloc(0))).toBe( + 'd41d8cd98f00b204e9800998ecf8427e', + ); + }); +}); diff --git a/packages/channels/weixin/src/media.ts b/packages/channels/weixin/src/media.ts index 8cd7fa9ebdf..1407b0fb422 100644 --- a/packages/channels/weixin/src/media.ts +++ b/packages/channels/weixin/src/media.ts @@ -3,7 +3,7 @@ * Ported from cc-weixin/plugins/weixin/src/media.ts (download path only). */ -import { createDecipheriv } from 'node:crypto'; +import { createCipheriv, createDecipheriv, createHash } from 'node:crypto'; const CDN_BASE_URL = 'https://novac2c.cdn.weixin.qq.com/c2c'; @@ -22,7 +22,7 @@ function decryptAesEcb(ciphertext: Buffer, key: Buffer): Buffer { * - base64(raw 16 bytes) → images * - base64(hex string of 16 bytes) → file/voice/video */ -function parseAesKey(aesKeyBase64: string): Buffer { +export function parseAesKey(aesKeyBase64: string): Buffer { const decoded = Buffer.from(aesKeyBase64, 'base64'); if (decoded.length === 16) { return decoded; @@ -54,3 +54,14 @@ export async function downloadAndDecrypt( const keyBuf = parseAesKey(aesKey); return decryptAesEcb(ciphertext, keyBuf); } + +/** AES-128-ECB encryption for CDN upload. */ +export function encryptAesEcb(plaintext: Buffer, key: Buffer): Buffer { + const cipher = createCipheriv('aes-128-ecb', key, null); + return Buffer.concat([cipher.update(plaintext), cipher.final()]); +} + +/** Compute MD5 hash of a buffer, returning hex string. */ +export function computeMd5(data: Buffer): string { + return createHash('md5').update(data).digest('hex'); +} diff --git a/packages/channels/weixin/src/send.test.ts b/packages/channels/weixin/src/send.test.ts index 95152672cff..dea4ae758c1 100644 --- a/packages/channels/weixin/src/send.test.ts +++ b/packages/channels/weixin/src/send.test.ts @@ -1,6 +1,42 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { markdownToPlainText } from './send.js'; +const { + mockReadFileSync, + mockGetUploadUrl, + mockUploadToCdn, + mockSendMessage, + mockRandomBytes, +} = vi.hoisted(() => ({ + mockReadFileSync: vi.fn(), + mockGetUploadUrl: vi.fn(), + mockUploadToCdn: vi.fn(), + mockSendMessage: vi.fn(), + mockRandomBytes: vi.fn((size: number) => Buffer.alloc(size, 0x42)), +})); + +vi.mock('node:fs', () => ({ + readFileSync: mockReadFileSync, +})); + +vi.mock('node:crypto', () => ({ + randomBytes: mockRandomBytes, + randomUUID: () => 'test-uuid', +})); + +vi.mock('./api.js', () => ({ + sendMessage: mockSendMessage, + getUploadUrl: mockGetUploadUrl, + uploadToCdn: mockUploadToCdn, +})); + +vi.mock('./media.js', () => ({ + encryptAesEcb: vi.fn((data: Buffer) => data), + computeMd5: vi.fn(() => 'd41d8cd98f00b204e9800998ecf8427e'), +})); + +const { sendImage } = await import('./send.js'); + describe('markdownToPlainText', () => { it('strips code blocks', () => { const input = '```js\nconst x = 1;\n```'; @@ -80,3 +116,97 @@ describe('markdownToPlainText', () => { expect(result).not.toContain('`'); }); }); + +describe('sendImage', () => { + const defaultParams = { + to: 'user-123', + imagePath: '/tmp/test.png', + baseUrl: 'https://api.example.com', + token: 'token-abc', + contextToken: 'ctx-456', + }; + + const fakeImageData = Buffer.from('fake-image-bytes'); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('completes the four-step upload and send flow', async () => { + mockReadFileSync.mockReturnValue(fakeImageData); + mockGetUploadUrl.mockResolvedValue('upload-param-value'); + mockUploadToCdn.mockResolvedValue('cdn-encrypt-param'); + mockSendMessage.mockResolvedValue(undefined); + + await sendImage(defaultParams); + + // Step 1: read file + expect(mockReadFileSync).toHaveBeenCalledWith('/tmp/test.png'); + + // Step 2: get upload URL called with correct params + const encryptedSize = Math.ceil((fakeImageData.length + 1) / 16) * 16; + const expectedFilekey = '42424242424242424242424242424242'; + const expectedAesKeyHex = '42424242424242424242424242424242'; + expect(mockGetUploadUrl).toHaveBeenCalledWith( + 'https://api.example.com', + 'token-abc', + 'user-123', + expectedFilekey, + fakeImageData.length, + 'd41d8cd98f00b204e9800998ecf8427e', + encryptedSize, + expectedAesKeyHex, + ); + + // Step 3: upload to CDN (uploadToCdn takes urlOrParam, filekey, encryptedData) + expect(mockUploadToCdn).toHaveBeenCalledWith( + 'upload-param-value', + expectedFilekey, + fakeImageData, + ); + + // Step 4: send message with image_item using CDN's x-encrypted-param + const expectedAesKeyBase64 = Buffer.from( + '42424242424242424242424242424242', + 'ascii', + ).toString('base64'); + expect(mockSendMessage).toHaveBeenCalledWith( + 'https://api.example.com', + 'token-abc', + expect.objectContaining({ + to_user_id: 'user-123', + context_token: 'ctx-456', + item_list: [ + expect.objectContaining({ + type: 2, // MessageItemType.IMAGE + image_item: expect.objectContaining({ + media: { + encrypt_query_param: 'cdn-encrypt-param', + aes_key: expectedAesKeyBase64, + encrypt_type: 1, + }, + }), + }), + ], + }), + ); + }); + + it('propagates getUploadUrl errors', async () => { + mockReadFileSync.mockReturnValue(fakeImageData); + mockGetUploadUrl.mockRejectedValue(new Error('Auth expired')); + + await expect(sendImage(defaultParams)).rejects.toThrow('Auth expired'); + expect(mockUploadToCdn).not.toHaveBeenCalled(); + expect(mockSendMessage).not.toHaveBeenCalled(); + }); + + it('propagates upload errors', async () => { + mockReadFileSync.mockReturnValue(fakeImageData); + mockGetUploadUrl.mockResolvedValue('upload-param-value'); + mockUploadToCdn.mockRejectedValue(new Error('CDN unavailable')); + + await expect(sendImage(defaultParams)).rejects.toThrow('CDN unavailable'); + expect(mockSendMessage).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/channels/weixin/src/send.ts b/packages/channels/weixin/src/send.ts index 54ca8fa52c1..3b1c2700d0e 100644 --- a/packages/channels/weixin/src/send.ts +++ b/packages/channels/weixin/src/send.ts @@ -2,9 +2,11 @@ * Send messages to WeChat users. */ -import { randomUUID } from 'node:crypto'; -import { sendMessage } from './api.js'; +import { randomBytes, randomUUID } from 'node:crypto'; +import { readFileSync } from 'node:fs'; +import { sendMessage, getUploadUrl, uploadToCdn } from './api.js'; import { MessageType, MessageState, MessageItemType } from './types.js'; +import { encryptAesEcb, computeMd5 } from './media.js'; /** Convert markdown to plain text (WeChat doesn't support markdown) */ export function markdownToPlainText(text: string): string { @@ -50,3 +52,76 @@ export async function sendText(params: { item_list: [{ type: MessageItemType.TEXT, text_item: { text: plainText } }], }); } + +/** + * Send an image message via the four-step CDN upload flow: + * 1. Read file, compute rawsize + MD5; generate random AES key + filekey + * 2. Request upload URL via getuploadurl + * 3. AES-128-ECB encrypt + POST upload to CDN; extract x-encrypted-param + * 4. Send message with image_item referencing the CDN media + */ +export async function sendImage(params: { + to: string; + imagePath: string; + baseUrl: string; + token: string; + contextToken: string; +}): Promise { + const { to, imagePath, baseUrl, token, contextToken } = params; + + // Step 1: read file, compute metadata + generate random identifiers + const fileBuffer = readFileSync(imagePath); + const rawsize = fileBuffer.length; + const rawfilemd5 = computeMd5(fileBuffer); + + // Generate random 16-byte AES key as hex string + const aesKeyBytes = randomBytes(16); + const aesKeyHex = aesKeyBytes.toString('hex'); + + // Generate random 32-char hex filekey + const filekey = randomBytes(16).toString('hex'); + + // AES-128-ECB PKCS#7 padding: encrypted size = ceil((rawsize + 1) / 16) * 16 + const encryptedSize = Math.ceil((rawsize + 1) / 16) * 16; + + // Step 2: get upload URL and CDN credentials + const uploadParam = await getUploadUrl( + baseUrl, + token, + to, + filekey, + rawsize, + rawfilemd5, + encryptedSize, + aesKeyHex, + ); + + // Step 3: encrypt and upload to CDN + const encrypted = encryptAesEcb(fileBuffer, aesKeyBytes); + const cdnEncryptParam = await uploadToCdn(uploadParam, filekey, encrypted); + + // Step 4: send message with image_item using CDN's x-encrypted-param + // aes_key in sendmessage should be base64(hex string) per protocol + const aesKeyBase64 = Buffer.from(aesKeyHex, 'ascii').toString('base64'); + + await sendMessage(baseUrl, token, { + to_user_id: to, + from_user_id: '', + client_id: randomUUID(), + message_type: MessageType.BOT, + message_state: MessageState.FINISH, + context_token: contextToken, + item_list: [ + { + type: MessageItemType.IMAGE, + image_item: { + media: { + encrypt_query_param: cdnEncryptParam, + aes_key: aesKeyBase64, + encrypt_type: 1, + }, + }, + }, + ], + }); +} From 0a255487fa573c301e69616d6714d487018676a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E6=B6=9B?= <408097061@qq.com> Date: Sun, 3 May 2026 11:21:48 +0800 Subject: [PATCH 2/4] =?UTF-8?q?fix(weixin):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20path=20validation,=20encoding,=20timeout,=20error?= =?UTF-8?q?=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical fixes from wenshao's review of feat/weixin-image-send: 1. File read vulnerability: add validateImagePath() in send.ts with directory allowlist, extension filter, magic-byte check, 20MB cap, and realpath resolution. Pass workspace cwd as allowed dir. 2. aes_key encoding: change from base64(hex-ascii) to base64(raw 16B) to match the protocol expectation (images use raw bytes, not hex). 3. uploadToCdn timeout: add AbortController + 40s timeout per retry attempt to prevent hanging on stalled CDN connections. 4. Unhandled rejection: wrap fallback sendText() in catch block with its own try/catch to prevent process crash on double failure. 5. Default instructions merge: append image capability guide when custom instructions lack [IMAGE:], instead of silently dropping it. 6. Dead code: remove unused imagePaths parameter from sendMessage(). 7. Regex hardening: strip code blocks before [IMAGE:] extraction, filter empty paths to prevent confusing readFileSync('') errors. 8. URL validation: reject http:// URLs and validate CDN hostname in uploadToCdn (SSRF prevention). Tests: replace identity mock with real encryptAesEcb/computeMd5 so padding mismatches are caught; fix partial node:crypto mock. Co-authored-by: Qwen-Coder --- packages/channels/weixin/src/WeixinAdapter.ts | 115 ++++----- packages/channels/weixin/src/api.ts | 229 +++++++++++++++--- packages/channels/weixin/src/send.test.ts | 78 ++++-- packages/channels/weixin/src/send.ts | 124 +++++++++- 4 files changed, 417 insertions(+), 129 deletions(-) diff --git a/packages/channels/weixin/src/WeixinAdapter.ts b/packages/channels/weixin/src/WeixinAdapter.ts index a357454c93c..1f37de70414 100644 --- a/packages/channels/weixin/src/WeixinAdapter.ts +++ b/packages/channels/weixin/src/WeixinAdapter.ts @@ -17,7 +17,7 @@ import type { import { loadAccount, DEFAULT_BASE_URL } from './accounts.js'; import { startPollLoop, getContextToken } from './monitor.js'; import type { CdnRef, FileCdnRef } from './monitor.js'; -import { sendText, sendImage } from './send.js'; +import { sendText, sendImage, detectImageMime } from './send.js'; import { downloadAndDecrypt } from './media.js'; import { getConfig, sendTyping } from './api.js'; import { TypingStatus } from './types.js'; @@ -43,21 +43,32 @@ export class WeixinChannel extends ChannelBase { } async connect(): Promise { - // Default channel instructions: tell the AI it can send images via WeChat + // Default channel instructions — always include image capability info + const imageInstructions = [ + '', + 'If you created an image file (screenshot, chart, etc.), you can send it to the user by writing:', + '[IMAGE: /absolute/path/to/file.png]', + '', + 'The marker is stripped from text and the image is uploaded automatically.', + '', + 'CRITICAL: Only use real file paths. Do NOT write [IMAGE: ...] with:', + '- Example paths like /path/to/file or /tmp/cat.png', + '- Placeholder symbols like ...', + "- Paths that don't exist on disk", + ].join('\n'); + if (!this.config.instructions) { this.config.instructions = [ - '## WeChat Channel Capabilities', - '', - 'You are communicating with users through WeChat. You CAN send images to the user.', - 'To send an image, include it in your response using this EXACT format:', - '[IMAGE: 图片文件的完整绝对路径]', + '## WeChat Channel', '', - 'Example: 如果你想发送 /tmp/cat.png 给用户, 在回复中写 [IMAGE: /tmp/cat.png]', - 'This marker will be automatically removed from the text and the image will be uploaded and sent.', - 'You can include multiple [IMAGE: ...] markers in one response.', + 'You are a concise coding assistant responding via WeChat.', + 'Keep responses under 500 characters. Use plain text only.', '', - 'Users can also send you images, which you can see and analyze.', + 'Users can also send you images.', + imageInstructions, ].join('\n'); + } else if (!this.config.instructions.includes('[IMAGE:')) { + this.config.instructions += '\n' + imageInstructions; } const account = loadAccount(); if (!account) { @@ -169,31 +180,26 @@ export class WeixinChannel extends ChannelBase { } } - // Always remind the AI about image-sending capability on every message - const IMAGE_INSTRUCTION = - '[WeChat Channel] 你可以通过微信发送图片。在回复中使用 [IMAGE: 文件绝对路径] 发送图片,例如 [IMAGE: /tmp/cat.png]。标记会被自动移除。'; - envelope.text = `${IMAGE_INSTRUCTION}\n\n${envelope.text}`; - await super.handleInbound(envelope); } - async sendMessage( - chatId: string, - text: string, - imagePaths?: string[], - ): Promise { + async sendMessage(chatId: string, text: string): Promise { const contextToken = getContextToken(chatId) || ''; - // Parse [IMAGE: /path/to/file.png] markers from text + // Parse [IMAGE: /path/to/file.png] markers from text. + // Strip code blocks first to avoid matching example syntax inside them. + const textWithoutCode = text + .replace(/```[\s\S]*?```/g, '') + .replace(/`[^`]*`/g, ''); + + // Extract image paths from code-free text, replace markers in original text. const imageRegex = /\[IMAGE:\s*([^\]]+)\]/gi; const parsedImages: string[] = []; - let cleanedText = text.replace(imageRegex, (_, path: string) => { - parsedImages.push(path.trim()); - return ''; - }); - - // Merge with any imagePaths from the ACP pipeline - const allImages = [...(imagePaths || []), ...parsedImages]; + for (const m of textWithoutCode.matchAll(imageRegex)) { + const trimmed = m[1]?.trim(); + if (trimmed) parsedImages.push(trimmed); + } + let cleanedText = text.replace(imageRegex, ''); // Clean up double blank lines left by removed markers cleanedText = cleanedText.replace(/\n{3,}/g, '\n\n').trim(); @@ -210,8 +216,10 @@ export class WeixinChannel extends ChannelBase { } // Send images - if (allImages.length) { - for (const imagePath of allImages) { + if (parsedImages.length) { + const workspaceDirs = [this.config.cwd]; + + for (const imagePath of parsedImages) { try { await sendImage({ to: chatId, @@ -219,19 +227,26 @@ export class WeixinChannel extends ChannelBase { baseUrl: this.baseUrl, token: this.token, contextToken, + workspaceDirs, }); } catch (err) { const errMsg = err instanceof Error ? err.message : String(err); process.stderr.write( `[Weixin:${this.name}] Failed to send image ${imagePath}: ${errMsg}\n`, ); - await sendText({ - to: chatId, - text: `图片发送失败: ${errMsg}`, - baseUrl: this.baseUrl, - token: this.token, - contextToken, - }); + try { + await sendText({ + to: chatId, + text: '图片发送失败,请稍后重试', + baseUrl: this.baseUrl, + token: this.token, + contextToken, + }); + } catch (fallbackErr) { + process.stderr.write( + `[Weixin:${this.name}] Fallback text also failed: ${fallbackErr instanceof Error ? fallbackErr.message : String(fallbackErr)}\n`, + ); + } } } } @@ -272,27 +287,3 @@ export class WeixinChannel extends ChannelBase { } } } - -/** Detect image MIME type from magic bytes. */ -function detectImageMime(data: Buffer): string { - if ( - data[0] === 0x89 && - data[1] === 0x50 && - data[2] === 0x4e && - data[3] === 0x47 - ) { - return 'image/png'; - } - if (data[0] === 0x47 && data[1] === 0x49 && data[2] === 0x46) { - return 'image/gif'; - } - if ( - data[0] === 0x52 && - data[1] === 0x49 && - data[2] === 0x46 && - data[3] === 0x46 - ) { - return 'image/webp'; - } - return 'image/jpeg'; -} diff --git a/packages/channels/weixin/src/api.ts b/packages/channels/weixin/src/api.ts index 3148e11e197..b63f67c1178 100644 --- a/packages/channels/weixin/src/api.ts +++ b/packages/channels/weixin/src/api.ts @@ -12,6 +12,69 @@ import type { BaseInfo, } from './types.js'; +// ── Error handling ──────────────────────────────────────────────── + +/** Structured error from WeChat iLink Bot API. */ +export class WeixinApiError extends Error { + /** HTTP status code (0 if network/timeout error). */ + status: number; + /** API-level return code (ret field in response body). */ + ret?: number; + /** API-level error code (errcode field in response body). */ + errcode?: number; + + constructor(message: string, status: number, ret?: number, errcode?: number) { + super(message); + this.name = 'WeixinApiError'; + this.status = status; + this.ret = ret; + this.errcode = errcode; + } +} + +/** Errors that are safe to retry (transient / network). */ +function isRetryableError(err: unknown): boolean { + if (err instanceof WeixinApiError) { + // Session expired — not retryable (needs re-login) + if (err.errcode === -14) return false; + // API-level transient errors (system busy, rate limit) + if (err.errcode === -1 || err.errcode === 45011) return true; + // Client errors (4xx except 429) — not retryable + if (err.status >= 400 && err.status < 500) return err.status === 429; + // Server errors (5xx) or network errors (status 0) — retryable + return err.status === 0 || err.status >= 500; + } + if (err instanceof TypeError || (err as NodeJS.ErrnoException).code) { + // Network errors (fetch TypeError, ECONNRESET, ETIMEDOUT, etc.) + return true; + } + return false; +} + +/** Exponential backoff retry wrapper. */ +async function retryWithBackoff( + fn: (attempt: number) => Promise, + maxRetries = 3, + baseDelayMs = 1000, +): Promise { + let lastError: unknown; + + for (let attempt = 1; attempt <= maxRetries + 1; attempt++) { + try { + return await fn(attempt); + } catch (err: unknown) { + lastError = err; + if (attempt > maxRetries || !isRetryableError(err)) { + throw err; + } + const delay = baseDelayMs * Math.pow(2, attempt - 1); + await new Promise((r) => setTimeout(r, delay)); + } + } + + throw lastError; +} + // iLink Bot API protocol version we are compatible with. // Used both in the request body (base_info.channel_version) and in the // iLink-App-ClientVersion header (encoded as 0x00MMNNPP). @@ -74,7 +137,26 @@ async function post( signal: controller.signal, }); if (!resp.ok) { - throw new Error(`HTTP ${resp.status}: ${resp.statusText}`); + // Try to parse the API error body for ret/errcode/errmsg + let ret: number | undefined; + let errcode: number | undefined; + let errmsg: string | undefined; + try { + const errBody = (await resp.json()) as { + ret?: number; + errcode?: number; + errmsg?: string; + }; + ret = errBody.ret; + errcode = errBody.errcode; + errmsg = errBody.errmsg; + } catch { + // ignore parse errors — use status-based message + } + const message = errmsg + ? `WeChat API error (HTTP ${resp.status}, ret=${ret}, errcode=${errcode}): ${errmsg}` + : `WeChat API error (HTTP ${resp.status})`; + throw new WeixinApiError(message, resp.status, ret, errcode); } return (await resp.json()) as T; } finally { @@ -116,7 +198,22 @@ export async function sendMessage( msg: SendMessageReq['msg'], ): Promise { const body: SendMessageReq = { msg, base_info: baseInfo() }; - await post(baseUrl, '/ilink/bot/sendmessage', body, token); + + await retryWithBackoff(async (_attempt) => { + const resp = await post<{ + ret?: number; + errcode?: number; + errmsg?: string; + }>(baseUrl, '/ilink/bot/sendmessage', body, token); + if (resp.ret !== undefined && resp.ret !== 0) { + throw new WeixinApiError( + `sendMessage failed: ret=${resp.ret} errcode=${resp.errcode} ${resp.errmsg || ''}`, + 200, + resp.ret, + resp.errcode, + ); + } + }); } export async function getConfig( @@ -188,54 +285,106 @@ export async function getUploadUrl( aeskey: aeskeyHex, base_info: baseInfo(), }; - const resp = await post( - baseUrl, - '/ilink/bot/getuploadurl', - body, - token, - ); - - // upload_full_url: CDN upload URL with all params embedded - if (resp.upload_full_url) { - return resp.upload_full_url; - } - // upload_param: CDN upload params only (must construct URL with filekey) - if (resp.upload_param) { - return resp.upload_param; - } + return retryWithBackoff(async (_attempt) => { + const resp = await post( + baseUrl, + '/ilink/bot/getuploadurl', + body, + token, + ); + + // Check API-level error first + if (resp.ret !== undefined && resp.ret !== 0) { + throw new WeixinApiError( + `getuploadurl failed: ret=${resp.ret} errcode=${undefined} errmsg=${resp.errmsg || '(none)'}`, + 200, + resp.ret, + ); + } - throw new Error( - `getuploadurl failed: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`, - ); + // upload_full_url: CDN upload URL with all params embedded + if (resp.upload_full_url) { + return resp.upload_full_url; + } + + // upload_param: CDN upload params only (must construct URL with filekey) + if (resp.upload_param) { + return resp.upload_param; + } + + throw new WeixinApiError( + `getuploadurl returned no URL: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`, + 200, + resp.ret, + ); + }); } /** Upload encrypted media to CDN. - * If urlOrParam is a full URL, use it directly. + * If urlOrParam is a full URL, use it directly (host must match). * If it's just a param, construct the URL. */ export async function uploadToCdn( urlOrParam: string, filekey: string, encryptedData: Buffer, ): Promise { - const url = urlOrParam.startsWith('http') - ? urlOrParam - : `https://novac2c.cdn.weixin.qq.com/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`; - - const resp = await fetch(url, { - method: 'POST', - headers: { 'Content-Type': 'application/octet-stream' }, - body: encryptedData, - }); - if (!resp.ok) { - throw new Error(`CDN upload failed: HTTP ${resp.status}`); - } - // Extract x-encrypted-param from response header - const encryptParam = resp.headers.get('x-encrypted-param'); - if (!encryptParam) { - throw new Error( - 'CDN upload succeeded but missing x-encrypted-param header', - ); + const CDN_HOST = 'novac2c.cdn.weixin.qq.com'; + + let url: string; + if (urlOrParam.startsWith('https://')) { + const parsed = new URL(urlOrParam); + if (parsed.hostname !== CDN_HOST) { + throw new Error(`CDN upload URL has unexpected host: ${parsed.hostname}`); + } + url = urlOrParam; + } else if (urlOrParam.startsWith('http://')) { + throw new Error('CDN upload URL must use HTTPS'); + } else { + url = `https://${CDN_HOST}/c2c/upload?encrypted_query_param=${encodeURIComponent(urlOrParam)}&filekey=${encodeURIComponent(filekey)}`; } - return encryptParam; + + return retryWithBackoff(async (_attempt) => { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 40000); + + try { + const resp = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/octet-stream' }, + body: encryptedData, + signal: controller.signal, + }); + if (!resp.ok) { + // Try to extract error details from CDN response + let cdnErrMsg: string | undefined; + try { + const errBody = (await resp.json()) as { + errmsg?: string; + ret?: number; + }; + cdnErrMsg = errBody.errmsg; + } catch { + // ignore + } + throw new WeixinApiError( + cdnErrMsg + ? `CDN upload failed: HTTP ${resp.status} — ${cdnErrMsg}` + : `CDN upload failed: HTTP ${resp.status}`, + resp.status, + ); + } + // Extract x-encrypted-param from response header + const encryptParam = resp.headers.get('x-encrypted-param'); + if (!encryptParam) { + throw new WeixinApiError( + 'CDN upload succeeded but missing x-encrypted-param header', + resp.status, + ); + } + return encryptParam; + } finally { + clearTimeout(timeout); + } + }); } diff --git a/packages/channels/weixin/src/send.test.ts b/packages/channels/weixin/src/send.test.ts index dea4ae758c1..e7b55ca2a21 100644 --- a/packages/channels/weixin/src/send.test.ts +++ b/packages/channels/weixin/src/send.test.ts @@ -1,28 +1,52 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { markdownToPlainText } from './send.js'; const { mockReadFileSync, + mockStatSync, + mockRealpathSync, mockGetUploadUrl, mockUploadToCdn, mockSendMessage, mockRandomBytes, } = vi.hoisted(() => ({ mockReadFileSync: vi.fn(), + mockStatSync: vi.fn(), + mockRealpathSync: vi.fn((p: string) => p), mockGetUploadUrl: vi.fn(), mockUploadToCdn: vi.fn(), mockSendMessage: vi.fn(), mockRandomBytes: vi.fn((size: number) => Buffer.alloc(size, 0x42)), })); +// PNG magic bytes: 89 50 4E 47 0D 0A 1A 0A +const PNG_HEADER = Buffer.from([ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, +]); + +vi.mock('node:os', () => ({ + tmpdir: () => '/tmp', +})); + vi.mock('node:fs', () => ({ readFileSync: mockReadFileSync, + statSync: mockStatSync, + realpathSync: mockRealpathSync, })); -vi.mock('node:crypto', () => ({ - randomBytes: mockRandomBytes, - randomUUID: () => 'test-uuid', -})); +vi.mock('node:path', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual }; +}); + +vi.mock('node:crypto', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + randomBytes: mockRandomBytes, + randomUUID: () => 'test-uuid', + }; +}); vi.mock('./api.js', () => ({ sendMessage: mockSendMessage, @@ -30,10 +54,9 @@ vi.mock('./api.js', () => ({ uploadToCdn: mockUploadToCdn, })); -vi.mock('./media.js', () => ({ - encryptAesEcb: vi.fn((data: Buffer) => data), - computeMd5: vi.fn(() => 'd41d8cd98f00b204e9800998ecf8427e'), -})); +// Use real encryptAesEcb / computeMd5 so tests catch padding mismatches. +const { encryptAesEcb, computeMd5 } = + await vi.importActual('./media.js'); const { sendImage } = await import('./send.js'); @@ -124,24 +147,40 @@ describe('sendImage', () => { baseUrl: 'https://api.example.com', token: 'token-abc', contextToken: 'ctx-456', + workspaceDirs: ['/home/user/project'], }; - const fakeImageData = Buffer.from('fake-image-bytes'); + const fakeImageData = Buffer.concat([ + PNG_HEADER, + Buffer.from('fake-image-bytes'), + ]); beforeEach(() => { vi.clearAllMocks(); + // statSync: must be a regular file under file size limit + mockStatSync.mockReturnValue({ + isFile: () => true, + size: fakeImageData.length, + } as unknown as ReturnType<(typeof import('node:fs'))['statSync']>); + // realpathSync: identity pass-through (default mock) + // readFileSync: returns PNG-headed data for MIME check + full read + mockReadFileSync.mockReturnValue(fakeImageData); }); it('completes the four-step upload and send flow', async () => { - mockReadFileSync.mockReturnValue(fakeImageData); mockGetUploadUrl.mockResolvedValue('upload-param-value'); mockUploadToCdn.mockResolvedValue('cdn-encrypt-param'); mockSendMessage.mockResolvedValue(undefined); await sendImage(defaultParams); - // Step 1: read file - expect(mockReadFileSync).toHaveBeenCalledWith('/tmp/test.png'); + // Step 1: validateImagePath calls readFileSync for MIME check, + // then sendImage calls readFileSync for full file read. + expect(mockReadFileSync).toHaveBeenCalledTimes(2); + expect(mockReadFileSync).toHaveBeenNthCalledWith(1, '/tmp/test.png', { + flag: 'r', + }); + expect(mockReadFileSync).toHaveBeenNthCalledWith(2, '/tmp/test.png'); // Step 2: get upload URL called with correct params const encryptedSize = Math.ceil((fakeImageData.length + 1) / 16) * 16; @@ -153,23 +192,22 @@ describe('sendImage', () => { 'user-123', expectedFilekey, fakeImageData.length, - 'd41d8cd98f00b204e9800998ecf8427e', + computeMd5(fakeImageData), encryptedSize, expectedAesKeyHex, ); - // Step 3: upload to CDN (uploadToCdn takes urlOrParam, filekey, encryptedData) + // Step 3: upload to CDN (with real encryptAesEcb output) + const aesKeyBytes = Buffer.alloc(16, 0x42); + const expectedEncrypted = encryptAesEcb(fakeImageData, aesKeyBytes); expect(mockUploadToCdn).toHaveBeenCalledWith( 'upload-param-value', expectedFilekey, - fakeImageData, + expectedEncrypted, ); // Step 4: send message with image_item using CDN's x-encrypted-param - const expectedAesKeyBase64 = Buffer.from( - '42424242424242424242424242424242', - 'ascii', - ).toString('base64'); + const expectedAesKeyBase64 = aesKeyBytes.toString('base64'); expect(mockSendMessage).toHaveBeenCalledWith( 'https://api.example.com', 'token-abc', diff --git a/packages/channels/weixin/src/send.ts b/packages/channels/weixin/src/send.ts index 3b1c2700d0e..c36c66d109a 100644 --- a/packages/channels/weixin/src/send.ts +++ b/packages/channels/weixin/src/send.ts @@ -3,7 +3,9 @@ */ import { randomBytes, randomUUID } from 'node:crypto'; -import { readFileSync } from 'node:fs'; +import { readFileSync, statSync, realpathSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { resolve, extname } from 'node:path'; import { sendMessage, getUploadUrl, uploadToCdn } from './api.js'; import { MessageType, MessageState, MessageItemType } from './types.js'; import { encryptAesEcb, computeMd5 } from './media.js'; @@ -31,6 +33,109 @@ export function markdownToPlainText(text: string): string { .trim(); } +// ── Image path validation ───────────────────────────────────────── + +const ALLOWED_EXTS = new Set(['.png', '.jpg', '.jpeg', '.gif', '.webp']); +const MAX_IMAGE_SIZE = 20 * 1024 * 1024; // 20 MB + +/** Image magic bytes → MIME type mapping. */ +export function detectImageMime(data: Buffer): string { + if ( + data[0] === 0x89 && + data[1] === 0x50 && + data[2] === 0x4e && + data[3] === 0x47 + ) { + return 'image/png'; + } + if (data[0] === 0x47 && data[1] === 0x49 && data[2] === 0x46) { + return 'image/gif'; + } + if ( + data[0] === 0x52 && + data[1] === 0x49 && + data[2] === 0x46 && + data[3] === 0x46 + ) { + return 'image/webp'; + } + // Default to JPEG; all others are rejected by extension whitelist + return 'image/jpeg'; +} + +/** + * Validate and resolve an image path before reading. + * + * Security: prevents AI-controlled [IMAGE: ...] markers from reading + * arbitrary files by enforcing directory allowlist, extension allowlist, + * size cap, and magic-byte verification. + * + * @param imagePath Raw path from the AI response. + * @param workspaceDirs Additional directories to allow (typically the cwd). + * @returns Resolved absolute realpath if valid. + */ +export function validateImagePath( + imagePath: string, + workspaceDirs: string[] = [], +): string { + const resolved = resolve(imagePath); + const ext = extname(resolved).toLowerCase(); + + if (!ALLOWED_EXTS.has(ext)) { + throw new Error(`Image extension not allowed: ${ext} (path: ${resolved})`); + } + + const real: string = (() => { + try { + return realpathSync(resolved); + } catch { + throw new Error(`Image file not found: ${resolved}`); + } + })(); + + const st = statSync(real); + if (!st.isFile()) { + throw new Error(`Not a regular file: ${real}`); + } + if (st.size > MAX_IMAGE_SIZE) { + throw new Error( + `Image too large: ${st.size} bytes (max ${MAX_IMAGE_SIZE})`, + ); + } + + // Build the allowlist: /tmp/ (and macOS real /private/tmp/), os.tmpdir(), + // plus workspace directories passed by the caller. + const ALLOWED_DIRS = [ + '/tmp/', + '/private/tmp/', + tmpdir() + '/', + ...workspaceDirs.map((d) => resolve(d) + '/'), + ]; + + if (!ALLOWED_DIRS.some((dir) => real.startsWith(dir))) { + throw new Error(`Image path outside allowed directories: ${real}`); + } + + // Verify magic bytes match the extension + const head = readFileSync(real, { flag: 'r' }); + const mime = detectImageMime(head); + const extToExpectedMime: Record = { + '.png': 'image/png', + '.jpg': 'image/jpeg', + '.jpeg': 'image/jpeg', + '.gif': 'image/gif', + '.webp': 'image/webp', + }; + const expected = extToExpectedMime[ext]; + if (mime !== expected) { + throw new Error( + `Image type mismatch: ext=${ext} expects ${expected} but got ${mime}`, + ); + } + + return real; +} + /** Send a text message */ export async function sendText(params: { to: string; @@ -55,7 +160,7 @@ export async function sendText(params: { /** * Send an image message via the four-step CDN upload flow: - * 1. Read file, compute rawsize + MD5; generate random AES key + filekey + * 1. Validate path + read file, compute rawsize + MD5; generate AES key + filekey * 2. Request upload URL via getuploadurl * 3. AES-128-ECB encrypt + POST upload to CDN; extract x-encrypted-param * 4. Send message with image_item referencing the CDN media @@ -66,11 +171,16 @@ export async function sendImage(params: { baseUrl: string; token: string; contextToken: string; + /** Workspace directories to allow for image paths. */ + workspaceDirs?: string[]; }): Promise { - const { to, imagePath, baseUrl, token, contextToken } = params; + const { to, imagePath, baseUrl, token, contextToken, workspaceDirs } = params; + + // Step 1 (security): validate and resolve the image path + const resolvedPath = validateImagePath(imagePath, workspaceDirs); - // Step 1: read file, compute metadata + generate random identifiers - const fileBuffer = readFileSync(imagePath); + // Step 2: read file, compute metadata + generate random identifiers + const fileBuffer = readFileSync(resolvedPath); const rawsize = fileBuffer.length; const rawfilemd5 = computeMd5(fileBuffer); @@ -101,8 +211,8 @@ export async function sendImage(params: { const cdnEncryptParam = await uploadToCdn(uploadParam, filekey, encrypted); // Step 4: send message with image_item using CDN's x-encrypted-param - // aes_key in sendmessage should be base64(hex string) per protocol - const aesKeyBase64 = Buffer.from(aesKeyHex, 'ascii').toString('base64'); + // aes_key: base64(raw 16 bytes) for images per protocol + const aesKeyBase64 = aesKeyBytes.toString('base64'); await sendMessage(baseUrl, token, { to_user_id: to, From 17600d5699ab24909eabbf89899d530b0e9fae02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E6=B6=9B?= <408097061@qq.com> Date: Mon, 4 May 2026 20:00:18 +0800 Subject: [PATCH 3/4] =?UTF-8?q?fix(weixin):=20address=202nd=20round=20PR?= =?UTF-8?q?=20review=20=E2=80=94=2010=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical fixes: 1. detectImageMime: add JPEG magic bytes (0xFF 0xD8 0xFF), throw on unrecognized format instead of defaulting to image/jpeg 2. getUploadUrl retry: pass errcode to WeixinApiError, add ret field check in isRetryableError so actual API errors trigger retries 3. ALLOWED_DIRS: add realpathSync('/tmp/') and realpathSync(tmpdir()) to handle macOS symlink resolution (/tmp → /private/tmp) 4. [IMAGE:] stripping: only replace markers that were actually parsed, preserving [IMAGE:] inside code blocks in displayed text 5. TOCTOU fix: use openSync/readSync(16B) for magic-byte check instead of reading the entire file twice 6. sendMessage: check both ret and errcode fields for error detection Suggestions: 7. connect(): avoid mutating this.config.instructions on reconnect 8. Fix duplicate Step 2 comment numbering 9. Replace errcode=${undefined} with errcode=${resp.errcode ?? '(none)'} 10. stderr: log structured (status=, ret=) instead of raw errmsg Co-authored-by: Qwen-Coder --- packages/channels/weixin/src/WeixinAdapter.ts | 29 +++++++-- packages/channels/weixin/src/api.ts | 14 ++++- packages/channels/weixin/src/send.test.ts | 32 ++++++---- packages/channels/weixin/src/send.ts | 63 ++++++++++++------- 4 files changed, 96 insertions(+), 42 deletions(-) diff --git a/packages/channels/weixin/src/WeixinAdapter.ts b/packages/channels/weixin/src/WeixinAdapter.ts index 1f37de70414..c3348797e58 100644 --- a/packages/channels/weixin/src/WeixinAdapter.ts +++ b/packages/channels/weixin/src/WeixinAdapter.ts @@ -19,12 +19,17 @@ import { startPollLoop, getContextToken } from './monitor.js'; import type { CdnRef, FileCdnRef } from './monitor.js'; import { sendText, sendImage, detectImageMime } from './send.js'; import { downloadAndDecrypt } from './media.js'; -import { getConfig, sendTyping } from './api.js'; +import { getConfig, sendTyping, WeixinApiError } from './api.js'; import { TypingStatus } from './types.js'; /** In-memory typing ticket cache: userId -> typingTicket */ const typingTickets = new Map(); +/** Escape special regex characters in a string. */ +function escapeRegex(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + export class WeixinChannel extends ChannelBase { private abortController: AbortController | null = null; private baseUrl: string; @@ -68,7 +73,9 @@ export class WeixinChannel extends ChannelBase { imageInstructions, ].join('\n'); } else if (!this.config.instructions.includes('[IMAGE:')) { - this.config.instructions += '\n' + imageInstructions; + // Use a local copy to avoid mutating this.config.instructions on reconnect. + this.config.instructions = + this.config.instructions + '\n' + imageInstructions; } const account = loadAccount(); if (!account) { @@ -192,14 +199,23 @@ export class WeixinChannel extends ChannelBase { .replace(/```[\s\S]*?```/g, '') .replace(/`[^`]*`/g, ''); - // Extract image paths from code-free text, replace markers in original text. + // Extract image paths from code-free text. const imageRegex = /\[IMAGE:\s*([^\]]+)\]/gi; const parsedImages: string[] = []; for (const m of textWithoutCode.matchAll(imageRegex)) { const trimmed = m[1]?.trim(); if (trimmed) parsedImages.push(trimmed); } - let cleanedText = text.replace(imageRegex, ''); + + // Only strip markers that were actually parsed (avoids silently + // removing [IMAGE:] inside code blocks from the displayed text). + let cleanedText = text; + for (const path of parsedImages) { + cleanedText = cleanedText.replace( + new RegExp(`\\[IMAGE:\\s*${escapeRegex(path)}\\]`, 'gi'), + '', + ); + } // Clean up double blank lines left by removed markers cleanedText = cleanedText.replace(/\n{3,}/g, '\n\n').trim(); @@ -230,9 +246,10 @@ export class WeixinChannel extends ChannelBase { workspaceDirs, }); } catch (err) { - const errMsg = err instanceof Error ? err.message : String(err); + const status = err instanceof WeixinApiError ? err.status : 0; + const ret = err instanceof WeixinApiError ? err.ret : undefined; process.stderr.write( - `[Weixin:${this.name}] Failed to send image ${imagePath}: ${errMsg}\n`, + `[Weixin:${this.name}] Failed to send image (status=${status} ret=${ret})\n`, ); try { await sendText({ diff --git a/packages/channels/weixin/src/api.ts b/packages/channels/weixin/src/api.ts index b63f67c1178..0f96bf68067 100644 --- a/packages/channels/weixin/src/api.ts +++ b/packages/channels/weixin/src/api.ts @@ -39,6 +39,8 @@ function isRetryableError(err: unknown): boolean { if (err.errcode === -14) return false; // API-level transient errors (system busy, rate limit) if (err.errcode === -1 || err.errcode === 45011) return true; + // ret field is used by getUploadUrl and other endpoints + if (err.ret !== undefined && err.ret !== 0) return false; // Client errors (4xx except 429) — not retryable if (err.status >= 400 && err.status < 500) return err.status === 429; // Server errors (5xx) or network errors (status 0) — retryable @@ -205,7 +207,10 @@ export async function sendMessage( errcode?: number; errmsg?: string; }>(baseUrl, '/ilink/bot/sendmessage', body, token); - if (resp.ret !== undefined && resp.ret !== 0) { + if ( + (resp.ret !== undefined && resp.ret !== 0) || + (resp.errcode !== undefined && resp.errcode !== 0) + ) { throw new WeixinApiError( `sendMessage failed: ret=${resp.ret} errcode=${resp.errcode} ${resp.errmsg || ''}`, 200, @@ -253,6 +258,7 @@ interface GetUploadUrlReq { interface GetUploadUrlResp { ret?: number; + errcode?: number; errmsg?: string; upload_full_url?: string; upload_param?: string; @@ -297,9 +303,10 @@ export async function getUploadUrl( // Check API-level error first if (resp.ret !== undefined && resp.ret !== 0) { throw new WeixinApiError( - `getuploadurl failed: ret=${resp.ret} errcode=${undefined} errmsg=${resp.errmsg || '(none)'}`, + `getuploadurl failed: ret=${resp.ret} errcode=${resp.errcode ?? '(none)'} errmsg=${resp.errmsg || '(none)'}`, 200, resp.ret, + resp.errcode, ); } @@ -314,9 +321,10 @@ export async function getUploadUrl( } throw new WeixinApiError( - `getuploadurl returned no URL: ret=${resp.ret} errmsg=${resp.errmsg || '(none)'}`, + `getuploadurl returned no URL: ret=${resp.ret} errcode=${resp.errcode ?? '(none)'} errmsg=${resp.errmsg || '(none)'}`, 200, resp.ret, + resp.errcode, ); }); } diff --git a/packages/channels/weixin/src/send.test.ts b/packages/channels/weixin/src/send.test.ts index e7b55ca2a21..046e842ec25 100644 --- a/packages/channels/weixin/src/send.test.ts +++ b/packages/channels/weixin/src/send.test.ts @@ -28,11 +28,21 @@ vi.mock('node:os', () => ({ tmpdir: () => '/tmp', })); -vi.mock('node:fs', () => ({ - readFileSync: mockReadFileSync, - statSync: mockStatSync, - realpathSync: mockRealpathSync, -})); +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readFileSync: mockReadFileSync, + statSync: mockStatSync, + realpathSync: mockRealpathSync, + openSync: vi.fn(() => 42), + readSync: vi.fn((_fd: number, buf: Buffer) => { + PNG_HEADER.copy(buf); + return PNG_HEADER.length; + }), + closeSync: vi.fn(), + }; +}); vi.mock('node:path', async (importOriginal) => { const actual = await importOriginal(); @@ -174,13 +184,11 @@ describe('sendImage', () => { await sendImage(defaultParams); - // Step 1: validateImagePath calls readFileSync for MIME check, - // then sendImage calls readFileSync for full file read. - expect(mockReadFileSync).toHaveBeenCalledTimes(2); - expect(mockReadFileSync).toHaveBeenNthCalledWith(1, '/tmp/test.png', { - flag: 'r', - }); - expect(mockReadFileSync).toHaveBeenNthCalledWith(2, '/tmp/test.png'); + // Step 1: validateImagePath uses openSync/readSync for magic-byte + // check (only 16 bytes), then sendImage calls readFileSync for + // full file read. + expect(mockReadFileSync).toHaveBeenCalledTimes(1); + expect(mockReadFileSync).toHaveBeenCalledWith('/tmp/test.png'); // Step 2: get upload URL called with correct params const encryptedSize = Math.ceil((fakeImageData.length + 1) / 16) * 16; diff --git a/packages/channels/weixin/src/send.ts b/packages/channels/weixin/src/send.ts index c36c66d109a..95fe96cfa2d 100644 --- a/packages/channels/weixin/src/send.ts +++ b/packages/channels/weixin/src/send.ts @@ -3,7 +3,14 @@ */ import { randomBytes, randomUUID } from 'node:crypto'; -import { readFileSync, statSync, realpathSync } from 'node:fs'; +import { + readFileSync, + statSync, + realpathSync, + openSync, + readSync, + closeSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { resolve, extname } from 'node:path'; import { sendMessage, getUploadUrl, uploadToCdn } from './api.js'; @@ -59,8 +66,12 @@ export function detectImageMime(data: Buffer): string { ) { return 'image/webp'; } - // Default to JPEG; all others are rejected by extension whitelist - return 'image/jpeg'; + if (data[0] === 0xff && data[1] === 0xd8 && data[2] === 0xff) { + return 'image/jpeg'; + } + throw new Error( + 'Unrecognized image format: magic bytes do not match any supported type', + ); } /** @@ -104,11 +115,13 @@ export function validateImagePath( } // Build the allowlist: /tmp/ (and macOS real /private/tmp/), os.tmpdir(), - // plus workspace directories passed by the caller. + // plus workspace directories passed by the caller. Use realpathSync to + // resolve symlinks (e.g. /tmp → /private/tmp on macOS). const ALLOWED_DIRS = [ '/tmp/', - '/private/tmp/', + realpathSync('/tmp/') + '/', tmpdir() + '/', + realpathSync(tmpdir()) + '/', ...workspaceDirs.map((d) => resolve(d) + '/'), ]; @@ -116,21 +129,29 @@ export function validateImagePath( throw new Error(`Image path outside allowed directories: ${real}`); } - // Verify magic bytes match the extension - const head = readFileSync(real, { flag: 'r' }); - const mime = detectImageMime(head); - const extToExpectedMime: Record = { - '.png': 'image/png', - '.jpg': 'image/jpeg', - '.jpeg': 'image/jpeg', - '.gif': 'image/gif', - '.webp': 'image/webp', - }; - const expected = extToExpectedMime[ext]; - if (mime !== expected) { - throw new Error( - `Image type mismatch: ext=${ext} expects ${expected} but got ${mime}`, - ); + // Verify magic bytes match the extension (read only first 16 bytes to + // avoid TOCTOU double-read — sendImage reads the full file later). + let fd: number | undefined; + try { + fd = openSync(real, 'r'); + const head = Buffer.alloc(16); + const bytesRead = readSync(fd, head, 0, 16, 0); + const mime = detectImageMime(head.slice(0, bytesRead)); + const extToExpectedMime: Record = { + '.png': 'image/png', + '.jpg': 'image/jpeg', + '.jpeg': 'image/jpeg', + '.gif': 'image/gif', + '.webp': 'image/webp', + }; + const expected = extToExpectedMime[ext]; + if (mime !== expected) { + throw new Error( + `Image type mismatch: ext=${ext} expects ${expected} but got ${mime}`, + ); + } + } finally { + if (fd !== undefined) closeSync(fd); } return real; @@ -179,7 +200,7 @@ export async function sendImage(params: { // Step 1 (security): validate and resolve the image path const resolvedPath = validateImagePath(imagePath, workspaceDirs); - // Step 2: read file, compute metadata + generate random identifiers + // Step 1 (continued): read file, compute metadata + generate random identifiers const fileBuffer = readFileSync(resolvedPath); const rawsize = fileBuffer.length; const rawfilemd5 = computeMd5(fileBuffer); From cbcc0f74788a49dfb4d7dcd9cfc7f58034b45eb6 Mon Sep 17 00:00:00 2001 From: Mr-Maidong <15557111679@163.com> Date: Tue, 5 May 2026 16:58:10 +0800 Subject: [PATCH 4/4] =?UTF-8?q?fix(weixin):=203rd=20round=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20errcode=20checks,=20error=20logging,=20timeout,=20p?= =?UTF-8?q?ath=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - api.ts: add errcode check in getUploadUrl (align with sendMessage) - api.ts: pass ret/errcode from CDN error to WeixinApiError - send.ts: resolve workspace dirs with realpathSync - WeixinAdapter.ts: include errcode and err.message in error log - media.ts: add 40s timeout to downloadAndDecrypt fetch - send.test.ts: add 12 tests covering detectImageMime and validateImagePath error branches Co-authored-by: Qwen-Coder --- packages/channels/weixin/src/WeixinAdapter.ts | 5 +- packages/channels/weixin/src/api.ts | 12 +- packages/channels/weixin/src/media.ts | 21 ++-- packages/channels/weixin/src/send.test.ts | 115 +++++++++++++++++- packages/channels/weixin/src/send.ts | 2 +- 5 files changed, 143 insertions(+), 12 deletions(-) diff --git a/packages/channels/weixin/src/WeixinAdapter.ts b/packages/channels/weixin/src/WeixinAdapter.ts index c3348797e58..0660c9a88d1 100644 --- a/packages/channels/weixin/src/WeixinAdapter.ts +++ b/packages/channels/weixin/src/WeixinAdapter.ts @@ -248,8 +248,11 @@ export class WeixinChannel extends ChannelBase { } catch (err) { const status = err instanceof WeixinApiError ? err.status : 0; const ret = err instanceof WeixinApiError ? err.ret : undefined; + const errcode = + err instanceof WeixinApiError ? err.errcode : undefined; + const msg = err instanceof Error ? err.message : String(err); process.stderr.write( - `[Weixin:${this.name}] Failed to send image (status=${status} ret=${ret})\n`, + `[Weixin:${this.name}] Failed to send image (status=${status} ret=${ret} errcode=${errcode}): ${msg}\n`, ); try { await sendText({ diff --git a/packages/channels/weixin/src/api.ts b/packages/channels/weixin/src/api.ts index 0f96bf68067..5097b978e5b 100644 --- a/packages/channels/weixin/src/api.ts +++ b/packages/channels/weixin/src/api.ts @@ -301,7 +301,10 @@ export async function getUploadUrl( ); // Check API-level error first - if (resp.ret !== undefined && resp.ret !== 0) { + if ( + (resp.ret !== undefined && resp.ret !== 0) || + (resp.errcode !== undefined && resp.errcode !== 0) + ) { throw new WeixinApiError( `getuploadurl failed: ret=${resp.ret} errcode=${resp.errcode ?? '(none)'} errmsg=${resp.errmsg || '(none)'}`, 200, @@ -366,12 +369,17 @@ export async function uploadToCdn( if (!resp.ok) { // Try to extract error details from CDN response let cdnErrMsg: string | undefined; + let cdnRet: number | undefined; + let cdnErrCode: number | undefined; try { const errBody = (await resp.json()) as { errmsg?: string; ret?: number; + errcode?: number; }; cdnErrMsg = errBody.errmsg; + cdnRet = errBody.ret; + cdnErrCode = errBody.errcode; } catch { // ignore } @@ -380,6 +388,8 @@ export async function uploadToCdn( ? `CDN upload failed: HTTP ${resp.status} — ${cdnErrMsg}` : `CDN upload failed: HTTP ${resp.status}`, resp.status, + cdnRet, + cdnErrCode, ); } // Extract x-encrypted-param from response header diff --git a/packages/channels/weixin/src/media.ts b/packages/channels/weixin/src/media.ts index 1407b0fb422..93dcd35fb05 100644 --- a/packages/channels/weixin/src/media.ts +++ b/packages/channels/weixin/src/media.ts @@ -45,14 +45,21 @@ export async function downloadAndDecrypt( ): Promise { const url = buildCdnDownloadUrl(encryptQueryParam); - const resp = await fetch(url); - if (!resp.ok) { - throw new Error(`CDN download failed: HTTP ${resp.status}`); - } + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 40000); + + try { + const resp = await fetch(url, { signal: controller.signal }); + if (!resp.ok) { + throw new Error(`CDN download failed: HTTP ${resp.status}`); + } - const ciphertext = Buffer.from(await resp.arrayBuffer()); - const keyBuf = parseAesKey(aesKey); - return decryptAesEcb(ciphertext, keyBuf); + const ciphertext = Buffer.from(await resp.arrayBuffer()); + const keyBuf = parseAesKey(aesKey); + return decryptAesEcb(ciphertext, keyBuf); + } finally { + clearTimeout(timeout); + } } /** AES-128-ECB encryption for CDN upload. */ diff --git a/packages/channels/weixin/src/send.test.ts b/packages/channels/weixin/src/send.test.ts index 046e842ec25..3d3c275c8f4 100644 --- a/packages/channels/weixin/src/send.test.ts +++ b/packages/channels/weixin/src/send.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import * as fs from 'node:fs'; import { markdownToPlainText } from './send.js'; const { @@ -68,7 +69,9 @@ vi.mock('./api.js', () => ({ const { encryptAesEcb, computeMd5 } = await vi.importActual('./media.js'); -const { sendImage } = await import('./send.js'); +const { sendImage, detectImageMime, validateImagePath } = await import( + './send.js' +); describe('markdownToPlainText', () => { it('strips code blocks', () => { @@ -150,6 +153,112 @@ describe('markdownToPlainText', () => { }); }); +describe('detectImageMime', () => { + it('detects PNG magic bytes', () => { + const buf = Buffer.from([0x89, 0x50, 0x4e, 0x47]); + expect(detectImageMime(buf)).toBe('image/png'); + }); + + it('detects GIF magic bytes', () => { + const buf = Buffer.from([0x47, 0x49, 0x46]); + expect(detectImageMime(buf)).toBe('image/gif'); + }); + + it('detects WebP magic bytes (RIFF)', () => { + const buf = Buffer.from([0x52, 0x49, 0x46, 0x46]); + expect(detectImageMime(buf)).toBe('image/webp'); + }); + + it('detects JPEG magic bytes', () => { + const buf = Buffer.from([0xff, 0xd8, 0xff]); + expect(detectImageMime(buf)).toBe('image/jpeg'); + }); + + it('throws for unrecognized magic bytes', () => { + const buf = Buffer.from([0x00, 0x00, 0x00, 0x00]); + expect(() => detectImageMime(buf)).toThrow('Unrecognized image format'); + }); +}); + +describe('validateImagePath', () => { + const workspaceDirs = ['/home/user/project']; + + beforeEach(() => { + vi.clearAllMocks(); + // Restore default mock behaviour: identity pass-through for realpath, + // regular file, small size, PNG magic in readSync. + mockRealpathSync.mockImplementation((p: string) => p); + mockStatSync.mockReturnValue({ + isFile: () => true, + size: 100, + } as unknown as ReturnType<(typeof fs)['statSync']>); + vi.mocked(fs.readSync).mockImplementation((_fd: number, buf: Buffer) => { + PNG_HEADER.copy(buf); + return PNG_HEADER.length; + }); + }); + + it('rejects disallowed extensions', () => { + expect(() => + validateImagePath('/tmp/screenshot.txt', workspaceDirs), + ).toThrow('Image extension not allowed'); + }); + + it('rejects non-existent files', () => { + mockRealpathSync.mockImplementation(() => { + throw new Error('ENOENT: no such file'); + }); + expect(() => validateImagePath('/tmp/missing.png', workspaceDirs)).toThrow( + 'Image file not found', + ); + }); + + it('rejects non-regular files (directories etc.)', () => { + mockStatSync.mockReturnValue({ + isFile: () => false, + size: 0, + } as unknown as ReturnType<(typeof fs)['statSync']>); + expect(() => validateImagePath('/tmp/some-dir.png', workspaceDirs)).toThrow( + 'Not a regular file', + ); + }); + + it('rejects files exceeding 20 MB cap', () => { + mockStatSync.mockReturnValue({ + isFile: () => true, + size: 21 * 1024 * 1024, + } as unknown as ReturnType<(typeof fs)['statSync']>); + expect(() => validateImagePath('/tmp/huge.png', workspaceDirs)).toThrow( + 'Image too large', + ); + }); + + it('rejects paths outside allowed directories', () => { + mockRealpathSync.mockImplementation((p: string) => p); + expect(() => validateImagePath('/etc/passwd.png', workspaceDirs)).toThrow( + 'Image path outside allowed directories', + ); + }); + + it('rejects image with magic bytes that do not match extension', () => { + // readSync returns JPEG magic, but file extension is .png + vi.mocked(fs.readSync).mockImplementation((_fd: number, buf: Buffer) => { + const jpegMagic = Buffer.from([0xff, 0xd8, 0xff]); + jpegMagic.copy(buf); + return jpegMagic.length; + }); + expect(() => + validateImagePath('/tmp/actually-jpeg.png', workspaceDirs), + ).toThrow('Image type mismatch'); + }); + + it('returns resolved realpath on success', () => { + mockRealpathSync.mockImplementation((p: string) => `/private${p}`); + const result = validateImagePath('/tmp/photo.png', workspaceDirs); + expect(result).toBe('/private/tmp/photo.png'); + }); +}); + describe('sendImage', () => { const defaultParams = { to: 'user-123', @@ -172,7 +281,9 @@ describe('sendImage', () => { isFile: () => true, size: fakeImageData.length, } as unknown as ReturnType<(typeof import('node:fs'))['statSync']>); - // realpathSync: identity pass-through (default mock) + // realpathSync: identity pass-through (restore default after + // validateImagePath tests may have overridden it). + mockRealpathSync.mockImplementation((p: string) => p); // readFileSync: returns PNG-headed data for MIME check + full read mockReadFileSync.mockReturnValue(fakeImageData); }); diff --git a/packages/channels/weixin/src/send.ts b/packages/channels/weixin/src/send.ts index 95fe96cfa2d..27c8ab5fddd 100644 --- a/packages/channels/weixin/src/send.ts +++ b/packages/channels/weixin/src/send.ts @@ -122,7 +122,7 @@ export function validateImagePath( realpathSync('/tmp/') + '/', tmpdir() + '/', realpathSync(tmpdir()) + '/', - ...workspaceDirs.map((d) => resolve(d) + '/'), + ...workspaceDirs.map((d) => realpathSync(resolve(d)) + '/'), ]; if (!ALLOWED_DIRS.some((dir) => real.startsWith(dir))) {