DEV Community

Todd Sullivan
Todd Sullivan

Posted on

The RLS Policy Was Correct. The Write Was Still Wrong.

The RLS Policy Was Correct. The Write Was Still Wrong.

I spent a chunk of last week doing the least glamorous kind of security work: reading database policies line by line and asking, "what can the client still put in this row?"

The useful bug pattern was simple:

Row-level security said which rows a user could touch, but not always which values they were allowed to write.

That difference matters a lot in offline-first apps, because the mobile client necessarily syncs whole records back to the server. If the database policy only checks tenant scope, a malicious or buggy client can sometimes write fields that were supposed to be server-owned.

A simplified version:

CREATE POLICY org_isolation ON inspections
  FOR ALL
  TO authenticated
  USING (org_id = my_org_id());
Enter fullscreen mode Exit fullscreen mode

Looks reasonable. A user can only see and edit inspections in their own org.

But for writes, that policy is incomplete. Without a WITH CHECK, the database is mostly answering "is this row in your org?" It is not necessarily answering "are you allowed to set approval_status = 'approved'?"

That distinction turned into a real class of findings:

  • an org member could mark an inspection approved without an approval record
  • approval rows could be forged with someone else's reviewer id/name/role
  • onboarding could attach a fresh account to an existing org id if the RPC accepted caller-supplied ids too literally
  • client-writable billing/quota columns were one PATCH away from entitlement nonsense

None of those are exotic exploits. They are boring trust-boundary mistakes. The client had too much ability to describe facts the server should have decided.

The fix was not "add more app-side checks." App-side checks are UI. They help honest users, not the boundary.

For approval decisions, the server now adjudicates the thing that matters:

CREATE OR REPLACE FUNCTION record_approval(
  p_inspection_id uuid,
  p_decision text,
  p_comment text DEFAULT NULL
)
RETURNS inspection_approvals
LANGUAGE plpgsql
SECURITY DEFINER
AS $$
DECLARE
  v_org_id uuid;
  v_role text;
BEGIN
  SELECT org_id, role
  INTO v_org_id, v_role
  FROM profiles
  WHERE id = auth.uid();

  IF v_role NOT IN ('supervisor', 'admin') THEN
    RAISE EXCEPTION 'Supervisor or admin role required';
  END IF;

  INSERT INTO inspection_approvals (
    inspection_id, org_id, reviewer_id, reviewer_role, decision, comment
  )
  VALUES (
    p_inspection_id, v_org_id, auth.uid(), v_role, p_decision, p_comment
  );

  UPDATE inspections
  SET approval_status = p_decision
  WHERE id = p_inspection_id AND org_id = v_org_id;
END;
$$;
Enter fullscreen mode Exit fullscreen mode

The caller can request a decision. It cannot choose the reviewer, role, org, or audit identity. Those come from auth.uid() and server-side profile state.

For the offline sync path, I kept the boring path boring. Inspectors still need to create and submit inspections with no signal, then sync later. So the value gate became narrower: clients can write draft/submitted states, but approved/rejected must match the latest server-recorded approval decision.

That's the part I like. The fix did not require turning the whole app into RPC soup. Only the authority-bearing transition moved server-side.

The takeaway: RLS is not one switch. Tenant isolation is only the first question.

For every writable field, ask:

  1. Can the client choose this value?
  2. If yes, is tenant scope enough?
  3. If no, where is the server-side writer?

Most bugs I found were hiding between question 1 and question 2. The policy knew whose row it was. It did not always know whose decision it was.

Top comments (0)