From dae3d99ae01487f3d05bc19aef538d8d6815050e Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 27 Sep 2025 21:00:30 -0500 Subject: [PATCH 1/5] fix: prevent JWT token exposure in debug logs - Remove sensitive JWT token from debug logging output - Log only non-authorization header keys for debugging - Keep authorization injection confirmation message concise - Improve security by preventing token leakage in logs This addresses a security issue where JWT tokens were being logged in plain text in the debug output, potentially exposing them in production logs. Signed-off-by: phernandez --- src/basic_memory/mcp/tools/headers.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/mcp/tools/headers.py b/src/basic_memory/mcp/tools/headers.py index a7d151819..132aeec54 100644 --- a/src/basic_memory/mcp/tools/headers.py +++ b/src/basic_memory/mcp/tools/headers.py @@ -26,13 +26,18 @@ def inject_auth_header(headers: HeaderTypes | None = None) -> HeaderTypes: headers = headers.copy() http_headers = get_http_headers() - logger.debug(f"HTTP headers: {http_headers}") + + # Log only non-sensitive header keys for debugging + if logger.opt(lazy=True).debug: + safe_headers = {k for k in http_headers.keys() if k.lower() != 'authorization'} + logger.debug(f"HTTP headers present: {list(safe_headers)}") authorization = http_headers.get("Authorization") or http_headers.get("authorization") if authorization: headers["Authorization"] = authorization # type: ignore - logger.debug("Injected JWT token into authorization request headers") + # Log only that auth was injected, not the token value + logger.debug("Injected authorization header into request") else: - logger.debug("No authorization found in request headers") + logger.debug("No authorization header found in request") return headers From 8c9335ea7a976217cf9187320ac5d9bebf825e82 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 27 Sep 2025 21:03:53 -0500 Subject: [PATCH 2/5] fix: increase async client timeout to 30 seconds for write operations - Increase read/write timeout from 5s to 30s to handle longer operations - Add explicit timeout configuration for all connection phases - Prevents timeout errors on write_note operations that take >5 seconds - Particularly important for cloud deployments with file sync overhead This fixes timeout issues where write_note operations would fail when they took longer than the default 5-second httpx timeout. Signed-off-by: phernandez --- src/basic_memory/mcp/async_client.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/basic_memory/mcp/async_client.py b/src/basic_memory/mcp/async_client.py index 644394f9f..aeffb2cfb 100644 --- a/src/basic_memory/mcp/async_client.py +++ b/src/basic_memory/mcp/async_client.py @@ -1,5 +1,5 @@ import os -from httpx import ASGITransport, AsyncClient +from httpx import ASGITransport, AsyncClient, Timeout from loguru import logger from basic_memory.api.app import app as fastapi_app @@ -14,14 +14,23 @@ def create_client() -> AsyncClient: proxy_base_url = os.getenv("BASIC_MEMORY_PROXY_URL", None) logger.info(f"BASIC_MEMORY_PROXY_URL: {proxy_base_url}") + # Configure timeout for longer operations like write_note + # Default httpx timeout is 5 seconds which is too short for file operations + timeout = Timeout( + connect=10.0, # 10 seconds for connection + read=30.0, # 30 seconds for reading response + write=30.0, # 30 seconds for writing request + pool=30.0 # 30 seconds for connection pool + ) + if proxy_base_url: # Use HTTP transport to proxy endpoint logger.info(f"Creating HTTP client for proxy at: {proxy_base_url}") - return AsyncClient(base_url=proxy_base_url) + return AsyncClient(base_url=proxy_base_url, timeout=timeout) else: # Default: use ASGI transport for local API (development mode) logger.debug("Creating ASGI client for local Basic Memory API") - return AsyncClient(transport=ASGITransport(app=fastapi_app), base_url="http://test") + return AsyncClient(transport=ASGITransport(app=fastapi_app), base_url="http://test", timeout=timeout) # Create shared async client From 063207662cfe51437769cd4aefa96dd2bc3ace05 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 27 Sep 2025 21:30:22 -0500 Subject: [PATCH 3/5] fix: handle string error responses in read_note fallback searches - Add hasattr() checks before accessing 'results' attribute - Handle case where search_notes returns error strings instead of SearchResponse - Prevents AttributeError when search operations fail or timeout - Applies fix to both title and text search fallbacks This fixes the 'AttributeError: str object has no attribute results' error that occurred when search operations failed in cloud deployments. Signed-off-by: phernandez --- src/basic_memory/mcp/tools/read_note.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic_memory/mcp/tools/read_note.py b/src/basic_memory/mcp/tools/read_note.py index 5bf6a3957..5a82594ae 100644 --- a/src/basic_memory/mcp/tools/read_note.py +++ b/src/basic_memory/mcp/tools/read_note.py @@ -130,7 +130,8 @@ async def read_note( query=identifier, search_type="title", project=project, context=context ) - if title_results and title_results.results: + # Handle both SearchResponse object and error strings + if title_results and hasattr(title_results, 'results') and title_results.results: result = title_results.results[0] # Get the first/best match if result.permalink: try: @@ -159,7 +160,8 @@ async def read_note( ) # We didn't find a direct match, construct a helpful error message - if not text_results or not text_results.results: + # Handle both SearchResponse object and error strings + if not text_results or not hasattr(text_results, 'results') or not text_results.results: # No results at all return format_not_found_message(active_project.name, identifier) else: From 03da5f18e98b48f5f388deb7a7c18e9784be8526 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 27 Sep 2025 21:36:11 -0500 Subject: [PATCH 4/5] style: apply linter formatting to cloud testing fixes - Format async_client.py timeout configuration - Format headers.py security improvements - Format read_note.py error handling Maintains all functionality while conforming to project code style. Signed-off-by: phernandez --- src/basic_memory/mcp/async_client.py | 10 ++++++---- src/basic_memory/mcp/tools/headers.py | 2 +- src/basic_memory/mcp/tools/read_note.py | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/basic_memory/mcp/async_client.py b/src/basic_memory/mcp/async_client.py index aeffb2cfb..882c627cb 100644 --- a/src/basic_memory/mcp/async_client.py +++ b/src/basic_memory/mcp/async_client.py @@ -18,9 +18,9 @@ def create_client() -> AsyncClient: # Default httpx timeout is 5 seconds which is too short for file operations timeout = Timeout( connect=10.0, # 10 seconds for connection - read=30.0, # 30 seconds for reading response - write=30.0, # 30 seconds for writing request - pool=30.0 # 30 seconds for connection pool + read=30.0, # 30 seconds for reading response + write=30.0, # 30 seconds for writing request + pool=30.0, # 30 seconds for connection pool ) if proxy_base_url: @@ -30,7 +30,9 @@ def create_client() -> AsyncClient: else: # Default: use ASGI transport for local API (development mode) logger.debug("Creating ASGI client for local Basic Memory API") - return AsyncClient(transport=ASGITransport(app=fastapi_app), base_url="http://test", timeout=timeout) + return AsyncClient( + transport=ASGITransport(app=fastapi_app), base_url="http://test", timeout=timeout + ) # Create shared async client diff --git a/src/basic_memory/mcp/tools/headers.py b/src/basic_memory/mcp/tools/headers.py index 132aeec54..8559572cf 100644 --- a/src/basic_memory/mcp/tools/headers.py +++ b/src/basic_memory/mcp/tools/headers.py @@ -29,7 +29,7 @@ def inject_auth_header(headers: HeaderTypes | None = None) -> HeaderTypes: # Log only non-sensitive header keys for debugging if logger.opt(lazy=True).debug: - safe_headers = {k for k in http_headers.keys() if k.lower() != 'authorization'} + safe_headers = {k for k in http_headers.keys() if k.lower() != "authorization"} logger.debug(f"HTTP headers present: {list(safe_headers)}") authorization = http_headers.get("Authorization") or http_headers.get("authorization") diff --git a/src/basic_memory/mcp/tools/read_note.py b/src/basic_memory/mcp/tools/read_note.py index 5a82594ae..c53d8d652 100644 --- a/src/basic_memory/mcp/tools/read_note.py +++ b/src/basic_memory/mcp/tools/read_note.py @@ -131,7 +131,7 @@ async def read_note( ) # Handle both SearchResponse object and error strings - if title_results and hasattr(title_results, 'results') and title_results.results: + if title_results and hasattr(title_results, "results") and title_results.results: result = title_results.results[0] # Get the first/best match if result.permalink: try: @@ -161,7 +161,7 @@ async def read_note( # We didn't find a direct match, construct a helpful error message # Handle both SearchResponse object and error strings - if not text_results or not hasattr(text_results, 'results') or not text_results.results: + if not text_results or not hasattr(text_results, "results") or not text_results.results: # No results at all return format_not_found_message(active_project.name, identifier) else: From b6f2823183a580ed1aafb4fb37a8b01386ffed62 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sat, 27 Sep 2025 21:46:16 -0500 Subject: [PATCH 5/5] fix: address code review feedback - enhance header security and add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Expand sensitive header filtering to include cookie, x-api-key, x-auth-token, api-key - Add comprehensive test coverage for 30-second timeout configuration - Ensure all sensitive authentication headers are excluded from debug logs Addresses review comments from github-actions bot on PR #317 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Signed-off-by: phernandez --- src/basic_memory/mcp/tools/headers.py | 3 ++- tests/api/test_async_client.py | 24 +++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/basic_memory/mcp/tools/headers.py b/src/basic_memory/mcp/tools/headers.py index 8559572cf..5cfc4b428 100644 --- a/src/basic_memory/mcp/tools/headers.py +++ b/src/basic_memory/mcp/tools/headers.py @@ -29,7 +29,8 @@ def inject_auth_header(headers: HeaderTypes | None = None) -> HeaderTypes: # Log only non-sensitive header keys for debugging if logger.opt(lazy=True).debug: - safe_headers = {k for k in http_headers.keys() if k.lower() != "authorization"} + sensitive_headers = {"authorization", "cookie", "x-api-key", "x-auth-token", "api-key"} + safe_headers = {k for k in http_headers.keys() if k.lower() not in sensitive_headers} logger.debug(f"HTTP headers present: {list(safe_headers)}") authorization = http_headers.get("Authorization") or http_headers.get("authorization") diff --git a/tests/api/test_async_client.py b/tests/api/test_async_client.py index d629f256a..2ef3de77f 100644 --- a/tests/api/test_async_client.py +++ b/tests/api/test_async_client.py @@ -1,7 +1,7 @@ """Tests for async_client configuration.""" from unittest.mock import patch -from httpx import AsyncClient, ASGITransport +from httpx import AsyncClient, ASGITransport, Timeout from basic_memory.mcp.async_client import create_client @@ -25,3 +25,25 @@ def test_create_client_uses_http_when_proxy_env_set(): assert not isinstance(client._transport, ASGITransport) # When using remote API, no base_url is set (dynamic from headers) assert str(client.base_url) == "http://localhost:8000" + + +def test_create_client_configures_extended_timeouts(): + """Test that create_client configures 30-second timeouts for long operations.""" + with patch.dict("os.environ", {}, clear=True): + client = create_client() + + # Verify timeout configuration + assert isinstance(client.timeout, Timeout) + assert client.timeout.connect == 10.0 # 10 seconds for connection + assert client.timeout.read == 30.0 # 30 seconds for reading + assert client.timeout.write == 30.0 # 30 seconds for writing + assert client.timeout.pool == 30.0 # 30 seconds for pool + + # Also test with proxy URL + with patch.dict("os.environ", {"BASIC_MEMORY_PROXY_URL": "http://localhost:8000"}): + client = create_client() + + # Same timeout configuration should apply + assert isinstance(client.timeout, Timeout) + assert client.timeout.read == 30.0 + assert client.timeout.write == 30.0