Skip to content

Commit 4dc17c4

Browse files
committed
refactor(plugin-security): type HeldScope as what resolveHeldScopes builds
`resolveHeldScopes` already normalises every adminScope flag to a boolean (`!== false` / `=== true`) and the allowlist to a `string[]`, so `HeldScope.scope` is the PARSED shape and now says so. The previous commit had reached for `?? true` / `?? false` at the report boundary instead — correct output, but dead code that reads like a default being applied where none is needed. The authored scope, which really does arrive as raw JSON with any subset of the flags stated, keeps the bare `AdminScope` at the two `parseMaybeJson` sites. Both containment paths there already read it with `!== false` / `=== true`, so the author state was the honest type for them all along — the flip is what made the two shapes distinguishable enough to say which is which. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015bLZKxxUUk4mahNfn3g3Ed
1 parent 9d2da7f commit 4dc17c4

1 file changed

Lines changed: 13 additions & 11 deletions

File tree

packages/plugins/plugin-security/src/delegated-admin-gate.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
*/
3939

4040
import { isGrantActive } from '@objectstack/core';
41-
import type { AdminScope, PermissionSet } from '@objectstack/spec/security';
41+
import type { AdminScope, AdminScopeParsed, PermissionSet } from '@objectstack/spec/security';
4242
import { PermissionDeniedError } from './errors.js';
4343

4444
const SYSTEM_CTX = { isSystem: true } as const;
@@ -121,7 +121,14 @@ export interface DelegableScopeReport {
121121
interface HeldScope {
122122
/** The set that carries the scope (for error messages). */
123123
setName: string;
124-
scope: AdminScope & { assignablePermissionSets: string[] };
124+
/**
125+
* The scope as `resolveHeldScopes` NORMALISES it, not as it was authored:
126+
* every flag there is forced to a boolean (`!== false` / `=== true`) and the
127+
* allowlist to a string[]. So this names the PARSED shape (ADR-0122) — the
128+
* authored one, which arrives as raw JSON and may state none of them, is
129+
* `AdminScope` and appears at the two `parseMaybeJson` sites below.
130+
*/
131+
scope: AdminScopeParsed & { assignablePermissionSets: string[] };
125132
/** Resolved BU ids covered (root + descendants when includeSubtree). Empty = misconfigured → approves nothing. */
126133
subtree: Set<string>;
127134
}
@@ -358,18 +365,13 @@ export class DelegatedAdminGate {
358365
}
359366

360367
const held = await this.resolveHeldScopes(sets);
361-
// The declared defaults are applied HERE because nothing parses these: the
362-
// scope arrives as raw JSON off the row (`parseMaybeJson`), so since ADR-0122
363-
// its type says what that actually is — every flag optional. The report
364-
// promises booleans, so it states `AdminScopeSchema`'s own defaults rather
365-
// than passing `undefined` through a field typed `boolean`.
366368
const scopes = held.map((h) => ({
367369
setName: h.setName,
368370
businessUnit: h.scope.businessUnit,
369-
includeSubtree: h.scope.includeSubtree ?? true,
370-
manageAssignments: h.scope.manageAssignments ?? false,
371-
manageBindings: h.scope.manageBindings ?? false,
372-
authorEnvironmentSets: h.scope.authorEnvironmentSets ?? false,
371+
includeSubtree: h.scope.includeSubtree,
372+
manageAssignments: h.scope.manageAssignments,
373+
manageBindings: h.scope.manageBindings,
374+
authorEnvironmentSets: h.scope.authorEnvironmentSets,
373375
assignablePermissionSets: [...h.scope.assignablePermissionSets],
374376
businessUnitIds: [...h.subtree],
375377
}));

0 commit comments

Comments
 (0)