Skip to content

Commit 7e04fd0

Browse files
hotlongclaude
andauthored
fix(rest): converge the /security/explain pair onto the ADR-0112 D5 envelope (#8073) (#8174)
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 (6ceffe0) 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. Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V Co-authored-by: Claude <noreply@anthropic.com>
1 parent a453130 commit 7e04fd0

5 files changed

Lines changed: 444 additions & 30 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): one error envelope across the `/security/explain` pair (#8073)
6+
7+
`registerSecurityExplainEndpoints``GET/POST /api/v1/security/explain` and
8+
`GET /api/v1/security/my-delegable-scope` — answered two retired dialects across
9+
its eight refusal arms: the 401 / 501 / 400 / 403 arms were flat
10+
`{ code, message }`, and the two 500s were `{ code, error: 'a bare string' }`. So
11+
`body.error.code`, the one position ADR-0112 D5 declares, read `undefined` on all
12+
six — while the immediately adjacent registrar (`/security/suggested-bindings`,
13+
converged in #7981) already answered the declared shape. A client calling
14+
`explain` and then `suggested-bindings` met two envelopes inside one `security`
15+
family.
16+
17+
Every arm now emits `{ success: false, error: { code, message } }` through the
18+
shared `sendError` from `@objectstack/types` — the same builder every conformant
19+
route module writes through — so the family agrees by construction rather than by
20+
eight literals happening to match. The 400 arm's Zod-issue dump moves from a
21+
top-level `detail` sibling to `error.details`, the slot `ApiErrorSchema` declares
22+
for structured context.
23+
24+
No status code moves, and no code VALUE changes: `UNAUTHORIZED`,
25+
`NOT_IMPLEMENTED`, `VALIDATION_FAILED`, `PERMISSION_DENIED`, `EXPLAIN_FAILED` and
26+
`DELEGABLE_SCOPE_FAILED` are all already registered, so nothing in
27+
`packages/spec` moves. `ObjectStackClient` reads both envelopes' declared spots
28+
(`errorBody?.code ?? errorBody?.error?.code`, and a bare-string limb for the
29+
message), so `client.security.explain()` and
30+
`client.security.describeDelegableScope()` keep throwing identical `err.code` and
31+
`err.message` — re-measured against these call paths rather than inherited from
32+
#7981. `err.details` does change on refusals, from "the whole response body" to
33+
the structured slot or the new body.

packages/rest/src/rest-server.ts

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ import type { DirectMountedRoute, MountedRouteSource } from './direct-mount.js';
5656
import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api';
5757
import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api';
5858
import type { FieldErrorCode } from '@objectstack/spec/api';
59+
// [#8073] The closed ADR-0112 error vocabulary, so the explain family's single
60+
// refusal emitter types its `code` parameter as the vocabulary rather than as
61+
// `string` — an invented code is a compile error at the call site instead of a
62+
// runtime surprise on whichever arm a test happens to drive.
63+
import type { ErrorCode } from '@objectstack/spec/api';
5964
// The async-import row ceiling has exactly one definition, in the spec, whose
6065
// TSDoc is its public statement (#6535). rest is the only enforcer, so it reads
6166
// that export rather than re-declaring the literal beside a "mirrors spec" comment.
@@ -9268,6 +9273,45 @@ export class RestServer {
92689273
catch { return undefined; }
92699274
};
92709275

9276+
/**
9277+
* [#8073] The ONE refusal emitter for this route family — every arm of
9278+
* both handlers goes through it, so "explain and my-delegable-scope
9279+
* answer the same shape" is a property of the code rather than of
9280+
* eight literals that happen to agree.
9281+
*
9282+
* Before this, the family carried BOTH dialects ADR-0112 D5 retires:
9283+
* the 401/501/400/403 arms were flat `{ code, message }` and the two
9284+
* 500s were `{ code, error: 'a bare string' }`, so `body.error.code` —
9285+
* the one position D5 declares — read `undefined` on all six. #7035
9286+
* (PR #7293) had already removed both from this file's `/meta`
9287+
* refusals and #7981 (PR #8071) from `registerSecurityEndpoints`, the
9288+
* immediately ADJACENT registrar: a client calling `explain` and then
9289+
* `suggested-bindings` met two shapes inside one `security` family.
9290+
*
9291+
* Emitted through the SHARED builder (`sendError` from
9292+
* `@objectstack/types`, imported as `sendEnvelopeError` because this
9293+
* module has a local `sendError` of its own — the sanitizing responder
9294+
* for THROWN errors, a different thing). That is what makes this the
9295+
* reference shape by construction rather than a ninth local literal
9296+
* agreeing with the eight it replaced, and it types `code` to the
9297+
* closed vocabulary for free.
9298+
*
9299+
* ⛔ Status codes are untouched: only the POSITION of `code` and
9300+
* `message` moves. `detail` — the 400 arm's Zod-issue dump — moves to
9301+
* `error.details`, the slot `ApiErrorSchema` actually declares for
9302+
* structured context; as a top-level sibling it was undeclared.
9303+
*/
9304+
const respondError = (
9305+
res: any,
9306+
status: number,
9307+
code: ErrorCode,
9308+
message: string,
9309+
details?: unknown,
9310+
): void => sendEnvelopeError(
9311+
res, status, code, message,
9312+
details === undefined ? undefined : { details },
9313+
);
9314+
92719315
const handler = async (req: any, res: any) => {
92729316
try {
92739317
const environmentId = isScoped ? req.params?.environmentId : undefined;
@@ -9276,18 +9320,18 @@ export class RestServer {
92769320
if (!context?.userId) {
92779321
// The explain surface stays authenticated-only — it is an
92789322
// admin diagnosis tool. (Anonymous is already 401ed above.)
9279-
return res.status(401).json({
9280-
code: 'UNAUTHORIZED',
9281-
message: 'The access-explanation endpoint requires an authenticated caller.',
9282-
});
9323+
return respondError(
9324+
res, 401, 'UNAUTHORIZED',
9325+
'The access-explanation endpoint requires an authenticated caller.',
9326+
);
92839327
}
92849328

92859329
const svc = await resolveService(environmentId, req);
92869330
if (!svc || typeof svc.explain !== 'function') {
9287-
return res.status(501).json({
9288-
code: 'NOT_IMPLEMENTED',
9289-
message: 'Access explanation is not available on this deployment (no security service with explain).',
9290-
});
9331+
return respondError(
9332+
res, 501, 'NOT_IMPLEMENTED',
9333+
'Access explanation is not available on this deployment (no security service with explain).',
9334+
);
92919335
}
92929336

92939337
// GET reads the request from the query string, POST from the
@@ -9310,11 +9354,11 @@ export class RestServer {
93109354
...(src.recordId != null && src.recordId !== '' ? { recordId: src.recordId } : {}),
93119355
});
93129356
if (!parsed.success) {
9313-
return res.status(400).json({
9314-
code: 'VALIDATION_FAILED',
9315-
message: 'Invalid explain request — expected { object: string, operation: read|create|update|delete|transfer|restore|purge, userId?: string, recordId?: string }.',
9316-
detail: String(parsed.error?.message ?? '').slice(0, 1000),
9317-
});
9357+
return respondError(
9358+
res, 400, 'VALIDATION_FAILED',
9359+
'Invalid explain request — expected { object: string, operation: read|create|update|delete|transfer|restore|purge, userId?: string, recordId?: string }.',
9360+
String(parsed.error?.message ?? '').slice(0, 1000),
9361+
);
93189362
}
93199363

93209364
const decision = await svc.explain(parsed.data, context);
@@ -9326,10 +9370,13 @@ export class RestServer {
93269370
error?.name === 'PermissionDeniedError' ||
93279371
msg.startsWith('[Security] Access denied')
93289372
) {
9329-
return res.status(403).json({ code: 'PERMISSION_DENIED', message: msg.slice(0, 1000) });
9373+
return respondError(res, 403, 'PERMISSION_DENIED', msg.slice(0, 1000));
93309374
}
93319375
logError('[REST] Security explain error:', error);
9332-
res.status(500).json({ code: 'EXPLAIN_FAILED', error: msg.slice(0, 500) });
9376+
// The 500 arm keeps its 500-char cap: an unexpected fault's
9377+
// message is not a contract, and truncating it stays a
9378+
// sanitization step — only the position of the words moves.
9379+
respondError(res, 500, 'EXPLAIN_FAILED', msg.slice(0, 500));
93339380
}
93349381
};
93359382

@@ -9369,25 +9416,25 @@ export class RestServer {
93699416
const context = await this.resolveExecCtx(environmentId, req);
93709417
if (this.enforceAuth(req, res, context)) return;
93719418
if (!context?.userId) {
9372-
return res.status(401).json({
9373-
code: 'UNAUTHORIZED',
9374-
message: 'The delegable-scope endpoint requires an authenticated caller.',
9375-
});
9419+
return respondError(
9420+
res, 401, 'UNAUTHORIZED',
9421+
'The delegable-scope endpoint requires an authenticated caller.',
9422+
);
93769423
}
93779424

93789425
const svc = await resolveService(environmentId, req);
93799426
if (!svc || typeof svc.describeDelegableScope !== 'function') {
9380-
return res.status(501).json({
9381-
code: 'NOT_IMPLEMENTED',
9382-
message: 'Delegated administration is not available on this deployment (no security service with describeDelegableScope).',
9383-
});
9427+
return respondError(
9428+
res, 501, 'NOT_IMPLEMENTED',
9429+
'Delegated administration is not available on this deployment (no security service with describeDelegableScope).',
9430+
);
93849431
}
93859432

93869433
res.json(await svc.describeDelegableScope(context));
93879434
} catch (error: any) {
93889435
const msg = String(error?.message ?? error ?? '');
93899436
logError('[REST] Delegable scope error:', error);
9390-
res.status(500).json({ code: 'DELEGABLE_SCOPE_FAILED', error: msg.slice(0, 500) });
9437+
respondError(res, 500, 'DELEGABLE_SCOPE_FAILED', msg.slice(0, 500));
93919438
}
93929439
};
93939440

0 commit comments

Comments
 (0)