From 903c7e07480abc9cb422b2440cf1dcbf514df31b Mon Sep 17 00:00:00 2001 From: Gustavo de Morais Date: Fri, 15 Dec 2023 01:31:43 +0100 Subject: [PATCH] Kfs 1330/don't store secrets in history (#2495) Co-authored-by: Tomasz Nguyen Co-authored-by: Steven Gagniere <108363707+sgagniere@users.noreply.github.com> --- .gitignore | 4 +- go.mod | 1 + go.sum | 2 + ...ntToHistoryAndStopsOnExecuteStatementError | 1 - pkg/flink/app/application.go | 4 +- pkg/flink/app/application_test.go | 39 +++++++++- pkg/flink/config/local_statements.go | 3 + pkg/flink/internal/store/store_utils.go | 26 +++++-- pkg/flink/internal/store/store_utils_test.go | 74 +++++++++++++++++++ pkg/flink/types/processed_statement.go | 25 ++++--- 10 files changed, 156 insertions(+), 23 deletions(-) delete mode 100644 pkg/flink/app/.snapshots/TestApplicationTestSuite-TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError diff --git a/.gitignore b/.gitignore index ce228be265..ca7feeb8bd 100644 --- 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 index 1edb330616..ce7f0d5694 100644 --- 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 index 21396d03e7..ff1dc0a236 100644 --- 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/pkg/flink/app/.snapshots/TestApplicationTestSuite-TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError b/pkg/flink/app/.snapshots/TestApplicationTestSuite-TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError deleted file mode 100644 index 8b13789179..0000000000 --- a/pkg/flink/app/.snapshots/TestApplicationTestSuite-TestReplAppendsStatementToHistoryAndStopsOnExecuteStatementError +++ /dev/null @@ -1 +0,0 @@ - diff --git a/pkg/flink/app/application.go b/pkg/flink/app/application.go index 29747d8c66..7f061cfe18 100644 --- 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 index b0cdf75a1b..1386a0845d 100644 --- 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 index 7d275149e6..5a1df9fa94 100644 --- 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 index 0d0cf1c48d..44a48849a4 100644 --- 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 index d32973034d..c201d82815 100644 --- 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 index d5be5883d8..510b0edcbc 100644 --- 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 {