Skip to content

Commit

Permalink
feat(analyzer): Add option to skip setup.py analysis of PIP dependencies
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
haikoschol committed Jul 17, 2024
1 parent dfb014d commit fd24c3c
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
numexpr==2.10.1
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
})
17 changes: 15 additions & 2 deletions plugins/package-managers/python/src/main/kotlin/Pip.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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") {
Expand All @@ -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) }

Expand All @@ -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 {
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")

Check warning on line 69 in plugins/package-managers/python/src/main/kotlin/utils/PythonInspector.kt

View check run for this annotation

Codecov / codecov/patch

plugins/package-managers/python/src/main/kotlin/utils/PythonInspector.kt#L69

Added line #L69 was not covered by tests
}

if (definitionFile.name == "setup.py") {
add("--setup-py")
Expand Down

0 comments on commit fd24c3c

Please sign in to comment.