refactor(server): remove the dead POST /api/packets endpoint (#223)

BUILD_PLAN.md

@@ -9,7 +9,7 @@ Open-source, self-hosted MeshCore mesh network packet analyzer. Community altern
 - **Frontend**: SPA, vanilla HTML/CSS/JS, Leaflet maps, WebSocket live feed, Canvas animations
 - **Backend**: Node.js + Express + better-sqlite3 + ws + mqtt
 - **Decoder**: Custom `decoder.js` (from MeshCore Packet.h spec)
-- **Data**: SQLite, MQTT ingestion, REST API, manual packet injection
+- **Data**: SQLite, MQTT ingestion, REST API
 
 ## Architecture
 
@@ -19,7 +19,6 @@ The `@michaelhart/meshcore-decoder` npm library has a path parsing bug — treat
 ### Packet Ingestion
 - MQTT subscriber (configurable broker/topic)
 - Companion bridge (BLE → MQTT via `meshcore_observer.py`)
-- POST `/api/packets` for manual injection
 - WebSocket broadcast to all connected clients
 
 ### Channel Decryption
@@ -129,7 +128,6 @@ meshcore-analyzer/
 │   ├── live.js/css     (live view + VCR)
 │   └── vendor/         (third-party libs)
 └── tools/
-    ├── generate-packets.js
     ├── e2e-test.js
     └── frontend-test.js
 ```

cmd/server/apikey_security_test.go

@@ -52,7 +52,7 @@ func TestRequireAPIKey_RejectsWeakKey(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
-	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req := httptest.NewRequest("POST", "/api/perf/reset", nil)
 	req.Header.Set("X-API-Key", "test")
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)
@@ -69,7 +69,7 @@ func TestRequireAPIKey_AcceptsStrongKey(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
-	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req := httptest.NewRequest("POST", "/api/perf/reset", nil)
 	req.Header.Set("X-API-Key", strongKey)
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)
@@ -85,7 +85,7 @@ func TestRequireAPIKey_EmptyKeyDisablesEndpoints(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
-	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req := httptest.NewRequest("POST", "/api/perf/reset", nil)
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)
 
@@ -100,7 +100,7 @@ func TestRequireAPIKey_WrongKeyUnauthorized(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	}))
 
-	req := httptest.NewRequest("POST", "/api/packets", nil)
+	req := httptest.NewRequest("POST", "/api/perf/reset", nil)
 	req.Header.Set("X-API-Key", "wrong-key-entirely-here")
 	rr := httptest.NewRecorder()
 	handler.ServeHTTP(rr, req)

cmd/server/decoder.go

@@ -4,7 +4,6 @@ import (
 	"crypto/sha256"
 	"encoding/binary"
 	"encoding/hex"
-	"encoding/json"
 	"fmt"
 	"math"
 	"strings"
@@ -720,15 +719,6 @@ func ComputeContentHash(rawHex string) string {
 	return hex.EncodeToString(h[:])[:16]
 }
 
-// PayloadJSON serializes the payload to JSON for DB storage.
-func PayloadJSON(p *Payload) string {
-	b, err := json.Marshal(p)
-	if err != nil {
-		return "{}"
-	}
-	return string(b)
-}
-
 // ValidateAdvert checks decoded advert data before DB insertion.
 func ValidateAdvert(p *Payload) (bool, string) {
 	if p == nil || p.Error != "" {

cmd/server/openapi.go

@@ -66,7 +66,6 @@ func routeDescriptions() map[string]routeMeta {
 				{Name: "search", Description: "Full-text search", Type: "string"},
 				{Name: "groupByHash", Description: "Group duplicate packets by hash", Type: "boolean"},
 			}},
-		"POST /api/packets":              {Summary: "Ingest a packet", Description: "Submit a raw packet for decoding and storage.", Tag: "packets", Auth: true},
 		"GET /api/packets/{id}":          {Summary: "Get packet detail", Tag: "packets"},
 		"GET /api/packets/timestamps":    {Summary: "Get packet timestamp ranges", Tag: "packets"},
 		"POST /api/packets/observations": {Summary: "Batch submit observations", Description: "Submit multiple observer sightings for existing packets.", Tag: "packets"},

cmd/server/openapi_test.go

@@ -6,6 +6,8 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+
+	"github.com/gorilla/mux"
 )
 
 func TestOpenAPISpecEndpoint(t *testing.T) {
@@ -139,4 +141,27 @@ func TestExtractPathParams(t *testing.T) {
 	}
 }
 
-
+// The served spec is built by walking the router, so a description left in
+// routeDescriptions for a removed route (like "POST /api/packets") never
+// shows up there and would rot silently. Every description must name a
+// registered method and path.
+func TestOpenAPIDescriptionsHaveRoutes(t *testing.T) {
+	_, router := setupTestServer(t)
+	registered := map[string]bool{}
+	router.Walk(func(route *mux.Route, _ *mux.Router, _ []*mux.Route) error {
+		path, err := route.GetPathTemplate()
+		if err != nil {
+			return nil
+		}
+		methods, _ := route.GetMethods()
+		for _, m := range methods {
+			registered[m+" "+path] = true
+		}
+		return nil
+	})
+	for key := range routeDescriptions() {
+		if !registered[key] {
+			t.Errorf("routeDescriptions has %q, but no such route is registered", key)
+		}
+	}
+}

cmd/server/post_packets_removed_223_test.go

@@ -0,0 +1,165 @@
+package main
+
+import (
+	"database/sql"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gorilla/mux"
+)
+
+// POST /api/packets (#223) inserted into transmissions, observers and
+// observations on the server's mode=ro handle (#1283), so every call failed
+// with a 500 carrying the raw SQLite error. It was removed: ingest goes
+// through MQTT and cmd/ingestor. These tests pin what is left; the source
+// guard against packet-table writes is TestServerHasNoPacketTableWrites
+// (readonly_invariant_test.go).
+
+const removedPostAPIKey = "test-secret-key-strong-enough"
+
+// removedPostPacketBody is a valid FLOOD/ADVERT body that the old handler
+// accepted (it decoded, then failed on the INSERT).
+const removedPostPacketBody = `{"hex":"110011223344556677889900AABBCCDD","observer":"obs1","snr":5.5,"rssi":-72}`
+
+// readOnlyPacketServer seeds a file DB, opens it the way main.go does
+// (OpenDB, mode=ro) and registers the API routes with a valid API key.
+func readOnlyPacketServer(t *testing.T) (dbPath string, router *mux.Router) {
+	t.Helper()
+	dbPath = filepath.Join(t.TempDir(), "ro.db")
+	now := time.Now().UTC()
+	seedTestDBRows(t, dbPath, 3, 2, func(i int) (string, int64) {
+		ts := now.Add(-time.Duration(i) * time.Minute)
+		return ts.Format(time.RFC3339), ts.Unix()
+	})
+	db, err := OpenDB(dbPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { db.conn.Close() })
+	srv := NewServer(db, &Config{Port: 3000, APIKey: removedPostAPIKey}, NewHub())
+	router = mux.NewRouter()
+	srv.RegisterRoutes(router)
+	return dbPath, router
+}
+
+// packetTableCounts reads the row counts of the tables the old handler wrote,
+// through a separate connection.
+func packetTableCounts(t *testing.T, dbPath string) map[string]int {
+	t.Helper()
+	conn, err := sql.Open("sqlite", "file:"+dbPath+"?mode=ro")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+	out := map[string]int{}
+	for _, table := range []string{"transmissions", "observations", "observers"} {
+		var n int
+		if err := conn.QueryRow("SELECT COUNT(*) FROM " + table).Scan(&n); err != nil {
+			t.Fatalf("count %s: %v", table, err)
+		}
+		out[table] = n
+	}
+	return out
+}
+
+func postRemovedPacket(router http.Handler) *httptest.ResponseRecorder {
+	req := httptest.NewRequest("POST", "/api/packets", strings.NewReader(removedPostPacketBody))
+	req.Header.Set("X-API-Key", removedPostAPIKey)
+	req.Header.Set("Content-Type", "application/json")
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+	return w
+}
+
+// On the API router, POST /api/packets now hits the GET route's path with the
+// wrong method: gorilla/mux answers 405 before any handler or DB access. A
+// valid key and a decodable body make no difference, and the read-only DB is
+// left untouched.
+func TestPostPacketsRemovedReturns405OnReadOnlyDB(t *testing.T) {
+	dbPath, router := readOnlyPacketServer(t)
+	before := packetTableCounts(t, dbPath)
+
+	w := postRemovedPacket(router)
+	if w.Code != http.StatusMethodNotAllowed {
+		t.Fatalf("POST /api/packets: want 405, got %d (body: %q)", w.Code, w.Body.String())
+	}
+	if body := strings.ToLower(w.Body.String()); strings.Contains(body, "sqlite") || strings.Contains(body, "readonly") || strings.Contains(body, "insert") {
+		t.Errorf("response leaks database error text: %q", w.Body.String())
+	}
+	if after := packetTableCounts(t, dbPath); fmt.Sprint(after) != fmt.Sprint(before) {
+		t.Errorf("packet tables changed: before %v, after %v", before, after)
+	}
+}
+
+// The other /api/packets routes keep their registration. Matching does not
+// run handlers, so this pins only the routing, not handler output.
+func TestPacketsRoutesSurviveRemoval(t *testing.T) {
+	_, router := readOnlyPacketServer(t)
+	for _, c := range []struct{ method, path string }{
+		{"GET", "/api/packets"},
+		{"GET", "/api/packets/timestamps"},
+		{"POST", "/api/packets/observations"},
+		{"GET", "/api/packets/abc123"},
+		{"GET", "/api/packets/abc123/path"},
+		{"POST", "/api/decode"},
+	} {
+		var m mux.RouteMatch
+		if !router.Match(httptest.NewRequest(c.method, c.path, nil), &m) || m.MatchErr != nil {
+			t.Errorf("%s %s: no longer routed (err %v)", c.method, c.path, m.MatchErr)
+		}
+	}
+}
+
+// The served OpenAPI spec keeps GET /api/packets and drops the POST.
+func TestOpenAPISpecHasNoPostPackets(t *testing.T) {
+	_, router := readOnlyPacketServer(t)
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, httptest.NewRequest("GET", "/api/spec", nil))
+	if w.Code != http.StatusOK {
+		t.Fatalf("GET /api/spec: want 200, got %d", w.Code)
+	}
+	var spec struct {
+		Paths map[string]map[string]json.RawMessage `json:"paths"`
+	}
+	if err := json.Unmarshal(w.Body.Bytes(), &spec); err != nil {
+		t.Fatal(err)
+	}
+	ops := spec.Paths["/api/packets"]
+	if _, ok := ops["get"]; !ok {
+		t.Errorf("/api/packets lost its get operation: %v", ops)
+	}
+	if _, ok := ops["post"]; ok {
+		t.Errorf("/api/packets still documents a post operation")
+	}
+}
+
+// main.go mounts a catch-all SPA handler after the API routes. With it in
+// place, gorilla/mux lets the catch-all win over the method mismatch, so a
+// POST to the removed endpoint is served index.html like any other unmatched
+// path (pre-existing fallback behaviour, not specific to #223). Pin that it
+// is the SPA page, not JSON, and that nothing is written.
+func TestPostPacketsRemovedFallsThroughToSPAInProductionRouter(t *testing.T) {
+	dbPath, router := readOnlyPacketServer(t)
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "index.html"), []byte("<html>SPA</html>"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	router.PathPrefix("/").Handler(wsOrStatic(NewHub(), spaHandler(dir, http.FileServer(http.Dir(dir)))))
+	before := packetTableCounts(t, dbPath)
+
+	w := postRemovedPacket(router)
+	if w.Code != http.StatusOK || !strings.HasPrefix(w.Header().Get("Content-Type"), "text/html") || w.Body.String() != "<html>SPA</html>" {
+		t.Fatalf("POST /api/packets with SPA fallback: want 200 text/html index.html, got %d %q %q",
+			w.Code, w.Header().Get("Content-Type"), w.Body.String())
+	}
+	if after := packetTableCounts(t, dbPath); fmt.Sprint(after) != fmt.Sprint(before) {
+		t.Errorf("packet tables changed: before %v, after %v", before, after)
+	}
+}