Skip to content

Check names, phone numbers and bare URLs in spam dashboards [sc-46591] - #3616

Open
EliezerIsrael wants to merge 1 commit into
masterfrom
spam-names
Open

Check names, phone numbers and bare URLs in spam dashboards [sc-46591]#3616
EliezerIsrael wants to merge 1 commit into
masterfrom
spam-names

Conversation

@EliezerIsrael

Copy link
Copy Markdown
Member

Shortcut: https://app.shortcut.com/sefaria/story/46591

The vector

Spammers moved to the first and last name fields, which neither spam dashboard checked.

Names live on the Django User record in Postgres, not in the Mongo profile doc, so profile_spam_dashboard's query could not reach them. The slug is checked and is built from the name at signup — but UserProfile.save() pushes a name edit to the Django User without regenerating the slug. Sign up as "Miriam Cohen", then rename to "Coinbase Support", and the slug stays miriam-cohen forever. The check was evaded entirely.

Changes

  • New sefaria/spam.py holds the patterns as one source of truth.
  • Names are matched in Postgres and folded into the Mongo query by user id. First and last are concatenated before matching, so a number split across the two fields (1-800 / 555-1234) is still caught.
  • Free text is checked for phone numbers and bare URLs, not just href markup. That rule is spam_text_clauses(), and both dashboards point it at their own fields — so sheet_spam_dashboard picks up the same coverage, having previously matched href only.
  • Links back to Sefaria stay excluded, matching the carve-out the existing href check already made.

Trade-offs worth a reviewer's eye

  • The phone pattern wants 9+ digits so that prose like a 2020-2024 date range is not read as a phone number. The cost is that a bare 7-digit local number is missed — it has the same shape as a year range, and spam numbers carry an area or country code anyway.
  • The sheet dashboard's $or grows from 4 regex branches to 12, on a candidate set already narrowed by dateCreated, owner, includedRefs and sources.ref.
  • Name matching uses __iregex, so those patterns stay inside the regex subset Postgres, Python and PCRE share — no \b, \d or lookarounds. The free-text URL variant runs only in Mongo and does use a lookahead.

Tests

sefaria/tests/spam_dashboard_test.py. The negative cases carry the weight, since the only action this queue offers is irreversible deletion: Miriam Cohen must not read as the .co TLD, Jean-Pierre St.Clair and B. Cohen must survive the URL pattern, 2020-2024 and Born 1948. Teaching since 1975. must not read as phone numbers, and text linking to sefaria.org must stay clear.

Verified locally by driving the test functions against a real auth_user table in SQLite (35/35). The repo's pytest suite could not start in my environment — sefaria/settings.py:404 raises NameError: APPLE_SSO_IOS_BUNDLE_ID without the SSO entries in local_settings, pre-existing and unrelated to this branch. CI is the real gate, both for pytest proper and for Postgres iregex.

🤖 Generated with Claude Code

Spammers moved to the first and last name fields, which neither dashboard
checked. Names live on the Django User record, not in the Mongo profile doc,
so the profile query could not reach them. The slug is checked and is built
from the name at signup, but a name edit never regenerates it, so signing up
benign and renaming afterwards evaded the check entirely.

Names are now matched in Postgres and folded back into the Mongo query by user
id, with first and last concatenated so that a phone number split across the
two fields is still caught.

Free text is now checked for phone numbers and bare URLs rather than href
markup alone. That rule lives in the new sefaria/spam.py as spam_text_clauses()
and both dashboards point it at their own fields, so the sheet dashboard picks
up the same coverage -- it previously matched href only. Links back to Sefaria
stay excluded, matching the carve-out the existing href check already made.

The phone pattern wants 9+ digits so that prose like a 2020-2024 date range is
not read as a phone number. The trade is that a bare 7 digit local number is
missed, having the same shape as a year range.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gitvelocity-reviewer

Copy link
Copy Markdown

📊 Code Quality Score: 31/100

Base Score 51 × ESF 0.6 = 30.6, rounded to 31

Category Score Factors
🔭 Scope 9/20 sefaria/spam.py is a new file. sefaria/tests/spam_dashboard_test.py is a new file. sefaria/views.py has two call sites updated to import from the new module. All three files belong to the spam-detection subsystem; no new endpoints, migrations, or external service integrations are introduced.
🏗️ Architecture 10/20 spam.py extracts inline regex string literals and an inline spam_keywords_regex definition from profile_spam_dashboard and sheet_spam_dashboard into a shared module. spam_name_user_ids introduces a new pattern where a Postgres ORM query result is fed into a Mongo $in clause, crossing the two data stores. No new external dependencies are added.
⚙️ Implementation 11/20 SPAM_URL_REGEX and SPAM_NAME_URL_REGEX are built from a shared TLD list so the two patterns cannot drift apart. SPAM_PHONE_REGEX uses {8,} with a loose grouping character class to distinguish phone numbers from date ranges. The module docstring documents the Postgres/Python/PCRE regex subset constraint and explains why `([^a-z]
⚠️ Risk 7/20 spam_name_user_ids now executes a synchronous Postgres query during Mongo query construction inside profile_spam_dashboard; a Postgres failure raises inside the view with no isolation. The href pattern (SPAM_HREF_REGEX) is preserved from the original inline regex, but the profile dashboard gains phone and URL patterns that were not previously applied to bios. Both dashboards feed a review queue whose action is irreversible deletion, so false positives have direct user impact.
✅ Quality 12/15 spam_dashboard_test.py drives 10 positive and 8 negative NAME_CASES and 6 positive and 8 negative TEXT_CASES through parametrized tests. test_spam_name_user_ids_matches_the_name_patterns runs the ORM query against a real Django test database and checks every NAME_CASES entry. test_spam_name_user_ids_ignores_users_below_the_cutoff verifies the min_user_id filter. test_spam_text_clauses_covers_every_field_it_is_given checks the structural output of the clause generator. SPAM_KEYWORDS_REGEX (the slug pattern used in views.py) has no direct test.
🔒 Perf / Security 2/5 SPAM_PHONE_REGEX uses a bounded repetition {8,} rather than an unbounded quantifier, limiting backtracking on long digit strings. No ReDoS analysis or benchmark accompanies the regex changes.

Was this score accurate? 👍 Yes · 👎 No

How this was scored →

Scored by GitVelocity · How are scores calculated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant