🐛 Handle editable pip installs (#2731)

* fix: Handle editable pip install Editable pip installs (-e) should be considered secure if the package is installed from a local source or a remote source (VCS install) but pinned by commit hash. To keep the behaviour we have for normal pip installs, we need to guarantee the package dependencies are pinned by hash too. For normal pip installs, we verify that by using --require-hashes flag. Unfortunately, --require-hashes flag is not compatible with editable installs, so we use --no-deps flag to verify the dependencies are not installed since we can't verify if they are pinned. Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * test: Editable pip install in GHA Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * test: Editable pip install in Dockerfile Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * test: Editable pip install in shell script Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * fix: Code complexity increase Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * fix: Simplify boolean return Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * docs: Add pip editable install references in comments Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * fix: Handle multiple packages in editable pip install Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * test: Multi editable pip install in GHA Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * test: Multi editable pip install in Dockerfile Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> * test: Multi editable pip install in shell script Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> --------- Signed-off-by: Gabriela Gutierrez <gabigutierrez@google.com> Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
ossf · Mar 13, 2023 · 6ff94eb · 6ff94eb
1 parent 110e352
commit 6ff94eb
Show file tree

Hide file tree

Showing 7 changed files with 322 additions and 5 deletions.
diff --git a/checks/raw/pinned_dependencies_test.go b/checks/raw/pinned_dependencies_test.go
@@ -261,7 +261,7 @@ func TestGithubWorkflowPkgManagerPinning(t *testing.T) {
 		{
 			name:     "npm packages without verification",
 			filename: "./testdata/.github/workflows/github-workflow-pkg-managers.yaml",
-			warns:    36,
+			warns:    46,
 		},
 	}
 	for _, tt := range tests {
@@ -557,6 +557,66 @@ func TestDockerfileInsecureDownloadsLineNumber(t *testing.T) {
 					endLine:   42,
 					t:         checker.DependencyUseTypePipCommand,
 				},
+				{
+					snippet:   "pip install --no-deps -e hg+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 46,
+					endLine:   46,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e svn+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 47,
+					endLine:   47,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e bzr+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 48,
+					endLine:   48,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git",
+					startLine: 49,
+					endLine:   49,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git#egg=package",
+					startLine: 50,
+					endLine:   50,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git@v1.0",
+					startLine: 51,
+					endLine:   51,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git@v1.0#egg=package",
+					startLine: 52,
+					endLine:   52,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 60,
+					endLine:   60,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e . git+https://github.com/username/repo.git",
+					startLine: 61,
+					endLine:   61,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "python -m pip install --no-deps -e git+https://github.com/username/repo.git",
+					startLine: 64,
+					endLine:   64,
+					t:         checker.DependencyUseTypePipCommand,
+				},
 			},
 		},
 		{
@@ -699,6 +759,66 @@ func TestShellscriptInsecureDownloadsLineNumber(t *testing.T) {
 					endLine:   31,
 					t:         checker.DependencyUseTypeChocoCommand,
 				},
+				{
+					snippet:   "pip install --no-deps -e hg+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 38,
+					endLine:   38,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e svn+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 39,
+					endLine:   39,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e bzr+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 40,
+					endLine:   40,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git",
+					startLine: 41,
+					endLine:   41,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git#egg=package",
+					startLine: 42,
+					endLine:   42,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git@v1.0",
+					startLine: 43,
+					endLine:   43,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e git+https://github.com/username/repo.git@v1.0#egg=package",
+					startLine: 44,
+					endLine:   44,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package",
+					startLine: 52,
+					endLine:   52,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "pip install --no-deps -e . git+https://github.com/username/repo.git",
+					startLine: 53,
+					endLine:   53,
+					t:         checker.DependencyUseTypePipCommand,
+				},
+				{
+					snippet:   "python -m pip install --no-deps -e git+https://github.com/username/repo.git",
+					startLine: 56,
+					endLine:   56,
+					t:         checker.DependencyUseTypePipCommand,
+				},
 			},
 		},
 	}
@@ -851,7 +971,7 @@ func TestDockerfileScriptDownload(t *testing.T) {
 		{
 			name:     "pkg managers",
 			filename: "./testdata/Dockerfile-pkg-managers",
-			warns:    47,
+			warns:    57,
 		},
 		{
 			name:     "download with some python",
@@ -969,7 +1089,7 @@ func TestShellScriptDownload(t *testing.T) {
 		{
 			name:     "pkg managers",
 			filename: "./testdata/script-pkg-managers",
-			warns:    43,
+			warns:    53,
 		},
 		{
 			name:     "invalid shell script",

diff --git a/checks/raw/shell_download_validate.go b/checks/raw/shell_download_validate.go
@@ -487,12 +487,35 @@ func isGoUnpinnedDownload(cmd []string) bool {
 	return found
 }
 
+func isPinnedEditableSource(pkgSource string) bool {
+	regexRemoteSource := regexp.MustCompile(`^(git|svn|hg|bzr).+$`)
+	// Is from local source
+	if !regexRemoteSource.MatchString(pkgSource) {
+		return true
+	}
+	// Is VCS install from Git and it's pinned
+	// https://pip.pypa.io/en/latest/topics/vcs-support/#vcs-support
+	regexGitSource := regexp.MustCompile(`^git(\+(https?|ssh|git))?\:\/\/.*(.git)?@[a-fA-F0-9]{40}(#egg=.*)?$`)
+	return regexGitSource.MatchString(pkgSource)
+	// Disclaimer: We are not handling if Subversion (svn),
+	// Mercurial (hg) or Bazaar (bzr) remote sources are pinned
+	// because they are not common on GitHub repos
+}
+
+func isFlag(cmd string) bool {
+	regexFlag := regexp.MustCompile(`^(\-\-?\w+)+$`)
+	return regexFlag.MatchString(cmd)
+}
+
 func isUnpinnedPipInstall(cmd []string) bool {
 	if !isBinaryName("pip", cmd[0]) && !isBinaryName("pip3", cmd[0]) {
 		return false
 	}
 
 	isInstall := false
+	hasNoDeps := false
+	isEditableInstall := false
+	isPinnedEditableInstall := true
 	hasRequireHashes := false
 	hasAdditionalArgs := false
 	hasWheel := false
@@ -507,12 +530,33 @@ func isUnpinnedPipInstall(cmd []string) bool {
 			break
 		}
 
+		// Require --no-deps to not install the dependencies when doing editable install
+		// because we can't verify if dependencies are pinned
+		// https://pip.pypa.io/en/stable/topics/secure-installs/#do-not-use-setuptools-directly
+		// https://github.com/pypa/pip/issues/4995
+		if strings.EqualFold(cmd[i], "--no-deps") {
+			hasNoDeps = true
+			continue
+		}
+
+		// https://pip.pypa.io/en/stable/cli/pip_install/#cmdoption-e
+		if slices.Contains([]string{"-e", "--editable"}, cmd[i]) {
+			isEditableInstall = true
+			continue
+		}
+
 		// https://github.com/ossf/scorecard/issues/1306#issuecomment-974539197.
 		if strings.EqualFold(cmd[i], "--require-hashes") {
 			hasRequireHashes = true
 			break
 		}
 
+		// Catch not handled flags, otherwise is package
+		if isFlag(cmd[i]) {
+			continue
+		}
+
+		// Wheel package
 		// Exclude *.whl as they're mostly used
 		// for tests. See https://github.com/ossf/scorecard/pull/611.
 		if strings.HasSuffix(cmd[i], ".whl") {
@@ -522,9 +566,29 @@ func isUnpinnedPipInstall(cmd []string) bool {
 			continue
 		}
 
+		// Editable install package source
+		if isEditableInstall {
+			isPinned := isPinnedEditableSource(cmd[i])
+			if !isPinned {
+				isPinnedEditableInstall = false
+			}
+			continue
+		}
+
 		hasAdditionalArgs = true
 	}
 
+	// --require-hashes and -e flags cannot be used together in pip install
+	// -e and *.whl package cannot be used together in pip install
+
+	// If is editable install, it's secure if package is from local source
+	// or from remote (VCS install) pinned by hash, and if dependencies are
+	// not installed.
+	// Example: `pip install --no-deps -e git+https://git.repo/some_pkg.git@da39a3ee5e6b4b0d3255bfef95601890afd80709`
+	if isEditableInstall {
+		return !hasNoDeps || !isPinnedEditableInstall
+	}
+
 	// If hashes are required, it's pinned.
 	if hasRequireHashes {
 		return false

diff --git a/checks/raw/testdata/.github/workflows/github-workflow-pkg-managers.yaml b/checks/raw/testdata/.github/workflows/github-workflow-pkg-managers.yaml
@@ -90,6 +90,44 @@ jobs:
         run: pip3 install somepkg==1.2.3
       - name:
         run: /bin/pip3 install -X -H somepkg
+      - name:
+        run: pip install --no-deps --editable .
+      - name:
+        run: pip install --no-deps -e .
+      - name:
+        run: pip install --no-deps -e hg+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e svn+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e bzr+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo.git
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo.git#egg=package
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo.git@v1.0
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo.git@v1.0#egg=package
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e git+https://github.com/username/repo@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e git+http://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e git+ssh://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e git+git://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e git://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+      - name:
+        run: pip install --no-deps -e . git+https://github.com/username/repo.git
+      - name:
+        run: pip install --no-deps -e . git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
       - name:
         run: python -m notpip -X bla
       - name:
@@ -108,6 +146,10 @@ jobs:
         run: python -m pip install 'some-pkg==1.2.3'
       - name:
         run: python -m pip install 'some-pkg>1.2.3'
+      - name:
+        run: python -m pip install --no-deps -e git+https://github.com/username/repo.git
+      - name:
+        run: python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
       - name:
         run: pip3 install -r bla-requirements.txt --require-hashes && pip3 install --require-hashes -r bla-requirements.txt
       - name:

diff --git a/checks/raw/testdata/Dockerfile-download-lines b/checks/raw/testdata/Dockerfile-download-lines
@@ -39,4 +39,27 @@ RUN echo hello && \
 #RUN curl -s ifconfig.co/json | grep "China" > /dev/null && \
 #    pip install -r requirements.txt -i https://pypi.doubanio.com/simple --trusted-host pypi.doubanio.com || \
 RUN bla && \
-	pip install -r requirements.txt
+	pip install -r requirements.txt
+
+RUN pip install --no-deps --editable .
+RUN pip install --no-deps -e .
+RUN pip install --no-deps -e hg+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e svn+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e bzr+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo.git
+RUN pip install --no-deps -e git+https://github.com/username/repo.git#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@v1.0
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@v1.0#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+http://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+ssh://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+git://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e . git+https://github.com/username/repo.git
+RUN pip install --no-deps -e . git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+
+RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git
+RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
diff --git a/checks/raw/testdata/Dockerfile-pkg-managers b/checks/raw/testdata/Dockerfile-pkg-managers
@@ -61,6 +61,26 @@ RUN pip install somepkg
 RUN pip3 install somepkg==1.2.3
 RUN /bin/pip3 install -X -H somepkg
 
+RUN pip install --no-deps --editable .
+RUN pip install --no-deps -e .
+RUN pip install --no-deps -e hg+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e svn+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e bzr+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo.git
+RUN pip install --no-deps -e git+https://github.com/username/repo.git#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@v1.0
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@v1.0#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567
+RUN pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+https://github.com/username/repo@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+http://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+ssh://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git+git://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e git://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+RUN pip install --no-deps -e . git+https://github.com/username/repo.git
+RUN pip install --no-deps -e . git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+
 RUN python -m notpip -X bla
 
 RUN python2.7 -m pip install -X -H somepkg \
@@ -84,6 +104,9 @@ RUN python -m pip install -r file
 RUN python -m pip install 'some-pkg==1.2.3'
 RUN python -m pip install 'some-pkg>1.2.3'
 
+RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git
+RUN python -m pip install --no-deps -e git+https://github.com/username/repo.git@0123456789abcdef0123456789abcdef01234567#egg=package
+
 RUN npm install typescript
 RUN npm install -g typescript
 RUN npm i typescript