Skip to content

Commit 0ffe07b

Browse files
akolotovclaude
andcommitted
test: address code review feedback on PRO API key tests
- Route malformed-key redaction tests through the real extraction → scope → resolve path instead of injecting _Malformed() directly, so redaction is verified in the actual request flow. - Move the _Malformed import to the module header in the contract test. - Make header name/value leak checks case-insensitive in the decorator and analytics tests. - Assert the periodic-progress callback was actually awaited. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent aa5ccfe commit 0ffe07b

5 files changed

Lines changed: 49 additions & 18 deletions

File tree

tests/test_analytics.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,8 @@ def test_pro_api_key_not_in_analytics_payload(monkeypatch):
200200
# Inspect every string in the call for the key value and header name
201201
all_text = str(call_args)
202202
assert client_key not in all_text
203-
assert "Blockscout-MCP-Pro-Api-Key" not in all_text
203+
# Case-insensitive: catch a leaked header name regardless of casing.
204+
assert "blockscout-mcp-pro-api-key" not in all_text.lower()
204205

205206
# Also explicitly check that the properties dict doesn't contain the key
206207
args, kwargs = call_args

tests/test_pro_api_key_context.py

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -216,24 +216,52 @@ def test_resolve_malformed_does_not_fall_back_to_server_key(monkeypatch):
216216
# ===========================================================================
217217

218218

219-
def test_malformed_error_does_not_embed_control_char_value(monkeypatch):
220-
"""ValueError message must not reproduce the raw submitted value."""
219+
@pytest.mark.asyncio
220+
async def test_malformed_error_does_not_embed_control_char_value(monkeypatch):
221+
"""The malformed-key ValueError must not reproduce the raw submitted value.
222+
223+
The value flows through the real extraction → scope → resolve path: the
224+
decorator extracts the control-char header, records the ``_Malformed`` state,
225+
and ``resolve_pro_api_key()`` raises inside the scope.
226+
"""
227+
monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False)
221228
monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False)
222229
raw_value = "key-with\ncontrol-char"
223-
with _set_key_state(_Malformed()):
224-
with pytest.raises(ValueError) as exc_info:
225-
resolve_pro_api_key()
226-
assert raw_value not in str(exc_info.value)
230+
231+
@pro_api_key_scope
232+
async def dummy(ctx) -> None:
233+
resolve_pro_api_key()
234+
235+
# Plain dict headers: real starlette Headers reject control-char values at
236+
# construction time, while the extractor works with any Mapping.
237+
request = SimpleNamespace(headers={"Blockscout-MCP-Pro-Api-Key": raw_value})
238+
ctx = SimpleNamespace(request_context=SimpleNamespace(request=request))
239+
240+
with pytest.raises(ValueError) as exc_info:
241+
await dummy(ctx=ctx)
242+
assert raw_value not in str(exc_info.value)
227243

228244

229-
def test_malformed_error_does_not_embed_over_length_value(monkeypatch):
230-
"""ValueError message must not reproduce an over-length submitted value."""
245+
@pytest.mark.asyncio
246+
async def test_malformed_error_does_not_embed_over_length_value(monkeypatch):
247+
"""The malformed-key ValueError must not reproduce an over-length submitted value.
248+
249+
Same real extraction → scope → resolve path as the control-char case.
250+
"""
251+
monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False)
231252
monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False)
232253
raw_value = "a" * 257
233-
with _set_key_state(_Malformed()):
234-
with pytest.raises(ValueError) as exc_info:
235-
resolve_pro_api_key()
236-
assert raw_value not in str(exc_info.value)
254+
255+
@pro_api_key_scope
256+
async def dummy(ctx) -> None:
257+
resolve_pro_api_key()
258+
259+
request = SimpleNamespace(headers={"Blockscout-MCP-Pro-Api-Key": raw_value})
260+
ctx = SimpleNamespace(request_context=SimpleNamespace(request=request))
261+
262+
with pytest.raises(ValueError) as exc_info:
263+
await dummy(ctx=ctx)
264+
assert raw_value not in str(exc_info.value)
237265

238266

239267
# ===========================================================================

tests/tools/contract/test_fetch_and_process_contract.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from blockscout_mcp_server.cache import CachedContract
77
from blockscout_mcp_server.config import config
8-
from blockscout_mcp_server.pro_api_key_context import _client_key_state, _Valid
8+
from blockscout_mcp_server.pro_api_key_context import _client_key_state, _Malformed, _Valid
99
from blockscout_mcp_server.tools.contract._shared import _fetch_and_process_contract
1010

1111

@@ -259,8 +259,6 @@ async def test_fetch_and_process_cache_hit_serverless_valid_client_key(mock_ctx)
259259
@pytest.mark.asyncio
260260
async def test_fetch_and_process_cache_hit_malformed_key_fails_closed(mock_ctx):
261261
"""Cache hit with malformed client key: gate raises ValueError before cache is consulted."""
262-
from blockscout_mcp_server.pro_api_key_context import _Malformed
263-
264262
cached = CachedContract(metadata={}, source_files={})
265263
token = _client_key_state.set(_Malformed())
266264
try:

tests/tools/test_common_blockscout_request_auth.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,8 @@ async def test_get_context_var_propagates_into_periodic_progress_task(monkeypatc
251251
_client_key_state.reset(token)
252252

253253
assert result == {"result": "ok"}
254+
# Progress beats must have been emitted along the periodic-progress path.
255+
mock_ctx.report_progress.assert_awaited()
254256
# The key must have been visible inside the spawned child task
255257
assert fake_client.get_headers.get("Authorization") == "Bearer propagated-client-key"
256258

tests/tools/test_decorators.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,5 +277,7 @@ async def dummy_tool(a: int, ctx: Context) -> int:
277277

278278
await dummy_tool(7, ctx=mock_ctx)
279279

280-
assert client_key not in caplog.text
281-
assert "Blockscout-MCP-Pro-Api-Key" not in caplog.text
280+
# Case-insensitive: catch a leaked header name/value regardless of logger casing.
281+
logged_text = caplog.text.lower()
282+
assert client_key.lower() not in logged_text
283+
assert "blockscout-mcp-pro-api-key" not in logged_text

0 commit comments

Comments
 (0)