diff --git a/.changeset/owd-authoring-gate-authoring-channel.md b/.changeset/owd-authoring-gate-authoring-channel.md new file mode 100644 index 0000000000..ed822eb853 --- /dev/null +++ b/.changeset/owd-authoring-gate-authoring-channel.md @@ -0,0 +1,9 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +Fix: the #3050 pre-persistence authoring gate now keys on the declared `authoringChannel` instead of `environmentId`, so ADR-0090 D11 object posture enforcement reaches host-config deployments. + +The gate call site in `saveMetaItem` was wrapped in `if (this.environmentId !== undefined)`. The CLI's lightweight host-config assembler constructs `new ObjectQLPlugin()` with no options, leaving `environmentId` undefined while serving an end-user `PUT /api/v1/meta/*` — so plugin-security's object posture gate (`owd_widening_forbidden` / `owd_external_wider`) ran on no self-hosted deployment at all. This is the same proxy-signal hazard #6710 retired for the sibling #4463 gate; the two doors now read one declared key. + +Behaviour change for self-hosted deployments: an object write whose `externalSharingModel` is wider than its `sharingModel` — or an environment overlay that widens a packaged object's OWD — is now refused with `403` (`owd_external_wider` / `owd_widening_forbidden`) on the draft path, the active path and package authoring, instead of being accepted. Fix the posture in the object definition; widening a packaged object legitimately is authored in the package source and published (ADR-0090 D7). A kernel that declares `authoringChannel: 'package-author'` is unaffected — package authoring stays gated at build time by `validateSecurityPosture`. diff --git a/packages/metadata-protocol/src/mutation-listeners.test.ts b/packages/metadata-protocol/src/mutation-listeners.test.ts index d08b14a83f..8bfbcb020b 100644 --- a/packages/metadata-protocol/src/mutation-listeners.test.ts +++ b/packages/metadata-protocol/src/mutation-listeners.test.ts @@ -119,10 +119,14 @@ describe('ObjectStackProtocolImplementation.registerMutationProjector (ADR-0094) // #3050 — the pre-persistence AUTHORING GATE seam (ADR-0094 addendum). The // inverse contract of the projector: it runs BEFORE persistence and a throw -// PROPAGATES (rejecting the write) instead of being swallowed. saveMetaItem -// invokes it for env writes only, both draft and publish-mode saves; the -// domain-gate behavior itself (OWD posture) is pinned in plugin-security's -// object-posture-gate suite. +// PROPAGATES (rejecting the write) instead of being swallowed. [#7674] +// saveMetaItem invokes it on every channel except a declared `package-author` +// one — both draft and publish-mode saves; it used to say "for env writes +// only", which was the `environmentId` proxy that left the gate dead on every +// host-config deployment. The domain-gate behavior itself (OWD posture) is +// pinned in plugin-security's object-posture-gate suite, and its journey +// through the real `PUT /api/v1/meta/object/*` in +// `packages/rest/src/meta-object-owd-gate.test.ts`. describe('ObjectStackProtocolImplementation.registerAuthoringGate (#3050)', () => { const save = (over: Record = {}) => ({ type: 'object', name: 'crm_account', state: 'active' as const, body: { sharingModel: 'private' }, ...over, diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index 33f6daae7c..e052819605 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -497,27 +497,73 @@ describe('#6710 — gate activation is keyed on the declared authoring channel', expect(shouted[0]).toContain('approval-expression-invalid'); }); - it('does not disturb the OTHER gates that legitimately read environmentId', async () => { - // #6710 re-keys ONE activation. The #3050 authoring gate keeps its own - // `environmentId !== undefined` scope check, and it must stay keyed - // there — that gate really is about row scope. Declaring the - // package-author channel must not switch it on for a control-plane - // kernel, and must not switch it off for a tenant one. + // [#7674] REPLACED, not re-spelled. The case that stood here asserted the + // opposite invariant — "the #3050 authoring gate keeps its own + // `environmentId !== undefined` scope check, and it must stay keyed there" + // — and that sentence was the defect, written down as a pin. #6710 retired + // the proxy for the #4463 gate and left its sibling on it, so the ADR-0090 + // D11 object posture gate (`owd_widening_forbidden` / `owd_external_wider`) + // ran on NO host-config deployment: `new ObjectQLPlugin()` leaves + // `environmentId` undefined and serves an end-user `PUT /api/v1/meta/*`. + // The old case could not see that, because it drove the control-plane row + // (undefined) only through the `package-author` channel — the one column + // where both keys agree. + // + // The four-cell matrix below is what makes the two keys distinguishable. + // Note the one cell whose verdict FLIPS: `('env_test', 'package-author')` + // was gated and is not any more. That is #6710's direction applied + // honestly rather than half-applied — a kernel that claims to BE the + // package author is treated as one by both doors, and package authoring is + // gated at build time instead (`validateSecurityPosture` is `CLI_ONLY` in + // `AUTHORING_RULES`, and R1's own message prescribes exactly that route: + // "widen it in the package source and publish through the package + // pipeline"). No assembly in this repo declares that channel today; only + // the genuine control plane may. + it.each([ + { envId: undefined, channel: undefined, gated: true, why: 'THE DEFECT: the host-config assembler — `new ObjectQLPlugin()`, no environment id, undeclared channel ⇒ the fail-safe default' }, + { envId: 'env_test', channel: undefined, gated: true, why: 'the ordinary tenant kernel, unchanged' }, + { envId: undefined, channel: 'package-author' as const, gated: false, why: 'the genuine control-plane bootstrap kernel' }, + { envId: 'env_test', channel: 'package-author' as const, gated: false, why: 'a declared package author that also carries a row scope — the cell that flips' }, + ])('#3050 gate: environmentId=$envId channel=$channel ⇒ gated=$gated ($why)', async ({ envId, channel, gated }) => { const seen: string[] = []; - const gate = (ctx: { type: string; name: string }) => { seen.push(`${ctx.type}/${ctx.name}`); }; + const { protocol } = makeProtocolOn(envId, channel); + protocol.registerAuthoringGate('flow', (ctx: { type: string; name: string }) => { + seen.push(`${ctx.type}/${ctx.name}`); + }); - const cp = makeProtocolOn(undefined, 'package-author'); - cp.protocol.registerAuthoringGate('flow', gate); - await cp.protocol.saveMetaItem({ type: 'flow', name: 'leave_approval', item: validApprovalFlow() }); - expect(seen, 'the #3050 gate stays OFF where environmentId is undefined').toEqual([]); + // A body the #4463 rules ACCEPT, so what this matrix measures is the + // #3050 dispatch alone: a broken body would be refused upstream on the + // two `'environment'` rows and the gate would never be reached, which + // would make the two keys look identical again. + await protocol.saveMetaItem({ type: 'flow', name: 'leave_approval', item: validApprovalFlow() }); - const tenant = makeProtocolOn('env_test', 'package-author'); - tenant.protocol.registerAuthoringGate('flow', gate); - await tenant.protocol.saveMetaItem({ type: 'flow', name: 'leave_approval', item: brokenApprovalFlow() }); - expect( - seen, - 'the #3050 gate stays ON where environmentId is set, even though the ' - + '#4463 gate was waived by the channel declaration — two gates, two keys', - ).toEqual(['flow/leave_approval']); + expect(seen).toEqual(gated ? ['flow/leave_approval'] : []); + }); + + it('the #3050 gate and the #4463 gate now read ONE key, and `environmentId` keeps only row scope', async () => { + // The positive statement of the matrix above: the two doors that ask + // "is this an author publishing?" can no longer disagree, which is the + // property whose absence let #7674 outlive #6710 by one gate. + const seen: string[] = []; + const gate = (ctx: { type: string; name: string }) => { seen.push(`${ctx.type}/${ctx.name}`); }; + + // Host config: #4463 refuses the broken body (422) AND #3050 would have + // run — the write never reaches persistence either way, and both gates + // are live on the topology that had neither. + const host = makeProtocolOn(undefined); + host.protocol.registerAuthoringGate('flow', gate); + const err = await host.protocol + .saveMetaItem({ type: 'flow', name: 'leave_approval', item: brokenApprovalFlow() }) + .catch((e: any) => e); + expect(err.status).toBe(422); + expect(err.code).toBe('INVALID_METADATA'); + expect(flowRows(host.rows), 'refused before persistence').toEqual([]); + + // …and the same host config, given a body the rules accept, runs the + // #3050 gate and stores the row. "Gated" must not mean "refuses + // everything". + await host.protocol.saveMetaItem({ type: 'flow', name: 'leave_approval', item: validApprovalFlow() }); + expect(seen).toEqual(['flow/leave_approval']); + expect(flowRows(host.rows)).toHaveLength(1); }); }); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index e782ef6afe..43f24e662f 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -2740,9 +2740,16 @@ export class ObjectStackProtocolImplementation implements * * [#6710] Row scoping ONLY. This key keeps every one of its other jobs — * the `environment_id` stamp/filter, the ADR-0005 overlay-whitelist gate, - * the #3050 authoring-gate scope, the local metadata-storage provisioning - * decision — but it no longer decides whether the #4463 runtime authoring - * rules run. See {@link authoringChannel}. + * the local metadata-storage provisioning decision — but it no longer + * decides whether the #4463 runtime authoring rules run. + * See {@link authoringChannel}. + * + * [#7674] …and no longer whether the #3050 pre-persistence authoring gate + * runs either. The sentence above used to list "the #3050 authoring-gate + * scope" among this key's surviving jobs, and that call site kept the + * `environmentId !== undefined` wrapper #6710 had just retired next door — + * so the identical proxy-signal defect outlived its own diagnosis on the + * sibling gate. Both doors read {@link authoringChannel} now. */ private environmentId?: string; @@ -2751,10 +2758,12 @@ export class ObjectStackProtocolImplementation implements * which is what makes the #4463 gate active on every kernel that does not * explicitly claim to be the package author's own bootstrap channel. * - * Read by {@link assertRuntimeAuthoringRules} and nothing else — this is - * deliberately NOT a general-purpose authorization key. The ADR-0005 - * overlay gate and the #3050 authoring gate keep reading `environmentId`, - * because those two really are about row scope. + * Read by {@link assertRuntimeAuthoringRules} and, since #7674, by the + * #3050 pre-persistence authoring gate's call site in {@link saveMetaItem} + * — the two doors that ask "is this an AUTHOR publishing, or the package + * author's own bootstrap?". It is deliberately NOT a general-purpose + * authorization key: the ADR-0005 overlay-whitelist gate keeps reading + * `environmentId`, because that one really is about row scope. */ private authoringChannel: MetadataAuthoringChannel; @@ -3089,8 +3098,12 @@ export class ObjectStackProtocolImplementation implements // mechanism, because the failure mode being designed out is precisely // "a new assembly variant nobody thought about". // - // `environmentId` keeps every other job it has, including the #3050 - // authoring gate's own scope check below. + // `environmentId` keeps its row-scoping jobs — the `environment_id` + // stamp/filter and the ADR-0005 overlay-whitelist gate. [#7674] It no + // longer keys the #3050 authoring gate below either: #6710 re-keyed + // this activation and left that one on the retired proxy, which cost + // the ADR-0090 D11 object posture gate every host-config deployment + // until #7674 finished the move. if (this.authoringChannel === 'package-author') return []; if (evt.state !== 'active') return []; // `os migrate meta --stored --apply` rewrites rows that ALREADY EXIST @@ -10052,10 +10065,32 @@ export class ObjectStackProtocolImplementation implements // Pre-persistence authoring gate (#3050): a domain plugin may veto the // body before it persists (throws propagate to the caller with their // status/code). Runs for BOTH draft and publish-mode saves, so a later - // publishMetaItem promotes an already-gated body. Environment writes - // only — control-plane bootstrap writes (environmentId undefined) are - // the package author's own channel, mirroring the ADR-0005 gate above. - if (this.environmentId !== undefined) { + // publishMetaItem promotes an already-gated body. + // + // [#7674] Keyed on the DECLARED authoring channel, exactly as #6710 + // re-keyed `assertRuntimeAuthoringRules` a few hundred lines up. This + // line used to read `if (this.environmentId !== undefined)`, and its + // own comment reaffirmed the reasoning #6710 had already retired: + // "control-plane bootstrap writes (environmentId undefined) are the + // package author's own channel". They are not the only such writes. + // The CLI's lightweight host-config assembler (`serve.ts`'s + // `config.objects && !hasObjectQL` branch → `new ObjectQLPlugin()` + // with no options) leaves `environmentId` undefined too, and it serves + // an END-USER `PUT /api/v1/meta/*` — `isHostConfig` → + // `shouldBootWithLibrary === false` is the flagship showcase's own boot + // shape. So plugin-security's ADR-0090 D11 object posture gate — R1 + // `owd_widening_forbidden` and R2 `owd_external_wider` — ran on NO + // self-hosted deployment at all, while `AUTHORING_RULES` deliberately + // withheld its own `validateSecurityPosture` from the runtime surface + // on the stated grounds that this gate already covered it + // (`packages/lint/src/authoring-rules.ts`, `surfaceReason`). Declared, + // not enforced, on both tables at once. + // + // The direction is #6710's and matters more than the mechanism: the + // DEFAULT is the gated one, so an assembly variant nobody has thought + // of yet gets more enforcement, never less. Only a caller that claims + // to BE the package author is treated as one. + if (this.authoringChannel !== 'package-author') { await this.runAuthoringGate({ type: request.type, name: request.name, diff --git a/packages/rest/package.json b/packages/rest/package.json index 9f075af929..ded8cc481a 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -35,6 +35,7 @@ "@objectstack/metadata": "workspace:*", "@objectstack/metadata-protocol": "workspace:*", "@objectstack/objectql": "workspace:*", + "@objectstack/plugin-security": "workspace:*", "@objectstack/service-analytics": "workspace:*", "@types/node": "^26.1.2", "typescript": "^6.0.3", diff --git a/packages/rest/src/meta-object-owd-gate.test.ts b/packages/rest/src/meta-object-owd-gate.test.ts new file mode 100644 index 0000000000..6fea253623 --- /dev/null +++ b/packages/rest/src/meta-object-owd-gate.test.ts @@ -0,0 +1,425 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7674 — the ADR-0090 D11 object posture gate, driven through the REAL save + * path on the topology it was measured absent from. + * + * ## Why this file exists, and why the unit suite could not stand in for it + * + * `plugin-security/src/object-posture-gate.test.ts` is **18/18 green** and was + * green throughout — it calls `objectPostureGate(ctx)` directly, so it proves + * the verdict function and nothing about whether anything ever calls it. On + * `origin/main` before this change, a grep for `owd_external_wider` across the + * whole repo found exactly two files: the gate source and that unit test. No + * test anywhere reached the gate through `saveMetaItem`, and the #3050 call + * site that dispatches it was wrapped in `if (this.environmentId !== undefined)` + * — a key the CLI's lightweight host-config assembler leaves undefined + * (`serve.ts`'s `config.objects && !hasObjectQL` branch → `new ObjectQLPlugin()` + * with no options; `isHostConfig` → `shouldBootWithLibrary === false` is the + * flagship showcase's own boot shape). So R1 and R2 executed on **no + * self-hosted deployment at all**, and the measured answers to the three PUTs + * below were `200`, `200`, `200`. + * + * A unit-tested gate with no integration coverage through the real save path is + * exactly how that survived, which is why this file is a peer of the one-line + * predicate change rather than a garnish on it. + * + * ## The harness is the host-config topology, deliberately + * + * Nothing here is hand-built: a REAL better-sqlite3 `:memory:` engine, a REAL + * `ObjectStackProtocolImplementation` constructed the way the lightweight + * assembler constructs it (**no environment id, no declared channel** — so the + * constructor default `'environment'` is what arms the gate), the REAL + * `registerObjectPostureGate` wiring plugin-security performs at init, and the + * REAL `PUT /api/v1/meta/:type/:name` route a client calls. `environmentId` + * being undefined is asserted in `boot()` rather than assumed: it is the + * premise of the whole file, and a harness that quietly grew one would turn + * every case below into a test of the pre-#7674 code path. + * + * ## Rejection cases assert the ENVELOPE (ADR-0112) + * + * Every refusal asserts `code` AND `status`. A bare "it failed" assertion would + * be worthless twice over here: this route ANSWERS rather than throws, and the + * unfixed build answers `200`, so a status-only check would at least catch it — + * but a `rejects.toThrow()`-shaped check at the protocol level would not, since + * an object body that reaches persistence on a misconfigured store throws a + * bare driver `Error` whose `code` and `status` are both `undefined`. The pair + * is what separates "refused by the gate" from "failed somewhere downstream". + * + * ## Scope + * + * This is an AUTHORING-validation pin, not an external-principal enforcement + * one. External-principal enforcement is #2696-planned; a wider external + * baseline discloses nothing today. Nothing here should be read as asserting + * that an external principal is refused at read time. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { + ObjectStackProtocolImplementation, + resetEnvWritableMetadataTypes, + type MetadataAuthoringChannel, +} from '@objectstack/metadata-protocol'; +// The REAL overlay store definitions the protocol writes to — not a mirror +// (#5785: a hand-declared `sys_metadata` restarts the drift clock). +import { + SysMetadata, + SysMetadataHistoryObject, + SysMetadataAuditObject, +} from '@objectstack/platform-objects/metadata'; +// The REAL wiring `security-plugin.ts` performs at init — not a re-implementation +// of the verdict. A local copy of the gate would make this suite a test of the +// copy, which is the failure mode the file exists to close. +import { registerObjectPostureGate } from '@objectstack/plugin-security'; +import { RestServer } from './rest-server.js'; + +const META_ITEM = '/api/v1/meta/:type/:name'; + +/** The real backend, constructed the canonical way (`examples/app-crm`). */ +const makeSqliteDriver = () => new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, +}); + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } + delete process.env.OS_METADATA_WRITABLE; + resetEnvWritable(); +}); + +/** + * `OS_METADATA_WRITABLE` is memoised in TWO places, and both must be cleared + * or the suite becomes order-dependent: the protocol's own static + * (`isOverlayAllowed`) and the repository module's + * (`SysMetadataRepository.assertAllowed`). Measured the hard way during reverse + * verification — with the fix in place the posture gate refuses first, so the + * repository memo is never populated and clearing the protocol's alone LOOKS + * sufficient; take the fix out and the repository memo is warmed empty by an + * earlier case, and the escape hatch silently stops working two tests later. + */ +function resetEnvWritable(): void { + ObjectStackProtocolImplementation.resetEnvWritableCache(); + resetEnvWritableMetadataTypes(); +} + +function createMockServer() { + const noop = () => {}; + return { + get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, + listen: async () => {}, close: async () => {}, + }; +} + +function makeRes() { + const res: any = { + _status: 200, + write: () => true, end: () => {}, send: () => res, setHeader: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +/** + * A packaged object, for R1. `_packageId` is what makes + * `registry.getArtifactItem` — and therefore `isArtifactBacked` — answer true; + * its posture is the BASELINE an environment overlay may only tighten. + */ +const PACKAGED_ACCOUNT = { + name: 'qa_packaged_account', + label: 'Packaged Account', + _packageId: 'demo_pkg', + sharingModel: 'public_read', + externalSharingModel: 'private', + fields: { name: { type: 'text' as const, label: 'Name' } }, +}; + +/** + * An overlay body for the packaged object — the same document a Studio author + * would PUT back. Deliberately WITHOUT `_packageId`: that key is the registry's + * provenance envelope, not an authorable one, and sending it would make the + * body fail spec validation before the posture gate ever sees it. + */ +const packagedOverlay = (over: Record = {}) => ({ + name: PACKAGED_ACCOUNT.name, + label: PACKAGED_ACCOUNT.label, + fields: { name: { type: 'text' as const, label: 'Name' } }, + ...over, +}); + +/** The probe body from the issue, parameterized by its OWD pair. */ +const probeObject = (over: Record = {}) => ({ + name: 'qa_probe', + label: 'QA Probe', + fields: { name: { type: 'text' as const, label: 'Name' } }, + ...over, +}); + +/** + * Boot the host-config topology. + * + * @param opts.channel omitted ⇒ the constructor DEFAULT (`'environment'`), + * which is the shape the lightweight assembler produces and the one under + * test. Passing `'package-author'` exercises the #6710 carve-out explicitly. + * @param opts.envWritableObject set `OS_METADATA_WRITABLE=object`. R1's own + * docblock names this as the path it judges — without it, + * `SysMetadataRepository.assertAllowed()` refuses an `object` overlay of a + * PACKAGED item outright (`NOT_OVERRIDABLE`), so an R1-legal overlay could + * never land and "R1 permits tightening" would be unobservable. + */ +async function boot(opts: { channel?: MetadataAuthoringChannel; envWritableObject?: boolean } = {}) { + const { channel } = opts; + if (opts.envWritableObject) { + process.env.OS_METADATA_WRITABLE = 'object'; + } + resetEnvWritable(); + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(makeSqliteDriver(), true); + await engine.init(); + // The metadata-storage platform objects, registered through the SAME seam + // `assembleMetadataProtocol` uses — one `registerApp` manifest under + // `com.objectstack.metadata-objects`, not three hand-rolled + // `registerObject` calls. The audit sink is in the list so a permitted + // write's audit row lands rather than degrading into a logged best-effort + // failure, which would otherwise read like a defect in a suite about + // refusals. + engine.registerApp({ + id: 'com.objectstack.metadata-objects', + name: 'Metadata Platform Objects', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [SysMetadata, SysMetadataHistoryObject, SysMetadataAuditObject], + }); + engine.registry.registerObject(PACKAGED_ACCOUNT as any, 'demo_pkg'); + await engine.syncSchemas(); + + const protocol = channel === undefined + ? new ObjectStackProtocolImplementation(engine as any) + : new ObjectStackProtocolImplementation(engine as any, undefined, undefined, channel); + + // THE PREMISE, asserted rather than assumed. Every case below is about a + // deployment with no environment id; a harness that acquired one would be + // testing the path that always worked. + expect( + (protocol as any).environmentId, + 'the host-config topology is the premise: #7674 re-keys the gate, it does not bind an environment id', + ).toBeUndefined(); + + // Exactly what `security-plugin.ts` does at init. + expect(registerObjectPostureGate(protocol as any)).toBe(true); + + const rest = new RestServer( + createMockServer() as any, + protocol as any, + { api: { requireAuth: false } } as any, + ); + // [#6603] the route demands `manage_metadata` — an authoring capability. + // The caller here HOLDS it: this suite is about the posture gate, and a + // capability refusal would answer 403 for an unrelated reason. + (rest as any).resolveExecCtx = async () => ({ userId: 'u_author', systemPermissions: ['manage_metadata'] }); + rest.registerRoutes(); + + const route = rest.getRoutes().find((r: any) => r.method === 'PUT' && r.path === META_ITEM); + if (!route) throw new Error(`PUT ${META_ITEM} is not registered`); + + const put = async (name: string, body: unknown, query: Record = {}) => { + const res = makeRes(); + await route.handler({ params: { type: 'object', name }, query, headers: {}, body } as any, res); + return res; + }; + + /** + * Overlay rows actually in `sys_metadata` for a name — the persistence half. + * + * The options bag is TYPED, not erased. `ObjectQL.find`'s second parameter + * is already `EngineQueryOptions`, so the literal infers against it and + * `tsc` stays the enforcing channel for these keys — the #4674 lesson the + * `query-options/no-any-erasure` rule exists for (an unknown key here is + * silently DROPPED, never rejected, because the options schemas are not + * `.strict()`). Nothing about this query is off-contract, so it needs no + * `as unknown as EngineQueryOptions` escape either. + */ + const storedRows = async (name: string) => + engine.find('sys_metadata', { where: { type: 'object', name } }); + + return { engine, protocol, put, storedRows }; +} + +// --------------------------------------------------------------------------- +// R2 — ADR-0090 D11: external ≤ internal, on all three doors the issue measured +// --------------------------------------------------------------------------- + +describe('[#7674] R2 `owd_external_wider` through PUT /api/v1/meta/object/:name', () => { + /** + * The issue's three measured 200s, as one table. They are separate doors + * rather than one: `?mode=draft` takes the draft branch of `saveMetaItem`, + * `?package=` binds the row to a software package, and the bare PUT is the + * active path. #4463 D1 records what happens when only one of two doors + * gates — the draft is the first half of a second minting path — so all + * three are pinned, not just the headline one. + */ + it.each([ + { door: 'the active path (bare PUT)', query: {} as Record }, + { door: 'the draft path (`?mode=draft`)', query: { mode: 'draft' } }, + { door: 'package authoring (`?package=demo_pkg`)', query: { package: 'demo_pkg' } }, + ])('refuses 403 owd_external_wider on $door', async ({ query }) => { + const { put, storedRows } = await boot(); + + const res = await put('qa_probe', probeObject({ + sharingModel: 'private', + externalSharingModel: 'public_read', + }), query); + + // ADR-0112 envelope — both halves. `200` is what this answered before. + expect(res._status).toBe(403); + expect(res._json?.code).toBe('owd_external_wider'); + expect(String(res._json?.error)).toContain('externalSharingModel'); + + // The point the status code alone cannot make: the gate is + // PRE-persistence. A 403 answered after the row landed would still be + // the defect, and `GET` would still return the violating pair. + expect(await storedRows('qa_probe')).toEqual([]); + }, 60_000); + + it('catches the unset-internal case too — an absent `sharingModel` is `private` (ADR-0090 D1)', async () => { + // The gate resolves an unset internal to `private` rather than skipping + // the comparison, so the most natural authoring mistake — declaring only + // the external side — is refused rather than waved through. + const { put, storedRows } = await boot(); + + const res = await put('qa_probe', probeObject({ externalSharingModel: 'public_read_write' })); + + expect(res._status).toBe(403); + expect(res._json?.code).toBe('owd_external_wider'); + expect(await storedRows('qa_probe')).toEqual([]); + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// R1 — ADR-0086 D1: an environment overlay may only TIGHTEN a packaged posture +// --------------------------------------------------------------------------- + +describe('[#7674] R1 `owd_widening_forbidden` through PUT /api/v1/meta/object/:name', () => { + /** + * On a host config R1 is the ONLY guard, which is why it belongs here and + * not only in the unit suite. The ADR-0005 two-tier authorization that + * would normally refuse an overlay of a packaged `object` with + * `not_overridable` is ITSELF scoped to `environmentId !== undefined`, so on + * this topology the write sails past it and arrives at the posture gate + * with nothing else in front of it. + */ + it('refuses an env overlay that widens a packaged object\'s internal OWD', async () => { + // Declared baseline: `public_read`. The overlay asks for + // `public_read_write`, and leaves the external side unset so R2 has + // nothing to compare — only R1 can produce this refusal. + const { put, storedRows } = await boot(); + + const res = await put('qa_packaged_account', packagedOverlay({ + sharingModel: 'public_read_write', + })); + + expect(res._status).toBe(403); + expect(res._json?.code).toBe('owd_widening_forbidden'); + expect(String(res._json?.error)).toContain('TIGHTEN'); + expect(await storedRows('qa_packaged_account')).toEqual([]); + }, 60_000); + + it('refuses a widened EXTERNAL side against the packaged baseline', async () => { + // Distinct from R2: `public_read` external against `public_read` + // internal is NOT external-wider, so R2 passes the body. Only the + // comparison against the PACKAGED declaration (external `private`) can + // refuse it. A suite that only ever sent an external-wider pair could + // not tell the two rules apart. + const { put, storedRows } = await boot(); + + const res = await put('qa_packaged_account', packagedOverlay({ + sharingModel: 'public_read', + externalSharingModel: 'public_read', + })); + + expect(res._status).toBe(403); + expect(res._json?.code).toBe('owd_widening_forbidden'); + expect(await storedRows('qa_packaged_account')).toEqual([]); + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// The negative direction — load-bearing, not decoration +// --------------------------------------------------------------------------- + +describe('[#7674] what the gate must still let through', () => { + /** + * Arming a gate that had never run is exactly the change that overshoots + * into refusing legitimate authoring, and a suite of nothing but refusals + * would be green for a gate that rejects everything. These cases are the + * other half of the claim. + */ + it.each([ + { pair: 'private / private', over: { sharingModel: 'private', externalSharingModel: 'private' } }, + { pair: 'public_read / private (external TIGHTER)', over: { sharingModel: 'public_read', externalSharingModel: 'private' } }, + { pair: 'public_read / public_read (equal, not wider)', over: { sharingModel: 'public_read', externalSharingModel: 'public_read' } }, + { pair: 'no OWD keys at all', over: {} }, + ])('a legal pair still saves — $pair', async ({ over }) => { + const { put, storedRows } = await boot(); + + const res = await put('qa_probe', probeObject(over)); + + expect(res._status, `unexpected refusal: ${JSON.stringify(res._json)}`).toBe(200); + expect(res._json?.success).toBe(true); + expect(await storedRows('qa_probe')).toHaveLength(1); + }, 60_000); + + it('an env overlay that TIGHTENS a packaged object is allowed (R1 is directional)', async () => { + // `OS_METADATA_WRITABLE=object` is the escape hatch R1's own docblock + // names as the path it judges. Without it the overlay is refused a + // layer later by `SysMetadataRepository.assertAllowed()` + // (`NOT_OVERRIDABLE`) — a DIFFERENT door, measured on this harness — + // and this case would then pass for a reason that has nothing to do + // with the posture gate. + const { put, storedRows } = await boot({ envWritableObject: true }); + + // The packaged baseline is `public_read` / `private`; the overlay + // narrows the internal side to `private`. ADR-0086 D1 permits exactly + // this direction, and R1 refusing it would be the overshoot. + const res = await put('qa_packaged_account', packagedOverlay({ + sharingModel: 'private', + externalSharingModel: 'private', + })); + + expect(res._status, `unexpected refusal: ${JSON.stringify(res._json)}`).toBe(200); + expect(await storedRows('qa_packaged_account')).toHaveLength(1); + }, 60_000); + + /** + * [#6710] The declared carve-out, preserved. A kernel that claims to BE the + * package author is treated as one by this door too — package authoring is + * gated at build time instead (`validateSecurityPosture` is `CLI_ONLY` in + * `AUTHORING_RULES`, and R1's own message prescribes that route: "widen it + * in the package source and publish through the package pipeline"). + * + * This case is the guard against the worse defect available here: a gate + * that starts refusing package authoring is a regression, not a fix. It is + * green both before and after #7674 — deliberately, because what it pins is + * the thing that must NOT have moved. + */ + it('the `package-author` channel still bypasses the gate — the #6710 carve-out is intact', async () => { + const { put, storedRows } = await boot({ channel: 'package-author' }); + + const res = await put('qa_probe', probeObject({ + sharingModel: 'private', + externalSharingModel: 'public_read', + })); + + expect(res._status, `the control plane must still install its own packages: ${JSON.stringify(res._json)}`).toBe(200); + expect(await storedRows('qa_probe')).toHaveLength(1); + }, 60_000); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 23a65aaa1b..8017ab9786 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1953,6 +1953,9 @@ importers: '@objectstack/objectql': specifier: workspace:* version: link:../objectql + '@objectstack/plugin-security': + specifier: workspace:* + version: link:../plugins/plugin-security '@objectstack/service-analytics': specifier: workspace:* version: link:../services/service-analytics