From 9a808877b5f42bf9f6b031b7fb2317aefc027613 Mon Sep 17 00:00:00 2001 From: Michal Date: Fri, 24 Apr 2026 00:57:06 +0100 Subject: [PATCH] feat(secrets): track key names so list/describe work for backend-stored secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-migration, every Secret on a non-plaintext backend had an empty `data` column (values live in the backend; only externalRef on the row). The CLI's \`get secrets\` showed \`KEYS: -\` and \`describe secret\` showed \`(empty)\` for all 9 migrated secrets — useless without --show-values. Fix: dedicated \`keyNames Json\` column on Secret that stores the sorted key list independently from the values. Populated on every write path, lazily backfilled on first read for pre-existing rows that pre-date the column. Schema default \`[]\` keeps prisma db push self-healing on rolling upgrades. - src/db/prisma/schema.prisma: add Secret.keyNames Json @default("[]") - src/mcpd/src/repositories/secret.repository.ts: pipe keyNames through create + update - src/mcpd/src/services/secret.service.ts: - create/update populate keyNames = sorted Object.keys(data) - getById lazy-backfills empty keyNames (cheap: derives from data for plaintext, single backend read for openbao) - src/mcpd/src/services/secret-migrate.service.ts: migrate writes keyNames alongside the new backendId so freshly-migrated rows are populated without a follow-up read - src/cli/src/commands/get.ts: KEYS column reads keyNames first, falls back to Object.keys(data) for older rows - src/cli/src/commands/describe.ts: shows the Data section keys whenever keyNames OR data has entries (so backend-stored secrets render their key list); --show-values still resolves through the backend After deploy, the 9 already-migrated secrets backfill their keyNames on the next describe-by-id, with no operator action needed. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cli/src/commands/describe.ts | 19 ++++++-- src/cli/src/commands/get.ts | 18 +++++++- src/db/prisma/schema.prisma | 6 +++ .../src/repositories/secret.repository.ts | 4 ++ .../src/services/secret-migrate.service.ts | 3 ++ src/mcpd/src/services/secret.service.ts | 45 ++++++++++++++++++- 6 files changed, 89 insertions(+), 6 deletions(-) diff --git a/src/cli/src/commands/describe.ts b/src/cli/src/commands/describe.ts index ed7f0d4..998d12d 100644 --- a/src/cli/src/commands/describe.ts +++ b/src/cli/src/commands/describe.ts @@ -207,12 +207,23 @@ function formatSecretDetail(secret: Record, showValues: boolean lines.push(`${pad('Name:')}${secret.name}`); const data = secret.data as Record | undefined; - if (data && Object.keys(data).length > 0) { + const keyNames = Array.isArray(secret.keyNames) ? secret.keyNames as string[] : []; + // The data row carries the actual values for plaintext-backed secrets and + // is empty for backend-stored ones — fall back to keyNames so we can still + // show what KEYS exist without their values. + const knownKeys = (data && Object.keys(data).length > 0) + ? Object.keys(data) + : keyNames; + + if (knownKeys.length > 0) { lines.push(''); lines.push('Data:'); - const keyW = Math.max(4, ...Object.keys(data).map((k) => k.length)) + 2; - for (const [key, value] of Object.entries(data)) { - const display = showValues ? value : '***'; + const keyW = Math.max(4, ...knownKeys.map((k) => k.length)) + 2; + for (const key of knownKeys) { + const value = data?.[key]; + const display = showValues + ? (value ?? '(reveal failed — backend unreachable?)') + : '***'; lines.push(` ${key.padEnd(keyW)}${display}`); } if (!showValues) { diff --git a/src/cli/src/commands/get.ts b/src/cli/src/commands/get.ts index b327bab..b79c97b 100644 --- a/src/cli/src/commands/get.ts +++ b/src/cli/src/commands/get.ts @@ -33,6 +33,10 @@ interface SecretRow { id: string; name: string; data: Record; + /** Sorted list of key names — populated by mcpd whether the values live in + * the row (plaintext) or in a remote backend (openbao). Falls back to + * Object.keys(data) for older rows that pre-date the column. */ + keyNames?: string[]; } interface TemplateRow { @@ -179,7 +183,19 @@ const mcpTokenColumns: Column[] = [ const secretColumns: Column[] = [ { header: 'NAME', key: 'name' }, - { header: 'KEYS', key: (r) => Object.keys(r.data).join(', ') || '-', width: 40 }, + { + header: 'KEYS', + key: (r) => { + // Prefer the dedicated column. It's populated whether the secret lives + // in the row (plaintext) or behind a backend (openbao); the row's `data` + // is empty in the latter case. + const tracked = Array.isArray(r.keyNames) && r.keyNames.length > 0 ? r.keyNames : null; + const fallback = Object.keys(r.data ?? {}); + const keys = tracked ?? fallback; + return keys.length > 0 ? keys.join(', ') : '-'; + }, + width: 40, + }, { header: 'ID', key: 'id' }, ]; diff --git a/src/db/prisma/schema.prisma b/src/db/prisma/schema.prisma index ed33913..75467d3 100644 --- a/src/db/prisma/schema.prisma +++ b/src/db/prisma/schema.prisma @@ -156,6 +156,12 @@ model Secret { backendId String @default("") data Json @default("{}") // populated by plaintext backend only externalRef String @default("") // populated by non-plaintext backends (e.g. "mount/path#v3") + // Sorted list of the secret's data keys WITHOUT their values. Populated on + // every create/update/migrate so list views and describe-without-reveal can + // show "this secret has GRAFANA_URL + GRAFANA_TOKEN" without fetching the + // backing data. For pre-existing rows the field is empty until the next + // write or a lazy resolve in getById fills it in. + keyNames Json @default("[]") version Int @default(1) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt diff --git a/src/mcpd/src/repositories/secret.repository.ts b/src/mcpd/src/repositories/secret.repository.ts index 72434ce..3f40bf9 100644 --- a/src/mcpd/src/repositories/secret.repository.ts +++ b/src/mcpd/src/repositories/secret.repository.ts @@ -6,12 +6,14 @@ export interface SecretRepoCreateInput { backendId: string; data?: Record; externalRef?: string; + keyNames?: string[]; } export interface SecretRepoUpdateInput { data?: Record; externalRef?: string; backendId?: string; + keyNames?: string[]; } export class SecretRepository implements ISecretRepository { @@ -40,6 +42,7 @@ export class SecretRepository implements ISecretRepository { backendId: data.backendId, data: (data.data ?? {}) as Prisma.InputJsonValue, externalRef: data.externalRef ?? '', + keyNames: (data.keyNames ?? []) as Prisma.InputJsonValue, }, }); } @@ -51,6 +54,7 @@ export class SecretRepository implements ISecretRepository { if (data.backendId !== undefined) { updateData.backend = { connect: { id: data.backendId } }; } + if (data.keyNames !== undefined) updateData.keyNames = data.keyNames as Prisma.InputJsonValue; return this.prisma.secret.update({ where: { id }, data: updateData }); } diff --git a/src/mcpd/src/services/secret-migrate.service.ts b/src/mcpd/src/services/secret-migrate.service.ts index 59ba36c..d5320d7 100644 --- a/src/mcpd/src/services/secret-migrate.service.ts +++ b/src/mcpd/src/services/secret-migrate.service.ts @@ -94,6 +94,9 @@ export class SecretMigrateService { backendId: dest.id, data: written.storedData, externalRef: written.externalRef, + // Keys travel with the secret — populate now so list/describe + // doesn't need a per-row backend read just to display key names. + keyNames: Object.keys(data).sort(), }); if (opts.keepSource !== true) { diff --git a/src/mcpd/src/services/secret.service.ts b/src/mcpd/src/services/secret.service.ts index 0603cd9..c2a6636 100644 --- a/src/mcpd/src/services/secret.service.ts +++ b/src/mcpd/src/services/secret.service.ts @@ -28,7 +28,7 @@ export class SecretService implements SecretRefResolver { if (secret === null) { throw new NotFoundError(`Secret not found: ${id}`); } - return secret; + return this.maybeBackfillKeyNames(secret); } async getByName(name: string): Promise { @@ -75,6 +75,7 @@ export class SecretService implements SecretRefResolver { backendId: backend.id, data: written.storedData, externalRef: written.externalRef, + keyNames: keysOf(data.data), }); } @@ -87,6 +88,7 @@ export class SecretService implements SecretRefResolver { return this.repo.update(id, { data: written.storedData, externalRef: written.externalRef, + keyNames: keysOf(data.data), }); } @@ -114,4 +116,45 @@ export class SecretService implements SecretRefResolver { if (existing === null) return; await this.delete(existing.id); } + + /** + * If a row's `keyNames` is empty (pre-Phase-5 migrated rows), fetch the + * data from the backend once and persist the keys back to the row. Means + * the next read of this secret takes the cheap path. We never store values + * here — only the sorted list of key names. + * + * Best-effort: if the backend is unreachable we just return the row as-is; + * the next read will retry. + */ + private async maybeBackfillKeyNames(secret: Secret): Promise { + const existingKeys = secret.keyNames as unknown; + if (Array.isArray(existingKeys) && existingKeys.length > 0) return secret; + + // Plaintext secrets carry their values directly — derive keys from `data` + // without touching the backend. + const data = secret.data as Record; + if (data && Object.keys(data).length > 0) { + const keys = keysOf(data as Record); + const updated = await this.repo.update(secret.id, { keyNames: keys }) + .catch(() => secret); + return updated; + } + + // Non-plaintext + empty data: ask the backend for the actual values just + // to learn the keys. Cheap (single backend read) and only happens once + // per row. + try { + const resolved = await this.resolveData(secret); + const keys = keysOf(resolved); + if (keys.length === 0) return secret; + return await this.repo.update(secret.id, { keyNames: keys }); + } catch { + return secret; + } + } +} + +/** Sorted list of keys from a data object — stable serialization for diffing. */ +function keysOf(data: Record): string[] { + return Object.keys(data).sort(); }