diff --git a/CLAUDE.md b/CLAUDE.md index 526cd74..4966231 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2374,6 +2374,102 @@ socket for half a minute. Measured: `last_error` is reported with the camera, because `connected: false` alone cannot tell a wrong IP from a wrong password, and those are different jobs. +## The admin console could list merchants and see nothing inside them + +`GET /api/admin/clients/{id}` and, under it, `/sites`, `/sites/{site}`, +`/sites/{site}/cameras`, `/sites/{site}/cameras/{camera}`, plus +`/api/admin/monitoring/summary`. The head-office console drills down +merchant -> store -> camera and every level below the first showed *"Backend +integration required"*. + +They cannot be the tenant routes, and the reason is structural. Every tenant +handler derives the client from the **session** - that is what makes +cross-tenant access impossible rather than merely disallowed - and a platform +admin has no client at all. The three workarounds each make it worse: passing +a company id to a tenant route puts a caller-chosen tenant back in the one +place this system refuses to take one, filtering the estate in the browser +ships every merchant's data to render one, and signing in as the owner audits +the wrong person. So the tenant STORE functions are reused with an explicit +client id - they already take one - and the scoping the tenant handlers get +from the session happens in the handler instead. + +- **`AdminCamera` is a separate type from `Camera`**, for the same reason + `AgentCamera` is: it cannot carry `host`, `port`, `path`, `username` or + `has_password`. A tenant seeing those for their own camera is correct; a + platform admin browsing another company's estate is a different question, + and an RTSP host with a username beside it is most of a live path into a + customer's camera. Blanking fields on a shared struct leaves "remember to + redact, on every path, forever" as the only thing preventing a leak. The + test asserts on the **raw JSON**, because decoding into the struct would + discard exactly what it is looking for. +- **An unowned site is 404, never an empty list.** `[]` says "this shop has no + cameras" when the truth is "not your shop". +- **Every read below the merchant list writes an audit row.** An admin is the + one account for which nothing else here leaves a trace. The counts-only + summary does not: a console refreshes it on a timer, and logging that buries + the reads worth finding. +- **A suspended merchant stays readable** - that is precisely what an admin + opens the console to look at. + +Alongside it, the two merchant-side reads that were only ever aggregates: +`GET /api/sales` and `/api/sales/{id}` over the `purchases` table the +conversion report has summed since it existed, and +`GET /api/dashboard/summary`. No cursor on the sales list, deliberately: a +keyset cursor needs a monotonic server-assigned column and `purchases` has +none, so ordering by `(occurred_at, id)` with a random uuid tie-break is +exactly the shape that dropped four of six simultaneous visits before +`visits.seq` existed. Offering one would imply a delivery guarantee this table +cannot make. + +### What the fake could not catch, and the database did immediately + +Both of these passed every in-memory test and failed on the first real call. + +**A wrong URL answered 500.** `c.id = $1::uuid` makes Postgres cast the path +segment, and casting a malformed string - or the empty one a shape check hands +back in its place - is an **error**, not a miss. `c.id::text = $1` cannot fail. +The two sibling resolvers were already written that way and correctly 404'd the +same input: the rule was applied to two of three places, which is the shape of +a rule that holds until somebody adds the next one. `api_admin_monitor_live_test.go` +asserts it where the property actually lives. + +**Five live endpoints answered 500 to a platform admin** - `/api/visits`, +`/api/cameras`, `/api/sites`, `/api/visitors`, `/api/reports/footfall` - and had +done since they shipped. Same cause one level up: a platform admin has no +client, every tenant query scopes on `client_id = $1::uuid`, and `''::uuid` is +a cast error. `tenantOnly` is the guard, beside `adminOnly` and for the +opposite audience. **403, not 404**, because the two hide opposite things: a +tenant must not learn a platform surface exists, while a platform admin already +knows the tenant surface does - so the refusal names the route to use instead. +`/api/auth/*` stays ungated: a session is not a company's data, and revoking a +lost device must work for an account with no tenant. + +Guarding at the chokepoint rather than per query is the point. A per-query cast +is a fix the next query forgets, and the next query would 500 in production +exactly as these did. + +### Two bugs in the deploy script, both found by running it + +- **`go: command not found` at step 1**, on the machine the script was written + on. Go sits in a directory `.zprofile` adds and a script does not inherit. A + deploy that needs the operator to fix their environment first is a deploy + that gets skipped, which is the failure this script exists to end. +- **Step 3 reported the wrong backup.** `ls | tail -1` sorts alphabetically, so + `pre-...-demo-12` sorts before `pre-...-demo-6` and it printed a dump from + four days earlier. A deploy that names the wrong safety net is worse than one + that names none - that is the file somebody reaches for at the worst moment. +- Step 7 verified five routes and none of them were the nine that had just + shipped. It checks all of them now and treats **401 as a pass**: an + unauthenticated call to a route that exists is refused, while one the binary + never registered is a 404. That makes the step prove the *routing*, which is + what a deploy gets wrong, and a missing route now fails the deploy loudly. + +Verified live on 2026-09-28 against production (`v0.4.8-demo-14-g830c1c1`): +merchant detail with its owner, the drill-down by slug and by uuid, camera rows +carrying no host or username, another merchant's shop and camera both 404, +malformed identifiers 404 rather than 500, one real sale (INR 1000, V-1) read +back by id, and every tenant route still 200 for an ordinary tenant account. + ## Setting up on a new machine 1. Copy the `Behavision` folder **including `.env`** (gitignored, holds