Make sure to sort secrets by longest when hiding

When we hide secrets in the logs we need to make sure that we start from
the longest one, otherwise if we have two secrets with the same prefix the
shortest one may hide first and leak the end of the longest one. Since
we are in a loop and text is replace the substitutions would not occurs.

Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: chmouel

docs/content/docs/guide/statuses.md

@@ -45,8 +45,11 @@ failed task.
 To prevent exposing secrets, Pipelines-as-Code analyze the PipelineRun and
 replace secret values with hidden characters. This is achieved by retrieving
 all secrets from the environment variables associated with tasks and steps, and
-searching for matches of these values in the output snippet. These matches are
-then replaced with a `"*****"` placeholder hiding these secrets.
+searching for matches of these values in the output snippet.
+
+These matches are first sorted by the longest and then replaced with a
+`"*****"` placeholder in the output snippet. This ensures that the output
+will not contain any leaked secrets.
 
 The hiding of the secret does not support concealing secrets from `workspaces`
 and

pkg/secrets/secrets.go

@@ -12,7 +12,8 @@ import (
 
 const leakedReplacement = "*****"
 
-// GetSecretsAttachedToPipelineRun get all secrets attached to a PipelineRun and grab their values
+// GetSecretsAttachedToPipelineRun get all secrets attached to a PipelineRun and
+// grab their values attached to it
 func GetSecretsAttachedToPipelineRun(ctx context.Context, k kubeinteraction.Interface, pr *tektonv1.PipelineRun) []ktypes.SecretValue {
 	ret := []ktypes.SecretValue{}
 	// check if pipelineRef is defined or exist
@@ -61,9 +62,25 @@ func GetSecretsAttachedToPipelineRun(ctx context.Context, k kubeinteraction.Inte
 	return ret
 }
 
+// sortSecretsByLongests sort all secrets by length, the longest first
+// if we don't sort by longest then if there two passwords with the same prefix
+// the shortest one will replace and would leak the end of the passwords of the longest after
+func sortSecretsByLongests(values []ktypes.SecretValue) []ktypes.SecretValue {
+	ret := []ktypes.SecretValue{}
+	ret = append(ret, values...)
+	for i := 0; i < len(ret); i++ {
+		for j := i + 1; j < len(ret); j++ {
+			if len(ret[i].Value) < len(ret[j].Value) {
+				ret[i], ret[j] = ret[j], ret[i]
+			}
+		}
+	}
+	return ret
+}
+
 // ReplaceSecretsInText this will take a text snippet and hide the leaked secret
 func ReplaceSecretsInText(text string, values []ktypes.SecretValue) string {
-	for _, sv := range values {
+	for _, sv := range sortSecretsByLongests(values) {
 		text = strings.ReplaceAll(text, sv.Value, leakedReplacement)
 	}
 	return text

pkg/secrets/secrets_test.go

@@ -202,6 +202,21 @@ func TestReplaceSecretsInText(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:   "replace secrets in text with same prefix",
+			text:   "I am the most beautifuller person in the world with the most beautiful friends",
+			result: "I am the most ***** person in the world with the most ***** friends",
+			values: []types.SecretValue{
+				{
+					Name:  "beautiful-secret",
+					Value: "beautiful",
+				},
+				{
+					Name:  "most-beautiful-secret",
+					Value: "beautifuller",
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -210,3 +225,22 @@ func TestReplaceSecretsInText(t *testing.T) {
 		})
 	}
 }
+
+func TestSortByLongest(t *testing.T) {
+	values := []types.SecretValue{
+		{
+			Name:  "beautiful-secret",
+			Value: "beautiful",
+		},
+		{
+			Name:  "Even more beautiful",
+			Value: "beautifuller",
+		},
+		{
+			Name:  "Not that beautiful",
+			Value: "notpretty",
+		},
+	}
+	ret := sortSecretsByLongests(values)
+	assert.Equal(t, ret[0].Name, "Even more beautiful")
+}