diff --git a/.changeset/21756-security-service-declared-members.md b/.changeset/21756-security-service-declared-members.md new file mode 100644 index 00000000000..34ffc298f26 --- /dev/null +++ b/.changeset/21756-security-service-declared-members.md @@ -0,0 +1,13 @@ +--- +'@objectstack/spec': minor +--- + +`ISecurityService` declares the two members the registered `security` service already served without a declaration: `discardPermissionSetOverlay` and `contributeOwnershipFloorAlternates`. Both are optional, and callers feature-detect them. + +Clause-②: yes (widening) + +- **`discardPermissionSetOverlay(callerContext, id)`** (`@objectstack/spec/contracts`). The audited operator action behind a permission set's "Discard Overlay" Setup action: it deletes the stale environment overlay that shadows a package-declared permission set, then re-projects the row from the declared artifact before it resolves. Its docblock names the refusals it throws and the codes they carry: `PERMISSION_DENIED` (403) when the caller is not a tenant-level administrator or no installed package declares the set, `NOT_FOUND` (404) for an unknown row, and `INVALID_STATE` (409) when there is no active overlay to discard. It resolves with the new `PermissionSetOverlayDiscardResult` type. The REST route answers `501 NOT_IMPLEMENTED` when the method is absent. +- **`contributeOwnershipFloorAlternates(plugin, alternates)`**. The seam through which a plugin that installs a tighter row gate stops the platform's `created_by` write floor pre-empting that gate on one object and one limb. Its docblock names what it refuses (a wildcard object, an operation other than exactly `update` or `delete`, a missing or malformed policy), and says a second call replaces the same plugin's first and an empty list withdraws it. The new `OwnershipFloorAlternate` type is the minimal contract shape of one alternate. +- **Optional, and absence is typed.** A security service without either member still satisfies the contract, and an unguarded call does not compile. + +Nothing an author writes changes. An implementation typed as `ISecurityService` that serves either name must now serve it under the declared signature; `@objectstack/plugin-security` already does, and needs no change. diff --git a/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts b/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts new file mode 100644 index 00000000000..315b2e0d56f --- /dev/null +++ b/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts @@ -0,0 +1,185 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every member of the `security` service this plugin registers is either + * DECLARED on `ISecurityService` (`@objectstack/spec/contracts`) or listed in + * this file's ledger with a reason. A served member in neither turns this pin + * red, by name. + * + * ## Why this pin exists + * + * The registered object is two pieces. The literal is typed against + * `ISecurityService`, so the compiler refuses any member the contract does not + * declare. The extension is merged on with `Object.assign` beside it, and + * nothing types that half against the contract. Two cross-package seams were + * served from the extension while the contract did not declare them: the + * overlay discard the REST route calls, and the ownership-floor alternate seam + * `service-storage` calls at boot. A contract reader could not see that either + * seam existed, what it refused, or that a caller must feature-detect it. Both + * are declared now; this pin keeps the next one from going undeclared. + * + * ## How the declared list stays equal to the interface + * + * `DECLARED_MEMBERS` is a test-local list, not a spec export, held to + * `keyof ISecurityService` by a `satisfies` clause. A member added to the + * interface and not listed here fails to compile (missing property); a name + * listed here that the interface does not declare fails to compile (excess + * property); and each entry's `required` / `optional` tag must match the + * interface. The compile half runs in this package's `typecheck` + * (`tsconfig.test.json` compiles every test here); vitest does not type-check. + * A runtime list exported from `packages/spec` would have grown the published + * surface to serve one test, and it could still drift from the interface + * unless something like this clause held it there. + * + * ## What it does NOT pin + * + * Declared-but-not-served is legal for an OPTIONAL member: the contract lets a + * partial implementation omit one, and callers feature-detect. So the reverse + * direction is used only as this pin's non-vacuity control: every REQUIRED + * member must be among the enumerated members, which fails if the boot stopped + * reaching the registration or the enumeration stopped seeing the members. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { ISecurityService } from '@objectstack/spec/contracts'; + +import { SecurityPlugin } from './security-plugin.js'; +import { discardPermissionSetOverlay } from './permission-set-overlay-discard.js'; +import { OwnershipFloorAlternates } from './ownership-floor-alternates.js'; + +/** `optional` exactly when the interface declares the member with `?`. */ +type DeclaredOptionality = { + readonly [K in keyof ISecurityService]-?: object extends Pick ? 'optional' : 'required'; +}; + +/** Every member `ISecurityService` declares — held equal to the interface by the compiler (see the header). */ +const DECLARED_MEMBERS = { + getReadFilter: 'required', + getReadableFields: 'required', + getMetadataReadableFields: 'optional', + getQueryableFields: 'optional', + getWritableFields: 'optional', + resolvePermissionSetNames: 'required', + resolvePermissionSetsForContext: 'optional', + getEffectiveObjectPermissions: 'optional', + canExport: 'required', + canReadObject: 'optional', + hasWriteBypass: 'required', + resolveWriteScope: 'required', + describeDelegationNarrowing: 'optional', + checkAuthoredRowWrite: 'optional', + explain: 'required', + describeDelegableScope: 'required', + listAudienceBindingSuggestions: 'required', + confirmAudienceBindingSuggestion: 'required', + dismissAudienceBindingSuggestion: 'required', + discardPermissionSetOverlay: 'optional', + contributeOwnershipFloorAlternates: 'optional', +} as const satisfies DeclaredOptionality; + +/** + * Members the registered service serves that are deliberately NOT part of the + * contract — plugin-internal, no caller outside this package — each with the + * reason. Empty: every served member is declared. An entry here is a decision + * that a member stays off the contract; a member another package calls is a + * contract, and belongs on `ISecurityService` instead. + */ +const SERVED_NOT_DECLARED: Readonly> = {}; + +/** + * Every member name the object exposes, along its whole prototype chain up to + * (not including) `Object.prototype`, enumerable or not, symbols included. A + * class-backed service keeps its methods on the prototype, where `Object.keys` + * would see nothing and this pin would pass over zero members. + */ +function servedMembers(service: object): string[] { + const names = new Set(); + for (let o: object | null = service; o !== null && o !== Object.prototype; o = Object.getPrototypeOf(o)) { + for (const key of Reflect.ownKeys(o)) { + if (key === 'constructor') continue; + names.add(typeof key === 'symbol' ? key.toString() : key); + } + } + return [...names].sort(); +} + +/** Boot the real plugin far enough to register `security`, and return what it registered. */ +async function registeredSecurityService(): Promise { + const services: Record = { + manifest: { register: vi.fn() }, + objectql: { + registerMiddleware: vi.fn(), + getSchema: () => undefined, + }, + metadata: { + get: async () => undefined, + list: async () => [], + }, + }; + const registerService = vi.fn(); + const ctx: Record = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService, + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin(); + await plugin.init(ctx as any); + await plugin.start(ctx as any); + const security = registerService.mock.calls.find((c: unknown[]) => c[0] === 'security')?.[1]; + if (security === null || typeof security !== 'object') { + throw new Error('the plugin did not register a `security` service object'); + } + return security; +} + +describe('the registered `security` service serves only what ISecurityService declares', () => { + it('every served member is declared on ISecurityService or ledgered here with a reason', async () => { + const served = servedMembers(await registeredSecurityService()); + + // Non-vacuity: the required members are served, so the enumeration below + // is looking at the real object. + const required = Object.entries(DECLARED_MEMBERS) + .filter(([, optionality]) => optionality === 'required') + .map(([name]) => name); + expect( + required.filter((name) => !served.includes(name)), + 'required ISecurityService members missing from the enumerated service', + ).toEqual([]); + + const undeclared = served.filter( + (name) => !Object.hasOwn(DECLARED_MEMBERS, name) && !Object.hasOwn(SERVED_NOT_DECLARED, name), + ); + expect( + undeclared, + 'served by the registered `security` service but neither declared on ISecurityService ' + + '(packages/spec/src/contracts/security-service.ts) nor ledgered in SERVED_NOT_DECLARED with a reason', + ).toEqual([]); + }); + + it('the ledger names only members that are served and not declared', async () => { + const served = servedMembers(await registeredSecurityService()); + for (const [name, reason] of Object.entries(SERVED_NOT_DECLARED)) { + expect(served, `ledger entry '${name}' is not served — delete it`).toContain(name); + expect(Object.hasOwn(DECLARED_MEMBERS, name), `ledger entry '${name}' is declared — delete it`).toBe(false); + expect(reason.trim().length, `ledger entry '${name}' carries no reason`).toBeGreaterThan(0); + } + }); + + it('the two extension members are served with the signatures the contract declares (compile-time)', () => { + // The `Object.assign` half is not typed against the contract, so these two + // witnesses are: each is the contract's member type, implemented by + // delegating to the function the registered member delegates to. A + // parameter the contract hands over that the implementation cannot take, + // or a result the implementation returns that the contract does not + // promise, stops this file compiling. Never invoked. + const discard: NonNullable = (callerContext, id) => + discardPermissionSetOverlay(null as never, callerContext, id); + const contribute: NonNullable = (plugin, alternates) => + new OwnershipFloorAlternates().contribute(plugin, alternates); + expect(typeof discard).toBe('function'); + expect(typeof contribute).toBe('function'); + }); +}); diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index 85f72a9b02d..9830b1eb56b 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -213,8 +213,10 @@ "NotificationMessage (interface)", "NotificationResult (interface)", "ObjectDraft (interface)", + "OwnershipFloorAlternate (interface)", "PendingActionRow (interface)", "PendingActionStatus (type)", + "PermissionSetOverlayDiscardResult (interface)", "PlanUpgradeInput (interface)", "Plugin (interface)", "PluginStartupResult (type)", diff --git a/packages/spec/export-origins/contracts.json b/packages/spec/export-origins/contracts.json index e8c01bdd981..910f1feede4 100644 --- a/packages/spec/export-origins/contracts.json +++ b/packages/spec/export-origins/contracts.json @@ -213,8 +213,10 @@ "NotificationMessage": "src/contracts/notification-service.ts#NotificationMessage (interface)", "NotificationResult": "src/contracts/notification-service.ts#NotificationResult (interface)", "ObjectDraft": "src/contracts/external-datasource-service.ts#ObjectDraft (interface)", + "OwnershipFloorAlternate": "src/contracts/security-service.ts#OwnershipFloorAlternate (interface)", "PendingActionRow": "src/contracts/ai-service.ts#PendingActionRow (interface)", "PendingActionStatus": "src/contracts/ai-service.ts#PendingActionStatus (type)", + "PermissionSetOverlayDiscardResult": "src/contracts/security-service.ts#PermissionSetOverlayDiscardResult (interface)", "PlanUpgradeInput": "src/contracts/package-service.ts#PlanUpgradeInput (interface)", "Plugin": "src/contracts/plugin-validator.ts#Plugin (interface)", "PluginStartupResult": "src/kernel/startup-orchestrator.zod.ts#PluginStartupResult (type)", diff --git a/packages/spec/src/contracts/security-service.test.ts b/packages/spec/src/contracts/security-service.test.ts index 416f82fcee0..1fb98b3a1f6 100644 --- a/packages/spec/src/contracts/security-service.test.ts +++ b/packages/spec/src/contracts/security-service.test.ts @@ -5,6 +5,7 @@ import type { ISecurityService, AuthoredRowWriteVerdict, AuthoredRowWriteOperation, + OwnershipFloorAlternate, } from './security-service'; /** @@ -558,4 +559,87 @@ describe('Security Service Contract', () => { ); expect(seen[0]).toEqual({ object: 'deal', operation: 'update', userId: 'u2', recordId: 'r1' }); }); + + it('discardPermissionSetOverlay is OPTIONAL — absence is typed, and the caller answers it as not implemented', async () => { + // A security service without the operator action still satisfies the + // contract, and a caller cannot reach the action without handling the + // absent case first. + const withoutIt: ISecurityService = makeService(); + expect(typeof withoutIt.discardPermissionSetOverlay).toBe('undefined'); + + // The unguarded call does not compile. Never invoked: its only job is to + // make the COMPILER prove the point. + const mustNotCompileWithoutAGuard = () => + // @ts-expect-error possibly undefined — a caller must feature-detect first + withoutIt.discardPermissionSetOverlay({ userId: 'admin' }, 'ps_1'); + expect(typeof mustNotCompileWithoutAGuard).toBe('function'); + + // The shape the REST route writes: an absent method is a 501, never a + // pretended success. + const route = async (svc: ISecurityService) => + typeof svc.discardPermissionSetOverlay === 'function' + ? { status: 200, data: await svc.discardPermissionSetOverlay({ userId: 'admin' }, 'ps_1') } + : { status: 501, data: undefined }; + await expect(route(withoutIt)).resolves.toEqual({ status: 501, data: undefined }); + + const withIt = makeService({ + discardPermissionSetOverlay: async (_context, id) => ({ + permissionSet: { id, name: 'sales_user' }, + healedObjectGrantCount: 3, + overlaysDiscarded: 1, + }), + }); + await expect(route(withIt)).resolves.toEqual({ + status: 200, + data: { permissionSet: { id: 'ps_1', name: 'sales_user' }, healedObjectGrantCount: 3, overlaysDiscarded: 1 }, + }); + }); + + it('contributeOwnershipFloorAlternates is OPTIONAL — absence is typed, and an alternate names exactly one floor limb', () => { + // A security service without the seam still satisfies the contract. + // Absence leaves the floor in force, which is the fail-closed direction: + // the contributor's wider rule stays unreachable and nothing is widened. + const withoutIt: ISecurityService = makeService(); + expect(typeof withoutIt.contributeOwnershipFloorAlternates).toBe('undefined'); + + const alternate: OwnershipFloorAlternate = { + name: 'sys_attachment_parent_editor_delete', + object: 'sys_attachment', + operation: 'delete', + using: 'id != null', + }; + + // The unguarded call does not compile. Never invoked. + const mustNotCompileWithoutAGuard = () => + // @ts-expect-error possibly undefined — a contributor must feature-detect first + withoutIt.contributeOwnershipFloorAlternates('com.example.plugin', [alternate]); + expect(typeof mustNotCompileWithoutAGuard).toBe('function'); + + // The shape a contributor writes: feature-detect, then contribute; the + // absent branch is a defined outcome rather than a crash. + const contribute = (svc: ISecurityService) => { + if (typeof svc.contributeOwnershipFloorAlternates !== 'function') return 'no-seam'; + svc.contributeOwnershipFloorAlternates('com.example.plugin', [alternate]); + return 'contributed'; + }; + expect(contribute(withoutIt)).toBe('no-seam'); + + const received: unknown[] = []; + const withIt = makeService({ + contributeOwnershipFloorAlternates: (plugin, alternates) => { + received.push([plugin, alternates]); + }, + }); + expect(contribute(withIt)).toBe('contributed'); + expect(received).toEqual([['com.example.plugin', [alternate]]]); + + // The operation is ONE floor limb. `all` would relieve both limbs at once, + // and each limb is its own decision, so the type does not admit it. + const notOneLimb: OwnershipFloorAlternate = { + ...alternate, + // @ts-expect-error `all` is not one floor limb — contribute `update` or `delete` + operation: 'all', + }; + expect(notOneLimb.operation).toBe('all'); + }); }); diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index 2ac73d55d14..98160e061f8 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -237,6 +237,58 @@ export type DelegationNarrowing = delegatorScope: ObjectAccessScope; }; +/** + * What {@link ISecurityService.discardPermissionSetOverlay} resolves with. + * + * The minimal contract shape. The reference implementation's own result type + * (`PermissionSetOverlayDiscardResult` in `@objectstack/plugin-security`) is + * plugin-internal and is assignable to this one; it types the row loosely. + */ +export interface PermissionSetOverlayDiscardResult { + /** + * The `sys_permission_set` row as stored once the overlay is gone and the row + * has been re-projected from the declared artifact. The column set is owned + * by the implementation's backing object rather than this contract, so it + * stays open, as {@link AudienceBindingSuggestion} does. + */ + permissionSet: Record; + /** + * The object grants the row carries NOW, read back after the re-projection. + * + * ⚠️ Not always a healed count. When the store refuses the re-projection + * write, the row keeps its pre-discard grants and this number equals the + * pre-discard count. The call still resolves, because the overlay deletion did + * land; the implementation reports the refused write on its audit log, not + * here. + */ + healedObjectGrantCount: number; + /** How many overlay rows were deleted. At least 1 on any call that resolves. */ + overlaysDiscarded: number; +} + +/** + * One alternate match a plugin contributes to the platform's row-level write + * ownership floor, through + * {@link ISecurityService.contributeOwnershipFloorAlternates}. + * + * The minimal contract shape. The reference implementation's own type of the + * same name (in `@objectstack/plugin-security`) is plugin-internal and + * structurally identical. + */ +export interface OwnershipFloorAlternate { + /** Policy name (snake_case), as `explain` and the denial logs show it. */ + readonly name: string; + /** The ONE object the floor is relieved on: a named object, never `'*'`. */ + readonly object: string; + /** The ONE floor limb relieved: `update` (the edit floor) or `delete`. */ + readonly operation: 'update' | 'delete'; + /** + * The rows of `object` the floor stops answering for on that limb, as a + * row-level security `using` predicate. + */ + readonly using: string; +} + /** * Public contract for the `security` service. * @@ -843,4 +895,88 @@ export interface ISecurityService { callerContext: SecurityContext, id: string, ): Promise<{ suggestion: AudienceBindingSuggestion }>; + + /** + * [ADR-0094] Discard the stale environment-wide `sys_metadata` overlay that + * shadows a PACKAGE-DECLARED permission set, then re-project the + * `sys_permission_set` row from the declared artifact before resolving. This + * is the audited operator action behind the set's "Discard Overlay" Setup + * action (`POST …/security/permission-sets/:id/discard-overlay`). `id` is the + * `sys_permission_set` row id, looked up in the caller's organization. + * + * One set at a time, and never a provenance change: it removes the overlay + * and lets the row converge to its artifact. It does not touch `managed_by` + * or `package_id`. + * + * **Refuses by throwing.** Each refusal carries a registered `code` and an + * HTTP status, which the REST route serves as they are: + * - `PERMISSION_DENIED` (403): the caller is not an authenticated tenant-level + * administrator (a system context bypasses this check), OR no installed + * package currently declares the set. That second refusal protects an + * environment-authored set: discarding its overlay would destroy that work + * with no trace and no way back. + * - `NOT_FOUND` (404): no `sys_permission_set` row has this id. + * - `INVALID_STATE` (409): the set has no active environment-wide overlay to + * discard. + * `NOT_FOUND` and `INVALID_STATE` are listed for this emitter under + * `ERROR_CODE_LEDGER['@objectstack/plugin-security']`. Any other throw is an + * unexpected fault (a failed overlay delete is rethrown as it is). + * + * A refused re-projection write is NOT a refusal: the overlay is already gone, + * so the call resolves. See + * {@link PermissionSetOverlayDiscardResult.healedObjectGrantCount}. + * + * **OPTIONAL, and absence is a defined state, not a bug.** A security service + * without this action omits it. Callers feature-detect + * (`typeof svc.discardPermissionSetOverlay === 'function'`), and the REST route + * answers `501 NOT_IMPLEMENTED` when the method is absent. Declaring it + * optional makes that degradation a property of the type: the unguarded call + * does not compile. + */ + discardPermissionSetOverlay?( + callerContext: SecurityContext, + id: string, + ): Promise; + + /** + * Contribute alternate matches to the platform's row-level write ownership + * floor. The floor is the `created_by == current_user.id` update and delete + * policies `member_default` ships. A plugin that installs a tighter row gate + * for an object calls this, beside that gate, so the parent-blind floor stops + * pre-empting the gate on that object. Where the gate is not installed, + * nobody contributes, and the floor stays the last word. + * + * The contributor says WHAT is relieved: one object, one limb, which rows. + * The implementation decides WHERE. It places each alternate beside every + * ENABLED floor policy of the same limb, in that policy's own `positions` + * domain. So an alternate cannot reach a principal the floor does not bind, + * and where no floor policy of that limb is present, it adds nothing. + * + * Keyed by `plugin`, the contributing plugin's id: a second call REPLACES the + * same plugin's first, and an empty list withdraws it. Synchronous; meant to + * be called at boot. + * + * **Refuses by throwing a plain `Error`**, with no registered `code`: this is + * a boot-time refusal, never a wire answer. A refused call changes nothing, + * and the plugin's earlier contribution stays in force. It refuses when: + * - `plugin` is empty, or `alternates` is not an array; + * - an alternate names no object, or names `'*'` (that would withdraw the + * floor on every object instead of relieving it on one); + * - its `operation` is not exactly one limb, `update` or `delete` (`all` + * would relieve both limbs at once, and each limb is its own decision); + * - it carries no `using` predicate, or it does not parse as a + * `RowLevelSecurityPolicySchema` (snake_case `name`, closed keys). + * + * **OPTIONAL, and absence is a defined state, not a bug.** A security service + * without the seam omits it. Contributors feature-detect + * (`typeof svc.contributeOwnershipFloorAlternates === 'function'`). Absence + * and a refusal both leave the floor in force, which fails CLOSED: the gate's + * wider rule stays unreachable, and nothing is widened. Declaring it optional + * makes that degradation a property of the type: the unguarded call does not + * compile. + */ + contributeOwnershipFloorAlternates?( + plugin: string, + alternates: readonly OwnershipFloorAlternate[], + ): void; }