From 77ba579272654b93a5b94c6cb3ea4906eca16819 Mon Sep 17 00:00:00 2001 From: objectstack-agent Date: Tue, 18 Aug 2026 15:19:42 +0000 Subject: [PATCH 1/3] fix(service-datasource): require authentication on the datasource-admin HTTP family MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The eleven `/api/v1/datasources` routes mount straight onto `IHttpServer` from a plugin `init()`, so they pass through neither seam that produces the platform's 401s — the REST server's `enforceAuth` runs inside `RestServer`'s own handlers, and the dispatcher domains' anonymous floor runs inside the dispatcher. Neither is reachable from a direct mount, and the registrar carried no check of its own, so the family answered every caller alike while its siblings answered 401 UNAUTHENTICATED. The registrar now applies the platform's shared anonymous-deny decision (`shouldDenyAnonymous`) over the platform's shared identity resolution (`resolveAuthzContext`) to all eleven routes, before any service is resolved and before any handler body runs. Both are imported rather than restated: the decision so this family cannot drift on who counts as anonymous, and the resolution so every credential kind the platform admits — session and API key alike — is admitted here too. Fail-closed throughout; the refusal carries the shared status, code and message in this surface's own `sendError` envelope, the same shape the sibling direct-mount registrar in `@objectstack/rest` uses. Authentication only. Whether these routes should further require a platform-configuration capability is a separate, separately-ruled question (#9593) and has no scaffolding here. Pinned by a both-sides test on one boot: an anonymous caller is refused on every read and every write verb, and an entitled caller still succeeds on the same routes in the same run. The package's existing fixtures now present a session, since the guard is their premise rather than their subject. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza --- .../remote-tables-twin.equivalence.test.ts | 19 +- .../__tests__/admin-routes-auth-guard.test.ts | 179 +++++++++++++++++ .../src/__tests__/admin-routes.test.ts | 38 +++- .../__tests__/envelope.conformance.test.ts | 24 ++- .../service-datasource/src/admin-routes.ts | 188 +++++++++++++++++- .../src/datasource-route-ledger.ts | 9 + .../service-datasource/vitest.config.ts | 21 ++ 7 files changed, 469 insertions(+), 9 deletions(-) create mode 100644 packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts diff --git a/packages/rest/src/remote-tables-twin.equivalence.test.ts b/packages/rest/src/remote-tables-twin.equivalence.test.ts index f16ef678e5..ce4eab576f 100644 --- a/packages/rest/src/remote-tables-twin.equivalence.test.ts +++ b/packages/rest/src/remote-tables-twin.equivalence.test.ts @@ -93,6 +93,15 @@ const REMOTE: IntrospectedSchema = { }, }; +/** The credential the admin spelling's authentication floor admits (#9391). */ +const SESSION = 'Bearer twin-session'; +const authService = { + api: { + getSession: async ({ headers }: { headers: Headers }) => + headers?.get?.('authorization') === SESSION ? { user: { id: 'u_twin' } } : null, + }, +}; + /** * One server, one service, both registrars — the point of the fixture. * @@ -112,6 +121,12 @@ function mountBoth() { const ctx = { getService: (name: string) => { if (name === 'external-datasource') return service; + // The admin spelling requires authentication (#9391) and resolves the + // caller through the platform's shared resolver, so the fixture wires an + // `auth` service that admits `SESSION` below. Without it this file would + // compare a 200 against a 401 and read the difference as a request-shape + // divergence — which is the one thing it exists NOT to confuse. + if (name === 'auth') return authService; throw new Error(`no service: ${name}`); }, } as any; @@ -133,7 +148,9 @@ interface Reading { /** Drive one spelling and read back the table set it answers with. */ async function read(app: any, spelling: keyof typeof SPELLING, qs: string): Promise { - const res = await app.fetch(new Request(`http://local${SPELLING[spelling](qs)}`)); + const res = await app.fetch( + new Request(`http://local${SPELLING[spelling](qs)}`, { headers: { authorization: SESSION } }), + ); const body = (await res.json()) as { success: boolean; data?: { tables?: Reading['tables'] } }; return { status: res.status, tables: body.data?.tables ?? [] }; } diff --git a/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts b/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts new file mode 100644 index 0000000000..92c410d6d2 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The authentication pin for the datasource-admin HTTP family. + * + * ## Why both halves, and why on ONE boot + * + * This family mounts straight onto `IHttpServer` from a plugin `init()`, which + * is outside every seam that produces the platform's 401s — the REST server's + * `enforceAuth` and the dispatcher domains' anonymous floor both sit on routes + * this registrar never passes through. A guard added here is therefore the only + * thing standing between an anonymous caller and datasource lifecycle + * management, and a test that only asserts the refusal cannot tell "guarded" + * apart from "broken": an unconditional 401 would pass it perfectly while + * taking the Setup → Datasources console offline for everyone. + * + * So every route below is asserted TWICE against the SAME mounted app — + * `family` is built once at module scope, so the anonymous refusal and the + * entitled success are answers from one boot of one registrar, not from two + * differently-wired fixtures that could disagree for reasons other than the + * caller's identity. + * + * The two halves are separate `it`s rather than one, deliberately: that is what + * makes the red/green split countable when the guard is reverted — the + * anonymous half must go red and the entitled half must stay green, and a + * single combined case would hide the second fact behind the first failure. + * + * ## What "entitled" means here, and what it does not + * + * Exactly one thing: the caller is AUTHENTICATED. This family's guard is an + * authentication floor, and nothing in this file asserts a capability — whether + * these routes should further require something like `manage_platform_settings` + * is a separate, separately-ruled question (#9593) and deliberately has no + * scaffolding here. + * + * Identity is resolved by the registrar through the platform's shared + * `resolveAuthzContext`, so the fake `auth` service below is the real seam a + * session arrives through, not a test-only bypass. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { HonoHttpServer } from '@objectstack/plugin-hono-server'; +import { registerDatasourceAdminRoutes } from '../admin-routes.js'; + +/** The credential the fake `auth` service below admits. */ +const ENTITLED = 'Bearer entitled-session'; + +/** Every service method the family dispatches to, as spies. */ +function createServiceDouble() { + return { + listDatasources: vi.fn().mockResolvedValue([{ name: 'pg', origin: 'runtime', health: 'ok' }]), + getDatasource: vi.fn().mockResolvedValue({ name: 'pg', driver: 'sqlite' }), + createDatasource: vi.fn().mockResolvedValue({ name: 'created', driver: 'sqlite' }), + updateDatasource: vi.fn().mockResolvedValue({ name: 'pg', driver: 'sqlite' }), + removeDatasource: vi.fn().mockResolvedValue(undefined), + migrateCredential: vi.fn().mockResolvedValue({ status: 'migrated' }), + listRemoteTables: vi.fn().mockResolvedValue([{ name: 'customers' }]), + generateObjectDraft: vi.fn().mockResolvedValue({ name: 'customer' }), + // `testConnection` is claimed by BOTH services this module dispatches to + // (an unsaved draft on `datasource-admin`, a saved name on + // `external-datasource`); one double serves both lookups. + testConnection: vi.fn().mockResolvedValue({ ok: true }), + }; +} + +/** + * Mount the family once, with a fake `auth` service that admits exactly one + * credential. `objectql` resolves to `undefined` — the shared resolver reads it + * only to aggregate permissions, and this pin asserts authentication, so an + * absent engine must not change who is admitted. + */ +function mountFamily() { + const service = createServiceDouble(); + const auth = { + api: { + getSession: async ({ headers }: { headers: Headers }) => + headers?.get?.('authorization') === ENTITLED ? { user: { id: 'u_entitled' } } : null, + }, + }; + const ctx = { + getService: vi.fn((name: string) => { + if (name === 'auth') return auth; + if (name === 'objectql' || name === 'data') return undefined; + return service; + }), + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + } as any; + const server = new HonoHttpServer(0); + registerDatasourceAdminRoutes(server, ctx, '/api/v1'); + return { app: server.getRawApp(), service }; +} + +/** One mount for the whole file — the "same boot" both halves are asserted on. */ +const family = mountFamily(); + +interface RouteCase { + /** Reads as a sentence in the test name. */ + name: string; + method: 'GET' | 'POST' | 'PATCH' | 'DELETE'; + path: string; + body?: Record; + /** The status an ENTITLED caller gets. */ + okStatus: number; + /** + * The service method this route dispatches to, if any. Asserted uncalled on + * the anonymous half: refusing AFTER dispatch would still leak the write. + * `GET /drivers` is static metadata and dispatches to nothing. + */ + dispatches?: keyof ReturnType; +} + +/** Reads. */ +const READ_CASES: RouteCase[] = [ + { name: 'GET /datasources (list)', method: 'GET', path: '/api/v1/datasources', okStatus: 200, dispatches: 'listDatasources' }, + { name: 'GET /datasources/drivers (driver catalog)', method: 'GET', path: '/api/v1/datasources/drivers', okStatus: 200 }, + { name: 'GET /datasources/:name (read)', method: 'GET', path: '/api/v1/datasources/pg', okStatus: 200, dispatches: 'getDatasource' }, + { name: 'GET /datasources/:name/remote-tables (remote-table introspection)', method: 'GET', path: '/api/v1/datasources/pg/remote-tables', okStatus: 200, dispatches: 'listRemoteTables' }, +]; + +/** + * Writes — every state-changing verb spelled out. The card's acceptance names + * create, patch and remove explicitly because a list-only assertion would have + * left the three routes that actually mutate the deployment unpinned. + */ +const WRITE_CASES: RouteCase[] = [ + { name: 'POST /datasources (create)', method: 'POST', path: '/api/v1/datasources', body: { name: 'new_ds', driver: 'sqlite' }, okStatus: 201, dispatches: 'createDatasource' }, + { name: 'PATCH /datasources/:name (patch)', method: 'PATCH', path: '/api/v1/datasources/pg', body: { driver: 'sqlite' }, okStatus: 200, dispatches: 'updateDatasource' }, + { name: 'DELETE /datasources/:name (remove)', method: 'DELETE', path: '/api/v1/datasources/pg', okStatus: 204, dispatches: 'removeDatasource' }, + { name: 'POST /datasources/test (probe an unsaved draft)', method: 'POST', path: '/api/v1/datasources/test', body: { driver: 'sqlite' }, okStatus: 200, dispatches: 'testConnection' }, + { name: 'POST /datasources/:name/test (probe a saved datasource)', method: 'POST', path: '/api/v1/datasources/pg/test', body: {}, okStatus: 200, dispatches: 'testConnection' }, + { name: 'POST /datasources/:name/object-draft (introspect + draft)', method: 'POST', path: '/api/v1/datasources/pg/object-draft', body: { table: 'customers' }, okStatus: 200, dispatches: 'generateObjectDraft' }, + { name: 'POST /datasources/:name/migrate-credential (re-home a stored credential)', method: 'POST', path: '/api/v1/datasources/pg/migrate-credential', body: {}, okStatus: 200, dispatches: 'migrateCredential' }, +]; + +const ALL_CASES = [...READ_CASES, ...WRITE_CASES]; + +async function drive(c: RouteCase, credential?: string) { + const headers: Record = { 'content-type': 'application/json' }; + if (credential) headers.authorization = credential; + const res = await family.app.fetch( + new Request(`http://local${c.path}`, { + method: c.method, + headers, + body: c.body === undefined ? undefined : JSON.stringify(c.body), + }), + ); + const text = await res.text(); + return { status: res.status, body: text ? JSON.parse(text) : undefined }; +} + +beforeEach(() => { + for (const fn of Object.values(family.service)) fn.mockClear(); +}); + +describe('datasource-admin family — the anonymous caller is refused (read AND write)', () => { + for (const c of ALL_CASES) { + it(`${c.name} answers 401 UNAUTHENTICATED with no session`, async () => { + const { status, body } = await drive(c); + // The status AND the machine-readable code, not merely "not 200": the + // contract this restores is the one the sibling families answer, and a + // bare "not 200" would be satisfied by the 503 an unwired service gives. + expect(status).toBe(401); + expect(body?.error?.code).toBe('UNAUTHENTICATED'); + // The refusal precedes dispatch — an anonymous DELETE that reached the + // service and was refused afterwards would already have removed the row. + if (c.dispatches) expect(family.service[c.dispatches]).not.toHaveBeenCalled(); + }); + } +}); + +describe('datasource-admin family — the entitled caller still succeeds', () => { + for (const c of ALL_CASES) { + it(`${c.name} answers ${c.okStatus} for an authenticated caller`, async () => { + const { status } = await drive(c, ENTITLED); + expect(status).toBe(c.okStatus); + if (c.dispatches) expect(family.service[c.dispatches]).toHaveBeenCalled(); + }); + } +}); diff --git a/packages/services/service-datasource/src/__tests__/admin-routes.test.ts b/packages/services/service-datasource/src/__tests__/admin-routes.test.ts index b56e36626b..623d5f1f8e 100644 --- a/packages/services/service-datasource/src/__tests__/admin-routes.test.ts +++ b/packages/services/service-datasource/src/__tests__/admin-routes.test.ts @@ -13,15 +13,45 @@ import { registerDatasourceAdminRoutes } from '../admin-routes.js'; * service. */ +/** + * The credential every request in this file carries, and the fake `auth` + * service that admits it. + * + * This family requires authentication (#9391), so a fixture that presented no + * identity would answer 401 to every case below — a suite measuring the guard + * instead of the routing and failure-attribution it exists to measure. The + * guard itself has its own both-sides pin, `admin-routes-auth-guard.test.ts`; + * here an authenticated caller is the premise, not the subject. + */ +const SESSION = 'Bearer test-session'; +const authService = { + api: { + getSession: async ({ headers }: { headers: Headers }) => + headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null, + }, +}; + +/** + * Wrap a `getService` so `auth` resolves to the fake above and every other + * lookup keeps the behaviour the case under test wired — including throwing, + * which is what drives the resolver's catch arm. + */ +const withAuth = (getService: (name: string) => unknown) => + vi.fn((name: string) => (name === 'auth' ? authService : getService(name))); + const json = (path: string, init?: RequestInit) => new Request(`http://local${path}`, { ...init, - headers: { 'content-type': 'application/json', ...(init?.headers ?? {}) }, + headers: { + 'content-type': 'application/json', + authorization: SESSION, + ...(init?.headers ?? {}), + }, }); function mount(svc: unknown) { const server = new HonoHttpServer(0); - const ctx = { getService: vi.fn().mockReturnValue(svc) } as any; + const ctx = { getService: withAuth(() => svc) } as any; registerDatasourceAdminRoutes(server, ctx, '/api/v1'); return server.getRawApp(); } @@ -37,7 +67,7 @@ function mount(svc: unknown) { */ function mountServices(services: Record) { const server = new HonoHttpServer(0); - const ctx = { getService: vi.fn((name: string) => services[name]) } as any; + const ctx = { getService: withAuth((name: string) => services[name]) } as any; registerDatasourceAdminRoutes(server, ctx, '/api/v1'); return server.getRawApp(); } @@ -174,7 +204,7 @@ describe('registerDatasourceAdminRoutes (real HonoHttpServer)', () => { // this file returns `undefined` instead, so nothing else drives this branch. const server = new HonoHttpServer(0); const ctx = { - getService: vi.fn(() => { + getService: withAuth(() => { throw new Error('service "datasource-admin" is not registered'); }), } as any; diff --git a/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts b/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts index 36f1f58579..a5eb4cde8c 100644 --- a/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts +++ b/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts @@ -39,15 +39,35 @@ import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; import { HonoHttpServer } from '@objectstack/plugin-hono-server'; import { registerDatasourceAdminRoutes } from '../admin-routes.js'; +/** + * The family requires authentication (#9391), so every request below carries a + * session and the mock context resolves an `auth` service that admits it. The + * subject here is the ENVELOPE of the success and refusal bodies; an + * unauthenticated fixture would replace all of them with the guard's 401 and + * this file would stop covering what it exists to cover. The 401's own + * envelope is asserted by `admin-routes-auth-guard.test.ts`. + */ +const SESSION = 'Bearer test-session'; +const authService = { + api: { + getSession: async ({ headers }: { headers: Headers }) => + headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null, + }, +}; + const req = (path: string, init?: RequestInit) => new Request(`http://local${path}`, { ...init, - headers: { 'content-type': 'application/json', ...(init?.headers ?? {}) }, + headers: { + 'content-type': 'application/json', + authorization: SESSION, + ...(init?.headers ?? {}), + }, }); function mount(svc: unknown) { const server = new HonoHttpServer(0); - const ctx = { getService: vi.fn().mockReturnValue(svc) } as any; + const ctx = { getService: vi.fn((name: string) => (name === 'auth' ? authService : svc)) } as any; registerDatasourceAdminRoutes(server, ctx, '/api/v1'); return server.getRawApp(); } diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index d0525574cd..5519a0d161 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -1,6 +1,17 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { PluginContext } from '@objectstack/core'; +// The authentication floor: the platform's ONE anonymous-deny decision plus the +// ONE identity resolution every other HTTP seam reads. Both are imported rather +// than restated — see `requireAuthenticated` below for why a local check would +// be the wrong shape even if it were written correctly. +import { + resolveAuthzContext, + shouldDenyAnonymous, + ANONYMOUS_DENY_STATUS, + ANONYMOUS_DENY_CODE, + ANONYMOUS_DENY_MESSAGE, +} from '@objectstack/core'; import type { ErrorCode } from '@objectstack/spec/api'; import type { IHttpServer } from '@objectstack/spec/contracts'; // The declared envelope is written in ONE place for the whole platform (#3973). @@ -65,6 +76,21 @@ const SERVICE_ERROR_CODE: Record = { * * `GET /datasources/drivers` is static metadata and needs neither service. * + * ## Every route above requires authentication (#9391) + * + * All eleven — reads, writes and the static catalog alike — answer `401` + * `UNAUTHENTICATED` to a caller with no resolvable identity, before any service + * is resolved and before any handler body runs. See `requireAuthenticated` + * inside the registrar for how that decision is reached and why it has to be + * made here rather than inherited from a seam. + * + * The catalog route is included on purpose. It needs no service and reveals no + * deployment data, but the family's own route ledger dispositions it + * `server-only` — as it does every row here; the ledger's `public` disposition + * exists and is used by nothing in this file — and a family whose floor has one + * hole is a family whose floor has to be read route by route. Uniform is the + * property worth having. + * * Request bodies carry the connection draft inline with an optional cleartext * `secret` field; the route splits `secret` out so it never reaches the draft * the service persists. @@ -110,6 +136,57 @@ const SERVICE_ERROR_CODE: Record = { * per-service 503. That puts the whole burden of naming the service on one * string — see `resolve` below for how it is kept honest (#4225). */ +/** + * Normalize the adapter's request headers to a Web `Headers`. + * + * `IHttpServer` hands handlers a plain `Record` (the hono + * adapter builds it from `c.req.header()`), while the session resolver behind + * `resolveAuthzContext` is better-auth's, which reads a Web `Headers`. A + * request whose headers are already a `Headers` is passed through untouched — + * an adapter is free to hand over either. + * + * Returns `undefined` for a request carrying no readable headers at all. That + * is a DENIAL, not a pass: the caller below treats it as "no identity could be + * read", which is what an anonymous request is. + */ +function toWebHeaders(raw: unknown): Headers | undefined { + if (raw && typeof (raw as Headers).get === 'function') return raw as Headers; + if (raw && typeof raw === 'object') { + const headers = new Headers(); + for (const [k, v] of Object.entries(raw as Record)) { + if (v == null) continue; + if (Array.isArray(v)) for (const one of v) headers.append(k, String(one)); + else headers.set(k, String(v)); + } + return headers; + } + return undefined; +} + +/** + * A `getSession(headers)` bound to the kernel's `auth` service, or `undefined` + * when this deployment registers none. + * + * `undefined` does NOT open the routes. It removes one of the two credential + * paths `resolveAuthzContext` consults (the better-auth session), leaving the + * API-key path; a caller presenting neither resolves anonymous and is refused + * below. A kernel with no auth service simply has no session to present. + */ +function buildGetSession(ctx: PluginContext): ((headers: Headers) => Promise) | undefined { + let authService: any; + try { + authService = ctx.getService('auth'); + } catch { + return undefined; + } + if (!authService) return undefined; + return async (headers: Headers) => { + let api: any = authService.api; + if (!api && typeof authService.getApi === 'function') api = await authService.getApi(); + return api?.getSession?.({ headers }); + }; +} + export function registerDatasourceAdminRoutes( server: IHttpServer, ctx: PluginContext, @@ -117,6 +194,102 @@ export function registerDatasourceAdminRoutes( ): void { const root = `${basePath}/datasources`; + /** + * The authentication floor for this whole family (#9391). Answers `401 + * UNAUTHENTICATED` and returns `true` when the caller must be refused, so + * every handler opens with `if (await requireAuthenticated(req, res)) return;`. + * + * ## Why this module needs its own line at all + * + * These routes are mounted straight onto `IHttpServer` from a plugin `init()` + * — the third mount style this family's route ledger describes — so they pass + * through neither of the seams that produce the platform's 401s: the REST + * server's `enforceAuth` (which guards `/data`, `/meta`, `/batch`, + * `/security/explain`) runs inside `RestServer`'s own handlers, and the + * dispatcher domains' anonymous floor runs inside the dispatcher. Neither is + * reachable from here, and neither is a middleware anything could be routed + * through. The sibling direct-mount registrar in `@objectstack/rest` + * (`package-routes.ts`) reached the same conclusion and took the same shape + * for the same reason. + * + * ## What is imported rather than restated, and why that matters + * + * Two things, and they are the whole point of the fix: + * + * - the DECISION — `shouldDenyAnonymous` (`@objectstack/core`), the one + * function every HTTP seam shares, so this family can never drift on who + * counts as anonymous. `isSystem` is never settable from the wire, and a + * CORS `OPTIONS` preflight passes, both by that function's construction. + * No `path` is passed: the control-plane allowlist exists for `/auth`, + * `/health`, `/ready` and `/discovery`, and nothing here is one of those. + * - the IDENTITY — `resolveAuthzContext` (`@objectstack/core`), the same + * resolution `RestServer` and the runtime dispatcher perform. Reading only + * a better-auth session here would have been cheaper and WRONG in the + * direction that matters: it would refuse a caller presenting a valid + * `sys_api_key`, which is a credential this platform admits everywhere + * else. Admitting every credential kind the platform admits is what makes + * this a floor rather than a second, narrower policy. + * + * ## Fail-closed, and where the check sits + * + * Anything that throws or resolves to no identity is a refusal — an + * unresolvable request is anonymous, and there is no fallback that opens the + * routes. The check also runs BEFORE `resolve()`: an anonymous caller must + * not learn from a `503` which services this deployment has wired, and a + * refusal that landed after dispatch would already have performed the write + * it was refusing. + * + * ## The envelope + * + * `sendError`, not the flat `ANONYMOUS_DENY_BODY` the `/data` + `/meta` + * `enforceAuth` seam writes. Both are live and sanctioned (ADR-0112's + * 2026-07-30 amendment records the flat and wrapped envelopes as the two); + * every other body in this module goes through the shared `sendOk`/`sendError` + * and `check:route-envelope` pins it at zero hand-written bodies, so the + * wrapper is this surface's, while the status, code and message are the + * shared ones. Same reasoning, same shape, as `package-routes.ts`. + * + * Authentication and nothing more: whether these routes should FURTHER + * require a platform-configuration capability is a separate, separately-ruled + * question (#9593) and is deliberately absent here. + */ + const requireAuthenticated = async (req: any, res: any): Promise => { + let userId: string | undefined; + try { + const headers = toWebHeaders(req?.headers); + if (headers) { + // Both lookups happen PER REQUEST, for the same reason `resolve()` + // below does it: this registrar runs inside a plugin `init()`, and a + // service resolved there is a boot-instant snapshot of a registry that + // is still filling. Binding the session resolver at registration time + // would answer "no auth service" on precisely the deployments that + // have one but register it later — every session-authenticated caller + // refused, with nothing in the registry to show for it. + const getSession = buildGetSession(ctx); + let ql: any; + try { + ql = ctx.getService('objectql') ?? ctx.getService('data'); + } catch { + ql = undefined; + } + const authz = await resolveAuthzContext({ ql, headers, getSession }); + userId = authz.userId; + } + } catch { + // Fail closed: an identity that could not be resolved is not an identity. + userId = undefined; + } + // `isSystem` is deliberately not read off anything the wire can reach — + // `ResolvedAuthzContext` has no such field, and inbound HTTP never carries + // one, so the only way past this line is a resolved caller. + if (shouldDenyAnonymous({ userId, method: req?.method })) { + sendError(res, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE); + return true; + } + return false; + }; + + /** * Resolve the service a route dispatches to — or answer * `503 SERVICE_UNAVAILABLE` naming THAT service and return `undefined`, which @@ -194,7 +367,8 @@ export function registerDatasourceAdminRoutes( // until #4264 — the one route in this module without one, so a backing-store // failure surfaced as the adapter's non-envelope 500 instead of the 400 its // eight siblings answer. - server.get(root, async (_req: any, res: any) => { + server.get(root, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'listDatasources'); if (!svc) return; try { @@ -208,7 +382,8 @@ 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}/drivers`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; sendOk(res, { drivers: DRIVER_CATALOG }); }); @@ -234,6 +409,7 @@ export function registerDatasourceAdminRoutes( // question #7606 owns globally; honouring it is correct under either answer, // so this route does not pre-empt it. server.get(`${root}/:name/remote-tables`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'external-datasource', 'listRemoteTables'); if (!svc) return; try { @@ -256,6 +432,7 @@ export function registerDatasourceAdminRoutes( // after the static `/drivers` route so that literal segment is never captured // as a name. server.get(`${root}/:name`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'getDatasource'); if (!svc) return; try { @@ -268,6 +445,7 @@ export function registerDatasourceAdminRoutes( }); server.post(`${root}/:name/test`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'external-datasource', 'testConnection'); if (!svc) return; try { @@ -291,6 +469,7 @@ export function registerDatasourceAdminRoutes( // plainly. Genuine refusals — an unknown name, a throwing store — still take // the `badRequest` arm below. server.post(`${root}/:name/migrate-credential`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'migrateCredential'); if (!svc) return; try { @@ -302,6 +481,7 @@ export function registerDatasourceAdminRoutes( }); server.post(`${root}/:name/object-draft`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'external-datasource', 'generateObjectDraft'); if (!svc) return; const { table, ...opts } = (req.body as Record) ?? {}; @@ -317,6 +497,7 @@ export function registerDatasourceAdminRoutes( // Probe a connection without persisting anything. Registered before the // `:name` routes so the literal `test` segment is never captured as a name. server.post(`${root}/test`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'testConnection'); if (!svc) return; const { draft, secret } = splitSecret(req.body); @@ -330,6 +511,7 @@ export function registerDatasourceAdminRoutes( // Create a runtime datasource. server.post(root, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'createDatasource'); if (!svc) return; const { draft, secret } = splitSecret(req.body); @@ -343,6 +525,7 @@ export function registerDatasourceAdminRoutes( // Patch a runtime datasource. server.patch(`${root}/:name`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'updateDatasource'); if (!svc) return; const { draft, secret } = splitSecret(req.body); @@ -356,6 +539,7 @@ export function registerDatasourceAdminRoutes( // Remove a runtime datasource. server.delete(`${root}/:name`, async (req: any, res: any) => { + if (await requireAuthenticated(req, res)) return; const svc = resolve(res, 'datasource-admin', 'removeDatasource'); if (!svc) return; try { diff --git a/packages/services/service-datasource/src/datasource-route-ledger.ts b/packages/services/service-datasource/src/datasource-route-ledger.ts index 56c90d8104..f58eaf42c2 100644 --- a/packages/services/service-datasource/src/datasource-route-ledger.ts +++ b/packages/services/service-datasource/src/datasource-route-ledger.ts @@ -27,6 +27,15 @@ * catalog, schema introspection — sat in the pre-#3563 posture: mounted, * working, and guarded by nothing. * + * That last clause was a live description of this family for as long as it + * stood here, and #7744 — which wrote it — added ledger rows rather than a + * guard. #9391 closed it: the registrar now applies the platform's shared + * anonymous-deny decision to all eleven routes before any of them dispatches, + * so every row below is reachable only by an authenticated caller. The + * sentence is kept rather than deleted because it is the reason the ledger + * exists — a mount style no ledger could see is how the posture went unnoticed + * — but it is history now, not the current posture. + * * WHAT #7744 ACTUALLY FOUND, and what it is NOT. The REST ledger carries five * `datasources` rows and every one of them is the FEDERATION family, spelled * `/api/v1/datasources/:name/external/…`. Read quickly that looks like the diff --git a/packages/services/service-datasource/vitest.config.ts b/packages/services/service-datasource/vitest.config.ts index ad9fc793c0..07c958f4cc 100644 --- a/packages/services/service-datasource/vitest.config.ts +++ b/packages/services/service-datasource/vitest.config.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { defineConfig } from 'vitest/config'; +import path from 'path'; export default defineConfig({ test: { @@ -18,4 +19,24 @@ export default defineConfig({ // set at the config layer so future cases are covered on arrival. testTimeout: 60_000, }, + resolve: { + // `@objectstack/core` became a VALUE import of this package's source when + // the datasource-admin routes took on the platform's authentication floor + // (#9391): they read the shared anonymous-deny decision and the shared + // identity resolution from it. Without this alias that import would follow + // the workspace link to `packages/core/dist/index.js` — a build artifact — + // and every case in this package touching the guard would be reporting on + // build state rather than on the source next to it. A dist merely BEHIND + // rather than missing the symbol is the dangerous direction: the pin runs + // GREEN against core's old behaviour with nothing in the output saying so, + // which on an authentication path means a green suite over an unguarded + // seam. `check:test-source-alias` is the gate; this is its intended repair + // (alias the import, never widen the baseline). + // + // Array form with an anchored pattern, deliberately: the object form + // matches by PREFIX, so a bare `@objectstack/core` entry would also swallow + // `@objectstack/core/logger` and resolve it to `core/src/index.ts/logger` + // (ENOTDIR). Same shape as `service-storage` and `service-knowledge`. + alias: [{ find: /^@objectstack\/core$/, replacement: path.resolve(__dirname, '../../core/src/index.ts') }], + }, }); From dd6ad3d6b55a77e229ae4cb8fb82e6324b1a91fe Mon Sep 17 00:00:00 2001 From: objectstack-agent Date: Tue, 18 Aug 2026 15:27:58 +0000 Subject: [PATCH 2/3] docs(changeset): datasource-admin authentication floor (#9391) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza --- .../datasource-admin-authentication-floor.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 .changeset/datasource-admin-authentication-floor.md diff --git a/.changeset/datasource-admin-authentication-floor.md b/.changeset/datasource-admin-authentication-floor.md new file mode 100644 index 0000000000..1f07d1e6fa --- /dev/null +++ b/.changeset/datasource-admin-authentication-floor.md @@ -0,0 +1,51 @@ +--- +"@objectstack/service-datasource": patch +--- + +fix(security): the datasource-admin HTTP family requires authentication (#9391) + +Every route `registerDatasourceAdminRoutes` mounts under `/api/v1/datasources` +— the list, the single read, the driver catalog, remote-table introspection, +the two connection probes, the credential migration, and create / patch / +remove — now answers `401 UNAUTHENTICATED` to a caller whose identity cannot be +resolved. The refusal is made before any service is resolved and before any +handler body runs, so an anonymous request reaches neither the datasource +lifecycle nor a configured remote. + +This family mounts straight onto `IHttpServer` from a plugin `init()`, which is +outside both seams that produce the platform's 401s: the REST server's +`enforceAuth` runs inside `RestServer`'s own handlers, and the dispatcher +domains' anonymous floor runs inside the dispatcher. Neither is a middleware a +direct mount can be routed through, and the registrar carried no check of its +own — so on a server where `/api/v1/data`, `/api/v1/meta`, `/api/v1/batch` and +`/api/v1/security/explain` all refuse an anonymous caller, this one family did +not. + +The guard imports rather than restates both halves of the decision: +`shouldDenyAnonymous` (the one anonymous-deny decision every HTTP seam shares, +so this family cannot drift on who counts as anonymous) over +`resolveAuthzContext` (the one identity resolution `RestServer` and the runtime +dispatcher perform, so every credential kind the platform admits — better-auth +session and `sys_api_key` alike — is admitted here too). It fails closed: +anything that throws or resolves to no identity is refused, and there is no +posture, config key or absent service that opens the routes. + +**Why this is a fix and not a feature, and why `patch` rather than a breaking +bump.** The change only ever narrows the accept set: every request admitted +after it was admitted before, and the requests it now refuses are exactly the +ones every sibling family already refuses. Nothing authorable is renamed, +retired or tombstoned, and no declared contract changes shape — the routes' +paths, request bodies, success payloads and existing failure codes are +untouched, so there is no ADR-0087 conversion to register and no upgrade +prescription to write. What changes is that a declared expectation starts being +enforced. A caller that depended on reaching platform datasource configuration +with no credential was depending on the defect. + +Authentication is the whole of it. Whether these routes should further require +a platform-configuration capability is a separate, separately-ruled question +(#9593) and is deliberately not anticipated here. + +Pinned by a both-sides test on one boot (`admin-routes-auth-guard.test.ts`): an +anonymous caller is refused on every read and on every write verb, and an +entitled caller still succeeds on the same routes in the same run — the second +half being what distinguishes a guarded family from a broken one. From 39235ad42ba51c2eb1106977f9c578cbfae70220 Mon Sep 17 00:00:00 2001 From: objectstack-agent Date: Tue, 18 Aug 2026 15:52:03 +0000 Subject: [PATCH 3/3] fix(service-datasource): type the guard's slot lookups against their contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The authentication guard resolved `auth`, `objectql` and `data` through `getService`, adding five erasure sites to a file the #4251 sweep ratchet grandfathers for its two pre-existing ones. `pnpm check:slot-lookup` caught it: 2 -> 7. ESLint alone could not — the file is grandfathered by `ignores`, and an ignored file is ignored completely, which is exactly why the ratchet re-measures with the grandfathering lifted. The lookups now carry their slots' contracts. That is not just gate compliance: `IAuthService` is what declares BOTH shapes of the session accessor, and the erased two-step read was the very shape the rule's message reports erasure hiding. The shipped `plugin-auth` registers an `AuthManager` with no `api` member at all, so a reader of `api` alone gets `undefined` on every current deployment and reads it as "no session" — the fallback to `getApi()` is now a checked expression against the contract rather than a `typeof` probe against `any`. `IDataEngine` types both engine spellings, matching how other services resolve the same pair; the resolver reads exactly `find` off it. Behaviour is unchanged: `??` and the optional call cover the same shapes the falsy check and the `typeof` probe did, and the both-sides pin still splits 11 red / 11 green when the guard is reverted. The file's two pre-existing sites are left as recorded. They are a dynamic method-name dispatch shared by two different service contracts, so typing them is a real refactor of all eleven handlers' dispatch — #4251's batch work, not a rider on a security fix. The ratchet's count for this file is unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza --- .../service-datasource/src/admin-routes.ts | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index 5519a0d161..0d2677c8cb 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -13,7 +13,13 @@ import { ANONYMOUS_DENY_MESSAGE, } from '@objectstack/core'; import type { ErrorCode } from '@objectstack/spec/api'; -import type { IHttpServer } from '@objectstack/spec/contracts'; +// The slots this module resolves, by their declared contracts. Erasing a +// lookup to `any` is banned (#4127/#4176/#4202/#4251) and the ban is right +// here: `IAuthService` is what declares BOTH shapes of the session accessor +// (`api` and the lazy `getApi()`), so the two-step read below is a checked +// expression rather than a pair of guesses — the exact gap the rule's own +// message reports that erasure hiding, on the exact member this guard reads. +import type { IAuthService, IDataEngine, IHttpServer } from '@objectstack/spec/contracts'; // The declared envelope is written in ONE place for the whole platform (#3973). import { sendOk, sendError } from '@objectstack/types'; import { DRIVER_CATALOG } from './driver-catalog.js'; @@ -173,16 +179,23 @@ function toWebHeaders(raw: unknown): Headers | undefined { * below. A kernel with no auth service simply has no session to present. */ function buildGetSession(ctx: PluginContext): ((headers: Headers) => Promise) | undefined { - let authService: any; + let authService: IAuthService | undefined; try { - authService = ctx.getService('auth'); + authService = ctx.getService('auth'); } catch { return undefined; } if (!authService) return undefined; + // Narrowed once, outside the closure, so the closure body needs no re-check. + const service = authService; return async (headers: Headers) => { - let api: any = authService.api; - if (!api && typeof authService.getApi === 'function') api = await authService.getApi(); + // Both accessors are declared on the contract, and reading only the first + // is a known way to get a silent anonymous: the shipped `plugin-auth` + // registers an `AuthManager`, which has no `api` member at all, so `api` + // alone yields `undefined` on every current deployment and the caller + // reads that as "no session". `getApi()` is the accessor to prefer; + // `api` is its legacy twin, kept because a provider may still mount it. + const api = service.api ?? (await service.getApi?.()); return api?.getSession?.({ headers }); }; } @@ -266,9 +279,14 @@ export function registerDatasourceAdminRoutes( // have one but register it later — every session-authenticated caller // refused, with nothing in the registry to show for it. const getSession = buildGetSession(ctx); - let ql: any; + // `IDataEngine` for both spellings: the resolver reads exactly `find` + // off this, which is the data plane's own surface, and the same pair + // is spelled this way where other services resolve the engine. The + // two names are one registration — `packages/objectql` registers one + // object under both, two lines apart. + let ql: IDataEngine | undefined; try { - ql = ctx.getService('objectql') ?? ctx.getService('data'); + ql = ctx.getService('objectql') ?? ctx.getService('data'); } catch { ql = undefined; }