codeant-ai-for-open-source[bot] commented on code in PR #37216:
URL: https://github.com/apache/superset/pull/37216#discussion_r2701895630
##########
superset/mcp_service/storage.py:
##########
@@ -109,15 +114,63 @@ def _create_redis_store(
return None
try:
+ # Parse URL to handle SSL properly
+ parsed = urlparse(redis_url)
+ use_ssl = parsed.scheme == "rediss"
+
+ # RedisStore doesn't handle SSL from URL - it parses URL manually
+ # and ignores the scheme. We must create the Redis client ourselves.
+ from redis.asyncio import Redis
+
+ db = 0
+ if parsed.path and parsed.path != "/":
+ try:
+ db = int(parsed.path.strip("/"))
+ except ValueError:
+ db = 0
+
+ if use_ssl:
+ # For ElastiCache with self-signed certs, disable cert verification
+ redis_client = Redis(
+ host=parsed.hostname or "localhost",
+ port=parsed.port or 6379,
+ db=db,
+ password=parsed.password,
+ decode_responses=True,
+ ssl=True,
+ ssl_cert_reqs=None, # Disable cert verification for
ElastiCache
Review Comment:
**Suggestion:** In the SSL branch the Redis client is created with
`ssl_cert_reqs=None`, which is incorrect and insecure: the redis client expects
a valid SSL verification constant (e.g. `ssl.CERT_NONE`) not `None`, and
passing `None` can either raise a runtime error or silently misconfigure SSL.
Replace `ssl_cert_reqs=None` with `ssl.CERT_NONE` and import the `ssl` module
locally; also document that disabling cert verification is insecure and should
be controlled by configuration. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Redis SSL connections may fail connecting
- ⚠️ SSL verification disabled insecurely by default
- ❌ MCP EventStore session sharing affected
```
</details>
```suggestion
import ssl
redis_client = Redis(
host=parsed.hostname or "localhost",
port=parsed.port or 6379,
db=db,
password=parsed.password,
decode_responses=True,
ssl=True,
ssl_cert_reqs=ssl.CERT_NONE, # Explicitly use ssl.CERT_NONE
if verification is intentionally disabled
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Flask app with MCP_STORE_CONFIG enabling Redis and an SSL URL
(e.g. set
MCP_STORE_CONFIG['CACHE_REDIS_URL']='rediss://your-elasticache:6379/1') in
your Superset
config.
2. Start Superset (or the MCP service) so get_mcp_store() is called;
get_mcp_store() ->
_get_store() calls _create_redis_store() (see
superset/mcp_service/storage.py:_get_store()
around lines 66-74).
3. Inside _create_redis_store (superset/mcp_service/storage.py lines
~84-177) urlparse
detects scheme 'rediss' and evaluates the if use_ssl branch at
storage.py:132 which
executes the Redis(...) call with ssl_cert_reqs=None (storage.py lines
132-143).
4. Observe either an SSL runtime error from redis.asyncio client or a
silently
misconfigured SSL handshake (client expects ssl.CERT_* constants from the
ssl module)
causing failed connections or insecure verification for EventStore/SSE
sessions.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/storage.py
**Line:** 134:141
**Comment:**
*Security: In the SSL branch the Redis client is created with
`ssl_cert_reqs=None`, which is incorrect and insecure: the redis client expects
a valid SSL verification constant (e.g. `ssl.CERT_NONE`) not `None`, and
passing `None` can either raise a runtime error or silently misconfigure SSL.
Replace `ssl_cert_reqs=None` with `ssl.CERT_NONE` and import the `ssl` module
locally; also document that disabling cert verification is insecure and should
be controlled by configuration.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
