Skip to content

Decision needed: should the sys_comment parent gates run the parent's owner-match at the caller's real write DEPTH, or stay at own? #7144

Description

@os-zhuang

Split out of #7141 (PR #7143) rather than decided there, because it is a widening and the two candidate shapes that card named diverge here — on nothing else.

The fact

ISharingService.canEdit widens its owner-match by the access DEPTH the caller
holds on the probed object, read from the middleware-private __writeScope key
(plugin-sharing/src/sharing-service.tsmatchesOwnerScope, and
if (writeScope === 'org') return true). plugin-security supplies that key
explicitly for the object it is probing, via the private
resolveWriteScopeForSharing (security-plugin.ts:2516).

The sys_comment gates in plugin-audit supply no depth. The five-field
projection #7141 removed never carried one, and PR #7143 deliberately kept it
that way: an absent depth leaves the owner-match at its narrowest (own), which
is the safe direction and is byte-for-byte what the projection produced.

The consequence, stated as behaviour

A caller whose write depth on the PARENT object is unit / unit_and_below /
org can edit that parent record directly through the CRUD path (where the
middleware stamps the depth for that object) but is refused when they try to
edit or delete a comment on it, because this gate asks the same service the same
question with the depth omitted. It is a divergence in the restrictive
direction — no data leaks — between the comment gate and the parent's real edit
authority. Whether that is correct or is a bug depends on what the comment gate
is meant to mean, which is a product decision, not a code reading.

Why it was not simply fixed in #7143

The tool a package outside plugin-security has for this is the published
ISecurityService.resolveWriteScope. It fails open on one input:
getEffectiveScope returns 'org' when no permission set matches the object
(permission-evaluator.ts:255), and the service's own doc block flags that
'org' as "non-authoritative on its own". Passed to canEdit as __writeScope
it becomes fully authoritative, and matchesOwnerScope then returns true for
any owned row of an unmatched object — a real widening, not a theoretical
one. Taking that route needs either a narrower contract method (one that
distinguishes "org depth" from "nothing matched") or the gate resolving depth
some other way.

Note this is the same question for service-storage's attachment kit, which
gates on canEdit the same way.

What a decision would look like

  1. Keep own (today, and fix(plugin-audit): forward the caller's execution envelope to the sys_comment sharing gates (#7141) #7143's behaviour) — the comment gate is
    deliberately tighter than the parent's edit authority. Then say so in the
    contract's doc block, so the next reader does not "fix" it.
  2. Resolve the parent's depth — the comment gate inherits the parent's real
    edit authority. Needs a depth primitive that does not fail open on an
    unmatched object before it can be wired.

Related: #7141, PR #7143, #6523, #6206, ADR-0057 D1, ADR-0111 D1.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions