diff --git a/electron/main/index.ts b/electron/main/index.ts index dd9623379..271a17450 100644 --- a/electron/main/index.ts +++ b/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 diff --git a/server/app/controller/mcp/proxy_controller.py b/server/app/controller/mcp/proxy_controller.py index 66c7e6531..b32dd01cb 100644 --- a/server/app/controller/mcp/proxy_controller.py +++ b/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}" ) diff --git a/server/app/model/chat/chat_share.py b/server/app/model/chat/chat_share.py index 1b7680912..30607519e 100644 --- a/server/app/model/chat/chat_share.py +++ b/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: diff --git a/server/tests/__init__.py b/server/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/server/tests/test_chat_share.py b/server/tests/test_chat_share.py new file mode 100644 index 000000000..5af72b43e --- /dev/null +++ b/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" diff --git a/server/tests/test_proxy_controller.py b/server/tests/test_proxy_controller.py new file mode 100644 index 000000000..c2ca19df2 --- /dev/null +++ b/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"] diff --git a/test/unit/electron/main/index.test.ts b/test/unit/electron/main/index.test.ts index 5937abe30..23b85f77b 100644 --- a/test/unit/electron/main/index.test.ts +++ b/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;