mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
feat: rotate, decrypt, and delete encrypted chat_mcp_servers headers (#29520)
Relates to #25381 `chat_mcp_servers.headers_key_id` has a foreign key to `dbcrypt_keys(active_key_digest)`, but `dbcrypt rotate`, `decrypt` and `delete` didn't know about the table. `rotate` finishes by revoking every old key, so an encrypted header row would fail that revoke with an FK violation, which is the same class of bug as #25381. This PR adds `chat_mcp_servers` to all three commands. `rotate` re-encrypts under the new key (and encrypts plaintext rows with no key ID, like `gitsshkeys`), `decrypt` writes the plaintext back and clears the key ID, and `delete` removes the encrypted rows outright, like `user_secrets`, since a server without its headers can't authenticate anyway. Rows with `{}` headers are never encrypted, so they're left alone. The org-level `mcp_server_configs` gap is still tracked at #25381. I've stacked this on the `mcpclient` PR, though it only depends on the `chat_mcp_servers` table PR, as `gh stack` is linear. Part of CODAGT-1131
This commit is contained in:
@@ -28,6 +28,7 @@ The following database fields are currently encrypted:
|
||||
- `crypto_keys.secret`
|
||||
- `user_secrets.value`
|
||||
- `gitsshkeys.private_key`
|
||||
- `chat_mcp_servers.headers`
|
||||
|
||||
Additional database fields may be encrypted in the future.
|
||||
|
||||
|
||||
@@ -195,6 +195,10 @@ func TestServerDBCrypt(t *testing.T) {
|
||||
require.NoError(t, err, "expected gitsshkey row to remain for user %s", usr.ID)
|
||||
require.Empty(t, sshKey.PrivateKey, "expected private_key to be cleared for user %s", usr.ID)
|
||||
require.False(t, sshKey.PrivateKeyKeyID.Valid, "expected private_key_key_id to be cleared for user %s", usr.ID)
|
||||
|
||||
chatMCPServers, err := db.GetChatMCPServersByChatOwnerID(ctx, usr.ID)
|
||||
require.NoError(t, err, "failed to get chat MCP servers for user %s", usr.ID)
|
||||
require.Empty(t, chatMCPServers)
|
||||
}
|
||||
|
||||
// Validate that the key has been revoked in the database.
|
||||
@@ -243,6 +247,18 @@ func genData(t *testing.T, db database.Store) []database.User {
|
||||
PrivateKey: "private-" + usr.ID.String(),
|
||||
PublicKey: "public-" + usr.ID.String(),
|
||||
})
|
||||
chatModel := dbgen.ChatModelConfig(t, db, database.ChatModelConfig{
|
||||
AIProviderID: uuid.NullUUID{UUID: provider.ID, Valid: true},
|
||||
})
|
||||
chat := dbgen.Chat(t, db, database.Chat{
|
||||
OrganizationID: chatModel.OrganizationID,
|
||||
OwnerID: usr.ID,
|
||||
LastModelConfigID: chatModel.ID,
|
||||
})
|
||||
_ = dbgen.ChatMCPServer(t, db, database.ChatMCPServer{
|
||||
ChatID: chat.ID,
|
||||
Headers: "headers-" + usr.ID.String(),
|
||||
})
|
||||
now := time.Now()
|
||||
_, err := db.UpsertUserAIProviderKey(context.Background(), database.UpsertUserAIProviderKeyParams{
|
||||
ID: uuid.New(),
|
||||
@@ -330,6 +346,12 @@ func requireEncryptedWithCipher(ctx context.Context, t *testing.T, db database.S
|
||||
// Public key is never encrypted.
|
||||
require.Equal(t, "public-"+userID.String(), sshKey.PublicKey)
|
||||
|
||||
chatMCPServers, err := db.GetChatMCPServersByChatOwnerID(ctx, userID)
|
||||
require.NoError(t, err, "failed to get chat MCP servers for user %s", userID)
|
||||
require.Len(t, chatMCPServers, 1)
|
||||
requireEncryptedEquals(t, c, "headers-"+userID.String(), chatMCPServers[0].Headers)
|
||||
require.Equal(t, c.HexDigest(), chatMCPServers[0].HeadersKeyID.String)
|
||||
|
||||
providers, err := db.GetAIProviders(ctx, database.GetAIProvidersParams{
|
||||
IncludeDeleted: true,
|
||||
IncludeDisabled: true,
|
||||
|
||||
@@ -130,6 +130,29 @@ func Rotate(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciphe
|
||||
}
|
||||
}
|
||||
|
||||
chatMCPServers, err := cryptTx.GetChatMCPServersByChatOwnerID(ctx, uid)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get chat MCP servers for user %s: %w", uid, err)
|
||||
}
|
||||
for _, server := range chatMCPServers {
|
||||
trimmed := strings.TrimSpace(server.Headers)
|
||||
switch {
|
||||
case trimmed == "" || trimmed == "{}":
|
||||
log.Debug(ctx, "skipping empty chat MCP server headers", slog.F("user_id", uid), slog.F("chat_id", server.ChatID), slog.F("slug", server.Slug), slog.F("current", idx+1))
|
||||
case server.HeadersKeyID.Valid && server.HeadersKeyID.String == ciphers[0].HexDigest():
|
||||
log.Debug(ctx, "skipping chat MCP server headers", slog.F("user_id", uid), slog.F("chat_id", server.ChatID), slog.F("slug", server.Slug), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest()))
|
||||
default:
|
||||
if err := cryptTx.UpdateEncryptedChatMCPServerHeaders(ctx, database.UpdateEncryptedChatMCPServerHeadersParams{
|
||||
ID: server.ID,
|
||||
Headers: server.Headers,
|
||||
HeadersKeyID: sql.NullString{}, // dbcrypt will re-encrypt
|
||||
}); err != nil {
|
||||
return xerrors.Errorf("rotate chat MCP server headers chat_id=%s slug=%s: %w", server.ChatID, server.Slug, err)
|
||||
}
|
||||
log.Debug(ctx, "rotated chat MCP server headers", slog.F("user_id", uid), slog.F("chat_id", server.ChatID), slog.F("slug", server.Slug), slog.F("current", idx+1), slog.F("cipher", ciphers[0].HexDigest()))
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}, &database.TxOptions{
|
||||
Isolation: sql.LevelRepeatableRead,
|
||||
@@ -338,6 +361,25 @@ func Decrypt(ctx context.Context, log slog.Logger, sqlDB *sql.DB, ciphers []Ciph
|
||||
log.Debug(ctx, "decrypted gitsshkey", slog.F("user_id", uid), slog.F("current", idx+1))
|
||||
}
|
||||
|
||||
chatMCPServers, err := tx.GetChatMCPServersByChatOwnerID(ctx, uid)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get chat MCP servers for user %s: %w", uid, err)
|
||||
}
|
||||
for _, server := range chatMCPServers {
|
||||
if !server.HeadersKeyID.Valid {
|
||||
log.Debug(ctx, "skipping chat MCP server headers", slog.F("user_id", uid), slog.F("chat_id", server.ChatID), slog.F("slug", server.Slug), slog.F("current", idx+1))
|
||||
continue
|
||||
}
|
||||
if err := tx.UpdateEncryptedChatMCPServerHeaders(ctx, database.UpdateEncryptedChatMCPServerHeadersParams{
|
||||
ID: server.ID,
|
||||
Headers: server.Headers,
|
||||
HeadersKeyID: sql.NullString{}, // clear the key ID
|
||||
}); err != nil {
|
||||
return xerrors.Errorf("decrypt chat MCP server headers chat_id=%s slug=%s: %w", server.ChatID, server.Slug, err)
|
||||
}
|
||||
log.Debug(ctx, "decrypted chat MCP server headers", slog.F("user_id", uid), slog.F("chat_id", server.ChatID), slog.F("slug", server.Slug), slog.F("current", idx+1))
|
||||
}
|
||||
|
||||
return nil
|
||||
}, &database.TxOptions{
|
||||
Isolation: sql.LevelRepeatableRead,
|
||||
@@ -432,6 +474,8 @@ DELETE FROM user_ai_provider_keys
|
||||
WHERE api_key_key_id IS NOT NULL;
|
||||
DELETE FROM user_secrets
|
||||
WHERE value_key_id IS NOT NULL;
|
||||
DELETE FROM chat_mcp_servers
|
||||
WHERE headers_key_id IS NOT NULL;
|
||||
-- gitsshkeys has no delete path in product code: rows are inserted on
|
||||
-- user creation and only ever mutated by regenerate. dbcrypt's 'delete'
|
||||
-- command is the one operation that needs to wipe encrypted content,
|
||||
|
||||
Reference in New Issue
Block a user