diff --git a/.changeset/protocol-2926-unknown-dollar-params.md b/.changeset/protocol-2926-unknown-dollar-params.md new file mode 100644 index 0000000000..4f21ae067c --- /dev/null +++ b/.changeset/protocol-2926-unknown-dollar-params.md @@ -0,0 +1,5 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): findData now rejects unknown `$`-prefixed query parameters with 400 `UNSUPPORTED_QUERY_PARAM` instead of silently treating them as implicit field-equality filters that match zero rows (#2926 ⑩). A `$`-prefixed key can never be a field name, so this is loud-failure only for the unsupported-alias class; bare-key implicit equality filtering is unchanged. The error message lists the supported aliases ($top, $skip, $orderby, $select, $count, $search, $searchFields, $filter, $expand). diff --git a/.changeset/rest-2926-status-passthrough.md b/.changeset/rest-2926-status-passthrough.md new file mode 100644 index 0000000000..9b396f527b --- /dev/null +++ b/.changeset/rest-2926-status-passthrough.md @@ -0,0 +1,5 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest): mapDataError now honors an explicit 4xx `error.status`/`error.code` carried by domain errors (#2926 ⑦). Record-scope authorization denials from plugin-sharing (status 403, code FORBIDDEN) previously degraded to a bare 400 with no machine-readable code because the generic data routes bypass sendError's status passthrough. Structured 409 envelopes (CONCURRENT_UPDATE, DELETE_RESTRICTED) keep their dedicated branches; 5xx statuses still go through the message-sanitizing heuristics. diff --git a/.changeset/security-2909-capability-scope.md b/.changeset/security-2909-capability-scope.md new file mode 100644 index 0000000000..2ff3842b3f --- /dev/null +++ b/.changeset/security-2909-capability-scope.md @@ -0,0 +1,5 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): stop clobbering admin-edited capability `scope` on boot (#2909 T3). `scope` is an admin-editable classification select on sys_capability, but the curated seeder refreshed it on every boot alongside label/description — silently reverting admin reclassifications. It is now seed-once: written on insert, never refreshed (a curated scope change in a new platform version requires a data migration; recorded in the ADR-0094 addendum). diff --git a/.changeset/security-2926-position-gate-vocab.md b/.changeset/security-2926-position-gate-vocab.md new file mode 100644 index 0000000000..c929138652 --- /dev/null +++ b/.changeset/security-2926-position-gate-vocab.md @@ -0,0 +1,5 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): re-arm the sys_position system-row write gate after the A4 managed_by rename (#2926 ①). The gate's provenance map still keyed on the legacy `system`/`config` values while rows are now stamped (and boot-normalized to) `platform`/`package`, so platform/package-managed positions — including the `everyone`/`guest` audience anchors — could be physically deleted through the data API once their bindings were removed. The map now guards both the canonical and legacy vocabularies, and the misleading "no runtime path branches on legacy values" safety notes were corrected. diff --git a/.changeset/sharing-2909-seed-not-clobber.md b/.changeset/sharing-2909-seed-not-clobber.md new file mode 100644 index 0000000000..2aa48edfca --- /dev/null +++ b/.changeset/sharing-2909-seed-not-clobber.md @@ -0,0 +1,6 @@ +--- +'@objectstack/plugin-sharing': minor +'@objectstack/spec': patch +--- + +feat(plugin-sharing): sys_sharing_rule provenance + seed-not-clobber (#2909 P0/T1). The object gains readonly `managed_by` (unified A4 tri-state platform/package/admin) and `customized` columns; declared rules seed with `managed_by: 'package'`. defineRule in seed mode adopts pristine/legacy rows (package upgrades stay deliverable) but never overwrites admin-authored or customized rows — an admin's `active: false` on an over-sharing rule now survives redeploys instead of being resurrected at boot. A beforeUpdate hook stamps `customized` on any non-system edit of a seeded rule. Deliberately NO write gate: sharing rules remain a first-class admin authoring surface (ADR-0094 addendum tradeoff). diff --git a/.changeset/sharing-2926-boot-backfill.md b/.changeset/sharing-2926-boot-backfill.md new file mode 100644 index 0000000000..90e8c6beb9 --- /dev/null +++ b/.changeset/sharing-2926-boot-backfill.md @@ -0,0 +1,5 @@ +--- +'@objectstack/plugin-sharing': patch +--- + +fix(plugin-sharing): reconcile every active sharing rule once at boot (#2926 ③). Rule grants are materialized by write hooks, which deliberately skip `isSystem` writes — so seed-loader records never produced `sys_record_share` rows and demo data shipping with matching sharing rules was broken out of the box until each record was touched at runtime. The boot backfill runs on `kernel:listening` — the phase the kernel fires only after every `kernel:ready` handler has settled, including the AppPlugin seed loader — so the reconcile sees the seeded rows rather than racing them. It is idempotent (diff-based reconcile) and best-effort per rule so one broken rule cannot block startup. diff --git a/.changeset/spec-2926-isdefault-dualtrack.md b/.changeset/spec-2926-isdefault-dualtrack.md new file mode 100644 index 0000000000..6c1dc01419 --- /dev/null +++ b/.changeset/spec-2926-isdefault-dualtrack.md @@ -0,0 +1,5 @@ +--- +'@objectstack/spec': patch +--- + +docs(spec): rewrite the `isDefault` permission-set docs to describe the actual dual-track behavior (#2926 ②): app-level `isDefault` sets are resolved as the SecurityPlugin's fallback and idempotently auto-bound to the `everyone` anchor at boot (guarded by the high-privilege-bits check), while package-level sets are never auto-bound and instead materialize a `sys_audience_binding_suggestion` an admin confirms. The previous "never auto-bound" wording contradicted the shipped app-level track. diff --git a/docs/adr/0094-sys-permission-set-pure-projection.md b/docs/adr/0094-sys-permission-set-pure-projection.md index e9069c4751..d366f5b220 100644 --- a/docs/adr/0094-sys-permission-set-pure-projection.md +++ b/docs/adr/0094-sys-permission-set-pure-projection.md @@ -301,14 +301,38 @@ only a seed-not-clobber discipline. | Type | Enforcement reads | Class | Decision | | :-- | :-- | :-- | :-- | | `sys_permission_set` | metadata (`PermissionEvaluator.resolvePermissionSets` → `metadata.list('permission')`, DB row only as fallback) | metadata-authoritative | **Record is a projection — done** (this ADR). | -| `sys_sharing_rule` | the record, live (`sharing-plugin.ts` "rule evaluation reads `sys_sharing_rule` live"; `sharing-rule-service` `engine.find`) | **record-authoritative** | Declared rules are a **boot seed**; the record is the authority. Do **not** project. Audit that `bootstrapDeclaredSharingRules` preserves env-edited rows (seed-not-clobber) and that the metadata overlay is not read as a live override. | -| `sys_position` | mixed — position→permission-set resolution is metadata-first for the *sets*, but the `sys_position` **record** (incl. its `permissions` field, bindings, `delegatable`, `admin` gating) is read live by the anchor gate and `DelegatedAdminGate` | **needs the seed-vs-authority audit** against the criterion above; likely record-authoritative for bindings with metadata seeding identity | Classify precisely, then either project (if the position *definition* is enforced from metadata) or make declared positions seed-only. | -| `sys_capability` | the record (curated registry read for capability existence) | record-authoritative (registry) | Seed-only; low authoring surface. Audit seed-not-clobber. | +| `sys_sharing_rule` | the record, live (`sharing-plugin.ts` "rule evaluation reads `sys_sharing_rule` live"; `sharing-rule-service` `engine.find`) | **record-authoritative** | **Resolved (#2909 P0/T1)**: declared rules are a **boot seed**; the record is the authority; not projected. Rows carry readonly `managed_by` (unified A4 tri-state) + `customized` provenance; the seeder (`defineRule` in seed mode, `managedBy:'package'`) adopts pristine/legacy rows and keeps updating them, but **never overwrites admin-authored or `customized` rows** — an admin's `active:false` on an over-sharing rule survives redeploys. A `beforeUpdate` hook stamps `customized` on any non-system edit of a seeded row. | +| `sys_position` | mixed — position→permission-set resolution is metadata-first for the *sets*, but the `sys_position` **record** (incl. its `permissions` field, bindings, `delegatable`, `admin` gating) is read live by the anchor gate and `DelegatedAdminGate` | **record-authoritative** (bindings and lifecycle), with metadata seeding **identity + display only** | **Resolved (#2909 T2)**: declared positions are seed-only — `bootstrapDeclaredPositions` refreshes `label`/`description` and nothing else; bindings/`active`/`is_default`/`delegatable`/`managed_by` belong to the runtime/admin. Locked by `bootstrap-declared-positions.test.ts`. | +| `sys_capability` | the record (curated registry read for capability existence) | record-authoritative (registry) | **Resolved (#2909 T3)**: seed-not-clobber holds — `label`/`description` are platform-owned and refresh each boot; `managed_by`/`active` were already preserved; `scope` is an admin-editable classification face and is now **seed-once** (insert only). Locked by `bootstrap-system-capabilities.test.ts`. | Only `sys_permission_set` was both metadata-authoritative **and** carried a harmful, actively-drifting split-brain, which is why it was fixed first and in -full. The others are recorded here with their class so the follow-up work is -scoped, not rediscovered — tracked in framework#2909. +full. The follow-up work above landed via framework#2909. + +#### Recorded tradeoffs (#2909) + +- **Why `sys_sharing_rule` chose seed-not-clobber over a write gate + (projection-lite).** A self-consistent alternative existed: gate + package-managed rows against admin writes (as #2918 does for + `sys_position`/`sys_capability`), making the boot overwrite harmless. It was + rejected because sharing rules are a first-class admin authoring *and + tuning* surface in Setup — including narrowing or deactivating a seeded + rule that turned out to over-share. Locking package rows would amputate + that product capability, and the classification criterion (enforcement + reads the record) already points at record authority. Admin edits are + therefore *remembered* (`customized`), not blocked. Only the two provenance + columns themselves are `readonly` (engine-stripped from non-system + payloads), so provenance cannot be forged or cleared through the data door. +- **Known boundaries of the `customized` stamp**: multi-row updates (no + single `input.id`) are not stamped — every rule-editing UI path updates by + id; an admin who *deletes* a package rule gets it back pristine on the next + boot (delete ≠ customize; deactivation is the supported way to retire a + seeded rule); a "reset to package default" affordance (system-context clear + of `customized`) is future work. +- **`sys_capability.scope` is seed-once**: a curated scope change shipped in + a new platform version no longer propagates to existing rows and needs a + data migration. Accepted — silently reverting an admin's reclassification + every boot was the worse failure mode. ### Why an addendum, not a new ADR diff --git a/examples/app-showcase/objectstack.config.ts b/examples/app-showcase/objectstack.config.ts index fb12a06c8b..59aa340089 100644 --- a/examples/app-showcase/objectstack.config.ts +++ b/examples/app-showcase/objectstack.config.ts @@ -16,6 +16,7 @@ import { ShowcaseExternalDatasource } from './src/system/datasources/showcase-ex import { ExternalCustomer, ExternalOrder } from './src/data/objects/external/index.js'; import { setupShowcaseExternalDatasource } from './src/system/datasources/external-fixture.js'; import { registerRecalcEndpoint } from './src/system/server/recalc-endpoint.js'; +import { registerShowcasePositionBindings } from './src/security/bind-position-sets.js'; import { TaskViews, ProjectViews, InquiryViews, BusinessUnitViews } from './src/ui/views/index.js'; import { ShowcaseApp } from './src/ui/apps/index.js'; import { ChartGalleryDashboard, OpsDashboard } from './src/ui/dashboards/index.js'; @@ -218,4 +219,7 @@ export const onEnable = async (ctx: unknown): Promise => { await setupShowcaseExternalDatasource(ctx as Parameters[0]); // Mount the custom REST endpoint behind the `showcase_recalc_estimate` api action. registerRecalcEndpoint(ctx as Parameters[0]); + // [#2926 ②] Ensure the persona position↔permission-set bindings exist after + // the security bootstraps (cannot be a seed — see bind-position-sets.ts). + registerShowcasePositionBindings(ctx as Parameters[0]); }; diff --git a/examples/app-showcase/src/data/seed/index.ts b/examples/app-showcase/src/data/seed/index.ts index 278d97a1a7..0d41c42a21 100644 --- a/examples/app-showcase/src/data/seed/index.ts +++ b/examples/app-showcase/src/data/seed/index.ts @@ -13,6 +13,7 @@ import { Product, Invoice, InvoiceLine } from '../objects/invoice.object.js'; import { Contact } from '../objects/contact.object.js'; import { Inquiry } from '../objects/inquiry.object.js'; import { FieldZoo } from '../objects/field-zoo.object.js'; +import { Announcement } from '../objects/announcement.object.js'; /** * Seed data sized to "feed every view": every Kanban column is populated, @@ -161,6 +162,13 @@ const orgUnits = SeedSchema.parse({ ], }); +// [#2926 ②] Position ↔ permission-set bindings are NOT seeded here: the seed +// loader runs before the security bootstrap creates the sys_position / +// sys_permission_set rows, so the required name references cannot resolve. +// They are ensured imperatively on kernel:listening instead (after every +// kernel:ready handler, incl. the security bootstrap, has settled) — see +// `src/security/bind-position-sets.ts` (wired via `onEnable`). + const teams = defineSeed(Team, { mode: 'upsert', externalId: 'name', @@ -286,4 +294,20 @@ const preferences = defineSeed(Preference, { ], }); -export const ShowcaseSeedData = [accounts, contacts, inquiries, products, projects, tasks, categories, businessUnits, orgUnits, teams, memberships, fieldZoo, invoices, invoiceLines, preferences]; +/** + * [#2926 ⑤] Announcements — the read-visibility demo object finally ships + * with data, so its assertions stop dry-running on a fresh DB. The object has + * NO `name` field (display name derives from `title`); `owner_id` stays + * unset — users can't be seeded, and creation rights are deliberately narrow + * (only `showcase_ops` may create; everyone else is read-only by design). + */ +const announcements = defineSeed(Announcement, { + mode: 'upsert', + externalId: 'title', + records: [ + { title: 'Welcome to the Showcase workspace', body: 'This demo org exercises the full permission model: positions, permission sets, sharing rules and field-level security. Log in as different personas to compare what each can see and edit.' }, + { title: 'Q3 field-ops rollout', body: 'Field Operations onboards the new inquiry intake flow this quarter. New public inquiries are shared automatically with the Field Ops subtree.' }, + ], +}); + +export const ShowcaseSeedData = [accounts, contacts, inquiries, products, projects, tasks, categories, businessUnits, orgUnits, teams, memberships, fieldZoo, invoices, invoiceLines, preferences, announcements]; diff --git a/examples/app-showcase/src/security/bind-position-sets.ts b/examples/app-showcase/src/security/bind-position-sets.ts new file mode 100644 index 0000000000..a41741108a --- /dev/null +++ b/examples/app-showcase/src/security/bind-position-sets.ts @@ -0,0 +1,118 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#2926 ②] Position ↔ permission-set bindings for the showcase personas. + * + * The permission model is record-authoritative (ADR-0090/0094): bindings live + * only as `sys_position_permission_set` rows. A fresh deploy used to boot with + * ZERO bindings — every persona silently degraded to the `everyone` baseline + * until an admin hand-assigned all sets. + * + * This cannot be a declarative SEED: the seed loader runs before the security + * bootstrap creates the `sys_position` / `sys_permission_set` rows, so the name + * references cannot resolve and the required lookups fail validation. So we play + * the admin's part imperatively — inserting each missing binding idempotently + * (dedup by position+set pair, stable ids). + * + * Timing matters. `kernel:ready` handlers run SEQUENTIALLY, each awaited, in + * registration order (`kernel.ts` `trigger`). The showcase AppPlugin starts + * BEFORE the Security plugin, so an app hook on `kernel:ready` runs *before* + * the security bootstrap has created the position/set rows — the rows never + * appear from inside that hook. We therefore bind on **`kernel:listening`**, + * the phase the kernel fires only AFTER every `kernel:ready` handler has + * completed (`kernel.ts` Phase 4 / `lite-kernel.ts`), so the bootstrap rows are + * guaranteed present. + * + * `everyone → showcase_member_default` IS bound here. The security plugin only + * auto-binds an app's `isDefault` set to `everyone` when that set is + * application-owned; the showcase ships as a package, so its default lands in + * `sys_audience_binding_suggestion` (pending admin confirmation) and is NOT + * live until confirmed. Binding it here keeps the demo's baseline working out + * of the box, idempotently and alongside the suggestion. + */ + +const BINDINGS: ReadonlyArray = [ + ['everyone', 'showcase_member_default'], + ['contributor', 'showcase_contributor'], + ['manager', 'showcase_manager'], + ['exec', 'showcase_executive'], + ['auditor', 'showcase_auditor'], + ['ops', 'showcase_ops'], + ['field_ops_delegate', 'showcase_field_ops_delegate'], + ['client_portal_user', 'showcase_guest_portal'], +]; + +const SYS = { isSystem: true } as const; + +interface BindHostContext { + ql: { + find: (object: string, query: unknown, options?: unknown) => Promise; + insert: (object: string, data: Record, options?: unknown) => Promise; + }; + logger?: { info?: (...a: unknown[]) => void; warn?: (...a: unknown[]) => void }; + hook?: (event: string, handler: () => Promise | void) => void; +} + +/** Find one row by `name`, passing the system context the way the engine's own + * read path expects it (merged from `query.context`; see objectql `find`). */ +async function findOneByName(ctx: BindHostContext, object: string, name: string): Promise<{ id?: string } | undefined> { + try { + const rows = (await ctx.ql.find(object, { where: { name }, limit: 1, context: SYS })) as + | Array<{ id?: string }> + | { records?: Array<{ id?: string }> }; + if (Array.isArray(rows)) return rows[0]; + return rows?.records?.[0]; + } catch (err) { + ctx.logger?.warn?.('[showcase] position binding lookup failed', { + object, + name, + error: err instanceof Error ? err.message : String(err), + }); + return undefined; + } +} + +export function registerShowcasePositionBindings(ctx: BindHostContext): void { + const run = async (): Promise => { + let created = 0; + for (const [positionName, setName] of BINDINGS) { + const position = await findOneByName(ctx, 'sys_position', positionName); + const set = await findOneByName(ctx, 'sys_permission_set', setName); + if (!position?.id || !set?.id) { + ctx.logger?.warn?.('[showcase] position binding skipped (row missing)', { position: positionName, set: setName }); + continue; + } + const existing = (await ctx.ql.find( + 'sys_position_permission_set', + { where: { position_id: position.id, permission_set_id: set.id }, limit: 1, context: SYS }, + )) as unknown; + const hit = Array.isArray(existing) ? existing[0] : (existing as { records?: unknown[] })?.records?.[0]; + if (hit) continue; + try { + await ctx.ql.insert( + 'sys_position_permission_set', + { id: `ppsb_showcase_${positionName}`, position_id: position.id, permission_set_id: set.id }, + { context: SYS }, + ); + created += 1; + } catch (err) { + ctx.logger?.warn?.('[showcase] position binding insert failed', { + position: positionName, + set: setName, + error: err instanceof Error ? err.message : String(err), + }); + } + } + ctx.logger?.info?.('[showcase] position bindings ensured', { created, total: BINDINGS.length }); + }; + + // Bind on `kernel:listening` — the phase that fires only after every + // `kernel:ready` handler (incl. the security bootstrap that seeds the + // position/set rows) has completed. Fall back to a deferred immediate run + // if the host context somehow omits the hook registrar. + if (typeof ctx.hook === 'function') { + ctx.hook('kernel:listening', run); + } else { + setTimeout(() => void run(), 0); + } +} diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 1043db6527..8a82562d40 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -2404,7 +2404,26 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { if (options[key] === 'true') options[key] = true; else if (options[key] === 'false') options[key] = false; } - + + // [#2926 ⑩] Every supported OData-style `$` alias has been consumed and + // deleted above ($top/$skip/$orderby/$select/$count/$search/ + // $searchFields/$filter/$expand). A `$`-prefixed key can never be a + // field name, so anything left is an unsupported query parameter — + // fail loudly instead of letting it fall into the implicit-filter + // bucket below, where it silently matched zero rows (or, before the + // $filter alias existed, was dropped entirely and returned the + // UNFILTERED page — a footgun for scripts resolving ids by name). + const unsupportedDollarParams = Object.keys(options).filter((k) => k.startsWith('$')); + if (unsupportedDollarParams.length > 0) { + const err: any = new Error( + `Unsupported query parameter(s): ${unsupportedDollarParams.join(', ')}. ` + + 'Supported $-prefixed parameters: $top, $skip, $orderby, $select, $count, $search, $searchFields, $filter, $expand.', + ); + err.status = 400; + err.code = 'UNSUPPORTED_QUERY_PARAM'; + throw err; + } + // Flat field filters: REST-style query params like ?id=abc&status=open // After extracting all known query parameters, any remaining keys are // treated as implicit field-level equality filters merged into `where`. diff --git a/packages/objectql/src/protocol-data.test.ts b/packages/objectql/src/protocol-data.test.ts index c25e61e2d5..2df6b194b9 100644 --- a/packages/objectql/src/protocol-data.test.ts +++ b/packages/objectql/src/protocol-data.test.ts @@ -38,6 +38,40 @@ describe('ObjectStackProtocolImplementation - Data Operations', () => { expect(opts.where?.searchFields).toBeUndefined(); }); + // [#2926 ⑩] Unknown `$`-prefixed params must fail loudly instead of + // silently matching zero rows via the implicit-filter bucket (or — + // pre-$filter alias — being dropped and returning the unfiltered page). + it('rejects an unknown $-prefixed query param with 400 UNSUPPORTED_QUERY_PARAM', async () => { + await expect( + protocol.findData({ object: 'task', query: { $foo: '1' } }), + ).rejects.toMatchObject({ status: 400, code: 'UNSUPPORTED_QUERY_PARAM' }); + expect(mockEngine.find).not.toHaveBeenCalled(); + }); + + it('names the offending params and the supported list in the rejection message', async () => { + await expect( + protocol.findData({ object: 'task', query: { $inlinecount: 'allpages', $format: 'json' } }), + ).rejects.toThrow(/\$inlinecount.*\$format|\$format.*\$inlinecount/s); + }); + + it('still accepts every supported $ alias after the unknown-$ guard', async () => { + await protocol.findData({ + object: 'task', + query: { $top: 5, $skip: 2, $orderby: 'name', $select: 'id,name', $search: 'x', $filter: { status: 'open' } }, + }); + expect(mockEngine.find).toHaveBeenCalledTimes(1); + const opts = mockEngine.find.mock.calls[0][1]; + expect(opts.limit).toBe(5); + expect(opts.offset).toBe(2); + expect(opts.where).toEqual({ status: 'open' }); + }); + + it('keeps bare unknown params as implicit field-equality filters (unchanged behavior)', async () => { + await protocol.findData({ object: 'task', query: { status: 'open' } }); + const opts = mockEngine.find.mock.calls[0][1]; + expect(opts.where).toEqual({ status: 'open' }); + }); + it('should normalize $expand (OData) string to expand Record', async () => { await protocol.findData({ object: 'order_item', query: { $expand: 'order,product' } }); diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-positions.test.ts b/packages/plugins/plugin-security/src/bootstrap-declared-positions.test.ts new file mode 100644 index 0000000000..2af6dca522 --- /dev/null +++ b/packages/plugins/plugin-security/src/bootstrap-declared-positions.test.ts @@ -0,0 +1,94 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#2909 T2] Lock the seed semantics of bootstrapDeclaredPositions. + * + * sys_position is RECORD-AUTHORITATIVE (ADR-0094 addendum): the declared + * `positions: []` metadata seeds row IDENTITY + display fields only. The + * record side — permission-set bindings, `active`, `is_default`, + * `delegatable`, `managed_by` provenance — belongs to the runtime/admin and + * must never be touched by a re-seed. These tests exist to keep that + * contract from regressing silently (the behavior predates them but was + * never locked). + */ + +import { describe, it, expect } from 'vitest'; +import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; + +/** Minimal in-memory ql for sys_position seeding. */ +function makeQl(declared: any[] = []) { + const rows: any[] = []; + return { + rows, + _registry: { listItems: (type: string) => (type === 'position' ? declared.map((c) => ({ content: c })) : []) }, + async find(object: string, q: any) { + if (object !== 'sys_position') return []; + const where = q?.where ?? {}; + return rows.filter((r) => Object.entries(where).every(([k, v]) => r[k] === v)); + }, + async insert(object: string, data: any) { + if (object !== 'sys_position') return null; + rows.push({ ...data }); + return { id: data.id }; + }, + async update(object: string, data: any) { + if (object !== 'sys_position') return; + const r = rows.find((x) => x.id === data.id); + if (r) Object.assign(r, data); + }, + }; +} + +describe('bootstrapDeclaredPositions (#2909 T2 — seed-only semantics locked)', () => { + it('inserts new declared positions with identity + display fields only', async () => { + const ql = makeQl([{ name: 'contributor', label: 'Contributor', description: 'Does work' }]); + const r = await bootstrapDeclaredPositions(ql, null); + expect(r.seeded).toBe(1); + const row = ql.rows[0]; + expect(row).toMatchObject({ name: 'contributor', label: 'Contributor', active: true, is_default: false }); + // Provenance is NOT stamped by the declared seeder (bootstrapBuiltinRoles + // owns the built-in anchors; declared positions carry the object default). + expect(row.managed_by).toBeUndefined(); + }); + + it('refreshes ONLY label/description on existing rows', async () => { + const ql = makeQl([{ name: 'contributor', label: 'Contributor v2', description: 'new text' }]); + ql.rows.push({ + id: 'pos_1', name: 'contributor', label: 'Contributor', description: 'old', + active: true, is_default: false, + }); + const r = await bootstrapDeclaredPositions(ql, null); + expect(r.updated).toBe(1); + expect(ql.rows[0].label).toBe('Contributor v2'); + expect(ql.rows[0].description).toBe('new text'); + }); + + it('NEVER touches authoritative record fields (active/is_default/delegatable/managed_by)', async () => { + const ql = makeQl([{ name: 'contributor', label: 'Contributor v2' }]); + // Admin turned the position off, made it default, marked it delegatable, + // and the row carries provenance — a re-seed must not reset any of it. + ql.rows.push({ + id: 'pos_1', name: 'contributor', label: 'Contributor', description: 'old', + active: false, is_default: true, delegatable: true, managed_by: 'package', + permissions: ['something_admin_set'], + }); + await bootstrapDeclaredPositions(ql, null); + const row = ql.rows[0]; + expect(row.active).toBe(false); + expect(row.is_default).toBe(true); + expect(row.delegatable).toBe(true); + expect(row.managed_by).toBe('package'); + expect(row.permissions).toEqual(['something_admin_set']); + // …while the display fields did refresh. + expect(row.label).toBe('Contributor v2'); + }); + + it('is idempotent — a re-run inserts nothing new', async () => { + const ql = makeQl([{ name: 'a', label: 'A' }, { name: 'b', label: 'B' }]); + const r1 = await bootstrapDeclaredPositions(ql, null); + expect(r1.seeded).toBe(2); + const r2 = await bootstrapDeclaredPositions(ql, null); + expect(r2.seeded).toBe(0); + expect(ql.rows).toHaveLength(2); + }); +}); diff --git a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts index a1e8c1e43c..3d701a7445 100644 --- a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.test.ts @@ -65,6 +65,29 @@ describe('bootstrapSystemCapabilities (ADR-0066 D1 back-compat seed)', () => { expect(ql.rows.find((x: any) => x.name === 'approve_invoice')).toBeDefined(); }); + // [#2909 T3] `scope` is an admin-editable classification face — seed-once. + it('does NOT clobber an admin-edited scope on re-seed (label/description still refresh)', async () => { + const ql = makeQl(); + await bootstrapSystemCapabilities(ql, []); + const cap = KNOWN_CAPABILITIES[0]; + const row = ql.rows.find((x) => x.name === cap.name)!; + // Admin reclassifies the capability and tweaks nothing else. + row.scope = row.scope === 'org' ? 'platform' : 'org'; + const adminScope = row.scope; + row.label = 'stale label'; + await bootstrapSystemCapabilities(ql, []); + expect(row.scope).toBe(adminScope); // admin's edit survives the boot + expect(row.label).toBe(cap.label); // platform display fields refreshed + }); + + it('still writes scope on first insert', async () => { + const ql = makeQl(); + await bootstrapSystemCapabilities(ql, []); + for (const cap of KNOWN_CAPABILITIES) { + expect(ql.rows.find((x) => x.name === cap.name)?.scope).toBe(cap.scope); + } + }); + it('marks manage_org_users as org-scoped and the rest platform', () => { const org = KNOWN_CAPABILITIES.find((c) => c.name === 'manage_org_users'); expect(org?.scope).toBe('org'); diff --git a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts index f584f390e2..56c375aac6 100644 --- a/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts +++ b/packages/plugins/plugin-security/src/bootstrap-system-capabilities.ts @@ -100,9 +100,13 @@ export async function bootstrapSystemCapabilities( for (const def of byName.values()) { const existing = await tryFind(ql, 'sys_capability', { name: def.name }, 1); if (existing[0]?.id) { - // Keep label/description/scope fresh, but do NOT clobber admin edits to - // managed_by/active — only platform-owned fields are reconciled. - if (await tryUpdate(ql, 'sys_capability', { id: existing[0].id, label: def.label, description: def.description, scope: def.scope })) { + // Keep label/description fresh, but do NOT clobber admin edits — only + // platform-owned display fields are reconciled. `scope` is an + // admin-editable classification face (plain select on sys_capability), + // so it is seed-once: written on insert, never refreshed (#2909 T3). + // A curated scope change in a new platform version needs a data + // migration — recorded in the ADR-0094 addendum. + if (await tryUpdate(ql, 'sys_capability', { id: existing[0].id, label: def.label, description: def.description })) { updated += 1; } } else { diff --git a/packages/plugins/plugin-security/src/normalize-managed-by.ts b/packages/plugins/plugin-security/src/normalize-managed-by.ts index e4c696f5f8..0a0f5b96cf 100644 --- a/packages/plugins/plugin-security/src/normalize-managed-by.ts +++ b/packages/plugins/plugin-security/src/normalize-managed-by.ts @@ -17,10 +17,12 @@ * `'config'` / `'user'`. Built-in position rows and declared package sets * self-heal on their own bootstrap upsert, so this only mops up the rest. * - * Safe by construction: NO runtime path branches on the legacy values (every - * read keys on `'package'` or `'platform'`, both unchanged by the rename), so - * this is a pure display-vocabulary migration — it never changes an access - * decision. Idempotent: canonical rows are skipped, so a re-run is a no-op. + * NOT purely cosmetic: the system-row write gate's provenance map + * (SYSTEM_ROW_PROVENANCE in security-plugin.ts) branches on `managed_by` + * values, so it must recognize BOTH the canonical and the legacy vocabulary — + * renaming a stored value without updating that map silently disarms the gate + * (#2926 ①). Keep the two in lockstep whenever this vocabulary changes. + * Idempotent: canonical rows are skipped, so a re-run is a no-op. * Best-effort and non-fatal, like the sibling boot reconcilers. * * Runs on `kernel:ready` after the seeders, as `isSystem` (the field is diff --git a/packages/plugins/plugin-security/src/objects/sys-position.object.ts b/packages/plugins/plugin-security/src/objects/sys-position.object.ts index 1aacee626b..db435dc7ce 100644 --- a/packages/plugins/plugin-security/src/objects/sys-position.object.ts +++ b/packages/plugins/plugin-security/src/objects/sys-position.object.ts @@ -224,9 +224,10 @@ export const SysPosition = ObjectSchema.create({ // built-in identity position (seeded by bootstrapBuiltinRoles, read-only); // `package` = stack/package-declared; `admin` = tenant-created in Setup. // Back-compat: legacy rows may carry system (== platform) / config (== package) - // / user (== admin); no runtime path branches on those (every read keys on - // 'package' / 'platform'), and the boot normalizer heals them to the canonical - // vocab. Built-in (`platform`) rows self-heal on the next bootstrap upsert. + // / user (== admin); the boot normalizer heals them to the canonical vocab, and + // the system-row write gate (SYSTEM_ROW_PROVENANCE, security-plugin.ts) guards + // both vocabularies — keep it in lockstep with any change here (#2926 ①). + // Built-in (`platform`) rows self-heal on the next bootstrap upsert. managed_by: Field.select({ label: 'Managed By', readonly: true, diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index b5241bd41f..b4cb6f22f7 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -1428,24 +1428,48 @@ describe('SecurityPlugin', () => { return harness.run(opCtx); }; - it('DENIES deleting a platform-managed position (sys_position managed_by:system)', async () => { + it('DENIES deleting a platform-managed position (sys_position managed_by:platform)', async () => { const opCtx: any = { object: 'sys_position', operation: 'delete', options: { where: { id: 'pos_admin' } }, context: adminCtx, }; await expect( - runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })), + runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'platform' })), ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); }); - it('DENIES updating a package-declared position (sys_position managed_by:config)', async () => { + it('DENIES updating a package-declared position (sys_position managed_by:package)', async () => { const opCtx: any = { object: 'sys_position', operation: 'update', data: { id: 'pos_sales', label: 'renamed' }, options: { where: { id: 'pos_sales' } }, context: adminCtx, }; await expect( - runGate(opCtx, () => ({ id: 'pos_sales', name: 'sales_rep', managed_by: 'config' })), + runGate(opCtx, () => ({ id: 'pos_sales', name: 'sales_rep', managed_by: 'package' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + // [#2926 ①] Regression: the A4 rename (system→platform, config→package) once + // disarmed this gate because the provenance map only knew the legacy values. + // Rows the boot normalizer has not healed yet must STAY protected too. + it('KEEPS denying legacy-vocab managed positions (managed_by:system, pre-normalizer rows)', async () => { + const opCtx: any = { + object: 'sys_position', operation: 'delete', + options: { where: { id: 'pos_legacy' } }, context: adminCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'pos_legacy', name: 'everyone', managed_by: 'system' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('KEEPS denying legacy-vocab managed positions (managed_by:config, pre-normalizer rows)', async () => { + const opCtx: any = { + object: 'sys_position', operation: 'update', + data: { id: 'pos_legacy2', label: 'renamed' }, options: { where: { id: 'pos_legacy2' } }, + context: adminCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'pos_legacy2', name: 'sales_rep', managed_by: 'config' })), ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); }); @@ -1470,13 +1494,23 @@ describe('SecurityPlugin', () => { ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); }); - it('ALLOWS deleting an admin-authored position (sys_position managed_by:user)', async () => { + it('ALLOWS deleting an admin-authored position (sys_position managed_by:admin)', async () => { const opCtx: any = { object: 'sys_position', operation: 'delete', options: { where: { id: 'pos_user' } }, context: adminCtx, }; await expect( - runGate(opCtx, () => ({ id: 'pos_user', name: 'my_team', managed_by: 'user' })), + runGate(opCtx, () => ({ id: 'pos_user', name: 'my_team', managed_by: 'admin' })), + ).resolves.toBeDefined(); + }); + + it('ALLOWS deleting a legacy admin-authored position (managed_by:user, pre-normalizer)', async () => { + const opCtx: any = { + object: 'sys_position', operation: 'delete', + options: { where: { id: 'pos_user_legacy' } }, context: adminCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'pos_user_legacy', name: 'my_team', managed_by: 'user' })), ).resolves.toBeDefined(); }); @@ -1502,10 +1536,18 @@ describe('SecurityPlugin', () => { ).resolves.toBeDefined(); }); - it('DENIES an admin-door insert that forges platform provenance (sys_position managed_by:system)', async () => { + it('DENIES an admin-door insert that forges platform provenance (sys_position managed_by:platform)', async () => { + const opCtx: any = { + object: 'sys_position', operation: 'insert', + data: { name: 'forged', managed_by: 'platform' }, context: adminCtx, + }; + await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('DENIES an admin-door insert that forges LEGACY platform provenance (managed_by:system)', async () => { const opCtx: any = { object: 'sys_position', operation: 'insert', - data: { name: 'forged', managed_by: 'system' }, context: adminCtx, + data: { name: 'forged_legacy', managed_by: 'system' }, context: adminCtx, }; await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); }); @@ -1549,14 +1591,14 @@ describe('SecurityPlugin', () => { context: adminCtx, }; await expect( - runGate(opCtx, () => ({ id: 'pos_admin', managed_by: 'system' })), + runGate(opCtx, () => ({ id: 'pos_admin', managed_by: 'platform' })), ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); }); it('ALLOWS a filter delete that matches only admin-authored rows (probe finds none)', async () => { const opCtx: any = { object: 'sys_position', operation: 'delete', - options: { where: { managed_by: 'user' } }, + options: { where: { managed_by: 'admin' } }, context: adminCtx, }; // The managed-row probe finds nothing → the write proceeds. @@ -1570,7 +1612,7 @@ describe('SecurityPlugin', () => { context: {}, // no roles, no permissions, no userId, not isSystem }; await expect( - runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })), + runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'platform' })), ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); }); @@ -1581,7 +1623,7 @@ describe('SecurityPlugin', () => { context: { isSystem: true }, }; await expect( - runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })), + runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'platform' })), ).resolves.toBeDefined(); }); }); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 92b702959f..021e25664d 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -113,13 +113,18 @@ const EMPTY_REQUIRED_PERMISSIONS: NormalizedRequiredPermissions = Object.freeze( * [ADR-0066 / #2918] Provenance spec for the platform/application asset objects * whose managed rows are write-protected by {@link SecurityPlugin.assertSystemRowWriteGate}. * - * The two objects use DIFFERENT `managed_by` vocabularies but the same ownership - * idea — a row authored by the platform or an application package is not the - * admin's to delete or rewrite: - * • sys_position.managed_by — `system` (platform built-in) / `config` - * (package declared) are managed; `user`/∅ (tenant-authored) are the admin's. - * • sys_capability.managed_by — `platform` / `package` are managed; `admin` - * (created in Setup) is the admin's. + * Both objects share the unified A4 (#2920) `managed_by` vocabulary — a row + * authored by the platform or an application package is not the admin's to + * delete or rewrite: + * • `platform` / `package` are managed; `admin`/∅ (tenant-authored) rows are + * the admin's. + * • sys_position additionally keeps its LEGACY values `system` (→ platform) + * and `config` (→ package) in the managed map: the boot normalizer + * (normalize-managed-by.ts) heals stored rows to the canonical vocabulary, + * but rows written before the normalizer runs — or in a store it has not + * touched yet — must not lose protection in the interim. Dropping the + * legacy keys here is what silently disarmed this gate for sys_position + * after the A4 rename (#2926 ①). * The map value for each managed `managed_by` is the human owner label used in * the (business-message-only) deny text. */ @@ -130,7 +135,13 @@ const SYSTEM_ROW_PROVENANCE: Record< sys_position: { noun: 'position', pluralNoun: 'positions', - managed: { system: 'the platform', config: 'an application package' }, + managed: { + platform: 'the platform', + package: 'an application package', + // Legacy pre-A4 values — keep guarded until every store is normalized. + system: 'the platform', + config: 'an application package', + }, }, sys_capability: { noun: 'capability', diff --git a/packages/plugins/plugin-sharing/src/boot-backfill.test.ts b/packages/plugins/plugin-sharing/src/boot-backfill.test.ts new file mode 100644 index 0000000000..e6e8397c30 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/boot-backfill.test.ts @@ -0,0 +1,128 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#2926 ③] Boot backfill of sharing-rule grants. + * + * Rule grants are materialized by write hooks, which deliberately skip + * `isSystem` writes (rule-hooks.ts) — so records created by the boot-time + * seed loader (always `isSystem`) never produced `sys_record_share` rows: + * demo data shipping with matching sharing rules was broken out of the box + * until an admin "touched" each record at runtime. `backfillRuleGrants` + * reconciles every active rule once per boot, idempotently. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; +import { backfillRuleGrants } from './sharing-plugin.js'; + +interface Row { [k: string]: any } + +const SYS = { isSystem: true } as any; + +function makeEngine() { + const tables: Record = {}; + const ensure = (n: string) => (tables[n] ??= []); + function matches(row: Row, f: any): boolean { + if (!f || typeof f !== 'object') return true; + if (Array.isArray(f.$or)) return f.$or.some((x: any) => matches(row, x)); + if (Array.isArray(f.$and)) return f.$and.every((x: any) => matches(row, x)); + for (const [k, v] of Object.entries(f)) { + if (k === '$or' || k === '$and') continue; + const rv = row[k]; + if (v != null && typeof v === 'object' && '$in' in (v as any)) { + if (!(v as any).$in.includes(rv)) return false; + continue; + } + if (v != null && typeof v === 'object' && '$gte' in (v as any)) { + if (!(rv >= (v as any).$gte)) return false; + continue; + } + if (rv !== v) return false; + } + return true; + } + return { + _tables: tables, + getSchema() { return undefined; }, + async find(o: string, opts?: any) { + const f = opts?.filter ?? opts?.where; + return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000); + }, + async insert(o: string, data: any) { const row = { ...data }; ensure(o).push(row); return row; }, + async update(o: string, idOrData: any, dataOrOpts?: any) { + const data = typeof idOrData === 'object' ? idOrData : dataOrOpts; + const id = typeof idOrData === 'object' ? idOrData.id : idOrData; + const t = ensure(o); const i = t.findIndex((r) => r.id === id); + if (i >= 0) t[i] = { ...t[i], ...data }; + return t[i]; + }, + async delete(o: string, opts?: any) { + const t = ensure(o); const where = opts?.where ?? {}; + for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); + return { ok: true }; + }, + }; +} + +describe('backfillRuleGrants (#2926 ③ — seed rows materialize at boot)', () => { + let engine: ReturnType; + let sharing: SharingService; + let rules: SharingRuleService; + + beforeEach(async () => { + engine = makeEngine(); + sharing = new SharingService({ engine: engine as any }); + rules = new SharingRuleService({ engine: engine as any, sharing }); + // Seed-loader analog: records written directly (isSystem path) — the + // write hooks never ran, so sys_record_share is empty. + engine._tables.showcase_inquiry = [ + { id: 'inq_new', status: 'new', owner_id: 'priya' }, + { id: 'inq_won', status: 'won', owner_id: 'priya' }, + ]; + await rules.defineRule({ + name: 'new_inquiries_to_wes', label: 'New inquiries → wes', object: 'showcase_inquiry', + criteria: { status: 'new' }, + recipientType: 'user', recipientId: 'wes', accessLevel: 'read', + }, SYS); + }); + + it('materializes grants for seed records that match an active rule', async () => { + expect(engine._tables.sys_record_share ?? []).toHaveLength(0); + const active = await rules.listRules({ activeOnly: true }, SYS); + const reconciled = await backfillRuleGrants(rules, active); + expect(reconciled).toBe(1); + const shares = engine._tables.sys_record_share ?? []; + expect(shares).toHaveLength(1); + expect(shares[0]).toMatchObject({ record_id: 'inq_new', recipient_id: 'wes' }); + }); + + it('is idempotent across repeated boots (no duplicate grants)', async () => { + const active = await rules.listRules({ activeOnly: true }, SYS); + await backfillRuleGrants(rules, active); + await backfillRuleGrants(rules, active); + expect(engine._tables.sys_record_share ?? []).toHaveLength(1); + }); + + it('one broken rule does not block the others (best-effort per rule)', async () => { + await rules.defineRule({ + name: 'zzz_broken', label: 'Broken', object: 'showcase_inquiry', + criteria: { status: 'new' }, + recipientType: 'user', recipientId: 'someone', accessLevel: 'read', + }, SYS); + const active = await rules.listRules({ activeOnly: true }, SYS); + // Blow up evaluation of the broken rule only. + const realEvaluate = rules.evaluateRule.bind(rules); + vi.spyOn(rules, 'evaluateRule').mockImplementation(async (idOrName: string, context: any) => { + const rule = await rules.getRule(idOrName, context); + if (rule?.name === 'zzz_broken') throw new Error('boom'); + return realEvaluate(idOrName, context); + }); + const warn = vi.fn(); + const reconciled = await backfillRuleGrants(rules, active, { warn }); + expect(reconciled).toBe(1); + expect(warn).toHaveBeenCalledOnce(); + // The healthy rule still materialized. + expect((engine._tables.sys_record_share ?? []).some((s) => s.record_id === 'inq_new')).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts index 18f8c6e677..b7b1043450 100644 --- a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts +++ b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts @@ -122,6 +122,10 @@ export async function bootstrapDeclaredSharingRules( recipientId: String(r.sharedWith.value), accessLevel: (r.accessLevel ?? 'read') as ShareAccessLevel, active: r.active !== false, + // [#2909 P0] Declared rules ship with the app/package → seed mode: + // pristine rows keep receiving declared updates; admin-authored or + // customized rows are never clobbered (defineRule seed-not-clobber). + managedBy: 'package', } as any, SYSTEM_CTX as any); seeded += 1; } catch (err: any) { diff --git a/packages/plugins/plugin-sharing/src/index.ts b/packages/plugins/plugin-sharing/src/index.ts index fa6d559dee..25132b2046 100644 --- a/packages/plugins/plugin-sharing/src/index.ts +++ b/packages/plugins/plugin-sharing/src/index.ts @@ -31,9 +31,15 @@ export { export { TeamGraphService, expandPrincipal, type TeamGraphOptions } from './team-graph.js'; export { BusinessUnitGraphService, type BusinessUnitGraphOptions } from './business-unit-graph.js'; export { bindRuleHooks, unbindAllRuleHooks, SHARING_RULE_HOOK_PACKAGE } from './rule-hooks.js'; +export { + bindRuleProvenanceStamp, + unbindRuleProvenanceStamp, + SHARING_RULE_PROVENANCE_PACKAGE, +} from './sharing-rule-provenance.js'; export { SharingServicePlugin, buildSharingMiddleware, + backfillRuleGrants, type SharingPluginOptions, } from './sharing-plugin.js'; export type { diff --git a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts index 153954c4f2..0bd3a29bfc 100644 --- a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts +++ b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts @@ -188,6 +188,42 @@ export const SysSharingRule = ObjectSchema.create({ group: 'Lifecycle', }), + // ── Provenance (#2909 P0 — record-authoritative seed-not-clobber) ── + // Unified A4 (#2920) tri-state, shared verbatim with sys_position / + // sys_capability / sys_permission_set. Both columns are `readonly`: + // the engine strips them from non-system payloads (forge/clear-proof), + // while the seeder and the provenance stamp hook write with isSystem. + // NOTE deliberately NOT in SYSTEM_ROW_PROVENANCE (no write gate): + // sharing rules are a first-class admin authoring/tuning surface — + // admins may edit or deactivate package rules; the seeder simply stops + // overwriting rows once `customized` is stamped (ADR-0094 addendum). + managed_by: Field.select({ + label: 'Managed By', + required: false, + readonly: true, + defaultValue: 'admin', + description: + 'Record provenance (unified tri-state, A4 #2920): platform = framework built-in / ' + + 'package = app/package-declared (boot-seeded) / admin = tenant-created in Setup.', + options: [ + { value: 'platform', label: 'Platform' }, + { value: 'package', label: 'Package' }, + { value: 'admin', label: 'Admin' }, + ], + group: 'System', + }), + + customized: Field.boolean({ + label: 'Customized', + required: false, + readonly: true, + defaultValue: false, + description: + 'Set when an admin edits a package-declared rule; boot seeding will no longer ' + + 'overwrite the row (deactivations survive redeploys). Meaningless on admin rows.', + group: 'System', + }), + created_at: Field.datetime({ label: 'Created At', required: true, diff --git a/packages/plugins/plugin-sharing/src/sharing-plugin.ts b/packages/plugins/plugin-sharing/src/sharing-plugin.ts index 7abf592a16..213022765e 100644 --- a/packages/plugins/plugin-sharing/src/sharing-plugin.ts +++ b/packages/plugins/plugin-sharing/src/sharing-plugin.ts @@ -11,6 +11,7 @@ import { SharingRuleService } from './sharing-rule-service.js'; import { ShareLinkService } from './share-link-service.js'; import { registerShareLinkRoutes } from './share-link-routes.js'; import { bindRuleHooks, unbindAllRuleHooks, RULE_REBIND_TRIGGER_PACKAGE } from './rule-hooks.js'; +import { bindRuleProvenanceStamp, unbindRuleProvenanceStamp } from './sharing-rule-provenance.js'; import { bindPrimaryBuHooks, backfillPrimaryBu } from './primary-bu-projection.js'; import { bootstrapDeclaredSharingRules } from './bootstrap-declared-sharing-rules.js'; @@ -36,6 +37,44 @@ export interface SharingPluginOptions { shareLinkBasePath?: string; } +/** + * [#2926 ③] Boot backfill: rule grants are materialized by the write hooks, + * but seed rows are written with `isSystem` (which the hooks deliberately + * skip — see rule-hooks.ts), so a fresh deploy's seed data carried no + * `sys_record_share` rows until each record was touched at runtime. + * Reconcile every active rule once per boot: `evaluateRule` is idempotent + * (diff-based grant/update/revoke), so repeated boots are no-ops. + * Best-effort per rule — one broken rule must not block startup or its + * siblings. Returns the number of rules successfully reconciled. + */ +export async function backfillRuleGrants( + ruleService: SharingRuleService, + rules: Array<{ id?: string; name?: string }>, + logger?: { info?: (msg: string, meta?: any) => void; warn?: (msg: string, meta?: any) => void }, +): Promise { + const start = Date.now(); + let reconciled = 0; + for (const rule of rules) { + try { + await ruleService.evaluateRule((rule.id ?? rule.name) as string, { isSystem: true } as any); + reconciled += 1; + } catch (err: any) { + logger?.warn?.('SharingServicePlugin: boot rule backfill failed for rule', { + rule: rule.name ?? rule.id, + error: err?.message, + }); + } + } + if (rules.length > 0) { + logger?.info?.('SharingServicePlugin: boot rule backfill done', { + rules: rules.length, + reconciled, + ms: Date.now() - start, + }); + } + return reconciled; +} + /** * SharingServicePlugin — registers `sys_record_share`, the `sharing` * service, and the engine middleware that enforces @@ -262,6 +301,18 @@ export class SharingServicePlugin implements Plugin { unbindAllRuleHooks(engine); bindRuleHooks(engine, this.ruleService, rules, ctx.logger as any); this.bindRuleRebindTriggers(engine, ctx); + + // [#2909 T1] Stamp `customized` on admin edits of seeded rules so + // the boot seeder stops overwriting them (seed-not-clobber). + unbindRuleProvenanceStamp(engine); + bindRuleProvenanceStamp(engine, ctx.logger as any); + + // [#2926 ③] Reconciling existing rows against every rule is + // deferred to `kernel:listening` (below): seed data is loaded on + // `kernel:ready` (raced against a budget, and the AppPlugin's seed + // hook is a *different* kernel:ready handler), so a backfill here + // would race the very records it must materialize. `kernel:listening` + // fires only after every kernel:ready handler has settled. } else { ctx.logger.warn('SharingServicePlugin: engine has no hook API — sharing rule auto-evaluation disabled'); } @@ -342,6 +393,23 @@ export class SharingServicePlugin implements Plugin { ctx.logger.warn('SharingServicePlugin: share-link subsystem not started', { error: err?.message }); } }); + + // [#2926 ③] Materialize sharing grants for rows already present at boot — + // notably SeedLoader-inserted seed records, whose write goes through the + // isSystem short-circuit in the rule hooks and therefore never produces a + // `sys_record_share`. Runs on `kernel:listening` (Phase 4), after every + // `kernel:ready` handler — including the AppPlugin seed loader — has + // completed, so the reconcile sees the seeded rows. Idempotent: a runtime + // write that already materialized a grant is reconciled to the same state. + ctx.hook('kernel:listening', async () => { + if (!this.ruleService) return; + try { + const rules = await this.ruleService.listRules({ activeOnly: true }, { isSystem: true } as any); + await backfillRuleGrants(this.ruleService, rules, ctx.logger as any); + } catch (err: any) { + ctx.logger.warn('SharingServicePlugin: boot rule backfill (kernel:listening) failed', { error: err?.message }); + } + }); } } diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts new file mode 100644 index 0000000000..a9cb185583 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts @@ -0,0 +1,200 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#2909 P0/T1] sys_sharing_rule provenance + seed-not-clobber. + * + * sys_sharing_rule is record-authoritative (ADR-0094 addendum): declared + * rules are a boot seed, the row is the authority. The seed (defineRule with + * managedBy package/platform): + * - adopts pristine/legacy rows and keeps updating them (package upgrades + * stay deliverable), + * - NEVER overwrites a row the admin authored (managed_by admin) or + * customized — most importantly an admin's `active: false` on an + * over-sharing rule must survive redeploys (no resurrection), + * - non-seed defineRule keeps its historical clobber semantics. + * The `customized` stamp is applied by a beforeUpdate hook on any + * non-system edit of a package/platform row. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; +import { bindRuleProvenanceStamp, SHARING_RULE_PROVENANCE_PACKAGE } from './sharing-rule-provenance.js'; + +interface Row { [k: string]: any } + +const SYS = { isSystem: true } as any; + +function makeEngine() { + const tables: Record = {}; + const hooks: Array<{ event: string; handler: (ctx: any) => any; options: Row }> = []; + const ensure = (n: string) => (tables[n] ??= []); + function matches(row: Row, f: any): boolean { + if (!f || typeof f !== 'object') return true; + for (const [k, v] of Object.entries(f)) { + if (row[k] !== v) return false; + } + return true; + } + return { + _tables: tables, + _hooks: hooks, + getSchema() { return undefined; }, + async find(o: string, opts?: any) { + const f = opts?.filter ?? opts?.where; + return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000); + }, + async insert(o: string, data: any) { const row = { ...data }; ensure(o).push(row); return row; }, + async update(o: string, idOrData: any, dataOrOpts?: any) { + const data = typeof idOrData === 'object' ? idOrData : dataOrOpts; + const id = typeof idOrData === 'object' ? idOrData.id : idOrData; + const t = ensure(o); const i = t.findIndex((r) => r.id === id); + if (i >= 0) t[i] = { ...t[i], ...data }; + return t[i]; + }, + async delete(o: string, opts?: any) { + const t = ensure(o); const where = opts?.where ?? {}; + for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); + return { ok: true }; + }, + registerHook(event: string, handler: (ctx: any) => any, options: Row = {}) { + hooks.push({ event, handler, options }); + }, + unregisterHooksByPackage(packageId: string) { + let removed = 0; + for (let i = hooks.length - 1; i >= 0; i--) { + if (hooks[i].options.packageId === packageId) { hooks.splice(i, 1); removed++; } + } + return removed; + }, + /** Test helper: simulate an engine update passing through beforeUpdate hooks. */ + async updateThroughHooks(o: string, id: string, data: Row, session: Row) { + for (const h of hooks) { + if (h.event === 'beforeUpdate' && h.options.object === o) { + await h.handler({ session, input: { id, data } }); + } + } + return this.update(o, id, data); + }, + }; +} + +const DECLARED = { + name: 'red_projects_to_exec', label: 'Red projects → exec', object: 'showcase_project', + criteria: { health: 'red' }, + recipientType: 'position' as const, recipientId: 'exec', accessLevel: 'read' as const, + managedBy: 'package' as const, +}; + +describe('defineRule seed-not-clobber (#2909 P0/T1)', () => { + let engine: ReturnType; + let rules: SharingRuleService; + + beforeEach(() => { + engine = makeEngine(); + const sharing = new SharingService({ engine: engine as any }); + rules = new SharingRuleService({ engine: engine as any, sharing }); + }); + + it('seeds a new declared rule with managed_by:package and customized:false', async () => { + const r = await rules.defineRule(DECLARED as any, SYS); + expect(r.managed_by).toBe('package'); + expect(r.customized).toBe(false); + expect(engine._tables.sys_sharing_rule[0]).toMatchObject({ managed_by: 'package', customized: false }); + }); + + it('keeps updating a PRISTINE seeded rule on re-seed (package upgrades deliverable)', async () => { + await rules.defineRule(DECLARED as any, SYS); + const r = await rules.defineRule({ ...DECLARED, label: 'Red projects → exec v2', accessLevel: 'edit' } as any, SYS); + expect(r.label).toBe('Red projects → exec v2'); + expect(r.access_level).toBe('edit'); + expect(engine._tables.sys_sharing_rule).toHaveLength(1); + }); + + it('ADOPTS a legacy row with no provenance (stamps managed_by on re-seed)', async () => { + engine._tables.sys_sharing_rule = [{ + id: 'srule_legacy', name: DECLARED.name, label: 'old', object_name: 'showcase_project', + criteria_json: null, recipient_type: 'position', recipient_id: 'exec', + access_level: 'read', active: true, + }]; + const r = await rules.defineRule(DECLARED as any, SYS); + expect(r.managed_by).toBe('package'); + expect(engine._tables.sys_sharing_rule[0].managed_by).toBe('package'); + }); + + it('does NOT resurrect an admin-deactivated seeded rule (customized survives redeploys)', async () => { + await rules.defineRule(DECLARED as any, SYS); + // Admin deactivates the over-sharing rule (stamp applied by the hook — here direct). + const row = engine._tables.sys_sharing_rule[0]; + await engine.update('sys_sharing_rule', { id: row.id, active: false, customized: true }); + // Next boot re-seeds with active:true… + const r = await rules.defineRule(DECLARED as any, SYS); + expect(r.active).toBe(false); + expect(engine._tables.sys_sharing_rule[0].active).toBe(false); + }); + + it('never touches an admin-authored rule that collides on name (admin row wins + warn)', async () => { + const warn = vi.fn(); + const sharing = new SharingService({ engine: engine as any }); + const svc = new SharingRuleService({ engine: engine as any, sharing, logger: { warn } }); + await svc.defineRule({ ...DECLARED, managedBy: undefined, label: 'Admin authored' } as any, SYS); + expect(engine._tables.sys_sharing_rule[0].managed_by).toBe('admin'); + const r = await svc.defineRule(DECLARED as any, SYS); + expect(r.label).toBe('Admin authored'); + expect(engine._tables.sys_sharing_rule[0].label).toBe('Admin authored'); + expect(warn).toHaveBeenCalled(); + }); + + it('non-seed defineRule keeps clobber semantics and stamps admin provenance on insert', async () => { + const first = await rules.defineRule({ ...DECLARED, managedBy: undefined } as any, SYS); + expect(first.managed_by).toBe('admin'); + const r = await rules.defineRule({ ...DECLARED, managedBy: undefined, label: 'edited' } as any, SYS); + expect(r.label).toBe('edited'); + // Programmatic re-define does not touch provenance columns. + expect(engine._tables.sys_sharing_rule[0].managed_by).toBe('admin'); + }); +}); + +describe('provenance stamp hook (#2909 T1)', () => { + let engine: ReturnType; + let rules: SharingRuleService; + + beforeEach(async () => { + engine = makeEngine(); + const sharing = new SharingService({ engine: engine as any }); + rules = new SharingRuleService({ engine: engine as any, sharing }); + await rules.defineRule(DECLARED as any, SYS); + bindRuleProvenanceStamp(engine as any); + }); + + it('stamps customized:true when a non-system caller edits a package rule', async () => { + const row = engine._tables.sys_sharing_rule[0]; + await engine.updateThroughHooks('sys_sharing_rule', row.id, { active: false }, { userId: 'admin1' }); + expect(engine._tables.sys_sharing_rule[0]).toMatchObject({ active: false, customized: true }); + }); + + it('does NOT stamp on isSystem writes (seeder/backfill are not customizations)', async () => { + const row = engine._tables.sys_sharing_rule[0]; + await engine.updateThroughHooks('sys_sharing_rule', row.id, { label: 'reseed' }, { isSystem: true }); + expect(engine._tables.sys_sharing_rule[0].customized).toBe(false); + }); + + it('does NOT stamp admin-authored rows (an env row IS the definition)', async () => { + await rules.defineRule({ ...DECLARED, name: 'admin_rule', managedBy: undefined } as any, SYS); + const adminRow = engine._tables.sys_sharing_rule.find((r) => r.name === 'admin_rule')!; + await engine.updateThroughHooks('sys_sharing_rule', adminRow.id, { active: false }, { userId: 'admin1' }); + expect(engine._tables.sys_sharing_rule.find((r) => r.name === 'admin_rule')!.customized).toBe(false); + }); + + it('ignores multi-row updates (no id) without crashing', async () => { + const hook = engine._hooks.find((h) => h.options.packageId === SHARING_RULE_PROVENANCE_PACKAGE)!; + await expect(hook.handler({ session: { userId: 'admin1' }, input: { data: { active: false } } })).resolves.toBeUndefined(); + }); + + it('end-to-end: admin edit through hooks → next seed does not clobber', async () => { + const row = engine._tables.sys_sharing_rule[0]; + await engine.updateThroughHooks('sys_sharing_rule', row.id, { active: false }, { userId: 'admin1' }); + const r = await rules.defineRule(DECLARED as any, SYS); + expect(r.active).toBe(false); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-provenance.ts b/packages/plugins/plugin-sharing/src/sharing-rule-provenance.ts new file mode 100644 index 0000000000..ad4d2cd354 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/sharing-rule-provenance.ts @@ -0,0 +1,84 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#2909 T1] Provenance stamp for `sys_sharing_rule`. + * + * sys_sharing_rule is RECORD-AUTHORITATIVE (ADR-0094 addendum): declared + * rules are a boot seed, and the row — including any admin tuning such as + * deactivating an over-sharing rule — is the authority. The seeder + * (defineRule in seed mode) skips rows marked `customized`, so this hook is + * the half that DETECTS the admin edit: any non-system update touching a + * package/platform-seeded row stamps `customized: true` onto the payload. + * + * Why a data hook (and not a write gate or the REST layer): + * - admins edit rules through several doors (Setup UI generic data door, + * scripts, console) — an engine hook covers them all; + * - unlike sys_position/sys_capability there is deliberately NO + * SYSTEM_ROW_PROVENANCE write gate here: sharing rules are a first-class + * admin authoring surface, so edits are allowed — they just have to be + * remembered; + * - both provenance columns are `readonly`, and the engine's readonly strip + * exempts isSystem callers while snapshotting supplied keys BEFORE hooks + * run — so a caller can never forge/clear `customized`, while this hook's + * stamp survives. + * + * Known boundary (recorded in the ADR): multi-row updates (no single + * `input.id`) are not stamped — every rule-editing UI path updates by id. + */ + +interface MinimalEngine { + find(object: string, opts?: any): Promise; + registerHook(event: string, handler: (ctx: any) => any, options?: Record): void; + unregisterHooksByPackage(packageId: string): number; +} + +interface MinimalLogger { + info?: (msg: string, meta?: Record) => void; + warn?: (msg: string, meta?: Record) => void; +} + +export const SHARING_RULE_PROVENANCE_PACKAGE = 'plugin-sharing:rule-provenance'; + +const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; + +export function bindRuleProvenanceStamp(engine: MinimalEngine, logger?: MinimalLogger): void { + engine.registerHook( + 'beforeUpdate', + async (ctx: any) => { + // Seeder / defineRule / boot reconcilers write with isSystem — those + // are the package door, not an admin customization. + if ((ctx?.session as any)?.isSystem) return; + const id = ctx?.input?.id ?? (ctx?.input?.data as any)?.id; + if (!id) return; // multi-row update — see boundary note above + const data = ctx?.input?.data; + if (!data || typeof data !== 'object') return; + try { + // `previous` is not resolved before beforeUpdate hooks run — read the + // current row ourselves (system ctx: this is a provenance check, not + // an authorization decision). + const rows = await engine.find('sys_sharing_rule', { + filter: { id }, + fields: ['id', 'managed_by', 'customized'], + limit: 1, + context: SYSTEM_CTX, + }); + const row = Array.isArray(rows) ? rows[0] : undefined; + if (!row) return; + if ((row.managed_by === 'package' || row.managed_by === 'platform') && row.customized !== true) { + (data as any).customized = true; + } + } catch (err: any) { + logger?.warn?.('[sharing-rule] provenance stamp failed (edit proceeds unstamped)', { + id, + error: err?.message, + }); + } + }, + { object: 'sys_sharing_rule', packageId: SHARING_RULE_PROVENANCE_PACKAGE, priority: 150 }, + ); + logger?.info?.('[sharing-rule] provenance stamp hook bound'); +} + +export function unbindRuleProvenanceStamp(engine: MinimalEngine): number { + return engine.unregisterHooksByPackage(SHARING_RULE_PROVENANCE_PACKAGE); +} diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index fdd9665c3c..5121aa550a 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -52,6 +52,8 @@ function rowFromRule(row: any): SharingRuleRow { recipient_id: row.recipient_id, access_level: row.access_level as ShareAccessLevel, active: row.active !== false, + managed_by: row.managed_by ?? null, + customized: row.customized === true, created_at: row.created_at ?? undefined, updated_at: row.updated_at ?? undefined, }; @@ -102,8 +104,30 @@ export class SharingRuleService implements ISharingRuleService { limit: 1, context: SYSTEM_CTX, }); + // [#2909 P0/T1] Seed mode: a package/platform managedBy marks this call + // as the boot seeder (bootstrapDeclaredSharingRules) rather than an + // admin/programmatic authoring path. sys_sharing_rule is + // RECORD-AUTHORITATIVE (ADR-0094 addendum): the declared metadata is a + // seed, not a live override, so the seeder must never clobber a row the + // admin owns or has customized — most importantly an admin's + // `active: false` on an over-sharing rule must survive redeploys. + const seedMode = input.managedBy === 'package' || input.managedBy === 'platform'; + if (Array.isArray(existing) && existing[0]) { const row: any = existing[0]; + if (seedMode) { + if (row.managed_by === 'admin') { + // Name collision with a tenant-authored rule — the admin's row wins. + this.logger?.warn?.('[sharing-rule] declared rule name collides with an admin-authored rule — seed skipped', { + rule: input.name, + }); + return rowFromRule(row); + } + if (row.customized === true) { + // Admin edited/deactivated this seeded rule — never resurrect it. + return rowFromRule(row); + } + } const patch: any = { id: row.id, label: input.label, @@ -115,6 +139,9 @@ export class SharingRuleService implements ISharingRuleService { access_level: accessLevel, active, updated_at: now, + // Seed mode adopts pristine/legacy (pre-provenance) rows so future + // boots recognize them; non-seed calls never touch provenance. + ...(seedMode ? { managed_by: input.managedBy } : {}), }; await this.engine.update('sys_sharing_rule', patch, { context: SYSTEM_CTX }); return rowFromRule({ ...row, ...patch }); @@ -132,6 +159,8 @@ export class SharingRuleService implements ISharingRuleService { recipient_id: input.recipientId, access_level: accessLevel, active, + managed_by: input.managedBy ?? 'admin', + customized: false, created_at: now, updated_at: now, }; diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 350522f32f..05b5bf9acb 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -157,6 +157,27 @@ export function mapDataError(error: any, object?: string): { status: number; bod }, }; } + // Generic passthrough for domain errors that already carry an explicit + // HTTP status (e.g. plugin-sharing's record-scope denial: status 403 + + // code FORBIDDEN) — mirrors sendError's `.status` handling, which the + // generic data routes bypass by calling mapDataError directly (#2926 ⑦). + // Placed AFTER the structured-code branches above (409s carry rich + // fields this envelope would drop) and deliberately limited to 4xx: + // 5xx messages keep going through the sanitizing heuristics below so + // internal/SQL details never reach the client verbatim. + if (typeof error?.status === 'number' && error.status >= 400 && error.status < 500) { + const msg = typeof error?.message === 'string' && error.message.length > 0 && error.message.length < 500 + ? error.message + : 'Request failed'; + return { + status: error.status, + body: { + error: msg, + ...(typeof error?.code === 'string' && error.code ? { code: error.code } : {}), + ...(object ? { object } : {}), + }, + }; + } const raw = String(error?.message ?? error ?? ''); const lower = raw.toLowerCase(); @@ -3259,7 +3280,7 @@ export class RestServer { res.json(result); } catch (error: any) { const mapped = mapDataError(error, req.params?.object); - if (mapped.status === 404 || mapped.status === 503 || mapped.status === 502) { + if (isExpectedDataStatus(mapped.status) || mapped.body?.code === 'UNSUPPORTED_QUERY_PARAM') { res.status(mapped.status).json(mapped.body); } else { logError("[REST] Unhandled error:", error); diff --git a/packages/rest/src/rest.test.ts b/packages/rest/src/rest.test.ts index ef2a48aa86..9c63ccff3c 100644 --- a/packages/rest/src/rest.test.ts +++ b/packages/rest/src/rest.test.ts @@ -2140,6 +2140,70 @@ describe('mapDataError — schema/constraint envelopes', () => { expect(r.body.object).toBe('crm_lead'); }); + // #2926 ⑦: plugin-sharing's record-scope denial throws with an explicit + // status 403 + code FORBIDDEN, but the generic data routes bypass + // sendError's `.status` passthrough — mapDataError must honor the + // explicit status itself or the 403 degrades to the catch-all 400. + it('passes through an explicit 4xx status + code (sharing FORBIDDEN → 403)', () => { + const r = mapDataError( + Object.assign(new Error('FORBIDDEN: insufficient privileges to update showcase_inquiry rec1'), { + code: 'FORBIDDEN', + status: 403, + }), + 'showcase_inquiry', + ); + expect(r.status).toBe(403); + expect(r.body.code).toBe('FORBIDDEN'); + expect(r.body.object).toBe('showcase_inquiry'); + expect(r.body.error).toContain('insufficient privileges'); + }); + + it('does NOT pass through an explicit 5xx status (message stays sanitized)', () => { + const r = mapDataError( + Object.assign(new Error('connect ECONNREFUSED 10.0.0.5:5432 (internal pool)'), { status: 502 }), + ); + // 5xx bypasses the passthrough and falls into the sanitizing heuristics. + expect(r.status).not.toBe(502); + expect(r.body.code).not.toBe('FORBIDDEN'); + }); + + it('guards the passthrough message length (oversized → generic text)', () => { + const r = mapDataError(Object.assign(new Error('x'.repeat(600)), { status: 403, code: 'FORBIDDEN' })); + expect(r.status).toBe(403); + expect(r.body.error).toBe('Request failed'); + }); + + it('keeps CONCURRENT_UPDATE 409 structured fields despite its own status property', () => { + const r = mapDataError( + Object.assign(new Error('Record was modified'), { + code: 'CONCURRENT_UPDATE', + status: 409, + currentVersion: 7, + currentRecord: { id: 'r1' }, + }), + 'sys_task', + ); + expect(r.status).toBe(409); + expect(r.body.code).toBe('CONCURRENT_UPDATE'); + expect(r.body.currentVersion).toBe(7); + expect(r.body.currentRecord).toEqual({ id: 'r1' }); + }); + + it('keeps DELETE_RESTRICTED 409 structured fields despite its own status property', () => { + const r = mapDataError( + Object.assign(new Error('Cannot delete sys_position (p1): 1 dependent record'), { + code: 'DELETE_RESTRICTED', + status: 409, + dependentObject: 'sys_position_permission_set', + dependentCount: 1, + }), + 'sys_position', + ); + expect(r.status).toBe(409); + expect(r.body.code).toBe('DELETE_RESTRICTED'); + expect(r.body.dependentCount).toBe(1); + }); + it('maps SQLite "has no column named" → 400 INVALID_FIELD with the field', () => { const r = mapDataError( sqliteError( diff --git a/packages/spec/src/contracts/sharing-service.ts b/packages/spec/src/contracts/sharing-service.ts index 1947549dcf..97f07f087e 100644 --- a/packages/spec/src/contracts/sharing-service.ts +++ b/packages/spec/src/contracts/sharing-service.ts @@ -155,6 +155,19 @@ export interface SharingRuleRow { recipient_id: string; access_level: ShareAccessLevel; active: boolean; + /** + * [#2909 P0] Record provenance — unified A4 tri-state + * (`platform` / `package` / `admin`). Package/platform rows are boot-seeded + * and become seed-not-clobber once `customized` is set; admin rows are + * tenant-authored and never touched by the seeder. + */ + managed_by?: 'platform' | 'package' | 'admin' | null; + /** + * [#2909 T1] Stamped when an admin edits a package/platform-seeded rule; + * the boot seeder then stops overwriting the row (an admin's + * `active: false` survives redeploys instead of being resurrected). + */ + customized?: boolean | null; created_at?: string; updated_at?: string; } @@ -170,6 +183,14 @@ export interface DefineSharingRuleInput { recipientId: string; accessLevel?: ShareAccessLevel; active?: boolean; + /** + * [#2909 P0] Provenance to stamp on the row. Passing `package` or + * `platform` puts defineRule in SEED mode: existing rows that an admin + * authored (managed_by `admin`) or customized are left untouched; + * pristine seeded rows keep receiving declared updates. Omitted / + * `admin` = programmatic/tenant authoring (existing clobber semantics). + */ + managedBy?: 'platform' | 'package' | 'admin'; } /** Result of a rule evaluation pass. */ diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index acddef74b4..2b5cdea77f 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -179,12 +179,24 @@ export const PermissionSetSchema = lazySchema(() => z.object({ .describe('[ADR-0086 D3] Record provenance: package (upgrade-owned metadata) vs platform/user (env config)'), /** - * [ADR-0090 D5] Package SUGGESTION: on install the admin is prompted to bind - * this set to the built-in `everyone` position (default grants for - * authenticated users). Never auto-bound; carries no runtime semantics of - * its own. (The former `isProfile` flag was removed by ADR-0090 D2.) + * [ADR-0090 D5] Marks this set as the app's baseline for the built-in + * `everyone` position (default grants for authenticated users). Two tracks + * consume it (#2926 ②): + * + * - **App-level** (the set is declared by the served app itself): the CLI + * resolves the first `isDefault` set as the SecurityPlugin's + * `fallbackPermissionSet`, and the plugin IDEMPOTENTLY AUTO-BINDS it to + * `everyone` at boot — after a high-privilege-bits check refuses + * dangerous sets. Without this a fresh deploy boots with zero bindings + * and every persona silently degrades. + * - **Package-level** (the set ships in an installed package with a + * `packageId`): never auto-bound — it materializes a pending + * `sys_audience_binding_suggestion` row that an admin confirms in Setup. + * + * Carries no runtime semantics of its own beyond these boot-time effects. + * (The former `isProfile` flag was removed by ADR-0090 D2.) */ - isDefault: z.boolean().default(false).describe('[ADR-0090 D5] Install-time suggestion to bind this set to the everyone position (admin confirms; never auto-bound)'), + isDefault: z.boolean().default(false).describe('[ADR-0090 D5] App baseline for the everyone position: app-level sets are auto-bound at boot (guarded, idempotent); package-level sets become install-time suggestions an admin confirms'), /** Object Permissions Map: -> permissions */ objects: z.record(z.string(), ObjectPermissionSchema).describe('Entity permissions'),