fix: harden geoip parser, probe miss-detection and CSV export (review)

Adversarial review of the six-feature batch found no critical/high defects;
these are the three low-severity robustness/correctness fixes it surfaced.

- geo: reject an over-long protobuf length varint by unsigned comparison — a
  length ≥ 2^63 cast to int went negative and slipped past the bounds check,
  panicking the slice on a corrupt/truncated geoip.dat (now errors cleanly)
- decoy: Miss() now mirrors serveMiss exactly — an extensionless miss on a
  single-page template is served as the index under 200, so it is NOT counted
  as a probe (only a 404-serving template or a missing asset is), preventing a
  future SPA-style decoy from flagging real visitors as scanners
- audit CSV: neutralize spreadsheet formula injection (leading = + - @ tab CR)
  in the free-text columns
- tests: malformed/truncated geoip.dat no-panic; Miss for SPA vs classic decoy

Author: AppsGanin

internal/decoy/decoy.go

@@ -207,18 +207,25 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	h.serve(w, r, a, http.StatusOK)
 }
 
-// Miss reports whether urlPath resolves to no asset this template ships — i.e. the
-// request would be served as a "not found" rather than a real page. Read-only; it
-// mirrors ServeHTTP's name resolution so probe detection sees exactly what the decoy
-// treats as a miss. A scanner guessing the hidden panel path hits misses; a browser
-// loading the decoy's own pages and assets does not.
+// Miss reports whether urlPath would be served as a genuine "not found" (a 404) rather
+// than a real page — mirroring what serveMiss actually does, so probe detection counts
+// exactly the requests the decoy treats as misses. A scanner guessing the hidden panel
+// path hits misses; a browser loading the decoy's own pages and assets does not.
+//
+// The subtlety is the single-page templates: they ship no 404 page and answer an
+// EXTENSIONLESS miss with the index under 200 (the `try_files $uri /index.html` case),
+// so that is NOT a miss — only a template with its own 404 page, or a missing asset
+// (a path with an extension), is. Counting extensionless SPA fallbacks would flag a
+// visitor following an internal client-route link as a scanner.
 func (h *Handler) Miss(urlPath string) bool {
 	name := strings.TrimPrefix(path.Clean("/"+urlPath), "/")
 	if name == "" {
 		name = "index.html"
 	}
-	_, ok := h.files[name]
-	return !ok
+	if _, ok := h.files[name]; ok {
+		return false // a real page or asset the template ships
+	}
+	return h.notFound != nil || path.Ext(name) != ""
 }
 
 // serveMiss answers a path the template doesn't have.

internal/decoy/decoy_test.go

@@ -156,19 +156,31 @@ func TestSPATemplateFallbackAndAssetMiss(t *testing.T) {
 	}
 }
 
-// Miss must agree with what ServeHTTP actually treats as a not-found: the site root
-// and the template's own assets are hits, guessed paths are misses. Probe detection
-// keys off this, so a disagreement would either miss scanners or flag real visitors.
-func TestMissAgreesWithServe(t *testing.T) {
+// Miss must agree with what serveMiss actually returns, since probe detection keys off
+// it. A single-page template (no 404 page) answers an extensionless miss with the
+// index under 200 — that is NOT a miss; only a missing asset (has an extension) is.
+func TestMissMatchesSPAServe(t *testing.T) {
 	h := newTestHandler(t, "filecloud")
 	if h.Miss("/") {
 		t.Error("root reported as a miss; a visitor loading the site must not count")
 	}
-	if !h.Miss("/definitely-not-a-real-path") {
-		t.Error("a guessed path reported as a hit; scanners would go unseen")
+	if h.Miss("/dashboard/files") {
+		t.Error("extensionless path is served the index under 200 — must not count as a miss")
 	}
 	if !h.Miss("/assets/nope.js") {
-		t.Error("a missing asset reported as a hit")
+		t.Error("a missing asset is a genuine 404 — must count as a miss")
+	}
+}
+
+// A classic template ships its own 404 page, so every miss IS a genuine 404 — a
+// guessed path there is the scan signal, extension or not.
+func TestMissMatchesClassicServe(t *testing.T) {
+	h := newTestHandler(t, "coming-soon")
+	if h.Miss("/") {
+		t.Error("root reported as a miss")
+	}
+	if !h.Miss("/definitely-not-a-real-path") {
+		t.Error("a guessed path on a 404-serving site must count as a miss")
 	}
 }
 

internal/geo/lookup.go

@@ -47,7 +47,9 @@ func LoadCountryLookup(dir string) (*CountryLookup, error) {
 		}
 		data = data[n:]
 		msgLen, n := binary.Uvarint(data)
-		if n <= 0 || int(msgLen) > len(data[n:]) {
+		// Unsigned compare: a length varint ≥ 2^63 makes int(msgLen) negative, which
+		// would slip past a signed `> len` check and then panic on the slice.
+		if n <= 0 || msgLen > uint64(len(data[n:])) {
 			break
 		}
 		data = data[n:]
@@ -208,7 +210,9 @@ func readTag(msg []byte) (field uint64, wire byte, rest []byte, ok bool) {
 
 func readBytes(msg []byte) (b, rest []byte, ok bool) {
 	l, n := binary.Uvarint(msg)
-	if n <= 0 || int(l) > len(msg[n:]) {
+	// Unsigned compare so a huge (or overflow-to-negative-when-cast) length can't slip
+	// past the bound and panic the slice below.
+	if n <= 0 || l > uint64(len(msg[n:])) {
 		return nil, nil, false
 	}
 	return msg[n : n+int(l)], msg[n+int(l):], true

internal/geo/lookup_test.go

@@ -76,3 +76,25 @@ func TestCountryLookup(t *testing.T) {
 		}
 	}
 }
+
+// A corrupt or truncated geoip.dat must fail cleanly, never panic — in particular a
+// length prefix ≥ 2^63 (which casts to a negative int) must not slip past the bounds
+// check and slice out of range.
+func TestCountryLookupMalformed(t *testing.T) {
+	dir := t.TempDir()
+	// Top-level entry tag 0x0A followed by an absurd length varint.
+	bad := binary.AppendUvarint([]byte{0x0A}, uint64(1)<<63)
+	if err := os.WriteFile(filepath.Join(dir, "geoip.dat"), bad, 0o644); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	if _, err := LoadCountryLookup(dir); err == nil {
+		t.Error("expected an error for a malformed geoip.dat, got nil")
+	}
+
+	// A truncated valid-looking entry must also not panic.
+	trunc := geoipDat(geoEntry("US", cidrMsg([]byte{8, 8, 8, 0}, 24)))
+	if err := os.WriteFile(filepath.Join(dir, "geoip.dat"), trunc[:len(trunc)-3], 0o644); err != nil {
+		t.Fatalf("write trunc: %v", err)
+	}
+	_, _ = LoadCountryLookup(dir) // must not panic; result is don't-care
+}

internal/server/panel_admin_audit.go

@@ -22,6 +22,21 @@ type adminAuditResponse struct {
 	NextBefore int64              `json:"next_before"`
 }
 
+// csvSafe neutralizes spreadsheet formula injection: a cell beginning with =, +, -, @,
+// or a leading tab/CR can execute as a formula when the export is opened in Excel or
+// Sheets. Prefixing a single quote makes the app treat it as text. Applied to the
+// fields that can carry free-form (potentially user-influenced) text.
+func csvSafe(s string) string {
+	if s == "" {
+		return s
+	}
+	switch s[0] {
+	case '=', '+', '-', '@', '\t', '\r':
+		return "'" + s
+	}
+	return s
+}
+
 // adminAuditFilterFromQuery builds the store filter shared by the paged list and the
 // CSV export, so a search/date/actor/category filter can never mean one thing on
 // screen and another in the exported file. ok=false means the query named a category
@@ -104,7 +119,8 @@ func (rt *Router) exportAdminAudit(w http.ResponseWriter, r *http.Request) {
 				}
 			}
 			ts := time.Unix(ev.CreatedAt, 0).UTC().Format(time.RFC3339)
-			return cw.Write([]string{ts, ev.Action, ev.Target, ev.ActorKind, ev.ActorName, ev.IP, det})
+			return cw.Write([]string{ts, ev.Action,
+				csvSafe(ev.Target), ev.ActorKind, csvSafe(ev.ActorName), ev.IP, csvSafe(det)})
 		})
 		if err != nil {
 			// The 200 and header row are already on the wire, so there is nothing to