Skip to content

Commit 7286dd5

Browse files
os-salesclaude
andauthored
fix(plugin-security): promote the oldest human that can authenticate, not the oldest directory row (#14348) (#14532)
* wip(plugin-security): promote the oldest authenticable human at bootstrap Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * wip(plugin-security): triage the single-posture fixtures onto the account declaration Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * chore(plugin-security): changeset for the authenticable-first-user promotion Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * test(plugin-auth): repoint the human-predicate pin's probe past the promotion conjunct (#14348) The pin reads plugin-security's `isHumanUser` verdict indirectly, as `bootstrapPlatformAdmin`'s `adminPromoted`. Promotion is now a conjunction — human AND holds a `sys_account` — so an empty account table reported a predicate disagreement that does not exist. Model an account for every corpus row that can key one, and handle the id-less row explicitly: both predicates still call it human, promotion refuses it, and the refusal is proven to come from the authenticable filter. Neither predicate is edited. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4e9caf3 commit 7286dd5

5 files changed

Lines changed: 678 additions & 18 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
Fix: the platform-admin promotion targets the oldest human that can SIGN IN, not the oldest `sys_user` row
6+
7+
Under the `single` posture the first-boot promotion ranked candidates by age
8+
alone, and "human" was its only filter. On an app that declares people in
9+
`defineStack({ data })` that picked the wrong row every time: a declared person
10+
is a credential-less directory row, the declarative seed is awaited inside
11+
`AppPlugin.start()` (kernel Phase 2), so those rows are always older than any
12+
account created at `kernel:ready` or later.
13+
14+
Measured on a driven composed boot, not inferred: `admin_full_access` was
15+
granted to `person0@demo.example` — a row with no `sys_account`, on a database
16+
whose `sys_account` table was entirely empty — and `claimSeedOwnership` handed
17+
that same unusable row both seeded business records. A real sign-up arriving
18+
afterwards was never promoted, because the promotion had already short-circuited
19+
on "an admin exists". The grant was written, unexercisable, and permanent.
20+
21+
The target is now the oldest human holding a `sys_account`. Any provider counts:
22+
a federated or SSO account is a login, and narrowing to `credential` would
23+
recreate this defect for SSO-only deployments. When human rows exist but none can
24+
authenticate, nobody is promoted and no grant row is written — an `info` line
25+
says so, and the bootstrap replay now also fires on `sys_account` inserts, so the
26+
first real login is promoted the moment it exists. That second half is
27+
load-bearing rather than incidental: a sign-up writes its `sys_user` row before
28+
its `sys_account` row, so the pre-existing `sys_user` trigger fires while the
29+
registrant still has no login.
30+
31+
Deployments that already carry a platform-admin grant are untouched. The
32+
"an admin already exists" short-circuit runs before any target selection, so this
33+
changes which row a FRESH bootstrap promotes and nothing else — moving an
34+
already-granted platform admin is not this change's to make.

packages/plugins/plugin-auth/src/human-user-predicate-agreement.pin.test.ts

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,25 @@ function makeQl(userRows: unknown[]) {
114114
sys_permission_set: [],
115115
sys_user: userRows.map((r) => (r && typeof r === 'object' ? { ...(r as object) } : r)) as any[],
116116
sys_user_permission_set: [],
117+
// [#14348] Every probed row that CAN hold an account gets one.
118+
//
119+
// This probe reads plugin-security's human verdict indirectly, as
120+
// `report.adminPromoted`, and since #14348 promotion is a conjunction:
121+
// human AND holds a `sys_account` (a login). Leaving this table empty would
122+
// make every row fail the second conjunct, so the probe would report
123+
// "non-human" for rows both owners call human — a disagreement that is not
124+
// there. Modelling the account keeps the HUMAN PREDICATE the only
125+
// discriminator, which is what this file measures.
126+
//
127+
// Rows with no usable `id` get no account, because nothing could key one to
128+
// them; that class is handled explicitly below rather than silently.
129+
sys_account: userRows
130+
.filter((r) => !!r && typeof r === 'object' && (r as any).id !== undefined && (r as any).id !== null)
131+
.map((r) => ({
132+
id: `acc_${String((r as any).id)}`,
133+
user_id: (r as any).id,
134+
provider_id: 'credential',
135+
})),
117136
};
118137
return {
119138
tables,
@@ -141,6 +160,21 @@ const ADMIN_SET = { name: 'admin_full_access', label: 'Administrator' } as any;
141160
/**
142161
* plugin-security's verdict on a single row, read through the published
143162
* `bootstrapPlatformAdmin` entry point.
163+
*
164+
* ⚠️ [#14348] This is a PROXY, and it now carries more than the human
165+
* predicate. `adminPromoted` means "human AND holds a `sys_account`", because
166+
* the `single`-posture promotion moved off "the oldest human row" and onto "the
167+
* oldest human that can authenticate" — a directory row seeded through
168+
* `defineStack({ data })` is older than any account, so the old rule granted
169+
* platform admin to a row nobody can sign in as.
170+
*
171+
* `makeQl` therefore models an account for every row that can key one, which
172+
* holds the second conjunct constant and leaves the human predicate as the only
173+
* discriminator this file measures. `isHumanUser` itself is UNCHANGED by
174+
* #14348, and so is `isHumanUserRow`; nothing about the invariant moved.
175+
*
176+
* ⛔ Do not "simplify" this by dropping the account modelling: the tests would
177+
* go red reporting a predicate disagreement that does not exist.
144178
*/
145179
async function securityVerdict(row: unknown): Promise<{ human: boolean; reason?: string }> {
146180
const ql = makeQl([row]);
@@ -152,7 +186,7 @@ async function securityVerdict(row: unknown): Promise<{ human: boolean; reason?:
152186
* The shared corpus. Every entry is a shape a `sys_user` read can really
153187
* return, and each names the property it is here to hold.
154188
*/
155-
const CORPUS: { name: string; row: unknown }[] = [
189+
const CORPUS: { name: string; row: unknown; idLessFailClosed?: true }[] = [
156190
{
157191
name: 'an ordinary human account',
158192
row: { id: 'usr_alice', role: 'member', email: 'alice@example.test' },
@@ -190,8 +224,11 @@ const CORPUS: { name: string; row: unknown }[] = [
190224
row: { id: `${SystemUserId.SYSTEM}_2`, role: 'member', email: 'frank@example.test' },
191225
},
192226
{
227+
// [#14348] Human to BOTH predicates, and deliberately NOT probed through
228+
// promotion — see the dedicated branch in the agreement loop below.
193229
name: 'a row with neither id nor role',
194230
row: { email: 'ghost@example.test' },
231+
idLessFailClosed: true,
195232
},
196233
{ name: 'a null row', row: null },
197234
{ name: 'an undefined row', row: undefined },
@@ -269,7 +306,65 @@ describe('human-user predicate agreement — plugin-security `isHumanUser` vs pl
269306
}
270307
});
271308

272-
for (const { name, row } of CORPUS) {
309+
for (const { name, row, idLessFailClosed } of CORPUS) {
310+
if (idLessFailClosed) {
311+
/**
312+
* [#14348] The one corpus row this probe cannot read a predicate verdict
313+
* from — and why that is NOT a predicate disagreement.
314+
*
315+
* Both owners call `{ email: 'ghost@example.test' }` HUMAN, and they
316+
* still agree: nothing in #14348 touched either predicate. What changed
317+
* is the PROXY. Promotion is now "human AND can authenticate", and the
318+
* second conjunct is unanswerable for a row with no `id`: there is no key
319+
* to hang a `sys_account` on, so no account can exist and none can be
320+
* modelled above. Reading `adminPromoted` here would therefore report the
321+
* missing conjunct as a missing predicate agreement.
322+
*
323+
* So this row asserts the OUTCOME instead, and the outcome is
324+
* fail-closed on purpose. A row with no `id` cannot hold an exercisable
325+
* grant: the pre-#14348 code promoted it by writing
326+
* `sys_user_permission_set.user_id = undefined` — a grant addressed to
327+
* nobody, in the table whose whole job is to say who may administer the
328+
* platform. Refusing it is the same direction this file's own
329+
* NON_OBJECT_CORPUS already fixed ("for a promotion predicate the safe
330+
* answer to malformed input is no"), applied to the one malformed shape
331+
* that is a real object.
332+
*
333+
* ⛔ This is NOT licence to relax the agreement assertion for any other
334+
* row. Every id-bearing row still proves the two predicates agree, and
335+
* `no_authenticable_user` is asserted below precisely so this case cannot
336+
* pass on a harness that failed earlier for some unrelated reason.
337+
*/
338+
it(`fails closed on ${name} — id-less, so no account can key to it (#14348)`, async () => {
339+
const authSays = isHumanUserRow(row);
340+
const security = await securityVerdict(row);
341+
342+
// The predicates still agree that this row is human: asserted on the
343+
// owner side so a regression there cannot hide behind this case.
344+
expect(
345+
authSays,
346+
'plugin-auth isHumanUserRow must still call an id-less human row HUMAN',
347+
).toBe(true);
348+
349+
// ...and promotion still refuses it, for the second conjunct.
350+
expect(
351+
security.human,
352+
`an id-less row must NOT be promoted: the grant row it would write is\n` +
353+
`addressed to \`user_id: undefined\`, which no principal can ever exercise.\n` +
354+
` row: ${JSON.stringify(row)}\n` +
355+
` reason: ${security.reason ?? 'none'}`,
356+
).toBe(false);
357+
358+
// Prove the refusal came from the authenticable filter and not from an
359+
// earlier branch — the same anti-vacuity discipline the loop below uses.
360+
expect(
361+
security.reason,
362+
'refusal did not come from the authenticable filter',
363+
).toBe('no_authenticable_user');
364+
});
365+
continue;
366+
}
367+
273368
it(`agrees on ${name}`, async () => {
274369
const authSays = isHumanUserRow(row);
275370
const security = await securityVerdict(row);

0 commit comments

Comments
 (0)