Skip to content

fix: [AI-266] Snowflake auth — support all auth methods, fix field name mismatches#268

Merged
anandgupta42 merged 6 commits intomainfrom
fix/snowflake-auth
Mar 18, 2026
Merged

fix: [AI-266] Snowflake auth — support all auth methods, fix field name mismatches#268
anandgupta42 merged 6 commits intomainfrom
fix/snowflake-auth

Conversation

@anandgupta42
Copy link
Contributor

What does this PR do?

Fixes Snowflake private key authentication failing with MissingParameterError: A password must be specified. Also adds support for all 8 Snowflake auth methods and hardens field name handling.

Root cause: The warehouse_add tool description only showed a Postgres password example, so the Builder LLM used private_key instead of private_key_path for file paths. The driver only checked private_key_path, so key-pair auth was never triggered.

Changes across 8 files:

File What changed
packages/drivers/src/snowflake.ts Support all 8 auth methods (password, key-pair file+inline, OAuth, SSO, Okta, JWT, programmatic token, MFA). Auto-detect file path vs PEM in private_key. Normalize \\n. Better error messages. connectAsync() for browser SSO.
packages/opencode/src/altimate/tools/warehouse-add.ts Tool description now shows Snowflake examples for password, key-pair, OAuth, SSO auth methods
packages/opencode/src/altimate/native/connections/credential-store.ts Added private_key, privateKey, token, oauth_client_secret, oauthClientSecret, privateKeyPassphrase, passcode to SENSITIVE_FIELDS
packages/opencode/src/altimate/native/connections/registry.ts detectAuthMethod returns sso, oauth, key_pair for all field name variants
packages/opencode/src/altimate/native/connections/dbt-profiles.ts Map private_key, authenticator, oauth_client_id, oauth_client_secret from dbt profiles
packages/opencode/src/altimate/tools/dbt-profiles.ts Use isSensitiveField() instead of hardcoded masking list
packages/opencode/test/altimate/connections.test.ts Tests for sensitive fields, OAuth credential stripping, dbt profile parsing
packages/opencode/test/altimate/telemetry-safety.test.ts Tests for all auth method detection variants

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Issue for this PR

Closes #266

How did you verify your code works?

  • 49 unit tests pass (connections + telemetry-safety)
  • Typecheck passes
  • 6-model consensus code review (Claude + GPT 5.2 + Gemini 3.1 Pro + Kimi K2.5 + MiniMax M2.5 + GLM-5)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

…rivate key and non-password methods

Root cause: the `warehouse_add` tool description only showed a Postgres password example,
so the Builder LLM used `private_key` (not `private_key_path`) for file paths. The driver
only checked `private_key_path`, so key-pair auth was never triggered.

Changes:
- Support all 8 Snowflake auth methods: password, key-pair (file + inline PEM),
  OAuth, external browser SSO, Okta SSO, JWT, programmatic access token, MFA
- Auto-detect whether `private_key` contains a file path or PEM content
- Normalize escaped `\n` in inline PEM from env vars / JSON configs
- Accept both snake_case (dbt) and camelCase (SDK) field name variants
- Add `private_key`, `privateKey`, `token`, `oauth_client_secret`, `passcode`
  and camelCase variants to `SENSITIVE_FIELDS` for secure keychain storage
- Use `isSensitiveField()` in `formatConnections` instead of hardcoded list
- Update `warehouse_add` tool description with Snowflake-specific examples
  for all auth methods so the Builder LLM picks correct field names
- Add `private_key`, `authenticator`, `oauth_client_id`, `oauth_client_secret`
  to dbt-profiles key mapper
- `detectAuthMethod` now returns `sso`, `oauth`, `key_pair` for all variants
- Better error messages for key decryption failures and missing OAuth tokens
- Use `connectAsync()` for browser-based SSO (external browser / Okta)

Closes #266

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

Comment on lines +183 to +185
// Use connectAsync for browser-based auth (SSO/Okta), connect for everything else
const useBrowserAuth = authUpper === "EXTERNALBROWSER" ||
(authenticator && /^https?:\/\/.+\.okta\.com/i.test(authenticator))

This comment was marked as outdated.

Comment on lines +73 to +78
if (!resolvedKeyPath && resolvedInlineKey && !resolvedInlineKey.includes("-----BEGIN")) {
// Looks like a file path, not PEM content
if (fs.existsSync(resolvedInlineKey)) {
resolvedKeyPath = resolvedInlineKey
resolvedInlineKey = undefined
}

This comment was marked as outdated.

Addresses code review findings from 6-model consensus review:
- JWT, PROGRAMMATIC_ACCESS_TOKEN: error if token missing (was silent fallthrough)
- USERNAME_PASSWORD_MFA: error if password missing (was undefined)
- Browser SSO: error if `connectAsync` unavailable instead of silent fallback
- Extract Okta URL regex to avoid duplication
- Fix comment: "Non-interactive" → "Interactive" for browser SSO

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +157 to +162
} else if (authUpper === "JWT") {
if (!oauthToken) {
throw new Error("Snowflake JWT authenticator specified but no token provided (expected 'token' or 'access_token')")
}
options.authenticator = "SNOWFLAKE_JWT"
options.token = oauthToken

This comment was marked as outdated.

Comment on lines +17 to +18
"private_key",
"privateKey",
Copy link

Choose a reason for hiding this comment

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

Bug: When keytar is unavailable, saving a connection with an inline private_key strips and permanently loses the key, causing subsequent connection attempts to fail.
Severity: HIGH

Suggested Fix

Modify saveConnection to not strip the private_key if keytar is unavailable. Instead, rely on the existing warning that advises the user to use environment variables for credentials in such environments. This prevents data loss while still informing the user of the recommended practice.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/opencode/src/altimate/native/connections/credential-store.ts#L17-L18

Potential issue: When a new connection with an inline `private_key` is saved in an
environment where `keytar` is unavailable (e.g., CI/CD or headless servers), the
`saveConnection` function strips the `private_key` from the configuration for security.
However, the stripped configuration is then persisted in memory, permanently losing the
key for the current session. Subsequent connection attempts using this configuration
will fail authentication because the private key is missing.

Addresses Gemini 3.1 Pro code review findings:
- JWT and PROGRAMMATIC_ACCESS_TOKEN: snowflake-sdk Node.js only accepts
  pre-generated tokens via OAUTH authenticator. SNOWFLAKE_JWT expects a
  `privateKey` for self-signing and would crash with TypeError.
- `private_key` containing a non-existent file path now throws a clear
  error instead of passing the path string to `crypto.createPrivateKey`
  which produces a cryptic OpenSSL error.
- Remove `oauthClientId`/`oauthClientSecret` passthrough — the SDK's
  `AuthOauth` class ignores them entirely.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +59 to +60
const oauthClientId = (config.oauth_client_id ?? config.oauthClientId) as string | undefined
const oauthClientSecret = (config.oauth_client_secret ?? config.oauthClientSecret) as string | undefined

This comment was marked as outdated.

Sentry flagged: when keytar is unavailable (CI/headless), `saveConnection`
strips sensitive fields from both the disk config AND the in-memory config.
This causes subsequent `warehouse_test` calls in the same session to fail
because the credentials are permanently lost.

Fix: store the original config (with credentials) in memory so the current
session can connect. Only the disk file uses the sanitized version.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +200 to +201
conn.connectAsync((err: Error | null) => {
if (err) reject(err)

This comment was marked as outdated.

Sentry flagged: `connectAsync()` returns a Promise in addition to accepting
a callback. If the SDK rejects the Promise (instead of calling the callback
with an error), the rejection was unhandled, potentially causing the
connection to hang silently.

Fix: chain `.catch(reject)` to forward Promise-based errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
// ---------------------------------------------------------------
const keyPath = (config.private_key_path ?? config.privateKeyPath) as string | undefined
const inlineKey = (config.private_key ?? config.privateKey) as string | undefined
const keyPassphrase = (config.private_key_passphrase ?? config.privateKeyPassphrase ?? config.privateKeyPass) as string | undefined
Copy link

Choose a reason for hiding this comment

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

Bug: The privateKeyPass field, an alias for the Snowflake private key passphrase, is not included in the SENSITIVE_FIELDS set, causing it to be stored in plaintext.
Severity: HIGH

Suggested Fix

Add "privateKeyPass" to the SENSITIVE_FIELDS set in packages/opencode/src/altimate/native/connections/credential-store.ts to ensure it is treated as a sensitive value and stored securely.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/drivers/src/snowflake.ts#L57

Potential issue: The Snowflake driver accepts `privateKeyPass` as an alias for the
private key passphrase. However, this field is missing from the `SENSITIVE_FIELDS` set
in `credential-store.ts`. As a result, the `saveConnection()` function does not identify
it as a sensitive credential and writes it to the `connections.json` file in plaintext
instead of storing it securely in the system keychain.

Sentry flagged: the driver accepts `privateKeyPass` as a passphrase alias
but it was missing from SENSITIVE_FIELDS, causing plaintext storage in
connections.json.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@anandgupta42 anandgupta42 merged commit 2c94182 into main Mar 18, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Snowflake private key auth fails with MissingParameterError

1 participant