From dec257e0441e25d3f91ab53b3b0a2a3ad3726e61 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:08 +0000 Subject: [PATCH 01/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../migrations/tables/20241002104104_UpdateUninstallScript.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go b/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go index 3ab64c4e1df..f2053be0451 100644 --- a/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go +++ b/server/datastore/mysql/migrations/tables/20241002104104_UpdateUninstallScript.go @@ -6,6 +6,7 @@ import ( _ "embed" "encoding/hex" "fmt" + "log" "regexp" "strings" @@ -120,6 +121,8 @@ ON DUPLICATE KEY UPDATE return fmt.Errorf("failed to update uninstall script ID %d: %w", script.ID, err) } + } else { + log.Printf("WARNING: uninstall script content ID %d for software installer ID %d did not match expected pattern; skipping update, please verify uninstall script manually", scriptContentID, script.ID) } } From 8e9f644bd050e8c835b7448acc4f2df52ad723aa Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:10 +0000 Subject: [PATCH 02/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ...20250219100000_AddVPPAppsTeamsTimestamps.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/server/datastore/mysql/migrations/tables/20250219100000_AddVPPAppsTeamsTimestamps.go b/server/datastore/mysql/migrations/tables/20250219100000_AddVPPAppsTeamsTimestamps.go index 684e1cb80da..190eb0e624c 100644 --- a/server/datastore/mysql/migrations/tables/20250219100000_AddVPPAppsTeamsTimestamps.go +++ b/server/datastore/mysql/migrations/tables/20250219100000_AddVPPAppsTeamsTimestamps.go @@ -21,6 +21,24 @@ func Up_20250219100000(tx *sql.Tx) error { } } + // Guard against data drift: vpp_apps is expected to have at most one row per + // (platform, adam_id). If that assumption is violated, the backfill UPDATE below + // could nondeterministically apply timestamps from an arbitrary matching row, so + // we assert uniqueness before running it. + var dupCount int + if err := tx.QueryRow(` + SELECT COUNT(*) FROM ( + SELECT platform, adam_id + FROM vpp_apps + GROUP BY platform, adam_id + HAVING COUNT(*) > 1 + ) dups`).Scan(&dupCount); err != nil { + return fmt.Errorf("checking vpp_apps for duplicate platform/adam_id rows: %w", err) + } + if dupCount > 0 { + return fmt.Errorf("found %d duplicate (platform, adam_id) combinations in vpp_apps; refusing to backfill vpp_apps_teams timestamps to avoid nondeterministic results", dupCount) + } + // make a quick guess at created/updated timestamps; getting more exact timestamps requires looking at the activity // feed, which may have been purged, so that query will be available for admins to run manually _, err := tx.Exec(`UPDATE vpp_apps_teams vt From 0f60cbeb3060e688a56f7fa838ae0eb3b6813880 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:11 +0000 Subject: [PATCH 03/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../20260316120008_RenameActivitiesToActivityPast.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/datastore/mysql/migrations/tables/20260316120008_RenameActivitiesToActivityPast.go b/server/datastore/mysql/migrations/tables/20260316120008_RenameActivitiesToActivityPast.go index 32deeb19c2e..cdff89e2eaf 100644 --- a/server/datastore/mysql/migrations/tables/20260316120008_RenameActivitiesToActivityPast.go +++ b/server/datastore/mysql/migrations/tables/20260316120008_RenameActivitiesToActivityPast.go @@ -23,5 +23,15 @@ func Up_20260316120008(tx *sql.Tx) error { } func Down_20260316120008(tx *sql.Tx) error { + // Reverse the rename performed in Up_20260316120008, if it was applied. + // This provides a rollback path for the destructive RENAME TABLE above. + if !tableExists(tx, "activity_past") { + return nil + } + _, err := tx.Exec(`RENAME TABLE activity_past TO activities, activity_host_past TO host_activities`) + if err != nil { + return fmt.Errorf("revert rename of activities tables: %w", err) + } return nil } + From ca652bd5a8a065eb6cbb6934eaaa63bf0cf49864 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:13 +0000 Subject: [PATCH 04/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ...0326210603_UpdateSoftwareTitleNamesToFMANames.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/server/datastore/mysql/migrations/tables/20260326210603_UpdateSoftwareTitleNamesToFMANames.go b/server/datastore/mysql/migrations/tables/20260326210603_UpdateSoftwareTitleNamesToFMANames.go index 430140a3e6a..4e4b8c17161 100644 --- a/server/datastore/mysql/migrations/tables/20260326210603_UpdateSoftwareTitleNamesToFMANames.go +++ b/server/datastore/mysql/migrations/tables/20260326210603_UpdateSoftwareTitleNamesToFMANames.go @@ -17,6 +17,14 @@ func Up_20260326210603(tx *sql.Tx) error { // A later migration adds idx_software_bundle_identifier on software.bundle_identifier // so the hourly FMA sync UPDATE below (and the runtime equivalent in // UpsertMaintainedApp) is an indexed lookup instead of a full-table scan. + // + // WARNING: this UPDATE is destructive and irreversible. It overwrites + // software_titles.name and software.name in-place, and the Down migration + // below is a no-op. Anyone deploying this migration should take a backup + // of the software_titles and software tables (or a full database snapshot) + // before upgrading, in case the FMA data used here (fleet_maintained_apps.name) + // is later found to be wrong for some bundle_identifiers, since there is no + // automated way to restore the original osquery-reported names afterward. _, err := tx.Exec(` UPDATE software_titles st JOIN fleet_maintained_apps fma @@ -49,5 +57,10 @@ func Up_20260326210603(tx *sql.Tx) error { func Down_20260326210603(tx *sql.Tx) error { // Down migration is a no-op because we cannot reliably restore the original // osquery-reported names. The FMA names are the canonical/correct names anyway. + // + // Because this change is irreversible, operators should take a backup of the + // software_titles and software tables (or a full database snapshot) before + // running the Up migration, so that a manual restore is possible if the FMA + // data proves incorrect for any bundle_identifiers. return nil } From d548ee019f4a8cf6d6b38be9c10cf2abf8465af5 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:14 +0000 Subject: [PATCH 05/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ...60518194422_AddEncodingTypeToHostSCDData_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/server/datastore/mysql/migrations/tables/20260518194422_AddEncodingTypeToHostSCDData_test.go b/server/datastore/mysql/migrations/tables/20260518194422_AddEncodingTypeToHostSCDData_test.go index f1d96f9c91b..a12219255e2 100644 --- a/server/datastore/mysql/migrations/tables/20260518194422_AddEncodingTypeToHostSCDData_test.go +++ b/server/datastore/mysql/migrations/tables/20260518194422_AddEncodingTypeToHostSCDData_test.go @@ -15,6 +15,19 @@ func TestUp_20260518194422(t *testing.T) { // migration this row must still be readable, with encoding_type defaulting // to 0 (dense). denseBytes := []byte{0x82, 0x05} // bits 1, 7, 8, 10 set: hosts {1, 7, 8, 10} + // Verify the comment's claim against the actual bit positions before + // relying on it below: byte0=0x82=0b10000010 (bits 1,7), byte1=0x05= + // 0b00000101 (bits 0,2 -> global bits 8,10). + var setBits []int + for byteIdx, b := range denseBytes { + for bit := 0; bit < 8; bit++ { + if b&(1< Date: Mon, 7 Sep 2026 08:14:15 +0000 Subject: [PATCH 06/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../mysql/policies_queries_openframe_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/server/datastore/mysql/policies_queries_openframe_test.go b/server/datastore/mysql/policies_queries_openframe_test.go index 990071d719c..2891ef306e3 100644 --- a/server/datastore/mysql/policies_queries_openframe_test.go +++ b/server/datastore/mysql/policies_queries_openframe_test.go @@ -78,9 +78,16 @@ func TestOpenframePolicyQueryByIDTeamFence(t *testing.T) { _, err = ds.PoliciesByID(ctxA, []uint{polB.ID}) require.True(t, fleet.IsNotFound(err), "foreign policy in batch by-id must be NotFound, got %v", err) - // A mixed batch fails too — the foreign id is indistinguishable from a nonexistent one. + // NOTE(fail-closed, cross-team mixed batch): the current PoliciesByID implementation fails + // the entire request as NotFound when the id list mixes an owned id with a foreign one — the + // foreign id is indistinguishable from a nonexistent one. This differs from typical batch-fetch + // semantics where callers expect partial results for the ids they are authorized to see. This + // is documented here rather than silently relied upon: any caller that gathers ids from + // multiple sources (e.g. a UI multi-select spanning teams) must not assume PoliciesByID will + // return the subset it can access — it must ensure ids passed to PoliciesByID are pre-filtered + // to a single tenant scope, or treat NotFound from a mixed batch as a signal to retry per-id. _, err = ds.PoliciesByID(ctxA, []uint{polA.ID, polB.ID}) - require.True(t, fleet.IsNotFound(err), "mixed batch with foreign id must be NotFound, got %v", err) + require.True(t, fleet.IsNotFound(err), "mixed batch with foreign id must be NotFound (fail-closed; callers must not mix cross-team ids), got %v", err) }) t.Run("unpinned baseline: foreign reads still succeed", func(t *testing.T) { From 6f5cd9cc7e7af735bf7624cc1dbfbae2e5fcfd44 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:16 +0000 Subject: [PATCH 07/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/datastore/s3/bootstrap_package.go | 49 +++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/server/datastore/s3/bootstrap_package.go b/server/datastore/s3/bootstrap_package.go index 8e893bde836..74c865f0fa3 100644 --- a/server/datastore/s3/bootstrap_package.go +++ b/server/datastore/s3/bootstrap_package.go @@ -11,15 +11,54 @@ type BootstrapPackageStore struct { // NewBootstrapPackageStore creates a new instance with the given S3 config. func NewBootstrapPackageStore(config config.S3Config) (*BootstrapPackageStore, error) { // bootstrap packages use the same S3 config as software installers + commonStore, err := newInstallerBackedFileStore(config, bootstrapPackagePrefix, "bootstrap package") + if err != nil { + return nil, err + } + return &BootstrapPackageStore{ + commonStore, + }, nil +} + +CURRENT>>> + +Wait, I cannot introduce a call to a helper that doesn't exist in this file without breaking compilation, since `newInstallerBackedFileStore` must be defined somewhere. Given the instructions restrict me to this single file, I will define the shared helper in this file itself. + +<< Date: Mon, 7 Sep 2026 08:14:18 +0000 Subject: [PATCH 08/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/mdm/scep/depot/bolt/depot_test.go | 25 +++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/server/mdm/scep/depot/bolt/depot_test.go b/server/mdm/scep/depot/bolt/depot_test.go index aadd5b81f8d..71dd2e8417b 100644 --- a/server/mdm/scep/depot/bolt/depot_test.go +++ b/server/mdm/scep/depot/bolt/depot_test.go @@ -11,25 +11,32 @@ import ( ) // createDepot creates a Bolt database in a temporary location. -func createDB(mode os.FileMode, options *bolt.Options) *Depot { +func createDB(t *testing.T, mode os.FileMode, options *bolt.Options) *Depot { // Create temporary path. - f, _ := ioutil.TempFile("", "bolt-") + f, err := ioutil.TempFile("", "bolt-") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } f.Close() os.Remove(f.Name()) db, err := bolt.Open(f.Name(), mode, options) if err != nil { - panic(err.Error()) + t.Fatalf(err.Error()) } + t.Cleanup(func() { + db.Close() + os.Remove(f.Name()) + }) d, err := NewBoltDepot(db) if err != nil { - panic(err.Error()) + t.Fatalf(err.Error()) } return d } func TestDepot_Serial(t *testing.T) { - db := createDB(0o666, nil) + db := createDB(t, 0o666, nil) tests := []struct { name string want *big.Int @@ -53,7 +60,7 @@ func TestDepot_Serial(t *testing.T) { } func TestDepot_writeSerial(t *testing.T) { - db := createDB(0o666, nil) + db := createDB(t, 0o666, nil) tests := []struct { name string @@ -75,7 +82,7 @@ func TestDepot_writeSerial(t *testing.T) { } func TestDepot_incrementSerial(t *testing.T) { - db := createDB(0o666, nil) + db := createDB(t, 0o666, nil) tests := []struct { name string @@ -104,7 +111,7 @@ func TestDepot_incrementSerial(t *testing.T) { } func TestDepot_CreateOrLoadKey(t *testing.T) { - db := createDB(0o666, nil) + db := createDB(t, 0o666, nil) tests := []struct { bits int wantErr bool @@ -124,7 +131,7 @@ func TestDepot_CreateOrLoadKey(t *testing.T) { } func TestDepot_CreateOrLoadCA(t *testing.T) { - db := createDB(0o666, nil) + db := createDB(t, 0o666, nil) tests := []struct { wantErr bool }{ From cf8556b47af8d52f82ce2a8dee6e38d8c7cfc396 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:19 +0000 Subject: [PATCH 09/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/platform/mysql/testing_utils/testing_utils.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/platform/mysql/testing_utils/testing_utils.go b/server/platform/mysql/testing_utils/testing_utils.go index 02cda5c07d2..3659042ae6e 100644 --- a/server/platform/mysql/testing_utils/testing_utils.go +++ b/server/platform/mysql/testing_utils/testing_utils.go @@ -72,12 +72,12 @@ func TruncateTables(t testing.TB, db *sqlx.DB, logger *slog.Logger, nonEmptyTabl table_type = 'BASE TABLE' ` if err := sqlx.SelectContext(ctx, tx, &tables, sql); err != nil { - return err + return fmt.Errorf("selecting table names from information_schema: %w", err) } } if _, err := tx.ExecContext(ctx, `SET FOREIGN_KEY_CHECKS=0`); err != nil { - return err + return fmt.Errorf("disabling foreign key checks: %w", err) } for _, tbl := range tables { if nonEmptyTables[tbl] { @@ -87,11 +87,11 @@ func TruncateTables(t testing.TB, db *sqlx.DB, logger *slog.Logger, nonEmptyTabl return fmt.Errorf("cannot truncate table %s, it contains seed data from schema.sql", tbl) } if _, err := tx.ExecContext(ctx, "TRUNCATE TABLE "+tbl); err != nil { - return err + return fmt.Errorf("truncating table %s: %w", tbl, err) } } if _, err := tx.ExecContext(ctx, `SET FOREIGN_KEY_CHECKS=1`); err != nil { - return err + return fmt.Errorf("enabling foreign key checks: %w", err) } return nil }, logger)) From d992a826d3672a0cfb50bb1e2942fd0a15d7d6ea Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:20 +0000 Subject: [PATCH 10/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/policies/failing_policies.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/server/policies/failing_policies.go b/server/policies/failing_policies.go index 04756ecdb73..5361117e921 100644 --- a/server/policies/failing_policies.go +++ b/server/policies/failing_policies.go @@ -228,14 +228,10 @@ func makeTeamConfigCache(ds fleet.Datastore, globalIntgs fleet.Integrations) fun func makeDefaultTeamConfigCache(ds fleet.Datastore, globalIntgs fleet.Integrations, logger *slog.Logger) func(ctx context.Context) (FailingPolicyAutomationConfig, error) { var cached *FailingPolicyAutomationConfig - var cachedErr error return func(ctx context.Context) (FailingPolicyAutomationConfig, error) { - // Return cached result if already loaded - if cached != nil || cachedErr != nil { - if cachedErr != nil { - return FailingPolicyAutomationConfig{}, cachedErr - } + // Return cached result if already loaded successfully + if cached != nil { return *cached, nil } @@ -243,14 +239,12 @@ func makeDefaultTeamConfigCache(ds fleet.Datastore, globalIntgs fleet.Integratio var cfg FailingPolicyAutomationConfig defaultTeamConfig, err := ds.DefaultTeamConfig(ctx) if err != nil { - cachedErr = err logger.ErrorContext(ctx, "failed to get default team config", "err", err) return cfg, err } intgs, err := defaultTeamConfig.Integrations.MatchWithIntegrations(globalIntgs) if err != nil { - cachedErr = err logger.ErrorContext(ctx, "failed to match default team integrations", "err", err) return cfg, err } From c3b3ac4ccd2545bb65cfdd38ad7ad22991a8c058 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:21 +0000 Subject: [PATCH 11/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/service/async/async_policy.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/service/async/async_policy.go b/server/service/async/async_policy.go index 6b839ae9805..66be56545af 100644 --- a/server/service/async/async_policy.go +++ b/server/service/async/async_policy.go @@ -31,8 +31,11 @@ var maxRedisPolicyResultsPerHost = 1000 func (t *Task) RecordPolicyQueryExecutions(ctx context.Context, host *fleet.Host, results map[uint]*bool, ts time.Time, deferred bool, newlyPassingPolicyIDs []uint) error { cfg := t.taskConfigs[config.AsyncTaskPolicyMembership] if !cfg.Enabled { + if err := t.datastore.RecordPolicyQueryExecutions(ctx, host, results, ts, deferred, newlyPassingPolicyIDs); err != nil { + return err + } host.PolicyUpdatedAt = ts - return t.datastore.RecordPolicyQueryExecutions(ctx, host, results, ts, deferred, newlyPassingPolicyIDs) + return nil } keyList := fmt.Sprintf(policyPassHostKey, host.ID) @@ -275,3 +278,4 @@ func (t *Task) GetHostPolicyReportedAt(ctx context.Context, host *fleet.Host) ti } return host.PolicyUpdatedAt } + From acd7684671ce0a6b007305fb211105a31aab87cb Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:23 +0000 Subject: [PATCH 12/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/service/devices_url_auth_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/server/service/devices_url_auth_test.go b/server/service/devices_url_auth_test.go index 5b29db6db41..0389c5039a5 100644 --- a/server/service/devices_url_auth_test.go +++ b/server/service/devices_url_auth_test.go @@ -44,11 +44,19 @@ func TestAuthenticatedDeviceFallbackAuth(t *testing.T) { t.Run("fallback_to_uuid_auth_for_ios", func(t *testing.T) { // iOS device with UUID in URL - token auth fails, falls back to UUID auth + var tokenAuthCalled, uuidAuthCalled bool + var tokenAuthCalledBeforeUUIDAuth bool + ds.LoadHostByDeviceAuthTokenFunc = func(ctx context.Context, authToken string, ttl time.Duration) (*fleet.Host, error) { + tokenAuthCalled = true + if !uuidAuthCalled { + tokenAuthCalledBeforeUUIDAuth = true + } return nil, newNotFoundError() } ds.HostByUUIDFunc = func(ctx context.Context, uuid string) (*fleet.Host, error) { + uuidAuthCalled = true if uuid == "ios-device-uuid" { return &fleet.Host{ ID: 1, @@ -62,15 +70,26 @@ func TestAuthenticatedDeviceFallbackAuth(t *testing.T) { req := mockDeviceAuthRequest{Token: "ios-device-uuid"} _, err := middleware(context.Background(), req) require.NoError(t, err) + require.True(t, tokenAuthCalled, "expected token-based auth to be attempted before falling back to UUID auth") + require.True(t, uuidAuthCalled, "expected UUID-based auth to be attempted as fallback") + require.True(t, tokenAuthCalledBeforeUUIDAuth, "expected token-based auth to be attempted before UUID-based auth") }) t.Run("fallback_to_uuid_auth_for_ipados", func(t *testing.T) { // iPadOS device with UUID in URL - token auth fails, falls back to UUID auth + var tokenAuthCalled, uuidAuthCalled bool + var tokenAuthCalledBeforeUUIDAuth bool + ds.LoadHostByDeviceAuthTokenFunc = func(ctx context.Context, authToken string, ttl time.Duration) (*fleet.Host, error) { + tokenAuthCalled = true + if !uuidAuthCalled { + tokenAuthCalledBeforeUUIDAuth = true + } return nil, newNotFoundError() } ds.HostByUUIDFunc = func(ctx context.Context, uuid string) (*fleet.Host, error) { + uuidAuthCalled = true if uuid == "ipados-device-uuid" { return &fleet.Host{ ID: 2, @@ -84,6 +103,9 @@ func TestAuthenticatedDeviceFallbackAuth(t *testing.T) { req := mockDeviceAuthRequest{Token: "ipados-device-uuid"} _, err := middleware(context.Background(), req) require.NoError(t, err) + require.True(t, tokenAuthCalled, "expected token-based auth to be attempted before falling back to UUID auth") + require.True(t, uuidAuthCalled, "expected UUID-based auth to be attempted as fallback") + require.True(t, tokenAuthCalledBeforeUUIDAuth, "expected token-based auth to be attempted before UUID-based auth") }) t.Run("failure_when_both_auth_methods_fail", func(t *testing.T) { From 561406f860cb813c502d36d050bde5171e3b69ee Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:24 +0000 Subject: [PATCH 13/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/service/secret_variables.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/server/service/secret_variables.go b/server/service/secret_variables.go index 159c61e3838..706edeb9cd5 100644 --- a/server/service/secret_variables.go +++ b/server/service/secret_variables.go @@ -41,10 +41,14 @@ func (svc *Service) CreateSecretVariables(ctx context.Context, secretVariables [ &fleet.BadRequestError{Message: "Couldn't save secret variables. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key"}) } - // Preprocess: strip FLEET_SECRET_ prefix from variable names + // Preprocess: strip FLEET_SECRET_ prefix from variable names. + // Copy into a new slice so we don't mutate the caller-provided slice in place. + processedSecretVariables := make([]fleet.SecretVariable, len(secretVariables)) for i, secretVariable := range secretVariables { - secretVariables[i].Name = fleet.Preprocess(strings.TrimPrefix(secretVariable.Name, SecretVariablePrefix)) + processedSecretVariables[i] = secretVariable + processedSecretVariables[i].Name = fleet.Preprocess(strings.TrimPrefix(secretVariable.Name, SecretVariablePrefix)) } + secretVariables = processedSecretVariables for _, secretVariable := range secretVariables { if err := fleet.ValidateSecretVariableName(secretVariable.Name); err != nil { From e0391fde50972b8e2c182fecbd44607a094cf84e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:25 +0000 Subject: [PATCH 14/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/service/software.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/server/service/software.go b/server/service/software.go index ea0c265ac29..0beda4373aa 100644 --- a/server/service/software.go +++ b/server/service/software.go @@ -10,7 +10,6 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/ptr" ) ///////////////////////////////////////////////////////////////////////////////// @@ -189,22 +188,8 @@ func (svc *Service) SoftwareByID(ctx context.Context, id uint, teamID *uint, inc IncludeObserver: true, }) if err != nil { - if fleet.IsNotFound(err) && teamID == nil { - // here we use a global admin as filter because we want - // to check if the software version exists - filter := fleet.TeamFilter{User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}} - sw, err := svc.ds.SoftwareByID(ctx, id, teamID, includeCVEScores, &filter) - if err != nil { - // Not found anywhere - return nil, ctxerr.Wrap(ctx, err, "software not found for any team") - } - // Found, but user has no permission to hosts it's installed on. - // Instead of PermissionError, return a stub with the name. - stub := &fleet.Software{ - ID: id, - Name: sw.Name, - } - return stub, nil + if fleet.IsNotFound(err) { + return nil, ctxerr.Wrap(ctx, err, "getting software version by id") } return nil, ctxerr.Wrap(ctx, err, "getting software version by id") } From 27086738a569db8d7bb2c9e43460f054cb8551e4 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:26 +0000 Subject: [PATCH 15/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/vulnerabilities/nvd/tools/cvefeed/nvd/match_node.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/vulnerabilities/nvd/tools/cvefeed/nvd/match_node.go b/server/vulnerabilities/nvd/tools/cvefeed/nvd/match_node.go index 72d623045ac..918ec45e1cb 100644 --- a/server/vulnerabilities/nvd/tools/cvefeed/nvd/match_node.go +++ b/server/vulnerabilities/nvd/tools/cvefeed/nvd/match_node.go @@ -18,7 +18,6 @@ import ( "fmt" "strings" - "github.com/facebookincubator/flog" "github.com/fleetdm/fleet/v4/server/vulnerabilities/nvd/tools/cvefeed/nvd/schema" "github.com/fleetdm/fleet/v4/server/vulnerabilities/nvd/tools/wfn" ) @@ -53,8 +52,7 @@ func nodeMatcher(id string, node *schema.NVDCVEFeedJSON10DefNode) (wfn.Matcher, switch strings.ToUpper(node.Operator) { default: - flog.Warningf("%s: unknown operator, defaulting to OR: got %q", id, node.Operator) - fallthrough + return nil, fmt.Errorf("%s: unknown operator: got %q", id, node.Operator) case "OR": m = wfn.MatchAny(ms...) case "AND": From f56331bb175497c7bf2467abb7e8e23d64180fbe Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:27 +0000 Subject: [PATCH 16/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../oval/parsed/object_state_simple_value.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/vulnerabilities/oval/parsed/object_state_simple_value.go b/server/vulnerabilities/oval/parsed/object_state_simple_value.go index 8472fee0b33..a1f63e9e0ce 100644 --- a/server/vulnerabilities/oval/parsed/object_state_simple_value.go +++ b/server/vulnerabilities/oval/parsed/object_state_simple_value.go @@ -26,13 +26,19 @@ func NewObjectStateSimpleValue(dtype string, op string, val string) ObjectStateS return ObjectStateSimpleValue(fmt.Sprintf("%s|%s|%s", dtype, op, val)) } -func (sta ObjectStateSimpleValue) unpack() (DataType, OperationType, string) { - parts := strings.Split(string(sta), "|") - return NewDataType(parts[0]), NewOperationType(parts[1]), parts[2] +func (sta ObjectStateSimpleValue) unpack() (DataType, OperationType, string, error) { + parts := strings.SplitN(string(sta), "|", 3) + if len(parts) != 3 { + return "", "", "", fmt.Errorf("malformed ObjectStateSimpleValue: %q", string(sta)) + } + return NewDataType(parts[0]), NewOperationType(parts[1]), parts[2], nil } func (sta ObjectStateSimpleValue) Eval(other string) (bool, error) { - dType, op, val := sta.unpack() + dType, op, val, err := sta.unpack() + if err != nil { + return false, err + } for _, cType := range complexTypes { if dType == cType { From f381817656d7842a5e84e9490ceaf2f44d5a4fab Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:29 +0000 Subject: [PATCH 17/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- server/worker/macos_setup_assistant.go | 120 +++++++++++++------------ 1 file changed, 64 insertions(+), 56 deletions(-) diff --git a/server/worker/macos_setup_assistant.go b/server/worker/macos_setup_assistant.go index 7f55e36926b..2467d36999a 100644 --- a/server/worker/macos_setup_assistant.go +++ b/server/worker/macos_setup_assistant.go @@ -84,47 +84,51 @@ func (m *MacosSetupAssistant) Run(ctx context.Context, argsJSON json.RawMessage) } } -func (m *MacosSetupAssistant) runProfileChanged(ctx context.Context, args macosSetupAssistantArgs) error { - team, err := m.getTeamNoTeam(ctx, args.TeamID) - if err != nil { - if fleet.IsNotFound(err) { - // team doesn't exist anymore, nothing to do (another job was enqueued to - // take care of team deletion) - return nil - } - return ctxerr.Wrap(ctx, err, "get team") - } - - // get the team's mdm-enrolled hosts, assign the profile to all of that - // team's hosts serials. - serials, err := m.Datastore.ListMDMAppleDEPSerialsInTeam(ctx, args.TeamID) +// assignProfileForSerialsFn resolves the setup assistant profile uuid to use +// for a given team/org name, as part of the shared list-serials/screen-cooldowns/ +// assign-per-org sequence used by runProfileChanged and runProfileDeleted. +type assignProfileForSerialsFn func(ctx context.Context, team *fleet.Team, orgName string) (profUUID string, err error) + +// runAssignProfileToTeamSerials implements the shared logic used by +// runProfileChanged and runProfileDeleted: list the team's DEP-enrolled host +// serials, screen them for cooldowns, and assign the resolved profile per +// ABM organization. logPrefix is used to keep the existing per-caller log +// wording (e.g. "run profile changed" vs "run profile deleted"). +func (m *MacosSetupAssistant) runAssignProfileToTeamSerials( + ctx context.Context, + team *fleet.Team, + teamID *uint, + logPrefix string, + resolveProfile assignProfileForSerialsFn, +) error { + serials, err := m.Datastore.ListMDMAppleDEPSerialsInTeam(ctx, teamID) if err != nil { return ctxerr.Wrap(ctx, err, "list mdm dep serials in team") } if len(serials) > 0 { skipSerials, assignSerials, err := m.Datastore.ScreenDEPAssignProfileSerialsForCooldown(ctx, serials) if err != nil { - return ctxerr.Wrap(ctx, err, "run profile changed") + return ctxerr.Wrap(ctx, err, logPrefix) } if len(skipSerials) > 0 { // NOTE: the `dep_cooldown` job of the `integrations`` cron picks up the assignments // after the cooldown period is over - m.Log.InfoContext(ctx, "run profile changed: skipping assign profile for devices on cooldown", "serials", fmt.Sprintf("%s", skipSerials)) + m.Log.InfoContext(ctx, logPrefix+": skipping assign profile for devices on cooldown", "serials", fmt.Sprintf("%s", skipSerials)) } if len(assignSerials) == 0 { - m.Log.InfoContext(ctx, "run profile changed: no devices to assign profile") + m.Log.InfoContext(ctx, logPrefix+": no devices to assign profile") return nil } for orgName, serials := range assignSerials { - profUUID, _, err := m.DEPService.EnsureCustomSetupAssistantIfExists(ctx, team, orgName) + profUUID, err := resolveProfile(ctx, team, orgName) if err != nil { - return ctxerr.Wrapf(ctx, err, "ensure custom setup assistant for ABM org name %q", orgName) + return err } if profUUID == "" { - // the custom setup assistant profile may have been deleted since the job - // was enqueued, if so another job will take care of assigning the default - // profile to the hosts, nothing to do. + // the caller has already decided this is a no-op case for this org + // (e.g. the custom setup assistant profile may have been deleted + // since the job was enqueued), so skip assigning for this org. continue } @@ -133,13 +137,43 @@ func (m *MacosSetupAssistant) runProfileChanged(ctx context.Context, args macosS return ctxerr.Wrap(ctx, err, "assign profile") } if err := m.Datastore.UpdateHostDEPAssignProfileResponsesSameABM(ctx, resp); err != nil { - return ctxerr.Wrap(ctx, err, "worker: run profile changed") + return ctxerr.Wrap(ctx, err, "worker: "+logPrefix) } } } return nil } +func (m *MacosSetupAssistant) runProfileChanged(ctx context.Context, args macosSetupAssistantArgs) error { + team, err := m.getTeamNoTeam(ctx, args.TeamID) + if err != nil { + if fleet.IsNotFound(err) { + // team doesn't exist anymore, nothing to do (another job was enqueued to + // take care of team deletion) + return nil + } + return ctxerr.Wrap(ctx, err, "get team") + } + + // get the team's mdm-enrolled hosts, assign the profile to all of that + // team's hosts serials. + return m.runAssignProfileToTeamSerials(ctx, team, args.TeamID, "run profile changed", + func(ctx context.Context, team *fleet.Team, orgName string) (string, error) { + profUUID, _, err := m.DEPService.EnsureCustomSetupAssistantIfExists(ctx, team, orgName) + if err != nil { + return "", ctxerr.Wrapf(ctx, err, "ensure custom setup assistant for ABM org name %q", orgName) + } + if profUUID == "" { + // the custom setup assistant profile may have been deleted since the job + // was enqueued, if so another job will take care of assigning the default + // profile to the hosts, nothing to do. + return "", nil + } + return profUUID, nil + }, + ) +} + func (m *MacosSetupAssistant) runProfileDeleted(ctx context.Context, args macosSetupAssistantArgs) error { team, err := m.getTeamNoTeam(ctx, args.TeamID) if err != nil { @@ -170,45 +204,19 @@ func (m *MacosSetupAssistant) runProfileDeleted(ctx context.Context, args macosS // get the team's mdm-enrolled hosts, assign the profile to all of that // team's hosts serials. - serials, err := m.Datastore.ListMDMAppleDEPSerialsInTeam(ctx, args.TeamID) - if err != nil { - return ctxerr.Wrap(ctx, err, "list mdm dep serials in team") - } - if len(serials) > 0 { - skipSerials, assignSerials, err := m.Datastore.ScreenDEPAssignProfileSerialsForCooldown(ctx, serials) - if err != nil { - return ctxerr.Wrap(ctx, err, "run profile deleted") - } - if len(skipSerials) > 0 { - // NOTE: the `dep_cooldown` job of the `integrations`` cron picks up the assignments - // after the cooldown period is over - m.Log.InfoContext(ctx, "run profile deleted: skipping assign profile for devices on cooldown", "serials", fmt.Sprintf("%s", skipSerials)) - } - if len(assignSerials) == 0 { - m.Log.InfoContext(ctx, "run profile deleted: no devices to assign profile") - return nil - } - - for orgName, serials := range assignSerials { + return m.runAssignProfileToTeamSerials(ctx, team, args.TeamID, "run profile deleted", + func(ctx context.Context, team *fleet.Team, orgName string) (string, error) { profUUID, _, err := m.DEPService.EnsureDefaultSetupAssistant(ctx, team, orgName) if err != nil { - return ctxerr.Wrapf(ctx, err, "ensure default setup assistant for ABM organization %q", orgName) + return "", ctxerr.Wrapf(ctx, err, "ensure default setup assistant for ABM organization %q", orgName) } if profUUID == "" { // this should not happen, return an error - return ctxerr.Errorf(ctx, "default setup assistant profile uuid is empty for ABM organization %q", orgName) + return "", ctxerr.Errorf(ctx, "default setup assistant profile uuid is empty for ABM organization %q", orgName) } - - resp, err := m.DEPClient.AssignProfile(ctx, orgName, profUUID, serials...) - if err != nil { - return ctxerr.Wrap(ctx, err, "assign profile") - } - if err := m.Datastore.UpdateHostDEPAssignProfileResponsesSameABM(ctx, resp); err != nil { - return ctxerr.Wrap(ctx, err, "worker: run profile deleted") - } - } - } - return nil + return profUUID, nil + }, + ) } func (m *MacosSetupAssistant) runTeamDeleted(ctx context.Context, args macosSetupAssistantArgs) error { From 8f18b96a341880d39d5b116ac9ebe589cc3a41d4 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:30 +0000 Subject: [PATCH 18/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- tools/dibble/pkg/seed/vulns.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tools/dibble/pkg/seed/vulns.go b/tools/dibble/pkg/seed/vulns.go index e322c9d0ad5..7a6943f39b2 100644 --- a/tools/dibble/pkg/seed/vulns.go +++ b/tools/dibble/pkg/seed/vulns.go @@ -80,12 +80,13 @@ func Vulns(ctx context.Context, log Logger, opt VulnsOptions) Result { res.Errors = append(res.Errors, fmt.Errorf("read %s: %w", p.file, err)) continue } - if err := insertSoftware(ctx, db, p.platform, rows, p.count, opt.BatchSiz); err != nil { + inserted, err := insertSoftware(ctx, db, p.platform, rows, p.count, opt.BatchSiz) + if err != nil { res.Errors = append(res.Errors, fmt.Errorf("insert %s: %w", p.platform, err)) continue } - log.Printf("vulns: %d %s rows inserted from %s", p.count, p.platform, p.file) - res.Created += p.count + log.Printf("vulns: %d %s rows inserted from %s", inserted, p.platform, p.file) + res.Created += inserted } return res } @@ -143,9 +144,13 @@ func softwareChecksum(name, version, source, bundleID, release, arch, vendor, ex // The platform argument is unused by the INSERT itself — source values in // the CSVs (e.g. "apps", "deb_packages", "programs") already encode the // platform. It's kept on the signature so the caller can log it. -func insertSoftware(ctx context.Context, db *sql.DB, _platform string, rows [][]string, count, batch int) error { +// +// insertSoftware returns the number of rows actually queued for insertion, +// which may be less than `count` if some CSV rows were skipped for having +// fewer than 3 columns. +func insertSoftware(ctx context.Context, db *sql.DB, _platform string, rows [][]string, count, batch int) (int, error) { if len(rows) == 0 { - return errors.New("empty csv") + return 0, errors.New("empty csv") } // SET FOREIGN_KEY_CHECKS=0 is a session variable. Pin everything below // to a single connection so the disable, the inserts, and the restore @@ -153,12 +158,12 @@ func insertSoftware(ctx context.Context, db *sql.DB, _platform string, rows [][] // FK-disabled connection to an unrelated caller. conn, err := db.Conn(ctx) if err != nil { - return fmt.Errorf("acquire dedicated conn: %w", err) + return 0, fmt.Errorf("acquire dedicated conn: %w", err) } defer conn.Close() if _, err := conn.ExecContext(ctx, "SET FOREIGN_KEY_CHECKS=0"); err != nil { - return err + return 0, err } defer func() { // Use a fresh context so the restore still runs even if ctx was @@ -166,6 +171,7 @@ func insertSoftware(ctx context.Context, db *sql.DB, _platform string, rows [][] _, _ = conn.ExecContext(context.Background(), "SET FOREIGN_KEY_CHECKS=1") }() + inserted := 0 for i := 0; i < count; i += batch { end := i + batch if end > count { @@ -200,10 +206,11 @@ func insertSoftware(ctx context.Context, db *sql.DB, _platform string, rows [][] "(name, version, source, bundle_identifier, `release`, arch, vendor, extension_for, checksum) " + "VALUES " + strings.Join(placeholders, ",") if _, err := conn.ExecContext(ctx, stmt, args...); err != nil { - return err + return inserted, err } + inserted += len(placeholders) } - return nil + return inserted, nil } func csvField(row []string, i int) string { From f04fd72c1fd476d87198a2088690ad3fcc140d1f Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:31 +0000 Subject: [PATCH 19/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- tools/fleet-slackbot/system-prompt.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/fleet-slackbot/system-prompt.js b/tools/fleet-slackbot/system-prompt.js index 9b3b8397b46..1d4e5bf05d8 100644 --- a/tools/fleet-slackbot/system-prompt.js +++ b/tools/fleet-slackbot/system-prompt.js @@ -75,6 +75,8 @@ it-and-security/ ## Fleet YAML Schema +**IMPORTANT: The schema summary below is a reference guide only and may drift from the actual schema over time.** Before proposing any change, use \`read_gitops_file\` to read the real, current file(s) you are about to modify (and, where possible, a sibling example of the same file type) and treat their exact structure, key names, and field ordering as authoritative. If anything below conflicts with what you observe in the repo, the repo wins — see Rule 11. + Each fleet file (e.g., \`fleets/workstations.yml\`) has this structure: \`\`\`yaml @@ -341,7 +343,7 @@ queries: 8. **Preserve all existing content** when modifying a file. Only add/change the specific items requested. 9. **For fleet_maintained_apps**, use the slug format: \`app-name/platform\` (e.g., \`google-chrome/macos\`, \`slack/windows\`) 10. **Calendar events should default to false.** When adding or modifying policies, always set \`calendar_events_enabled: false\` unless the user explicitly requests otherwise. -11. **The \`it-and-security/\` directory is the authoritative source of truth.** The schemas above are reference guides, but if the actual files in the repo differ from these schemas (e.g., different key names, field ordering, or conventions), **always match the repo**. Study the provided file contents carefully and replicate their exact patterns, key names, formatting, and field ordering. Never rename existing keys to match the schema examples. +11. **The \`it-and-security/\` directory is the authoritative source of truth.** The schema sections above are only a hand-maintained reference and may be out of date or incomplete relative to the real GitOps schema. Always call \`read_gitops_file\` to inspect the actual files you are about to modify (and comparable existing files of the same type) before proposing a change, and if the actual files differ from these schemas (e.g., different key names, field ordering, or conventions), **always match the repo**. Study the provided file contents carefully and replicate their exact patterns, key names, formatting, and field ordering. Never rename existing keys to match the schema examples. ## Response Format From 180218f9dbfbae165bafca2c0f55dff24b86f20f Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:32 +0000 Subject: [PATCH 20/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- tools/github-manage/pkg/ghapi/cli.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tools/github-manage/pkg/ghapi/cli.go b/tools/github-manage/pkg/ghapi/cli.go index ad5d395b154..d852f107bbd 100644 --- a/tools/github-manage/pkg/ghapi/cli.go +++ b/tools/github-manage/pkg/ghapi/cli.go @@ -8,6 +8,10 @@ import ( ) // RunCommandAndReturnOutput runs a bash command, captures its output, and returns the output as a byte slice. +// +// Deprecated: This executes the given string via `bash -c`, which is prone to shell-injection +// if the command string is ever built from untrusted input. Prefer RunArgsAndReturnOutput, which +// executes a fixed argv without shell interpretation. func RunCommandAndReturnOutput(command string) ([]byte, error) { logger.Debugf("Running COMMAND: %s", command) cmd := exec.Command("bash", "-c", command) @@ -21,3 +25,20 @@ func RunCommandAndReturnOutput(command string) ([]byte, error) { } return out.Bytes(), nil } + +// RunArgsAndReturnOutput runs a command given as an explicit argv (name plus arguments), captures +// its output, and returns the output as a byte slice. Unlike RunCommandAndReturnOutput, this does +// not invoke a shell, so caller-supplied argument values cannot be interpreted as shell syntax. +func RunArgsAndReturnOutput(name string, args ...string) ([]byte, error) { + logger.Debugf("Running COMMAND: %s %v", name, args) + cmd := exec.Command(name, args...) + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + + if err := cmd.Run(); err != nil { + logger.Errorf("Error running command: %s", out.String()) + return nil, err + } + return out.Bytes(), nil +} From cabd6e26cd10f16184a3fda2ef6bd838f23635c9 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:33 +0000 Subject: [PATCH 21/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- tools/mdm/apple/apnspush/main.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/tools/mdm/apple/apnspush/main.go b/tools/mdm/apple/apnspush/main.go index 3f911580970..4a5bc1343f8 100644 --- a/tools/mdm/apple/apnspush/main.go +++ b/tools/mdm/apple/apnspush/main.go @@ -21,6 +21,7 @@ import ( "log/slog" "net/http" "os" + "strings" "github.com/WatchBeam/clock" "github.com/fleetdm/fleet/v4/pkg/fleethttp" @@ -33,6 +34,8 @@ import ( func main() { mysqlAddr := flag.String("mysql", "localhost:3306", "mysql address") + mysqlUsername := flag.String("mysql-username", "", "mysql username (defaults to the development 'fleet' user only when -mysql targets localhost)") + mysqlPassword := flag.String("mysql-password", "", "mysql password (defaults to the development 'insecure' password only when -mysql targets localhost)") serverPrivateKey := flag.String("server-private-key", "", "fleet server's private key (to decrypt MDM assets)") flag.Parse() @@ -52,13 +55,30 @@ func main() { serverPrivateKey = &truncatedServerPrivateKey } - // this matches the development config in /cmd/fleet/main.go + isLocalMySQL := strings.HasPrefix(*mysqlAddr, "localhost:") || strings.HasPrefix(*mysqlAddr, "127.0.0.1:") || strings.HasPrefix(*mysqlAddr, "[::1]:") + + username := *mysqlUsername + password := *mysqlPassword + if username == "" || password == "" { + if !isLocalMySQL { + log.Fatal("must provide -mysql-username and -mysql-password when -mysql does not target localhost") + } + // this matches the development config in /cmd/fleet/main.go, and is only used + // as a fallback when targeting a local development database. + if username == "" { + username = "fleet" + } + if password == "" { + password = "insecure" + } + } + cfg := config.MysqlConfig{ Protocol: "tcp", Address: *mysqlAddr, Database: "fleet", - Username: "fleet", - Password: "insecure", + Username: username, + Password: password, MaxOpenConns: 50, MaxIdleConns: 50, ConnMaxLifetime: 0, From 55a7770934a14b7487ddfc48abe39546df3f5d3c Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:34 +0000 Subject: [PATCH 22/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- tools/seed_data/queries/seed_queries.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/seed_data/queries/seed_queries.go b/tools/seed_data/queries/seed_queries.go index c7e1f00dd19..a055ef2da02 100644 --- a/tools/seed_data/queries/seed_queries.go +++ b/tools/seed_data/queries/seed_queries.go @@ -37,6 +37,12 @@ func main() { if err != nil { log.Fatal(err) //nolint:gocritic // ignore exitAfterDefer } + // Ensure foreign key checks are always re-enabled, even on early exit. + defer func() { + if _, err := db.Exec("SET FOREIGN_KEY_CHECKS=1"); err != nil { + log.Println(err) + } + }() // Prepare the insert statement stmtPrefix := "INSERT INTO `queries` (`saved`, `name`, `description`, `query`, `author_id`, `observer_can_run`, `team_id`, `team_id_char`, `platform`, `min_osquery_version`, `schedule_interval`, `automations_enabled`, `logging_type`, `discard_data`) VALUES " @@ -58,17 +64,12 @@ func main() { stmt := stmtPrefix + strings.Join(valueStrings, ",") + stmtSuffix _, err := db.Exec(stmt, valueArgs...) if err != nil { - log.Fatal(err) + log.Fatal(err) //nolint:gocritic // ignore exitAfterDefer } fmt.Printf("Inserted batch %d/%d\n", batch+1, totalRecords/batchSize) } - // Re-enable foreign key checks - _, err = db.Exec("SET FOREIGN_KEY_CHECKS=1") - if err != nil { - log.Fatal(err) - } - fmt.Println("Finished inserting 1 million records.") } + From ace6544810a2e3ca09c57a4fb87945c7bd0b20cc Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:35 +0000 Subject: [PATCH 23/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../api/controllers/account/update-profile.js | 40 +++++++------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/website/api/controllers/account/update-profile.js b/website/api/controllers/account/update-profile.js index 03b4d7bbdb6..cff31e87ec7 100644 --- a/website/api/controllers/account/update-profile.js +++ b/website/api/controllers/account/update-profile.js @@ -35,6 +35,11 @@ module.exports = { description: 'The provided email address is already in use.', }, + emailChangeNotSupported: { + statusCode: 400, + description: 'Changing this user\'s email address is not currently supported, because it would require re-confirmation.', + }, + }, @@ -65,6 +70,14 @@ module.exports = { desiredEmailEffect = 'begin-change'; } + // The email confirmation feature is unused and has not been adapted for fleetdm.com, + // so if this request would require sending a confirmation email for a pending email + // address change, fail early with a clear error instead of silently leaving the + // account in a broken 'change-requested' state. + if (desiredEmailEffect === 'begin-change' || desiredEmailEffect === 'modify-pending-change') { + throw 'emailChangeNotSupported'; + } + // If the email address is changing, make sure it is not already being used. if (_.contains(['begin-change', 'change-immediately', 'modify-pending-change'], desiredEmailEffect)) { @@ -101,17 +114,6 @@ module.exports = { }); break; - // Begin new email change, or modify a pending email change - case 'begin-change': - case 'modify-pending-change': - _.extend(valuesToSet, { - emailChangeCandidate: newEmailAddress, - emailProofToken: await sails.helpers.strings.random('url-friendly'), - emailProofTokenExpiresAt: Date.now() + sails.config.custom.emailProofTokenTTL, - emailStatus: 'change-requested' - }); - break; - // Cancel pending email change case 'cancel-pending-change': _.extend(valuesToSet, { @@ -150,22 +152,8 @@ module.exports = { } } - // If an email address change was requested, and re-confirmation is required, - // send the "confirm account" email. - if (desiredEmailEffect === 'begin-change' || desiredEmailEffect === 'modify-pending-change') { - throw new Error('Not yet supported: the email confirmation feature is unused and has not been adapted for fleetdm.com. This error should never be displayed.'); - // await sails.helpers.sendTemplateEmail.with({ - // to: newEmailAddress, - // subject: 'Your account has been updated', - // template: 'email-verify-new-email', - // templateData: { - // fullName: fullName||this.req.me.fullName, - // token: valuesToSet.emailProofToken - // } - // }); - } - } }; + From 1ba9630bab088ccc9cfea8fc6ecf9d6e740c1419 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:36 +0000 Subject: [PATCH 24/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../api/controllers/android-proxy/create-enterprise-webapp.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/api/controllers/android-proxy/create-enterprise-webapp.js b/website/api/controllers/android-proxy/create-enterprise-webapp.js index b2d400b7525..0671791527b 100644 --- a/website/api/controllers/android-proxy/create-enterprise-webapp.js +++ b/website/api/controllers/android-proxy/create-enterprise-webapp.js @@ -106,7 +106,7 @@ module.exports = { }).intercept({ status: 400 }, (err) => { return {'invalidWebApp': `Attempted to create a webApp with an invalid value for an Android enterprise (${androidEnterpriseId}): ${err}`}; }).intercept((err)=>{ - return new Error(`When attempting to create a webapp for an Android enterprise (${androidEnterpriseId}), an error occurred. Error: ${require('util').inspect(err)}`); + return new Error(`When attempting to create a webapp for an Android enterprise (${androidEnterpriseId}), an error occurred. Error: ${err.message}`); }); @@ -117,3 +117,4 @@ module.exports = { }; +
From 66a1ccf77feec035e92fc166fa908107f18d26ba Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:37 +0000 Subject: [PATCH 25/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../receive-redirect-from-microsoft.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js b/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js index 006f555a53b..eb02154e3e4 100644 --- a/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js +++ b/website/api/controllers/microsoft-proxy/receive-redirect-from-microsoft.js @@ -39,6 +39,10 @@ module.exports = { fn: async function ({tenant, state, error, error_description}) {// eslint-disable-line camelcase + // Whether verbose debug logging of raw Microsoft API response bodies is enabled. + // Controlled via the MICROSOFT_PROXY_VERBOSE_LOGGING environment variable rather than a hardcoded Fleet instance URL. + let isVerboseDebugLoggingEnabled = !!sails.config.custom.microsoftProxyVerboseLogging; + // If an error or error_description are provided, then the admin did not consent, and we will return a 200 response. if(error || error_description) {// eslint-disable-line camelcase // If an admin did not consent (or a user who started connecting the integration does not have admin permissions), try to match the provided state to a MicrosoftComplianceTenant record, and redirect to that. @@ -97,7 +101,7 @@ module.exports = { return {redirect: fleetInstanceUrlToRedirectTo }; }); // Log responses from Micrsoft APIs for Fleet's integration - if(informationAboutThisTenant.fleetInstanceUrl === 'https://dogfood.fleetdm.com') { + if(isVerboseDebugLoggingEnabled) { sails.log.info(`Microsoft proxy: receive-redirect-from-microsoft provisioned a new tenant: ${complianceTenantProvisionResponse.body}`); } // Example response: @@ -146,7 +150,7 @@ module.exports = { }); // Log responses from Micrsoft APIs for Fleet's integration - if(informationAboutThisTenant.fleetInstanceUrl === 'https://dogfood.fleetdm.com') { + if(isVerboseDebugLoggingEnabled) { sails.log.info(`Microsoft proxy: receive-redirect-from-microsoft created/found a compliance policy: ${createPolicyResponse.body}`); } @@ -191,7 +195,7 @@ module.exports = { }); // Log responses from Micrsoft APIs for Fleet's integration. - if(informationAboutThisTenant.fleetInstanceUrl === 'https://dogfood.fleetdm.com') { + if(isVerboseDebugLoggingEnabled) { sails.log.info(`Microsoft proxy: receive-redirect-from-microsoft created/found a entra ID group: ${groupResponse.body}`); } // Get the ID returned in the response. @@ -240,7 +244,7 @@ module.exports = { // } // Log responses from Micrsoft APIs for Fleet's integration. - if(informationAboutThisTenant.fleetInstanceUrl === 'https://dogfood.fleetdm.com') { + if(isVerboseDebugLoggingEnabled) { sails.log.info(`Microsoft proxy: receive-redirect-from-microsoft assigned a compliance policy: ${assignPolicyResponse.body}`); } From 1496e905501547b0377fc23a1f4a623b2b77cf3a Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:38 +0000 Subject: [PATCH 26/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../provision-new-fleet-sandbox-instance.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/website/api/helpers/fleet-sandbox-cloud-provisioner/provision-new-fleet-sandbox-instance.js b/website/api/helpers/fleet-sandbox-cloud-provisioner/provision-new-fleet-sandbox-instance.js index c672a3e50f0..0f86f703777 100644 --- a/website/api/helpers/fleet-sandbox-cloud-provisioner/provision-new-fleet-sandbox-instance.js +++ b/website/api/helpers/fleet-sandbox-cloud-provisioner/provision-new-fleet-sandbox-instance.js @@ -90,6 +90,9 @@ module.exports = { } // Start polling the /healthz endpoint of the created Fleet Sandbox instance, once it returns a 200 response, we'll continue. + // Note: the second argument to .until() is the overall timeout (in ms) for this polling operation, not the poll interval. + // We bound this to 5 minutes so that a permanently-unhealthy sandbox instance cannot hang this request indefinitely. + const FIVE_MINUTES_IN_MS = (5*60*1000); await sails.helpers.flow.until( async()=>{ let healthCheckResponse = await sails.helpers.http.sendHttpRequest('GET', cloudProvisionerResponseData.URL+'/healthz') .timeout(5000) @@ -99,7 +102,7 @@ module.exports = { if(healthCheckResponse) { return true; } - }, 10000)//∞ + }, FIVE_MINUTES_IN_MS) .intercept('tookTooLong', ()=>{ return new Error('This newly provisioned Fleet Sandbox instance (for '+emailAddress+') is taking too long to respond with a 2xx status code, even after repeatedly polling the health check endpoint. Note that failed requests and non-2xx responses from the health check endpoint were ignored during polling. Search for a bit of non-dynamic text from this error message in the fleetdm.com source code for more info on exactly how this polling works.'); }); From c7a6ea17f837a1cff96f98fb65dc5a69c2233c6a Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:39 +0000 Subject: [PATCH 27/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- website/api/policies/is-cloud-customer.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/website/api/policies/is-cloud-customer.js b/website/api/policies/is-cloud-customer.js index 8888def5de5..e67a42c22b9 100644 --- a/website/api/policies/is-cloud-customer.js +++ b/website/api/policies/is-cloud-customer.js @@ -8,11 +8,20 @@ * https://sailsjs.com/docs/concepts/policies * https://sailsjs.com/docs/concepts/policies/access-control-and-permissions */ +const crypto = require('crypto'); + module.exports = async function (req, res, proceed) { // If an MS API KEY header was provided, check to see if it matches the entraSharedSecret. if (req.get('MS-API-KEY')) { - if([sails.config.custom.cloudCustomerCompliancePartnerSharedSecret, sails.config.custom.alternateCompliancePartnerSharedSecret].includes(req.get('MS-API-KEY'))){ + let providedKey = Buffer.from(req.get('MS-API-KEY')); + let matchesSecret = [sails.config.custom.cloudCustomerCompliancePartnerSharedSecret, sails.config.custom.alternateCompliancePartnerSharedSecret].some((configuredSecret) => { + if (!configuredSecret) { return false; } + let configuredKey = Buffer.from(configuredSecret); + if (configuredKey.length !== providedKey.length) { return false; } + return crypto.timingSafeEqual(providedKey, configuredKey); + }); + if (matchesSecret) { return proceed(); } } From 2940880124dd57e3cc86569fd808fa17181ca4e3 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:40 +0000 Subject: [PATCH 28/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- website/scripts/send-trial-usage-information-to-crm.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/scripts/send-trial-usage-information-to-crm.js b/website/scripts/send-trial-usage-information-to-crm.js index 3b25eb7bd41..03c75a02121 100644 --- a/website/scripts/send-trial-usage-information-to-crm.js +++ b/website/scripts/send-trial-usage-information-to-crm.js @@ -93,7 +93,7 @@ module.exports = { contactSource: 'Website - Sign up', trialInstanceUsageDetails: trialInstanceUsageDetails }).tolerate((err)=>{ - sails.log.warn(`When reporting usage information about a Render trial instance (slug: ${renderTrial.slug}), an error occured when updating/creating a Salesforce contact/account. Full error: ${require('util').inspect(err)}`); + sails.log.warn(`When reporting usage information about a Render trial instance (slug: ${renderTrial.slug}), an error occured when updating/creating a Salesforce contact/account. Error message: ${err && err.message}`); }); });// After each Render trial Instance From 0cb3ccc39ebbe6e58575ad3f29d6061ae5111581 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:41 +0000 Subject: [PATCH 29/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .github/steps/sign-windows-package/action.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/steps/sign-windows-package/action.yml b/.github/steps/sign-windows-package/action.yml index 333e6ed9885..01b59cb59ea 100644 --- a/.github/steps/sign-windows-package/action.yml +++ b/.github/steps/sign-windows-package/action.yml @@ -40,7 +40,7 @@ runs: echo "BINARY_PATH=$BinaryPath" >> $env:GITHUB_ENV - name: Sign Windows Executable - uses: azure/trusted-signing-action@v0.5.0 + uses: azure/trusted-signing-action@95de1e51cbb1a115f0e4f47ba193da2ba0a4a806 # v0.5.0 with: azure-tenant-id: ${{ inputs.azure_tenant_id }} azure-client-id: ${{ inputs.azure_client_id }} @@ -83,3 +83,4 @@ runs: } Write-Host "Signature verified successfully!" + From c13645128b2e9c855333cd4e9258702c44ffd236 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:42 +0000 Subject: [PATCH 30/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .github/workflows/sync-upstream.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sync-upstream.yml b/.github/workflows/sync-upstream.yml index 7a5d9f0625c..b28a7817742 100644 --- a/.github/workflows/sync-upstream.yml +++ b/.github/workflows/sync-upstream.yml @@ -15,6 +15,7 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} BRANCH: sync/upstream-main + MAX_CHANGED_FILES: 500 steps: - uses: actions/checkout@v4 with: @@ -37,6 +38,13 @@ jobs: exit 0 fi + CHANGED_FILES=$(git diff --name-only origin/main...upstream/main | wc -l) + echo "::notice::upstream/main differs from origin/main by $CHANGED_FILES file(s)" + if [ "$CHANGED_FILES" -gt "$MAX_CHANGED_FILES" ]; then + echo "::error::refusing to sync — $CHANGED_FILES changed files exceeds MAX_CHANGED_FILES ($MAX_CHANGED_FILES); manual review required" + exit 1 + fi + git checkout -B "$BRANCH" origin/main if ! git merge --no-ff --no-edit upstream/main; then echo "::notice::merge has conflicts — skipping, will retry next run" @@ -46,4 +54,4 @@ jobs: gh pr create --repo "$GITHUB_REPOSITORY" --base main --head "$BRANCH" \ --title "Sync from Fork" \ - --body "Automatic weekly sync from \`fleetdm/fleet@main\`." \ No newline at end of file + --body "Automatic weekly sync from \`fleetdm/fleet@main\`." From 6e5480e4e3356a9fb091643a7e3c83b50c411b2e Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:43 +0000 Subject: [PATCH 31/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../java/com/fleetdm/agent/KeystoreManager.kt | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/android/app/src/main/java/com/fleetdm/agent/KeystoreManager.kt b/android/app/src/main/java/com/fleetdm/agent/KeystoreManager.kt index c37de9438cd..4ee90e6a769 100644 --- a/android/app/src/main/java/com/fleetdm/agent/KeystoreManager.kt +++ b/android/app/src/main/java/com/fleetdm/agent/KeystoreManager.kt @@ -8,6 +8,8 @@ import android.security.keystore.KeyGenParameterSpec import android.security.keystore.KeyProperties import android.util.Base64 import java.security.KeyStore +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference object KeystoreManager { private const val ANDROID_KEYSTORE = "AndroidKeyStore" @@ -17,28 +19,32 @@ object KeystoreManager { private const val IV_SEPARATOR = "]" // Test mode uses in-memory key instead of Android Keystore - private var testMode = false - private var testKey: SecretKey? = null + private val testMode = AtomicBoolean(false) + private val testKey = AtomicReference(null) /** * Enables test mode which uses an in-memory key instead of Android Keystore. * This allows unit tests to run without Android's hardware-backed keystore. */ + @Synchronized fun enableTestMode() { - testMode = true - testKey = KeyGenerator.getInstance("AES").apply { - init(256) - }.generateKey() + testKey.set( + KeyGenerator.getInstance("AES").apply { + init(256) + }.generateKey(), + ) + testMode.set(true) } + @Synchronized fun disableTestMode() { - testMode = false - testKey = null + testMode.set(false) + testKey.set(null) } private fun getOrCreateKey(): SecretKey { - if (testMode) { - return testKey ?: error("Test mode enabled but no test key available") + if (testMode.get()) { + return testKey.get() ?: error("Test mode enabled but no test key available") } val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) From 68efa2ff3363e7ca57d8c83b1fd8e43136330d24 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:44 +0000 Subject: [PATCH 32/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- cmd/fleet/mail.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cmd/fleet/mail.go b/cmd/fleet/mail.go index 03ba38b7e9e..3615d9ca682 100644 --- a/cmd/fleet/mail.go +++ b/cmd/fleet/mail.go @@ -2,6 +2,7 @@ package main import ( "context" + "fmt" "log/slog" "github.com/fleetdm/fleet/v4/server/config" @@ -20,10 +21,10 @@ func shouldForceSMTPBackend(appCfg *fleet.AppConfig, emailBackend string) bool { emailBackend != "" } -// initMailService configures the mail service. Mail is best-effort at startup: -// a construction failure is logged and the (possibly nil) service is returned -// rather than aborting boot. -func initMailService(ctx context.Context, cfg config.FleetConfig, appCfg *fleet.AppConfig, logger *slog.Logger) fleet.MailService { +// initMailService configures the mail service. If construction fails, the +// error is logged and returned so callers can fail fast at startup instead of +// receiving a nil service silently. +func initMailService(ctx context.Context, cfg config.FleetConfig, appCfg *fleet.AppConfig, logger *slog.Logger) (fleet.MailService, error) { if shouldForceSMTPBackend(appCfg, cfg.Email.EmailBackend) { // Force-load the SMTP implementation by clearing the configured backend. cfg.Email.EmailBackend = "" @@ -33,6 +34,7 @@ func initMailService(ctx context.Context, cfg config.FleetConfig, appCfg *fleet. mailService, err := mail.NewService(cfg) if err != nil { logger.ErrorContext(ctx, "failed to configure mailing service", "err", err) + return nil, fmt.Errorf("failed to configure mailing service: %w", err) } - return mailService + return mailService, nil } From d6844efcf57483571687209019f9d05b0310c3ad Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:45 +0000 Subject: [PATCH 33/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- cmd/osv-processor/sync-and-detect-changes.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/osv-processor/sync-and-detect-changes.sh b/cmd/osv-processor/sync-and-detect-changes.sh index 1159512c92f..a22ee094529 100755 --- a/cmd/osv-processor/sync-and-detect-changes.sh +++ b/cmd/osv-processor/sync-and-detect-changes.sh @@ -85,7 +85,11 @@ fi cd "$REPO_DIR" TODAY_UTC=$(date -u +%Y-%m-%d) -YESTERDAY_UTC=$(date -u -v-1d +%Y-%m-%d 2>/dev/null || date -u -d "yesterday" +%Y-%m-%d) +YESTERDAY_UTC=$(date -u -v-1d +%Y-%m-%d 2>/dev/null || date -u -d "yesterday" +%Y-%m-%d 2>/dev/null || true) +if [ -z "$YESTERDAY_UTC" ]; then + echo "ERROR: Unable to compute yesterday's date; 'date' binary supports neither -v (BSD) nor -d (GNU) flags." >&2 + exit 1 +fi # Get files changed today (since midnight UTC today) git log --since="${TODAY_UTC}T00:00:00Z" --name-only --pretty="" -- osv/cve \ @@ -119,3 +123,4 @@ echo "TODAY_COUNT=$TODAY_COUNT" echo "YESTERDAY_COUNT=$YESTERDAY_COUNT" exit 0 + From 37053eda93121526f74a5bd3c36ee93c34870dac Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:46 +0000 Subject: [PATCH 34/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ee/cis/macos-14/test/scripts/CIS_6.1.1.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ee/cis/macos-14/test/scripts/CIS_6.1.1.sh b/ee/cis/macos-14/test/scripts/CIS_6.1.1.sh index 2b2bdc687e7..e28f84c2198 100755 --- a/ee/cis/macos-14/test/scripts/CIS_6.1.1.sh +++ b/ee/cis/macos-14/test/scripts/CIS_6.1.1.sh @@ -1,5 +1,12 @@ #!/bin/bash +# Get the current console user (the actual logged-in user), excluding root and loginwindow +CURRENT_USER=$(/usr/bin/stat -f "%Su" /dev/console) + +if [[ -z "$CURRENT_USER" || "$CURRENT_USER" == "root" ]]; then + echo "Unable to determine a valid non-root console user. Aborting." + exit 1 +fi + +/usr/bin/sudo -u "$CURRENT_USER" /usr/bin/defaults write "/Users/$CURRENT_USER/Library/Preferences/.GlobalPreferences.plist" AppleShowAllExtensions -bool true -# For QA: Replace with your test user -/usr/bin/sudo -u /usr/bin/defaults write /Users//Library/Preferences/.GlobalPreferences.plist AppleShowAllExtensions -bool true From 4e17c8479b3ba36b24ab9483dfc32fae2fe8b752 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:47 +0000 Subject: [PATCH 35/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ee/fleet-agent-downloader/config/session.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ee/fleet-agent-downloader/config/session.js b/ee/fleet-agent-downloader/config/session.js index 7b21600ed77..ce19b74a628 100644 --- a/ee/fleet-agent-downloader/config/session.js +++ b/ee/fleet-agent-downloader/config/session.js @@ -17,8 +17,17 @@ module.exports.session = { * Replace at your own risk in production-- you will invalidate the cookies * * of your users, forcing them to log in again. * * * + * The secret must be provided via the SESSION_SECRET environment variable. * + * There is no committed literal fallback -- if SESSION_SECRET is not set, * + * the app will fail to start rather than sign sessions with a known value. * + * * ***************************************************************************/ - secret: 'DUMMY_SECRET_REPLACED_IN_PROD', + secret: (function () { + if (!process.env.SESSION_SECRET) { + throw new Error('SESSION_SECRET environment variable must be set (no default secret is provided).'); + } + return process.env.SESSION_SECRET; + })(), /*************************************************************************** @@ -37,3 +46,4 @@ module.exports.session = { // }, }; + From 8a5c419f46d0112196d1da9ea0b5c62cc2b5d519 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:48 +0000 Subject: [PATCH 36/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../homebrew/scripts/microsoft-edge-install.sh | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/ee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.sh b/ee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.sh index d5f0610f1c6..30b2b835271 100755 --- a/ee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.sh +++ b/ee/maintained-apps/inputs/homebrew/scripts/microsoft-edge-install.sh @@ -52,19 +52,14 @@ hdiutil detach "$MOUNT_POINT" # Clean up any backup files that might exist from previous failed installations # This ensures we start with a clean slate cleanup_backup_files() { - # Clean up backup in the installer's temp directory + # Clean up backup in the installer's temp directory only. + # Scoped to the known install/backup location actually used by this + # installer to avoid a broad, unconditional find+rm -rf across shared + # temp trees (/tmp, /var/folders, /private/var/folders). if [ -d "$TMPDIR/Microsoft Edge.app.bkp" ]; then echo "Removing existing backup file: $TMPDIR/Microsoft Edge.app.bkp" sudo rm -rf "$TMPDIR/Microsoft Edge.app.bkp" 2>/dev/null || true fi - - # Search for backup files in all common temp locations - # Use -exec to avoid pipe subshell issues - for search_base in /tmp /var/folders /private/var/folders; do - if [ -d "$search_base" ]; then - find "$search_base" -type d -name "Microsoft Edge.app.bkp" -exec sudo rm -rf {} + 2>/dev/null || true - fi - done } # copy to the applications folder @@ -92,3 +87,4 @@ else fi + From 553829519ef689051ec54b4b10018c25583a5061 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:49 +0000 Subject: [PATCH 37/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ee/orbit/pkg/securehw/securehw_tpm.go | 102 ++++++++++++++------------ 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/ee/orbit/pkg/securehw/securehw_tpm.go b/ee/orbit/pkg/securehw/securehw_tpm.go index 8a2b4087d6e..f5df5b8e23e 100644 --- a/ee/orbit/pkg/securehw/securehw_tpm.go +++ b/ee/orbit/pkg/securehw/securehw_tpm.go @@ -52,14 +52,22 @@ func NewTestSecureHW(device transport.TPMCloser, metadataDir string, logger zero }, nil } -// CreateKey partially implements SecureHW. -func (t *tpm2SecureHW) CreateKey() (Key, error) { - t.logger.Info().Msg("creating new ECC key in TPM") - +// withParentKey creates a transient parent key, invokes fn with its handle, and +// guarantees the parent key handle is flushed afterwards regardless of the +// outcome of fn or any future error paths added to fn. +func (t *tpm2SecureHW) withParentKey(fn func(parentKeyHandle tpm2.NamedHandle) error) error { parentKeyHandle, err := t.createParentKey() if err != nil { - return nil, fmt.Errorf("get or create TPM parent key: %w", err) + return err } + defer t.flushHandle(parentKeyHandle.Handle, "parent") + + return fn(parentKeyHandle) +} + +// CreateKey partially implements SecureHW. +func (t *tpm2SecureHW) CreateKey() (Key, error) { + t.logger.Info().Msg("creating new ECC key in TPM") curveID, curveName := t.selectBestECCCurve() t.logger.Info().Str("curve", curveName).Msg("selected ECC curve for key creation") @@ -88,33 +96,36 @@ func (t *tpm2SecureHW) CreateKey() (Key, error) { ), }) - // Create the key under the transient parent - t.logger.Debug().Msg("creating child key") - createKey, err := tpm2.Create{ - ParentHandle: parentKeyHandle, - InPublic: eccTemplate, - }.Execute(t.device) - if err != nil { - // Flush the parent key before returning error - t.flushHandle(parentKeyHandle.Handle, "parent") - return nil, fmt.Errorf("create child key: %w", err) - } + var createKey *tpm2.CreateResponse + var loadedKey *tpm2.LoadResponse + + err := t.withParentKey(func(parentKeyHandle tpm2.NamedHandle) error { + // Create the key under the transient parent + t.logger.Debug().Msg("creating child key") + var err error + createKey, err = tpm2.Create{ + ParentHandle: parentKeyHandle, + InPublic: eccTemplate, + }.Execute(t.device) + if err != nil { + return fmt.Errorf("create child key: %w", err) + } - t.logger.Debug().Msg("Loading created key") - loadedKey, err := tpm2.Load{ - ParentHandle: parentKeyHandle, - InPrivate: createKey.OutPrivate, - InPublic: createKey.OutPublic, - }.Execute(t.device) + t.logger.Debug().Msg("Loading created key") + loadedKey, err = tpm2.Load{ + ParentHandle: parentKeyHandle, + InPrivate: createKey.OutPrivate, + InPublic: createKey.OutPublic, + }.Execute(t.device) + if err != nil { + return fmt.Errorf("load key: %w", err) + } + return nil + }) if err != nil { - // Flush the parent key before returning error - t.flushHandle(parentKeyHandle.Handle, "parent") - return nil, fmt.Errorf("load key: %w", err) + return nil, err } - // Flush the parent key as it's no longer needed - t.flushHandle(parentKeyHandle.Handle, "parent") - t.logger.Debug(). Str("handle", fmt.Sprintf("0x%x", loadedKey.ObjectHandle)). Msg("key loaded successfully") @@ -276,31 +287,30 @@ func (t *tpm2SecureHW) LoadKey() (Key, error) { return nil, err } + var loadedKey *tpm2.LoadResponse + // Get the parent key handle. // // NOTE: createParentKey calls CreatePrimary which creates the parent key // deterministically so this can be called when loadind a child key. - parentKeyHandle, err := t.createParentKey() - if err != nil { - return nil, fmt.Errorf("get parent key: %w", err) - } - - // Load the key using the parent handle. - t.logger.Debug().Uint32("parent_handle", uint32(parentKeyHandle.Handle)).Msg("loading parent key") - loadedKey, err := tpm2.Load{ - ParentHandle: parentKeyHandle, - InPrivate: *private, - InPublic: *public, - }.Execute(t.device) + err = t.withParentKey(func(parentKeyHandle tpm2.NamedHandle) error { + // Load the key using the parent handle. + t.logger.Debug().Uint32("parent_handle", uint32(parentKeyHandle.Handle)).Msg("loading parent key") + var loadErr error + loadedKey, loadErr = tpm2.Load{ + ParentHandle: parentKeyHandle, + InPrivate: *private, + InPublic: *public, + }.Execute(t.device) + if loadErr != nil { + return fmt.Errorf("load parent key: %w", loadErr) + } + return nil + }) if err != nil { - // Flush the parent key before returning error - t.flushHandle(parentKeyHandle.Handle, "parent") - return nil, fmt.Errorf("load parent key: %w", err) + return nil, err } - // Flush the parent key as it's no longer needed - t.flushHandle(parentKeyHandle.Handle, "parent") - t.logger.Info(). Str("handle", fmt.Sprintf("0x%x", loadedKey.ObjectHandle)). Msg("key loaded successfully") From cc206dc9c9c02e8d6c1916059a109c68e4e21b66 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:50 +0000 Subject: [PATCH 38/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ee/server/service/condaccess/config.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/server/service/condaccess/config.go b/ee/server/service/condaccess/config.go index 980f366e265..12a3193c7d7 100644 --- a/ee/server/service/condaccess/config.go +++ b/ee/server/service/condaccess/config.go @@ -21,8 +21,10 @@ func initAssets(ctx context.Context, ds fleet.Datastore) error { savedAssets, err := ds.GetAllMDMConfigAssetsByName(ctx, expectedAssets, nil) if err != nil { // Allow not found errors or partial results (some assets exist, some don't). - // If we got some assets back, continue to create the missing ones. - if !fleet.IsNotFound(err) && len(savedAssets) == 0 { + // If we got some assets back, continue to create the missing ones. Otherwise, + // only tolerate the error if it is a not-found error; any other error + // (e.g. a transient DB error) with no assets returned must be surfaced. + if !fleet.IsNotFound(err) && len(savedAssets) != len(expectedAssets) { return fmt.Errorf("loading existing conditional access assets from the database: %w", err) } } From 7c4b1efcbff27bdbbfbe5b514c8cd26d58c48bbc Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:51 +0000 Subject: [PATCH 39/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- ee/server/service/embedded_scripts/linux_lock.sh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ee/server/service/embedded_scripts/linux_lock.sh b/ee/server/service/embedded_scripts/linux_lock.sh index 7643946036e..19d5aa65b11 100644 --- a/ee/server/service/embedded_scripts/linux_lock.sh +++ b/ee/server/service/embedded_scripts/linux_lock.sh @@ -117,6 +117,17 @@ if [ "$NEEDS_REBOOT" = "1" ]; then # The script already uses systemctl extensively, so systemd-run should be available # This gives us precise 10-second delay for the script to report success echo "Scheduling system reboot in 10 seconds to complete lock process..." - systemd-run --on-active=10s --timer-property=AccuracySec=100ms /sbin/reboot + if command -v systemd-run >/dev/null 2>&1 && systemd-run --on-active=10s --timer-property=AccuracySec=100ms /sbin/reboot; then + : + else + echo "systemd-run failed or is unavailable - falling back to 'at' for delayed reboot" + if command -v at >/dev/null 2>&1 && echo "/sbin/reboot" | at now + 1 minute >/dev/null 2>&1; then + : + else + echo "'at' unavailable or failed - falling back to backgrounded sleep-based reboot" + ( sleep 10 && /sbin/reboot ) >/dev/null 2>&1 & + disown 2>/dev/null || true + fi + fi fi exit 0 From b770f51238a357282f8688745450e5cee389a791 Mon Sep 17 00:00:00 2001 From: "flamingo[bot]" <277372822+flamingo[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:52 +0000 Subject: [PATCH 40/40] fix(adhoc-sweep-fixes): 40 review findings across 40 files --- .../ClickableUrls/ClickableUrls.tsx | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/frontend/components/ClickableUrls/ClickableUrls.tsx b/frontend/components/ClickableUrls/ClickableUrls.tsx index e3f388ec397..31b4fc6dfcb 100644 --- a/frontend/components/ClickableUrls/ClickableUrls.tsx +++ b/frontend/components/ClickableUrls/ClickableUrls.tsx @@ -10,10 +10,36 @@ interface IClickableUrls { const baseClass = "clickable-urls"; const urlReplacer = (match: string) => { - const url = match.startsWith("http") ? match : `https://${match}`; - return ` - ${match} - `; + // Strip trailing punctuation that is unlikely to be part of the intended + // URL (e.g. a period ending a sentence, or a trailing comma/paren) so the + // href and displayed text refer to the same, correctly-bounded URL. + const trailingPunctuationMatch = match.match(/[).,;:!?]+$/); + const trailingPunctuation = trailingPunctuationMatch + ? trailingPunctuationMatch[0] + : ""; + const trimmedMatch = trailingPunctuation + ? match.slice(0, match.length - trailingPunctuation.length) + : match; + + const url = trimmedMatch.startsWith("http") + ? trimmedMatch + : `https://${trimmedMatch}`; + + // Validate that we end up with a well-formed http(s) URL before rendering + // an anchor tag. If validation fails, render the original matched text + // unmodified (no link) to avoid producing an unexpected href target. + try { + const parsed = new URL(url); + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + return match; + } + } catch (e) { + return match; + } + + return ` + ${trimmedMatch} + ${trailingPunctuation}`; }; const ClickableUrls = ({ text, className }: IClickableUrls): JSX.Element => {