Treat the server and the filesystem as untrusted input

internal/account/account.go

@@ -81,18 +81,28 @@ func Balance(session api.Session, arguments []string) error {
 
 	idWidth := len("ID")
 	nameWidth := len("NAME")
+	nameValues := make([]string, len(balances))
+	amountValues := make([]string, len(balances))
 
-	for _, balance := range balances {
+	for index, balance := range balances {
+		name, known := fleetNames[balance.Fleet]
+
+		if !known {
+			name = "-"
+		}
+
+		formatted, _, _ := api.FormatBalance(balance)
+
+		nameValues[index] = api.Printable(name)
+		amountValues[index] = api.Printable(formatted)
 		idWidth = max(idWidth, len(strconv.FormatInt(balance.Fleet, 10)))
-		nameWidth = max(nameWidth, len(fleetNames[balance.Fleet]))
+		nameWidth = max(nameWidth, len(nameValues[index]))
 	}
 
 	fmt.Fprintf(session.Out, "%-*s  %-*s  %s\n", idWidth, "ID", nameWidth, "NAME", "BALANCE")
 
-	for _, balance := range balances {
-		formatted, _, _ := api.FormatBalance(balance)
-
-		fmt.Fprintf(session.Out, "%-*d  %-*s  %s\n", idWidth, balance.Fleet, nameWidth, fleetNames[balance.Fleet], formatted)
+	for index, balance := range balances {
+		fmt.Fprintf(session.Out, "%-*d  %-*s  %s\n", idWidth, balance.Fleet, nameWidth, nameValues[index], amountValues[index])
 	}
 
 	return nil
@@ -132,13 +142,13 @@ func Topup(session api.Session, arguments []string) error {
 		Url string `json:"url"`
 	}{}
 
-	err = json.NewDecoder(response.Body).Decode(&opened)
+	err = api.Decode(response, &opened)
 
 	if err != nil || opened.Url == "" {
 		return errors.New("could not open the top-up page, try again")
 	}
 
-	fmt.Fprintf(session.Out, "Open this link to choose an amount and pay:\n\n  %s\n\nThe credit appears on the balance once the top-up completes.\nPress enter to open the browser.\n", opened.Url)
+	fmt.Fprintf(session.Out, "Open this link to choose an amount and pay:\n\n  %s\n\nThe credit appears on the balance once the top-up completes.\nPress enter to open the browser.\n", api.Printable(opened.Url))
 
 	_, err = bufio.NewReader(session.In).ReadString('\n')
 
@@ -170,6 +180,10 @@ func Delete(session api.Session, arguments []string) error {
 
 	response.Body.Close()
 
+	if response.StatusCode == http.StatusUnauthorized {
+		return errors.New("you are not logged in, run login first")
+	}
+
 	fmt.Fprint(session.Out, "Delete your account, its logins, and your access to every fleet? This cannot be undone. [y/N] ")
 
 	answer, _ := bufio.NewReader(session.In).ReadString('\n')
@@ -199,6 +213,8 @@ func Delete(session api.Session, arguments []string) error {
 		return api.ServerError(response)
 	}
 
+	fmt.Fprintln(session.Out, "Account deleted.")
+
 	path, err := api.KeyPath()
 
 	if err != nil {
@@ -208,10 +224,8 @@ func Delete(session api.Session, arguments []string) error {
 	err = os.Remove(path)
 
 	if err != nil && !errors.Is(err, fs.ErrNotExist) {
-		return err
+		return fmt.Errorf("the login stored at %s is no longer valid but could not be removed, delete it yourself", path)
 	}
 
-	fmt.Fprintln(session.Out, "Account deleted.")
-
 	return nil
 }

internal/account/account_test.go

@@ -38,6 +38,14 @@ func TestAccountBalance(t *testing.T) {
 			wantLines:  []string{"pilot", "€0.00"},
 			wantAbsent: []string{"crew"},
 		},
+		{
+			name:       "a fleet name with control characters is escaped",
+			arguments:  []string{},
+			fleets:     `[{"id":1,"name":"\u001b[2Kquiet","owner":true}]`,
+			balances:   `[{"fleet":1,"balance":"15.000000","currency":"eur"}]`,
+			wantLines:  []string{`\x1b[2Kquiet`},
+			wantAbsent: []string{"\x1b"},
+		},
 		{
 			name:      "machine readable",
 			arguments: []string{"--json"},
@@ -394,3 +402,25 @@ func TestAccountDeleteAsksNothingWhenTheServerIsGone(t *testing.T) {
 		t.Errorf("it asked %q before finding the server was gone", out.String())
 	}
 }
+
+func TestAccountDeleteStopsWhenTheStoredLoginIsNoLongerValid(t *testing.T) {
+	mux := http.NewServeMux()
+
+	mux.HandleFunc("GET /", func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+	})
+
+	session, out := apitest.LoggedInSession(t, mux)
+
+	session.In = strings.NewReader("y\n")
+
+	err := Delete(session, nil)
+
+	if err == nil || !strings.Contains(err.Error(), "not logged in") {
+		t.Fatalf("error = %v, want it to say the login is not valid", err)
+	}
+
+	if out.String() != "" {
+		t.Errorf("output = %q, want the question never to be put", out.String())
+	}
+}

internal/api/api_test.go

@@ -3,12 +3,14 @@ package api_test
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/siliconwitchery/superstack-cli/internal/api"
 	"github.com/siliconwitchery/superstack-cli/internal/api/apitest"
@@ -171,7 +173,8 @@ func TestFetchKeysFailures(t *testing.T) {
 		wantError string
 	}{
 		{name: "server refusal", status: http.StatusServiceUnavailable, body: "keys unavailable", wantError: "keys unavailable"},
-		{name: "undecodable body", status: http.StatusOK, body: `{`, wantError: "unexpected EOF"},
+		{name: "undecodable body", status: http.StatusOK, body: `{`, wantError: "could not be read"},
+		{name: "a refusal carrying control characters", status: http.StatusServiceUnavailable, body: "\x1b[2Kgone", wantError: `\x1b[2Kgone`},
 	}
 
 	for _, test := range tests {
@@ -248,3 +251,87 @@ func TestFormatBalance(t *testing.T) {
 		})
 	}
 }
+
+func TestDecode(t *testing.T) {
+	tests := []struct {
+		name      string
+		body      string
+		wantId    int64
+		wantError string
+	}{
+		{name: "a whole object", body: `{"id":7}`, wantId: 7},
+		{name: "a truncated object", body: `{"id":`, wantError: "could not be read"},
+		{name: "a page from something that is not the server", body: "<html>bad gateway</html>", wantError: "could not be read"},
+		{name: "nothing at all", body: "", wantError: "could not be read"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			response := &http.Response{Body: io.NopCloser(strings.NewReader(test.body))}
+
+			value := struct {
+				Id int64 `json:"id"`
+			}{}
+
+			err := api.Decode(response, &value)
+
+			if test.wantError != "" {
+				if err == nil || !strings.Contains(err.Error(), test.wantError) {
+					t.Fatalf("error = %v, want it to mention %q", err, test.wantError)
+				}
+
+				for _, machinery := range []string{"unexpected EOF", "invalid character", "json", "EOF"} {
+					if strings.Contains(err.Error(), machinery) {
+						t.Errorf("the error %q shows the user %q", err, machinery)
+					}
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if value.Id != test.wantId {
+				t.Errorf("id = %d, want %d", value.Id, test.wantId)
+			}
+		})
+	}
+}
+
+type endlessBody struct{}
+
+func (endlessBody) Read(destination []byte) (int, error) {
+	for index := range destination {
+		destination[index] = 'x'
+	}
+
+	return len(destination), nil
+}
+
+func TestDecodeStopsReadingABodyThatNeverEnds(t *testing.T) {
+	response := &http.Response{
+		Body: io.NopCloser(io.MultiReader(strings.NewReader(`{"name":"`), endlessBody{})),
+	}
+
+	value := struct {
+		Name string `json:"name"`
+	}{}
+
+	finished := make(chan error, 1)
+
+	go func() {
+		finished <- api.Decode(response, &value)
+	}()
+
+	select {
+	case err := <-finished:
+		if err == nil {
+			t.Fatal("Decode accepted a body that never ends")
+		}
+
+	case <-time.After(30 * time.Second):
+		t.Fatal("Decode is still reading a body that never ends")
+	}
+}

internal/api/balances.go

@@ -1,7 +1,6 @@
 package api
 
 import (
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -35,7 +34,7 @@ func FetchBalances(session Session) ([]BalanceEntry, error) {
 
 	balances := []BalanceEntry{}
 
-	err = json.NewDecoder(response.Body).Decode(&balances)
+	err = Decode(response, &balances)
 
 	if err != nil {
 		return nil, err

internal/api/client.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"encoding/json"
 	"errors"
 	"io"
 	"io/fs"
@@ -11,6 +12,10 @@ import (
 	"strings"
 )
 
+// Larger than any answer the server can produce: it caps fleets at 100 per
+// user and keys at 100 per fleet, and only the device list is uncapped.
+const maximumBody = 32 << 20
+
 func Request(session Session, method string, path string, body io.Reader) (*http.Request, error) {
 	request, err := http.NewRequest(method, strings.TrimSuffix(session.Base, "/")+path, body)
 
@@ -37,7 +42,7 @@ func AuthenticatedRequest(session Session, method string, path string, body io.R
 	}
 
 	if err != nil {
-		return nil, err
+		return nil, errors.New("the login stored on this computer could not be read")
 	}
 
 	key := strings.TrimSpace(string(keyBytes))
@@ -66,7 +71,17 @@ func ServerError(response *http.Response) error {
 		return errors.New("that did not go through, try again in a moment")
 	}
 
-	return errors.New(detail)
+	return errors.New(Printable(detail))
+}
+
+func Decode(response *http.Response, value any) error {
+	err := json.NewDecoder(io.LimitReader(response.Body, maximumBody)).Decode(value)
+
+	if err != nil {
+		return errors.New("the server's answer could not be read, try again in a moment")
+	}
+
+	return nil
 }
 
 func KeyPath() (string, error) {
@@ -78,7 +93,7 @@ func KeyPath() (string, error) {
 			home, err := os.UserHomeDir()
 
 			if err != nil {
-				return "", err
+				return "", errors.New("your home folder could not be found, so the login has nowhere to live")
 			}
 
 			stateHome = filepath.Join(home, ".local", "state")
@@ -90,7 +105,7 @@ func KeyPath() (string, error) {
 	configDirectory, err := os.UserConfigDir()
 
 	if err != nil {
-		return "", err
+		return "", errors.New("your settings folder could not be found, so the login has nowhere to live")
 	}
 
 	return filepath.Join(configDirectory, "superstack", "key"), nil

internal/api/devices.go

@@ -1,7 +1,6 @@
 package api
 
 import (
-	"encoding/json"
 	"errors"
 	"net/http"
 )
@@ -37,7 +36,7 @@ func FetchDevices(session Session) ([]DeviceEntry, error) {
 
 	devices := []DeviceEntry{}
 
-	err = json.NewDecoder(response.Body).Decode(&devices)
+	err = Decode(response, &devices)
 
 	if err != nil {
 		return nil, err

internal/api/fleets.go

@@ -1,7 +1,6 @@
 package api
 
 import (
-	"encoding/json"
 	"errors"
 	"net/http"
 )
@@ -33,7 +32,7 @@ func FetchFleets(session Session) ([]FleetEntry, error) {
 
 	fleets := []FleetEntry{}
 
-	err = json.NewDecoder(response.Body).Decode(&fleets)
+	err = Decode(response, &fleets)
 
 	if err != nil {
 		return nil, err

internal/api/keys.go

@@ -1,7 +1,6 @@
 package api
 
 import (
-	"encoding/json"
 	"errors"
 	"net/http"
 )
@@ -34,7 +33,7 @@ func FetchKeys(session Session) ([]KeyEntry, error) {
 
 	keys := []KeyEntry{}
 
-	err = json.NewDecoder(response.Body).Decode(&keys)
+	err = Decode(response, &keys)
 
 	if err != nil {
 		return nil, err