Skip to content

Commit 96e7a43

Browse files
fix(runtime)!: the package draft doors refuse a query parameter they do not read (#26853)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XAR1TDS8qyw6DaeA7t9jepP)_ Fixes #26713 Clause-②: yes (narrowing) Metadata-protocol: none Skills-docs: none — no skill or doc teaches a query parameter on either draft door: `skills/**` and `content/docs` name `publish-drafts` / `discard-drafts` only in `plugin-hooks.md`, `metadata-api.mdx` (a route list) and two reference tables, none with a query string, and no caller in `ui/**`, `packages/client`, the CLI or `skills/**` sends one. ## 维护者速读 - 改了什么:包的两个草稿门 `POST /packages/:id/publish-drafts` 与 `discard-drafts` 现在拒收任何查询参数,答 `400 VALIDATION_ERROR`,文案用仓里唯一的那句「不认识的查询参数」。 - 为什么改:`publish-drafts?dryRun=true` 以前忽略参数、真的发布草稿并答 200 `published`;想预览的工具与 AI 拿到的是不可逆的发布。 - 风险与回滚:只拒带查询参数的请求;控制台、`MetadataClient`、SDK 都不带参数,行为不变。回滚即还原 `packages.ts` 一处 helper 与两行调用。 - 先看:`packages/runtime/src/domains/packages.ts` 的 `refuseDraftDoorQuery` 与两处调用(在权限与 wall 闸之后、协议之前);其次新测试 `packages-draft-doors-closed-query.test.ts`。 - 两分钟验证:`cd packages/runtime && pnpm exec vitest run src/domains/packages-draft-doors-closed-query.test.ts`,预期 10 passed;带 `?dryRun=true` 的两例答 400 且协议未被调用。 - 需要你做的:无,席位落地。 ## What changes - `packages/runtime/src/domains/packages.ts`: the draft doors' closed query set is declared (`DRAFT_DOOR_QUERY_PARAMS`, measured EMPTY: both doors read the path id, the session and the body, never the query), and `refuseDraftDoorQuery` answers any name on the query string `400 VALIDATION_ERROR` with `unknownQueryParamRefusalMessage` from `@objectstack/rest`, the recognition sentence every closed-query door answers, written through `deps.error` with the code the way the `/meta` doors write it (#23072). No new sentence, no new code, no helper copied. - Placement: after `requireManageMetadata` and `refuseWalledCatalogPackageWrite`, before the protocol is resolved, the #23214 placement on this domain's other write doors. A caller without `manage_metadata` keeps its `403`. - `discard-drafts` is closed in the same change: it is the twin door the order names, reads no query either, and `?dryRun=true` there dropped every pending draft. Its hunk is the same one call line. - ⛔ No `dryRun`. A dry run would be a declared parameter and a protocol addition, which goes back to triage. - Transports: only the runtime dispatcher serves these two doors (`dispatcher-plugin.ts` `mountPackagesRoute`, and the `@objectstack/hono` catch-all into the same `dispatch()`); `RestServer` mounts neither (`packages/rest/src` has no route for them), so there is one place to close. - Changeset `@objectstack/runtime` patch, with the BREAKING banner and the ADR-0087 `not-required (no-migration-prescription)` disposition. ## Why The QA run (S3) measured `POST /api/v1/packages/:id/publish-drafts?dryRun=true` with `{}` answering `200 outcome: "published"` with the draft live. A caller asking for a preview got the irreversible act, shaped like a success. Triage `6446824931` ruled: close the query set and refuse an unknown parameter with 400. ## Tests `packages/runtime/src/domains/packages-draft-doors-closed-query.test.ts`, 10 cases, driving `dispatch()` over a real `SchemaRegistry` and a recording protocol, for each door: - refusal: `{dryRun: 'true'}`, a repeated `dryRun`, and two unknown names each answer 400 `VALIDATION_ERROR` with exactly `unknownQueryParamRefusalMessage(query, [])`, every name quoted, and the protocol's `publishPackageDrafts` / `discardPackageDrafts` never called; - preservation: no query, 200, the protocol called once; - order: a caller without `manage_metadata` sending `?dryRun=true` gets 403, protocol not called. There is no declared flag on either door, so the order's "repeated declared flag still refused the #23214 way" has nothing to pin here; a repeated unknown name is answered by recognition, which runs first (the `query-allowlist.ts` rule). <details><summary>Red first on base c8583f8, green on head, ablation</summary> - Red, base `c8583f80` (implementation absent): the new file 6 failed | 4 passed (`expected 400, got {"status":200,..."outcome":"published"...}` and the discard twin `discardedCount: 1`). - Real-door probe (one-off, not committed; real `ObjectStackProtocolImplementation` over SQLite, one pending view draft, `handlePackages('/com.example.repairs/publish-drafts', 'POST', {}, {dryRun:'true'})`): - base: `{"status":200,"code":null,"outcome":"published","draftsBefore":["view/repairs_board"],"draftsAfter":[],"servedLabel":"Board v1"}`: the draft went live. - head: `{"status":400,"code":"VALIDATION_ERROR","message":"The \"dryRun\" query parameter is not supported by this endpoint. ... This endpoint takes no query parameters.","draftsBefore":["view/repairs_board"],"draftsAfter":["view/repairs_board"],"servedLabel":null}`: nothing published, the draft still pending. - Ablation on head `e542b231` through `scripts/ablation-replace.mjs` (anchor `if (message === undefined) return null;` → `... || true ...`, anchor 1 → 0, blob `8dcfe74db65c` → `f592f4389363`): 6 failed | 4 passed, exactly the six refusal cases; restored blob == HEAD, `git diff HEAD` empty. The test imports the dispatcher from `src`, so no build sits between the mutation and the run. </details> ## Acceptance notes Sibling package write doors, measured on `c8583f80` (none of them is changed here): | door | closed query set? | |---|---| | `POST /meta/:type/:name/publish` (per-item publish, `RestServer`) | yes, `META_ITEM_PUBLISH_PARAMS` (#22877) | | `POST /packages` (install), reads `overwrite` | no; repeats refused (#23214), unknown names ignored | | `DELETE /packages/:id` (uninstall), reads `keepData` | no; repeats refused (#23214), unknown names ignored | | `PATCH /packages/:id`, `PATCH /:id/enable`, `PATCH /:id/disable`, `POST /:id/publish`, `/:id/revert`, `/:id/rollback`, `/:id/commits/:c/revert`, `/:id/adopt-orphans`, `/:id/duplicate` | no; none reads the query | | `POST /packages/publish` (`RestServer` package route) | no; reads no query | Measured at the dispatcher door with the `packages-write-flag-arity` harness: `DELETE /packages/:id?keepdata=true` (and `?keep_data=true`) answers 200 and calls `deletePackage({ packageId })` with no `keepData`, so the protocol drops the object tables the caller asked to keep. That is reported to the seat as a proposed card, not widened into this PR. ## Verification All on head `e542b231` (base `c8583f80`). - `pnpm --filter @objectstack/runtime test`: Test Files 460 passed (460), Tests 7458 passed | 19 skipped; lock VERDICT command-exit 0. - `pnpm --filter @objectstack/runtime typecheck`: exit 0. - Lint scoped to the touched package: `eslint packages/runtime --no-inline-config` exit 0 (the repo-wide `pnpm lint` is CI's). - Derived gates: `node scripts/pm/dispatch-gates.mjs --commands` on this diff named 60 families; all 60 ran and exited 0; `--ran` reconciliation: `60 derived famil(ies) accounted for — 60 run, 0 NOT-MEASURED`. Two first-pass exit 3s were container prerequisites, re-run after fixing them: `check-plugin-teardown-shape.mjs --self-test` (positive-control commit fetched) and `check:dual-build-cjs-loads` (the seven packages without `dist/` built: 112 entries across 69 packages load). - Hand gates: `check-commit-card-trailers.mjs --range origin/main..HEAD` exit 0, `check-nul-bytes.mjs` exit 0. - Full dogfood battery (the `(narrowing)` arm), `pnpm --filter @objectstack/dogfood test` under the lock after its `^...` closure build: Test Files 314 passed | 3 skipped (317), Tests 2609 passed | 20 skipped (2629); VERDICT command-exit 0. - `test:repo` (vitest `repo` project): not run locally; the diff touches no repo-project test and the derived list does not name it; CI Test Core runs it. - Merge check against `origin/main` `8eef9a8d`: `git merge-tree` clean; no open PR for #26708 when this was opened. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No spec change.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes (scoped to `packages/runtime`, exit 0; the full sweep is CI's). - [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 (the 维护者速读 block is the dispatch contract's Chinese summary). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. Written by an os-dev card thread in session `session_01FEkQ5nX7RrMithjQTtAtVb`, dispatched by the PM loop (server seat 2). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5nX7RrMithjQTtAtVb --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5nX7RrMithjQTtAtVb)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent a20824f commit 96e7a43

3 files changed

Lines changed: 172 additions & 1 deletion

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime)!: the package draft doors refuse a query parameter they do not read, so `publish-drafts?dryRun=true` no longer publishes for real
6+
7+
Clause-②: yes (narrowing)
8+
Metadata-protocol: none
9+
10+
<!-- 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 two HTTP doors give a request that carries a query parameter they never read: it is refused instead of ignored. The remedy is a request edit (send no query parameters), never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (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). -->
11+
12+
**BREAKING** accept-set narrowing at two runtime dispatcher doors: `POST /api/v1/packages/:id/publish-drafts` and `POST /api/v1/packages/:id/discard-drafts`.
13+
14+
- **Before.** Both doors read nothing from the query string and ignored whatever was on it. `POST /packages/:id/publish-drafts?dryRun=true` answered `200 outcome: "published"` and the drafts went live; the same parameter on `discard-drafts` dropped every pending draft. A caller asking for a preview got the irreversible act, answered like a success.
15+
- **Now.** Any query parameter on either door is refused `400 VALIDATION_ERROR` before anything is published or discarded. The message is the one every closed-query door answers (`unknownQueryParamRefusalMessage`, `@objectstack/rest`): it names each parameter it did not understand and says the door takes none. The capability gate still answers first, so a caller without `manage_metadata` keeps its `403`.
16+
- **Unchanged.** A call with no query parameters, which is how the Console, `MetadataClient.publishPackageDrafts` and the client SDK's `packages.publishDrafts` / `packages.discardDrafts` call both doors. The body (`actor`, `backfill`) is read as before.
17+
- **Who sees it.** A script, tool or AI agent that adds a query parameter to either door now gets `400` instead of a publish or a discard. Neither door has a dry run; a preview of what a publish would do is not offered yet.
18+
19+
**The remedy.** Send the request without query parameters.
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#26713] The package draft doors — `POST /packages/:id/publish-drafts` and
5+
* its twin `POST /packages/:id/discard-drafts` — read no query parameter, and
6+
* now say so: any name on the query string is refused `400 VALIDATION_ERROR`
7+
* with the repo's one recognition sentence (`unknownQueryParamRefusalMessage`,
8+
* `@objectstack/rest`), before the protocol is asked for anything.
9+
*
10+
* Measured before this change, driving `dispatch()`:
11+
*
12+
* | request | answer |
13+
* |---|---|
14+
* | `POST /packages/:id/publish-drafts?dryRun=true` | `200 outcome: 'published'`; `publishPackageDrafts` ran, so the draft went live |
15+
* | `POST /packages/:id/discard-drafts?dryRun=true` | `200`; `discardPackageDrafts` ran, so the drafts were dropped |
16+
*
17+
* A caller asking for a preview got the irreversible act, answered like a
18+
* success. A dry run is not a parameter these doors declare; adding one is a
19+
* protocol change, so until then the name is refused rather than ignored.
20+
*
21+
* Both halves (AGENTS.md route rule 5): a REFUSAL pin — status, code, the
22+
* shared sentence, and the protocol never called — beside a PRESERVATION pin
23+
* on the call that sends no query.
24+
*/
25+
26+
import { describe, it, expect, vi } from 'vitest';
27+
import { SchemaRegistry } from '@objectstack/objectql';
28+
import { unknownQueryParamRefusalMessage } from '@objectstack/rest';
29+
import { HttpDispatcher } from '../http-dispatcher.js';
30+
31+
const PKG = 'com.acme.app';
32+
33+
/** Admitted by the draft doors' capability gate. */
34+
const AUTHOR = { userId: 'u_author', isSystem: false, systemPermissions: ['manage_metadata', 'studio.access'] };
35+
/** Signed in, but without `manage_metadata`. */
36+
const VIEWER = { userId: 'u_viewer', isSystem: false, systemPermissions: [] as string[] };
37+
38+
const PUBLISHED_ONE = {
39+
success: true, outcome: 'published', publishedCount: 1, failedCount: 0,
40+
published: [{ type: 'flow', name: 'nightly_rollup', version: 'sha256:aa11' }], failed: [],
41+
};
42+
const DISCARDED_ONE = {
43+
success: true, discardedCount: 1, failedCount: 0,
44+
discarded: [{ type: 'flow', name: 'nightly_rollup' }], failed: [],
45+
};
46+
47+
/** The dispatcher over a REAL registry holding one installed package, and a recording protocol. */
48+
function boot(caller: Record<string, unknown> = AUTHOR) {
49+
const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' });
50+
(registry as any).logLevel = 'silent';
51+
registry.installPackage({ id: PKG, name: PKG, namespace: 'app', version: '1.0.0', type: 'app' } as any, 'code');
52+
const protocol = {
53+
publishPackageDrafts: vi.fn(async () => JSON.parse(JSON.stringify(PUBLISHED_ONE))),
54+
discardPackageDrafts: vi.fn(async () => JSON.parse(JSON.stringify(DISCARDED_ONE))),
55+
// The ADR-0045 visibility flip after a publish: no app to unhide.
56+
getMetaItems: vi.fn(async () => ({ items: [] })),
57+
saveMetaItem: vi.fn(async () => ({ ok: true })),
58+
};
59+
const services: Record<string, unknown> = { objectql: { registry }, protocol };
60+
const get = (n: string) => services[n] ?? null;
61+
const kernel: any = {
62+
context: { getService: get, trigger: vi.fn() },
63+
getService: get,
64+
getServiceAsync: async (n: string) => get(n),
65+
};
66+
const dispatcher = new HttpDispatcher(kernel);
67+
// The seam `dispatch()` resolves identity through (as the sibling dispatch-level pins do).
68+
(dispatcher as any).timedResolveExecutionContext = async () => ({ ...caller });
69+
return { dispatcher, protocol };
70+
}
71+
72+
interface Answer { status: number; body: any }
73+
74+
async function call(dispatcher: HttpDispatcher, path: string, query: Record<string, unknown>): Promise<Answer> {
75+
const res = await dispatcher.dispatch('POST', path, {}, query, { request: { headers: {} } } as any);
76+
return { status: res.response?.status ?? (res.handled ? 200 : 0), body: res.response?.body };
77+
}
78+
79+
function expectRecognitionRefusal(answer: Answer, query: Record<string, unknown>): void {
80+
expect(answer.status, `expected 400, got ${JSON.stringify(answer)}`).toBe(400);
81+
expect(answer.body?.success).toBe(false);
82+
expect(answer.body?.error?.code).toBe('VALIDATION_ERROR');
83+
// The shared sentence, never a door-local copy: it names what was not
84+
// understood and says the door takes no query parameters.
85+
expect(answer.body?.error?.message).toBe(unknownQueryParamRefusalMessage(query, []));
86+
}
87+
88+
const DOORS = [
89+
{ door: 'publish-drafts', protocolCall: 'publishPackageDrafts' },
90+
{ door: 'discard-drafts', protocolCall: 'discardPackageDrafts' },
91+
] as const;
92+
93+
describe.each(DOORS)('[#26713] POST /packages/:id/$door — a closed, empty query set', ({ door, protocolCall }) => {
94+
it.each([
95+
[{ dryRun: 'true' }],
96+
[{ dryRun: ['true', 'true'] }],
97+
[{ dryRun: 'true', preview: '1' }],
98+
])('refuses %j before the protocol is asked', async (query) => {
99+
const { dispatcher, protocol } = boot();
100+
const answer = await call(dispatcher, `/packages/${PKG}/${door}`, query);
101+
expectRecognitionRefusal(answer, query);
102+
for (const name of Object.keys(query)) expect(answer.body.error.message).toContain(`"${name}"`);
103+
expect(protocol[protocolCall]).not.toHaveBeenCalled();
104+
});
105+
106+
it('preserves the call without a query: the protocol runs once and the door answers 200', async () => {
107+
const { dispatcher, protocol } = boot();
108+
const answer = await call(dispatcher, `/packages/${PKG}/${door}`, {});
109+
expect(answer.status).toBe(200);
110+
expect(protocol[protocolCall]).toHaveBeenCalledTimes(1);
111+
});
112+
113+
it('answers a caller without manage_metadata 403 first: the capability gate outranks the query', async () => {
114+
const { dispatcher, protocol } = boot(VIEWER);
115+
const answer = await call(dispatcher, `/packages/${PKG}/${door}`, { dryRun: 'true' });
116+
expect(answer.status).toBe(403);
117+
expect(protocol[protocolCall]).not.toHaveBeenCalled();
118+
});
119+
});

‎packages/runtime/src/domains/packages.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ import { ManifestSchema, SEMVER_2_0_0_VERSION_PATTERN, manifestIdRefusal } from
119119
// domain calls: it writes the bare `{ error: { code, message } }` body onto a
120120
// `res`, and every error body on this surface is `deps.error`'s. See the
121121
// `@objectstack/rest` barrel entry that publishes the pair.
122-
import { repeatedQueryParamMessage } from '@objectstack/rest';
122+
// [#26713] …and its recognition sibling, the one sentence for a query
123+
// parameter a door does not read (`packages/rest/src/query-allowlist.ts`),
124+
// asked by the draft doors (`refuseDraftDoorQuery`).
125+
import { repeatedQueryParamMessage, unknownQueryParamRefusalMessage } from '@objectstack/rest';
123126
// [commit 0862063ba] The repo's ONE coercion for a query parameter its schema declares
124127
// `z.boolean()`, from the module whose header is the authority on the rule
125128
// (`packages/runtime/src/query-param.ts`). Imported, never restated: the list
@@ -1062,6 +1065,34 @@ function refuseRepeatedWriteFlag(
10621065
return null;
10631066
}
10641067

1068+
/**
1069+
* [#26713] The closed query set of the package draft doors —
1070+
* `POST /packages/:id/publish-drafts` and `POST /packages/:id/discard-drafts`.
1071+
* **Measured**: EMPTY. Both read the path's id, the session and the body
1072+
* (`actor`, and `backfill` on publish); neither reads a query parameter.
1073+
*/
1074+
const DRAFT_DOOR_QUERY_PARAMS: readonly string[] = Object.freeze([]);
1075+
1076+
/**
1077+
* [#26713] AGENTS.md route rule 5 at the draft doors: a query parameter they
1078+
* do not read is refused `400 VALIDATION_ERROR` with the recognition sentence
1079+
* every closed-query door answers ({@link unknownQueryParamRefusalMessage}),
1080+
* in this domain's envelope, as the `/meta` doors write it (#23072). Asked
1081+
* after the capability and wall gates and before anything else is read.
1082+
*
1083+
* Both doors ignored the query string, so `publish-drafts?dryRun=true`
1084+
* published the drafts for real and answered `200 outcome: "published"`, and
1085+
* the same parameter on `discard-drafts` dropped them. A dry run is not a
1086+
* parameter these doors declare; until one is, the name is refused.
1087+
*
1088+
* @returns the refusal to answer, or `null` when the query names nothing.
1089+
*/
1090+
function refuseDraftDoorQuery(deps: DomainHandlerDeps, query: unknown): HttpDispatcherResult | null {
1091+
const message = unknownQueryParamRefusalMessage(query, DRAFT_DOOR_QUERY_PARAMS);
1092+
if (message === undefined) return null;
1093+
return { handled: true, response: deps.error(message, 400, { code: 'VALIDATION_ERROR' }) };
1094+
}
1095+
10651096
/**
10661097
* Whether a registry row counts as enabled, for {@link readEnabledFilter}'s
10671098
* comparison (commit 0862063ba).
@@ -1699,6 +1730,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
16991730
// [ADR-0131 D12 items 7–8] Under a wall, the platform operator's.
17001731
const walled = await refuseWalledCatalogPackageWrite(deps, _context, `Publishing the drafts of package "${id}"`);
17011732
if (walled) return walled;
1733+
const queryRefused = refuseDraftDoorQuery(deps, query); if (queryRefused) return queryRefused;
17021734
const protocol = await resolveProtocol(deps, _context);
17031735
if (protocol && typeof protocol.publishPackageDrafts === 'function') {
17041736
try {
@@ -2059,6 +2091,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
20592091
// [ADR-0131 D12 items 7–8] Under a wall, the platform operator's.
20602092
const walled = await refuseWalledCatalogPackageWrite(deps, _context, `Discarding the drafts of package "${id}"`);
20612093
if (walled) return walled;
2094+
const queryRefused = refuseDraftDoorQuery(deps, query); if (queryRefused) return queryRefused;
20622095
const protocol = await resolveProtocol(deps, _context);
20632096
if (protocol && typeof protocol.discardPackageDrafts === 'function') {
20642097
try {

0 commit comments

Comments
 (0)