fix(permissions): scope a revoke to the issuer that granted it

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

@@ -321,6 +321,44 @@ describe('PermissionService (integration)', () => {
             ).rejects.toMatchObject({ statusCode: 404 });
         });
 
+        it('lets a holder give up a permission it cannot manage', async () => {
+            const { user: issuer, actor: issuerActor } = await makeUserActor();
+            const { user: target, actor: targetActor } = await makeUserActor();
+            const permission = `zztest:self-revoke-${uuidv4()}:ii:read`;
+            await server.stores.permission.setFlatUserPerm(
+                issuer.id,
+                `manage:${permission}`,
+                {
+                    permission: `manage:${permission}`,
+                    deleted: false,
+                    issuer_user_id: issuer.id,
+                } as never,
+            );
+            await runWithContext({ actor: issuerActor }, () =>
+                permService.grantUserUserPermission(
+                    issuerActor,
+                    target.username,
+                    permission,
+                ),
+            );
+            expect(await permService.check(targetActor, permission)).toBe(true);
+
+            // The holder has no manage authority here — renouncing access is
+            // allowed anyway, since it can only narrow their own reach.
+            await runWithContext({ actor: targetActor }, () =>
+                permService.revokeUserUserPermission(
+                    targetActor,
+                    target.username,
+                    permission,
+                    {},
+                    { issuerUserId: issuer.id },
+                ),
+            );
+            expect(
+                await permService.check(targetActor, permission),
+            ).toBeFalsy();
+        });
+
         it('revokeUserUserPermission throws 403 when the issuer lacks manage', async () => {
             const { actor: issuer } = await makeUserActor();
             const { user: target } = await makeUserActor();
@@ -996,6 +1034,48 @@ describe('PermissionService (integration)', () => {
             expect(await permService.check(targetActor, permission)).toBeFalsy();
         });
 
+        it('keeps the flat entry while another issuer still grants the permission', async () => {
+            const { user: issuerA, actor: actorA } = await makeUserActor();
+            const { user: issuerB, actor: actorB } = await makeUserActor();
+            const { user: target, actor: targetActor } = await makeUserActor();
+            const permission = `zztest:two-issuers-${uuidv4()}:ii:read`;
+            await grantManage(issuerA, permission);
+            await grantManage(issuerB, permission);
+
+            for (const actor of [actorA, actorB]) {
+                await runWithContext({ actor }, () =>
+                    permService.grantUserUserPermission(
+                        actor,
+                        target.username,
+                        permission,
+                    ),
+                );
+            }
+
+            await runWithContext({ actor: actorA }, () =>
+                permService.revokeUserUserPermission(
+                    actorA,
+                    target.username,
+                    permission,
+                ),
+            );
+
+            // B's grant stands, so the shared flat key must survive with it —
+            // the key isn't issuer-scoped and B may not resolve via the chain.
+            expect(await permService.check(targetActor, permission)).toBe(true);
+
+            await runWithContext({ actor: actorB }, () =>
+                permService.revokeUserUserPermission(
+                    actorB,
+                    target.username,
+                    permission,
+                ),
+            );
+            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();

src/backend/services/permission/PermissionService.ts

@@ -882,6 +882,10 @@ export class PermissionService extends PuterService {
     }
 
     /**
+     * Remove the grant `actor` issued, or the one named by `opts.issuerUserId`
+     * when the caller has established authority over another issuer's grant (a
+     * resource owner clearing a delegate's re-grant).
+     *
      * 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.
@@ -891,6 +895,7 @@ export class PermissionService extends PuterService {
         username: string,
         permission: string,
         meta: GrantMeta = {},
+        opts: { issuerUserId?: number } = {},
     ): Promise<boolean> {
         permission = await this.rewritePermission(permission);
         const user = await this.stores.user.getByUsername(username);
@@ -899,18 +904,24 @@ export class PermissionService extends PuterService {
                 legacyCode: 'subject_does_not_exist',
             });
 
-        if (!(await this.canManagePermission(actor, permission))) {
-            throw new HttpError(403, `permission_denied: ${permission}`, {
-                legacyCode: 'permission_denied',
-            });
-        }
         if (!actor.user?.id)
             throw new HttpError(403, 'actor must be a user', {
                 legacyCode: 'forbidden',
             });
         const issuerId = actor.user.id;
 
-        await this.stores.permission.delFlatUserPerm(user.id, permission);
+        // Giving up access you hold needs no authority over the permission —
+        // it can only ever narrow what you can reach.
+        const isSelfRevoke = user.id === issuerId;
+        if (
+            !isSelfRevoke &&
+            !(await this.canManagePermission(actor, permission))
+        ) {
+            throw new HttpError(403, `permission_denied: ${permission}`, {
+                legacyCode: 'permission_denied',
+            });
+        }
+
         // 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
@@ -921,8 +932,21 @@ export class PermissionService extends PuterService {
         const revoked = await this.stores.permission.deleteUserUserPermByHolder(
             user.id,
             permission,
+            opts.issuerUserId ?? issuerId,
         );
 
+        // The flat key isn't issuer-scoped, so it may only go once no issuer
+        // grants this any more. Dropping it while another grant stands would
+        // cut access outright for a `manage:`-only issuer, whose grant the
+        // linked chain can't resolve.
+        const remaining = await this.stores.permission.readLinkedUserUserPerms(
+            user.id,
+            [permission],
+        );
+        if (remaining.length === 0) {
+            await this.stores.permission.delFlatUserPerm(user.id, permission);
+        }
+
         // Only record a revoke that happened.
         if (revoked) {
             this.stores.permission

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

@@ -848,7 +848,11 @@ describe('PermissionStore', () => {
             expect(rows).toHaveLength(1);
             expect(rows[0].issuer_user_id).toBe(issuer.id);
 
-            await store.deleteUserUserPermByHolder(holder.id, 'fs:u:read');
+            await store.deleteUserUserPermByHolder(
+                holder.id,
+                'fs:u:read',
+                issuer.id,
+            );
             expect(
                 await store.readLinkedUserUserPerms(holder.id, ['fs:u:read']),
             ).toEqual([]);
@@ -927,6 +931,40 @@ describe('PermissionStore', () => {
             ).toEqual([]);
         });
 
+        it('revokes only the named issuer grant, not another issuer identical one', async () => {
+            const issuerA = await makeUser();
+            const issuerB = await makeUser();
+            const holder = await makeUser();
+            await store.upsertUserUserPerm(
+                holder.id,
+                issuerA.id,
+                'fs:shared:read',
+                {},
+            );
+            await store.upsertUserUserPerm(
+                holder.id,
+                issuerB.id,
+                'fs:shared:read',
+                {},
+            );
+
+            expect(
+                await store.deleteUserUserPermByHolder(
+                    holder.id,
+                    'fs:shared:read',
+                    issuerA.id,
+                ),
+            ).toBe(true);
+
+            // Two people can grant the same access independently; one of them
+            // withdrawing must not take the other's grant with it.
+            const rows = await store.readLinkedUserUserPerms(holder.id, [
+                'fs:shared:read',
+            ]);
+            expect(rows).toHaveLength(1);
+            expect(rows[0].issuer_user_id).toBe(issuerB.id);
+        });
+
         it('reports whether the delete matched a row', async () => {
             const issuer = await makeUser();
             const holder = await makeUser();
@@ -938,15 +976,24 @@ describe('PermissionStore', () => {
             );
 
             expect(
-                await store.deleteUserUserPermByHolder(holder.id, 'fs:u:read'),
+                await store.deleteUserUserPermByHolder(
+                    holder.id,
+                    'fs:u:read',
+                    issuer.id,
+                ),
             ).toBe(true);
             expect(
-                await store.deleteUserUserPermByHolder(holder.id, 'fs:u:read'),
+                await store.deleteUserUserPermByHolder(
+                    holder.id,
+                    'fs:u:read',
+                    issuer.id,
+                ),
             ).toBe(false);
             expect(
                 await store.deleteUserUserPermByHolder(
                     holder.id,
                     'fs:never-granted:read',
+                    issuer.id,
                 ),
             ).toBe(false);
         });

src/backend/stores/permission/PermissionStore.ts

@@ -260,14 +260,22 @@ export class PermissionStore extends PuterStore {
         });
     }
 
-    /** Returns whether a row was actually deleted. */
+    /**
+     * Remove one issuer's grant. Scoped to the issuer because grants are keyed
+     * on (holder, issuer, permission) — two people can grant the same access
+     * independently, and one withdrawing must not take the other's with it.
+     *
+     * Returns whether a row was actually deleted.
+     */
     async deleteUserUserPermByHolder(
         holderUserId: number,
         permission: string,
+        issuerUserId: number,
     ): Promise<boolean> {
         const result = await this.clients.db.write(
-            'DELETE FROM `user_to_user_permissions` WHERE `holder_user_id` = ? AND `permission` = ?',
-            [holderUserId, permission],
+            'DELETE FROM `user_to_user_permissions` WHERE `holder_user_id` = ? ' +
+                'AND `permission` = ? AND `issuer_user_id` = ?',
+            [holderUserId, permission, issuerUserId],
         );
         if (!result.anyRowsAffected) return false;
         await this.publishCacheKeys({