Skip to content

Commit cb9841a

Browse files
fix(hono,runtime)!: createHonoApp keeps a repeated query name as an array, so the dispatcher refuses it; /packages refuses a repeated keepData or overwrite (#23466)
Fixes #23214 Clause-②: yes (narrowing) Slice 2 of this card, built under seat ruling `6165288468` (Q1 B, Q2 A, Q3 A) and claim `6165302054`. Slice 1 (PR #23263, landed as `d45d0e938a`) guarded the dispatcher's read points. This PR guards the two `/packages` write flags that slice 1's census did not name, then flips the adapter: the order "guard first, then flip" is met inside one PR. ## What changes **`@objectstack/hono`: the catch-all keeps a repeated query name.** `createHonoApp`'s `${prefix}/*` catch-all built the dispatcher's query with `URLSearchParams.forEach` into an object, so `?package=a&package=b` reached `HttpDispatcher.dispatch()` as `'b'`. Every arity rule the dispatcher declares was unreachable on a `createHonoApp` host. It now reads the query through `readDispatchQuery`, a module-private mirror of `@objectstack/plugin-hono-server`'s `readQuery` (that one is module-private, and that package's `exports` has only `.`). It uses `c.req.queries()` with the length normalisation (one value stays a string, a repeat is the array) and `Object.fromEntries`, never a dynamic write, so `?__proto__=x` is an own key and no prototype moves. The docblock names the twin and the #6878 ruling. The catch arms are untouched; #23365 is not addressed here. **`@objectstack/runtime`: `/packages` refuses a repeated `keepData` or `overwrite`.** Both were read as `query?.NAME === 'true'`, so a repeat read as absent: - `DELETE /packages/:id?keepData=true&keepData=true` answered `200` and `deletePackage` received no `keepData`. The protocol keeps object storage only for `keepData === true`, so the tables the caller asked to keep were dropped. This was measured on a `plugin-hono-server` host before this PR, through the explicit `/packages` mount. Today's `createHonoApp` keeps them (last value), so the flip alone would have carried the drop to a second door. - `POST /packages?overwrite=true&overwrite=true` answered `409` "already exists" for a package one `overwrite=true` replaces. Both now refuse a repeat with `400 VALIDATION_ERROR` and `repeatedQueryParamMessage`, as the domain already refuses `enabled` and `version`. Each refusal sits after the two authorization gates (the capability gate and the wall) and before any registry or protocol call. A one-element array is one occurrence and is unwrapped in place, as the `status`/`type` rule does (`refuseRepeatedWriteFlag`, module-private). ## The order lesson, re-measured - On `origin/main` `823da2f21c`, a static census of every non-comment query read in the 22 domain modules and the `@objectstack/rest` helpers they call, plus a host map and empirical `dispatch()` probes with the post-flip array shape (os-dev-report `6165233131`), found: - no read point added since slice 1; - two unguarded `/packages` write flags, the ones this PR guards. - Re-checked at `61b12584f0`: the 5 commits since `823da2f21c` touch none of `packages/runtime`, `packages/rest`, `packages/adapters`, `plugin-hono-server` or `http-conformance` (diff stat: 0 lines). - #23265 (in flight in `domains/data.ts`, a different file) adds a `DELETE /data/:object/:id ?expectedVersion` read point that refuses a repeat. It is not on `main` and is not part of this PR. ## Pins - `packages/adapters/hono/src/hono-query-multiplicity.test.ts` (15 tests, over the package's recording stub): - a repeated name reaches `dispatch()` as the array of its values; - nine single-valued inputs (plain, `+`, `%20`, a UTF-8 escape, a percent-encoded key, an empty value, a bare flag, a comma list, none) arrive exactly as the OLD line produced them, compared against a reference Hono app running that line; - `__proto__`, once and repeated, is an own key with `Object.prototype` untouched. - `packages/qa/http-conformance/src/hono-query-multiplicity.conformance.test.ts` (7 tests): a REAL `createHonoApp` over the REAL `HttpDispatcher`, composed as its six `hono-*` siblings are. It lives here because `packages/adapters/hono/vitest.config.ts` aliases `@objectstack/runtime` to a stub for that whole suite. It pins: - `GET /meta/app/crm?package=a&package=b` refused `400` before `getMetaItem`; - `GET /packages` with a repeated `status` or `enabled` refused `400`; - `DELETE /packages/:id?keepData=true&keepData=true` refused `400` with `deletePackage` never called; - a single-value control for each, asserting what the service received. Refusals assert status, nested `error.code` `VALIDATION_ERROR`, and the named parameter with its count ("supplied 2 times"). This is not `query-multiplicity.conformance.test.ts`, which is untouched (ruling item 3). - `packages/runtime/src/domains/packages-write-flag-arity.test.ts` (11 tests, through `dispatch()` over a real `SchemaRegistry`): - three `keepData` repeat shapes refused, with neither `deletePackage` nor `registry.getPackage` called; - one `keepData` (`'true'`, `'1'`, `['true']`, absent) reaches `deletePackage` as `{ packageId, keepData: true }` or `{ packageId }`; - a repeated `overwrite` refused, with neither `installPackage` nor `registry.getPackage` called; - one `overwrite` (`'true'`, `['true']`) overwrites with `201`; - a control: no `overwrite` is still `409`. ## Ablations (one-time proofs; no permanent test files) 1. **The flip.** `node scripts/ablation-replace.mjs` (WRAP mode, restore on exit) put the old `forEach` construction back in `packages/adapters/hono/src/index.ts`. - Anchor x1 → x0, blob `4089af0a973d` → `6948cd6354ba`. - No build leg: the hono pin file imports `./index` from source, and http-conformance aliases `@objectstack/hono` (`/^@objectstack\/hono$/`) to `src/index.ts`. - Hono pin file: **6 failed, 9 passed**. The 4 array-shape and 2 `__proto__` cases went red; all 9 single-value controls stayed green. - Conformance file: **4 failed, 3 passed**. Every refusal went red; every control stayed green. - Restore: blob == HEAD (`4089af0a973d`), `git diff HEAD` empty. 2. **The `keepData` guard.** The guard call in `domains/packages.ts` was disabled behind a marker property, in HOLD mode inside a script with an EXIT/INT/TERM trap. - Anchor x1 → x0, blob `20fc9b7cd2c2` → `ecba3c0a4be4`, then `pnpm --filter @objectstack/runtime build`. - `ablation-dist-preflight` found the marker in 2 built files. The conformance file reads runtime through `dist/`. - Runtime pin file: **4 failed, 7 passed**. The three `keepData` refusals went red, plus the one-element-array preservation (the unwrap is the guard's own); every `overwrite` case stayed green. - Conformance file: **1 failed, 6 passed** (the `keepData` refusal). - Restore: blob == HEAD (`20fc9b7cd2c2`), rebuild, `--absent` preflight (marker absent from all 6 built files; whole tree clean). ## Tests and gates (head `99c7c050b2`, after merging `origin/main` `7b3d5585ca`) All runs went through `os-verify-lock` (slot `issue-23214-s2`), each with `VERDICT command-exit 0`. - **Post-merge, at `99c7c050b2`:** - closure rebuild `turbo run build --filter='@objectstack/http-conformance^...' --concurrency=2`: 36/36 tasks; - runtime `packages-write-flag-arity.test.ts` plus slice 1's `dispatcher-query-arity.test.ts`: 44 passed; - `@objectstack/hono` test: 7 files / 154 passed, and typecheck exit 0 (both configs; the test files are under `src/**`); - `@objectstack/http-conformance` test: 12 files / 124 passed, and typecheck exit 0 (test-layer ledger held at 3 files / 27 errors / 10 signatures); - `@objectstack/runtime` typecheck: exit 0, ledger held at 27 / 190 / 68; - `pnpm --filter @objectstack/spec check:generated`: all 14 artifacts up to date. - **Runtime full local suite** at `e5da6a04aa` (runtime source identical to this head): 374 files, 5466 passed / 19 skipped. - **Gate union:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths) derives 63 commands at `99c7c050b2`. All 63 were run, each exit captured before any pipe: **63 exit 0**. `--ran`: "63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN". On a pre-merge head the union reddened `check-registry-log-declared`, because the conformance file constructed a `SchemaRegistry` in a suite that declares no registry log level; the rows now use the three-method double. - **Lint**, as the proven narrowing over the 5 changed `.ts` files: 1. Population: `eslint.config.mjs:971`'s block covers them all, and none is ignored. 2. Count: `--format json` with `--no-inline-config` gives 0 errors, 0 warnings and 0 fatal (eslint exit 0). 3. Invariance: no type-aware linting (`:328`), so no untouched file's verdict can move. ## Acceptance notes - **Excluded by ruling Q3 → A:** each of these already answers a repeat exactly as its canonical, already array-fed twin does, so the flip opens no divergence. - P3 `GET /analytics/meta ?cube` (typeof: a repeat reads as absent, an unscoped list). Twin: the `dispatcher-plugin` `/analytics/meta` mount. - P4 the share-link list (`?object`, `?recordId`, `?includeRevoked`) and resolve (`?email`, `?password`) names (typeof or `=== 'true'`: a repeat reads as absent). Twin: `plugin-sharing`'s share-link routes. - **One malformed input moves the other way.** A query pair with an EMPTY name (`?=x`) is dropped by hono's parser, so it no longer reaches the dispatcher. Measured through a real `createHonoApp`: `GET /api/v1/meta/app/crm?=x` was `400 VALIDATION_ERROR` ('The "" query parameter is not supported…') with the old line, and is `200` now. `plugin-hono-server`'s `readQuery` already drops it the same way, so this converges the two Hono doors. It is stated in the changeset. The ruled `Clause-②: no (narrowing)` line is kept; whether this input changes that line is the seat's to say. - **Multi-valued names.** Names such as `select` and `expand` now reach the dispatcher as arrays on a `createHonoApp` host, exactly as `RestServer` hands them to `findData` / `getData` through `plugin-hono-server`. - **Not changed in this PR:** - The domain's inline `status`/`type` loop (slice 1) could adopt `refuseRepeatedWriteFlag`; it is outside this claim's surface. - Slice 1's changeset (`.changeset/23214-dispatcher-query-arity.md`, unreleased) still says the refusals are reachable only through `dispatch()` until this adapter keeps repeats. It is outside this claim's surface; this PR's changeset states the new reach. - **hono version.** `c.req.queries()`'s null-prototype record is what keeps `__proto__` as an own key on the installed hono 4.13.12. On the peer-range floor 4.12.8 (read statically), hono's own parser drops a `__proto__` name, so the pin is a reading of the installed version. No prototype is polluted on either. --- _Generated by [Claude Code](https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 584e737 commit cb9841a

6 files changed

Lines changed: 516 additions & 3 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/hono': minor
3+
'@objectstack/runtime': minor
4+
---
5+
6+
fix(hono,runtime)!: `createHonoApp` hands the dispatcher a repeated query name as an array, so the dispatcher's arity rules refuse it instead of answering its last value; `/packages` refuses a repeated `keepData` or `overwrite`
7+
8+
`@objectstack/hono`'s `createHonoApp` catch-all built the dispatcher's query by copying `URLSearchParams` into an object, so a name sent twice reached `HttpDispatcher.dispatch()` as its LAST value and the first was dropped without a word. Every arity rule the dispatcher declares was therefore unreachable on a `createHonoApp` host: `GET /meta/app/crm?package=a&package=b` was answered `200` for package `b`, where the same request through `@objectstack/plugin-hono-server` is refused. The catch-all now reads the query the way `@objectstack/plugin-hono-server` does: a name sent once is the string it always was, a repeated name is the array of its values, and the object is built without dynamic property writes, so a `__proto__` name is an ordinary own key.
9+
10+
Two `/packages` write flags read a repeat as absent, and are now refused before the registry or the protocol is touched, as `enabled` and `version` already are. `DELETE /packages/:id?keepData=true&keepData=true` dropped the package's object tables, because the protocol keeps storage only for `keepData === true`, and was answered `200`. `POST /packages?overwrite=true&overwrite=true` answered `409` "already exists" for a package one `overwrite=true` replaces. Both doors were reached that way through `@objectstack/plugin-hono-server`, whose adapter already keeps repeats, and would have been through `createHonoApp` from this change.
11+
12+
Clause-②: yes (narrowing)
13+
14+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling or stored shape is added, removed, renamed or re-shaped, and no exported declaration is added, removed or narrowed (the adapter's query reader and the domain's flag guard are both module-private). No stored row or package source changes, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is a query accept set: on a `createHonoApp` host every dispatcher read point that refuses a repeated single-valued name now sees the repeat and refuses it, and on every host `DELETE /packages/:id` and `POST /packages` refuse a repeated `keepData` and `overwrite`. One malformed input widens: on a `createHonoApp` host, a query pair with an empty name is no longer refused, matching `@objectstack/plugin-hono-server`. The remedy is in the request a caller sends, never a rewrite of a package source or a stored row. The other categories are closed on facts: both packages publish (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
15+
16+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
17+
18+
What changes for you: send each single-valued query name once. On a `createHonoApp` host, a request that repeats one is now refused `400` in the dispatcher's envelope, `{ success: false, error: { code, message } }`, with the code and sentence the same request gets through `@objectstack/plugin-hono-server` (`VALIDATION_ERROR`, "The "NAME" query parameter was supplied N times…"; `INVALID_FILTER` on the `/data` filter slot), where it used to be answered with the last value. On every host, `DELETE /packages/:id` refuses a repeated `keepData` and `POST /packages` a repeated `overwrite`, with that same `VALIDATION_ERROR` and sentence. A name sent once is answered as before; genuinely multi-valued names such as `select` and `expand` now reach the dispatcher as the array they are. One malformed input moves the other way, to match `@objectstack/plugin-hono-server`: a query pair with an EMPTY name (`?=x`) is no longer handed to the dispatcher, so a door with a closed parameter set reads it as absent where it used to refuse it as an unknown parameter `""`. The `@objectstack/client` SDK sends each single-valued name once, and the Console's package delete sends `keepData` once.
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#23214] The `${prefix}/*` catch-all hands `dispatch()` a REPEATED query name
5+
* as an array, and a single-valued one as the plain string it always handed.
6+
*
7+
* ## The defect
8+
*
9+
* The catch-all copied `new URL(c.req.url).searchParams.forEach` into an
10+
* object, so `?package=a&package=b` reached `dispatch()` as `'b'`: the first
11+
* value was dropped without a word, and every arity rule the dispatcher
12+
* declares was unreachable on a `createHonoApp` host. It now reads the query as
13+
* `plugin-hono-server`'s `readQuery` does (`readDispatchQuery`).
14+
*
15+
* ## What is pinned here
16+
*
17+
* 1. THE SHAPE: a repeated name arrives as the array of every value, in order.
18+
* 2. THE CONTROL: every single-valued name arrives as the string the old line
19+
* produced — checked against a reference Hono app that runs that exact
20+
* line, so "unchanged" is a comparison, not a recollection.
21+
* 3. `__proto__` is an OWN key and no prototype moves. The old dynamic write
22+
* dropped the name silently; `Object.fromEntries` keeps it as data.
23+
*
24+
* The dispatcher is a recording fixture: this package's config aliases
25+
* `@objectstack/runtime` to a stub for the whole suite. That the REAL
26+
* dispatcher then refuses the repeat — `/meta`, `/packages`, `?keepData` — is
27+
* pinned through a real `createHonoApp` in
28+
* `packages/qa/http-conformance/src/hono-query-multiplicity.conformance.test.ts`.
29+
*/
30+
31+
import { describe, it, expect, vi, beforeEach } from 'vitest';
32+
import { Hono } from 'hono';
33+
34+
const seen: unknown[] = [];
35+
36+
const mockDispatcher = {
37+
getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', routes: {} }),
38+
handleAuth: vi.fn(async () => ({ handled: true, response: { status: 200, body: { ok: true } } })),
39+
dispatch: vi.fn(async (_method: string, _path: string, _body: unknown, query: unknown) => {
40+
seen.push(query);
41+
return { handled: true, response: { status: 200, body: { ok: true } } };
42+
}),
43+
};
44+
45+
vi.mock('@objectstack/runtime', () => ({
46+
HttpDispatcher: function HttpDispatcher() { return mockDispatcher; },
47+
}));
48+
49+
import { createHonoApp } from './index';
50+
51+
const PREFIX = '/api/v1';
52+
const app = createHonoApp({ kernel: { name: 'test-kernel' } as any, prefix: PREFIX, cors: false });
53+
54+
/** What `dispatch()` was handed as its query for one request through the catch-all. */
55+
async function dispatchedQuery(search: string): Promise<any> {
56+
seen.length = 0;
57+
const res = await app.request(`http://localhost${PREFIX}/meta/app/crm${search}`);
58+
expect(res.status).toBe(200);
59+
expect(seen).toHaveLength(1);
60+
return seen[0];
61+
}
62+
63+
/** The catch-all's query line as it was before #23214 — the reference for "unchanged". */
64+
async function referenceQuery(search: string): Promise<Record<string, any>> {
65+
let out: Record<string, any> = {};
66+
const ref = new Hono();
67+
ref.get('/*', (c) => {
68+
const queryParams: Record<string, any> = {};
69+
const url = new URL(c.req.url);
70+
url.searchParams.forEach((val, key) => { queryParams[key] = val; });
71+
out = queryParams;
72+
return c.text('ok');
73+
});
74+
await ref.request(`http://localhost/x${search}`);
75+
return out;
76+
}
77+
78+
beforeEach(() => { seen.length = 0; });
79+
80+
describe('[#23214] a repeated query name reaches dispatch() as an array', () => {
81+
it.each([
82+
['?package=a&package=b', { package: ['a', 'b'] }],
83+
['?package=a&package=b&package=c', { package: ['a', 'b', 'c'] }],
84+
['?keepData=true&keepData=true&layers=1', { keepData: ['true', 'true'], layers: '1' }],
85+
['?status=a&type=x&status=b', { status: ['a', 'b'], type: 'x' }],
86+
])('%s', async (search, expected) => {
87+
expect(await dispatchedQuery(search)).toEqual(expected);
88+
});
89+
});
90+
91+
describe('[#23214] a single-valued name arrives exactly as the old line produced it', () => {
92+
it.each([
93+
[''],
94+
['?package=crm'],
95+
['?package=crm&preview=draft&locale=fr'],
96+
['?q=hello+world'],
97+
['?q=hello%20world&x=%E4%B8%AD'],
98+
['?na%6De=v'],
99+
['?empty='],
100+
['?flag'],
101+
['?select=id,name'],
102+
])('%j', async (search) => {
103+
const got = await dispatchedQuery(search);
104+
expect(got).toEqual(await referenceQuery(search));
105+
for (const value of Object.values(got)) expect(typeof value).toBe('string');
106+
});
107+
});
108+
109+
describe('[#23214] `__proto__` is an own key, and no prototype moves', () => {
110+
it('keeps one `__proto__` as an own data property', async () => {
111+
const got = await dispatchedQuery('?__proto__=x&polluted=1');
112+
expect(Object.prototype.hasOwnProperty.call(got, '__proto__')).toBe(true);
113+
expect(Object.getOwnPropertyDescriptor(got, '__proto__')?.value).toBe('x');
114+
expect(Object.getPrototypeOf(got)).toBe(Object.prototype);
115+
expect(({} as any).polluted).toBeUndefined();
116+
expect(got.polluted).toBe('1');
117+
});
118+
119+
it('keeps a repeated `__proto__` as an own array', async () => {
120+
const got = await dispatchedQuery('?__proto__=x&__proto__=y');
121+
expect(Object.getOwnPropertyDescriptor(got, '__proto__')?.value).toEqual(['x', 'y']);
122+
expect(Object.getPrototypeOf(got)).toBe(Object.prototype);
123+
});
124+
});

‎packages/adapters/hono/src/index.ts‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,44 @@ async function readDispatchBody(c: Context): Promise<any> {
328328
}
329329
}
330330

331+
/**
332+
* The query the catch-all hands `dispatch()`: a REPEATED name as an array, a
333+
* single-valued one as a plain string (#23214).
334+
*
335+
* The twin is `readQuery` in `@objectstack/plugin-hono-server`
336+
* (`src/adapter.ts`), which every `plugin-hono-server` route reads its query
337+
* through. It is mirrored here, not imported: it is module-private there, and
338+
* that package's `exports` has only `.`. Keep the two in step.
339+
*
340+
* The catch-all used to copy `URLSearchParams.forEach` into an object, so a
341+
* repeated name arrived as its LAST value: `?package=a&package=b` reached a
342+
* dispatcher domain as `'b'`. Every arity rule the dispatcher declares
343+
* (`repeatedQueryParamMessage`, answered `400` through `plugin-hono-server`
344+
* and the node reference adapter) was unreachable on a `createHonoApp` host,
345+
* and the request was answered with one of the two values the caller sent.
346+
* The cli-lane ruling on #6878 decided the direction for every adapter: the
347+
* handler must be able to SEE the ambiguity in order to refuse it.
348+
*
349+
* ⚠️ The normalisation is NOT optional. `c.req.queries()` returns an array
350+
* for EVERY key, single-valued ones included (measured on hono 4.13:
351+
* `?a=1&a=2&b=3` gives `{ a: ['1', '2'], b: ['3'] }`), so a bare swap would
352+
* turn every single-value read point in the dispatcher into an array.
353+
*
354+
* Built with `Object.fromEntries`, never dynamic property writes: the keys
355+
* come straight off the wire, and `fromEntries` creates own data properties
356+
* only, so `?__proto__=x` is an own key named `__proto__` and no prototype is
357+
* touched. The old `queryParams[key] = val` write silently dropped that name.
358+
*/
359+
function readDispatchQuery(c: Context): Record<string, string | string[]> {
360+
const all: Record<string, string[]> = c.req.queries() ?? {};
361+
return Object.fromEntries(
362+
Object.entries(all).map(([key, values]) => [
363+
key,
364+
values.length > 1 ? values : values[0],
365+
]),
366+
) as Record<string, string | string[]>;
367+
}
368+
331369
/**
332370
* Creates a full-featured Hono app with all ObjectStack route dispatchers.
333371
*
@@ -784,9 +822,8 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
784822
body = await readDispatchBody(c);
785823
}
786824

787-
const queryParams: Record<string, any> = {};
788-
const url = new URL(c.req.url);
789-
url.searchParams.forEach((val, key) => { queryParams[key] = val; });
825+
// [#23214] A repeated name stays an array (`readDispatchQuery`).
826+
const queryParams = readDispatchQuery(c);
790827

791828
const result = await dispatcher.dispatch(method, subPath, body, queryParams, { request: c.req.raw }, prefix);
792829
return toResponse(c, result);

0 commit comments

Comments
 (0)