Re: WITH CHECK and Column-Level Privileges

Noah,

* Noah Misch (noah(at)leadboat(dot)com) wrote:
> On Mon, Jan 12, 2015 at 05:16:40PM -0500, Stephen Frost wrote:
> > Alright, here's an updated patch which doesn't return any detail if no
> > values are visible or if only a partial key is visible.
>
> I browsed this patch. There's been no mention of foreign key constraints, but
> ri_ReportViolation() deserves similar hardening. If a user has only DELETE
> privilege on a PK table, FK violation messages should not leak the PK values.
> Modifications to the foreign side are less concerning, since the user will
> often know the attempted value; still, I would lock down both sides.

Done.

> Please add a comment explaining the safety of each row-emitting message you
> haven't changed. For example, the one in refresh_by_match_merge() is safe
> because getting there requires MV ownership.

Done.

[...]
> Instead of duplicating an entire ereport() to change whether you include an
> errdetail, use "condition ? errdetail(...) : 0".

Done.

I've also updated the commit message to note the assigned CVE.

One remaining question is about single-column key violations. Should we
special-case those and allow them to be shown or no? I can't see a
reason not to currently but I wonder if we might have cause to act
differently in the future (not that I can think of a reason we'd ever
need to).

Certainly happy to change the specific messages around, if folks would
prefer something different from what I've chosen. I've kept errdetail's
for the cases where I feel it's still useful clarification.

Thanks!

Stephen