diff --git a/src/mcpd/src/main.ts b/src/mcpd/src/main.ts index 5b2b0df..6067042 100644 --- a/src/mcpd/src/main.ts +++ b/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): Promise { const definitions = await rbacRepo.findAll(); for (const def of definitions) { const bindings = def.roleBindings as Array>; 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> = []; for (const b of bindings) { if (b['role'] === 'admin') { @@ -237,20 +255,32 @@ async function migrateAdminRole(rbacRepo: InstanceType); } } - // 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}'`); + } } } diff --git a/src/mcpd/src/services/secret-migrate.service.ts b/src/mcpd/src/services/secret-migrate.service.ts index 80c27fb..59ba36c 100644 --- a/src/mcpd/src/services/secret-migrate.service.ts +++ b/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> { + const all = await this.backends.list(); + const names = new Set(); + 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 { diff --git a/src/mcpd/tests/secret-migrate-service.test.ts b/src/mcpd/tests/secret-migrate-service.test.ts new file mode 100644 index 0000000..098ef39 --- /dev/null +++ b/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 { + 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 { + 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']); + }); +});