fix(share): revoke every requested item and recipient, not just the first

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 <noreply@anthropic.com>
This commit is contained in:
Juan Castro
2026-08-14 14:35:19 -04:00
co-authored by Claude Fable 5
parent 982830a3ae
commit 18943d8c7c
2 changed files with 89 additions and 7 deletions
@@ -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<Record<string, unknown>>;
};
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);
@@ -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<void> {
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 });
}
/**