Skip to content

Delete/clear git submodules on job initialization

Axel von Bertoldi requested to merge avonbertoldi/359039-cleanup-git-submodules into main

Related issues

gitlab#359039 (closed)

What does this MR do?

This MR does 2 things:

  1. deletes/clears a project's submodules during job init/setup.
  2. adds a runner-only configuration (keep_git_submodules) to disable doing this

Why was this MR needed?

gitlab#359039 (closed) describes a possible security vulnerability with specific runners (or more specifically, non-ephemeral runners): With non-ephemeral runners, submodules checked out by a job initiated by an authorised (as in has at least read access to the submodule) are left after the job completes. The submodules and can subsequently be accessed by a job initiated by an unauthorised (as in does not have at least read permission on the submodule) user. If the submodule has secrets, the "unauthorised" user can leak them.

The fix, suggested by @dcouture, was to just delete the submodules on every job run. This is fine in that it eliminates the vulnerability and is easy to implement, but it could represent a significant CI pipeline performance and network traffic consequences for projects with many and/or large submodules, or with submodules that have lfs. For this reason I've added an option for runner admins to disable cleaning of submodules. In the documentation which I will add elsewhere, I'll mention that the conditions under which a runner admin might want to skip disabling submodule cleaning are:

  1. a project has many and/or large submodules, and/or said submodules use lfs, AND
  2. the project's developers are all also trusted developers of the project's submodules (i.e. all that users that have >= read access the the project, also have >= read access to the submodules).

What's the best way to test this MR?

gitlab#359039 (closed) describes how to reproduce the vulnerability, and this can be used to test this change. It's quite an involved and onerous procedure, and too much for an MR reviewer. A much easier (though not quite as thorough) way to test it is to:

  • to your favourite test project, add one or more submodules
  • add CI_DEBUG_TRACE to the projects gitlab-ci-.yaml
  • register a specific runner with the project (docker or shell executor is probably easiest), and disable/pause other runners
    1. kick off a CI job and ensure git submodule deinit --force --all appears in the job trace
    1. in the specific runner's configuration, add keep_git_submodule on the executor's configuration
    • kick off a CI job and ensure git submodule deinit --force --all DOES NOT appear in the job trace

Note that this method of testing is rather indirect. Because the cleaning happens on job initialization (and immediately before the submodules are initialized), it's not practical to directly inspect that the submodules have actually been deleted. Short of reproducing the issue, directly testing this is not practical (for MR reviewers).

Merge request reports