Skip to content

Commit 3b1ac8a

Browse files
committed
feat(security): split managedBy:'system' into engine-owned vs admin-writable, guard engine-owned writes (#3220)
The `system` bucket conflated two incompatible write policies — engine-owned rows (never user-written) and platform-schema, admin/user-writable data — under one all-false affordance row with no engine enforcement (only better-auth had a write guard). This left a wildcard admin able to raw-write engine-owned rows through the generic data API (ADR-0049 gap). Rather than add a new managedBy enum value (which falls through to fully-editable platform defaults on deployed Console clients), the write policy is now the resolved affordance (resolveCrudAffordances = bucket default + userActions), and "engine-owned" is defined as a system/append-only object granting no write: - Writable set declares userActions{create,edit,delete}: the RBAC link tables, sys_user_preference, sys_approval_delegation, and the messaging config grids (notification preference/subscription/template). Affordance only — the DelegatedAdminGate / RLS / permission sets remain the authz. - Engine-owned objects locked to apiMethods ['get','list'] where absent (jobs, notifications, approvals, record-share, automation-run, mail/settings/secret audit, messaging delivery pipeline). sys_secret is read-locked explicitly (an empty apiMethods array fails open). - sys_import_job stays engine-owned: the REST import route writes job rows isSystem-elevated (attribution preserved via explicit created_by). - New engine write guard (assertEngineOwnedWriteAllowed, plugin-security) fail-closed rejects user-context generic writes keyed off resolved affordances; isSystem/context-less writes bypass. Wired into the security middleware alongside the other data-layer gates. - reconcileManagedApiMethods (objectql registry) now runs for every managed bucket, stripping advertised write verbs the affordances forbid. - /me/permissions clamp (plugin-hono-server) now clamps system/append-only too. ADR-0103. Refs #3220, follows #3222 / ADR-0049 / ADR-0092. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fp9yZxRQ3mb7p4vVwqFXKE
1 parent b67d38c commit 3b1ac8a

38 files changed

Lines changed: 719 additions & 66 deletions
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
"@objectstack/objectql": minor
4+
"@objectstack/plugin-hono-server": minor
5+
"@objectstack/spec": patch
6+
"@objectstack/platform-objects": patch
7+
"@objectstack/plugin-approvals": patch
8+
"@objectstack/service-automation": patch
9+
"@objectstack/service-messaging": patch
10+
"@objectstack/plugin-sharing": patch
11+
"@objectstack/rest": patch
12+
---
13+
14+
**Split the overloaded `managedBy: 'system'` bucket into engine-owned vs. admin-writable, and enforce engine-owned writes (ADR-0103, #3220).** The `system` bucket conflated two incompatible write policies: rows a platform service owns end to end (never user-written), and platform-defined schema whose rows are legitimately admin/user-writable. It carried the same all-false affordance row as `better-auth`/`append-only` but, unlike `better-auth`, had no engine enforcement — a wildcard admin could raw-write these rows through the generic data API (ADR-0049 gap).
15+
16+
Rather than add a new `managedBy` enum value (which would fall through to fully-editable `platform` defaults on already-deployed Console clients), the write policy is now the **resolved affordance** (`resolveCrudAffordances` = bucket default + `userActions`), and *engine-owned* is defined as a `system`/`append-only` object that grants no write:
17+
18+
- **Writable set declares `userActions`** — the RBAC link tables (`sys_user_position`, `sys_user_permission_set`, `sys_position_permission_set`), `sys_user_preference`, `sys_approval_delegation`, and the messaging config grids (`sys_notification_preference` / `…_subscription` / `…_template`) now declare `userActions: { create, edit, delete: true }`. The affordance is a declaration only — the `DelegatedAdminGate` / RLS / permission sets remain the authz.
19+
- **Engine-owned objects locked to reads**`apiMethods: ['get','list']` added where absent (jobs, notifications, approval request/approver/token/action, `sys_record_share`, `sys_automation_run`, mail/settings/secret audit, the messaging delivery pipeline). `sys_secret` is explicitly read-locked (an empty `apiMethods` array fails open).
20+
- **`sys_import_job`** stays engine-owned: the REST import route now writes its job rows `isSystem`-elevated (attribution preserved via the explicit `created_by` stamp) and the object is locked to `['get','list']`.
21+
- **New engine write guard** (`assertEngineOwnedWriteAllowed`, plugin-security) fail-closed rejects user-context generic writes to engine-owned `system`/`append-only` objects, keyed off the resolved affordance; `isSystem` and context-less engine/service writes bypass by construction. Wired into the security middleware alongside the other data-layer gates.
22+
- **`reconcileManagedApiMethods`** (objectql registry) now runs for **every** managed bucket, not just `better-auth`: any advertised write verb an object's resolved affordances forbid is stripped at registration with a warning (the drift backstop, ADR-0049).
23+
- **`/me/permissions` clamp** (plugin-hono-server) now clamps `system`/`append-only` as well as `better-auth`, so the client hint reflects `permission ∩ guard`.
24+
25+
**Potentially breaking:** a downstream/third-party `system` object that advertised generic write verbs relying on today's fail-open behaviour will have those verbs stripped (with a warning) and user-context generic writes to it rejected. Declare `userActions` opening the verbs the object legitimately takes from a user context. `better-auth` keeps plugin-auth's identity write guard unchanged; the row-level `managed_by` provenance vocabulary (ADR-0066) is a different axis and is untouched.
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
# ADR-0103: `managedBy` write policy — split the overloaded `system` bucket, enforce engine-owned writes
2+
3+
- **Status**: Accepted
4+
- **Date**: 2026-07-19
5+
- **Issue**: #3220 (root cause surfaced by the #1591 / #3213 better-auth guard work; safe slice shipped in #3222)
6+
- **Relates to**: ADR-0049 (no unenforced security properties), ADR-0092 (identity
7+
write guard mechanism), ADR-0066 (system-row provenance gate)
8+
9+
## Context
10+
11+
`managedBy` is the object-lifecycle bucket UI clients read to render CRUD
12+
affordances, and the security layer reads to enforce matching write defaults.
13+
Its `system` value had come to mean two **incompatible** things:
14+
15+
1. **Engine-owned** — runtime rows a platform service owns end to end and no user
16+
ever writes directly: `sys_automation_run`, `sys_job` / `sys_job_queue`,
17+
`sys_notification`, the approval engine's request/approver/token rows, the
18+
sharing engine's `sys_record_share`, `sys_setting` / `sys_secret` /
19+
`sys_setting_audit`, metadata history/commit, the messaging delivery pipeline
20+
(`sys_notification_delivery`, `…_receipt`, `http_delivery`). Every write goes
21+
through `isSystem: true` / a `SYSTEM_CTX` service engine, or a context-less
22+
engine call (the messaging service's raw-engine writes, the metadata-protocol
23+
repository's transaction context — neither carries a `userId`).
24+
2. **Platform-schema, admin/user-writable data** — objects whose *schema* is
25+
platform-defined but whose *rows* are legitimately written from a user
26+
context by design:
27+
- the RBAC link tables `sys_user_position`, `sys_user_permission_set`,
28+
`sys_position_permission_set` — governed at write time by the
29+
`DelegatedAdminGate` (ADR-0090 D12), which depends on the write carrying the
30+
real caller (e.g. `suggested-audience-bindings.ts` inserts a direct grant
31+
with `context: callerCtx`, deliberately **not** `isSystem`);
32+
- `sys_user_preference` — a user authors their own preferences (RLS self-grant);
33+
- `sys_approval_delegation` — user-facing "who covers for me" config;
34+
- the messaging config grids surfaced in Setup — `sys_notification_preference`
35+
(a user mutes their own topics), `sys_notification_subscription`,
36+
`sys_notification_template` (admin-authored).
37+
38+
All three restricted buckets (`system`, `append-only`, `better-auth`) collapsed
39+
to one identical all-false affordance row, but only `better-auth` had real
40+
enforcement: an engine write guard (ADR-0092) plus registration-time
41+
`apiMethods` reconciliation. `system` had **neither**. A blast-radius audit
42+
(#3220) disproved the original "just guard `system` wholesale" proposal — a
43+
blanket guard would break delegated administration and self-service — and
44+
narrowed the safe, mechanical slice to two latent holes (`sys_presence`,
45+
`sys_metadata`), which shipped as #3222. This ADR is the root-cause fix: make the
46+
write policy **expressible and enforced** without breaking the writable set.
47+
48+
> **Not to be confused with** the row-level `managed_by` *provenance* column
49+
> (`'platform' | 'package' | 'admin'`) on `sys_sharing_rule` / `sys_position` /
50+
> `sys_capability`, guarded by `assertSystemRowWriteGate` (ADR-0066). That is a
51+
> per-row provenance axis; this ADR is about the per-object schema bucket. The
52+
> shared word is a coincidence.
53+
54+
## Decision
55+
56+
### D1 — The bucket is a default; the *resolved affordance* is the policy
57+
58+
`managedBy` supplies only the **default** CRUD affordance row. The authoritative,
59+
enforced write policy for an object is `resolveCrudAffordances(schema)` — the
60+
bucket default with `userActions` overrides applied. We do **not** add a new
61+
enum value. Instead:
62+
63+
- **Engine-owned** ≔ an object in the `system` or `append-only` bucket whose
64+
resolved affordances grant **no** write (`create`/`edit`/`delete` all false) —
65+
i.e. the bucket default, un-opened.
66+
- **Admin/user-writable** ≔ a `system`/`append-only` object that declares
67+
`userActions` opening the writes it legitimately takes. `userActions` is an
68+
affordance declaration, **not** an authorization: the real authz for these
69+
rows remains the `DelegatedAdminGate` / RLS self-grant / permission sets. The
70+
declaration exists so the affordance matrix, the `apiMethods` reconciliation,
71+
and the engine guard below all agree on what the object permits.
72+
73+
Why not a new enum value: a new `managedBy` string falls through to the
74+
fully-editable `platform` default on every already-deployed Console client
75+
(the UI keeps a hand-maintained closed union and an open-ended type), silently
76+
re-granting New/Edit/Delete on an engine table until each client is updated. The
77+
`userActions` approach changes no wire vocabulary, and every UI surface already
78+
honours `userActions`. Splitting the enum later remains possible as a pure
79+
rename on top of the now-correct affordances.
80+
81+
### D2 — Engine write guard for the engine-owned set
82+
83+
A new `system-write-guard` (plugin-security), modelled on the ADR-0092 identity
84+
guard, fails-closed on any **user-context** write to a `system`/`append-only`
85+
object whose resolved affordances do not grant the corresponding verb. Verb →
86+
affordance: `insert → create`, `update`/`upsert``edit`,
87+
`delete`/`purge`/`restore``delete`, `transfer → edit` (aligned with the
88+
`DelegatedAdminGate` governed-operation set). A write is **user-context** when
89+
`context.userId` is set and `context.isSystem !== true`; `isSystem` and
90+
context-less engine/service writes bypass by construction — exactly the calls the
91+
legitimate engine writers make. It is wired into the security middleware
92+
alongside the other unconditional data-layer gates (package-managed, system-row,
93+
audience-anchor, delegated-admin), after the `isSystem` short-circuit. Denials
94+
raise `PermissionDeniedError` (HTTP 403).
95+
96+
The writable set passes the guard because its `userActions` grant the verb; the
97+
`DelegatedAdminGate` / RLS then adjudicate the principal, unchanged.
98+
99+
### D3 — Generalize `apiMethods` reconciliation beyond `better-auth`
100+
101+
`reconcileManagedApiMethods` (objectql registry) previously ran only for
102+
`better-auth`. It now runs for **every** managed bucket, still purely
103+
affordance-driven: any generic write verb an object advertises in
104+
`enable.apiMethods` that its resolved affordances do not grant is stripped at
105+
registration with a warning, so the HTTP exposure gate answers a clean 405
106+
instead of admitting a write that the engine guard (or permission layer) would
107+
later 403. This makes the contradiction impossible to ship and is the drift
108+
backstop for future `system` objects.
109+
110+
### D4 — Generalize the `/me/permissions` clamp
111+
112+
The hono-server `clampManagedObjectWrites` previously clamped only `better-auth`
113+
objects' advertised affordances. It now clamps `system`/`append-only` too, so the
114+
permissions payload the Console consumes reflects the true (guarded) write
115+
policy.
116+
117+
### D5 — `sys_import_job` writes become attributed system writes
118+
119+
The REST import route created/updated `sys_import_job` under the caller's
120+
context. Those rows are engine-owned (the import worker owns the lifecycle), so
121+
the route now writes them `isSystem`-elevated while preserving `created_by`
122+
attribution, and the object is locked to `apiMethods: ['get', 'list']`. This
123+
keeps the object engine-owned rather than carving a `userActions` exception for
124+
a row users never hand-edit.
125+
126+
## Alternatives considered
127+
128+
- **New `managedBy` enum value** (e.g. `engine`) — rejected: silent
129+
fully-editable fallthrough on deployed clients, plus enum/matrix churn and
130+
~30 re-bucketed declarations across three UI mirrors and ten locale bundles,
131+
for a distinction `userActions` already expresses. Revisitable later as a
132+
rename.
133+
- **`writeVia` capability** for third-party-managed objects — deferred (#1878):
134+
no such object exists today; speculative until a concrete need arises
135+
(explicitly out of scope per #3220).
136+
137+
## Consequences
138+
139+
- `system`/`append-only` objects reject user-context generic writes unless
140+
`userActions` opens them — defence-in-depth for non-REST callers, matching the
141+
HTTP-layer `apiMethods` lock several already carry.
142+
- Third-party or downstream `system` objects that advertised write verbs relying
143+
on today's fail-open behaviour will have those verbs stripped with a warning —
144+
the intended ADR-0049 correction, called out in the release note.
145+
- The taxonomy is now honest: a reader can tell engine-owned from writable by the
146+
resolved affordances, and the guard enforces it.

packages/objectql/src/registry.test.ts

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -723,9 +723,10 @@ describe('applySystemFields', () => {
723723
});
724724

725725
// ==========================================
726-
// reconcileManagedApiMethods — #1591 / ADR-0092
727-
// Registration-time consistency: a better-auth-managed object may not
728-
// advertise generic write verbs it doesn't grant.
726+
// reconcileManagedApiMethods — #1591 / ADR-0092 / ADR-0103
727+
// Registration-time consistency: ANY managed object may not advertise generic
728+
// write verbs its resolved affordances don't grant. Scope generalized from
729+
// better-auth to every managed bucket in ADR-0103.
729730
// ==========================================
730731
describe('reconcileManagedApiMethods', () => {
731732
const managed = (extra: any = {}): any => ({
@@ -773,7 +774,7 @@ describe('reconcileManagedApiMethods', () => {
773774
expect(out.enable.apiMethods).toEqual(['get']);
774775
});
775776

776-
it('leaves non-better-auth objects untouched (platform bucket keeps full CRUD)', () => {
777+
it('leaves platform-bucket objects untouched (grant full CRUD → nothing stripped)', () => {
777778
const warn = vi.fn();
778779
const platform: any = {
779780
name: 'sys_business_unit',
@@ -786,6 +787,58 @@ describe('reconcileManagedApiMethods', () => {
786787
expect(warn).not.toHaveBeenCalled();
787788
});
788789

790+
it('leaves unmanaged objects (no managedBy) untouched', () => {
791+
const warn = vi.fn();
792+
const plain: any = {
793+
name: 'crm_lead',
794+
enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] },
795+
};
796+
const out = reconcileManagedApiMethods(plain, { warn });
797+
expect(out).toBe(plain);
798+
expect(warn).not.toHaveBeenCalled();
799+
});
800+
801+
// ADR-0103 — the scope generalization: engine-owned system/append-only
802+
// objects derive to reads, while the writable set keeps its verbs.
803+
it('strips write verbs from a system-bucket object with no write affordances (engine-owned)', () => {
804+
const warn = vi.fn();
805+
const engineOwned: any = {
806+
name: 'sys_automation_run',
807+
managedBy: 'system',
808+
enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] },
809+
};
810+
const out = reconcileManagedApiMethods(engineOwned, { warn });
811+
expect(out.enable.apiMethods).toEqual(['get', 'list']);
812+
expect(warn).toHaveBeenCalledTimes(1);
813+
expect(warn.mock.calls[0][0]).toContain("managedBy:'system'");
814+
});
815+
816+
it('strips write verbs from an append-only object with no write affordances', () => {
817+
const warn = vi.fn();
818+
const appendOnly: any = {
819+
name: 'sys_email',
820+
managedBy: 'append-only',
821+
enable: { apiEnabled: true, apiMethods: ['get', 'list', 'update'] },
822+
};
823+
const out = reconcileManagedApiMethods(appendOnly, { warn });
824+
expect(out.enable.apiMethods).toEqual(['get', 'list']);
825+
expect(warn).toHaveBeenCalledTimes(1);
826+
});
827+
828+
it('keeps CRUD on a system-bucket object whose userActions open the writes (writable set)', () => {
829+
const warn = vi.fn();
830+
const writable: any = {
831+
name: 'sys_user_position',
832+
managedBy: 'system',
833+
userActions: { create: true, edit: true, delete: true },
834+
enable: { apiEnabled: true, apiMethods: ['get', 'list', 'create', 'update', 'delete'] },
835+
};
836+
const out = reconcileManagedApiMethods(writable, { warn });
837+
expect(out).toBe(writable); // nothing stripped
838+
expect(out.enable.apiMethods).toEqual(['get', 'list', 'create', 'update', 'delete']);
839+
expect(warn).not.toHaveBeenCalled();
840+
});
841+
789842
it('applies on registerObject (the contradiction cannot be stored)', () => {
790843
const reg = new SchemaRegistry({ multiTenant: false });
791844
reg.registerObject(managed(), 'sys', 'sys', 'own');

0 commit comments

Comments
 (0)