fix(plugin-security): fail closed on a non-object row in the platform-admin promotion predicate

[claude]

Fixes #12515

Two owners answer "is this sys_user row a HUMAN?", and they disagreed on truthy
non-object input. The copy that disagreed is the one that performs
platform-admin promotion, and it disagreed in the fail-OPEN direction.

// before — plugin-security/src/bootstrap-platform-admin.ts:401
const isHumanUser = (u: any) => u && u.id !== SystemUserId.SYSTEM && u.role !== 'system';

On 'usr_alice', 42 or true, .id and .role are both undefined, so both
comparisons pass and the input scores human. isHumanUserRow
(plugin-auth/src/audience-posture.ts:317), the #11767-consolidated owner of the
same question, requires typeof row === 'object' and answers non-human.

Shape ruled at triage's 07:20Z grading and re-confirmed at the 09:14Z unlock:
option B — align the plugin-security copy to the fail-closed rule — plus the
card's option C, extending #12512's agreement pin to the class it had to
exclude. Option A (close on record) was rejected: the asymmetry has no scheduled
retirement, since consolidation was explicitly declined at #11959's grading.

The change

// after — mirrors isHumanUserRow exactly
const isHumanUser = (u: any) =>
  !!u && typeof u === 'object' && u.id !== SystemUserId.SYSTEM && u.role !== 'system';

!! completes the mirror: isHumanUserRow returns a real boolean, where the old
closure echoed a falsy input back. Both consumers (:426, :472) read it in
boolean position, so that half is decision-identical.

⭐ The measurement that had to come first

Over-tightening this predicate is worse than the bug it fixes: a guard that
excluded a row shape the real sys_user read legitimately produces would mean an
install unable to promote its first admin — locked out of itself, the same
symptom class the parent card exists to prevent, arriving from the other
direction. So the guard was measured before it was chosen, and it mirrors
isHumanUserRow rather than inventing a stricter rule.

Leg 1 — what the real read yields. A real SqlDriver (sqlite :memory:),
real DDL from the shipped SysUser declaration in @objectstack/platform-objects,
read through the exact production call shape:

rows returned: 5
typeof | isArray | proto  | id            | role      | BEFORE | AFTER | authRow | MOVED?
object | false   | Object | "usr_alice"   | "member"  | true   | true  | true    | same
object | false   | Object | "usr_bob"     | null      | true   | true  | true    | same
object | false   | Object | "usr_carol"   | null      | true   | true  | true    | same
object | false   | Object | "usr_dana"    | ""        | true   | true  | true    | same
object | false   | Object | "usr_system"  | "system"  | false  | false | false   | same

truthy NON-OBJECT rows the real read produced: 0
rows whose verdict MOVED when the typeof guard is added: 0
VERDICT: mirroring the guard changes NO reachable row. Safe to ship.

Leg 2 — the guard is already the incumbent on this exact population.
plugin-auth's dev-admin seed (auth-plugin.ts:1704) filters the byte-identical
read through isHumanUserRow today:

	read	predicate
auth-plugin.ts:1704	find(SystemObjectName.USER, { where: {}, limit: 50 }, { context: { isSystem: true } })	isHumanUserRow
bootstrap-platform-admin.ts:471	find('sys_user', { where: {}, limit: 50 }, { context: { isSystem: true } })	isHumanUser

SystemObjectName.USER === 'sys_user'. Same object, same empty where, same limit,
same system context. If this guard excluded a legitimately-produced row, that seed
would already be misfiring and the audience gate already refusing sign-ups.

⇒ No reachable answer changes. The 14 existing agreement cases are unmoved.

The pin extension

NON_OBJECT_CORPUS is a separate array from CORPUS, deliberately.
CORPUS's stated contract is that every entry is a shape a real read can return,
and these are not — filing them in would have quietly falsified that promise and
blurred how a failure should be read. A red in CORPUS means a reachable answer
moved; a red in NON_OBJECT_CORPUS means the fail-closed guard was dropped.

Seven cases: a bare id string, the system account's own id as a bare string
(fail-open would have promoted the service account), a number, true, a function,
and two falsy non-objects whose decision already agreed. Each asserts
isHumanUserRow is false absolutely rather than only that the two agree —
two predicates can agree by both failing open, which is the outcome this group
exists to forbid. Plus a group-level anti-vacuity test that fails if the truthy
members are ever quietly removed.

Dissolution verification

Guard reverted, pin left extended. Both legs rebuilt — the pin resolves
@objectstack/plugin-security through its exports map, i.e. through dist/, so
a verdict read without rebuilding would have measured the wrong artifact.

Mutation confirmed on disk before anything was read (removed-text occurrences 0,
injected-text occurrences 1), and confirmed absent from the built artifact:

ablation-dist-preflight: expecting NO "typeof u === "object"" in packages/plugins/plugin-security/dist
✓ marker absent from all 6 built files

Result — red on the non-object rows specifically, not a whole-file failure:

Tests  5 failed | 17 passed (22)

× the non-object input class > agrees on a bare id STRING where a row was expected
× the non-object input class > agrees on the SYSTEM account's own id as a bare string
× the non-object input class > agrees on a number
× the non-object input class > agrees on the boolean true
× the non-object input class > agrees on a function
✓ ...all 14 existing agreement cases green throughout

plugin-security and plugin-auth disagree on a NON-OBJECT row — the security
copy is failing OPEN on malformed input, and it is the copy that PERFORMS
platform-admin promotion.
  row:            usr_system (typeof string)
  plugin-auth  isHumanUserRow -> false
  plugin-security isHumanUser -> true (reason: none)

reason: none rather than no_users: the ablated code did not merely
mis-classify the string, it promoted it to platform admin.

Restored and verified: disk == index == HEAD (blob
945365e67553c357611091e92211ca68ad9551b8 on both sides, git diff HEAD empty),
rebuilt, marker present in dist again.

Verification (all at 235de87c)

check	result
plugin-security suite	83 files, 1539 passed
plugin-auth suite	81 files, 1653 passed
the pin	22 passed (14 existing + 8 new)
typecheck (both packages)	Done, both script names echoed
derived gate union (19 path-derived + convention-triggered)	all exit 0
check:i18n	OK (9 package(s) — all bundles in sync)
check:type-check-debt --re-measure	OK — 32 ledger entr(ies) re-measured, none above its recorded number
pnpm lint (whole repo, eslint . --no-inline-config)	exit 0

Gate union derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
against the real changed set, exit codes captured before any pipe.

check:i18n first returned PREREQUISITE NOT MET — the workspace CLI is not built
(exit 1 while checking nothing). That is NOT MEASURED, not a red: the CLI was built
and it was re-run to the green above. check:type-check-debt likewise refused
outright until @objectstack/service-knowledge was built.

Pin file type coverage — measured, not assumed

plugin-auth's tsconfig.json excludes **/*.test.ts, so pnpm typecheck says
nothing about the pin file. Proven rather than assumed, via tsc --noEmit --listFiles:
0 of 81 test files appear in that program.

The pin file is compiled — by the program check:type-check-debt --re-measure
builds over the hidden test layer. Reproducing that program (same tsconfig, test
exclusion lifted) lists the pin file among all 81 test files and reports 97
errors, matching plugin-auth's recorded TEST_DEBT of 97 to the unit, with zero
of them in the pin file. That ledger entry has zero margin ("surplus: none —
every entry sits exactly at its measurement, so any new error is red"), so the
green re-measure is a real statement about this file.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 2 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/automation/approvals.mdx (via usr_alice (literal))

What this run could not see

• the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 14 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 52a98238876fd5183ff087ce6cca1adb195a9636 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 11b0b2882e37c025e50687dc267445360a752570 — the merge of head 235de87c8c26f4c2dc4a32af79df0db6821b3acb into base 52a98238876fd5183ff087ce6cca1adb195a9636, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 11b0b2882e37c025e50687dc267445360a752570 && git checkout 11b0b2882e37c025e50687dc267445360a752570
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 52a98238876fd5183ff087ce6cca1adb195a9636 235de87c8c26f4c2dc4a32af79df0db6821b3acb && git checkout -B drift-repro 52a98238876fd5183ff087ce6cca1adb195a9636 && git merge --no-ff 235de87c8c26f4c2dc4a32af79df0db6821b3acb

node scripts/docs-audit/affected-docs.mjs --json 52a98238876fd5183ff087ce6cca1adb195a9636

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 52a98238876fd5183ff087ce6cca1adb195a9636 → pass the list as
args.docs, on the commit named under Which tree this was computed on.