diff --git a/.changeset/datasource-def-credentials-ref-retained.md b/.changeset/datasource-def-credentials-ref-retained.md new file mode 100644 index 0000000000..0ebc44300c --- /dev/null +++ b/.changeset/datasource-def-credentials-ref-retained.md @@ -0,0 +1,56 @@ +--- +"@objectstack/objectql": minor +--- + +feat(objectql): retain and expose `external.credentialsRef` on datasource definitions (#12758) + +`ObjectQL.registerDatasourceDef`'s parameter type carried only `name`, +`schemaMode` and `external.allowWrites`, so a caller passing a fresh object +literal with `external.credentialsRef` was refused by excess-property checking +(`TS2353`) — while the docs (`/docs/data-modeling/external-datasources`) +prescribe exactly that key on a code-declared datasource, and +`@objectstack/spec` has declared it all along on +`ExternalDatasourceSettingsSchema`, valid in every `schemaMode` (#8153). The +engine also exposed **no reader at all** onto its datasource index; its sole +consumer was the private write gate. + +Measured before anything was changed: nothing stripped the reference at +runtime. The writer stores the caller's `external` object whole, by reference, +and the package-manifest install path spreads the def straight through — so the +value was already in the index, unreachable to every typed producer and every +consumer. The defect was type-level, and the fix is a widening plus the +accessor that was missing. + +- `registerDatasourceDef` now takes the named, exported `DatasourceDef`, whose + `external` block carries `credentialsRef?: string` beside `allowWrites`. + Retention, not invention: the key is the spec's, and every shape that + compiled before still compiles. +- New `ObjectQL.listDatasourceDefs()` answers every definition the engine + holds, from both entry routes. Deliberately unfiltered — `credentialsRef` is + valid on a managed datasource too, so filtering by schema mode would hide + live handles from a `sys_secret` reference sweep, and under-reporting is the + direction that deletes live credentials. Each entry carries a copied + `external` block so a reader cannot reach through it and mutate the write + gate's own input. + +Why this matters beyond tidiness: a datasource declared **in code** never +reaches `sys_metadata`, so the cross-producer `sys_secret` reference union +(#12663) cannot see the handle it holds and must be handed the list by its +host. That makes the completeness of the union — the precondition an orphan +sweep's deletion predicate rests on — depend on every caller remembering to +pass a list. This moves the guarantee from process to mechanism. The union is +not rewired here; that is consumer-side work on a shipped contract and is +tracked separately. + +The write gate is untouched: it reads `schemaMode` + `allowWrites`, the new key +is inert to it, and both directions of the gate stay pinned. + +**Why `minor` and not `patch`.** Zero runtime behaviour changes, which is the +honest case for `patch` — but the bump describes the **contract**, not the +bytes executed, and this release adds public API three ways: a new public +method (`listDatasourceDefs`), a newly exported type (`DatasourceDef`), and a +widened accepted set on an existing public method (calls that were rejected at +compile time now compile). A consumer pinning `~` would receive new API under a +`patch`, which misdescribes the release. Nothing is removed, narrowed or +renamed, so no breaking-change declaration and no ADR-0087 entry arise; `minor` +is the additive-surface bump, not the launch-window breaking convention. diff --git a/packages/objectql/src/datasource-def-credentials-ref.pin.ts b/packages/objectql/src/datasource-def-credentials-ref.pin.ts new file mode 100644 index 0000000000..1442f9535e --- /dev/null +++ b/packages/objectql/src/datasource-def-credentials-ref.pin.ts @@ -0,0 +1,87 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12758 — compile-time pin for the shape `registerDatasourceDef` accepts and + * the shape `listDatasourceDefs` answers. + * + * THE DEFECT THIS PINS WAS PURELY TYPE-LEVEL, which is why the pin lives here + * and not only in a `.test.ts`. Measured on the pre-change tree: nothing ever + * stripped `external.credentialsRef` at runtime — `registerDatasourceDef` + * stored the caller's `external` object whole, by reference, and the manifest + * install path spread the def straight through — so the reference was already + * in the engine's index. What did not exist was any way to put it there + * honestly or to read it back: + * + * - a caller passing a FRESH object literal was refused with TS2353 + * ("'credentialsRef' does not exist in type '{ allowWrites?: boolean }'"), + * so the only way in was a pre-typed variable or an `as any`; and + * - the engine exposed no accessor onto the index at all — its sole reader + * was the private write gate. + * + * A runtime test therefore cannot cover this card: the runtime never changed. + * The accepted set of a public method did, and only `tsc` can see that. + * + * WHY A `.pin.ts` AND NOT A `*.test.ts`: `packages/objectql/tsconfig.json` + * excludes `**\/*.test.ts`, so a `@ts-expect-error` written in a test file here + * is a phantom check — no tsc program the `typecheck` script runs would ever + * evaluate it, and deleting the directive would leave every gate green. This + * file IS in that program. Same convention, and same reasoning, as + * `register-object-authored-shape.pin.ts`. It carries no executable pin: the + * assertions live in a function nobody calls, and the companion + * `datasource-def-credentials-ref.test.ts` covers the runtime half. + */ + +import type { DatasourceDef, ObjectQL } from './engine.js'; + +/** + * Taken off the METHOD, not off {@link DatasourceDef}, so that re-narrowing the + * method's own signature moves this pin even if the named type survives. + */ +type RegisterArg = Parameters[0]; +type ListedDefs = ReturnType; + +/** + * Never called — every line is a type-level assertion evaluated by + * `tsc --noEmit`. The members are taken as parameters rather than read off a + * live engine so the pin needs no instance. + */ +export function __pinDatasourceDefCarriesCredentialsRef( + register: (def: RegisterArg) => void, + listed: ListedDefs, +): void { + // ── POSITIVE: the calls this card exists for. ──────────────────────────── + // FRESH object literals throughout — excess-property checking is the thing + // under test, so a pre-typed variable here would defeat the pin entirely. + register({ + name: 'warehouse', + schemaMode: 'external', + external: { allowWrites: true, credentialsRef: 'sys_secret:sec_1' }, + }); + // `credentialsRef` alone, no federation key: legal on a MANAGED datasource + // per #8153, and the shape the Studio wizard's createDatasource writes. + register({ name: 'warehouse', external: { credentialsRef: 'secret:warehouse/password' } }); + + // ── The pre-#12758 shapes must keep compiling — this is a WIDENING. ────── + register({ name: 'warehouse' }); + register({ name: 'warehouse', schemaMode: 'external', external: { allowWrites: true } }); + + // ── NEGATIVE: the widening must not admit garbage. ─────────────────────── + // @ts-expect-error `name` is required — a definition without one registers nothing + register({ schemaMode: 'external' }); + // @ts-expect-error `credentialsRef` is a REFERENCE into the secrets store, so a string + register({ name: 'warehouse', external: { credentialsRef: 12_345 } }); + // @ts-expect-error inline credentials are refused everywhere — `password` is not a key here + register({ name: 'warehouse', external: { password: 'hunter2' } }); + // @ts-expect-error the widening is scoped to credentialsRef; `validation` has no engine reader + register({ name: 'warehouse', external: { validation: { onMismatch: 'warn' } } }); + + // ── READ-BACK: the accessor answers definitions, keyed by name. ────────── + const one: DatasourceDef | undefined = listed[0]; + const ref: string | undefined = one?.external?.credentialsRef; + const gate: boolean | undefined = one?.external?.allowWrites; + void ref; + void gate; + // @ts-expect-error the accessor answers definitions, not bare datasource names + const notAName: string = listed[0]; + void notAName; +} diff --git a/packages/objectql/src/datasource-def-credentials-ref.test.ts b/packages/objectql/src/datasource-def-credentials-ref.test.ts new file mode 100644 index 0000000000..75fe07b025 --- /dev/null +++ b/packages/objectql/src/datasource-def-credentials-ref.test.ts @@ -0,0 +1,204 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #12758 — runtime half of the datasource-definition credentials-reference + * contract. The compile-time half is in + * `datasource-def-credentials-ref.pin.ts` (it has to be: this file is excluded + * from every tsc program the `typecheck` script runs, so a `@ts-expect-error` + * written here would never be evaluated). + * + * ⛔ NOTHING HERE IS PHRASED AS "the reference is no longer dropped". Measured + * on the pre-change tree, the reference was never dropped: `registerDatasourceDef` + * stored the caller's `external` object whole, by reference, and the manifest + * install path spread the def straight through. A test claiming otherwise would + * pin something that was never true. What IS new — and what this file covers — + * is that the value is now READABLE, through an accessor that did not exist: + * the engine had no reader onto its datasource index at all, only the private + * write gate. + * + * Why it matters: a datasource declared IN CODE never reaches `sys_metadata`, + * so the cross-producer `sys_secret` reference union cannot see the handle it + * holds and has to be handed the list by its host. This accessor is what lets + * the engine answer instead of the caller remembering. + */ + +import { describe, expect, it } from 'vitest'; +import type { IDataDriver } from '@objectstack/spec/contracts'; +import { ExternalWriteForbiddenError } from '@objectstack/spec/shared'; +import { ObjectQL } from './engine'; + +const REF = 'sys_secret:sec_credref_12758'; + +function makeDriver(name: string): IDataDriver { + const store = new Map>(); + return { + name, + version: '1.0.0', + async connect() {}, + async disconnect() {}, + async find() { return []; }, + async findOne() { return null; }, + async count() { return 0; }, + async create(object: string, data: Record) { + const id = (data.id as string) ?? String(store.size + 1); + const row = { ...data, id }; + store.set(`${object}:${id}`, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const row = { ...(store.get(`${object}:${id}`) ?? {}), ...data, id }; + store.set(`${object}:${id}`, row); + return row; + }, + async delete(object: string, id: string) { return store.delete(`${object}:${id}`); }, + async syncSchema() {}, + async dropTable() {}, + } as unknown as IDataDriver; +} + +/** The one definition, as every route below declares it. */ +const DEF = { + name: 'warehouse', + schemaMode: 'external', + external: { allowWrites: true, credentialsRef: REF }, +} as const; + +describe('datasource definitions retain external.credentialsRef and are readable (#12758)', () => { + describe('entry route 1 — the direct registerDatasourceDef call', () => { + it('lists the definition back with its credentials reference', () => { + const engine = new ObjectQL(); + // No cast. If the parameter is ever re-narrowed this line stops compiling + // in the pin file; here it is the runtime read-back that is under test. + engine.registerDatasourceDef({ ...DEF, external: { ...DEF.external } }); + + const listed = engine.listDatasourceDefs(); + expect(listed).toHaveLength(1); + expect(listed[0]).toMatchObject({ + name: 'warehouse', + schemaMode: 'external', + external: { allowWrites: true, credentialsRef: REF }, + }); + }); + }); + + describe('entry route 2 — the package-manifest install path (registerApp)', () => { + // The widest blast radius of the narrowing: a code-declared datasource + // reaches the engine here and nowhere else. Manifests may spell + // `datasources` as an array OR as a name-keyed map, and the two take + // different branches, so both are pinned. + it('retains the reference through the ARRAY spelling', () => { + const engine = new ObjectQL(); + engine.registerApp({ + id: 'wh_pkg_array', + name: 'Warehouse', + datasources: [{ ...DEF, external: { ...DEF.external } }], + }); + + expect(engine.listDatasourceDefs()).toEqual([ + { name: 'warehouse', schemaMode: 'external', external: { allowWrites: true, credentialsRef: REF } }, + ]); + }); + + it('retains the reference through the NAME-KEYED MAP spelling', () => { + const engine = new ObjectQL(); + engine.registerApp({ + id: 'wh_pkg_map', + name: 'Warehouse', + datasources: { warehouse: { schemaMode: 'external', external: { allowWrites: true, credentialsRef: REF } } }, + }); + + expect(engine.listDatasourceDefs()).toEqual([ + { name: 'warehouse', schemaMode: 'external', external: { allowWrites: true, credentialsRef: REF } }, + ]); + }); + }); + + describe('the accessor is unfiltered, which is the whole point of it', () => { + it('lists a MANAGED datasource that carries only a credentials reference (#8153)', () => { + // `credentialsRef` is valid in every schemaMode. A reader that filtered + // by schema mode would hide a live handle from a credentials sweep, and + // under-reporting is the direction that deletes live credentials. + const engine = new ObjectQL(); + engine.registerDatasourceDef({ name: 'billing', external: { credentialsRef: 'secret:billing/password' } }); + + expect(engine.listDatasourceDefs()).toEqual([ + { name: 'billing', external: { credentialsRef: 'secret:billing/password' } }, + ]); + }); + + it('lists definitions that carry no reference at all, rather than dropping them', () => { + const engine = new ObjectQL(); + engine.registerDatasourceDef({ name: 'plain', schemaMode: 'external', external: { allowWrites: false } }); + engine.registerDatasourceDef({ name: 'bare' }); + + const names = engine.listDatasourceDefs().map((d) => d.name).sort(); + expect(names).toEqual(['bare', 'plain']); + }); + + it('answers an empty list on an engine that was told about no datasources', () => { + // The control for every case above: the accessor reads a real index, and + // an empty answer here is what makes a non-empty one elsewhere a reading. + expect(new ObjectQL().listDatasourceDefs()).toEqual([]); + }); + }); + + describe('the accessor hands out a copy, never the write gate\'s own input', () => { + it('mutating the returned external block does not change what the engine holds', () => { + const engine = new ObjectQL(); + engine.registerDatasourceDef({ ...DEF, external: { ...DEF.external } }); + + const first = engine.listDatasourceDefs()[0]; + first.external!.credentialsRef = 'sys_secret:tampered'; + first.external!.allowWrites = false; + + expect(engine.listDatasourceDefs()[0].external).toEqual({ allowWrites: true, credentialsRef: REF }); + }); + }); + + describe('the write gate is unmoved by the widening', () => { + function makeGatedEngine(allowWrites: boolean, objWritable: boolean) { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver('default'), true); + engine.registerDriver(makeDriver('warehouse')); + // Carries a credentialsRef in every case — the widened key must be inert + // to Gate 3, which reads schemaMode + allowWrites and nothing else. + engine.registerDatasourceDef({ + name: 'warehouse', + schemaMode: 'external', + external: { allowWrites, credentialsRef: REF }, + }); + engine.registerApp({ + id: 'wh_gate_pkg', + name: 'Warehouse', + objects: [{ + name: 'wh_order', + datasource: 'warehouse', + external: { remoteName: 'fact_orders', writable: objWritable }, + fields: { order_id: { type: 'text' } }, + }], + }); + return engine; + } + + it('still refuses a write without the double opt-in, with the ADR-0112 envelope intact', async () => { + const engine = makeGatedEngine(false, true); + // The envelope, not merely "it threw": a driver throwing a bare Error + // would satisfy `toThrow()` and tell us nothing about the gate. + const err = await engine.insert('wh_order', { order_id: 'o1' }).then( + () => { throw new Error('insert resolved — the write gate did not fire'); }, + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(ExternalWriteForbiddenError); + expect(err).toMatchObject({ + code: (new ExternalWriteForbiddenError()).code, + status: (new ExternalWriteForbiddenError()).status, + }); + expect((err as Error).message).toContain("datasource 'warehouse' is external"); + }); + + it('still allows a write when both halves opt in, credentials reference present', async () => { + const engine = makeGatedEngine(true, true); + await expect(engine.insert('wh_order', { order_id: 'o1' })).resolves.toBeDefined(); + }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index fd288ff756..31f7c1735a 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -2180,6 +2180,39 @@ interface TransactionScope { readonly reportedOutOfScope: Set; } +/** + * A datasource *definition* as the engine keeps it (ADR-0015) — the declarative + * facts a datasource states about itself, not a live connection (that is + * {@link ObjectQL.registerDriver}). + * + * Named rather than restated inline because three sites share it: the private + * index, {@link ObjectQL.registerDatasourceDef} which writes it, and + * {@link ObjectQL.listDatasourceDefs} which reads it back. Three copies of one + * shape is a second de-facto contract that drifts silently, and the drift this + * one produces is a credentials handle missing from a `sys_secret` sweep. + * + * The keys are a deliberate SUBSET of the spec's authored datasource surface + * (`ExternalDatasourceSettingsSchema` in `@objectstack/spec`) — the engine + * carries only what it has a use for. ⛔ Not a mirror of the spec block and not + * a place to grow one: `validation` and `queryTimeoutMs` are absent because + * nothing in this engine reads them. + */ +export interface DatasourceDef { + name: string; + schemaMode?: string; + external?: { + /** Datasource-wide write gate — ADR-0015 §5.3 Gate 3. */ + allowWrites?: boolean; + /** + * Reference into the secrets store, never an inline credential. Valid in + * EVERY `schemaMode` — it is the one `external` key a managed datasource + * may carry (#8153) — so a reader sweeping for handles must not filter by + * schema mode. + */ + credentialsRef?: string; + }; +} + export class ObjectQL implements IObjectQLEngine { /** * Ambient transaction store (ADR-0034). While a `transaction()` callback @@ -2248,9 +2281,11 @@ export class ObjectQL implements IObjectQLEngine { // Datasource definitions by name (ADR-0015): carries schemaMode + // external.allowWrites so the write gate (Gate 3) can enforce federation - // ownership. Populated from manifests in registerApp and via - // registerDatasourceDef. Absent entry ⇒ treated as managed (default DB). - private datasourceDefs = new Map(); + // ownership, and external.credentialsRef so a sys_secret reference sweep can + // see the handle a code-declared datasource holds. Populated from manifests + // in registerApp and via registerDatasourceDef. Absent entry ⇒ treated as + // managed (default DB). + private datasourceDefs = new Map>(); // Declared-but-unusable datasources, keyed by name (framework#3828). Written // by the datasource connection layer via markDatasourceUnavailable; read only @@ -5134,15 +5169,47 @@ export class ObjectQL implements IObjectQLEngine { * Register a Datasource *definition* (ADR-0015). * * Distinct from {@link registerDriver}, which registers a live connection. - * This captures the declarative `schemaMode` + `external.allowWrites` so the - * write gate ({@link assertWriteAllowed}) can enforce external-datasource - * ownership. Safe to call repeatedly; last write wins. + * This captures the declarative {@link DatasourceDef}: `schemaMode` + + * `external.allowWrites` so the write gate ({@link assertWriteAllowed}) can + * enforce external-datasource ownership, and `external.credentialsRef` so + * {@link listDatasourceDefs} can hand a credentials sweep the handle a + * code-declared datasource holds. Safe to call repeatedly; last write wins. */ - registerDatasourceDef(def: { name: string; schemaMode?: string; external?: { allowWrites?: boolean } }): void { + registerDatasourceDef(def: DatasourceDef): void { if (!def?.name) return; this.datasourceDefs.set(def.name, { schemaMode: def.schemaMode, external: def.external }); } + /** + * Every datasource DEFINITION this engine holds — from BOTH entry routes, + * {@link registerDatasourceDef} and the package-manifest install path in + * {@link registerApp}. + * + * Exists because a datasource declared IN CODE never reaches the metadata + * store, so a `sys_secret` reference sweep reading `sys_metadata` alone + * cannot see the handle such a datasource holds at `external.credentialsRef` + * and must be handed the list by its caller instead. A completeness + * guarantee that depends on every caller remembering to pass a list is + * weaker than one the engine can answer. + * + * ⛔ Deliberately UNFILTERED: every definition, whatever its `schemaMode` and + * whether or not it carries a reference. `credentialsRef` is valid on a + * managed datasource too (#8153), so filtering by schema mode here would hide + * live handles from the sweep — and under-reporting is the direction that + * deletes live credentials. + * + * Each entry is a fresh object with a COPIED `external` block. The index + * stores the caller's `external` by reference, and a reader must not be able + * to reach through this accessor and mutate the write gate's own input. + */ + listDatasourceDefs(): DatasourceDef[] { + return Array.from(this.datasourceDefs, ([name, def]) => ({ + name, + ...(def.schemaMode !== undefined ? { schemaMode: def.schemaMode } : {}), + ...(def.external ? { external: { ...def.external } } : {}), + })); + } + /** * Record that a **declared** datasource has no live driver, and why * (framework#3828). Called by `DatasourceConnectionService` when a connect is diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 075b82b8b8..510f958c88 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -90,6 +90,10 @@ export type { CompanionFieldMeta, CompanionObjectMeta } from './search-companion export { ObjectQL, ObjectRepository, ScopedContext } from './engine.js'; export type { HookHandler, HookEntry, OperationContext, EngineMiddleware, HeldFileResolver } from './engine.js'; export type { AdmittedValueShapeViolationTally } from './engine.js'; +// The declarative datasource definition the engine indexes, and the element +// type of `ObjectQL.listDatasourceDefs()`. Exported so a consumer sweeping for +// `sys_secret` references can name the shape it reads instead of re-declaring it. +export type { DatasourceDef } from './engine.js'; export { SummaryRecomputeError } from './summary-errors.js'; export type { SummaryRecomputeFailure } from './summary-errors.js'; // [#5126] Thrown by `update` when `options.strictReadonlyWrites` is set and the