mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
feat(rbac): add AsChatd subject to replace AsSystemRestricted in chatd (#22487)
Add a new SubjectTypeChatd RBAC subject with minimal permissions: - Chat: CRUD - Workspace: Read - DeploymentConfig: Read Replace all 10 AsSystemRestricted calls in coderd/chatd/chatd.go: - Line 890: Use AsChatd instead of AsSystemRestricted for the background processor context. - Subscribe() path (5 calls): Remove system escalation entirely; these run under the authenticated user's context from the HTTP handler. - processChat path (4 calls): Remove redundant per-call wraps; the context already carries AsChatd from the processor start. Add TestAsChatd verifying allowed and denied actions. Created using Mux (Opus 4.6)
This commit is contained in:
+11
-20
@@ -886,8 +886,8 @@ func New(cfg Config) *Server {
|
||||
inFlightChatStaleAfter: inFlightChatStaleAfter,
|
||||
}
|
||||
|
||||
//nolint:gocritic // The chat processor is a system-level service.
|
||||
ctx = dbauthz.AsSystemRestricted(ctx)
|
||||
//nolint:gocritic // The chat processor uses a scoped chatd context.
|
||||
ctx = dbauthz.AsChatd(ctx)
|
||||
go p.start(ctx)
|
||||
|
||||
return p
|
||||
@@ -1044,8 +1044,7 @@ func (p *Server) Subscribe(
|
||||
}
|
||||
|
||||
// Load initial messages from DB
|
||||
//nolint:gocritic // System context needed to read chat messages for stream.
|
||||
messages, err := p.db.GetChatMessagesByChatID(dbauthz.AsSystemRestricted(ctx), chatID)
|
||||
messages, err := p.db.GetChatMessagesByChatID(ctx, chatID)
|
||||
if err == nil {
|
||||
for _, msg := range messages {
|
||||
sdkMsg := db2sdk.ChatMessage(msg)
|
||||
@@ -1058,8 +1057,7 @@ func (p *Server) Subscribe(
|
||||
}
|
||||
|
||||
// Load initial queue
|
||||
//nolint:gocritic // System context needed to read queued messages for stream.
|
||||
queued, err := p.db.GetChatQueuedMessages(dbauthz.AsSystemRestricted(ctx), chatID)
|
||||
queued, err := p.db.GetChatQueuedMessages(ctx, chatID)
|
||||
if err == nil && len(queued) > 0 {
|
||||
initialSnapshot = append(initialSnapshot, codersdk.ChatStreamEvent{
|
||||
Type: codersdk.ChatStreamEventTypeQueueUpdate,
|
||||
@@ -1069,8 +1067,7 @@ func (p *Server) Subscribe(
|
||||
}
|
||||
|
||||
// Get initial chat state to determine if we need a relay
|
||||
//nolint:gocritic // System context needed to read chat state for relay.
|
||||
chat, err := p.db.GetChatByID(dbauthz.AsSystemRestricted(ctx), chatID)
|
||||
chat, err := p.db.GetChatByID(ctx, chatID)
|
||||
var relayCancel func()
|
||||
var relayParts <-chan codersdk.ChatStreamEvent
|
||||
if err == nil && chat.Status == database.ChatStatusRunning && chat.WorkerID.Valid && chat.WorkerID.UUID != p.workerID && p.remotePartsProvider != nil {
|
||||
@@ -1225,8 +1222,7 @@ func (p *Server) Subscribe(
|
||||
// Handle different notification types
|
||||
if notify.AfterMessageID > 0 {
|
||||
// Read new messages from DB
|
||||
//nolint:gocritic // System context needed to read chat messages for stream.
|
||||
messages, err := p.db.GetChatMessagesByChatID(dbauthz.AsSystemRestricted(mergedCtx), chatID)
|
||||
messages, err := p.db.GetChatMessagesByChatID(mergedCtx, chatID)
|
||||
if err == nil {
|
||||
for _, msg := range messages {
|
||||
if msg.ID > lastMessageID {
|
||||
@@ -1282,8 +1278,7 @@ func (p *Server) Subscribe(
|
||||
}
|
||||
}
|
||||
if notify.QueueUpdate {
|
||||
//nolint:gocritic // System context needed to read queued messages for stream.
|
||||
queued, err := p.db.GetChatQueuedMessages(dbauthz.AsSystemRestricted(mergedCtx), chatID)
|
||||
queued, err := p.db.GetChatQueuedMessages(mergedCtx, chatID)
|
||||
if err == nil {
|
||||
select {
|
||||
case <-mergedCtx.Done():
|
||||
@@ -1884,8 +1879,7 @@ func (p *Server) runChat(
|
||||
loadCtx context.Context,
|
||||
chatID uuid.UUID,
|
||||
) (database.Chat, error) {
|
||||
//nolint:gocritic // System context required to load chat snapshots for the stream.
|
||||
return p.db.GetChatByID(dbauthz.AsSystemRestricted(loadCtx), chatID)
|
||||
return p.db.GetChatByID(loadCtx, chatID)
|
||||
}
|
||||
var (
|
||||
chatStateMu sync.Mutex
|
||||
@@ -1936,9 +1930,8 @@ func (p *Server) runChat(
|
||||
return nil, xerrors.New("chat has no workspace")
|
||||
}
|
||||
|
||||
//nolint:gocritic // System context needed to look up workspace agents.
|
||||
agents, err := p.db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(
|
||||
dbauthz.AsSystemRestricted(ctx),
|
||||
ctx,
|
||||
chatSnapshot.WorkspaceID.UUID,
|
||||
)
|
||||
if err != nil || len(agents) == 0 {
|
||||
@@ -2480,9 +2473,8 @@ func (p *Server) resolveInstructions(
|
||||
return ""
|
||||
}
|
||||
|
||||
//nolint:gocritic // System context needed to look up workspace agents.
|
||||
agents, agentsErr := p.db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(
|
||||
dbauthz.AsSystemRestricted(ctx),
|
||||
ctx,
|
||||
chat.WorkspaceID.UUID,
|
||||
)
|
||||
if agentsErr != nil || len(agents) == 0 {
|
||||
@@ -2499,8 +2491,7 @@ func (p *Server) resolveInstructions(
|
||||
}
|
||||
|
||||
// Look up the agent's OS and working directory.
|
||||
//nolint:gocritic // System context needed to read workspace agent metadata.
|
||||
agent, err := p.db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), agentID)
|
||||
agent, err := p.db.GetWorkspaceAgentByID(ctx, agentID)
|
||||
if err != nil {
|
||||
p.logger.Debug(ctx, "failed to look up workspace agent for instruction context",
|
||||
slog.F("agent_id", agentID),
|
||||
|
||||
@@ -694,6 +694,26 @@ var (
|
||||
}),
|
||||
Scope: rbac.ScopeAll,
|
||||
}.WithCachedASTValue()
|
||||
|
||||
subjectChatd = rbac.Subject{
|
||||
Type: rbac.SubjectTypeChatd,
|
||||
FriendlyName: "Chatd",
|
||||
ID: uuid.Nil.String(),
|
||||
Roles: rbac.Roles([]rbac.Role{
|
||||
{
|
||||
Identifier: rbac.RoleIdentifier{Name: "chatd"},
|
||||
DisplayName: "Chat Daemon",
|
||||
Site: rbac.Permissions(map[string][]policy.Action{
|
||||
rbac.ResourceChat.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete},
|
||||
rbac.ResourceWorkspace.Type: {policy.ActionRead},
|
||||
rbac.ResourceDeploymentConfig.Type: {policy.ActionRead},
|
||||
}),
|
||||
User: []rbac.Permission{},
|
||||
ByOrgID: map[string]rbac.OrgPermissions{},
|
||||
},
|
||||
}),
|
||||
Scope: rbac.ScopeAll,
|
||||
}.WithCachedASTValue()
|
||||
)
|
||||
|
||||
// AsProvisionerd returns a context with an actor that has permissions required
|
||||
@@ -808,6 +828,13 @@ func AsWorkspaceBuilder(ctx context.Context) context.Context {
|
||||
return As(ctx, subjectWorkspaceBuilder)
|
||||
}
|
||||
|
||||
// AsChatd returns a context with an actor scoped to the chat
|
||||
// daemon's background worker. It can manage chats and read
|
||||
// workspaces and deployment config, but nothing else.
|
||||
func AsChatd(ctx context.Context) context.Context {
|
||||
return As(ctx, subjectChatd)
|
||||
}
|
||||
|
||||
var AsRemoveActor = rbac.Subject{
|
||||
ID: "remove-actor",
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
|
||||
"github.com/brianvoe/gofakeit/v7"
|
||||
"github.com/google/uuid"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"github.com/sqlc-dev/pqtype"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/mock/gomock"
|
||||
@@ -5358,3 +5359,58 @@ func TestGetWorkspaceAgentByID_FastPath(t *testing.T) {
|
||||
require.Equal(t, agent, result)
|
||||
})
|
||||
}
|
||||
|
||||
func TestAsChatd(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := dbauthz.AsChatd(context.Background())
|
||||
actor, ok := dbauthz.ActorFromContext(ctx)
|
||||
require.True(t, ok, "actor must be present")
|
||||
|
||||
auth := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
|
||||
|
||||
t.Run("AllowedActions", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Chat CRUD.
|
||||
for _, action := range []policy.Action{
|
||||
policy.ActionCreate, policy.ActionRead,
|
||||
policy.ActionUpdate, policy.ActionDelete,
|
||||
} {
|
||||
err := auth.Authorize(ctx, actor, action, rbac.ResourceChat)
|
||||
require.NoError(t, err, "chat %s should be allowed", action)
|
||||
}
|
||||
|
||||
// Workspace read.
|
||||
err := auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceWorkspace)
|
||||
require.NoError(t, err, "workspace read should be allowed")
|
||||
|
||||
// DeploymentConfig read.
|
||||
err = auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceDeploymentConfig)
|
||||
require.NoError(t, err, "deployment config read should be allowed")
|
||||
})
|
||||
|
||||
t.Run("DeniedActions", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Cannot write workspaces.
|
||||
for _, action := range []policy.Action{
|
||||
policy.ActionUpdate, policy.ActionDelete,
|
||||
} {
|
||||
err := auth.Authorize(ctx, actor, action, rbac.ResourceWorkspace)
|
||||
require.Error(t, err, "workspace %s should be denied", action)
|
||||
}
|
||||
|
||||
// Cannot access users.
|
||||
err := auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceUser)
|
||||
require.Error(t, err, "user read should be denied")
|
||||
|
||||
// Cannot access API keys.
|
||||
err = auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceApiKey)
|
||||
require.Error(t, err, "api key read should be denied")
|
||||
|
||||
// Cannot access provisioner daemons.
|
||||
err = auth.Authorize(ctx, actor, policy.ActionRead, rbac.ResourceProvisionerDaemon)
|
||||
require.Error(t, err, "provisioner daemon read should be denied")
|
||||
})
|
||||
}
|
||||
|
||||
@@ -82,6 +82,7 @@ const (
|
||||
SubjectTypeDBPurge SubjectType = "dbpurge"
|
||||
SubjectTypeBoundaryUsageTracker SubjectType = "boundary_usage_tracker"
|
||||
SubjectTypeWorkspaceBuilder SubjectType = "workspace_builder"
|
||||
SubjectTypeChatd SubjectType = "chatd"
|
||||
)
|
||||
|
||||
const (
|
||||
|
||||
Reference in New Issue
Block a user