From 5a8e82afc56fb82c484244ec57ec0098838db856 Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Tue, 3 Feb 2026 18:18:07 +0000 Subject: [PATCH] fix: Use TLS dial for HTTPS healthchecks to prevent handshake warnings The healthcheck functionality was using raw TCP connections which caused TLS handshake warnings on HTTPS servers with self-signed certificates. Changes: - Use tls.DialWithDialer for HTTPS endpoints with InsecureSkipVerify respecting the --insecure flag - Add --no-healthcheck flag to disable health monitoring entirely - Update tests for HTTPS healthcheck scenarios - Update documentation for both flags Fixes #198 Co-authored-by: Cursor --- README.md | 10 +- REFERENCE.md | 4 + pkg/cmd/listen.go | 4 + pkg/listen/healthcheck.go | 7 +- pkg/listen/healthcheck/healthcheck.go | 26 ++++- pkg/listen/healthcheck/healthcheck_test.go | 124 ++++++++++++++++++++- pkg/listen/listen.go | 42 +++---- pkg/listen/proxy/proxy.go | 28 +++-- 8 files changed, 203 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index a89558a..814b533 100644 --- a/README.md +++ b/README.md @@ -394,7 +394,7 @@ hookdeck logout When forwarding events to an HTTPS URL as the first argument to `hookdeck listen` (e.g., `https://localhost:1234/webhook`), you might encounter SSL validation errors if the destination is using a self-signed certificate. -For local development scenarios, you can instruct the `listen` command to bypass this SSL certificate validation by using its `--insecure` flag. You must provide the full HTTPS URL. +For local development scenarios, you can instruct the `listen` command to bypass this SSL certificate validation by using its `--insecure` flag. You must provide the full HTTPS URL. This flag also applies to the periodic server health checks that the CLI performs. **This is dangerous and should only be used in trusted local development environments for destinations you control.** @@ -404,6 +404,14 @@ Example of skipping SSL validation for an HTTPS destination: hookdeck listen --insecure https:/// ``` +### Disable health checks + +The CLI periodically checks if your local server is reachable and displays warnings if the connection fails. If these health checks cause issues in your environment, you can disable them with the `--no-healthcheck` flag: + +```sh +hookdeck listen --no-healthcheck 3000 +``` + ### Version Print your CLI version and whether or not a new version is available. diff --git a/REFERENCE.md b/REFERENCE.md index 33aae2e..8c1e20d 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -186,6 +186,7 @@ hookdeck project use --profile production [source] # Optional positional argument for source name [connection] # Optional positional argument for connection name --path string # Specific path to forward to (e.g., "/webhooks") +--no-healthcheck # Disable periodic health checks of the local server --no-wss # Force unencrypted WebSocket connection (hidden flag) ``` @@ -206,6 +207,9 @@ hookdeck listen 3000 stripe-webhooks payment-connection # Forward to specific path hookdeck listen --path /webhooks +# Disable periodic health checks of the local server +hookdeck listen --no-healthcheck 3000 + # Force unencrypted WebSocket connection (hidden flag) hookdeck listen --no-wss diff --git a/pkg/cmd/listen.go b/pkg/cmd/listen.go index 25e925d..88a9505 100644 --- a/pkg/cmd/listen.go +++ b/pkg/cmd/listen.go @@ -32,6 +32,7 @@ import ( type listenCmd struct { cmd *cobra.Command noWSS bool + noHealthcheck bool path string maxConnections int output string @@ -155,6 +156,8 @@ Destination CLI path will be "/". To set the CLI path, use the "--path" flag.`, lc.cmd.Flags().StringVar(&lc.output, "output", "interactive", "Output mode: interactive (full UI), compact (simple logs), quiet (errors and warnings only)") + lc.cmd.Flags().BoolVar(&lc.noHealthcheck, "no-healthcheck", false, "Disable periodic health checks of the local server") + lc.cmd.Flags().StringVar(&lc.filterBody, "filter-body", "", "Filter events by request body using Hookdeck filter syntax (JSON)") lc.cmd.Flags().StringVar(&lc.filterHeaders, "filter-headers", "", "Filter events by request headers using Hookdeck filter syntax (JSON)") lc.cmd.Flags().StringVar(&lc.filterQuery, "filter-query", "", "Filter events by query parameters using Hookdeck filter syntax (JSON)") @@ -255,6 +258,7 @@ func (lc *listenCmd) runListenCmd(cmd *cobra.Command, args []string) error { return listen.Listen(url, sourceQuery, connectionQuery, listen.Flags{ NoWSS: lc.noWSS, + NoHealthcheck: lc.noHealthcheck, Path: lc.path, Output: lc.output, MaxConnections: lc.maxConnections, diff --git a/pkg/listen/healthcheck.go b/pkg/listen/healthcheck.go index 567d4f3..4cb6324 100644 --- a/pkg/listen/healthcheck.go +++ b/pkg/listen/healthcheck.go @@ -16,10 +16,11 @@ const ( HealthUnreachable = healthcheck.HealthUnreachable ) -// CheckServerHealth performs a TCP connection check to the target URL +// CheckServerHealth performs a connection check to the target URL +// For HTTPS URLs, it performs a TLS handshake with optional certificate verification skip. // This is a wrapper around the healthcheck package function for backward compatibility -func CheckServerHealth(targetURL *url.URL, timeout time.Duration) HealthCheckResult { - return healthcheck.CheckServerHealth(targetURL, timeout) +func CheckServerHealth(targetURL *url.URL, timeout time.Duration, insecure bool) HealthCheckResult { + return healthcheck.CheckServerHealth(targetURL, timeout, insecure) } // FormatHealthMessage creates a user-friendly health status message diff --git a/pkg/listen/healthcheck/healthcheck.go b/pkg/listen/healthcheck/healthcheck.go index 9ac5c48..466b198 100644 --- a/pkg/listen/healthcheck/healthcheck.go +++ b/pkg/listen/healthcheck/healthcheck.go @@ -1,6 +1,7 @@ package healthcheck import ( + "crypto/tls" "fmt" "net" "net/url" @@ -27,11 +28,14 @@ type HealthCheckResult struct { Duration time.Duration } -// CheckServerHealth performs a TCP connection check to verify a server is listening. +// CheckServerHealth performs a connection check to verify a server is listening. +// For HTTPS URLs, it performs a TLS handshake to avoid incomplete handshake warnings +// on the server side. The insecure parameter controls whether to skip TLS certificate +// verification (matching the --insecure flag behavior for webhook forwarding). // The timeout parameter should be appropriate for the deployment context: // - Local development: 3s is typically sufficient // - Production/edge: May require longer timeouts due to network conditions -func CheckServerHealth(targetURL *url.URL, timeout time.Duration) HealthCheckResult { +func CheckServerHealth(targetURL *url.URL, timeout time.Duration, insecure bool) HealthCheckResult { start := time.Now() host := targetURL.Hostname() @@ -48,7 +52,23 @@ func CheckServerHealth(targetURL *url.URL, timeout time.Duration) HealthCheckRes address := net.JoinHostPort(host, port) - conn, err := net.DialTimeout("tcp", address, timeout) + var conn net.Conn + var err error + + if targetURL.Scheme == "https" { + // Use TLS connection for HTTPS endpoints to complete handshake properly + // and avoid TLS handshake warnings on the server + dialer := &net.Dialer{Timeout: timeout} + tlsConfig := &tls.Config{ + InsecureSkipVerify: insecure, + ServerName: host, + } + conn, err = tls.DialWithDialer(dialer, "tcp", address, tlsConfig) + } else { + // Use plain TCP for HTTP endpoints + conn, err = net.DialTimeout("tcp", address, timeout) + } + duration := time.Since(start) result := HealthCheckResult{ diff --git a/pkg/listen/healthcheck/healthcheck_test.go b/pkg/listen/healthcheck/healthcheck_test.go index a2e8c92..288bbb4 100644 --- a/pkg/listen/healthcheck/healthcheck_test.go +++ b/pkg/listen/healthcheck/healthcheck_test.go @@ -24,8 +24,8 @@ func TestCheckServerHealth_HealthyServer(t *testing.T) { t.Fatalf("Failed to parse server URL: %v", err) } - // Perform health check - result := CheckServerHealth(serverURL, 3*time.Second) + // Perform health check (insecure=false, not relevant for HTTP) + result := CheckServerHealth(serverURL, 3*time.Second, false) // Verify result if !result.Healthy { @@ -50,7 +50,7 @@ func TestCheckServerHealth_UnreachableServer(t *testing.T) { } // Perform health check - result := CheckServerHealth(targetURL, 1*time.Second) + result := CheckServerHealth(targetURL, 1*time.Second, false) // Verify result if result.Healthy { @@ -101,8 +101,8 @@ func TestCheckServerHealth_DefaultPorts(t *testing.T) { } defer listener.Close() - // Perform health check - result := CheckServerHealth(targetURL, 1*time.Second) + // Perform health check (insecure=true to handle self-signed certs in test) + result := CheckServerHealth(targetURL, 1*time.Second, true) // Should be healthy since we have a listener if !result.Healthy { @@ -185,7 +185,7 @@ func TestCheckServerHealth_PortInURL(t *testing.T) { targetURL, _ := url.Parse(fmt.Sprintf("http://localhost:%d/path", addr.Port)) // Perform health check - result := CheckServerHealth(targetURL, 3*time.Second) + result := CheckServerHealth(targetURL, 3*time.Second, false) // Verify that the health check succeeded // This confirms that when a port is already in the URL, we don't append @@ -197,3 +197,115 @@ func TestCheckServerHealth_PortInURL(t *testing.T) { t.Errorf("Expected no error for server with port in URL, got: %v", result.Error) } } + +func TestCheckServerHealth_HTTPS_SelfSigned_InsecureTrue(t *testing.T) { + // Start a test HTTPS server with self-signed certificate + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // Parse server URL (will be https://...) + serverURL, err := url.Parse(server.URL) + if err != nil { + t.Fatalf("Failed to parse server URL: %v", err) + } + + // Verify it's HTTPS + if serverURL.Scheme != "https" { + t.Fatalf("Expected HTTPS scheme, got: %s", serverURL.Scheme) + } + + // Perform health check with insecure=true (should succeed) + result := CheckServerHealth(serverURL, 3*time.Second, true) + + // Should be healthy because we skip certificate verification + if !result.Healthy { + t.Errorf("Expected server to be healthy with insecure=true, got unhealthy: %v", result.Error) + } + if result.Status != HealthHealthy { + t.Errorf("Expected status HealthHealthy, got %v", result.Status) + } + if result.Error != nil { + t.Errorf("Expected no error with insecure=true, got: %v", result.Error) + } +} + +func TestCheckServerHealth_HTTPS_SelfSigned_InsecureFalse(t *testing.T) { + // Start a test HTTPS server with self-signed certificate + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // Parse server URL (will be https://...) + serverURL, err := url.Parse(server.URL) + if err != nil { + t.Fatalf("Failed to parse server URL: %v", err) + } + + // Perform health check with insecure=false (should fail due to self-signed cert) + result := CheckServerHealth(serverURL, 3*time.Second, false) + + // Should be unhealthy because certificate verification fails + if result.Healthy { + t.Errorf("Expected server to be unhealthy with insecure=false on self-signed cert, got healthy") + } + if result.Status != HealthUnreachable { + t.Errorf("Expected status HealthUnreachable, got %v", result.Status) + } + if result.Error == nil { + t.Errorf("Expected certificate error, got nil") + } + // Verify it's a certificate-related error + if result.Error != nil && !strings.Contains(result.Error.Error(), "certificate") { + t.Logf("Error message: %v (may vary by platform)", result.Error) + } +} + +func TestCheckServerHealth_HTTPS_UsesTLSHandshake(t *testing.T) { + // This test verifies that HTTPS URLs use TLS dial (not raw TCP) + // by using httptest.NewTLSServer which creates a proper TLS server + // and checking that the health check completes successfully + + // Start a test HTTPS server - this will only succeed if TLS handshake completes + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + serverURL, err := url.Parse(server.URL) + if err != nil { + t.Fatalf("Failed to parse server URL: %v", err) + } + + // Verify it's HTTPS + if serverURL.Scheme != "https" { + t.Fatalf("Expected HTTPS scheme, got: %s", serverURL.Scheme) + } + + // Perform health check with insecure=true (to accept self-signed cert) + // If this succeeds, it proves TLS handshake was performed (not just TCP connect) + result := CheckServerHealth(serverURL, 3*time.Second, true) + + // Should be healthy - this proves TLS handshake succeeded + if !result.Healthy { + t.Errorf("Expected healthy result for HTTPS server (TLS handshake should succeed), got: %v", result.Error) + } + + // Verify that for HTTP URLs, we still use TCP (not TLS) + httpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer httpServer.Close() + + httpURL, _ := url.Parse(httpServer.URL) + if httpURL.Scheme != "http" { + t.Fatalf("Expected HTTP scheme, got: %s", httpURL.Scheme) + } + + httpResult := CheckServerHealth(httpURL, 3*time.Second, false) + if !httpResult.Healthy { + t.Errorf("Expected healthy result for HTTP server, got: %v", httpResult.Error) + } +} diff --git a/pkg/listen/listen.go b/pkg/listen/listen.go index bf4bd65..10c0d6a 100644 --- a/pkg/listen/listen.go +++ b/pkg/listen/listen.go @@ -35,6 +35,7 @@ import ( type Flags struct { NoWSS bool + NoHealthcheck bool Path string MaxConnections int Output string @@ -123,28 +124,30 @@ Specify a single destination to update the path. For example, pass a connection return err } - // Perform initial health check on target server + // Perform initial health check on target server (unless disabled) // Using 3-second timeout optimized for local development scenarios. // This assumes low latency to localhost. For production/edge deployments, // this timeout may need to be configurable in future iterations. - healthCheckTimeout := 3 * time.Second - healthResult := CheckServerHealth(URL, healthCheckTimeout) - - // For all output modes, warn if server isn't reachable - if !healthResult.Healthy { - warningMsg := FormatHealthMessage(healthResult, URL) - - if flags.Output == "interactive" { - // Interactive mode will show warning before TUI starts - fmt.Println() - fmt.Println(warningMsg) - fmt.Println() - time.Sleep(500 * time.Millisecond) // Give user time to see warning before TUI starts - } else { - // Compact/quiet modes: print warning before connection info - fmt.Println() - fmt.Println(warningMsg) - fmt.Println() + if !flags.NoHealthcheck { + healthCheckTimeout := 3 * time.Second + healthResult := CheckServerHealth(URL, healthCheckTimeout, config.Insecure) + + // For all output modes, warn if server isn't reachable + if !healthResult.Healthy { + warningMsg := FormatHealthMessage(healthResult, URL) + + if flags.Output == "interactive" { + // Interactive mode will show warning before TUI starts + fmt.Println() + fmt.Println(warningMsg) + fmt.Println() + time.Sleep(500 * time.Millisecond) // Give user time to see warning before TUI starts + } else { + // Compact/quiet modes: print warning before connection info + fmt.Println() + fmt.Println(warningMsg) + fmt.Println() + } } } @@ -168,6 +171,7 @@ Specify a single destination to update the path. For example, pass a connection ConsoleBaseURL: config.ConsoleBaseURL, WSBaseURL: config.WSBaseURL, NoWSS: flags.NoWSS, + NoHealthcheck: flags.NoHealthcheck, URL: URL, Log: log.StandardLogger(), Insecure: config.Insecure, diff --git a/pkg/listen/proxy/proxy.go b/pkg/listen/proxy/proxy.go index 5437fae..b39a549 100644 --- a/pkg/listen/proxy/proxy.go +++ b/pkg/listen/proxy/proxy.go @@ -48,8 +48,10 @@ type Config struct { // Force use of unencrypted ws:// protocol instead of wss:// NoWSS bool Insecure bool + // Disable periodic health checks of the local server + NoHealthcheck bool // Output mode: interactive, compact, quiet - Output string + Output string GuestURL string // MaxConnections allows tuning the maximum concurrent connections per host. // Default: 50 concurrent connections @@ -165,13 +167,19 @@ func (p *Proxy) Run(parentCtx context.Context) error { if !hasConnectedOnce { hasConnectedOnce = true - // Perform initial health check and notify renderer immediately - healthy, err := checkServerHealth(p.cfg.URL, 3*time.Second) - p.serverHealthy.Store(healthy) - p.renderer.OnServerHealthChanged(healthy, err) + // Skip health monitoring if disabled via --no-healthcheck flag + if p.cfg.NoHealthcheck { + // Assume server is healthy when healthchecks are disabled + p.serverHealthy.Store(true) + } else { + // Perform initial health check and notify renderer immediately + healthy, err := checkServerHealth(p.cfg.URL, 3*time.Second, p.cfg.Insecure) + p.serverHealthy.Store(healthy) + p.renderer.OnServerHealthChanged(healthy, err) - // Start health check monitor after initial check - go p.startHealthCheckMonitor(signalCtx, p.cfg.URL) + // Start health check monitor after initial check + go p.startHealthCheckMonitor(signalCtx, p.cfg.URL) + } } }() @@ -459,8 +467,8 @@ func (p *Proxy) processEndpointResponse(eventID string, webhookEvent *websocket. } // checkServerHealth is a simple wrapper around the healthcheck package's CheckServerHealth -func checkServerHealth(targetURL *url.URL, timeout time.Duration) (bool, error) { - result := healthcheck.CheckServerHealth(targetURL, timeout) +func checkServerHealth(targetURL *url.URL, timeout time.Duration, insecure bool) (bool, error) { + result := healthcheck.CheckServerHealth(targetURL, timeout, insecure) return result.Healthy, result.Error } @@ -482,7 +490,7 @@ func (p *Proxy) startHealthCheckMonitor(ctx context.Context, targetURL *url.URL) return case <-ticker.C: // Perform health check - healthy, err := checkServerHealth(targetURL, 3*time.Second) + healthy, err := checkServerHealth(targetURL, 3*time.Second, p.cfg.Insecure) // Only notify on state changes, atomically prevHealthy := p.serverHealthy.Swap(healthy)