Sprint 15 Closes: Closing the Multi-Tenancy Gaps, and the Vulnerability a New Admin Role Exposed the Same Day
Building out demo material for four differently-configured businesses surfaced a bug worth taking seriously: logging into one business's admin dashboard showed "Total Beers: 74" and "Active Members: 13" — the sum across all four demo businesses, not that business's own 8 beers and 1 customer. The customer-facing side of the app had just earned real, tested tenant isolation. The admin side hadn't.
Scoping it properly before touching any code
The instinct with a bug like this is to grep for the obvious controller and patch it. Instead this got a proper audit first: a read-only pass over every admin-facing endpoint, followed by a design pass sizing the actual fix. The scope came back bigger than the symptom suggested. Seven of nine admin controllers needed a genuine new tenant filter. Four of those needed something extra on top of the list filter — an ownership check, since an admin could still open or edit another business's specific record by guessing its id, even once the list view only showed their own. And three tables had no tenant column at all, which meant those three needed to be scoped indirectly, by joining through the customer or item they belonged to instead.
An assembly line, not one long session
With the plan sized at nine discrete tasks, I ran it as an assembly line instead of working through the list myself end to end: each task got a fresh implementer starting from nothing but that task's own brief, and a separate, independent reviewer who also saw only that task's brief and diff, never the full plan or what any earlier task had done. That isolation is the point. A reviewer who's read the whole plan tends to review it against their own memory of the plan, which is a weaker check than reviewing the actual diff cold, brief in hand, with no accumulated assumptions about what "should" be there.
Nine tasks went through that pattern cleanly: catalog mutations, an audit-log read path, the dashboard totals that started this, an analytics report, a couple of controllers with no direct tenant column that needed the join-through approach, an account-management controller where fixing the read side surfaced an unrelated write-side bug (staff accounts invited by an admin had been silently defaulting to the wrong business's ownership), and an anomaly detector that turned out to be the exact source of the false alert the moldability demo had tripped by seeding four businesses within the same hour.
What nine careful reviews still missed
Every one of those nine tasks passed its own review clean. A tenth pass, a single independent read of the entire finished branch rather than any one task's diff, still found real problems the task-by-task structure had been blind to.
The nine-task plan had been built by searching for controller files with "Admin" in the name. Two controllers carried the same admin-only authorization check without that naming convention, and both had the exact bug this whole effort existed to close: one leaked which customers had earned a reward across every business, not just their own; the other let an admin from one business set or deactivate another business's staff PIN outright, which is a real way to lock a competitor's confirmation flow, not a theoretical one. The whole-branch pass also caught a missing test at the actual HTTP-request layer (the earlier fix had only been tested at the method level underneath it), and roughly thirty lines of explanatory comments that had been silently dropped across three of the nine tasks' full-file rewrites.
All of it got fixed in one more pass, independently re-verified afterward. The lesson worth keeping: "admin-facing" in this codebase is defined by an authorization attribute, not by a filename pattern, and the next audit of this shape should search for the attribute directly.
The gap that stayed open on purpose, and the one that didn't
One control point came out of that audit flagged, not fixed: the settings endpoint that lets a business configure its own goal, item names, and branding had no tenant check at all, meaning any admin could rewrite any business's settings. That one needed a product decision, not just a code fix — should every admin be strictly scoped to their own business, or is there a separate role for someone who legitimately manages more than one? That question got carried forward rather than answered under time pressure.
The answer, decided the next day, was a new role: a platform-level administrator, distinct from a business's own admin, who can manage any business's settings. That closed the settings gap. But adding a new, more powerful role is itself an event worth scrutinizing, and it was: the same session that shipped it found that the endpoint for assigning roles had never validated what it was given. It blindly replaced all of a person's current roles with whatever was posted, which meant an unrelated role change could silently strip the new platform-level role from someone who had it, or, worse, any ordinary admin could hand it to themselves outright. That got restricted to the three roles an admin is actually allowed to grant, with the new role explicitly protected from being touched by that endpoint at all — fixed the same day it was introduced, before it shipped as a separate finding.
The last remaining gap, a staff PIN collision between two different businesses that happened to issue the same six-digit code, got the same tenant-scoped check the confirmation flow already used elsewhere in the app.
The PM angle
The real decision here wasn't any individual fix. It was treating "the review passed" as a checkpoint, not a finish line. Nine independently reviewed tasks all came back clean, and the branch still had two live, exploitable gaps in it — not because any single review was careless, but because task-level review is structurally incapable of seeing a pattern across tasks. The fix for that isn't reviewing harder within each task. It's adding a different kind of look, at a different altitude, after the pieces are all in place.
The second habit worth keeping: a new permission level is a bigger event than the feature it unblocks, and it deserves scrutiny in its own right, immediately, not on its own separate timeline. Shipping the platform-admin role and finding its side effect in the same sitting, rather than a week later as an incident, is the version of that I want to keep repeating.
Where the project stands
The tenant-scoping audit closed at 527 backend tests passing, ten commits, one per task plus the whole-branch fix wave. Sprint 15 itself closed the same week at 535, four more commits: the platform-admin role, the same-day role-assignment fix, the staff-PIN collision fix, and a strengthened regression test that now exercises both branches of the PIN-matching logic instead of just one. One deliberate non-fix stayed on the record rather than getting silently patched: a legacy fallback that resolves an untagged account to "the first business in the database" instead of a true "can't determine this account's business" state — inherited behavior from the original multi-tenancy work, flagged again here, not something a scoping audit should change unilaterally.
That closed the last known code gap standing between the app and an actual AWS deployment — the next thing up, and covered next.
Comments
Loading comments...