Skip to content

plugin-security: the insert-side RLS check post-image is the raw caller payload evaluated before beforeInsert, so a field a hook stamps can never satisfy an insert check — the caller must send the value the hook will overwrite anyway #16608

Description

@os-sam

Measured on @objectstack/plugin-security 17.3.0 / @objectstack/cli 17.3.0 while delivering objectstack-ai/ats#43 (ats main 068c7c3, both drivers). Filed unassigned and unlabelled for triage. Sibling of objectstack#16607 (write-path membership staging); this one stays after that one is fixed, and the measurement below is taken on an object where staging is NOT the confound.

What happens

ats_employer_member is a controlled_by_parent child of ats_employer; the app's ats_employer_member_stamp hook (beforeInsert, runAs: 'system') reads the parent and stamps employer_org from it. The policy: check: 'record.employer_org in current_user.employer_org_ids'. Same identity (admin@quillstone.example), same object, same second:

POST /api/v1/data/ats_employer_member result
payload WITH employer_org: org_ats_quillstone (the exact value the hook would stamp) 201
payload WITHOUT employer_org (left to the hook) 403 PERMISSION_DENIED[Security] RLS check FAILED on insert 'ats_employer_member' — write denied (fail-closed)

The stored row is identical in both cases: the stamp overwrites employer_org from the parent whatever the caller sent.

Why

In the security middleware's write gate:

let postImage = { ...opCtx.data };
if (opCtx.operation === "update") { ... postImage = { ...pre, ...opCtx.data }; }
if (postImage && !checkParts.every((f) => matchesFilterCondition(postImage, f))) { /* deny */ }

For an insert the "post-image" is the caller's payload as it arrived. beforeInsert hooks run later, inside the engine's operation, so a value a hook derives (an organization copied from a parent, a candidate_user copied from a candidate) is not on the image the check is evaluated against. The check therefore judges a value that never lands and ignores the one that does.

Why it is worth a decision rather than a workaround

Denormalised scoping fields are exactly the fields RLS predicates compare (a predicate cannot traverse a lookup, ADR-0055), and they are exactly the fields an app stamps in beforeInsert so a caller cannot choose them. Under the current order the only way for such an insert to pass a check is for the client to send the stamped field — i.e. to send the value the hook exists to make un-sendable. Two coherent resolutions:

  1. Evaluate the insert check on the image the row will actually have — after beforeInsert (the engine already has the hook-mutated payload at that point; the update path already merges a pre-image for the same reason).
  2. Keep the order and state the contract: a field referenced by an insert-side check must be present in the caller's payload; hooks may only re-derive it. Then os validate should flag a check that references a field the object's beforeInsert hooks write.

Option 1 matches what the update path already does and what the policy author means by "the row being written".

Downstream effect

In ats every employer-side object stamps employer_org (and candidate_user) in beforeInsert; with the current order an employer administrator's own inserts on those objects are refused unless the client duplicates the stamp (measured for ats_job, together with #16607, and for ats_employer_member above).

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

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions