mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 16:13:15 +00:00
Linus Rath
f056db5a05
Review follow-up to #751. The offload path could not actually run: onBeforeBlobUpload goes through HookBus.transform, which capped every handler at the 5s default. A handler that pushes the file to an external store blows past that, and on timeout the error is swallowed, the original file id is kept, and the composer attaches the big file anyway - silently - while three such timeouts in a minute auto-disable the plugin. Buses can now carry their own per-handler budget and this one gets 120s. The same ceiling existed inside the sandbox, where http.post/http.fetch were bounded by the 30s default API timeout; those two are transfers rather than round-trips and now get 120s as well. PRIVILEGED_ONLY_METHODS listed 'upfiles.set', which matches no dispatched method - the dispatcher calls it 'upfiles.save'. So the write was never tier-gated despite the comment saying it was, and an untrusted plugin with email:blob-write could replace the bytes of a file the user was about to send. Fixed the name. Relaxing the read while that stayed open was the wrong order. Moving upfiles.get to email:blob-read is kept, but it is a wider grant than the PR described: onBeforeBlobUpload hands the file id to every registered handler, so the id is not a secret and any untrusted plugin holding email:blob-read can read every file the user attaches. The consent copy now says that; email:blob-write's copy was describing the read and now describes the replace it actually gates. Also in the composer offload path: - sanitize the handler's html before it reaches the document and the sent message, and insert it through the editor at the caret rather than concatenating it after the signature and quoted block - honour an abort that lands while the handler is offloading - delete the right staged file when the handler re-saved it under a new id via upfiles.save, which otherwise stranded a copy in IndexedDB forever
96 lines
3.7 KiB
TypeScript
96 lines
3.7 KiB
TypeScript
import { describe, it, expect, vi } from 'vitest';
|
|
import type { InstalledPlugin } from '../plugin-types';
|
|
|
|
vi.mock('@/stores/toast-store', () => ({
|
|
toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() },
|
|
}));
|
|
|
|
// host-api pulls in the whole store layer; nothing asserted here reaches a
|
|
// store, but the module graph has to resolve.
|
|
import { dispatchApiCall } from '../plugin-sandbox/host-api';
|
|
|
|
function plugin(overrides: Partial<InstalledPlugin> = {}): InstalledPlugin {
|
|
return {
|
|
id: 'p1',
|
|
name: 'Test plugin',
|
|
version: '1.0.0',
|
|
author: 'test',
|
|
description: '',
|
|
type: 'hook',
|
|
permissions: [],
|
|
entrypoint: 'index.js',
|
|
enabled: true,
|
|
status: 'running',
|
|
settings: {},
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Message of the rejection, or '' when the call got through. The gates are
|
|
* what's under test; what happens afterwards needs an IndexedDB and a JMAP
|
|
* session, so "did not fail at a gate" is the useful assertion for the
|
|
* allowed cases.
|
|
*/
|
|
async function gateError(...args: Parameters<typeof dispatchApiCall>): Promise<string> {
|
|
try {
|
|
await dispatchApiCall(...args);
|
|
return '';
|
|
} catch (err) {
|
|
return (err as Error).message;
|
|
}
|
|
}
|
|
|
|
const GATE_FAILURE = /lacks permission|requires the privileged plugin tier|Unknown API method/;
|
|
|
|
/**
|
|
* The staged-attachment API is the one place an untrusted plugin can touch the
|
|
* bytes of a file the user is sending, so the read/write asymmetry is worth
|
|
* pinning down: reading back a file it was just handed the id of is a normal
|
|
* permission, replacing that file is privileged-tier only.
|
|
*/
|
|
describe('upfiles tier + permission gates', () => {
|
|
it('refuses upfiles.get without email:blob-read', async () => {
|
|
expect(await gateError(plugin(), 'upfiles.get', ['some-id']))
|
|
.toMatch(/lacks permission "email:blob-read"/);
|
|
});
|
|
|
|
it('refuses upfiles.get when the permission is declared but not granted', async () => {
|
|
const p = plugin({ permissions: ['email:blob-read'], grantedPermissions: [] });
|
|
expect(await gateError(p, 'upfiles.get', ['some-id']))
|
|
.toMatch(/lacks permission "email:blob-read"/);
|
|
});
|
|
|
|
it('allows upfiles.get for an untrusted plugin granted email:blob-read', async () => {
|
|
const p = plugin({ permissions: ['email:blob-read'], grantedPermissions: ['email:blob-read'] });
|
|
expect(await gateError(p, 'upfiles.get', ['some-id'])).not.toMatch(GATE_FAILURE);
|
|
});
|
|
|
|
it('refuses upfiles.save from an untrusted plugin even with email:blob-write', async () => {
|
|
const p = plugin({ permissions: ['email:blob-write'], grantedPermissions: ['email:blob-write'] });
|
|
expect(await gateError(p, 'upfiles.save', ['old-id', new File(['x'], 'x.txt')]))
|
|
.toMatch(/requires the privileged plugin tier/);
|
|
});
|
|
|
|
it('lets a privileged plugin call upfiles.save', async () => {
|
|
const p = plugin({ permissions: ['email:blob-write'], grantedPermissions: ['email:blob-write'] });
|
|
const err = await gateError(p, 'upfiles.save', ['old-id', new File(['x'], 'x.txt')], { privileged: true });
|
|
expect(err).not.toMatch(GATE_FAILURE);
|
|
});
|
|
|
|
it('keeps stored message blobs privileged-only', async () => {
|
|
const p = plugin({ permissions: ['email:blob-read'], grantedPermissions: ['email:blob-read'] });
|
|
expect(await gateError(p, 'jmap.fetchBlob', ['blob-1']))
|
|
.toMatch(/requires the privileged plugin tier/);
|
|
});
|
|
});
|
|
|
|
describe('method-name typos', () => {
|
|
// Guards the class of bug this suite was added for: an entry in the
|
|
// privileged set naming a method the dispatcher never sees, which gates
|
|
// nothing while looking like it does.
|
|
it('rejects a method with no entry in the permission map', async () => {
|
|
expect(await gateError(plugin(), 'upfiles.set', ['x'])).toMatch(/Unknown API method/);
|
|
});
|
|
});
|