mirror of
https://github.com/HeyPuter/puter.git
synced 2026-09-30 09:06:37 +00:00
fix: stop disclosing invite addresses to apps, tokens and delegates
getShares (and stat's return_shares, which runs the same listing) gated on #assertCanManage's default 'see' mode, so any credential that could see the node got every unclaimed invite's raw email — including an app handed one file by the picker, a list-scoped token on the stat surface, and a manage delegate reading the owner's invitees. Two bounds, matching the invariant clientShare.ts already claimed: - An invite's address goes only to the item's owner and to whoever sent it, and never to an app or token. A delegate can revoke only what they issued, so withholding costs them nothing they could act on. - An app or token must hold manage reach of its own to read the listing at all; it answers for the ancestors too, which is not what being handed one file grants. tryListSharesOf turns that into an empty shares array, so stat itself keeps working. The share dialog names an unattributable invite rather than rendering a blank row with a dead revoke button. Closes PUT-1806.
This commit is contained in:
@@ -1069,4 +1069,123 @@ 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);
|
||||
|
||||
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).not.toBe(200);
|
||||
expect(await peeked.text()).not.toContain(email);
|
||||
|
||||
// `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('refuses a list-scoped access token on the `return_shares` surface', 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([]);
|
||||
});
|
||||
|
||||
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();
|
||||
// The invite is listed; the address it names is not.
|
||||
expect(JSON.parse(text).items.some((i: { pending?: boolean }) => i.pending)).toBe(true);
|
||||
expect(text).not.toContain(email);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 } : {}),
|
||||
|
||||
@@ -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;
|
||||
@@ -4868,4 +4869,112 @@ 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('refuses the listing to an app holding only the file', async () => {
|
||||
const { owner, file, email } = await withInvite();
|
||||
const app = await makeApp(owner.user.id);
|
||||
await grantAppReach(owner, app, file, 'read');
|
||||
|
||||
// The file it was given, not the company it keeps.
|
||||
const refused = await listOf(asApp(owner, app), file.uuid).then(
|
||||
() => null,
|
||||
(err: { statusCode?: number }) => err,
|
||||
);
|
||||
expect([403, 404]).toContain(refused?.statusCode);
|
||||
// `return_shares` rides this, and answers an empty list instead.
|
||||
expect(
|
||||
await server.services.share.tryListSharesOf(asApp(owner, app), {
|
||||
uid: file.uuid,
|
||||
}),
|
||||
).toBeNull();
|
||||
expect(JSON.stringify(refused)).not.toContain(email);
|
||||
});
|
||||
|
||||
it('still answers an app that holds manage on the item', 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}`,
|
||||
),
|
||||
);
|
||||
|
||||
const listed = await listOf(asApp(owner, app), file.uuid);
|
||||
expect(listed.some((row) => row.pending)).toBe(true);
|
||||
// Manage or not, an address is not an app's to read.
|
||||
expect(JSON.stringify(listed)).not.toContain(email);
|
||||
});
|
||||
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2064,6 +2064,11 @@ export class ShareService extends PuterService {
|
||||
): Promise<ResolvedShare[]> {
|
||||
const entry = await this.#resolveEntry(target, actor);
|
||||
await this.#assertCanManage(actor, entry);
|
||||
// A management view, answering for the ancestors too: an app or token
|
||||
// handed one file at `see` needs `manage` reach of its own.
|
||||
if (!(await this.#hasOwnReach(actor, entry, MANAGE_PERM_PREFIX))) {
|
||||
throw await this.#manageRefusal(actor, entry);
|
||||
}
|
||||
|
||||
// Access is inherited down the tree, so a node's own rows are only
|
||||
// half the answer — without the ancestors' the caller is told nobody
|
||||
@@ -2170,6 +2175,7 @@ export class ShareService extends PuterService {
|
||||
(row: OutboundShareRow) =>
|
||||
this.#resolvedShareRow(row, entry, users, {
|
||||
path: maskedPath,
|
||||
inviteAddress: this.#maySeeInviteAddress(actor, entry, row),
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -2886,6 +2892,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;
|
||||
@@ -2935,9 +2946,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 } : {}),
|
||||
@@ -3228,16 +3246,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 (actor.app || actor.accessToken) 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
|
||||
|
||||
@@ -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
|
||||
@@ -48,6 +50,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
|
||||
|
||||
@@ -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.
|
||||
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 gets an empty array unless the credential itself holds `manage`, and an invitation's `recipientEmail` is withheld from everyone but the owner and its sender — see [`getShares()`](/FS/getShares/).
|
||||
|
||||
## Examples
|
||||
|
||||
|
||||
@@ -217,12 +217,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;
|
||||
}
|
||||
|
||||
@@ -437,6 +437,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:
|
||||
|
||||
Reference in New Issue
Block a user