Skip to content

Commit 1801873

Browse files
committed
fix(plugin-auth): state the Case D refusal cost, pin the org-stamped wrong grant, correct two counts
- changeset: name the family that now gets a refusal where it previously got a grant (an active, correctly-named row with a missing/blank id — including a malformed GLOBAL row beside a well-formed org-scoped one), what it now sees, and that repairing or deleting the row restores the grant with no code change. - the wrong-GRANT pin now RESOLVES an organization via `getTenancy`, so it drives the shape that actually bit: `ps_global` stamped `organization_id: 'org_1'`. Asserted by column, not only by row count. - counts corrected: four guard-clause spellings (not five — the fifth match was `member-role-canonical`'s `raw.join(',')`, a different helper); and this package has 47 `.find(` sites in non-test source, a count that already includes `Array#find` (the earlier "~140" was unmeasured). Also "four local engine ports" -> "three", which is what that sentence names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent 37fa7a2 commit 1801873

2 files changed

Lines changed: 39 additions & 6 deletions

File tree

.changeset/plugin-auth-find-envelope-limbs.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44

55
A self-registration grant is refused, not silently redirected, when a permission-set row is malformed — and the fourteen dead `{ records }` / `{ data }` normalizer limbs behind that code are gone.
66

7-
`plugin-auth` carried fourteen array-or-envelope normalizer blocks of the shape `Array.isArray(x) ? x : x.records ?? []` (thirteen on a `records` limb, one on a `data` limb, five of them written as a guard clause rather than a ternary). All fourteen read the same concrete engine — the `ObjectQL` instance the kernel registers as the `objectql` / `data` service — which answers a bare array on every path, populated or empty. The envelope limb was unreachable code that read as a contract, so the next author writing a defensive normalizer here believed an envelope was possible. The limbs are removed, and the four local engine ports that declared `Promise<unknown>` (`BootProbeEngine`, `DevAdminSeedProbeEngine`, `PhoneSmsTemplateEngine`) now declare the array they always returned.
7+
`plugin-auth` carried fourteen array-or-envelope normalizer blocks of the shape `Array.isArray(x) ? x : x.records ?? []` (thirteen on a `records` limb, one on a `data` limb, four of them written as a guard clause rather than a ternary). All fourteen read the same concrete engine — the `ObjectQL` instance the kernel registers as the `objectql` / `data` service — which answers a bare array on every path, populated or empty. The envelope limb was unreachable code that read as a contract, so the next author writing a defensive normalizer here believed an envelope was possible. The limbs are removed, and the three local engine ports that declared `Promise<unknown>` (`BootProbeEngine`, `DevAdminSeedProbeEngine`, `PhoneSmsTemplateEngine`) now declare the array they always returned.
88

99
The user-visible change is in `settleSelfRegistrationGrant`, which carried the opposite defect. Its candidate filter dropped any permission-set row whose `id` was missing or blank, silently, before choosing which row to grant:
1010

1111
- When the malformed row was the only one, the operator was told `no active sys_permission_set row named 'X' resolves` — false, since an active row named exactly that was present. That report is the only signal this path emits, and nothing retries it.
1212
- When the malformed row was the **organization-scoped** one and a global row also carried the declared name, dropping it let the `organization_id == null` arm match instead, and the self-registrant was granted the **global** permission set their organization never declared — with a success log and no other trace.
1313

1414
`active !== false` remains a selection predicate: a deactivated set still reports the ordinary "does not resolve". A malformed row is no longer a selection at all — the grant is refused and the report names the malformed row, so the ambiguity is surfaced instead of resolved by accident. A well-formed family grants exactly as before.
15+
16+
**Upgrade note — one family now gets a refusal where it previously got a grant.** If a deployment's `sys_permission_set` already contains a row that is active and carries the declared name but whose `id` is missing or blank, self-registration grants against that name now stop and report, including the case where the malformed row is one nobody was relying on: a malformed **global** row sitting alongside a well-formed **organization-scoped** row used to be dropped silently, letting the org row be granted, and is now refused. This is deliberate — the old behaviour could not tell that family apart from the one where the silent drop granted the *wrong* set — and it is fully reversible without a code change: repair or delete the malformed row and the grant proceeds exactly as before. The refusal is loud and names the row, so it is visible rather than something to discover later; nothing is written while it stands.

packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*
77
* ## What was removed, and why a test is owed for it
88
*
9-
* Every block looked like `Array.isArray(x) ? x : x.records ?? []` (five of them
9+
* Every block looked like `Array.isArray(x) ? x : x.records ?? []` (four of them
1010
* in the guard-clause spelling, one on a `data` limb). The envelope limb was
1111
* dead: nothing in this tree ever produced that shape. But "dead" is a claim
1212
* about RUNTIME, and the declared type cannot establish it — `IDataEngine.find`
@@ -35,8 +35,10 @@
3535
* goes red). That is also the reason removal is right rather than merely safe:
3636
* a hook that corrupted `find()` into `{ records }` would be a contract
3737
* violation, and the limb did not repair it — it silently absorbed it at these
38-
* fourteen sites while the ~140 other `find()` call sites in this package broke
39-
* anyway. Fourteen sites of false immunity is worse than one visible failure.
38+
* fourteen sites while this package's remaining `find()` call sites broke anyway
39+
* (47 `.find(` sites in its non-test source in total, a count that already
40+
* includes `Array#find`). Fourteen sites of false immunity is worse than one
41+
* visible failure.
4042
*
4143
* ## The fifteenth case is a different defect
4244
*
@@ -398,11 +400,23 @@ describe('#15597 — the blocks driven through their real production entry point
398400
describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row instead of dropping it', () => {
399401
const USER = { id: 'usr_new', name: 'New Person', email: 'new@corp.example' };
400402

401-
async function settle(engine: ObjectQL, setName: string, lines: string[]): Promise<void> {
403+
async function settle(
404+
engine: ObjectQL,
405+
setName: string,
406+
lines: string[],
407+
/**
408+
* The organization `settleSelfRegistrationGrant` resolves, read the way the
409+
* method itself reads it — `this.config.getTenancy?.()` then `defaultOrgId()`.
410+
* Omitted, nothing resolves and `organizationId` stays null; supplied, the
411+
* org-scoped arm of the row selection is the one that runs.
412+
*/
413+
orgId?: string,
414+
): Promise<void> {
402415
const manager = new AuthManager({
403416
secret: SECRET,
404417
baseUrl: 'http://localhost:3000',
405418
dataEngine: engine as never,
419+
...(orgId ? { getTenancy: () => ({ defaultOrgId: async () => orgId }) } : {}),
406420
logger: { error: (m: string) => lines.push(String(m)), warn: (m: string) => lines.push(String(m)), info: () => {} },
407421
} as never);
408422
(manager as never as { stageSelfRegistrationGrant: Function }).stageSelfRegistrationGrant(USER.email, setName);
@@ -450,14 +464,22 @@ describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row inst
450464
// dropped the org row, `rows.find((r) => r.organization_id == null)` then
451465
// matched the GLOBAL row, and the self-registrant was granted a permission
452466
// set their organization never declared — silently, with a success log.
467+
//
468+
// The organization is RESOLVED here (`getTenancy`), which is the shape that
469+
// actually bit and the reason this case does not lean on its sibling: with
470+
// an org in hand the wrong grant is not merely "some global row", it is
471+
// `ps_global` STAMPED `organization_id: 'org_1'` — the write spreads the
472+
// resolved org onto the row — so the store ends up asserting that org_1
473+
// granted a set org_1 never declared. Asserted by column below, not just by
474+
// row count, so the stamp itself is pinned.
453475
const engine = await bootEngine();
454476
await engine.insert('sys_user', USER, SYSTEM);
455477
await engine.insert('sys_organization', { id: 'org_1', name: 'Default', slug: 'default' }, SYSTEM);
456478
await engine.insert('sys_permission_set', { id: '', name: 'portal_user', label: 'Org scoped', organization_id: 'org_1' }, SYSTEM);
457479
await engine.insert('sys_permission_set', { id: 'ps_global', name: 'portal_user', label: 'Global' }, SYSTEM);
458480

459481
const lines: string[] = [];
460-
await settle(engine, 'portal_user', lines);
482+
await settle(engine, 'portal_user', lines, 'org_1');
461483

462484
// Reverse-verified by ablation (recorded because the pre-fix behaviour is
463485
// the whole argument): with the old trailing filter restored and nothing
@@ -471,6 +493,15 @@ describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row inst
471493
// Refused, and in particular NOT granted the global set.
472494
const rows = await grants(engine);
473495
expect(rows, `granted anyway: ${JSON.stringify(rows)}`).toEqual([]);
496+
// The specific wrong write, named: no row may claim org_1 granted ps_global.
497+
expect(
498+
rows.some(
499+
(r) =>
500+
(r as { permission_set_id?: unknown }).permission_set_id === 'ps_global' &&
501+
(r as { organization_id?: unknown }).organization_id === 'org_1',
502+
),
503+
'ps_global was granted STAMPED with org_1 — the organization is now recorded as having granted a set it never declared',
504+
).toBe(false);
474505
expect(lines.join('\n')).toContain('no usable id');
475506
});
476507

0 commit comments

Comments
 (0)