diff --git a/.changeset/27719-explain-empty-object.md b/.changeset/27719-explain-empty-object.md new file mode 100644 index 00000000000..c2147399ffe --- /dev/null +++ b/.changeset/27719-explain-empty-object.md @@ -0,0 +1,14 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest): `security/explain` refuses an empty `object` with `400 VALIDATION_FAILED` instead of a `500 EXPLAIN_FAILED` server fault (#27719) + +Clause-②: no — the request already failed; only its answer moves from `500` to `400`. +Metadata-protocol: none + +- **Before.** `POST /api/v1/security/explain` with `"object": ""` (or `GET …?object=`) passed the request schema, reached the explain engine, and the engine's plain error was answered `500 EXPLAIN_FAILED` and logged as a server fault. +- **Now.** The door answers `400 VALIDATION_FAILED` in its own envelope, naming the empty key and its remedy, before any evaluation — the same check, in the same place, that refuses an empty `userId`. +- **Unchanged.** A request naming an object is explained exactly as before; an empty `userId` is refused `400 VALIDATION_FAILED` as before; a missing `object` is still refused by the request schema. + +**The remedy.** Send the name of the object whose access to explain. diff --git a/packages/rest/src/security-routes.test.ts b/packages/rest/src/security-routes.test.ts index df2a89d0796..ced5b019d35 100644 --- a/packages/rest/src/security-routes.test.ts +++ b/packages/rest/src/security-routes.test.ts @@ -202,6 +202,57 @@ describe('[#27442] GET/POST /security/explain with an empty userId', () => { }); }); +// ── [#27719] an empty object is refused at the door, like an empty userId ──── + +describe('[#27719] GET/POST /security/explain with an empty object', () => { + /** The engine's own answer to an empty object: a plain Error, which the route's catch-all reads as a 500. */ + const engineLike = () => vi.fn(async (request: any) => { + if (!request.object) throw new Error('[Security] explain: request.object is required'); + return DECISION; + }); + + it('POST object "" is refused 400 VALIDATION_FAILED in the route envelope, and the engine is never called', async () => { + const explain = engineLike(); + const { post } = buildServer(async () => ({ explain }), { callerCtx: CALLER }); + const res = mockRes(); + await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: '', operation: 'read' } } 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 ?object= is refused the same way', async () => { + const explain = engineLike(); + const { get } = buildServer(async () => ({ explain }), { callerCtx: CALLER }); + const res = mockRes(); + await get!.handler({ method: 'GET', params: {}, headers: {}, query: { object: '', operation: 'read' } } as any, res); + + expect(res.statusCode).toBe(400); + expect(res.body.success).toBe(false); + expect(res.body.error.code).toBe('VALIDATION_FAILED'); + expect(explain).not.toHaveBeenCalled(); + }); + + it('a named object is still explained with exactly the request it was sent', async () => { + const explain = engineLike(); + const { get, post } = buildServer(async () => ({ explain }), { callerCtx: CALLER }); + + let res = mockRes(); + await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'update' } } as any, res); + expect(res.statusCode).toBe(200); + expect(res.body).toEqual(DECISION); + expect(explain).toHaveBeenLastCalledWith({ object: 'task', operation: 'update' }, 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); + }); +}); + // ── [#8326] batch form — recordIds on the same request shape ───────────────── /** diff --git a/packages/rest/src/security-routes.ts b/packages/rest/src/security-routes.ts index dddcada6564..73f00e095d3 100644 --- a/packages/rest/src/security-routes.ts +++ b/packages/rest/src/security-routes.ts @@ -100,6 +100,17 @@ export interface SecurityRouteHost { enforceAuth: RestServer['enforceAuth']; } +/** + * The explain request keys whose explicit empty string the door refuses + * `400 VALIDATION_FAILED` before any evaluation, each with the remedy its + * refusal names. The schema's `z.string()` admits "", so the line is drawn + * here; see the check in the handler for why each key is on it. + */ +const EMPTY_KEY_REMEDIES: ReadonlyArray = [ + ['object', 'Give the name of the object whose access to explain.'], + ['userId', 'Omit userId to explain your own access, or give the id of the user to explain.'], +]; + /** * Register the access-explanation endpoints — * `RestServer#registerSecurityExplainEndpoints`' body, called by that method @@ -204,16 +215,23 @@ 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.', - ); + // An explicit empty string names nothing, so these keys refuse it + // here: the schema's `z.string()` admits "", and `?key=` names the + // key on GET, so it is explicit there too. One check, one remedy + // per key, in the route's own envelope, before any evaluation. + // - [#27442] `userId`: an omitted one means the caller; an empty + // one was dropped and answered about the caller, a principal + // nobody asked about. + // - [#27719] `object`: an empty one reached the engine, whose + // plain `Error` fell to the `500 EXPLAIN_FAILED` arm below — a + // request the caller can fix, answered and logged as a fault. + for (const [key, remedy] of EMPTY_KEY_REMEDIES) { + if (src[key] === '') { + return respondError( + res, 400, 'VALIDATION_FAILED', + `Invalid explain request — ${key} is empty. ${remedy}`, + ); + } } const parsed = (ExplainRequestSchema as any).safeParse({ object: src.object,