[processor/redaction] Apply blocked_values patterns in configuration order

Author: yamoyamoto

.chloggen/redactionprocessor-deterministic-blocked-values-order.yaml

@@ -0,0 +1,30 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
+component: processor/redaction
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Apply `blocked_values` patterns in the order they are listed in the configuration instead of a nondeterministic order
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [49858]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  Previously the patterns were applied in Go map iteration order. When two patterns
+  could match overlapping parts of the same value, the result changed from run to run,
+  and some orders left data unmasked that another order would have redacted.
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [user]

processor/redactionprocessor/README.md

@@ -87,7 +87,8 @@ processors:
       - ".*token.*"
       - ".*api_key.*"
     # blocked_values is a list of regular expressions for blocking values of
-    # allowed span attributes. Values that match are masked
+    # allowed span attributes. Values that match are masked. The patterns are
+    # applied in the order they are listed.
     blocked_values:
       - "4[0-9]{12}(?:[0-9]{3})?" ## Visa credit card number
       - "(5[1-5][0-9]{14})"       ## MasterCard number
@@ -139,6 +140,11 @@ part of the value is not masked even if it matches the regular expression for a
 If the value matches the regular expression for a blocked value only, the matching
 part of the value is masked with a fixed length of asterisks.
 
+The `blocked_values` patterns are applied in the order they are listed in the
+configuration. When two patterns can match overlapping parts of the same value,
+the pattern listed first is applied first, which determines what the later
+pattern can still match.
+
 ### Precedence between `allowed_values` and `blocked_values`
 
 When both `allowed_values` and `blocked_values` are configured, `allowed_values` takes precedence.

processor/redactionprocessor/processor.go

@@ -38,13 +38,13 @@ type redaction struct {
 	// Attribute keys ignored in a span
 	ignoreList map[string]string
 	// Attribute key patterns ignored in a span
-	ignoreKeyRegexList map[string]*regexp.Regexp
+	ignoreKeyRegexList []*regexp.Regexp
 	// Attribute values blocked in a span
-	blockRegexList map[string]*regexp.Regexp
+	blockRegexList []*regexp.Regexp
 	// Attribute values allowed in a span
-	allowRegexList map[string]*regexp.Regexp
+	allowRegexList []*regexp.Regexp
 	// Attribute keys blocked in a span
-	blockKeyRegexList map[string]*regexp.Regexp
+	blockKeyRegexList []*regexp.Regexp
 	// Hash function to hash blocked values
 	hashFunction HashFunction
 	// Redaction processor configuration
@@ -631,16 +631,17 @@ func makeIgnoreList(c *Config) map[string]string {
 	return ignoreList
 }
 
-// makeRegexList precompiles all the regex patterns in the defined list
-func makeRegexList(_ context.Context, valuesList []string) (map[string]*regexp.Regexp, error) {
-	regexList := make(map[string]*regexp.Regexp, len(valuesList))
+// makeRegexList precompiles all the regex patterns in the defined list,
+// preserving the order in which they are listed in the configuration
+func makeRegexList(_ context.Context, valuesList []string) ([]*regexp.Regexp, error) {
+	regexList := make([]*regexp.Regexp, 0, len(valuesList))
 	for _, pattern := range valuesList {
 		re, err := regexp.Compile(pattern)
 		if err != nil {
 			// TODO: Placeholder for an error metric in the next PR
 			return nil, fmt.Errorf("error compiling regex in list: %w", err)
 		}
-		regexList[pattern] = re
+		regexList = append(regexList, re)
 	}
 	return regexList, nil
 }

processor/redactionprocessor/processor_test.go

@@ -1043,6 +1043,62 @@ func TestMultipleBlockValues(t *testing.T) {
 	}
 }
 
+// TestBlockedValuesAppliedInConfigOrder validates that blocked_values patterns
+// are applied in the order they are listed in the configuration. When two
+// patterns match overlapping regions of the same value, the outcome depends on
+// the application order, so the order must be deterministic and follow the
+// configuration.
+func TestBlockedValuesAppliedInConfigOrder(t *testing.T) {
+	emailPattern := `[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}`
+	postalCodePattern := `\b\d{3}-\d{4}\b`
+	// The local part of this email address also matches the postal code
+	// pattern, so the two patterns overlap on this value.
+	input := "777-7777@example.com"
+
+	testCases := []struct {
+		name          string
+		blockedValues []string
+		expected      string
+	}{
+		{
+			name:          "email pattern first masks the whole address",
+			blockedValues: []string{emailPattern, postalCodePattern},
+			expected:      "****",
+		},
+		{
+			name:          "postal code pattern first masks only the local part",
+			blockedValues: []string{postalCodePattern, emailPattern},
+			expected:      "****@example.com",
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			// Repeat to make a regression to nondeterministic ordering fail
+			// reliably instead of intermittently.
+			for i := 0; i < 10; i++ {
+				config := &Config{
+					AllowAllKeys:  true,
+					BlockedValues: tt.blockedValues,
+				}
+				processor, err := newRedaction(t.Context(), config, zaptest.NewLogger(t))
+				require.NoError(t, err)
+
+				attrs := pcommon.NewMap()
+				attrs.PutStr("email", input)
+				processor.processAttrs(t.Context(), attrs)
+				val, found := attrs.Get("email")
+				require.True(t, found)
+				assert.Equal(t, tt.expected, val.Str())
+
+				body := pcommon.NewValueStr(input)
+				processor.processLogBody(t.Context(), body, pcommon.NewMap())
+				assert.Equal(t, tt.expected, body.Str())
+			}
+		})
+	}
+}
+
 // TestProcessAttrsAppliedTwice validates a use case when data is coming through redaction processor more than once.
 // Existing attributes must be updated, not overridden or ignored.
 func TestProcessAttrsAppliedTwice(t *testing.T) {