From fd24c3caa0aad0208417736481d1d3fea2eae894 Mon Sep 17 00:00:00 2001 From: Haiko Schol Date: Tue, 16 Jul 2024 17:49:38 +0200 Subject: [PATCH] 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 --- .../requirements.txt | 1 + .../kotlin/utils/PythonInspectorFunTest.kt | 24 ++++++++++++++++++- .../python/src/main/kotlin/Pip.kt | 17 +++++++++++-- .../src/main/kotlin/utils/PythonInspector.kt | 12 ++++++++-- 4 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 plugins/package-managers/python/src/funTest/assets/projects/synthetic/python-inspector-no-analyze-setup-py/requirements.txt diff --git a/plugins/package-managers/python/src/funTest/assets/projects/synthetic/python-inspector-no-analyze-setup-py/requirements.txt b/plugins/package-managers/python/src/funTest/assets/projects/synthetic/python-inspector-no-analyze-setup-py/requirements.txt new file mode 100644 index 0000000000000..0cac49fa27b71 --- /dev/null +++ b/plugins/package-managers/python/src/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 index 8a94074c839ae..dc97674caecc7 100644 --- 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 index 5ccfdd47849c7..50328b26ce9dd 100644 --- 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") { @@ -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): List { 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 index 8df2a3bc985ce..10a0524b21183 100644 --- 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")