From f1445f717cb54c0f5f1f81b49598f375f102d82b Mon Sep 17 00:00:00 2001 From: Gabrielle Poncey Date: Wed, 17 Jun 2026 10:04:15 -0700 Subject: [PATCH 1/4] feat: add validation for shared_preload_libraries to ensure "spock" is included --- server/internal/api/apiv1/validate.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/server/internal/api/apiv1/validate.go b/server/internal/api/apiv1/validate.go index 947d94bc..99fc8084 100644 --- a/server/internal/api/apiv1/validate.go +++ b/server/internal/api/apiv1/validate.go @@ -41,6 +41,23 @@ func validateAuthFileGUCs(conf map[string]any, path validation.Path) []error { return errs } +func validateConfLibraries(conf map[string]any, path validation.Path) []error { + val, ok := utils.TypedFromMap[string](conf, "shared_preload_libraries") + + if !ok { + return nil //defaults will be applied, including spock + } + for _, lib := range strings.Split(val, ",") { + if strings.TrimSpace(lib) == "spock" { + return nil + } + } + + err := errors.New(`"spock" must be included in shared_preload_libraries`) + return []error{validation.NewError(err, path.AppendMapKey("shared_preload_libraries"))} + +} + // validatePgHbaConf checks that every non-comment pg_hba_conf entry parses. // Blank and comment lines are allowed and skipped. Validation is intentionally // minimal — see server/internal/postgres/hba/parse.go. @@ -134,8 +151,9 @@ func validateDatabaseSpec(orchestrator config.Orchestrator, databaseID string, s } // Reject postgresql_conf GUCs that would make user-supplied pg_hba/pg_ident - // entries ineffective, then validate the entries themselves. + // entries ineffective, check spock is present, then validate the entries themselves. errs = append(errs, validateAuthFileGUCs(spec.PostgresqlConf, validation.NewPath("postgresql_conf"))...) + errs = append(errs, validateConfLibraries(spec.PostgresqlConf, validation.NewPath("postgresql_conf"))...) errs = append(errs, validatePgHbaConf(spec.PgHbaConf, validation.NewPath("pg_hba_conf"))...) errs = append(errs, validatePgIdentConf(spec.PgIdentConf, validation.NewPath("pg_ident_conf"))...) @@ -281,6 +299,7 @@ func validateNode( } errs = append(errs, validateAuthFileGUCs(node.PostgresqlConf, path.Append("postgresql_conf"))...) + errs = append(errs, validateConfLibraries(node.PostgresqlConf, validation.NewPath("postgresql_conf"))...) errs = append(errs, validatePgHbaConf(node.PgHbaConf, path.Append("pg_hba_conf"))...) errs = append(errs, validatePgIdentConf(node.PgIdentConf, path.Append("pg_ident_conf"))...) From b01cd1bfe767faf764aca6a9a90c33cf46576eec Mon Sep 17 00:00:00 2001 From: Gabrielle Poncey Date: Wed, 17 Jun 2026 11:08:08 -0700 Subject: [PATCH 2/4] fix: typos, phrasing, path bug --- server/internal/api/apiv1/validate.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/server/internal/api/apiv1/validate.go b/server/internal/api/apiv1/validate.go index 99fc8084..1318ca7a 100644 --- a/server/internal/api/apiv1/validate.go +++ b/server/internal/api/apiv1/validate.go @@ -43,19 +43,16 @@ func validateAuthFileGUCs(conf map[string]any, path validation.Path) []error { func validateConfLibraries(conf map[string]any, path validation.Path) []error { val, ok := utils.TypedFromMap[string](conf, "shared_preload_libraries") - if !ok { - return nil //defaults will be applied, including spock + return nil // defaults will be applied, including spock } for _, lib := range strings.Split(val, ",") { if strings.TrimSpace(lib) == "spock" { return nil } } - err := errors.New(`"spock" must be included in shared_preload_libraries`) return []error{validation.NewError(err, path.AppendMapKey("shared_preload_libraries"))} - } // validatePgHbaConf checks that every non-comment pg_hba_conf entry parses. @@ -151,7 +148,7 @@ func validateDatabaseSpec(orchestrator config.Orchestrator, databaseID string, s } // Reject postgresql_conf GUCs that would make user-supplied pg_hba/pg_ident - // entries ineffective, check spock is present, then validate the entries themselves. + // entries ineffective or remove spock, then validate the entries themselves. errs = append(errs, validateAuthFileGUCs(spec.PostgresqlConf, validation.NewPath("postgresql_conf"))...) errs = append(errs, validateConfLibraries(spec.PostgresqlConf, validation.NewPath("postgresql_conf"))...) errs = append(errs, validatePgHbaConf(spec.PgHbaConf, validation.NewPath("pg_hba_conf"))...) @@ -299,7 +296,7 @@ func validateNode( } errs = append(errs, validateAuthFileGUCs(node.PostgresqlConf, path.Append("postgresql_conf"))...) - errs = append(errs, validateConfLibraries(node.PostgresqlConf, validation.NewPath("postgresql_conf"))...) + errs = append(errs, validateConfLibraries(node.PostgresqlConf, path.Append("postgresql_conf"))...) errs = append(errs, validatePgHbaConf(node.PgHbaConf, path.Append("pg_hba_conf"))...) errs = append(errs, validatePgIdentConf(node.PgIdentConf, path.Append("pg_ident_conf"))...) From 8f3d4285ae93efb3ea3fbf189d6b3c3e7276a1e9 Mon Sep 17 00:00:00 2001 From: Gabrielle Poncey Date: Wed, 17 Jun 2026 11:19:51 -0700 Subject: [PATCH 3/4] fix: case sensitivity issue, formatting issue --- server/internal/api/apiv1/validate.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/server/internal/api/apiv1/validate.go b/server/internal/api/apiv1/validate.go index 1318ca7a..f9865b05 100644 --- a/server/internal/api/apiv1/validate.go +++ b/server/internal/api/apiv1/validate.go @@ -42,17 +42,24 @@ func validateAuthFileGUCs(conf map[string]any, path validation.Path) []error { } func validateConfLibraries(conf map[string]any, path validation.Path) []error { - val, ok := utils.TypedFromMap[string](conf, "shared_preload_libraries") - if !ok { - return nil // defaults will be applied, including spock - } - for _, lib := range strings.Split(val, ",") { - if strings.TrimSpace(lib) == "spock" { - return nil + for key, val := range conf { + if strings.ToLower(strings.TrimSpace(key)) != "shared_preload_libraries" { + continue + } + strVal, ok := val.(string) + if !ok { + err := fmt.Errorf("%q must be a string", key) + return []error{validation.NewError(err, path.AppendMapKey(key))} + } + for _, lib := range strings.Split(strVal, ",") { + if strings.TrimSpace(lib) == "spock" { + return nil + } } + err := errors.New(`"spock" must be included in shared_preload_libraries`) + return []error{validation.NewError(err, path.AppendMapKey(key))} } - err := errors.New(`"spock" must be included in shared_preload_libraries`) - return []error{validation.NewError(err, path.AppendMapKey("shared_preload_libraries"))} + return nil // key absent; defaults will be applied, including spock } // validatePgHbaConf checks that every non-comment pg_hba_conf entry parses. From 57a8a5a1ae2ec4095b37727b4aa8a2c8d26583ab Mon Sep 17 00:00:00 2001 From: Gabrielle Poncey Date: Tue, 23 Jun 2026 09:33:41 -0700 Subject: [PATCH 4/4] fix: test coverage, update db validation --- server/internal/api/apiv1/validate.go | 9 ++ server/internal/api/apiv1/validate_test.go | 117 +++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/server/internal/api/apiv1/validate.go b/server/internal/api/apiv1/validate.go index f9865b05..78a040ba 100644 --- a/server/internal/api/apiv1/validate.go +++ b/server/internal/api/apiv1/validate.go @@ -42,6 +42,7 @@ func validateAuthFileGUCs(conf map[string]any, path validation.Path) []error { } func validateConfLibraries(conf map[string]any, path validation.Path) []error { + // param names are case-insensitive, so comparison is done after casting to lowercase for key, val := range conf { if strings.ToLower(strings.TrimSpace(key)) != "shared_preload_libraries" { continue @@ -256,6 +257,14 @@ func validateDatabaseUpdate(old *database.Spec, new *api.DatabaseSpec) error { errs = append(errs, validateServiceSpec(svc, svcPath, isExistingService, old.DatabaseID, new.DatabaseUsers, newNodeNames)...) } + // Validate that shared_preload_libraries, if explicitly set, still includes + // "spock" — both at the spec level and on individual nodes. + errs = append(errs, validateConfLibraries(new.PostgresqlConf, validation.NewPath("postgresql_conf"))...) + for i, n := range new.Nodes { + nodePath := validation.NewPath("nodes", validation.ArrayIndexElement(i)) + errs = append(errs, validateConfLibraries(n.PostgresqlConf, nodePath.Append("postgresql_conf"))...) + } + return errors.Join(errs...) } diff --git a/server/internal/api/apiv1/validate_test.go b/server/internal/api/apiv1/validate_test.go index b43aa701..5f86959b 100644 --- a/server/internal/api/apiv1/validate_test.go +++ b/server/internal/api/apiv1/validate_test.go @@ -451,6 +451,92 @@ func TestValidateBackupConfig(t *testing.T) { } } +func TestValidateDatabaseUpdate_LibraryConf(t *testing.T) { + for _, tc := range []struct { + name string + old *database.Spec + new *api.DatabaseSpec + expected []string + }{ + { + name: "absent valid", + old: &database.Spec{}, + new: &api.DatabaseSpec{}, + }, + { + name: "invalid spec-level conf missing spock", + old: &database.Spec{}, + new: &api.DatabaseSpec{ + PostgresqlConf: map[string]any{"shared_preload_libraries": "pg_stat_statements"}, + }, + expected: []string{`"spock" must be included in shared_preload_libraries`}, + }, + { + name: "invalid node-level conf missing spock", + old: &database.Spec{}, + new: &api.DatabaseSpec{ + Nodes: []*api.DatabaseNodeSpec{ + {PostgresqlConf: map[string]any{"shared_preload_libraries": "pg_stat_statements"}}, + }, + }, + expected: []string{`"spock" must be included in shared_preload_libraries`}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := validateDatabaseUpdate(tc.old, tc.new) + if len(tc.expected) < 1 { + assert.NoError(t, err) + } else { + for _, expected := range tc.expected { + assert.ErrorContains(t, err, expected) + } + } + }) + } +} + +func TestValidateConfLibraries(t *testing.T) { + for _, tc := range []struct { + name string + conf map[string]any + expected []string + }{ + { + name: "absent valid", + }, + { + name: "valid spock conf", + conf: map[string]any{"shared_preload_libraries": "snowflake, spock"}, + }, + { + name: "valid uppercase param name", + conf: map[string]any{"SHARED_PRELOAD_LIBRARIES": "snowflake, spock"}, + }, + { + name: "valid conf spock and custom", + conf: map[string]any{"shared_preload_libraries": "snowflake, spock, custom_lib"}, + }, + { + name: "invalid conf missing spock", + conf: map[string]any{"shared_preload_libraries": "snowflake"}, + expected: []string{ + `"spock" must be included in shared_preload_libraries`, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := errors.Join(validateConfLibraries(tc.conf, nil)...) + if len(tc.expected) < 1 { + assert.NoError(t, err) + } else { + for _, expected := range tc.expected { + assert.ErrorContains(t, err, expected) + } + } + }) + } +} + func TestValidateNode(t *testing.T) { for _, tc := range []struct { name string @@ -574,6 +660,20 @@ func TestValidateNode(t *testing.T) { "restore_config.repository.base_path: base_path must be absolute for posix repositories", }, }, + { + name: "invalid lib conf", + orchestrator: config.OrchestratorSwarm, + db: &api.DatabaseSpec{}, + node: &api.DatabaseNodeSpec{ + HostIds: []api.Identifier{ + api.Identifier("host-1"), + }, + PostgresqlConf: map[string]any{"shared_preload_libraries": "snowflake"}, + }, + expected: []string{ + `"spock" must be included in shared_preload_libraries`, + }, + }, } { t.Run(tc.name, func(t *testing.T) { err := errors.Join(validateNode(tc.orchestrator, tc.db, tc.node, nil)...) @@ -1307,6 +1407,23 @@ func TestValidateDatabaseSpec(t *testing.T) { `nodes[0].postgresql_conf[HBA_FILE]: "HBA_FILE" is not allowed`, }, }, + { + name: "invalid lib conf", + spec: &api.DatabaseSpec{ + Nodes: []*api.DatabaseNodeSpec{ + { + Name: "n1", + HostIds: []api.Identifier{ + api.Identifier("host-1"), + }, + PostgresqlConf: map[string]any{"shared_preload_libraries": "snowflake"}, + }, + }, + }, + expected: []string{ + `"spock" must be included in shared_preload_libraries`, + }, + }, } { t.Run(tc.name, func(t *testing.T) { err := validateDatabaseSpec(config.OrchestratorSwarm, "test-db", tc.spec)