mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-17 13:13:35 +08:00
fix(desktop): persist settings tab and redact mcp secrets
Batch J follows the v0.4.3 validation pass. It closes the remaining Settings tab persistence gap after renderer recreation and prevents MCP settings from exposing sensitive command args/env/header values in visible UI or input values while preserving unchanged raw values on save. Tested: cd desktop && bun run test -- --run src/stores/uiStore.test.ts src/__tests__/mcpSettings.test.tsx Tested: cd desktop && bun run lint Tested: bun run check:desktop Not-tested: live Electron MCP settings smoke after restart; trace header revalidation is deferred because inspected trace files predate the header fix. Confidence: high Scope-risk: narrow
This commit is contained in:
parent
2e6fe17b58
commit
be64e16621
@ -214,6 +214,138 @@ describe('McpSettings', () => {
|
||||
expect(screen.getByText('global-user')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('redacts sensitive MCP command details from the list and details views', async () => {
|
||||
const server = {
|
||||
name: 'context7',
|
||||
scope: 'local',
|
||||
transport: 'stdio',
|
||||
enabled: true,
|
||||
status: 'connected',
|
||||
statusLabel: 'Connected',
|
||||
configLocation: '/workspace/project/.mcp.json',
|
||||
summary: 'npx context7 --api-key sk-summary-secret',
|
||||
canEdit: false,
|
||||
canRemove: false,
|
||||
canReconnect: true,
|
||||
canToggle: true,
|
||||
projectPath: '/workspace/project',
|
||||
config: {
|
||||
type: 'stdio',
|
||||
command: 'npx',
|
||||
args: ['context7', '--api-key', 'sk-argument-secret'],
|
||||
env: { CONTEXT7_API_KEY: 'sk-env-secret' },
|
||||
},
|
||||
} as const
|
||||
|
||||
useMcpStore.setState({ servers: [server] })
|
||||
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
expect(document.body.textContent).not.toContain('sk-summary-secret')
|
||||
expect(document.body.textContent).not.toContain('sk-argument-secret')
|
||||
expect(document.body.textContent).not.toContain('sk-env-secret')
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open context7' }))
|
||||
})
|
||||
|
||||
expect(document.body.textContent).not.toContain('sk-summary-secret')
|
||||
expect(document.body.textContent).not.toContain('sk-argument-secret')
|
||||
expect(document.body.textContent).not.toContain('sk-env-secret')
|
||||
})
|
||||
|
||||
it('redacts editable MCP secrets without replacing unchanged values on save', async () => {
|
||||
const server = {
|
||||
name: 'context7',
|
||||
scope: 'local',
|
||||
transport: 'stdio',
|
||||
enabled: true,
|
||||
status: 'connected',
|
||||
statusLabel: 'Connected',
|
||||
configLocation: '/workspace/project/.mcp.json',
|
||||
summary: 'npx context7 --api-key sk-summary-edit-secret',
|
||||
canEdit: true,
|
||||
canRemove: true,
|
||||
canReconnect: true,
|
||||
canToggle: true,
|
||||
projectPath: '/workspace/project',
|
||||
config: {
|
||||
type: 'stdio',
|
||||
command: 'npx',
|
||||
args: ['context7', '--api-key', 'sk-argument-edit-secret'],
|
||||
env: {
|
||||
CONTEXT7_API_KEY: 'sk-env-edit-secret',
|
||||
LOG_LEVEL: 'debug',
|
||||
},
|
||||
},
|
||||
} as const
|
||||
const updateServer = vi.fn().mockResolvedValue(server)
|
||||
|
||||
useMcpStore.setState({ servers: [server], updateServer })
|
||||
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
expect(document.body.textContent).not.toContain('sk-summary-edit-secret')
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open context7' }))
|
||||
})
|
||||
|
||||
expect(document.body.textContent).not.toContain('sk-summary-edit-secret')
|
||||
expect(document.body.textContent).not.toContain('sk-argument-edit-secret')
|
||||
expect(document.body.textContent).not.toContain('sk-env-edit-secret')
|
||||
expect(screen.queryByDisplayValue('sk-argument-edit-secret')).not.toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('sk-env-edit-secret')).not.toBeInTheDocument()
|
||||
expect(screen.getByDisplayValue('debug')).toBeInTheDocument()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Save' }))
|
||||
})
|
||||
|
||||
expect(updateServer).toHaveBeenCalledWith(
|
||||
server,
|
||||
{
|
||||
scope: 'local',
|
||||
config: {
|
||||
type: 'stdio',
|
||||
command: 'npx',
|
||||
args: ['context7', '--api-key', 'sk-argument-edit-secret'],
|
||||
env: {
|
||||
CONTEXT7_API_KEY: 'sk-env-edit-secret',
|
||||
LOG_LEVEL: 'debug',
|
||||
},
|
||||
},
|
||||
},
|
||||
'/workspace/project',
|
||||
)
|
||||
})
|
||||
|
||||
it('uses the server MCP name rules before enabling Save', async () => {
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: /add server/i }))
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: /^User/i }))
|
||||
})
|
||||
|
||||
fireEvent.change(screen.getByLabelText(/Name/), { target: { value: 'bad name' } })
|
||||
fireEvent.change(screen.getByLabelText(/Command to launch/), { target: { value: 'echo' } })
|
||||
|
||||
expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled()
|
||||
|
||||
fireEvent.change(screen.getByLabelText(/Name/), { target: { value: 'bad/name' } })
|
||||
expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled()
|
||||
|
||||
fireEvent.change(screen.getByLabelText(/Name/), { target: { value: 'bad.name' } })
|
||||
expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled()
|
||||
|
||||
fireEvent.change(screen.getByLabelText(/Name/), { target: { value: '某某服务器' } })
|
||||
expect(screen.getByRole('button', { name: 'Save' })).toBeEnabled()
|
||||
})
|
||||
|
||||
it('keeps same-name project MCP servers distinct by project path', async () => {
|
||||
useMcpStore.setState({
|
||||
servers: [
|
||||
|
||||
@ -76,6 +76,56 @@ const STATUS_TONE: Record<McpServerRecord['status'], string> = {
|
||||
disabled: 'bg-[var(--color-surface-hover)] text-[var(--color-text-secondary)] border-[var(--color-border)]',
|
||||
}
|
||||
|
||||
const SENSITIVE_MCP_FIELD = /(?:api[_-]?key|auth[_-]?token|authorization|bearer|token|secret|password|credential)/i
|
||||
const SENSITIVE_CLI_FLAG = /^--(?:api-key|api_key|auth-token|auth_token|authorization|bearer|token|secret|password|credential)$/i
|
||||
const REDACTED_INPUT_VALUE = '[redacted]'
|
||||
|
||||
function isMcpServerNameValid(name: string): boolean {
|
||||
const trimmed = name.trim()
|
||||
return trimmed.length > 0 && !/[^\p{L}\p{N}_-]/u.test(trimmed)
|
||||
}
|
||||
|
||||
function redactSensitiveText(value: string): string {
|
||||
return value
|
||||
.replace(/(bearer\s+)(?:"[^"]+"|'[^']+'|[^\s"',}]+)/gi, '$1[redacted]')
|
||||
.replace(/(--(?:api-key|api_key|auth-token|auth_token|authorization|bearer|token|secret|password|credential)(?:=|\s+))(?:"[^"]+"|'[^']+'|[^\s"',}]+)/gi, '$1[redacted]')
|
||||
.replace(/((?:api[_-]?key|auth[_-]?token|authorization|bearer|token|secret|password|credential)(?:["']?\s*[:=]\s*["']?))([^"',\s}]+)/gi, '$1[redacted]')
|
||||
.replace(/\bsk-[A-Za-z0-9][A-Za-z0-9_-]{5,}\b/g, '[redacted]')
|
||||
}
|
||||
|
||||
function redactMcpDisplayValue(value: unknown): unknown {
|
||||
if (typeof value === 'string') return redactSensitiveText(value)
|
||||
if (Array.isArray(value)) {
|
||||
return value.map((item, index) => {
|
||||
const previous = value[index - 1]
|
||||
if (typeof previous === 'string' && SENSITIVE_CLI_FLAG.test(previous)) return '[redacted]'
|
||||
return redactMcpDisplayValue(item)
|
||||
})
|
||||
}
|
||||
if (value && typeof value === 'object') {
|
||||
return Object.fromEntries(
|
||||
Object.entries(value).map(([key, nested]) => [
|
||||
key,
|
||||
SENSITIVE_MCP_FIELD.test(key) ? '[redacted]' : redactMcpDisplayValue(nested),
|
||||
]),
|
||||
)
|
||||
}
|
||||
return value
|
||||
}
|
||||
|
||||
function displayMcpArgumentValue(rows: StringRow[], index: number): string {
|
||||
const row = rows[index]
|
||||
if (!row) return ''
|
||||
const previous = rows[index - 1]?.value
|
||||
if (row.value && previous && SENSITIVE_CLI_FLAG.test(previous.trim())) return REDACTED_INPUT_VALUE
|
||||
return redactSensitiveText(row.value)
|
||||
}
|
||||
|
||||
function displayMcpKeyValueRowValue(row: KeyValueRow): string {
|
||||
if (row.value && SENSITIVE_MCP_FIELD.test(row.key)) return REDACTED_INPUT_VALUE
|
||||
return redactSensitiveText(row.value)
|
||||
}
|
||||
|
||||
function createId() {
|
||||
if (typeof crypto !== 'undefined' && 'randomUUID' in crypto) return crypto.randomUUID()
|
||||
return `${Date.now()}-${Math.random().toString(16).slice(2)}`
|
||||
@ -212,7 +262,7 @@ function buildPayload(draft: McpDraft): McpUpsertPayload {
|
||||
}
|
||||
|
||||
function isDraftValid(draft: McpDraft) {
|
||||
if (!draft.name.trim()) return false
|
||||
if (!isMcpServerNameValid(draft.name)) return false
|
||||
if (scopeRequiresProject(draft.scope) && !draft.projectPath.trim()) return false
|
||||
if (draft.transport === 'stdio') return draft.command.trim().length > 0
|
||||
return draft.url.trim().length > 0
|
||||
@ -308,6 +358,7 @@ function ArraySection({
|
||||
valuePlaceholder,
|
||||
singleValue = false,
|
||||
addLabel,
|
||||
displayValue,
|
||||
}: {
|
||||
title: string
|
||||
rows: KeyValueRow[] | StringRow[]
|
||||
@ -318,12 +369,13 @@ function ArraySection({
|
||||
valuePlaceholder: string
|
||||
singleValue?: boolean
|
||||
addLabel: string
|
||||
displayValue?: (row: KeyValueRow | StringRow, index: number) => string
|
||||
}) {
|
||||
return (
|
||||
<section className="rounded-[var(--radius-xl)] border border-[var(--color-border)] bg-[var(--color-surface)] p-5">
|
||||
<div className="text-sm font-semibold text-[var(--color-text-primary)] mb-4">{title}</div>
|
||||
<div className="space-y-3">
|
||||
{rows.map((row) => (
|
||||
{rows.map((row, index) => (
|
||||
<div key={row.id} className={`grid gap-3 ${singleValue ? 'grid-cols-[minmax(0,1fr)_32px]' : 'grid-cols-[minmax(0,1fr)_minmax(0,1fr)_32px]'}`}>
|
||||
{!singleValue && 'key' in row && (
|
||||
<Input
|
||||
@ -333,7 +385,7 @@ function ArraySection({
|
||||
/>
|
||||
)}
|
||||
<Input
|
||||
value={row.value}
|
||||
value={displayValue ? displayValue(row, index) : row.value}
|
||||
onChange={(event) => onChange(row.id, 'value', event.target.value)}
|
||||
placeholder={valuePlaceholder}
|
||||
/>
|
||||
@ -420,7 +472,7 @@ function ServerRow({
|
||||
{server.projectPath}
|
||||
</span>
|
||||
)}
|
||||
<span className="truncate">{server.summary}</span>
|
||||
<span className="truncate">{redactSensitiveText(server.summary)}</span>
|
||||
</div>
|
||||
{server.statusDetail && (
|
||||
<div className="mt-2 text-xs text-[var(--color-text-tertiary)] truncate">{server.statusDetail}</div>
|
||||
@ -766,7 +818,7 @@ export function McpSettings() {
|
||||
<div className="flex items-start justify-between gap-4 mb-8">
|
||||
<div>
|
||||
<h2 className="text-[2.2rem] font-semibold tracking-[-0.03em] text-[var(--color-text-primary)]">{server.name}</h2>
|
||||
<p className="mt-3 text-base text-[var(--color-text-secondary)]">{server.summary}</p>
|
||||
<p className="mt-3 text-base text-[var(--color-text-secondary)]">{redactSensitiveText(server.summary)}</p>
|
||||
<div className="mt-4 flex flex-wrap items-center gap-3">
|
||||
<StatusBadge server={server} />
|
||||
{server.statusDetail && (
|
||||
@ -792,7 +844,7 @@ export function McpSettings() {
|
||||
<div className="mt-5">
|
||||
<div className="text-sm font-semibold text-[var(--color-text-primary)] mb-2">{t('settings.mcp.form.rawConfig')}</div>
|
||||
<pre className="overflow-x-auto rounded-[var(--radius-lg)] bg-[var(--color-surface-hover)] p-4 text-xs text-[var(--color-text-secondary)]">
|
||||
{JSON.stringify(server.config, null, 2)}
|
||||
{JSON.stringify(redactMcpDisplayValue(server.config), null, 2)}
|
||||
</pre>
|
||||
</div>
|
||||
</section>
|
||||
@ -987,6 +1039,7 @@ export function McpSettings() {
|
||||
onAdd={() => addRow('args')}
|
||||
onRemove={(id) => removeRow('args', id)}
|
||||
singleValue
|
||||
displayValue={(_row, index) => displayMcpArgumentValue(draft.args, index)}
|
||||
valuePlaceholder={t('settings.mcp.form.argumentPlaceholder')}
|
||||
addLabel={t('settings.mcp.form.addArgument')}
|
||||
/>
|
||||
@ -997,6 +1050,7 @@ export function McpSettings() {
|
||||
onChange={(id, field, value) => updateKeyValueRows('env', id, field, value)}
|
||||
onAdd={() => addRow('env')}
|
||||
onRemove={(id) => removeRow('env', id)}
|
||||
displayValue={(row) => ('key' in row ? displayMcpKeyValueRowValue(row) : row.value)}
|
||||
keyPlaceholder={t('settings.mcp.form.keyPlaceholder')}
|
||||
valuePlaceholder={t('settings.mcp.form.valuePlaceholder')}
|
||||
addLabel={t('settings.mcp.form.addEnv')}
|
||||
@ -1020,6 +1074,7 @@ export function McpSettings() {
|
||||
onChange={(id, field, value) => updateKeyValueRows('headers', id, field, value)}
|
||||
onAdd={() => addRow('headers')}
|
||||
onRemove={(id) => removeRow('headers', id)}
|
||||
displayValue={(row) => ('key' in row ? displayMcpKeyValueRowValue(row) : row.value)}
|
||||
keyPlaceholder={t('settings.mcp.form.keyPlaceholder')}
|
||||
valuePlaceholder={t('settings.mcp.form.valuePlaceholder')}
|
||||
addLabel={t('settings.mcp.form.addHeader')}
|
||||
|
||||
@ -44,3 +44,31 @@ describe('uiStore theme handling', () => {
|
||||
expect(document.documentElement.style.colorScheme).toBe('light')
|
||||
})
|
||||
})
|
||||
|
||||
describe('uiStore settings tab persistence', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules()
|
||||
window.localStorage.clear()
|
||||
})
|
||||
|
||||
it('hydrates the last selected Settings tab after the renderer store is recreated', async () => {
|
||||
const first = await import('./uiStore')
|
||||
|
||||
first.useUIStore.getState().setActiveSettingsTab('general')
|
||||
|
||||
expect(window.localStorage.getItem('cc-haha-active-settings-tab')).toBe('general')
|
||||
|
||||
vi.resetModules()
|
||||
const recreated = await import('./uiStore')
|
||||
|
||||
expect(recreated.useUIStore.getState().activeSettingsTab).toBe('general')
|
||||
})
|
||||
|
||||
it('ignores an invalid persisted Settings tab', async () => {
|
||||
window.localStorage.setItem('cc-haha-active-settings-tab', 'not-a-settings-tab')
|
||||
|
||||
const { useUIStore } = await import('./uiStore')
|
||||
|
||||
expect(useUIStore.getState().activeSettingsTab).toBe('providers')
|
||||
})
|
||||
})
|
||||
|
||||
@ -2,6 +2,25 @@ import { create } from 'zustand'
|
||||
import { isThemeMode, THEME_MODES, type ThemeMode } from '../types/settings'
|
||||
|
||||
const THEME_STORAGE_KEY = 'cc-haha-theme'
|
||||
const ACTIVE_SETTINGS_TAB_STORAGE_KEY = 'cc-haha-active-settings-tab'
|
||||
|
||||
const SETTINGS_TABS = [
|
||||
'providers',
|
||||
'activity',
|
||||
'general',
|
||||
'h5Access',
|
||||
'adapters',
|
||||
'terminal',
|
||||
'mcp',
|
||||
'agents',
|
||||
'skills',
|
||||
'memory',
|
||||
'plugins',
|
||||
'computerUse',
|
||||
'trace',
|
||||
'diagnostics',
|
||||
'about',
|
||||
] as const
|
||||
|
||||
function getStoredTheme(): ThemeMode {
|
||||
try {
|
||||
@ -11,6 +30,18 @@ function getStoredTheme(): ThemeMode {
|
||||
return 'white'
|
||||
}
|
||||
|
||||
function isSettingsTab(value: unknown): value is SettingsTab {
|
||||
return typeof value === 'string' && (SETTINGS_TABS as readonly string[]).includes(value)
|
||||
}
|
||||
|
||||
function getStoredSettingsTab(): SettingsTab {
|
||||
try {
|
||||
const stored = localStorage.getItem(ACTIVE_SETTINGS_TAB_STORAGE_KEY)
|
||||
if (isSettingsTab(stored)) return stored
|
||||
} catch { /* localStorage unavailable */ }
|
||||
return 'providers'
|
||||
}
|
||||
|
||||
export function applyTheme(theme: ThemeMode) {
|
||||
if (typeof document === 'undefined') return
|
||||
document.documentElement.setAttribute('data-theme', theme)
|
||||
@ -77,7 +108,7 @@ export const useUIStore = create<UIStore>((set) => ({
|
||||
theme: getStoredTheme(),
|
||||
sidebarOpen: true,
|
||||
activeView: 'code',
|
||||
activeSettingsTab: 'providers',
|
||||
activeSettingsTab: getStoredSettingsTab(),
|
||||
pendingSettingsTab: null,
|
||||
pendingMemoryPath: null,
|
||||
activeModal: null,
|
||||
@ -102,7 +133,10 @@ export const useUIStore = create<UIStore>((set) => ({
|
||||
toggleSidebar: () => set((s) => ({ sidebarOpen: !s.sidebarOpen })),
|
||||
setSidebarOpen: (open) => set({ sidebarOpen: open }),
|
||||
setActiveView: (view) => set({ activeView: view }),
|
||||
setActiveSettingsTab: (tab) => set({ activeSettingsTab: tab }),
|
||||
setActiveSettingsTab: (tab) => {
|
||||
try { localStorage.setItem(ACTIVE_SETTINGS_TAB_STORAGE_KEY, tab) } catch { /* noop */ }
|
||||
set({ activeSettingsTab: tab })
|
||||
},
|
||||
setPendingSettingsTab: (tab) => set({ pendingSettingsTab: tab }),
|
||||
setPendingMemoryPath: (path) => set({ pendingMemoryPath: path }),
|
||||
openModal: (id) => set({ activeModal: id }),
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user