From 7ac901afd769739fb9628ef562ccba6671e7b76e Mon Sep 17 00:00:00 2001 From: Yuvraj Kolkar Date: Fri, 30 Jan 2026 20:06:32 +0530 Subject: [PATCH 1/2] test(filter) Migrate test and Improve code coverage Signed-off-by: Yuvraj Kolkar --- pkg/utils/security/coverage.html | 221 ++++++++++++++++++++++++++++++ pkg/utils/security/filter_test.go | 107 +++++++-------- 2 files changed, 268 insertions(+), 60 deletions(-) create mode 100644 pkg/utils/security/coverage.html 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_test.go b/pkg/utils/security/filter_test.go index 707bff37d01..3868321268e 100644 --- a/pkg/utils/security/filter_test.go +++ b/pkg/utils/security/filter_test.go @@ -17,72 +17,59 @@ 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 ]"}, - }, - } - - 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) - } - } - +func TestSecurity(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Security Suite") } -func TestFilterCommandWithSensitive(t *testing.T) { +var _ = Describe("FilterCommand", func() { + 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)) + }) - type testCase struct { - name string - filterKey string - input []string - expect []string - } + 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)) + }) - 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 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)) + }) + }) +}) - 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() { + 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)) + }) + }) +}) From 17cb7d3847dabebbea03ac6e19961c6b05b03da6 Mon Sep 17 00:00:00 2001 From: Yuvraj Kolkar Date: Fri, 30 Jan 2026 20:32:06 +0530 Subject: [PATCH 2/2] Bug fix Signed-off-by: Yuvraj Kolkar --- pkg/utils/security/filter.go | 31 +++++++++++++++++++++++++++---- pkg/utils/security/filter_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) 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 3868321268e..b1be9cfa26b 100644 --- a/pkg/utils/security/filter_test.go +++ b/pkg/utils/security/filter_test.go @@ -28,6 +28,16 @@ func TestSecurity(t *testing.T) { } 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"} @@ -49,10 +59,27 @@ var _ = Describe("FilterCommand", func() { got := FilterCommand(input) Expect(got).To(Equal(expect)) }) + + 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)) + }) }) }) 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"