From 50e20c0b0c634e92d187306f8890f34442798ccc Mon Sep 17 00:00:00 2001 From: Bjorn Tipling Date: Sat, 25 Jul 2026 19:07:31 +0000 Subject: [PATCH 1/3] feat: library API for NewFromYAML + LookupFunc secret expansion Add injectable LookupFunc for ${KEY} expansion (no process env mutation), NewWithConfig/NewFromYAML for library embeds, OfflineValidate and RejectNonV1ProductFeatures for editor/agent offline checks, and a C1 golden Postgres fixture with externalized map-value secrets. CE-1007 / ds-663v Co-authored-by: c1-squire-dev[bot] --- examples/c1-golden-postgres.yml | 53 +++++++ pkg/bsql/offline_validate.go | 121 ++++++++++++++++ pkg/bsql/offline_validate_test.go | 166 ++++++++++++++++++++++ pkg/connector/connector.go | 34 ++++- pkg/connector/connector_from_yaml_test.go | 78 ++++++++++ pkg/database/database.go | 74 +++++++--- pkg/database/database_test.go | 4 +- pkg/database/lookup_test.go | 111 +++++++++++++++ 8 files changed, 615 insertions(+), 26 deletions(-) create mode 100644 examples/c1-golden-postgres.yml create mode 100644 pkg/bsql/offline_validate.go create mode 100644 pkg/bsql/offline_validate_test.go create mode 100644 pkg/connector/connector_from_yaml_test.go create mode 100644 pkg/database/lookup_test.go diff --git a/examples/c1-golden-postgres.yml b/examples/c1-golden-postgres.yml new file mode 100644 index 00000000..8c4d81eb --- /dev/null +++ b/examples/c1-golden-postgres.yml @@ -0,0 +1,53 @@ +--- +# C1 agent-authored v1 golden fixture (Postgres, sync-only). +# Secrets must live in C1 map values, not in this YAML. +# +# Expected c1 sql_connector_map_values keys: +# DB_HOST, DB_PORT, DB_DATABASE, DB_USER, DB_PASSWORD +# +# SaaS network: customer DB must allowlist ConductorOne documented egress IPs. +app_name: Golden Postgres +app_description: Sync-only Postgres users for C1 SQL catalog golden path + +connect: + scheme: postgres + host: ${DB_HOST} + port: ${DB_PORT} + database: ${DB_DATABASE} + user: ${DB_USER} + password: ${DB_PASSWORD} + params: + sslmode: disable + +resource_types: + user: + name: User + description: A user row in the database + list: + query: | + SELECT + u.id, + u.username, + u.email, + u.status + FROM users u + pagination: + strategy: offset + primary_key: id + map: + id: ".username" + display_name: ".username" + description: ".email" + traits: + user: + status: ".status" + login: ".username" + emails: + - ".email" + static_entitlements: + - id: member + display_name: "'Member'" + description: "'Application member'" + purpose: assignment + grantable_to: + - user diff --git a/pkg/bsql/offline_validate.go b/pkg/bsql/offline_validate.go new file mode 100644 index 00000000..04f4dad5 --- /dev/null +++ b/pkg/bsql/offline_validate.go @@ -0,0 +1,121 @@ +package bsql + +import ( + "errors" + "fmt" + "net/url" + "strings" +) + +// OfflineValidate performs YAML-level structural checks without opening a DB or +// requiring SQLSyncer. Suitable for editor/RPC offline validation. +func OfflineValidate(cfg *Config) error { + if cfg == nil { + return errors.New("config is nil") + } + if strings.TrimSpace(cfg.AppName) == "" { + return errors.New("app_name is required") + } + if err := validateConnectOffline(&cfg.Connect); err != nil { + return err + } + if len(cfg.ResourceTypes) == 0 { + return errors.New("resource_types is required") + } + for name, rt := range cfg.ResourceTypes { + if strings.TrimSpace(rt.Name) == "" { + return fmt.Errorf("resource_types.%s: name is required", name) + } + if rt.List == nil || strings.TrimSpace(rt.List.Query) == "" { + return fmt.Errorf("resource_types.%s: list.query is required", name) + } + if err := validateScope(rt.List.Scope); err != nil { + return fmt.Errorf("resource_types.%s.list: %w", name, err) + } + } + return RejectNonV1ProductFeatures(cfg) +} + +// RejectNonV1ProductFeatures fails configs that are outside the C1 agent-authored +// v1 product surface (Postgres single-DB sync-only). +func RejectNonV1ProductFeatures(cfg *Config) error { + if cfg == nil { + return errors.New("config is nil") + } + if cfg.Connect.Databases != nil { + return errors.New("connect.databases is not supported in v1 (single database only)") + } + if len(cfg.Actions) > 0 { + return errors.New("actions are not supported in v1 (sync-only)") + } + for name, rt := range cfg.ResourceTypes { + if rt.AccountProvisioning != nil { + return fmt.Errorf("resource_types.%s: account_provisioning is not supported in v1", name) + } + if rt.CredentialRotation != nil { + return fmt.Errorf("resource_types.%s: credential_rotation is not supported in v1", name) + } + } + scheme, err := resolveConnectScheme(&cfg.Connect) + if err != nil { + return err + } + if scheme != "postgres" { + return fmt.Errorf("scheme %q is not supported in v1; use postgres:// only (not postgresql)", scheme) + } + return nil +} + +// ValidateYAML parses YAML bytes and runs OfflineValidate. +func ValidateYAML(data []byte) error { + cfg, err := Parse(data) + if err != nil { + return fmt.Errorf("invalid YAML: %w", err) + } + return OfflineValidate(cfg) +} + +func validateConnectOffline(c *DatabaseConfig) error { + if c == nil { + return errors.New("connect is required") + } + hasDSN := strings.TrimSpace(c.DSN) != "" + hasScheme := strings.TrimSpace(c.Scheme) != "" + hasHost := strings.TrimSpace(c.Host) != "" + if !hasDSN && !hasScheme { + return errors.New("connect: dsn or scheme is required") + } + if !hasDSN && hasScheme && !hasHost { + return errors.New("connect: host is required when using structured fields without dsn") + } + return nil +} + +func resolveConnectScheme(c *DatabaseConfig) (string, error) { + if c == nil { + return "", errors.New("connect is required") + } + if s := strings.TrimSpace(c.Scheme); s != "" { + return strings.ToLower(s), nil + } + dsn := strings.TrimSpace(c.DSN) + if dsn == "" { + return "", errors.New("connect: scheme or dsn is required") + } + // Placeholders like postgres://${HOST}/db — peel scheme before parse when possible. + if idx := strings.Index(dsn, "://"); idx > 0 { + return strings.ToLower(dsn[:idx]), nil + } + // Entire DSN may be a single ${VAR}; cannot resolve scheme offline without lookup. + if strings.HasPrefix(dsn, "${") && strings.HasSuffix(dsn, "}") { + return "", errors.New("connect: scheme must be set explicitly when dsn is a single placeholder") + } + u, err := url.Parse(dsn) + if err != nil { + return "", fmt.Errorf("connect: invalid dsn: %w", err) + } + if u.Scheme == "" { + return "", errors.New("connect: scheme missing from dsn") + } + return strings.ToLower(u.Scheme), nil +} diff --git a/pkg/bsql/offline_validate_test.go b/pkg/bsql/offline_validate_test.go new file mode 100644 index 00000000..430e0be3 --- /dev/null +++ b/pkg/bsql/offline_validate_test.go @@ -0,0 +1,166 @@ +package bsql + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func minimalPostgresYAML() string { + return ` +app_name: Golden Postgres +app_description: v1 sync-only fixture +connect: + scheme: postgres + host: ${DB_HOST} + port: ${DB_PORT} + database: ${DB_DATABASE} + user: ${DB_USER} + password: ${DB_PASSWORD} + params: + sslmode: disable +resource_types: + user: + name: User + description: Database user + list: + query: | + SELECT id, username, email, status FROM users + pagination: + strategy: offset + primary_key: id + map: + id: ".username" + display_name: ".username" + description: ".email" + traits: + user: + status: ".status" + login: ".username" + emails: + - ".email" + static_entitlements: + - id: member + display_name: "'Member'" + description: "'Member of app'" + purpose: assignment + grantable_to: + - user +` +} + +func TestOfflineValidate_HappyMinimalPostgres(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + require.NoError(t, OfflineValidate(cfg)) + require.NoError(t, ValidateYAML([]byte(minimalPostgresYAML()))) +} + +func TestOfflineValidate_NoNetwork(t *testing.T) { + // Ensure OfflineValidate does not attempt any DB connection for bad configs either. + err := ValidateYAML([]byte(` +app_name: x +connect: + scheme: postgres + host: 127.0.0.1 +resource_types: + user: + name: User + list: + query: SELECT 1 +`)) + require.NoError(t, err) +} + +func TestRejectNonV1_MultiDB(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + cfg.Connect.Databases = &DatabasesConfig{Static: []string{"a", "b"}} + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "databases") +} + +func TestRejectNonV1_Actions(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + cfg.Actions = map[string]ActionConfig{ + "enable": {Name: "Enable", Query: "SELECT 1"}, + } + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "actions") +} + +func TestRejectNonV1_AccountProvisioning(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + rt := cfg.ResourceTypes["user"] + rt.AccountProvisioning = &AccountProvisioning{} + cfg.ResourceTypes["user"] = rt + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "account_provisioning") +} + +func TestRejectNonV1_CredentialRotation(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + rt := cfg.ResourceTypes["user"] + rt.CredentialRotation = &CredentialRotation{} + cfg.ResourceTypes["user"] = rt + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "credential_rotation") +} + +func TestRejectNonV1_NonPostgresScheme(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + cfg.Connect.Scheme = "mysql" + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, strings.ToLower(err.Error()), "postgres") +} + +func TestRejectNonV1_PostgresqlAliasRejected(t *testing.T) { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + cfg.Connect.Scheme = "postgresql" + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "postgresql") +} + +func TestRejectNonV1_SchemeFromDSN(t *testing.T) { + cfg, err := Parse([]byte(` +app_name: DSN App +connect: + dsn: "postgres://${DB_HOST}:${DB_PORT}/${DB_DATABASE}?sslmode=disable" + user: "${DB_USER}" + password: "${DB_PASSWORD}" +resource_types: + user: + name: User + list: + query: SELECT 1 +`)) + require.NoError(t, err) + require.NoError(t, RejectNonV1ProductFeatures(cfg)) +} + +func TestOfflineValidate_MissingAppName(t *testing.T) { + err := ValidateYAML([]byte(` +connect: + scheme: postgres + host: localhost +resource_types: + user: + name: User + list: + query: SELECT 1 +`)) + require.Error(t, err) + require.Contains(t, err.Error(), "app_name") +} diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index f87e6af1..cb69ba02 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -103,6 +103,10 @@ func (c *Connector) Validate(ctx context.Context) (annotations.Annotations, erro return nil, nil } +// LookupFunc resolves ${KEY} placeholders for library embeds (same shape as baton-http). +// Prefer database.LookupFunc; this alias keeps the connector package self-documenting. +type LookupFunc = database.LookupFunc + // New returns a new instance of the connector. func New(ctx context.Context, configFilePath string) (*Connector, error) { c, err := bsql.LoadConfigFromFile(configFilePath) @@ -110,10 +114,35 @@ func New(ctx context.Context, configFilePath string) (*Connector, error) { return nil, err } - return newConnector(ctx, c) + return newConnector(ctx, c, nil) +} + +// NewWithConfig constructs a connector from an already-parsed config. +// Placeholder expansion uses process environment (nil LookupFunc). +func NewWithConfig(ctx context.Context, cfg *bsql.Config) (*Connector, error) { + if cfg == nil { + return nil, errors.New("connector: config is nil") + } + return newConnector(ctx, cfg, nil) +} + +// NewFromYAML parses YAML config bytes, expands ${KEY} via lookup (or env when nil), +// and opens the connector. Library embeds must pass a non-nil lookup over map values +// and must not mutate process environment. +func NewFromYAML(ctx context.Context, data []byte, lookup LookupFunc) (*Connector, error) { + cfg, err := bsql.Parse(data) + if err != nil { + return nil, err + } + return newConnector(ctx, cfg, lookup) +} + +// Config returns the parsed connector configuration. +func (c *Connector) Config() *bsql.Config { + return c.config } -func newConnector(ctx context.Context, c *bsql.Config) (*Connector, error) { +func newConnector(ctx context.Context, c *bsql.Config, lookup LookupFunc) (*Connector, error) { opts := database.ConnectOptions{ DSN: c.Connect.DSN, Scheme: c.Connect.Scheme, @@ -123,6 +152,7 @@ func newConnector(ctx context.Context, c *bsql.Config) (*Connector, error) { User: c.Connect.User, Password: c.Connect.Password, Params: c.Connect.Params, + Lookup: lookup, } dbs, dbEngine, err := openDatabases(ctx, opts, c.Connect.Databases) diff --git a/pkg/connector/connector_from_yaml_test.go b/pkg/connector/connector_from_yaml_test.go new file mode 100644 index 00000000..cafc1ebe --- /dev/null +++ b/pkg/connector/connector_from_yaml_test.go @@ -0,0 +1,78 @@ +package connector + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewFromYAML_LookupWithoutEnv(t *testing.T) { + // Use an unsupported scheme so Connect fails before dialing — we only assert + // that placeholders resolved via LookupFunc (error names resolved host, not ${}). + yaml := []byte(` +app_name: Test +connect: + scheme: unsupported-engine + host: ${DB_HOST} + database: ${DB_DATABASE} + user: ${DB_USER} + password: ${DB_PASSWORD} +resource_types: + user: + name: User + list: + query: SELECT 1 +`) + lookup := func(key string) (string, bool) { + m := map[string]string{ + "DB_HOST": "lookup-host.example", + "DB_DATABASE": "lookup-db", + "DB_USER": "lookup-user", + "DB_PASSWORD": "lookup-pass-secret", + } + v, ok := m[key] + return v, ok + } + + // Clear env so resolution cannot come from process environment. + t.Setenv("DB_HOST", "") + t.Setenv("DB_DATABASE", "") + t.Setenv("DB_USER", "") + t.Setenv("DB_PASSWORD", "") + + _, err := NewFromYAML(t.Context(), yaml, lookup) + require.Error(t, err) + // Expansion succeeded; connect failed on scheme after building URL. + require.Contains(t, err.Error(), "unsupported database scheme") + // Must not leave unresolved placeholders in the error path for scheme. + require.False(t, strings.Contains(err.Error(), "${DB_HOST}")) +} + +func TestNewFromYAML_MissingLookupKey(t *testing.T) { + yaml := []byte(` +app_name: Test +connect: + scheme: postgres + host: ${MISSING_KEY} + database: db + user: u + password: p +resource_types: + user: + name: User + list: + query: SELECT 1 +`) + lookup := func(key string) (string, bool) { + return "", false + } + _, err := NewFromYAML(t.Context(), yaml, lookup) + require.Error(t, err) + require.Contains(t, err.Error(), "MISSING_KEY") +} + +func TestNewWithConfig_Nil(t *testing.T) { + _, err := NewWithConfig(t.Context(), nil) + require.Error(t, err) +} diff --git a/pkg/database/database.go b/pkg/database/database.go index a9b057dd..005164ef 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -22,6 +22,11 @@ import ( var DSNREnvRegex = regexp.MustCompile(`\$\{([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. +type LookupFunc func(key string) (string, bool) + type DbEngine uint8 const ( @@ -50,15 +55,28 @@ type ConnectOptions struct { Password string Params map[string]string + + // Lookup resolves ${KEY} placeholders. When nil, os.LookupEnv is used. + Lookup LookupFunc } -func updateFromEnv(dsn string) (string, error) { +func (opts ConnectOptions) resolveLookup() LookupFunc { + if opts.Lookup != nil { + return opts.Lookup + } + return os.LookupEnv +} + +func updateFromLookup(dsn string, lookup LookupFunc) (string, error) { + if lookup == nil { + lookup = os.LookupEnv + } var err error result := DSNREnvRegex.ReplaceAllStringFunc(dsn, func(match string) string { varName := match[2 : len(match)-1] - value, exists := os.LookupEnv(varName) + value, exists := lookup(varName) if !exists { err = errors.Join(err, fmt.Errorf("environment variable %s is not set", varName)) return match @@ -72,11 +90,19 @@ func updateFromEnv(dsn string) (string, error) { return result, nil } +// updateFromEnv expands placeholders using process environment (CLI path). +func updateFromEnv(dsn string) (string, error) { + return updateFromLookup(dsn, os.LookupEnv) +} + // extractPlaceholders replaces ${...} placeholders with unique sentinels // using the _PH-999000_ format for all placeholders to avoid collisions. // Port sentinels are expanded before URL parsing (in expandDSN) since ports must be numeric. // Returns: the string with sentinels, a mapping of sentinel->value, and any error. -func extractPlaceholders(s string) (string, map[string]string, error) { +func extractPlaceholders(s string, lookup LookupFunc) (string, map[string]string, error) { + if lookup == nil { + lookup = os.LookupEnv + } mapping := make(map[string]string) counter := 0 var err error @@ -85,8 +111,7 @@ func extractPlaceholders(s string) (string, map[string]string, error) { sentinel := fmt.Sprintf("_PH-999%03d_", counter) varName := match[2 : len(match)-1] // Extract VAR from ${VAR} - // Look up the environment variable immediately - value, exists := os.LookupEnv(varName) + value, exists := lookup(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 @@ -258,15 +283,18 @@ func expandPortSentinel(sentinelDSN string, mapping map[string]string) string { } // expandDSN expands environment variable placeholders in a DSN using a three-phase approach: -// 1. Replace ${...} with safe sentinel values and lookup env vars +// 1. Replace ${...} with safe sentinel values and lookup values // 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) +func expandDSN(dsn string, lookup LookupFunc) (string, error) { + if lookup == nil { + lookup = os.LookupEnv + } + // Phase 1: Replace ${...} with sentinels and lookup values + sentinelDSN, mapping, err := extractPlaceholders(dsn, lookup) if err != nil { return "", err } @@ -282,7 +310,7 @@ func expandDSN(dsn string) (string, error) { if len(matches) == 1 && strings.TrimSpace(dsn) == strings.TrimSpace(matches[0]) { // Get the variable name varName := matches[0][2 : len(matches[0])-1] - value, exists := os.LookupEnv(varName) + value, exists := lookup(varName) if !exists { return "", fmt.Errorf("environment variable %s is not set", varName) } @@ -327,8 +355,9 @@ func expandDSN(dsn string) (string, error) { // ResolveDatabaseName returns the database opts would connect to, merging the DSN // path with the structured field. Empty when neither supplies one. func ResolveDatabaseName(opts ConnectOptions) string { + lookup := opts.resolveLookup() if opts.Database != "" { - if expanded, err := expandValue(opts.Database); err == nil && expanded != "" { + if expanded, err := expandValue(opts.Database, lookup); err == nil && expanded != "" { return expanded } } @@ -444,9 +473,10 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { parsedUrl *url.URL err error ) + lookup := opts.resolveLookup() if opts.DSN != "" { - populatedDSN, err := expandDSN(opts.DSN) + populatedDSN, err := expandDSN(opts.DSN, lookup) if err != nil { return nil, err } @@ -458,7 +488,7 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { parsedUrl = &url.URL{} } - scheme, err := expandValue(opts.Scheme) + scheme, err := expandValue(opts.Scheme, lookup) if err != nil { return nil, err } @@ -466,12 +496,12 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { parsedUrl.Scheme = scheme } - host, err := expandValue(opts.Host) + host, err := expandValue(opts.Host, lookup) if err != nil { return nil, err } - port, err := expandValue(opts.Port) + port, err := expandValue(opts.Port, lookup) if err != nil { return nil, err } @@ -510,7 +540,7 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { return nil, fmt.Errorf("port provided without host") } - databaseName, err := expandValue(opts.Database) + databaseName, err := expandValue(opts.Database, lookup) if err != nil { return nil, err } @@ -522,12 +552,12 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { } } - user, err := expandValue(opts.User) + user, err := expandValue(opts.User, lookup) if err != nil { return nil, err } - password, err := expandValue(opts.Password) + password, err := expandValue(opts.Password, lookup) if err != nil { return nil, err } @@ -546,11 +576,11 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { values = url.Values{} } for k, v := range opts.Params { - key, err := expandValue(k) + key, err := expandValue(k, lookup) if err != nil { return nil, err } - value, err := expandValue(v) + value, err := expandValue(v, lookup) if err != nil { return nil, err } @@ -566,12 +596,12 @@ func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { return parsedUrl, nil } -func expandValue(s string) (string, error) { +func expandValue(s string, lookup LookupFunc) (string, error) { if s == "" { return s, nil } if DSNREnvRegex.MatchString(s) { - return updateFromEnv(s) + return updateFromLookup(s, lookup) } return s, nil } diff --git a/pkg/database/database_test.go b/pkg/database/database_test.go index ea8eba8f..2e2fab9b 100644 --- a/pkg/database/database_test.go +++ b/pkg/database/database_test.go @@ -548,7 +548,7 @@ func Test_expandDSN(t *testing.T) { t.Setenv(k, v) } - got, err := expandDSN(tt.dsn) + got, err := expandDSN(tt.dsn, nil) if (err != nil) != tt.wantErr { t.Errorf("expandDSN() error = %v, wantErr %v", err, tt.wantErr) return @@ -631,7 +631,7 @@ func Test_extractPlaceholders(t *testing.T) { t.Setenv(k, v) } - gotSentinel, gotMapping, err := extractPlaceholders(tt.input) + gotSentinel, gotMapping, err := extractPlaceholders(tt.input, nil) if (err != nil) != tt.wantErr { t.Errorf("extractPlaceholders() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/database/lookup_test.go b/pkg/database/lookup_test.go new file mode 100644 index 00000000..07c0d9c3 --- /dev/null +++ b/pkg/database/lookup_test.go @@ -0,0 +1,111 @@ +package database + +import ( + "fmt" + "strings" + "sync" + "testing" +) + +func mapLookup(m map[string]string) LookupFunc { + return func(key string) (string, bool) { + v, ok := m[key] + return v, ok + } +} + +func TestLookupFunc_NoCrossContamination(t *testing.T) { + const n = 50 + var wg sync.WaitGroup + errs := make(chan error, n*2) + + for i := 0; i < n; i++ { + wg.Add(2) + go func(i int) { + defer wg.Done() + host := fmt.Sprintf("host-a-%d", i) + opts := ConnectOptions{ + Scheme: "postgres", + Host: "${DB_HOST}", + Database: "db", + User: "u", + Password: "p", + Lookup: mapLookup(map[string]string{"DB_HOST": host}), + } + u, err := buildConnectionURL(opts) + if err != nil { + errs <- err + return + } + if u.Hostname() != host { + errs <- fmt.Errorf("expected host %q, got %q", host, u.Hostname()) + } + }(i) + go func(i int) { + defer wg.Done() + host := fmt.Sprintf("host-b-%d", i) + opts := ConnectOptions{ + Scheme: "postgres", + Host: "${DB_HOST}", + Database: "db", + User: "u", + Password: "p", + Lookup: mapLookup(map[string]string{"DB_HOST": host}), + } + u, err := buildConnectionURL(opts) + if err != nil { + errs <- err + return + } + if u.Hostname() != host { + errs <- fmt.Errorf("expected host %q, got %q", host, u.Hostname()) + } + }(i) + } + wg.Wait() + close(errs) + for err := range errs { + t.Error(err) + } +} + +func TestLookupFunc_MissingKeyNamesKeyNotValue(t *testing.T) { + opts := ConnectOptions{ + Scheme: "postgres", + Host: "${SECRET_HOST}", + Database: "db", + Lookup: mapLookup(map[string]string{ + // intentionally missing SECRET_HOST + }), + } + _, err := buildConnectionURL(opts) + if err == nil { + t.Fatal("expected missing key error") + } + msg := err.Error() + if !strings.Contains(msg, "SECRET_HOST") { + t.Errorf("error should name the missing key: %v", err) + } + // Ensure no accidental secret material from other keys could appear + if strings.Contains(msg, "super-secret-password") { + t.Errorf("error leaked secret value: %v", err) + } +} + +func TestLookupFunc_ResolvesWithoutProcessEnv(t *testing.T) { + // Ensure process env does not supply the value. + t.Setenv("DB_HOST", "from-env") + opts := ConnectOptions{ + Scheme: "postgres", + Host: "${DB_HOST}", + Database: "db", + Lookup: mapLookup(map[string]string{"DB_HOST": "from-lookup"}), + } + u, err := buildConnectionURL(opts) + if err != nil { + t.Fatal(err) + } + if u.Hostname() != "from-lookup" { + t.Fatalf("expected from-lookup, got %q", u.Hostname()) + } +} From 65e311fc92c7c2bad802e8281f484f9673476fb7 Mon Sep 17 00:00:00 2001 From: Bjorn Tipling Date: Sat, 25 Jul 2026 22:42:55 +0000 Subject: [PATCH 2/3] fix: golden Postgres fixture defaults sslmode to require C1 SaaS / public-internet customer DBs should use TLS by default. disable remains documented only for non-TLS local lab DBs. ds-kjkw Co-authored-by: c1-squire-dev[bot] --- examples/c1-golden-postgres.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/examples/c1-golden-postgres.yml b/examples/c1-golden-postgres.yml index 8c4d81eb..78f14586 100644 --- a/examples/c1-golden-postgres.yml +++ b/examples/c1-golden-postgres.yml @@ -17,7 +17,9 @@ connect: user: ${DB_USER} password: ${DB_PASSWORD} params: - sslmode: disable + # require: default for C1 SaaS / public-internet customer DBs (TLS). + # disable: only for non-TLS local lab DBs — not the golden path. + sslmode: require resource_types: user: From 8f181e1386d928dbb11aabf5a6f84714c88030e2 Mon Sep 17 00:00:00 2001 From: Bjorn Tipling Date: Sat, 25 Jul 2026 22:47:25 +0000 Subject: [PATCH 3/3] fix: golden map.id uses stable u.id not username Resource renames would otherwise surface as delete+create and break grant continuity. Keep display_name/login on .username. Co-authored-by: c1-squire-dev[bot] --- examples/c1-golden-postgres.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/c1-golden-postgres.yml b/examples/c1-golden-postgres.yml index 78f14586..ee0272dd 100644 --- a/examples/c1-golden-postgres.yml +++ b/examples/c1-golden-postgres.yml @@ -37,7 +37,8 @@ resource_types: strategy: offset primary_key: id map: - id: ".username" + # Resource id must be a stable immutable PK (not a renameable login). + id: ".id" display_name: ".username" description: ".email" traits: