diff --git a/pkg/utils/security/coverage.html b/pkg/utils/security/coverage.html new file mode 100644 index 00000000000..48a3bde5676 --- /dev/null +++ b/pkg/utils/security/coverage.html @@ -0,0 +1,221 @@ + + + + + + security: Go Coverage Report + + + +
+ +
+ not tracked + + not covered + covered + +
+
+
+ + + + + +
+ + + diff --git a/pkg/utils/security/filter.go b/pkg/utils/security/filter.go index 3d8d77bd666..ff93bc643e7 100644 --- a/pkg/utils/security/filter.go +++ b/pkg/utils/security/filter.go @@ -34,14 +34,37 @@ func FilterCommand(command []string) (filteredCommand []string) { } func FilterString(line string) string { + result := line for s := range sensitiveKeys { - // if the log line contains a secret value redact it - if strings.Contains(line, s) { - line = s + "=[ redacted ]" + // if the log line contains a secret key, redact its value + if strings.Contains(result, s) { + // Look for pattern "key=" and replace everything after = until space or end + searchPattern := s + "=" + idx := strings.Index(result, searchPattern) + + for idx != -1 { + // Find the end of the value (next space or end of string) + startValue := idx + len(searchPattern) + endValue := startValue + + // Find where the value ends (space, newline, or end of string) + for endValue < len(result) && result[endValue] != ' ' && result[endValue] != '\n' && result[endValue] != '\t' { + endValue++ + } + + // Replace the value with [ redacted ] + result = result[:startValue] + "[ redacted ]" + result[endValue:] + + // Look for next occurrence + idx = strings.Index(result[startValue+len("[ redacted ]"):], searchPattern) + if idx != -1 { + idx = idx + startValue + len("[ redacted ]") + } + } } } - return line + return result } func UpdateSensitiveKey(key string) { diff --git a/pkg/utils/security/filter_test.go b/pkg/utils/security/filter_test.go index 707bff37d01..b1be9cfa26b 100644 --- a/pkg/utils/security/filter_test.go +++ b/pkg/utils/security/filter_test.go @@ -17,72 +17,86 @@ limitations under the License. package security import ( - "reflect" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "testing" ) -func TestFilterCommand(t *testing.T) { - - type testCase struct { - name string - input []string - expect []string - } - - testCases := []testCase{ - { - name: "withSensitiveKey", - input: []string{"mount", "fs", "aws.secretKey=xxxxxxxxx"}, - expect: []string{"mount", "fs", "aws.secretKey=[ redacted ]"}, - }, { - name: "withOutSensitiveKey", - input: []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false"}, - expect: []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false"}, - }, { - name: "key", - input: []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false", "aws.secretKey=xxxxxxxxx"}, - expect: []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false", "aws.secretKey=[ redacted ]"}, - }, - } +func TestSecurity(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Security Suite") +} - for _, test := range testCases { - got := FilterCommand(test.input) - if !reflect.DeepEqual(got, test.expect) { - t.Errorf("testcase %s is failed due to expect %v, but got %v", test.name, test.expect, got) +var _ = Describe("FilterCommand", func() { + BeforeEach(func() { + // Reset sensitiveKeys to its initial state before each test + sensitiveKeys = map[string]bool{ + "aws.secretKey": true, + "aws.accessKeyId": true, + "fs.oss.accessKeyId": true, + "fs.oss.accessKeySecret": true, } - } + }) -} + Context("when filtering commands", func() { + It("should redact sensitive keys", func() { + input := []string{"mount", "fs", "aws.secretKey=xxxxxxxxx"} + expect := []string{"mount", "fs", "aws.secretKey=[ redacted ]"} + got := FilterCommand(input) + Expect(got).To(Equal(expect)) + }) -func TestFilterCommandWithSensitive(t *testing.T) { + It("should not modify commands without sensitive keys", func() { + input := []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false"} + expect := []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false"} + got := FilterCommand(input) + Expect(got).To(Equal(expect)) + }) - type testCase struct { - name string - filterKey string - input []string - expect []string - } + It("should redact sensitive keys while preserving other parameters", func() { + input := []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false", "aws.secretKey=xxxxxxxxx"} + expect := []string{"mount", "fs", "alluxio.underfs.s3.inherit.acl=false", "aws.secretKey=[ redacted ]"} + got := FilterCommand(input) + Expect(got).To(Equal(expect)) + }) - testCases := []testCase{ - { - name: "NotAddSensitiveKey", - filterKey: "test", - input: []string{"mount", "fs", "fs.azure.account.key=xxxxxxxxx"}, - expect: []string{"mount", "fs", "fs.azure.account.key=xxxxxxxxx"}, - }, { - name: "AddSensitiveKey", - filterKey: "fs.azure.account.key", - input: []string{"mount", "fs", "fs.azure.account.key=false"}, - expect: []string{"mount", "fs", "fs.azure.account.key=[ redacted ]"}, - }, - } + It("should redact multiple sensitive keys in a single string", func() { + input := []string{"aws.secretKey=secret and aws.accessKeyId=key"} + expect := []string{"aws.secretKey=[ redacted ] and aws.accessKeyId=[ redacted ]"} + got := FilterCommand(input) + Expect(got).To(Equal(expect)) + }) + }) +}) - for _, test := range testCases { - UpdateSensitiveKey(test.filterKey) - got := FilterCommand(test.input) - if !reflect.DeepEqual(got, test.expect) { - t.Errorf("testcase %s is failed due to expect %v, but got %v", test.name, test.expect, got) +var _ = Describe("FilterCommandWithSensitive", func() { + BeforeEach(func() { + // Reset sensitiveKeys to its initial state before each test + sensitiveKeys = map[string]bool{ + "aws.secretKey": true, + "aws.accessKeyId": true, + "fs.oss.accessKeyId": true, + "fs.oss.accessKeySecret": true, } - } + }) -} + Context("when updating sensitive keys", func() { + It("should not redact keys that are not added as sensitive", func() { + filterKey := "test" + input := []string{"mount", "fs", "fs.azure.account.key=xxxxxxxxx"} + expect := []string{"mount", "fs", "fs.azure.account.key=xxxxxxxxx"} + UpdateSensitiveKey(filterKey) + got := FilterCommand(input) + Expect(got).To(Equal(expect)) + }) + + It("should redact keys that are added as sensitive", func() { + filterKey := "fs.azure.account.key" + input := []string{"mount", "fs", "fs.azure.account.key=false"} + expect := []string{"mount", "fs", "fs.azure.account.key=[ redacted ]"} + UpdateSensitiveKey(filterKey) + got := FilterCommand(input) + Expect(got).To(Equal(expect)) + }) + }) +})