diff --git a/pkg/routes/api/v2/avatar_upload.go b/pkg/routes/api/v2/avatar_upload.go index d085a8c70..b56566ef8 100644 --- a/pkg/routes/api/v2/avatar_upload.go +++ b/pkg/routes/api/v2/avatar_upload.go @@ -37,13 +37,16 @@ import ( // Huma's MultipartFormFiles renders the "avatar" field as a binary file in the // generated OpenAPI spec; the file bytes are read from in.RawBody.Data().Avatar. type avatarUploadInput struct { - // contentType is intentionally permissive: the real, byte-level image - // check is done in the handler via mimetype.DetectReader (the same - // allow-list v1 uses), not via the client-declared part Content-Type. - // "image/*" here would make Huma reject on the declared header before the - // handler runs and trust a value the client controls. + // contentType lists the part Content-Types Huma's MimeTypeValidator accepts + // before our handler runs. Browsers set a real image Content-Type on the + // part (image/png, image/jpeg, ...) while programmatic clients often send + // application/octet-stream, so both must be allowed or a legitimate upload + // would be rejected with a 422 before reaching the handler. This is NOT the + // security gate: the real, byte-level image check is done in the handler via + // mimetype.DetectReader (the same allow-list v1 uses); the part Content-Type + // is client-controlled and must never be trusted on its own. RawBody huma.MultipartFormFiles[struct { - Avatar huma.FormFile `form:"avatar" contentType:"application/octet-stream" required:"true" doc:"The avatar image to upload. Must be an image; it is resized server-side and re-encoded as PNG."` + Avatar huma.FormFile `form:"avatar" contentType:"image/png,image/jpeg,image/gif,image/webp,image/svg+xml,application/octet-stream" required:"true" doc:"The avatar image to upload. Must be an image; it is resized server-side and re-encoded as PNG."` }] } diff --git a/pkg/webtests/huma_avatar_upload_test.go b/pkg/webtests/huma_avatar_upload_test.go index 232057539..f92a65418 100644 --- a/pkg/webtests/huma_avatar_upload_test.go +++ b/pkg/webtests/huma_avatar_upload_test.go @@ -19,12 +19,14 @@ package webtests import ( "bytes" "encoding/json" + "fmt" "image" "image/color" "image/png" "mime/multipart" "net/http" "net/http/httptest" + "net/textproto" "testing" "code.vikunja.io/api/pkg/db" @@ -54,6 +56,8 @@ func pngBytes(t *testing.T) []byte { // multipartAvatarBody returns a multipart/form-data body with a single // "avatar" file field plus the matching Content-Type header (with boundary). +// CreateFormFile sets the part's Content-Type to application/octet-stream, which +// mirrors how many programmatic clients upload. func multipartAvatarBody(t *testing.T, fieldName, filename string, content []byte) (*bytes.Buffer, string) { t.Helper() buf := &bytes.Buffer{} @@ -66,6 +70,24 @@ func multipartAvatarBody(t *testing.T, fieldName, filename string, content []byt return buf, w.FormDataContentType() } +// multipartAvatarBodyWithPartType is like multipartAvatarBody but lets the +// caller set the part's Content-Type header, mirroring how a browser declares a +// real image type (e.g. image/png) on the file part. +func multipartAvatarBodyWithPartType(t *testing.T, fieldName, filename, partContentType string, content []byte) (*bytes.Buffer, string) { + t.Helper() + buf := &bytes.Buffer{} + w := multipart.NewWriter(buf) + h := make(textproto.MIMEHeader) + h.Set("Content-Disposition", fmt.Sprintf(`form-data; name=%q; filename=%q`, fieldName, filename)) + h.Set("Content-Type", partContentType) + fw, err := w.CreatePart(h) + require.NoError(t, err) + _, err = fw.Write(content) + require.NoError(t, err) + require.NoError(t, w.Close()) + return buf, w.FormDataContentType() +} + // uploadAvatarRequest dispatches a multipart avatar upload against a prepared echo.Echo. func uploadAvatarRequest(t *testing.T, e *echo.Echo, body *bytes.Buffer, contentType, token string) *httptest.ResponseRecorder { t.Helper() @@ -99,6 +121,27 @@ func TestAvatarUpload(t *testing.T) { assert.NotZero(t, u.AvatarFileID) }) + t.Run("Real image content-type on the part", func(t *testing.T) { + // Browsers declare a real image Content-Type (e.g. image/png) on the + // file part. Huma's MimeTypeValidator must accept it so the request + // reaches the handler instead of being rejected with a 422. + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + body, contentType := multipartAvatarBodyWithPartType(t, "avatar", "avatar.png", "image/png", pngBytes(t)) + rec := uploadAvatarRequest(t, e, body, contentType, token) + require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), "uploaded successfully") + + s := db.NewSession() + defer s.Close() + u, err := user.GetUserByID(s, testuser1.ID) + require.NoError(t, err) + assert.Equal(t, "upload", u.AvatarProvider) + assert.NotZero(t, u.AvatarFileID) + }) + t.Run("Non-image rejected", func(t *testing.T) { e, err := setupTestEnv() require.NoError(t, err)