Skip to content

Propagate ENV vars to SAST and Dependency Scanning Docker containers only if they are set

Victor Zagorodny requested to merge 10694-propagate-env-vars-only-if-set into master

What does this MR do?

SAST and Dependency Scanning both have a bunch of ENV vars that configure their invocation. In particular, the Docker image tag for underlying analyzer plugins can be configured via those. And some of them, like SAST_ANALYZER_IMAGE_TAG, were previously set at build stage (via Dockerfile ENV instruction) for the respective images.

Additionally, it makes sense to propagate those variables from CI job to containers only if they are set, because their default values matter. The docker run --env VAR sast syntax of docker run seems the best fit for such conditional ENV vars propagation.

But, it does not work as documented (and this is probably a bug in Docker CLI). Contrary, it an ENV var was set during the build stage, it was passed to docker run using the --env VAR syntax and it was not actually set in the docker executable environment, it gets reset to an empty string value inside the running container.

This breaks SAST and Dependency Scanning tools' plugin architecture causing wrong plugin Docker images being fetched.

The workaround presented in this MR fixes the problem at the CI shell command execution level. It should be removed when the removal of Docker-in-Docker for SAST and Dependency Scanning is implemented.

Related issues

#10694 (closed)

CE backport MR

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/27659

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Victor Zagorodny

Merge request reports

Loading