Skip to content

Re-hash response caching keys to avoid persisting raw request input#3414

Merged
jlowin merged 1 commit intomainfrom
codex/fix-unsafe-cache-key-generation
Mar 7, 2026
Merged

Re-hash response caching keys to avoid persisting raw request input#3414
jlowin merged 1 commit intomainfrom
codex/fix-unsafe-cache-key-generation

Conversation

@jlowin
Copy link
Member

@jlowin jlowin commented Mar 6, 2026

Motivation

  • Response caching previously used plaintext tool names/arguments and resource URIs directly as cache keys, which allows client-controlled values to appear in backend keys and logs and can enable path traversal/oversize key issues for file-backed stores.
  • The change restores fixed-length, safe cache keys (SHA-256) so identity is preserved without storing raw user input in the backing key-value implementation.

Description

  • Added hashlib-backed _hash_cache_key and three key-builder helpers: _make_call_tool_cache_key, _make_read_resource_cache_key, and _make_get_prompt_cache_key, each producing a SHA-256 hex digest of the request-derived string.
  • Updated ResponseCachingMiddleware to call the new helpers for tools/call, resources/read, and prompts/get instead of embedding raw names/arguments/URIs in keys.
  • Added focused unit tests (tests/server/middleware/test_caching.py::TestCacheKeyGeneration) to assert keys are 64-char SHA-256 hex strings, do not contain raw secret/URI fragments, and are stable across repeated calls.
  • PR authored by agent attribution: 🤖 Generated with GPT-5.2-Codex.

Testing

  • Ran uv sync which succeeded.
  • Ran the repository unit tests for the changed file (uv run pytest tests/server/middleware/test_caching.py) and the new unit tests passed (3 passed) and relevant integration checks for caching exercised passed (5 passed).
  • Ran the full test suite (uv run pytest -n auto) which completed but reported a small number of unrelated failures/timeouts (integration/network-related test flakes and proxy/httpx ProxyError) that are external to the key-hashing change.
  • Ran static hooks (uv run prek run --all-files) which failed to initialize hooks due to outbound GitHub access/proxy 403 (environment network constraint), not due to lint/type errors in this change.

Codex Task

@marvin-context-protocol marvin-context-protocol bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. server Related to FastMCP server implementation or server-side functionality. high-priority labels Mar 6, 2026
@jlowin
Copy link
Member Author

jlowin commented Mar 7, 2026

Auto-reviewed and merging on behalf of @jlowin — CI is green (Windows OAuth proxy timeouts are pre-existing flaky tests).

@jlowin jlowin merged commit 0b97aca into main Mar 7, 2026
8 checks passed
@jlowin jlowin deleted the codex/fix-unsafe-cache-key-generation branch March 7, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. codex high-priority server Related to FastMCP server implementation or server-side functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant