@AgentWrapper@cursor[bot]agent3mo ago
Bugbot Autofix prepared fixes for 3 of the 3 bugs found in the latest run.
- ✅ Fixed: Cache returns mutable reference; caller mutation corrupts cache
- Added copy.deepcopy() when returning cached data for redact=False and when storing data in the cache to prevent callers from mutating shared references.
- ✅ Fixed: Unused NO_AUTH_MOCK_DATA; docstring overstates test coverage
- Removed the unused NO_AUTH_MOCK_DATA constant and updated the module docstring to remove the claim about NO_AUTH apps being tested.
- ✅ Fixed: Unbounded cache growth; expired entries never evicted
- Added _cleanup_expired_entries() function that evicts expired entries when the cache exceeds _AUTH_DATA_CACHE_MAX_SIZE (1000 entries).
Or push these changes by commenting:
@cursor push f853c160af
diff --git a/cortex/common/composio_client.py b/cortex/common/composio_client.py
--- a/cortex/common/composio_client.py
+++ b/cortex/common/composio_client.py
@@ -14,11 +14,21 @@
# cuts the number of identical requests within a single workflow run.
_AUTH_DATA_CACHE_TTL = 60
+# Maximum number of entries in the cache before triggering cleanup.
+_AUTH_DATA_CACHE_MAX_SIZE = 1000
+
# Cache: (app_name, connection_id) -> (unredacted_data, expires_at_monotonic)
_auth_data_cache: t.Dict[t.Tuple[str, str], t.Tuple[t.Dict[str, t.Any], float]] = {}
_auth_data_lock = threading.Lock()
+def _cleanup_expired_entries(now: float) -> None:
+ """Remove expired entries from the cache. Caller must hold _auth_data_lock."""
+ expired_keys = [k for k, (_, exp) in _auth_data_cache.items() if now >= exp]
+ for k in expired_keys:
+ del _auth_data_cache[k]
+
+
def _validate_env() -> t.Literal["production", "staging"]:
env = os.getenv("COMPOSIO_ENV", None)
if env is None:
@@ -37,6 +47,9 @@
The cache stores the unredacted response so that a single API call can
satisfy both redacted (for display) and unredacted (for execution) callers.
+
+ Note: This function always returns a deep copy of the cached data to prevent
+ callers from inadvertently mutating the cache.
"""
cache_key = (app_name, connection_id)
now = time.monotonic()
@@ -45,7 +58,7 @@
entry = _auth_data_cache.get(cache_key)
if entry is not None and now < entry[1]:
data = entry[0]
- return _redact_auth_data(data) if redact else data
+ return _redact_auth_data(data) if redact else copy.deepcopy(data)
# Cache miss – always fetch unredacted so we can cache one copy.
data = ComposioAPIClient.from_env(_validate_env()).get_auth_data(
@@ -53,7 +66,11 @@
)
with _auth_data_lock:
- _auth_data_cache[cache_key] = (data, now + _AUTH_DATA_CACHE_TTL)
+ # Cleanup expired entries if cache is getting large
+ if len(_auth_data_cache) >= _AUTH_DATA_CACHE_MAX_SIZE:
+ _cleanup_expired_entries(now)
+ # Store a deep copy so we don't share references with the returned value
+ _auth_data_cache[cache_key] = (copy.deepcopy(data), now + _AUTH_DATA_CACHE_TTL)
return _redact_auth_data(data) if redact else data
diff --git a/cortex/tests/test_common/test_cortex_composio_client.py b/cortex/tests/test_common/test_cortex_composio_client.py
--- a/cortex/tests/test_common/test_cortex_composio_client.py
+++ b/cortex/tests/test_common/test_cortex_composio_client.py
@@ -6,7 +6,6 @@
- The cache is keyed by (app_name, connection_id), so different IDs are independent.
- Expired entries trigger a fresh API call.
- redact=True returns a deep copy with Authorization redacted, not the cached object.
-- NO_AUTH apps are cached just like regular apps.
"""
import time
@@ -30,15 +29,7 @@
"toolkit_slug": "gmail",
}
-NO_AUTH_MOCK_DATA: t.Dict[str, t.Any] = {
- "headers": {},
- "queryParams": {},
- "base_url": None,
- "toolkit_slug": "hackernews",
- "composio": {"composio_api_key": "test-key"},
-}
-
def _reset_cache() -> None:
"""Clear the module-level cache between tests."""
import cortex.common.composio_client as m