fix: Memory leak in cancan cache (#13194)

server/policies/cancan.ts

@@ -24,6 +24,11 @@ type Ability = {
   condition?: Condition<Constructor, Constructor>;
 };
 
+/** A model class, which may be a throwaway subclass produced by `Model.scope()`. */
+interface ModelClass extends Function {
+  scoped?: boolean;
+}
+
 /** A single evaluation, within which memoized answers may be shared. */
 interface Scope {
   cache: Map<Model, Map<Model | null, Map<string, boolean | string[]>>> | null;
@@ -138,8 +143,8 @@ export class CanCan {
 
     // The set of actions worth checking depends only on the classes involved,
     // so scanning the whole ability index is done once per class pair.
-    const performerClass = performer?.constructor ?? null;
-    const targetClass = target?.constructor ?? null;
+    const performerClass = this.unscopedClass(performer);
+    const targetClass = this.unscopedClass(target);
     let byTargetClass = this.actionsByClass.get(performerClass);
     if (!byTargetClass) {
       byTargetClass = new Map();
@@ -222,6 +227,19 @@ export class CanCan {
     Map<Function | null, Set<string>>
   > = new Map();
 
+  /**
+   * Resolves the class a model should be cached under. `Model.scope()` mints a
+   * fresh subclass on every call, so the scoped ones are walked past to reach
+   * the stable class the abilities were actually registered against.
+   */
+  private unscopedClass = (model: Model | null): ModelClass | null => {
+    let klass: ModelClass | null = model?.constructor ?? null;
+    while (klass?.scoped) {
+      klass = Object.getPrototypeOf(klass);
+    }
+    return klass;
+  };
+
   /**
    * Resolves the per-(performer, target) action cache, or null when the call
    * carries options and so cannot be shared.

server/policies/index.test.ts

@@ -1,11 +1,12 @@
 import { CollectionPermission, UserRole } from "@shared/types";
-import { Document } from "@server/models";
+import { Document, User } from "@server/models";
 import {
   buildUser,
   buildTeam,
   buildCollection,
   buildDocument,
 } from "@server/test/factories";
+import { CanCan } from "./cancan";
 import { can, serialize } from "./index";
 
 describe("serialize", () => {
@@ -77,4 +78,27 @@ describe("memoization", () => {
     user.role = UserRole.Viewer;
     expect(can(user, "update", document)).toEqual(false);
   });
+
+  it("should cache actions once per model, not once per scope", async () => {
+    const cancan = new CanCan();
+    cancan.allow(User, "read", Document, () => true);
+    const user = await buildUser();
+
+    // Reaching into the cache directly, as its size is the whole point here.
+    const cache: Map<unknown, Map<unknown, unknown>> = Reflect.get(
+      cancan,
+      "actionsByClass"
+    );
+
+    // Endpoints reach documents through Document.findByPk and
+    // Document.withMembershipScope, and every scope() mints a fresh subclass.
+    for (let i = 0; i < 10; i++) {
+      const scoped = Document.scope([
+        { method: ["withMembership", user.id, false] },
+      ]);
+      cancan.serialize(user, scoped.build());
+    }
+
+    expect(cache.get(User)?.size).toEqual(1);
+  });
 });