diff --git a/.changeset/29164-reserve-route-shadowed-datasource-names.md b/.changeset/29164-reserve-route-shadowed-datasource-names.md new file mode 100644 index 00000000000..d30d762d3d1 --- /dev/null +++ b/.changeset/29164-reserve-route-shadowed-datasource-names.md @@ -0,0 +1,18 @@ +--- +'@objectstack/service-datasource': patch +--- + +fix(service-datasource)!: creating a runtime datasource refuses `drivers`, the name the driver-catalog route answers, so a datasource is never created that its edit form cannot load (#29164) + +Clause-②: no (narrowing) — `POST /api/v1/datasources` with `name: 'drivers'`, which answered 201 before, now answers 400 `DATASOURCE_ADMIN_ERROR` naming the reserved name. Nothing is widened. +Metadata-protocol: none + + + +**BREAKING** accept-set narrowing at the datasource-admin create door (`@objectstack/service-datasource`). + +- **Before.** `GET /api/v1/datasources/drivers` is the driver catalog, mounted ahead of `GET /api/v1/datasources/:name`. A runtime datasource named `drivers` was created and listed, but its detail read always answered the catalog, so Setup → Datasources could never open its edit form. +- **Now.** Create refuses every literal path segment the admin routes mount beside a same-method `/datasources/:name` route; today that is `drivers` alone. The refusal is the same 400 `DATASOURCE_ADMIN_ERROR` that `default` already answers. `test` stays creatable: `POST /datasources/test` (the unsaved-draft probe) sits beside no `POST /datasources/:name`, so a datasource named `test` keeps every by-name door. +- **Stored rows.** A `drivers` row created before this release is not refused or rewritten. It stays listed, patchable, removable and probe-able by name; its detail read still answers the catalog. + +**The remedy.** Create the datasource under another name. For a stored `drivers` row, create the same connection under another name, move the objects bound to `drivers` onto it, then remove `drivers` (`DELETE /api/v1/datasources/drivers`, which refuses while any object is still bound). diff --git a/content/docs/data-modeling/drivers.mdx b/content/docs/data-modeling/drivers.mdx index 9ac7d209c8c..fc2b227908e 100644 --- a/content/docs/data-modeling/drivers.mdx +++ b/content/docs/data-modeling/drivers.mdx @@ -278,6 +278,9 @@ through the same datasource connection path — one failure verdict, one escape hatch, and a real `status` for the primary DB in **Setup → Datasources**. The name `default` is host-reserved: an app bundle declaring a datasource with that name is rejected at load. +The name `drivers` is reserved at the datasource admin API: creating a runtime +datasource under it is refused, because `GET /api/v1/datasources/drivers` is the +driver catalog, so a datasource of that name could never be opened by name. `OS_ALLOW_DRIVER_CONNECT_FAILURE=1` boots anyway, in an explicitly degraded diff --git a/packages/services/service-datasource/src/__tests__/datasource-route-reserved-names.test.ts b/packages/services/service-datasource/src/__tests__/datasource-route-reserved-names.test.ts new file mode 100644 index 00000000000..9d8db2c97d3 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/datasource-route-reserved-names.test.ts @@ -0,0 +1,207 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Route-shadowed datasource names (#29164). + * + * `GET /datasources/drivers` is the driver catalog, mounted before + * `GET /datasources/:name`, so a runtime datasource named `drivers` was + * created (201) and listed while its detail read — the edit form's load — + * always answered the catalog. Create now refuses every literal the registrar + * mounts beside a same-method `/:name` route, the way it refuses the + * host-owned `default`. + * + * Two halves: + * - the DRIFT PIN derives the shadowing literals from what + * `registerDatasourceAdminRoutes` actually mounts (a capturing mock + * `IHttpServer`, the route-ledger conformance seam) and holds the reserved + * list equal to them, so a new literal sibling of `/:name` mounted without + * a reservation fails here; + * - the DOOR cases run the real `HonoHttpServer` over a real + * `DatasourceAdminService`: the refusal envelope, the service never + * writing, `default` unchanged, and the names that stay creatable. + */ + +import { describe, it, expect, vi } from 'vitest'; +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 { ROUTE_SHADOWED_DATASOURCE_NAMES } from '../datasource-route-reserved-names.js'; +import { + ENTITLED_CREDENTIAL, + createSessionAuthService, + createGrantsEngine, +} from './entitled-caller.fixture.js'; + +const BASE = '/api/v1'; +const ROOT = `${BASE}/datasources`; +const VERBS = ['get', 'post', 'put', 'patch', 'delete'] as const; + +/** Every `[VERB, path]` the registrar mounts at the default base. */ +function enumerateMounts(): Array<[string, string]> { + const server = Object.fromEntries([ + ...VERBS.map((v) => [v, vi.fn()]), + ['use', vi.fn()], + ['listen', vi.fn().mockResolvedValue(undefined)], + ['close', vi.fn().mockResolvedValue(undefined)], + ]) as Record<(typeof VERBS)[number], ReturnType>; + const ctx = { getService: () => undefined, logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() } }; + registerDatasourceAdminRoutes(server as never, ctx as never, BASE); + return VERBS.flatMap((v) => server[v].mock.calls.map((c): [string, string] => [v.toUpperCase(), String(c[0])])); +} + +/** + * The literal single segments under the root that share a method with a + * `/:name` route — the names that route would never see. + */ +function shadowingLiterals(mounts: Array<[string, string]>): string[] { + const byNameMethods = new Set(mounts.filter(([, p]) => p === `${ROOT}/:name`).map(([m]) => m)); + const literals = new Set(); + for (const [method, path] of mounts) { + if (!path.startsWith(`${ROOT}/`)) continue; + const rest = path.slice(ROOT.length + 1); + if (rest.includes('/') || rest.startsWith(':')) continue; + if (byNameMethods.has(method)) literals.add(rest); + } + return [...literals].sort(); +} + +describe('reserved datasource names ↔ the registrar\'s literal siblings of /:name', () => { + it('reserves exactly the literals that shadow a same-method /:name route', () => { + const mounts = enumerateMounts(); + // Control: the enumeration saw the by-name read and a literal at all, so + // an empty answer below could not be a silent nothing-enumerated. + expect(mounts).toContainEqual(['GET', `${ROOT}/:name`]); + expect(mounts).toContainEqual(['POST', `${ROOT}/test`]); + + expect([...ROUTE_SHADOWED_DATASOURCE_NAMES].sort()).toEqual(shadowingLiterals(mounts)); + }); + + it('a literal beside a /:name route of ANOTHER method is not a shadowing one', () => { + // `POST /datasources/test` shares no method with a `/:name` route; were a + // `GET /datasources/test` mounted, `test` would join the derived set and + // the pin above would fail until it is reserved. + expect(shadowingLiterals([['POST', `${ROOT}/test`], ['GET', `${ROOT}/:name`]])).toEqual([]); + expect(shadowingLiterals([['GET', `${ROOT}/test`], ['GET', `${ROOT}/:name`]])).toEqual(['test']); + }); +}); + +// --------------------------------------------------------------------------- +// The create door, end to end. + +function harness(seed: StoredDatasource[] = []) { + const records: StoredDatasource[] = seed.map((r) => ({ ...r })); + const puts: string[] = []; + const config: DatasourceAdminServiceConfig = { + probe: async () => ({ ok: true, latencyMs: 1 }), + listDatasourceRecords: async () => records.map((r) => ({ ...r })), + getDatasourceRecord: async (n) => { + const r = records.find((x) => x.name === n); + return r ? { ...r } : undefined; + }, + putDatasourceRecord: async (record) => { + puts.push(record.name); + const i = records.findIndex((r) => r.name === record.name); + if (i >= 0) records[i] = { ...record }; + else records.push({ ...record }); + }, + deleteDatasourceRecord: async (n) => { + const i = records.findIndex((r) => r.name === n); + if (i >= 0) records.splice(i, 1); + }, + writeSecret: async (_input, hint) => `sys_secret://datasource/${hint.name}`, + countBoundObjects: async () => 0, + listObjectsBoundTo: async () => [], + }; + const service = new DatasourceAdminService(config); + const authService = createSessionAuthService(); + const grantsEngine = createGrantsEngine(); + const server = new HonoHttpServer(0); + const ctx = { + getService: (name: string) => + name === 'auth' + ? authService + : name === 'objectql' || name === 'data' + ? grantsEngine + : name === 'datasource-admin' + ? service + : undefined, + }; + registerDatasourceAdminRoutes(server, ctx as never, BASE); + const app = server.getRawApp(); + const call = (path: string, method = 'GET', body?: unknown) => + app.fetch( + new Request(`http://local${path}`, { + method, + headers: { 'content-type': 'application/json', authorization: ENTITLED_CREDENTIAL }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }), + ); + return { call, puts, records }; +} + +const draft = (name: string) => ({ name, driver: 'postgres', config: { database: 'analytics' } }); + +describe('POST /api/v1/datasources — route-shadowed names', () => { + it('refuses `drivers` with the 400 DATASOURCE_ADMIN_ERROR naming the segment, and writes nothing', async () => { + const { call, puts, records } = harness(); + const res = await call(ROOT, 'POST', draft('drivers')); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: { code: string; message: string } }; + expect(body.error.code).toBe('DATASOURCE_ADMIN_ERROR'); + expect(body.error.message).toContain("'drivers'"); + expect(puts).toEqual([]); + expect(records).toEqual([]); + }); + + it('keeps refusing the host-owned `default` the same way', async () => { + const { call, puts } = harness(); + const res = await call(ROOT, 'POST', draft('default')); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: { code: string; message: string } }; + expect(body.error.code).toBe('DATASOURCE_ADMIN_ERROR'); + expect(body.error.message).toContain("'default'"); + expect(puts).toEqual([]); + }); + + it('still creates an ordinary name (201) and reads it back by name', async () => { + const { call, puts } = harness(); + const res = await call(ROOT, 'POST', draft('reporting')); + expect(res.status).toBe(201); + expect(puts).toEqual(['reporting']); + const read = await call(`${ROOT}/reporting`); + expect(read.status).toBe(200); + expect(((await read.json()) as any).data.datasource.name).toBe('reporting'); + }); + + it('still creates `test` — the draft-probe literal shadows no by-name door', async () => { + const { call, puts } = harness(); + const res = await call(ROOT, 'POST', draft('test')); + expect(res.status).toBe(201); + expect(puts).toEqual(['test']); + const read = await call(`${ROOT}/test`); + expect(read.status).toBe(200); + expect(((await read.json()) as any).data.datasource.name).toBe('test'); + }); +}); + +describe('a stored `drivers` row written before the refusal', () => { + it('stays listed, patchable and removable — the refusal is at create only', async () => { + const { call, records } = harness([{ name: 'drivers', driver: 'postgres', config: { database: 'a' }, origin: 'runtime' }]); + + const list = await call(ROOT); + expect(list.status).toBe(200); + expect(((await list.json()) as any).data.datasources.map((d: { name: string }) => d.name)).toContain('drivers'); + + const patched = await call(`${ROOT}/drivers`, 'PATCH', { label: 'Old drivers' }); + expect(patched.status).toBe(200); + expect(records.find((r) => r.name === 'drivers')?.label).toBe('Old drivers'); + + const removed = await call(`${ROOT}/drivers`, 'DELETE'); + expect(removed.ok).toBe(true); + expect(records.some((r) => r.name === 'drivers')).toBe(false); + }); +}); diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index fe3f01a3029..b77801e2edd 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -45,6 +45,7 @@ import type { IAuthService, IDataEngine, IHttpServer } from '@objectstack/spec/c // The declared envelope is written in ONE place for the whole platform (#3973). import { inProcessSessionReadInput, sendOk, sendError } from '@objectstack/types'; import { DRIVER_CATALOG } from './driver-catalog.js'; +import { DRIVER_CATALOG_SEGMENT } from './datasource-route-reserved-names.js'; // [#27618] The package's one "no such datasource" refusal: GET answers an // unknown name in its words, as every other by-name door does. import { datasourceNotFoundError } from './datasource-admin-service.js'; @@ -692,7 +693,7 @@ export function registerDatasourceAdminRoutes( // Catalog of connection drivers + their JSON-Schema config (drives the // Studio connection form). Static metadata — no service dependency, so it // is always available even before any datasource-admin service is wired. - server.get(`${root}/drivers`, async (req: any, res: any) => { + server.get(`${root}/${DRIVER_CATALOG_SEGMENT}`, async (req: any, res: any) => { if (await requireDatasourceAdmin(req, res)) return; sendOk(res, { drivers: DRIVER_CATALOG }); }); diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index 144b0a89334..452539a9cf4 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -43,6 +43,7 @@ import { datasourceConnectivityChanged } from './datasource-connectivity-change. import { redactDatasourceConfig, restoreRedactedConfig } from './datasource-config-redaction.js'; import { planCredentialMigration } from './datasource-credential-migration.js'; import { borrowedConnectorOf } from './datasource-connector-credential.js'; +import { ROUTE_SHADOWED_DATASOURCE_NAMES } from './datasource-route-reserved-names.js'; import type { IDatasourceAdminService, DatasourceDraft, @@ -1401,6 +1402,17 @@ export class DatasourceAdminService implements IDatasourceAdminService { `Datasource name 'default' is reserved for the host's primary datasource. Pick another name.`, ); } + // Route-shadowed names (#29164): a literal the admin registrar mounts beside + // a same-method `/datasources/:name` route captures that path first, so a + // datasource of that name could never be read through its by-name door + // (`GET /datasources/drivers` is the driver catalog, not the edit form's + // detail read). The list is the one the registrar mounts from. + if (ROUTE_SHADOWED_DATASOURCE_NAMES.includes(name)) { + throw new Error( + `Datasource name '${name}' is reserved: the datasource admin API serves its own route at ` + + `'/datasources/${name}', so a datasource of that name could never be read by name. Pick another name.`, + ); + } } /** diff --git a/packages/services/service-datasource/src/datasource-route-reserved-names.ts b/packages/services/service-datasource/src/datasource-route-reserved-names.ts new file mode 100644 index 00000000000..f7cd8d664c2 --- /dev/null +++ b/packages/services/service-datasource/src/datasource-route-reserved-names.ts @@ -0,0 +1,38 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Datasource names the admin route family can never serve by name (#29164). + * + * `registerDatasourceAdminRoutes` mounts the by-name doors at + * `/datasources/:name` and, beside them, literal single-segment routes under + * the same root. Where a literal shares a METHOD with a `/:name` route it is + * registered first and captures the path: `GET /datasources/drivers` answers + * the driver catalog, so a datasource named `drivers` could be created and + * listed while its detail read — the load of its edit form — was never + * reached. Each such literal is therefore a reserved datasource name, refused + * at create the same way the host-owned `default` is. + * + * A literal that shares no method with a `/:name` route captures no name, and + * is not reserved: `POST /datasources/test` (the unsaved-draft probe) sits + * beside no `POST /datasources/:name`, so a datasource named `test` is read, + * patched, removed and probed (`POST /datasources/test/test`) through its own + * doors. + * + * ONE list for both halves. The registrar mounts each shadowing literal from + * the constant below, and `DatasourceAdminService` refuses each name in + * {@link ROUTE_SHADOWED_DATASOURCE_NAMES}. The drift pin + * (`__tests__/datasource-route-reserved-names.test.ts`) enumerates what the + * registrar actually mounts and fails both ways: a literal sibling of a + * same-method `/:name` route that is not listed here, and a listed name that + * no longer shadows anything. + */ + +/** The driver-catalog literal: `GET /datasources/drivers`, mounted beside `GET /datasources/:name`. */ +export const DRIVER_CATALOG_SEGMENT = 'drivers'; + +/** + * Every literal segment the admin registrar mounts beside a same-method + * `/datasources/:name` route — a datasource of one of these names could never + * be reached through that by-name door. + */ +export const ROUTE_SHADOWED_DATASOURCE_NAMES: readonly string[] = Object.freeze([DRIVER_CATALOG_SEGMENT]);