diff --git a/server/internal/database/service_instance.go b/server/internal/database/service_instance.go index 0fffc3d0..a5e8b0b2 100644 --- a/server/internal/database/service_instance.go +++ b/server/internal/database/service_instance.go @@ -58,60 +58,50 @@ type HealthCheckResult struct { // ServiceUser represents database credentials for a service instance. // -// Each service instance receives dedicated database credentials with read-only access. -// This provides security isolation between service instances and prevents services from -// modifying database data. This may be relaxed or configurable in the future depending -// on use-case requirements. +// Each service instance receives two dedicated database users: one read-only (RO) and +// one read-write (RW). The active user is selected based on the service's allow_writes +// setting. This provides security isolation between service instances. // // # Credential Generation // // Credentials are generated during service instance provisioning by the CreateServiceUser -// workflow activity. The username is deterministic (based on service instance ID), while -// the password is cryptographically random. +// workflow activity. The username is deterministic (based on service instance ID and +// mode), while the password is cryptographically random. // // # Security Properties // // - Unique per service instance (not shared between instances) -// - Read-only database access (SELECT + EXECUTE only, no DML/DDL) // - 32-character random passwords // - Storage in etcd alongside service instance metadata -// - Injected as environment variables into service containers -// -// # Usage -// -// Service containers receive credentials via environment variables: -// - PGUSER: Username (e.g., "svc_db1mcp") -// - PGPASSWORD: Password (32-character random string) -// -// The service connects to the database using these credentials, which are restricted -// to read-only operations via the "pgedge_application_read_only" role. +// - Injected into service containers via config.yaml type ServiceUser struct { - Username string `json:"username"` // Format: "svc_{first-8-chars-of-instance-id}" + Username string `json:"username"` // Format: "svc_{service_id}_{mode}" Password string `json:"password"` // 32-character cryptographically random string - Role string `json:"role"` // Database role, e.g., "pgedge_application_read_only" + Role string `json:"role"` // Database role, e.g., "pgedge_application_read_only" or "pgedge_application" } // GenerateServiceUsername creates a deterministic username for a service. // // # Username Format // -// The username follows the pattern: "svc_{service_id}" +// The username follows the pattern: "svc_{service_id}_{mode}" // // Example: // -// service_id: "mcp-server" -// Generated username: "svc_mcp_server" +// service_id: "mcp-server", mode: "ro" +// Generated username: "svc_mcp_server_ro" // // # Rationale // // - "svc_" prefix: Clearly identifies service accounts vs. application users // - service_id: Uniquely identifies the service within the database -// - Deterministic: Same service_id always generates the same username -// - Shared: One database user role per service, shared across all instances +// - mode: Distinguishes RO ("ro") from RW ("rw") users for the same service +// - Deterministic: Same service_id + mode always generates the same username +// - Shared: One database user role per service per mode, shared across all instances // // # Uniqueness // -// Service IDs are unique within a database. By using the service_id, we +// Service IDs are unique within a database. By using the service_id and mode, we // guarantee uniqueness even when multiple services exist on the same database. // // # PostgreSQL Compatibility @@ -122,13 +112,13 @@ type ServiceUser struct { // name) to a truncated prefix. This guarantees uniqueness even when two inputs // share a long common prefix. // -// Short name format: svc_{service_id} -// Long name format: svc_{first 50 chars of service_id}_{8-hex-hash} -func GenerateServiceUsername(serviceID string) string { +// Short name format: svc_{service_id}_{mode} +// Long name format: svc_{truncated service_id}_{8-hex-hash}_{mode} +func GenerateServiceUsername(serviceID string, mode string) string { // Sanitize hyphens to underscores for PostgreSQL compatibility. // Hyphens in identifiers require double-quoting in SQL. serviceID = strings.ReplaceAll(serviceID, "-", "_") - username := fmt.Sprintf("svc_%s", serviceID) + username := fmt.Sprintf("svc_%s_%s", serviceID, mode) if len(username) <= 63 { return username @@ -136,15 +126,17 @@ func GenerateServiceUsername(serviceID string) string { // Hash the full untruncated username for uniqueness h := sha256.Sum256([]byte(username)) - suffix := hex.EncodeToString(h[:4]) // 8 hex chars + hashSuffix := hex.EncodeToString(h[:4]) // 8 hex chars - // svc_ (4) + prefix (50) + _ (1) + hash (8) = 63 + // svc_ (4) + prefix + _ (1) + hash (8) + _ (1) + mode (len) = 14 + len(mode) + prefix + // Max prefix = 63 - 14 - len(mode) + maxPrefix := 63 - 14 - len(mode) raw := serviceID - if len(raw) > 50 { - raw = raw[:50] + if len(raw) > maxPrefix { + raw = raw[:maxPrefix] } - return fmt.Sprintf("svc_%s_%s", raw, suffix) + return fmt.Sprintf("svc_%s_%s_%s", raw, hashSuffix, mode) } // GenerateServiceInstanceID creates a unique ID for a service instance. diff --git a/server/internal/database/service_instance_test.go b/server/internal/database/service_instance_test.go index 721c7b89..f47fa0ab 100644 --- a/server/internal/database/service_instance_test.go +++ b/server/internal/database/service_instance_test.go @@ -8,53 +8,74 @@ func TestGenerateServiceUsername(t *testing.T) { tests := []struct { name string serviceID string + mode string want string }{ { - name: "standard service instance", + name: "standard service instance ro", serviceID: "mcp-server", - want: "svc_mcp_server", + mode: "ro", + want: "svc_mcp_server_ro", + }, + { + name: "standard service instance rw", + serviceID: "mcp-server", + mode: "rw", + want: "svc_mcp_server_rw", }, { name: "multiple services on same database - service 1", serviceID: "appmcp-1", - want: "svc_appmcp_1", + mode: "ro", + want: "svc_appmcp_1_ro", }, { name: "multiple services on same database - service 2", serviceID: "appmcp-2", - want: "svc_appmcp_2", + mode: "ro", + want: "svc_appmcp_2_ro", }, { name: "service with multi-part service ID", serviceID: "my-mcp-service", - want: "svc_my_mcp_service", + mode: "ro", + want: "svc_my_mcp_service_ro", }, { name: "simple service ID", serviceID: "mcp", - want: "svc_mcp", + mode: "ro", + want: "svc_mcp_ro", }, { name: "long service ID uses hash suffix", serviceID: "very-long-service-name-that-exceeds-postgres-limit-significantly", + mode: "ro", want: "", // computed below }, { name: "long names with shared prefix produce different usernames (case A)", serviceID: "very-long-service-name-that-exceeds-postgres-limit-AAA", + mode: "ro", want: "", // computed below }, { name: "long names with shared prefix produce different usernames (case B)", serviceID: "very-long-service-name-that-exceeds-postgres-limit-BBB", + mode: "ro", want: "", // computed below }, + { + name: "long service ID with rw mode still fits", + serviceID: "very-long-service-name-that-exceeds-postgres-limit-significantly", + mode: "rw", + want: "", // computed below, just check length + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := GenerateServiceUsername(tt.serviceID) + got := GenerateServiceUsername(tt.serviceID, tt.mode) if tt.want != "" { if got != tt.want { t.Errorf("GenerateServiceUsername() = %v, want %v", got, tt.want) @@ -70,11 +91,18 @@ func TestGenerateServiceUsername(t *testing.T) { } // Verify long names with shared prefix produce different usernames - a := GenerateServiceUsername("very-long-service-name-that-exceeds-postgres-limit-AAA") - b := GenerateServiceUsername("very-long-service-name-that-exceeds-postgres-limit-BBB") + a := GenerateServiceUsername("very-long-service-name-that-exceeds-postgres-limit-AAA", "ro") + b := GenerateServiceUsername("very-long-service-name-that-exceeds-postgres-limit-BBB", "ro") if a == b { t.Errorf("long names with shared prefix should produce different usernames, both got %v", a) } + + // Verify different modes produce different usernames for the same serviceID + roUser := GenerateServiceUsername("mcp-server", "ro") + rwUser := GenerateServiceUsername("mcp-server", "rw") + if roUser == rwUser { + t.Errorf("different modes should produce different usernames, both got %v", roUser) + } } func TestGenerateServiceInstanceID(t *testing.T) { diff --git a/server/internal/orchestrator/swarm/mcp_config_resource.go b/server/internal/orchestrator/swarm/mcp_config_resource.go index 138f4e85..aa05e3d6 100644 --- a/server/internal/orchestrator/swarm/mcp_config_resource.go +++ b/server/internal/orchestrator/swarm/mcp_config_resource.go @@ -41,19 +41,23 @@ type MCPConfigResource struct { DatabaseName string `json:"database_name"` DatabaseHosts []database.ServiceHostEntry `json:"database_hosts"` TargetSessionAttrs string `json:"target_session_attrs"` - Username string `json:"username"` - Password string `json:"password"` + ROUsername string `json:"ro_username"` + ROPassword string `json:"ro_password"` + RWUsername string `json:"rw_username"` + RWPassword string `json:"rw_password"` } func (r *MCPConfigResource) ResourceVersion() string { - return "1" + return "2" } func (r *MCPConfigResource) DiffIgnore() []string { return []string{ - // Credentials are populated from ServiceUserRole during refresh. - "/username", - "/password", + // Credentials are populated from ServiceUserRole resources during refresh. + "/ro_username", + "/ro_password", + "/rw_username", + "/rw_password", } } @@ -68,7 +72,8 @@ func (r *MCPConfigResource) Executor() resource.Executor { func (r *MCPConfigResource) Dependencies() []resource.Identifier { return []resource.Identifier{ filesystem.DirResourceIdentifier(r.DirResourceID), - ServiceUserRoleIdentifier(r.ServiceID), + ServiceUserRoleIdentifier(r.ServiceID, ServiceUserRoleRO), + ServiceUserRoleIdentifier(r.ServiceID, ServiceUserRoleRW), } } @@ -87,11 +92,6 @@ func (r *MCPConfigResource) Refresh(ctx context.Context, rc *resource.Context) e return fmt.Errorf("failed to get service data dir path: %w", err) } - // Populate credentials from ServiceUserRole - if err := r.populateCredentials(rc); err != nil { - return err - } - // Check if config.yaml exists _, err = readResourceFile(fs, filepath.Join(dirPath, "config.yaml")) if err != nil { @@ -168,15 +168,25 @@ func (r *MCPConfigResource) Delete(ctx context.Context, rc *resource.Context) er return nil } +// activeCredentials returns the username and password for the active service +// user based on the AllowWrites config setting. +func (r *MCPConfigResource) activeCredentials() (username, password string) { + if r.Config.AllowWrites != nil && *r.Config.AllowWrites { + return r.RWUsername, r.RWPassword + } + return r.ROUsername, r.ROPassword +} + // writeConfigFile generates and writes the config.yaml file. func (r *MCPConfigResource) writeConfigFile(fs afero.Fs, dirPath string) error { + username, password := r.activeCredentials() content, err := GenerateMCPConfig(&MCPConfigParams{ Config: r.Config, DatabaseName: r.DatabaseName, DatabaseHosts: r.DatabaseHosts, TargetSessionAttrs: r.TargetSessionAttrs, - Username: r.Username, - Password: r.Password, + Username: username, + Password: password, }) if err != nil { return fmt.Errorf("failed to generate MCP config: %w", err) @@ -257,13 +267,22 @@ func (r *MCPConfigResource) writeUserFileIfNeeded(fs afero.Fs, usersPath string) return nil } -// populateCredentials fetches the username/password from the ServiceUserRole resource. +// populateCredentials fetches credentials from both ServiceUserRole resources +// (RO and RW). Credential selection happens at usage time based on AllowWrites. func (r *MCPConfigResource) populateCredentials(rc *resource.Context) error { - userRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(r.ServiceID)) + roRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(r.ServiceID, ServiceUserRoleRO)) + if err != nil { + return fmt.Errorf("failed to get RO service user role from state: %w", err) + } + r.ROUsername = roRole.Username + r.ROPassword = roRole.Password + + rwRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(r.ServiceID, ServiceUserRoleRW)) if err != nil { - return fmt.Errorf("failed to get service user role from state: %w", err) + return fmt.Errorf("failed to get RW service user role from state: %w", err) } - r.Username = userRole.Username - r.Password = userRole.Password + r.RWUsername = rwRole.Username + r.RWPassword = rwRole.Password + return nil } diff --git a/server/internal/orchestrator/swarm/orchestrator.go b/server/internal/orchestrator/swarm/orchestrator.go index 701f596f..982b4d4b 100644 --- a/server/internal/orchestrator/swarm/orchestrator.go +++ b/server/internal/orchestrator/swarm/orchestrator.go @@ -440,19 +440,21 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn Allocator: o.dbNetworkAllocator, } - // Service user role resource (manages database user lifecycle) - serviceUserRole := &ServiceUserRole{ + // Service user role resources (manages database user lifecycle). + // Two roles are created per service: read-only and read-write. + serviceUserRoleRO := &ServiceUserRole{ ServiceID: spec.ServiceSpec.ServiceID, DatabaseID: spec.DatabaseID, DatabaseName: spec.DatabaseName, NodeName: spec.NodeName, + Mode: ServiceUserRoleRO, } - // Username and Password are populated from existing state during Refresh, - // or generated during Create. Only set if credentials exist (backward - // compatibility with existing state). - if spec.Credentials != nil { - serviceUserRole.Username = spec.Credentials.Username - serviceUserRole.Password = spec.Credentials.Password + serviceUserRoleRW := &ServiceUserRole{ + ServiceID: spec.ServiceSpec.ServiceID, + DatabaseID: spec.DatabaseID, + DatabaseName: spec.DatabaseName, + NodeName: spec.NodeName, + Mode: ServiceUserRoleRW, } // Service data directory resource (host-side bind mount directory) @@ -466,6 +468,7 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn } // MCP config resource (generates config.yaml, tokens.yaml, users.yaml) + // Credentials are populated from ServiceUserRole resources during refresh. mcpConfigResource := &MCPConfigResource{ ServiceInstanceID: spec.ServiceInstanceID, ServiceID: spec.ServiceSpec.ServiceID, @@ -476,10 +479,6 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn DatabaseHosts: spec.DatabaseHosts, TargetSessionAttrs: spec.TargetSessionAttrs, } - if spec.Credentials != nil { - mcpConfigResource.Username = spec.Credentials.Username - mcpConfigResource.Password = spec.Credentials.Password - } // Service instance spec resource serviceName := ServiceInstanceName(spec.ServiceSpec.ServiceType, spec.DatabaseID, spec.ServiceSpec.ServiceID, spec.HostID) @@ -511,10 +510,11 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn HostID: spec.HostID, } - // Resource chain: Network → ServiceUserRole → DirResource → MCPConfigResource → ServiceInstanceSpec → ServiceInstance + // Resource chain: Network → ServiceUserRole(RO,RW) → DirResource → MCPConfigResource → ServiceInstanceSpec → ServiceInstance orchestratorResources := []resource.Resource{ databaseNetwork, - serviceUserRole, + serviceUserRoleRO, + serviceUserRoleRW, dataDir, mcpConfigResource, serviceInstanceSpec, diff --git a/server/internal/orchestrator/swarm/service_instance.go b/server/internal/orchestrator/swarm/service_instance.go index 0e537b0f..d9c9c32b 100644 --- a/server/internal/orchestrator/swarm/service_instance.go +++ b/server/internal/orchestrator/swarm/service_instance.go @@ -61,7 +61,8 @@ func (s *ServiceInstanceResource) Executor() resource.Executor { func (s *ServiceInstanceResource) Dependencies() []resource.Identifier { return []resource.Identifier{ - ServiceUserRoleIdentifier(s.ServiceSpecID), + ServiceUserRoleIdentifier(s.ServiceSpecID, ServiceUserRoleRO), + ServiceUserRoleIdentifier(s.ServiceSpecID, ServiceUserRoleRW), ServiceInstanceSpecResourceIdentifier(s.ServiceInstanceID), } } diff --git a/server/internal/orchestrator/swarm/service_instance_spec.go b/server/internal/orchestrator/swarm/service_instance_spec.go index e8b44847..f613bac1 100644 --- a/server/internal/orchestrator/swarm/service_instance_spec.go +++ b/server/internal/orchestrator/swarm/service_instance_spec.go @@ -63,7 +63,8 @@ func (s *ServiceInstanceSpecResource) Dependencies() []resource.Identifier { // Service instances depend on the database network, service user role, and MCP config return []resource.Identifier{ NetworkResourceIdentifier(s.DatabaseNetworkID), - ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID), + ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID, ServiceUserRoleRO), + ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID, ServiceUserRoleRW), MCPConfigResourceIdentifier(s.ServiceInstanceID), } } @@ -73,7 +74,7 @@ func (s *ServiceInstanceSpecResource) TypeDependencies() []resource.Type { } func (s *ServiceInstanceSpecResource) populateCredentials(rc *resource.Context) error { - userRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID)) + userRole, err := resource.FromContext[*ServiceUserRole](rc, ServiceUserRoleIdentifier(s.ServiceSpec.ServiceID, ServiceUserRoleRO)) if err != nil { return fmt.Errorf("failed to get service user role from state: %w", err) } diff --git a/server/internal/orchestrator/swarm/service_user_role.go b/server/internal/orchestrator/swarm/service_user_role.go index 5dbe4c38..5ac29166 100644 --- a/server/internal/orchestrator/swarm/service_user_role.go +++ b/server/internal/orchestrator/swarm/service_user_role.go @@ -23,27 +23,37 @@ var _ resource.Resource = (*ServiceUserRole)(nil) const ResourceTypeServiceUserRole resource.Type = "swarm.service_user_role" +const ( + ServiceUserRoleRO = "ro" + ServiceUserRoleRW = "rw" +) + // sanitizeIdentifier quotes a string for use as a PostgreSQL identifier. // It doubles any internal double-quotes and wraps the result in double-quotes. func sanitizeIdentifier(name string) string { return `"` + strings.ReplaceAll(name, `"`, `""`) + `"` } -func ServiceUserRoleIdentifier(serviceID string) resource.Identifier { +func ServiceUserRoleIdentifier(serviceInstanceID string, mode string) resource.Identifier { return resource.Identifier{ - ID: serviceID, + ID: serviceInstanceID + "-" + mode, Type: ResourceTypeServiceUserRole, } } // ServiceUserRole manages the lifecycle of a database user for a service. // -// This resource is keyed by ServiceID, so one role is shared across all -// instances of the same service. It handles creation, verification, and -// cleanup of database users. On Create, it generates a deterministic username -// and random password, creates the Postgres role, and stores the credentials -// in the resource state. On subsequent reconciliation cycles, the credentials -// are reused from the persisted state (no password regeneration). +// Two ServiceUserRole resources are created per service: one with Mode set to +// ServiceUserRoleRO (read-only) and one with Mode set to ServiceUserRoleRW +// (read-write). Mode determines which group role the user is granted membership +// in: pgedge_application_read_only for RO, or pgedge_application for RW. All +// permissions are inherited via the group role — no custom grants are applied. +// +// On Create, a deterministic username (incorporating the mode) and a random +// password are generated, the Postgres role is created with the appropriate +// group role membership, and the credentials are stored in the resource state. +// On subsequent reconciliation cycles, credentials are reused from the persisted +// state (no password regeneration). // // The role is created on the primary instance and Spock replicates it to all // other nodes automatically. @@ -52,24 +62,26 @@ type ServiceUserRole struct { DatabaseID string `json:"database_id"` DatabaseName string `json:"database_name"` NodeName string `json:"node_name"` // Database node name for PrimaryExecutor routing + Mode string `json:"mode"` // ServiceUserRoleRO or ServiceUserRoleRW Username string `json:"username"` Password string `json:"password"` // Generated on Create, persisted in state } func (r *ServiceUserRole) ResourceVersion() string { - return "3" + return "4" } func (r *ServiceUserRole) DiffIgnore() []string { return []string{ "/node_name", + "/mode", "/username", "/password", } } func (r *ServiceUserRole) Identifier() resource.Identifier { - return ServiceUserRoleIdentifier(r.ServiceID) + return ServiceUserRoleIdentifier(r.ServiceID, r.Mode) } func (r *ServiceUserRole) Executor() resource.Executor { @@ -106,7 +118,7 @@ func (r *ServiceUserRole) Create(ctx context.Context, rc *resource.Context) erro logger.Info().Msg("creating service user role") // Generate deterministic username and random password - r.Username = database.GenerateServiceUsername(r.ServiceID) + r.Username = database.GenerateServiceUsername(r.ServiceID, r.Mode) password, err := utils.RandomString(32) if err != nil { return fmt.Errorf("failed to generate password: %w", err) @@ -130,17 +142,24 @@ func (r *ServiceUserRole) createUserRole(ctx context.Context, rc *resource.Conte } defer conn.Close(ctx) - // Create the role with LOGIN but no inherited roles. We grant permissions - // directly rather than using pgedge_application_read_only because that role - // includes read access to the spock schema (replication internals) which the - // MCP service should not expose. - // https://github.com/pgEdge/pgedge-postgres-mcp/blob/main/docs/guide/security_mgmt.md + // Determine group role based on mode + var groupRole string + switch r.Mode { + case ServiceUserRoleRO: + groupRole = "pgedge_application_read_only" + case ServiceUserRoleRW: + groupRole = "pgedge_application" + default: + return fmt.Errorf("unknown service user role mode: %q", r.Mode) + } + statements, err := postgres.CreateUserRole(postgres.UserRoleOptions{ Name: r.Username, Password: r.Password, DBName: r.DatabaseName, DBOwner: false, Attributes: []string{"LOGIN"}, + Roles: []string{groupRole}, }) if err != nil { return fmt.Errorf("failed to generate create user role statements: %w", err) @@ -150,21 +169,6 @@ func (r *ServiceUserRole) createUserRole(ctx context.Context, rc *resource.Conte return fmt.Errorf("failed to create service user: %w", err) } - // grants based on MCP doc guidelines, but open to change as needed - grants := postgres.Statements{ - // Database-level connect permission - postgres.Statement{SQL: fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s;", sanitizeIdentifier(r.DatabaseName), sanitizeIdentifier(r.Username))}, - // Read-only access to the public schema (application tables) - postgres.Statement{SQL: fmt.Sprintf("GRANT USAGE ON SCHEMA public TO %s;", sanitizeIdentifier(r.Username))}, - postgres.Statement{SQL: fmt.Sprintf("GRANT SELECT ON ALL TABLES IN SCHEMA public TO %s;", sanitizeIdentifier(r.Username))}, - postgres.Statement{SQL: fmt.Sprintf("ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT ON TABLES TO %s;", sanitizeIdentifier(r.Username))}, - // Allow viewing PostgreSQL configuration via diagnostic tools - postgres.Statement{SQL: fmt.Sprintf("GRANT pg_read_all_settings TO %s;", sanitizeIdentifier(r.Username))}, - } - if err := grants.Exec(ctx, conn); err != nil { - return fmt.Errorf("failed to grant service user permissions: %w", err) - } - return nil }