feat(mcpd): pluggable SecretBackend abstraction + OpenBao driver + migrate
All checks were successful
CI/CD / typecheck (pull_request) Successful in 51s
CI/CD / lint (pull_request) Successful in 1m47s
CI/CD / test (pull_request) Successful in 1m3s
CI/CD / smoke (pull_request) Successful in 4m34s
CI/CD / build (pull_request) Successful in 3m50s
CI/CD / publish (pull_request) Has been skipped

Why: API keys live in Postgres as plaintext JSON. A DB read exposes every
credential in the system. Before centralising more secrets (LLM keys, etc.)
we want to be able to point at an external KV store and drop DB access to
sensitive rows.

New model:
- `SecretBackend` resource (CRUD + isDefault invariant) owns how a secret is
  stored. `Secret` gains `backendId` FK and `externalRef`. Reads/writes
  dispatch through a driver.
- `plaintext` driver (near-noop, uses existing Secret.data column) is seeded
  as the `default` row at startup. Acts as trust root / bootstrap.
- `openbao` driver (also HashiCorp Vault KV v2 compatible) talks plain HTTP,
  no SDK dependency. Auth via static token pulled from a plaintext-backed
  `Secret` through the injected SecretRefResolver. Caches resolved token.
- `SecretMigrateService` moves secrets one-at-a-time: read → write dest →
  flip row → best-effort source delete. Interrupted runs are idempotent
  (skips secrets already on destination).

CLI surface:
- `mcpctl create|get|describe|delete secretbackend` + `--default` on create.
- `mcpctl migrate secrets --from X --to Y [--names a,b] [--keep-source] [--dry-run]`
- `apply -f` round-trips secretbackends (yaml/json multi-doc + grouped).
- RBAC: `secretbackends` resource + `run:migrate-secrets` operation.
- Fish + bash completions regenerated.

docs/secret-backends.md covers the OpenBao policy, chicken-and-egg auth flow,
and the migration semantics.

Broke the circular dep (OpenBao needs SecretService to resolve its own token,
SecretService needs SecretBackendService) with a deferred-resolver bridge in
mcpd startup. 11 new driver unit tests; existing env-resolver/secret-route/
backup tests updated for the new service signatures. Full suite: 1792/1792.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michal
2026-04-18 19:29:55 +01:00
parent 6946250090
commit 029c3d5f34
34 changed files with 1686 additions and 138 deletions

View File

@@ -9,6 +9,25 @@ import type { IProjectRepository } from '../src/repositories/project.repository.
import type { IUserRepository } from '../src/repositories/user.repository.js';
import type { IGroupRepository } from '../src/repositories/group.repository.js';
import type { IRbacDefinitionRepository } from '../src/repositories/rbac-definition.repository.js';
import type { SecretService } from '../src/services/secret.service.js';
/**
* Minimal SecretService shim over a mock repo — just the `.create()` / `.update()`
* methods that RestoreService calls. We don't need the backend-dispatch path
* here since the restore happy-path tests don't exercise remote backends.
*/
function mockSecretService(repo: ISecretRepository): SecretService {
return {
create: vi.fn(async (input: unknown) => {
const data = input as { name: string; data: Record<string, string> };
return repo.create({ name: data.name, backendId: 'backend-plaintext', data: data.data, externalRef: '' });
}),
update: vi.fn(async (id: string, input: unknown) => {
const data = input as { data: Record<string, string> };
return repo.update(id, { data: data.data });
}),
} as unknown as SecretService;
}
// Mock data
const mockServers = [
@@ -295,7 +314,7 @@ describe('RestoreService', () => {
(userRepo.findByEmail as ReturnType<typeof vi.fn>).mockResolvedValue(null);
(groupRepo.findByName as ReturnType<typeof vi.fn>).mockResolvedValue(null);
(rbacRepo.findByName as ReturnType<typeof vi.fn>).mockResolvedValue(null);
restoreService = new RestoreService(serverRepo, projectRepo, secretRepo, userRepo, groupRepo, rbacRepo);
restoreService = new RestoreService(serverRepo, projectRepo, secretRepo, mockSecretService(secretRepo), userRepo, groupRepo, rbacRepo);
});
const validBundle = {
@@ -576,7 +595,7 @@ describe('Backup Routes', () => {
(rGroupRepo.findByName as ReturnType<typeof vi.fn>).mockResolvedValue(null);
const rRbacRepo = mockRbacRepo();
(rRbacRepo.findByName as ReturnType<typeof vi.fn>).mockResolvedValue(null);
restoreService = new RestoreService(rSRepo, rPrRepo, rSecRepo, rUserRepo, rGroupRepo, rRbacRepo);
restoreService = new RestoreService(rSRepo, rPrRepo, rSecRepo, mockSecretService(rSecRepo), rUserRepo, rGroupRepo, rRbacRepo);
});
async function buildApp() {

View File

@@ -1,6 +1,5 @@
import { describe, it, expect, vi } from 'vitest';
import { resolveServerEnv } from '../src/services/env-resolver.js';
import type { ISecretRepository } from '../src/repositories/interfaces.js';
import { resolveServerEnv, type SecretResolver } from '../src/services/env-resolver.js';
import type { McpServer } from '@prisma/client';
function makeServer(env: unknown[]): McpServer {
@@ -23,18 +22,16 @@ function makeServer(env: unknown[]): McpServer {
} as McpServer;
}
function mockSecretRepo(secrets: Record<string, Record<string, string>>): ISecretRepository {
/** A SecretResolver backed by a {secretName: {key: value}} map. */
function mockResolver(secrets: Record<string, Record<string, string>>): SecretResolver {
return {
findAll: vi.fn(async () => []),
findById: vi.fn(async () => null),
findByName: vi.fn(async (name: string) => {
resolve: vi.fn(async (name: string, key: string): Promise<string> => {
const data = secrets[name];
if (!data) return null;
return { id: `sec-${name}`, name, data, version: 1, createdAt: new Date(), updatedAt: new Date() };
if (!data) throw new Error(`Secret '${name}' not found`);
const value = data[key];
if (value === undefined) throw new Error(`Key '${key}' not found in secret '${name}'`);
return value;
}),
create: vi.fn(async () => ({} as never)),
update: vi.fn(async () => ({} as never)),
delete: vi.fn(async () => {}),
};
}
@@ -44,8 +41,7 @@ describe('resolveServerEnv', () => {
{ name: 'FOO', value: 'bar' },
{ name: 'BAZ', value: 'qux' },
]);
const repo = mockSecretRepo({});
const result = await resolveServerEnv(server, repo);
const result = await resolveServerEnv(server, mockResolver({}));
expect(result).toEqual({ FOO: 'bar', BAZ: 'qux' });
});
@@ -53,10 +49,8 @@ describe('resolveServerEnv', () => {
const server = makeServer([
{ name: 'TOKEN', valueFrom: { secretRef: { name: 'ha-creds', key: 'HOMEASSISTANT_TOKEN' } } },
]);
const repo = mockSecretRepo({
'ha-creds': { HOMEASSISTANT_TOKEN: 'secret-token-123' },
});
const result = await resolveServerEnv(server, repo);
const resolver = mockResolver({ 'ha-creds': { HOMEASSISTANT_TOKEN: 'secret-token-123' } });
const result = await resolveServerEnv(server, resolver);
expect(result).toEqual({ TOKEN: 'secret-token-123' });
});
@@ -65,48 +59,42 @@ describe('resolveServerEnv', () => {
{ name: 'URL', value: 'https://ha.local' },
{ name: 'TOKEN', valueFrom: { secretRef: { name: 'creds', key: 'TOKEN' } } },
]);
const repo = mockSecretRepo({
creds: { TOKEN: 'my-token' },
});
const result = await resolveServerEnv(server, repo);
const resolver = mockResolver({ creds: { TOKEN: 'my-token' } });
const result = await resolveServerEnv(server, resolver);
expect(result).toEqual({ URL: 'https://ha.local', TOKEN: 'my-token' });
});
it('caches secret lookups', async () => {
it('calls the resolver once per distinct ref', async () => {
const server = makeServer([
{ name: 'A', valueFrom: { secretRef: { name: 'shared', key: 'KEY_A' } } },
{ name: 'B', valueFrom: { secretRef: { name: 'shared', key: 'KEY_B' } } },
]);
const repo = mockSecretRepo({
shared: { KEY_A: 'val-a', KEY_B: 'val-b' },
});
const result = await resolveServerEnv(server, repo);
const resolver = mockResolver({ shared: { KEY_A: 'val-a', KEY_B: 'val-b' } });
const result = await resolveServerEnv(server, resolver);
expect(result).toEqual({ A: 'val-a', B: 'val-b' });
expect(repo.findByName).toHaveBeenCalledTimes(1);
// Resolver is called per-entry now — caching moved to the SecretService layer,
// which is where downstream drivers can be hit at most once per (name, key) pair.
expect(resolver.resolve).toHaveBeenCalledTimes(2);
});
it('throws when secret not found', async () => {
const server = makeServer([
{ name: 'TOKEN', valueFrom: { secretRef: { name: 'missing', key: 'TOKEN' } } },
]);
const repo = mockSecretRepo({});
await expect(resolveServerEnv(server, repo)).rejects.toThrow("Secret 'missing' not found");
await expect(resolveServerEnv(server, mockResolver({}))).rejects.toThrow(/Secret 'missing' not found/);
});
it('throws when secret key not found', async () => {
const server = makeServer([
{ name: 'TOKEN', valueFrom: { secretRef: { name: 'creds', key: 'NONEXISTENT' } } },
]);
const repo = mockSecretRepo({
creds: { OTHER_KEY: 'val' },
});
await expect(resolveServerEnv(server, repo)).rejects.toThrow("Key 'NONEXISTENT' not found in secret 'creds'");
const resolver = mockResolver({ creds: { OTHER_KEY: 'val' } });
await expect(resolveServerEnv(server, resolver)).rejects.toThrow(/Key 'NONEXISTENT' not found/);
});
it('returns empty map for empty env', async () => {
const server = makeServer([]);
const repo = mockSecretRepo({});
const result = await resolveServerEnv(server, repo);
const result = await resolveServerEnv(server, mockResolver({}));
expect(result).toEqual({});
});
});

View File

@@ -0,0 +1,132 @@
import { describe, it, expect, vi } from 'vitest';
import { PlaintextDriver } from '../src/services/secret-backends/plaintext.js';
import { OpenBaoDriver } from '../src/services/secret-backends/openbao.js';
describe('PlaintextDriver', () => {
const driver = new PlaintextDriver({ listAllPlaintext: async () => [{ name: 'a', data: { k: 'v' } }] });
it('read returns the data passed in', async () => {
const result = await driver.read({ name: 's', externalRef: '', data: { token: 'abc' } });
expect(result).toEqual({ token: 'abc' });
});
it('write returns storedData = input, externalRef = empty', async () => {
const result = await driver.write({ name: 's', data: { k: 'v' } });
expect(result).toEqual({ externalRef: '', storedData: { k: 'v' } });
});
it('list delegates to the injected dep', async () => {
const list = await driver.list();
expect(list).toEqual([{ name: 'a', externalRef: '' }]);
});
it('delete is a no-op', async () => {
await expect(driver.delete({ name: 's', externalRef: '' })).resolves.toBeUndefined();
});
});
describe('OpenBaoDriver', () => {
function makeFetch(responses: Array<{ url: RegExp; status: number; body?: unknown }>): ReturnType<typeof vi.fn> {
return vi.fn(async (url: string | URL, _init?: RequestInit) => {
const urlStr = String(url);
const match = responses.find((r) => r.url.test(urlStr));
if (!match) throw new Error(`unexpected fetch: ${urlStr}`);
return new Response(match.body ? JSON.stringify(match.body) : '', { status: match.status });
});
}
const resolver = { resolve: vi.fn(async () => 'test-vault-token') };
it('write sends POST to .../data/<path> with {data: ...}', async () => {
const fetchFn = makeFetch([{ url: /\/v1\/secret\/data\/mcpctl\/mytoken$/, status: 200 }]);
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: resolver },
);
const result = await driver.write({ name: 'mytoken', data: { api_key: 'secret-xyz' } });
expect(result.externalRef).toBe('secret/mcpctl/mytoken');
expect(result.storedData).toEqual({});
expect(fetchFn).toHaveBeenCalledTimes(1);
const [, init] = fetchFn.mock.calls[0] as [unknown, RequestInit];
expect(init.method).toBe('POST');
expect(JSON.parse(init.body as string)).toEqual({ data: { api_key: 'secret-xyz' } });
const headers = init.headers as Record<string, string>;
expect(headers['X-Vault-Token']).toBe('test-vault-token');
});
it('read returns body.data.data', async () => {
const fetchFn = makeFetch([{
url: /\/v1\/secret\/data\/mcpctl\/mytoken$/,
status: 200,
body: { data: { data: { api_key: 'secret-xyz' } } },
}]);
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: resolver },
);
const result = await driver.read({ name: 'mytoken', externalRef: 'secret/mcpctl/mytoken', data: {} });
expect(result).toEqual({ api_key: 'secret-xyz' });
});
it('read throws when the path 404s', async () => {
const fetchFn = makeFetch([{ url: /\/data\//, status: 404 }]);
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: resolver },
);
await expect(driver.read({ name: 'missing', externalRef: '', data: {} })).rejects.toThrow(/not found/);
});
it('delete swallows 404', async () => {
const fetchFn = makeFetch([{ url: /\/metadata\//, status: 404 }]);
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: resolver },
);
await expect(driver.delete({ name: 'gone', externalRef: '' })).resolves.toBeUndefined();
});
it('list returns names from the metadata LIST call', async () => {
const fetchFn = makeFetch([{
url: /\/v1\/secret\/metadata\/mcpctl\/$/,
status: 200,
body: { data: { keys: ['token1', 'token2', 'sub-folder/'] } },
}]);
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: resolver },
);
const result = await driver.list();
// Sub-folders (trailing slash) are excluded; only leaf keys are returned.
expect(result).toEqual([
{ name: 'token1', externalRef: 'secret/mcpctl/token1' },
{ name: 'token2', externalRef: 'secret/mcpctl/token2' },
]);
});
it('caches the vault token after first resolve', async () => {
const fetchFn = makeFetch([
{ url: /\/v1\/secret\/data\/mcpctl\//, status: 200, body: { data: { data: { k: 'v' } } } },
]);
const singleResolver = { resolve: vi.fn(async () => 'test-vault-token') };
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: singleResolver },
);
await driver.read({ name: 'a', externalRef: '', data: {} });
await driver.read({ name: 'a', externalRef: '', data: {} });
expect(singleResolver.resolve).toHaveBeenCalledTimes(1);
});
it('propagates X-Vault-Namespace when configured', async () => {
const fetchFn = makeFetch([{ url: /\/v1\/secret\/data\/mcpctl\//, status: 200 }]);
const driver = new OpenBaoDriver(
{ url: 'http://bao.example:8200', namespace: 'myteam', tokenSecretRef: { name: 'bao', key: 'token' } },
{ fetch: fetchFn as unknown as typeof fetch, secretRefResolver: resolver },
);
await driver.write({ name: 'x', data: { k: 'v' } });
const [, init] = fetchFn.mock.calls[0] as [unknown, RequestInit];
const headers = init.headers as Record<string, string>;
expect(headers['X-Vault-Namespace']).toBe('myteam');
});
});

View File

@@ -3,43 +3,68 @@ import Fastify from 'fastify';
import type { FastifyInstance } from 'fastify';
import { registerSecretRoutes } from '../src/routes/secrets.js';
import { SecretService } from '../src/services/secret.service.js';
import { SecretBackendService } from '../src/services/secret-backend.service.js';
import { errorHandler } from '../src/middleware/error-handler.js';
import type { ISecretRepository } from '../src/repositories/interfaces.js';
import type { ISecretBackendRepository } from '../src/repositories/secret-backend.repository.js';
import type { SecretBackend } from '@prisma/client';
let app: FastifyInstance;
function mockRepo(): ISecretRepository {
let lastCreated: Record<string, unknown> | null = null;
const PLAINTEXT_BACKEND: SecretBackend = {
id: 'backend-plaintext',
name: 'default',
type: 'plaintext',
config: {},
isDefault: true,
description: '',
version: 1,
createdAt: new Date(),
updatedAt: new Date(),
};
function makeSecret(overrides: Partial<{ id: string; name: string; data: Record<string, string>; externalRef: string; backendId: string }> = {}) {
return {
findAll: vi.fn(async () => [
{ id: '1', name: 'ha-creds', data: { TOKEN: 'abc' }, version: 1, createdAt: new Date(), updatedAt: new Date() },
]),
id: overrides.id ?? 'sec-1',
name: overrides.name ?? 'ha-creds',
backendId: overrides.backendId ?? PLAINTEXT_BACKEND.id,
data: overrides.data ?? { TOKEN: 'abc' },
externalRef: overrides.externalRef ?? '',
version: 1,
createdAt: new Date(),
updatedAt: new Date(),
};
}
function mockRepo(): ISecretRepository {
let lastCreated: ReturnType<typeof makeSecret> | null = null;
return {
findAll: vi.fn(async () => [makeSecret()]),
findById: vi.fn(async (id: string) => {
if (lastCreated && (lastCreated as { id: string }).id === id) return lastCreated as never;
if (lastCreated && lastCreated.id === id) return lastCreated;
return null;
}),
findByName: vi.fn(async () => null),
findByBackend: vi.fn(async () => []),
create: vi.fn(async (data) => {
const secret = {
const secret = makeSecret({
id: 'new-id',
name: data.name,
data: data.data ?? {},
version: 1,
createdAt: new Date(),
updatedAt: new Date(),
};
externalRef: data.externalRef ?? '',
backendId: data.backendId,
});
lastCreated = secret;
return secret;
}),
update: vi.fn(async (id, data) => {
const secret = {
const secret = makeSecret({
id,
name: 'ha-creds',
name: lastCreated?.name ?? 'ha-creds',
data: data.data,
version: 2,
createdAt: new Date(),
updatedAt: new Date(),
};
externalRef: data.externalRef,
backendId: data.backendId ?? PLAINTEXT_BACKEND.id,
});
lastCreated = secret;
return secret;
}),
@@ -47,14 +72,32 @@ function mockRepo(): ISecretRepository {
};
}
function mockBackendRepo(): ISecretBackendRepository {
return {
findAll: vi.fn(async () => [PLAINTEXT_BACKEND]),
findById: vi.fn(async (id) => (id === PLAINTEXT_BACKEND.id ? PLAINTEXT_BACKEND : null)),
findByName: vi.fn(async (name) => (name === PLAINTEXT_BACKEND.name ? PLAINTEXT_BACKEND : null)),
findDefault: vi.fn(async () => PLAINTEXT_BACKEND),
create: vi.fn(async () => PLAINTEXT_BACKEND),
update: vi.fn(async () => PLAINTEXT_BACKEND),
setAsDefault: vi.fn(async () => PLAINTEXT_BACKEND),
delete: vi.fn(async () => {}),
countReferencingSecrets: vi.fn(async () => 0),
};
}
afterEach(async () => {
if (app) await app.close();
});
function createApp(repo: ISecretRepository) {
async function createApp(repo: ISecretRepository) {
app = Fastify({ logger: false });
app.setErrorHandler(errorHandler);
const service = new SecretService(repo);
const backends = new SecretBackendService(mockBackendRepo(), {
plaintext: { listAllPlaintext: async () => [] },
secretRefResolver: { resolve: async () => '' },
});
const service = new SecretService(repo, backends);
registerSecretRoutes(app, service);
return app.ready();
}
@@ -129,7 +172,7 @@ describe('Secret Routes', () => {
describe('PUT /api/v1/secrets/:id', () => {
it('updates a secret', async () => {
const repo = mockRepo();
vi.mocked(repo.findById).mockResolvedValue({ id: '1', name: 'ha-creds' } as never);
vi.mocked(repo.findById).mockResolvedValue(makeSecret({ id: '1' }) as never);
await createApp(repo);
const res = await app.inject({
method: 'PUT',
@@ -154,7 +197,7 @@ describe('Secret Routes', () => {
describe('DELETE /api/v1/secrets/:id', () => {
it('deletes a secret and returns 204', async () => {
const repo = mockRepo();
vi.mocked(repo.findById).mockResolvedValue({ id: '1', name: 'ha-creds' } as never);
vi.mocked(repo.findById).mockResolvedValue(makeSecret({ id: '1' }) as never);
await createApp(repo);
const res = await app.inject({ method: 'DELETE', url: '/api/v1/secrets/1' });
expect(res.statusCode).toBe(204);