From de8fac4c2b2fb5f11cb3b4d7eb82f5253349a91e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 01:43:35 +0000 Subject: [PATCH] security(rest): require manage_platform_settings on POST /datasources/:name/external/validate The one federation route the #9901 ruling did not name joins the family's read gate: validateAll drives the same live remote-schema introspection the gated read twins expose, so it answers to the same capability. The now-unused 'authenticated' guard kind is removed. Ruled on #10255 (2026-08-20, option A). Fixes #10255 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --- .../external-validate-read-capability.md | 11 +++ ...al-datasource-envelope.conformance.test.ts | 7 +- ...ernal-datasource-routes-auth-guard.test.ts | 74 ++++++++++++++----- .../rest/src/external-datasource-routes.ts | 50 +++++++++---- 4 files changed, 103 insertions(+), 39 deletions(-) create mode 100644 .changeset/external-validate-read-capability.md diff --git a/.changeset/external-validate-read-capability.md b/.changeset/external-validate-read-capability.md new file mode 100644 index 0000000000..c1b5b84cfb --- /dev/null +++ b/.changeset/external-validate-read-capability.md @@ -0,0 +1,11 @@ +--- +"@objectstack/rest": patch +--- + +**Behaviour change (tightening) — `POST /datasources/:name/external/validate` now requires `manage_platform_settings`** (#10255, completing the #9901 federation-family gate). This was the one route of the external-datasource federation family still admitting **any authenticated caller**; it now requires the same capability as the family's two read routes. Maintainer ruling, 2026-08-20 (verbatim: 「同意你的意见。」, accepting option A on #10255). + +**This is published SDK surface.** `datasources.external.validate` on `ObjectStackClient` and the CLI's `os datasource validate` reach exactly this route. An existing integration that presents a valid credential — a better-auth session or a `sys_api_key` — and does not hold `manage_platform_settings` was served before and is **refused now**: `403` with the standard catalog code `PERMISSION_DENIED` (ADR-0112), the message naming the missing capability so the caller knows which grant to request. The anonymous floor is unchanged: no identity is still `401 UNAUTHENTICATED`. + +**Why the read capability.** `validateAll` drives the same live remote-schema introspection the family's gated read routes expose (`introspect` per datasource), and its report — schema diffs naming remote columns and types, driver error strings for unreachable remotes — is a read of the same federation surface. An unentitled caller refused at `GET /:name/external/tables` could previously still trigger live remote introspection through this route and read what it found. One family, one door-type: reads on `manage_platform_settings`, writes on `manage_metadata`. + +**Migration.** Grant the calling credential's permission set `manage_platform_settings` — the same grant the family's read routes have required since #10254, so an integration already migrated for those is covered. The platform's `admin_full_access` set carries it; a purpose-built operator set is the case to check. diff --git a/packages/rest/src/external-datasource-envelope.conformance.test.ts b/packages/rest/src/external-datasource-envelope.conformance.test.ts index 3db0c09422..cc3736574d 100644 --- a/packages/rest/src/external-datasource-envelope.conformance.test.ts +++ b/packages/rest/src/external-datasource-envelope.conformance.test.ts @@ -51,9 +51,10 @@ interface Captured { * would read the 401 body instead of the arm it names, and this file would * silently stop measuring what it exists to measure. * - * [#9901] …and an ENTITLED one: four of the five routes now also require a - * capability (`manage_platform_settings` on the reads, `manage_metadata` on the - * writes), so this stub holds both. Same reasoning one step further — a + * [#9901/#10255] …and an ENTITLED one: every route now also requires a + * capability (`manage_platform_settings` on the reads — `validate` among them + * since the 2026-08-20 #10255 ruling — `manage_metadata` on the writes), so + * this stub holds both. Same reasoning one step further — a * resolver carrying an identity but no grants would turn every case below into * a reading of the 403 body. Holding both rather than one per case is * deliberate: which capability each route requires is not this file's subject, diff --git a/packages/rest/src/external-datasource-routes-auth-guard.test.ts b/packages/rest/src/external-datasource-routes-auth-guard.test.ts index 15c5a8eec1..24435da5b8 100644 --- a/packages/rest/src/external-datasource-routes-auth-guard.test.ts +++ b/packages/rest/src/external-datasource-routes-auth-guard.test.ts @@ -3,7 +3,7 @@ /** * [#9686] The `/api/v1/datasources/:name/external/*` federation family requires * an authenticated caller — on every route, read and write alike — and - * [#9901] a CAPABILITY above that on four of the five. + * [#9901/#10255] a CAPABILITY above that on every route. * * ## What this pins, and why it is driven through the real plugin * @@ -56,10 +56,16 @@ * here, which is what makes the read/write split falsifiable rather than * merely written down. * - * `POST /external/validate` is the one route the ruling does not name: it has - * no admin twin and creates no metadata, so it keeps the #9686 authentication - * floor. That is pinned too — an un-ruled route silently acquiring a - * neighbour's gate is a change nobody decided. + * [#10255] `POST /external/validate` was the one route the #9901 ruling did + * not name: no admin twin, no metadata created, so it kept the #9686 + * authentication floor — pinned here as an explicit `capability: null` row so + * that gating it later had to change the table. That later card is #10255, + * ruled 2026-08-20 (verbatim: 「同意你的意见。」, accepting option A): validate + * takes the READ capability, because `validateAll` drives the same live + * remote-schema introspection the read twins gate and reports on it. The row + * now carries `READ_CAPABILITY`, and its case below flips from "still served + * holding nothing" to "refused holding nothing" — deliberately, not by a + * neighbour's loop swallowing it. * * Both credential kinds the platform admits are exercised, because the cheap * mistake here is to read only a better-auth session: that would refuse a @@ -96,10 +102,12 @@ type Handler = (req: any, res: any) => any; * runtime-origin federated object, the refresh rewrites the cached catalog * snapshot. Both are asserted to be unreachable without an identity. * - * `capability: null` is `POST /external/validate`, the one route the ruling - * does not name. Spelled as an explicit `null` rather than omitted so that a - * later edit which gates it has to change this table — an absent field would - * let that happen silently. + * [#10255] There is no `capability: null` row any more: `POST + * /external/validate` carried one — spelled as an explicit `null` rather than + * omitted, so that a later edit gating it had to change this table — and the + * 2026-08-20 #10255 ruling is that later edit: validate is a read + * (`validateAll` drives the same live remote introspection the read twins + * gate), so its row now carries `READ_CAPABILITY` like its two read siblings. */ const READ_CAPABILITY = 'manage_platform_settings'; const WRITE_CAPABILITY = 'manage_metadata'; @@ -109,7 +117,7 @@ const FAMILY = [ { method: 'POST', url: `${BASE}/datasources/${DS}/external/tables/customers/draft`, ok: 200, call: 'generateObjectDraft', writes: false, capability: READ_CAPABILITY }, { method: 'POST', url: `${BASE}/datasources/${DS}/external/tables/customers/import`, ok: 201, call: 'importObject', writes: true, capability: WRITE_CAPABILITY }, { method: 'POST', url: `${BASE}/datasources/${DS}/external/refresh-catalog`, ok: 200, call: 'refreshCatalog', writes: true, capability: WRITE_CAPABILITY }, - { method: 'POST', url: `${BASE}/datasources/${DS}/external/validate`, ok: 200, call: 'validateAll', writes: false, capability: null }, + { method: 'POST', url: `${BASE}/datasources/${DS}/external/validate`, ok: 200, call: 'validateAll', writes: false, capability: READ_CAPABILITY }, ] as const; /** Every capability an entitled caller needs to clear all five routes. */ @@ -394,12 +402,15 @@ describe('[#9686] the same boot still serves an entitled caller', () => { }); describe('[#9901] the family requires a capability above authentication', () => { - it('refuses an authenticated caller holding NOTHING on all four ruled routes — 403 PERMISSION_DENIED, before the service', async () => { + it('refuses an authenticated caller holding NOTHING on all five routes — 403 PERMISSION_DENIED, before the service', async () => { + // [#10255] Five, not four: `POST /external/validate` joined the ruled set + // on 2026-08-20, so there is no `capability: null` row left to filter out + // and this loop runs the whole family. const { table, service, lookups } = await bootFederation({ withAuth: true, withEngine: true, grants: [], }); - for (const route of FAMILY.filter((r) => r.capability !== null)) { + for (const route of FAMILY) { const { statusCode, body } = await call(table, route, { authorization: `Bearer ${SESSION}` }); // Status AND code. "not 200" would be satisfied by the 401 the anonymous @@ -414,7 +425,7 @@ describe('[#9901] the family requires a capability above authentication', () => // The refusal precedes dispatch — so on the two routes that WRITE, nothing // was created before the caller was turned away. - for (const route of FAMILY.filter((r) => r.capability !== null)) { + for (const route of FAMILY) { expect( (service as any)[route.call], `${route.call} must not run for an unentitled caller`, @@ -459,17 +470,40 @@ describe('[#9901] the family requires a capability above authentication', () => } }); - it('POST /external/validate keeps the #9686 authentication floor — the ruling does not name it', async () => { - // The route the 2026-08-20 ruling enumerates NO capability for: no admin - // twin, no metadata created. An authenticated caller holding nothing is - // served here while being refused the other four on the same boot, which is - // the difference stated rather than implied. A later card may change this; - // it will have to change this case to do it. + it('[#10255] POST /external/validate requires the READ capability — the authentication-floor era is over', async () => { + // This case is the previous pin FLIPPED, deliberately. Until the + // 2026-08-20 #10255 ruling it asserted the exact opposite — an + // authenticated caller holding nothing was SERVED here while refused the + // other four — because #9901's ruling did not name this route. The ruling + // that changed it is recorded on #10255 (option A): `validateAll` drives + // the same live remote-schema introspection the read twins gate, so + // validate is a read and answers to the read capability. const { table, service } = await bootFederation({ withAuth: true, withEngine: true, grants: [], }); - const validate = FAMILY.find((r) => r.capability === null)!; + const validate = FAMILY.find((r) => r.call === 'validateAll')!; + const { statusCode, body } = await call(table, validate, { authorization: `Bearer ${SESSION}` }); + + // Refused with the capability NAMED — the one thing the refused caller can + // act on — and the service never ran, so an unentitled caller cannot + // trigger remote introspection as a side effect of being refused. + expect(statusCode).toBe(403); + expect(body?.success).toBe(false); + expect(body?.error?.code).toBe('PERMISSION_DENIED'); + expect(body?.error?.message).toContain(READ_CAPABILITY); + expect(service.validateAll).not.toHaveBeenCalled(); + }); + + it('[#10255] …and `manage_platform_settings` alone clears it, like its two read siblings', async () => { + // The success half of the flip, on its own boot: the capability that + // clears the read twins clears validate too — this is what "joined the + // reads" means, stated as a served request rather than a table row. + const { table, service } = await bootFederation({ + withAuth: true, withEngine: true, grants: [READ_CAPABILITY], + }); + + const validate = FAMILY.find((r) => r.call === 'validateAll')!; const { statusCode, body } = await call(table, validate, { authorization: `Bearer ${SESSION}` }); expect(statusCode).toBe(validate.ok); diff --git a/packages/rest/src/external-datasource-routes.ts b/packages/rest/src/external-datasource-routes.ts index 53b5bd245e..d42a98ec0c 100644 --- a/packages/rest/src/external-datasource-routes.ts +++ b/packages/rest/src/external-datasource-routes.ts @@ -110,11 +110,12 @@ export interface ExternalDatasourceRoutesOptions { /** * [#9901] The capability the federation family's READ routes require: - * `GET /external/tables` and `POST /external/tables/:remote/draft`. + * `GET /external/tables`, `POST /external/tables/:remote/draft` and + * [#10255] `POST /external/validate`. * - * It is `manage_platform_settings` because these two routes are the DECLARED - * TWINS of `GET /:name/remote-tables` and `POST /:name/object-draft` on the - * admin spelling, which measured exactly this capability in #9593 + * It is `manage_platform_settings` because the first two routes are the + * DECLARED TWINS of `GET /:name/remote-tables` and `POST /:name/object-draft` + * on the admin spelling, which measured exactly this capability in #9593 * (`DATASOURCE_ADMIN_CAPABILITY`). One operation reached through two mounted * routes cannot admit two different sets of callers — that asymmetry is what * this card closes, and `remote-tables-twin.equivalence.test.ts` is where the @@ -122,6 +123,17 @@ export interface ExternalDatasourceRoutesOptions { * * Maintainer ruling, 2026-08-20 (verbatim: 「其他接受你的建议。」): the * federation family is NOT deliberately the lower-privilege door. + * + * [#10255] `validate` has no admin twin to converge with, so #9901 left it on + * the #9686 authentication floor and filed the question instead of deciding + * it. The follow-up ruling (maintainer, 2026-08-20, verbatim: + * 「同意你的意见。」, accepting option A on #10255) converged it here: what + * `validateAll` does is drive the SAME live remote-schema introspection the + * two read twins gate (`introspect` per datasource, in + * `service-datasource/src/external-datasource-service.ts`), and its report — + * schema diffs naming remote columns and types, driver error strings for + * unreachable remotes — is a read of the same federation surface. One family, + * one door-type: reads here, writes on {@link FEDERATION_WRITE_CAPABILITY}. */ export const FEDERATION_READ_CAPABILITY = 'manage_platform_settings'; @@ -194,7 +206,7 @@ export function registerExternalDatasourceRoutes( * `503` which services a deployment has wired, and — for the two routes that * write — so the refusal provably precedes the write rather than following it. * - * ## [#9901] …and a CAPABILITY above it, on four of the five routes + * ## [#9901/#10255] …and a CAPABILITY above it, on every route * * #9686 left this family gated on authentication alone and pointed the * capability question at #9593, which answered it for the admin half only. @@ -240,19 +252,23 @@ export function registerExternalDatasourceRoutes( * — since `isSystem` is never resolved from inbound HTTP — one no wire caller * could ever take, so it would be unfalsifiable divergence from the twin. * - * ## `'authenticated'`: the one route the ruling does not name + * ## [#10255] `validate` joined the reads; the `'authenticated'` kind retired * - * `POST /external/validate` has no twin on the admin spelling, creates no - * metadata, and is NOT one of the four routes the 2026-08-20 ruling - * enumerates. It keeps the #9686 authentication floor and says so with its - * own kind rather than silently inheriting a neighbour's gate — an un-ruled - * route that shared a constant would read as ruled. Filed separately rather - * than decided here. + * #9901's ruling enumerated four routes, so `POST /external/validate` — no + * twin on the admin spelling, no metadata created — kept the #9686 + * authentication floor under its own explicit kind: an un-ruled route + * silently inheriting a neighbour's gate would have read as ruled. The + * question was filed as #10255 and ruled on 2026-08-20: validate takes the + * READ capability (see {@link FEDERATION_READ_CAPABILITY}'s note for why it + * is a read). With every route now ruled, the `'authenticated'` kind would + * be a door no route walks through, so it is REMOVED rather than kept — a + * spare lower gate is exactly what a future un-ruled route could silently + * adopt. */ const refuseFederationRequest = async ( req: any, res: any, - kind: 'read' | 'write' | 'authenticated', + kind: 'read' | 'write', ): Promise => { let authz: | { userId?: string | null; isSystem?: boolean; systemPermissions?: string[] } @@ -268,7 +284,6 @@ export function registerExternalDatasourceRoutes( sendError(res, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE); return true; } - if (kind === 'authenticated') return false; const required = kind === 'write' ? FEDERATION_WRITE_CAPABILITY : FEDERATION_READ_CAPABILITY; const held = Array.isArray(authz?.systemPermissions) ? authz.systemPermissions : []; if (!held.includes(required)) { @@ -421,13 +436,16 @@ export function registerExternalDatasourceRoutes( }, }, - // Validate the federated objects on this datasource. + // Validate the federated objects on this datasource. [#10255] A 'read': + // validateAll drives the same live remote-schema introspection the two + // read twins gate, so it answers to the same capability (ruled 2026-08-20; + // the constant's doc carries the reasoning). { method: 'POST', path: `${ext}/validate`, metadata: { summary: 'Validate the federated objects on a datasource', tags: ['datasources'] }, handler: async (req: any, res: any) => { - if (await refuseFederationRequest(req, res, 'authenticated')) return; + if (await refuseFederationRequest(req, res, 'read')) return; const svc = externalService(); if (!svc?.validateAll) return unavailable(res); try {