Skip to content

Conversation

@mjfork
Copy link

@mjfork mjfork commented Jan 31, 2026

Summary

  • Add optional HTTPS support via SSL_CERT and SSL_KEY environment variables
  • Server automatically uses HTTPS when both variables are set and certificate files exist
  • Falls back to HTTP when SSL is not configured (backward compatible)

Fixes #361

Changes

  • server/index.js: Import https module, conditionally create HTTPS server when SSL configured
  • .env.example: Added SSL/TLS configuration section with documentation

Example Usage

# Generate self-signed cert (or use Let's Encrypt, etc.)
openssl req -x509 -nodes -days 365 -newkey rsa:2048 \
  -keyout server.key -out server.crt -subj "/CN=localhost"

# Configure in .env
SSL_CERT=/path/to/server.crt
SSL_KEY=/path/to/server.key
PORT=443

Test plan

  • Without SSL_CERT/SSL_KEY: server starts with HTTP (existing behavior)
  • With valid SSL_CERT/SSL_KEY: server starts with HTTPS
  • With invalid paths: server falls back to HTTP gracefully
  • WebSocket connections work over both HTTP and HTTPS

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Optional HTTPS/SSL support: server can run with encrypted connections when certificate and key paths are provided via environment variables; secure mode is enabled automatically and logged.
  • Improvements

    • Protocol-aware startup messaging and clearer logs indicating HTTP vs HTTPS, including automatic fallback and notifications when secure files or settings are missing.

✏️ Tip: You can customize this high-level summary in your review settings.

Add support for running the server with HTTPS by configuring SSL_CERT
and SSL_KEY environment variables. When both variables are set and the
certificate files exist, the server automatically uses HTTPS instead
of HTTP.

This enables secure connections for deployments that require encryption,
while maintaining full backward compatibility (HTTP remains the default).

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 31, 2026

Walkthrough

Adds optional HTTPS support: .env.example documents SSL_CERT, SSL_KEY, and SSL_PORT; server/index.js checks for certificate/key files and creates an HTTPS server when both exist (using SSL_PORT if set), otherwise falls back to HTTP. WebSocket attaches to the same server instance.

Changes

Cohort / File(s) Summary
Environment Configuration
\.env.example
Added commented SSL/TLS block with SSL_CERT, SSL_KEY, and SSL_PORT variables and guidance indicating HTTPS is enabled when certificate and key files both exist.
Server Initialization
server/index.js
Reads SSL_CERT, SSL_KEY, and SSL_PORT; checks filesystem for cert/key files; conditionally creates an https server or falls back to http; sets useHttps flag and chooses listen port accordingly; logs protocol-aware server URL; reuses server instance for WebSocket wiring.

Sequence Diagram(s)

sequenceDiagram
  participant Env as "Environment"
  participant FS as "Filesystem"
  participant Server as "Node process"
  participant Client as "Client / Browser"
  participant WS as "WebSocket"

  Env->>Server: Provide PORT, SSL_PORT, SSL_CERT, SSL_KEY
  Server->>FS: Check existence of SSL_CERT & SSL_KEY
  alt both files exist
    Server->>Server: create HTTPS server (https.createServer)
    Server->>WS: attach WebSocket to HTTPS server
    Server->>Server: listen on SSL_PORT or PORT
  else missing/invalid
    Server->>Server: create HTTP server (http.createServer)
    Server->>WS: attach WebSocket to HTTP server
    Server->>Server: listen on PORT
  end
  Server->>Client: Log protocol-aware URL (http(s)://host:port)
  Client->>Server: Connect via http(s)
  Client->>WS: Establish WebSocket over same server
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I found a cert and key beneath the log door,
I stitched a tiny HTTPS tunnel on the floor.
Sockets now hop through secure, cozy night,
My hops keep packets snug till morning light. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding optional SSL/TLS support for HTTPS to the server.
Linked Issues check ✅ Passed The PR implementation fully addresses all coding requirements from issue #361: SSL_CERT and SSL_KEY environment variables, conditional HTTPS activation with file existence checks, graceful fallback to HTTP, and backward compatibility.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing SSL/TLS support as specified in issue #361; no extraneous modifications detected in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@server/index.js`:
- Around line 1844-1845: The log currently derives protocol from
SSL_CERT/SSL_KEY which can be wrong if TLS setup fails; change the console.log
that prints Server URL to use the actual runtime flag `useHttps` (the boolean
produced by your server creation logic) instead of checking SSL_CERT/SSL_KEY,
e.g. compute protocol = useHttps ? 'https' : 'http' and use that in the
c.bright(...) output so the log reflects the real server mode (refer to the
`useHttps` variable and the existing console.log line).
- Around line 193-207: Wrap the SSL certificate reading and HTTPS server
creation in a try/catch to handle read permissions, invalid/corrupt certs, or
createServer errors: when SSL_CERT/SSL_KEY are present and files exist, attempt
to fs.readFileSync both files and call https.createServer inside a try block,
set a boolean flag useHttps = true on success and fall back to http.createServer
with useHttps = false on any thrown error; ensure processLogger/console.error
logs include the caught error and a clear message; finally, update the
startup/protocol log to reference the useHttps flag (not just presence of
SSL_CERT/SSL_KEY) so it accurately reflects whether the HTTPS server was
actually created.

mjfork and others added 2 commits January 31, 2026 22:10
- Wrap SSL certificate reading and HTTPS server creation in try/catch
- Add useHttps flag to track actual server state
- Update protocol log to use useHttps flag instead of env vars
- Gracefully fallback to HTTP if certificate loading fails

Addresses code review feedback from CodeRabbit.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Allow configuring a separate port for HTTPS via SSL_PORT environment
variable. When SSL is enabled and SSL_PORT is set, the server listens
on that port. Otherwise, it falls back to PORT.

This enables common setups where HTTP runs on port 3001 and HTTPS
runs on port 443.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
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.

Feature Request: Optional SSL/TLS support for HTTPS

1 participant