diff --git a/.changeset/28633-datasource-admin-declared-keys.md b/.changeset/28633-datasource-admin-declared-keys.md new file mode 100644 index 00000000000..e28e2c0cf91 --- /dev/null +++ b/.changeset/28633-datasource-admin-declared-keys.md @@ -0,0 +1,20 @@ +--- +'@objectstack/service-datasource': patch +--- + +fix(service-datasource)!: the datasource admin doors store `ssl`, `autoConnect` and `description` instead of dropping them, and the connection test probes the same TLS block a Save stores + +Clause-②: yes (narrowing) — `POST /api/v1/datasources` and `PATCH /api/v1/datasources/:name` now store every top-level key `DatasourceSchema` declares (`ssl`, `autoConnect` and `description` were accepted and dropped), `GET /api/v1/datasources/:name` serves them, `POST /api/v1/datasources/test` hands the draft's `ssl` to the probe, and the exported `DatasourceDraft`, `StoredDatasource`, `ProbeInput` and `ConnectivityBearingFields` types gain those members. One body that used to answer 201 / 200 (with the whole `ssl` block dropped) is now refused with `400 DATASOURCE_ADMIN_ERROR`: an `ssl` block carrying a client private key (`ssl.key`). +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the datasource admin doors (`@objectstack/service-datasource`): an inline `ssl.key` is refused. + +- **Before.** Create and edit built the stored record from a hand list of eight keys (`name`, `label`, `driver`, `schemaMode`, `config`, `external`, `pool`, `active`). `ssl`, `autoConnect` and `description` passed the undeclared-key check (the schema declares them) and were then dropped, so a datasource saved with a TLS block was stored, and connected, without one. The connection test probed the same short record. +- **Now.** The stored set is read from `DatasourceSchema`'s shape at run time, so a key the schema declares later is stored the day it lands. Create, edit and the test probe use that one projection; an edit that changes `ssl` rebuilds the live connection. A read serves `ssl`, `autoConnect` and `description`. +- **Not taken from the body**, by design: `origin` (stamped `runtime`) and the ADR-0010 protection envelope (`_lock*`, `_provenance`, `_packageId`, `_packageVersion`), which only a code package's loader sets. +- **`ssl.key`.** The client private key has no sealed channel at this door (a `secret` is the only one, and it takes no TLS key), so storing it would put key material in the datasource row in cleartext. All three doors refuse it by name, never echoing the value; `enabled`, `rejectUnauthorized`, `ca` and `cert` are stored. A read never serves a key a stored row already holds, and an edit that sends back the served `ssl` block keeps that stored key. +- This supersedes the line of the #28612 entry that names `description`, `ssl` and `autoConnect` among the keys "the doors do not store". + +**The remedy.** Leave `ssl.key` out of the request; the rest of the TLS block is honoured. diff --git a/content/docs/data-modeling/drivers.mdx b/content/docs/data-modeling/drivers.mdx index baced2ec2ce..9ac7d209c8c 100644 --- a/content/docs/data-modeling/drivers.mdx +++ b/content/docs/data-modeling/drivers.mdx @@ -154,7 +154,11 @@ Two things live **outside** `config`, because they are not driver-specific: delete the rejected keys; they were reaching nothing either way. - **TLS certificates** — the `ssl` block on the datasource (`enabled`, `rejectUnauthorized`, `ca`, `cert`, `key`). Inside `config`, `ssl` is the - on/off boolean shorthand. + on/off boolean shorthand. A datasource created or edited in Setup + (`POST` / `PATCH /api/v1/datasources`) stores the block, and its connection + test probes with it, but `key` is refused there: that door seals only the + `secret` it is sent, so a client private key would sit in the stored row in + cleartext. - **`schemaMode`** — the ADR-0015 ownership mode, declared next to `driver`. A plugin-contributed driver (`com.vendor.snowflake`) has no contract in this diff --git a/packages/services/service-datasource/src/__tests__/datasource-admin-declared-keys-kept.test.ts b/packages/services/service-datasource/src/__tests__/datasource-admin-declared-keys-kept.test.ts new file mode 100644 index 00000000000..10a9783bd70 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/datasource-admin-declared-keys-kept.test.ts @@ -0,0 +1,330 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi } from 'vitest'; +import { DatasourceSchema } from '@objectstack/spec/data'; +import { MetadataProtectionFields } from '@objectstack/spec/kernel'; +import { HonoHttpServer } from '@objectstack/plugin-hono-server'; +import { registerDatasourceAdminRoutes } from '../admin-routes.js'; +import { + DatasourceAdminService, + type DatasourceAdminServiceConfig, + type ProbeInput, + type StoredDatasource, +} from '../datasource-admin-service.js'; +import { DatasourceAdminServicePlugin } from '../datasource-admin-plugin.js'; +import type { + DatasourceConnectionSpec, + IDatasourceAdminService, + IDatasourceDriverFactory, +} from '../contracts/index.js'; +import { + ENTITLED_CREDENTIAL, + createSessionAuthService, + createGrantsEngine, +} from './entitled-caller.fixture.js'; + +/** + * #28633 — the datasource admin doors keep every top-level key + * `DatasourceSchema` declares, `ssl`, `autoConnect` and `description` among + * them. + * + * `POST /api/v1/datasources` and `PATCH /api/v1/datasources/:name` built the + * stored record from a hand list of eight keys, so these three passed the + * undeclared-key refusal (the schema declares them) and were then dropped: a + * datasource saved with a TLS block was stored, and connected, without one. + * `POST /api/v1/datasources/test` probed the same short projection. + * + * Driven through the REAL routes over the REAL service on in-memory stores, + * reading the stored record, the record the pool seam is handed, the probe + * input and the read answer. The plugin's own probe is read through a fake + * driver factory, for the hand-over to the driver. + * + * Every certificate and key below is an obvious placeholder. + */ + +const authService = createSessionAuthService(); +const grantsEngine = createGrantsEngine(); + +const PLACEHOLDER_CA = 'PLACEHOLDER_CA_CERTIFICATE'; +const PLACEHOLDER_KEY = 'PLACEHOLDER_NOT_A_PRIVATE_KEY'; +const SSL = { enabled: true, rejectUnauthorized: true, ca: PLACEHOLDER_CA }; +const BASE = { driver: 'postgres', config: { host: 'db.example.invalid', database: 'analytics' } }; + +/** In-memory stores behind a real {@link DatasourceAdminService}, recording every seam call. */ +function makeDoor(seed: StoredDatasource[] = []) { + const records = new Map(seed.map((r) => [r.name, structuredClone(r)])); + const ops: string[] = []; + const pooled: StoredDatasource[] = []; + const probes: ProbeInput[] = []; + + const config: DatasourceAdminServiceConfig = { + probe: async (input) => { + ops.push('probe'); + probes.push(structuredClone(input)); + return { ok: true, latencyMs: 1 }; + }, + listDatasourceRecords: async () => [...records.values()].map((r) => structuredClone(r)), + getDatasourceRecord: async (name) => { + const r = records.get(name); + return r ? structuredClone(r) : undefined; + }, + putDatasourceRecord: async (record) => { + ops.push(`put:${record.name}`); + records.set(record.name, structuredClone(record)); + }, + deleteDatasourceRecord: async (name) => { + records.delete(name); + }, + writeSecret: async (_input, hint) => { + ops.push('writeSecret'); + return `sys_secret:sec_${hint.name}`; + }, + countBoundObjects: async () => 0, + listObjectsBoundTo: async () => [], + registerPool: (record) => { + ops.push('registerPool'); + pooled.push(structuredClone(record)); + }, + unregisterPool: () => { + ops.push('unregisterPool'); + }, + reregisterPool: (_previous, next) => { + ops.push('reregisterPool'); + pooled.push(structuredClone(next)); + }, + }; + + const service = new DatasourceAdminService(config); + const server = new HonoHttpServer(0); + const ctx = { + getService: vi.fn((name: string) => + name === 'auth' + ? authService + : name === 'objectql' || name === 'data' + ? grantsEngine + : name === 'datasource-admin' + ? service + : undefined, + ), + } as any; + registerDatasourceAdminRoutes(server, ctx, '/api/v1'); + const app = server.getRawApp(); + + const send = (method: string, path: string, body?: unknown) => + app.fetch( + new Request(`http://local/api/v1${path}`, { + method, + headers: { 'content-type': 'application/json', authorization: ENTITLED_CREDENTIAL }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }), + ); + + return { send, records, ops, pooled, probes }; +} + +const STORED: StoredDatasource = { name: 'wh', ...BASE, origin: 'runtime' }; + +async function expectRefusal(res: Response): Promise { + const raw = await res.text(); + const body = JSON.parse(raw) as { success: boolean; error?: { code: string; message: string } }; + expect({ status: res.status, success: body.success, code: body.error?.code }).toEqual({ + status: 400, + success: false, + code: 'DATASOURCE_ADMIN_ERROR', + }); + // Named, never echoed. + expect(raw).not.toContain(PLACEHOLDER_KEY); + return body.error!.message; +} + +describe('#28633 premise — the keys this card is about are declared by DatasourceSchema', () => { + it('declares ssl, autoConnect and description (so the undeclared-key refusal lets them through)', () => { + const shape = Object.keys((DatasourceSchema as unknown as { shape: object }).shape); + expect(shape).toEqual(expect.arrayContaining(['ssl', 'autoConnect', 'description'])); + }); +}); + +describe('POST /api/v1/datasources — create keeps every declared key (#28633)', () => { + it('stores ssl, autoConnect and description, and the pool is opened from a record carrying ssl', async () => { + const { send, records, pooled } = makeDoor(); + const res = await send('POST', '/datasources', { + name: 'wh', + ...BASE, + ssl: SSL, + autoConnect: true, + description: 'Analytics warehouse', + }); + expect(res.status).toBe(201); + expect(records.get('wh')).toMatchObject({ ssl: SSL, autoConnect: true, description: 'Analytics warehouse' }); + expect(pooled).toHaveLength(1); + expect(pooled[0].ssl).toEqual(SSL); + }); + + it('a read returns them', async () => { + const { send } = makeDoor(); + await send('POST', '/datasources', { name: 'wh', ...BASE, ssl: SSL, autoConnect: true, description: 'd' }); + const res = await send('GET', '/datasources/wh'); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { datasource: Record } }; + expect(body.data.datasource).toMatchObject({ ssl: SSL, autoConnect: true, description: 'd' }); + }); + + it('a stored TLS block is still judged by the schema: an unknown ssl key is refused, nothing stored', async () => { + const { send, records, ops } = makeDoor(); + const message = await expectRefusal( + await send('POST', '/datasources', { name: 'wh', ...BASE, ssl: { enabled: true, insecure: true } }), + ); + expect(message).toContain('ssl'); + expect(records.has('wh')).toBe(false); + expect(ops).toEqual([]); + }); + + it('control: a draft without these keys stores none of them, as before', async () => { + const { send, records } = makeDoor(); + expect((await send('POST', '/datasources', { name: 'wh', ...BASE })).status).toBe(201); + const stored = records.get('wh')!; + expect(Object.keys(stored).sort()).toEqual(['config', 'driver', 'name', 'origin']); + }); + + it('server-stamped keys are not taken from the body: origin is stamped runtime, the protection envelope is not stored', async () => { + const { send, records } = makeDoor(); + const res = await send('POST', '/datasources', { + name: 'wh', + ...BASE, + origin: 'code', + _lock: 'full', + _lockReason: 'locked by the author', + _provenance: 'package', + _packageId: 'com.example.pkg', + }); + expect(res.status).toBe(201); + const stored = records.get('wh')!; + expect(stored.origin).toBe('runtime'); + for (const key of Object.keys(MetadataProtectionFields)) expect(stored).not.toHaveProperty(key); + }); +}); + +describe('PATCH /api/v1/datasources/:name — edit keeps every declared key (#28633)', () => { + it('stores a new ssl block and rebuilds the pool from a record carrying it', async () => { + const { send, records, ops, pooled } = makeDoor([STORED]); + const res = await send('PATCH', '/datasources/wh', { ssl: SSL }); + expect(res.status).toBe(200); + expect(records.get('wh')?.ssl).toEqual(SSL); + expect(ops).toContain('reregisterPool'); + expect(pooled[pooled.length - 1]?.ssl).toEqual(SSL); + }); + + it('stores autoConnect and description', async () => { + const { send, records } = makeDoor([STORED]); + expect((await send('PATCH', '/datasources/wh', { autoConnect: true, description: 'Moved' })).status).toBe(200); + expect(records.get('wh')).toMatchObject({ autoConnect: true, description: 'Moved' }); + }); + + it('control: a description-only edit does not churn the live connection', async () => { + const { send, ops } = makeDoor([STORED]); + expect((await send('PATCH', '/datasources/wh', { description: 'Moved' })).status).toBe(200); + expect(ops).toContain('registerPool'); + expect(ops).not.toContain('reregisterPool'); + }); + + it('control: a label-only edit leaves a stored ssl block as it was', async () => { + const { send, records } = makeDoor([{ ...STORED, ssl: SSL }]); + expect((await send('PATCH', '/datasources/wh', { label: 'Warehouse' })).status).toBe(200); + expect(records.get('wh')?.ssl).toEqual(SSL); + }); +}); + +describe('POST /api/v1/datasources/test — the probe reads the record a Save stores (#28633)', () => { + it('hands the draft ssl block to the probe', async () => { + const { send, probes } = makeDoor(); + const res = await send('POST', '/datasources/test', { ...BASE, ssl: SSL }); + expect(res.status).toBe(200); + expect(probes).toHaveLength(1); + expect(probes[0].ssl).toEqual(SSL); + }); + + it('control: a draft without ssl probes without it, as before', async () => { + const { send, probes } = makeDoor(); + expect((await send('POST', '/datasources/test', BASE)).status).toBe(200); + expect(probes[0]).not.toHaveProperty('ssl'); + }); + + it('the plugin probe hands ssl to the driver factory', async () => { + const created: DatasourceConnectionSpec[] = []; + const factory: IDatasourceDriverFactory = { + supports: () => true, + create: async (spec) => { + created.push(structuredClone(spec)); + return { connect: async () => {}, ping: async () => {}, disconnect: async () => {} } as any; + }, + } as IDatasourceDriverFactory; + let admin: IDatasourceAdminService | undefined; + const ctx: any = { + getService: (name: string) => { + if (name === 'metadata') return { get: async () => undefined, list: async () => [] }; + throw new Error(`no service ${name}`); + }, + registerService: (name: string, svc: unknown) => { + if (name === 'datasource-admin') admin = svc as IDatasourceAdminService; + }, + trigger: async () => {}, + logger: { warn() {}, info() {} }, + }; + await new DatasourceAdminServicePlugin({ driverFactory: factory }).init(ctx); + const result = await admin!.testConnection({ name: 'x', ...BASE, ssl: SSL }); + expect(result.ok).toBe(true); + expect(created).toHaveLength(1); + expect(created[0].ssl).toEqual(SSL); + }); +}); + +describe('ssl.key — a TLS client private key is never stored in cleartext or served (#28633)', () => { + it('create refuses an inline ssl.key by name: nothing stored, minted or opened', async () => { + const { send, records, ops } = makeDoor(); + const message = await expectRefusal( + await send('POST', '/datasources', { name: 'wh', ...BASE, ssl: { ...SSL, key: PLACEHOLDER_KEY } }), + ); + expect(message.split('\n')[0]).toBe("Invalid datasource 'wh'."); + expect(message).toContain('`ssl.key`'); + expect(records.has('wh')).toBe(false); + expect(ops).toEqual([]); + }); + + it('edit refuses an inline ssl.key by name: the stored record is unchanged', async () => { + const { send, records, ops } = makeDoor([STORED]); + const message = await expectRefusal( + await send('PATCH', '/datasources/wh', { ssl: { ...SSL, key: PLACEHOLDER_KEY } }), + ); + expect(message).toContain('`ssl.key`'); + expect(records.get('wh')).toEqual(STORED); + expect(ops).toEqual([]); + }); + + it('the test door refuses it the same way, as a failed probe that never ran', async () => { + const { send, probes } = makeDoor(); + const res = await send('POST', '/datasources/test', { ...BASE, ssl: { ...SSL, key: PLACEHOLDER_KEY } }); + const raw = await res.text(); + expect(raw).not.toContain(PLACEHOLDER_KEY); + const body = JSON.parse(raw) as { data: { result: { ok: boolean; error: string } } }; + expect(res.status).toBe(200); + expect(body.data.result.ok).toBe(false); + expect(body.data.result.error).toContain('`ssl.key`'); + expect(probes).toEqual([]); + }); + + it('a read withholds a private key a stored row already holds', async () => { + const { send } = makeDoor([{ ...STORED, ssl: { ...SSL, key: PLACEHOLDER_KEY } }]); + const res = await send('GET', '/datasources/wh'); + const raw = await res.text(); + expect(res.status).toBe(200); + expect(raw).not.toContain(PLACEHOLDER_KEY); + const body = JSON.parse(raw) as { data: { datasource: { ssl: Record } } }; + expect(body.data.datasource.ssl).toEqual(SSL); + }); + + it('an edit sending back the served ssl block keeps the stored key', async () => { + const { send, records } = makeDoor([{ ...STORED, ssl: { ...SSL, key: PLACEHOLDER_KEY } }]); + expect((await send('PATCH', '/datasources/wh', { ssl: { ...SSL, rejectUnauthorized: false } })).status).toBe(200); + expect(records.get('wh')?.ssl).toEqual({ ...SSL, rejectUnauthorized: false, key: PLACEHOLDER_KEY }); + }); +}); diff --git a/packages/services/service-datasource/src/contracts/datasource-admin-service.ts b/packages/services/service-datasource/src/contracts/datasource-admin-service.ts index d287b3a7876..0fcb60e8b8b 100644 --- a/packages/services/service-datasource/src/contracts/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/contracts/datasource-admin-service.ts @@ -52,6 +52,16 @@ export interface DatasourceDraft { external?: Record; pool?: Record; active?: boolean; + /** + * TLS block (`DatasourceSchema.ssl`): `enabled`, `rejectUnauthorized`, `ca`, + * `cert`. Stored and probed as sent (#28633); a client private key + * (`ssl.key`) is refused, since it would be stored in cleartext. + */ + ssl?: Record; + /** Force a live connection at boot even when managed + unrouted (ADR-0062 D2(c)). */ + autoConnect?: boolean; + /** Internal description. */ + description?: string; } /** Result of probing a connection (live driver connect + cheap round-trip). */ diff --git a/packages/services/service-datasource/src/datasource-admin-plugin.ts b/packages/services/service-datasource/src/datasource-admin-plugin.ts index 9f6f5229a2a..bdf7f31452b 100644 --- a/packages/services/service-datasource/src/datasource-admin-plugin.ts +++ b/packages/services/service-datasource/src/datasource-admin-plugin.ts @@ -966,6 +966,10 @@ export class DatasourceAdminServicePlugin implements Plugin { config: input.config, secret: input.secret, external: input.external, + // [#28633] The draft's TLS block, as the saved datasource's connect + // hands it over (`toSpec`): a probe without it tested a plain-text + // connection to a server the save would reach over TLS. + ...(input.ssl !== undefined ? { ssl: input.ssl } : {}), ...(credential?.ok ? { credential: credential.handle } : {}), }); } catch (err) { diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index 30943306cc1..ef8fb4d4c1a 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -36,6 +36,7 @@ */ import { DatasourceSchema, externalBindingDatasourceRefusal, validateDriverConfig } from '@objectstack/spec/data'; +import { MetadataProtectionFields } from '@objectstack/spec/kernel'; import type { IPubSub } from '@objectstack/spec/contracts'; import { assertDatasourcePoolSupported } from './datasource-pool-support.js'; import { datasourceConnectivityChanged } from './datasource-connectivity-change.js'; @@ -84,7 +85,101 @@ const PROBE_DRAFT_NAME = 'connection_test_draft'; * a gate landed. Every other patched key, and any supplied secret, is a change * to what the record binds, and the record is judged whole. */ -const RECORD_JUDGED_PATCH_KEYS = ['driver', 'schemaMode', 'config', 'external', 'pool'] as const; +const RECORD_JUDGE_EXEMPT_PATCH_KEYS: ReadonlySet = new Set(['label', 'active']); + +/** + * [#28633] The top-level keys `DatasourceSchema` declares that a write door + * deliberately does NOT take from the request body, each for its reason: + * + * - `origin` — provenance, "server-managed, read-only" and "never accepted + * from client input" (the schema's own words): the door stamps `runtime`. + * - the ADR-0010 protection envelope (`MetadataProtectionFields`: the `_lock*` + * family, `_provenance`, `_packageId`, `_packageVersion`) — set by a code + * package's loader, never by an author's write. The metadata save door + * refuses an authored lock for that reason (ADR-0010 §3.5); storing one here + * would be the way around it. + * + * Read off those declarations, so a protection field added later is left out + * here the day it lands. + */ +const SERVER_STAMPED_KEYS: ReadonlySet = new Set(['origin', ...Object.keys(MetadataProtectionFields)]); + +let storedKeysCache: readonly string[] | undefined; + +/** + * [#28633] The top-level keys a create, an edit and a probe take from the + * body: every key `DatasourceSchema` declares, minus {@link SERVER_STAMPED_KEYS}. + * + * Derived from the schema's shape at run time, never listed by hand. The hand + * list that stood here (`name`, `label`, `driver`, `schemaMode`, `config`, + * `external`, `pool`, `active`) left out `ssl`, `autoConnect` and + * `description`: the doors accepted them (the schema declares them, so the + * undeclared-key refusal lets them through) and then dropped them, and a + * datasource saved with a TLS block connected without one. A key the schema + * declares later is stored the day it lands. + */ +function storedDatasourceKeys(): readonly string[] { + if (!storedKeysCache) { + const shape = (DatasourceSchema as unknown as { shape: Record }).shape; + storedKeysCache = Object.freeze(Object.keys(shape).filter((key) => !SERVER_STAMPED_KEYS.has(key))); + } + return storedKeysCache; +} + +/** `body`'s members under {@link storedDatasourceKeys} whose value is not `undefined`. */ +function storedKeysOf(body: object): Record { + const out: Record = {}; + for (const key of storedDatasourceKeys()) { + const value = (body as Record)[key]; + if (value !== undefined) out[key] = value; + } + return out; +} + +/** + * [#28633] The one `ssl` member that is key material: the client private key. + * `ca` and `cert` are public certificates; `key` is the secret half of the + * client certificate (the schema: "Client private key"). + */ +const SSL_PRIVATE_KEY = 'key'; + +/** + * [#28633] Why `body` cannot be taken as it is: its `ssl` block carries a + * client private key. `undefined` when it carries none. + * + * This door keeps credentials out of the stored record: a cleartext `secret` + * transits the request and only the `credentialsRef` the secret store mints is + * stored. `ssl.key` has no such channel, so storing it would put key material + * in the datasource row in cleartext. Refused by name, never by value, before + * anything is read, written or probed. The rest of the block (`enabled`, + * `rejectUnauthorized`, `ca`, `cert`) is stored. + */ +function inlineSslKeyRefusal(heading: string, body: object): string | undefined { + const ssl = (body as { ssl?: unknown }).ssl; + if (!ssl || typeof ssl !== 'object') return undefined; + if ((ssl as Record)[SSL_PRIVATE_KEY] === undefined) return undefined; + return ( + `${heading}\n` + + '`ssl.key`: a TLS client private key is not accepted through this door. It would be stored in cleartext ' + + 'in the datasource record, and this door stores no credential there: only a `secret` it seals into the ' + + 'secret store, which takes no TLS key. Leave `ssl.key` out (`enabled`, `rejectUnauthorized`, `ca` and ' + + '`cert` are stored) and send the request again. Nothing was written or probed.' + ); +} + +/** {@link inlineSslKeyRefusal} for a write door: refuses by throwing (a `400` at the route). */ +function assertNoInlineSslKey(heading: string, body: object): void { + const refusal = inlineSslKeyRefusal(heading, body); + if (refusal) throw new Error(refusal); +} + +/** `ssl` as a read may serve it: the private key withheld, every other member as stored. */ +function servableSsl(ssl: Record | undefined): Record | undefined { + if (!ssl || typeof ssl !== 'object') return ssl; + if (!(SSL_PRIVATE_KEY in ssl)) return ssl; + const { [SSL_PRIVATE_KEY]: _withheld, ...rest } = ssl; + return rest; +} /** A copy of `record` carrying the binding a supplied secret is about to create. */ function withPendingSecretRef(record: StoredDatasource): StoredDatasource { @@ -148,6 +243,10 @@ export interface StoredDatasource { active?: boolean; /** Force a live connection at boot even when managed + unrouted (ADR-0062 D2(c)). */ autoConnect?: boolean; + /** Datasource-level TLS block (`DatasourceSchema.ssl`), read by the driver factory. */ + ssl?: Record; + /** Internal description. */ + description?: string; origin?: 'code' | 'runtime'; /** Package that defines a code-origin datasource, when known. */ definedIn?: string; @@ -160,6 +259,8 @@ export interface ProbeInput { /** Cleartext secret used for this probe only (e.g. password / DSN). */ secret?: string; external?: Record; + /** [#28633] The draft's TLS block, exactly as a Save would store it. */ + ssl?: Record; timeoutMs?: number; } @@ -598,7 +699,10 @@ export class DatasourceAdminService implements IDatasourceAdminService { * stops it being SERVED, not what removes it. */ async getDatasource(name: string): Promise< - | (Pick & { + | (Pick< + StoredDatasource, + 'name' | 'label' | 'driver' | 'schemaMode' | 'config' | 'active' | 'definedIn' | 'ssl' | 'autoConnect' | 'description' + > & { origin: 'code' | 'runtime'; hasSecret: boolean; redactedConfigKeys: string[]; @@ -616,6 +720,13 @@ export class DatasourceAdminService implements IDatasourceAdminService { schemaMode: rec.schemaMode ?? 'managed', config, active: rec.active ?? true, + // [#28633] The other keys the doors now store, so an edit form reads back + // what was saved. `ssl` is served without its private key, whatever the + // row holds: the key never leaves this service, as a `config` credential + // does not (#8081). + ...(rec.ssl !== undefined ? { ssl: servableSsl(rec.ssl) } : {}), + ...(rec.autoConnect !== undefined ? { autoConnect: rec.autoConnect } : {}), + ...(rec.description !== undefined ? { description: rec.description } : {}), origin: rec.origin === 'runtime' ? 'runtime' : 'code', hasSecret, redactedConfigKeys: redactedKeys, @@ -651,8 +762,15 @@ export class DatasourceAdminService implements IDatasourceAdminService { // reports green for a pairing `createDatasource` refuses. Judged on the // record a Save would persist, as create judges it, minus the identity the // probe does not have. + // + // [#28633] And probed from that same record: the projection a Save stores + // is the one the probe reads, so a test opens TLS exactly as the saved + // datasource will. + const draft = this.toRecord({ ...input, name: PROBE_DRAFT_NAME }); + const keyRefusal = inlineSslKeyRefusal('This connection draft is not a valid datasource.', draft); + if (keyRefusal) return { ok: false, error: keyRefusal }; const refusal = this.recordRefusal( - this.withSecretBinding({ ...this.toRecord({ ...input, name: PROBE_DRAFT_NAME }), origin: 'runtime' }, secret), + this.withSecretBinding({ ...draft, origin: 'runtime' }, secret), 'This connection draft is not a valid datasource.', ); if (refusal) return { ok: false, error: refusal }; @@ -661,10 +779,11 @@ export class DatasourceAdminService implements IDatasourceAdminService { const queryTimeoutMs = (input.external as { queryTimeoutMs?: number } | undefined)?.queryTimeoutMs; try { return await this.config.probe({ - driver: input.driver, - config: input.config ?? {}, + driver: draft.driver, + config: draft.config ?? {}, secret: secret?.value, - external: input.external, + external: draft.external, + ...(draft.ssl !== undefined ? { ssl: draft.ssl } : {}), ...(typeof queryTimeoutMs === 'number' ? { timeoutMs: queryTimeoutMs } : {}), }); } catch (err) { @@ -676,6 +795,7 @@ export class DatasourceAdminService implements IDatasourceAdminService { this.assertValidName(input?.name); this.assertNoDraftCredentialsRef(input); this.assertNoUndeclaredTopLevelKey(`Invalid datasource '${input.name}'.`, input); + assertNoInlineSslKey(`Invalid datasource '${input.name}'.`, input); if (!input.driver) throw new Error('A driver is required to create a datasource.'); this.assertValidConfig(input.driver, input.config); // The wizard is a publish door for `pool` too (#5714). Rejected BEFORE the @@ -741,16 +861,16 @@ export class DatasourceAdminService implements IDatasourceAdminService { // edit included: that exemption spares a row stored before a gate, and this // judges a key the author is writing now. this.assertNoUndeclaredTopLevelKey(`Invalid datasource '${existing.name}'.`, patch); + assertNoInlineSslKey(`Invalid datasource '${existing.name}'.`, patch); // Merge patch over the existing record; `name`/`origin` are never patched. + // [#28633] Every key the create door stores is patchable the same way + // ({@link storedDatasourceKeys}); `external` keeps its own rule below. + const { name: _name, external: _external, ...patched } = storedKeysOf(patch); + const patchedKeys = Object.keys(patched).concat(patch.external !== undefined ? ['external'] : []); const merged: StoredDatasource = { ...existing, - ...(patch.label !== undefined ? { label: patch.label } : {}), - ...(patch.driver !== undefined ? { driver: patch.driver } : {}), - ...(patch.schemaMode !== undefined ? { schemaMode: patch.schemaMode } : {}), - ...(patch.config !== undefined ? { config: patch.config } : {}), - ...(patch.pool !== undefined ? { pool: patch.pool } : {}), - ...(patch.active !== undefined ? { active: patch.active } : {}), + ...(patched as Partial), name: existing.name, origin: 'runtime', }; @@ -789,7 +909,7 @@ export class DatasourceAdminService implements IDatasourceAdminService { // not move across the restore — it grafts back credential keys and swaps a // url for its stored twin, which names the same user (`redactUrlPassword` // keeps the userinfo username). - if (secret !== undefined || RECORD_JUDGED_PATCH_KEYS.some((key) => patch[key] !== undefined)) { + if (secret !== undefined || patchedKeys.some((key) => !RECORD_JUDGE_EXEMPT_PATCH_KEYS.has(key))) { this.assertValidRecord(this.withSecretBinding(merged, secret)); // [ADR-0015 §4.2 · #25277] The merged record against the objects bound // to it, under the same condition: a `label`/`active`-only write stays @@ -835,6 +955,15 @@ export class DatasourceAdminService implements IDatasourceAdminService { if (patch.config !== undefined && merged.driver === existing.driver) { merged.config = restoreRedactedConfig(existing.driver, merged.config, existing.config); } + // [#28633] The same carry-forward for the one `ssl` member a read withholds: + // a patched `ssl` block without a private key (what `getDatasource()` + // served) keeps the stored one. This door never takes a new one + // ({@link assertNoInlineSslKey}), so it preserves a key already at rest and + // creates none. + const storedKey = existing.ssl?.[SSL_PRIVATE_KEY]; + if (patch.ssl !== undefined && storedKey !== undefined && merged.ssl && !(SSL_PRIVATE_KEY in merged.ssl)) { + merged.ssl = { ...merged.ssl, [SSL_PRIVATE_KEY]: storedKey }; + } await this.config.putDatasourceRecord(merged); @@ -1292,17 +1421,13 @@ export class DatasourceAdminService implements IDatasourceAdminService { throw new Error(`${heading}\n${detail.join('\n')}`); } + /** + * The record a create stores and a probe reads: the draft's members under + * every key `DatasourceSchema` declares, minus the server-stamped ones + * ({@link storedDatasourceKeys}). `name` and `driver` are always present. + */ private toRecord(input: DatasourceDraft): StoredDatasource { - return { - name: input.name, - ...(input.label !== undefined ? { label: input.label } : {}), - driver: input.driver, - ...(input.schemaMode !== undefined ? { schemaMode: input.schemaMode } : {}), - ...(input.config !== undefined ? { config: input.config } : {}), - ...(input.external !== undefined ? { external: input.external } : {}), - ...(input.pool !== undefined ? { pool: input.pool } : {}), - ...(input.active !== undefined ? { active: input.active } : {}), - }; + return { ...storedKeysOf(input), name: input.name, driver: input.driver } as StoredDatasource; } private toSummary(record: StoredDatasource): DatasourceSummary { diff --git a/packages/services/service-datasource/src/datasource-connectivity-change.ts b/packages/services/service-datasource/src/datasource-connectivity-change.ts index a9044f2c7fb..9a8869e7ebc 100644 --- a/packages/services/service-datasource/src/datasource-connectivity-change.ts +++ b/packages/services/service-datasource/src/datasource-connectivity-change.ts @@ -25,6 +25,10 @@ * dereferences to), which is why `updateDatasource` treats "a secret was * supplied" as a connectivity change alongside this function's verdict. * - `pool` — read by the pool-support gate and `toSpec`. + * - `ssl` — read by `toSpec` (`ssl: record.ssl`) and turned into the SQL + * client's TLS option by the driver factory. Joined the set with #28633, + * when the admin doors started storing it: before that no write path here + * could change it. * - `schemaMode` — read by the connect policy gate (`canConnect`), by * `toSpec` (so it reaches `factory.create` and the driver it builds), and * by `registerDatasourceDef` (the write gate's def). It is patchable on @@ -59,7 +63,7 @@ import type { StoredDatasource } from './datasource-admin-service.js'; /** The slice of a stored record this comparison consults. */ export type ConnectivityBearingFields = Pick< StoredDatasource, - 'driver' | 'config' | 'external' | 'pool' | 'schemaMode' | 'active' + 'driver' | 'config' | 'external' | 'pool' | 'schemaMode' | 'active' | 'ssl' >; /** @@ -85,6 +89,7 @@ export function datasourceConnectivityChanged( if (!deepEqual(before.config ?? {}, after.config ?? {})) return true; if (!deepEqual(before.external, after.external)) return true; if (!deepEqual(before.pool, after.pool)) return true; + if (!deepEqual(before.ssl, after.ssl)) return true; if (before.schemaMode !== after.schemaMode) return true; if ((before.active ?? true) !== (after.active ?? true)) return true; return false;