Greetings, * Peter Geoghegan (p...@bowt.ie) wrote: > On Tue, Sep 22, 2020 at 12:41 PM Robert Haas <robertmh...@gmail.com> wrote: > > But now I see that there's no secondary permission check in the > > verify_nbtree.c code. Is that intentional? Peter, what's the > > justification for that? > > As noted by comments in contrib/amcheck/sql/check_btree.sql (the > verify_nbtree.c tests), this is intentional. Note that we explicitly > test that a non-superuser role can perform verification following > GRANT EXECUTE ON FUNCTION ... .
> As I mentioned earlier, this is supported (or at least it is supported > in my interpretation of things). It just isn't documented anywhere > outside the test itself. Would certainly be good to document this but I tend to agree with the comments that ideally- a) it'd be nice for a relatively low-privileged user/process could run the tests in an ongoing manner b) we don't want to add more is-superuser checks c) users shouldn't really be given the ability to see rows they're not supposed to have access to In other places in the code, when an error is generated and the user doesn't have access to the underlying table or doesn't have BYPASSRLS, we don't include the details or the actual data in the error. Perhaps that approach would make sense here (or perhaps not, but it doesn't seem entirely crazy to me, anyway). In other words: a) keep the ability for someone who has EXECUTE on the function to be able to run the function against any relation b) when we detect an issue, perform a permissions check to see if the user calling the function has rights to read the rows of the table and, if RLS is enabled on the table, if they have BYPASSRLS c) if the user has appropriate privileges, log the detailed error, if not, return a generic error with a HINT that details weren't available due to lack of privileges on the relation I can appreciate the concerns regarding dead rows ending up being visible to someone who wouldn't normally be able to see them but I'd argue we could simply document that fact rather than try to build something to address it, for this particular case. If there's push back on that then I'd suggest we have a "can read dead rows" or some such capability that can be GRANT'd (in the form of a default role, I would think) which a user would also have to have in order to get detailed error reports from this function. Thanks, Stephen
signature.asc
Description: PGP signature
