Browse Source

fix(release): Validate GCP GetSecret json format (#5336)

* chore/json-parser - Updated GCP GetSecret to validate json format

Signed-off-by: Gabriel Madrid <gabrielmadrid73@gmail.com>

* fix/gcp-json-parser - Updated GCP needUpdate according getDataByProperty changes

Signed-off-by: Gabriel Madrid <gabrielmadrid73@gmail.com>

* fix/gcp-json-parse - Added test to GCP client getDataByProperty

Signed-off-by: Gabriel Madrid <gabrielmadrid73@gmail.com>

---------

Signed-off-by: Gabriel Madrid <gabrielmadrid73@gmail.com>
Co-authored-by: Gergely Brautigam <skarlso777@gmail.com>
Gabriel Madrid 6 months ago
parent
commit
2bc24e93c2

+ 10 - 4
pkg/provider/gcp/secretmanager/client.go

@@ -525,7 +525,10 @@ func (c *Client) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemot
 		return nil, fmt.Errorf("invalid secret received. no secret string for key: %s", ref.Key)
 	}
 
-	val := getDataByProperty(result.Payload.Data, ref.Property)
+	val, err := getDataByProperty(result.Payload.Data, ref.Property)
+	if err != nil {
+		return nil, err
+	}
 	if !val.Exists() {
 		return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key)
 	}
@@ -660,7 +663,10 @@ func (c *Client) Validate() (esv1.ValidationResult, error) {
 	return esv1.ValidationResultReady, nil
 }
 
-func getDataByProperty(data []byte, property string) gjson.Result {
+func getDataByProperty(data []byte, property string) (gjson.Result, error) {
+	if !json.Valid(data) {
+		return gjson.Result{}, errors.New(errJSONSecretUnmarshal)
+	}
 	var payload string
 	if data != nil {
 		payload = string(data)
@@ -671,10 +677,10 @@ func getDataByProperty(data []byte, property string) gjson.Result {
 		refProperty = strings.ReplaceAll(refProperty, ".", "\\.")
 		val := gjson.Get(payload, refProperty)
 		if val.Exists() {
-			return val
+			return val, nil
 		}
 	}
-	return gjson.Get(payload, property)
+	return gjson.Get(payload, property), nil
 }
 
 func getName(projectID, location, key string) string {

+ 42 - 0
pkg/provider/gcp/secretmanager/client_test.go

@@ -1399,3 +1399,45 @@ func TestValidateStore(t *testing.T) {
 		})
 	}
 }
+
+func TestGetDataByProperty(t *testing.T) {
+	tests := []struct {
+		desc                          string
+		accessSecretVersionMockReturn fakesm.AccessSecretVersionMockReturn
+		ref                           *esv1.ExternalSecretDataRemoteRef
+		wantErr                       bool
+	}{
+		{
+			desc: "valid json",
+			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
+				Res: &secretmanagerpb.AccessSecretVersionResponse{
+					Payload: &secretmanagerpb.SecretPayload{
+						Data: []byte(`{"testKey1":{"testKey2":"testValue1"}}`),
+					},
+				},
+			},
+			ref:     makeValidRef(),
+			wantErr: false,
+		},
+		{
+			desc: "invalid json",
+			accessSecretVersionMockReturn: fakesm.AccessSecretVersionMockReturn{
+				Res: &secretmanagerpb.AccessSecretVersionResponse{
+					Payload: &secretmanagerpb.SecretPayload{
+						Data: []byte(`{"testKey1":{"testKey2":"testValue1"},}`),
+					},
+				},
+			},
+			ref:     makeValidRef(),
+			wantErr: true,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.desc, func(t *testing.T) {
+			_, err := getDataByProperty(tc.accessSecretVersionMockReturn.Res.Payload.Data, tc.ref.Property)
+			if (err != nil) != tc.wantErr {
+				t.Errorf("getDataByProperty() error = %v, wantErr %v", err, tc.wantErr)
+			}
+		})
+	}
+}

+ 1 - 1
pkg/provider/gcp/secretmanager/push_secret.go

@@ -146,7 +146,7 @@ func (b *propertyPSBuilder) needUpdate(original []byte) bool {
 		return true
 	}
 
-	val := getDataByProperty(original, b.pushSecretData.GetProperty())
+	val, _ := getDataByProperty(original, b.pushSecretData.GetProperty())
 	return !val.Exists() || val.String() != string(b.payload)
 }