diff --git a/cli/configssh_test.go b/cli/configssh_test.go index 7589dff5c49a4..140cee2dbece7 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -4,10 +4,11 @@ import ( "os" "testing" + "github.com/stretchr/testify/require" + "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/pty/ptytest" - "github.com/stretchr/testify/require" ) func TestConfigSSH(t *testing.T) { diff --git a/cli/ssh.go b/cli/ssh.go index d04986b8e6605..db8395454df2c 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -13,11 +13,12 @@ import ( gossh "golang.org/x/crypto/ssh" "golang.org/x/xerrors" + "golang.org/x/crypto/ssh/terminal" + "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" - "golang.org/x/crypto/ssh/terminal" ) func ssh() *cobra.Command { diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index 52fabee3694da..808530d614e1a 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -115,3 +115,21 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool { } return true } + +const websocketCloseMaxLen = 123 + +// WebsocketCloseSprintf formats a websocket close message and ensures it is +// truncated to the maximum allowed length. +func WebsocketCloseSprintf(format string, vars ...any) string { + msg := fmt.Sprintf(format, vars...) + + // Cap msg length at 123 bytes. nhooyr/websocket only allows close messages + // of this length. + if len(msg) > websocketCloseMaxLen { + // Trim the string to 123 bytes. If we accidentally cut in the middle of + // a UTF-8 character, remove it from the string. + return strings.ToValidUTF8(string(msg[123]), "") + } + + return msg +} diff --git a/coderd/httpapi/httpapi_test.go b/coderd/httpapi/httpapi_test.go index 87ce03f51d36c..0e7e45881a2bd 100644 --- a/coderd/httpapi/httpapi_test.go +++ b/coderd/httpapi/httpapi_test.go @@ -5,8 +5,10 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/httpapi" @@ -142,3 +144,23 @@ func TestReadUsername(t *testing.T) { }) } } + +func WebsocketCloseMsg(t *testing.T) { + t.Parallel() + + t.Run("TruncateSingleByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("d", 255) + trunc := httpapi.WebsocketCloseSprintf(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) + + t.Run("TruncateMultiByteCharacters", func(t *testing.T) { + t.Parallel() + + msg := strings.Repeat("こんにちは", 10) + trunc := httpapi.WebsocketCloseSprintf(msg) + assert.LessOrEqual(t, len(trunc), 123) + }) +} diff --git a/coderd/provisionerdaemons.go b/coderd/provisionerdaemons.go index 89f4cce105724..28f4bb8bb9cf4 100644 --- a/coderd/provisionerdaemons.go +++ b/coderd/provisionerdaemons.go @@ -56,7 +56,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Provisioners: []database.ProvisionerType{database.ProvisionerTypeEcho, database.ProvisionerTypeTerraform}, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("insert provisioner daemon:% s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("insert provisioner daemon: %s", err)) return } @@ -67,7 +67,7 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request config.LogOutput = io.Discard session, err := yamux.Server(websocket.NetConn(r.Context(), conn, websocket.MessageBinary), config) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("multiplex server: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("multiplex server: %s", err)) return } mux := drpcmux.New() @@ -80,13 +80,13 @@ func (api *api) provisionerDaemonsListen(rw http.ResponseWriter, r *http.Request Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)), }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("drpc register provisioner daemon: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("drpc register provisioner daemon: %s", err)) return } server := drpcserver.New(mux) err = server.Serve(r.Context(), session) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("serve: %s", err)) return } _ = conn.Close(websocket.StatusGoingAway, "") diff --git a/coderd/workspaceresources.go b/coderd/workspaceresources.go index 7342b01ea2d4b..532fc64ea5c51 100644 --- a/coderd/workspaceresources.go +++ b/coderd/workspaceresources.go @@ -108,7 +108,7 @@ func (api *api) workspaceResourceDial(rw http.ResponseWriter, r *http.Request) { Pubsub: api.Pubsub, }) if err != nil { - _ = conn.Close(websocket.StatusInternalError, fmt.Sprintf("serve: %s", err)) + _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("serve: %s", err)) return } } diff --git a/provisioner/terraform/serve.go b/provisioner/terraform/serve.go index 804dbd64131e1..a5fc22c094050 100644 --- a/provisioner/terraform/serve.go +++ b/provisioner/terraform/serve.go @@ -4,16 +4,14 @@ import ( "context" "path/filepath" + "github.com/cli/safeexec" "github.com/hashicorp/go-version" + "github.com/hashicorp/hc-install/product" + "github.com/hashicorp/hc-install/releases" "golang.org/x/xerrors" "cdr.dev/slog" - - "github.com/cli/safeexec" "github.com/coder/coder/provisionersdk" - - "github.com/hashicorp/hc-install/product" - "github.com/hashicorp/hc-install/releases" ) var (