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

android/app/src/main/java/com/fleetdm/agent/ApiClient.kt

@@ -257,15 +257,17 @@ object ApiClient : CertificateApiClient {
         val result = block()
         if (result.isFailure && result.exceptionOrNull() is UnauthorizedException) {
             Log.d(TAG, "Received 401, clearing node key and retrying with re-enrollment")
-            clearApiKey()
+            enrollmentMutex.withLock {
+                clearApiKey()
+            }
             return block()
         }
         return result
     }
 
-    suspend fun enroll(): Result<EnrollResponse> {
+    suspend fun enroll(): Result<EnrollResponse> = enrollmentMutex.withLock {
         val credentials = getEnrollmentCredentials()
-        credentials ?: return Result.failure(Exception("Credentials not set"))
+        credentials ?: return@withLock Result.failure(Exception("Credentials not set"))
         val resp = makeRequest(
             endpoint = "/api/fleet/orbit/enroll",
             method = "POST",
@@ -286,7 +288,7 @@ object ApiClient : CertificateApiClient {
             Log.d(TAG, "Enrollment failed: ${exception.message}")
         }
 
-        return resp
+        resp
     }
 
     suspend fun getOrbitConfig(): Result<OrbitConfig> = withReenrollOnUnauthorized {
@@ -405,30 +407,29 @@ object ApiClient : CertificateApiClient {
     }
 
     private suspend fun getNodeKeyOrEnroll(): Result<String> {
-        enrollmentMutex.withLock {
-            // Check again inside lock in case another coroutine just enrolled
-            val existingKey = getApiKey()
-            if (existingKey != null) {
-                return Result.success(existingKey)
-            }
-
-            // Node key is missing, attempt auto-enrollment
-            Log.d(TAG, "Orbit node key missing, attempting auto-enrollment")
-
-            // Re-enroll
-            val enrollResult = enroll()
-
-            return enrollResult.fold(
-                onSuccess = { response ->
-                    Log.d(TAG, "Auto-enrollment successful")
-                    Result.success(response.orbitNodeKey)
-                },
-                onFailure = { error ->
-                    FleetLog.e(TAG, "Auto-enrollment failed: ${error.message}")
-                    Result.failure(error)
-                },
-            )
+        // Check outside the lock to avoid unnecessary contention when a key already exists.
+        val existingKey = getApiKey()
+        if (existingKey != null) {
+            return Result.success(existingKey)
         }
+
+        // Node key is missing, attempt auto-enrollment
+        Log.d(TAG, "Orbit node key missing, attempting auto-enrollment")
+
+        // enroll() acquires enrollmentMutex internally to guard the enroll/store sequence
+        // against concurrent re-enrollment attempts.
+        val enrollResult = enroll()
+
+        return enrollResult.fold(
+            onSuccess = { response ->
+                Log.d(TAG, "Auto-enrollment successful")
+                Result.success(response.orbitNodeKey)
+            },
+            onFailure = { error ->
+                FleetLog.e(TAG, "Auto-enrollment failed: ${error.message}")
+                Result.failure(error)
+            },
+        )
     }
 
     private data class EnrollmentCredentials(
@@ -633,3 +634,4 @@ data class GetCertificateTemplateResponse(
  */
 fun GetCertificateTemplateResponse.buildScepUrl(serverUrl: String, hostUUID: String): String =
     "$serverUrl/mdm/scep/proxy/$hostUUID,g$id,$certificateAuthorityType,${fleetChallenge ?: ""}"
+

android/settings.gradle.kts

@@ -19,6 +19,6 @@ dependencyResolutionManagement {
     }
 }
 
-rootProject.name = "My Application"
+rootProject.name = "fleetmdm"
 include(":app")
-
+

cmd/osquery-perf/osquery_perf/stats.go

@@ -273,10 +273,15 @@ func (s *Stats) Log() {
 	s.l.Lock()
 	defer s.l.Unlock()
 
+	var errorRate float64
+	if s.osqueryEnrollments > 0 {
+		errorRate = float64(s.errors) / float64(s.osqueryEnrollments)
+	}
+
 	log.Printf(
 		"uptime: %s, error rate: %.2f, osquery enrolls: %d, orbit enrolls: %d, mdm enrolls: %d, distributed/reads: %d, distributed/writes: %d, config requests: %d, result log requests: %d, mdm sessions initiated: %d, mdm on-demand syncs: %d, mdm commands received: %d, config errors: %d, distributed/read errors: %d, distributed/write errors: %d, log result errors: %d, orbit errors: %d, desktop errors: %d, mdm errors: %d, mdm scep requests: %d, mdm scep success: %d, mdm scep errors: %d, ddm tokens success: %d, ddm tokens errors: %d, ddm declaration items success: %d, ddm declaration items errors: %d, ddm activation success: %d, ddm activation errors: %d, ddm configuration success: %d, ddm configuration errors: %d, ddm status success: %d, ddm status errors: %d, buffered logs: %d, script execs (errs): %d (%d), software installs (errs): %d (%d)",
 		time.Since(s.StartTime).Round(time.Second),
-		float64(s.errors)/float64(s.osqueryEnrollments),
+		errorRate,
 		s.osqueryEnrollments,
 		s.orbitEnrollments,
 		s.mdmEnrollments,

ee/fleetd-chrome/src/tables/os_version.ts

@@ -59,6 +59,9 @@ export default class TableOSVersion extends Table {
         console.warn(
           `Chrome version ${version} does not have expected 4 segments`
         );
+        for (const column of ["major", "minor", "build", "patch"]) {
+          warningsArray.push({ column, error_message: "unexpected version format" });
+        }
       } else {
         [major, minor, build, patch] = splits;
       }

ee/maintained-apps/ingesters/homebrew/external_refs/version_shortener.go

@@ -15,7 +15,7 @@ import (
 func makeVersionShortener(keepSegments int) func(*maintained_apps.FMAManifestApp) (*maintained_apps.FMAManifestApp, error) {
 	return func(app *maintained_apps.FMAManifestApp) (*maintained_apps.FMAManifestApp, error) {
 		if app.Version == "" {
-			return app, fmt.Errorf("empty version for app %s", app.Slug)
+			return app, errors.New(fmt.Sprintf("empty version for app %s", app.Slug))
 		}
 		parts := strings.Split(app.Version, ".")
 		if len(parts) <= keepSegments {
@@ -46,7 +46,7 @@ var (
 // the host version always greater, breaking patch policy detection.
 func SublimeVersionTransformer(app *maintained_apps.FMAManifestApp) (*maintained_apps.FMAManifestApp, error) {
 	if app.Version == "" {
-		return app, fmt.Errorf("empty version for Sublime app %s", app.Slug)
+		return app, errors.New(fmt.Sprintf("empty version for Sublime app %s", app.Slug))
 	}
 	if strings.HasPrefix(app.Version, "Build ") {
 		return app, nil

ee/maintained-apps/inputs/homebrew/scripts/gpg-suite-uninstall.sh

@@ -134,7 +134,7 @@ remove_receipt_files() {
   fi
 
   echo "sudo pkgutil --only-files --files \"$PKGID\" | sed \"s|^|${FULL_INSTALL_LOCATION}/|\" | tr '\\\\n' '\\\\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf"
-  sudo pkgutil --only-files --files "$PKGID" | sed "s|^|/${INSTALL_LOCATION}/|" | tr '\n' '\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf
+  sudo pkgutil --only-files --files "$PKGID" | sed "s|^|${FULL_INSTALL_LOCATION}/|" | tr '\n' '\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf
 
   echo "sudo pkgutil --only-dirs --files \"$PKGID\" | sed \"s|^|${FULL_INSTALL_LOCATION}/|\" | grep '\\.app$' | tr '\\\\n' '\\\\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf"
   sudo pkgutil --only-dirs --files "$PKGID" | sed "s|^|${FULL_INSTALL_LOCATION}/|" | grep '\.app$' | tr '\n' '\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf
@@ -237,3 +237,4 @@ trash $LOGGED_IN_USER '~/Library/PreferencePanes/GPGPreferences.prefPane'
 trash $LOGGED_IN_USER '~/Library/Preferences/org.gpgtools.*'
 trash $LOGGED_IN_USER '~/Library/Services/GPGServices.service'
 
+

ee/maintained-apps/inputs/homebrew/scripts/microsoft_word_uninstall.sh

@@ -134,7 +134,7 @@ remove_receipt_files() {
   fi
 
   echo "sudo pkgutil --only-files --files \"$PKGID\" | sed \"s|^|${FULL_INSTALL_LOCATION}/|\" | tr '\\\\n' '\\\\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf"
-  sudo pkgutil --only-files --files "$PKGID" | sed "s|^|/${INSTALL_LOCATION}/|" | tr '\n' '\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf
+  sudo pkgutil --only-files --files "$PKGID" | sed "s|^|${FULL_INSTALL_LOCATION}/|" | tr '\n' '\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf
 
   echo "sudo pkgutil --only-dirs --files \"$PKGID\" | sed \"s|^|${FULL_INSTALL_LOCATION}/|\" | grep '\\.app$' | tr '\\\\n' '\\\\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf"
   sudo pkgutil --only-dirs --files "$PKGID" | sed "s|^|${FULL_INSTALL_LOCATION}/|" | grep '\.app$' | tr '\n' '\0' | /usr/bin/sudo -u root -E -- /usr/bin/xargs -0 -- /bin/rm -rf
@@ -188,3 +188,4 @@ trash $LOGGED_IN_USER '~/Library/Application Support/CrashReporter/Microsoft Wor
 trash $LOGGED_IN_USER '~/Library/Containers/com.microsoft.Word'
 trash $LOGGED_IN_USER '~/Library/Preferences/com.microsoft.Word.plist'
 trash $LOGGED_IN_USER '~/Library/Saved Application State/com.microsoft.Word.savedState'
+

ee/server/service/hostidentity/config.go

@@ -9,13 +9,13 @@ import (
 	"github.com/fleetdm/fleet/v4/server/mdm/scep/depot"
 )
 
-func initAssets(ds fleet.Datastore) error {
+func initAssets(ctx context.Context, ds fleet.Datastore) error {
 	// Check if we have existing certs and keys
 	expectedAssets := []fleet.MDMAssetName{
 		fleet.MDMAssetHostIdentityCACert,
 		fleet.MDMAssetHostIdentityCAKey,
 	}
-	savedAssets, err := ds.GetAllMDMConfigAssetsByName(context.Background(), expectedAssets, nil)
+	savedAssets, err := ds.GetAllMDMConfigAssetsByName(ctx, expectedAssets, nil)
 	if err != nil {
 		// allow not found errors as it means we're generating the assets for the first time.
 		if !fleet.IsNotFound(err) {
@@ -49,9 +49,10 @@ func initAssets(ds fleet.Datastore) error {
 			})
 		}
 
-		if err := ds.InsertMDMConfigAssets(context.Background(), assets, nil); err != nil {
+		if err := ds.InsertMDMConfigAssets(ctx, assets, nil); err != nil {
 			return fmt.Errorf("inserting host identity SCEP assets: %w", err)
 		}
 	}
 	return nil
 }
+

ee/server/service/scep/sceptest/sceptest.go

@@ -11,7 +11,6 @@ import (
 	"crypto/x509"
 	_ "embed"
 	"encoding/binary"
-	"fmt"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -151,7 +150,7 @@ func NewTestDynamicChallengeServer(t *testing.T) *httptest.Server {
 
 	dynamicChallengeServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
-		fmt.Println(r.URL.Path)
+		t.Logf("dynamic challenge request: %s", r.URL.Path)
 		if _, err := w.Write([]byte("dynamic challenge")); err != nil {
 			t.Errorf("write dynamic challenge response: %v", err)
 		}

frontend/components/ActivityItem/ActivityItem.tsx

@@ -91,11 +91,9 @@ const ActivityItem = ({
     ? addGravatarUrlToResource({ email: actor_email })
     : { gravatar_url: undefined };
 
-  // wrapped just in case the date string does not parse correctly
-  let activityCreatedAt: Date;
-  try {
-    activityCreatedAt = new Date(activity.created_at);
-  } catch (e) {
+  // fall back to the current date if the date string does not parse correctly
+  let activityCreatedAt = new Date(activity.created_at);
+  if (isNaN(activityCreatedAt.getTime())) {
     activityCreatedAt = new Date();
   }
 
@@ -139,3 +137,4 @@ const ActivityItem = ({
 };
 
 export default ActivityItem;
+

frontend/components/CommandPalette/components/HostPicker.tests.tsx

@@ -105,16 +105,15 @@ describe("HostPicker", () => {
       team_name: "Engineering",
     });
 
-    // The shared QueryClient persists React Query's cache across tests
-    // in this file. Earlier tests register an empty result under
-    // queryKey ["commandPaletteHosts", ""], so subsequent renders with
-    // the same search would read that cached emptiness and never hit
-    // the mock. Each column test uses a unique search string to get a
-    // fresh queryFn invocation. The mock ignores the query value, so
-    // the same `hosts` is returned regardless.
-    it("renders a status dot next to the host name (no text)", async () => {
+    // Each test in this file renders with its own fresh QueryClient (see
+    // createCustomRenderer), so there is no cross-test cache to worry
+    // about here. The mock ignores the query value, so the same `hosts`
+    // is returned regardless of the search string used.
+    beforeEach(() => {
       mockedHosts.loadHosts.mockResolvedValue(hosts);
+    });
 
+    it("renders a status dot next to the host name (no text)", async () => {
       const { findByText, container } = renderPicker(
         <HostPicker search="col-dot-test" onSelect={jest.fn()} />
       );
@@ -131,17 +130,13 @@ describe("HostPicker", () => {
     });
 
     it("renders the host's team in the right-aligned column when showTeamColumn", async () => {
-      mockedHosts.loadHosts.mockResolvedValue(hosts);
-
       const { findByText } = renderPicker(
         <HostPicker search="col-team-on" showTeamColumn onSelect={jest.fn()} />
       );
       expect(await findByText("Engineering")).toBeInTheDocument();
     });
 
     it("suppresses the team column by default (Free / Primo / single-fleet)", async () => {
-      mockedHosts.loadHosts.mockResolvedValue(hosts);
-
       const { findByText, queryByText } = renderPicker(
         <HostPicker search="col-team-off" onSelect={jest.fn()} />
       );
@@ -151,8 +146,6 @@ describe("HostPicker", () => {
     });
 
     it("highlights the matched substring of the host name", async () => {
-      mockedHosts.loadHosts.mockResolvedValue(hosts);
-
       const { findByText, container } = renderPicker(
         <HostPicker search="Rachel" onSelect={jest.fn()} />
       );

frontend/components/CommandPalette/components/PolicyPicker.tests.tsx

@@ -158,9 +158,10 @@ describe("PolicyPicker", () => {
   });
 
   describe("Patch badge", () => {
-    // Unique search values per test sidestep React Query cache pollution
-    // from earlier empty-state tests, which registered an empty result
-    // under queryKey ["commandPalettePolicies", ..., "<search>"].
+    // Each test uses its own QueryClient (via renderPickerInCommand ->
+    // createCustomRenderer) so results from earlier tests registered under
+    // queryKey ["commandPalettePolicies", ...] cannot leak into these
+    // assertions.
     it("renders the Patch badge when policy.type === 'patch'", async () => {
       mockedGlobal.loadAllNew.mockResolvedValue({
         policies: [

frontend/components/InfoBanner/InfoBanner.tsx

@@ -20,7 +20,8 @@ export interface IInfoBannerProps {
   cta?: JSX.Element;
   /** closable and link are mutually exclusive */
   closable?: boolean;
-  icon?: IconNames; // TODO: This is unused but several banners have icons within children that can be refactored to use this for consistent styling
+  /** Renders an icon at the start of the banner message */
+  icon?: IconNames;
 }
 
 const InfoBanner = ({
@@ -46,6 +47,9 @@ const InfoBanner = ({
 
   const content = (
     <>
+      {icon && (
+        <Icon name={icon} className={`${baseClass}__icon-image`} />
+      )}
       <div className={`${baseClass}__info`}>{children}</div>
 
       {(cta || closable) && (

frontend/components/List/List.tests.tsx

@@ -90,7 +90,7 @@ describe("List", () => {
       { customId: "beta", name: "Beta" },
     ];
 
-    const { container } = render(
+    const { container, rerender } = render(
       <List<ICustomItem, "customId">
         data={data}
         idKey="customId"
@@ -107,7 +107,31 @@ describe("List", () => {
       li.textContent?.includes("Beta")
     );
 
-    expect(alphaLi?.getAttribute("key")).toBeNull(); // React doesn't expose "key" to the DOM
+    expect(alphaLi).toBeInTheDocument();
     expect(betaLi).toBeInTheDocument();
+
+    // Reordering data by customId should reuse the same DOM nodes when idKey
+    // is honored, since React matches elements by their key rather than
+    // position. This verifies idKey drives the keying, not array index.
+    const reorderedData: ICustomItem[] = [data[1], data[0]];
+
+    rerender(
+      <List<ICustomItem, "customId">
+        data={reorderedData}
+        idKey="customId"
+        renderItemRow={(item) => <span>{item.name}</span>}
+      />
+    );
+
+    const reorderedListItems = container.querySelectorAll("li.list__row");
+    const reorderedAlphaLi = Array.from(reorderedListItems).find((li) =>
+      li.textContent?.includes("Alpha")
+    );
+    const reorderedBetaLi = Array.from(reorderedListItems).find((li) =>
+      li.textContent?.includes("Beta")
+    );
+
+    expect(reorderedAlphaLi).toBe(alphaLi);
+    expect(reorderedBetaLi).toBe(betaLi);
   });
 });

frontend/components/MDM/AppleBMTermsMessage/AppleBMTermsMessage.tsx

@@ -13,7 +13,7 @@ const AppleBMTermsMessage = () => {
       cta={
         <CustomLink
           url="https://business.apple.com/" // TODO: maybe point to new /settings/integrations/mdm/abm
-          text="Go to AB"
+          text="Go to Apple Business"
           className={`${baseClass}__new-tab`}
           newTab
           variant="banner-link"
@@ -30,3 +30,4 @@ const AppleBMTermsMessage = () => {
 };
 
 export default AppleBMTermsMessage;
+

frontend/components/TableContainer/utilities/TableContainerUtils.ts

@@ -28,4 +28,3 @@ export const generateResultsCountText = (
   return `${resultsCount.toLocaleString()} ${name}`;
 };
 
-export default { generateResultsCountText };

frontend/components/TableContainer/utilities/config_utils.ts

@@ -24,7 +24,7 @@ export const getConditionalSelectHeaderCheckboxProps = ({
   );
   const indeterminate =
     !allSelectableRowsSelected &&
-    headerProps.rows.some((row) => row.isSelected);
+    headerProps.rows.filter(checkIfRowIsSelectable).some((row) => row.isSelected);
 
   const onChange = () => {
     if (checkIfAllSelectableRowsSelected(headerProps.rows)) {