fix(mcpd): skip bootstrap tokens on migrate + back-fill ops on existing admins

Two production issues caught running the wizard end-to-end:

1. `mcpctl migrate secrets --from default --to bao` listed `bao-creds` as a
   candidate — the very token that lets mcpd reach bao. Moving it would
   brick the auth chain (destination backend needs its own bootstrap token
   to read its own bootstrap token). Fix: SecretMigrateService now calls
   backends.list() and filters out any Secret whose name matches ANY
   SecretBackend's `config.tokenSecretRef.name`. dryRun mirrors the same
   filter so candidates match reality. `--names` explicitly bypasses the
   filter for operators who really mean it.

2. Initial rotation in the wizard 403'd because the global RBAC hook
   demands the `rotate-secretbackend` operation, which wasn't in
   bootstrap-admin — migrateAdminRole only added ops when processing a
   legacy `role: admin` entry, so already-migrated admin rows missed every
   new op added after their initial migration. Fix: migrateAdminRole now
   also runs a back-fill pass on rows that look admin-equivalent (have both
   `edit:*` and `run:*`), appending any missing op from ADMIN_OPS. Writes
   only when something actually changed, so restarts stay quiet. Same path
   also retroactively grants `migrate-secrets` which had the same problem
   yesterday.

Tests: 4 new migrate-service cases (bootstrap filter on/off, dryRun parity,
--names bypass). Full suite 1889/1889.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/mcpd/src/main.ts

@@ -213,20 +213,38 @@ function mapUrlToPermission(method: string, url: string): PermissionCheck {
   return check;
 }
 
+/** Operations an admin-equivalent user should always be able to run. */
+const ADMIN_OPS = [
+  'impersonate', 'logs', 'backup', 'restore', 'audit-purge',
+  'migrate-secrets', 'rotate-secretbackend',
+] as const;
+
 /**
- * Migrate legacy 'admin' role bindings → granular roles.
- * Old format:  { role: 'admin', resource: '*' }
- * New format:  { role: 'edit', resource: '*' }, { role: 'run', resource: '*' },
- *              plus operation bindings for impersonate, logs, backup, restore, audit-purge
+ * Keep admin-equivalent RBAC definitions in sync with the current set of
+ * required operations. Handles two cases:
+ *
+ *   1. Legacy migration — any binding with `role: admin` is split into
+ *      `{role: edit, resource}` + `{role: run, resource}`, plus every op in
+ *      `ADMIN_OPS` is added (for `admin:*`).
+ *   2. Ongoing back-fill — a row that ALREADY has `edit:*` AND `run:*` is
+ *      treated as admin-equivalent: any op in `ADMIN_OPS` that isn't bound
+ *      yet is appended. Runs every startup so new releases that introduce a
+ *      new operation don't silently 403 existing admin users.
+ *
+ * Idempotent — writes only when something actually changed.
  */
 async function migrateAdminRole(rbacRepo: InstanceType<typeof RbacDefinitionRepository>): Promise<void> {
   const definitions = await rbacRepo.findAll();
   for (const def of definitions) {
     const bindings = def.roleBindings as Array<Record<string, unknown>>;
     const hasAdminRole = bindings.some((b) => b['role'] === 'admin');
-    if (!hasAdminRole) continue;
+    const hasEditWildcard = bindings.some((b) => b['role'] === 'edit' && b['resource'] === '*');
+    const hasRunWildcard = bindings.some((b) => b['role'] === 'run' && b['resource'] === '*');
+    const isAlreadyAdminEquivalent = hasEditWildcard && hasRunWildcard;
 
-    // Replace admin bindings with granular equivalents
+    if (!hasAdminRole && !isAlreadyAdminEquivalent) continue;
+
+    // Start from existing bindings, splitting any legacy `admin` roles.
     const newBindings: Array<Record<string, string>> = [];
     for (const b of bindings) {
       if (b['role'] === 'admin') {
@@ -237,20 +255,32 @@ async function migrateAdminRole(rbacRepo: InstanceType<typeof RbacDefinitionRepo
         newBindings.push(b as Record<string, string>);
       }
     }
-    // Add operation bindings (idempotent — only for wildcard admin)
-    const hasWildcard = bindings.some((b) => b['role'] === 'admin' && b['resource'] === '*');
-    if (hasWildcard) {
-      const ops = ['impersonate', 'logs', 'backup', 'restore', 'audit-purge', 'migrate-secrets', 'rotate-secretbackend'];
-      for (const op of ops) {
+
+    // Back-fill any missing ops. `admin:*` qualifies, and so does the
+    // already-migrated {edit:*, run:*} pair — both are wildcard-admin.
+    const hasWildcardAdmin = bindings.some((b) => b['role'] === 'admin' && b['resource'] === '*');
+    const shouldAddOps = hasWildcardAdmin || isAlreadyAdminEquivalent;
+    let opsAdded = 0;
+    if (shouldAddOps) {
+      for (const op of ADMIN_OPS) {
         if (!newBindings.some((b) => b['action'] === op)) {
           newBindings.push({ role: 'run', action: op });
+          opsAdded++;
         }
       }
     }
 
+    // Only write if something actually changed — avoids noisy no-op startups.
+    const changed = hasAdminRole || opsAdded > 0;
+    if (!changed) continue;
+
     await rbacRepo.update(def.id, { roleBindings: newBindings as UpdateRbacDefinitionInput['roleBindings'] });
     // eslint-disable-next-line no-console
-    console.log(`mcpd: migrated RBAC '${def.name}' from admin → granular roles`);
+    if (hasAdminRole) {
+      console.log(`mcpd: migrated RBAC '${def.name}' from admin → granular roles (+${String(opsAdded)} ops)`);
+    } else {
+      console.log(`mcpd: back-filled ${String(opsAdded)} op binding(s) on admin-equivalent RBAC '${def.name}'`);
+    }
   }
 }
 

src/mcpd/src/services/secret-migrate.service.ts

@@ -55,7 +55,26 @@ export class SecretMigrateService {
       secrets = secrets.filter((s) => wanted.has(s.name));
     }
 
+    // Protect bootstrap tokens: any Secret named in a SecretBackend's
+    // `config.tokenSecretRef.name` is the credential that lets mcpd reach
+    // that backend at all. Moving it off the plaintext backend (the trust
+    // root) breaks the chain — the destination backend would need its own
+    // token to read its own token. Skip silently unless the user explicitly
+    // names it via --names.
     const result: MigrateResult = { migrated: [], skipped: [], failed: [] };
+    if (!(opts.names && opts.names.length > 0)) {
+      const bootstrapNames = await this.collectBootstrapTokenNames();
+      const kept: typeof secrets = [];
+      for (const s of secrets) {
+        if (bootstrapNames.has(s.name)) {
+          result.skipped.push({ name: s.name, reason: 'bootstrap token for a SecretBackend — kept on source backend by design' });
+        } else {
+          kept.push(s);
+        }
+      }
+      secrets = kept;
+    }
+
     for (const secret of secrets) {
       try {
         // Skip if somehow already on destination (re-run safety).
@@ -103,9 +122,31 @@ export class SecretMigrateService {
     if (opts.names && opts.names.length > 0) {
       const wanted = new Set(opts.names);
       secrets = secrets.filter((s) => wanted.has(s.name));
+    } else {
+      // Same bootstrap-token protection as `migrate()` — dry-run MUST report
+      // the same candidate list the real run would move.
+      const bootstrapNames = await this.collectBootstrapTokenNames();
+      secrets = secrets.filter((s) => !bootstrapNames.has(s.name));
     }
     return secrets;
   }
+
+  /**
+   * Return the set of Secret names that are referenced by any SecretBackend's
+   * `config.tokenSecretRef.name`. These are the bootstrap credentials that let
+   * mcpd reach the backend itself; moving them anywhere off their current
+   * backend would break the auth chain.
+   */
+  private async collectBootstrapTokenNames(): Promise<Set<string>> {
+    const all = await this.backends.list();
+    const names = new Set<string>();
+    for (const b of all) {
+      const cfg = b.config as { tokenSecretRef?: { name?: string } } | null;
+      const name = cfg?.tokenSecretRef?.name;
+      if (typeof name === 'string' && name !== '') names.add(name);
+    }
+    return names;
+  }
 }
 
 export interface SecretMigrateRouteDeps {

src/mcpd/tests/secret-migrate-service.test.ts

@@ -0,0 +1,146 @@
+import { describe, it, expect, vi } from 'vitest';
+import { SecretMigrateService } from '../src/services/secret-migrate.service.js';
+import type { Secret, SecretBackend } from '@prisma/client';
+
+function makeSecret(overrides: Partial<Secret>): Secret {
+  return {
+    id: 'sec',
+    name: 'sec',
+    backendId: 'b-plain',
+    data: {} as Secret['data'],
+    externalRef: '',
+    version: 1,
+    createdAt: new Date(),
+    updatedAt: new Date(),
+    ...overrides,
+  };
+}
+
+function makeBackend(overrides: Partial<SecretBackend>): SecretBackend {
+  return {
+    id: overrides.id ?? 'b',
+    name: overrides.name ?? 'b',
+    type: overrides.type ?? 'plaintext',
+    config: overrides.config ?? ({} as SecretBackend['config']),
+    tokenMeta: overrides.tokenMeta ?? ({} as SecretBackend['tokenMeta']),
+    isDefault: overrides.isDefault ?? false,
+    description: overrides.description ?? '',
+    version: 1,
+    createdAt: new Date(),
+    updatedAt: new Date(),
+  };
+}
+
+/**
+ * Build a service with in-memory state. Mocks the secret repo + backend
+ * service enough to exercise the bootstrap-token filter.
+ */
+function makeService(state: {
+  sourceName: string;
+  destName: string;
+  sourceBackend: SecretBackend;
+  destBackend: SecretBackend;
+  allBackends: SecretBackend[];
+  secretsOnSource: Secret[];
+}) {
+  const secretRepo = {
+    findByBackend: vi.fn(async (backendId: string) => {
+      return state.secretsOnSource.filter((s) => s.backendId === backendId);
+    }),
+    update: vi.fn(async () => ({}) as Secret),
+  };
+  const backends = {
+    list: vi.fn(async () => state.allBackends),
+    getByName: vi.fn(async (name: string) => {
+      if (name === state.sourceBackend.name) return state.sourceBackend;
+      if (name === state.destBackend.name) return state.destBackend;
+      throw new Error(`unknown backend: ${name}`);
+    }),
+    driverFor: vi.fn(() => ({
+      read: vi.fn(async () => ({ k: 'v' })),
+      write: vi.fn(async () => ({ externalRef: 'ext', storedData: {} })),
+      delete: vi.fn(async () => {}),
+    })),
+  };
+  return new SecretMigrateService(
+    secretRepo as never,
+    backends as never,
+  );
+}
+
+describe('SecretMigrateService — bootstrap-token protection', () => {
+  const sourceBackend = makeBackend({ id: 'b-plain', name: 'default', type: 'plaintext' });
+  const destBackend = makeBackend({
+    id: 'b-bao',
+    name: 'bao',
+    type: 'openbao',
+    config: { tokenSecretRef: { name: 'bao-creds', key: 'token' } } as unknown as SecretBackend['config'],
+  });
+
+  const secrets = [
+    makeSecret({ id: 's1', name: 'grafana-creds' }),
+    makeSecret({ id: 's2', name: 'bao-creds' }),          // ← bootstrap token for `bao`
+    makeSecret({ id: 's3', name: 'ha-creds' }),
+  ];
+
+  it('migrate() without --names skips the bootstrap-token secret', async () => {
+    const svc = makeService({
+      sourceName: 'default',
+      destName: 'bao',
+      sourceBackend, destBackend,
+      allBackends: [sourceBackend, destBackend],
+      secretsOnSource: secrets,
+    });
+    const result = await svc.migrate({ from: 'default', to: 'bao' });
+
+    const migratedNames = result.migrated.map((m) => m.name).sort();
+    expect(migratedNames).toEqual(['grafana-creds', 'ha-creds']);
+
+    const skippedBootstrap = result.skipped.find((s) => s.name === 'bao-creds');
+    expect(skippedBootstrap, 'bao-creds must be in skipped with a bootstrap-token reason').toBeDefined();
+    expect(skippedBootstrap!.reason).toMatch(/bootstrap token/i);
+  });
+
+  it('migrate() WITH --names including the bootstrap token still migrates it (explicit override)', async () => {
+    const svc = makeService({
+      sourceName: 'default',
+      destName: 'bao',
+      sourceBackend, destBackend,
+      allBackends: [sourceBackend, destBackend],
+      secretsOnSource: secrets,
+    });
+    const result = await svc.migrate({ from: 'default', to: 'bao', names: ['bao-creds'] });
+
+    const migratedNames = result.migrated.map((m) => m.name);
+    expect(migratedNames).toEqual(['bao-creds']);
+    // No bootstrap-skip entry when the user explicitly asked for it
+    expect(result.skipped.find((s) => s.reason.includes('bootstrap'))).toBeUndefined();
+  });
+
+  it('dryRun() matches migrate() — also filters bootstrap tokens by default', async () => {
+    const svc = makeService({
+      sourceName: 'default',
+      destName: 'bao',
+      sourceBackend, destBackend,
+      allBackends: [sourceBackend, destBackend],
+      secretsOnSource: secrets,
+    });
+    const candidates = await svc.dryRun({ from: 'default', to: 'bao' });
+    const names = candidates.map((c) => c.name).sort();
+    expect(names).toEqual(['grafana-creds', 'ha-creds']);
+    expect(names).not.toContain('bao-creds');
+  });
+
+  it('dryRun() with --names reports exactly the named secrets', async () => {
+    const svc = makeService({
+      sourceName: 'default',
+      destName: 'bao',
+      sourceBackend, destBackend,
+      allBackends: [sourceBackend, destBackend],
+      secretsOnSource: secrets,
+    });
+    const candidates = await svc.dryRun({ from: 'default', to: 'bao', names: ['bao-creds', 'grafana-creds'] });
+    const names = candidates.map((c) => c.name).sort();
+    expect(names).toEqual(['bao-creds', 'grafana-creds']);
+  });
+});