Skip to content

fix(plugin-security,plugin-sharing): 写入路径补上 VAMA 旁路 —— explain 与 /data 收敛到同一判定函数 (#4647) - #5124

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-4647-vama-write-path
Aug 4, 2026
Merged

fix(plugin-security,plugin-sharing): 写入路径补上 VAMA 旁路 —— explain 与 /data 收敛到同一判定函数 (#4647)#5124
os-zhuang merged 1 commit into
mainfrom
claude/issue-4647-vama-write-path

Conversation

@claude

@claude claude Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #4647

问题

同一个 (principal, record, operation) 三元组,security/explain/data 写入路径给出相反答案:VAMA(Modify All Data)持有者 + sharingModel: 'private' 对象 + owner_id 为 NULL 的无主记录,explain 回 allowed: truevama_bypass 层写着"ownership and sharing checks are skipped",而 PATCH /data/… 回 403。把 owner_id 填上同一个 PATCH 就 200 —— 证明写入路径确实在跑那层声称已跳过的记录级归属检查。sys_attachmentcanEdit(parent) 门禁与 403 一致、与 explain 不一致。

无主行不是边角料:系统上下文的种子写入按设计就会产生(seed loader 关掉 owner_id 注入)。

裁定与实现(方案 A:写入路径缺 VAMA 旁路)

1. 一个判定函数,不是两份抄写。
PermissionEvaluator.superuserBypassSets(object, sets, { isPrivate, bit }) 成为唯一的旁路判定,返回授予该旁路的权限集名(而不是布尔值)——这样"层里报出来的 contributors"和"门禁做出的决定"物理上是同一个列表,不可能一个说有一个说没有。hasSuperuserReadBypass / hasSuperuserWriteBypass 改为委托它;explain 的 vama_bypass 层删掉自己那份内联的 objects[name] ?? objects['*'] 读法,改调同一函数。写入侧经由既有的 ISecurityService.hasWriteBypass 落到同一函数。

2. 写入门禁补查旁路(在归属/共享排除之后)。
SharingService.canEdit / canDelete 在 ownership 与 share 都失败后,通过既有的晚绑定 probe 追问一次 Modify All Data。原来它们依赖的 __writeScope === 'org' 代理是半截的:matchesOwnerScope 在看 scope 之前就先对 NULL owner 返回 false,所以无主行永远到不了那个分支。canManageShares 改用同一个私有 helper,三个门禁不再各写一遍。

sys_attachmentcanEdit(parent) 门禁与 sys_comment 门禁不需要改动就一起收敛了 —— 它们调的就是这个 canEdit。这正是"同一判定函数"这条要求的价值所在。

3. 放宽范围严格限定在 Modify 位。
viewAllRecords("View All Data")是读权力,永远不放宽写:explain 的 vama_bypass 层现在按 operation 取位(写取 modify,读取 view),只持有 View All Data 的 principal 在两条路径上都被拒,并且该层会明说缺的是哪一位(而不是沉默地报"no bypass")。probe 依旧 fail closed:没有 plugin-security、probe 抛错、principal-less、on-behalf-of,一律退回 owner-only。

4. explain payload 自洽。
记录级判定现在反映旁路:decidedBy: 'vama_bypass'vama_bypass 层带上自己的 per-record attribution、sharing 层不再在 allowed: true 旁边写"no ownership and no edit/full share grants write"。旁路不是决定因素时(owner / 有 admitting share),decidedBy 与改前一致。未改 payload 形状,packages/spec 零改动(decidedBy 枚举与 layer 的 record 字段本来就已经允许)。

没有旁路的 principal,顶层 allowed: truerecord.visible: false 并存仍然是对的、也是有意的:allowed 回答对象级问题,record 回答记录级问题,而写入路径镜像的是 record。测试里对此有断言(decision.record.visible === write.ok)。

顺带发现(同因、方向相反)

已 owned 但不属于自己的记录上,改前 explain 也和写入路径不一致——只是方向反过来:写入靠中间件 stash 的 __writeScope === 'org' 通过,而 explain 调 canEdit 时上下文里没有那个 stash,于是报"不可见"。同一个修复把这一侧也收敛了(见 an OWNED record was already consistent and stays so 用例:去掉写入侧修复后它同样变红)。

测试

新增跨路径收敛测试 packages/plugins/plugin-security/src/vama-write-path-convergence.test.ts:在一个内存引擎上装真实的 SecurityPlugin(拿它注册的 security 服务,也就是 hasWriteBypass 的真实实现)、真实的 SharingService、以及引擎对 by-id 写入真正会跑的 security → sharing 中间件链,然后对同一三元组同时打 explain 和写入,断言两者同答案。覆盖 update / delete / attachment canEdit(parent) 门禁,以及两个反向对照:非 VAMA 成员两条路径都被拒;只有 View All Data 的审计员两条路径都被拒(读则两条路径都放行)。

反向验证过:把写入侧修复 revert 掉、重建 dist 后,该文件 4 个用例转红。

pnpm turbo run test --filter=@objectstack/plugin-security --filter=@objectstack/plugin-sharing
  @objectstack/plugin-sharing:test   Test Files 12 passed (12)   Tests 273 passed (273)
  @objectstack/plugin-security:test  Test Files 34 passed (34)   Tests 731 passed (731)
pnpm turbo run typecheck --filter=@objectstack/plugin-security --filter=@objectstack/plugin-sharing
  Tasks: 18 successful, 18 total
pnpm turbo run test --filter=@objectstack/service-storage   # attachment 门禁消费方
  Test Files 19 passed (19)   Tests 260 passed (260)

packages/plugins/plugin-security 新增 @objectstack/plugin-sharing 作为 devDependency(仅测试用,无环:plugin-sharing 不依赖 plugin-security),跨路径测试需要它。

packages/spec/**content/docs/releases/** 零改动。changeset 已加。

🤖 Generated with Claude Code

https://claude.ai/code/session_015W6nhsDrz6zWQc8je12a1t


Generated by Claude Code

…ass through one shared predicate (#4647)

`security/explain` and the data write path answered one (principal, record,
operation) triple differently: a Modify All Data holder against an OWNERLESS
row of a `private` object got `allowed: true` with a `vama_bypass` layer
claiming "ownership and sharing checks are skipped", and a 403 from
`PATCH /data/…`. Filling `owner_id` in made the same PATCH succeed, so the
write path was running the record-level ownership check the layer said was
skipped; `sys_attachment`'s `canEdit(parent)` gate agreed with the 403.

Per the maintainer ruling (option A), the write path was the wrong side:

- `PermissionEvaluator.superuserBypassSets` is now THE bypass predicate,
  returning the granting set names. `hasSuperuserReadBypass` /
  `hasSuperuserWriteBypass` delegate to it, so `ISecurityService.hasWriteBypass`
  and explain's `vama_bypass` layer fold through one function.
- `SharingService.canEdit` / `canDelete` consult that bypass via the existing
  late-bound probe AFTER ownership and shares fail (the `__writeScope === 'org'`
  proxy they leaned on is only reached past a NULL-owner early return).
  `canManageShares` shares the same helper. The attachment parent gate and the
  comment gate converge for free — they call `canEdit`.
- The widening is exactly Modify-scoped: explain's layer is operation-aware
  (modify bit on writes, view bit on reads) and names the missing bit when a
  View All Data holder is refused a write. The probe still fails closed with no
  plugin-security, a throwing probe, or an on-behalf-of context.
- Record-grained explain reflects the bypass: `decidedBy: 'vama_bypass'`, a
  per-record attribution on the layer, and a sharing-layer detail that credits
  the bypass instead of reporting "no share grants write" beside `allowed: true`.

Tests: a cross-path convergence suite wires the real SecurityPlugin service, the
real SharingService and the real security+sharing middleware chain over one
engine and asserts explain == the write for update, delete and the attachment
gate, plus the non-VAMA and view-only negative controls.

Fixes #4647

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015W6nhsDrz6zWQc8je12a1t
@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 4, 2026 5:29am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file tests tooling size/l and removed documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file tests tooling labels Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/plugin-sharing.

14 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-security)
  • content/docs/kernel/runtime-services/sharing-service.mdx (via @objectstack/plugin-security)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-security, @objectstack/plugin-sharing)
  • content/docs/permissions/access-recipes.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx (via @objectstack/plugin-security, packages/plugins/plugin-sharing)
  • content/docs/permissions/explain.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx (via packages/plugins/plugin-security, packages/plugins/plugin-sharing)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-security, @objectstack/plugin-sharing)
  • content/docs/protocol/objectql/security.mdx (via packages/plugins/plugin-sharing)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-security, @objectstack/plugin-sharing)
  • content/docs/ui/audience-based-interfaces.mdx (via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx (via @objectstack/plugin-security)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review August 4, 2026 05:53
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 4, 2026
Merged via the queue into main with commit 69a89ce Aug 4, 2026
25 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4647-vama-write-path branch August 4, 2026 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security/explain 与写入路径对同一条记录给出相反答案:VAMA 持有者对无主 private 记录 explain 答 allowed:true,PATCH 回 403

2 participants