mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: align chat attachment picker with allowed file types (#24917)
The agent chat composer only advertised image uploads to the OS file picker and filtered drag-and-drop and paste events to `image/*`, even though the backend accepts text, CSV, JSON, PDF, and a narrower set of image types. Move the allowed chat attachment media types into `codersdk` so the frontend picker and backend enforcement share one source of truth. Use the generated TypeScript list to drive the file input `accept` attribute and the drag-and-drop and paste filters, while adding common text extensions so platforms without MIME registrations still surface those files in the picker.
This commit is contained in:
+9
-7
@@ -5544,7 +5544,9 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) {
|
||||
if mediaType, _, err := mime.ParseMediaType(contentType); err == nil {
|
||||
contentType = mediaType
|
||||
}
|
||||
if !chatfiles.IsAllowedStoredMediaType(contentType) {
|
||||
// application/octet-stream means the client could not classify the file
|
||||
// ahead of time, so we defer to byte classification below.
|
||||
if contentType != "application/octet-stream" && !chatfiles.IsAllowedStoredMediaType(contentType) {
|
||||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
|
||||
Message: "Unsupported file type.",
|
||||
Detail: fmt.Sprintf("Allowed types: %s.", chatfiles.AllowedStoredMediaTypesString()),
|
||||
@@ -5602,12 +5604,12 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
// The compatibility check below is security-critical: it keeps exact
|
||||
// media-type matching by default while allowing safe text/plain
|
||||
// refinements such as JSON, CSV, and Markdown now that upload
|
||||
// classification can return richer stored media types. Combined with
|
||||
// the X-Content-Type-Options: nosniff header applied globally, this
|
||||
// still prevents clients from smuggling binary or active content under
|
||||
// a safer declared Content-Type.
|
||||
// media-type matching by default while allowing application/
|
||||
// octet-stream uploads to defer to byte classification, and letting
|
||||
// text/plain refine to safe text subtypes such as JSON, CSV, and
|
||||
// Markdown. Combined with the X-Content-Type-Options: nosniff header
|
||||
// applied globally, this still prevents clients from smuggling binary
|
||||
// or active content under a safer declared Content-Type.
|
||||
if !chatfiles.IsCompatibleUploadMediaType(contentType, detected) {
|
||||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
|
||||
Message: "File content type does not match Content-Type header.",
|
||||
|
||||
@@ -8654,6 +8654,54 @@ widgets,3
|
||||
require.NotEqual(t, uuid.Nil, resp.ID)
|
||||
})
|
||||
|
||||
t.Run("Success/OctetStreamPNG", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
client := newChatClient(t)
|
||||
firstUser := coderdtest.CreateFirstUser(t, client.Client)
|
||||
|
||||
data := append([]byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A}, make([]byte, 64)...)
|
||||
uploaded, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/octet-stream", "test.png", bytes.NewReader(data))
|
||||
require.NoError(t, err)
|
||||
require.NotEqual(t, uuid.Nil, uploaded.ID)
|
||||
|
||||
got, contentType, err := client.GetChatFile(ctx, uploaded.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "image/png", contentType)
|
||||
require.Equal(t, data, got)
|
||||
})
|
||||
|
||||
t.Run("Success/OctetStreamMarkdown", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
client := newChatClient(t)
|
||||
firstUser := coderdtest.CreateFirstUser(t, client.Client)
|
||||
|
||||
data := []byte(`# Markdown upload
|
||||
|
||||
This arrived as octet-stream.
|
||||
`)
|
||||
uploaded, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/octet-stream", "notes.md", bytes.NewReader(data))
|
||||
require.NoError(t, err)
|
||||
require.NotEqual(t, uuid.Nil, uploaded.ID)
|
||||
|
||||
got, contentType, err := client.GetChatFile(ctx, uploaded.ID)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "text/markdown", contentType)
|
||||
require.Equal(t, data, got)
|
||||
})
|
||||
|
||||
t.Run("OctetStreamRejectsUnsupportedBytes", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
client := newChatClient(t)
|
||||
firstUser := coderdtest.CreateFirstUser(t, client.Client)
|
||||
|
||||
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/octet-stream", "payload.zip", bytes.NewReader([]byte("PK")))
|
||||
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
|
||||
require.Contains(t, sdkErr.Message, "Unsupported file type")
|
||||
})
|
||||
|
||||
t.Run("UnsupportedContentType", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
|
||||
+19
-15
@@ -13,6 +13,8 @@ import (
|
||||
|
||||
"github.com/gabriel-vasile/mimetype"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
)
|
||||
|
||||
const MaxStoredFileNameBytes = 255
|
||||
@@ -28,17 +30,17 @@ var (
|
||||
|
||||
utf8BOM = []byte{0xEF, 0xBB, 0xBF}
|
||||
|
||||
allowedStoredMediaTypes = map[string]struct{}{
|
||||
"image/png": {},
|
||||
"image/jpeg": {},
|
||||
"image/gif": {},
|
||||
"image/webp": {},
|
||||
"text/plain": {},
|
||||
"text/markdown": {},
|
||||
"text/csv": {},
|
||||
"application/json": {},
|
||||
"application/pdf": {},
|
||||
}
|
||||
// allowedStoredMediaTypes is derived from codersdk.AllChatAttachmentMediaTypes
|
||||
// so the frontend file picker and the server enforcement share a single
|
||||
// source of truth. Do not edit this map directly; add new entries to the
|
||||
// codersdk const block instead.
|
||||
allowedStoredMediaTypes = func() map[string]struct{} {
|
||||
m := make(map[string]struct{}, len(codersdk.AllChatAttachmentMediaTypes))
|
||||
for _, t := range codersdk.AllChatAttachmentMediaTypes {
|
||||
m[string(t)] = struct{}{}
|
||||
}
|
||||
return m
|
||||
}()
|
||||
|
||||
recordingArtifactMediaTypes = map[string]struct{}{
|
||||
"video/mp4": {},
|
||||
@@ -139,14 +141,16 @@ func PrepareRecordingArtifact(name, expectedMediaType string, data []byte) (stor
|
||||
|
||||
// IsCompatibleUploadMediaType reports whether an upload request that declared
|
||||
// declaredMediaType may be stored as storedMediaType after byte
|
||||
// classification. Exact matches are always compatible; the compatibility
|
||||
// table only covers explicit refinements like text/plain uploads that safely
|
||||
// store as richer text subtypes.
|
||||
// classification. Exact matches are always compatible. Clients that declare
|
||||
// application/octet-stream are treated as "unknown", so the classified bytes
|
||||
// decide the stored type. The compatibility table also covers explicit
|
||||
// refinements like text/plain uploads that safely store as richer text
|
||||
// subtypes.
|
||||
func IsCompatibleUploadMediaType(declaredMediaType, storedMediaType string) bool {
|
||||
declaredMediaType = BaseMediaType(declaredMediaType)
|
||||
storedMediaType = BaseMediaType(storedMediaType)
|
||||
|
||||
if declaredMediaType == storedMediaType {
|
||||
if declaredMediaType == storedMediaType || declaredMediaType == "application/octet-stream" {
|
||||
return true
|
||||
}
|
||||
if declaredMediaType != "text/plain" {
|
||||
|
||||
@@ -270,6 +270,18 @@ func TestIsCompatibleUploadMediaType(t *testing.T) {
|
||||
stored: "text/plain",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "OctetStreamMatchesPNG",
|
||||
declared: "application/octet-stream",
|
||||
stored: "image/png",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "OctetStreamMatchesJSON",
|
||||
declared: "application/octet-stream",
|
||||
stored: "application/json",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "TextPlainRefinesToMarkdown",
|
||||
declared: "text/plain",
|
||||
|
||||
Reference in New Issue
Block a user