security: fix 7 credential exposure and secret management issues

.env.example

@@ -45,5 +45,10 @@ JAEGER_ENDPOINT=http://localhost:4318
 
 # PowerDNS Configuration
 POWERDNS_API_URL=http://localhost:8081
-POWERDNS_API_KEY=thecloud-dns-secret
+# POWERDNS_API_KEY is REQUIRED - must be set to a secure value
+POWERDNS_API_KEY=your-secure-dns-api-key
 POWERDNS_SERVER_ID=localhost
+
+# Storage Configuration
+# STORAGE_SECRET is REQUIRED for presigned URL signing - must be set to a secure value
+STORAGE_SECRET=your-secure-storage-secret

.github/workflows/benchmarks.yml

@@ -6,6 +6,11 @@ on:
   pull_request:
     branches: [ main ]
 
+env:
+  SECRETS_ENCRYPTION_KEY: abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789
+  STORAGE_SECRET: ci-test-storage-secret-key
+  POWERDNS_API_KEY: ci-test-powerdns-api-key
+
 jobs:
   benchmark:
     name: Run Benchmarks

.github/workflows/ci.yml

@@ -2,6 +2,9 @@ name: CI
 
 env:
   FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
+  SECRETS_ENCRYPTION_KEY: abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789
+  STORAGE_SECRET: ci-test-storage-secret-key
+  POWERDNS_API_KEY: ci-test-powerdns-api-key
 
 on:
   push:
@@ -245,18 +248,21 @@ jobs:
       - name: Test Backend Switching
         env:
           DATABASE_URL: postgres://cloud:cloud@127.0.0.1:5433/cloud?sslmode=disable
+          SECRETS_ENCRYPTION_KEY: abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789
+          STORAGE_SECRET: ci-test-storage-secret-key
+          POWERDNS_API_KEY: ci-test-powerdns-api-key
         run: |
           # Ensure clean DB for switching tests
           PGPASSWORD=cloud psql -h 127.0.0.1 -p 5433 -U cloud -d postgres -c "DROP DATABASE IF EXISTS cloud;"
           PGPASSWORD=cloud psql -h 127.0.0.1 -p 5433 -U cloud -d postgres -c "CREATE DATABASE cloud;"
           go run ./cmd/api -migrate-only
-          
+
           # Test with Docker backend (default)
           export TEST_DOCKER_NETWORK=cloud-network
           docker network create ${TEST_DOCKER_NETWORK} || true
           export COMPUTE_BACKEND=docker
           go test -p 1 -v -run TestInstanceService ./internal/core/services/...
-          
+
           # Test with Libvirt backend
           export COMPUTE_BACKEND=libvirt
           go test -p 1 -v -run TestInstanceService ./internal/core/services/...

.github/workflows/coverage.yml

@@ -1,5 +1,10 @@
 name: Coverage Gate
 
+env:
+  SECRETS_ENCRYPTION_KEY: abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789
+  STORAGE_SECRET: ci-test-storage-secret-key
+  POWERDNS_API_KEY: ci-test-powerdns-api-key
+
 on:
   pull_request:
     branches: [ main ]

.github/workflows/database-replication.yml

@@ -102,12 +102,15 @@ jobs:
           DATABASE_URL: postgres://cloud:cloud@127.0.0.1:5433/cloud?sslmode=disable
           COMPUTE_BACKEND: docker
           TEST_DOCKER_NETWORK: cloud-network
+          SECRETS_ENCRYPTION_KEY: abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789
+          STORAGE_SECRET: ci-test-storage-secret-key
+          POWERDNS_API_KEY: ci-test-powerdns-api-key
         run: |
           # Start the API server in the background
           # The server will run migrations automatically on startup
           ./api > api.log 2>&1 &
           echo $! > api.pid
-          
+
           # Wait for the server to be ready
           echo "Waiting for API server to start..."
           timeout 90s bash -c 'until curl -s http://localhost:8080/health/live; do sleep 2; done' || (echo "API Server Logs:" && cat api.log && exit 1)

.github/workflows/e2e.yml

@@ -35,6 +35,8 @@ jobs:
           SECRETS_ENCRYPTION_KEY: abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789
           JWT_SECRET: e2e-test-secret-key-that-is-long-enough-for-32-chars
           DATABASE_READ_URL: postgres://cloud:cloud@postgres:5432/cloud?sslmode=disable
+          STORAGE_SECRET: e2e-test-storage-secret-key
+          POWERDNS_API_KEY: e2e-test-powerdns-api-key
         run: |
           # Start internal dependencies
           docker compose up -d postgres redis jaeger
@@ -67,6 +69,8 @@ jobs:
           JWT_SECRET: e2e-test-secret-key-that-is-long-enough-for-32-chars
           DATABASE_READ_URL: postgres://cloud:cloud@postgres:5432/cloud?sslmode=disable
           TRACING_ENABLED: false
+          STORAGE_SECRET: e2e-test-storage-secret-key
+          POWERDNS_API_KEY: e2e-test-powerdns-api-key
         run: |
           # Export variables so docker-compose can access them
           export SECRETS_ENCRYPTION_KEY="${SECRETS_ENCRYPTION_KEY}"
@@ -76,6 +80,8 @@ jobs:
           export DB_PORT_MAPPING="${DB_PORT_MAPPING}"
           export COMPOSE_PROJECT_NAME="${COMPOSE_PROJECT_NAME}"
           export DOCKER_DEFAULT_NETWORK="${DOCKER_DEFAULT_NETWORK}"
+          export STORAGE_SECRET="${STORAGE_SECRET}"
+          export POWERDNS_API_KEY="${POWERDNS_API_KEY}"
 
           # Ensure Docker socket has proper permissions for API to create containers
           sudo chmod 666 /var/run/docker.sock

docker-compose.yml

@@ -105,7 +105,8 @@ services:
       - TRACING_ENABLED=${TRACING_ENABLED:-false}
       - JAEGER_ENDPOINT=http://jaeger:4318
       - POWERDNS_API_URL=http://powerdns:8081
-      - POWERDNS_API_KEY=${POWERDNS_API_KEY:-thecloud-dns-secret}
+      - POWERDNS_API_KEY=${POWERDNS_API_KEY}
+      - STORAGE_SECRET=${STORAGE_SECRET}
       - DOCKER_DEFAULT_NETWORK=${DOCKER_DEFAULT_NETWORK:-cloud-network}
     volumes:
       - /var/run/docker.sock:/var/run/docker.sock

internal/core/domain/database.go

@@ -73,4 +73,5 @@ type Database struct {
 	KmsKeyID            string            `json:"kms_key_id,omitempty"`
 	EncryptedVolume     bool              `json:"encrypted_volume"`
 	VolumeKeyRef        string            `json:"volume_key_ref,omitempty"`
+	CredentialVersion   int               `json:"-"`
 }

internal/core/services/database.go

@@ -563,7 +563,8 @@ func (s *DatabaseService) PromoteToPrimary(ctx context.Context, id uuid.UUID) er
 				}
 			}
 		}
-		cmd = []string{"mysql", "-u", "root", "-p" + password, "-e", "STOP REPLICA; RESET REPLICA ALL;"}
+		// Use MYSQL_PWD env var to avoid password appearing in process list
+		cmd = []string{"sh", "-c", fmt.Sprintf("MYSQL_PWD='%s' mysql -u root --execute='STOP REPLICA; RESET REPLICA ALL;'", sqlStringLiteral(password))}
 	default:
 		return errors.New(errors.Internal, "unsupported engine for promotion")
 	}
@@ -852,7 +853,7 @@ func (s *DatabaseService) doRotateCredentials(ctx context.Context, id uuid.UUID,
 		return errors.Wrap(errors.Internal, "failed to generate new password", err)
 	}
 
-	// Get current password for MySQL auth
+	// Get current password for DB auth
 	currentPassword := db.Password
 	if db.CredentialPath != "" {
 		secret, err := s.secrets.GetSecret(ctx, db.CredentialPath)
@@ -863,7 +864,19 @@ func (s *DatabaseService) doRotateCredentials(ctx context.Context, id uuid.UUID,
 		}
 	}
 
-	// 1. Execute ALTER USER in container FIRST
+	// 1. Store new password in Vault at versioned path FIRST (before DB change)
+	vaultPath := db.CredentialPath
+	if vaultPath == "" {
+		vaultPath = s.getVaultPath(db.ID)
+	}
+	// Increment version and compute new versioned path
+	newVersion := db.CredentialVersion + 1
+	versionedPath := vaultPath + "/v" + strconv.Itoa(newVersion)
+	if err := s.secrets.StoreSecret(ctx, versionedPath, map[string]interface{}{"password": newPassword}); err != nil {
+		return errors.Wrap(errors.Internal, "failed to store new credential in vault", err)
+	}
+
+	// 2. Execute ALTER USER in container using currentPassword for auth
 	cmd := s.buildPasswordChangeCmd(db.Engine, db.Username, currentPassword, newPassword)
 	if cmd == nil {
 		return errors.New(errors.Internal, "unsupported engine for credential rotation")
@@ -880,27 +893,20 @@ func (s *DatabaseService) doRotateCredentials(ctx context.Context, id uuid.UUID,
 		return errors.Wrap(errors.Internal, "failed to execute password rotation in container", execErr)
 	}
 
-	// 2. Update in Vault ONLY after DB success
-	vaultPath := db.CredentialPath
-	if vaultPath == "" {
-		vaultPath = s.getVaultPath(db.ID)
-	}
-	if err := s.secrets.StoreSecret(ctx, vaultPath, map[string]interface{}{"password": newPassword}); err != nil {
-		// Vault store failed but DB already has new password - rollback to original
-		rollbackCmd := s.buildPasswordChangeCmd(db.Engine, db.Username, currentPassword, newPassword)
-		if _, rollbackErr := s.compute.Exec(ctx, db.ContainerID, rollbackCmd); rollbackErr != nil {
-			// Rollback also failed - system is in critical state requiring manual intervention
-			return errors.Wrap(errors.Internal,
-				fmt.Sprintf("credential rotation failed and rollback also failed - manual intervention required (vault store error: %v)", err),
-				rollbackErr)
-		}
-		return errors.Wrap(errors.Internal, "vault store failed, DB password rolled back", err)
+	// 3. Update DB record to point to new versioned path (ONLY after DB confirmed updated)
+	// The old path still has the old password for rollback if needed
+	db.CredentialPath = versionedPath
+	db.CredentialVersion = newVersion
+	if err := s.repo.Update(ctx, db); err != nil {
+		return errors.Wrap(errors.Internal, "failed to update DB credential path", err)
 	}
 
-	// 3. Update DB record if needed (metadata or path)
-	db.CredentialPath = vaultPath
-	if err := s.repo.Update(ctx, db); err != nil {
-		return err
+	// 4. Cleanup old versioned path from Vault
+	oldVersionedPath := vaultPath + "/v" + strconv.Itoa(db.CredentialVersion-1)
+	if err := s.secrets.DeleteSecret(ctx, oldVersionedPath); err != nil {
+		// Log but don't fail - old version may not exist if this is first rotation
+		s.logger.Warn("failed to cleanup old credential version from vault", "path", oldVersionedPath, "error", err)
+		platform.CredentialCleanupFailures.Inc()
 	}
 
 	// 4. If pooler is enabled, restart it to pick up new credentials
@@ -965,11 +971,18 @@ func postgresIdentifier(id string) string {
 func (s *DatabaseService) buildPasswordChangeCmd(engine domain.DatabaseEngine, username, authPassword, targetPassword string) []string {
 	switch engine {
 	case domain.EnginePostgres:
-		return []string{"psql", "-h", "127.0.0.1", "-U", username, "-d", "postgres", "-c",
-			fmt.Sprintf("ALTER USER %s WITH PASSWORD '%s';", postgresIdentifier(username), sqlStringLiteral(targetPassword))}
+		// Use PGPASSWORD env var for authentication (libpq standard).
+		// Note: While env vars avoid cmdline exposure, the password is still visible to
+		// root users via /proc/$pid/environ. This is a known tradeoff - the alternative
+		// (stdin password) requires more complex interaction patterns with psql.
+		// authPassword (current password) authenticates the connection; targetPassword is the new password being set.
+		stmt := fmt.Sprintf("ALTER USER %s WITH PASSWORD '%s';", postgresIdentifier(username), sqlStringLiteral(targetPassword))
+		return []string{"sh", "-c", "PGPASSWORD='" + sqlStringLiteral(authPassword) + "' psql -h 127.0.0.1 -U " + username + " -d postgres -c '" + stmt + "'"}
 	case domain.EngineMySQL:
-		return []string{"mysql", "-u", "root", "-p" + authPassword, "-e",
-			fmt.Sprintf("ALTER USER '%s'@'%%' IDENTIFIED BY '%s';", sqlStringLiteral(username), sqlStringLiteral(targetPassword))}
+		// Use mysql with password via MYSQL_PWD env var to avoid cmdline exposure.
+		// Same tradeoff as above - password visible in /proc environment to root users.
+		stmt := fmt.Sprintf("ALTER USER '%s'@'%%' IDENTIFIED BY '%s';", sqlStringLiteral(username), sqlStringLiteral(targetPassword))
+		return []string{"sh", "-c", "MYSQL_PWD='" + sqlStringLiteral(authPassword) + "' mysql -u " + sqlStringLiteral(username) + " --execute='" + stmt + "'"}
 	}
 	return nil
 }

internal/core/services/database_vault_test.go

@@ -62,34 +62,50 @@ func TestDatabaseService_RotateCredentials(t *testing.T) {
 	ctx := context.Background()
 	dbID := uuid.New()
 	db := &domain.Database{
-		ID:             dbID,
-		UserID:         uuid.New(),
-		Name:           "test-db",
-		Engine:         domain.EnginePostgres,
-		Username:       "cloud_user",
-		ContainerID:    "cid-1",
-		CredentialPath: "secret/rds/" + dbID.String() + "/credentials",
+		ID:               dbID,
+		UserID:           uuid.New(),
+		Name:             "test-db",
+		Engine:           domain.EnginePostgres,
+		Username:         "cloud_user",
+		ContainerID:      "cid-1",
+		CredentialPath:    "secret/rds/" + dbID.String() + "/credentials",
+		CredentialVersion: 1,
 	}
 
 	t.Run("RotateCredentials_Success", func(t *testing.T) {
-		mockRepo.On("GetByID", mock.Anything, dbID).Return(db, nil).Once()
-		mockSecrets.On("GetSecret", mock.Anything, db.CredentialPath).Return(map[string]interface{}{"password": "old-pass"}, nil).Once()
-
-		// 1. Execute ALTER USER in container
-		mockCompute.On("Exec", mock.Anything, db.ContainerID, mock.Anything).Return("ALTER ROLE", nil).Once()
-
-		// 2. Update in Vault
-		mockSecrets.On("StoreSecret", mock.Anything, db.CredentialPath, mock.MatchedBy(func(data map[string]interface{}) bool {
+		// Create a fresh db for this test to avoid mutation from previous test affecting this one
+		testDB := &domain.Database{
+			ID:               db.ID,
+			UserID:           db.UserID,
+			Name:             db.Name,
+			Engine:           db.Engine,
+			Username:         db.Username,
+			ContainerID:      db.ContainerID,
+			CredentialPath:   "secret/rds/" + dbID.String() + "/credentials",
+			CredentialVersion: 1,
+		}
+		mockRepo.On("GetByID", mock.Anything, dbID).Return(testDB, nil).Once()
+		mockSecrets.On("GetSecret", mock.Anything, testDB.CredentialPath).Return(map[string]interface{}{"password": "old-pass"}, nil).Once()
+
+		// 1. Store new credential at versioned path in Vault FIRST (version 2 since db starts at version 1)
+		versionedPath := testDB.CredentialPath + "/v2"
+		mockSecrets.On("StoreSecret", mock.Anything, versionedPath, mock.MatchedBy(func(data map[string]interface{}) bool {
 			return data["password"] != ""
 		})).Return(nil).Once()
 
-		// 3. Update DB record
+		// 2. Execute ALTER USER in container
+		mockCompute.On("Exec", mock.Anything, testDB.ContainerID, mock.Anything).Return("ALTER ROLE", nil).Once()
+
+		// 3. Update DB record to point to new versioned path and increment version
 		mockRepo.On("Update", mock.Anything, mock.MatchedBy(func(d *domain.Database) bool {
-			return d.ID == dbID
+			return d.ID == dbID && d.CredentialPath == versionedPath && d.CredentialVersion == 2
 		})).Return(nil).Once()
 
+		// 4. Cleanup old versioned path from Vault (v1)
+		mockSecrets.On("DeleteSecret", mock.Anything, testDB.CredentialPath+"/v1").Return(nil).Once()
+
 		mockEventSvc.On("RecordEvent", mock.Anything, "DATABASE_CREDENTIALS_ROTATE", dbID.String(), "DATABASE", mock.Anything).Return(nil).Once()
-		mockAuditSvc.On("Log", mock.Anything, db.UserID, "database.rotate_credentials", "database", db.ID.String(), mock.Anything).Return(nil).Once()
+		mockAuditSvc.On("Log", mock.Anything, testDB.UserID, "database.rotate_credentials", "database", testDB.ID.String(), mock.Anything).Return(nil).Once()
 
 		err := svc.RotateCredentials(ctx, dbID, "")
 		require.NoError(t, err)
@@ -99,23 +115,31 @@ func TestDatabaseService_RotateCredentials(t *testing.T) {
 		mockRepo.AssertExpectations(t)
 	})
 
-	t.Run("RotateCredentials_VaultFailure_WithRollback", func(t *testing.T) {
-		mockRepo.On("GetByID", mock.Anything, dbID).Return(db, nil).Once()
-		mockSecrets.On("GetSecret", mock.Anything, db.CredentialPath).Return(map[string]interface{}{"password": "old-pass"}, nil).Once()
-		// First Exec: ALTER USER with new password (succeeds)
-		mockCompute.On("Exec", mock.Anything, db.ContainerID, mock.Anything).Return("ALTER ROLE", nil).Once()
-		// Vault store fails
-		mockSecrets.On("StoreSecret", mock.Anything, db.CredentialPath, mock.Anything).Return(fmt.Errorf("vault error")).Once()
-		// Second Exec: rollback to original password (succeeds)
-		mockCompute.On("Exec", mock.Anything, db.ContainerID, mock.Anything).Return("ALTER ROLE", nil).Once()
+	t.Run("RotateCredentials_VaultFailure_WithoutDBChange", func(t *testing.T) {
+		// Create a fresh db for this test
+		testDB := &domain.Database{
+			ID:               db.ID,
+			UserID:           db.UserID,
+			Name:             db.Name,
+			Engine:           db.Engine,
+			Username:         db.Username,
+			ContainerID:      db.ContainerID,
+			CredentialPath:   "secret/rds/" + dbID.String() + "/credentials",
+			CredentialVersion: 1,
+		}
+		mockRepo.On("GetByID", mock.Anything, dbID).Return(testDB, nil).Once()
+		mockSecrets.On("GetSecret", mock.Anything, testDB.CredentialPath).Return(map[string]interface{}{"password": "old-pass"}, nil).Once()
+		// Vault store fails at step 1 (versioned path) - DB is NOT changed
+		versionedPath := testDB.CredentialPath + "/v2"
+		mockSecrets.On("StoreSecret", mock.Anything, versionedPath, mock.Anything).Return(fmt.Errorf("vault error")).Once()
 
 		err := svc.RotateCredentials(ctx, dbID, "")
 		require.Error(t, err)
-		assert.Contains(t, err.Error(), "vault store failed, DB password rolled back")
+		assert.Contains(t, err.Error(), "failed to store new credential in vault")
 
 		mockSecrets.AssertExpectations(t)
+		// Compute.Exec should NOT be called since Vault failed before DB change
 		mockCompute.AssertExpectations(t)
-		mockRepo.AssertExpectations(t)
 	})
 }
 

internal/core/services/identity.go

@@ -8,6 +8,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"log/slog"
+	"os"
 	"time"
 
 	"github.com/google/uuid"
@@ -19,21 +20,20 @@ import (
 )
 
 // serverSecret is used as HMAC key to prevent rainbow table attacks on API key hashes.
-// This is derived from SECRETS_ENCRYPTION_KEY env var if set, otherwise uses a static value.
-// In production, set SECRETS_ENCRYPTION_KEY for proper security.
 var serverSecret = getServerSecret()
 
 func getServerSecret() string {
-	// Use the secrets encryption key if available, otherwise fall back to a warning string
-	// that will be rejected in production
 	secret := platform.GetSecretsEncryptionKey()
 	if secret != "" {
 		return secret
 	}
-	// Fallback for development - in production this should not be used
-	// Log warning to help diagnose configuration issues
-	slog.Default().Warn("SECRETS_ENCRYPTION_KEY not set, using development secret for API key hashing - configure for production")
-	return "thecloud-development-secret-do-not-use-in-production"
+	// For tests, allow a fallback to avoid os.Exit in package init
+	if os.Getenv("TEST_SECRETS") != "" {
+		return os.Getenv("TEST_SECRETS")
+	}
+	slog.Default().Error("SECRETS_ENCRYPTION_KEY environment variable is required")
+	os.Exit(1)
+	return "" // unreachable but satisfies compiler
 }
 
 // computeKeyHash creates a HMAC-SHA256 hash of the API key using the server secret.