From 2e2306d961bb53b442887ab410252f8ccf204661 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 03:00:36 +0000 Subject: [PATCH] fix(rest): GET /meta/diagnostics refuses an unknown query parameter name instead of answering the full sweep The diagnostics route now declares its closed query-parameter set (type, severity, package) through the shared closed-set decision, and refuses any other name with 400 INVALID_REQUEST in the route's nested envelope. Before, `?typ=view` was ignored and answered 200 with every type swept. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_016WV4cxscKusnsCNkzSsjxM --- .../27633-diagnostics-unknown-param-name.md | 18 +++++++++ packages/rest/src/meta-collection-routes.ts | 22 ++++++++++- ...st-meta-diagnostics-unknown-params.test.ts | 39 +++++++++++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 .changeset/27633-diagnostics-unknown-param-name.md diff --git a/.changeset/27633-diagnostics-unknown-param-name.md b/.changeset/27633-diagnostics-unknown-param-name.md new file mode 100644 index 00000000000..7f07dada577 --- /dev/null +++ b/.changeset/27633-diagnostics-unknown-param-name.md @@ -0,0 +1,18 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest): `GET /meta/diagnostics` refuses an unknown query parameter name instead of ignoring it and answering the full sweep (#27633) + +Clause-②: no (narrowing) +Metadata-protocol: none + + + +`/meta/diagnostics` is the door an author, an agent or CI reads to learn whether metadata is healthy. #27514 made it refuse an unknown VALUE of `type`, `severity` or `package`; an unknown parameter NAME was still dropped, so `?typ=view` answered `200` with every type swept and the caller believed the answer was narrowed to `view`. + +- The route now declares its closed query-parameter set, `type`, `severity` and `package` (the three it reads and the three both SDKs send), through the shared closed-set decision the converted routes use. +- Any other name answers `400 INVALID_REQUEST` in the route's nested `{ error: { code, message } }` envelope. The message names the unknown parameter and lists the supported ones. +- Recognition runs before the repeated-parameter refusal, so a request with both faults is told about the unknown name first. + +**BREAKING** for a caller that sends `GET /meta/diagnostics` a parameter outside `type`, `severity` and `package`: it was ignored and now answers `400 INVALID_REQUEST`. The remedy is to drop the parameter, or to use the supported spelling (`package`, not `packageId`). diff --git a/packages/rest/src/meta-collection-routes.ts b/packages/rest/src/meta-collection-routes.ts index 30dcd1f6ac8..e6771aaf447 100644 --- a/packages/rest/src/meta-collection-routes.ts +++ b/packages/rest/src/meta-collection-routes.ts @@ -91,6 +91,7 @@ import type { GetMetaItemsRequest } from '@objectstack/spec/api'; import { truncateClientMessage } from '@objectstack/types'; import { handleRouteError, sendFieldVisibilityFault } from './error-response.js'; import * as metaReadGate from './meta-item-read-gate.js'; +import { unknownQueryParamRefusalMessage } from './query-allowlist.js'; import { refuseRepeatedQueryParams } from './query-multiplicity.js'; import { mayReadPendingDrafts, metaItemPackageBinding, type RestServer } from './rest-server.js'; import type { RouteManager } from './route-manager.js'; @@ -201,6 +202,15 @@ function diagnosticsRefusal(message: string): Error { return Object.assign(new Error(message), { code: 'INVALID_REQUEST', status: 400 }); } +/** + * [#27633] The closed query-parameter set of `GET {metaPath}/diagnostics`. + * **Measured**: the three names the handler reads — `type`, `severity` and + * `package` — and the three both SDKs send (`client.meta.getDiagnostics`, + * the Console's `metadata-client`). Each is single-valued, so the same list + * is the route's multiplicity declaration (#6877). + */ +const META_DIAGNOSTICS_PARAMS: readonly string[] = ['type', 'severity', 'package']; + /** * [#27514] Is this a classified `400 INVALID_REQUEST` refusal of the * diagnostics request: {@link diagnosticsRefusal}, the list door's @@ -314,7 +324,14 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop // [#6877] All three narrow the diagnostics query to ONE // value each; the `as string` casts are exactly the // laundering that kept `tsc` silent about the array arm. - if (refuseRepeatedQueryParams(req, res, ['severity', 'type', 'package'])) return; + // [#27633] Recognition first, as the closed-set rule orders + // it: an unknown NAME (`?typ=view`) was ignored and answered + // the full sweep, the same false all-clear #27514 closed for + // unknown values. The decision is the shared one; the refusal + // is thrown so it leaves in this route's one envelope. + const unknownParams = unknownQueryParamRefusalMessage(req.query, META_DIAGNOSTICS_PARAMS); + if (unknownParams !== undefined) throw diagnosticsRefusal(unknownParams); + if (refuseRepeatedQueryParams(req, res, META_DIAGNOSTICS_PARAMS)) return; // [#27514] The sweep reports one of two severities, and any // other value used to be read as `error` and answered 200: // a caller asking for something this sweep does not have @@ -356,7 +373,8 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop // [#27514] ONE envelope for every refusal this route // answers. The repeated-parameter refusal above writes the // ADR-0112 nested `{ error: { code, message } }`, and the - // classified 400s that are THROWN (the severity check above, + // classified 400s that are THROWN (the [#27633] unknown + // parameter name and the severity check above, // the list door's unknown-type verdict, and the sweep's // [#8924] near-miss spelling and unknown package) used to // leave through `handleRouteError` as `{ error: '…', code }`, diff --git a/packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts b/packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts index 0d111bdba7c..8ebffe3963d 100644 --- a/packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts +++ b/packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts @@ -179,3 +179,42 @@ describe('[#27514] preservation: what the door could already serve still answers expect(answer.body.scannedTypes).toBe(1); }); }); + +/** + * [#27633] The same false all-clear one axis over: an unknown parameter NAME. + * Measured on `main` before this change, `?typ=view` answered `200` with the + * full sweep, so a caller who believed the answer was narrowed to `view` was + * handed every type. The route now declares its closed set (the three names + * the handler reads) and refuses any other name in its one nested envelope. + */ +describe('[#27633] an unknown parameter name is refused, not ignored into a full sweep', () => { + it('?typ=view answers 400 INVALID_REQUEST naming the parameter and the supported set', async () => { + const { diagnostics } = boot(); + const answer = await diagnostics({ typ: 'view' }); + expectRefusal(answer, 'INVALID_REQUEST'); + expect(answer.body.error.message).toContain('"typ"'); + expect(answer.body.error.message).toContain('Supported parameters: package, severity, type.'); + }); + + it('?packageId= (the `_drafts` spelling, never read here) answers 400 INVALID_REQUEST', async () => { + const { diagnostics } = boot(); + expectRefusal(await diagnostics({ packageId: PACKAGE_ID }), 'INVALID_REQUEST'); + }); + + it('an unknown name beside known ones is still refused, even with an empty value', async () => { + const { diagnostics } = boot(); + expectRefusal(await diagnostics({ type: 'view', severity: 'warning', sevrity: '' }), 'INVALID_REQUEST'); + }); + + it('an unknown name outranks a repeated known one', async () => { + const { diagnostics } = boot(); + expectRefusal(await diagnostics({ typ: 'view', type: ['view', 'object'] }), 'INVALID_REQUEST'); + }); + + it('the three names the handler reads still answer together', async () => { + const { diagnostics } = boot(); + const answer = await diagnostics({ type: 'view', severity: 'warning', package: PACKAGE_ID }); + expect(answer.status).toBe(200); + expect(answer.body.stats.view.count).toBe(1); + }); +});