fix: serialize administrator user mutations

backend/__tests__/service/adminPauseLeavesHostedRow.test.js

@@ -1,3 +1,10 @@
+// This fixture stubs User and auth; real lock/authorization behavior is tested
+// across Mongo in service/adminRoleConcurrency.test.js.
+jest.mock('../../services/adminUserMutationService', () => ({
+  withAdminUserMutation: (_actor, operation) => operation(() => {}),
+  AdminUserMutationError: class extends Error {},
+}));
+
 // TASK-172 §10 step 6b: the admin pause/resume lever projects onto the rows an
 // InstallableInstallation owns, and it selects them by `installationId`
 // (`routes/admin/installables.ts:108` / `:145`). A hosted-MCP connection is

backend/__tests__/service/adminRoleConcurrency.test.js

@@ -0,0 +1,190 @@
+const express = require('express');
+const request = require('supertest');
+const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils');
+const User = require('../../models/User');
+const { issueSessionToken } = require('../../services/sessionTokenService');
+const routes = require('../../routes/admin/users');
+
+const deferred = () => {
+  let resolve;
+  const promise = new Promise((done) => { resolve = done; });
+  return { promise, resolve };
+};
+const makeAdmin = (name) => User.create({ username: name, email: `${name}@example.test`,
+  password: 'Role-concurrency-test-123!', role: 'admin' });
+
+describe('Administrator role concurrency', () => {
+  const app = express(); app.use(express.json()); app.use('/api/admin/users', routes);
+  beforeAll(async () => { process.env.JWT_SECRET = 'admin-role-service-test'; await setupMongoDb(); });
+  afterEach(async () => { jest.restoreAllMocks(); await clearMongoDb(); });
+  afterAll(closeMongoDb);
+
+  it.each(['demote', 'delete'])('preserves an administrator when demotion races %s', async (operation) => {
+    const [alice, bob] = await Promise.all([makeAdmin('role-alice'), makeAdmin('role-bob')]);
+    const firstCount = deferred();
+    const releaseFirst = deferred();
+    const count = User.countDocuments.bind(User);
+    let calls = 0;
+    jest.spyOn(User, 'countDocuments').mockImplementation(async (...args) => {
+      const value = await count(...args);
+      if (++calls === 1) { firstCount.resolve(); await releaseFirst.promise; }
+      return value;
+    });
+    const first = request(app).patch(`/api/admin/users/${bob._id}/role`)
+      .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'user' }).then((res) => res);
+    await firstCount.promise;
+    let second;
+    try {
+      const pending = operation === 'demote'
+        ? request(app).patch(`/api/admin/users/${alice._id}/role`).send({ role: 'user' })
+        : request(app).delete(`/api/admin/users/${alice._id}`);
+      second = await pending.set('Authorization', `Bearer ${issueSessionToken(bob)}`);
+    } finally { releaseFirst.resolve(); }
+    const firstResult = await first;
+    expect(firstResult.status).toBe(200);
+    expect({ secondStatus: second.status, remainingAdmins: await count({ role: 'admin' }) })
+      .toEqual({ secondStatus: 503, remainingAdmins: 1 });
+  });
+  it.each(['demote', 'delete', 'ban'])('never steals an abandoned lock for %s', async (operation) => {
+    const [alice, bob] = await Promise.all([makeAdmin('lock-alice'), makeAdmin('lock-bob')]);
+    const Lock = require('../../models/AdminUserMutationLock');
+    await Lock.create({ _id: 'global-admin-user-mutations', owner: 'stopped-worker', acquiredAt: new Date('2000-01-01') });
+    const pending = operation === 'demote'
+      ? request(app).patch(`/api/admin/users/${bob._id}/role`).send({ role: 'user' })
+      : operation === 'delete' ? request(app).delete(`/api/admin/users/${bob._id}`)
+        : request(app).patch(`/api/admin/users/${bob._id}/ban`).send({ banned: true });
+    const res = await pending.set('Authorization', `Bearer ${issueSessionToken(alice)}`);
+    expect(res.status).toBe(503); expect(res.body.code).toBe('ADMIN_MUTATION_BUSY');
+    expect((await Lock.findById('global-admin-user-mutations')).owner).toBe('stopped-worker');
+    expect(await User.countDocuments({ role: 'admin' })).toBe(2);
+  });
+
+  it('rechecks actor authorization and only releases a known pre-write refusal', async () => {
+    const alice = await makeAdmin('stale-alice');
+    const { withAdminUserMutation } = require('../../services/adminUserMutationService');
+    const Lock = require('../../models/AdminUserMutationLock');
+    const operation = jest.fn();
+    await User.updateOne({ _id: alice._id }, { $set: { role: 'user' } });
+    await expect(withAdminUserMutation(String(alice._id), operation)).rejects.toMatchObject({ status: 403 });
+    expect(operation).not.toHaveBeenCalled();
+    expect(await Lock.countDocuments()).toBe(0);
+    await User.updateOne({ _id: alice._id }, { $set: { role: 'admin' } });
+    await expect(withAdminUserMutation(String(alice._id), async (beginWrite) => { beginWrite(); throw new Error('write failure'); }))
+      .rejects.toThrow('write failure');
+    expect(await Lock.countDocuments()).toBe(1);
+    await expect(withAdminUserMutation(String(alice._id), async () => 'next writer'))
+      .rejects.toMatchObject({ code: 'ADMIN_MUTATION_BUSY' });
+  });
+
+  it.each(['actor', 'count'])('releases the lock after a pre-write %s read fails', async (stage) => {
+    const [alice, bob] = await Promise.all([makeAdmin(`read-${stage}-alice`), makeAdmin(`read-${stage}-bob`)]);
+    const { withAdminUserMutation } = require('../../services/adminUserMutationService');
+    const Lock = require('../../models/AdminUserMutationLock');
+    if (stage === 'actor') {
+      jest.spyOn(User, 'findById').mockRejectedValueOnce(new Error('actor read unavailable'));
+      await expect(withAdminUserMutation(String(alice._id), jest.fn())).rejects.toThrow('actor read unavailable');
+    } else {
+      jest.spyOn(User, 'countDocuments').mockRejectedValueOnce(new Error('count unavailable'));
+      const failed = await request(app).patch(`/api/admin/users/${bob._id}/role`)
+        .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'user' });
+      expect(failed.status).toBe(500);
+    }
+    expect(await Lock.countDocuments()).toBe(0);
+    const retry = await request(app).patch(`/api/admin/users/${bob._id}/role`)
+      .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'user' });
+    expect(retry.status).toBe(200);
+    expect(await User.countDocuments({ role: 'admin' })).toBe(1);
+  });
+
+  it.each(['role', 'ban', 'delete'])('retains the lock if the %s handler write rejects', async (kind) => {
+    const [alice, bob] = await Promise.all([makeAdmin(`write-${kind}-alice`), makeAdmin(`write-${kind}-bob`)]);
+    const Lock = require('../../models/AdminUserMutationLock');
+    if (kind === 'ban') await User.updateOne({ _id: bob._id }, { $set: { role: 'user' } });
+    const write = jest.spyOn(User.prototype, kind === 'delete' ? 'deleteOne' : 'save')
+      .mockRejectedValueOnce(new Error('write outcome unknown'));
+    try {
+      const pending = kind === 'delete' ? request(app).delete(`/api/admin/users/${bob._id}`)
+        : request(app).patch(`/api/admin/users/${bob._id}/${kind}`)
+          .send(kind === 'role' ? { role: 'user' } : { banned: true });
+      const failed = await pending.set('Authorization', `Bearer ${issueSessionToken(alice)}`);
+      expect(failed.status).toBe(500);
+    } finally { write.mockRestore(); }
+    expect(await Lock.countDocuments()).toBe(1);
+  });
+
+  it('does not send success before releasing the lock, and fails closed if release fails', async () => {
+    const [alice, bob] = await Promise.all([makeAdmin('release-alice'), makeAdmin('release-bob')]);
+    const Lock = require('../../models/AdminUserMutationLock');
+    jest.spyOn(Lock, 'deleteOne').mockRejectedValueOnce(new Error('release unavailable'));
+    const res = await request(app).patch(`/api/admin/users/${bob._id}/role`)
+      .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'user' });
+    expect(res.status).toBe(500);
+    expect(await Lock.countDocuments()).toBe(1);
+    const retry = await request(app).patch(`/api/admin/users/${bob._id}/role`)
+      .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'user' });
+    expect(retry.status).toBe(503);
+    expect(await User.countDocuments({ role: 'admin' })).toBe(1);
+  });
+
+  it('serializes ban with a concurrent promotion', async () => {
+    const alice = await makeAdmin('promotion-alice');
+    const bob = await User.create({ username: 'promotion-bob', email: 'promotion-bob@example.test',
+      password: 'Role-concurrency-test-123!', role: 'user' });
+    const writing = deferred(); const resume = deferred();
+    const save = User.prototype.save;
+    jest.spyOn(User.prototype, 'save').mockImplementation(async function (...args) {
+      if (String(this._id) === String(bob._id) && this.role === 'admin') {
+        writing.resolve(); await resume.promise;
+      }
+      return save.apply(this, args);
+    });
+    const first = request(app).patch(`/api/admin/users/${bob._id}/role`)
+      .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'admin' }).then((res) => res);
+    await writing.promise;
+    let second;
+    try {
+      second = await request(app).patch(`/api/admin/users/${bob._id}/ban`)
+        .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ banned: true });
+    } finally { resume.resolve(); }
+    expect((await first).status).toBe(200); expect(second.status).toBe(503);
+    const target = await User.findById(bob._id);
+    expect(target.role).toBe('admin'); expect(target.banned).not.toBe(true);
+  });
+
+  it('retains the lock when a rejected operation can still finish its write later', async () => {
+    const [alice, bob] = await Promise.all([makeAdmin('uncertain-alice'), makeAdmin('uncertain-bob')]);
+    const { withAdminUserMutation } = require('../../services/adminUserMutationService');
+    const resume = deferred(); let late;
+    await expect(withAdminUserMutation(String(alice._id), async (beginWrite) => {
+      beginWrite();
+      late = resume.promise.then(() => User.updateOne({ _id: bob._id }, { $set: { role: 'user' } }));
+      throw new Error('unknown write outcome');
+    })).rejects.toThrow('unknown write outcome');
+    let second;
+    try {
+      second = await request(app).patch(`/api/admin/users/${alice._id}/role`)
+        .set('Authorization', `Bearer ${issueSessionToken(bob)}`).send({ role: 'user' });
+    } finally { resume.resolve(); await late; }
+    expect({ secondStatus: second.status, remainingAdmins: await User.countDocuments({ role: 'admin' }) })
+      .toEqual({ secondStatus: 503, remainingAdmins: 1 });
+  });
+
+  it('cannot release a replacement owner’s lock', async () => {
+    const alice = await makeAdmin('owner-alice');
+    const Lock = require('../../models/AdminUserMutationLock');
+    const { withAdminUserMutation } = require('../../services/adminUserMutationService');
+    await expect(withAdminUserMutation(String(alice._id), async () => {
+      await Lock.updateOne({ _id: 'global-admin-user-mutations' }, { $set: { owner: 'replacement' } });
+    })).rejects.toMatchObject({ code: 'ADMIN_MUTATION_LOCK_LOST' });
+    expect((await Lock.findById('global-admin-user-mutations')).owner).toBe('replacement');
+  });
+
+  it('rejects a malformed target before any database write without leaving a lock', async () => {
+    const alice = await makeAdmin('invalid-target-alice');
+    const Lock = require('../../models/AdminUserMutationLock');
+    const res = await request(app).patch('/api/admin/users/not-an-id/role')
+      .set('Authorization', `Bearer ${issueSessionToken(alice)}`).send({ role: 'user' });
+    expect(res.status).toBe(400); expect(await Lock.countDocuments()).toBe(0);
+  });
+
+});

backend/__tests__/unit/routes/admin.users.test.js

@@ -1,3 +1,10 @@
+// This fixture stubs User and auth; real lock/authorization behavior is tested
+// across Mongo in service/adminRoleConcurrency.test.js.
+jest.mock('../../../services/adminUserMutationService', () => ({
+  withAdminUserMutation: (_actor, operation) => operation(() => {}),
+  AdminUserMutationError: class extends Error {},
+}));
+
 jest.mock('../../../middleware/auth', () => (req, _res, next) => {
   req.user = req.user || { id: 'admin-1' };
   req.userId = req.user.id;
@@ -105,12 +112,12 @@ describe('admin users routes', () => {
   it('deletes a non-bot user', async () => {
     const handler = getRouteHandler('/:userId', 'delete');
     const req = {
-      params: { userId: 'u2' },
+      params: { userId: '000000000000000000000002' },
       user: { id: 'admin-1' },
     };
     const res = createRes();
     const target = {
-      _id: 'u2',
+      _id: '000000000000000000000002',
       role: 'user',
       isBot: false,
       deleteOne: jest.fn().mockResolvedValue(true),
@@ -126,13 +133,13 @@ describe('admin users routes', () => {
   it('updates a user role', async () => {
     const handler = getRouteHandler('/:userId/role', 'patch');
     const req = {
-      params: { userId: 'u2' },
+      params: { userId: '000000000000000000000002' },
       body: { role: 'admin' },
       user: { id: 'admin-1' },
     };
     const res = createRes();
     const target = {
-      _id: 'u2',
+      _id: '000000000000000000000002',
       username: 'bob',
       email: 'b@example.com',
       role: 'user',
@@ -148,7 +155,7 @@ describe('admin users routes', () => {
     expect(res.json).toHaveBeenCalledWith(
       expect.objectContaining({
         message: 'User role updated successfully',
-        user: expect.objectContaining({ id: 'u2', role: 'admin' }),
+        user: expect.objectContaining({ id: '000000000000000000000002', role: 'admin' }),
       }),
     );
   });
@@ -354,9 +361,9 @@ describe('admin users routes', () => {
     it('bans a non-admin user with a reason', async () => {
       const handler = getRouteHandler('/:userId/ban', 'patch');
       const save = jest.fn().mockResolvedValue(undefined);
-      const target = { _id: 'u2', username: 'mallory', role: 'user', isBot: false, save };
+      const target = { _id: '000000000000000000000002', username: 'mallory', role: 'user', isBot: false, save };
       User.findById.mockResolvedValue(target);
-      const req = { params: { userId: 'u2' }, body: { banned: true, reason: 'spam' }, user: { id: 'admin1' } };
+      const req = { params: { userId: '000000000000000000000002' }, body: { banned: true, reason: 'spam' }, user: { id: 'admin1' } };
       const res = createRes();
 
       await handler(req, res);
@@ -373,9 +380,9 @@ describe('admin users routes', () => {
     it('unbans and clears the reason', async () => {
       const handler = getRouteHandler('/:userId/ban', 'patch');
       const save = jest.fn().mockResolvedValue(undefined);
-      const target = { _id: 'u2', username: 'mallory', role: 'user', isBot: false, banned: true, banReason: 'spam', save };
+      const target = { _id: '000000000000000000000002', username: 'mallory', role: 'user', isBot: false, banned: true, banReason: 'spam', save };
       User.findById.mockResolvedValue(target);
-      const req = { params: { userId: 'u2' }, body: { banned: false }, user: { id: 'admin1' } };
+      const req = { params: { userId: '000000000000000000000002' }, body: { banned: false }, user: { id: 'admin1' } };
       const res = createRes();
 
       await handler(req, res);
@@ -387,8 +394,8 @@ describe('admin users routes', () => {
 
     it('refuses to ban an admin account', async () => {
       const handler = getRouteHandler('/:userId/ban', 'patch');
-      User.findById.mockResolvedValue({ _id: 'a2', role: 'admin', isBot: false, save: jest.fn() });
-      const req = { params: { userId: 'a2' }, body: { banned: true }, user: { id: 'admin1' } };
+      User.findById.mockResolvedValue({ _id: '0000000000000000000000a2', role: 'admin', isBot: false, save: jest.fn() });
+      const req = { params: { userId: '0000000000000000000000a2' }, body: { banned: true }, user: { id: 'admin1' } };
       const res = createRes();
 
       await handler(req, res);

backend/__tests__/unit/routes/routeRateLimitGuard.baseline.json

@@ -10,11 +10,9 @@
   "routes/admin/globalIntegrations.ts POST /sync [unlimited]",
   "routes/admin/globalIntegrations.ts POST /x/oauth/start [unlimited]",
   "routes/admin/globalIntegrations.ts POST /x/test [unlimited]",
-  "routes/admin/users.ts DELETE /:userId [unlimited]",
   "routes/admin/users.ts GET / [unlimited]",
   "routes/admin/users.ts GET /invitations [unlimited]",
   "routes/admin/users.ts GET /waitlist [unlimited]",
-  "routes/admin/users.ts PATCH /:userId/role [unlimited]",
   "routes/admin/users.ts PATCH /waitlist/:requestId [unlimited]",
   "routes/admin/users.ts POST /invitations [unlimited]",
   "routes/admin/users.ts POST /invitations/:invitationId/revoke [unlimited]",

backend/__tests__/unit/services/syntheticAccountPurgeService.test.js

@@ -75,6 +75,55 @@ describe('syntheticAccountPurgeService', () => {
     expect(isSyntheticAccount({ username: 'eng-smoke-journey-1015124709', email: 'a@ops.commonly.invalid', isBot: true })).toBe(false);
   });
 
+  it('preserves a synthetic account promoted to administrator, including its data', async () => {
+    const { human, pod, scout } = await syntheticAccount();
+    await User.updateOne({ _id: human._id }, { $set: { role: 'admin' } });
+    const pool = fakePool();
+    const report = await purgeSyntheticAccounts({ apply: true, now: NOW, pool });
+    expect(report.accounts).toHaveLength(0);
+    expect(await User.findById(human._id)).not.toBeNull();
+    expect(await User.findById(scout._id)).not.toBeNull();
+    expect(await Pod.findById(pod._id)).not.toBeNull();
+    expect(pool.statements).toHaveLength(0);
+  });
+
+  it('uses the administrator mutation lock before an applying purge', async () => {
+    const { human } = await syntheticAccount();
+    const { withAdminUserMutationLock } = require('../../../services/adminUserMutationService');
+    await withAdminUserMutationLock(async () => {
+      await expect(purgeSyntheticAccounts({ apply: true, now: NOW, pool: fakePool() }))
+        .rejects.toMatchObject({ code: 'ADMIN_MUTATION_BUSY' });
+    });
+    expect(await User.findById(human._id)).not.toBeNull();
+  });
+
+  it('releases the applying lock if candidate selection fails before writes', async () => {
+    const Lock = require('../../../models/AdminUserMutationLock');
+    const read = jest.spyOn(User, 'find').mockReturnValueOnce({
+      select: () => ({ lean: async () => { throw new Error('selection unavailable'); } }),
+    });
+    try {
+      await expect(purgeSyntheticAccounts({ apply: true, now: NOW, pool: fakePool() }))
+        .rejects.toThrow('selection unavailable');
+    } finally { read.mockRestore(); }
+    expect(await Lock.countDocuments()).toBe(0);
+    await expect(purgeSyntheticAccounts({ apply: true, now: NOW, pool: fakePool() }))
+      .resolves.toMatchObject({ examined: 0 });
+  });
+
+  it('retains the applying lock after its first cleanup write rejects', async () => {
+    await syntheticAccount();
+    const Lock = require('../../../models/AdminUserMutationLock');
+    const write = jest.spyOn(Task, 'deleteMany').mockRejectedValueOnce(new Error('cleanup outcome unknown'));
+    try {
+      await expect(purgeSyntheticAccounts({ apply: true, now: NOW, pool: fakePool() }))
+        .rejects.toThrow('cleanup outcome unknown');
+    } finally { write.mockRestore(); }
+    expect(await Lock.countDocuments()).toBe(1);
+    await expect(purgeSyntheticAccounts({ apply: true, now: NOW, pool: fakePool() }))
+      .rejects.toMatchObject({ code: 'ADMIN_MUTATION_BUSY' });
+  });
+
   it('a dry run counts the footprint and deletes nothing', async () => {
     const { human, pod } = await syntheticAccount();
     const report = await purgeSyntheticAccounts({ now: NOW, pool: fakePool() });

backend/models/AdminUserMutationLock.ts

@@ -0,0 +1,14 @@
+import mongoose, { Schema } from 'mongoose';
+
+// Intentionally no TTL or takeover deadline: a stopped process can resume and
+// still write User rows. Expiring its lock would admit a concurrent writer.
+const schema = new Schema({
+  _id: { type: String, required: true },
+  owner: { type: String, required: true },
+  acquiredAt: { type: Date, required: true },
+}, { collection: 'admin_user_mutation_locks' });
+
+export default mongoose.model('AdminUserMutationLock', schema);
+// CJS compat: let require() return the default export directly.
+// eslint-disable-next-line @typescript-eslint/no-require-imports
+module.exports = exports["default"]; Object.assign(module.exports, exports);