Security hardening: IPC, SSRF, path traversal, and input validation#388
Closed
Audun97 wants to merge 30 commits intoOpenWhispr:mainfrom
Closed
Security hardening: IPC, SSRF, path traversal, and input validation#388Audun97 wants to merge 30 commits intoOpenWhispr:mainfrom
Audun97 wants to merge 30 commits intoOpenWhispr:mainfrom
Conversation
… API calls through IPC Remove webSecurity:false and sandbox:false from the control panel window, which disabled same-origin policy entirely — amplifying every other security finding. All renderer fetch() calls are now proxied through dedicated IPC handlers in the main process, keeping API keys server-side. Changes: - Add 6 IPC handlers: OpenAI/Gemini/Groq reasoning, cloud transcription BYOK, custom model discovery, and generic auth proxy (with origin allowlist) - Replace all direct fetch() in ReasoningService, audioManager, ReasoningModelSelector, neonAuth, AuthenticationStep, EmailVerificationStep - Register app:// custom protocol to replace file:// in production, enabling proper CSP delivery via onHeadersReceived - Add Content-Security-Policy: connect-src restricted to self + Neon Auth origin only; object-src 'none'; script-src without unsafe-eval - Enable sandbox:true on control panel window (was false) - Update Vite base config for app:// protocol in production builds Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ltration Fix three vulnerabilities in the IPC security layer introduced in 57fe73f: 1. proxy-fetch: empty allowlist now blocks all requests instead of allowing all 2. app:// protocol: validate resolved path stays within src/dist/ to prevent traversal 3. proxy-cloud-transcription-byok: add endpoint host allowlist and remove renderer-provided apiKey Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… validation P1 fixes: - fetch-custom-models: reject non-HTTPS/non-private URLs, source key from environmentManager instead of renderer params - process-openai-reasoning: validate custom base URL with isSecureEndpoint - Replace all get-X-key IPC channels with has-X-key returning booleans, preventing raw API key exposure to the renderer process - Settings store uses sentinel value for .env-sourced keys - audioManager: getAPIKey() → hasAPIKey(), remove key caching and preview logging, remove dead readTranscriptionStream method P2 fixes: - Strip broken stream param from BYOK transcription handler - Validate Gemini modelId against /^[a-zA-Z0-9._-]+$/ to prevent path injection - Wrap OpenAI/Anthropic/Gemini/Groq reasoning handlers in withRetry() with exponential backoff on 5xx/network errors - Add 120s timeout to postMultipart requests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add TypeScript definitions for 11 IPC channels (proxyFetch, fetchCustomModels, processOpenAIReasoning, processGeminiReasoning, processGroqReasoning, proxyCloudTranscriptionByok, resizeMainWindow, server management, and onWindowsPushToTalkUnavailable) that were added during security hardening but lacked type coverage. Replace all (window as any).electronAPI casts with properly typed window.electronAPI across 5 files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Restrict proxy-fetch to only forward GET/POST methods and content-type/accept/authorization headers, preventing a compromised renderer from sending dangerous methods (DELETE, PUT, PATCH) or injecting arbitrary headers (Host, Cookie, X-Forwarded-For) against allowlisted backends. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…F via renderer Custom base URLs for reasoning and transcription endpoints were accepted directly from the renderer process, allowing a compromised renderer to redirect API calls (with user's API key) to attacker-controlled endpoints. Now the main process reads custom URLs from its own persisted config (.env) instead of trusting renderer input. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…vent bypass Adds normalizeIP helper that converts octal, hex, decimal-integer, and IPv4-mapped IPv6 addresses to canonical dotted-decimal before private-range checks, closing a bypass vector in HTTP/HTTPS enforcement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… bypasses Remove duplicate src/utils/urlUtils.ts, making src/helpers/urlValidation.js the single source of truth for both CJS (main process) and ESM (renderer). Fix IPv4-mapped IPv6 hex-pair bypass (::ffff:7f00:0001), abbreviated IP normalization (127.1), and block full 0.0.0.0/8 range. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…trolled endpoints Eliminate renderer control over transcription endpoints by hardcoding known provider URLs and requiring persisted config for custom providers. Surface URL save errors in the UI instead of silently swallowing them, and remove the unused `endpoint` field from the BYOK type contract. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Re-add stage-by-stage debugLogger.logReasoning() calls for BYOK and OpenAI URL resolution that were lost when URL handling moved to the main process. Replace all Promise<any> and config: any types in electron.ts with concrete return/parameter types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add redirect: "manual" to net.fetch options so server-side redirects from allowlisted origins cannot redirect to internal/private addresses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Apply the same /^[a-zA-Z0-9._-]+$/ validation already used by the Gemini handler to prevent injection via crafted model ID strings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ESM file is never imported; the CJS port at src/helpers/retry.js is what ipcHandlers.js actually uses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…f hardcoding audio/webm The renderer's optimizeAudio() converts to WAV but the IPC handler hardcoded audio/webm, causing a mismatch that stricter APIs (Groq, custom endpoints) may reject. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
maskKey() was showing first 3 + last 4 characters of actual keys. After saving, the real key also stayed in Zustand state and localStorage. Now maskKey() always returns "••••••••", enterEdit() starts with an empty draft, and all setXxxApiKey setters replace the real key with the sentinel immediately after sending it to the main process via IPC. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…interop Node 22.14.0 (bundled with Electron 36) supports `require(esm)` natively, so we can use a .mjs extension instead of the fragile hybrid export pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Groq model IDs use org/model format (e.g. qwen/qwen3-32b) but the validation regex only allowed alphanumeric, dots, hyphens, and underscores, causing all Groq reasoning requests to fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…) helper Replace 5 inconsistent inline validations (3 different regex patterns, 1 truthy-only check, 1 missing) with a unified validateModelId() function that consistently validates all provider model IDs against the same regex. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…from renderer The transcribe-audio-file-byok handler accepted apiKey from the renderer, breaking the invariant that API keys stay in the main process. Now accepts a provider string and resolves the key via environmentManager, matching the pattern used by proxy-cloud-transcription-byok. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hin tmpdir A compromised renderer could pass arbitrary file paths (e.g. /etc/passwd) to transcription handlers that call fs.readFileSync in the main process. Add validateTempFilePath() that ensures paths resolve within os.tmpdir() before any filesystem access, applied to transcribe-audio-file-cloud and transcribe-audio-file-byok handlers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Values containing newlines could inject arbitrary environment variables when written to the .env file. Wrap values in double quotes and escape backslashes, double quotes, newlines, and carriage returns — standard dotenv format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Delete orphaned SecureCache.ts, remove unused shouldStreamTranscription() method and its references in audioManager.js, and consolidate duplicate urlSaveFailed i18n keys from reasoning.custom and transcription.custom into a shared common.urlSaveFailed key across all 10 locale files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Block cloud metadata endpoints (169.254.169.254, metadata.google.internal,
fe80::) in URL validation to prevent SSRF attacks targeting cloud instance
metadata services. Replace silent .catch(() => {}) on saveAllKeysToEnvFile()
calls with debugLogger.error() for better debuggability.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tmpdir check The tmpdir guard added in 12635e6 broke file-upload transcription since user-selected files (via dialog or drag-drop) are never in tmpdir. Replace with validateUserFilePath() that checks for supported audio extensions and file readability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ed builds VITE_* env vars aren't available in the main process in packaged builds. Use the same runtime-env.json fallback pattern already used in ipcHandlers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The file upload handler checked persistedCustomUrl first regardless of provider, causing all providers to silently route to the custom endpoint when one was configured — leaking API keys. Now resolves endpoint by provider name first (matching the live transcription handler pattern), and only uses the persisted custom URL when provider === "custom". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…DPOINTS Move runtimeEnv/getAuthUrl/getApiUrl from setupHandlers() local scope to instance properties via _initRuntimeEnv(), fixing ReferenceError in _setupProxyHandlers() that broke all authentication. Consolidate duplicate PROVIDER_ENDPOINTS maps into a single module-level constant and add the missing Mistral provider entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…scription Mistral requires x-api-key header (not Bearer) and defaults to voxtral-mini-latest. The BYOK file-upload and cloud-transcription handlers were using Bearer auth and missing the Mistral key lookup. Centralizes provider auth, key lookup, and default model into shared helpers used by all three transcription IPC handlers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consolidate provider readiness logic into shared providerSecurity.mjs module, enforce persisted custom endpoints for reasoning requests, restrict URL schemes to http/https, and check only the selected provider's credentials instead of accepting any key as available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Closing in favor of a new PR with additional fixes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive security hardening of the Electron IPC layer and related attack surfaces:
webSecurity: false— route all API calls (OpenAI, Anthropic, Gemini, Groq, custom providers) through IPC handlers in the main process, re-enabling Chromium's same-origin policy.envfile valuesNotes
mainand is ~110 commits behind upstream. Merge conflicts are expected onmain.js,ipcHandlers.js,windowManager.js, and other core files.Test plan
webSecurityis no longer disabled in window config.envfile writes with values containing special characters (",\, newlines)🤖 Generated with Claude Code