fix: make sure to use unaltered fields when saving a shadow copy for updated profiles or comments (#8533)

Follow-up of !6977

### Manual testing
- User **S** creates an organization **O** and posts a comment **C** (on a random issue);
- User **R** report as abuse the comment **C**, the organization **O** as well as the user **S**;
- User **S** changes the content of comment **C** and the description of organization **O** as well as the description of their own profile;
- Check (within DB) that shadow copies are being created (and linked to corresponding abuse reports) for comment **C**, organization **O** and user **S** and the content is the one from the moment when the reports were submitted (therefore before the updates made by **S**).

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8533
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: floss4good <floss4good@disroot.org>
Co-committed-by: floss4good <floss4good@disroot.org>

models/fixtures/ModerationFeatures/abuse_report.yml

@@ -0,0 +1,21 @@
+-
+  id: 1
+  status: 1
+  reporter_id: 2  # @user2
+  content_type: 4 # Comment
+  content_id: 18  # user2/repo2/issues/2#issuecomment-18
+  category: 2     # Spam
+  remarks: The comment I'm reporting is pure SPAM.
+  shadow_copy_id: null
+  created_unix: 1752697980 # 2025-07-16 20:33:00
+
+-
+  id: 2
+  status: 1        # Open
+  reporter_id: 2   # @user2
+  content_type: 1  # User (users or organizations)
+  content_id: 1002 # @alexsmith
+  category: 2      # Spam
+  remarks: This user just posted a spammy comment on my issue.
+  shadow_copy_id: null
+  created_unix: 1752698010 # 2025-07-16 20:33:30

models/fixtures/ModerationFeatures/comment.yml

@@ -0,0 +1,7 @@
+- # This is a spam comment (abusive content), created for testing moderation functionalities.
+  id: 18
+  type: 0 # Standard comment
+  poster_id: 1002 # @alexsmith
+  issue_id: 7 # user2/repo2#2
+  content: If anyone needs help for promoting their business online using SEO, just contact me (check my profile page).
+  created_unix: 1752697860 # 2025-07-16 20:31:00

models/fixtures/ModerationFeatures/user.yml

@@ -0,0 +1,22 @@
+- # This user is a spammer and will create abusive content (for testing moderation functionalities).
+  id: 1002
+  lower_name: alexsmith
+  name: alexsmith
+  full_name: Alex Smith
+  email: alexsmith@example.org
+  keep_email_private: false
+  passwd: passwdSalt:password
+  passwd_hash_algo: dummy
+  type: 0
+  location: '@master@seo.net'
+  website: http://promote-your-business.biz
+  pronouns: SEO
+  salt: passwdSalt
+  description: I can help you promote your business online using SEO.
+  created_unix: 1752697800 # 2025-07-16 20:30:00
+  is_active: true
+  is_admin: false
+  is_restricted: false
+  avatar: avatar-hash-1002
+  avatar_email: alexsmith@example.org
+  use_custom_avatar: false

models/issues/comment.go

@@ -1156,7 +1156,7 @@ func UpdateComment(ctx context.Context, c *Comment, contentVersion int, doer *us
 	defer committer.Close()
 
 	// If the comment was reported as abusive, a shadow copy should be created before first update.
-	if err := IfNeededCreateShadowCopyForComment(ctx, c); err != nil {
+	if err := IfNeededCreateShadowCopyForComment(ctx, c, true); err != nil {
 		return err
 	}
 
@@ -1197,7 +1197,7 @@ func DeleteComment(ctx context.Context, comment *Comment) error {
 	e := db.GetEngine(ctx)
 
 	// If the comment was reported as abusive, a shadow copy should be created before deletion.
-	if err := IfNeededCreateShadowCopyForComment(ctx, comment); err != nil {
+	if err := IfNeededCreateShadowCopyForComment(ctx, comment, false); err != nil {
 		return err
 	}
 

models/issues/moderation.go

@@ -87,13 +87,19 @@ func IfNeededCreateShadowCopyForIssue(ctx context.Context, issue *Issue) error {
 // IfNeededCreateShadowCopyForComment checks if for the given comment there are any reports of abusive content submitted
 // and if found a shadow copy of relevant comment fields will be stored into DB and linked to the above report(s).
 // This function should be called before a comment is deleted or updated.
-func IfNeededCreateShadowCopyForComment(ctx context.Context, comment *Comment) error {
+func IfNeededCreateShadowCopyForComment(ctx context.Context, comment *Comment, forUpdates bool) error {
 	shadowCopyNeeded, err := moderation.IsShadowCopyNeeded(ctx, moderation.ReportedContentTypeComment, comment.ID)
 	if err != nil {
 		return err
 	}
 
 	if shadowCopyNeeded {
+		if forUpdates {
+			// get the unaltered comment fields (for updates the provided variable is already altered but not yet saved)
+			if comment, err = GetCommentByID(ctx, comment.ID); err != nil {
+				return err
+			}
+		}
 		commentData := newCommentData(comment)
 		content, err := json.Marshal(commentData)
 		if err != nil {

models/user/moderation.go

@@ -73,16 +73,20 @@ var userDataColumnNames = sync.OnceValue(func() []string {
 // and if found a shadow copy of relevant user fields will be stored into DB and linked to the above report(s).
 // This function should be called before a user is deleted or updated.
 //
+// In case the User object was already altered before calling this method, just provide the userID and
+// nil for unalteredUser; when it is decided that a shadow copy should be created and unalteredUser is nil,
+// the user will be retrieved from DB based on the provided userID.
+//
 // For deletions alteredCols argument must be omitted.
 //
 // In case of updates it will first checks whether any of the columns being updated (alteredCols argument)
 // is relevant for moderation purposes (i.e. included in the UserData struct).
-func IfNeededCreateShadowCopyForUser(ctx context.Context, user *User, alteredCols ...string) error {
+func IfNeededCreateShadowCopyForUser(ctx context.Context, userID int64, unalteredUser *User, alteredCols ...string) error {
 	// TODO: this can be triggered quite often (e.g. by routers/web/repo/middlewares.go SetDiffViewStyle())
 
 	shouldCheckIfNeeded := len(alteredCols) == 0 // no columns being updated, therefore a deletion
 	if !shouldCheckIfNeeded {
-		// for updates we need to go further only if certain column are being changed
+		// for updates we need to go further only if certain columns are being changed
 		for _, colName := range userDataColumnNames() {
 			if shouldCheckIfNeeded = slices.Contains(alteredCols, colName); shouldCheckIfNeeded {
 				break
@@ -94,18 +98,23 @@ func IfNeededCreateShadowCopyForUser(ctx context.Context, user *User, alteredCol
 		return nil
 	}
 
-	shadowCopyNeeded, err := moderation.IsShadowCopyNeeded(ctx, moderation.ReportedContentTypeUser, user.ID)
+	shadowCopyNeeded, err := moderation.IsShadowCopyNeeded(ctx, moderation.ReportedContentTypeUser, userID)
 	if err != nil {
 		return err
 	}
 
 	if shadowCopyNeeded {
-		userData := newUserData(user)
+		if unalteredUser == nil {
+			if unalteredUser, err = GetUserByID(ctx, userID); err != nil {
+				return err
+			}
+		}
+		userData := newUserData(unalteredUser)
 		content, err := json.Marshal(userData)
 		if err != nil {
 			return err
 		}
-		return moderation.CreateShadowCopyForUser(ctx, user.ID, string(content))
+		return moderation.CreateShadowCopyForUser(ctx, userID, string(content))
 	}
 
 	return nil

models/user/user.go

@@ -927,7 +927,9 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error {
 
 	// If the user was reported as abusive and any of the columns being updated is relevant
 	// for moderation purposes a shadow copy should be created before first update.
-	if err := IfNeededCreateShadowCopyForUser(ctx, u, cols...); err != nil {
+	// Since u is already altered at this point we are sending nil instead as an argument
+	// so that the unaltered version will be retrieved from DB.
+	if err := IfNeededCreateShadowCopyForUser(ctx, u.ID, nil, cols...); err != nil {
 		return err
 	}
 

services/issue/comments_test.go

@@ -8,9 +8,11 @@ import (
 
 	"forgejo.org/models/db"
 	issues_model "forgejo.org/models/issues"
+	"forgejo.org/models/moderation"
 	"forgejo.org/models/unittest"
 	user_model "forgejo.org/models/user"
 	webhook_model "forgejo.org/models/webhook"
+	"forgejo.org/modules/json"
 	"forgejo.org/modules/setting"
 	"forgejo.org/modules/test"
 	issue_service "forgejo.org/services/issue"
@@ -148,3 +150,40 @@ func TestUpdateComment(t *testing.T) {
 		unittest.AssertNotExistsBean(t, &issues_model.ContentHistory{CommentID: comment.ID})
 	})
 }
+
+func TestCreateShadowCopyOnCommentUpdate(t *testing.T) {
+	defer unittest.OverrideFixtures("models/fixtures/ModerationFeatures")()
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	userAlexSmithID := int64(1002)
+	spamCommentID := int64(18) // posted by @alexsmith
+	abuseReportID := int64(1)  // submitted for above comment
+	newCommentContent := "If anyone needs help, just contact me."
+
+	// Retrieve the abusive user (@alexsmith), their SPAM comment and the abuse report already created for this comment.
+	poster := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userAlexSmithID})
+	comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: spamCommentID, PosterID: poster.ID})
+	report := unittest.AssertExistsAndLoadBean(t, &moderation.AbuseReport{
+		ID:          abuseReportID,
+		ContentType: moderation.ReportedContentTypeComment,
+		ContentID:   comment.ID,
+	})
+	// The report should not already have a shadow copy linked.
+	assert.False(t, report.ShadowCopyID.Valid)
+
+	// The abusive user is updating their comment.
+	oldContent := comment.Content
+	comment.Content = newCommentContent
+	require.NoError(t, issue_service.UpdateComment(t.Context(), comment, 0, poster, oldContent))
+
+	// Reload the report.
+	report = unittest.AssertExistsAndLoadBean(t, &moderation.AbuseReport{ID: report.ID})
+	// A shadow copy should have been created and linked to our report.
+	assert.True(t, report.ShadowCopyID.Valid)
+	// Retrieve the newly created shadow copy and unmarshal the stored JSON so that we can check the values.
+	shadowCopy := unittest.AssertExistsAndLoadBean(t, &moderation.AbuseReportShadowCopy{ID: report.ShadowCopyID.Int64})
+	shadowCopyCommentData := new(issues_model.CommentData)
+	require.NoError(t, json.Unmarshal([]byte(shadowCopy.RawValue), &shadowCopyCommentData))
+	// Check to see if the initial content of the comment was stored within the shadow copy.
+	assert.Equal(t, oldContent, shadowCopyCommentData.Content)
+}

services/user/delete.go

@@ -218,7 +218,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
 	// ***** END: ExternalLoginUser *****
 
 	// If the user was reported as abusive, a shadow copy should be created before deletion.
-	if err = user_model.IfNeededCreateShadowCopyForUser(ctx, u); err != nil {
+	if err = user_model.IfNeededCreateShadowCopyForUser(ctx, u.ID, u); err != nil {
 		return err
 	}
 

services/user/email.go

@@ -205,7 +205,7 @@ func MakeEmailAddressPrimary(ctx context.Context, u *user_model.User, newPrimary
 	oldPrimaryEmail := u.Email
 
 	// If the user was reported as abusive, a shadow copy should be created before first update (of certain columns).
-	if err = user_model.IfNeededCreateShadowCopyForUser(ctx, u, "email"); err != nil {
+	if err = user_model.IfNeededCreateShadowCopyForUser(ctx, u.ID, u, "email"); err != nil {
 		return err
 	}
 

services/user/user_test.go

@@ -15,10 +15,13 @@ import (
 	asymkey_model "forgejo.org/models/asymkey"
 	"forgejo.org/models/auth"
 	"forgejo.org/models/db"
+	"forgejo.org/models/moderation"
 	"forgejo.org/models/organization"
 	repo_model "forgejo.org/models/repo"
 	"forgejo.org/models/unittest"
 	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/json"
+	"forgejo.org/modules/optional"
 	"forgejo.org/modules/setting"
 	"forgejo.org/modules/test"
 	"forgejo.org/modules/timeutil"
@@ -277,3 +280,56 @@ func TestDeleteInactiveUsers(t *testing.T) {
 	unittest.AssertExistsIf(t, true, newUser)
 	unittest.AssertExistsIf(t, true, newEmail)
 }
+
+func TestCreateShadowCopyOnUserUpdate(t *testing.T) {
+	defer unittest.OverrideFixtures("models/fixtures/ModerationFeatures")()
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	userAlexSmithID := int64(1002)
+	abuseReportID := int64(2)     // submitted for @alexsmith
+	newDummyValue := "[REDACTED]" // used for updating profile text fields
+
+	// Retrieve the abusive user (@alexsmith) and the abuse report already created for this user.
+	abuser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userAlexSmithID})
+	report := unittest.AssertExistsAndLoadBean(t, &moderation.AbuseReport{
+		ID:          abuseReportID,
+		ContentType: moderation.ReportedContentTypeUser,
+		ContentID:   abuser.ID,
+	})
+	// The report should not already have a shadow copy linked.
+	assert.False(t, report.ShadowCopyID.Valid)
+
+	// Keep a copy of old field values before updating them.
+	oldUserData := user_model.UserData{
+		FullName:    abuser.FullName,
+		Location:    abuser.Location,
+		Website:     abuser.Website,
+		Pronouns:    abuser.Pronouns,
+		Description: abuser.Description,
+	}
+
+	// The abusive user is updating their profile.
+	opts := &UpdateOptions{
+		FullName:    optional.Some(newDummyValue),
+		Location:    optional.Some(newDummyValue),
+		Website:     optional.Some(newDummyValue),
+		Pronouns:    optional.Some(newDummyValue),
+		Description: optional.Some(newDummyValue),
+	}
+	require.NoError(t, UpdateUser(t.Context(), abuser, opts))
+
+	// Reload the report.
+	report = unittest.AssertExistsAndLoadBean(t, &moderation.AbuseReport{ID: report.ID})
+	// A shadow copy should have been created and linked to our report.
+	assert.True(t, report.ShadowCopyID.Valid)
+	// Retrieve the newly created shadow copy and unmarshal the stored JSON so that we can check the values.
+	shadowCopy := unittest.AssertExistsAndLoadBean(t, &moderation.AbuseReportShadowCopy{ID: report.ShadowCopyID.Int64})
+	shadowCopyUserData := new(user_model.UserData)
+	require.NoError(t, json.Unmarshal([]byte(shadowCopy.RawValue), &shadowCopyUserData))
+	// Check to see if the initial field values of the user were stored within the shadow copy.
+	assert.Equal(t, oldUserData.FullName, shadowCopyUserData.FullName)
+	assert.Equal(t, oldUserData.Location, shadowCopyUserData.Location)
+	assert.Equal(t, oldUserData.Website, shadowCopyUserData.Website)
+	assert.Equal(t, oldUserData.Pronouns, shadowCopyUserData.Pronouns)
+	assert.Equal(t, oldUserData.Description, shadowCopyUserData.Description)
+}