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

cmd/maintained-apps/validate/windows.go

@@ -5,11 +5,13 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
@@ -20,6 +22,18 @@ import (
 
 var preInstalled = []string{}
 
+// sqlIdentifierAllowlist restricts identifiers interpolated into SQL LIKE
+// queries to a safe, narrow character set as defense-in-depth on top of
+// validateSqlInput's blocklist-style checks.
+var sqlIdentifierAllowlist = regexp.MustCompile(`^[a-zA-Z0-9 ._:()/\\+-]*$`)
+
+func validateSqlIdentifierStrict(input string) error {
+	if !sqlIdentifierAllowlist.MatchString(input) {
+		return fmt.Errorf("contains disallowed characters")
+	}
+	return nil
+}
+
 func postApplicationInstall(_ context.Context, _ *slog.Logger, _ string) error {
 	return nil
 }
@@ -46,9 +60,15 @@ func appExists(ctx context.Context, logger *slog.Logger, appName, uniqueIdentifi
 	if err := validateSqlInput(appName); err != nil {
 		return false, fmt.Errorf("Invalid character found in appName: '%w'. Not executing query...", err)
 	}
+	if err := validateSqlIdentifierStrict(appName); err != nil {
+		return false, fmt.Errorf("Invalid character found in appName: '%w'. Not executing query...", err)
+	}
 	if err := validateSqlInput(appPath); err != nil {
 		return false, fmt.Errorf("Invalid character found in appPath: '%w'. Not executing query...", err)
 	}
+	if err := validateSqlIdentifierStrict(appPath); err != nil {
+		return false, fmt.Errorf("Invalid character found in appPath: '%w'. Not executing query...", err)
+	}
 
 	logger.InfoContext(ctx, fmt.Sprintf("Looking for app: %s, version: %s", appName, appVersion))
 	query := `
@@ -65,6 +85,9 @@ func appExists(ctx context.Context, logger *slog.Logger, appName, uniqueIdentifi
 		if err := validateSqlInput(uniqueIdentifier); err != nil {
 			return false, fmt.Errorf("Invalid character found in uniqueIdentifier: '%w'. Not executing query...", err)
 		}
+		if err := validateSqlIdentifierStrict(uniqueIdentifier); err != nil {
+			return false, fmt.Errorf("Invalid character found in uniqueIdentifier: '%w'. Not executing query...", err)
+		}
 		query += `	OR LOWER(name) LIKE LOWER('%` + uniqueIdentifier + `%')`
 	}
 	if appPath != "" {
@@ -294,7 +317,8 @@ func executeScript(cfg *Config, scriptContents string) (string, error) {
 	// (pkgscripts.MaxHostSoftwareInstallExecutionTime); 10 minutes is a
 	// reasonable validator cap that covers large-payload installers without
 	// letting a hung script run indefinitely.
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
+	const scriptTimeout = 10 * time.Minute
+	ctx, cancel := context.WithTimeout(context.Background(), scriptTimeout)
 	defer cancel()
 
 	// Use custom execution with non-interactive flags for Windows
@@ -320,6 +344,9 @@ func executeScript(cfg *Config, scriptContents string) (string, error) {
 --------------------`, string(output))
 
 	if err != nil {
+		if errors.Is(ctx.Err(), context.DeadlineExceeded) {
+			return result, fmt.Errorf("script execution exceeded the %s validator timeout (this is the validator's own cap, not the script's logic): %w", scriptTimeout, err)
+		}
 		return result, err
 	}
 	if exitCode != 0 {

cmd/osquery-perf/software-library/tools/import-data/main.go

@@ -12,7 +12,7 @@ import (
 	"regexp"
 	"strings"
 
-	_ "github.com/mattn/go-sqlite3"
+	"github.com/mattn/go-sqlite3"
 )
 
 // SoftwareEntry represents a software item from server data
@@ -46,7 +46,7 @@ var knownPublicSoftware = []string{
 // - 172.16.0.0/12 (172.16.x.x - 172.31.x.x)
 // - 192.168.0.0/16 (192.168.x.x)
 // - 127.0.0.0/8 (127.x.x.x - loopback)
-var privateIPRegex = regexp.MustCompile(`^(10(\.\d{1,3}){3}|127(\.\d{1,3}){3}|192\.168(\.\d{1,3}){2}|172\.(1[6-9]|2[0-9]|3[0-1])(\.\d{1,3}){2})`)
+var privateIPRegex = regexp.MustCompile(`^(10(\.\d{1,3}){3}|127(\.\d{1,3}){3}|192\.168(\.\d{1,3}){2}|172\.(1[6-9]|2[0-9]|3[0-1])(\.\d{1,3}){2})$`)
 
 type ImportStats struct {
 	Total             int
@@ -246,7 +246,8 @@ func (imp *Importer) importEntry(entry SoftwareEntry) {
 	if !imp.dryRun {
 		err := imp.insertSoftware(entry)
 		if err != nil {
-			if strings.Contains(err.Error(), "UNIQUE constraint failed") {
+			var sqliteErr sqlite3.Error
+			if errors.As(err, &sqliteErr) && sqliteErr.ExtendedCode == sqlite3.ErrConstraintUnique {
 				imp.stats.Duplicates++
 				if imp.verbose {
 					fmt.Printf("  ⏭️  Duplicate: %s v%s\n", entry.Name, entry.Version)

ee/maintained-apps/inputs/homebrew/scripts/expressvpn-install.sh

@@ -4,6 +4,11 @@
 APPDIR="/Applications/"
 TMPDIR=$(mktemp -d)
 
+cleanup() {
+  rm -rf "$TMPDIR"
+}
+trap cleanup EXIT
+
 # functions
 
 quit_application() {
@@ -16,8 +21,8 @@ quit_application() {
   fi
 
   local console_user
-  console_user=$(stat -f "%Su" /dev/console)
-  if [[ $EUID -eq 0 && "$console_user" == "root" ]]; then
+  console_user=$(stat -f "%Su" /dev/console 2>/dev/null || echo "")
+  if [[ -z "$console_user" || "$console_user" == "root" || "$console_user" == "loginwindow" ]]; then
     echo "Not logged into a non-root GUI; skipping quitting application ID '$bundle_id'."
     return
   fi
@@ -83,3 +88,4 @@ if [ $EXIT_CODE -ne 0 ]; then
   exit $EXIT_CODE
 fi
 
+

ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb

@@ -175,7 +175,7 @@ def req(method: :get, path: '', body: nil, headers: {}, cached: false, environme
           end
         end
       rescue => e
-        out['error'] = e
+        out['error'] = e.message
       end
 
       out

frontend/components/ActivityDetails/InstallDetails/SoftwareInstallDetailsModal/SoftwareInstallDetailsModal.tsx

@@ -365,7 +365,7 @@ export const SoftwareInstallDetailsModal = ({
   const overrideFailedMessageWithInstalledMessage =
     canOverrideFailureWithInstalled &&
     ["failed_install", "failed_uninstall"].includes(
-      swInstallResult?.status || "" || ""
+      swInstallResult?.status || ""
     );
 
   // Hide version section from pending installs or failures that aren't overridden to installed (4.82 #31663)

frontend/hooks/useTeamIdParam.ts

@@ -76,14 +76,28 @@ const rebuildQueryStringWithTeamId = (
     parts.splice(pageIndex, 1, "page=0");
   }
 
-  // Backward compat: rewrite legacy team_id= to fleet_id=
-  const legacyIndex = parts.findIndex((p) => p.startsWith("team_id="));
-  if (legacyIndex !== -1) {
-    parts.splice(
-      legacyIndex,
-      1,
-      parts[legacyIndex].replace("team_id=", "fleet_id=")
-    );
+  // Backward compat: rewrite legacy team_id= to fleet_id=, but only if there
+  // isn't already a fleet_id= param present. If both are present, the
+  // fleet_id= param takes precedence and the stale legacy param is simply
+  // dropped below in the main fleet_id handling, avoiding a second splice
+  // against stale indices.
+  const hasFleetIdParam = parts.some((p) => p.startsWith("fleet_id="));
+  if (!hasFleetIdParam) {
+    const legacyIndex = parts.findIndex((p) => p.startsWith("team_id="));
+    if (legacyIndex !== -1) {
+      parts.splice(
+        legacyIndex,
+        1,
+        parts[legacyIndex].replace("team_id=", "fleet_id=")
+      );
+    }
+  } else {
+    // Drop any stray legacy team_id= params since fleet_id= is authoritative
+    for (let i = parts.length - 1; i >= 0; i -= 1) {
+      if (parts[i].startsWith("team_id=")) {
+        parts.splice(i, 1);
+      }
+    }
   }
 
   const teamIndex = parts.findIndex((p) => p.startsWith("fleet_id="));
@@ -186,13 +200,25 @@ const getUserTeams = ({
     : filterUserTeamsByRole(currentUser.teams, permittedAccessByTeamRole);
 };
 
+// Name of the built-in "Workstations" fleet, as seeded/created elsewhere in
+// the app (e.g. wherever the default "Workstations" team is provisioned).
+// Kept as a single documented constant here since this hook is the only
+// consumer of the match today; if a shared location for team-name constants
+// is introduced, this should be moved there to avoid drift.
+const WORKSTATIONS_TEAM_NAME = "workstations";
+// U+1F4BB PERSONAL COMPUTER emoji, optionally followed by a variation
+// selector (U+FE0F), as may be produced by different emoji input methods.
+const WORKSTATIONS_EMOJI_PREFIX_PATTERN = /^\u{1F4BB}\uFE0F?\s*/u;
+
 // Prefer a fleet named "Workstations" (with or without emoji prefix),
 // otherwise fall back to the fleet with the lowest ID.
 export const preferredOrLowestIdFleet = (fleets: ITeamSummary[]) => {
-  const name = "workstations";
   const workstations = fleets.find((t) => {
-    const lower = t.name.toLowerCase();
-    return lower === name || lower === `\u{1F4BB} ${name}`;
+    const lower = t.name
+      .toLowerCase()
+      .replace(WORKSTATIONS_EMOJI_PREFIX_PATTERN, "")
+      .trim();
+    return lower === WORKSTATIONS_TEAM_NAME;
   });
   return workstations ?? sortBy(fleets, (t) => t.id)[0];
 };
@@ -462,11 +488,17 @@ export const useTeamIdParam = ({
   if (hasLegacyTeamIdParam) {
     // Backward compat: redirect legacy ?team_id= URLs to ?fleet_id=
     // Skip other reconciliation to avoid a second redirect overwriting this one.
-    router.replace(
-      pathname
-        .concat(search.replace(/\bteam_id=/g, "fleet_id="))
-        .concat(hash || "")
-    );
+    // If a fleet_id= param is also already present in the URL, treat it as
+    // authoritative and simply drop the stale legacy team_id= param instead
+    // of overwriting fleet_id=, so this path can never disagree with
+    // rebuildQueryStringWithTeamId's own legacy-param handling.
+    const hasFleetIdParam = /(?:^|[?&])fleet_id=/.test(search);
+    const newSearch = hasFleetIdParam
+      ? search.replace(/([?&])team_id=[^&]*(&)?/g, (_match, lead, trail) =>
+          trail ? lead : lead === "?" ? "?" : ""
+        )
+      : search.replace(/\bteam_id=/g, "fleet_id=");
+    router.replace(pathname.concat(newSearch).concat(hash || ""));
   } else if (isFreeTier) {
     // free tier should never have fleet_id param, so change to "All teams"
     if (query.fleet_id) {

frontend/pages/DashboardPage/cards/Software/Software.tsx

@@ -97,7 +97,7 @@ const Software = ({
                   data={(isSoftwareEnabled && software?.software) || []}
                   isLoading={isSoftwareFetching}
                   pageIndex={softwarePageIndex}
-                  defaultSortHeader={SOFTWARE_DEFAULT_SORT_DIRECTION}
+                  defaultSortHeader={SOFTWARE_DEFAULT_SORT_HEADER}
                   defaultSortDirection={SOFTWARE_DEFAULT_SORT_DIRECTION}
                   resultsTitle="software"
                   emptyComponent={() => <EmptySoftwareTable />}

frontend/pages/ManageControlsPage/SetupExperience/cards/BootstrapPackage/BootstrapPackage.tsx

@@ -141,13 +141,34 @@ const BootstrapPackage = ({
   const onDelete = async () => {
     try {
       await mdmAPI.deleteBootstrapPackage(currentTeamId);
+    } catch (error) {
+      console.error("Failed to delete bootstrap package:", error);
+      renderFlash("error", "Couldn't delete. Please try again.");
+      setShowDeleteBootstrapPackageModal(false);
+      refretchBootstrapMetadata();
+      if (currentTeamId !== API_NO_TEAM_ID) {
+        refetchTeamConfig();
+      } else {
+        refetchGlobalConfig();
+      }
+      return;
+    }
+
+    try {
       await mdmAPI.updateSetupExperienceSettings({
         fleet_id: currentTeamId,
         macos_manual_agent_install: false,
       });
       renderFlash("success", "Successfully deleted.");
-    } catch {
-      renderFlash("error", "Couldn't delete. Please try again.");
+    } catch (error) {
+      console.error(
+        "Bootstrap package deleted, but failed to update setup experience settings:",
+        error
+      );
+      renderFlash(
+        "error",
+        "Bootstrap package deleted, but couldn't update settings. Please try again."
+      );
     } finally {
       setShowDeleteBootstrapPackageModal(false);
       refretchBootstrapMetadata();

frontend/pages/ManageControlsPage/SetupExperience/cards/InstallSoftware/components/InstallSoftwareForm/InstallSoftwareForm.tsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useContext, useState, useMemo } from "react";
+import React, { useCallback, useContext, useState, useMemo, useEffect } from "react";
 import { isEqual } from "lodash";
 import { InjectedRouter } from "react-router";
 
@@ -124,6 +124,14 @@ const InstallSoftwareForm = ({
     initialSelectedSoftware
   );
 
+  // Keep local selection in sync with the latest server-side data whenever
+  // softwareTitles changes (e.g. after a refetch following a save), so that
+  // stale local selections don't diverge from the freshly fetched truth.
+  useEffect(() => {
+    setSelectedSoftwareIds(initialSelectedSoftware);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [softwareTitles]);
+
   const installSoftwareDuringSetupCount = selectedSoftwareIds.length;
 
   const onChangeSoftwareSelect = useCallback((select: boolean, id: number) => {
@@ -158,6 +166,7 @@ const InstallSoftwareForm = ({
 
     const errorNotifications: INotification[] = [];
     let hadSuccess = false;
+    let softwareUpdateFailed = false;
 
     // 1. Software selection update
     if (shouldUpdateSoftware) {
@@ -170,6 +179,7 @@ const InstallSoftwareForm = ({
         hadSuccess = true;
         // Still let parent refetch even if the macOS call later fails
       } catch (e) {
+        softwareUpdateFailed = true;
         errorNotifications.push({
           id: "update-software",
           alertType: "error",
@@ -216,7 +226,14 @@ const InstallSoftwareForm = ({
       renderFlash("success", "Successfully updated.");
     }
 
-    refetchSoftwareTitles();
+    // Only refetch (which resyncs local selection state from server data via
+    // the effect above) if the software-selection save didn't fail. If it
+    // failed, refetching would discard the user's unsaved local selection
+    // and replace it with stale server data without any indication that the
+    // save didn't happen.
+    if (!softwareUpdateFailed) {
+      refetchSoftwareTitles();
+    }
     setIsSaving(false);
   };