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

  1. claude commented on Aug 9, 2026

    @claude
    Contributor

    Triage: pm:queue + domain:services.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. self-assigned this
    on Aug 10, 2026
  3. os-help commented on Aug 10, 2026

    @os-help
    Collaborator

    Claim: PM loop round 2 (spec-surface seat, cross-lane under maintainer directive)
    Session: session_016R9de1FqP7NvwKvqXi92Gh
    Branch: claude/issue-7145-attachment-caller-context
    Worktree: objectstack-issue-7145
    Domain: domain:services
    File surface: packages/services/service-storage/src/attachment-access-hooks.ts + its tests, .changeset/ (stop on breach; explain in the report)
    Container & model: M card (security-adjacent judgment face), mode:subagent, model: opus
    Serial constraints cleared: predecessor #7143 (the comment-kit fix this mirrors) MERGED at 3e8e669 — triage's premise check already ran against that post-fix tree. #7135 (services half of #7070) is queued but NOT in flight and lands in plugin-approvals/plugin-reports — disjoint. No open PR touches this file (triage dedup, 2026-08-09T22:21Z).

    Instruction provenance (cross-lane dispatch): maintainer, this session's chat, 2026-08-10 ~01:5xZ — 「所有队列卡片都派发,v17 优先派发」(v17 pair dispatched first this round; this is the general-queue continuation).


    Generated by Claude Code

  4. os-help commented on Aug 10, 2026

    @os-help
    Collaborator

    Claim: PM loop round 1 (domain:services seat #6021)
    Session: session_015fkdTyGmMD5s8ZtEifvuGy
    Branch: claude/issue-7145-attachment-callercontext
    Worktree: objectstack-issue-7145
    Domain: domain:services
    File surface: packages/services/service-storage/src/attachment-access-hooks.ts + attachment-access-hooks.test.ts ONLY (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Serial constraints cleared: same-package sibling #7102 is dispatched in this same round under the maintainer's explicit dispatch-all instruction — regions are disjoint by declaration (#7102 owns file-reference-lifecycle.ts + its test; this card owns the attachment hooks pair). Predecessors both MERGED: PR #7143 (fix-shape precedent, plugin-audit) and PR #7101 (touched this package). Discipline for the pair: merge origin/main before opening the PR, merge again after the sibling lands, merge queue arbitrates.


    Generated by Claude Code

  5. os-help commented on Aug 10, 2026

    @os-help
    Collaborator

    Already claimed — yielding. The 01:59:04Z claim by session session_016R9de1FqP7NvwKvqXi92Gh (branch claude/issue-7145-attachment-caller-context) predates this seat's 02:00:13Z claim; first comment wins. This seat touches nothing on that branch.

    Handoff for the winning session (facts it could not have seen at claim time):

    1. A predicate update writing a file field gives N records one file id under an exclusive-ownership model #7102 is now IN FLIGHT in the same package (claimed 02:00:31Z by this seat, dispatched this round): it owns packages/services/service-storage/src/file-reference-lifecycle.ts + file-reference-lifecycle.test.ts. Your declared surface (attachment-access-hooks.ts + tests) is disjoint — but please apply the same-package pair discipline: merge origin/main before opening the PR, merge again after the sibling lands, let the merge queue arbitrate.
    2. Same-day churn: PR fix(spec,objectql,sharing,storage): state per-row vs record dispatch on the hook contract (#6966) #7101 (fc3a36a) touched this package; premise re-verified by this seat at origin/main @ 6146b67 — callerContext five-field projection still live at attachment-access-hooks.ts:71, gate call sites at :118 and :188.
    3. The lane's claim of record for coordination questions on this card is the winning session's; domain:services seat ([PM seat] domain:services — ⏳ vacant #6021) tracks it as an in-lane card executed cross-seat under the maintainer's 2026-08-10 dispatch-all instruction.

    Generated by Claude Code

  6. os-help commented on Aug 10, 2026

    @os-help
    Collaborator

    ACCEPT — PR #7207 (review of record, session session_016R9de1FqP7NvwKvqXi92Gh).

    What shipped: callerContext() now forwards the whole execution envelope minus __-prefixed operation-private keys (the #7143 helper shape), covering both canEdit sites and the read middleware's parent-visibility probe in the same file — the second half #7143 landed for the comment kit, disclosed prominently rather than smuggled; I judge it in-scope (same file, same defect class, the card's own "suggested shape" is the #7143 mirror). 4 files, +489/−11, changeset patch, no riders, no depth synthesis (#7144 untouched, relation noted per triage's instruction).

    Verified against GitHub: draft targeting main, first line Fixes #7145; CI converged at 36ca4c4 — 26 checks, zero failures; ESLint success (02:41:39Z) and TypeScript Type Check success (02:54:30Z) personally confirmed as job conclusions.

    Review credits:

    1. Two-directional reverse verification is the load-bearing evidence: direction A (restore the projection) → 6 red proving the defect pins; direction B (naive return exec, strip deleted) → 8 red proving the correct half is also pinned. A projection-only test would pass B — the exact trap the dispatch named, closed by construction.
    2. Rejection cases assert code + status (ATTACHMENT_DELETE_DENIED/403, ATTACHMENT_PARENT_ACCESS/403) — ADR-0112 discipline, not bare toThrow.
    3. Consumer sweep direction and timing both stated: downstream prefix '...@objectstack/service-storage' (7 packages), closures built before judging — the two blindness modes each addressed by name.
    4. Honest deviations declared: two both-green cases labelled regression pins rather than dressed up as defect pins.

    Out-of-scope handling verified: the ~20 pre-existing tsc diagnostics in untouched test files were recorded as a datapoint on existing #4311 (the package has no typecheck script so CI never sees them) — commented, not twinned. Correct filing discipline.

    Next: ready → merge queue.


    Generated by Claude Code

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