From fcf98cdfc2803c50e4500304b5622f6ed3d491f7 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 | 19 ++++++++++++++- .../src/main/kotlin/utils/PythonInspector.kt | 12 ++++++++-- 4 files changed, 52 insertions(+), 4 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..4802f33a658ad 100644 --- 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") { @@ -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): List { 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 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")