diff options
author | Dan Duvall <dduvall@wikimedia.org> | 2017-07-12 11:54:25 -0700 |
---|---|---|
committer | Dan Duvall <dduvall@wikimedia.org> | 2017-07-17 08:57:56 -0700 |
commit | 3071d9290e5f91c11e4682cebde558346c4a0908 (patch) | |
tree | ea57bf3117dd74c768a558d773b865049397e6e6 /config | |
parent | 19b47a273717612e28374be9b63905985ce32ac9 (diff) | |
download | blubber-3071d9290e5f91c11e4682cebde558346c4a0908.tar.gz |
Quote CLI arguments in `RunsConfig` instructions
Summary:
Refactored build instructions in `RunsConfig` to properly quote command
arguments injected from user data.
Established unit tests for `RunsConfig` instruction phases.
Refs T170285. Depends on D711
Omit ENV instruction when `runs.environment` is empty
Fixes T170285
Test Plan:
Run `arc unit`. Create a config without `runs.environment` defined and verify
that no bare `ENV` line ends up in the `Dockerfile` output.
Reviewers: thcipriani, hashar, mmodell, #release-engineering-team
Reviewed By: thcipriani, #release-engineering-team
Tags: #release-engineering-team
Maniphest Tasks: T170285
Differential Revision: https://phabricator.wikimedia.org/D715
Diffstat (limited to 'config')
-rw-r--r-- | config/runs.go | 14 | ||||
-rw-r--r-- | config/runs_test.go | 57 |
2 files changed, 65 insertions, 6 deletions
diff --git a/config/runs.go b/config/runs.go index c61bff3..b2b6623 100644 --- a/config/runs.go +++ b/config/runs.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "sort" "strconv" @@ -70,20 +71,19 @@ func (run RunsConfig) InstructionsForPhase(phase build.Phase) []build.Instructio case build.PhasePrivileged: if run.In != "" { ins = append(ins, build.Instruction{build.Run, []string{ - "mkdir -p ", run.In, + fmt.Sprintf("mkdir -p %q", run.In), }}) } if run.As != "" { ins = append(ins, build.Instruction{build.Run, []string{ - "groupadd -o -g ", strconv.Itoa(run.Gid), " -r ", run.As, " && ", - "useradd -o -m -d ", strconv.Quote(run.Home()), " -r -g ", run.As, - " -u ", strconv.Itoa(run.Uid), " ", run.As, + fmt.Sprintf("groupadd -o -g %d -r %q", run.Gid, run.As) + " && " + + fmt.Sprintf("useradd -o -m -d %q -r -g %q -u %d %q", run.Home(), run.As, run.Uid, run.As), }}) if run.In != "" { ins = append(ins, build.Instruction{build.Run, []string{ - "chown ", run.As, ":", run.As, " ", run.In, + fmt.Sprintf("chown %q:%q %q", run.As, run.As, run.In), }}) } } @@ -92,7 +92,9 @@ func (run RunsConfig) InstructionsForPhase(phase build.Phase) []build.Instructio "HOME=" + strconv.Quote(run.Home()), }}) - ins = append(ins, build.Instruction{build.Env, run.EnvironmentDefinitions()}) + if definitions := run.EnvironmentDefinitions(); len(definitions) > 0 { + ins = append(ins, build.Instruction{build.Env, definitions}) + } } return ins diff --git a/config/runs_test.go b/config/runs_test.go index f8c626e..cef740c 100644 --- a/config/runs_test.go +++ b/config/runs_test.go @@ -5,6 +5,7 @@ import ( "gopkg.in/stretchr/testify.v1/assert" + "phabricator.wikimedia.org/source/blubber.git/build" "phabricator.wikimedia.org/source/blubber.git/config" ) @@ -59,3 +60,59 @@ func TestEnvironmentDefinitionsIsSortedAndQuoted(t *testing.T) { `quxname="quxvalue"`, }, runs.EnvironmentDefinitions()) } + +func TestRunsConfigInstructions(t *testing.T) { + cfg := config.RunsConfig{ + As: "someuser", + In: "/some/directory", + Uid: 666, + Gid: 777, + Environment: map[string]string{ + "fooname": "foovalue", + "barname": "barvalue", + }, + } + + t.Run("PhasePrivileged", func(t *testing.T) { + assert.Equal(t, + []build.Instruction{ + {build.Run, []string{`mkdir -p "/some/directory"`}}, + {build.Run, []string{ + `groupadd -o -g 777 -r "someuser" && ` + + `useradd -o -m -d "/home/someuser" -r -g "someuser" -u 666 "someuser"`, + }}, + {build.Run, []string{`chown "someuser":"someuser" "/some/directory"`}}, + }, + cfg.InstructionsForPhase(build.PhasePrivileged), + ) + }) + + t.Run("PhasePrivilegeDropped", func(t *testing.T) { + assert.Equal(t, + []build.Instruction{ + {build.Env, []string{`HOME="/home/someuser"`}}, + {build.Env, []string{`barname="barvalue"`, `fooname="foovalue"`}}, + }, + cfg.InstructionsForPhase(build.PhasePrivilegeDropped), + ) + + t.Run("with empty Environment", func(t *testing.T) { + cfg.Environment = map[string]string{} + + assert.Equal(t, + []build.Instruction{ + {build.Env, []string{`HOME="/home/someuser"`}}, + }, + cfg.InstructionsForPhase(build.PhasePrivilegeDropped), + ) + }) + }) + + t.Run("PhasePreInstall", func(t *testing.T) { + assert.Empty(t, cfg.InstructionsForPhase(build.PhasePreInstall)) + }) + + t.Run("PhasePostInstall", func(t *testing.T) { + assert.Empty(t, cfg.InstructionsForPhase(build.PhasePostInstall)) + }) +} |