Re: RLS with check option - surprised design

* Peter Geoghegan (pg(at)heroku(dot)com) wrote:
> On Fri, Nov 21, 2014 at 6:57 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > Are you sure this isn't just another example of an existing issue we
> > have wrt column privileges..? I'm working on a patch already to address
> > those issues in back-branches and will be considering what needs to be
> > done for RLS also.
>
> I thought it was pretty conclusively the case that it wasn't (i.e.
> that I definitely had an obligation to figure out a way to get the
> security quals appended to the UPDATE predicate). I'm slightly
> surprised that you don't immediately agree - after all, I could
> manually append a qual that will "stop the leak", since the UPDATE
> then won't affect what it shouldn't (though you're still locking the
> row, as always happens with ON CONFLICT UPDATE when the WHERE clause
> doesn't pass [1], which might need to be considered. But the same is
> true with postgres_fdw,)

The issue is that the entire row is being reported though, no? That's
the same issue we have today. If only the values provided by the user
were reported then there wouldn't be an issue with the detail in the
error message. This is what Dean was getting at when he suggested
looking at modifiedCols to see what should be reported back to the user
in the details.

As for adding quals to the UPDATE- what would end up happening instead?

There's two ways to consider how this can be handled and it depends on
if we think the command should end up failing or doing nothing. I can
see arguments for both, but my feeling is the the command needs to fail
in some way since we can't allow the INSERT to complete (due to the PK
violation) nor the UPDATE (as the user should not be able to see that
row per the USING policy, and likely the new row wouldn't be allowed by
the WITH CHECK policy either).

I don't think we'd want the command to appear to succeed for the same
reason that a straight INSERT doesn't succeed- we'd be accepting data
and then throwing it away. I don't think anyone would intentionally
issue a UPSERT and expect it to be a no-op, while that can certainly
happen with an UPDATE that has quals which end up not matching any
records in the table.

Thanks,

Stephen