CXH-2380: reach the DB2 native DSN form through connector config

.github/workflows/ci.yaml

@@ -49,7 +49,7 @@ jobs:
           baton-principal-type: user
           bad-credentials: DB_PASSWORD=invalid
       - name: Run account provisioning tests
-        uses: ConductorOne/github-workflows/actions/account-provisioning@v3
+        uses: ConductorOne/github-workflows/actions/account-provisioning@v4
         with:
           connector: ./baton-sql
           account-email: robert.tables2@example.com

docs/db2.md

@@ -110,12 +110,20 @@ Query parameters are forwarded as additional DB2 connection keywords
 (`HOSTNAME`, `DATABASE`, `PORT`, `PROTOCOL`, `UID`, `PWD`) are rejected — use the native
 form below for full control.
 
-DB2's native form is also accepted as-is:
+DB2's native form is also accepted as-is (ODBC keywords are case-insensitive and may carry
+spaces after each `;`):
 
 ```
 HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP
 ```
 
+The native form is self-contained: it already carries the host, port, credentials, params and
+target database. It is therefore mutually exclusive with the structured `connect` fields
+(`host`, `port`, `user`, `password`, `params`) and with a per-database override (`connect.database`
+or the `databases` block for multi-database sync). Combining them is rejected with an explicit
+error rather than silently ignoring the extra settings, so use the `db2://` URL form when you
+need multi-database discovery or want to supply fields separately.
+
 ## Writing a Db2 spec
 
 Db2 needs two things in every spec. Other engines need them only in spots (Oracle folds

pkg/bsql/offline_validate.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"net/url"
 	"strings"
+
+	"github.com/conductorone/baton-sql/pkg/database/db2"
 )
 
 // OfflineValidate performs YAML-level structural checks without opening a DB or
@@ -102,6 +104,12 @@ func resolveConnectScheme(c *DatabaseConfig) (string, error) {
 	if dsn == "" {
 		return "", errors.New("connect: scheme or dsn is required")
 	}
+	// A native DB2 DSN (HOSTNAME=...;DATABASE=...) carries no scheme prefix. Classify it
+	// via the shared detector so this check matches pkg/database's routing and does not
+	// misread a "://" inside a value as a scheme.
+	if db2.IsNativeDSN(dsn) {
+		return "db2", nil
+	}
 	// Placeholders like postgres://${HOST}/db — peel scheme before parse when possible.
 	if idx := strings.Index(dsn, "://"); idx > 0 {
 		return strings.ToLower(dsn[:idx]), nil

pkg/bsql/offline_validate_test.go

@@ -124,6 +124,27 @@ func TestRejectNonV1_NonPostgresScheme(t *testing.T) {
 	require.Contains(t, strings.ToLower(err.Error()), "postgres")
 }
 
+func TestRejectNonV1_NativeDB2DSNRejectedAsDB2(t *testing.T) {
+	// A native DB2 DSN carries no scheme prefix; it must be classified as "db2" (via the
+	// shared detector), so v1 rejects it with the scheme message rather than the confusing
+	// "scheme missing from dsn". Also guards against a "://" inside a value misclassifying it.
+	for _, dsn := range []string{
+		"HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=u;PWD=p;PROTOCOL=TCPIP",
+		"HOSTNAME=localhost;DATABASE=TESTDB;PWD=my://secret",
+	} {
+		cfg, err := Parse([]byte(minimalPostgresYAML()))
+		require.NoError(t, err)
+		cfg.Connect.Scheme = ""
+		cfg.Connect.DSN = dsn
+		s, err := resolveConnectScheme(&cfg.Connect)
+		require.NoError(t, err)
+		require.Equal(t, "db2", s)
+		err = RejectNonV1ProductFeatures(cfg)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "db2")
+	}
+}
+
 func TestRejectNonV1_PostgresqlAliasRejected(t *testing.T) {
 	cfg, err := Parse([]byte(minimalPostgresYAML()))
 	require.NoError(t, err)

pkg/connector/connector.go

@@ -98,7 +98,7 @@ func (c *Connector) Validate(ctx context.Context) (annotations.Annotations, erro
 	for name, db := range c.dbs {
 		if err := db.PingContext(ctx); err != nil {
 			if authErr := database.AuthError(err); authErr != nil {
-				return nil, authErr
+				return nil, fmt.Errorf("database %q: %w", name, authErr)
 			}
 			return nil, fmt.Errorf("database %q ping failed: %w", name, err)
 		}

pkg/database/autherror.go

@@ -4,32 +4,38 @@ import (
 	"errors"
 	"strings"
 
+	"github.com/conductorone/baton-sdk/pkg/uhttp"
+	"github.com/conductorone/baton-sql/pkg/database/db2"
 	"github.com/go-sql-driver/mysql"
 	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 const mysqlAccessDenied = 1045
 
-// AuthError returns an Unauthenticated gRPC status when err is a database
-// authentication/authorization failure, or nil otherwise. SQLSTATE class 28
-// ("invalid authorization") is the ANSI code drivers report on bad credentials
-// (Postgres/Redshift/Vertica/etc. surface it via SQLState()); MySQL is the
-// exception, reporting error 1045 with no SQLSTATE.
+// AuthError wraps err in an Unauthenticated gRPC status when it is a database
+// authentication failure, or returns nil otherwise. Each driver surfaces bad
+// credentials differently: SQLSTATE class 28 via a SQLState() method (Postgres/
+// Redshift), MySQL error 1045 with no SQLSTATE, and DB2 in a driver-specific
+// field (see db2.IsAuthError). Drivers that expose SQLSTATE only as a struct field
+// (Vertica) or not at all (Oracle, MSSQL, SAP HDB) are not covered and fall through
+// to a generic ping error. Wrapping keeps the original error for errors.As.
 func AuthError(err error) error {
-	if err == nil {
+	if err == nil || !isAuthFailure(err) {
 		return nil
 	}
+	return uhttp.WrapErrors(codes.Unauthenticated, "database authentication failed", err)
+}
 
+func isAuthFailure(err error) bool {
 	var sqlState interface{ SQLState() string }
 	if errors.As(err, &sqlState) && strings.HasPrefix(sqlState.SQLState(), "28") {
-		return status.Error(codes.Unauthenticated, "database authentication failed")
+		return true
 	}
 
 	var myErr *mysql.MySQLError
 	if errors.As(err, &myErr) && myErr.Number == mysqlAccessDenied {
-		return status.Error(codes.Unauthenticated, "database authentication failed")
+		return true
 	}
 
-	return nil
+	return db2.IsAuthError(err)
 }

pkg/database/database.go

@@ -18,6 +18,8 @@ import (
 	"github.com/conductorone/baton-sql/pkg/database/postgres"
 	"github.com/conductorone/baton-sql/pkg/database/sqlserver"
 	"github.com/conductorone/baton-sql/pkg/database/vertica"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 var DSNREnvRegex = regexp.MustCompile(`\$\{([A-Za-z0-9_]+)\}`)
@@ -361,13 +363,26 @@ func ResolveDatabaseName(opts ConnectOptions) string {
 			return expanded
 		}
 	}
+	if _, database, isNativeDB2, err := nativeDB2DSN(opts); err == nil && isNativeDB2 {
+		return database
+	}
 	parsedUrl, err := buildConnectionURL(opts)
 	if err != nil || parsedUrl == nil {
 		return ""
 	}
 	return strings.TrimPrefix(parsedUrl.Path, "/")
 }
 
+// hasStructuredConnectFields reports whether opts carries any structured connect
+// field that a native DB2 DSN would make redundant. A native DSN is self-contained;
+// combining it with these (or a per-database override) silently drops them, so the
+// caller rejects the combination. Scheme is excluded: "db2" alongside a native DSN
+// is a supported, explicit hint.
+func hasStructuredConnectFields(opts ConnectOptions) bool {
+	return opts.Host != "" || opts.Port != "" || opts.User != "" ||
+		opts.Password != "" || opts.Database != "" || len(opts.Params) > 0
+}
+
 // ConnectMany opens one *sql.DB per name in dbNames. On any per-database failure,
 // every handle opened so far is closed before returning the error.
 func ConnectMany(ctx context.Context, opts ConnectOptions, dbNames []string) (map[string]*sql.DB, DbEngine, error) {
@@ -404,13 +419,41 @@ func ConnectMany(ctx context.Context, opts ConnectOptions, dbNames []string) (ma
 }
 
 func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error) {
+	// A native DB2 DSN is an opaque ODBC keyword=value string, not a URL. Routing it
+	// through buildConnectionURL corrupts it (url.Parse/.String mangles the opaque form),
+	// so hand it to the driver verbatim. See docs/db2.md.
+	nativeDSN, _, isNativeDB2, err := nativeDB2DSN(opts)
+	if err != nil {
+		return nil, Unknown, err
+	}
+	if isNativeDB2 {
+		// A native DSN already carries host, port, credentials, params and the target
+		// database. Structured fields or a per-database override (set directly, or by
+		// ConnectMany for databases.static / discovery_query) would be silently dropped
+		// on the verbatim path, so reject the combination instead of connecting to the
+		// wrong database. See docs/db2.md.
+		if hasStructuredConnectFields(opts) {
+			return nil, Unknown, status.Error(codes.InvalidArgument,
+				"native DB2 DSN is self-contained and cannot be combined with structured "+
+					"connect fields (host, port, user, password, params) or a per-database "+
+					"override (connect.database, databases); put every setting in the DSN or "+
+					"use the db2:// URL form",
+			)
+		}
+		db, err := db2.Connect(ctx, nativeDSN)
+		if err != nil {
+			return nil, Unknown, err
+		}
+		return db, DB2, nil
+	}
+
 	parsedDsn, err := buildConnectionURL(opts)
 	if err != nil {
 		return nil, Unknown, err
 	}
 
 	if parsedDsn.Scheme == "" {
-		return nil, Unknown, errors.New("database scheme must be specified in DSN or configuration")
+		return nil, Unknown, status.Error(codes.InvalidArgument, "database scheme must be specified in DSN or configuration")
 	}
 
 	switch parsedDsn.Scheme {
@@ -468,6 +511,43 @@ func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error
 	}
 }
 
+// nativeDB2DSN reports whether opts carries a native DB2 DSN: an opaque ODBC
+// keyword=value string (e.g. "HOSTNAME=...;DATABASE=...") rather than a db2:// URL.
+// When it does, the env-expanded string (for verbatim handoff to the driver) and its
+// DATABASE value are returned. The scheme, when set, must be db2; a URL-shaped DSN or
+// foreign scheme is left to the normal URL path. Detection is db2.ParseNativeDSN, shared
+// with convertToDB2DSN's passthrough, so one pass yields both facts without re-splitting.
+func nativeDB2DSN(opts ConnectOptions) (string, string, bool, error) {
+	if opts.DSN == "" {
+		return "", "", false, nil
+	}
+	lookup := opts.resolveLookup()
+
+	scheme, err := expandValue(opts.Scheme, lookup)
+	if err != nil {
+		return "", "", false, err
+	}
+	if scheme != "" && scheme != "db2" {
+		return "", "", false, nil
+	}
+
+	dsn, err := expandValue(opts.DSN, lookup)
+	if err != nil {
+		return "", "", false, err
+	}
+	if _, native := db2.ParseNativeDSN(dsn); !native {
+		return "", "", false, nil
+	}
+	// Confirmed native: re-expand with keyword-injection validation. The expansion above
+	// only decides routing; the driver gets this string verbatim.
+	safeDSN, err := expandNativeDSN(opts.DSN, lookup)
+	if err != nil {
+		return "", "", false, err
+	}
+	database, _ := db2.ParseNativeDSN(safeDSN)
+	return safeDSN, database, true, nil
+}
+
 func buildConnectionURL(opts ConnectOptions) (*url.URL, error) {
 	var (
 		parsedUrl *url.URL
@@ -605,3 +685,39 @@ func expandValue(s string, lookup LookupFunc) (string, error) {
 	}
 	return s, nil
 }
+
+// expandNativeDSN expands ${KEY} placeholders in a native DB2 DSN, rejecting any expanded
+// value that carries an ODBC keyword separator (; { } =). The db2:// URL path quotes each
+// field with quoteDB2Value, but a native DSN is handed to the driver verbatim, so a
+// placeholder value here could otherwise inject or override DSN keywords.
+func expandNativeDSN(dsn string, lookup LookupFunc) (string, error) {
+	if !DSNREnvRegex.MatchString(dsn) {
+		return dsn, nil
+	}
+	// A DSN that is a single ${KEY} spanning the whole string is the full value, not a
+	// field embedded in literal structure, so its separators are legitimate: expand as-is.
+	if DSNREnvRegex.FindString(dsn) == dsn {
+		return expandValue(dsn, lookup)
+	}
+	if lookup == nil {
+		lookup = os.LookupEnv
+	}
+	var err error
+	result := DSNREnvRegex.ReplaceAllStringFunc(dsn, func(match string) string {
+		varName := match[2 : len(match)-1]
+		value, exists := lookup(varName)
+		if !exists {
+			err = errors.Join(err, fmt.Errorf("environment variable %s is not set", varName))
+			return match
+		}
+		if strings.ContainsAny(value, ";{}=") {
+			err = errors.Join(err, fmt.Errorf("value for %s must not contain ODBC keyword separators (; { } =)", varName))
+			return match
+		}
+		return value
+	})
+	if err != nil {
+		return "", err
+	}
+	return result, nil
+}

pkg/database/db2/autherror_test.go

@@ -0,0 +1,33 @@
+//go:build db2
+
+package db2
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/ibmdb/go_ibm_db"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsAuthError(t *testing.T) {
+	badCreds := &go_ibm_db.Error{Diag: []go_ibm_db.DiagRecord{{State: "28000"}}}
+
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{"class 28 bad credentials", badCreds, true},
+		{"class 28 wrapped", fmt.Errorf("connect: %w", badCreds), true},
+		{"non-auth sqlstate", &go_ibm_db.Error{Diag: []go_ibm_db.DiagRecord{{State: "42501"}}}, false},
+		{"no diag records", &go_ibm_db.Error{}, false},
+		{"unrelated error", fmt.Errorf("boom"), false},
+		{"nil", nil, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, IsAuthError(tt.err))
+		})
+	}
+}

pkg/database/db2/db2.go

@@ -5,9 +5,11 @@ package db2
 import (
 	"context"
 	"database/sql"
+	"errors"
+	"strings"
 	"time"
 
-	_ "github.com/ibmdb/go_ibm_db"
+	"github.com/ibmdb/go_ibm_db"
 )
 
 // Connect establishes a connection to DB2 database.
@@ -35,3 +37,18 @@ func Connect(ctx context.Context, dsn string) (*sql.DB, error) {
 
 	return db, nil
 }
+
+// IsAuthError reports whether err is a DB2 auth failure. go_ibm_db exposes SQLSTATE
+// via Error.Diag[].State, not a SQLState() method, so class 28 must be matched here.
+func IsAuthError(err error) bool {
+	var db2Err *go_ibm_db.Error
+	if !errors.As(err, &db2Err) {
+		return false
+	}
+	for _, rec := range db2Err.Diag {
+		if strings.HasPrefix(rec.State, "28") {
+			return true
+		}
+	}
+	return false
+}

pkg/database/db2/db2_stub.go

@@ -14,3 +14,8 @@ import (
 func Connect(_ context.Context, _ string) (*sql.DB, error) {
 	return nil, errors.New("baton-sql: DB2 support not compiled into this binary; rebuild with -tags db2 (see docs/db2.md)")
 }
+
+// IsAuthError is a stub; the DB2 driver error types are unavailable without -tags db2.
+func IsAuthError(_ error) bool {
+	return false
+}