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 16, 2024 · fcf98cd · fcf98cd
1 parent dfb014d
commit fcf98cd
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 4 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,6 +42,9 @@ 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
+internal val BOOLEAN_VALUES = listOf("true", "false")
+
 /**
  * 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/)
@@ -51,6 +54,8 @@ internal val PYTHON_VERSIONS = listOf("2.7", "3.6", "3.7", "3.8", "3.9", "3.10",
  * - *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".
+ * - *analyzeSetupPyInsecurely*: If "true", `python-inspector` resolves dependencies from setup.py files by executing
+ *   them. This is a potential security risk. Defaults to "true".
  */
 class Pip(
     name: String,
@@ -61,6 +66,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 +93,14 @@ class Pip(
         }
     }
 
+    private val analyzeSetupPyInsecurelyOption = options[OPTION_ANALYZE_SETUP_PY_INSECURELY]?.also { analyzeSetupPy ->
+        require(analyzeSetupPy in BOOLEAN_VALUES) {
+            val acceptedValues = BOOLEAN_VALUES.joinToString { "'$it'" }
+            "The '$OPTION_ANALYZE_SETUP_PY_INSECURELY' option must be one of $acceptedValues, but was " +
+                "'$analyzeSetupPy'."
+        }
+    }
+
     override fun resolveDependencies(definitionFile: File, labels: Map<String, String>): List<ProjectAnalyzerResult> {
         val result = runPythonInspector(definitionFile) { detectPythonVersion(definitionFile.parentFile) }
 
@@ -102,6 +116,8 @@ class Pip(
     ): PythonInspector.Result {
         val operatingSystem = operatingSystemOption ?: OPTION_OPERATING_SYSTEM_DEFAULT
         val pythonVersion = pythonVersionOption ?: detectPythonVersion() ?: OPTION_PYTHON_VERSION_DEFAULT
+        val analyzeSetupPyInsecurely =
+            analyzeSetupPyInsecurelyOption?.toBoolean() ?: OPTION_ANALYZE_SETUP_PY_INSECURELY_DEFAULT
 
         val workingDir = definitionFile.parentFile
 
@@ -116,7 +132,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")