Skip to content

Commit 1659072

Browse files
os-zhuangclaude
andauthored
feat(spec): publish ISecurityService and enforce it at both ends (#3660)
* feat(spec): publish ISecurityService and enforce it at both ends The `security` service registers seven cross-package methods but had no contract in `@objectstack/spec/contracts`. Consumers duck-typed it and each invented its own fallback for a missing method or an "empty" answer. Adds `ISecurityService` and types BOTH ends against it, so the surface is enforced rather than declared: plugin-security assigns its registration to the interface (a renamed, dropped, or re-typed method fails that build), and the REST layer resolves the service as `Partial<ISecurityService>` (call sites must keep feature-detecting rather than assume the full surface). The contract states the one thing consumers cannot guess — the methods do not share a failure convention. `getReadFilter` fails CLOSED (a failure is a deny filter, never `undefined`; `undefined` means "no row restriction" and nothing else). `getReadableFields` fails SOFT, and its two empty answers are opposites: `undefined` is "no answer, use your own projection", `[]` is authoritative "no field is readable". Typing the producer immediately caught one real discrepancy, fixed here: getReadFilter declared `Record<string, unknown> | null | undefined` while every return path yields a filter or `undefined` (`filter ?? undefined` normalizes the null away). Dropping the dead `| null` leaves "no restriction" with exactly one representation. Type-level only. Tests: spec 6665, plugin-security 593, rest 399 — all passing; the three packages build clean (CJS/ESM/DTS). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZ2BGusRo58ZW8FMkDhGTb * docs(kernel): list ISecurityService in the contracts index The contracts index table and the runtime-services source-of-truth list enumerate every published service contract; a new contract missing from them makes the index quietly wrong. Adds the Security Service row and its source path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZ2BGusRo58ZW8FMkDhGTb * chore(spec): regenerate the public API surface snapshot Adding ISecurityService and its companion types exports six new names from `@objectstack/spec/contracts`; the api-surface gate pins the public surface so additions have to be acknowledged. Diff is exactly those six additions — 0 removed, 0 narrowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZ2BGusRo58ZW8FMkDhGTb --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9aa5510 commit 1659072

9 files changed

Lines changed: 364 additions & 6 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-security": patch
4+
"@objectstack/rest": patch
5+
---
6+
7+
feat(spec): publish `ISecurityService` — the `security` service surface becomes an enforced contract
8+
9+
The `security` service registers seven cross-package methods (`getReadFilter`,
10+
`getReadableFields`, `resolvePermissionSetNames`, `explain`, and the three
11+
audience-binding suggestion calls) but had no contract in
12+
`@objectstack/spec/contracts`. Consumers duck-typed it, and each one invented its
13+
own fallback for a missing method or an "empty" answer — with more consumers
14+
arriving, that is a drift surface.
15+
16+
`ISecurityService` now documents the surface, and both ends are typed against it
17+
so it is **enforced rather than declared**: `plugin-security` assigns its
18+
registration to `ISecurityService` (a renamed, dropped, or re-typed method fails
19+
that build), and the REST layer resolves the service as a `Partial<ISecurityService>`
20+
(so call sites must keep feature-detecting instead of assuming the full surface).
21+
22+
The contract makes explicit the one thing consumers cannot guess — that the
23+
methods do **not** share a failure convention:
24+
25+
- `getReadFilter` fails **CLOSED**: a resolution failure yields a deny filter
26+
matching zero rows, never `undefined`. `undefined` means "no row restriction",
27+
and nothing else.
28+
- `getReadableFields` fails **SOFT**: `undefined` means "no answer, use your own
29+
projection", while `[]` is authoritative and means "no field is readable" —
30+
opposite instructions that a consumer must not conflate.
31+
32+
Typing the producer immediately caught one real discrepancy, fixed here:
33+
`getReadFilter` declared `Promise<Record<string, unknown> | null | undefined>`
34+
while every return path yields a filter or `undefined` (`filter ?? undefined`
35+
normalizes the null away). The dead `| null` is removed, so "no restriction" has
36+
exactly one representation. Type-level only — no runtime behaviour changes.

content/docs/kernel/contracts/index.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ Service contracts are **TypeScript interfaces** that define the boundaries betwe
4141
| Contract | Interface | Description |
4242
|:---|:---|:---|
4343
| Auth Service | `IAuthService` | Authentication — login, verify, logout, session management |
44+
| Security Service | `ISecurityService` | Query surface for access decisions — row-level read scope, readable-field projection, effective permission sets, access explanation |
4445
| Sharing Service | `ISharingService` | Record sharing rules and access grants |
4546

4647
### Storage & Caching Contracts

content/docs/kernel/runtime-services/index.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ This chapter documents the runtime `services.*` APIs used in hook/action/flow/pl
3636
Each page links the canonical TypeScript source used to derive signatures.
3737

3838
- Data: `packages/client/src/index.ts`
39+
- Security: `packages/spec/src/contracts/security-service.ts`
3940
- Sharing: `packages/spec/src/contracts/sharing-service.ts`
4041
- Queue: `packages/spec/src/contracts/queue-service.ts`
4142
- Email: `packages/spec/src/contracts/email-service.ts`

packages/plugins/plugin-security/src/security-plugin.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
RLS_MEMBERSHIP_RESOLVER_SERVICE,
5252
RESERVED_RLS_MEMBERSHIP_KEYS,
5353
type IRlsMembershipResolver,
54+
type ISecurityService,
5455
} from '@objectstack/spec/contracts';
5556
import { matchesFilterCondition } from '@objectstack/formula';
5657
import { FieldMasker } from './field-masker.js';
@@ -630,7 +631,11 @@ export class SecurityPlugin implements Plugin {
630631
resolveSets: (context: any) => this.resolvePermissionSetsForContext(context),
631632
logger: ctx.logger,
632633
};
633-
ctx.registerService('security', {
634+
// Typed against the published contract so the registered surface cannot
635+
// drift from what cross-package consumers are promised: a renamed method,
636+
// a dropped one, or a changed return type fails THIS build rather than
637+
// silently degrading a consumer's feature detection at runtime.
638+
const securityService: ISecurityService = {
634639
getReadFilter: (object: string, context?: any) => this.getReadFilter(object, context),
635640
// [#3547] Readable-field projection for a context — the authoritative
636641
// column set for a read-derived export (`export ⊆ list`, #3391).
@@ -651,8 +656,11 @@ export class SecurityPlugin implements Plugin {
651656
// [ADR-0090 D6] First-class access explanation. Same code paths as
652657
// the middleware (resolution/evaluator/RLS compiler) — explained by
653658
// construction. Explaining ANOTHER user requires `manage_users`.
654-
explain: (request: { object: string; operation: string; userId?: string }, callerContext?: any) =>
655-
this.explainAccessForCaller(request, callerContext),
659+
explain: (request, callerContext?: any) =>
660+
this.explainAccessForCaller(
661+
{ ...request, operation: String(request.operation) },
662+
callerContext,
663+
),
656664
// [ADR-0090 D5/D9] Install-time suggestion surface: packages suggest
657665
// audience-anchor bindings; a tenant admin confirms (the binding is
658666
// written under the anchor + delegated-admin gates) or dismisses.
@@ -662,7 +670,8 @@ export class SecurityPlugin implements Plugin {
662670
confirmAudienceBindingSuggestion(suggestionDeps, callerContext, id),
663671
dismissAudienceBindingSuggestion: (callerContext: any, id: string) =>
664672
dismissAudienceBindingSuggestion(suggestionDeps, callerContext, id),
665-
});
673+
};
674+
ctx.registerService('security', securityService);
666675
ctx.logger.info('[security] registered "security" service (getReadFilter, getReadableFields, explain, audience-binding suggestions) — ADR-0021 D-C / ADR-0090 D5/D6/D9 / #3547');
667676
} catch (e) {
668677
ctx.logger.warn?.('[security] failed to register "security" service', {
@@ -2112,7 +2121,7 @@ export class SecurityPlugin implements Plugin {
21122121
async getReadFilter(
21132122
object: string,
21142123
context?: any,
2115-
): Promise<Record<string, unknown> | null | undefined> {
2124+
): Promise<Record<string, unknown> | undefined> {
21162125
// System operations bypass scoping (mirrors the middleware's isSystem skip).
21172126
if (context?.isSystem) return undefined;
21182127
const positions = context?.positions ?? [];

packages/rest/src/rest-server.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpoints
1111
import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api';
1212
import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security';
1313
import type { DroppedFieldsEvent } from '@objectstack/spec/data';
14+
import type { ISecurityService } from '@objectstack/spec/contracts';
1415
import {
1516
resolveEffectiveApiMethods,
1617
isApiOperationAllowed,
@@ -4542,8 +4543,15 @@ export class RestServer {
45424543
* Mirrors the resolver in registerSecurityExplainEndpoints. Returns
45434544
* `undefined` when no security service is reachable (no plugin-security /
45444545
* single-kernel without a provider), so callers degrade gracefully.
4546+
*
4547+
* Typed as a PARTIAL {@link ISecurityService}: an implementation may omit a
4548+
* method it cannot honour, so every call site must keep feature-detecting
4549+
* (`typeof svc.x === 'function'`) rather than assume the full surface.
45454550
*/
4546-
private async resolveSecurityService(environmentId?: string, req?: any): Promise<any | undefined> {
4551+
private async resolveSecurityService(
4552+
environmentId?: string,
4553+
req?: any,
4554+
): Promise<Partial<ISecurityService> | undefined> {
45474555
try {
45484556
const envId = await this.resolveRequestEnvironmentId(environmentId, req);
45494557
if (envId && envId !== 'platform' && this.kernelManager) {

packages/spec/api-surface.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3437,6 +3437,9 @@
34373437
"ApprovalSendBackResult (interface)",
34383438
"ApprovalStatus (type)",
34393439
"AssistantModelMessage (type)",
3440+
"AudienceBindingSuggestion (type)",
3441+
"AudienceBindingSuggestionFilter (interface)",
3442+
"AudienceBindingSuggestionSync (interface)",
34403443
"AuthResult (interface)",
34413444
"AuthSession (interface)",
34423445
"AuthUser (interface)",
@@ -3464,6 +3467,7 @@
34643467
"EmailAttachment (interface)",
34653468
"EmailDeliveryStatus (type)",
34663469
"ExecuteUpgradeInput (interface)",
3470+
"ExplainAccessRequest (interface)",
34673471
"ExportJobDownload (interface)",
34683472
"ExportJobListResult (interface)",
34693473
"FinishReason (type)",
@@ -3520,6 +3524,7 @@
35203524
"ISchemaDiffService (interface)",
35213525
"ISchemaDriver (interface)",
35223526
"ISearchService (interface)",
3527+
"ISecurityService (interface)",
35233528
"ISeedLoaderService (interface)",
35243529
"IServiceRegistry (interface)",
35253530
"IShareLinkService (interface)",
@@ -3623,6 +3628,7 @@
36233628
"SearchHit (interface)",
36243629
"SearchOptions (interface)",
36253630
"SearchResult (interface)",
3631+
"SecurityContext (type)",
36263632
"SendEmailInput (interface)",
36273633
"SendEmailResult (interface)",
36283634
"SendSmsInput (interface)",

packages/spec/src/contracts/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export * from './workflow-service.js';
3434
export * from './export-service.js';
3535
export * from './email-service.js';
3636
export * from './sms-service.js';
37+
export * from './security-service.js';
3738
export * from './sharing-service.js';
3839
export * from './rls-membership-resolver.js';
3940
export * from './share-link-service.js';
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import type { ISecurityService } from './security-service';
5+
6+
/**
7+
* These tests pin the two things a consumer of the `security` service reasons
8+
* about and that a refactor could silently change: the SHAPE of the surface
9+
* (compile-time) and the MEANING of each "empty" answer (runtime). The second
10+
* matters more than it looks — `undefined` and `[]` from getReadableFields are
11+
* opposite instructions, and a consumer that conflates them either leaks a
12+
* column set or blanks one out.
13+
*/
14+
15+
/** Minimal stub implementing the full surface. */
16+
function makeService(overrides: Partial<ISecurityService> = {}): ISecurityService {
17+
return {
18+
getReadFilter: async () => undefined,
19+
getReadableFields: async () => [],
20+
resolvePermissionSetNames: async () => [],
21+
explain: async () => ({}) as any,
22+
listAudienceBindingSuggestions: async () => ({
23+
suggestions: [],
24+
synced: { created: 0, confirmedObserved: 0, pruned: 0 },
25+
}),
26+
confirmAudienceBindingSuggestion: async () => ({ suggestion: {}, bindingCreated: true }),
27+
dismissAudienceBindingSuggestion: async () => ({ suggestion: {} }),
28+
...overrides,
29+
};
30+
}
31+
32+
describe('Security Service Contract', () => {
33+
it('a full implementation satisfies the surface', () => {
34+
const service = makeService();
35+
for (const m of [
36+
'getReadFilter',
37+
'getReadableFields',
38+
'resolvePermissionSetNames',
39+
'explain',
40+
'listAudienceBindingSuggestions',
41+
'confirmAudienceBindingSuggestion',
42+
'dismissAudienceBindingSuggestion',
43+
] as const) {
44+
expect(typeof service[m]).toBe('function');
45+
}
46+
});
47+
48+
it('getReadFilter: undefined means NO row restriction — the only thing it may mean', async () => {
49+
// A deny is expressed as a filter that matches nothing, never as `undefined`,
50+
// so a consumer can safely read `undefined` as "apply no filter".
51+
const open = makeService({ getReadFilter: async () => undefined });
52+
await expect(open.getReadFilter('deal', { userId: 'u1' })).resolves.toBeUndefined();
53+
54+
const denied = makeService({ getReadFilter: async () => ({ id: { $eq: null } }) as any });
55+
await expect(denied.getReadFilter('deal', { userId: 'u1' })).resolves.toBeDefined();
56+
});
57+
58+
it('getReadableFields: undefined (no answer) and [] (nothing readable) are opposite answers', async () => {
59+
const noAnswer = makeService({ getReadableFields: async () => undefined });
60+
// `undefined` → the caller must fall back to its own projection…
61+
await expect(noAnswer.getReadableFields('deal', { userId: 'u1' })).resolves.toBeUndefined();
62+
63+
const nothingReadable = makeService({ getReadableFields: async () => [] });
64+
// …whereas `[]` is authoritative: expose no columns at all.
65+
await expect(nothingReadable.getReadableFields('deal', { userId: 'u1' })).resolves.toEqual([]);
66+
});
67+
68+
it('a system context is a full field-level bypass', async () => {
69+
const service = makeService({
70+
getReadableFields: async (_object, context) =>
71+
context?.isSystem ? ['id', 'name', 'secret'] : ['id', 'name'],
72+
});
73+
await expect(service.getReadableFields('deal', { isSystem: true }))
74+
.resolves.toEqual(['id', 'name', 'secret']);
75+
await expect(service.getReadableFields('deal', { userId: 'u1' }))
76+
.resolves.toEqual(['id', 'name']);
77+
});
78+
79+
it('a partial implementation is feature-detectable rather than wrong', () => {
80+
// Consumers probe (`typeof svc.getReadableFields === 'function'`) so an
81+
// implementation may omit a method it cannot honour and still be usable.
82+
const partial: Partial<ISecurityService> = { getReadFilter: async () => undefined };
83+
expect(typeof partial.getReadFilter).toBe('function');
84+
expect(typeof partial.getReadableFields).toBe('undefined');
85+
});
86+
87+
it('explain accepts a record-scoped request and an explicit target user', async () => {
88+
const seen: unknown[] = [];
89+
const service = makeService({
90+
explain: async (request) => { seen.push(request); return {} as any; },
91+
});
92+
await service.explain(
93+
{ object: 'deal', operation: 'update', userId: 'u2', recordId: 'r1' },
94+
{ userId: 'admin' },
95+
);
96+
expect(seen[0]).toEqual({ object: 'deal', operation: 'update', userId: 'u2', recordId: 'r1' });
97+
});
98+
});

0 commit comments

Comments
 (0)