Improve handling of timeouts and maximum size for the upload software installer endpoint (#19657)

changes/19453-improve-software-installer-upload-endpoint

@@ -0,0 +1 @@
+* Extended the timeout for the endpoint to upload a software installer (`POST /fleet/software/package`), and improved handling of the maximum size.

cmd/fleet/serve.go

@@ -1027,7 +1027,7 @@ the way that the Fleet server works.
 				}
 			}
 
-			// We must wrap the Handler here to set special per-endpoint Write
+			// We must wrap the Handler here to set special per-endpoint Read/Write
 			// timeouts, so that we have access to the raw http.ResponseWriter.
 			// Otherwise, the handler is wrapped by the promhttp response delegator,
 			// which does not support the Unwrap call needed to work with
@@ -1038,6 +1038,9 @@ the way that the Fleet server works.
 			// does not implement.
 			rootMux.HandleFunc("/api/", func(rw http.ResponseWriter, req *http.Request) {
 				if req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/fleet/scripts/run/sync") {
+					// when running a script synchronously, we wait a while for a script
+					// execution result, so the write timeout (to write the response)
+					// must be extended.
 					rc := http.NewResponseController(rw)
 					// add an additional 30 seconds to prevent race conditions where the
 					// request is terminated early.
@@ -1045,6 +1048,19 @@ the way that the Fleet server works.
 						level.Error(logger).Log("msg", "http middleware failed to override endpoint write timeout", "err", err)
 					}
 				}
+
+				if req.Method == http.MethodPost && strings.HasSuffix(req.URL.Path, "/fleet/software/package") {
+					// when uploading a software installer, the file might be large so
+					// the read timeout (to read the full request body) must be extended.
+					rc := http.NewResponseController(rw)
+					// the frontend times out waiting for the upload after 2 minutes, so
+					// use that same timeout:
+					// https://www.figma.com/design/oQl2oQUG0iRkUy0YOxc307/%2314921-Deploy-security-agents-to-macOS%2C-Windows%2C-and-Linux-hosts?node-id=773-18032&t=QjEU6tc73tddNSqn-0
+					if err := rc.SetReadDeadline(time.Now().Add(2 * time.Minute)); err != nil {
+						level.Error(logger).Log("msg", "http middleware failed to override endpoint read timeout", "err", err)
+					}
+					req.Body = http.MaxBytesReader(rw, req.Body, service.MaxSoftwareInstallerSize)
+				}
 				apiHandler.ServeHTTP(rw, req)
 			})
 			rootMux.Handle("/", frontendHandler)

server/service/software_installers.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"mime/multipart"
@@ -29,14 +30,25 @@ type uploadSoftwareInstallerResponse struct {
 	Err error `json:"error,omitempty"`
 }
 
+// MaxSoftwareInstallerSize is the maximum size allowed for software
+// installers. This is enforced by the endpoint that uploads installers.
+const MaxSoftwareInstallerSize = 500 * units.MiB
+
 // TODO: We parse the whole body before running svc.authz.Authorize.
 // An authenticated but unauthorized user could abuse this.
 func (uploadSoftwareInstallerRequest) DecodeRequest(ctx context.Context, r *http.Request) (interface{}, error) {
 	decoded := uploadSoftwareInstallerRequest{}
 	err := r.ParseMultipartForm(512 * units.MiB)
 	if err != nil {
+		var mbe *http.MaxBytesError
+		if errors.As(err, &mbe) {
+			return nil, &fleet.BadRequestError{
+				Message:     "The maximum file size is 500 MB.",
+				InternalErr: err,
+			}
+		}
 		return nil, &fleet.BadRequestError{
-			Message:     "failed to parse multipart form",
+			Message:     "failed to parse multipart form: " + err.Error(),
 			InternalErr: err,
 		}
 	}
@@ -49,9 +61,9 @@ func (uploadSoftwareInstallerRequest) DecodeRequest(ctx context.Context, r *http
 	}
 
 	decoded.File = r.MultipartForm.File["software"][0]
-
-	if decoded.File.Size > 500*units.MiB {
-		// TODO: Should we try to assess the size earlier in the request processing (before parsing the form)?
+	if decoded.File.Size > MaxSoftwareInstallerSize {
+		// Should never happen here since the request's body is limited to the
+		// maximum size.
 		return nil, &fleet.BadRequestError{
 			Message: "The maximum file size is 500 MB.",
 		}