From 0cc981aa238a292eff24079a3dfcb7a9fe55df1a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 07:00:47 +0000 Subject: [PATCH 1/7] fix(service-datasource)!: refuse a top-level password or connectionString at the datasource admin write doors POST /api/v1/datasources and PATCH /api/v1/datasources/:name projected the body onto the record's known keys, so a top-level password or connectionString vanished and the write answered success while os compile refuses the same datasource by name. Both doors now refuse either key before anything is written, with DatasourceSchema's own refusal for that key under the heading the record judgement uses. Only key names are judged, so the answer never echoes the credential. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5moUefysT3ba6SpdYY --- .../28549-datasource-top-level-credential.md | 18 ++ ...asource-admin-top-level-credential.test.ts | 277 ++++++++++++++++++ .../src/datasource-admin-service.ts | 45 +++ 3 files changed, 340 insertions(+) create mode 100644 .changeset/28549-datasource-top-level-credential.md create mode 100644 packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts diff --git a/.changeset/28549-datasource-top-level-credential.md b/.changeset/28549-datasource-top-level-credential.md new file mode 100644 index 00000000000..43f657f50d6 --- /dev/null +++ b/.changeset/28549-datasource-top-level-credential.md @@ -0,0 +1,18 @@ +--- +'@objectstack/service-datasource': patch +--- + +fix(service-datasource)!: `POST /api/v1/datasources` and `PATCH /api/v1/datasources/:name` refuse a top-level `password` or `connectionString` by name, with the build door's prescription, instead of dropping it and answering success + +Clause-②: no (narrowing) — a create or edit body that carries `password` or `connectionString` at its top level, which answered 201 / 200 with the key silently dropped, is refused with `400 DATASOURCE_ADMIN_ERROR`. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the datasource admin write doors (`@objectstack/service-datasource`). + +- **Before.** The doors built the stored record from its known keys only, so a `password` or a `connectionString` written at the top level of the body vanished and the write answered success. The author got a datasource that could not connect and no explanation, while `os compile` refused the same datasource by name. +- **Now.** Either key at the top level of a create or edit body is refused before anything is read or written, naming the key and never its value. The message is `DatasourceSchema`'s own refusal for that key, the one `os compile` prints: a `password` is bound through the connection form's secret field or `external.credentialsRef`; a `connectionString` belongs in `config` as `url`. An edit is refused even when it touches only `label` or `active`. +- **Unchanged.** A credential sent as `secret` is still stored in the secret store and bound at `external.credentialsRef`. `config.password` keeps the driver contract's refusal. Other unknown top-level keys are still dropped as before. + +**The remedy.** Send the credential as `secret` (the connection form's secret field), or move a connection string into `config.url` without userinfo. diff --git a/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts b/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts new file mode 100644 index 00000000000..196eb24d895 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts @@ -0,0 +1,277 @@ +// 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 { HonoHttpServer } from '@objectstack/plugin-hono-server'; +import { registerDatasourceAdminRoutes } from '../admin-routes.js'; +import { + DatasourceAdminService, + type DatasourceAdminServiceConfig, + type StoredDatasource, +} from '../datasource-admin-service.js'; +import { + ENTITLED_CREDENTIAL, + createSessionAuthService, + createGrantsEngine, +} from './entitled-caller.fixture.js'; + +/** + * #28549 — a credential written at the TOP level of a datasource body is + * refused by name at the admin write doors, as the build door refuses it. + * + * `os compile` judges a datasource against `DatasourceSchema`, whose strict + * shape refuses a top-level `password` ("must never be inlined", bind it + * through `external.credentialsRef`) and a top-level `connectionString` + * ("belongs inside `config`"). `POST /api/v1/datasources` and + * `PATCH /api/v1/datasources/:name` projected the body onto the record's known + * keys first, so both keys vanished and the write answered success: the + * author got a datasource that could not connect and no explanation. + * + * Driven through the REAL routes over the REAL service. Each refusal reads the + * envelope (status + `error.code`), the message (the heading plus the spec's + * own refusal for that key, derived here from `DatasourceSchema`, never + * restated), the absence of the value from the answer, and the store behind + * it: nothing stored, minted or registered. + * + * Every credential below is an obvious placeholder. + */ + +const authService = createSessionAuthService(); +const grantsEngine = createGrantsEngine(); + +const PLACEHOLDER_PASSWORD = 'PLACEHOLDER_NOT_A_SECRET'; +const PLACEHOLDER_DSN = `postgres://app:${PLACEHOLDER_PASSWORD}@db.example.invalid:5432/analytics`; + +/** In-memory datasource store and secret store behind a real {@link DatasourceAdminService}. */ +function makeDoor(seed: StoredDatasource[] = []) { + const records = new Map(seed.map((r) => [r.name, structuredClone(r)])); + const secrets = new Map(); + let seq = 0; + const ops: string[] = []; + + const config: DatasourceAdminServiceConfig = { + probe: async () => ({ 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'); + const ref = `sys_secret:sec_${hint.name}_${++seq}`; + secrets.set(ref, input.value); + return ref; + }, + removeSecret: async (ref) => { + ops.push('removeSecret'); + secrets.delete(ref); + }, + countBoundObjects: async () => 0, + listObjectsBoundTo: async () => [], + registerPool: () => { + ops.push('registerPool'); + }, + unregisterPool: () => { + ops.push('unregisterPool'); + }, + reregisterPool: () => { + ops.push('reregisterPool'); + }, + }; + + 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: JSON.stringify(body), + }), + ); + + return { send, records, secrets, ops }; +} + +/** + * The build door's refusal of `keys` at the top level of a datasource: + * `DatasourceSchema`'s own `unrecognized_keys` message. Throws when the spec + * makes no such refusal, so a pin can never pass on a premise the spec no + * longer holds. + */ +function specRefusal(keys: string[]): string { + const result = DatasourceSchema.safeParse(Object.fromEntries(keys.map((k) => [k, null]))); + const issue = result.success + ? undefined + : result.error.issues.find((i) => i.code === 'unrecognized_keys' && i.path.length === 0); + if (!issue) throw new Error(`fixture premise broken: DatasourceSchema does not refuse ${keys.join(', ')}`); + return issue.message; +} + +async function expectRefusal(res: Response): Promise { + const body = (await res.json()) 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', + }); + return body.error!.message; +} + +const BASE = { driver: 'postgres', config: { host: 'db.example.invalid', database: 'analytics' } }; +const STORED: StoredDatasource = { name: 'wh', ...BASE, origin: 'runtime' }; + +describe('#28549 — POST /api/v1/datasources refuses a top-level credential by name', () => { + it('refuses a top-level `password` with the build door\'s prescription, storing nothing', async () => { + const { send, records, secrets, ops } = makeDoor(); + + const message = await expectRefusal( + await send('POST', '/datasources', { name: 'r7p_top', ...BASE, password: PLACEHOLDER_PASSWORD }), + ); + + expect(message).toBe(`Invalid datasource 'r7p_top'.\n${specRefusal(['password'])}`); + expect(message).toContain('`password`'); + expect(message).toContain('external.credentialsRef'); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(records.has('r7p_top')).toBe(false); + expect(secrets.size).toBe(0); + expect(ops).toEqual([]); + }); + + it('refuses a top-level `connectionString` carrying URL userinfo, storing nothing', async () => { + const { send, records, secrets, ops } = makeDoor(); + + const message = await expectRefusal( + await send('POST', '/datasources', { name: 'r7p_top2', ...BASE, connectionString: PLACEHOLDER_DSN }), + ); + + expect(message).toBe(`Invalid datasource 'r7p_top2'.\n${specRefusal(['connectionString'])}`); + expect(message).toContain('`connectionString`'); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(message).not.toContain('db.example.invalid'); + expect(records.has('r7p_top2')).toBe(false); + expect(secrets.size).toBe(0); + expect(ops).toEqual([]); + }); + + it('names both keys in one answer when both are written', async () => { + const { send, records } = makeDoor(); + + const message = await expectRefusal( + await send('POST', '/datasources', { + name: 'r7p_both', + ...BASE, + password: PLACEHOLDER_PASSWORD, + connectionString: PLACEHOLDER_DSN, + }), + ); + + expect(message).toBe(`Invalid datasource 'r7p_both'.\n${specRefusal(['password', 'connectionString'])}`); + expect(records.has('r7p_both')).toBe(false); + }); + + it('refuses it beside a supplied `secret` too: the secret field is the one credential channel', async () => { + const { send, records, secrets, ops } = makeDoor(); + + await expectRefusal( + await send('POST', '/datasources', { + name: 'r7p_secret', + ...BASE, + password: PLACEHOLDER_PASSWORD, + secret: PLACEHOLDER_PASSWORD, + }), + ); + + expect(records.has('r7p_secret')).toBe(false); + expect(secrets.size).toBe(0); + expect(ops).toEqual([]); + }); +}); + +describe('#28549 — PATCH /api/v1/datasources/:name refuses a top-level credential by name', () => { + it.each([ + ['password', { password: PLACEHOLDER_PASSWORD }], + ['connectionString', { connectionString: PLACEHOLDER_DSN }], + ])('refuses a top-level `%s`, leaving the stored record as it was', async (key, extra) => { + const { send, records, ops } = makeDoor([STORED]); + + const message = await expectRefusal(await send('PATCH', '/datasources/wh', { config: BASE.config, ...extra })); + + expect(message).toBe(`Invalid datasource 'wh'.\n${specRefusal([key])}`); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(records.get('wh')).toEqual(STORED); + expect(ops).toEqual([]); + }); + + it('refuses it on a `label`-only edit too: the exemption covers a stored row, not a credential in the body', async () => { + const { send, records, ops } = makeDoor([STORED]); + + await expectRefusal(await send('PATCH', '/datasources/wh', { label: 'Warehouse', password: PLACEHOLDER_PASSWORD })); + + expect(records.get('wh')).toEqual(STORED); + expect(ops).toEqual([]); + }); +}); + +describe('#28549 — controls: the doors keep their other answers', () => { + it('still creates a datasource whose credential arrives as `secret`, bound at `external.credentialsRef`', async () => { + const { send, records, secrets } = makeDoor(); + + const res = await send('POST', '/datasources', { name: 'wh_ok', ...BASE, secret: PLACEHOLDER_PASSWORD }); + + expect(res.status).toBe(201); + const ref = records.get('wh_ok')?.external?.credentialsRef; + expect(ref).toMatch(/^sys_secret:/); + expect(secrets.get(ref!)).toBe(PLACEHOLDER_PASSWORD); + expect(JSON.stringify(records.get('wh_ok'))).not.toContain(PLACEHOLDER_PASSWORD); + }); + + it('still answers an edit that keeps the bound `external.credentialsRef`', async () => { + const bound: StoredDatasource = { ...STORED, external: { credentialsRef: 'sys_secret:sec_wh_own' } }; + const { send, records } = makeDoor([bound]); + + const res = await send('PATCH', '/datasources/wh', { label: 'Warehouse' }); + + expect(res.status).toBe(200); + expect(records.get('wh')?.external?.credentialsRef).toBe('sys_secret:sec_wh_own'); + }); + + it('keeps the driver contract\'s refusal of `config.password`', async () => { + const { send, records, ops } = makeDoor(); + + const message = await expectRefusal( + await send('POST', '/datasources', { + name: 'wh_cfg', + ...BASE, + config: { ...BASE.config, password: PLACEHOLDER_PASSWORD }, + }), + ); + + expect(message.split('\n')[0]).toBe("Invalid configuration for driver 'postgres'."); + expect(message).toContain('config.password'); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(records.has('wh_cfg')).toBe(false); + expect(ops).toEqual([]); + }); +}); diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index 420e0221f6d..fae370d6fc0 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -86,6 +86,17 @@ const PROBE_DRAFT_NAME = 'connection_test_draft'; */ const RECORD_JUDGED_PATCH_KEYS = ['driver', 'schemaMode', 'config', 'external', 'pool'] as const; +/** + * [#28549] The top-level body keys a credential arrives under that the record + * projection ({@link DatasourceAdminService.toRecord}, the update merge) would + * otherwise drop without a word: a `password`, and a `connectionString` that + * can carry one as URL userinfo. `DatasourceSchema` refuses both at the build + * door; these doors judge exactly these keys through it. Not every key the + * projection drops: those carry no credential, and a closed body at these + * doors is a change of its own. + */ +const TOP_LEVEL_CREDENTIAL_KEYS = ['password', 'connectionString'] as const; + /** A copy of `record` carrying the binding a supplied secret is about to create. */ function withPendingSecretRef(record: StoredDatasource): StoredDatasource { return { ...record, external: { ...(record.external ?? {}), credentialsRef: PENDING_SECRET_REF } }; @@ -667,6 +678,7 @@ export class DatasourceAdminService implements IDatasourceAdminService { async createDatasource(input: DatasourceDraft, secret?: SecretInput): Promise { this.assertValidName(input?.name); this.assertNoDraftCredentialsRef(input); + this.assertNoTopLevelCredential(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 @@ -728,6 +740,10 @@ export class DatasourceAdminService implements IDatasourceAdminService { if (existing.origin !== 'runtime') { throw new Error(`Datasource '${name}' is code-defined and cannot be edited at runtime.`); } + // [#28549] Whatever else the patch touches, a `label`/`active`-only edit + // included: that exemption spares a row stored before a gate, and this + // judges a credential the author is writing now. + this.assertNoTopLevelCredential(existing.name, patch); // Merge patch over the existing record; `name`/`origin` are never patched. const merged: StoredDatasource = { @@ -1244,6 +1260,35 @@ export class DatasourceAdminService implements IDatasourceAdminService { ); } + /** + * [#28549] Refuse a body that writes a credential at the top level + * ({@link TOP_LEVEL_CREDENTIAL_KEYS}). The record is built from the known + * keys only, so such a key would vanish and the write answer success over a + * datasource that cannot connect, while the build door refuses the same + * datasource by name. + * + * The verdict and its prescription are `DatasourceSchema`'s own refusal of + * those keys, the one `os compile` prints, under the heading + * {@link assertValidRecord} gives. Only the key names are judged, never the + * values, so the answer names the key and cannot echo the credential. ⛔ No + * prescription is restated here; the spec is the one copy. Thrown before + * anything is read or written (a `400` at the route). + */ + private assertNoTopLevelCredential(name: string, body: object): void { + const present = TOP_LEVEL_CREDENTIAL_KEYS.filter( + (key) => (body as Record)[key] !== undefined, + ); + if (present.length === 0) return; + const result = DatasourceSchema.safeParse(Object.fromEntries(present.map((key) => [key, null]))); + const detail = result.success + ? [] + : result.error.issues + .filter((issue) => issue.code === 'unrecognized_keys' && issue.path.length === 0) + .map((issue) => issue.message); + if (detail.length === 0) return; + throw new Error(`Invalid datasource '${name}'.\n${detail.join('\n')}`); + } + private toRecord(input: DatasourceDraft): StoredDatasource { return { name: input.name, From d808a13d8dc06da2ab42919be41edfecf5581846 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 08:06:12 +0000 Subject: [PATCH 2/7] fix(service-datasource)!: refuse every top-level key DatasourceSchema refuses at the datasource test, create and update doors The unsaved-draft probe (POST /api/v1/datasources/test) and the two write doors built their record from the known keys only, so a top-level password on the probe, and any other key the strict shape refuses (url one level too high, an alias such as type, a retired block, an unknown key) on all three doors, vanished without a word. The doors now refuse them 400 DATASOURCE_ADMIN_ERROR with DatasourceSchema's own root unrecognized-key refusal, derived from the body's key names at run time; values are never judged or echoed. Co-Authored-By: Claude --- ...asource-admin-top-level-credential.test.ts | 125 +++++++++++++++++- .../src/datasource-admin-service.ts | 67 +++++----- 2 files changed, 159 insertions(+), 33 deletions(-) diff --git a/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts b/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts index 196eb24d895..9b98dc141ef 100644 --- a/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts +++ b/packages/services/service-datasource/src/__tests__/datasource-admin-top-level-credential.test.ts @@ -50,7 +50,10 @@ function makeDoor(seed: StoredDatasource[] = []) { const ops: string[] = []; const config: DatasourceAdminServiceConfig = { - probe: async () => ({ ok: true, latencyMs: 1 }), + probe: async () => { + ops.push('probe'); + return { ok: true, latencyMs: 1 }; + }, listDatasourceRecords: async () => [...records.values()].map((r) => structuredClone(r)), getDatasourceRecord: async (name) => { const r = records.get(name); @@ -275,3 +278,123 @@ describe('#28549 — controls: the doors keep their other answers', () => { expect(ops).toEqual([]); }); }); + +/** + * #28612 — the rest of #28549: the unsaved-draft probe, and every top-level + * key `DatasourceSchema` refuses, not only the two credential keys. + * + * `POST /api/v1/datasources/test` built its probe through the same known-key + * projection, so a top-level `password` vanished and the probe tested a + * different connection than the one described. And the write doors dropped + * every other key the strict shape refuses (`foo`, a `url` one level too high, + * an alias such as `type`). All three doors now give the schema's own + * root refusal for whatever top-level keys it does not declare. Keys the + * schema declares stay accepted, and `secret` (split off by the route before + * the service sees the draft) stays the credential channel. + */ +describe('#28612 — POST /api/v1/datasources/test refuses a top-level key the schema refuses', () => { + const PROBE_HEADING = 'This connection draft is not a valid datasource.'; + + it.each([ + ['password', { password: PLACEHOLDER_PASSWORD }], + ['connectionString', { connectionString: PLACEHOLDER_DSN }], + ['foo', { foo: 'bar' }], + ])('refuses a top-level `%s` before any probe runs', async (key, extra) => { + const { send, records, secrets, ops } = makeDoor(); + + const message = await expectRefusal(await send('POST', '/datasources/test', { ...BASE, ...extra })); + + expect(message).toBe(`${PROBE_HEADING}\n${specRefusal([key])}`); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(records.size).toBe(0); + expect(secrets.size).toBe(0); + expect(ops).toEqual([]); + }); + + it('refuses it beside a supplied `secret` too', async () => { + const { send, ops } = makeDoor(); + + const message = await expectRefusal( + await send('POST', '/datasources/test', { ...BASE, password: PLACEHOLDER_PASSWORD, secret: PLACEHOLDER_PASSWORD }), + ); + + expect(message).toBe(`${PROBE_HEADING}\n${specRefusal(['password'])}`); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(ops).toEqual([]); + }); +}); + +describe('#28612 — the write doors refuse every top-level key the schema refuses', () => { + it.each([ + ['foo', { foo: 'bar' }], + ['url', { url: PLACEHOLDER_DSN }], + ['type', { type: 'postgres' }], + ['readReplicas', { readReplicas: [] }], + ])('POST refuses a top-level `%s`, storing nothing', async (key, extra) => { + const { send, records, secrets, ops } = makeDoor(); + + const message = await expectRefusal(await send('POST', '/datasources', { name: 'u_key', ...BASE, ...extra })); + + expect(message).toBe(`Invalid datasource 'u_key'.\n${specRefusal([key])}`); + expect(message).not.toContain(PLACEHOLDER_PASSWORD); + expect(records.has('u_key')).toBe(false); + expect(secrets.size).toBe(0); + expect(ops).toEqual([]); + }); + + it('POST names every refused key in one answer', async () => { + const { send, records } = makeDoor(); + + const message = await expectRefusal( + await send('POST', '/datasources', { name: 'u_many', ...BASE, foo: 1, host: 'db.example.invalid', password: PLACEHOLDER_PASSWORD }), + ); + + expect(message).toBe(`Invalid datasource 'u_many'.\n${specRefusal(['foo', 'host', 'password'])}`); + expect(records.has('u_many')).toBe(false); + }); + + it.each([ + ['foo', { config: BASE.config, foo: 'bar' }], + ['foo', { label: 'Warehouse', foo: 'bar' }], + ['hasSecret', { label: 'Warehouse', hasSecret: false }], + ])('PATCH refuses a top-level `%s`, leaving the stored record as it was', async (key, body) => { + const { send, records, ops } = makeDoor([STORED]); + + const message = await expectRefusal(await send('PATCH', '/datasources/wh', body)); + + expect(message).toBe(`Invalid datasource 'wh'.\n${specRefusal([key])}`); + expect(records.get('wh')).toEqual(STORED); + expect(ops).toEqual([]); + }); +}); + +describe('#28612 — controls: declared keys and the secret channel still pass', () => { + it('still probes a valid draft, its credential arriving as `secret`', async () => { + const { send, ops } = makeDoor(); + + const res = await send('POST', '/datasources/test', { ...BASE, secret: PLACEHOLDER_PASSWORD }); + + expect(res.status).toBe(200); + const body = (await res.json()) as { data?: { result?: { ok: boolean } } }; + expect(body.data?.result?.ok).toBe(true); + expect(ops).toEqual(['probe']); + }); + + it('still creates with a key the schema declares and the projection leaves out (`description`, `origin`)', async () => { + const { send, records } = makeDoor(); + + const res = await send('POST', '/datasources', { name: 'wh_decl', ...BASE, description: 'Warehouse', origin: 'code' }); + + expect(res.status).toBe(201); + expect(records.get('wh_decl')?.origin).toBe('runtime'); + }); + + it('still edits with a key the schema declares (`name`, `origin`)', async () => { + const { send, records } = makeDoor([STORED]); + + const res = await send('PATCH', '/datasources/wh', { name: 'wh', origin: 'runtime', label: 'Warehouse' }); + + expect(res.status).toBe(200); + expect(records.get('wh')?.label).toBe('Warehouse'); + }); +}); diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index fae370d6fc0..30943306cc1 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -86,17 +86,6 @@ const PROBE_DRAFT_NAME = 'connection_test_draft'; */ const RECORD_JUDGED_PATCH_KEYS = ['driver', 'schemaMode', 'config', 'external', 'pool'] as const; -/** - * [#28549] The top-level body keys a credential arrives under that the record - * projection ({@link DatasourceAdminService.toRecord}, the update merge) would - * otherwise drop without a word: a `password`, and a `connectionString` that - * can carry one as URL userinfo. `DatasourceSchema` refuses both at the build - * door; these doors judge exactly these keys through it. Not every key the - * projection drops: those carry no credential, and a closed body at these - * doors is a change of its own. - */ -const TOP_LEVEL_CREDENTIAL_KEYS = ['password', 'connectionString'] as const; - /** A copy of `record` carrying the binding a supplied secret is about to create. */ function withPendingSecretRef(record: StoredDatasource): StoredDatasource { return { ...record, external: { ...(record.external ?? {}), credentialsRef: PENDING_SECRET_REF } }; @@ -635,6 +624,14 @@ export class DatasourceAdminService implements IDatasourceAdminService { } async testConnection(input: DatasourceDraft, secret?: SecretInput): Promise { + // [#28612] First, and as a refusal (a `400` at the route) rather than a + // failed probe: the probe below is built from the known keys only, so a + // top-level `password` or any other key the schema refuses would vanish + // and the answer would be about a different connection than the one + // described. + if (input && typeof input === 'object') { + this.assertNoUndeclaredTopLevelKey('This connection draft is not a valid datasource.', input); + } if (!input?.driver) { return { ok: false, error: 'A driver is required to test a connection.' }; } @@ -678,7 +675,7 @@ export class DatasourceAdminService implements IDatasourceAdminService { async createDatasource(input: DatasourceDraft, secret?: SecretInput): Promise { this.assertValidName(input?.name); this.assertNoDraftCredentialsRef(input); - this.assertNoTopLevelCredential(input.name, input); + this.assertNoUndeclaredTopLevelKey(`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 @@ -740,10 +737,10 @@ export class DatasourceAdminService implements IDatasourceAdminService { if (existing.origin !== 'runtime') { throw new Error(`Datasource '${name}' is code-defined and cannot be edited at runtime.`); } - // [#28549] Whatever else the patch touches, a `label`/`active`-only edit - // included: that exemption spares a row stored before a gate, and this - // judges a credential the author is writing now. - this.assertNoTopLevelCredential(existing.name, patch); + // [#28549 · #28612] Whatever else the patch touches, a `label`/`active`-only + // 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); // Merge patch over the existing record; `name`/`origin` are never patched. const merged: StoredDatasource = { @@ -1261,23 +1258,29 @@ export class DatasourceAdminService implements IDatasourceAdminService { } /** - * [#28549] Refuse a body that writes a credential at the top level - * ({@link TOP_LEVEL_CREDENTIAL_KEYS}). The record is built from the known - * keys only, so such a key would vanish and the write answer success over a - * datasource that cannot connect, while the build door refuses the same - * datasource by name. + * [#28549 · #28612] Refuse a body that carries a top-level key + * `DatasourceSchema` does not declare: a credential (`password`, a + * `connectionString` with URL userinfo), a connection key one level too high + * (`url`, `host`, …), an alias (`type`), a retired block, or any unknown key. + * The record and the probe are built from the known keys only, so such a key + * would vanish and the door answer success (or probe) over a datasource + * other than the one described, while the build door refuses it by name. + * + * The key set and the prescription are `DatasourceSchema`'s own: the body's + * key names, each with `null` for its value, are parsed through the strict + * shape and only its root `unrecognized_keys` refusal is kept, the one + * `os compile` prints. A key the schema declares passes here even when the + * projection leaves it out (`origin`, `description`, …). Only key names are + * judged, never values, so the answer cannot echo a credential. ⛔ No key + * list or prescription is restated here; the spec is the one copy. Thrown + * before anything is read, written or probed (a `400` at the route). * - * The verdict and its prescription are `DatasourceSchema`'s own refusal of - * those keys, the one `os compile` prints, under the heading - * {@link assertValidRecord} gives. Only the key names are judged, never the - * values, so the answer names the key and cannot echo the credential. ⛔ No - * prescription is restated here; the spec is the one copy. Thrown before - * anything is read or written (a `400` at the route). + * `secret` never reaches this check: the route splits it off the body + * (`splitSecret`) before the service sees the draft, and it stays the one + * credential channel. */ - private assertNoTopLevelCredential(name: string, body: object): void { - const present = TOP_LEVEL_CREDENTIAL_KEYS.filter( - (key) => (body as Record)[key] !== undefined, - ); + private assertNoUndeclaredTopLevelKey(heading: string, body: object): void { + const present = Object.keys(body).filter((key) => (body as Record)[key] !== undefined); if (present.length === 0) return; const result = DatasourceSchema.safeParse(Object.fromEntries(present.map((key) => [key, null]))); const detail = result.success @@ -1286,7 +1289,7 @@ export class DatasourceAdminService implements IDatasourceAdminService { .filter((issue) => issue.code === 'unrecognized_keys' && issue.path.length === 0) .map((issue) => issue.message); if (detail.length === 0) return; - throw new Error(`Invalid datasource '${name}'.\n${detail.join('\n')}`); + throw new Error(`${heading}\n${detail.join('\n')}`); } private toRecord(input: DatasourceDraft): StoredDatasource { From d3bd3c6c0069a2b3893e07b1106fd01ab6b19421 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 08:07:40 +0000 Subject: [PATCH 3/7] chore(changeset): service-datasource patch for the undeclared top-level key refusal Co-Authored-By: Claude --- ...8612-datasource-undeclared-top-level-key.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .changeset/28612-datasource-undeclared-top-level-key.md diff --git a/.changeset/28612-datasource-undeclared-top-level-key.md b/.changeset/28612-datasource-undeclared-top-level-key.md new file mode 100644 index 00000000000..9287c9f062f --- /dev/null +++ b/.changeset/28612-datasource-undeclared-top-level-key.md @@ -0,0 +1,18 @@ +--- +'@objectstack/service-datasource': patch +--- + +fix(service-datasource)!: `POST /api/v1/datasources/test`, `POST /api/v1/datasources` and `PATCH /api/v1/datasources/:name` refuse every top-level key `DatasourceSchema` refuses, with that schema's own message, instead of dropping it + +Clause-②: no (narrowing) — a test, create or edit body carrying a top-level key the datasource schema does not declare (a `password` on the test door; on all three doors a `url` or `host` one level too high, an alias such as `type`, a retired block such as `readReplicas`, or any unknown key), which the test door probed without and the write doors answered 201 / 200 with the key silently dropped, is refused with `400 DATASOURCE_ADMIN_ERROR`. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the datasource admin doors (`@objectstack/service-datasource`). + +- **Before.** The unsaved-draft probe and the two write doors built their record from its known keys only. A top-level `password` on the probe vanished, so the probe tested a different connection than the one described. On all three doors any other key the strict schema refuses vanished the same way, and the write answered success. +- **Now.** A top-level key `DatasourceSchema` does not declare is refused before anything is read, written or probed, naming the key and never its value. The message is the schema's own refusal, the one `os compile` prints: for example `url` belongs inside `config`, `type` is `driver`, `readReplicas` is retired. The test door answers under the heading `This connection draft is not a valid datasource.`; create and edit under `Invalid datasource 'NAME'.`. This supersedes the "other unknown top-level keys are still dropped as before" line of the #28549 entry. +- **Unchanged.** Keys the schema declares pass the check, including the ones the doors do not store (`origin`, `description`, `ssl`, `autoConnect`). A credential sent as `secret` is still split off by the route and bound at `external.credentialsRef`. The probe's other answers (`ok: false` with a reason) are unchanged. + +**The remedy.** Remove the key, or move it where the message says: connection keys into `config`, the credential into `secret`. Keys copied from a `GET /api/v1/datasources` or `GET /api/v1/datasources/NAME` answer that the schema does not declare (`hasSecret`, `redactedConfigKeys`, `definedIn`, `status`, `statusReason`) are not sent back on an edit. From 56b83f9b96d6711a1af0df75a37af57de1bcbb2e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 09:46:41 +0000 Subject: [PATCH 4/7] fix(service-datasource): the datasource admin doors keep every top-level key DatasourceSchema declares Co-Authored-By: Claude --- ...atasource-admin-declared-keys-kept.test.ts | 330 ++++++++++++++++++ .../src/contracts/datasource-admin-service.ts | 10 + .../src/datasource-admin-plugin.ts | 4 + .../src/datasource-admin-service.ts | 172 +++++++-- .../src/datasource-connectivity-change.ts | 7 +- 5 files changed, 499 insertions(+), 24 deletions(-) create mode 100644 packages/services/service-datasource/src/__tests__/datasource-admin-declared-keys-kept.test.ts 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..7b9d360d3b5 --- /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.at(-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..8b5420db165 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,102 @@ 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, where every metadata read of the row + * serves it (that read path redacts `config`, not `ssl`). 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 +244,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 +260,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 +700,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 +721,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 +763,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 +780,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 +796,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 +862,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 +910,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 +956,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 +1422,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; From b202d460c95734067e0a6412bc7069c99d61ecb2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 09:50:50 +0000 Subject: [PATCH 5/7] test(service-datasource): typecheck-clean last-element read; changeset for the declared-key projection Co-Authored-By: Claude --- .../28633-datasource-admin-declared-keys.md | 20 +++++++++++++++++++ ...atasource-admin-declared-keys-kept.test.ts | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 .changeset/28633-datasource-admin-declared-keys.md 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/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 index 7b9d360d3b5..10a9783bd70 100644 --- 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 @@ -211,7 +211,7 @@ describe('PATCH /api/v1/datasources/:name — edit keeps every declared key (#28 expect(res.status).toBe(200); expect(records.get('wh')?.ssl).toEqual(SSL); expect(ops).toContain('reregisterPool'); - expect(pooled.at(-1)?.ssl).toEqual(SSL); + expect(pooled[pooled.length - 1]?.ssl).toEqual(SSL); }); it('stores autoConnect and description', async () => { From a946c3f276c1d3fd8d6bb915ecd483b7380530ab Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 09:52:52 +0000 Subject: [PATCH 6/7] docs(data-modeling): the Setup datasource doors store the ssl block and refuse its private key Co-Authored-By: Claude --- content/docs/data-modeling/drivers.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 From 88d0f1516b37007ceecd25ea8ab2baf4ffd88d98 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 09:55:01 +0000 Subject: [PATCH 7/7] docs(service-datasource): the ssl.key refusal comment states the storage reason only Co-Authored-By: Claude --- .../service-datasource/src/datasource-admin-service.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index 8b5420db165..ef8fb4d4c1a 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -150,10 +150,9 @@ const SSL_PRIVATE_KEY = 'key'; * 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, where every metadata read of the row - * serves it (that read path redacts `config`, not `ssl`). Refused by name, - * never by value, before anything is read, written or probed. The rest of the - * block (`enabled`, `rejectUnauthorized`, `ca`, `cert`) is stored. + * 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;