From 70edef561fd3e4255721cad8670e649ebe6f67ff Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Wed, 26 Aug 2026 11:47:22 -0400 Subject: [PATCH] Stop offering an access level a delegate cannot grant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A recipient given "can edit & share" could not pass that level on: the dialog offered it, the server refused it, and the refusal was a bare 403 Forbidden that reads as a bug. Handing out manage needs authority over manage, which only the owner has — the refusal is right, the dead end and the silence were not. The dropdown now withholds it from anyone who does not own the item; a row already set to it keeps it, so opening the dialog cannot downgrade the owner's own grant, and a mixed selection follows its strictest item. The server says why, and only to someone who can already share the item — a stranger still gets the ACL's own safe error, which does not admit the node exists. Verified against a running server: a delegate grants read and write as before, and manage now answers cannot_delegate_manage with a sentence naming the owner as the one who can. --- .../services/share/ShareService.test.ts | 30 ++++++++++++++++++- src/backend/services/share/ShareService.ts | 16 ++++++++++ src/gui/src/UI/Dashboard/UIShareModal.js | 8 +++-- src/gui/src/UI/UIWindowShare.js | 8 +++-- src/gui/src/helpers/share_modes.js | 10 +++++-- src/gui/src/helpers/share_modes.test.js | 13 ++++++++ 6 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/backend/services/share/ShareService.test.ts b/src/backend/services/share/ShareService.test.ts index 759278c24..5a6089f01 100644 --- a/src/backend/services/share/ShareService.test.ts +++ b/src/backend/services/share/ShareService.test.ts @@ -806,7 +806,35 @@ describe('ShareService', () => { recipient: { email: third.email }, mode: 'manage', }), - ).rejects.toMatchObject({ statusCode: 403 }); + ).rejects.toMatchObject({ + statusCode: 403, + legacyCode: 'cannot_delegate_manage', + }); + + // What they can do is unchanged. + await expect( + share(delegate.actor, { + uid: file.uuid, + recipient: { email: third.email }, + mode: 'write', + }), + ).resolves.toMatchObject({ mode: 'write' }); + }); + + it('tells a stranger nothing when they ask to grant `manage`', async () => { + const owner = await makeUser(); + const stranger = await makeUser(); + const third = await makeUser(); + const file = await makeFile(owner.user); + + // No access at all, so the refusal must not confirm the file exists. + await expect( + share(stranger.actor, { + uid: file.uuid, + recipient: { email: third.email }, + mode: 'manage', + }), + ).rejects.not.toMatchObject({ legacyCode: 'cannot_delegate_manage' }); }); it('leaves a delegate alone when their authority survives another issuer', async () => { diff --git a/src/backend/services/share/ShareService.ts b/src/backend/services/share/ShareService.ts index 9e6ce17da..6327ae06d 100644 --- a/src/backend/services/share/ShareService.ts +++ b/src/backend/services/share/ShareService.ts @@ -1720,6 +1720,22 @@ export class ShareService extends PuterService { // given, rather than everything its user owns. if (allowed && (await this.#hasOwnReach(actor, entry, mode))) return; + // Only for someone who can already share here, so it leaks nothing. + if (mode === MANAGE_PERM_PREFIX) { + const canDelegateAccess = + await this.services.permission.canManagePermission( + userRelatedActor(actor), + entryPermissionForMode(entry.uuid, 'write'), + ); + if (canDelegateAccess) { + throw new HttpError( + 403, + 'Only the owner can grant edit & share access', + { legacyCode: 'cannot_delegate_manage' }, + ); + } + } + const safe = await this.services.acl.getSafeAclError( actor, this.#descriptorFor(entry), diff --git a/src/gui/src/UI/Dashboard/UIShareModal.js b/src/gui/src/UI/Dashboard/UIShareModal.js index 65e4217b8..9e84a55b2 100644 --- a/src/gui/src/UI/Dashboard/UIShareModal.js +++ b/src/gui/src/UI/Dashboard/UIShareModal.js @@ -19,7 +19,7 @@ import path from '../../lib/path.js'; import item_icon from '../../helpers/item_icon.js'; -import { owner_of_path } from '../../helpers/path_owner.js'; +import { is_owned_by_me, owner_of_path } from '../../helpers/path_owner.js'; import { invalidate_shared_roots } from '../../helpers/shared_access.js'; import { icons } from '../../helpers/actionIcons.js'; import { mode_label, options_for } from '../../helpers/share_modes.js'; @@ -115,6 +115,8 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse })); const target_paths = targets.map((item) => item.path); const total = targets.length; + // Strictest item decides: one borrowed item withholds it for the rest. + const allow_manage = target_paths.every((p) => is_owned_by_me(p)); const is_multi = total > 1; // Nothing to share: an empty selection is a caller's mistake, not a dialog. if ( total === 0 ) return { close: () => {} }; @@ -159,7 +161,7 @@ export default function UIShareModal ({ items, path: item_path, name, owner, fse
- +
`; @@ -143,7 +145,7 @@ async function UIWindowShare (options) { } rows += '
'; rows += ``; - rows += ``; + rows += ``; rows += ``; rows += '
'; } diff --git a/src/gui/src/helpers/share_modes.js b/src/gui/src/helpers/share_modes.js index 8415da8df..1c4d7f1f8 100644 --- a/src/gui/src/helpers/share_modes.js +++ b/src/gui/src/helpers/share_modes.js @@ -48,11 +48,17 @@ export const mode_label = (mode) => { * unselectable placeholder, so a batch of mixed modes can't be read as one of * them, and picking a real mode is what levels them. * + * `allow_manage: false` drops "Can edit & share", bar a row already on it. + * * @param {string|null} current + * @param {{ allow_manage?: boolean }} [options] * @returns {string} HTML-safe markup */ -export const options_for = (current) => { - const listed = MODES +export const options_for = (current, { allow_manage = true } = {}) => { + const offered = MODES.filter( + (mode) => allow_manage || mode !== 'manage' || mode === current, + ); + const listed = offered .map( (mode) => ``, diff --git a/src/gui/src/helpers/share_modes.test.js b/src/gui/src/helpers/share_modes.test.js index a55fc554b..b693b356f 100644 --- a/src/gui/src/helpers/share_modes.test.js +++ b/src/gui/src/helpers/share_modes.test.js @@ -57,6 +57,19 @@ describe('options_for', () => { expect(html).not.toContain('