feat: geofilter customizer tab + PUT /api/config/geo-filter

[efiten]

Summary

Part of #669 — M3: Customizer integration.

Backend

• PUT /api/config/geo-filter (requires X-API-Key) — saves geo_filter to config.json atomically (temp+rename) and updates in-memory config immediately. No restart needed.
• SaveGeoFilter() in config.go reads config as a raw map (preserving _comment fields and any unknown keys), updates just the geo_filter key, writes back.
• GET /api/config/geo-filter now includes writeEnabled: bool — true when server has a strong apiKey configured.
• Server.configDir field wired from -config-dir flag.
• 7 new tests (TestPutConfigGeoFilter × 4, TestSaveGeoFilter × 3, updated TestConfigGeoFilterEndpoint × 2).

Frontend

• New 🗺️ GeoFilter tab in the customizer between Display and Export.
• Reads GET /api/config/geo-filter on tab open; shows current polygon on a Leaflet map.
• Editing controls are hidden on public deployments (writeEnabled=false). On servers with a write-capable apiKey, the edit UI reveals itself: undo/clear, buffer km, API key input, Save and Remove buttons.
• Save calls PUT /api/config/geo-filter; applies immediately in-memory.
• Leaflet map instance is cleaned up on tab switch and panel close.

Docs

• New docs/user-guide/geofilter.md: complete guide covering config schema, customizer tab, builder, prune script, and API.
• Updated docs/user-guide/configuration.md and customization.md.
• Updated config.example.json _comment to mention the Customizer tab.

Note: Editing controls are gated on writeEnabled (server-side flag) — non-admin users see a read-only polygon view. Admins enter their API key per-save; it is never stored client-side.

Depends on: #734 (M1) and #735 (M2) for context, but can be merged independently.

Test plan

• Server with no apiKey: GeoFilter tab shows map, no edit controls
• Server with apiKey: edit controls appear after tab loads
• Draw polygon → Save with correct API key → polygon saved to config.json, reloads correctly
• Clear filter → Remove filter → config.json no longer has geo_filter
• Wrong API key → error message shown
• Tab switch and panel close do not leave orphaned Leaflet maps
• go test ./... passes

🤖 Generated with Claude Code

[Kpa-clawbot]

Security Review — PUT /api/config/geo-filter

Reviewed with a security-focused lens on the config write endpoint. The overall design is solid — API key gating, atomic file writes, input shape validation, clear separation of read-only vs write-enabled UI. Good work. But there are a few issues that need addressing before merge.

Must-fix

1. Data race on s.cfg.GeoFilter (routes.go)
The PUT handler writes s.cfg.GeoFilter = gf with no synchronization. The GET handler and any ingest-time filtering read s.cfg.GeoFilter concurrently from other goroutines. This is a textbook data race — Go's race detector will flag it. Wrap config mutations in a mutex (or use atomic.Pointer if you want lock-free reads).

2. No request body size limit (routes.go:~L440)
json.NewDecoder(r.Body).Decode(&body) reads unbounded input. An attacker with a valid API key (or a compromised key) can send a multi-GB JSON body and exhaust server memory. Wrap with http.MaxBytesReader:

r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // 1 MB

3. No coordinate range validation (routes.go)
Polygon points are stored as-is — no check that lat ∈ [-90, 90] and lon ∈ [-180, 180]. NaN, Infinity, or wildly out-of-range values would be persisted to config.json and could cause downstream issues (point-in-polygon math, Leaflet rendering). Validate each point after decoding.

Should-fix

4. writeEnabled leaks API key presence to unauthenticated callers (routes.go:~L429)
The GET endpoint (no auth required) returns writeEnabled: true/false, which tells any anonymous visitor whether the server has a strong API key configured. This is a minor information leak — consider removing it from the unauthenticated response and having the UI infer editability from a separate authenticated probe, or accept the risk with a comment explaining the trade-off.

5. No upper bound on polygon point count
A polygon with 100,000 points is technically valid per the current checks. Consider a reasonable cap (e.g., 1000 points) to bound storage and computation.

Nits (non-blocking)

• SaveGeoFilter error messages include filesystem paths (config.json not found in /app/data). These get logged server-side (fine) but also returned to the client via writeError. Consider generic client-facing messages.
• The _comment field in config.example.json is a nice touch for discoverability.

What's done well

• requireAPIKey middleware on the PUT — correct layer for authz
• IsWeakAPIKey check for writeEnabled — defense in depth
• Atomic write via temp + rename — prevents partial writes on crash
• Polygon minimum 3 points validation
• Comprehensive test coverage: auth rejection, save/clear/validation, disk persistence
• UI correctly gates edit controls on server-reported writeEnabled
• Clean separation: standalone builder for offline use, customizer tab for live editing

Items 1–3 are must-fix before merge. The rest can be addressed in follow-ups.

[efiten]

All must-fix and should-fix items from the security review have been addressed in commit aae2e13 (plus a build fix in e12dc6d):

Must-fix — resolved:

1. Data race — added cfgMu sync.RWMutex to Server, reads go through getGeoFilter() and writes through setGeoFilter().
2. Body size limit — http.MaxBytesReader(w, r.Body, 1<<20) added at the top of the PUT handler.
3. Coordinate validation — each polygon point is validated: lat ∈ [-90, 90], lon ∈ [-180, 180], rejects NaN and Infinity.

Should-fix — resolved:
4. writeEnabled info leak — accepted risk, added an inline comment in the GET handler explaining the trade-off (low-sensitivity info, public read-only clients need it).
5. Polygon point cap — hard limit of 1000 points enforced, returns 400 if exceeded.

Nit (filesystem path in error response) — left as-is; the path leaks only to authenticated callers (PUT requires X-API-Key) and matches the existing pattern in the codebase.

Build was also broken by a missing closing } on SaveGeoFilter — fixed in e12dc6d.

[Kpa-clawbot]

Final Re-Review — NEEDS REBASE

Follow-up to the security review from 2026-04-15. Verifying @efiten's claimed fixes in aae2e13.

🔒 djb — Security Verification

Must-fix #1 (data race): ✅ Fixed. cfgMu sync.RWMutex added to Server, with getGeoFilter() (RLock) and setGeoFilter() (Lock). Both the GET handler (L449) and handleNodes (L1120) now use getGeoFilter(). PUT handler uses setGeoFilter().

Must-fix #2 (unbounded body): ✅ Fixed. http.MaxBytesReader(w, r.Body, 1<<20) at L471.

Must-fix #3 (coordinate validation): ✅ Fixed. NaN/Inf/range checks at L487-490, lat ∈ [-90,90], lon ∈ [-180,180].

Should-fix #4 (writeEnabled leak): ✅ Acknowledged. Comment at L448-450 documents the risk-accepted trade-off. Reasonable.

Should-fix #5 (polygon cap): ✅ Fixed. 1000-point cap at L484.

Nit (error paths leak): ✅ Fixed. SaveGeoFilter errors are logged server-side (log.Printf at L503) but the client gets generic "failed to save config" (L504).

All five original findings verified as resolved.

📐 dijkstra — Correctness of the Mutex Discipline

The RWMutex usage is correct: reads take RLock, writes take Lock. The invariant "in-memory ≥ disk" holds because SaveGeoFilter writes disk before setGeoFilter updates memory (L498-505). If the process crashes between disk write and memory update, the next startup loads from disk — convergent. If disk write fails, memory is unchanged — consistent.

One observation: s.cfg.APIKey is read outside the mutex (L451), but it's set once at startup and never mutated — no race.

🆕 New Findings

1. SaveGeoFilter raw-map injection: Not exploitable. The PUT handler decodes into a strict struct{Polygon, BufferKm} — only those two typed fields reach SaveGeoFilter. The raw-map preserves other config keys but can't be poisoned via the API.

2. UI read-only degradation: Clean. #cv2-gf-edit starts display:none, only shown when writeEnabled=true (L1421). Modal title changes to "read only" (L1215). No edit affordances leak.

3. Test coverage: Good breadth — auth rejection, save/clear/validation, disk persistence, coordinate range, polygon cap (L2841-4033). However, no concurrent test exercises the race fix (e.g., parallel PUT + GET). Structural fix is sound, but a TestGeoFilterConcurrent with -race would be a nice addition in a follow-up.

Verdict

Ready after rebase. All must-fix and should-fix items verified as resolved. No new blocking issues. The PR is currently CONFLICTING against master — @efiten please rebase onto current master and force-push. After that, this is merge-ready.

[Kpa-clawbot]

@efiten can you please resurrect this PR - either rebase it and let's get it merged or if it's stale let's close it as such.

[Kpa-clawbot]

Consolidated review — PR #736

Adversarial review. Verdict: NEEDS-WORK — security regression at P0.

MUST-FIX

1. 🔴 Security: file mode downgrade on every save. SaveGeoFilter does os.WriteFile(tmp, out, 0644) then os.Rename. Operators commonly chmod config.json to 0600 because it contains apiKey (the very same key the PUT endpoint authenticates with). Every successful PUT will silently re-create the file world-readable. Fix: os.Stat(configPath) to capture original mode, apply via os.Chmod(tmp, st.Mode()) (or OpenFile with that mode) before rename.

2. bufferKm is not validated. PUT validates every polygon point for NaN/Inf/range but lets bufferKm through unchecked. {"polygon":[…],"bufferKm":-1e9} or NaN writes straight to disk and into GeoFilterConfig. Negative buffer inverts geometric semantics in NodePassesGeoFilter; NaN poisons comparisons. Fix: reject IsNaN/IsInf, require >= 0, impose a sane upper bound (e.g., ≤ 20000 km — half-Earth circumference is the only meaningful ceiling).

3. Concurrent-PUT read-modify-write race on disk. cfgMu only guards the in-memory pointer. SaveGeoFilter reads config.json, mutates the raw map, writes tmp, renames — with no serialization. Two concurrent authenticated PUTs can (a) collide on the shared configPath+".tmp" filename (one rename clobbers the other's tmp mid-write → corrupt JSON possible), and (b) interleave such that disk ends up with writer B's payload but memory ends up with writer A's (setGeoFilter calls reorder vs. disk writes). Prior reviewer's "structurally sound, race-test follow-up" stopped at the in-memory race — the disk race is real, just rare. Fix: hold cfgMu (Lock) for the duration of SaveGeoFilter + setGeoFilter, or introduce a dedicated saveMu covering the file I/O. Use a per-process unique tmp suffix (.tmp.<pid>.<nano>) for extra safety.

Prior CHANGES_REQUESTED: all 5 ✅ addressed (cfgMu RWMutex, MaxBytesReader, coord validation, writeEnabled comment, polygon 1000-point cap).

Out-of-scope (non-blocking)

• No TestGeoFilterConcurrent exercising parallel PUT+GET under -race (prior reviewer already flagged as follow-up).
• _gfWriteEnabled JS module var is monotonically set to true, never reset on subsequent loads (stale if admin rotates apiKey without page reload).
• _gfPoints persists across panel reopens — surprising UX after a Cancel.
• No fsync on tmp file before rename (standard "atomic write" caveat, but no other code in repo does this either).

Each must-fix is small (<15 LoC). Once they land, this is ready.

[efiten]

All 3 must-fix items are now addressed:

1. File mode preserved — SaveGeoFilter reads the original mode via os.Stat and applies it to the tmp file before rename.
2. bufferKm validated — rejects NaN/Inf, negative values, and values >20000 km.
3. Concurrent-PUT race fixed — saveMu serializes all file I/O; per-process unique tmp suffix prevents clobber.

Ready for re-review.

[Kpa-clawbot]

Rebase blocked — needs your input

We tried to auto-rebase onto current master but hit a 6-file config-design conflict on your first commit:

• cmd/server/config.go
• cmd/server/routes.go
• cmd/server/routes_test.go
• config.example.json
• docs/user-guide/configuration.md, docs/user-guide/geofilter.md
• public/customize-v2.js

The bot can't safely guess at the M3 customizer's intended design vs what master has accumulated. Aborting rather than risk breaking your PR's intent.

Recommendation: please rebase locally — git rebase origin/master, resolve the config/routes/customizer conflicts the way you want, then force-push.

We'll auto-merge once it's clean.

[efiten]

Confirming the existing CHANGES_REQUESTED review. Two must-fix items: (1) SaveGeoFilter uses os.WriteFile(tmp, out, 0644) then renames — this silently resets config.json to world-readable on every PUT, even if the operator chmod'd it to 0600 to protect the apiKey. Fix: stat the original file, capture its mode, and apply that mode to the temp file before rename. (2) bufferKm is not validated — NaN/Inf/-1e9 all write to disk. Add bounds check (e.g. must be finite and >= 0). The data race on s.cfg.GeoFilter in routes.go also needs a mutex or atomic pointer. Overall design is solid once these are addressed.