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

android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt

@@ -1,5 +1,6 @@
 package com.fleetdm.agent.scep
 
+import android.util.Log
 import com.fleetdm.agent.GetCertificateTemplateResponse
 import org.bouncycastle.asn1.DERPrintableString
 import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers
@@ -40,6 +41,7 @@ class ScepClientImpl : ScepClient {
         // Sending null causes jScep to omit the parameter; the server returns its default CA.
         private val SCEP_PROFILE: String? = null
         private const val SELF_SIGNED_CERT_VALIDITY_DAYS = 100L
+        private const val TAG = "ScepClientImpl"
 
         init {
             // Ensure BouncyCastle provider is loaded
@@ -75,19 +77,25 @@ class ScepClientImpl : ScepClient {
                 throw ScepNetworkException("Invalid SCEP URL: $scepUrl", e)
             }
 
-            // OptimisticCertificateVerifier is used intentionally because:
-            // 1. SCEP URL is provided by the authenticated MDM server
-            // 2. Challenge password authenticates the enrollment request
-            // 3. Enterprise SCEP servers often use internal CAs not in system trust stores
-            // 4. The enrolled certificate itself is validated when used
+            // NOTE: OptimisticCertificateVerifier accepts any server certificate presented
+            // during enrollment without validation. This is a known weakness: if scepUrl or
+            // DNS resolution is ever manipulated, the client could complete enrollment
+            // against an attacker-controlled CA and leak the challenge password embedded in
+            // the CSR. A proper fix requires pinning against a known certificate/fingerprint
+            // supplied by the MDM server when available. Until that plumbing exists, we keep
+            // OptimisticCertificateVerifier as a fallback but this should be revisited.
             val verifier = OptimisticCertificateVerifier()
             val client = Client(server, verifier)
 
             // Step 5: Build Certificate Signing Request (CSR)
+            val challenge = config.scepChallenge
+            if (challenge.isNullOrEmpty()) {
+                throw ScepCsrException("SCEP challenge password is missing; refusing to enroll without it")
+            }
             val csr = buildCsr(
                 entity,
                 keyPair,
-                config.scepChallenge ?: "",
+                challenge,
                 config.signatureAlgorithm,
                 config.subjectAlternativeName,
             )
@@ -139,10 +147,10 @@ class ScepClientImpl : ScepClient {
                 }
             }
         } catch (e: ScepException) {
-            // Re-throw ScepException as-is (Log.e removed to avoid test failures)
+            Log.e(TAG, "SCEP enrollment failed: ${e.message}", e)
             throw e
         } catch (e: Exception) {
-            // Wrap unexpected exceptions in ScepException (Log.e removed to avoid test failures)
+            Log.e(TAG, "Unexpected SCEP enrollment error: ${e.message}", e)
             throw ScepException("Unexpected SCEP enrollment error: ${e.message}", e)
         }
     }

client/base_client.go

@@ -100,8 +100,6 @@ func (bc *BaseClient) ParseResponse(verb, path string, response *http.Response,
 		}
 	}
 
-	bc.SetServerCapabilities(response)
-
 	return nil
 }
 
@@ -286,7 +284,11 @@ func (pr *progressReader) Read(p []byte) (int, error) {
 
 // DoHTTPRequest performs an HTTP request using the underlying HTTP client.
 func (bc *BaseClient) DoHTTPRequest(req *http.Request) (*http.Response, error) {
-	return bc.HTTP.Do(req)
+	resp, err := bc.HTTP.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("performing http request: %w", err)
+	}
+	return resp, nil
 }
 
 // GetRawHTTPClient returns the underlying HTTP client for type assertions (e.g., idle connection cleanup).

cmd/osquery-perf/softwaredb/softwaredb.go

@@ -9,6 +9,7 @@ import (
 	"math/rand/v2"
 	"os"
 	"strings"
+	"sync"
 
 	_ "github.com/mattn/go-sqlite3"
 )
@@ -44,14 +45,17 @@ var (
 		"ipados_apps":       "ipados_apps",
 		"jetbrains_plugins": "jetbrains_plugins",
 	}
-	vendorPool = make(map[string]string) // populated during load
+	vendorPool   = make(map[string]string) // populated during load
+	vendorPoolMu sync.Mutex                // guards vendorPool
 )
 
 // internString returns an interned version of s from the vendor pool, reducing memory usage
 func internString(s string) string {
 	if s == "" {
 		return ""
 	}
+	vendorPoolMu.Lock()
+	defer vendorPoolMu.Unlock()
 	if interned, ok := vendorPool[s]; ok {
 		return interned
 	}
@@ -318,6 +322,8 @@ func LoadFromDatabase(dbPath string) (*DB, error) {
 	var count int
 	err = db.QueryRow("SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='software'").Scan(&count)
 	if err != nil || count == 0 {
+		db.Close()
+		os.Remove(dbPath) // Clean up partial/corrupt database so next run can regenerate it
 		return nil, errors.New("database exists but 'software' table not found\n\nPlease initialize the database:\n  cd cmd/osquery-perf/software-library\n  sqlite3 software.db < software.sql")
 	}
 
@@ -381,19 +387,37 @@ func generateDatabaseFromSQL(dbPath, sqlPath string) error {
 	return nil
 }
 
+// sourcePlaceholders returns a comma-separated list of "?" placeholders, one per source,
+// for use with parameterized queries.
+func sourcePlaceholders(sources []string) string {
+	placeholders := make([]string, len(sources))
+	for i := range sources {
+		placeholders[i] = "?"
+	}
+	return strings.Join(placeholders, ", ")
+}
+
+// sourcesToArgs converts a slice of source strings to a slice of interface{} for use as
+// query arguments.
+func sourcesToArgs(sources []string) []interface{} {
+	args := make([]interface{}, len(sources))
+	for i, s := range sources {
+		args[i] = s
+	}
+	return args
+}
+
 // loadDarwinSoftware loads all macOS/iOS software from the database for the given sources
 func loadDarwinSoftware(db *sql.DB, sources []string) ([]DarwinSoftware, error) {
-	sourceList := "'" + strings.Join(sources, "', '") + "'"
-	// nolint:gosec // sources are hardcoded, not user input
 	query := fmt.Sprintf(`
 		SELECT name, version, source, bundle_identifier, vendor, extension_id, extension_for
 		FROM software
 		WHERE source IN (%s)
 		ORDER BY RANDOM()
 		LIMIT %d
-	`, sourceList, MaxSoftwarePerPlatform)
+	`, sourcePlaceholders(sources), MaxSoftwarePerPlatform)
 
-	rows, err := db.Query(query)
+	rows, err := db.Query(query, sourcesToArgs(sources)...)
 	if err != nil {
 		return nil, fmt.Errorf("querying darwin software: %w", err)
 	}
@@ -439,17 +463,15 @@ func loadDarwinSoftware(db *sql.DB, sources []string) ([]DarwinSoftware, error)
 
 // loadWindowsSoftware loads all Windows software from the database for the given sources
 func loadWindowsSoftware(db *sql.DB, sources []string) ([]WindowsSoftware, error) {
-	sourceList := "'" + strings.Join(sources, "', '") + "'"
-	// nolint:gosec // sources are hardcoded, not user input
 	query := fmt.Sprintf(`
 		SELECT name, version, source, vendor, upgrade_code, extension_id, extension_for
 		FROM software
 		WHERE source IN (%s)
 		ORDER BY RANDOM()
 		LIMIT %d
-	`, sourceList, MaxSoftwarePerPlatform)
+	`, sourcePlaceholders(sources), MaxSoftwarePerPlatform)
 
-	rows, err := db.Query(query)
+	rows, err := db.Query(query, sourcesToArgs(sources)...)
 	if err != nil {
 		return nil, fmt.Errorf("querying windows software: %w", err)
 	}
@@ -495,17 +517,15 @@ func loadWindowsSoftware(db *sql.DB, sources []string) ([]WindowsSoftware, error
 
 // loadUbuntuSoftware loads all Ubuntu/Linux software from the database for the given sources
 func loadUbuntuSoftware(db *sql.DB, sources []string) ([]UbuntuSoftware, error) {
-	sourceList := "'" + strings.Join(sources, "', '") + "'"
-	// nolint:gosec // sources are hardcoded, not user input
 	query := fmt.Sprintf(`
 		SELECT name, version, source, vendor, arch, release, extension_id, extension_for
 		FROM software
 		WHERE source IN (%s)
 		ORDER BY RANDOM()
 		LIMIT %d
-	`, sourceList, MaxSoftwarePerPlatform)
+	`, sourcePlaceholders(sources), MaxSoftwarePerPlatform)
 
-	rows, err := db.Query(query)
+	rows, err := db.Query(query, sourcesToArgs(sources)...)
 	if err != nil {
 		return nil, fmt.Errorf("querying ubuntu software: %w", err)
 	}

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

@@ -16,6 +16,9 @@ class FleetClient
     # [1]: https://www.puppet.com/docs/puppet/8/server/config_file_puppetserver.html
     @instance_mutex = Mutex.new
 
+    # Maximum time, in seconds, that a cached entry is considered valid.
+    CACHE_TTL = 60
+
     def self.instance
       return @instance if @instance
       @instance_mutex.synchronize do
@@ -138,9 +141,12 @@ def req(method: :get, path: '', body: nil, headers: {}, cached: false, environme
 
       if cached
         @cache_mutex.synchronize do
-          unless @cache[path].nil?
-            return @cache[path]
+          entry = @cache[path]
+          if !entry.nil? && (Time.now - entry[:cached_at]) < CACHE_TTL
+            return entry[:value]
           end
+
+          @cache.delete(path) unless entry.nil?
         end
       end
 
@@ -171,11 +177,11 @@ def req(method: :get, path: '', body: nil, headers: {}, cached: false, environme
 
         if cached && out['error'].empty?
           @cache_mutex.synchronize do
-            @cache[path] = out
+            @cache[path] = { value: out, cached_at: Time.now }
           end
         end
       rescue => e
-        out['error'] = e
+        out['error'] = e.message
       end
 
       out
@@ -195,8 +201,8 @@ def parse_response(response)
       if (400...600).cover?(response.code.to_i)
         message = 'server returned a non-ok status code without an error'
 
-        if response.body
-          body = JSON.parse(response.body)
+        if out['body'].is_a?(Hash) && !out['body'].empty?
+          body = out['body']
           message = body['message']
 
           unless body['errors'].nil?

ee/vulnerability-dashboard/scripts/update-critical-software.js

@@ -91,8 +91,8 @@ module.exports = {
       if(osVersionNamesByHostCount[os.name] === undefined) {
         osVersionsToReport.push(osToReport);
       } else if(osVersionNamesByHostCount[os.name] !== os.hosts_count) {
+        osVersionsToUpdate.push(osToReport);
       }
-      osVersionsToUpdate.push(osToReport);
     }
 
     let nativeQueryToFindOperatingSystemsWithNoHosts =
@@ -239,7 +239,7 @@ module.exports = {
           if(!hostsOperatingSystem){
             hostsOperatingSystem = _.find(allOsRecords, {'fullName': 'Microsoft '+host.os_version});
             if(!hostsOperatingSystem){
-              throw new Error(`Host's operating system not found in Operating System records`);
+              throw new Error(`Host's operating system (${host.os_version}) not found in Operating System records for host ${host.id} (${host.display_name})`);
             }
           }
           byHostFleetApidsSeenInLatestCriticalSoftwareScan[host.id] = {
@@ -357,3 +357,4 @@ module.exports = {
 
 
 };
+

frontend/components/EnrollSecrets/SecretEditorModal/SecretEditorModal.tsx

@@ -21,13 +21,11 @@ const baseClass = "secret-editor-modal";
 
 const randomSecretGenerator = () => {
   const randomChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
-  let result = "";
-  for (let i = 0; i < 32; i += 1) {
-    result += randomChars.charAt(
-      Math.floor(Math.random() * randomChars.length)
-    );
-  }
-  return result;
+  const bytes = new Uint32Array(32);
+  window.crypto.getRandomValues(bytes);
+  return Array.from(bytes, (b) => randomChars[b % randomChars.length]).join(
+    ""
+  );
 };
 
 const SecretEditorModal = ({
@@ -45,14 +43,15 @@ const SecretEditorModal = ({
   const [errors, setErrors] = useState<{ [key: string]: string }>({});
 
   const renderTeam = () => {
-    if (typeof selectedTeam === "string") {
-      selectedTeam = parseInt(selectedTeam, 10);
-    }
+    const parsedSelectedTeam =
+      typeof selectedTeam === "string"
+        ? parseInt(selectedTeam, 10)
+        : selectedTeam;
 
-    if (selectedTeam === 0) {
+    if (parsedSelectedTeam === 0) {
       return { name: "Unassigned" };
     }
-    return teams.find((team) => team.id === selectedTeam);
+    return teams.find((team) => team.id === parsedSelectedTeam);
   };
 
   const onSecretChange = (value: string) => {

frontend/components/QueryFrequencyIndicator/QueryFrequencyIndicator.tsx

@@ -24,7 +24,8 @@ const QueryFrequencyIndicator = ({
   const frequencyClassName = classnames(
     "query-frequency-indicator",
     `query-frequency-indicator--${classTag}`,
-    `frequency--${classTag}`
+    `frequency--${classTag}`,
+    { grey: frequency === 0 && !checked }
   );
   const readableQueryFrequency = () => {
     switch (frequency) {
@@ -53,10 +54,7 @@ const QueryFrequencyIndicator = ({
   };
 
   return (
-    <div
-      className={`${frequencyClassName}
-        ${frequency === 0 && !checked && "grey"}`}
-    >
+    <div className={frequencyClassName}>
       {frequencyIcon()}
       {readableQueryFrequency()}
     </div>

frontend/components/forms/fields/InputField/InputField.tsx

@@ -9,6 +9,8 @@ import FormField from "components/forms/FormField";
 import Button from "components/buttons/Button";
 import Icon from "components/Icon";
 
+import { InputFieldOnChange } from "interfaces/form_field";
+
 const baseClass = "input-field";
 
 export interface IInputFieldProps {
@@ -27,7 +29,7 @@ export interface IInputFieldProps {
    * parseTarget is true. See IInputFieldParseTarget and InputFieldOnChange
    * in interfaces/form_field.ts for caller-side typing helpers.
    */
-  onChange?: (value: any) => void;
+  onChange?: InputFieldOnChange;
   onBlur?: (
     evt: React.FocusEvent<HTMLInputElement | HTMLTextAreaElement>
   ) => void;
@@ -126,7 +128,7 @@ const InputField = ({
   const onClickCopy = useCallback(
     (e: React.MouseEvent) => {
       e.preventDefault();
-      stringToClipboard(value).then(() => {
+      stringToClipboard(String(value ?? "")).then(() => {
         setCopied(true);
         setTimeout(() => {
           setCopied(false);
@@ -163,26 +165,30 @@ const InputField = ({
                 Copied!
               </span>
             )}
-            <button
-              type="button"
+            <Button
+              variant="icon"
               className={`${baseClass}__action-button`}
               onClick={onClickCopy}
+              size="small"
+              iconStroke
               aria-label="Copy to clipboard"
             >
               <Icon name="copy" />
-            </button>
+            </Button>
           </div>
         )}
         {enableShowSecret && (
-          <button
-            type="button"
+          <Button
+            variant="icon"
             className={`${baseClass}__action-button`}
             onClick={onToggleSecret}
+            size="small"
+            iconStroke
             aria-label={showSecret ? "Hide secret" : "Show secret"}
             aria-pressed={showSecret}
           >
             <Icon name="eye" />
-          </button>
+          </Button>
         )}
       </div>
     );