fix(permissions): report whether a revoke removed anything and persist the linked grant row before the flat view

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

@@ -943,6 +943,116 @@ describe('PermissionService (integration)', () => {
             );
         });
 
+        it('grantUserUserPermission persists the linked SQL row before resolving', async () => {
+            const { user: issuer, actor: issuerActor } = await makeUserActor();
+            const { user: target } = await makeUserActor();
+            const permission = `zztest:grant-sync-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            // A fire-and-forget upsert leaves the flat view claiming a grant
+            // that nothing durable backs — losing it if the entry is ever
+            // dropped, with no SQL row to re-derive from.
+            const rows = await server.stores.permission.readLinkedUserUserPerms(
+                target.id,
+                [permission],
+            );
+            expect(rows).toHaveLength(1);
+            expect(rows[0].issuer_user_id).toBe(issuer.id);
+        });
+
+        it('grantUserUserPermission surfaces a failed SQL upsert instead of swallowing it', async () => {
+            const { user: issuer, actor: issuerActor } = await makeUserActor();
+            const { user: target, actor: targetActor } = await makeUserActor();
+            const permission = `zztest:grant-fail-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+
+            const spy = vi
+                .spyOn(server.stores.permission, 'upsertUserUserPerm')
+                .mockRejectedValue(new Error('simulated db failure'));
+            try {
+                await expect(
+                    runWithContext({ actor: issuerActor }, () =>
+                        permService.grantUserUserPermission(
+                            issuerActor,
+                            target.username,
+                            permission,
+                        ),
+                    ),
+                ).rejects.toThrow('simulated db failure');
+            } finally {
+                spy.mockRestore();
+            }
+
+            // Fails closed: the durable write went first, so a failure there
+            // leaves no flat entry granting access either.
+            expect(await permService.check(targetActor, permission)).toBeFalsy();
+        });
+
+        it('reports whether a grant was actually removed', async () => {
+            const { user: issuer, actor: issuerActor } = await makeUserActor();
+            const { user: target } = await makeUserActor();
+            const permission = `zztest:rvk-reports-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            const first = await runWithContext({ actor: issuerActor }, () =>
+                permService.revokeUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+            expect(first).toBe(true);
+
+            // Nothing left to revoke — still not an error, but it must not
+            // claim to have removed something.
+            const second = await runWithContext({ actor: issuerActor }, () =>
+                permService.revokeUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+            expect(second).toBe(false);
+        });
+
+        it('writes no audit row for a revoke that matched nothing', async () => {
+            const { user: issuer, actor: issuerActor } = await makeUserActor();
+            const { user: target } = await makeUserActor();
+            const permission = `zztest:rvk-noaudit-${uuidv4()}:ii:read`;
+            await grantManage(issuer, permission);
+
+            // Never granted, so there is no row to remove.
+            const revoked = await runWithContext({ actor: issuerActor }, () =>
+                permService.revokeUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+            expect(revoked).toBe(false);
+
+            const rows = await server.clients.db.read(
+                'SELECT `action` FROM `audit_user_to_user_permissions` WHERE `holder_user_id` = ? AND `permission` = ?',
+                [target.id, permission],
+            );
+            expect(rows).toHaveLength(0);
+        });
+
         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();

src/backend/services/permission/PermissionService.ts

@@ -845,7 +845,15 @@ export class PermissionService extends PuterService {
             });
         const issuerId = actor.user.id;
 
-        // Flat upsert (awaited so callers see immediate effect)
+        // Durable row before the flat view, both awaited. A `manage:`-only
+        // delegate's grant resolves via flat and not via the linked chain, so
+        // writing SQL first makes a partial failure fail closed.
+        await this.stores.permission.upsertUserUserPerm(
+            user.id,
+            issuerId,
+            permission,
+            extra,
+        );
         await this.stores.permission.setFlatUserPerm(user.id, permission, {
             ...extra,
             issuer_user_id: issuerId,
@@ -853,10 +861,7 @@ export class PermissionService extends PuterService {
             deleted: false,
         });
 
-        // Linked upsert + audit fire-and-forget.
-        this.stores.permission
-            .upsertUserUserPerm(user.id, issuerId, permission, extra)
-            .catch(() => {});
+        // Off the critical path, but a silent drop makes the log untrustworthy.
         this.stores.permission
             .auditUserUserPerm({
                 holder_user_id: user.id,
@@ -865,18 +870,28 @@ export class PermissionService extends PuterService {
                 action: 'grant',
                 reason: meta.reason ?? 'granted via PermissionService',
             })
-            .catch(() => {});
+            .catch((err) => {
+                console.warn(
+                    '[PermissionService] failed to audit user-user grant:',
+                    err,
+                );
+            });
 
         // Bust any cached "denied" reading so the grant is live immediately.
         if (user.uuid) await this.#bumpUserCacheGeneration(user.uuid);
     }
 
+    /**
+     * Returns whether a grant was actually removed. Matching nothing isn't an
+     * error (an owner has no grant row to delete), but callers must be able to
+     * tell rather than reporting a removal that didn't happen.
+     */
     async revokeUserUserPermission(
         actor: Actor,
         username: string,
         permission: string,
         meta: GrantMeta = {},
-    ): Promise<void> {
+    ): Promise<boolean> {
         permission = await this.rewritePermission(permission);
         const user = await this.stores.user.getByUsername(username);
         if (!user)
@@ -903,22 +918,33 @@ export class PermissionService extends PuterService {
         // 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(
+        const revoked = await this.stores.permission.deleteUserUserPermByHolder(
             user.id,
             permission,
         );
-        this.stores.permission
-            .auditUserUserPerm({
-                holder_user_id: user.id,
-                issuer_user_id: issuerId,
-                permission,
-                action: 'revoke',
-                reason: meta.reason ?? 'revoked via PermissionService',
-            })
-            .catch(() => {});
 
-        // The holder loses access on their next check, not after the TTL.
+        // Only record a revoke that happened.
+        if (revoked) {
+            this.stores.permission
+                .auditUserUserPerm({
+                    holder_user_id: user.id,
+                    issuer_user_id: issuerId,
+                    permission,
+                    action: 'revoke',
+                    reason: meta.reason ?? 'revoked via PermissionService',
+                })
+                .catch((err) => {
+                    console.warn(
+                        '[PermissionService] failed to audit user-user revoke:',
+                        err,
+                    );
+                });
+        }
+
+        // Unconditional: the flat delete above can't report what it removed, so
+        // skipping the bump on a no-op risks leaving a cached allow standing.
         if (user.uuid) await this.#bumpUserCacheGeneration(user.uuid);
+        return revoked;
     }
 
     /**

src/backend/stores/permission/PermissionStore.test.ts

@@ -651,6 +651,129 @@ describe('PermissionStore', () => {
             expect(await store.getScanCache(nextKey)).toBeNull();
         });
 
+        it('announces a bump so peer regions can bump their own counter', async () => {
+            const actorUid = `actor-${uuidv4()}`;
+            const seen: unknown[] = [];
+            server.clients.event.on(
+                'outer.permission.generationBumped',
+                (_key, data) => {
+                    seen.push(data);
+                },
+            );
+
+            await store.bumpCacheGeneration(actorUid);
+            expect(seen).toContainEqual({ actorUid });
+        });
+
+        it('applies a remote bump without re-announcing it', async () => {
+            const actorUid = `actor-${uuidv4()}`;
+            const before = await store.getCacheGeneration(actorUid);
+            let announced = 0;
+            server.clients.event.on(
+                'outer.permission.generationBumped',
+                (_key, _data, meta) => {
+                    if (!(meta as { from_outside?: boolean })?.from_outside) {
+                        announced++;
+                    }
+                },
+            );
+
+            // What BroadcastService does with an inbound webhook event.
+            await server.clients.event.emitAndWait(
+                'outer.permission.generationBumped',
+                { actorUid },
+                { from_outside: true },
+            );
+
+            expect(await store.getCacheGeneration(actorUid)).toBeGreaterThan(
+                before,
+            );
+            // Re-announcing would ping-pong between regions forever.
+            expect(announced).toBe(0);
+        });
+
+        it('ignores a locally-emitted bump event', async () => {
+            const actorUid = `actor-${uuidv4()}`;
+            await server.clients.event.emitAndWait(
+                'outer.permission.generationBumped',
+                { actorUid },
+                {},
+            );
+            expect(await store.getCacheGeneration(actorUid)).toBe(0);
+        });
+
+        it('announces a flat delete so peer regions drop their own copy', async () => {
+            const holder = await makeUser();
+            await store.setFlatUserPerm(holder.id, 'fs:u:read', {
+                permission: 'fs:u:read',
+                deleted: false,
+            } as never);
+
+            const seen: unknown[] = [];
+            server.clients.event.on(
+                'outer.permission.flatInvalidated',
+                (_key, data) => {
+                    seen.push(data);
+                },
+            );
+
+            await store.delFlatUserPerm(holder.id, 'fs:u:read');
+            expect(seen).toContainEqual({
+                holderUserId: holder.id,
+                permission: 'fs:u:read',
+            });
+        });
+
+        it('applies a remote flat delete without re-announcing it', async () => {
+            const holder = await makeUser();
+            await store.setFlatUserPerm(holder.id, 'fs:u:read', {
+                permission: 'fs:u:read',
+                deleted: false,
+            } as never);
+            expect(
+                await store.getFlatUserPerms(holder.id, ['fs:u:read']),
+            ).toHaveLength(1);
+
+            let announced = 0;
+            server.clients.event.on(
+                'outer.permission.flatInvalidated',
+                (_key, _data, meta) => {
+                    if (!(meta as { from_outside?: boolean })?.from_outside) {
+                        announced++;
+                    }
+                },
+            );
+
+            await server.clients.event.emitAndWait(
+                'outer.permission.flatInvalidated',
+                { holderUserId: holder.id, permission: 'fs:u:read' },
+                { from_outside: true },
+            );
+
+            expect(
+                await store.getFlatUserPerms(holder.id, ['fs:u:read']),
+            ).toEqual([]);
+            expect(announced).toBe(0);
+        });
+
+        it('ignores a locally-emitted flat invalidation', async () => {
+            const holder = await makeUser();
+            await store.setFlatUserPerm(holder.id, 'fs:u:read', {
+                permission: 'fs:u:read',
+                deleted: false,
+            } as never);
+
+            await server.clients.event.emitAndWait(
+                'outer.permission.flatInvalidated',
+                { holderUserId: holder.id, permission: 'fs:u:read' },
+                {},
+            );
+
+            expect(
+                await store.getFlatUserPerms(holder.id, ['fs:u:read']),
+            ).toHaveLength(1);
+        });
+
         it('drops a scan cache entry on explicit invalidation', async () => {
             const key = store.buildScanCacheKey(`actor-${uuidv4()}`, ['p'], 0);
             await store.setScanCache(key, { allowed: false });
@@ -737,5 +860,95 @@ describe('PermissionStore', () => {
                 [],
             );
         });
+
+        it('deletes every grant at or beneath a permission prefix', async () => {
+            const issuer = await makeUser();
+            const holder = await makeUser();
+            const other = await makeUser();
+            const uid = uuidv4();
+
+            for (const [h, perm] of [
+                [holder, `fs:${uid}:read`],
+                [other, `fs:${uid}:write`],
+                [holder, `fs:${uid}`],
+            ] as const) {
+                await store.upsertUserUserPerm(h.id, issuer.id, perm, {});
+                await store.setFlatUserPerm(h.id, perm, {
+                    permission: perm,
+                    deleted: false,
+                } as never);
+            }
+            // A different entry must survive.
+            const keeper = `fs:${uuidv4()}:read`;
+            await store.upsertUserUserPerm(holder.id, issuer.id, keeper, {});
+
+            const removed = await store.deleteUserUserPermsByPermissionPrefix(
+                `fs:${uid}`,
+            );
+
+            expect(removed).toHaveLength(3);
+            expect(
+                await store.readLinkedUserUserPerms(holder.id, [
+                    `fs:${uid}:read`,
+                    `fs:${uid}`,
+                ]),
+            ).toEqual([]);
+            expect(
+                await store.getFlatUserPerms(holder.id, [`fs:${uid}:read`]),
+            ).toEqual([]);
+            expect(
+                await store.readLinkedUserUserPerms(holder.id, [keeper]),
+            ).toHaveLength(1);
+        });
+
+        it('does not let a wildcard in the prefix widen the match', async () => {
+            const issuer = await makeUser();
+            const holder = await makeUser();
+            const victim = `fs:${uuidv4()}:read`;
+            await store.upsertUserUserPerm(holder.id, issuer.id, victim, {});
+
+            // `_` and `%` are LIKE wildcards; unescaped they would match this.
+            expect(
+                await store.deleteUserUserPermsByPermissionPrefix('fs:%'),
+            ).toEqual([]);
+            expect(
+                await store.deleteUserUserPermsByPermissionPrefix('fs:_'),
+            ).toEqual([]);
+            expect(
+                await store.readLinkedUserUserPerms(holder.id, [victim]),
+            ).toHaveLength(1);
+        });
+
+        it('returns an empty list when the prefix matches nothing', async () => {
+            expect(
+                await store.deleteUserUserPermsByPermissionPrefix(
+                    `fs:${uuidv4()}`,
+                ),
+            ).toEqual([]);
+        });
+
+        it('reports whether the delete matched a row', async () => {
+            const issuer = await makeUser();
+            const holder = await makeUser();
+            await store.upsertUserUserPerm(
+                holder.id,
+                issuer.id,
+                'fs:u:read',
+                {},
+            );
+
+            expect(
+                await store.deleteUserUserPermByHolder(holder.id, 'fs:u:read'),
+            ).toBe(true);
+            expect(
+                await store.deleteUserUserPermByHolder(holder.id, 'fs:u:read'),
+            ).toBe(false);
+            expect(
+                await store.deleteUserUserPermByHolder(
+                    holder.id,
+                    'fs:never-granted:read',
+                ),
+            ).toBe(false);
+        });
     });
 });

src/backend/stores/permission/PermissionStore.ts

@@ -98,6 +98,48 @@ export interface AuditEntry {
 export class PermissionStore extends PuterStore {
     declare protected stores: LayerInstances<typeof puterStores>;
 
+    override onServerStart(): void {
+        this.#subscribeRemoteGenerationBumps();
+        this.#subscribeRemoteFlatInvalidations();
+    }
+
+    /**
+     * Apply a peer region's flat-permission delete. Independent of whether the
+     * KV table replicates: a redundant delete is a no-op, a needed one is the
+     * only thing that makes the revoke real there.
+     */
+    #subscribeRemoteFlatInvalidations(): void {
+        this.clients.event.on(
+            'outer.permission.flatInvalidated',
+            (_key, data, meta) => {
+                if (!(meta as { from_outside?: boolean })?.from_outside) return;
+                const { holderUserId, permission } = (data ?? {}) as {
+                    holderUserId?: unknown;
+                    permission?: unknown;
+                };
+                if (typeof holderUserId !== 'number') return;
+                if (typeof permission !== 'string' || permission === '') return;
+                void this.#applyFlatUserPermDelete(holderUserId, permission);
+            },
+        );
+    }
+
+    /**
+     * Apply a peer region's cache-generation bump. Our own emit reaches local
+     * listeners too, and that half already ran before it went out.
+     */
+    #subscribeRemoteGenerationBumps(): void {
+        this.clients.event.on(
+            'outer.permission.generationBumped',
+            (_key, data, meta) => {
+                if (!(meta as { from_outside?: boolean })?.from_outside) return;
+                const actorUid = (data as { actorUid?: unknown })?.actorUid;
+                if (typeof actorUid !== 'string' || actorUid === '') return;
+                void this.#applyCacheGenerationBump(actorUid);
+            },
+        );
+    }
+
     // -- Flat view (KV under system namespace) ------------------------
 
     /**
@@ -154,6 +196,23 @@ export class PermissionStore extends PuterStore {
     async delFlatUserPerm(
         holderUserId: number,
         permission: string,
+    ): Promise<void> {
+        await this.#applyFlatUserPermDelete(holderUserId, permission);
+        try {
+            this.clients.event.emit(
+                'outer.permission.flatInvalidated',
+                { holderUserId, permission },
+                {},
+            );
+        } catch {
+            // Peer regions keep the entry until their KV replicates the delete.
+        }
+    }
+
+    /** Local half of a flat delete. Never emits, so a remote one can't loop. */
+    async #applyFlatUserPermDelete(
+        holderUserId: number,
+        permission: string,
     ): Promise<void> {
         const key = PermissionUtil.join(
             PERM_KEY_PREFIX,
@@ -201,17 +260,73 @@ export class PermissionStore extends PuterStore {
         });
     }
 
+    /** Returns whether a row was actually deleted. */
     async deleteUserUserPermByHolder(
         holderUserId: number,
         permission: string,
-    ): Promise<void> {
-        await this.clients.db.write(
+    ): Promise<boolean> {
+        const result = await this.clients.db.write(
             'DELETE FROM `user_to_user_permissions` WHERE `holder_user_id` = ? AND `permission` = ?',
             [holderUserId, permission],
         );
+        if (!result.anyRowsAffected) return false;
         await this.publishCacheKeys({
             keys: [this.#u2uCacheKey(holderUserId)],
         });
+        return true;
+    }
+
+    /**
+     * Delete every user-to-user grant at or beneath `permission`, clearing the
+     * flat KV view too, and return the rows removed so the caller can audit
+     * them and bust caches.
+     *
+     * The subject lives in the permission text rather than a column, so no
+     * foreign key can cascade it — this is how a deleted fsentry's grants get
+     * withdrawn. `permission` has no index, so this is a table scan: fine on
+     * deletion, never on a hot path.
+     */
+    async deleteUserUserPermsByPermissionPrefix(permission: string): Promise<
+        Array<{
+            holder_user_id: number;
+            issuer_user_id: number;
+            permission: string;
+        }>
+    > {
+        // See deleteAppGrantsByPermissionPrefix for why `!` is the escape.
+        const escaped = permission.replace(/([!%_])/g, '!$1');
+        const prefix = `${escaped}:%`;
+
+        const rows = (await this.clients.db.read(
+            'SELECT `holder_user_id`, `issuer_user_id`, `permission` FROM `user_to_user_permissions` ' +
+                "WHERE `permission` = ? OR `permission` LIKE ? ESCAPE '!'",
+            [permission, prefix],
+        )) as Array<{
+            holder_user_id: number;
+            issuer_user_id: number;
+            permission: string;
+        }>;
+        if (rows.length === 0) return [];
+
+        await this.clients.db.write(
+            'DELETE FROM `user_to_user_permissions` ' +
+                "WHERE `permission` = ? OR `permission` LIKE ? ESCAPE '!'",
+            [permission, prefix],
+        );
+
+        // Via delFlatUserPerm so peer regions hear about it too.
+        await Promise.all(
+            rows.map((row) =>
+                this.delFlatUserPerm(row.holder_user_id, row.permission),
+            ),
+        );
+
+        const keys = [
+            ...new Set(rows.map((r) => this.#u2uCacheKey(r.holder_user_id))),
+        ];
+        if (keys.length > 0) await this.publishCacheKeys({ keys });
+
+        return rows;
     }
 
     async auditUserUserPerm(
@@ -689,6 +804,20 @@ export class PermissionStore extends PuterStore {
     }
 
     async bumpCacheGeneration(actorUid: string): Promise<void> {
+        await this.#applyCacheGenerationBump(actorUid);
+        try {
+            this.clients.event.emit(
+                'outer.permission.generationBumped',
+                { actorUid },
+                {},
+            );
+        } catch {
+            // Peer regions fall back to their scan-cache TTL.
+        }
+    }
+
+    /** Local half of a bump. Never emits, so a remote bump can't ping-pong. */
+    async #applyCacheGenerationBump(actorUid: string): Promise<void> {
         const key = this.#cacheGenerationKey(actorUid);
         try {
             const next = await this.clients.redis.incr(key);