fix(channels): address code review issues
- Fix double useChannels() instantiation: call once in ChannelsPage,
pass as props to TelegramCard
- Mask bot tokens in channels:getConfig before sending to renderer
- Add input validation (isValidId, token length) on all IPC handlers
- Fix stopAccount() to clean up typingTimer, lastRoute, aggregator,
and debouncer when stopping the account they belong to
- Add try/catch to stopChannel/startChannel in useChannels hook
- Consistent return type { ok, error? } on channels:stop handler
- Add tooltip hint on disabled Remove button
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c99675b6e4
commit
43d11a6e5d
5 changed files with 95 additions and 18 deletions
2
apps/desktop/electron/electron-env.d.ts
vendored
2
apps/desktop/electron/electron-env.d.ts
vendored
|
|
@ -202,7 +202,7 @@ interface ElectronAPI {
|
|||
getConfig: () => Promise<Record<string, Record<string, Record<string, unknown>> | undefined>>
|
||||
saveToken: (channelId: string, accountId: string, token: string) => Promise<{ ok: boolean; error?: string }>
|
||||
removeToken: (channelId: string, accountId: string) => Promise<{ ok: boolean; error?: string }>
|
||||
stop: (channelId: string, accountId: string) => Promise<{ ok: boolean }>
|
||||
stop: (channelId: string, accountId: string) => Promise<{ ok: boolean; error?: string }>
|
||||
start: (channelId: string, accountId: string) => Promise<{ ok: boolean; error?: string }>
|
||||
}
|
||||
localChat: {
|
||||
|
|
|
|||
|
|
@ -10,6 +10,21 @@ import { getCurrentHub } from './hub.js'
|
|||
import { credentialManager } from '../../../../src/agent/credentials.js'
|
||||
import { listChannels } from '../../../../src/channels/registry.js'
|
||||
|
||||
/** Validate that a string is a safe identifier (alphanumeric, dashes, underscores) */
|
||||
function isValidId(value: unknown): value is string {
|
||||
return typeof value === 'string' && /^[a-zA-Z0-9_-]+$/.test(value) && value.length <= 64
|
||||
}
|
||||
|
||||
/**
|
||||
* Mask a token string for safe display: show first 5 and last 5 chars.
|
||||
* Returns undefined if the input is not a string.
|
||||
*/
|
||||
function maskToken(token: unknown): string | undefined {
|
||||
if (typeof token !== 'string' || token.length === 0) return undefined
|
||||
if (token.length <= 12) return '*'.repeat(token.length)
|
||||
return `${token.slice(0, 5)}${'*'.repeat(10)}${token.slice(-5)}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Register all Channel-related IPC handlers.
|
||||
*/
|
||||
|
|
@ -25,20 +40,42 @@ export function registerChannelsIpcHandlers(): void {
|
|||
|
||||
/**
|
||||
* Get the channels config from credentials.json5.
|
||||
* Returns the raw `channels` section: { telegram: { default: { botToken: "..." } } }
|
||||
* Returns a sanitized version with tokens masked (not the raw secret values).
|
||||
*/
|
||||
ipcMain.handle('channels:getConfig', async () => {
|
||||
return credentialManager.getChannelsConfig()
|
||||
const raw = credentialManager.getChannelsConfig()
|
||||
// Mask secret values before sending to renderer
|
||||
const masked: Record<string, Record<string, Record<string, unknown>> | undefined> = {}
|
||||
for (const [channelId, accounts] of Object.entries(raw)) {
|
||||
if (!accounts) continue
|
||||
const maskedAccounts: Record<string, Record<string, unknown>> = {}
|
||||
for (const [accountId, accountConfig] of Object.entries(accounts)) {
|
||||
const maskedConfig = { ...accountConfig }
|
||||
if ('botToken' in maskedConfig) {
|
||||
maskedConfig.botToken = maskToken(maskedConfig.botToken)
|
||||
}
|
||||
maskedAccounts[accountId] = maskedConfig
|
||||
}
|
||||
masked[channelId] = maskedAccounts
|
||||
}
|
||||
return masked
|
||||
})
|
||||
|
||||
/**
|
||||
* Save a channel account token and start the bot immediately.
|
||||
* Flow: write to credentials.json5 → start the channel account.
|
||||
* Flow: validate → write to credentials.json5 → start the channel account.
|
||||
*/
|
||||
ipcMain.handle(
|
||||
'channels:saveToken',
|
||||
async (_event, channelId: string, accountId: string, token: string): Promise<{ ok: boolean; error?: string }> => {
|
||||
try {
|
||||
// Validate inputs
|
||||
if (!isValidId(channelId)) return { ok: false, error: 'Invalid channel ID' }
|
||||
if (!isValidId(accountId)) return { ok: false, error: 'Invalid account ID' }
|
||||
if (typeof token !== 'string' || token.trim().length === 0 || token.length > 256) {
|
||||
return { ok: false, error: 'Invalid token' }
|
||||
}
|
||||
|
||||
const hub = getCurrentHub()
|
||||
if (!hub) return { ok: false, error: 'Hub not initialized' }
|
||||
|
||||
|
|
@ -73,6 +110,9 @@ export function registerChannelsIpcHandlers(): void {
|
|||
'channels:removeToken',
|
||||
async (_event, channelId: string, accountId: string): Promise<{ ok: boolean; error?: string }> => {
|
||||
try {
|
||||
if (!isValidId(channelId)) return { ok: false, error: 'Invalid channel ID' }
|
||||
if (!isValidId(accountId)) return { ok: false, error: 'Invalid account ID' }
|
||||
|
||||
const hub = getCurrentHub()
|
||||
if (!hub) return { ok: false, error: 'Hub not initialized' }
|
||||
|
||||
|
|
@ -97,9 +137,11 @@ export function registerChannelsIpcHandlers(): void {
|
|||
*/
|
||||
ipcMain.handle(
|
||||
'channels:stop',
|
||||
async (_event, channelId: string, accountId: string): Promise<{ ok: boolean }> => {
|
||||
async (_event, channelId: string, accountId: string): Promise<{ ok: boolean; error?: string }> => {
|
||||
if (!isValidId(channelId)) return { ok: false, error: 'Invalid channel ID' }
|
||||
if (!isValidId(accountId)) return { ok: false, error: 'Invalid account ID' }
|
||||
const hub = getCurrentHub()
|
||||
if (!hub) return { ok: false }
|
||||
if (!hub) return { ok: false, error: 'Hub not initialized' }
|
||||
hub.channelManager.stopAccount(channelId, accountId)
|
||||
return { ok: true }
|
||||
}
|
||||
|
|
@ -112,6 +154,9 @@ export function registerChannelsIpcHandlers(): void {
|
|||
'channels:start',
|
||||
async (_event, channelId: string, accountId: string): Promise<{ ok: boolean; error?: string }> => {
|
||||
try {
|
||||
if (!isValidId(channelId)) return { ok: false, error: 'Invalid channel ID' }
|
||||
if (!isValidId(accountId)) return { ok: false, error: 'Invalid account ID' }
|
||||
|
||||
const hub = getCurrentHub()
|
||||
if (!hub) return { ok: false, error: 'Hub not initialized' }
|
||||
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@
|
|||
*/
|
||||
import { useState, useEffect, useCallback } from 'react'
|
||||
|
||||
interface UseChannelsReturn {
|
||||
export interface UseChannelsReturn {
|
||||
/** Runtime states of all channel accounts */
|
||||
states: ChannelAccountStateInfo[]
|
||||
/** Raw channel config from credentials.json5 */
|
||||
|
|
@ -94,17 +94,31 @@ export function useChannels(): UseChannelsReturn {
|
|||
}, [refresh])
|
||||
|
||||
const stopChannel = useCallback(async (channelId: string, accountId: string) => {
|
||||
await window.electronAPI.channels.stop(channelId, accountId)
|
||||
await refresh()
|
||||
setError(null)
|
||||
try {
|
||||
const result = await window.electronAPI.channels.stop(channelId, accountId)
|
||||
if (!result.ok) {
|
||||
setError(result.error ?? 'Failed to stop channel')
|
||||
}
|
||||
await refresh()
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
setError(message)
|
||||
}
|
||||
}, [refresh])
|
||||
|
||||
const startChannel = useCallback(async (channelId: string, accountId: string) => {
|
||||
setError(null)
|
||||
const result = await window.electronAPI.channels.start(channelId, accountId)
|
||||
if (!result.ok) {
|
||||
setError(result.error ?? 'Failed to start channel')
|
||||
try {
|
||||
const result = await window.electronAPI.channels.start(channelId, accountId)
|
||||
if (!result.ok) {
|
||||
setError(result.error ?? 'Failed to start channel')
|
||||
}
|
||||
await refresh()
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
setError(message)
|
||||
}
|
||||
await refresh()
|
||||
}, [refresh])
|
||||
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import {
|
|||
import { Button } from '@multica/ui/components/ui/button'
|
||||
import { Input } from '@multica/ui/components/ui/input'
|
||||
import { Badge } from '@multica/ui/components/ui/badge'
|
||||
import { useChannels } from '../hooks/use-channels'
|
||||
import { useChannels, type UseChannelsReturn } from '../hooks/use-channels'
|
||||
|
||||
/** Status badge color mapping */
|
||||
function statusVariant(status: string): 'default' | 'secondary' | 'destructive' | 'outline' {
|
||||
|
|
@ -21,8 +21,8 @@ function statusVariant(status: string): 'default' | 'secondary' | 'destructive'
|
|||
}
|
||||
}
|
||||
|
||||
function TelegramCard() {
|
||||
const { states, config, saveToken, removeToken, startChannel, stopChannel } = useChannels()
|
||||
function TelegramCard({ channels }: { channels: UseChannelsReturn }) {
|
||||
const { states, config, saveToken, removeToken, startChannel, stopChannel } = channels
|
||||
const [token, setToken] = useState('')
|
||||
const [saving, setSaving] = useState(false)
|
||||
const [localError, setLocalError] = useState<string | null>(null)
|
||||
|
|
@ -118,6 +118,7 @@ function TelegramCard() {
|
|||
size="sm"
|
||||
onClick={handleRemove}
|
||||
disabled={saving || isRunning}
|
||||
title={isRunning ? 'Stop the bot before removing' : undefined}
|
||||
>
|
||||
Remove
|
||||
</Button>
|
||||
|
|
@ -152,7 +153,8 @@ function TelegramCard() {
|
|||
}
|
||||
|
||||
export default function ChannelsPage() {
|
||||
const { loading, error } = useChannels()
|
||||
const channels = useChannels()
|
||||
const { loading, error } = channels
|
||||
|
||||
return (
|
||||
<div className="max-w-4xl mx-auto space-y-4">
|
||||
|
|
@ -168,7 +170,7 @@ export default function ChannelsPage() {
|
|||
) : error ? (
|
||||
<p className="text-sm text-destructive">{error}</p>
|
||||
) : (
|
||||
<TelegramCard />
|
||||
<TelegramCard channels={channels} />
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -439,15 +439,31 @@ export class ChannelManager {
|
|||
/**
|
||||
* Stop a specific channel account.
|
||||
* Public so the desktop IPC layer can call it when removing config.
|
||||
* Cleans up typing timer, debouncer, aggregator, and lastRoute if they
|
||||
* belong to this account.
|
||||
*/
|
||||
stopAccount(channelId: string, accountId: string): void {
|
||||
const key = `${channelId}:${accountId}`;
|
||||
const handle = this.accounts.get(key);
|
||||
if (!handle) return;
|
||||
|
||||
// Clean up shared resources if they target this account
|
||||
if (this.lastRoute && this.lastRoute.plugin.id === channelId && this.lastRoute.deliveryCtx.accountId === accountId) {
|
||||
this.stopTyping();
|
||||
this.lastRoute = null;
|
||||
this.aggregator = null;
|
||||
}
|
||||
|
||||
handle.abortController.abort();
|
||||
handle.state = { ...handle.state, status: "stopped" };
|
||||
this.accounts.delete(key);
|
||||
|
||||
// Dispose debouncer if no accounts remain
|
||||
if (this.accounts.size === 0 && this.debouncer) {
|
||||
this.debouncer.dispose();
|
||||
this.debouncer = null;
|
||||
}
|
||||
|
||||
console.log(`[Channels] Stopped ${key}`);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue