Skip to content

Commit 78f78ac

Browse files
fix(metadata-protocol)!: a runtime permission-set save refuses a grant on an object that does not exist (#29093)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XMKCA9XHroSxfdBE6T4UZr6)_ Fixes #28990 Clause-②: no (narrowing) Metadata-protocol: narrowed Skills-docs: none — `skills/objectstack-data/rules/security.md` teaches permission sets declared in source, where `defineStack` already refuses a grant on an undeclared object; the runtime refusal names its own fix. ## 维护者速读 - **改了什么**:运行时保存权限集(`PUT /api/v1/meta/permission/NAME`,直接保存、草稿、发布、回滚/撤销提交)时,`objects` 里如果授权给一个运行时不存在的对象,现在返回 `422 INVALID_METADATA`,在 `objects.对象名` 点名,不再存下。和构建期 `defineStack` 的同一条规则对齐。 - **为什么改**:之前返回 200,管理员以为授权成功,实际什么都没授(例如写成 `lead` 而不是 `crm_lead`)。构建期早就硬失败,运行时这条路静默。 - **风险与回滚**:低。已有的授权不受影响:权限集当前已持有的授权键在重存、`os migrate meta --stored`、回滚时都不判;`'*'`、平台对象、同批草稿里的对象都放行。实测 showcase/crm/multi-package 三个启动里所有内置权限集重存都不会被拒。回滚即撤回 `@objectstack/metadata-protocol` 这次改动。 - **先看**:`packages/metadata-protocol/src/protocol-save-door-refusals.ts`(新增 `unknownGrantObjectSaveRefusal`);`packages/metadata-protocol/src/protocol.ts`(三处调用点与 `batchObjects`);`packages/metadata-protocol/src/protocol-authoring-gate.ts`(回滚门)。 - **两分钟验证**:`pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.unknown-grant-object-refusal.test.ts`,预期 16/16 通过(main 上 9 条红)。 - **需要你做的**:无,席位落地。 ## What changes - **`unknownGrantObjectSaveRefusal`** (`packages/metadata-protocol/src/protocol-save-door-refusals.ts`), beside #26724's `unknownDatasourceSaveRefusal` and in its shape. A `permission` body whose `objects` map has a key the runtime does not know is refused with the save door's existing envelope: `422 INVALID_METADATA`, one `issues[]` entry per unknown key at `objects.NAME`, `code: 'custom'`, opening with the build rule's own sentence ("Permission 'SET' grants on object 'NAME' which is not defined in objects"), then why (the grant applies to nothing) and the fix. No new error code, no new gate, no `packages/spec/**` edit. - **Known**, in order: the `'*'` wildcard (the evaluator's fallback entry); an object the engine's registry registers under exactly that name (`registry.getObject(key)?.name === key`, so the legacy `pkg__name` lookup does not count, and platform objects pass because they are registered, without restating `isPlatformObjectName`); an object `readMetaItem` resolves (stored row, else code layer); on a draft save, any object draft (`listDrafts`); on a package publish, an object draft of the same batch (`batchObjects`, beside `batchDatasources`, since `listDrafts` has no type order). A registry with no `getObject` refuses nothing. - **Exempt:** a grant key the set's current definition (`readMetaItem` of the set) already holds, so a re-save, `os migrate meta --stored` (which re-saves every stored row through `saveMetaItem`) or a restore that keeps it is not refused, even when the object has gone since (#26724's declaration-boot trap). - **Call sites:** `saveMetaItem` (draft and live, after the datasource refusal), `promoteDraftForPublish` (shared by `publishMetaItem` and `publishPackageDrafts`), and `refuseRestoredBodyTheSaveDoorRefuses` (rollback and commit revert), with the new member on `AuthoringGateHost`. - **Changeset** `.changeset/28990-permission-grant-unknown-object.md`: `minor` on `@objectstack/metadata-protocol`, BREAKING banner, both declaration lines, ADR-0087 `not-required (no-migration-prescription)`. - **`scripts/engine-double-contract.pinned.json`**: the new test file's engine double recorded (`check:engine-double-contract --write`). ## Tests - **`packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts`** (new, 16 cases): live, draft (with and without a package) and `force` saves refused at `objects.NAME` with nothing stored; one finding per unknown key and a short `lead` beside a registered `crm_lead`; the per-item publish of a set whose object is still a draft; `publishMetaItem` and `publishPackageDrafts` of a draft stored before the rule (batch `refused`, nothing promoted). Controls: a registered object, a platform object (`sys_user_position`), `'*'`, no grant, an object stored but not registered, an object drafted in the same package and promoted after the set, a re-save keeping a held grant on an object gone since (and adding a new unknown one is refused), a registry with no `getObject`. - **`packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts`**: one new `pinBothDoors` case: `rollbackMetaItem` and `revertCommit` refuse a version granting on an object that has gone, and restore an acceptable version (the double's `getObject` now answers a mutable object set, empty by default). - **`packages/qa/dogfood/test/permission-unknown-grant-object-refusal.dogfood.test.ts`** (new, 7 cases, real showcase over REST): the card's live `PUT` and draft `PUT` refused 422 with nothing stored; a rollback to a version granting on an object deleted since refused; controls `showcase_task`, `sys_user_position`, `'*'` save live, and an object plus a set drafted in their own package publish in one batch (`publishedCount: 2`). - **Red on base `0be247ce`** (implementation files restored from base, tests kept): unit files 11 failed / 41 passed; dogfood (metadata-protocol rebuilt from base, `ablation-dist-preflight --absent` exit 0) 3 failed / 4 passed. Every failure is a refusal pin; every control passes on both trees. Restored: `git status --porcelain` empty. - **Green on head:** unit files 52/52; dogfood file 7/7 (rebuilt, preflight present). <details><summary>Ablation (one-off, from committed HEAD)</summary> `node scripts/ablation-replace.mjs` on `protocol-save-door-refusals.ts`: the first guard of `unknownGrantObjectSaveRefusal` made to return early behind the marker `ABLATED_28990`; anchor 1 → 0, blob `dc2672fab3e1` → `a1fbab11be87`. The two unit files: 11 failed / 41 passed (the same 11 refusal pins). Restored by the tool: blob equals HEAD `dc2672fab3e1`, `git diff HEAD` empty. </details> ## Acceptance notes - **Measured first (ruling 2).** Runtime probe over three real boots (`@objectstack/verify` `bootStack`, at head of this branch; the registry listing does not depend on the change): showcase 18 sets / 217 grant keys, crm 11 / 179, multi-package 9 / 172. The only key no object answers is `sys_audit_log`, in plugin-security's shipped `organization_admin` and `organization_admin_no_bypass` (plugin-audit registers it, and the verify composition does not load plugin-audit). Re-saving every one of those shipped sets through the new refusal with its own body: **0 refused** (the held-grant exemption). hotcrm `d4ffe99`: 8 profiles, 23 distinct keys, 20 `crm_*` (declared, the build rule holds them) and `sys_comment`, `sys_activity` (plugin-audit), `sys_attachment` (platform-objects). No QA seed stores a permission row. So no shipped set is refused. - **Re-save paths:** `migrateStoredMetadata` (`os migrate meta --stored`) re-saves every stored row through `saveMetaItem` with `force` and `source: 'migrate-stored'`; the org-ownership ceremony and the restore doors re-save too. All keep the grants the stored set already holds, which the exemption passes. - **Out of scope, noted:** `fields` keys (`object.field`) are not judged here; QA record #28981 also observed a translation's `objects.no_such_object` saving 200 (different metadata type, not this card). ## Verification - `@objectstack/metadata-protocol`: `pnpm --filter @objectstack/metadata-protocol test` under the lock, VERDICT command-exit 0, Test Files 298 passed, 4 skipped / Tests 22126 passed, 26 skipped; `typecheck` exit 0; `pnpm exec eslint packages/metadata-protocol packages/qa/dogfood/test/permission-unknown-grant-object-refusal.dogfood.test.ts --no-inline-config` exit 0. - **Derived gates** (`dispatch-gates --commands`, 65 families): run at `db04498a`+changeset; at hand-back 55 had run, all exit 0 except three: `check-plugin-teardown-shape --self-test` exit 3 (shallow clone; green after fetching its fixture commit), `check:engine-double-contract` exit 1 (the new double; ledger written, recheck exit 0, committed in `7d6842b3`), `check:dual-build-cjs-loads` exit 3 PREREQUISITE NOT MET (7 packages outside this closure unbuilt): NOT MEASURED, CI runs it. The last 10 families: NOT MEASURED at hand-back (wrap-up ordered by the seat). - **Dogfood battery:** NOT MEASURED at hand-back: it was running under the lock when the seat ordered the wrap-up. - **CI** runs the full farm on this PR; `test:repo` (Test Core) is CI's, not run locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5oyJ4R1betVYFyp6SD --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent db6b408 commit 78f78ac

10 files changed

Lines changed: 727 additions & 7 deletions
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/metadata-protocol': minor
3+
---
4+
5+
fix(metadata-protocol)!: a runtime permission-set save refuses a grant on an object that does not exist
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: narrowed
9+
10+
**BREAKING** for one shape only: a `permission` set whose `objects` map grants on an object this runtime does not know. It was stored and published (200), and the grant applied to nothing, because the permission evaluator keys a grant by the object's name: a short `lead` where the object is `crm_lead` silently granted nothing. `defineStack` already refuses the same grant at build; the metadata save doors now refuse it too.
11+
12+
- **Where.** `PUT /api/v1/meta/permission/NAME`, live and `?mode=draft`; the publish of a draft (`POST /meta/permission/NAME/publish`, `POST /packages/ID/publish-drafts`); and the restore doors (`POST /meta/permission/NAME/rollback`, a commit revert).
13+
- **The refusal.** `422 INVALID_METADATA`, the save door's existing envelope, with one finding per unknown object at `objects.NAME`: "Permission 'SET' grants on object 'NAME' which is not defined in objects: …". Nothing is stored; a package publish aborts the batch.
14+
- **What counts as known.** The `'*'` wildcard; an object the runtime registers, from any package, platform objects (`sys_*`, the RBAC link tables) included; an object stored in this environment or defined in code; on a draft save, any object draft; on a package publish, an object drafted in the same batch.
15+
- **What is not judged.** A grant the set already holds: a re-save, `os migrate meta --stored` or a restore that keeps it is not refused, even if the object has gone since. Editing a shipped set is therefore never refused for a grant it ships with.
16+
- **What to do.** Correct the object name to the object's full name, create the object first (or draft it in the same package), or remove the grant.
17+
18+
<!-- adr-0087: not-required (no-migration-prescription) the diff registers no retirement and prescribes no rewrite: the refused value granted nothing, so there is no stored shape to convert; a set that already holds such a grant is not refused on re-save -->

‎packages/metadata-protocol/src/protocol-authoring-gate.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export interface AuthoringGateHost {
9090
runAuthoringGate: ObjectStackProtocolImplementation['runAuthoringGate'];
9191
tenancyPostureInForce: ObjectStackProtocolImplementation['tenancyPostureInForce'];
9292
unknownDatasourceSaveRefusal: ObjectStackProtocolImplementation['unknownDatasourceSaveRefusal'];
93+
unknownGrantObjectSaveRefusal: ObjectStackProtocolImplementation['unknownGrantObjectSaveRefusal'];
9394
seedExternalIdSaveRefusal: ObjectStackProtocolImplementation['seedExternalIdSaveRefusal'];
9495
}
9596

@@ -124,7 +125,7 @@ export function restoredBodyWriter(
124125

125126
/** The body of `ObjectStackProtocolImplementation#refuseRestoredBodyTheSaveDoorRefuses`; the method's docblock there is its contract. */
126127
export async function refuseRestoredBodyTheSaveDoorRefuses(
127-
this: Pick<AuthoringGateHost, 'anonymousElevatedFlowRefusal' | 'assertRuntimeAuthoringRules' | 'authoringChannel' | 'credentialChannelFor' | 'defaultRecordPageSaveRefusal' | 'externalBindingSaveRefusal' | 'getOverlayRepo' | 'permissionSetNameCollisionRefusal' | 'runAuthoringGate' | 'seedExternalIdSaveRefusal' | 'unknownDatasourceSaveRefusal'>,
128+
this: Pick<AuthoringGateHost, 'anonymousElevatedFlowRefusal' | 'assertRuntimeAuthoringRules' | 'authoringChannel' | 'credentialChannelFor' | 'defaultRecordPageSaveRefusal' | 'externalBindingSaveRefusal' | 'getOverlayRepo' | 'permissionSetNameCollisionRefusal' | 'runAuthoringGate' | 'seedExternalIdSaveRefusal' | 'unknownDatasourceSaveRefusal' | 'unknownGrantObjectSaveRefusal'>,
128129
type: string,
129130
name: string,
130131
body: unknown,
@@ -141,6 +142,8 @@ export async function refuseRestoredBodyTheSaveDoorRefuses(
141142
if (bindingRefusal) throw bindingRefusal;
142143
const datasourceRefusal = await this.unknownDatasourceSaveRefusal(type, body, name);
143144
if (datasourceRefusal) throw datasourceRefusal;
145+
const grantRefusal = await this.unknownGrantObjectSaveRefusal(type, body, name);
146+
if (grantRefusal) throw grantRefusal;
144147
const seedKeyRefusal = await this.seedExternalIdSaveRefusal(type, body, name);
145148
if (seedKeyRefusal) throw seedKeyRefusal;
146149
const repo = this.getOverlayRepo();

‎packages/metadata-protocol/src/protocol-save-door-refusals.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,82 @@ export async function unknownDatasourceSaveRefusal(
527527
return err;
528528
}
529529

530+
/**
531+
* The body of `ObjectStackProtocolImplementation#unknownGrantObjectSaveRefusal`;
532+
* the method's docblock there is its contract. [#28990] Written here beside
533+
* {@link unknownDatasourceSaveRefusal}, whose shape it follows: the engine's
534+
* registered objects, then the by-name read (`readMetaItem`), then the pending
535+
* drafts, then the definition the set already holds.
536+
*/
537+
export async function unknownGrantObjectSaveRefusal(
538+
this: Pick<SaveDoorRefusalHost, 'engine' | 'listDrafts' | 'readMetaItem'>,
539+
singularType: string,
540+
item: unknown,
541+
name: string,
542+
pending?: 'drafts' | ReadonlySet<string>,
543+
writeFace?: MetadataWriteFace,
544+
): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> {
545+
if (singularType !== 'permission') return undefined;
546+
if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined;
547+
const grants = (item as { objects?: unknown }).objects;
548+
if (!grants || typeof grants !== 'object' || Array.isArray(grants)) return undefined;
549+
// A registry that cannot look an object up by name cannot say a name is
550+
// unknown, so nothing is refused on it.
551+
const registry = this.engine?.registry as { getObject?: unknown } | undefined;
552+
const getObject = registry?.getObject;
553+
if (typeof getObject !== 'function') return undefined;
554+
// `'*'` is the wildcard grant the permission evaluator falls back to for
555+
// every object with no entry of its own: it names no object.
556+
let unknown = Object.keys(grants).filter((key) => {
557+
if (key === '*') return false;
558+
// The object's own name, exactly: the registry's legacy `pkg__name`
559+
// lookup answers an object the evaluator never keys a grant by.
560+
const registered = getObject.call(registry, key) as { name?: unknown } | undefined;
561+
return registered?.name !== key;
562+
});
563+
if (unknown.length === 0) return undefined;
564+
// An object the environment stores, or the code layer defines, that the
565+
// registry has not (yet) registered: known here, and judged where it is used.
566+
const declared = await Promise.all(unknown.map(async (key) =>
567+
Boolean((await this.readMetaItem({ type: 'object', name: key }))?.item)));
568+
unknown = unknown.filter((_, index) => !declared[index]);
569+
if (unknown.length === 0) return undefined;
570+
// An object still pending: any object draft, for a draft save; the batch's
571+
// own object drafts, for a package publish.
572+
if (pending instanceof Set) {
573+
unknown = unknown.filter((key) => !pending.has(key));
574+
} else if (pending === 'drafts') {
575+
const drafted = new Set((await this.listDrafts({ type: 'object' })).drafts.map((d) => d.name));
576+
unknown = unknown.filter((key) => !drafted.has(key));
577+
}
578+
if (unknown.length === 0) return undefined;
579+
// A grant the set already holds is not this save's: a re-save, a
580+
// stored-row migration or a restore that keeps it is judged at the save
581+
// that introduced it, and an object that has gone since is not the
582+
// re-saving author's to answer for. (A declaration boot never runs
583+
// `onEnable`, so what a plugin registers there is absent from it.)
584+
const current = (await this.readMetaItem({ type: 'permission', name }))?.item as { objects?: unknown } | undefined;
585+
const held = current?.objects && typeof current.objects === 'object' && !Array.isArray(current.objects)
586+
? new Set(Object.keys(current.objects))
587+
: new Set<string>();
588+
unknown = unknown.filter((key) => !held.has(key));
589+
if (unknown.length === 0) return undefined;
590+
const issues: MetadataIssueEntry[] = unknown.map((key) => ({
591+
path: `objects.${key}`,
592+
message: `Permission '${name}' grants on object '${key}' which is not defined in objects: no package registers `
593+
+ 'an object of that name and none is saved here, so the grant applies to nothing. Correct the name to the '
594+
+ `object's full name (e.g. 'crm_lead', not 'lead'), create the object first, or remove the grant.`,
595+
code: 'custom',
596+
}));
597+
const err = new Error(
598+
`${singularType}/${name} failed spec validation: ${specValidationFindings(writeFace, issues)}`,
599+
) as Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] };
600+
err.code = 'INVALID_METADATA';
601+
err.status = 422;
602+
err.issues = issues;
603+
return err;
604+
}
605+
530606
/** The body of `ObjectStackProtocolImplementation#objectsBoundExternallyTo`; the method's docblock there is its contract. */
531607
export async function objectsBoundExternallyTo(
532608
this: Pick<SaveDoorRefusalHost, 'engine' | 'readActiveOverlayRows' | 'rethrowUnlessMetadataStoreUnprovisioned' | 'storedOverlayEntries'>,

‎packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
* family of shapes after its schema gate: a hook with no `body`, a second
99
* default record page of an object, an `external` object binding on a managed
1010
* datasource (both halves of the pair), an object `datasource` that names no
11-
* datasource (#26724), a keyed seed whose `externalId` names no field of its
11+
* datasource (#26724), a permission-set grant on an object that does not
12+
* exist (#28990), a keyed seed whose `externalId` names no field of its
1213
* object (#28952), a `runAs: 'system'` flow an anonymous
1314
* endpoint names, the shared runtime authoring rules, and the authoring gate a
1415
* domain plugin registers. Measured on `origin/main` `5801a4f2c` before this
@@ -506,6 +507,28 @@ pinBothDoors({
506507
},
507508
});
508509

510+
pinBothDoors({
511+
title: 'a permission set restored with a grant on an object that does not exist now (unknownGrantObjectSaveRefusal)',
512+
type: 'permission',
513+
name: 'qa_widget_editor',
514+
code: 'INVALID_METADATA',
515+
status: 422,
516+
async stage(h) {
517+
h.objects.set('qa_widget', { name: 'qa_widget' });
518+
const set = (objects: Record<string, unknown>, label = 'Widget editor') => ({ name: 'qa_widget_editor', label, objects });
519+
await h.protocol.saveMetaItem({ type: 'permission', name: 'qa_widget_editor', item: set({ qa_widget: { allowRead: true } }) });
520+
await h.protocol.saveMetaItem({ type: 'permission', name: 'qa_widget_editor', item: set({}) });
521+
await h.protocol.saveMetaItem({ type: 'permission', name: 'qa_widget_editor', item: set({}, 'Widget editor v3') });
522+
// The object v1 granted on is gone, and the set holds no grant on it now.
523+
h.objects.delete('qa_widget');
524+
return { refused: 1, accepted: 2 };
525+
},
526+
expectDetail: (error) => {
527+
expect(error.issues).toEqual([expect.objectContaining({ path: 'objects.qa_widget', code: 'custom' })]);
528+
expect(error.issues[0].message.startsWith("Permission 'qa_widget_editor' grants on object 'qa_widget' which is not defined in objects")).toBe(true);
529+
},
530+
});
531+
509532
pinBothDoors({
510533
title: 'a seed restored keyed on a field its object no longer has (seedExternalIdSaveRefusal)',
511534
type: 'seed',

0 commit comments

Comments
 (0)