Kfs 1330/don't store secrets in history (#2495)

Co-authored-by: Tomasz Nguyen <[email protected]> Co-authored-by: Steven Gagniere <[email protected]>
confluentinc · Dec 15, 2023 · 903c7e0 · 903c7e0
1 parent e8fe56e
commit 903c7e0
Show file tree

Hide file tree

Showing 10 changed files with 156 additions and 23 deletions.
diff --git a/.gitignore b/.gitignore
@@ -44,7 +44,8 @@ go.work
 .LSOverride
 
 # Icon must end with two \r
-Icon
+Icon
+
 
 # Thumbnails
 ._*
@@ -78,6 +79,7 @@ Temporary Items
 .idea/**/usage.statistics.xml
 .idea/**/dictionaries
 .idea/**/shelf
+/.vscode/
 
 # AWS User-specific
 .idea/**/aws.xml

diff --git a/go.mod b/go.mod
@@ -171,6 +171,7 @@ require (
 	github.com/spf13/afero v1.9.3 // indirect
 	github.com/swaggest/jsonschema-go v0.3.45 // indirect
 	github.com/swaggest/refl v1.1.0 // indirect
+	github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c // indirect
 	github.com/tidwall/match v1.1.1 // indirect
 	github.com/travisjeffery/mocker v1.1.0 // indirect
 	github.com/travisjeffery/proto-go-sql v0.0.0-20190911121832-39ff47280e87 // indirect

diff --git a/go.sum b/go.sum
@@ -633,6 +633,8 @@ github.com/swaggest/jsonschema-go v0.3.45/go.mod h1:pW2jvwFFD2cQ0jv51bAo8+RLmma7
 github.com/swaggest/refl v1.1.0 h1:a+9a75Kv6ciMozPjVbOfcVTEQe81t2R3emvaD9oGQGc=
 github.com/swaggest/refl v1.1.0/go.mod h1:g3Qa6ki0A/L2yxiuUpT+cuBURuRaltF5SDQpg1kMZSY=
 github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww=
+github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c h1:HelZ2kAFadG0La9d+4htN4HzQ68Bm2iM9qKMSMES6xg=
+github.com/texttheater/golang-levenshtein/levenshtein v0.0.0-20200805054039-cae8b0eaed6c/go.mod h1:JlzghshsemAMDGZLytTFY8C1JQxQPhnatWqNwUXjggo=
 github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
 github.com/tidwall/gjson v1.17.0 h1:/Jocvlh98kcTfpN2+JzGQWQcqrPQwDrVEMApx/M5ZwM=
 github.com/tidwall/gjson v1.17.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=

diff --git a/...TestApplicationTestSuite-TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError b/...TestApplicationTestSuite-TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError
diff --git a/pkg/flink/app/application.go b/pkg/flink/app/application.go
@@ -137,12 +137,14 @@ func (a *Application) readEvalPrint() {
 		a.appController.ExitApplication()
 		return
 	}
-	a.history.Append(userInput)
 
 	executedStatement, err := a.statementController.ExecuteStatement(userInput)
 	if err != nil {
 		return
 	}
+	if !executedStatement.IsSensitiveStatement {
+		a.history.Append(userInput)
+	}
 
 	a.resultFetcher.Init(*executedStatement)
 	a.getOutputController(*executedStatement).VisualizeResults()

diff --git a/pkg/flink/app/application_test.go b/pkg/flink/app/application_test.go
@@ -104,19 +104,52 @@ func (s *ApplicationTestSuite) TestReplExitsAppWhenUserInitiatedExit() {
 	cupaloy.SnapshotT(s.T(), actual)
 }
 
-func (s *ApplicationTestSuite) TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError() {
+func (s *ApplicationTestSuite) TestReplAppendsStatementToHistoryIfNoErrorAndNotSensistiveStatement() {
 	userInput := "test-input"
+	statement := types.ProcessedStatement{PageToken: "not-empty"}
 	s.inputController.EXPECT().GetUserInput().Return(userInput)
 	s.inputController.EXPECT().HasUserEnabledReverseSearch().Return(false)
 	s.inputController.EXPECT().HasUserInitiatedExit(userInput).Return(false)
-	s.statementController.EXPECT().ExecuteStatement(userInput).Return(nil, &types.StatementError{})
+	s.statementController.EXPECT().ExecuteStatement(userInput).Return(&statement, nil)
+	s.resultFetcher.EXPECT().Init(statement)
+	s.interactiveOutputController.EXPECT().VisualizeResults()
 
 	actual := test.RunAndCaptureSTDOUT(s.T(), s.app.readEvalPrint)
 
-	cupaloy.SnapshotT(s.T(), actual)
+	require.Empty(s.T(), actual)
 	require.Equal(s.T(), []string{userInput}, s.history.Data)
 }
 
+func (s *ApplicationTestSuite) TestReplDoesntAppendStatementToHistoryIfError() {
+	userInput := "test-input"
+
+	s.inputController.EXPECT().GetUserInput().Return(userInput)
+	s.inputController.EXPECT().HasUserEnabledReverseSearch().Return(false)
+	s.inputController.EXPECT().HasUserInitiatedExit(userInput).Return(false)
+	s.statementController.EXPECT().ExecuteStatement(userInput).Return(nil, &types.StatementError{})
+
+	actual := test.RunAndCaptureSTDOUT(s.T(), s.app.readEvalPrint)
+
+	require.Empty(s.T(), actual)
+	require.Equal(s.T(), []string{}, s.history.Data)
+}
+
+func (s *ApplicationTestSuite) TestReplDoesntAppendStatementToHistoryIfSensistiveStatement() {
+	userInput := "test-input"
+	statement := types.ProcessedStatement{PageToken: "not-empty", IsSensitiveStatement: true}
+	s.inputController.EXPECT().GetUserInput().Return(userInput)
+	s.inputController.EXPECT().HasUserEnabledReverseSearch().Return(false)
+	s.inputController.EXPECT().HasUserInitiatedExit(userInput).Return(false)
+	s.statementController.EXPECT().ExecuteStatement(userInput).Return(&statement, nil)
+	s.resultFetcher.EXPECT().Init(statement)
+	s.interactiveOutputController.EXPECT().VisualizeResults()
+
+	actual := test.RunAndCaptureSTDOUT(s.T(), s.app.readEvalPrint)
+
+	require.Empty(s.T(), actual)
+	require.Equal(s.T(), []string{}, s.history.Data)
+}
+
 func (s *ApplicationTestSuite) TestReplStopsOnExecuteStatementError() {
 	userInput := "test-input"
 	s.inputController.EXPECT().GetUserInput().Return(userInput)

diff --git a/pkg/flink/config/local_statements.go b/pkg/flink/config/local_statements.go
@@ -19,4 +19,7 @@ const (
 	KeyResultsTimeout = "client.results-timeout"
 	KeyServiceAccount = "client.service-account"
 	KeyStatementName  = "client.statement-name"
+	KeyFlinkSecret    = "confluent.user.flink.secret"
 )
+
+var SensitiveKeys = []string{KeyFlinkSecret}
diff --git a/pkg/flink/internal/store/store_utils.go b/pkg/flink/internal/store/store_utils.go
@@ -10,6 +10,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/samber/lo"
+	"github.com/texttheater/golang-levenshtein/levenshtein"
+
 	flinkgatewayv1beta1 "github.com/confluentinc/ccloud-sdk-go-v2/flink-gateway/v1beta1"
 
 	"github.com/confluentinc/cli/v3/pkg/flink/config"
@@ -70,14 +73,20 @@ func (s *Store) processSetStatement(statement string) (*types.ProcessedStatement
 			Suggestion: `please provide a non-empty statement name with "SET 'client.statement-name'='non-empty-name'"`,
 		}
 	}
+
+	hasSensitiveKey := lo.SomeBy(config.SensitiveKeys, func(sensitiveKey string) bool {
+		return isKeySimilarToSensitiveKey(sensitiveKey, configKey)
+	})
+
 	s.Properties.Set(configKey, configVal)
 
 	return &types.ProcessedStatement{
-		Kind:             config.OpSet,
-		StatusDetail:     "configuration updated successfully",
-		Status:           types.COMPLETED,
-		StatementResults: createStatementResults([]string{"Key", "Value"}, [][]string{{configKey, configVal}}),
-		IsLocalStatement: true,
+		Kind:                 config.OpSet,
+		StatusDetail:         "configuration updated successfully",
+		Status:               types.COMPLETED,
+		StatementResults:     createStatementResults([]string{"Key", "Value"}, [][]string{{configKey, configVal}}),
+		IsLocalStatement:     true,
+		IsSensitiveStatement: hasSensitiveKey,
 	}, nil
 }
 
@@ -388,6 +397,13 @@ func startsWithValidSQL(statement string) bool {
 	return config.SQLKeywords.Contains(firstWord)
 }
 
+func isKeySimilarToSensitiveKey(sensitiveKeyName string, key string) bool {
+	key = strings.ToLower(key)
+	distance := levenshtein.DistanceForStrings([]rune(sensitiveKeyName), []rune(key), levenshtein.DefaultOptions)
+
+	return distance <= 2
+}
+
 // Removes leading, trailling spaces, and semicolon from end, if present
 func removeStatementTerminator(s string) string {
 	for strings.HasSuffix(s, config.StatementTerminator) {

diff --git a/pkg/flink/internal/store/store_utils_test.go b/pkg/flink/internal/store/store_utils_test.go
@@ -142,6 +142,16 @@ func TestProcessSetStatement(t *testing.T) {
 			Suggestion: `please provide a non-empty statement name with "SET 'client.statement-name'='non-empty-name'"`,
 		}, err)
 	})
+
+	t.Run("should parse and identify sensitive set statement", func(t *testing.T) {
+		result, err := s.processSetStatement("set 'confluent.user.flink.secret' = 'mysecret'")
+		assert.Nil(t, err)
+		assert.EqualValues(t, true, result.IsSensitiveStatement)
+
+		result, err = s.processSetStatement("set 'confluent.user.flink.seecret' = 'mysecret'")
+		assert.Nil(t, err)
+		assert.EqualValues(t, true, result.IsSensitiveStatement)
+	})
 }
 
 func TestProcessResetStatement(t *testing.T) {
@@ -306,6 +316,70 @@ func hoursToSeconds(hours float32) int {
 	return int(hours * 60 * 60)
 }
 
+func TestIsUserSecretKey(t *testing.T) {
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flinsecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.ecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.ssecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.scret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.seecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.seret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.seccret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secet"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secrret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secrt"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secreet"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secre"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secrett"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "confluent.user.flink.secrettt"))
+
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.seCrEt"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINKsecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINsecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINKsecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.ecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.ssecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.scret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.seecret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.seret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.seccret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secet"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secrret"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secrt"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secreet"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secre"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secrett"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.secrettt"))
+
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINKSECRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINSECRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.ECRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SSECRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SCRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SEECRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SERET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECCRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECRRET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECRT"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECREET"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECRE"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECRETT"))
+	require.True(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECRETTT"))
+
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, ""))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "gustavo"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "sql.current-catalog"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "client.results-timeout"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "OPENAPI.KEY"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.NAME"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SCERECETASDT"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "CONFLUENT.USER.FLINK.SECCCCCCCCRET"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "SEEEEECRET.OPENAPI.KEY"))
+	require.False(t, isKeySimilarToSensitiveKey(config.KeyFlinkSecret, "SECRET.OPENAPI.KEY"))
+}
+
 func TestFormatUTCOffsetToTimezone(t *testing.T) {
 	testCases := []struct {
 		offsetSeconds int

diff --git a/pkg/flink/types/processed_statement.go b/pkg/flink/types/processed_statement.go
@@ -21,18 +21,19 @@ const (
 
 // Custom Internal type that shall be used internally by the client
 type ProcessedStatement struct {
-	Statement         string `json:"statement"`
-	StatementName     string `json:"statement_name"`
-	Kind              string `json:"kind"`
-	ComputePool       string `json:"compute_pool"`
-	Principal         string `json:"principal"`
-	Status            PHASE  `json:"status"`
-	StatusDetail      string `json:"status_detail,omitempty"` // Shown at the top before the table
-	IsLocalStatement  bool
-	IsSelectStatement bool
-	PageToken         string
-	ResultSchema      flinkgatewayv1beta1.SqlV1beta1ResultSchema
-	StatementResults  *StatementResults
+	Statement            string `json:"statement"`
+	StatementName        string `json:"statement_name"`
+	Kind                 string `json:"kind"`
+	ComputePool          string `json:"compute_pool"`
+	Principal            string `json:"principal"`
+	Status               PHASE  `json:"status"`
+	StatusDetail         string `json:"status_detail,omitempty"` // Shown at the top before the table
+	IsLocalStatement     bool
+	IsSelectStatement    bool
+	IsSensitiveStatement bool
+	PageToken            string
+	ResultSchema         flinkgatewayv1beta1.SqlV1beta1ResultSchema
+	StatementResults     *StatementResults
 }
 
 func NewProcessedStatement(statementObj flinkgatewayv1beta1.SqlV1beta1Statement) *ProcessedStatement {