Repository: ossf/package-analysis (go)

Issue:
Remove legacy per-ecosystem sandbox images
Since #714 was merged, we no longer need the individual dynamic sandboxes for each ecosystem. These can be removed from the build system and analysis code

However, we should still not hardcode usage of dynamic analysis image in (it should remain a member of the ecosystem object) 

In the future, we should separate sandbox selection from the ecosystem API. We should also copy the analysis script into the image during startup rather than build it into the image. 

Pull request description:
Remove individual ecosystem sandboxes from production code and builds
Closes #725 

1. remove individual ecosystem sandboxes from analysis code and cloudbuild
2. update Makefile to maintain recipes for building the legacy images, but `prod` and `legacy` tags are used in recipe names to differentiate
3. remove legacy build_docker.sh script which was no longer in use
4. update READMEs accordingly

Patch:
diff --git a/.github/workflows/build-images.yml b/.github/workflows/build-images.yml
index 88be1ac..ae0aef5 100644
--- a/.github/workflows/build-images.yml
+++ b/.github/workflows/build-images.yml
@@ -28,4 +28,4 @@ jobs:
         docker version -f '{{.Server.Experimental}}'
 
     - name: build_docker
-      run: make build_all_images
+      run: make build_prod_images
diff --git a/Makefile b/Makefile
index 118edb6..d5c426d 100644
--- a/Makefile
+++ b/Makefile
@@ -24,7 +24,7 @@ endif
 # This recipe builds and pushes images for production. Note: RELEASE_TAG must be set
 #
 .PHONY: cloudbuild
-cloudbuild: require_release_tag push_all_images
+cloudbuild: require_release_tag push_prod_images
 
 .PHONY: require_release_tag
 require_release_tag:
@@ -83,11 +83,11 @@ build_dynamic_analysis_sandbox: DIR=$(SANDBOX_DIR)/dynamicanalysis
 build_dynamic_analysis_sandbox: DOCKERFILE=$(SANDBOX_DIR)/dynamicanalysis/Dockerfile
 build_dynamic_analysis_sandbox: IMAGE_NAME=dynamic-analysis
 
-.PHONY: build_all_sandboxes
-build_all_sandboxes: build_node_sandbox build_python_sandbox build_ruby_sandbox build_packagist_sandbox build_crates_sandbox build_dynamic_analysis_sandbox build_static_analysis_sandbox
+.PHONY: build_legacy_sandboxes
+build_legacy_sandboxes: build_node_sandbox build_python_sandbox build_ruby_sandbox build_packagist_sandbox build_crates_sandbox
 
-.PHONY: build_all_images
-build_all_images: build_all_sandboxes build_analysis_image build_scheduler_image
+.PHONY: build_prod_images
+build_prod_images: build_dynamic_analysis_sandbox build_static_analysis_sandbox build_analysis_image build_scheduler_image
 
 #
 # Builds then pushes analysis and sandbox images
@@ -102,32 +102,17 @@ push_analysis_image: build_analysis_image
 push_scheduler_image: IMAGE_NAME=scheduler
 push_scheduler_image: build_scheduler_image
 
-push_node_sandbox: IMAGE_NAME=node
-push_node_sandbox: build_node_sandbox
-
-push_python_sandbox: IMAGE_NAME=python
-push_python_sandbox: build_python_sandbox
-
-push_ruby_sandbox: IMAGE_NAME=ruby
-push_ruby_sandbox: build_ruby_sandbox
-
-push_packagist_sandbox: IMAGE_NAME=packagist
-push_packagist_sandbox: build_packagist_sandbox
-
-push_crates_sandbox: IMAGE_NAME=crates.io
-push_crates_sandbox: build_crates_sandbox
-
 push_dynamic_analysis_sandbox: IMAGE_NAME=dynamic-analysis
 push_dynamic_analysis_sandbox: build_dynamic_analysis_sandbox
 
 push_static_analysis_sandbox: IMAGE_NAME=static-analysis
 push_static_analysis_sandbox: build_static_analysis_sandbox
 
-.PHONY: push_all_sandboxes
-push_all_sandboxes: push_node_sandbox push_python_sandbox push_ruby_sandbox push_packagist_sandbox push_crates_sandbox push_dynamic_analysis_sandbox push_static_analysis_sandbox
+.PHONY: push_prod_sandboxes
+push_prod_sandboxes: push_dynamic_analysis_sandbox push_static_analysis_sandbox
 
-.PHONY: push_all_images
-push_all_images: push_all_sandboxes push_analysis_image push_scheduler_image
+.PHONY: push_prod_images
+push_prod_images: push_prod_sandboxes push_analysis_image push_scheduler_image
 
 
 #
@@ -159,8 +144,11 @@ sync_dynamic_analysis_sandbox: build_dynamic_analysis_sandbox
 sync_static_analysis_sandbox: IMAGE_NAME=static-analysis
 sync_static_analysis_sandbox: build_static_analysis_sandbox
 
-.PHONY: sync_all_sandboxes
-sync_all_sandboxes: sync_node_sandbox sync_python_sandbox sync_ruby_sandbox sync_packagist_sandbox sync_crates_sandbox sync_dynamic_analysis_sandbox sync_static_analysis_sandbox
+.PHONY: sync_legacy_sandboxes
+sync_legacy_sandboxes: sync_node_sandbox sync_python_sandbox sync_ruby_sandbox sync_packagist_sandbox sync_crates_sandbox
+
+.PHONY: sync_prod_sandboxes
+sync_prod_sandboxes: sync_dynamic_analysis_sandbox sync_static_analysis_sandbox
 
 
 #
diff --git a/README.md b/README.md
index 6da20f3..9f85769 100644
--- a/README.md
+++ b/README.md
@@ -101,8 +101,8 @@ Values should follow
 ## Local Analysis
 
 To run the analysis code locally, the easiest way is to use the Docker image
-`gcr.io/ossf-malware-analysis/analysis`. This can be built from
-`./build/build_docker.sh`, or the public images can be used instead.
+`gcr.io/ossf-malware-analysis/analysis`. This can be built with
+`make build_analysis_image`, or the public images can be used instead.
 
 This container uses `podman` to run a nested, sandboxed ([gVisor]) container for
 analysis.
diff --git a/build/build_docker.sh b/build/build_docker.sh
deleted file mode 100755
index b82cb20..0000000
--- a/build/build_docker.sh
+++ /dev/null
@@ -1,35 +0,0 @@
-#!/bin/bash -ex
-
-push=${PUSH:-"false"}
-tag=${RELEASE_TAG}
-
-BASE_PATH="$(dirname "$(dirname "$(realpath "$0")")")"
-REGISTRY=gcr.io/ossf-malware-analysis
-
-# Mapping from the container name to the path containing the Dockerfile.
-declare -A ANALYSIS_IMAGES=( [node]=npm [python]=pypi [ruby]=rubygems [packagist]=packagist [crates.io]=crates.io )
-
-pushd "$BASE_PATH/sandboxes"
-for image in "${!ANALYSIS_IMAGES[@]}"; do
-  extra_args=""
-  if [ "$tag" != "" ]; then
-    extra_args="-t $REGISTRY/$image:$tag"
-  fi
-  docker build $extra_args -t $REGISTRY/$image ${ANALYSIS_IMAGES[$image]}
-  [[ "$push" == "true" ]] && docker push --all-tags $REGISTRY/$image
-done
-popd
-
-# Mapping from the container name to the path containing the Dockerfile.
-declare -A CMD_IMAGES=( [analysis]=analyze [scheduler]=scheduler )
-
-pushd "$BASE_PATH"
-for image in "${!CMD_IMAGES[@]}"; do
-  extra_args=""
-  if [ "$tag" != "" ]; then
-    extra_args="-t $REGISTRY/$image:$tag --build-arg=SANDBOX_IMAGE_TAG=$tag"
-  fi
-  docker build $extra_args -t $REGISTRY/$image -f cmd/${CMD_IMAGES[$image]}/Dockerfile .
-  [[ "$push" == "true" ]] && docker push --all-tags $REGISTRY/$image
-done
-popd
diff --git a/cmd/analyze/main.go b/cmd/analyze/main.go
index 5ee3d4a..e191ed3 100644
--- a/cmd/analyze/main.go
+++ b/cmd/analyze/main.go
@@ -31,7 +31,6 @@ var (
 	staticUpload        = flag.String("upload-static", "", "bucket path for uploading static analysis results")
 	uploadFileWriteInfo = flag.String("upload-file-write-info", "", "bucket path for uploading information from file writes")
 	offline             = flag.Bool("offline", false, "disables sandbox network access")
-	combinedSandbox     = flag.Bool("combined-sandbox", true, "use combined sandbox image for dynamic analysis")
 	listModes           = flag.Bool("list-modes", false, "prints out a list of available analysis modes")
 	help                = flag.Bool("help", false, "print help on available options")
 	analysisMode        = utils.CommaSeparatedFlags("mode", []string{"static", "dynamic"},
@@ -173,7 +172,7 @@ func main() {
 		return
 	}
 
-	manager := pkgmanager.Manager(ecosystem, *combinedSandbox)
+	manager := pkgmanager.Manager(ecosystem)
 	if manager == nil {
 		log.Panic("Unsupported pkg manager",
 			log.Label("ecosystem", string(ecosystem)))
diff --git a/cmd/worker/main.go b/cmd/worker/main.go
index b33c96d..425d358 100644
--- a/cmd/worker/main.go
+++ b/cmd/worker/main.go
@@ -121,7 +121,7 @@ func handleMessage(ctx context.Context, msg *pubsub.Message, packagesBucket *blo
 		return nil
 	}
 
-	manager := pkgmanager.Manager(ecosystem, true)
+	manager := pkgmanager.Manager(ecosystem)
 	if manager == nil {
 		log.Warn("Unsupported pkg manager",
 			log.Label("ecosystem", ecosystem.String()),
diff --git a/internal/pkgmanager/crates.io.go b/internal/pkgmanager/crates.io.go
index d932a58..f8f3f4c 100644
--- a/internal/pkgmanager/crates.io.go
+++ b/internal/pkgmanager/crates.io.go
@@ -33,14 +33,6 @@ func getCratesLatest(pkg string) (string, error) {
 }
 
 var cratesPkgManager = PkgManager{
-	ecosystem:     pkgecosystem.CratesIO,
-	image:         "gcr.io/ossf-malware-analysis/crates.io",
-	command:       "/usr/local/bin/analyze.py",
-	latestVersion: getCratesLatest,
-	dynamicPhases: analysisrun.DefaultDynamicPhases(),
-}
-
-var cratesPkgManagerCombinedSandbox = PkgManager{
 	ecosystem:     pkgecosystem.CratesIO,
 	image:         combinedDynamicAnalysisImage,
 	command:       "/usr/local/bin/analyze-rust.py",
diff --git a/internal/pkgmanager/ecosystem.go b/internal/pkgmanager/ecosystem.go
index f186653..698552c 100644
--- a/internal/pkgmanager/ecosystem.go
+++ b/internal/pkgmanager/ecosystem.go
@@ -29,21 +29,9 @@ var (
 		packagistPkgManager.ecosystem: &packagistPkgManager,
 		cratesPkgManager.ecosystem:    &cratesPkgManager,
 	}
-
-	supportedPkgManagersCombinedSandbox = map[pkgecosystem.Ecosystem]*PkgManager{
-		npmPkgManagerCombinedSandbox.ecosystem:       &npmPkgManagerCombinedSandbox,
-		pypiPkgManagerCombinedSandbox.ecosystem:      &pypiPkgManagerCombinedSandbox,
-		rubygemsPkgManagerCombinedSandbox.ecosystem:  &rubygemsPkgManagerCombinedSandbox,
-		packagistPkgManagerCombinedSandbox.ecosystem: &packagistPkgManagerCombinedSandbox,
-		cratesPkgManagerCombinedSandbox.ecosystem:    &cratesPkgManagerCombinedSandbox,
-	}
 )
 
-func Manager(e pkgecosystem.Ecosystem, combinedSandbox bool) *PkgManager {
-	if combinedSandbox {
-		return supportedPkgManagersCombinedSandbox[e]
-	}
-
+func Manager(e pkgecosystem.Ecosystem) *PkgManager {
 	return supportedPkgManagers[e]
 }
 
diff --git a/internal/pkgmanager/npm.go b/internal/pkgmanager/npm.go
index 5ef9280..29a00be 100644
--- a/internal/pkgmanager/npm.go
+++ b/internal/pkgmanager/npm.go
@@ -72,16 +72,6 @@ func getNPMArchiveURL(pkgName, version string) (string, error) {
 }
 
 var npmPkgManager = PkgManager{
-	ecosystem:      pkgecosystem.NPM,
-	image:          "gcr.io/ossf-malware-analysis/node",
-	command:        "/usr/local/bin/analyze.js",
-	latestVersion:  getNPMLatest,
-	archiveURL:     getNPMArchiveURL,
-	extractArchive: utils.ExtractTarGzFile,
-	dynamicPhases:  analysisrun.DefaultDynamicPhases(),
-}
-
-var npmPkgManagerCombinedSandbox = PkgManager{
 	ecosystem:      pkgecosystem.NPM,
 	image:          combinedDynamicAnalysisImage,
 	command:        "/usr/local/bin/analyze-node.js",
diff --git a/internal/pkgmanager/packagist.go b/internal/pkgmanager/packagist.go
index ce3a83a..ffb2fb9 100644
--- a/internal/pkgmanager/packagist.go
+++ b/internal/pkgmanager/packagist.go
@@ -50,14 +50,6 @@ func getPackagistLatest(pkg string) (string, error) {
 }
 
 var packagistPkgManager = PkgManager{
-	ecosystem:     pkgecosystem.Packagist,
-	image:         "gcr.io/ossf-malware-analysis/packagist",
-	command:       "/usr/local/bin/analyze.php",
-	latestVersion: getPackagistLatest,
-	dynamicPhases: analysisrun.DefaultDynamicPhases(),
-}
-
-var packagistPkgManagerCombinedSandbox = PkgManager{
 	ecosystem:     pkgecosystem.Packagist,
 	image:         combinedDynamicAnalysisImage,
 	command:       "/usr/local/bin/analyze-php.php",
diff --git a/internal/pkgmanager/pypi.go b/internal/pkgmanager/pypi.go
index c711b86..4715571 100644
--- a/internal/pkgmanager/pypi.go
+++ b/internal/pkgmanager/pypi.go
@@ -79,16 +79,6 @@ func getPyPIArchiveURL(pkgName, version string) (string, error) {
 }
 
 var pypiPkgManager = PkgManager{
-	ecosystem:      pkgecosystem.PyPI,
-	image:          "gcr.io/ossf-malware-analysis/python",
-	command:        "/usr/local/bin/analyze.py",
-	latestVersion:  getPyPILatest,
-	archiveURL:     getPyPIArchiveURL,
-	extractArchive: utils.ExtractTarGzFile,
-	dynamicPhases:  analysisrun.DefaultDynamicPhases(),
-}
-
-var pypiPkgManagerCombinedSandbox = PkgManager{
 	ecosystem:      pkgecosystem.PyPI,
 	image:          combinedDynamicAnalysisImage,
 	command:        "/usr/local/bin/analyze-python.py",
diff --git a/internal/pkgmanager/rubygems.go b/internal/pkgmanager/rubygems.go
index 24d1a4e..26c2202 100644
--- a/internal/pkgmanager/rubygems.go
+++ b/internal/pkgmanager/rubygems.go
@@ -31,14 +31,6 @@ func getRubyGemsLatest(pkg string) (string, error) {
 }
 
 var rubygemsPkgManager = PkgManager{
-	ecosystem:     pkgecosystem.RubyGems,
-	image:         "gcr.io/ossf-malware-analysis/ruby",
-	command:       "/usr/local/bin/analyze.rb",
-	latestVersion: getRubyGemsLatest,
-	dynamicPhases: analysisrun.DefaultDynamicPhases(),
-}
-
-var rubygemsPkgManagerCombinedSandbox = PkgManager{
 	ecosystem:     pkgecosystem.RubyGems,
 	image:         combinedDynamicAnalysisImage,
 	command:       "/usr/local/bin/analyze-ruby.rb",
diff --git a/sandboxes/README.md b/sandboxes/README.md
index 1a0fac9..b14c6dc 100644
--- a/sandboxes/README.md
+++ b/sandboxes/README.md
@@ -3,37 +3,43 @@
 ## Sandbox Image Testing
 
 By default the analysis command will update the sandbox images from the grc.io
-repository.
+repository. To test local changes to sandboxes, they need to be built locally,
+and the analysis needs to redirect its container source to the local filesystem.
 
-To use local images for analysis:
+Details are below, but basically, this means two things:
 
-1. The images need to be manually pulled into `/var/lib/containers` using the
-   `buildah` tool included with `podman`.
-2. `/var/lib/containers` then needs to be mounted inside the container so it can
-   be used by `podman`.
-3. Finally, the `analysis` command should be run using the `-nopull` option to
-   use the local image only.
+1. Run `make sync_prod_sandboxes` to build and sync the sandbox images locally
+2. Use the `-nopull` option with `scripts/run_analysis.sh` to disable use of remote images
 
-For example, assuming there is a locally built npm image:
 
+### Building the images
+
+To use local images for analysis, they first need to be synced with the
+container directory used by `podman` (which is `/var/lib/containers`).
+This is automated using the project Makefile.
+
+To sync the both dynamic and static analysis sandboxes, run
+
+```bash
+make sync_dynamic_analyis_sandbox
+make sync_static_analyis_sandbox
+```
+or simply
 ```bash
-# Prepare the local machine.
-$ sudo apt install podman
-$ mkdir /tmp/results
-
-# Populate the local podman images with the image we want to use from docker.
-$ sudo buildah pull docker-daemon:gcr.io/ossf-malware-analysis/node:latest
-
-# Run the analysis locally, without pulling the image again.
-$ docker run --privileged -ti \
-    -v /tmp/results:/results \
-    -v /var/lib/containers:/var/lib/containers \
-    gcr.io/ossf-malware-analysis/analysis analyze \
-    -package test -ecosystem npm \
-    -upload file:///results/ -nopull
+make sync_prod_sandboxes
 ```
 
-## Adding a new Runtime Analysis Sandbox
+These commands will (re-)build both sandboxes and copy them to the correct location.
+
+### Running the analysis
+
+The `scripts/run_analysis.sh` script automates much of the setup for running
+local analysis, but it the default setting will pull the sandbox images from
+the remote container registry rather than using locally built ones. To change
+this, add the `-nopull` option to the script.
+
+
+## Adding a new Runtime Analysis script
 
 Each runtime analysis sandbox requires:
 
@@ -161,7 +167,7 @@ The `Dockerfile` must also set the analysis command as the `ENTRYPOINT`.
 
 ### Wiring It Up
 
-1. Update [build_docker.sh](../build/build_docker.sh) to reference the image.
+1. Update [Makefile](../Makefile) to reference the image.
 2. Extend [internal/pkgecosystem](../internal/pkgecosystem) to add support for
    the new sandbox.
 3. Ensure your new ecosystem is supported by
diff --git a/sandboxes/staticanalysis/staticanalyze.go b/sandboxes/staticanalysis/staticanalyze.go
index d681525..b57f349 100644
--- a/sandboxes/staticanalysis/staticanalyze.go
+++ b/sandboxes/staticanalysis/staticanalyze.go
@@ -116,7 +116,7 @@ func run() (err error) {
 		return fmt.Errorf("ecosystem and package are required arguments")
 	}
 
-	manager := pkgmanager.Manager(ecosystem, false)
+	manager := pkgmanager.Manager(ecosystem)
 	if manager == nil {
 		return fmt.Errorf("unsupported pkg manager for ecosystem %s", ecosystem)
 	}


Test patch:
diff --git a/internal/pkgmanager/download_test.go b/internal/pkgmanager/download_test.go
index af3e905..fa6aafb 100644
--- a/internal/pkgmanager/download_test.go
+++ b/internal/pkgmanager/download_test.go
@@ -60,7 +60,7 @@ func TestDownloadNpmArchive(t *testing.T) {
 		{"pkgname=(invalid)", "fr(2t5j923)", "latest", true},
 	}
 
-	testDownload(t, tests, Manager(pkgecosystem.NPM, false))
+	testDownload(t, tests, Manager(pkgecosystem.NPM))
 }
 
 func TestDownloadPyPIArchive(t *testing.T) {
@@ -70,7 +70,7 @@ func TestDownloadPyPIArchive(t *testing.T) {
 		{"pkgname=(invalid)", "fr(2t5j923)", "123", true},
 	}
 
-	testDownload(t, tests, Manager(pkgecosystem.PyPI, false))
+	testDownload(t, tests, Manager(pkgecosystem.PyPI))
 }
 
 func TestDownloadToDirectory(t *testing.T) {