A hidden menu item is not a gate

Behind almost every button in the Bookatu admin is a server action: a function that runs on our server when the button is pressed. Who may press it is decided per business, by a grid of permissions. Owner, admin, staff and trainee start from defaults, and the owner can change every row but their own. A front desk might be given Edit the calendar.

The grid was already checked in many places. What we had not done was ask the question the other way round, for every action we have: if this function runs, what made sure the person running it may?

The menu is not the gate

The admin menu decides what each person sees. Staff see only the items marked safe for them, and trainees see none:

export function isNavItemVisible(item: { staff?: boolean }, role: Role | null | undefined): boolean {
  if (role === "trainee") return false;
  if (role === "staff") return item.staff === true;
  return true;
}

That is a good rule for a menu, whose job is to keep the screen tidy for someone who does not set the business up. It is not a rule for anything else. Packages, memberships, promo codes, the floor plan, forms, programs, the gallery and the blog all sit behind items staff never see, and on several of them the actions only checked that the caller belonged to the business. Delete was usually gated. Create, edit and publish often were not.

A menu decides what you see. Only the action decides what you can do.

A server action is an endpoint. The page that draws its button is one way to reach it, not the only one. So a hidden menu item says nothing about who can run the action behind it.

A page gate is not an action gate

The next layer is the page. Most setup pages call requireAdmin, which checks the Manage permission on this business's grid and sends anyone else back to the dashboard:

export async function requireAdmin(slug: string): Promise<OrgAccess> {
  const access = await requireOrgAccess(slug);
  if (!(await hasPermission(access.role, PERMISSION.Manage, access.org.id))) redirect(`/${slug}/admin`);
  return access;
}

The Settings page has used it for a long time. Its actions did not all follow. Several asked something narrower and older, whether the caller was an owner or an admin by name:

const access = await requireOrgAccess(slug, ["owner", "admin"]);

Some pages disagreed the other way: the menu item was for Manage only, but the page and its actions let any member in. Only the action decides what gets written, so a page's gate is now a promise its actions keep. If a page is for Manage, every action it renders checks Manage itself, and the packages, memberships, promo codes, blog, announcements and reviews pages now use requireAdmin too.

The role's name is not the permission

Checking the name looks safe, because on the default grid owners and admins are exactly the people with Manage. It stops being safe the day an owner changes the grid, and it goes wrong in both directions.

An owner can untick Manage the business for Admin. The admin can then no longer open Settings, as the owner asked, but the saves behind it still asked for the name, and an admin is still called admin. The other way round, an owner can trust a staff member with Manage: requireAdmin let them onto the page, and the name check behind it refused everything they saved. The Settings actions now ask what the page asks:

/**
 * Who may save the Settings page: the page is requireAdmin, so what it saves is
 * the business's setup, the Manage permission on this business's own grid. Not
 * the role NAME: an owner can take Manage from Admin (the admin is then turned
 * away from the page, so they must be from its saves too) or trust a staff
 * member with it.
 * ...
 */
async function settingsEditor(slug: string): Promise<OrgAccess> {
  const access = await mayEditSettings(slug);
  if (!access) redirect(`/${slug}/admin`);
  return access;
}

/** The same check for the saves that answer their form: null = refused, nothing written. */
async function mayEditSettings(slug: string): Promise<OrgAccess | null> {
  const access = await requireOrgAccess(slug);
  return (await hasPermission(access.role, PERMISSION.Manage, access.org.id)) ? access : null;
}

The business's id in that call matters as much as the permission. Without it, hasPermission answers from the platform's default grid, which is wrong for exactly the businesses that changed theirs. The same fix covered the team, services, products and stock, classes, forms, programs, reviews, newsletters and privacy. Invoices now follow the Finance page they live on, Manage plus See analytics & earnings, and adding a gallery storage pack changes the bill, so it needs Manage billing.

The admin assistant needed the same rule one level down. It stays for owners and admins, on purpose, but an admin the owner had made read-only could still ask it to book someone in, confirm, and the booking was made. Each tool now names what it needs, in a Record over every tool, so a new one cannot be added without deciding:

export const TOOL_PERMISSION: Record<AssistantTool, Permission | null> = {
  create_product: PERMISSION.Manage,
  update_service: PERMISSION.Manage,
  // ...
  sell_product: null,
  create_booking: PERMISSION.EditCalendar,
  reschedule_booking: PERMISSION.EditCalendar,
  // ...
  create_customer: null,
  create_form: PERMISSION.Manage,
  undo_create_form: PERMISSION.Manage,
  // ...
};

The check runs at every entry point: confirming one change, running a plan, undo and redo. A plan the role can only partly carry out runs none of it, because running the allowed steps and refusing halfway leaves the business in a state nobody asked for. The model is also told this month's revenue only when the person asking may see it.

Reads leak too

Not every gap was a write. Without See everyone's calendar, which staff lack by default, the calendar draws only your own column, but the jump-to-date helper beside it returned every column's bookings with the client's name and service. It now keeps the whole grid, so you can still see when people are busy, and labels any booking you are not on as "Busy", without reading that client's name.

Refusing quietly, or telling the person

Every refusal chooses between saying nothing and saying why. Our rule: if the app never offers the button to this person, the refusal can be quiet. The action does nothing, or sends them to the dashboard as requireAdmin always has, because nobody pressed a button to get there. If the app does offer the button, the person gets a sentence in the words of the grid:

const SETTINGS_REFUSED = "Your role can't change the business's settings. Ask the owner.";

export function toolRefusalMessage(permission: Permission): string {
  return permission === PERMISSION.EditCalendar
    ? "Your role can't change the calendar. Ask the owner."
    : "Your role can't change the business's services, products, stock, forms or announcements. Ask the owner.";
}

Where the better answer was to stop offering the button, we stopped offering it. Deleting a client's photo needs Delete records, and the button is now hidden from anyone without it. Mark fulfilled follows Fulfil orders the same way.

Our browser journeys found the same mistake from the other side: a Remove button the server refused and the page ignored. It is in our post on browser tests, and fixed in this release too.

The public side is about ownership

On the booking site and in the client portal, the question is whose record this is. The auditor for that area followed every client-facing action to the row it changes, and asked whether the session or signed link proves the client owns it, at the business in the address.

Most actions already got this right. A client cancelling a class place from their account sends the place's id, and the id only chooses among their own:

/**
 * A signed-in client cancels one of their own class places from their account
 * page. Whose place it can be is the session's to say: the enrolment id arrives
 * from the browser, and cancelMyPlace only ever reaches this client's own place
 * at this business, by the business's own rules.
 */
export async function cancelMyClassPlaceAction(slug: string, enrolmentId: string): Promise<void> {
  const org = await requireOrg(slug);
  const session = await getCustomerSession();
  if (!session || session.orgId !== org.id) redirect(`/${slug}/account`);

  const res = await cancelMyPlace({
    org,
    customerId: session.customerId,
    enrolmentId: String(enrolmentId ?? ""),
    via: "account",
  });
  // ...
}

That is only as good as the session behind it, which is where both public gaps were.

Stop every link means every link

A studio can press "stop every link" when a gallery link has gone to the wrong person. It adds one to the client's session epoch, and every link carries the epoch it was issued with, so all of them stop at once. They did stop. But the 30-day portal cookie a link sets never looked at the epoch again, so a browser that had already opened one stayed signed in as the client. The cookie now carries the epoch too, and every read of the session checks it:

export async function getCustomerSession(): Promise<CustomerSession | null> {
  const store = await cookies();
  const parsed = decode(store.get(COOKIE)?.value ?? store.get(LEGACY_COOKIE)?.value);
  if (!parsed) return null;
  // ...
  const { customerSessionEpoch } = await import("@/lib/customer-links");
  const current = await customerSessionEpoch(parsed.orgId, parsed.customerId);
  if (current === null || current !== (parsed.epoch ?? 0)) return null;
  return parsed;
}

Cookies set before the change read as epoch 0, the starting value. A client whose links were never stopped is still at 0, so the release itself signs nobody out. A client whose links were stopped is not, and their old cookie stops working, which is what the button always promised. Magic links and Google sign-in both stamp the epoch now.

A card on file is changed by its owner

The booking form can save a card for no-show protection. It finds the client from the contact details typed into it, and finding is not proof: the person at the keyboard might not be that client. Two changes close it. A client who already has a card or a consent on file has to be signed in to their own portal to change it:

if (onFile?.stripePaymentMethodId || onFile?.cardMandateAcceptedAt) {
  const session = await getCustomerSession();
  if (!session || session.orgId !== org.id || session.customerId !== customerId) {
    return { ok: false, reason: "sign_in_required" };
  }
}

That answer comes before any call to the payment provider or any write, and the booking step shows it as a sign-in prompt. And the card and the consent are now written to the client only when the payment provider confirms the card was saved, so a form someone abandons changes nothing.

How the audit ran

The audit had four steps, each with one job.

1. Audit, read-only, by area. Five auditors, one area each: configuration, the catalog, operations, marketing and the rest of the admin, and the public site. They could read the code and change nothing. For every exported action they followed the gate to the end, through helpers and library code, found the page and menu item that offer it, and wrote down who can call it today and who should. 40 candidates came back.
2. Verify, adversarially. One judge, whose job was to refute each candidate in the code: a check further down the chain, an action nothing can reach, or behaviour meant for staff because a page they use offers it. The judge was told to be careful about taking abilities away. 34 survived.
3. Fix, test-first, in two batches. Two fixers in separate worktrees: 25 findings about the business's setup, and 9 about operations, the assistant and the public side. Tests first, then code.
4. Merge and run everything. Both batches merged with no conflicts, then the full suite ran on a freshly migrated scratch database, and a full build.

The six rejections were worth as much as the 34. Someone without Edit the calendar may schedule class sessions that do not clash, on purpose; only Schedule anyway, over a clash, needs it. Opening hours changed from the calendar follow Edit the calendar, as their own comment says. A photographer's delivery controls sit on the client page staff already use. A form's share link returns nothing clients cannot already see. Price tiers are not a record type Delete records covers. And opening the assistant to staff would have widened it. Each would have taken away something a business may rely on.

Tests on the old code first

The fixes followed our rule for any change to code that already works. Characterization tests pin who must keep working: the owner, an admin on the default grid, and a staff member the owner trusted with the permission, and that staff and trainees who were refused stay refused. They have to pass on the old code. New-behaviour tests check that a refused caller writes nothing, sends nothing and gets nothing back. They have to fail on it.

Each case says how to set up a fresh target, how to call the action and what to look at. The same look decides both directions:

/** A staff member the owner trusted with `perms` on top of the default grid. */
export function staffGranted(...perms: Permission[]): Caller {
  return { label: `a staff member granted ${perms.join(" + ")}`, login: "staff", grid: { role: "staff", grant: perms } };
}

/** An admin from whom the owner took `perms` away. */
export function adminWithout(...perms: Permission[]): Caller {
  return { label: `an admin without ${perms.join(" + ")}`, login: "admin", grid: { role: "admin", revoke: perms } };
}

/** The caller is refused: nothing `look` sees changes, and nothing leaks back. */
export async function expectNoWrite<S>(c: GateCase<S>, who: Caller): Promise<Outcome> {
  const s = await c.seed();
  const before = await c.look(s);
  const out = await settle(c.run(s));
  await drain();
  const after = await c.look(s);
  expect(after, `${c.name} by ${who.label}`).toEqual(before);
  c.refused?.(out, s);
  return out;
}

Nothing about access is faked: each call carries a real session cookie, against real membership and grid rows. The admin without Manage and the staff member granted it are the two callers a name check gets wrong, and every gate is tested with both.

A script runs the new test files on a worktree of the base commit. The first batch had 12 characterization files with 388 tests, all green on the base, and 12 new-behaviour files with 223 tests, every one red with an assertion failure: the write happened, or the redirect did not. The second batch added 58 characterization and 43 new-behaviour tests, with the same result. Fourteen existing tests had to change because a fix contradicts them, each marked CHANGED DELIBERATELY with the reason. Four of them had pinned that a staff member could still save a service as it was. After the merge the full suite passed: 1,129 files and 14,563 tests.

What we left alone on purpose

• Staff with Manage still cannot make API keys. A characterization test pins that decision, and a fix does not overturn one in passing. The gap the audit found was the other direction, an admin without Manage, and that is closed.
• The assistant stays for owners and admins. Each tool inside it now follows the grid.
• The retention page stays open to staff, because their dashboard links to it. Its send buttons were the gap, and they need Manage.
• Removing a gallery storage pack still needs only Manage, because it never adds to the bill.

We found all of this in our own audit and fixed it before anyone reported misuse. We have no evidence of misuse.

What we take from it

• Gate the action, not the menu. A menu is for tidiness. Anything that writes or reads something private asks for itself.
• If a page has a gate, every action it renders repeats it. The page makes the promise and the actions keep it.
• Ask for the permission on this business's grid, never the role's name, and always pass the business. A check that is right on the default grid is wrong for exactly the owners who used the grid.
• Refuse quietly only where the app never offered the button. Where it did, say why, or stop offering it.
• Give your auditors a skeptic. Six of forty findings would have taken away something a business relies on.