Skip to content

Commit 49da36e

Browse files
xuyushun441-sysos-zhuangclaude
authored
feat(datasource): fail-closed credential resolution at connect (ADR-0062 Phase 2, D3) (#2199)
* feat(datasource): fail-closed credential resolution at connect (ADR-0062 Phase 2, D3) A declared external.credentialsRef MUST resolve to a cleartext secret before the driver is built — an absent secret store or an unresolvable/undecryptable ref now fails closed (clear message, datasource left unconnected) instead of silently building a driver without the credential. Follows the same fail-fast (declared external + onMismatch:fail) vs degrade policy as connect failures. Converges with the runtime-admin secret path (same SecretBinder threaded through the shared connection service). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(analytics): analytics over federated objects honors the remote table (ADR-0062 Phase 3, D6) (#2200) * feat(datasource): reject field.columnName on external + drop showcase onEnable bridge (ADR-0062 Phase 4, D7/D8) (#2203) --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ac79f16 commit 49da36e

20 files changed

Lines changed: 533 additions & 52 deletions
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-analytics": minor
4+
---
5+
6+
feat(analytics): correct analytics over federated objects (ADR-0062 Phase 3, D6)
7+
8+
Analytics over an external (federated) object now aggregates against the
9+
**correct** remote table instead of silently querying the wrong one. The
10+
`NativeSQLStrategy` hand-compiles `FROM "<object>"` and bare column references,
11+
which bypass the driver's physical-table resolution (`external.remoteName` /
12+
`remoteSchema` / `columnMap`). It now **declines** any query whose base or joined
13+
object is federated, routing it to the `ObjectQLStrategy` — whose
14+
`engine.aggregate()` goes through the driver's `getBuilder` and already honours
15+
`remoteName`/`remoteSchema` (#2138/#2149). This "reuses the driver's resolution"
16+
(D6) rather than re-implementing it.
17+
18+
Adds an optional `StrategyContext.isExternalObject(objectName)` hook (reported by
19+
the analytics plugin from the object's `external` block). Purely additive — with
20+
no hook, behavior is unchanged for managed objects.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/service-datasource": minor
3+
---
4+
5+
feat(datasource): fail-closed credential resolution at connect (ADR-0062 Phase 2, D3)
6+
7+
`DatasourceConnectionService` now treats a declared `external.credentialsRef` as
8+
**fail-closed**: the credential must resolve to a cleartext secret (via the
9+
host's `SecretBinder` over `ICryptoProvider`) *before* the driver is built. An
10+
absent secret store, or a ref that cannot be resolved/decrypted (missing
11+
`sys_secret` row, rotated key, or a throwing resolver), leaves the datasource
12+
**unconnected with a clear message** — never a silent build-without-secret that
13+
would connect with no/wrong auth or fail later with a confusing driver error.
14+
15+
The same policy as connect failures applies: a code-defined `external` datasource
16+
with `validation.onMismatch: 'fail'` auto-connected at boot fails fast (bricks
17+
boot); runtime-admin create/update + boot rehydration degrade-with-warning. Code-
18+
and runtime-origin secrets converge on the one connection path (the same
19+
`SecretBinder` is threaded through the shared service). New `failed-credentials`
20+
connect status.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@objectstack/cli": minor
3+
"@objectstack/verify": minor
4+
---
5+
6+
feat(datasource): reject field.columnName on external objects + drop showcase onEnable bridge (ADR-0062 Phase 4, D7/D8)
7+
8+
**D7 — reconcile column mapping.** `os compile`/`build` (`validateStackExpressions`)
9+
now rejects `field.columnName` on a federated (external) object with a corrective
10+
message: the driver's query pipeline ignores `field.columnName` for external
11+
objects, so `external.columnMap` is the single authoritative mechanism. Managed
12+
objects are untouched.
13+
14+
**D8 — drop the canonical example's driver bridge.** `examples/app-showcase`
15+
declares its external datasource with **no** `onEnable` driver registration — the
16+
declared datasource auto-connects at boot (ADR-0062 D1). `onEnable` now only
17+
provisions the "remote" fixture tables. To cover this end-to-end, the
18+
`@objectstack/verify` harness wires the datasource-admin plugin (registering the
19+
`'datasource-connection'` service) when an app declares datasources, so it mirrors
20+
`objectstack dev`/serve; a new dogfood test reads the federated objects through the
21+
real REST stack (incl. the `remoteName` remap). `onEnable` + `ctx.drivers.register`
22+
remains supported as an escape hatch for drivers built dynamically at runtime.

docs/adr/0062-external-datasource-runtime.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,14 @@ Code-defined datasources surface in `GET /api/v1/datasources`, `GET /api/v1/meta
7777

7878
The analytics native-SQL strategy compiles its own `FROM "<table>"` / column references outside the driver (ADR-0015 §18 noted this). It must resolve an external object's physical table (`remoteName`/`remoteSchema`) and columns (`columnMap`) the same way `SqlDriver` now does — reusing the driver's resolution (e.g. an exposed `physicalTableFor(object)` / `physicalColumnFor(object, field)`), not a second copy. Until then, analytics over external objects stays disabled rather than silently querying the wrong table.
7979

80+
> **Phase 3 implementation note (#2163) — "reuse the driver's resolution" = route external objects to the ObjectQL path.** Rather than re-implement `remoteName`/`remoteSchema`/`columnMap` resolution inside the native-SQL strategy (a second copy — explicitly rejected), `NativeSQLStrategy.canHandle` now **declines** any query whose base or joined object is federated (new optional `StrategyContext.isExternalObject` hook, reported by the analytics plugin from the object's `external` block). Declining routes the query to the lower-priority `ObjectQLStrategy`, whose `engine.aggregate()` already goes through the driver's `getBuilder` — which honours `remoteName`/`remoteSchema` (#2138/#2149). So external analytics aggregates against the **correct** remote table via the single source of truth (the driver), and native-SQL never queries the wrong table. (A native-SQL fast path for external objects can be added later by exposing `physicalTableFor`/`physicalColumnFor` on the driver; deeper `columnMap`-in-`GROUP BY` support is a separate driver concern.)
81+
8082
### D7 — `columnMap` is the external mechanism; reconcile `field.columnName`
8183

8284
`external.columnMap` ({ remoteColumn → localField }) is the supported way to map external columns (shipped #2149). `field.columnName` (localField → physicalColumn) is its inverse and is **not** applied by the driver's query pipeline for external objects. Decision: for external objects, `columnMap` is authoritative; `field.columnName` on an external object is rejected at validation (no silent dual-source) until a unified column-resolution model is designed. Managed objects' `field.columnName` semantics are untouched.
8385

86+
> **Phase 4 implementation note (#2163).** *D7* is enforced at build time in `validateStackExpressions` (`os compile`/`build`): any object that declares an `external` binding and a field with `columnName` is an error with a corrective message (use `external.columnMap`). Managed objects are untouched. *D8*: `examples/app-showcase` now declares its external datasource with **no** `onEnable` driver registration — `onEnable` only provisions the "remote" fixture tables; the declared datasource auto-connects (D1). To exercise this in the dogfood gate, the `@objectstack/verify` harness now wires the datasource-admin plugin (hence the `'datasource-connection'` service) when an app declares datasources — so the harness matches `objectstack dev`/serve and the federated read is covered end-to-end (a new dogfood test reads `showcase_ext_customer`/`_order`, including the `remoteName` remap). `onEnable` + `ctx.drivers.register` remains documented as the escape hatch for drivers built dynamically at runtime.
87+
8488
### D8 — Drop the `onEnable` bridge from the canonical example; keep it as an escape hatch
8589

8690
Once D1 lands, `examples/app-showcase` declares its external datasource with **no** `onEnable` driver-registration code (fixture provisioning may remain). `onEnable` + `ctx.drivers.register` stays supported for advanced/dynamic cases (drivers built at runtime from external config).

examples/app-showcase/objectstack.config.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,13 @@ export default defineStack({
129129
// AI-authored metadata survive restarts (a `:memory:` datasource would wipe
130130
// everything on every restart, which makes local app-building unusable).
131131
//
132-
// External-datasource federation demo (ADR-0015): a second, read-only SQLite
133-
// file declared as a code-defined external datasource. It appears in
134-
// Setup → Datasources and its federated objects (below) are queryable via
135-
// REST. The fixture file + live driver are wired by `onEnable` (bottom of
136-
// this file). `os dev` needs no extra setup.
132+
// External-datasource federation demo (ADR-0015 / ADR-0062): a second,
133+
// read-only SQLite file declared as a code-defined external datasource. It
134+
// appears in Setup → Datasources and its federated objects (below) are
135+
// queryable via REST — with NO driver wiring: the declared `external`
136+
// datasource AUTO-CONNECTS at boot (ADR-0062 D1/D8). `onEnable` (bottom of
137+
// this file) only provisions the "remote" fixture file's tables + seed data;
138+
// `os dev` needs no extra setup.
137139
datasources: [ShowcaseExternalDatasource],
138140

139141
// i18n
@@ -185,11 +187,14 @@ export default defineStack({
185187
});
186188

187189
/**
188-
* Runtime wiring for the external-datasource federation demo (ADR-0015).
190+
* Provisions the "remote" fixture database for the external-datasource
191+
* federation demo (ADR-0015 / ADR-0062). Creating + seeding the remote tables is
192+
* CODE (DDL on a separate SQLite file), so it can't live in the declarative
193+
* artifact — it runs here. The AppPlugin invokes `onEnable` at boot.
189194
*
190-
* Code (fixture provisioning + live external-driver registration) can't live in
191-
* the declarative artifact, so it runs here. The AppPlugin invokes `onEnable` at
192-
* boot (Phase 2), before the external-validation gate fires on `kernel:ready`.
195+
* NOTE (ADR-0062 D8): this no longer registers the external driver. The declared
196+
* `external` datasource auto-connects at boot, so the federated objects are
197+
* queryable with no driver wiring.
193198
*/
194199
export const onEnable = async (ctx: unknown): Promise<void> => {
195200
await setupShowcaseExternalDatasource(ctx as Parameters<typeof setupShowcaseExternalDatasource>[0]);

examples/app-showcase/src/datasources/external-fixture.ts

Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,22 @@
33
import { SqlDriver } from '@objectstack/driver-sql';
44

55
/**
6-
* Runtime wiring for the external-datasource federation demo (ADR-0015).
6+
* Fixture provisioning for the external-datasource federation demo
7+
* (ADR-0015 / ADR-0062).
78
*
8-
* This is CODE, so it can't live in the declarative artifact — it runs from the
9-
* stack's `onEnable` hook, which the AppPlugin invokes at boot (Phase 2), before
10-
* the external-validation gate fires on `kernel:ready` (Phase 3).
9+
* This is the "remote database" stand-in: it idempotently creates the separate
10+
* SQLite file with `customers` / `orders` tables + a little seed data (a MANAGED
11+
* driver — DDL allowed) so `os dev` needs no external server. It runs from the
12+
* stack's `onEnable` hook at boot.
1113
*
12-
* It does three things, all zero-config so `os dev` "just works":
13-
* 1. Idempotently provisions the "remote" SQLite file with `customers` /
14-
* `orders` tables + a little seed data (a MANAGED driver — DDL allowed).
15-
* 2. Registers a read-only `schemaMode: 'external'` driver for that same file
16-
* under the datasource name `showcase_external`, so ObjectQL can route
17-
* queries to it. (Declared datasources are surfaced in the metadata
18-
* registry for Setup → Datasources, but a live driver must be registered
19-
* for the federated objects to be queryable in standalone mode.)
20-
* 3. Registers the federated objects' read metadata (DDL-free) so coercion +
21-
* the remote-table mapping exist immediately.
14+
* **It no longer registers a driver or syncs object schemas (ADR-0062 D8).** The
15+
* declared `external` datasource (see `showcase-external.datasource.ts`) now
16+
* AUTO-CONNECTS: at boot the runtime's `DatasourceConnectionService` builds the
17+
* read-only `schemaMode:'external'` driver, registers it under the datasource
18+
* name, and registers the federated objects' read metadata — with zero app code.
19+
* The old `onEnable` + `ctx.drivers.register` bridge is gone; `onEnable` +
20+
* `ctx.drivers.register` remains *supported* only as an advanced escape hatch
21+
* (drivers built dynamically at runtime).
2222
*/
2323

2424
// Same relative path as the datasource config — resolved against the project
@@ -60,15 +60,17 @@ const ORDER_ROWS = [
6060
{ id: 'o4', customer_id: 'c3', amount: 3300, status: 'paid', placed_on: '2026-03-01' },
6161
];
6262

63-
/** Stack `onEnable` payload (subset we use). */
63+
/** Stack `onEnable` payload (subset we use — provisioning only logs). */
6464
interface OnEnableContext {
65-
ql: { syncObjectSchema?: (name: string) => Promise<void> };
66-
drivers: { register: (driver: unknown) => void };
6765
logger?: { info?: (msg: string, meta?: unknown) => void; warn?: (msg: string, meta?: unknown) => void };
6866
}
6967

68+
/**
69+
* Provision the "remote" fixture database. Idempotent: creates the tables and
70+
* seeds them only when empty. Does NOT register a driver — the declared external
71+
* datasource auto-connects at boot (ADR-0062 D1/D8).
72+
*/
7073
export async function setupShowcaseExternalDatasource(ctx: OnEnableContext): Promise<void> {
71-
// 1. Provision the remote fixture with a MANAGED driver (DDL allowed). Idempotent.
7274
const fixture = new SqlDriver({
7375
client: 'better-sqlite3',
7476
connection: { filename: EXTERNAL_DB_FILE },
@@ -93,21 +95,7 @@ export async function setupShowcaseExternalDatasource(ctx: OnEnableContext): Pro
9395
await fixture.disconnect();
9496
}
9597

96-
// 2. Register the read-only EXTERNAL driver under the datasource name.
97-
const ext = new SqlDriver({
98-
client: 'better-sqlite3',
99-
connection: { filename: EXTERNAL_DB_FILE },
100-
useNullAsDefault: true,
101-
schemaMode: 'external',
102-
} as never) as unknown as { name: string; connect: () => Promise<void> };
103-
ext.name = 'showcase_external'; // MUST equal the datasource name (ObjectQL routes by driver name).
104-
await ext.connect();
105-
ctx.drivers.register(ext);
106-
107-
// 3. Register read metadata for the federated objects (DDL-free; ADR-0015).
108-
// Needed because the driver is registered after the boot schema-sync.
109-
await ctx.ql.syncObjectSchema?.('showcase_ext_customer');
110-
await ctx.ql.syncObjectSchema?.('showcase_ext_order');
111-
112-
ctx.logger?.info?.('[showcase] external datasource "showcase_external" ready — federation demo (ADR-0015)');
98+
ctx.logger?.info?.(
99+
'[showcase] external fixture provisioned — datasource "showcase_external" auto-connects (ADR-0062)',
100+
);
113101
}

packages/cli/src/utils/validate-expressions.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,4 +343,55 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => {
343343
expect(issues.some(i => i.where.includes('when') && /bare reference `tier`/.test(i.message))).toBe(true);
344344
});
345345
});
346+
347+
describe('ADR-0062 D7 — field.columnName on external objects', () => {
348+
it('rejects field.columnName on a federated (external) object', () => {
349+
const issues = validateStackExpressions({
350+
objects: [{
351+
name: 'ext_customer',
352+
datasource: 'warehouse',
353+
external: { remoteName: 'customers' },
354+
fields: { region: { type: 'text', columnName: 'cust_region' } },
355+
}],
356+
});
357+
const issue = issues.find(i => /columnName/.test(i.message));
358+
expect(issue).toBeDefined();
359+
expect(issue!.severity).toBe('error');
360+
expect(issue!.where).toContain("object 'ext_customer'");
361+
expect(issue!.where).toContain("field 'region'");
362+
expect(issue!.message).toMatch(/external\.columnMap|ADR-0062 D7/);
363+
});
364+
365+
it('rejects field.columnName declared as a map-shaped field too', () => {
366+
const issues = validateStackExpressions({
367+
objects: [{
368+
name: 'ext_order',
369+
external: {},
370+
fields: { amount: { type: 'number', columnName: 'amt' } },
371+
}],
372+
});
373+
expect(issues.some(i => /columnName/.test(i.message) && i.where.includes("field 'amount'"))).toBe(true);
374+
});
375+
376+
it('leaves field.columnName on a MANAGED object untouched (no issue)', () => {
377+
const issues = validateStackExpressions({
378+
objects: [{
379+
name: 'crm_account',
380+
fields: { region: { type: 'text', columnName: 'acct_region' } },
381+
}],
382+
});
383+
expect(issues.some(i => /columnName/.test(i.message))).toBe(false);
384+
});
385+
386+
it('passes an external object that uses no columnName', () => {
387+
const issues = validateStackExpressions({
388+
objects: [{
389+
name: 'ext_customer',
390+
external: { remoteName: 'customers' },
391+
fields: { region: { type: 'text' } },
392+
}],
393+
});
394+
expect(issues.some(i => /columnName/.test(i.message))).toBe(false);
395+
});
396+
});
346397
});

packages/cli/src/utils/validate-expressions.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,35 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
150150
const fieldList = Array.isArray(fields)
151151
? (fields as AnyRec[])
152152
: (fields && typeof fields === 'object' ? Object.values(fields as AnyRec) as AnyRec[] : []);
153+
154+
// ── ADR-0062 D7 — reject `field.columnName` on external objects ──────
155+
// `field.columnName` (localField → physicalColumn) is the managed-object
156+
// mechanism; it is NOT applied by the driver's query pipeline for federated
157+
// objects, where `external.columnMap` (remoteColumn → localField) is the
158+
// authoritative — and inverse — mapping. Allowing both would be a silent
159+
// dual-source ambiguity, so reject `columnName` on any object that declares
160+
// an `external` binding (a federated object). Managed objects are untouched.
161+
if (obj.external != null) {
162+
const fieldEntries: Array<[string, AnyRec]> = Array.isArray(fields)
163+
? (fields as AnyRec[]).map((f) => [((f as AnyRec)?.name as string) ?? '?', f as AnyRec])
164+
: (fields && typeof fields === 'object'
165+
? (Object.entries(fields as AnyRec) as Array<[string, AnyRec]>)
166+
: []);
167+
for (const [fname, fdef] of fieldEntries) {
168+
if (fdef && typeof fdef === 'object' && (fdef as AnyRec).columnName != null) {
169+
issues.push({
170+
where: `object '${objectName}' · field '${fname}'`,
171+
message:
172+
`external object '${objectName}': field '${fname}' sets columnName='${String((fdef as AnyRec).columnName)}', ` +
173+
`which is not supported on federated objects (ADR-0062 D7). The driver's query pipeline ignores ` +
174+
`field.columnName for external objects; map remote columns via the datasource's external.columnMap instead.`,
175+
source: `columnName='${String((fdef as AnyRec).columnName)}'`,
176+
severity: 'error',
177+
});
178+
}
179+
}
180+
}
181+
153182
for (const f of fieldList) {
154183
// Field-level conditional rules are server-enforced (rule-validator) and
155184
// record-scoped — a bare ref silently fails the rule (required/readonly

0 commit comments

Comments
 (0)