Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
43b3fd0
fix(qqbot): validate gateway URL protocol to prevent SSRF
Eric-GoodBoy-Tech Jul 2, 2026
9c248ba
fix(qqbot): atomic state persistence and error log sanitization
Eric-GoodBoy-Tech Jul 2, 2026
8774eb9
fix(qqbot): add replyMsgId validation in restoreQQState()
Eric-GoodBoy-Tech Jul 2, 2026
9786cd9
fix(qqbot): add test coverage for restoreQQState filters, atomic writ…
Eric-GoodBoy-Tech Jul 2, 2026
46910de
docs(qqbot): update restoreQQState doc and add disposed guard comment
Eric-GoodBoy-Tech Jul 3, 2026
3ec3877
docs(qqbot): update validateGatewayUrl docstring to reflect TLS enfor…
Eric-GoodBoy-Tech Jul 3, 2026
231f7c7
fix(qqbot): address wenshao review — hostname rejection, error saniti…
Eric-GoodBoy-Tech Jul 3, 2026
ebd6f30
fix(qqbot): address PR review — drain body, narrow gateway hostname
Eric-GoodBoy-Tech Jul 3, 2026
2ccbfe6
test(qqbot): tighten token-error assertion to exact match
Eric-GoodBoy-Tech Jul 3, 2026
14ea726
test(qqbot): add connect retry sanitization + msgSeqMap edge-case tests
Eric-GoodBoy-Tech Jul 3, 2026
a1367e0
fix(qqbot): address wenshao review round 3 — error preservation, vali…
Eric-GoodBoy-Tech Jul 3, 2026
3e8f713
test(qqbot): fix connect retry sanitization assertion — sanitizeLogTe…
Eric-GoodBoy-Tech Jul 3, 2026
707b752
fix(qqbot): address wenshao review — error preservation, validation l…
Eric-GoodBoy-Tech Jul 3, 2026
483305e
fix(qqbot): add Array.isArray guards and replyMsgId drop logging in r…
Eric-GoodBoy-Tech Jul 3, 2026
3da730f
fix(qqbot): drain response body in token error path, sanitize URL Typ…
Eric-GoodBoy-Tech Jul 3, 2026
eff0084
fix(qqbot): test Infinity rejection via 1e999 raw JSON, not JSON.stri…
Eric-GoodBoy-Tech Jul 3, 2026
9eff75f
fix(qqbot): address PR #6200 review — beforeExit hook, key validation…
Eric-GoodBoy-Tech Jul 3, 2026
2acf178
fix(qqbot): address PR #6200 review round — non-object JSON restore, …
Eric-GoodBoy-Tech Jul 3, 2026
a9bd70c
fix(qqbot): hoist tmpPath declaration before try block in saveQQState
Eric-GoodBoy-Tech Jul 3, 2026
548512a
fix(qqbot): update beforeExit JSDoc to accurately describe behavior
Eric-GoodBoy-Tech Jul 3, 2026
86615ea
fix(qqbot): drain response body in fetchGatewayUrl error path
Eric-GoodBoy-Tech Jul 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 68 additions & 22 deletions packages/channels/qqbot/src/QQChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<string, unknown> = {
content: chunk,
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -362,22 +370,28 @@ export class QQChannel extends ChannelBase {

/** Debounced state persistence to avoid blocking event loop. */
private saveQQState(): void {
if (this.disposed) return;
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
if (this.saveTimer) clearTimeout(this.saveTimer);
this.saveTimer = setTimeout(() => {
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
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 },
);
} catch {
/* best-effort */
renameSync(tmpPath, this.qqStatePath);
} catch (e) {
process.stderr.write(
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
`[QQ:${this.name}] saveQQState write failed: ${sanitizeLogText(e instanceof Error ? e.message : String(e), 200)}\n`,
);
}
}, 500);
this.saveTimer.unref();
}
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved

/** Flush pending state writes immediately (called on disconnect). */
Expand All @@ -387,16 +401,21 @@ export class QQChannel extends ChannelBase {
this.saveTimer = null;
}
try {
const tmpPath = this.qqStatePath + '.tmp';
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
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 */
}
}

Expand All @@ -410,13 +429,34 @@ 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.replyMsgId) this.replyMsgId = new Map(raw.replyMsgId);
if (raw.msgSeqMap) this.msgSeqMap = new Map(raw.msgSeqMap);
if (raw.chatTypeMap) {
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
// Validate: only accept 'c2c' | 'group' values
this.chatTypeMap = new Map(
(raw.chatTypeMap as Array<[string, unknown]>).filter(
([, v]) => v === 'c2c' || v === 'group',
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
),
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
) as Map<string, 'c2c' | 'group'>;
}
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<string, string>;
}
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,
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
),
) as Map<string, number>;
}
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;
}
Expand Down Expand Up @@ -549,7 +589,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();
});
Expand All @@ -564,7 +604,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();
});
Expand Down Expand Up @@ -611,7 +651,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`,
);
}
});
Expand Down Expand Up @@ -680,7 +720,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);
}
Expand Down Expand Up @@ -853,7 +895,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);
}
Expand Down Expand Up @@ -944,7 +986,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`,
),
);
}

Expand Down Expand Up @@ -1013,7 +1057,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`,
),
);
}
}
92 changes: 90 additions & 2 deletions packages/channels/qqbot/src/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -190,3 +195,86 @@ describe('fetchGatewayUrl', () => {
);
});
});

Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
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',
);
});
});

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',
);
});
});
37 changes: 35 additions & 2 deletions packages/channels/qqbot/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`,
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
);
}

Expand All @@ -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 (
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
!hostname.endsWith('.qq.com') &&
!hostname.endsWith('.tencent.com') &&
!hostname.endsWith('.tencentcs.com')
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
) {
process.stderr.write(
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
`[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}`);
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
}
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
throw e;
Comment thread
Eric-GoodBoy-Tech marked this conversation as resolved.
Resolved
}
}

/**
* Resolve the WebSocket Gateway URL.
* Throws on HTTP errors or missing URL in the response.
Expand All @@ -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. */
Expand Down
Loading
Loading