Merge pull request #3892 from HeyPuter/juancastro/put-1806-invite-email-addresses-are-disclosed-to-apps-manage

fix: stop disclosing invite addresses to apps, tokens and delegates

approvals already in place

src/backend/controllers/share/ShareController.http.test.ts

@@ -26,6 +26,7 @@ import {
     type PuterTestEnv,
 } from '../../testUtil.js';
 import { ShareController } from './ShareController.js';
+import { FULL_API_ACCESS } from '../../services/permission/consts.js';
 
 /**
  * Route-level coverage for the sharing endpoints. The service unit tests drive
@@ -1069,4 +1070,156 @@ describe('share endpoints over HTTP', () => {
             expect(route.options?.allowFullAccessToken).toBeUndefined();
         }
     });
+
+    describe('an invited address over the wire', () => {
+        /** An app of the owner's, holding `mode` on one file, with a token. */
+        const appHolding = async (
+            owner: { username: string },
+            uid: string,
+            permission: (fileUid: string) => string,
+        ) => {
+            const user = await env.server.stores.user.getByUsername(
+                owner.username,
+            );
+            const actor = makeActor({ user: user! });
+            const app = await env.server.stores.app.create(
+                {
+                    name: `peek-app-${crypto.randomUUID().slice(0, 8)}`,
+                    title: 'Peek app',
+                    index_url: `https://peek-${crypto.randomUUID()}.test/`,
+                },
+                { ownerUserId: user!.id },
+            );
+            await runWithContext({ actor }, () =>
+                env.server.services.permission.grantUserAppPermission(
+                    actor,
+                    app.uid,
+                    permission(uid),
+                ),
+            );
+            const token = await env.server.services.auth.getUserAppToken(
+                actor,
+                app.uid,
+            );
+            return { ...app, token };
+        };
+
+        const invitedFile = async (owner: { username: string; token: string }) => {
+            const file = await makeFile(owner);
+            const email = `wire-${crypto.randomUUID().slice(0, 8)}@test.local`;
+            const res = await post('/share', owner.token, {
+                recipients: [email],
+                items: [{ uid: file.uid }],
+                mode: 'read',
+            });
+            expect(res.status).toBe(200);
+            return { file, email };
+        };
+
+        it('reaches the owner but not an app the file was handed to', async () => {
+            const owner = env.users.user;
+            const { file, email } = await invitedFile(owner);
+
+            // The owner asked, so the owner is told.
+            const mine = await get('/share/shares', owner.token, {
+                uid: file.uid,
+            });
+            expect(mine.status).toBe(200);
+            expect(await mine.text()).toContain(email);
+
+            // The app is answered, but bounded to the rows it issued — and it
+            // issued none, so the owner's invite is not among them.
+            const app = await appHolding(owner, file.uid, (u) => `fs:${u}:read`);
+            const peeked = await get('/share/shares', app.token, {
+                uid: file.uid,
+            });
+            expect(peeked.status).toBe(200);
+            const peekedBody = await peeked.text();
+            expect(peekedBody).not.toContain(email);
+            expect(JSON.parse(peekedBody).items).toEqual([]);
+
+            // `return_shares` is the same listing on an access-token surface.
+            const stat = await post('/fs/stat', app.token, {
+                uid: file.uid,
+                return_shares: true,
+            });
+            expect(stat.status).toBe(200);
+            const body = await stat.text();
+            expect(body).not.toContain(email);
+            expect(JSON.parse(body).shares).toEqual([]);
+        });
+
+        // The ticket's second route in: `ShareController` admits no access
+        // token, but `/fs/stat` does, and `return_shares` is the same listing.
+        it('withholds the listing from a list-scoped access token', async () => {
+            const owner = env.users.user;
+            const { file, email } = await invitedFile(owner);
+            const user = await env.server.stores.user.getByUsername(
+                owner.username,
+            );
+            const actor = makeActor({ user: user! });
+            const scoped = await runWithContext({ actor }, () =>
+                env.server.services.auth.createAccessToken(actor, [
+                    [`fs:${file.uid}:list`],
+                ]),
+            );
+
+            const stat = await post('/fs/stat', scoped, {
+                uid: file.uid,
+                return_shares: true,
+            });
+            expect(stat.status).toBe(200);
+            const body = await stat.text();
+            expect(body).not.toContain(email);
+            expect(JSON.parse(body).shares).toEqual([]);
+        });
+
+        // Withholding here would buy nothing: `shared-by-me` hands the same
+        // addresses to the same credential.
+        it('tells a full-access token what it could read from shared-by-me anyway', async () => {
+            const owner = env.users.user;
+            const { file, email } = await invitedFile(owner);
+            const user = await env.server.stores.user.getByUsername(
+                owner.username,
+            );
+            const actor = makeActor({ user: user! });
+            const pat = await runWithContext({ actor }, () =>
+                env.server.services.auth.createAccessToken(actor, [
+                    [FULL_API_ACCESS],
+                ]),
+            );
+
+            const outbound = await get('/share/shared-by-me', pat, {});
+            expect(outbound.status).toBe(200);
+            expect(await outbound.text()).toContain(email);
+
+            const listed = await get('/share/shares', pat, { uid: file.uid });
+            expect(listed.status).toBe(200);
+            expect(await listed.text()).toContain(email);
+        });
+
+        it('answers an app that was handed the item to manage', async () => {
+            const owner = env.users.user;
+            const { file, email } = await invitedFile(owner);
+            const app = await appHolding(
+                owner,
+                file.uid,
+                (u) => `manage:fs:${u}`,
+            );
+
+            const res = await get('/share/shares', app.token, {
+                uid: file.uid,
+            });
+            expect(res.status).toBe(200);
+            const text = await res.text();
+            // `manage` buys an app authority over the item, not sight of who
+            // else the owner invited to it.
+            expect(
+                JSON.parse(text).items.some(
+                    (i: { pending?: boolean }) => i.pending,
+                ),
+            ).toBe(false);
+            expect(text).not.toContain(email);
+        });
+    });
 });

src/backend/controllers/share/clientShare.ts

@@ -54,8 +54,14 @@ export async function toClientShare(
         ...(share.type === undefined ? {} : { type: share.type }),
         ...(thumbnail === undefined ? {} : { thumbnail }),
         ...(share.owner === undefined ? {} : { owner: share.owner.username }),
+        // Withheld from a listing whose caller it isn't for, hence the omission.
         ...(share.pending
-            ? { pending: true, recipient_email: share.recipientEmail }
+            ? {
+                  pending: true,
+                  ...(share.recipientEmail === undefined
+                      ? {}
+                      : { recipient_email: share.recipientEmail }),
+              }
             : {}),
         // A link share: no holder of any kind, this is what says so.
         ...(share.anyone ? { anyone: true } : {}),

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

@@ -23,6 +23,7 @@ import { makeActor, type Actor } from '../../core/actor.js';
 import { runWithContext } from '../../core/context.js';
 import { PuterServer } from '../../server.js';
 import { createTestUser, setupTestServer } from '../../testUtil.js';
+import { MANAGE_PERM_PREFIX } from '../permission/consts.js';
 
 describe('ShareService', () => {
     let server: PuterServer;
@@ -5312,4 +5313,105 @@ describe('ShareService', () => {
             );
         });
     });
+
+    // An invite names a third party who never agreed to be named. The listing
+    // is the one place those addresses surface, so who may read one is a gate
+    // of its own, independent of who may read the listing at all.
+    describe('who may read an invite address', () => {
+        /** An owner, a file, and an unclaimed invite on it. */
+        const withInvite = async () => {
+            const owner = await makeUser();
+            const file = await makeFile(owner.user);
+            const email = `invited-${Math.random().toString(36).slice(2, 8)}@test.local`;
+            await share(owner.actor, {
+                uid: file.uuid,
+                recipient: { email },
+                mode: 'read',
+            });
+            return { owner, file, email };
+        };
+
+        const listOf = (actor: Actor, uuid: string) =>
+            server.services.share.listSharesOf(actor, { uid: uuid });
+
+        it('tells the owner who they invited', async () => {
+            const { owner, file, email } = await withInvite();
+            const [invite] = await listOf(owner.actor, file.uuid);
+            expect(invite.pending).toBe(true);
+            expect(invite.recipientEmail).toBe(email);
+        });
+
+        it('withholds it from a manage delegate who did not send it', async () => {
+            const { owner, file, email } = await withInvite();
+            const delegate = await makeUser();
+            await share(owner.actor, {
+                uid: file.uuid,
+                recipient: { username: delegate.user.username },
+                mode: 'manage',
+            });
+
+            const listed = await listOf(delegate.actor, file.uuid);
+            const invite = listed.find((row) => row.pending);
+            // Listed — they manage the item and must know an invite is out —
+            // but the address is the owner's to know.
+            expect(invite).toBeDefined();
+            expect(invite!.recipientEmail).toBeUndefined();
+            expect(JSON.stringify(listed)).not.toContain(email);
+        });
+
+        it('tells a delegate the address of an invite they sent', async () => {
+            const { owner, file } = await withInvite();
+            const delegate = await makeUser();
+            await share(owner.actor, {
+                uid: file.uuid,
+                recipient: { username: delegate.user.username },
+                mode: 'manage',
+            });
+            const theirs = `theirs-${Math.random().toString(36).slice(2, 8)}@test.local`;
+            await share(delegate.actor, {
+                uid: file.uuid,
+                recipient: { email: theirs },
+                mode: 'read',
+            });
+
+            const listed = await listOf(delegate.actor, file.uuid);
+            const addresses = listed
+                .filter((row) => row.pending)
+                .map((row) => row.recipientEmail);
+            // Their own, and only their own.
+            expect(addresses).toContain(theirs);
+            expect(addresses).toContain(undefined);
+        });
+
+        it('shows an app none of the invites it did not send', async () => {
+            const { owner, file, email } = await withInvite();
+            const app = await makeApp(owner.user.id);
+            await grantAppReach(owner, app, file, 'read');
+
+            // An app is bounded to the rows it issued, so the owner's invite
+            // is not among them — and its address cannot leak with it.
+            const listed = await listOf(asApp(owner, app), file.uuid);
+            expect(listed.some((row) => row.pending)).toBe(false);
+            expect(JSON.stringify(listed)).not.toContain(email);
+        });
+
+        it('shows an app with manage no more than one without', async () => {
+            const { owner, file, email } = await withInvite();
+            const app = await makeApp(owner.user.id);
+            await runWithContext({ actor: owner.actor }, () =>
+                server.services.permission.grantUserAppPermission(
+                    owner.actor,
+                    app.uid,
+                    `${MANAGE_PERM_PREFIX}:fs:${file.uuid}`,
+                ),
+            );
+
+            // `manage` buys an app authority over the item, not sight of who
+            // else the owner invited to it.
+            const listed = await listOf(asApp(owner, app), file.uuid);
+            expect(listed.some((row) => row.pending)).toBe(false);
+            expect(JSON.stringify(listed)).not.toContain(email);
+        });
+
+    });
 });

src/backend/services/share/ShareService.ts

@@ -20,6 +20,7 @@
 import { contentType as contentTypeFromMime } from 'mime-types';
 import { posix as pathPosix } from 'node:path';
 import {
+    isAccountContext,
     isPlainUserActor,
     makeActor,
     userRelatedActor,
@@ -2269,10 +2270,19 @@ export class ShareService extends PuterService {
         // reaches the node — another app, a delegate, an address the user
         // invited — is the user's business, not the app's.
         const actingApp = this.#actingAppUid(actor);
-        const issuedHere = <T extends { data?: unknown }>(list: T[]): T[] =>
-            actingApp
-                ? list.filter((row) => issuedByApp(row) === actingApp)
-                : list;
+        // A scoped token issues nothing under its own name, so it is bounded
+        // to nothing: filtering it by a null app would hand it the owner's own
+        // rows instead of none. A full-access token holds the account's reach
+        // and is not bounded at all.
+        const scopedToken =
+            !!actor.accessToken && actor.accessToken.fullAccess !== true;
+        const issuedHere = <T extends { data?: unknown }>(list: T[]): T[] => {
+            if (scopedToken) return [];
+            if (actingApp) {
+                return list.filter((row) => issuedByApp(row) === actingApp);
+            }
+            return list;
+        };
 
         const inherited: Array<{ row: ShareIndexRow; via: string }> =
             issuedHere(
@@ -2365,6 +2375,7 @@ export class ShareService extends PuterService {
             (row: OutboundShareRow) =>
                 this.#resolvedShareRow(row, entry, users, {
                     path: maskedPath,
+                    inviteAddress: this.#maySeeInviteAddress(actor, entry, row),
                 }),
         );
 
@@ -3138,6 +3149,11 @@ export class ShareService extends PuterService {
         opts: {
             entryMeta?: boolean;
             provenance?: boolean;
+            /**
+             * Withholds an invite's address when false; see
+             * `#maySeeInviteAddress`.
+             */
+            inviteAddress?: boolean;
             holderUsername?: string | null;
             holderTeam?: {
                 uid: string;
@@ -3187,9 +3203,16 @@ export class ShareService extends PuterService {
             ...(pending
                 ? {
                       pending: true,
-                      recipientEmail:
-                          (row.data as { invitedAddress?: string } | null)
-                              ?.invitedAddress ?? row.recipient_email,
+                      ...(opts.inviteAddress === false
+                          ? {}
+                          : {
+                                recipientEmail:
+                                    (
+                                        row.data as {
+                                            invitedAddress?: string;
+                                        } | null
+                                    )?.invitedAddress ?? row.recipient_email,
+                            }),
                   }
                 : {}),
             ...(opts.holderTeam ? { holderTeam: opts.holderTeam } : {}),
@@ -3480,16 +3503,39 @@ export class ShareService extends PuterService {
             }
         }
 
+        throw await this.#manageRefusal(actor, entry);
+    }
+
+    /**
+     * The ACL's own safe refusal, so a caller who can't see the node learns
+     * nothing.
+     */
+    async #manageRefusal(actor: Actor, entry: FSEntry): Promise<HttpError> {
         const safe = await this.services.acl.getSafeAclError(
             actor,
             this.#descriptorFor(entry),
             'manage',
         );
-        throw new HttpError(safe.status, safe.message, {
+        return new HttpError(safe.status, safe.message, {
             legacyCode: safe.fields.code,
         });
     }
 
+    /** An invite's address is the owner's and the issuer's, and no app's. */
+    #maySeeInviteAddress(
+        actor: Actor,
+        entry: FSEntry,
+        row: OutboundShareRow,
+    ): boolean {
+        if (!isAccountContext(actor)) return false;
+        const userId = actor.user?.id;
+        if (typeof userId !== 'number') return false;
+        return (
+            userId === Number(entry.userId) ||
+            userId === Number(row.issuer_user_id)
+        );
+    }
+
     /**
      * Of `entries`, the uuids the acting credential reaches in its own right. A
      * plain session reaches all of them; the checks only run for an app or a

src/docs/src/FS/getShares.md

@@ -6,6 +6,8 @@ platforms: [websites, apps, nodejs, workers]
 
 This method lists who can reach a file or directory you own, or one you have `manage` access to.
 
+Being handed the item is not enough for an **app or API token**: the credential itself must hold `manage` on it, because the answer covers the folders above the item as well. One given a file to read gets a rejection here, and an empty `shares` from [`stat()`](/FS/stat/).
+
 > **What an app can share.** An app never gets more reach than it was given. It
 > can share its own AppData, and files the user specifically granted it, at up
 > to the level of access it holds itself — so an app with read access can grant
@@ -52,6 +54,8 @@ If the item is open to **anyone with the link** (see [`share()`](/FS/share/)), t
 
 It also includes **invitations** — shares aimed at an email address with no confirmed account yet. Those carry `pending: true`, a `null` `holder`, and the address in `recipientEmail`. They grant nothing until the recipient confirms that address, and [`unshare()`](/FS/unshare/) cancels one before it is claimed.
 
+`recipientEmail` is set only for the item's **owner** and for whoever **sent** that invitation; for anyone else the invitation is listed without it. Someone else's invitation is not yours to cancel either, so nothing is lost with the address. An app never sees it, whoever it acts for.
+
 If you cannot see the item at all, this rejects the same way a missing file would — it will not confirm that the item exists.
 
 ## Examples

src/docs/src/FS/stat.md

@@ -39,7 +39,7 @@ A `Promise` that resolves to the [`FSItem`](/Objects/fsitem) object of the speci
 
 The item carries `is_shared`: `true` when it has been shared with someone, `false` when it has not, and `null` when the item is not yours — whether someone else's file has other recipients is not yours to see. It covers shares granted by anyone holding `manage` on the item, not only your own, the same way [`getShares()`](/FS/getShares/) does. Only shares **on the item itself** count. A file inside a folder you shared is reachable through that folder without being shared itself, so it reports `false`; `getShares()` is what reports inherited access.
 
-With `returnShares: true`, the result also carries `shares` — an array of the same share objects [`getShares()`](/FS/getShares/) returns, including access inherited from a parent folder and unclaimed invitations. It is empty unless you own the item or hold `manage` on it, so asking for it never fails a `stat()` you were otherwise allowed to make. Because it does `getShares()`'s work, it also spends from the [share-read limit](/rate-limits-and-quotas/#sharing) on top of `stat()`'s own.
+With `returnShares: true`, the result also carries `shares` — an array of the same share objects [`getShares()`](/FS/getShares/) returns, including access inherited from a parent folder and unclaimed invitations. It is empty unless you own the item or hold `manage` on it, so asking for it never fails a `stat()` you were otherwise allowed to make. An app or API token sees only the shares it issued itself, and an invitation's `recipientEmail` is withheld from everyone but the owner and its sender — see [`getShares()`](/FS/getShares/). Because it does `getShares()`'s work, it also spends from the [share-read limit](/rate-limits-and-quotas/#sharing) on top of `stat()`'s own.
 
 ## Examples
 

src/gui/src/UI/Dashboard/UIShareModal.js

@@ -326,8 +326,10 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
             : '';
         // Only direct grants live on the items themselves; an inherited one
         // belongs to the ancestor folder and has to be changed there.
-        const can_change = group.directPaths.length > 0;
-        const can_revoke = can_change || group.pendingPaths.length > 0;
+        // Every action addresses a person by name; a withheld invite has none.
+        const can_change = group.directPaths.length > 0 && ! group.anonymous;
+        const can_revoke = ! group.anonymous
+            && (can_change || group.pendingPaths.length > 0);
         // Extending someone needs a mode to extend; a person whose grants
         // disagree levels them with the select first.
         const missing = missingPathsFor(target_paths, group).length;

src/gui/src/UI/Dashboard/shareAggregate.js

@@ -97,6 +97,15 @@ const identify = (share, bucket) => {
     const name = bucket === 'pending'
         ? (share.recipientEmail ?? '')
         : (share.holder ?? '');
+    // Withheld address: still reaches someone, and has no name to group on.
+    if ( name === '' && bucket === 'pending' ) {
+        return {
+            key: `invite:${share.uid}`,
+            name: i18n('share_invited_someone'),
+            teamUid: null,
+            anonymous: true,
+        };
+    }
     if ( name === '' ) return null;
     return {
         key: `${bucket === 'pending' ? 'invite' : 'user'}:${name}`,
@@ -129,7 +138,7 @@ export const aggregateShares = (paths, sharesByPath) => {
             const bucket = bucket_of(share);
             const identity = identify(share, bucket);
             if ( ! identity ) continue;
-            const { key, name, teamUid } = identity;
+            const { key, name, teamUid, anonymous } = identity;
 
             if ( counted.has(`${key}|${bucket}`) ) continue;
             counted.add(`${key}|${bucket}`);
@@ -139,6 +148,7 @@ export const aggregateShares = (paths, sharesByPath) => {
                     key,
                     name,
                     teamUid,
+                    anonymous: anonymous === true,
                     pending: bucket === 'pending',
                     directPaths: [],
                     pendingPaths: [],
@@ -175,6 +185,7 @@ export const aggregateShares = (paths, sharesByPath) => {
             key: group.key,
             name: group.name,
             teamUid: group.teamUid,
+            anonymous: group.anonymous,
             pending: group.pending,
             directPaths: group.directPaths,
             pendingPaths: group.pendingPaths,

src/gui/src/UI/Dashboard/shareAggregate.test.js

@@ -84,6 +84,26 @@ describe('aggregateShares', () => {
         expect(groups[1].directPaths).toEqual(['/me/b']);
     });
 
+    it('keeps an invitation whose address was withheld, unnamed and unactionable', () => {
+        // Hiding the row would under-report who reaches the item.
+        const groups = aggregateShares(['/me/a', '/me/b'], new Map([
+            ['/me/a', [grant(null, 'read', { pending: true, uid: 's1' })]],
+            ['/me/b', [grant(null, 'read', { pending: true, uid: 's2' })]],
+        ]));
+
+        // Two invites, not one row folded together on the empty name.
+        expect(groups).toHaveLength(2);
+        expect(groups.map((g) => g.key)).toEqual(['invite:s1', 'invite:s2']);
+        for ( const group of groups ) {
+            expect(group).toMatchObject({
+                anonymous: true,
+                pending: true,
+                name: 'share_invited_someone',
+                accessCount: 1,
+            });
+        }
+    });
+
     it('counts an item once when two grants on it name the same person', () => {
         // Two holders can grant the same access; the item is still one item.
         const groups = aggregateShares(['/me/a'], new Map([

src/gui/src/UI/UIWindowShare.js

@@ -224,12 +224,15 @@ async function UIWindowShare (options) {
                 continue;
             }
             if ( share.pending ) {
+                // Withheld unless the invite is ours to act on: nobody to name,
+                // and the server would refuse the cancellation anyway.
                 const invited = html_encode(share.recipientEmail ?? '');
                 rows += '<div class="share-row share-row-pending">';
-                rows += `<span class="share-row-who">${invited}</span>`;
+                rows += `<span class="share-row-who">${invited || html_encode(i18n('share_invited_someone'))}</span>`;
                 rows += `<span class="share-row-via">${i18n('share_awaiting_signup')}</span>`;
                 rows += `<span class="share-row-mode">${mode_label(share.mode)}</span>`;
-                rows += `<button class="share-revoke" data-holder="${invited}" title="${html_encode(i18n('share_cancel_invite'))}" aria-label="${html_encode(i18n('share_cancel_invite'))}">${icons.trash}</button>`;
+                if ( invited )
+                    rows += `<button class="share-revoke" data-holder="${invited}" title="${html_encode(i18n('share_cancel_invite'))}" aria-label="${html_encode(i18n('share_cancel_invite'))}">${icons.trash}</button>`;
                 rows += '</div>';
                 continue;
             }

src/gui/src/i18n/translations/en.js

@@ -444,6 +444,7 @@ const en = {
         share_access_updated: 'Updated access for {{recipient}}',
         share_invited: 'Invited {{recipient}} — they’ll get access once they join',
         share_awaiting_signup: 'Invited',
+        share_invited_someone: 'Someone by email',
         share_cancel_invite: 'Cancel invitation',
         share_cancel_invite_for: 'Cancel the invitation to {{recipient}}',
         share_confirm_cancel_invite: