Skip to content

Commit e1ab32e

Browse files
fix(runtime)!: the package write doors refuse a query parameter they do not read (#26919)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XJHgKKRa9iGmUbp1paLnPzX)_ Fixes #26857 Clause-②: yes (narrowing) Metadata-protocol: none Skills-docs: none — no skill or doc teaches a query parameter these doors would now refuse: `skills/**` names a package write door only in `plugin-hooks.md` (a `publish-drafts` route mention, no query string); `content/docs` teaches `?keepData=true` (`kernel/contracts/metadata-service.mdx`, the generated `references/api/package-api.mdx`) and the QA checklist `?overwrite=true`, both declared names that keep working. ## 维护者速读 - 改了什么:包的全部写门(卸载、安装、PATCH、启停、发布、回退、回滚、领养孤儿、复制,以及 RestServer 的 `POST /packages/publish`)现在只认自己声明的查询参数,其余一律答 `400 VALIDATION_ERROR`,文案用仓里唯一那句「不认识的查询参数」。 - 为什么改:`DELETE /packages/:id?keepdata=true`(小写拼错)以前被当成没写,卸载照常删掉了调用方要保留的对象表并答 200;真实 SQLite 探针在 main 上复现了「表没了」。 - 风险与回滚:只拒带未声明参数的请求;`?keepData=true`、`?overwrite=true` 与不带参数的调用照旧,控制台与 SDK 都在此列。回滚即还原 `packages.ts` 的 helper 与各门入口一行、`package-routes.ts` 一处。 - 先看:`packages/runtime/src/domains/packages.ts` 的 `refusePackageWriteDoorQuery` 与三个声明集,和 DELETE 门入口那一行(在权限与 wall 闸之后、注册表与协议之前);其次 `packages/rest/src/package-routes.ts` 的发布路由。 - 两分钟验证:`cd packages/runtime && pnpm exec vitest run src/domains/packages-write-doors-closed-query.test.ts`,预期 65 passed;其中 `?keepdata=true` 各例答 400,`deletePackage` 未被调用,包仍在注册表里。 - 需要你做的:无,席位落地。 ## What changes - `packages/runtime/src/domains/packages.ts`: #26853's `refuseDraftDoorQuery` is generalised in place into `refusePackageWriteDoorQuery(deps, query, declared)`. Same sentence (`unknownQueryParamRefusalMessage` from `@objectstack/rest`), same `400 VALIDATION_ERROR` through `deps.error`, no new code, no second helper. Each door passes its measured closed set: - `DELETE /packages/:id`: `PACKAGE_DELETE_QUERY_PARAMS = ['keepData']` (the door reads `query.keepData` and nothing else). - `POST /packages` (install): `PACKAGE_INSTALL_QUERY_PARAMS = ['overwrite']` (every other install option rides the body). - every other write door: `PACKAGE_WRITE_NO_QUERY_PARAMS = []` (`PATCH /packages/:id`, `…/enable`, `…/disable`, `POST …/publish`, `…/publish-drafts`, `…/discard-drafts`, `…/revert`, `…/rollback`, `…/commits/:commitId/revert`, `…/adopt-orphans`, `…/duplicate`). Measured from the handlers and the helpers they call (`requireWritablePackage`, `requireDuplicableSource`, `refuseWalledCatalogPackageWrite`, `resolveProtocol`, `installBodyRefusal`): none reads the query. The two draft doors now route through the generalised form; `DRAFT_DOOR_QUERY_PARAMS` became `PACKAGE_WRITE_NO_QUERY_PARAMS`. - Placement on each door: right after `requireManageMetadata` and `refuseWalledCatalogPackageWrite`, before the registry or the protocol is read or called (the #23214 / #26853 placement), so a refused call deletes, installs or changes nothing. On install it also runs ahead of the body parse. A repeated declared flag stays refused by `refuseRepeatedWriteFlag` at the place it has today; recognition runs before it (AGENTS.md route rule 5), so `?keepData=true&keepData=true&keepdata=true` is answered as the unknown name. - `packages/rest/src/package-routes.ts`: `RestServer`'s `POST /packages/publish` is still served (the registrar's one route), and it cannot reach the runtime helper. It declares `PACKAGE_PUBLISH_PARAMS = []` and answers the same `unknownQueryParamRefusalMessage` sentence after its capability gate and before the package service is resolved. It writes it through the module's own `sendError`, because this surface answers every body through that wrapped envelope; `refuseUnknownQueryParams` writes the bare nested body the `/meta` routes use. Same sentence, same code, no third phrasing. - Read-only `GET` doors are not touched. - Changeset: `@objectstack/runtime` and `@objectstack/rest` patch, `fix(runtime)!:` with both declaration lines and the ADR-0087 `not-required (no-migration-prescription)` disposition. ### Caller census (this is a narrowing, so measured before the first edit, at `e78d7057`) Every call to these doors in `ui/**`, `packages/client*/**`, `packages/cli/**`, `skills/**`, `examples/**`, `content/docs/**`, `docs/**` and the repo's test and dogfood trees that sends a query parameter: - `ui/packages/app-shell/src/views/metadata-admin/PackagesPage.tsx`: `DELETE …?keepData=true` (structure-only delete). Declared, unchanged. - `docs/qa/platform-checklist/areas/api-backend.json`, `platform-core.json`: `POST /api/v1/packages?overwrite=true`, and a doubled `keepData` arity probe. Declared, unchanged (the arity refusal keeps its sentence). - `packages/runtime/src/domain-handler-registry.test.ts`: `POST /packages` with `{ overwrite: 'true' }`. Declared. - `packages/client` (`packages.install/uninstall/enable/disable/update/publish/publishDrafts/discardDrafts/revertCommit/rollback/revert/adoptOrphans/duplicate`): no query on any write door (install sends `overwrite` in the body). The client fetch wrapper adds no cache-buster. - Console `PackagesPage`, `PackageFormDialog`, `StudioDesignSurface`, `packages-io` (`duplicate`), `commitHistory` (`commits/:c/revert`), `MetadataClient.publishPackageDrafts`: no query. - `packages/cli`: no call to these doors (its `package publish` goes to `/api/v1/cloud/packages`). No caller sends a name a door now refuses: nothing to fix, no blocker. ## Tests - New `packages/runtime/src/domains/packages-write-doors-closed-query.test.ts`, 65 cases, through `dispatch()` over a real `SchemaRegistry` with a recording protocol and metadata service (the `packages-write-flag-arity` harness): - `DELETE /packages/:id`: `?keepdata=true`, `?keep_data=true`, `?KeepData=1`, `?keepData=true&keepdata=true`, `?dryRun=true` each answer `400 VALIDATION_ERROR` with exactly `unknownQueryParamRefusalMessage(query, ['keepData'])`; `deletePackage` is never called; the package is still in the registry. Recognition outranks arity. Preservation: `keepData=true` and `keepData=1` still reach `deletePackage({ packageId, keepData: true })`. - `POST /packages`: `overwrite=true` still overwrites an installed id (`201`); a misspelled `overWrite` is refused before the install instead of answering the `409` it read as absent. - Per door, all eleven: three refusal shapes (an unknown name, the same name repeated, two unknown names) with the act never performed; the call without a query performs the act once and answers as today; a caller without `manage_metadata` still gets `403` first. - New `packages/rest/src/package-publish-closed-query.test.ts`, 5 cases: refusal (status, code, the shared sentence, `publish` never called), preservation (`200`, `publish` once), `403` first. - The existing `packages-draft-doors-closed-query.test.ts` (10) and `packages-write-flag-arity.test.ts` (11) pass unchanged. ## Acceptance notes - `Clause-②:` is the claim's line, copied as the os-dev rule requires. Measured on the diff: the accept set narrows and no public surface grows (no export, route or key added), which is the reading the `(narrowing)` arm describes; the seat may prefer `no (narrowing)`, as the line's own question (does this widen the accept set or grow the public surface) answers no. `Check Changeset` reads `none` as `patch` either way. - `packages/client`'s `packages.uninstall(id)` exposes no keep-data option, so an SDK caller cannot keep the tables at all. Not in scope here (no misbehaviour at a door); noted for the seat. ## Verification All readings at head `5ce4a3c1`, base `e78d7057`. Evidence: the project's `pm-orders/server/evidence/26857/`. <details><summary>Red first on base: 40 runtime and 3 rest cases fail, exactly the refusal cases</summary> `packages.ts` and `package-routes.ts` restored from `e78d7057` (tree only), the new tests at head: - runtime (`packages-write-doors-closed-query` + the draft-door and arity files): `Test Files 1 failed | 2 passed (3)`, `Tests 40 failed | 46 passed (86)`. First failure: `expected 400, got {"status":200,"body":{"success":true,"data":{"packageId":"com.acme.app","success":true,"registryRemoved":true,"persisted":{"success":true}}}}` for `?keepdata=true`. The misspelled install flag: `expected 400, got … 409 RESOURCE_CONFLICT "Package 'com.acme.app' already exists"`. - rest (`package-publish-closed-query`): `Tests 3 failed | 2 passed (5)`. Every preservation and `403` case passed on base, so the refusals are the only thing the diff moves. </details> <details><summary>Real door, real tables: base drops the table on ?keepdata=true, head keeps it</summary> One-off probe, not committed (`zz-probe-26857.test.ts` in the evidence folder): the dispatcher over `ObjectStackProtocolImplementation` on SQLite, a package owning one object `repairs_ticket` with one row, then `DELETE /packages/com.example.repairs` with each query. | query | base `e78d7057` | head `5ce4a3c1` | |---|---|---| | `keepdata=true` | `200`, table gone | `400 VALIDATION_ERROR` naming `keepdata`, table and its 1 row kept | | `keepData=true` (control) | `200`, table and row kept | `200`, table and row kept | | none (control) | `200`, table dropped | `200`, table dropped | </details> <details><summary>Ablation: the helper forced to answer nothing reds the refusal cases only</summary> `node scripts/ablation-replace.mjs` on `packages.ts`, anchor `const message = unknownQueryParamRefusalMessage(query, declared);` replaced by a constant `undefined` (anchor 1 to 0, blob `3bb82a0d7c5d` to `8d620d85fe51`): `Test Files 2 failed (2)`, `Tests 46 failed | 29 passed (75)` over the new file and the draft-door file (40 + 6 refusal cases red, every preservation and `403` case green). Restored: blob after restore `3bb82a0d7c5d` = HEAD, `git diff HEAD` empty. The tests import the runtime source, so no build sits between. </details> <details><summary>Green at head: package suites, typecheck, lint, test:repo</summary> - `pnpm --filter @objectstack/runtime test`: exit 0, `Test Files 462 passed (462)`, `Tests 7532 passed | 19 skipped (7551)`. - `pnpm --filter @objectstack/rest test`: exit 0, `Test Files 329 passed (329)`, `Tests 6518 passed | 326 skipped (6844)`. - `pnpm --filter @objectstack/runtime --filter @objectstack/rest typecheck`: exit 0. - `pnpm exec eslint packages/runtime packages/rest --no-inline-config`: exit 0. - `vitest run --project repo` in `packages/runtime`, `packages/rest`, `packages/core`: exit 0 each (6, 5 and 3 files; 83, 280 + 18 skipped, 947 tests). - `node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD`: exit 0. </details> <details><summary>Derived gates: 61 of 61 run, all exit 0</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `5ce4a3c1` derived 61 families; every one run, exit code captured before any pipe, all exit 0. Two had container prerequisites on the first pass (exit 3) and were re-run after meeting them: `check-plugin-teardown-shape --self-test` (positive control `621a4876` fetched; 48 cases pass) and `check:dual-build-cjs-loads` (7 packages without `dist` built; 112 entries across 69 packages load). `--ran`: `61 derived famil(ies) accounted for — 61 run, 0 NOT-MEASURED`. </details> <details><summary>Full dogfood once at head: 332 of 333 run files green; the one red never touches a package door and passes alone</summary> Closure built (`pnpm --filter '@objectstack/dogfood^...' build`, exit 0), then `pnpm --filter @objectstack/dogfood test` under the verify lock at `5ce4a3c1`: VERDICT command-exit 1 (held 19m00s), `Test Files 1 failed | 332 passed | 3 skipped (336)`, `Tests 2 failed | 2704 passed | 20 skipped (2726)`. The red file is `test/packaged-action-disabled-dispatch-live.dogfood.test.ts` (acceptance[0]: `expected true to be false` on `after.done`; acceptance[5]: a different `sys_metadata` row id after the cold boot). It drives the packaged-action disable switch over `/actions`, `/meta` and MCP and calls no `/packages` route, so this diff does not reach it. Re-run alone at the same head and build: `Test Files 1 passed (1)`, `Tests 8 passed (8)`. Read as order or load dependent within the full battery on a shared box, not as this change's consequence. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5nZMs618JppKUMdDh4)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8074feb commit e1ab32e

5 files changed

Lines changed: 398 additions & 22 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/runtime': patch
3+
'@objectstack/rest': patch
4+
---
5+
6+
fix(runtime)!: the package write doors refuse a query parameter they do not read, so `DELETE /packages/:id?keepdata=true` no longer drops the tables the caller asked to keep
7+
8+
Clause-②: yes (narrowing)
9+
Metadata-protocol: none
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition, stored row or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What changes is the answer the package write doors give a request that carries a query parameter they never read: it is refused instead of ignored. The remedy is a request edit (spell the declared flag exactly, or send no query parameter), 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). -->
12+
13+
**BREAKING** accept-set narrowing at the package write doors of the runtime dispatcher, and at `RestServer`'s `POST /api/v1/packages/publish`.
14+
15+
- **Before.** Each door read the query names it knew and ignored every other one. `DELETE /api/v1/packages/:id?keepdata=true` (or `?keep_data=true`) was not `?keepData=true`, so the uninstall dropped the package's object tables, the ones the caller asked to keep, and answered `200`. Every other write door performed its act whatever names rode on the query string.
16+
- **Now.** Each door declares the names it reads and refuses any other `400 VALIDATION_ERROR` before anything is deleted, installed or changed. The message is the one every closed-query door answers (`unknownQueryParamRefusalMessage`, `@objectstack/rest`): it names each parameter it did not understand and lists the ones the door supports. The capability gate still answers first, so a caller without `manage_metadata` keeps its `403`, and a repeated declared flag keeps its `supplied N times` refusal.
17+
- **The declared names.** `DELETE /packages/:id`: `keepData`. `POST /packages` (install): `overwrite`. Every other write door takes none: `PATCH /packages/:id`, `PATCH …/enable`, `PATCH …/disable`, `POST …/publish`, `…/publish-drafts`, `…/discard-drafts`, `…/revert`, `…/rollback`, `…/commits/:commitId/revert`, `…/adopt-orphans`, `…/duplicate`, and `RestServer`'s `POST /packages/publish`.
18+
- **Unchanged.** A call with no query parameter, and the declared flags (`?keepData=true` / `?keepData=1`, `?overwrite=true`), which is how the Console and the client SDK call these doors. Read-only `GET` doors are not touched.
19+
- **Who sees it.** A script, tool or AI agent that misspells a flag or adds any other query parameter to a package write door now gets `400` instead of the act.
20+
21+
**The remedy.** Spell the declared flag exactly (`keepData`, `overwrite`), or send the request without query parameters.
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#26857] `POST /packages/publish` declares its closed query set — EMPTY —
5+
* and refuses any name on the query string `400 VALIDATION_ERROR` with the
6+
* repo's one recognition sentence (`unknownQueryParamRefusalMessage`), before
7+
* the package service is resolved or asked to publish.
8+
*
9+
* Measured before this change: `POST /api/v1/packages/publish?dryRun=true`
10+
* published for real and answered `200`; the handler never read the query.
11+
*
12+
* Both halves (AGENTS.md route rule 5): a REFUSAL pin — status, code, the
13+
* shared sentence, and `publish` never called — beside a PRESERVATION pin on
14+
* the call without a query.
15+
*/
16+
17+
import { describe, it, expect, vi } from 'vitest';
18+
import { registerPackageRoutes } from './package-routes.js';
19+
import { unknownQueryParamRefusalMessage } from './query-allowlist.js';
20+
21+
const AUTHOR = { resolveExecutionContext: async () => ({ userId: 'u_pkg', systemPermissions: ['manage_metadata'] }) };
22+
const VIEWER = { resolveExecutionContext: async () => ({ userId: 'u_viewer', systemPermissions: [] as string[] }) };
23+
24+
const BODY = { manifest: { id: 'com.acme.app', version: '1.0.0' }, metadata: {} };
25+
26+
function mount(options: typeof AUTHOR) {
27+
const server: any = { get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn() };
28+
const service = { publish: vi.fn().mockResolvedValue({ success: true }) };
29+
registerPackageRoutes(server, () => service as any, '/api/v1', options);
30+
const handler = server.post.mock.calls.find((args: unknown[]) => args[0] === '/api/v1/packages/publish')?.[1];
31+
expect(handler, 'POST /api/v1/packages/publish must be mounted').toBeDefined();
32+
return { handler, service };
33+
}
34+
35+
async function publish(handler: any, query: Record<string, unknown>) {
36+
const out: { status?: number; body?: any } = {};
37+
const res: any = {
38+
status(code: number) { out.status = code; return res; },
39+
json(body: any) { out.body = body; return res; },
40+
send(body: any) { out.body = body; return res; },
41+
header() { return res; },
42+
};
43+
await handler({ method: 'POST', body: BODY, params: {}, query, headers: {} }, res);
44+
return out;
45+
}
46+
47+
describe('[#26857] POST /packages/publish — a closed, empty query set', () => {
48+
it.each([
49+
[{ dryRun: 'true' }],
50+
[{ dryRun: ['true', 'true'] }],
51+
[{ dryRun: 'true', force: '1' }],
52+
])('refuses %j before the package service publishes', async (query) => {
53+
const { handler, service } = mount(AUTHOR);
54+
const out = await publish(handler, query);
55+
expect(out.status, JSON.stringify(out)).toBe(400);
56+
expect(out.body?.success).toBe(false);
57+
expect(out.body?.error?.code).toBe('VALIDATION_ERROR');
58+
expect(out.body?.error?.message).toBe(unknownQueryParamRefusalMessage(query, []));
59+
expect(service.publish).not.toHaveBeenCalled();
60+
});
61+
62+
it('preserves the call without a query: publish runs once and the route answers 200', async () => {
63+
const { handler, service } = mount(AUTHOR);
64+
const out = await publish(handler, {});
65+
expect(out.status).toBe(200);
66+
expect(service.publish).toHaveBeenCalledTimes(1);
67+
});
68+
69+
it('answers a caller without manage_metadata 403 first: the capability gate outranks the query', async () => {
70+
const { handler, service } = mount(VIEWER);
71+
const out = await publish(handler, { dryRun: 'true' });
72+
expect(out.status).toBe(403);
73+
expect(service.publish).not.toHaveBeenCalled();
74+
});
75+
});

‎packages/rest/src/package-routes.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
import { classifiedRefusalAnswer, nestedRefusalEnvelope, sandboxFaultAnswer } from './error-response.js';
2222
import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js';
2323
import { logError } from './log.js';
24+
import { unknownQueryParamRefusalMessage } from './query-allowlist.js';
2425

2526
/**
2627
* [#7033 / #7023] The authorization gate for the REST package transport.
@@ -370,6 +371,13 @@ export interface PackageRoutesOptions {
370371
} | undefined>;
371372
}
372373

374+
/**
375+
* [#26857] The closed query set of `POST /packages/publish`. **Measured**:
376+
* EMPTY — the handler reads the session and the body (`manifest`,
377+
* `metadata`), never the query string.
378+
*/
379+
const PACKAGE_PUBLISH_PARAMS: readonly string[] = Object.freeze([]);
380+
373381
/**
374382
* Register the REST package management route.
375383
*
@@ -486,6 +494,16 @@ export function registerPackageRoutes(
486494
handler: async (req, res) => {
487495
try {
488496
if (await refusePackageRequest(options, req, res)) return;
497+
// [#26857] Recognition next, over the measured closed set, before the
498+
// package service is resolved or asked: an unknown name is refused
499+
// `400`, never ignored. The repo's one recognition sentence, written
500+
// through this surface's own `sendError` envelope (the module answers
501+
// every body through it), not `refuseUnknownQueryParams`' bare body.
502+
const unknownQuery = unknownQueryParamRefusalMessage(req?.query, PACKAGE_PUBLISH_PARAMS);
503+
if (unknownQuery !== undefined) {
504+
sendError(res, 400, 'VALIDATION_ERROR', unknownQuery);
505+
return;
506+
}
489507
// Resolved HERE, not at composition (#7563). Authorization runs first so
490508
// an anonymous prober cannot read a deployment's capability composition
491509
// off this seam.

0 commit comments

Comments
 (0)