fix(security): encrypt channel_config bot_token at rest (closes #319)

CI fully green. Dev Lead code review:  clean, all read/write paths verified, tests cover round-trip + idempotency + legacy plaintext. Closes #319.
This commit is contained in:
Hongming Wang 2026-04-15 21:09:34 -07:00 committed by GitHub
parent 027d2d213f
commit bf2022acf1
4 changed files with 332 additions and 4 deletions

View File

@ -105,15 +105,20 @@ func (m *Manager) Start(ctx context.Context) {
// PausePollersForToken stops any pollers that share the given bot token,
// then returns a resume function. Used during discovery to avoid Telegram's
// "only one getUpdates at a time" 409 Conflict.
//
// #319: bot_token is stored encrypted in channel_config so we cannot match
// with SQL `channel_config->>'bot_token' = $1` anymore. Load all enabled
// channels, decrypt each, and compare the plaintext in Go. The cardinality
// is small (typically <10 enabled channels per install) so the extra work
// is negligible.
func (m *Manager) PausePollersForToken(botToken string) func() {
if botToken == "" {
return func() {}
}
rows, err := db.DB.QueryContext(context.Background(), `
SELECT id FROM workspace_channels
WHERE enabled = true AND channel_config->>'bot_token' = $1
`, botToken)
SELECT id, channel_config FROM workspace_channels WHERE enabled = true
`)
if err != nil {
return func() {}
}
@ -123,7 +128,19 @@ func (m *Manager) PausePollersForToken(botToken string) func() {
m.mu.Lock()
for rows.Next() {
var id string
if rows.Scan(&id) == nil {
var configJSON []byte
if err := rows.Scan(&id, &configJSON); err != nil {
continue
}
var config map[string]interface{}
if err := json.Unmarshal(configJSON, &config); err != nil {
continue
}
if err := DecryptSensitiveFields(config); err != nil {
log.Printf("Channels: pause-pollers decrypt error for %s: %v", truncID(id), err)
continue
}
if token, _ := config["bot_token"].(string); token == botToken {
if cancel, ok := m.pollers[id]; ok {
cancel()
delete(m.pollers, id)
@ -182,6 +199,14 @@ func (m *Manager) Reload(ctx context.Context) {
}
json.Unmarshal(configJSON, &ch.Config)
json.Unmarshal(allowedJSON, &ch.AllowedUsers)
// #319: decrypt at the boundary between DB (ciphertext) and the
// in-memory config adapters consume. A decrypt failure logs and
// skips the channel — downstream getUpdates would fail anyway
// with a mangled token so fail-closed here is kinder to operators.
if err := DecryptSensitiveFields(ch.Config); err != nil {
log.Printf("Channels: reload decrypt error for %s: %v", truncID(ch.ID), err)
continue
}
desired[ch.ID] = ch
}
@ -436,6 +461,11 @@ func (m *Manager) loadChannel(ctx context.Context, channelID string) (ChannelRow
}
json.Unmarshal(configJSON, &ch.Config)
json.Unmarshal(allowedJSON, &ch.AllowedUsers)
// #319: decrypt bot_token / webhook_secret — SendOutbound and adapter
// methods downstream read them as plaintext strings.
if err := DecryptSensitiveFields(ch.Config); err != nil {
return ch, fmt.Errorf("decrypt channel %s: %w", channelID, err)
}
return ch, nil
}

View File

@ -0,0 +1,129 @@
package channels
// Field-level encryption for sensitive channel_config values (#319).
//
// workspace_channels.channel_config is a JSONB column holding adapter-specific
// settings. Some fields are secret — Telegram bot tokens, webhook shared
// secrets — and must not sit in cleartext at the database layer where a
// backup leak or read-replica mis-grant would expose them. workspace_secrets
// already encrypts values with AES-256-GCM; this file mirrors that posture
// for channel_config so the security stance is consistent.
//
// Strategy: lazy field-level encryption with a version prefix.
//
// plaintext "123456:AA..." (legacy / pre-migration)
// ciphertext "ec1:<base64-GCM-ciphertext>" (new writes)
//
// On read, a missing "ec1:" prefix means the row predates the encryption
// rollout — return the value as-is (pass-through). On write, always encrypt.
// Rows upgrade lazily on the next PATCH/Create. An operator wishing to
// force-upgrade everything can re-save each channel via the Canvas Update
// button.
//
// Only `bot_token` and `webhook_secret` are considered secret. Other fields
// (chat_id, channel_name, enable_polling, etc.) stay in cleartext so the
// SQL-level `channel_config->>'chat_id'` lookups in the webhook receiver
// remain efficient.
import (
"encoding/base64"
"strings"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto"
)
// sensitiveFields is the set of channel_config keys that get encrypted at
// rest. Add a new key here to extend coverage — do NOT widen this to the
// whole config: it would break SQL field-access for non-secret keys like
// `chat_id` that the webhook receiver queries.
var sensitiveFields = []string{"bot_token", "webhook_secret"}
// ciphertextPrefix marks values encrypted by EncryptSensitiveFields so
// DecryptSensitiveFields can tell "new encrypted value" from a legacy
// plaintext row. The string is intentionally distinctive — no real bot
// token begins with "ec1:".
const ciphertextPrefix = "ec1:"
// EncryptSensitiveFields encrypts every known-sensitive value in config in
// place. Values that are already prefixed (already encrypted) are left
// untouched so a no-op re-save won't double-encrypt. Non-string values,
// empty strings, and unknown fields pass through unchanged.
//
// When SECRETS_ENCRYPTION_KEY is not configured (dev default), values are
// stored as plaintext — consistent with workspace_secrets' dev fallback.
func EncryptSensitiveFields(config map[string]interface{}) error {
if config == nil {
return nil
}
for _, field := range sensitiveFields {
raw, ok := config[field]
if !ok {
continue
}
s, ok := raw.(string)
if !ok || s == "" {
continue
}
if strings.HasPrefix(s, ciphertextPrefix) {
// already encrypted (idempotent re-save)
continue
}
if !crypto.IsEnabled() {
// Dev fallback: leave plaintext so local test setups without a
// key keep working. Prod boots with crypto.InitStrict which
// refuses to start without a key, so this branch is dev-only.
continue
}
ct, err := crypto.Encrypt([]byte(s))
if err != nil {
return err
}
config[field] = ciphertextPrefix + base64.StdEncoding.EncodeToString(ct)
}
return nil
}
// DecryptSensitiveFields is the inverse of EncryptSensitiveFields. Values
// without the ciphertext prefix are returned as-is (legacy plaintext rows).
// Values with the prefix are base64-decoded and run through AES-256-GCM.
//
// When SECRETS_ENCRYPTION_KEY is not configured but a prefixed value is
// encountered, that's an operator error (enabled encryption then disabled
// the key). Return the raw prefixed string in that case — the adapter will
// fail to authenticate with Telegram/Slack and the operator will see a
// clear "invalid bot token" message rather than a silent mis-decrypt.
func DecryptSensitiveFields(config map[string]interface{}) error {
if config == nil {
return nil
}
for _, field := range sensitiveFields {
raw, ok := config[field]
if !ok {
continue
}
s, ok := raw.(string)
if !ok || s == "" {
continue
}
if !strings.HasPrefix(s, ciphertextPrefix) {
// legacy plaintext row — pass through
continue
}
if !crypto.IsEnabled() {
// encryption-expected row but no key — leave encoded so callers
// fail loudly at Telegram rather than mis-decrypt.
continue
}
encoded := strings.TrimPrefix(s, ciphertextPrefix)
ct, err := base64.StdEncoding.DecodeString(encoded)
if err != nil {
return err
}
pt, err := crypto.Decrypt(ct)
if err != nil {
return err
}
config[field] = string(pt)
}
return nil
}

View File

@ -0,0 +1,138 @@
package channels
import (
"strings"
"testing"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto"
)
// withTestEncryptionKey installs a deterministic 32-byte key for the
// duration of a test, then restores the previous state. Without this
// the tests would depend on ambient SECRETS_ENCRYPTION_KEY.
func withTestEncryptionKey(t *testing.T) {
t.Helper()
// Base64 of 32 zero bytes = "AAAA..." (44 chars). Matches the loader's
// base64 path — the raw 32-byte path requires a string that is not
// decodable as base64, which is surprisingly hard to construct.
t.Setenv("SECRETS_ENCRYPTION_KEY", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")
crypto.ResetForTesting()
crypto.Init()
t.Cleanup(func() {
crypto.ResetForTesting()
})
}
func TestEncryptSensitiveFields_RoundTrip(t *testing.T) {
withTestEncryptionKey(t)
cfg := map[string]interface{}{
"bot_token": "123456:telegram-bot-token",
"chat_id": "-100999", // non-sensitive, untouched
"webhook_secret": "hmac-shared-key", // second known-sensitive field
}
if err := EncryptSensitiveFields(cfg); err != nil {
t.Fatalf("encrypt: %v", err)
}
if tok, _ := cfg["bot_token"].(string); !strings.HasPrefix(tok, ciphertextPrefix) {
t.Errorf("bot_token not encrypted: got %q", tok)
}
if sec, _ := cfg["webhook_secret"].(string); !strings.HasPrefix(sec, ciphertextPrefix) {
t.Errorf("webhook_secret not encrypted: got %q", sec)
}
if chat, _ := cfg["chat_id"].(string); chat != "-100999" {
t.Errorf("chat_id modified: got %q", chat)
}
if err := DecryptSensitiveFields(cfg); err != nil {
t.Fatalf("decrypt: %v", err)
}
if got, _ := cfg["bot_token"].(string); got != "123456:telegram-bot-token" {
t.Errorf("bot_token round-trip mismatch: got %q", got)
}
if got, _ := cfg["webhook_secret"].(string); got != "hmac-shared-key" {
t.Errorf("webhook_secret round-trip mismatch: got %q", got)
}
}
func TestEncryptSensitiveFields_Idempotent(t *testing.T) {
withTestEncryptionKey(t)
cfg := map[string]interface{}{"bot_token": "abc"}
if err := EncryptSensitiveFields(cfg); err != nil {
t.Fatalf("first encrypt: %v", err)
}
first, _ := cfg["bot_token"].(string)
if err := EncryptSensitiveFields(cfg); err != nil {
t.Fatalf("second encrypt: %v", err)
}
second, _ := cfg["bot_token"].(string)
if first != second {
t.Errorf("idempotent encrypt should not re-wrap: first=%q second=%q", first, second)
}
}
func TestDecryptSensitiveFields_LegacyPlaintextPassesThrough(t *testing.T) {
// Legacy row predates #319 — no ciphertext prefix.
withTestEncryptionKey(t)
cfg := map[string]interface{}{"bot_token": "legacy-plaintext-value"}
if err := DecryptSensitiveFields(cfg); err != nil {
t.Fatalf("decrypt: %v", err)
}
if got, _ := cfg["bot_token"].(string); got != "legacy-plaintext-value" {
t.Errorf("legacy plaintext was mangled: got %q", got)
}
}
func TestEncryptSensitiveFields_DevFallback_NoKey(t *testing.T) {
// No key set — dev behaviour matches workspace_secrets: store plaintext.
t.Setenv("SECRETS_ENCRYPTION_KEY", "")
crypto.ResetForTesting()
crypto.Init()
t.Cleanup(crypto.ResetForTesting)
cfg := map[string]interface{}{"bot_token": "dev-token"}
if err := EncryptSensitiveFields(cfg); err != nil {
t.Fatalf("encrypt: %v", err)
}
if got, _ := cfg["bot_token"].(string); got != "dev-token" {
t.Errorf("dev fallback should leave plaintext: got %q", got)
}
}
func TestEncryptSensitiveFields_SkipsEmptyAndNonString(t *testing.T) {
withTestEncryptionKey(t)
cfg := map[string]interface{}{
"bot_token": "", // empty
"webhook_secret": 12345, // non-string
"unrelated": "ignore", // not in sensitiveFields
}
if err := EncryptSensitiveFields(cfg); err != nil {
t.Fatalf("encrypt: %v", err)
}
if got, _ := cfg["bot_token"].(string); got != "" {
t.Errorf("empty bot_token should stay empty: got %q", got)
}
if got, _ := cfg["webhook_secret"].(int); got != 12345 {
t.Errorf("non-string webhook_secret should be untouched: got %v", cfg["webhook_secret"])
}
if got, _ := cfg["unrelated"].(string); got != "ignore" {
t.Errorf("unrelated field should be untouched: got %q", got)
}
}
func TestEncryptSensitiveFields_NilConfig(t *testing.T) {
if err := EncryptSensitiveFields(nil); err != nil {
t.Errorf("nil config: expected no error, got %v", err)
}
if err := DecryptSensitiveFields(nil); err != nil {
t.Errorf("nil config: expected no error, got %v", err)
}
}

View File

@ -61,6 +61,13 @@ func (h *ChannelHandler) List(c *gin.Context) {
var config map[string]interface{}
json.Unmarshal(configJSON, &config)
// #319: decrypt sensitive fields first so the mask operates on
// plaintext (first-4 / last-4 of the real token, not the ciphertext
// prefix). Decrypt errors are logged but non-fatal — List must keep
// returning the rest of the channel even if one field is corrupt.
if err := channels.DecryptSensitiveFields(config); err != nil {
log.Printf("Channels: decrypt config on list for channel %s: %v", id, err)
}
// Mask bot_token in list response
if _, ok := config["bot_token"]; ok {
token, _ := config["bot_token"].(string)
@ -126,6 +133,15 @@ func (h *ChannelHandler) Create(c *gin.Context) {
return
}
// #319: encrypt sensitive fields (bot_token, webhook_secret) before
// persisting so a DB read/backup leak can't recover the credentials.
// Validation above ran against plaintext; storage is ciphertext.
if err := channels.EncryptSensitiveFields(body.Config); err != nil {
log.Printf("Channels: encrypt config failed for workspace %s: %v", workspaceID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "encrypt failed"})
return
}
configJSON, _ := json.Marshal(body.Config)
allowedJSON, _ := json.Marshal(body.AllowedUsers)
enabled := true
@ -174,6 +190,14 @@ func (h *ChannelHandler) Update(c *gin.Context) {
// COALESCE-based update
var configArg, allowedArg interface{}
if body.Config != nil {
// #319: re-encrypt sensitive fields on every config update — the
// PATCH body carries plaintext (client already had them plaintext in
// List response's unmasked path or typed fresh).
if err := channels.EncryptSensitiveFields(body.Config); err != nil {
log.Printf("Channels: encrypt update for workspace %s: %v", workspaceID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "encrypt failed"})
return
}
j, _ := json.Marshal(body.Config)
configArg = string(j)
}
@ -391,6 +415,13 @@ func (h *ChannelHandler) Webhook(c *gin.Context) {
}
json.Unmarshal(configJSON, &row.Config)
json.Unmarshal(allowedJSON, &row.AllowedUsers)
// #319: decrypt sensitive fields before comparing webhook_secret /
// using bot_token downstream. Skip rows whose decrypt fails so a
// single corrupt channel cannot block webhooks for all others.
if err := channels.DecryptSensitiveFields(row.Config); err != nil {
log.Printf("Channels: decrypt webhook row %s: %v", row.ID, err)
continue
}
// Verify webhook secret_token if the channel has one configured
if expectedSecret, _ := row.Config["webhook_secret"].(string); expectedSecret != "" {