Skip to content

fix(plugin-security): walled elevation requires a verified owner-email match, and the bootstrap replays on the verifying update - #11639

Queued
os-sam wants to merge 6 commits into
mainfrom
claude/issue-11343-verified-email-elevation
Queued

fix(plugin-security): walled elevation requires a verified owner-email match, and the bootstrap replays on the verifying update#11639
os-sam wants to merge 6 commits into
mainfrom
claude/issue-11343-verified-email-elevation

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Fixes#11343

What this closes

Under walled postures (group/isolated), the platform-admin elevation in bootstrapPlatformAdmin matched the env-declared OS_PLATFORM_OWNER_EMAIL against the raw email string on sys_user, with no email_verified condition — while email verification is off by default. PR #11211 was a real and large narrowing — it moved the hole from "whoever registers first" to "whoever knows the owner's email and registers first". This PR closes that remainder, exactly as card #11343 records it; no new reproduction detail is added here.

Direction per the card's triage grading (maintainer-invoked, 2026-08-24): invariant restoration — the elevation match must be a verified email. No product fork.

Both halves, deliberately in one change

The card's own measurement is that the obvious fix taken alone creates a worse defect: the bootstrap re-run listened for sys_userinsert only, and email verification is an update — so requiring verification without an update trigger would refuse the genuine owner at sign-up and then never look again, leaving a first boot with no administrator at all.

  1. bootstrap-platform-admin.ts — the elevation half. The walled owner-email match additionally requires email_verified, via a fail-closed allow-list predicate over driver representations (true/1/'1'/'true'); an absent field on an imported/legacy row reads as unverified. An unverified holder of the owner's address is refused with the new reason walled_owner_not_verified, logged loudly at warn with the unblock in the line, and never falls back — the same fail-closed direction as the existing undeclared-owner refusal. single posture is untouched in both directions (first-user promotion gains no verification requirement; the owner-email variable is still never consulted there — both pinned).
  2. security-plugin.ts — the replay half. The bootstrap-replay middleware now fires on sys_userupdates touching email_verified or email (the change-email flow rewrites both), in addition to insert/create. The trigger set is extracted as shouldReplayBootstrapFor in bootstrap-platform-admin.ts, consumed by the middleware and its pins alike (the resolveEngineUpdateDispatch pattern), and deliberately does not fire on unrelated profile edits.

Pins, in both directions

A suite pinning only the refusal would score green on a platform nobody can administer, so both directions are pinned in bootstrap-platform-admin-walled-owner.test.ts and security-plugin.test.ts:

  • Refusal: unverified holder of the owner email (explicit false, absent field, driver 0) ⇒ adminPromoted: false, reason walled_owner_not_verified, nosys_user_permission_set write, loud warn naming OS_PLATFORM_OWNER_EMAIL.
  • Elevation: verified owner elevates — including across the refuse-then-verify-then-re-run sequence, and when an unverified row with the same address is older; driver 1 reads verified.
  • Trigger set: exhaustive predicate pins, plus a wiring pin that the registered middleware re-runs the bootstrap on the verifying update, not on a profile edit, and still on insert.

Reverse verification (mutate → confirm on disk → run → restore byte-identically, trap'd): removing the verified filter reds exactly the 5 refusal-direction pins (15 others stay green); disabling the predicate's update branch reds exactly the 2 update-trigger pins plus the wiring pin (258 others stay green). Restoration proven by git hash-object against the HEAD blob. Both suites resolve the mutated modules via same-package relative imports (no dist/ in the subject's path), so no rebuild was required for the ablation legs.

Verification at head cd8a891892

All run through the shared verify lock, after origin/main was merged in (clean merge; the #11610 edit to security-plugin.ts is in a different region):

  • pnpm --filter @objectstack/plugin-security test — 76 files, 1479 passed · typecheck (tsc --noEmit) clean
  • Derived gate batch (dispatch-gates at this tree): check:nul-bytes · check:changeset-gate-self-tests · check:cross-package-test-inputs · check:objectui-changeset · check:published-files · check:slot-lookup · check:test-source-alias · check:type-source-resolution · ADR-0087 registration · changeset-no-major · ci-filter-parity · empty-changeset · plugin-teardown-shape · affected-docs · release-rehearsal self-test · check:query-options-erasure · check:type-check-coverage · check:engine-double-contract · check:where-matcher · check:i18nall green (BATCH-RESULT: FAIL=0)
  • check:type-check-debt (--re-measure, closure built): OK — 32 ledger entries re-measured, none above its recorded number
  • The new fake engine double routes update() through assertEngineUpdateDispatch and is recorded in the pinned ledger (--write; coverage grew, nothing lost)

Repo-wide pnpm lint was not run locally — CI owns that run on every PR.

Changeset: .changeset/walled-elevation-verified-email.md (patch, @objectstack/plugin-security).

Generated by Claude Code


Generated by Claude Code

…l match, and the bootstrap replays on the verifying update
Closes the remainder #11211 left open (issue #11343): under walled postures
the platform-admin elevation matched the env-declared owner email as a raw
string, and email verification is off by default — so an account created
with the owner's address before the owner registered would be elevated.
Both halves land together, deliberately:
1. bootstrapPlatformAdmin: the walled owner-email match must be VERIFIED
(fail-closed allow-list over email_verified representations; absent
reads unverified). Unverified holder => refused with the new reason
walled_owner_not_verified, logged loudly with the unblock in the line,
never falling back — the same direction as the undeclared-owner refusal.
2. security-plugin bootstrap-replay middleware: also fires on sys_user
updates touching email_verified / email (trigger set extracted as
shouldReplayBootstrapFor, consumed by middleware and pins alike).
Verification is an UPDATE; with the old insert-only replay, half 1
alone would refuse the genuine owner at sign-up and never look again,
leaving the platform without any administrator.
single posture is untouched in both directions. Pins cover both invariant
directions: the unverified holder is refused AND the verified owner is
elevated, including across the refuse-then-verify-then-re-run sequence.
Fixes#11343
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
…tEngineUpdateDispatch; record the pinned double
check:engine-double-contract named the new fake's bare update(); pin it to
the producer's dispatch predicate and let the RETAINED ledger learn the
new pinned coverage (--write; 0 lost, coverage grew).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

github-actionsBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

24 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 266436a7f78b52a1c7a79d631ed6eea75118700d.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 20 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 266436a7f78b52a1c7a79d631ed6eea75118700dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 0f80bbea5920790a445c827beed3cb9df624667b — the merge of head c9be0f18ced013be3a166f6d530444a21742e4f5 into base 266436a7f78b52a1c7a79d631ed6eea75118700d, 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 0f80bbea5920790a445c827beed3cb9df624667b && git checkout 0f80bbea5920790a445c827beed3cb9df624667b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 266436a7f78b52a1c7a79d631ed6eea75118700d c9be0f18ced013be3a166f6d530444a21742e4f5 && git checkout -B drift-repro 266436a7f78b52a1c7a79d631ed6eea75118700d && git merge --no-ff c9be0f18ced013be3a166f6d530444a21742e4f5
node scripts/docs-audit/affected-docs.mjs --json 266436a7f78b52a1c7a79d631ed6eea75118700d

⚠️ 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 266436a7f78b52a1c7a79d631ed6eea75118700d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…-provisioned, not a self-registrant
The Dogfood Regression Gate caught #11343's invariant end-to-end: two
walled-posture fixtures (automation-toggle-tenant-scope :189,
federated-rls-injectors :135) build their platform admin through the
shared dev-admin seed, which drives the real sign-up pipeline and so
left the account unverified — correctly refused by the restored
invariant, i.e. the fixtures modeled a deployment state the platform no
longer considers valid.
The repair is the shared seed path, not the fixtures: maybeSeedDevAdmin
now stamps email_verified on the account it just provisioned. The seeded
dev admin is created by the deployment's own boot command with
operator-known credentials — the same trust shape as a trusted-SSO
insert — not an unknown self-registrant, which is the class the
invariant refuses. Dev-only by the existing NODE_ENV gate; the generic
sign-up factory is untouched, so unverified-user flows keep testing what
they name. Fail-open on the stamp, fail-closed on elevation.
Part of #11343
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
…ied stamp ships under the same card
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — PASS. Flipping ready and arming the merge queue at c9be0f18ce.

domain:services seat (session session_01APWX2AwT3a4xDcjPCe8bk4).

⭐ The revert-check passed — but NOT via the lever I specified, and that is the finding

I dispatched this: "with isEmailVerified reverted, BOTH automation-toggle-tenant-scope.dogfood.test.ts:189 AND federated-rls-injectors.dogfood.test.ts:135 must go red again; only-one-reds is a FINDING."

That instruction was wrong, and the verification dev did not simply obey it — it worked out that my lever could not answer the question, then found the one that could.

  • Lever A (mine — revert isEmailVerified in plugin-security): reds exactly the 5 refusal-direction pins (15 others in that file green), confirming the earlier claim at this sha — but leaves both named dogfood assertions green, along with the whole suite (924 passed / 0 failed). ⭐ And that is correct, not a repair-by-something-else: those two are pre-existing tests that were green on main. The requirement is what broke them; the stamp is what repaired them. Removing the requirement simply returns them to their pre-[security] 围墙提权只按未验证邮箱匹配 —— #11184 落地后仍存在一条更窄的同形路径:抢在 owner 之前用其邮箱注册即可被提权 #11343 green. My lever cannot red them.
  • Lever B (the one that isolates it — revert the plugin-auth stamp): :189 reds expected false to be true; :135 reds expected 403 to be 200. Both named assertions, both predicted signatures, while all 8 siblings — including both CONTROL tests — stay green. The mutated blob b78e74c4… is byte-identical to the merge-base blob of auth-plugin.ts, so leg B was an exact complete revert, not an approximation.

The dogfood tests genuinely measure the fix, and the green CI at this sha is earned. That is the conclusion my instruction was reaching for; it took correcting my instruction to get there.

⭐ A new instrument hazard, worth the whole repo's attention

The first ablation-A build failed (TS6133) — but tsup had already written the mutated JS bundle before the dts step failed, so a build reporting exit 1 left plugin-security/distablated. The trap restores SOURCE, not dist.

It was caught only because the next run's pre-mutation positive control read dist walled_owner_not_verified: 0 where the baseline had read 1.

This is a sibling of #11539 ("a trap firing is not proof of restoration"), and arguably nastier: a failed build reads as "nothing happened", and is not. Any ablation whose subject resolves through dist/ inherits it. Recorded here so the next agent does not have to rediscover it.

Corrections to the PR body — read them, the body is stale in one place

⚠️The body says "Verification at head cd8a891892". The head is c9be0f18ce — the changeset commit landed after. A verification claim pinned to a non-head sha is unfalsifiable at review time, which is exactly why the union must be re-run on the final head. It was: the full gate union (26 gates, each quoted from its own verdict line, every exit captured before any pipe) and the type-check-debt ratchet were re-run at c9be0f18ce and are green — check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured … none above its recorded number. So the claim is true and the sha citing it was stale; the body was deliberately left unedited by a verification-only dispatch, and this comment is the correction of record.

Also: five exit 99 / queue-timeouts were each re-run to a real verdict rather than read as results, and the body carries a duplicated generation footer (cosmetic).

Clause-② — assessed as NO, and here is the reasoning so it is reviewable

I re-read the live text this round: "Clause ② is NOT reachable from paths … judged from the card CONTENT. This line is a FLOOR, never a clearance." Applying it honestly rather than conveniently, because this is precisely the card where one would most want to skip the control:

  • The change is a tightening of an internal bootstrap's elevation predicate. My standing constraint is explicit that security-boundary loosening is the maintainer floor and tightening is not.
  • It changes no surface an author or caller programs against: bootstrapPlatformAdmin accepts no author input, and its "accept/reject" is over rows, not over a contract. No public surface widens.
  • Triage — whose field this grading is — reviewed it maintainer-invoked on 2026-08-24 and graded it "invariant restoration … no product fork", without applying needs:contract-review. Adding a control triage deliberately withheld is as much an override as removing one.

⇒ Not Clause-②. Stated openly rather than assumed, so anyone who disagrees can point at the reasoning.

Verified independently by this seat, not read off the report

  • maybeSeedDevAdmin's gate is if (process.env.NODE_ENV !== 'development') return; as its first line (auth-plugin.ts:1544), strict equality; one real call site; plus the OS_SEED_ADMIN opt-out and a zero-human-users precondition; never touches an existing account.
  • Ratchet handling correct: engine-double-contract.pinned.json +1 row (coverage ledger, legitimately grows); baseline.json untouched.
  • Zero forbidden paths across all 7 files; no assertion removed anywhere.
  • Net exposure vs main today: nothing widens. Confirmed by the dev's reachability census of every production consumer of email_verified and by lever B reproducing the pre-[security] 围墙提权只按未验证邮箱匹配 —— #11184 落地后仍存在一条更窄的同形路径:抢在 owner 之前用其邮箱注册即可被提权 #11343 world exactly — the dev-seeded admin was already elevated on main, so the stamp is restoration, not expansion, while every other account's elevation path is now strictly narrower.

One correction against me, caught for the second time today

The dispatch brief said @objectstack/plugin-auth TEST_DEBT is 109. It is 97#10615 re-tallied it, and I cited that very card as my authority. Two independent devs flagged it. Untouched in both directions; --lower never run. Corrected on #10348 as well.

(Declared deviation, accepted: the verification reused the previous dev's on-disk worktree for this same branch rather than creating a second one — git refuses two worktrees on one branch, and a detached checkout would have cost a fresh multi-minute install in a contended container. Still a dedicated per-task worktree for this card, never the shared checkout. Leave it in place until this merges.)


Generated by Claude Code

@os-sam
os-sam marked this pull request as ready for review August 24, 2026 12:55
@os-sam
os-sam added this pull request to the merge queueAug 24, 2026
Any commits made after this event will not be merged.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-sam@claude