diff --git a/electron/utils/channel-config.ts b/electron/utils/channel-config.ts index 4f85ce055..18b11aea4 100644 --- a/electron/utils/channel-config.ts +++ b/electron/utils/channel-config.ts @@ -49,6 +49,10 @@ async function fileExists(p: string): Promise { try { await access(p, constants.F_OK); return true; } catch { return false; } } +function normalizeCredentialValue(value: string): string { + return value.trim(); +} + // ── Types ──────────────────────────────────────────────────────── export interface ChannelConfigData { @@ -350,7 +354,16 @@ function assertNoDuplicateCredential( if (!uniqueKey) return; const incomingValue = config[uniqueKey]; - if (!incomingValue || typeof incomingValue !== 'string') return; + if (typeof incomingValue !== 'string') return; + const normalizedIncomingValue = normalizeCredentialValue(incomingValue); + if (!normalizedIncomingValue) return; + if (normalizedIncomingValue !== incomingValue) { + logger.warn('Normalized channel credential value for duplicate check', { + channelType, + accountId: resolvedAccountId, + key: uniqueKey, + }); + } const accounts = channelSection.accounts as Record | undefined; if (!accounts) return; @@ -359,9 +372,12 @@ function assertNoDuplicateCredential( if (existingAccountId === resolvedAccountId) continue; if (!accountCfg || typeof accountCfg !== 'object') continue; const existingValue = accountCfg[uniqueKey]; - if (typeof existingValue === 'string' && existingValue === incomingValue) { + if ( + typeof existingValue === 'string' + && normalizeCredentialValue(existingValue) === normalizedIncomingValue + ) { throw new Error( - `The ${channelType} bot (${uniqueKey}: ${incomingValue}) is already bound to another agent (account: ${existingAccountId}). ` + + `The ${channelType} bot (${uniqueKey}: ${normalizedIncomingValue}) is already bound to another agent (account: ${existingAccountId}). ` + `Each agent must use a unique bot.`, ); } @@ -416,6 +432,19 @@ export async function saveChannelConfig( const existingAccountConfig = resolveAccountConfig(channelSection, resolvedAccountId); const transformedConfig = transformChannelConfig(channelType, config, existingAccountConfig); + const uniqueKey = CHANNEL_UNIQUE_CREDENTIAL_KEY[channelType]; + if (uniqueKey && typeof transformedConfig[uniqueKey] === 'string') { + const rawCredentialValue = transformedConfig[uniqueKey] as string; + const normalizedCredentialValue = normalizeCredentialValue(rawCredentialValue); + if (normalizedCredentialValue !== rawCredentialValue) { + logger.warn('Normalizing channel credential value before save', { + channelType, + accountId: resolvedAccountId, + key: uniqueKey, + }); + transformedConfig[uniqueKey] = normalizedCredentialValue; + } + } // Write credentials into accounts. if (!channelSection.accounts || typeof channelSection.accounts !== 'object') { diff --git a/electron/utils/openclaw-doctor.ts b/electron/utils/openclaw-doctor.ts index a53b183f2..66f6bed78 100644 --- a/electron/utils/openclaw-doctor.ts +++ b/electron/utils/openclaw-doctor.ts @@ -6,6 +6,7 @@ import { logger } from './logger'; import { getUvMirrorEnv } from './uv-env'; const OPENCLAW_DOCTOR_TIMEOUT_MS = 60_000; +const MAX_DOCTOR_OUTPUT_BYTES = 10 * 1024 * 1024; const OPENCLAW_DOCTOR_ARGS = ['doctor', '--json']; const OPENCLAW_DOCTOR_FIX_ARGS = ['doctor', '--fix', '--yes', '--non-interactive']; @@ -24,6 +25,39 @@ export interface OpenClawDoctorResult { error?: string; } +function appendDoctorOutput( + current: string, + currentBytes: number, + data: Buffer | string, + stream: 'stdout' | 'stderr', + alreadyTruncated: boolean, +): { output: string; bytes: number; truncated: boolean } { + if (alreadyTruncated) { + return { output: current, bytes: currentBytes, truncated: true }; + } + + const chunk = typeof data === 'string' ? Buffer.from(data) : data; + if (currentBytes + chunk.length <= MAX_DOCTOR_OUTPUT_BYTES) { + return { + output: current + chunk.toString(), + bytes: currentBytes + chunk.length, + truncated: false, + }; + } + + const remaining = Math.max(0, MAX_DOCTOR_OUTPUT_BYTES - currentBytes); + const appended = remaining > 0 ? chunk.subarray(0, remaining).toString() : ''; + logger.warn( + `OpenClaw doctor ${stream} exceeded ${MAX_DOCTOR_OUTPUT_BYTES} bytes; truncating additional output`, + ); + + return { + output: current + appended, + bytes: MAX_DOCTOR_OUTPUT_BYTES, + truncated: true, + }; +} + function getBundledBinPath(): string { const target = `${process.platform}-${process.arch}`; return app.isPackaged @@ -79,6 +113,10 @@ async function runDoctorCommand(mode: OpenClawDoctorMode): Promise) => { @@ -111,11 +149,17 @@ async function runDoctorCommand(mode: OpenClawDoctorMode): Promise { - stdout += data.toString(); + const next = appendDoctorOutput(stdout, stdoutBytes, data, 'stdout', stdoutTruncated); + stdout = next.output; + stdoutBytes = next.bytes; + stdoutTruncated = next.truncated; }); child.stderr?.on('data', (data) => { - stderr += data.toString(); + const next = appendDoctorOutput(stderr, stderrBytes, data, 'stderr', stderrTruncated); + stderr = next.output; + stderrBytes = next.bytes; + stderrTruncated = next.truncated; }); child.on('error', (error) => { diff --git a/tests/unit/channel-config.test.ts b/tests/unit/channel-config.test.ts new file mode 100644 index 000000000..6e9f961e3 --- /dev/null +++ b/tests/unit/channel-config.test.ts @@ -0,0 +1,104 @@ +import { readFile, rm } from 'fs/promises'; +import { join } from 'path'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const { testHome, testUserData, mockLoggerWarn, mockLoggerInfo, mockLoggerError } = vi.hoisted(() => { + const suffix = Math.random().toString(36).slice(2); + return { + testHome: `/tmp/clawx-channel-config-${suffix}`, + testUserData: `/tmp/clawx-channel-config-user-data-${suffix}`, + mockLoggerWarn: vi.fn(), + mockLoggerInfo: vi.fn(), + mockLoggerError: vi.fn(), + }; +}); + +vi.mock('os', async () => { + const actual = await vi.importActual('os'); + const mocked = { + ...actual, + homedir: () => testHome, + }; + return { + ...mocked, + default: mocked, + }; +}); + +vi.mock('electron', () => ({ + app: { + isPackaged: false, + getPath: () => testUserData, + getVersion: () => '0.0.0-test', + getAppPath: () => '/tmp', + }, +})); + +vi.mock('@electron/utils/logger', () => ({ + warn: mockLoggerWarn, + info: mockLoggerInfo, + error: mockLoggerError, +})); + +async function readOpenClawJson(): Promise> { + const content = await readFile(join(testHome, '.openclaw', 'openclaw.json'), 'utf8'); + return JSON.parse(content) as Record; +} + +describe('channel credential normalization and duplicate checks', () => { + beforeEach(async () => { + vi.resetAllMocks(); + vi.resetModules(); + await rm(testHome, { recursive: true, force: true }); + await rm(testUserData, { recursive: true, force: true }); + }); + + it('assertNoDuplicateCredential detects duplicates with different whitespace', async () => { + const { saveChannelConfig } = await import('@electron/utils/channel-config'); + + await saveChannelConfig('feishu', { appId: 'bot-123', appSecret: 'secret-a' }, 'agent-a'); + + await expect( + saveChannelConfig('feishu', { appId: ' bot-123 ', appSecret: 'secret-b' }, 'agent-b'), + ).rejects.toThrow('already bound to another agent'); + }); + + it('assertNoDuplicateCredential does NOT detect duplicates with different case', async () => { + // Case-sensitive credentials (like tokens) should NOT be normalized to lowercase + // to avoid false positives where different tokens become the same after lowercasing + const { saveChannelConfig } = await import('@electron/utils/channel-config'); + + await saveChannelConfig('feishu', { appId: 'Bot-ABC', appSecret: 'secret-a' }, 'agent-a'); + + // Should NOT throw - different case is considered a different credential + await expect( + saveChannelConfig('feishu', { appId: 'bot-abc', appSecret: 'secret-b' }, 'agent-b'), + ).resolves.not.toThrow(); + }); + + it('normalizes credential values when saving (trim only, preserve case)', async () => { + const { saveChannelConfig } = await import('@electron/utils/channel-config'); + + await saveChannelConfig('feishu', { appId: ' BoT-XyZ ', appSecret: 'secret' }, 'agent-a'); + + const config = await readOpenClawJson(); + const channels = config.channels as Record }>; + // Should trim whitespace but preserve original case + expect(channels.feishu.accounts['agent-a'].appId).toBe('BoT-XyZ'); + }); + + it('emits warning logs when credential normalization (trim) occurs', async () => { + const { saveChannelConfig } = await import('@electron/utils/channel-config'); + + await saveChannelConfig('feishu', { appId: ' BoT-Log ', appSecret: 'secret' }, 'agent-a'); + + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Normalized channel credential value for duplicate check', + expect.objectContaining({ channelType: 'feishu', accountId: 'agent-a', key: 'appId' }), + ); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Normalizing channel credential value before save', + expect.objectContaining({ channelType: 'feishu', accountId: 'agent-a', key: 'appId' }), + ); + }); +}); diff --git a/tests/unit/openclaw-doctor.test.ts b/tests/unit/openclaw-doctor.test.ts new file mode 100644 index 000000000..fc0ef5345 --- /dev/null +++ b/tests/unit/openclaw-doctor.test.ts @@ -0,0 +1,158 @@ +import { EventEmitter } from 'node:events'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const MAX_DOCTOR_OUTPUT_BYTES = 10 * 1024 * 1024; + +const { + mockExistsSync, + mockFork, + mockGetUvMirrorEnv, + mockLoggerWarn, + mockLoggerInfo, + mockLoggerError, +} = vi.hoisted(() => ({ + mockExistsSync: vi.fn(), + mockFork: vi.fn(), + mockGetUvMirrorEnv: vi.fn(), + mockLoggerWarn: vi.fn(), + mockLoggerInfo: vi.fn(), + mockLoggerError: vi.fn(), +})); + +vi.mock('node:fs', async () => { + const actual = await vi.importActual('node:fs'); + return { + ...actual, + existsSync: mockExistsSync, + default: { + ...actual, + existsSync: mockExistsSync, + }, + }; +}); + +vi.mock('electron', () => ({ + app: { + isPackaged: false, + }, + utilityProcess: { + fork: mockFork, + }, +})); + +vi.mock('@electron/utils/paths', () => ({ + getOpenClawDir: () => '/tmp/openclaw', + getOpenClawEntryPath: () => '/tmp/openclaw/openclaw-entry.js', +})); + +vi.mock('@electron/utils/uv-env', () => ({ + getUvMirrorEnv: mockGetUvMirrorEnv, +})); + +vi.mock('@electron/utils/logger', () => ({ + logger: { + warn: mockLoggerWarn, + info: mockLoggerInfo, + error: mockLoggerError, + }, +})); + +class MockUtilityChild extends EventEmitter { + stdout = new EventEmitter(); + stderr = new EventEmitter(); + kill = vi.fn(); +} + +describe('openclaw doctor output handling', () => { + beforeEach(() => { + vi.resetAllMocks(); + vi.resetModules(); + + mockExistsSync.mockReturnValue(true); + mockGetUvMirrorEnv.mockResolvedValue({}); + }); + + it('collects normal output under the buffer limit', async () => { + const child = new MockUtilityChild(); + mockFork.mockReturnValue(child); + + const { runOpenClawDoctor } = await import('@electron/utils/openclaw-doctor'); + const resultPromise = runOpenClawDoctor(); + + await vi.waitFor(() => { + expect(mockFork).toHaveBeenCalledTimes(1); + }); + child.stdout.emit('data', Buffer.from('doctor ok\n')); + child.emit('exit', 0); + + const result = await resultPromise; + expect(result.success).toBe(true); + expect(result.exitCode).toBe(0); + expect(result.stdout).toBe('doctor ok\n'); + expect(result.stderr).toBe(''); + expect(mockLoggerWarn).not.toHaveBeenCalled(); + }); + + it('truncates output when stdout exceeds MAX_DOCTOR_OUTPUT_BYTES', async () => { + const child = new MockUtilityChild(); + mockFork.mockReturnValue(child); + + const { runOpenClawDoctor } = await import('@electron/utils/openclaw-doctor'); + const resultPromise = runOpenClawDoctor(); + + await vi.waitFor(() => { + expect(mockFork).toHaveBeenCalledTimes(1); + }); + child.stdout.emit('data', Buffer.from('a'.repeat(MAX_DOCTOR_OUTPUT_BYTES - 5))); + child.stdout.emit('data', Buffer.from('b'.repeat(10))); + child.stdout.emit('data', Buffer.from('c'.repeat(1000))); + child.emit('exit', 0); + + const result = await resultPromise; + expect(result.stdout.length).toBe(MAX_DOCTOR_OUTPUT_BYTES); + expect(result.stdout.endsWith('bbbbb')).toBe(true); + expect(result.stdout.includes('c')).toBe(false); + }); + + it('logs a warning when truncation occurs', async () => { + const child = new MockUtilityChild(); + mockFork.mockReturnValue(child); + + const { runOpenClawDoctor } = await import('@electron/utils/openclaw-doctor'); + const resultPromise = runOpenClawDoctor(); + + await vi.waitFor(() => { + expect(mockFork).toHaveBeenCalledTimes(1); + }); + child.stdout.emit('data', Buffer.from('x'.repeat(MAX_DOCTOR_OUTPUT_BYTES + 1))); + child.emit('exit', 0); + + await resultPromise; + expect(mockLoggerWarn).toHaveBeenCalledWith( + `OpenClaw doctor stdout exceeded ${MAX_DOCTOR_OUTPUT_BYTES} bytes; truncating additional output`, + ); + }); + + it('collects stdout and stderr independently', async () => { + const child = new MockUtilityChild(); + mockFork.mockReturnValue(child); + + const { runOpenClawDoctor } = await import('@electron/utils/openclaw-doctor'); + const resultPromise = runOpenClawDoctor(); + + await vi.waitFor(() => { + expect(mockFork).toHaveBeenCalledTimes(1); + }); + child.stdout.emit('data', Buffer.from('line-1\n')); + child.stderr.emit('data', Buffer.from('warn-1\n')); + child.stdout.emit('data', Buffer.from('line-2\n')); + child.stderr.emit('data', Buffer.from('warn-2\n')); + child.emit('exit', 1); + + const result = await resultPromise; + expect(result.success).toBe(false); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe('line-1\nline-2\n'); + expect(result.stderr).toBe('warn-1\nwarn-2\n'); + }); +});