fix: patch 5 security vulnerabilities across electron, server, and proxy layers#1292
Open
eren-karakus0 wants to merge 1 commit intoeigent-ai:mainfrom
Conversation
…oxy layers - localfile:// path traversal: add directory boundary validation to prevent arbitrary file reads via ../../../etc/passwd style traversal attacks - Hardcoded share token secret: replace public default key (EGB1WRC9xMUVgNoIPH8tLw) with random ephemeral generation when CHAT_SHARE_SECRET_KEY is not set - Double requestSingleInstanceLock: remove redundant second call that could release and re-acquire the lock, creating a race window for duplicate instances - execute-command debug leak: remove hardcoded --debug --host dev.eigent.ai flag that was left over from development and breaks production MCP installations - Google Search URL injection: URL-encode query parameter to prevent parameter injection and broken URLs when queries contain &, =, #, or unicode characters
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
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.
Related Issue
Closes #1291
Description
This PR patches 5 security and correctness vulnerabilities discovered during a cross-layer code audit of the Electron main process, server model layer, and proxy controller.
1. CRITICAL —
localfile://Path Traversal (electron/main/index.ts)Before: The
localfile://protocol handler usedpath.normalize()which does NOT prevent directory traversal. A request tolocalfile:///../../../etc/passwdwould resolve to/etc/passwdand serve the file contents.After: Added
path.resolve()+ directory boundary validation against an allowlist (os.homedir(),app.getPath('userData'),app.getPath('temp')). Requests outside these directories now return 403 Forbidden. The check also prevents prefix-matching attacks (/home/user-evil/won't match/home/user/).2. CRITICAL — Hardcoded Share Token Secret (
server/app/model/chat/chat_share.py)Before:
SECRET_KEYdefaulted to the hardcoded string"EGB1WRC9xMUVgNoIPH8tLw"andSALTto"r4U2M". Since the source code is public, anyone could forge valid share tokens.After: When
CHAT_SHARE_SECRET_KEYorCHAT_SHARE_SALTenvironment variables are not set, the system generates a cryptographically secure random key usingsecrets.token_urlsafe()and logs a warning. Token roundtrip still works correctly; tokens simply won't survive server restarts unless the env var is configured.3. HIGH — Double
requestSingleInstanceLock()(electron/main/index.ts)Before: The lock was acquired twice — once at module level (line 212) without event handlers, and again inside
setupSingleInstanceLock(). The second call could release and re-acquire the lock, creating a race window for duplicate instances. Protocol URLs between the calls were also lost.After: Removed the redundant second
requestSingleInstanceLock()call. The module-level call (line 212) still guards against duplicate instances.setupSingleInstanceLock()now only registerssecond-instanceandopen-urlevent handlers.4. HIGH — Hardcoded Debug URL in
execute-command(electron/main/index.ts)Before: Every command executed via the
execute-commandIPC handler had--debug --host dev.eigent.ai/api/oauth/notion/callback?code=1appended to it. The correct production code (const commandWithHost = command;) was commented out.After: Removed the hardcoded debug flags. Commands now execute as-is.
5. HIGH — Unencoded Query in Google Search URL (
server/app/controller/mcp/proxy_controller.py)Before: The
queryparameter was interpolated directly into the Google Custom Search API URL via f-string:f"...&q={query}&...". Queries containing&,=,#, or Unicode characters would break the URL or allow parameter injection.After: Added
urllib.parse.quote_plus()to properly encode the query parameter:f"...&q={quote_plus(query)}&...".Testing Evidence (REQUIRED)
Tests added:
test/unit/electron/main/index.test.ts— 8 new tests forlocalfile://path traversal prevention:/../../../etc/passwd)/home/user-evil/)server/tests/test_chat_share.py— 7 new tests for secret key generation:_get_secret_key()returns env var when set_get_secret_key()generates random key when env not setserver/tests/test_proxy_controller.py— 6 new tests for URL encoding:&,=) are encoded#) doesn't truncate URLC++) are handled correctlytest&key=STOLEN_KEY) is preventedWhat is the purpose of this pull request?
Contribution Guidelines Acknowledgement