From 18943d8c7ce617ebe00cce7a1a3024e3fc91ee80 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Fri, 14 Aug 2026 14:35:19 -0400 Subject: [PATCH] fix(share): revoke every requested item and recipient, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit revokeShare destructured only the first recipient and first item while the parsers accept arrays up to the request caps — unshare({items: [a, b, c]}) returned success having revoked only a, leaving access the caller believes is gone. Revoke now fans out over every (recipient, item) pair exactly like POST /share, reports per-pair outcomes, and sums the revoked count; the response stays backward compatible. Co-Authored-By: Claude Fable 5 --- .../share/ShareController.http.test.ts | 38 ++++++++++++ .../controllers/share/ShareController.ts | 58 ++++++++++++++++--- 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index 5bf58e7c9..bfedd6955 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/src/backend/controllers/share/ShareController.http.test.ts @@ -120,6 +120,44 @@ describe('share endpoints over HTTP', () => { expect(after.items.find((i) => i.uid_entry === file.uid)).toBeUndefined(); }); + it('revokes every item in the request, not just the first', async () => { + const owner = env.users.user; + const recipient = env.users.other; + const fileA = await makeFile(owner); + const fileB = await makeFile(owner); + + for (const file of [fileA, fileB]) { + const res = await post('/share', owner.token, { + recipients: [recipient.username], + items: [{ uid: file.uid }], + mode: 'read', + }); + expect(res.status).toBe(200); + } + + // A truncated revoke is a silent security failure: the caller is told + // "success" while items after the first keep their grants. + const revokeRes = await post('/share/revoke', owner.token, { + recipients: [recipient.username], + items: [{ uid: fileA.uid }, { uid: fileB.uid }], + }); + expect(revokeRes.status).toBe(200); + expect(await revokeRes.json()).toMatchObject({ + status: 'success', + revoked: 2, + }); + + const afterRes = await get('/share/shared-with-me', recipient.token, {}); + const after = (await afterRes.json()) as { + items: Array>; + }; + for (const file of [fileA, fileB]) { + expect( + after.items.find((i) => i.uid_entry === file.uid), + ).toBeUndefined(); + } + }); + it('reports per-pair outcomes when only some recipients resolve', async () => { const owner = env.users.user; const file = await makeFile(owner); diff --git a/src/backend/controllers/share/ShareController.ts b/src/backend/controllers/share/ShareController.ts index ead80c55c..d68a2e658 100644 --- a/src/backend/controllers/share/ShareController.ts +++ b/src/backend/controllers/share/ShareController.ts @@ -159,7 +159,12 @@ export class ShareController extends PuterController { }); } - /** POST /share/revoke — withdraw a recipient's access to an item. */ + /** + * POST /share/revoke — withdraw recipients' access to items. Same fan-out + * contract as POST /share: every (recipient, item) pair is its own revoke + * with its own outcome. Silently dropping pairs after the first would leave + * access standing that the caller believes is gone. + */ @Post('/revoke', { subdomain: 'api', requireVerified: true, @@ -169,14 +174,53 @@ export class ShareController extends PuterController { async revokeShare(req: Request, res: Response): Promise { const actor = this.#requireActor(req); const body = this.#body(req); - const [recipient] = this.#recipients(body); - const [item] = this.#items(body); + const recipients = this.#recipients(body); + const items = this.#items(body); - const result = await this.services.share.unshare(actor, { - ...item, - recipient, + const pairs = recipients.flatMap((recipient) => + items.map((item) => ({ recipient, item })), + ); + + const settled = await runWithConcurrencyLimitSettled( + pairs, + SHARE_CONCURRENCY, + ({ recipient, item }) => + this.services.share.unshare(actor, { ...item, recipient }), + ); + + let revoked = 0; + const results: ShareOutcome[] = settled.map((outcome, index) => { + const { recipient, item } = pairs[index]; + const label = recipient.email ?? recipient.username ?? ''; + if (outcome.status === 'fulfilled') { + revoked += outcome.value.revoked; + return { + recipient: label, + ...(item.path ? { path: item.path } : {}), + ...(item.uid ? { uid: item.uid } : {}), + status: 'success', + }; + } + return { + recipient: label, + ...(item.path ? { path: item.path } : {}), + ...(item.uid ? { uid: item.uid } : {}), + status: 'error', + ...this.#errorShape(outcome.reason), + }; + }); + + const succeeded = results.filter((r) => r.status === 'success').length; + res.json({ + status: + succeeded === results.length + ? 'success' + : succeeded > 0 + ? 'mixed' + : 'aborted', + revoked, + results, }); - res.json({ status: 'success', revoked: result.revoked }); } /**