fix(coderd): reject API operations on archived chats (#24633)

Archived chats accept mutations (messages, edits, queued-message
promotions, tool-result submissions) via the API, causing them to
re-enter the processing pipeline. This violates the hard-stop
design intent from PR #23758.

Add archived checks at three layers:

- HTTP handlers (postChatMessages, patchChatMessage,
  promoteChatQueuedMessage, postChatToolResults): return 400
  after auth so callers get a clear error.
- Daemon functions (SendMessage, EditMessage, PromoteQueued,
  SubmitToolResults): return ErrChatArchived after row lock,
  guarding against future callers that bypass the handler.
- AcquireChats SQL: filter out archived chats so they are never
  acquired for processing.

Fixes CODAGT-245
This commit is contained in:
Mathias Fredriksson
2026-04-23 19:03:33 +03:00
committed by GitHub
parent a13f7f18e5
commit f8fe5d680b
6 changed files with 437 additions and 0 deletions
+1
View File
@@ -5077,6 +5077,7 @@ WHERE
chats
WHERE
status = 'pending'::chat_status
AND archived = false
ORDER BY
updated_at ASC
FOR UPDATE
+1
View File
@@ -695,6 +695,7 @@ WHERE
chats
WHERE
status = 'pending'::chat_status
AND archived = false
ORDER BY
updated_at ASC
FOR UPDATE
+48
View File
@@ -2356,6 +2356,13 @@ func (api *API) postChatMessages(rw http.ResponseWriter, r *http.Request) {
return
}
if chat.Archived {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot send messages to an archived chat.",
})
return
}
if api.chatDaemon == nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Chat processor is unavailable.",
@@ -2453,6 +2460,12 @@ func (api *API) postChatMessages(rw http.ResponseWriter, r *http.Request) {
if maybeWriteLimitErr(ctx, rw, sendErr) {
return
}
if xerrors.Is(sendErr, chatd.ErrChatArchived) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot send messages to an archived chat.",
})
return
}
if xerrors.Is(sendErr, chatd.ErrMessageQueueFull) {
httpapi.Write(ctx, rw, http.StatusTooManyRequests, codersdk.Response{
Message: "Message queue is full.",
@@ -2496,6 +2509,13 @@ func (api *API) patchChatMessage(rw http.ResponseWriter, r *http.Request) {
apiKey := httpmw.APIKey(r)
chat := httpmw.ChatParam(r)
if chat.Archived {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot edit messages in an archived chat.",
})
return
}
if api.chatDaemon == nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Chat processor is unavailable.",
@@ -2540,6 +2560,10 @@ func (api *API) patchChatMessage(rw http.ResponseWriter, r *http.Request) {
}
switch {
case xerrors.Is(editErr, chatd.ErrChatArchived):
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot edit messages in an archived chat.",
})
case xerrors.Is(editErr, chatd.ErrEditedMessageNotFound):
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
Message: "Chat message not found.",
@@ -2625,6 +2649,13 @@ func (api *API) promoteChatQueuedMessage(rw http.ResponseWriter, r *http.Request
return
}
if chat.Archived {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot promote queued messages in an archived chat.",
})
return
}
queuedMessageIDStr := chi.URLParam(r, "queuedMessage")
queuedMessageID, err := strconv.ParseInt(queuedMessageIDStr, 10, 64)
if err != nil {
@@ -2653,6 +2684,12 @@ func (api *API) promoteChatQueuedMessage(rw http.ResponseWriter, r *http.Request
if maybeWriteLimitErr(ctx, rw, txErr) {
return
}
if xerrors.Is(txErr, chatd.ErrChatArchived) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot promote queued messages in an archived chat.",
})
return
}
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to promote queued message.",
Detail: txErr.Error(),
@@ -6726,6 +6763,13 @@ func (api *API) postChatToolResults(rw http.ResponseWriter, r *http.Request) {
return
}
if chat.Archived {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot submit tool results to an archived chat.",
})
return
}
// Cap the raw request body to prevent excessive memory use.
r.Body = http.MaxBytesReader(rw, r.Body, int64(2*maxSystemPromptLenBytes))
var req codersdk.SubmitToolResultsRequest
@@ -6767,6 +6811,10 @@ func (api *API) postChatToolResults(rw http.ResponseWriter, r *http.Request) {
var validationErr *chatd.ToolResultValidationError
var conflictErr *chatd.ToolResultStatusConflictError
switch {
case xerrors.Is(err, chatd.ErrChatArchived):
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Cannot submit tool results to an archived chat.",
})
case errors.As(err, &conflictErr):
httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{
Message: "Chat is not waiting for tool results.",
+152
View File
@@ -5786,6 +5786,38 @@ func TestPostChatMessages(t *testing.T) {
})
requireSDKError(t, err, http.StatusNotFound)
})
t.Run("ArchivedChat", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
_ = createChatModelConfig(t, client)
chat, err := client.CreateChat(ctx, codersdk.CreateChatRequest{
OrganizationID: firstUser.OrganizationID,
Content: []codersdk.ChatInputPart{{
Type: codersdk.ChatInputPartTypeText,
Text: "hello",
}},
})
require.NoError(t, err)
err = client.UpdateChat(ctx, chat.ID, codersdk.UpdateChatRequest{
Archived: ptr.Ref(true),
})
require.NoError(t, err)
_, err = client.CreateChatMessage(ctx, chat.ID, codersdk.CreateChatMessageRequest{
Content: []codersdk.ChatInputPart{{
Type: codersdk.ChatInputPartTypeText,
Text: "should fail",
}},
})
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "archived")
})
}
func TestChatMessageWithFileReferences(t *testing.T) {
@@ -6950,6 +6982,50 @@ func TestPatchChatMessage(t *testing.T) {
require.Len(t, chatResult.Files, codersdk.MaxChatFileIDs,
"file count should not exceed the cap")
})
t.Run("ArchivedChat", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
_ = createChatModelConfig(t, client)
chat, err := client.CreateChat(ctx, codersdk.CreateChatRequest{
OrganizationID: firstUser.OrganizationID,
Content: []codersdk.ChatInputPart{{
Type: codersdk.ChatInputPartTypeText,
Text: "hello before edit",
}},
})
require.NoError(t, err)
messagesResult, err := client.GetChatMessages(ctx, chat.ID, nil)
require.NoError(t, err)
var userMessageID int64
for _, message := range messagesResult.Messages {
if message.Role == codersdk.ChatMessageRoleUser {
userMessageID = message.ID
break
}
}
require.NotZero(t, userMessageID)
err = client.UpdateChat(ctx, chat.ID, codersdk.UpdateChatRequest{
Archived: ptr.Ref(true),
})
require.NoError(t, err)
_, err = client.EditChatMessage(ctx, chat.ID, userMessageID, codersdk.EditChatMessageRequest{
Content: []codersdk.ChatInputPart{{
Type: codersdk.ChatInputPartTypeText,
Text: "should fail",
}},
})
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "archived")
})
}
func TestStreamChat(t *testing.T) {
@@ -8021,6 +8097,56 @@ func TestPromoteChatQueuedMessage(t *testing.T) {
defer promoteRes.Body.Close()
require.Equal(t, http.StatusForbidden, promoteRes.StatusCode)
})
t.Run("ArchivedChat", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, db := newChatClientWithDatabase(t)
user := coderdtest.CreateFirstUser(t, client.Client)
modelConfig := createChatModelConfig(t, client)
chat, err := db.InsertChat(dbauthz.AsSystemRestricted(ctx), database.InsertChatParams{
OrganizationID: user.OrganizationID,
Status: database.ChatStatusWaiting,
ClientType: database.ChatClientTypeUi,
OwnerID: user.UserID,
LastModelConfigID: modelConfig.ID,
Title: "promote queued archived",
})
require.NoError(t, err)
queuedContent, err := json.Marshal([]codersdk.ChatMessagePart{
codersdk.ChatMessageText("queued"),
})
require.NoError(t, err)
queuedMessage, err := db.InsertChatQueuedMessage(
dbauthz.AsSystemRestricted(ctx),
database.InsertChatQueuedMessageParams{
ChatID: chat.ID,
Content: queuedContent,
},
)
require.NoError(t, err)
// Archive the chat.
_, err = db.ArchiveChatByID(dbauthz.AsSystemRestricted(ctx), chat.ID)
require.NoError(t, err)
promoteRes, err := client.Request(
ctx,
http.MethodPost,
fmt.Sprintf("/api/experimental/chats/%s/queue/%d/promote", chat.ID, queuedMessage.ID),
nil,
)
require.NoError(t, err)
defer promoteRes.Body.Close()
require.Equal(t, http.StatusBadRequest, promoteRes.StatusCode)
promoteErr := codersdk.ReadBodyAsError(promoteRes)
var promoteSDKErr *codersdk.Error
require.ErrorAs(t, promoteErr, &promoteSDKErr)
require.Contains(t, promoteSDKErr.Message, "archived")
})
}
func TestChatUsageLimitOverrideRoutes(t *testing.T) {
@@ -11339,6 +11465,32 @@ func TestSubmitToolResults(t *testing.T) {
})
requireSDKError(t, err, http.StatusForbidden)
})
t.Run("ArchivedChat", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, db := newChatClientWithDatabase(t)
user := coderdtest.CreateFirstUser(t, client.Client)
modelConfig := createChatModelConfig(t, client)
const toolName = "my_dynamic_tool"
toolCallIDs := []string{"call_archived"}
chat := setupRequiresAction(ctx, t, db, user.UserID, user.OrganizationID, modelConfig.ID, toolName, toolCallIDs)
// Archive the chat.
_, err := db.ArchiveChatByID(dbauthz.AsSystemRestricted(ctx), chat.ID)
require.NoError(t, err)
err = client.SubmitToolResults(ctx, chat.ID, codersdk.SubmitToolResultsRequest{
Results: []codersdk.ToolResult{
{ToolCallID: "call_archived", Output: json.RawMessage(`"should fail"`)},
},
})
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "archived")
})
}
func TestPostChats_DynamicToolValidation(t *testing.T) {
+20
View File
@@ -878,6 +878,10 @@ var (
ErrEditedMessageNotFound = xerrors.New("edited message not found")
// ErrEditedMessageNotUser indicates a non-user message edit attempt.
ErrEditedMessageNotUser = xerrors.New("only user messages can be edited")
// ErrChatArchived indicates the chat is archived and cannot
// accept modifications (messages, edits, promotions, or
// tool-result submissions).
ErrChatArchived = xerrors.New("chat is archived")
// errChatTakenByOtherWorker is a sentinel used inside the
// processChat cleanup transaction to signal that another
@@ -1194,6 +1198,10 @@ func (p *Server) SendMessage(
return xerrors.Errorf("lock chat: %w", err)
}
if lockedChat.Archived {
return ErrChatArchived
}
// Enforce usage limits before queueing or inserting.
if limitErr := p.checkUsageLimit(ctx, tx, lockedChat.OwnerID, uuid.NullUUID{UUID: lockedChat.OrganizationID, Valid: true}); limitErr != nil {
return limitErr
@@ -1384,6 +1392,10 @@ func (p *Server) EditMessage(
return xerrors.Errorf("lock chat: %w", err)
}
if lockedChat.Archived {
return ErrChatArchived
}
if limitErr := p.checkUsageLimit(ctx, tx, lockedChat.OwnerID, uuid.NullUUID{UUID: lockedChat.OrganizationID, Valid: true}); limitErr != nil {
return limitErr
}
@@ -1743,6 +1755,11 @@ func (p *Server) PromoteQueued(
if err != nil {
return xerrors.Errorf("lock chat: %w", err)
}
if lockedChat.Archived {
return ErrChatArchived
}
modelConfigID := lockedChat.LastModelConfigID
if opts.ModelConfigID != nil {
modelConfigID = *opts.ModelConfigID
@@ -1881,6 +1898,9 @@ func (p *Server) SubmitToolResults(
if lockErr != nil {
return xerrors.Errorf("lock chat for update: %w", lockErr)
}
if locked.Archived {
return ErrChatArchived
}
if locked.Status != database.ChatStatusRequiresAction {
statusConflict = &ToolResultStatusConflictError{
ActualStatus: locked.Status,
+215
View File
@@ -7294,3 +7294,218 @@ func TestAgentContextFilesAndSkillsLoadedIntoChat(t *testing.T) {
require.Zero(t, standalonePlanBlockCount,
"plan-file-path block should be part of the main system prompt, not a standalone message")
}
func TestSendMessageRejectsArchivedChat(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
replica := newTestServer(t, db, ps, uuid.New())
ctx := testutil.Context(t, testutil.WaitLong)
user, org, model := seedChatDependencies(ctx, t, db)
chat, err := replica.CreateChat(ctx, chatd.CreateOptions{
OwnerID: user.ID,
OrganizationID: org.ID,
Title: "send-archived",
ModelConfigID: model.ID,
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("hello")},
})
require.NoError(t, err)
err = replica.ArchiveChat(ctx, chat)
require.NoError(t, err)
_, err = replica.SendMessage(ctx, chatd.SendMessageOptions{
ChatID: chat.ID,
Content: []codersdk.ChatMessagePart{codersdk.ChatMessageText("should fail")},
BusyBehavior: chatd.SendMessageBusyBehaviorQueue,
})
require.ErrorIs(t, err, chatd.ErrChatArchived)
}
func TestEditMessageRejectsArchivedChat(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
replica := newTestServer(t, db, ps, uuid.New())
ctx := testutil.Context(t, testutil.WaitLong)
user, org, model := seedChatDependencies(ctx, t, db)
chat, err := replica.CreateChat(ctx, chatd.CreateOptions{
OwnerID: user.ID,
OrganizationID: org.ID,
Title: "edit-archived",
ModelConfigID: model.ID,
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("original")},
})
require.NoError(t, err)
messages, err := db.GetChatMessagesByChatID(ctx, database.GetChatMessagesByChatIDParams{
ChatID: chat.ID,
AfterID: 0,
})
require.NoError(t, err)
require.Len(t, messages, 1)
err = replica.ArchiveChat(ctx, chat)
require.NoError(t, err)
_, err = replica.EditMessage(ctx, chatd.EditMessageOptions{
ChatID: chat.ID,
EditedMessageID: messages[0].ID,
Content: []codersdk.ChatMessagePart{codersdk.ChatMessageText("edited")},
})
require.ErrorIs(t, err, chatd.ErrChatArchived)
}
func TestPromoteQueuedRejectsArchivedChat(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
replica := newTestServer(t, db, ps, uuid.New())
ctx := testutil.Context(t, testutil.WaitLong)
user, org, model := seedChatDependencies(ctx, t, db)
chat, err := replica.CreateChat(ctx, chatd.CreateOptions{
OwnerID: user.ID,
OrganizationID: org.ID,
Title: "promote-archived",
ModelConfigID: model.ID,
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("hello")},
})
require.NoError(t, err)
// Queue a message by setting the chat to running first.
chat, err = db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{
ID: chat.ID,
Status: database.ChatStatusRunning,
WorkerID: uuid.NullUUID{UUID: uuid.New(), Valid: true},
StartedAt: sql.NullTime{Time: time.Now(), Valid: true},
HeartbeatAt: sql.NullTime{Time: time.Now(), Valid: true},
})
require.NoError(t, err)
queuedResult, err := replica.SendMessage(ctx, chatd.SendMessageOptions{
ChatID: chat.ID,
Content: []codersdk.ChatMessagePart{codersdk.ChatMessageText("queued")},
BusyBehavior: chatd.SendMessageBusyBehaviorQueue,
})
require.NoError(t, err)
require.True(t, queuedResult.Queued)
// Move back to waiting, then archive.
chat, err = db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{
ID: chat.ID,
Status: database.ChatStatusWaiting,
WorkerID: uuid.NullUUID{},
StartedAt: sql.NullTime{},
HeartbeatAt: sql.NullTime{},
})
require.NoError(t, err)
err = replica.ArchiveChat(ctx, chat)
require.NoError(t, err)
_, err = replica.PromoteQueued(ctx, chatd.PromoteQueuedOptions{
ChatID: chat.ID,
QueuedMessageID: queuedResult.QueuedMessage.ID,
CreatedBy: user.ID,
})
require.ErrorIs(t, err, chatd.ErrChatArchived)
}
func TestSubmitToolResultsRejectsArchivedChat(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
replica := newTestServer(t, db, ps, uuid.New())
ctx := testutil.Context(t, testutil.WaitLong)
user, org, model := seedChatDependencies(ctx, t, db)
chat, err := replica.CreateChat(ctx, chatd.CreateOptions{
OwnerID: user.ID,
OrganizationID: org.ID,
Title: "submit-tool-archived",
ModelConfigID: model.ID,
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("hello")},
})
require.NoError(t, err)
err = replica.ArchiveChat(ctx, chat)
require.NoError(t, err)
// Set requires_action so the test exercises a realistic
// scenario where SubmitToolResults would be called.
_, err = db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{
ID: chat.ID,
Status: database.ChatStatusRequiresAction,
})
require.NoError(t, err)
err = replica.SubmitToolResults(ctx, chatd.SubmitToolResultsOptions{
ChatID: chat.ID,
UserID: user.ID,
ModelConfigID: model.ID,
Results: []codersdk.ToolResult{{
ToolCallID: "fake-tool-call-id",
Output: json.RawMessage(`{"result":"ignored"}`),
}},
})
require.ErrorIs(t, err, chatd.ErrChatArchived)
}
func TestAcquireChatsSkipsArchivedPendingChat(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
_ = newTestServer(t, db, ps, uuid.New())
ctx := testutil.Context(t, testutil.WaitLong)
user, org, model := seedChatDependencies(ctx, t, db)
archivedChat, err := db.InsertChat(ctx, database.InsertChatParams{
OwnerID: user.ID,
OrganizationID: org.ID,
Title: "acquire-skip-archived",
LastModelConfigID: model.ID,
Status: database.ChatStatusWaiting,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
// Archive the chat, then force it to pending.
_, err = db.ArchiveChatByID(ctx, archivedChat.ID)
require.NoError(t, err)
_, err = db.UpdateChatStatus(ctx, database.UpdateChatStatusParams{
ID: archivedChat.ID,
Status: database.ChatStatusPending,
})
require.NoError(t, err)
// Insert a second, non-archived pending chat so the result
// slice is non-empty and the assertion is not vacuously true.
activeChat, err := db.InsertChat(ctx, database.InsertChatParams{
OwnerID: user.ID,
OrganizationID: org.ID,
Title: "acquire-active",
LastModelConfigID: model.ID,
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
now := time.Now()
acquired, err := db.AcquireChats(ctx, database.AcquireChatsParams{
WorkerID: uuid.New(),
StartedAt: now,
NumChats: 10,
})
require.NoError(t, err)
require.Len(t, acquired, 1, "only the non-archived chat should be acquired")
require.Equal(t, activeChat.ID, acquired[0].ID)
}