Skip to content

Commit b6f2823

Browse files
phernandezclaude
andcommitted
fix: address code review feedback - enhance header security and add tests
- 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 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 03da5f1 commit b6f2823

2 files changed

Lines changed: 25 additions & 2 deletions

File tree

src/basic_memory/mcp/tools/headers.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ def inject_auth_header(headers: HeaderTypes | None = None) -> HeaderTypes:
2929

3030
# Log only non-sensitive header keys for debugging
3131
if logger.opt(lazy=True).debug:
32-
safe_headers = {k for k in http_headers.keys() if k.lower() != "authorization"}
32+
sensitive_headers = {"authorization", "cookie", "x-api-key", "x-auth-token", "api-key"}
33+
safe_headers = {k for k in http_headers.keys() if k.lower() not in sensitive_headers}
3334
logger.debug(f"HTTP headers present: {list(safe_headers)}")
3435

3536
authorization = http_headers.get("Authorization") or http_headers.get("authorization")

tests/api/test_async_client.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Tests for async_client configuration."""
22

33
from unittest.mock import patch
4-
from httpx import AsyncClient, ASGITransport
4+
from httpx import AsyncClient, ASGITransport, Timeout
55

66
from basic_memory.mcp.async_client import create_client
77

@@ -25,3 +25,25 @@ def test_create_client_uses_http_when_proxy_env_set():
2525
assert not isinstance(client._transport, ASGITransport)
2626
# When using remote API, no base_url is set (dynamic from headers)
2727
assert str(client.base_url) == "http://localhost:8000"
28+
29+
30+
def test_create_client_configures_extended_timeouts():
31+
"""Test that create_client configures 30-second timeouts for long operations."""
32+
with patch.dict("os.environ", {}, clear=True):
33+
client = create_client()
34+
35+
# Verify timeout configuration
36+
assert isinstance(client.timeout, Timeout)
37+
assert client.timeout.connect == 10.0 # 10 seconds for connection
38+
assert client.timeout.read == 30.0 # 30 seconds for reading
39+
assert client.timeout.write == 30.0 # 30 seconds for writing
40+
assert client.timeout.pool == 30.0 # 30 seconds for pool
41+
42+
# Also test with proxy URL
43+
with patch.dict("os.environ", {"BASIC_MEMORY_PROXY_URL": "http://localhost:8000"}):
44+
client = create_client()
45+
46+
# Same timeout configuration should apply
47+
assert isinstance(client.timeout, Timeout)
48+
assert client.timeout.read == 30.0
49+
assert client.timeout.write == 30.0

0 commit comments

Comments
 (0)