Fix SensitiveKeys - broke when custom spec types were introduced (#714)

Only return truly sensitive keys. Introduce a test case to guard the
feature against breakage. The assertions were incomplete.

internal/owl/graph.go

@@ -27,9 +27,10 @@ const (
 )
 
 type atomicType struct {
-	typeName   string
-	typeObject *graphql.Object
-	resolveFn  graphql.FieldResolveFn
+	typeName      string
+	typeSensitive bool
+	typeObject    *graphql.Object
+	resolveFn     graphql.FieldResolveFn
 }
 
 var (
@@ -150,9 +151,10 @@ func registerAtomicType(name string, sensitive, mask bool, resolver graphql.Fiel
 	})
 
 	return &atomicType{
-		typeName:   name,
-		typeObject: typ,
-		resolveFn:  resolver,
+		typeName:      name,
+		typeSensitive: sensitive && mask,
+		typeObject:    typ,
+		resolveFn:     resolver,
 	}
 }
 
@@ -291,6 +293,7 @@ func resolveSensitiveKeys() graphql.FieldResolveFn {
 	return func(p graphql.ResolveParams) (interface{}, error) {
 		sensitive := SetVarItems{}
 		var opSet *OperationSet
+		var specOpSet *SpecOperationSet
 
 		switch p.Source.(type) {
 		case nil, string:
@@ -299,17 +302,41 @@ func resolveSensitiveKeys() graphql.FieldResolveFn {
 		case *OperationSet:
 			opSet = p.Source.(*OperationSet)
 		case *SpecOperationSet:
-			opSet = p.Source.(*SpecOperationSet).OperationSet
+			specOpSet = p.Source.(*SpecOperationSet)
+			opSet = specOpSet.OperationSet
 		default:
 			return nil, errors.New("source does not contain an OperationSet")
 		}
 
+		specDefs, ok := p.Context.Value(OwlEnvSpecDefsKey).(SpecDefs)
+		if !ok {
+			return nil, errors.New("missing specDefs in context")
+		}
+
+		sensitiveAtomics := make(map[string]bool)
+		for _, a := range AtomicTypes {
+			sensitiveAtomics[a.typeName] = a.typeSensitive
+		}
+
 		for _, v := range opSet.values {
 			s, ok := opSet.specs[v.Var.Key]
 			if !ok {
 				return nil, fmt.Errorf("missing spec for %s", v.Var.Key)
 			}
 
+			specName := s.Spec.Name
+
+			if specOpSet != nil {
+				_, aitem, err := specOpSet.GetAtomic(s, specDefs)
+				if err == nil {
+					specName = aitem.Spec.Name
+				}
+			}
+
+			if !sensitiveAtomics[specName] {
+				continue
+			}
+
 			item := &SetVarItem{
 				Var:  v.Var,
 				Spec: s.Spec,

internal/owl/graph_test.go

@@ -1,6 +1,7 @@
 package owl
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -81,10 +82,12 @@ func (testCases fileTestCases) runAll(t *testing.T) {
 				tc.pre(t, vars, &query)
 			}
 
+			noSpecDefs := SpecDefs{}
 			result := graphql.Do(graphql.Params{
 				Schema:         Schema,
 				RequestString:  string(query),
 				VariableValues: vars,
+				Context:        context.WithValue(context.Background(), OwlEnvSpecDefsKey, noSpecDefs),
 			})
 			require.False(t, result.HasErrors())
 
@@ -225,15 +228,31 @@ func TestGraph_SensitiveKeys(t *testing.T) {
 		{
 			name: "Sensitive keys",
 			post: func(t *testing.T, result *graphql.Result) {
-				render, err := extractDataKey(result.Data, "render")
+				sensitiveKeys, err := extractDataKey(result.Data, "sensitiveKeys")
 				require.NoError(t, err)
-				require.NotNil(t, render)
-
-				b, err := yaml.Marshal(render)
-				// b, err := json.MarshalIndent(result, "", " ")
-				require.NoError(t, err)
-				_, _ = fmt.Println(string(b))
-				require.NotNil(t, b)
+				require.NotNil(t, sensitiveKeys)
+
+				expected := []interface{}{
+					map[string]interface{}{
+						"spec": map[string]interface{}{
+							"name":     "Password",
+							"required": true,
+						},
+						"var": map[string]interface{}{
+							"key": "KRAFTCLOUD_TOKEN",
+						},
+					},
+					map[string]interface{}{
+						"spec": map[string]interface{}{
+							"name":     "Secret",
+							"required": true,
+						},
+						"var": map[string]interface{}{
+							"key": "OPENAI_API_KEY",
+						},
+					},
+				}
+				require.EqualValues(t, expected, sensitiveKeys)
 			},
 		},
 	}

internal/owl/store.go

@@ -178,9 +178,11 @@ func (res SetVarItems) getSensitiveKeys(data interface{}) ([]string, error) {
 
 	var filtered []string
 	for _, item := range res {
-		if specsSensitivity[item.Spec.Name] {
-			filtered = append(filtered, item.Var.Key)
-		}
+		// todo(sebastian): check sensitivity inside graph for now
+		// if specsSensitivity[item.Spec.Name] {
+		// 	filtered = append(filtered, item.Var.Key)
+		// }
+		filtered = append(filtered, item.Var.Key)
 	}
 
 	return filtered, nil

internal/owl/store_test.go

@@ -757,3 +757,22 @@ func TestStore_LoadEnvSpecDefs(t *testing.T) {
 		require.Equal(t, 0, errors)
 	})
 }
+
+func TestStore_InsecureValues(t *testing.T) {
+	t.Run("Custom", func(t *testing.T) {
+		store, err := NewStore(
+			WithSpecFile(".env.example", []byte(`SLACK_CLIENT_ID="Client ID for Slack" # Slack!
+SLACK_CLIENT_SECRET="Client Secret for Slack" # Slack!
+SLACK_REDIRECT_URL="Redirect URL for Slack. Use a tunnel with ngrok" # Slack!`)),
+			WithEnvFile(".env.local", []byte(`SLACK_CLIENT_ID=abc-123-xyz
+SLACK_CLIENT_SECRET=this-is-my-secret`)),
+			WithEnvFile(".env", []byte(`SLACK_REDIRECT_URL='https://my-redirect-url.com/slack/redirect'`)),
+		)
+		require.NoError(t, err)
+		require.NotNil(t, store)
+
+		keys, err := store.SensitiveKeys()
+		require.NoError(t, err)
+		require.EqualValues(t, []string{"SLACK_CLIENT_SECRET", "SLACK_REDIRECT_URL"}, keys)
+	})
+}