diff --git a/cmd/ocm-backplane/cloud/ssm.go b/cmd/ocm-backplane/cloud/ssm.go index 91797f24..04cd0e01 100644 --- a/cmd/ocm-backplane/cloud/ssm.go +++ b/cmd/ocm-backplane/cloud/ssm.go @@ -248,10 +248,10 @@ func runSSMsession(ssmClient SSMClient, instanceID string, command []string, reg return fmt.Errorf("session details are incomplete: SessionId=%v, StreamUrl=%v, TokenValue=%v", result.SessionId, result.StreamUrl, result.TokenValue) } - // Log session details for debugging - logger.Infof("SessionId: %v", *result.SessionId) - logger.Infof("StreamUrl: %v", *result.StreamUrl) - logger.Infof("TokenValue: %v", *result.TokenValue) + // Log non-sensitive session identifier for debugging. + // StreamUrl and TokenValue are intentionally excluded from logs + // to prevent credential exposure (CWE-532, CWE-312). + logger.Debugf("SessionId: %v", *result.SessionId) sessionJSON, err := json.Marshal(map[string]string{ "SessionId": *result.SessionId, diff --git a/cmd/ocm-backplane/cloud/ssm_test.go b/cmd/ocm-backplane/cloud/ssm_test.go index b3145a7e..02b071e3 100644 --- a/cmd/ocm-backplane/cloud/ssm_test.go +++ b/cmd/ocm-backplane/cloud/ssm_test.go @@ -1,9 +1,11 @@ package cloud import ( + "bytes" "context" "encoding/json" "errors" + "io" "fmt" "os" "os/exec" @@ -11,7 +13,6 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/ssm" - "go.uber.org/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" @@ -21,7 +22,9 @@ import ( ocmMock "github.com/openshift/backplane-cli/pkg/ocm/mocks" "github.com/openshift/backplane-cli/pkg/ssm/mocks" "github.com/openshift/backplane-cli/pkg/utils" + log "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "go.uber.org/mock/gomock" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -136,6 +139,70 @@ var _ = Describe("SSM command", func() { }) }) + Context("SSM session should not log sensitive credentials", func() { + var ( + logBuf bytes.Buffer + originalExecCommand func(string, ...string) *exec.Cmd + originalLevel log.Level + originalOutput io.Writer + ) + + BeforeEach(func() { + logBuf.Reset() + originalLevel = log.GetLevel() + originalOutput = log.StandardLogger().Out + log.SetLevel(log.InfoLevel) + log.SetOutput(&logBuf) + + originalExecCommand = ExecCommand + ExecCommand = func(name string, arg ...string) *exec.Cmd { + return exec.Command("echo", "mock command") + } + + mockSSMClient.EXPECT().StartSession( + context.TODO(), + gomock.Any(), + ).Return(&ssm.StartSessionOutput{ + SessionId: aws.String("test-session-id"), + StreamUrl: aws.String("wss://secret-stream-url.example.com"), + TokenValue: aws.String("secret-token-value-abc123"), + }, nil) + }) + + AfterEach(func() { + ExecCommand = originalExecCommand + log.SetLevel(originalLevel) + log.SetOutput(originalOutput) + }) + + It("should not log TokenValue at Info level", func() { + err := runSSMsession(mockSSMClient, "i-1234567890abcdef0", nil, "us-west-2") + Expect(err).ToNot(HaveOccurred()) + + logOutput := logBuf.String() + Expect(logOutput).ToNot(ContainSubstring("secret-token-value-abc123"), + "TokenValue should not appear in log output at Info level") + }) + + It("should not log StreamUrl at Info level", func() { + err := runSSMsession(mockSSMClient, "i-1234567890abcdef0", nil, "us-west-2") + Expect(err).ToNot(HaveOccurred()) + + logOutput := logBuf.String() + Expect(logOutput).ToNot(ContainSubstring("wss://secret-stream-url.example.com"), + "StreamUrl should not appear in log output at Info level") + }) + + It("should not log SessionId at Info level", func() { + err := runSSMsession(mockSSMClient, "i-1234567890abcdef0", nil, "us-west-2") + Expect(err).ToNot(HaveOccurred()) + + logOutput := logBuf.String() + Expect(logOutput).ToNot(ContainSubstring("test-session-id"), + "SessionId should only appear at Debug level, not Info") + }) + }) + var _ = Describe("SSM command", func() { var ( originalExecCommand func(string, ...string) *exec.Cmd