mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: grant chatd ActionReadPersonal on User and parallelize runChat DB calls (#22970)
## Problem
1. **Personal behavior prompt not applied**: The chatd background worker
was missing `ActionReadPersonal` on `ResourceUser` in its RBAC subject.
When `resolveUserPrompt` calls `GetUserChatCustomPrompt`, the dbauthz
layer checks `ActionReadPersonal` on the user — which the chatd role
didn't have. The error was silently swallowed (returns `""`), so the
user's custom prompt was never injected into the system messages.
2. **Sequential DB calls on chat startup**: Several independent database
queries in `runChat` and `resolveChatModel` were running sequentially,
adding unnecessary latency before the LLM stream begins.
## Changes
### RBAC fix (`dbauthz.go`)
- Add `rbac.ResourceUser.Type: {policy.ActionReadPersonal}` to
`subjectChatd` site permissions
- This is the minimal permission needed — `ActionRead` on User remains
denied
### Parallelization (`chatd.go`)
Three parallelization points using `errgroup.Group`:
1. **`resolveChatModel`**: `resolveModelConfig` and
`GetEnabledChatProviders` run concurrently (both needed for
`ModelFromConfig`, which stays sequential after the wait)
2. **`runChat` startup**: `resolveChatModel` and
`GetChatMessagesForPromptByChatID` run concurrently (completely
independent)
3. **`runChat` prompt assembly**: `resolveInstructions` and
`resolveUserPrompt` run concurrently (both produce strings;
`InsertSystem` calls maintain correct order after the wait)
Same pattern applied to the `ReloadMessages` callback.
### Test (`dbauthz_test.go`)
- Add assertion in `TestAsChatd/AllowedActions` that
`ActionReadPersonal` on `ResourceUser` is permitted
This commit is contained in:
+85
-31
@@ -15,6 +15,7 @@ import (
|
||||
"charm.land/fantasy/providers/anthropic"
|
||||
"github.com/google/uuid"
|
||||
"github.com/sqlc-dev/pqtype"
|
||||
"golang.org/x/sync/errgroup"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
@@ -2100,21 +2101,38 @@ func (p *Server) runChat(
|
||||
chat database.Chat,
|
||||
logger slog.Logger,
|
||||
) error {
|
||||
model, modelConfig, providerKeys, err := p.resolveChatModel(ctx, chat)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
var (
|
||||
model fantasy.LanguageModel
|
||||
modelConfig database.ChatModelConfig
|
||||
providerKeys chatprovider.ProviderAPIKeys
|
||||
callConfig codersdk.ChatModelCallConfig
|
||||
messages []database.ChatMessage
|
||||
)
|
||||
|
||||
var callConfig codersdk.ChatModelCallConfig
|
||||
if len(modelConfig.Options) > 0 {
|
||||
if err := json.Unmarshal(modelConfig.Options, &callConfig); err != nil {
|
||||
return xerrors.Errorf("parse model call config: %w", err)
|
||||
var g errgroup.Group
|
||||
g.Go(func() error {
|
||||
var err error
|
||||
model, modelConfig, providerKeys, err = p.resolveChatModel(ctx, chat)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
messages, err := p.db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get chat messages: %w", err)
|
||||
if len(modelConfig.Options) > 0 {
|
||||
if err := json.Unmarshal(modelConfig.Options, &callConfig); err != nil {
|
||||
return xerrors.Errorf("parse model call config: %w", err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
})
|
||||
g.Go(func() error {
|
||||
var err error
|
||||
messages, err = p.db.GetChatMessagesForPromptByChatID(ctx, chat.ID)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get chat messages: %w", err)
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err := g.Wait(); err != nil {
|
||||
return err
|
||||
}
|
||||
// Fire title generation asynchronously so it doesn't block the
|
||||
// chat response. It uses a detached context so it can finish
|
||||
@@ -2237,11 +2255,23 @@ func (p *Server) runChat(
|
||||
return currentConn, nil
|
||||
}
|
||||
|
||||
if instruction := p.resolveInstructions(ctx, chat, getWorkspaceConn); instruction != "" {
|
||||
var instruction, resolvedUserPrompt string
|
||||
var g2 errgroup.Group
|
||||
g2.Go(func() error {
|
||||
instruction = p.resolveInstructions(ctx, chat, getWorkspaceConn)
|
||||
return nil
|
||||
})
|
||||
g2.Go(func() error {
|
||||
resolvedUserPrompt = p.resolveUserPrompt(ctx, chat.OwnerID)
|
||||
return nil
|
||||
})
|
||||
_ = g2.Wait()
|
||||
|
||||
if instruction != "" {
|
||||
prompt = chatprompt.InsertSystem(prompt, instruction)
|
||||
}
|
||||
if userPrompt := p.resolveUserPrompt(ctx, chat.OwnerID); userPrompt != "" {
|
||||
prompt = chatprompt.InsertSystem(prompt, userPrompt)
|
||||
if resolvedUserPrompt != "" {
|
||||
prompt = chatprompt.InsertSystem(prompt, resolvedUserPrompt)
|
||||
}
|
||||
|
||||
// Use the model config's context_limit as a fallback when the LLM // provider doesn't include context_limit in its response metadata
|
||||
@@ -2532,11 +2562,23 @@ func (p *Server) runChat(
|
||||
if chat.ParentChatID.Valid {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, defaultSubagentInstruction)
|
||||
}
|
||||
if instruction := p.resolveInstructions(reloadCtx, chat, getWorkspaceConn); instruction != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, instruction)
|
||||
var reloadInstruction, reloadUserPrompt string
|
||||
var rg errgroup.Group
|
||||
rg.Go(func() error {
|
||||
reloadInstruction = p.resolveInstructions(reloadCtx, chat, getWorkspaceConn)
|
||||
return nil
|
||||
})
|
||||
rg.Go(func() error {
|
||||
reloadUserPrompt = p.resolveUserPrompt(reloadCtx, chat.OwnerID)
|
||||
return nil
|
||||
})
|
||||
_ = rg.Wait()
|
||||
|
||||
if reloadInstruction != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, reloadInstruction)
|
||||
}
|
||||
if userPrompt := p.resolveUserPrompt(reloadCtx, chat.OwnerID); userPrompt != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, userPrompt)
|
||||
if reloadUserPrompt != "" {
|
||||
reloadedPrompt = chatprompt.InsertSystem(reloadedPrompt, reloadUserPrompt)
|
||||
}
|
||||
return reloadedPrompt, nil
|
||||
},
|
||||
@@ -2764,18 +2806,30 @@ func (p *Server) resolveChatModel(
|
||||
ctx context.Context,
|
||||
chat database.Chat,
|
||||
) (fantasy.LanguageModel, database.ChatModelConfig, chatprovider.ProviderAPIKeys, error) {
|
||||
dbConfig, err := p.resolveModelConfig(ctx, chat)
|
||||
if err != nil {
|
||||
return nil, database.ChatModelConfig{}, chatprovider.ProviderAPIKeys{}, xerrors.Errorf(
|
||||
"resolve model config: %w", err,
|
||||
)
|
||||
}
|
||||
var (
|
||||
dbConfig database.ChatModelConfig
|
||||
providers []database.ChatProvider
|
||||
)
|
||||
|
||||
providers, err := p.db.GetEnabledChatProviders(ctx)
|
||||
if err != nil {
|
||||
return nil, database.ChatModelConfig{}, chatprovider.ProviderAPIKeys{}, xerrors.Errorf(
|
||||
"get enabled chat providers: %w", err,
|
||||
)
|
||||
var g errgroup.Group
|
||||
g.Go(func() error {
|
||||
var err error
|
||||
dbConfig, err = p.resolveModelConfig(ctx, chat)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("resolve model config: %w", err)
|
||||
}
|
||||
return nil
|
||||
})
|
||||
g.Go(func() error {
|
||||
var err error
|
||||
providers, err = p.db.GetEnabledChatProviders(ctx)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get enabled chat providers: %w", err)
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err := g.Wait(); err != nil {
|
||||
return nil, database.ChatModelConfig{}, chatprovider.ProviderAPIKeys{}, err
|
||||
}
|
||||
dbProviders := make(
|
||||
[]chatprovider.ConfiguredProvider, 0, len(providers),
|
||||
|
||||
@@ -707,6 +707,7 @@ var (
|
||||
rbac.ResourceChat.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
|
||||
rbac.ResourceWorkspace.Type: {policy.ActionRead},
|
||||
rbac.ResourceDeploymentConfig.Type: {policy.ActionRead},
|
||||
rbac.ResourceUser.Type: {policy.ActionReadPersonal},
|
||||
}),
|
||||
User: []rbac.Permission{},
|
||||
ByOrgID: map[string]rbac.OrgPermissions{},
|
||||
|
||||
@@ -5459,6 +5459,10 @@ func TestAsChatd(t *testing.T) {
|
||||
// DeploymentConfig read.
|
||||
err = auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceDeploymentConfig)
|
||||
require.NoError(t, err, "deployment config read should be allowed")
|
||||
|
||||
// User read_personal (needed for GetUserChatCustomPrompt).
|
||||
err = auth.Authorize(ctx, actor, policy.ActionReadPersonal, rbac.ResourceUser)
|
||||
require.NoError(t, err, "user read_personal should be allowed")
|
||||
})
|
||||
|
||||
t.Run("DeniedActions", func(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user