Test out and fix non-superuser behaviour

[JelteF]

Right now only a Postgres superuser can use the extension. Any other user will get the following error:

ERROR:  XX000: (PGDuckDB/CreatePlan) Executor Error: permission denied for sequence secrets_table_seq
LOCATION:  DuckDBFunctionGuard, pgduckdb_utils.hpp:98

This is obviously by accident. And can be easily fixed using a command like:

GRANT ALL ON SEQUENCE duckdb.secrets_table_seq TO PUBLIC; -- Probably not use ALL though

But doing so is currently extremely bad security-wise. Because now, any user can simply read from all tables in any of schemas they have access to by simply enabling duckdb.execution:

> select * from main.foo4;
ERROR:  42501: permission denied for table foo4
> set duckdb.execution = TRUE;
SET
> select * from main.foo4;
 a
───
 1
(1 rows)

We should be checking ACLs of tables and columns before executing the query to see if the current user has access to them. A very crude, but possibly easy way of doing that is by invoking the Postgres planner on the query again like we did before #247, i.e. we don't actually use the resulting plan, we just have the postgres planner check the permissions on the query.

Also we should probably just bail out in IsAllowedStatement if there's row level security set up on any of the tables.

[wuputah]

Good catch on the sequence. I think that's a slightly tangential and certainly unintentional issue.

A very crude, but possibly easy way of doing that is by invoking the Postgres planner on the query again like we did before #247, i.e. we don't actually use the resulting plan, we just have the postgres planner check the permissions on the query.

+1 to this approach as it's basically what I assumed is/was/should be happening. Not sure I would classify it as crude 😂 but instead "leveraging Postgres internals", but that's just marketing.

RLS should definitely be a feature for later... I can see some cool stuff here if we can get it right. For now if we can bail when we detect it (or fallback to Postgres execution) that sounds fine to me.

[JelteF]

Not sure I would classify it as crude

I mainly meant it was crude, because in theory there's no reason for Postgres to fully plan the query, we only really care about the permission checks that it does . But yeah I agree it's definitely good enough for now.