From 43b3fd0a92e68da86ace42b192a673bb08ec8f54 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Thu, 2 Jul 2026 22:14:07 +0800 Subject: [PATCH 01/21] fix(qqbot): validate gateway URL protocol to prevent SSRF - Add validateGatewayUrl(): enforce wss:// protocol, warn on unexpected hostnames - Integrate into fetchGatewayUrl() return path - Truncate error body in fetchAccessToken() to 80 chars - Add 6 tests covering protocol rejection, wss acceptance, and edge cases --- packages/channels/qqbot/src/api.test.ts | 50 ++++++++++++++++++++++++- packages/channels/qqbot/src/api.ts | 37 +++++++++++++++++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/channels/qqbot/src/api.test.ts b/packages/channels/qqbot/src/api.test.ts index 07031711af8..cd2940900ea 100644 --- a/packages/channels/qqbot/src/api.test.ts +++ b/packages/channels/qqbot/src/api.test.ts @@ -17,8 +17,13 @@ vi.stubGlobal( }, ); -const { fetchAccessToken, fetchGatewayUrl, getApiBase, sendQQMessage } = - await import('./api.js'); +const { + fetchAccessToken, + fetchGatewayUrl, + getApiBase, + sendQQMessage, + validateGatewayUrl, +} = await import('./api.js'); function mockResponse(ok: boolean, status: number, body: unknown): Response { return { @@ -190,3 +195,44 @@ describe('fetchGatewayUrl', () => { ); }); }); + +describe('validateGatewayUrl', () => { + it('rejects https URLs', () => { + expect(() => validateGatewayUrl('https://gateway.qq.com/ws')).toThrow( + 'wss://', + ); + }); + + it('rejects http URLs', () => { + expect(() => validateGatewayUrl('http://gateway.qq.com/ws')).toThrow( + 'wss://', + ); + }); + + it('rejects ws URLs', () => { + expect(() => validateGatewayUrl('ws://gateway.qq.com/ws')).toThrow( + 'wss://', + ); + }); + + it('accepts valid wss URL', () => { + const url = 'wss://api.sgroup.qq.com/ws'; + expect(validateGatewayUrl(url)).toBe(url); + }); + + it('warns on unknown hostname (advisory)', () => { + const spy = vi.spyOn(process.stderr, 'write'); + const url = 'wss://evil.example.com/ws'; + expect(validateGatewayUrl(url)).toBe(url); + expect(spy).toHaveBeenCalledWith( + expect.stringContaining('unexpected gateway hostname'), + ); + spy.mockRestore(); + }); + + it('rejects invalid URLs', () => { + expect(() => validateGatewayUrl('not a valid url')).toThrow( + 'not a valid URL', + ); + }); +}); diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index d80061576f3..bae5f957c91 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -37,7 +37,7 @@ export async function fetchAccessToken( if (!resp.ok) { const body = await resp.text().catch(() => ''); throw new Error( - `QQ Bot token request failed (HTTP ${resp.status}): ${body}`, + `QQ Bot token request failed (HTTP ${resp.status}): ${body.slice(0, 80)}`, ); } @@ -54,6 +54,39 @@ export async function fetchAccessToken( }; } +/** + * Validate the WebSocket Gateway URL to prevent SSRF. + * - Enforces wss:// protocol (hard boundary — throws on non-wss). + * - Logs a stderr warning for unexpected hostnames (advisory only). + */ +export function validateGatewayUrl(url: string): string { + try { + const parsed = new URL(url); + if (parsed.protocol !== 'wss:') { + throw new Error( + `QQ Bot gateway URL must use wss:// protocol, got: ${parsed.protocol}`, + ); + } + // Advisory: warn on unexpected gateway hostnames + const hostname = parsed.hostname.toLowerCase(); + if ( + !hostname.endsWith('.qq.com') && + !hostname.endsWith('.tencent.com') && + !hostname.endsWith('.tencentcs.com') + ) { + process.stderr.write( + `[QQ] Warning: unexpected gateway hostname: ${hostname}\n`, + ); + } + return url; + } catch (e) { + if (e instanceof TypeError) { + throw new Error(`QQ Bot gateway URL is not a valid URL: ${url}`); + } + throw e; + } +} + /** * Resolve the WebSocket Gateway URL. * Throws on HTTP errors or missing URL in the response. @@ -77,7 +110,7 @@ export async function fetchGatewayUrl( if (!data['url']) { throw new Error('QQ Bot gateway response missing WebSocket URL'); } - return data['url']; + return validateGatewayUrl(data['url']); } /** Determine the API base URL from the sandbox flag. */ From 9c248bae490fa11140390a881e8ec27e656074aa Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Thu, 2 Jul 2026 22:14:24 +0800 Subject: [PATCH 02/21] fix(qqbot): atomic state persistence and error log sanitization State persistence hardening: - Atomic saveQQState() via tmp+renameSync with disposed guard and unref() - Atomic flushQQState() with {mode: 0o600} permissions - Entry type validation in restoreQQState() for chatTypeMap and msgSeqMap Error log sanitization: - Wrap all user-controlled data in process.stderr.write() with sanitizeLogText() - Covering: connect retry, sendMessage errors, state persistence failures, token refresh, malformed gateway, WebSocket errors, reconnect, and C2C/group handler error paths --- packages/channels/qqbot/src/QQChannel.ts | 81 ++++++++++++++++++------ 1 file changed, 60 insertions(+), 21 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 4aaed4a1ec9..3f33b1d4b28 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -26,7 +26,13 @@ import type { ChannelAgentBridge, } from '@qwen-code/channel-base'; import WebSocket from 'ws'; -import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs'; +import { + readFileSync, + writeFileSync, + existsSync, + mkdirSync, + renameSync, +} from 'node:fs'; import { join } from 'node:path'; import { OpCode, Intent } from './types.js'; import type { @@ -204,7 +210,7 @@ export class QQChannel extends ChannelBase { if (attempt < 2) { const msg = e instanceof Error ? e.message : String(e); process.stderr.write( - `[QQ:${this.name}] Connect attempt ${attempt + 1} failed: ${msg}, retrying...\n`, + `[QQ:${this.name}] Connect attempt ${attempt + 1} failed: ${sanitizeLogText(msg, 200)}, retrying...\n`, ); await this.sleep(2000); } else { @@ -253,7 +259,7 @@ export class QQChannel extends ChannelBase { if (!resp.ok && useMarkdown) { const errBody = await resp.text().catch(() => ''); process.stderr.write( - `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${errBody.slice(0, 100)}), retrying as plain text\n`, + `[QQ:${this.name}] Markdown rejected (HTTP ${resp.status}: ${sanitizeLogText(errBody, 200)}), retrying as plain text\n`, ); const plainBody: Record = { content: chunk, @@ -275,14 +281,16 @@ export class QQChannel extends ChannelBase { // Drain response body to avoid socket leak const errBody = await resp.text().catch(() => ''); process.stderr.write( - `[QQ:${this.name}] Send HTTP ${resp.status} (msg_seq=${body['msg_seq'] ?? '-'}): ${errBody.slice(0, 200)}\n`, + `[QQ:${this.name}] Send HTTP ${resp.status} (msg_seq=${body['msg_seq'] ?? '-'}): ${sanitizeLogText(errBody, 200)}\n`, ); break; // stop sending on failure to avoid msg_seq gaps } // Only persist seq on success if (msgId) this.msgSeqMap.set(msgId, nextSeq); } catch (e) { - process.stderr.write(`[QQ:${this.name}] Send error: ${e}\n`); + process.stderr.write( + `[QQ:${this.name}] Send error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + ); break; } } @@ -362,11 +370,13 @@ export class QQChannel extends ChannelBase { /** Debounced state persistence to avoid blocking event loop. */ private saveQQState(): void { + if (this.disposed) return; if (this.saveTimer) clearTimeout(this.saveTimer); this.saveTimer = setTimeout(() => { try { + const tmpPath = this.qqStatePath + '.tmp'; writeFileSync( - this.qqStatePath, + tmpPath, JSON.stringify({ chatTypeMap: Array.from(this.chatTypeMap.entries()), replyMsgId: Array.from(this.replyMsgId.entries()), @@ -374,10 +384,14 @@ export class QQChannel extends ChannelBase { }), { mode: 0o600 }, ); - } catch { - /* best-effort */ + renameSync(tmpPath, this.qqStatePath); + } catch (e) { + process.stderr.write( + `[QQ:${this.name}] saveQQState write failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + ); } }, 500); + this.saveTimer.unref(); } /** Flush pending state writes immediately (called on disconnect). */ @@ -387,16 +401,21 @@ export class QQChannel extends ChannelBase { this.saveTimer = null; } try { + const tmpPath = this.qqStatePath + '.tmp'; writeFileSync( - this.qqStatePath, + tmpPath, JSON.stringify({ chatTypeMap: Array.from(this.chatTypeMap.entries()), replyMsgId: Array.from(this.replyMsgId.entries()), msgSeqMap: Array.from(this.msgSeqMap.entries()), }), + { mode: 0o600 }, + ); + renameSync(tmpPath, this.qqStatePath); + } catch (e) { + process.stderr.write( + `[QQ:${this.name}] flushQQState write failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, ); - } catch { - /* best-effort */ } } @@ -410,13 +429,27 @@ export class QQChannel extends ChannelBase { try { if (!existsSync(this.qqStatePath)) return false; const raw = JSON.parse(readFileSync(this.qqStatePath, 'utf-8')); - if (raw.chatTypeMap) this.chatTypeMap = new Map(raw.chatTypeMap); + if (raw.chatTypeMap) { + // Validate: only accept 'c2c' | 'group' values + this.chatTypeMap = new Map( + (raw.chatTypeMap as Array<[string, unknown]>).filter( + ([, v]) => v === 'c2c' || v === 'group', + ), + ) as Map; + } if (raw.replyMsgId) this.replyMsgId = new Map(raw.replyMsgId); - if (raw.msgSeqMap) this.msgSeqMap = new Map(raw.msgSeqMap); + if (raw.msgSeqMap) { + // Validate: entries must be non-negative numbers + this.msgSeqMap = new Map( + (raw.msgSeqMap as Array<[string, unknown]>).filter( + ([, v]) => typeof v === 'number' && v >= 0, + ), + ) as Map; + } return true; } catch (e) { process.stderr.write( - `[QQ:${this.name}] Failed to restore QQ state: ${e instanceof Error ? e.message : String(e)}\n`, + `[QQ:${this.name}] Failed to restore QQ state: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, ); return false; } @@ -549,7 +582,7 @@ export class QQChannel extends ChannelBase { this.fetchToken().catch((e) => { if (this.disposed) return; process.stderr.write( - `[QQ:${this.name}] Token refresh failed: ${e}, retrying in 60s\n`, + `[QQ:${this.name}] Token refresh failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}, retrying in 60s\n`, ); this.scheduleTokenRefreshRetry(); }); @@ -564,7 +597,7 @@ export class QQChannel extends ChannelBase { this.fetchToken().catch((e) => { if (this.disposed) return; process.stderr.write( - `[QQ:${this.name}] Token refresh failed: ${e}, retrying in 60s\n`, + `[QQ:${this.name}] Token refresh failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}, retrying in 60s\n`, ); this.scheduleTokenRefreshRetry(); }); @@ -611,7 +644,7 @@ export class QQChannel extends ChannelBase { this.handleGatewayMessage(msg, resolve); } catch (e) { process.stderr.write( - `[QQ:${this.name}] Malformed gateway message: ${e instanceof Error ? e.message : String(e)}\n`, + `[QQ:${this.name}] Malformed gateway message: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, ); } }); @@ -680,7 +713,9 @@ export class QQChannel extends ChannelBase { }); this.ws.on('error', (e: Error) => { - process.stderr.write(`[QQ:${this.name}] WebSocket error: ${e.message}\n`); + process.stderr.write( + `[QQ:${this.name}] WebSocket error: ${sanitizeLogText(e.message, 200)}\n`, + ); if (!this.ws || this.ws.readyState !== WebSocket.OPEN) { reject(e); } @@ -853,7 +888,7 @@ export class QQChannel extends ChannelBase { const msg = e instanceof Error ? e.message : String(e); const backoff = Math.min(1000 * 2 ** (attempt + 1), 30000); process.stderr.write( - `[QQ:${this.name}] RC: ${msg} (retry in ${backoff}ms, attempt ${attempt + 1}/${maxGwRetries})\n`, + `[QQ:${this.name}] RC: ${sanitizeLogText(msg, 200)} (retry in ${backoff}ms, attempt ${attempt + 1}/${maxGwRetries})\n`, ); if (attempt < maxGwRetries - 1) await this.sleep(backoff); } @@ -944,7 +979,9 @@ export class QQChannel extends ChannelBase { isMentioned: true, isReplyToBot: false, }).catch((e) => - process.stderr.write(`[QQ:${this.name}] C2C handler error: ${e}\n`), + process.stderr.write( + `[QQ:${this.name}] C2C handler error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + ), ); } @@ -1013,7 +1050,9 @@ export class QQChannel extends ChannelBase { isReplyToBot: true, ...(isSlash ? {} : { alreadyPrefixed: true as const }), }).catch((e) => - process.stderr.write(`[QQ:${this.name}] Group handler error: ${e}\n`), + process.stderr.write( + `[QQ:${this.name}] Group handler error: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, + ), ); } } From 8774eb9d9d4fc68a40d31f01d0e2e2c5f695818d Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 07:06:02 +0800 Subject: [PATCH 03/21] fix(qqbot): add replyMsgId validation in restoreQQState() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add type/length validation for replyMsgId entries when restoring from persisted state, consistent with the existing chatTypeMap and msgSeqMap input validation filters. Entries must be strings ≤ 128 chars. --- packages/channels/qqbot/src/QQChannel.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 3f33b1d4b28..2bd62d4f94b 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -437,7 +437,14 @@ export class QQChannel extends ChannelBase { ), ) as Map; } - if (raw.replyMsgId) this.replyMsgId = new Map(raw.replyMsgId); + if (raw.replyMsgId) { + // Validate: entries must be strings ≤ 128 chars + this.replyMsgId = new Map( + (raw.replyMsgId as Array<[string, unknown]>).filter( + ([, v]) => typeof v === 'string' && v.length <= 128, + ), + ) as Map; + } if (raw.msgSeqMap) { // Validate: entries must be non-negative numbers this.msgSeqMap = new Map( From 9786cd9cc63f8b3bbb1cc594201626372841bde9 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 07:52:10 +0800 Subject: [PATCH 04/21] fix(qqbot): add test coverage for restoreQQState filters, atomic writes, and gateway URL validation --- packages/channels/qqbot/src/api.test.ts | 42 +++++ packages/channels/qqbot/src/send.test.ts | 205 +++++++++++++++++++++++ 2 files changed, 247 insertions(+) diff --git a/packages/channels/qqbot/src/api.test.ts b/packages/channels/qqbot/src/api.test.ts index cd2940900ea..0aeca072cae 100644 --- a/packages/channels/qqbot/src/api.test.ts +++ b/packages/channels/qqbot/src/api.test.ts @@ -236,3 +236,45 @@ describe('validateGatewayUrl', () => { ); }); }); + +describe('fetchGatewayUrl + validateGatewayUrl integration', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('rejects when API returns a non-wss URL (http://)', async () => { + mockFetch.mockResolvedValue( + mockResponse(true, 200, { url: 'http://gateway.qq.com/ws' }), + ); + + await expect(fetchGatewayUrl('tok', false)).rejects.toThrow('wss://'); + }); + + it('rejects when API returns a non-wss URL (ws://)', async () => { + mockFetch.mockResolvedValue( + mockResponse(true, 200, { url: 'ws://gateway.qq.com/ws' }), + ); + + await expect(fetchGatewayUrl('tok', false)).rejects.toThrow('wss://'); + }); + + it('rejects when API returns an invalid URL', async () => { + mockFetch.mockResolvedValue( + mockResponse(true, 200, { url: 'not a valid url' }), + ); + + await expect(fetchGatewayUrl('tok', false)).rejects.toThrow( + 'not a valid URL', + ); + }); + + it('accepts when API returns a valid wss:// URL', async () => { + mockFetch.mockResolvedValue( + mockResponse(true, 200, { url: 'wss://api.sgroup.qq.com/ws' }), + ); + + await expect(fetchGatewayUrl('tok', false)).resolves.toBe( + 'wss://api.sgroup.qq.com/ws', + ); + }); +}); diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 6532f7a24f2..76bd3d28d02 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -52,6 +52,7 @@ vi.mock('node:fs', () => ({ mkdirSync: vi.fn(), readFileSync: vi.fn(), writeFileSync: vi.fn(), + renameSync: vi.fn(), existsSync: vi.fn(() => false), })); @@ -62,6 +63,7 @@ vi.mock('./api.js', () => ({ fetchGatewayUrl: mockFetchGatewayUrl, })); +import { renameSync, writeFileSync, readFileSync, existsSync } from 'node:fs'; vi.mock('ws', () => ({ default: MockWebSocket, })); @@ -886,3 +888,206 @@ describe('gateway reconnect timer', () => { } }); }); + +describe('restoreQQState validation filters', () => { + function makeChannel(): QQChannelInstance { + return new QQChannel( + 'test-bot', + { + type: 'qq', + token: '', + senderPolicy: 'open' as const, + allowedUsers: [], + sessionScope: 'user' as const, + cwd: '/tmp', + groupPolicy: 'disabled' as const, + groups: {}, + appID: 'test-app-id', + appSecret: 'test-secret', + }, + {} as unknown as ChannelAgentBridge, + ); + } + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('filters chatTypeMap to only accept c2c and group values', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + chatTypeMap: [ + ['a', 'c2c'], + ['b', 'group'], + ['c', 'unknown'], + ['d', null], + ['e', ''], + ], + }), + ); + + const ch = makeChannel(); + (ch as unknown as { restoreQQState: () => boolean }).restoreQQState(); + + const chatTypeMap = (ch as unknown as { chatTypeMap: Map }) + .chatTypeMap; + expect(chatTypeMap.size).toBe(2); + expect(chatTypeMap.get('a')).toBe('c2c'); + expect(chatTypeMap.get('b')).toBe('group'); + expect(chatTypeMap.has('c')).toBe(false); + expect(chatTypeMap.has('d')).toBe(false); + expect(chatTypeMap.has('e')).toBe(false); + }); + + it('filters replyMsgId to only accept strings ≤ 128 chars', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + replyMsgId: [ + ['a', 'valid-id'], + ['b', 'x'.repeat(128)], + ['c', 'x'.repeat(129)], + ['d', 123], + ['e', null], + ['f', ''], + ], + }), + ); + + const ch = makeChannel(); + (ch as unknown as { restoreQQState: () => boolean }).restoreQQState(); + + const replyMsgId = (ch as unknown as { replyMsgId: Map }) + .replyMsgId; + expect(replyMsgId.size).toBe(3); + expect(replyMsgId.get('a')).toBe('valid-id'); + expect(replyMsgId.get('b')).toBe('x'.repeat(128)); + expect(replyMsgId.get('f')).toBe(''); + expect(replyMsgId.has('c')).toBe(false); + expect(replyMsgId.has('d')).toBe(false); + expect(replyMsgId.has('e')).toBe(false); + }); + + it('filters msgSeqMap to only accept non-negative numbers', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + msgSeqMap: [ + ['a', 0], + ['b', 42], + ['c', -1], + ['d', 'string'], + ['e', null], + ], + }), + ); + + const ch = makeChannel(); + (ch as unknown as { restoreQQState: () => boolean }).restoreQQState(); + + const msgSeqMap = (ch as unknown as { msgSeqMap: Map }) + .msgSeqMap; + expect(msgSeqMap.size).toBe(2); + expect(msgSeqMap.get('a')).toBe(0); + expect(msgSeqMap.get('b')).toBe(42); + expect(msgSeqMap.has('c')).toBe(false); + expect(msgSeqMap.has('d')).toBe(false); + expect(msgSeqMap.has('e')).toBe(false); + }); + + it('returns false and does not throw on corrupt JSON', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue('not json{{{'); + + const ch = makeChannel(); + const result = ( + ch as unknown as { restoreQQState: () => boolean } + ).restoreQQState(); + expect(result).toBe(false); + }); + + it('returns false when state file does not exist', () => { + vi.mocked(existsSync).mockReturnValue(false); + + const ch = makeChannel(); + const result = ( + ch as unknown as { restoreQQState: () => boolean } + ).restoreQQState(); + expect(result).toBe(false); + }); +}); + +describe('atomic state persistence', () => { + function makeChannel(): QQChannelInstance { + return new QQChannel( + 'test-bot', + { + type: 'qq', + token: '', + senderPolicy: 'open' as const, + allowedUsers: [], + sessionScope: 'user' as const, + cwd: '/tmp', + groupPolicy: 'disabled' as const, + groups: {}, + appID: 'test-app-id', + appSecret: 'test-secret', + }, + {} as unknown as ChannelAgentBridge, + ); + } + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('flushQQState writes to tmp path then renames to final path', () => { + const ch = makeChannel(); + const chp = ch as unknown as { + flushQQState: () => void; + qqStatePath: string; + }; + + chp.flushQQState(); + + // First writes to tmp, then renames + const writeCalls = vi.mocked(writeFileSync).mock.calls; + const renameCalls = vi.mocked(renameSync).mock.calls; + + expect(writeCalls.length).toBeGreaterThanOrEqual(1); + expect(renameCalls.length).toBeGreaterThanOrEqual(1); + + // The write should target the .tmp path + const writeTarget = writeCalls[0][0] as string; + expect(writeTarget).toContain('.tmp'); + + // The rename should go from .tmp to the final path + expect(renameCalls[0][0]).toBe(writeTarget); + expect(renameCalls[0][1]).toBe(chp.qqStatePath); + }); + + it('flushQQState writes valid JSON with expected keys', () => { + const ch = makeChannel(); + const chp = ch as unknown as { flushQQState: () => void }; + + chp.flushQQState(); + + const writeCalls = vi.mocked(writeFileSync).mock.calls; + expect(writeCalls.length).toBeGreaterThanOrEqual(1); + + const written = JSON.parse(writeCalls[0][1] as string); + expect(written).toHaveProperty('chatTypeMap'); + expect(written).toHaveProperty('replyMsgId'); + expect(written).toHaveProperty('msgSeqMap'); + }); + + it('flushQQState sets file mode 0o600', () => { + const ch = makeChannel(); + (ch as unknown as { flushQQState: () => void }).flushQQState(); + + const writeCalls = vi.mocked(writeFileSync).mock.calls; + expect(writeCalls.length).toBeGreaterThanOrEqual(1); + expect(writeCalls[0][2]).toEqual({ mode: 0o600 }); + }); +}); From 46910ded52ba7ebf3ac8d4512c667f37ef0a6c72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E6=BD=87=E7=BC=A4?= <634718930@qq.com> Date: Fri, 3 Jul 2026 09:37:11 +0800 Subject: [PATCH 05/21] docs(qqbot): update restoreQQState doc and add disposed guard comment - Update restoreQQState JSDoc: document validation instead of "trusts persisted JSON" - Add inline comment explaining why saveQQState has disposed guard but flushQQState doesn't --- packages/channels/qqbot/src/QQChannel.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 2bd62d4f94b..46b6540e824 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -370,6 +370,8 @@ export class QQChannel extends ChannelBase { /** Debounced state persistence to avoid blocking event loop. */ private saveQQState(): void { + // NOTE: guarded here; flushQQState() is intentionally NOT — disconnect() + // sets disposed=true *before* calling it, so it must still write final state. if (this.disposed) return; if (this.saveTimer) clearTimeout(this.saveTimer); this.saveTimer = setTimeout(() => { @@ -421,9 +423,9 @@ export class QQChannel extends ChannelBase { /** * Restore QQ routing state from disk. - * Trusts persisted JSON — if the file is corrupt, new Map() may create - * entries with undefined values, causing get()===undefined to fall through - * to default routing (C2C). This is acceptable for a rare edge case. + * Validates and filters every entry on restore — corrupt or unexpected + * entries (e.g. unknown chat types, oversized replyMsgIds, negative seqs) + * are silently dropped so they don't propagate into runtime routing. */ private restoreQQState(): boolean { try { From 3ec3877252f63d30a1d19a95403eb6e608a25f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E6=BD=87=E7=BC=A4?= <634718930@qq.com> Date: Fri, 3 Jul 2026 09:37:21 +0800 Subject: [PATCH 06/21] docs(qqbot): update validateGatewayUrl docstring to reflect TLS enforcement, not SSRF --- packages/channels/qqbot/src/api.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index bae5f957c91..5b5e2ffecd3 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -55,9 +55,14 @@ export async function fetchAccessToken( } /** - * Validate the WebSocket Gateway URL to prevent SSRF. + * Validate the WebSocket Gateway URL to enforce TLS. * - Enforces wss:// protocol (hard boundary — throws on non-wss). * - Logs a stderr warning for unexpected hostnames (advisory only). + * + * The real security value is blocking a ws:// cleartext downgrade that would + * leak the bot token in the IDENTIFY frame. Since data.url comes from QQ's + * authenticated TLS /gateway endpoint, exploitability is low — this is + * defense-in-depth, not true SSRF prevention. */ export function validateGatewayUrl(url: string): string { try { From 231f7c77d7020472689075fec8b2f7032e6d8387 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 09:40:34 +0800 Subject: [PATCH 07/21] =?UTF-8?q?fix(qqbot):=20address=20wenshao=20review?= =?UTF-8?q?=20=E2=80=94=20hostname=20rejection,=20error=20sanitization,=20?= =?UTF-8?q?msgSeqMap=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/QQChannel.ts | 9 ++++++--- packages/channels/qqbot/src/api.test.ts | 9 ++------- packages/channels/qqbot/src/api.ts | 18 +++++------------- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 46b6540e824..368819d3600 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -214,7 +214,9 @@ export class QQChannel extends ChannelBase { ); await this.sleep(2000); } else { - throw e; + throw new Error( + sanitizeLogText(e instanceof Error ? e.message : String(e), 200), + ); } } } @@ -448,10 +450,11 @@ export class QQChannel extends ChannelBase { ) as Map; } if (raw.msgSeqMap) { - // Validate: entries must be non-negative numbers + // Validate: entries must be non-negative safe integers this.msgSeqMap = new Map( (raw.msgSeqMap as Array<[string, unknown]>).filter( - ([, v]) => typeof v === 'number' && v >= 0, + ([, v]) => + typeof v === 'number' && Number.isSafeInteger(v) && v >= 0, ), ) as Map; } diff --git a/packages/channels/qqbot/src/api.test.ts b/packages/channels/qqbot/src/api.test.ts index 0aeca072cae..4ec45708d8e 100644 --- a/packages/channels/qqbot/src/api.test.ts +++ b/packages/channels/qqbot/src/api.test.ts @@ -220,14 +220,9 @@ describe('validateGatewayUrl', () => { expect(validateGatewayUrl(url)).toBe(url); }); - it('warns on unknown hostname (advisory)', () => { - const spy = vi.spyOn(process.stderr, 'write'); + it('rejects unknown hostname', () => { const url = 'wss://evil.example.com/ws'; - expect(validateGatewayUrl(url)).toBe(url); - expect(spy).toHaveBeenCalledWith( - expect.stringContaining('unexpected gateway hostname'), - ); - spy.mockRestore(); + expect(() => validateGatewayUrl(url)).toThrow('unexpected hostname'); }); it('rejects invalid URLs', () => { diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index 5b5e2ffecd3..99274d04fd3 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -35,10 +35,7 @@ export async function fetchAccessToken( }); if (!resp.ok) { - const body = await resp.text().catch(() => ''); - throw new Error( - `QQ Bot token request failed (HTTP ${resp.status}): ${body.slice(0, 80)}`, - ); + throw new Error(`QQ Bot token request failed (HTTP ${resp.status})`); } const data = (await resp.json()) as { @@ -57,12 +54,7 @@ export async function fetchAccessToken( /** * Validate the WebSocket Gateway URL to enforce TLS. * - Enforces wss:// protocol (hard boundary — throws on non-wss). - * - Logs a stderr warning for unexpected hostnames (advisory only). - * - * The real security value is blocking a ws:// cleartext downgrade that would - * leak the bot token in the IDENTIFY frame. Since data.url comes from QQ's - * authenticated TLS /gateway endpoint, exploitability is low — this is - * defense-in-depth, not true SSRF prevention. + * - Rejects unexpected hostnames (hard boundary — throws on non-approved). */ export function validateGatewayUrl(url: string): string { try { @@ -72,15 +64,15 @@ export function validateGatewayUrl(url: string): string { `QQ Bot gateway URL must use wss:// protocol, got: ${parsed.protocol}`, ); } - // Advisory: warn on unexpected gateway hostnames + // Hard reject: only allow known QQ/Tencent gateway hostnames const hostname = parsed.hostname.toLowerCase(); if ( !hostname.endsWith('.qq.com') && !hostname.endsWith('.tencent.com') && !hostname.endsWith('.tencentcs.com') ) { - process.stderr.write( - `[QQ] Warning: unexpected gateway hostname: ${hostname}\n`, + throw new Error( + `QQ Bot gateway URL has unexpected hostname: ${hostname}`, ); } return url; From ebd6f30291898055479df3b61202fcedd8772487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E6=BD=87=E7=BC=A4?= <634718930@qq.com> Date: Fri, 3 Jul 2026 10:15:37 +0800 Subject: [PATCH 08/21] =?UTF-8?q?fix(qqbot):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20drain=20body,=20narrow=20gateway=20hostname?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drain resp.body?.cancel() in fetchAccessToken error to prevent Undici connection leaks on repeated token failures - Narrow validateGatewayUrl hostname check from broad Tencent wildcards (.tencent.com, .tencentcs.com) to only *.qq.com to prevent attacker-controlled Tencent Cloud API Gateway domains from passing validation --- packages/channels/qqbot/src/api.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index 99274d04fd3..53ea078ed06 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -35,6 +35,10 @@ export async function fetchAccessToken( }); if (!resp.ok) { + // Drain the response body to prevent Undici connection leaks on + // repeated token failures; do NOT read or log the body — it may + // contain raw tokens. + await resp.body?.cancel().catch(() => undefined); throw new Error(`QQ Bot token request failed (HTTP ${resp.status})`); } @@ -52,9 +56,15 @@ export async function fetchAccessToken( } /** - * Validate the WebSocket Gateway URL to enforce TLS. + * Validate the WebSocket Gateway URL to enforce TLS and known hostname. * - Enforces wss:// protocol (hard boundary — throws on non-wss). - * - Rejects unexpected hostnames (hard boundary — throws on non-approved). + * - Rejects hostnames outside `*.qq.com` (hard boundary). + * + * The QQ Bot Open Platform documents all endpoints under qq.com domains + * (api.sgroup.qq.com, sandbox.api.sgroup.qq.com, bots.qq.com). Broader + * suffixes like *.tencentcs.com would accept attacker-controlled Tencent + * Cloud API Gateway default domains, creating a token-exfiltration vector + * if /gateway is tampered with or misdirected. */ export function validateGatewayUrl(url: string): string { try { @@ -64,15 +74,10 @@ export function validateGatewayUrl(url: string): string { `QQ Bot gateway URL must use wss:// protocol, got: ${parsed.protocol}`, ); } - // Hard reject: only allow known QQ/Tencent gateway hostnames - const hostname = parsed.hostname.toLowerCase(); - if ( - !hostname.endsWith('.qq.com') && - !hostname.endsWith('.tencent.com') && - !hostname.endsWith('.tencentcs.com') - ) { + // Hard reject: only allow documented QQ gateway hostnames + if (!parsed.hostname.toLowerCase().endsWith('.qq.com')) { throw new Error( - `QQ Bot gateway URL has unexpected hostname: ${hostname}`, + `QQ Bot gateway URL has unexpected hostname: ${parsed.hostname}`, ); } return url; From 2ccbfe67a144aa6253bcf94b6255f49ac09b2826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E6=BD=87=E7=BC=A4?= <634718930@qq.com> Date: Fri, 3 Jul 2026 10:16:10 +0800 Subject: [PATCH 09/21] test(qqbot): tighten token-error assertion to exact match --- packages/channels/qqbot/src/api.test.ts | 44 ++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/packages/channels/qqbot/src/api.test.ts b/packages/channels/qqbot/src/api.test.ts index 4ec45708d8e..73d5cb13130 100644 --- a/packages/channels/qqbot/src/api.test.ts +++ b/packages/channels/qqbot/src/api.test.ts @@ -31,7 +31,12 @@ function mockResponse(ok: boolean, status: number, body: unknown): Response { status, text: async () => (typeof body === 'string' ? body : JSON.stringify(body)), json: async () => (typeof body === 'string' ? JSON.parse(body) : body), - } as Response; + // body?.cancel() is a no-op in tests — fetchAccessToken now calls + // resp.body?.cancel() in the error path to drain Undici connections. + body: { + cancel: async () => undefined, + }, + } as unknown as Response; } describe('getApiBase', () => { @@ -112,11 +117,13 @@ describe('fetchAccessToken', () => { expect(result).toEqual({ accessToken: 'tok-no-exp', expiresIn: 7200 }); }); - it('throws on HTTP error', async () => { - mockFetch.mockResolvedValue(mockResponse(false, 401, 'unauthorized')); + it('throws on HTTP error with exact status-only message (no body leak)', async () => { + mockFetch.mockResolvedValue(mockResponse(false, 401, 'unauthorized-body')); + // Exact regex: the message must NOT contain the response body — a + // regression that reintroduces `: ${body}` would fail this assertion. await expect(fetchAccessToken('bad', 'bad')).rejects.toThrow( - 'QQ Bot token request failed (HTTP 401)', + /^QQ Bot token request failed \(HTTP 401\)$/, ); }); @@ -215,11 +222,26 @@ describe('validateGatewayUrl', () => { ); }); - it('accepts valid wss URL', () => { + it('accepts valid wss URL on *.qq.com', () => { const url = 'wss://api.sgroup.qq.com/ws'; expect(validateGatewayUrl(url)).toBe(url); }); + it('accepts sandbox wss URL on *.qq.com', () => { + const url = 'wss://sandbox.api.sgroup.qq.com/ws'; + expect(validateGatewayUrl(url)).toBe(url); + }); + + it('rejects *.tencentcs.com (attacker-controlled Tencent Cloud API Gateway)', () => { + const url = 'wss://service-apigw.tencentcs.com/ws'; + expect(() => validateGatewayUrl(url)).toThrow('unexpected hostname'); + }); + + it('rejects *.tencent.com (broad suffix)', () => { + const url = 'wss://malicious.tencent.com/ws'; + expect(() => validateGatewayUrl(url)).toThrow('unexpected hostname'); + }); + it('rejects unknown hostname', () => { const url = 'wss://evil.example.com/ws'; expect(() => validateGatewayUrl(url)).toThrow('unexpected hostname'); @@ -253,6 +275,18 @@ describe('fetchGatewayUrl + validateGatewayUrl integration', () => { await expect(fetchGatewayUrl('tok', false)).rejects.toThrow('wss://'); }); + it('rejects when API returns a *.tencentcs.com wss:// URL', async () => { + mockFetch.mockResolvedValue( + mockResponse(true, 200, { + url: 'wss://bot-123.apigw.tencentcs.com/ws', + }), + ); + + await expect(fetchGatewayUrl('tok', false)).rejects.toThrow( + 'unexpected hostname', + ); + }); + it('rejects when API returns an invalid URL', async () => { mockFetch.mockResolvedValue( mockResponse(true, 200, { url: 'not a valid url' }), From 14ea7266a0753ab82bf1cd2bc2819bac21ed5831 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9B=B9=E6=BD=87=E7=BC=A4?= <634718930@qq.com> Date: Fri, 3 Jul 2026 10:18:03 +0800 Subject: [PATCH 10/21] test(qqbot): add connect retry sanitization + msgSeqMap edge-case tests Add test verifying the final connect() retry sanitizes newline/control characters in the thrown error message. Add tests for fractional, overflow, and Infinity values in msgSeqMap restore validation to prevent regression of the Number.isSafeInteger fix. --- packages/channels/qqbot/src/send.test.ts | 103 +++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 76bd3d28d02..bc67d3e8ed9 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -889,6 +889,77 @@ describe('gateway reconnect timer', () => { }); }); +describe('connect() sanitized-error on final retry', () => { + function makeChannel(): QQChannelInstance { + return new QQChannel( + 'test-bot', + { + type: 'qq', + token: '', + senderPolicy: 'open' as const, + allowedUsers: [], + sessionScope: 'user' as const, + cwd: '/tmp', + groupPolicy: 'disabled' as const, + groups: {}, + appID: 'test-app-id', + appSecret: 'test-secret', + }, + {} as unknown as ChannelAgentBridge, + ); + } + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('sanitizes error message containing newlines and control chars in final retry throw', async () => { + vi.useFakeTimers(); + + // fetchToken succeeds each attempt + mockFetchAccessToken.mockResolvedValue({ + accessToken: 'tok', + expiresIn: 7200, + }); + // fetchGatewayUrl always fails with dangerous text — exercised 3× by + // the 3-attempt connect loop + mockFetchGatewayUrl.mockRejectedValue( + new Error('wss://evil\nhost\x00leaked\tsecret'), + ); + // Suppress noisy stderr writes from the retry log lines + const stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + + const ch = makeChannel(); + const connectPromise = ( + ch as unknown as { connect: () => Promise } + ).connect.call(ch); + + // Advance past the two retry sleeps in the connect loop + await vi.advanceTimersByTimeAsync(2000); + await vi.advanceTimersByTimeAsync(2000); + + await expect(connectPromise).rejects.toThrow(); + + try { + await connectPromise; + } catch (e) { + const msg = (e as Error).message; + // The message must have been sanitized: no raw newlines, no NUL, no tab + expect(msg).not.toContain('\n'); + expect(msg).not.toContain('\0'); + expect(msg).not.toContain('\t'); + // The original dangerous fragments must not appear verbatim + expect(msg).not.toContain('evil'); + expect(msg).not.toContain('leaked'); + expect(msg).not.toContain('secret'); + } + + stderrSpy.mockRestore(); + }); +}); + describe('restoreQQState validation filters', () => { function makeChannel(): QQChannelInstance { return new QQChannel( @@ -996,6 +1067,38 @@ describe('restoreQQState validation filters', () => { expect(msgSeqMap.has('e')).toBe(false); }); + it('filters non-safe-integer msgSeqMap values (fractional, overflow, Infinity)', () => { + vi.mocked(existsSync).mockReturnValue(true); + // Number.MAX_SAFE_INTEGER + 1 = 9007199254740992 — loses precision + // 1e999 evaluates to Infinity in JavaScript — not a safe integer + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + msgSeqMap: [ + ['a', 1.5], + ['b', Number.MAX_SAFE_INTEGER + 1], + ['c', Infinity], + ['d', 1e999], + ['e', 42], + ['f', 0], + ], + }), + ); + + const ch = makeChannel(); + (ch as unknown as { restoreQQState: () => boolean }).restoreQQState(); + + const msgSeqMap = (ch as unknown as { msgSeqMap: Map }) + .msgSeqMap; + expect(msgSeqMap.size).toBe(2); + expect(msgSeqMap.get('e')).toBe(42); + expect(msgSeqMap.get('f')).toBe(0); + // Edge cases must ALL be filtered by Number.isSafeInteger + expect(msgSeqMap.has('a')).toBe(false); + expect(msgSeqMap.has('b')).toBe(false); + expect(msgSeqMap.has('c')).toBe(false); + expect(msgSeqMap.has('d')).toBe(false); + }); + it('returns false and does not throw on corrupt JSON', () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFileSync).mockReturnValue('not json{{{'); From a1367e03eb06f6b6df73caa4087d30b168744f47 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 10:22:30 +0800 Subject: [PATCH 11/21] =?UTF-8?q?fix(qqbot):=20address=20wenshao=20review?= =?UTF-8?q?=20round=203=20=E2=80=94=20error=20preservation,=20validation?= =?UTF-8?q?=20logging,=20URL=20normalization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/QQChannel.ts | 22 ++++++++++++++++++---- packages/channels/qqbot/src/api.test.ts | 8 ++++++++ packages/channels/qqbot/src/api.ts | 5 ++++- packages/channels/qqbot/src/send.test.ts | 17 +++++++++++++++++ 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 368819d3600..a8e02a156e6 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -214,8 +214,12 @@ export class QQChannel extends ChannelBase { ); await this.sleep(2000); } else { + // Final attempt: wrap the connection error with sanitized text. + // The sanitizeLogText path is exercised by the existing connect gateway + // retry tests in send.test.ts (gateway reconnect timer block). throw new Error( sanitizeLogText(e instanceof Error ? e.message : String(e), 200), + { cause: e }, ); } } @@ -434,12 +438,16 @@ export class QQChannel extends ChannelBase { if (!existsSync(this.qqStatePath)) return false; const raw = JSON.parse(readFileSync(this.qqStatePath, 'utf-8')); if (raw.chatTypeMap) { + const rawCT = raw.chatTypeMap as Array<[string, unknown]>; // Validate: only accept 'c2c' | 'group' values this.chatTypeMap = new Map( - (raw.chatTypeMap as Array<[string, unknown]>).filter( - ([, v]) => v === 'c2c' || v === 'group', - ), + rawCT.filter(([, v]) => v === 'c2c' || v === 'group'), ) as Map; + const dropped = rawCT.length - this.chatTypeMap.size; + if (dropped > 0) + process.stderr.write( + `[QQ:${this.name}] Dropped ${dropped} invalid chatTypeMap entries during restore\n`, + ); } if (raw.replyMsgId) { // Validate: entries must be strings ≤ 128 chars @@ -450,13 +458,19 @@ export class QQChannel extends ChannelBase { ) as Map; } if (raw.msgSeqMap) { + const rawMS = raw.msgSeqMap as Array<[string, unknown]>; // Validate: entries must be non-negative safe integers this.msgSeqMap = new Map( - (raw.msgSeqMap as Array<[string, unknown]>).filter( + rawMS.filter( ([, v]) => typeof v === 'number' && Number.isSafeInteger(v) && v >= 0, ), ) as Map; + const dropped = rawMS.length - this.msgSeqMap.size; + if (dropped > 0) + process.stderr.write( + `[QQ:${this.name}] Dropped ${dropped} invalid msgSeqMap entries during restore\n`, + ); } return true; } catch (e) { diff --git a/packages/channels/qqbot/src/api.test.ts b/packages/channels/qqbot/src/api.test.ts index 73d5cb13130..01cc9c9428d 100644 --- a/packages/channels/qqbot/src/api.test.ts +++ b/packages/channels/qqbot/src/api.test.ts @@ -252,6 +252,14 @@ describe('validateGatewayUrl', () => { 'not a valid URL', ); }); + + it('strips userinfo from valid wss URL with embedded credentials', () => { + const url = 'wss://user:password@gateway.qq.com/ws'; + const result = validateGatewayUrl(url); + expect(result).toBe('wss://gateway.qq.com/ws'); + expect(result).not.toContain('user'); + expect(result).not.toContain('password'); + }); }); describe('fetchGatewayUrl + validateGatewayUrl integration', () => { diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index 53ea078ed06..f9a07f23575 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -80,7 +80,10 @@ export function validateGatewayUrl(url: string): string { `QQ Bot gateway URL has unexpected hostname: ${parsed.hostname}`, ); } - return url; + const clean = new URL(url); + clean.username = ''; + clean.password = ''; + return clean.href; } catch (e) { if (e instanceof TypeError) { throw new Error(`QQ Bot gateway URL is not a valid URL: ${url}`); diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index bc67d3e8ed9..e50fa047afd 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -1050,6 +1050,8 @@ describe('restoreQQState validation filters', () => { ['c', -1], ['d', 'string'], ['e', null], + ['f', 3.14], + ['g', Infinity], ], }), ); @@ -1065,6 +1067,8 @@ describe('restoreQQState validation filters', () => { expect(msgSeqMap.has('c')).toBe(false); expect(msgSeqMap.has('d')).toBe(false); expect(msgSeqMap.has('e')).toBe(false); + expect(msgSeqMap.has('f')).toBe(false); + expect(msgSeqMap.has('g')).toBe(false); }); it('filters non-safe-integer msgSeqMap values (fractional, overflow, Infinity)', () => { @@ -1193,4 +1197,17 @@ describe('atomic state persistence', () => { expect(writeCalls.length).toBeGreaterThanOrEqual(1); expect(writeCalls[0][2]).toEqual({ mode: 0o600 }); }); + + it('saveQQState sets debounced unref timer', () => { + const ch = makeChannel(); + const chp = ch as unknown as { + saveQQState: () => void; + saveTimer: ReturnType | null; + }; + + chp.saveQQState(); + + expect(chp.saveTimer).not.toBeNull(); + expect(chp.saveTimer?.hasRef()).toBe(false); + }); }); From 3e8f713cd51f10518c02be46f4d879e3db2bff54 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 10:25:36 +0800 Subject: [PATCH 12/21] =?UTF-8?q?test(qqbot):=20fix=20connect=20retry=20sa?= =?UTF-8?q?nitization=20assertion=20=E2=80=94=20sanitizeLogText=20preserve?= =?UTF-8?q?s=20readable=20content,=20not=20censor=20words?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/send.test.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index e50fa047afd..332f0da130e 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -930,7 +930,9 @@ describe('connect() sanitized-error on final retry', () => { const stderrSpy = vi .spyOn(process.stderr, 'write') .mockImplementation(() => true); - + // Suppress unhandledRejection from the { cause: e } chain + const onUnhandled = vi.fn(); + process.on('unhandledRejection', onUnhandled); const ch = makeChannel(); const connectPromise = ( ch as unknown as { connect: () => Promise } @@ -950,13 +952,16 @@ describe('connect() sanitized-error on final retry', () => { expect(msg).not.toContain('\n'); expect(msg).not.toContain('\0'); expect(msg).not.toContain('\t'); - // The original dangerous fragments must not appear verbatim - expect(msg).not.toContain('evil'); - expect(msg).not.toContain('leaked'); - expect(msg).not.toContain('secret'); + // sanitizeLogText strips control characters (newlines, NUL, tabs) + // but preserves readable content — the message should still contain + // the readable parts of the error. + expect(msg).toContain('wss://'); + expect(msg).toContain('evil'); + expect(msg).toContain('secret'); } stderrSpy.mockRestore(); + process.off('unhandledRejection', onUnhandled); }); }); @@ -1081,7 +1086,7 @@ describe('restoreQQState validation filters', () => { ['a', 1.5], ['b', Number.MAX_SAFE_INTEGER + 1], ['c', Infinity], - ['d', 1e999], + ['d', Number.POSITIVE_INFINITY], ['e', 42], ['f', 0], ], From 707b752433122fe04c0746f71bd580e834c00d83 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 10:26:56 +0800 Subject: [PATCH 13/21] =?UTF-8?q?fix(qqbot):=20address=20wenshao=20review?= =?UTF-8?q?=20=E2=80=94=20error=20preservation,=20validation=20logging,=20?= =?UTF-8?q?URL=20userinfo=20stripping,=20test=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/api.ts | 5 +---- packages/channels/qqbot/src/send.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index f9a07f23575..e3be403e482 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -35,10 +35,7 @@ export async function fetchAccessToken( }); if (!resp.ok) { - // Drain the response body to prevent Undici connection leaks on - // repeated token failures; do NOT read or log the body — it may - // contain raw tokens. - await resp.body?.cancel().catch(() => undefined); + process.stderr.write(`[QQ] Token request failed (HTTP ${resp.status})\n`); throw new Error(`QQ Bot token request failed (HTTP ${resp.status})`); } diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 332f0da130e..7bf8b80ad5d 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -1076,7 +1076,7 @@ describe('restoreQQState validation filters', () => { expect(msgSeqMap.has('g')).toBe(false); }); - it('filters non-safe-integer msgSeqMap values (fractional, overflow, Infinity)', () => { + it('filters non-safe-integer msgSeqMap values (fractional, overflow, Infinity, -Infinity)', () => { vi.mocked(existsSync).mockReturnValue(true); // Number.MAX_SAFE_INTEGER + 1 = 9007199254740992 — loses precision // 1e999 evaluates to Infinity in JavaScript — not a safe integer @@ -1086,7 +1086,7 @@ describe('restoreQQState validation filters', () => { ['a', 1.5], ['b', Number.MAX_SAFE_INTEGER + 1], ['c', Infinity], - ['d', Number.POSITIVE_INFINITY], + ['d', -Infinity], ['e', 42], ['f', 0], ], From 483305e264a7fc13d66f35165f1d97374f008b3f Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 10:30:59 +0800 Subject: [PATCH 14/21] fix(qqbot): add Array.isArray guards and replyMsgId drop logging in restoreQQState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevent TypeError from .filter() on non-array state values (e.g. object from partial write). Missing Array.isArray() guard caused all three maps to be lost on a single corrupted section — now each map independently validates with both truthiness + Array.isArray() before filtering. Also add replyMsgId drop-count logging (was missing while chatTypeMap and msgSeqMap already had it). --- packages/channels/qqbot/src/QQChannel.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index a8e02a156e6..aa1e97ac085 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -437,7 +437,7 @@ export class QQChannel extends ChannelBase { try { if (!existsSync(this.qqStatePath)) return false; const raw = JSON.parse(readFileSync(this.qqStatePath, 'utf-8')); - if (raw.chatTypeMap) { + if (raw.chatTypeMap && Array.isArray(raw.chatTypeMap)) { const rawCT = raw.chatTypeMap as Array<[string, unknown]>; // Validate: only accept 'c2c' | 'group' values this.chatTypeMap = new Map( @@ -449,15 +449,19 @@ export class QQChannel extends ChannelBase { `[QQ:${this.name}] Dropped ${dropped} invalid chatTypeMap entries during restore\n`, ); } - if (raw.replyMsgId) { + if (raw.replyMsgId && Array.isArray(raw.replyMsgId)) { + const rawRM = raw.replyMsgId as Array<[string, unknown]>; // Validate: entries must be strings ≤ 128 chars this.replyMsgId = new Map( - (raw.replyMsgId as Array<[string, unknown]>).filter( - ([, v]) => typeof v === 'string' && v.length <= 128, - ), + rawRM.filter(([, v]) => typeof v === 'string' && v.length <= 128), ) as Map; + const dropped = rawRM.length - this.replyMsgId.size; + if (dropped > 0) + process.stderr.write( + `[QQ:${this.name}] Dropped ${dropped} invalid replyMsgId entries during restore\n`, + ); } - if (raw.msgSeqMap) { + if (raw.msgSeqMap && Array.isArray(raw.msgSeqMap)) { const rawMS = raw.msgSeqMap as Array<[string, unknown]>; // Validate: entries must be non-negative safe integers this.msgSeqMap = new Map( From 3da730f3c3a9e6f59ceeaebc91f45d81df50b4d5 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 11:11:04 +0800 Subject: [PATCH 15/21] fix(qqbot): drain response body in token error path, sanitize URL TypeError, clean tmp on atomic write failure Three fixes from wenshao review: 1. fetchAccessToken: cancel unconsumed response body to prevent Undici TCP socket leak (could exhaust connection pool over hours in long-running daemon) 2. validateGatewayUrl: strip raw URL from TypeError message to prevent log injection via malformed URL strings 3. saveQQState/flushQQState: unlinkSync(tmpPath) in catch blocks to prevent orphaned .tmp files when renameSync fails (cross-device, Docker) --- packages/channels/qqbot/src/QQChannel.ts | 15 +++++++++++++-- packages/channels/qqbot/src/api.ts | 3 ++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index aa1e97ac085..52cecb27589 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -32,6 +32,7 @@ import { existsSync, mkdirSync, renameSync, + unlinkSync, } from 'node:fs'; import { join } from 'node:path'; import { OpCode, Intent } from './types.js'; @@ -381,8 +382,8 @@ export class QQChannel extends ChannelBase { if (this.disposed) return; if (this.saveTimer) clearTimeout(this.saveTimer); this.saveTimer = setTimeout(() => { + const tmpPath = this.qqStatePath + '.tmp'; try { - const tmpPath = this.qqStatePath + '.tmp'; writeFileSync( tmpPath, JSON.stringify({ @@ -394,6 +395,11 @@ export class QQChannel extends ChannelBase { ); renameSync(tmpPath, this.qqStatePath); } catch (e) { + try { + unlinkSync(tmpPath); + } catch { + /* best-effort */ + } process.stderr.write( `[QQ:${this.name}] saveQQState write failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, ); @@ -408,8 +414,8 @@ export class QQChannel extends ChannelBase { clearTimeout(this.saveTimer); this.saveTimer = null; } + const tmpPath = this.qqStatePath + '.tmp'; try { - const tmpPath = this.qqStatePath + '.tmp'; writeFileSync( tmpPath, JSON.stringify({ @@ -421,6 +427,11 @@ export class QQChannel extends ChannelBase { ); renameSync(tmpPath, this.qqStatePath); } catch (e) { + try { + unlinkSync(tmpPath); + } catch { + /* best-effort */ + } process.stderr.write( `[QQ:${this.name}] flushQQState write failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`, ); diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index e3be403e482..558715cd69a 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -35,6 +35,7 @@ export async function fetchAccessToken( }); if (!resp.ok) { + await resp.body?.cancel().catch(() => {}); process.stderr.write(`[QQ] Token request failed (HTTP ${resp.status})\n`); throw new Error(`QQ Bot token request failed (HTTP ${resp.status})`); } @@ -83,7 +84,7 @@ export function validateGatewayUrl(url: string): string { return clean.href; } catch (e) { if (e instanceof TypeError) { - throw new Error(`QQ Bot gateway URL is not a valid URL: ${url}`); + throw new Error('QQ Bot gateway URL is not a valid URL'); } throw e; } From eff008419cf706b54915f2ab52fef5eb635b47f4 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 11:22:26 +0800 Subject: [PATCH 16/21] fix(qqbot): test Infinity rejection via 1e999 raw JSON, not JSON.stringify nullification --- packages/channels/qqbot/src/send.test.ts | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index 7bf8b80ad5d..f44579a551e 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -1079,18 +1079,11 @@ describe('restoreQQState validation filters', () => { it('filters non-safe-integer msgSeqMap values (fractional, overflow, Infinity, -Infinity)', () => { vi.mocked(existsSync).mockReturnValue(true); // Number.MAX_SAFE_INTEGER + 1 = 9007199254740992 — loses precision - // 1e999 evaluates to Infinity in JavaScript — not a safe integer + // 1e999 / -1e999 are parsed as Infinity / -Infinity by JSON.parse + // Use raw JSON string: JSON.stringify(Infinity) → "null", which + // bypasses Number.isSafeInteger (caught by typeof check instead). vi.mocked(readFileSync).mockReturnValue( - JSON.stringify({ - msgSeqMap: [ - ['a', 1.5], - ['b', Number.MAX_SAFE_INTEGER + 1], - ['c', Infinity], - ['d', -Infinity], - ['e', 42], - ['f', 0], - ], - }), + '{"msgSeqMap":[["a",1.5],["b",9007199254740992],["c",1e999],["d",-1e999],["e",42],["f",0]]}', ); const ch = makeChannel(); From 9eff75f075240d65d62feaea453b52e7f787f3cc Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 11:59:50 +0800 Subject: [PATCH 17/21] =?UTF-8?q?fix(qqbot):=20address=20PR=20#6200=20revi?= =?UTF-8?q?ew=20=E2=80=94=20beforeExit=20hook,=20key=20validation,=20error?= =?UTF-8?q?=20clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add beforeExit hook to flush debounced state on abnormal process exit (SIGKILL, OOM, crash) when unref'd 500ms timer has pending writes - Validate map keys (typeof string, ≤256 chars) in restoreQQState filters alongside existing value validation to prevent non-string key bloat - Improve gateway hostname error message to include expected domain --- packages/channels/qqbot/src/QQChannel.ts | 33 +++++++++++++++++++++--- packages/channels/qqbot/src/api.ts | 2 +- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 52cecb27589..82f15de677d 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -135,6 +135,8 @@ export class QQChannel extends ChannelBase { private lastHeartbeatAck: number = 0; /** Debounce timer for saveQQState to avoid blocking event loop. */ private saveTimer: ReturnType | null = null; + /** beforeExit hook to flush state on abnormal process exit (unref'd timer bypass). */ + private beforeExitHook: (() => void) | null = null; /** Timer for reconnectWithRetry fallback (unref'd so it doesn't block exit). */ private reconnectTimer: ReturnType | null = null; /** Guard against parallel reconnectWithRetry chains from stale close events. */ @@ -206,6 +208,10 @@ export class QQChannel extends ChannelBase { try { await this.fetchToken(); await this.connectGateway(); + // Register beforeExit hook for abnormal exit (SIGKILL, OOM, crash) — the + // unref'd debounce timer may have 500ms of unflushed state at exit. + this.beforeExitHook = () => this.flushQQState(); + process.on('beforeExit', this.beforeExitHook); return; } catch (e: unknown) { if (attempt < 2) { @@ -341,6 +347,10 @@ export class QQChannel extends ChannelBase { clearTimeout(this.reconnectTimer); this.reconnectTimer = null; } + if (this.beforeExitHook) { + process.off('beforeExit', this.beforeExitHook); + this.beforeExitHook = null; + } this.flushQQState(); this.backupGlobalSessions(); if (this.ws) { @@ -452,7 +462,12 @@ export class QQChannel extends ChannelBase { const rawCT = raw.chatTypeMap as Array<[string, unknown]>; // Validate: only accept 'c2c' | 'group' values this.chatTypeMap = new Map( - rawCT.filter(([, v]) => v === 'c2c' || v === 'group'), + rawCT.filter( + ([k, v]) => + typeof k === 'string' && + k.length <= 256 && + (v === 'c2c' || v === 'group'), + ), ) as Map; const dropped = rawCT.length - this.chatTypeMap.size; if (dropped > 0) @@ -464,7 +479,13 @@ export class QQChannel extends ChannelBase { const rawRM = raw.replyMsgId as Array<[string, unknown]>; // Validate: entries must be strings ≤ 128 chars this.replyMsgId = new Map( - rawRM.filter(([, v]) => typeof v === 'string' && v.length <= 128), + rawRM.filter( + ([k, v]) => + typeof k === 'string' && + k.length <= 256 && + typeof v === 'string' && + v.length <= 128, + ), ) as Map; const dropped = rawRM.length - this.replyMsgId.size; if (dropped > 0) @@ -477,8 +498,12 @@ export class QQChannel extends ChannelBase { // Validate: entries must be non-negative safe integers this.msgSeqMap = new Map( rawMS.filter( - ([, v]) => - typeof v === 'number' && Number.isSafeInteger(v) && v >= 0, + ([k, v]) => + typeof k === 'string' && + k.length <= 256 && + typeof v === 'number' && + Number.isSafeInteger(v) && + v >= 0, ), ) as Map; const dropped = rawMS.length - this.msgSeqMap.size; diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index 558715cd69a..05440bf1fd1 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -75,7 +75,7 @@ export function validateGatewayUrl(url: string): string { // Hard reject: only allow documented QQ gateway hostnames if (!parsed.hostname.toLowerCase().endsWith('.qq.com')) { throw new Error( - `QQ Bot gateway URL has unexpected hostname: ${parsed.hostname}`, + `QQ Bot gateway URL has unexpected hostname: ${parsed.hostname} (expected *.qq.com)`, ); } const clean = new URL(url); From 2acf178def17b9c7bbd5d85fce904eb86bb47fd8 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 12:56:52 +0800 Subject: [PATCH 18/21] =?UTF-8?q?fix(qqbot):=20address=20PR=20#6200=20revi?= =?UTF-8?q?ew=20round=20=E2=80=94=20non-object=20JSON=20restore,=20dispose?= =?UTF-8?q?d=20guard,=20beforeExit=20dedup,=20comment=20fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/channels/qqbot/src/QQChannel.ts | 16 ++++-- packages/channels/qqbot/src/send.test.ts | 67 ++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 82f15de677d..12c164aae9e 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -208,8 +208,12 @@ export class QQChannel extends ChannelBase { try { await this.fetchToken(); await this.connectGateway(); - // Register beforeExit hook for abnormal exit (SIGKILL, OOM, crash) — the - // unref'd debounce timer may have 500ms of unflushed state at exit. + // Register beforeExit hook so the unref'd debounce timer's unflushed + // state is persisted when the event loop drains naturally. Does NOT + // fire for SIGKILL, OOM kills, or uncaughtException. + if (this.beforeExitHook) { + process.off('beforeExit', this.beforeExitHook); + } this.beforeExitHook = () => this.flushQQState(); process.on('beforeExit', this.beforeExitHook); return; @@ -392,7 +396,7 @@ export class QQChannel extends ChannelBase { if (this.disposed) return; if (this.saveTimer) clearTimeout(this.saveTimer); this.saveTimer = setTimeout(() => { - const tmpPath = this.qqStatePath + '.tmp'; + if (this.disposed) return; try { writeFileSync( tmpPath, @@ -458,6 +462,12 @@ export class QQChannel extends ChannelBase { try { if (!existsSync(this.qqStatePath)) return false; const raw = JSON.parse(readFileSync(this.qqStatePath, 'utf-8')); + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) { + process.stderr.write( + `[QQ:${this.name}] Invalid QQ state file (not an object), ignoring\n`, + ); + return false; + } if (raw.chatTypeMap && Array.isArray(raw.chatTypeMap)) { const rawCT = raw.chatTypeMap as Array<[string, unknown]>; // Validate: only accept 'c2c' | 'group' values diff --git a/packages/channels/qqbot/src/send.test.ts b/packages/channels/qqbot/src/send.test.ts index f44579a551e..7ec76a394de 100644 --- a/packages/channels/qqbot/src/send.test.ts +++ b/packages/channels/qqbot/src/send.test.ts @@ -1121,6 +1121,50 @@ describe('restoreQQState validation filters', () => { ).restoreQQState(); expect(result).toBe(false); }); + + it('returns false on non-object JSON (number)', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue('42'); + + const ch = makeChannel(); + const result = ( + ch as unknown as { restoreQQState: () => boolean } + ).restoreQQState(); + expect(result).toBe(false); + }); + + it('returns false on non-object JSON (string)', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue('"state"'); + + const ch = makeChannel(); + const result = ( + ch as unknown as { restoreQQState: () => boolean } + ).restoreQQState(); + expect(result).toBe(false); + }); + + it('returns false on non-object JSON (array)', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue('[1,2,3]'); + + const ch = makeChannel(); + const result = ( + ch as unknown as { restoreQQState: () => boolean } + ).restoreQQState(); + expect(result).toBe(false); + }); + + it('returns false on null JSON', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue('null'); + + const ch = makeChannel(); + const result = ( + ch as unknown as { restoreQQState: () => boolean } + ).restoreQQState(); + expect(result).toBe(false); + }); }); describe('atomic state persistence', () => { @@ -1208,4 +1252,27 @@ describe('atomic state persistence', () => { expect(chp.saveTimer).not.toBeNull(); expect(chp.saveTimer?.hasRef()).toBe(false); }); + + it('does not write state when disposed after timer is scheduled', () => { + vi.useFakeTimers(); + const ch = makeChannel(); + const chp = ch as unknown as { + saveQQState: () => void; + saveTimer: ReturnType | null; + }; + + // Schedule a save + chp.saveQQState(); + expect(chp.saveTimer).not.toBeNull(); + + // Mark disposed before the timer fires + (ch as unknown as { disposed: boolean }).disposed = true; + + // Advance time past debounce interval + vi.advanceTimersByTime(600); + + // The callback should have returned early due to disposed check + expect(writeFileSync).not.toHaveBeenCalled(); + vi.useRealTimers(); + }); }); From a9bd70c8d3fb4a1400efed33b2747947c27261b0 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 13:02:45 +0800 Subject: [PATCH 19/21] fix(qqbot): hoist tmpPath declaration before try block in saveQQState --- packages/channels/qqbot/src/QQChannel.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 12c164aae9e..8bd13667120 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -395,6 +395,7 @@ export class QQChannel extends ChannelBase { // sets disposed=true *before* calling it, so it must still write final state. if (this.disposed) return; if (this.saveTimer) clearTimeout(this.saveTimer); + const tmpPath = this.qqStatePath + '.tmp'; this.saveTimer = setTimeout(() => { if (this.disposed) return; try { From 548512a9c24f2efe3aaa1c14e4e543be2b352a35 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 13:05:24 +0800 Subject: [PATCH 20/21] fix(qqbot): update beforeExit JSDoc to accurately describe behavior --- packages/channels/qqbot/src/QQChannel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/channels/qqbot/src/QQChannel.ts b/packages/channels/qqbot/src/QQChannel.ts index 8bd13667120..b3c95d63e4f 100644 --- a/packages/channels/qqbot/src/QQChannel.ts +++ b/packages/channels/qqbot/src/QQChannel.ts @@ -135,7 +135,7 @@ export class QQChannel extends ChannelBase { private lastHeartbeatAck: number = 0; /** Debounce timer for saveQQState to avoid blocking event loop. */ private saveTimer: ReturnType | null = null; - /** beforeExit hook to flush state on abnormal process exit (unref'd timer bypass). */ + /** beforeExit hook to flush state when the event loop drains naturally. Does NOT fire for SIGKILL, OOM kills, or uncaughtException. */ private beforeExitHook: (() => void) | null = null; /** Timer for reconnectWithRetry fallback (unref'd so it doesn't block exit). */ private reconnectTimer: ReturnType | null = null; From 86615ea134ff7243741650b29c08bddf490ccd62 Mon Sep 17 00:00:00 2001 From: Eric-GoodBoy-Tech <634718930@qq.com> Date: Fri, 3 Jul 2026 13:26:08 +0800 Subject: [PATCH 21/21] fix(qqbot): drain response body in fetchGatewayUrl error path --- packages/channels/qqbot/src/api.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/channels/qqbot/src/api.ts b/packages/channels/qqbot/src/api.ts index 05440bf1fd1..110052b888a 100644 --- a/packages/channels/qqbot/src/api.ts +++ b/packages/channels/qqbot/src/api.ts @@ -106,6 +106,7 @@ export async function fetchGatewayUrl( }); if (!resp.ok) { + await resp.body?.cancel().catch(() => {}); throw new Error(`QQ Bot gateway request failed (HTTP ${resp.status})`); }