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

[flamingo]

Closes 92 review findings across 38 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

#	Fix confidence	Finding	Location
1	🟡 88 medium	HasCertHash uses substring match instead of exact CSV field comparison, risking false positives	server/mdm/nanomdm/storage/file/certauth.go:36
2	🟡 85 medium	EnrollmentFromHash's substring match can return the wrong enrollment ID	server/mdm/nanomdm/storage/file/certauth.go:80
3	🟡 70 medium	AssociateCertHash returns bare errors without call-boundary context wrapping	server/mdm/nanomdm/storage/file/certauth.go:57
4	🟢 92 high	FleetClient#req swallows exceptions into a hash field instead of raising or logging	ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb:168
5	🟡 60 medium	FleetClient cache never expires and can serve stale host lookups indefinitely	ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb:118
6	🟡 70 medium	parse_response double-parses response.body redundantly	ee/tools/puppet/fleetdm/lib/puppet/util/fleet_client.rb:191
7	🟢 90 high	Dead branch in update-critical-software.js: OS-versions-changed case does nothing	ee/vulnerability-dashboard/scripts/update-critical-software.js:91
8	🟡 60 medium	Fallback OS lookup mutates variable but original null check is bypassed incorrectly	ee/vulnerability-dashboard/scripts/update-critical-software.js:237
9	🔴 25 low — review closely	Unused/incorrectly-scoped missingCriticalInstallsByHostIDs dictionary	ee/vulnerability-dashboard/scripts/update-critical-software.js:51
10	🟡 60 medium	onChange callback typed as any defeats the documented dual-shape contract	frontend/components/forms/fields/InputField/InputField.tsx:30
11	🟡 85 medium	stringToClipboard(value) passes a possibly non-string value without conversion	frontend/components/forms/fields/InputField/InputField.tsx:126
12	🔴 50 low — review closely	Textarea copy button uses raw while single-line input copy/eye buttons use raw — inconsistent a11y semantics	frontend/components/forms/fields/InputField/InputField.tsx:140
13	🟢 90 high	Unsafe array-index access on API error response can throw when errors array is empty	frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx:98
14	🟢 90 high	Same unguarded errors[0] indexing repeated in the regular-user update catch handler	frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx:147
15	🔴 35 low — review closely	Duplicated already exists / error-classification logic for invite vs. user submit paths	frontend/pages/admin/ManageUsersPage/EditUserPage/EditUserPage.tsx:98
16	🟡 70 medium	startProcessAsCurrentUser leaks userToken, impersonationToken-derived handle, and envInfo on all paths	orbit/pkg/execuser/execuser_windows.go:209
17	🟡 65 medium	processInfo handles (hProcess/hThread) are never closed after CreateProcessAsUser succeeds	orbit/pkg/execuser/execuser_windows.go:248
18	🔴 55 low — review closely	wtsEnumerateSessions leaks the native WTS_SESSION_INFO buffer returned by WTSEnumerateSessionsW	orbit/pkg/execuser/execuser_windows.go:164
19	🟢 90 high	satisfiesConstraints operator logic appears inverted for GreaterThan/LessThan comparisons	orbit/pkg/table/tcc_access/tcc_access.go:166
20	🟡 85 medium	parseTCCDbReadOutput panics/truncates data if sqlite3 output does not end with a trailing empty line	orbit/pkg/table/tcc_access/tcc_access.go:130
21	🟡 80 medium	sqlite3 subprocess spawned per-user without timeout or context cancellation	orbit/pkg/table/tcc_access/tcc_access.go:109
22	🟡 75 medium	SyncEnrolledHostIDs closes and reuses a pooled connection incorrectly	server/datastore/mysqlredis/hosts.go:60
23	🟡 60 medium	Redis SyncEnrolledHostIDs is not atomic: DEL then SADD can leave the set empty on crash	server/datastore/mysqlredis/hosts.go:60
24	🟡 85 medium	Bare return err after DeleteHost call loses call-site context	server/datastore/mysqlredis/hosts.go:162
25	🟡 85 medium	DigiCert validation loop only reports the first mismatched CA due to unconditional break	server/service/mdm_profiles.go:50
26	🟡 70 medium	Renewal-ID chained assignment silently drops earlier duplicate detection when later Set*RenewalID calls succeed	server/service/mdm_profiles.go:458
27	🟡 60 medium	additionalDigiCertValidation error returned without wrapping context	server/service/mdm_profiles.go:492
28	🔴 30 low — review closely	SCEP client uses OptimisticCertificateVerifier, effectively disabling TLS/certificate validation for enrollment	android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt:78
29	🟡 85 medium	SCEP challenge password defaults to empty string instead of failing enrollment	android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt:87
30	🟡 75 medium	ScepClientImpl swallows exception message context by removing Log.e calls, comment says 'to avoid test failures'	android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt:141
31	🟢 90 high	vendorPool map mutated without synchronization in internString	cmd/osquery-perf/softwaredb/softwaredb.go:51
32	🟡 80 medium	SQL query built with fmt.Sprintf using unescaped source list in loadDarwinSoftware	cmd/osquery-perf/softwaredb/softwaredb.go:386
33	🔴 55 low — review closely	Partial database file only cleaned up in generateDatabaseFromSQL, not in LoadFromDatabase's later checks	cmd/osquery-perf/softwaredb/softwaredb.go:318
34	🔴 55 low — review closely	recordAccumulate reads existing bitmaps from the writer but relies on ODKU merge without a transaction, risking lost updates under concurrent writers	server/chart/internal/mysql/data.go:118
35	🔴 55 low — review closely	recordSnapshot close-then-insert is not transactional, risking a window where an entity has no open row	server/chart/internal/mysql/data.go:240
36	🟡 75 medium	GetSCDData numBuckets computed via integer division can silently truncate on non-divisible ranges	server/chart/internal/mysql/data.go:315
37	🟡 75 medium	Temporary index add/drop on host_software_installed_paths is not guarded against migration re-entry across the whole function scope, risking leaving the index behind on partial failure	server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go:91
38	🔴 55 low — review closely	Malformed host_software query using sqlx.In and NOT EXISTS subquery may silently skip merging on ErrNoRows even when the outer selectedID host DOES need updating	server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go:113
39	🔴 30 low — review closely	New software.checksum computation omits name from the hash while the pre-migration duplicate-detection query also omits it, but the comment claims to 'simulate new hash'	server/datastore/mysql/migrations/tables/20250410104321_UpdateMacOSSoftwareNames.go:201
40	🔴 40 low — review closely	CA private key written with weak 3DES PEM encryption and password may be empty	server/mdm/scep/cmd/scepserver/scepserver.go:236
41	🟡 85 medium	context.TODO() used for the entire lifetime of main() in scepserver	server/mdm/scep/cmd/scepserver/scepserver.go:98
42	🟡 80 medium	SIGINT-only signal handling omits SIGTERM, risking un-graceful shutdown in containerized deployments	server/mdm/scep/cmd/scepserver/scepserver.go:180
43	🔴 55 low — review closely	Webhook comment authorization trusts author_association without verifying it against the actual PR, allowing spoofing via issue vs PR mismatch	tools/fleet-slackbot/webhook-handler.js:146
44	🟡 70 medium	File path validation allows path.posix.normalize results equal to '..' variants beyond the simple prefix check but misses backslash-based traversal on case-insensitive filesystems	tools/fleet-slackbot/webhook-handler.js:251
45	🔴 40 low — review closely	CI auto-fix loop-prevention only inspects the immediate parent commit, allowing more than 2 total auto-fix attempts across non-consecutive commits	tools/fleet-slackbot/webhook-handler.js:312
46	🔴 55 low — review closely	Webhook throws a bare string instead of an Error/exit, bypassing declared exits	website/api/controllers/webhooks/receive-from-clay.js:135
47	🟢 90 high	Webhook error message leaks the received (attacker-controlled) secret value into logs/error text	website/api/controllers/webhooks/receive-from-clay.js:113
48	🟡 70 medium	Webhook secret comparison uses non-constant-time string equality	website/api/controllers/webhooks/receive-from-clay.js:113
49	🟢 90 high	String concatenation with boolean produces literal 'false' text in className template	frontend/components/QueryFrequencyIndicator/QueryFrequencyIndicator.tsx:56
50	🔴 30 low — review closely	generateClassTag() switches on frequency string but classTag used only for CSS while readableQueryFrequency has separate switch — duplicated frequency mapping logic	frontend/components/QueryFrequencyIndicator/QueryFrequencyIndicator.tsx:12
51	🟢 95 high	document.title template literal introduces stray whitespace/newline in browser tab title	frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx:106
52	🟡 75 medium	Second router.push during render for queryDiscardData has the same render-time side-effect issue	frontend/pages/hosts/details/HostQueryReport/HostQueryReport.tsx:98
53	🟢 95 high	PushHandler ignores the error from pusher.Push when computing per-id success/error counts, using stale err variable reuse pattern later on	server/mdm/nanomdm/http/api/api.go:213
54	🟡 85 medium	readPEMCertAndKey swallows the encrypted-private-key error via break inside a switch, not the enclosing for loop	server/mdm/nanomdm/http/api/api.go:288
55	🟢 95 high	fleetctl-npm platform switch computes 'windows' arch label but binName check compares against literal 'windows' which never matches	tools/fleetctl-npm/run.js:28
56	🔴 55 low — review closely	fleetctl-npm run.js swallows a stale-binary race and unhandled promise rejection potential in install()	tools/fleetctl-npm/run.js:50
57	🟢 95 high	Wrong flag name referenced in panic messages for existing-url parse failure	tools/mdm/migration/mdmproxy/mdmproxy.go:273
58	🟡 65 medium	Panic used instead of returned/wrapped error for URL parse failures in mdmproxy	tools/mdm/migration/mdmproxy/mdmproxy.go:273
59	🟢 95 high	IDP() emits misleading error for AppConfig-defined-twice check when parsing enroll_secret kind	pkg/spec/spec.go:203
60	🟡 70 medium	generateRandomString panics on rand.Read failure instead of returning an error	pkg/spec/spec.go:281
61	🟢 90 high	EnrollmentFromHash queries a different table name than the rest of the file (cert_auth_associations vs nano_cert_auth_associations)	server/mdm/nanomdm/storage/mysql/certauth.go:59
62	🟡 85 medium	EnrollmentFromHash does not lowercase the hash before lookup, unlike sibling functions	server/mdm/nanomdm/storage/mysql/certauth.go:59
63	🟢 95 high	Self-comparing require.Equal assertions in TestMaybeUpdateSetupExperience make the checks tautological/no-ops	server/service/setup_experience_test.go:336
64	🟢 95 high	Same self-comparison bug repeated in the software-install-results sub-test	server/service/setup_experience_test.go:391
65	🟢 92 high	DefaultTeam construction logic duplicated between getTeamEndpoint and modifyTeamEndpoint	server/service/teams.go:87
66	🟢 95 high	ApplyTeamSpecs receiver uses value receiver (svc Service) instead of pointer receiver used elsewhere	server/service/teams.go:317
67	🟢 90 high	GetIssuesCreatedSinceWithLabel: unsynchronized read of stopError inside goroutine loop causes data race	tools/github-manage/pkg/ghapi/issues.go:372
68	🔴 55 low — review closely	RemoveIssueFromProject compares error strings instead of using a typed/sentinel error	tools/github-manage/pkg/ghapi/issues.go:124
69	🟢 90 high	build_teams.sh has a duplicate/shadowed getopts flag f used for two different purposes	tools/team-builder/build_teams.sh:15
70	🔴 45 low — review closely	Insecure temp file used for /random reads without buffering/consistency check in team-builder secret generation	tools/team-builder/build_teams.sh:58
71	🟢 95 high	Button in PlatformHostCounts policy list missing a unique key when mapping over policies	frontend/pages/DashboardPage/cards/WelcomeHost/WelcomeHost.tsx:230
72	🔴 40 low — review closely	WelcomeHost hardcodes HOST_ID = 1 to load 'personal device' host details	frontend/pages/DashboardPage/cards/WelcomeHost/WelcomeHost.tsx:25
73	🟡 85 medium	CustomValueContainer destructures nonexistent 'props' field, breaking all usages	frontend/pages/hosts/ManageHostsPage/components/CustomValueContainer/CustomValueContainer.tsx:8
74	🟡 70 medium	CustomValueContainer component misnamed 'CustomDropdownIndicator', duplicating the actual CustomDropdownIndicator component's name and rendering the wrong sub-component	frontend/pages/hosts/ManageHostsPage/components/CustomValueContainer/CustomValueContainer.tsx:2
75	🔴 40 low — review closely	aws-backup.tf duplicates the 'backup:TagResource' statement block verbatim	infrastructure/dogfood/terraform/aws-tf-module/aws-backup.tf:175
76	🔴 45 low — review closely	Backup selection resource ARN hardcodes account ID and region, breaking portability across environments	infrastructure/dogfood/terraform/aws-tf-module/aws-backup.tf:521
77	🟢 95 high	TestGenerate references wrong key name in fatal error message	orbit/pkg/table/csrutil_info/csrutil_info_test.go:22
78	🟢 95 high	TestGenerate ignores the returned err from getSSVEnabled before asserting on ssvEnabled	orbit/pkg/table/csrutil_info/csrutil_info_test.go:30
79	🟡 85 medium	listS3Carves passes ContinuationToken by reference to an empty string on first page, likely causing an API error or ignored parameter	server/datastore/s3/carves.go:101
80	🟢 90 high	GetBlock swallows the original GetObject error when UpdateCarve also fails	server/datastore/s3/carves.go:303
81	🟢 90 high	GetHostsReport does not close the HTTP response body	server/service/client_hosts.go:152
82	🟡 85 medium	GetHostsReport propagates bare errors without call-site context wrapping	server/service/client_hosts.go:152
83	🟢 95 high	fmt.Println used in Jira test instead of t.Log, leaks output unconditionally	server/worker/jira_test.go:83
84	🔴 55 low — review closely	TestJiraRun table entry omits expectedDescription assertion for a scored-premium case, weakening coverage	server/worker/jira_test.go:172
85	🟢 95 high	vex-parser assumes non-empty Statements slice, will panic on empty VEX file	tools/vex-parser/vex-parser.go:55
86	🟢 90 high	vex-parser swallows underlying error context by discarding timestamp parse failure detail path	tools/vex-parser/vex-parser.go:97
87	🟡 85 medium	RunQueryPage triggers router.push during render, causing side effects on every render	frontend/pages/queries/live/LiveQueryPage/LiveQueryPage.tsx:87
88	🟡 75 medium	goToQueryEditor useCallback has empty dependency array but closes over queryId and currentTeamId	frontend/pages/queries/live/LiveQueryPage/LiveQueryPage.tsx:163
89	🔴 40 low — review closely	SetServerCapabilities is called twice in ParseResponse	client/base_client.go:74
90	🔴 55 low — review closely	Bare error returns without wrapping in client/base_client.go's DoHTTPRequest	client/base_client.go:288
91	🟢 90 high	randomSecretGenerator in SecretEditorModal uses Math.random(), not a cryptographically secure RNG, for enrollment secrets	frontend/components/EnrollSecrets/SecretEditorModal/SecretEditorModal.tsx:22
92	🔴 55 low — review closely	renderTeam() mutates its own prop parameter, defeating React unidirectional data flow and hiding a type bug	frontend/components/EnrollSecrets/SecretEditorModal/SecretEditorModal.tsx:47

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

---

Run: https://product-hub.flamingo.so/admin/code-review
Run id: 3ba0fd37-c226-436b-ba81-b544a7d0a515

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akbhhtv FleetMDM bulk review findings sweep (10 PRs)