fix: cap the plugin packages kept for approval review and sweep broken kept-version folders #1066

This commit is contained in:
Linus Rath
2026-11-01 01:04:51 +01:00
parent dcecd50b7f
commit ac29a94fc3
3 changed files with 114 additions and 0 deletions
@@ -0,0 +1,69 @@
// @vitest-environment node
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import path from 'node:path';
// What admins never see must not pile up (in the config dir or the shared
// store, #1066): the packages users sent for review are capped in total,
// the longest-decided going first, and kept-version folders without their
// entry.json (a copy cut short) are swept with the history.
const HASH = (c: string) => c.repeat(64);
let dir: string;
beforeEach(() => {
dir = mkdtempSync(path.join(tmpdir(), 'bw-ext-growth-'));
vi.stubEnv('ADMIN_CONFIG_DIR', dir);
vi.resetModules();
for (const name of ['bulwark.admin.pluginApprovals', 'bulwark.admin.pluginRegistry', 'bulwark.admin.pluginSigning']) {
delete (globalThis as Record<symbol, unknown>)[Symbol.for(name)];
}
});
afterEach(() => {
vi.unstubAllEnvs();
rmSync(dir, { recursive: true, force: true });
});
describe('packages kept for approval review', () => {
it('drop the longest-decided package once they hold more than the cap, never a pending one', async () => {
const approvals = await import('../plugin-approvals');
approvals.setApprovalPackageCapForTests(10);
const zip = new Uint8Array(6);
await approvals.requestApproval('first-one', HASH('a'), {}, 'alice@example.org');
await approvals.saveApprovalBundle('first-one', HASH('a'), zip);
await approvals.decideApproval('first-one', HASH('a'), 'approved', 'admin');
await approvals.requestApproval('second-one', HASH('b'), {}, 'bob@example.org');
await approvals.saveApprovalBundle('second-one', HASH('b'), zip);
await approvals.requestApproval('third-one', HASH('c'), {}, 'carol@example.org');
await approvals.saveApprovalBundle('third-one', HASH('c'), zip);
expect(await approvals.readApprovalBundle('first-one', HASH('a'))).toBeNull();
// Over the cap now, but both are still pending: kept.
expect(await approvals.readApprovalBundle('second-one', HASH('b'))).not.toBeNull();
expect(await approvals.readApprovalBundle('third-one', HASH('c'))).not.toBeNull();
expect((await approvals.listApprovals()).map((e) => [e.pluginId, e.packageBytes])).toEqual([
['first-one', undefined], ['second-one', 6], ['third-one', 6],
]);
approvals.setApprovalPackageCapForTests(null);
});
});
describe('extension history', () => {
it('sweeps a kept-version folder that has no entry.json', async () => {
const broken = path.join(dir, 'plugins', 'history', 'demo-plugin', '9.9.9');
mkdirSync(broken, { recursive: true });
writeFileSync(path.join(broken, 'bundle.js'), 'half()');
const registry = await import('../plugin-registry');
const plugin = { id: 'demo-plugin', name: 'D', version: '1.0.0', author: 'a', description: '', type: 'hook', permissions: [], entrypoint: 'index.js', enabled: true, installedAt: 'x', updatedAt: 'x' };
await registry.savePlugin(plugin, 'v1()');
await registry.savePlugin({ ...plugin, version: '1.1.0' }, 'v2()');
expect(existsSync(broken)).toBe(false);
expect(existsSync(path.join(dir, 'plugins', 'history', 'demo-plugin', '1.0.0', 'entry.json'))).toBe(true);
});
});
+7
View File
@@ -168,6 +168,13 @@ async function prune(kind: ExtensionKind, id: string): Promise<void> {
for (const stale of all.slice(KEEP)) {
await removeFolder(kind, stale.dir);
}
// Folders without a readable entry.json (a copy that was cut short) are
// never listed or restored; sweep them so they don't pile up.
const root = historyRoot(id);
const listed = new Set(all.map((e) => e.dir));
for (const name of await versionFolders(kind, root).catch(() => [] as string[])) {
if (!listed.has(`${root}/${name}`)) await removeFolder(kind, `${root}/${name}`);
}
}
/**
+38
View File
@@ -46,6 +46,8 @@ export interface ApprovalEntry {
requestedAt: string; // ISO 8601
decidedBy?: string; // admin username (set on approve/deny)
decidedAt?: string;
/** Size of the package kept for review (plugin-approvals/<id>-<hash>.zip), when one is. */
packageBytes?: number;
}
interface ApprovalsFile {
@@ -62,9 +64,45 @@ function bundleId(pluginId: string, bundleHash: string): string {
return `plugin-approvals/${pluginId}-${bundleHash}.zip`;
}
/**
* The packages kept for review hold at most this much together. Beyond it
* the packages of the longest-decided requests go first; pending ones are
* always kept (the admin still has to read them). Packages stored before
* their size was recorded aren't counted.
*/
let maxPackageBytes = 100 * 1024 * 1024;
/** A smaller cap (tests). */
export function setApprovalPackageCapForTests(bytes: number | null): void {
maxPackageBytes = bytes ?? 100 * 1024 * 1024;
}
export async function saveApprovalBundle(pluginId: string, bundleHash: string, zip: Uint8Array): Promise<void> {
assertWritable('plugin-approvals.saveBundle');
await pluginApprovalDocs().put(bundleId(pluginId, bundleHash), Buffer.from(zip));
let dropped: Array<{ pluginId: string; bundleHash: string }> = [];
await changeStored((file) => {
dropped = [];
let changed = false;
const entry = findEntry(file, pluginId, bundleHash);
if (entry && entry.packageBytes !== zip.byteLength) {
entry.packageBytes = zip.byteLength;
changed = true;
}
let total = file.entries.reduce((sum, e) => sum + (e.packageBytes ?? 0), 0);
const oldestDecidedFirst = file.entries
.filter((e) => e.status !== 'pending' && e.packageBytes)
.sort((a, b) => (a.decidedAt ?? a.requestedAt).localeCompare(b.decidedAt ?? b.requestedAt));
for (const old of oldestDecidedFirst) {
if (total <= maxPackageBytes) break;
total -= old.packageBytes!;
delete old.packageBytes;
dropped.push({ pluginId: old.pluginId, bundleHash: old.bundleHash });
changed = true;
}
return changed;
});
for (const old of dropped) await deleteApprovalBundle(old.pluginId, old.bundleHash);
}
export async function readApprovalBundle(pluginId: string, bundleHash: string): Promise<Uint8Array | null> {