fix(rest)!: security/explain refuses an explicit empty userId with 400 instead of explaining the caller

.changeset/27442-explain-empty-user-id.md

@@ -0,0 +1,20 @@
+---
+'@objectstack/rest': patch
+'@objectstack/plugin-security': patch
+---
+
+fix(rest)!: `security/explain` refuses an explicit empty `userId` with 400 instead of explaining the caller (#27442)
+
+Clause-②: no (narrowing) — `GET`/`POST /api/v1/security/explain` refuses a request whose `userId` is the empty string, which it answered before. Nothing is widened.
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: the explain door refuses one request value, an empty userId, that it used to drop and answer as the caller. There is no authored or stored shape for a migration to rewrite and no tombstone. -->
+
+**BREAKING** accept-set narrowing at the access-explanation door (`@objectstack/rest`), held by the engine too (`@objectstack/plugin-security`).
+
+- **Before.** `POST /api/v1/security/explain` with `"userId": ""` (or `GET …?userId=`) dropped the empty id and answered `200` with an explanation of the CALLER. A client that lost the selected user (an empty picker) got a confident answer about a principal nobody asked about.
+- **Now.** The door answers `400 VALIDATION_FAILED`, in the same envelope as its other request refusals, and no explanation is computed. On `GET`, `?userId=` names the key, so it counts as explicit and is refused the same way.
+- **Unchanged.** An omitted `userId` still explains the caller; a `userId` naming another user is explained, refused `403`, or refused `404 RECORD_NOT_FOUND` exactly as before.
+- **Engine.** `explainAccessForCaller` no longer reads an empty `userId` as "the caller": it goes through the other-user arm, so a direct caller gets the `403` (no right to explain others) or the `404 RECORD_NOT_FOUND` an unknown id gets.
+
+**The remedy.** Omit `userId` to explain your own access, or send the id of the user to explain.

content/docs/permissions/explain.mdx

@@ -71,6 +71,8 @@ curl -X POST -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/jso
 
 `operation` is one of `read | create | update | delete | transfer | restore |
 purge | export` (defaults to `read`); omit `userId` to explain **yourself**.
+An empty `userId` (`"userId": ""`, or `?userId=` with no value) names nobody, so
+it is refused `400 VALIDATION_FAILED` rather than read as "yourself".
 
 `export` is the one operation whose answer you cannot infer from `read`. Since
 the [export axis](/docs/permissions/permission-sets#allowexport--the-export-axis)

packages/plugins/plugin-security/src/security-plugin-explain-access.ts

@@ -133,7 +133,10 @@ export async function explainAccessForCaller(
   const recordId = request?.recordId != null && request.recordId !== '' ? String(request.recordId) : undefined;
 
   let targetContext = callerContext ?? {};
-  if (request.userId && request.userId !== callerContext?.userId) {
+  // [#27442] Any `userId` that is not omitted, the empty string included, goes
+  // through the other-user arm: an empty id names nobody, so it is refused
+  // there like an unknown one, never folded into explaining the caller.
+  if (request.userId !== undefined && request.userId !== callerContext?.userId) {
     const callerIsSystem = callerContext?.isSystem === true;
     if (!callerIsSystem) {
       const callerSets = await this.resolvePermissionSetsForContext(callerContext).catch(() => []);

packages/plugins/plugin-security/src/security-plugin.test.ts

@@ -3891,6 +3891,34 @@ describe('explainAccessForCaller (ADR-0090 D6/D12)', () => {
     });
   });
 
+  // [#27442] An explicit empty `userId` names nobody, so it is never read as
+  // "explain me": it goes through the other-user arm, where it is refused like
+  // any id no `sys_user` row has. The REST door refuses it 400 before this; the
+  // engine holds the same line for a caller that reaches it directly.
+  describe('[#27442] an explicit empty userId', () => {
+    it('is never answered as the caller: manage_users gets 404, a plain member 403', async () => {
+      const { plugin } = await boot();
+      const hr = { userId: 'u_hr', positions: [], permissions: ['hr_admin'] };
+      const admin: any = await plugin
+        .explainAccessForCaller({ object: 'task', operation: 'read', userId: '' }, hr)
+        .then((d) => d, (e) => e);
+      expect(admin, 'refused, not an explanation of the caller').toMatchObject({
+        name: 'ExplainUserNotFoundError', code: 'RECORD_NOT_FOUND', status: 404, userId: '',
+      });
+      const plain = { userId: 'u_east_1', positions: [], permissions: [] };
+      await expect(
+        plugin.explainAccessForCaller({ object: 'task', operation: 'read', userId: '' }, plain),
+      ).rejects.toMatchObject({ name: 'PermissionDeniedError', code: 'PERMISSION_DENIED' });
+    });
+
+    it('an omitted userId still explains the caller', async () => {
+      const { plugin } = await boot();
+      const plain = { userId: 'u_east_1', positions: [], permissions: [] };
+      const self = await plugin.explainAccessForCaller({ object: 'task', operation: 'read' }, plain);
+      expect(self.principal.userId).toBe('u_east_1');
+    });
+  });
+
   // [#20515] Explaining ANOTHER user resolves that user's grants in the CALLER's
   // organization — the one the explain right was checked in. A grant scoped to
   // an organization applies only while that organization is the one resolved

packages/qa/dogfood/test/explain-unknown-user.dogfood.test.ts

@@ -23,6 +23,9 @@
  *    `PERMISSION_DENIED` it gets naming a user who exists.
  *  - `[unknown]` the seed admin naming the unknown id → 404 `RECORD_NOT_FOUND`
  *    naming it, for `read` and `create`, consistent with the data door's 404.
+ *  - `[empty]` (#27442) an explicit empty `userId`, in the body or as
+ *    `?userId=`, is refused 400 `VALIDATION_FAILED`: it names nobody, so it is
+ *    never answered as the caller.
  *  - `[existing]` the seed admin explaining the member who exists is answered
  *    as before (200, the principal is that member), and explaining yourself is
  *    unchanged.
@@ -108,6 +111,24 @@ describe('#26641: security explain refuses a userId no sys_user row has', () =>
     }
   });
 
+  // [#27442] The same door's remaining gap: an explicit empty `userId` names
+  // nobody, and was dropped before the schema, so it explained the CALLER. It
+  // is refused 400 on both transports; an omitted `userId` (below) is the caller.
+  it('[empty] an explicit empty userId is refused 400 VALIDATION_FAILED, never answered as the caller', async () => {
+    const posted = await explain(adminTok, { object: OBJECT, operation: 'read', userId: '' });
+    const queried = await stack.apiAs(adminTok, 'GET', `/security/explain?object=${OBJECT}&operation=read&userId=`);
+    const queriedText = await queried.text();
+    let queriedJson: any = null;
+    try { queriedJson = JSON.parse(queriedText); } catch { /* the status still reports */ }
+    for (const [label, r] of [['POST', posted], ['GET', { status: queried.status, text: queriedText, json: queriedJson }]] as const) {
+      expect(r.status, `${label}: ${r.text}`).toBe(400);
+      expect(r.json?.success, `${label}: ${r.text}`).toBe(false);
+      expect(r.json?.error?.code, `${label}: ${r.text}`).toBe('VALIDATION_FAILED');
+      expect(r.json?.allowed, `${label}: no verdict is answered`).toBeUndefined();
+      expect(r.json?.principal, `${label}: no principal is answered`).toBeUndefined();
+    }
+  });
+
   it('[existing] the seed admin explaining the member who exists, and anyone explaining themselves, is answered', async () => {
     const other = await explain(adminTok, { object: OBJECT, operation: 'read', userId: memberId });
     expect(other.status, other.text).toBe(200);

packages/rest/src/security-routes.test.ts

@@ -154,6 +154,54 @@ describe('GET/POST /security/explain (ADR-0090 D6)', () => {
   });
 });
 
+// ── [#27442] an explicit empty userId is refused, an omitted one is the caller ─
+
+describe('[#27442] GET/POST /security/explain with an empty userId', () => {
+  it('POST userId "" is refused 400 VALIDATION_FAILED and nothing is explained', async () => {
+    const explain = vi.fn().mockResolvedValue(DECISION);
+    const { post } = buildServer(async () => ({ explain }), { callerCtx: CALLER });
+    const res = mockRes();
+    await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'read', userId: '' } } as any, res);
+
+    expect(res.statusCode).toBe(400);
+    expect(res.body.success).toBe(false);
+    expect(res.body.error.code).toBe('VALIDATION_FAILED');
+    expect(res.body.allowed, 'no verdict is answered').toBeUndefined();
+    expect(explain).not.toHaveBeenCalled();
+  });
+
+  it('GET ?userId= names the key too, so it is refused the same way', async () => {
+    const explain = vi.fn().mockResolvedValue(DECISION);
+    const { get } = buildServer(async () => ({ explain }), { callerCtx: CALLER });
+    const res = mockRes();
+    await get!.handler({ method: 'GET', params: {}, headers: {}, query: { object: 'task', userId: '' } } as any, res);
+
+    expect(res.statusCode).toBe(400);
+    expect(res.body.error.code).toBe('VALIDATION_FAILED');
+    expect(explain).not.toHaveBeenCalled();
+  });
+
+  it('an omitted userId still explains the caller, and a named user is still passed through', async () => {
+    const explain = vi.fn().mockResolvedValue(DECISION);
+    const { get, post } = buildServer(async () => ({ explain }), { callerCtx: CALLER });
+
+    let res = mockRes();
+    await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'read' } } as any, res);
+    expect(res.statusCode).toBe(200);
+    expect(explain).toHaveBeenLastCalledWith({ object: 'task', operation: 'read' }, CALLER);
+
+    res = mockRes();
+    await get!.handler({ method: 'GET', params: {}, headers: {}, query: { object: 'task' } } as any, res);
+    expect(res.statusCode).toBe(200);
+    expect(explain).toHaveBeenLastCalledWith({ object: 'task', operation: 'read' }, CALLER);
+
+    res = mockRes();
+    await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'read', userId: 'u_target' } } as any, res);
+    expect(res.statusCode).toBe(200);
+    expect(explain).toHaveBeenLastCalledWith({ object: 'task', operation: 'read', userId: 'u_target' }, CALLER);
+  });
+});
+
 // ── [#8326] batch form — recordIds on the same request shape ─────────────────
 
 /**

packages/rest/src/security-routes.ts

@@ -204,10 +204,21 @@ export function registerSecurityExplainEndpoints(this: SecurityExplainRouteHost,
             const rawRecordIds = req.method === 'GET' && typeof src.recordIds === 'string' && src.recordIds !== ''
                 ? [src.recordIds]
                 : src.recordIds;
+            // [#27442] An omitted `userId` means the caller; an explicit empty
+            // one names nobody, so it is refused rather than dropped (dropping
+            // it answered about the caller, a principal nobody asked about).
+            // `?userId=` names the key on GET, so it is explicit there too.
+            // The schema's `z.string()` admits "", so the refusal is here.
+            if (src.userId === '') {
+                return respondError(
+                    res, 400, 'VALIDATION_FAILED',
+                    'Invalid explain request — userId is empty. Omit userId to explain your own access, or give the id of the user to explain.',
+                );
+            }
             const parsed = (ExplainRequestSchema as any).safeParse({
                 object: src.object,
                 operation: src.operation ?? 'read',
-                ...(src.userId != null && src.userId !== '' ? { userId: src.userId } : {}),
+                ...(src.userId != null ? { userId: src.userId } : {}),
                 // [C2 / ADR-0095] Optional record id — explains ONE concrete
                 // row at record granularity; omitted stays object-level.
                 ...(src.recordId != null && src.recordId !== '' ? { recordId: src.recordId } : {}),