From 7c949e653cc2e16b72b595baba2bd8361e5a6bff Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Fri, 15 May 2026 02:39:07 +0530 Subject: [PATCH 1/2] Don't fatal on encryption_key rotation --- internal/context_link/context_link.go | 28 +++++------ internal/inbox/inbox.go | 17 ++++--- internal/inbox/models/models.go | 22 ++++---- internal/oidc/oidc.go | 72 +++++++++++---------------- internal/webhook/encryption.go | 20 ++++---- internal/webhook/webhook.go | 15 ++---- 6 files changed, 75 insertions(+), 99 deletions(-) diff --git a/internal/context_link/context_link.go b/internal/context_link/context_link.go index 6eaa5fb8..374ff8c9 100644 --- a/internal/context_link/context_link.go +++ b/internal/context_link/context_link.go @@ -83,9 +83,7 @@ func (m *Manager) Get(id int) (models.ContextLink, error) { m.lo.Error("error fetching context link", "error", err) return link, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - if err := m.decryptLink(&link); err != nil { - return link, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) - } + m.decryptLink(&link) return link, nil } @@ -114,9 +112,7 @@ func (m *Manager) Create(link models.ContextLink) (models.ContextLink, error) { return models.ContextLink{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - if err := m.decryptLink(&result); err != nil { - m.lo.Error("error decrypting context link secret after creation", "id", result.ID, "error", err) - } + m.decryptLink(&result) return result, nil } @@ -144,9 +140,7 @@ func (m *Manager) Update(id int, link models.ContextLink) (models.ContextLink, e return models.ContextLink{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - if err := m.decryptLink(&result); err != nil { - m.lo.Error("error decrypting context link secret after update", "id", result.ID, "error", err) - } + m.decryptLink(&result) return result, nil } @@ -242,20 +236,22 @@ func (m *Manager) encryptSecret(secret string) (string, error) { return encrypted, nil } -func (m *Manager) decryptLink(link *models.ContextLink) error { +// Decrypt failures clear the secret so the app stays usable across encryption_key rotation. +func (m *Manager) decryptLink(link *models.ContextLink) { + if link.Secret == "" { + return + } decrypted, err := crypto.Decrypt(link.Secret, m.encryptionKey) if err != nil { - m.lo.Error("error decrypting context link secret", "id", link.ID, "error", err) - return err + m.lo.Error("error decrypting context link secret, clearing field", "id", link.ID, "error", err) + link.Secret = "" + return } link.Secret = decrypted - return nil } func (m *Manager) decryptLinks(links []models.ContextLink) { for i := range links { - if err := m.decryptLink(&links[i]); err != nil { - continue - } + m.decryptLink(&links[i]) } } diff --git a/internal/inbox/inbox.go b/internal/inbox/inbox.go index f630e15b..2e901336 100644 --- a/internal/inbox/inbox.go +++ b/internal/inbox/inbox.go @@ -641,7 +641,7 @@ func (m *Manager) encryptInboxConfig(config json.RawMessage) (json.RawMessage, e return encrypted, nil } -// decryptInboxConfig decrypts sensitive fields in the inbox config JSON. +// Decrypt failures clear the field so the app stays usable across encryption_key rotation. func (m *Manager) decryptInboxConfig(config json.RawMessage) (json.RawMessage, error) { if len(config) == 0 { return config, nil @@ -652,14 +652,15 @@ func (m *Manager) decryptInboxConfig(config json.RawMessage) (json.RawMessage, e return nil, fmt.Errorf("unmarshalling config: %w", err) } - // Decrypt SMTP passwords if smtpSlice, ok := cfg["smtp"].([]any); ok { for i, smtpItem := range smtpSlice { if smtpMap, ok := smtpItem.(map[string]any); ok { if password, ok := smtpMap["password"].(string); ok && password != "" { decrypted, err := crypto.Decrypt(password, m.encryptionKey) if err != nil { - return nil, fmt.Errorf("decrypting SMTP password at index %d: %w", i, err) + m.lo.Error("error decrypting SMTP password, clearing field", "index", i, "error", err) + smtpMap["password"] = "" + continue } smtpMap["password"] = decrypted } @@ -667,14 +668,15 @@ func (m *Manager) decryptInboxConfig(config json.RawMessage) (json.RawMessage, e } } - // Decrypt IMAP passwords if imapSlice, ok := cfg["imap"].([]any); ok { for i, imapItem := range imapSlice { if imapMap, ok := imapItem.(map[string]any); ok { if password, ok := imapMap["password"].(string); ok && password != "" { decrypted, err := crypto.Decrypt(password, m.encryptionKey) if err != nil { - return nil, fmt.Errorf("decrypting IMAP password at index %d: %w", i, err) + m.lo.Error("error decrypting IMAP password, clearing field", "index", i, "error", err) + imapMap["password"] = "" + continue } imapMap["password"] = decrypted } @@ -682,14 +684,15 @@ func (m *Manager) decryptInboxConfig(config json.RawMessage) (json.RawMessage, e } } - // Decrypt OAuth fields if present if oauthMap, ok := cfg["oauth"].(map[string]any); ok { fields := []string{"client_secret", "access_token", "refresh_token"} for _, fieldName := range fields { if fieldValue, ok := oauthMap[fieldName].(string); ok && fieldValue != "" { decrypted, err := crypto.Decrypt(fieldValue, m.encryptionKey) if err != nil { - return nil, fmt.Errorf("decrypting OAuth %s: %w", fieldName, err) + m.lo.Error("error decrypting OAuth field, clearing field", "field", fieldName, "error", err) + oauthMap[fieldName] = "" + continue } oauthMap[fieldName] = decrypted } diff --git a/internal/inbox/models/models.go b/internal/inbox/models/models.go index b646af35..ef54f327 100644 --- a/internal/inbox/models/models.go +++ b/internal/inbox/models/models.go @@ -92,7 +92,7 @@ type IMAPConfig struct { TLSSkipVerify bool `json:"tls_skip_verify"` } -// ClearPasswords masks all config passwords +// Empty fields stay empty so admins can see when a credential needs re-entry after key rotation. func (m *Inbox) ClearPasswords() error { switch m.Channel { case "email": @@ -103,29 +103,32 @@ func (m *Inbox) ClearPasswords() error { dummyPassword := strings.Repeat(stringutil.PasswordDummy, 10) - // Clear IMAP passwords if imapSlice, ok := cfg["imap"].([]interface{}); ok { for _, imapItem := range imapSlice { if imapMap, ok := imapItem.(map[string]interface{}); ok { - imapMap["password"] = dummyPassword + if pw, _ := imapMap["password"].(string); pw != "" { + imapMap["password"] = dummyPassword + } } } } - // Clear SMTP passwords if smtpSlice, ok := cfg["smtp"].([]interface{}); ok { for _, smtpItem := range smtpSlice { if smtpMap, ok := smtpItem.(map[string]interface{}); ok { - smtpMap["password"] = dummyPassword + if pw, _ := smtpMap["password"].(string); pw != "" { + smtpMap["password"] = dummyPassword + } } } } - // Clear OAuth sensitive fields if present if oauthMap, ok := cfg["oauth"].(map[string]interface{}); ok { - oauthMap["access_token"] = dummyPassword - oauthMap["refresh_token"] = dummyPassword - oauthMap["client_secret"] = dummyPassword + for _, field := range []string{"access_token", "refresh_token", "client_secret"} { + if v, _ := oauthMap[field].(string); v != "" { + oauthMap[field] = dummyPassword + } + } } clearedConfig, err := json.Marshal(cfg) @@ -135,7 +138,6 @@ func (m *Inbox) ClearPasswords() error { m.Config = clearedConfig case "livechat": - // Mask the secret field for livechat if m.Secret.Valid && m.Secret.String != "" { m.Secret = null.StringFrom(strings.Repeat(stringutil.PasswordDummy, 10)) } diff --git a/internal/oidc/oidc.go b/internal/oidc/oidc.go index 5ce31588..49ff2e4a 100644 --- a/internal/oidc/oidc.go +++ b/internal/oidc/oidc.go @@ -77,12 +77,8 @@ func (o *Manager) Get(id int) (models.OIDC, error) { return oidc, envelope.NewError(envelope.GeneralError, o.i18n.T("globals.messages.somethingWentWrong"), nil) } - // Decrypt sensitive fields - if err := o.decryptOIDC(&oidc); err != nil { - return models.OIDC{}, envelope.NewError(envelope.GeneralError, o.i18n.T("globals.messages.somethingWentWrong"), nil) - } + o.decryptOIDC(&oidc) - // Set logo and redirect URL. oidc.SetProviderLogo() rootURL, err := o.setting.GetAppRootURL() if err != nil { @@ -106,10 +102,7 @@ func (o *Manager) GetAll() ([]models.OIDC, error) { return nil, err } - // Decrypt sensitive fields - if err := o.decryptOIDCSlice(oidc); err != nil { - return nil, envelope.NewError(envelope.GeneralError, o.i18n.T("globals.messages.somethingWentWrong"), nil) - } + o.decryptOIDCSlice(oidc) // Set logo and redirect URL for each record for i := range oidc { @@ -133,10 +126,7 @@ func (o *Manager) Create(oidc models.OIDC) (models.OIDC, error) { return models.OIDC{}, envelope.NewError(envelope.GeneralError, o.i18n.T("globals.messages.somethingWentWrong"), nil) } - // Decrypt fields before returning (ignore errors as these are non-critical for creation response) - if err := o.decryptOIDC(&createdOIDC); err != nil { - o.lo.Error("error decrypting after creation", "error", err) - } + o.decryptOIDC(&createdOIDC) return createdOIDC, nil } @@ -165,10 +155,7 @@ func (o *Manager) Update(id int, oidc models.OIDC) (models.OIDC, error) { return models.OIDC{}, envelope.NewError(envelope.GeneralError, o.i18n.T("globals.messages.somethingWentWrong"), nil) } - // Decrypt fields before returning (ignore errors as these are non-critical for update response) - if err := o.decryptOIDC(&updatedOIDC); err != nil { - o.lo.Error("error decrypting after update", "error", err) - } + o.decryptOIDC(&updatedOIDC) return updatedOIDC, nil } @@ -200,32 +187,31 @@ func (o *Manager) encryptOIDC(clientID, clientSecret string) (encClientID, encCl return encClientID, encClientSecret, nil } -// decryptOIDC decrypts sensitive OIDC fields in-place. -// Returns an error if decryption of any field fails. -func (o *Manager) decryptOIDC(oidc *models.OIDC) error { - var err error - oidc.ClientID, err = crypto.Decrypt(oidc.ClientID, o.encryptionKey) - if err != nil { - o.lo.Error("error decrypting client_id", "error", err, "oidc_id", oidc.ID) - return err - } - - oidc.ClientSecret, err = crypto.Decrypt(oidc.ClientSecret, o.encryptionKey) - if err != nil { - o.lo.Error("error decrypting client_secret", "error", err, "oidc_id", oidc.ID) - return err - } - - return nil -} - -// decryptOIDCSlice decrypts sensitive fields for all OIDC records in a slice. -// Returns an error if decryption of any record fails. -func (o *Manager) decryptOIDCSlice(oidcs []models.OIDC) error { - for i := range oidcs { - if err := o.decryptOIDC(&oidcs[i]); err != nil { - return err +// Decrypt failures clear the field so the app stays usable across encryption_key rotation. +func (o *Manager) decryptOIDC(oidc *models.OIDC) { + if oidc.ClientID != "" { + decrypted, err := crypto.Decrypt(oidc.ClientID, o.encryptionKey) + if err != nil { + o.lo.Error("error decrypting client_id, clearing field", "error", err, "oidc_id", oidc.ID) + oidc.ClientID = "" + } else { + oidc.ClientID = decrypted + } + } + + if oidc.ClientSecret != "" { + decrypted, err := crypto.Decrypt(oidc.ClientSecret, o.encryptionKey) + if err != nil { + o.lo.Error("error decrypting client_secret, clearing field", "error", err, "oidc_id", oidc.ID) + oidc.ClientSecret = "" + } else { + oidc.ClientSecret = decrypted } } - return nil +} + +func (o *Manager) decryptOIDCSlice(oidcs []models.OIDC) { + for i := range oidcs { + o.decryptOIDC(&oidcs[i]) + } } diff --git a/internal/webhook/encryption.go b/internal/webhook/encryption.go index f912be9c..827e6a34 100644 --- a/internal/webhook/encryption.go +++ b/internal/webhook/encryption.go @@ -15,24 +15,22 @@ func (m *Manager) encryptSecret(secret string) (string, error) { return encrypted, nil } -// decryptWebhook decrypts webhook secret in-place. -func (m *Manager) decryptWebhook(webhook *models.Webhook) error { +// Decrypt failures clear the secret so the app stays usable across encryption_key rotation. +func (m *Manager) decryptWebhook(webhook *models.Webhook) { + if webhook.Secret == "" { + return + } decrypted, err := crypto.Decrypt(webhook.Secret, m.encryptionKey) if err != nil { - m.lo.Error("error decrypting webhook secret", "webhook_id", webhook.ID, "error", err) - return err + m.lo.Error("error decrypting webhook secret, clearing field", "webhook_id", webhook.ID, "error", err) + webhook.Secret = "" + return } - webhook.Secret = decrypted - return nil } -// decryptWebhooks decrypts secrets for a slice of webhooks. func (m *Manager) decryptWebhooks(webhooks []models.Webhook) { for i := range webhooks { - if err := m.decryptWebhook(&webhooks[i]); err != nil { - m.lo.Error("error decrypting webhook secret", "webhook_id", webhooks[i].ID, "error", err) - continue - } + m.decryptWebhook(&webhooks[i]) } } diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go index 005ffe87..08eed415 100644 --- a/internal/webhook/webhook.go +++ b/internal/webhook/webhook.go @@ -143,10 +143,7 @@ func (m *Manager) Get(id int) (models.Webhook, error) { return webhook, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - // Decrypt secret - if err := m.decryptWebhook(&webhook); err != nil { - return webhook, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) - } + m.decryptWebhook(&webhook) return webhook, nil } @@ -169,10 +166,7 @@ func (m *Manager) Create(webhook models.Webhook) (models.Webhook, error) { return models.Webhook{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - // Decrypt secret before returning (ignore errors as non-critical) - if err := m.decryptWebhook(&result); err != nil { - m.lo.Error("error decrypting webhook secret after creation", "webhook_id", result.ID, "error", err) - } + m.decryptWebhook(&result) return result, nil } @@ -204,10 +198,7 @@ func (m *Manager) Update(id int, webhook models.Webhook) (models.Webhook, error) return models.Webhook{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - // Decrypt secret before returning (ignore errors as non-critical) - if err := m.decryptWebhook(&result); err != nil { - m.lo.Error("error decrypting webhook secret after update", "webhook_id", result.ID, "error", err) - } + m.decryptWebhook(&result) return result, nil } From 3e1c6471ea97370ebf06f672a19e8b65587d380b Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Fri, 15 May 2026 10:36:13 +0530 Subject: [PATCH 2/2] Scope encryption_key rotation fix to boot path --- internal/context_link/context_link.go | 28 +++++++++++++++------------ internal/inbox/models/models.go | 22 ++++++++++----------- internal/webhook/encryption.go | 20 ++++++++++--------- internal/webhook/webhook.go | 15 +++++++++++--- 4 files changed, 49 insertions(+), 36 deletions(-) diff --git a/internal/context_link/context_link.go b/internal/context_link/context_link.go index 374ff8c9..6eaa5fb8 100644 --- a/internal/context_link/context_link.go +++ b/internal/context_link/context_link.go @@ -83,7 +83,9 @@ func (m *Manager) Get(id int) (models.ContextLink, error) { m.lo.Error("error fetching context link", "error", err) return link, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.decryptLink(&link) + if err := m.decryptLink(&link); err != nil { + return link, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) + } return link, nil } @@ -112,7 +114,9 @@ func (m *Manager) Create(link models.ContextLink) (models.ContextLink, error) { return models.ContextLink{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.decryptLink(&result) + if err := m.decryptLink(&result); err != nil { + m.lo.Error("error decrypting context link secret after creation", "id", result.ID, "error", err) + } return result, nil } @@ -140,7 +144,9 @@ func (m *Manager) Update(id int, link models.ContextLink) (models.ContextLink, e return models.ContextLink{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.decryptLink(&result) + if err := m.decryptLink(&result); err != nil { + m.lo.Error("error decrypting context link secret after update", "id", result.ID, "error", err) + } return result, nil } @@ -236,22 +242,20 @@ func (m *Manager) encryptSecret(secret string) (string, error) { return encrypted, nil } -// Decrypt failures clear the secret so the app stays usable across encryption_key rotation. -func (m *Manager) decryptLink(link *models.ContextLink) { - if link.Secret == "" { - return - } +func (m *Manager) decryptLink(link *models.ContextLink) error { decrypted, err := crypto.Decrypt(link.Secret, m.encryptionKey) if err != nil { - m.lo.Error("error decrypting context link secret, clearing field", "id", link.ID, "error", err) - link.Secret = "" - return + m.lo.Error("error decrypting context link secret", "id", link.ID, "error", err) + return err } link.Secret = decrypted + return nil } func (m *Manager) decryptLinks(links []models.ContextLink) { for i := range links { - m.decryptLink(&links[i]) + if err := m.decryptLink(&links[i]); err != nil { + continue + } } } diff --git a/internal/inbox/models/models.go b/internal/inbox/models/models.go index ef54f327..b646af35 100644 --- a/internal/inbox/models/models.go +++ b/internal/inbox/models/models.go @@ -92,7 +92,7 @@ type IMAPConfig struct { TLSSkipVerify bool `json:"tls_skip_verify"` } -// Empty fields stay empty so admins can see when a credential needs re-entry after key rotation. +// ClearPasswords masks all config passwords func (m *Inbox) ClearPasswords() error { switch m.Channel { case "email": @@ -103,32 +103,29 @@ func (m *Inbox) ClearPasswords() error { dummyPassword := strings.Repeat(stringutil.PasswordDummy, 10) + // Clear IMAP passwords if imapSlice, ok := cfg["imap"].([]interface{}); ok { for _, imapItem := range imapSlice { if imapMap, ok := imapItem.(map[string]interface{}); ok { - if pw, _ := imapMap["password"].(string); pw != "" { - imapMap["password"] = dummyPassword - } + imapMap["password"] = dummyPassword } } } + // Clear SMTP passwords if smtpSlice, ok := cfg["smtp"].([]interface{}); ok { for _, smtpItem := range smtpSlice { if smtpMap, ok := smtpItem.(map[string]interface{}); ok { - if pw, _ := smtpMap["password"].(string); pw != "" { - smtpMap["password"] = dummyPassword - } + smtpMap["password"] = dummyPassword } } } + // Clear OAuth sensitive fields if present if oauthMap, ok := cfg["oauth"].(map[string]interface{}); ok { - for _, field := range []string{"access_token", "refresh_token", "client_secret"} { - if v, _ := oauthMap[field].(string); v != "" { - oauthMap[field] = dummyPassword - } - } + oauthMap["access_token"] = dummyPassword + oauthMap["refresh_token"] = dummyPassword + oauthMap["client_secret"] = dummyPassword } clearedConfig, err := json.Marshal(cfg) @@ -138,6 +135,7 @@ func (m *Inbox) ClearPasswords() error { m.Config = clearedConfig case "livechat": + // Mask the secret field for livechat if m.Secret.Valid && m.Secret.String != "" { m.Secret = null.StringFrom(strings.Repeat(stringutil.PasswordDummy, 10)) } diff --git a/internal/webhook/encryption.go b/internal/webhook/encryption.go index 827e6a34..f912be9c 100644 --- a/internal/webhook/encryption.go +++ b/internal/webhook/encryption.go @@ -15,22 +15,24 @@ func (m *Manager) encryptSecret(secret string) (string, error) { return encrypted, nil } -// Decrypt failures clear the secret so the app stays usable across encryption_key rotation. -func (m *Manager) decryptWebhook(webhook *models.Webhook) { - if webhook.Secret == "" { - return - } +// decryptWebhook decrypts webhook secret in-place. +func (m *Manager) decryptWebhook(webhook *models.Webhook) error { decrypted, err := crypto.Decrypt(webhook.Secret, m.encryptionKey) if err != nil { - m.lo.Error("error decrypting webhook secret, clearing field", "webhook_id", webhook.ID, "error", err) - webhook.Secret = "" - return + m.lo.Error("error decrypting webhook secret", "webhook_id", webhook.ID, "error", err) + return err } + webhook.Secret = decrypted + return nil } +// decryptWebhooks decrypts secrets for a slice of webhooks. func (m *Manager) decryptWebhooks(webhooks []models.Webhook) { for i := range webhooks { - m.decryptWebhook(&webhooks[i]) + if err := m.decryptWebhook(&webhooks[i]); err != nil { + m.lo.Error("error decrypting webhook secret", "webhook_id", webhooks[i].ID, "error", err) + continue + } } } diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go index 08eed415..005ffe87 100644 --- a/internal/webhook/webhook.go +++ b/internal/webhook/webhook.go @@ -143,7 +143,10 @@ func (m *Manager) Get(id int) (models.Webhook, error) { return webhook, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.decryptWebhook(&webhook) + // Decrypt secret + if err := m.decryptWebhook(&webhook); err != nil { + return webhook, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) + } return webhook, nil } @@ -166,7 +169,10 @@ func (m *Manager) Create(webhook models.Webhook) (models.Webhook, error) { return models.Webhook{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.decryptWebhook(&result) + // Decrypt secret before returning (ignore errors as non-critical) + if err := m.decryptWebhook(&result); err != nil { + m.lo.Error("error decrypting webhook secret after creation", "webhook_id", result.ID, "error", err) + } return result, nil } @@ -198,7 +204,10 @@ func (m *Manager) Update(id int, webhook models.Webhook) (models.Webhook, error) return models.Webhook{}, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.decryptWebhook(&result) + // Decrypt secret before returning (ignore errors as non-critical) + if err := m.decryptWebhook(&result); err != nil { + m.lo.Error("error decrypting webhook secret after update", "webhook_id", result.ID, "error", err) + } return result, nil }