diff --git a/.changeset/27722-datasource-create-credentials-ref.md b/.changeset/27722-datasource-create-credentials-ref.md new file mode 100644 index 00000000000..9dd9091b4ce --- /dev/null +++ b/.changeset/27722-datasource-create-credentials-ref.md @@ -0,0 +1,19 @@ +--- +'@objectstack/service-datasource': patch +--- + +fix(service-datasource)!: a datasource create may not name a stored credential reference; it stores only the one its own secret is bound under (#27722) + +Clause-②: no (narrowing) — the datasource admin door's create refuses a draft that names `external.credentialsRef`, where it stored the value as given before. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing on the datasource admin door's create (`POST /api/v1/datasources`, `IDatasourceAdminService.createDatasource`), the same rule #27582 set for `secret` fields: a write may only seal a new value or keep its own record's stored reference. + +- **Before.** A create draft's `external.credentialsRef` was stored as given, beside or instead of the reference a supplied `secret` is bound under. +- **Now.** A create has no stored reference of its own, so a draft that names `external.credentialsRef` is refused with `400` before anything is written: no record, no secret, no pool. The message names the key and never the value, and the answer is the same whatever the value names. The only reference a create stores is the one its own `secret` is bound under. +- **Unchanged.** An update keeps the record's own reference unless a supplied `secret` rewraps it, as before. A create with a `secret` and no `external.credentialsRef` binds it as before. The Console's datasource form never sends the key, so it is unaffected. +- **Who sees it.** A caller of the admin door's create that sends `external.credentialsRef`. + +**The remedy.** Send the credential as `secret` and leave `external.credentialsRef` out; the door binds it and stores the reference. diff --git a/packages/services/service-datasource/src/__tests__/datasource-credentials-ref-own-reference.test.ts b/packages/services/service-datasource/src/__tests__/datasource-credentials-ref-own-reference.test.ts new file mode 100644 index 00000000000..09d21eb3820 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/datasource-credentials-ref-own-reference.test.ts @@ -0,0 +1,197 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #27722 — the datasource admin door applies the #27582 own-reference rule to + * `external.credentialsRef`: **a write may only seal a new value or keep its + * own record's stored reference.** + * + * The door's callers are the admin routes (`POST` / `PATCH /datasources`), a + * signed-in operator every time; nothing system-side writes through this + * service, so there is no pass-through to keep here. + * + * - **create** — a new record has no reference of its own to keep, so a draft + * that names `external.credentialsRef` is refused, with one answer whatever + * it names, before anything is written. The credential arrives as `secret` + * and the door binds it; that reference is the only one a create stores. + * - **update** — the record keeps the reference it already holds unless a + * supplied `secret` rewraps it. A patch that names any other reference never + * reaches the stored record. + * + * The harness is the service's own config seam, so each case reads exactly + * what the door persisted, bound, removed and registered. + */ + +import { describe, expect, it } from 'vitest'; +import { + DatasourceAdminService, + type DatasourceAdminServiceConfig, + type StoredDatasource, +} from '../datasource-admin-service.js'; + +/** A reference some other write minted — the kind a caller must not store. */ +const OTHER_REF = 'sys_secret:sec_minted_elsewhere'; + +function makeHarness(seed: StoredDatasource[] = []) { + const records: StoredDatasource[] = seed.map((r) => ({ ...r })); + const secrets = new Map([[OTHER_REF, 'other-credential']]); + const ops: string[] = []; + let seq = 0; + + const config: DatasourceAdminServiceConfig = { + probe: async () => ({ ok: true }), + listDatasourceRecords: async () => records.map((r) => ({ ...r })), + getDatasourceRecord: async (name) => { + const r = records.find((x) => x.name === name); + return r ? { ...r } : undefined; + }, + putDatasourceRecord: async (record) => { + ops.push(`put:${record.name}`); + const idx = records.findIndex((r) => r.name === record.name); + if (idx >= 0) records[idx] = { ...record }; + else records.push({ ...record }); + }, + deleteDatasourceRecord: async (name) => { + ops.push(`delete:${name}`); + const idx = records.findIndex((r) => r.name === name); + if (idx >= 0) records.splice(idx, 1); + }, + writeSecret: async (input) => { + const ref = `sys_secret:sec_minted_${++seq}`; + ops.push(`writeSecret:${ref}`); + secrets.set(ref, input.value); + return ref; + }, + removeSecret: async (ref) => { + ops.push(`removeSecret:${ref}`); + secrets.delete(ref); + }, + readSecret: async (ref) => secrets.get(ref), + countBoundObjects: async () => 0, + listObjectsBoundTo: async () => [], + registerPool: (record) => { + ops.push(`registerPool:${record.name}`); + }, + unregisterPool: (name) => { + ops.push(`unregisterPool:${name}`); + }, + }; + + const service = new DatasourceAdminService(config); + const stored = (name: string) => records.find((r) => r.name === name); + return { service, stored, secrets, ops }; +} + +const DRAFT = { + name: 'reporting', + driver: 'postgres', + config: { host: 'db.internal', database: 'analytics' }, +}; + +describe('#27722 — a create may not carry a stored credential reference', () => { + it.each([ + ['a reference another write minted', OTHER_REF], + ['a reference that names nothing', 'sys_secret:sec_never_minted'], + ['a value in no reference shape', 'not-a-reference'], + ])('refuses %s, writing nothing', async (_label, ref) => { + const { service, stored, secrets, ops } = makeHarness(); + + await expect( + service.createDatasource({ ...DRAFT, external: { credentialsRef: ref } }), + ).rejects.toThrow(/external\.credentialsRef/); + + expect(stored('reporting')).toBeUndefined(); + expect(ops).toEqual([]); + expect(secrets.get(OTHER_REF)).toBe('other-credential'); + }); + + it('answers the same whatever the reference names, and never echoes it', async () => { + const messages: string[] = []; + for (const ref of [OTHER_REF, 'sys_secret:sec_never_minted', 'not-a-reference']) { + const { service } = makeHarness(); + const err = await service + .createDatasource({ ...DRAFT, external: { credentialsRef: ref } }) + .catch((e: unknown) => e as Error); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).not.toContain(ref); + messages.push((err as Error).message); + } + expect(new Set(messages).size).toBe(1); + }); + + it('refuses it beside a supplied secret too: the door mints the only reference a create stores', async () => { + const { service, stored, ops } = makeHarness(); + + await expect( + service.createDatasource( + { ...DRAFT, external: { credentialsRef: OTHER_REF } }, + { value: 'fresh-credential' }, + ), + ).rejects.toThrow(/external\.credentialsRef/); + + expect(stored('reporting')).toBeUndefined(); + expect(ops).toEqual([]); + }); + + it('stores the reference its own secret was sealed under', async () => { + const { service, stored, secrets } = makeHarness(); + + await service.createDatasource(DRAFT, { value: 'fresh-credential' }); + + const ref = stored('reporting')?.external?.credentialsRef; + expect(ref).toMatch(/^sys_secret:sec_minted_/); + expect(ref).not.toBe(OTHER_REF); + expect(secrets.get(ref!)).toBe('fresh-credential'); + }); + + it('still creates a record that names no reference at all', async () => { + const { service, stored } = makeHarness(); + + await service.createDatasource({ ...DRAFT, schemaMode: 'external', external: { allowWrites: false } }); + + expect(stored('reporting')).toBeDefined(); + expect(stored('reporting')?.external?.credentialsRef).toBeUndefined(); + }); +}); + +describe('#27722 — an update keeps its own reference and stores no other', () => { + const OWN_REF = 'sys_secret:sec_own'; + + it('keeps the record\'s own reference when a patch names another', async () => { + const { service, stored, secrets, ops } = makeHarness([ + { ...DRAFT, origin: 'runtime', external: { credentialsRef: OWN_REF } }, + ]); + + await service.updateDatasource('reporting', { external: { credentialsRef: OTHER_REF } }); + + expect(stored('reporting')?.external?.credentialsRef).toBe(OWN_REF); + expect(ops).not.toContain(`removeSecret:${OTHER_REF}`); + expect(secrets.get(OTHER_REF)).toBe('other-credential'); + }); + + it('stores no reference on a record that holds none when a patch names one', async () => { + const { service, stored } = makeHarness([{ ...DRAFT, origin: 'runtime' }]); + + await service.updateDatasource('reporting', { external: { credentialsRef: OTHER_REF } }); + + expect(stored('reporting')?.external?.credentialsRef).toBeUndefined(); + }); + + it('replaces its own reference only with the one a supplied secret is sealed under', async () => { + const { service, stored, secrets, ops } = makeHarness([ + { ...DRAFT, origin: 'runtime', external: { credentialsRef: OWN_REF } }, + ]); + + await service.updateDatasource( + 'reporting', + { external: { credentialsRef: OTHER_REF } }, + { value: 'rotated-credential' }, + ); + + const ref = stored('reporting')?.external?.credentialsRef; + expect(ref).toMatch(/^sys_secret:sec_minted_/); + expect(secrets.get(ref!)).toBe('rotated-credential'); + expect(ops).toContain(`removeSecret:${OWN_REF}`); + expect(ops).not.toContain(`removeSecret:${OTHER_REF}`); + expect(secrets.get(OTHER_REF)).toBe('other-credential'); + }); +}); 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 a62d063af44..d287b3a7876 100644 --- a/packages/services/service-datasource/src/contracts/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/contracts/datasource-admin-service.ts @@ -160,7 +160,9 @@ export interface IDatasourceAdminService { /** * Persist a new runtime datasource (`origin: 'runtime'`, environment-scoped). - * Rejects when a code-defined datasource of the same name exists. + * Rejects when a code-defined datasource of the same name exists, and rejects + * a draft that names `external.credentialsRef`: the only reference a create + * stores is the one a supplied `secret` is bound under. */ createDatasource(input: DatasourceDraft, secret?: SecretInput): Promise; diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index e6cea3eff55..d1ce3f7ad72 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -649,6 +649,7 @@ export class DatasourceAdminService implements IDatasourceAdminService { async createDatasource(input: DatasourceDraft, secret?: SecretInput): Promise { this.assertValidName(input?.name); + this.assertNoDraftCredentialsRef(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 @@ -1175,6 +1176,29 @@ export class DatasourceAdminService implements IDatasourceAdminService { } } + /** + * [#27722] The #27582 rule at this door: a write may only seal a new value or + * keep its own record's stored reference. A create has no record of its own, + * so the only `external.credentialsRef` it stores is the one a supplied + * `secret` is bound under, and a draft that names one is refused. One answer + * whatever it names: the message names the key and never its value, and + * nothing about the reference is looked up. Thrown before anything is + * written (a `400` at the route). + * + * An update needs no such refusal: it keeps the stored reference unless a + * supplied secret rewraps it, whatever the patch's `external` says. + */ + private assertNoDraftCredentialsRef(input: DatasourceDraft): void { + const external = input.external as { credentialsRef?: unknown } | undefined; + if (external?.credentialsRef === undefined) return; + throw new Error( + `Invalid datasource '${input.name}': \`external.credentialsRef\` is not accepted on create. ` + + 'It names a credential already stored, and only the reference this door binds a supplied `secret` ' + + 'under can be stored on a new datasource. Send the credential as `secret`, and leave ' + + '`external.credentialsRef` out. Nothing was written.', + ); + } + private toRecord(input: DatasourceDraft): StoredDatasource { return { name: input.name, diff --git a/packages/services/service-settings/src/settings-secret-handle-own-reference.pin.test.ts b/packages/services/service-settings/src/settings-secret-handle-own-reference.pin.test.ts new file mode 100644 index 00000000000..d6ed64f96b9 --- /dev/null +++ b/packages/services/service-settings/src/settings-secret-handle-own-reference.pin.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #27722 — the settings write door against the #27582 own-reference rule: + * **a write may only seal a new value or keep its own record's stored + * reference.** + * + * Measured, not changed: this door never takes a reference from its caller at + * all. For a declared-encrypted key, `setMany` seals whatever value it is + * handed and stores the handle that seal minted; the submitted value is never + * the stored handle, whatever shape it has. These pins hold that answer, so a + * later change that let a caller-supplied value stand in for the stored handle + * reds here. + * + * Each case names a live handle another organization's write minted, so the + * pin measures a real stored handle rather than one that names nothing. + * + * The writer is an enforced caller (the settings routes' context) holding the + * namespace's write capability. The in-process caller takes the same path: + * the door has no reference-bearing branch for any caller. + */ + +import { randomBytes } from 'node:crypto'; +import { describe, expect, it } from 'vitest'; +import type { SettingsManifest } from '@objectstack/spec/system'; +import { SettingsService } from './settings-service.js'; +import { LocalCryptoProvider } from './local-crypto-provider.js'; +import type { SettingsContext, SettingsSecretStore } from './settings-service.types.js'; + +const manifest: SettingsManifest = { + namespace: 'sms', + version: 1, + label: 'SMS', + scope: 'tenant', + readPermission: 'setup.access', + writePermission: 'setup.write', + specifiers: [ + { type: 'password', key: 'auth_token', label: 'Auth token', required: false, encrypted: true }, + ], +}; + +const asOrg = (tenantId: string): SettingsContext => ({ + tenantId, + userId: `admin_${tenantId}`, + enforced: true, + permissions: ['setup.access', 'setup.write'], +}); + +function boot() { + const rows = new Map>(); + const secretStore: SettingsSecretStore = { + async insert(row) { rows.set(row.id, { ...row }); return { id: row.id }; }, + async get(id) { return (rows.get(id) as any) ?? null; }, + async update(id, patch) { rows.set(id, { ...rows.get(id), ...patch }); }, + }; + const svc = new SettingsService({ + env: {}, + cryptoProvider: new LocalCryptoProvider({ key: randomBytes(32) }), + secretStore, + }); + svc.registerManifest(manifest); + return { svc, rows }; +} + +/** Org A holds a live secret; returns the handle its write minted. */ +async function seedOtherOrganization(svc: SettingsService, rows: Map) { + const before = new Set(rows.keys()); + await svc.set('sms', 'auth_token', 'org-a-credential', asOrg('org_a')); + const minted = [...rows.keys()].filter((id) => !before.has(id)); + expect(minted).toHaveLength(1); + return minted[0]; +} + +describe('#27722 — the settings write door stores only a handle its own write minted', () => { + it.each([ + ['the bare handle', (h: string) => h], + ['a `secret:` reference to it', (h: string) => `secret:${h}`], + ])('seals %s as a new value rather than storing it as the handle', async (_label, spell) => { + const { svc, rows } = boot(); + const foreign = await seedOtherOrganization(svc, rows); + const foreignRow = { ...(rows.get(foreign) as Record) }; + const submitted = spell(foreign); + + const before = new Set(rows.keys()); + await svc.set('sms', 'auth_token', submitted, asOrg('org_b')); + const minted = [...rows.keys()].filter((id) => !before.has(id)); + + // One new seal, under a handle that is not the submitted one. + expect(minted).toHaveLength(1); + expect(minted[0]).not.toBe(foreign); + // Org B reads back exactly what it wrote — never the credential the + // named handle holds. + expect((await svc.get('sms', 'auth_token', asOrg('org_b'))).value).toBe(submitted); + // The other organization's handle and its value are untouched. + expect(rows.get(foreign)).toEqual(foreignRow); + expect((await svc.get('sms', 'auth_token', asOrg('org_a'))).value).toBe('org-a-credential'); + }); +});