diff --git a/helper/DockerHelper.go b/helper/DockerHelper.go index 21630a19..c2988fc1 100644 --- a/helper/DockerHelper.go +++ b/helper/DockerHelper.go @@ -59,20 +59,17 @@ func StartDockerDaemon(dockerConnection, dockerRegistryUrl, dockerCert, defaultA if err != nil { log.Fatal(err) } - dockerdstart := "" - defaultAddressPoolFlag := "" - dockerMtuValueFlag := "" + dockerdStart := util.NewCommand("dockerd") if len(defaultAddressPoolBaseCidr) > 0 { if defaultAddressPoolSize <= 0 { defaultAddressPoolSize = 24 } - defaultAddressPoolFlag = fmt.Sprintf("--default-address-pool base=%s,size=%d", defaultAddressPoolBaseCidr, defaultAddressPoolSize) - } - if ciRunnerDockerMtuValue > 0 { - dockerMtuValueFlag = fmt.Sprintf("--mtu=%d", ciRunnerDockerMtuValue) + defaultAddressPoolFlag := fmt.Sprintf("base=%s,size=%d", defaultAddressPoolBaseCidr, defaultAddressPoolSize) + dockerdStart.AppendCommand("--default-address-pool", defaultAddressPoolFlag) } + if connection == util.INSECURE { - dockerdstart = fmt.Sprintf("dockerd %s --insecure-registry %s --host=unix:///var/run/docker.sock %s --host=tcp://0.0.0.0:2375 > /usr/local/bin/nohup.out 2>&1 &", defaultAddressPoolFlag, u.Host, dockerMtuValueFlag) + dockerdStart.AppendCommand("--insecure-registry", u.Host) util.LogStage("Insecure Registry") } else { if connection == util.SECUREWITHCERT { @@ -92,10 +89,17 @@ func StartDockerDaemon(dockerConnection, dockerRegistryUrl, dockerCert, defaultA } util.LogStage("Secure with Cert") } - dockerdstart = fmt.Sprintf("dockerd %s --host=unix:///var/run/docker.sock %s --host=tcp://0.0.0.0:2375 > /usr/local/bin/nohup.out 2>&1 &", defaultAddressPoolFlag, dockerMtuValueFlag) } - out, _ := exec.Command("/bin/sh", "-c", dockerdstart).Output() - log.Println(string(out)) + dockerdStart.AppendCommand("--host=unix:///var/run/docker.sock") + if ciRunnerDockerMtuValue > 0 { + dockerMtuValueFlag := fmt.Sprintf("--mtu=%d", ciRunnerDockerMtuValue) + dockerdStart.AppendCommand(dockerMtuValueFlag) + } + dockerdStart.AppendCommand("--host=tcp://0.0.0.0:2375", ">", "/usr/local/bin/nohup.out", "2>&1") + log.Println(util.DEVTRON, " ", dockerdStart.GetCommandToBeExecuted("-c")) + dockerdStartCmd := exec.Command("/bin/sh", dockerdStart.GetCommandToBeExecuted("-c")...) + err = dockerdStartCmd.Start() + log.Println(util.DEVTRON, "docker daemon ran in subprocess:", dockerdStartCmd.Process.Pid) waitForDockerDaemon(util.RETRYCOUNT) } @@ -171,8 +175,7 @@ func DockerLogin(dockerCredentials *DockerCredentials) error { pwd = pwd[:len(pwd)-1] } } - dockerLogin := fmt.Sprintf("docker login -u '%s' -p '%s' '%s' ", username, pwd, dockerCredentials.DockerRegistryURL) - awsLoginCmd := exec.Command("/bin/sh", "-c", dockerLogin) + awsLoginCmd := exec.Command("/bin/sh", "-c", "docker", "login", "-u", username, "-p", pwd, dockerCredentials.DockerRegistryURL) err := util.RunCommand(awsLoginCmd) if err != nil { log.Println(err) @@ -212,49 +215,48 @@ func BuildArtifact(ciRequest *CommonWorkflowRequest) (string, error) { return "", err } if ciBuildConfig.CiBuildType == SELF_DOCKERFILE_BUILD_TYPE || ciBuildConfig.CiBuildType == MANAGED_DOCKERFILE_BUILD_TYPE { - dockerBuild := "docker build " + dockerBuild := util.NewCommand("docker", "build") if ciRequest.CacheInvalidate && ciRequest.IsPvcMounted { - dockerBuild = dockerBuild + "--no-cache " + dockerBuild.AppendCommand("--no-cache") } dockerBuildConfig := ciBuildConfig.DockerBuildConfig isTargetPlatformSet := dockerBuildConfig.TargetPlatform != "" useBuildx := dockerBuildConfig.CheckForBuildX() - dockerBuildxBuild := "docker buildx build " + dockerBuildxBuild := util.NewCommand("docker", "buildx", "build") if useBuildx { + dockerBuild = dockerBuildxBuild if ciRequest.CacheInvalidate && ciRequest.IsPvcMounted { - dockerBuild = dockerBuildxBuild + "--no-cache " - } else { - dockerBuild = dockerBuildxBuild + " " + dockerBuild.AppendCommand("--no-cache") } if isTargetPlatformSet { - dockerBuild += "--platform " + dockerBuildConfig.TargetPlatform + " " + dockerBuild.AppendCommand("--platform", dockerBuildConfig.TargetPlatform) } } - dockerBuildFlags := make(map[string]string) dockerBuildArgsMap := dockerBuildConfig.Args for k, v := range dockerBuildArgsMap { - flagKey := fmt.Sprintf("%s %s", BUILD_ARG_FLAG, k) + dockerBuild.AppendCommand(BUILD_ARG_FLAG) if strings.HasPrefix(v, DEVTRON_ENV_VAR_PREFIX) { valueFromEnv := os.Getenv(strings.TrimPrefix(v, DEVTRON_ENV_VAR_PREFIX)) - dockerBuildFlags[flagKey] = fmt.Sprintf("=\"%s\"", valueFromEnv) + dockerBuildArg := fmt.Sprintf("%s=\"%s\"", strings.TrimSpace(k), strings.TrimSpace(valueFromEnv)) + dockerBuild.AppendCommand(dockerBuildArg) } else { - dockerBuildFlags[flagKey] = fmt.Sprintf("=%s", v) + dockerBuildArg := fmt.Sprintf("%s=%s", strings.TrimSpace(k), strings.TrimSpace(v)) + dockerBuild.AppendCommand(dockerBuildArg) } } dockerBuildOptionsMap := dockerBuildConfig.DockerBuildOptions for k, v := range dockerBuildOptionsMap { - flagKey := "--" + k + dockerBuildFlag := fmt.Sprintf("--%s", strings.TrimSpace(k)) if strings.HasPrefix(v, DEVTRON_ENV_VAR_PREFIX) { valueFromEnv := os.Getenv(strings.TrimPrefix(v, DEVTRON_ENV_VAR_PREFIX)) - dockerBuildFlags[flagKey] = fmt.Sprintf("=%s", valueFromEnv) + dockerBuildFlag += fmt.Sprintf("=%s", strings.TrimSpace(valueFromEnv)) } else { - dockerBuildFlags[flagKey] = fmt.Sprintf("=%s", v) + dockerBuildFlag += fmt.Sprintf("=%s", strings.TrimSpace(v)) } + dockerBuild.AppendCommand(dockerBuildFlag) } - for key, value := range dockerBuildFlags { - dockerBuild = dockerBuild + " " + key + value - } + if !ciRequest.EnableBuildContext || dockerBuildConfig.BuildContext == "" { dockerBuildConfig.BuildContext = ROOT_PATH } @@ -292,13 +294,12 @@ func BuildArtifact(ciRequest *CommonWorkflowRequest) (string, error) { } oldCacheBuildxPath = oldCacheBuildxPath + "/cache" } - - dockerBuild = getBuildxBuildCommand(cacheEnabled, dockerBuild, oldCacheBuildxPath, localCachePath, dest, dockerBuildConfig) + createBuildxBuildCommand(dockerBuild, cacheEnabled, oldCacheBuildxPath, localCachePath, dest, dockerBuildConfig) } else { - dockerBuild = fmt.Sprintf("%s -f %s --network host -t %s %s", dockerBuild, dockerBuildConfig.DockerfilePath, ciRequest.DockerRepository, dockerBuildConfig.BuildContext) + dockerBuild.AppendCommand("-f", dockerBuildConfig.DockerfilePath, "--network host", "-t", ciRequest.DockerRepository, dockerBuildConfig.BuildContext) } if envVars.ShowDockerBuildCmdInLogs { - log.Println("Starting docker build : ", dockerBuild) + log.Println("Starting docker build : ", dockerBuild.PrintCommand()) } else { log.Println("Docker build started..") } @@ -327,24 +328,23 @@ func BuildArtifact(ciRequest *CommonWorkflowRequest) (string, error) { projectPath = "./" + projectPath } handleLanguageVersion(projectPath, buildPackParams) - buildPackCmd := fmt.Sprintf("pack build %s --path %s --builder %s", dest, projectPath, buildPackParams.BuilderId) + buildPackCmd := util.NewCommand("pack", "build", dest, "--path", projectPath, "--builder", buildPackParams.BuilderId) BuildPackArgsMap := buildPackParams.Args for k, v := range BuildPackArgsMap { - buildPackCmd = buildPackCmd + " --env " + k + "=" + v + buildPackCmd.AppendCommand("--env", fmt.Sprintf("%s=%s", strings.TrimSpace(k), strings.TrimSpace(v))) } if len(buildPackParams.BuildPacks) > 0 { for _, buildPack := range buildPackParams.BuildPacks { - buildPackCmd = buildPackCmd + " --buildpack " + buildPack + buildPackCmd.AppendCommand("--buildpack", strings.TrimSpace(buildPack)) } } - log.Println(" -----> " + buildPackCmd) + log.Println(" -----> " + buildPackCmd.PrintCommand()) err = executeCmd(buildPackCmd) if err != nil { return "", err } - builderRmCmdString := "docker image rm " + buildPackParams.BuilderId - builderRmCmd := exec.Command("/bin/sh", "-c", builderRmCmdString) + builderRmCmd := exec.Command("/bin/sh", "-c", "docker", "image", "rm", buildPackParams.BuilderId) err := builderRmCmd.Run() if err != nil { return "", err @@ -354,18 +354,19 @@ func BuildArtifact(ciRequest *CommonWorkflowRequest) (string, error) { return dest, nil } -func getBuildxBuildCommand(cacheEnabled bool, dockerBuild, oldCacheBuildxPath, localCachePath, dest string, dockerBuildConfig *DockerBuildConfig) string { - dockerBuild = fmt.Sprintf("%s -f %s -t %s --push %s --network host --allow network.host --allow security.insecure", dockerBuild, dockerBuildConfig.DockerfilePath, dest, dockerBuildConfig.BuildContext) +func createBuildxBuildCommand(dockerBuild *util.CommandType, cacheEnabled bool, oldCacheBuildxPath, localCachePath, dest string, dockerBuildConfig *DockerBuildConfig) { + dockerBuild.AppendCommand("-f", dockerBuildConfig.DockerfilePath, "-t", dest, "--push", dockerBuildConfig.BuildContext, "--network", "host", "--allow network.host", "--allow security.insecure") if cacheEnabled { - dockerBuild = fmt.Sprintf("%s --cache-to=type=local,dest=%s,mode=max --cache-from=type=local,src=%s", dockerBuild, localCachePath, oldCacheBuildxPath) + cacheDest := fmt.Sprintf("--cache-to=type=local,dest=%s,mode=max", localCachePath) + cacheSrc := fmt.Sprintf("--cache-from=type=local,src=%s", oldCacheBuildxPath) + dockerBuild.AppendCommand(cacheDest, cacheSrc) } provenanceFlag := dockerBuildConfig.GetProvenanceFlag() - dockerBuild = fmt.Sprintf("%s %s", dockerBuild, provenanceFlag) + dockerBuild.AppendCommand(provenanceFlag) manifestLocation := util.LOCAL_BUILDX_LOCATION + "/manifest.json" - dockerBuild = fmt.Sprintf("%s --metadata-file %s", dockerBuild, manifestLocation) - - return dockerBuild + dockerBuild.AppendCommand("--metadata-file", manifestLocation) + return } func handleLanguageVersion(projectPath string, buildpackConfig *BuildPackConfig) { @@ -416,24 +417,26 @@ func handleLanguageVersion(projectPath string, buildpackConfig *BuildPackConfig) log.Println("final Path is ", finalPath) ext := filepath.Ext(finalPath) if ext == ".json" { - jqCmd := fmt.Sprintf("jq '.engines.node' %s", finalPath) - outputBytes, err := exec.Command("/bin/sh", "-c", jqCmd).Output() + jqCmd := util.NewCommand("jq", "'.engines.node'", finalPath) + outputBytes, err := exec.Command("/bin/sh", jqCmd.GetCommandToBeExecuted("-c")...).Output() if err != nil { log.Println("error occurred while fetching node version", "err", err) return } if strings.TrimSpace(string(outputBytes)) == "null" { + languageVersionFlag := fmt.Sprintf("'.engines.node = \"%s\"'", languageVersion) tmpJsonFile := "./tmp.json" - versionUpdateCmd := fmt.Sprintf("jq '.engines.node = \"%s\"' %s >%s", languageVersion, finalPath, tmpJsonFile) + tmpJsonFileFlag := fmt.Sprintf(">%s", tmpJsonFile) + versionUpdateCmd := util.NewCommand("jq", languageVersionFlag, finalPath, tmpJsonFileFlag) err := executeCmd(versionUpdateCmd) if err != nil { log.Println("error occurred while inserting node version", "err", err) return } - fileReplaceCmd := fmt.Sprintf("mv %s %s", tmpJsonFile, finalPath) + fileReplaceCmd := util.NewCommand("mv", tmpJsonFile, finalPath) err = executeCmd(fileReplaceCmd) if err != nil { - log.Println("error occurred while executing cmd ", fileReplaceCmd, "err", err) + log.Println("error occurred while executing cmd ", fileReplaceCmd.PrintCommand(), "err", err) return } } @@ -445,8 +448,15 @@ func handleLanguageVersion(projectPath string, buildpackConfig *BuildPackConfig) } -func executeCmd(dockerBuild string) error { - dockerBuildCMD := exec.Command("/bin/sh", "-c", dockerBuild) +// executeCmd uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) +func executeCmd(dockerBuild *util.CommandType) error { + dockerBuildCMD := exec.Command("/bin/sh", dockerBuild.GetCommandToBeExecuted("-c")...) err := util.RunCommand(dockerBuildCMD) if err != nil { log.Println(err) @@ -455,9 +465,9 @@ func executeCmd(dockerBuild string) error { } func tagDockerBuild(dockerRepository string, dest string) error { - dockerTag := "docker tag " + dockerRepository + ":latest" + " " + dest - log.Println(" -----> " + dockerTag) - dockerTagCMD := exec.Command("/bin/sh", "-c", dockerTag) + dockerTagCmd := util.NewCommand("docker", "tag", fmt.Sprintf("%s:latest", dockerRepository), dest) + log.Println(" -----> " + dockerTagCmd.PrintCommand()) + dockerTagCMD := exec.Command("/bin/sh", dockerTagCmd.GetCommandToBeExecuted("-c")...) err := util.RunCommand(dockerTagCMD) if err != nil { log.Println(err) @@ -475,16 +485,14 @@ func setupCacheForBuildx(localCachePath string, oldCacheBuildxPath string) error if err != nil { return err } - copyContent := "cp -R " + localCachePath + " " + oldCacheBuildxPath - copyContentCmd := exec.Command("/bin/sh", "-c", copyContent) + copyContentCmd := exec.Command("/bin/sh", "-c", "cp", "-R", localCachePath, oldCacheBuildxPath) err = util.RunCommand(copyContentCmd) if err != nil { log.Println(err) return err } - cleanContent := "rm -rf " + localCachePath + "/*" - cleanContentCmd := exec.Command("/bin/sh", "-c", cleanContent) + cleanContentCmd := exec.Command("/bin/sh", "-c", "rm", "-rf", localCachePath, "/*") err = util.RunCommand(cleanContentCmd) if err != nil { log.Println(err) @@ -494,9 +502,9 @@ func setupCacheForBuildx(localCachePath string, oldCacheBuildxPath string) error } func createBuildxBuilder() error { - multiPlatformCmd := "docker buildx create --use --buildkitd-flags '--allow-insecure-entitlement network.host --allow-insecure-entitlement security.insecure'" - log.Println(" -----> " + multiPlatformCmd) - dockerBuildCMD := exec.Command("/bin/sh", "-c", multiPlatformCmd) + multiPlatformCmd := util.NewCommand("docker", "buildx", "create", "--use", "--buildkitd-flags", "'--allow-insecure-entitlement network.host --allow-insecure-entitlement security.insecure'") + log.Println(" -----> " + multiPlatformCmd.PrintCommand()) + dockerBuildCMD := exec.Command("/bin/sh", multiPlatformCmd.GetCommandToBeExecuted("-c")...) err := util.RunCommand(dockerBuildCMD) if err != nil { log.Println(err) @@ -506,9 +514,9 @@ func createBuildxBuilder() error { } func installAllSupportedPlatforms() error { - multiPlatformCmd := "docker run --privileged --rm quay.io/devtron/binfmt:stable --install all" - log.Println(" -----> " + multiPlatformCmd) - dockerBuildCMD := exec.Command("/bin/sh", "-c", multiPlatformCmd) + multiPlatformCmd := util.NewCommand("docker", "run", "--privileged", "--rm", "quay.io/devtron/binfmt:stable", "--install", "all") + log.Println(" -----> " + multiPlatformCmd.PrintCommand()) + dockerBuildCMD := exec.Command("/bin/sh", multiPlatformCmd.GetCommandToBeExecuted("-c")...) err := util.RunCommand(dockerBuildCMD) if err != nil { log.Println(err) @@ -518,8 +526,7 @@ func installAllSupportedPlatforms() error { } func checkAndCreateDirectory(localCachePath string) error { - makeDirCmd := "mkdir -p " + localCachePath - pathCreateCommand := exec.Command("/bin/sh", "-c", makeDirCmd) + pathCreateCommand := exec.Command("/bin/sh", "-c", "mkdir", "-pv", localCachePath) err := util.RunCommand(pathCreateCommand) if err != nil { log.Println(err) @@ -547,9 +554,9 @@ func BuildDockerImagePath(ciRequest *CommonWorkflowRequest) (string, error) { func PushArtifact(dest string) error { //awsLogin := "$(aws ecr get-login --no-include-email --region " + ciRequest.AwsRegion + ")" - dockerPush := "docker push " + dest + dockerPush := fmt.Sprintf("docker push %s", dest) log.Println("-----> " + dockerPush) - dockerPushCMD := exec.Command("/bin/sh", "-c", dockerPush) + dockerPushCMD := exec.Command("/bin/sh", "-c", "docker", "push", dest) err := util.RunCommand(dockerPushCMD) if err != nil { log.Println(err) @@ -581,8 +588,7 @@ func ExtractDigestForBuildx(dest string) (string, error) { } func ExtractDigestUsingPull(dest string) (string, error) { - dockerPull := "docker pull " + dest - dockerPullCmd := exec.Command("/bin/sh", "-c", dockerPull) + dockerPullCmd := exec.Command("/bin/sh", "-c", "docker", "pull", dest) digest, err := runGetDockerImageDigest(dockerPullCmd) if err != nil { log.Println(err) @@ -647,11 +653,10 @@ func createBuildxBuilderWithK8sDriver(builderNodes []map[string]string, ciPipeli defaultNodeOpts := builderNodes[0] buildxCreate := getBuildxK8sDriverCmd(defaultNodeOpts, ciPipelineId, ciWorkflowId) - buildxCreate = fmt.Sprintf("%s %s", buildxCreate, "--use") - + buildxCreate.AppendCommand("--use") err, errBuf := runCmd(buildxCreate) if err != nil { - fmt.Println(util.DEVTRON, "buildxCreate : ", buildxCreate, " err : ", err, " error : ", errBuf.String(), "\n ") + fmt.Println(util.DEVTRON, "buildxCreate : ", buildxCreate.PrintCommand(), " err : ", err, " error : ", errBuf.String(), "\n ") return err } @@ -659,11 +664,11 @@ func createBuildxBuilderWithK8sDriver(builderNodes []map[string]string, ciPipeli for i := 1; i < len(builderNodes); i++ { nodeOpts := builderNodes[i] appendNode := getBuildxK8sDriverCmd(nodeOpts, ciPipelineId, ciWorkflowId) - appendNode = fmt.Sprintf("%s %s", appendNode, "--append") + appendNode.AppendCommand("--append") err, errBuf = runCmd(appendNode) if err != nil { - fmt.Println(util.DEVTRON, " appendNode : ", appendNode, " err : ", err, " error : ", errBuf.String(), "\n ") + fmt.Println(util.DEVTRON, " appendNode : ", appendNode.PrintCommand(), " err : ", err, " error : ", errBuf.String(), "\n ") return err } } @@ -691,15 +696,17 @@ func leaveNodesFromBuildxK8sDriver(nodeNames []string) (error, *bytes.Buffer) { var err error var errBuf *bytes.Buffer defer func() { - removeCmd := fmt.Sprintf("docker buildx rm %s", BUILDX_K8S_DRIVER_NAME) + removeCmd := util.NewCommand("docker", "buildx", "rm", BUILDX_K8S_DRIVER_NAME) err, errBuf = runCmd(removeCmd) if err != nil { log.Println(util.DEVTRON, "error in removing docker buildx err : ", errBuf.String()) } }() for _, node := range nodeNames { - cmds := fmt.Sprintf("docker buildx create --name=%s --node=%s --leave", BUILDX_K8S_DRIVER_NAME, node) - err, errBuf = runCmd(cmds) + k8sDriverNameFlag := fmt.Sprintf("--name=%s", BUILDX_K8S_DRIVER_NAME) + k8sDriverNodeFlag := fmt.Sprintf("--node=%s", node) + k8sDriverLeaveNodeCmd := util.NewCommand("docker", "buildx", "create", k8sDriverNameFlag, k8sDriverNodeFlag, "--leave") + err, errBuf = runCmd(k8sDriverLeaveNodeCmd) if err != nil { log.Println(util.DEVTRON, "error in leaving node : ", errBuf.String()) return err, errBuf @@ -708,32 +715,34 @@ func leaveNodesFromBuildxK8sDriver(nodeNames []string) (error, *bytes.Buffer) { return err, errBuf } -func runCmd(cmd string) (error, *bytes.Buffer) { - fmt.Println(util.DEVTRON, " cmd : ", cmd) - builderCreateCmd := exec.Command("/bin/sh", "-c", cmd) +func runCmd(cmd *util.CommandType) (error, *bytes.Buffer) { + fmt.Println(util.DEVTRON, " cmd : ", cmd.PrintCommand()) + builderCreateCmd := exec.Command("/bin/sh", cmd.GetCommandToBeExecuted("-c")...) errBuf := &bytes.Buffer{} builderCreateCmd.Stderr = errBuf err := builderCreateCmd.Run() return err, errBuf } -func getBuildxK8sDriverCmd(driverOpts map[string]string, ciPipelineId, ciWorkflowId int) string { - buildxCreate := "docker buildx create --buildkitd-flags '--allow-insecure-entitlement network.host --allow-insecure-entitlement security.insecure' --name=%s --driver=kubernetes --node=%s --bootstrap " +func getBuildxK8sDriverCmd(driverOpts map[string]string, ciPipelineId, ciWorkflowId int) *util.CommandType { nodeName := driverOpts["node"] if nodeName == "" { nodeName = BUILDX_NODE_NAME + fmt.Sprintf("%v-%v", ciPipelineId, ciWorkflowId) + util.Generate(3) //need this to generate unique name for builder node in same builder. } - buildxCreate = fmt.Sprintf(buildxCreate, BUILDX_K8S_DRIVER_NAME, nodeName) + k8sDriverNameFlag := fmt.Sprintf("--name=%s", BUILDX_K8S_DRIVER_NAME) + k8sDriverNodeFlag := fmt.Sprintf("--node=%s", nodeName) + buildxCreateCmd := util.NewCommand("docker", "buildx", "create", "--buildkitd-flags", "'--allow-insecure-entitlement network.host --allow-insecure-entitlement security.insecure'", k8sDriverNameFlag, "--driver=kubernetes", k8sDriverNodeFlag, "--bootstrap") + platforms := driverOpts["platform"] if platforms != "" { - buildxCreate += " --platform=%s " - buildxCreate = fmt.Sprintf(buildxCreate, platforms) + buildxPlatformFlag := fmt.Sprintf("--platform=%s", platforms) + buildxCreateCmd.AppendCommand(buildxPlatformFlag) } if len(driverOpts["driverOptions"]) > 0 { - buildxCreate += " '--driver-opt=%s' " - buildxCreate = fmt.Sprintf(buildxCreate, driverOpts["driverOptions"]) + buildxDriverOptions := fmt.Sprintf("'--driver-opt=%s'", driverOpts["driverOptions"]) + buildxCreateCmd.AppendCommand(buildxDriverOptions) } - return buildxCreate + return buildxCreateCmd } func StopDocker() error { @@ -742,18 +751,17 @@ func StopDocker() error { return err } if len(out) > 0 { - stopCmdS := "docker stop -t 5 $(docker ps -a -q)" log.Println(util.DEVTRON, " -----> stopping docker container") - stopCmd := exec.Command("/bin/sh", "-c", stopCmdS) + stopCmd := exec.Command("/bin/sh", "-c", "docker", "stop", "-t", "5", "$(docker ps -a -q)") err := util.RunCommand(stopCmd) log.Println(util.DEVTRON, " -----> stopped docker container") if err != nil { log.Fatal(err) return err } - removeContainerCmds := "docker rm -v -f $(docker ps -a -q)" + removeContainerCmds := util.NewCommand("docker", "rm", "-v", "-f", "$(docker ps -a -q)") log.Println(util.DEVTRON, " -----> removing docker container") - removeContainerCmd := exec.Command("/bin/sh", "-c", removeContainerCmds) + removeContainerCmd := exec.Command("/bin/sh", removeContainerCmds.GetCommandToBeExecuted("-c")...) err = util.RunCommand(removeContainerCmd) log.Println(util.DEVTRON, " -----> removed docker container") if err != nil { @@ -802,8 +810,8 @@ func waitForDockerDaemon(retryCount int) { } func DockerdUpCheck() error { - dockerCheck := "docker ps" - dockerCheckCmd := exec.Command("/bin/sh", "-c", dockerCheck) + dockerCheck := util.NewCommand("docker", "ps") + dockerCheckCmd := exec.Command("/bin/sh", dockerCheck.GetCommandToBeExecuted("-c")...) err := dockerCheckCmd.Run() return err } diff --git a/helper/GitCliHelper.go b/helper/GitCliHelper.go index 6ce6b542..c739f85a 100644 --- a/helper/GitCliHelper.go +++ b/helper/GitCliHelper.go @@ -20,6 +20,13 @@ func NewGitUtil() *GitUtil { const GIT_AKS_PASS = "/git-ask-pass.sh" +// Fetch uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) Fetch(gitContext GitContext, rootDir string) (response, errMsg string, err error) { log.Println(util.DEVTRON, "git fetch ", "location", rootDir) cmd := exec.Command("git", "-C", rootDir, "fetch", "origin", "--tags", "--force") @@ -28,6 +35,13 @@ func (impl *GitUtil) Fetch(gitContext GitContext, rootDir string) (response, err return output, "", nil } +// Checkout uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) Checkout(rootDir string, checkout string) (response, errMsg string, err error) { log.Println(util.DEVTRON, "git checkout ", "location", rootDir) cmd := exec.Command("git", "-C", rootDir, "checkout", checkout, "--force") @@ -36,11 +50,18 @@ func (impl *GitUtil) Checkout(rootDir string, checkout string) (response, errMsg return output, "", nil } +// runCommandWithCred uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) runCommandWithCred(cmd *exec.Cmd, userName, password string) (response, errMsg string, err error) { cmd.Env = append(os.Environ(), fmt.Sprintf("GIT_ASKPASS=%s", GIT_AKS_PASS), - fmt.Sprintf("GIT_USERNAME=%s", userName), // ignored - fmt.Sprintf("GIT_PASSWORD=%s", password), // this value is used + fmt.Sprintf("GIT_USERNAME=%q", userName), // ignored; %q is used intentionally to sanitise the username + fmt.Sprintf("GIT_PASSWORD=%q", password), // this value is used; %q is used intentionally to sanitise the password ) return impl.runCommand(cmd) } @@ -99,16 +120,30 @@ func (impl *GitUtil) Clone(gitContext GitContext, rootDir string, remoteUrl stri return response, errMsg, err } -// setting user.name and user.email as for non-fast-forward merge, git ask for user.name and email +// Merge sets user.name and user.email as for non-fast-forward merge, git ask for user.name and email | +// Merge uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) Merge(rootDir string, commit string) (response, errMsg string, err error) { log.Println(util.DEVTRON, "git merge ", "location", rootDir) - command := "cd " + rootDir + " && git config user.email git@devtron.com && git config user.name Devtron && git merge " + commit + " --no-commit" - cmd := exec.Command("/bin/sh", "-c", command) + command := util.NewCommand("cd", rootDir, "&&", "git", "config", "user.email", "git@devtron.com", "&&", "git", "config", "user.name", "Devtron", "&&", "git", "merge", commit, "--no-commit") + cmd := exec.Command("/bin/sh", command.GetCommandToBeExecuted("-c")...) output, errMsg, err := impl.runCommand(cmd) log.Println(util.DEVTRON, "merge output", "root", rootDir, "opt", output, "errMsg", errMsg, "error", err) return output, errMsg, err } +// RecursiveFetchSubmodules uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) RecursiveFetchSubmodules(rootDir string) (response, errMsg string, error error) { log.Println(util.DEVTRON, "git recursive fetch submodules ", "location", rootDir) cmd := exec.Command("git", "-C", rootDir, "submodule", "update", "--init", "--recursive") @@ -117,6 +152,13 @@ func (impl *GitUtil) RecursiveFetchSubmodules(rootDir string) (response, errMsg return output, eMsg, err } +// UpdateCredentialHelper uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) UpdateCredentialHelper(rootDir string) (response, errMsg string, error error) { log.Println(util.DEVTRON, "git credential helper store ", "location", rootDir) cmd := exec.Command("git", "-C", rootDir, "config", "--global", "credential.helper", "store") @@ -125,6 +167,13 @@ func (impl *GitUtil) UpdateCredentialHelper(rootDir string) (response, errMsg st return output, eMsg, err } +// UnsetCredentialHelper uses CLI to run git command and it is prone to script injection | +// Don'ts: +// 1- Never concatenate the whole cmd args into a single string and pass it as exec.Command(name, fmt.Sprintf("--flag1 %s --flag2 %s --flag3 %s", value1, value2, value3)) | +// DOs: +// 1- Break the command to name and []args as exec.Command(name, []arg...) +// 2- Use strings.TrimSpace() to build an user defined flags; e.g: fmt.Sprintf("--%s", strings.TrimSpace(userDefinedFlag)) +// 3- In case a single arg contains multiple user defined inputs, then use fmt.Sprintf(); exec.Command(name, "--flag=", fmt.Sprintf("key1=%s,key2=%s,key3=%s", userDefinedArg-1, userDefinedArg-2, userDefinedArg-2)) func (impl *GitUtil) UnsetCredentialHelper(rootDir string) (response, errMsg string, error error) { log.Println(util.DEVTRON, "git credential helper unset ", "location", rootDir) cmd := exec.Command("git", "-C", rootDir, "config", "--global", "--unset", "credential.helper") diff --git a/helper/GitHelper.go b/helper/GitHelper.go index 38819267..6b3051be 100644 --- a/helper/GitHelper.go +++ b/helper/GitHelper.go @@ -128,7 +128,6 @@ func CloneAndCheckout(ciProjectDetails []CiProjectDetails) error { if cErr != nil { log.Fatal("could not checkout hash ", " err ", cErr, "msgMsg", msgMsg) } - } else if prj.SourceType == SOURCE_TYPE_WEBHOOK { webhookData := prj.WebhookData diff --git a/util/CmdUtil.go b/util/CmdUtil.go index 2eecf670..acb72682 100644 --- a/util/CmdUtil.go +++ b/util/CmdUtil.go @@ -22,6 +22,7 @@ import ( "io" "os" "os/exec" + "strings" ) func DeleteFile(path string) error { @@ -43,3 +44,36 @@ func RunCommand(cmd *exec.Cmd) error { //log.Println(stdBuffer.String()) return nil } + +type CommandType []string + +func NewCommand(newArgs ...string) *CommandType { + cmd := make(CommandType, 0) + cmd.AppendCommand(newArgs...) + return &cmd +} + +func (c *CommandType) AppendCommand(newArgs ...string) { + for _, newArg := range newArgs { + trimmedArg := strings.TrimSpace(newArg) + if trimmedArg != "" { + *c = append(*c, trimmedArg) + } + } +} + +func (c *CommandType) PrintCommand() string { + if c == nil { + return "" + } + return strings.Join(*c, " ") +} + +func (c *CommandType) GetCommandToBeExecuted(initialArgs ...string) []string { + runCmd := initialArgs + if c == nil { + return runCmd + } + runCmd = append(runCmd, *c...) + return runCmd +}