Skip to content

Commit 9cd54fa

Browse files
fix(rest): GET /meta/diagnostics refuses an unknown query parameter name instead of answering the full sweep (#27913)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DADd6hnVtG5rSJrF9DUrLEF)_ Fixes #27633 Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: follow-up #27632 (the same `/meta/diagnostics` query table; the unknown-name refusal is added to that card in a comment) > **BREAKING** for a caller that sends `GET /meta/diagnostics` a query parameter outside `type`, `severity` and `package`: it was ignored and now answers `400 INVALID_REQUEST`. The remedy is to drop it or use the supported spelling (`package`, not `packageId`). ADR-0087 disposition: `not-required (no-migration-prescription)`, in the changeset. ## 维护者速读 - **改了什么**:诊断接口 `GET /meta/diagnostics` 现在只认 `type`、`severity`、`package` 三个查询参数,拼错或多传一个参数名(如 `?typ=view`)会回 400 并列出支持的参数,而不是悄悄忽略、回全量扫描。 - **为什么改**:#27620 已经让它拒绝未知的参数「值」;参数「名」拼错时仍回 200 全量结果,调用方以为只看了 `view`,其实是全部类型,同样是假的「全部正常」。 - **风险与回滚**:只收窄这一个只读接口的请求面。两个官方 SDK(`client.meta.getDiagnostics` 与 Console 的 `metadata-client`)只发这三个参数,不受影响。回滚即还原本 PR。 - **先看**:`packages/rest/src/meta-collection-routes.ts` 里诊断路由开头新增的两行,以及文件顶部的 `META_DIAGNOSTICS_PARAMS`。 - **两分钟验证**:`pnpm --filter @objectstack/rest exec vitest run src/rest-meta-diagnostics-unknown-params.test.ts`,16 条全绿(其中 #27633 的 5 条)。 - **需要你做的**:无(合同复审由席位安排)。 ## What changes - `GET {metaPath}/diagnostics` declares its closed query-parameter set, `META_DIAGNOSTICS_PARAMS = ['type', 'severity', 'package']`, measured from the handler's read points and matching what both SDKs send. - The handler asks the shared closed-set decision, `unknownQueryParamRefusalMessage` from `query-allowlist.ts` (the one `refuseUnknownQueryParams` writes; no second helper), and throws the route's own `diagnosticsRefusal`. So the refusal is `400 INVALID_REQUEST` in the nested `{ error: { code, message } }` envelope, the code every other thrown refusal of this route answers since #27514. The message names the unknown parameter and lists the supported set. - Recognition runs before the repeated-parameter refusal, as the closed-set rule orders it. The multiplicity call now reads the same list (all three names are single-valued). ## Why #27514 / #27620 closed the false all-clear for unknown VALUES. An unknown NAME was still dropped: `?typ=view` answered `200` with every type swept, and a caller who believed the answer was narrowed could not tell. ## Verification - Pinned on `main` first (b13a1ed): of 5 new cases, the 4 refusal pins were red (`?typ=view`, `?packageId=`, an unknown name beside known ones with an empty value, an unknown name with a repeated known one) and the preservation pin (all three known names together answer 200) was green. - Ablation: disabling the new throw reds exactly those 4; restored, 16/16 green. - `pnpm --filter @objectstack/rest exec vitest run`: 352 files passed, 7025 tests passed, 350 skipped. `tsc --noEmit` on `packages/rest`: exit 0. - `dispatch-gates --ran`: 60 derived, 58 run (exit 0), 2 NOT-MEASURED (exit 3, prerequisite in a fresh container: `check-plugin-teardown-shape --self-test` needs an unshallow clone, `check:dual-build-cjs-loads` needs every package's dist), 0 UNRUN. - `pnpm lint`: exit 0. - Dogfood battery at the final head: posted as a PR comment when the run completes. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: n/a. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016WV4cxscKusnsCNkzSsjxM Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6caca51 commit 9cd54fa

3 files changed

Lines changed: 77 additions & 2 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest): `GET /meta/diagnostics` refuses an unknown query parameter name instead of ignoring it and answering the full sweep (#27633)
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) The narrowed face is a REQUEST to one read door: a query parameter name the route never read (`?typ=view`, `?packageId=`) is now refused instead of silently ignored. No stored metadata, package source or object definition holds a diagnostics request, no schema key or export changes, and the remedy is the caller's own query string, so there is nothing for `objectstack migrate meta` to rewrite. -->
11+
12+
`/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`.
13+
14+
- 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.
15+
- 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.
16+
- Recognition runs before the repeated-parameter refusal, so a request with both faults is told about the unknown name first.
17+
18+
**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`).

‎packages/rest/src/meta-collection-routes.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ import type { GetMetaItemsRequest } from '@objectstack/spec/api';
9191
import { truncateClientMessage } from '@objectstack/types';
9292
import { handleRouteError, sendFieldVisibilityFault } from './error-response.js';
9393
import * as metaReadGate from './meta-item-read-gate.js';
94+
import { unknownQueryParamRefusalMessage } from './query-allowlist.js';
9495
import { refuseRepeatedQueryParams } from './query-multiplicity.js';
9596
import { mayReadPendingDrafts, metaItemPackageBinding, type RestServer } from './rest-server.js';
9697
import type { RouteManager } from './route-manager.js';
@@ -201,6 +202,15 @@ function diagnosticsRefusal(message: string): Error {
201202
return Object.assign(new Error(message), { code: 'INVALID_REQUEST', status: 400 });
202203
}
203204

205+
/**
206+
* [#27633] The closed query-parameter set of `GET {metaPath}/diagnostics`.
207+
* **Measured**: the three names the handler reads — `type`, `severity` and
208+
* `package` — and the three both SDKs send (`client.meta.getDiagnostics`,
209+
* the Console's `metadata-client`). Each is single-valued, so the same list
210+
* is the route's multiplicity declaration (#6877).
211+
*/
212+
const META_DIAGNOSTICS_PARAMS: readonly string[] = ['type', 'severity', 'package'];
213+
204214
/**
205215
* [#27514] Is this a classified `400 INVALID_REQUEST` refusal of the
206216
* diagnostics request: {@link diagnosticsRefusal}, the list door's
@@ -314,7 +324,14 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop
314324
// [#6877] All three narrow the diagnostics query to ONE
315325
// value each; the `as string` casts are exactly the
316326
// laundering that kept `tsc` silent about the array arm.
317-
if (refuseRepeatedQueryParams(req, res, ['severity', 'type', 'package'])) return;
327+
// [#27633] Recognition first, as the closed-set rule orders
328+
// it: an unknown NAME (`?typ=view`) was ignored and answered
329+
// the full sweep, the same false all-clear #27514 closed for
330+
// unknown values. The decision is the shared one; the refusal
331+
// is thrown so it leaves in this route's one envelope.
332+
const unknownParams = unknownQueryParamRefusalMessage(req.query, META_DIAGNOSTICS_PARAMS);
333+
if (unknownParams !== undefined) throw diagnosticsRefusal(unknownParams);
334+
if (refuseRepeatedQueryParams(req, res, META_DIAGNOSTICS_PARAMS)) return;
318335
// [#27514] The sweep reports one of two severities, and any
319336
// other value used to be read as `error` and answered 200:
320337
// a caller asking for something this sweep does not have
@@ -356,7 +373,8 @@ export function registerMetaCollectionRoutes(this: MetaCollectionRouteHost, scop
356373
// [#27514] ONE envelope for every refusal this route
357374
// answers. The repeated-parameter refusal above writes the
358375
// ADR-0112 nested `{ error: { code, message } }`, and the
359-
// classified 400s that are THROWN (the severity check above,
376+
// classified 400s that are THROWN (the [#27633] unknown
377+
// parameter name and the severity check above,
360378
// the list door's unknown-type verdict, and the sweep's
361379
// [#8924] near-miss spelling and unknown package) used to
362380
// leave through `handleRouteError` as `{ error: '…', code }`,

‎packages/rest/src/rest-meta-diagnostics-unknown-params.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,42 @@ describe('[#27514] preservation: what the door could already serve still answers
179179
expect(answer.body.scannedTypes).toBe(1);
180180
});
181181
});
182+
183+
/**
184+
* [#27633] The same false all-clear one axis over: an unknown parameter NAME.
185+
* Measured on `main` before this change, `?typ=view` answered `200` with the
186+
* full sweep, so a caller who believed the answer was narrowed to `view` was
187+
* handed every type. The route now declares its closed set (the three names
188+
* the handler reads) and refuses any other name in its one nested envelope.
189+
*/
190+
describe('[#27633] an unknown parameter name is refused, not ignored into a full sweep', () => {
191+
it('?typ=view answers 400 INVALID_REQUEST naming the parameter and the supported set', async () => {
192+
const { diagnostics } = boot();
193+
const answer = await diagnostics({ typ: 'view' });
194+
expectRefusal(answer, 'INVALID_REQUEST');
195+
expect(answer.body.error.message).toContain('"typ"');
196+
expect(answer.body.error.message).toContain('Supported parameters: package, severity, type.');
197+
});
198+
199+
it('?packageId= (the `_drafts` spelling, never read here) answers 400 INVALID_REQUEST', async () => {
200+
const { diagnostics } = boot();
201+
expectRefusal(await diagnostics({ packageId: PACKAGE_ID }), 'INVALID_REQUEST');
202+
});
203+
204+
it('an unknown name beside known ones is still refused, even with an empty value', async () => {
205+
const { diagnostics } = boot();
206+
expectRefusal(await diagnostics({ type: 'view', severity: 'warning', sevrity: '' }), 'INVALID_REQUEST');
207+
});
208+
209+
it('an unknown name outranks a repeated known one', async () => {
210+
const { diagnostics } = boot();
211+
expectRefusal(await diagnostics({ typ: 'view', type: ['view', 'object'] }), 'INVALID_REQUEST');
212+
});
213+
214+
it('the three names the handler reads still answer together', async () => {
215+
const { diagnostics } = boot();
216+
const answer = await diagnostics({ type: 'view', severity: 'warning', package: PACKAGE_ID });
217+
expect(answer.status).toBe(200);
218+
expect(answer.body.stats.view.count).toBe(1);
219+
});
220+
});

0 commit comments

Comments
 (0)