A shop filter that did not filter handed back the whole tenant

GET /api/cameras read only site_id, while every other filtered endpoint
takes both spellings through siteParam. So ?site=chennai was not a
filter at all but an unknown query parameter, silently ignored, and the
caller got every camera in the tenant believing it had one shop's.

Found by using it: a setup script saw another shop's cameras, concluded
three shops already had theirs and created none; then a delete aimed at
a test shop removed the live Coimbatore entrance camera, which had to be
restored. This is exactly the hazard already recorded for site vs
site_id - the note existed, the handler was simply missed.

One line to fix, and a test that asserts the whole class rather than
this one route: both spellings must narrow, and only an absent filter
may return more than one shop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGcjxF1cNLcuwc3DAPcnfj

server/internal/api/handlers_cameras.go

@@ -21,7 +21,13 @@ const snapshotTTL = 5 * time.Minute
 
 func (s *Server) handleCameras(w http.ResponseWriter, r *http.Request) {
 	p := PrincipalFrom(r.Context())
-	siteID := trim(r.URL.Query().Get("site_id"))
+	// siteParam, not Query().Get("site_id"): every other filtered endpoint
+	// takes both spellings, and this one took only the longer. `?site=chennai`
+	// was therefore not a filter but an unknown parameter, silently ignored -
+	// so a caller asking for one shop's cameras was handed the whole tenant's.
+	// Measured: it made a script skip creating cameras for three shops because
+	// another shop's already existed, and deleted a camera from the wrong shop.
+	siteID := siteParam(r)
 	if siteID != "" {
 		var ok bool
 		if siteID, ok = s.resolveSiteFilter(w, r, siteID); !ok {

server/internal/api/sitefilter_test.go

@@ -0,0 +1,61 @@
+package api
+
+import (
+	"encoding/json"
+	"net/http"
+	"testing"
+)
+
+// Every endpoint that narrows by shop must accept BOTH spellings, because an
+// unknown query parameter is silently ignored - so the wrong one is not an
+// error, it is the whole estate returned as though it were one shop. That is a
+// wrong answer nobody would question, and it has already caused a camera to be
+// deleted from the wrong shop.
+func TestEveryShopFilterAcceptsBothSpellings(t *testing.T) {
+	s, fs := newServer(t)
+	seedUser(fs)
+	fs.sites = []SiteHealth{
+		{SiteID: siteA, Slug: "chennai", Name: "TeNext Coimbatore"},
+		{SiteID: "site-other", Slug: "other-shop", Name: "Other Shop"},
+	}
+	fs.cameras = []Camera{
+		{ID: "c1", SiteID: siteA, CameraID: "entrance"},
+		{ID: "c2", SiteID: "site-other", CameraID: "backdoor"},
+	}
+	sess := login(t, s, "manager@acme.com", "correct horse battery")
+
+	for _, path := range []string{
+		"/api/cameras?site=chennai",
+		"/api/cameras?site_id=" + siteA,
+	} {
+		rec := do(t, s, "GET", path, sess.Token, nil)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("%s: got %d: %s", path, rec.Code, rec.Body.String())
+		}
+		var got []Camera
+		if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
+			t.Fatalf("%s: %v", path, err)
+		}
+		if len(got) != 1 || got[0].CameraID != "entrance" {
+			t.Errorf("%s returned %d cameras %v - a shop filter that does not filter hands back the whole tenant",
+				path, len(got), names(got))
+		}
+	}
+
+	// And with no filter at all, the tenant's cameras - which is the only case
+	// that should ever return more than one shop's.
+	rec := do(t, s, "GET", "/api/cameras", sess.Token, nil)
+	var all []Camera
+	_ = json.Unmarshal(rec.Body.Bytes(), &all)
+	if len(all) != 2 {
+		t.Errorf("unfiltered list returned %d, want 2", len(all))
+	}
+}
+
+func names(cams []Camera) []string {
+	out := make([]string, 0, len(cams))
+	for _, c := range cams {
+		out = append(out, c.CameraID)
+	}
+	return out
+}