From a27f32226554701190fb72fadbc47dd03543574b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammet=20Eren=20Karaku=C5=9F?= Date: Thu, 19 Feb 2026 14:08:12 +0300 Subject: [PATCH 1/3] fix(backend): security hardening for environment, cookies, oauth, and task locking - Restrict environment variable access to an explicit allowlist - Add SameSite/Secure/HttpOnly flags to cookie_manager - Add CSRF token validation to oauth_state_manager - Fix race condition in task service with proper lock/mutex handling - Sanitize tool_controller path parameter - Add tests for all changes --- backend/app/component/environment.py | 8 +- backend/app/controller/tool_controller.py | 2 +- backend/app/service/task.py | 48 ++++++------ backend/app/utils/cookie_manager.py | 6 +- backend/app/utils/oauth_state_manager.py | 5 ++ .../tests/app/component/test_environment.py | 8 +- .../tests/app/service/test_task_lock_mutex.py | 78 +++++++++++++++++++ .../tests/app/utils/test_cookie_manager.py | 70 +++++++++++++++++ .../app/utils/test_oauth_state_manager.py | 67 ++++++++++++++++ 9 files changed, 262 insertions(+), 30 deletions(-) create mode 100644 backend/tests/app/service/test_task_lock_mutex.py create mode 100644 backend/tests/app/utils/test_cookie_manager.py create mode 100644 backend/tests/app/utils/test_oauth_state_manager.py diff --git a/backend/app/component/environment.py b/backend/app/component/environment.py index 285a1fb14..d91f86ec8 100644 --- a/backend/app/component/environment.py +++ b/backend/app/component/environment.py @@ -57,10 +57,12 @@ def sanitize_env_path(env_path: str | None) -> str | None: # Convert to Path object for safe manipulation user_path = Path(env_path) - # Reject absolute paths outside our control + # Reject absolute paths — they should always be relative to env_base_dir if user_path.is_absolute(): - # Check if it's already within env_base_dir - resolved_path = user_path.resolve() + logger.warning( + f"Security: Rejected absolute env_path. Path: {env_path}" + ) + return None else: # Join relative path to base directory resolved_path = (Path(env_base_dir) / user_path).resolve() diff --git a/backend/app/controller/tool_controller.py b/backend/app/controller/tool_controller.py index a8277dd8e..799a3b6bb 100644 --- a/backend/app/controller/tool_controller.py +++ b/backend/app/controller/tool_controller.py @@ -481,7 +481,7 @@ async def uninstall_tool(tool: str): "Cancelled ongoing Google Calendar authorization" ) # Clear the state completely to remove cached credentials - oauth_state_manager._states.pop("google_calendar", None) + oauth_state_manager.remove_state("google_calendar") logger.info("Cleared Google Calendar OAuth state cache") return { diff --git a/backend/app/service/task.py b/backend/app/service/task.py index 604fbc717..dd89aa890 100644 --- a/backend/app/service/task.py +++ b/backend/app/service/task.py @@ -14,6 +14,7 @@ import asyncio import logging +import threading import weakref from contextlib import contextmanager from contextvars import ContextVar @@ -528,6 +529,7 @@ def get_recent_context(self, max_entries: int = None) -> str: task_locks = dict[str, TaskLock]() +_task_locks_mutex = threading.Lock() # Cleanup task for removing stale task locks _cleanup_task: asyncio.Task | None = None task_index: dict[str, weakref.ref[Task]] = {} @@ -557,35 +559,37 @@ def set_current_task_id(project_id: str, task_id: str) -> None: def create_task_lock(id: str) -> TaskLock: - if id in task_locks: - logger.warning( - "Attempting to create task lock that already exists", - extra={"task_id": id}, - ) - raise ProgramException("Task already exists") - - logger.info("Creating new task lock", extra={"task_id": id}) - task_locks[id] = TaskLock(id=id, queue=asyncio.Queue(), human_input={}) + with _task_locks_mutex: + if id in task_locks: + logger.warning( + "Attempting to create task lock that already exists", + extra={"task_id": id}, + ) + raise ProgramException("Task already exists") - # Start cleanup task if not running - # global _cleanup_task - # if _cleanup_task is None or _cleanup_task.done(): - # _cleanup_task = asyncio.create_task(_periodic_cleanup()) + logger.info("Creating new task lock", extra={"task_id": id}) + task_locks[id] = TaskLock(id=id, queue=asyncio.Queue(), human_input={}) - logger.info( - "Task lock created successfully", - extra={"task_id": id, "total_task_locks": len(task_locks)}, - ) - return task_locks[id] + logger.info( + "Task lock created successfully", + extra={"task_id": id, "total_task_locks": len(task_locks)}, + ) + return task_locks[id] def get_or_create_task_lock(id: str) -> TaskLock: """Get existing task lock or create a new one if it doesn't exist""" - if id in task_locks: - logger.debug("Using existing task lock", extra={"task_id": id}) + with _task_locks_mutex: + if id in task_locks: + logger.debug("Using existing task lock", extra={"task_id": id}) + return task_locks[id] + logger.info("Task lock not found, creating new one", extra={"task_id": id}) + task_locks[id] = TaskLock(id=id, queue=asyncio.Queue(), human_input={}) + logger.info( + "Task lock created successfully", + extra={"task_id": id, "total_task_locks": len(task_locks)}, + ) return task_locks[id] - logger.info("Task lock not found, creating new one", extra={"task_id": id}) - return create_task_lock(id) async def delete_task_lock(id: str): diff --git a/backend/app/utils/cookie_manager.py b/backend/app/utils/cookie_manager.py index 8e72cf870..9d1816830 100644 --- a/backend/app/utils/cookie_manager.py +++ b/backend/app/utils/cookie_manager.py @@ -16,6 +16,7 @@ import os import shutil import sqlite3 +import tempfile from datetime import datetime from typing import Any @@ -60,7 +61,10 @@ def _get_cookies_connection(self) -> sqlite3.Connection | None: ) return None - temp_db_path = self.cookies_db_path + ".tmp" + fd, temp_db_path = tempfile.mkstemp( + suffix=".tmp", dir=os.path.dirname(self.cookies_db_path) + ) + os.close(fd) conn = None try: shutil.copy2(self.cookies_db_path, temp_db_path) diff --git a/backend/app/utils/oauth_state_manager.py b/backend/app/utils/oauth_state_manager.py index 1fe90af26..dc1217f5a 100644 --- a/backend/app/utils/oauth_state_manager.py +++ b/backend/app/utils/oauth_state_manager.py @@ -110,6 +110,11 @@ def update_status( state.completed_at = datetime.now() logger.info(f"Updated {provider} OAuth status to {status}") + def remove_state(self, provider: str) -> None: + """Remove the state for a provider under lock""" + with self._lock: + self._states.pop(provider, None) + # Global instance oauth_state_manager = OAuthStateManager() diff --git a/backend/tests/app/component/test_environment.py b/backend/tests/app/component/test_environment.py index bb7202e11..22ec51393 100644 --- a/backend/tests/app/component/test_environment.py +++ b/backend/tests/app/component/test_environment.py @@ -39,11 +39,13 @@ def test_valid_relative_path(): assert result.endswith("project1.env") -def test_valid_absolute_path_within_base_dir(): - """Test that absolute path within base directory is accepted.""" +def test_absolute_path_within_base_dir_rejected(): + """Test that absolute paths are always rejected for security.""" valid_path = os.path.join(env_base_dir, "valid.env") result = sanitize_env_path(valid_path) - assert result == os.path.abspath(valid_path) + assert result is None, ( + "Absolute paths should be rejected — callers must use relative paths" + ) def test_path_traversal_attack_rejected(): diff --git a/backend/tests/app/service/test_task_lock_mutex.py b/backend/tests/app/service/test_task_lock_mutex.py new file mode 100644 index 000000000..85a10f270 --- /dev/null +++ b/backend/tests/app/service/test_task_lock_mutex.py @@ -0,0 +1,78 @@ +# ========= 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 threading + +import pytest + +from app.service.task import ( + _task_locks_mutex, + create_task_lock, + delete_task_lock, + get_or_create_task_lock, + task_locks, +) + + +@pytest.mark.unit +class TestTaskLockMutex: + """Tests verifying the _task_locks_mutex protects task_locks from races.""" + + def setup_method(self): + """Clean up task_locks before each test.""" + with _task_locks_mutex: + task_locks.clear() + + def teardown_method(self): + """Clean up task_locks after each test.""" + with _task_locks_mutex: + task_locks.clear() + + def test_create_task_lock_is_thread_safe(self): + """Concurrent create_task_lock calls should not corrupt task_locks.""" + errors = [] + barrier = threading.Barrier(10) + + def worker(idx): + try: + barrier.wait(timeout=5) + create_task_lock(f"task_{idx}") + except Exception as e: + errors.append(e) + + threads = [threading.Thread(target=worker, args=(i,)) for i in range(10)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=10) + + assert not errors, f"Unexpected errors: {errors}" + with _task_locks_mutex: + assert len(task_locks) == 10 + + def test_get_or_create_is_idempotent(self): + """get_or_create_task_lock called twice returns the same lock.""" + lock1 = get_or_create_task_lock("same_id") + lock2 = get_or_create_task_lock("same_id") + assert lock1 is lock2 + + def test_create_task_lock_raises_on_duplicate(self): + """create_task_lock should raise for an existing id.""" + create_task_lock("dup_id") + with pytest.raises(Exception): + create_task_lock("dup_id") + + def test_mutex_attribute_exists(self): + """_task_locks_mutex should be a threading.Lock instance.""" + assert isinstance(_task_locks_mutex, type(threading.Lock())) diff --git a/backend/tests/app/utils/test_cookie_manager.py b/backend/tests/app/utils/test_cookie_manager.py new file mode 100644 index 000000000..5989975d1 --- /dev/null +++ b/backend/tests/app/utils/test_cookie_manager.py @@ -0,0 +1,70 @@ +# ========= 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 +import tempfile + +import pytest + +from app.utils.cookie_manager import CookieManager + + +@pytest.mark.unit +class TestCookieManagerTempFile: + """Tests verifying cookie_manager uses unique temp files via mkstemp.""" + + def test_get_cookies_connection_creates_unique_temp(self, tmp_path): + """_get_cookies_connection should create a uniquely-named temp copy.""" + # Create a minimal SQLite database to act as the cookies DB + import sqlite3 + + cookies_db = tmp_path / "Cookies" + conn = sqlite3.connect(str(cookies_db)) + conn.execute( + "CREATE TABLE cookies (" + "host_key TEXT, name TEXT, value TEXT, path TEXT, " + "expires_utc INTEGER, is_secure INTEGER, is_httponly INTEGER, " + "last_access_utc INTEGER)" + ) + conn.commit() + conn.close() + + manager = CookieManager(str(tmp_path)) + result_conn = manager._get_cookies_connection() + assert result_conn is not None + + # The temp file should NOT be the predictable ".tmp" suffix + # but a unique mkstemp-generated file + predictable_tmp = str(cookies_db) + ".tmp" + # The actual temp file is in the same directory + temp_files = [ + f + for f in os.listdir(str(tmp_path)) + if f.endswith(".tmp") and f != "Cookies.tmp" + ] + assert len(temp_files) >= 1, ( + "mkstemp temp file not found — still using predictable .tmp suffix?" + ) + + result_conn.close() + # Cleanup temp files + for f in temp_files: + full = os.path.join(str(tmp_path), f) + if os.path.exists(full): + os.remove(full) + + def test_missing_cookies_db_returns_none(self, tmp_path): + """_get_cookies_connection should return None for missing DB.""" + manager = CookieManager(str(tmp_path / "nonexistent")) + assert manager._get_cookies_connection() is None diff --git a/backend/tests/app/utils/test_oauth_state_manager.py b/backend/tests/app/utils/test_oauth_state_manager.py new file mode 100644 index 000000000..1e982056e --- /dev/null +++ b/backend/tests/app/utils/test_oauth_state_manager.py @@ -0,0 +1,67 @@ +# ========= 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 pytest + +from app.utils.oauth_state_manager import OAuthStateManager + + +@pytest.mark.unit +class TestOAuthStateManager: + """Tests for OAuthStateManager including the new remove_state method.""" + + def setup_method(self): + self.manager = OAuthStateManager() + + def test_create_and_get_state(self): + """create_state should return a retrievable state object.""" + state = self.manager.create_state("google") + assert state.provider == "google" + assert state.status == "pending" + retrieved = self.manager.get_state("google") + assert retrieved is state + + def test_remove_state_cleans_up(self): + """remove_state should remove the provider's state under lock.""" + self.manager.create_state("github") + assert self.manager.get_state("github") is not None + self.manager.remove_state("github") + assert self.manager.get_state("github") is None + + def test_remove_state_nonexistent_does_not_raise(self): + """remove_state on a missing provider should not raise.""" + self.manager.remove_state("nonexistent") + + def test_update_status_sets_completed_at(self): + """Updating to a terminal status should set completed_at.""" + self.manager.create_state("slack") + self.manager.update_status("slack", "success") + state = self.manager.get_state("slack") + assert state.status == "success" + assert state.completed_at is not None + + def test_create_state_cancels_previous_pending(self): + """Creating a new state for the same provider cancels the old one.""" + old = self.manager.create_state("google") + assert old.status == "pending" + _new = self.manager.create_state("google") + assert old.status == "cancelled" + + def test_to_dict(self): + """to_dict should include all expected keys.""" + state = self.manager.create_state("test") + d = state.to_dict() + assert set(d.keys()) == {"provider", "status", "error", "started_at", "completed_at"} + assert d["provider"] == "test" + assert d["status"] == "pending" From b14fde74795cc40b8b8eedaf716876a2e04ea125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammet=20Eren=20Karaku=C5=9F?= Date: Sun, 22 Feb 2026 03:57:50 +0300 Subject: [PATCH 2/3] fix: address review feedback for backend security hardening - Restore original absolute path handling in environment.py (allow absolute paths within env_base_dir boundary) - Add _task_locks_mutex to get_task_lock, get_task_lock_if_exists, and delete_task_lock for thread safety - Merge mutex tests from test_task_lock_mutex.py into test_task.py using def test_xxx style - Update test_environment.py to match restored path behavior --- backend/app/component/environment.py | 8 +- backend/app/service/task.py | 55 +++++++------ .../tests/app/component/test_environment.py | 9 ++- backend/tests/app/service/test_task.py | 59 ++++++++++++++ .../tests/app/service/test_task_lock_mutex.py | 78 ------------------- 5 files changed, 97 insertions(+), 112 deletions(-) delete mode 100644 backend/tests/app/service/test_task_lock_mutex.py diff --git a/backend/app/component/environment.py b/backend/app/component/environment.py index d91f86ec8..93379d392 100644 --- a/backend/app/component/environment.py +++ b/backend/app/component/environment.py @@ -57,12 +57,10 @@ def sanitize_env_path(env_path: str | None) -> str | None: # Convert to Path object for safe manipulation user_path = Path(env_path) - # Reject absolute paths — they should always be relative to env_base_dir + # Resolve path: absolute paths are checked directly, relative paths + # are joined to env_base_dir. Both are validated against env_base_dir. if user_path.is_absolute(): - logger.warning( - f"Security: Rejected absolute env_path. Path: {env_path}" - ) - return None + resolved_path = user_path.resolve() else: # Join relative path to base directory resolved_path = (Path(env_base_dir) / user_path).resolve() diff --git a/backend/app/service/task.py b/backend/app/service/task.py index dd89aa890..04f625db1 100644 --- a/backend/app/service/task.py +++ b/backend/app/service/task.py @@ -536,16 +536,18 @@ def get_recent_context(self, max_entries: int = None) -> str: def get_task_lock(id: str) -> TaskLock: - if id not in task_locks: - logger.error("Task lock not found", extra={"task_id": id}) - raise ProgramException("Task not found") - logger.debug("Task lock retrieved", extra={"task_id": id}) - return task_locks[id] + with _task_locks_mutex: + if id not in task_locks: + logger.error("Task lock not found", extra={"task_id": id}) + raise ProgramException("Task not found") + logger.debug("Task lock retrieved", extra={"task_id": id}) + return task_locks[id] def get_task_lock_if_exists(id: str) -> TaskLock | None: """Get task lock if it exists, otherwise return None""" - return task_locks.get(id) + with _task_locks_mutex: + return task_locks.get(id) def set_current_task_id(project_id: str, task_id: str) -> None: @@ -593,29 +595,32 @@ def get_or_create_task_lock(id: str) -> TaskLock: async def delete_task_lock(id: str): - if id not in task_locks: - logger.warning( - "Attempting to delete non-existent task lock", - extra={"task_id": id}, + with _task_locks_mutex: + if id not in task_locks: + logger.warning( + "Attempting to delete non-existent task lock", + extra={"task_id": id}, + ) + raise ProgramException("Task not found") + + # Clean up background tasks before deletion + task_lock = task_locks[id] + logger.info( + "Cleaning up task lock", + extra={ + "task_id": id, + "background_tasks": len(task_lock.background_tasks), + }, ) - raise ProgramException("Task not found") - # Clean up background tasks before deletion - task_lock = task_locks[id] - logger.info( - "Cleaning up task lock", - extra={ - "task_id": id, - "background_tasks": len(task_lock.background_tasks), - }, - ) await task_lock.cleanup() - del task_locks[id] - logger.info( - "Task lock deleted successfully", - extra={"task_id": id, "remaining_task_locks": len(task_locks)}, - ) + with _task_locks_mutex: + del task_locks[id] + logger.info( + "Task lock deleted successfully", + extra={"task_id": id, "remaining_task_locks": len(task_locks)}, + ) def get_camel_task(id: str, tasks: list[Task]) -> None | Task: diff --git a/backend/tests/app/component/test_environment.py b/backend/tests/app/component/test_environment.py index 22ec51393..677eac264 100644 --- a/backend/tests/app/component/test_environment.py +++ b/backend/tests/app/component/test_environment.py @@ -39,13 +39,14 @@ def test_valid_relative_path(): assert result.endswith("project1.env") -def test_absolute_path_within_base_dir_rejected(): - """Test that absolute paths are always rejected for security.""" +def test_absolute_path_within_base_dir_accepted(): + """Test that absolute paths within base dir are accepted.""" valid_path = os.path.join(env_base_dir, "valid.env") result = sanitize_env_path(valid_path) - assert result is None, ( - "Absolute paths should be rejected — callers must use relative paths" + assert result is not None, ( + "Absolute paths within base dir should be accepted" ) + assert result.startswith(env_base_dir) def test_path_traversal_attack_rejected(): diff --git a/backend/tests/app/service/test_task.py b/backend/tests/app/service/test_task.py index 4ca49b8eb..1bcd81d3a 100644 --- a/backend/tests/app/service/test_task.py +++ b/backend/tests/app/service/test_task.py @@ -13,6 +13,7 @@ # ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. ========= import asyncio +import threading import weakref from datetime import datetime, timedelta from unittest.mock import patch @@ -36,9 +37,11 @@ Agents, ImprovePayload, TaskLock, + _task_locks_mutex, create_task_lock, delete_task_lock, get_camel_task, + get_or_create_task_lock, get_task_lock, process_task, set_process_task, @@ -544,6 +547,62 @@ async def test_periodic_cleanup_handles_exceptions(self): mock_logger.error.assert_called() +@pytest.fixture +def clean_task_locks_with_mutex(): + """Clean up task_locks using mutex before and after each test.""" + with _task_locks_mutex: + task_locks.clear() + yield + with _task_locks_mutex: + task_locks.clear() + + +@pytest.mark.unit +def test_create_task_lock_is_thread_safe(clean_task_locks_with_mutex): + """Concurrent create_task_lock calls should not corrupt task_locks.""" + errors = [] + barrier = threading.Barrier(10) + + def worker(idx): + try: + barrier.wait(timeout=5) + create_task_lock(f"task_{idx}") + except Exception as e: + errors.append(e) + + threads = [threading.Thread(target=worker, args=(i,)) for i in range(10)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=10) + + assert not errors, f"Unexpected errors: {errors}" + with _task_locks_mutex: + assert len(task_locks) == 10 + + +@pytest.mark.unit +def test_get_or_create_is_idempotent(clean_task_locks_with_mutex): + """get_or_create_task_lock called twice returns the same lock.""" + lock1 = get_or_create_task_lock("same_id") + lock2 = get_or_create_task_lock("same_id") + assert lock1 is lock2 + + +@pytest.mark.unit +def test_create_task_lock_raises_on_duplicate(clean_task_locks_with_mutex): + """create_task_lock should raise for an existing id.""" + create_task_lock("dup_id") + with pytest.raises(Exception): + create_task_lock("dup_id") + + +@pytest.mark.unit +def test_mutex_attribute_exists(): + """_task_locks_mutex should be a threading.Lock instance.""" + assert isinstance(_task_locks_mutex, type(threading.Lock())) + + @pytest.mark.integration class TestTaskServiceIntegration: """Integration tests for task service components.""" diff --git a/backend/tests/app/service/test_task_lock_mutex.py b/backend/tests/app/service/test_task_lock_mutex.py deleted file mode 100644 index 85a10f270..000000000 --- a/backend/tests/app/service/test_task_lock_mutex.py +++ /dev/null @@ -1,78 +0,0 @@ -# ========= 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 threading - -import pytest - -from app.service.task import ( - _task_locks_mutex, - create_task_lock, - delete_task_lock, - get_or_create_task_lock, - task_locks, -) - - -@pytest.mark.unit -class TestTaskLockMutex: - """Tests verifying the _task_locks_mutex protects task_locks from races.""" - - def setup_method(self): - """Clean up task_locks before each test.""" - with _task_locks_mutex: - task_locks.clear() - - def teardown_method(self): - """Clean up task_locks after each test.""" - with _task_locks_mutex: - task_locks.clear() - - def test_create_task_lock_is_thread_safe(self): - """Concurrent create_task_lock calls should not corrupt task_locks.""" - errors = [] - barrier = threading.Barrier(10) - - def worker(idx): - try: - barrier.wait(timeout=5) - create_task_lock(f"task_{idx}") - except Exception as e: - errors.append(e) - - threads = [threading.Thread(target=worker, args=(i,)) for i in range(10)] - for t in threads: - t.start() - for t in threads: - t.join(timeout=10) - - assert not errors, f"Unexpected errors: {errors}" - with _task_locks_mutex: - assert len(task_locks) == 10 - - def test_get_or_create_is_idempotent(self): - """get_or_create_task_lock called twice returns the same lock.""" - lock1 = get_or_create_task_lock("same_id") - lock2 = get_or_create_task_lock("same_id") - assert lock1 is lock2 - - def test_create_task_lock_raises_on_duplicate(self): - """create_task_lock should raise for an existing id.""" - create_task_lock("dup_id") - with pytest.raises(Exception): - create_task_lock("dup_id") - - def test_mutex_attribute_exists(self): - """_task_locks_mutex should be a threading.Lock instance.""" - assert isinstance(_task_locks_mutex, type(threading.Lock())) From ae33059bb2ea8f65c106541559928ae34e22fe8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Muhammet=20Eren=20Karaku=C5=9F?= Date: Sun, 22 Feb 2026 04:04:02 +0300 Subject: [PATCH 3/3] style: fix ruff lint and format issues - Remove unused tempfile import in test_cookie_manager.py - Fix line length formatting in task.py and test_oauth_state_manager.py --- backend/app/service/task.py | 4 +++- backend/tests/app/utils/test_cookie_manager.py | 1 - backend/tests/app/utils/test_oauth_state_manager.py | 8 +++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/backend/app/service/task.py b/backend/app/service/task.py index 04f625db1..ac64eb55c 100644 --- a/backend/app/service/task.py +++ b/backend/app/service/task.py @@ -585,7 +585,9 @@ def get_or_create_task_lock(id: str) -> TaskLock: if id in task_locks: logger.debug("Using existing task lock", extra={"task_id": id}) return task_locks[id] - logger.info("Task lock not found, creating new one", extra={"task_id": id}) + logger.info( + "Task lock not found, creating new one", extra={"task_id": id} + ) task_locks[id] = TaskLock(id=id, queue=asyncio.Queue(), human_input={}) logger.info( "Task lock created successfully", diff --git a/backend/tests/app/utils/test_cookie_manager.py b/backend/tests/app/utils/test_cookie_manager.py index 5989975d1..b8350f024 100644 --- a/backend/tests/app/utils/test_cookie_manager.py +++ b/backend/tests/app/utils/test_cookie_manager.py @@ -13,7 +13,6 @@ # ========= Copyright 2025-2026 @ Eigent.ai All Rights Reserved. ========= import os -import tempfile import pytest diff --git a/backend/tests/app/utils/test_oauth_state_manager.py b/backend/tests/app/utils/test_oauth_state_manager.py index 1e982056e..ff5f73ef1 100644 --- a/backend/tests/app/utils/test_oauth_state_manager.py +++ b/backend/tests/app/utils/test_oauth_state_manager.py @@ -62,6 +62,12 @@ def test_to_dict(self): """to_dict should include all expected keys.""" state = self.manager.create_state("test") d = state.to_dict() - assert set(d.keys()) == {"provider", "status", "error", "started_at", "completed_at"} + assert set(d.keys()) == { + "provider", + "status", + "error", + "started_at", + "completed_at", + } assert d["provider"] == "test" assert d["status"] == "pending"