diff --git a/internal/temporalcli/client.go b/internal/temporalcli/client.go index 230ff5a35..e2786770c 100644 --- a/internal/temporalcli/client.go +++ b/internal/temporalcli/client.go @@ -2,7 +2,9 @@ package temporalcli import ( "context" + "errors" "fmt" + "io/fs" "os" "os/user" @@ -46,7 +48,6 @@ func dialClientWithCodec(cctx *CommandContext, c *cliext.ClientOptions) (client. } c.Identity = "temporal-cli:" + username + "@" + hostname } - // Build client options using cliext builder := &cliext.ClientOptionsBuilder{ CommonOptions: cctx.RootCommand.CommonOptions, @@ -56,6 +57,15 @@ func dialClientWithCodec(cctx *CommandContext, c *cliext.ClientOptions) (client. } clientOpts, err := builder.Build(cctx) if err != nil { + // Build did not return authoritative effective options. Preserve the + // original setup error instead of attaching a guessed address or profile. + var pathErr *fs.PathError + if errors.As(err, &pathErr) { + return nil, nil, newConnectError(&connectDiagnosis{ + Cause: causeCertFileUnreadable, + Detail: pathErr.Path, + }, connectMeta{}, err) + } return nil, nil, err } @@ -87,7 +97,7 @@ func dialClientWithCodec(cctx *CommandContext, c *cliext.ClientOptions) (client. cl, err := client.DialContext(dialCtx, clientOpts) if err != nil { - return nil, nil, err + return nil, nil, dialConnectError(cctx, dialCtx, clientOpts, err) } // Since this namespace value is used by many commands after this call, @@ -97,6 +107,43 @@ func dialClientWithCodec(cctx *CommandContext, c *cliext.ClientOptions) (client. return cl, builder.PayloadCodec, nil } +// dialConnectError enriches a client.DialContext failure with a staged +// connection diagnosis and a suggested fix. See connectdiag.go. +func dialConnectError( + cctx *CommandContext, + dialCtx context.Context, + clientOpts client.Options, + origErr error, +) error { + // If a CLI-side timeout fired, surface its descriptive cause, which is + // otherwise lost ("context deadline exceeded" wins over "command timed + // out after 5s"). + if dialCtx.Err() != nil { + if cause := context.Cause(dialCtx); cause != nil && !errors.Is(cause, dialCtx.Err()) { + origErr = fmt.Errorf("%w (%v)", origErr, cause) + } + } + // Never probe after an interrupt or when the whole command timed out. + if cctx.Err() != nil { + return origErr + } + if v, _ := cctx.Options.EnvLookup.LookupEnv("TEMPORAL_CLI_DISABLE_CONNECT_DIAGNOSIS"); v != "" { + return fmt.Errorf("failed connecting to Temporal server at %v: %w", clientOpts.HostPort, origErr) + } + // Diagnosis uses the remaining command context and applies its own 3s cap. + // ClientConnectTimeout governs only the original dial. + diag := diagnoseConnection(cctx, clientOpts.HostPort, clientOpts.ConnectionOptions.TLS, origErr) + return newConnectError(diag, connectMetaFromOptions(clientOpts), origErr) +} + +func connectMetaFromOptions(resolved client.Options) connectMeta { + return connectMeta{ + Address: resolved.HostPort, + Namespace: resolved.Namespace, + TLSConfigured: resolved.ConnectionOptions.TLS != nil, + } +} + func fixedHeaderOverrideInterceptor( ctx context.Context, method string, req, reply any, diff --git a/internal/temporalcli/client_test.go b/internal/temporalcli/client_test.go index 2aa7ae391..5263e0799 100644 --- a/internal/temporalcli/client_test.go +++ b/internal/temporalcli/client_test.go @@ -1,7 +1,17 @@ package temporalcli_test import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" "net" + "os" "testing" "time" @@ -9,11 +19,13 @@ import ( "github.com/stretchr/testify/require" ) -func TestClientConnectTimeout(t *testing.T) { - // Start a listener that accepts connections but never responds +// startBlackHoleListener starts a listener that accepts connections but never +// responds. +func startBlackHoleListener(t *testing.T) net.Listener { + t.Helper() ln, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) - defer ln.Close() + t.Cleanup(func() { ln.Close() }) go func() { for { conn, err := ln.Accept() @@ -24,6 +36,62 @@ func TestClientConnectTimeout(t *testing.T) { select {} } }() + return ln +} + +// startMTLSListener starts a TLS listener that requires client certificates, +// returning its address and the server CA cert as PEM. +func startMTLSListener(t *testing.T) (addr, caPEM string) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "client-test"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + } + der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + require.NoError(t, err) + leaf, err := x509.ParseCertificate(der) + require.NoError(t, err) + pool := x509.NewCertPool() + pool.AddCert(leaf) + + ln, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{ + Certificates: []tls.Certificate{{Certificate: [][]byte{der}, PrivateKey: key}}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: pool, + }) + require.NoError(t, err) + t.Cleanup(func() { ln.Close() }) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func() { + if tlsConn, ok := conn.(*tls.Conn); ok { + _ = tlsConn.HandshakeContext(context.Background()) + buf := make([]byte, 1) + _ = tlsConn.SetReadDeadline(time.Now().Add(2 * time.Second)) + _, _ = tlsConn.Read(buf) + } + conn.Close() + }() + } + }() + return ln.Addr().String(), string(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) +} + +func TestClientConnectTimeout(t *testing.T) { + ln := startBlackHoleListener(t) h := NewCommandHarness(t) @@ -36,6 +104,164 @@ func TestClientConnectTimeout(t *testing.T) { elapsed := time.Since(start) require.Error(t, res.Err) + assert.Contains(t, res.Err.Error(), "failed connecting to Temporal server at") assert.Contains(t, res.Err.Error(), "deadline exceeded") - assert.Less(t, elapsed, time.Second, "dial should have timed out quickly") + // The friendly cause attached to the connect timeout must survive. + assert.Contains(t, res.Err.Error(), "command timed out after 50ms") + assert.Less(t, elapsed, 4*time.Second, "diagnosis should respect its independent three-second cap") +} + +func TestConnectDiagnosis_MTLSRequired(t *testing.T) { + addr, caPEM := startMTLSListener(t) + + h := NewCommandHarness(t) + res := h.Execute( + "workflow", "list", + "--address", addr, + "--tls", + "--tls-ca-data", caPEM, + "--client-connect-timeout", "2s", + ) + + require.Error(t, res.Err) + assert.Contains(t, res.Err.Error(), "failed connecting to Temporal server at "+addr) + assert.Contains(t, res.Err.Error(), "server requires client certificate (mTLS)") + assert.Empty(t, res.Stderr.String(), "the test harness overrides Fail and captures the error") +} + +func TestConnectDiagnosis_Refused(t *testing.T) { + // Grab a port and close it so nothing is listening. + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := ln.Addr().String() + require.NoError(t, ln.Close()) + + h := NewCommandHarness(t) + res := h.Execute( + "workflow", "list", + "--address", addr, + "--client-connect-timeout", "2s", + ) + + require.Error(t, res.Err) + assert.Contains(t, res.Err.Error(), "failed connecting to Temporal server at "+addr) + assert.Contains(t, res.Err.Error(), "connection refused") + assert.Empty(t, res.Stderr.String(), "the test harness overrides Fail and captures the error") +} + +func TestConnectDiagnosis_DNSFailure(t *testing.T) { + h := NewCommandHarness(t) + res := h.Execute( + "workflow", "list", + // .invalid is reserved (RFC 2606) and can never resolve. + "--address", "does-not-exist.invalid:7233", + "--client-connect-timeout", "5s", + ) + + require.Error(t, res.Err) + assert.Contains(t, res.Err.Error(), "failed connecting to Temporal server at does-not-exist.invalid:7233") + assert.Contains(t, res.Err.Error(), "could not resolve host") + assert.Empty(t, res.Stderr.String(), "the test harness overrides Fail and captures the error") +} + +func TestConnectDiagnosis_JSONOutputHasNoANSI(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := ln.Addr().String() + require.NoError(t, ln.Close()) + + h := NewCommandHarness(t) + res := h.Execute( + "workflow", "list", + "--address", addr, + "--client-connect-timeout", "2s", + "-o", "json", + ) + + require.Error(t, res.Err) + assert.NotContains(t, res.Err.Error(), "\x1b[", "connection errors must remain uncolored") + assert.Empty(t, res.Stderr.String(), "the test harness overrides Fail and captures the error") + assert.Empty(t, res.Stdout.String(), "connection failures must not write to stdout") +} + +func TestConnectDiagnosis_Disabled(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := ln.Addr().String() + require.NoError(t, ln.Close()) + + h := NewCommandHarness(t) + h.Options.EnvLookup = EnvLookupMap{"TEMPORAL_CLI_DISABLE_CONNECT_DIAGNOSIS": "1"} + res := h.Execute( + "workflow", "list", + "--address", addr, + "--client-connect-timeout", "2s", + ) + + require.Error(t, res.Err) + msg := res.Err.Error() + assert.Contains(t, msg, "failed connecting to Temporal server at "+addr) + assert.NotContains(t, msg, "✗", "diagnosis must be suppressed when disabled") + assert.NotContains(t, msg, "Namespace:") + assert.NotContains(t, msg, "TLS:") + assert.NotContains(t, msg, "Connection checks") +} + +func TestConnectDiagnosis_CertFileMissing(t *testing.T) { + h := NewCommandHarness(t) + res := h.Execute( + "workflow", "list", + "--address", "127.0.0.1:7233", + "--tls-cert-path", "/definitely/does/not/exist.pem", + "--tls-key-path", "/definitely/does/not/exist.key", + ) + + require.Error(t, res.Err) + var pathErr *os.PathError + require.ErrorAs(t, res.Err, &pathErr) + msg := res.Err.Error() + assert.Contains(t, msg, "failed preparing Temporal server connection") + assert.NotContains(t, msg, "server connection at", "Build returned no authoritative effective address") + assert.Contains(t, msg, "cannot read file") + assert.Contains(t, msg, "/definitely/does/not/exist") + assert.Empty(t, res.Stderr.String(), "the test harness overrides Fail and captures the error") +} + +func TestConnectDiagnosis_ProfileAddressUsesEffectiveAddressWithoutGuessingProvenance(t *testing.T) { + f, err := os.CreateTemp("", "") + require.NoError(t, err) + t.Cleanup(func() { os.Remove(f.Name()) }) + _, err = f.WriteString(` +[profile.default] +address = "does-not-exist.invalid:7233" +`) + require.NoError(t, err) + require.NoError(t, f.Close()) + + h := NewCommandHarness(t) + h.Options.EnvLookup = EnvLookupMap{"TEMPORAL_CONFIG_FILE": f.Name()} + res := h.Execute( + "workflow", "list", + "--client-connect-timeout", "5s", + ) + + require.Error(t, res.Err) + msg := res.Err.Error() + assert.Contains(t, msg, "failed connecting to Temporal server at does-not-exist.invalid:7233") + assert.NotContains(t, msg, "config profile") + assert.NotContains(t, msg, "temporal config get") +} + +func TestConnectDiagnosis_CommandTimeoutCauseSurvives(t *testing.T) { + ln := startBlackHoleListener(t) + + h := NewCommandHarness(t) + res := h.Execute( + "workflow", "list", + "--address", ln.Addr().String(), + "--command-timeout", "1s", + ) + + require.Error(t, res.Err) + assert.Contains(t, res.Err.Error(), "command timed out after 1s") } diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index eaf08dbdf..2dee3d5be 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -150,6 +150,9 @@ func (c *CommandContext) preprocessOptions() error { // to its own error handling logic, and just copy the exit code through. os.Exit(exitError.ExitCode()) } + if writeConnectionError(c.Options.Stderr, err, !color.NoColor) { + os.Exit(1) + } fmt.Fprintf(c.Options.Stderr, "Error: %v\n", err) os.Exit(1) } diff --git a/internal/temporalcli/connectdiag.go b/internal/temporalcli/connectdiag.go new file mode 100644 index 000000000..cd96c5354 --- /dev/null +++ b/internal/temporalcli/connectdiag.go @@ -0,0 +1,349 @@ +package temporalcli + +import ( + "context" + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "net" + "strings" + "syscall" + "time" + + "go.temporal.io/api/serviceerror" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// connectCause classifies why a connection to the server failed. It drives +// the suggested fix shown alongside the staged diagnosis. +type connectCause int + +const ( + causeUnknown connectCause = iota + causeDNS + causeTCPRefused + causeTCPTimeout + // causeServerPlaintext means TLS was configured but the server responded + // with a non-TLS handshake. + causeServerPlaintext + // causeServerSpeaksTLS means TLS was not configured but the server + // completed a TLS handshake when offered one. + causeServerSpeaksTLS + causeClientCertRequired + causeCAVerify + causeHostnameMismatch + causeCertFileUnreadable + causeUnauthenticated + causePermissionDenied + causeTimeout +) + +type diagStatus int + +const ( + diagOK diagStatus = iota + diagFail + diagInconclusive + diagSkipped +) + +type diagStage struct { + Status diagStatus + Label string +} + +// connectDiagnosis is the result of probing a failed connection. +type connectDiagnosis struct { + Address string + Stages []diagStage + Cause connectCause + // Detail carries cause-specific info (e.g. an unreadable file path or the + // raw TLS alert text) for use in stage labels and suggestions. + Detail string +} + +const ( + connectDiagnosisBudget = 3 * time.Second + connectDiagnosisReadProbe = 500 * time.Millisecond +) + +// diagnoseConnection probes address in stages (DNS, TCP, TLS) to pinpoint why +// a dial failed, and classifies origErr for anything past the transport. It +// must only be called on an already-failed dial: it makes fresh network +// connections (including, when TLS is not configured, one anonymous TLS +// handshake to detect a TLS-only server, which may appear in server logs). +func diagnoseConnection(ctx context.Context, address string, tlsCfg *tls.Config, origErr error) *connectDiagnosis { + d := &connectDiagnosis{Address: address, Cause: causeUnknown} + origCause, origDetail := classifyGRPCError(origErr) + switch origCause { + case causeUnauthenticated: + d.Cause, d.Detail = origCause, origDetail + d.fail("gRPC request was unauthenticated") + return d + case causePermissionDenied: + d.Cause, d.Detail = origCause, origDetail + d.fail("gRPC request was denied") + return d + } + ctx, cancel := context.WithTimeout(ctx, connectDiagnosisBudget) + defer cancel() + + host, _, err := net.SplitHostPort(address) + if err != nil { + d.inconclusive("Connection checks unavailable: address is not in host:port form") + d.Cause, d.Detail = classifyGRPCError(origErr) + return d + } + + // Stage: DNS (skipped for IP literals) + if net.ParseIP(host) == nil { + addrs, err := net.DefaultResolver.LookupHost(ctx, host) + if err != nil { + if d.interrupted(ctx, "DNS") { + return d + } + d.fail(fmt.Sprintf("DNS lookup for %q failed: %v", host, dnsErrShort(err))) + d.Cause = causeDNS + return d + } + if d.interrupted(ctx, "DNS") { + return d + } + plural := "es" + if len(addrs) == 1 { + plural = "" + } + d.ok(fmt.Sprintf("DNS resolved (%d address%s)", len(addrs), plural)) + } else { + d.skipped("DNS lookup skipped: address uses an IP literal") + } + + // Stage: TCP + conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", address) + if err != nil { + if d.interrupted(ctx, "TCP") { + return d + } + if isConnRefused(err) { + d.fail("TCP connection refused: nothing is listening at " + address) + d.Cause = causeTCPRefused + } else if isTimeout(err) { + d.fail("TCP connection timed out") + d.Cause = causeTCPTimeout + } else { + d.inconclusive(fmt.Sprintf("TCP check inconclusive: %v", err)) + d.Cause = causeUnknown + } + return d + } + defer conn.Close() + d.ok("TCP connection established") + + if tlsCfg != nil { + d.probeTLS(ctx, conn, host, tlsCfg) + if ctx.Err() != nil { + return d + } + if d.Cause != causeUnknown { + return d + } + } else if cause, detail := probeServerSpeaksTLS(ctx, conn, host); cause != causeUnknown { + d.fail("server expects TLS, but the CLI is connecting without it" + detail) + d.Cause = cause + return d + } else if d.interrupted(ctx, "TLS") { + return d + } + + // Stage: gRPC — no re-dial; classify the original error. + d.Cause, d.Detail = origCause, origDetail + switch d.Cause { + case causeUnauthenticated: + d.fail("gRPC request was unauthenticated") + case causePermissionDenied: + d.fail("gRPC request was denied") + case causeTimeout: + d.inconclusive("gRPC failure could not be diagnosed before its deadline") + default: + d.inconclusive("gRPC failure was not classified: " + shortErr(origErr)) + } + return d +} + +// probeTLS performs a TLS handshake over conn using the client's own config +// and classifies the failure, if any. On handshake success it briefly reads to +// catch post-handshake rejections: TLS 1.3 servers requiring client +// certificates complete the handshake first and only then send an alert. +func (d *connectDiagnosis) probeTLS(ctx context.Context, conn net.Conn, host string, tlsCfg *tls.Config) { + cfg := tlsCfg.Clone() + if cfg.ServerName == "" && !cfg.InsecureSkipVerify { + cfg.ServerName = host + } + tlsConn := tls.Client(conn, cfg) + if err := tlsConn.HandshakeContext(ctx); err != nil { + if d.interrupted(ctx, "TLS handshake") { + return + } + d.classifyTLSError(err) + return + } + // Handshake OK; a short read distinguishes an mTLS rejection alert from a + // genuinely healthy connection (where the read just times out). + stopCancel := armReadDeadline(ctx, tlsConn) + defer stopCancel() + buf := make([]byte, 1) + _, err := tlsConn.Read(buf) + if ctx.Err() != nil { + d.interrupted(ctx, "TLS post-handshake") + return + } + if err != nil && !isTimeout(err) { + if cause := classifyTLSAlert(err); cause != causeUnknown { + d.classifyTLSError(err) + return + } + } + d.ok("TLS handshake succeeded") +} + +func armReadDeadline(ctx context.Context, conn net.Conn) func() bool { + readDeadline := time.Now().Add(connectDiagnosisReadProbe) + if contextDeadline, ok := ctx.Deadline(); ok && contextDeadline.Before(readDeadline) { + readDeadline = contextDeadline + } + _ = conn.SetReadDeadline(readDeadline) + return context.AfterFunc(ctx, func() { _ = conn.SetReadDeadline(time.Now()) }) +} + +func (d *connectDiagnosis) classifyTLSError(err error) { + var recordErr tls.RecordHeaderError + var certErr *tls.CertificateVerificationError + var unknownAuthErr x509.UnknownAuthorityError + var hostnameErr x509.HostnameError + switch { + case errors.As(err, &recordErr): + d.fail("TLS handshake failed: server did not respond with TLS (it may be a plaintext gRPC endpoint)") + d.Cause = causeServerPlaintext + case errors.As(err, &hostnameErr): + d.fail("TLS handshake failed: server certificate is not valid for this host: " + shortErr(err)) + d.Cause = causeHostnameMismatch + case errors.As(err, &unknownAuthErr), errors.As(err, &certErr): + d.fail("TLS handshake failed: cannot verify server certificate: " + shortErr(err)) + d.Cause = causeCAVerify + case classifyTLSAlert(err) == causeClientCertRequired: + d.fail("TLS handshake failed: server requires mTLS, no valid client certificate was provided") + d.Cause = causeClientCertRequired + default: + d.fail("TLS handshake failed: " + shortErr(err)) + d.Cause = causeUnknown + d.Detail = err.Error() + } +} + +// classifyTLSAlert detects remote TLS alerts that explicitly indicate the +// server required or rejected a client certificate. Go does not export alert +// types for remote errors, so this matches the alert descriptions crypto/tls +// emits: alert 116 "certificate required" (TLS 1.3) and alert 42 "bad +// certificate". Generic alerts such as alert 40 "handshake failure" are not +// sufficient evidence of mTLS. Matching is scoped to TLS probe errors only; +// if these strings drift in a future Go release, unit tests pin them and the +// diagnosis degrades to showing the raw error without a suggestion. +func classifyTLSAlert(err error) connectCause { + msg := err.Error() + if strings.Contains(msg, "certificate required") || + strings.Contains(msg, "bad certificate") { + return causeClientCertRequired + } + return causeUnknown +} + +// probeServerSpeaksTLS opportunistically offers a TLS handshake to a server +// the CLI is configured to reach in plaintext. If the server negotiates TLS +// (or rejects us at the certificate step, which still means it spoke TLS), the +// mismatch is the likely root cause. +func probeServerSpeaksTLS(ctx context.Context, conn net.Conn, host string) (connectCause, string) { + tlsConn := tls.Client(conn, &tls.Config{InsecureSkipVerify: true, ServerName: host}) + err := tlsConn.HandshakeContext(ctx) + if err == nil { + return causeServerSpeaksTLS, "" + } + if classifyTLSAlert(err) == causeClientCertRequired { + return causeServerSpeaksTLS, " (and it requires client certificates)" + } + return causeUnknown, "" +} + +func classifyGRPCError(err error) (connectCause, string) { + var deadlineErr *serviceerror.DeadlineExceeded + if errors.As(err, &deadlineErr) || errors.Is(err, context.DeadlineExceeded) { + return causeTimeout, "" + } + if st, ok := status.FromError(err); ok { + switch st.Code() { + case codes.Unauthenticated: + return causeUnauthenticated, "" + case codes.PermissionDenied: + return causePermissionDenied, "" + case codes.DeadlineExceeded: + return causeTimeout, "" + } + } + return causeUnknown, "" +} + +func (d *connectDiagnosis) ok(label string) { d.Stages = append(d.Stages, diagStage{diagOK, label}) } +func (d *connectDiagnosis) fail(label string) { + d.Stages = append(d.Stages, diagStage{diagFail, label}) +} +func (d *connectDiagnosis) inconclusive(label string) { + d.Stages = append(d.Stages, diagStage{diagInconclusive, label}) +} +func (d *connectDiagnosis) skipped(label string) { + d.Stages = append(d.Stages, diagStage{diagSkipped, label}) +} + +func (d *connectDiagnosis) interrupted(ctx context.Context, stage string) bool { + if ctx.Err() == nil { + return false + } + d.Cause = causeUnknown + d.Detail = "" + if errors.Is(ctx.Err(), context.Canceled) { + d.skipped(stage + " check skipped: diagnosis canceled") + } else { + d.inconclusive(stage + " check inconclusive: diagnosis deadline reached") + } + return true +} + +// isConnRefused reports whether err is a refused TCP connection. errors.Is +// against syscall.ECONNREFUSED matches on unix, but Windows dials fail with +// WSAECONNREFUSED ("No connection could be made because the target machine +// actively refused it"), which Go does not map to syscall.ECONNREFUSED. +func isConnRefused(err error) bool { + return errors.Is(err, syscall.ECONNREFUSED) || + strings.Contains(err.Error(), "refused") +} + +func isTimeout(err error) bool { + var netErr net.Error + return errors.Is(err, context.DeadlineExceeded) || + (errors.As(err, &netErr) && netErr.Timeout()) +} + +// shortErr renders an error compactly for a stage line, trimming the SDK's +// "failed reaching server:" prefix that would be redundant inside a +// connection diagnosis. +func shortErr(err error) string { + return strings.TrimPrefix(err.Error(), "failed reaching server: ") +} + +func dnsErrShort(err error) string { + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) && dnsErr.IsNotFound { + return "no such host" + } + return err.Error() +} diff --git a/internal/temporalcli/connectdiag_test.go b/internal/temporalcli/connectdiag_test.go new file mode 100644 index 000000000..736f3e9e8 --- /dev/null +++ b/internal/temporalcli/connectdiag_test.go @@ -0,0 +1,534 @@ +package temporalcli + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "errors" + "fmt" + "io" + "math/big" + "net" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.temporal.io/api/serviceerror" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// newTestCert generates a self-signed server certificate for 127.0.0.1. +func newTestCert(t *testing.T) tls.Certificate { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "connectdiag-test"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}, + DNSNames: []string{"localhost"}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + } + der, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + require.NoError(t, err) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: mustParseCert(t, der)} +} + +func mustParseCert(t *testing.T, der []byte) *x509.Certificate { + t.Helper() + cert, err := x509.ParseCertificate(der) + require.NoError(t, err) + return cert +} + +func certPool(t *testing.T, cert tls.Certificate) *x509.CertPool { + t.Helper() + pool := x509.NewCertPool() + pool.AddCert(cert.Leaf) + return pool +} + +// serveTLS accepts connections and completes TLS handshakes until the +// listener closes. Returned address is host:port. +func serveTLS(t *testing.T, cfg *tls.Config) string { + t.Helper() + ln, err := tls.Listen("tcp", "127.0.0.1:0", cfg) + require.NoError(t, err) + t.Cleanup(func() { ln.Close() }) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func() { + // Drive the handshake; keep the connection open briefly so + // post-handshake alerts (TLS 1.3 client-cert enforcement) + // reach the client, then close. + if tlsConn, ok := conn.(*tls.Conn); ok { + _ = tlsConn.HandshakeContext(context.Background()) + buf := make([]byte, 1) + _ = tlsConn.SetReadDeadline(time.Now().Add(2 * time.Second)) + _, _ = tlsConn.Read(buf) + } + conn.Close() + }() + } + }() + return ln.Addr().String() +} + +func testDiagnose(t *testing.T, address string, tlsCfg *tls.Config) *connectDiagnosis { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + // Mirror the SDK's eager GetSystemInfo failure shape. + origErr := fmt.Errorf("failed reaching server: %w", serviceerror.NewDeadlineExceeded("context deadline exceeded")) + return diagnoseConnection(ctx, address, tlsCfg, origErr) +} + +func TestDiagnoseConnection_ClientCertRequired(t *testing.T) { + cert := newTestCert(t) + addr := serveTLS(t, &tls.Config{ + Certificates: []tls.Certificate{cert}, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: certPool(t, cert), + }) + + // Client trusts the server CA but has no client cert. + d := testDiagnose(t, addr, &tls.Config{RootCAs: certPool(t, cert)}) + // This test also pins the authoritative Go crypto/tls alert text + // ("certificate required" / "bad certificate") that classifyTLSAlert + // depends on; if it fails after a Go upgrade, revisit classifyTLSAlert. + assert.Equal(t, causeClientCertRequired, d.Cause) + requireStage(t, d, diagOK, "TCP connection established") + requireStage(t, d, diagFail, "server requires mTLS") +} + +func TestGenericTLSHandshakeFailureDoesNotImplyClientCertificate(t *testing.T) { + err := errors.New("remote error: tls: handshake failure") + assert.Equal(t, causeUnknown, classifyTLSAlert(err)) + + d := &connectDiagnosis{} + d.classifyTLSError(err) + assert.Equal(t, causeUnknown, d.Cause) + requireStage(t, d, diagFail, "remote error: tls: handshake failure") + assert.Nil(t, suggestAction(d, connectMeta{Address: "example:7233"})) + for _, stage := range d.Stages { + assert.NotContains(t, stage.Label, "mTLS") + assert.NotContains(t, stage.Label, "client certificate") + } + + client, server := net.Pipe() + t.Cleanup(func() { _ = client.Close() }) + go func() { + defer server.Close() + buf := make([]byte, 4096) + _, _ = server.Read(buf) + // TLS alert record: fatal handshake_failure (alert 40). + _, _ = server.Write([]byte{21, 3, 3, 0, 2, 2, 40}) + }() + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + cause, detail := probeServerSpeaksTLS(ctx, client, "example.com") + assert.Equal(t, causeUnknown, cause) + assert.Empty(t, detail) +} + +func TestDiagnoseConnection_ServerPlaintext(t *testing.T) { + // Plain TCP listener that immediately writes a non-TLS response. + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { ln.Close() }) + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + _, _ = conn.Write([]byte("HTTP/1.1 400 Bad Request\r\n\r\n")) + // Drain the client's ClientHello before closing: closing with + // unread data pending sends an RST, which on Windows discards + // the response before the client can read it. + _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + buf := make([]byte, 1024) + for { + if _, err := conn.Read(buf); err != nil { + return + } + } + }() + } + }() + + d := testDiagnose(t, ln.Addr().String(), &tls.Config{InsecureSkipVerify: true}) + assert.Equal(t, causeServerPlaintext, d.Cause) + requireStage(t, d, diagFail, "did not respond with TLS") +} + +func TestDiagnoseConnection_ServerSpeaksTLS(t *testing.T) { + cert := newTestCert(t) + addr := serveTLS(t, &tls.Config{Certificates: []tls.Certificate{cert}}) + + // No TLS configured on the client. + d := testDiagnose(t, addr, nil) + assert.Equal(t, causeServerSpeaksTLS, d.Cause) + requireStage(t, d, diagFail, "server expects TLS") +} + +func TestDiagnoseConnection_Refused(t *testing.T) { + // Grab a port and close it so nothing is listening. + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := ln.Addr().String() + require.NoError(t, ln.Close()) + + d := testDiagnose(t, addr, nil) + assert.Equal(t, causeTCPRefused, d.Cause) + requireStage(t, d, diagSkipped, "DNS lookup skipped") + requireStage(t, d, diagFail, "TCP connection refused") +} + +func TestDiagnoseConnection_DNSFailure(t *testing.T) { + // .invalid is reserved (RFC 2606) and can never resolve. + d := testDiagnose(t, "does-not-exist.invalid:7233", nil) + assert.Equal(t, causeDNS, d.Cause) + requireStage(t, d, diagFail, "DNS lookup") +} + +func TestDiagnoseConnection_UnknownCA(t *testing.T) { + cert := newTestCert(t) + addr := serveTLS(t, &tls.Config{Certificates: []tls.Certificate{cert}}) + + // Client does not trust the server's self-signed cert. + d := testDiagnose(t, addr, &tls.Config{}) + assert.Equal(t, causeCAVerify, d.Cause) + requireStage(t, d, diagFail, "cannot verify server certificate") +} + +func TestDiagnoseConnection_HealthyTLSFallsThroughToGRPC(t *testing.T) { + cert := newTestCert(t) + addr := serveTLS(t, &tls.Config{Certificates: []tls.Certificate{cert}}) + + // TLS is fine; the original (gRPC-level) error should be classified. + d := testDiagnose(t, addr, &tls.Config{RootCAs: certPool(t, cert)}) + assert.Equal(t, causeTimeout, d.Cause) + requireStage(t, d, diagOK, "TLS handshake succeeded") + requireStage(t, d, diagInconclusive, "gRPC failure could not be diagnosed") +} + +func TestDiagnoseConnection_CancellationDuringTLSIsSkippedAndPrompt(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + release := make(chan struct{}) + tlsStarted := make(chan error, 1) + serverDone := make(chan struct{}) + go func() { + defer close(serverDone) + conn, acceptErr := ln.Accept() + if acceptErr != nil { + tlsStarted <- acceptErr + return + } + defer conn.Close() + header := make([]byte, 5) + _, readErr := io.ReadFull(conn, header) + tlsStarted <- readErr + if readErr != nil { + return + } + <-release + }() + t.Cleanup(func() { + close(release) + _ = ln.Close() + select { + case <-serverDone: + case <-time.After(time.Second): + t.Error("TLS test server did not stop") + } + }) + + ctx, cancel := context.WithCancel(t.Context()) + t.Cleanup(cancel) + diagnosisDone := make(chan *connectDiagnosis, 1) + started := time.Now() + go func() { + diagnosisDone <- diagnoseConnection(ctx, ln.Addr().String(), &tls.Config{InsecureSkipVerify: true}, errors.New("dial failed")) + }() + + select { + case tlsErr := <-tlsStarted: + require.NoError(t, tlsErr) + case d := <-diagnosisDone: + t.Fatalf("diagnosis completed before TLS handshake began: %+v", d) + case <-time.After(time.Second): + t.Fatal("TLS handshake did not begin") + } + cancel() + + var d *connectDiagnosis + select { + case d = <-diagnosisDone: + case <-time.After(time.Second): + t.Fatal("diagnosis did not return after cancellation") + } + + assert.Less(t, time.Since(started), time.Second) + assert.Equal(t, causeUnknown, d.Cause) + requireStage(t, d, diagSkipped, "TLS handshake check skipped: diagnosis canceled") + for _, stage := range d.Stages { + assert.False(t, stage.Status == diagOK && strings.Contains(stage.Label, "TLS handshake succeeded")) + } +} + +func requireStage(t *testing.T, d *connectDiagnosis, status diagStatus, labelSubstr string) { + t.Helper() + for _, stage := range d.Stages { + if stage.Status == status && strings.Contains(stage.Label, labelSubstr) { + return + } + } + t.Fatalf("no stage with status %v containing %q in %+v", status, labelSubstr, d.Stages) +} + +func TestClassifyGRPCError(t *testing.T) { + tests := []struct { + name string + err error + cause connectCause + detail string + }{ + { + name: "serviceerror deadline exceeded", + err: fmt.Errorf("failed reaching server: %w", serviceerror.NewDeadlineExceeded("context deadline exceeded")), + cause: causeTimeout, + }, + { + name: "plain context deadline", + err: fmt.Errorf("failed reaching server: %w", context.DeadlineExceeded), + cause: causeTimeout, + }, + { + name: "unauthenticated status", + err: fmt.Errorf("failed reaching server: %w", status.Error(codes.Unauthenticated, "bad credentials")), + cause: causeUnauthenticated, + }, + { + name: "permission denied status", + err: fmt.Errorf("failed reaching server: %w", status.Error(codes.PermissionDenied, "nope")), + cause: causePermissionDenied, + }, + { + name: "unclassified error", + err: fmt.Errorf("something else entirely"), + cause: causeUnknown, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cause, detail := classifyGRPCError(tt.err) + assert.Equal(t, tt.cause, cause) + if tt.detail != "" { + assert.Contains(t, detail, tt.detail) + } + }) + } +} + +func TestDiagnoseConnection_AuthStatusesUseTypedOriginalErrorWithoutSpeculativeProbe(t *testing.T) { + for _, tt := range []struct { + name string + code codes.Code + cause connectCause + stage string + }{ + {name: "unauthenticated", code: codes.Unauthenticated, cause: causeUnauthenticated, stage: "unauthenticated"}, + {name: "permission denied", code: codes.PermissionDenied, cause: causePermissionDenied, stage: "denied"}, + } { + t.Run(tt.name, func(t *testing.T) { + d := diagnoseConnection(t.Context(), "does-not-exist.invalid:7233", nil, + fmt.Errorf("failed reaching server: %w", status.Error(tt.code, "server-controlled detail"))) + assert.Equal(t, tt.cause, d.Cause) + require.Len(t, d.Stages, 1, "typed auth failures must not trigger post-failure network probes") + requireStage(t, d, diagFail, tt.stage) + assert.Nil(t, suggestAction(d, connectMeta{Address: d.Address})) + }) + } +} + +func TestConnectSummary(t *testing.T) { + origErr := fmt.Errorf("failed reaching server: context deadline exceeded") + tests := []struct { + cause connectCause + want string + }{ + {causeDNS, "could not resolve host"}, + {causeTCPRefused, "connection refused"}, + {causeTCPTimeout, "connection timed out"}, + {causeServerPlaintext, "TLS is enabled but the server did not respond with TLS"}, + {causeServerSpeaksTLS, "the server requires TLS but the CLI is connecting without it"}, + {causeClientCertRequired, "server requires client certificate (mTLS)"}, + {causeCAVerify, "cannot verify server TLS certificate"}, + {causeHostnameMismatch, "server TLS certificate does not match host"}, + // Unclassified failures keep the original error text (minus the SDK + // prefix) so scripts matching on it keep working. + {causeUnknown, "context deadline exceeded"}, + {causeTimeout, "context deadline exceeded"}, + } + for _, tt := range tests { + got := connectSummary(&connectDiagnosis{Cause: tt.cause}, origErr) + assert.Contains(t, got, tt.want, "cause %v", tt.cause) + } +} + +func TestAuthSummariesDoNotCopyServerControlledStatusMessage(t *testing.T) { + assert.Equal(t, "authentication failed", connectSummary( + &connectDiagnosis{Cause: causeUnauthenticated, Detail: "attacker text\nrun this"}, + errors.New("fallback attacker text"), + )) + assert.Equal(t, "permission denied", connectSummary( + &connectDiagnosis{Cause: causePermissionDenied, Detail: "attacker text\nrun this"}, + errors.New("fallback attacker text"), + )) +} + +func TestArmReadDeadlineUsesEarlierContextDeadlineAndCancellation(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + conn := &deadlineRecordingConn{} + stop := armReadDeadline(ctx, conn) + defer stop() + require.Eventually(t, func() bool { return !conn.deadline().IsZero() }, time.Second, time.Millisecond) + assert.WithinDuration(t, time.Now().Add(50*time.Millisecond), conn.deadline(), 25*time.Millisecond) + cancel() + require.Eventually(t, func() bool { return time.Until(conn.deadline()) <= 0 }, time.Second, time.Millisecond) +} + +type deadlineRecordingConn struct { + mu sync.Mutex + d time.Time +} + +func (c *deadlineRecordingConn) Read([]byte) (int, error) { return 0, io.EOF } +func (c *deadlineRecordingConn) Write(p []byte) (int, error) { return len(p), nil } +func (c *deadlineRecordingConn) Close() error { return nil } +func (c *deadlineRecordingConn) LocalAddr() net.Addr { return nil } +func (c *deadlineRecordingConn) RemoteAddr() net.Addr { return nil } +func (c *deadlineRecordingConn) SetDeadline(time.Time) error { return nil } +func (c *deadlineRecordingConn) SetWriteDeadline(time.Time) error { return nil } +func (c *deadlineRecordingConn) SetReadDeadline(d time.Time) error { + c.mu.Lock() + c.d = d + c.mu.Unlock() + return nil +} +func (c *deadlineRecordingConn) deadline() time.Time { c.mu.Lock(); defer c.mu.Unlock(); return c.d } + +func TestSuggestFix(t *testing.T) { + tests := []struct { + name string + diag *connectDiagnosis + meta connectMeta + contains []string + }{ + { + name: "mTLS uses long flags", + diag: &connectDiagnosis{Cause: causeClientCertRequired}, + meta: connectMeta{Address: "myhost:7233"}, + contains: []string{"requires client certificates", "--tls-cert-path", "--tls-key-path"}, + }, + { + name: "refused on local default port", + diag: &connectDiagnosis{Cause: causeTCPRefused}, + meta: connectMeta{Address: "127.0.0.1:7233"}, + contains: []string{"temporal server start-dev"}, + }, + { + name: "refused elsewhere", + diag: &connectDiagnosis{Cause: causeTCPRefused}, + meta: connectMeta{Address: "myhost:9999"}, + contains: []string{"Nothing is listening at myhost:9999"}, + }, + { + name: "dns failure does not guess provenance", + diag: &connectDiagnosis{Cause: causeDNS}, + meta: connectMeta{Address: "typo.example.com:7233"}, + contains: []string{`Could not resolve "typo.example.com"`}, + }, + { + name: "server speaks TLS", + diag: &connectDiagnosis{Cause: causeServerSpeaksTLS}, + meta: connectMeta{Address: "myhost:7233"}, + contains: []string{"requires TLS", "--tls"}, + }, + { + name: "server plaintext", + diag: &connectDiagnosis{Cause: causeServerPlaintext}, + meta: connectMeta{Address: "myhost:7233", TLSConfigured: true}, + contains: []string{"does not appear to use TLS"}, + }, + { + name: "cert file unreadable", + diag: &connectDiagnosis{Cause: causeCertFileUnreadable, Detail: "/nope.pem"}, + meta: connectMeta{Address: "myhost:7233"}, + contains: []string{`Cannot read "/nope.pem"`}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := string(renderConnectionReport(connectionReport{Summary: "failure", Action: suggestAction(tt.diag, tt.meta)}, false, displayShellPOSIX)) + for _, want := range tt.contains { + assert.Contains(t, got, want) + } + }) + } + assert.Nil(t, suggestAction(&connectDiagnosis{Cause: causeUnauthenticated}, connectMeta{Address: "example:7233"})) + assert.Nil(t, suggestAction(&connectDiagnosis{Cause: causePermissionDenied}, connectMeta{Address: "example:7233"})) + assert.Nil(t, suggestAction(&connectDiagnosis{Cause: causeTimeout}, connectMeta{Address: "example:7233"})) + assert.Nil(t, suggestAction(&connectDiagnosis{Cause: causeUnknown}, connectMeta{Address: "example:7233"})) +} + +func TestConnectErrorRendering(t *testing.T) { + origErr := fmt.Errorf("failed reaching server: context deadline exceeded") + d := &connectDiagnosis{ + Address: "foo.bar.tmprl.cloud:7233", + Cause: causeClientCertRequired, + Stages: []diagStage{ + {diagOK, "DNS resolved (3 addresses)"}, + {diagOK, "TCP connection established"}, + {diagFail, "TLS handshake failed: server requires mTLS, no valid client certificate was provided"}, + }, + } + err := newConnectError(d, connectMeta{ + Address: "foo.bar.tmprl.cloud:7233", + Namespace: "example.namespace", + TLSConfigured: true, + }, origErr) + + msg := string(renderConnectionReport(connectionErrorReport(err), false, displayShellPOSIX)) + assert.Contains(t, msg, "failed connecting to Temporal server at foo.bar.tmprl.cloud:7233: TLS handshake failed: server requires client certificate (mTLS)") + assert.Contains(t, msg, "Namespace: example.namespace") + assert.Contains(t, msg, "TLS: true") + assert.Contains(t, msg, "Connection checks for foo.bar.tmprl.cloud:7233") + assert.Contains(t, msg, "✓ DNS resolved (3 addresses)") + assert.Contains(t, msg, "✗ TLS handshake failed") + assert.Contains(t, msg, "--tls-cert-path") + // Unwrap preserves the original error. + assert.ErrorContains(t, err.Unwrap(), "context deadline exceeded") +} diff --git a/internal/temporalcli/connecterror.go b/internal/temporalcli/connecterror.go new file mode 100644 index 000000000..15d4b7da9 --- /dev/null +++ b/internal/temporalcli/connecterror.go @@ -0,0 +1,118 @@ +package temporalcli + +import ( + "fmt" + "net" +) + +// connectError is returned by dialClient when connecting to the server fails. +// It retains semantic observations for terminal normalization; Error stays a +// concise, uncolored compatibility string and Unwrap preserves the cause. +type connectError struct { + diagnosis connectDiagnosis + meta connectMeta + cause error +} + +func (e *connectError) Error() string { + if e.meta.Address == "" { + return "failed preparing Temporal server connection: " + connectSummary(&e.diagnosis, e.cause) + } + return fmt.Sprintf("failed connecting to Temporal server at %s: %s", e.meta.Address, connectSummary(&e.diagnosis, e.cause)) +} + +func (e *connectError) Unwrap() error { return e.cause } + +// connectMeta contains the effective non-secret connection settings returned +// by the same ClientOptionsBuilder.Build call used for the failed dial. +type connectMeta struct { + Address string + Namespace string + TLSConfigured bool +} + +func newConnectError(d *connectDiagnosis, meta connectMeta, origErr error) *connectError { + if meta.Address == "" { + meta.Address = d.Address + } + diagnosis := *d + diagnosis.Stages = append([]diagStage(nil), d.Stages...) + return &connectError{diagnosis: diagnosis, meta: meta, cause: origErr} +} + +// connectSummary is the one-line cause appended to the first error line. For +// unclassified failures and timeouts it keeps the original error text so +// scripts matching on strings like "context deadline exceeded" keep working. +func connectSummary(d *connectDiagnosis, origErr error) string { + switch d.Cause { + case causeDNS: + return "could not resolve host" + case causeTCPRefused: + return "connection refused" + case causeTCPTimeout: + return "connection timed out" + case causeServerPlaintext: + return "TLS is enabled but the server did not respond with TLS; check tls configuration" + case causeServerSpeaksTLS: + return "the server requires TLS but the CLI is connecting without it" + case causeClientCertRequired: + return "TLS handshake failed: server requires client certificate (mTLS)" + case causeCAVerify: + return "cannot verify server TLS certificate" + case causeHostnameMismatch: + return "server TLS certificate does not match host" + case causeCertFileUnreadable: + return fmt.Sprintf("cannot read file %q", d.Detail) + case causeUnauthenticated: + return "authentication failed" + case causePermissionDenied: + return "permission denied" + default: + return shortErr(origErr) + } +} + +// suggestAction maps only directly observed failures to a typed next step. +// It never replays argv or guesses credential kind or configuration provenance. +func suggestAction(d *connectDiagnosis, meta connectMeta) *displayAction { + host, port, _ := net.SplitHostPort(meta.Address) + switch d.Cause { + case causeCertFileUnreadable: + return &displayAction{Label: fmt.Sprintf("Cannot read %q — check that the path exists and is readable.", d.Detail)} + case causeClientCertRequired: + return &displayAction{Label: "The server requires client certificates (mTLS). Configure both --tls-cert-path and --tls-key-path."} + case causeUnauthenticated, causePermissionDenied: + // client.Options keeps credentials opaque, so no credential-specific + // action is authoritative here. + return nil + case causeServerPlaintext: + return &displayAction{Label: fmt.Sprintf("The server at %s does not appear to use TLS. Remove --tls and related TLS flags, or check the address.", meta.Address)} + case causeServerSpeaksTLS: + return &displayAction{Label: "The server requires TLS. Retry with --tls."} + case causeDNS: + return &displayAction{Label: fmt.Sprintf("Could not resolve %q — check the server address.", host)} + case causeTCPRefused: + if isLoopbackHost(host) && port == "7233" { + return &displayAction{ + Label: fmt.Sprintf("No Temporal server is running at %s. Start a local dev server:", meta.Address), + Invocations: []displayInvocation{{Command: []string{"temporal", "server", "start-dev"}}}, + } + } + return &displayAction{Label: fmt.Sprintf("Nothing is listening at %s — verify the address and that the server is running.", meta.Address)} + case causeCAVerify: + return &displayAction{Label: "The server certificate is not trusted. Configure its CA certificate with --tls-ca-path."} + case causeHostnameMismatch: + return &displayAction{Label: fmt.Sprintf("The server certificate is not valid for %q. Set --tls-server-name to a certificate name.", host)} + case causeTCPTimeout: + return &displayAction{Label: fmt.Sprintf("The TCP check timed out. Verify the address and network path to %s.", meta.Address)} + } + return nil +} + +func isLoopbackHost(host string) bool { + if host == "localhost" { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} diff --git a/internal/temporalcli/terminal.go b/internal/temporalcli/terminal.go new file mode 100644 index 000000000..27cabb035 --- /dev/null +++ b/internal/temporalcli/terminal.go @@ -0,0 +1,227 @@ +package temporalcli + +import ( + "errors" + "fmt" + "io" + "runtime" + "strings" + "unicode" +) + +type checkOutcome int + +const ( + checkSucceeded checkOutcome = iota + checkFailed + checkInconclusive + checkSkipped +) + +type safeField struct { + Label string + Value string +} + +type errorCheck struct { + Outcome checkOutcome + Message string +} + +type displayInvocation struct { + Command []string + Args []string +} + +type displayAction struct { + Label string + Invocations []displayInvocation +} + +type connectionReport struct { + Summary string + Context []safeField + CheckHeading string + Checks []errorCheck + Action *displayAction +} + +type displayShell int + +const ( + displayShellPOSIX displayShell = iota + displayShellPowerShell +) + +// writeConnectionError renders err only when it contains a connectError. It +// returns whether the error was handled, leaving all generic failures on the +// CLI's longstanding fallback path. +func writeConnectionError(stderr io.Writer, err error, useColor bool) bool { + var connectionErr *connectError + if !errors.As(err, &connectionErr) { + return false + } + + shell := displayShellPOSIX + if runtime.GOOS == "windows" { + shell = displayShellPowerShell + } + _, _ = stderr.Write(renderConnectionReport(connectionErrorReport(connectionErr), useColor, shell)) + return true +} + +func connectionErrorReport(err *connectError) connectionReport { + report := connectionReport{ + Summary: err.Error(), + Action: suggestAction(&err.diagnosis, err.meta), + } + if err.meta.Namespace != "" { + report.Context = append(report.Context, safeField{Label: "Namespace", Value: err.meta.Namespace}) + } + if err.meta.Address != "" { + report.Context = append(report.Context, safeField{Label: "TLS", Value: fmt.Sprintf("%t", err.meta.TLSConfigured)}) + } + if len(err.diagnosis.Stages) > 0 { + report.CheckHeading = "Connection checks" + if err.meta.Address != "" { + report.CheckHeading += " for " + err.meta.Address + } + } + for _, stage := range err.diagnosis.Stages { + outcome := checkInconclusive + switch stage.Status { + case diagOK: + outcome = checkSucceeded + case diagFail: + outcome = checkFailed + case diagSkipped: + outcome = checkSkipped + } + report.Checks = append(report.Checks, errorCheck{Outcome: outcome, Message: stage.Label}) + } + return report +} + +func renderConnectionReport(report connectionReport, useColor bool, shell displayShell) []byte { + var b strings.Builder + b.WriteString("Error: ") + b.WriteString(escapeTerminalControls(report.Summary)) + b.WriteByte('\n') + for _, field := range report.Context { + fmt.Fprintf( + &b, + " %s: %s\n", + escapeTerminalControls(field.Label), + escapeTerminalControls(field.Value), + ) + } + if len(report.Checks) > 0 { + heading := report.CheckHeading + if heading == "" { + heading = "Connecting" + } + fmt.Fprintf(&b, "\n %s\n", escapeTerminalControls(heading)) + for _, check := range report.Checks { + symbol := "✓" + colorCode := "32" + switch check.Outcome { + case checkFailed: + symbol = "✗" + colorCode = "31" + case checkInconclusive: + symbol = "?" + colorCode = "33" + case checkSkipped: + symbol = "-" + colorCode = "33" + } + if useColor { + symbol = "\x1b[" + colorCode + "m" + symbol + "\x1b[0m" + } + fmt.Fprintf(&b, " %s %s\n", symbol, escapeTerminalControls(check.Message)) + } + } + if report.Action != nil { + b.WriteByte('\n') + if report.Action.Label != "" { + b.WriteString(indentLines(escapeTerminalControls(report.Action.Label), " ")) + b.WriteByte('\n') + } + for _, invocation := range report.Action.Invocations { + rendered, ok := renderInvocation(invocation, shell) + if !ok { + continue + } + b.WriteByte('\n') + b.WriteString(" ") + b.WriteString(rendered) + b.WriteByte('\n') + } + } + return []byte(b.String()) +} + +func renderInvocation(invocation displayInvocation, shell displayShell) (string, bool) { + parts := append([]string(nil), invocation.Command...) + parts = append(parts, invocation.Args...) + if len(parts) == 0 { + return "", false + } + for i := range parts { + parts[i] = escapeTerminalControls(parts[i]) + if shell == displayShellPowerShell { + parts[i] = quotePowerShell(parts[i]) + } else { + parts[i] = quotePOSIX(parts[i]) + } + } + rendered := strings.Join(parts, " ") + if shell == displayShellPowerShell { + rendered = "& " + rendered + } + return rendered, true +} + +func quotePOSIX(value string) string { + if value != "" && strings.IndexFunc(value, func(r rune) bool { + return !(unicode.IsLetter(r) || unicode.IsDigit(r) || strings.ContainsRune("_@%+=:,./-", r)) + }) < 0 { + return value + } + return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" +} + +func quotePowerShell(value string) string { + return "'" + strings.ReplaceAll(value, "'", "''") + "'" +} + +func escapeTerminalControls(value string) string { + var b strings.Builder + for _, r := range value { + if !unicode.IsControl(r) { + b.WriteRune(r) + continue + } + switch r { + case '\n': + b.WriteString(`\n`) + case '\r': + b.WriteString(`\r`) + case '\t': + b.WriteString(`\t`) + default: + fmt.Fprintf(&b, `\u{%x}`, r) + } + } + return b.String() +} + +func indentLines(s, indent string) string { + lines := strings.Split(s, "\n") + for i, line := range lines { + if line != "" { + lines[i] = indent + line + } + } + return strings.Join(lines, "\n") +} diff --git a/internal/temporalcli/terminal_test.go b/internal/temporalcli/terminal_test.go new file mode 100644 index 000000000..2a9327af9 --- /dev/null +++ b/internal/temporalcli/terminal_test.go @@ -0,0 +1,125 @@ +package temporalcli + +import ( + "bytes" + "errors" + "fmt" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWriteConnectionErrorHandlesWrappedConnectError(t *testing.T) { + cause := errors.New("dial failed") + err := fmt.Errorf("client setup: %w", newConnectError(&connectDiagnosis{ + Address: "127.0.0.1:7233", + Cause: causeTCPRefused, + Stages: []diagStage{{Status: diagFail, Label: "TCP connection refused"}}, + }, connectMeta{Address: "127.0.0.1:7233", Namespace: "default"}, cause)) + + stderr := &countingWriter{} + assert.True(t, writeConnectionError(stderr, err, false)) + assert.Equal(t, 1, stderr.writes) + assert.Contains(t, stderr.String(), "Error: failed connecting to Temporal server at 127.0.0.1:7233: connection refused") + assert.Contains(t, stderr.String(), "Namespace: default") + assert.Contains(t, stderr.String(), "✗ TCP connection refused") + expectedCommand := "temporal server start-dev" + if runtime.GOOS == "windows" { + expectedCommand = "& 'temporal' 'server' 'start-dev'" + } + assert.Contains(t, stderr.String(), expectedCommand) + assert.NotContains(t, stderr.String(), "\x1b[") +} + +func TestWriteConnectionErrorLeavesGenericErrorsUnhandled(t *testing.T) { + var stderr bytes.Buffer + assert.False(t, writeConnectionError(&stderr, errors.New("ordinary failure"), true)) + assert.Empty(t, stderr.String()) +} + +func TestRenderConnectionReportUsesExplicitColorPolicy(t *testing.T) { + report := connectionReport{ + Summary: "connection refused", + Checks: []errorCheck{{ + Outcome: checkFailed, + Message: "TCP connection refused", + }}, + } + + plain := string(renderConnectionReport(report, false, displayShellPOSIX)) + colored := string(renderConnectionReport(report, true, displayShellPOSIX)) + + assert.Equal(t, "Error: connection refused\n\n Connecting\n ✗ TCP connection refused\n", plain) + assert.NotContains(t, plain, "\x1b[") + assert.Contains(t, colored, "\x1b[31m✗\x1b[0m") +} + +func TestConnectionReportEscapesTerminalControls(t *testing.T) { + report := connectionReport{ + Summary: "bad\x1b[31m", + Context: []safeField{{Label: "Target\nName", Value: "host\tname"}}, + CheckHeading: "Check\rHeading", + Checks: []errorCheck{{Outcome: checkFailed, Message: "failed\ncheck"}}, + Action: &displayAction{Label: "retry\tnow"}, + } + + rendered := string(renderConnectionReport(report, false, displayShellPOSIX)) + assert.Contains(t, rendered, `bad\u{1b}[31m`) + assert.Contains(t, rendered, `Target\nName: host\tname`) + assert.Contains(t, rendered, `Check\rHeading`) + assert.Contains(t, rendered, `failed\ncheck`) + assert.Contains(t, rendered, `retry\tnow`) +} + +func TestRenderInvocationUsesExplicitShellQuoting(t *testing.T) { + invocation := displayInvocation{ + Command: []string{"temporal", "config", "set"}, + Args: []string{"--value", "space and 'quote'", "--profile", "-prod"}, + } + posix, ok := renderInvocation(invocation, displayShellPOSIX) + require.True(t, ok) + assert.Contains(t, posix, `'space and '"'"'quote'"'"''`) + assert.Contains(t, posix, "--profile -prod") + + powerShell, ok := renderInvocation(invocation, displayShellPowerShell) + require.True(t, ok) + assert.True(t, strings.HasPrefix(powerShell, "& 'temporal' ")) + assert.Contains(t, powerShell, `'space and ''quote'''`) + assert.Contains(t, powerShell, `'--profile' '-prod'`) + + escaped, ok := renderInvocation( + displayInvocation{Command: []string{"temporal"}, Args: []string{"unsafe\nvalue"}}, + displayShellPOSIX, + ) + require.True(t, ok) + assert.Equal(t, `temporal 'unsafe\nvalue'`, escaped) +} + +func TestConnectErrorCopiesDiagnosisBeforeRendering(t *testing.T) { + cause := errors.New("dial failed") + diagnosis := &connectDiagnosis{ + Address: "127.0.0.1:7233", + Cause: causeTCPRefused, + Stages: []diagStage{{Status: diagFail, Label: "TCP connection refused"}}, + } + err := newConnectError(diagnosis, connectMeta{Address: diagnosis.Address}, cause) + diagnosis.Stages[0].Label = "mutated after construction" + + report := connectionErrorReport(err) + require.Len(t, report.Checks, 1) + assert.Equal(t, "TCP connection refused", report.Checks[0].Message) + assert.ErrorIs(t, err, cause) +} + +type countingWriter struct { + bytes.Buffer + writes int +} + +func (w *countingWriter) Write(p []byte) (int, error) { + w.writes++ + return w.Buffer.Write(p) +}