From 4e5f7856c5360b75aee8ebd89cf5741309f0859a Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:00 +0000 Subject: [PATCH 01/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/mdm/nanomdm/storage/file/certauth.go | 23 +++++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/server/mdm/nanomdm/storage/file/certauth.go b/server/mdm/nanomdm/storage/file/certauth.go index 6cd4ddf7699..2d204b1482f 100644 --- a/server/mdm/nanomdm/storage/file/certauth.go +++ b/server/mdm/nanomdm/storage/file/certauth.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "errors" + "fmt" "os" "path" "strings" @@ -35,7 +36,8 @@ func (s *FileStorage) HasCertHash(r *mdm.Request, hash string) (bool, error) { defer f.Close() scanner := bufio.NewScanner(f) for scanner.Scan() { - if strings.Contains(scanner.Text(), hash) { + parts := strings.SplitN(scanner.Text(), ",", 2) + if len(parts) == 2 && parts[1] == hash { return true, nil } } @@ -61,14 +63,17 @@ func (s *FileStorage) AssociateCertHash(r *mdm.Request, hash string, _ time.Time 0644, ) if err != nil { - return err + return fmt.Errorf("opening cert auth associations file: %w", err) } defer f.Close() if _, err := f.WriteString(r.ID + "," + hash + "\n"); err != nil { - return err + return fmt.Errorf("writing cert auth association: %w", err) } e := s.newEnrollment(r.ID) - return e.writeFile(CertAuthFilename, []byte(hash)) + if err := e.writeFile(CertAuthFilename, []byte(hash)); err != nil { + return fmt.Errorf("writing cert auth file: %w", err) + } + return nil } func (s *FileStorage) EnrollmentFromHash(_ context.Context, hash string) (string, error) { @@ -80,11 +85,11 @@ func (s *FileStorage) EnrollmentFromHash(_ context.Context, hash string) (string scanner := bufio.NewScanner(f) for scanner.Scan() { text := scanner.Text() - if strings.Contains(text, hash) { - split := strings.Split(text, ",") - if len(split) < 2 { - return "", errors.New("hash and enrollment id not present on line") - } + split := strings.SplitN(text, ",", 2) + if len(split) < 2 { + continue + } + if split[1] == hash { return split[0], nil } } From 1e2f49f909c826f9076f3f92a7397fdc34dd2a28 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:02 +0000 Subject: [PATCH 02/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../fleetdm/lib/puppet/util/fleet_client.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb b/ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb index 1eb10295433..98bcab86949 100644 --- a/ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb +++ b/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? From 4270e42a933caa7ae5410f733c7f6505488d12e6 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:03 +0000 Subject: [PATCH 03/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../scripts/update-critical-software.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ee/vulnerability-dashboard/scripts/update-critical-software.js b/ee/vulnerability-dashboard/scripts/update-critical-software.js index 29c0cc462b3..54855ec2b3b 100644 --- a/ee/vulnerability-dashboard/scripts/update-critical-software.js +++ b/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 = { }; + From 9fd8f5c3f5879a590caab70bf0582004a3a1d468 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:04 +0000 Subject: [PATCH 04/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../forms/fields/InputField/InputField.tsx | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/frontend/components/forms/fields/InputField/InputField.tsx b/frontend/components/forms/fields/InputField/InputField.tsx index a6b135f8da4..6a2c7bab5d6 100644 --- a/frontend/components/forms/fields/InputField/InputField.tsx +++ b/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 ) => 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! )} - + )} {enableShowSecret && ( - + )} ); From f36be74937b4f24103959cee3b7c0920d6c0aae3 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:05 +0000 Subject: [PATCH 05/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../ManageUsersPage/EditUserPage/EditUserPage.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx b/frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx index 504de6f3962..1e759b19022 100644 --- a/frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx +++ b/frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx @@ -96,7 +96,8 @@ const EditUserPage = ({ router, params, location }: IEditUserPageProps) => { router.push(PATHS.ADMIN_USERS); }) .catch((inviteErrors: { data: IApiError }) => { - if (inviteErrors.data.errors[0].reason.includes("already exists")) { + const reason = inviteErrors?.data?.errors?.[0]?.reason ?? ""; + if (reason.includes("already exists")) { setFormErrors({ email: "A user with this email address already exists", }); @@ -145,13 +146,12 @@ const EditUserPage = ({ router, params, location }: IEditUserPageProps) => { router.push(PATHS.ADMIN_USERS); }) .catch((userErrors: { data: IApiError }) => { - if (userErrors.data.errors[0].reason.includes("already exists")) { + const reason = userErrors?.data?.errors?.[0]?.reason ?? ""; + if (reason.includes("already exists")) { setFormErrors({ email: "A user with this email address already exists", }); - } else if ( - userErrors.data.errors[0].reason.includes("required criteria") - ) { + } else if (reason.includes("required criteria")) { setFormErrors({ password: "Password must meet the criteria below", }); From c13b0cdf4f8b7c254cf77f22d36752cad72bd1fb Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:06 +0000 Subject: [PATCH 06/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- orbit/pkg/execuser/execuser_windows.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/orbit/pkg/execuser/execuser_windows.go b/orbit/pkg/execuser/execuser_windows.go index 215e295ae50..bdfc73b1b7f 100644 --- a/orbit/pkg/execuser/execuser_windows.go +++ b/orbit/pkg/execuser/execuser_windows.go @@ -29,6 +29,8 @@ var ( procDuplicateTokenEx *windows.LazyProc = modadvapi32.NewProc("DuplicateTokenEx") procCreateEnvironmentBlock *windows.LazyProc = moduserenv.NewProc("CreateEnvironmentBlock") procCreateProcessAsUser *windows.LazyProc = modadvapi32.NewProc("CreateProcessAsUserW") + procWTSFreeMemory *windows.LazyProc = modwtsapi32.NewProc("WTSFreeMemory") + procDestroyEnvironmentBlock *windows.LazyProc = moduserenv.NewProc("DestroyEnvironmentBlock") ) const ( @@ -171,11 +173,13 @@ func wtsEnumerateSessions() ([]*WTS_SESSION_INFO, error) { if returnCode, _, err := procWTSEnumerateSessionsW.Call(WTS_CURRENT_SERVER_HANDLE, 0, 1, uintptr(unsafe.Pointer(&sessionInformation)), uintptr(unsafe.Pointer(&sessionCount))); returnCode == 0 { return nil, fmt.Errorf("call native WTSEnumerateSessionsW: %s", err) } + defer procWTSFreeMemory.Call(uintptr(sessionInformation)) structSize := unsafe.Sizeof(WTS_SESSION_INFO{}) current := uintptr(sessionInformation) for i := 0; i < sessionCount; i++ { - sessionList = append(sessionList, (*WTS_SESSION_INFO)(unsafe.Pointer(current))) + sessionInfo := *(*WTS_SESSION_INFO)(unsafe.Pointer(current)) + sessionList = append(sessionList, &sessionInfo) current += structSize } @@ -228,10 +232,12 @@ func startProcessAsCurrentUser(appPath, cmdLine, workDir string) error { if userToken, err = duplicateUserTokenFromSessionID(sessionId); err != nil { return fmt.Errorf("get duplicate user token for current user session: %s", err) } + defer windows.CloseHandle(windows.Handle(userToken)) //nolint:errcheck if returnCode, _, err := procCreateEnvironmentBlock.Call(uintptr(unsafe.Pointer(&envInfo)), uintptr(userToken), 1); returnCode == 0 { return fmt.Errorf("create environment details for process: %s", err) } + defer procDestroyEnvironmentBlock.Call(uintptr(envInfo)) // TODO(lucas): Test out creation flags and startup info values. creationFlags := CREATE_UNICODE_ENVIRONMENT | CREATE_NEW_CONSOLE @@ -252,6 +258,9 @@ func startProcessAsCurrentUser(appPath, cmdLine, workDir string) error { return fmt.Errorf("create process as user: %s", err) } + windows.CloseHandle(processInfo.Process) //nolint:errcheck + windows.CloseHandle(processInfo.Thread) //nolint:errcheck + return nil } @@ -314,3 +323,4 @@ func setupSyncChannel(channelId string) error { return nil } + From e156a48d86f15bea70e7e7b065bb7049459f538e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:07 +0000 Subject: [PATCH 07/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- orbit/pkg/table/tcc_access/tcc_access.go | 34 ++++++++++++++++-------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/orbit/pkg/table/tcc_access/tcc_access.go b/orbit/pkg/table/tcc_access/tcc_access.go index 57f270bb789..30fbed364a6 100644 --- a/orbit/pkg/table/tcc_access/tcc_access.go +++ b/orbit/pkg/table/tcc_access/tcc_access.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "os/exec" + "strconv" "strings" "github.com/osquery/osquery-go/plugin/table" @@ -78,7 +79,7 @@ func Generate(ctx context.Context, queryContext table.QueryContext) ([]map[strin } return nil, err } - uRs, err := getTCCAccessRows(uid, tccPath) + uRs, err := getTCCAccessRows(ctx, uid, tccPath) if err != nil { return nil, err } @@ -96,7 +97,7 @@ func Generate(ctx context.Context, queryContext table.QueryContext) ([]map[strin } } if sysSatisfiesUidConstraints { - sRs, err := getTCCAccessRows("0", tccPathPrefix+tccPathSuffix) + sRs, err := getTCCAccessRows(ctx, "0", tccPathPrefix+tccPathSuffix) if err != nil { return nil, err } @@ -106,10 +107,10 @@ func Generate(ctx context.Context, queryContext table.QueryContext) ([]map[strin return rows, nil } -func getTCCAccessRows(uid, tccPath string) ([]map[string]string, error) { +func getTCCAccessRows(ctx context.Context, uid, tccPath string) ([]map[string]string, error) { // querying directly with sqlite3 avoids additional C compilation requirements that would be introduced by using // https://github.com/mattn/go-sqlite3 - cmd := exec.Command(sqlite3Path, tccPath, dbQuery) + cmd := exec.CommandContext(ctx, sqlite3Path, tccPath, dbQuery) var dbOut bytes.Buffer var stderr bytes.Buffer cmd.Stdout = &dbOut @@ -134,8 +135,11 @@ func parseTCCDbReadOutput(dbOut []byte) [][]string { if n == 0 { return nil } - // the end of the db response is "\n", making the final row "", which we want to omit - rawRows = rawRows[:n-1] + // the end of the db response is normally "\n", making the final row "", which we want to omit; + // only drop it if it is actually empty so we don't silently discard a real trailing row + if rawRows[n-1] == "" { + rawRows = rawRows[:n-1] + } parsedRows := make([][]string, 0, len(rawRows)) for _, rawRow := range rawRows { @@ -164,27 +168,35 @@ func buildTableRows(uid string, parsedRows [][]string) ([]map[string]string, err } func satisfiesConstraints(uid string, constraints []table.Constraint) (bool, error) { + uidNum, err := strconv.Atoi(uid) + if err != nil { + return false, fmt.Errorf("invalid uid %q: %w", uid, err) + } for _, constraint := range constraints { + exprNum, err := strconv.Atoi(constraint.Expression) + if err != nil { + return false, fmt.Errorf("invalid uid constraint expression %q: %w", constraint.Expression, err) + } // for each constraint on the column switch constraint.Operator { case table.OperatorEquals: - if constraint.Expression != uid { + if exprNum != uidNum { return false, nil } case table.OperatorGreaterThan: - if constraint.Expression >= uid { + if uidNum <= exprNum { return false, nil } case table.OperatorLessThan: - if constraint.Expression <= uid { + if uidNum >= exprNum { return false, nil } case table.OperatorGreaterThanOrEquals: - if constraint.Expression > uid { + if uidNum < exprNum { return false, nil } case table.OperatorLessThanOrEquals: - if constraint.Expression < uid { + if uidNum > exprNum { return false, nil } default: From 7d8d8212af567d4a52fe9c345b5e0fef47880b47 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:08 +0000 Subject: [PATCH 08/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/datastore/mysqlredis/hosts.go | 57 +++++++++++++++++++++------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/server/datastore/mysqlredis/hosts.go b/server/datastore/mysqlredis/hosts.go index a4bea4c8842..4db7e76f060 100644 --- a/server/datastore/mysqlredis/hosts.go +++ b/server/datastore/mysqlredis/hosts.go @@ -39,10 +39,11 @@ func (d *Datastore) SyncEnrolledHostIDs(ctx context.Context) error { return ctxerr.Wrap(ctx, err, "count enrolled hosts from the database") } - conn := redis.ConfigureDoer(d.pool, d.pool.Get()) - defer conn.Close() - - redisCount, err := redigo.Int(conn.Do("SCARD", enrolledHostsSetKey)) + redisCount, err := func() (int, error) { + conn := redis.ConfigureDoer(d.pool, d.pool.Get()) + defer conn.Close() + return redigo.Int(conn.Do("SCARD", enrolledHostsSetKey)) + }() if err != nil { return ctxerr.Wrap(ctx, err, "count enrolled hosts from redis") } @@ -57,15 +58,43 @@ func (d *Datastore) SyncEnrolledHostIDs(ctx context.Context) error { return ctxerr.Wrap(ctx, err, "get enrolled host IDs from the database") } - if _, err := conn.Do("DEL", enrolledHostsSetKey); err != nil { - return ctxerr.Wrap(ctx, err, "clear redis enrolled hosts set") + if err := replaceEnrolledHostIDs(ctx, d.pool, ids...); err != nil { + return ctxerr.Wrap(ctx, err, "replace redis enrolled hosts set") + } + return nil +} + +// replaceEnrolledHostIDs atomically clears the enrolled hosts set and +// repopulates it with the given IDs, using a single connection and a +// MULTI/EXEC transaction so that a crash or dropped connection between the +// clear and the repopulate cannot leave the set empty. +func replaceEnrolledHostIDs(ctx context.Context, pool fleet.RedisPool, hostIDs ...uint) error { + conn := redis.ConfigureDoer(pool, pool.Get()) + defer conn.Close() + + if err := conn.Send("MULTI"); err != nil { + return ctxerr.Wrap(ctx, err, "start redis transaction") } + if err := conn.Send("DEL", enrolledHostsSetKey); err != nil { + return ctxerr.Wrap(ctx, err, "queue clear redis enrolled hosts set") + } + + for len(hostIDs) > 0 { + maxSize := len(hostIDs) + if maxSize > redisSetMembersBatchSize { + maxSize = redisSetMembersBatchSize + } - // return the connection to the pool so it can be reused in addHosts - conn.Close() + args := redigo.Args{enrolledHostsSetKey} + args = args.AddFlat(hostIDs[:maxSize]) + if err := conn.Send("SADD", args...); err != nil { + return ctxerr.Wrap(ctx, err, "queue add database host IDs to the redis set") + } + hostIDs = hostIDs[maxSize:] + } - if err := addHosts(ctx, d.pool, ids...); err != nil { - return ctxerr.Wrap(ctx, err, "add database host IDs to the redis set") + if _, err := conn.Do("EXEC"); err != nil { + return ctxerr.Wrap(ctx, err, "execute redis transaction") } return nil } @@ -162,7 +191,7 @@ func (d *Datastore) EnrollOsquery(ctx context.Context, opts ...fleet.DatastoreEn func (d *Datastore) DeleteHost(ctx context.Context, hid uint) error { err := d.Datastore.DeleteHost(ctx, hid) if err != nil { - return err + return fmt.Errorf("delete host: %w", err) } if d.enforceHostLimit > 0 { if err := removeHosts(ctx, d.pool, hid); err != nil { @@ -178,7 +207,7 @@ func (d *Datastore) DeleteHost(ctx context.Context, hid uint) error { func (d *Datastore) DeleteHosts(ctx context.Context, ids []uint) error { err := d.Datastore.DeleteHosts(ctx, ids) if err != nil { - return err + return fmt.Errorf("delete hosts: %w", err) } if d.enforceHostLimit > 0 { if err := removeHosts(ctx, d.pool, ids...); err != nil { @@ -193,7 +222,7 @@ func (d *Datastore) DeleteHosts(ctx context.Context, ids []uint) error { func (d *Datastore) CleanupExpiredHosts(ctx context.Context) ([]fleet.DeletedHostDetails, error) { details, err := d.Datastore.CleanupExpiredHosts(ctx) if err != nil { - return details, err + return details, fmt.Errorf("cleanup expired hosts: %w", err) } ids := make([]uint, len(details)) for i, detail := range details { @@ -211,7 +240,7 @@ func (d *Datastore) CleanupExpiredHosts(ctx context.Context) ([]fleet.DeletedHos func (d *Datastore) CleanupIncomingHosts(ctx context.Context, now time.Time) ([]uint, error) { ids, err := d.Datastore.CleanupIncomingHosts(ctx, now) if err != nil { - return ids, err + return ids, fmt.Errorf("cleanup incoming hosts: %w", err) } if d.enforceHostLimit > 0 { if err := removeHosts(ctx, d.pool, ids...); err != nil { From daafcb8d9211247e700c28e8ad48b1380496874a Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:09 +0000 Subject: [PATCH 09/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/service/mdm_profiles.go | 75 ++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 18 deletions(-) diff --git a/server/service/mdm_profiles.go b/server/service/mdm_profiles.go index ee7d38b1679..933c030c665 100644 --- a/server/service/mdm_profiles.go +++ b/server/service/mdm_profiles.go @@ -2,6 +2,7 @@ package service import ( "fmt" + "sort" "strings" "github.com/fleetdm/fleet/v4/server/fleet" @@ -48,16 +49,26 @@ func (d *DigiCertVarsFound) CAs() []string { } func (d *DigiCertVarsFound) ErrorMessage() string { + passwordMismatches := make([]string, 0, len(d.passwordCA)) for ca := range d.passwordCA { if _, ok := d.dataCA[ca]; !ok { - return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarDigiCertDataPrefix, ca) + passwordMismatches = append(passwordMismatches, ca) } } + if len(passwordMismatches) > 0 { + sort.Strings(passwordMismatches) + return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarDigiCertDataPrefix, passwordMismatches[0]) + } + dataMismatches := make([]string, 0, len(d.dataCA)) for ca := range d.dataCA { if _, ok := d.passwordCA[ca]; !ok { - return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarDigiCertPasswordPrefix, ca) + dataMismatches = append(dataMismatches, ca) } } + if len(dataMismatches) > 0 { + sort.Strings(dataMismatches) + return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarDigiCertPasswordPrefix, dataMismatches[0]) + } return fmt.Sprintf("CA name mismatch between $FLEET_VAR_%s and $FLEET_VAR_%s in the profile.", fleet.FleetVarDigiCertDataPrefix, fleet.FleetVarDigiCertPasswordPrefix) } @@ -197,16 +208,26 @@ func (cs *CustomSCEPVarsFound) ErrorMessage() string { return fmt.Sprintf("SCEP profile for custom SCEP certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", fleet.FleetVarCustomSCEPChallengePrefix, fleet.FleetVarCustomSCEPProxyURLPrefix, fleet.FleetVarCertificateRenewalID) } + challengeMismatches := make([]string, 0, len(cs.challengeCA)) for ca := range cs.challengeCA { if _, ok := cs.urlCA[ca]; !ok { - return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarCustomSCEPProxyURLPrefix, ca) + challengeMismatches = append(challengeMismatches, ca) } } + if len(challengeMismatches) > 0 { + sort.Strings(challengeMismatches) + return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarCustomSCEPProxyURLPrefix, challengeMismatches[0]) + } + urlMismatches := make([]string, 0, len(cs.urlCA)) for ca := range cs.urlCA { if _, ok := cs.challengeCA[ca]; !ok { - return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarCustomSCEPChallengePrefix, ca) + urlMismatches = append(urlMismatches, ca) } } + if len(urlMismatches) > 0 { + sort.Strings(urlMismatches) + return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarCustomSCEPChallengePrefix, urlMismatches[0]) + } return fmt.Sprintf("CA name mismatch between $FLEET_VAR_%s and $FLEET_VAR_%s in the profile.", fleet.FleetVarCustomSCEPProxyURLPrefix, fleet.FleetVarCustomSCEPChallengePrefix) @@ -299,16 +320,26 @@ func (cs *SmallstepVarsFound) ErrorMessage() string { if !cs.renewalIdFound || len(cs.challengeCA) == 0 || len(cs.urlCA) == 0 { return fmt.Sprintf("SCEP profile for Smallstep certificate authority requires: $FLEET_VAR_%s, $FLEET_VAR_%s, and $FLEET_VAR_%s variables.", fleet.FleetVarSmallstepSCEPChallengePrefix, fleet.FleetVarSmallstepSCEPProxyURLPrefix, fleet.FleetVarCertificateRenewalID) } + challengeMismatches := make([]string, 0, len(cs.challengeCA)) for ca := range cs.challengeCA { if _, ok := cs.urlCA[ca]; !ok { - return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarSmallstepSCEPProxyURLPrefix, ca) + challengeMismatches = append(challengeMismatches, ca) } } + if len(challengeMismatches) > 0 { + sort.Strings(challengeMismatches) + return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarSmallstepSCEPProxyURLPrefix, challengeMismatches[0]) + } + urlMismatches := make([]string, 0, len(cs.urlCA)) for ca := range cs.urlCA { if _, ok := cs.challengeCA[ca]; !ok { - return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarSmallstepSCEPChallengePrefix, ca) + urlMismatches = append(urlMismatches, ca) } } + if len(urlMismatches) > 0 { + sort.Strings(urlMismatches) + return fmt.Sprintf("Missing $FLEET_VAR_%s%s in the profile", fleet.FleetVarSmallstepSCEPChallengePrefix, urlMismatches[0]) + } return fmt.Sprintf("CA name mismatch between $FLEET_VAR_%s and $FLEET_VAR_%s in the profile.", fleet.FleetVarSmallstepSCEPProxyURLPrefix, fleet.FleetVarSmallstepSCEPChallengePrefix) } @@ -373,6 +404,12 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f smallstepVars *SmallstepVarsFound customSCEPVars *CustomSCEPVarsFound ) + // renewalIDSeen tracks whether a renewal-ID Fleet variable has already + // been observed in this profile, independent of which CA-type struct + // (Custom SCEP, NDES, or Smallstep) ends up being relevant, so that + // duplicate detection is correct even when only one of those struct + // types applies to the profile. + renewalIDSeen := false for _, k := range fleetVars { caFound := false ok := true @@ -460,14 +497,16 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f // Custom SCEP, NDES, and Smallstep all share the renewal-ID // Fleet variable. The legacy SCEP_RENEWAL_ID and the preferred // CERTIFICATE_RENEWAL_ID names are interchangeable here. - - customSCEPVars, ok = customSCEPVars.SetRenewalID() - if ok { - ndesVars, ok = ndesVars.SetRenewalID() - if ok { - smallstepVars, ok = smallstepVars.SetRenewalID() - } - } + // Duplicate detection is based on renewalIDSeen, which is + // independent of which CA-type struct is actually relevant for + // this profile, so a repeated renewal-ID variable is always + // correctly flagged. + + ok = !renewalIDSeen + renewalIDSeen = true + customSCEPVars, _ = customSCEPVars.SetRenewalID() + ndesVars, _ = ndesVars.SetRenewalID() + smallstepVars, _ = smallstepVars.SetRenewalID() } if !ok { @@ -492,7 +531,7 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if additionalDigiCertValidation != nil { err := additionalDigiCertValidation(profileContents, digiCertVars) if err != nil { - return err + return fmt.Errorf("additional DigiCert validation: %w", err) } } } @@ -532,7 +571,7 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if additionalCustomSCEPValidation != nil { err := additionalCustomSCEPValidation(profileContents, customSCEPVars) if err != nil { - return err + return fmt.Errorf("additional custom SCEP validation: %w", err) } } } @@ -543,7 +582,7 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if additionalNDESValidation != nil { err := additionalNDESValidation(profileContents, ndesVars) if err != nil { - return err + return fmt.Errorf("additional NDES validation: %w", err) } } } @@ -554,7 +593,7 @@ func validateProfileCertificateAuthorityVariables(profileContents string, lic *f if additionalSmallstepValidation != nil { err := additionalSmallstepValidation(profileContents, smallstepVars) if err != nil { - return err + return fmt.Errorf("additional Smallstep validation: %w", err) } } } From ae82f79b43c62025189a935f95fec660d3d021e5 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:10 +0000 Subject: [PATCH 10/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../com/fleetdm/agent/scep/ScepClientImpl.kt | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt b/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt index 14651ae087f..cf0d14b6bda 100644 --- a/android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt +++ b/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) } } From 4533234bb50abfe5fd26a61864358358e570c826 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:11 +0000 Subject: [PATCH 11/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- cmd/osquery-perf/softwaredb/softwaredb.go | 46 ++++++++++++++++------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/cmd/osquery-perf/softwaredb/softwaredb.go b/cmd/osquery-perf/softwaredb/softwaredb.go index 44d5820cb63..ae335f0ff4b 100644 --- a/cmd/osquery-perf/softwaredb/softwaredb.go +++ b/cmd/osquery-perf/softwaredb/softwaredb.go @@ -9,6 +9,7 @@ import ( "math/rand/v2" "os" "strings" + "sync" _ "github.com/mattn/go-sqlite3" ) @@ -44,7 +45,8 @@ 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 @@ -52,6 +54,8 @@ 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) } From f410a08d72cf99b845fcc622fee8aa750853c0e2 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:12 +0000 Subject: [PATCH 12/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/chart/internal/mysql/data.go | 69 ++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/server/chart/internal/mysql/data.go b/server/chart/internal/mysql/data.go index cd566742e89..53bd65174e4 100644 --- a/server/chart/internal/mysql/data.go +++ b/server/chart/internal/mysql/data.go @@ -97,12 +97,29 @@ func (ds *Datastore) recordAccumulate( entityIDs = append(entityIDs, id) } + // The read-then-ODKU-write sequence below is wrapped in a single + // transaction with SELECT ... FOR UPDATE so concurrent callers merging + // against the same (dataset, bucketStart, entity_id) rows serialize on + // the row lock instead of racing to compute independent OR-merges that + // could silently clobber one another via ODKU. + tx, err := ds.writer(ctx).BeginTxx(ctx, nil) + if err != nil { + return ctxerr.Wrap(ctx, err, "begin accumulate transaction") + } + committed := false + defer func() { + if !committed { + _ = tx.Rollback() + } + }() + // Fetch the current in-bucket bitmaps so we can OR-merge before writing. existing := make(map[string]*roaring.Bitmap, len(entityIDs)) if len(entityIDs) > 0 { query, args, err := sqlx.In( `SELECT entity_id, host_bitmap, encoding_type FROM host_scd_data - WHERE dataset = ? AND valid_from = ? AND entity_id IN (?)`, + WHERE dataset = ? AND valid_from = ? AND entity_id IN (?) + FOR UPDATE`, dataset, bucketStart, entityIDs) if err != nil { return ctxerr.Wrap(ctx, err, "expand accumulate select args") @@ -115,10 +132,14 @@ func (ds *Datastore) recordAccumulate( EncodingType uint8 `db:"encoding_type"` } var rows []row - // Using writer here since a stale read would OR-merge against an older - // bitmap, then ODKU would overwrite the row with the partial merge — silently - // dropping hosts from any sample the replica hadn't replicated yet. - if err := sqlx.SelectContext(ctx, ds.writer(ctx), &rows, query, args...); err != nil { + // Using the writer, within the transaction above, with FOR UPDATE: + // a stale read would OR-merge against an older bitmap, then ODKU + // would overwrite the row with the partial merge — silently + // dropping hosts from any sample the replica hadn't replicated yet + // or from a concurrent writer's merge. The row lock held until + // commit ensures concurrent accumulate calls for the same rows + // serialize instead of racing. + if err := sqlx.SelectContext(ctx, tx, &rows, query, args...); err != nil { return ctxerr.Wrap(ctx, err, "fetch in-bucket bitmaps") } for _, r := range rows { @@ -154,10 +175,15 @@ func (ds *Datastore) recordAccumulate( stmt := `INSERT INTO host_scd_data (dataset, entity_id, host_bitmap, encoding_type, valid_from, valid_to) VALUES ` + //nolint:gosec // G202 strings.Join(placeholders, ", ") + ` ON DUPLICATE KEY UPDATE host_bitmap = VALUES(host_bitmap), encoding_type = VALUES(encoding_type)` - if _, err := ds.writer(ctx).ExecContext(ctx, stmt, args...); err != nil { + if _, err := tx.ExecContext(ctx, stmt, args...); err != nil { return ctxerr.Wrap(ctx, err, "upsert accumulate rows") } } + + if err := tx.Commit(); err != nil { + return ctxerr.Wrap(ctx, err, "commit accumulate transaction") + } + committed = true return nil } @@ -237,6 +263,20 @@ func (ds *Datastore) recordSnapshot( } } + // The close UPDATE and the reopening INSERT ... ON DUPLICATE KEY UPDATE + // are wrapped in a single transaction so a reader can never observe an + // entity with its row closed but not yet reopened. + tx, err := ds.writer(ctx).BeginTxx(ctx, nil) + if err != nil { + return ctxerr.Wrap(ctx, err, "begin snapshot transaction") + } + committed := false + defer func() { + if !committed { + _ = tx.Rollback() + } + }() + if len(toClose) > 0 { closeQuery, closeArgs, err := sqlx.In( `UPDATE host_scd_data SET valid_to = ? @@ -246,7 +286,7 @@ func (ds *Datastore) recordSnapshot( return ctxerr.Wrap(ctx, err, "expand close SCD query args") } closeQuery = ds.rebind(closeQuery) - if _, err := ds.writer(ctx).ExecContext(ctx, closeQuery, closeArgs...); err != nil { + if _, err := tx.ExecContext(ctx, closeQuery, closeArgs...); err != nil { return ctxerr.Wrap(ctx, err, "close stale SCD rows") } } @@ -269,11 +309,15 @@ func (ds *Datastore) recordSnapshot( stmt := `INSERT INTO host_scd_data (dataset, entity_id, host_bitmap, encoding_type, valid_from) VALUES ` + //nolint:gosec // G202 strings.Join(placeholders, ", ") + ` ON DUPLICATE KEY UPDATE host_bitmap = VALUES(host_bitmap), encoding_type = VALUES(encoding_type)` - if _, err := ds.writer(ctx).ExecContext(ctx, stmt, args...); err != nil { + if _, err := tx.ExecContext(ctx, stmt, args...); err != nil { return ctxerr.Wrap(ctx, err, "upsert snapshot rows") } } + if err := tx.Commit(); err != nil { + return ctxerr.Wrap(ctx, err, "commit snapshot transaction") + } + committed = true return nil } @@ -299,7 +343,8 @@ func (ds *Datastore) recordSnapshot( // // The caller is responsible for passing bucket-aligned startDate/endDate (e.g. // local-midnight-aligned for tz-sensitive rendering); the walker does not -// truncate. +// truncate. If the requested range is not an exact multiple of bucketSize, an +// error is returned rather than silently discarding the remainder. func (ds *Datastore) GetSCDData( ctx context.Context, dataset string, @@ -312,7 +357,11 @@ func (ds *Datastore) GetSCDData( startDate = startDate.UTC() endDate = endDate.UTC() - numBuckets := int(endDate.Sub(startDate) / bucketSize) + totalRange := endDate.Sub(startDate) + if totalRange > 0 && bucketSize > 0 && totalRange%bucketSize != 0 { + return nil, ctxerr.Errorf(ctx, "date range %s is not an exact multiple of bucket size %s", totalRange, bucketSize) + } + numBuckets := int(totalRange / bucketSize) if numBuckets <= 0 { return nil, nil } From 11860bade6190644ffa98b17862346eab9ef071c Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:14 +0000 Subject: [PATCH 13/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- ...20250410104321_UpdateMacOSSoftwareNames.go | 50 ++++++++++++++++--- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go b/server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go index 3cea835f0ae..44146d28b51 100644 --- a/server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go +++ b/server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go @@ -24,7 +24,8 @@ func Up_20250410104321(tx *sql.Tx) error { } dupeIDsStmt := `SELECT GROUP_CONCAT(id) AS ids, MD5( - -- simulate new hash + -- simulate new hash (note: does not include name, matching the checksum + -- recomputation below; verify this matches the live ingestion checksum formula) CONCAT_WS(CHAR(0), version, source, @@ -89,10 +90,24 @@ WHERE updateHostSoftwareInstalledPathsStmt := `UPDATE host_software_installed_paths SET software_id = ? WHERE software_id IN (?)` var allExcludedIDs []uint64 + addedTempIndex := false if !indexExistsTx(tx, "host_software_installed_paths", "software_id") { if _, err = tx.Exec(`ALTER TABLE host_software_installed_paths ADD INDEX software_id (software_id)`); err != nil { return fmt.Errorf("adding temporary index to host_software_installed_paths: %w", err) } + addedTempIndex = true + } + + // dropTempIndex ensures the temporary index added above (if any) is removed + // before returning from this function, even on error paths, since DDL + // statements are not part of the surrounding transaction on MySQL. + dropTempIndex := func() error { + if addedTempIndex && indexExistsTx(tx, "host_software_installed_paths", "software_id") { + if _, err := tx.Exec(`ALTER TABLE host_software_installed_paths DROP INDEX software_id`); err != nil { + return fmt.Errorf("removing temporary index from host_software_installed_paths: %w", err) + } + } + return nil } for newChecksum, idsToMerge := range idsToMergeByNewChecksum { @@ -102,20 +117,31 @@ WHERE } selectedID, ok := selectedIDs[newChecksum] if !ok { + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("%v excluded IDs but no selected ID", idsToMerge) } stmt, args, err := sqlx.In(getRecordToUpdateStmt, idsToMerge, selectedID) if err != nil { + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("sqlx.In for getting host software records to update for old software IDs %v: %w", idsToMerge, err) } if err := txx.Select(&hostIDRecordList, stmt, args...); err != nil { + // Note: sqlx's Select into a slice does not return sql.ErrNoRows when + // there are no matching rows (it returns nil with an empty slice), so + // this branch is not expected to be reached in that case; it is kept + // only as a defensive no-op for that specific error. if errors.Is(err, sql.ErrNoRows) { - // if there are no rows, this means the host is already pointed at the selected software - // ID, so no update needed continue } + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("getting host software record to update for old software IDs %v: %w", idsToMerge, err) } @@ -130,6 +156,9 @@ WHERE if len(hostSoftwareInsertParams) >= 20_000 { // update up to 10k hosts at a time _, err = tx.Exec(strings.TrimSuffix(hostSoftwareInsertQuery, ","), hostSoftwareInsertParams...) if err != nil { + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("updating host_software.software_id for old software IDs %v: %w", idsToMerge, err) } hostSoftwareInsertQuery = `INSERT IGNORE INTO host_software (host_id, software_id) VALUES ` @@ -140,6 +169,9 @@ WHERE if len(hostSoftwareInsertParams) > 0 { // flush last batch _, err = tx.Exec(strings.TrimSuffix(hostSoftwareInsertQuery, ","), hostSoftwareInsertParams...) if err != nil { + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("updating host_software.software_id for old software IDs %v: %w", idsToMerge, err) } } @@ -148,18 +180,22 @@ WHERE // repoint host software installed paths to the software ID we're keeping stmt, args, err = sqlx.In(updateHostSoftwareInstalledPathsStmt, selectedID, idsToMerge) if err != nil { + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("sqlx.In for updating host software installed paths records for old software IDs %v: %w", idsToMerge, err) } if _, err := tx.Exec(stmt, args...); err != nil { + if dropErr := dropTempIndex(); dropErr != nil { + return dropErr + } return fmt.Errorf("updating host software installed paths records for old software IDs %v: %w", idsToMerge, err) } } - if indexExistsTx(tx, "host_software_installed_paths", "software_id") { - if _, err = tx.Exec(`ALTER TABLE host_software_installed_paths DROP INDEX software_id`); err != nil { - return fmt.Errorf("removing temporary index from host_software_installed_paths: %w", err) - } + if err := dropTempIndex(); err != nil { + return err } // at this point, every host that needs one has a pointer to the selected ID, so we can delete From 03a9a4a8489f76d8d507ea2b6d9bcfc6933032ff Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:15 +0000 Subject: [PATCH 14/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/mdm/scep/cmd/scepserver/scepserver.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/server/mdm/scep/cmd/scepserver/scepserver.go b/server/mdm/scep/cmd/scepserver/scepserver.go index 7c452078e2c..7a31c135594 100644 --- a/server/mdm/scep/cmd/scepserver/scepserver.go +++ b/server/mdm/scep/cmd/scepserver/scepserver.go @@ -95,7 +95,7 @@ func main() { } } lginfo := logger - ctx := context.TODO() + ctx := context.Background() var err error var depot scepdepot.Depot // cert storage @@ -179,7 +179,7 @@ func main() { }() go func() { c := make(chan os.Signal, 1) - signal.Notify(c, syscall.SIGINT) + signal.Notify(c, syscall.SIGINT, syscall.SIGTERM) errs <- fmt.Errorf("%s", <-c) }() @@ -233,12 +233,23 @@ func createKey(bits int, password []byte, depot string) (*rsa.PrivateKey, error) if err != nil { return nil, err } - privPEMBlock, err := x509.EncryptPEMBlock( + if len(password) == 0 { + privPEMBlock := &pem.Block{ + Type: rsaPrivateKeyPEMBlockType, + Bytes: x509.MarshalPKCS1PrivateKey(key), + } + if err := pem.Encode(file, privPEMBlock); err != nil { + os.Remove(name) + return nil, err + } + return key, nil + } + privPEMBlock, err := x509.EncryptPEMBlock( //nolint:staticcheck // legacy PEM encryption retained for backward compatibility; caller must supply a strong -capass rand.Reader, rsaPrivateKeyPEMBlockType, x509.MarshalPKCS1PrivateKey(key), password, - x509.PEMCipher3DES, + x509.PEMCipherAES256, ) if err != nil { return nil, err From e9e064e3bc94360fa2a7ba965ac3bcced0491fb8 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:16 +0000 Subject: [PATCH 15/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- tools/fleet-slackbot/webhook-handler.js | 72 ++++++++++++++++++------- 1 file changed, 53 insertions(+), 19 deletions(-) diff --git a/tools/fleet-slackbot/webhook-handler.js b/tools/fleet-slackbot/webhook-handler.js index 7849ae366ac..afebea416be 100644 --- a/tools/fleet-slackbot/webhook-handler.js +++ b/tools/fleet-slackbot/webhook-handler.js @@ -41,6 +41,13 @@ function verifySignature(rawBody, signatureHeader, secret) { } } +// Rejects any path containing a ".." segment anywhere (not just as a +// leading prefix), to guard against traversal via backslash or other +// separators that path.posix.normalize does not collapse. +function hasTraversalSegment(p) { + return p.split(/[\\/]/).some((segment) => segment === ".."); +} + function createWebhookHandler(config, github, claude) { return async function handleWebhook(req, res) { // Collect raw body from the IncomingMessage stream (capped at 1MB) @@ -152,15 +159,31 @@ function createWebhookHandler(config, github, claude) { return; } - // Respond to GitHub immediately to avoid timeout - res.writeHead(200, { "Content-Type": "application/json" }); - res.end(JSON.stringify({ ok: true, message: "processing" })); - // Extract PR number — different payload structure per event type const prNumber = event === "issue_comment" ? payload.issue.number : payload.pull_request.number; const commentId = payload.comment.id; + + // Extra safety: author_association reflects association to the repo the + // event fired on, not necessarily the PR's head repo. Verify the PR's + // head repo matches the base repo (i.e. not a fork) before proceeding, + // since this bot can auto-commit AI-authored content. + const baseRepoFullName = payload.repository && payload.repository.full_name; + const prForAssocCheck = event === "issue_comment" ? payload.issue.pull_request : payload.pull_request; + const headRepoFullName = prForAssocCheck && prForAssocCheck.head && prForAssocCheck.head.repo && + prForAssocCheck.head.repo.full_name; + if (headRepoFullName && baseRepoFullName && headRepoFullName !== baseRepoFullName) { + console.log(`[webhook] Ignoring comment on PR #${prNumber} — head repo "${headRepoFullName}" does not match base repo "${baseRepoFullName}" (fork)`); + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ ok: true, message: "cross-repo PR not allowed" })); + return; + } + + // Respond to GitHub immediately to avoid timeout + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ ok: true, message: "processing" })); + console.log(`[webhook] PR #${prNumber} comment from ${commentAuthor}: "${commentBody.slice(0, 100)}"`); // Check if the bot was @mentioned @@ -251,7 +274,8 @@ async function processComment({ prNumber, commentBody, commentId, event, mention const changes = []; for (const c of proposal.changes) { const normalized = path.posix.normalize(c.filePath); - if (normalized.startsWith("..") || path.posix.isAbsolute(normalized) || + if (normalized.startsWith("..") || path.posix.isAbsolute(normalized) || hasTraversalSegment(c.filePath) || + hasTraversalSegment(normalized) || !(normalized === "default.yml" || normalized.startsWith("fleets/") || normalized.startsWith("lib/"))) { throw new Error(`Invalid file path in response (must be under default.yml, fleets/, or lib/): ${c.filePath}`); } @@ -308,20 +332,29 @@ async function handleCheckRun(payload, config, github, claude) { const headSha = checkRun.head_sha; console.log(`[ci-fix] Check "${checkRun.name}" failed on PR #${prNumber} (sha: ${headSha.slice(0, 8)})`); - // Loop prevention: allow up to 2 consecutive CI fix attempts, then stop + // Loop prevention: walk back through consecutive "CI fix:" commits and + // count how many auto-fix attempts have already been made. This covers + // non-consecutive bot commits interleaved with human commits by tracking + // an attempt counter embedded in the commit message rather than only + // inspecting the immediate parent. + const MAX_CI_FIX_ATTEMPTS = 2; try { - const commit = await github.getCommit(headSha); - if (commit.message.startsWith("CI fix:")) { - // Check the parent commit too — if it's also a CI fix, we've already retried once - const parentSha = commit.parentSha; - if (parentSha) { - const parent = await github.getCommit(parentSha); - if (parent.message.startsWith("CI fix:")) { - console.log(`[ci-fix] Skipping — already attempted CI fix twice`); - return; - } - } - console.log(`[ci-fix] Previous CI fix failed, retrying (attempt 2)...`); + let attemptCount = 0; + let sha = headSha; + for (let i = 0; i < MAX_CI_FIX_ATTEMPTS + 1; i++) { + const commit = await github.getCommit(sha); + const match = commit.message.match(/^CI fix:(?:\s*\(attempt (\d+)\))?/); + if (!match) break; + attemptCount = match[1] ? parseInt(match[1], 10) : attemptCount + 1; + if (!commit.parentSha) break; + sha = commit.parentSha; + } + if (attemptCount >= MAX_CI_FIX_ATTEMPTS) { + console.log(`[ci-fix] Skipping — already attempted CI fix ${attemptCount} time(s)`); + return; + } + if (attemptCount > 0) { + console.log(`[ci-fix] Previous CI fix failed, retrying (attempt ${attemptCount + 1})...`); } } catch (err) { console.warn(`[ci-fix] Could not check commit history: ${err.message}, skipping as a safety precaution`); @@ -395,7 +428,8 @@ async function handleCheckRun(payload, config, github, claude) { const changes = []; for (const c of proposal.changes) { const normalized = path.posix.normalize(c.filePath); - if (normalized.startsWith("..") || path.posix.isAbsolute(normalized) || + if (normalized.startsWith("..") || path.posix.isAbsolute(normalized) || hasTraversalSegment(c.filePath) || + hasTraversalSegment(normalized) || !(normalized === "default.yml" || normalized.startsWith("fleets/") || normalized.startsWith("lib/"))) { throw new Error(`Invalid file path in CI fix response (must be under default.yml, fleets/, or lib/): ${c.filePath}`); } From 448e94294370d716ccdc9b9797748eb3a35e23bb Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:17 +0000 Subject: [PATCH 16/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- website/api/controllers/webhooks/receive-from-clay.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/website/api/controllers/webhooks/receive-from-clay.js b/website/api/controllers/webhooks/receive-from-clay.js index 62e67bb6c99..18bd89a16c9 100644 --- a/website/api/controllers/webhooks/receive-from-clay.js +++ b/website/api/controllers/webhooks/receive-from-clay.js @@ -110,8 +110,12 @@ module.exports = { throw new Error('No webhook secret configured! (Please set `sails.config.custom.zapierWebhookSecret`.)'); } - if(webhookSecret !== sails.config.custom.clayWebhookSecret){ - throw new Error('Received unexpected webhook request with webhookSecret set to: '+webhookSecret); + let crypto = require('crypto'); + let expectedSecretBuffer = Buffer.from(sails.config.custom.clayWebhookSecret); + let receivedSecretBuffer = Buffer.from(webhookSecret); + let isSecretValid = expectedSecretBuffer.length === receivedSecretBuffer.length && crypto.timingSafeEqual(expectedSecretBuffer, receivedSecretBuffer); + if(!isSecretValid){ + throw new Error('Received unexpected webhook request with an invalid webhookSecret.'); } @@ -134,7 +138,7 @@ module.exports = { if(!recordDetails.salesforceAccountId) { sails.log.warn(`When the receive-from-clay received information about a user's activity (name: ${firstName} ${lastName}), activity: ${intentSignal}). A contact was successfully updated, but the webhook is unable to continue because this contact is not associated with any Salesforce account record. Contact ID: ${recordDetails.salesforceContactId}`); - throw 'couldNotCreateActivity'; + throw {couldNotCreateActivity: new Error(`Could not create activity: contact (Contact ID: ${recordDetails.salesforceContactId}) is not associated with any Salesforce account record.`)}; } let trimmedLinkedinUrl; From bca95a3741bf10cce7cfb8d788340cdf221084cc Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:18 +0000 Subject: [PATCH 17/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../QueryFrequencyIndicator/QueryFrequencyIndicator.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frontend/components/QueryFrequencyIndicator/QueryFrequencyIndicator.tsx b/frontend/components/QueryFrequencyIndicator/QueryFrequencyIndicator.tsx index 95704093d7e..79e38e5726d 100644 --- a/frontend/components/QueryFrequencyIndicator/QueryFrequencyIndicator.tsx +++ b/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 ( -
+
{frequencyIcon()} {readableQueryFrequency()}
From 59be5669c110e2bb75af1ea5994ec46f14b4e0c1 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:19 +0000 Subject: [PATCH 18/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../details/HostQueryReport/HostQueryReport.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx b/frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx index 386b6677206..67cfef8e7d6 100644 --- a/frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx +++ b/frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useContext, useState } from "react"; +import React, { useCallback, useContext, useEffect, useState } from "react"; import { useQuery } from "react-query"; import { browserHistory, InjectedRouter } from "react-router"; import { Params } from "react-router/lib/Router"; @@ -96,15 +96,16 @@ const HostQueryReport = ({ } = queryResponse || {}; // previous reroute can be done before API call, not this one, hence 2 - if (queryDiscardData) { - router.push(PATHS.HOST_REPORTS(hostId)); - } + useEffect(() => { + if (queryDiscardData) { + router.push(PATHS.HOST_REPORTS(hostId)); + } + }, [queryDiscardData, router, hostId]); // Updates title that shows up on browser tabs if (queryName && hostName) { // e.g., Discover TLS certificates (Rachel's MacBook Pro) | Hosts | Fleet - document.title = `${queryName} (${hostName}) | - Hosts | ${DOCUMENT_TITLE_SUFFIX}`; + document.title = `${queryName} (${hostName}) | Hosts | ${DOCUMENT_TITLE_SUFFIX}`; } else { document.title = `Hosts | ${DOCUMENT_TITLE_SUFFIX}`; } From cc7f1b6f09295d7b166d75f0cb204aa5390a58d6 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:20 +0000 Subject: [PATCH 19/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/mdm/nanomdm/http/api/api.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/mdm/nanomdm/http/api/api.go b/server/mdm/nanomdm/http/api/api.go index 7954331dbbc..c6bc2589e45 100644 --- a/server/mdm/nanomdm/http/api/api.go +++ b/server/mdm/nanomdm/http/api/api.go @@ -214,9 +214,9 @@ func RawCommandEnqueueHandler(enqueuer storage.CommandEnqueuer, pusher push.Push var pushErr error if !nopush && pusher != nil { pushResp, pushErr = pusher.Push(ctx, ids) - if err != nil { - logger.Info("msg", "push", "err", err) - output.PushError = err.Error() + if pushErr != nil { + logger.Info("msg", "push", "err", pushErr) + output.PushError = pushErr.Error() } } else if !nopush && pusher == nil { pushErr = errors.New("nil pusher") @@ -291,7 +291,7 @@ func readPEMCertAndKey(input []byte) (cert []byte, key []byte, err error) { case block.Type == "PRIVATE KEY" || strings.HasSuffix(block.Type, " PRIVATE KEY"): if x509.IsEncryptedPEMBlock(block) { err = errors.New("private key PEM appears to be encrypted") - break + return } key = pem.EncodeToMemory(block) default: From 84bfe00538ad8e0c61b78e5e128096c8c5ca0443 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:21 +0000 Subject: [PATCH 20/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- tools/fleetctl-npm/run.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/fleetctl-npm/run.js b/tools/fleetctl-npm/run.js index 545d9c35932..9838ccc7350 100755 --- a/tools/fleetctl-npm/run.js +++ b/tools/fleetctl-npm/run.js @@ -38,7 +38,7 @@ const platform = (() => { } })(); -const binName = platform === "windows" ? "fleetctl.exe" : "fleetctl"; +const binName = platform.startsWith("windows") ? "fleetctl.exe" : "fleetctl"; const binPath = join(installDir, binName); const install = async () => { @@ -55,6 +55,7 @@ const install = async () => { // Need to return a promise with the writer to ensure we can await for it to complete. return new Promise((resolve, reject) => { + response.data.on("error", reject); tarWriter.on("finish", resolve); tarWriter.on("error", reject); }); @@ -121,3 +122,4 @@ const run = async () => { }; run(); + From 8e6e0f051364a779a09b763e8041b20796a4dbe7 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:22 +0000 Subject: [PATCH 21/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- tools/mdm/migration/mdmproxy/mdmproxy.go | 25 ++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tools/mdm/migration/mdmproxy/mdmproxy.go b/tools/mdm/migration/mdmproxy/mdmproxy.go index baff23a506c..bf9689a77c0 100644 --- a/tools/mdm/migration/mdmproxy/mdmproxy.go +++ b/tools/mdm/migration/mdmproxy/mdmproxy.go @@ -270,10 +270,10 @@ func udidIncludedByPercentage(udid string, percentage int) bool { return int(index) < percentage //nolint:gosec // G115 false positive } -func makeExistingProxy(existingURL, existingDNSName string) *httputil.ReverseProxy { +func makeExistingProxy(existingURL, existingDNSName string) (*httputil.ReverseProxy, error) { targetURL, err := url.Parse(existingURL) if err != nil { - panic("failed to parse fleet-url: " + err.Error()) + return nil, fmt.Errorf("failed to parse existing-url: %w", err) } proxy := httputil.NewSingleHostReverseProxy(targetURL) @@ -282,13 +282,13 @@ func makeExistingProxy(existingURL, existingDNSName string) *httputil.ReversePro transport.TLSClientConfig.ServerName = existingDNSName proxy.Transport = transport - return proxy + return proxy, nil } -func makeFleetProxy(fleetURL string, debug bool) *httputil.ReverseProxy { +func makeFleetProxy(fleetURL string, debug bool) (*httputil.ReverseProxy, error) { targetURL, err := url.Parse(fleetURL) if err != nil { - panic("failed to parse fleet-url: " + err.Error()) + return nil, fmt.Errorf("failed to parse fleet-url: %w", err) } proxy := httputil.NewSingleHostReverseProxy(targetURL) if debug { @@ -309,7 +309,7 @@ func makeFleetProxy(fleetURL string, debug bool) *httputil.ReverseProxy { } } - return proxy + return proxy, nil } func main() { @@ -330,6 +330,15 @@ func main() { panic(err) } + existingProxy, err := makeExistingProxy(*existingURL, *existingHostname) + if err != nil { + log.Fatal(err) + } + fleetProxy, err := makeFleetProxy(*fleetURL, *debug) + if err != nil { + log.Fatal(err) + } + proxy := mdmProxy{ token: *authToken, existingServerURL: *existingURL, @@ -337,8 +346,8 @@ func main() { existingHostname: *existingHostname, migratePercentage: *migratePercentage, migrateUDIDs: udids, - existingProxy: makeExistingProxy(*existingURL, *existingHostname), - fleetProxy: makeFleetProxy(*fleetURL, *debug), + existingProxy: existingProxy, + fleetProxy: fleetProxy, debug: *debug, logSkipped: *logSkipped, } From 0d45336070dab72bc80dfb5e725245f37cd18cb0 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:23 +0000 Subject: [PATCH 22/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- pkg/spec/spec.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 0cc31a273c2..9027ba697c3 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -201,7 +201,7 @@ func GroupFromBytes(b []byte, options ...GroupFromBytesOpts) (*Group, error) { specs.AppConfig = appConfigSpec case fleet.EnrollSecretKind: - if specs.AppConfig != nil { + if specs.EnrollSecret != nil { return nil, errors.New("enroll_secret defined twice in the same file") } @@ -278,12 +278,12 @@ func SplitYaml(in string) []string { return out } -func generateRandomString(sizeBytes int) string { +func generateRandomString(sizeBytes int) (string, error) { b := make([]byte, sizeBytes) if _, err := rand.Read(b); err != nil { - panic(err) + return "", err } - return hex.EncodeToString(b) + return hex.EncodeToString(b), nil } // secretHandling defines how to handle FLEET_SECRET_ variables @@ -312,7 +312,11 @@ func expandEnv(s string, secretMode secretHandling) (string, error) { // Generate a random escaping prefix that doesn't exist in s. var preventEscapingPrefix string for { - preventEscapingPrefix = "PREVENT_ESCAPING_" + generateRandomString(8) + randStr, err := generateRandomString(8) + if err != nil { + return "", fmt.Errorf("generating random string: %w", err) + } + preventEscapingPrefix = "PREVENT_ESCAPING_" + randStr if !strings.Contains(s, preventEscapingPrefix) { break } From 687d8a1fb58d05beae33a1cdd22fa25212c02c13 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:24 +0000 Subject: [PATCH 23/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/mdm/nanomdm/storage/mysql/certauth.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/mdm/nanomdm/storage/mysql/certauth.go b/server/mdm/nanomdm/storage/mysql/certauth.go index c64cb1bb696..cc1da0dcdd3 100644 --- a/server/mdm/nanomdm/storage/mysql/certauth.go +++ b/server/mdm/nanomdm/storage/mysql/certauth.go @@ -60,11 +60,12 @@ func (s *MySQLStorage) EnrollmentFromHash(ctx context.Context, hash string) (str var id string err := s.db.QueryRowContext( ctx, - `SELECT id FROM cert_auth_associations WHERE sha256 = ? LIMIT 1;`, - hash, + `SELECT id FROM nano_cert_auth_associations WHERE sha256 = ? LIMIT 1;`, + strings.ToLower(hash), ).Scan(&id) if errors.Is(err, sql.ErrNoRows) { return "", nil } return id, err } + From d4210b81a8adb74b1f6cac3ac4e514fbd9d99f85 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:26 +0000 Subject: [PATCH 24/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/service/setup_experience_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/service/setup_experience_test.go b/server/service/setup_experience_test.go index 89badaa3e96..90990e9c2ce 100644 --- a/server/service/setup_experience_test.go +++ b/server/service/setup_experience_test.go @@ -333,9 +333,9 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - ds.MaybeUpdateSetupExperienceScriptStatusFunc = func(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - require.Equal(t, hostUUID, hostUUID) - require.Equal(t, executionID, scriptUUID) + ds.MaybeUpdateSetupExperienceScriptStatusFunc = func(ctx context.Context, gotHostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, gotHostUUID) + require.Equal(t, scriptUUID, executionID) require.Equal(t, tt.expected, status) require.True(t, status.IsValid()) return true, nil @@ -388,9 +388,9 @@ func TestMaybeUpdateSetupExperience(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, hostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { - require.Equal(t, hostUUID, hostUUID) - require.Equal(t, executionID, softwareUUID) + ds.MaybeUpdateSetupExperienceSoftwareInstallStatusFunc = func(ctx context.Context, gotHostUUID string, executionID string, status fleet.SetupExperienceStatusResultStatus) (bool, error) { + require.Equal(t, hostUUID, gotHostUUID) + require.Equal(t, softwareUUID, executionID) require.Equal(t, tt.expectStatus, status) require.True(t, status.IsValid()) require.True(t, status.IsTerminalStatus()) From 3f79d98ae64cdaca12ae1b30790ceff474269fa0 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:27 +0000 Subject: [PATCH 25/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- server/service/teams.go | 51 +++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/server/service/teams.go b/server/service/teams.go index 1f6f1ce438f..6f45b6716f8 100644 --- a/server/service/teams.go +++ b/server/service/teams.go @@ -75,6 +75,24 @@ type defaultTeamResponse struct { func (r defaultTeamResponse) Error() error { return r.Err } +// newDefaultTeamResponse constructs a fleet.DefaultTeam from a fleet.Team, +// copying only the fields exposed for the default (team ID 0) response. +func newDefaultTeamResponse(team *fleet.Team) *fleet.DefaultTeam { + return &fleet.DefaultTeam{ + ID: team.ID, + Name: team.Name, + DefaultTeamConfig: fleet.DefaultTeamConfig{ + WebhookSettings: fleet.DefaultTeamWebhookSettings{ + FailingPoliciesWebhook: team.Config.WebhookSettings.FailingPoliciesWebhook, + }, + Integrations: fleet.DefaultTeamIntegrations{ + Jira: team.Config.Integrations.Jira, + Zendesk: team.Config.Integrations.Zendesk, + }, + }, + } +} + func getTeamEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { req := request.(*getTeamRequest) @@ -85,20 +103,7 @@ func getTeamEndpoint(ctx context.Context, request interface{}, svc fleet.Service // Special handling for team ID 0 - return DefaultTeam structure if team.ID == 0 { - defaultTeam := &fleet.DefaultTeam{ - ID: team.ID, - Name: team.Name, - DefaultTeamConfig: fleet.DefaultTeamConfig{ - WebhookSettings: fleet.DefaultTeamWebhookSettings{ - FailingPoliciesWebhook: team.Config.WebhookSettings.FailingPoliciesWebhook, - }, - Integrations: fleet.DefaultTeamIntegrations{ - Jira: team.Config.Integrations.Jira, - Zendesk: team.Config.Integrations.Zendesk, - }, - }, - } - return defaultTeamResponse{Team: defaultTeam}, nil + return defaultTeamResponse{Team: newDefaultTeamResponse(team)}, nil } return getTeamResponse{Team: team}, nil @@ -174,21 +179,7 @@ func modifyTeamEndpoint(ctx context.Context, request interface{}, svc fleet.Serv // Special handling for team ID 0 - return limited fields if req.ID == 0 { - // Convert to DefaultTeam with limited fields - defaultTeam := &fleet.DefaultTeam{ - ID: team.ID, - Name: team.Name, - DefaultTeamConfig: fleet.DefaultTeamConfig{ - WebhookSettings: fleet.DefaultTeamWebhookSettings{ - FailingPoliciesWebhook: team.Config.WebhookSettings.FailingPoliciesWebhook, - }, - Integrations: fleet.DefaultTeamIntegrations{ - Jira: team.Config.Integrations.Jira, - Zendesk: team.Config.Integrations.Zendesk, - }, - }, - } - return defaultTeamResponse{Team: defaultTeam}, nil + return defaultTeamResponse{Team: newDefaultTeamResponse(team)}, nil } return teamResponse{Team: team}, err @@ -314,7 +305,7 @@ func applyTeamSpecsEndpoint(ctx context.Context, request interface{}, svc fleet. return applyTeamSpecsResponse{TeamIDsByName: idsByName}, nil } -func (svc Service) ApplyTeamSpecs(ctx context.Context, _ []*fleet.TeamSpec, _ fleet.ApplyTeamSpecOptions) (map[string]uint, error) { +func (svc *Service) ApplyTeamSpecs(ctx context.Context, _ []*fleet.TeamSpec, _ fleet.ApplyTeamSpecOptions) (map[string]uint, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) From d216e974943f2d17603e98ff6ff21e98795d2fce Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:28 +0000 Subject: [PATCH 26/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- tools/github-manage/pkg/ghapi/issues.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/github-manage/pkg/ghapi/issues.go b/tools/github-manage/pkg/ghapi/issues.go index 32e00e31791..b02fce1efc1 100644 --- a/tools/github-manage/pkg/ghapi/issues.go +++ b/tools/github-manage/pkg/ghapi/issues.go @@ -4,6 +4,7 @@ package ghapi import ( "encoding/json" + "errors" "fmt" "os" "strings" @@ -124,7 +125,7 @@ func RemoveIssueFromProject(issueNumber int, projectID int) error { itemID, err := GetProjectItemID(issueNumber, projectID) if err != nil { // If the issue is not found in the project, that's not an error - if err.Error() == fmt.Sprintf("issue #%d not found in project %d", issueNumber, projectID) { + if errors.Is(err, ErrIssueNotFoundInProject) { return nil } return fmt.Errorf("failed to get project item ID: %v", err) @@ -371,7 +372,10 @@ listLoop: for i, issue := range issues { // if we find an error, we'll drain concurrent executions and then bail - if stopError != nil { + mu.Lock() + currentErr := stopError + mu.Unlock() + if currentErr != nil { break } @@ -400,7 +404,9 @@ listLoop: fmt.Fprintf(os.Stderr, " ERROR\n") mu.Unlock() } + mu.Lock() stopError = err + mu.Unlock() logger.Errorf("Error checking timeline for issue #%d: %v", iss.Number, err) return } From 8e2879cc1cd63743e40f69ed08b59a91ebb51269 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 07:54:29 +0000 Subject: [PATCH 27/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- tools/team-builder/build_teams.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tools/team-builder/build_teams.sh b/tools/team-builder/build_teams.sh index 68766a0264b..a24fbed8662 100755 --- a/tools/team-builder/build_teams.sh +++ b/tools/team-builder/build_teams.sh @@ -9,7 +9,7 @@ run(){ flags+="--disable-open-folder" #Read flags - while getopts s:p:u:f:d:o:x flag + while getopts s:p:u:a:d:o:x flag do case "${flag}" in f) #path to file containing team names. Must end with newline char. @@ -18,7 +18,7 @@ run(){ types+=($OPTARG);; u) #Fleet server url url=($OPTARG);; - f) #Additional flags to apply to `fleetctl package` + a) #Additional flags to apply to `fleetctl package` flags+=($OPTARG);; d) #include Fleet Desktop flags+="--desktop";; @@ -55,7 +55,12 @@ create_teams(){ #Loop over file contents and generate a secret for each team, then create the team and generate packages while IFS=",", read -r name do - secret=$(LC_ALL=C tr -dc A-Za-z0-9 Date: Mon, 7 Sep 2026 07:54:30 +0000 Subject: [PATCH 28/38] fix(adhoc-sweep-fixes): 92 review findings across 38 files --- .../DashboardPage/cards/WelcomeHost/WelcomeHost.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/frontend/pages/DashboardPage/cards/WelcomeHost/WelcomeHost.tsx b/frontend/pages/DashboardPage/cards/WelcomeHost/WelcomeHost.tsx index 69a82e4550c..f0b7b38cb0d 100644 --- a/frontend/pages/DashboardPage/cards/WelcomeHost/WelcomeHost.tsx +++ b/frontend/pages/DashboardPage/cards/WelcomeHost/WelcomeHost.tsx @@ -7,6 +7,7 @@ import { NotificationContext } from "context/notification"; import { IHost, IHostResponse } from "interfaces/host"; import { IHostPolicy } from "interfaces/policy"; import hostAPI from "services/entities/hosts"; +import { AppContext } from "context/app"; import Spinner from "components/Spinner"; import Button from "components/buttons/Button"; @@ -22,7 +23,6 @@ interface IWelcomeHostCardProps { } const baseClass = "welcome-host"; -const HOST_ID = 1; const POLICY_PASS = "pass"; const POLICY_FAIL = "fail"; @@ -31,6 +31,8 @@ const WelcomeHost = ({ toggleAddHostsModal, }: IWelcomeHostCardProps): JSX.Element => { const { renderFlash } = useContext(NotificationContext); + const { currentUser } = useContext(AppContext); + const hostId = currentUser?.id_verified_host_id; const [refetchStartTime, setRefetchStartTime] = useState(null); const [currentPolicyShown, setCurrentPolicyShown] = useState(); const [showPolicyModal, setShowPolicyModal] = useState(false); @@ -46,8 +48,9 @@ const WelcomeHost = ({ refetch: fullyReloadHost, } = useQuery( ["host"], - () => hostAPI.loadHostDetails(HOST_ID), + () => hostAPI.loadHostDetails(hostId as number), { + enabled: !!hostId, retry: false, select: (data: IHostResponse) => data.host, onSuccess: (returnedHost) => { @@ -229,6 +232,7 @@ const WelcomeHost = ({ if (p.response) { return (