Uh oh!
There was an error while loading. Please reload this page.
feat(core): anchor PLATFORM_ADMIN on a verified OS_PLATFORM_OWNER_EMAIL match, inside the one derivation site - #13146
Conversation
📓 Docs Drift CheckThis PR changes 2 package(s): 25 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 29 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8e27fb3189c9663e32cf256356d2de051ffa5fd8 && git checkout 8e27fb3189c9663e32cf256356d2de051ffa5fd8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 901355c3bc80f50e4b74d3dbf0d2b8a4c8b09d1c 9e9d05365ed177670ae527c740a0dd3dc1c29d0f && git checkout -B drift-repro 901355c3bc80f50e4b74d3dbf0d2b8a4c8b09d1c && git merge --no-ff 9e9d05365ed177670ae527c740a0dd3dc1c29d0f
node scripts/docs-audit/affected-docs.mjs --json 901355c3bc80f50e4b74d3dbf0d2b8a4c8b09d1c
|
os-zhuang
commented
Aug 29, 2026
PM 复核 —— 四项关键主张逐条独立复测,全部成立;⛔ 但本 PR 落不了地PM seat #6367 · session ✅ 1. 「pinned golden 没有移动」—— 结构性成立,不只是断言卡片正文把 golden 移动列为已知代价("this moves the pinned golden … Move it with an explanation")。本 PR 声称它没有移动。这是对卡片预期的偏离,故按最高标准查: ⇒ 再查那 +35/-1 是不是在削弱断言: constENV='OS_PLATFORM_OWNER_EMAIL';beforeAll(()=>{ambientOwnerEmail=process.env[ENV];deleteprocess.env[ENV];resetPlatformAdminEmailMemo();});afterAll(()=>{/* 原样还原 */resetPlatformAdminEmailMemo();});⇒ 纯粹的环境密封性(save/clear/restore + memo 重置),⛔ 一条断言都没动;那个
机制也复测了,不是只看结论 —— constneedsUserRow=!grants.email||!grants.permissions.includes('ai_seat')||platformAdminConfig.emails.length>0;⇒ config 项是 OR 进去的:列表为空时 ✅ 2. 安全钉 —— 读存储行,绝不读 |
os-zhuang
commented
Aug 29, 2026
|
| 分句 | 判定 | 依据(现读本 PR head) |
|---|---|---|
| 「un-makes every platform admin at once」 | ❌ 假 | §6b-config 分支自行置 hasPlatformAdminGrant = true,能力取自 @objectstack/spec 的 ADMIN_FULL_ACCESS_CAPABILITIES,⛔ 不经过 sys_permission_set 行 ⇒ 改名/删除/停用该行后,配置派生的管理员原封不动(resolve-authz-context.ts ~640-655) |
| 「with no identity table touched」 | ❌ 假 | 配置路径读调用者的存储 sys_user 行 —— 身份表 —— 而这正是本 PR 在同一次提交里把它改判 derives 的原因 |
⭐ 定性依据是本 PR 自己写的那句话
PR 正文为改判 sys_user 给出的理由,逐字:
Its old reason said "Neither confers administrator standing"; that sentence is now false, which is exactly the staleness this file exists to catch.
⇒ 同一句论证原样适用于它的同胞。⛔ 这不是本席另立标准,是本 PR 的标准没有贯彻到底。
ADMIN_STANDING_SURFACE上方的文件级 docblock,把两条路径都写进去了(git diff 可见)。漏的只是 sys_permission_set条目自身的 reason —— 而逐条 reason 恰恰是对应门与其读者真正读的那个字段,文件级 docblock 不是。
已处置
已把该修正回传给实现席(同分支、仅改 reason 字符串):⛔ 不动 role、不动 columns、⛔ 不动任何可执行逻辑、⛔ 不再扩大 PR。要求写成对两种配置都为真的形式 —— 变量未声明时旧句子仍完全正确,这个条件性本身就是诚实的版本。本席已把整份条目集 diff 过,⛔ 该文件其余部分无需再动。
顺带两项,⛔ 不在本 PR 处理
content/docs/permissions/authorization.mdx:410-412用散文说了同一个现在不完整的主张:「admin_full_accessis what makes the environment's platform admins, so switching it off would un-make all of them in one write」。⛔ 本席不要求在代码 PR 里改文档。OS_PLATFORM_OWNER_EMAIL在content/docs里零命中(实测),而本腿拓宽了它的含义。
⇒ 两项都待实现席判断后由本席单独立卡,⛔ 不夹带进本 PR。
sys_user (symbol, 29 pages) 被它自己判为 "matched too much of the corpus to be a work list" ⇒ 那 23 行手写页清单主要由这个过宽锚点撑起来,⛔ 不宜当作工作清单逐页复核。本席因此改问「有没有一处主张会被证伪」,而不是复算 23 页 —— 上表就是那一问的答案。
Generated by Claude Code
…mbinators check:where-matcher graded the new fixture's matches() as silently wrong: with no combinator branch it read $or as a field name, compared row.$or (undefined) against the array and excluded the row, leaving the suite asserting on an empty result with nothing erroring. Refuses instead of implementing, which is what most of this repo's conforming doubles do and what the sibling batch-equivalence double already spells. $in stays supported: it is a per-field value operator the resolver really issues, not a top-level combinator.
…, not the row Two defects in one sentence, both pre-existing on main: - it said the row `platform_admin` is resolved by name; the row is `admin_full_access` and `platform_admin` is the POSITION that row derives (resolve-authz-context.ts:594 matches the row, :665-666 unshifts the position); - 'un-makes every platform admin at once' stopped being true for a configured deployment: the config anchor sets the same standing off the caller's own sys_user row and never reads this table. A flat replacement would only swap which half is wrong, so the reason is now conditional and states the condition -- true whether or not OS_PLATFORM_OWNER_EMAIL is declared. Reason string only; role, columns and every executable path are untouched.
os-zhuang
commented
Aug 29, 2026
CI 红: |
os-zhuang
commented
Aug 29, 2026
给契约复审者的一条输入:你要审的这个文法加宽,今天在四个读者处是「声明了不执行」⛔ 本席不主张这该阻塞复审,也⛔ 不主张本 PR 要修它。但复审者读的是 PR,不是卡片线,所以这条必须出现在这里。 事实本腿按 Choice 2B 把 已立卡 #13147,并由本席在本 PR head 上独立复测(⛔ 非采信卡片):
运维实际看到的
⇒ ⭐ 「授权那一半生效,另外三半静默不生效」 —— 无一处报错。运维照本设计将要发布的文档写列表,得到的就是这个自相矛盾的状态。
为什么仍不主张阻塞
复审者真正该拿去用的一句话
⛔ 本席清不了 Generated by Claude Code |
os-zhuang
commented
Aug 29, 2026
re-run 结果:没变绿 —— 按承诺不当 flake 处理,已根因并给出补丁方向接前一条( ⭐ 但它红的方式改变了结论
相同:文件、行号 ⭐ 真正的代码回归会两次打中同一个 case。 换 arm 而同点失败 ⇒ 病灶在两条 arm 共用的读取点,不在任一条产品逻辑里。已根因: 完整诊断、补丁方向与同族辨析已写在 #13158(该卡自陈是「a NAME, not a diagnosis」并向受害者索要诊断 —— 现已提供)。顺带把它从「只有 对本 PR 的处置
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#11970
Leg L2 (core) of the accepted platform-admin re-anchor design filed under issue 11663, which stays open as the coordination parent — nothing here closes it. Provenance: design comment 5394453215 (sections 2, 3, 4 Choices 1-3 and 7, section 6 row L2); maintainer acceptance comment 5404675670 (2026-08-25), bundle 1A/2B/3A/4A/5A/6A/7A.
PLATFORM_ADMINcan now be anchored on deployment configuration instead of a stored grant row: an account whosesys_user.emailis onOS_PLATFORM_OWNER_EMAILand whoseemail_verifiedreads verified resolvesPLATFORM_ADMINwith the declaredadmin_full_accesscapability set, derived live inside the one derivation site.Premise re-verification against current
origin/mainThe design was measured at
cad8b42f00; the card's standing instruction is to re-verify every citation before implementing. Result:origin/mainpackages/types/src/env.ts:172—PLATFORM_OWNER_EMAIL_ENVresolve-authz-context.ts:341—getUserRow()resolve-authz-context.ts:365—needsUserRowresolve-authz-context.ts:573— the section-6b platform-admin testresolve-authz-context.ts:332—seedEmailseedinggrants.email:293, is now:294)packages/verify/src/harness.ts:360-371serve-organizations-host-resolution.e2e.test.ts:153,serve-organizations-mount-failure.e2e.test.ts:141ADMIN_FULL_ACCESS_CAPABILITIESis exported from@objectstack/specisEmailVerifiedis a private function inbootstrap-platform-admin.ts, to be promoted@objectstack/typesasisEmailVerifiedUserRow— a different home from the one the design proposed (@objectstack/core/security). Choice 7A'scoretotypesdependency makes it directly consumable, so this leg imports it instead of re-homing it. One definition, two consumers, now three.Two more premise refinements, both stated because they make the diff smaller than the card predicted:
packages/verify/src/harness.tsand the two CLI fixtures needed no change. The card anticipated a verified-email fixture shape; it is not needed while the derivation is purely additive, because the harness's seeded owner already holds the legacy grant and resolves exactly as before whether or not the config route also fires. That fixture work belongs with the leg that stops minting grants.The pinned golden: it did not move, on purpose, and the suite now pins why
The card states the known cost up front: reading
email_verifiedmakes thesys_userread unconditional for authenticated principals, moving the query multiset pinned inresolve-authz-context.batch-equivalence.test.tsplus its.golden.json.Measured, that cost is conditional on configuration, and pin P2 of the design already rules the shape that makes it so: "
resolvePlatformAdminEmails()returns an empty list for unset/blank; the derivation returns false on an empty list before touching the user row." So:OS_PLATFORM_OWNER_EMAILunset — every deployment that has not adopted the config anchor, and every fixture in that suite — the config branch short-circuits before any row is read,needsUserRowis unchanged, and all eleven goldens are byte-identical. No golden was re-captured, re-baselined or edited.sys_userread does become unconditional. Only one fixture (seeded-permissions-and-email, the fully-seeded API-key principal) skips that read today, so that is the one query multiset that would move — and it moves only for a configured deployment.That property is worth nothing unless the suite pins the condition it rests on, so the suite now clears the variable and drops the parse memo around itself rather than assuming an unset ambient environment: an inherited value in a CI worker would otherwise add one
sys_userread and turn a differential control into a mystery. A comment on that block says the golden moves with an explanation if a later leg makes the read unconditional, and is never re-captured to agree with new output.The conditional read is pinned directly too, in
resolve-authz-context.platform-admin-config.test.ts: a fully-seeded principal reads zerosys_userrows with nothing declared, and exactly one with administrators declared (once, not twice — the config branch consumes the same memoized row the email fallback and theai_seatsynthesis do).What is in the diff
packages/core/src/security/platform-admin.ts(new) — the config anchor, whole:OS_PLATFORM_OWNER_EMAIL. No second variable.trim().toLowerCase()), duplicates collapsed, blank entries dropped, and any unparseable entry fails the whole variable closed, loudly, naming the entry. Never skip-and-continue: the two valid entries beside a typo do not survive it either, because a silently narrower administrator set than the operator declared is the failure this arm exists to prevent.email_verifiedcolumn reads unverified.One measurement is written into that file because it would otherwise be re-attempted: the parseability predicate is a deliberate shape check, not zod's
.email(). Measured on zod 4.4.3, the version this package resolves,z.string().email()rejectsa@b.c— this card's own acceptance-criterion value — andadmin@localhost. Under Choice 2B a rejected entry fails the whole variable closed, so an over-strict validator here is not a stricter contract, it is an unrecoverable lockout on a value nobody can fix from inside the product. The predicate asks only what an address must minimally be to ever match asys_user.email: oneatsign, something either side of it, no whitespace.resolve-authz-context.tssection 6b-config — the second route to the samehasPlatformAdminGrant, inside the same derivation site, never a second one. Capability content comes fromADMIN_FULL_ACCESS_CAPABILITIESin@objectstack/spec(Choice 6A), so the derived envelope and the declared set cannot drift.⭐ The most important mechanical pin of the leg: the comparison reads the caller's own stored
sys_userrow, nevergrants.email.grants.emailis seeded fromopts.seedEmail, a caller/session-supplied string that deliberately wins over the stored read for RLS purposes, so deriving superuser standing from it would open a new escalation channel inside the change meant to close one. Pinned by test in both directions: a session payload carrying a configured address over a row that does not resolves non-admin, and a verified stored match still resolves even when the seed disagrees.Nothing is revoked. The legacy unscoped
admin_full_accessgrant confers exactly as before (design section 5 step 3). Pin P5's migration pointer is wired: a holder whose standing rests on the row alone gets one warning per process naming the row's holder and the exact config line that re-anchors them. It is silent when the same user also resolves through the config anchor, and it never forces asys_userread of its own.ADMIN_STANDING_SURFACE—sys_useris reclassified fromreads-onlytoderives, withid,email,email_verified,ai_accessdeclared and asserted equal to what the recording harness observes. Its old reason said "Neither confers administrator standing"; that sentence is now false, which is exactly the staleness this file exists to catch. Because a conditional read is invisible in a fixture that never takes the branch, the observation suite gains a variant that declares administrators and takes it, plus a second positive control proving that variant reaches the config anchor and not a grant.ADMIN_STANDING_NON_TABLE_INPUTSis new beside it: pin P6 asks for a way to express a non-table derivation input, or the file goes on being accurate about tables while silently claiming the derivation reads only tables. It is a separate export rather than a pseudo-row, because the table map is compared for equality against observed table reads and a pseudo-row would need excluding by name.The same file's
sys_permission_setreason carries two corrections, both PRE-EXISTING onmainand both deliberate: it namedplatform_adminas the row resolved by name, when that is the POSITION the row derives and the row isadmin_full_access(resolve-authz-context.ts:594matches the row,:665-666unshifts the position); and its claim that switching the row off un-makes every platform admin stopped being true once the config anchor exists, so the sentence is now conditional and states the condition rather than swapping which half is wrong. Reason string only —role,columnsand every executable path are untouched.@objectstack/coregains@objectstack/types(Choice 7A). Re-measured acyclic on currentmain:typesdepends only onspec;specdepends only onzodandpg-connection-string.The one file outside the card's stated surface, and why
packages/plugins/plugin-auth/src/last-admin-guard.tsis touched. This is mechanically forced by reclassifyingsys_user, not scope creep, and the boundary is drawn deliberately.The correspondence gate
last-admin-standing-keys.test.tsreadsADMIN_STANDING_SURFACEfrom core and requires every column of everyderivestable to be either judged by a standing-key list or excluded with the reason it cannot empty the administrator population. There is no third state. So this leg either lands with plugin-auth red, or it answers the gate. The two available answers were weighed:sys_user.emailoremail_verifiedwrite cannot empty the administrator population. That is false the moment an administrator's standing is config-derived only, which is the state this very leg creates. Writing it would be the precise false sentence the file exists to prevent.USER_STANDING_KEYS = ['email', 'email_verified']— and a list nothing consults is a phantom, so the arm and the enumeration half come with it.What landed, therefore, is the minimum that is honest:
USER_STANDING_KEYS, inSTANDING_KEYS_BY_TABLE, withidandai_accessexcluded with reasons.resolveAdminUserIdscounts config-derived administrators, through the resolver's ownmatchesConfiguredPlatformAdmin— never a re-spelled copy, so the normalization, the list parse and the fail-closed verified check cannot drift between the two readers. The scan pushes the normalized addresses down and the predicate re-checks each row in JS: the same two-stepauth-manager.ts:3825-3846already argues for, for the same measured reason (better-auth'sinternalAdapter.createUserlowercasesuser.emailbefore storing, and every producer in this repo does the same, so the pushed-down filter is exact on a case-sensitive store, while a case-folding collation returns extra rows the predicate drops).email_verifiedreset, that would leave the environment with no administrator is refused, naming the configuration as the remedy rather than this guard.What deliberately did not land: re-pricing which of the guard's existing refusals become obsolete once no runtime write can empty the platform-admin population. That is design section 5 step 5, it belongs to the plugin-auth leg, and the code says so at the seam. This addition can only make the guard refuse more, never less.
With no variable declared the whole plugin-auth change is inert: the list is empty, the enumeration reads no
sys_userrows for it, and every deployment that has not adopted the config anchor sees the guard behave exactly as it did.Verification
All figures below are from the final commit,
fb2200096, after mergingorigin/main. Shared-box seconds throughout (parallel agents on one container).Tests:
@objectstack/core— 43 files, 1054 passed, including the untouched batch-equivalence goldens.@objectstack/plugin-auth— 84 files, 1711 passed.@objectstack/plugin-security— 89 files, 1619 passed.@objectstack/rest— 154 files, 2543 passed.@objectstack/runtime— 2846 passed. Eight files failed to collect on the first attempt with "Failed to resolve entry for package @objectstack/service-cluster" — an unbuilt dependency in this worktree, not this change; afterpnpm --filter '@objectstack/runtime^...' buildthose eight files run 52/52 green.pnpm --filter @objectstack/plugin-auth typecheck— clean (tsc --noEmitplus the examples program).New coverage: 19 parse/match unit cases, 18 derivation cases (the acceptance criterion, every fail-closed arm, both directions of the seeded-email pin, the legacy-grant pointer, and the conditional-read property), 11 break-glass cases including two reverse-verification cases on an unguarded engine.
Reverse verification of the derivation itself, run from the committed state: forcing the section 6b-config branch false turned 9 tests red across the two suites. The mutation was confirmed on disk before the run — the file's blob hash changed, the injected text matched once and the replaced text zero times — and the restore was confirmed by state, not by an exit code:
git diff HEADempty and the worktree hash equal to the HEAD blob.Gates — the derived family from
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackagainst this branch's real change set, plus the ones the diff implicates, all run atfb2200096, each exit code captured before any pipe:pnpm lint(whole repo, not narrowed) ·check:authz-resolver·check:test-source-alias·check:type-source-resolution·check:undeclared-dep-imports·check:cross-package-test-inputs·check:objectql-double-limit·check:nul-bytes·check:published-files·check:kernel-hook-pairs·check:slot-lookup·check:page-declaration-shape·check:override-consistency·check:objectui-changeset·check:changeset-gate-self-tests·check:durability-log-level·check:startup-registry-verdict·check:type-check-coverage·check-comment-mask-adoption·check-adr-0087-registration·check-changeset-no-major·check-empty-changeset·check-changeset-fixed·check-osv-exemptions·check-ci-filter-parity·check-plugin-teardown-shape·check-shard-attestation·check-keyed-text-bounds·docs-audit/check-affected-docs·pm/release-rehearsal-clone --self-test— 30 of 30 exit 0.pnpm --filter @objectstack/spec build && check:generated— all 14 generated artifacts up to date after theorigin/mainmerge (the merge movedpackages/spec).Two derived gates report NOT MEASURED, neither of them a failure, both named rather than counted as passes:
check:dual-build-cjs-loadsexits 3 with "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/". CI builds everything. Substituted locally with a direct smoke load ofpackages/core/dist/index.cjsanddist/index.js: both resolve the new exports andADMIN_STANDING_SURFACE.sys_user.rolereadsderives.scripts/pm/check-half-states.mjsexits 3 with "PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential". Nothing was swept.Also NOT MEASURED, and pre-existing: neither package type-checks its own test files.
@objectstack/corehas notypecheckscript at all (it sits in the coverage DEBT ledger), and@objectstack/plugin-auth'stsconfig.jsonexcludes tests — confirmed bytsc --noEmit --listFiles, which does not list the new test file. The runtime verdict on those files is the vitest run above.Two supporting config changes came out of the gates rather than out of taste, each carrying its own reason in the file:
packages/core/vitest.config.tsgains an anchored alias so the suites read@objectstack/typessource rather than the workspace link'sdist/(check:test-source-alias), andpackages/core/tsconfig.jsongains the matchingpathsrule with therootDirwidening it forces (check:type-source-resolution), following the patternpackages/plugins/plugin-security/tsconfig.jsondocuments.Landing
⛔ This PR is draft on purpose and stops here. The card declares Clause-2 yes and carries
needs:contract-review, which a subagent cannot self-certify and which the dispatching seat cannot clear either. Parking at that gate is the expected end state. Do not flip it ready, do not arm auto-merge, do not queue it, and do not strip the label.The seven downstream legs wait on this card closing. This PR does not touch any of them and does not widen to cover them.
Generated by Claude Code
Generated by Claude Code