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. 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..0d2677c8cb 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -1,8 +1,25 @@ // 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 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'; @@ -65,6 +82,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 +142,64 @@ 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: IAuthService | undefined; + try { + 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) => { + // 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 }); + }; +} + export function registerDatasourceAdminRoutes( server: IHttpServer, ctx: PluginContext, @@ -117,6 +207,107 @@ 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); + // `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'); + } 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 +385,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 +400,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 +427,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 +450,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 +463,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 +487,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 +499,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 +515,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 +529,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 +543,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 +557,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') }], + }, });