From c964242a35eff15de1a612f1f6e95aaa76b39c2d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 18:40:58 +0000 Subject: [PATCH] fix(rest): converge the /security/explain pair onto the ADR-0112 D5 envelope (#8073) registerSecurityExplainEndpoints answered two retired dialects across its eight refusal arms: 401/501/400/403 were flat { code, message } and the two 500s were { code, error: 'a bare string' }, so body.error.code -- the one position ADR-0112 D5 declares -- read undefined on all six. The immediately adjacent registrar (#7981, PR #8071) already answered the declared shape, so a client calling explain then suggested-bindings met two envelopes inside one security family. Every arm now emits through one family-local emitter that delegates to the SHARED sendError from @objectstack/types (aliased sendEnvelopeError, as #8135 did, because this module has a local sendError of its own). No status code moves and no code value changes; the 400 arm's Zod dump moves from a top-level `detail` sibling to `error.details`, the slot ApiErrorSchema declares. The three flat-shape pins in security-routes.test.ts are MIGRATED to the D5 position, not deleted, and a driven envelope suite covers all eight arms plus a derived cross-arm shape pin. Ratchet banked: siblingCode 75 -> 73. Measured at merge-base (6ceffe0ac) 75 and at branch head 73; the two vanished sites are merge-base lines 9332/9390, both inside this function, and every surviving site maps 1:1 by the edit's line shift. stringError is unmoved at 44 -- both 500 arms carried a computed message that counter cannot see. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V --- .../security-explain-envelope-convergence.md | 33 ++ packages/rest/src/rest-server.ts | 95 ++++-- .../src/security-explain-envelope.test.ts | 315 ++++++++++++++++++ packages/rest/src/security-routes.test.ts | 18 +- scripts/check-route-envelope.mjs | 13 +- 5 files changed, 444 insertions(+), 30 deletions(-) create mode 100644 .changeset/security-explain-envelope-convergence.md create mode 100644 packages/rest/src/security-explain-envelope.test.ts diff --git a/.changeset/security-explain-envelope-convergence.md b/.changeset/security-explain-envelope-convergence.md new file mode 100644 index 0000000000..3e0daee5bd --- /dev/null +++ b/.changeset/security-explain-envelope-convergence.md @@ -0,0 +1,33 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): one error envelope across the `/security/explain` pair (#8073) + +`registerSecurityExplainEndpoints` — `GET/POST /api/v1/security/explain` and +`GET /api/v1/security/my-delegable-scope` — answered two retired dialects across +its eight refusal arms: the 401 / 501 / 400 / 403 arms were flat +`{ code, message }`, and the two 500s were `{ code, error: 'a bare string' }`. So +`body.error.code`, the one position ADR-0112 D5 declares, read `undefined` on all +six — while the immediately adjacent registrar (`/security/suggested-bindings`, +converged in #7981) already answered the declared shape. A client calling +`explain` and then `suggested-bindings` met two envelopes inside one `security` +family. + +Every arm now emits `{ success: false, error: { code, message } }` through the +shared `sendError` from `@objectstack/types` — the same builder every conformant +route module writes through — so the family agrees by construction rather than by +eight literals happening to match. The 400 arm's Zod-issue dump moves from a +top-level `detail` sibling to `error.details`, the slot `ApiErrorSchema` declares +for structured context. + +No status code moves, and no code VALUE changes: `UNAUTHORIZED`, +`NOT_IMPLEMENTED`, `VALIDATION_FAILED`, `PERMISSION_DENIED`, `EXPLAIN_FAILED` and +`DELEGABLE_SCOPE_FAILED` are all already registered, so nothing in +`packages/spec` moves. `ObjectStackClient` reads both envelopes' declared spots +(`errorBody?.code ?? errorBody?.error?.code`, and a bare-string limb for the +message), so `client.security.explain()` and +`client.security.describeDelegableScope()` keep throwing identical `err.code` and +`err.message` — re-measured against these call paths rather than inherited from +#7981. `err.details` does change on refusals, from "the whole response body" to +the structured slot or the new body. diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 883d7ab2d7..17fda035aa 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -56,6 +56,11 @@ import type { DirectMountedRoute, MountedRouteSource } from './direct-mount.js'; import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api'; import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api'; import type { FieldErrorCode } from '@objectstack/spec/api'; +// [#8073] The closed ADR-0112 error vocabulary, so the explain family's single +// refusal emitter types its `code` parameter as the vocabulary rather than as +// `string` — an invented code is a compile error at the call site instead of a +// runtime surprise on whichever arm a test happens to drive. +import type { ErrorCode } from '@objectstack/spec/api'; // The async-import row ceiling has exactly one definition, in the spec, whose // TSDoc is its public statement (#6535). rest is the only enforcer, so it reads // that export rather than re-declaring the literal beside a "mirrors spec" comment. @@ -9268,6 +9273,45 @@ export class RestServer { catch { return undefined; } }; + /** + * [#8073] The ONE refusal emitter for this route family — every arm of + * both handlers goes through it, so "explain and my-delegable-scope + * answer the same shape" is a property of the code rather than of + * eight literals that happen to agree. + * + * Before this, the family carried BOTH dialects ADR-0112 D5 retires: + * the 401/501/400/403 arms were flat `{ code, message }` and the two + * 500s were `{ code, error: 'a bare string' }`, so `body.error.code` — + * the one position D5 declares — read `undefined` on all six. #7035 + * (PR #7293) had already removed both from this file's `/meta` + * refusals and #7981 (PR #8071) from `registerSecurityEndpoints`, the + * immediately ADJACENT registrar: a client calling `explain` and then + * `suggested-bindings` met two shapes inside one `security` family. + * + * Emitted through the SHARED builder (`sendError` from + * `@objectstack/types`, imported as `sendEnvelopeError` because this + * module has a local `sendError` of its own — the sanitizing responder + * for THROWN errors, a different thing). That is what makes this the + * reference shape by construction rather than a ninth local literal + * agreeing with the eight it replaced, and it types `code` to the + * closed vocabulary for free. + * + * ⛔ Status codes are untouched: only the POSITION of `code` and + * `message` moves. `detail` — the 400 arm's Zod-issue dump — moves to + * `error.details`, the slot `ApiErrorSchema` actually declares for + * structured context; as a top-level sibling it was undeclared. + */ + const respondError = ( + res: any, + status: number, + code: ErrorCode, + message: string, + details?: unknown, + ): void => sendEnvelopeError( + res, status, code, message, + details === undefined ? undefined : { details }, + ); + const handler = async (req: any, res: any) => { try { const environmentId = isScoped ? req.params?.environmentId : undefined; @@ -9276,18 +9320,18 @@ export class RestServer { if (!context?.userId) { // The explain surface stays authenticated-only — it is an // admin diagnosis tool. (Anonymous is already 401ed above.) - return res.status(401).json({ - code: 'UNAUTHORIZED', - message: 'The access-explanation endpoint requires an authenticated caller.', - }); + return respondError( + res, 401, 'UNAUTHORIZED', + 'The access-explanation endpoint requires an authenticated caller.', + ); } const svc = await resolveService(environmentId, req); if (!svc || typeof svc.explain !== 'function') { - return res.status(501).json({ - code: 'NOT_IMPLEMENTED', - message: 'Access explanation is not available on this deployment (no security service with explain).', - }); + return respondError( + res, 501, 'NOT_IMPLEMENTED', + 'Access explanation is not available on this deployment (no security service with explain).', + ); } // GET reads the request from the query string, POST from the @@ -9310,11 +9354,11 @@ export class RestServer { ...(src.recordId != null && src.recordId !== '' ? { recordId: src.recordId } : {}), }); if (!parsed.success) { - return res.status(400).json({ - code: 'VALIDATION_FAILED', - message: 'Invalid explain request — expected { object: string, operation: read|create|update|delete|transfer|restore|purge, userId?: string, recordId?: string }.', - detail: String(parsed.error?.message ?? '').slice(0, 1000), - }); + return respondError( + res, 400, 'VALIDATION_FAILED', + 'Invalid explain request — expected { object: string, operation: read|create|update|delete|transfer|restore|purge, userId?: string, recordId?: string }.', + String(parsed.error?.message ?? '').slice(0, 1000), + ); } const decision = await svc.explain(parsed.data, context); @@ -9326,10 +9370,13 @@ export class RestServer { error?.name === 'PermissionDeniedError' || msg.startsWith('[Security] Access denied') ) { - return res.status(403).json({ code: 'PERMISSION_DENIED', message: msg.slice(0, 1000) }); + return respondError(res, 403, 'PERMISSION_DENIED', msg.slice(0, 1000)); } logError('[REST] Security explain error:', error); - res.status(500).json({ code: 'EXPLAIN_FAILED', error: msg.slice(0, 500) }); + // The 500 arm keeps its 500-char cap: an unexpected fault's + // message is not a contract, and truncating it stays a + // sanitization step — only the position of the words moves. + respondError(res, 500, 'EXPLAIN_FAILED', msg.slice(0, 500)); } }; @@ -9369,25 +9416,25 @@ export class RestServer { const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; if (!context?.userId) { - return res.status(401).json({ - code: 'UNAUTHORIZED', - message: 'The delegable-scope endpoint requires an authenticated caller.', - }); + return respondError( + res, 401, 'UNAUTHORIZED', + 'The delegable-scope endpoint requires an authenticated caller.', + ); } const svc = await resolveService(environmentId, req); if (!svc || typeof svc.describeDelegableScope !== 'function') { - return res.status(501).json({ - code: 'NOT_IMPLEMENTED', - message: 'Delegated administration is not available on this deployment (no security service with describeDelegableScope).', - }); + return respondError( + res, 501, 'NOT_IMPLEMENTED', + 'Delegated administration is not available on this deployment (no security service with describeDelegableScope).', + ); } res.json(await svc.describeDelegableScope(context)); } catch (error: any) { const msg = String(error?.message ?? error ?? ''); logError('[REST] Delegable scope error:', error); - res.status(500).json({ code: 'DELEGABLE_SCOPE_FAILED', error: msg.slice(0, 500) }); + respondError(res, 500, 'DELEGABLE_SCOPE_FAILED', msg.slice(0, 500)); } }; diff --git a/packages/rest/src/security-explain-envelope.test.ts b/packages/rest/src/security-explain-envelope.test.ts new file mode 100644 index 0000000000..f1a323e232 --- /dev/null +++ b/packages/rest/src/security-explain-envelope.test.ts @@ -0,0 +1,315 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8073] ONE error envelope across the `/security/explain` pair (ADR-0112 D5). + * + * ## What was wrong + * + * `registerSecurityExplainEndpoints` — `GET/POST /security/explain` and + * `GET /security/my-delegable-scope` — answered TWO retired dialects across its + * eight refusal arms, both of which #7035 (PR #7293) had already removed from + * this file's `/meta` refusals and #7981 (PR #8071) from + * `registerSecurityEndpoints`, the immediately ADJACENT registrar: + * + * | arms | shape | + * | :------------------------------------------ | :------------------------------- | + * | 401 / 501 / 400 / 403 | `{ code, message }` — flat | + * | 500 `EXPLAIN_FAILED`, `DELEGABLE_SCOPE_FAILED` | `{ code, error: '' }` — bare string | + * + * So `body.error.code` — the one position ADR-0112 D5 declares — read + * `undefined` on all six, and a client calling `explain` and then + * `suggested-bindings` met two shapes inside one `security` family. + * + * ## What these cases assert, and why not `toThrow` + * + * These handlers *send*; they never throw. A `rejects.toThrow()`-shaped + * assertion would report "the promise resolved" and could not separate + * "refused with the wrong envelope" from "did not refuse at all" — and the + * wrong envelope IS the defect. So every case asserts the ADR-0112 **pair**, + * HTTP `status` AND nested `body.error.code`, plus both retired dialects' + * absence: no top-level `code` sibling, and `error` an object rather than a + * bare string. + * + * ## The cross-arm pin is DERIVED + * + * Eight hand-written literal expectations that agree today is how this defect + * started — each arm was individually defensible and nobody compared them. So + * `shapeOf()` reduces a body to its structural skeleton (key paths + value + * types) and the family case asserts every arm reduces to the SAME skeleton + * without naming what that skeleton is. A third dialect added to any arm fails + * there even if someone also adds a matching literal case. + * + * ## What does NOT move + * + * No status code, and no code VALUE: `UNAUTHORIZED`, `NOT_IMPLEMENTED`, + * `VALIDATION_FAILED`, `PERMISSION_DENIED`, `EXPLAIN_FAILED` and + * `DELEGABLE_SCOPE_FAILED` are all already registered (`StandardErrorCode` for + * the first two of those, `ERROR_CODE_LEDGER`'s `@objectstack/rest` block for + * the rest), so nothing in `packages/spec` moves for this. Only the POSITION + * changes — plus the 400 arm's `detail`, which lands in `error.details`, the + * slot `ApiErrorSchema` declares for structured context; as a top-level sibling + * it was undeclared. + * + * The 401 the ANONYMOUS caller gets is a different seam — `enforceAuth`'s + * shared `ANONYMOUS_DENY_BODY` (#2567) fires before these arms and is pinned in + * `security-routes.test.ts`. This registrar's own 401 is the authenticated-but- + * no-`userId` posture, which is what `SYSTEM_NO_USER` below reaches. + */ + +import { describe, it, expect, vi } from 'vitest'; +// `.js` on purpose — NodeNext resolution requires the extension, and this +// package's TEST_DEBT ceiling has no margin for another TS2835 (#7248). +import { RestServer } from './rest-server.js'; + +const EXPLAIN = '/api/v1/security/explain'; +const DELEGABLE = '/api/v1/security/my-delegable-scope'; + +function mockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), + use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + }; +} + +function mockRes() { + const res: any = { + statusCode: 200, + json: vi.fn(function (this: any, body: any) { this._body = body; return this; }), + send: vi.fn(function (this: any) { return this; }), + setHeader: vi.fn(function (this: any) { return this; }), + status: vi.fn(function (this: any, code: number) { this.statusCode = code; return this; }), + header: vi.fn(function (this: any) { return this; }), + }; + return res; +} + +/** An authenticated caller — clears `enforceAuth` and this registrar's own 401. */ +const CALLER = { userId: 'u_admin', positions: ['everyone'], systemPermissions: ['manage_users'] }; + +/** + * Authenticated enough for the SHARED anonymous-deny seam (`isSystem` short- + * circuits `shouldDenyAnonymous`) but carrying no `userId`, which is the only + * posture that reaches this registrar's OWN `401 UNAUTHORIZED` arm. + */ +const SYSTEM_NO_USER = { isSystem: true }; + +/** What a healthy `explain` answers, so "explained" ≠ "refused". */ +const DECISION = { + allowed: true, + object: 'task', + operation: 'read', + principal: { userId: 'u_admin', positions: ['everyone'], permissionSets: ['member_default'] }, + layers: [], + readFilter: null, +}; + +type Answer = { status: number; body: any }; + +/** + * @param service what `securityServiceProvider` resolves to. `undefined` leaves + * the provider unset — one half of the 501 arm. An object + * MISSING the method is the other half (the route's own + * duck-type check), which is how both routes reach 501 while + * still being driven normally. + */ +function boot(service?: any, context: any = CALLER) { + const rest = new RestServer( + mockServer() as any, + { getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: {} }) } as any, + { api: { requireAuth: false } } as any, + ); + (rest as any).resolveExecCtx = async () => context; + if (service !== undefined) (rest as any).securityServiceProvider = async () => service; + rest.registerRoutes(); + + const route = (method: string, path: string) => { + const found = (rest as any).getRoutes().find( + (r: any) => r.method === method && r.path === path, + ); + if (!found) throw new Error(`route not registered: ${method} ${path}`); + return found; + }; + + const drive = async ( + method: string, + path: string, + req: Record = {}, + ): Promise => { + const res = mockRes(); + await route(method, path).handler( + { method, path, params: {}, query: {}, headers: {}, body: {}, ...req } as any, + res, + ); + return { status: res.statusCode, body: res.json.mock.calls.at(-1)?.[0] }; + }; + + return { + explainPost: (body: Record = { object: 'task', operation: 'read' }) => + drive('POST', EXPLAIN, { body }), + explainGet: (query: Record = { object: 'task' }) => + drive('GET', EXPLAIN, { query }), + delegable: () => drive('GET', DELEGABLE), + }; +} + +/** A service whose `explain` rejects with `err`. */ +function throwingExplain(err: unknown) { + return { explain: vi.fn().mockRejectedValue(err), describeDelegableScope: vi.fn() }; +} + +/** + * The full ADR-0112 assertion for one refusal: the PAIR (status + code) at the + * nested position, and both retired dialects absent. + */ +function expectNestedEnvelope(answer: Answer, status: number, code: string) { + expect( + answer.status, + `expected ${status}, got ${answer.status} with body ${JSON.stringify(answer.body)}`, + ).toBe(status); + // The pair ADR-0112 D5 declares — nested, because the flat position is the defect. + expect(answer.body?.error?.code).toBe(code); + expect(typeof answer.body?.error?.message).toBe('string'); + // Dialect 1 retired: `code` as a sibling of `error` (all eight arms had it). + expect(answer.body).not.toHaveProperty('code'); + // Dialect 2 retired: `error` as a bare string (the two 500s), which is what + // made `error.code` and `error.message` both read `undefined`. + expect(typeof answer.body?.error).toBe('object'); +} + +/** + * A body reduced to its STRUCTURE: every leaf key path with the type of its + * value, sorted. Values are dropped on purpose — arms legitimately differ in + * code and message, and the claim under test is that they agree in shape. + */ +function shapeOf(body: unknown): string { + const walk = (node: unknown, prefix: string): string[] => { + if (node === null || typeof node !== 'object' || Array.isArray(node)) { + return [`${prefix}:${Array.isArray(node) ? 'array' : node === null ? 'null' : typeof node}`]; + } + return Object.entries(node as Record) + .flatMap(([k, v]) => walk(v, prefix ? `${prefix}.${k}` : k)); + }; + return walk(body, '').sort().join('|'); +} + +// ───────────────────────────────────────────────────────────────────────────── +// 1. Each arm, on its own terms — status AND nested code, per ADR-0112 +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#8073] /security/explain — every refusal arm answers the ADR-0112 D5 envelope', () => { + it('401 UNAUTHORIZED — authenticated transport, no resolved userId', async () => { + const api = boot({ explain: vi.fn() }, SYSTEM_NO_USER); + expectNestedEnvelope(await api.explainPost(), 401, 'UNAUTHORIZED'); + }); + + it('501 NOT_IMPLEMENTED — no security service provider at all', async () => { + expectNestedEnvelope(await boot(undefined).explainPost(), 501, 'NOT_IMPLEMENTED'); + }); + + it('501 NOT_IMPLEMENTED — a service that does not expose explain', async () => { + const api = boot({ getReadFilter: vi.fn() }); + expectNestedEnvelope(await api.explainPost(), 501, 'NOT_IMPLEMENTED'); + }); + + it('400 VALIDATION_FAILED — the request fails ExplainRequestSchema', async () => { + const api = boot({ explain: vi.fn() }); + expectNestedEnvelope(await api.explainPost({ operation: 'read' }), 400, 'VALIDATION_FAILED'); + expectNestedEnvelope( + await api.explainPost({ object: 'task', operation: 'frobnicate' }), + 400, 'VALIDATION_FAILED', + ); + }); + + it("403 PERMISSION_DENIED — the service's D12 gate refuses", async () => { + const denial = Object.assign( + new Error("[Security] Access denied: explaining another user's access requires 'manage_users'."), + { code: 'PERMISSION_DENIED', name: 'PermissionDeniedError' }, + ); + const api = boot(throwingExplain(denial)); + expectNestedEnvelope(await api.explainPost(), 403, 'PERMISSION_DENIED'); + }); + + it('500 EXPLAIN_FAILED — an unexpected service fault', async () => { + const api = boot(throwingExplain(new Error('boom'))); + const answer = await api.explainPost(); + expectNestedEnvelope(answer, 500, 'EXPLAIN_FAILED'); + // The bare-string dialect put this text at `body.error`; it is the + // declared `message` now, and the 500-char sanitization cap is kept. + expect(answer.body.error.message).toBe('boom'); + }); + + it('GET and POST refuse identically — one contract, two transports', async () => { + const api = boot(undefined); + const [get, post] = [await api.explainGet(), await api.explainPost()]; + expectNestedEnvelope(get, 501, 'NOT_IMPLEMENTED'); + expectNestedEnvelope(post, 501, 'NOT_IMPLEMENTED'); + expect(shapeOf(get.body)).toBe(shapeOf(post.body)); + }); +}); + +describe('[#8073] /security/my-delegable-scope — same envelope, same emitter', () => { + it('401 UNAUTHORIZED — authenticated transport, no resolved userId', async () => { + const api = boot({ describeDelegableScope: vi.fn() }, SYSTEM_NO_USER); + expectNestedEnvelope(await api.delegable(), 401, 'UNAUTHORIZED'); + }); + + it('501 NOT_IMPLEMENTED — no service, or one without describeDelegableScope', async () => { + expectNestedEnvelope(await boot(undefined).delegable(), 501, 'NOT_IMPLEMENTED'); + expectNestedEnvelope(await boot({ explain: vi.fn() }).delegable(), 501, 'NOT_IMPLEMENTED'); + }); + + it('500 DELEGABLE_SCOPE_FAILED — an unexpected service fault', async () => { + const api = boot({ describeDelegableScope: vi.fn().mockRejectedValue(new Error('kaboom')) }); + const answer = await api.delegable(); + expectNestedEnvelope(answer, 500, 'DELEGABLE_SCOPE_FAILED'); + expect(answer.body.error.message).toBe('kaboom'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// 2. The cross-arm claim, derived rather than restated +// ───────────────────────────────────────────────────────────────────────────── + +describe('[#8073] the whole explain family reduces to ONE skeleton', () => { + it('every refusal arm of both routes has the same structural shape', async () => { + const denial = Object.assign(new Error('denied'), { code: 'PERMISSION_DENIED' }); + const arms: Array<[string, Answer]> = [ + ['explain 401', await boot({ explain: vi.fn() }, SYSTEM_NO_USER).explainPost()], + ['explain 501', await boot(undefined).explainPost()], + ['explain 403', await boot(throwingExplain(denial)).explainPost()], + ['explain 500', await boot(throwingExplain(new Error('boom'))).explainPost()], + ['delegable 401', await boot({ describeDelegableScope: vi.fn() }, SYSTEM_NO_USER).delegable()], + ['delegable 501', await boot(undefined).delegable()], + [ + 'delegable 500', + await boot({ describeDelegableScope: vi.fn().mockRejectedValue(new Error('x')) }).delegable(), + ], + ]; + const [, reference] = arms[0]; + for (const [label, answer] of arms) { + expect(shapeOf(answer.body), `${label} drifted: ${JSON.stringify(answer.body)}`) + .toBe(shapeOf(reference.body)); + } + }); + + it('the 400 arm carries its Zod dump in the DECLARED slot, not as a sibling', async () => { + const answer = await boot({ explain: vi.fn() }).explainPost({ operation: 'read' }); + expectNestedEnvelope(answer, 400, 'VALIDATION_FAILED'); + // `details` is `ApiErrorSchema`'s slot for structured context. The old + // top-level `detail` was undeclared — it survived only because + // `envelopeViolations` inspects the body's top level and the schema + // governs `error`, so nothing ever parsed it. + expect(typeof answer.body.error.details).toBe('string'); + expect(answer.body).not.toHaveProperty('detail'); + }); + + it('a healthy explain still answers the decision unwrapped — only refusals moved', async () => { + const explain = vi.fn().mockResolvedValue(DECISION); + const api = boot({ explain }); + const answer = await api.explainPost({ object: 'task', operation: 'read' }); + expect(answer.status).toBe(200); + expect(answer.body).toEqual(DECISION); + }); +}); diff --git a/packages/rest/src/security-routes.test.ts b/packages/rest/src/security-routes.test.ts index 82d1362259..7f1f449518 100644 --- a/packages/rest/src/security-routes.test.ts +++ b/packages/rest/src/security-routes.test.ts @@ -106,12 +106,14 @@ describe('GET/POST /security/explain (ADR-0090 D6)', () => { let res = mockRes(); await post!.handler({ method: 'POST', params: {}, headers: {}, body: { operation: 'read' } } as any, res); expect(res.statusCode).toBe(400); - expect(res.body.code).toBe('VALIDATION_FAILED'); + // [#8073] The code moved from the retired FLAT position to the ADR-0112 D5 + // one. Same code, same 400 — `body.error.code` is where it is now declared. + expect(res.body.error.code).toBe('VALIDATION_FAILED'); res = mockRes(); await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'frobnicate' } } as any, res); expect(res.statusCode).toBe(400); - expect(res.body.code).toBe('VALIDATION_FAILED'); + expect(res.body.error.code).toBe('VALIDATION_FAILED'); }); it("maps the service's PermissionDeniedError to 403 (manage_users / D12 gate)", async () => { @@ -124,7 +126,8 @@ describe('GET/POST /security/explain (ADR-0090 D6)', () => { await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'read', userId: 'u_target' } } as any, res); expect(res.statusCode).toBe(403); - expect(res.body.code).toBe('PERMISSION_DENIED'); + // [#8073] Migrated from the flat `res.body.code` to the D5 position. + expect(res.body.error.code).toBe('PERMISSION_DENIED'); }); it('returns 501 when no security service exposes explain', async () => { @@ -133,7 +136,8 @@ describe('GET/POST /security/explain (ADR-0090 D6)', () => { const res = mockRes(); await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'read' } } as any, res); expect(res.statusCode).toBe(501); - expect(res.body.code).toBe('NOT_IMPLEMENTED'); + // [#8073] Migrated from the flat `res.body.code` to the D5 position. + expect(res.body.error.code).toBe('NOT_IMPLEMENTED'); } }); @@ -142,6 +146,10 @@ describe('GET/POST /security/explain (ADR-0090 D6)', () => { const res = mockRes(); await post!.handler({ method: 'POST', params: {}, headers: {}, body: { object: 'task', operation: 'read' } } as any, res); expect(res.statusCode).toBe(500); - expect(res.body.code).toBe('EXPLAIN_FAILED'); + // [#8073] Migrated from the flat `res.body.code`. This arm carried the OTHER + // retired dialect too — `{ code, error: '' }` — so its message + // now lives at `body.error.message` rather than being `body.error` itself. + expect(res.body.error.code).toBe('EXPLAIN_FAILED'); + expect(res.body.error.message).toBe('boom'); }); }); diff --git a/scripts/check-route-envelope.mjs b/scripts/check-route-envelope.mjs index acac88d137..76dee8072f 100644 --- a/scripts/check-route-envelope.mjs +++ b/scripts/check-route-envelope.mjs @@ -247,7 +247,18 @@ const MODULES = { // 77 → 75 (#7981): registerSecurityEndpoints' two `handleError` arms moved // off the `{ code, error }` sibling-code literal onto the shared // `respondError` helper, banking that progress per the ratchet's own rule. - siblingCode: 75, + // + // 75 → 73 (#8073): the ADJACENT registrar, `registerSecurityExplainEndpoints`, + // followed — its two 500 arms (`EXPLAIN_FAILED`, `DELEGABLE_SCOPE_FAILED`) + // now emit through the shared `sendError` from `@objectstack/types`. + // Measured: merge-base (6ceffe0ac) siblingCode=75, branch head=73; the two + // vanished sites are merge-base lines 9332/9390, both inside that function, + // and every surviving site maps 1:1 onto a head line by the edit's own line + // shift. `stringError` is unmoved at 44 by construction: both arms carried a + // COMPUTED message (`msg.slice(0, 500)`), which that counter cannot see — + // the six flat `{ code, message }` arms converted alongside them were never + // counted by either dialect, having no `error` key at all. + siblingCode: 73, }, };