[Backend] Minor-bug batch from SonarQube triage: freeze the remediation allowlist + spell registry, constrain generic event handlers, dead null-coalesce, poll-loop default cases #637

Closed
opened 2026-07-17 05:05:51 +00:00 by spikerj · 2 comments
Owner

Batch of small verified-real findings from the SonarQube triage loop (2026-07-17), each checked in context for regression safety:

  1. RemediationActions.SafeSet was a mutable public static HashSet (S3887 CRITICAL adjacent) — it is the allowlist that decides which remediation commands auto-execute on swarm hosts, so mutability is a hardening gap. Now FrozenSet<string> (same OrdinalIgnoreCase semantics, only ever read via .Contains; the block-ip-stays-out test still passes).
  2. SpellDefinitions.Spells — mutable static game-content registry, read only via GetValueOrDefault. Now FrozenDictionary.
  3. GameEventsConsumer.HandleEntity{Updated,Created}Async<T> (S2955 x2) — evt == null on unconstrained T; every instantiation is a class DTO, so where T : class makes the null-check well-defined.
  4. ChatHub (S2583) — dead ?? string.Empty on a value already null-guarded by the enclosing if.
  5. scripts/mac-dev-smoke{,-quiz}.sh (shellcheck S131 x2) — poll-loop case blocks get explicit *) defaults documenting 'non-terminal: keep polling'.

Also FP'd with comments (not fixed, deliberately): EntityDelta.CompressedData mutable array (per-tick hot path, pooled buffer — immutability would allocate per entity per tick) and the email-verified page's self-registering snake-game constructor (S1848).

All affected test projects pass (SystemRemediation 82, GameServer 913, NodeAgent 137, GameEvents 10); full slnf builds clean.

Batch of small verified-real findings from the SonarQube triage loop (2026-07-17), each checked in context for regression safety: 1. **`RemediationActions.SafeSet` was a mutable public static `HashSet`** (S3887 CRITICAL adjacent) — it is the allowlist that decides which remediation commands auto-execute on swarm hosts, so mutability is a hardening gap. Now `FrozenSet<string>` (same OrdinalIgnoreCase semantics, only ever read via `.Contains`; the block-ip-stays-out test still passes). 2. **`SpellDefinitions.Spells`** — mutable static game-content registry, read only via `GetValueOrDefault`. Now `FrozenDictionary`. 3. **`GameEventsConsumer.HandleEntity{Updated,Created}Async<T>`** (S2955 x2) — `evt == null` on unconstrained `T`; every instantiation is a class DTO, so `where T : class` makes the null-check well-defined. 4. **`ChatHub`** (S2583) — dead `?? string.Empty` on a value already null-guarded by the enclosing `if`. 5. **`scripts/mac-dev-smoke{,-quiz}.sh`** (shellcheck S131 x2) — poll-loop `case` blocks get explicit `*)` defaults documenting 'non-terminal: keep polling'. Also FP'd with comments (not fixed, deliberately): `EntityDelta.CompressedData` mutable array (per-tick hot path, pooled buffer — immutability would allocate per entity per tick) and the email-verified page's self-registering snake-game constructor (S1848). All affected test projects pass (SystemRemediation 82, GameServer 913, NodeAgent 137, GameEvents 10); full slnf builds clean.
Author
Owner

Added a 6th item to PR #344: JsonPacketSerializer's S3923 BUG — an if/else whose branches were byte-identical (both set dict["type"] = GetEventTypeName(evt)). Collapsed to one unconditional assignment; GameServer tests (913) pass. That completes disposition of ALL 44 original BUG-type findings: 14 fixed across PRs #342/#344, 30 false-positived with per-issue reasoning in SonarQube.

Added a 6th item to PR #344: JsonPacketSerializer's S3923 BUG — an if/else whose branches were byte-identical (both set dict["type"] = GetEventTypeName(evt)). Collapsed to one unconditional assignment; GameServer tests (913) pass. That completes disposition of ALL 44 original BUG-type findings: 14 fixed across PRs #342/#344, 30 false-positived with per-issue reasoning in SonarQube.
Author
Owner

Resolved in spikersoft-backend PR #344 (merged to master). RemediationActions.SafeSet (the host auto-remediation allowlist) and SpellDefinitions.Spells are frozen collections now; GameEventsConsumer's generic handlers carry 'where T : class'; ChatHub's dead null-coalesce and JsonPacketSerializer's identical if/else are gone; both mac smoke scripts document their poll-loop defaults. Verified: full slnf clean, SystemRemediation 82 + GameServer 913 + NodeAgent 137 + GameEvents 10 tests pass. The related FP dispositions (EntityDelta hot-path array, snake-game bootstrap) are commented in SonarQube. Closing.

Resolved in spikersoft-backend PR #344 (merged to master). RemediationActions.SafeSet (the host auto-remediation allowlist) and SpellDefinitions.Spells are frozen collections now; GameEventsConsumer's generic handlers carry 'where T : class'; ChatHub's dead null-coalesce and JsonPacketSerializer's identical if/else are gone; both mac smoke scripts document their poll-loop defaults. Verified: full slnf clean, SystemRemediation 82 + GameServer 913 + NodeAgent 137 + GameEvents 10 tests pass. The related FP dispositions (EntityDelta hot-path array, snake-game bootstrap) are commented in SonarQube. Closing.
Sign in to join this conversation.