Skip to content

Fix SSL hostname verification bug and update env var names #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix SSL hostname verification bug and update env var names #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
079ebd6
Fix SSL hostname verification bug and update env var names
taylorleese Nov 4, 2025
c4b84f7
style: format src/common/config.py
vchomakov Nov 4, 2025
c7bd0ad
fix: add ssl_check_hostname to connection test mocks
vchomakov Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ REDIS_SSL_KEYFILE=/path/to/key.pem
REDIS_SSL_CERTFILE=/path/to/cert.pem
REDIS_SSL_CERT_REQS=required
REDIS_SSL_CA_CERTS=/path/to/ca_certs.pem
REDIS_SSL_CHECK_HOSTNAME=true
REDIS_CLUSTER_MODE=False
29 changes: 15 additions & 14 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,20 +336,21 @@ uvx --from redis-mcp-server@latest redis-mcp-server --help

If desired, you can use environment variables. Defaults are provided for all variables.

| Name | Description | Default Value |
|----------------------|-----------------------------------------------------------|---------------|
| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` |
| `REDIS_PORT` | Redis port | `6379` |
| `REDIS_DB` | Database | 0 |
| `REDIS_USERNAME` | Default database username | `"default"` |
| `REDIS_PWD` | Default database password | "" |
| `REDIS_SSL` | Enables or disables SSL/TLS | `False` |
| `REDIS_SSL_CA_PATH` | CA certificate for verifying server | None |
| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None |
| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None |
| `REDIS_SSL_CERT_REQS`| Whether the client should verify the server's certificate | `"required"` |
| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None |
| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` |
| Name | Description | Default Value |
|----------------------------|------------------------------------------------------------------- |---------------|
| `REDIS_HOST` | Redis IP or hostname | `"127.0.0.1"` |
| `REDIS_PORT` | Redis port | `6379` |
| `REDIS_DB` | Database | 0 |
| `REDIS_USERNAME` | Default database username | `"default"` |
| `REDIS_PWD` | Default database password | "" |
| `REDIS_SSL` | Enables or disables SSL/TLS | `False` |
| `REDIS_SSL_CA_PATH` | CA certificate path for verifying server | None |
| `REDIS_SSL_KEYFILE` | Client's private key file for client authentication | None |
| `REDIS_SSL_CERTFILE` | Client's certificate file for client authentication | None |
| `REDIS_SSL_CERT_REQS` | Certificate requirements (none, optional, or required) | `"required"` |
| `REDIS_SSL_CA_CERTS` | Path to the trusted CA certificates file | None |
| `REDIS_SSL_CHECK_HOSTNAME` | Verify SSL certificate hostname (auto-disabled when cert_reqs=none)| `True` |
Copy link
Member

@vchomakov vchomakov Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when REDIS_SSL_CERT_REQS=none

| `REDIS_CLUSTER_MODE` | Enable Redis Cluster mode | `False` |

### EntraID Authentication for Azure Managed Redis

Expand Down
13 changes: 13 additions & 0 deletions src/common/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@
"db": int(os.getenv("REDIS_DB", 0)),
}

# When ssl_cert_reqs is "none", we should disable hostname checking by default
# This matches the behavior of redis-cli --insecure flag
default_check_hostname = "false" if REDIS_CFG["ssl_cert_reqs"] == "none" else "true"
REDIS_CFG["ssl_check_hostname"] = os.getenv(
"REDIS_SSL_CHECK_HOSTNAME", default_check_hostname
) in ("true", "1", "t")

# Entra ID Authentication Configuration
ENTRAID_CFG = {
# Authentication flow selection
Expand Down Expand Up @@ -125,6 +132,12 @@ def parse_redis_uri(uri: str) -> dict:
config["ssl_keyfile"] = query_params["ssl_keyfile"][0]
if "ssl_certfile" in query_params:
config["ssl_certfile"] = query_params["ssl_certfile"][0]
if "ssl_check_hostname" in query_params:
config["ssl_check_hostname"] = query_params["ssl_check_hostname"][0] in (
"true",
"1",
"t",
)

# Handle other parameters. According to https://www.iana.org/assignments/uri-schemes/prov/redis,
# The database number to use for the Redis SELECT command comes from
Expand Down
2 changes: 2 additions & 0 deletions src/common/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def get_connection(cls, decode_responses=True) -> Redis:
"ssl_certfile": REDIS_CFG["ssl_certfile"],
"ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"],
"ssl_ca_certs": REDIS_CFG["ssl_ca_certs"],
"ssl_check_hostname": REDIS_CFG["ssl_check_hostname"],
"decode_responses": decode_responses,
"lib_name": f"redis-py(mcp-server_v{__version__})",
"max_connections_per_node": 10,
Expand All @@ -72,6 +73,7 @@ def get_connection(cls, decode_responses=True) -> Redis:
"ssl_certfile": REDIS_CFG["ssl_certfile"],
"ssl_cert_reqs": REDIS_CFG["ssl_cert_reqs"],
"ssl_ca_certs": REDIS_CFG["ssl_ca_certs"],
"ssl_check_hostname": REDIS_CFG["ssl_check_hostname"],
"decode_responses": decode_responses,
"lib_name": f"redis-py(mcp-server_v{__version__})",
"max_connections": 10,
Expand Down
86 changes: 86 additions & 0 deletions tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,22 @@ def test_parse_uri_with_ssl_parameters(self):
assert result["ssl_certfile"] == "/cert.pem"
assert result["ssl_ca_path"] == "/ca.pem"

def test_parse_uri_with_ssl_check_hostname(self):
"""Test parsing URI with ssl_check_hostname query parameter."""
uri = "rediss://localhost:6379/0?ssl_check_hostname=false"
result = parse_redis_uri(uri)

assert result["ssl"] is True
assert result["ssl_check_hostname"] is False

def test_parse_uri_with_ssl_check_hostname_true(self):
"""Test parsing URI with ssl_check_hostname set to true."""
uri = "rediss://localhost:6379/0?ssl_check_hostname=true"
result = parse_redis_uri(uri)

assert result["ssl"] is True
assert result["ssl_check_hostname"] is True

def test_parse_uri_defaults(self):
"""Test parsing URI with default values."""
uri = "redis://example.com"
Expand Down Expand Up @@ -286,3 +302,73 @@ def test_config_from_environment(self, mock_load_dotenv):
assert config["port"] == 6380
assert config["ssl"] is True
assert config["cluster_mode"] is True

@patch.dict(
os.environ,
{
"REDIS_SSL": "true",
"REDIS_SSL_CERT_REQS": "none",
},
)
@patch("src.common.config.load_dotenv")
def test_ssl_check_hostname_disabled_with_cert_reqs_none(self, mock_load_dotenv):
"""Test that ssl_check_hostname is disabled by default when ssl_cert_reqs is none."""
# Re-import to get fresh config
import importlib

import src.common.config

importlib.reload(src.common.config)

config = src.common.config.REDIS_CFG

assert config["ssl"] is True
assert config["ssl_cert_reqs"] == "none"
assert config["ssl_check_hostname"] is False

@patch.dict(
os.environ,
{
"REDIS_SSL": "true",
"REDIS_SSL_CERT_REQS": "required",
},
)
@patch("src.common.config.load_dotenv")
def test_ssl_check_hostname_enabled_with_cert_reqs_required(self, mock_load_dotenv):
"""Test that ssl_check_hostname is enabled by default when ssl_cert_reqs is required."""
# Re-import to get fresh config
import importlib

import src.common.config

importlib.reload(src.common.config)

config = src.common.config.REDIS_CFG

assert config["ssl"] is True
assert config["ssl_cert_reqs"] == "required"
assert config["ssl_check_hostname"] is True

@patch.dict(
os.environ,
{
"REDIS_SSL": "true",
"REDIS_SSL_CERT_REQS": "none",
"REDIS_SSL_CHECK_HOSTNAME": "true",
},
)
@patch("src.common.config.load_dotenv")
def test_ssl_check_hostname_override(self, mock_load_dotenv):
"""Test that ssl_check_hostname can be explicitly overridden."""
# Re-import to get fresh config
import importlib

import src.common.config

importlib.reload(src.common.config)

config = src.common.config.REDIS_CFG

assert config["ssl"] is True
assert config["ssl_cert_reqs"] == "none"
assert config["ssl_check_hostname"] is True
9 changes: 9 additions & 0 deletions tests/test_connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def test_get_connection_standalone_mode(self, mock_config, mock_redis_class):
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

mock_redis_instance = Mock()
Expand Down Expand Up @@ -75,6 +76,7 @@ def test_get_connection_cluster_mode(self, mock_config, mock_cluster_class):
"ssl_certfile": "/path/to/cert.pem",
"ssl_cert_reqs": "required",
"ssl_ca_certs": "/path/to/ca-bundle.pem",
"ssl_check_hostname": True,
}[key]

mock_cluster_instance = Mock()
Expand Down Expand Up @@ -114,6 +116,7 @@ def test_get_connection_singleton_behavior(self, mock_config, mock_redis_class):
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

mock_redis_instance = Mock()
Expand Down Expand Up @@ -148,6 +151,7 @@ def test_get_connection_with_decode_responses_false(
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

mock_redis_instance = Mock()
Expand Down Expand Up @@ -176,6 +180,7 @@ def test_get_connection_with_ssl_configuration(self, mock_config, mock_redis_cla
"ssl_certfile": "/path/to/cert.pem",
"ssl_cert_reqs": "optional",
"ssl_ca_certs": "/path/to/ca-bundle.pem",
"ssl_check_hostname": True,
}[key]

mock_redis_instance = Mock()
Expand Down Expand Up @@ -211,6 +216,7 @@ def test_get_connection_includes_version_in_lib_name(
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

mock_redis_instance = Mock()
Expand Down Expand Up @@ -241,6 +247,7 @@ def test_connection_error_handling(self, mock_config, mock_redis_class):
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

# Mock Redis constructor to raise ConnectionError
Expand All @@ -265,6 +272,7 @@ def test_cluster_connection_error_handling(self, mock_config, mock_cluster_class
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

# Mock RedisCluster constructor to raise ConnectionError
Expand Down Expand Up @@ -305,6 +313,7 @@ def test_connection_parameters_filtering(self, mock_config, mock_redis_class):
"ssl_certfile": None,
"ssl_cert_reqs": "required",
"ssl_ca_certs": None,
"ssl_check_hostname": True,
}[key]

mock_redis_instance = Mock()
Expand Down