From 293b4762f6c8a6f5f264eaf7edd048896e41d2e7 Mon Sep 17 00:00:00 2001 From: Robin5605 Date: Sat, 5 Oct 2024 20:44:26 -0500 Subject: [PATCH 1/4] Remove email recipient configuration option --- compose.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/compose.yaml b/compose.yaml index e5b6e59e..5a583707 100644 --- a/compose.yaml +++ b/compose.yaml @@ -13,7 +13,6 @@ services: MICROSOFT_TENANT_ID: tenant_id MICROSOFT_CLIENT_ID: client_id MICROSOFT_CLIENT_SECRET: client_secret - EMAIL_RECIPIENT: email_recipient DRAGONFLY_GITHUB_TOKEN: test volumes: - "./src:/app/src" From 1245574d04ae92d038236a17f74b3b1be6b2ffda Mon Sep 17 00:00:00 2001 From: Robin5605 Date: Sat, 5 Oct 2024 20:44:59 -0500 Subject: [PATCH 2/4] Remove all email related features --- src/mainframe/endpoints/report.py | 60 ++++++++++------------------ src/mainframe/models/schemas.py | 10 ----- tests/test_report.py | 65 ++++++++----------------------- 3 files changed, 37 insertions(+), 98 deletions(-) diff --git a/src/mainframe/endpoints/report.py b/src/mainframe/endpoints/report.py index 22d5cbe2..f324aefc 100644 --- a/src/mainframe/endpoints/report.py +++ b/src/mainframe/endpoints/report.py @@ -14,7 +14,6 @@ from mainframe.json_web_token import AuthenticationData from mainframe.models.orm import Scan from mainframe.models.schemas import ( - EmailReport, Error, ObservationKind, ObservationReport, @@ -121,20 +120,17 @@ def _validate_additional_information(body: ReportPackageBody, scan: Scan): log = logger.bind(package={"name": body.name, "version": body.version}) if body.additional_information is None: - if len(scan.rules) == 0 or body.use_email is False: - if len(scan.rules) == 0: - detail = ( - f"additional_information is a required field as package " - f"`{body.name}@{body.version}` has no matched rules in the database" - ) - else: - detail = "additional_information is required when using Observation API" - - error = HTTPException(400, detail=detail) - log.error( - "Missing additional_information field", error_message=detail, tag="missing_additional_information" + if len(scan.rules) == 0: + detail = ( + f"additional_information is a required field as package " + f"`{body.name}@{body.version}` has no matched rules in the database" ) - raise error + else: + detail = "additional_information is required when using Observation API" + + error = HTTPException(400, detail=detail) + log.error("Missing additional_information field", error_message=detail, tag="missing_additional_information") + raise error def _validate_pypi(name: str, version: str, http_client: httpx.Client): @@ -163,9 +159,6 @@ def report_package( """ Report a package to PyPI. - The optional `use_email` field can be used to send reports by email. This - defaults to `False`. - There are some restrictions on what packages can be reported. They must: - exist in the database - exist on PyPI @@ -208,30 +201,18 @@ def report_package( rules_matched: list[str] = [rule.name for rule in scan.rules] - if body.use_email is True: - report = EmailReport( - name=body.name, - version=body.version, - rules_matched=rules_matched, - recipient=body.recipient, - inspector_url=inspector_url, - additional_information=body.additional_information, - ) + # We previously checked this condition, but the typechecker isn't smart + # enough to figure that out + assert body.additional_information is not None - httpx_client.post(f"{mainframe_settings.reporter_url}/report/email", json=jsonable_encoder(report)) - else: - # We previously checked this condition, but the typechecker isn't smart - # enough to figure that out - assert body.additional_information is not None - - report = ObservationReport( - kind=ObservationKind.Malware, - summary=body.additional_information, - inspector_url=inspector_url, - extra=dict(yara_rules=rules_matched), - ) + report = ObservationReport( + kind=ObservationKind.Malware, + summary=body.additional_information, + inspector_url=inspector_url, + extra=dict(yara_rules=rules_matched), + ) - httpx_client.post(f"{mainframe_settings.reporter_url}/report/{name}", json=jsonable_encoder(report)) + httpx_client.post(f"{mainframe_settings.reporter_url}/report/{name}", json=jsonable_encoder(report)) with session.begin(): scan.reported_by = auth.subject @@ -247,7 +228,6 @@ def report_package( "inspector_url": inspector_url, "additional_information": body.additional_information, "rules_matched": rules_matched, - "use_email": body.use_email, }, reported_by=auth.subject, ) diff --git a/src/mainframe/models/schemas.py b/src/mainframe/models/schemas.py index cad4d420..7804ab2c 100644 --- a/src/mainframe/models/schemas.py +++ b/src/mainframe/models/schemas.py @@ -95,16 +95,6 @@ class ReportPackageBody(PackageSpecifier): recipient: Optional[str] inspector_url: Optional[str] additional_information: Optional[str] - use_email: bool = False - - -class EmailReport(PackageSpecifier): - """Model for a report using email""" - - rules_matched: list[str] - recipient: Optional[str] = None - inspector_url: Optional[str] - additional_information: Optional[str] # Taken from diff --git a/tests/test_report.py b/tests/test_report.py index fde95f05..c1e63d0b 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -26,60 +26,31 @@ from mainframe.json_web_token import AuthenticationData from mainframe.models.orm import DownloadURL, Rule, Scan, Status from mainframe.models.schemas import ( - EmailReport, ObservationKind, ObservationReport, ReportPackageBody, ) -@pytest.mark.parametrize( - "body,url,expected", - [ - ( - ReportPackageBody( - name="c", - version="1.0.0", - recipient=None, - inspector_url=None, - additional_information="this package is bad", - use_email=True, - ), - "/report/email", - EmailReport( - name="c", - version="1.0.0", - rules_matched=["rule 1", "rule 2"], - inspector_url="test inspector url", - additional_information="this package is bad", - ), - ), - ( - ReportPackageBody( - name="c", - version="1.0.0", - recipient=None, - inspector_url=None, - additional_information="this package is bad", - ), - "/report/c", - ObservationReport( - kind=ObservationKind.Malware, - summary="this package is bad", - inspector_url="test inspector url", - extra=dict(yara_rules=["rule 1", "rule 2"]), - ), - ), - ], -) def test_report( sm: sessionmaker[Session], db_session: Session, auth: AuthenticationData, - body: ReportPackageBody, - url: str, - expected: EmailReport | ObservationReport, ): + body = ReportPackageBody( + name="c", + version="1.0.0", + recipient=None, + inspector_url=None, + additional_information="this package is bad", + ) + + report = ObservationReport( + kind=ObservationKind.Malware, + summary="this package is bad", + inspector_url="test inspector url", + extra=dict(yara_rules=["rule 1", "rule 2"]), + ) scan = Scan( name="c", version="1.0.0", @@ -107,7 +78,7 @@ def test_report( report_package(body, sm(), auth, mock_httpx_client) - mock_httpx_client.post.assert_called_once_with(url, json=jsonable_encoder(expected)) + mock_httpx_client.post.assert_called_once_with("/report/c", json=jsonable_encoder(report)) with sm() as sess, sess.begin(): s = sess.scalar(select(Scan).where(Scan.name == "c").where(Scan.version == "1.0.0")) @@ -180,14 +151,13 @@ def test_report_inspector_url(body_url: Optional[str], scan_url: Optional[str]): @pytest.mark.parametrize( ("body", "scan"), [ - ( # No additional information, and no rules with email + ( ReportPackageBody( name="c", version="1.0.0", recipient=None, inspector_url="inspector url override", additional_information=None, - use_email=True, ), Scan( name="c", @@ -209,14 +179,13 @@ def test_report_inspector_url(body_url: Optional[str], scan_url: Optional[str]): commit_hash="test commit hash", ), ), - ( # No additional information with Observations + ( ReportPackageBody( name="c", version="1.0.0", recipient=None, inspector_url="inspector url override", additional_information=None, - use_email=False, ), Scan( name="c", From a0496bfa88e0a95eed1e3daad968257d3604bd13 Mon Sep 17 00:00:00 2001 From: Robin5605 Date: Sun, 13 Oct 2024 02:36:57 -0500 Subject: [PATCH 3/4] Remove recipient field from report schema --- src/mainframe/models/schemas.py | 1 - tests/test_report.py | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/mainframe/models/schemas.py b/src/mainframe/models/schemas.py index 7804ab2c..45d2ee68 100644 --- a/src/mainframe/models/schemas.py +++ b/src/mainframe/models/schemas.py @@ -92,7 +92,6 @@ class PackageSpecifier(BaseModel): class ReportPackageBody(PackageSpecifier): - recipient: Optional[str] inspector_url: Optional[str] additional_information: Optional[str] diff --git a/tests/test_report.py b/tests/test_report.py index c1e63d0b..daa964fc 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -40,7 +40,6 @@ def test_report( body = ReportPackageBody( name="c", version="1.0.0", - recipient=None, inspector_url=None, additional_information="this package is bad", ) @@ -155,7 +154,6 @@ def test_report_inspector_url(body_url: Optional[str], scan_url: Optional[str]): ReportPackageBody( name="c", version="1.0.0", - recipient=None, inspector_url="inspector url override", additional_information=None, ), @@ -183,7 +181,6 @@ def test_report_inspector_url(body_url: Optional[str], scan_url: Optional[str]): ReportPackageBody( name="c", version="1.0.0", - recipient=None, inspector_url="inspector url override", additional_information=None, ), From c6d9417f245cefe8b77982d5f177bc89097ac7f6 Mon Sep 17 00:00:00 2001 From: Robin5605 Date: Wed, 30 Oct 2024 16:00:39 -0500 Subject: [PATCH 4/4] Make additional_information required Additional information is required when using the Observation API. The tests that used to validate the presence for additional information when we used email have been removed. Docstrings have also been updated. --- src/mainframe/endpoints/report.py | 56 +++---------------------- src/mainframe/models/schemas.py | 2 +- tests/test_report.py | 68 ------------------------------- 3 files changed, 6 insertions(+), 120 deletions(-) diff --git a/src/mainframe/endpoints/report.py b/src/mainframe/endpoints/report.py index 969039df..1a0e2e53 100644 --- a/src/mainframe/endpoints/report.py +++ b/src/mainframe/endpoints/report.py @@ -108,33 +108,6 @@ def _validate_inspector_url(name: str, version: str, body_url: Optional[str], sc return inspector_url -def _validate_additional_information(body: ReportPackageBody, scan: Scan): - """ - Validates the additional_information field. - - Returns: - None if `body.additional_information` is valid. - - Raises: - HTTPException: 400 Bad Request if `additional_information` was required - and was not passed - """ - log = logger.bind(package={"name": body.name, "version": body.version}) - - if body.additional_information is None: - if len(scan.rules) == 0: - detail = ( - f"additional_information is a required field as package " - f"`{body.name}@{body.version}` has no matched rules in the database" - ) - else: - detail = "additional_information is required when using Observation API" - - error = HTTPException(400, detail=detail) - log.error("Missing additional_information field", error_message=detail, tag="missing_additional_information") - raise error - - def _validate_pypi(name: str, version: str, http_client: httpx.Client): log = logger.bind(package={"name": name, "version": version}) @@ -166,25 +139,11 @@ def report_package( - exist on PyPI - not already be reported - While the `inspector_url` and `additional_information` fields are optional - in the schema, the API requires you to provide them in certain cases. Some - of those are outlined below. - - `inspector_url` and `additional_information` both must be provided if the - package being reported is in a `QUEUED` or `PENDING` state. That is, the - package has not yet been scanned and therefore has no records for - `inspector_url` or any matched rules - - If the package has successfully been scanned (that is, it is in - a `FINISHED` state), and it has been determined to be malicious, then - neither `inspector_url` nor `additional_information` is required. If the - `inspector_url` is omitted, then it will default to a URL that points to - the file with the highest total score. - - If the package has successfully been scanned (that is, it is in - a `FINISHED` state), and it has been determined NOT to be malicious (that - is, it has no matched rules), then you must provide `inspector_url` AND - `additional_information`. + `inspector_url` argument is required if the package has no matched rules. + If `inspector_url` argument is not provided for a package with matched rules, + the Inspector URL of the file with the highest total score will be used. + If `inspector_url` argument is provided for a package with matched rules, + the given Inspector URL will override the default one. """ name = body.name @@ -195,7 +154,6 @@ def report_package( # Check our database first to avoid unnecessarily using PyPI API. scan = _lookup_package(name, version, session) inspector_url = _validate_inspector_url(name, version, body.inspector_url, scan.inspector_url) - _validate_additional_information(body, scan) # If execution reaches here, we must have found a matching scan in our # database. Check if the package we want to report exists on PyPI. @@ -203,10 +161,6 @@ def report_package( rules_matched: list[str] = [rule.name for rule in scan.rules] - # We previously checked this condition, but the typechecker isn't smart - # enough to figure that out - assert body.additional_information is not None - report = ObservationReport( kind=ObservationKind.Malware, summary=body.additional_information, diff --git a/src/mainframe/models/schemas.py b/src/mainframe/models/schemas.py index 45d2ee68..d09e06f4 100644 --- a/src/mainframe/models/schemas.py +++ b/src/mainframe/models/schemas.py @@ -93,7 +93,7 @@ class PackageSpecifier(BaseModel): class ReportPackageBody(PackageSpecifier): inspector_url: Optional[str] - additional_information: Optional[str] + additional_information: str # Taken from diff --git a/tests/test_report.py b/tests/test_report.py index daa964fc..c163ec88 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -13,9 +13,6 @@ from mainframe.endpoints.report import ( _lookup_package, # pyright: ignore [reportPrivateUsage] ) -from mainframe.endpoints.report import ( - _validate_additional_information, # pyright: ignore [reportPrivateUsage] -) from mainframe.endpoints.report import ( _validate_inspector_url, # pyright: ignore [reportPrivateUsage] ) @@ -147,71 +144,6 @@ def test_report_inspector_url(body_url: Optional[str], scan_url: Optional[str]): assert "test url" == _validate_inspector_url("a", "1.0.0", body_url, scan_url) -@pytest.mark.parametrize( - ("body", "scan"), - [ - ( - ReportPackageBody( - name="c", - version="1.0.0", - inspector_url="inspector url override", - additional_information=None, - ), - Scan( - name="c", - version="1.0.0", - status=Status.FINISHED, - score=0, - inspector_url=None, - rules=[], - download_urls=[], - queued_at=datetime.now() - timedelta(seconds=60), - queued_by="remmy", - pending_at=datetime.now() - timedelta(seconds=30), - pending_by="remmy", - finished_at=datetime.now() - timedelta(seconds=10), - finished_by="remmy", - reported_at=None, - reported_by=None, - fail_reason=None, - commit_hash="test commit hash", - ), - ), - ( - ReportPackageBody( - name="c", - version="1.0.0", - inspector_url="inspector url override", - additional_information=None, - ), - Scan( - name="c", - version="1.0.0", - status=Status.FINISHED, - score=0, - inspector_url=None, - rules=[Rule(name="ayo")], - download_urls=[], - queued_at=datetime.now() - timedelta(seconds=60), - queued_by="remmy", - pending_at=datetime.now() - timedelta(seconds=30), - pending_by="remmy", - finished_at=datetime.now() - timedelta(seconds=10), - finished_by="remmy", - reported_at=None, - reported_by=None, - fail_reason=None, - commit_hash="test commit hash", - ), - ), - ], -) -def test_report_missing_additional_information(body: ReportPackageBody, scan: Scan): - with pytest.raises(HTTPException) as e: - _validate_additional_information(body, scan) - assert e.value.status_code == 400 - - @pytest.mark.parametrize( ("scans", "name", "version", "expected_status_code"), [