fix(qqbot): security hardening — gateway validation, atomic state, sanitized logging

packages/channels/qqbot/src/QQChannel.ts

@@ -26,7 +26,14 @@ 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,
+  unlinkSync,
+} from 'node:fs';
 import { join } from 'node:path';
 import { OpCode, Intent } from './types.js';
 import type {
@@ -128,6 +135,8 @@ export class QQChannel extends ChannelBase {
   private lastHeartbeatAck: number = 0;
   /** Debounce timer for saveQQState to avoid blocking event loop. */
   private saveTimer: ReturnType<typeof setTimeout> | null = null;
+  /** 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<typeof setTimeout> | null = null;
   /** Guard against parallel reconnectWithRetry chains from stale close events. */
@@ -199,16 +208,30 @@ export class QQChannel extends ChannelBase {
       try {
         await this.fetchToken();
         await this.connectGateway();
+        // 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;
       } catch (e: unknown) {
         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 {
-          throw e;
+          // 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 },
+          );
         }
       }
     }
@@ -253,7 +276,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,
@@ -275,14 +298,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;
       }
     }
@@ -326,6 +351,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) {
@@ -362,22 +391,36 @@ 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);
+    const tmpPath = this.qqStatePath + '.tmp';
     this.saveTimer = setTimeout(() => {
+      if (this.disposed) return;
       try {
         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) {
+        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`,
+        );
       }
     }, 500);
+    this.saveTimer.unref();
   }
 
   /** Flush pending state writes immediately (called on disconnect). */
@@ -386,37 +429,104 @@ export class QQChannel extends ChannelBase {
       clearTimeout(this.saveTimer);
       this.saveTimer = null;
     }
+    const tmpPath = this.qqStatePath + '.tmp';
     try {
       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) {
+      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`,
       );
-    } catch {
-      /* best-effort */
     }
   }
 
   /**
    * 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 {
       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 (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
+        this.chatTypeMap = new Map(
+          rawCT.filter(
+            ([k, v]) =>
+              typeof k === 'string' &&
+              k.length <= 256 &&
+              (v === 'c2c' || v === 'group'),
+          ),
+        ) as Map<string, 'c2c' | 'group'>;
+        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 && Array.isArray(raw.replyMsgId)) {
+        const rawRM = raw.replyMsgId as Array<[string, unknown]>;
+        // Validate: entries must be strings ≤ 128 chars
+        this.replyMsgId = new Map(
+          rawRM.filter(
+            ([k, v]) =>
+              typeof k === 'string' &&
+              k.length <= 256 &&
+              typeof v === 'string' &&
+              v.length <= 128,
+          ),
+        ) as Map<string, string>;
+        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 && Array.isArray(raw.msgSeqMap)) {
+        const rawMS = raw.msgSeqMap as Array<[string, unknown]>;
+        // Validate: entries must be non-negative safe integers
+        this.msgSeqMap = new Map(
+          rawMS.filter(
+            ([k, v]) =>
+              typeof k === 'string' &&
+              k.length <= 256 &&
+              typeof v === 'number' &&
+              Number.isSafeInteger(v) &&
+              v >= 0,
+          ),
+        ) as Map<string, number>;
+        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) {
       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 +659,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 +674,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 +721,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 +790,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 +965,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 +1056,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 +1127,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`,
+      ),
     );
   }
 }