Skip to content

service-storage's attachment kit carries the same 5-field callerContext projection #7141 fixed for comments #7145

Description

@os-zhuang

Out-of-scope observation from implementing #7141 (PR #7143), filed rather than fixed — that card is bounded to packages/plugins/plugin-audit.

Fact

packages/services/service-storage/src/attachment-access-hooks.ts:70
callerContext() rebuilds the identical five-field projection of the caller's
execution envelope:

function callerContext(ctx: any): Record<string, unknown> {
  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 sharing.canEdit(parentObject, parentId, …) from two
sites (the beforeInsert parent gate and the multi-row authorizeRows loop).
This is the kit comment-access-hooks.ts was derived from, so the defect is the
same one, one package over:

  • onBehalfOf is dropped, so ISecurityService.hasWriteBypass — documented
    to fail CLOSED on a delegated context, implemented by reading exactly
    context?.onBehalfOf?.userId (plugin-security/src/security-plugin.ts:704-707)
    — cannot fire on the attachment path either. The field is populated on a live
    path (runtime/src/security/resolve-execution-context.ts:191, OAuth agent
    principals on /mcp).
  • principalKind is dropped, so resolvePermissionSetsForContext
    (security-plugin.ts:3024-3034) resolves an ADR-0090 D10 agent principal as
    a human
    and appends the deployment's additive baseline to the agent's
    scope-derived ceiling.
  • systemPermissions, accessible_org_ids, posture, audience and
    rlsMembership go with them.

The middleware-key stripping half of the projection is correct and must
survive
any fix here too — plugin-security mutates the operation context in
place (sc.__readScope = …, security-plugin.ts:1159), so the arriving context
carries the depth resolved for sys_attachment while the gate asks about the
parent record's object.

Suggested shape

The same one #7141 landed: forward the envelope minus the operation-private keys,
dropped by the __ prefix (which also covers __expandRead, the marker that
waives the object-level CRUD check, and __referentialFieldClear). See PR #7143
for the fix and its reverse-verification evidence, including the measurement that
the /mcp agent ceiling sets cannot themselves carry modifyAllRecords — so the
dropped bypass guard is latent rather than exploitable with the built-in
permission sets, while the dropped ceiling rule widens unconditionally.

Dedup

Searched open issues for attachment-access-hooks, sys_attachment + sharing,
and the onBehalfOf family: only the #7070 group (#7135, #7136) and #7141, none
of which covers service-storage.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions