feat(server): add setupSQL support to DuckDB connection configurations

[mathisdrn]

Closes #970.

What this addresses

Currently, environment-authored duckdb connections in @malloy-publisher/server require attachedDatabases to be present, and restrict connection fields via PUBLISHER_DUCKDB_API_FIELDS. Passing setupSQL (which is standard in malloy-cli) throws:

Unsupported DuckDB connection field(s): setupSQL. Publisher only supports attachedDatabases for environment-authored DuckDB connections.

Furthermore, Publisher's native type: ducklake requires a Postgres catalog and Cloud Storage bucket (catalog.postgresConnection & storage.bucketUrl). Adding setupSQL to type: duckdb brings feature parity with malloy-cli and unlocks local file-based DuckLakes (.ducklake), remote HTTP DuckLakes, Iceberg, Delta, and custom DuckDB extensions with zero extra server code.

Changes

1. Schema & API Surface (connection_config.ts & api-doc.yaml):
   • Added setupSQL to PUBLISHER_DUCKDB_API_FIELDS.
   • Added setupSQL (type: string, optional) to DuckdbConnection schema in api-doc.yaml and EnvironmentConnectionMetadata interface.
2. Validation (connection_config.ts):
   • Updated case duckdb validation to allow attachedDatabases to be empty if setupSQL is defined.
3. Execution (connection.ts):
   • Executed setupSQL via duckdbConnection.runSQL(setupSQL) upon initializing the DuckDB session in attachDatabasesToDuckDB.
4. Tests (connection_config.spec.ts):
   • Added unit tests in connection_config.spec.ts verifying validation succeeds with setupSQL and rejects empty configurations without attachedDatabases or setupSQL.

[Copilot]

Pull request overview

This PR adds setupSQL support to environment-authored DuckDB connection configurations in Publisher, aiming to match malloy-cli behavior and allow initializing DuckDB sessions via an optional SQL script.

Changes:

• Expands the allowed DuckDB connection config API surface to include setupSQL (schema + runtime validation).
• Allows DuckDB connections with empty attachedDatabases as long as setupSQL is provided.
• Executes setupSQL during DuckDB session initialization/attachment flow and adds unit tests for the new validation behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
packages/server/src/service/connection.ts	Passes setupSQL into the DuckDB attach/init path and runs it during session initialization.
packages/server/src/service/connection_config.ts	Adds setupSQL to allowed DuckDB config fields, carries it through metadata, and loosens validation to allow setupSQL instead of required attachments.
packages/server/src/service/connection_config.spec.ts	Adds unit tests covering acceptance/rejection of DuckDB configs based on setupSQL presence.
api-doc.yaml	Documents setupSQL on the DuckdbConnection schema.

Suppressed comments (1)

packages/server/src/service/connection_config.ts:468

• hasSetupSQL = !!connection.duckdbConnection.setupSQL will treat whitespace-only strings as present and will also accept non-string JSON values (e.g. numbers/objects) as truthy, letting invalid configs through to a runtime failure in runSQL(). Since these configs are user-authored JSON, it’s safer to validate setupSQL is a non-empty string (after trimming) when provided.

            const attached =
               connection.duckdbConnection.attachedDatabases ?? [];
            const hasSetupSQL = !!connection.duckdbConnection.setupSQL;
            if (attached.length === 0 && !hasSetupSQL) {
               throw new Error(

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[housejester]

Thanks for this. The parity argument with malloy-cli is fair, and points 2 and 3 are real gaps on our side, not user error: type: "ducklake" currently assumes a Postgres catalog plus object storage, and a single-file or local DuckLake catalog is an ordinary thing to want to serve.

I'd like to land this. There's one thing I do need changed first, and three small tidies. Everything else I might have said, I'd rather do in a follow-up than ask you to carry... details at the bottom so you can see those.

The one blocking thing: setupSQL makes EXTENSION_FETCH_POLICY=local-only bypassable

Publisher has a deployment setting for whether a server may fetch DuckDB extensions at runtime. EXTENSION_FETCH_POLICY=local-only means "never fetch: use only what's baked into the image," and it's enforced in two places:

• installAndLoadExtension skips INSTALL entirely under local-only.
• applyExtensionSessionSettings sets autoinstall_known_extensions=false, which stops DuckDB's implicit auto-install.

There's no DuckDB-level lockdown behind those. no enable_external_access=false, no restricted extension_directory. The policy holds because every install goes through that one function.

setupSQL gets around both, in two independent ways:

1. An explicit INSTALL doesn't go through that function. Under local-only, a config containing INSTALL httpfs; LOAD httpfs; isn't skipped by installAndLoadExtension (never called) and isn't stopped by autoinstall_known_extensions=false, which governs implicit autoload rather than
   explicit statements.
2. setupSQL can re-open implicit auto-install for the rest of the connection's life — the Copilot review spotted this one and it's the sharper of the two. A script containing SET autoinstall_known_extensions=true sticks, because applyExtensionSessionSettings early-returns once extensionSessionPinned.has(connection) and the pin check sits ahead of the alwaysDisableAutoinstall option — so none of its callers can re-close it. Where (1) affects only what the script itself pulls in, this affects every query that later runs on that connection.

Either way the setting quietly stops meaning what it says.

I don't think a new env var is needed for this, and I don't want to send you off building config plumbing. The two features just need to not contradict each other: refuse setupSQL when the policy is local-only, with an error that says why. getExtensionFetchPolicy() is already exported from ../config, so in validateDuckdbApiSurface that's an import and a few lines, and the operator gets a clear failure at config load rather than a surprise at session setup.

A deployment on the default on-demand already permits extension fetching, so INSTALL in setupSQL isn't violating anything there and your own use case is unaffected. Only a deployment that has deliberately turned fetching off refuses the field... which is exactly the deployment that shouldn't be running arbitrary SQL at connection setup.

Three small tidies in the same diff

The post-runSQL re-apply can't do what it's there for — and to be clear, that's not on you: moving setupSQL after applyExtensionSessionSettings and re-asserting afterwards is exactly what the earlier review asked for, and you implemented it faithfully. The problem is upstream of the advice. The settings are already applied immediately above the block, and applyExtensionSessionSettings early-returns once the connection is pinned, so the second call is a no-op; and even if it ran it only
re-sets two flags, which can't unload or uninstall what the script already brought in. Given the local-only refusal above makes the question moot, I'd just drop the line rather than leave something that reads like a guard — or keep it with a comment stating what it does and doesn't cover.

setupSQL presence should be a non-empty-string check. !!connection.duckdbConnection.setupSQL treats a whitespace-only string as present, and accepts non-string JSON (a number, an object) as truthy — so an invalid config passes validation and fails later inside runSQL() with a much worse error. This was raised as a suppressed comment on the earlier review and I think it deserved to surface: it's a one-liner, and the config is user-authored JSON.

The new api-doc description promises a control that doesn't exist yet. It says the surface is "subject to Publisher policy," but no policy is added in this PR. Once the local-only refusal above is in, that sentence becomes true, so it may just be a matter of landing them together — worth a re-read at the end either way.

What I'm not asking you for

These are things I'd otherwise raise, and I'd rather own them than expand your PR. Flagging them so you can see the shape of where this goes, and so nothing here reads as a hidden objection:

• A first-class DuckLake attachment. What you /may/ need is "attach this DuckLake and use it," which is declarative intent rather than arbitrary SQL — a ducklake entry in attachedDatabases taking a catalog path, or letting type: "ducklake" accept a local/file catalog. That would keep extension loading inside installAndLoadExtension, give you validation and a real error message instead of a raw DuckDB failure, and resolve your point 2 for free. It's the better long-term answer (but likely requires some diligence on protected local paths on a managed publisher, etc) and it's our design work, not yours. I'll open an issue for it and link this PR.
• A general gate for setupSQL. If shared-service deployments end up wanting to refuse it independently of extension policy, that's a PERSIST_STORAGE_MODE-style env var and it's a maintainer decision about defaults. Not needed to land this.
• Path constraints. #682 notes that "future work owns any path-traversal/allowlist enforcement," and that's still true and still ours. Worth saying plainly: setupSQL widens what a config author can express before that enforcement exists, and I'm accepting that for now because Publisher already declines to claim filesystem isolation — attachedDatabases handling isn't normalized either. This PR isn't the thing that makes that decision; it just makes it more visible.

[housejester]

A colleague pointed this out, so adding it here. It's a smaller change than my last comment asked for: pass setupSQL through the connection pojo, add it to what buildDuckdbEntry returns rather than running it via runSQL.

Malloy's DuckDBConnection already accepts it as an option and Publisher already does exactly this for Databricks (connection_config.ts:1048), so it's about a line. It matters beyond tidiness: getDigest() folds setupSQL in, and that digest feeds computeSourceEntityId, so run out-of-band the script is invisible to build addressing: repoint it at a different .ducklake and artifacts built against the old one still look current and get reused. Going native also gets you replay-after-idle and statement ordering for free (and makes the earlier reorder discussion moot).

One gotcha before you switch: Malloy splits the script on ;\n, not ;, so the ATTACH …; USE …; from your write-up needs each statement on its own line.

This is in addition to the local-only refusal rather than instead of it: the statements still run either way, just executed by Malloy.

[mathisdrn]

Hi James, thanks for the detailed review and for your time.

Disclaimer: This PR is AI generated. I am not proficient in TypeScript and this was initially a ~20 loc PR which I felt confident about. This is not the case anymore.

What I did: Addressed your comments above, ran the test, challenge the PR by another agent.

Addressed all items in 6c276c7f:

1. setupSQL refused under EXTENSION_FETCH_POLICY=local-only — Added a check in validateDuckdbApiSurface that throws at config load when setupSQL is present and the policy is local-only, with an error naming the conflicting setting.
2. Native POJO passing — setupSQL now flows through buildDuckdbEntry into the CoreConnectionEntry, letting Malloy's DuckDBConnection handle execution, getDigest() invalidation, idle replay, and statement ordering natively. The manual runSQL() call and the no-op applyExtensionSessionSettings re-assert have been removed from connection.ts.
3. Non-empty-string validation — All setupSQL checks now use typeof setupSQL === "string" && setupSQL.trim().length > 0.
4. api-doc.yaml — Updated DuckdbConnection description to note the local-only refusal and documented the ;\n statement separator requirement.
5. Test updates — Updated connection.spec.ts to use a truly unsupported field, fixed the "no attachments" test regex, and added 4 new tests covering POJO output, local-only refusal, and whitespace rejection. All 109 tests pass.