From f960186502d6848965fec5c3dfa69f14b96b4265 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 18 Jun 2026 18:38:15 +0800 Subject: [PATCH 1/2] fix(api): sanitize status error bodies --- cmd/crabbox-ssh-gateway/main_test.go | 5 ++- cmd/crabfleet/main_test.go | 2 +- internal/fleetapi/client.go | 66 +++++++++++++++++++++++++++- internal/fleetapi/client_test.go | 42 ++++++++++++++++++ 4 files changed, 111 insertions(+), 4 deletions(-) diff --git a/cmd/crabbox-ssh-gateway/main_test.go b/cmd/crabbox-ssh-gateway/main_test.go index e561806c..7935ee3c 100644 --- a/cmd/crabbox-ssh-gateway/main_test.go +++ b/cmd/crabbox-ssh-gateway/main_test.go @@ -708,7 +708,10 @@ func TestRunCommandSanitizesControlPlaneErrors(t *testing.T) { if strings.ContainsAny(got, "\x1b\x07") { t.Fatalf("error output retained terminal controls: %q", got) } - if !strings.Contains(got, "bad]52;c;secretstate") { + if strings.Contains(got, "secret") || strings.Contains(got, "]52") { + t.Fatalf("error output retained terminal payload: %q", got) + } + if !strings.Contains(got, "badstate") { t.Fatalf("error output = %q", got) } } diff --git a/cmd/crabfleet/main_test.go b/cmd/crabfleet/main_test.go index 06692f57..9486a4fe 100644 --- a/cmd/crabfleet/main_test.go +++ b/cmd/crabfleet/main_test.go @@ -287,7 +287,7 @@ func TestDoctorSanitizesControlPlaneErrors(t *testing.T) { if strings.ContainsAny(output, "\x1b\x07") { t.Fatalf("doctor output contains terminal controls: %q", output) } - if !strings.Contains(output, "auth: failed: crabfleet API 500 Internal Server Error: bad]52;c;secretstate") { + if !strings.Contains(output, "auth: failed: crabfleet API 500 Internal Server Error: badstate") { t.Fatalf("doctor output = %q", output) } } diff --git a/internal/fleetapi/client.go b/internal/fleetapi/client.go index baa1f4a0..5e53ed5a 100644 --- a/internal/fleetapi/client.go +++ b/internal/fleetapi/client.go @@ -232,7 +232,7 @@ func (c *Client) terminal(ctx context.Context, id string, cols uint32, rows uint return nil, &StatusError{ StatusCode: statusErr.StatusCode, Status: statusErr.Status, - Body: statusErr.Body, + Body: sanitizeErrorBody(statusErr.Body), } } return nil, err @@ -302,12 +302,74 @@ func responseError(resp *http.Response) error { return &StatusError{ StatusCode: resp.StatusCode, Status: resp.Status, - Body: strings.TrimSpace(string(data)), + Body: sanitizeErrorBody(string(data)), } } return nil } +func sanitizeErrorBody(value string) string { + var out strings.Builder + const ( + stateText = iota + stateEscape + stateCSI + stateStringControl + stateStringControlEscape + ) + state := stateText + for _, r := range value { + switch state { + case stateText: + switch { + case r == '\x1b': + state = stateEscape + case r == '\x9b': + state = stateCSI + case r == '\x90' || r == '\x9d' || r == '\x9e' || r == '\x9f': + state = stateStringControl + case r == '\n' || r == '\r' || r == '\t': + out.WriteRune(' ') + case isErrorControl(r): + continue + default: + out.WriteRune(r) + } + case stateEscape: + switch r { + case '[': + state = stateCSI + case ']', 'P', '^', '_': + state = stateStringControl + default: + state = stateText + } + case stateCSI: + if r >= 0x40 && r <= 0x7e { + state = stateText + } + case stateStringControl: + switch r { + case '\x07', '\x9c': + state = stateText + case '\x1b': + state = stateStringControlEscape + } + case stateStringControlEscape: + if r == '\\' { + state = stateText + } else if r != '\x1b' { + state = stateStringControl + } + } + } + return strings.TrimSpace(out.String()) +} + +func isErrorControl(r rune) bool { + return r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) +} + func readBoundedResponse(body io.Reader) ([]byte, error) { data, err := io.ReadAll(io.LimitReader(body, maxResponseBytes+1)) if len(data) > maxResponseBytes { diff --git a/internal/fleetapi/client_test.go b/internal/fleetapi/client_test.go index 1610a0da..7bb54775 100644 --- a/internal/fleetapi/client_test.go +++ b/internal/fleetapi/client_test.go @@ -66,6 +66,48 @@ func TestClientRejectsIncompleteAuthentication(t *testing.T) { } } +func TestClientSanitizesStatusErrorBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("bad\x1b]52;c;secret\x07state\x1b[31m!\nnext")) + })) + defer server.Close() + + client := NewClient(server.URL, server.Client(), SSHAuth("gateway-token", "SHA256:test")) + _, err := client.State(context.Background()) + if err == nil { + t.Fatal("expected status error") + } + message := err.Error() + if strings.ContainsAny(message, "\x1b\x07") || strings.Contains(message, "secret") || strings.Contains(message, "]52") { + t.Fatalf("error retained terminal controls: %q", message) + } + if !strings.Contains(message, "crabfleet API 500 Internal Server Error: badstate! next") { + t.Fatalf("error = %q", message) + } +} + +func TestClientSanitizesTerminalHandshakeStatusErrorBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("terminal\x1b]52;c;secret\x07failed\x1b[31m")) + })) + defer server.Close() + + client := NewClient(server.URL, server.Client(), SSHAuth("gateway-token", "SHA256:test")) + err := client.Message(context.Background(), "IS-7", "hello", true, 80, 24) + if err == nil { + t.Fatal("expected terminal status error") + } + message := err.Error() + if strings.ContainsAny(message, "\x1b\x07") || strings.Contains(message, "secret") || strings.Contains(message, "]52") { + t.Fatalf("error retained terminal controls: %q", message) + } + if !strings.Contains(message, "crabfleet API 500 Internal Server Error: terminalfailed") { + t.Fatalf("error = %q", message) + } +} + func TestClientRejectsOversizedJSONResponses(t *testing.T) { largeLogin := strings.Repeat("a", maxResponseBytes+1) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { From 6936a9495ae8c7cd93dd8c0068361a8bc5e9c1f3 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 18 Jun 2026 18:40:50 +0800 Subject: [PATCH 2/2] fix(gateway): sanitize attach errors --- cmd/crabbox-ssh-gateway/main.go | 4 +- cmd/crabbox-ssh-gateway/main_test.go | 95 ++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/cmd/crabbox-ssh-gateway/main.go b/cmd/crabbox-ssh-gateway/main.go index faf37be9..142a305b 100644 --- a/cmd/crabbox-ssh-gateway/main.go +++ b/cmd/crabbox-ssh-gateway/main.go @@ -706,7 +706,7 @@ func safeError(err error) string { if err == nil { return "" } - return fleettext.Safe(err.Error()) + return fleettext.Safe(fleettext.SafeMultiline(err.Error())) } func printList(out io.Writer, state fleetapi.State) { @@ -930,7 +930,7 @@ func attach( ) uint32 { err := api.Attach(ctx, id, terminal, pty.cols, pty.rows, pty.resizes) if err != nil && !errors.Is(err, net.ErrClosed) && !strings.Contains(err.Error(), "closed") { - fmt.Fprintf(terminal, "\nattach closed: %v\n", err) + fmt.Fprintf(terminal, "\nattach closed: %s\n", safeError(err)) return 1 } return 0 diff --git a/cmd/crabbox-ssh-gateway/main_test.go b/cmd/crabbox-ssh-gateway/main_test.go index 7935ee3c..52af35b6 100644 --- a/cmd/crabbox-ssh-gateway/main_test.go +++ b/cmd/crabbox-ssh-gateway/main_test.go @@ -5,15 +5,19 @@ import ( "context" "crypto/ed25519" "crypto/rand" + "encoding/binary" "encoding/json" + "io" "net" "net/http" "net/http/httptest" "reflect" "strings" + "sync" "testing" "time" + "github.com/coder/websocket" "github.com/openclaw/crabfleet/internal/fleetapi" "golang.org/x/crypto/ssh" ) @@ -716,6 +720,51 @@ func TestRunCommandSanitizesControlPlaneErrors(t *testing.T) { } } +func TestAttachSanitizesTerminalErrors(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/terminal/ws" { + t.Errorf("path = %q", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + return + } + conn, err := websocket.Accept(w, r, nil) + if err != nil { + t.Error(err) + return + } + defer conn.Close(websocket.StatusNormalClosure, "") + + for range 2 { + if _, _, err := conn.Read(r.Context()); err != nil { + t.Error(err) + return + } + } + subscribed, _ := json.Marshal(map[string]any{"type": "subscribed", "canInput": true}) + if err := conn.Write(r.Context(), websocket.MessageBinary, testTerminalFrame(22, "IS-7", subscribed)); err != nil { + t.Error(err) + return + } + failure, _ := json.Marshal(map[string]string{"error": "bad\x1b]52;c;secret\x07state\x1b[31m"}) + _ = conn.Write(r.Context(), websocket.MessageBinary, testTerminalFrame(23, "IS-7", failure)) + })) + defer server.Close() + + client := fleetapi.NewClient(server.URL, server.Client(), fleetapi.SSHAuth("gateway-token", "SHA256:test")) + terminal := newBlockingTestTerminal() + exit := attach(context.Background(), terminal, client, "IS-7", sessionPTY{cols: 80, rows: 24}) + if exit != 1 { + t.Fatalf("exit=%d output=%q", exit, terminal.String()) + } + output := terminal.String() + if strings.ContainsAny(output, "\x1b\x07") || strings.Contains(output, "secret") || strings.Contains(output, "]52") { + t.Fatalf("attach output retained terminal controls: %q", output) + } + if !strings.Contains(output, "attach closed: badstate") { + t.Fatalf("attach output = %q", output) + } +} + func TestRunCommandSanitizesLinkURL(t *testing.T) { permissions := &ssh.Permissions{Extensions: map[string]string{ "authorized": "false", @@ -734,6 +783,52 @@ func TestRunCommandSanitizesLinkURL(t *testing.T) { } } +type blockingTestTerminal struct { + mu sync.Mutex + output bytes.Buffer + done chan struct{} +} + +func newBlockingTestTerminal() *blockingTestTerminal { + return &blockingTestTerminal{done: make(chan struct{})} +} + +func (t *blockingTestTerminal) Read(_ []byte) (int, error) { + <-t.done + return 0, io.EOF +} + +func (t *blockingTestTerminal) Write(data []byte) (int, error) { + t.mu.Lock() + defer t.mu.Unlock() + return t.output.Write(data) +} + +func (t *blockingTestTerminal) CancelRead() error { + close(t.done) + return nil +} + +func (t *blockingTestTerminal) String() string { + t.mu.Lock() + defer t.mu.Unlock() + return t.output.String() +} + +func testTerminalFrame(messageType byte, sessionID string, data []byte) []byte { + session := []byte(sessionID) + payload := make([]byte, 12+len(session)+len(data)) + binary.LittleEndian.PutUint16(payload[0:2], 0x5943) + payload[2] = 2 + payload[3] = messageType + binary.LittleEndian.PutUint32(payload[4:8], uint32(len(session))) + copy(payload[8:], session) + offset := 8 + len(session) + binary.LittleEndian.PutUint32(payload[offset:offset+4], uint32(len(data))) + copy(payload[offset+4:], data) + return payload +} + func TestTranscriptCommandSanitizesTerminalControls(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/api/ssh/interactive-sessions/IS-7/transcript" {