Skip to content

Commit 3e9c0d8

Browse files
os-steveclaude
andauthored
test(plugin-security): managed-deny floor sees the evaluator's first grant route — allowTransfer (#14137) (#14220)
The independent-property floor (#14029) read the three CRUD write flags and modifyAllRecords, missing the evaluator's FIRST grant route: the direct bit read off OPERATION_TO_PERMISSION (transfer: 'allowTransfer'), ENFORCED today through the insert/update owner_id door (#3004). A transfer-only wildcard default set would never have been required to be a managed-deny target. Extend the floor with a value test (=== true, never key-existence — the bits are present-as-false via .default(false), #14129 first review), sync both exhaustive docblocks to name the route, and pin: positive transfer-only shape, read-only reverse control, dedicated present-as-false pin, and a zero-delta invariance sweep over the seeded sets. Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs Co-authored-by: Claude <noreply@anthropic.com>
1 parent 74a32a2 commit 3e9c0d8

3 files changed

Lines changed: 141 additions & 22 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
---
4+
5+
test(plugin-security): the managed-deny floor now sees the evaluator's first grant route — `allowTransfer` (#14137)
6+
7+
The independent-property floor that derives which seeded default permission
8+
sets MUST be managed-deny targets ("a default set whose `'*'` wildcard grants
9+
a write", pinned in `default-permission-sets.test.ts` and diffed against
10+
`MANAGED_DENY_TARGET_SETS`, #14029) read only the three CRUD write flags plus
11+
`modifyAllRecords`. That missed the evaluator's FIRST grant route — the
12+
direct bit read off `OPERATION_TO_PERMISSION` (`transfer: 'allowTransfer'`),
13+
a real grant ENFORCED today through the insert/update `owner_id` door (#3004).
14+
A future default set shaped `'*': { allowRead: true, allowTransfer: true }`
15+
would have held ownership reassignment on every `managedBy: 'better-auth'`
16+
identity table while tripping neither floor clause, so it was never required
17+
to become a managed-deny target and would have kept its wildcard silently.
18+
19+
The floor now also checks `wc.allowTransfer === true` (a value test, never
20+
key-existence — Zod materialises these bits with `.default(false)`, so they
21+
are present-as-false; #14129 first review), and both exhaustive docblocks
22+
name the first route. Zero behaviour delta today: no existing seeded set
23+
carries a transfer-granting wildcard, every existing set keeps its exact
24+
verdict (pinned), and the runtime deny application is byte-identical — this
25+
hardens a CI-time pin, not the shipped permission surface.

packages/plugins/plugin-security/src/managed-object-write-denies.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,21 @@ export const MANAGED_DENY_ENTRY = {
8282
*
8383
* Holding a write-granting `'*'` wildcard is the FLOOR of membership, not its
8484
* definition: every default set whose wildcard grants any generic write class
85-
* — via the three write flags (`allowCreate`/`allowEdit`/`allowDelete`) OR via
85+
* — via the three write flags (`allowCreate`/`allowEdit`/`allowDelete`), OR
86+
* via `allowTransfer` (#14137), the ownership-reassignment bit the
87+
* evaluator's FIRST route reads directly off `OPERATION_TO_PERMISSION`
88+
* (`transfer: 'allowTransfer'`, `permission-evaluator.ts`; a real grant,
89+
* ENFORCED today through the insert/update `owner_id` door, #3004), OR via
8690
* `modifyAllRecords`, whose super-user bypass grants edit/delete and the
8791
* destructive class by the evaluator's second route (`MODIFY_ALL_WRITE_KEYS`,
8892
* `permission-evaluator.ts`) — must be listed here (or carry a documented
8993
* exclusion below), because the wildcard is what would otherwise grant raw
9094
* writes on a newly-declared identity table — that floor is what
9195
* `default-permission-sets.test.ts` derives independently and diffs against
9296
* this list (#14029), so a future write-granting set that is not added here
93-
* fails a pin instead of silently keeping its wildcard.
97+
* fails a pin instead of silently keeping its wildcard. (The clause list is
98+
* exhaustive over the evaluator's two known grant tables; a census of routes
99+
* beyond them has not been done — #14137 "Not established".)
94100
* Membership is WIDER than the floor: `viewer_readonly`'s wildcard is read-only
95101
* and `member_default` has held none since #5491 — their injected entries are
96102
* belt-and-suspenders and the read grant itself, respectively (see the module

packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts

Lines changed: 108 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -384,26 +384,43 @@ describe('admin_full_access imports the kernel capability declaration unchanged
384384
*/
385385
describe('managed-deny targets — independent-property floor + registry union reaches the derived variant (#14029)', () => {
386386
// Derived from `defaultPermissionSets`, never from the list under test.
387-
// "Grants a write" in the evaluator's own terms: the three CRUD flags OR
388-
// `modifyAllRecords` — the super-user bypass grants edit/delete and the
389-
// destructive class by a second route (`MODIFY_ALL_WRITE_KEYS`,
390-
// `permission-evaluator.ts`), so `'*': { modifyAllRecords: true }` is
391-
// write-granting even with all three CRUD flags false. Value tests
392-
// (`=== true`), not key-existence: Zod materialises the superuser bits with
393-
// `.default(false)` (`permission.zod.ts`), so they are present-as-false.
394-
const writeGrantingWildcardSets: string[] = defaultPermissionSets
395-
.filter((s: any) => {
396-
const wc = s.objects?.['*'];
397-
return (
398-
!!wc &&
399-
(wc.allowCreate === true ||
400-
wc.allowEdit === true ||
401-
wc.allowDelete === true ||
402-
wc.modifyAllRecords === true)
403-
);
404-
})
405-
.map((s) => s.name)
406-
.sort();
387+
// "Grants a write" in the evaluator's own terms — the bits its grant routes
388+
// read (`permission-evaluator.ts`):
389+
// - FIRST route, a direct bit read off `OPERATION_TO_PERMISSION`: the three
390+
// CRUD write flags, plus `allowTransfer` (`transfer: 'allowTransfer'` —
391+
// reassigning `owner_id`; a real grant, ENFORCED today through the
392+
// insert/update owner_id door, #3004), so
393+
// `'*': { allowRead: true, allowTransfer: true }` is write-granting even
394+
// with all three CRUD flags AND `modifyAllRecords` false (#14137);
395+
// - SECOND route, the `modifyAllRecords` super-user bypass, which grants
396+
// edit/delete and the destructive class (`MODIFY_ALL_WRITE_KEYS`), so
397+
// `'*': { modifyAllRecords: true }` is write-granting even with all three
398+
// CRUD flags false.
399+
// (A census of grant routes beyond these two tables has NOT been done —
400+
// #14137 "Not established" — so this clause list is exhaustive over the two
401+
// known routes, not a claim about the whole evaluator.)
402+
// Value tests (`=== true`), not key-existence: Zod materialises these bits
403+
// with `.default(false)` (`permission.zod.ts`), so they are present-as-false
404+
// (#14129 first review; pinned below).
405+
const grantsWildcardWrite = (wc: any): boolean =>
406+
wc.allowCreate === true ||
407+
wc.allowEdit === true ||
408+
wc.allowDelete === true ||
409+
wc.modifyAllRecords === true ||
410+
wc.allowTransfer === true;
411+
412+
// The REAL derivation under pin — also applied to synthetic sets below so
413+
// the floor is testable against shapes no seeded set carries yet.
414+
const deriveWriteGrantingWildcardSets = (sets: readonly any[]): string[] =>
415+
sets
416+
.filter((s: any) => {
417+
const wc = s.objects?.['*'];
418+
return !!wc && grantsWildcardWrite(wc);
419+
})
420+
.map((s) => s.name)
421+
.sort();
422+
423+
const writeGrantingWildcardSets: string[] = deriveWriteGrantingWildcardSets(defaultPermissionSets);
407424

408425
/**
409426
* The one documented exclusion: `admin_full_access` keeps its unqualified
@@ -435,6 +452,77 @@ describe('managed-deny targets — independent-property floor + registry union r
435452
expect([...MANAGED_DENY_TARGET_SETS]).not.toContain('admin_full_access');
436453
});
437454

455+
// ── The floor judged against synthetic wildcards (#14137) ──
456+
// `allowTransfer` is the evaluator's FIRST grant route (a direct bit read
457+
// off `OPERATION_TO_PERMISSION`) and is ENFORCED today through the
458+
// insert/update owner_id door (#3004): a transfer-only wildcard holds
459+
// ownership reassignment on every managed identity table while all three
460+
// CRUD write flags and `modifyAllRecords` are false — visible only in the
461+
// evaluator's grant semantics, never in the flags the older clauses read.
462+
// Every shape below goes through `PermissionSetSchema.parse` first so Zod
463+
// materialises the `.default(false)` bits: the parsed wildcard carries the
464+
// unauthored bits present-as-false, exactly what the seeded sets look like
465+
// to the filter.
466+
describe('the floor sees the evaluator first grant route — allowTransfer (#14137)', () => {
467+
const parseProbeSet = (wildcard: Record<string, boolean>): any =>
468+
PermissionSetSchema.parse({
469+
name: 'synthetic_floor_probe',
470+
label: 'Synthetic floor probe',
471+
objects: { '*': wildcard },
472+
});
473+
474+
it('a transfer-only wildcard is required to be a managed-deny target (the card)', () => {
475+
const probe = parseProbeSet({ allowRead: true, allowTransfer: true });
476+
const wc: any = probe.objects['*'];
477+
// The shape really is the card's: all three CRUD write flags AND
478+
// `modifyAllRecords` are (present-as-)false after parse.
479+
expect(wc.allowCreate).toBe(false);
480+
expect(wc.allowEdit).toBe(false);
481+
expect(wc.allowDelete).toBe(false);
482+
expect(wc.modifyAllRecords).toBe(false);
483+
expect(wc.allowTransfer).toBe(true);
484+
expect(deriveWriteGrantingWildcardSets([probe])).toEqual(['synthetic_floor_probe']);
485+
});
486+
487+
it('reverse control: a read-only wildcard is still NOT required', () => {
488+
const probe = parseProbeSet({ allowRead: true });
489+
expect(deriveWriteGrantingWildcardSets([probe])).toEqual([]);
490+
});
491+
492+
it('present-as-false: an explicit allowTransfer:false wildcard does not trip the floor (value test, not key-existence)', () => {
493+
const probe = parseProbeSet({ allowRead: true, allowTransfer: false });
494+
const wc: any = probe.objects['*'];
495+
// The key IS present after parse — `.default(false)` materialises it —
496+
// so a key-existence rewrite of the floor turns exactly this pin red
497+
// (#14129 first review; not to be re-litigated).
498+
expect('allowTransfer' in wc).toBe(true);
499+
expect(wc.allowTransfer).toBe(false);
500+
expect(deriveWriteGrantingWildcardSets([probe])).toEqual([]);
501+
});
502+
503+
it('invariance: the allowTransfer clause changes no existing seeded set verdict (zero delta today)', () => {
504+
// The pre-#14137 four-clause floor, restated ONLY to diff verdicts
505+
// against: if a future seeded set legitimately relies on the
506+
// `allowTransfer` clause, this pin goes red and the zero-delta claim is
507+
// consciously retired — the same review moment the membership diff
508+
// above forces.
509+
let wildcardsSeen = 0;
510+
for (const s of defaultPermissionSets as any[]) {
511+
const wc = s.objects?.['*'];
512+
if (!wc) continue;
513+
wildcardsSeen += 1;
514+
const preFloor =
515+
wc.allowCreate === true ||
516+
wc.allowEdit === true ||
517+
wc.allowDelete === true ||
518+
wc.modifyAllRecords === true;
519+
expect(grantsWildcardWrite(wc), `verdict drifted for ${s.name}`).toBe(preFloor);
520+
}
521+
// Non-vacuousness: the loop really visited the seeded wildcards.
522+
expect(wildcardsSeen).toBeGreaterThanOrEqual(3);
523+
});
524+
});
525+
438526
// ── The behaviour the membership buys, measured on the REAL derived set ──
439527
// (clones so the module-level instances other tests read stay unmutated;
440528
// the kernel path hands the same objects to the same function in place).

0 commit comments

Comments
 (0)