fix: validate group avatar attachments

server/im_http.go

@@ -4655,6 +4655,12 @@ func serveIMGroupCreate(wrt http.ResponseWriter, req *http.Request) {
 		CreatedAt:         now,
 		UpdatedAt:         now,
 	}
+	if settings.AvatarFileID != "" {
+		if err := linkIMGroupAvatarAttachment(principal.TenantID, topicName, settings.AvatarFileID); err != nil {
+			writeIMError(wrt, req, http.StatusBadRequest, 30000, imReasonMalformedRequest, "malformed request", map[string]any{"field": "avatar_file_id"})
+			return
+		}
+	}
 	memberIDs := make([]types.Uid, 0, len(body.MemberUserIDs))
 	seen := map[types.Uid]bool{principal.Uid: true}
 	for _, rawID := range body.MemberUserIDs {
@@ -4671,11 +4677,6 @@ func serveIMGroupCreate(wrt http.ResponseWriter, req *http.Request) {
 		writeIMRelationError(wrt, req, err)
 		return
 	}
-	if settings.AvatarFileID != "" {
-		if err := store.Files.LinkAttachments(principal.TenantID, topicName, types.ZeroUid, []string{settings.AvatarFileID}); err != nil {
-			logs.Warn.Printf("group[%s]: failed to link avatar attachment: %v", topicName, err)
-		}
-	}
 	dispatchIMGroupEvents(principal.TenantID)
 	created, err := store.Topics.Get(principal.TenantID, topicName)
 	if err != nil {
@@ -4932,7 +4933,14 @@ func serveIMGroupDetail(wrt http.ResponseWriter, req *http.Request, topic string
 			public["fn"] = name
 		}
 		if body.AvatarFileID != nil {
-			public["avatar_file_id"] = strings.TrimSpace(*body.AvatarFileID)
+			avatar := strings.TrimSpace(*body.AvatarFileID)
+			if avatar != "" {
+				if err := linkIMGroupAvatarAttachment(principal.TenantID, topic, avatar); err != nil {
+					writeIMError(wrt, req, http.StatusBadRequest, 30000, imReasonMalformedRequest, "malformed request", map[string]any{"field": "avatar_file_id"})
+					return
+				}
+			}
+			public["avatar_file_id"] = avatar
 		}
 		if body.Description != nil {
 			description := strings.TrimSpace(*body.Description)
@@ -4961,13 +4969,6 @@ func serveIMGroupDetail(wrt http.ResponseWriter, req *http.Request, topic string
 				return
 			}
 		}
-		if body.AvatarFileID != nil {
-			if avatar := strings.TrimSpace(*body.AvatarFileID); avatar != "" {
-				if err := store.Files.LinkAttachments(principal.TenantID, topic, types.ZeroUid, []string{avatar}); err != nil {
-					logs.Warn.Printf("group[%s]: failed to link avatar attachment: %v", topic, err)
-				}
-			}
-		}
 		dispatchIMGroupEvents(principal.TenantID)
 		updated, err := store.Topics.Get(principal.TenantID, topic)
 		if err != nil {
@@ -6949,6 +6950,14 @@ func normalizeIMUploadedFileID(raw string) string {
 	return ""
 }
 
+func linkIMGroupAvatarAttachment(tenantID types.TenantID, topic, avatarFileID string) error {
+	fileID := normalizeIMUploadedFileID(avatarFileID)
+	if fileID == "" {
+		return types.ErrMalformed
+	}
+	return store.Files.LinkAttachments(tenantID, topic, types.ZeroUid, []string{fileID})
+}
+
 func parseIMLimit(raw string, fallback, max int) int {
 	limit := fallback
 	if raw != "" {

server/im_http_test.go

@@ -4095,6 +4095,7 @@ func TestServeIMGroupCreate(t *testing.T) {
 	defer ctrl.Finish()
 	oldStore := store.Store
 	oldGroupBusiness := store.GroupBusiness
+	oldFiles := store.Files
 	mockStore := mock_store.NewMockPersistentStorageInterface(ctrl)
 	tokenAuth := mock_auth.NewMockAuthHandler(ctrl)
 	mockStore.EXPECT().GetLogicalAuthHandler("token").Return(tokenAuth)
@@ -4107,23 +4108,27 @@ func TestServeIMGroupCreate(t *testing.T) {
 	oldSubs := store.Subs
 	topics := mock_store.NewMockTopicsPersistenceInterface(ctrl)
 	subs := mock_store.NewMockSubsPersistenceInterface(ctrl)
+	files := mock_store.NewMockFilePersistenceInterface(ctrl)
 	store.Topics = topics
 	store.Subs = subs
+	store.Files = files
 	groupBusiness := &testGroupBusinessStore{}
 	store.GroupBusiness = groupBusiness
 	defer func() {
 		store.Store = oldStore
 		store.Topics = oldTopics
 		store.Subs = oldSubs
+		store.Files = oldFiles
 		store.GroupBusiness = oldGroupBusiness
 	}()
+	avatarID := types.Uid(101).String()
 
 	topics.EXPECT().Get(types.TenantID(7), "grpnewgroup").Return(&types.Topic{
 		ObjHeader: types.ObjHeader{Id: "grpnewgroup"},
 		TenantID:  7,
 		Owner:     types.Uid(42).String(),
 		SubCnt:    2,
-		Public:    map[string]any{"name": "New Group", "avatar_file_id": "file-1", "description": "Project discussion", "version": 1},
+		Public:    map[string]any{"name": "New Group", "avatar_file_id": avatarID + ".jpg", "description": "Project discussion", "version": 1},
 	}, nil)
 	subs.EXPECT().Get(types.TenantID(7), "grpnewgroup", types.Uid(42), false).Return(&types.Subscription{
 		TenantID:  7,
@@ -4132,8 +4137,9 @@ func TestServeIMGroupCreate(t *testing.T) {
 		ModeWant:  types.ModeCFull,
 		ModeGiven: types.ModeCFull,
 	}, nil)
+	files.EXPECT().LinkAttachments(types.TenantID(7), "grpnewgroup", types.ZeroUid, []string{avatarID}).Return(nil)
 
-	req := httptest.NewRequest(http.MethodPost, "/v1/im/groups", bytes.NewBufferString(`{"name":"New Group","description":"Project discussion","member_user_ids":["usrAgAAAAAAAAA"],"avatar_file_id":"file-1"}`))
+	req := httptest.NewRequest(http.MethodPost, "/v1/im/groups", bytes.NewBufferString(`{"name":"New Group","description":"Project discussion","member_user_ids":["usrAgAAAAAAAAA"],"avatar_file_id":"`+avatarID+`.jpg"}`))
 	req.Header.Set("Authorization", "Bearer token-1")
 	rec := httptest.NewRecorder()
 
@@ -4243,6 +4249,7 @@ func TestServeIMGroupDetailPatchKeepsTopicProfileWhenBusinessSettingsMissing(t *
 	defer ctrl.Finish()
 	oldStore := store.Store
 	oldGroupBusiness := store.GroupBusiness
+	oldFiles := store.Files
 	mockStore := mock_store.NewMockPersistentStorageInterface(ctrl)
 	tokenAuth := mock_auth.NewMockAuthHandler(ctrl)
 	mockStore.EXPECT().GetLogicalAuthHandler("token").Return(tokenAuth)
@@ -4254,13 +4261,16 @@ func TestServeIMGroupDetailPatchKeepsTopicProfileWhenBusinessSettingsMissing(t *
 	oldSubs := store.Subs
 	topics := mock_store.NewMockTopicsPersistenceInterface(ctrl)
 	subs := mock_store.NewMockSubsPersistenceInterface(ctrl)
+	files := mock_store.NewMockFilePersistenceInterface(ctrl)
 	store.Topics = topics
 	store.Subs = subs
+	store.Files = files
 	store.GroupBusiness = &testGroupBusinessStore{updateSettingsErr: types.ErrNotFound}
 	defer func() {
 		store.Store = oldStore
 		store.Topics = oldTopics
 		store.Subs = oldSubs
+		store.Files = oldFiles
 		store.GroupBusiness = oldGroupBusiness
 	}()
 	selfSub := &types.Subscription{
@@ -4278,9 +4288,11 @@ func TestServeIMGroupDetailPatchKeepsTopicProfileWhenBusinessSettingsMissing(t *
 		SubCnt:    3,
 		Public:    map[string]any{"name": "Alpha", "version": 1},
 	}
+	avatarID := types.Uid(100).String()
 	var savedPublic map[string]any
 	subs.EXPECT().Get(types.TenantID(7), "grpAlpha", types.Uid(42), false).Return(selfSub, nil)
 	topics.EXPECT().Get(types.TenantID(7), "grpAlpha").Return(group, nil)
+	files.EXPECT().LinkAttachments(types.TenantID(7), "grpAlpha", types.ZeroUid, []string{avatarID}).Return(nil)
 	topics.EXPECT().Update(types.TenantID(7), "grpAlpha", gomock.Any()).DoAndReturn(
 		func(_ types.TenantID, _ string, update map[string]any) error {
 			public, ok := update["Public"].(map[string]any)
@@ -4303,7 +4315,7 @@ func TestServeIMGroupDetailPatchKeepsTopicProfileWhenBusinessSettingsMissing(t *
 		},
 	)
 
-	req := httptest.NewRequest(http.MethodPatch, "/v1/im/groups/grpAlpha", bytes.NewBufferString(`{"avatar_file_id":"file-avatar"}`))
+	req := httptest.NewRequest(http.MethodPatch, "/v1/im/groups/grpAlpha", bytes.NewBufferString(`{"avatar_file_id":"`+avatarID+`.jpg"}`))
 	req.Header.Set("Authorization", "Bearer token-1")
 	rec := httptest.NewRecorder()
 	serveIMGroupResource(rec, req)
@@ -4311,7 +4323,7 @@ func TestServeIMGroupDetailPatchKeepsTopicProfileWhenBusinessSettingsMissing(t *
 	if rec.Code != http.StatusOK {
 		t.Fatalf("patch status: got %d body=%s", rec.Code, rec.Body.String())
 	}
-	if data := decodeIMHTTPResponse(t, rec).Data.(map[string]any); data["avatar_file_id"] != "file-avatar" || data["version"].(float64) != 2 {
+	if data := decodeIMHTTPResponse(t, rec).Data.(map[string]any); data["avatar_file_id"] != avatarID+".jpg" || data["version"].(float64) != 2 {
 		t.Fatalf("unexpected patch response: %#v", data)
 	}
 }