duckdb-server-go: --function-blocklist flag has no effect and exec command bypasses schema validation

[danielbodart]

Summary

While investigating multi-tenant security for duckdb-server-go (following up on #817), I found two bugs and an additional security consideration:

1. --function-blocklist flag is not wired up - The flag is parsed and the validator exists and is tested, but it's never actually applied to queries
2. exec command bypasses all validation - Schema validation via --schema-match-headers only applies to arrow and json commands, not exec
3. Shared preagg schema exposes cross-tenant data - If multiple tenants share the default mosaic preagg schema, aggregated data leaks between tenants

Issue 1: Function blocklist is dead code

The --function-blocklist flag is parsed in main.go:40-42 and passed to options via query.WithFunctionBlocklist() at line 89:

// main.go:40-42
var functionBlocklist []string
if *functionBlocklistStr != "" {
    functionBlocklist = strings.Split(*functionBlocklistStr, ",")
}

// main.go:89
query.WithFunctionBlocklist(functionBlocklist),

The option is stored in Options.FunctionBlocklist (options.go:26), and a working validator exists (validator.go:208-259).

However, query.New() never reads FunctionBlocklist from the options, and the validator is never instantiated in production code.

The tests pass because they create the validator directly:

// validator_test.go:213-214
if len(tt.functionBlocklist) > 0 {
    validators = append(validators, newFunctionBlocklistValidator(tt.functionBlocklist))
}

Impact: Users who set --function-blocklist "read_parquet,iceberg_scan" expecting to block these functions are not protected.

Issue 2: exec command bypasses schema validation

In server.go:260-268, schema validation is only applied to arrow and json commands:

switch *params.Type {
case CommandExec:
    return nil, false, s.db.Exec(ctx, *params.SQL)  // No allowedSchemas!

case CommandArrow:
    return s.db.QueryArrow(ctx, *params.SQL, allowedSchemas, useCache)

case CommandJSON:
    return s.db.QueryJSON(ctx, *params.SQL, allowedSchemas, useCache)
}

And in query.go:186-193, Exec() runs queries directly without any validation:

func (db *DB) Exec(ctx context.Context, query string) error {
    _, err := db.db.ExecContext(ctx, query)  // No ValidateSQL call
    if err != nil {
        return fmt.Errorf("query: failed to execute query: %w", err)
    }
    return nil
}

Impact: A malicious client can bypass schema isolation. Since exec doesn't return results, the attack vectors are data exfiltration via COPY/INSERT and destructive operations:

Data exfiltration via INSERT:

{"type": "exec", "sql": "INSERT INTO my_tenant.stolen SELECT * FROM other_tenant.sensitive_data"}

Data exfiltration via COPY:

{"type": "exec", "sql": "COPY (SELECT * FROM other_tenant.sensitive_data) TO '/tmp/exfil.parquet'"}

Destructive attacks:

{"type": "exec", "sql": "DROP TABLE other_tenant.data"}
{"type": "exec", "sql": "DELETE FROM other_tenant.bookings WHERE 1=1"}
{"type": "exec", "sql": "UPDATE other_tenant.bookings SET revenue = 0"}

Issue 3: Shared preagg schema exposes cross-tenant data

Even with schema validation fixed for exec, there's an additional multi-tenant concern with Mosaic's pre-aggregation feature.

Mosaic creates materialized tables in the configured preagg.schema (default: mosaic):

CREATE TABLE IF NOT EXISTS mosaic.preagg_<hash> AS
  SELECT client_key, sum(revenue), count(*)
  FROM client_abc.bookings
  GROUP BY ...

If multiple tenants share the same mosaic schema:

• Tenant A's preagg tables contain their aggregated data (sums, counts, averages)
• Tenant B could query or DROP Tenant A's preagg tables
• Even aggregated data may be sensitive (total revenue, booking counts, etc.)

Impact: Cross-tenant data exposure via shared preagg tables, even when source data schemas are properly isolated.

Suggested Fix

1. Wire up function blocklist: Store FunctionBlocklist in the DB struct and create the validator in WriteJSON/WriteArrow

2. Apply validation to Exec: Pass allowedSchemas to Exec() and call ValidateSQL() with the same validators

3. Per-tenant preagg schemas: For multi-tenant deployments, the Mosaic coordinator should be configured with a per-tenant preagg.schema (e.g., mosaic_client_abc instead of shared mosaic), and this schema should be included in the --schema-match-headers allowed list alongside the data schema

4. Documentation: The multi-tenant security model should be documented, making clear that:

   • --schema-match-headers only protects arrow/json commands currently
   • exec requires additional protection for full isolation
   • Preagg schemas must be isolated per-tenant to prevent cross-tenant data exposure

Alternative: Full Isolation

It's worth noting that MotherDuck's approach to multi-tenancy is full isolation - separate Ducklings per tenant. This sidesteps all the complexity above but requires more infrastructure. For some use cases (like entities moving between tenants), shared infrastructure with proper validation may still be preferable.

Happy to submit a PR if this approach is acceptable.

Environment

• duckdb-server-go from mosaic main branch
• Related discussion: Security + multi-tenancy #817

[derekperkins]

Thanks for looking into that. I think 1 and 2 seem like good fixes. Long term, I want to replace my custom parsing logic with an extension, but that will still need to be wired up.

• go: replace custom table parser #947

3 and 4 are already documented, feel free to clarify if you think it's needed
https://github.com/uwdata/mosaic/tree/main/packages/server/duckdb-server-go#multi-tenant-access-control

[danielbodart]

Ahh yes sorry missed the doc on option 3. So an example of using preagg.schema would be great, is it like vg.coordinator({"preagg.schema": "tenant-1"}) or is it like vg.coordinator({preagg: { schema: "tenant-1" }}) or is it only supported by the constructor and I have to set that coordinator as the default that vg uses somehow?

On a more general point, do the current docs represent what is should do in the future or what it currently does?

Lastly I would be happy to help here if you are open to pull requests? Also happy to explore using more stable SQL parsing to set the foundations for all of this work. Totally open to critique and guidance, see real potential for solving a number of real issues I have with my works current analytics but for that to be a reality I need rock sold tenant isolation without MotherDuck style just separate the datasets (as my tentants are actually arbitary collections of entities that can change per user.) This might mean my use case is too complex for this setup but I'd like to get single tenant isolation working before I move on more complex versions.

[danielbodart]

Ahh I just noticed you can do coordinator().preaggregator.schema = 'tenant_123'; as well

[derekperkins]

@danielbodart thanks for the deep dive, I appreciate the extra eyeballs. Definitely open to PRs

Ahh yes sorry missed the doc on option 3. So an example of using preagg.schema would be great, is it like vg.coordinator({"preagg.schema": "tenant-1"}) or is it like vg.coordinator({preagg: { schema: "tenant-1" }}) or is it only supported by the constructor and I have to set that coordinator as the default that vg uses somehow?

In the Go docs, I just linked to the primary Mosaic api documentation, where the preagg is set in the constructor.
https://idl.uw.edu/mosaic/api/core/coordinator.html#constructor

Ahh I just noticed you can do coordinator().preaggregator.schema = 'tenant_123'; as well

I'm not totally sure on the javascript side, but my gut feeling is that changing that during processing is likely to have some race conditions. For my anticipated use case, my customers will be logged into a tenant, so client side would generally not be switching preagg schemas. If they did switch to another tenant, I would need to change it, but it feels like it might just be safer to throw away the coordinator and instantiate a new one at that point. @domoritz would have a better idea

On a more general point, do the current docs represent what is should do in the future or what it currently does?

I'm not quite sure which part you're referring to. Generally the docs represent the current state of things. The need to tie into authentication makes this specific feature trickier, since it is relying on trusted headers, which would have to be injected / validated by an intermediary api gateway. If there's a different approach you think would be useful, I'm open to hearing your use case.

for that to be a reality I need rock sold tenant isolation without MotherDuck style just separate the datasets (as my tentants are actually arbitary collections of entities that can change per user.) This might mean my use case is too complex for this setup but I'd like to get single tenant isolation working before I move on more complex versions.

I don't think there's anything you're describing that is going to be too complex. We're still working on our client side mosaic integration, so we have yet to deploy this into production, hence the lack of fully wiring everything up you called out here.

I'm very grateful to have someone else thinking about this, so it's not just catering to my specific use cases, and to help make the api and design fully featured.