From 73421ddb4d54c25edc33e7ab6050f0a30117d6a5 Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Thu, 20 Nov 2025 20:23:09 +0000 Subject: [PATCH 1/4] Improve DSN env vairable expansion --- pkg/database/database.go | 148 +++++++++- pkg/database/database_test.go | 519 ++++++++++++++++++++++++++++++++++ 2 files changed, 666 insertions(+), 1 deletion(-) diff --git a/pkg/database/database.go b/pkg/database/database.go index 7e1751f3..61126f91 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -8,6 +8,7 @@ import ( "net/url" "os" "regexp" + "strings" "github.com/conductorone/baton-sql/pkg/database/hdb" "github.com/conductorone/baton-sql/pkg/database/mysql" @@ -50,8 +51,153 @@ func updateFromEnv(dsn string) (string, error) { return result, nil } +// extractPlaceholders replaces ${...} placeholders with unique numeric sentinels +// and looks up the environment variable values immediately. +// Numeric sentinels (999000, 999001, etc.) are valid in all URL components: +// ports (must be numeric), hostnames, userinfo, paths, and query strings. +// This allows us to parse the URL structure before expanding environment variables. +// Returns: the string with sentinels, a mapping of sentinel->value, and any error. +func extractPlaceholders(s string) (string, map[string]string, error) { + mapping := make(map[string]string) + counter := 0 + var err error + + result := DSNREnvRegex.ReplaceAllStringFunc(s, func(match string) string { + sentinel := fmt.Sprintf("999%03d", counter) + varName := match[2 : len(match)-1] // Extract VAR from ${VAR} + + // Look up the environment variable immediately + value, exists := os.LookupEnv(varName) + if !exists { + err = errors.Join(err, fmt.Errorf("environment variable %s is not set", varName)) + return sentinel // Return sentinel anyway to allow URL parsing for better error messages + } + + mapping[sentinel] = value + counter++ + return sentinel + }) + + if err != nil { + return "", nil, err + } + + return result, mapping, nil +} + +// expandWithMapping expands sentinels in a string by replacing them with their +// corresponding values from the mapping. +func expandWithMapping(s string, mapping map[string]string) string { + result := s + for sentinel, value := range mapping { + result = strings.ReplaceAll(result, sentinel, value) + } + return result +} + +// expandUserInfo expands environment variable placeholders in the URL's user info component. +// It handles the special case where an entire "user:password" string might be in a single variable. +// The url.UserPassword function automatically handles URL encoding of special characters. +func expandUserInfo(parsedUrl *url.URL, mapping map[string]string) { + if parsedUrl.User == nil { + return + } + + username := parsedUrl.User.Username() + password, hasPass := parsedUrl.User.Password() + + // Expand sentinels in username + expandedUser := expandWithMapping(username, mapping) + + // Expand sentinels in password + var expandedPass string + if hasPass { + expandedPass = expandWithMapping(password, mapping) + } + + // Handle the case where the entire userinfo (user:password) is in a single variable. + // For example: ${CREDENTIALS} where CREDENTIALS="admin:p@ss#word" + if strings.Contains(expandedUser, ":") && !hasPass { + parts := strings.SplitN(expandedUser, ":", 2) + expandedUser = parts[0] + expandedPass = parts[1] + hasPass = true + } + + // url.UserPassword automatically handles URL encoding of special characters + if hasPass { + parsedUrl.User = url.UserPassword(expandedUser, expandedPass) + } else { + parsedUrl.User = url.User(expandedUser) + } +} + +// expandHost expands environment variable placeholders in the URL's host component. +func expandHost(parsedUrl *url.URL, mapping map[string]string) { + if parsedUrl.Host != "" { + parsedUrl.Host = expandWithMapping(parsedUrl.Host, mapping) + } +} + +// expandPath expands environment variable placeholders in the URL's path component. +func expandPath(parsedUrl *url.URL, mapping map[string]string) { + if parsedUrl.Path != "" { + parsedUrl.Path = expandWithMapping(parsedUrl.Path, mapping) + } +} + +// expandQuery expands environment variable placeholders in the URL's query string. +func expandQuery(parsedUrl *url.URL, mapping map[string]string) { + if parsedUrl.RawQuery != "" { + parsedUrl.RawQuery = expandWithMapping(parsedUrl.RawQuery, mapping) + } +} + +// expandFragment expands environment variable placeholders in the URL's fragment. +func expandFragment(parsedUrl *url.URL, mapping map[string]string) { + if parsedUrl.Fragment != "" { + parsedUrl.Fragment = expandWithMapping(parsedUrl.Fragment, mapping) + } +} + +// expandDSN expands environment variable placeholders in a DSN using a three-phase approach: +// 1. Replace ${...} with safe sentinel values and lookup env vars +// 2. Parse the URL structure with sentinels +// 3. Expand sentinels component-by-component with appropriate encoding +// +// This approach ensures that special characters in environment variables (like #, @, :) +// don't break URL parsing, since they are expanded after the URL structure is established. +func expandDSN(dsn string) (string, error) { + // Phase 1: Replace ${...} with sentinels and lookup env vars + sentinelDSN, mapping, err := extractPlaceholders(dsn) + if err != nil { + return "", err + } + + // If there are no placeholders, return as-is + if len(mapping) == 0 { + return dsn, nil + } + + // Phase 2: Parse with sentinels + parsedUrl, err := url.Parse(sentinelDSN) + if err != nil { + return "", fmt.Errorf("invalid DSN structure: %w", err) + } + + // Phase 3: Expand by component with appropriate encoding + expandUserInfo(parsedUrl, mapping) + expandHost(parsedUrl, mapping) + expandPath(parsedUrl, mapping) + expandQuery(parsedUrl, mapping) + expandFragment(parsedUrl, mapping) + + return parsedUrl.String(), nil +} + func Connect(ctx context.Context, dsn string, user string, password string) (*sql.DB, DbEngine, error) { - populatedDSN, err := updateFromEnv(dsn) + // Use the new expandDSN function which handles special characters correctly + populatedDSN, err := expandDSN(dsn) if err != nil { return nil, Unknown, err } diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index 9949957e..6250e490 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -82,3 +82,522 @@ func Test_updateDSNFromEnv(t *testing.T) { }) } } + +func Test_expandDSN(t *testing.T) { + tests := []struct { + name string + env map[string]string + dsn string + want string + wantErr bool + }{ + { + name: "No placeholders", + env: map[string]string{}, + dsn: "mysql://user:password@localhost:3306/dbname", + want: "mysql://user:password@localhost:3306/dbname", + }, + { + name: "Simple password with hash symbol", + env: map[string]string{ + "DB_PASSWORD": "p@ss#123", + }, + dsn: "mysql://admin:${DB_PASSWORD}@localhost:3306/dbname", + want: "mysql://admin:p%40ss%23123@localhost:3306/dbname", + }, + { + name: "Password with multiple special characters", + env: map[string]string{ + "DB_PASSWORD": "p@ss#wo:rd/123", + }, + dsn: "mysql://admin:${DB_PASSWORD}@localhost:3306/dbname", + want: "mysql://admin:p%40ss%23wo%3Ard%2F123@localhost:3306/dbname", + }, + { + name: "Username with special characters", + env: map[string]string{ + "DB_USER": "user@domain", + }, + dsn: "mysql://${DB_USER}:password@localhost:3306/dbname", + want: "mysql://user%40domain:password@localhost:3306/dbname", + }, + { + name: "Both username and password with special chars", + env: map[string]string{ + "DB_USER": "user:name", + "DB_PASS": "pass#word", + }, + dsn: "mysql://${DB_USER}:${DB_PASS}@localhost:3306/dbname", + want: "mysql://user%3Aname:pass%23word@localhost:3306/dbname", + }, + { + name: "Hostname from env variable", + env: map[string]string{ + "DB_HOST": "db.example.com", + }, + dsn: "mysql://admin:password@${DB_HOST}:3306/dbname", + want: "mysql://admin:password@db.example.com:3306/dbname", + }, + { + name: "Port from env variable", + env: map[string]string{ + "DB_PORT": "3306", + }, + dsn: "mysql://user:password@localhost:${DB_PORT}/dbname", + want: "mysql://user:password@localhost:3306/dbname", + }, + { + name: "All components from env including port", + env: map[string]string{ + "DB_USER": "admin", + "DB_PASS": "p@ss#123", + "DB_HOST": "db.example.com:3306", + "DB_NAME": "mydb", + }, + dsn: "mysql://${DB_USER}:${DB_PASS}@${DB_HOST}/${DB_NAME}", + want: "mysql://admin:p%40ss%23123@db.example.com:3306/mydb", + }, + { + name: "Entire userinfo as single variable", + env: map[string]string{ + "DB_CREDENTIALS": "admin:p@ss#123", + }, + dsn: "mysql://${DB_CREDENTIALS}@localhost:3306/dbname", + want: "mysql://admin:p%40ss%23123@localhost:3306/dbname", + }, + { + name: "Same variable used multiple times", + env: map[string]string{ + "DB_NAME": "mydb", + }, + dsn: "mysql://user:pass@localhost:3306/${DB_NAME}?schema=${DB_NAME}", + want: "mysql://user:pass@localhost:3306/mydb?schema=mydb", + }, + { + name: "Query parameters with special chars", + env: map[string]string{ + "QUERY_VAL": "value&special", + }, + dsn: "postgres://user:pass@localhost/db?param=${QUERY_VAL}", + want: "postgres://user:pass@localhost/db?param=value&special", + }, + { + name: "Path with special characters", + env: map[string]string{ + "DB_NAME": "my/db", + }, + dsn: "mysql://user:pass@localhost:3306/${DB_NAME}", + want: "mysql://user:pass@localhost:3306/my/db", + }, + { + name: "PostgreSQL style DSN", + env: map[string]string{ + "DB_PASSWORD": "test#pass", + }, + dsn: "postgres://postgres:${DB_PASSWORD}@localhost:5432/testdb?sslmode=disable", + want: "postgres://postgres:test%23pass@localhost:5432/testdb?sslmode=disable", + }, + { + name: "SQL Server style DSN", + env: map[string]string{ + "DB_PASSWORD": "P@ss#123", + }, + dsn: "sqlserver://sa:${DB_PASSWORD}@localhost:1433?database=master", + want: "sqlserver://sa:P%40ss%23123@localhost:1433?database=master", + }, + { + name: "Oracle style DSN", + env: map[string]string{ + "DB_PASSWORD": "Oracle#123", + }, + dsn: "oracle://system:${DB_PASSWORD}@localhost:1521/ORCLPDB1", + want: "oracle://system:Oracle%23123@localhost:1521/ORCLPDB1", + }, + { + name: "HDB style DSN", + env: map[string]string{ + "DB_PASSWORD": "hdb#pass", + }, + dsn: "hdb://SYSTEM:${DB_PASSWORD}@localhost:39017", + want: "hdb://SYSTEM:hdb%23pass@localhost:39017", + }, + { + name: "Missing environment variable", + env: map[string]string{}, + dsn: "mysql://admin:${MISSING_VAR}@localhost:3306/dbname", + want: "", + wantErr: true, + }, + { + name: "Empty password", + env: map[string]string{ + "DB_PASSWORD": "", + }, + dsn: "mysql://admin:${DB_PASSWORD}@localhost:3306/dbname", + want: "mysql://admin:@localhost:3306/dbname", + }, + { + name: "URL with fragment", + env: map[string]string{ + "DB_FRAGMENT": "section", + }, + dsn: "mysql://user:pass@localhost/db#${DB_FRAGMENT}", + want: "mysql://user:pass@localhost/db#section", + }, + { + name: "Complex real-world example", + env: map[string]string{ + "DB_USER": "app_user", + "DB_PASSWORD": "C0mpl3x!P@ss#w0rd$123", + "DB_HOST": "prod-db.example.com", + "DB_NAME": "production_db", + }, + dsn: "postgres://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:5432/${DB_NAME}?sslmode=require&connect_timeout=10", + want: "postgres://app_user:C0mpl3x%21P%40ss%23w0rd$123@prod-db.example.com:5432/production_db?sslmode=require&connect_timeout=10", + }, + // Edge cases - security and parser confusion + { + name: "Sentinel collision in password", + env: map[string]string{ + "DB_PASSWORD": "hack__PH_0__attack", + }, + dsn: "mysql://admin:${DB_PASSWORD}@localhost/db", + want: "mysql://admin:hack__PH_0__attack@localhost/db", + }, + { + name: "Percent sign in password (literal)", + env: map[string]string{ + "DB_PASSWORD": "50%off", + }, + dsn: "mysql://admin:${DB_PASSWORD}@localhost/db", + want: "mysql://admin:50%25off@localhost/db", + }, + { + name: "At-sign in username", + env: map[string]string{ + "DB_USER": "user@domain.com", + }, + dsn: "mysql://${DB_USER}:password@localhost/db", + want: "mysql://user%40domain.com:password@localhost/db", + wantErr: false, // Should encode the @ properly + }, + { + name: "IPv6 address", + env: map[string]string{ + "DB_HOST": "[2001:db8::1]", + }, + dsn: "mysql://user:pass@${DB_HOST}:3306/db", + want: "mysql://user:pass@[2001:db8::1]:3306/db", + }, + { + name: "Empty username with password", + env: map[string]string{ + "DB_PASSWORD": "secret", + }, + dsn: "mysql://:${DB_PASSWORD}@localhost/db", + want: "mysql://:secret@localhost/db", + }, + { + name: "Literal ${ without closing brace", + env: map[string]string{ + "DB_PASSWORD": "my${incomplete", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:my$%7Bincomplete@localhost/db", + }, + { + name: "Multiple colons in password", + env: map[string]string{ + "DB_PASSWORD": "pass:word:with:colons", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%3Aword%3Awith%3Acolons@localhost/db", + }, + { + name: "Multiple colons in entire userinfo variable", + env: map[string]string{ + "DB_CREDENTIALS": "user:pass:word:extra", + }, + dsn: "mysql://${DB_CREDENTIALS}@localhost/db", + want: "mysql://user:pass%3Aword%3Aextra@localhost/db", + }, + { + name: "Ampersand in password (allowed per RFC 3986)", + env: map[string]string{ + "DB_PASSWORD": "pass&word", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass&word@localhost/db", + }, + { + name: "Ampersand in query parameter value", + env: map[string]string{ + "PARAM_VALUE": "value&sneaky", + }, + dsn: "postgres://user:pass@localhost/db?setting=${PARAM_VALUE}", + want: "postgres://user:pass@localhost/db?setting=value&sneaky", + }, + { + name: "Equals sign in database name", + env: map[string]string{ + "DB_NAME": "db=production", + }, + dsn: "mysql://user:pass@localhost/${DB_NAME}", + want: "mysql://user:pass@localhost/db=production", + }, + { + name: "Unicode in password", + env: map[string]string{ + "DB_PASSWORD": "пароль密码", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:%D0%BF%D0%B0%D1%80%D0%BE%D0%BB%D1%8C%E5%AF%86%E7%A0%81@localhost/db", + }, + { + name: "Emoji in password", + env: map[string]string{ + "DB_PASSWORD": "🔒secure", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:%F0%9F%94%92secure@localhost/db", + }, + { + name: "Backslash in host (SQL Server named instance - gets encoded)", + env: map[string]string{ + "DB_HOST": "server\\SQLEXPRESS", + }, + dsn: "sqlserver://user:pass@${DB_HOST}/db", + want: "sqlserver://user:pass@server%5CSQLEXPRESS/db", + }, + { + name: "Forward slash in password", + env: map[string]string{ + "DB_PASSWORD": "pass/word", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%2Fword@localhost/db", + }, + { + name: "Question mark in password", + env: map[string]string{ + "DB_PASSWORD": "pass?word", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%3Fword@localhost/db", + }, + { + name: "Empty password (explicitly set)", + env: map[string]string{ + "DB_PASSWORD": "", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:@localhost/db", + }, + { + name: "Space in password", + env: map[string]string{ + "DB_PASSWORD": "pass word", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%20word@localhost/db", + }, + { + name: "Newline in password", + env: map[string]string{ + "DB_PASSWORD": "pass\nword", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%0Aword@localhost/db", + }, + { + name: "Tab in password", + env: map[string]string{ + "DB_PASSWORD": "pass\tword", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%09word@localhost/db", + }, + { + name: "Double quotes in password", + env: map[string]string{ + "DB_PASSWORD": "pass\"word", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%22word@localhost/db", + }, + { + name: "Single quote in password", + env: map[string]string{ + "DB_PASSWORD": "pass'word", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:pass%27word@localhost/db", + }, + { + name: "Password that looks like numeric sentinel", + env: map[string]string{ + "DB_PASSWORD": "999000", + }, + dsn: "mysql://user:${DB_PASSWORD}@localhost/db", + want: "mysql://user:999000@localhost/db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set up environment variables + for k, v := range tt.env { + t.Setenv(k, v) + } + + got, err := expandDSN(tt.dsn) + if (err != nil) != tt.wantErr { + t.Errorf("expandDSN() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("expandDSN() got = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_extractPlaceholders(t *testing.T) { + tests := []struct { + name string + input string + env map[string]string + wantSentinel string + wantMappingSize int + wantErr bool + }{ + { + name: "No placeholders", + input: "mysql://user:pass@localhost/db", + env: map[string]string{}, + wantSentinel: "mysql://user:pass@localhost/db", + wantMappingSize: 0, + }, + { + name: "Single placeholder", + input: "mysql://user:${PASSWORD}@localhost/db", + env: map[string]string{ + "PASSWORD": "secret", + }, + wantSentinel: "mysql://user:999000@localhost/db", + wantMappingSize: 1, + }, + { + name: "Multiple placeholders", + input: "mysql://${USER}:${PASSWORD}@${HOST}/db", + env: map[string]string{ + "USER": "admin", + "PASSWORD": "secret", + "HOST": "localhost", + }, + wantSentinel: "mysql://999000:999001@999002/db", + wantMappingSize: 3, + }, + { + name: "Same variable twice", + input: "${VAR}:${VAR}", + env: map[string]string{ + "VAR": "value", + }, + wantSentinel: "999000:999001", + wantMappingSize: 2, + }, + { + name: "Port placeholder", + input: "mysql://user:pass@localhost:${PORT}/db", + env: map[string]string{ + "PORT": "3306", + }, + wantSentinel: "mysql://user:pass@localhost:999000/db", + wantMappingSize: 1, + }, + { + name: "Missing environment variable", + input: "mysql://user:${MISSING}@localhost/db", + env: map[string]string{}, + wantSentinel: "", + wantMappingSize: 0, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set up environment variables + for k, v := range tt.env { + t.Setenv(k, v) + } + + gotSentinel, gotMapping, err := extractPlaceholders(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("extractPlaceholders() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + if gotSentinel != tt.wantSentinel { + t.Errorf("extractPlaceholders() sentinel = %v, want %v", gotSentinel, tt.wantSentinel) + } + if len(gotMapping) != tt.wantMappingSize { + t.Errorf("extractPlaceholders() mapping size = %v, want %v", len(gotMapping), tt.wantMappingSize) + } + }) + } +} + +func Test_expandWithMapping(t *testing.T) { + tests := []struct { + name string + input string + mapping map[string]string + want string + }{ + { + name: "No sentinels", + input: "plain text", + mapping: map[string]string{}, + want: "plain text", + }, + { + name: "Single sentinel", + input: "user:999000", + mapping: map[string]string{ + "999000": "secret", + }, + want: "user:secret", + }, + { + name: "Multiple sentinels", + input: "999000:999001@999002", + mapping: map[string]string{ + "999000": "admin", + "999001": "secret", + "999002": "localhost", + }, + want: "admin:secret@localhost", + }, + { + name: "Sentinel not in input", + input: "no sentinels here", + mapping: map[string]string{ + "999000": "ignored", + }, + want: "no sentinels here", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := expandWithMapping(tt.input, tt.mapping) + if got != tt.want { + t.Errorf("expandWithMapping() got = %v, want %v", got, tt.want) + } + }) + } +} From 43b54f0b4850995585ee1cdb0fd70edeb449ded6 Mon Sep 17 00:00:00 2001 From: Geoff Greer Date: Thu, 20 Nov 2025 15:09:52 -0800 Subject: [PATCH 2/4] Add failing test case. --- pkg/database/database_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index 6250e490..bf5b8421 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -105,6 +105,14 @@ func Test_expandDSN(t *testing.T) { dsn: "mysql://admin:${DB_PASSWORD}@localhost:3306/dbname", want: "mysql://admin:p%40ss%23123@localhost:3306/dbname", }, + { + name: "Port", + env: map[string]string{ + "DB_PORT": "3306", + }, + dsn: "mysql://user:password@localhost:${DB_PORT}/dbname", + want: "mysql://user:password@localhost:3306/dbname", + }, { name: "Password with multiple special characters", env: map[string]string{ From 9d01224b44b275c29d82b059efb8aa774b7c97cf Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Fri, 21 Nov 2025 18:17:13 +0000 Subject: [PATCH 3/4] improve query param handling --- pkg/database/database.go | 18 +++++++++++++++++- pkg/database/database_test.go | 14 +++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/database/database.go b/pkg/database/database.go index 61126f91..6179f8c6 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -149,7 +149,23 @@ func expandPath(parsedUrl *url.URL, mapping map[string]string) { // expandQuery expands environment variable placeholders in the URL's query string. func expandQuery(parsedUrl *url.URL, mapping map[string]string) { if parsedUrl.RawQuery != "" { - parsedUrl.RawQuery = expandWithMapping(parsedUrl.RawQuery, mapping) + values, err := url.ParseQuery(parsedUrl.RawQuery) + if err != nil { + // Fallback: if parsing fails for some reason, do a direct expansion. + // This preserves prior behavior rather than failing entirely. + parsedUrl.RawQuery = expandWithMapping(parsedUrl.RawQuery, mapping) + return + } + + newValues := url.Values{} + for key, vals := range values { + expandedKey := expandWithMapping(key, mapping) + for _, v := range vals { + expandedVal := expandWithMapping(v, mapping) + newValues.Add(expandedKey, expandedVal) + } + } + parsedUrl.RawQuery = newValues.Encode() } } diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index bf5b8421..b82bfb48 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -187,7 +187,15 @@ func Test_expandDSN(t *testing.T) { "QUERY_VAL": "value&special", }, dsn: "postgres://user:pass@localhost/db?param=${QUERY_VAL}", - want: "postgres://user:pass@localhost/db?param=value&special", + want: "postgres://user:pass@localhost/db?param=value%26special", + }, + { + name: "Space in query parameter value", + env: map[string]string{ + "PARAM_VALUE": "foo bar", + }, + dsn: "postgres://user:pass@localhost/db?setting=${PARAM_VALUE}", + want: "postgres://user:pass@localhost/db?setting=foo+bar", }, { name: "Path with special characters", @@ -261,7 +269,7 @@ func Test_expandDSN(t *testing.T) { "DB_NAME": "production_db", }, dsn: "postgres://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:5432/${DB_NAME}?sslmode=require&connect_timeout=10", - want: "postgres://app_user:C0mpl3x%21P%40ss%23w0rd$123@prod-db.example.com:5432/production_db?sslmode=require&connect_timeout=10", + want: "postgres://app_user:C0mpl3x%21P%40ss%23w0rd$123@prod-db.example.com:5432/production_db?connect_timeout=10&sslmode=require", }, // Edge cases - security and parser confusion { @@ -343,7 +351,7 @@ func Test_expandDSN(t *testing.T) { "PARAM_VALUE": "value&sneaky", }, dsn: "postgres://user:pass@localhost/db?setting=${PARAM_VALUE}", - want: "postgres://user:pass@localhost/db?setting=value&sneaky", + want: "postgres://user:pass@localhost/db?setting=value%26sneaky", }, { name: "Equals sign in database name", From 45f3cf1825ea81654e30a9dde0b3726bf6e8b5a0 Mon Sep 17 00:00:00 2001 From: Paul Querna Date: Fri, 21 Nov 2025 22:49:59 +0000 Subject: [PATCH 4/4] add new config syntax --- examples/example.yml | 28 ++++-- pkg/bsql/config.go | 28 +++++- pkg/connector/connector.go | 13 ++- pkg/database/database.go | 182 +++++++++++++++++++++++++++++----- pkg/database/database_test.go | 105 ++++++++++++++++++++ 5 files changed, 317 insertions(+), 39 deletions(-) diff --git a/examples/example.yml b/examples/example.yml index d57ace48..dee124de 100644 --- a/examples/example.yml +++ b/examples/example.yml @@ -10,17 +10,27 @@ app_name: Example Application # Connection Configuration # ---------------------- # Specifies how to connect to the data source. Supports various connection methods. +# +# RECOMMENDED: Use structured fields! connect: - # Database connection string (DSN) with environment variable interpolation - dsn: "mysql://${DB_USER}:${DB_PASS}@${DB_HOST}:3306/${DB_NAME}?parseTime=true" -# If your database username or password includes characters that require URL encoding, -# you can specify them as separate options instead of embedding them directly in the DSN. -# Environment variables are expanded. -# For example, you might include: -# username: my_username -# password: my_secure_password + scheme: "mysql" + host: "${DB_HOST}" + port: "3306" + database: "${DB_NAME}" + user: "${DB_USER}" + password: "${DB_PASS}" + params: + parseTime: "true" # -# This allows the connector to handle proper URL encoding during DSN construction. +# ALTERNATIVE: Use DSN with separate user/password fields +# connect: +# dsn: "mysql://${DB_HOST}:3306/${DB_NAME}?parseTime=true" +# user: "${DB_USER}" +# password: "${DB_PASS}" +# +# LEGACY: Complete DSN (not recommended if credentials contain special characters) +# connect: +# dsn: "mysql://${DB_USER}:${DB_PASS}@${DB_HOST}:3306/${DB_NAME}?parseTime=true" # Resource Types # ------------- diff --git a/pkg/bsql/config.go b/pkg/bsql/config.go index bb22a057..b580b77d 100644 --- a/pkg/bsql/config.go +++ b/pkg/bsql/config.go @@ -36,18 +36,36 @@ func (c Config) HasActions() bool { } // DatabaseConfig contains settings required to connect to the database. +// You can specify either a complete DSN, or use structured fields, or a combination. +// Structured fields override corresponding parts of the DSN when both are provided. type DatabaseConfig struct { - // DSN is the Database Source Name connection string used to establish the database connection. + // DSN is the Database Source Name connection string (optional if using structured fields). + // Supports environment variable expansion via ${VAR_NAME} syntax. + // Example: "postgres://${DB_HOST}:${DB_PORT}/${DB_DATABASE}?sslmode=disable" DSN string `yaml:"dsn" json:"dsn"` - // These fields are not required if the DSN already includes the credentials. - // They should only be provided if the username or password contain characters that need URL encoding. + // Structured connection fields (optional, override DSN components when set) - // User is the database username used for authentication. + // Scheme is the database type (e.g., "postgres", "mysql", "sqlserver", "oracle", "hdb") + Scheme string `yaml:"scheme" json:"scheme"` + + // Host is the database server hostname or IP address (may include port for some databases) + Host string `yaml:"host" json:"host"` + + // Port is the database server port number + Port string `yaml:"port" json:"port"` + + // Database is the name of the database to connect to + Database string `yaml:"database" json:"database"` + + // User is the database username used for authentication User string `yaml:"user" json:"user"` - // Password is the database password used for authentication. + // Password is the database password used for authentication Password string `yaml:"password" json:"password"` + + // Params contains additional connection parameters (e.g., {"sslmode": "disable", "timeout": "30s"}) + Params map[string]string `yaml:"params" json:"params"` } // ResourceType defines configuration for a specific type of resource. diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 6a0a877b..f078a09a 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -94,7 +94,18 @@ func New(ctx context.Context, configFilePath string) (*Connector, error) { } func newConnector(ctx context.Context, c *bsql.Config) (*Connector, error) { - db, dbEngine, err := database.Connect(ctx, c.Connect.DSN, c.Connect.User, c.Connect.Password) + opts := database.ConnectOptions{ + DSN: c.Connect.DSN, + Scheme: c.Connect.Scheme, + Host: c.Connect.Host, + Port: c.Connect.Port, + Database: c.Connect.Database, + User: c.Connect.User, + Password: c.Connect.Password, + Params: c.Connect.Params, + } + + db, dbEngine, err := database.Connect(ctx, opts) if err != nil { return nil, err } diff --git a/pkg/database/database.go b/pkg/database/database.go index 6179f8c6..168b1202 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "net" "net/url" "os" "regexp" @@ -31,6 +32,22 @@ const ( HDB ) +// ConnectOptions represents the structured configuration used to build a DSN. +// Any field may include ${ENV_VAR} placeholders that will be expanded before use. +type ConnectOptions struct { + DSN string + + Scheme string + Host string + Port string + Database string + + User string + Password string + + Params map[string]string +} + func updateFromEnv(dsn string) (string, error) { var err error @@ -211,34 +228,14 @@ func expandDSN(dsn string) (string, error) { return parsedUrl.String(), nil } -func Connect(ctx context.Context, dsn string, user string, password string) (*sql.DB, DbEngine, error) { - // Use the new expandDSN function which handles special characters correctly - populatedDSN, err := expandDSN(dsn) +func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error) { + parsedDsn, err := buildConnectionURL(opts) if err != nil { return nil, Unknown, err } - parsedDsn, err := url.Parse(populatedDSN) - if err != nil { - return nil, Unknown, err - } - - if parsedDsn.User == nil { - if user == "" || password == "" { - return nil, Unknown, errors.New("user and password must be set in DSN or in the configuration") - } - - populatedUser, err := updateFromEnv(user) - if err != nil { - return nil, Unknown, err - } - - populatedPassword, err := updateFromEnv(password) - if err != nil { - return nil, Unknown, err - } - - parsedDsn.User = url.UserPassword(populatedUser, populatedPassword) + if parsedDsn.Scheme == "" { + return nil, Unknown, errors.New("database scheme must be specified in DSN or configuration") } switch parsedDsn.Scheme { @@ -281,3 +278,140 @@ func Connect(ctx context.Context, dsn string, user string, password string) (*sq return nil, Unknown, fmt.Errorf("unsupported database scheme: %s", parsedDsn.Scheme) } } + +func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { + var ( + parsedUrl *url.URL + err error + ) + + if opts.DSN != "" { + populatedDSN, err := expandDSN(opts.DSN) + if err != nil { + return nil, err + } + parsedUrl, err = url.Parse(populatedDSN) + if err != nil { + return nil, err + } + } else { + parsedUrl = &url.URL{} + } + + scheme, err := expandValue(opts.Scheme) + if err != nil { + return nil, err + } + if scheme != "" { + parsedUrl.Scheme = scheme + } + + host, err := expandValue(opts.Host) + if err != nil { + return nil, err + } + + port, err := expandValue(opts.Port) + if err != nil { + return nil, err + } + + hostValue := parsedUrl.Hostname() + if hostValue == "" && parsedUrl.Host != "" { + hostValue = parsedUrl.Host + } + if host != "" { + hostValue = host + } + + portValue := parsedUrl.Port() + if portValue == "" && parsedUrl.Host != "" { + if _, p, err := net.SplitHostPort(parsedUrl.Host); err == nil { + portValue = p + } + } + if port != "" { + portValue = port + } + + if hostValue != "" { + if portValue != "" { + // JoinHostPort expects the host without brackets for IPv6 + // If hostValue has brackets, strip them; if it's a raw IPv6, leave as-is + cleanHost := hostValue + if len(hostValue) > 2 && hostValue[0] == '[' && hostValue[len(hostValue)-1] == ']' { + cleanHost = hostValue[1 : len(hostValue)-1] + } + parsedUrl.Host = net.JoinHostPort(cleanHost, portValue) + } else { + parsedUrl.Host = hostValue + } + } else if portValue != "" { + return nil, fmt.Errorf("port provided without host") + } + + databaseName, err := expandValue(opts.Database) + if err != nil { + return nil, err + } + if databaseName != "" { + if strings.HasPrefix(databaseName, "/") { + parsedUrl.Path = databaseName + } else { + parsedUrl.Path = "/" + databaseName + } + } + + user, err := expandValue(opts.User) + if err != nil { + return nil, err + } + + password, err := expandValue(opts.Password) + if err != nil { + return nil, err + } + + if user != "" || password != "" { + if password != "" { + parsedUrl.User = url.UserPassword(user, password) + } else { + parsedUrl.User = url.User(user) + } + } + + if len(opts.Params) > 0 { + values := parsedUrl.Query() + if values == nil { + values = url.Values{} + } + for k, v := range opts.Params { + key, err := expandValue(k) + if err != nil { + return nil, err + } + value, err := expandValue(v) + if err != nil { + return nil, err + } + values.Set(key, value) + } + parsedUrl.RawQuery = values.Encode() + } + + if parsedUrl.Scheme == "" && opts.DSN == "" { + return nil, fmt.Errorf("database scheme must be specified") + } + + return parsedUrl, nil +} + +func expandValue(s string) (string, error) { + if s == "" { + return s, nil + } + if DSNREnvRegex.MatchString(s) { + return updateFromEnv(s) + } + return s, nil +} diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index b82bfb48..257307d7 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -617,3 +617,108 @@ func Test_expandWithMapping(t *testing.T) { }) } } + +func Test_buildConnectionURL(t *testing.T) { + t.Setenv("DB_HOST", "db.internal") + t.Setenv("DB_PORT", "5432") + t.Setenv("DB_NAME", "appdb") + t.Setenv("DB_USER", "app_user") + t.Setenv("DB_PASSWORD", "s3cr3t!") + + tests := []struct { + name string + opts ConnectOptions + want string + wantErr bool + }{ + { + name: "DSN only", + opts: ConnectOptions{ + DSN: "postgres://user:pass@localhost:5432/db?sslmode=disable", + }, + want: "postgres://user:pass@localhost:5432/db?sslmode=disable", + }, + { + name: "Override DSN components", + opts: ConnectOptions{ + DSN: "postgres://user:pass@localhost:5432/db?sslmode=disable", + Host: "override.internal", + Port: "6543", + Database: "override_db", + User: "override_user", + Password: "override#pass", + Params: map[string]string{ + "sslmode": "require", + "connect_timeout": "10", + "application_name": "baton", + }, + }, + want: "postgres://override_user:override%23pass@override.internal:6543/override_db?application_name=baton&connect_timeout=10&sslmode=require", + }, + { + name: "Structured config only", + opts: ConnectOptions{ + Scheme: "postgres", + Host: "${DB_HOST}", + Port: "${DB_PORT}", + Database: "${DB_NAME}", + User: "${DB_USER}", + Password: "${DB_PASSWORD}", + Params: map[string]string{ + "sslmode": "disable", + }, + }, + want: "postgres://app_user:s3cr3t%21@db.internal:5432/appdb?sslmode=disable", + }, + { + name: "Port without host", + opts: ConnectOptions{ + Scheme: "postgres", + Port: "5432", + }, + wantErr: true, + }, + { + name: "IPv6 host with port", + opts: ConnectOptions{ + Scheme: "postgres", + Host: "[::1]", + Port: "5432", + Database: "testdb", + User: "testuser", + Password: "testpass", + }, + want: "postgres://testuser:testpass@[::1]:5432/testdb", + }, + { + name: "IPv6 host without brackets gets brackets added by JoinHostPort", + opts: ConnectOptions{ + Scheme: "postgres", + Host: "2001:db8::1", + Port: "5432", + Database: "testdb", + }, + want: "postgres://[2001:db8::1]:5432/testdb", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := buildConnectionURL(tt.opts) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error but got nil") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got.String() != tt.want { + t.Fatalf("buildConnectionURL() = %s, want %s", got.String(), tt.want) + } + }) + } +}