diff --git a/common/config.go b/common/config.go index e71c03c08f420173efecb920af25d741f6a3447e..d2905178f13b126c5ac0e456f45608191b79eb22 100644 --- a/common/config.go +++ b/common/config.go @@ -1190,6 +1190,7 @@ type RunnerSettings struct { DebugTraceDisabled bool `toml:"debug_trace_disabled,omitempty" json:"debug_trace_disabled" long:"debug-trace-disabled" env:"RUNNER_DEBUG_TRACE_DISABLED" description:"When set to true Runner will disable the possibility of using the CI_DEBUG_TRACE feature"` SafeDirectoryCheckout *bool `toml:"safe_directory_checkout,omitempty" json:"safe_directory_checkout,omitempty" long:"safe-directory-checkout" env:"RUNNER_SAFE_DIRECTORY_CHECKOUT" description:"When set to true, Git global configuration will get a safe.directory directive pointing the job's working directory'"` + CleanGitConfig *bool `toml:"clean_git_config,omitempty" json:"clean_git_config,omitempty" long:"clean-git-config" env:"RUNNER_CLEAN_GIT_CONFIG" description:"Clean git configuration before and after the build. Defaults to true, except the shell executor is used or the git strategy is \"none\""` Shell string `toml:"shell,omitempty" json:"shell" long:"shell" env:"RUNNER_SHELL" description:"Select bash, sh, cmd, pwsh or powershell" jsonschema:"enum=bash,enum=sh,enum=cmd,enum=pwsh,enum=powershell,enum="` CustomBuildDir CustomBuildDir `toml:"custom_build_dir,omitempty" json:"custom_build_dir,omitempty" group:"custom build dir configuration" namespace:"custom_build_dir"` diff --git a/docs/configuration/advanced-configuration.md b/docs/configuration/advanced-configuration.md index 60defe7aeb411dc91128e8b00d484c8170813343..75c586d22da5ab8ceaaccd3830d62ee526688877 100644 --- a/docs/configuration/advanced-configuration.md +++ b/docs/configuration/advanced-configuration.md @@ -257,6 +257,7 @@ Each `[[runners]]` section defines one runner. | `post_build_script` | Commands to be executed on the runner just after executing the job, but before executing `after_script`. To insert multiple commands, use a (triple-quoted) multi-line string or `\n` character. | | `clone_url` | Overwrite the URL for the GitLab instance. Used only if the runner can't connect to the GitLab URL. | | `debug_trace_disabled` | Disables [debug tracing](https://docs.gitlab.com/ci/variables/#enable-debug-logging). When set to `true`, the debug log (trace) remains disabled even if `CI_DEBUG_TRACE` is set to `true`. | +| `clean_git_config` | Cleans the Git configuration. For more information, see [Cleaning Git configuration](#cleaning-git-configuration). | | `referees` | Extra job monitoring workers that pass their results as job artifacts to GitLab. | | `unhealthy_requests_limit` | The number of `unhealthy` responses to new job requests after which a runner worker is disabled. | | `unhealthy_interval` | Duration that a runner worker is disabled for after it exceeds the unhealthy requests limit. Supports syntax like '3600 s', '1 h 30 min' etc. | @@ -1770,6 +1771,46 @@ provide stability in such cases. If you have dependencies that are required for your CI, you must install them in some other place. +## Cleaning Git configuration + +{{< history >}} + +- [Introduced](https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/5438) in GitLab Runner 17.10. + +{{< /history >}} + +At the beginning and end of every build, GitLab Runner removes the following +files from the repository and its submodules: + +- Git lock files (`{index,shallow,HEAD,config}.lock`) +- Post-checkout hooks (`hooks/post-checkout`) + +If you enable `clean_git_config`, the following additional files or directories +are removed from the repository, its submodules, and the Git template directory: + +- `.git/config` file +- `.git/hooks` directory + +This cleanup prevents custom, ephemeral, or potentially malicious Git configuration +from caching between jobs. + +Before GitLab Runner 17.10, cleanups behaved differently: + +- Git lock files and Post-checkout hooks cleanup only occurred at the + beginning of a job and not at the end. +- Other Git configurations (now controlled by `clean_git_config`) were not removed unless + `FF_ENABLE_JOB_CLEANUP` was set. When you set this flag, only the main repository's + `.git/config` was deleted but not submodule configurations. + +The `clean_git_config` setting defaults to `true`. But, it defaults to `false` when: + +- [Shell executor](../executors/shell.md) is used. +- [Git strategy](https://docs.gitlab.com/ci/runners/configure_runners/#git-strategy) + is set to `none`. + +Explicit `clean_git_config` configuration takes precedence over the default +setting. + ## The `[runners.referees]` section Use GitLab Runner referees to pass extra job monitoring data to GitLab. Referees are workers in the runner manager that query and collect additional data related to a job. The results diff --git a/executors/custom/integration_test.go b/executors/custom/integration_test.go index 1f65bc09fe8ce067d2dfa224d6e87a6ce13b3c40..7d8853b9ec78e29718981b9f97be51fbb0843eaf 100644 --- a/executors/custom/integration_test.go +++ b/executors/custom/integration_test.go @@ -468,46 +468,73 @@ func TestBuildChangesBranchesWhenFetchingRepo(t *testing.T) { } func TestBuildPowerShellCatchesExceptions(t *testing.T) { + tests := map[string]struct { + cleanGitConfig *bool + expectFreshRepoMessage bool + }{ + "no git cleanup": { + expectFreshRepoMessage: true, + }, + "git cleanup explicitly enabled": { + cleanGitConfig: &[]bool{true}[0], + expectFreshRepoMessage: true, + }, + "git cleanup explicitly disabled": { + cleanGitConfig: &[]bool{false}[0], + expectFreshRepoMessage: false, + }, + } + for _, shell := range []string{"powershell", "pwsh"} { t.Run(shell, func(t *testing.T) { - helpers.SkipIntegrationTests(t, shell) - - successfulBuild, err := common.GetRemoteSuccessfulBuild() - require.NoError(t, err) - - build := newBuild(t, successfulBuild, shell) - build.Variables = append( - build.Variables, - common.JobVariable{Key: "ErrorActionPreference", Value: "Stop"}, - common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"}, - ) - - out, err := buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.Contains(t, out, "Created fresh repository") - - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Regexp(t, "Checking out [a-f0-9]+ as", out) - - build.Variables = append( - build.Variables, - common.JobVariable{Key: "ErrorActionPreference", Value: "Continue"}, - ) - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Regexp(t, "Checking out [a-f0-9]+ as", out) - - build.Variables = append( - build.Variables, - common.JobVariable{Key: "ErrorActionPreference", Value: "SilentlyContinue"}, - ) - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + helpers.SkipIntegrationTests(t, shell) + + successfulBuild, err := common.GetRemoteSuccessfulBuild() + require.NoError(t, err) + + build := newBuild(t, successfulBuild, shell) + build.Variables = append( + build.Variables, + common.JobVariable{Key: "ErrorActionPreference", Value: "Stop"}, + common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"}, + ) + build.Runner.RunnerSettings.CleanGitConfig = test.cleanGitConfig + + checkFreshRepoMessage := assert.NotContains + if test.expectFreshRepoMessage { + checkFreshRepoMessage = assert.Contains + } + + out, err := buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + assert.Contains(t, out, "Created fresh repository") + + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + checkFreshRepoMessage(t, out, "Created fresh repository") + assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + + build.Variables = append( + build.Variables, + common.JobVariable{Key: "ErrorActionPreference", Value: "Continue"}, + ) + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + checkFreshRepoMessage(t, out, "Created fresh repository") + assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + + build.Variables = append( + build.Variables, + common.JobVariable{Key: "ErrorActionPreference", Value: "SilentlyContinue"}, + ) + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + checkFreshRepoMessage(t, out, "Created fresh repository") + assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + }) + } }) } } diff --git a/executors/shell/shell_integration_test.go b/executors/shell/shell_integration_test.go index fe2effe243042d334c792d35f231af0ddb9352c0..4950fca7ee80ea71ec706f3fc3323f30bfc002ea 100644 --- a/executors/shell/shell_integration_test.go +++ b/executors/shell/shell_integration_test.go @@ -1186,39 +1186,68 @@ func TestBuildWithGitSubmoduleStrategyRecursiveAndGitSubmoduleDepth(t *testing.T } func TestBuildWithGitFetchSubmoduleStrategyRecursive(t *testing.T) { + tests := map[string]struct { + cleanGitConfig *bool + expectFreshRepoMessage bool + }{ + "no git cleanup": { + // shell executor defaults to not clean up git configs + expectFreshRepoMessage: false, + }, + "git cleanup explicitly enabled": { + cleanGitConfig: &[]bool{true}[0], + expectFreshRepoMessage: true, + }, + "git cleanup explicitly disabled": { + cleanGitConfig: &[]bool{false}[0], + expectFreshRepoMessage: false, + }, + } + shellstest.OnEachShell(t, func(t *testing.T, shell string) { - successfulBuild, err := common.GetSuccessfulBuild() - assert.NoError(t, err) - build := newBuild(t, successfulBuild, shell) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + successfulBuild, err := common.GetSuccessfulBuild() + assert.NoError(t, err) + build := newBuild(t, successfulBuild, shell) - build.Variables = append( - build.Variables, - common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"}, - common.JobVariable{Key: "GIT_SUBMODULE_STRATEGY", Value: "recursive"}, - ) + build.Variables = append( + build.Variables, + common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"}, + common.JobVariable{Key: "GIT_SUBMODULE_STRATEGY", Value: "recursive"}, + ) + build.Runner.RunnerSettings.CleanGitConfig = test.cleanGitConfig - out, err := buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Skipping Git submodules setup") - assert.NotContains(t, out, "Updating/initializing submodules...") - assert.Contains(t, out, "Updating/initializing submodules recursively...") + out, err := buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + assert.NotContains(t, out, "Skipping Git submodules setup") + assert.NotContains(t, out, "Updating/initializing submodules...") + assert.Contains(t, out, "Updating/initializing submodules recursively...") - _, err = os.Stat(filepath.Join(build.BuildDir, "gitlab-grack", ".git")) - assert.NoError(t, err, "Submodule should have been initialized") + _, err = os.Stat(filepath.Join(build.BuildDir, "gitlab-grack", ".git")) + assert.NoError(t, err, "Submodule should have been initialized") - _, err = os.Stat(filepath.Join(build.BuildDir, "gitlab-grack", "tests", "example", ".git")) - assert.NoError(t, err, "The submodule's submodule should have been initialized") + _, err = os.Stat(filepath.Join(build.BuildDir, "gitlab-grack", "tests", "example", ".git")) + assert.NoError(t, err, "The submodule's submodule should have been initialized") - // Create a file not tracked that should be cleaned in submodule. - excludedFilePath := filepath.Join(build.BuildDir, "gitlab-grack", "excluded_file") - err = os.WriteFile(excludedFilePath, []byte{}, os.ModePerm) - require.NoError(t, err) + // Create a file not tracked that should be cleaned in submodule. + excludedFilePath := filepath.Join(build.BuildDir, "gitlab-grack", "excluded_file") + err = os.WriteFile(excludedFilePath, []byte{}, os.ModePerm) + require.NoError(t, err) - // Run second build, to run fetch. - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Contains(t, out, "Removing excluded_file") + // Run second build, to run fetch. + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + + checkFreshRepoMessage := assert.NotContains + if test.expectFreshRepoMessage { + checkFreshRepoMessage = assert.Contains + } + checkFreshRepoMessage(t, out, "Created fresh repository") + + assert.Contains(t, out, "Removing excluded_file") + }) + } }) } @@ -1605,45 +1634,73 @@ func TestBuildChangesBranchesWhenFetchingRepo(t *testing.T) { } func TestBuildPowerShellCatchesExceptions(t *testing.T) { + tests := map[string]struct { + cleanGitConfig *bool + expectFreshRepoMessage bool + }{ + "no git cleanup": { + // shell executor defaults to not clean up git configs + expectFreshRepoMessage: false, + }, + "git cleanup explicitly enabled": { + cleanGitConfig: &[]bool{true}[0], + expectFreshRepoMessage: true, + }, + "git cleanup explicitly disabled": { + cleanGitConfig: &[]bool{false}[0], + expectFreshRepoMessage: false, + }, + } + for _, shell := range []string{"powershell", "pwsh"} { t.Run(shell, func(t *testing.T) { - helpers.SkipIntegrationTests(t, shell) + for name, test := range tests { + t.Run(name, func(t *testing.T) { + helpers.SkipIntegrationTests(t, shell) - successfulBuild, err := common.GetRemoteSuccessfulBuild() - assert.NoError(t, err) - build := newBuild(t, successfulBuild, shell) - build.Variables = append( - build.Variables, - common.JobVariable{Key: "ErrorActionPreference", Value: "Stop"}, - common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"}, - ) + successfulBuild, err := common.GetRemoteSuccessfulBuild() + assert.NoError(t, err) + build := newBuild(t, successfulBuild, shell) + build.Variables = append( + build.Variables, + common.JobVariable{Key: "ErrorActionPreference", Value: "Stop"}, + common.JobVariable{Key: "GIT_STRATEGY", Value: "fetch"}, + ) + build.Runner.RunnerSettings.CleanGitConfig = test.cleanGitConfig - out, err := buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.Contains(t, out, "Created fresh repository") + checkFreshRepoMessage := assert.NotContains + if test.expectFreshRepoMessage { + checkFreshRepoMessage = assert.Contains + } - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Regexp(t, "Checking out [a-f0-9]+ as", out) - - build.Variables = append( - build.Variables, - common.JobVariable{Key: "ErrorActionPreference", Value: "Continue"}, - ) - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Regexp(t, "Checking out [a-f0-9]+ as", out) - - build.Variables = append( - build.Variables, - common.JobVariable{Key: "ErrorActionPreference", Value: "SilentlyContinue"}, - ) - out, err = buildtest.RunBuildReturningOutput(t, build) - assert.NoError(t, err) - assert.NotContains(t, out, "Created fresh repository") - assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + out, err := buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + assert.Contains(t, out, "Created fresh repository") + + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + checkFreshRepoMessage(t, out, "Created fresh repository") + assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + + build.Variables = append( + build.Variables, + common.JobVariable{Key: "ErrorActionPreference", Value: "Continue"}, + ) + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + checkFreshRepoMessage(t, out, "Created fresh repository") + assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + + build.Variables = append( + build.Variables, + common.JobVariable{Key: "ErrorActionPreference", Value: "SilentlyContinue"}, + ) + out, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + checkFreshRepoMessage(t, out, "Created fresh repository") + assert.Regexp(t, "Checking out [a-f0-9]+ as", out) + }) + } }) } } @@ -1781,6 +1838,7 @@ func TestSanitizeGitDirectory(t *testing.T) { jobResponse, err := common.GetLocalBuildResponse( "git remote set-url origin /tmp/some/invalid/directory", ) + require.NoError(t, err, "getting job response") build := newBuild(t, jobResponse, shell) @@ -1790,6 +1848,8 @@ func TestSanitizeGitDirectory(t *testing.T) { common.JobVariable{Key: featureflags.EnableJobCleanup, Value: "true"}, ) + build.Runner.RunnerSettings.CleanGitConfig = &[]bool{true}[0] + err = buildtest.RunBuild(t, build) require.NoError(t, err) @@ -2343,6 +2403,57 @@ func TestSubmoduleAutoBump(t *testing.T) { } } +func TestBuildWithCleanGitConfig(t *testing.T) { + // only update a couple of submodules, to make the test a bit faster + submodules := []string{"private-repo-ssh", "public-repo-relative"} + require.GreaterOrEqual(t, len(submodules), 1, "must manage/update at least one submodule") + + assertFilesAreCleaned := func(t *testing.T, buildDir string) { + dirs := []string{ + filepath.Join(buildDir, ".git"), + filepath.Join(buildDir, "..", "mixed-submodules-test.tmp", "git-template"), + } + for _, m := range submodules { + dirs = append(dirs, filepath.Join(buildDir, ".git", "modules", m)) + } + for _, d := range dirs { + assert.DirExists(t, d) + assert.NoFileExists(t, filepath.Join(d, "config")) + assert.NoDirExists(t, filepath.Join(d, "hooks")) + } + } + + shellstest.OnEachShell(t, func(t *testing.T, shell string) { + t.Parallel() + + jobResponse, err := common.GetSuccessfulBuild() + assert.NoError(t, err) + + jobResponse.Variables = append(jobResponse.Variables, + common.JobVariable{Key: "GIT_SUBMODULE_PATHS", Value: strings.Join(submodules, " ")}, + common.JobVariable{Key: "GIT_SUBMODULE_STRATEGY", Value: string(common.SubmoduleRecursive)}, + common.JobVariable{Key: "GIT_SUBMODULE_FORCE_HTTPS", Value: "1"}, + common.JobVariable{Key: "CI_SERVER_HOST", Value: "gitlab.com"}, + ) + jobResponse.GitInfo.RepoURL = repoURLWithSubmodules + jobResponse.GitInfo.Sha = repoShaWithSubmodules + injectJobToken(t, &jobResponse) + + build := newBuild(t, jobResponse, shell) + build.Runner.RunnerCredentials.URL = "https://gitlab.com/" + build.Runner.RunnerSettings.CleanGitConfig = &[]bool{true}[0] + + _, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + assertFilesAreCleaned(t, build.BuildDir) + + // run a second build to ensure submodules still work, even though we blew away their git config. + _, err = buildtest.RunBuildReturningOutput(t, build) + assert.NoError(t, err) + assertFilesAreCleaned(t, build.BuildDir) + }) +} + // injectJobToken injects a job token into an existing jobResponse by // - setting the jobResponse's token // - updating the jobResponse's gitInfo with an URL with the token diff --git a/shells/abstract.go b/shells/abstract.go index dfe5193bf8b746722d6d756593746578a441792e..9a9cc9cb28131ae97a8be1846ca5f130dd764577 100644 --- a/shells/abstract.go +++ b/shells/abstract.go @@ -19,22 +19,23 @@ import ( url_helpers "gitlab.com/gitlab-org/gitlab-runner/helpers/url" ) -// When umask is disabled for the Kubernetes executor, -// a hidden file, .giltab-build-uid-gid, is created in the `builds_dir` directory to assist the helper container -// in retrieving the build image's configured `uid:gid`. -// This information is then applied to the working directories to prevent them from being writable by anyone. -const BuildUiGidFile = ".giltab-build-uid-gid" - -const StartupProbeFile = ".gitlab-startup-marker" - -const credHelperConfFile = "cred-helper.conf" +const ( + // When umask is disabled for the Kubernetes executor, + // a hidden file, .giltab-build-uid-gid, is created in the `builds_dir` directory to assist the helper container + // in retrieving the build image's configured `uid:gid`. + // This information is then applied to the working directories to prevent them from being writable by anyone. + BuildUiGidFile = ".giltab-build-uid-gid" + StartupProbeFile = ".gitlab-startup-marker" + + gitlabEnvFileName = "gitlab_runner_env" + gitlabCacheEnvFileName = "gitlab_runner_cache_env" + credHelperConfFile = "cred-helper.conf" + gitDir = ".git" + gitTemplateDir = "git-template" +) var errUnknownGitStrategy = errors.New("unknown GIT_STRATEGY") -const gitlabEnvFileName = "gitlab_runner_env" - -const gitlabCacheEnvFileName = "gitlab_runner_cache_env" - type stringQuoter func(string) string func singleQuote(s string) string { @@ -583,10 +584,12 @@ func (b *AbstractShell) writeRefspecFetchCmd(w ShellWriter, info common.ShellScr } // initializing - templateDir := w.MkTmpDir("git-template") + templateDir := w.MkTmpDir(gitTemplateDir) templateFile := w.Join(templateDir, "config") objectFormat := build.GetRepositoryObjectFormat() + b.writeGitCleanup(w, build) + if build.SafeDirectoryCheckout { // Solves problem with newer Git versions when files existing in the working directory // are owned by different system owners. This may happen for example with Docker executor, @@ -607,8 +610,6 @@ func (b *AbstractShell) writeRefspecFetchCmd(w ShellWriter, info common.ShellScr b.writeGitSSLConfig(w, build, []string{"-f", templateFile}) } - b.writeGitCleanup(w, build.GetSubmoduleStrategy(), projectDir) - if objectFormat != common.DefaultObjectFormat { w.Command("git", "init", projectDir, "--template", templateDir, "--object-format", objectFormat) } else { @@ -663,6 +664,7 @@ func (b *AbstractShell) writeRefspecFetchCmd(w ShellWriter, info common.ShellScr } func (b *AbstractShell) configureGitCredHelper(w ShellWriter, info common.ShellScriptInfo, credConfigFile string) error { + w.RmFile(credConfigFile) build := info.Build shellName := build.Runner.Shell if shellName == "" { @@ -706,8 +708,10 @@ func (b *AbstractShell) configureGitCredHelper(w ShellWriter, info common.ShellS return nil } -func (b *AbstractShell) writeGitCleanup(w ShellWriter, submoduleStrategy common.SubmoduleStrategy, projectDir string) { - const gitDir = ".git" +func (b *AbstractShell) writeGitCleanup(w ShellWriter, build *common.Build) { + projectDir := build.FullProjectDir() + submoduleStrategy := build.GetSubmoduleStrategy() + cleanForSubmodules := submoduleStrategy == common.SubmoduleNormal || submoduleStrategy == common.SubmoduleRecursive // Remove .git/{index,shallow,HEAD,config}.lock files from .git, which can fail the fetch command // The file can be left if previous build was terminated during git operation. @@ -723,12 +727,46 @@ func (b *AbstractShell) writeGitCleanup(w ShellWriter, submoduleStrategy common. for _, f := range files { w.RmFile(path.Join(projectDir, gitDir, f)) - if submoduleStrategy == common.SubmoduleNormal || submoduleStrategy == common.SubmoduleRecursive { + if cleanForSubmodules { w.RmFilesRecursive(path.Join(projectDir, gitDir, "modules"), path.Base(f)) } } w.RmFilesRecursive(path.Join(projectDir, gitDir, "refs"), "*.lock") + + b.writeGitCleanupAllConfigs(w, build, cleanForSubmodules) +} + +// writeGitCleanupAllConfigs removes all git configs which are potentially open to malicious code injection: +// - the main git config & hooks +// - the template git config & hooks +// - any submodule's git config & hooks +// It's by default disabled for the shell executor or when the git strategy is "none", and enabled otherwise; explicit +// configuration however always has precedence. +func (b *AbstractShell) writeGitCleanupAllConfigs(sw ShellWriter, build *common.Build, cleanForSubmodules bool) { + executor := build.Runner.Executor + shouldCleanUp := (executor != "shell" && executor != "shell-integration-test" && build.GetGitStrategy() != common.GitNone) + if config := build.Runner.CleanGitConfig; config != nil { + shouldCleanUp = *config + } + if !shouldCleanUp { + return + } + + projectDir := build.FullProjectDir() + + // clean out configs in the main git dir and in the template dir + for _, dir := range []string{sw.TmpFile(gitTemplateDir), sw.Join(projectDir, gitDir)} { + sw.RmFile(sw.Join(dir, "config")) + sw.RmDir(sw.Join(dir, "hooks")) + } + + // clean out configs in the modules' git dirs + if cleanForSubmodules { + modulesDir := sw.Join(projectDir, gitDir, "modules") + sw.RmFilesRecursive(modulesDir, "config") + sw.RmDirsRecursive(modulesDir, "hooks") + } } func (b *AbstractShell) writeCheckoutCmd(w ShellWriter, build *common.Build) { @@ -1352,10 +1390,10 @@ func (b *AbstractShell) writeCleanupScript(_ context.Context, w ShellWriter, inf if err := b.writeCleanupBuildDirectoryScript(w, info); err != nil { return err } - - w.RmFile(filepath.Join(info.Build.FullProjectDir(), ".git", "config")) } + b.writeGitCleanup(w, info.Build) + return nil } diff --git a/shells/abstract_test.go b/shells/abstract_test.go index 74456e96e96d66ed88626be6f4feb92bae1375bd..d04d0048298f7908b674ae7b7ab229fe721de965 100644 --- a/shells/abstract_test.go +++ b/shells/abstract_test.go @@ -7,6 +7,7 @@ import ( "fmt" "path" "path/filepath" + "regexp" "slices" "strings" "testing" @@ -845,7 +846,8 @@ func TestGitFetchFlags(t *testing.T) { mockWriter.EXPECT().Command("git", "config", "-f", mock.Anything, "credential.interactive", "never").Once() mockWriter.EXPECT().Command("git", "config", "-f", mock.Anything, "transfer.bundleURI", "true").Once() - mockWriter.EXPECT().RmFilesRecursive(path.Join(dummyProjectDir, ".git", "refs"), "*.lock").Once() + expectFileCleanup(mockWriter, path.Join(dummyProjectDir, ".git"), false) + expectGitConfigCleanup(mockWriter, dummyProjectDir, false) if expectedObjectFormat != "sha1" { mockWriter.EXPECT().Command("git", "init", dummyProjectDir, "--template", mock.Anything, "--object-format", expectedObjectFormat).Once() @@ -890,6 +892,26 @@ func TestGitFetchFlags(t *testing.T) { } } +func expectGitConfigCleanup(sw *MockShellWriter, buildDir string, withSubmodules bool) { + sw.EXPECT().TmpFile("git-template").Return("someGitTemplateDir").Once() + sw.EXPECT().Join(buildDir, ".git").Return("someGitDir").Once() + + sw.EXPECT().Join("someGitTemplateDir", "config").Return("someGitTemplateDir/config").Once() + sw.EXPECT().RmFile("someGitTemplateDir/config") + sw.EXPECT().Join("someGitTemplateDir", "hooks").Return("someGitTemplateDir/hooks").Once() + sw.EXPECT().RmDir("someGitTemplateDir/hooks") + sw.EXPECT().Join("someGitDir", "config").Return("someGitDir/config").Once() + sw.EXPECT().RmFile("someGitDir/config") + sw.EXPECT().Join("someGitDir", "hooks").Return("someGitDir/hooks").Once() + sw.EXPECT().RmDir("someGitDir/hooks") + + if withSubmodules { + sw.EXPECT().Join(buildDir, ".git", "modules").Return("someModulesDir").Once() + sw.EXPECT().RmFilesRecursive("someModulesDir", "config") + sw.EXPECT().RmDirsRecursive("someModulesDir", "hooks") + } +} + func TestAbstractShell_writeSubmoduleUpdateCmd(t *testing.T) { const ( exampleBaseURL = "http://test.remote" @@ -2273,7 +2295,7 @@ func TestSkipBuildStage(t *testing.T) { } } -func TestAbstractShell_writeCleanupFileVariablesScript(t *testing.T) { +func TestAbstractShell_writeCleanupScript(t *testing.T) { testVar1 := "VAR_1" testVar2 := "VAR_2" testVar3 := "VAR_3" @@ -2282,38 +2304,98 @@ func TestAbstractShell_writeCleanupFileVariablesScript(t *testing.T) { testPath1 := "path/VAR_1_file" testPath3 := "path/VAR_3_file" - info := common.ShellScriptInfo{ - Build: &common.Build{ - JobResponse: common.JobResponse{ - Variables: common.JobVariables{ - {Key: testVar1, Value: "test", File: true}, - {Key: testVar2, Value: "test", File: false}, - {Key: testVar3, Value: "test", File: true}, - {Key: testVar4, Value: "test", File: false}, - }, + someTrue, someFalse := true, false + type executorName = string + + tests := map[executorName]map[string]struct { + cleanGitConfig *bool + gitStrategy string + shouldCleanGitConfig bool + }{ + "shell": { + "no clean-git-config set": { + shouldCleanGitConfig: false, + }, + "clean-git-config explicitly enabled": { + cleanGitConfig: &someTrue, + shouldCleanGitConfig: true, + }, + "clean-git-config explicitly disabled": { + cleanGitConfig: &someFalse, + shouldCleanGitConfig: false, + }, + }, + "not-shell": { + "no clean-git-config set": { + shouldCleanGitConfig: true, + }, + "no clean-git-config set, but git strategy is none": { + shouldCleanGitConfig: false, + gitStrategy: "none", + }, + "clean-git-config explicitly enabled": { + cleanGitConfig: &someTrue, + gitStrategy: "none", + shouldCleanGitConfig: true, + }, + "clean-git-config explicitly disabled": { + cleanGitConfig: &someFalse, + shouldCleanGitConfig: false, }, - Runner: &common.RunnerConfig{}, }, } - mockShellWriter := &MockShellWriter{} - defer mockShellWriter.AssertExpectations(t) + for executorName, testCases := range tests { + t.Run("executor:"+executorName, func(t *testing.T) { + for name, test := range testCases { + t.Run(name, func(t *testing.T) { + info := common.ShellScriptInfo{ + Build: &common.Build{ + JobResponse: common.JobResponse{ + Variables: common.JobVariables{ + {Key: testVar1, Value: "test", File: true}, + {Key: testVar2, Value: "test", File: false}, + {Key: testVar3, Value: "test", File: true}, + {Key: testVar4, Value: "test", File: false}, + }, + }, + Runner: &common.RunnerConfig{ + RunnerSettings: common.RunnerSettings{ + CleanGitConfig: test.cleanGitConfig, + Executor: executorName, + Environment: []string{"GIT_STRATEGY=" + test.gitStrategy}, + }, + }, + }, + } + + mockShellWriter := NewMockShellWriter(t) - mockShellWriter.On("TmpFile", "masking.db").Return("masking.db").Once() - mockShellWriter.On("RmFile", "masking.db").Once() + mockShellWriter.On("TmpFile", "masking.db").Return("masking.db").Once() + mockShellWriter.On("RmFile", "masking.db").Once() - mockShellWriter.On("TmpFile", testVar1).Return(testPath1).Once() - mockShellWriter.On("RmFile", testPath1).Once() - mockShellWriter.On("TmpFile", testVar3).Return(testPath3).Once() - mockShellWriter.On("RmFile", testPath3).Once() + mockShellWriter.On("TmpFile", testVar1).Return(testPath1).Once() + mockShellWriter.On("RmFile", testPath1).Once() + mockShellWriter.On("TmpFile", testVar3).Return(testPath3).Once() + mockShellWriter.On("RmFile", testPath3).Once() - mockShellWriter.On("TmpFile", "gitlab_runner_env").Return("temp_env").Once() - mockShellWriter.On("RmFile", "temp_env").Once() + mockShellWriter.On("TmpFile", "gitlab_runner_env").Return("temp_env").Once() + mockShellWriter.On("RmFile", "temp_env").Once() - shell := new(AbstractShell) + expectFileCleanup(mockShellWriter, ".git", false) - err := shell.writeCleanupScript(context.Background(), mockShellWriter, info) - assert.NoError(t, err) + if test.shouldCleanGitConfig { + expectGitConfigCleanup(mockShellWriter, "", false) + } + + shell := new(AbstractShell) + + err := shell.writeCleanupScript(context.Background(), mockShellWriter, info) + assert.NoError(t, err) + }) + } + }) + } } func testGenerateArtifactsMetadataData() (common.ShellScriptInfo, []interface{}) { @@ -2638,8 +2720,10 @@ func TestAbstractShell_writeGetSourcesScript_scriptHooks(t *testing.T) { m.EXPECT().MkTmpDir("git-template").Return("git-template-dir").Once() m.EXPECT().Join("git-template-dir", "config").Return("git-template-dir-config").Once() m.EXPECT().Command("git", "config", "-f", "git-template-dir-config", mock.Anything, mock.Anything) - m.EXPECT().RmFile(mock.Anything) - m.EXPECT().RmFilesRecursive(path.Join("build-dir", ".git", "refs"), "*.lock").Once() + + expectFileCleanup(m, "build-dir/.git", false) + expectGitConfigCleanup(m, "build-dir", false) + m.EXPECT().Command("git", "init", "build-dir", "--template", "git-template-dir").Once() m.EXPECT().Cd("build-dir").Once() @@ -2681,16 +2765,35 @@ func TestAbstractShell_writeGetSourcesScript_scriptHooks(t *testing.T) { } } +func expectFileCleanup(shellWriter *MockShellWriter, dir string, withSubmodules bool) { + files := []string{"index.lock", "shallow.lock", "HEAD.lock", "hooks/post-checkout", "config.lock"} + + for _, f := range files { + shellWriter.EXPECT().RmFile(dir + "/" + f).Once() + } + + if withSubmodules { + for _, f := range files { + shellWriter.EXPECT().RmFilesRecursive(dir+"/modules", filepath.Base(f)).Once() + } + } + + shellWriter.EXPECT().RmFilesRecursive(dir+"/refs", "*.lock").Once() +} + func expectGitCredHelperSetup(shellWriter *MockShellWriter, remoteURL string) { + expectedCredHelperPath := "/some/path/cred-helper.conf" expectedCredSection := "credential." + remoteURL + shellWriter.EXPECT().RmFile(expectedCredHelperPath).Once() + shellWriter.EXPECT().TmpFile(mock.Anything).Return("/some/path/cred-helper.conf").Once() shellWriter.EXPECT().CommandWithStdin("url="+remoteURL, "git", "credential", "reject").Once() - shellWriter.EXPECT().Command("git", "config", "-f", "/some/path/cred-helper.conf", expectedCredSection+".username", mock.AnythingOfType("string")).Once() - shellWriter.EXPECT().Command("git", "config", "-f", "/some/path/cred-helper.conf", expectedCredSection+".helper", mock.MatchedBy(startWithBang)).Once() - shellWriter.EXPECT().Command("git", "config", "include.path", "/some/path/cred-helper.conf") + shellWriter.EXPECT().Command("git", "config", "-f", expectedCredHelperPath, expectedCredSection+".username", mock.AnythingOfType("string")).Once() + shellWriter.EXPECT().Command("git", "config", "-f", expectedCredHelperPath, expectedCredSection+".helper", mock.MatchedBy(startWithBang)).Once() + shellWriter.EXPECT().Command("git", "config", "include.path", expectedCredHelperPath).Once() } func startWithBang(val string) bool { @@ -2738,3 +2841,116 @@ func TestSanitizeCacheFallbackKey(t *testing.T) { }) } } + +func TestAbstractShell_writeGitCleanup(t *testing.T) { + submoduleStrategies := map[common.SubmoduleStrategy]bool{ + common.SubmoduleNone: false, + common.SubmoduleNormal: true, + common.SubmoduleRecursive: true, + common.SubmoduleInvalid: false, + } + cleanGitConfigs := map[string]struct { + configValue *bool + expectGitConfigsToBeCleaned bool + }{ + "": { + expectGitConfigsToBeCleaned: true, + }, + "enabled": { + configValue: &[]bool{true}[0], + expectGitConfigsToBeCleaned: true, + }, + "disabled": { + configValue: &[]bool{false}[0], + expectGitConfigsToBeCleaned: false, + }, + } + + for name, cleanGitConfig := range cleanGitConfigs { + t.Run("cleanGitConfig:"+name, func(t *testing.T) { + for submoduleStrategy, expectSubmoduleCleanupCalls := range submoduleStrategies { + t.Run("submoduleStrategy:"+string(submoduleStrategy), func(t *testing.T) { + shell := new(AbstractShell) + + info := common.ShellScriptInfo{ + Build: &common.Build{ + JobResponse: common.JobResponse{ + Variables: common.JobVariables{ + {Key: "GIT_SUBMODULE_STRATEGY", Value: string(submoduleStrategy)}, + }, + GitInfo: common.GitInfo{ + RepoURL: "https://repo-url/some/repo", + }, + }, + Runner: &common.RunnerConfig{ + RunnerSettings: common.RunnerSettings{ + CleanGitConfig: cleanGitConfig.configValue, + }, + }, + }, + } + + // ensure the cleanup is called at the beginning, from writeRefspecFetchCmd + t.Run("writeRefspecFetchCmd", func(t *testing.T) { + sw := NewMockShellWriter(t) + + sw.EXPECT().Noticef("Fetching changes...").Once() + + expectFileCleanup(sw, ".git", expectSubmoduleCleanupCalls) + if cleanGitConfig.expectGitConfigsToBeCleaned { + expectGitConfigCleanup(sw, "", expectSubmoduleCleanupCalls) + } + + sw.EXPECT().MkTmpDir("git-template").Return("someTmpDir").Once() + sw.EXPECT().Join("someTmpDir", "config").Return("someGitTemplateConfig").Once() + + sw.EXPECT().Command("git", "config", "-f", "someGitTemplateConfig", "init.defaultBranch", "none").Once() + sw.EXPECT().Command("git", "config", "-f", "someGitTemplateConfig", "fetch.recurseSubmodules", "false").Once() + sw.EXPECT().Command("git", "config", "-f", "someGitTemplateConfig", "credential.interactive", "never").Once() + sw.EXPECT().Command("git", "config", "-f", "someGitTemplateConfig", "transfer.bundleURI", "true").Once() + + sw.EXPECT().Command("git", "init", "", "--template", "someTmpDir").Once() + sw.EXPECT().Cd("").Once() + + sw.EXPECT().IfCmd("git", "remote", "add", "origin", "https://repo-url/some/repo") + sw.EXPECT().Noticef("Created fresh repository.").Once() + sw.EXPECT().Else().Once() + sw.EXPECT().Command("git", "remote", "set-url", "origin", "https://repo-url/some/repo").Once() + sw.EXPECT().EndIf().Once() + + uaConfRE := regexp.MustCompile(`^http.userAgent=gitlab-runner development version [a-zA-Z0-9]+\/[a-zA-Z0-9]+$`) + uaConfMatcher := mock.MatchedBy(uaConfRE.MatchString) + + sw.EXPECT().IfFile(".git/shallow").Once() + sw.EXPECT().Command("git", "-c", uaConfMatcher, "fetch", "origin", "--no-recurse-submodules", "--prune", "--quiet", "--unshallow").Once() + sw.EXPECT().Else().Once() + sw.EXPECT().Command("git", "-c", uaConfMatcher, "fetch", "origin", "--no-recurse-submodules", "--prune", "--quiet").Once() + sw.EXPECT().EndIf().Once() + + err := shell.writeRefspecFetchCmd(sw, info) + assert.NoError(t, err) + }) + + // ensure the cleanup is also called at the end, from writeCleanupScript + t.Run("writeCleanupScript", func(t *testing.T) { + sw := NewMockShellWriter(t) + + sw.EXPECT().TmpFile("masking.db").Return("masking.db").Once() + sw.EXPECT().RmFile("masking.db").Once() + + sw.EXPECT().TmpFile("gitlab_runner_env").Return("someRunnerEnv").Once() + sw.EXPECT().RmFile("someRunnerEnv").Once() + + expectFileCleanup(sw, ".git", expectSubmoduleCleanupCalls) + if cleanGitConfig.expectGitConfigsToBeCleaned { + expectGitConfigCleanup(sw, "", expectSubmoduleCleanupCalls) + } + + err := shell.writeCleanupScript(context.TODO(), sw, info) + assert.NoError(t, err) + }) + }) + } + }) + } +} diff --git a/shells/bash.go b/shells/bash.go index cacedb5e78f30f6befb2dcab813985e1c4b13693..afaa2af551ffc17d32269104ef2f62d5df70c21f 100644 --- a/shells/bash.go +++ b/shells/bash.go @@ -292,6 +292,13 @@ func (b *BashWriter) RmFilesRecursive(path string, name string) { b.EndIf() } +func (b *BashWriter) RmDirsRecursive(path string, name string) { + b.IfDirectory(path) + // `find -delete` is not portable; https://unix.stackexchange.com/a/194348 + b.Linef("find %q -name %q -type d -exec rm -rf -- {} +", path, name) + b.EndIf() +} + func (b *BashWriter) Absolute(dir string) string { if path.IsAbs(dir) || strings.HasPrefix(dir, "$PWD") { return dir diff --git a/shells/mock_ShellWriter.go b/shells/mock_ShellWriter.go index 4d88c9586f06c2fd198cb5d68f611c522e345aea..9bffe4acba0714a852a622c1fe87f227e443eab5 100644 --- a/shells/mock_ShellWriter.go +++ b/shells/mock_ShellWriter.go @@ -1009,6 +1009,40 @@ func (_c *MockShellWriter_RmDir_Call) RunAndReturn(run func(string)) *MockShellW return _c } +// RmDirsRecursive provides a mock function with given fields: path, name +func (_m *MockShellWriter) RmDirsRecursive(path string, name string) { + _m.Called(path, name) +} + +// MockShellWriter_RmDirsRecursive_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RmDirsRecursive' +type MockShellWriter_RmDirsRecursive_Call struct { + *mock.Call +} + +// RmDirsRecursive is a helper method to define mock.On call +// - path string +// - name string +func (_e *MockShellWriter_Expecter) RmDirsRecursive(path interface{}, name interface{}) *MockShellWriter_RmDirsRecursive_Call { + return &MockShellWriter_RmDirsRecursive_Call{Call: _e.mock.On("RmDirsRecursive", path, name)} +} + +func (_c *MockShellWriter_RmDirsRecursive_Call) Run(run func(path string, name string)) *MockShellWriter_RmDirsRecursive_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(string), args[1].(string)) + }) + return _c +} + +func (_c *MockShellWriter_RmDirsRecursive_Call) Return() *MockShellWriter_RmDirsRecursive_Call { + _c.Call.Return() + return _c +} + +func (_c *MockShellWriter_RmDirsRecursive_Call) RunAndReturn(run func(string, string)) *MockShellWriter_RmDirsRecursive_Call { + _c.Call.Return(run) + return _c +} + // RmFile provides a mock function with given fields: path func (_m *MockShellWriter) RmFile(path string) { _m.Called(path) diff --git a/shells/powershell.go b/shells/powershell.go index e45dd87003ea7885d736aeef3aed6641c5da2a47..b26ed718ee0a59e5494e35fedbba469335978af9 100644 --- a/shells/powershell.go +++ b/shells/powershell.go @@ -520,6 +520,18 @@ func (p *PsWriter) RmFilesRecursive(path string, name string) { p.EndIf() } +func (p *PsWriter) RmDirsRecursive(path string, name string) { + resolvedPath := p.resolvePath(path) + p.IfDirectory(path) + p.Linef( + // `Remove-Item -Recurse` has a known issue (see Example 4 in + // https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.management/remove-item) + "Get-ChildItem -Path %s -Filter %s -Recurse | ?{ $_.PSIsContainer } | ForEach-Object { Remove-Item -Recurse -Force $_.FullName }", + resolvedPath, psQuoteVariable(name), + ) + p.EndIf() +} + func (p *PsWriter) Printf(format string, arguments ...interface{}) { coloredText := helpers.ANSI_RESET + fmt.Sprintf(format, arguments...) p.Line("echo " + psQuoteVariable(coloredText)) diff --git a/shells/powershell_test.go b/shells/powershell_test.go index e9e53bd3468d5c39fe7700111021a08c61630b24..07cc60bc38dcb8b9e48ed11c2eb0d14831c30e51 100644 --- a/shells/powershell_test.go +++ b/shells/powershell_test.go @@ -464,27 +464,18 @@ func TestPowershellPathResolveOperations(t *testing.T) { } func TestPowershell_GenerateScript(t *testing.T) { - shellInfo := common.ShellScriptInfo{ - Shell: "pwsh", - Type: common.NormalShell, - RunnerCommand: "/usr/bin/gitlab-runner-helper", - Build: &common.Build{ - Runner: &common.RunnerConfig{RunnerSettings: common.RunnerSettings{ProxyExec: func() *bool { v := false; return &v }()}}, - }, - } - shellInfo.Build.Runner.Executor = "kubernetes" - shellInfo.Build.Hostname = "Test Hostname" - + pwshShell := common.GetShell("pwsh") eol := "\n" - switch v := common.GetShell("pwsh").(type) { + switch v := pwshShell.(type) { case *PowerShell: eol = v.EOL case *ProxyExecShell: eol = v.Shell.(*PowerShell).EOL } - shebang := "" - rmGitLabEnvScript := `$CurrentDirectory = (Resolve-Path ./).Path` + eol + + + rmGitLabEnvScript := `` + + `$CurrentDirectory = (Resolve-Path ./).Path` + eol + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path "$CurrentDirectory/.tmp/gitlab_runner_env" -PathType Leaf) ) {` + eol + ` Remove-Item2 -Force "$CurrentDirectory/.tmp/gitlab_runner_env"` + eol + `} elseif(Test-Path "$CurrentDirectory/.tmp/gitlab_runner_env") {` + eol + @@ -495,53 +486,167 @@ func TestPowershell_GenerateScript(t *testing.T) { ` Remove-Item2 -Force "$CurrentDirectory/.tmp/masking.db"` + eol + `} elseif(Test-Path "$CurrentDirectory/.tmp/masking.db") {` + eol + ` Remove-Item -Force "$CurrentDirectory/.tmp/masking.db"` + eol + - `}` + eol + eol + eol + "}" + eol + eol + `}` + cleanGitFiles := `` + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/index.lock" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force ".git/index.lock"` + eol + + `} elseif(Test-Path ".git/index.lock") {` + eol + + ` Remove-Item -Force ".git/index.lock"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/shallow.lock" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force ".git/shallow.lock"` + eol + + `} elseif(Test-Path ".git/shallow.lock") {` + eol + + ` Remove-Item -Force ".git/shallow.lock"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/HEAD.lock" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force ".git/HEAD.lock"` + eol + + `} elseif(Test-Path ".git/HEAD.lock") {` + eol + + ` Remove-Item -Force ".git/HEAD.lock"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/hooks/post-checkout" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force ".git/hooks/post-checkout"` + eol + + `} elseif(Test-Path ".git/hooks/post-checkout") {` + eol + + ` Remove-Item -Force ".git/hooks/post-checkout"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/config.lock" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force ".git/config.lock"` + eol + + `} elseif(Test-Path ".git/config.lock") {` + eol + + ` Remove-Item -Force ".git/config.lock"` + eol + + `}` + eol + + `` + eol + + `if(Test-Path ".git/refs" -PathType Container) {` + eol + + ` Get-ChildItem -Path ".git/refs" -Filter "*.lock" -Recurse | ForEach-Object { Remove-Item -Force $_.FullName }` + eol + + `}` + eol + + cleanGitConfigs := `` + + `$CurrentDirectory = (Resolve-Path ./).Path` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path "$CurrentDirectory/.tmp/git-template/config" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force "$CurrentDirectory/.tmp/git-template/config"` + eol + + `} elseif(Test-Path "$CurrentDirectory/.tmp/git-template/config") {` + eol + + ` Remove-Item -Force "$CurrentDirectory/.tmp/git-template/config"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path "$CurrentDirectory/.tmp/git-template/hooks" -PathType Container) ) {` + eol + + ` Remove-Item2 -Force -Recurse "$CurrentDirectory/.tmp/git-template/hooks"` + eol + + `} elseif(Test-Path "$CurrentDirectory/.tmp/git-template/hooks") {` + eol + + ` Remove-Item -Force -Recurse "$CurrentDirectory/.tmp/git-template/hooks"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/config" -PathType Leaf) ) {` + eol + + ` Remove-Item2 -Force ".git/config"` + eol + + `} elseif(Test-Path ".git/config") {` + eol + + ` Remove-Item -Force ".git/config"` + eol + + `}` + eol + + `` + eol + + `if( (Get-Command -Name Remove-Item2 -Module NTFSSecurity -ErrorAction SilentlyContinue) -and (Test-Path ".git/hooks" -PathType Container) ) {` + eol + + ` Remove-Item2 -Force -Recurse ".git/hooks"` + eol + + `} elseif(Test-Path ".git/hooks") {` + eol + + ` Remove-Item -Force -Recurse ".git/hooks"` + eol + + `}` if eol == "\n" { shebang = "#!/usr/bin/env pwsh\n" } else { rmGitLabEnvScript = strings.ReplaceAll(rmGitLabEnvScript, `/`, `\`) + cleanGitFiles = strings.ReplaceAll(cleanGitFiles, `/`, `\`) + cleanGitConfigs = strings.ReplaceAll(cleanGitConfigs, `/`, `\`) } testCases := map[string]struct { stage common.BuildStage - info common.ShellScriptInfo + updateShellInfo func(*common.ShellScriptInfo) expectedFailure bool - expectedScript string + expectedScript func(common.ShellScriptInfo) string }{ "prepare script": { stage: common.BuildStagePrepare, - info: shellInfo, expectedFailure: false, - expectedScript: shebang + "& {" + - eol + eol + - fmt.Sprintf(pwshJSONInitializationScript, shellInfo.Shell) + - eol + eol + - `$ErrorActionPreference = "Stop"` + eol + - `echo "Running on $([Environment]::MachineName) via "Test Hostname"..."` + eol + rmGitLabEnvScript, - }, - "cleanup variables": { + expectedScript: func(shellInfo common.ShellScriptInfo) string { + return shebang + "& {" + + eol + eol + + fmt.Sprintf(pwshJSONInitializationScript, shellInfo.Shell) + + eol + eol + + `$ErrorActionPreference = "Stop"` + eol + + `echo "Running on $([Environment]::MachineName) via "Test Hostname"..."` + + eol + + rmGitLabEnvScript + + eol + eol + eol + + "}" + eol + eol + }, + }, + "cleanup variables but not git config": { + stage: common.BuildStageCleanup, + updateShellInfo: func(shellInfo *common.ShellScriptInfo) { + shellInfo.Build.Runner.RunnerSettings.CleanGitConfig = &[]bool{false}[0] + }, + expectedFailure: false, + expectedScript: func(shellInfo common.ShellScriptInfo) string { + return shebang + "& {" + + eol + eol + + fmt.Sprintf(pwshJSONInitializationScript, shellInfo.Shell) + + eol + eol + + `$ErrorActionPreference = "Stop"` + eol + + rmGitLabEnvScript + + eol + eol + + cleanGitFiles + + eol + + "}" + eol + eol + }, + }, + "cleanup variables and git config": { stage: common.BuildStageCleanup, - info: shellInfo, expectedFailure: false, - expectedScript: shebang + "& {" + - eol + eol + - fmt.Sprintf(pwshJSONInitializationScript, shellInfo.Shell) + - eol + eol + - `$ErrorActionPreference = "Stop"` + eol + rmGitLabEnvScript, + expectedScript: func(shellInfo common.ShellScriptInfo) string { + return shebang + "& {" + + eol + eol + + fmt.Sprintf(pwshJSONInitializationScript, shellInfo.Shell) + + eol + eol + + `$ErrorActionPreference = "Stop"` + eol + + rmGitLabEnvScript + + eol + eol + + cleanGitFiles + + cleanGitConfigs + + eol + eol + eol + + "}" + eol + eol + }, }, "no script": { stage: "no_script", - info: shellInfo, expectedFailure: true, - expectedScript: "", }, } for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { - script, err := common.GetShell("pwsh").GenerateScript(context.Background(), tc.stage, tc.info) - assert.Equal(t, tc.expectedScript, script) + shellInfo := common.ShellScriptInfo{ + Shell: "pwsh", + Type: common.NormalShell, + RunnerCommand: "/usr/bin/gitlab-runner-helper", + Build: &common.Build{ + Runner: &common.RunnerConfig{ + RunnerSettings: common.RunnerSettings{ + Executor: "kubernetes", + }, + }, + Hostname: "Test Hostname", + }, + } + if u := tc.updateShellInfo; u != nil { + u(&shellInfo) + } + + var expectedScript string + if s := tc.expectedScript; s != nil { + expectedScript = s(shellInfo) + } + + script, err := pwshShell.GenerateScript(context.Background(), tc.stage, shellInfo) + assert.Equal(t, expectedScript, script) + if tc.expectedFailure { assert.Error(t, err) } diff --git a/shells/shell_writer.go b/shells/shell_writer.go index 39510ebd150de0ac2794ee023a328744090e1214..b81c1f4234522d6fec869ff5a70a1c05cbe30a5d 100644 --- a/shells/shell_writer.go +++ b/shells/shell_writer.go @@ -28,7 +28,12 @@ type ShellWriter interface { MkDir(path string) RmDir(path string) RmFile(path string) + + // RmFilesRecursive deletes all files in path with a basename of name RmFilesRecursive(path string, name string) + // RmDirsRecursive deletes all directories and their content in path with a basename of name + RmDirsRecursive(path string, name string) + Absolute(path string) string Join(elem ...string) string TmpFile(name string) string diff --git a/shells/shell_writer_integration_test.go b/shells/shell_writer_integration_test.go index 061fb4caea787722e49b924b68eba0e12bbc9e59..960921668f529c6583a0a012700a90aa7be83a2c 100644 --- a/shells/shell_writer_integration_test.go +++ b/shells/shell_writer_integration_test.go @@ -121,6 +121,31 @@ func TestRmFilesRecursive(t *testing.T) { }) } +func TestRmDirsRecursive(t *testing.T) { + testFiles := testFileTree{ + "some/dir2rm/even/nested/dir2rm/file": "should be deleted incl. ancestor dirs", + "dir2rm": "this is a file and should not be deleted", + "not/really_dir2rm": "", + "random/dir2rm": "", + } + + shellstest.OnEachShellWithWriter(t, func(t *testing.T, shell string, writer shells.ShellWriter) { + tmpDir := t.TempDir() + testFiles.Create(t, tmpDir) + + writer.RmDirsRecursive(tmpDir, "dir2rm") + + runShell(t, shell, tmpDir, writer) + + assert.DirExists(t, filepath.Join(tmpDir, "some")) + assert.NoDirExists(t, filepath.Join(tmpDir, "some/dir2rm")) + assert.FileExists(t, filepath.Join(tmpDir, "dir2rm")) + assert.DirExists(t, filepath.Join(tmpDir, "not/really_dir2rm")) + assert.DirExists(t, filepath.Join(tmpDir, "random")) + assert.NoDirExists(t, filepath.Join(tmpDir, "random/dir2rm")) + }) +} + func TestCommandArgumentExpansion(t *testing.T) { tmpDir, err := ioutil.TempDir("", "runner-test") require.NoError(t, err) @@ -226,3 +251,21 @@ password=some-pass assert.Equal(t, expectedOutput, output) }) } + +type testFileTree map[string]string + +func (tft testFileTree) Create(t *testing.T, baseDir string) { + for path, content := range tft { + if content == "" { + // on empty content, we don't create a file but a leaf directory + err := os.MkdirAll(filepath.Join(baseDir, path), 0750) + require.NoError(t, err) + continue + } + + err := os.MkdirAll(filepath.Join(baseDir, filepath.Dir(path)), 0750) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(baseDir, path), []byte(content), 0644) + require.NoError(t, err) + } +}