mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: resize chat image attachments client-side for provider budgets (#24533)
Anthropic rejects inline images over 5,242,880 bytes, but our upload endpoint accepts images up to 10 MiB — so 5–10 MiB images were reaching the provider and failing. This adds two layers of protection: the browser resizes oversized images before upload, and the server rejects any that still slip through before an upstream request is issued. Client-side resizing uses `createImageBitmap` with `resizeWidth`/`resizeHeight` to clamp the decoded bitmap at decode time, then iteratively shrinks on an `OffscreenCanvas` (falling back to `HTMLCanvasElement`) until the output fits the applicable budget. Anthropic (and Bedrock-hosted Claude — fantasy's bedrock provider is a thin wrapper around the Anthropic client) uses a ~5 MiB budget; other providers use a ~10 MiB budget to stay under the server cap. Doing the resize in the browser avoids decoding attacker-controlled image bytes in `coderd` (image-bomb DoS surface). Server-side, `chatFileResolver` now takes a provider string and looks up the inline-image cap via a new `chatprovider.InlineImageByteCap` helper; oversized `image/*` files for capped providers are rejected with a pre-classified `chaterror` before the SDK call. The backstop fires for older clients, direct API callers, or any image that was committed to the composer before the user switched to a stricter provider. Attachments commit to composer state synchronously with a new `"processing"` `UploadState` so paste+Enter can't dispatch before the resize finishes; the `"uploading"` send gate now covers both states. Dismissed-while-resizing attachments are tracked in a `WeakSet` so a late swap can't resurrect a removed file. Closes CODAGT-215
This commit is contained in:
+2
-7
@@ -4343,11 +4343,6 @@ func parseCompactionThresholdKey(key string) (uuid.UUID, error) {
|
||||
return id, nil
|
||||
}
|
||||
|
||||
const (
|
||||
// maxChatFileSize is the maximum size of a chat file upload (10 MB).
|
||||
maxChatFileSize = 10 << 20
|
||||
)
|
||||
|
||||
//nolint:revive // get-return: revive assumes get* must be a getter, but this is an HTTP handler.
|
||||
func (api *API) getChatSystemPrompt(rw http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
@@ -5832,14 +5827,14 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(rw, r.Body, maxChatFileSize)
|
||||
r.Body = http.MaxBytesReader(rw, r.Body, codersdk.MaxChatFileSizeBytes)
|
||||
data, err := io.ReadAll(r.Body)
|
||||
if err != nil {
|
||||
var maxBytesErr *http.MaxBytesError
|
||||
if errors.As(err, &maxBytesErr) {
|
||||
httpapi.Write(ctx, rw, http.StatusRequestEntityTooLarge, codersdk.Response{
|
||||
Message: "File too large.",
|
||||
Detail: fmt.Sprintf("Maximum file size is %d bytes.", maxChatFileSize),
|
||||
Detail: fmt.Sprintf("Maximum file size is %d bytes.", codersdk.MaxChatFileSizeBytes),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
+40
-5
@@ -19,6 +19,7 @@ import (
|
||||
|
||||
"charm.land/fantasy"
|
||||
"charm.land/fantasy/providers/anthropic"
|
||||
"github.com/dustin/go-humanize"
|
||||
"github.com/google/uuid"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
"github.com/shopspring/decimal"
|
||||
@@ -5161,16 +5162,50 @@ func (p *Server) subscribeChatControl(
|
||||
return controlCancel
|
||||
}
|
||||
|
||||
// chatFileResolver returns a FileResolver that fetches chat file
|
||||
// content from the database by ID.
|
||||
func (p *Server) chatFileResolver() chatprompt.FileResolver {
|
||||
// Rejects oversize images on capped providers before any upstream
|
||||
// request is issued.
|
||||
//
|
||||
// Gotcha: a historical oversize image bricks the chat on a capped
|
||||
// provider until the user switches providers back, starts a new
|
||||
// chat, or edits a message above the offending one (which truncates
|
||||
// the prompt forward). A future change should skip the file with a
|
||||
// user-facing warning, but that requires altering the FileResolver
|
||||
// contract.
|
||||
func (p *Server) chatFileResolver(provider string) chatprompt.FileResolver {
|
||||
return func(ctx context.Context, ids []uuid.UUID) (map[uuid.UUID]chatprompt.FileData, error) {
|
||||
files, err := p.db.GetChatFilesByIDs(ctx, ids)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
imageCap, hasImageCap := chatprovider.InlineImageCapBytes(provider)
|
||||
normalizedProvider := chatprovider.NormalizeProvider(provider)
|
||||
result := make(map[uuid.UUID]chatprompt.FileData, len(files))
|
||||
for _, f := range files {
|
||||
if hasImageCap &&
|
||||
strings.HasPrefix(f.Mimetype, "image/") &&
|
||||
len(f.Data) >= imageCap {
|
||||
err := xerrors.Errorf(
|
||||
"image attachment %q is %d bytes; %s inline image limit is %d bytes",
|
||||
f.Name, len(f.Data),
|
||||
chatprovider.ProviderDisplayName(normalizedProvider),
|
||||
imageCap,
|
||||
)
|
||||
// User-facing message stays client-agnostic since
|
||||
// older web clients and direct API callers don't
|
||||
// auto-resize; the wrapped error above keeps the
|
||||
// exact byte count for operator logs.
|
||||
return nil, chaterror.WithClassification(err, chaterror.ClassifiedError{
|
||||
Kind: codersdk.ChatErrorKindConfig,
|
||||
Provider: normalizedProvider,
|
||||
Message: fmt.Sprintf(
|
||||
"Image attachment exceeds %s's %s inline image limit. Replace it with a smaller image.",
|
||||
chatprovider.ProviderDisplayName(normalizedProvider),
|
||||
//nolint:gosec // imageCap is a small positive constant defined in chatprovider.
|
||||
humanize.IBytes(uint64(imageCap)),
|
||||
),
|
||||
Retryable: false,
|
||||
})
|
||||
}
|
||||
result[f.ID] = chatprompt.FileData{
|
||||
Name: f.Name,
|
||||
Data: f.Data,
|
||||
@@ -6478,7 +6513,7 @@ func (p *Server) runChat(
|
||||
var g2 errgroup.Group
|
||||
g2.Go(func() error {
|
||||
var err error
|
||||
prompt, err = chatprompt.ConvertMessagesWithFiles(ctx, messages, p.chatFileResolver(), logger)
|
||||
prompt, err = chatprompt.ConvertMessagesWithFiles(ctx, messages, p.chatFileResolver(modelConfig.Provider), logger)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("build chat prompt: %w", err)
|
||||
}
|
||||
@@ -7263,7 +7298,7 @@ func (p *Server) runChat(
|
||||
if compactionOptions != nil {
|
||||
compactionOptions.HistoryTipMessageID = compactionHistoryTipMessageID
|
||||
}
|
||||
reloadedPrompt, err := chatprompt.ConvertMessagesWithFiles(reloadCtx, reloadedMsgs, p.chatFileResolver(), logger)
|
||||
reloadedPrompt, err := chatprompt.ConvertMessagesWithFiles(reloadCtx, reloadedMsgs, p.chatFileResolver(modelConfig.Provider), logger)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("convert reloaded messages: %w", err)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,313 @@
|
||||
package chatd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strconv"
|
||||
"testing"
|
||||
|
||||
"github.com/dustin/go-humanize"
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/mock/gomock"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbmock"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chatprovider"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
// inlineImageCapFor returns the provider's inline image cap. Fails
|
||||
// the test if the provider has no documented cap.
|
||||
func inlineImageCapFor(t *testing.T, provider string) int {
|
||||
t.Helper()
|
||||
imageCap, ok := chatprovider.InlineImageCapBytes(provider)
|
||||
require.Truef(t, ok, "expected provider %q to have an inline image cap", provider)
|
||||
return imageCap
|
||||
}
|
||||
|
||||
// TestChatFileResolver_RejectsOversizedImages is the server-side
|
||||
// safety net for browser-side resize: oversize images that reach the
|
||||
// resolver are rejected before any upstream request.
|
||||
func TestChatFileResolver_RejectsOversizedImages(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Computed so the table tracks any future cap retune.
|
||||
anthropicCap := inlineImageCapFor(t, "anthropic")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
provider string
|
||||
mimetype string
|
||||
size int
|
||||
expectReject bool
|
||||
expectProviderID string // classified.Provider after normalization
|
||||
}{
|
||||
{
|
||||
name: "OversizedAnthropicPNG_Rejected",
|
||||
provider: "anthropic",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: true,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
name: "OversizedAnthropicJPEG_Rejected",
|
||||
provider: "anthropic",
|
||||
mimetype: "image/jpeg",
|
||||
size: anthropicCap + 1024,
|
||||
expectReject: true,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
// Boundary is >=: exactly-at-limit is rejected.
|
||||
// Anthropic's docs say "5 MB maximum" without
|
||||
// specifying inclusivity, so reject strictly.
|
||||
name: "AtLimitAnthropicImage_Rejected",
|
||||
provider: "anthropic",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap,
|
||||
expectReject: true,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
name: "JustUnderLimitAnthropicImage_Accepted",
|
||||
provider: "anthropic",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap - 1,
|
||||
expectReject: false,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
name: "UndersizedAnthropicImage_Accepted",
|
||||
provider: "anthropic",
|
||||
mimetype: "image/png",
|
||||
size: 1024,
|
||||
expectReject: false,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
// Bedrock reuses Anthropic's cap.
|
||||
name: "OversizedBedrockPNG_Rejected",
|
||||
provider: "bedrock",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: true,
|
||||
expectProviderID: "bedrock",
|
||||
},
|
||||
{
|
||||
name: "OversizedOpenAIImage_Accepted",
|
||||
provider: "openai",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: false,
|
||||
expectProviderID: "openai",
|
||||
},
|
||||
{
|
||||
name: "OversizedAnthropicText_Accepted",
|
||||
provider: "anthropic",
|
||||
mimetype: "text/plain",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: false,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
name: "ProviderMixedCase_Rejected",
|
||||
provider: "Anthropic",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: true,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
name: "ProviderAllCaps_Rejected",
|
||||
provider: "ANTHROPIC",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: true,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
{
|
||||
name: "ProviderPaddedWhitespace_Rejected",
|
||||
provider: " anthropic ",
|
||||
mimetype: "image/png",
|
||||
size: anthropicCap + 1,
|
||||
expectReject: true,
|
||||
expectProviderID: "anthropic",
|
||||
},
|
||||
}
|
||||
|
||||
// One shared backing buffer sliced per case. The resolver only
|
||||
// reads len(f.Data), so shared backing is safe and avoids N×max
|
||||
// allocations in parallel.
|
||||
maxSize := 0
|
||||
for _, tc := range tests {
|
||||
if tc.size > maxSize {
|
||||
maxSize = tc.size
|
||||
}
|
||||
}
|
||||
sharedData := make([]byte, maxSize)
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
server := &Server{db: db}
|
||||
|
||||
fileID := uuid.New()
|
||||
row := database.ChatFile{
|
||||
ID: fileID,
|
||||
Name: "attachment.png",
|
||||
Mimetype: tc.mimetype,
|
||||
Data: sharedData[:tc.size],
|
||||
}
|
||||
db.EXPECT().
|
||||
GetChatFilesByIDs(gomock.Any(), []uuid.UUID{fileID}).
|
||||
Return([]database.ChatFile{row}, nil).
|
||||
Times(1)
|
||||
|
||||
resolver := server.chatFileResolver(tc.provider)
|
||||
got, err := resolver(ctx, []uuid.UUID{fileID})
|
||||
|
||||
if tc.expectReject {
|
||||
require.Error(t, err)
|
||||
require.Nil(t, got)
|
||||
// Classification turns the generic upstream error
|
||||
// into an actionable user-facing message.
|
||||
classified := chaterror.Classify(err)
|
||||
require.Equal(t, codersdk.ChatErrorKindConfig, classified.Kind)
|
||||
require.Equal(t, tc.expectProviderID, classified.Provider)
|
||||
require.False(t, classified.Retryable)
|
||||
// User-facing message names the provider and shows
|
||||
// the cap in human units; raw byte count stays in
|
||||
// the wrapped developer error.
|
||||
displayName := chatprovider.ProviderDisplayName(tc.expectProviderID)
|
||||
require.Contains(t, classified.Message, displayName)
|
||||
imageCap := inlineImageCapFor(t, tc.expectProviderID)
|
||||
//nolint:gosec // imageCap is a small positive constant defined in chatprovider.
|
||||
require.Contains(t, classified.Message, humanize.IBytes(uint64(imageCap)))
|
||||
require.NotContains(
|
||||
t,
|
||||
classified.Message,
|
||||
strconv.Itoa(imageCap),
|
||||
"user-facing message should not include raw bytes",
|
||||
)
|
||||
// Wrapped error preserves exact bytes for logs.
|
||||
require.Contains(t, err.Error(), strconv.Itoa(imageCap))
|
||||
return
|
||||
}
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, got, fileID)
|
||||
require.Equal(t, row.Data, got[fileID].Data)
|
||||
require.Equal(t, tc.mimetype, got[fileID].MediaType)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestChatFileResolver_MultiFileFailsFastOnFirstOversized pins the
|
||||
// "first bad file aborts the batch" contract.
|
||||
func TestChatFileResolver_MultiFileFailsFastOnFirstOversized(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
server := &Server{db: db}
|
||||
|
||||
anthropicCap := inlineImageCapFor(t, "anthropic")
|
||||
// Shared buffer; ok files take small prefixes.
|
||||
buf := make([]byte, anthropicCap+1)
|
||||
okFileA := database.ChatFile{
|
||||
ID: uuid.New(),
|
||||
Name: "ok-a.png",
|
||||
Mimetype: "image/png",
|
||||
Data: buf[:1024],
|
||||
}
|
||||
oversized := database.ChatFile{
|
||||
ID: uuid.New(),
|
||||
Name: "too-big.png",
|
||||
Mimetype: "image/png",
|
||||
Data: buf,
|
||||
}
|
||||
okFileB := database.ChatFile{
|
||||
ID: uuid.New(),
|
||||
Name: "ok-b.png",
|
||||
Mimetype: "image/png",
|
||||
Data: buf[:1024],
|
||||
}
|
||||
ids := []uuid.UUID{okFileA.ID, oversized.ID, okFileB.ID}
|
||||
|
||||
db.EXPECT().
|
||||
GetChatFilesByIDs(gomock.Any(), ids).
|
||||
Return([]database.ChatFile{okFileA, oversized, okFileB}, nil).
|
||||
Times(1)
|
||||
|
||||
resolver := server.chatFileResolver("anthropic")
|
||||
got, err := resolver(ctx, ids)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, got)
|
||||
classified := chaterror.Classify(err)
|
||||
require.Equal(t, codersdk.ChatErrorKindConfig, classified.Kind)
|
||||
// The error must identify the specific offending file so a user
|
||||
// with several attachments knows which one to replace.
|
||||
require.Contains(t, err.Error(), oversized.Name)
|
||||
}
|
||||
|
||||
// TestChatFileResolver_PropagatesDBError confirms unrelated database
|
||||
// failures pass through unchanged (not masked by the size check).
|
||||
func TestChatFileResolver_PropagatesDBError(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
server := &Server{db: db}
|
||||
|
||||
sentinel := xerrors.New("boom")
|
||||
fileID := uuid.New()
|
||||
db.EXPECT().
|
||||
GetChatFilesByIDs(gomock.Any(), []uuid.UUID{fileID}).
|
||||
Return(nil, sentinel).
|
||||
Times(1)
|
||||
|
||||
resolver := server.chatFileResolver("anthropic")
|
||||
got, err := resolver(ctx, []uuid.UUID{fileID})
|
||||
require.ErrorIs(t, err, sentinel)
|
||||
require.Nil(t, got)
|
||||
}
|
||||
|
||||
// TestChatFileResolver_UnknownProviderSkipsCapCheck confirms providers
|
||||
// without a documented inline cap are never rejected by the backstop.
|
||||
func TestChatFileResolver_UnknownProviderSkipsCapCheck(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
db := dbmock.NewMockStore(ctrl)
|
||||
server := &Server{db: db}
|
||||
|
||||
fileID := uuid.New()
|
||||
// Exactly 1 byte above the Anthropic cap is enough to prove
|
||||
// the backstop is skipped for uncapped providers; no need to
|
||||
// allocate tens of MiB in CI.
|
||||
overAnyCap := inlineImageCapFor(t, "anthropic") + 1
|
||||
row := database.ChatFile{
|
||||
ID: fileID,
|
||||
Name: "huge.png",
|
||||
Mimetype: "image/png",
|
||||
Data: make([]byte, overAnyCap),
|
||||
}
|
||||
db.EXPECT().
|
||||
GetChatFilesByIDs(gomock.Any(), []uuid.UUID{fileID}).
|
||||
Return([]database.ChatFile{row}, nil).
|
||||
Times(1)
|
||||
|
||||
resolver := server.chatFileResolver("openrouter")
|
||||
got, err := resolver(ctx, []uuid.UUID{fileID})
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, got, fileID)
|
||||
}
|
||||
@@ -83,6 +83,19 @@ func ProviderAllowsAmbientCredentials(provider string) bool {
|
||||
return NormalizeProvider(provider) == fantasybedrock.Name
|
||||
}
|
||||
|
||||
// InlineImageCapBytes returns the per-image byte cap for inline
|
||||
// image parts, or (0, false) when no documented cap applies.
|
||||
// Bedrock shares Anthropic's cap because fantasy's bedrock provider
|
||||
// wraps the anthropic client.
|
||||
func InlineImageCapBytes(provider string) (int, bool) {
|
||||
switch NormalizeProvider(provider) {
|
||||
case fantasyanthropic.Name, fantasybedrock.Name:
|
||||
return codersdk.AnthropicInlineImageCapBytes, true
|
||||
default:
|
||||
return 0, false
|
||||
}
|
||||
}
|
||||
|
||||
// ProviderAPIKeys contains API keys for provider calls.
|
||||
type ProviderAPIKeys struct {
|
||||
OpenAI string
|
||||
|
||||
Reference in New Issue
Block a user