go: add function allowlist

[derekperkins]

There's a function blocklist implemented in the Go server, but an allowlist is a much safer way to limit usage. IIRC, the biggest hurdle I encountered when looking at this a few months ago was implicit function execution. For example, a function like

read_parquet('gcs://some/file/that/uses/a/gcs/secret.parquet');

isn't explicitly using the httpfs extension, but the magic gcs or s3 prefix triggers it to run. You might want to allow read_parquet for local files, but not for remote files. I was hoping that the json from json_serialize_sql would expand those use cases to show the underlying function, but it didn't seem to.

Other than that, the tree walking functionality that is already built should accommodate this easily.

cc @danielbodart

[domoritz]

I'm not a fan of anything custom that looks at the SQL query as a string. It's super hard to get this right. Can we use DuckDBs support for read only or their built in parser?

[derekperkins]

That is how the current implementation works. My point here is that their query parser doesn't expand these implicit references to the underlying functions, which is what makes this tougher than expected.
https://github.com/uwdata/mosaic/blob/main/packages/server/duckdb-server-go/internal/query/validator.go#L42-L46

edit: not sure why the line reference is being stripped when clicking on the link

[domoritz]

I see. Then an allowlist makes sense.

[danielbodart]

Funny enough, I actually had already implemented an allow list in my parser_tools PR that I was going to send in as I am a strong believer in uniform APIs.

I also had added a 'SELECT' only option which would disallow INSERT, UPDATE, DELETE etc. Combined with an allow function list this can make it much more robust. Mosaic and user code could then still do CREATE TABLE IF NOT etc to setup data, basically making the whole DB append only (at a table level)

[danielbodart]

On the URL side for parquet files,: local vs remote, one schema(protocol) vs another, that feels like an extreme solvable problem. In fact I'd be extremely surprised if this has not already been done, I'll do some research...

[derekperkins]

It's certainly solvable today, but I don't think it's 100% future proof, as other remote features or glob patterns are added that could bypass detection. A best effort implementation is still worth it IMO