Record the admin API, and the two 500s only production found
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KGcjxF1cNLcuwc3DAPcnfj
This commit is contained in:
96
CLAUDE.md
96
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
|
||||
|
||||
Reference in New Issue
Block a user