From 567cd6dc52ea669e72e119d1257b7bea7760ee5e Mon Sep 17 00:00:00 2001 From: AJ <6538071+snowopsdev@users.noreply.github.com> Date: Thu, 7 May 2026 09:21:33 -0400 Subject: [PATCH 1/6] fix: sanitize agent svg options before rendering --- .../src/renderer/src/components/AskModal.tsx | 123 +++++++++++++++++- 1 file changed, 118 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/renderer/src/components/AskModal.tsx b/apps/desktop/src/renderer/src/components/AskModal.tsx index 87c53086..52764104 100644 --- a/apps/desktop/src/renderer/src/components/AskModal.tsx +++ b/apps/desktop/src/renderer/src/components/AskModal.tsx @@ -23,6 +23,120 @@ import type { type AnswerValue = string | number | string[] | null; +const SVG_ALLOWED_TAGS = new Set([ + 'svg', + 'g', + 'path', + 'rect', + 'circle', + 'ellipse', + 'line', + 'polyline', + 'polygon', + 'text', + 'tspan', + 'defs', + 'linearGradient', + 'radialGradient', + 'stop', + 'clipPath', + 'mask', + 'title', + 'desc', +]); + +const SVG_ALLOWED_ATTRS = new Set([ + 'aria-hidden', + 'aria-label', + 'class', + 'clip-path', + 'cx', + 'cy', + 'd', + 'dx', + 'dy', + 'fill', + 'fill-opacity', + 'fill-rule', + 'font-family', + 'font-size', + 'font-weight', + 'height', + 'id', + 'mask', + 'offset', + 'opacity', + 'points', + 'preserveAspectRatio', + 'r', + 'rx', + 'ry', + 'stop-color', + 'stop-opacity', + 'stroke', + 'stroke-dasharray', + 'stroke-linecap', + 'stroke-linejoin', + 'stroke-opacity', + 'stroke-width', + 'text-anchor', + 'transform', + 'viewBox', + 'width', + 'x', + 'x1', + 'x2', + 'y', + 'y1', + 'y2', +]); + +const SAFE_SVG_IRI_RE = /^url\(#[-_a-zA-Z0-9:.]+\)$/; +const SAFE_SVG_COLOR_RE = + /^(none|currentColor|transparent|#[0-9a-fA-F]{3,8}|rgb\(\s*\d{1,3}\s*,\s*\d{1,3}\s*,\s*\d{1,3}\s*\)|rgba\(\s*\d{1,3}\s*,\s*\d{1,3}\s*,\s*\d{1,3}\s*,\s*(?:0|1|0?\.\d+)\s*\)|[a-zA-Z]+)$/i; + +function isSafeSvgAttrValue(name: string, value: string): boolean { + const trimmed = value.trim(); + if (/javascript:|data:|https?:|file:|vbscript:/i.test(trimmed)) return false; + if (name === 'fill' || name === 'stroke' || name === 'clip-path' || name === 'mask') { + return SAFE_SVG_COLOR_RE.test(trimmed) || SAFE_SVG_IRI_RE.test(trimmed); + } + return true; +} + +export function sanitizeInlineSvg(raw: string): string { + if (typeof DOMParser === 'undefined' || typeof XMLSerializer === 'undefined') { + return ''; + } + const parsed = new DOMParser().parseFromString(raw, 'image/svg+xml'); + if (parsed.querySelector('parsererror') !== null) return ''; + const root = parsed.documentElement; + if (root.tagName !== 'svg') return ''; + + const visit = (node: Element): void => { + for (const child of Array.from(node.children)) { + if (!SVG_ALLOWED_TAGS.has(child.tagName)) { + child.remove(); + continue; + } + visit(child); + } + for (const attr of Array.from(node.attributes)) { + if ( + attr.name.startsWith('on') || + attr.name.includes(':') || + !SVG_ALLOWED_ATTRS.has(attr.name) || + !isSafeSvgAttrValue(attr.name, attr.value) + ) { + node.removeAttribute(attr.name); + } + } + }; + + visit(root); + return new XMLSerializer().serializeToString(root); +} + export interface AskQueueState { active: AskRequest | null; queue: AskRequest[]; @@ -294,11 +408,10 @@ function SvgOptions({
{opt.label} From 500c8288d1477e9e376212d0902720ce441e7f7f Mon Sep 17 00:00:00 2001 From: AJ <6538071+snowopsdev@users.noreply.github.com> Date: Thu, 7 May 2026 09:21:37 -0400 Subject: [PATCH 2/6] fix: restore encrypted secret storage migration --- apps/desktop/src/main/diagnostics/redact.ts | 8 +-- apps/desktop/src/main/keychain.ts | 73 ++++++++------------- 2 files changed, 33 insertions(+), 48 deletions(-) diff --git a/apps/desktop/src/main/diagnostics/redact.ts b/apps/desktop/src/main/diagnostics/redact.ts index 245f8a62..5ee87e18 100644 --- a/apps/desktop/src/main/diagnostics/redact.ts +++ b/apps/desktop/src/main/diagnostics/redact.ts @@ -66,10 +66,10 @@ export function redactForIssueUrl( /** Mask the VALUE of any TOML line whose key looks sensitive, regardless of * the value's format. Google (AIzaSy...), Azure base64, DeepSeek, and future * bearer tokens all slip past format-based regexes. The `ciphertext` field - * is this codebase's specific storage slot for persisted secrets (safeStorage - * ciphertext, or in migrated/dev paths a literal plaintext token prefixed - * `plain:`) — redact unconditionally. `mask` is the user-visible display - * form and already pre-obscured, so it's intentionally NOT on this list. */ + * is this codebase's specific storage slot for persisted secrets (`safe:` + * safeStorage ciphertext, legacy safeStorage rows, or fallback `plain:` + * tokens) — redact unconditionally. `mask` is the user-visible display form + * and already pre-obscured, so it's intentionally NOT on this list. */ export function redactSensitiveTomlFields(s: string): string { return s.replace( /^(\s*(?:api_?key|token|bearer|secret|access_?token|refresh_?token|password|ciphertext|auth_?token|credential)\s*=\s*)"[^"]*"/gim, diff --git a/apps/desktop/src/main/keychain.ts b/apps/desktop/src/main/keychain.ts index dc03ddc2..23d296bc 100644 --- a/apps/desktop/src/main/keychain.ts +++ b/apps/desktop/src/main/keychain.ts @@ -1,26 +1,28 @@ import { CodesignError, type Config, ERROR_CODES, type SecretRef } from '@open-codesign/shared'; import { safeStorage } from './electron-runtime'; +import { getLogger } from './logger'; -/** - * Secret storage is now plaintext-in-config.toml (mode 0600), matching - * Claude Code / Codex / gh / aws / gcloud. safeStorage was removed because - * unsigned macOS builds triggered a system keychain password prompt on - * every decrypt — real UX tax for no real security gain (an attacker with - * filesystem access could read the plaintext ciphertext either way). - * - * The file name stays `keychain.ts` to minimise churn across importers; - * the module is now really a plaintext passthrough with one-shot legacy - * migration (safeStorage ciphertext → plaintext on first boot after - * upgrade). - */ - -/** Prefix that marks a new-format (plaintext) stored secret. */ +const ENCRYPTED_PREFIX = 'safe:'; const PLAIN_PREFIX = 'plain:'; +const logger = getLogger('keychain'); + +let warnedPlaintextFallback = false; + +function warnPlaintextFallback(): void { + if (warnedPlaintextFallback) return; + warnedPlaintextFallback = true; + logger.warn('keychain.safeStorage.unavailable_plaintext_fallback'); +} export function encryptSecret(plaintext: string): string { if (plaintext.length === 0) { throw new CodesignError('Cannot store empty secret', ERROR_CODES.KEYCHAIN_EMPTY_INPUT); } + if (safeStorage.isEncryptionAvailable()) { + const ciphertext = safeStorage.encryptString(plaintext).toString('base64'); + return `${ENCRYPTED_PREFIX}${ciphertext}`; + } + warnPlaintextFallback(); return `${PLAIN_PREFIX}${plaintext}`; } @@ -28,27 +30,21 @@ export function decryptSecret(stored: string): string { if (stored.length === 0) { throw new CodesignError('Cannot read empty secret', ERROR_CODES.KEYCHAIN_EMPTY_INPUT); } - const plaintext = stored.startsWith(PLAIN_PREFIX) - ? stored.slice(PLAIN_PREFIX.length) - : decryptLegacy(stored); + const plaintext = stored.startsWith(ENCRYPTED_PREFIX) + ? decryptSafeStorage(stored.slice(ENCRYPTED_PREFIX.length), 'encrypted') + : stored.startsWith(PLAIN_PREFIX) + ? stored.slice(PLAIN_PREFIX.length) + : decryptSafeStorage(stored, 'legacy'); if (plaintext.length === 0) { throw new CodesignError('Cannot read empty secret', ERROR_CODES.KEYCHAIN_EMPTY_INPUT); } return plaintext; } -/** - * Legacy safeStorage recovery. Invoked only for secrets written by older - * app versions that encrypted via Electron's `safeStorage`. On macOS this - * will prompt for the keychain password the FIRST time it's called (and - * only then — subsequent calls in the same process are served from - * safeStorage's in-process key cache). After `migrateSecrets` rewrites the - * config, this path is never hit again. - */ -function decryptLegacy(base64: string): string { +function decryptSafeStorage(base64: string, format: 'encrypted' | 'legacy'): string { if (!safeStorage.isEncryptionAvailable()) { throw new CodesignError( - 'A legacy encrypted API key was found but the OS keychain is unavailable. Please re-enter your API key in Settings.', + `A ${format} API key was found but the OS keychain is unavailable. Please re-enter your API key in Settings.`, ERROR_CODES.KEYCHAIN_UNAVAILABLE, ); } @@ -56,7 +52,7 @@ function decryptLegacy(base64: string): string { return safeStorage.decryptString(Buffer.from(base64, 'base64')); } catch (err) { throw new CodesignError( - 'Failed to decrypt a legacy API key. Please re-enter your API key in Settings.', + `Failed to decrypt a ${format} API key. Please re-enter your API key in Settings.`, ERROR_CODES.KEYCHAIN_UNAVAILABLE, { cause: err }, ); @@ -77,28 +73,17 @@ export function buildSecretRef(plaintext: string): SecretRef { }; } -/** - * One-shot migration run on boot: - * 1. Any secret stored in legacy safeStorage base64 format → decrypt - * once (last keychain prompt ever) → rewrite as `plain:`. - * 2. Any plaintext secret missing its display `mask` → fill it. - * - * Idempotent. Decrypt/empty failures are surfaced immediately so the app does - * not boot with a silently corrupt credential state. - */ -/** Per-entry migration step: either returns a replacement SecretRef (when - * the row is legacy ciphertext or is missing its display mask), or null - * when the row is already fully up-to-date and should be left untouched. - */ function migrateSecretRef(ref: SecretRef): SecretRef | null { - const isLegacy = !ref.ciphertext.startsWith(PLAIN_PREFIX); + const isEncrypted = ref.ciphertext.startsWith(ENCRYPTED_PREFIX); const needsMask = ref.mask === undefined || ref.mask.length === 0; - if (!isLegacy && !needsMask) return null; + if (isEncrypted && !needsMask) return null; const plaintext = decryptSecret(ref.ciphertext); + const nextCiphertext = + safeStorage.isEncryptionAvailable() && !isEncrypted ? encryptSecret(plaintext) : ref.ciphertext; return { - ciphertext: `${PLAIN_PREFIX}${plaintext}`, + ciphertext: nextCiphertext, mask: maskSecret(plaintext), }; } From 048ab2ff78a6750c6bb2dbc2b5e3e1894c934f27 Mon Sep 17 00:00:00 2001 From: AJ <6538071+snowopsdev@users.noreply.github.com> Date: Thu, 7 May 2026 09:21:43 -0400 Subject: [PATCH 3/6] fix: guard private network provider probes --- apps/desktop/src/main/connection-ipc.ts | 88 ++++++++++++++++++- apps/desktop/src/preload/index.ts | 1 + .../src/components/AddCustomProviderModal.tsx | 23 +++++ .../src/components/settings/ModelsTab.tsx | 7 +- 4 files changed, 117 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/main/connection-ipc.ts b/apps/desktop/src/main/connection-ipc.ts index ccc42755..1852ed21 100644 --- a/apps/desktop/src/main/connection-ipc.ts +++ b/apps/desktop/src/main/connection-ipc.ts @@ -1,4 +1,5 @@ import { createHash } from 'node:crypto'; +import { isIP } from 'node:net'; import { BUILTIN_PROVIDERS, CodesignError, @@ -41,7 +42,13 @@ interface ModelsListPayloadV1 { const CONNECTION_TEST_FIELDS = ['provider', 'apiKey', 'baseUrl'] as const; const MODELS_LIST_FIELDS = ['provider', 'apiKey', 'baseUrl'] as const; -const TEST_ENDPOINT_FIELDS = ['wire', 'baseUrl', 'apiKey', 'httpHeaders'] as const; +const TEST_ENDPOINT_FIELDS = [ + 'wire', + 'baseUrl', + 'apiKey', + 'httpHeaders', + 'allowPrivateNetwork', +] as const; function assertKnownFields( record: Record, @@ -177,6 +184,62 @@ function parseHttpBaseUrl(value: unknown, field: string): string { return trimmed; } +export type NetworkTargetClass = 'public' | 'loopback' | 'private' | 'link-local' | 'metadata'; + +function ipv4ToNumber(ip: string): number | null { + const parts = ip.split('.').map((part) => Number(part)); + if ( + parts.length !== 4 || + parts.some((part) => !Number.isInteger(part) || part < 0 || part > 255) + ) { + return null; + } + const [a, b, c, d] = parts as [number, number, number, number]; + return ((a << 24) >>> 0) + (b << 16) + (c << 8) + d; +} + +function inIpv4Range(ip: string, base: string, bits: number): boolean { + const value = ipv4ToNumber(ip); + const baseValue = ipv4ToNumber(base); + if (value === null || baseValue === null) return false; + const mask = bits === 0 ? 0 : (0xffffffff << (32 - bits)) >>> 0; + return (value & mask) === (baseValue & mask); +} + +export function classifyNetworkTarget(rawBaseUrl: string): NetworkTargetClass { + let parsed: URL; + try { + parsed = new URL(rawBaseUrl); + } catch { + return 'public'; + } + const host = parsed.hostname.replace(/^\[(.*)\]$/, '$1').toLowerCase(); + if ( + host === 'metadata.google.internal' || + host === 'metadata' || + host === '169.254.169.254' || + host === 'fd00:ec2::254' + ) { + return 'metadata'; + } + if (host === 'localhost') return 'loopback'; + const family = isIP(host); + if (family === 4) { + if (inIpv4Range(host, '127.0.0.0', 8)) return 'loopback'; + if (inIpv4Range(host, '10.0.0.0', 8)) return 'private'; + if (inIpv4Range(host, '172.16.0.0', 12)) return 'private'; + if (inIpv4Range(host, '192.168.0.0', 16)) return 'private'; + if (inIpv4Range(host, '169.254.0.0', 16)) return 'link-local'; + return 'public'; + } + if (family === 6) { + if (host === '::1') return 'loopback'; + if (host.startsWith('fe80:')) return 'link-local'; + if (host.startsWith('fc') || host.startsWith('fd')) return 'private'; + } + return 'public'; +} + // --------------------------------------------------------------------------- // Models endpoint construction // --------------------------------------------------------------------------- @@ -978,6 +1041,22 @@ export async function handleConfigV1TestEndpoint(raw: unknown): Promise; + allowPrivateNetwork?: boolean; } export type TestEndpointResponse = @@ -1172,6 +1252,12 @@ function parseTestEndpointPayload(raw: unknown): TestEndpointPayload { baseUrl: parseHttpBaseUrl(baseUrl, 'baseUrl'), apiKey: trimmedApiKey, }; + if (r['allowPrivateNetwork'] !== undefined) { + if (typeof r['allowPrivateNetwork'] !== 'boolean') { + throw new CodesignError('allowPrivateNetwork must be a boolean', ERROR_CODES.IPC_BAD_INPUT); + } + out.allowPrivateNetwork = r['allowPrivateNetwork']; + } const headers = parseTestEndpointHttpHeaders(r['httpHeaders']); if (headers !== undefined) out.httpHeaders = headers; return out; diff --git a/apps/desktop/src/preload/index.ts b/apps/desktop/src/preload/index.ts index 95e33553..419f7df5 100644 --- a/apps/desktop/src/preload/index.ts +++ b/apps/desktop/src/preload/index.ts @@ -515,6 +515,7 @@ const api = { baseUrl: string; apiKey: string; httpHeaders?: Record; + allowPrivateNetwork?: boolean; }) => ipcRenderer.invoke('config:v1:test-endpoint', input) as Promise, listEndpointModels: (input: { wire: WireApi; baseUrl: string; apiKey: string }) => ipcRenderer.invoke('config:v1:list-endpoint-models', input) as Promise< diff --git a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx index 4cf302d5..234c8e55 100644 --- a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx +++ b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx @@ -100,6 +100,7 @@ export function AddCustomProviderModal({ const [test, setTest] = useState({ kind: 'idle' }); const [saving, setSaving] = useState(false); const [error, setError] = useState(null); + const [allowPrivateNetwork, setAllowPrivateNetwork] = useState(false); const [discovery, setDiscovery] = useState({ kind: 'idle' }); // When true, user explicitly chose to type a model name instead of picking from the dropdown. @@ -131,6 +132,7 @@ export function AddCustomProviderModal({ wire: currentWire, baseUrl: currentBaseUrl.trim(), apiKey: '', + allowPrivateNetwork, }); if (seq !== discoverySeq.current) return; if (res.ok && res.models.length > 0) { @@ -183,6 +185,7 @@ export function AddCustomProviderModal({ wire, baseUrl: baseUrl.trim(), apiKey: apiKey.trim(), + allowPrivateNetwork, }); if (res.ok) setTest({ kind: 'ok', modelCount: res.modelCount }); else setTest({ kind: 'error', message: res.message }); @@ -337,6 +340,26 @@ export function AddCustomProviderModal({

)} + {!lockEndpoint && ( + + )} diff --git a/apps/desktop/src/renderer/src/components/settings/ModelsTab.tsx b/apps/desktop/src/renderer/src/components/settings/ModelsTab.tsx index b2952216..72df4906 100644 --- a/apps/desktop/src/renderer/src/components/settings/ModelsTab.tsx +++ b/apps/desktop/src/renderer/src/components/settings/ModelsTab.tsx @@ -449,7 +449,12 @@ export function ModelsTab() { setCpaDetection('detecting'); void window.codesign.config - .testEndpoint({ wire: 'anthropic', baseUrl: 'http://127.0.0.1:8317', apiKey: '' }) + .testEndpoint({ + wire: 'anthropic', + baseUrl: 'http://127.0.0.1:8317', + apiKey: '', + allowPrivateNetwork: true, + }) .then((res) => { setCpaDetection(res.ok ? 'available' : 'unavailable'); }) From d65f359dc23806abab9bc655d43b83fe19cd5d07 Mon Sep 17 00:00:00 2001 From: AJ <6538071+snowopsdev@users.noreply.github.com> Date: Thu, 7 May 2026 09:21:54 -0400 Subject: [PATCH 4/6] test: cover security hardening paths --- apps/desktop/src/main/connection-ipc.test.ts | 76 ++++++++++++++++ apps/desktop/src/main/diagnostics-ipc.test.ts | 6 +- apps/desktop/src/main/keychain.test.ts | 89 +++++++++++++++++-- .../AddCustomProviderModal.test.tsx | 1 + .../renderer/src/components/AskModal.test.ts | 32 ++++++- 5 files changed, 194 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/main/connection-ipc.test.ts b/apps/desktop/src/main/connection-ipc.test.ts index 6e5091fa..0c603835 100644 --- a/apps/desktop/src/main/connection-ipc.test.ts +++ b/apps/desktop/src/main/connection-ipc.test.ts @@ -12,6 +12,7 @@ import { buildAuthHeadersForWire, CONNECTION_FETCH_TIMEOUT_MS, classifyHttpError, + classifyNetworkTarget, extractIds, extractModelIds, fetchWithTimeout, @@ -1202,6 +1203,81 @@ describe('config:v1:test-endpoint response parsing', () => { } }); + it('classifies private and metadata network targets', () => { + expect(classifyNetworkTarget('https://provider.example/v1')).toBe('public'); + expect(classifyNetworkTarget('http://localhost:8317')).toBe('loopback'); + expect(classifyNetworkTarget('http://127.0.0.1:8317')).toBe('loopback'); + expect(classifyNetworkTarget('http://[::1]:8317')).toBe('loopback'); + expect(classifyNetworkTarget('http://10.0.0.5:8080')).toBe('private'); + expect(classifyNetworkTarget('http://172.16.4.5:8080')).toBe('private'); + expect(classifyNetworkTarget('http://192.168.1.50:8080')).toBe('private'); + expect(classifyNetworkTarget('http://169.254.1.10:8080')).toBe('link-local'); + expect(classifyNetworkTarget('http://169.254.169.254/latest')).toBe('metadata'); + expect(classifyNetworkTarget('http://metadata.google.internal')).toBe('metadata'); + }); + + it('requires explicit confirmation for private endpoint probes', async () => { + const { restore } = installFakeFetch(() => { + throw new Error('fetch should not be called'); + }); + try { + await expect( + handleConfigV1TestEndpoint({ + wire: 'openai-chat', + baseUrl: 'http://127.0.0.1:8317/v1', + apiKey: 'sk-test', + }), + ).resolves.toEqual({ + ok: false, + error: 'private-network-confirmation-required', + message: 'Private or local network provider URLs require explicit confirmation.', + }); + } finally { + restore(); + } + }); + + it('allows private endpoint probes after explicit confirmation', async () => { + const { restore } = installFakeFetch(() => ({ + status: 200, + body: { data: [{ id: 'local' }] }, + })); + try { + await expect( + handleConfigV1TestEndpoint({ + wire: 'openai-chat', + baseUrl: 'http://127.0.0.1:8317/v1', + apiKey: 'sk-test', + allowPrivateNetwork: true, + }), + ).resolves.toEqual({ ok: true, modelCount: 1, models: ['local'] }); + } finally { + restore(); + } + }); + + it('blocks metadata endpoint probes even with private-network confirmation', async () => { + const { restore } = installFakeFetch(() => { + throw new Error('fetch should not be called'); + }); + try { + await expect( + handleConfigV1TestEndpoint({ + wire: 'openai-chat', + baseUrl: 'http://169.254.169.254/latest', + apiKey: 'sk-test', + allowPrivateNetwork: true, + }), + ).resolves.toEqual({ + ok: false, + error: 'blocked-network-target', + message: 'Metadata service endpoints cannot be used as model provider base URLs.', + }); + } finally { + restore(); + } + }); + it('rejects malformed baseUrl before attempting fetch', async () => { const { restore } = installFakeFetch(() => { throw new Error('fetch should not be called'); diff --git a/apps/desktop/src/main/diagnostics-ipc.test.ts b/apps/desktop/src/main/diagnostics-ipc.test.ts index e827ca97..84cab77c 100644 --- a/apps/desktop/src/main/diagnostics-ipc.test.ts +++ b/apps/desktop/src/main/diagnostics-ipc.test.ts @@ -672,9 +672,9 @@ describe('redactSensitiveTomlFields', () => { it('masks the ciphertext field used by this codebase to persist secrets', () => { // Reproduces the real bundle leak a user reported on 2026-04-22: the // `[secrets.*] ciphertext = "..."` field was slipping through because - // it wasn't on the field allowlist. "plain:" is the dev-mode - // pass-through encoding (see keychain.ts), so the raw token is right - // there in the exported zip. + // it wasn't on the field allowlist. Modern rows use safeStorage-backed + // `safe:`, but fallback and legacy config rows can still contain + // `plain:`, so the raw token must never appear in the exported zip. const input = [ '[secrets.claude-code-imported]', 'ciphertext = "plain:another-your-anthropic-auth-token"', diff --git a/apps/desktop/src/main/keychain.test.ts b/apps/desktop/src/main/keychain.test.ts index 77e3441c..1e814bbc 100644 --- a/apps/desktop/src/main/keychain.test.ts +++ b/apps/desktop/src/main/keychain.test.ts @@ -1,23 +1,26 @@ import { CodesignError, ERROR_CODES, hydrateConfig } from '@open-codesign/shared'; import { describe, expect, it, vi } from 'vitest'; +const loggerMock = vi.hoisted(() => ({ + warn: vi.fn(), + info: vi.fn(), + error: vi.fn(), +})); + vi.mock('./electron-runtime', () => ({ safeStorage: { isEncryptionAvailable: vi.fn(() => true), + encryptString: vi.fn((s: string) => Buffer.from(`encrypted:${s}`, 'utf8')), decryptString: vi.fn(() => ''), }, })); vi.mock('./logger', () => ({ - getLogger: () => ({ - warn: vi.fn(), - info: vi.fn(), - error: vi.fn(), - }), + getLogger: () => loggerMock, })); import { safeStorage } from './electron-runtime'; -import { decryptSecret, migrateSecrets } from './keychain'; +import { decryptSecret, encryptSecret, migrateSecrets } from './keychain'; function expectKeychainEmpty(fn: () => unknown): void { try { @@ -31,6 +34,29 @@ function expectKeychainEmpty(fn: () => unknown): void { } describe('decryptSecret', () => { + it('encrypts new secrets with safeStorage when available', () => { + const stored = encryptSecret('sk-test-secret'); + expect(stored).toBe(`safe:${Buffer.from('encrypted:sk-test-secret').toString('base64')}`); + }); + + it('falls back to plaintext rows when encryption is unavailable', () => { + vi.mocked(safeStorage.isEncryptionAvailable).mockReturnValueOnce(false); + expect(encryptSecret('sk-test-secret')).toBe('plain:sk-test-secret'); + expect(loggerMock.warn).toHaveBeenCalledWith( + 'keychain.safeStorage.unavailable_plaintext_fallback', + ); + }); + + it('reads existing plaintext secret rows', () => { + expect(decryptSecret('plain:sk-test-secret')).toBe('sk-test-secret'); + }); + + it('reads new encrypted secret rows', () => { + vi.mocked(safeStorage.decryptString).mockReturnValueOnce('sk-test-secret'); + const stored = `safe:${Buffer.from('ciphertext').toString('base64')}`; + expect(decryptSecret(stored)).toBe('sk-test-secret'); + }); + it('rejects plaintext secret rows that decrypt to an empty string', () => { expectKeychainEmpty(() => decryptSecret('plain:')); }); @@ -41,6 +67,57 @@ describe('decryptSecret', () => { }); describe('migrateSecrets', () => { + it('migrates plaintext rows to encrypted rows when safeStorage is available', () => { + const cfg = hydrateConfig({ + version: 3, + activeProvider: 'openai', + activeModel: 'gpt-5.4', + providers: { + openai: { + id: 'openai', + name: 'OpenAI', + builtin: true, + wire: 'openai-chat', + baseUrl: 'https://api.openai.com/v1', + defaultModel: 'gpt-5.4', + }, + }, + secrets: { openai: { ciphertext: 'plain:sk-test-secret', mask: '' } }, + }); + + const migrated = migrateSecrets(cfg); + expect(migrated.changed).toBe(true); + expect(migrated.config.secrets['openai']?.ciphertext).toBe( + `safe:${Buffer.from('encrypted:sk-test-secret').toString('base64')}`, + ); + expect(migrated.config.secrets['openai']?.mask).toBe('sk-***cret'); + }); + + it('keeps plaintext rows when safeStorage is unavailable', () => { + vi.mocked(safeStorage.isEncryptionAvailable).mockReturnValue(false); + const cfg = hydrateConfig({ + version: 3, + activeProvider: 'openai', + activeModel: 'gpt-5.4', + providers: { + openai: { + id: 'openai', + name: 'OpenAI', + builtin: true, + wire: 'openai-chat', + baseUrl: 'https://api.openai.com/v1', + defaultModel: 'gpt-5.4', + }, + }, + secrets: { openai: { ciphertext: 'plain:sk-test-secret', mask: '' } }, + }); + + const migrated = migrateSecrets(cfg); + expect(migrated.changed).toBe(true); + expect(migrated.config.secrets['openai']?.ciphertext).toBe('plain:sk-test-secret'); + vi.mocked(safeStorage.isEncryptionAvailable).mockReturnValue(true); + }); + it('rejects legacy secret rows that decrypt to an empty string', () => { vi.mocked(safeStorage.decryptString).mockReturnValueOnce(''); const cfg = hydrateConfig({ diff --git a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx index 7698ad33..ea1f0169 100644 --- a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx +++ b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx @@ -14,6 +14,7 @@ describe('AddCustomProviderModal', () => { expect(html).toContain('settings.providers.custom.compatibilityHintTitle'); expect(html).toContain('settings.providers.custom.compatibilityHintBody'); + expect(html).toContain('settings.providers.custom.allowPrivateNetwork'); }); it('hides the compatibility warning when editing a locked builtin endpoint', () => { diff --git a/apps/desktop/src/renderer/src/components/AskModal.test.ts b/apps/desktop/src/renderer/src/components/AskModal.test.ts index ed5b5d7c..9409a4f2 100644 --- a/apps/desktop/src/renderer/src/components/AskModal.test.ts +++ b/apps/desktop/src/renderer/src/components/AskModal.test.ts @@ -1,6 +1,8 @@ +// @vitest-environment happy-dom + import { describe, expect, it } from 'vitest'; import type { AskRequest } from '../../../preload/index'; -import { advanceAskQueue, enqueueAskRequest } from './AskModal'; +import { advanceAskQueue, enqueueAskRequest, sanitizeInlineSvg } from './AskModal'; const request = (requestId: string): AskRequest => ({ requestId, @@ -10,6 +12,34 @@ const request = (requestId: string): AskRequest => ({ }, }); +describe('sanitizeInlineSvg', () => { + it('keeps inert SVG presentation markup', () => { + const out = sanitizeInlineSvg( + '', + ); + + expect(out).toContain(' { + const out = sanitizeInlineSvg( + '', + ); + + expect(out).not.toContain('onload'); + expect(out).not.toContain('onclick'); + expect(out).not.toContain(' { it('queues concurrent requests without replacing the active request', () => { const first = request('ask-1'); From 3f7e1aaeb901e45ca1d9a746d679c2a8adea5615 Mon Sep 17 00:00:00 2001 From: AJ <6538071+snowopsdev@users.noreply.github.com> Date: Thu, 7 May 2026 11:09:16 -0400 Subject: [PATCH 5/6] ci: ensure electron binary is installed --- .github/workflows/ci.yml | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24b45d47..f2e95d63 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,25 @@ jobs: cache: pnpm - name: Install - run: pnpm install --frozen-lockfile + run: env -u ELECTRON_SKIP_BINARY_DOWNLOAD -u ELECTRON_SKIP_DOWNLOAD pnpm install --frozen-lockfile --ignore-scripts=false + + - name: Install Electron binary + run: | + env -u ELECTRON_SKIP_BINARY_DOWNLOAD -u ELECTRON_SKIP_DOWNLOAD pnpm -C apps/desktop exec node -e " + const { execFileSync } = require('node:child_process'); + const path = require('node:path'); + const electronDir = path.dirname(require.resolve('electron/package.json')); + execFileSync(process.execPath, [path.join(electronDir, 'install.js')], { stdio: 'inherit' }); + " + + - name: Verify Electron binary + run: | + env -u ELECTRON_SKIP_BINARY_DOWNLOAD -u ELECTRON_SKIP_DOWNLOAD pnpm -C apps/desktop exec node -e " + const fs = require('node:fs'); + const electronPath = require('electron'); + fs.accessSync(electronPath, fs.constants.X_OK); + console.log(electronPath); + " - name: Lint run: pnpm lint From 8ed001709a804a20c8a83cdf988726f38df5918f Mon Sep 17 00:00:00 2001 From: hqhq1025 <1506751656@qq.com> Date: Sat, 9 May 2026 14:00:24 +0800 Subject: [PATCH 6/6] fix(desktop): stabilize private endpoint discovery --- .changeset/harden-provider-boundaries.md | 5 ++ .../AddCustomProviderModal.test.tsx | 11 ++++- .../src/components/AddCustomProviderModal.tsx | 46 ++++++++++++++----- 3 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 .changeset/harden-provider-boundaries.md diff --git a/.changeset/harden-provider-boundaries.md b/.changeset/harden-provider-boundaries.md new file mode 100644 index 00000000..8cbb4831 --- /dev/null +++ b/.changeset/harden-provider-boundaries.md @@ -0,0 +1,5 @@ +--- +"@open-codesign/desktop": patch +--- + +Harden desktop provider setup by sanitizing agent-supplied SVG choice icons, storing new API keys with Electron safeStorage when available, redacting encrypted secret rows from diagnostics, and requiring explicit opt-in before testing local or private-network provider URLs. diff --git a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx index ea1f0169..bdf49428 100644 --- a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx +++ b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.test.tsx @@ -1,6 +1,6 @@ import { renderToStaticMarkup } from 'react-dom/server'; import { describe, expect, it, vi } from 'vitest'; -import { AddCustomProviderModal } from './AddCustomProviderModal'; +import { AddCustomProviderModal, buildEndpointDiscoveryPayload } from './AddCustomProviderModal'; vi.mock('@open-codesign/i18n', () => ({ useT: () => (key: string) => key, @@ -37,4 +37,13 @@ describe('AddCustomProviderModal', () => { expect(html).not.toContain('settings.providers.custom.compatibilityHintTitle'); expect(html).not.toContain('settings.providers.custom.compatibilityHintBody'); }); + + it('builds endpoint discovery payloads from the latest private-network opt-in value', () => { + expect(buildEndpointDiscoveryPayload('openai-chat', ' http://127.0.0.1:8317 ', true)).toEqual({ + wire: 'openai-chat', + baseUrl: 'http://127.0.0.1:8317', + apiKey: '', + allowPrivateNetwork: true, + }); + }); }); diff --git a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx index 234c8e55..8adf483a 100644 --- a/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx +++ b/apps/desktop/src/renderer/src/components/AddCustomProviderModal.tsx @@ -70,6 +70,24 @@ function pickBestModel(models: string[]): string { return models[0] ?? ''; } +export function buildEndpointDiscoveryPayload( + wire: WireApi, + baseUrl: string, + allowPrivateNetwork: boolean, +): { + wire: WireApi; + baseUrl: string; + apiKey: string; + allowPrivateNetwork: boolean; +} { + return { + wire, + baseUrl: baseUrl.trim(), + apiKey: '', + allowPrivateNetwork, + }; +} + /** * Minimal Custom Provider form — wire-agnostic endpoint onboarding. * Deliberately barebones (native form + FormData-ish accessors, no schema), @@ -111,7 +129,11 @@ export function AddCustomProviderModal({ const debounceTimer = useRef | null>(null); const discoverySeq = useRef(0); - function scheduleDiscovery(currentBaseUrl: string, currentWire: WireApi) { + function scheduleDiscovery( + currentBaseUrl: string, + currentWire: WireApi, + privateNetworkAllowed = allowPrivateNetwork, + ) { if (debounceTimer.current !== null) clearTimeout(debounceTimer.current); if (!currentBaseUrl.trim().match(/^https?:\/\//)) { discoverySeq.current += 1; @@ -119,21 +141,22 @@ export function AddCustomProviderModal({ return; } debounceTimer.current = setTimeout(() => { - void runDiscovery(currentBaseUrl, currentWire); + void runDiscovery(currentBaseUrl, currentWire, privateNetworkAllowed); }, 500); } - async function runDiscovery(currentBaseUrl: string, currentWire: WireApi) { + async function runDiscovery( + currentBaseUrl: string, + currentWire: WireApi, + privateNetworkAllowed = allowPrivateNetwork, + ) { if (!window.codesign?.config) return; const seq = ++discoverySeq.current; setDiscovery({ kind: 'discovering' }); try { - const res = await window.codesign.config.testEndpoint({ - wire: currentWire, - baseUrl: currentBaseUrl.trim(), - apiKey: '', - allowPrivateNetwork, - }); + const res = await window.codesign.config.testEndpoint( + buildEndpointDiscoveryPayload(currentWire, currentBaseUrl, privateNetworkAllowed), + ); if (seq !== discoverySeq.current) return; if (res.ok && res.models.length > 0) { setDiscovery({ kind: 'found', models: res.models }); @@ -346,9 +369,10 @@ export function AddCustomProviderModal({ type="checkbox" checked={allowPrivateNetwork} onChange={(e) => { - setAllowPrivateNetwork(e.target.checked); + const nextAllowPrivateNetwork = e.target.checked; + setAllowPrivateNetwork(nextAllowPrivateNetwork); setTest({ kind: 'idle' }); - scheduleDiscovery(baseUrl, wire); + scheduleDiscovery(baseUrl, wire, nextAllowPrivateNetwork); }} className="mt-0.5 accent-[var(--color-accent)]" />