From 0cc981aa238a292eff24079a3dfcb7a9fe55df1a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 07:00:47 +0000 Subject: [PATCH 1/3] 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/3] 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/3] 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.