From 828df028f3ac8dd0ade29958f8126d4706a52e48 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Fri, 24 Jul 2026 15:58:38 +0200 Subject: [PATCH] Guard Postgres service/target column lists against gorm drift --- management/server/store/sql_store.go | 29 +++++--- .../server/store/sql_store_pgx_parity_test.go | 74 +++++++++++++++++++ 2 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 management/server/store/sql_store_pgx_parity_test.go diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index c2125e75b..186eb9c96 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -2258,13 +2258,23 @@ func (s *SqlStore) getPostureChecks(ctx context.Context, accountID string) ([]*p return checks, nil } +// serviceSelectColumns and targetSelectColumns are the column lists the Postgres +// pgx read path scans. They must stay in sync with the rpservice.Service and +// rpservice.Target gorm models; TestPgxServiceColumnsMatchGorm enforces this. +const serviceSelectColumns = `id, account_id, name, domain, enabled, auth, restrictions, + meta_created_at, meta_certificate_issued_at, meta_last_renewed_at, meta_status, proxy_cluster, + pass_host_header, rewrite_redirects, session_private_key, session_public_key, + mode, listen_port, port_auto_assigned, source, source_peer, terminated, + private, access_groups` + +const targetSelectColumns = `id, account_id, service_id, path, host, port, protocol, + target_id, target_type, enabled, proxy_protocol, + skip_tls_verify, request_timeout, session_idle_timeout, path_rewrite, custom_headers, + direct_upstream, middlewares, capture_max_request_bytes, capture_max_response_bytes, + capture_content_types, agent_network, disable_access_log` + func (s *SqlStore) getServices(ctx context.Context, accountID string) ([]*rpservice.Service, error) { - const serviceQuery = `SELECT id, account_id, name, domain, enabled, auth, restrictions, - meta_created_at, meta_certificate_issued_at, meta_last_renewed_at, meta_status, proxy_cluster, - pass_host_header, rewrite_redirects, session_private_key, session_public_key, - mode, listen_port, port_auto_assigned, source, source_peer, terminated, - private, access_groups - FROM services WHERE account_id = $1` + const serviceQuery = `SELECT ` + serviceSelectColumns + ` FROM services WHERE account_id = $1` serviceRows, err := s.pool.Query(ctx, serviceQuery, accountID) if err != nil { @@ -2410,12 +2420,7 @@ func scanService(row pgx.CollectableRow) (*rpservice.Service, error) { } func (s *SqlStore) getServiceTargets(ctx context.Context, serviceIDs []string) ([]*rpservice.Target, error) { - const targetsQuery = `SELECT id, account_id, service_id, path, host, port, protocol, - target_id, target_type, enabled, proxy_protocol, - skip_tls_verify, request_timeout, session_idle_timeout, path_rewrite, custom_headers, - direct_upstream, middlewares, capture_max_request_bytes, capture_max_response_bytes, - capture_content_types, agent_network, disable_access_log - FROM targets WHERE service_id = ANY($1)` + const targetsQuery = `SELECT ` + targetSelectColumns + ` FROM targets WHERE service_id = ANY($1)` rows, err := s.pool.Query(ctx, targetsQuery, serviceIDs) if err != nil { diff --git a/management/server/store/sql_store_pgx_parity_test.go b/management/server/store/sql_store_pgx_parity_test.go new file mode 100644 index 000000000..1f17817d0 --- /dev/null +++ b/management/server/store/sql_store_pgx_parity_test.go @@ -0,0 +1,74 @@ +package store + +import ( + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm/schema" + + rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service" +) + +// TestPgxServiceColumnsMatchGorm guards the Postgres pgx read path against +// drifting from the gorm model. The SQLite/MySQL gorm path loads rows by struct, +// so a new column on a model is picked up automatically, but the hand-written +// pgx SELECT in sql_store.go must be updated by hand. This test fails when a +// gorm column is missing from the pgx column list, which otherwise silently +// returns zero-valued on Postgres with no compile error. +func TestPgxServiceColumnsMatchGorm(t *testing.T) { + tests := []struct { + name string + model any + selectColumns string + // excluded lists gorm columns intentionally not loaded by the pgx path. + excluded map[string]struct{} + }{ + { + name: "service", + model: &rpservice.Service{}, + selectColumns: serviceSelectColumns, + }, + { + name: "target", + model: &rpservice.Target{}, + selectColumns: targetSelectColumns, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + selected := parseColumnList(tc.selectColumns) + for _, col := range gormColumnNames(t, tc.model) { + if _, ok := tc.excluded[col]; ok { + continue + } + _, ok := selected[col] + assert.Truef(t, ok, + "gorm column %q is not read by the Postgres pgx SELECT; add it to %sSelectColumns in sql_store.go (or to the test's excluded set if it is intentionally not loaded)", + col, tc.name) + } + }) + } +} + +func parseColumnList(cols string) map[string]struct{} { + set := make(map[string]struct{}) + for _, c := range strings.Split(cols, ",") { + if c = strings.TrimSpace(c); c != "" { + set[c] = struct{}{} + } + } + return set +} + +// gormColumnNames returns the DB column names gorm would migrate for the model, +// using the same default naming strategy the store configures. +func gormColumnNames(t *testing.T, model any) []string { + t.Helper() + sch, err := schema.Parse(model, &sync.Map{}, schema.NamingStrategy{}) + require.NoError(t, err) + return sch.DBNames +}