From ab6579dbf195a8e0a52963af1afa96aa0c1d41eb Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Wed, 12 Mar 2025 18:15:50 +0000
Subject: [PATCH 1/6] Move typo checks to after_insert
---
warehouse/packaging/models.py | 89 +++++++++++++++++++++++++++++++++
warehouse/packaging/services.py | 83 ++----------------------------
2 files changed, 92 insertions(+), 80 deletions(-)
diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py
index 0ceb3cc635a9..4a097e62cd84 100644
--- a/warehouse/packaging/models.py
+++ b/warehouse/packaging/models.py
@@ -34,6 +34,7 @@
Text,
UniqueConstraint,
cast,
+ event,
func,
or_,
orm,
@@ -67,6 +68,7 @@
from warehouse.classifiers.models import Classifier
from warehouse.events.models import HasEvents
from warehouse.forklift import metadata
+from warehouse.helpdesk.interfaces import IAdminNotificationService
from warehouse.integrations.vulnerabilities.models import VulnerabilityRecord
from warehouse.observations.models import HasObservations
from warehouse.organizations.models import (
@@ -77,6 +79,10 @@
Team,
TeamProjectRole,
)
+from warehouse.packaging.interfaces import (
+ IProjectService,
+ ProjectNameUnavailableTypoSquattingError,
+)
from warehouse.sitemap.models import SitemapMixin
from warehouse.utils import dotted_navigator, wheel
from warehouse.utils.attrs import make_repr
@@ -496,6 +502,89 @@ def yanked_releases(self):
)
+@event.listens_for(Project, "after_insert")
+def receive_after_insert(mapper, connection, project):
+ request = get_current_request()
+ project_service = request.find_service(IProjectService)
+ name = project.name
+ try:
+ project_service.check_project_name_after_insert(name)
+ except ProjectNameUnavailableTypoSquattingError as exc:
+ request.log.warning(
+ "ProjectNameUnavailableTypoSquattingError",
+ check_name=exc.check_name,
+ existing_project_name=exc.existing_project_name,
+ )
+ # Send notification to Admins for review
+ notification_service = request.find_service(IAdminNotificationService)
+
+ warehouse_domain = request.registry.settings.get("warehouse.domain")
+ new_project_page = request.route_url(
+ "packaging.project",
+ name=name,
+ _host=warehouse_domain,
+ )
+ new_project_text = (
+ f"During `file_upload`, Project Create for "
+ f"*<{new_project_page}|{name}>* was detected as a potential "
+ f"typo by the `{exc.check_name!r}` check."
+ )
+ existing_project_page = request.route_url(
+ "packaging.project",
+ name=exc.existing_project_name,
+ _host=warehouse_domain,
+ )
+ existing_project_text = (
+ f"<{existing_project_page}|Existing project: "
+ f"{exc.existing_project_name}>"
+ )
+
+ webhook_payload = {
+ "blocks": [
+ {
+ "type": "header",
+ "text": {
+ "type": "plain_text",
+ "text": "TypoSnyper :warning:",
+ "emoji": True,
+ },
+ },
+ {
+ "type": "section",
+ "text": {
+ "type": "mrkdwn",
+ "text": new_project_text,
+ },
+ },
+ {
+ "type": "section",
+ "text": {
+ "type": "mrkdwn",
+ "text": existing_project_text,
+ },
+ },
+ {"type": "divider"},
+ {
+ "type": "context",
+ "elements": [
+ {
+ "type": "plain_text",
+ "text": "Once reviewed/confirmed, "
+ "react to this message with :white_check_mark:",
+ "emoji": True,
+ }
+ ],
+ },
+ ]
+ }
+ notification_service.send_notification(payload=webhook_payload)
+
+ request.metrics.increment(
+ "warehouse.packaging.services.create_project.typo_squatting",
+ tags=[f"check_name:{exc.check_name!r}"],
+ )
+
+
class DependencyKind(enum.IntEnum):
requires = 1
provides = 2
diff --git a/warehouse/packaging/services.py b/warehouse/packaging/services.py
index d3b14eb1fb8f..fe44870086e3 100644
--- a/warehouse/packaging/services.py
+++ b/warehouse/packaging/services.py
@@ -36,7 +36,6 @@
from warehouse.admin.flags import AdminFlagValue
from warehouse.email import send_pending_trusted_publisher_invalidated_email
from warehouse.events.tags import EventTag
-from warehouse.helpdesk.interfaces import IAdminNotificationService
from warehouse.metrics import IMetricsService
from warehouse.oidc.models import PendingOIDCPublisher
from warehouse.packaging.interfaces import (
@@ -485,6 +484,9 @@ def check_project_name(self, name: str) -> None:
).first():
raise ProjectNameUnavailableSimilarError(similar_project_name)
+ return None
+
+ def check_project_name_after_insert(self, name: str) -> None:
# Check for typo-squatting.
if typo_check_match := typo_check_name(canonicalize_name(name)):
raise ProjectNameUnavailableTypoSquattingError(
@@ -558,85 +560,6 @@ def create_project(
projecthelp=request.help_url(_anchor="project-name"),
),
) from None
- except ProjectNameUnavailableTypoSquattingError as exc:
- # Don't yet raise an error here, as we want to allow the
- # user to proceed with the project creation. We'll log a warning
- # instead.
- request.log.warning(
- "ProjectNameUnavailableTypoSquattingError",
- check_name=exc.check_name,
- existing_project_name=exc.existing_project_name,
- )
- # Send notification to Admins for review
- notification_service = request.find_service(IAdminNotificationService)
-
- warehouse_domain = request.registry.settings.get("warehouse.domain")
- new_project_page = request.route_url(
- "packaging.project",
- name=name,
- _host=warehouse_domain,
- )
- new_project_text = (
- f"During `file_upload`, Project Create for "
- f"*<{new_project_page}|{name}>* was detected as a potential "
- f"typo by the `{exc.check_name!r}` check."
- )
- existing_project_page = request.route_url(
- "packaging.project",
- name=exc.existing_project_name,
- _host=warehouse_domain,
- )
- existing_project_text = (
- f"<{existing_project_page}|Existing project: "
- f"{exc.existing_project_name}>"
- )
-
- webhook_payload = {
- "blocks": [
- {
- "type": "header",
- "text": {
- "type": "plain_text",
- "text": "TypoSnyper :warning:",
- "emoji": True,
- },
- },
- {
- "type": "section",
- "text": {
- "type": "mrkdwn",
- "text": new_project_text,
- },
- },
- {
- "type": "section",
- "text": {
- "type": "mrkdwn",
- "text": existing_project_text,
- },
- },
- {"type": "divider"},
- {
- "type": "context",
- "elements": [
- {
- "type": "plain_text",
- "text": "Once reviewed/confirmed, "
- "react to this message with :white_check_mark:",
- "emoji": True,
- }
- ],
- },
- ]
- }
- notification_service.send_notification(payload=webhook_payload)
-
- request.metrics.increment(
- "warehouse.packaging.services.create_project.typo_squatting",
- tags=[f"check_name:{exc.check_name!r}"],
- )
- # and continue with the project creation
- pass
# The project name is valid: create it and add it
project = Project(name=name)
From d3a87105fc27be870d5a283690024bc63030b010 Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Thu, 13 Mar 2025 14:21:35 +0000
Subject: [PATCH 2/6] Update tests
---
tests/unit/packaging/test_services.py | 2 +-
warehouse/packaging/models.py | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/unit/packaging/test_services.py b/tests/unit/packaging/test_services.py
index 71f53993317c..d9e489855d59 100644
--- a/tests/unit/packaging/test_services.py
+++ b/tests/unit/packaging/test_services.py
@@ -1057,7 +1057,7 @@ def test_check_project_name_typosquatting_prohibited(self, db_session):
ProhibitedProjectFactory.create(name="numpy")
with pytest.raises(ProjectNameUnavailableTypoSquattingError):
- service.check_project_name("numpi")
+ service.check_project_name_after_insert("numpi")
def test_check_project_name_ok(self, db_session):
service = ProjectService(session=db_session)
diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py
index 4a097e62cd84..0d58a9623db3 100644
--- a/warehouse/packaging/models.py
+++ b/warehouse/packaging/models.py
@@ -505,6 +505,9 @@ def yanked_releases(self):
@event.listens_for(Project, "after_insert")
def receive_after_insert(mapper, connection, project):
request = get_current_request()
+ if not request:
+ # Can't do anything if there isn't a request
+ return
project_service = request.find_service(IProjectService)
name = project.name
try:
From 97ed8aa478b3d60063ba6af63387ee19ba0973ae Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Thu, 13 Mar 2025 17:40:07 +0000
Subject: [PATCH 3/6] WIP
---
tests/unit/packaging/test_services.py | 2 +-
warehouse/packaging/models.py | 92 -------------------------
warehouse/packaging/services.py | 2 +-
warehouse/packaging/typosnyper.py | 99 +++++++++++++++++++++++++++
4 files changed, 101 insertions(+), 94 deletions(-)
diff --git a/tests/unit/packaging/test_services.py b/tests/unit/packaging/test_services.py
index d9e489855d59..29ac7c6f323d 100644
--- a/tests/unit/packaging/test_services.py
+++ b/tests/unit/packaging/test_services.py
@@ -1057,7 +1057,7 @@ def test_check_project_name_typosquatting_prohibited(self, db_session):
ProhibitedProjectFactory.create(name="numpy")
with pytest.raises(ProjectNameUnavailableTypoSquattingError):
- service.check_project_name_after_insert("numpi")
+ service.check_project_name_after_commit("numpi")
def test_check_project_name_ok(self, db_session):
service = ProjectService(session=db_session)
diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py
index 0d58a9623db3..0ceb3cc635a9 100644
--- a/warehouse/packaging/models.py
+++ b/warehouse/packaging/models.py
@@ -34,7 +34,6 @@
Text,
UniqueConstraint,
cast,
- event,
func,
or_,
orm,
@@ -68,7 +67,6 @@
from warehouse.classifiers.models import Classifier
from warehouse.events.models import HasEvents
from warehouse.forklift import metadata
-from warehouse.helpdesk.interfaces import IAdminNotificationService
from warehouse.integrations.vulnerabilities.models import VulnerabilityRecord
from warehouse.observations.models import HasObservations
from warehouse.organizations.models import (
@@ -79,10 +77,6 @@
Team,
TeamProjectRole,
)
-from warehouse.packaging.interfaces import (
- IProjectService,
- ProjectNameUnavailableTypoSquattingError,
-)
from warehouse.sitemap.models import SitemapMixin
from warehouse.utils import dotted_navigator, wheel
from warehouse.utils.attrs import make_repr
@@ -502,92 +496,6 @@ def yanked_releases(self):
)
-@event.listens_for(Project, "after_insert")
-def receive_after_insert(mapper, connection, project):
- request = get_current_request()
- if not request:
- # Can't do anything if there isn't a request
- return
- project_service = request.find_service(IProjectService)
- name = project.name
- try:
- project_service.check_project_name_after_insert(name)
- except ProjectNameUnavailableTypoSquattingError as exc:
- request.log.warning(
- "ProjectNameUnavailableTypoSquattingError",
- check_name=exc.check_name,
- existing_project_name=exc.existing_project_name,
- )
- # Send notification to Admins for review
- notification_service = request.find_service(IAdminNotificationService)
-
- warehouse_domain = request.registry.settings.get("warehouse.domain")
- new_project_page = request.route_url(
- "packaging.project",
- name=name,
- _host=warehouse_domain,
- )
- new_project_text = (
- f"During `file_upload`, Project Create for "
- f"*<{new_project_page}|{name}>* was detected as a potential "
- f"typo by the `{exc.check_name!r}` check."
- )
- existing_project_page = request.route_url(
- "packaging.project",
- name=exc.existing_project_name,
- _host=warehouse_domain,
- )
- existing_project_text = (
- f"<{existing_project_page}|Existing project: "
- f"{exc.existing_project_name}>"
- )
-
- webhook_payload = {
- "blocks": [
- {
- "type": "header",
- "text": {
- "type": "plain_text",
- "text": "TypoSnyper :warning:",
- "emoji": True,
- },
- },
- {
- "type": "section",
- "text": {
- "type": "mrkdwn",
- "text": new_project_text,
- },
- },
- {
- "type": "section",
- "text": {
- "type": "mrkdwn",
- "text": existing_project_text,
- },
- },
- {"type": "divider"},
- {
- "type": "context",
- "elements": [
- {
- "type": "plain_text",
- "text": "Once reviewed/confirmed, "
- "react to this message with :white_check_mark:",
- "emoji": True,
- }
- ],
- },
- ]
- }
- notification_service.send_notification(payload=webhook_payload)
-
- request.metrics.increment(
- "warehouse.packaging.services.create_project.typo_squatting",
- tags=[f"check_name:{exc.check_name!r}"],
- )
-
-
class DependencyKind(enum.IntEnum):
requires = 1
provides = 2
diff --git a/warehouse/packaging/services.py b/warehouse/packaging/services.py
index fe44870086e3..fb9fe7c3d6bb 100644
--- a/warehouse/packaging/services.py
+++ b/warehouse/packaging/services.py
@@ -486,7 +486,7 @@ def check_project_name(self, name: str) -> None:
return None
- def check_project_name_after_insert(self, name: str) -> None:
+ def check_project_name_after_commit(self, name: str) -> None:
# Check for typo-squatting.
if typo_check_match := typo_check_name(canonicalize_name(name)):
raise ProjectNameUnavailableTypoSquattingError(
diff --git a/warehouse/packaging/typosnyper.py b/warehouse/packaging/typosnyper.py
index 91ffd9988b07..d94ccf9eefb8 100644
--- a/warehouse/packaging/typosnyper.py
+++ b/warehouse/packaging/typosnyper.py
@@ -25,6 +25,16 @@
from itertools import permutations
+from pyramid.threadlocal import get_current_request
+
+from warehouse import db
+from warehouse.helpdesk.interfaces import IAdminNotificationService
+from warehouse.packaging.interfaces import (
+ IProjectService,
+ ProjectNameUnavailableTypoSquattingError,
+)
+from warehouse.packaging.models import Project
+
# Ensure all checks return a similar type,
# where the first string is the check name,
# followed by the matched project name,
@@ -431,3 +441,92 @@ def typo_check_name(project_name: str, corpus=None) -> TypoCheckMatch:
if result := check(project_name, corpus=corpus):
return result
return None
+
+
+@db.listens_for(db.Session, "after_commit")
+def check_typo_after_commit(config, session):
+ # Go through each new object and find any Project instances
+ for obj in session.new:
+ if obj.__class__ == Project:
+ request = get_current_request()
+ if not request:
+ # Can't do anything if there isn't a request
+ return
+ project_service = request.find_service(IProjectService)
+ name = obj.name
+ try:
+ project_service.check_project_name_after_commit(name)
+ except ProjectNameUnavailableTypoSquattingError as exc:
+ request.log.warning(
+ "ProjectNameUnavailableTypoSquattingError",
+ check_name=exc.check_name,
+ existing_project_name=exc.existing_project_name,
+ )
+ # Send notification to Admins for review
+ notification_service = request.find_service(IAdminNotificationService)
+
+ warehouse_domain = request.registry.settings.get("warehouse.domain")
+ new_project_page = request.route_url(
+ "packaging.project",
+ name=name,
+ _host=warehouse_domain,
+ )
+ new_project_text = (
+ f"During `file_upload`, Project Create for "
+ f"*<{new_project_page}|{name}>* was detected as a potential "
+ f"typo by the `{exc.check_name!r}` check."
+ )
+ existing_project_page = request.route_url(
+ "packaging.project",
+ name=exc.existing_project_name,
+ _host=warehouse_domain,
+ )
+ existing_project_text = (
+ f"<{existing_project_page}|Existing project: "
+ f"{exc.existing_project_name}>"
+ )
+
+ webhook_payload = {
+ "blocks": [
+ {
+ "type": "header",
+ "text": {
+ "type": "plain_text",
+ "text": "TypoSnyper :warning:",
+ "emoji": True,
+ },
+ },
+ {
+ "type": "section",
+ "text": {
+ "type": "mrkdwn",
+ "text": new_project_text,
+ },
+ },
+ {
+ "type": "section",
+ "text": {
+ "type": "mrkdwn",
+ "text": existing_project_text,
+ },
+ },
+ {"type": "divider"},
+ {
+ "type": "context",
+ "elements": [
+ {
+ "type": "plain_text",
+ "text": "Once reviewed/confirmed, "
+ "react to this message with :white_check_mark:",
+ "emoji": True,
+ }
+ ],
+ },
+ ]
+ }
+ notification_service.send_notification(payload=webhook_payload)
+
+ request.metrics.increment(
+ "warehouse.packaging.services.create_project.typo_squatting",
+ tags=[f"check_name:{exc.check_name!r}"],
+ )
From 84bbfb22eeae79f1ef003d0448140ae11d2e1907 Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Fri, 14 Mar 2025 15:22:52 +0000
Subject: [PATCH 4/6] WIP
---
tests/unit/packaging/test_services.py | 7 +++++++
tests/unit/packaging/test_typosnyper.py | 14 ++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/tests/unit/packaging/test_services.py b/tests/unit/packaging/test_services.py
index 29ac7c6f323d..8b21ef3fc07b 100644
--- a/tests/unit/packaging/test_services.py
+++ b/tests/unit/packaging/test_services.py
@@ -1059,6 +1059,13 @@ def test_check_project_name_typosquatting_prohibited(self, db_session):
with pytest.raises(ProjectNameUnavailableTypoSquattingError):
service.check_project_name_after_commit("numpi")
+ def test_check_project_name_typosquatting_no_typo(self, db_session):
+ # TODO: Update this test once we have a dynamic TopN approach
+ service = ProjectService(session=db_session)
+ ProhibitedProjectFactory.create(name="numpy")
+
+ assert service.check_project_name_after_commit("foobar") is None
+
def test_check_project_name_ok(self, db_session):
service = ProjectService(session=db_session)
diff --git a/tests/unit/packaging/test_typosnyper.py b/tests/unit/packaging/test_typosnyper.py
index 9ad021d15734..838fcd392bf2 100644
--- a/tests/unit/packaging/test_typosnyper.py
+++ b/tests/unit/packaging/test_typosnyper.py
@@ -10,14 +10,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import pretend
import pytest
+from warehouse.packaging import typosnyper
+from warehouse.packaging.models import Project
from warehouse.packaging.typosnyper import typo_check_name
@pytest.mark.parametrize(
("name", "expected"),
[
+ ("x", None), # Pass, shorter than 4 characters
("numpy", None), # Pass, no typos, exists
("NuMpy", None), # Pass, same as `numpy` after canonicalization
("nuumpy", ("repeated_characters", "numpy")),
@@ -43,3 +47,13 @@ def test_typo_check_name(name, expected):
}
assert typo_check_name(name, corpus=test_names_corpus) == expected
+
+
+def test_check_typo_after_commit(db_request, monkeypatch):
+ # db_request.config.find_service_factory = pretend.raiser(LookupError)
+
+ project = Project(name="foobar")
+ db_request.db.add(project)
+ db_request.db.commit()
+
+ assert typosnyper.check_typo_after_commit.calls == []
From 617d4361dc609fa77f6fff2b58257bc095dffa0b Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Fri, 14 Mar 2025 15:29:38 +0000
Subject: [PATCH 5/6] linting
---
tests/unit/packaging/test_typosnyper.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/unit/packaging/test_typosnyper.py b/tests/unit/packaging/test_typosnyper.py
index 838fcd392bf2..ced39f71ffbd 100644
--- a/tests/unit/packaging/test_typosnyper.py
+++ b/tests/unit/packaging/test_typosnyper.py
@@ -10,7 +10,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-import pretend
import pytest
from warehouse.packaging import typosnyper
From 5a0cca804e575de71d3d95e47805892021129a11 Mon Sep 17 00:00:00 2001
From: Dustin Ingram <di@users.noreply.github.com>
Date: Fri, 14 Mar 2025 17:05:16 +0000
Subject: [PATCH 6/6] Try adding a service fixture
---
tests/conftest.py | 14 +++++++++++++-
tests/unit/packaging/test_typosnyper.py | 4 +---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index b25d476147ef..b7510abb2c9a 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -52,6 +52,8 @@
from warehouse.email.interfaces import IEmailSender
from warehouse.helpdesk import services as helpdesk_services
from warehouse.helpdesk.interfaces import IAdminNotificationService, IHelpDeskService
+from warehouse.legacy.api.xmlrpc.cache import services as xmlrpc_services
+from warehouse.legacy.api.xmlrpc.cache.interfaces import IXMLRPCCache
from warehouse.macaroons import services as macaroon_services
from warehouse.macaroons.interfaces import IMacaroonService
from warehouse.metrics import IMetricsService
@@ -163,6 +165,7 @@ def pyramid_services(
macaroon_service,
helpdesk_service,
notification_service,
+ xmlrpccache_service,
):
services = _Services()
@@ -186,6 +189,7 @@ def pyramid_services(
services.register_service(macaroon_service, IMacaroonService, None, name="")
services.register_service(helpdesk_service, IHelpDeskService, None)
services.register_service(notification_service, IAdminNotificationService)
+ services.register_service(xmlrpccache_service, IXMLRPCCache)
return services
@@ -331,7 +335,7 @@ def get_app_config(database, nondefaults=None):
"sessions.secret": "123456",
"sessions.url": "redis://localhost:0/",
"statuspage.url": "https://2p66nmmycsj3.statuspage.io",
- "warehouse.xmlrpc.cache.url": "redis://localhost:0/",
+ "warehouse.xmlrpc.cache.url": "null://",
"terms.revision": "initial",
}
@@ -524,6 +528,14 @@ def notification_service():
return helpdesk_services.ConsoleAdminNotificationService()
+@pytest.fixture
+def xmlrpccache_service():
+ def purger(tags):
+ return None
+
+ return xmlrpc_services.NullXMLRPCCache("null://", purger)
+
+
class QueryRecorder:
def __init__(self):
self.queries = []
diff --git a/tests/unit/packaging/test_typosnyper.py b/tests/unit/packaging/test_typosnyper.py
index ced39f71ffbd..68d80869ee89 100644
--- a/tests/unit/packaging/test_typosnyper.py
+++ b/tests/unit/packaging/test_typosnyper.py
@@ -48,9 +48,7 @@ def test_typo_check_name(name, expected):
assert typo_check_name(name, corpus=test_names_corpus) == expected
-def test_check_typo_after_commit(db_request, monkeypatch):
- # db_request.config.find_service_factory = pretend.raiser(LookupError)
-
+def test_check_typo_after_commit(db_request):
project = Project(name="foobar")
db_request.db.add(project)
db_request.db.commit()