Merge pull request #173 from v-yarotsky/vy-env-var-whitelist

Only expand concourse build metadata env vars

out.go

@@ -49,7 +49,7 @@ func Put(request PutRequest, manager Github, inputDir string) (*PutResponse, err
 			description = string(content)
 		}
 
-		if err := manager.UpdateCommitStatus(version.Commit, p.BaseContext, p.Context, p.Status, os.ExpandEnv(p.TargetURL), description); err != nil {
+		if err := manager.UpdateCommitStatus(version.Commit, p.BaseContext, p.Context, p.Status, safeExpandEnv(p.TargetURL), description); err != nil {
 			return nil, fmt.Errorf("failed to set status: %s", err)
 		}
 	}
@@ -64,7 +64,7 @@ func Put(request PutRequest, manager Github, inputDir string) (*PutResponse, err
 
 	// Set comment if specified
 	if p := request.Params; p.Comment != "" {
-		err = manager.PostComment(version.PR, os.ExpandEnv(p.Comment))
+		err = manager.PostComment(version.PR, safeExpandEnv(p.Comment))
 		if err != nil {
 			return nil, fmt.Errorf("failed to post comment: %s", err)
 		}
@@ -78,7 +78,7 @@ func Put(request PutRequest, manager Github, inputDir string) (*PutResponse, err
 		}
 		comment := string(content)
 		if comment != "" {
-			err = manager.PostComment(version.PR, os.ExpandEnv(comment))
+			err = manager.PostComment(version.PR, safeExpandEnv(comment))
 			if err != nil {
 				return nil, fmt.Errorf("failed to post comment: %s", err)
 			}
@@ -140,3 +140,13 @@ func (p *PutParameters) Validate() error {
 
 	return nil
 }
+
+func safeExpandEnv(s string) string {
+	return os.Expand(s, func(v string) string {
+		switch v {
+		case "BUILD_ID", "BUILD_NAME", "BUILD_JOB_NAME", "BUILD_PIPELINE_NAME", "BUILD_TEAM_NAME", "ATC_EXTERNAL_URL":
+			return os.Getenv(v)
+		}
+		return "$" + v
+	})
+}

out_test.go

@@ -220,8 +220,8 @@ func TestPut(t *testing.T) {
 func TestVariableSubstitution(t *testing.T) {
 
 	var (
-		variableName  = "EXAMPLE_VARIABLE"
-		variableValue = "value"
+		variableName  = "BUILD_JOB_NAME"
+		variableValue = "my-job"
 		variableURL   = "https://concourse-ci.org/"
 	)
 
@@ -271,6 +271,24 @@ func TestVariableSubstitution(t *testing.T) {
 			expectedTargetURL: fmt.Sprintf("%s%s", variableURL, variableValue),
 			pullRequest:       createTestPR(1, "master", false, false, 0, nil),
 		},
+
+		{
+			description: "we do not substitute variables other then concourse build metadata",
+			source: resource.Source{
+				Repository:  "itsdalmo/test-repository",
+				AccessToken: "oauthtoken",
+			},
+			version: resource.Version{
+				PR:            "pr1",
+				Commit:        "commit1",
+				CommittedDate: time.Time{},
+			},
+			parameters: resource.PutParameters{
+				Comment: "$THIS_IS_NOT_SUBSTITUTED",
+			},
+			expectedComment: "$THIS_IS_NOT_SUBSTITUTED",
+			pullRequest:     createTestPR(1, "master", false, false, 0, nil),
+		},
 	}
 
 	for _, tc := range tests {