fix: refuse a policy save from a page opened before someone else saved #1066

The admin pages save the whole policy from the draft they loaded. A
save from a page opened before another tab's or another admin's save,
or before a deleted plugin was dropped from the policy, used to put the
older policy back. GET /api/admin/policy now sends the policy version as
an ETag; the policy, organisation, plugins and themes pages send it back
as If-Match, and a save based on an older version answers 409 and keeps
the draft on the page. A PUT without If-Match replaces the policy as
before. Deleting a plugin or theme changes only its id in the stored
policy, and a policy import compares against what it replaces.

Test mocks of the config manager gain getPolicyVersion and updatePolicy,
which the route and the extension cleanup now call.

app/(main)/admin/_pages/plugins.tsx

@@ -33,6 +33,8 @@ export function PluginsPage() {
   const [policy, setPolicy] = useState<SettingsPolicy>({ ...DEFAULT_POLICY });
   const [savedPolicy, setSavedPolicy] = useState<SettingsPolicy>({ ...DEFAULT_POLICY });
   const [savingPolicy, setSavingPolicy] = useState(false);
+  // The version of the policy this page loaded (see the policy page).
+  const policyETag = useRef<string | null>(null);
   const iconFor = useExtensionIcons();
 
   const fetchPlugins = useCallback(async () => {
@@ -49,6 +51,7 @@ export function PluginsPage() {
       const res = await apiFetch('/api/admin/policy');
       if (res.ok) {
         const data = await res.json();
+        policyETag.current = res.headers.get('ETag');
         setPolicy(data);
         setSavedPolicy(data);
       }
@@ -77,10 +80,11 @@ export function PluginsPage() {
     try {
       const res = await apiFetch('/api/admin/policy', {
         method: 'PUT',
-        headers: { 'Content-Type': 'application/json' },
+        headers: { 'Content-Type': 'application/json', ...(policyETag.current ? { 'If-Match': policyETag.current } : {}) },
         body: JSON.stringify(policy),
       });
       if (res.ok) {
+        policyETag.current = res.headers.get('ETag');
         setSavedPolicy(policy);
         return null;
       }

app/(main)/admin/_pages/policy.tsx

@@ -1,6 +1,6 @@
 'use client';
 
-import { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import { Loader2, Plus, Trash2, ArrowUp, ArrowDown } from '@/components/icons';
 import { iconForName } from '@/components/icons';
 import type { SettingsPolicy, FeatureGates, PushRelayOption, AdminSidebarApp } from '@/lib/admin/types';
@@ -79,6 +79,9 @@ export function PolicyPage() {
   const [saving, setSaving] = useState(false);
   const setMessage = useAdminFeedback();
   const [dirty, setDirty] = useState(false);
+  // The version of the policy this page loaded: a save based on an older
+  // one is refused instead of undoing someone else's change.
+  const policyETag = useRef<string | null>(null);
 
   useEffect(() => { fetchPolicy(); }, []);
 
@@ -88,6 +91,7 @@ export function PolicyPage() {
       const res = await apiFetch('/api/admin/policy');
       if (res.ok) {
         const data = await res.json();
+        policyETag.current = res.headers.get('ETag');
         setPolicy(data);
         setSavedPolicy(data);
       }
@@ -219,11 +223,12 @@ export function PolicyPage() {
     try {
       const res = await apiFetch('/api/admin/policy', {
         method: 'PUT',
-        headers: { 'Content-Type': 'application/json' },
+        headers: { 'Content-Type': 'application/json', ...(policyETag.current ? { 'If-Match': policyETag.current } : {}) },
         body: JSON.stringify(policy),
       });
 
       if (res.ok) {
+        policyETag.current = res.headers.get('ETag');
         setSavedPolicy(policy);
         setMessage({ type: 'success', text: 'Policy saved. Users see the change on their next login.' });
         setDirty(false);

app/(main)/admin/_pages/themes.tsx

@@ -49,6 +49,8 @@ export function ThemesPage() {
   const [policy, setPolicy] = useState<SettingsPolicy>(() => withThemeDefaults({ ...DEFAULT_POLICY }));
   const [savedPolicy, setSavedPolicy] = useState<SettingsPolicy>(() => withThemeDefaults({ ...DEFAULT_POLICY }));
   const [savingPolicy, setSavingPolicy] = useState(false);
+  // The version of the policy this page loaded (see the policy page).
+  const policyETag = useRef<string | null>(null);
 
   const fetchThemes = useCallback(async () => {
     try {
@@ -64,6 +66,7 @@ export function ThemesPage() {
       const res = await apiFetch('/api/admin/policy');
       if (res.ok) {
         const data = withThemeDefaults(await res.json());
+        policyETag.current = res.headers.get('ETag');
         setPolicy(data);
         setSavedPolicy(data);
       }
@@ -152,10 +155,11 @@ export function ThemesPage() {
     try {
       const res = await apiFetch('/api/admin/policy', {
         method: 'PUT',
-        headers: { 'Content-Type': 'application/json' },
+        headers: { 'Content-Type': 'application/json', ...(policyETag.current ? { 'If-Match': policyETag.current } : {}) },
         body: JSON.stringify(policy),
       });
       if (res.ok) {
+        policyETag.current = res.headers.get('ETag');
         setSavedPolicy(policy);
         return null;
       }

app/api/admin/__tests__/policy-if-match.test.ts

@@ -0,0 +1,104 @@
+// @vitest-environment node
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { NextRequest } from 'next/server';
+
+// The admin pages save the whole policy from a draft they loaded earlier.
+// A save from a page opened before someone else's save (another tab,
+// another admin) used to put the old policy back over theirs.
+
+vi.mock('@/lib/auth/instance-details', () => ({
+  canSeeInstanceDetails: async () => true,
+  signedInIdentity: async () => ({ admin: true }),
+}));
+vi.mock('@/lib/admin/session', () => ({
+  requireAdminAuth: async () => ({ payload: { role: 'admin' } }),
+  getClientIP: () => '127.0.0.1',
+}));
+vi.mock('@/lib/admin/audit', () => ({ auditLog: async () => {} }));
+
+import { GET, PUT } from '@/app/api/admin/policy/route';
+import { forgetExtensionInPolicy } from '@/lib/admin/extension-cleanup';
+import { configManager } from '@/lib/admin/config-manager';
+
+let dir: string;
+const URL = 'https://mail.example/api/admin/policy';
+const stored = async () => JSON.parse(await readFile(path.join(dir, 'policy.json'), 'utf-8'));
+
+async function load(): Promise<{ policy: Record<string, unknown>; etag: string }> {
+  const res = await GET(new NextRequest(URL));
+  expect(res.status).toBe(200);
+  const etag = res.headers.get('ETag');
+  expect(etag).toMatch(/^".+"$/);
+  return { policy: await res.json(), etag: etag! };
+}
+
+function save(policy: Record<string, unknown>, etag?: string) {
+  return PUT(new NextRequest(URL, {
+    method: 'PUT',
+    headers: { 'content-type': 'application/json', ...(etag ? { 'if-match': etag } : {}) },
+    body: JSON.stringify(policy),
+  }));
+}
+
+beforeEach(async () => {
+  dir = await mkdtemp(path.join(tmpdir(), 'bw-policy-if-match-'));
+  vi.stubEnv('ADMIN_CONFIG_DIR', dir);
+  await writeFile(path.join(dir, 'policy.json'), JSON.stringify({ features: { calendarEnabled: true }, forceEnabledPlugins: ['demo'] }));
+  await configManager.reload();
+});
+
+afterEach(async () => {
+  vi.unstubAllEnvs();
+  await rm(dir, { recursive: true, force: true });
+});
+
+describe('policy save with If-Match', () => {
+  it('refuses a save based on an older policy and keeps the newer one', async () => {
+    const tabA = await load();
+    const tabB = await load();
+
+    const first = await save({ ...tabA.policy, features: { ...(tabA.policy.features as object), calendarEnabled: false } }, tabA.etag);
+    expect(first.status).toBe(200);
+    expect(first.headers.get('ETag')).not.toBe(tabA.etag);
+
+    const stale = await save({ ...tabB.policy, features: { ...(tabB.policy.features as object), filesEnabled: false } }, tabB.etag);
+    expect(stale.status).toBe(409);
+    expect((await stale.json()).error).toBe(
+      'Someone else changed the policy after you opened this page. Reload to see their change; your edits stay here until you reload.',
+    );
+    expect((await stored()).features.calendarEnabled).toBe(false);
+    expect((await stored()).features.filesEnabled).not.toBe(false);
+    expect(configManager.getPolicy().features.calendarEnabled).toBe(false);
+
+    // Saving again with the version the first save answered works.
+    const again = await save({ ...tabA.policy, features: { ...(tabA.policy.features as object), calendarEnabled: true } }, first.headers.get('ETag')!);
+    expect(again.status).toBe(200);
+  });
+
+  it('sees a change saved to policy.json by another replica on the same volume', async () => {
+    const page = await load();
+    await writeFile(path.join(dir, 'policy.json'), JSON.stringify({ features: { calendarEnabled: false } }));
+    const res = await save({ ...page.policy, features: { ...(page.policy.features as object), filesEnabled: false } }, page.etag);
+    expect(res.status).toBe(409);
+    expect((await stored()).features).toEqual({ calendarEnabled: false });
+  });
+
+  it('refuses a page loaded before a plugin was dropped from the policy', async () => {
+    const page = await load();
+    await forgetExtensionInPolicy('plugin', 'demo');
+    expect((await stored()).forceEnabledPlugins).toEqual([]);
+    expect((await save(page.policy, page.etag)).status).toBe(409);
+    expect((await stored()).forceEnabledPlugins).toEqual([]);
+  });
+
+  it('replaces the policy without If-Match, as before', async () => {
+    await load();
+    await configManager.setPolicy({ ...configManager.getPolicy(), forceEnabledPlugins: [] });
+    const res = await save({ features: { calendarEnabled: false } });
+    expect(res.status).toBe(200);
+    expect((await stored()).features.calendarEnabled).toBe(false);
+  });
+});

app/api/admin/__tests__/policy-scopes-route.test.ts

@@ -28,6 +28,7 @@ vi.mock('@/lib/admin/config-manager', () => ({
       organisation: { disclaimer: { html: '<p>Acme GmbH</p>', text: 'Acme GmbH' } },
       scopes: SCOPES,
     }),
+    getPolicyVersion: () => 'v1',
   },
 }));
 vi.mock('@/lib/logger', () => ({ logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() } }));

app/api/admin/plugins/__tests__/route.test.ts

@@ -18,6 +18,7 @@ vi.mock('@/lib/admin/config-manager', () => ({
     get: vi.fn(),
     getPolicy: vi.fn(() => policy),
     setPolicy: vi.fn(async (p: Record<string, unknown>) => { policy = p; }),
+    updatePolicy: vi.fn(async (change: (p: Record<string, unknown>) => Record<string, unknown> | undefined) => { policy = change(policy) ?? policy; }),
   },
 }));
 

app/api/admin/policy/file/route.ts

@@ -1,5 +1,5 @@
 import { NextRequest, NextResponse } from 'next/server';
-import { completePolicy, configManager } from '@/lib/admin/config-manager';
+import { completePolicy, configManager, PolicyConflictError } from '@/lib/admin/config-manager';
 import { requireAdminAuth, getClientIP } from '@/lib/admin/session';
 import { auditLog } from '@/lib/admin/audit';
 import { logger } from '@/lib/logger';
@@ -57,12 +57,24 @@ export async function POST(request: NextRequest) {
 
     await configManager.ensureLoaded();
     const next = completePolicy(raw as Partial<SettingsPolicy>);
-    const summary = summarizePolicyChange(configManager.getPolicy(), next);
     if (request.nextUrl.searchParams.get('preview') === '1') {
-      return NextResponse.json({ summary });
+      return NextResponse.json({ summary: summarizePolicyChange(configManager.getPolicy(), next) });
     }
 
-    await configManager.setPolicy(next);
+    // The import replaces the policy it was compared with: a save that lands
+    // in between is compared again, so the summary says what was replaced.
+    let summary: ReturnType<typeof summarizePolicyChange>;
+    for (let attempt = 1; ; attempt++) {
+      const version = configManager.getPolicyVersion();
+      summary = summarizePolicyChange(configManager.getPolicy(), next);
+      try {
+        await configManager.setPolicy(next, { ifVersion: version });
+        break;
+      } catch (error) {
+        if (!(error instanceof PolicyConflictError)) throw error;
+        if (attempt >= 3) return NextResponse.json({ error: error.message }, { status: 409 });
+      }
+    }
     await auditLog('policy.import', {
       scopeCount: next.scopes?.length ?? 0,
       scopesAdded: summary.scopes.added.length,

app/api/admin/policy/route.ts

@@ -1,5 +1,5 @@
 import { NextRequest, NextResponse } from 'next/server';
-import { configManager } from '@/lib/admin/config-manager';
+import { configManager, PolicyConflictError } from '@/lib/admin/config-manager';
 import { requireAdminAuth, getClientIP } from '@/lib/admin/session';
 import { auditLog } from '@/lib/admin/audit';
 import { logger } from '@/lib/logger';
@@ -9,6 +9,21 @@ import { bearerIdentity } from '@/lib/auth/bearer-identity';
 import { scopesVisibleTo } from '@/lib/admin/policy-scopes';
 import { configReadOnlyResponse } from '@/lib/admin/config-read-only-response';
 
+/** The policy version as an HTTP entity tag. */
+function policyETag(version: string): string {
+  return `"${version}"`;
+}
+
+/**
+ * The version an If-Match header names, or undefined for none (or `*`).
+ * The admin pages send the ETag of the policy they loaded.
+ */
+function ifMatchVersion(request: NextRequest): string | undefined {
+  const raw = request.headers.get('if-match')?.trim();
+  if (!raw || raw === '*') return undefined;
+  return raw.split(',')[0].trim().replace(/^W\//, '').replace(/^"(.*)"$/, '$1');
+}
+
 /**
  * GET /api/admin/policy - Get settings policy (NOT admin-protected - users read this;
  * visitors who are not signed in get the subset the login page needs)
@@ -52,8 +67,10 @@ export async function GET(request: NextRequest) {
     // answers with all of them (documented).
     const identity = await signedInIdentity();
     if (identity !== null && !identity.admin) return userPolicyResponse(policy, identity.addresses);
+    // The whole policy, as the admin pages edit it: with its version, which
+    // they send back as If-Match when they save.
     return NextResponse.json({ ...policy, scopes: policy.scopes ?? [] }, {
-      headers: { 'Cache-Control': 'no-store' },
+      headers: { 'Cache-Control': 'no-store', ETag: policyETag(configManager.getPolicyVersion()) },
     });
   } catch (error) {
     logger.error('Policy read error', { error: error instanceof Error ? error.message : 'Unknown error' });
@@ -74,6 +91,10 @@ function userPolicyResponse(policy: SettingsPolicy, addresses: string[]): NextRe
 
 /**
  * PUT /api/admin/policy - Update settings policy (admin-protected)
+ *
+ * With `If-Match: <ETag from GET>` the save is refused with 409 when the
+ * policy changed since it was read, so a page opened earlier can't undo
+ * someone else's save. Without it the policy is replaced, as it always was.
  */
 export async function PUT(request: NextRequest) {
   try {
@@ -110,14 +131,22 @@ export async function PUT(request: NextRequest) {
       return NextResponse.json({ error: 'organisation must be an object' }, { status: 400 });
     }
 
-    await configManager.setPolicy(policy);
+    let version: string;
+    try {
+      version = await configManager.setPolicy(policy, { ifVersion: ifMatchVersion(request) });
+    } catch (error) {
+      if (error instanceof PolicyConflictError) {
+        return NextResponse.json({ error: error.message }, { status: 409 });
+      }
+      throw error;
+    }
     await auditLog('policy.update', {
       restrictionCount: Object.keys(policy.restrictions || {}).length,
       // Entries the sanitizer dropped never reach users, so log what stuck.
       defaultSidebarAppCount: configManager.getPolicy().defaultSidebarApps?.length ?? 0,
     }, ip);
 
-    return NextResponse.json({ ok: true });
+    return NextResponse.json({ ok: true }, { headers: { ETag: policyETag(version) } });
   } catch (error) {
     const readOnly = configReadOnlyResponse(error);
     if (readOnly) return readOnly;

app/api/admin/themes/__tests__/route.test.ts

@@ -17,6 +17,7 @@ vi.mock('@/lib/admin/config-manager', () => ({
     get: vi.fn(),
     getPolicy: vi.fn(() => policy),
     setPolicy: vi.fn(async (p: Record<string, unknown>) => { policy = p; }),
+    updatePolicy: vi.fn(async (change: (p: Record<string, unknown>) => Record<string, unknown> | undefined) => { policy = change(policy) ?? policy; }),
   },
 }));
 const saved: Array<{ theme: Record<string, unknown>; css: string; skin: string | null }> = [];

components/admin/org/use-org-policy-draft.ts

@@ -1,6 +1,6 @@
 'use client';
 
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { create } from 'zustand';
 import type { FeatureGates, SettingRestriction, SettingsPolicy } from '@/lib/admin/types';
 import { DEFAULT_POLICY } from '@/lib/admin/types';
@@ -50,6 +50,9 @@ export function useOrgPolicyDraft(dirtyId: string) {
   const [policy, setPolicy] = useState<SettingsPolicy | null>(null);
   const [saved, setSaved] = useState<SettingsPolicy | null>(null);
   const [saving, setSaving] = useState(false);
+  // The version of the policy this draft started from: a save based on an
+  // older one is refused instead of undoing someone else's change.
+  const policyETag = useRef<string | null>(null);
   // Outcomes show in a toast, like every admin page's.
   const setMessage = useAdminFeedback();
   const scopeId = useOrgScopeStore((s) => s.scopeId);
@@ -60,6 +63,7 @@ export function useOrgPolicyDraft(dirtyId: string) {
       const res = await apiFetch('/api/admin/policy');
       if (!res.ok) throw new Error();
       const data = (await res.json()) as SettingsPolicy;
+      policyETag.current = res.headers.get('ETag');
       const normalised = { ...DEFAULT_POLICY, ...data, organisation: sanitizeOrganisationPolicy(data.organisation), scopes: data.scopes ?? [] };
       setPolicy(normalised);
       setSaved(clone(normalised));
@@ -186,7 +190,7 @@ export function useOrgPolicyDraft(dirtyId: string) {
     try {
       const res = await apiFetch('/api/admin/policy', {
         method: 'PUT',
-        headers: { 'Content-Type': 'application/json' },
+        headers: { 'Content-Type': 'application/json', ...(policyETag.current ? { 'If-Match': policyETag.current } : {}) },
         body: JSON.stringify(policy),
       });
       if (res.ok) {

lib/__tests__/instance-details-gates.test.ts

@@ -32,6 +32,7 @@ vi.mock('@/lib/admin/config-manager', () => ({
     ensureLoaded: async () => {},
     get: (key: string, fallback: unknown) => (key in config.values ? config.values[key] : fallback),
     getPolicy: () => POLICY,
+    getPolicyVersion: () => 'v1',
   },
 }));
 vi.mock('@/lib/logger', () => ({ logger: { error: vi.fn(), warn: vi.fn(), info: vi.fn(), debug: vi.fn() } }));

lib/admin/config-manager.ts

@@ -8,6 +8,8 @@ import { sanitizeDefaultSidebarApps } from '@/lib/sidebar-apps';
 import { sanitizePolicyDefaults } from './policy-defaults';
 import { sanitizeOrganisationPolicy } from './organisation-policy';
 import { sanitizeRestrictions, sanitizeScopes } from './policy-scopes';
+import { versionOf } from '@/lib/shared-store/keys';
+import { serializeLocal } from '@/lib/shared-store/local';
 
 function parseEnvValue(value: string, type: string): unknown {
   switch (type) {
@@ -39,6 +41,34 @@ type WatchedFile = typeof WATCHED_FILES[number];
  */
 const CHANGE_CHECK_INTERVAL_MS = 5_000;
 
+/** The policy version while no policy has been saved (the defaults apply). */
+export const DEFAULT_POLICY_VERSION = 'default';
+
+/**
+ * A policy save based on a version that is no longer the current one:
+ * someone else saved in between (another tab, another admin, a plugin
+ * deletion). The caller's draft would undo their change.
+ */
+export class PolicyConflictError extends Error {
+  constructor() {
+    super('Someone else changed the policy after you opened this page. Reload to see their change; your edits stay here until you reload.');
+    this.name = 'PolicyConflictError';
+  }
+}
+
+/** Policy writes in this process, one after another, so a version check and its write can't interleave. */
+const POLICY_WRITE_QUEUE = 'admin-config\0policy.json';
+
+interface ParsedDoc {
+  data: Record<string, unknown>;
+  /** As the shared store computes it: changes whenever the bytes do. */
+  version: string;
+}
+
+function parseDoc(raw: string): ParsedDoc {
+  return { data: JSON.parse(raw), version: versionOf(Buffer.from(raw, 'utf-8')) };
+}
+
 /** Changes whenever the file is replaced or rewritten; null when it is missing. */
 async function fileStamp(filename: string): Promise<string | null> {
   try {
@@ -52,6 +82,8 @@ async function fileStamp(filename: string): Promise<string | null> {
 class ConfigManager {
   private adminConfig: Record<string, unknown> = {};
   private policyCache: SettingsPolicy = { ...DEFAULT_POLICY };
+  /** Version of the stored policy the cache came from; null while none is stored. */
+  private policyVersion: string | null = null;
   private loaded = false;
   private fileStamps: Partial<Record<WatchedFile, string | null>> = {};
   private lastChangeCheck = 0;
@@ -67,11 +99,12 @@ class ConfigManager {
     logger.debug('ConfigManager loaded', { configKeys: Object.keys(this.adminConfig).length });
   }
 
-  private apply(config: Record<string, unknown> | null, policy: Record<string, unknown> | null): void {
-    this.adminConfig = config || {};
-    this.policyCache = policy
-      ? ConfigManager.completePolicy(policy as Partial<SettingsPolicy>)
+  private apply(config: ParsedDoc | null, policy: ParsedDoc | null): void {
+    this.adminConfig = config?.data || {};
+    this.policyCache = policy?.data
+      ? ConfigManager.completePolicy(policy.data as Partial<SettingsPolicy>)
       : { ...DEFAULT_POLICY };
+    this.policyVersion = policy?.version ?? null;
     this.loaded = true;
     this.applyLoggerSettings();
   }
@@ -273,12 +306,47 @@ class ConfigManager {
   }
 
   /**
-   * Update the settings policy. Writes to disk.
+   * The version of the policy getPolicy() returns, for an ETag. Changes with
+   * every save; DEFAULT_POLICY_VERSION while none was saved.
    */
-  async setPolicy(policy: SettingsPolicy): Promise<void> {
+  getPolicyVersion(): string {
+    return this.policyVersion ?? DEFAULT_POLICY_VERSION;
+  }
+
+  /**
+   * Update the settings policy. Writes to disk. With `ifVersion` (from
+   * getPolicyVersion when the caller read the policy) it refuses with a
+   * PolicyConflictError when the stored policy has changed since, so a
+   * draft based on an older policy can't undo someone else's save.
+   * Returns the new version.
+   */
+  async setPolicy(policy: SettingsPolicy, options: { ifVersion?: string } = {}): Promise<string> {
     assertWritable('update settings policy');
-    this.policyCache = ConfigManager.completePolicy(policy);
-    await this.writeJsonFile('policy.json', this.policyCache as unknown as Record<string, unknown>);
+    return serializeLocal(POLICY_WRITE_QUEUE, async () => {
+      if (options.ifVersion !== undefined) {
+        // Compare with what is on disk now, not with what this process saw last.
+        await this.reloadIfChanged();
+        if (options.ifVersion !== this.getPolicyVersion()) throw new PolicyConflictError();
+      }
+      this.policyCache = ConfigManager.completePolicy(policy);
+      await this.writeJsonFile('policy.json', this.policyCache as unknown as Record<string, unknown>);
+      return this.getPolicyVersion();
+    });
+  }
+
+  /**
+   * Change the stored policy with `change` (return undefined to leave it).
+   * For changes to one part of the policy that must not undo a save that
+   * lands at the same time, such as dropping a deleted plugin's id.
+   */
+  async updatePolicy(change: (current: SettingsPolicy) => SettingsPolicy | undefined): Promise<void> {
+    assertWritable('update settings policy');
+    await serializeLocal(POLICY_WRITE_QUEUE, async () => {
+      const next = change(this.policyCache);
+      if (next === undefined) return;
+      this.policyCache = ConfigManager.completePolicy(next);
+      await this.writeJsonFile('policy.json', this.policyCache as unknown as Record<string, unknown>);
+    });
   }
 
   /**
@@ -353,7 +421,7 @@ class ConfigManager {
     await this.load();
   }
 
-  private async readJsonFile(filename: string): Promise<Record<string, unknown> | null> {
+  private async readJsonFile(filename: string): Promise<ParsedDoc | null> {
     try {
       return await this.readJsonFileOrThrow(filename);
     } catch (error) {
@@ -363,9 +431,9 @@ class ConfigManager {
   }
 
   /** null when the file doesn't exist; throws when it can't be read or parsed. */
-  private async readJsonFileOrThrow(filename: string): Promise<Record<string, unknown> | null> {
+  private async readJsonFileOrThrow(filename: string): Promise<ParsedDoc | null> {
     try {
-      return JSON.parse(await readFile(getConfigPath(filename), 'utf-8'));
+      return parseDoc(await readFile(getConfigPath(filename), 'utf-8'));
     } catch (error) {
       if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null;
       throw error;
@@ -376,9 +444,11 @@ class ConfigManager {
     await ensureConfigDir();
     const targetPath = getConfigPath(filename);
     const tmpPath = targetPath + '.tmp';
+    const text = JSON.stringify(data, null, 2);
     // Owner-only: config.json carries the session secret and OAuth client secrets.
-    await writeFile(tmpPath, JSON.stringify(data, null, 2), { encoding: 'utf-8', mode: 0o600 });
+    await writeFile(tmpPath, text, { encoding: 'utf-8', mode: 0o600 });
     await rename(tmpPath, targetPath);
+    if (filename === 'policy.json') this.policyVersion = versionOf(Buffer.from(text, 'utf-8'));
     // Our own write is not a change to pick up.
     this.fileStamps[filename] = await fileStamp(filename);
   }

lib/admin/extension-cleanup.ts

@@ -8,11 +8,14 @@ import { deletePluginConfig } from './plugin-config';
  */
 export async function forgetExtensionInPolicy(kind: 'plugin' | 'theme', id: string): Promise<void> {
   await configManager.ensureLoaded();
-  const policy = configManager.getPolicy();
   const key = kind === 'plugin' ? 'forceEnabledPlugins' : 'forceEnabledThemes';
-  const list = policy[key] ?? [];
-  if (!list.includes(id)) return;
-  await configManager.setPolicy({ ...policy, [key]: list.filter((x) => x !== id) });
+  // Against the stored policy, so a policy save landing at the same time
+  // keeps its changes.
+  await configManager.updatePolicy((policy) => {
+    const list = policy[key] ?? [];
+    if (!list.includes(id)) return undefined;
+    return { ...policy, [key]: list.filter((x) => x !== id) };
+  });
 }
 
 /**