Copilot commented on code in PR #40861:
URL: https://github.com/apache/superset/pull/40861#discussion_r3375250946
##########
tests/unit_tests/mcp_service/test_auth_api_key.py:
##########
@@ -113,13 +113,37 @@ def test_api_key_disabled_skips_auth(app) -> None:
# Without API key auth or MCP_DEV_USERNAME, should raise ValueError
# about no authenticated user (not about invalid API key)
- with pytest.raises(ValueError, match="No authenticated user found"):
+ with pytest.raises(ValueError, match="Authentication required"):
get_user_from_request()
# SecurityManager API key methods should never be called
mock_sm.extract_api_key_from_request.assert_not_called()
[email protected]("_disable_api_keys")
+def test_unauthenticated_error_does_not_leak_config(app) -> None:
+ """The error returned to an unauthenticated client must not reveal which
+ auth mechanisms are configured."""
+ app.config["MCP_AUTH_ENABLED"] = True
+ app.config["MCP_JWT_SECRET"] = "super-secret-value" # noqa: S105
+
+ with app.test_request_context():
+ g.user = None
+ with pytest.raises(ValueError, match="Authentication required") as
exc_info:
+ get_user_from_request()
+
+ message = str(exc_info.value)
+ assert message == "Authentication required. No valid credentials provided."
+ for leaked in (
+ "MCP_AUTH_ENABLED",
+ "JWT keys configured",
+ "API key",
+ "MCP_DEV_USERNAME",
+ "super-secret-value",
+ ):
+ assert leaked not in message
+
Review Comment:
This test mutates `app.config` (MCP_AUTH_ENABLED / MCP_JWT_SECRET) but
doesn’t restore it. The `app` fixture is module-scoped
(tests/unit_tests/conftest.py), so these config changes can leak into later
tests in this module and create order-dependent failures. Also, the PR
description mentions that the diagnostic detail is still logged server-side,
but this test doesn’t assert that (using `caplog`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
