Skip to content

Commit fc6cfff

Browse files
committed
test(dogfood),spec,changeset: name the two derived surfaces the update grant moves (#15873 contract-review patch round)
Review FAIL was on the accept-set statement, not the code: granting `update` also derives the update-mode import door (API_METHOD_DERIVATION: import = any of create/update) and flips /auth/me/permissions for sys_organization (allowEdit true, apiOperations gains update and import). Both named in the changeset; pinned on the real door in the dogfood file (import: 200, timezone lands, name stripped; better-auth-only row refused per row; insert mode 405 naming create; /me/permissions allowEdit true with update+import and a sibling better-auth table as the clamp control); the SINGLE_RECORD_WRITE_ONLY sentence now says what is true (bulk not granted; the derived import door is, column-clamped per row). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
1 parent f0b8c0d commit fc6cfff

3 files changed

Lines changed: 149 additions & 10 deletions

File tree

.changeset/organization-update-door.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66

77
The organization table carries four platform-owned columns better-auth never reads or writes — `require_mfa` (ADR-0069 D3), `parent_organization_id` and `sort_order` (ADR-0105 D6), `timezone` (#14238). plugin-auth declares them generically editable (`MANAGED_EXTENSION_EDITABLE_FIELDS.sys_organization`, the guard's per-object update whitelist), while the object's `enable.apiMethods: ['get', 'list']` answered every `PATCH /api/v1/data/sys_organization/:id` with 405 `OBJECT_API_METHOD_NOT_ALLOWED` before the engine — and the guard — was reached. Declared editable, reachable from no product surface: the columns could be set only by a system-context caller. The ruling answers the card's question — yes, an administrator sets these columns through the product — and refuses the alternative of declaring them system-writable only.
88

9-
What widens (Clause ②): the accept set of the published door. `enable.apiMethods` becomes `['get', 'list', 'update']`, and `userActions: { edit: true }` declares the affordance ADR-0103 D3's `reconcileManagedApiMethods` requires before it lets a `managedBy` object keep a write verb at registration (without it the verb is stripped with a warning and the door keeps answering 405 — the second silent gate #7727 measured on `sys_api_key`). `update` alone: `create` / `delete` still answer 405, and `bulk` is not granted (recorded in `SINGLE_RECORD_WRITE_ONLY`).
9+
What widens (Clause ②) — three published surfaces move, all column-clamped by the same guard:
10+
11+
1. The data door's accept set. `enable.apiMethods` becomes `['get', 'list', 'update']`, and `userActions: { edit: true }` declares the affordance ADR-0103 D3's `reconcileManagedApiMethods` requires before it lets a `managedBy` object keep a write verb at registration (without it the verb is stripped with a warning and the door keeps answering 405 — the second silent gate #7727 measured on `sys_api_key`). `PATCH /api/v1/data/sys_organization/:id` is admitted; `create` / `delete` still answer 405; `bulk` (`/batch`, the `*Many` routes) is not granted (recorded in `SINGLE_RECORD_WRITE_ONLY`).
12+
2. The derived `import` door. `API_METHOD_DERIVATION` (`@objectstack/spec` `api-derivation.ts`) derives `import` from `any: ['create', 'update']`, so granting `update` admits `POST /api/v1/data/sys_organization/import` (and the async `/import/jobs` route) in `writeMode: 'update'` — one request updates N rows, each row clamped per row by the ADR-0092 D2 guard under the caller's context (a row carrying only better-auth columns is refused `PERMISSION_DENIED`; `treatAsHistorical` does not elevate). Insert-mode and upsert-mode import stay 405 (the conjunct named is `create`). The door's own 405 envelope advertises the derived set in `allowed`.
13+
3. `/auth/me/permissions`. For a principal the permission layer already admits (the seeded platform admin's `admin_full_access` wildcard), `sys_organization.allowEdit` goes `false → true` (`clampManagedObjectWrites` reads `userActions.edit` for the `better-auth` bucket) and `apiOperations` gains `update` and `import` (`annotateEffectiveApiOperations`) — the payload the console renders its edit affordance from. `organization_admin` / `member_default` stay hard-denied on every better-auth table by `managed-object-write-denies.ts`, unchanged.
1014

1115
What does not widen: the column set. The guard clamps every user-context update on this table to the whitelist. A PATCH of a better-auth column sent alone (`name`, `slug`, `logo`, `metadata`) is now refused by the guard's own verdict — 403 `PERMISSION_DENIED` — instead of the method gate's 405; sent beside a whitelisted column it is stripped and the whitelisted column lands. better-auth's own columns keep changing through better-auth's `organization/update` (the `update_organization` row action, unchanged). Per ADR-0092 D4's form-rendering constraint the four better-auth columns are now `readonly: true` on the object, so a standard edit form offers exactly what the guard admits; the engine's static-readonly strip exempts system-context writers, so better-auth's adapter is unaffected.
1216

packages/qa/dogfood/test/organization-update-door.dogfood.test.ts

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,33 @@
4040
*
4141
* Refusal cases assert `code` AND `status` (ADR-0112): a status-only assertion
4242
* stays green against an implementation that answers the wrong refusal.
43+
*
44+
* ## Two more published surfaces move with the verb, and are pinned here too
45+
*
46+
* The contract review of PR #16687 measured what the first round did not name:
47+
*
48+
* - the DERIVED `import` door. `API_METHOD_DERIVATION` (`@objectstack/spec`,
49+
* `api-derivation.ts`) derives `import` from `any: ['create', 'update']`, so
50+
* granting `update` admits `POST /data/sys_organization/import` in
51+
* `writeMode: 'update'` — one request updates N rows. It is column-safe for
52+
* the same reason the PATCH is: the import runner writes each row under the
53+
* caller's context, so the D2 guard clamps every row (`timezone` lands,
54+
* `name` is stripped; a row carrying only better-auth columns is refused
55+
* per row; `treatAsHistorical` does not elevate). Insert / upsert modes
56+
* stay 405, and the conjunct the envelope names is `create`;
57+
* - `/auth/me/permissions`, the payload the console renders its edit
58+
* affordance from: `clampManagedObjectWrites` reads `userActions.edit` for
59+
* the `better-auth` bucket and `annotateEffectiveApiOperations` reports the
60+
* effective operation set, so for a principal the permission layer already
61+
* admits, `sys_organization.allowEdit` goes false → true and
62+
* `apiOperations` gains `update` and `import`.
63+
*
64+
* ⚠️ Instrument note for anything `reconcileManagedApiMethods` touches: it runs
65+
* at REGISTRATION, not at build, so a property-read of `dist/` cannot see it —
66+
* with `userActions` removed, `dist` still says `["get","list","update"]` while
67+
* the registered schema says `["get","list"]`. The registered-schema pin at the
68+
* bottom of this file, and the `/me/permissions` pin, are the instruments that
69+
* can.
4370
*/
4471

4572
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
@@ -191,6 +218,106 @@ describe('#15873: sys_organization platform-owned columns through PATCH /data/sy
191218
expect(deletedBody.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED');
192219
});
193220

221+
it('the payload the console consumes: /auth/me/permissions says sys_organization is editable, with update and import in apiOperations', async () => {
222+
const res = await stack.apiAs(token, 'GET', '/auth/me/permissions');
223+
expect(res.status).toBe(200);
224+
const body: any = await res.json();
225+
const entry = body.objects?.sys_organization;
226+
expect(entry, 'sys_organization entry present in /me/permissions').toBeTruthy();
227+
228+
// `clampManagedObjectWrites` — the `better-auth` bucket is clamped to its
229+
// `userActions`; `edit` is the one opened, `create` / `delete` stay off.
230+
expect(entry.allowEdit).toBe(true);
231+
expect(entry.allowCreate).toBe(false);
232+
expect(entry.allowDelete).toBe(false);
233+
234+
// `annotateEffectiveApiOperations` — the effective set the console renders:
235+
// the ruled verb and the door it derives, never the ones not granted.
236+
expect(entry.apiOperations).toContain('update');
237+
expect(entry.apiOperations).toContain('import');
238+
expect(entry.apiOperations).not.toContain('create');
239+
expect(entry.apiOperations).not.toContain('delete');
240+
expect(entry.apiOperations).not.toContain('bulk');
241+
242+
// Control: the clamp is live, not a wildcard fold reporting everything
243+
// editable — a sibling better-auth table with no `userActions` stays
244+
// `allowEdit: false` for the very same principal.
245+
const control = body.objects?.sys_member;
246+
expect(control, 'sys_member entry present (control)').toBeTruthy();
247+
expect(control.allowEdit).toBe(false);
248+
expect(control.apiOperations ?? []).not.toContain('update');
249+
});
250+
251+
it('the derived import door is open in update mode, and the guard clamps every row: timezone lands, name is stripped', async () => {
252+
const before = await readOrg(orgId);
253+
254+
const res = await stack.apiAs(token, 'POST', '/data/sys_organization/import', {
255+
format: 'json',
256+
writeMode: 'update',
257+
matchFields: ['id'],
258+
rows: [{ id: orgId, name: 'Imported Name', timezone: 'Asia/Tokyo' }],
259+
});
260+
expect(res.status).toBe(200);
261+
const body: any = await res.json();
262+
expect(body.writeMode).toBe('update');
263+
expect(body.updated).toBe(1);
264+
expect(body.errors).toBe(0);
265+
266+
const after = await readOrg(orgId);
267+
// The door is open: the whitelisted column landed through import…
268+
expect(after.timezone).toBe('Asia/Tokyo');
269+
// …and the row was written under the caller's context, not as system:
270+
// `name` did not land. Measured (ablation, contract-review patch round):
271+
// with `name` added to the guard's whitelist this assertion STAYS green,
272+
// because `name` is `readonly` (ADR-0092 D4) and the engine's
273+
// static-readonly strip — after the guard, non-system callers only — holds
274+
// it too. Two layers, one observable. The guard-SPECIFIC control on the
275+
// import path is the next pin (a better-auth-only row is refused per row):
276+
// under the same cut it goes red. What THIS assertion fails on is the
277+
// runner elevating rows to system context, which exempts both layers.
278+
expect(after.name).toBe(before.name);
279+
expect(after.name).not.toBe('Imported Name');
280+
});
281+
282+
it('import: a row carrying only better-auth columns is refused per row, PERMISSION_DENIED — treatAsHistorical does not elevate', async () => {
283+
const before = await readOrg(orgId);
284+
const res = await stack.apiAs(token, 'POST', '/data/sys_organization/import', {
285+
format: 'json',
286+
writeMode: 'update',
287+
matchFields: ['id'],
288+
treatAsHistorical: true,
289+
rows: [{ id: orgId, name: 'Imported Name 2' }],
290+
});
291+
// The import route's contract is a per-row outcome report: the request is
292+
// answered 200 and the refusal lives on the row. This is the guard's own
293+
// verdict on the import path (measured red the moment the whitelist admits
294+
// `name`), the same way the name-only PATCH pin above is on the PATCH path.
295+
expect(res.status).toBe(200);
296+
const body: any = await res.json();
297+
expect(body.updated).toBe(0);
298+
expect(body.results?.[0]?.ok).toBe(false);
299+
expect(body.results?.[0]?.code).toBe('PERMISSION_DENIED');
300+
expect((await readOrg(orgId)).name).toBe(before.name);
301+
});
302+
303+
it('import: insert mode is still refused at the method gate — 405, and the conjunct named is create', async () => {
304+
const res = await stack.apiAs(token, 'POST', '/data/sys_organization/import', {
305+
format: 'json',
306+
writeMode: 'insert',
307+
rows: [{ name: 'Forged Via Import', slug: 'forged-via-import' }],
308+
});
309+
expect(res.status).toBe(405);
310+
const body: any = await res.json();
311+
expect(body.code).toBe('OBJECT_API_METHOD_NOT_ALLOWED');
312+
// `deniedConjunctName` names the primitive that actually failed: import
313+
// in insert mode needs `create`, which stays off.
314+
expect(String(body.error)).toContain("'create'");
315+
// …and the same envelope advertises the derived door the ruling opened.
316+
expect(body.allowed).toContain('update');
317+
expect(body.allowed).toContain('import');
318+
expect(body.allowed).not.toContain('create');
319+
});
320+
194321
it('the REGISTERED schema serves `update` (post-reconcile) and better-auth keeps its own door', async () => {
195322
// The original defect was a DECLARATION disagreeing with the runtime, so
196323
// pin what the runtime actually serves. `reconcileManagedApiMethods`

packages/spec/src/data/api-methods-batch-conformance.test.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,18 @@ const SINGLE_RECORD_WRITE_ONLY: Record<string, string> = {
113113
// verbatim 「同意」): the data door admits `update` so an administrator can set
114114
// the four platform-owned columns (`require_mfa`, `parent_organization_id`,
115115
// `sort_order`, `timezone`) the ADR-0092 D2 whitelist already admitted on
116-
// the engine path. The ruling widened ONE verb on an identity table, and
116+
// the engine path. The ruling named ONE verb on an identity table, and
117117
// `bulk` is a second widening it did not take: granting it would open
118118
// `POST /data/sys_organization/batch` and the `*Many` routes to every API
119-
// client. The object's one list view (`all_orgs`) declares no `bulkActions`
120-
// / selection, and the implicit bulk-delete entry gates on the `delete`
119+
// client. What `update` DOES derive is admitted, and named: `import` is
120+
// `any: ['create', 'update']` in `API_METHOD_DERIVATION`, so update-mode
121+
// `POST /data/sys_organization/import` now passes the method gate and
122+
// updates N rows in one request — each row clamped to the D2 whitelist under
123+
// the caller's context, insert/upsert modes still 405. That is not the batch
124+
// shape this ledger is about (`bulk` gates `/batch` and `*Many`, `import`
125+
// does not read it), which is why the exemption stands beside it. The
126+
// object's one list view (`all_orgs`) declares no `bulkActions` /
127+
// selection, and the implicit bulk-delete entry gates on the `delete`
121128
// affordance — off three times over (`managedBy: 'better-auth'` denies by
122129
// default, `userActions` opens `edit` alone, `delete` is not in
123130
// `apiMethods`) — so there is no multi-select to batch today. The cost the
@@ -128,12 +135,13 @@ const SINGLE_RECORD_WRITE_ONLY: Record<string, string> = {
128135
// this entry and add `'bulk'`; the stale-entry test below refuses to let
129136
// both stand.
130137
sys_organization:
131-
'Administrators set the platform-owned columns one organization at a time ' +
132-
'(#15873 ruled `update` alone, column-gated by ADR-0092 D2). No console ' +
133-
'surface multi-selects organizations — the list view declares no bulk ' +
134-
'actions and the object grants no delete affordance — and a promoted bulk ' +
135-
'edit would fan out per row through the action runner rather than hitting ' +
136-
'/batch (#7802).',
138+
'Administrators set the platform-owned columns through single-record PATCH ' +
139+
'and the derived update-mode import door (#15873 ruled `update`; both are ' +
140+
'column-clamped per row by ADR-0092 D2). `bulk` — /batch and the *Many ' +
141+
'routes — is not granted: no console surface multi-selects organizations ' +
142+
'(the list view declares no bulk actions and the object grants no delete ' +
143+
'affordance), and a promoted bulk edit would fan out per row through the ' +
144+
'action runner rather than hitting /batch (#7802).',
137145
};
138146

139147
/** Every `*.object.ts` under `packages/`, skipping build output and deps. */

0 commit comments

Comments
 (0)