Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions api/projects/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func (c *KeyController) AddKey(w http.ResponseWriter, r *http.Request) {
// Plain cannot be passed via a request
key.Plain = nil
key.IgnorePlain = true
key.Synchronized = false

//if err := key.Validate(true); err != nil {
// helpers.WriteJSON(w, http.StatusBadRequest, map[string]string{
Expand Down Expand Up @@ -138,9 +139,19 @@ func (c *KeyController) UpdateKey(w http.ResponseWriter, r *http.Request) {
return
}

if oldKey.Synchronized {
if key.Name != oldKey.Name || key.Type != oldKey.Type {
helpers.WriteJSON(w, http.StatusBadRequest, map[string]string{
"error": "Name and type of synchronized key cannot be changed",
})
return
}
}

// Plain cannot be passed via a request
key.Plain = nil
key.IgnorePlain = true
key.Synchronized = oldKey.Synchronized

repos, err := helpers.Store(r).GetRepositories(*key.ProjectID, db.RetrieveQueryParams{})
if err != nil {
Expand Down
8 changes: 7 additions & 1 deletion api/projects/secret_storages.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,13 @@ func (c *SecretStorageController) SyncSecrets(w http.ResponseWriter, r *http.Req
return
}

err := c.secretStorageService.SyncSecrets(storage)
sync, err := helpers.Store(r).GetStorageSecretSync(storage.ID)
if err != nil {
helpers.WriteError(w, err)
return
}

err = c.secretStorageService.SyncSecrets(sync)
if err != nil {
helpers.WriteError(w, err)
return
Expand Down
5 changes: 5 additions & 0 deletions cli/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func runService() {
)
accessKeyService := server.NewAccessKeyService(store, encryptionService, store)
secretStorageService := server.NewSecretStorageService(store, store, accessKeyService, encryptionService)
secretStorageSyncScheduler := server.NewSecretStorageSyncScheduler(store, secretStorageService)
environmentService := server.NewEnvironmentService(store, encryptionService)
subscriptionService := proServer.NewSubscriptionService(store, store, store, terraformStore)
logWriteService := proServer.NewLogWriteService()
Expand Down Expand Up @@ -136,6 +137,7 @@ func runService() {

if dedup := proHA.NewScheduleDeduplicator(); dedup != nil {
schedulePool.SetDeduplicator(dedup)
secretStorageSyncScheduler.SetTickDeduplicator(dedup)
}

// Each process holds its own in-memory cron table. Schedule CRUD handlers only
Expand Down Expand Up @@ -185,6 +187,9 @@ func runService() {
go schedulePool.Run()
go taskPool.Run()

secretStorageSyncScheduler.Start()
Comment thread
fiftin marked this conversation as resolved.
defer secretStorageSyncScheduler.Stop()

route := api.Route(
store,
terraformStore,
Expand Down
2 changes: 2 additions & 0 deletions db/AccessKey.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ type AccessKey struct {
// If SourceStorageID is nil, this field is references to an environment variable.
SourceStorageKey *string `db:"source_storage_key" json:"source_storage_key,omitempty"`
SourceStorageType *AccessKeySourceStorageType `db:"source_storage_type" json:"source_storage_type,omitempty"`

Synchronized bool `db:"synchronized" json:"synchronized,omitempty"`
}

func (key *AccessKey) IsNativelyReadOnly() bool {
Expand Down
8 changes: 8 additions & 0 deletions db/Environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package db
import (
"encoding/json"
"errors"
"time"
)

type EnvironmentSecretOperation string
Expand Down Expand Up @@ -53,6 +54,13 @@ type Environment struct {

SecretStorageID *int `db:"secret_storage_id" json:"secret_storage_id,omitempty" backup:"-"`
SecretStorageKeyPrefix *string `db:"secret_storage_key_prefix" json:"secret_storage_key_prefix,omitempty"`

// Sync fields are transfer-only; persisted in project__secret_sync.
SyncEnabled bool `db:"-" json:"sync_enabled"`
SyncInterval int `db:"-" json:"sync_interval"`
LastSyncedAt *time.Time `db:"-" json:"last_synced_at,omitempty"`
LastSyncFailedAt *time.Time `db:"-" json:"last_sync_failed_at,omitempty"`
SyncPaths []SecretSyncPath `db:"-" json:"sync_paths"`
}

func (s *EnvironmentSecret) Validate() error {
Expand Down
1 change: 1 addition & 0 deletions db/Migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func GetMigrations(dialect string) []Migration {
{Version: "2.17.15"},
{Version: "2.17.16"},
{Version: "2.17.17"},
{Version: "2.18.1"},
}

return append(initScripts, commonScripts...)
Expand Down
15 changes: 12 additions & 3 deletions db/SecretStorage.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package db

import "time"

type SecretStorageType string

const (
SecretStorageTypeLocal SecretStorageType = "local"
SecretStorageTypeVault SecretStorageType = "vault"
SecretStorageTypeDvls SecretStorageType = "dvls"
SecretStorageTypeLocal SecretStorageType = "local"
SecretStorageTypeVault SecretStorageType = "vault"
SecretStorageTypeDvls SecretStorageType = "dvls"
SecretStorageTypeAwsSm SecretStorageType = "aws_sm"
SecretStorageTypeAzureKv SecretStorageType = "azure_kv"
)
Expand All @@ -18,6 +20,13 @@ type SecretStorage struct {
Params MapStringAnyField `db:"params" json:"params"`
ReadOnly bool `db:"readonly" json:"readonly"`

// Sync fields are transfer-only; persisted in project__secret_sync.
SyncEnabled bool `db:"-" json:"sync_enabled" backup:"sync_enabled"`
SyncInterval int `db:"-" json:"sync_interval" backup:"sync_interval"`
LastSyncedAt *time.Time `db:"-" json:"last_synced_at,omitempty" backup:"-"`
LastSyncFailedAt *time.Time `db:"-" json:"last_sync_failed_at,omitempty" backup:"-"`
SyncPaths []SecretSyncPath `db:"-" json:"sync_paths" backup:"-"`

SourceStorageType *AccessKeySourceStorageType `db:"-" json:"source_storage_type,omitempty" backup:"-"`
// Secret is a source value: literal secret for local storage,
// env var name for "env", or file path for "file".
Expand Down
34 changes: 34 additions & 0 deletions db/SecretSync.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package db

import "time"

// SecretSync describes a unit of remote-storage synchronization.
// A row with EnvironmentID == nil represents storage-level sync (imports
// access keys). A row with EnvironmentID set represents env-scoped sync
// (imports environment variables for that variable group).
type SecretSync struct {
ID int `db:"id" json:"id" backup:"-"`
ProjectID int `db:"project_id" json:"project_id" backup:"-"`
StorageID int `db:"storage_id" json:"storage_id" backup:"-"`
EnvironmentID *int `db:"environment_id" json:"environment_id,omitempty" backup:"-"`

SyncEnabled bool `db:"sync_enabled" json:"sync_enabled"`
// SyncInterval is the auto-sync period in minutes. Zero disables auto-sync.
SyncInterval int `db:"sync_interval" json:"sync_interval"`
LastSyncedAt *time.Time `db:"last_synced_at" json:"last_synced_at,omitempty"`
LastSyncFailedAt *time.Time `db:"last_sync_failed_at" json:"last_sync_failed_at,omitempty"`

Paths []SecretSyncPath `db:"-" json:"paths"`
}

type SecretSyncPath struct {
ID int `db:"id" json:"id" backup:"-"`
SyncID int `db:"sync_id" json:"sync_id" backup:"-"`
Path string `db:"path" json:"path"`
Prefix string `db:"prefix" json:"prefix"`
Separator string `db:"separator" json:"separator"`
}

// SecretStorageSyncPath is retained as an alias for backward compatibility
// with callers that predate the SecretSync refactor.
type SecretStorageSyncPath = SecretSyncPath
21 changes: 21 additions & 0 deletions db/Store.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,26 @@ type SecretStorageRepository interface {
DeleteSecretStorage(projectID int, storageID int) error
}

type SecretSyncRepository interface {
// GetSyncEnabledSecretSyncs returns every sync config (storage-level
// and env-scoped) that is enabled with a positive interval.
GetSyncEnabledSecretSyncs() ([]SecretSync, error)
MarkSecretSyncSynced(syncID int, success bool, at time.Time) error

// GetStorageSecretSync returns the storage-level sync (EnvironmentID=nil)
// for the given storage, or ErrNotFound.
GetStorageSecretSync(storageID int) (SecretSync, error)
// GetEnvironmentSecretSync returns the env-scoped sync for the env,
// or ErrNotFound.
GetEnvironmentSecretSync(environmentID int) (SecretSync, error)

// SaveSecretSync upserts a sync config (and its paths) identified by
// (StorageID, EnvironmentID) on the passed struct. When SyncEnabled
// is false, SyncInterval is zero, and Paths is empty, the row is
// deleted instead of being written.
SaveSecretSync(sync SecretSync) error
}

type RoleRepository interface {
GetGlobalRoleBySlug(slug string) (Role, error)
GetProjectOrGlobalRoleBySlug(projectID int, slug string) (Role, error)
Expand Down Expand Up @@ -511,6 +531,7 @@ type Store interface {
RunnerManager
EventManager
SecretStorageRepository
SecretSyncRepository
RoleRepository
}

Expand Down
26 changes: 25 additions & 1 deletion db/bolt/secret_storage.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package bolt

import "github.com/semaphoreui/semaphore/db"
import (
"time"

"github.com/semaphoreui/semaphore/db"
)

func (d *BoltDb) GetSecretStorages(projectID int) ([]db.SecretStorage, error) {
return []db.SecretStorage{}, nil
Expand Down Expand Up @@ -28,3 +32,23 @@ func (d *BoltDb) UpdateSecretStorage(storage db.SecretStorage) error {
func (d *BoltDb) GetSecretStorageRefs(projectID int, storageID int) (db.ObjectReferrers, error) {
return d.getObjectRefs(projectID, db.SecretStorageProps, storageID)
}

func (d *BoltDb) GetSyncEnabledSecretSyncs() ([]db.SecretSync, error) {
return []db.SecretSync{}, nil
}

func (d *BoltDb) MarkSecretSyncSynced(syncID int, success bool, at time.Time) error {
return nil
}

func (d *BoltDb) GetStorageSecretSync(storageID int) (db.SecretSync, error) {
return db.SecretSync{}, db.ErrNotFound
}

func (d *BoltDb) GetEnvironmentSecretSync(environmentID int) (db.SecretSync, error) {
return db.SecretSync{}, db.ErrNotFound
}

func (d *BoltDb) SaveSecretSync(sync db.SecretSync) error {
return nil
}
12 changes: 8 additions & 4 deletions db/sql/access_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ func (d *SqlDb) CreateAccessKey(key db.AccessKey) (newKey db.AccessKey, err erro
"storage_id, "+
"source_storage_id, "+
"source_storage_key, "+
"source_storage_type) "+
"values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
"source_storage_type, "+
"synchronized) "+
"values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
key.Name,
key.Type,
key.ProjectID,
Expand All @@ -123,6 +124,7 @@ func (d *SqlDb) CreateAccessKey(key db.AccessKey) (newKey db.AccessKey, err erro
key.SourceStorageID,
key.SourceStorageKey,
key.SourceStorageType,
key.Synchronized,
)
} else {
insertID, err = d.insert(
Expand All @@ -138,8 +140,9 @@ func (d *SqlDb) CreateAccessKey(key db.AccessKey) (newKey db.AccessKey, err erro
"storage_id, "+
"source_storage_id, "+
"source_storage_key, "+
"source_storage_type) "+
"values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
"source_storage_type, "+
"synchronized) "+
"values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)",
key.Name,
key.Type,
key.ProjectID,
Expand All @@ -151,6 +154,7 @@ func (d *SqlDb) CreateAccessKey(key db.AccessKey) (newKey db.AccessKey, err erro
key.SourceStorageID,
key.SourceStorageKey,
key.SourceStorageType,
key.Synchronized,
)

}
Expand Down
76 changes: 72 additions & 4 deletions db/sql/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ import (

func (d *SqlDb) GetEnvironment(projectID int, environmentID int) (environment db.Environment, err error) {
err = d.getObject(projectID, db.EnvironmentProps, environmentID, &environment)
if err != nil {
return
}

err = d.fillEnvironmentSync(&environment)
return
}

Expand All @@ -14,9 +19,19 @@ func (d *SqlDb) GetEnvironmentRefs(projectID int, environmentID int) (db.ObjectR
}

func (d *SqlDb) GetEnvironments(projectID int, params db.RetrieveQueryParams) ([]db.Environment, error) {
var environment []db.Environment
err := d.getObjects(projectID, db.EnvironmentProps, params, nil, &environment)
return environment, err
var environments []db.Environment
err := d.getObjects(projectID, db.EnvironmentProps, params, nil, &environments)
if err != nil {
return environments, err
}

for i := range environments {
if err = d.fillEnvironmentSync(&environments[i]); err != nil {
return environments, err
}
}

return environments, nil
}

func (d *SqlDb) UpdateEnvironment(env db.Environment) error {
Expand All @@ -33,7 +48,12 @@ func (d *SqlDb) UpdateEnvironment(env db.Environment) error {
env.ENV,
env.Password,
env.ID)
return err

if err != nil {
return err
}

return d.saveEnvironmentSync(env)
}

func (d *SqlDb) CreateEnvironment(env db.Environment) (newEnv db.Environment, err error) {
Expand Down Expand Up @@ -62,6 +82,12 @@ func (d *SqlDb) CreateEnvironment(env db.Environment) (newEnv db.Environment, er

newEnv = env
newEnv.ID = insertID

if err = d.saveEnvironmentSync(newEnv); err != nil {
return
}

err = d.fillEnvironmentSync(&newEnv)
return
}

Expand Down Expand Up @@ -90,3 +116,45 @@ func (d *SqlDb) GetEnvironmentSecrets(projectID int, environmentID int) (keys []

return
}

func (d *SqlDb) fillEnvironmentSync(env *db.Environment) error {
sync, err := d.GetEnvironmentSecretSync(env.ID)
if err == db.ErrNotFound {
env.SyncEnabled = false
env.SyncInterval = 0
env.LastSyncedAt = nil
env.LastSyncFailedAt = nil
env.SyncPaths = []db.SecretSyncPath{}
return nil
}
if err != nil {
return err
}
env.SyncEnabled = sync.SyncEnabled
env.SyncInterval = sync.SyncInterval
env.LastSyncedAt = sync.LastSyncedAt
env.LastSyncFailedAt = sync.LastSyncFailedAt
env.SyncPaths = sync.Paths
if env.SyncPaths == nil {
env.SyncPaths = []db.SecretSyncPath{}
}
return nil
}

// saveEnvironmentSync persists sync settings for an environment. Syncs
// require a linked SecretStorage; without one, any pending sync row is
// removed.
func (d *SqlDb) saveEnvironmentSync(env db.Environment) error {
envID := env.ID
sync := db.SecretSync{
ProjectID: env.ProjectID,
EnvironmentID: &envID,
}
if env.SecretStorageID != nil {
sync.StorageID = *env.SecretStorageID
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High — IDOR / cross-project secret storage binding

saveEnvironmentSync copies env.SecretStorageID from the JSON body into sync.StorageID without checking that that storage row belongs to env.ProjectID. A user with access to project A can set secret_storage_id to a storage ID from project B (guessed or observed); the FK only constrains storage_id to exist somewhere in project__secret_storage, not to match the environment's project. project_id on the sync row stays A while storage_id points at B's vault — wrong data boundary and a plausible path to future sync/import logic operating on another tenant's backend once implemented.

Fix: Before persisting, load the storage with GetSecretStorage(env.ProjectID, *env.SecretStorageID) (or equivalent) and reject on ErrNotFound / mismatch.

sync.SyncEnabled = env.SyncEnabled
sync.SyncInterval = env.SyncInterval
sync.Paths = env.SyncPaths
}
return d.SaveSecretSync(sync)
}
3 changes: 3 additions & 0 deletions db/sql/migrations/v2.18.1.err.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
drop table `project__secret_sync_path`;
drop table `project__secret_sync`;
alter table `access_key` drop column `synchronized`;
Loading
Loading