Skip to content

action.visible 的 current_user.positions 装的是 auth 角色而非安全层岗位,按岗位收敛的按钮对所有人静默消失(17.2.0) #15136

Description

@baozhoutao

现象

在对象动作(defineAction)的 visible CEL 里,current_user.positions 拿到的是 auth 层的角色(["user","org_member"]),不是安全层的岗位。持有业务岗位的用户,按岗位收敛的按钮谓词一律判假,按钮对所有人消失 —— 包括本该看到它的那个岗位。

current_user 这个根本身是绑上的(has(current_user.id) 与 has(current_user.positions) 都为真),所以谓词不报错、控制台无告警,只是数组里装的是另一根轴上的值。对作者而言,这是一次静默的判假。

最小复现(17.2.0,Console UI)

  1. 应用里给某对象加一个动作:

    defineAction({
      name: 'demo_archive', label: '归档', objectName: 'demo_sheet', type: 'script',
      body: { language: 'js', capabilities: ['api.write'], source: "return { ok: true };" },
      visible: "has(record.status) && record.status == 'approved' && ('demo_reviewer' in current_user.positions)",
      locations: ['record_header'],
    });
  2. 用一个确实持有 demo_reviewer 岗位的账号登录 Console,打开一条 status == 'approved' 的记录。

  3. 观察:按钮不显示。控制台无 CEL 告警。

  4. 对照两个接口:

    • GET /api/v1/auth/me/permissions → {"positions":["org_member","demo_reviewer","everyone"], ...} —— 岗位在这里,服务端也是按它判的;
    • GET /api/v1/auth/get-session → {"user":{ ..., "positions":["user","org_member"], ...}} —— Console 的 current_user 取自这里,岗位不在其中。
  5. 把谓词换成 has(current_user.positions),按钮显示 —— 说明根与字段都在,只是内容是 auth 角色。

期望能力

UI 谓词里的 current_user.positions 与服务端授权判定用的是同一根轴:即 /auth/me/permissions 报告的那组岗位。这样「按钮只对当前节点的审核岗位显示」这类需求可以在元数据里表达,而不必在应用侧另建一套。

如果 get-session 的 user.positions 是有意保留给 auth 角色的,那么希望给 UI 谓词另开一个明确的根(例如 current_user.security_positions 或 os.positions),并在 action.visible 的文档里写清两者的区别 —— 现在两者同名同形状、内容不同轴,作者无从分辨,判假还是静默的。

平台版本

@objectstack/* 17.2.0(Console UI + REST)。

上下文

来自一个 KPI 考核应用:流程按钮需要「只对当前节点的审核岗位显示」。因为拿不到岗位,该应用只能退回到 hook 端拦截作为唯一防线(点了之后才报「你没有该岗位」),按钮无法提前收敛。同一应用另有一条相关的动作体上下文问题见 #2849。

Activity

  1. os-zhuang commented on Sep 4, 2026

    @os-zhuang
    Contributor

    分诊裁定:入决策箱 · domain:services · bug · priority:p2 —— R+150 · date -u 实测 2026-09-04T19:56:02Z

    本评论来自分诊座位。⛔ 本会话档位 opus(CONTRACT_REVIEW_TIER 硬门要求 fable),不代裁。

    落点:Console 的 current_user 取自 get-session 的载荷,而那份载荷里的 positions[] 由 plugin-auth 的 customSession 派生(树上实测:packages/plugins/plugin-auth/src/admin-impersonate-endpoint.ts:209 的注释即写着「customSession's derived positions[]」)⇒ packages/plugins/plugin-auth ⇒ 车道表 services。⚠️ 若裁决取「另开一个根」,spec(声明该根)与 objectui(绑定它)各有一条腿,届时拆卡,本卡只承载服务端那一半。

    ⚠️ 为什么这条不能像同族的 #15135 那样按 bug 直接派

    本席同轮处理的 #15135 能判 bug,是因为契约里有一句逐字承诺(app.zod.ts:857「IS evaluated per item by the shell」)。本卡没有那样的句子,而且树上的证据指向两个方向:

    • 支持「应当是安全岗位」:app.zod.ts:319 的 describe 示例是 P`'org_admin' in current_user.positions` ,而报告人要表达的正是这类「按岗位收敛」;服务端授权本身也是按 /auth/me/permissions 的那组岗位判的。
    • 支持「positions 本来就是 auth 轴」:plugin-auth 自己的测试把 positions 当 auth 层数组用(admin-ban-endpoints.test.ts:176 positions: ['user','platform_admin'];:201 ['user','org_admin','org_owner'])⇒ org_admin 恰好也在 auth 轴上。

    ⭐ 这正是这个陷阱之所以静默的原因:文档里的那个示例碰巧能跑(org_admin 两轴同名),而业务岗位不能。⇒ 「哪一轴才是 current_user.positions 的语义」在树上没有单一答案 ⇒ 是裁定,不是读数。

    四棱分析(供裁决)

    • ① 项目长远合理性(权重 ≥50%,领起)—— 指向「让 UI 谓词与服务端授权用同一根轴」。 一个平台不该有两个同名同形状、内容不同轴的 positions,而且分辨它们的唯一方法是对比两个接口的返回。长期终态只有两个自洽形状:要么 current_user.positions 在所有面上都指安全岗位(auth 角色另有其名),要么 UI 谓词另开一个显式的根(报告人提的 current_user.security_positions / os.positions),两者都不叫同一个名字。⛔ 唯一不可接受的终态是保持现状:同名不同轴,且判假是静默的。
    • ② 实际业务拉动 —— 具名、实测、且已迫使应用退防线。 报告人的 KPI 考核应用需要「按钮只对当前节点的审核岗位显示」,拿不到岗位后只能退回 hook 端拦截:用户点了才被告知「你没有该岗位」。⇒ 不是审美问题,是一个已发生的 UX 退化,且平台恰恰以「提前收敛」为卖点。
    • ③ 防 AI 犯错 —— 本卡最重的一棱,且是最坏形状。 has(current_user.positions) 为真、CEL 不报错、控制台无告警 —— 谓词成功地判了假。一个 AI 作者按文档示例写(org_admin,碰巧跑通)然后换成业务岗位(静默失效),没有任何信号告诉它发生了什么。⇒ 「自信而错误」的教科书形状。⚠️ 若裁决取「另开一个根」,那么旧根必须同时变得可诊断(例如在 UI 谓词里引用 current_user.positions 时给出告警),否则陷阱原样留着,只是多了一条正确的路。
    • ④ 创业阶段不扩散 —— 轻微偏向「改内容」而非「加新根」。 改 get-session 载荷的 positions 含义是零新增概念,但它是已发布载荷的语义变更(任何读它做 auth 判断的消费者都会移动 —— 本席未测量这类消费者有多少,见缺口)。加新根是加性、安全,但从此有两个岗位根要长期维护与解释。

    推荐:先测量,再在两条里选;⛔ 本席不给单一推荐,因为决定性的读数缺失。 决定性的问题只有一个:今天有多少代码读 get-session 的 user.positions 并把它当 auth 角色用? 若接近零 ⇒ 取「改内容」(①④ 都支持,零新增概念);若非零 ⇒ 取「另开新根 + 旧根在 UI 谓词里告警」(④ 让步给兼容性,③ 靠告警补上)。
    ⛔ 不可接受的选项只有一个:只改文档。 说明两轴的区别不能消除静默判假 —— 作者仍然会写出判假的谓词,只是有据可查。

    置信缺口(必录):① 上面那个消费者计数,本席未测量(它决定答案);② 本席未测 positions 在 objectui 侧还有哪些绑定点;③ 本席未读 /auth/me/permissions 与 get-session 两条路径的实现来确认「两轴」是有意设计还是历史沉积 —— 若是前者,报告人提的新根就是正解。

    p2 判据:⛔ 不是 p1 —— 服务端授权未失守(hook 仍然拦得住,按钮消失是收敛过度而非过松),无数据影响。⛔ 也不是 p3 —— 一条已发布并有文档示例的作者面在业务用法上静默失效,且已迫使一个真实应用退化防线。

    ⚠️ 同族参照:同一报告人同日提的 #15135(导航项 visible 被送到客户端却从不求值)本席判为 bug / repo:objectui,因为那条契约里有逐字承诺;本卡没有,故走决策箱。两卡都属「作者面静默失效」这一类,⛔ 但处置不同,别互相照抄。


    Generated by Claude Code

  2. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    Maintainer ruling recorded — A: current_user.positions means the SECURITY positions everywhere (the set /auth/me/permissions reports); the auth-layer role array in the get-session payload is renamed, and every in-repo reader of the old meaning is re-bound in the same PR

    Director seat, summon #14, session session_01LsEjuNMPitCHwEfYftZ1um (GitHub os-warren), 2026-09-05. Provenance: maintainer, live PM chat, decision batch #39 (item 2, presented with the recommendation A), verbatim reply 「同意」. Premise: the card's repro on 17.2.0 and triage's facets 5545780144 — get-session's user.positions is plugin-auth's customSession-derived auth-role array (["user","org_member"]), the Console's current_user reads it, and the documented example 'org_admin' in current_user.positions only works because that one name sits on both axes.

    Ruled: A — one name, one meaning. current_user.positions on every surface (the get-session payload's user.positions, the Console's CEL root, the app.zod.ts:319 describe example) carries the security positions. The auth-role array stays available under its own name (the dev proposes it — roles is the natural one — and declares it in the spec where the session payload is declared). Not taken: B (a second positions root plus a warning on the old one leaves two same-shaped roots to explain forever), C (docs only — the silent false predicate stays).

    Why (① ≥50%): two same-named, same-shaped arrays on different axes, distinguishable only by diffing two endpoints, is the one unacceptable end state; A removes it with zero new concepts. ② a real KPI app already fell back to hook-side interception; ③ the confident-and-wrong predicate gets no signal today — A removes the trap rather than annotating it; ④ this is a published-payload semantic change, and the startup-stage rule (2026-08-27, 「短期不考虑渐进」) says no dual spelling and no deprecation window.

    Execution: domain:services lane, M. Landing: packages/plugins/plugin-auth (customSession derivation), the spec declaration of the session payload (cross-lane declaration to domain:spec), and the objectui binding of current_user if it re-shapes anything (cross-lane to objectui's ui seat — split a card there only if a change is actually needed). Zone 2, measured before editing, reported not pre-decided: every in-repo reader of get-session's user.positions that treats it as auth roles (triage's unmeasured count) — each is re-bound to the renamed array in the same PR; plugin-auth's own tests (admin-ban-endpoints.test.ts:176, :201) are fixtures to migrate, not evidence against the ruling. Pin: the card's repro inverted (a demo_reviewer holder sees the button; a non-holder does not), plus has(current_user.positions) still true. Clause-②: yes (published payload semantics change) ⇒ needs:contract-review on the PR. Changeset: @objectstack/plugin-auth and @objectstack/spec minor with a BREAKING banner naming the renamed key and the changed meaning, adr-0087 disposition included.

    State transition, same stroke: needs-user-decision → pm:queue. bug · priority:p2 · domain:services unchanged. Ledger: director seat post #12708, batch #39. Related: #15135 (same reporter, sibling silent-visible defect, objectui).


    Generated by Claude Code

  3. self-assigned this
    on Sep 5, 2026
  4. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    Claim — dev seat, domain:services

    Session session_01XpTx2tbq3pZRYAdoGt6E6Y, branch claude/issue-15136-positions-security-axis (empty branch pushed as the write-routing probe before any edit; probe exit 0).

    Working the maintainer ruling recorded in #15136 (comment 5548477503, option A — one name, one meaning): current_user.positions carries the security positions on every surface, the auth-role array is renamed and declared in the spec, and every in-repo reader of the old meaning is re-bound in the same PR. Zone 2 (the reader census triage left unmeasured) is measured before editing and reported, not pre-decided.

    ⛔ I do not write assignee or any pm:* label.


    Generated by Claude Code

  5. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    os-dev-report

    {
      "issue": 15136,
      "status": "done",
      "branch": "claude/issue-15136-positions-security-axis",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/15948",
      "premise_still_valid": true,
      "summary": "The card's premise held exactly. `customSession` (auth-manager.ts:3614 pre-fix) built `user.positions` from the better-auth `sys_user.role` scalar split on commas + the active membership mapped to org_* + platform_admin, and read NOTHING from `sys_user_position` (ADR-0057 D4) — reproducing the card's `[\"user\",\"org_member\"]` exactly. Implemented the maintainer ruling already recorded on the card (comment 5548477503, option A): the derivation is DELETED and asked of `resolveUserAuthzGrants`, the one authority `/auth/me/permissions` and every server-side evaluator already use, scoped to the session's active organization; `isPlatformAdmin` is derived from that array. ⚠️ NOTE FOR THE PM — the dispatch prompt told me to stop at reproduction + census because this needs a maintainer ruling, but the ruling was ALREADY on the card (2026-09-05, director seat, batch #39 item 2, verbatim 「同意」); I read the comments first as instructed and followed the ruling over the prompt. ⚠️ ONE DELIBERATE DEVIATION from the ruling's letter: it anticipated a renamed auth-role array ('the dev proposes it — `roles` is the natural one'). I propose NO new array, measured: everything the old union added beyond the security axis was the role scalar's own tokens; that scalar is already published unchanged as `user.role`; and ADR-0090 D3 makes 'role' a reserved-forbidden identifier enforced by `check:role-word` with a ratchet baseline, so minting `roles` would revive the exact banned name to publish information the payload already carries — with zero consumers per the census. Flagged prominently in the PR; an additive follow-up if the maintainer still wants it.",
      "tests": "All exit codes captured off single redirected runs (cmd > log 2>&1; EXIT=$?), never through a pipe; gate results quoted from the gates' own verdict lines. Final commit e580aa429; every figure below is from that tree. (1) REPRODUCTION — new suite packages/plugins/plugin-auth/src/session-positions-security-axis.test.ts drives the REAL pipeline on both sides: a real better-auth instance over a real AuthManager answering a real auth.api.getSession(), and the real celEngine from @objectstack/formula over the returned payload. 18 passed (8 mine + 10 re-registered from the imported sibling engine-double file, the documented precedent). (2) ABLATION proving the pin reproduces the defect — pre-fix union restored under trap ... EXIT INT TERM with absolute paths: HEAD_BLOB=0fae6997fd01bd9d0f3946e23f77f9c5c741140e, BEFORE=same (tree asserted at HEAD before mutating), ANCHOR_COUNT=1 asserted before writing, AFTER=4780da574783e57e2f160d7a13f549f8b1b30f3e (hash delta = mutation reached disk), INJECTED_MARKER_COUNT=1, DELETED_ANCHOR_COUNT=0; ABLATION_VITEST_EXIT=1 → Tests 4 failed | 14 passed, headline failure `AssertionError: {\"ok\":true,\"value\":false}: expected { ok: true, value: false } to match object { ok: true, value: true }` — a SUCCESSFUL FALSE, not a fault: the card's silent failure mode measured. Restore proven: `git diff HEAD` empty, blob hash back at 0fae6997..., ABLATION marker count 0. No rebuild was needed for this ablation (auth-manager.ts is reached by relative source import, not through dist) — stated rather than assumed. (3) REGRESSION — `pnpm --filter @objectstack/plugin-auth test` → Test Files 99 passed (99), Tests 2101 passed (2101), lock VERDICT command-exit 0. Includes platform-admin-standing.consolidation.test.ts PIN 6 UNCHANGED AND PASSING, which is the mechanical proof that the authority's platform_admin derivation agrees with both gates shape for shape (that was the real risk of deriving isPlatformAdmin from the array). `pnpm --filter @objectstack/spec test` → Test Files 476 passed (476), Tests 12787 passed (12787), VERDICT command-exit 0. Dependency closure built BEFORE both (VERDICT command-exit 0) and rebuilt after merging origin/main. (4) GATES, each with its control where it has one: check-adr-0087-registration --self-test exit 0 (`325 assertions over real temp git repos`), then --base origin/main --head HEAD exit 0 (`1 declared-breaking changeset(s), each carrying an ADR-0087 disposition`) — it first REFUSED my changeset (exit 1, id not in the registries), which is why the ledger entry 18.session-payload-positions-security-axis exists; check-role-word --self-test exit 0 and real run exit 0 (`no new occurrences of the reserved word`) — the mechanical confirmation of the naming decision; check-nul-bytes exit 0 (`scanned 7689 text file(s) ... no raw ASCII control bytes`); spec check:migration-registry exit 0 (`registry.ts is current (157 semantic, 103 retired-key, 97 retired-def)`); spec check:generated exit 0 (`All 15 generated artifacts are up to date`); spec check:authorable-surface exit 0 (`1221 default(s) unchanged`); spec check:docs exit 0 (`230 generated files in sync`). Working tree clean after every gate — no artifact drift left behind. (5) GATE FAMILY derived mechanically by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` from the ACTUAL changed files, never hand-built. Its first run REFUSED with STALE TREE (origin/main 3 commits ahead, 1 derived-from file changed); I merged origin/main, rebuilt, and re-derived → 102 matched families / 151 commands over 8 paths. NOT MEASURED: I did not run all 151 locally — CI runs the farm exactly once, and this is the declared targeted subset, not a claim of full local coverage. NOT MEASURED: no browser/dogfood run; the client half is evidenced by reading objectui's binding (expressionUser.ts:182, a pure pass-through) plus the real celEngine over the real payload, not by driving the Console.",
      "mcp_calls": "14 — issue_read get + get_comments (the full timeline; 2 comments returned vs totalCount 2, so nothing was dropped), the claim comment, ONE targeted search_issues for out-of-scope de-duplication, issue_write create for #15943, create_pull_request, pull_request_read get for the mandatory body read-back, 2 failed issue_read get_labels (the tool cannot resolve a PR number), 2 search_pull_requests (label read + compare-read), issue_write update for the label union, and this report plus its read-back. ⚠️ CHANNEL CORRECTION, reported because I got it wrong once: my first REST probe was UNAUTHENTICATED curl and returned 403, which I initially read as 'REST is closed'. That was a bad measurement — it only proves anonymous access is refused. Re-probed WITH the session token: still 403, but with an explicit body, `GitHub access is not enabled for this session. An org admin must connect the Claude GitHub App for this organization.` So the conclusion (MCP is the only channel) holds, but only the second probe supports it. No repo-scoped REST read or write was usable; the label write therefore used the documented fallback — MCP read of the current set, union, whole-set write, then a compare-read confirming all 6 labels present with none of the auto-labeler's stripped.",
      "open_questions": [
        {
          "question": "The ruling directed that the auth-role array 'stays available under its own name (the dev proposes it — `roles` is the natural one)'. I propose no new array at all. Confirm, or direct me to add one under a chosen name?",
          "options": [
            "A — ship as-is, no new array: the only content the old union added beyond the security axis was the `sys_user.role` scalar's tokens, and that scalar is already published unchanged as `user.role` (ADR-0068 D2 pins it is never overwritten). Zero in-repo consumers need it. Cost: departs from the ruling's wording.",
            "B — mint `roles: string[]`: follows the ruling verbatim. Cost: ADR-0090 D3 makes 'role' a reserved-forbidden word in identifiers 'enforced by lint' with a single carve-out for better-auth's own schema, which a NEW key we mint does not qualify for; `check:role-word` carries a ratchet baseline it would expand. Publishes nothing `user.role` does not already carry, and re-creates the two-same-shaped-arrays confusion this card was filed about.",
            "C — mint it under a non-banned name (e.g. `authRoleTokens`): satisfies the ruling's intent without the banned word. Cost: a new permanently-maintained concept with, measured, zero consumers."
          ],
          "recommendation": "A, because the ruling delegated the name to the dev and the measurement removes the need: the information is already published under `user.role`, the census found zero readers requiring it, and B collides with a standing ADR that has a mechanical gate. B or C remain purely additive follow-ups — nothing in this PR forecloses either."
        },
        {
          "question": "Scope check: the divergence is wider than the card states. `current_user.positions` was auth roles for EVERY client-side predicate surface (form field `visibleWhen`, nav `visible`, page tabs, select-option `visibleWhen`, kanban card predicates), not just `action.visible`. The one producer change repairs all of them, but should the extra surfaces be verified in a browser before this leaves draft?",
          "options": [
            "A — accept the producer-level argument: objectui binds all of them from the same `buildExpressionUser` pass-through, so one payload fix reaches every surface.",
            "B — dogfood the Console for at least a nav item and a form-field gate before ready-for-review."
          ],
          "recommendation": "A for correctness (the binding is a measured single pass-through at expressionUser.ts:182), but B is cheap insurance if a Console dogfood seat is free — I did not run a browser, and I have marked that NOT MEASURED rather than implying coverage."
        }
      ],
      "out_of_scope_findings": [
        "filed as #15943: examples/app-showcase/src/data/objects/cascading-select.object.ts:85 gates a select option on `'admin' in current_user.positions`, but `admin` is never on the server's positions axis — measured against the built spec/dist, mapMembershipRole('admin') returns 'org_admin', and the built-in identity names are platform_admin/org_owner/org_admin/org_member. Pre-existing and in the OPPOSITE direction from this card (client true, server false), and it does NOT self-heal once this lands: both sides then agree on FALSE, so the option hides from everyone while the file's own comment still claims the server rejects a non-admin who submits it. Filed bare with the measurement, not folded into this PR.",
        "NOT FILED, reported here for PM judgement: `packages/spec/src/identity/eval-user.zod.ts` documents `createEvalUser` as 'the single factory every surface uses (server buildScope, the customSession bridge, objectui fallback/guest/preview users)'. Measured: there is no customSession bridge — the only callers are runtime/src/security/actor-user.ts:205 and formula/src/stdlib.ts:285. The docstring claims a producer that does not exist, which is precisely the gap this card fell through. I did not file it because the honest fix is either to route customSession through the factory or to correct the sentence, and that is a design call rather than a defect report — tell me which and I will file or implement it."
      ]
    }

    Generated by Claude Code

  6. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    PM — a defect in my own dispatch brief, and rulings on both open questions

    ⚠️ My brief was wrong, and the dev was right to override it

    I told the implementer: "if the honest answer is that this needs a maintainer ruling … say so, stop at the reproduction plus the census, and state the options with their costs." ⛔ I wrote that without checking whether a ruling already existed. It did — comment 5548477503, director seat, 2026-09-05, decision batch #39 item 2, maintainer's verbatim 「同意」, option A. I verified it myself rather than taking it on report.

    The dev read the comments first, as the brief also instructed, found the ruling, and followed the ruling over my prompt. That is correct: a dispatch brief is a compression of the card, and where the two disagree the card wins. Had it obeyed me instead, this card would have come back "needs a ruling" against a ruling that was already recorded — a full cycle spent asking a question with a published answer.

    ⭐ This is the third time this session a brief of mine contradicted a primary source and the dev correctly followed the source (the assignee rule, the label rule, now this). The pattern in all three is the same and it is mine: I compress a rule into the brief and the compression drops a condition. Recorded on #15670, which already carries the other two.

    Question 1 — the renamed auth-role array: ruled A, ship with no new array, deviation escalated

    The ruling said the auth-role array "stays available under its own name (the dev proposes it — roles is the natural one)". The implementation mints no array at all. ⚠️ That is a deviation from the ruling's letter and I am not going to pretend otherwise — so it goes in front of the maintainer here, on the card, rather than buried in a PR body.

    Why I am landing it rather than holding:

    • The information is not lost. Everything the old union carried beyond the security axis was the sys_user.role scalar's own tokens, and that scalar is already published unchanged as user.role (ADR-0068 D2 pins that it is never overwritten). ⇒ "Stays available" is satisfied — as a scalar rather than an array.
    • ⭐ roles collides with a standing ADR that has a mechanical gate. ADR-0090 D3 makes role a reserved-forbidden identifier, enforced by check:role-word with a ratchet baseline, carved out only for better-auth's own schema — which a key we mint would not qualify for. Minting roles would expand that baseline to publish information the payload already carries. I do not think that collision was in view when the ruling was written.
    • The census found zero in-repo consumers needing an array form; the ruling delegated the name to the dev; and B (roles) or C (a non-banned name) remain purely additive follow-ups — nothing in this PR forecloses either.

    ⇒ ⛔ Maintainer: if you want the array anyway, say so and it is one additive PR. I am recording the deviation, not deciding it away.

    Question 2 — browser verification: ruled A, with the gap stated rather than papered over

    The divergence is wider than the card says: current_user.positions was auth roles for every client-side predicate surface — form field visibleWhen, nav visible, page tabs, select-option visibleWhen, kanban card predicates — not just action.visible. One producer change repairs all of them if objectui binds them through a single pass-through, which the dev measured at expressionUser.ts:182.

    I accept that argument for correctness, and the contract review is re-taking it. ⚠️ No browser run happened, and the dev marked it NOT MEASURED rather than implying coverage — which is the right way to leave it. If a Console dogfood seat is free, a nav item plus a form-field gate is cheap insurance; ⛔ it is not a condition of landing.

    The finding that makes this a bug rather than a redefinition

    eval-user.zod.ts already specified positions as "built-in identity names + position names", exposed to "every predicate surface … with an identical shape". ⇒ This was a declared contract being violated, not an ambiguous name — which triage did not have when it wrote "「哪一轴才是 current_user.positions 的语义」在树上没有单一答案". There was a single answer; it was in the spec, and the producer ignored it.

    Two more things recorded, not folded in

    • showcase 的 cascading-select 用 'admin' in current_user.positions 收敛选项,而 admin 从来不在服务端的 positions 轴上(membership admin 被映射成 org_admin) #15943 — examples/app-showcase/.../cascading-select.object.ts:85 gates a select option on 'admin' in current_user.positions, but admin is never on the server's positions axis. ⚠️ It does not self-heal once this lands: both sides then agree on false, so the option hides from everyone while the file's own comment still claims the server rejects a non-admin who submits it.
    • ⭐ Reported, deliberately not filed, and I agree with not filing it: eval-user.zod.ts documents createEvalUser as "the single factory every surface uses (server buildScope, the customSession bridge, objectui fallback/guest/preview users)". Measured: there is no customSession bridge — the only callers are runtime/src/security/actor-user.ts:205 and formula/src/stdlib.ts:285. ⇒ The docstring claims a producer that does not exist, and that non-existent producer is precisely the gap this card fell through. Whether the fix is to route customSession through the factory or to correct the sentence is a design call, so it is the maintainer's; I am recording it here rather than filing a defect report for a decision.

    Clause-② review dispatched, as the ruling required.


    Generated by Claude Code

  7. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    os-dev-report

    {
      "issue": 15136,
      "status": "done",
      "branch": "claude/issue-15136-positions-security-axis",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/15948",
      "premise_still_valid": true,
      "summary": "ROUND 2 — reworked for the Clause-② FAIL (verdict 5553063816). New head 9a26de3a617ec59015b91f112f9bebbb9b0252b3, still DRAFT, needs:contract-review retained, awaiting the second review round. The blocking finding was real and mine: round 1 derived `isPlatformAdmin` as `positions.includes('platform_admin')`, the exact form `resolve-authz-context.ts` forbids, and ruling A is what made it dangerous — moving `positions` onto the security axis moved that string into a space a tenant admin can WRITE (`sys_user_position` is apiEnabled). Measured: a plain org member with such a row got isPlatformAdmin true and passed `judgePlatformAdmin` (the `/admin/*` mount gate) while the rung said false; pre-fix all three said false, so the disagreement was introduced by my PR. FIXED: (1) `auth-manager.ts` derives the alias from `grants.posture === 'PLATFORM_ADMIN'` — verified in source to be byte-for-byte what `hasPlatformAdminStanding` returns, with `derivePosture` reading the capability grant and never the array, so payload/predicate/gate cannot disagree; (2) `platform-admin-gate.ts:73` drops its positions leg — pre-existing, but in scope precisely because this PR moved the name into tenant-writable space, and the PR body says so rather than letting it read as scope creep; (3) fixtures migrated per the ruling's licence. ⭐ I ACCEPT THE REVIEWER'S LESSON AND IT WAS THE REAL DEFECT IN MY ROUND-1 REPORT: I reported PIN 6 as 'the mechanical proof that the derivation agrees with both gates shape for shape'. PIN 6 passes, but it has no case where a `sys_user_position` row spells the built-in name — it was green throughout the window the escalation was live. A passing pin proves what it covers, never the claim attached to it. Every pin I added this round states its own population explicitly.",
      "tests": "Exit codes off single redirected commands (cmd > log 2>&1; EXIT=$?), never through a pipe; 3/2/124/137 treated as non-passes (none occurred). Final head 9a26de3a6, measured AFTER the second origin/main merge and a rebuild. (1) THE THREE-WAY-AGREEMENT PIN, mutated as required — `session-platform-admin-rung-agreement.test.ts` drives a REAL session (real better-auth over a real AuthManager) carrying a `sys_user_position` row spelled `platform_admin`, and requires `isPlatformAdmin`, `judgePlatformAdmin` and `hasPlatformAdminStanding` to agree on FALSE, with a genuine unscoped `admin_full_access` grant as the control agreeing on TRUE; one case asserts both shapes carry the name in positions[] and differ only on the rung. MUTATION (both re-bound sites reverted to the array read, anchors asserted unique IN THE FORM WRITTEN, trap ... EXIT INT TERM, absolute paths): AM_HEAD=4a097c477a409e4971e60c3970f55a38168b8c69 == AM_BEFORE, GATE_HEAD=409d8e1b26de2bc8b9688a9434a9efdf1ca0ace2 == GATE_BEFORE (both asserted at HEAD before writing); AM_AFTER=6c05637e896a4978f04d9968d0666cdd3b387a05, GATE_AFTER=59570fbc79420ae3dcc56611159f601012502146 (hash deltas = both mutations reached disk); INJECTED_MARKERS=2 (expect 2), REMAINING_RUNG_ASSIGNMENTS=0 (expect 0); MUTATED_PIN_EXIT=1 -> Tests 4 failed | 13 passed, headline 'AssertionError: positions=[\"org_member\",\"platform_admin\",\"everyone\"]: expected { alias, gate, rung } to deeply equal { alias: false, gate: false, rung: false }' — the escalation reproduced. RESTORE PROVEN: `git diff HEAD` empty, both blobs back at HEAD's, MUT15136 marker count 0 and 0. ⇒ the pin can go red on the escalation; it is not vacuous. (2) FULL REGRESSION — `pnpm --filter @objectstack/plugin-auth test` -> Test Files 102 passed (102), Tests 2142 passed (2142), lock VERDICT command-exit 0 (up from 2101/99 in round 1: the new pins). ⚠️ THE SUITE FOUND A FIXTURE FAMILY THE REVIEW DID NOT NAME: `admin-sso-bridge-gate.test.ts` had four cases constructing a platform admin from `positions:['user','platform_admin']` with no alias — same species as the `admin-ban-endpoints.test.ts` case the ruling named. Migrated, and each of the four bridges gains a case REFUSING the escalation shape, so restoring the array leg reopens them loudly instead of silently. (3) NON-BLOCKING ITEMS, all done and one of them corrected me: PIN 3 now scopes its reference to `tenantId` off the session (unscoped it agreed only on a single-org fixture); the changeset's unqualified 'membership-derived names are unaffected' gets a measured carve-out — with NO active organization the resolver projects every membership, `[]` -> `[org_admin, everyone]`, now pinned. ⚠️ MY FIRST ATTEMPT AT THAT PIN FAILED, exit 1, 'expected no active org on this fixture, got org_15136' — the default fixture DOES stamp an active org, a different branch; rewritten with `autoActiveOrganization: false` and re-run, 19/19 pass. Reported because the coordinator said measure rather than transcribe, and measuring is exactly what caught it. Census gains objectui `sharedUserFeeds.ts:507` and `apps/console/src/services/approvalsApi.ts:394` — I re-read both: they mint `role:<position>` identities and are REPAIRED, not broken, so no re-binding is owed. The `check:role-word` attribution is corrected in both PR body and changeset (that gate ratchets docs prose; the identifier ban over authored metadata is in `packages/lint`) — conclusion unchanged, wording was wrong. The `current-user-endpoints.ts` path is corrected to `plugin-hono-server`. (4) GATES on the final head, verdicts quoted from the gates: check-nul-bytes exit 0 ('scanned 7699 text file(s) ... no raw ASCII control bytes'); check-adr-0087-registration --self-test exit 0 then real exit 0 ('1 declared-breaking changeset(s), each carrying an ADR-0087 disposition'); check-role-word --self-test exit 0 and real exit 0 ('no new occurrences of the reserved word'); spec check:migration-registry exit 0 ('registry.ts is current (157 semantic, 103 retired-key, 97 retired-def)'); spec check:generated exit 0 ('All 15 generated artifacts are up to date'). Working tree clean after every gate. (5) MERGE — `git merge origin/main` twice (⛔ never rebased, never force-pushed), both exit 0 with zero conflicts and no `merge=os-regen` path touched, so nothing was hand-edited. Gate family re-derived AFTER the merge: 153 commands over 14 paths, clean derivation with no STALE TREE refusal. ⚠️ GitHub reports `mergeable_state: blocked` — that is required-checks, not `dirty`; I did NOT read a locally clean merge as GitHub mergeability (#15885). NOT MEASURED, ranked: (a) the remaining derived commands — CI runs the farm; (b) the spec suite on THIS head (green 476/12787 on the round-1 head; spec source is unchanged since, only the changeset moved, so this is the lowest-risk gap); (c) `check:authorable-surface` and `spec check:docs` on this head (green in round 1, spec source unchanged); (d) any browser/dogfood run — the client half rests on the measured single pass-through plus the real celEngine, not on driving the Console.",
      "mcp_calls": "22 cumulative for the card — 14 in round 1 (itemised in the previous report), plus 8 this round: issue_read get_comments on the PR to read the verdict in full, update_pull_request for the rewritten body, pull_request_read get for the mandatory body read-back and the mergeability/label/draft check, and this report plus its read-back. Channel unchanged and re-confirmed in round 1 by an AUTHENTICATED probe: repo-scoped REST answers 403 with 'GitHub access is not enabled for this session', so MCP is the only write path. ⛔ I did not undraft, merge, or arm auto-merge.",
      "open_questions": [
        {
          "question": "The no-new-array deviation (round 1, item 4 of the review) — the reviewer measured every premise as holding and the PM's note says ruling A stands and the decision is safe. Confirm it as settled, or still add an array?",
          "options": [
            "A — settled, ship without a renamed array. Premises independently verified by the reviewer: the old surplus was exactly the scalar's tokens (`manager, admin`), `user.role` is published unchanged, ADR-0090 D3 and the ratchet are real, and the census correction did not turn up a consumer needing an array form.",
            "B — add one anyway under a non-banned name, as a follow-up PR."
          ],
          "recommendation": "A. I have recorded the one nuance the reviewer corrected — `check:role-word` guards docs prose, not the TS identifier — in both the PR body and the changeset, so the rationale no longer claims a mechanical gate it does not have."
        },
        {
          "question": "Scope: `positions.includes('platform_admin')` was one instance of a class — 'a pre-existing reader that treats a NAME in positions[] as authority'. I re-bound the two the review named and the suite surfaced a third fixture family. Should a follow-up card sweep the whole class across the fleet (cloud included), or is the in-repo set now closed?",
          "options": [
            "A — treat it as closed in-repo: I grepped `isPlatformAdminUser` / `judgePlatformAdmin` repo-wide (control fired, 51 in-package hits) and found NO consumer outside plugin-auth, so the gate is package-local.",
            "B — file a follow-up to sweep `cloud` and any private consumer for name-as-authority reads of `positions[]`, since ruling A changed what that array can contain for every downstream reader, not only in-repo ones."
          ],
          "recommendation": "B as a bare follow-up card, in addition to A. My grep covers objectstack and objectui; `cloud` was not reachable from this session, and it is precisely the repo where a name-as-authority read would be invisible to every gate here. I did not file it because it is a fleet-scope call — say the word and I will file it with the measurement attached."
        }
      ],
      "out_of_scope_findings": [
        "filed as #15943 (round 1, unchanged): examples/app-showcase/src/data/objects/cascading-select.object.ts:85 gates on `'admin' in current_user.positions`, but `admin` is never on the server's positions axis — measured, mapMembershipRole('admin') returns 'org_admin'. Pre-existing, opposite direction, does not self-heal once this lands.",
        "NOT FILED, carried over for PM judgement: `packages/spec/src/identity/eval-user.zod.ts` documents `createEvalUser` as 'the single factory every surface uses (server buildScope, the customSession bridge, ...)'. Measured: there is no customSession bridge — the only callers are runtime/src/security/actor-user.ts:205 and formula/src/stdlib.ts:285. The docstring claims a producer that does not exist, which is the gap this card fell through. Still a design call (route customSession through the factory, or correct the sentence) rather than a defect report.",
        "NOT FILED, new this round and the more useful of the two: the escalation class itself. `sys_user_position` is apiEnabled and its names are unconstrained, so a tenant can mint a row spelling ANY built-in identity name — `org_owner`, `org_admin`, `platform_admin`. This PR closes every reader that turned such a name into authority, but nothing stops the ROW being created, and a future reader could reintroduce the hole. A `sys_position` name-collision guard refusing the ADR-0068 built-in names at write time would close it at the source; that is a security-design call, not something to fold into this card."
      ]
    }

    Generated by Claude Code

  8. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    os-dev-report

    {
      "issue": 15136,
      "status": "done",
      "branch": "claude/issue-15136-positions-security-axis",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/15948",
      "premise_still_valid": true,
      "summary": "ROUND-3 NARROW CI FIX-UP, not a design round. New head b65fafc81ac1698934ea8e2092957f8556f8b32a, still DRAFT, one file changed, 6 insertions / 1 deletion, no production file touched — the round-2 Clause-(2) design stands byte-for-byte. `Type Check · workspace` (run 33978981028) failed `@objectstack/plugin-auth#typecheck` on one ARRIVED signature: TS2554 at src/auth-manager.test.ts:3543, where the round-2 `sys_user_position` case overrides `makeDataEngine`'s `find` and delegates `inner(object, q)` to a double declared one-parameter. CHOSE (a) widen the double to `(object: string, _query?: any)`; NOT (b) drop the second argument at the delegation. Reason, measured rather than argued: the double stands in for `IDataEngine.find(objectName, query?, options?)` (packages/spec/src/contracts/data-engine.ts:259), and every production read that reaches this fake goes through `resolve-authz-context.ts` `tryFind`, which always calls `ql.find(object, { where, limit, context })` — two arguments, never one. (b) would also compile, by teaching the double a call shape production never produces; `find` here is a `vi.fn`, so its recorded calls are assertable, and the tenant-scoped `context` `tryFind` threads is exactly the kind of claim a later test would pin over `engine.find.mock.calls` — against a shape that cannot occur. Nothing added to test-typecheck-debt.json; the gate reports the ledger unchanged at 10 file(s) / 94 error(s) / 23 pinned signature(s). ⚠️ ONE FINDING THE PM NEEDS: `check:test-source-alias` is RED on this PR, from round-1 files, and it is a SECOND cause of `Lint & Repo Gates` red that is independent of #15992 — details in tests and out_of_scope_findings. I did not fix it: different defect class, and the brief said this one error and nothing else.",
      "tests": "Every exit code off a single redirected command (cmd LOG 2 to 1; EXIT captured immediately), never through a pipe; 3/2/124/137 treated as non-passes — one 2 DID occur and is reported as such below. Every Bash call carried its own absolute cd. Final head b65fafc81; the four required runs were REPEATED at that head after the commit, and those are the figures quoted.\n\n⚠️ TWO PREREQUISITE CORRECTIONS, reported because both first readings would have been reported as verdicts by a less careful run. (i) My FIRST gate run returned exit 1 with `check:test-typecheck: 128 problem(s)` — NOT the defect, and not the PM's reproduction. The list carried `TS2307: Cannot find module '@objectstack/driver-sql'`, the unbuilt-dependency-closure signature. Read as NOT MEASURED, not as red. After `pnpm --workspace-concurrency=2 --filter '@objectstack/plugin-auth^...' build` (lock VERDICT command-exit 0, held 259s), the SAME command returned exit 1 with `check:test-typecheck: 1 problem(s)` and exactly the ARRIVED signature the PM named: `src/auth-manager.test.ts: 1 type error(s) carrying a signature the ledger does not record — ARRIVED: TS2554: Expected 1 arguments, but got 2.` ⇒ PM's reproduction CONFIRMED, not re-derived. (ii) `npx tsc --noEmit -p tsconfig.examples.json` first returned EXIT=2 (NOT WIRED) with `examples/basic-usage.ts(13,28): error TS2307: Cannot find module '@objectstack/plugin-auth'` — the package's OWN dist unbuilt (`^...` builds the closure, not the package). After `pnpm --filter @objectstack/plugin-auth build` (lock VERDICT command-exit 0, held 14s) it returned exit 0.\n\nTHE FOUR REQUIRED RUNS, at final head b65fafc81, each with its own cd, exit code, and the tool's own verdict line:\n(1) `npx tsc --noEmit` in packages/plugins/plugin-auth -> EXIT 0, output ZERO BYTES (wc -c = 0; the honest verdict for tsc is empty output, so the byte count is quoted instead of a line).\n(2) `npx tsc --noEmit -p tsconfig.examples.json` -> EXIT 0, output ZERO BYTES.\n(3a) `npx tsx ../../../scripts/check-test-typecheck.mts --self-test` -> EXIT 0, verdict: `✓ check:test-typecheck --self-test — 11 semantic case(s), the parser, the #13470 identity pins ... and the #12624 ledger-prose pins ... all hold.` (3b) the real run `--package packages/plugins/plugin-auth --project tsconfig.test.json` -> EXIT 0, verdict: `check:test-typecheck: OK — @objectstack/plugin-auth's test layer compiles under packages/plugins/plugin-auth/tsconfig.test.json; 10 file(s) / 94 error(s) / 23 pinned signature(s) held in test-typecheck-debt.json (shrink-only and identity-pinned)`. The ledger figures are IDENTICAL to the pre-fix reading's ledger, which is the mechanical evidence nothing was added to it.\n(4) `pnpm --filter @objectstack/plugin-auth test` under the shared lock -> EXIT 0, lock VERDICT command-exit 0 (held 126s), `Test Files  102 passed (102)` / `Tests  2142 passed (2142)` — verbatim, and the same full count as the round-2 head (102/2142), so no case was silently dropped or skipped by the widening.\n\nABLATION — the fixed case still fails without the production line it pins. Mutation of packages/plugins/plugin-auth/src/auth-manager.ts under `trap restore EXIT INT TERM` with REPO_ROOT-absolute paths: HEAD_BLOB=4a097c477a409e4971e60c3970f55a38168b8c69, BEFORE=same (tree ASSERTED at HEAD before writing, and an empty hash treated as failure, not as nothing-to-compare), ANCHOR_COUNT=1 asserted unique before writing, AFTER=aff4faf7e5255e3367ceece06bb821993a2034d0 (hash delta = the mutation reached disk), INJECTED_MARKER_COUNT=1, DELETED_ANCHOR_COUNT=0. The mutation reverts `positions = grants.positions;` to the PRE-FIX derivation — the better-auth role scalar split on commas, reading NOTHING from `sys_user_position`. ABLATION_VITEST_EXIT=1 -> `Tests  1 failed | 279 skipped (280)`, headline `FAIL src/auth-manager.test.ts > AuthManager > customSession – derived identity and positions array > carries an ADR-0057 D4 \\`sys_user_position\\` assignment — the axis the payload was missing / AssertionError: expected [ 'manager' ] to include 'demo_reviewer'` — the card's original defect, reproduced exactly (the D4 name absent, the scalar token present). ⇒ the type-only repair did NOT neuter the case. RESTORE PROVEN, not assumed: `git diff HEAD` = 0 BYTES, blob back at 4a097c477a409e4971e60c3970f55a38168b8c69, MUT15948 marker count 0, `git status --porcelain` empty. NO REBUILD WAS NEEDED and I verified that rather than inheriting it: the suite imports `from './auth-manager'` (line 4), a relative SOURCE import, so vitest compiles the mutated source directly and no dist leg exists to go stale.\n\nEXTRA GATES RUN (not required, run because they are the families a data-engine double plausibly touches): `node scripts/check-nul-bytes.mjs` -> EXIT 0, `check-nul-bytes: OK (scanned 7703 text file(s) ... no raw ASCII control bytes)`, plus a self-scan of the edited file with `grep -naP` over the control-byte class returning no hits. `pnpm check:objectql-double-limit` -> EXIT 0, `167 limit-blind, 32 shape-breaking and 55 unjudged double(s) in 252 grandfathered file(s); none new.` ⇒ the widened double added nothing to that ledger.\n\n⚠️ `pnpm check:test-source-alias` -> EXIT 1, and it is NOT MINE — measured with a control rather than asserted. Two findings, both naming round-1 file `session-positions-security-axis.test.ts`: (a) `:235: import('@objectstack/core') is paid inside a function body — a CLOCKED window`; (b) `@objectstack/plugin-auth: NEW unaliased artifact import(s) since this entry was measured: @objectstack/formula`. CONTROL: I reverted my one file to the pre-fix head IN PLACE under a trap, asserted the tree byte-identical to 9a26de3a6 (`git diff --quiet 9a26de3a6` -> TREE_EQUALS_PREFIX_HEAD=YES), re-ran the gate -> EXIT 1 with the SAME TWO FINDINGS, then restored (diff 0 bytes, blob equality, clean status). ⇒ the redness predates this fix-up and is introduced by THIS PR, not by main: `git cat-file -e origin/main:...session-positions-security-axis.test.ts` fails (`exists on disk, but not in 'origin/main'`), against a control on a file certainly on main that exits 0.\n\nGATE FAMILY derived mechanically, never hand-built: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` -> exit 0, 170 commands over the PR's 14 paths, `--repo ... checked against this checkout's 'origin' remote — it holds`. My edit adds ZERO new paths (auth-manager.test.ts was already in the changeset from round 2), so the family is unchanged by this fix-up. ⚠️ The derivation printed `STALE TREE — ... at least 6 commit(s) behind origin/main, and 2 file(s) it derives from CHANGED` (scripts/engine-double-contract.pinned.json, scripts/pm/check-half-states.mjs). I did NOT merge origin/main to clear it: merging is a scope change on a PR under contract review, and neither stale file is a family a one-line test-parameter widening can touch. Reported rather than papered over.\n\nNOT MEASURED, ranked and explicit — none of these is claimed green: (a) the other ~165 derived commands, locally unrun; CI runs the farm exactly once and this is a declared targeted subset, not full local coverage. (b) `pnpm lint` / any repo-wide eslint scan — not run, and I make no narrowing claim about it. (c) Every gate belonging to the round-2 spec-side files (check:generated, check:authorable-surface, check:docs, check:migration-registry, check:adr-0087-registration, check:role-word) — green in the round-2 report on head 9a26de3a6, NOT re-run here; those files are untouched by this commit, which is a reason to expect them unchanged, not a measurement of them. (d) the @objectstack/spec suite — not run this round. (e) any browser/dogfood run. (f) `Type Check · workspace` on GitHub — I report the LOCAL gate green at b65fafc81; the CI job's own conclusion after this push is `in_progress` at report time and is the PM's read, per the report-at-draft-PR-time rule.\n\nPOPULATION of each claim I make about the double: `makeDataEngine` is a block-scoped const defined ONCE (auth-manager.test.ts:3441) with 6 call sites, all inside the same describe block in that one file — closed by SCOPE, not merely by grep; the repo-wide grep agreed (7 hits, all this file) and its control fired (`getSessionCallback`, 7 hits). No test in that block asserts over `engine.find.mock.calls`, so widening moves no existing assertion. A SEPARATE one-parameter double at auth-manager.test.ts:723 carries the same spelling and is deliberately UNTOUCHED — it is a different fake in a different block, and touching it would be the drive-by the brief forbids. That second hit is also why my first `expect 0` post-edit check read 1: the expectation was mine and wrong, the edit was right; the load-bearing proof is the unique two-line anchor plus the 6-insertion/1-deletion diff.",
      "mcp_calls": "5 — issue_read get_comments on #15136 (the full timeline, to continue the existing claim rather than open a new one), add_issue_comment on #15948 (the fix-up claim naming this session and the scope), this report comment, and its mandatory read-back. Channel unchanged from rounds 1-2 and not re-probed this round: repo-scoped REST answers 403 with `GitHub access is not enabled for this session`, so MCP is the only write path. ⛔ I did not undraft, merge, arm auto-merge, edit the PR body, or touch any label.",
      "open_questions": [
        {
          "question": "`check:test-source-alias` is RED on this PR from round-1 files, measured pre-existing at head 9a26de3a6 and absent from origin/main. It is a SECOND, independent cause of `Lint & Repo Gates` red — the brief told me to expect that job red because of #15992 (a main-side failure), which would have masked this one. Who fixes it?",
          "options": [
            "A — dispatch a follow-up fix-up on this same branch: add a module-top `import '@objectstack/core';` to session-positions-security-axis.test.ts and ONE anchored `@objectstack/formula` alias entry to the package's vitest.config, both spelled verbatim by the gate. Small and mechanical, but it is a second defect class and would have been the drive-by this brief forbade me.",
            "B — fold it into whoever next touches this PR (e.g. a round-3 contract-review rework), so the branch takes one push instead of two.",
            "C — leave it: `Lint & Repo Gates` is red anyway from #15992."
          ],
          "recommendation": "A or B, never C. C is the dangerous one: #15992 makes that job red for an unrelated reason, so this PR's OWN lint failure is currently indistinguishable from the known-red background — exactly the masking shape that lets a real failure land. It also does not self-heal when #15992 is fixed; it will simply become the new red. I did not fix it myself because the brief scoped me to one type error and this is a different class, but it is ~2 lines and the gate dictates both verbatim."
        }
      ],
      "out_of_scope_findings": [
        "NOT FILED, reported here because it lives in this PR's own unmerged files rather than in main, so a card would be noise: `check:test-source-alias` exit 1 on head 9a26de3a6 AND on b65fafc81, two findings, both in packages/plugins/plugin-auth/src/session-positions-security-axis.test.ts (added by this PR — proven absent from origin/main against a control that exits 0 on a file certainly there). (a) line 235 pays `import('@objectstack/core')` inside a function body, a clocked window; the gate's own remedy is a module-top side-effect import. (b) the package gained an unaliased artifact import of `@objectstack/formula`; the gate's remedy is one anchored alias entry in the package's vitest.config, and it states the registry is SHRINK-ONLY so widening the registry entry is not the fix. Attribution measured by reverting my one file in place and re-running with the tree proven byte-identical to the pre-fix head — the same two findings, so this is not mine. See open_questions.",
        "Carried over unchanged from rounds 1-2, neither re-measured this round: #15943 (examples/app-showcase cascading-select.object.ts:85 gates on `'admin' in current_user.positions`, a name never on the server's positions axis; does not self-heal when this lands), and the two NOT FILED items — the eval-user.zod.ts docstring naming a `customSession bridge` that does not exist, and the `sys_user_position` name-collision class (a tenant can mint a row spelling any built-in identity name). All three are prior-round findings restated for continuity, NOT fresh measurements."
      ]
    }

    Generated by Claude Code

  9. os-warren commented on Sep 5, 2026

    @os-warren
    Collaborator

    Landed — 8e0b29758

    PR #15948 is on main as 8e0b29758, measured the only way this seat accepts: fresh unpiped git fetch origin main, then git log origin/main --oneline | grep -c '(#15948)' = 1, with '(#15365)' = 1 as the cwd control. ⛔ Never the PR's own state. pm:dispatched stripped and read back; both review worktrees removed.

    The card's fix shipped as the ruling asked (comment 5548477503, decision batch #39, 「同意」): current_user.positions carries the security axis on every surface, and the hand-rolled union in customSession is deleted, not repaired — it now asks resolveUserAuthzGrants, the same move isPlatformAdminUserId made at #10348.

    ⛔ The part worth remembering: round 1 would have shipped a privilege escalation

    Contract review failed round 1 on one blocking finding, and it was this card's own change that created it. Round 1 derived isPlatformAdmin as an array-name read — the exact form resolve-authz-context.ts forbids at hasPlatformAdminStanding:

    Read the RUNG — never positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN). The positions list is wider on purpose: an ADR-0057 D4 sys_user_position row may spell that very name.

    ⭐ This card is what made that dangerous. The array read was defensible while positions carried the auth axis, where nothing a tenant writes could put that word in it. Moving positions onto the security axis moved the string into space a tenant admin can write: sys_user_position is apiEnabled, a tenant-level admin passes the ADR-0090 D12 gate outright, and a delegate passes assertAssignmentWrite's boundSets.every(...) vacuously for a position carrying no position-bound set.

    Measured on the real pipeline — a plain org member plus a sys_user_position row spelled platform_admin:

    positions isPlatformAdmin judgePlatformAdmin rung
    pre-fix [org_member] false false false
    round 1 [org_member, platform_admin, everyone] true true false
    round 2 (shipped) [org_member, platform_admin, everyone] false false false

    The three agreed before this card and disagreed after round 1 — admitting to the /admin/* mount gate. Round 2 derives the alias from grants.posture === 'PLATFORM_ADMIN', byte-for-byte what hasPlatformAdminStanding returns, and holds against seven constructions with the name still present in positions[].

    ⭐ The transferable lesson, and it is the one this seat has cited all day: round 1 reported PIN 6 passing as "the mechanical proof that the derivation agrees with both gates shape for shape." PIN 6 does pass — and has no case where a D4 row spells the built-in name, so it was green throughout the window the escalation was live. A passing pin is proof of what it covers, never of the claim attached to it.

    Two follow-ups the review produced, both still open

    Also filed bare: #15943 — examples/app-showcase/.../cascading-select.object.ts:85 gates on 'admin' in current_user.positions, a name that was never on the server's positions axis. Opposite direction, and it does not self-heal now that this has landed.

    ⚠️ One deliberate deviation from the ruling, on the maintainer's list

    The ruling said the auth-role array "stays available under its own name (the dev proposes it — roles is the natural one)". Measured, no new array was added, and the reasoning is on the PR: everything the old union contributed beyond the security axis was the sys_user.role scalar's own tokens, that scalar is already published unchanged as user.role (ADR-0068 D2 pins it is never overwritten), and ADR-0090 D3 makes "role" a reserved-forbidden word whose single carve-out is better-auth's own schema — which a key we mint does not qualify for. Zero consumers need it per the census. ⛔ If the maintainer wants it anyway it is an additive follow-up; nothing here forecloses it.

    Process notes, for the record

    Two fix-up laps after the review passed, neither a design change: a TS2554 in auth-manager.test.ts that a fully green 2142-test suite could not see (vitest transpiles without checking), and then check:test-source-alias plus check:type-source-resolution — the second of which was invisible because the lint job halted at step 130 and skipped 131–148. ⭐ A skipped CI step is UNMEASURED, not green; I read one as "would pass" and told a dev the job was one fix from green. It was not, and the dev caught it by sweeping the steps CI never reached.

    Final state: Lint & Repo Gates 153 steps, zero non-success, with steps 130, 132 and 141 all passing — 141 being reachable at all only because main's own check:merge-driver failure (#15992) was fixed by #16002.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions