Fix RLS checks for UPDATE/DELETE FOR PORTION OF leftover rows

Hi,

While revisiting “[8e72d914c] Add UPDATE/DELETE FOR PORTION OF”, I found a new issue where inserting leftover rows may skip row-level security checks.

This is a repro:

1. Prepare a role and a table with an int4range column, then add policies that require the lower bound to be less than 50:
```
evantest=# create role u;
CREATE ROLE
evantest=# create table t (id int, valid_at int4range);
CREATE TABLE
evantest=# alter table t enable row level security;
ALTER TABLE
evantest=# alter table t force row level security;
ALTER TABLE
evantest=# create policy t_sel on t for select to u using (true);
CREATE POLICY
evantest=# create policy t_upd on t for update to u using (lower(valid_at)<50) with check (lower(valid_at)<50);
CREATE POLICY
evantest=# create policy t_del on t for delete to u using (lower(valid_at)<50);
CREATE POLICY
evantest=# create policy t_ins on t for insert to u with check (lower(valid_at)<50);
CREATE POLICY
evantest=# grant select, update, delete, insert on t to u;
GRANT
evantest=# insert into t values (1, '[10,100)');
INSERT 0 1
evantest=# set role u;
SET
```

2. Update the row:
```
evantest=> update t for portion of valid_at from 30 to 70 set id=2;
UPDATE 1
evantest=> select * from t;
id | valid_at
----+----------
2 | [30,70)
1 | [10,30)
1 | [70,100)
(3 rows)
```

Here, the right leftover [70,100) was inserted, even though the INSERT policy requires lower(valid_at) < 50. If the policy were honored, the update should fail because the leftover insert violates policy t_ins.

After tracing the update statement, I think the problem is as follows.

1.In the rewrite phase, get_policies_for_relation() gets only the UPDATE policy, because commandType is CMD_UPDATE:
```
/*
* For SELECT, UPDATE and DELETE, add security quals to enforce the USING
* policies. These security quals control access to existing table rows.
* Restrictive policies are combined together using AND, and permissive
* policies are combined together using OR.
*/

get_policies_for_relation(rel, commandType, user_id, &permissive_policies,
&restrictive_policies);

…..

/*
* For INSERT and UPDATE, add withCheckOptions to verify that any new
* records added are consistent with the security policies. This will use
* each policy's WITH CHECK clause, or its USING clause if no explicit
* WITH CHECK clause is defined.
*/
if (commandType == CMD_INSERT || commandType == CMD_UPDATE)
{
/* This should be the target relation */
Assert(rt_index == root->resultRelation);

add_with_check_options(rel, rt_index,
commandType == CMD_INSERT ?
WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
permissive_policies,
restrictive_policies,
withCheckOptions,
hasSubLinks,
false);
```

2. In ExecForPortionOfLeftovers(), before inserting a leftover row, it temporarily saves mtstate->operation and changes it to CMD_INSERT:
```
/*
* Save some mtstate things so we can restore them below. XXX:
* Should we create our own ModifyTableState instead?
*/
oldOperation = mtstate->operation;
mtstate->operation = CMD_INSERT;
oldTcs = mtstate->mt_transition_capture;
```

3. Then it calls ExecInsert() with that state to insert the leftover row:
```
/*
* The standard says that each temporal leftover should execute its
* own INSERT statement, firing all statement and row triggers, but
* skipping insert permission checks. Therefore we give each insert
* its own transition table. If we just push & pop a new trigger level
* for each insert, we get exactly what we need.
*
* We have to make sure that the inserts don't add to the ROW_COUNT
* diagnostic or the command tag, so we pass false for canSetTag.
*/
AfterTriggerBeginQuery();
ExecSetupTransitionCaptureState(mtstate, estate);
fireBSTriggers(mtstate);
ExecInsert(context, resultRelInfo, leftoverSlot, false, NULL, NULL);
fireASTriggers(mtstate);
AfterTriggerEndQuery(estate);
```

4. In ExecInsert(), wco_kind is chosen from mtstate->operation. Since it is now CMD_INSERT, the value becomes WCO_RLS_INSERT_CHECK:
```
/*
* Check any RLS WITH CHECK policies.
*
* Normally we should check INSERT policies. But if the insert is the
* result of a partition key update that moved the tuple to a new
* partition, we should instead check UPDATE policies, because we are
* executing policies defined on the target table, and not those
* defined on the child partitions.
*
* If we're running MERGE, we refer to the action that we're executing
* to know if we're doing an INSERT or UPDATE to a partition table.
*/
if (mtstate->operation == CMD_UPDATE)
wco_kind = WCO_RLS_UPDATE_CHECK;
else if (mtstate->operation == CMD_MERGE)
wco_kind = (mtstate->mt_merge_action->mas_action->commandType == CMD_UPDATE) ?
WCO_RLS_UPDATE_CHECK : WCO_RLS_INSERT_CHECK;
else
wco_kind = WCO_RLS_INSERT_CHECK;
```

5. With this wco_kind, it calls ExecWithCheckOptions():
```
/*
* ExecWithCheckOptions() will skip any WCOs which are not of the kind
* we are looking for at this point.
*/
if (resultRelInfo->ri_WithCheckOptions != NIL)
ExecWithCheckOptions(wco_kind, resultRelInfo, slot, estate);
```

6. In ExecWithCheckOptions(), it loops over all WithCheckOptions built during rewrite:
```
/* Check each of the constraints */
forboth(l1, resultRelInfo->ri_WithCheckOptions,
l2, resultRelInfo->ri_WithCheckOptionExprs)
{
WithCheckOption *wco = (WithCheckOption *) lfirst(l1);
ExprState *wcoExpr = (ExprState *) lfirst(l2);

/*
* Skip any WCOs which are not the kind we are looking for at this
* time.
*/
if (wco->kind != kind)
continue;
```

In this loop, wco->kind is WCO_RLS_UPDATE_CHECK, but kind is WCO_RLS_INSERT_CHECK, so the check is skipped and the right leftover row [70,100) is inserted.

The same problem exists for DELETE as well:
```
evantest=> delete from t for portion of valid_at from 30 to 70;
DELETE 1
evantest=> select * from t;
id | valid_at
----+----------
1 | [10,30)
1 | [70,100)
(2 rows)
```

For DELETE, in code snippet 1 above, add_with_check_options() is not called at all because commandType is CMD_DELETE. Then, in code snippet 5, resultRelInfo->ri_WithCheckOptions is NULL, so ExecWithCheckOptions() is not called.

I checked the docs, and they seem to mention only that INSERT privilege is not required for the hidden insertion of leftover rows. But I think RLS should still be honored, because otherwise the policies are violated. For example, directly inserting [70,100) is blocked by policy t_ins, but a user can work around that by inserting [1,100) and then updating [30,70), which seems like a security hole.

To fix this, I think the rewriter should load INSERT policies for UPDATE and DELETE operations when parse->forPortionOfexists, since UPDATE/DELETE FOR PORTION OF may insert leftover rows.

See the attached patch for details. With the fix, both the update and delete above now fail:
```
evantest=> update t for portion of valid_at from 30 to 70 set id=2;
ERROR: new row violates row-level security policy for table "t"
evantest=> delete from t for portion of valid_at from 30 to 70;
ERROR: new row violates row-level security policy for table "t"
```

BTW, I just saw that the v19 branch has been cut and HEAD is now v20, so it looks like this patch will need to be back-patched if accepted.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/