Skip to content

Commit

Permalink
FIX: only hide posts detected explicitly as spam (#1070)
Browse files Browse the repository at this point in the history
When enabling spam scanner it there may be old unscanned posts
this can create a risky situation where spam scanner operates
on legit posts during false positives

To keep this a lot safer we no longer try to hide old stuff by
the spammers.
  • Loading branch information
SamSaffron authored Jan 15, 2025
1 parent c881f8b commit 81b952d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 17 deletions.
26 changes: 9 additions & 17 deletions lib/ai_moderation/spam_scanner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,26 +377,18 @@ def self.handle_spam(post, log)
silencer.silence

# silencer will not hide tl1 posts, so we do this here
hide_posts_and_topics(post.user)
hide_post(post)
end

def self.hide_posts_and_topics(user)
Post
.where(user_id: user.id)
.where("created_at > ?", 24.hours.ago)
.update_all(
[
"hidden = true, hidden_reason_id = COALESCE(hidden_reason_id, ?)",
Post.hidden_reasons[:new_user_spam_threshold_reached],
],
)
topic_ids =
Post
.where(user_id: user.id, post_number: 1)
.where("created_at > ?", 24.hours.ago)
.select(:topic_id)
def self.hide_post(post)
Post.where(id: post.id).update_all(
[
"hidden = true, hidden_reason_id = COALESCE(hidden_reason_id, ?)",
Post.hidden_reasons[:new_user_spam_threshold_reached],
],
)

Topic.where(id: topic_ids).update_all(visible: false)
Topic.where(id: post.topic_id).update_all(visible: false) if post.post_number == 1
end
end
end
Expand Down
30 changes: 30 additions & 0 deletions spec/lib/modules/ai_moderation/spam_scanner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require "rails_helper"

RSpec.describe DiscourseAi::AiModeration::SpamScanner do
fab!(:moderator)
fab!(:user) { Fabricate(:user, trust_level: TrustLevel[0]) }
fab!(:topic)
fab!(:post) { Fabricate(:post, user: user, topic: topic) }
Expand Down Expand Up @@ -183,6 +184,29 @@
end
end

describe ".hide_post" do
fab!(:spam_post) { Fabricate(:post, user: user) }
fab!(:second_spam_post) { Fabricate(:post, topic: spam_post.topic, user: user) }

it "hides spam post and topic for first post" do
described_class.hide_post(spam_post)

expect(spam_post.reload.hidden).to eq(true)
expect(second_spam_post.reload.hidden).to eq(false)
expect(spam_post.reload.hidden_reason_id).to eq(
Post.hidden_reasons[:new_user_spam_threshold_reached],
)
end

it "doesn't hide the topic for non-first posts" do
described_class.hide_post(second_spam_post)

expect(spam_post.reload.hidden).to eq(false)
expect(second_spam_post.reload.hidden).to eq(true)
expect(spam_post.topic.reload.visible).to eq(true)
end
end

describe "integration test" do
fab!(:llm_model)
let(:api_audit_log) { Fabricate(:api_audit_log) }
Expand Down Expand Up @@ -257,6 +281,12 @@

expect(log.reviewable).to be_present
expect(log.reviewable.created_by_id).to eq(described_class.flagging_user.id)

log.reviewable.perform(moderator, :disagree_and_restore)

expect(post.reload.hidden?).to eq(false)
expect(post.topic.reload.visible).to eq(true)
expect(post.user.reload.silenced?).to eq(false)
end
end
end

0 comments on commit 81b952d

Please sign in to comment.