fix: patch 5 security vulnerabilities across electron, server, and proxy layers (#1292)

Co-authored-by: bytecii <994513625@qq.com>

electron/main/index.ts

@@ -304,24 +304,22 @@ function processQueuedProtocolUrls() {
 
 // ==================== single instance lock ====================
 const setupSingleInstanceLock = () => {
-  const gotLock = app.requestSingleInstanceLock();
-  if (!gotLock) {
-    log.info('no-lock');
-    app.quit();
-  } else {
-    app.on('second-instance', (event, argv) => {
-      log.info('second-instance', argv);
-      const url = argv.find((arg) => arg.startsWith('eigent://'));
-      if (url) handleProtocolUrl(url);
-      if (win) win.show();
-    });
+  // The lock is already acquired at module level (requestSingleInstanceLock
+  // above). Calling it again here would release and re-acquire the lock,
+  // creating a window where a second instance could start. We only need
+  // to register the event handlers.
+  app.on('second-instance', (event, argv) => {
+    log.info('second-instance', argv);
+    const url = argv.find((arg) => arg.startsWith('eigent://'));
+    if (url) handleProtocolUrl(url);
+    if (win) win.show();
+  });
 
-    app.on('open-url', (event, url) => {
-      log.info('open-url');
-      event.preventDefault();
-      handleProtocolUrl(url);
-    });
-  }
+  app.on('open-url', (event, url) => {
+    log.info('open-url');
+    event.preventDefault();
+    handleProtocolUrl(url);
+  });
 };
 
 // ==================== initialize config ====================
@@ -443,9 +441,7 @@ function registerIpcHandlers() {
       try {
         const { spawn } = await import('child_process');
 
-        // Add --host parameter
-        const commandWithHost = `${command} --debug --host dev.eigent.ai/api/oauth/notion/callback?code=1`;
-        // const commandWithHost = `${command}`;
+        const commandWithHost = command;
 
         log.info(' start execute command:', commandWithHost);
 
@@ -2869,10 +2865,34 @@ app.whenReady().then(async () => {
   // Register protocol handler for both default session and main window session
   const protocolHandler = async (request: Request) => {
     const url = decodeURIComponent(request.url.replace('localfile://', ''));
-    const filePath = path.normalize(url);
+    const filePath = path.resolve(path.normalize(url));
 
     log.info(`[PROTOCOL] Handling localfile request: ${request.url}`);
-    log.info(`[PROTOCOL] Decoded path: ${filePath}`);
+    log.info(`[PROTOCOL] Resolved path: ${filePath}`);
+
+    // Security: Restrict file access to allowed directories only.
+    // Without this check, path traversal (e.g. /../../../etc/passwd)
+    // would allow reading arbitrary files on the filesystem.
+    const allowedBases = [
+      os.homedir(),
+      app.getPath('userData'),
+      app.getPath('temp'),
+    ];
+
+    const isPathAllowed = allowedBases.some((base) => {
+      const resolvedBase = path.resolve(base);
+      return (
+        filePath === resolvedBase ||
+        filePath.startsWith(resolvedBase + path.sep)
+      );
+    });
+
+    if (!isPathAllowed) {
+      log.error(
+        `[PROTOCOL] Security: Blocked access to path outside allowed directories: ${filePath}`
+      );
+      return new Response('Forbidden', { status: 403 });
+    }
 
     try {
       // Check if file exists

server/app/controller/mcp/proxy_controller.py

@@ -14,6 +14,7 @@
 
 import logging
 from typing import Any, cast
+from urllib.parse import quote_plus
 
 import requests
 from exa_py import Exa
@@ -129,7 +130,7 @@ def google_search(query: str, search_type: str = "web", key: Key = Depends(key_m
     # Doc: https://developers.google.com/custom-search/v1/using_rest
     base_url = (
         f"https://www.googleapis.com/customsearch/v1?"
-        f"key={GOOGLE_API_KEY}&cx={SEARCH_ENGINE_ID}&q={query}&start="
+        f"key={GOOGLE_API_KEY}&cx={SEARCH_ENGINE_ID}&q={quote_plus(query)}&start="
         f"{start_page_idx}&lr={search_language}&num={num_result_pages}"
     )
 

server/app/model/chat/chat_share.py

@@ -12,17 +12,46 @@
 # limitations under the License.
 # ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. =========
 
+import logging
 import os
+import secrets
 
 from itsdangerous import URLSafeTimedSerializer
 from pydantic import BaseModel
 
+logger = logging.getLogger(__name__)
+
+
+def _get_secret_key() -> str:
+    """Return the share-token signing key.
+
+    Falls back to a random ephemeral key when the environment variable
+    is not set.  A hardcoded default must never be used because the
+    source code is public and anyone could forge valid share tokens.
+    """
+    key = os.getenv("CHAT_SHARE_SECRET_KEY")
+    if key:
+        return key
+    logger.warning(
+        "CHAT_SHARE_SECRET_KEY not set — using a random ephemeral key. "
+        "Share links will not survive server restarts. "
+        "Set the CHAT_SHARE_SECRET_KEY environment variable for persistence."
+    )
+    return secrets.token_urlsafe(32)
+
+
+def _get_salt() -> str:
+    salt = os.getenv("CHAT_SHARE_SALT")
+    if salt:
+        return salt
+    return secrets.token_urlsafe(8)
+
 
 class ChatShare:
-    SECRET_KEY = os.getenv("CHAT_SHARE_SECRET_KEY", "EGB1WRC9xMUVgNoIPH8tLw")
-    SALT = os.getenv("CHAT_SHARE_SALT", "r4U2M")
+    SECRET_KEY = _get_secret_key()
+    SALT = _get_salt()
     # Set expiration to 1 day
-    EXPIRATION_SECONDS = int(os.getenv("CHAT_SHARE_EXPIRATION_SECONDS", 60 * 60 * 24))
+    EXPIRATION_SECONDS = int(os.getenv("CHAT_SHARE_EXPIRATION_SECONDS", str(60 * 60 * 24)))
 
     @classmethod
     def generate_token(cls, task_id: str) -> str:

server/tests/test_chat_share.py

@@ -0,0 +1,93 @@
+# ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. =========
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+# ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. =========
+
+import os
+from unittest.mock import patch
+
+import pytest
+
+
+class TestChatShareSecretKey:
+    """Tests for ChatShare secret key generation.
+
+    Validates that the hardcoded default secret key has been replaced
+    with secure random generation when environment variables are not set.
+    """
+
+    def test_get_secret_key_returns_env_var_when_set(self):
+        """_get_secret_key should return the environment variable value."""
+        with patch.dict(os.environ, {"CHAT_SHARE_SECRET_KEY": "my-custom-key"}):
+            from app.model.chat.chat_share import _get_secret_key
+
+            assert _get_secret_key() == "my-custom-key"
+
+    def test_get_secret_key_generates_random_when_env_not_set(self):
+        """_get_secret_key should generate a random key when env var is absent."""
+        env = os.environ.copy()
+        env.pop("CHAT_SHARE_SECRET_KEY", None)
+        with patch.dict(os.environ, env, clear=True):
+            from app.model.chat.chat_share import _get_secret_key
+
+            key = _get_secret_key()
+            assert key is not None
+            assert len(key) > 20  # token_urlsafe(32) produces ~43 chars
+            # Must NOT be the old hardcoded value
+            assert key != "EGB1WRC9xMUVgNoIPH8tLw"
+
+    def test_get_secret_key_generates_unique_values(self):
+        """Each call without env var should produce a different key."""
+        env = os.environ.copy()
+        env.pop("CHAT_SHARE_SECRET_KEY", None)
+        with patch.dict(os.environ, env, clear=True):
+            from app.model.chat.chat_share import _get_secret_key
+
+            key1 = _get_secret_key()
+            key2 = _get_secret_key()
+            assert key1 != key2
+
+    def test_get_salt_returns_env_var_when_set(self):
+        """_get_salt should return the environment variable value."""
+        with patch.dict(os.environ, {"CHAT_SHARE_SALT": "custom-salt"}):
+            from app.model.chat.chat_share import _get_salt
+
+            assert _get_salt() == "custom-salt"
+
+    def test_get_salt_generates_random_when_env_not_set(self):
+        """_get_salt should generate a random salt when env var is absent."""
+        env = os.environ.copy()
+        env.pop("CHAT_SHARE_SALT", None)
+        with patch.dict(os.environ, env, clear=True):
+            from app.model.chat.chat_share import _get_salt
+
+            salt = _get_salt()
+            assert salt is not None
+            assert len(salt) > 5
+            # Must NOT be the old hardcoded value
+            assert salt != "r4U2M"
+
+    def test_token_roundtrip_with_random_keys(self):
+        """Tokens generated with random keys should verify correctly."""
+        env = os.environ.copy()
+        env.pop("CHAT_SHARE_SECRET_KEY", None)
+        env.pop("CHAT_SHARE_SALT", None)
+        with patch.dict(os.environ, env, clear=True):
+            import importlib
+
+            from app.model.chat import chat_share
+
+            importlib.reload(chat_share)
+            cls = chat_share.ChatShare
+            token = cls.generate_token("test-task-id")
+            result = cls.verify_token(token)
+            assert result == "test-task-id"

server/tests/test_proxy_controller.py

@@ -0,0 +1,90 @@
+# ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. =========
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+# ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. =========
+
+from urllib.parse import quote_plus, urlparse, parse_qs
+
+import pytest
+
+
+class TestGoogleSearchUrlEncoding:
+    """Tests for Google Search query URL encoding.
+
+    Validates that the query parameter is properly URL-encoded to prevent
+    broken URLs and parameter injection when queries contain special characters.
+    """
+
+    def _build_url(self, query: str) -> str:
+        """Replicate the URL construction logic from proxy_controller."""
+        google_api_key = "TEST_KEY"
+        search_engine_id = "TEST_CX"
+        start_page_idx = 1
+        search_language = "en"
+        num_result_pages = 10
+
+        return (
+            f"https://www.googleapis.com/customsearch/v1?"
+            f"key={google_api_key}&cx={search_engine_id}&q={quote_plus(query)}&start="
+            f"{start_page_idx}&lr={search_language}&num={num_result_pages}"
+        )
+
+    def test_simple_query_encoded_correctly(self):
+        """A simple query should pass through without issues."""
+        url = self._build_url("python tutorial")
+        parsed = urlparse(url)
+        params = parse_qs(parsed.query)
+        assert params["q"] == ["python tutorial"]
+
+    def test_special_characters_encoded(self):
+        """Queries with ampersands and equals signs must be encoded."""
+        url = self._build_url("key=value&other=test")
+        parsed = urlparse(url)
+        params = parse_qs(parsed.query)
+        # The query should be a single value, not split into multiple params
+        assert params["q"] == ["key=value&other=test"]
+        # Verify the original params are preserved
+        assert params["key"] == ["TEST_KEY"]
+        assert params["cx"] == ["TEST_CX"]
+
+    def test_hash_character_encoded(self):
+        """Hash characters must not truncate the URL."""
+        url = self._build_url("C# programming")
+        parsed = urlparse(url)
+        params = parse_qs(parsed.query)
+        assert params["q"] == ["C# programming"]
+
+    def test_unicode_characters_encoded(self):
+        """Non-ASCII characters must be properly encoded."""
+        url = self._build_url("eigent AI asistan")
+        parsed = urlparse(url)
+        params = parse_qs(parsed.query)
+        assert params["q"] == ["eigent AI asistan"]
+
+    def test_plus_signs_in_query(self):
+        """Plus signs should be encoded (not treated as spaces)."""
+        url = self._build_url("C++ templates")
+        assert "C%2B%2B" in url or "C%2B%2B+templates" in url
+        parsed = urlparse(url)
+        params = parse_qs(parsed.query)
+        assert params["q"] == ["C++ templates"]
+
+    def test_parameter_injection_prevented(self):
+        """A malicious query must not inject extra URL parameters."""
+        url = self._build_url("test&key=STOLEN_KEY&cx=STOLEN_CX")
+        parsed = urlparse(url)
+        params = parse_qs(parsed.query)
+        # The injected key/cx must not overwrite the real ones
+        assert params["key"] == ["TEST_KEY"]
+        assert params["cx"] == ["TEST_CX"]
+        # The malicious string should be the query value
+        assert params["q"] == ["test&key=STOLEN_KEY&cx=STOLEN_CX"]

test/unit/electron/main/index.test.ts

@@ -1283,6 +1283,70 @@ describe('Electron Main Index Functions', () => {
     });
   });
 
+  describe('localfile:// Protocol Path Traversal Prevention', () => {
+    /**
+     * Tests for the path validation logic in the localfile:// protocol handler.
+     * Without validation, path.normalize() does NOT prevent directory traversal,
+     * allowing requests like localfile:///../../../etc/passwd to read arbitrary files.
+     */
+    const path = require('node:path');
+
+    const isPathAllowed = (filePath: string, allowedBases: string[]): boolean => {
+      const resolvedPath = path.resolve(filePath);
+      return allowedBases.some((base: string) => {
+        const resolvedBase = path.resolve(base);
+        return (
+          resolvedPath === resolvedBase ||
+          resolvedPath.startsWith(resolvedBase + path.sep)
+        );
+      });
+    };
+
+    const ALLOWED_BASES = ['/home/user', '/mock/user/data', '/tmp'];
+
+    it('should allow files within home directory', () => {
+      expect(isPathAllowed('/home/user/documents/file.pdf', ALLOWED_BASES)).toBe(true);
+    });
+
+    it('should allow files within userData directory', () => {
+      expect(isPathAllowed('/mock/user/data/cache/image.png', ALLOWED_BASES)).toBe(true);
+    });
+
+    it('should allow files within temp directory', () => {
+      expect(isPathAllowed('/tmp/upload-123.txt', ALLOWED_BASES)).toBe(true);
+    });
+
+    it('should block path traversal to /etc/passwd', () => {
+      const traversalPath = path.resolve(
+        path.normalize('/home/user/.eigent/data/../../../etc/passwd')
+      );
+      expect(isPathAllowed(traversalPath, ALLOWED_BASES)).toBe(false);
+    });
+
+    it('should block absolute paths outside allowed directories', () => {
+      expect(isPathAllowed('/etc/shadow', ALLOWED_BASES)).toBe(false);
+      expect(isPathAllowed('/var/log/syslog', ALLOWED_BASES)).toBe(false);
+      expect(isPathAllowed('/root/.ssh/id_rsa', ALLOWED_BASES)).toBe(false);
+    });
+
+    it('should block encoded traversal after normalize', () => {
+      // Simulate what happens after decodeURIComponent + normalize
+      const decodedUrl = '/home/user/data/../../../../etc/passwd';
+      const normalized = path.normalize(decodedUrl);
+      const resolved = path.resolve(normalized);
+      expect(isPathAllowed(resolved, ALLOWED_BASES)).toBe(false);
+    });
+
+    it('should allow exact base directory path', () => {
+      expect(isPathAllowed('/home/user', ALLOWED_BASES)).toBe(true);
+    });
+
+    it('should block paths that are prefixes but not subdirectories', () => {
+      // /home/user-evil should NOT match /home/user
+      expect(isPathAllowed('/home/user-evil/file.txt', ALLOWED_BASES)).toBe(false);
+    });
+  });
+
   describe('Application Lifecycle', () => {
     it('should quit on window-all-closed for non-darwin platforms', () => {
       const originalPlatform = process.platform;