Escape the values in the duckdb secret statements - #1446
Merged
Conversation
The credentials for S3, GCS and Azure are stored with CREATE SECRET, built by interpolation. Two of the values come from the contract rather than the environment: endpointUrl, and the storage account derived from location. A contract can be a URL someone else published, and testing one is a documented workflow. A single quote is a legal URI sub-delimiter, so the schema's format: uri check passes it through. It then ended the string literal and duckdb parsed the rest as SQL, reported as a parser error at the injected text. Every interpolated value is escaped now.
📖 Docs previewPreview site: https://brave-water-0cee36e03-1446.westeurope.7.azurestaticapps.net Changed pages (most-changed first):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The S3, GCS and Azure credentials are handed to duckdb with
CREATE SECRET, built by string interpolation. Two of the interpolated values come from the contract rather than the environment:server.endpointUrl— used for MinIO and other S3-compatible storesserver.locationfor AzureA data contract can be a URL someone else published, and testing one is a documented workflow — the quickstart does exactly that.
The schema check does not stop it
endpointUrlis validated asformat: uri, which rejects an obvious payload containing a space. But a single quote is a legal sub-delimiter in RFC 3986, so a valid URI can carry one. With this contract:datacontract testreported:The quote ended the string literal and duckdb parsed the remainder as SQL. I stopped at establishing that — no exploit chain was built — but duckdb can read and write local files and install extensions, so the ceiling is high for something reachable by testing a third-party contract.
After the fix the same contract yields:
— the payload is now a literal endpoint string, rejected by the HTTP layer, with the SQL intact.
The fix
One helper escaping single quotes, applied to all 26 interpolations across the three
setup_*_connectionfunctions — credentials, region, endpoint, URL style, tenant and client ids, and the derived storage account. Credentials are escaped too: an access key containing a quote would previously have broken the statement.Testing
Two new tests: a quote in
endpointUrlstays inside the literal, and a quote in a credential is escaped. The existing test that a custom endpoint still uses path-style addressing continues to pass, so the MinIO feature is unaffected.The MinIO-backed suites could not run locally (no Docker daemon at the time); CI covers them.