Add methods to instantiate a StatusError without formatting (#13987)

Adds two new methods to instantiate an `api.StatusError`: - `NewStatusError(status int, msg string)`. - `NewGenericStatusError(status int)`. These can be used in the following contexts: 1. When there is an error to wrap but we want to add a status code to it, use `api.StatusErrorf`. 2. When there is no error to wrap but we want to return an error with a status code, use `api.NewStatusError`. 3. When we want to mask an error message to prevent any information leakage (e.g. for auth), use `api.NewGenericStatusError`.
canonical · Aug 27, 2024 · 9a5dda1 · 9a5dda1
2 parents a894583 + ba484a8
commit 9a5dda1
Show file tree

Hide file tree

Showing 48 changed files with 3,340 additions and 3,355 deletions.
diff --git a/lxc/remote.go b/lxc/remote.go
@@ -3,6 +3,7 @@ package main
 import (
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -156,7 +157,7 @@ func (c *cmdRemoteAdd) runToken(server string, token string, rawToken *api.Certi
 	conf := c.global.conf
 
 	if !conf.HasClientCertificate() {
-		fmt.Fprintf(os.Stderr, i18n.G("Generating a client certificate. This may take a minute...")+"\n")
+		fmt.Fprint(os.Stderr, i18n.G("Generating a client certificate. This may take a minute...")+"\n")
 		err := conf.GenerateClientCertificate()
 		if err != nil {
 			return err
@@ -179,15 +180,15 @@ func (c *cmdRemoteAdd) runToken(server string, token string, rawToken *api.Certi
 	}
 
 	fmt.Println(i18n.G("All server addresses are unavailable"))
-	fmt.Printf(i18n.G("Please provide an alternate server address (empty to abort):") + " ")
+	fmt.Print(i18n.G("Please provide an alternate server address (empty to abort):") + " ")
 
 	line, err := shared.ReadStdin()
 	if err != nil {
 		return err
 	}
 
 	if len(line) == 0 {
-		return fmt.Errorf(i18n.G("Failed to add remote"))
+		return errors.New(i18n.G("Failed to add remote"))
 	}
 
 	err = c.addRemoteFromToken(string(line), server, token, rawToken.Fingerprint)
@@ -210,7 +211,7 @@ func (c *cmdRemoteAdd) addRemoteFromToken(addr string, server string, token stri
 	if err != nil {
 		certificate, err = shared.GetRemoteCertificate(addr, c.global.conf.UserAgent)
 		if err != nil {
-			return api.StatusErrorf(http.StatusServiceUnavailable, i18n.G("Unavailable remote server")+": %w", err)
+			return api.StatusErrorf(http.StatusServiceUnavailable, "%s: %w", i18n.G("Unavailable remote server"), err)
 		}
 
 		certDigest := shared.CertFingerprint(certificate)
@@ -221,7 +222,7 @@ func (c *cmdRemoteAdd) addRemoteFromToken(addr string, server string, token stri
 		dnam := conf.ConfigPath("servercerts")
 		err := os.MkdirAll(dnam, 0750)
 		if err != nil {
-			return fmt.Errorf(i18n.G("Could not create server cert dir"))
+			return errors.New(i18n.G("Could not create server cert dir"))
 		}
 
 		certf := conf.ServerCertPath(server)
@@ -244,7 +245,7 @@ func (c *cmdRemoteAdd) addRemoteFromToken(addr string, server string, token stri
 
 	d, err := conf.GetInstanceServer(server)
 	if err != nil {
-		return api.StatusErrorf(http.StatusServiceUnavailable, i18n.G("Unavailable remote server")+": %w", err)
+		return api.StatusErrorf(http.StatusServiceUnavailable, "%s: %w", i18n.G("Unavailable remote server"), err)
 	}
 
 	req := api.CertificatesPost{}
@@ -290,12 +291,12 @@ func (c *cmdRemoteAdd) run(cmd *cobra.Command, args []string) error {
 	}
 
 	if len(addr) == 0 {
-		return fmt.Errorf(i18n.G("Remote address must not be empty"))
+		return errors.New(i18n.G("Remote address must not be empty"))
 	}
 
 	// Validate the server name.
 	if strings.Contains(server, ":") {
-		return fmt.Errorf(i18n.G("Remote names may not contain colons"))
+		return errors.New(i18n.G("Remote names may not contain colons"))
 	}
 
 	// Check for existing remote
@@ -332,7 +333,7 @@ func (c *cmdRemoteAdd) run(cmd *cobra.Command, args []string) error {
 	// Fast track simplestreams
 	if c.flagProtocol == "simplestreams" {
 		if remoteURL.Scheme != "https" {
-			return fmt.Errorf(i18n.G("Only https URLs are supported for simplestreams"))
+			return errors.New(i18n.G("Only https URLs are supported for simplestreams"))
 		}
 
 		conf.Remotes[server] = config.Remote{Addr: addr, Public: true, Protocol: c.flagProtocol}
@@ -397,7 +398,7 @@ func (c *cmdRemoteAdd) run(cmd *cobra.Command, args []string) error {
 	// adding the remote server.
 	if rScheme != "unix" && !c.flagPublic && (c.flagAuthType == api.AuthenticationMethodTLS || c.flagAuthType == "") {
 		if !conf.HasClientCertificate() {
-			fmt.Fprintf(os.Stderr, i18n.G("Generating a client certificate. This may take a minute...")+"\n")
+			fmt.Fprint(os.Stderr, i18n.G("Generating a client certificate. This may take a minute...")+"\n")
 			err = conf.GenerateClientCertificate()
 			if err != nil {
 				return err
@@ -451,26 +452,26 @@ func (c *cmdRemoteAdd) run(cmd *cobra.Command, args []string) error {
 		if !c.flagAcceptCert {
 			digest := shared.CertFingerprint(certificate)
 
-			fmt.Printf(i18n.G("Certificate fingerprint: %s")+"\n", digest)
-			fmt.Printf(i18n.G("ok (y/n/[fingerprint])?") + " ")
+			fmt.Printf("%s: %s\n", i18n.G("Certificate fingerprint"), digest)
+			fmt.Print(i18n.G("ok (y/n/[fingerprint])?") + " ")
 			line, err := shared.ReadStdin()
 			if err != nil {
 				return err
 			}
 
 			if string(line) != digest {
 				if len(line) < 1 || strings.ToLower(string(line[0])) == i18n.G("n") {
-					return fmt.Errorf(i18n.G("Server certificate NACKed by user"))
+					return errors.New(i18n.G("Server certificate NACKed by user"))
 				} else if strings.ToLower(string(line[0])) != i18n.G("y") {
-					return fmt.Errorf(i18n.G("Please type 'y', 'n' or the fingerprint:"))
+					return errors.New(i18n.G("Please type 'y', 'n' or the fingerprint:"))
 				}
 			}
 		}
 
 		dnam := conf.ConfigPath("servercerts")
 		err := os.MkdirAll(dnam, 0750)
 		if err != nil {
-			return fmt.Errorf(i18n.G("Could not create server cert dir"))
+			return errors.New(i18n.G("Could not create server cert dir"))
 		}
 
 		certf := conf.ServerCertPath(server)
@@ -612,7 +613,7 @@ func (c *cmdRemoteAdd) run(cmd *cobra.Command, args []string) error {
 		}
 
 		if srv.Auth != "trusted" {
-			return fmt.Errorf(i18n.G("Server doesn't trust us after authentication"))
+			return errors.New(i18n.G("Server doesn't trust us after authentication"))
 		}
 
 		if c.flagAuthType == api.AuthenticationMethodTLS {
@@ -874,7 +875,7 @@ func (c *cmdRemoteRemove) run(cmd *cobra.Command, args []string) error {
 	}
 
 	if conf.DefaultRemote == args[0] {
-		return fmt.Errorf(i18n.G("Can't remove the default remote"))
+		return errors.New(i18n.G("Can't remove the default remote"))
 	}
 
 	delete(conf.Remotes, args[0])

diff --git a/lxd/auth/drivers/openfga.go b/lxd/auth/drivers/openfga.go
@@ -133,7 +133,7 @@ func (e *embeddedOpenFGA) CheckPermission(ctx context.Context, entityURL *api.UR
 
 	// Untrusted requests are denied.
 	if !auth.IsTrusted(ctx) {
-		return api.StatusErrorf(http.StatusForbidden, "%s", http.StatusText(http.StatusForbidden))
+		return api.NewGenericStatusError(http.StatusForbidden)
 	}
 
 	isRoot, err := auth.IsServerAdmin(ctx, e.identityCache)
@@ -284,7 +284,7 @@ func (e *embeddedOpenFGA) CheckPermission(ctx context.Context, entityURL *api.UR
 			l.Info("Access denied", logger.Ctx{"http_code": responseCode})
 		}
 
-		return api.StatusErrorf(responseCode, "%s", http.StatusText(responseCode))
+		return api.NewGenericStatusError(responseCode)
 	}
 
 	return nil

diff --git a/lxd/auth/drivers/tls.go b/lxd/auth/drivers/tls.go
@@ -43,7 +43,7 @@ func (t *tls) load(ctx context.Context, identityCache *identity.Cache, opts Opts
 func (t *tls) CheckPermission(ctx context.Context, entityURL *api.URL, entitlement auth.Entitlement) error {
 	// Untrusted requests are denied.
 	if !auth.IsTrusted(ctx) {
-		return api.StatusErrorf(http.StatusForbidden, "%s", http.StatusText(http.StatusForbidden))
+		return api.NewGenericStatusError(http.StatusForbidden)
 	}
 
 	isRoot, err := auth.IsServerAdmin(ctx, t.identities)

diff --git a/lxd/devlxd.go b/lxd/devlxd.go
@@ -249,7 +249,7 @@ func devlxdAPIHandlerFunc(d *Daemon, c instance.Instance, w http.ResponseWriter,
 
 		err := json.NewDecoder(r.Body).Decode(&req)
 		if err != nil {
-			return response.DevLxdErrorResponse(api.StatusErrorf(http.StatusBadRequest, err.Error()), c.Type() == instancetype.VM)
+			return response.DevLxdErrorResponse(api.StatusErrorf(http.StatusBadRequest, "Invalid request body: %w", err), c.Type() == instancetype.VM)
 		}
 
 		state := api.StatusCodeFromString(req.State)
@@ -260,7 +260,7 @@ func devlxdAPIHandlerFunc(d *Daemon, c instance.Instance, w http.ResponseWriter,
 
 		err = c.VolatileSet(map[string]string{"volatile.last_state.ready": strconv.FormatBool(state == api.Ready)})
 		if err != nil {
-			return response.DevLxdErrorResponse(api.StatusErrorf(http.StatusInternalServerError, err.Error()), c.Type() == instancetype.VM)
+			return response.DevLxdErrorResponse(api.StatusErrorf(http.StatusInternalServerError, "Failed to set instance state: %w", err), c.Type() == instancetype.VM)
 		}
 
 		if state == api.Ready {
@@ -270,7 +270,7 @@ func devlxdAPIHandlerFunc(d *Daemon, c instance.Instance, w http.ResponseWriter,
 		return response.DevLxdResponse(http.StatusOK, "", "raw", c.Type() == instancetype.VM)
 	}
 
-	return response.DevLxdErrorResponse(api.StatusErrorf(http.StatusMethodNotAllowed, fmt.Sprintf("method %q not allowed", r.Method)), c.Type() == instancetype.VM)
+	return response.DevLxdErrorResponse(api.StatusErrorf(http.StatusMethodNotAllowed, "method %q not allowed", r.Method), c.Type() == instancetype.VM)
 }
 
 var devlxdDevicesGet = devLxdHandler{

diff --git a/lxd/instance/drivers/qmp/commands.go b/lxd/instance/drivers/qmp/commands.go
@@ -505,7 +505,7 @@ func (m *Monitor) RemoveBlockDevice(blockDevName string) error {
 		err := m.run("blockdev-del", blockDevName, nil)
 		if err != nil {
 			if strings.Contains(err.Error(), "is in use") {
-				return api.StatusErrorf(http.StatusLocked, err.Error())
+				return api.StatusErrorf(http.StatusLocked, "%w", err)
 			}
 
 			if strings.Contains(err.Error(), "Failed to find") {

diff --git a/lxd/instance_sftp.go b/lxd/instance_sftp.go
@@ -135,7 +135,7 @@ func (r *sftpServeResponse) Render(w http.ResponseWriter) error {
 
 	err = response.Upgrade(remoteConn, "sftp")
 	if err != nil {
-		return api.StatusErrorf(http.StatusInternalServerError, err.Error())
+		return api.StatusErrorf(http.StatusInternalServerError, "Failed to upgrade SFTP connection: %w", err)
 	}
 
 	ctx, cancel := context.WithCancel(r.req.Context())

diff --git a/lxd/project/permissions.go b/lxd/project/permissions.go
@@ -1632,7 +1632,7 @@ func CheckTargetMember(p *api.Project, targetMemberName string, allMembers []db.
 			// If restricted groups are specified then check member is in at least one of them.
 			err := AllowClusterMember(p, &potentialMember)
 			if err != nil {
-				return nil, api.StatusErrorf(http.StatusForbidden, err.Error())
+				return nil, api.StatusErrorf(http.StatusForbidden, "%w", err)
 			}
 
 			return &potentialMember, nil
@@ -1647,7 +1647,7 @@ func CheckTargetGroup(ctx context.Context, tx *db.ClusterTx, p *api.Project, gro
 	// If restricted groups are specified then check the requested group is in the list.
 	err := AllowClusterGroup(p, groupName)
 	if err != nil {
-		return api.StatusErrorf(http.StatusForbidden, err.Error())
+		return api.StatusErrorf(http.StatusForbidden, "%w", err)
 	}
 
 	// Check if the target group exists.