From a942195889d959ddd6f5f1ffbed9e26df1ef978c Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Wed, 2 Sep 2026 11:52:44 -0500 Subject: [PATCH 1/9] CXH-2380: reach the DB2 native DSN form through connector config A native DB2 DSN (opaque ODBC keyword string like HOSTNAME=...;DATABASE=...) was documented in docs/db2.md but unreachable through config: the engine forced every DSN through url.Parse/String, so a scheme-less native form hit "database scheme must be specified" and a native form with scheme:db2 got mangled into db2://HOSTNAME=... and hit "database name is required in DSN path". Connect now detects a native DB2 DSN and hands it to the driver verbatim, bypassing the URL builder. ResolveDatabaseName extracts DATABASE= so the resolved database name matches the equivalent db2://.../DB URL, keeping resource IDs stable across the two DSN forms. --- pkg/database/database.go | 83 +++++++++++++++++++++++ pkg/database/native_db2_dsn_test.go | 96 +++++++++++++++++++++++++++ pkg/database/native_db2_route_test.go | 34 ++++++++++ 3 files changed, 213 insertions(+) create mode 100644 pkg/database/native_db2_dsn_test.go create mode 100644 pkg/database/native_db2_route_test.go diff --git a/pkg/database/database.go b/pkg/database/database.go index 005164ef..a460d266 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -361,6 +361,9 @@ func ResolveDatabaseName(opts ConnectOptions) string { return expanded } } + if nativeDSN, isNativeDB2, err := nativeDB2DSN(opts); err == nil && isNativeDB2 { + return db2DSNDatabase(nativeDSN) + } parsedUrl, err := buildConnectionURL(opts) if err != nil || parsedUrl == nil { return "" @@ -368,6 +371,39 @@ func ResolveDatabaseName(opts ConnectOptions) string { return strings.TrimPrefix(parsedUrl.Path, "/") } +func db2DSNDatabase(dsn string) string { + for _, part := range splitDB2DSN(dsn) { + if value, found := strings.CutPrefix(part, "DATABASE="); found { + if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { + value = value[1 : len(value)-1] + } + return value + } + } + return "" +} + +// splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. +func splitDB2DSN(dsn string) []string { + var parts []string + start := 0 + inBraces := false + for i, r := range dsn { + switch r { + case '{': + inBraces = true + case '}': + inBraces = false + case ';': + if !inBraces { + parts = append(parts, dsn[start:i]) + start = i + 1 + } + } + } + return append(parts, dsn[start:]) +} + // 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,6 +440,21 @@ 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 { + 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 @@ -468,6 +519,38 @@ 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 is returned for verbatim handoff to the driver. +// The scheme, when set, must be db2; a URL-shaped DSN or foreign scheme is left to the +// normal URL path. Markers match db2.convertToDB2DSN's passthrough. +func nativeDB2DSN(opts ConnectOptions) (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 strings.Contains(dsn, "://") { + return "", false, nil + } + if !strings.Contains(dsn, "HOSTNAME=") && !strings.Contains(dsn, "DATABASE=") { + return "", false, nil + } + return dsn, true, nil +} + func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { var ( parsedUrl *url.URL diff --git a/pkg/database/native_db2_dsn_test.go b/pkg/database/native_db2_dsn_test.go new file mode 100644 index 00000000..d6ae3dc8 --- /dev/null +++ b/pkg/database/native_db2_dsn_test.go @@ -0,0 +1,96 @@ +package database + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNativeDB2DSN(t *testing.T) { + const native = "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP" + + lookup := func(m map[string]string) LookupFunc { + return func(k string) (string, bool) { v, ok := m[k]; return v, ok } + } + + tests := []struct { + name string + opts ConnectOptions + wantDSN string + wantOk bool + wantErr string + }{ + {name: "native form no scheme", opts: ConnectOptions{DSN: native}, wantDSN: native, wantOk: true}, + {name: "native form with scheme db2", opts: ConnectOptions{DSN: native, Scheme: "db2"}, wantDSN: native, wantOk: true}, + {name: "database marker only", opts: ConnectOptions{DSN: "DATABASE=TESTDB;HOST=x"}, wantDSN: "DATABASE=TESTDB;HOST=x", wantOk: true}, + { + name: "native form with placeholders", + opts: ConnectOptions{ + DSN: "HOSTNAME=${DB_HOST};PORT=50000;DATABASE=${DB_NAME};UID=u;PWD=p", + Lookup: lookup(map[string]string{"DB_HOST": "h", "DB_NAME": "d"}), + }, + wantDSN: "HOSTNAME=h;PORT=50000;DATABASE=d;UID=u;PWD=p", + wantOk: true, + }, + { + name: "scheme placeholder expands to db2", + opts: ConnectOptions{DSN: native, Scheme: "${SCH}", Lookup: lookup(map[string]string{"SCH": "db2"})}, + wantDSN: native, + wantOk: true, + }, + {name: "db2 url form", opts: ConnectOptions{DSN: "db2://u:p@h:50000/db"}, wantOk: false}, + {name: "postgres url form", opts: ConnectOptions{DSN: "postgres://h/db"}, wantOk: false}, + {name: "native markers but foreign scheme", opts: ConnectOptions{DSN: native, Scheme: "postgres"}, wantOk: false}, + {name: "url with database marker in query", opts: ConnectOptions{DSN: "db2://h:50000/db?DATABASE=x"}, wantOk: false}, + {name: "empty dsn", opts: ConnectOptions{}, wantOk: false}, + { + name: "unset placeholder errors", + opts: ConnectOptions{DSN: "HOSTNAME=${MISSING};DATABASE=d", Lookup: lookup(map[string]string{})}, + wantErr: "MISSING", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotDSN, gotOk, err := nativeDB2DSN(tt.opts) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.wantOk, gotOk) + require.Equal(t, tt.wantDSN, gotDSN) + }) + } +} + +func TestResolveDatabaseNameNativeDB2(t *testing.T) { + // The native form must resolve the same database name as the equivalent db2:// URL, + // so resource IDs stay stable across the two DSN forms. + tests := []struct { + name string + opts ConnectOptions + want string + }{ + { + name: "native form", + opts: ConnectOptions{DSN: "HOSTNAME=h;PORT=50000;DATABASE=TESTDB;UID=u;PWD=p;PROTOCOL=TCPIP"}, + want: "TESTDB", + }, + { + name: "native form braced database", + opts: ConnectOptions{DSN: "HOSTNAME=h;DATABASE={my;db};UID=u"}, + want: "my;db", + }, + { + name: "equivalent url form", + opts: ConnectOptions{DSN: "db2://u:p@h:50000/TESTDB"}, + want: "TESTDB", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, ResolveDatabaseName(tt.opts)) + }) + } +} diff --git a/pkg/database/native_db2_route_test.go b/pkg/database/native_db2_route_test.go new file mode 100644 index 00000000..f7e9e9d0 --- /dev/null +++ b/pkg/database/native_db2_route_test.go @@ -0,0 +1,34 @@ +//go:build !db2 + +package database + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +// A native DB2 DSN set through config must reach the DB2 driver, not be rejected by the +// URL builder. On a default (non-db2) build that means Connect returns the "not compiled" +// stub error, never the "scheme must be specified" / "database name is required" errors +// the URL builder raises for an opaque DSN. +func TestConnectNativeDB2DSNReachesDriver(t *testing.T) { + const native = "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP" + + for _, tt := range []struct { + name string + opts ConnectOptions + }{ + {name: "no scheme", opts: ConnectOptions{DSN: native}}, + {name: "scheme db2", opts: ConnectOptions{DSN: native, Scheme: "db2"}}, + } { + t.Run(tt.name, func(t *testing.T) { + _, _, err := Connect(context.Background(), tt.opts) + require.Error(t, err) + require.ErrorContains(t, err, "DB2 support not compiled") + require.NotContains(t, err.Error(), "scheme must be specified") + require.NotContains(t, err.Error(), "database name is required") + }) + } +} From 209040e11814bc9e8e07bcf2082fbe21257b4cb4 Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Wed, 2 Sep 2026 14:36:16 -0500 Subject: [PATCH 2/9] ci: bump sync-test action to v4 so baton reads pebble c1z sync-test@v2 installs the CLI from conductorone/baton (latest v0.4.5, pre-pebble), so baton grants rejects the pebble-format c1z with "c1z: invalid file". @v4 pulls the CLI from conductorone/baton-sdk, which reads pebble. --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1df22297..0437c6af 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -41,7 +41,7 @@ jobs: - name: Build baton-sql run: go build ./cmd/baton-sql - name: Run sync tests - uses: ConductorOne/github-workflows/actions/sync-test@v2 + uses: ConductorOne/github-workflows/actions/sync-test@v4 with: connector: ./baton-sql baton-entitlement: 'role:admin:member' From 640c973c8e6edbd91ecfb507c6b648cdb4b1c20f Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Thu, 3 Sep 2026 06:24:39 -0500 Subject: [PATCH 3/9] CXH-2380: harden native DB2 DSN routing per review feedback Address the CI review findings on the native DB2 DSN path: - Reject a native DSN combined with structured connect fields (host, port, user, password, params) or a per-database override (connect.database, databases). The verbatim path never reaches buildConnectionURL, so those were silently dropped and multi-database sync opened every handle against the DSN's single DATABASE=. Now it errors clearly instead. - Detect native DSNs and extract DATABASE= case-insensitively and with whitespace tolerance (ODBC keywords are case-insensitive; "; " spacing is common). A lowercase/spaced native DSN previously fell through to the URL path and hit "scheme must be specified", or resolved an empty database name. - Apply the same case-insensitive passthrough in db2.convertToDB2DSN, which the native path now reaches for lowercase DSNs. - Anchor the URL-shape check to a leading scheme so a native DSN whose value contains "://" (e.g. PWD=my://secret) is not misread as a URL. - Bump account-provisioning to @v4 so it no longer depends on step ordering to pick up the pebble-compatible CLI. - Document native-DSN exclusivity and case-insensitivity in docs/db2.md. Verified live against a local DB2 container: lowercase native DSN syncs, native DSN + connect.database is rejected, uppercase and db2:// URL forms unaffected. --- .github/workflows/ci.yaml | 2 +- docs/db2.md | 10 ++++- pkg/database/database.go | 57 +++++++++++++++++++++++---- pkg/database/db2/dsn.go | 28 +++++++++++-- pkg/database/db2/dsn_test.go | 10 +++++ pkg/database/native_db2_dsn_test.go | 28 +++++++++++++ pkg/database/native_db2_route_test.go | 33 ++++++++++++++++ 7 files changed, 156 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0437c6af..6a243303 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -48,7 +48,7 @@ jobs: baton-principal: john.smith baton-principal-type: user - 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 diff --git a/docs/db2.md b/docs/db2.md index 6922a829..821bcf6f 100644 --- a/docs/db2.md +++ b/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. + ## Troubleshooting **`'sqlcli1.h' file not found`** — clidriver missing or `DB2HOME` wrong. Check that diff --git a/pkg/database/database.go b/pkg/database/database.go index a460d266..f1806552 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -22,6 +22,11 @@ import ( var DSNREnvRegex = regexp.MustCompile(`\$\{([A-Za-z0-9_]+)\}`) +// urlSchemeDSNRegex matches a DSN that begins with a URL scheme (e.g. "db2://"). +// Anchored to the start so a native ODBC DSN carrying "://" inside a value +// (e.g. PWD=my://secret) is not misread as a URL. +var urlSchemeDSNRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) + // LookupFunc resolves ${KEY} placeholders during DSN/connect field expansion. // When nil is passed to expand helpers, os.LookupEnv is used (CLI compatibility). // Library embedders should pass a map-backed LookupFunc and never mutate process env. @@ -373,16 +378,41 @@ func ResolveDatabaseName(opts ConnectOptions) string { func db2DSNDatabase(dsn string) string { for _, part := range splitDB2DSN(dsn) { - if value, found := strings.CutPrefix(part, "DATABASE="); found { - if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { - value = value[1 : len(value)-1] - } - return value + keyword, value, found := strings.Cut(strings.TrimSpace(part), "=") + if !found || !strings.EqualFold(keyword, "DATABASE") { + continue + } + if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { + value = value[1 : len(value)-1] } + return value } return "" } +// hasDB2Marker reports whether any ';'-separated part of dsn is a HOSTNAME= or +// DATABASE= keyword. ODBC keywords are case-insensitive and parts often carry +// whitespace after the ';', so both are normalized before comparison. +func hasDB2Marker(dsn string) bool { + for _, part := range splitDB2DSN(dsn) { + keyword, _, found := strings.Cut(strings.TrimSpace(part), "=") + if found && (strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE")) { + return true + } + } + return false +} + +// 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 +} + // splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. func splitDB2DSN(dsn string) []string { var parts []string @@ -448,6 +478,19 @@ func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error 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, errors.New( + "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 @@ -542,10 +585,10 @@ func nativeDB2DSN(opts ConnectOptions) (string, bool, error) { if err != nil { return "", false, err } - if strings.Contains(dsn, "://") { + if urlSchemeDSNRegex.MatchString(dsn) { return "", false, nil } - if !strings.Contains(dsn, "HOSTNAME=") && !strings.Contains(dsn, "DATABASE=") { + if !hasDB2Marker(dsn) { return "", false, nil } return dsn, true, nil diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 87b7d3ed..5b5edb83 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -3,10 +3,32 @@ package db2 import ( "fmt" "net/url" + "regexp" "sort" "strings" ) +// urlSchemeRegex matches a DSN that begins with a URL scheme (e.g. "db2://"). +// Anchored to the start so a native ODBC DSN carrying "://" inside a value +// (e.g. PWD=my://secret) is not misread as a URL. +var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) + +// isNativeDB2Format reports whether dsn is already in DB2's native ODBC +// keyword=value form (rather than a URL). ODBC keywords are case-insensitive and +// parts may carry whitespace after the ';', so both are normalized. +func isNativeDB2Format(dsn string) bool { + if urlSchemeRegex.MatchString(dsn) { + return false + } + for _, part := range strings.Split(dsn, ";") { + keyword, _, found := strings.Cut(strings.TrimSpace(part), "=") + if found && (strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE")) { + return true + } + } + return false +} + // Keywords derived from the URL itself; query parameters may not override them. // Anyone needing full control over these can pass a native DB2 DSN instead. var reservedDSNKeywords = map[string]bool{ @@ -33,9 +55,9 @@ func quoteDB2Value(v string) (string, error) { // convertToDB2DSN converts URL format to DB2 DSN format. func convertToDB2DSN(dsn string) (string, error) { - // If it's already in DB2 format (contains HOSTNAME= or DATABASE=), return as-is. - // URL-format DSNs are exempt from this check so those markers may appear in credentials. - if !strings.HasPrefix(dsn, "db2://") && (strings.Contains(dsn, "HOSTNAME=") || strings.Contains(dsn, "DATABASE=")) { + // If it's already in DB2's native keyword=value format, return as-is. + // URL-format DSNs are exempt so those markers may appear in credentials. + if isNativeDB2Format(dsn) { return dsn, nil } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index 9ab7956a..1d7b1571 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -28,6 +28,16 @@ func TestConvertToDB2DSN(t *testing.T) { dsn: "HOSTNAME=dbhost;PORT=50000;DATABASE=testdb;UID=user;PWD=pass", want: "HOSTNAME=dbhost;PORT=50000;DATABASE=testdb;UID=user;PWD=pass", }, + { + name: "lowercase native dsn passed through", + dsn: "hostname=dbhost;port=50000;database=testdb;uid=user;pwd=pass", + want: "hostname=dbhost;port=50000;database=testdb;uid=user;pwd=pass", + }, + { + name: "native dsn with whitespace passed through", + dsn: "HOSTNAME=dbhost; DATABASE=testdb; UID=user", + want: "HOSTNAME=dbhost; DATABASE=testdb; UID=user", + }, { name: "wrong scheme", dsn: "postgres://dbhost/testdb", diff --git a/pkg/database/native_db2_dsn_test.go b/pkg/database/native_db2_dsn_test.go index d6ae3dc8..be4cdf7a 100644 --- a/pkg/database/native_db2_dsn_test.go +++ b/pkg/database/native_db2_dsn_test.go @@ -23,6 +23,24 @@ func TestNativeDB2DSN(t *testing.T) { {name: "native form no scheme", opts: ConnectOptions{DSN: native}, wantDSN: native, wantOk: true}, {name: "native form with scheme db2", opts: ConnectOptions{DSN: native, Scheme: "db2"}, wantDSN: native, wantOk: true}, {name: "database marker only", opts: ConnectOptions{DSN: "DATABASE=TESTDB;HOST=x"}, wantDSN: "DATABASE=TESTDB;HOST=x", wantOk: true}, + { + name: "lowercase keywords", + opts: ConnectOptions{DSN: "hostname=h;port=50000;database=X;uid=u;pwd=p"}, + wantDSN: "hostname=h;port=50000;database=X;uid=u;pwd=p", + wantOk: true, + }, + { + name: "whitespace after separators", + opts: ConnectOptions{DSN: "HOSTNAME=h; DATABASE=X; UID=u"}, + wantDSN: "HOSTNAME=h; DATABASE=X; UID=u", + wantOk: true, + }, + { + name: "value containing :// is not a url", + opts: ConnectOptions{DSN: "HOSTNAME=h;DATABASE=X;PWD=my://secret"}, + wantDSN: "HOSTNAME=h;DATABASE=X;PWD=my://secret", + wantOk: true, + }, { name: "native form with placeholders", opts: ConnectOptions{ @@ -82,6 +100,16 @@ func TestResolveDatabaseNameNativeDB2(t *testing.T) { opts: ConnectOptions{DSN: "HOSTNAME=h;DATABASE={my;db};UID=u"}, want: "my;db", }, + { + name: "lowercase database keyword", + opts: ConnectOptions{DSN: "hostname=h;database=testdb;uid=u"}, + want: "testdb", + }, + { + name: "whitespace before database keyword", + opts: ConnectOptions{DSN: "HOSTNAME=h; DATABASE=TESTDB; UID=u"}, + want: "TESTDB", + }, { name: "equivalent url form", opts: ConnectOptions{DSN: "db2://u:p@h:50000/TESTDB"}, diff --git a/pkg/database/native_db2_route_test.go b/pkg/database/native_db2_route_test.go index f7e9e9d0..f782cbee 100644 --- a/pkg/database/native_db2_route_test.go +++ b/pkg/database/native_db2_route_test.go @@ -32,3 +32,36 @@ func TestConnectNativeDB2DSNReachesDriver(t *testing.T) { }) } } + +// A native DSN already carries every connection setting, so pairing it with structured +// connect fields or a per-database override must be rejected up front (before the driver +// stub), never silently dropped. This also covers the multi-database path, where +// ConnectMany sets perOpts.Database per name. +func TestConnectNativeDB2DSNRejectsStructuredFields(t *testing.T) { + const native = "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP" + + for _, tt := range []struct { + name string + opts ConnectOptions + }{ + {name: "database override", opts: ConnectOptions{DSN: native, Database: "OTHERDB"}}, + {name: "host", opts: ConnectOptions{DSN: native, Host: "elsewhere"}}, + {name: "port", opts: ConnectOptions{DSN: native, Port: "50001"}}, + {name: "user", opts: ConnectOptions{DSN: native, User: "someone"}}, + {name: "password", opts: ConnectOptions{DSN: native, Password: "secret"}}, + {name: "params", opts: ConnectOptions{DSN: native, Params: map[string]string{"SECURITY": "SSL"}}}, + } { + t.Run(tt.name, func(t *testing.T) { + _, _, err := Connect(context.Background(), tt.opts) + require.Error(t, err) + require.ErrorContains(t, err, "self-contained") + require.NotContains(t, err.Error(), "DB2 support not compiled") + }) + } + + t.Run("multi-database via ConnectMany", func(t *testing.T) { + _, _, err := ConnectMany(context.Background(), ConnectOptions{DSN: native}, []string{"A", "B"}) + require.Error(t, err) + require.ErrorContains(t, err, "self-contained") + }) +} From dc989396fb479e85df3b29577d4e9a1058630ac0 Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Thu, 3 Sep 2026 14:46:02 -0500 Subject: [PATCH 4/9] CXH-2380: fix brace-start quoting and share one native-DSN detector Address review: splitDB2DSN now enters brace mode only when a value starts with '{' (so PWD=p{q no longer swallows the next ';'), and detection/DATABASE-extraction move to db2.IsNativeDSN/db2.DSNDatabase, used by both the router and convertToDB2DSN so they cannot drift. Spaced native DSN verified live against Db2 v12.1. --- pkg/database/database.go | 62 ++--------------------------------- pkg/database/db2/dsn.go | 63 ++++++++++++++++++++++++++++++++---- pkg/database/db2/dsn_test.go | 44 +++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 65 deletions(-) diff --git a/pkg/database/database.go b/pkg/database/database.go index f1806552..bba27c9a 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -22,11 +22,6 @@ import ( var DSNREnvRegex = regexp.MustCompile(`\$\{([A-Za-z0-9_]+)\}`) -// urlSchemeDSNRegex matches a DSN that begins with a URL scheme (e.g. "db2://"). -// Anchored to the start so a native ODBC DSN carrying "://" inside a value -// (e.g. PWD=my://secret) is not misread as a URL. -var urlSchemeDSNRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) - // LookupFunc resolves ${KEY} placeholders during DSN/connect field expansion. // When nil is passed to expand helpers, os.LookupEnv is used (CLI compatibility). // Library embedders should pass a map-backed LookupFunc and never mutate process env. @@ -367,7 +362,7 @@ func ResolveDatabaseName(opts ConnectOptions) string { } } if nativeDSN, isNativeDB2, err := nativeDB2DSN(opts); err == nil && isNativeDB2 { - return db2DSNDatabase(nativeDSN) + return db2.DSNDatabase(nativeDSN) } parsedUrl, err := buildConnectionURL(opts) if err != nil || parsedUrl == nil { @@ -376,33 +371,6 @@ func ResolveDatabaseName(opts ConnectOptions) string { return strings.TrimPrefix(parsedUrl.Path, "/") } -func db2DSNDatabase(dsn string) string { - for _, part := range splitDB2DSN(dsn) { - keyword, value, found := strings.Cut(strings.TrimSpace(part), "=") - if !found || !strings.EqualFold(keyword, "DATABASE") { - continue - } - if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { - value = value[1 : len(value)-1] - } - return value - } - return "" -} - -// hasDB2Marker reports whether any ';'-separated part of dsn is a HOSTNAME= or -// DATABASE= keyword. ODBC keywords are case-insensitive and parts often carry -// whitespace after the ';', so both are normalized before comparison. -func hasDB2Marker(dsn string) bool { - for _, part := range splitDB2DSN(dsn) { - keyword, _, found := strings.Cut(strings.TrimSpace(part), "=") - if found && (strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE")) { - return true - } - } - return false -} - // 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 @@ -413,27 +381,6 @@ func hasStructuredConnectFields(opts ConnectOptions) bool { opts.Password != "" || opts.Database != "" || len(opts.Params) > 0 } -// splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. -func splitDB2DSN(dsn string) []string { - var parts []string - start := 0 - inBraces := false - for i, r := range dsn { - switch r { - case '{': - inBraces = true - case '}': - inBraces = false - case ';': - if !inBraces { - parts = append(parts, dsn[start:i]) - start = i + 1 - } - } - } - return append(parts, dsn[start:]) -} - // 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) { @@ -566,7 +513,7 @@ func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error // keyword=value string (e.g. "HOSTNAME=...;DATABASE=...") rather than a db2:// URL. // When it does, the env-expanded string is returned for verbatim handoff to the driver. // The scheme, when set, must be db2; a URL-shaped DSN or foreign scheme is left to the -// normal URL path. Markers match db2.convertToDB2DSN's passthrough. +// normal URL path. Detection is db2.IsNativeDSN, shared with convertToDB2DSN's passthrough. func nativeDB2DSN(opts ConnectOptions) (string, bool, error) { if opts.DSN == "" { return "", false, nil @@ -585,10 +532,7 @@ func nativeDB2DSN(opts ConnectOptions) (string, bool, error) { if err != nil { return "", false, err } - if urlSchemeDSNRegex.MatchString(dsn) { - return "", false, nil - } - if !hasDB2Marker(dsn) { + if !db2.IsNativeDSN(dsn) { return "", false, nil } return dsn, true, nil diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 5b5edb83..8d7c8f63 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -13,14 +13,15 @@ import ( // (e.g. PWD=my://secret) is not misread as a URL. var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) -// isNativeDB2Format reports whether dsn is already in DB2's native ODBC -// keyword=value form (rather than a URL). ODBC keywords are case-insensitive and -// parts may carry whitespace after the ';', so both are normalized. -func isNativeDB2Format(dsn string) bool { +// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form rather +// than a URL. Shared by pkg/database's routing and convertToDB2DSN's passthrough so +// the two decisions cannot drift. ODBC keywords are case-insensitive and parts may +// carry whitespace after the ';', so both are normalized. +func IsNativeDSN(dsn string) bool { if urlSchemeRegex.MatchString(dsn) { return false } - for _, part := range strings.Split(dsn, ";") { + for _, part := range splitDB2DSN(dsn) { keyword, _, found := strings.Cut(strings.TrimSpace(part), "=") if found && (strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE")) { return true @@ -29,6 +30,56 @@ func isNativeDB2Format(dsn string) bool { return false } +// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent. +func DSNDatabase(dsn string) string { + for _, part := range splitDB2DSN(dsn) { + keyword, value, found := strings.Cut(strings.TrimSpace(part), "=") + if !found || !strings.EqualFold(keyword, "DATABASE") { + continue + } + if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { + value = value[1 : len(value)-1] + } + return value + } + return "" +} + +// splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. +// ODBC only treats '{' as quoting when a value starts with it (right after '='); a '{' +// anywhere else is literal, so PWD=p{q does not swallow the following ';'. +func splitDB2DSN(dsn string) []string { + var parts []string + start := 0 + braced := false // inside a {...} quoted value + atValueStart := false // previous char was '=' outside braces + for i := 0; i < len(dsn); i++ { + switch dsn[i] { + case '}': + braced = false + atValueStart = false + case '{': + if atValueStart { + braced = true + } + atValueStart = false + case '=': + if !braced { + atValueStart = true + } + case ';': + if !braced { + parts = append(parts, dsn[start:i]) + start = i + 1 + } + atValueStart = false + default: + atValueStart = false + } + } + return append(parts, dsn[start:]) +} + // Keywords derived from the URL itself; query parameters may not override them. // Anyone needing full control over these can pass a native DB2 DSN instead. var reservedDSNKeywords = map[string]bool{ @@ -57,7 +108,7 @@ func quoteDB2Value(v string) (string, error) { func convertToDB2DSN(dsn string) (string, error) { // If it's already in DB2's native keyword=value format, return as-is. // URL-format DSNs are exempt so those markers may appear in credentials. - if isNativeDB2Format(dsn) { + if IsNativeDSN(dsn) { return dsn, nil } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index 1d7b1571..eadfde5f 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -112,3 +112,47 @@ func TestConvertToDB2DSN(t *testing.T) { }) } } + +func TestIsNativeDSN(t *testing.T) { + tests := []struct { + name string + dsn string + want bool + }{ + {name: "native markers", dsn: "HOSTNAME=h;DATABASE=X", want: true}, + {name: "lowercase keywords", dsn: "hostname=h;database=x", want: true}, + {name: "whitespace after separator", dsn: "HOSTNAME=h; DATABASE=X", want: true}, + {name: "db2 url", dsn: "db2://u:p@h:50000/db", want: false}, + {name: "postgres url", dsn: "postgres://h/db", want: false}, + {name: "value carrying :// is not a url", dsn: "HOSTNAME=h;PWD=my://secret", want: true}, + // DATABASE= appears only inside a braced PWD value, so the brace-aware split keeps it + // as one PWD part: not a native marker. Routing and passthrough now agree here. + {name: "database marker only inside braced value", dsn: "UID=u;PWD={x;DATABASE=y}", want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, IsNativeDSN(tt.dsn)) + }) + } +} + +func TestDSNDatabase(t *testing.T) { + tests := []struct { + name string + dsn string + want string + }{ + {name: "plain", dsn: "HOSTNAME=h;DATABASE=TESTDB;UID=u", want: "TESTDB"}, + {name: "braced value with semicolon", dsn: "HOSTNAME=h;DATABASE={my;db}", want: "my;db"}, + {name: "lowercase", dsn: "hostname=h;database=testdb", want: "testdb"}, + {name: "whitespace before keyword", dsn: "HOSTNAME=h; DATABASE=TESTDB", want: "TESTDB"}, + // A literal '{' mid-value (not ODBC quoting) must not swallow the following ';'. + {name: "unquoted brace in earlier value", dsn: "HOSTNAME=h;PWD=p{q;DATABASE=TESTDB", want: "TESTDB"}, + {name: "absent", dsn: "HOSTNAME=h;UID=u", want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, DSNDatabase(tt.dsn)) + }) + } +} From d03f4c511e7c895a59934e72533f050699c6dc04 Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Thu, 3 Sep 2026 16:10:47 -0500 Subject: [PATCH 5/9] CXH-2380: exit with gRPC status code on error via exit.LogExit Replace os.Exit(1) in main.go with exit.LogExit(err) so an auth failure exits with the mapped gRPC status code instead of a bare 1, letting the CI sync-test auth-error check actually assert. --- cmd/baton-sql/main.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/cmd/baton-sql/main.go b/cmd/baton-sql/main.go index 5fa963d5..90245d3d 100644 --- a/cmd/baton-sql/main.go +++ b/cmd/baton-sql/main.go @@ -2,11 +2,10 @@ package main import ( "context" - "fmt" - "os" configSdk "github.com/conductorone/baton-sdk/pkg/config" "github.com/conductorone/baton-sdk/pkg/connectorbuilder" + "github.com/conductorone/baton-sdk/pkg/exit" "github.com/conductorone/baton-sdk/pkg/field" "github.com/conductorone/baton-sdk/pkg/types" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" @@ -31,16 +30,14 @@ func main() { }, ) if err != nil { - fmt.Fprintln(os.Stderr, err.Error()) - os.Exit(1) + exit.LogExit(err) } cmd.Version = version err = cmd.Execute() if err != nil { - fmt.Fprintln(os.Stderr, err.Error()) - os.Exit(1) + exit.LogExit(err) } } From 66e11c1c4dbab8b7534d7bf130386675f67a66a4 Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Thu, 3 Sep 2026 16:39:28 -0500 Subject: [PATCH 6/9] CXH-2380: map DB auth failures to Unauthenticated in Validate Pairs with the exit.LogExit change: Validate wrapped the ping error plainly, so exit mapped auth failures to Unknown(2). database.AuthError maps SQLSTATE class 28 (Postgres/Redshift/Vertica/etc.) and MySQL 1045 to codes.Unauthenticated. --- pkg/connector/connector.go | 3 +++ pkg/database/autherror.go | 35 +++++++++++++++++++++++++++ pkg/database/autherror_test.go | 44 ++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 pkg/database/autherror.go create mode 100644 pkg/database/autherror_test.go diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index cb69ba02..eabbf346 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -97,6 +97,9 @@ 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 ping failed: %w", name, err) } } diff --git a/pkg/database/autherror.go b/pkg/database/autherror.go new file mode 100644 index 00000000..96593707 --- /dev/null +++ b/pkg/database/autherror.go @@ -0,0 +1,35 @@ +package database + +import ( + "errors" + "strings" + + "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. +func AuthError(err error) error { + if err == nil { + return nil + } + + var sqlState interface{ SQLState() string } + if errors.As(err, &sqlState) && strings.HasPrefix(sqlState.SQLState(), "28") { + return status.Error(codes.Unauthenticated, "database authentication failed") + } + + var myErr *mysql.MySQLError + if errors.As(err, &myErr) && myErr.Number == mysqlAccessDenied { + return status.Error(codes.Unauthenticated, "database authentication failed") + } + + return nil +} diff --git a/pkg/database/autherror_test.go b/pkg/database/autherror_test.go new file mode 100644 index 00000000..3e1153cc --- /dev/null +++ b/pkg/database/autherror_test.go @@ -0,0 +1,44 @@ +package database + +import ( + "errors" + "fmt" + "testing" + + "github.com/go-sql-driver/mysql" + "github.com/jackc/pgx/v5/pgconn" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestAuthError(t *testing.T) { + tests := []struct { + name string + err error + want codes.Code // codes.OK means expect nil + }{ + {"nil", nil, codes.OK}, + {"postgres invalid_password 28P01", &pgconn.PgError{Code: "28P01"}, codes.Unauthenticated}, + {"postgres invalid_authorization 28000", &pgconn.PgError{Code: "28000"}, codes.Unauthenticated}, + {"postgres non-auth relation missing 42P01", &pgconn.PgError{Code: "42P01"}, codes.OK}, + {"postgres auth error wrapped", fmt.Errorf("ping: %w", &pgconn.PgError{Code: "28P01"}), codes.Unauthenticated}, + {"mysql access denied 1045", &mysql.MySQLError{Number: 1045}, codes.Unauthenticated}, + {"mysql other 1146", &mysql.MySQLError{Number: 1146}, codes.OK}, + {"plain error", errors.New("boom"), codes.OK}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AuthError(tt.err) + if tt.want == codes.OK { + if got != nil { + t.Fatalf("want nil, got %v", got) + } + return + } + if status.Code(got) != tt.want { + t.Fatalf("want %v, got %v", tt.want, status.Code(got)) + } + }) + } +} From a166e4c9de54eef7f07ea545dc18ca80bdfe14df Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Fri, 4 Sep 2026 06:22:23 -0500 Subject: [PATCH 7/9] CXH-2380: fix native DSN whitespace and unterminated-brace parsing Address FeliLucero1's review on PR #149: - splitDB2DSN now keeps the value-start position across whitespace, so a space between '=' and a brace ("DATABASE= {my;db}") is still brace-quoted instead of letting the ';' inside the braces split the part and truncate the database name. - IsNativeDSN and DSNDatabase trim the keyword and value, so "DATABASE = X" and "DATABASE= X" match the keyword and resolve the value without the leading space that previously leaked into the handle key and synthetic Database column. - An unterminated '{' is now treated as a literal char (lookahead for a closing '}'), so it no longer swallows the rest of the DSN and hide the HOSTNAME/DATABASE markers; the malformed value reaches the driver's own error instead of the generic "scheme must be specified" misroute. - Note the third, separate scheme check in pkg/bsql/offline_validate.go resolveConnectScheme on IsNativeDSN so it stays in sync if v1 ever accepts DB2. Verified live against a local DB2 container: a native DSN with "DATABASE= TESTDB" (space after '=') connects and syncs. New unit cases cover the whitespace and unterminated-brace paths. --- pkg/database/db2/dsn.go | 36 ++++++++++++++++++++++++------------ pkg/database/db2/dsn_test.go | 9 +++++++++ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 8d7c8f63..9fc80bcd 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -13,17 +13,23 @@ import ( // (e.g. PWD=my://secret) is not misread as a URL. var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) -// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form rather -// than a URL. Shared by pkg/database's routing and convertToDB2DSN's passthrough so -// the two decisions cannot drift. ODBC keywords are case-insensitive and parts may -// carry whitespace after the ';', so both are normalized. +// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form rather than +// a URL. Shared by pkg/database routing and convertToDB2DSN passthrough so they can't +// drift. A third, separate check (pkg/bsql/offline_validate.go resolveConnectScheme, +// ad-hoc "://") stays dormant while v1 validation allows only postgres; keep it in sync +// if that ever accepts DB2. ODBC keywords are case-insensitive and keyword/value may +// carry surrounding whitespace, so both are normalized. func IsNativeDSN(dsn string) bool { if urlSchemeRegex.MatchString(dsn) { return false } for _, part := range splitDB2DSN(dsn) { - keyword, _, found := strings.Cut(strings.TrimSpace(part), "=") - if found && (strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE")) { + keyword, _, found := strings.Cut(part, "=") + if !found { + continue + } + keyword = strings.TrimSpace(keyword) + if strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE") { return true } } @@ -33,10 +39,11 @@ func IsNativeDSN(dsn string) bool { // DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent. func DSNDatabase(dsn string) string { for _, part := range splitDB2DSN(dsn) { - keyword, value, found := strings.Cut(strings.TrimSpace(part), "=") - if !found || !strings.EqualFold(keyword, "DATABASE") { + keyword, value, found := strings.Cut(part, "=") + if !found || !strings.EqualFold(strings.TrimSpace(keyword), "DATABASE") { continue } + value = strings.TrimSpace(value) if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { value = value[1 : len(value)-1] } @@ -46,20 +53,23 @@ func DSNDatabase(dsn string) string { } // splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. -// ODBC only treats '{' as quoting when a value starts with it (right after '='); a '{' -// anywhere else is literal, so PWD=p{q does not swallow the following ';'. +// A '{' quotes only when it starts a value (right after '=', across any whitespace) AND +// is closed by a later '}'. A '{' elsewhere, or one left unterminated, is literal, so +// PWD=p{q keeps the following ';' and an unclosed '{' does not swallow the rest of the +// DSN (its HOSTNAME/DATABASE markers stay visible and the malformed value reaches the +// driver's own error rather than a silent misroute). func splitDB2DSN(dsn string) []string { var parts []string start := 0 braced := false // inside a {...} quoted value - atValueStart := false // previous char was '=' outside braces + atValueStart := false // at a value position (right after '=', across whitespace) outside braces for i := 0; i < len(dsn); i++ { switch dsn[i] { case '}': braced = false atValueStart = false case '{': - if atValueStart { + if atValueStart && strings.IndexByte(dsn[i:], '}') != -1 { braced = true } atValueStart = false @@ -73,6 +83,8 @@ func splitDB2DSN(dsn string) []string { start = i + 1 } atValueStart = false + case ' ', '\t': + // keep atValueStart so "DATABASE= {my;db}" still brace-detects. default: atValueStart = false } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index eadfde5f..6312c59f 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -125,9 +125,13 @@ func TestIsNativeDSN(t *testing.T) { {name: "db2 url", dsn: "db2://u:p@h:50000/db", want: false}, {name: "postgres url", dsn: "postgres://h/db", want: false}, {name: "value carrying :// is not a url", dsn: "HOSTNAME=h;PWD=my://secret", want: true}, + {name: "space before the =", dsn: "DATABASE = X", want: true}, // DATABASE= appears only inside a braced PWD value, so the brace-aware split keeps it // as one PWD part: not a native marker. Routing and passthrough now agree here. {name: "database marker only inside braced value", dsn: "UID=u;PWD={x;DATABASE=y}", want: false}, + // Unterminated '{' is literal, so the ';' still splits and DATABASE= stays visible; + // the malformed value then reaches the driver instead of silently misrouting. + {name: "unterminated brace keeps marker visible", dsn: "PWD={oops;DATABASE=X", want: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -146,6 +150,11 @@ func TestDSNDatabase(t *testing.T) { {name: "braced value with semicolon", dsn: "HOSTNAME=h;DATABASE={my;db}", want: "my;db"}, {name: "lowercase", dsn: "hostname=h;database=testdb", want: "testdb"}, {name: "whitespace before keyword", dsn: "HOSTNAME=h; DATABASE=TESTDB", want: "TESTDB"}, + {name: "space after the =", dsn: "HOSTNAME=h;DATABASE= TESTDB", want: "TESTDB"}, + {name: "space before the =", dsn: "HOSTNAME=h;DATABASE = TESTDB", want: "TESTDB"}, + // Space between '=' and a braced value must still brace-detect, else the ';' + // inside the braces splits and the database name comes back truncated. + {name: "space before braced value", dsn: "HOSTNAME=h;DATABASE= {my;db}", want: "my;db"}, // A literal '{' mid-value (not ODBC quoting) must not swallow the following ';'. {name: "unquoted brace in earlier value", dsn: "HOSTNAME=h;PWD=p{q;DATABASE=TESTDB", want: "TESTDB"}, {name: "absent", dsn: "HOSTNAME=h;UID=u", want: ""}, From 4ca63a2fae2a24b1677af501c211169350f93efa Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Fri, 4 Sep 2026 08:14:34 -0500 Subject: [PATCH 8/9] CXH-2380: classify native DB2 DSN in offline scheme resolution resolveConnectScheme did its own ad-hoc "://" scheme detection, separate from db2.IsNativeDSN, so a native DB2 DSN (no scheme prefix) resolved to the confusing "scheme missing from dsn" error, and a native DSN with "://" inside a value (e.g. PWD=my://secret) was misread as a URL scheme. Route native detection through the shared db2.IsNativeDSN so this path matches the engine's connection routing: a native DB2 DSN now resolves to scheme "db2", giving the clear "scheme \"db2\" is not supported in v1" message today and correct routing if v1 ever accepts DB2. No behavior change for postgres or URL-form DSNs. --- pkg/bsql/offline_validate.go | 8 ++++++++ pkg/bsql/offline_validate_test.go | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/pkg/bsql/offline_validate.go b/pkg/bsql/offline_validate.go index 04f4dad5..4ba441d6 100644 --- a/pkg/bsql/offline_validate.go +++ b/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 diff --git a/pkg/bsql/offline_validate_test.go b/pkg/bsql/offline_validate_test.go index 430e0be3..15656400 100644 --- a/pkg/bsql/offline_validate_test.go +++ b/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) From 296d680e14909537cc053d99db8ed4b20c82eb13 Mon Sep 17 00:00:00 2001 From: Alejandro Bernal Date: Fri, 4 Sep 2026 08:33:25 -0500 Subject: [PATCH 9/9] CXH-2380: fold native-DSN detection and DATABASE extraction into one pass Consolidate db2.IsNativeDSN and db2.DSNDatabase onto a single db2.ParseNativeDSN that returns both facts from one splitDB2DSN pass; the two remain as thin wrappers for existing callers. nativeDB2DSN now returns the DATABASE value alongside the DSN, so ResolveDatabaseName no longer re-splits the string (it dropped from two passes to one) while Connect's single-pass cost is unchanged. --- pkg/database/database.go | 30 ++++++++-------- pkg/database/db2/dsn.go | 56 ++++++++++++++++------------- pkg/database/native_db2_dsn_test.go | 2 +- 3 files changed, 49 insertions(+), 39 deletions(-) diff --git a/pkg/database/database.go b/pkg/database/database.go index bba27c9a..088706ff 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -361,8 +361,8 @@ func ResolveDatabaseName(opts ConnectOptions) string { return expanded } } - if nativeDSN, isNativeDB2, err := nativeDB2DSN(opts); err == nil && isNativeDB2 { - return db2.DSNDatabase(nativeDSN) + if _, database, isNativeDB2, err := nativeDB2DSN(opts); err == nil && isNativeDB2 { + return database } parsedUrl, err := buildConnectionURL(opts) if err != nil || parsedUrl == nil { @@ -420,7 +420,7 @@ 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) + nativeDSN, _, isNativeDB2, err := nativeDB2DSN(opts) if err != nil { return nil, Unknown, err } @@ -511,31 +511,33 @@ 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 is returned for verbatim handoff to the driver. -// The scheme, when set, must be db2; a URL-shaped DSN or foreign scheme is left to the -// normal URL path. Detection is db2.IsNativeDSN, shared with convertToDB2DSN's passthrough. -func nativeDB2DSN(opts ConnectOptions) (string, bool, error) { +// 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 + return "", "", false, nil } lookup := opts.resolveLookup() scheme, err := expandValue(opts.Scheme, lookup) if err != nil { - return "", false, err + return "", "", false, err } if scheme != "" && scheme != "db2" { - return "", false, nil + return "", "", false, nil } dsn, err := expandValue(opts.DSN, lookup) if err != nil { - return "", false, err + return "", "", false, err } - if !db2.IsNativeDSN(dsn) { - return "", false, nil + database, native := db2.ParseNativeDSN(dsn) + if !native { + return "", "", false, nil } - return dsn, true, nil + return dsn, database, true, nil } func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 9fc80bcd..dcda341c 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -13,43 +13,51 @@ import ( // (e.g. PWD=my://secret) is not misread as a URL. var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) -// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form rather than -// a URL. Shared by pkg/database routing and convertToDB2DSN passthrough so they can't -// drift. A third, separate check (pkg/bsql/offline_validate.go resolveConnectScheme, -// ad-hoc "://") stays dormant while v1 validation allows only postgres; keep it in sync -// if that ever accepts DB2. ODBC keywords are case-insensitive and keyword/value may -// carry surrounding whitespace, so both are normalized. -func IsNativeDSN(dsn string) bool { +// ParseNativeDSN reports whether dsn is DB2's native ODBC keyword=value form (rather +// than a URL) and, when it is, returns its DATABASE value ("" if the DSN omits one). +// One pass over the DSN; IsNativeDSN and DSNDatabase are thin wrappers so all callers +// (pkg/database routing, convertToDB2DSN passthrough, pkg/bsql offline scheme check) +// share one decision and cannot drift. ODBC keywords are case-insensitive and +// keyword/value may carry surrounding whitespace, so both are normalized. +func ParseNativeDSN(dsn string) (string, bool) { if urlSchemeRegex.MatchString(dsn) { - return false + return "", false } + var database string + native, haveDB := false, false for _, part := range splitDB2DSN(dsn) { - keyword, _, found := strings.Cut(part, "=") + keyword, value, found := strings.Cut(part, "=") if !found { continue } keyword = strings.TrimSpace(keyword) - if strings.EqualFold(keyword, "HOSTNAME") || strings.EqualFold(keyword, "DATABASE") { - return true + switch { + case strings.EqualFold(keyword, "HOSTNAME"): + native = true + case strings.EqualFold(keyword, "DATABASE"): + native = true + if !haveDB { // first DATABASE= wins + value = strings.TrimSpace(value) + if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { + value = value[1 : len(value)-1] + } + database, haveDB = value, true + } } } - return false + return database, native +} + +// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form. +func IsNativeDSN(dsn string) bool { + _, native := ParseNativeDSN(dsn) + return native } // DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent. func DSNDatabase(dsn string) string { - for _, part := range splitDB2DSN(dsn) { - keyword, value, found := strings.Cut(part, "=") - if !found || !strings.EqualFold(strings.TrimSpace(keyword), "DATABASE") { - continue - } - value = strings.TrimSpace(value) - if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { - value = value[1 : len(value)-1] - } - return value - } - return "" + database, _ := ParseNativeDSN(dsn) + return database } // splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. diff --git a/pkg/database/native_db2_dsn_test.go b/pkg/database/native_db2_dsn_test.go index be4cdf7a..c46dbeb2 100644 --- a/pkg/database/native_db2_dsn_test.go +++ b/pkg/database/native_db2_dsn_test.go @@ -70,7 +70,7 @@ func TestNativeDB2DSN(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotDSN, gotOk, err := nativeDB2DSN(tt.opts) + gotDSN, _, gotOk, err := nativeDB2DSN(tt.opts) if tt.wantErr != "" { require.ErrorContains(t, err, tt.wantErr) return