fix(sharing)!: remove the full access level — it promised delete/transfer/share and granted edit (#3865) - #3901
Merged
Conversation
…ansfer/share and granted `edit` (#3865) `sys_sharing_rule.access_level` / `sys_record_share.access_level` offered three levels, the third documented as **Full Access (Transfer, Share, Delete)**. No code path granted transfer, re-share, or delete because of it: both enforcement sites matched `access_level in ('edit','full')`, so `full` was byte-equivalent to `edit`. An admin picking "Full Access" in Setup was told they had granted delete rights and had not — declared-but-unenforced metadata (ADR-0078, ADR-0049), the same defect that retired the `queue` recipient before it. Measured on showcase, a `full` recipient got read/update allowed and delete DENIED with `decidedBy=object_crud` — the object-level CRUD gate rejected the delete *before* sharing was consulted at all. That is the model working, not an oversight to patch around. Record sharing widens WHICH ROWS a principal reaches, never WHICH VERBS they may use: Salesforce sharing rules stop at Read-Only / Read-Write (its Full Access is owner / hierarchy / Modify All only, never grantable by a rule), and Dataverse ANDs every shared access right against the security role's own privilege. Delete and transfer belong to ownership, the ADR-0057 DEPTH scopes, and admin scope. Authoring rejects, enforcement tolerates, data normalises (the ADR-0090 D4 idiom): - `SharingLevel` (spec/security) and `ShareAccessLevel` (spec/contracts) narrow to `read | edit`; the `Field.select` on both objects offers the same two, so the Setup dropdown no longer shows the misleading option. - `SharingService.grant()` and `SharingRuleService.defineRule()` gain the access-level validation they never had. Previously ANY value was persisted verbatim, so a typo'd level became a grant no gate would ever match — the same inert-metadata bug one layer down. `full` normalises to `edit`; anything unrecognised is a `VALIDATION_FAILED` the REST layer maps to 400. - The read/write gates keep matching `edit`/`full` on purpose. Narrowing them would silently REVOKE every not-yet-migrated grant; authoring narrowness and enforcement tolerance are different jobs. - A boot backfill normalises stored `full` rows on both tables (writing with `isSystem` so the provenance hook does not mark a package-seeded rule `customized`), and the `sharing-rule-access-level-full-to-edit` conversion rewrites declarative stacks at load. Lossless by construction: the two levels were already equivalent, so the rewrite cannot change an access decision — unlike the OWD `sharingModel: 'full'` alias retired in ADR-0090 D4, which changed posture and had to be delegated to the author. The explain surfaces keep accepting `'full'`: they REPORT stored rows, and a legacy row must stay explainable rather than crash the panel. Verified on a running CRM backend: live metadata offers only read/edit; a grant of `full` returns 201 persisted as `edit`; a bogus level returns 400; and legacy `full` rows seeded directly into SQLite came back `edit` after reboot with `managed_by`/`customized` untouched. Reviving a real per-record delete grant is a separate design — a capability mask ANDed with object CRUD, plus the share-administration model that would have to authorise re-sharing — not a fourth enum member. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017346r3TMNqpbTLkT5d49uQ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Conflict: `CONVERSIONS_BY_MAJOR` — main opened protocol 17 with three alias-removal conversions while this branch had filed the `full` → `edit` rewrite under 16. Resolved by moving this branch's entry into 17, which is the correct major regardless of the conflict: 16 has already SHIPPED (`PROTOCOL_VERSION` 16.0.0), and a conversion retires at `toMajor + 1`, so leaving it at 16 would have given a stack still authoring `accessLevel: 'full'` no acceptance window at all on the very next release. The migration-chain entry moves from step16 to step17 with it. Unlike its three step-17 siblings, this entry is NOT `retiredFromLoadPath`: those are already-deprecated keys whose schemas tombstone them with a fix-it error, whereas `full` carried no prior deprecation and a removed enum VALUE yields only a generic zod message. It therefore keeps the ADR-0087 D2 default one-major window — zero-risk here precisely because the rewrite is behaviour-preserving. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017346r3TMNqpbTLkT5d49uQ
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 107 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
… sharing level `content/docs/references/` is generated from the zod schemas and is checked in CI (`@objectstack/spec check:docs`); narrowing `SharingLevel` left it stale. - `security/sharing.mdx` — `accessLevel` is now `read | edit` and the enum member list drops `full`. - `security/explain.mdx` — `grants` deliberately still lists `full`; only its description changes, since explain reports stored rows and a legacy row must stay explainable until the boot backfill normalises it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017346r3TMNqpbTLkT5d49uQ
os-zhuang
marked this pull request as ready for review
July 28, 2026 16:01
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.
Closes #3865.
问题
sys_sharing_rule.access_level/sys_record_share.access_level提供三档,第三档注释写着 Full Access (Transfer, Share, Delete)。但没有任何一处代码因为full而授予删除、转移或再共享——两处仅有的 enforcement 都是access_level in ('edit','full'),full与edit逐字节等价。管理员在 Setup 选「完全访问」,被告知授予了删除权,实际什么都没多给。这是 ADR-0078 / ADR-0049 所禁的 declared-but-unenforced,与此前摘除queuerecipient 是同一种缺陷。issue 里的实测显示,
full接收人的 delete 被拒且decidedBy=object_crud——对象级 CRUD 门在轮到 sharing 之前就拒了。为什么选择摘除而不是实现
对标主流平台后,这不是"缺一个功能",而是模型本就如此:共享放宽的是能碰哪些行,从不放宽能做哪些动作。
full想解决的每个真实场景都已有更正确的通道:接手离职同事的客户 → 转移所有者;主管删下属记录 → ADR-0057 的 write DEPTH;管理员清理 → admin scope;跨部门协作 → 保留的 read/edit 两档。这解释了它为什么一直没长出真逻辑。完整对标见 issue 评论。
改动:authoring 拒绝、runtime 容忍、数据归一(ADR-0090 D4 的模式)
SharingLevel、ShareAccessLevel收窄为read | edit;两个对象的Field.select同步,Setup 下拉不再出现误导选项(元数据驱动,objectui 无需改动)。SharingService.grant()/SharingRuleService.defineRule()补上此前完全缺失的 accessLevel 校验。原先任何值都被原样持久化,一个拼错的档位就成了没有任何门会匹配的共享行——同一个 inert-metadata 缺陷的下一层。现在full归一为edit,未知值抛VALIDATION_FAILED(REST 映射 400)。edit/full。收窄它会静默撤销所有尚未迁移的授权;authoring 的严与 enforcement 的宽是两件事。full行(以isSystem写入,避免 provenance hook 把 package 规则误标customized);sharing-rule-access-level-full-to-editconversion 在 load 时重写声明式 stack。'full'— 那是对已存在行的呈现面,legacy 行必须仍可解释,而不是让面板崩溃。迁移
无需消费者操作。 两档本就行为等价,所以重写不可能改变任何访问判定——这与 ADR-0090 D4 摘除 OWD
sharingModel: 'full'不同(那次改变了 posture,只能交给作者判断)。仍在声明accessLevel: 'full'的 stack 在 load 时带 deprecation notice 转换;存量行在下次启动归一。将ShareAccessLevel类型钉在'full'的代码不再编译,改用'edit'。验证
pnpm build71/71 通过(含类型)。access-level.test.ts与 backfill 幂等/失败降级用例。os i18n extract重新生成,check:i18n-bundles/check:i18n-coverage通过。full→ 201 且落库为edit;grantadmin→ 400;直接写入 SQLite 的 legacyfull行重启后变为edit,且managed_by/customized未被改动。后续(不在本 PR 范围)
评估过程中另外发现两处,建议单独跟进:
edit级共享目前已允许删除 ——sharing-plugin.ts对update/delete走同一个canEdit门。Salesforce 的 Read-Write 共享是明确不能删除的。收紧属破坏性变更,需独立 ADR。POST /data/:object/:id/shares只做enforceAuth(登录态),随后以SYSTEM_CTX写入共享行,没有"调用者是否有权共享这条记录"的判断,revoke同样。可能构成提权,将另开 issue。真要做 per-record 授删,应按 Dataverse 模式设计:权限掩码而非线性档位、与对象 CRUD 做 AND、先建共享管理权、同步收紧
edit——那是独立的 ADR 级项目,不应与"消除界面误导"的修复捆绑。Generated by Claude Code