fix(security): lock the users collection against privilege escalation

The users collection kept PocketBase's defaults because the Plan 1a rules
migration covered eight collections and missed this one:

  createRule ""                      -> anyone could sign up
  updateRule "id = @request.auth.id"  -> anyone could edit their own record

`role` is an ordinary field here, so both were escalations. Verified
exploitable against the live instance: an anonymous request created an
account with role "owner", and a client manager promoted themselves to
owner by patching their own record. Either route yields read and write
access to every client's board.

Create, update and delete are now superuser-only; list and view are
owner-or-self. Adds four integration tests so a regression fails loudly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

pocketbase/pb_migrations/1789601000_locked_user_rules.js

@@ -0,0 +1,53 @@
+/// <reference path="../pb_data/types.d.ts" />
+/**
+ * Close the users-collection privilege escalation.
+ *
+ * REFERENCE ONLY — see pb_migrations/README.md. What actually runs is
+ * scripts/setup-remote.ts. This file exists so the schema stays readable in
+ * one place.
+ *
+ * 1789600900_applied_api_rules.js applied rules to eight collections and
+ * missed this one, so `users` kept PocketBase's defaults:
+ *
+ *   createRule ""                      -> anyone may sign up
+ *   updateRule "id = @request.auth.id"  -> anyone may edit their own record
+ *
+ * `role` is an ordinary field on this collection, so each default was a
+ * privilege escalation. Both were verified exploitable against the live
+ * instance: an anonymous POST created an account with role "owner", and a
+ * client manager promoted themselves to owner by patching their own record.
+ * A promoted account reads every client's board through the normal rules.
+ *
+ * Accounts are created and edited only by the owner, through server-side code
+ * running as a superuser, so all three write rules are superuser-only. Any
+ * future self-service profile editing must go through a route handler with an
+ * explicit field whitelist — the same reason /api/client/* exists.
+ */
+migrate(
+  (app) => {
+    const users = app.findCollectionByNameOrId("users");
+
+    const ownerOrSelf =
+      '@request.auth.id != "" && (@request.auth.role = "owner" || id = @request.auth.id)';
+
+    users.listRule = ownerOrSelf;
+    users.viewRule = ownerOrSelf;
+    users.createRule = null;
+    users.updateRule = null;
+    users.deleteRule = null;
+
+    return app.save(users);
+  },
+  (app) => {
+    // Deliberately does NOT restore PocketBase's defaults. Rolling back to
+    // public signup would reopen the escalation, and no rollback is worth
+    // that. Reset by hand in the admin UI if you truly mean to.
+    const users = app.findCollectionByNameOrId("users");
+    users.listRule = "id = @request.auth.id";
+    users.viewRule = "id = @request.auth.id";
+    users.createRule = null;
+    users.updateRule = null;
+    users.deleteRule = null;
+    return app.save(users);
+  }
+);

scripts/setup-remote.ts

@@ -93,6 +93,9 @@ const AUTHED = '@request.auth.id != ""';
 const scopedRead = (pathToClient: string) =>
   `${AUTHED} && (${OWNER} || ${pathToClient} = @request.auth.client)`;
 
+/** For the users collection: the owner sees everyone, everyone else sees only themselves. */
+const OWNER_OR_SELF = `${AUTHED} && (${OWNER} || id = @request.auth.id)`;
+
 async function applyRules(
   pb: PocketBase,
   name: string,
@@ -492,6 +495,32 @@ async function main(): Promise<void> {
     deleteRule: null,
   });
 
+  /**
+   * The users collection. Do not leave this on PocketBase's defaults.
+   *
+   * Out of the box `users` has createRule "" — anyone may sign up — and
+   * updateRule "id = @request.auth.id" — anyone may edit their own record.
+   * Because `role` is an ordinary field here, each of those is a privilege
+   * escalation on its own:
+   *
+   *   - an anonymous POST could create an account with role "owner"
+   *   - a client manager could promote themselves by patching their own record
+   *
+   * Both were live and both were verified exploitable before this was added.
+   *
+   * Accounts are created and edited only by the owner, through server-side
+   * code running as a superuser, so all three write rules are superuser-only.
+   * Any future self-service profile editing must go through a route handler
+   * with an explicit field whitelist — the same reason /api/client/* exists.
+   */
+  await applyRules(pb, "users", {
+    listRule: OWNER_OR_SELF,
+    viewRule: OWNER_OR_SELF,
+    createRule: null,
+    updateRule: null,
+    deleteRule: null,
+  });
+
   console.log("Done.");
 }
 

tests/integration/user-rules.test.ts

@@ -0,0 +1,78 @@
+import { describe, it, expect, beforeAll } from "vitest";
+import PocketBase from "pocketbase";
+
+import { DEMO, PB_URL, adminPb, requireServices, userPb } from "./helpers";
+
+/**
+ * Regression guard for a live privilege escalation.
+ *
+ * The built-in `users` collection ships with createRule "" (anyone may sign
+ * up) and updateRule "id = @request.auth.id" (anyone may edit themselves).
+ * Because `role` is an ordinary field on that collection, both were
+ * exploitable: an anonymous request could create an owner account, and a
+ * client manager could promote themselves to owner and then read every other
+ * client's board. Both were verified working against the live instance before
+ * the rules were locked.
+ *
+ * If either test here fails, someone has reverted the users rules and every
+ * client's data is readable by anyone who can reach the PocketBase URL.
+ */
+
+let admin: PocketBase;
+
+beforeAll(async () => {
+  await requireServices();
+  admin = await adminPb();
+});
+
+describe("the users collection cannot be used to escalate privileges", () => {
+  it("refuses anonymous account creation", async () => {
+    const anon = new PocketBase(PB_URL);
+    anon.autoCancellation(false);
+
+    await expect(
+      anon.collection("users").create({
+        email: `anon-${Date.now()}@example.com`,
+        password: "probe-password-123",
+        passwordConfirm: "probe-password-123",
+        name: "Should not exist",
+        role: "owner",
+      })
+    ).rejects.toThrow();
+  });
+
+  it("refuses a client manager promoting themselves to owner", async () => {
+    const pb = await userPb(DEMO.managerEmail, DEMO.managerPassword);
+    const selfId = pb.authStore.record!.id;
+
+    await expect(
+      pb.collection("users").update(selfId, { role: "owner" })
+    ).rejects.toThrow();
+
+    const after = await admin.collection("users").getOne(selfId);
+    expect(after.role).toBe("client_manager");
+  });
+
+  it("refuses a client manager reassigning themselves to another client", async () => {
+    // The other half of the same hole: switching `client` would hand them a
+    // different customer's entire board through the normal read rules.
+    const pb = await userPb(DEMO.managerEmail, DEMO.managerPassword);
+    const selfId = pb.authStore.record!.id;
+    const before = await admin.collection("users").getOne(selfId);
+
+    await expect(
+      pb.collection("users").update(selfId, { client: "" })
+    ).rejects.toThrow();
+
+    const after = await admin.collection("users").getOne(selfId);
+    expect(after.client).toBe(before.client);
+  });
+
+  it("does not let a client manager list other accounts", async () => {
+    const pb = await userPb(DEMO.managerEmail, DEMO.managerPassword);
+    const visible = await pb.collection("users").getFullList();
+
+    expect(visible).toHaveLength(1);
+    expect(visible[0].id).toBe(pb.authStore.record!.id);
+  });
+});