fix: show optional permissions in the marketplace, and ask before an update adds a permission

This commit is contained in:
Linus Rath
2026-11-04 03:23:46 +01:00
parent 9b61b79d5c
commit 35a4b034b1
9 changed files with 221 additions and 18 deletions
@@ -49,6 +49,36 @@ describe('installVersion', () => {
expect(calls).toHaveLength(1);
});
it('says what an update adds, and repeats with what the admin saw', async () => {
responses = [json({ error: 'adds', permissionsAdded: ['keystore'] }, 409), json({ warnings: [] })];
const asked: string[] = [];
const result = await installVersion(BODY, (q) => { asked.push(`${q.title} ${q.message}`); return true; });
expect(asked[0]).toContain('new permissions');
expect(asked[0]).toContain('keystore');
expect(calls.map((c) => c.body.acceptPermissions)).toEqual([undefined, ['keystore']]);
expect(calls.map((c) => c.body.force), 'not the same as forcing it').toEqual([undefined, undefined]);
expect(result.ok).toBe(true);
});
it('does not update past new permissions the admin declined', async () => {
responses = [json({ error: 'adds', permissionsAdded: ['keystore'] }, 409)];
expect(await installVersion(BODY, () => false)).toEqual({ ok: false, error: 'Not installed.' });
expect(calls).toHaveLength(1);
});
it('asks about both when a withdrawn version also adds permissions, and keeps both answers', async () => {
responses = [
json({ error: 'withdrawn', yanked: { reason: null } }, 409),
json({ error: 'adds', permissionsAdded: ['keystore'] }, 409),
json({ warnings: [] }),
];
const asked: string[] = [];
const result = await installVersion(BODY, (q) => { asked.push(q.title); return true; });
expect(asked).toHaveLength(2);
expect(calls.at(-1)!.body).toMatchObject({ force: true, acceptPermissions: ['keystore'] });
expect(result.ok).toBe(true);
});
it('shows any other 409 as the error it is', async () => {
responses = [json({ error: 'The configuration is read-only' }, 409)];
const result = await installVersion(BODY, () => true);
@@ -28,6 +28,7 @@ import { apiFetch } from '@/lib/browser-navigation';
import { compareVersions, isVersionSatisfied } from '@/lib/version-compare';
import type { MarketplaceListing } from '@/lib/admin/marketplace-directory';
import { adminPath } from '@/lib/admin/admin-nav';
import { declaredPermissions, HIGH_RISK_PERMISSIONS } from '@/lib/plugin-sandbox/permission-text';
import { cn } from '@/lib/utils';
import { SettingsSection } from '@/components/settings/settings-section';
import { Button } from '@/components/ui/button';
@@ -97,6 +98,8 @@ interface PreviewData {
requestedVersionMissing: string | null;
bundle: {
manifest: Record<string, unknown> | null;
/** What this version asks for that the installed one doesn't. */
addedPermissions?: string[];
source: { name: string; content: string; truncated: boolean } | null;
size: number;
error: string | null;
@@ -307,8 +310,10 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) {
const bundle = data.bundle;
const preview = data.preview;
const isPlugin = ext.type === 'plugin';
const manifestPerms = (bundle.manifest?.permissions as string[] | undefined) || preview && ext.versions.find((v) => v.version === preview.version)?.permissions || ext.permissions || [];
// Optional permissions are granted like the others, so they count here too.
const manifestPerms = bundle.manifest ? declaredPermissions(bundle.manifest) : (preview && ext.versions.find((v) => v.version === preview.version)?.permissions) || ext.permissions || [];
const permissionText = new Map(ext.permissionDetails.map((p) => [p.id, p]));
const addedPermissions = new Set(bundle.addedPermissions ?? []);
const frameOrigins = (bundle.manifest?.frameOrigins as string[] | undefined) || [];
const settingsSchema = bundle.manifest?.settingsSchema as Record<string, { type: string; label: string; description?: string; default?: unknown }> | undefined;
@@ -705,7 +710,7 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) {
<ul className="space-y-1.5">
{manifestPerms.map(perm => {
const text = permissionText.get(perm);
const risky = text?.highRisk || RISKY_PERMISSIONS.has(perm);
const risky = text?.highRisk || RISKY_PERMISSIONS.has(perm) || HIGH_RISK_PERMISSIONS.has(perm);
return (
<li
key={perm}
@@ -721,6 +726,7 @@ export function MarketplaceDetailPage({ slug }: { slug: string }) {
<div className="flex flex-wrap items-center gap-2">
{text && <span className="font-medium">{text.title}</span>}
<code className="font-mono text-xs opacity-80">{perm}</code>
{addedPermissions.has(perm) && <Badge tone="warning">New in this version</Badge>}
</div>
{text?.body && <p className="mt-0.5 text-xs opacity-90">{text.body}</p>}
</div>
+28 -9
View File
@@ -45,6 +45,7 @@ interface InstalledEntry {
interface UpdatesResult {
installed: InstalledEntry[];
updates: Array<{
addedPermissions?: string[];
slug: string; currentVersion: string; version: string; channel: 'stable' | 'beta'; publishedAt: string | null;
changelog: string | null; notes?: ChangelogFlags; changelogs?: ChangelogEntry[];
}>;
@@ -95,7 +96,9 @@ const pillClass = (active: boolean) => cn(
* Installs one version of an extension. A version the publisher withdrew,
* or one older than the installed version, comes back as 409; the admin is
* asked (in the page's ConfirmDialog), and the install is repeated with
* `force` if they insist. Returns the error text to show, or null.
* `force` if they insist. An update that asks for more permissions comes back
* the same way, and is repeated with the ones the admin saw. "Update all" goes
* through here too, so it asks about each. Returns the error text to show, or null.
*/
export interface InstallQuestion {
title: string;
@@ -108,28 +111,39 @@ export async function installVersion(
body: { slug: string; version: string; type: 'plugin' | 'theme' },
confirm: (question: InstallQuestion) => boolean | Promise<boolean>,
): Promise<{ ok: true; warnings: string[]; signature: string | null; updated: boolean } | { ok: false; error: string }> {
const send = (force: boolean) => apiFetch('/api/admin/marketplace', {
let force = false;
let acceptPermissions: string[] | undefined;
const send = () => apiFetch('/api/admin/marketplace', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(force ? { ...body, force: true } : body),
body: JSON.stringify({ ...body, ...(force ? { force: true } : {}), ...(acceptPermissions ? { acceptPermissions } : {}) }),
});
try {
let res = await send(false);
if (res.status === 409) {
let res = await send();
// At most one of each question: withdrawn or older, then permissions.
for (let asked = 0; res.status === 409 && asked < 3; asked += 1) {
const data = await res.json().catch(() => ({}));
if (data.yanked) {
if (Array.isArray(data.permissionsAdded) && !acceptPermissions) {
const names = (data.permissionsAdded as string[]).join(', ');
const message = `${body.slug} ${body.version} asks for new permissions that the installed version doesn't have: ${names}.`;
if (!(await confirm({ title: 'Allow the new permissions?', message, confirmText: 'Update', cancelText: 'Cancel' }))) {
return { ok: false, error: 'Not installed.' };
}
acceptPermissions = data.permissionsAdded;
} else if (data.yanked && !force) {
if (!(await confirm({ title: 'Install it anyway?', message: data.error, confirmText: 'Install', cancelText: 'Cancel' }))) {
return { ok: false, error: 'Not installed.' };
}
res = await send(true);
} else if (data.downgrade) {
force = true;
} else if (data.downgrade && !force) {
if (!(await confirm({ title: 'Install the older version anyway?', message: data.error, confirmText: 'Install', cancelText: 'Cancel' }))) {
return { ok: false, error: 'Not installed.' };
}
res = await send(true);
force = true;
} else {
return { ok: false, error: data.error || 'Installation failed' };
}
res = await send();
}
const data = await res.json().catch(() => ({}));
if (!res.ok) return { ok: false, error: data.error || 'Installation failed' };
@@ -463,6 +477,11 @@ function UpdatesSection({
{u.channel === 'beta' && <Badge tone="info">beta</Badge>}
<ChangelogBadges flags={{ security: anyFlag(entries, 'security'), breaking: anyFlag(entries, 'breaking') }} />
</div>
{(u.addedPermissions?.length ?? 0) > 0 && (
<p className="mt-1 text-xs text-amber-800 dark:text-amber-300" data-testid={`update-permissions-${u.slug}`}>
New permissions: <span className="font-mono">{u.addedPermissions!.join(', ')}</span>
</p>
)}
{withNotes.length > 0 ? (
<details className="mt-1" data-testid={`update-notes-${u.slug}`}>
<summary className="cursor-pointer text-xs font-medium text-muted-foreground hover:text-foreground">
+24 -2
View File
@@ -23,12 +23,22 @@ import {
type DirectoryDetail,
type DirectoryVersion,
} from '@/lib/admin/marketplace-directory';
import { MAX_PLUGIN_SIZE, MAX_THEME_SIZE } from '@/lib/plugin-types';
import { IMPLICIT_PERMISSIONS, MAX_PLUGIN_SIZE, MAX_THEME_SIZE } from '@/lib/plugin-types';
import { declaredPermissions, HIGH_RISK_PERMISSIONS } from '@/lib/plugin-sandbox/permission-text';
import { getPluginRegistry } from '@/lib/admin/plugin-registry';
import { productName } from '@/lib/product-name';
import { withAppName } from '@/lib/settings-search';
const MAX_PREVIEW_SOURCE_LEN = 100_000;
/** For a directory that predates it. It only lists what it knows about, and admin pages are in English. */
const LOCAL_PERMISSION_TEXT: Record<string, { title: string; body: string }> = {
keystore: {
title: 'Use certificates on this computer',
body: 'Ask to sign and decrypt messages with certificates on this computer or a smart card. The app asks each user first. The plugin never gets the private key, but while they allow it, it can sign in their name and read what it decrypts.',
},
};
/** A version row as the detail page renders it. */
function versionRow(v: DirectoryVersion) {
return {
@@ -176,6 +186,17 @@ export async function GET(
const state = installedStateOf(installed, slug, type);
const history = await historyFor(type, slug);
const appName = productName(configManager.get<string>('appName'));
// What the bundle itself declares, optional permissions too: the directory's list can leave some out.
const declared = type === 'plugin' ? declaredPermissions(manifest) : [];
const listed = new Set((detail.permissions ?? []).map((p) => p.id));
const unlisted = declared.filter((id) => !listed.has(id) && LOCAL_PERMISSION_TEXT[id]).map((id) => ({
id,
...LOCAL_PERMISSION_TEXT[id],
highRisk: HIGH_RISK_PERMISSIONS.has(id),
}));
const installedPlugin = state.installed && type === 'plugin' ? (await getPluginRegistry()).plugins.find((p) => p.id === slug) : null;
const implicit = new Set<string>(IMPLICIT_PERMISSIONS);
const addedPermissions = installedPlugin ? declared.filter((p) => !installedPlugin.permissions?.includes(p) && !implicit.has(p)) : [];
return NextResponse.json(
{
@@ -189,7 +210,7 @@ export async function GET(
themePreviews: detail.themePreviews ?? [],
advisories: detail.advisories ?? [],
// The directory ships the webmail's permission texts unfilled.
permissionDetails: (detail.permissions ?? []).map((p) => withAppName(p, appName) as typeof p),
permissionDetails: [...(detail.permissions ?? []), ...unlisted].map((p) => withAppName(p, appName) as typeof p),
source: detail.source,
createdAt: detail.createdAt ?? null,
updatedAt: detail.updatedAt ?? null,
@@ -202,6 +223,7 @@ export async function GET(
requestedVersionMissing: requested && !requestedRow ? requested : null,
bundle: {
manifest,
addedPermissions,
source: sourcePreview,
size: bundleSize,
error: bundleError,
@@ -19,7 +19,7 @@ vi.mock('@/lib/admin/config-manager', () => ({
}));
vi.mock('@/lib/admin/plugin-registry', () => ({
getPluginRegistry: vi.fn(async () => ({
plugins: [{ id: 'pretty-dates', name: 'Pretty Dates', version: '1.1.0', enabled: true, source: { kind: 'marketplace', slug: 'pretty-dates', directoryUrl: 'https://extensions.test' } }],
plugins: [{ id: 'pretty-dates', name: 'Pretty Dates', version: '1.1.0', permissions: ['email:read'], enabled: true, source: { kind: 'marketplace', slug: 'pretty-dates', directoryUrl: 'https://extensions.test' } }],
})),
getThemeRegistry: vi.fn(async () => ({ themes: [] })),
}));
@@ -127,3 +127,43 @@ describe('GET /api/admin/marketplace/[slug]', () => {
expect((await preview('?version=..%2F..%2Fetc')).status).toBe(400);
});
});
describe('permissions in the preview', () => {
async function previewOf(manifest: Record<string, unknown>) {
const zip = new JSZip();
zip.file('manifest.json', JSON.stringify({ id: 'pretty-dates', name: 'Pretty Dates', version: '1.2.0', author: 'x', description: 'd', type: 'hook', entrypoint: 'index.js', ...manifest }));
zip.file('index.js', '// x');
const bytes = (await zip.generateAsync({ type: 'uint8array' })).slice().buffer as ArrayBuffer;
const versions = [v('1.2.0')];
vi.stubGlobal('fetch', vi.fn(async (url: string) => {
const u = new URL(String(url));
if (u.pathname === '/api/v2/extensions/pretty-dates') {
return Response.json({ data: { ...summary, latest: versions[0], newest: versions[0], longDescription: 'long', versions, permissions: [{ id: 'crypto:full', title: 'Full crypto', body: 'b', highRisk: true }], screenshots: [], themePreviews: [], advisories: [] } });
}
if (u.pathname.endsWith('/versions')) return Response.json({ data: versions });
if (u.pathname.endsWith('/bundle')) return new Response(bytes);
return Response.json({ data: versions[0] });
}));
const res = await GET(new NextRequest('http://localhost/api/admin/marketplace/pretty-dates'), { params: Promise.resolve({ slug: 'pretty-dates' }) });
return res.json();
}
it('names an optional keystore with its own text and as high risk, even if the directory does not', async () => {
const body = await previewOf({ tier: 'privileged', permissions: ['crypto:full'], optionalPermissions: ['keystore'] });
const entry = body.extension.permissionDetails.find((p: { id: string }) => p.id === 'keystore');
expect(entry).toMatchObject({ highRisk: true });
expect(entry.title).toMatch(/certificates/i);
expect(body.extension.permissionDetails.filter((p: { id: string }) => p.id === 'crypto:full')).toHaveLength(1);
});
it('says what this version adds to the installed one', async () => {
const body = await previewOf({ tier: 'privileged', permissions: ['crypto:full', 'email:read'], optionalPermissions: ['keystore'] });
expect(body.bundle.addedPermissions).toEqual(['crypto:full', 'keystore']);
});
it('adds nothing for an untrusted plugin, which can not get it', async () => {
const body = await previewOf({ permissions: ['email:read'], optionalPermissions: ['keystore'] });
expect(body.extension.permissionDetails.map((p: { id: string }) => p.id)).not.toContain('keystore');
expect(body.bundle.addedPermissions).toEqual([]);
});
});
@@ -215,3 +215,49 @@ describe('POST /api/admin/marketplace', () => {
expect(registry.get('pgp-plugin')?.version).toBe('2.0.0');
});
});
describe('an update that asks for more than the installed version', () => {
const INSTALLED = { id: 'pretty-dates', name: 'Pretty Dates', version: '0.9.0', permissions: ['email:read'], enabled: true };
it('is refused until someone has seen what it adds, and installs nothing meanwhile', async () => {
registry.set('pretty-dates', { ...INSTALLED });
directory(await bundle({ ...MANIFEST, id: 'pretty-dates', permissions: ['email:read', 'email:send'] }));
const res = await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin' });
expect(res.status).toBe(409);
expect((await res.json()).permissionsAdded).toEqual(['email:send']);
expect(registry.get('pretty-dates')!.version).toBe('0.9.0');
});
it('counts an optional permission, since it is granted like any other', async () => {
registry.set('pretty-dates', { ...INSTALLED });
directory(await bundle({ ...MANIFEST, id: 'pretty-dates', optionalPermissions: ['email:send'] }));
const res = await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin' });
expect(res.status).toBe(409);
expect((await res.json()).permissionsAdded).toEqual(['email:send']);
});
it('forcing it is about something else, and does not get past this', async () => {
registry.set('pretty-dates', { ...INSTALLED });
directory(await bundle({ ...MANIFEST, id: 'pretty-dates', permissions: ['email:read', 'email:send'] }));
expect((await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin', force: true })).status).toBe(409);
});
it('goes through when what was accepted covers it, and only then', async () => {
registry.set('pretty-dates', { ...INSTALLED });
directory(await bundle({ ...MANIFEST, id: 'pretty-dates', permissions: ['email:read', 'email:send', 'email:raw-send'] }));
expect((await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin', acceptPermissions: ['email:send'] })).status).toBe(409);
expect((await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin', acceptPermissions: 'email:send,email:raw-send' })).status).toBe(409);
const res = await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin', acceptPermissions: ['email:send', 'email:raw-send'] });
expect(res.status).toBe(200);
expect(registry.get('pretty-dates')!.permissions).toEqual(['email:read', 'email:send', 'email:raw-send']);
});
it('asks nothing when nothing is added, or on a first install', async () => {
registry.set('pretty-dates', { ...INSTALLED });
directory(await bundle({ ...MANIFEST, id: 'pretty-dates' }));
expect((await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin' })).status).toBe(200);
registry.clear();
directory(await bundle({ ...MANIFEST, id: 'pretty-dates', permissions: ['email:read', 'email:send'] }));
expect((await install({ slug: 'pretty-dates', version: '1.0.0', type: 'plugin' })).status).toBe(200);
});
});
@@ -17,7 +17,7 @@ vi.mock('@/lib/admin/config-manager', () => ({
vi.mock('@/lib/admin/plugin-registry', () => ({
getPluginRegistry: vi.fn(async () => ({
plugins: [
{ id: 'quick-reply', name: 'Quick Reply', version: '1.2.0', enabled: true, source: { kind: 'marketplace', slug: 'quick-reply', directoryUrl: 'https://extensions.test' } },
{ id: 'quick-reply', name: 'Quick Reply', version: '1.2.0', permissions: ['email:read'], enabled: true, source: { kind: 'marketplace', slug: 'quick-reply', directoryUrl: 'https://extensions.test' } },
{ id: 'house-plugin', name: 'House', version: '0.1.0', enabled: true, source: { kind: 'upload' } },
// Installed before the source was recorded: still asked about.
{ id: 'old-install', name: 'Old', version: '3.0.0', enabled: false },
@@ -34,6 +34,21 @@ import { GET } from '../updates/route';
afterEach(() => vi.unstubAllGlobals());
describe('GET /api/admin/marketplace/updates', () => {
it('says which permissions each update adds', async () => {
vi.stubGlobal('fetch', vi.fn(async () => Response.json({
data: {
updates: [
{ slug: 'quick-reply', currentVersion: '1.2.0', version: '1.3.0', channel: 'stable', changelog: null, permissions: ['email:read', 'keystore'] },
{ slug: 'old-install', currentVersion: '3.0.0', version: '3.1.0', channel: 'stable', changelog: null },
],
yanked: [], deprecated: [], suspended: [], advisories: [], unknown: [],
},
})));
const body = await (await GET(new NextRequest('http://localhost/api/admin/marketplace/updates'))).json();
expect(body.updates[0].addedPermissions).toEqual(['keystore']);
expect(body.updates[1].addedPermissions).toEqual([]);
});
it('asks the directory about the marketplace installs only, on the configured channel', async () => {
let posted: { url: string; body: unknown } | null = null;
vi.stubGlobal('fetch', vi.fn(async (url: string, init: RequestInit) => {
+17 -2
View File
@@ -42,7 +42,7 @@ import {
type DirectorySummary,
type DirectoryVersion,
} from '@/lib/admin/marketplace-directory';
import { MAX_PLUGIN_SIZE, MAX_THEME_SIZE } from '@/lib/plugin-types';
import { MAX_PLUGIN_SIZE, MAX_THEME_SIZE, IMPLICIT_PERMISSIONS } from '@/lib/plugin-types';
import { createHash } from 'node:crypto';
import { devicePolicyPluginRefusal } from '@/lib/desktop/server-policy';
@@ -116,7 +116,7 @@ export async function POST(request: NextRequest) {
}
const ip = getClientIP(request);
const { slug, version, type, force } = await request.json();
const { slug, version, type, force, acceptPermissions } = await request.json();
if (!slug || !version || !type) {
return NextResponse.json({ error: 'Missing slug, version, or type' }, { status: 400 });
@@ -340,6 +340,20 @@ export async function POST(request: NextRequest) {
const existingPlugin = await getPlugin(slug);
const isUpdate = existingPlugin !== null;
// An update must not widen what a plugin may do with nobody seeing it. `force` is for
// withdrawn and older versions, and says nothing about this.
const implicit = new Set<string>(IMPLICIT_PERMISSIONS);
const permissionsAdded = isUpdate
? manifest.permissions.filter((p) => !existingPlugin.permissions?.includes(p) && !implicit.has(p))
: [];
const accepted = Array.isArray(acceptPermissions) ? acceptPermissions : [];
if (permissionsAdded.some((p) => !accepted.includes(p))) {
return NextResponse.json(
{ error: `Version ${version} asks for new permissions: ${permissionsAdded.join(', ')}.`, permissionsAdded },
{ status: 409 },
);
}
const plugin: ServerPlugin = {
id: slug,
name: manifest.name,
@@ -382,6 +396,7 @@ export async function POST(request: NextRequest) {
httpOrigins: plugin.httpOrigins ?? [],
apiPostPaths: plugin.apiPostPaths ?? [],
signature: signature.status,
...(permissionsAdded.length > 0 ? { permissionsAdded } : {}),
...(meta.yanked ? { yanked: true } : {}),
...(isUpdate ? { previousVersion: existingPlugin.version } : {}),
},
+11 -1
View File
@@ -1,6 +1,8 @@
import { NextRequest, NextResponse } from 'next/server';
import { requireAdminAuth } from '@/lib/admin/session';
import { logger } from '@/lib/logger';
import { getPluginRegistry } from '@/lib/admin/plugin-registry';
import { IMPLICIT_PERMISSIONS } from '@/lib/plugin-types';
import {
APP_VERSION,
DirectoryError,
@@ -19,7 +21,8 @@ import {
export interface UpdatesAnswer {
/** `changelogs` is missing from directories older than 2026-10-30; `changelog` is then the offered version's alone. */
updates: Array<DirectoryVersion & { slug: string; currentVersion: string; changelog: string | null; changelogs?: DirectoryChangelogEntry[] }>;
/** `addedPermissions` is what the directory lists against what's installed; the install itself reads the bundle. */
updates: Array<DirectoryVersion & { slug: string; currentVersion: string; changelog: string | null; changelogs?: DirectoryChangelogEntry[]; addedPermissions?: string[] }>;
yanked: Array<{ slug: string; version: string; reason: string | null; at: string }>;
deprecated: Array<{ slug: string; message: string | null; replacement: string | null }>;
suspended: string[];
@@ -66,6 +69,13 @@ export async function GET(request: NextRequest) {
body: JSON.stringify({ appVersion: APP_VERSION || undefined, channel, installed: ask }),
});
answer = { ...EMPTY, ...res.data };
const { plugins } = await getPluginRegistry();
const held = new Map(plugins.map((p) => [p.id, new Set<string>(p.permissions ?? [])]));
const implicit = new Set<string>(IMPLICIT_PERMISSIONS);
answer.updates = answer.updates.map((u) => ({
...u,
addedPermissions: (u.permissions ?? []).filter((p) => !implicit.has(p) && !(held.get(u.slug)?.has(p) ?? true)),
}));
} catch (err) {
// The installed list is still useful without the directory; say what
// went wrong instead of failing the whole view.