mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-07-14 03:38:57 +00:00
cd19285f7f
## Summary Follow-up to PR #1569 (merged). Adds defense-in-depth at the DB layer for the #1534 default_scope-overwrite class of bug. PR #1569 fixed #1534 by guarding the call site in `handleMessage` with `if shouldUpdateDefaultScope(pktData)`. Adversarial review of #1569 flagged this as one-layer defense: a future refactor that drops the call-site `if` and calls `store.UpdateNodeDefaultScope(pubkey, pktData.ScopeName)` unconditionally would silently re-introduce the bug — overwriting a previously-correct `default_scope` (e.g. `#belgium`) with the empty string. This PR adds the belt-and-braces guard recommended by that review: - `Store.UpdateNodeDefaultScope(pk, "")` is now a silent no-op (early `return nil`) - New DB-layer regression test that fails on `master` and proves the DB function used to write `""` straight through - Two new call-site anchor tests that drive a transport-scoped ADVERT end-to-end through `handleMessage` (matched + unmatched region key) so the existing call-site guard from #1569 can't be deleted without a test going red Net production change: 8 lines in `cmd/ingestor/db.go`. No behavior change for any non-empty scope. ## Why this is a follow-up, not a re-fix Issue #1534 is already closed by #1569 and `master` no longer regresses for users (the call-site guard is in place). This PR is purely belt-and-braces — it adds the second layer of defense the adversarial reviewer asked for and the test coverage that anchors both layers. ## Files changed | File | Change | |------|--------| | `cmd/ingestor/db.go` | +8 — empty-scope early return in `UpdateNodeDefaultScope` | | `cmd/ingestor/db_test.go` | +43 — `TestUpdateNodeDefaultScope_EmptyScopeIsNoop` | | `cmd/ingestor/main_test.go` | +97 — `TestHandleMessageAdvert_EmptyScopeSkipsDefaultScopeUpdate` + `TestHandleMessageAdvert_MatchedScopeUpdatesDefaultScope` | ## Red → green commits - **red** `c062af59` — `test(ingestor): red — DB-layer empty-scope guard regression test for #1534` - Adds three tests; `TestUpdateNodeDefaultScope_EmptyScopeIsNoop` fails on assertion (`default_scope` overwritten with `""`) - Two call-site tests pass already (call-site guard merged in #1569) — they anchor that behavior against future refactors - **green** `7ab12d53` — `fix(ingestor): defense-in-depth empty-scope guard in UpdateNodeDefaultScope (#1534)` - Adds the early-return; all three tests green ## Operator remediation (from issue #1534) Operators whose production DB still has rows where `default_scope` was overwritten with the empty string before #1569 deployed can clean up with: ```sql -- Inspect affected rows first SELECT public_key, name, default_scope FROM nodes WHERE default_scope = ''; SELECT public_key, name, default_scope FROM inactive_nodes WHERE default_scope = ''; -- Convert empty-string default_scope back to NULL so the next valid -- matched-scope advert can re-populate it cleanly. UPDATE nodes SET default_scope = NULL WHERE default_scope = ''; UPDATE inactive_nodes SET default_scope = NULL WHERE default_scope = ''; ``` After #1569 + this PR are deployed, no new rows can be created with `default_scope = ''` from this code path. ## Test plan ```bash cd cmd/ingestor && go test ./... -count=1 # ok github.com/corescope/ingestor ~98s ``` ## Preflight Clean — PII, branch scope, red commit, CSS-var defined, CSS self-fallback, LIKE-on-JSON, sync migration, async-migration gate, XSS sinks all pass. No warnings. --------- Co-authored-by: Kpa-clawbot <bot@meshcore-analyzer>