perf: some perf gains from fable (#3249)

src/backend/services/permission/PermissionService.test.ts

@@ -17,7 +17,7 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
 import { v4 as uuidv4 } from 'uuid';
 import type { Actor } from '../../core/actor.js';
 import { runWithContext } from '../../core/context.js';
@@ -729,4 +729,276 @@ describe('PermissionService (integration)', () => {
             );
         });
     });
+
+    describe('derived-actor cache invalidation (app-under-user)', () => {
+        // An app-under-user actor's reading embeds its user's reading, and
+        // its cache keys fold in the user's generation counter — so a
+        // user-level grant/revoke must take effect for the user's app
+        // actors on their very next check, not after the scan-cache TTL.
+        const grantManage = async (
+            issuer: { id: number },
+            permission: string,
+        ) => {
+            await server.stores.permission.setFlatUserPerm(
+                issuer.id,
+                `manage:${permission}`,
+                {
+                    permission: `manage:${permission}`,
+                    deleted: false,
+                    issuer_user_id: issuer.id,
+                } as never,
+            );
+        };
+
+        const makeApp = async (ownerUserId: number) =>
+            (server.stores.app.create as unknown as (
+                fields: Record<string, unknown>,
+                opts: { ownerUserId: number },
+            ) => Promise<{ uid: string; id: number }>)(
+                {
+                    name: `dac-${uuidv4()}`,
+                    title: 'Derived-actor cache app',
+                    index_url: `https://dac-${uuidv4()}.test/`,
+                },
+                { ownerUserId },
+            );
+
+        it('a user-level revoke is visible immediately to the user\'s app actors', async () => {
+            const { user: issuer, actor: issuerActor } =
+                await makeUserActor();
+            const { user: target, actor: targetActor } =
+                await makeUserActor();
+            const app = await makeApp(target.id);
+            const permission = `service:app-revoke-now-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+            // The user lets the app act with this permission, so the app
+            // actor resolves it through the user's own reading.
+            await runWithContext({ actor: targetActor }, () =>
+                permService.grantUserAppPermission(
+                    targetActor,
+                    app.uid,
+                    permission,
+                ),
+            );
+
+            const appActor = {
+                user: targetActor.user,
+                app: { id: app.id, uid: app.uid },
+            } as unknown as Actor;
+
+            // Prime the app actor's cache with a "granted" reading.
+            expect(await permService.check(appActor, permission)).toBe(true);
+
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.revokeUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            // Without the user generation folded into the app actor's
+            // cache keys this would still read `true` for up to the TTL.
+            expect(await permService.check(appActor, permission)).toBe(false);
+        });
+
+        it('a user-level grant busts an app actor\'s cached denial immediately', async () => {
+            const { user: issuer, actor: issuerActor } =
+                await makeUserActor();
+            const { user: target, actor: targetActor } =
+                await makeUserActor();
+            const app = await makeApp(target.id);
+            const permission = `service:app-grant-now-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+
+            // App is allowed to act with the permission, but the user does
+            // not hold it yet — primes a "denied" reading for the app actor.
+            await runWithContext({ actor: targetActor }, () =>
+                permService.grantUserAppPermission(
+                    targetActor,
+                    app.uid,
+                    permission,
+                ),
+            );
+            const appActor = {
+                user: targetActor.user,
+                app: { id: app.id, uid: app.uid },
+            } as unknown as Actor;
+            expect(await permService.check(appActor, permission)).toBe(false);
+
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            expect(await permService.check(appActor, permission)).toBe(true);
+        });
+    });
+
+    describe('revoke durability (flat/linked consistency)', () => {
+        const grantManage = async (
+            issuer: { id: number },
+            permission: string,
+        ) => {
+            await server.stores.permission.setFlatUserPerm(
+                issuer.id,
+                `manage:${permission}`,
+                {
+                    permission: `manage:${permission}`,
+                    deleted: false,
+                    issuer_user_id: issuer.id,
+                } as never,
+            );
+        };
+
+        it('revokeUserUserPermission deletes the linked SQL row before resolving', async () => {
+            const { user: issuer, actor: issuerActor } =
+                await makeUserActor();
+            const { user: target } = await makeUserActor();
+            const permission = `service:rvk-sync-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.revokeUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            // The linked row must be gone the moment the revoke resolves —
+            // a fire-and-forget delete could lose the race against the
+            // post-bump rescan, which would re-warm the flat view from the
+            // surviving SQL row and resurrect the grant.
+            const rows = await server.stores.permission.readLinkedUserUserPerms(
+                target.id,
+                [permission],
+            );
+            expect(rows).toHaveLength(0);
+        });
+
+        it('revokeUserUserPermission surfaces a failed SQL delete instead of swallowing it', async () => {
+            const { user: issuer, actor: issuerActor } =
+                await makeUserActor();
+            const { user: target } = await makeUserActor();
+            const permission = `service:rvk-fail-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            const spy = vi
+                .spyOn(server.stores.permission, 'deleteUserUserPermByHolder')
+                .mockRejectedValue(new Error('simulated db failure'));
+            try {
+                await expect(
+                    runWithContext({ actor: issuerActor }, () =>
+                        permService.revokeUserUserPermission(
+                            issuerActor,
+                            target.username,
+                            permission,
+                        ),
+                    ),
+                ).rejects.toThrow('simulated db failure');
+            } finally {
+                spy.mockRestore();
+            }
+
+            // Retry once the store works again — the revoke completes.
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.revokeUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+        });
+
+        it('scan-path warms of the flat view carry an expiry (grants are permanent)', async () => {
+            const { user: issuer, actor: issuerActor } =
+                await makeUserActor();
+            const { user: target, actor: targetActor } =
+                await makeUserActor();
+            const permission = `service:warm-ttl-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+            // The linked (SQL) path is a delegation chain: it only grants
+            // if the issuer holds the permission themselves. Give the
+            // issuer a terminal flat grant so the fallback below resolves.
+            await server.stores.permission.setFlatUserPerm(
+                issuer.id,
+                permission,
+                {
+                    permission,
+                    deleted: false,
+                    issuer_user_id: issuer.id,
+                } as never,
+            );
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+            // The grant's linked-row upsert is fire-and-forget — wait for
+            // it so the linked fallback below has something to find.
+            await vi.waitFor(async () => {
+                const rows =
+                    await server.stores.permission.readLinkedUserUserPerms(
+                        target.id,
+                        [permission],
+                    );
+                expect(rows.length).toBeGreaterThan(0);
+            });
+            // Drop the flat entry so the next check takes the linked SQL
+            // fallback and re-warms the flat view.
+            await server.stores.permission.delFlatUserPerm(
+                target.id,
+                permission,
+            );
+
+            const spy = vi.spyOn(server.stores.permission, 'setFlatUserPerm');
+            try {
+                expect(await permService.check(targetActor, permission)).toBe(
+                    true,
+                );
+                // The warm is fire-and-forget; wait for it to land.
+                await vi.waitFor(() => {
+                    const warmCall = spy.mock.calls.find(
+                        (c) => c[1] === permission,
+                    );
+                    expect(warmCall).toBeDefined();
+                    // Derived warms must self-expire so one that races a
+                    // concurrent revoke cannot persist indefinitely.
+                    expect(warmCall![3]?.expireAt).toBeGreaterThan(
+                        Math.floor(Date.now() / 1000),
+                    );
+                });
+            } finally {
+                spy.mockRestore();
+            }
+        });
+    });
 });

src/backend/services/permission/PermissionService.ts

@@ -23,6 +23,7 @@ import { Context } from '../../core/context';
 import { HttpError } from '../../core/http/HttpError.js';
 import { PuterService } from '../types';
 import {
+    FLAT_PERM_WARM_TTL_SECONDS,
     MANAGE_PERM_PREFIX,
     PERMISSION_SCAN_CACHE_TTL_SECONDS,
 } from './consts';
@@ -215,8 +216,7 @@ export class PermissionService extends PuterService {
 
         // -- Cache pass: one pipelined MGET --
         const aUid = actorUid(actor);
-        const generation =
-            await this.stores.permission.getCacheGeneration(aUid);
+        const generation = await this.#cacheGenerationTag(actor);
         const cached = await this.stores.permission.getMultiCheckCache(
             aUid,
             dedup,
@@ -296,9 +296,10 @@ export class PermissionService extends PuterService {
         // -- Redis scan cache --
         // The per-actor cache generation is folded into the key so a
         // grant/revoke bump orphans this actor's cached readings at once.
+        // For derived actors the tag combines every relevant counter, so a
+        // user-level bump also orphans that user's app/token actors.
         const aUid = actorUid(actor);
-        const generation =
-            await this.stores.permission.getCacheGeneration(aUid);
+        const generation = await this.#cacheGenerationTag(actor);
         const cacheKey = this.stores.permission.buildScanCacheKey(
             aUid,
             options,
@@ -763,24 +764,43 @@ export class PermissionService extends PuterService {
         const linkedReading = await linkedPromise;
         const flatOptions = PermissionUtil.readingToOptions(linkedReading);
 
-        // Warm flat KV cache for future hits (fire-and-forget, don't block result)
+        // Warm flat KV cache for future hits (fire-and-forget, don't block
+        // result). Warms expire: they are derived from the SQL traversal
+        // above, and a warm whose KV write lands after a concurrent
+        // revoke's flat delete would otherwise re-materialize the revoked
+        // grant permanently. The expiry bounds that to the warm TTL — the
+        // next scan re-derives from SQL, which the revoke deletes
+        // synchronously. (Grant-path flat writes are authoritative and
+        // carry no expiry.)
+        const warmExpireAt =
+            Math.floor(Date.now() / 1000) + FLAT_PERM_WARM_TTL_SECONDS;
         for (const opt of flatOptions) {
             if (!opt.permission) continue;
             const data = Array.isArray(opt.data) ? opt.data : [opt.data];
             const issuerUserId = (data[0] as { issuer_user_id?: number })
                 ?.issuer_user_id;
             this.stores.permission
-                .setFlatUserPerm(actor.user.id, opt.permission, {
-                    permission: opt.permission,
-                    issuer_user_id: issuerUserId,
-                    data,
-                })
+                .setFlatUserPerm(
+                    actor.user.id,
+                    opt.permission,
+                    {
+                        permission: opt.permission,
+                        issuer_user_id: issuerUserId,
+                        data,
+                    },
+                    { expireAt: warmExpireAt },
+                )
                 .catch(() => {
                     /* swallow — this is a cache warm */
                 });
         }
 
-        return flatReading;
+        // Return the traversal result itself — not the (empty) flat
+        // reading — so the fallback grants on the check that took it
+        // rather than only after the warm lands. Returning the empty flat
+        // reading here would cache a false "denied" for the TTL every
+        // time a warm expires.
+        return linkedReading;
     }
 
     async #flatValidateUserPerms(
@@ -951,9 +971,17 @@ export class PermissionService extends PuterService {
         const issuerId = actor.user.id;
 
         await this.stores.permission.delFlatUserPerm(user.id, permission);
-        this.stores.permission
-            .deleteUserUserPermByHolder(user.id, permission)
-            .catch(() => {});
+        // Awaited (unlike the grant-path upsert): the generation bump below
+        // guarantees the holder's very next check re-derives from SQL, so a
+        // fire-and-forget delete here could lose the race and let that scan
+        // resurrect the revoked grant into the flat view. If this fails the
+        // caller gets the error — the permission is then still effectively
+        // granted (flat falls back to the surviving SQL row), which is the
+        // consistent, retryable outcome.
+        await this.stores.permission.deleteUserUserPermByHolder(
+            user.id,
+            permission,
+        );
         this.stores.permission
             .auditUserUserPerm({
                 holder_user_id: user.id,
@@ -1375,6 +1403,53 @@ export class PermissionService extends PuterService {
     // cached scan/check readings at once. This is cluster-safe (a single
     // INCR, no pattern scan) and makes a grant/revoke take effect on the
     // holder's very next check rather than after the cache TTL.
+    //
+    // Derived actors fold their user's counter into their cache keys (see
+    // #cacheGenerationKeys), so a `user:<uuid>` bump also takes immediate
+    // effect for that user's app-under-user and access-token actors.
+    // Readings that embed a *different* user's reading (group/dev-app/
+    // user-user issuer chains) are not generation-linked to that issuer;
+    // those remain bounded by the scan-cache TTL.
+
+    /**
+     * Generation keys whose counters this actor's cached readings depend
+     * on. A derived actor (app-under-user, access-token) acts through its
+     * user — its readings embed that user's reading via the recursive
+     * issuer scan — so the user's counter is folded into its cache keys.
+     * A plain user actor depends only on its own counter.
+     */
+    #cacheGenerationKeys(actor: Actor): string[] {
+        const keys = [actorUid(actor)];
+        if (actor.accessToken) {
+            keys.push(...this.#cacheGenerationKeys(actor.accessToken.issuer));
+            if (actor.accessToken.authorized) {
+                keys.push(
+                    ...this.#cacheGenerationKeys(actor.accessToken.authorized),
+                );
+            }
+        } else if (actor.app && actor.user?.uuid) {
+            keys.push(`user:${actor.user.uuid}`);
+        }
+        return Array.from(new Set(keys));
+    }
+
+    /**
+     * Cache-generation tag for an actor: the single counter value for a
+     * plain user actor (key format unchanged: `g<n>`), or the dependent
+     * counters joined with '.' for derived actors (e.g. `g<own>.<user>`).
+     * Joined rather than summed so distinct counter states can never
+     * collide on the same tag.
+     */
+    async #cacheGenerationTag(actor: Actor): Promise<number | string> {
+        const keys = this.#cacheGenerationKeys(actor);
+        if (keys.length === 1) {
+            return this.stores.permission.getCacheGeneration(keys[0]);
+        }
+        const gens = await Promise.all(
+            keys.map((k) => this.stores.permission.getCacheGeneration(k)),
+        );
+        return gens.join('.');
+    }
 
     /** Bump a plain user holder (`user:<uuid>`). */
     async #bumpUserCacheGeneration(userUuid: string): Promise<void> {

src/backend/services/permission/consts.ts

@@ -41,6 +41,18 @@ export const PERMISSION_SCAN_CACHE_TTL_SECONDS = 20;
  */
 export const PERMISSION_CACHE_GENERATION_TTL_SECONDS = 24 * 60 * 60;
 
+/**
+ * TTL (seconds) for flat user-permission entries written by the scan-path
+ * cache warm (`validateUserPerms`), as opposed to entries written by an
+ * explicit grant, which are authoritative and permanent. A warm is derived
+ * from a SQL traversal, so a warm that races a concurrent revoke (its KV
+ * write landing after the revoke's flat delete) can re-materialize a
+ * just-revoked grant. The expiry bounds that failure to this window —
+ * after it lapses the next scan re-derives from SQL, which the revoke
+ * deletes synchronously — instead of letting it persist indefinitely.
+ */
+export const FLAT_PERM_WARM_TTL_SECONDS = 60;
+
 /**
  * TTL (seconds) for the per-node in-process cache of the generation
  * counter. Permission checks are very hot, so reading the counter from

src/backend/stores/permission/PermissionStore.ts

@@ -127,18 +127,26 @@ export class PermissionStore extends PuterStore {
         );
     }
 
-    /** Write a single flat user-to-user permission entry to KV. */
+    /**
+     * Write a single flat user-to-user permission entry to KV.
+     *
+     * `opts.expireAt` (epoch seconds) marks the entry as a derived cache
+     * warm rather than an authoritative grant: warms self-expire so a warm
+     * that raced a concurrent revoke cannot re-materialize the grant
+     * indefinitely. Grant-path writes omit it and are permanent.
+     */
     async setFlatUserPerm(
         holderUserId: number,
         permission: string,
         value: FlatPermValue,
+        opts: { expireAt?: number } = {},
     ): Promise<void> {
         const key = PermissionUtil.join(
             PERM_KEY_PREFIX,
             String(holderUserId),
             permission,
         );
-        await this.stores.kv.set({ key, value });
+        await this.stores.kv.set({ key, value, expireAt: opts.expireAt });
     }
 
     /** Delete a single flat user-to-user permission entry from KV. */
@@ -563,6 +571,11 @@ export class PermissionStore extends PuterStore {
     // no CROSSSLOT pattern scan) instantly orphans every cached reading for
     // that actor: subsequent lookups compute a new key and miss.
     //
+    // Derived actors (app-under-user, access-token) fold the generations of
+    // every actor they act through into their cache keys — see
+    // PermissionService's cacheGenerationTag — so a bump of `user:<uuid>`
+    // also orphans that user's app and token actors' readings.
+    //
     // The authoritative counter lives in shared Redis. Permission checks are
     // extremely hot, so each node keeps a tiny in-process cache (the kv.js
     // singleton, ~2s TTL) in front of the Redis read — this collapses the
@@ -632,7 +645,7 @@ export class PermissionStore extends PuterStore {
     buildScanCacheKey(
         actorUid: string,
         permissionOptions: string[],
-        generation = 0,
+        generation: number | string = 0,
     ): string {
         return PermissionUtil.join(
             'permission-scan',
@@ -680,7 +693,7 @@ export class PermissionStore extends PuterStore {
     #checkCacheKey(
         actorUid: string,
         permission: string,
-        generation = 0,
+        generation: number | string = 0,
     ): string {
         return PermissionUtil.join(
             'permission-check',
@@ -694,7 +707,7 @@ export class PermissionStore extends PuterStore {
     async getMultiCheckCache(
         actorUid: string,
         permissions: string[],
-        generation = 0,
+        generation: number | string = 0,
     ): Promise<Map<string, boolean>> {
         const out = new Map<string, boolean>();
         if (permissions.length === 0) return out;
@@ -720,7 +733,7 @@ export class PermissionStore extends PuterStore {
     async setMultiCheckCache(
         actorUid: string,
         entries: Array<{ permission: string; granted: boolean }>,
-        generation = 0,
+        generation: number | string = 0,
         ttlSeconds: number = PERMISSION_SCAN_CACHE_TTL_SECONDS,
     ): Promise<void> {
         if (entries.length === 0) return;