feat(go-server): add function allowlist

[derekperkins]

Actually enforcing an allowlist is easy, especially since we already had the blocklist implementation. The majority of this PR was building an inventory of function names of both core and core extensions, categorizing them as either compute only and thus safe / side-effect free, and elevated risk functions that might read across the network, have side-effects, etc. We're including all compute-only functions in the default allowlist and none of the elevated risk functions.

If there was interest for the other servers to implement something similar, we could move the inventory to a separate json file they could all consume. For upgrades, the worst case scenario of drift is that newly introduced functions aren't automatically included in the default, but can be trivially added by the user. I added a specific AGENTS.md file in that directory so future agents can update the list.

I'm also clarifying that the CLI here is not intended to support 100% of the custom options. It's built to provide a decent experience out of the box, but for production use cases, the expectation is users will provide their own main.go implementation, importing these libraries.

[derekperkins]

Reviewed the four commits. The feature is well-shaped: Exec stays closed, the validator fails closed on unknown nodes, the nil-vs-empty distinction is carried end to end from the flag through to functionAllowlistConfigured, and the README is unusually honest about what a syntactic policy can't do.

Verified locally on aa374e6 (Go 1.26.5, darwin/arm64, DuckDB 1.5.5): go build, go test -count=1, go vet (all -tags=duckdb_arrow) and golangci-lint run are clean, matching CI.

I also probed the validator's coverage independently, serializing ~60 SQL constructs through json_serialize_sql on 1.5.5 to look for function invocations the FUNCTION/WINDOW class check would miss. It holds up — DESCRIBE, recursive CTEs, UNION branches, lambda bodies, named WINDOW clauses and FILTER clauses all get walked, and CAST/IN/BETWEEN/array-index resolve to non-function classes, which is the right call. count_star in the README example is the correct name for count(*).

Seven comments below. Nothing blocking; the first is the one I'd most want a decision on before this ships.

— Claude Opus 5

[derekperkins]

Re-reviewed d5e9f39b..26654219. These are labelled refactor, but they revert both code fixes from this review round, so I re-ran the matrices that verified them.

Build, go test -count=1, go vet, and golangci-lint run are all clean on 26654219 — this is about behavior, not breakage.

Three comments. The qualifier change is a judgment call where I think you were half right — I've conceded the part you got right with the AST evidence, and proposed a ~15-line structural alternative that needs no lexer and measures at 0 false positives / 0 misses. The dedup removal I think is unintentional collateral.

— Claude Opus 5

[derekperkins]

Reviewed 7c9b0b4f..7da5b64a — five commits adding the reviewed default inventory, the include/exclude/defaults plumbing, and docs.

I spent most of this pass auditing the inventory itself, since a default allowlist is the security artifact here. It holds up. All 482 names are real DuckDB 1.5.5 functions with zero stale entries, the only table functions are five pure generators, and every file/network/catalog/SQL-executing function in the catalog is absent — I checked the full read_*/glob/duckdb_*/query/json_execute/getenv/currval surface explicitly. Coverage against packages/mosaic/sql/src is complete for core DuckDB.

The API change reads well too: FunctionAllowlistOptions with Include/Exclude/DisableDefaults is clearer than the old nil-means-deny-all overload, exclusions correctly take precedence, and deny-all is still expressible (DisableDefaults: true with no includes → size 0). Worth noting for anyone reading the diff that --function-allowlist= silently changed meaning — deny-all before, defaults-only now — but the README documents the new idiom and nothing is released yet.

Build, tests, vet, and golangci-lint are clean on 7da5b64.

Two comments, both about inventory completeness rather than safety: a macro-coverage gap where the exclusions look accidental rather than decided, and a docs note about Mosaic's own spatial functions.

— Claude Opus 5

[derekperkins]

Re-reviewed 91a76754..7752d6ae. The big change is 7fd27011, which puts every core extension's compute list into
DefaultFunctions — 542 names to 864. That is a large jump in default-allowed surface, so I audited the split rather
than the diff.

It holds up. The compute/elevated cut is correct everywhere I could check it:

parquet    compute: variant_bytes_to_variant, variant_to_parquet_variant
           elevated: read_parquet, parquet_scan, parquet_schema, parquet_*metadata, add_parquet_key
json       compute: json_extract/transform/valid/... (33 pure value operations)
           elevated: read_json, read_json_auto, read_json_objects, json_execute_serialized_sql
spatial    compute: 160 pure st_* geometry ops, no reader or writer
           elevated: st_read, st_readosm, st_readshp, st_read_meta, st_transform, shapefile_meta, rtree_index_*
iceberg    compute: iceberg_bucket, iceberg_truncate (partition-transform math)     elevated: 14
postgres   compute: postgres_hstore_get, postgres_hstore_to_json                    elevated: 8
ducklake   compute: murmur3_32                                                      elevated: 21
motherduck compute: (none)                                                          elevated: 198
mysql / sqlite / odbc / ui / aws / azure / lance / vortex / avro / unity_catalog: compute 0

st_transform in elevated is a nice touch — it is pure math by signature but loads PROJ data from disk.

Verifications I ran beyond reading the lists:

• Disjointness holds: no name in DefaultFunctions() appears in any extension's elevated list. TestCoreExtensionPolicies enforces this mechanically, which is the invariant that actually matters.
• Nothing non-deterministic leaked in via the extension sets: no now, current_date, current_timestamp, today, transaction_timestamp, random, uuid, nextval, currval, getenv. The care taken over the builtin set survived the expansion.
• Names are real. For the four extensions I can load locally (json, parquet, icu, autocomplete), all 243 listed names exist in the catalog — zero fabricated entries.
• httpfs 0/0 is correct, not an oversight. I diffed duckdb_functions() before and after LOAD httpfs: it registers no SQL functions at all.
• Six compute entries are DuckDB table functions (json_each, json_tree, icu.range, icu.generate_series, icu_calendar_names, check_peg_parser). All operate on values rather than resources — I confirmed check_peg_parser('SELECT 1') just returns success from the PEG parser. Table-ness alone isn't the risk property, so this is fine.

1ef7c94b also covers the attached-table/view clarification, and more tightly than my pending suggestion — adding "catalog integrity ... remain the security boundary" is the part I had missed, since a view definition is what decides what actually gets read. I've closed that suggestion as redundant.

Build, tests, go vet, and golangci-lint are clean on 7752d6ae.

One comment, about keeping this correct rather than about it being wrong today.

— Claude Opus 5

[derekperkins]

Re-reviewed e3051281. It is a file split, and for a reorganization of security-relevant data the only question worth
answering is whether the data survived it unchanged. It did.

I dumped the full resolved state at both commits — every DefaultFunctions() entry plus each extension's Compute() and
Elevated() lists — and diffed:

inv-7752d6ae.txt  1613 lines
inv-e3051281.txt  1613 lines
diff: (empty)

864 defaults and 355 elevated names across 29 extensions, byte-identical. Outside pkg/functionset the only change is
one line of AGENTS.md documenting the new filename convention, so nothing else could have shifted.

Build, go test -count=1, go vet, and golangci-lint run are clean on e3051281.

The one-file-per-extension layout is a real improvement for the thing I flagged last round: the upgrade review is now
per-extension rather than a scan through two 250-line files, and each file carries its own provenance. Details in the
open thread.

— Claude Opus 5

[derekperkins]

Re-reviewed e3051281..3db0eae4. 656eb8e7 drops --function-allowlist-exclude and --function-allowlist-defaults,
and the docs commits condense the Function Policies section from 295 to 244 lines. I checked the condensation for lost
substance and it holds up: the limitation paragraphs collapse into one sentence that still names binding, argument
inspection, macro and view expansion, recursive SQL strings, replacement scans, and attached-table binding, and the
current-time rationale, the CURRENT_DATE-is-not-a-function-node nuance, the ST_Read loader caveat, and the Go
Exclude/DisableDefaults semantics all survive. The parquet.go and spatial.go provenance pins I mentioned last round
are now explicit revisions.

I also compiled the README's Go example rather than eyeballing it — append(functionset.Spatial.Elevated(), "my_function")
resolves to 878 names, and the API shape matches the new ExtensionFunctions constants.

Build, tests, go vet, and golangci-lint clean on 3db0eae4.

Two comments, one on the CLI reduction and one small docs point.

— Claude Opus 5