plugin-audit's `callerContext` rebuilds a 5-field subset of the execution envelope, dropping `onBehalfOf` before the sharing gates that are documented to fail closed on it

[os-zhuang]

Out-of-scope observation from implementing #7136 (PR #7140), filed rather than fixed — that card was bounded to annotation widening plus redundant-cast deletion, and the change below is a RUNTIME change with an enforcement consequence.

Fact

packages/plugins/plugin-audit/src/comment-access-hooks.ts — callerContext() does not forward the caller's execution envelope. It rebuilds a five-field projection of it:

function callerContext(ctx: any): ExecutionContext {
  const exec = ctx?.input?.options?.context;
  if (exec && typeof exec === 'object') {
    return {
      userId: exec.userId,
      tenantId: exec.tenantId,
      positions: exec.positions,
      permissions: exec.permissions,
      isSystem: exec.isSystem,
    };
  }
  ...
}

The result is handed to ISharingService.buildReadFilter / canEdit / canDelete, whose contract since #6523 declares the FULL envelope and whose doc block states callers "MUST NOT rebuild a subset of it" (the #6206 ruling). PR #7140 widened this function's return ANNOTATION to ExecutionContext; the body was deliberately left alone, because changing it is not inert.

The field that matters: onBehalfOf

ISecurityService.hasWriteBypass — the modifyAllRecords super-user probe the sharing write gates consult — is documented to fail CLOSED on a delegated context, and implements it by reading exactly that field (packages/plugins/plugin-security/src/security-plugin.ts:704):

hasWriteBypass: async (object, context) => {
  if (context?.isSystem) return true;
  if (!context?.userId) return false;
  if (context?.onBehalfOf?.userId) return false;   // no ADR-0090 D10 intersection on this path
  ...
}

onBehalfOf is not in the projection, so on the comment path that guard can never fire. It is not dormant vocabulary: packages/runtime/src/security/resolve-execution-context.ts:191 populates it for OAuth agent principals on the /mcp surface —

ctx.principalKind = 'agent';
ctx.onBehalfOf = { userId: authz.userId, principalKind: 'human' };

so an MCP agent touching sys_comment reaches these gates with the delegation link already stripped, and hasWriteBypass evaluates it as an ordinary direct call. principalKind: 'agent', systemPermissions, accessible_org_ids and posture are dropped by the same projection.

What I could NOT establish, and it bounds the severity honestly: whether the agent's scope-derived permission ceiling (scopesToAgentPermissionSets) can actually carry modifyAllRecords. If it cannot, hasSuperuserWriteBypass returns false anyway and the stripped guard is currently redundant in practice. What is definite is that a documented fail-closed invariant is not honoured on this path, and that it is load-bearing only because of a second predicate nobody wired the two together deliberately. Grading this is triage's call, not mine — filed unlabeled.

The same projection also drops __delegatorReadScope / __delegatorWriteScope, so the ADR-0090 D10 delegator intersection that sharing-plugin.ts re-runs on the ordinary CRUD path does not happen for comment reads on the parent record.

Why the naive fix is wrong

return exec; is NOT the fix. plugin-security's middleware MUTATES the operation context in place (sc.__readScope = …, security-plugin.ts:1159), so the context arriving here carries the access DEPTH resolved for sys_comment — the object of the operation — while these gates ask the sharing service about the parent record's object. Forwarding it whole would hand one object's depth to another object's owner-match: a WIDENING input applied to the wrong object. That is the precise leak resolveWriteScopeForSharing was extracted to prevent ("Always returned (even as undefined) so the caller can write the key unconditionally and a stale value can never leak in through a spread", security-plugin.ts:2516).

So the projection is currently doing two jobs at once — stripping the middleware keys (correct, and the safe direction) and stripping the principal fields (not correct). Any fix has to separate them, e.g. forward the envelope minus the middleware-private keys, or resolve the parent object's own depth the way resolveWriteScopeForSharing does.

Dedup

Searched open issues for onBehalfOf + delegator + sharing bypass, comment-access-hooks / sys_comment, and SharingExecutionContext: no hits other than the #7070 family (#7135, #7136), none of which covers this. Not inside #7136's completion scope — that card is annotation widening plus redundant-cast deletion, with implementation bodies untouched, exactly as #6523's contract half separated from its consumer half. Standalone rather than a sub-issue.

Related: #7136 / PR #7140 (where the annotation was widened and this was deliberately not folded in), #6523, #6206 (ruling), #7070.

[os-zhuang]

Triage: pm:queue + domain:identity.

• Classification: concrete defect with a named landing site and a verified enforcement consequence — not observation-class. callerContext() (packages/plugins/plugin-audit/src/comment-access-hooks.ts:202) forwards a 5-field projection of the execution envelope, dropping onBehalfOf and the D10 delegator-scope keys before the ISharingService gates, while hasWriteBypass implements its documented fail-closed check by reading exactly context.onBehalfOf.userId (packages/plugins/plugin-security/src/security-plugin.ts:704-707), and that field is populated on a live path (packages/runtime/src/security/resolve-execution-context.ts:191, OAuth agent principals on the /mcp surface). The finding: SharingExecutionContext 是同族第四个窄 enforcement 契约类型(sharing / approval / report 三个服务共用),#6206 裁决的「不留 per-site 子集」默认尚未覆盖它 #6523 contract's "MUST NOT rebuild a subset" clause (同族第三处组装:share-link 路由把授权信封裁成 4 个字段后直接当 enforcement context 喂给 engine.find —— group 租户姿态下 Layer 0 墙恒判否 #6206 ruling) is violated at a live call site. The card's honest severity bound (whether the agent scope ceiling can mint modifyAllRecords) affects exploit depth, not whether the invariant is broken — the dispatched dev should make that measurement part of the fix's test evidence.
• Routing anchor (read, not inferred): fix lands in packages/plugins/plugin-audit ⇒ domain:identity per the domain table (plugin-audit row). The body's "why the naive fix is wrong" section is binding guidance: the middleware-key stripping half of the projection is correct and must survive any fix.
• target:v17: not applied — the release-blocking test isn't clearly met while exploitability of the dropped guard is unestablished. If the identity lane establishes that the agent permission ceiling can carry modifyAllRecords (or an equivalent live widening via the dropped delegator scopes), flag back to triage for a target re-grade.
• Dedup: repo-scoped search on onBehalfOf + sharing returns only the finding: consumer half of #6523 — three plugin implementations still annotate SharingExecutionContext, so (context as any).posture is still needed to read a field the contract now declares #7070 family (Re-annotate plugin-approvals + plugin-reports onto the full ExecutionContext and delete the now-redundant casts (services half of #7070) #7135, Re-annotate plugin-sharing + plugin-audit onto the full ExecutionContext and delete the double-casts (identity half of #7070) #7136) — annotation-widening scope, implementation bodies untouched by design (PR refactor(plugin-sharing,plugin-audit): annotate enforcement on the full ExecutionContext and delete the double-casts (#7136) #7140 deliberately left this body alone). Clean.
• Stale-premise check: all three anchors verified on origin/main @ 3415a61 (projection at comment-access-hooks.ts:202; fail-closed read at security-plugin.ts:707; producer at resolve-execution-context.ts:191).

本评论来自分诊座位 Routine（#5474 试点），不构成认领。

---

Generated by Claude Code

[os-zhuang]

Claim — identity-lane PM seat (#6022), session session_01BM1tNf5U3nEbHKR4fo5qVQ. Dispatching to a developer subagent now.

• Branch: claude/issue-7141-comment-caller-envelope
• Worktree: ../objectstack-issue-7141 (dedicated per-task worktree; never the shared checkout)
• Domain: domain:identity — label untouched (single-producer rule; graded and routed by the triage seat 21:25Z)
• Status: pm:queue → pm:dispatched. Thread re-read before claiming: the only comment is the triage grading; no competing session ID.

Serial constraints cleared: the identity lane is otherwise empty in this repo — #7136 landed 21:09:02Z (PR #7140), which widened this very function's return annotation while deliberately leaving the body alone; this card is that deferred body change. plugin-audit has nothing else in flight. The lane's only other open item, cloud#1220, is a different repo.

Binding guidance carried to the claimant, from the triage grading and the card body:

• The projection is doing two jobs at once. Stripping the middleware-private keys is correct and must survive any fix — plugin-security's middleware mutates the operation context in place (sc.__readScope = …, security-plugin.ts:1159), so the arriving context carries the depth resolved for sys_comment, while these gates ask the sharing service about the parent record's object. return exec; would hand one object's depth to another object's owner-match — the exact widening leak resolveWriteScopeForSharing was extracted to prevent (security-plugin.ts:2516).
• Stripping the principal fields is the defect: onBehalfOf, plus principalKind, systemPermissions, accessible_org_ids, posture, and the __delegator*ReadScope/WriteScope keys.
• The severity measurement is part of the deliverable, not optional: establish whether the agent's scope-derived ceiling (scopesToAgentPermissionSets) can actually carry modifyAllRecords. That bounds whether the stripped guard is load-bearing today or latent — and it belongs in the test evidence either way. Report it as measured, including "could not establish, here is why" if that is the honest answer.

If the two candidate shapes (forward the envelope minus middleware-private keys · resolve the parent object's own depth the way resolveWriteScopeForSharing does) diverge in enforcement outcome rather than only in structure, stop and return needs_decision with both measured — this seat will take it to the maintainer. A fail-closed contract is not a place to pick by taste.

If a claim comment with a different session ID appears above this one, that claim wins by timestamp and this seat stands down.

---

Generated by Claude Code

[os-zhuang]

Implemented in draft PR #7143 (branch claude/issue-7141-comment-caller-envelope, session session_01BM1tNf5U3nEbHKR4fo5qVQ). Not merged and not marked ready — the PM seat reviews and lands it.

Shape chosen: forward the envelope minus the middleware-private keys, dropped by the __ prefix rather than by a four-name list — the same channel carries __expandRead, which waives the object-level CRUD check (security-plugin.ts:1082), and __referentialFieldClear. The second candidate (resolve the parent object's own depth) was not taken: the only depth primitive published outside plugin-security is ISecurityService.resolveWriteScope, and getEffectiveScope returns 'org' when no permission set matches the object (permission-evaluator.ts:255) — the fail-open its own doc block calls "non-authoritative on its own". Handed to canEdit as __writeScope it would make matchesOwnerScope return true for any owned row of an unmatched object.

On the divergence test the dispatch asked about: the two shapes do not diverge on the fail-closed question this card is about — resolveWriteScope itself returns 'own' for an onBehalfOf context, so a delegated principal is refused identically under either. They diverge only on an orthogonal widening (whether this gate should inherit the parent's real write depth). That decision is filed as #7144 rather than taken by taste here.

Severity, as measured (triage asked for this explicitly):

• scopesToAgentPermissionSets cannot mint modifyAllRecords. It returns exactly three set names and all three are literal declarations in default-permission-sets.ts; none carries viewAllRecords/modifyAllRecords (only admin_full_access and organization_admin do). So on the live /mcp producer path the stripped guard was latent — hasSuperuserWriteBypass would have answered false anyway.
• A second dropped field on the same path is not latent. resolvePermissionSetsForContext (security-plugin.ts:3024-3034) keys the ADR-0090 D10 "an agent's grants are exactly its ceiling" rule on principalKind === 'agent', which the projection also stripped — so an agent was resolved as a human here and the deployment's additive baseline was appended to its consented ceiling. Unconditional widening; still not a modifyAllRecords bypass with stock defaults, since member_default does not carry the bit, but fallbackPermissionSet is deployment-configurable (ADR-0056 D7).
• Conclusion: the invariant was broken on a live path, and it is not exploitable into a super-user write bypass with the built-in permission sets. The target:v17 re-grade offered in the triage grading is therefore not claimed.

One anchor correction: the projection reaches canEdit only, not buildReadFilter/canDelete — this module holds the service as Pick<ISharingService, 'canEdit'> and calls it from one site. All other anchors verified unmoved at origin/main @ 3415a61.

Also filed: #7145 — service-storage's attachment kit carries the byte-identical projection.

---

Generated by Claude Code