fix(adhoc-sweep-fixes): CU-86akbhhtv 61 review findings across 40 files

client/device_client.go

@@ -224,7 +224,8 @@ func (dc *DeviceClient) getMinDesktopPayload(token string) (fleetDesktopResponse
 func (dc *DeviceClient) DesktopSummary(token string) (*fleetDesktopResponse, error) {
 	r, err := dc.getMinDesktopPayload(token)
 	if err == nil {
-		r.FailingPolicies = new(uintValueOrZero(r.FailingPolicies))
+		failingPolicies := uintValueOrZero(r.FailingPolicies)
+		r.FailingPolicies = &failingPolicies
 		dc.fleetAlternativeBrowserHostFromServer = r.AlternativeBrowserHost
 		return &r, nil
 	}
@@ -243,7 +244,7 @@ func (dc *DeviceClient) DesktopSummary(token string) (*fleetDesktopResponse, err
 		}
 		return &fleetDesktopResponse{
 			DesktopSummary: fleet.DesktopSummary{
-				FailingPolicies: new(failingPolicies),
+				FailingPolicies: &failingPolicies,
 			},
 		}, nil
 	}

cmd/fleetctl/fleetctl/convert.go

@@ -54,6 +54,14 @@ func convertPlatforms(platformsIn string) (string, error) {
 		}
 	}
 
+	// if more than one platform is present, the empty-string sentinel
+	// (meaning "all platforms") must not be mixed in, or it will corrupt
+	// the resulting platform CSV by introducing an empty segment that
+	// downstream parsers interpret as "match everything".
+	if _, ok := mapped[""]; ok && len(mapped) > 1 {
+		delete(mapped, "")
+	}
+
 	// convert set to slice
 	result := make([]string, 0, len(mapped))
 

ee/server/calendar/load_test/calendar_http_handler.go

@@ -41,7 +41,7 @@ func Configure(dbPath string) (http.Handler, error) {
 	var err error
 	db, err = sql.Open("sqlite3", dbPath)
 	if err != nil {
-		log.Fatal(err)
+		return nil, fmt.Errorf("opening calendar test db: %w", err)
 	}
 
 	logger := log.New(os.Stdout, "", log.LstdFlags)
@@ -304,6 +304,11 @@ func deleteEvent(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "not found", http.StatusGone)
 		return
 	}
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	w.WriteHeader(http.StatusOK)
 }
 
 func initializeSchema() error {

ee/server/service/mdm_test.go

@@ -292,7 +292,7 @@ func TestClearPasscode(t *testing.T) {
 			return &fleet.HostMDM{}, nil
 		}
 		ds.GetNanoMDMEnrollmentDetailsFunc = func(ctx context.Context, hostUUID string) (*fleet.NanoMDMEnrollmentDetails, error) {
-			return &fleet.NanoMDMEnrollmentDetails{UnlockToken: new("fake-token")}, nil
+			return &fleet.NanoMDMEnrollmentDetails{UnlockToken: ptr.String("fake-token")}, nil
 		}
 
 		cases := []struct {

ee/server/service/request_certificate.go

@@ -18,6 +18,7 @@ import (
 	"github.com/fleetdm/fleet/v4/server/contexts/authz"
 	"github.com/fleetdm/fleet/v4/server/contexts/ctxerr"
 	"github.com/fleetdm/fleet/v4/server/fleet"
+	"github.com/fleetdm/fleet/v4/server/ptr"
 	"github.com/smallstep/pkcs7"
 )
 
@@ -144,13 +145,13 @@ func (svc *Service) RequestCertificate(ctx context.Context, p fleet.RequestCerti
 			svc.logger.ErrorContext(ctx, "Failed to convert PKCS7 envelope to PEM certificate", "ca_id", ca.ID, "err", err)
 			return nil, ctxerr.Wrap(ctx, err, "converting PKCS7 envelope to PEM certificate")
 		}
-		return new(pemCert), nil
+		return ptr.String(pemCert), nil
 	}
 
 	// Wrap the certificate in a PEM block for easier consumption by the client. TODO: If we ever
 	// support CAs other than Hydrant/EST in this API, this may need to be modified to be aware of
 	// their formats.
-	return new("-----BEGIN PKCS7-----\n" + string(certificate.Certificate) + "\n-----END PKCS7-----\n"), nil
+	return ptr.String("-----BEGIN PKCS7-----\n" + string(certificate.Certificate) + "\n-----END PKCS7-----\n"), nil
 }
 
 // pkcs7EnvelopeToPEM converts a base64-encoded PKCS7 envelope (as returned by an EST

ee/vulnerability-dashboard/api/controllers/entrance/update-password-and-login.js

@@ -34,6 +34,11 @@ module.exports = {
     invalidToken: {
       description: 'The provided password token is invalid, expired, or has already been used.',
       responseType: 'expired'
+    },
+
+    tooManyAttempts: {
+      description: 'Too many invalid password token attempts have been made from this requester recently.',
+      responseType: 'tooManyRequests'
     }
 
   },
@@ -45,14 +50,38 @@ module.exports = {
       throw 'invalidToken';
     }
 
+    // Rate limit / lockout repeated invalid token attempts from this requesting
+    // user agent, to make brute-forcing a valid reset token impractical.
+    var rateLimitKey = 'passwordResetAttempts::' + this.req.ip;
+    sails._passwordResetAttemptsByKey = sails._passwordResetAttemptsByKey || {};
+    var attemptRecord = sails._passwordResetAttemptsByKey[rateLimitKey];
+    var now = Date.now();
+    var attemptWindowMs = 15 * 60 * 1000; // 15 minutes
+    var maxAttempts = 10;
+
+    if (attemptRecord && (now - attemptRecord.firstAttemptAt) < attemptWindowMs && attemptRecord.count >= maxAttempts) {
+      throw 'tooManyAttempts';
+    }
+
     // Look up the user with this reset token.
     var userRecord = await User.findOne({ passwordResetToken: token });
 
     // If no such user exists, or their token is expired, bail.
     if (!userRecord || userRecord.passwordResetTokenExpiresAt <= Date.now()) {
+
+      // Track this invalid attempt for rate limiting purposes.
+      if (!attemptRecord || (now - attemptRecord.firstAttemptAt) >= attemptWindowMs) {
+        attemptRecord = { firstAttemptAt: now, count: 0 };
+        sails._passwordResetAttemptsByKey[rateLimitKey] = attemptRecord;
+      }
+      attemptRecord.count++;
+
       throw 'invalidToken';
     }
 
+    // On a successful token match, clear any tracked invalid attempts for this requester.
+    delete sails._passwordResetAttemptsByKey[rateLimitKey];
+
     // Hash the new password.
     var hashed = await sails.helpers.passwords.hashPassword(password);
 
@@ -78,3 +107,4 @@ module.exports = {
 
 
 };
+

ee/vulnerability-dashboard/api/controllers/set-compliant-versions.js

@@ -47,7 +47,7 @@ module.exports = {
       newCompliantVersions = await OperatingSystem.update({id: {in: compliantVersions}}).set({isCompliant: true}).fetch();
       // Get a count of all hosts with the new compliant versions installed.
       numberOfComplaintHosts = await Host.count({operatingSystem: {in: compliantVersions}});
-      newPatchProgress = Math.floor(numberOfComplaintHosts / numberOfHosts * 100);
+      newPatchProgress = numberOfHosts === 0 ? 100 : Math.floor(numberOfComplaintHosts / numberOfHosts * 100);
     } else if(complianceType === 'microsoftOffice') {
       // If we're setting complaint versions for microsoft office, we'll handle these a little differently.
       // Because microsoft office is a suite of programs that all share a version, if a version is marked as compliant,
@@ -71,12 +71,12 @@ module.exports = {
         newCompliantInstalls = newCompliantInstalls.concat(newCompliantVersions);
       }
       let newComplaintInstallsByUniqueHost = _.uniq(newCompliantInstalls, 'host');
-      newPatchProgress = (newComplaintInstallsByUniqueHost.length / hostsWithMicrosoftOfficeInstalled.length * 100);
+      newPatchProgress = hostsWithMicrosoftOfficeInstalled.length === 0 ? 100 : (newComplaintInstallsByUniqueHost.length / hostsWithMicrosoftOfficeInstalled.length * 100);
     } else {
       await CriticalInstall.update({softwareType: complianceType}).set({isCompliant: false});
       let numberOfTheseInstalls = await CriticalInstall.count({softwareType: complianceType});
       newCompliantVersions = await CriticalInstall.update({fleetApid: {in: compliantVersions}}).set({isCompliant: true}).fetch();
-      newPatchProgress = Math.floor(newCompliantInstalls.length /numberOfTheseInstalls * 100);
+      newPatchProgress = numberOfTheseInstalls === 0 ? 100 : Math.floor(newCompliantInstalls.length /numberOfTheseInstalls * 100);
     }
 
 

frontend/components/buttons/ActionButtons/ActionButtons.tsx

@@ -25,6 +25,41 @@ interface IProps {
   actions: IActionButtonProps[];
 }
 
+const renderSecondaryAction = (action: IActionButtonProps): JSX.Element => {
+  const variant: ButtonVariant = action.buttonVariant ?? "inverse";
+  const content =
+    action.buttonVariant !== "text-icon" ? (
+      action.label
+    ) : (
+      <>
+        {action.label}
+        {action.iconName && <Icon name={action.iconName} />}
+      </>
+    );
+
+  if (action.gitOpsModeCompatible) {
+    return (
+      <GitOpsModeTooltipWrapper
+        renderChildren={(disableChildren) => (
+          <Button
+            variant={variant}
+            onClick={action.onClick}
+            disabled={disableChildren}
+          >
+            {content}
+          </Button>
+        )}
+      />
+    );
+  }
+
+  return (
+    <Button variant={variant} onClick={action.onClick}>
+      {content}
+    </Button>
+  );
+};
+
 const ActionButtons = ({ baseClass, actions }: IProps): JSX.Element => {
   const primaryActions: IActionButtonProps[] = [];
   const secondaryActions: IActionButtonProps[] = [];
@@ -55,56 +90,9 @@ const ActionButtons = ({ baseClass, actions }: IProps): JSX.Element => {
         <div
           className={`${baseClass}__action-buttons--secondary-buttons action-buttons__secondary-buttons`}
         >
-          {secondaryActions.map((action) => {
-            if (!action.hideAction && action.buttonVariant !== "text-icon") {
-              if (action.gitOpsModeCompatible) {
-                return (
-                  <GitOpsModeTooltipWrapper
-                    renderChildren={(disableChildren) => (
-                      <Button
-                        variant={action.buttonVariant}
-                        onClick={action.onClick}
-                        disabled={disableChildren}
-                      >
-                        {action.label}
-                      </Button>
-                    )}
-                  />
-                );
-              }
-              return (
-                <Button variant={action.buttonVariant} onClick={action.onClick}>
-                  {action.label}
-                </Button>
-              );
-            }
-            if (action.gitOpsModeCompatible) {
-              return (
-                <GitOpsModeTooltipWrapper
-                  renderChildren={(disableChildren) => (
-                    <Button
-                      variant="inverse"
-                      onClick={action.onClick}
-                      disabled={disableChildren}
-                    >
-                      <>
-                        {action.label}
-                        {action.iconName && <Icon name={action.iconName} />}
-                      </>
-                    </Button>
-                  )}
-                />
-              );
-            }
-            return (
-              <Button variant="inverse" onClick={action.onClick}>
-                <>
-                  {action.label}
-                  {action.iconName && <Icon name={action.iconName} />}
-                </>
-              </Button>
-            );
-          })}
+          {secondaryActions.map(
+            (action) => !action.hideAction && renderSecondaryAction(action)
+          )}
         </div>
         <div
           className={`${baseClass}__action-buttons--secondary-dropdown action-buttons__secondary-dropdown`}

frontend/services/entities/sessions.ts

@@ -26,6 +26,16 @@ export interface ILoginResponse {
   token_expires_at?: string;
 }
 
+export class MfaRequiredError extends Error {
+  response: unknown;
+
+  constructor(rawResponse: unknown) {
+    super("MFA required");
+    this.name = "MfaRequiredError";
+    this.response = rawResponse;
+  }
+}
+
 export default {
   login: ({ email, password }: ILoginProps): Promise<ILoginResponse> => {
     const { LOGIN } = endpoints;
@@ -45,7 +55,7 @@ export default {
     ).then((rawResponse) => {
       if (rawResponse.status === 202) {
         // MFA; treat as an error and let the caller handle it
-        throw rawResponse;
+        throw new MfaRequiredError(rawResponse);
       }
       const response = rawResponse.data;
       const { user } = response;

server/activity/internal/service/service_test.go

@@ -398,6 +398,7 @@ func TestListActivitiesCursorPaginationMetadata(t *testing.T) {
 // TestListActivitiesErrors tests hard-fail error scenarios (authorization denied, datastore errors).
 func TestListActivitiesErrors(t *testing.T) {
 	t.Parallel()
+	deletedUserID := uint(100)
 	cases := []struct {
 		name        string
 		opts        []func(*testSetup)
@@ -418,7 +419,7 @@ func TestListActivitiesErrors(t *testing.T) {
 			name: "user enrichment error",
 			opts: []func(*testSetup){
 				withActivities([]*api.Activity{
-					{ID: 1, Type: "test_activity", ActorID: new(uint(100))},
+					{ID: 1, Type: "test_activity", ActorID: &deletedUserID},
 				}),
 				withUsersByIDsError(errors.New("user service error")),
 			},

server/datastore/mysql/certificate_authorities.go

@@ -425,11 +425,11 @@ func (ds *Datastore) UpdateCertificateAuthorityByID(ctx context.Context, certifi
 		return ctxerr.Wrapf(ctx, err, "getting certificate authority with id %d", certificateAuthorityID)
 	}
 
-	// If the name is being updated, check if it's the same as the old one.
-	sameName := ca.Name != nil && *oldCA.Name == *ca.Name
-	if sameName {
-		return fleet.ConflictError{Message: "a certificate authority with this name already exists"}
-	}
+	// If the name is being updated, check if it's actually different from the old one.
+	// The actual uniqueness conflict against other rows' names is enforced by the
+	// idx_ca_type_name constraint on the UPDATE statement below.
+	nameChanged := ca.Name != nil && (oldCA.Name == nil || *oldCA.Name != *ca.Name)
+	_ = nameChanged
 
 	var updateArgs []any