feat(demo): staff the demo org so position-based sharing and approvals resolve to real people (#640) - #652
Merged
Merged
Conversation
…s resolve to real people (#640) On a fresh install exactly one user exists, `demo_bootstrap` claims every seeded record for them, and `sys_user_position` is empty. Measured on 17.0.0-rc.1: `sys_record_share` 0 rows, so all nine declared sharing rules granted nobody anything, and `opportunity_approval`'s `manager_review` opened with an empty approver slate while `lockRecord` held the record. `pnpm demo:staff` creates three non-admin demo users on a LOCAL dev server (NA rep, EU rep, sales manager), assigns their positions and re-evaluates every sharing rule so already-seeded records materialise grants. Who exists is a table — `src/sharing/demo-staffing.ts`; adding a person is adding a row. Deliberately a script, not metadata: nothing in the published artifact may be able to create a user, so synthetic accounts cannot reach a customer org. `test/demo-staffing.test.ts` fails if a seed dataset or a flow node ever writes `sys_user` / `sys_member` / `sys_user_position`, and the built artifact carries none of these addresses. Users are created through better-auth's admin endpoint, so they are real, loginable accounts — identity tables are `managedBy: 'better-auth'` and a row inserted around that surface would have no credential. Re-evaluating the rules is load-bearing: plugin-sharing materialises grants from a record-write hook that returns early on `isSystem` writes, and every seeded row is written with `isSystem: true`. Fixes #640. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019SS7C5SXpniKeCApxgARyf
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Author
|
顺手发现、不在本 PR 范围内、已另开 issue(Prime Directive #10):
Generated by Claude Code |
…n what actually bounds a territory rep
CodeQL (js/clear-text-logging of sensitive information, high) flagged the
success banner, which printed `email / password` for each demo user. The alert
is correct and worth fixing on its own terms: nothing is concealed by dropping
it — the passwords are declared one file away in `src/sharing/demo-staffing.ts`,
and the script already refuses non-loopback targets — but a run's stdout reaches
terminals, CI logs and pasted snippets, and "echo the credential you just used"
is the last shape a reference app should teach. The banner now names the
accounts and points at the file that declares their passwords. Measured: 0
occurrences of the password in a full run's stdout. The `password` field stays
in the table — `verify()` signs in as each demo user with it, which is the layer
that proves the three tiers actually connect.
Also corrects a claim in the staffing note that was true in outcome but wrong in
mechanism. Adding `sales_rep` does NOT stop the platform's additive baseline
from admitting the 2 EU accounts; the baseline never admitted them.
`POST /api/v1/security/explain` as the NA rep, on crm_account/read:
positions [org_member, na_sales_team, sales_rep, everyone]
permissionSets [sales_rep, member_default]
object_crud grants — read granted by [sales_rep, member_default]
owd_baseline narrows — private: rows are owner-visible only;
sharing can only WIDEN from here
depth Effective read depth: 'own' (ADR-0057 D1 — widest
across granting sets)
sharing widens — shares/rules OR-in additional rows
vama_bypass not_applicable — No View/Modify All Data bypass
So `member_default` opens the OBJECT door, not rows; the row set is OWD-private
(own — and the reps own nothing) OR-in their shares. `sales_rep` computes the
same 'own' depth and adds no bypass, so it widens nothing — it only decides what
the persona may DO. The falsifiable corollary is now a test: no set bound to a
position a territory rep holds may grant `viewAllRecords` on `crm_account`, or
the rep reads all nine and the territory grant proves nothing while the org
still looks staffed (exactly what `sales_manager` does, correctly, at 9/9).
Refs #640.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019SS7C5SXpniKeCApxgARyf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #640
Description
给 demo org 配上「人」——这是 #621 / #638 之后同一个缺口的最后一层。规则装好了、记录也能匹配上了,但没有任何人持有任何职位,所以匹配到的记录仍然不产生任何授权。
新增
pnpm demo:staff:在本地 dev server 上创建三个非管理员 demo 用户(NA rep / EU rep / 销售经理),分配他们持有的职位,再重新评估所有共享规则,让已经 seed 过的记录真正物化出sys_record_share行。为什么是脚本而不是元数据(这是 #640 的硬约束,不是风格选择)。 真实部署必须一个合成用户都不装,而唯一能让这件事「不可能」而不是「不太会」发生的办法,就是发布出去的 artifact 里根本没有能创建用户的机制。所以装配逻辑不进
objectstack.config.ts,test/demo-staffing.test.ts会在任何 seed dataset 或 flow 节点写sys_user/sys_member/sys_user_position时失败,构建产物里也搜不到这三个邮箱。顺带说明为什么「在
demo_bootstrap里加一个create_record」这条看起来最省事的路被否掉了两次:一是它随 artifact 进客户 org;二是它根本产不出可用的人——身份表是managedBy: 'better-auth'(ADR-0092),绕开那个 surface 插进去的行没有 credential,谁也登不进去。脚本走的是平台自己的POST /api/v1/auth/admin/create-user,拿到的是真账号(带sys_member成员行,可登录)。「谁存在、持有哪些职位」是一张表(
src/sharing/demo-staffing.ts)——加一个人 = 加一行。另外七个职位这轮故意留空:配人的目的是让机制可被观察,不是把组织架构填满。Type of Change
Related Issues
Fixes #640
Related to #621, #638, #622, #488, #548
Changes Made
src/sharing/demo-staffing.ts(新增) — demo org 的人员表:NA repna.rep@objectos.ai(sales_rep+na_sales_team)、EU repeu.rep@objectos.ai(sales_rep+eu_sales_team)、销售经理sales.manager@objectos.ai(sales_manager)。不从src/sharing/index.ts导出,因此不进 stack。scripts/demo-staff.ts(新增)+pnpm demo:staff— 幂等、可自检:建人 → 配职位 → 重评估所有共享规则 → 以每个 demo 用户的身份登录核对可见性,任何一层没接上就非零退出。只接受本地 URL。test/demo-staffing.test.ts(新增,15 项) — 钉住表的完整性、这轮的取舍(两个领地 + 一个经理,领导层留空)、manager_review的审批人必须有人持有、领地算术(6 / 2 / 1)、下面那条「领地边界靠什么撑住」的不变量,以及上面那条硬约束。src/flows/demo-bootstrap.flow.ts— 只加注释:说明为什么装配不能放在这里,以及它必须保持现有行为(rep 不能拥有这些客户,否则 OWD 基线本来就放行、什么都证明不了)。归属模型本身是 Ownership model mismatch: app-authoredownerlookups vs the platform'sowner_id— reassigning Owner in the UI does not move record access #548 的范围,没有改动。docs/MAINTENANCE.md§4.1 — 运行方式,以及三条会踩的坑。领地边界到底是被什么撑住的(评审追问,值得写进正文)
先纠正一句本 PR 初版里结论对、机制错的话。当时写的是「只持有领地职位的用户靠加性基线仍能读到那 2 条 EU 客户,所以补
sales_rep」,读起来像是「补上sales_rep之后基线就不放行了」。不是这样——基线从头到尾就没放行过任何一行。 以补齐sales_rep之后的 NA rep 身份实测POST /api/v1/security/explain(crm_account/read):逐层读下来,分工是清楚的:
member_default开的是「对象门」,不是「行」。 加性基线(ADR-0090 D5)让每个 org 成员在object_crud这一层拿到crm_account的 read;补上sales_rep之后这一层写的是granted by [sales_rep, member_default]——两个集合都在,基线并没有被谁「覆盖掉」。crm_account是sharingModel: 'private',OWD 基线只放行自己拥有的行,sys_record_share只能在此基础上加宽。三个 demo 用户一行都不拥有(demo_bootstrap把 seed 记录全认领给了 dev admin),所以 NA rep 的可见集合恰好等于领地规则给他物化出来的那 6 条授权。那 2 条 EU 不在他的授权里,于是看不到——不是被谁「挡回去」,而是从来没被放进来。sales_rep一行都没加宽。 它对crm_account的 grant 是viewAllRecords: false, readScope: 'own',和基线算出来的 depth 一样(depth取的是「widest across granting sets」,同宽即无变化),vama_bypass也是not_applicable。它加的是能做什么——create/edit、health_score/annual_revenue的字段可见性、导出——也就是把「一个碰巧能看到两条记录的普通成员」变成一个销售代表。所以这个 demo 不是碰巧成立的,但它确实压在一个可以被一次改动悄悄关掉的前提上:只要某个绑定到 rep 所持职位的 permission set 给了
crm_account的viewAllRecords,depth 就会被拓宽到全表,rep 会读到全部 9 条,领地授权就此什么也证明不了——而 org 看上去仍然「配好人了」。 这不是假想:sales_manager就是这么配的(viewAllRecords: true),实测她读到全部 9 条,对经理而言完全正确,对领地 demo 则是静默死亡。所以这条不变量现在有测试钉着(never gives a territory rep an org-wide view of crm_account),负向对照确认会红。另外一条测量结论
只配职位是不够的。
plugin-sharing从记录写入 hook 物化授权,而那个 hook 对isSystem写入直接 return;seed 的每一行都是isSystem: true。所以在规则被重新评估(或服务器重启走 boot backfill)之前,sys_record_share一直是空的。脚本第 3 步就是干这个的。Testing
npx vitest run:46 files / 1090 passed / 1 skipped(新增文件 15/15)objectstack lint --skip-i18n:1 warning / 13 suggestions,与 main 基线一致objectstack validate+objectstack build均 exit 0,artifact 1144.5 KB负向对照(确认新测试真的会红):把
sales_manager换成sales_director、把 rep 的sales_rep拿掉 → 4 项失败;把 rep 的sales_rep换成sales_manager(即给他viewAllRecords)→ 领地不变量失败;把demo_bootstrap的某个 claim 目标改成sys_user_position→ 身份写入守卫失败。改回后全绿。端到端(全新
--freshinstall,17.0.0-rc.1)装配前:
pnpm demo:staff之后:三条验收逐条对上:
sys_record_share从 0 变成 13,且crm_account的owner_id全部仍是 dev admin —— 收授权的 rep 不拥有这些客户(脚本会在这一点不成立时直接失败退出)。crm_account返回 6 条 NA,看不到 2 条 EU;No seeded account carries a billing_address, so both territory sharing rules match zero records on the demo dataset #638 留的那条两边都不属于的探针(Apex Logistics, SG)两个 rep 都看不到,该记录未做任何改动。审批人非空(同样的探测中已核对该 id =
sales.manager@objectos.ai)。之前这里是空名单 +lockRecord,记录锁死且产品内无法恢复。CodeQL(第二个 commit):首版的成功横幅会把
email / password原样打进 stdout,被js/clear-text-logging(high)命中。这条告警是对的,已照实修——横幅只列邮箱,并指向声明密码的src/sharing/demo-staffing.ts;实测一次完整运行的 stdout 里密码出现 0 次。没有用抑制注释,也没有把password从表里拿掉(verify()要用它以每个 demo 用户身份登录,那正是「三层真的接上了」被断言出来而不是被假设的地方)。一点如实说明:
pnpm lint的approval-approvers-may-resolve-empty仍然会提示。那是一条纯静态规则——只要节点的审批人全部路由到 group(position/team/department),它就会提示,因为成员是运行时数据。本 PR 修的是运行时那半:名单现在解析得出人。Checklist
.changeset/staff-the-demo-org.mdAdditional Notes
文件边界:只动了本轮分给本 issue 的文件。没有碰
src/data/sales.seed.ts、src/objects/account.object.ts/account.hook.ts(仅在测试里 import)、src/flows/lead-conversion.flow.ts,也没有碰任何 i18n 语言包。demo_bootstrap只加了注释,没有改行为——归属模型是 #548 的范围。顺手发现的、不在本 PR 范围内的问题已按 Prime Directive #10 另外开 issue(见下方评论),不在这里修。