Share attribution and cascade: finish PUT-1856 and PUT-1857

Replayed onto main after #4118 squash-merged. Same tree as the reviewed
head (8d8bd871f); only the redundant merges from the old base are gone.

src/backend/services/share/ShareService.test.ts

@@ -1178,9 +1178,9 @@ describe('ShareService', () => {
             const listed = await listSharedByMe(owner.actor, {
                 includeTotal: true,
             });
-            expect(
-                listed.items.map((r) => r.holder?.username),
-            ).toContain(third.user.username);
+            expect(listed.items.map((r) => r.holder?.username)).toContain(
+                third.user.username,
+            );
             expect(listed.total).toBeGreaterThanOrEqual(2);
         });
 
@@ -1882,7 +1882,35 @@ describe('ShareService', () => {
 
         // One row records one issuance, so attribution follows the most
         // recent one — in both directions.
-        it('re-attributes a grant to whoever issued it last', async () => {
+        it('does not let an app claim a share the user made in person', async () => {
+            const owner = await makeUser();
+            const recipient = await makeUser();
+            const app = await makeApp(owner.user.id);
+            const file = await makeFile(owner.user);
+            await grantAppReach(owner, app, file);
+
+            // The user's own share, made without any app involved.
+            await share(owner.actor, {
+                uid: file.uuid,
+                recipient: { username: recipient.user.username },
+                mode: 'write',
+            });
+            const [mine] = (await listSharedByMe(owner.actor, { appUid: null }))
+                .items;
+
+            // The app re-shares the same pair; the row is still the user's.
+            await share(asApp(owner, app), {
+                uid: file.uuid,
+                recipient: { username: recipient.user.username },
+                mode: 'read',
+            });
+            expect((await listSharedByMe(asApp(owner, app))).items).toEqual([]);
+            await expect(
+                revokeByUid(asApp(owner, app), mine.uid),
+            ).rejects.toMatchObject({ statusCode: 404 });
+        });
+
+        it('lets a grant lose its app, and never take one back', async () => {
             const owner = await makeUser();
             const recipient = await makeUser();
             const app = await makeApp(owner.user.id);
@@ -1913,18 +1941,21 @@ describe('ShareService', () => {
                 revokeByUid(asApp(owner, app), uid),
             ).rejects.toMatchObject({ statusCode: 404 });
 
-            // And back: re-shared through the app, the same row is the app's
-            // again.
+            // Not back: re-crediting would let the app withdraw it.
             await share(asApp(owner, app), {
                 uid: file.uuid,
                 recipient: { username: recipient.user.username },
                 mode: 'read',
             });
+            expect((await listSharedByMe(asApp(owner, app))).items).toEqual([]);
             expect(
-                (await listSharedByMe(asApp(owner, app))).items.map(
+                (await listSharedByMe(owner.actor, { appUid: null })).items.map(
                     (i) => i.uid,
                 ),
             ).toEqual([uid]);
+            await expect(
+                revokeByUid(asApp(owner, app), uid),
+            ).rejects.toMatchObject({ statusCode: 404 });
         });
 
         // Every uid the caller may not act on answers alike, or the endpoint
@@ -2233,6 +2264,73 @@ describe('ShareService', () => {
         expect(await canRead(third.actor, file.path)).toBe(false);
     });
 
+    it("leaves a re-share resting on the delegate's own grant below", async () => {
+        const owner = await makeUser();
+        const delegate = await makeUser();
+        const third = await makeUser();
+        const { dir, file } = await makeDirWithFile(owner.user);
+
+        // Two separate grants: the folder, and the file inside it.
+        await share(owner.actor, {
+            uid: dir.uuid,
+            recipient: { email: delegate.email },
+            mode: 'manage',
+        });
+        await share(owner.actor, {
+            uid: file.uuid,
+            recipient: { email: delegate.email },
+            mode: 'manage',
+        });
+        await share(delegate.actor, {
+            uid: file.uuid,
+            recipient: { email: third.email },
+            mode: 'read',
+        });
+        expect(await canRead(third.actor, file.path)).toBe(true);
+
+        // The folder grant goes; the one on the file does not.
+        await unshare(owner.actor, {
+            uid: dir.uuid,
+            recipient: { username: delegate.user.username },
+        });
+
+        expect(await canRead(delegate.actor, dir.path)).toBe(false);
+        expect(await canRead(delegate.actor, file.path)).toBe(true);
+        expect(await canRead(third.actor, file.path)).toBe(true);
+    });
+
+    it("leaves the delegate's invite below a revoked folder standing", async () => {
+        const owner = await makeUser();
+        const delegate = await makeUser();
+        const { dir, file } = await makeDirWithFile(owner.user);
+
+        for (const uid of [dir.uuid, file.uuid]) {
+            await share(owner.actor, {
+                uid,
+                recipient: { email: delegate.email },
+                mode: 'manage',
+            });
+        }
+        // An invite, not a grant: nobody has claimed it yet.
+        await share(delegate.actor, {
+            uid: file.uuid,
+            recipient: { email: `nobody-${Math.random()}@test.local` },
+            mode: 'read',
+        });
+        const before = await server.stores.share.listPendingOnFsentry(file.id);
+        expect(before.length).toBeGreaterThan(0);
+
+        await unshare(owner.actor, {
+            uid: dir.uuid,
+            recipient: { username: delegate.user.username },
+        });
+
+        // The file grant stands, so the invite made under it does too.
+        expect(
+            await server.stores.share.listPendingOnFsentry(file.id),
+        ).toHaveLength(before.length);
+    });
+
     it('keeps the index row when the actor could not revoke anything', async () => {
         const owner = await makeUser();
         const delegate = await makeUser();
@@ -5639,6 +5737,5 @@ describe('ShareService', () => {
             expect(listed.some((row) => row.pending)).toBe(false);
             expect(JSON.stringify(listed)).not.toContain(email);
         });
-
     });
 });

src/backend/services/share/ShareService.ts

@@ -1449,6 +1449,61 @@ export class ShareService extends PuterService {
         );
     }
 
+    /** Nodes under `entry` that `userId` manages by a grant of their own. */
+    async #nodesManagedDirectly(
+        entry: FSEntry,
+        userId: number,
+        unwinding: Set<number>,
+    ): Promise<Set<number>> {
+        // Every row, not just the applied ones: a node whose only share is a
+        // pending invite or a team grant is exactly what the group and invite
+        // sweeps ask about.
+        const rows = await this.stores.share.listAllByFsentrySubtree(entry.id);
+        const nodes = await this.stores.fsEntry.getEntriesByIds(
+            rows.map((row: { fsentry_id: number }) => Number(row.fsentry_id)),
+        );
+        const candidates = [...nodes.values()].filter(
+            (node) => node.id !== entry.id,
+        );
+        if (candidates.length === 0) return new Set<number>();
+
+        // One read for the whole subtree: the query is holder-scoped, so
+        // asking per node is the same rows over and over.
+        const permOf = (node: FSEntry) =>
+            entryPermissionForMode(node.uuid, MANAGE_PERM_PREFIX);
+        const wanted = candidates.map(permOf);
+        const [linked, viaGroup] = await Promise.all([
+            this.stores.permission.readLinkedUserUserPermsFromPrimary(
+                userId,
+                wanted,
+            ),
+            this.stores.permission.readUserGroupPerms(userId, wanted),
+        ]);
+        // Not from anyone being unwound: two delegates can hold each other up.
+        const held = new Set<string>([
+            ...linked
+                .filter((row) => !unwinding.has(Number(row.issuer_user_id)))
+                .map((row) => String(row.permission)),
+            ...viaGroup.map((row) => String(row.permission)),
+        ]);
+
+        // `manage` inherits downwards, so a grant on a folder between `entry`
+        // and the node answers for the node too.
+        const managedNodes = candidates.filter((node) =>
+            held.has(permOf(node)),
+        );
+        const managed = new Set<number>();
+        for (const node of candidates) {
+            const covered = managedNodes.some(
+                (held) =>
+                    held.id === node.id ||
+                    node.path.startsWith(`${held.path}/`),
+            );
+            if (covered) managed.add(node.id);
+        }
+        return managed;
+    }
+
     /**
      * Withdraw everything `issuerId` granted on this node, and everything those
      * recipients granted in turn.
@@ -1465,19 +1520,6 @@ export class ShareService extends PuterService {
         if (seen.has(issuerId)) return 0;
         seen.add(issuerId);
 
-        // Their unclaimed invites go the same way as their re-shares: an
-        // invite rests on the same authority, and nothing else retires it —
-        // claiming re-checks, but only when the recipient shows up, and until
-        // then the row keeps the entry in the revoked issuer's listing.
-        await this.stores.share.deletePendingByIssuerSubtree(
-            issuerId,
-            entry.id,
-        );
-
-        // What they re-shared to teams goes too: a group grant left behind is
-        // dormant, and springs back if the issuer ever requalifies.
-        let revoked = await this.#revokeGroupSharesBy(actor, entry, issuerId);
-
         // The whole subtree, not just this node: `manage` inherits downwards,
         // so a grant on a descendant can rest on authority held here.
         const rows = (
@@ -1486,6 +1528,28 @@ export class ShareService extends PuterService {
             (row: { issuer_user_id: number }) =>
                 Number(row.issuer_user_id) === issuerId,
         );
+
+        // Nodes they manage in their own right, exempt from all three sweeps.
+        const exempt = await this.#nodesManagedDirectly(entry, issuerId, seen);
+
+        // Their unclaimed invites go the same way as their re-shares: an
+        // invite rests on the same authority, and nothing else retires it —
+        // claiming re-checks, but only when the recipient shows up, and until
+        // then the row keeps the entry in the revoked issuer's listing.
+        await this.stores.share.deletePendingByIssuerSubtree(
+            issuerId,
+            entry.id,
+            { exemptFsentryIds: [...exempt] },
+        );
+
+        // What they re-shared to teams goes too: a group grant left behind is
+        // dormant, and springs back if the issuer ever requalifies.
+        let revoked = await this.#revokeGroupSharesBy(
+            actor,
+            entry,
+            issuerId,
+            exempt,
+        );
         if (rows.length === 0) return revoked;
 
         const [nodes, holders] = await Promise.all([
@@ -1507,6 +1571,9 @@ export class ShareService extends PuterService {
             const downstream = holders.get(holderId);
             if (!node || !downstream?.username) continue;
 
+            // Their own authority here, not the one being withdrawn above.
+            if (node.id !== entry.id && exempt.has(node.id)) continue;
+
             const { revoked: didRevoke, authorized } = await this.#revokeFor(
                 actor,
                 node,
@@ -1547,12 +1614,14 @@ export class ShareService extends PuterService {
         actor: Actor,
         entry: FSEntry,
         issuerId: number,
+        exempt: Set<number> = new Set(),
     ): Promise<number> {
         const rows = (
             await this.stores.share.listGroupSharesBySubtree(entry.id)
         ).filter(
-            (row: { issuer_user_id: number }) =>
-                Number(row.issuer_user_id) === issuerId,
+            (row: { issuer_user_id: number; fsentry_id: number }) =>
+                Number(row.issuer_user_id) === issuerId &&
+                !exempt.has(Number(row.fsentry_id)),
         );
         if (rows.length === 0) return 0;
 

src/backend/services/share/TeamShareNotify.test.ts

@@ -208,7 +208,8 @@ describe('announcing a team share', () => {
         expect(
             sent.some((mail) => mail.to.includes(fx.b.owner.username)),
         ).toBe(false);
-    });
+        // Longer than the wait above, which the 5s default cuts short.
+    }, 30_000);
 
     it('does not announce to a member who blocked the sharer', async () => {
         // A member nobody has notified yet, so a new row is detectable. Reusing

src/backend/stores/share/ShareStore.js

@@ -600,17 +600,19 @@ export class ShareStore extends PuterStore {
         }
 
         const existing = await this.clients.db.read(
-            'SELECT `uid` FROM `share` WHERE `recipient_email` = ? AND ' +
+            'SELECT `uid`, `data` FROM `share` WHERE `recipient_email` = ? AND ' +
                 '`fsentry_id` = ? AND `issuer_user_id` = ? AND ' +
                 '`holder_user_id` IS NULL LIMIT 1',
             [recipientEmail, fsentryId, issuerUserId],
         );
-        // Same key an active share records the app under, so one reader covers
-        // an invite and the grant it becomes. Attribution follows the most
-        // recent issuance: re-inviting refreshes `data`, exactly as
-        // `upsertActive` does on conflict.
+        // The key an active share uses, so one reader covers both.
+        const attribution = existing[0]
+            ? this.#keptAttribution(existing[0].data, issuerAppUid)
+            : issuerAppUid
+              ? { issuedByApp: issuerAppUid }
+              : {};
         const data = JSON.stringify({
-            ...(issuerAppUid ? { issuedByApp: issuerAppUid } : {}),
+            ...attribution,
             ...(displayEmail && displayEmail !== recipientEmail
                 ? { invitedAddress: displayEmail }
                 : {}),
@@ -689,11 +691,21 @@ export class ShareStore extends PuterStore {
             );
         }
 
-        // A share issued through an app is attributed to the user, because the
-        // grant is theirs. `data` records which app asked for it, so the owner
-        // can tell an app-issued share from one they made themselves.
+        // The user's grant; `data` records which app asked for it.
+        // No app asked, so there is nothing to keep and nothing to read for.
+        const prior = issuerAppUid
+            ? await this.getActive({
+                  holderUserId,
+                  fsentryId,
+                  issuerUserId,
+              })
+            : null;
         const data = JSON.stringify(
-            issuerAppUid ? { issuedByApp: issuerAppUid } : {},
+            prior
+                ? this.#keptAttribution(prior.data, issuerAppUid)
+                : issuerAppUid
+                  ? { issuedByApp: issuerAppUid }
+                  : {},
         );
         await this.clients.db.write(
             'INSERT INTO `share` (`uid`, `issuer_user_id`, `recipient_email`, ' +
@@ -745,8 +757,20 @@ export class ShareStore extends PuterStore {
                 'upsertActiveGroup: issuerUserId, holderGroupId, fsentryId and mode are required',
             );
         }
+        // No app asked, so there is nothing to keep and nothing to read for.
+        const prior = issuerAppUid
+            ? await this.getActiveGroup({
+                  holderGroupId,
+                  fsentryId,
+                  issuerUserId,
+              })
+            : null;
         const data = JSON.stringify(
-            issuerAppUid ? { issuedByApp: issuerAppUid } : {},
+            prior
+                ? this.#keptAttribution(prior.data, issuerAppUid)
+                : issuerAppUid
+                  ? { issuedByApp: issuerAppUid }
+                  : {},
         );
         await this.clients.db.write(
             'INSERT INTO `share` (`uid`, `issuer_user_id`, `recipient_email`, ' +
@@ -928,13 +952,22 @@ export class ShareStore extends PuterStore {
         );
     }
 
-    async deletePendingByIssuerSubtree(issuerUserId, fsentryId) {
+    /**
+     * @param {number} issuerUserId @param {number} fsentryId
+     * @param {{ exemptFsentryIds?: number[] }} [opts]
+     */
+    async deletePendingByIssuerSubtree(
+        issuerUserId,
+        fsentryId,
+        { exemptFsentryIds = [] } = {},
+    ) {
         // Read-then-delete rather than a CTE inside the DELETE, which the
         // dialects disagree on. The gap between the two only ever leaves an
         // invite standing, and the claim path re-checks authority anyway.
+        const exempt = new Set(exemptFsentryIds.map(Number));
         const rows = await this.clients.db.read(
             this.#subtreeCte() +
-                'SELECT `share`.`uid` FROM `share` ' +
+                'SELECT `share`.`uid`, `share`.`fsentry_id` FROM `share` ' +
                 'JOIN `subtree` ON `share`.`fsentry_id` = `subtree`.`id` ' +
                 // Group and link rows also have no holder user; deleting one
                 // here would drop the index row and leave its grant standing.
@@ -944,11 +977,15 @@ export class ShareStore extends PuterStore {
                 '`share`.`issuer_user_id` = ?',
             [fsentryId, issuerUserId],
         );
-        if (rows.length === 0) return 0;
-        const placeholders = rows.map(() => '?').join(', ');
+        // Not the ones on a node they manage in their own right.
+        const retired = rows.filter(
+            (row) => !exempt.has(Number(row.fsentry_id)),
+        );
+        if (retired.length === 0) return 0;
+        const placeholders = retired.map(() => '?').join(', ');
         const result = await this.clients.db.write(
             `DELETE FROM \`share\` WHERE \`uid\` IN (${placeholders})`,
-            rows.map((row) => row.uid),
+            retired.map((row) => row.uid),
         );
         return result?.affectedRows ?? result?.changes ?? 0;
     }
@@ -1049,8 +1086,14 @@ export class ShareStore extends PuterStore {
                 'upsertAnyone: issuerUserId, fsentryId and mode are required',
             );
         }
+        // As the other upserts: re-issuing may lose the app, never switch it.
+        const prior = issuerAppUid ? await this.getAnyone(fsentryId) : null;
         const data = JSON.stringify(
-            issuerAppUid ? { issuedByApp: issuerAppUid } : {},
+            prior
+                ? this.#keptAttribution(prior.data, issuerAppUid)
+                : issuerAppUid
+                  ? { issuedByApp: issuerAppUid }
+                  : {},
         );
         await this.clients.db.write(
             'INSERT INTO `share` (`uid`, `issuer_user_id`, `recipient_email`, ' +
@@ -1123,6 +1166,27 @@ export class ShareStore extends PuterStore {
         return typeof count === 'number' ? count : amount;
     }
 
+    /**
+     * Who a re-issued share stays credited to: only the same app re-issuing
+     * keeps it, so a share may lose attribution but never gain or switch it.
+     */
+    #keptAttribution(existingData, issuerAppUid) {
+        let prior = existingData ?? {};
+        if (typeof prior === 'string') {
+            // `data` is not always JSON on sqlite; `#normalizeRow` says so too.
+            try {
+                prior = JSON.parse(prior || '{}');
+            } catch {
+                prior = {};
+            }
+        }
+        // `issuerAppUid` is the older spelling, read in two other places.
+        const priorApp = prior?.issuedByApp ?? prior?.issuerAppUid;
+        return issuerAppUid && priorApp === issuerAppUid
+            ? { issuedByApp: issuerAppUid }
+            : {};
+    }
+
     /** @param {number} userId @param {string} scope */
     #dailyQuotaKey(userId, scope = 'quota') {
         const day = new Date().toISOString().slice(0, 10);

src/backend/stores/share/ShareStore.test.js

@@ -3,18 +3,19 @@
  *
  * This file is part of Puter.
  *
- * Puter is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as published
- * by the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * Puter is free software: you can redistribute it and/or modify it under the
+ * terms of the GNU Affero General Public License as published by the Free
+ * Software Foundation, either version 3 of the License, or (at your option) any
+ * later version.
  *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ * details.
  *
  * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ * along with this program. If not, see
+ * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/).
  */
 
 import { v4 as uuidv4 } from 'uuid';
@@ -268,7 +269,13 @@ describe('ShareStore', () => {
             const dirUuid = uuidv4();
             await server.clients.db.write(
                 'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`) VALUES (?, ?, ?, ?, 1, ?)',
-                [dirUuid, `d-${dirUuid.slice(0, 8)}`, `/x/${dirUuid}`, issuer.id, now],
+                [
+                    dirUuid,
+                    `d-${dirUuid.slice(0, 8)}`,
+                    `/x/${dirUuid}`,
+                    issuer.id,
+                    now,
+                ],
             );
             const dirRows = await server.clients.db.read(
                 'SELECT `id` FROM `fsentries` WHERE `uuid` = ?',
@@ -279,7 +286,14 @@ describe('ShareStore', () => {
             const childUuid = uuidv4();
             await server.clients.db.write(
                 'INSERT INTO `fsentries` (`uuid`, `name`, `path`, `user_id`, `is_dir`, `modified`, `parent_id`, `parent_uid`) VALUES (?, ?, NULL, ?, 0, ?, ?, ?)',
-                [childUuid, `f-${childUuid.slice(0, 8)}`, issuer.id, now, dirId, dirUuid],
+                [
+                    childUuid,
+                    `f-${childUuid.slice(0, 8)}`,
+                    issuer.id,
+                    now,
+                    dirId,
+                    dirUuid,
+                ],
             );
             const childRows = await server.clients.db.read(
                 'SELECT `id` FROM `fsentries` WHERE `uuid` = ?',
@@ -295,9 +309,9 @@ describe('ShareStore', () => {
             });
 
             const rows = await store.listByFsentrySubtree(dirId);
-            expect(
-                rows.some((r) => Number(r.fsentry_id) === childId),
-            ).toBe(true);
+            expect(rows.some((r) => Number(r.fsentry_id) === childId)).toBe(
+                true,
+            );
         });
 
         it('records an active share and lists it for the holder', async () => {
@@ -318,6 +332,43 @@ describe('ShareStore', () => {
             expect(page.items.map((r) => r.uid)).toContain(created.uid);
         });
 
+        it('lets a link share lose its app, and never take another one', async () => {
+            const entry = await makeEntry(issuer);
+            const appOf = (row) => {
+                const data =
+                    typeof row.data === 'string'
+                        ? JSON.parse(row.data || '{}')
+                        : (row.data ?? {});
+                return data.issuedByApp ?? null;
+            };
+
+            await store.upsertAnyone({
+                issuerUserId: issuer.id,
+                fsentryId: entry.id,
+                mode: 'read',
+                issuerAppUid: 'app-one',
+            });
+            expect(appOf(await store.getAnyone(entry.id))).toBe('app-one');
+
+            // The same app again keeps it.
+            await store.upsertAnyone({
+                issuerUserId: issuer.id,
+                fsentryId: entry.id,
+                mode: 'write',
+                issuerAppUid: 'app-one',
+            });
+            expect(appOf(await store.getAnyone(entry.id))).toBe('app-one');
+
+            // A different one does not take it.
+            await store.upsertAnyone({
+                issuerUserId: issuer.id,
+                fsentryId: entry.id,
+                mode: 'read',
+                issuerAppUid: 'app-two',
+            });
+            expect(appOf(await store.getAnyone(entry.id))).toBeNull();
+        });
+
         it('moves an existing share to a new mode instead of duplicating it', async () => {
             const entry = await makeEntry(issuer);
             const first = await store.upsertActive({