From 3e3d02bafee376640ca2fc3331e16d51532f0233 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Wed, 16 Sep 2026 18:59:10 -0400 Subject: [PATCH 1/4] fix: stop disclosing invite addresses to apps, tokens and delegates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../share/ShareController.http.test.ts | 119 ++++++++++++++++++ src/backend/controllers/share/clientShare.ts | 8 +- .../services/share/ShareService.test.ts | 109 ++++++++++++++++ src/backend/services/share/ShareService.ts | 49 +++++++- src/docs/src/FS/getShares.md | 4 + src/docs/src/FS/stat.md | 2 +- src/gui/src/UI/UIWindowShare.js | 7 +- src/gui/src/i18n/translations/en.js | 1 + 8 files changed, 291 insertions(+), 8 deletions(-) diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index 1cdde789a..a240560a9 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/src/backend/controllers/share/ShareController.http.test.ts @@ -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); + }); + }); }); diff --git a/src/backend/controllers/share/clientShare.ts b/src/backend/controllers/share/clientShare.ts index 0a9a61cce..b171fc21d 100644 --- a/src/backend/controllers/share/clientShare.ts +++ b/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 } : {}), diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 37e35102f..071736ffb 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/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; @@ -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); + }); + + }); }); diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index b4d1eb2d7..0b6c936e8 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -2064,6 +2064,11 @@ export class ShareService extends PuterService { ): Promise { 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 { 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 diff --git a/src/docs/src/FS/getShares.md b/src/docs/src/FS/getShares.md index cc65f4554..547c998b2 100644 --- a/src/docs/src/FS/getShares.md +++ b/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 @@ -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 diff --git a/src/docs/src/FS/stat.md b/src/docs/src/FS/stat.md index 5f845c062..6fdd46021 100755 --- a/src/docs/src/FS/stat.md +++ b/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. +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 diff --git a/src/gui/src/UI/UIWindowShare.js b/src/gui/src/UI/UIWindowShare.js index ab65908de..22852e87a 100644 --- a/src/gui/src/UI/UIWindowShare.js +++ b/src/gui/src/UI/UIWindowShare.js @@ -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 += ''; continue; } diff --git a/src/gui/src/i18n/translations/en.js b/src/gui/src/i18n/translations/en.js index 57fe3a89f..5ccaaf52b 100644 --- a/src/gui/src/i18n/translations/en.js +++ b/src/gui/src/i18n/translations/en.js @@ -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: From 461cb7d0a972eb236cb11aef682dbf51daf9ae0c Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Thu, 17 Sep 2026 13:21:13 -0400 Subject: [PATCH 2/4] fix: close two gaps the review found in the invite-address fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A full-access token was denied the address while `shared-by-me` still handed it the same rows, so the clause bought no privacy and cost an API client the address `unshare()` takes. `isAccountContext` is the boundary the rest of the codebase already uses for 'acting as the account': plain session or full-access token, never a scoped one. - The Dashboard share modal dropped a withheld invite entirely, since its aggregate keys a pending row on the address — so a delegate saw no sign of an outstanding invite and accessCount under-reported who could reach the item. It is now kept, keyed on the share uid, labelled, and without the controls that would need a recipient to address. --- .../share/ShareController.http.test.ts | 25 +++++++++++++++++++ src/backend/services/share/ShareService.ts | 9 +++++-- src/gui/src/UI/Dashboard/UIShareModal.js | 6 +++-- src/gui/src/UI/Dashboard/shareAggregate.js | 13 +++++++++- .../src/UI/Dashboard/shareAggregate.test.js | 20 +++++++++++++++ 5 files changed, 68 insertions(+), 5 deletions(-) diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index a240560a9..b93484811 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/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 @@ -1169,6 +1170,30 @@ describe('share endpoints over HTTP', () => { 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); diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index 0b6c936e8..4ac5b5869 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -19,7 +19,12 @@ import { contentType as contentTypeFromMime } from 'mime-types'; import { posix as pathPosix } from 'node:path'; -import { makeActor, userRelatedActor, type Actor } from '../../core/actor'; +import { + isAccountContext, + makeActor, + userRelatedActor, + type Actor, +} from '../../core/actor'; import { HttpError, isHttpError } from '../../core/http/HttpError.js'; import { runWithConcurrencyLimitSettled } from '../../util/concurrency.js'; import { isUniqueViolation } from '../../util/dbError.js'; @@ -3270,7 +3275,7 @@ export class ShareService extends PuterService { entry: FSEntry, row: OutboundShareRow, ): boolean { - if (actor.app || actor.accessToken) return false; + if (!isAccountContext(actor)) return false; const userId = actor.user?.id; if (typeof userId !== 'number') return false; return ( diff --git a/src/gui/src/UI/Dashboard/UIShareModal.js b/src/gui/src/UI/Dashboard/UIShareModal.js index 453d7528b..d4802ec72 100644 --- a/src/gui/src/UI/Dashboard/UIShareModal.js +++ b/src/gui/src/UI/Dashboard/UIShareModal.js @@ -324,8 +324,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; diff --git a/src/gui/src/UI/Dashboard/shareAggregate.js b/src/gui/src/UI/Dashboard/shareAggregate.js index 98248dfae..c68ba82ea 100644 --- a/src/gui/src/UI/Dashboard/shareAggregate.js +++ b/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, diff --git a/src/gui/src/UI/Dashboard/shareAggregate.test.js b/src/gui/src/UI/Dashboard/shareAggregate.test.js index 1b2790ae8..5db68aa6f 100644 --- a/src/gui/src/UI/Dashboard/shareAggregate.test.js +++ b/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([ From 72203152d9043f68eac47df98376c004c8651c47 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Mon, 21 Sep 2026 11:12:37 -0400 Subject: [PATCH 3/4] fix: bound a scoped token to what it issued, which is nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught three HTTP-level tests the earlier merge left behind, and they were right to fail: dropping this branch's `manage` gate in favour of main's row filter lost a case main's filter does not cover. Main bounds an app to the rows it issued. A *scoped* access token is not an app, so it was falling through unbounded — `/fs/stat` with `return_shares` handed a `fs::list` token the owner's whole share list. Addresses stayed withheld, but who else can reach a file is no more a narrow token's business than the addresses are. So the filter is generalised rather than the gate restored: a scoped token is bounded to nothing, since it issues nothing under its own name. Filtering it by a null app would have been worse than not filtering — that matches the owner's own rows. Apps and sessions behave exactly as they do on main, and a full-access token still holds the account's reach. The three tests now assert the answer instead of a refusal, including the one whose name had always promised a refusal its body never checked. --- package-lock.json | 1 + .../share/ShareController.http.test.ts | 19 ++++++++++++++----- src/backend/services/share/ShareService.ts | 17 +++++++++++++---- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index b37dd8a59..7ca0a1796 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1216,6 +1216,7 @@ }, "node_modules/@clack/prompts/node_modules/is-unicode-supported": { "version": "1.3.0", + "extraneous": true, "inBundle": true, "license": "MIT", "engines": { diff --git a/src/backend/controllers/share/ShareController.http.test.ts b/src/backend/controllers/share/ShareController.http.test.ts index b93484811..047aacd91 100644 --- a/src/backend/controllers/share/ShareController.http.test.ts +++ b/src/backend/controllers/share/ShareController.http.test.ts @@ -1127,12 +1127,16 @@ describe('share endpoints over HTTP', () => { 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).not.toBe(200); - expect(await peeked.text()).not.toContain(email); + 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, { @@ -1147,7 +1151,7 @@ describe('share endpoints over HTTP', () => { // 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 () => { + 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( @@ -1208,8 +1212,13 @@ describe('share endpoints over HTTP', () => { }); 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); + // `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); }); }); diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index e7d751df5..6dc9d1908 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -2208,10 +2208,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 = (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 = (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( From 18ada4764ab8b8aa66698ff73e7267cbccdd4d22 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Mon, 21 Sep 2026 11:26:27 -0400 Subject: [PATCH 4/4] chore: drop a stray lockfile marker A local `npm install` (needed to pick up main's new `postal-mime`) wrote `"extraneous": true` onto a bundled `@clack/prompts` dependency. It was the branch's only lockfile change and it does not belong to this PR. --- package-lock.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 7ca0a1796..b37dd8a59 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1216,7 +1216,6 @@ }, "node_modules/@clack/prompts/node_modules/is-unicode-supported": { "version": "1.3.0", - "extraneous": true, "inBundle": true, "license": "MIT", "engines": {