Skip to content

plugin-security: the Layer 0 tenant write wall judges the PRE-hook image, so a beforeInsert hook that writes the tenant column decides the stored organization unjudged #16876

Description

@os-trump

Filed unassigned and unlabelled for triage. Sibling of #16790. Found by the isolated CONTRACT_REVIEW_TIER review of PR #16805 as finding F2 (not blocking that PR): #16805 (comment)

What happens

Two facts compose into a hole, and neither is a defect on its own.

  1. The Layer 0 write wall judges opCtx.data — the caller's payload as it arrived, in the security middleware, BEFORE the engine's operation and therefore before any beforeInsert hook has run (packages/plugins/plugin-security/src/security-plugin.ts, the write-wall block around lines 2990-3060).

  2. driver-sql's injectTenantOnInsert fills the tenant column only when it is absent or empty (around lines 12602-12614). An explicit value already on the row is stored as-is — which is correct on its own terms: an explicitly-scoped write must not be silently re-scoped.

So a beforeInsert hook that writes the tenant column — the runAs: 'system' stamp shape that #16608 is entirely about, and which #16608's own census counted 29 instances of across objectstack, ats and hotcrm — decides the row's stored organization, and no Layer 0 judgement ever sees that value. The wall judged a payload that did not carry it; the driver preserved it because it was there.

Why it is worth a card rather than a note

This is the same defect family as #16608 one layer down, and #16608's fix does not close it. #16608 moves the RLS check onto the post-hook row; it does not move the Layer 0 tenant wall, which is a different gate with a different scope (it applies whether or not a permission set declares a check, so it is the broader of the two).

The awkward part is that #16608's fix makes the gap easier to overlook: after that PR, a policy whose check names the tenant column IS judged on the post-hook value — so the narrow, opt-in gate is post-hook while the wide, always-on gate is still pre-hook.

The fix has a host already

PR #16805 installs OperationContext.postHookWriteImageCheck — a judgement the enforcement layer puts on the operation context and ObjectQL.insert runs once the beforeInsert chain has produced the row, after the post-hook declared-field door and before every producer with a side effect. The seam already hands over exactly the rows the driver is about to be given. Installing the Layer 0 write filter on that same seam is the obvious shape, and the mechanism does not have to be built.

⚠️ Not a bounded in-place fix on #16805, deliberately: moving a second, wider gate's evaluation point is a second accept/refuse change and wants its own decision. Deciding it needs at least: whether the wall should judge the post-hook row for every write or only where a hook actually wrote the tenant column, and what a refusal reads as (the wall's existing envelope, or the check gate's).

Not verified from here

The review read this off the source on refs/review/16805 and origin/main; it did not stand up a runtime that exercises it. The first step on this card should be the reproduction — a non-system caller, an object whose beforeInsert hook stamps the tenant column from a parent in another organization, ground truth read straight off the driver's table — the same harness shape #16608's measurements used (issue comment 5579761278). ⛔ Grade the severity from that measurement, not from this description.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions