Skip to content

Commit 38392cb

Browse files
committed
refactor(plugin-security): move the everyone anchor, the explanation, the sharing verdicts and the authored-row write verdict out of security-plugin.ts
The bodies of declareEveryone and declareBuiltinPositions (the everyone anchor's definition and the built-in positions) move unedited to security-plugin-everyone-anchor.ts, with the four import names only they read. The body of explainAccessForCaller moves unedited to security-plugin-explain-access.ts, with the import names only it reads; the two caller-read classifications it shares with the write path stay in security-plugin.ts and are exported. The bodies of the six sharing verdicts (resolveSharingReadFilter, resolveSharingCanEdit, resolveWriteScopeForSharing, resolveSharingWriteVerdict, resolvePreImageFloorDrop, delegatorOwnWriteFloorOptions) move unedited to security-plugin-sharing-verdicts.ts. The body of checkAuthoredRowWrite moves unedited to security-plugin-authored-row-write.ts, with the probe scope constant only it reads. Each method keeps its name, signature, docblock and position and hands its body one host literal of exactly the members it reads. everyoneBaseline and destroy stay whole. The two readers that read the moved lines by path are re-pointed: the system-context page's explain anchor names the new module, and the delegator enumeration pin lists it. Move-only: no verdict changes. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017BoLT8Gaa8t9a7AWVH8qZa
1 parent 05cc3ee commit 38392cb

7 files changed

Lines changed: 1187 additions & 598 deletions

‎content/docs/permissions/system-context.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ that silently does not happen.
107107
| 7 | Write bypass = `true`, effective write scope = `org` | plugin-security | Get: widest write scope without holding any capability | `packages/plugins/plugin-security/src/security-plugin.ts#start` |
108108
| 8 | Metadata-plane schema masking exempt (ADR-0106 D4) | metadata-core | Get: unmasked object schema. Note: the exemption is a **caller** property — it short-circuits before the security service is consulted | `packages/metadata-core/src/object-schema-fls.ts#isObjectSchemaMaskExempt` |
109109
| 8b | Authoring-trail reads admitted | rest, runtime | Get: a metadata item's version history, audit rows and stored-version diff (`GET /meta/:type/:name/history`, `/audit`, `/diff`) without `studio.access` or `manage_metadata`, and a package's commit timeline (`GET /packages/:id/commits`) without `studio.access`. Note: narrower than the schema-mask exemption the draft switches ask (`isObjectSchemaMaskExempt`): neither door admits `setup.access`, which organization admins hold | `packages/rest/src/rest-server.ts#mayReadAuthoringTrail`, `packages/runtime/src/domains/packages.ts#requireCommitTrailReadCapability` |
110-
| 9 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `packages/plugins/plugin-security/src/security-plugin.ts#explainAccessForCaller` |
110+
| 9 | `explain()` may target a principal other than the caller | plugin-security | Get: no `manage_users` / delegated-admin check | `packages/plugins/plugin-security/src/security-plugin-explain-access.ts#explainAccessForCaller` |
111111
| 9b | The master-detail write check the `security` service serves (`checkControlledByParentWrite`, ADR-0055) answers `allow` | plugin-security | Get: `allow` for any record, with no master judged — the write path's own first exit, so a gate composing it admits exactly what the record's own system-context update would. Lose: nothing a caller reaches today: its two consumers, the `sys_attachment` and `sys_comment` parent gates, return before they ask for a system session | `packages/plugins/plugin-security/src/security-plugin-cbp-write-gate.ts#checkControlledByParentWrite` |
112112
| 10 | Anonymous-deny treats the caller as authenticated | core | Get: passes the 401 seam with no `userId` | `packages/core/src/security/anonymous-deny.ts#shouldDenyAnonymous` |
113113
| 12 | Session-resolution middleware skipped | plugin-auth | Get: no session lookup attempted | `packages/plugins/plugin-auth/src/auth-plugin.ts#start` |

‎packages/plugins/plugin-security/src/delegated-organization-membership.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ describe('[#25107] every reader in this package that resolves a delegator asks t
685685
const calls = (text: string, name: string) =>
686686
(text.match(new RegExp(`\\b${name}\\(`, 'g')) ?? []).length - (text.match(new RegExp(`function ${name}\\(`, 'g')) ?? []).length;
687687

688-
it('per source file, the asks equal the resolutions — and the six files that resolve one are all counted', () => {
688+
it('per source file, the asks equal the resolutions — and the seven files that resolve one are all counted', () => {
689689
const counted: Record<string, [number, number]> = {};
690690
for (const file of readdirSync(SRC, { recursive: true }) as string[]) {
691691
if (!file.endsWith('.ts') || file.endsWith('.test.ts') || file.includes('.testkit.')) continue;
@@ -696,6 +696,7 @@ describe('[#25107] every reader in this package that resolves a delegator asks t
696696
}
697697
expect(Object.keys(counted).sort()).toEqual([
698698
'explain-engine.ts',
699+
'security-plugin-explain-access.ts',
699700
'security-plugin-field-readers.ts',
700701
'security-plugin-object-admission.ts',
701702
'security-plugin-operation-principals.ts',
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25223 cut 10] `SecurityPlugin#checkAuthoredRowWrite`, moved out of
5+
* `security-plugin.ts`: the `ISecurityService.checkAuthoredRowWrite` verdict,
6+
* whether an app-authored row policy admits one row for an `update` or a
7+
* `delete` — `admit` or `abstain`, never a refusal, and `abstain` on every
8+
* failure — together with the scope its probe read runs under,
9+
* `AUTHORED_ROW_WRITE_PROBE_CONTEXT`, which nothing else reads.
10+
*
11+
* A MOVE, not a redesign. The function below is the body of the method of the
12+
* same name as it stood, re-indented one level and otherwise unedited: the
13+
* same abstentions in the same order, the same provenance pre-check, the same
14+
* layered predicate and the same id-only probe read. The scope constant and
15+
* its docblock moved with it unedited, still unexported, and the probe still
16+
* spreads it at its one call site.
17+
*
18+
* ## How the moved body reaches the class
19+
*
20+
* The shape cuts 1 to 9 used (`security-plugin-object-inputs.ts`, whose header
21+
* carries the reasoning). The class keeps the public method — the published
22+
* `dist/*.d.ts` lists its name, docblock and position — and its body is one
23+
* literal of exactly the {@link AuthoredRowWriteHost} members it reads, plus a
24+
* `.call`. A method is handed over as a closure that calls it on the plugin
25+
* at call time, so a test double or spy set on the plugin is still the one
26+
* the body reaches: the verdict still asks the plugin's set resolution, RLS
27+
* policy collection and layered RLS filter, and the `security` service
28+
* binding and every other caller still reach the method through the plugin.
29+
* The method was async and the function still is; the method now returns its
30+
* function's promise.
31+
*
32+
* Each field was classified on its own declaration, by the rule cut 1 set.
33+
* `logger` and `ql` are not declared `readonly` (`start()` assigns both after
34+
* construction), so they go as GETTERS, and each read happens when the body
35+
* makes it. Both are objects whose own methods the body calls, so no receiver
36+
* changes.
37+
*
38+
* ## What stays in `security-plugin.ts`, and how it is read
39+
*
40+
* The verdict and operation types are imported from
41+
* `@objectstack/spec/contracts`, the platform-ownership floor predicate from
42+
* `platform-ownership-policies.js`, and the clause predicate from
43+
* `rls-compiler.js`, as `security-plugin.ts` imports them.
44+
*
45+
* Nothing re-exports this module, so the published surface does not move.
46+
*/
47+
import type { AuthoredRowWriteOperation, AuthoredRowWriteVerdict } from '@objectstack/spec/contracts';
48+
import type { SecurityPlugin } from './security-plugin.js';
49+
import { isPlatformOwnershipFloorPolicy } from './platform-ownership-policies.js';
50+
import { policyDeclaresClause } from './rls-compiler.js';
51+
52+
/**
53+
* The `SecurityPlugin` members the function below reads — every one of them,
54+
* and no other; the function's `this` is the `Pick` of the members it reads.
55+
* Each is typed AS the class member (an indexed access on the class), so a
56+
* change to a member's signature on the class is a change here too, and naming
57+
* a member the class does not have is a compile error. See the module header
58+
* for which members are getters.
59+
*/
60+
export interface AuthoredRowWriteHost {
61+
readonly logger: SecurityPlugin['logger'];
62+
readonly ql: SecurityPlugin['ql'];
63+
collectRLSPolicies: SecurityPlugin['collectRLSPolicies'];
64+
computeLayeredRlsFilter: SecurityPlugin['computeLayeredRlsFilter'];
65+
resolvePermissionSetsForContext: SecurityPlugin['resolvePermissionSetsForContext'];
66+
}
67+
68+
/**
69+
* [#7281] The scope `checkAuthoredRowWrite`'s probe read runs under.
70+
*
71+
* The method asks ONE question — "does the declared, app-authored widener admit
72+
* this row for this operation?" — and that question is about the row and the
73+
* declaration, never about what the CALLER may see. Resolving it under the
74+
* caller's own context folded a READ decision into a WRITE question: on a
75+
* `private`-OWD object `plugin-sharing`'s read filter scopes every re-read to
76+
* owner-match OR shares, so a cross-owner row was invisible to the probe and
77+
* the verdict was `abstain` for a row the declaration names by predicate. The
78+
* by-id widener surface was therefore structurally dead on `private` — the
79+
* posture #5493 built it for (maintainer ruling, 2026-08-10: reading 2).
80+
*
81+
* Elevating the READ does not widen the ANSWER, because the predicate carries
82+
* the whole of the question and travels in the `where`, not in the scope:
83+
* `{ id } AND layer0(tenant wall) AND layer1(app-authored policies)`. Both
84+
* layers are computed from the CALLER's permission sets and the CALLER's
85+
* tenant BEFORE the read (see {@link SecurityPlugin.checkAuthoredRowWrite}), so
86+
* a row in another tenant, a row no authored policy matches, and a caller
87+
* holding no authored policy at all are all still `abstain` — measured, not
88+
* asserted (`authored-row-write-verdict.test.ts`, and the real-stack pin
89+
* `packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts`).
90+
*
91+
* Principal-less on purpose: it carries no `userId`, so nothing downstream can
92+
* mistake it for the caller acting with more authority than they hold. It is
93+
* SPREAD at the single call site rather than passed by reference, so no
94+
* middleware can stamp state onto a shared object.
95+
*/
96+
const AUTHORED_ROW_WRITE_PROBE_CONTEXT = { isSystem: true, positions: [], permissions: [] } as const;
97+
98+
/** The body of `SecurityPlugin#checkAuthoredRowWrite`; the method's docblock there is its contract. */
99+
export async function checkAuthoredRowWrite(
100+
this: Pick<AuthoredRowWriteHost, 'collectRLSPolicies' | 'computeLayeredRlsFilter' | 'logger' | 'ql' | 'resolvePermissionSetsForContext'>,
101+
object: string,
102+
recordId: string,
103+
operation: AuthoredRowWriteOperation,
104+
context?: any,
105+
): Promise<AuthoredRowWriteVerdict> {
106+
try {
107+
if (!object || recordId == null || recordId === '') return 'abstain';
108+
if (operation !== 'update' && operation !== 'delete') return 'abstain';
109+
if (!this.ql) return 'abstain';
110+
// No principal, or a delegated identity this path cannot intersect —
111+
// both are "cannot measure", which is `abstain` (never `admit`).
112+
if (!context?.userId) return 'abstain';
113+
if (context?.onBehalfOf?.userId) return 'abstain';
114+
115+
const permissionSets = await this.resolvePermissionSetsForContext(context);
116+
if (permissionSets.length === 0) return 'abstain';
117+
118+
// Cheap provenance pre-check: if the caller holds NO app-authored policy
119+
// applicable to (object, operation), there is nothing that could admit by
120+
// declaration — answer without spending a database round-trip. This is
121+
// the same collection the compiler consumes, filtered by the same
122+
// provenance predicate, so the two cannot disagree about what "authored"
123+
// means.
124+
//
125+
// [#8059] The pre-check asks for an authored policy that ADMITS ROWS —
126+
// one carrying a `using` scope predicate. A `check`-only policy declares
127+
// no row scope at all (ADR-0058 D4: post-image validation), so it can
128+
// admit nothing, and the verdict for a caller holding only check-only
129+
// policies is `abstain` — which is the verdict this method already
130+
// returned before #8059 and must keep returning after it. It got there
131+
// by a route the fix removes: a check-only policy compiles to no `using`
132+
// filter, so Layer 1 was null and the `layer1 == null → abstain` rule
133+
// below fired. Site 1 now DERIVES a Layer 1 from the caller's select
134+
// narrowing in exactly that case, so without this clause the derived
135+
// readable-set scope would start answering `admit` — and `admit` is what
136+
// the by-id widener uses to WIDEN. That is precisely the masquerade the
137+
// doc comment above promises cannot happen (#5493 / #7281), so the
138+
// promise is now kept by an explicit test rather than by the accident of
139+
// a null Layer 1. Nothing narrows here: `abstain` changes nothing on its
140+
// own, and the pre-image gate still refuses on its own terms.
141+
const authored = this.collectRLSPolicies(
142+
permissionSets,
143+
object,
144+
operation,
145+
(context?.positions ?? []) as string[],
146+
context,
147+
).filter((p) => !isPlatformOwnershipFloorPolicy(p) && policyDeclaresClause(p, 'using'));
148+
if (authored.length === 0) return 'abstain';
149+
150+
const { layer0, layer1 } = await this.computeLayeredRlsFilter(
151+
permissionSets,
152+
object,
153+
operation,
154+
context,
155+
{ dropPlatformOwnershipFloor: true },
156+
);
157+
// See the doc above: a null Layer 1 means no authored predicate is
158+
// gating this write, which is an abstention and not an admission.
159+
if (layer1 == null) return 'abstain';
160+
161+
const parts = [{ id: recordId }, ...(layer0 ? [layer0] : []), layer1];
162+
// [#7281] The predicate is the question; the scope is not. Read ELEVATED
163+
// (see AUTHORED_ROW_WRITE_PROBE_CONTEXT) so the answer is decided by
164+
// `{id} AND layer0 AND layer1` alone, and projected to `id` so the probe
165+
// can only ever learn EXISTENCE — no column of a row the caller may not
166+
// read is materialised, let alone returned. The verdict remains evidence,
167+
// never authorization: the by-id write pre-image gate still resolves the
168+
// write under the caller's own context.
169+
const row = await this.ql.findOne(object, {
170+
where: { $and: parts },
171+
fields: ['id'],
172+
context: { ...AUTHORED_ROW_WRITE_PROBE_CONTEXT },
173+
});
174+
return row ? 'admit' : 'abstain';
175+
} catch (e) {
176+
this.logger.warn?.(
177+
`[security] checkAuthoredRowWrite could not resolve an authored-policy verdict for ` +
178+
`'${object}' record '${recordId}' (${operation}, user ${context?.userId ?? 'unknown'}) — ` +
179+
`abstaining (fail-closed: a verdict that cannot be resolved never lifts the sharing refusal)`,
180+
e instanceof Error ? e : new Error(String(e)),
181+
);
182+
return 'abstain';
183+
}
184+
}

0 commit comments

Comments
 (0)