Skip to content

Support Redis TLS server name#105

Merged
sjmiller609 merged 1 commit into
mainfrom
hypeship/redis-tls-server-name
May 28, 2026
Merged

Support Redis TLS server name#105
sjmiller609 merged 1 commit into
mainfrom
hypeship/redis-tls-server-name

Conversation

@sjmiller609
Copy link
Copy Markdown
Contributor

@sjmiller609 sjmiller609 commented May 28, 2026

Summary

  • read REDIS_TLS_SERVER_NAME in the MCP Redis client
  • pass it through as the TLS servername for rediss:// connections
  • document the optional env var in the example env file

Testing

  • bunx prettier --check src/lib/redis.ts
  • bunx tsc --noEmit
  • bun run build reaches compile/typecheck, then local prerender requires valid Clerk env values

Note

Low Risk
Small, opt-in connection configuration for managed Redis TLS; no changes to auth or data key logic.

Overview
Adds optional TLS SNI support for Redis when connecting over rediss://.

The MCP Redis client now reads REDIS_TLS_SERVER_NAME and, when set, configures the socket with tls: true and servername (using the URL hostname for host). Startup fails fast if that variable is set but REDIS_URL is not rediss://. Reconnect backoff is unchanged; it is just shared via a small refactor.

.env.example documents the new optional variable.

Reviewed by Cursor Bugbot for commit 28fe644. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mcp Ready Ready Preview, Comment May 28, 2026 5:35pm

@sjmiller609 sjmiller609 marked this pull request as ready for review May 28, 2026 17:37
@sjmiller609 sjmiller609 requested a review from rgarcia May 28, 2026 17:37
@firetiger-agent
Copy link
Copy Markdown

Firetiger deploy monitoring skipped

This PR didn't match the auto-monitor filter configured on your GitHub connection:

Any PR that changes the kernel API. Monitor changes to API endpoints (packages/api/cmd/api/) and Temporal workflows (packages/api/lib/temporal) in the kernel repo

Reason: Changes are to Redis client configuration (src/lib/redis.ts) and documentation, not to API endpoints (packages/api/cmd/api/) or Temporal workflows (packages/api/lib/temporal) as specified in the filter.

To monitor this PR anyway, reply with @firetiger monitor this.

@sjmiller609 sjmiller609 merged commit 74f8ac7 into main May 28, 2026
10 checks passed
@sjmiller609 sjmiller609 deleted the hypeship/redis-tls-server-name branch May 28, 2026 17:41
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 28fe644. Configure here.

Comment thread src/lib/redis.ts
tls: true,
servername: redisTlsServerName,
reconnectStrategy,
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Redundant socket.host may drop non-standard URL port

Medium Severity

Setting socket.host without socket.port when url is also provided risks losing the port from the URL. If socket.host takes effect, socket.port defaults to 6379, silently ignoring any non-standard port in the rediss:// URL. The official node-redis pattern for adding servername alongside a url only sets tls and servername in the socket config—not host. Omitting socket.host (and socket.tls, which rediss:// already implies) avoids this risk and removes the need for parsedRedisUrl and the non-null assertion entirely.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 28fe644. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants