From fffac5229542f1b806170b60d4375f8dffc3f101 Mon Sep 17 00:00:00 2001 From: Tomas Dvorak Date: Thu, 17 Sep 2026 20:39:31 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20address=20PR=20#1=20security=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20platform=20seeds,=20seal=20db=20passwords?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove managed connection seeding: platform-postgres/dragonfly carried the backend's own DATABASE_URL/DRAGONFLY_URL into tenant projects where any member could run arbitrary DBX queries against core tables (Codex P1). NewPlatformService loses the seeds param; firstPartyDBSeeds deleted. - Seal db_connections passwords with the existing AES-256-GCM encryptor (enc:v1: marker); decrypted only in dbxArgsFor. Legacy plaintext values still read, but migration 00008 strips them (Codex P1). - Migration 00008: delete is_managed rows, drop plaintext password keys. - dbxConnectionName uses the full project UUID — the 8-hex prefix could collide and alias another project's credentials (Codex P2). - ci.yml triggers on master, not the nonexistent main/develop branches. - demo-mode stubs renamed to user-managed connections; CHANGELOG notes under Unreleased. Authored By: TDvorak Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 4 +- CHANGELOG.md | 6 ++ .../00008_drop_managed_connections.sql | 11 ++ apps/backend/internal/app/app.go | 36 +------ apps/backend/internal/services/databases.go | 100 ++++++++++-------- .../internal/services/platform_service.go | 5 +- apps/frontend/src/lib/demo-mode.ts | 18 ++-- 7 files changed, 85 insertions(+), 95 deletions(-) create mode 100644 apps/backend/db/migrations/00008_drop_managed_connections.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a14aab8..1fe8ed8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: push: - branches: [main, develop] + branches: [master] pull_request: - branches: [main, develop] + branches: [master] jobs: backend-tests: diff --git a/CHANGELOG.md b/CHANGELOG.md index 03fc17b..1e6d15a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to Primora. Format follows [Keep a Changelog](https://keepac ## [Unreleased] +### Security + +- **Managed platform connections removed** — `platform-postgres`/`platform-dragonfly` seeds carried the backend's own `DATABASE_URL`/`DRAGONFLY_URL` into every project, where any member could run arbitrary queries through DBX against `core` tables. The seeds are gone and migration 00008 deletes existing managed rows. Operators keep direct `psql`/`redis-cli` access. +- **Connection passwords encrypted at rest** — `core.db_connections.config.password` is now AES-256-GCM sealed (`enc:v1:` marker) via `PRIMORA_ENCRYPTION_KEY`; decrypted only when registering the connection with DBX. Migration 00008 strips pre-existing plaintext passwords. +- **DBX naming** — connection names now prefix the full project UUID; the previous 8-hex prefix could collide across projects and alias the wrong credentials. + ## [0.5.0] - 2026-09-16 ### Added diff --git a/apps/backend/db/migrations/00008_drop_managed_connections.sql b/apps/backend/db/migrations/00008_drop_managed_connections.sql new file mode 100644 index 0000000..c94ddd6 --- /dev/null +++ b/apps/backend/db/migrations/00008_drop_managed_connections.sql @@ -0,0 +1,11 @@ +-- +goose Up +-- Managed seeds stored the platform's own DATABASE_URL/DRAGONFLY_URL in +-- tenant-visible rows; the feature is removed, so drop the leaked rows. +DELETE FROM core.db_connections WHERE is_managed; +-- Passwords are AES-GCM sealed app-side from now on; strip any plaintext +-- password left in existing configs (users re-enter on next edit). +UPDATE core.db_connections SET config = config - 'password' WHERE config ? 'password'; + +-- +goose Down +-- Deleted managed rows and stripped passwords are not recoverable. +SELECT 1; diff --git a/apps/backend/internal/app/app.go b/apps/backend/internal/app/app.go index 1225c46..34530fa 100644 --- a/apps/backend/internal/app/app.go +++ b/apps/backend/internal/app/app.go @@ -6,11 +6,8 @@ import ( "fmt" "log/slog" "net/http" - "net/url" "os" "os/signal" - "strconv" - "strings" "syscall" "time" @@ -94,7 +91,7 @@ func Bootstrap(ctx context.Context) (*App, error) { if err != nil { return nil, fmt.Errorf("encryption key: %w", err) } - platform := services.NewPlatformService(repo, store, services.NewMailer(cfg), os.Getenv("VITE_APP_URL"), dbxClient, firstPartyDBSeeds(cfg), encryptor, logger) + platform := services.NewPlatformService(repo, store, services.NewMailer(cfg), os.Getenv("VITE_APP_URL"), dbxClient, encryptor, logger) if cfg.Env == "production" { gin.SetMode(gin.ReleaseMode) @@ -212,37 +209,6 @@ func (a *App) Run() error { return nil } -// firstPartyDBSeeds turns the platform's own Postgres and Dragonfly URLs into -// managed connection seeds registered on every project. -func firstPartyDBSeeds(cfg config.Config) []services.ManagedDBSeed { - var seeds []services.ManagedDBSeed - if u, err := url.Parse(cfg.DatabaseURL); err == nil && u.Hostname() != "" { - cfg2 := services.DBConnectionConfig{ - Host: u.Hostname(), - Database: strings.TrimPrefix(u.Path, "/"), - Username: u.User.Username(), - } - if p, ok := u.User.Password(); ok { - cfg2.Password = p - } - if port, err := strconv.Atoi(u.Port()); err == nil { - cfg2.Port = &port - } - seeds = append(seeds, services.ManagedDBSeed{Name: "platform-postgres", DBType: "postgres", Config: cfg2}) - } - if u, err := url.Parse(cfg.DragonflyURL); err == nil && u.Hostname() != "" { - cfg2 := services.DBConnectionConfig{Host: u.Hostname()} - if p, ok := u.User.Password(); ok { - cfg2.Password = p - } - if port, err := strconv.Atoi(u.Port()); err == nil { - cfg2.Port = &port - } - seeds = append(seeds, services.ManagedDBSeed{Name: "platform-dragonfly", DBType: "redis", Config: cfg2}) - } - return seeds -} - func (a *App) Close() error { if a.sweepCancel != nil { a.sweepCancel() diff --git a/apps/backend/internal/services/databases.go b/apps/backend/internal/services/databases.go index c13da28..5d0168e 100644 --- a/apps/backend/internal/services/databases.go +++ b/apps/backend/internal/services/databases.go @@ -2,6 +2,7 @@ package services import ( "context" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -9,7 +10,6 @@ import ( "strings" "github.com/google/uuid" - "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" db "github.com/tdvorak/primora/apps/backend/internal/database/db" @@ -19,7 +19,8 @@ import ( // DBConnectionConfig holds the server-side credential payload for a saved // connection. It is stored in core.db_connections.config and never serialized -// to the client — see DBConnectionSummary. +// to the client — see DBConnectionSummary. Password is AES-GCM sealed +// (enc:v1:) before persisting; openPassword restores it. type DBConnectionConfig struct { Host string `json:"host"` Port *int `json:"port,omitempty"` @@ -30,14 +31,6 @@ type DBConnectionConfig struct { DriverProfile string `json:"driver_profile,omitempty"` } -// ManagedDBSeed describes a first-party connection auto-registered on every -// project (the platform's own Postgres and Dragonfly). -type ManagedDBSeed struct { - Name string - DBType string - Config DBConnectionConfig -} - type CreateDBConnectionInput struct { Name string `json:"name" validate:"required,min=1"` DBType string `json:"db_type" validate:"required,oneof=postgres mysql sqlite redis mongodb sqlserver duckdb clickhouse cassandra"` @@ -88,15 +81,44 @@ func toDBConnectionSummary(row db.CoreDbConnection) DBConnectionSummary { } } -// dbxConnectionName derives the DBX-internal name. The project prefix keeps -// same-named connections in different projects from clobbering each other in -// DBX's shared storage. +// dbxConnectionName derives the DBX-internal name. The full project UUID +// prefix keeps same-named connections in different projects from colliding +// in DBX's shared storage. func dbxConnectionName(projectID uuid.UUID, name string) string { - short := strings.ReplaceAll(projectID.String(), "-", "")[:8] - return "prj_" + short + "_" + name + return "prj_" + strings.ReplaceAll(projectID.String(), "-", "") + "_" + name } -func (s *PlatformService) dbxArgsFor(row db.CoreDbConnection) map[string]any { +// encPasswordPrefix marks a sealed password inside the JSONB config. Values +// without the prefix are legacy plaintext from pre-encryption rows. +const encPasswordPrefix = "enc:v1:" + +func (s *PlatformService) sealPassword(plain string) (string, error) { + if plain == "" { + return "", nil + } + sealed, err := s.enc.Encrypt([]byte(plain)) + if err != nil { + return "", fmt.Errorf("encrypt password: %w", err) + } + return encPasswordPrefix + base64.StdEncoding.EncodeToString(sealed), nil +} + +func (s *PlatformService) openPassword(stored string) (string, error) { + if !strings.HasPrefix(stored, encPasswordPrefix) { + return stored, nil + } + sealed, err := base64.StdEncoding.DecodeString(strings.TrimPrefix(stored, encPasswordPrefix)) + if err != nil { + return "", fmt.Errorf("decode sealed password: %w", err) + } + plain, err := s.enc.Decrypt(sealed) + if err != nil { + return "", fmt.Errorf("decrypt password: %w", err) + } + return string(plain), nil +} + +func (s *PlatformService) dbxArgsFor(row db.CoreDbConnection) (map[string]any, error) { cfg := parseDBConnectionConfig(row.Config) args := map[string]any{ "name": dbxConnectionName(row.ProjectID, row.Name), @@ -113,7 +135,11 @@ func (s *PlatformService) dbxArgsFor(row db.CoreDbConnection) map[string]any { args["username"] = cfg.Username } if cfg.Password != "" { - args["password"] = cfg.Password + plain, err := s.openPassword(cfg.Password) + if err != nil { + return nil, err + } + args["password"] = plain } if cfg.SSL != nil { args["ssl"] = *cfg.SSL @@ -121,32 +147,7 @@ func (s *PlatformService) dbxArgsFor(row db.CoreDbConnection) map[string]any { if cfg.DriverProfile != "" { args["driver_profile"] = cfg.DriverProfile } - return args -} - -// seedManagedConnections registers the platform's own Postgres and Dragonfly -// on the project. Runs on list so existing projects pick them up too. -func (s *PlatformService) seedManagedConnections(ctx context.Context, projectID uuid.UUID) { - for _, seed := range s.managedDBSeeds { - _, err := s.repo.Queries().GetDBConnectionByName(ctx, db.GetDBConnectionByNameParams{ - ProjectID: projectID, - Name: seed.Name, - }) - if err == nil { - continue - } - if !errors.Is(err, pgx.ErrNoRows) { - continue - } - cfgBytes, _ := json.Marshal(seed.Config) - _, _ = s.repo.Queries().CreateDBConnection(ctx, db.CreateDBConnectionParams{ - ProjectID: projectID, - Name: seed.Name, - DbType: seed.DBType, - Config: cfgBytes, - IsManaged: true, - }) - } + return args, nil } func (s *PlatformService) DBXStatus(ctx context.Context, actor *models.Actor) (map[string]any, error) { @@ -163,7 +164,6 @@ func (s *PlatformService) ListDBConnections(ctx context.Context, actor *models.A if err := s.requireProjectRole(ctx, actor, projectID, "admin", "developer", "viewer"); err != nil { return nil, err } - s.seedManagedConnections(ctx, projectID) rows, err := s.repo.Queries().ListDBConnections(ctx, projectID) if err != nil { return nil, err @@ -183,12 +183,16 @@ func (s *PlatformService) CreateDBConnection(ctx context.Context, actor *models. if name == "" { return DBConnectionSummary{}, errors.New("name must contain letters or digits") } + sealedPassword, err := s.sealPassword(input.Password) + if err != nil { + return DBConnectionSummary{}, err + } cfg := DBConnectionConfig{ Host: strings.TrimSpace(input.Host), Port: input.Port, Database: strings.TrimSpace(input.Database), Username: strings.TrimSpace(input.Username), - Password: input.Password, + Password: sealedPassword, SSL: input.SSL, DriverProfile: strings.TrimSpace(input.DriverProfile), } @@ -269,7 +273,11 @@ func (s *PlatformService) connForTool(ctx context.Context, actor *models.Actor, if s.dbx == nil { return db.CoreDbConnection{}, errors.New("dbx not configured") } - if err := s.dbx.EnsureConnection(ctx, dbxConnectionName(projectID, row.Name), s.dbxArgsFor(row)); err != nil { + args, err := s.dbxArgsFor(row) + if err != nil { + return db.CoreDbConnection{}, err + } + if err := s.dbx.EnsureConnection(ctx, dbxConnectionName(projectID, row.Name), args); err != nil { return db.CoreDbConnection{}, err } return row, nil diff --git a/apps/backend/internal/services/platform_service.go b/apps/backend/internal/services/platform_service.go index 007f17e..fb9a6d0 100644 --- a/apps/backend/internal/services/platform_service.go +++ b/apps/backend/internal/services/platform_service.go @@ -33,7 +33,6 @@ type PlatformService struct { mailer *Mailer publicURL string dbx *dbx.Client - managedDBSeeds []ManagedDBSeed hub *EventHub enc *secrets.Encryptor dispatcher *WebhookDispatcher @@ -222,8 +221,8 @@ type InvitationSummary struct { Status string `json:"status"` } -func NewPlatformService(repo *repositories.CoreRepository, store *storage.LocalStore, mailer *Mailer, publicURL string, dbxClient *dbx.Client, managedDBSeeds []ManagedDBSeed, enc *secrets.Encryptor, logger *slog.Logger) *PlatformService { - s := &PlatformService{repo: repo, store: store, mailer: mailer, publicURL: publicURL, dbx: dbxClient, managedDBSeeds: managedDBSeeds, hub: NewEventHub(), enc: enc} +func NewPlatformService(repo *repositories.CoreRepository, store *storage.LocalStore, mailer *Mailer, publicURL string, dbxClient *dbx.Client, enc *secrets.Encryptor, logger *slog.Logger) *PlatformService { + s := &PlatformService{repo: repo, store: store, mailer: mailer, publicURL: publicURL, dbx: dbxClient, hub: NewEventHub(), enc: enc} if enc != nil { s.dispatcher = NewWebhookDispatcher(repo.Queries(), enc, logger) s.dispatcher.Start(context.Background()) diff --git a/apps/frontend/src/lib/demo-mode.ts b/apps/frontend/src/lib/demo-mode.ts index 8b4a704..5e94c7f 100644 --- a/apps/frontend/src/lib/demo-mode.ts +++ b/apps/frontend/src/lib/demo-mode.ts @@ -340,27 +340,27 @@ let dbConnections: DBConnection[] = [ { id: "dbc-1", project_id: "proj-1", - name: "platform-postgres", + name: "warehouse", db_type: "postgres", - host: "postgres", + host: "db.example.internal", port: 5432, - database: "primora", - username: "primora", - ssl: null, - is_managed: true, + database: "analytics", + username: "primora_read", + ssl: true, + is_managed: false, has_password: true, }, { id: "dbc-2", project_id: "proj-1", - name: "platform-dragonfly", + name: "cache", db_type: "redis", - host: "dragonfly", + host: "redis.example.internal", port: 6379, database: "", username: "", ssl: null, - is_managed: true, + is_managed: false, has_password: false, }, ];