Skip to content

[DBA-140] Fix BigQuery credentials_json auth in DuckDB ATTACH#271

Open
gasparian wants to merge 6 commits into
mainfrom
andrei.gasparian/DBA-140-fix-bigquery-credentials-json
Open

[DBA-140] Fix BigQuery credentials_json auth in DuckDB ATTACH#271
gasparian wants to merge 6 commits into
mainfrom
andrei.gasparian/DBA-140-fix-bigquery-credentials-json

Conversation

@gasparian
Copy link
Copy Markdown
Contributor

@gasparian gasparian commented Mar 26, 2026

Summary

  • Fix Unknown key in connection string: credentials_json error when using BigQuery with service account credentials
  • DuckDB's BigQuery extension does not accept any credentials in the ATTACH connection string — auth must go through CREATE SECRET
  • Adds _create_secret_if_needed() that uses SERVICE_ACCOUNT_PATH or SERVICE_ACCOUNT_JSON via DuckDB's secret manager

Changes

Change 1: Use CREATE SECRET for BigQuery auth

Credentials are no longer passed in the ATTACH connection string. Instead, register_in_duckdb creates a DuckDB secret with the appropriate auth parameter before attaching.

Affected files
  • databao/agent/databases/bigquery_adapter.py

Change 2: Update tests

Tests now verify that CREATE SECRET is called with the correct parameters and that credentials are excluded from the ATTACH SQL.

Affected files
  • tests/test_bigquery_adapter.py

Test Plan

  • All 34 BigQuery adapter unit tests pass
  • make check passes (ruff, mypy)
  • Manually verified against real BigQuery (datalore-internal project) with both credentials_json and credentials_file auth types

🤖 Generated with Claude Code

DuckDB's BigQuery extension only recognizes credentials_file (a file path),
not credentials_json. When BigQueryServiceAccountJsonAuth is used, write the
JSON credentials to a temporary file and substitute credentials_file in the
connection string.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gasparian gasparian requested a review from a team March 26, 2026 16:22
…ion string

DuckDB's BigQuery extension does not accept credentials in the ATTACH
connection string. Use CREATE SECRET with SERVICE_ACCOUNT_PATH or
SERVICE_ACCOUNT_JSON to pass credentials via DuckDB's secret manager,
which is the mechanism the extension actually supports.

Verified manually against real BigQuery with both credentials_json
and credentials_file auth types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes BigQuery authentication with DuckDB by moving service-account credentials out of the ATTACH connection string (which DuckDB’s BigQuery extension rejects) and into DuckDB’s secret manager.

Changes:

  • Create a DuckDB secret for BigQuery auth (service account file or inline JSON) before running ATTACH.
  • Remove credentials from the BigQuery ATTACH connection string generation.
  • Update unit tests to assert CREATE SECRET behavior and ordering.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
databao/agent/databases/bigquery_adapter.py Adds _create_secret_if_needed() and updates DuckDB registration flow to use secrets for BigQuery auth.
tests/test_bigquery_adapter.py Updates tests to validate secret creation, exclude creds from ATTACH, and assert correct call ordering.

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

Comment thread databao/agent/databases/bigquery_adapter.py Outdated
Address Copilot review feedback:
- Use DuckDB parameter binding (?) instead of string interpolation
  to prevent SQL injection and handle special characters safely
- Use CREATE OR REPLACE SECRET for idempotency
- Escape double quotes in secret name

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

Comment thread databao/agent/databases/bigquery_adapter.py Outdated
Comment thread databao/agent/databases/bigquery_adapter.py Outdated
- Use TYPE bigquery (lowercase) in CREATE SECRET for consistency with
  ATTACH and other adapters
- Escape double quotes in name for ATTACH SQL identifier

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


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

Comment thread databao/agent/databases/bigquery_adapter.py Outdated
Prevent invalid SQL / injection if additional_properties values
contain single quotes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


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

Comment thread databao/agent/databases/bigquery_adapter.py
…tion string

Prevent credentials_json/credentials_file from leaking into the ATTACH
connection string if they end up in additional_properties.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants