From ff5a53fb7c638a76caa0898adc4ada72184eb1d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 04:48:50 +0000 Subject: [PATCH 1/2] fix(service-datasource): refuse an importObject name override that violates the namespace prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The opts.name override was taken verbatim and persisted through metadata.register — the one runtime write path no namespace gate looks at. It now answers to the same ADR-0028 rule the derived name already obeys: loud refusal (the publish pre-flight's treatment of the identical violation) with validateObjectNamespacePrefix's own actionable message, thrown in the #8016 declaration shape (status 400, code EXTERNAL_IMPORT_ERROR). No namespace resolvable = rule skipped, mirroring defineStack. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .../external-datasource-service.test.ts | 104 ++++++++++++++++++ .../src/external-datasource-service.ts | 49 +++++++++ 2 files changed, 153 insertions(+) diff --git a/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts b/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts index 6d45521db1..3a17d834b2 100644 --- a/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts @@ -2,6 +2,10 @@ import { describe, it, expect } from 'vitest'; import type { IntrospectedSchema } from '@objectstack/spec/contracts'; +// The prefix rule's ONE authored message source — refusal pins below compute +// their expected text from it rather than copying strings, so the assertion +// can never drift from the rule (#11061). +import { validateObjectNamespacePrefix } from '@objectstack/spec/kernel'; import { ExternalDatasourceService, type DatasourceLike, @@ -219,6 +223,106 @@ describe('importObject', () => { }); }); +/** + * ADR-0028 namespace prefix on the `opts.name` OVERRIDE path (#11061). + * + * The derived path resolves the datasource's package namespace and prefixes + * the generated name; `opts.name` used to be taken verbatim, so a caller could + * persist an unprefixed federated object through `metadata.register` — the one + * runtime write path with no namespace gate. The override now answers to the + * SAME rule (loud refusal — the publish pre-flight's treatment of the + * identical violation — never a silent rewrite). + * + * Refusal pins assert the ADR-0112 envelope declaration (`status` + `code`, + * the #8016 thrown-refusal shape) AND that nothing was persisted or + * introspected; the message is asserted byte-equal to + * `validateObjectNamespacePrefix`'s own return, computed — not copied — so the + * rule keeps exactly one authored message everywhere it fires. + */ +describe('importObject — namespace prefix on the name override (#11061)', () => { + function makeNamespacedImporter(namespace?: string) { + const persisted: Array<{ name: string; def: Record }> = []; + let introspectCalls = 0; + const svc = new ExternalDatasourceService({ + introspect: async () => { + introspectCalls += 1; + return warehouseSchema(); + }, + getDatasource: async () => ({ name: 'warehouse', schemaMode: 'external' }), + getObject: async () => undefined, + listObjects: async () => [], + persistObject: async (name, def) => { persisted.push({ name, def }); }, + getNamespace: async () => namespace, + }); + return { svc, persisted, introspectCalls: () => introspectCalls }; + } + + it('refuses an unprefixed override with the rule\'s own message — 400 EXTERNAL_IMPORT_ERROR, nothing persisted, nothing introspected', async () => { + const { svc, persisted, introspectCalls } = makeNamespacedImporter('wh'); + const err = await svc.importObject('warehouse', 'fact_orders', { name: 'orders' }).catch((e) => e); + + expect(err).toBeInstanceOf(Error); + expect((err as { status?: number }).status).toBe(400); + expect((err as { code?: string }).code).toBe('EXTERNAL_IMPORT_ERROR'); + // Byte-equal to the validator's own authored prescription — one rule, one + // message, at this gate and the publish gate alike. + expect((err as Error).message).toBe(validateObjectNamespacePrefix('orders', 'wh')); + expect((err as Error).message).toContain("Rename it to 'wh_orders'"); + // The service was never persuaded: no persist, and no remote round trip — + // the verdict needs only the injected namespace read. + expect(persisted).toHaveLength(0); + expect(introspectCalls()).toBe(0); + }); + + it('refuses the legacy FQN form (ns__short) with the validator\'s legacy-form message', async () => { + const { svc, persisted } = makeNamespacedImporter('wh'); + const err = await svc.importObject('warehouse', 'fact_orders', { name: 'wh__orders' }).catch((e) => e); + + expect(err).toBeInstanceOf(Error); + expect((err as { status?: number }).status).toBe(400); + expect((err as { code?: string }).code).toBe('EXTERNAL_IMPORT_ERROR'); + expect((err as Error).message).toBe(validateObjectNamespacePrefix('wh__orders', 'wh')); + expect(persisted).toHaveLength(0); + }); + + it('accepts a compliant override verbatim — no double prefixing, persisted as asked', async () => { + const { svc, persisted } = makeNamespacedImporter('wh'); + const result = await svc.importObject('warehouse', 'fact_orders', { name: 'wh_orders' }); + expect(result.name).toBe('wh_orders'); + expect(persisted).toHaveLength(1); + expect(persisted[0].name).toBe('wh_orders'); + }); + + it('accepts a sys_* override — the platform-reserved carve-out every gate on this rule shares', async () => { + const { svc, persisted } = makeNamespacedImporter('wh'); + const result = await svc.importObject('warehouse', 'fact_orders', { name: 'sys_orders' }); + expect(result.name).toBe('sys_orders'); + expect(persisted).toHaveLength(1); + }); + + it('skips the rule when no namespace resolves — an unprefixed override stays accepted (mirrors defineStack)', async () => { + const { svc, persisted } = makeNamespacedImporter(undefined); + const result = await svc.importObject('warehouse', 'fact_orders', { name: 'orders' }); + expect(result.name).toBe('orders'); + expect(persisted).toHaveLength(1); + }); + + it('treats a blank namespace as absent — same normalisation as the derived path', async () => { + const { svc, persisted } = makeNamespacedImporter(' '); + const result = await svc.importObject('warehouse', 'fact_orders', { name: 'orders' }); + expect(result.name).toBe('orders'); + expect(persisted).toHaveLength(1); + }); + + it('still prefixes the DERIVED name under a namespace when no override is passed', async () => { + const { svc, persisted } = makeNamespacedImporter('wh'); + const result = await svc.importObject('warehouse', 'fact_orders'); + expect(result.name).toBe('wh_fact_orders'); + expect(persisted).toHaveLength(1); + expect(persisted[0].name).toBe('wh_fact_orders'); + }); +}); + describe('validateObject', () => { const baseObject: ObjectLike = { name: 'wh_order', diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index 544afe67b0..a88f4e6402 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -298,6 +298,37 @@ function applyNamespacePrefix(objectName: string, namespace: string | undefined) : `${namespace}_${objectName}`; } +/** + * The refusal `importObject` answers when an explicit `opts.name` violates the + * ADR-0028 namespace-prefix rule. + * + * REFUSED, never silently rewritten: the derived path above may prefix its own + * invention (`applyNamespacePrefix` adjusts a name the service itself derived), + * but an override is the caller's explicit input, and every other gate on this + * rule — `defineStack()` at compile time, the publish pre-flight at runtime + * (`NAMESPACE_PREFIX`) — refuses a violating name rather than editing it. + * Persisting a rewritten name behind a 201 would leave the caller holding a + * name that does not exist, which is worse than the 400 (and exactly the + * tolerant-consumer accommodation Prime Directive #12 forbids). + * + * `message` is `validateObjectNamespacePrefix`'s own authored text, verbatim — + * the same prescription the publish gate serves for the identical violation, + * so one rule keeps one message everywhere it fires. + * + * The throw carries its own `status`/`code` — the #8016 declaration shape + * (`resolveThrownHttpError` reads them): this is a *refusal*, not a fault. + * `EXTERNAL_IMPORT_ERROR` is the ledger's registered code for a refused + * federated import, and 400 + that code is also exactly what the REST import + * route answers for any `importObject` throw, so the declaration and the + * served envelope agree by construction. + */ +function importNameRefusedError(message: string): Error { + const err = new Error(message) as Error & { code?: string; status?: number }; + err.code = 'EXTERNAL_IMPORT_ERROR'; + err.status = 400; + return err; +} + /** snake_case → Title Case label. */ function toLabel(name: string): string { return name @@ -471,6 +502,24 @@ export class ExternalDatasourceService implements IExternalDatasourceService { ); } + // ADR-0028 invariant on the OVERRIDE path. The derived path below resolves + // the datasource's package namespace and prefixes the name it generates; + // `opts.name` used to be taken verbatim, so a caller could persist an + // unprefixed federated object through the one runtime write path no gate + // looks at (`metadata.register` applies no namespace check — the checks + // live at `defineStack()` and the publish pre-flight, and an imported + // object passes through neither). Same normalisation, same validator, so + // the override answers to exactly the rule the derived name already obeys; + // when no namespace resolves the rule is skipped, mirroring `defineStack`. + // Checked BEFORE the draft on purpose: the verdict needs only the injected + // namespace read, and a doomed request should not cost a live remote + // introspection round trip. + if (opts.name !== undefined) { + const namespace = normaliseNamespace(await this.config.getNamespace?.(datasource)); + const violation = validateObjectNamespacePrefix(opts.name, namespace); + if (violation) throw importNameRefusedError(violation); + } + // Reuse the draft pipeline (type mapping, review notes, external binding). const draft = await this.generateObjectDraft(datasource, remoteName, opts); From 4a86656751cf7c9942210d50daa33a0162edb358 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 04:52:10 +0000 Subject: [PATCH 2/2] changeset for the importObject namespace-prefix refusal Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- ...t-object-name-override-namespace-prefix.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/import-object-name-override-namespace-prefix.md diff --git a/.changeset/import-object-name-override-namespace-prefix.md b/.changeset/import-object-name-override-namespace-prefix.md new file mode 100644 index 0000000000..221d8d8320 --- /dev/null +++ b/.changeset/import-object-name-override-namespace-prefix.md @@ -0,0 +1,21 @@ +--- +"@objectstack/service-datasource": patch +--- + +`importObject` now refuses an explicit `opts.name` that violates the ADR-0028 +namespace-prefix rule (#11061). The override used to be taken verbatim +(`opts.name ?? draft.name`) and persisted through `metadata.register('object', +…)` — the one runtime write path no namespace gate looks at — so +`POST /api/v1/datasources/:name/external/tables/:remote/import` with +`{"name": "customers"}` minted an unprefixed federated object that +`defineStack()` and the publish pre-flight would both have refused. + +The refusal answers `400 EXTERNAL_IMPORT_ERROR` (the family's registered +ADR-0112 code, in the #8016 thrown-refusal shape) carrying +`validateObjectNamespacePrefix`'s own actionable message — the same text the +publish gate serves for the identical violation, e.g. `Object 'customers' is +missing the package namespace prefix. Rename it to 'wh_customers' (namespace = +'wh').` A compliant override (`wh_customers`), a `sys_*` platform-reserved +name, and any override on a datasource whose package resolves no namespace are +accepted exactly as before; the derived-name path (no `name` in the body) is +unchanged.