From e6ea4fd75fcb659a61288c7603403c0e077f1e53 Mon Sep 17 00:00:00 2001 From: Steven Troxler Date: Thu, 16 May 2024 05:39:23 -0700 Subject: [PATCH 1/4] Fix lint errors in github CI Summary: Unfortunately we still don't get flake8 errors on unused imports internally, so I introduced a few of these when I was fixing broken unit tests in github CI. This should make the lint signal clean, so that on github only the `pyre` and `pysa` signals will be red (they have been broken for a while, I'll try to fix them but that will be in separate diffs). Reviewed By: alexkassil Differential Revision: D57386423 --- tools/upgrade/commands/tests/fixme_all_test.py | 4 +--- tools/upgrade/commands/tests/fixme_single_test.py | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/upgrade/commands/tests/fixme_all_test.py b/tools/upgrade/commands/tests/fixme_all_test.py index e518599c8b5..a245937f76c 100644 --- a/tools/upgrade/commands/tests/fixme_all_test.py +++ b/tools/upgrade/commands/tests/fixme_all_test.py @@ -9,12 +9,10 @@ import subprocess import unittest from pathlib import Path -from unittest.mock import call, MagicMock, mock_open, patch +from unittest.mock import MagicMock, mock_open, patch from ... import upgrade -from ...errors import Errors from ...repository import Repository -from .. import command from ..command import ErrorSource, ErrorSuppressingCommand from ..fixme_all import Configuration, FixmeAll diff --git a/tools/upgrade/commands/tests/fixme_single_test.py b/tools/upgrade/commands/tests/fixme_single_test.py index 491e80e138c..3b3b53ce3ee 100644 --- a/tools/upgrade/commands/tests/fixme_single_test.py +++ b/tools/upgrade/commands/tests/fixme_single_test.py @@ -9,7 +9,6 @@ from pathlib import Path from unittest.mock import MagicMock, mock_open, patch -from ... import upgrade from ...errors import Errors from ...repository import Repository from ..command import ErrorSuppressingCommand From 58b264e481f9e1f5c544cfa5523726783301a675 Mon Sep 17 00:00:00 2001 From: Steven Troxler Date: Thu, 16 May 2024 05:39:23 -0700 Subject: [PATCH 2/4] Handle package changes in github CI Summary: The dataclasses-json version we are getting in github has types that are incompatible with the internal version. As a result, we need to pin the version on github to get comparable results (in addition it's at least possible we would get bugs in open-source without pinning). I filed T189226004 to bump the version internally. In addition: - we aren't marking `toml` as a dependency, but we need it as a dev dependency because type checking the typeshed patcher requires it - we need to list both tabulate and toml in our .pyre_configuration for github to pick them up because we are still using the legacy setup where packages aren't picked up by default... this would probably be good to change at some point. Reviewed By: arthaud Differential Revision: D57387918 --- .github/workflows/pyre.yml | 3 ++- .pyre_configuration | 6 ++++++ requirements-dev.txt | 1 + requirements.txt | 2 +- 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pyre.yml b/.github/workflows/pyre.yml index e51769f484e..206f771a3ea 100644 --- a/.github/workflows/pyre.yml +++ b/.github/workflows/pyre.yml @@ -22,6 +22,7 @@ jobs: run: | pip install --upgrade pip pip install -r requirements.txt + pip install -r requirements-dev.txt pip install cython flask flask_cors graphql-core typing_inspect VERSION=$(grep "version" .pyre_configuration | sed -n -e 's/.*\(0\.0\.[0-9]*\).*/\1/p') pip install pyre-check-nightly==$VERSION @@ -29,7 +30,7 @@ jobs: - name: Run Pyre continue-on-error: true run: | - pyre --output=sarif check > sarif.json + pyre -n --output=sarif check > sarif.json - name: Expose SARIF Results uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3 diff --git a/.pyre_configuration b/.pyre_configuration index ae648888760..ef63cab75a8 100644 --- a/.pyre_configuration +++ b/.pyre_configuration @@ -41,6 +41,12 @@ { "site-package": "testslide" }, + { + "site-package": "toml" + }, + { + "site-package": "tabulate" + }, { "is_toplevel_module": true, "site-package": "typing_extensions" diff --git a/requirements-dev.txt b/requirements-dev.txt index 416634f5288..217be79f85f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1 +1,2 @@ pre-commit +toml diff --git a/requirements.txt b/requirements.txt index 26027d060d7..4dafefd195f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ click>=8.0 -dataclasses-json +dataclasses-json==0.5.7 intervaltree libcst psutil From 11d1246028a63fa4dcba049640bf31aa927ff1ea Mon Sep 17 00:00:00 2001 From: Steven Troxler Date: Thu, 16 May 2024 05:39:23 -0700 Subject: [PATCH 3/4] Remove dead `pyre-ignore` directives (#855) Summary: The dataclasses-json version we are getting in github has types that are incompatible with the internal version. As a result, we need to pin the version on github to get comparable results (in addition it's at least possible we would get bugs in open-source without pinning). I filed T189226004 to bump the version internally. In addition: - we aren't marking `toml` as a dependency, but we need it as a dev dependency because type checking the typeshed patcher requires it - we need to list both tabulate and toml in our .pyre_configuration for github to pick them up because we are still using the legacy setup where packages aren't picked up by default... this would probably be good to change at some point. Differential Revision: D57433947 --- tools/typeshed_patcher/typeshed.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/typeshed_patcher/typeshed.py b/tools/typeshed_patcher/typeshed.py index 0b5afff59cd..857eb6d2690 100644 --- a/tools/typeshed_patcher/typeshed.py +++ b/tools/typeshed_patcher/typeshed.py @@ -57,8 +57,6 @@ class Typeshed(abc.ABC): Representation of a collection of Python stub files. """ - # pyre-fixme[56]: Pyre doesn't yet support decorators with ParamSpec applied to - # generic functions Please add # pyre-ignore[56] to `abc.abstractclassmethod`. @abc.abstractclassmethod def get_file_content(self, path: pathlib.Path) -> Optional[str]: """ @@ -69,8 +67,6 @@ def get_file_content(self, path: pathlib.Path) -> Optional[str]: """ raise NotImplementedError() - # pyre-fixme[56]: Pyre doesn't yet support decorators with ParamSpec applied to - # generic functions Please add # pyre-ignore[56] to `abc.abstractclassmethod`. @abc.abstractclassmethod def all_files(self) -> Iterable[pathlib.Path]: """ From 8a1c3c1e99fe20f79b5ef1345eb25e2c57aa47a4 Mon Sep 17 00:00:00 2001 From: Steven Troxler Date: Thu, 16 May 2024 05:39:23 -0700 Subject: [PATCH 4/4] Update full_result.json Summary: This updates to what we are currently seeing in the integration tests. There is a regression here: we don't seem to be resolving werkzeug.utils.redirect properly; Maxime was able to verify that reveal_type(redirect) comes back as `unknown`. It's likely that this is either Pyre not finding the package due to some configuration handling change, or a signature extraction issue. I filed T189346515 to investigate, but it seems wise to make CI pass in the meantime so that we'll notice the next regression quicker. Differential Revision: D57437213 --- .../full_result.json | 187 ------------------ 1 file changed, 187 deletions(-) diff --git a/documentation/deliberately_vulnerable_flask_app/full_result.json b/documentation/deliberately_vulnerable_flask_app/full_result.json index ced0fde85b5..1a55f874be2 100644 --- a/documentation/deliberately_vulnerable_flask_app/full_result.json +++ b/documentation/deliberately_vulnerable_flask_app/full_result.json @@ -43,17 +43,6 @@ "description": "Commandline arguments injection may result in RCE [6065]: Data from [UserControlled] source(s) may reach [ExecArgSink] sink(s)", "define": "app.definite_rce" }, - { - "line": 52, - "column": 53, - "stop_line": 52, - "stop_column": 60, - "path": "app.py", - "code": 5045, - "name": "SQL injection.", - "description": "SQL injection. [5045]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeSQL[main]]] sink(s)", - "define": "app.definite_sql" - }, { "line": 52, "column": 16, @@ -76,17 +65,6 @@ "description": "Potential Server-side request forgery (SSRF) [5012]: Data from [UserControlled] source(s) may reach [HTTPClientRequest_URI] sink(s)", "define": "app.definite_ssrf" }, - { - "line": 45, - "column": 27, - "stop_line": 45, - "stop_column": 34, - "path": "app.py", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "app.definite_xss" - }, { "line": 45, "column": 4, @@ -98,28 +76,6 @@ "description": "User input returned to user [5015]: Data from [UserControlled] source(s) may reach [ReturnedToUser] sink(s)", "define": "app.definite_xss" }, - { - "line": 81, - "column": 13, - "stop_line": 81, - "stop_column": 20, - "path": "app.py", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "app.open_redirect_tp" - }, - { - "line": 81, - "column": 13, - "stop_line": 81, - "stop_column": 20, - "path": "app.py", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "app.open_redirect_tp" - }, { "line": 28, "column": 19, @@ -142,17 +98,6 @@ "description": "Commandline arguments injection may result in RCE [6065]: Data from [UserControlled] source(s) may reach [ExecArgSink] sink(s)", "define": "app.potential_rce_2" }, - { - "line": 100, - "column": 28, - "stop_line": 100, - "stop_column": 35, - "path": "app.py", - "code": 6462, - "name": "User-controlled data flows into URL-like string (potential SSRF)", - "description": "User-controlled data flows into URL-like string (potential SSRF) [6462]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeURL[main]]] sink(s)", - "define": "app.user_controlled_data_flows_into_url_like_string_tp" - }, { "line": 90, "column": 38, @@ -207,137 +152,5 @@ "name": "Environment variable or import injection may result in RCE", "description": "Environment variable or import injection may result in RCE [6064]: Data from [UserControlled] source(s) may reach [ExecImportSink] sink(s)", "define": "app.user_data_to_getattr_tp" - }, - { - "line": 497, - "column": 35, - "stop_line": 497, - "stop_column": 39, - "path": "*", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "flask.app.Flask.__init__" - }, - { - "line": 497, - "column": 35, - "stop_line": 497, - "stop_column": 39, - "path": "*", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "flask.app.Flask.__init__" - }, - { - "line": 1487, - "column": 37, - "stop_line": 1487, - "stop_column": 39, - "path": "*", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "flask.app.Flask.full_dispatch_request" - }, - { - "line": 1487, - "column": 37, - "stop_line": 1487, - "stop_column": 39, - "path": "*", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "flask.app.Flask.full_dispatch_request" - }, - { - "line": 1487, - "column": 37, - "stop_line": 1487, - "stop_column": 39, - "path": "*", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "flask.app.Flask.full_dispatch_request" - }, - { - "line": 1487, - "column": 37, - "stop_line": 1487, - "stop_column": 39, - "path": "*", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "flask.app.Flask.full_dispatch_request" - }, - { - "line": 2005, - "column": 48, - "stop_line": 2005, - "stop_column": 52, - "path": "*", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "flask.app.Flask.process_response" - }, - { - "line": 2005, - "column": 48, - "stop_line": 2005, - "stop_column": 52, - "path": "*", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "flask.app.Flask.process_response" - }, - { - "line": 2189, - "column": 16, - "stop_line": 2189, - "stop_column": 19, - "path": "*", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "flask.app.Flask.wsgi_app" - }, - { - "line": 2206, - "column": 12, - "stop_line": 2206, - "stop_column": 15, - "path": "*", - "code": 5046, - "name": "XSS", - "description": "XSS [5046]: Data from [UserControlled] source(s) may reach [TriggeredPartialSink[UserControlledAndStringMayBeHTML[main]]] sink(s)", - "define": "flask.app.Flask.wsgi_app" - }, - { - "line": 2189, - "column": 16, - "stop_line": 2189, - "stop_column": 19, - "path": "*", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "flask.app.Flask.wsgi_app" - }, - { - "line": 2206, - "column": 12, - "stop_line": 2206, - "stop_column": 15, - "path": "*", - "code": 5018, - "name": "Open redirect", - "description": "Open redirect [5018]: Data from [UserControlled] source(s) may be used in an open redirect via [Redirect] sink(s)", - "define": "flask.app.Flask.wsgi_app" } ]