feat(analyzer): Add option to skip setup.py analysis of PIP dependencies

Previously, the PIP package manager has always set the "--analyze-setup-py-insecurely" flag on invocations of python-inspector. This is a potential security risk, as python-inspector will execute setup.py files of resolved dependencies. It also causes analysis to fail if those setup.py files import packages that are not part of the Python standard library (such as numexpr in version 2.10.1). This change makes it configurable whether the flag is set or not. Signed-off-by: Haiko Schol <[email protected]>
oss-review-toolkit · Jul 17, 2024 · 2d3847e · 2d3847e
1 parent b0b1f7c
commit 2d3847e
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 5 deletions.
diff --git a/...c/funTest/assets/projects/synthetic/python-inspector-no-analyze-setup-py/requirements.txt b/...c/funTest/assets/projects/synthetic/python-inspector-no-analyze-setup-py/requirements.txt
@@ -0,0 +1 @@
+numexpr==2.10.1
diff --git a/plugins/package-managers/python/src/funTest/kotlin/utils/PythonInspectorFunTest.kt b/plugins/package-managers/python/src/funTest/kotlin/utils/PythonInspectorFunTest.kt
@@ -38,7 +38,8 @@ class PythonInspectorFunTest : StringSpec({
                 workingDir = workingDir,
                 definitionFile = definitionFile,
                 pythonVersion = "27",
-                operatingSystem = "linux"
+                operatingSystem = "linux",
+                analyzeSetupPyInsecurely = true
             )
         } finally {
             workingDir.resolve(".cache").safeDeleteRecursively(force = true)
@@ -48,4 +49,25 @@ class PythonInspectorFunTest : StringSpec({
         result.resolvedDependenciesGraph should haveSize(1)
         result.packages should haveSize(11)
     }
+
+    "python-inspector can be run without setup.py file analysis" {
+        val definitionFile = projectsDir.resolve("synthetic/python-inspector-no-analyze-setup-py/requirements.txt")
+        val workingDir = definitionFile.parentFile
+
+        val result = try {
+            PythonInspector.inspect(
+                workingDir = workingDir,
+                definitionFile = definitionFile,
+                pythonVersion = "311",
+                operatingSystem = "linux",
+                analyzeSetupPyInsecurely = false
+            )
+        } finally {
+            workingDir.resolve(".cache").safeDeleteRecursively(force = true)
+        }
+
+        result.projects should haveSize(1)
+        result.resolvedDependenciesGraph should haveSize(1)
+        result.packages should haveSize(3)
+    }
 })
diff --git a/plugins/package-managers/python/src/main/kotlin/Pip.kt b/plugins/package-managers/python/src/main/kotlin/Pip.kt
@@ -42,12 +42,16 @@ private val OPERATING_SYSTEMS = listOf(OPTION_OPERATING_SYSTEM_DEFAULT, "macos",
 private const val OPTION_PYTHON_VERSION_DEFAULT = "3.11"
 internal val PYTHON_VERSIONS = listOf("2.7", "3.6", "3.7", "3.8", "3.9", "3.10", OPTION_PYTHON_VERSION_DEFAULT)
 
+private const val OPTION_ANALYZE_SETUP_PY_INSECURELY_DEFAULT = true
+
 /**
  * The [PIP](https://pip.pypa.io/) package manager for Python. Also see
  * [install_requires vs requirements files](https://packaging.python.org/discussions/install-requires-vs-requirements/)
  * and [setup.py vs. requirements.txt](https://caremad.io/posts/2013/07/setup-vs-requirement/).
  *
  * This package manager supports the following [options][PackageManagerConfiguration.options]:
+ * - *analyzeSetupPyInsecurely*: If "true", `python-inspector` resolves dependencies from setup.py files by executing
+ *   them. This is a potential security risk. Defaults to "true".
  * - *operatingSystem*: The name of the operating system to resolve dependencies for. One of "linux", "macos", or
  *   "windows". Defaults to "linux".
  * - *pythonVersion*: The Python version to resolve dependencies for. Defaults to "3.10".
@@ -61,6 +65,7 @@ class Pip(
     companion object {
         const val OPTION_OPERATING_SYSTEM = "operatingSystem"
         const val OPTION_PYTHON_VERSION = "pythonVersion"
+        const val OPTION_ANALYZE_SETUP_PY_INSECURELY = "analyzeSetupPyInsecurely"
     }
 
     class Factory : AbstractPackageManagerFactory<Pip>("PIP") {
@@ -87,6 +92,12 @@ class Pip(
         }
     }
 
+    private val analyzeSetupPyInsecurelyOption = options[OPTION_ANALYZE_SETUP_PY_INSECURELY]?.also { analyzeSetupPy ->
+        requireNotNull(analyzeSetupPy.toBooleanStrictOrNull()) {
+            "The '$OPTION_ANALYZE_SETUP_PY_INSECURELY' option must be 'true' or 'false', but was '$analyzeSetupPy'."
+        }
+    }
+
     override fun resolveDependencies(definitionFile: File, labels: Map<String, String>): List<ProjectAnalyzerResult> {
         val result = runPythonInspector(definitionFile) { detectPythonVersion(definitionFile.parentFile) }
 
@@ -102,7 +113,8 @@ class Pip(
     ): PythonInspector.Result {
         val operatingSystem = operatingSystemOption ?: OPTION_OPERATING_SYSTEM_DEFAULT
         val pythonVersion = pythonVersionOption ?: detectPythonVersion() ?: OPTION_PYTHON_VERSION_DEFAULT
-
+        val analyzeSetupPyInsecurely = analyzeSetupPyInsecurelyOption?.toBooleanStrict()
+            ?: OPTION_ANALYZE_SETUP_PY_INSECURELY_DEFAULT
         val workingDir = definitionFile.parentFile
 
         logger.info {
@@ -116,7 +128,8 @@ class Pip(
                     workingDir = workingDir,
                     definitionFile = definitionFile,
                     pythonVersion = pythonVersion.split('.', limit = 3).take(2).joinToString(""),
-                    operatingSystem = operatingSystem
+                    operatingSystem = operatingSystem,
+                    analyzeSetupPyInsecurely = analyzeSetupPyInsecurely
                 )
             } finally {
                 workingDir.resolve(".cache").safeDeleteRecursively(force = true)

diff --git a/plugins/package-managers/python/src/main/kotlin/utils/PythonInspector.kt b/plugins/package-managers/python/src/main/kotlin/utils/PythonInspector.kt
@@ -46,7 +46,13 @@ internal object PythonInspector : CommandLineTool {
 
     override fun getVersionRequirement(): RangesList = RangesListFactory.create("[0.9.2,)")
 
-    fun inspect(workingDir: File, definitionFile: File, pythonVersion: String, operatingSystem: String): Result {
+    fun inspect(
+        workingDir: File,
+        definitionFile: File,
+        pythonVersion: String,
+        operatingSystem: String,
+        analyzeSetupPyInsecurely: Boolean
+    ): Result {
         val outputFile = createOrtTempFile(prefix = "python-inspector", suffix = ".json")
 
         val commandLineOptions = buildList {
@@ -59,7 +65,9 @@ internal object PythonInspector : CommandLineTool {
             add("--json-pdt")
             add(outputFile.absolutePath)
 
-            add("--analyze-setup-py-insecurely")
+            if (analyzeSetupPyInsecurely) {
+                add("--analyze-setup-py-insecurely")
+            }
 
             if (definitionFile.name == "setup.py") {
                 add("--setup-py")