diff --git a/.changeset/comment-gate-own-depth-contract.md b/.changeset/comment-gate-own-depth-contract.md new file mode 100644 index 0000000000..73659c0abc --- /dev/null +++ b/.changeset/comment-gate-own-depth-contract.md @@ -0,0 +1,30 @@ +--- +"@objectstack/spec": patch +"@objectstack/plugin-audit": patch +--- + +docs(spec,plugin-audit): record that the parent-record write gates match ownership at `own` BY DESIGN (#7144) + +Documentation only — no gate changes what it returns for any input. + +`ISharingService`'s write gates widen ownership "by write DEPTH", but that depth +is an INPUT the caller supplies: the CRUD middleware resolves it for the object +of the operation in flight and stamps it on the operation context. The +`sys_comment` gates (`@objectstack/plugin-audit`) and the `sys_attachment` kit +(`@objectstack/service-storage`) ask this service about the PARENT record's +object, so the stamped depth belongs to a different object and is dropped — and +the owner-match runs at its narrowest, `own`. A caller whose write depth on the +parent is `unit` / `unit_and_below` / `org` can therefore edit that parent +directly and is refused when editing a comment or attachment on it. + +That divergence is deliberate and runs in the restrictive direction (refusals, +never a leak). The contract now says so, and — the part that matters for anyone +tempted to "fix" it — says WHY the alternative is not merely unimplemented: +`ISecurityService.resolveWriteScope`, the only tool a package outside +`plugin-security` has for the parent's depth, fails OPEN, because +`getEffectiveScope` returns `'org'` when no permission set mentions the object +at all — indistinguishable from a genuine `modifyAllRecords` holder. Handed to a +write gate as the depth it becomes authoritative on its own and the owner-match +short-exits `true` for every owned row of that object. Inheriting the parent's +edit authority starts with a depth primitive that can tell "org depth" from +"nothing matched", not with these gates. diff --git a/packages/plugins/plugin-audit/src/comment-access-hooks.ts b/packages/plugins/plugin-audit/src/comment-access-hooks.ts index e37a0b6b5a..e63462f638 100644 --- a/packages/plugins/plugin-audit/src/comment-access-hooks.ts +++ b/packages/plugins/plugin-audit/src/comment-access-hooks.ts @@ -258,8 +258,16 @@ function withoutOperationPrivateKeys(exec: Record): ExecutionCo * Note what deliberately did NOT change: no access DEPTH is synthesised for the * parent object. Absent depth leaves the sharing owner-match at its narrowest * (`own`) — the safe direction, and byte-for-byte the behaviour the projection - * produced. Resolving the parent's own depth (the other candidate shape) would - * WIDEN this gate and is a separate decision; see #7141's PR discussion. */ + * produced. + * + * [#7144] That is now the RULED shape, not a pending question: the maintainer's + * ruling of 2026-08-10 keeps these gates at `own`, deliberately tighter than + * the parent's real edit authority. The reasoning — including the fail-open in + * `ISecurityService.resolveWriteScope` that makes the widening unsafe to wire + * today — is recorded once on the contract that owns this gate's meaning + * (`@objectstack/spec` — `ISharingService`, "Write DEPTH is an input the CALLER + * supplies"), because the `sys_attachment` kit reaches the same gate from + * another package; it is not restated here. */ function callerContext(ctx: any): ExecutionContext { const exec = ctx?.input?.options?.context; if (exec && typeof exec === 'object') { diff --git a/packages/spec/src/contracts/sharing-service.ts b/packages/spec/src/contracts/sharing-service.ts index 34f721b68e..9534792c59 100644 --- a/packages/spec/src/contracts/sharing-service.ts +++ b/packages/spec/src/contracts/sharing-service.ts @@ -257,6 +257,46 @@ export type SharingWriteVerdict = 'allow' | 'abstain' | 'deny'; * `org_user_ids`, `systemPermissions`, `posture` (ADR-0095 D2) and * `tabPermissions` included. Which of those a deployment makes load-bearing * depends on its tenancy posture, which the caller cannot know. + * + * ## Write DEPTH is an input the CALLER supplies — absent means `own` (#7144) + * + * The write gates below widen ownership "by write DEPTH" (ADR-0057 D1: `own` / + * `own_and_reports` / `unit` / `unit_and_below` / `org`). That depth is NOT a + * field of {@link ExecutionContext}, and this service never derives it: the + * CRUD middleware resolves it for the object of the operation IN FLIGHT and + * stamps it on the operation context as an operation-private (`__`-prefixed) + * key, which the default implementation's owner-match reads. A caller that is + * not that middleware hands over no depth, and the owner-match then runs at its + * NARROWEST — owner-only, i.e. `own`. + * + * **For the PARENT-record gates that is deliberate, not an oversight.** The + * `sys_comment` gates (`@objectstack/plugin-audit`) and the `sys_attachment` + * kit (`@objectstack/service-storage`) ask this service about the PARENT + * record's object while the operation in flight is on the CHILD, so the stamped + * depth was resolved for a different object and is dropped rather than carried + * across. Stated as behaviour: a caller whose write depth on the parent object + * is `unit` / `unit_and_below` / `org` can edit that parent directly through + * the CRUD path, and is REFUSED when editing or deleting a comment or + * attachment on it. The divergence runs in the RESTRICTIVE direction — + * refusals, never a leak — and the maintainer ruling of 2026-08-10 (#7144, + * split from #7141 / PR #7143) is that these gates STAY at `own`. + * + * **Why the obvious "fix" is off the table** — recorded here so the next reader + * can tell "deliberately tighter" from "nobody got round to it". Making these + * gates inherit the parent's real edit authority means resolving the parent's + * depth through the only tool a package outside `plugin-security` has, + * `ISecurityService.resolveWriteScope` — and that fails OPEN on one input: + * `getEffectiveScope` returns `'org'` when NO permission set mentions the + * object at all (`plugin-security/src/permission-evaluator.ts` — + * `if (!matched) return 'org'`), which is byte-identical to what a genuine + * `modifyAllRecords` holder gets. That is why the method's own doc block calls + * `'org'` authoritative ONLY when paired with an explicit + * `ISecurityService.hasWriteBypass` check. Handed to a write gate as the depth + * it becomes fully authoritative on its own, and the owner-match short-exits + * `true` for EVERY owned row of an unmatched object — a real widening, not a + * theoretical one. So inheriting the parent's depth starts with a depth + * primitive that can tell "org depth" from "nothing matched", never with these + * gates. */ export interface ISharingService { /** @@ -308,6 +348,13 @@ export interface ISharingService { * `modifyAllRecords` super-user bypass. Always true for system context, * `public` objects, and objects with no owner field. * + * [#7144] The write DEPTH in that first sentence is an INPUT this service is + * handed, never one it resolves — so a caller outside the CRUD middleware + * (the `sys_comment` and `sys_attachment` PARENT-record gates, which both + * reach this service through this method) matches ownership at `own`. That is + * the ruled shape, and the fail-open that blocks widening it is written up + * once in "Write DEPTH is an input the CALLER supplies" on this interface. + * * [#6428] The two-state PROJECTION of {@link checkEdit}: `true` for every * verdict that is not `deny`, i.e. `allow` and `abstain` alike. That * collapse is the historical semantics, kept byte-for-byte so existing