From 1a62d320026ff8b412417220685aa002b7a5fb83 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 04:03:51 +0000 Subject: [PATCH 1/5] fix(runtime)!: the package disable door refuses while an enabled package's validation rule reads a field the package adds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PATCH /packages/:id/disable now answers 409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE while a validation rule of any enabled package (declared dependent or not, any severity, an app's .custom pair included) reads a field the package adds through objectExtensions, on the rule's own object or one hop through a reference field. The refusal is stamped after the read-only check and before disablePackage, names every such rule as package · object · rule in its message, carries no error.details beyond the code, and changes nothing: the row, the served objects and the environment's state file stay as they were. A formula over the field does not refuse the disable (it answers null), and a rule switched off reads nothing. The rule's reads come from analyzeRelationshipTraversals, the member reader the rule validator asks; hops are judged with referenceTargetOf. The code is held as its wire pair, which check:error-status-conformance reads as the producer. Claude-Session: https://claude.ai/code/session_016WV4cwCWp9qZ5SgMof1qLa Co-authored-by: Claude --- ...es-disable-module-extension-fields.test.ts | 208 +++++++++++-- packages/runtime/src/domains/packages.ts | 291 ++++++++++++++++++ 2 files changed, 477 insertions(+), 22 deletions(-) diff --git a/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts b/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts index ca56714e8c5..87d93d0de04 100644 --- a/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts +++ b/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts @@ -33,6 +33,16 @@ * disable never makes the storage layer lose a column. Pinned on a boot that * starts with the module already disabled (the environment's state file): the * column is created anyway, so an enable needs no re-sync before a write. + * + * ## An enabled dependent rule refuses the disable (#30379 ruling A) + * + * While a validation rule of any other ENABLED package (declared dependent or + * not; any severity; an app's `.custom` pair included) reads one of the + * module's extension fields, the disable door answers `409` + * `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, its message naming every + * such rule as package · object · rule, and nothing changes: once withheld, + * the field could not be read and every write to the object would be refused. + * A formula over the field does not refuse the disable: it answers `null`. */ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -47,7 +57,7 @@ import { composeStacks, defineStack } from '@objectstack/spec'; import { resolveThrownHttpError } from '@objectstack/types'; import { AppPlugin } from '../app-plugin.js'; import { HttpDispatcher } from '../http-dispatcher.js'; -import { setPackageDisabled } from '../package-state-store.js'; +import { loadDisabledPackageIds, setPackageDisabled } from '../package-state-store.js'; const ENVIRONMENT_ID = 'env_suite_extension_fields'; const ADMIN = { @@ -65,22 +75,45 @@ const CREDIT = 'qx_credit'; /** The module's roll-up summary of its own orders, on the app's object. */ const ORDER_TOTAL = 'qx_order_total'; const MODULE_FIELDS = [CREDIT, ORDER_TOTAL] as const; -/** A second, ENABLED module whose formula and rule read the first module's field. */ +/** A second, ENABLED module whose formula reads the first module's field. */ const SCORING = 'com.qa.extsuite.scoring'; const CREDIT_BAND = 'qx_credit_band'; /** [#30377] A third module whose roll-up, added to the app's object by extension, sums a child the APP owns. */ const ROLLUP = 'com.qa.extsuite.visits'; const VISIT = 'qx_visit'; const VISIT_TOTAL = 'qx_visit_total'; +/** + * [#30379] A fourth, ENABLED module whose validation rules read the first + * module's field: on the app's object, at error and warning severity (the + * warning a `conditional` whose `when` reads it), one switched off, and one + * on the app's visit reading it one hop through the visit's account lookup. + */ +const RULES = 'com.qa.extsuite.rules'; +const RULE_NOT_NEGATIVE = 'qx_credit_not_negative'; +const RULE_REVIEW = 'qx_credit_review'; +const RULE_SWITCHED_OFF = 'qx_credit_switched_off'; +const RULE_VISIT_HOP = 'qx_visit_account_credit'; +/** The app's paired customization package, where a tenant-written rule lives. */ +const PAIR = `${APP}.custom`; +const RULE_TENANT = 'qx_credit_tenant_cap'; + +interface SuiteOptions { + /** {@link SCORING}: a formula over the module's field. */ + scoring?: boolean; + /** {@link ROLLUP}: a roll-up over the app's visits. */ + rollup?: boolean; + /** {@link RULES}: validation rules over the module's field. */ + rules?: boolean; +} -function suiteArtifact(withScoring = false, withRollup = false) { +function suiteArtifact({ scoring = false, rollup = false, rules = false }: SuiteOptions = {}) { const app = defineStack({ manifest: { id: APP, name: 'QA Ext Suite', namespace: 'qx', version: '1.0.0', type: 'app' }, objects: [{ name: ACCOUNT, label: 'Account', fields: { name: { name: 'name', type: 'text', label: 'Name' } }, - }, ...(withRollup ? [{ + }, ...(rollup || rules ? [{ name: VISIT, label: 'Visit', fields: { @@ -115,7 +148,7 @@ function suiteArtifact(withScoring = false, withRollup = false) { }, }], } as any); - const scoring = defineStack({ + const scoringModule = defineStack({ manifest: { id: SCORING, name: 'QA Ext Suite Scoring', namespace: 'qx', version: '1.0.0', type: 'module', dependencies: { [APP]: '^1.0.0' }, @@ -128,13 +161,35 @@ function suiteArtifact(withScoring = false, withRollup = false) { expression: `record.${CREDIT} > 100 ? "high" : "low"`, }, }, + }], + } as any); + const ruleModule = defineStack({ + manifest: { + id: RULES, name: 'QA Ext Suite Rules', namespace: 'qx', version: '1.0.0', type: 'module', + dependencies: { [APP]: '^1.0.0' }, + }, + objectExtensions: [{ + extend: ACCOUNT, validations: [{ - name: 'qx_credit_not_negative', type: 'script', severity: 'error', message: 'Credit cannot be negative', + name: RULE_NOT_NEGATIVE, type: 'script', severity: 'error', message: 'Credit cannot be negative', condition: `record.${CREDIT} < 0`, + }, { + name: RULE_REVIEW, type: 'conditional', severity: 'warning', message: 'A large credit needs a capitalised name', + when: `record.${CREDIT} > 1000`, + then: { name: 'qx_credit_review_name', type: 'format', field: 'name', regex: '^[A-Z]', message: 'Capitalise the name' }, + }, { + name: RULE_SWITCHED_OFF, type: 'script', active: false, message: 'Switched off', + condition: `record.${CREDIT} > 99999999`, + }], + }, { + extend: VISIT, + validations: [{ + name: RULE_VISIT_HOP, type: 'script', severity: 'info', message: 'The account carries no credit', + condition: `record.account.${CREDIT} < 0`, }], }], } as any); - const rollup = defineStack({ + const rollupModule = defineStack({ manifest: { id: ROLLUP, name: 'QA Ext Suite Visits', namespace: 'qx', version: '1.0.0', type: 'module', dependencies: { [APP]: '^1.0.0' }, @@ -149,7 +204,13 @@ function suiteArtifact(withScoring = false, withRollup = false) { }, }], } as any); - return composeStacks([orders, ...(withScoring ? [scoring] : []), ...(withRollup ? [rollup] : []), app], { manifest: 'preserve' }); + return composeStacks([ + orders, + ...(scoring ? [scoringModule] : []), + ...(rollup ? [rollupModule] : []), + ...(rules ? [ruleModule] : []), + app, + ], { manifest: 'preserve' }); } interface Booted { @@ -157,7 +218,7 @@ interface Booted { engine: ObjectQL; } -async function bootKernel(root: string, withScoring = false, withRollup = false): Promise { +async function bootKernel(root: string, options: SuiteOptions = {}): Promise { const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); const engine = new ObjectQL(); engine.registerDriver(new SqlDriver({ @@ -167,7 +228,7 @@ async function bootKernel(root: string, withScoring = false, withRollup = false) }) as any, true); await kernel.use(new MetadataPlugin({ watch: false, rootDir: root })); await kernel.use(new ObjectQLPlugin({ ql: engine })); - await kernel.use(new AppPlugin(suiteArtifact(withScoring, withRollup), { environmentId: ENVIRONMENT_ID, organizationId: 'org_qa' })); + await kernel.use(new AppPlugin(suiteArtifact(options), { environmentId: ENVIRONMENT_ID, organizationId: 'org_qa' })); const host: Plugin = { name: 'qa.extsuite-host', dependencies: ['com.objectstack.engine.objectql'], @@ -197,10 +258,10 @@ async function dispatchAs(kernel: ObjectKernel, method: string, path: string, bo try { const out: any = await dispatcher.dispatch(method, path, body, query, { request: raw, environmentId: ENVIRONMENT_ID } as any, '/api/v1'); const answer = out.response?.body; - return { status: out.response?.status as number | undefined, code: codeOf(answer), message: messageOf(answer), data: answer?.data }; + return { status: out.response?.status as number | undefined, code: codeOf(answer), message: messageOf(answer), data: answer?.data, details: answer?.error?.details }; } catch (e) { const thrown = resolveThrownHttpError(e); - return { status: thrown.status as number | undefined, code: thrown.code as unknown, message: (e as Error)?.message, data: undefined }; + return { status: thrown.status as number | undefined, code: thrown.code as unknown, message: (e as Error)?.message, data: undefined, details: undefined }; } } @@ -370,7 +431,7 @@ describe('a boot that starts with the module disabled still creates the module\' }, 120_000); }); -describe('an enabled module whose formula and rule read a field a disabled module contributes', () => { +describe('an enabled module whose formula reads a field a disabled module contributes (the disable proceeds)', () => { let root: string; let booted: Booted | undefined; let accountId: string; @@ -381,11 +442,13 @@ describe('an enabled module whose formula and rule read a field a disabled modul root = mkdtempSync(join(tmpdir(), 'os-suite-ext-ref-')); writeFileSync(join(root, '.keep'), ''); process.env.OS_HOME = mkdtempSync(join(home, 'ref-')); - booted = await bootKernel(root, true); + booted = await bootKernel(root, { scoring: true }); const created = await dispatchAs(booted.kernel, 'POST', `/data/${ACCOUNT}`, { name: 'acme', [CREDIT]: 500 }); expect(created.status, 'precondition: the account is written').toBe(201); accountId = idOf(created); expect(await readAccount(booted.kernel, accountId)).toMatchObject({ [CREDIT]: 500, [CREDIT_BAND]: 'high' }); + // [#30379 ruling A] The CONTROL of the dependent-rule refusal: a formula + // over the field is not a dependent, so the disable proceeds. expect((await dispatchAs(booted.kernel, 'PATCH', `/packages/${MODULE}/disable`)).status).toBe(200); }, 60_000); afterAll(async () => { @@ -413,13 +476,9 @@ describe('an enabled module whose formula and rule read a field a disabled modul } }); - it('measures the rule over the withheld field: it cannot be evaluated, so a write is refused 400 VALIDATION_FAILED', async () => { - // The answer a rule reading a field its object does not declare already - // gets (fail closed, never a 5xx). Recorded as measured: the module the - // rule belongs to reads a field of a module it does not depend on. - const updated = await dispatchAs(booted!.kernel, 'PATCH', `/data/${ACCOUNT}/${accountId}`, { name: 'renamed' }); - expect({ status: updated.status, code: updated.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' }); - expect(String(updated.message)).toContain('qx_credit_not_negative'); + it('lets a write to the object land while the module is disabled: no rule reads the withheld field', async () => { + const updated = await dispatchAs(booted!.kernel, 'PATCH', `/data/${ACCOUNT}/${accountId}`, { name: 'acme' }); + expect(updated.status).toBe(200); }); it('serves the formula over the stored value again once the module is re-enabled', async () => { @@ -436,7 +495,7 @@ describe('[#30377] a disabled module\'s roll-up over a child the enabled app own root = mkdtempSync(join(tmpdir(), 'os-suite-ext-rollup-')); writeFileSync(join(root, '.keep'), ''); process.env.OS_HOME = mkdtempSync(join(home, 'rollup-')); - booted = await bootKernel(root, false, true); + booted = await bootKernel(root, { rollup: true }); }, 60_000); afterAll(async () => { await shutdown(booted); @@ -463,3 +522,108 @@ describe('[#30377] a disabled module\'s roll-up over a child the enabled app own expect(await readAccount(kernel, beta)).toMatchObject({ name: 'beta', [VISIT_TOTAL]: 7 }); }); }); + +describe('[#30379] the disable door refuses while an enabled package\'s validation rule reads a field the package adds', () => { + let root: string; + let booted: Booted | undefined; + let accountId: string; + // Ledgered under `@objectstack/runtime` by #30724; asserted by value. + const CODE = 'PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE'; + /** The listing the refusal's message carries, in its order (package, object, rule). */ + const listingOf = (rules: ReadonlyArray<{ packageId: string; object: string; rule: string; fields: string[] }>) => + rules.map((d) => `${d.packageId} · ${d.object} · ${d.rule} (reads ${d.fields.join(', ')})`).join('; '); + + beforeAll(async () => { + root = mkdtempSync(join(tmpdir(), 'os-suite-ext-dep-')); + writeFileSync(join(root, '.keep'), ''); + process.env.OS_HOME = mkdtempSync(join(home, 'dep-')); + booted = await bootKernel(root, { scoring: true, rules: true }); + const { kernel } = booted; + // The app's `.custom` pair, through its one creation door, and a tenant + // rule saved and published into it as an extension of the app's object. + const pair = await dispatchAs(kernel, 'POST', `/packages/${APP}/ensure-customization`); + expect(pair.status, `precondition: the pair is created (${String(pair.message)})`).toBe(201); + const protocol = kernel.getService('protocol'); + await protocol.saveMetaItem({ + type: 'object_extension', name: ACCOUNT, mode: 'draft', packageId: PAIR, + item: { + extend: ACCOUNT, + validations: [{ + name: RULE_TENANT, type: 'script', severity: 'error', message: 'Credit is capped', + condition: `record.${CREDIT} > 5000000`, + }], + }, + }); + await protocol.publishMetaItem({ type: 'object_extension', name: ACCOUNT, packageId: PAIR }); + const rules = ((booted.engine as any).registry.getObject(ACCOUNT)?.validations ?? []).map((v: any) => v?.name); + expect(rules, 'precondition: the tenant rule is served').toContain(RULE_TENANT); + + const created = await dispatchAs(kernel, 'POST', `/data/${ACCOUNT}`, { name: 'Acme', [CREDIT]: 500 }); + expect(created.status, 'precondition: the account is written').toBe(201); + accountId = idOf(created); + }, 120_000); + afterAll(async () => { + await shutdown(booted); + rmSync(root, { recursive: true, force: true }); + }, 60_000); + + it('answers 409 with the code, naming every reading rule as package · object · rule, and changes nothing', async () => { + const { kernel, engine } = booted!; + const refused = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); + expect({ status: refused.status, code: refused.code }).toEqual({ status: 409, code: CODE }); + const dependents = [ + { packageId: PAIR, object: ACCOUNT, rule: RULE_TENANT, fields: [CREDIT] }, + { packageId: RULES, object: ACCOUNT, rule: RULE_NOT_NEGATIVE, fields: [CREDIT] }, + { packageId: RULES, object: ACCOUNT, rule: RULE_REVIEW, fields: [CREDIT] }, + { packageId: RULES, object: VISIT, rule: RULE_VISIT_HOP, fields: [`account.${CREDIT}`] }, + ]; + // The message lists exactly these four, in order; the refusal fixes no + // `error.details` shape (a machine-readable one is a spec contract first). + expect(String(refused.message)).toContain(`Package '${MODULE}' cannot be disabled: 4 validation rules`); + expect(String(refused.message)).toContain(`— ${listingOf(dependents)}. Switch off or change those rules first`); + expect(refused.details).toBeUndefined(); + // A rule switched off reads nothing, and a formula is no dependent. + expect(String(refused.message)).not.toContain(RULE_SWITCHED_OFF); + expect(String(refused.message)).not.toContain(SCORING); + + // Nothing changed: the row, the served object, the data plane and the state file. + expect(engine.registry.getPackage(MODULE)?.enabled).not.toBe(false); + expect(await servedFieldNames(kernel)).toEqual(expect.arrayContaining([...MODULE_FIELDS])); + const written = await dispatchAs(kernel, 'PATCH', `/data/${ACCOUNT}/${accountId}`, { [CREDIT]: 700 }); + expect(written.status).toBe(200); + expect(await readAccount(kernel, accountId)).toMatchObject({ [CREDIT]: 700 }); + expect(loadDisabledPackageIds(ENVIRONMENT_ID).has(MODULE)).toBe(false); + }); + + it('names only the rules of packages still enabled, and proceeds once none reads the field', async () => { + const { kernel } = booted!; + expect((await dispatchAs(kernel, 'PATCH', `/packages/${RULES}/disable`)).status).toBe(200); + const narrowed = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); + expect({ status: narrowed.status, code: narrowed.code }).toEqual({ status: 409, code: CODE }); + expect(String(narrowed.message)).toContain( + `a validation rule of an enabled package reads a field it adds through objectExtensions, which the disable would withhold — ${listingOf([{ packageId: PAIR, object: ACCOUNT, rule: RULE_TENANT, fields: [CREDIT] }])}. Switch off or change that rule first`, + ); + expect(String(narrowed.message)).not.toContain(RULES); + + expect((await dispatchAs(kernel, 'PATCH', `/packages/${PAIR}/disable`)).status).toBe(200); + const disabled = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); + expect(disabled.status).toBe(200); + const served = await servedFieldNames(kernel); + for (const field of MODULE_FIELDS) expect(served).not.toContain(field); + // The formula module stays enabled throughout: its formula answers null. + expect(await readAccount(kernel, accountId)).toMatchObject({ name: 'Acme', [CREDIT_BAND]: null }); + expect((await dispatchAs(kernel, 'PATCH', `/data/${ACCOUNT}/${accountId}`, { name: 'Acme Ltd' })).status).toBe(200); + }); + + it('measures an enable the ruling did not cover: a reading package enabled while the field stays withheld refuses writes (400 VALIDATION_FAILED)', async () => { + // The disable door is the one ruled. Enabling the reading package after + // the field's package is off is not judged by the enable door, and the + // rule then fails closed on every write, naming itself. The enable + // door's mirror is #30756, which flips this pin when it lands. + const { kernel } = booted!; + expect((await dispatchAs(kernel, 'PATCH', `/packages/${RULES}/enable`)).status).toBe(200); + const updated = await dispatchAs(kernel, 'PATCH', `/data/${ACCOUNT}/${accountId}`, { name: 'Acme' }); + expect({ status: updated.status, code: updated.code }).toEqual({ status: 400, code: 'VALIDATION_FAILED' }); + expect(String(updated.message)).toContain(RULE_NOT_NEGATIVE); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index c4bcab531c8..d640db0e5b8 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -172,6 +172,12 @@ import { // withheld: the declaration of that list, and the redactor that decided it. import { PackageExportManifestSchema, type PackageExportManifest } from '@objectstack/spec/api'; import { getMetadataTypeRedactor } from '@objectstack/spec/kernel'; +// [#30379 ruling A] The disable door's dependent-rule scan reads a rule's +// fields with the platform's one member reader, the reader the rule validator +// asks before it refuses a write (`readsRecordColumn`), and judges a hop with +// the spec's one reference arbiter: never a second parser. +import { analyzeRelationshipTraversals } from '@objectstack/formula'; +import { referenceTargetOf } from '@objectstack/spec/data'; /** * [#13598] The `protocol` service slot **as this domain reaches it** — one @@ -783,6 +789,270 @@ function isDisableablePackage(engine: unknown, id: string | undefined): boolean return (id !== undefined && isDisableableAppModule(engine, id)) || isWritablePackage(engine, id); } +/** + * [#30379 ruling A, ADR-0141 Open items 4] The code `PATCH /packages/:id/disable` + * answers, with `409`, while a validation rule of any enabled package (declared + * dependent or not, an app's `.custom` pair included) reads a field the package + * adds through `objectExtensions` ({@link readDisableDependentRules}). Registered + * under `@objectstack/runtime` in `ERROR_CODE_LEDGER` (#30724); asserted by value. + * The refusal carries the code and a message naming every rule, and no + * `error.details` beyond the code: a machine-readable shape for the + * package · object · rule triples would be a `packages/spec` contract first. + * Held as its wire pair, the shape `check:error-status-conformance` reads. + */ +const PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE = { code: 'PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE', status: 409 } as const; + +/** + * One validation rule a disable would leave reading a field it withholds: the + * package that serves the rule, the object the rule is declared on, the rule's + * name, and the withheld fields it reads (`FIELD`, or `REF.FIELD` for a read + * one hop through a reference field of the rule's object). + */ +interface DisableDependentRule { + packageId: string; + object: string; + rule: string; + fields: string[]; +} + +/** The fields one validation rule reads: on its own object, and one hop through a reference field. */ +interface RuleFieldReads { + readonly own: Set; + readonly hops: Map>; +} + +/** + * The CEL source of a rule's predicate slot, or `undefined` when it is not CEL: + * the dialect is checked, not assumed (a bare string is CEL by the envelope's + * own default), as the rule validator reads the slot it evaluates. + */ +function celSourceOfRuleSlot(slot: unknown): string | undefined { + if (typeof slot === 'string') return slot.trim() ? slot : undefined; + if (!slot || typeof slot !== 'object') return undefined; + const { dialect, source } = slot as { dialect?: unknown; source?: unknown }; + if (dialect !== undefined && dialect !== 'cel') return undefined; + return typeof source === 'string' && source.trim() ? source : undefined; +} + +/** + * Fold what `rule` reads into `reads`, at any `conditional` depth. + * + * - A predicate (`script` / `cross_field` `condition`, a `conditional`'s + * `when`) is read with `analyzeRelationshipTraversals` (`@objectstack/formula`) + * over both roots the evaluator binds, `record` and `previous`: every member + * of the root is a read of that column (the rule validator's own reading, + * `readsRecordColumn`), and a `record.REF.FIELD` read is also a hop the + * engine hydrates from REF's target. A `previous` hop is never hydrated, so + * it reads only its own column. A source that does not parse reads nothing + * here: such a rule already refuses every write, whatever is enabled. + * - A `state_machine` / `format` / `json_schema` rule reads the one `field` it + * names. A `cross_field` rule's `fields` list is advisory (only its first + * entry labels the violation), so its `condition` is what it reads. + */ +function addRuleFieldReads(rule: unknown, reads: RuleFieldReads, depth: number): void { + if (!rule || typeof rule !== 'object' || depth > 8) return; + const r = rule as { type?: unknown; condition?: unknown; when?: unknown; field?: unknown; then?: unknown; otherwise?: unknown }; + const addPredicate = (slot: unknown): void => { + const source = celSourceOfRuleSlot(slot); + if (!source) return; + for (const root of ['record', 'previous'] as const) { + const analysis = analyzeRelationshipTraversals(source, root); + if (!analysis) continue; + for (const field of analysis.bareFields) reads.own.add(field); + for (const field of analysis.multiHopFields) reads.own.add(field); + for (const [field, related] of analysis.traversals) { + reads.own.add(field); + if (root !== 'record') continue; + let hop = reads.hops.get(field); + if (!hop) reads.hops.set(field, (hop = new Set())); + for (const name of related) hop.add(name); + } + } + }; + if (r.type === 'script' || r.type === 'cross_field') addPredicate(r.condition); + if ((r.type === 'state_machine' || r.type === 'format' || r.type === 'json_schema') && typeof r.field === 'string') { + reads.own.add(r.field); + } + if (r.type === 'conditional') { + addPredicate(r.when); + addRuleFieldReads(r.then, reads, depth + 1); + addRuleFieldReads(r.otherwise, reads, depth + 1); + } +} + +/** The field names a contribution body declares (`fields` keys holding a definition). */ +function declaredFieldNamesOf(body: unknown): string[] { + const fields = (body as { fields?: unknown } | null | undefined)?.fields; + if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return []; + return Object.keys(fields).filter((name) => { + const def = (fields as Record)[name]; + return !!def && typeof def === 'object' && !Array.isArray(def); + }); +} + +/** The validation rules a contribution body declares. */ +function declaredRulesOf(body: unknown): Array<{ name?: unknown; active?: unknown }> { + const rules = (body as { validations?: unknown } | null | undefined)?.validations; + return Array.isArray(rules) ? rules.filter((rule) => !!rule && typeof rule === 'object') : []; +} + +/** + * [#30379 ruling A, ADR-0141 Open items 4] The validation rules that would read + * a field the disable of `packageId` withholds, while the package that serves + * each rule stays enabled: any enabled package, declared dependent or not (no + * `manifest.dependencies` entry is read), an app's `.custom` pair included. + * `PATCH /packages/:id/disable` refuses while this is + * not empty: a disabled package's `objectExtensions` fields are withheld from + * the served object (`SchemaRegistry.resolveObject`), and a rule that reads a + * field its object no longer serves cannot be evaluated, so every write to that + * object would be refused for as long as the package stays off. + * + * ## What the disable withholds + * + * Per object the package extends: the fields its `extend` contributions add + * that neither the owner's own declaration nor an extension that stays served + * also declares. The registry's reading of a withheld member + * (`withholdDisabledExtensions`), asked of the state the disable would leave: + * the package's own extensions and every already-disabled package's leave the + * fold; everything else stays. + * + * ## Which rules are judged + * + * Every rule the served object would still carry, of every severity, on every + * object whose owner is enabled and is not the package itself (a disabled + * owner's object is refused whole, so no write reaches its rules): + * + * - the base layer's (the tenant overlay when one is registered, the owner + * otherwise, as the fold selects it), less the rules the disable withholds + * with the fields; + * - each `extend` contribution of any enabled package other than this one, + * declared dependent or not, an app's `.custom` pair (where + * tenant-written rules live) included. + * + * A rule switched off (`active: false`) reads nothing: switching the rule off + * is one of the remedies the refusal names. Formula fields are not judged: a + * formula over a withheld field answers `null`, never a refused write. + * + * A rule reads a withheld field when it names it on its own object, or names + * it one hop through a reference field whose target is an object the package + * extends ({@link addRuleFieldReads}). + * + * Ordered by package, object and rule. Empty for a package already disabled + * (the disable withholds nothing more) and for one that extends nothing. + */ +function readDisableDependentRules(registry: any, packageId: string): DisableDependentRule[] { + if (typeof registry?.getObjectContributors !== 'function' || typeof registry?.getAllObjects !== 'function') return []; + const disabled = (id: unknown): boolean => + typeof registry.isPackageDisabled === 'function' && registry.isPackageDisabled(id); + if (disabled(packageId)) return []; + + type Contributor = { packageId?: unknown; ownership?: unknown; definition?: unknown }; + const leaves = (c: Contributor): boolean => + c.ownership === 'extend' && (c.packageId === packageId || disabled(c.packageId)); + + const names: string[] = []; + for (const object of registry.getAllObjects() as Array<{ name?: unknown }>) { + if (typeof object?.name === 'string') names.push(object.name); + } + + // Object → the fields the disable withholds from it; and the rules the fold + // takes off its base layer once the disable lands, on every object. + const withheldFields = new Map>(); + const withheldRules = new Map>(); + for (const name of names) { + const contributors = registry.getObjectContributors(name) as Contributor[]; + const own = contributors.filter((c) => c.ownership === 'extend' && c.packageId === packageId); + const kept = contributors.filter((c) => !leaves(c)); + const keptFields = new Set(); + const keptRules = new Set(); + for (const c of kept) { + if (c.ownership !== 'own' && c.ownership !== 'extend') continue; + for (const field of declaredFieldNamesOf(c.definition)) keptFields.add(field); + for (const rule of declaredRulesOf(c.definition)) if (typeof rule.name === 'string') keptRules.add(rule.name); + } + // The fields THIS disable withholds: the package's own additions. A field + // only an already-disabled package adds is withheld already, whatever + // this disable does. + const fields = new Set(); + for (const c of own) { + for (const field of declaredFieldNamesOf(c.definition)) if (!keptFields.has(field)) fields.add(field); + } + // The rules the fold would withhold once the disable lands — every + // leaving extension's, as the fold takes them off a base that carries them. + const rules = new Set(); + for (const c of contributors.filter(leaves)) { + for (const rule of declaredRulesOf(c.definition)) { + if (typeof rule.name === 'string' && !keptRules.has(rule.name)) rules.add(rule.name); + } + } + if (fields.size > 0) withheldFields.set(name, fields); + if (rules.size > 0) withheldRules.set(name, rules); + } + if (withheldFields.size === 0) return []; + + const dependents: DisableDependentRule[] = []; + for (const name of names) { + const contributors = registry.getObjectContributors(name) as Contributor[]; + const owner = contributors.find((c) => c.ownership === 'own'); + if (!owner || owner.packageId === packageId || disabled(owner.packageId)) continue; + const base = contributors.find((c) => c.ownership === 'overlay') ?? owner; + const servedFields = ((registry.getObject?.(name) as { fields?: Record } | undefined)?.fields) + ?? ((base.definition as { fields?: Record } | undefined)?.fields) ?? {}; + const ownWithheld = withheldFields.get(name); + const rulesWithheld = withheldRules.get(name) ?? new Set(); + + const sources: Array<{ packageId: unknown; rules: Array<{ name?: unknown; active?: unknown }> }> = [ + { packageId: base.packageId, rules: declaredRulesOf(base.definition).filter((rule) => !(typeof rule.name === 'string' && rulesWithheld.has(rule.name))) }, + ]; + for (const c of contributors) { + if (c.ownership !== 'extend' || leaves(c)) continue; + sources.push({ packageId: c.packageId, rules: declaredRulesOf(c.definition) }); + } + + for (const source of sources) { + for (const rule of source.rules) { + if (rule.active === false || typeof rule.name !== 'string') continue; + const reads: RuleFieldReads = { own: new Set(), hops: new Map() }; + addRuleFieldReads(rule, reads, 0); + const hit: string[] = []; + if (ownWithheld) for (const field of reads.own) if (ownWithheld.has(field)) hit.push(field); + for (const [ref, related] of reads.hops) { + const target = referenceTargetOf(servedFields[ref]); + const targetWithheld = target === undefined ? undefined : withheldFields.get(target); + if (!targetWithheld) continue; + for (const field of related) if (targetWithheld.has(field)) hit.push(`${ref}.${field}`); + } + if (hit.length === 0) continue; + dependents.push({ + packageId: typeof source.packageId === 'string' ? source.packageId : '(no package)', + object: name, + rule: rule.name, + fields: [...new Set(hit)].sort(), + }); + } + } + } + return dependents.sort((a, b) => a.packageId.localeCompare(b.packageId) + || a.object.localeCompare(b.object) + || a.rule.localeCompare(b.rule)); +} + +/** + * The sentence the disable door's dependent-rule refusal carries: the package, + * then every rule as `package · object · rule (reads FIELD, …)`, then the remedy. + */ +function disableDependentRulesMessage(id: string, dependents: readonly DisableDependentRule[]): string { + const one = dependents.length === 1; + const listed = dependents + .map((d) => `${d.packageId} · ${d.object} · ${d.rule} (reads ${d.fields.join(', ')})`) + .join('; '); + return `Package '${id}' cannot be disabled: ${one + ? 'a validation rule of an enabled package reads a field' + : `${dependents.length} validation rules of enabled packages read fields`} it adds through objectExtensions, ` + + `which the disable would withhold — ${listed}. Switch off or change ${one ? 'that rule' : 'those rules'} first, ` + + 'or keep the package enabled.'; +} + /** * [#14451] `POST /packages/:id/duplicate` — the SOURCE must be a BASE. * @@ -1970,6 +2240,27 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin ? null : requireWritablePackage(deps, qlService, id, 'disable'); if (readOnly) return readOnly; + // [#30379 ruling A, ADR-0141 Open items 4] An enabled dependent + // refuses the disable: a validation rule of any enabled package + // (declared dependent or not, an app's `.custom` pair included) + // that reads a field this package adds through `objectExtensions` + // would refuse every write to its object once the field is + // withheld. Asked after the read-only check and BEFORE + // `disablePackage` and `setPackageDisabled`, so a refused disable + // leaves the row, the served objects and the environment's state + // file exactly as they were. The message names every rule; the + // refusal carries no `error.details` beyond the code. + const dependents = readDisableDependentRules(registry, id); + if (dependents.length > 0) { + return { + handled: true, + response: deps.error( + disableDependentRulesMessage(id, dependents), + PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE.status, + { code: PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE.code }, + ), + }; + } const pkg = registry.disablePackage(id); if (!pkg) return { handled: true, response: deps.error(`Package '${id}' not found`, 404) }; try { From 2e9a976c4670fb3b8e9942fdd41434f7d2564db2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 04:03:51 +0000 Subject: [PATCH 2/5] docs(packages): the disable door's 409 when an enabled rule reads a field the package adds The marketplace handbook's Enable-and-disable step and Disabling-a-module list, the metadata API's disable entry and the error catalog's Conflict section name PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE, what it names and the remedy. Claude-Session: https://claude.ai/code/session_016WV4cwCWp9qZ5SgMof1qLa Co-authored-by: Claude --- content/docs/api/error-catalog.mdx | 5 +++++ content/docs/api/metadata-api.mdx | 9 +++++++++ content/docs/deployment/marketplace-handbook.mdx | 14 ++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index c24afb9dbff..5a429429480 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -546,6 +546,11 @@ spelling never crosses HTTP. **Fix:** Wait for the lock to be released, or contact the lock holder. **Retry:** `retry_backoff` +### `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE` +**Cause:** `PATCH /api/v1/packages/:id/disable` was refused because a validation rule of an enabled package (declared dependent or not, an app's `.custom` pair included, any severity) reads a field this package adds to another object through `objectExtensions`. Disabling it would withhold that field, and every write to the object would then fail the rule. The message names each rule as `package · object · rule (reads FIELD)`, one or several. Nothing changed. +**Fix:** Switch off or change each named rule first, or disable the package that owns it, then disable this package; or keep it enabled. A formula over the field is no reason to refuse: it answers `null`. +**Retry:** `no_retry` + --- ## Request Errors (405/428) diff --git a/content/docs/api/metadata-api.mdx b/content/docs/api/metadata-api.mdx index 5aef4d4a82c..8d125ed51c1 100644 --- a/content/docs/api/metadata-api.mdx +++ b/content/docs/api/metadata-api.mdx @@ -182,6 +182,15 @@ Enable a disabled package. Disable a package without uninstalling. +A package that adds fields to another object through `objectExtensions` is +refused `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE` while a validation +rule of any enabled package reads one of those fields (declared dependent or +not, an app's `.custom` pair included, any severity): withheld, the field could +not be read, and every write to that object would fail the rule. The message +names each rule as `package · object · rule (reads FIELD)`, and nothing changes. +Switch off or change each named rule first, or disable the package that owns it. +A formula over the field does not refuse the disable; it answers `null`. + --- ## See also diff --git a/content/docs/deployment/marketplace-handbook.mdx b/content/docs/deployment/marketplace-handbook.mdx index 309e5c1fc06..79712637e30 100644 --- a/content/docs/deployment/marketplace-handbook.mdx +++ b/content/docs/deployment/marketplace-handbook.mdx @@ -228,6 +228,9 @@ the detail. For every other package provided by code or by an installed app (the app itself, a plugin, a platform package), the runtime answers `PATCH /api/v1/packages/:id/disable` with `422 WRITABLE_PACKAGE_REQUIRED`. + A disable that would leave a validation rule reading a field the package + adds is refused with `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE` + ([below](#disabling-a-module)). To take an installed app away, uninstall it. 8. **Upgrading.** You publish a new version; nothing running changes. An environment installed through the cloud shows **Update available → v…** and @@ -302,6 +305,17 @@ its data is kept. - **Restart.** The choice is kept per environment and survives a restart. - **Authoring.** A disabled module is still read-only: edit, publish, revert and delete keep answering `422 WRITABLE_PACKAGE_REQUIRED`. +- **A rule that reads its fields.** A module that adds fields to another + object through `objectExtensions` cannot be disabled while a validation rule + of any enabled package reads one of those fields: declared dependent or not, + your app's `.custom` pair (where the customer's own rules live) included, at + any severity. The disable door answers + `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, and its message names + each rule as `package · object · rule (reads FIELD)`; nothing changes. + Withheld, the field could not be read, and every write to that object would + fail the rule. Switch off or change each named rule first, or disable the + package that owns it. A formula over the field does not block the disable: + it answers `null` while the module is off. **What your app's own code meets.** A hook of your app that reads or writes a module's object through `ctx.api.object('')` runs inside the server, From a154e1ba1b6ce5f9552f567b62088bf0e4360e78 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 04:03:51 +0000 Subject: [PATCH 3/5] chore(changeset): the package disable door's dependent-rule refusal Claude-Session: https://claude.ai/code/session_016WV4cwCWp9qZ5SgMof1qLa Co-authored-by: Claude --- .../30379-disable-door-dependent-rule.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .changeset/30379-disable-door-dependent-rule.md diff --git a/.changeset/30379-disable-door-dependent-rule.md b/.changeset/30379-disable-door-dependent-rule.md new file mode 100644 index 00000000000..ad038149f02 --- /dev/null +++ b/.changeset/30379-disable-door-dependent-rule.md @@ -0,0 +1,21 @@ +--- +'@objectstack/runtime': patch +--- + +fix(runtime)!: `PATCH /api/v1/packages/:id/disable` refuses while an enabled package's validation rule reads a field the package adds through `objectExtensions` + +Clause-②: no (narrowing) — the package disable door refuses (`409` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`) a disable it accepted before: one that would withhold a field a validation rule of any enabled package reads. Nothing is widened: no export, request shape, response member or authorable key is added, and the code is the one #30724 registered. +Metadata-protocol: none +Skills-docs: updated here — `content/docs/api/metadata-api.mdx` (the disable door's entry), `content/docs/deployment/marketplace-handbook.mdx` (the Enable-and-disable step and the Disabling-a-module list) and `content/docs/api/error-catalog.mdx` (the code's 409 entry) name the refusal; `skills/**` teaches no package disable door (grep for `packages/:id/disable`, `disableable`, `WRITABLE_PACKAGE_REQUIRED`: none), so no skill or eval moves. + + + +**BREAKING** accept-set narrowing on the package disable door (`@objectstack/runtime`, `domains/packages.ts`), for one case only. + +- **Before.** A disabled package's `objectExtensions` fields are withheld from the object they extend. When a validation rule of another package that stayed enabled read one of those fields, the disable answered `200`, and every write to that object was then refused (`400 VALIDATION_FAILED`, naming the rule) for as long as the package stayed off. +- **Now.** The door refuses such a disable with `409` and `error.code` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, after the read-only check and before anything changes: the package row, the served objects and the environment's stored disable state stay as they were. The message names every reading rule as `package · object · rule (reads FIELD)`, one or several. The refusal carries no other `error.details`. +- **Who is scanned.** Any enabled package, declared dependent or not (no `manifest.dependencies` entry is read), an app's `.custom` pair (where tenant-written rules live) included, at every severity. A rule reads a field when its predicate names it on the rule's own object or one hop through a reference field of that object (`record.account.FIELD`), or when it is a `state_machine` / `format` / `json_schema` rule on that field. +- **Unchanged.** A formula over the field is no reason to refuse (it answers `null`), nor is a rule switched off (`active: false`) or a rule of a package that is itself disabled. Every other disable answers as before. +- **Who moves.** An administrator or client disabling such a package: a refusal naming the rules, instead of a disable that left the object refusing every write. + +**The remedy.** Switch off or change each named rule first, or disable the package that owns it, then disable the package; or keep the package enabled. From d315efca9756b74d7593a1657229c260b3207e0a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 05:41:45 +0000 Subject: [PATCH 4/5] fix(runtime): judge the disable scan's rules as the served fold serves them, and ask it on the install door's enableOnInstall false arm The dependent-rule scan now reads each object as the registry's fold serves it once the disable lands (getAllObjects with the package's extensions left out, less the rules the disable takes off a base that carries them): one rule per name, the copy a later layer declares winning, with a tenant overlay's message and active on it, so a packaged rule the tenant switched off through the overlay no longer refuses the disable. Each rule is named after the last layer in fold order that declares it, a tenant overlay by that name, never a placeholder. POST /packages with enableOnInstall false asks the same scan before it disables: such an install still lands, the package stays enabled on the row, the registry and the state file, and the door answers the same 409 instead of a 201 that claims a disable. The error catalog's advertised wire-code count moves to 58 for the new entry, and the docs name the fold reading and the install arm. Claude-Session: https://claude.ai/code/session_016WV4cwCWp9qZ5SgMof1qLa Co-authored-by: Claude --- .../30379-disable-door-dependent-rule.md | 9 +- content/docs/api/error-catalog.mdx | 4 +- content/docs/api/metadata-api.mdx | 8 +- .../docs/deployment/marketplace-handbook.mdx | 9 +- ...es-disable-module-extension-fields.test.ts | 213 +++++++++++++++++- packages/runtime/src/domains/packages.ts | 159 +++++++++---- 6 files changed, 339 insertions(+), 63 deletions(-) diff --git a/.changeset/30379-disable-door-dependent-rule.md b/.changeset/30379-disable-door-dependent-rule.md index ad038149f02..5cd7b9c876b 100644 --- a/.changeset/30379-disable-door-dependent-rule.md +++ b/.changeset/30379-disable-door-dependent-rule.md @@ -4,18 +4,19 @@ fix(runtime)!: `PATCH /api/v1/packages/:id/disable` refuses while an enabled package's validation rule reads a field the package adds through `objectExtensions` -Clause-②: no (narrowing) — the package disable door refuses (`409` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`) a disable it accepted before: one that would withhold a field a validation rule of any enabled package reads. Nothing is widened: no export, request shape, response member or authorable key is added, and the code is the one #30724 registered. +Clause-②: no (narrowing) — the package disable door, and the install door's `enableOnInstall: false` arm, refuse (`409` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`) a disable they accepted before: one that would withhold a field an active validation rule reads. Nothing is widened: no export, request shape, response member or authorable key is added, and the code is the one #30724 registered. Metadata-protocol: none Skills-docs: updated here — `content/docs/api/metadata-api.mdx` (the disable door's entry), `content/docs/deployment/marketplace-handbook.mdx` (the Enable-and-disable step and the Disabling-a-module list) and `content/docs/api/error-catalog.mdx` (the code's 409 entry) name the refusal; `skills/**` teaches no package disable door (grep for `packages/:id/disable`, `disableable`, `WRITABLE_PACKAGE_REQUIRED`: none), so no skill or eval moves. -**BREAKING** accept-set narrowing on the package disable door (`@objectstack/runtime`, `domains/packages.ts`), for one case only. +**BREAKING** accept-set narrowing on the package disable door and the install door's `enableOnInstall: false` arm (`@objectstack/runtime`, `domains/packages.ts`), for one case only. - **Before.** A disabled package's `objectExtensions` fields are withheld from the object they extend. When a validation rule of another package that stayed enabled read one of those fields, the disable answered `200`, and every write to that object was then refused (`400 VALIDATION_FAILED`, naming the rule) for as long as the package stayed off. - **Now.** The door refuses such a disable with `409` and `error.code` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, after the read-only check and before anything changes: the package row, the served objects and the environment's stored disable state stay as they were. The message names every reading rule as `package · object · rule (reads FIELD)`, one or several. The refusal carries no other `error.details`. -- **Who is scanned.** Any enabled package, declared dependent or not (no `manifest.dependencies` entry is read), an app's `.custom` pair (where tenant-written rules live) included, at every severity. A rule reads a field when its predicate names it on the rule's own object or one hop through a reference field of that object (`record.account.FIELD`), or when it is a `state_machine` / `format` / `json_schema` rule on that field. +- **Who is scanned.** Any enabled package, declared dependent or not (no `manifest.dependencies` entry is read), an app's `.custom` pair (where tenant-written rules live) included, at every severity. Each rule is judged as the object will serve it once the disable lands: one rule per name (a later layer's copy replaces an earlier one), with a tenant overlay's `message` and `active` on it, so a packaged rule the tenant switched off through the overlay does not refuse the disable. The message names each rule after the layer whose copy is served (`tenant overlay` for a rule the overlay declares itself). A rule reads a field when its predicate names it on the rule's own object or one hop through a reference field of that object (`record.account.FIELD`), or when it is a `state_machine` / `format` / `json_schema` rule on that field. +- **The install door too.** `POST /api/v1/packages` with `enableOnInstall: false` (an overwrite of an enabled package that asks to come back disabled) asks the same scan before it disables: such an install still lands, the package stays enabled, and the door answers the same `409` instead of a `201` claiming a disable. - **Unchanged.** A formula over the field is no reason to refuse (it answers `null`), nor is a rule switched off (`active: false`) or a rule of a package that is itself disabled. Every other disable answers as before. -- **Who moves.** An administrator or client disabling such a package: a refusal naming the rules, instead of a disable that left the object refusing every write. +- **Who moves.** An administrator or client disabling such a package, or installing it with `enableOnInstall: false`: a refusal naming the rules, instead of a disable that left the object refusing every write. **The remedy.** Switch off or change each named rule first, or disable the package that owns it, then disable the package; or keep the package enabled. diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index 5a429429480..f1d9dba1f21 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -4,7 +4,7 @@ navTitle: Error Code Catalog description: Complete reference for all ObjectStack error codes with causes, fixes, and retry strategies --- -ObjectStack uses a structured error system of **error categories** and **57 error codes reachable on the wire**, catalogued in full below (that count is pinned by `packages/spec/src/api/error-catalog-docs.test.ts` against the wire face). Every error includes a machine-readable code, HTTP status mapping, and retry guidance. +ObjectStack uses a structured error system of **error categories** and **58 error codes reachable on the wire**, catalogued in full below (that count is pinned by `packages/spec/src/api/error-catalog-docs.test.ts` against the wire face). Every error includes a machine-readable code, HTTP status mapping, and retry guidance. This catalog documents the **wire face** — the codes a client can actually receive. That is not quite the `StandardErrorCode` enum: the enum also carries in-process spellings the REST door translates at the @@ -547,7 +547,7 @@ spelling never crosses HTTP. **Retry:** `retry_backoff` ### `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE` -**Cause:** `PATCH /api/v1/packages/:id/disable` was refused because a validation rule of an enabled package (declared dependent or not, an app's `.custom` pair included, any severity) reads a field this package adds to another object through `objectExtensions`. Disabling it would withhold that field, and every write to the object would then fail the rule. The message names each rule as `package · object · rule (reads FIELD)`, one or several. Nothing changed. +**Cause:** `PATCH /api/v1/packages/:id/disable` was refused because a validation rule of an enabled package (declared dependent or not, an app's `.custom` pair included, any severity) reads a field this package adds to another object through `objectExtensions`. Disabling it would withhold that field, and every write to the object would then fail the rule. Each rule is judged as the object serves it: one rule per name (the copy a later layer declares wins), with a tenant overlay's switch honoured. The message names each rule as `package · object · rule (reads FIELD)`, one or several, after the layer whose copy is served (`tenant overlay` for a rule the overlay declares itself). Nothing changed. `POST /api/v1/packages` with `enableOnInstall: false` answers the same code when that install would leave such a package disabled: the install itself lands and the package stays enabled. **Fix:** Switch off or change each named rule first, or disable the package that owns it, then disable this package; or keep it enabled. A formula over the field is no reason to refuse: it answers `null`. **Retry:** `no_retry` diff --git a/content/docs/api/metadata-api.mdx b/content/docs/api/metadata-api.mdx index 8d125ed51c1..d85fe387e4e 100644 --- a/content/docs/api/metadata-api.mdx +++ b/content/docs/api/metadata-api.mdx @@ -188,8 +188,12 @@ rule of any enabled package reads one of those fields (declared dependent or not, an app's `.custom` pair included, any severity): withheld, the field could not be read, and every write to that object would fail the rule. The message names each rule as `package · object · rule (reads FIELD)`, and nothing changes. -Switch off or change each named rule first, or disable the package that owns it. -A formula over the field does not refuse the disable; it answers `null`. +A rule is judged as the object serves it: one rule per name, a tenant overlay's +switch honoured. Switch off or change each named rule first, or disable the +package that owns it. A formula over the field does not refuse the disable; it +answers `null`. `POST /packages` with `enableOnInstall: false` answers the same +`409` when the install would leave such a package disabled; the install lands +and the package stays enabled. --- diff --git a/content/docs/deployment/marketplace-handbook.mdx b/content/docs/deployment/marketplace-handbook.mdx index 79712637e30..2383eda54b0 100644 --- a/content/docs/deployment/marketplace-handbook.mdx +++ b/content/docs/deployment/marketplace-handbook.mdx @@ -313,9 +313,12 @@ its data is kept. `409 PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, and its message names each rule as `package · object · rule (reads FIELD)`; nothing changes. Withheld, the field could not be read, and every write to that object would - fail the rule. Switch off or change each named rule first, or disable the - package that owns it. A formula over the field does not block the disable: - it answers `null` while the module is off. + fail the rule. Switch off or change each named rule first (a rule the + customer switched off on their copy of the object counts as off), or disable + the package that owns it. A formula over the field does not block the + disable: it answers `null` while the module is off. An install with + `enableOnInstall: false` that would leave the module disabled in that state + answers the same `409`, and the module stays enabled. **What your app's own code meets.** A hook of your app that reads or writes a module's object through `ctx.api.object('')` runs inside the server, diff --git a/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts b/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts index 87d93d0de04..12caedc9fc7 100644 --- a/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts +++ b/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts @@ -51,7 +51,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { ObjectKernel, type Plugin, type PluginContext } from '@objectstack/core'; import { MetadataPlugin } from '@objectstack/metadata'; -import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { ObjectQL, ObjectQLPlugin, SchemaRegistry } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { composeStacks, defineStack } from '@objectstack/spec'; import { resolveThrownHttpError } from '@objectstack/types'; @@ -96,6 +96,14 @@ const RULE_VISIT_HOP = 'qx_visit_account_credit'; /** The app's paired customization package, where a tenant-written rule lives. */ const PAIR = `${APP}.custom`; const RULE_TENANT = 'qx_credit_tenant_cap'; +/** + * [#30379 contract review] A fifth, ENABLED module folded AFTER {@link RULES} + * (priority 300) that redeclares two of its rules by name: one copy that no + * longer reads the field, one that still does. + */ +const OVERRIDE = 'com.qa.extsuite.rules-override'; +/** A rule a tenant overlay declares itself, reading the module's field. */ +const RULE_OVERLAY_OWN = 'qx_credit_overlay_cap'; interface SuiteOptions { /** {@link SCORING}: a formula over the module's field. */ @@ -104,9 +112,11 @@ interface SuiteOptions { rollup?: boolean; /** {@link RULES}: validation rules over the module's field. */ rules?: boolean; + /** {@link OVERRIDE}: two of {@link RULES}' rules redeclared by name, folded later. */ + override?: boolean; } -function suiteArtifact({ scoring = false, rollup = false, rules = false }: SuiteOptions = {}) { +function suiteArtifact({ scoring = false, rollup = false, rules = false, override = false }: SuiteOptions = {}) { const app = defineStack({ manifest: { id: APP, name: 'QA Ext Suite', namespace: 'qx', version: '1.0.0', type: 'app' }, objects: [{ @@ -189,6 +199,25 @@ function suiteArtifact({ scoring = false, rollup = false, rules = false }: Suite }], }], } as any); + const overrideModule = defineStack({ + manifest: { + id: OVERRIDE, name: 'QA Ext Suite Rules Override', namespace: 'qx', version: '1.0.0', type: 'module', + dependencies: { [APP]: '^1.0.0' }, + }, + objectExtensions: [{ + extend: ACCOUNT, + priority: 300, + validations: [{ + // Replaces RULES' copy, which reads the field, with one that does not. + name: RULE_NOT_NEGATIVE, type: 'script', severity: 'error', message: 'The name is blocked', + condition: "record.name == 'blocked'", + }, { + // Replaces RULES' conditional with a script that still reads it. + name: RULE_REVIEW, type: 'script', severity: 'warning', message: 'A very large credit needs review', + condition: `record.${CREDIT} > 5000`, + }], + }], + } as any); const rollupModule = defineStack({ manifest: { id: ROLLUP, name: 'QA Ext Suite Visits', namespace: 'qx', version: '1.0.0', type: 'module', @@ -209,6 +238,7 @@ function suiteArtifact({ scoring = false, rollup = false, rules = false }: Suite ...(scoring ? [scoringModule] : []), ...(rollup ? [rollupModule] : []), ...(rules ? [ruleModule] : []), + ...(override ? [overrideModule] : []), app, ], { manifest: 'preserve' }); } @@ -579,7 +609,7 @@ describe('[#30379] the disable door refuses while an enabled package\'s validati ]; // The message lists exactly these four, in order; the refusal fixes no // `error.details` shape (a machine-readable one is a spec contract first). - expect(String(refused.message)).toContain(`Package '${MODULE}' cannot be disabled: 4 validation rules`); + expect(String(refused.message)).toContain(`Package '${MODULE}' cannot be disabled: 4 active validation rules read fields`); expect(String(refused.message)).toContain(`— ${listingOf(dependents)}. Switch off or change those rules first`); expect(refused.details).toBeUndefined(); // A rule switched off reads nothing, and a formula is no dependent. @@ -601,7 +631,7 @@ describe('[#30379] the disable door refuses while an enabled package\'s validati const narrowed = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); expect({ status: narrowed.status, code: narrowed.code }).toEqual({ status: 409, code: CODE }); expect(String(narrowed.message)).toContain( - `a validation rule of an enabled package reads a field it adds through objectExtensions, which the disable would withhold — ${listingOf([{ packageId: PAIR, object: ACCOUNT, rule: RULE_TENANT, fields: [CREDIT] }])}. Switch off or change that rule first`, + `an active validation rule reads a field it adds through objectExtensions, which the disable would withhold — ${listingOf([{ packageId: PAIR, object: ACCOUNT, rule: RULE_TENANT, fields: [CREDIT] }])}. Switch off or change that rule first`, ); expect(String(narrowed.message)).not.toContain(RULES); @@ -627,3 +657,178 @@ describe('[#30379] the disable door refuses while an enabled package\'s validati expect(String(updated.message)).toContain(RULE_NOT_NEGATIVE); }); }); + +/** The listing a dependent-rule refusal's message carries, in its order (package, object, rule). */ +const listingOfRules = (rules: ReadonlyArray<{ packageId: string; object: string; rule: string; fields: string[] }>) => + rules.map((d) => `${d.packageId} · ${d.object} · ${d.rule} (reads ${d.fields.join(', ')})`).join('; '); + +describe('[#30379 contract review] the scan judges the rules the served fold serves', () => { + let root: string; + let booted: Booted | undefined; + let accountId: string; + const CODE = 'PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE'; + + /** The app's object as a tenant saved it from a served read: the module's fields ride along, with the overlay's rules. */ + const accountOverlay = (validations: unknown[]) => ({ + name: ACCOUNT, + label: 'Account', + fields: { + name: { name: 'name', type: 'text', label: 'Name' }, + [CREDIT]: { name: CREDIT, type: 'number', label: 'Credit' }, + [ORDER_TOTAL]: { + name: ORDER_TOTAL, type: 'summary', label: 'Order total', + summaryOperations: { object: ORDER, field: 'amount', function: 'sum' }, + }, + }, + validations, + _provenance: 'org', + }); + const visitOverlay = (validations: unknown[]) => ({ + name: VISIT, + label: 'Visit', + fields: { + name: { name: 'name', type: 'text', label: 'Name' }, + amount: { name: 'amount', type: 'number', label: 'Amount' }, + account: { name: 'account', type: 'lookup', label: 'Account', reference: ACCOUNT }, + }, + validations, + _provenance: 'org', + }); + /** A served predicate's CEL source (the loader may wrap it as `{ dialect, source }`). */ + const sourceOf = (slot: unknown): unknown => (slot && typeof slot === 'object' ? (slot as { source?: unknown }).source : slot); + /** A tenant copy of a packaged rule: only its sentence and its switch are the tenant's. */ + const switchedOff = (name: string, message: string) => ({ name, type: 'script', message, condition: 'true', active: false }); + + beforeAll(async () => { + root = mkdtempSync(join(tmpdir(), 'os-suite-ext-fold-')); + writeFileSync(join(root, '.keep'), ''); + process.env.OS_HOME = mkdtempSync(join(home, 'fold-')); + booted = await bootKernel(root, { rules: true, override: true }); + const created = await dispatchAs(booted.kernel, 'POST', `/data/${ACCOUNT}`, { name: 'Acme', [CREDIT]: 500 }); + expect(created.status, 'precondition: the account is written').toBe(201); + accountId = idOf(created); + }, 120_000); + afterAll(async () => { + await shutdown(booted); + rmSync(root, { recursive: true, force: true }); + }, 60_000); + + it('a rule a later layer replaced by name is judged once, from the copy that wins, and named after that layer', async () => { + const { kernel, engine } = booted!; + const served = ((engine.registry.getObject(ACCOUNT) as any)?.validations ?? []) as Array<{ name?: string; condition?: unknown }>; + expect(served.filter((r) => r.name === RULE_REVIEW), 'precondition: one served copy per name').toHaveLength(1); + expect(sourceOf(served.find((r) => r.name === RULE_NOT_NEGATIVE)?.condition), 'precondition: the override\'s copy wins').toBe("record.name == 'blocked'"); + + const refused = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); + expect({ status: refused.status, code: refused.code }).toEqual({ status: 409, code: CODE }); + // RULES' copy of RULE_NOT_NEGATIVE read the field, but the override's + // copy is the one served, and it does not. RULE_REVIEW is listed once, + // from the override's copy, which still reads it. + expect(String(refused.message)).toContain(`cannot be disabled: 2 active validation rules read fields`); + expect(String(refused.message)).toContain(`— ${listingOfRules([ + { packageId: RULES, object: VISIT, rule: RULE_VISIT_HOP, fields: [`account.${CREDIT}`] }, + { packageId: OVERRIDE, object: ACCOUNT, rule: RULE_REVIEW, fields: [CREDIT] }, + ])}. Switch off or change those rules first`); + expect(String(refused.message)).not.toContain(RULE_NOT_NEGATIVE); + }); + + it('a tenant overlay\'s switch is honoured, its own rule is named after the overlay, and the disable proceeds once every served reader is off', async () => { + const { kernel, engine } = booted!; + // The tenant switches the packaged rule off through the overlay (the + // one switch a tenant admin has on a read-only module), and adds a rule + // of its own that reads the field. + engine.registry.registerObject(accountOverlay([ + switchedOff(RULE_REVIEW, 'Switched off by the tenant'), + { name: RULE_OVERLAY_OWN, type: 'script', severity: 'error', message: 'Credit is capped', condition: `record.${CREDIT} > 9000000` }, + ]) as any, APP, undefined, 'overlay'); + const served = ((engine.registry.getObject(ACCOUNT) as any)?.validations ?? []) as Array<{ name?: string; active?: unknown; condition?: unknown }>; + const review = served.find((r) => r.name === RULE_REVIEW); + expect({ active: review?.active, condition: sourceOf(review?.condition) }, 'precondition: the packaged copy runs, switched off') + .toEqual({ active: false, condition: `record.${CREDIT} > 5000` }); + + const overlayOwn = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); + expect({ status: overlayOwn.status, code: overlayOwn.code }).toEqual({ status: 409, code: CODE }); + expect(String(overlayOwn.message)).toContain(`— ${listingOfRules([ + { packageId: RULES, object: VISIT, rule: RULE_VISIT_HOP, fields: [`account.${CREDIT}`] }, + { packageId: `tenant overlay of ${APP}`, object: ACCOUNT, rule: RULE_OVERLAY_OWN, fields: [CREDIT] }, + ])}. Switch off or change those rules first`); + expect(String(overlayOwn.message)).not.toContain(RULE_REVIEW); + expect(String(overlayOwn.message)).not.toContain('(no package)'); + + // Every served reader switched off through the overlays: the disable proceeds. + engine.registry.registerObject(accountOverlay([switchedOff(RULE_REVIEW, 'Switched off by the tenant')]) as any, APP, undefined, 'overlay'); + engine.registry.registerObject(visitOverlay([switchedOff(RULE_VISIT_HOP, 'Switched off by the tenant')]) as any, APP, undefined, 'overlay'); + const disabled = await dispatchAs(kernel, 'PATCH', `/packages/${MODULE}/disable`); + expect(disabled.status, String(disabled.message)).toBe(200); + const fields = await servedFieldNames(kernel); + for (const field of MODULE_FIELDS) expect(fields, 'the overlay\'s copies of the module\'s fields are withheld too').not.toContain(field); + const written = await dispatchAs(kernel, 'PATCH', `/data/${ACCOUNT}/${accountId}`, { name: 'Acme Ltd' }); + expect(written.status, String(written.message)).toBe(200); + }); +}); + +describe('[#30379 contract review] POST /packages with enableOnInstall: false asks the same scan', () => { + const OWNER = 'com.qa.inst.owner'; + const EXTENDER = 'com.qa.inst.flags'; + const READER = 'com.qa.inst.reader'; + const THING = 'qz_thing'; + const FLAG = 'qz_flag'; + const RULE = 'qz_flag_required'; + const CODE = 'PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE'; + const manifestOf = (id: string) => ({ id, name: id, namespace: 'qz', version: '1.0.0', type: 'module' }); + + let previousHome: string | undefined; + beforeAll(() => { + previousHome = process.env.OS_HOME; + process.env.OS_HOME = mkdtempSync(join(home, 'install-')); + }); + afterAll(() => { + process.env.OS_HOME = previousHome; + }); + + /** A registry with the extender installed and enabled, its field on the owner's object, and (optionally) a rule of another package that reads it. */ + async function door(withReader: boolean) { + const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' } as any); + (registry as any).logLevel = 'silent'; + const kernel: any = { + getService: (name: string) => (name === 'objectql' ? Promise.resolve({ registry }) : null), + context: { getService: () => null }, + }; + const dispatcher = new HttpDispatcher(kernel); + const install = (body: unknown) => dispatcher.handlePackages('', 'POST', body, {}, { + request: {}, + executionContext: { userId: 'u_admin', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'] }, + } as any); + const first = await install({ manifest: manifestOf(EXTENDER) }); + expect(first.response?.status, 'precondition: the extender installs').toBe(201); + registry.registerObject({ name: THING, label: 'Thing', fields: { name: { name: 'name', type: 'text', label: 'Name' } } } as any, OWNER); + registry.registerObject({ name: THING, fields: { [FLAG]: { name: FLAG, type: 'boolean', label: 'Flag' } } } as any, EXTENDER, undefined, 'extend'); + if (withReader) { + registry.registerObject({ + name: THING, + validations: [{ name: RULE, type: 'script', severity: 'error', message: 'Flag it', condition: `record.${FLAG} == null` }], + } as any, READER, undefined, 'extend'); + } + return { registry, install }; + } + + it('an overwrite that asks to come back disabled is refused while a rule reads its field, and the package stays enabled', async () => { + const { registry, install } = await door(true); + const result = await install({ manifest: manifestOf(EXTENDER), overwrite: true, enableOnInstall: false }); + expect({ status: result.response?.status, code: result.response?.body?.error?.code }).toEqual({ status: 409, code: CODE }); + expect(String(result.response?.body?.error?.message)).toContain( + `Package '${EXTENDER}' was installed and stays enabled: \`enableOnInstall: false\` cannot disable it: an active validation rule reads a field it adds through objectExtensions, which the disable would withhold — ${READER} · ${THING} · ${RULE} (reads ${FLAG})`, + ); + expect(registry.getPackage(EXTENDER)?.enabled).toBe(true); + expect(loadDisabledPackageIds(undefined).has(EXTENDER)).toBe(false); + expect(Object.keys((registry.getObject(THING) as any)?.fields ?? {})).toContain(FLAG); + }); + + it('control: with no rule reading the field the same overwrite installs it disabled', async () => { + const { registry, install } = await door(false); + const result = await install({ manifest: manifestOf(EXTENDER), overwrite: true, enableOnInstall: false }); + expect(result.response?.status).toBe(201); + expect(registry.getPackage(EXTENDER)?.enabled).toBe(false); + expect(loadDisabledPackageIds(undefined).has(EXTENDER)).toBe(true); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index d640db0e5b8..7b5aecef2b8 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -918,20 +918,29 @@ function declaredRulesOf(body: unknown): Array<{ name?: unknown; active?: unknow * * ## Which rules are judged * - * Every rule the served object would still carry, of every severity, on every - * object whose owner is enabled and is not the package itself (a disabled - * owner's object is refused whole, so no write reaches its rules): + * The rules the served object would carry once the disable lands, of every + * severity, on every object whose owner is enabled and is not the package + * itself (a disabled owner's object is refused whole, so no write reaches its + * rules), read off the registry's own fold + * (`getAllObjects(undefined, { withoutExtensionsOf })`), never re-assembled + * here: * - * - the base layer's (the tenant overlay when one is registered, the owner - * otherwise, as the fold selects it), less the rules the disable withholds - * with the fields; - * - each `extend` contribution of any enabled package other than this one, - * declared dependent or not, an app's `.custom` pair (where - * tenant-written rules live) included. + * - ONE rule per name, the copy the fold serves: a later layer's rule replaces + * an earlier one of its name (`mergeValidationsByName`); + * - with a tenant overlay's `message` and `active` on it (ADR-0029 D9.2a): a + * packaged rule the tenant switched off through the overlay is off; + * - from the base layer (the tenant overlay when one is registered, the owner + * otherwise) and each `extend` contribution of any enabled package other + * than this one, declared dependent or not, an app's `.custom` pair + * (where tenant-written rules live) included; less the rules the disable + * withholds with the fields, which it also takes off a base that carries a + * copy of them. * - * A rule switched off (`active: false`) reads nothing: switching the rule off - * is one of the remedies the refusal names. Formula fields are not judged: a - * formula over a withheld field answers `null`, never a refused write. + * Each rule is named after the LAST layer in fold order that declares its name + * (the layer its text comes from; a tenant overlay by that name). A rule + * switched off (`active: false`) reads nothing: switching the rule off is one + * of the remedies the refusal names. Formula fields are not judged: a formula + * over a withheld field answers `null`, never a refused write. * * A rule reads a withheld field when it names it on its own object, or names * it one hop through a reference field whose target is an object the package @@ -990,46 +999,61 @@ function readDisableDependentRules(registry: any, packageId: string): DisableDep } if (withheldFields.size === 0) return []; + // [#30379 contract review, must-fix 1] Every object as the served fold will + // serve it once the disable lands: `getAllObjects`' own fold with this + // package's `extend` contributions left out (an already-disabled package's + // are withheld on every fold), so a rule is judged ONCE per name, from the + // copy that wins (`mergeValidationsByName`), carrying the tenant overlay's + // `message` and `active` (`tenantRuleSettingsOf` / `withPackagedOwnerRules`). + // The one step a left-out fold does not take, a disable does: a base that + // carries a copy of a rule only the leaving extensions add has it taken off + // (`withoutWithheldMembers`), so those names are dropped below. + const after = new Map; validations?: unknown }>(); + for (const object of registry.getAllObjects(undefined, { withoutExtensionsOf: packageId }) as Array<{ name?: unknown }>) { + if (typeof object?.name === 'string') after.set(object.name, object as { fields?: Record; validations?: unknown }); + } + const dependents: DisableDependentRule[] = []; for (const name of names) { const contributors = registry.getObjectContributors(name) as Contributor[]; const owner = contributors.find((c) => c.ownership === 'own'); if (!owner || owner.packageId === packageId || disabled(owner.packageId)) continue; + const served = after.get(name); + if (!served) continue; const base = contributors.find((c) => c.ownership === 'overlay') ?? owner; - const servedFields = ((registry.getObject?.(name) as { fields?: Record } | undefined)?.fields) - ?? ((base.definition as { fields?: Record } | undefined)?.fields) ?? {}; + const servedFields = served.fields ?? {}; const ownWithheld = withheldFields.get(name); const rulesWithheld = withheldRules.get(name) ?? new Set(); - const sources: Array<{ packageId: unknown; rules: Array<{ name?: unknown; active?: unknown }> }> = [ - { packageId: base.packageId, rules: declaredRulesOf(base.definition).filter((rule) => !(typeof rule.name === 'string' && rulesWithheld.has(rule.name))) }, - ]; - for (const c of contributors) { - if (c.ownership !== 'extend' || leaves(c)) continue; - sources.push({ packageId: c.packageId, rules: declaredRulesOf(c.definition) }); + // The layer each served rule's text comes from: the LAST layer, in fold + // order (the base, then each kept `extend` contribution), that declares + // a rule of its name — the copy `mergeValidationsByName` serves. + const layerOf = new Map(); + for (const layer of [base, ...contributors.filter((c) => c.ownership === 'extend' && !leaves(c))]) { + for (const rule of declaredRulesOf(layer.definition)) { + if (typeof rule.name === 'string') layerOf.set(rule.name, layer); + } } - for (const source of sources) { - for (const rule of source.rules) { - if (rule.active === false || typeof rule.name !== 'string') continue; - const reads: RuleFieldReads = { own: new Set(), hops: new Map() }; - addRuleFieldReads(rule, reads, 0); - const hit: string[] = []; - if (ownWithheld) for (const field of reads.own) if (ownWithheld.has(field)) hit.push(field); - for (const [ref, related] of reads.hops) { - const target = referenceTargetOf(servedFields[ref]); - const targetWithheld = target === undefined ? undefined : withheldFields.get(target); - if (!targetWithheld) continue; - for (const field of related) if (targetWithheld.has(field)) hit.push(`${ref}.${field}`); - } - if (hit.length === 0) continue; - dependents.push({ - packageId: typeof source.packageId === 'string' ? source.packageId : '(no package)', - object: name, - rule: rule.name, - fields: [...new Set(hit)].sort(), - }); + for (const rule of declaredRulesOf(served)) { + if (rule.active === false || typeof rule.name !== 'string' || rulesWithheld.has(rule.name)) continue; + const reads: RuleFieldReads = { own: new Set(), hops: new Map() }; + addRuleFieldReads(rule, reads, 0); + const hit: string[] = []; + if (ownWithheld) for (const field of reads.own) if (ownWithheld.has(field)) hit.push(field); + for (const [ref, related] of reads.hops) { + const target = referenceTargetOf(servedFields[ref]); + const targetWithheld = target === undefined ? undefined : withheldFields.get(target); + if (!targetWithheld) continue; + for (const field of related) if (targetWithheld.has(field)) hit.push(`${ref}.${field}`); } + if (hit.length === 0) continue; + dependents.push({ + packageId: ruleLayerLabel(layerOf.get(rule.name) ?? base), + object: name, + rule: rule.name, + fields: [...new Set(hit)].sort(), + }); } } return dependents.sort((a, b) => a.packageId.localeCompare(b.packageId) @@ -1038,17 +1062,28 @@ function readDisableDependentRules(registry: any, packageId: string): DisableDep } /** - * The sentence the disable door's dependent-rule refusal carries: the package, + * The name a served rule's layer goes by in the refusal: its package, or, for + * a tenant overlay (whose `packageId` is the provenance it was saved under, when + * there is one), the overlay itself — never a placeholder. + */ +function ruleLayerLabel(layer: { packageId?: unknown; ownership?: unknown }): string { + const pkg = typeof layer.packageId === 'string' && layer.packageId !== '' ? layer.packageId : undefined; + if (layer.ownership === 'overlay') return pkg === undefined ? 'tenant overlay' : `tenant overlay of ${pkg}`; + return pkg ?? (layer.ownership === 'own' ? 'unpackaged object' : 'unpackaged extension'); +} + +/** + * The sentence a dependent-rule refusal carries: `lead` (what was refused), * then every rule as `package · object · rule (reads FIELD, …)`, then the remedy. */ -function disableDependentRulesMessage(id: string, dependents: readonly DisableDependentRule[]): string { +function disableDependentRulesMessage(lead: string, dependents: readonly DisableDependentRule[]): string { const one = dependents.length === 1; const listed = dependents .map((d) => `${d.packageId} · ${d.object} · ${d.rule} (reads ${d.fields.join(', ')})`) .join('; '); - return `Package '${id}' cannot be disabled: ${one - ? 'a validation rule of an enabled package reads a field' - : `${dependents.length} validation rules of enabled packages read fields`} it adds through objectExtensions, ` + return `${lead}: ${one + ? 'an active validation rule reads a field' + : `${dependents.length} active validation rules read fields`} it adds through objectExtensions, ` + `which the disable would withhold — ${listed}. Switch off or change ${one ? 'that rule' : 'those rules'} first, ` + 'or keep the package enabled.'; } @@ -2139,13 +2174,31 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // declaration carries no `.default()` for the key, so absence // survives the parse and is read as absence. const requestedEnabled = request?.enableOnInstall; + let installDisableRefusal: string | undefined; if (requestedEnabled === true) { const enabled = registry.enablePackage(pkgId); if (enabled) pkg = enabled; await summaryRefreshSettled(qlService); } else if (requestedEnabled === false) { - const disabled = registry.disablePackage(pkgId); - if (disabled) pkg = disabled; + // [#30379 ruling A, contract review should-fix] The same refusal + // as `PATCH /packages/:id/disable`, asked of the state the + // install just left: an overwrite of an enabled package that + // asks to come back disabled would withhold its extension + // fields from rules that stay active, and every write to their + // objects would be refused. The install itself has landed, so + // the package stays ENABLED (the row, the registry and the + // state file below all follow `pkg.enabled`) and the door + // answers the refusal instead of a 201 that claims a disable. + const dependents = readDisableDependentRules(registry, pkgId); + if (dependents.length > 0) { + installDisableRefusal = disableDependentRulesMessage( + `Package '${pkgId}' was installed and stays enabled: \`enableOnInstall: false\` cannot disable it`, + dependents, + ); + } else { + const disabled = registry.disablePackage(pkgId); + if (disabled) pkg = disabled; + } } // ⭐ The DURABLE half, and it follows THE ROW THIS DOOR RETURNED — // never the request's intent. Written unconditionally rather than @@ -2193,6 +2246,16 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin } catch (err) { console.warn('[handlePackages] failed to persist enableOnInstall', { id: pkgId, disabled: rowDisabled, error: (err as Error)?.message }); } + if (installDisableRefusal !== undefined) { + return { + handled: true, + response: deps.error( + installDisableRefusal, + PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE.status, + { code: PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE.code }, + ), + }; + } const res = deps.success(pkg); res.status = 201; return { handled: true, response: res }; @@ -2255,7 +2318,7 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin return { handled: true, response: deps.error( - disableDependentRulesMessage(id, dependents), + disableDependentRulesMessage(`Package '${id}' cannot be disabled`, dependents), PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE.status, { code: PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE.code }, ), From c15ba57084196643bacd7b42344bb95563a09dcf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 07:18:20 +0000 Subject: [PATCH 5/5] fix(runtime): ask the install door's dependent-rule scan only on an overwrite of an enabled package POST /packages with enableOnInstall false asked the scan on every install, so a fresh install (or a reinstall after a delete) of a package whose field an enabled rule already names landed ENABLED against the caller's explicit false, though nothing served is withheld there. The door now reads, before either install limb writes, whether the id's row exists and is enabled, and asks the scan only then; a fresh install and an overwrite of a disabled row honour false exactly as #18877 rules. The docs and the changeset say "an overwrite of an enabled package". The scan follows a rule's record.REF.FIELD hop with referenceTargetOf, which the second-object read census in @objectstack/core holds to a classification: it is named there as not-a-served-read (it reads no record of the target and serves none of its fields). Claude-Session: https://claude.ai/code/session_016WV4cwCWp9qZ5SgMof1qLa Co-authored-by: Claude --- .../30379-disable-door-dependent-rule.md | 8 +++---- content/docs/api/error-catalog.mdx | 2 +- content/docs/api/metadata-api.mdx | 7 +++--- .../docs/deployment/marketplace-handbook.mdx | 7 +++--- .../second-object-read-exposure.pin.test.ts | 9 ++++++++ ...es-disable-module-extension-fields.test.ts | 23 +++++++++++++++---- packages/runtime/src/domains/packages.ts | 13 ++++++++++- 7 files changed, 53 insertions(+), 16 deletions(-) diff --git a/.changeset/30379-disable-door-dependent-rule.md b/.changeset/30379-disable-door-dependent-rule.md index 5cd7b9c876b..0eb2b99f99a 100644 --- a/.changeset/30379-disable-door-dependent-rule.md +++ b/.changeset/30379-disable-door-dependent-rule.md @@ -4,19 +4,19 @@ fix(runtime)!: `PATCH /api/v1/packages/:id/disable` refuses while an enabled package's validation rule reads a field the package adds through `objectExtensions` -Clause-②: no (narrowing) — the package disable door, and the install door's `enableOnInstall: false` arm, refuse (`409` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`) a disable they accepted before: one that would withhold a field an active validation rule reads. Nothing is widened: no export, request shape, response member or authorable key is added, and the code is the one #30724 registered. +Clause-②: no (narrowing) — the package disable door, and the install door's `enableOnInstall: false` arm on an overwrite of an enabled package, refuse (`409` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`) a disable they accepted before: one that would withhold a field an active validation rule reads. Nothing is widened: no export, request shape, response member or authorable key is added, and the code is the one #30724 registered. Metadata-protocol: none Skills-docs: updated here — `content/docs/api/metadata-api.mdx` (the disable door's entry), `content/docs/deployment/marketplace-handbook.mdx` (the Enable-and-disable step and the Disabling-a-module list) and `content/docs/api/error-catalog.mdx` (the code's 409 entry) name the refusal; `skills/**` teaches no package disable door (grep for `packages/:id/disable`, `disableable`, `WRITABLE_PACKAGE_REQUIRED`: none), so no skill or eval moves. -**BREAKING** accept-set narrowing on the package disable door and the install door's `enableOnInstall: false` arm (`@objectstack/runtime`, `domains/packages.ts`), for one case only. +**BREAKING** accept-set narrowing on the package disable door and the install door's `enableOnInstall: false` arm on an overwrite of an enabled package (`@objectstack/runtime`, `domains/packages.ts`), for one case only. - **Before.** A disabled package's `objectExtensions` fields are withheld from the object they extend. When a validation rule of another package that stayed enabled read one of those fields, the disable answered `200`, and every write to that object was then refused (`400 VALIDATION_FAILED`, naming the rule) for as long as the package stayed off. - **Now.** The door refuses such a disable with `409` and `error.code` `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE`, after the read-only check and before anything changes: the package row, the served objects and the environment's stored disable state stay as they were. The message names every reading rule as `package · object · rule (reads FIELD)`, one or several. The refusal carries no other `error.details`. - **Who is scanned.** Any enabled package, declared dependent or not (no `manifest.dependencies` entry is read), an app's `.custom` pair (where tenant-written rules live) included, at every severity. Each rule is judged as the object will serve it once the disable lands: one rule per name (a later layer's copy replaces an earlier one), with a tenant overlay's `message` and `active` on it, so a packaged rule the tenant switched off through the overlay does not refuse the disable. The message names each rule after the layer whose copy is served (`tenant overlay` for a rule the overlay declares itself). A rule reads a field when its predicate names it on the rule's own object or one hop through a reference field of that object (`record.account.FIELD`), or when it is a `state_machine` / `format` / `json_schema` rule on that field. -- **The install door too.** `POST /api/v1/packages` with `enableOnInstall: false` (an overwrite of an enabled package that asks to come back disabled) asks the same scan before it disables: such an install still lands, the package stays enabled, and the door answers the same `409` instead of a `201` claiming a disable. +- **The install door too, on an overwrite of an enabled package.** `POST /api/v1/packages` with `overwrite` and `enableOnInstall: false`, over a package that is enabled before the install, asks the same scan before it disables: such an overwrite still lands, the package stays enabled, and the door answers the same `409` instead of a `201` claiming a disable. A fresh install (or a reinstall after a delete) and an overwrite of a disabled package withhold nothing a rule is served against, and land disabled as asked, as before. - **Unchanged.** A formula over the field is no reason to refuse (it answers `null`), nor is a rule switched off (`active: false`) or a rule of a package that is itself disabled. Every other disable answers as before. -- **Who moves.** An administrator or client disabling such a package, or installing it with `enableOnInstall: false`: a refusal naming the rules, instead of a disable that left the object refusing every write. +- **Who moves.** An administrator or client disabling such a package, or overwriting it, while enabled, with `enableOnInstall: false`: a refusal naming the rules, instead of a disable that left the object refusing every write. **The remedy.** Switch off or change each named rule first, or disable the package that owns it, then disable the package; or keep the package enabled. diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index f1d9dba1f21..3c8a2fa1a2a 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -547,7 +547,7 @@ spelling never crosses HTTP. **Retry:** `retry_backoff` ### `PACKAGE_DISABLE_RESTRICTED_BY_VALIDATION_RULE` -**Cause:** `PATCH /api/v1/packages/:id/disable` was refused because a validation rule of an enabled package (declared dependent or not, an app's `.custom` pair included, any severity) reads a field this package adds to another object through `objectExtensions`. Disabling it would withhold that field, and every write to the object would then fail the rule. Each rule is judged as the object serves it: one rule per name (the copy a later layer declares wins), with a tenant overlay's switch honoured. The message names each rule as `package · object · rule (reads FIELD)`, one or several, after the layer whose copy is served (`tenant overlay` for a rule the overlay declares itself). Nothing changed. `POST /api/v1/packages` with `enableOnInstall: false` answers the same code when that install would leave such a package disabled: the install itself lands and the package stays enabled. +**Cause:** `PATCH /api/v1/packages/:id/disable` was refused because a validation rule of an enabled package (declared dependent or not, an app's `.custom` pair included, any severity) reads a field this package adds to another object through `objectExtensions`. Disabling it would withhold that field, and every write to the object would then fail the rule. Each rule is judged as the object serves it: one rule per name (the copy a later layer declares wins), with a tenant overlay's switch honoured. The message names each rule as `package · object · rule (reads FIELD)`, one or several, after the layer whose copy is served (`tenant overlay` for a rule the overlay declares itself). Nothing changed. `POST /api/v1/packages` answers the same code for an overwrite of an enabled package with `enableOnInstall: false` that would leave such a package disabled: the overwrite itself lands and the package stays enabled. A fresh install with `enableOnInstall: false` lands disabled, as before. **Fix:** Switch off or change each named rule first, or disable the package that owns it, then disable this package; or keep it enabled. A formula over the field is no reason to refuse: it answers `null`. **Retry:** `no_retry` diff --git a/content/docs/api/metadata-api.mdx b/content/docs/api/metadata-api.mdx index d85fe387e4e..bfe4b0b97aa 100644 --- a/content/docs/api/metadata-api.mdx +++ b/content/docs/api/metadata-api.mdx @@ -191,9 +191,10 @@ names each rule as `package · object · rule (reads FIELD)`, and nothing change A rule is judged as the object serves it: one rule per name, a tenant overlay's switch honoured. Switch off or change each named rule first, or disable the package that owns it. A formula over the field does not refuse the disable; it -answers `null`. `POST /packages` with `enableOnInstall: false` answers the same -`409` when the install would leave such a package disabled; the install lands -and the package stays enabled. +answers `null`. `POST /packages` answers the same `409` for an overwrite of an +enabled package with `enableOnInstall: false` that would leave such a package +disabled; the overwrite lands and the package stays enabled. A fresh install +with `enableOnInstall: false` lands disabled, as before. --- diff --git a/content/docs/deployment/marketplace-handbook.mdx b/content/docs/deployment/marketplace-handbook.mdx index 2383eda54b0..b555a68b59c 100644 --- a/content/docs/deployment/marketplace-handbook.mdx +++ b/content/docs/deployment/marketplace-handbook.mdx @@ -316,9 +316,10 @@ its data is kept. fail the rule. Switch off or change each named rule first (a rule the customer switched off on their copy of the object counts as off), or disable the package that owns it. A formula over the field does not block the - disable: it answers `null` while the module is off. An install with - `enableOnInstall: false` that would leave the module disabled in that state - answers the same `409`, and the module stays enabled. + disable: it answers `null` while the module is off. An overwrite of the + enabled module with `enableOnInstall: false` that would leave it disabled in + that state answers the same `409`, and the module stays enabled; a fresh + install with `enableOnInstall: false` lands disabled, as before. **What your app's own code meets.** A hook of your app that reads or writes a module's object through `ctx.api.object('')` runs inside the server, diff --git a/packages/core/src/security/second-object-read-exposure.pin.test.ts b/packages/core/src/security/second-object-read-exposure.pin.test.ts index 3002024fb8a..39154b10658 100644 --- a/packages/core/src/security/second-object-read-exposure.pin.test.ts +++ b/packages/core/src/security/second-object-read-exposure.pin.test.ts @@ -313,6 +313,15 @@ const CALLERS: Record = { "ADDRESSED object, whose record is `getData` behind the route's own `get` exposure gate, and the related rows " + "are the child's `findData` behind the child's `list` exposure. Pinned by `packages/rest/src/record-page-routes.test.ts`", }, + // [#30379] The package disable door's dependent-rule scan. + 'packages/runtime/src/domains/packages.ts': { + kind: 'not-a-served-read', + reason: + "the package disable door's dependent-rule scan (`readDisableDependentRules`, also asked by the install door's " + + '`enableOnInstall: false` arm): a rule\'s `record.REF.FIELD` hop names the object REF points at, only to ask ' + + 'whether FIELD is one the disable would withhold from it, from the registry\'s DECLARED metadata. It reads no ' + + 'record of the target and serves none of its fields; the door answers a `409` naming the rule, or proceeds', + }, }; const SCAN_TIMEOUT_MS = 60_000; diff --git a/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts b/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts index 12caedc9fc7..fda07dc90d8 100644 --- a/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts +++ b/packages/runtime/src/domains/packages-disable-module-extension-fields.test.ts @@ -786,8 +786,11 @@ describe('[#30379 contract review] POST /packages with enableOnInstall: false as process.env.OS_HOME = previousHome; }); - /** A registry with the extender installed and enabled, its field on the owner's object, and (optionally) a rule of another package that reads it. */ - async function door(withReader: boolean) { + /** + * A registry with the extender's field on the owner's object and (optionally) a rule of another package that + * reads it; the extender is installed and enabled first unless `installed: false` (a fresh install, then). + */ + async function door(withReader: boolean, { installed = true }: { installed?: boolean } = {}) { const registry = new SchemaRegistry({ multiTenant: false, collisionPolicy: 'error' } as any); (registry as any).logLevel = 'silent'; const kernel: any = { @@ -799,8 +802,10 @@ describe('[#30379 contract review] POST /packages with enableOnInstall: false as request: {}, executionContext: { userId: 'u_admin', systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'] }, } as any); - const first = await install({ manifest: manifestOf(EXTENDER) }); - expect(first.response?.status, 'precondition: the extender installs').toBe(201); + if (installed) { + const first = await install({ manifest: manifestOf(EXTENDER) }); + expect(first.response?.status, 'precondition: the extender installs').toBe(201); + } registry.registerObject({ name: THING, label: 'Thing', fields: { name: { name: 'name', type: 'text', label: 'Name' } } } as any, OWNER); registry.registerObject({ name: THING, fields: { [FLAG]: { name: FLAG, type: 'boolean', label: 'Flag' } } } as any, EXTENDER, undefined, 'extend'); if (withReader) { @@ -824,6 +829,16 @@ describe('[#30379 contract review] POST /packages with enableOnInstall: false as expect(Object.keys((registry.getObject(THING) as any)?.fields ?? {})).toContain(FLAG); }); + it('[delta review, must-fix 3] a FRESH install with enableOnInstall: false lands disabled as asked, though a rule names its field', async () => { + // No row before the install, so nothing served is withheld: #18877's 「有旗 = 设置」 holds, and the scan is not asked. + const { registry, install } = await door(true, { installed: false }); + expect(registry.getPackage(EXTENDER), 'precondition: no row before the install').toBeUndefined(); + const result = await install({ manifest: manifestOf(EXTENDER), enableOnInstall: false }); + expect(result.response?.status, String(result.response?.body?.error?.message)).toBe(201); + expect(registry.getPackage(EXTENDER)?.enabled).toBe(false); + expect(loadDisabledPackageIds(undefined).has(EXTENDER)).toBe(true); + }); + it('control: with no rule reading the field the same overwrite installs it disabled', async () => { const { registry, install } = await door(false); const result = await install({ manifest: manifestOf(EXTENDER), overwrite: true, enableOnInstall: false }); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 7b5aecef2b8..b1cec174829 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -2062,6 +2062,13 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // mode is stored as sent — unchanged by this gate, and not a class // this door answers for.) const settings = request?.settings; + // [#30379 delta review, must-fix 3] Whether this install OVERWRITES a + // row that is enabled right now, read before either limb writes. Only + // then can `enableOnInstall: false` withhold a field a rule is served + // against; a fresh install (or a reinstall after a delete) and an + // overwrite of a disabled row withhold nothing served, so they honour + // `false` exactly as #18877 rules. + const overwritesEnabledRow = (registry.getPackage(pkgId) as { enabled?: unknown } | undefined)?.enabled === true; let pkg: any; const protocolSvc = await resolveProtocol(deps, _context).catch(() => null); try { @@ -2189,7 +2196,11 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // the package stays ENABLED (the row, the registry and the // state file below all follow `pkg.enabled`) and the door // answers the refusal instead of a 201 that claims a disable. - const dependents = readDisableDependentRules(registry, pkgId); + // [delta review, must-fix 3] Asked ONLY on an overwrite of a row + // that was enabled before this install: a fresh install, a + // reinstall after a delete and an overwrite of a disabled row + // withhold nothing served, and land disabled as asked. + const dependents = overwritesEnabledRow ? readDisableDependentRules(registry, pkgId) : []; if (dependents.length > 0) { installDisableRefusal = disableDependentRulesMessage( `Package '${pkgId}' was installed and stays enabled: \`enableOnInstall: false\` cannot disable it`,