security(service-datasource): require manage_platform_settings on the datasource-admin routes (#9887)

* security(service-datasource): require manage_platform_settings on the datasource-admin routes

The datasource-admin HTTP family took any authenticated caller once #9391 landed
its anonymous floor. Datasource create/patch/remove/introspect are
platform-configuration actions, so all eleven routes now also require the
capability the adjacent Setup-admin families gate on.

The capability is measured, not minted: service-settings' platform-infrastructure
namespaces (mail, storage, sms, auth, ai, knowledge) and objectql's lifecycle
namespace all declare manage_platform_settings for reads AND writes, while the
cohort that splits setup.access/setup.write is the tenant-cosmetic one. This
service's own Setup nav entry already declared requiredPermissions:
['manage_platform_settings'] on the console door in front of these routes, so
the change makes declared equal enforced.

Refusal is the standard-catalog 403 PERMISSION_DENIED through the shared
sendError, not the grandfathered FORBIDDEN synonym whose ADR-0112 waiver covers
three other packages and does not endorse the spelling for new code.

The both-sides-on-one-boot pin grows a third posture (entitled succeeds,
authenticated-but-unentitled is refused, anonymous stays refused), and the two
suites for which an entitled caller is the premise share its fixture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx

* test(rest): entitle the listRemoteTables twin fixture and pin the new capability divergence

The datasource-admin capability gate flips a public admission semantic, and the
twin-equivalence suite one package over was the consumer pin asserting the old
one: its fixture caller was authenticated but unentitled, so five request-shape
cases compared a 200 against the new 403 and read an admission difference as a
request-shape divergence.

The fixture now resolves grants through the platform's own RBAC chain — one fake
engine wired into BOTH spellings, the admin registrar's `objectql` lookup and the
resolveAuthzContext call behind the federation registrar's resolveExecutionContext,
so the twins still read one identity and now one grant aggregation.

The #9686 "WHO may ask" block keeps both refusal cases unchanged (anonymous and
unrecognised credential still refuse identically) and gains a case pinning what is
now true: an authenticated but unentitled caller is refused 403 PERMISSION_DENIED
at the admin spelling and served at the federation spelling. That asymmetry is
filed as #9901, not accepted; the case is labelled a record of a known gap and is
expected to fail when the gap closes.

No packages/rest runtime code is touched — gating the federation family is a
separate decision and a different lane.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-warren

.changeset/rare-donkeys-repeat.md

@@ -0,0 +1,23 @@
+---
+'@objectstack/service-datasource': patch
+---
+
+Datasource-admin HTTP routes now require the `manage_platform_settings` capability, not merely authentication.
+
+All eleven routes under `/api/v1/datasources` — list, read, driver catalog, remote-table
+introspection, connection probes, credential migration, create, patch and remove — answer
+`403 PERMISSION_DENIED` to a caller that resolves to an identity holding no
+`manage_platform_settings` grant. The anonymous floor is unchanged (`401 UNAUTHENTICATED`).
+
+The capability is matched to what the adjacent Setup-admin families already gate on, not
+minted: `@objectstack/service-settings`'s platform-infrastructure namespaces (`mail`,
+`storage`, `sms`, `auth`, `ai`, `knowledge`) declare it for reads and writes alike, and this
+service's own Setup nav entry already declared `requiredPermissions:
+['manage_platform_settings']` for the console door in front of these routes. There is no
+read/write split for the same reason those namespaces have none: a datasource read returns
+stored connection configuration and live remote-schema introspection.
+
+Impact: `admin_full_access` carries `manage_platform_settings`, so platform admins are
+unaffected. A deployment that granted non-admin users access to Setup → Datasources through
+some other capability must now grant `manage_platform_settings` (or bind those users to a
+permission set carrying it).

packages/rest/src/remote-tables-twin.equivalence.test.ts

@@ -94,15 +94,89 @@ const REMOTE: IntrospectedSchema = {
   },
 };
 
-/** The credential the admin spelling's authentication floor admits (#9391). */
+/**
+ * The credential every request-shape case presents: authenticated (#9391) AND
+ * holding `manage_platform_settings`, which the admin spelling requires as of
+ * #9593.
+ *
+ * The entitlement is not decoration. Before #9593 the admin spelling admitted
+ * any authenticated caller, so a bare session was enough to compare the two
+ * answers; now an unentitled session makes the admin spelling answer 403 and
+ * every case below would be comparing a 200 against a refusal — reading an
+ * ADMISSION difference as a request-shape divergence, the one thing this file
+ * exists not to confuse (the same reason #9391 made it wire `auth` at all).
+ */
 const SESSION = 'Bearer twin-session';
+
+/**
+ * [#9593] A second credential: authenticated, holding nothing. This is the
+ * posture on which the two spellings now genuinely DIVERGE, and the divergence
+ * gets its own pinned case at the bottom of this file rather than being
+ * papered over here.
+ */
+const UNENTITLED_SESSION = 'Bearer twin-session-unentitled';
+
+const USERS: Record<string, string> = {
+  [SESSION]: 'u_twin',
+  [UNENTITLED_SESSION]: 'u_twin_plain',
+};
+
 const authService = {
   api: {
-    getSession: async ({ headers }: { headers: Headers }) =>
-      headers?.get?.('authorization') === SESSION ? { user: { id: 'u_twin' } } : null,
+    getSession: async ({ headers }: { headers: Headers }) => {
+      const id = USERS[headers?.get?.('authorization') ?? ''];
+      return id ? { user: { id } } : null;
+    },
   },
 };
 
+/** The permission set carrying the grant `u_twin` holds and `u_twin_plain` does not. */
+const GRANT_SET_ID = 'ps_twin_datasource_operator';
+
+/**
+ * The RBAC tables `resolveAuthzContext` reads, as a fake data engine — the
+ * same idiom this package's other authz fixtures use
+ * (`rest-exec-ctx-principal-kind.test.ts`), and the same four-table shape the
+ * admin family's own pin builds in `@objectstack/service-datasource`.
+ *
+ * ONE engine serves BOTH spellings: it is wired into the plugin context the
+ * admin registrar resolves `objectql` from, and handed to the
+ * `resolveAuthzContext` call behind the federation registrar's
+ * `resolveExecutionContext`. That is deliberate and load-bearing — the two
+ * spellings must read one identity AND one grant aggregation, or this file
+ * could manufacture agreement (or disagreement) out of two different notions of
+ * who the caller is.
+ *
+ * The set is deliberately not `admin_full_access`: that platform set carries
+ * `manage_platform_settings` among six other capabilities, so a gate keyed on
+ * platform-admin posture rather than on the capability would pass unnoticed.
+ */
+const makeQl = () => ({
+  find: async (object: string, opts: any) => {
+    const where = opts?.where ?? {};
+    if (object === 'sys_user_permission_set') {
+      return where.user_id === 'u_twin'
+        ? [{ id: 'ups_twin', user_id: 'u_twin', permission_set_id: GRANT_SET_ID, organization_id: null }]
+        : [];
+    }
+    if (object === 'sys_permission_set') {
+      const ids: string[] = where.id?.$in ?? [];
+      return ids.includes(GRANT_SET_ID)
+        ? [{
+            id: GRANT_SET_ID,
+            name: 'twin_datasource_operator',
+            // JSON string — the spelling SQLite hands back, which the resolver
+            // parses. Pinning the stored shape keeps the fixture on the real
+            // read path.
+            system_permissions: JSON.stringify(['manage_platform_settings']),
+            object_permissions: '{}',
+          }]
+        : [];
+    }
+    return [];
+  },
+});
+
 /**
  * One server, one service, both registrars — the point of the fixture.
  *
@@ -119,6 +193,7 @@ function mountBoth() {
     listObjects: async () => [],
   });
   const server = new HonoHttpServer(0);
+  const ql = makeQl();
   const ctx = {
     getService: (name: string) => {
       if (name === 'external-datasource') return service;
@@ -128,6 +203,12 @@ function mountBoth() {
       // 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;
+      // [#9593] …and the admin spelling now also requires a CAPABILITY, which
+      // the same resolver aggregates off the data engine. Same reasoning one
+      // step further: without a grant to read, the comparison would be a 200
+      // against a 403. `objectql` and `data` are one registration under two
+      // names, and the registrar tries them in that order.
+      if (name === 'objectql' || name === 'data') return ql;
       throw new Error(`no service: ${name}`);
     },
   } as any;
@@ -153,15 +234,24 @@ function mountBoth() {
       }
     }
     const authz = await resolveAuthzContext({
-      // No data engine here, stated rather than omitted: `ql` is a required
-      // member, and it is what the api-key admission path reads. This fixture
-      // wires only a session, so that path resolves nothing and the session
-      // path is the one under comparison.
-      ql: undefined,
+      // [#9593] The SAME engine the admin spelling resolves `objectql` to,
+      // stated rather than omitted. It used to be `undefined` here, which was
+      // right while only a session mattered; now that one spelling reads
+      // GRANTS, handing this side a different (or absent) engine would let the
+      // two spellings disagree about the caller for a reason that is the
+      // fixture's, not the code's. One identity function, and now one grant
+      // aggregation.
+      ql,
       headers,
       getSession: async (h: any) => authService.api.getSession({ headers: h }),
     });
-    return authz.userId ? { userId: authz.userId } : undefined;
+    // `systemPermissions` is carried through even though no route in this
+    // package reads it today: the federation spelling gates on authentication
+    // only (see the divergence case at the bottom of this file), and the day
+    // that changes, this resolver already supplies what such a gate would read.
+    return authz.userId
+      ? { userId: authz.userId, systemPermissions: authz.systemPermissions }
+      : undefined;
   };
 
   registerExternalDatasourceRoutes(server, ctx, '/api/v1', { resolveExecutionContext });
@@ -294,8 +384,30 @@ describe('listRemoteTables twins agree on the request shape (#7955)', () => {
  * answers, exactly as the request-shape cases compare table sets. A guard added
  * to one spelling and not the other now fails here, whichever side it is added
  * to — which is the property the equivalence is for.
+ *
+ * ## [#9593] The axis is no longer a single line, and this block says so
+ *
+ * #9593 raised the ADMIN spelling from "any authenticated caller" to
+ * `manage_platform_settings`; the federation spelling still gates on
+ * authentication alone, by its own registrar's stated decision (#9686 ruled
+ * the capability question out of its scope and pointed it here). So the
+ * spellings now agree at the two ends of the axis and diverge in the middle:
+ *
+ *  - anonymous — both refuse `401 UNAUTHENTICATED` (unchanged);
+ *  - unrecognised credential — both refuse `401 UNAUTHENTICATED` (unchanged);
+ *  - authenticated AND entitled — both serve, identically (unchanged);
+ *  - authenticated but UNENTITLED — the admin spelling refuses `403`, the
+ *    federation spelling serves.
+ *
+ * The last row is a real governance asymmetry — one operation, two doors, one
+ * gate — and it is FILED, not accepted: #9901. It is pinned here rather than left unasserted for the reason this
+ * whole block exists: an axis nothing drives is an axis that goes silently
+ * false, which is exactly how the pre-#9686 gap survived. ⚠️ When the
+ * federation spelling grows its own capability gate, that case is EXPECTED to
+ * fail — it is a record of a known gap, not a defence of it, and the correct
+ * response is to fold the row back into the agreement above.
  */
-describe('listRemoteTables twins agree on WHO may ask (#9686)', () => {
+describe('listRemoteTables twins agree on WHO may ask (#9686, #9593)', () => {
   it('an anonymous caller is refused identically on both spellings', async () => {
     const { federation, admin } = await readBoth('', {});
 
@@ -319,13 +431,44 @@ describe('listRemoteTables twins agree on WHO may ask (#9686)', () => {
     expect(admin.code).toBe(federation.code);
   });
 
-  it('the credential that clears one spelling clears the other — same identity, same answer', async () => {
+  it('an ENTITLED credential clears both spellings — same identity, same answer', async () => {
     // The other direction, and the one that makes the refusal cases mean
     // something: the two spellings do not agree merely by refusing everyone.
+    // "Entitled" is now two facts (authenticated, and holding
+    // `manage_platform_settings`), and the default credential carries both.
     const { federation, admin } = await readBoth('?schema=public');
 
     expect(federation.status).toBe(200);
     expect(admin.status).toBe(200);
     expect(qualified(admin)).toEqual(qualified(federation));
   });
+
+  it('[#9593] an authenticated but UNENTITLED caller diverges: admin refuses 403, federation serves', async () => {
+    // ⚠️ A pinned RECORD OF A KNOWN GAP, not a contract worth keeping — see
+    // this block's header and the finding card #9901. The two
+    // halves are asserted separately and in full so that closing the gap
+    // fails this case loudly instead of drifting past it.
+    const { federation, admin } = await readBoth('', { authorization: UNENTITLED_SESSION });
+
+    // The admin spelling: the #9593 refusal, asserted by status AND
+    // machine-readable code (ADR-0112 envelope) — "not 200" would be satisfied
+    // by the 401 the anonymous case already covers, which would mean the
+    // credential was never read.
+    expect(admin.status).toBe(403);
+    expect(admin.code).toBe('PERMISSION_DENIED');
+    // …and it refused before serving anything.
+    expect(admin.tables).toEqual([]);
+
+    // The federation spelling: authentication was the whole gate here, so the
+    // same caller is served. This is the asymmetry, stated rather than implied.
+    expect(federation.status).toBe(200);
+    expect(qualified(federation)).toEqual([
+      'analytics.events',
+      'public.customers',
+      'public.orders',
+    ]);
+
+    // And it is genuinely a divergence — the point the equivalence axis makes.
+    expect(admin.status).not.toBe(federation.status);
+  });
 });