fix(esa-1866): fix SQL injection and apply role to all hosts

- Quote role/schema/table identifiers with pq.QuoteIdentifier
- Validate privilege keywords against an allowlist to prevent injection
  via unquotable SQL keywords
- Loop over all HostCredentials instead of a single spec-referenced host
- Add customRoleRequeueStrategy with correct log messages

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: tmablunar

internal/controller/customrole_controller.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/go-logr/logr"
 	"github.com/google/uuid"
@@ -33,7 +34,6 @@ import (
 
 	postgresqlv1alpha1 "go.lunarway.com/postgresql-controller/api/v1alpha1"
 	ctlerrors "go.lunarway.com/postgresql-controller/pkg/errors"
-	"go.lunarway.com/postgresql-controller/pkg/kube"
 	"go.lunarway.com/postgresql-controller/pkg/postgres"
 )
 
@@ -61,7 +61,7 @@ func (r *CustomRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	reqLogger = reqLogger.WithValues("requestId", requestID.String())
 
 	err = r.reconcile(ctx, reqLogger, req)
-	return requeueStrategy(reqLogger, err)
+	return customRoleRequeueStrategy(reqLogger, err)
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -109,53 +109,55 @@ func (r *CustomRoleReconciler) reconcile(ctx context.Context, reqLogger logr.Log
 	reqLogger = reqLogger.WithValues("roleName", customRole.Spec.RoleName)
 	reqLogger.V(1).Info("Reconciling CustomRole resource")
 
-	host, adminCredentials, err := r.resolveHostCredentials(ctx, req.Namespace, customRole.Spec.HostCredentials)
-	if err != nil {
-		r.persistStatus(ctx, customRole, err)
-		return fmt.Errorf("resolve host credentials: %w", err)
+	grants := toPostgresGrants(customRole.Spec.Grants)
+
+	for host, creds := range r.HostCredentials {
+		if err := r.reconcileOnHost(reqLogger, host, creds, customRole.Spec.RoleName, customRole.Spec.GrantRoles, grants); err != nil {
+			r.persistStatus(ctx, customRole, err)
+			return fmt.Errorf("reconcile on host %s: %w", host, err)
+		}
 	}
-	reqLogger = reqLogger.WithValues("host", host)
+
+	r.persistStatus(ctx, customRole, nil)
+	return nil
+}
+
+func (r *CustomRoleReconciler) reconcileOnHost(log logr.Logger, host string, creds postgres.Credentials, roleName string, grantRoles []string, grants []postgres.CustomRoleGrant) error {
+	log = log.WithValues("host", host)
 
 	// Connect to the postgres maintenance database for server-level operations.
 	adminConnStr := postgres.ConnectionString{
 		Host:     host,
 		Database: "postgres",
-		User:     adminCredentials.User,
-		Password: adminCredentials.Password,
-		Params:   adminCredentials.Params,
+		User:     creds.User,
+		Password: creds.Password,
+		Params:   creds.Params,
 	}
-	adminDB, err := postgres.Connect(reqLogger, adminConnStr)
+	adminDB, err := postgres.Connect(log, adminConnStr)
 	if err != nil {
-		r.persistStatus(ctx, customRole, err)
 		return fmt.Errorf("connect to host: %w", err)
 	}
 	defer adminDB.Close()
 
 	// Create the role and apply server-level role grants.
-	if err := postgres.EnsureCustomRole(reqLogger, adminDB, customRole.Spec.RoleName, customRole.Spec.GrantRoles); err != nil {
-		r.persistStatus(ctx, customRole, err)
+	if err := postgres.EnsureCustomRole(log, adminDB, roleName, grantRoles); err != nil {
 		return fmt.Errorf("ensure role: %w", err)
 	}
 
-	// Apply per-database grants across all user databases.
-	if len(customRole.Spec.Grants) > 0 {
+	// Apply per-database grants across all user databases on this host.
+	if len(grants) > 0 {
 		databases, err := postgres.UserDatabases(adminDB)
 		if err != nil {
-			r.persistStatus(ctx, customRole, err)
 			return fmt.Errorf("list databases: %w", err)
 		}
 
-		grants := toPostgresGrants(customRole.Spec.Grants)
-
 		for _, dbName := range databases {
-			if err := r.applyGrantsOnDatabase(reqLogger, host, *adminCredentials, customRole.Spec.RoleName, dbName, grants); err != nil {
-				r.persistStatus(ctx, customRole, err)
+			if err := r.applyGrantsOnDatabase(log, host, creds, roleName, dbName, grants); err != nil {
 				return fmt.Errorf("apply grants on database %s: %w", dbName, err)
 			}
 		}
 	}
 
-	r.persistStatus(ctx, customRole, nil)
 	return nil
 }
 
@@ -192,39 +194,6 @@ func (r *CustomRoleReconciler) applyGrantsOnDatabase(log logr.Logger, host strin
 	return postgres.ApplyDatabaseGrants(log, db, roleName, grants)
 }
 
-func (r *CustomRoleReconciler) resolveHostCredentials(ctx context.Context, namespace, hostCredentialsName string) (string, *postgres.Credentials, error) {
-	var hostCreds postgresqlv1alpha1.PostgreSQLHostCredentials
-	err := r.Client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: hostCredentialsName}, &hostCreds)
-	if err != nil {
-		if apierrors.IsNotFound(err) {
-			return "", nil, ctlerrors.NewTemporary(fmt.Errorf("PostgreSQLHostCredentials %s/%s not found", namespace, hostCredentialsName))
-		}
-		return "", nil, fmt.Errorf("get PostgreSQLHostCredentials %s/%s: %w", namespace, hostCredentialsName, err)
-	}
-
-	user, err := kube.ResourceValue(r.Client, hostCreds.Spec.User, hostCreds.Namespace)
-	if err != nil {
-		return "", nil, fmt.Errorf("resolve user: %w", err)
-	}
-
-	password, err := kube.ResourceValue(r.Client, hostCreds.Spec.Password, hostCreds.Namespace)
-	if err != nil {
-		return "", nil, fmt.Errorf("resolve password: %w", err)
-	}
-
-	host, err := kube.ResourceValue(r.Client, hostCreds.Spec.Host, hostCreds.Namespace)
-	if err != nil {
-		return "", nil, fmt.Errorf("resolve host: %w", err)
-	}
-
-	return host, &postgres.Credentials{
-		Name:     user,
-		User:     user,
-		Password: password,
-		Params:   hostCreds.Spec.Params,
-	}, nil
-}
-
 func (r *CustomRoleReconciler) persistStatus(ctx context.Context, customRole *postgresqlv1alpha1.CustomRole, reconcileErr error) {
 	var phase postgresqlv1alpha1.CustomRolePhase
 	var errorMessage string
@@ -252,3 +221,22 @@ func (r *CustomRoleReconciler) persistStatus(ctx context.Context, customRole *po
 		r.Log.Error(err, "failed to update CustomRole status")
 	}
 }
+
+func customRoleRequeueStrategy(log logr.Logger, err error) (ctrl.Result, error) {
+	if err == nil {
+		return ctrl.Result{}, nil
+	}
+
+	if ctlerrors.IsInvalid(err) {
+		log.Info("Dropping CustomRole from queue as it is invalid", "error", err)
+		return reconcile.Result{}, nil
+	}
+
+	if ctlerrors.IsTemporary(err) {
+		log.Info("Failed to reconcile CustomRole object, attempting again shortly", "error", err)
+		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+	}
+
+	log.Info("Failed to reconcile CustomRole object due to unknown error", "error", err)
+	return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+}

pkg/postgres/customrole.go

@@ -19,13 +19,36 @@ type CustomRoleGrant struct {
 	Privileges []string
 }
 
+// allowedTablePrivileges is the set of valid PostgreSQL table-level privilege keywords.
+var allowedTablePrivileges = map[string]struct{}{
+	"SELECT":     {},
+	"INSERT":     {},
+	"UPDATE":     {},
+	"DELETE":     {},
+	"TRUNCATE":   {},
+	"REFERENCES": {},
+	"TRIGGER":    {},
+	"ALL":        {},
+}
+
+// validatePrivileges returns an error if any value in privs is not a recognised
+// PostgreSQL table-level privilege keyword. Comparison is case-insensitive.
+func validatePrivileges(privs []string) error {
+	for _, p := range privs {
+		if _, ok := allowedTablePrivileges[strings.ToUpper(p)]; !ok {
+			return fmt.Errorf("invalid privilege %q: must be one of SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER, ALL", p)
+		}
+	}
+	return nil
+}
+
 // EnsureCustomRole creates a PostgreSQL role if it does not exist and applies
 // server-level role grants. The role is created with NOLOGIN.
 func EnsureCustomRole(log logr.Logger, db *sql.DB, roleName string, grantRoles []string) error {
 	log = log.WithValues("role", roleName)
 	log.V(1).Info("Ensuring custom role")
 
-	_, err := db.Exec(fmt.Sprintf("CREATE ROLE %s NOLOGIN", roleName))
+	_, err := db.Exec(fmt.Sprintf("CREATE ROLE %s NOLOGIN", pq.QuoteIdentifier(roleName)))
 	if err != nil {
 		pqError, ok := err.(*pq.Error)
 		if !ok || pqError.Code.Name() != "duplicate_object" {
@@ -40,10 +63,14 @@ func EnsureCustomRole(log logr.Logger, db *sql.DB, roleName string, grantRoles [
 		return nil
 	}
 
-	joined := strings.Join(grantRoles, ", ")
-	_, err = db.Exec(fmt.Sprintf("GRANT %s TO %s", joined, roleName))
+	quotedRoles := make([]string, len(grantRoles))
+	for i, r := range grantRoles {
+		quotedRoles[i] = pq.QuoteIdentifier(r)
+	}
+	joined := strings.Join(quotedRoles, ", ")
+	_, err = db.Exec(fmt.Sprintf("GRANT %s TO %s", joined, pq.QuoteIdentifier(roleName)))
 	if err != nil {
-		return fmt.Errorf("grant roles %s to %s: %w", joined, roleName, err)
+		return fmt.Errorf("grant roles %s to %s: %w", strings.Join(grantRoles, ", "), roleName, err)
 	}
 	log.V(1).Info("Granted roles", "roles", grantRoles)
 
@@ -122,22 +149,36 @@ func resolveSchemas(db *sql.DB, schema string) ([]string, error) {
 }
 
 func applyPrivilegeGrant(log logr.Logger, db *sql.DB, roleName, schema, table string, privileges []string) error {
-	privs := strings.Join(privileges, ", ")
+	if err := validatePrivileges(privileges); err != nil {
+		return err
+	}
+
+	// Normalise keywords to uppercase for clarity; PostgreSQL is case-insensitive
+	// for keywords but this keeps generated SQL consistent.
+	upper := make([]string, len(privileges))
+	for i, p := range privileges {
+		upper[i] = strings.ToUpper(p)
+	}
+	privs := strings.Join(upper, ", ")
+
+	quotedRole := pq.QuoteIdentifier(roleName)
+	quotedSchema := pq.QuoteIdentifier(schema)
 
-	_, err := db.Exec(fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s", schema, roleName))
+	_, err := db.Exec(fmt.Sprintf("GRANT USAGE ON SCHEMA %s TO %s", quotedSchema, quotedRole))
 	if err != nil {
 		return fmt.Errorf("grant usage on schema %s to %s: %w", schema, roleName, err)
 	}
 	log.V(1).Info("Granted USAGE on schema", "schema", schema)
 
 	if table == "" || table == "*" {
-		_, err = db.Exec(fmt.Sprintf("GRANT %s ON ALL TABLES IN SCHEMA %s TO %s", privs, schema, roleName))
+		_, err = db.Exec(fmt.Sprintf("GRANT %s ON ALL TABLES IN SCHEMA %s TO %s", privs, quotedSchema, quotedRole))
 		if err != nil {
 			return fmt.Errorf("grant %s on all tables in schema %s to %s: %w", privs, schema, roleName, err)
 		}
 		log.V(1).Info("Granted privileges on all tables in schema", "privileges", privs, "schema", schema)
 	} else {
-		_, err = db.Exec(fmt.Sprintf("GRANT %s ON TABLE %s.%s TO %s", privs, schema, table, roleName))
+		quotedTable := pq.QuoteIdentifier(table)
+		_, err = db.Exec(fmt.Sprintf("GRANT %s ON TABLE %s.%s TO %s", privs, quotedSchema, quotedTable, quotedRole))
 		if err != nil {
 			return fmt.Errorf("grant %s on table %s.%s to %s: %w", privs, schema, table, roleName, err)
 		}