From 63cb540e77344a66abb7bce201083ad68fdacaa8 Mon Sep 17 00:00:00 2001 From: sk593 Date: Tue, 6 Jan 2026 15:31:22 -0800 Subject: [PATCH 01/10] add filter for schema annotations Signed-off-by: sk593 --- pkg/dynamicrp/frontend/updatefilter.go | 71 +++++ pkg/dynamicrp/frontend/updatefilter_test.go | 214 +++++++++++++ pkg/dynamicrp/schema/annotations.go | 114 +++++++ pkg/dynamicrp/schema/annotations_test.go | 318 ++++++++++++++++++++ 4 files changed, 717 insertions(+) create mode 100644 pkg/dynamicrp/frontend/updatefilter.go create mode 100644 pkg/dynamicrp/frontend/updatefilter_test.go create mode 100644 pkg/dynamicrp/schema/annotations.go create mode 100644 pkg/dynamicrp/schema/annotations_test.go diff --git a/pkg/dynamicrp/frontend/updatefilter.go b/pkg/dynamicrp/frontend/updatefilter.go new file mode 100644 index 0000000000..b8e58ff228 --- /dev/null +++ b/pkg/dynamicrp/frontend/updatefilter.go @@ -0,0 +1,71 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "context" + + "github.com/radius-project/radius/pkg/armrpc/frontend/controller" + "github.com/radius-project/radius/pkg/armrpc/rest" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/dynamicrp/schema" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +// UpdateFilterFactory creates an UpdateFilter that has access to the UCP client for schema lookups. +type UpdateFilterFactory struct { + UCPClient *v20231001preview.ClientFactory +} + +// NewPrepareResourceFilter returns an UpdateFilter function that can access the UCP client. +func (f *UpdateFilterFactory) NewPrepareResourceFilter() controller.UpdateFilter[datamodel.DynamicResource] { + return func(ctx context.Context, newResource, oldResource *datamodel.DynamicResource, opt *controller.Options) (rest.Response, error) { + return f.prepareResource(ctx, newResource, oldResource, opt) + } +} + +// prepareResource extracts sensitive field paths from the schema during PUT/PATCH operations. +// The backend will independently call the same schema function when it needs the paths. +func (f *UpdateFilterFactory) prepareResource(ctx context.Context, newResource, _ *datamodel.DynamicResource, _ *controller.Options) (rest.Response, error) { + logger := ucplog.FromContextOrDiscard(ctx) + logger.Info("PrepareResource filter executing for dynamic resource", "resourceID", newResource.ID) + + // Extract sensitive field paths from the schema + // This is called here in the frontend; the backend will call the same function independently + sensitiveFieldPaths, err := schema.GetSensitiveFieldPaths( + ctx, + f.UCPClient, + newResource.ID, + newResource.Type, + newResource.InternalMetadata.UpdatedAPIVersion, + ) + if err != nil { + logger.Error(err, "Failed to get sensitive field paths from schema", "resourceID", newResource.ID) + // Continue processing even if we can't get the schema - don't fail the request + } + + if len(sensitiveFieldPaths) > 0 { + logger.Info("Found sensitive fields in schema", "resourceID", newResource.ID, "paths", sensitiveFieldPaths) + } + + // Note: We intentionally do NOT store the paths on the resource. + // The backend will call schema.GetSensitiveFieldPaths() independently when needed. + + // TODO: We need to update this to return or save the data from sensitive fields once we make changes to the frontend code. + return nil, nil +} diff --git a/pkg/dynamicrp/frontend/updatefilter_test.go b/pkg/dynamicrp/frontend/updatefilter_test.go new file mode 100644 index 0000000000..ec9ab8e3c6 --- /dev/null +++ b/pkg/dynamicrp/frontend/updatefilter_test.go @@ -0,0 +1,214 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "context" + "net/http" + "testing" + + armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" + azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/armrpc/frontend/controller" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" + "github.com/stretchr/testify/require" +) + +func TestPrepareResourceFilter(t *testing.T) { + ctx := context.Background() + + t.Run("succeeds with sensitive fields", func(t *testing.T) { + clientFactory, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "host": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) + require.NoError(t, err) + + factory := &UpdateFilterFactory{ + UCPClient: clientFactory, + } + + filter := factory.NewPrepareResourceFilter() + + resource := &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + Type: "Foo.Bar/myResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: "2024-01-01", + }, + }, + Properties: map[string]any{}, + } + + resp, err := filter(ctx, resource, nil, &controller.Options{}) + + require.NoError(t, err) + require.Nil(t, resp) + }) + + t.Run("succeeds with no sensitive fields", func(t *testing.T) { + clientFactory, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "host": map[string]any{ + "type": "string", + }, + }, + }) + require.NoError(t, err) + + factory := &UpdateFilterFactory{ + UCPClient: clientFactory, + } + + filter := factory.NewPrepareResourceFilter() + + resource := &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + Type: "Foo.Bar/myResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: "2024-01-01", + }, + }, + Properties: map[string]any{}, + } + + resp, err := filter(ctx, resource, nil, &controller.Options{}) + + require.NoError(t, err) + require.Nil(t, resp) + }) + + t.Run("succeeds with nil UCP client", func(t *testing.T) { + factory := &UpdateFilterFactory{ + UCPClient: nil, + } + + filter := factory.NewPrepareResourceFilter() + + resource := &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + Type: "Foo.Bar/myResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: "2024-01-01", + }, + }, + Properties: map[string]any{}, + } + + resp, err := filter(ctx, resource, nil, &controller.Options{}) + + require.NoError(t, err) + require.Nil(t, resp) + }) + + t.Run("continues on schema fetch error", func(t *testing.T) { + // Create a client that will fail to fetch the schema + clientFactory, err := testUCPClientFactoryWithError() + require.NoError(t, err) + + factory := &UpdateFilterFactory{ + UCPClient: clientFactory, + } + + filter := factory.NewPrepareResourceFilter() + + resource := &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + Type: "Foo.Bar/myResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: "2024-01-01", + }, + }, + Properties: map[string]any{}, + } + + // Should not fail even if schema fetch fails + resp, err := filter(ctx, resource, nil, &controller.Options{}) + + require.NoError(t, err) + require.Nil(t, resp) + }) +} + +// testUCPClientFactory creates a mock UCP client factory that returns the given schema. +func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { + apiVersionsServer := fake.APIVersionsServer{ + Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { + resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} + resp.SetResponse(http.StatusOK, v20231001preview.APIVersionsClientGetResponse{ + APIVersionResource: v20231001preview.APIVersionResource{ + Properties: &v20231001preview.APIVersionProperties{ + Schema: schema, + }, + }, + }, nil) + return resp, azfake.ErrorResponder{} + }, + } + + return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ + APIVersionsServer: apiVersionsServer, + }), + }, + }) +} + +// testUCPClientFactoryWithError creates a mock UCP client factory that returns an error. +func testUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { + apiVersionsServer := fake.APIVersionsServer{ + Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { + resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} + errResp := azfake.ErrorResponder{} + errResp.SetResponseError(http.StatusNotFound, "NotFound") + return resp, errResp + }, + } + + return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ + APIVersionsServer: apiVersionsServer, + }), + }, + }) +} diff --git a/pkg/dynamicrp/schema/annotations.go b/pkg/dynamicrp/schema/annotations.go new file mode 100644 index 0000000000..dec88e4541 --- /dev/null +++ b/pkg/dynamicrp/schema/annotations.go @@ -0,0 +1,114 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schema + +import ( + "context" + "strings" + + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/resources" +) + +const ( + // XRadiusSensitiveAnnotation is the annotation key used to mark fields as sensitive in the resource schema. + XRadiusSensitiveAnnotation = "x-radius-sensitive" +) + +// GetSensitiveFieldPaths fetches the schema for a resource and returns paths to fields marked with x-radius-sensitive. +// Paths are in dot notation, e.g., "credentials.password" or "config.apiKey". +// +// Parameters: +// - ctx: The request context +// - ucpClient: UCP client factory for fetching the schema +// - resourceID: The full resource ID (e.g., "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test") +// - resourceType: The resource type (e.g., "Foo.Bar/myResources") +// - apiVersion: The API version to fetch the schema for +// +// Returns: +// - []string: Paths to sensitive fields, or empty slice if none found +// - error: Any error encountered while fetching the schema +func GetSensitiveFieldPaths(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) ([]string, error) { + if ucpClient == nil { + return nil, nil + } + + // Parse the resource ID to get plane information + ID, err := resources.Parse(resourceID) + if err != nil { + return nil, err + } + + plane := ID.PlaneNamespace() + planeName := strings.Split(plane, "/")[1] + resourceProvider := strings.Split(resourceType, "/")[0] + resourceTypeName := strings.Split(resourceType, "/")[1] + + // Fetch the API version resource which contains the schema + apiVersionResource, err := ucpClient.NewAPIVersionsClient().Get(ctx, planeName, resourceProvider, resourceTypeName, apiVersion, nil) + if err != nil { + return nil, err + } + + schema := apiVersionResource.APIVersionResource.Properties.Schema + if schema == nil { + return nil, nil + } + + // Extract paths to fields with x-radius-sensitive annotation + return ExtractSensitiveFieldPaths(schema, ""), nil +} + +// ExtractSensitiveFieldPaths recursively walks the schema and returns paths to fields marked with x-radius-sensitive. +// The prefix parameter builds up the path as we traverse nested objects. +func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { + var paths []string + + properties, ok := schema["properties"].(map[string]any) + if !ok { + return paths + } + + for fieldName, fieldSchema := range properties { + fieldSchemaMap, ok := fieldSchema.(map[string]any) + if !ok { + continue + } + + // Build the full path for this field + var fullPath string + if prefix == "" { + fullPath = fieldName + } else { + fullPath = prefix + "." + fieldName + } + + // Check if this field has the x-radius-sensitive annotation + if isSensitive, ok := fieldSchemaMap[XRadiusSensitiveAnnotation].(bool); ok && isSensitive { + paths = append(paths, fullPath) + } + + // Recursively check nested objects + if nestedProps, ok := fieldSchemaMap["properties"].(map[string]any); ok { + nestedSchema := map[string]any{"properties": nestedProps} + nestedPaths := ExtractSensitiveFieldPaths(nestedSchema, fullPath) + paths = append(paths, nestedPaths...) + } + } + + return paths +} diff --git a/pkg/dynamicrp/schema/annotations_test.go b/pkg/dynamicrp/schema/annotations_test.go new file mode 100644 index 0000000000..03ec6fbdac --- /dev/null +++ b/pkg/dynamicrp/schema/annotations_test.go @@ -0,0 +1,318 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schema + +import ( + "context" + "net/http" + "testing" + + armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" + azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" + "github.com/stretchr/testify/require" +) + +func TestExtractSensitiveFieldPaths(t *testing.T) { + tests := []struct { + name string + schema map[string]any + expected []string + }{ + { + name: "empty schema", + schema: map[string]any{}, + expected: []string{}, + }, + { + name: "no sensitive fields", + schema: map[string]any{ + "properties": map[string]any{ + "name": map[string]any{ + "type": "string", + }, + "port": map[string]any{ + "type": "integer", + }, + }, + }, + expected: []string{}, + }, + { + name: "single sensitive field", + schema: map[string]any{ + "properties": map[string]any{ + "name": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + }, + }, + expected: []string{"password"}, + }, + { + name: "multiple sensitive fields", + schema: map[string]any{ + "properties": map[string]any{ + "username": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + "apiKey": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + }, + }, + expected: []string{"password", "apiKey"}, + }, + { + name: "nested sensitive field", + schema: map[string]any{ + "properties": map[string]any{ + "name": map[string]any{ + "type": "string", + }, + "credentials": map[string]any{ + "type": "object", + "properties": map[string]any{ + "username": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + }, + }, + }, + }, + expected: []string{"credentials.password"}, + }, + { + name: "deeply nested sensitive fields", + schema: map[string]any{ + "properties": map[string]any{ + "config": map[string]any{ + "type": "object", + "properties": map[string]any{ + "database": map[string]any{ + "type": "object", + "properties": map[string]any{ + "connectionString": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + "host": map[string]any{ + "type": "string", + }, + }, + }, + }, + }, + }, + }, + expected: []string{"config.database.connectionString"}, + }, + { + name: "mixed sensitive fields at different levels", + schema: map[string]any{ + "properties": map[string]any{ + "apiKey": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + "settings": map[string]any{ + "type": "object", + "properties": map[string]any{ + "token": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + "endpoint": map[string]any{ + "type": "string", + }, + }, + }, + }, + }, + expected: []string{"apiKey", "settings.token"}, + }, + { + name: "sensitive annotation set to false", + schema: map[string]any{ + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: false, + }, + }, + }, + expected: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ExtractSensitiveFieldPaths(tt.schema, "") + + // Sort both slices for comparison since map iteration order is not guaranteed + require.ElementsMatch(t, tt.expected, result) + }) + } +} + +func TestExtractSensitiveFieldPaths_WithPrefix(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + }, + } + + result := ExtractSensitiveFieldPaths(schema, "parent") + + require.Equal(t, []string{"parent.secret"}, result) +} + +func TestGetSensitiveFieldPaths(t *testing.T) { + ctx := context.Background() + + t.Run("nil client returns nil", func(t *testing.T) { + result, err := GetSensitiveFieldPaths(ctx, nil, "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", "Foo.Bar/myResources", "2024-01-01") + require.NoError(t, err) + require.Nil(t, result) + }) + + t.Run("extracts sensitive fields from schema", func(t *testing.T) { + clientFactory, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "host": map[string]any{ + "type": "string", + }, + "password": map[string]any{ + "type": "string", + XRadiusSensitiveAnnotation: true, + }, + }, + }) + require.NoError(t, err) + + result, err := GetSensitiveFieldPaths( + ctx, + clientFactory, + "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + "Foo.Bar/myResources", + "2024-01-01", + ) + + require.NoError(t, err) + require.Equal(t, []string{"password"}, result) + }) + + t.Run("returns empty for schema with no sensitive fields", func(t *testing.T) { + clientFactory, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "host": map[string]any{ + "type": "string", + }, + "port": map[string]any{ + "type": "integer", + }, + }, + }) + require.NoError(t, err) + + result, err := GetSensitiveFieldPaths( + ctx, + clientFactory, + "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + "Foo.Bar/myResources", + "2024-01-01", + ) + + require.NoError(t, err) + require.Empty(t, result) + }) + + t.Run("returns nil for nil schema", func(t *testing.T) { + clientFactory, err := testUCPClientFactory(nil) + require.NoError(t, err) + + result, err := GetSensitiveFieldPaths( + ctx, + clientFactory, + "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + "Foo.Bar/myResources", + "2024-01-01", + ) + + require.NoError(t, err) + require.Nil(t, result) + }) +} + +// testUCPClientFactory creates a mock UCP client factory that returns the given schema. +func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { + apiVersionsServer := fake.APIVersionsServer{ + Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { + resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} + resp.SetResponse(http.StatusOK, v20231001preview.APIVersionsClientGetResponse{ + APIVersionResource: v20231001preview.APIVersionResource{ + Properties: &v20231001preview.APIVersionProperties{ + Schema: schema, + }, + }, + }, nil) + return resp, azfake.ErrorResponder{} + }, + } + + return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ + APIVersionsServer: apiVersionsServer, + }), + }, + }) +} + +func TestGetSensitiveFieldPaths_InvalidResourceID(t *testing.T) { + clientFactory, err := testUCPClientFactory(nil) + require.NoError(t, err) + + _, err = GetSensitiveFieldPaths( + context.Background(), + clientFactory, + "invalid-resource-id", + "Foo.Bar/myResources", + "2024-01-01", + ) + + require.Error(t, err) +} From 0cbbf3af81381e8b4399d4df023efc30e410f026 Mon Sep 17 00:00:00 2001 From: sk593 Date: Fri, 6 Feb 2026 09:16:17 -0800 Subject: [PATCH 02/10] add decryption and redaction logic Signed-off-by: sk593 --- .../dev/radius_security.yaml | 1 + .../self-hosted/radius_security.yaml | 1 + pkg/crypto/encryption/sensitive.go | 28 + pkg/crypto/encryption/sensitive_test.go | 152 +++++ pkg/dynamicrp/frontend/getresource.go | 108 +++ pkg/dynamicrp/frontend/getresource_test.go | 186 +++++ pkg/dynamicrp/frontend/listresources.go | 123 ++++ pkg/dynamicrp/frontend/routes.go | 6 +- pkg/dynamicrp/frontend/updatefilter.go | 2 +- pkg/dynamicrp/frontend/updatefilter_test.go | 6 +- pkg/dynamicrp/schema/annotations.go | 114 ---- pkg/dynamicrp/schema/annotations_test.go | 318 --------- .../controller/createorupdateresource.go | 150 ++++- .../controller/createorupdateresource_test.go | 635 ++++++++++++++++++ pkg/schema/annotations.go | 22 +- pkg/schema/annotations_test.go | 35 + 16 files changed, 1434 insertions(+), 453 deletions(-) create mode 100644 pkg/dynamicrp/frontend/getresource.go create mode 100644 pkg/dynamicrp/frontend/getresource_test.go create mode 100644 pkg/dynamicrp/frontend/listresources.go delete mode 100644 pkg/dynamicrp/schema/annotations.go delete mode 100644 pkg/dynamicrp/schema/annotations_test.go diff --git a/deploy/manifest/built-in-providers/dev/radius_security.yaml b/deploy/manifest/built-in-providers/dev/radius_security.yaml index 1e0dc64539..cf31a19e7b 100644 --- a/deploy/manifest/built-in-providers/dev/radius_security.yaml +++ b/deploy/manifest/built-in-providers/dev/radius_security.yaml @@ -100,6 +100,7 @@ types: data: type: object description: '(Required) Map of secret data. For example: `data: { username: { value: user1 } password: { value: pass }}`' + x-radius-sensitive: true additionalProperties: type: object properties: diff --git a/deploy/manifest/built-in-providers/self-hosted/radius_security.yaml b/deploy/manifest/built-in-providers/self-hosted/radius_security.yaml index 2db297cb08..0d8f4877ca 100644 --- a/deploy/manifest/built-in-providers/self-hosted/radius_security.yaml +++ b/deploy/manifest/built-in-providers/self-hosted/radius_security.yaml @@ -100,6 +100,7 @@ types: data: type: object description: '(Required) Map of secret data. For example: `data: { username: { value: user1 } password: { value: pass }}`' + x-radius-sensitive: true additionalProperties: type: object properties: diff --git a/pkg/crypto/encryption/sensitive.go b/pkg/crypto/encryption/sensitive.go index 9e8016f161..55235e7711 100644 --- a/pkg/crypto/encryption/sensitive.go +++ b/pkg/crypto/encryption/sensitive.go @@ -37,6 +37,9 @@ var ( // ErrFieldDecryptionFailed is returned when decryption of a field fails. ErrFieldDecryptionFailed = errors.New("field decryption failed") + + // ErrFieldRedactionFailed is returned when redaction of a field fails. + ErrFieldRedactionFailed = errors.New("field redaction failed") ) // SensitiveDataHandler provides methods for encrypting and decrypting sensitive fields @@ -160,6 +163,23 @@ func (h *SensitiveDataHandler) DecryptSensitiveFieldsWithSchema(ctx context.Cont return nil } +// RedactSensitiveFields nullifies all sensitive fields in the data based on the provided field paths. +// The data is modified in place. Field paths support dot notation and [*] for arrays/maps. +// +// Fields that are not found are skipped - this allows optional sensitive fields to be absent. +func (h *SensitiveDataHandler) RedactSensitiveFields(data map[string]any, sensitiveFieldPaths []string) error { + for _, path := range sensitiveFieldPaths { + if err := h.redactFieldAtPath(data, path); err != nil { + // Skip fields that are not found - they may not exist in this resource instance + if errors.Is(err, ErrFieldNotFound) { + continue + } + return fmt.Errorf("%w: path %q: %v", ErrFieldRedactionFailed, path, err) + } + } + return nil +} + // getEncryptorForDecryption returns the appropriate encryptor for decrypting data. // If a keyProvider is available and the data contains a version, it fetches the versioned key. // Otherwise, it falls back to the default encryptor. @@ -206,6 +226,14 @@ func (h *SensitiveDataHandler) decryptFieldAtPath(ctx context.Context, data map[ return h.processFieldAtPath(data, path, processor) } +// redactFieldAtPath replaces the value at the given field path with nil. +func (h *SensitiveDataHandler) redactFieldAtPath(data map[string]any, path string) error { + processor := func(any) (any, error) { + return nil, nil + } + return h.processFieldAtPath(data, path, processor) +} + // processFieldAtPath traverses the data structure and applies the processor function to the field at the path. func (h *SensitiveDataHandler) processFieldAtPath(data map[string]any, path string, processor func(any) (any, error)) error { if path == "" { diff --git a/pkg/crypto/encryption/sensitive_test.go b/pkg/crypto/encryption/sensitive_test.go index 990656cb93..70e0ae0fed 100644 --- a/pkg/crypto/encryption/sensitive_test.go +++ b/pkg/crypto/encryption/sensitive_test.go @@ -151,6 +151,30 @@ func TestSensitiveDataHandler_EncryptDecrypt_SimpleField(t *testing.T) { require.Equal(t, "admin", data["username"]) } +func TestSensitiveDataHandler_RedactSensitiveFields(t *testing.T) { + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + + data := map[string]any{ + "username": "admin", + "credentials": map[string]any{ + "password": "nested-secret", + "token": "token-123", + }, + } + + err = handler.RedactSensitiveFields(data, []string{"credentials.password", "credentials.token"}) + require.NoError(t, err) + + creds := data["credentials"].(map[string]any) + require.Nil(t, creds["password"]) + require.Nil(t, creds["token"]) + require.Equal(t, "admin", data["username"]) +} + func TestSensitiveDataHandler_EncryptDecrypt_NestedField(t *testing.T) { key, err := GenerateKey() require.NoError(t, err) @@ -1003,6 +1027,134 @@ func TestSensitiveDataHandler_DecryptWithMissingKeyVersion(t *testing.T) { require.Contains(t, err.Error(), "key version not found") } + +func TestSensitiveDataHandler_DecryptWithADMismatch(t *testing.T) { + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + + originalResourceID := "/planes/radius/local/resourceGroups/test/providers/Test.Resource/testResources/original" + attackerResourceID := "/planes/radius/local/resourceGroups/test/providers/Test.Resource/testResources/attacker" + + data := map[string]any{ + "secret": "cross-resource-attack-value", + } + + // Encrypt with the original resource ID as associated data + err = handler.EncryptSensitiveFields(data, []string{"secret"}, originalResourceID) + require.NoError(t, err) + + // Verify the field is encrypted + _, isEncrypted := data["secret"].(map[string]any) + require.True(t, isEncrypted, "secret should be encrypted") + + // Attempt to decrypt with a DIFFERENT resource ID — the AD hash will not match + // and ChaCha20-Poly1305 authentication will reject the ciphertext. + err = handler.DecryptSensitiveFields(context.Background(), data, []string{"secret"}, attackerResourceID) + require.Error(t, err, "decryption must fail when resource ID does not match") + require.ErrorIs(t, err, ErrFieldDecryptionFailed) + + // Verify the encrypted data was NOT mutated by the failed decryption attempt + _, stillEncrypted := data["secret"].(map[string]any) + require.True(t, stillEncrypted, "encrypted data must remain intact after failed decryption") +} + +func TestSensitiveDataHandler_FullDecryptRedactWorkflow(t *testing.T) { + // Validates the security-critical backend data flow: + // 1. Encrypt fields (simulates frontend write to DB) + // 2. Deep copy for recipe properties + // 3. Decrypt the recipe copy (in-memory only) + // 4. Derive redacted copy from the ORIGINAL encrypted data — never from decrypted + // 5. Verify: redacted copy has nil, recipe copy has plaintext, + // original encrypted data is untouched + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + + // Simulate DB state: resource with two sensitive fields at different nesting levels + dbProperties := map[string]any{ + "name": "my-resource", + "secret": "top-secret-value", + "nested": map[string]any{ + "password": "nested-password", + "host": "localhost", + }, + } + + sensitivePaths := []string{"secret", "nested.password"} + + // Step 1: Encrypt (simulates frontend) + err = handler.EncryptSensitiveFields(dbProperties, sensitivePaths, testResourceID) + require.NoError(t, err) + + // Step 2: Deep copy for recipe (simulates controller deep copy before decryption) + recipeProperties := deepCopyMap(dbProperties) + + // Step 3: Decrypt the recipe copy — in-memory only + err = handler.DecryptSensitiveFields(context.Background(), recipeProperties, sensitivePaths, testResourceID) + require.NoError(t, err) + + // Verify recipe properties have the decrypted plaintext values + require.Equal(t, "top-secret-value", recipeProperties["secret"]) + require.Equal(t, "nested-password", recipeProperties["nested"].(map[string]any)["password"]) + // Non-sensitive field preserved + require.Equal(t, "localhost", recipeProperties["nested"].(map[string]any)["host"]) + + // Step 4: Derive redacted copy from the ORIGINAL dbProperties (security-critical). + // If this were derived from recipeProperties (decrypted), a partial redaction + // failure would persist plaintext to the database. + redactedProperties := deepCopyMap(dbProperties) + err = handler.RedactSensitiveFields(redactedProperties, sensitivePaths) + require.NoError(t, err) + + // Step 5: Verify all invariants + // — Redacted copy: sensitive fields are nil, non-sensitive fields are preserved + require.Nil(t, redactedProperties["secret"], "redacted secret must be nil") + require.Nil(t, redactedProperties["nested"].(map[string]any)["password"], "redacted nested.password must be nil") + require.Equal(t, "my-resource", redactedProperties["name"]) + require.Equal(t, "localhost", redactedProperties["nested"].(map[string]any)["host"]) + + // — Original dbProperties: still contain encrypted ciphertext, not modified + _, secretStillEncrypted := dbProperties["secret"].(map[string]any) + require.True(t, secretStillEncrypted, "original dbProperties must still hold encrypted secret") + _, passwordStillEncrypted := dbProperties["nested"].(map[string]any)["password"].(map[string]any) + require.True(t, passwordStillEncrypted, "original dbProperties must still hold encrypted nested.password") + + // — Recipe properties: decrypted values are untouched by the redaction of the separate copy + require.Equal(t, "top-secret-value", recipeProperties["secret"]) + require.Equal(t, "nested-password", recipeProperties["nested"].(map[string]any)["password"]) +} + +func TestSensitiveDataHandler_Redact_AlreadyNilField(t *testing.T) { + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + + // Field exists in the map but is already nil — e.g. previously redacted or + // an optional sensitive field the user did not provide. + data := map[string]any{ + "secret": nil, + "username": "admin", + } + + // Redacting an already-nil field must succeed silently + err = handler.RedactSensitiveFields(data, []string{"secret"}) + require.NoError(t, err) + require.Nil(t, data["secret"]) + require.Equal(t, "admin", data["username"]) + + // Verify idempotence: redacting again is still a no-op + err = handler.RedactSensitiveFields(data, []string{"secret"}) + require.NoError(t, err) + require.Nil(t, data["secret"]) +} + // Helper function to deep copy a map for testing func deepCopyMap(original map[string]any) map[string]any { result := make(map[string]any) diff --git a/pkg/dynamicrp/frontend/getresource.go b/pkg/dynamicrp/frontend/getresource.go new file mode 100644 index 0000000000..e728cc33d7 --- /dev/null +++ b/pkg/dynamicrp/frontend/getresource.go @@ -0,0 +1,108 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "context" + "net/http" + + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + ctrl "github.com/radius-project/radius/pkg/armrpc/frontend/controller" + "github.com/radius-project/radius/pkg/armrpc/rest" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/schema" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +// GetResourceWithRedaction is a custom GET controller that redacts sensitive fields. +type GetResourceWithRedaction struct { + ctrl.Operation[*datamodel.DynamicResource, datamodel.DynamicResource] + ucpClient *v20231001preview.ClientFactory +} + +// NewGetResourceWithRedaction creates a new GetResourceWithRedaction controller. +func NewGetResourceWithRedaction( + opts ctrl.Options, + resourceOpts ctrl.ResourceOptions[datamodel.DynamicResource], + ucpClient *v20231001preview.ClientFactory, +) (ctrl.Controller, error) { + return &GetResourceWithRedaction{ + Operation: ctrl.NewOperation[*datamodel.DynamicResource](opts, resourceOpts), + ucpClient: ucpClient, + }, nil +} + +// Run returns the requested resource with sensitive fields redacted. +func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (rest.Response, error) { + serviceCtx := v1.ARMRequestContextFromContext(ctx) + logger := ucplog.FromContextOrDiscard(ctx) + + resource, etag, err := c.GetResource(ctx, serviceCtx.ResourceID) + if err != nil { + return nil, err + } + if resource == nil { + return rest.NewNotFoundResponse(serviceCtx.ResourceID), nil + } + + // Redact sensitive fields before returning the response + if resource.Properties != nil { + resourceID := serviceCtx.ResourceID.String() + resourceType := serviceCtx.ResourceID.Type() + apiVersion := serviceCtx.APIVersion + + sensitiveFieldPaths, err := schema.GetSensitiveFieldPaths( + ctx, + c.ucpClient, + resourceID, + resourceType, + apiVersion, + ) + if err != nil { + logger.Error(err, "Failed to fetch sensitive field paths for GET redaction", + "resourceType", resourceType, "apiVersion", apiVersion) + // Continue without redaction on error - don't fail the GET + } else if len(sensitiveFieldPaths) > 0 { + // Redact sensitive fields by setting them to nil + for _, path := range sensitiveFieldPaths { + redactField(resource.Properties, path) + } + logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in GET response", + "count", len(sensitiveFieldPaths), "resourceType", resourceType) + } + } + + return c.ConstructSyncResponse(ctx, req.Method, etag, resource) +} + +// redactField sets the field at the given path to nil. +// Supports simple field names like "data" or nested paths like "config.password". +func redactField(properties map[string]any, path string) { + if properties == nil { + return + } + + // For simple paths (no dots), just set to nil + if _, exists := properties[path]; exists { + properties[path] = nil + return + } + + // For nested paths, we would need to traverse - but for now we only support top-level + // The "data" field is top-level in Radius.Security/secrets +} diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go new file mode 100644 index 0000000000..ad2e9b35e8 --- /dev/null +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -0,0 +1,186 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRedactField_SimpleField(t *testing.T) { + // Test redacting a simple top-level field + properties := map[string]any{ + "name": "test-resource", + "password": "secret123", + "data": map[string]any{"key": "value"}, + } + + redactField(properties, "password") + + require.Equal(t, "test-resource", properties["name"]) + require.Nil(t, properties["password"]) + require.NotNil(t, properties["data"]) +} + +func TestRedactField_DataField(t *testing.T) { + // Test redacting the "data" field (common pattern for secrets) + properties := map[string]any{ + "environment": "/planes/radius/local/resourcegroups/default/providers/Radius.Core/environments/test", + "application": "/planes/radius/local/resourcegroups/default/providers/Applications.Core/applications/test", + "data": map[string]any{ + "password": map[string]any{ + "value": "secret123", + "encoding": "string", + }, + "apiKey": map[string]any{ + "value": "my-api-key", + }, + }, + } + + redactField(properties, "data") + + require.NotNil(t, properties["environment"]) + require.NotNil(t, properties["application"]) + require.Nil(t, properties["data"]) +} + +func TestRedactField_NonExistentField(t *testing.T) { + // Test that redacting a non-existent field doesn't cause errors + properties := map[string]any{ + "name": "test-resource", + "value": "test-value", + } + + // Should not panic or error + redactField(properties, "nonexistent") + + // Original fields should remain unchanged + require.Equal(t, "test-resource", properties["name"]) + require.Equal(t, "test-value", properties["value"]) +} + +func TestRedactField_NilProperties(t *testing.T) { + // Test that nil properties don't cause panic + var properties map[string]any + + // Should not panic + redactField(properties, "anyfield") +} + +func TestRedactField_EmptyProperties(t *testing.T) { + // Test redacting from empty properties + properties := map[string]any{} + + redactField(properties, "password") + + require.Empty(t, properties) +} + +func TestRedactField_MultipleFields(t *testing.T) { + // Test redacting multiple fields sequentially + properties := map[string]any{ + "name": "test", + "password": "secret", + "apiKey": "key123", + "data": "sensitive-data", + } + + redactField(properties, "password") + redactField(properties, "apiKey") + redactField(properties, "data") + + require.Equal(t, "test", properties["name"]) + require.Nil(t, properties["password"]) + require.Nil(t, properties["apiKey"]) + require.Nil(t, properties["data"]) +} + +func TestRedactField_NestedFieldNotSupported(t *testing.T) { + // Test that nested paths (with dots) are not currently supported + // The redactField function currently only handles top-level fields + properties := map[string]any{ + "config": map[string]any{ + "password": "secret", + }, + } + + // Nested path - should not redact since we only support top-level + redactField(properties, "config.password") + + // Config should still contain the password (nested redaction not supported) + config, ok := properties["config"].(map[string]any) + require.True(t, ok) + require.Equal(t, "secret", config["password"]) +} + +func TestRedactField_FieldWithNilValue(t *testing.T) { + // Test redacting a field that already has nil value + properties := map[string]any{ + "name": "test", + "password": nil, + } + + redactField(properties, "password") + + require.Equal(t, "test", properties["name"]) + require.Nil(t, properties["password"]) +} + +func TestRedactField_FieldWithDifferentTypes(t *testing.T) { + // Test redacting fields of various types + testCases := []struct { + name string + value any + fieldName string + properties map[string]any + }{ + { + name: "string field", + fieldName: "secret", + properties: map[string]any{"secret": "password123"}, + }, + { + name: "map field", + fieldName: "data", + properties: map[string]any{"data": map[string]any{"key": "value"}}, + }, + { + name: "slice field", + fieldName: "tokens", + properties: map[string]any{"tokens": []string{"token1", "token2"}}, + }, + { + name: "int field", + fieldName: "pin", + properties: map[string]any{"pin": 1234}, + }, + { + name: "bool field", + fieldName: "enabled", + properties: map[string]any{"enabled": true}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + redactField(tc.properties, tc.fieldName) + require.Nil(t, tc.properties[tc.fieldName]) + }) + } +} diff --git a/pkg/dynamicrp/frontend/listresources.go b/pkg/dynamicrp/frontend/listresources.go new file mode 100644 index 0000000000..12dcf4c4a4 --- /dev/null +++ b/pkg/dynamicrp/frontend/listresources.go @@ -0,0 +1,123 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "context" + "net/http" + + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + ctrl "github.com/radius-project/radius/pkg/armrpc/frontend/controller" + "github.com/radius-project/radius/pkg/armrpc/rest" + "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/schema" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +// ListResourcesWithRedaction is a custom LIST controller that redacts sensitive fields. +type ListResourcesWithRedaction struct { + ctrl.Operation[*datamodel.DynamicResource, datamodel.DynamicResource] + ucpClient *v20231001preview.ClientFactory + listRecursiveQuery bool +} + +// NewListResourcesWithRedaction creates a new ListResourcesWithRedaction controller. +func NewListResourcesWithRedaction( + opts ctrl.Options, + resourceOpts ctrl.ResourceOptions[datamodel.DynamicResource], + ucpClient *v20231001preview.ClientFactory, +) (ctrl.Controller, error) { + return &ListResourcesWithRedaction{ + Operation: ctrl.NewOperation[*datamodel.DynamicResource](opts, resourceOpts), + ucpClient: ucpClient, + listRecursiveQuery: resourceOpts.ListRecursiveQuery, + }, nil +} + +// Run returns the list of resources with sensitive fields redacted. +func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (rest.Response, error) { + serviceCtx := v1.ARMRequestContextFromContext(ctx) + logger := ucplog.FromContextOrDiscard(ctx) + + query := database.Query{ + RootScope: serviceCtx.ResourceID.RootScope(), + ResourceType: serviceCtx.ResourceID.Type(), + ScopeRecursive: c.listRecursiveQuery, + } + + result, err := c.DatabaseClient().Query(ctx, query, database.WithPaginationToken(serviceCtx.SkipToken), database.WithMaxQueryItemCount(serviceCtx.Top)) + if err != nil { + return nil, err + } + + // Cache sensitive field paths for this resource type (same for all items) + var sensitiveFieldPaths []string + var sensitiveFieldPathsFetched bool + + items := []any{} + for _, item := range result.Items { + resource := &datamodel.DynamicResource{} + if err := item.As(resource); err != nil { + return nil, err + } + + // Redact sensitive fields before adding to the response + if resource.Properties != nil { + // Fetch sensitive field paths once for this resource type + if !sensitiveFieldPathsFetched { + sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( + ctx, + c.ucpClient, + resource.ID, + serviceCtx.ResourceID.Type(), + serviceCtx.APIVersion, + ) + if err != nil { + logger.Error(err, "Failed to fetch sensitive field paths for LIST redaction", + "resourceType", serviceCtx.ResourceID.Type(), "apiVersion", serviceCtx.APIVersion) + // Continue without redaction on error + } + sensitiveFieldPathsFetched = true + } + + if len(sensitiveFieldPaths) > 0 { + for _, path := range sensitiveFieldPaths { + redactField(resource.Properties, path) + } + } + } + + versioned, err := c.ResponseConverter()(resource, serviceCtx.APIVersion) + if err != nil { + return nil, err + } + items = append(items, versioned) + } + + if len(sensitiveFieldPaths) > 0 { + logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in LIST response", + "count", len(sensitiveFieldPaths), "resourceType", serviceCtx.ResourceID.Type(), + "itemCount", len(items)) + } + + return rest.NewOKResponse(&v1.PaginatedList{ + Value: items, + NextLink: ctrl.GetNextLinkURL(ctx, req, result.PaginationToken), + }), nil +} diff --git a/pkg/dynamicrp/frontend/routes.go b/pkg/dynamicrp/frontend/routes.go index 7ea30a5496..5493969d60 100644 --- a/pkg/dynamicrp/frontend/routes.go +++ b/pkg/dynamicrp/frontend/routes.go @@ -74,7 +74,7 @@ func (s *Service) registerRoutes( func(opts controller.Options) (controller.Controller, error) { optsCopy := resourceOptions optsCopy.ListRecursiveQuery = true - return defaultoperation.NewListResources(opts, optsCopy) + return NewListResourcesWithRedaction(opts, optsCopy, ucpClient) })) // Async operation status/results @@ -88,11 +88,11 @@ func (s *Service) registerRoutes( r.Route("/{rg:resource[gG]roups}/{resourceGroupName}/providers/{providerNamespace}/{resourceType}", func(r chi.Router) { r.Get("/", dynamicOperationHandler(v1.OperationList, controllerOptions, func(opts controller.Options) (controller.Controller, error) { - return defaultoperation.NewListResources(opts, resourceOptions) + return NewListResourcesWithRedaction(opts, resourceOptions, ucpClient) })) r.Get("/{resourceName}", dynamicOperationHandler(v1.OperationGet, controllerOptions, func(opts controller.Options) (controller.Controller, error) { - return defaultoperation.NewGetResource(opts, resourceOptions) + return NewGetResourceWithRedaction(opts, resourceOptions, ucpClient) })) r.Put("/{resourceName}", dynamicOperationHandler(v1.OperationPut, controllerOptions, func(opts controller.Options) (controller.Controller, error) { diff --git a/pkg/dynamicrp/frontend/updatefilter.go b/pkg/dynamicrp/frontend/updatefilter.go index b8e58ff228..17052806e2 100644 --- a/pkg/dynamicrp/frontend/updatefilter.go +++ b/pkg/dynamicrp/frontend/updatefilter.go @@ -22,7 +22,7 @@ import ( "github.com/radius-project/radius/pkg/armrpc/frontend/controller" "github.com/radius-project/radius/pkg/armrpc/rest" "github.com/radius-project/radius/pkg/dynamicrp/datamodel" - "github.com/radius-project/radius/pkg/dynamicrp/schema" + "github.com/radius-project/radius/pkg/schema" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" "github.com/radius-project/radius/pkg/ucp/ucplog" ) diff --git a/pkg/dynamicrp/frontend/updatefilter_test.go b/pkg/dynamicrp/frontend/updatefilter_test.go index ec9ab8e3c6..c421718a5b 100644 --- a/pkg/dynamicrp/frontend/updatefilter_test.go +++ b/pkg/dynamicrp/frontend/updatefilter_test.go @@ -138,7 +138,7 @@ func TestPrepareResourceFilter(t *testing.T) { t.Run("continues on schema fetch error", func(t *testing.T) { // Create a client that will fail to fetch the schema - clientFactory, err := testUCPClientFactoryWithError() + clientFactory, err := testUpdateFilterUCPClientFactoryWithError() require.NoError(t, err) factory := &UpdateFilterFactory{ @@ -193,8 +193,8 @@ func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactor }) } -// testUCPClientFactoryWithError creates a mock UCP client factory that returns an error. -func testUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { +// testUpdateFilterUCPClientFactoryWithError creates a mock UCP client factory that returns an error. +func testUpdateFilterUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { apiVersionsServer := fake.APIVersionsServer{ Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} diff --git a/pkg/dynamicrp/schema/annotations.go b/pkg/dynamicrp/schema/annotations.go deleted file mode 100644 index dec88e4541..0000000000 --- a/pkg/dynamicrp/schema/annotations.go +++ /dev/null @@ -1,114 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package schema - -import ( - "context" - "strings" - - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" - "github.com/radius-project/radius/pkg/ucp/resources" -) - -const ( - // XRadiusSensitiveAnnotation is the annotation key used to mark fields as sensitive in the resource schema. - XRadiusSensitiveAnnotation = "x-radius-sensitive" -) - -// GetSensitiveFieldPaths fetches the schema for a resource and returns paths to fields marked with x-radius-sensitive. -// Paths are in dot notation, e.g., "credentials.password" or "config.apiKey". -// -// Parameters: -// - ctx: The request context -// - ucpClient: UCP client factory for fetching the schema -// - resourceID: The full resource ID (e.g., "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test") -// - resourceType: The resource type (e.g., "Foo.Bar/myResources") -// - apiVersion: The API version to fetch the schema for -// -// Returns: -// - []string: Paths to sensitive fields, or empty slice if none found -// - error: Any error encountered while fetching the schema -func GetSensitiveFieldPaths(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) ([]string, error) { - if ucpClient == nil { - return nil, nil - } - - // Parse the resource ID to get plane information - ID, err := resources.Parse(resourceID) - if err != nil { - return nil, err - } - - plane := ID.PlaneNamespace() - planeName := strings.Split(plane, "/")[1] - resourceProvider := strings.Split(resourceType, "/")[0] - resourceTypeName := strings.Split(resourceType, "/")[1] - - // Fetch the API version resource which contains the schema - apiVersionResource, err := ucpClient.NewAPIVersionsClient().Get(ctx, planeName, resourceProvider, resourceTypeName, apiVersion, nil) - if err != nil { - return nil, err - } - - schema := apiVersionResource.APIVersionResource.Properties.Schema - if schema == nil { - return nil, nil - } - - // Extract paths to fields with x-radius-sensitive annotation - return ExtractSensitiveFieldPaths(schema, ""), nil -} - -// ExtractSensitiveFieldPaths recursively walks the schema and returns paths to fields marked with x-radius-sensitive. -// The prefix parameter builds up the path as we traverse nested objects. -func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { - var paths []string - - properties, ok := schema["properties"].(map[string]any) - if !ok { - return paths - } - - for fieldName, fieldSchema := range properties { - fieldSchemaMap, ok := fieldSchema.(map[string]any) - if !ok { - continue - } - - // Build the full path for this field - var fullPath string - if prefix == "" { - fullPath = fieldName - } else { - fullPath = prefix + "." + fieldName - } - - // Check if this field has the x-radius-sensitive annotation - if isSensitive, ok := fieldSchemaMap[XRadiusSensitiveAnnotation].(bool); ok && isSensitive { - paths = append(paths, fullPath) - } - - // Recursively check nested objects - if nestedProps, ok := fieldSchemaMap["properties"].(map[string]any); ok { - nestedSchema := map[string]any{"properties": nestedProps} - nestedPaths := ExtractSensitiveFieldPaths(nestedSchema, fullPath) - paths = append(paths, nestedPaths...) - } - } - - return paths -} diff --git a/pkg/dynamicrp/schema/annotations_test.go b/pkg/dynamicrp/schema/annotations_test.go deleted file mode 100644 index 03ec6fbdac..0000000000 --- a/pkg/dynamicrp/schema/annotations_test.go +++ /dev/null @@ -1,318 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package schema - -import ( - "context" - "net/http" - "testing" - - armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" - azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" - "github.com/stretchr/testify/require" -) - -func TestExtractSensitiveFieldPaths(t *testing.T) { - tests := []struct { - name string - schema map[string]any - expected []string - }{ - { - name: "empty schema", - schema: map[string]any{}, - expected: []string{}, - }, - { - name: "no sensitive fields", - schema: map[string]any{ - "properties": map[string]any{ - "name": map[string]any{ - "type": "string", - }, - "port": map[string]any{ - "type": "integer", - }, - }, - }, - expected: []string{}, - }, - { - name: "single sensitive field", - schema: map[string]any{ - "properties": map[string]any{ - "name": map[string]any{ - "type": "string", - }, - "password": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - }, - }, - expected: []string{"password"}, - }, - { - name: "multiple sensitive fields", - schema: map[string]any{ - "properties": map[string]any{ - "username": map[string]any{ - "type": "string", - }, - "password": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - "apiKey": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - }, - }, - expected: []string{"password", "apiKey"}, - }, - { - name: "nested sensitive field", - schema: map[string]any{ - "properties": map[string]any{ - "name": map[string]any{ - "type": "string", - }, - "credentials": map[string]any{ - "type": "object", - "properties": map[string]any{ - "username": map[string]any{ - "type": "string", - }, - "password": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - }, - }, - }, - }, - expected: []string{"credentials.password"}, - }, - { - name: "deeply nested sensitive fields", - schema: map[string]any{ - "properties": map[string]any{ - "config": map[string]any{ - "type": "object", - "properties": map[string]any{ - "database": map[string]any{ - "type": "object", - "properties": map[string]any{ - "connectionString": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - "host": map[string]any{ - "type": "string", - }, - }, - }, - }, - }, - }, - }, - expected: []string{"config.database.connectionString"}, - }, - { - name: "mixed sensitive fields at different levels", - schema: map[string]any{ - "properties": map[string]any{ - "apiKey": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - "settings": map[string]any{ - "type": "object", - "properties": map[string]any{ - "token": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - "endpoint": map[string]any{ - "type": "string", - }, - }, - }, - }, - }, - expected: []string{"apiKey", "settings.token"}, - }, - { - name: "sensitive annotation set to false", - schema: map[string]any{ - "properties": map[string]any{ - "password": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: false, - }, - }, - }, - expected: []string{}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := ExtractSensitiveFieldPaths(tt.schema, "") - - // Sort both slices for comparison since map iteration order is not guaranteed - require.ElementsMatch(t, tt.expected, result) - }) - } -} - -func TestExtractSensitiveFieldPaths_WithPrefix(t *testing.T) { - schema := map[string]any{ - "properties": map[string]any{ - "secret": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - }, - } - - result := ExtractSensitiveFieldPaths(schema, "parent") - - require.Equal(t, []string{"parent.secret"}, result) -} - -func TestGetSensitiveFieldPaths(t *testing.T) { - ctx := context.Background() - - t.Run("nil client returns nil", func(t *testing.T) { - result, err := GetSensitiveFieldPaths(ctx, nil, "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", "Foo.Bar/myResources", "2024-01-01") - require.NoError(t, err) - require.Nil(t, result) - }) - - t.Run("extracts sensitive fields from schema", func(t *testing.T) { - clientFactory, err := testUCPClientFactory(map[string]any{ - "properties": map[string]any{ - "host": map[string]any{ - "type": "string", - }, - "password": map[string]any{ - "type": "string", - XRadiusSensitiveAnnotation: true, - }, - }, - }) - require.NoError(t, err) - - result, err := GetSensitiveFieldPaths( - ctx, - clientFactory, - "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - "Foo.Bar/myResources", - "2024-01-01", - ) - - require.NoError(t, err) - require.Equal(t, []string{"password"}, result) - }) - - t.Run("returns empty for schema with no sensitive fields", func(t *testing.T) { - clientFactory, err := testUCPClientFactory(map[string]any{ - "properties": map[string]any{ - "host": map[string]any{ - "type": "string", - }, - "port": map[string]any{ - "type": "integer", - }, - }, - }) - require.NoError(t, err) - - result, err := GetSensitiveFieldPaths( - ctx, - clientFactory, - "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - "Foo.Bar/myResources", - "2024-01-01", - ) - - require.NoError(t, err) - require.Empty(t, result) - }) - - t.Run("returns nil for nil schema", func(t *testing.T) { - clientFactory, err := testUCPClientFactory(nil) - require.NoError(t, err) - - result, err := GetSensitiveFieldPaths( - ctx, - clientFactory, - "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - "Foo.Bar/myResources", - "2024-01-01", - ) - - require.NoError(t, err) - require.Nil(t, result) - }) -} - -// testUCPClientFactory creates a mock UCP client factory that returns the given schema. -func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { - apiVersionsServer := fake.APIVersionsServer{ - Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { - resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} - resp.SetResponse(http.StatusOK, v20231001preview.APIVersionsClientGetResponse{ - APIVersionResource: v20231001preview.APIVersionResource{ - Properties: &v20231001preview.APIVersionProperties{ - Schema: schema, - }, - }, - }, nil) - return resp, azfake.ErrorResponder{} - }, - } - - return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ - APIVersionsServer: apiVersionsServer, - }), - }, - }) -} - -func TestGetSensitiveFieldPaths_InvalidResourceID(t *testing.T) { - clientFactory, err := testUCPClientFactory(nil) - require.NoError(t, err) - - _, err = GetSensitiveFieldPaths( - context.Background(), - clientFactory, - "invalid-resource-id", - "Foo.Bar/myResources", - "2024-01-01", - ) - - require.Error(t, err) -} diff --git a/pkg/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index 06d9ba5027..e5a4ef9b58 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource.go +++ b/pkg/portableresources/backend/controller/createorupdateresource.go @@ -18,12 +18,15 @@ package controller import ( "context" + "encoding/json" "errors" "fmt" "github.com/go-logr/logr" + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" ctrl "github.com/radius-project/radius/pkg/armrpc/asyncoperation/controller" "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/crypto/encryption" "github.com/radius-project/radius/pkg/portableresources/datamodel" "github.com/radius-project/radius/pkg/portableresources/processors" "github.com/radius-project/radius/pkg/recipes" @@ -32,6 +35,7 @@ import ( "github.com/radius-project/radius/pkg/recipes/util" "github.com/radius-project/radius/pkg/resourceutil" rpv1 "github.com/radius-project/radius/pkg/rp/v1" + schemautil "github.com/radius-project/radius/pkg/schema" "github.com/radius-project/radius/pkg/ucp/ucplog" ) @@ -76,6 +80,86 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques return ctrl.Result{}, err } + currentETag := storedResource.ETag + var recipeProperties map[string]any + redactionCompleted := false + + // Attempt to decrypt and redact sensitive fields before recipe execution. + apiVersion := getResourceAPIVersion(resource) + if apiVersion != "" { + resourceType := resource.GetBaseResource().Type + schema, schemaErr := schemautil.GetSchema(ctx, c.UcpClient(), req.ResourceID, resourceType, apiVersion) + if schemaErr != nil { + logger.Error(schemaErr, "Failed to fetch schema for sensitive field detection", "resourceID", req.ResourceID) + } else if schema != nil { + sensitiveFieldPaths := schemautil.ExtractSensitiveFieldPaths(schema, "") + if len(sensitiveFieldPaths) > 0 { + logger.Info("Sensitive fields detected for resource", "resourceID", req.ResourceID, "paths", sensitiveFieldPaths) + + properties, err := resourceutil.GetPropertiesFromResource(resource) + if err != nil { + return ctrl.Result{}, err + } + + recipeProperties, err = deepCopyProperties(properties) + if err != nil { + return ctrl.Result{}, err + } + + if c.KubeClient() == nil { + err = fmt.Errorf("kubernetes client not configured for sensitive data decryption") + logger.Error(err, "Failed to initialize encryption key provider", "resourceID", req.ResourceID) + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + + keyProvider := encryption.NewKubernetesKeyProvider(c.KubeClient(), nil) + handler, err := encryption.NewSensitiveDataHandlerFromProvider(ctx, keyProvider) + if err != nil { + logger.Error(err, "Failed to initialize sensitive data handler", "resourceID", req.ResourceID) + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + + if err = handler.DecryptSensitiveFieldsWithSchema(ctx, recipeProperties, sensitiveFieldPaths, req.ResourceID, schema); err != nil { + logger.Error(err, "Failed to decrypt sensitive fields", "resourceID", req.ResourceID) + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + + // Security: derive the redacted copy from the original encrypted + // properties, NOT from the decrypted recipeProperties. If + // RedactSensitiveFields fails to nil a field (partial failure), + // the persisted value remains encrypted ciphertext, never + // plaintext. recipeProperties (decrypted) is kept exclusively + // for in-memory recipe execution. + redactedProperties, err := deepCopyProperties(properties) + if err != nil { + return ctrl.Result{}, err + } + if err = handler.RedactSensitiveFields(redactedProperties, sensitiveFieldPaths); err != nil { + logger.Error(err, "Failed to redact sensitive fields", "resourceID", req.ResourceID) + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + + if err = applyPropertiesToResource(resource, redactedProperties); err != nil { + logger.Error(err, "Failed to apply redacted properties", "resourceID", req.ResourceID) + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + + update := &database.Object{ + Metadata: database.Metadata{ID: req.ResourceID}, + Data: resource, + } + if err = c.DatabaseClient().Save(ctx, update, database.WithETag(currentETag)); err != nil { + logger.Error(err, "Failed to persist redacted resource", "resourceID", req.ResourceID) + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + currentETag = update.ETag + redactionCompleted = true + } + } + } else { + logger.Info("Skipping sensitive field detection due to missing apiVersion", "resourceID", req.ResourceID) + } + // Clone existing output resources so we can diff them later. previousOutputResources := c.copyOutputResources(resource) @@ -83,6 +167,9 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques metadata := recipes.ResourceMetadata{EnvironmentID: resource.ResourceMetadata().EnvironmentID(), ApplicationID: resource.ResourceMetadata().ApplicationID(), ResourceID: resource.GetBaseResource().ID} config, err := c.configurationLoader.LoadConfiguration(ctx, metadata) if err != nil { + if redactionCompleted { + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } return ctrl.Result{}, err } @@ -90,9 +177,9 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques recipeDataModel, supportsRecipes := any(resource).(datamodel.RecipeDataModel) var recipeOutput *recipes.RecipeOutput if supportsRecipes && recipeDataModel.GetRecipe() != nil { - recipeOutput, err = c.executeRecipeIfNeeded(ctx, resource, recipeDataModel, previousOutputResources, config.Simulated) + recipeOutput, err = c.executeRecipeIfNeeded(ctx, resource, recipeDataModel, previousOutputResources, config.Simulated, recipeProperties) if err != nil { - return c.handleRecipeError(ctx, err, recipeDataModel, req.ResourceID, storedResource.ETag, logger) + return c.handleRecipeError(ctx, err, recipeDataModel, req.ResourceID, currentETag, logger) } } @@ -102,6 +189,9 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques // Now we're ready to process the resource. This will handle the updates to any user-visible state. err = c.processor.Process(ctx, resource, processors.Options{RecipeOutput: recipeOutput, RuntimeConfiguration: config.Runtime, UcpClient: c.BaseController.UcpClient()}) if err != nil { + if redactionCompleted { + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } return ctrl.Result{}, err } } @@ -123,8 +213,11 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques }, Data: recipeDataModel.(rpv1.RadiusResourceModel), } - err = c.DatabaseClient().Save(ctx, update, database.WithETag(storedResource.ETag)) + err = c.DatabaseClient().Save(ctx, update, database.WithETag(currentETag)) if err != nil { + if redactionCompleted { + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } return ctrl.Result{}, err } @@ -164,13 +257,17 @@ func (c *CreateOrUpdateResource[P, T]) copyOutputResources(resource P) []string return previousOutputResources } -func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context, resource P, recipeDataModel datamodel.RecipeDataModel, prevState []string, simulated bool) (*recipes.RecipeOutput, error) { +func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context, resource P, recipeDataModel datamodel.RecipeDataModel, prevState []string, simulated bool, recipeProperties map[string]any) (*recipes.RecipeOutput, error) { // Caller ensures recipeDataModel supports recipes and has a non-nil recipe recipe := recipeDataModel.GetRecipe() - resourceProperties, err := resourceutil.GetPropertiesFromResource(resource) - if err != nil { - return nil, err + resourceProperties := recipeProperties + if resourceProperties == nil { + var err error + resourceProperties, err = resourceutil.GetPropertiesFromResource(resource) + if err != nil { + return nil, err + } } connectionsAndSourceIDs, err := resourceutil.GetConnectionNameandSourceIDs(resource) @@ -220,6 +317,45 @@ func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context }) } +func getResourceAPIVersion[P rpv1.RadiusResourceModel](resource P) string { + metadata := resource.GetBaseResource().InternalMetadata + if metadata.UpdatedAPIVersion != "" { + return metadata.UpdatedAPIVersion + } + return metadata.CreatedAPIVersion +} + +func deepCopyProperties(source map[string]any) (map[string]any, error) { + if source == nil { + return map[string]any{}, nil + } + + bytes, err := json.Marshal(source) + if err != nil { + return nil, err + } + + var copy map[string]any + if err = json.Unmarshal(bytes, ©); err != nil { + return nil, err + } + + return copy, nil +} + +func applyPropertiesToResource[P rpv1.RadiusResourceModel](resource P, properties map[string]any) error { + payload := map[string]any{ + "properties": properties, + } + + bytes, err := json.Marshal(payload) + if err != nil { + return err + } + + return json.Unmarshal(bytes, resource) +} + // setRecipeStatus sets the recipe status for the given resource model. // It retrieves the resource metadata from the provided model, deep copies the current resource status, // updates the Recipe field with the supplied recipeStatus, and then applies the updated status back to the resource. diff --git a/pkg/portableresources/backend/controller/createorupdateresource_test.go b/pkg/portableresources/backend/controller/createorupdateresource_test.go index d1045a066b..9aca2a9dcc 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource_test.go +++ b/pkg/portableresources/backend/controller/createorupdateresource_test.go @@ -18,18 +18,30 @@ package controller import ( "context" + "encoding/base64" + "encoding/json" "errors" "fmt" + "net/http" "testing" + armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" + azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/google/uuid" "github.com/mitchellh/mapstructure" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + controllerfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" ctrl "github.com/radius-project/radius/pkg/armrpc/asyncoperation/controller" "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/crypto/encryption" "github.com/radius-project/radius/pkg/portableresources" "github.com/radius-project/radius/pkg/portableresources/datamodel" "github.com/radius-project/radius/pkg/portableresources/processors" @@ -39,6 +51,8 @@ import ( "github.com/radius-project/radius/pkg/recipes/engine" "github.com/radius-project/radius/pkg/resourceutil" rpv1 "github.com/radius-project/radius/pkg/rp/v1" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" "github.com/radius-project/radius/pkg/ucp/resources" ) @@ -91,6 +105,8 @@ type TestResourceProperties struct { rpv1.BasicResourceProperties IsProcessed bool `json:"isProcessed"` Recipe portableresources.ResourceRecipe `json:"recipe,omitempty"` + Secret any `json:"secret,omitempty"` + Credentials map[string]any `json:"credentials,omitempty"` } type SuccessProcessor struct { @@ -470,3 +486,622 @@ func TestCreateOrUpdateResource_Run(t *testing.T) { }) } } + +func TestCreateOrUpdateResource_Run_SensitiveRedaction(t *testing.T) { + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + eng := engine.NewMockEngine(mctrl) + cfg := configloader.NewMockConfigurationLoader(mctrl) + + key, err := encryption.GenerateKey() + require.NoError(t, err) + + provider, err := encryption.NewInMemoryKeyProvider(key) + require.NoError(t, err) + + handler, err := encryption.NewSensitiveDataHandlerFromProvider(context.Background(), provider) + require.NoError(t, err) + + secretValue := "top-secret" + properties := map[string]any{ + "application": TestApplicationID, + "environment": TestEnvironmentID, + "provisioningState": "Accepted", + "secret": secretValue, + "recipe": map[string]any{ + "name": "test-recipe", + "parameters": map[string]any{ + "p1": "v1", + }, + }, + "status": map[string]any{ + "outputResources": []map[string]any{}, + }, + } + + err = handler.EncryptSensitiveFields(properties, []string{"secret"}, TestResourceID) + require.NoError(t, err) + + data := map[string]any{ + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "internalMetadata": map[string]any{ + "updatedApiVersion": "2024-01-01", + }, + "properties": properties, + } + + msc.EXPECT(). + Get(gomock.Any(), TestResourceID). + Return(&database.Object{Metadata: database.Metadata{ID: TestResourceID, ETag: "etag-1"}, Data: data}, nil). + Times(1) + + ucpClient, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: encryption.DefaultEncryptionKeySecretName, + Namespace: encryption.RadiusNamespace, + }, + Data: map[string][]byte{ + encryption.DefaultEncryptionKeySecretKey: mustKeyStoreJSON(t, key), + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + k8sClient := controllerfake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build() + + configuration := &recipes.Configuration{ + Runtime: recipes.RuntimeConfiguration{ + Kubernetes: &recipes.KubernetesRuntime{ + Namespace: "test-namespace", + EnvironmentNamespace: "test-env-namespace", + }, + }, + } + + redactionSave := msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + props, err := resourceutil.GetPropertiesFromResource(obj.Data) + require.NoError(t, err) + require.Nil(t, props["secret"]) + obj.Metadata.ETag = "etag-2" + return nil + }, + ) + + finalSave := msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + props, err := resourceutil.GetPropertiesFromResource(obj.Data) + require.NoError(t, err) + require.Nil(t, props["secret"]) + return nil + }, + ) + + engExecute := eng.EXPECT().Execute(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, opts engine.ExecuteOptions) (*recipes.RecipeOutput, error) { + require.Equal(t, secretValue, opts.BaseOptions.Recipe.Properties["secret"]) + return &recipes.RecipeOutput{}, nil + }, + ) + + gomock.InOrder( + redactionSave, + cfg.EXPECT().LoadConfiguration(gomock.Any(), recipes.ResourceMetadata{EnvironmentID: TestEnvironmentID, ApplicationID: TestApplicationID, ResourceID: TestResourceID}).Return(configuration, nil), + engExecute, + finalSave, + ) + + opts := ctrl.Options{ + DatabaseClient: msc, + UcpClient: ucpClient, + KubeClient: k8sClient, + } + + recipeCfg := &controllerconfig.RecipeControllerConfig{ + Engine: eng, + ConfigLoader: cfg, + } + + ctrlr, err := NewCreateOrUpdateResource(opts, successProcessorReference, recipeCfg.Engine, recipeCfg.ConfigLoader) + require.NoError(t, err) + + res, err := ctrlr.Run(context.Background(), &ctrl.Request{ + OperationID: uuid.New(), + OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", + ResourceID: TestResourceID, + CorrelationID: uuid.NewString(), + OperationTimeout: &ctrl.DefaultAsyncOperationTimeout, + }) + require.NoError(t, err) + require.Equal(t, ctrl.Result{}, res) +} + +func TestCreateOrUpdateResource_Run_SensitiveMissingKey(t *testing.T) { + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + eng := engine.NewMockEngine(mctrl) + cfg := configloader.NewMockConfigurationLoader(mctrl) + + data := map[string]any{ + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "internalMetadata": map[string]any{ + "updatedApiVersion": "2024-01-01", + }, + "properties": map[string]any{ + "application": TestApplicationID, + "environment": TestEnvironmentID, + "provisioningState": "Accepted", + "secret": map[string]any{ + "encrypted": "not-real", + "nonce": "not-real", + }, + "recipe": map[string]any{ + "name": "test-recipe", + }, + }, + } + + msc.EXPECT(). + Get(gomock.Any(), TestResourceID). + Return(&database.Object{Metadata: database.Metadata{ID: TestResourceID, ETag: "etag-1"}, Data: data}, nil). + Times(1) + + ucpClient, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) + require.NoError(t, err) + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + k8sClient := controllerfake.NewClientBuilder().WithScheme(scheme).Build() + + cfg.EXPECT().LoadConfiguration(gomock.Any(), gomock.Any()).Times(0) + eng.EXPECT().Execute(gomock.Any(), gomock.Any()).Times(0) + msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + + opts := ctrl.Options{ + DatabaseClient: msc, + UcpClient: ucpClient, + KubeClient: k8sClient, + } + + recipeCfg := &controllerconfig.RecipeControllerConfig{ + Engine: eng, + ConfigLoader: cfg, + } + + ctrlr, err := NewCreateOrUpdateResource(opts, successProcessorReference, recipeCfg.Engine, recipeCfg.ConfigLoader) + require.NoError(t, err) + + res, err := ctrlr.Run(context.Background(), &ctrl.Request{ + OperationID: uuid.New(), + OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", + ResourceID: TestResourceID, + CorrelationID: uuid.NewString(), + OperationTimeout: &ctrl.DefaultAsyncOperationTimeout, + }) + require.Error(t, err) + require.Equal(t, v1.ProvisioningStateFailed, res.ProvisioningState()) + require.False(t, res.Requeue) +} + +func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { + apiVersionsServer := fake.APIVersionsServer{ + Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { + resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} + resp.SetResponse(http.StatusOK, v20231001preview.APIVersionsClientGetResponse{ + APIVersionResource: v20231001preview.APIVersionResource{ + Properties: &v20231001preview.APIVersionProperties{ + Schema: schema, + }, + }, + }, nil) + return resp, azfake.ErrorResponder{} + }, + } + + return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ + APIVersionsServer: apiVersionsServer, + }), + }, + }) +} + +func mustKeyStoreJSON(t *testing.T, key []byte) []byte { + keyStore := encryption.KeyStore{ + CurrentVersion: 1, + Keys: map[string]encryption.KeyData{ + "1": { + Key: base64.StdEncoding.EncodeToString(key), + Version: 1, + CreatedAt: "2026-01-01T00:00:00Z", + ExpiresAt: "2026-12-31T00:00:00Z", + }, + }, + } + + bytes, err := json.Marshal(keyStore) + require.NoError(t, err) + return bytes +} + +func TestCreateOrUpdateResource_Run_SensitiveNilKubeClient(t *testing.T) { + // When sensitive fields are detected but KubeClient is nil, the controller + // must fail immediately. Per design: no retry (Requeue: false), and recipe + // execution must not proceed. + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + eng := engine.NewMockEngine(mctrl) + cfg := configloader.NewMockConfigurationLoader(mctrl) + + data := map[string]any{ + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "internalMetadata": map[string]any{ + "updatedApiVersion": "2024-01-01", + }, + "properties": map[string]any{ + "application": TestApplicationID, + "environment": TestEnvironmentID, + "provisioningState": "Accepted", + "secret": "value-does-not-matter", + "recipe": map[string]any{ + "name": "test-recipe", + }, + }, + } + + msc.EXPECT(). + Get(gomock.Any(), TestResourceID). + Return(&database.Object{Metadata: database.Metadata{ID: TestResourceID, ETag: "etag-1"}, Data: data}, nil). + Times(1) + + ucpClient, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) + require.NoError(t, err) + + // Nothing downstream should execute + cfg.EXPECT().LoadConfiguration(gomock.Any(), gomock.Any()).Times(0) + eng.EXPECT().Execute(gomock.Any(), gomock.Any()).Times(0) + msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) + + opts := ctrl.Options{ + DatabaseClient: msc, + UcpClient: ucpClient, + KubeClient: nil, // deliberately nil + } + + recipeCfg := &controllerconfig.RecipeControllerConfig{ + Engine: eng, + ConfigLoader: cfg, + } + + ctrlr, err := NewCreateOrUpdateResource(opts, successProcessorReference, recipeCfg.Engine, recipeCfg.ConfigLoader) + require.NoError(t, err) + + res, err := ctrlr.Run(context.Background(), &ctrl.Request{ + OperationID: uuid.New(), + OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", + ResourceID: TestResourceID, + CorrelationID: uuid.NewString(), + OperationTimeout: &ctrl.DefaultAsyncOperationTimeout, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "kubernetes client not configured") + require.Equal(t, v1.ProvisioningStateFailed, res.ProvisioningState()) + require.False(t, res.Requeue) +} + +func TestCreateOrUpdateResource_Run_SensitiveRedactionSaveFails(t *testing.T) { + // When the redaction save fails, the controller must return a non-retryable + // failed result and must NOT proceed to recipe execution. Per design: + // users must resubmit with fresh credentials. + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + eng := engine.NewMockEngine(mctrl) + cfg := configloader.NewMockConfigurationLoader(mctrl) + + key, err := encryption.GenerateKey() + require.NoError(t, err) + + provider, err := encryption.NewInMemoryKeyProvider(key) + require.NoError(t, err) + + handler, err := encryption.NewSensitiveDataHandlerFromProvider(context.Background(), provider) + require.NoError(t, err) + + properties := map[string]any{ + "application": TestApplicationID, + "environment": TestEnvironmentID, + "provisioningState": "Accepted", + "secret": "save-failure-secret", + "recipe": map[string]any{ + "name": "test-recipe", + }, + "status": map[string]any{ + "outputResources": []map[string]any{}, + }, + } + + err = handler.EncryptSensitiveFields(properties, []string{"secret"}, TestResourceID) + require.NoError(t, err) + + data := map[string]any{ + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "internalMetadata": map[string]any{ + "updatedApiVersion": "2024-01-01", + }, + "properties": properties, + } + + msc.EXPECT(). + Get(gomock.Any(), TestResourceID). + Return(&database.Object{Metadata: database.Metadata{ID: TestResourceID, ETag: "etag-1"}, Data: data}, nil). + Times(1) + + ucpClient, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: encryption.DefaultEncryptionKeySecretName, + Namespace: encryption.RadiusNamespace, + }, + Data: map[string][]byte{ + encryption.DefaultEncryptionKeySecretKey: mustKeyStoreJSON(t, key), + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + k8sClient := controllerfake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build() + + saveErr := errors.New("database unavailable") + // The single Save call is the redaction save — verify the payload has nil + // for the sensitive field (no plaintext leaked) before returning the error. + msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + props, err := resourceutil.GetPropertiesFromResource(obj.Data) + require.NoError(t, err) + require.Nil(t, props["secret"], "plaintext must never appear in the save payload") + return saveErr + }, + ).Times(1) + + // Recipe and config must never execute + cfg.EXPECT().LoadConfiguration(gomock.Any(), gomock.Any()).Times(0) + eng.EXPECT().Execute(gomock.Any(), gomock.Any()).Times(0) + + opts := ctrl.Options{ + DatabaseClient: msc, + UcpClient: ucpClient, + KubeClient: k8sClient, + } + + recipeCfg := &controllerconfig.RecipeControllerConfig{ + Engine: eng, + ConfigLoader: cfg, + } + + ctrlr, err := NewCreateOrUpdateResource(opts, successProcessorReference, recipeCfg.Engine, recipeCfg.ConfigLoader) + require.NoError(t, err) + + res, err := ctrlr.Run(context.Background(), &ctrl.Request{ + OperationID: uuid.New(), + OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", + ResourceID: TestResourceID, + CorrelationID: uuid.NewString(), + OperationTimeout: &ctrl.DefaultAsyncOperationTimeout, + }) + require.Error(t, err) + require.Equal(t, v1.ProvisioningStateFailed, res.ProvisioningState()) + require.False(t, res.Requeue) +} + +func TestCreateOrUpdateResource_Run_SensitiveMultipleFields(t *testing.T) { + // Two sensitive fields at different nesting depths: a top-level "secret" and + // a nested "credentials.password". Validates: + // - Both fields are nil in both the redaction save and the final save + // - The recipe engine receives both decrypted values + mctrl := gomock.NewController(t) + msc := database.NewMockClient(mctrl) + eng := engine.NewMockEngine(mctrl) + cfg := configloader.NewMockConfigurationLoader(mctrl) + + key, err := encryption.GenerateKey() + require.NoError(t, err) + + provider, err := encryption.NewInMemoryKeyProvider(key) + require.NoError(t, err) + + handler, err := encryption.NewSensitiveDataHandlerFromProvider(context.Background(), provider) + require.NoError(t, err) + + properties := map[string]any{ + "application": TestApplicationID, + "environment": TestEnvironmentID, + "provisioningState": "Accepted", + "secret": "secret-value", + "credentials": map[string]any{ + "password": "password-value", + }, + "recipe": map[string]any{ + "name": "test-recipe", + "parameters": map[string]any{ + "p1": "v1", + }, + }, + "status": map[string]any{ + "outputResources": []map[string]any{}, + }, + } + + err = handler.EncryptSensitiveFields(properties, []string{"secret", "credentials.password"}, TestResourceID) + require.NoError(t, err) + + data := map[string]any{ + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "internalMetadata": map[string]any{ + "updatedApiVersion": "2024-01-01", + }, + "properties": properties, + } + + msc.EXPECT(). + Get(gomock.Any(), TestResourceID). + Return(&database.Object{Metadata: database.Metadata{ID: TestResourceID, ETag: "etag-1"}, Data: data}, nil). + Times(1) + + ucpClient, err := testUCPClientFactory(map[string]any{ + "properties": map[string]any{ + "secret": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + "credentials": map[string]any{ + "type": "object", + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }, + }, + }) + require.NoError(t, err) + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: encryption.DefaultEncryptionKeySecretName, + Namespace: encryption.RadiusNamespace, + }, + Data: map[string][]byte{ + encryption.DefaultEncryptionKeySecretKey: mustKeyStoreJSON(t, key), + }, + } + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + k8sClient := controllerfake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build() + + configuration := &recipes.Configuration{ + Runtime: recipes.RuntimeConfiguration{ + Kubernetes: &recipes.KubernetesRuntime{ + Namespace: "test-namespace", + EnvironmentNamespace: "test-env-namespace", + }, + }, + } + + assertBothFieldsRedacted := func(obj *database.Object) { + props, err := resourceutil.GetPropertiesFromResource(obj.Data) + require.NoError(t, err) + require.Nil(t, props["secret"], "secret must be nil in DB") + creds, ok := props["credentials"].(map[string]any) + require.True(t, ok, "credentials must be a map") + require.Nil(t, creds["password"], "credentials.password must be nil in DB") + } + + redactionSave := msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + assertBothFieldsRedacted(obj) + obj.Metadata.ETag = "etag-2" + return nil + }, + ) + + finalSave := msc.EXPECT().Save(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, obj *database.Object, _ ...database.SaveOptions) error { + assertBothFieldsRedacted(obj) + return nil + }, + ) + + engExecute := eng.EXPECT().Execute(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, opts engine.ExecuteOptions) (*recipes.RecipeOutput, error) { + // Recipe must receive ALL decrypted values + require.Equal(t, "secret-value", opts.BaseOptions.Recipe.Properties["secret"]) + creds, ok := opts.BaseOptions.Recipe.Properties["credentials"].(map[string]any) + require.True(t, ok, "recipe must receive credentials map") + require.Equal(t, "password-value", creds["password"]) + return &recipes.RecipeOutput{}, nil + }, + ) + + gomock.InOrder( + redactionSave, + cfg.EXPECT().LoadConfiguration(gomock.Any(), recipes.ResourceMetadata{EnvironmentID: TestEnvironmentID, ApplicationID: TestApplicationID, ResourceID: TestResourceID}).Return(configuration, nil), + engExecute, + finalSave, + ) + + opts := ctrl.Options{ + DatabaseClient: msc, + UcpClient: ucpClient, + KubeClient: k8sClient, + } + + recipeCfg := &controllerconfig.RecipeControllerConfig{ + Engine: eng, + ConfigLoader: cfg, + } + + ctrlr, err := NewCreateOrUpdateResource(opts, successProcessorReference, recipeCfg.Engine, recipeCfg.ConfigLoader) + require.NoError(t, err) + + res, err := ctrlr.Run(context.Background(), &ctrl.Request{ + OperationID: uuid.New(), + OperationType: "APPLICATIONS.TEST/TESTRESOURCES|PUT", + ResourceID: TestResourceID, + CorrelationID: uuid.NewString(), + OperationTimeout: &ctrl.DefaultAsyncOperationTimeout, + }) + require.NoError(t, err) + require.Equal(t, ctrl.Result{}, res) +} + diff --git a/pkg/schema/annotations.go b/pkg/schema/annotations.go index f593e3583d..0ff0409d29 100644 --- a/pkg/schema/annotations.go +++ b/pkg/schema/annotations.go @@ -38,6 +38,20 @@ import ( // - []string: Paths to sensitive fields, or empty slice if none found // - error: Any error encountered while fetching the schema func GetSensitiveFieldPaths(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) ([]string, error) { + schema, err := GetSchema(ctx, ucpClient, resourceID, resourceType, apiVersion) + if err != nil { + return nil, err + } + if schema == nil { + return nil, nil + } + + return ExtractSensitiveFieldPaths(schema, ""), nil +} + +// GetSchema fetches the OpenAPI schema for a resource type and api version. +// Returns nil if the schema is not found or the client is nil. +func GetSchema(ctx context.Context, ucpClient *v20231001preview.ClientFactory, resourceID string, resourceType string, apiVersion string) (map[string]any, error) { if ucpClient == nil { return nil, nil } @@ -59,13 +73,7 @@ func GetSensitiveFieldPaths(ctx context.Context, ucpClient *v20231001preview.Cli return nil, err } - schema := apiVersionResource.APIVersionResource.Properties.Schema - if schema == nil { - return nil, nil - } - - // Extract paths to fields with x-radius-sensitive annotation - return ExtractSensitiveFieldPaths(schema, ""), nil + return apiVersionResource.APIVersionResource.Properties.Schema, nil } // ExtractSensitiveFieldPaths recursively walks the schema and returns paths to fields marked with x-radius-sensitive. diff --git a/pkg/schema/annotations_test.go b/pkg/schema/annotations_test.go index f242fb47ef..7859757381 100644 --- a/pkg/schema/annotations_test.go +++ b/pkg/schema/annotations_test.go @@ -510,6 +510,41 @@ func TestGetSensitiveFieldPaths(t *testing.T) { }) } +func TestGetSchema(t *testing.T) { + ctx := context.Background() + + t.Run("nil client returns nil", func(t *testing.T) { + result, err := GetSchema(ctx, nil, "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", "Foo.Bar/myResources", "2024-01-01") + require.NoError(t, err) + require.Nil(t, result) + }) + + t.Run("returns schema from UCP", func(t *testing.T) { + expectedSchema := map[string]any{ + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + }, + }, + } + + clientFactory, err := testUCPClientFactory(expectedSchema) + require.NoError(t, err) + + result, err := GetSchema( + ctx, + clientFactory, + "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", + "Foo.Bar/myResources", + "2024-01-01", + ) + + require.NoError(t, err) + require.Equal(t, expectedSchema, result) + }) +} + // testUCPClientFactory creates a mock UCP client factory that returns the given schema. func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { apiVersionsServer := fake.APIVersionsServer{ From 4d4cc5aee54482d15c6d57bb9b76a50cc12312e7 Mon Sep 17 00:00:00 2001 From: sk593 Date: Mon, 9 Feb 2026 09:10:52 -0800 Subject: [PATCH 03/10] remove unused code Signed-off-by: sk593 --- pkg/dynamicrp/frontend/updatefilter.go | 71 ------- pkg/dynamicrp/frontend/updatefilter_test.go | 214 -------------------- 2 files changed, 285 deletions(-) delete mode 100644 pkg/dynamicrp/frontend/updatefilter.go delete mode 100644 pkg/dynamicrp/frontend/updatefilter_test.go diff --git a/pkg/dynamicrp/frontend/updatefilter.go b/pkg/dynamicrp/frontend/updatefilter.go deleted file mode 100644 index 17052806e2..0000000000 --- a/pkg/dynamicrp/frontend/updatefilter.go +++ /dev/null @@ -1,71 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package frontend - -import ( - "context" - - "github.com/radius-project/radius/pkg/armrpc/frontend/controller" - "github.com/radius-project/radius/pkg/armrpc/rest" - "github.com/radius-project/radius/pkg/dynamicrp/datamodel" - "github.com/radius-project/radius/pkg/schema" - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" - "github.com/radius-project/radius/pkg/ucp/ucplog" -) - -// UpdateFilterFactory creates an UpdateFilter that has access to the UCP client for schema lookups. -type UpdateFilterFactory struct { - UCPClient *v20231001preview.ClientFactory -} - -// NewPrepareResourceFilter returns an UpdateFilter function that can access the UCP client. -func (f *UpdateFilterFactory) NewPrepareResourceFilter() controller.UpdateFilter[datamodel.DynamicResource] { - return func(ctx context.Context, newResource, oldResource *datamodel.DynamicResource, opt *controller.Options) (rest.Response, error) { - return f.prepareResource(ctx, newResource, oldResource, opt) - } -} - -// prepareResource extracts sensitive field paths from the schema during PUT/PATCH operations. -// The backend will independently call the same schema function when it needs the paths. -func (f *UpdateFilterFactory) prepareResource(ctx context.Context, newResource, _ *datamodel.DynamicResource, _ *controller.Options) (rest.Response, error) { - logger := ucplog.FromContextOrDiscard(ctx) - logger.Info("PrepareResource filter executing for dynamic resource", "resourceID", newResource.ID) - - // Extract sensitive field paths from the schema - // This is called here in the frontend; the backend will call the same function independently - sensitiveFieldPaths, err := schema.GetSensitiveFieldPaths( - ctx, - f.UCPClient, - newResource.ID, - newResource.Type, - newResource.InternalMetadata.UpdatedAPIVersion, - ) - if err != nil { - logger.Error(err, "Failed to get sensitive field paths from schema", "resourceID", newResource.ID) - // Continue processing even if we can't get the schema - don't fail the request - } - - if len(sensitiveFieldPaths) > 0 { - logger.Info("Found sensitive fields in schema", "resourceID", newResource.ID, "paths", sensitiveFieldPaths) - } - - // Note: We intentionally do NOT store the paths on the resource. - // The backend will call schema.GetSensitiveFieldPaths() independently when needed. - - // TODO: We need to update this to return or save the data from sensitive fields once we make changes to the frontend code. - return nil, nil -} diff --git a/pkg/dynamicrp/frontend/updatefilter_test.go b/pkg/dynamicrp/frontend/updatefilter_test.go deleted file mode 100644 index c421718a5b..0000000000 --- a/pkg/dynamicrp/frontend/updatefilter_test.go +++ /dev/null @@ -1,214 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package frontend - -import ( - "context" - "net/http" - "testing" - - armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" - azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" - "github.com/radius-project/radius/pkg/armrpc/frontend/controller" - aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" - "github.com/radius-project/radius/pkg/dynamicrp/datamodel" - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" - "github.com/stretchr/testify/require" -) - -func TestPrepareResourceFilter(t *testing.T) { - ctx := context.Background() - - t.Run("succeeds with sensitive fields", func(t *testing.T) { - clientFactory, err := testUCPClientFactory(map[string]any{ - "properties": map[string]any{ - "host": map[string]any{ - "type": "string", - }, - "password": map[string]any{ - "type": "string", - "x-radius-sensitive": true, - }, - }, - }) - require.NoError(t, err) - - factory := &UpdateFilterFactory{ - UCPClient: clientFactory, - } - - filter := factory.NewPrepareResourceFilter() - - resource := &datamodel.DynamicResource{ - BaseResource: v1.BaseResource{ - TrackedResource: v1.TrackedResource{ - ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - Type: "Foo.Bar/myResources", - }, - InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: "2024-01-01", - }, - }, - Properties: map[string]any{}, - } - - resp, err := filter(ctx, resource, nil, &controller.Options{}) - - require.NoError(t, err) - require.Nil(t, resp) - }) - - t.Run("succeeds with no sensitive fields", func(t *testing.T) { - clientFactory, err := testUCPClientFactory(map[string]any{ - "properties": map[string]any{ - "host": map[string]any{ - "type": "string", - }, - }, - }) - require.NoError(t, err) - - factory := &UpdateFilterFactory{ - UCPClient: clientFactory, - } - - filter := factory.NewPrepareResourceFilter() - - resource := &datamodel.DynamicResource{ - BaseResource: v1.BaseResource{ - TrackedResource: v1.TrackedResource{ - ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - Type: "Foo.Bar/myResources", - }, - InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: "2024-01-01", - }, - }, - Properties: map[string]any{}, - } - - resp, err := filter(ctx, resource, nil, &controller.Options{}) - - require.NoError(t, err) - require.Nil(t, resp) - }) - - t.Run("succeeds with nil UCP client", func(t *testing.T) { - factory := &UpdateFilterFactory{ - UCPClient: nil, - } - - filter := factory.NewPrepareResourceFilter() - - resource := &datamodel.DynamicResource{ - BaseResource: v1.BaseResource{ - TrackedResource: v1.TrackedResource{ - ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - Type: "Foo.Bar/myResources", - }, - InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: "2024-01-01", - }, - }, - Properties: map[string]any{}, - } - - resp, err := filter(ctx, resource, nil, &controller.Options{}) - - require.NoError(t, err) - require.Nil(t, resp) - }) - - t.Run("continues on schema fetch error", func(t *testing.T) { - // Create a client that will fail to fetch the schema - clientFactory, err := testUpdateFilterUCPClientFactoryWithError() - require.NoError(t, err) - - factory := &UpdateFilterFactory{ - UCPClient: clientFactory, - } - - filter := factory.NewPrepareResourceFilter() - - resource := &datamodel.DynamicResource{ - BaseResource: v1.BaseResource{ - TrackedResource: v1.TrackedResource{ - ID: "/planes/radius/local/resourceGroups/test/providers/Foo.Bar/myResources/test", - Type: "Foo.Bar/myResources", - }, - InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: "2024-01-01", - }, - }, - Properties: map[string]any{}, - } - - // Should not fail even if schema fetch fails - resp, err := filter(ctx, resource, nil, &controller.Options{}) - - require.NoError(t, err) - require.Nil(t, resp) - }) -} - -// testUCPClientFactory creates a mock UCP client factory that returns the given schema. -func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { - apiVersionsServer := fake.APIVersionsServer{ - Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { - resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} - resp.SetResponse(http.StatusOK, v20231001preview.APIVersionsClientGetResponse{ - APIVersionResource: v20231001preview.APIVersionResource{ - Properties: &v20231001preview.APIVersionProperties{ - Schema: schema, - }, - }, - }, nil) - return resp, azfake.ErrorResponder{} - }, - } - - return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ - APIVersionsServer: apiVersionsServer, - }), - }, - }) -} - -// testUpdateFilterUCPClientFactoryWithError creates a mock UCP client factory that returns an error. -func testUpdateFilterUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { - apiVersionsServer := fake.APIVersionsServer{ - Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { - resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} - errResp := azfake.ErrorResponder{} - errResp.SetResponseError(http.StatusNotFound, "NotFound") - return resp, errResp - }, - } - - return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ - APIVersionsServer: apiVersionsServer, - }), - }, - }) -} From cc1801ae86b761190cf756ae9e224301f3242661 Mon Sep 17 00:00:00 2001 From: Lakshmi Javadekar <103459615+lakshmimsft@users.noreply.github.com> Date: Mon, 9 Feb 2026 17:06:08 -0800 Subject: [PATCH 04/10] Add frontend encryption (#11157) This pull request encrypts sensitive fields in dynamic resource properties before they are saved to the database. The main changes add a filter that detects sensitive fields using resource schemas, encrypts them using a key from Kubernetes secrets. The changes also include comprehensive unit tests and necessary wiring in the service and route initialization. ref: [design doc](https://github.com/radius-project/design-notes/blob/main/resources/2025-11-11-secrets-redactdata.md) note: this pr depends on merge of pr https://github.com/radius-project/radius/pull/11098 for tests to run. - This pull request adds or changes features of Radius and has an approved issue (#11092). Fixes: #11092 Please verify that the PR meets the following requirements, where applicable: - An overview of proposed schema changes is included in a linked GitHub issue. - [ ] Yes - [x] Not applicable - A design document PR is created in the [design-notes repository](https://github.com/radius-project/design-notes/), if new APIs are being introduced. - [x] Yes - [ ] Not applicable - The design document has been reviewed and approved by Radius maintainers/approvers. - [x] Yes - [ ] Not applicable - A PR for the [samples repository](https://github.com/radius-project/samples) is created, if existing samples are affected by the changes in this PR. - [ ] Yes - [x] Not applicable - A PR for the [documentation repository](https://github.com/radius-project/docs) is created, if the changes in this PR affect the documentation or any user facing updates are made. - [ ] Yes - [x] Not applicable - A PR for the [recipes repository](https://github.com/radius-project/recipes) is created, if existing recipes are affected by the changes in this PR. - [ ] Yes - [x] Not applicable --------- Signed-off-by: lakshmimsft Signed-off-by: sk593 --- pkg/dynamicrp/frontend/routes.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pkg/dynamicrp/frontend/routes.go b/pkg/dynamicrp/frontend/routes.go index 5493969d60..0a7ceeaa30 100644 --- a/pkg/dynamicrp/frontend/routes.go +++ b/pkg/dynamicrp/frontend/routes.go @@ -25,12 +25,19 @@ import ( "github.com/radius-project/radius/pkg/armrpc/frontend/controller" "github.com/radius-project/radius/pkg/armrpc/frontend/defaultoperation" "github.com/radius-project/radius/pkg/crypto/encryption" + "github.com/radius-project/radius/pkg/crypto/encryption" "github.com/radius-project/radius/pkg/dynamicrp/datamodel" "github.com/radius-project/radius/pkg/dynamicrp/datamodel/converter" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" "github.com/radius-project/radius/pkg/validator" ) +func (s *Service) registerRoutes( + r *chi.Mux, + controllerOptions controller.Options, + ucpClient *v20231001preview.ClientFactory, + handler *encryption.SensitiveDataHandler, +) error { func (s *Service) registerRoutes( r *chi.Mux, controllerOptions controller.Options, @@ -64,6 +71,20 @@ func (s *Service) registerRoutes( AsyncOperationTimeout: time.Hour * 24, } + // Create encryption filter for sensitive fields + encryptionFilter := makeEncryptionFilter(ucpClient, handler) + + // Resource options with encryption filter applied to PUT operations + resourceOptions := controller.ResourceOptions[datamodel.DynamicResource]{ + RequestConverter: converter.DynamicResourceDataModelFromVersioned, + ResponseConverter: converter.DynamicResourceDataModelToVersioned, + UpdateFilters: []controller.UpdateFilter[datamodel.DynamicResource]{ + encryptionFilter, + }, + AsyncOperationRetryAfter: time.Second * 5, + AsyncOperationTimeout: time.Hour * 24, + } + r.Route(pathBase+"planes/radius/{planeName}", func(r chi.Router) { // Plane-scoped From eeb9be146157c2d341ca6dcebb82850f182633a8 Mon Sep 17 00:00:00 2001 From: sk593 Date: Tue, 10 Feb 2026 09:31:32 -0800 Subject: [PATCH 05/10] update get and list resource Signed-off-by: sk593 --- pkg/dynamicrp/frontend/getresource.go | 126 +++- pkg/dynamicrp/frontend/getresource_test.go | 177 ++++- pkg/dynamicrp/frontend/listresources.go | 7 +- pkg/dynamicrp/frontend/listresources_test.go | 620 ++++++++++++++++++ .../controller/createorupdateresource.go | 15 +- 5 files changed, 926 insertions(+), 19 deletions(-) create mode 100644 pkg/dynamicrp/frontend/listresources_test.go diff --git a/pkg/dynamicrp/frontend/getresource.go b/pkg/dynamicrp/frontend/getresource.go index e728cc33d7..e984241d78 100644 --- a/pkg/dynamicrp/frontend/getresource.go +++ b/pkg/dynamicrp/frontend/getresource.go @@ -19,6 +19,7 @@ package frontend import ( "context" "net/http" + "strings" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" ctrl "github.com/radius-project/radius/pkg/armrpc/frontend/controller" @@ -48,6 +49,12 @@ func NewGetResourceWithRedaction( } // Run returns the requested resource with sensitive fields redacted. +// +// Design consideration (GET Operation Update): When provisioningState is "Succeeded", +// the backend has already redacted sensitive data from the database, so we skip the +// schema fetch and redaction (fast path). For all other states (e.g., "Updating", +// "Accepted", "Failed"), the resource may still contain encrypted data, so we fetch +// the schema and redact sensitive fields to prevent exposure. func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWriter, req *http.Request) (rest.Response, error) { serviceCtx := v1.ARMRequestContextFromContext(ctx) logger := ucplog.FromContextOrDiscard(ctx) @@ -60,8 +67,10 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite return rest.NewNotFoundResponse(serviceCtx.ResourceID), nil } - // Redact sensitive fields before returning the response - if resource.Properties != nil { + // Fast path: if provisioningState is Succeeded, the backend has already redacted + // sensitive fields. Skip the schema fetch for better performance. + provisioningState := resource.ProvisioningState() + if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { resourceID := serviceCtx.ResourceID.String() resourceType := serviceCtx.ResourceID.Type() apiVersion := serviceCtx.APIVersion @@ -83,6 +92,7 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite redactField(resource.Properties, path) } logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in GET response", + "provisioningState", provisioningState, "count", len(sensitiveFieldPaths), "resourceType", resourceType) } } @@ -91,18 +101,116 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite } // redactField sets the field at the given path to nil. -// Supports simple field names like "data" or nested paths like "config.password". +// Supports: +// - Simple field names: "data" +// - Nested dot-separated paths: "credentials.password" +// - Array wildcards: "secrets[*].value" +// - Map wildcards: "config[*]" func redactField(properties map[string]any, path string) { - if properties == nil { + if properties == nil || path == "" { return } - // For simple paths (no dots), just set to nil - if _, exists := properties[path]; exists { - properties[path] = nil + segments := parseRedactPath(path) + if len(segments) == 0 { return } - // For nested paths, we would need to traverse - but for now we only support top-level - // The "data" field is top-level in Radius.Security/secrets + redactAtSegments(properties, segments) +} + +// redactPathSegment represents a component of a field path for redaction. +type redactPathSegment struct { + name string // field name (empty for wildcard) + wildcard bool // true for [*] segments +} + +// parseRedactPath parses a field path like "credentials.password" or "secrets[*].value" +// into path segments for traversal. +func parseRedactPath(path string) []redactPathSegment { + var segments []redactPathSegment + var current strings.Builder + + for i := 0; i < len(path); i++ { + switch path[i] { + case '.': + if current.Len() > 0 { + segments = append(segments, redactPathSegment{name: current.String()}) + current.Reset() + } + case '[': + if current.Len() > 0 { + segments = append(segments, redactPathSegment{name: current.String()}) + current.Reset() + } + end := strings.Index(path[i:], "]") + if end == -1 { + return nil // invalid path - unterminated bracket + } + content := path[i+1 : i+end] + if content == "*" { + segments = append(segments, redactPathSegment{wildcard: true}) + } + i += end // skip past ']' + default: + current.WriteByte(path[i]) + } + } + + if current.Len() > 0 { + segments = append(segments, redactPathSegment{name: current.String()}) + } + + return segments +} + +// redactAtSegments traverses the data following the path segments and sets the final value to nil. +func redactAtSegments(current any, segments []redactPathSegment) { + if len(segments) == 0 { + return + } + + segment := segments[0] + remaining := segments[1:] + + if segment.wildcard { + // Handle [*] - iterate over array or map + switch v := current.(type) { + case []any: + for i := range v { + if len(remaining) == 0 { + v[i] = nil + } else { + redactAtSegments(v[i], remaining) + } + } + case map[string]any: + for key := range v { + if len(remaining) == 0 { + v[key] = nil + } else { + redactAtSegments(v[key], remaining) + } + } + } + return + } + + // Handle field name + dataMap, ok := current.(map[string]any) + if !ok { + return + } + + value, exists := dataMap[segment.name] + if !exists { + return + } + + if len(remaining) == 0 { + dataMap[segment.name] = nil + return + } + + redactAtSegments(value, remaining) } diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go index ad2e9b35e8..2c580a2645 100644 --- a/pkg/dynamicrp/frontend/getresource_test.go +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -111,22 +111,136 @@ func TestRedactField_MultipleFields(t *testing.T) { require.Nil(t, properties["data"]) } -func TestRedactField_NestedFieldNotSupported(t *testing.T) { - // Test that nested paths (with dots) are not currently supported - // The redactField function currently only handles top-level fields +func TestRedactField_NestedDotPath(t *testing.T) { + // Test redacting a nested field via dot-separated path properties := map[string]any{ "config": map[string]any{ "password": "secret", + "host": "localhost", }, } - // Nested path - should not redact since we only support top-level redactField(properties, "config.password") - // Config should still contain the password (nested redaction not supported) config, ok := properties["config"].(map[string]any) require.True(t, ok) - require.Equal(t, "secret", config["password"]) + require.Nil(t, config["password"]) + require.Equal(t, "localhost", config["host"]) +} + +func TestRedactField_DeeplyNestedDotPath(t *testing.T) { + // Test redacting a deeply nested field + properties := map[string]any{ + "level1": map[string]any{ + "level2": map[string]any{ + "secret": "top-secret", + "other": "keep-this", + }, + }, + } + + redactField(properties, "level1.level2.secret") + + level1 := properties["level1"].(map[string]any) + level2 := level1["level2"].(map[string]any) + require.Nil(t, level2["secret"]) + require.Equal(t, "keep-this", level2["other"]) +} + +func TestRedactField_ArrayWildcard(t *testing.T) { + // Test redacting fields within array elements using [*] + properties := map[string]any{ + "secrets": []any{ + map[string]any{"name": "secret1", "value": "s1"}, + map[string]any{"name": "secret2", "value": "s2"}, + }, + } + + redactField(properties, "secrets[*].value") + + secrets := properties["secrets"].([]any) + s0 := secrets[0].(map[string]any) + s1 := secrets[1].(map[string]any) + require.Nil(t, s0["value"]) + require.Nil(t, s1["value"]) + require.Equal(t, "secret1", s0["name"]) + require.Equal(t, "secret2", s1["name"]) +} + +func TestRedactField_MapWildcard(t *testing.T) { + // Test redacting all values of a map using [*] + properties := map[string]any{ + "config": map[string]any{ + "key1": "value1", + "key2": "value2", + }, + } + + redactField(properties, "config[*]") + + config := properties["config"].(map[string]any) + require.Nil(t, config["key1"]) + require.Nil(t, config["key2"]) +} + +func TestRedactField_MapWildcardWithNestedField(t *testing.T) { + // Test redacting a nested field within map values using [*] + properties := map[string]any{ + "backends": map[string]any{ + "kv": map[string]any{"url": "http://vault", "token": "secret-token"}, + "azure": map[string]any{"url": "http://azure", "token": "azure-token"}, + }, + } + + redactField(properties, "backends[*].token") + + backends := properties["backends"].(map[string]any) + kv := backends["kv"].(map[string]any) + azure := backends["azure"].(map[string]any) + require.Nil(t, kv["token"]) + require.Nil(t, azure["token"]) + require.Equal(t, "http://vault", kv["url"]) + require.Equal(t, "http://azure", azure["url"]) +} + +func TestRedactField_NestedPathFieldNotFound(t *testing.T) { + // Test that a non-existent nested path doesn't cause errors + properties := map[string]any{ + "config": map[string]any{ + "host": "localhost", + }, + } + + // Should not panic - field doesn't exist at this nested path + redactField(properties, "config.nonexistent") + + config := properties["config"].(map[string]any) + require.Equal(t, "localhost", config["host"]) +} + +func TestRedactField_EmptyPath(t *testing.T) { + // Test that empty path doesn't cause errors + properties := map[string]any{ + "data": "value", + } + + redactField(properties, "") + + require.Equal(t, "value", properties["data"]) +} + +func TestRedactField_ArrayWildcardAllElements(t *testing.T) { + // Test redacting all elements in an array using [*] as the final segment + properties := map[string]any{ + "tokens": []any{"token1", "token2", "token3"}, + } + + redactField(properties, "tokens[*]") + + tokens := properties["tokens"].([]any) + for _, token := range tokens { + require.Nil(t, token) + } } func TestRedactField_FieldWithNilValue(t *testing.T) { @@ -184,3 +298,54 @@ func TestRedactField_FieldWithDifferentTypes(t *testing.T) { }) } } + +func TestParseRedactPath(t *testing.T) { + tests := []struct { + name string + path string + expected []redactPathSegment + }{ + { + name: "simple field", + path: "data", + expected: []redactPathSegment{{name: "data"}}, + }, + { + name: "nested dot path", + path: "credentials.password", + expected: []redactPathSegment{{name: "credentials"}, {name: "password"}}, + }, + { + name: "array wildcard", + path: "secrets[*].value", + expected: []redactPathSegment{{name: "secrets"}, {wildcard: true}, {name: "value"}}, + }, + { + name: "map wildcard", + path: "config[*]", + expected: []redactPathSegment{{name: "config"}, {wildcard: true}}, + }, + { + name: "deeply nested", + path: "a.b.c.d", + expected: []redactPathSegment{{name: "a"}, {name: "b"}, {name: "c"}, {name: "d"}}, + }, + { + name: "wildcard with nested field", + path: "backends[*].token", + expected: []redactPathSegment{{name: "backends"}, {wildcard: true}, {name: "token"}}, + }, + { + name: "empty path", + path: "", + expected: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := parseRedactPath(tc.path) + require.Equal(t, tc.expected, result) + }) + } +} diff --git a/pkg/dynamicrp/frontend/listresources.go b/pkg/dynamicrp/frontend/listresources.go index 12dcf4c4a4..0227ad56eb 100644 --- a/pkg/dynamicrp/frontend/listresources.go +++ b/pkg/dynamicrp/frontend/listresources.go @@ -77,8 +77,11 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri return nil, err } - // Redact sensitive fields before adding to the response - if resource.Properties != nil { + // Redact sensitive fields before adding to the response. + // Fast path: if provisioningState is Succeeded, the backend has already redacted + // sensitive fields. Skip redaction for these items. + provisioningState := resource.ProvisioningState() + if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { // Fetch sensitive field paths once for this resource type if !sensitiveFieldPathsFetched { sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( diff --git a/pkg/dynamicrp/frontend/listresources_test.go b/pkg/dynamicrp/frontend/listresources_test.go new file mode 100644 index 0000000000..6f523fe135 --- /dev/null +++ b/pkg/dynamicrp/frontend/listresources_test.go @@ -0,0 +1,620 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + ctrl "github.com/radius-project/radius/pkg/armrpc/frontend/controller" + "github.com/radius-project/radius/pkg/armrpc/rest" + "github.com/radius-project/radius/pkg/armrpc/rpctest" + "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel/converter" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +const ( + testListURL = "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources?api-version=2023-10-01-preview" +) + +func newTestListController(t *testing.T, databaseClient database.Client, ucpClient *v20231001preview.ClientFactory) ctrl.Controller { + t.Helper() + + opts := ctrl.Options{ + DatabaseClient: databaseClient, + } + resourceOpts := ctrl.ResourceOptions[datamodel.DynamicResource]{ + ResponseConverter: converter.DynamicResourceDataModelToVersioned, + } + c, err := NewListResourcesWithRedaction(opts, resourceOpts, ucpClient) + require.NoError(t, err) + + return c +} + +func newTestDynamicResource(id string, name string, provisioningState v1.ProvisioningState, properties map[string]any) *datamodel.DynamicResource { + return &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: id, + Name: name, + Type: "Applications.Test/testResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: testAPIVersion, + AsyncProvisioningState: provisioningState, + }, + }, + Properties: properties, + } +} + +func TestListResourcesWithRedaction_EmptyResult(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{Items: []database.Object{}}, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + _ = resp.Apply(ctx, w, req) + require.Equal(t, http.StatusOK, w.Result().StatusCode) +} + +func TestListResourcesWithRedaction_SucceededResourcesNotRedacted(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateSucceeded, + map[string]any{ + "name": "test", + "password": "secret123", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + // Schema should NOT be fetched for Succeeded resources + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + // Verify we get a valid paginated response with one item + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_NonSucceededResourcesRedacted(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateAccepted, + map[string]any{ + "name": "test", + "password": "secret123", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + // Verify the response is valid + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_MixedProvisioningStates(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + succeededResource := newTestDynamicResource( + testResourceID, + "succeededResource", + v1.ProvisioningStateSucceeded, + map[string]any{ + "name": "succeeded", + "password": "already-redacted", + }, + ) + acceptedResource := newTestDynamicResource( + "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/acceptedResource", + "acceptedResource", + v1.ProvisioningStateAccepted, + map[string]any{ + "name": "accepted", + "password": "should-be-redacted", + }, + ) + failedResource := newTestDynamicResource( + "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/failedResource", + "failedResource", + v1.ProvisioningStateFailed, + map[string]any{ + "name": "failed", + "password": "should-also-be-redacted", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{ + *rpctest.FakeStoreObject(succeededResource), + *rpctest.FakeStoreObject(acceptedResource), + *rpctest.FakeStoreObject(failedResource), + }, + }, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 3) +} + +func TestListResourcesWithRedaction_NoSensitiveFields(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateAccepted, + map[string]any{ + "name": "test", + "value": "not-sensitive", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + ucpClient, err := testUCPClientFactoryNoSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_SchemaFetchError(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateAccepted, + map[string]any{ + "name": "test", + "password": "secret123", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + // Use a UCP client that returns an error for schema fetch + ucpClient, err := testUCPClientFactoryWithError() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + // Should succeed even though schema fetch fails (continues without redaction) + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_DatabaseQueryError(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, fmt.Errorf("database connection error")) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.Error(t, err) + require.Nil(t, resp) +} + +func TestListResourcesWithRedaction_EmptyProperties(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateAccepted, + map[string]any{}, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + // Should handle empty properties gracefully (no fields to redact) + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_NestedSensitiveFields(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateAccepted, + map[string]any{ + "name": "test", + "credentials": map[string]any{ + "username": "user", + "password": "secret123", + }, + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + ucpClient, err := testUCPClientFactoryWithNestedSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_NilUCPClient(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateAccepted, + map[string]any{ + "name": "test", + "password": "secret123", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + }, nil) + + // nil UCP client — schema fetch should fail gracefully + c := newTestListController(t, databaseClient, nil) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + // Should handle nil UCP client gracefully (continues without redaction) + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) +} + +func TestListResourcesWithRedaction_MultipleNonSucceededAllRedacted(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource1 := newTestDynamicResource( + testResourceID, + "resource1", + v1.ProvisioningStateUpdating, + map[string]any{ + "name": "res1", + "password": "secret1", + }, + ) + resource2 := newTestDynamicResource( + "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/resource2", + "resource2", + v1.ProvisioningStateUpdating, + map[string]any{ + "name": "res2", + "password": "secret2", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{ + *rpctest.FakeStoreObject(resource1), + *rpctest.FakeStoreObject(resource2), + }, + }, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + // Verify both items are returned + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 2) +} + +func TestListResourcesWithRedaction_PaginationToken(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newTestDynamicResource( + testResourceID, + "myResource", + v1.ProvisioningStateSucceeded, + map[string]any{ + "name": "test", + }, + ) + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Query(gomock.Any(), gomock.Any(), gomock.Any()). + Return(&database.ObjectQueryResult{ + Items: []database.Object{*rpctest.FakeStoreObject(resource)}, + PaginationToken: "next-page-token", + }, nil) + + ucpClient, err := testUCPClientFactoryNoSensitiveFields() + require.NoError(t, err) + + c := newTestListController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testListURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + paginatedResp, ok := resp.(*rest.OKResponse) + require.True(t, ok) + paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) + require.True(t, ok) + require.Len(t, paginatedList.Value, 1) + // NextLink should be set when pagination token is present + require.NotEmpty(t, paginatedList.NextLink) +} + +func TestNewListResourcesWithRedaction(t *testing.T) { + opts := ctrl.Options{ + DatabaseClient: nil, + } + resourceOpts := ctrl.ResourceOptions[datamodel.DynamicResource]{ + ResponseConverter: converter.DynamicResourceDataModelToVersioned, + } + + c, err := NewListResourcesWithRedaction(opts, resourceOpts, nil) + require.NoError(t, err) + require.NotNil(t, c) + + listCtrl, ok := c.(*ListResourcesWithRedaction) + require.True(t, ok) + require.False(t, listCtrl.listRecursiveQuery) + require.Nil(t, listCtrl.ucpClient) +} + +func TestNewListResourcesWithRedaction_RecursiveQuery(t *testing.T) { + opts := ctrl.Options{ + DatabaseClient: nil, + } + resourceOpts := ctrl.ResourceOptions[datamodel.DynamicResource]{ + ResponseConverter: converter.DynamicResourceDataModelToVersioned, + ListRecursiveQuery: true, + } + + c, err := NewListResourcesWithRedaction(opts, resourceOpts, nil) + require.NoError(t, err) + require.NotNil(t, c) + + listCtrl, ok := c.(*ListResourcesWithRedaction) + require.True(t, ok) + require.True(t, listCtrl.listRecursiveQuery) +} diff --git a/pkg/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index e5a4ef9b58..250452cfc0 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource.go +++ b/pkg/portableresources/backend/controller/createorupdateresource.go @@ -179,7 +179,7 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques if supportsRecipes && recipeDataModel.GetRecipe() != nil { recipeOutput, err = c.executeRecipeIfNeeded(ctx, resource, recipeDataModel, previousOutputResources, config.Simulated, recipeProperties) if err != nil { - return c.handleRecipeError(ctx, err, recipeDataModel, req.ResourceID, currentETag, logger) + return c.handleRecipeError(ctx, err, recipeDataModel, req.ResourceID, currentETag, logger, redactionCompleted) } } @@ -225,7 +225,9 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques } // handleRecipeError handles recipe execution errors: logs, updates status, persists, and returns the appropriate result. -func (c *CreateOrUpdateResource[P, T]) handleRecipeError(ctx context.Context, err error, recipeDataModel datamodel.RecipeDataModel, resourceID string, etag string, logger logr.Logger) (ctrl.Result, error) { +// When redactionCompleted is true, all failure paths return NewFailedResult to prevent retries that would fail +// because sensitive data has already been nullified from the database. +func (c *CreateOrUpdateResource[P, T]) handleRecipeError(ctx context.Context, err error, recipeDataModel datamodel.RecipeDataModel, resourceID string, etag string, logger logr.Logger, redactionCompleted bool) (ctrl.Result, error) { var recipeErr *recipes.RecipeError if errors.As(err, &recipeErr) { logger.Error(recipeErr, fmt.Sprintf("failed to execute recipe. Encountered error while processing %s ", recipeErr.ErrorDetails.Target)) @@ -240,12 +242,21 @@ func (c *CreateOrUpdateResource[P, T]) handleRecipeError(ctx context.Context, er // Save portable resource with updated deployment status to track errors during deletion. err = c.DatabaseClient().Save(ctx, update, database.WithETag(etag)) if err != nil { + if redactionCompleted { + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } return ctrl.Result{}, err } return ctrl.NewFailedResult(recipeErr.ErrorDetails), nil } + // For non-RecipeError: if sensitive data was redacted, prevent retry since + // the data is gone from the database and retry would fail. + if redactionCompleted { + return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err + } + return ctrl.Result{}, err } From b19446ae7c1d15129eea9ed5a9058cb316168f7c Mon Sep 17 00:00:00 2001 From: sk593 Date: Tue, 10 Feb 2026 21:48:08 -0800 Subject: [PATCH 06/10] update after rebase Signed-off-by: sk593 --- pkg/dynamicrp/api/dynamicresource.go | 10 + .../api/dynamicresource_conversion.go | 9 +- .../api/dynamicresource_conversion_test.go | 3 +- .../backend/controller/dynamicresource.go | 1 + pkg/dynamicrp/backend/service.go | 7 + .../converter/dynamicresource_converter.go | 2 + pkg/dynamicrp/frontend/getresource.go | 31 ++- pkg/dynamicrp/frontend/getresource_test.go | 251 ++++++++++++++++++ pkg/dynamicrp/frontend/listresources.go | 18 +- pkg/dynamicrp/frontend/routes.go | 21 -- pkg/schema/annotations.go | 2 +- pkg/schema/sensitive_validation.go | 200 ++++++++++++++ pkg/schema/sensitive_validation_test.go | 110 ++++++++ pkg/schema/validator.go | 29 ++ 14 files changed, 667 insertions(+), 27 deletions(-) create mode 100644 pkg/schema/sensitive_validation.go create mode 100644 pkg/schema/sensitive_validation_test.go diff --git a/pkg/dynamicrp/api/dynamicresource.go b/pkg/dynamicrp/api/dynamicresource.go index 35a95941c9..a7fe1867b4 100644 --- a/pkg/dynamicrp/api/dynamicresource.go +++ b/pkg/dynamicrp/api/dynamicresource.go @@ -35,4 +35,14 @@ type DynamicResource struct { Properties map[string]any `json:"properties,omitempty"` // SystemData stores the system data of the resource. SystemData map[string]any `json:"systemData,omitempty"` + + // apiVersion stores the API version for this resource (not serialized). + // This is set by the converter and used during ConvertTo for proper schema lookups. + apiVersion string +} + +// SetAPIVersion sets the API version for this resource. +// This is used by the converter to pass the actual API version from the request. +func (d *DynamicResource) SetAPIVersion(version string) { + d.apiVersion = version } diff --git a/pkg/dynamicrp/api/dynamicresource_conversion.go b/pkg/dynamicrp/api/dynamicresource_conversion.go index 1934e3c041..6b0f87f50c 100644 --- a/pkg/dynamicrp/api/dynamicresource_conversion.go +++ b/pkg/dynamicrp/api/dynamicresource_conversion.go @@ -45,6 +45,13 @@ func (d *DynamicResource) ConvertTo() (v1.DataModelInterface, error) { return nil, fmt.Errorf("failed to unmarshal properties: %w", err) } + // Use the API version from the versioned model if available. + // This is set by the converter and allows proper schema lookups for decryption/redaction. + apiVersion := d.apiVersion + if apiVersion == "" { + apiVersion = Version + } + dm := &datamodel.DynamicResource{ BaseResource: v1.BaseResource{ TrackedResource: v1.TrackedResource{ @@ -55,7 +62,7 @@ func (d *DynamicResource) ConvertTo() (v1.DataModelInterface, error) { Tags: to.StringMap(d.Tags), }, InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: Version, + UpdatedAPIVersion: apiVersion, }, }, Properties: properties, diff --git a/pkg/dynamicrp/api/dynamicresource_conversion_test.go b/pkg/dynamicrp/api/dynamicresource_conversion_test.go index 936448b6a1..58da09a3aa 100644 --- a/pkg/dynamicrp/api/dynamicresource_conversion_test.go +++ b/pkg/dynamicrp/api/dynamicresource_conversion_test.go @@ -48,7 +48,7 @@ func Test_DynamicResource_ConvertVersionedToDataModel(t *testing.T) { }, }, InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: Version, + UpdatedAPIVersion: "2025-08-01-preview", }, }, Properties: map[string]any{ @@ -65,6 +65,7 @@ func Test_DynamicResource_ConvertVersionedToDataModel(t *testing.T) { err := json.Unmarshal(rawPayload, r) require.NoError(t, err) + r.SetAPIVersion("2025-08-01-preview") dm, err := r.ConvertTo() if tt.err != nil { diff --git a/pkg/dynamicrp/backend/controller/dynamicresource.go b/pkg/dynamicrp/backend/controller/dynamicresource.go index 7d2f110c5b..a8ba4eba6c 100644 --- a/pkg/dynamicrp/backend/controller/dynamicresource.go +++ b/pkg/dynamicrp/backend/controller/dynamicresource.go @@ -92,6 +92,7 @@ func (c *DynamicResourceController) selectController(ctx context.Context, reques DatabaseClient: c.DatabaseClient(), ResourceType: *resourceTypeDetails.Name, UcpClient: c.ucp, + KubeClient: c.KubeClient(), } switch operationType.Method { diff --git a/pkg/dynamicrp/backend/service.go b/pkg/dynamicrp/backend/service.go index 87ca865372..8b810a07cc 100644 --- a/pkg/dynamicrp/backend/service.go +++ b/pkg/dynamicrp/backend/service.go @@ -18,6 +18,7 @@ package backend import ( "context" + "fmt" ctrl "github.com/radius-project/radius/pkg/armrpc/asyncoperation/controller" "github.com/radius-project/radius/pkg/armrpc/asyncoperation/worker" @@ -91,8 +92,14 @@ func (w *Service) Run(ctx context.Context) error { } func (w *Service) registerControllers() error { + kubeClient, err := w.options.KubernetesProvider.RuntimeClient() + if err != nil { + return fmt.Errorf("failed to get Kubernetes runtime client: %w", err) + } + options := ctrl.Options{ DatabaseClient: w.Service.DatabaseClient, + KubeClient: kubeClient, } ucp, err := v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, sdk.NewClientOptions(w.options.UCP)) diff --git a/pkg/dynamicrp/datamodel/converter/dynamicresource_converter.go b/pkg/dynamicrp/datamodel/converter/dynamicresource_converter.go index 6483c5e136..0c1db8a2de 100644 --- a/pkg/dynamicrp/datamodel/converter/dynamicresource_converter.go +++ b/pkg/dynamicrp/datamodel/converter/dynamicresource_converter.go @@ -44,6 +44,8 @@ func DynamicResourceDataModelFromVersioned(content []byte, version string) (*dat if err := json.Unmarshal(content, vm); err != nil { return nil, err } + // Set the API version on the versioned model so ConvertTo can use it + vm.SetAPIVersion(version) dm, err := vm.ConvertTo() if err != nil { return nil, err diff --git a/pkg/dynamicrp/frontend/getresource.go b/pkg/dynamicrp/frontend/getresource.go index e984241d78..cf1e7dbbe3 100644 --- a/pkg/dynamicrp/frontend/getresource.go +++ b/pkg/dynamicrp/frontend/getresource.go @@ -73,7 +73,7 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { resourceID := serviceCtx.ResourceID.String() resourceType := serviceCtx.ResourceID.Type() - apiVersion := serviceCtx.APIVersion + apiVersion := getResourceAPIVersion(serviceCtx.APIVersion, resource) sensitiveFieldPaths, err := schema.GetSensitiveFieldPaths( ctx, @@ -82,6 +82,19 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite resourceType, apiVersion, ) + if err != nil { + fallbackAPIVersion := getResourceAPIVersion("", resource) + if fallbackAPIVersion != "" && fallbackAPIVersion != apiVersion { + sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( + ctx, + c.ucpClient, + resourceID, + resourceType, + fallbackAPIVersion, + ) + apiVersion = fallbackAPIVersion + } + } if err != nil { logger.Error(err, "Failed to fetch sensitive field paths for GET redaction", "resourceType", resourceType, "apiVersion", apiVersion) @@ -100,6 +113,22 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite return c.ConstructSyncResponse(ctx, req.Method, etag, resource) } +func getResourceAPIVersion(requestAPIVersion string, resource *datamodel.DynamicResource) string { + if requestAPIVersion != "" { + return requestAPIVersion + } + + if resource == nil { + return "" + } + + metadata := resource.InternalMetadata + if metadata.UpdatedAPIVersion != "" { + return metadata.UpdatedAPIVersion + } + return metadata.CreatedAPIVersion +} + // redactField sets the field at the given path to nil. // Supports: // - Simple field names: "data" diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go index 2c580a2645..8b06b2fd97 100644 --- a/pkg/dynamicrp/frontend/getresource_test.go +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -16,6 +16,257 @@ limitations under the License. package frontend +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" + azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" + "github.com/radius-project/radius/pkg/armrpc/frontend/controller" + "github.com/radius-project/radius/pkg/armrpc/rest" + "github.com/radius-project/radius/pkg/armrpc/rpctest" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" + "github.com/radius-project/radius/pkg/components/database" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel" + "github.com/radius-project/radius/pkg/dynamicrp/datamodel/converter" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" + "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +const ( + testGetURL = "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/myResource?api-version=2023-10-01-preview" + getTestResourceID = "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/myResource" + getTestAPIVersion = "2023-10-01-preview" +) + +func newTestGetController(t *testing.T, databaseClient database.Client, ucpClient *v20231001preview.ClientFactory) controller.Controller { + t.Helper() + + opts := controller.Options{ + DatabaseClient: databaseClient, + } + resourceOpts := controller.ResourceOptions[datamodel.DynamicResource]{ + ResponseConverter: converter.DynamicResourceDataModelToVersioned, + } + + c, err := NewGetResourceWithRedaction(opts, resourceOpts, ucpClient) + require.NoError(t, err) + + return c +} + +func newGetTestDynamicResource(provisioningState v1.ProvisioningState, properties map[string]any) *datamodel.DynamicResource { + return &datamodel.DynamicResource{ + BaseResource: v1.BaseResource{ + TrackedResource: v1.TrackedResource{ + ID: getTestResourceID, + Name: "myResource", + Type: "Applications.Test/testResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: getTestAPIVersion, + AsyncProvisioningState: provisioningState, + }, + }, + Properties: properties, + } +} + +func TestGetResourceWithRedaction_NonSucceededRedacts(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newGetTestDynamicResource(v1.ProvisioningStateAccepted, map[string]any{ + "password": "secret123", + }) + + storeObject := rpctest.FakeStoreObject(resource) + storeObject.Metadata = database.Metadata{ID: getTestResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), getTestResourceID). + Return(storeObject, nil) + + ucpClient, err := testGetUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestGetController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testGetURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + _, ok := resp.(*rest.OKResponse) + require.True(t, ok) + _ = resp.Apply(ctx, w, req) + require.Equal(t, http.StatusOK, w.Result().StatusCode) + + var body map[string]any + require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) + properties, ok := body["properties"].(map[string]any) + require.True(t, ok) + require.Nil(t, properties["password"]) +} + +func TestGetResourceWithRedaction_SucceededSkipsRedaction(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newGetTestDynamicResource(v1.ProvisioningStateSucceeded, map[string]any{ + "password": "secret123", + }) + + storeObject := rpctest.FakeStoreObject(resource) + storeObject.Metadata = database.Metadata{ID: getTestResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), getTestResourceID). + Return(storeObject, nil) + + ucpClient, err := testGetUCPClientFactoryWithSensitiveFields() + require.NoError(t, err) + + c := newTestGetController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testGetURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + _ = resp.Apply(ctx, w, req) + require.Equal(t, http.StatusOK, w.Result().StatusCode) + + var body map[string]any + require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) + properties, ok := body["properties"].(map[string]any) + require.True(t, ok) + require.Equal(t, "secret123", properties["password"]) +} + +func TestGetResourceWithRedaction_SchemaFetchErrorContinues(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() + + resource := newGetTestDynamicResource(v1.ProvisioningStateAccepted, map[string]any{ + "password": "secret123", + }) + + storeObject := rpctest.FakeStoreObject(resource) + storeObject.Metadata = database.Metadata{ID: getTestResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), getTestResourceID). + Return(storeObject, nil) + + ucpClient, err := testGetUCPClientFactoryWithError() + require.NoError(t, err) + + c := newTestGetController(t, databaseClient, ucpClient) + + req, err := http.NewRequest(http.MethodGet, testGetURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() + + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) + + _ = resp.Apply(ctx, w, req) + require.Equal(t, http.StatusOK, w.Result().StatusCode) + + var body map[string]any + require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) + properties, ok := body["properties"].(map[string]any) + require.True(t, ok) + require.Equal(t, "secret123", properties["password"]) +} + +func testGetUCPClientFactoryWithSensitiveFields() (*v20231001preview.ClientFactory, error) { + return createGetFakeUCPClientFactory(map[string]any{ + "type": "object", + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + "x-radius-sensitive": true, + }, + }, + }) +} + +func testGetUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { + apiVersionsServer := fake.APIVersionsServer{ + Get: func(ctx context.Context, planeName, resourceProviderName, resourceTypeName, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { + errResp.SetResponseError(http.StatusNotFound, "NotFound") + return + }, + } + + return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: fake.NewAPIVersionsServerTransport(&apiVersionsServer), + }, + }) +} + +func createGetFakeUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { + apiVersionsServer := fake.APIVersionsServer{ + Get: func(ctx context.Context, planeName, resourceProviderName, resourceTypeName, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { + response := v20231001preview.APIVersionsClientGetResponse{ + APIVersionResource: v20231001preview.APIVersionResource{ + Name: &apiVersionName, + Properties: &v20231001preview.APIVersionProperties{ + Schema: schema, + }, + }, + } + resp.SetResponse(http.StatusOK, response, nil) + return + }, + } + + return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Transport: fake.NewAPIVersionsServerTransport(&apiVersionsServer), + }, + }) +}/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package frontend + import ( "testing" diff --git a/pkg/dynamicrp/frontend/listresources.go b/pkg/dynamicrp/frontend/listresources.go index 0227ad56eb..3c857f84cb 100644 --- a/pkg/dynamicrp/frontend/listresources.go +++ b/pkg/dynamicrp/frontend/listresources.go @@ -84,16 +84,30 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { // Fetch sensitive field paths once for this resource type if !sensitiveFieldPathsFetched { + apiVersion := getResourceAPIVersion(serviceCtx.APIVersion, resource) sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( ctx, c.ucpClient, resource.ID, serviceCtx.ResourceID.Type(), - serviceCtx.APIVersion, + apiVersion, ) + if err != nil { + fallbackAPIVersion := getResourceAPIVersion("", resource) + if fallbackAPIVersion != "" && fallbackAPIVersion != apiVersion { + sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( + ctx, + c.ucpClient, + resource.ID, + serviceCtx.ResourceID.Type(), + fallbackAPIVersion, + ) + apiVersion = fallbackAPIVersion + } + } if err != nil { logger.Error(err, "Failed to fetch sensitive field paths for LIST redaction", - "resourceType", serviceCtx.ResourceID.Type(), "apiVersion", serviceCtx.APIVersion) + "resourceType", serviceCtx.ResourceID.Type(), "apiVersion", apiVersion) // Continue without redaction on error } sensitiveFieldPathsFetched = true diff --git a/pkg/dynamicrp/frontend/routes.go b/pkg/dynamicrp/frontend/routes.go index 0a7ceeaa30..5493969d60 100644 --- a/pkg/dynamicrp/frontend/routes.go +++ b/pkg/dynamicrp/frontend/routes.go @@ -25,19 +25,12 @@ import ( "github.com/radius-project/radius/pkg/armrpc/frontend/controller" "github.com/radius-project/radius/pkg/armrpc/frontend/defaultoperation" "github.com/radius-project/radius/pkg/crypto/encryption" - "github.com/radius-project/radius/pkg/crypto/encryption" "github.com/radius-project/radius/pkg/dynamicrp/datamodel" "github.com/radius-project/radius/pkg/dynamicrp/datamodel/converter" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" "github.com/radius-project/radius/pkg/validator" ) -func (s *Service) registerRoutes( - r *chi.Mux, - controllerOptions controller.Options, - ucpClient *v20231001preview.ClientFactory, - handler *encryption.SensitiveDataHandler, -) error { func (s *Service) registerRoutes( r *chi.Mux, controllerOptions controller.Options, @@ -71,20 +64,6 @@ func (s *Service) registerRoutes( AsyncOperationTimeout: time.Hour * 24, } - // Create encryption filter for sensitive fields - encryptionFilter := makeEncryptionFilter(ucpClient, handler) - - // Resource options with encryption filter applied to PUT operations - resourceOptions := controller.ResourceOptions[datamodel.DynamicResource]{ - RequestConverter: converter.DynamicResourceDataModelFromVersioned, - ResponseConverter: converter.DynamicResourceDataModelToVersioned, - UpdateFilters: []controller.UpdateFilter[datamodel.DynamicResource]{ - encryptionFilter, - }, - AsyncOperationRetryAfter: time.Second * 5, - AsyncOperationTimeout: time.Hour * 24, - } - r.Route(pathBase+"planes/radius/{planeName}", func(r chi.Router) { // Plane-scoped diff --git a/pkg/schema/annotations.go b/pkg/schema/annotations.go index 0ff0409d29..2c4445d915 100644 --- a/pkg/schema/annotations.go +++ b/pkg/schema/annotations.go @@ -103,7 +103,7 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { } // Check if this field has the x-radius-sensitive annotation - // If sensitive, add the path and skip nested properties since the entire field is sensitive + // If sensitive, treat the whole field as sensitive and skip nested properties. if isSensitive, ok := fieldSchemaMap[annotationRadiusSensitive].(bool); ok && isSensitive { paths = append(paths, fullPath) continue diff --git a/pkg/schema/sensitive_validation.go b/pkg/schema/sensitive_validation.go new file mode 100644 index 0000000000..184170564d --- /dev/null +++ b/pkg/schema/sensitive_validation.go @@ -0,0 +1,200 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schema + +import "strings" + +type sensitivePathSegment struct { + name string + wildcard bool +} + +func sanitizeSensitiveEncryptedValues(properties map[string]any, schema map[string]any) { + if properties == nil || schema == nil { + return + } + + paths := ExtractSensitiveFieldPaths(schema, "") + for _, path := range paths { + sanitizeEncryptedAtPath(properties, schema, path) + } +} + +func sanitizeEncryptedAtPath(properties map[string]any, schema map[string]any, path string) { + segments := parseSensitivePath(path) + if len(segments) == 0 { + return + } + + fieldSchema := getSchemaForSensitivePath(schema, segments) + placeholder := placeholderValue(fieldSchema) + applySanitizeAtSegments(properties, segments, placeholder) +} + +func applySanitizeAtSegments(current any, segments []sensitivePathSegment, placeholder any) { + if len(segments) == 0 { + return + } + + segment := segments[0] + remaining := segments[1:] + + if segment.wildcard { + switch v := current.(type) { + case []any: + for i := range v { + if len(remaining) == 0 { + v[i] = sanitizeValueIfEncrypted(v[i], placeholder) + } else { + applySanitizeAtSegments(v[i], remaining, placeholder) + } + } + case map[string]any: + for key := range v { + if len(remaining) == 0 { + v[key] = sanitizeValueIfEncrypted(v[key], placeholder) + } else { + applySanitizeAtSegments(v[key], remaining, placeholder) + } + } + } + return + } + + dataMap, ok := current.(map[string]any) + if !ok { + return + } + + value, exists := dataMap[segment.name] + if !exists { + return + } + + if len(remaining) == 0 { + dataMap[segment.name] = sanitizeValueIfEncrypted(value, placeholder) + return + } + + applySanitizeAtSegments(value, remaining, placeholder) +} + +func sanitizeValueIfEncrypted(value any, placeholder any) any { + encMap, ok := value.(map[string]any) + if !ok { + return value + } + + _, hasEncrypted := encMap["encrypted"].(string) + _, hasNonce := encMap["nonce"].(string) + if !hasEncrypted || !hasNonce { + return value + } + + return placeholder +} + +func placeholderValue(schema map[string]any) any { + if schema == nil { + return "" + } + + if t, ok := schema["type"].(string); ok { + switch t { + case "object": + return map[string]any{} + case "array": + return []any{} + default: + return "" + } + } + + return "" +} + +func getSchemaForSensitivePath(schema map[string]any, segments []sensitivePathSegment) map[string]any { + current := schema + + for _, segment := range segments { + if segment.wildcard { + if items, ok := current["items"].(map[string]any); ok { + current = items + continue + } + if addProps, ok := current["additionalProperties"].(map[string]any); ok { + current = addProps + continue + } + return nil + } + + properties, ok := current["properties"].(map[string]any) + if !ok { + return nil + } + + next, ok := properties[segment.name].(map[string]any) + if !ok { + return nil + } + current = next + } + + return current +} + +func parseSensitivePath(path string) []sensitivePathSegment { + if path == "" { + return nil + } + + var segments []sensitivePathSegment + var current strings.Builder + + for i := 0; i < len(path); i++ { + switch path[i] { + case '.': + if current.Len() > 0 { + segments = append(segments, sensitivePathSegment{name: current.String()}) + current.Reset() + } + case '[': + if current.Len() > 0 { + segments = append(segments, sensitivePathSegment{name: current.String()}) + current.Reset() + } + end := strings.Index(path[i:], "]") + if end == -1 { + return nil + } + content := path[i+1 : i+end] + if content == "*" { + segments = append(segments, sensitivePathSegment{wildcard: true}) + } + i += end + default: + current.WriteByte(path[i]) + } + } + + if current.Len() > 0 { + segments = append(segments, sensitivePathSegment{name: current.String()}) + } + + return segments +} diff --git a/pkg/schema/sensitive_validation_test.go b/pkg/schema/sensitive_validation_test.go new file mode 100644 index 0000000000..d776997f5a --- /dev/null +++ b/pkg/schema/sensitive_validation_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2023 The Radius Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schema + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSanitizeSensitiveEncryptedValues_StringPlaceholder(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "password": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + }, + }, + } + + properties := map[string]any{ + "password": map[string]any{ + "encrypted": "ciphertext", + "nonce": "nonce", + "version": 1, + }, + } + + sanitizeSensitiveEncryptedValues(properties, schema) + + value, ok := properties["password"] + require.True(t, ok) + require.Equal(t, "", value) +} + +func TestSanitizeSensitiveEncryptedValues_ObjectPlaceholder(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "data": map[string]any{ + "type": "object", + annotationRadiusSensitive: true, + "properties": map[string]any{ + "value": map[string]any{ + "type": "string", + }, + }, + }, + }, + } + + properties := map[string]any{ + "data": map[string]any{ + "encrypted": "ciphertext", + "nonce": "nonce", + "version": 1, + }, + } + + sanitizeSensitiveEncryptedValues(properties, schema) + + value, ok := properties["data"] + require.True(t, ok) + require.Equal(t, map[string]any{}, value) +} + +func TestSanitizeSensitiveEncryptedValues_MapValues(t *testing.T) { + schema := map[string]any{ + "properties": map[string]any{ + "secretMap": map[string]any{ + "type": "object", + "additionalProperties": map[string]any{ + "type": "string", + annotationRadiusSensitive: true, + }, + }, + }, + } + + properties := map[string]any{ + "secretMap": map[string]any{ + "first": map[string]any{ + "encrypted": "ciphertext", + "nonce": "nonce", + "version": 1, + }, + "second": "plain", + }, + } + + sanitizeSensitiveEncryptedValues(properties, schema) + + secretMap, ok := properties["secretMap"].(map[string]any) + require.True(t, ok) + require.Equal(t, "", secretMap["first"]) + require.Equal(t, "plain", secretMap["second"]) +} diff --git a/pkg/schema/validator.go b/pkg/schema/validator.go index c3b3c47a42..3e21eed8f7 100644 --- a/pkg/schema/validator.go +++ b/pkg/schema/validator.go @@ -490,6 +490,24 @@ func ConvertToOpenAPISchema(schemaData any) (*openapi3.Schema, error) { return &schema, nil } +func deepCopyMap(source map[string]any) (map[string]any, error) { + if source == nil { + return map[string]any{}, nil + } + + bytes, err := json.Marshal(source) + if err != nil { + return nil, err + } + + var copy map[string]any + if err = json.Unmarshal(bytes, ©); err != nil { + return nil, err + } + + return copy, nil +} + // validateSchemaWithOpenAPI validates schema data by creating a minimal OpenAPI document // and using the library's built-in validation which includes format validation func (v *Validator) validateSchemaWithOpenAPI(schema *openapi3.Schema) error { @@ -890,6 +908,17 @@ func ValidateResourceAgainstSchema(ctx context.Context, resourceData map[string] return fmt.Errorf("resource data missing 'properties' field") } + if schemaMap, ok := schemaData.(map[string]any); ok { + if propertiesMap, ok := propertiesData.(map[string]any); ok { + propertiesCopy, err := deepCopyMap(propertiesMap) + if err != nil { + return fmt.Errorf("failed to copy properties: %w", err) + } + sanitizeSensitiveEncryptedValues(propertiesCopy, schemaMap) + propertiesData = propertiesCopy + } + } + if err := schemaRef.Value.VisitJSON(propertiesData); err != nil { // Try to extract structured error information if openAPIErr, ok := err.(*openapi3.SchemaError); ok { From c88cdf4467ca155cb1745a6c57d9898419c70420 Mon Sep 17 00:00:00 2001 From: sk593 Date: Fri, 13 Feb 2026 14:12:03 -0800 Subject: [PATCH 07/10] address comments Signed-off-by: sk593 --- pkg/crypto/encryption/sensitive.go | 134 ++---------- pkg/crypto/encryption/sensitive_test.go | 59 +++--- pkg/dynamicrp/frontend/getresource.go | 168 ++------------- pkg/dynamicrp/frontend/getresource_test.go | 89 +++----- pkg/dynamicrp/frontend/listresources.go | 55 ++--- pkg/dynamicrp/frontend/listresources_test.go | 15 +- .../controller/createorupdateresource.go | 8 +- .../controller/createorupdateresource_test.go | 97 ++++----- pkg/schema/annotations.go | 190 +++++++++++++++++ pkg/schema/sensitive_validation.go | 200 ------------------ pkg/schema/sensitive_validation_test.go | 110 ---------- pkg/schema/validator.go | 29 --- 12 files changed, 375 insertions(+), 779 deletions(-) delete mode 100644 pkg/schema/sensitive_validation.go delete mode 100644 pkg/schema/sensitive_validation_test.go diff --git a/pkg/crypto/encryption/sensitive.go b/pkg/crypto/encryption/sensitive.go index 55235e7711..caf022c553 100644 --- a/pkg/crypto/encryption/sensitive.go +++ b/pkg/crypto/encryption/sensitive.go @@ -22,7 +22,8 @@ import ( "errors" "fmt" "strconv" - "strings" + + "github.com/radius-project/radius/pkg/schema" ) var ( @@ -168,15 +169,7 @@ func (h *SensitiveDataHandler) DecryptSensitiveFieldsWithSchema(ctx context.Cont // // Fields that are not found are skipped - this allows optional sensitive fields to be absent. func (h *SensitiveDataHandler) RedactSensitiveFields(data map[string]any, sensitiveFieldPaths []string) error { - for _, path := range sensitiveFieldPaths { - if err := h.redactFieldAtPath(data, path); err != nil { - // Skip fields that are not found - they may not exist in this resource instance - if errors.Is(err, ErrFieldNotFound) { - continue - } - return fmt.Errorf("%w: path %q: %v", ErrFieldRedactionFailed, path, err) - } - } + schema.RedactFields(data, sensitiveFieldPaths) return nil } @@ -226,21 +219,13 @@ func (h *SensitiveDataHandler) decryptFieldAtPath(ctx context.Context, data map[ return h.processFieldAtPath(data, path, processor) } -// redactFieldAtPath replaces the value at the given field path with nil. -func (h *SensitiveDataHandler) redactFieldAtPath(data map[string]any, path string) error { - processor := func(any) (any, error) { - return nil, nil - } - return h.processFieldAtPath(data, path, processor) -} - // processFieldAtPath traverses the data structure and applies the processor function to the field at the path. func (h *SensitiveDataHandler) processFieldAtPath(data map[string]any, path string, processor func(any) (any, error)) error { if path == "" { return ErrInvalidFieldPath } - segments := parseFieldPath(path) + segments := schema.ParseFieldPath(path) if len(segments) == 0 { return ErrInvalidFieldPath } @@ -249,7 +234,7 @@ func (h *SensitiveDataHandler) processFieldAtPath(data map[string]any, path stri } // processPathSegments recursively processes path segments to find and transform the target field. -func (h *SensitiveDataHandler) processPathSegments(current any, segments []pathSegment, processor func(any) (any, error)) error { +func (h *SensitiveDataHandler) processPathSegments(current any, segments []schema.FieldPathSegment, processor func(any) (any, error)) error { if len(segments) == 0 { return nil } @@ -257,20 +242,19 @@ func (h *SensitiveDataHandler) processPathSegments(current any, segments []pathS segment := segments[0] remainingSegments := segments[1:] - switch segment.segmentType { - case segmentTypeField: - return h.processFieldSegment(current, segment.value, remainingSegments, processor) - case segmentTypeWildcard: + if segment.IsField() { + return h.processFieldSegment(current, segment.Value, remainingSegments, processor) + } else if segment.IsWildcard() { return h.processWildcardSegment(current, remainingSegments, processor) - case segmentTypeIndex: - return h.processIndexSegment(current, segment.value, remainingSegments, processor) - default: - return ErrInvalidFieldPath + } else if segment.IsIndex() { + return h.processIndexSegment(current, segment.Value, remainingSegments, processor) } + + return ErrInvalidFieldPath } // processFieldSegment handles a regular field name segment in the path. -func (h *SensitiveDataHandler) processFieldSegment(current any, fieldName string, remainingSegments []pathSegment, processor func(any) (any, error)) error { +func (h *SensitiveDataHandler) processFieldSegment(current any, fieldName string, remainingSegments []schema.FieldPathSegment, processor func(any) (any, error)) error { dataMap, ok := current.(map[string]any) if !ok { return ErrFieldNotFound @@ -296,7 +280,7 @@ func (h *SensitiveDataHandler) processFieldSegment(current any, fieldName string } // processWildcardSegment handles [*] segments for arrays and maps. -func (h *SensitiveDataHandler) processWildcardSegment(current any, remainingSegments []pathSegment, processor func(any) (any, error)) error { +func (h *SensitiveDataHandler) processWildcardSegment(current any, remainingSegments []schema.FieldPathSegment, processor func(any) (any, error)) error { // Handle array if arr, ok := current.([]any); ok { for i := range arr { @@ -347,7 +331,7 @@ func (h *SensitiveDataHandler) processWildcardSegment(current any, remainingSegm } // processIndexSegment handles specific index segments like [0], [1], etc. -func (h *SensitiveDataHandler) processIndexSegment(current any, indexStr string, remainingSegments []pathSegment, processor func(any) (any, error)) error { +func (h *SensitiveDataHandler) processIndexSegment(current any, indexStr string, remainingSegments []schema.FieldPathSegment, processor func(any) (any, error)) error { arr, ok := current.([]any) if !ok { return ErrFieldNotFound @@ -506,29 +490,28 @@ func getSchemaType(schema map[string]any) string { // getSchemaForPath retrieves the schema definition for a specific field path. // It navigates through the schema following the path segments (supporting nested properties, // array items via [*], and additionalProperties for maps). -func getSchemaForPath(schema map[string]any, path string) map[string]any { - if schema == nil || path == "" { +func getSchemaForPath(schemaData map[string]any, path string) map[string]any { + if schemaData == nil || path == "" { return nil } - segments := parseFieldPath(path) - current := schema + segments := schema.ParseFieldPath(path) + current := schemaData for _, segment := range segments { - switch segment.segmentType { - case segmentTypeField: + if segment.IsField() { // Navigate to properties -> fieldName properties, ok := current["properties"].(map[string]any) if !ok { return nil } - fieldSchema, ok := properties[segment.value].(map[string]any) + fieldSchema, ok := properties[segment.Value].(map[string]any) if !ok { return nil } current = fieldSchema - case segmentTypeWildcard: + } else if segment.IsWildcard() { // Could be array items or additionalProperties if items, ok := current["items"].(map[string]any); ok { current = items @@ -538,7 +521,7 @@ func getSchemaForPath(schema map[string]any, path string) map[string]any { return nil } - case segmentTypeIndex: + } else if segment.IsIndex() { // Specific array index - use items schema if items, ok := current["items"].(map[string]any); ok { current = items @@ -625,74 +608,3 @@ func coerceTypesFromSchema(data map[string]any, schema map[string]any) { } } } - -// pathSegment represents a segment of a field path. -type pathSegment struct { - segmentType segmentType - value string -} - -type segmentType int - -const ( - segmentTypeField segmentType = iota - segmentTypeWildcard - segmentTypeIndex -) - -// parseFieldPath parses a field path into segments. -// Examples: -// - "credentials.password" -> [field:credentials, field:password] -// - "secrets[*].value" -> [field:secrets, wildcard, field:value] -// - "config[*]" -> [field:config, wildcard] -// - "items[0].name" -> [field:items, index:0, field:name] -func parseFieldPath(path string) []pathSegment { - var segments []pathSegment - var current strings.Builder - - i := 0 - for i < len(path) { - ch := path[i] - - switch ch { - case '.': - if current.Len() > 0 { - segments = append(segments, pathSegment{segmentType: segmentTypeField, value: current.String()}) - current.Reset() - } - i++ - - case '[': - if current.Len() > 0 { - segments = append(segments, pathSegment{segmentType: segmentTypeField, value: current.String()}) - current.Reset() - } - - // Find the closing bracket - end := strings.Index(path[i:], "]") - if end == -1 { - // Invalid path - unterminated bracket, return nil to signal error - return nil - } - - bracketContent := path[i+1 : i+end] - if bracketContent == "*" { - segments = append(segments, pathSegment{segmentType: segmentTypeWildcard}) - } else { - segments = append(segments, pathSegment{segmentType: segmentTypeIndex, value: bracketContent}) - } - i += end + 1 - - default: - current.WriteByte(ch) - i++ - } - } - - // Don't forget the last segment - if current.Len() > 0 { - segments = append(segments, pathSegment{segmentType: segmentTypeField, value: current.String()}) - } - - return segments -} diff --git a/pkg/crypto/encryption/sensitive_test.go b/pkg/crypto/encryption/sensitive_test.go index 70e0ae0fed..1d98e4a569 100644 --- a/pkg/crypto/encryption/sensitive_test.go +++ b/pkg/crypto/encryption/sensitive_test.go @@ -20,6 +20,7 @@ import ( "context" "testing" + "github.com/radius-project/radius/pkg/schema" "github.com/stretchr/testify/require" ) @@ -32,68 +33,68 @@ func TestParseFieldPath(t *testing.T) { tests := []struct { name string path string - expected []pathSegment + expected []schema.FieldPathSegment }{ { name: "simple-field", path: "password", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "password"}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "password"}, }, }, { name: "nested-field", path: "credentials.password", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "credentials"}, - {segmentType: segmentTypeField, value: "password"}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "credentials"}, + {Type: schema.SegmentTypeField, Value: "password"}, }, }, { name: "deeply-nested-field", path: "config.database.connection.password", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "config"}, - {segmentType: segmentTypeField, value: "database"}, - {segmentType: segmentTypeField, value: "connection"}, - {segmentType: segmentTypeField, value: "password"}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "config"}, + {Type: schema.SegmentTypeField, Value: "database"}, + {Type: schema.SegmentTypeField, Value: "connection"}, + {Type: schema.SegmentTypeField, Value: "password"}, }, }, { name: "array-wildcard", path: "secrets[*].value", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "secrets"}, - {segmentType: segmentTypeWildcard}, - {segmentType: segmentTypeField, value: "value"}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "secrets"}, + {Type: schema.SegmentTypeWildcard}, + {Type: schema.SegmentTypeField, Value: "value"}, }, }, { name: "map-wildcard", path: "config[*]", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "config"}, - {segmentType: segmentTypeWildcard}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "config"}, + {Type: schema.SegmentTypeWildcard}, }, }, { name: "specific-index", path: "items[0].name", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "items"}, - {segmentType: segmentTypeIndex, value: "0"}, - {segmentType: segmentTypeField, value: "name"}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "items"}, + {Type: schema.SegmentTypeIndex, Value: "0"}, + {Type: schema.SegmentTypeField, Value: "name"}, }, }, { name: "multiple-wildcards", path: "data[*].secrets[*].value", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "data"}, - {segmentType: segmentTypeWildcard}, - {segmentType: segmentTypeField, value: "secrets"}, - {segmentType: segmentTypeWildcard}, - {segmentType: segmentTypeField, value: "value"}, + expected: []schema.FieldPathSegment{ + {Type: schema.SegmentTypeField, Value: "data"}, + {Type: schema.SegmentTypeWildcard}, + {Type: schema.SegmentTypeField, Value: "secrets"}, + {Type: schema.SegmentTypeWildcard}, + {Type: schema.SegmentTypeField, Value: "value"}, }, }, { @@ -110,7 +111,7 @@ func TestParseFieldPath(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := parseFieldPath(tt.path) + result := schema.ParseFieldPath(tt.path) require.Equal(t, tt.expected, result) }) } diff --git a/pkg/dynamicrp/frontend/getresource.go b/pkg/dynamicrp/frontend/getresource.go index cf1e7dbbe3..e19bb42a9e 100644 --- a/pkg/dynamicrp/frontend/getresource.go +++ b/pkg/dynamicrp/frontend/getresource.go @@ -19,7 +19,6 @@ package frontend import ( "context" "net/http" - "strings" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" ctrl "github.com/radius-project/radius/pkg/armrpc/frontend/controller" @@ -73,7 +72,10 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { resourceID := serviceCtx.ResourceID.String() resourceType := serviceCtx.ResourceID.Type() - apiVersion := getResourceAPIVersion(serviceCtx.APIVersion, resource) + + // Use the API version the resource was last updated with to ensure + // encryption and redaction use the same schema + apiVersion := resource.InternalMetadata.UpdatedAPIVersion sensitiveFieldPaths, err := schema.GetSensitiveFieldPaths( ctx, @@ -82,28 +84,21 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite resourceType, apiVersion, ) - if err != nil { - fallbackAPIVersion := getResourceAPIVersion("", resource) - if fallbackAPIVersion != "" && fallbackAPIVersion != apiVersion { - sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( - ctx, - c.ucpClient, - resourceID, - resourceType, - fallbackAPIVersion, - ) - apiVersion = fallbackAPIVersion - } - } if err != nil { logger.Error(err, "Failed to fetch sensitive field paths for GET redaction", "resourceType", resourceType, "apiVersion", apiVersion) - // Continue without redaction on error - don't fail the GET - } else if len(sensitiveFieldPaths) > 0 { - // Redact sensitive fields by setting them to nil - for _, path := range sensitiveFieldPaths { - redactField(resource.Properties, path) - } + // Fail-safe: return error to prevent potential exposure of sensitive data + // This is consistent with the write path (encryption filter) + return rest.NewInternalServerErrorARMResponse(v1.ErrorResponse{ + Error: &v1.ErrorDetails{ + Code: v1.CodeInternal, + Message: "Failed to fetch schema for security validation", + }, + }), nil + } + + if len(sensitiveFieldPaths) > 0 { + schema.RedactFields(resource.Properties, sensitiveFieldPaths) logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in GET response", "provisioningState", provisioningState, "count", len(sensitiveFieldPaths), "resourceType", resourceType) @@ -112,134 +107,3 @@ func (c *GetResourceWithRedaction) Run(ctx context.Context, w http.ResponseWrite return c.ConstructSyncResponse(ctx, req.Method, etag, resource) } - -func getResourceAPIVersion(requestAPIVersion string, resource *datamodel.DynamicResource) string { - if requestAPIVersion != "" { - return requestAPIVersion - } - - if resource == nil { - return "" - } - - metadata := resource.InternalMetadata - if metadata.UpdatedAPIVersion != "" { - return metadata.UpdatedAPIVersion - } - return metadata.CreatedAPIVersion -} - -// redactField sets the field at the given path to nil. -// Supports: -// - Simple field names: "data" -// - Nested dot-separated paths: "credentials.password" -// - Array wildcards: "secrets[*].value" -// - Map wildcards: "config[*]" -func redactField(properties map[string]any, path string) { - if properties == nil || path == "" { - return - } - - segments := parseRedactPath(path) - if len(segments) == 0 { - return - } - - redactAtSegments(properties, segments) -} - -// redactPathSegment represents a component of a field path for redaction. -type redactPathSegment struct { - name string // field name (empty for wildcard) - wildcard bool // true for [*] segments -} - -// parseRedactPath parses a field path like "credentials.password" or "secrets[*].value" -// into path segments for traversal. -func parseRedactPath(path string) []redactPathSegment { - var segments []redactPathSegment - var current strings.Builder - - for i := 0; i < len(path); i++ { - switch path[i] { - case '.': - if current.Len() > 0 { - segments = append(segments, redactPathSegment{name: current.String()}) - current.Reset() - } - case '[': - if current.Len() > 0 { - segments = append(segments, redactPathSegment{name: current.String()}) - current.Reset() - } - end := strings.Index(path[i:], "]") - if end == -1 { - return nil // invalid path - unterminated bracket - } - content := path[i+1 : i+end] - if content == "*" { - segments = append(segments, redactPathSegment{wildcard: true}) - } - i += end // skip past ']' - default: - current.WriteByte(path[i]) - } - } - - if current.Len() > 0 { - segments = append(segments, redactPathSegment{name: current.String()}) - } - - return segments -} - -// redactAtSegments traverses the data following the path segments and sets the final value to nil. -func redactAtSegments(current any, segments []redactPathSegment) { - if len(segments) == 0 { - return - } - - segment := segments[0] - remaining := segments[1:] - - if segment.wildcard { - // Handle [*] - iterate over array or map - switch v := current.(type) { - case []any: - for i := range v { - if len(remaining) == 0 { - v[i] = nil - } else { - redactAtSegments(v[i], remaining) - } - } - case map[string]any: - for key := range v { - if len(remaining) == 0 { - v[key] = nil - } else { - redactAtSegments(v[key], remaining) - } - } - } - return - } - - // Handle field name - dataMap, ok := current.(map[string]any) - if !ok { - return - } - - value, exists := dataMap[segment.name] - if !exists { - return - } - - if len(remaining) == 0 { - dataMap[segment.name] = nil - return - } - - redactAtSegments(value, remaining) -} diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go index 8b06b2fd97..173e534ac8 100644 --- a/pkg/dynamicrp/frontend/getresource_test.go +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -34,6 +34,7 @@ import ( "github.com/radius-project/radius/pkg/components/database" "github.com/radius-project/radius/pkg/dynamicrp/datamodel" "github.com/radius-project/radius/pkg/dynamicrp/datamodel/converter" + "github.com/radius-project/radius/pkg/schema" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" "github.com/stretchr/testify/require" @@ -161,7 +162,7 @@ func TestGetResourceWithRedaction_SucceededSkipsRedaction(t *testing.T) { require.Equal(t, "secret123", properties["password"]) } -func TestGetResourceWithRedaction_SchemaFetchErrorContinues(t *testing.T) { +func TestGetResourceWithRedaction_SchemaFetchErrorReturnsError(t *testing.T) { mctrl := gomock.NewController(t) defer mctrl.Finish() @@ -192,13 +193,13 @@ func TestGetResourceWithRedaction_SchemaFetchErrorContinues(t *testing.T) { require.NotNil(t, resp) _ = resp.Apply(ctx, w, req) - require.Equal(t, http.StatusOK, w.Result().StatusCode) + // Expect fail-safe behavior: return error instead of exposing potentially sensitive data + require.Equal(t, http.StatusInternalServerError, w.Result().StatusCode) - var body map[string]any + var body v1.ErrorResponse require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) - properties, ok := body["properties"].(map[string]any) - require.True(t, ok) - require.Equal(t, "secret123", properties["password"]) + require.Equal(t, v1.CodeInternal, body.Error.Code) + require.Contains(t, body.Error.Message, "Failed to fetch schema for security validation") } func testGetUCPClientFactoryWithSensitiveFields() (*v20231001preview.ClientFactory, error) { @@ -249,29 +250,7 @@ func createGetFakeUCPClientFactory(schema map[string]any) (*v20231001preview.Cli Transport: fake.NewAPIVersionsServerTransport(&apiVersionsServer), }, }) -}/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package frontend - -import ( - "testing" - - "github.com/stretchr/testify/require" -) +} func TestRedactField_SimpleField(t *testing.T) { // Test redacting a simple top-level field @@ -281,7 +260,7 @@ func TestRedactField_SimpleField(t *testing.T) { "data": map[string]any{"key": "value"}, } - redactField(properties, "password") + schema.RedactFields(properties, []string{"password"}) require.Equal(t, "test-resource", properties["name"]) require.Nil(t, properties["password"]) @@ -304,7 +283,7 @@ func TestRedactField_DataField(t *testing.T) { }, } - redactField(properties, "data") + schema.RedactFields(properties, []string{"data"}) require.NotNil(t, properties["environment"]) require.NotNil(t, properties["application"]) @@ -319,7 +298,7 @@ func TestRedactField_NonExistentField(t *testing.T) { } // Should not panic or error - redactField(properties, "nonexistent") + schema.RedactFields(properties, []string{"nonexistent"}) // Original fields should remain unchanged require.Equal(t, "test-resource", properties["name"]) @@ -331,14 +310,14 @@ func TestRedactField_NilProperties(t *testing.T) { var properties map[string]any // Should not panic - redactField(properties, "anyfield") + schema.RedactFields(properties, []string{"anyfield"}) } func TestRedactField_EmptyProperties(t *testing.T) { // Test redacting from empty properties properties := map[string]any{} - redactField(properties, "password") + schema.RedactFields(properties, []string{"password"}) require.Empty(t, properties) } @@ -352,9 +331,9 @@ func TestRedactField_MultipleFields(t *testing.T) { "data": "sensitive-data", } - redactField(properties, "password") - redactField(properties, "apiKey") - redactField(properties, "data") + schema.RedactFields(properties, []string{"password"}) + schema.RedactFields(properties, []string{"apiKey"}) + schema.RedactFields(properties, []string{"data"}) require.Equal(t, "test", properties["name"]) require.Nil(t, properties["password"]) @@ -371,7 +350,7 @@ func TestRedactField_NestedDotPath(t *testing.T) { }, } - redactField(properties, "config.password") + schema.RedactFields(properties, []string{"config.password"}) config, ok := properties["config"].(map[string]any) require.True(t, ok) @@ -390,7 +369,7 @@ func TestRedactField_DeeplyNestedDotPath(t *testing.T) { }, } - redactField(properties, "level1.level2.secret") + schema.RedactFields(properties, []string{"level1.level2.secret"}) level1 := properties["level1"].(map[string]any) level2 := level1["level2"].(map[string]any) @@ -407,7 +386,7 @@ func TestRedactField_ArrayWildcard(t *testing.T) { }, } - redactField(properties, "secrets[*].value") + schema.RedactFields(properties, []string{"secrets[*].value"}) secrets := properties["secrets"].([]any) s0 := secrets[0].(map[string]any) @@ -427,7 +406,7 @@ func TestRedactField_MapWildcard(t *testing.T) { }, } - redactField(properties, "config[*]") + schema.RedactFields(properties, []string{"config[*]"}) config := properties["config"].(map[string]any) require.Nil(t, config["key1"]) @@ -443,7 +422,7 @@ func TestRedactField_MapWildcardWithNestedField(t *testing.T) { }, } - redactField(properties, "backends[*].token") + schema.RedactFields(properties, []string{"backends[*].token"}) backends := properties["backends"].(map[string]any) kv := backends["kv"].(map[string]any) @@ -463,7 +442,7 @@ func TestRedactField_NestedPathFieldNotFound(t *testing.T) { } // Should not panic - field doesn't exist at this nested path - redactField(properties, "config.nonexistent") + schema.RedactFields(properties, []string{"config.nonexistent"}) config := properties["config"].(map[string]any) require.Equal(t, "localhost", config["host"]) @@ -475,7 +454,7 @@ func TestRedactField_EmptyPath(t *testing.T) { "data": "value", } - redactField(properties, "") + schema.RedactFields(properties, []string{""}) require.Equal(t, "value", properties["data"]) } @@ -486,7 +465,7 @@ func TestRedactField_ArrayWildcardAllElements(t *testing.T) { "tokens": []any{"token1", "token2", "token3"}, } - redactField(properties, "tokens[*]") + schema.RedactFields(properties, []string{"tokens[*]"}) tokens := properties["tokens"].([]any) for _, token := range tokens { @@ -501,7 +480,7 @@ func TestRedactField_FieldWithNilValue(t *testing.T) { "password": nil, } - redactField(properties, "password") + schema.RedactFields(properties, []string{"password"}) require.Equal(t, "test", properties["name"]) require.Nil(t, properties["password"]) @@ -544,7 +523,7 @@ func TestRedactField_FieldWithDifferentTypes(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - redactField(tc.properties, tc.fieldName) + schema.RedactFields(tc.properties, []string{tc.fieldName}) require.Nil(t, tc.properties[tc.fieldName]) }) } @@ -554,37 +533,37 @@ func TestParseRedactPath(t *testing.T) { tests := []struct { name string path string - expected []redactPathSegment + expected []schema.FieldPathSegment }{ { name: "simple field", path: "data", - expected: []redactPathSegment{{name: "data"}}, + expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "data"}}, }, { name: "nested dot path", path: "credentials.password", - expected: []redactPathSegment{{name: "credentials"}, {name: "password"}}, + expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "credentials"}, {Type: schema.SegmentTypeField, Value: "password"}}, }, { name: "array wildcard", path: "secrets[*].value", - expected: []redactPathSegment{{name: "secrets"}, {wildcard: true}, {name: "value"}}, + expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "secrets"}, {Type: schema.SegmentTypeWildcard}, {Type: schema.SegmentTypeField, Value: "value"}}, }, { name: "map wildcard", path: "config[*]", - expected: []redactPathSegment{{name: "config"}, {wildcard: true}}, + expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "config"}, {Type: schema.SegmentTypeWildcard}}, }, { name: "deeply nested", path: "a.b.c.d", - expected: []redactPathSegment{{name: "a"}, {name: "b"}, {name: "c"}, {name: "d"}}, + expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "a"}, {Type: schema.SegmentTypeField, Value: "b"}, {Type: schema.SegmentTypeField, Value: "c"}, {Type: schema.SegmentTypeField, Value: "d"}}, }, { name: "wildcard with nested field", path: "backends[*].token", - expected: []redactPathSegment{{name: "backends"}, {wildcard: true}, {name: "token"}}, + expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "backends"}, {Type: schema.SegmentTypeWildcard}, {Type: schema.SegmentTypeField, Value: "token"}}, }, { name: "empty path", @@ -595,7 +574,7 @@ func TestParseRedactPath(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - result := parseRedactPath(tc.path) + result := schema.ParseFieldPath(tc.path) require.Equal(t, tc.expected, result) }) } diff --git a/pkg/dynamicrp/frontend/listresources.go b/pkg/dynamicrp/frontend/listresources.go index 3c857f84cb..921189a3cc 100644 --- a/pkg/dynamicrp/frontend/listresources.go +++ b/pkg/dynamicrp/frontend/listresources.go @@ -66,9 +66,9 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri return nil, err } - // Cache sensitive field paths for this resource type (same for all items) - var sensitiveFieldPaths []string - var sensitiveFieldPathsFetched bool + // Cache sensitive field paths per API version + // Different resources in the list may have been created with different API versions + sensitiveFieldPathsCache := make(map[string][]string) items := []any{} for _, item := range result.Items { @@ -82,9 +82,13 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri // sensitive fields. Skip redaction for these items. provisioningState := resource.ProvisioningState() if provisioningState != v1.ProvisioningStateSucceeded && resource.Properties != nil { - // Fetch sensitive field paths once for this resource type - if !sensitiveFieldPathsFetched { - apiVersion := getResourceAPIVersion(serviceCtx.APIVersion, resource) + // Use the API version the resource was last updated with to ensure + // encryption and redaction use the same schema + apiVersion := resource.InternalMetadata.UpdatedAPIVersion + + // Check cache first to avoid redundant schema fetches for same API version + sensitiveFieldPaths, cached := sensitiveFieldPathsCache[apiVersion] + if !cached { sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( ctx, c.ucpClient, @@ -92,31 +96,23 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri serviceCtx.ResourceID.Type(), apiVersion, ) - if err != nil { - fallbackAPIVersion := getResourceAPIVersion("", resource) - if fallbackAPIVersion != "" && fallbackAPIVersion != apiVersion { - sensitiveFieldPaths, err = schema.GetSensitiveFieldPaths( - ctx, - c.ucpClient, - resource.ID, - serviceCtx.ResourceID.Type(), - fallbackAPIVersion, - ) - apiVersion = fallbackAPIVersion - } - } if err != nil { logger.Error(err, "Failed to fetch sensitive field paths for LIST redaction", "resourceType", serviceCtx.ResourceID.Type(), "apiVersion", apiVersion) - // Continue without redaction on error + // Fail-safe: return error to prevent potential exposure of sensitive data + // This is consistent with the write path (encryption filter) + return rest.NewInternalServerErrorARMResponse(v1.ErrorResponse{ + Error: &v1.ErrorDetails{ + Code: v1.CodeInternal, + Message: "Failed to fetch schema for security validation", + }, + }), nil } - sensitiveFieldPathsFetched = true + sensitiveFieldPathsCache[apiVersion] = sensitiveFieldPaths } if len(sensitiveFieldPaths) > 0 { - for _, path := range sensitiveFieldPaths { - redactField(resource.Properties, path) - } + schema.RedactFields(resource.Properties, sensitiveFieldPaths) } } @@ -127,9 +123,16 @@ func (c *ListResourcesWithRedaction) Run(ctx context.Context, w http.ResponseWri items = append(items, versioned) } - if len(sensitiveFieldPaths) > 0 { + // Log redaction summary if any schemas were fetched + if len(sensitiveFieldPathsCache) > 0 { + totalSensitiveFields := 0 + for _, paths := range sensitiveFieldPathsCache { + totalSensitiveFields += len(paths) + } logger.V(ucplog.LevelDebug).Info("Redacted sensitive fields in LIST response", - "count", len(sensitiveFieldPaths), "resourceType", serviceCtx.ResourceID.Type(), + "totalSensitiveFields", totalSensitiveFields, + "apiVersions", len(sensitiveFieldPathsCache), + "resourceType", serviceCtx.ResourceID.Type(), "itemCount", len(items)) } diff --git a/pkg/dynamicrp/frontend/listresources_test.go b/pkg/dynamicrp/frontend/listresources_test.go index 6f523fe135..8442ad96f2 100644 --- a/pkg/dynamicrp/frontend/listresources_test.go +++ b/pkg/dynamicrp/frontend/listresources_test.go @@ -17,6 +17,7 @@ limitations under the License. package frontend import ( + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -322,16 +323,18 @@ func TestListResourcesWithRedaction_SchemaFetchError(t *testing.T) { ctx := rpctest.NewARMRequestContext(req) w := httptest.NewRecorder() - // Should succeed even though schema fetch fails (continues without redaction) + // Should fail-safe: return error instead of exposing potentially sensitive data resp, err := c.Run(ctx, w, req) require.NoError(t, err) require.NotNil(t, resp) - paginatedResp, ok := resp.(*rest.OKResponse) - require.True(t, ok) - paginatedList, ok := paginatedResp.Body.(*v1.PaginatedList) - require.True(t, ok) - require.Len(t, paginatedList.Value, 1) + _ = resp.Apply(ctx, w, req) + require.Equal(t, http.StatusInternalServerError, w.Result().StatusCode) + + var body v1.ErrorResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) + require.Equal(t, v1.CodeInternal, body.Error.Code) + require.Contains(t, body.Error.Message, "Failed to fetch schema for security validation") } func TestListResourcesWithRedaction_DatabaseQueryError(t *testing.T) { diff --git a/pkg/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index 250452cfc0..a2bd1ec830 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource.go +++ b/pkg/portableresources/backend/controller/createorupdateresource.go @@ -90,7 +90,7 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques resourceType := resource.GetBaseResource().Type schema, schemaErr := schemautil.GetSchema(ctx, c.UcpClient(), req.ResourceID, resourceType, apiVersion) if schemaErr != nil { - logger.Error(schemaErr, "Failed to fetch schema for sensitive field detection", "resourceID", req.ResourceID) + return ctrl.Result{}, fmt.Errorf("failed to fetch schema for sensitive field detection: %w", schemaErr) } else if schema != nil { sensitiveFieldPaths := schemautil.ExtractSensitiveFieldPaths(schema, "") if len(sensitiveFieldPaths) > 0 { @@ -329,11 +329,7 @@ func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context } func getResourceAPIVersion[P rpv1.RadiusResourceModel](resource P) string { - metadata := resource.GetBaseResource().InternalMetadata - if metadata.UpdatedAPIVersion != "" { - return metadata.UpdatedAPIVersion - } - return metadata.CreatedAPIVersion + return resource.GetBaseResource().InternalMetadata.UpdatedAPIVersion } func deepCopyProperties(source map[string]any) (map[string]any, error) { diff --git a/pkg/portableresources/backend/controller/createorupdateresource_test.go b/pkg/portableresources/backend/controller/createorupdateresource_test.go index 9aca2a9dcc..5bb293bf36 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource_test.go +++ b/pkg/portableresources/backend/controller/createorupdateresource_test.go @@ -37,9 +37,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" controllerfake "sigs.k8s.io/controller-runtime/pkg/client/fake" - aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" ctrl "github.com/radius-project/radius/pkg/armrpc/asyncoperation/controller" + aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" "github.com/radius-project/radius/pkg/components/database" "github.com/radius-project/radius/pkg/crypto/encryption" "github.com/radius-project/radius/pkg/portableresources" @@ -504,10 +504,10 @@ func TestCreateOrUpdateResource_Run_SensitiveRedaction(t *testing.T) { secretValue := "top-secret" properties := map[string]any{ - "application": TestApplicationID, - "environment": TestEnvironmentID, + "application": TestApplicationID, + "environment": TestEnvironmentID, "provisioningState": "Accepted", - "secret": secretValue, + "secret": secretValue, "recipe": map[string]any{ "name": "test-recipe", "parameters": map[string]any{ @@ -523,14 +523,12 @@ func TestCreateOrUpdateResource_Run_SensitiveRedaction(t *testing.T) { require.NoError(t, err) data := map[string]any{ - "name": "tr", - "type": TestResourceType, - "id": TestResourceID, - "location": v1.LocationGlobal, - "internalMetadata": map[string]any{ - "updatedApiVersion": "2024-01-01", - }, - "properties": properties, + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "updatedApiVersion": "2024-01-01", + "properties": properties, } msc.EXPECT(). @@ -541,8 +539,8 @@ func TestCreateOrUpdateResource_Run_SensitiveRedaction(t *testing.T) { ucpClient, err := testUCPClientFactory(map[string]any{ "properties": map[string]any{ "secret": map[string]any{ - "type": "string", - "x-radius-sensitive": true, + "type": "string", + "x-radius-sensitive": true, }, }, }) @@ -636,13 +634,11 @@ func TestCreateOrUpdateResource_Run_SensitiveMissingKey(t *testing.T) { cfg := configloader.NewMockConfigurationLoader(mctrl) data := map[string]any{ - "name": "tr", - "type": TestResourceType, - "id": TestResourceID, - "location": v1.LocationGlobal, - "internalMetadata": map[string]any{ - "updatedApiVersion": "2024-01-01", - }, + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "updatedApiVersion": "2024-01-01", "properties": map[string]any{ "application": TestApplicationID, "environment": TestEnvironmentID, @@ -665,8 +661,8 @@ func TestCreateOrUpdateResource_Run_SensitiveMissingKey(t *testing.T) { ucpClient, err := testUCPClientFactory(map[string]any{ "properties": map[string]any{ "secret": map[string]any{ - "type": "string", - "x-radius-sensitive": true, + "type": "string", + "x-radius-sensitive": true, }, }, }) @@ -708,24 +704,22 @@ func TestCreateOrUpdateResource_Run_SensitiveMissingKey(t *testing.T) { func testUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { apiVersionsServer := fake.APIVersionsServer{ - Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (azfake.Responder[v20231001preview.APIVersionsClientGetResponse], azfake.ErrorResponder) { - resp := azfake.Responder[v20231001preview.APIVersionsClientGetResponse]{} - resp.SetResponse(http.StatusOK, v20231001preview.APIVersionsClientGetResponse{ + Get: func(ctx context.Context, planeName string, resourceProviderName string, resourceTypeName string, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { + response := v20231001preview.APIVersionsClientGetResponse{ APIVersionResource: v20231001preview.APIVersionResource{ Properties: &v20231001preview.APIVersionProperties{ Schema: schema, }, }, - }, nil) - return resp, azfake.ErrorResponder{} + } + resp.SetResponse(http.StatusOK, response, nil) + return }, } return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ ClientOptions: policy.ClientOptions{ - Transport: fake.NewServerFactoryTransport(&fake.ServerFactory{ - APIVersionsServer: apiVersionsServer, - }), + Transport: fake.NewAPIVersionsServerTransport(&apiVersionsServer), }, }) } @@ -758,13 +752,11 @@ func TestCreateOrUpdateResource_Run_SensitiveNilKubeClient(t *testing.T) { cfg := configloader.NewMockConfigurationLoader(mctrl) data := map[string]any{ - "name": "tr", - "type": TestResourceType, - "id": TestResourceID, - "location": v1.LocationGlobal, - "internalMetadata": map[string]any{ - "updatedApiVersion": "2024-01-01", - }, + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "updatedApiVersion": "2024-01-01", "properties": map[string]any{ "application": TestApplicationID, "environment": TestEnvironmentID, @@ -858,14 +850,12 @@ func TestCreateOrUpdateResource_Run_SensitiveRedactionSaveFails(t *testing.T) { require.NoError(t, err) data := map[string]any{ - "name": "tr", - "type": TestResourceType, - "id": TestResourceID, - "location": v1.LocationGlobal, - "internalMetadata": map[string]any{ - "updatedApiVersion": "2024-01-01", - }, - "properties": properties, + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "updatedApiVersion": "2024-01-01", + "properties": properties, } msc.EXPECT(). @@ -981,14 +971,12 @@ func TestCreateOrUpdateResource_Run_SensitiveMultipleFields(t *testing.T) { require.NoError(t, err) data := map[string]any{ - "name": "tr", - "type": TestResourceType, - "id": TestResourceID, - "location": v1.LocationGlobal, - "internalMetadata": map[string]any{ - "updatedApiVersion": "2024-01-01", - }, - "properties": properties, + "name": "tr", + "type": TestResourceType, + "id": TestResourceID, + "location": v1.LocationGlobal, + "updatedApiVersion": "2024-01-01", + "properties": properties, } msc.EXPECT(). @@ -1104,4 +1092,3 @@ func TestCreateOrUpdateResource_Run_SensitiveMultipleFields(t *testing.T) { require.NoError(t, err) require.Equal(t, ctrl.Result{}, res) } - diff --git a/pkg/schema/annotations.go b/pkg/schema/annotations.go index 2c4445d915..03527ea3d2 100644 --- a/pkg/schema/annotations.go +++ b/pkg/schema/annotations.go @@ -18,6 +18,7 @@ package schema import ( "context" + "strconv" "strings" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" @@ -157,3 +158,192 @@ func ExtractSensitiveFieldPaths(schema map[string]any, prefix string) []string { return paths } + +// FieldPathSegment represents a single segment in a field path. +// A field path can contain field names, wildcards, and array indices. +type FieldPathSegment struct { + Type SegmentType + Value string // field name or index value (empty for wildcards) +} + +// SegmentType represents the type of a field path segment. +type SegmentType int + +const ( + // SegmentTypeField represents a named field (e.g., "password" in "credentials.password") + SegmentTypeField SegmentType = iota + // SegmentTypeWildcard represents a wildcard segment (e.g., [*] in "secrets[*].value") + SegmentTypeWildcard + // SegmentTypeIndex represents an array index (e.g., [0] in "items[0].name") + SegmentTypeIndex +) + +// IsWildcard returns true if the segment is a wildcard. +func (s FieldPathSegment) IsWildcard() bool { + return s.Type == SegmentTypeWildcard +} + +// IsField returns true if the segment is a named field. +func (s FieldPathSegment) IsField() bool { + return s.Type == SegmentTypeField +} + +// IsIndex returns true if the segment is an array index. +func (s FieldPathSegment) IsIndex() bool { + return s.Type == SegmentTypeIndex +} + +// ParseFieldPath parses a field path string into segments. +// Supports dot notation, wildcards [*], and array indices [N]. +// +// Examples: +// - "credentials.password" -> [field:credentials, field:password] +// - "secrets[*].value" -> [field:secrets, wildcard, field:value] +// - "config[*]" -> [field:config, wildcard] +// - "items[0].name" -> [field:items, index:0, field:name] +// +// Returns nil if the path is invalid (e.g., unterminated bracket). +func ParseFieldPath(path string) []FieldPathSegment { + if path == "" { + return nil + } + + var segments []FieldPathSegment + var current strings.Builder + + i := 0 + for i < len(path) { + ch := path[i] + + switch ch { + case '.': + if current.Len() > 0 { + segments = append(segments, FieldPathSegment{Type: SegmentTypeField, Value: current.String()}) + current.Reset() + } + i++ + + case '[': + if current.Len() > 0 { + segments = append(segments, FieldPathSegment{Type: SegmentTypeField, Value: current.String()}) + current.Reset() + } + + // Find the closing bracket + end := strings.Index(path[i:], "]") + if end == -1 { + // Invalid path - unterminated bracket + return nil + } + + bracketContent := path[i+1 : i+end] + if bracketContent == "*" { + segments = append(segments, FieldPathSegment{Type: SegmentTypeWildcard}) + } else { + segments = append(segments, FieldPathSegment{Type: SegmentTypeIndex, Value: bracketContent}) + } + i += end + 1 + + default: + current.WriteByte(ch) + i++ + } + } + + // Don't forget the last segment + if current.Len() > 0 { + segments = append(segments, FieldPathSegment{Type: SegmentTypeField, Value: current.String()}) + } + + return segments +} + +// RedactFields sets the values at the given field paths to nil in the data map. +// Paths support dot notation, wildcards [*], and array indices [N]. +// Missing fields and invalid paths are silently skipped. +func RedactFields(data map[string]any, paths []string) { + if data == nil { + return + } + for _, path := range paths { + if path == "" { + continue + } + segments := ParseFieldPath(path) + if len(segments) == 0 { + continue + } + redactAtSegments(data, segments) + } +} + +// redactAtSegments traverses the data following the path segments and sets the final value to nil. +func redactAtSegments(current any, segments []FieldPathSegment) { + if len(segments) == 0 { + return + } + + segment := segments[0] + remaining := segments[1:] + + // Handle wildcard [*] - iterate over array or map + if segment.IsWildcard() { + switch v := current.(type) { + case []any: + for i := range v { + if len(remaining) == 0 { + v[i] = nil + } else { + redactAtSegments(v[i], remaining) + } + } + case map[string]any: + for key := range v { + if len(remaining) == 0 { + v[key] = nil + } else { + redactAtSegments(v[key], remaining) + } + } + } + return + } + + // Handle array index [N] + if segment.IsIndex() { + arr, ok := current.([]any) + if !ok { + return + } + + idx, err := strconv.Atoi(segment.Value) + if err != nil || idx < 0 || idx >= len(arr) { + return + } + + if len(remaining) == 0 { + arr[idx] = nil + } else { + redactAtSegments(arr[idx], remaining) + } + return + } + + // Handle field name + dataMap, ok := current.(map[string]any) + if !ok { + return + } + + value, exists := dataMap[segment.Value] + if !exists { + return + } + + if len(remaining) == 0 { + dataMap[segment.Value] = nil + return + } + + redactAtSegments(value, remaining) +} diff --git a/pkg/schema/sensitive_validation.go b/pkg/schema/sensitive_validation.go deleted file mode 100644 index 184170564d..0000000000 --- a/pkg/schema/sensitive_validation.go +++ /dev/null @@ -1,200 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package schema - -import "strings" - -type sensitivePathSegment struct { - name string - wildcard bool -} - -func sanitizeSensitiveEncryptedValues(properties map[string]any, schema map[string]any) { - if properties == nil || schema == nil { - return - } - - paths := ExtractSensitiveFieldPaths(schema, "") - for _, path := range paths { - sanitizeEncryptedAtPath(properties, schema, path) - } -} - -func sanitizeEncryptedAtPath(properties map[string]any, schema map[string]any, path string) { - segments := parseSensitivePath(path) - if len(segments) == 0 { - return - } - - fieldSchema := getSchemaForSensitivePath(schema, segments) - placeholder := placeholderValue(fieldSchema) - applySanitizeAtSegments(properties, segments, placeholder) -} - -func applySanitizeAtSegments(current any, segments []sensitivePathSegment, placeholder any) { - if len(segments) == 0 { - return - } - - segment := segments[0] - remaining := segments[1:] - - if segment.wildcard { - switch v := current.(type) { - case []any: - for i := range v { - if len(remaining) == 0 { - v[i] = sanitizeValueIfEncrypted(v[i], placeholder) - } else { - applySanitizeAtSegments(v[i], remaining, placeholder) - } - } - case map[string]any: - for key := range v { - if len(remaining) == 0 { - v[key] = sanitizeValueIfEncrypted(v[key], placeholder) - } else { - applySanitizeAtSegments(v[key], remaining, placeholder) - } - } - } - return - } - - dataMap, ok := current.(map[string]any) - if !ok { - return - } - - value, exists := dataMap[segment.name] - if !exists { - return - } - - if len(remaining) == 0 { - dataMap[segment.name] = sanitizeValueIfEncrypted(value, placeholder) - return - } - - applySanitizeAtSegments(value, remaining, placeholder) -} - -func sanitizeValueIfEncrypted(value any, placeholder any) any { - encMap, ok := value.(map[string]any) - if !ok { - return value - } - - _, hasEncrypted := encMap["encrypted"].(string) - _, hasNonce := encMap["nonce"].(string) - if !hasEncrypted || !hasNonce { - return value - } - - return placeholder -} - -func placeholderValue(schema map[string]any) any { - if schema == nil { - return "" - } - - if t, ok := schema["type"].(string); ok { - switch t { - case "object": - return map[string]any{} - case "array": - return []any{} - default: - return "" - } - } - - return "" -} - -func getSchemaForSensitivePath(schema map[string]any, segments []sensitivePathSegment) map[string]any { - current := schema - - for _, segment := range segments { - if segment.wildcard { - if items, ok := current["items"].(map[string]any); ok { - current = items - continue - } - if addProps, ok := current["additionalProperties"].(map[string]any); ok { - current = addProps - continue - } - return nil - } - - properties, ok := current["properties"].(map[string]any) - if !ok { - return nil - } - - next, ok := properties[segment.name].(map[string]any) - if !ok { - return nil - } - current = next - } - - return current -} - -func parseSensitivePath(path string) []sensitivePathSegment { - if path == "" { - return nil - } - - var segments []sensitivePathSegment - var current strings.Builder - - for i := 0; i < len(path); i++ { - switch path[i] { - case '.': - if current.Len() > 0 { - segments = append(segments, sensitivePathSegment{name: current.String()}) - current.Reset() - } - case '[': - if current.Len() > 0 { - segments = append(segments, sensitivePathSegment{name: current.String()}) - current.Reset() - } - end := strings.Index(path[i:], "]") - if end == -1 { - return nil - } - content := path[i+1 : i+end] - if content == "*" { - segments = append(segments, sensitivePathSegment{wildcard: true}) - } - i += end - default: - current.WriteByte(path[i]) - } - } - - if current.Len() > 0 { - segments = append(segments, sensitivePathSegment{name: current.String()}) - } - - return segments -} diff --git a/pkg/schema/sensitive_validation_test.go b/pkg/schema/sensitive_validation_test.go deleted file mode 100644 index d776997f5a..0000000000 --- a/pkg/schema/sensitive_validation_test.go +++ /dev/null @@ -1,110 +0,0 @@ -/* -Copyright 2023 The Radius Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package schema - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestSanitizeSensitiveEncryptedValues_StringPlaceholder(t *testing.T) { - schema := map[string]any{ - "properties": map[string]any{ - "password": map[string]any{ - "type": "string", - annotationRadiusSensitive: true, - }, - }, - } - - properties := map[string]any{ - "password": map[string]any{ - "encrypted": "ciphertext", - "nonce": "nonce", - "version": 1, - }, - } - - sanitizeSensitiveEncryptedValues(properties, schema) - - value, ok := properties["password"] - require.True(t, ok) - require.Equal(t, "", value) -} - -func TestSanitizeSensitiveEncryptedValues_ObjectPlaceholder(t *testing.T) { - schema := map[string]any{ - "properties": map[string]any{ - "data": map[string]any{ - "type": "object", - annotationRadiusSensitive: true, - "properties": map[string]any{ - "value": map[string]any{ - "type": "string", - }, - }, - }, - }, - } - - properties := map[string]any{ - "data": map[string]any{ - "encrypted": "ciphertext", - "nonce": "nonce", - "version": 1, - }, - } - - sanitizeSensitiveEncryptedValues(properties, schema) - - value, ok := properties["data"] - require.True(t, ok) - require.Equal(t, map[string]any{}, value) -} - -func TestSanitizeSensitiveEncryptedValues_MapValues(t *testing.T) { - schema := map[string]any{ - "properties": map[string]any{ - "secretMap": map[string]any{ - "type": "object", - "additionalProperties": map[string]any{ - "type": "string", - annotationRadiusSensitive: true, - }, - }, - }, - } - - properties := map[string]any{ - "secretMap": map[string]any{ - "first": map[string]any{ - "encrypted": "ciphertext", - "nonce": "nonce", - "version": 1, - }, - "second": "plain", - }, - } - - sanitizeSensitiveEncryptedValues(properties, schema) - - secretMap, ok := properties["secretMap"].(map[string]any) - require.True(t, ok) - require.Equal(t, "", secretMap["first"]) - require.Equal(t, "plain", secretMap["second"]) -} diff --git a/pkg/schema/validator.go b/pkg/schema/validator.go index 3e21eed8f7..c3b3c47a42 100644 --- a/pkg/schema/validator.go +++ b/pkg/schema/validator.go @@ -490,24 +490,6 @@ func ConvertToOpenAPISchema(schemaData any) (*openapi3.Schema, error) { return &schema, nil } -func deepCopyMap(source map[string]any) (map[string]any, error) { - if source == nil { - return map[string]any{}, nil - } - - bytes, err := json.Marshal(source) - if err != nil { - return nil, err - } - - var copy map[string]any - if err = json.Unmarshal(bytes, ©); err != nil { - return nil, err - } - - return copy, nil -} - // validateSchemaWithOpenAPI validates schema data by creating a minimal OpenAPI document // and using the library's built-in validation which includes format validation func (v *Validator) validateSchemaWithOpenAPI(schema *openapi3.Schema) error { @@ -908,17 +890,6 @@ func ValidateResourceAgainstSchema(ctx context.Context, resourceData map[string] return fmt.Errorf("resource data missing 'properties' field") } - if schemaMap, ok := schemaData.(map[string]any); ok { - if propertiesMap, ok := propertiesData.(map[string]any); ok { - propertiesCopy, err := deepCopyMap(propertiesMap) - if err != nil { - return fmt.Errorf("failed to copy properties: %w", err) - } - sanitizeSensitiveEncryptedValues(propertiesCopy, schemaMap) - propertiesData = propertiesCopy - } - } - if err := schemaRef.Value.VisitJSON(propertiesData); err != nil { // Try to extract structured error information if openAPIErr, ok := err.(*openapi3.SchemaError); ok { From ee142ef09f008cf1c2da04f463ce83f039044a42 Mon Sep 17 00:00:00 2001 From: sk593 Date: Fri, 13 Feb 2026 16:26:24 -0800 Subject: [PATCH 08/10] update comments Signed-off-by: sk593 --- pkg/crypto/encryption/sensitive.go | 12 -------- pkg/crypto/encryption/sensitive_test.go | 29 ++++--------------- .../controller/createorupdateresource.go | 14 +++------ 3 files changed, 10 insertions(+), 45 deletions(-) diff --git a/pkg/crypto/encryption/sensitive.go b/pkg/crypto/encryption/sensitive.go index caf022c553..c93f6ec879 100644 --- a/pkg/crypto/encryption/sensitive.go +++ b/pkg/crypto/encryption/sensitive.go @@ -38,9 +38,6 @@ var ( // ErrFieldDecryptionFailed is returned when decryption of a field fails. ErrFieldDecryptionFailed = errors.New("field decryption failed") - - // ErrFieldRedactionFailed is returned when redaction of a field fails. - ErrFieldRedactionFailed = errors.New("field redaction failed") ) // SensitiveDataHandler provides methods for encrypting and decrypting sensitive fields @@ -164,15 +161,6 @@ func (h *SensitiveDataHandler) DecryptSensitiveFieldsWithSchema(ctx context.Cont return nil } -// RedactSensitiveFields nullifies all sensitive fields in the data based on the provided field paths. -// The data is modified in place. Field paths support dot notation and [*] for arrays/maps. -// -// Fields that are not found are skipped - this allows optional sensitive fields to be absent. -func (h *SensitiveDataHandler) RedactSensitiveFields(data map[string]any, sensitiveFieldPaths []string) error { - schema.RedactFields(data, sensitiveFieldPaths) - return nil -} - // getEncryptorForDecryption returns the appropriate encryptor for decrypting data. // If a keyProvider is available and the data contains a version, it fetches the versioned key. // Otherwise, it falls back to the default encryptor. diff --git a/pkg/crypto/encryption/sensitive_test.go b/pkg/crypto/encryption/sensitive_test.go index 1d98e4a569..d7a8cab602 100644 --- a/pkg/crypto/encryption/sensitive_test.go +++ b/pkg/crypto/encryption/sensitive_test.go @@ -152,13 +152,7 @@ func TestSensitiveDataHandler_EncryptDecrypt_SimpleField(t *testing.T) { require.Equal(t, "admin", data["username"]) } -func TestSensitiveDataHandler_RedactSensitiveFields(t *testing.T) { - key, err := GenerateKey() - require.NoError(t, err) - - handler, err := NewSensitiveDataHandlerFromKey(key) - require.NoError(t, err) - +func TestRedactFields_NestedPaths(t *testing.T) { data := map[string]any{ "username": "admin", "credentials": map[string]any{ @@ -167,8 +161,7 @@ func TestSensitiveDataHandler_RedactSensitiveFields(t *testing.T) { }, } - err = handler.RedactSensitiveFields(data, []string{"credentials.password", "credentials.token"}) - require.NoError(t, err) + schema.RedactFields(data, []string{"credentials.password", "credentials.token"}) creds := data["credentials"].(map[string]any) require.Nil(t, creds["password"]) @@ -1028,7 +1021,6 @@ func TestSensitiveDataHandler_DecryptWithMissingKeyVersion(t *testing.T) { require.Contains(t, err.Error(), "key version not found") } - func TestSensitiveDataHandler_DecryptWithADMismatch(t *testing.T) { key, err := GenerateKey() require.NoError(t, err) @@ -1109,8 +1101,7 @@ func TestSensitiveDataHandler_FullDecryptRedactWorkflow(t *testing.T) { // If this were derived from recipeProperties (decrypted), a partial redaction // failure would persist plaintext to the database. redactedProperties := deepCopyMap(dbProperties) - err = handler.RedactSensitiveFields(redactedProperties, sensitivePaths) - require.NoError(t, err) + schema.RedactFields(redactedProperties, sensitivePaths) // Step 5: Verify all invariants // — Redacted copy: sensitive fields are nil, non-sensitive fields are preserved @@ -1130,13 +1121,7 @@ func TestSensitiveDataHandler_FullDecryptRedactWorkflow(t *testing.T) { require.Equal(t, "nested-password", recipeProperties["nested"].(map[string]any)["password"]) } -func TestSensitiveDataHandler_Redact_AlreadyNilField(t *testing.T) { - key, err := GenerateKey() - require.NoError(t, err) - - handler, err := NewSensitiveDataHandlerFromKey(key) - require.NoError(t, err) - +func TestRedactFields_AlreadyNilField(t *testing.T) { // Field exists in the map but is already nil — e.g. previously redacted or // an optional sensitive field the user did not provide. data := map[string]any{ @@ -1145,14 +1130,12 @@ func TestSensitiveDataHandler_Redact_AlreadyNilField(t *testing.T) { } // Redacting an already-nil field must succeed silently - err = handler.RedactSensitiveFields(data, []string{"secret"}) - require.NoError(t, err) + schema.RedactFields(data, []string{"secret"}) require.Nil(t, data["secret"]) require.Equal(t, "admin", data["username"]) // Verify idempotence: redacting again is still a no-op - err = handler.RedactSensitiveFields(data, []string{"secret"}) - require.NoError(t, err) + schema.RedactFields(data, []string{"secret"}) require.Nil(t, data["secret"]) } diff --git a/pkg/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index a2bd1ec830..e0ae4f06bd 100644 --- a/pkg/portableresources/backend/controller/createorupdateresource.go +++ b/pkg/portableresources/backend/controller/createorupdateresource.go @@ -124,20 +124,14 @@ func (c *CreateOrUpdateResource[P, T]) Run(ctx context.Context, req *ctrl.Reques return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err } - // Security: derive the redacted copy from the original encrypted - // properties, NOT from the decrypted recipeProperties. If - // RedactSensitiveFields fails to nil a field (partial failure), - // the persisted value remains encrypted ciphertext, never - // plaintext. recipeProperties (decrypted) is kept exclusively - // for in-memory recipe execution. + // Derive the redacted copy from the original encrypted properties, + // NOT from the decrypted recipeProperties. recipeProperties + // (decrypted) is kept exclusively for in-memory recipe execution. redactedProperties, err := deepCopyProperties(properties) if err != nil { return ctrl.Result{}, err } - if err = handler.RedactSensitiveFields(redactedProperties, sensitiveFieldPaths); err != nil { - logger.Error(err, "Failed to redact sensitive fields", "resourceID", req.ResourceID) - return ctrl.NewFailedResult(v1.ErrorDetails{Message: err.Error()}), err - } + schemautil.RedactFields(redactedProperties, sensitiveFieldPaths) if err = applyPropertiesToResource(resource, redactedProperties); err != nil { logger.Error(err, "Failed to apply redacted properties", "resourceID", req.ResourceID) From 2ca13a869efd596647a14fcfd3617645f450457a Mon Sep 17 00:00:00 2001 From: sk593 Date: Tue, 17 Feb 2026 15:14:57 -0800 Subject: [PATCH 09/10] cleanup tests Signed-off-by: sk593 --- pkg/crypto/encryption/sensitive_test.go | 193 ++++----- pkg/dynamicrp/frontend/getresource_test.go | 443 +++------------------ pkg/schema/annotations_test.go | 429 ++++++++++++++++++++ 3 files changed, 550 insertions(+), 515 deletions(-) diff --git a/pkg/crypto/encryption/sensitive_test.go b/pkg/crypto/encryption/sensitive_test.go index d7a8cab602..10d275669b 100644 --- a/pkg/crypto/encryption/sensitive_test.go +++ b/pkg/crypto/encryption/sensitive_test.go @@ -29,94 +29,6 @@ const ( testResourceID = "/planes/radius/local/resourceGroups/test/providers/Test.Resource/testResources/myResource" ) -func TestParseFieldPath(t *testing.T) { - tests := []struct { - name string - path string - expected []schema.FieldPathSegment - }{ - { - name: "simple-field", - path: "password", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "password"}, - }, - }, - { - name: "nested-field", - path: "credentials.password", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "credentials"}, - {Type: schema.SegmentTypeField, Value: "password"}, - }, - }, - { - name: "deeply-nested-field", - path: "config.database.connection.password", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "config"}, - {Type: schema.SegmentTypeField, Value: "database"}, - {Type: schema.SegmentTypeField, Value: "connection"}, - {Type: schema.SegmentTypeField, Value: "password"}, - }, - }, - { - name: "array-wildcard", - path: "secrets[*].value", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "secrets"}, - {Type: schema.SegmentTypeWildcard}, - {Type: schema.SegmentTypeField, Value: "value"}, - }, - }, - { - name: "map-wildcard", - path: "config[*]", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "config"}, - {Type: schema.SegmentTypeWildcard}, - }, - }, - { - name: "specific-index", - path: "items[0].name", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "items"}, - {Type: schema.SegmentTypeIndex, Value: "0"}, - {Type: schema.SegmentTypeField, Value: "name"}, - }, - }, - { - name: "multiple-wildcards", - path: "data[*].secrets[*].value", - expected: []schema.FieldPathSegment{ - {Type: schema.SegmentTypeField, Value: "data"}, - {Type: schema.SegmentTypeWildcard}, - {Type: schema.SegmentTypeField, Value: "secrets"}, - {Type: schema.SegmentTypeWildcard}, - {Type: schema.SegmentTypeField, Value: "value"}, - }, - }, - { - name: "unterminated-bracket", - path: "secrets[*", - expected: nil, - }, - { - name: "unterminated-bracket-with-index", - path: "items[0", - expected: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := schema.ParseFieldPath(tt.path) - require.Equal(t, tt.expected, result) - }) - } -} - func TestSensitiveDataHandler_EncryptDecrypt_SimpleField(t *testing.T) { key, err := GenerateKey() require.NoError(t, err) @@ -152,23 +64,6 @@ func TestSensitiveDataHandler_EncryptDecrypt_SimpleField(t *testing.T) { require.Equal(t, "admin", data["username"]) } -func TestRedactFields_NestedPaths(t *testing.T) { - data := map[string]any{ - "username": "admin", - "credentials": map[string]any{ - "password": "nested-secret", - "token": "token-123", - }, - } - - schema.RedactFields(data, []string{"credentials.password", "credentials.token"}) - - creds := data["credentials"].(map[string]any) - require.Nil(t, creds["password"]) - require.Nil(t, creds["token"]) - require.Equal(t, "admin", data["username"]) -} - func TestSensitiveDataHandler_EncryptDecrypt_NestedField(t *testing.T) { key, err := GenerateKey() require.NoError(t, err) @@ -1121,22 +1016,86 @@ func TestSensitiveDataHandler_FullDecryptRedactWorkflow(t *testing.T) { require.Equal(t, "nested-password", recipeProperties["nested"].(map[string]any)["password"]) } -func TestRedactFields_AlreadyNilField(t *testing.T) { - // Field exists in the map but is already nil — e.g. previously redacted or - // an optional sensitive field the user did not provide. +func TestSensitiveDataHandler_DecryptNonEncryptedMap(t *testing.T) { + // When a "sensitive" field contains a plain map (not our encrypted format), + // decryptValue should pass it through unchanged instead of returning an error. + // This handles the case where data was written without encryption enabled. + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + data := map[string]any{ - "secret": nil, - "username": "admin", + "config": map[string]any{ + "host": "localhost", + "port": 5432, + }, } - // Redacting an already-nil field must succeed silently - schema.RedactFields(data, []string{"secret"}) - require.Nil(t, data["secret"]) - require.Equal(t, "admin", data["username"]) + // Attempt to decrypt a map that was never encrypted — should pass through + err = handler.DecryptSensitiveFields(context.Background(), data, []string{"config"}, testResourceID) + require.NoError(t, err) + + config := data["config"].(map[string]any) + require.Equal(t, "localhost", config["host"]) + require.Equal(t, 5432, config["port"]) +} + +func TestSensitiveDataHandler_EncryptDecrypt_BareIntegerField(t *testing.T) { + // The encryptValue default case marshals non-string/non-map/non-slice types to JSON. + // Verify that a bare integer field round-trips correctly. + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + + data := map[string]any{ + "name": "test-resource", + "secretPort": 5432, + } + + // Encrypt the bare integer + err = handler.EncryptSensitiveFields(data, []string{"secretPort"}, testResourceID) + require.NoError(t, err) + + // Should be encrypted to a map + _, isEncrypted := data["secretPort"].(map[string]any) + require.True(t, isEncrypted, "secretPort should be encrypted") + + // Name should be unchanged + require.Equal(t, "test-resource", data["name"]) + + // Decrypt — without schema, JSON unmarshals integers as float64 + err = handler.DecryptSensitiveFields(context.Background(), data, []string{"secretPort"}, testResourceID) + require.NoError(t, err) + + // Standard JSON behavior: integers become float64 without schema + require.Equal(t, float64(5432), data["secretPort"]) +} + +func TestSensitiveDataHandler_EncryptDecrypt_BareBooleanField(t *testing.T) { + key, err := GenerateKey() + require.NoError(t, err) + + handler, err := NewSensitiveDataHandlerFromKey(key) + require.NoError(t, err) + + data := map[string]any{ + "secretFlag": true, + } + + err = handler.EncryptSensitiveFields(data, []string{"secretFlag"}, testResourceID) + require.NoError(t, err) + + _, isEncrypted := data["secretFlag"].(map[string]any) + require.True(t, isEncrypted, "secretFlag should be encrypted") + + err = handler.DecryptSensitiveFields(context.Background(), data, []string{"secretFlag"}, testResourceID) + require.NoError(t, err) - // Verify idempotence: redacting again is still a no-op - schema.RedactFields(data, []string{"secret"}) - require.Nil(t, data["secret"]) + require.Equal(t, true, data["secretFlag"]) } // Helper function to deep copy a map for testing diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go index 173e534ac8..4ff2aa0b8e 100644 --- a/pkg/dynamicrp/frontend/getresource_test.go +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -17,34 +17,25 @@ limitations under the License. package frontend import ( - "context" "encoding/json" "net/http" "net/http/httptest" "testing" - armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" - azfake "github.com/Azure/azure-sdk-for-go/sdk/azcore/fake" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" v1 "github.com/radius-project/radius/pkg/armrpc/api/v1" "github.com/radius-project/radius/pkg/armrpc/frontend/controller" "github.com/radius-project/radius/pkg/armrpc/rest" "github.com/radius-project/radius/pkg/armrpc/rpctest" - aztoken "github.com/radius-project/radius/pkg/azure/tokencredentials" "github.com/radius-project/radius/pkg/components/database" "github.com/radius-project/radius/pkg/dynamicrp/datamodel" "github.com/radius-project/radius/pkg/dynamicrp/datamodel/converter" - "github.com/radius-project/radius/pkg/schema" "github.com/radius-project/radius/pkg/ucp/api/v20231001preview" - "github.com/radius-project/radius/pkg/ucp/api/v20231001preview/fake" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" ) const ( - testGetURL = "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/myResource?api-version=2023-10-01-preview" - getTestResourceID = "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/myResource" - getTestAPIVersion = "2023-10-01-preview" + testGetURL = "/planes/radius/local/resourceGroups/test-group/providers/Applications.Test/testResources/myResource?api-version=2023-10-01-preview" ) func newTestGetController(t *testing.T, databaseClient database.Client, ucpClient *v20231001preview.ClientFactory) controller.Controller { @@ -67,12 +58,12 @@ func newGetTestDynamicResource(provisioningState v1.ProvisioningState, propertie return &datamodel.DynamicResource{ BaseResource: v1.BaseResource{ TrackedResource: v1.TrackedResource{ - ID: getTestResourceID, + ID: testResourceID, Name: "myResource", Type: "Applications.Test/testResources", }, InternalMetadata: v1.InternalMetadata{ - UpdatedAPIVersion: getTestAPIVersion, + UpdatedAPIVersion: testAPIVersion, AsyncProvisioningState: provisioningState, }, }, @@ -89,14 +80,14 @@ func TestGetResourceWithRedaction_NonSucceededRedacts(t *testing.T) { }) storeObject := rpctest.FakeStoreObject(resource) - storeObject.Metadata = database.Metadata{ID: getTestResourceID, ETag: "etag-1"} + storeObject.Metadata = database.Metadata{ID: testResourceID, ETag: "etag-1"} databaseClient := database.NewMockClient(mctrl) databaseClient.EXPECT(). - Get(gomock.Any(), getTestResourceID). + Get(gomock.Any(), testResourceID). Return(storeObject, nil) - ucpClient, err := testGetUCPClientFactoryWithSensitiveFields() + ucpClient, err := testUCPClientFactoryWithSensitiveFields() require.NoError(t, err) c := newTestGetController(t, databaseClient, ucpClient) @@ -131,14 +122,14 @@ func TestGetResourceWithRedaction_SucceededSkipsRedaction(t *testing.T) { }) storeObject := rpctest.FakeStoreObject(resource) - storeObject.Metadata = database.Metadata{ID: getTestResourceID, ETag: "etag-1"} + storeObject.Metadata = database.Metadata{ID: testResourceID, ETag: "etag-1"} databaseClient := database.NewMockClient(mctrl) databaseClient.EXPECT(). - Get(gomock.Any(), getTestResourceID). + Get(gomock.Any(), testResourceID). Return(storeObject, nil) - ucpClient, err := testGetUCPClientFactoryWithSensitiveFields() + ucpClient, err := testUCPClientFactoryWithSensitiveFields() require.NoError(t, err) c := newTestGetController(t, databaseClient, ucpClient) @@ -162,23 +153,23 @@ func TestGetResourceWithRedaction_SucceededSkipsRedaction(t *testing.T) { require.Equal(t, "secret123", properties["password"]) } -func TestGetResourceWithRedaction_SchemaFetchErrorReturnsError(t *testing.T) { +func TestGetResourceWithRedaction_EmptyPropertiesNonSucceeded(t *testing.T) { + // When Properties are empty and state is non-Succeeded, redaction should be + // attempted but find nothing to redact, and the resource returned as 200 OK. mctrl := gomock.NewController(t) defer mctrl.Finish() - resource := newGetTestDynamicResource(v1.ProvisioningStateAccepted, map[string]any{ - "password": "secret123", - }) + resource := newGetTestDynamicResource(v1.ProvisioningStateAccepted, map[string]any{}) storeObject := rpctest.FakeStoreObject(resource) - storeObject.Metadata = database.Metadata{ID: getTestResourceID, ETag: "etag-1"} + storeObject.Metadata = database.Metadata{ID: testResourceID, ETag: "etag-1"} databaseClient := database.NewMockClient(mctrl) databaseClient.EXPECT(). - Get(gomock.Any(), getTestResourceID). + Get(gomock.Any(), testResourceID). Return(storeObject, nil) - ucpClient, err := testGetUCPClientFactoryWithError() + ucpClient, err := testUCPClientFactoryWithSensitiveFields() require.NoError(t, err) c := newTestGetController(t, databaseClient, ucpClient) @@ -193,389 +184,45 @@ func TestGetResourceWithRedaction_SchemaFetchErrorReturnsError(t *testing.T) { require.NotNil(t, resp) _ = resp.Apply(ctx, w, req) - // Expect fail-safe behavior: return error instead of exposing potentially sensitive data - require.Equal(t, http.StatusInternalServerError, w.Result().StatusCode) - - var body v1.ErrorResponse - require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) - require.Equal(t, v1.CodeInternal, body.Error.Code) - require.Contains(t, body.Error.Message, "Failed to fetch schema for security validation") -} - -func testGetUCPClientFactoryWithSensitiveFields() (*v20231001preview.ClientFactory, error) { - return createGetFakeUCPClientFactory(map[string]any{ - "type": "object", - "properties": map[string]any{ - "password": map[string]any{ - "type": "string", - "x-radius-sensitive": true, - }, - }, - }) -} - -func testGetUCPClientFactoryWithError() (*v20231001preview.ClientFactory, error) { - apiVersionsServer := fake.APIVersionsServer{ - Get: func(ctx context.Context, planeName, resourceProviderName, resourceTypeName, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { - errResp.SetResponseError(http.StatusNotFound, "NotFound") - return - }, - } - - return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Transport: fake.NewAPIVersionsServerTransport(&apiVersionsServer), - }, - }) + require.Equal(t, http.StatusOK, w.Result().StatusCode) } -func createGetFakeUCPClientFactory(schema map[string]any) (*v20231001preview.ClientFactory, error) { - apiVersionsServer := fake.APIVersionsServer{ - Get: func(ctx context.Context, planeName, resourceProviderName, resourceTypeName, apiVersionName string, options *v20231001preview.APIVersionsClientGetOptions) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { - response := v20231001preview.APIVersionsClientGetResponse{ - APIVersionResource: v20231001preview.APIVersionResource{ - Name: &apiVersionName, - Properties: &v20231001preview.APIVersionProperties{ - Schema: schema, - }, - }, - } - resp.SetResponse(http.StatusOK, response, nil) - return - }, - } - - return v20231001preview.NewClientFactory(&aztoken.AnonymousCredential{}, &armpolicy.ClientOptions{ - ClientOptions: policy.ClientOptions{ - Transport: fake.NewAPIVersionsServerTransport(&apiVersionsServer), - }, - }) -} +func TestGetResourceWithRedaction_SchemaFetchErrorReturnsError(t *testing.T) { + mctrl := gomock.NewController(t) + defer mctrl.Finish() -func TestRedactField_SimpleField(t *testing.T) { - // Test redacting a simple top-level field - properties := map[string]any{ - "name": "test-resource", + resource := newGetTestDynamicResource(v1.ProvisioningStateAccepted, map[string]any{ "password": "secret123", - "data": map[string]any{"key": "value"}, - } - - schema.RedactFields(properties, []string{"password"}) - - require.Equal(t, "test-resource", properties["name"]) - require.Nil(t, properties["password"]) - require.NotNil(t, properties["data"]) -} - -func TestRedactField_DataField(t *testing.T) { - // Test redacting the "data" field (common pattern for secrets) - properties := map[string]any{ - "environment": "/planes/radius/local/resourcegroups/default/providers/Radius.Core/environments/test", - "application": "/planes/radius/local/resourcegroups/default/providers/Applications.Core/applications/test", - "data": map[string]any{ - "password": map[string]any{ - "value": "secret123", - "encoding": "string", - }, - "apiKey": map[string]any{ - "value": "my-api-key", - }, - }, - } - - schema.RedactFields(properties, []string{"data"}) - - require.NotNil(t, properties["environment"]) - require.NotNil(t, properties["application"]) - require.Nil(t, properties["data"]) -} - -func TestRedactField_NonExistentField(t *testing.T) { - // Test that redacting a non-existent field doesn't cause errors - properties := map[string]any{ - "name": "test-resource", - "value": "test-value", - } - - // Should not panic or error - schema.RedactFields(properties, []string{"nonexistent"}) - - // Original fields should remain unchanged - require.Equal(t, "test-resource", properties["name"]) - require.Equal(t, "test-value", properties["value"]) -} - -func TestRedactField_NilProperties(t *testing.T) { - // Test that nil properties don't cause panic - var properties map[string]any - - // Should not panic - schema.RedactFields(properties, []string{"anyfield"}) -} - -func TestRedactField_EmptyProperties(t *testing.T) { - // Test redacting from empty properties - properties := map[string]any{} - - schema.RedactFields(properties, []string{"password"}) - - require.Empty(t, properties) -} - -func TestRedactField_MultipleFields(t *testing.T) { - // Test redacting multiple fields sequentially - properties := map[string]any{ - "name": "test", - "password": "secret", - "apiKey": "key123", - "data": "sensitive-data", - } - - schema.RedactFields(properties, []string{"password"}) - schema.RedactFields(properties, []string{"apiKey"}) - schema.RedactFields(properties, []string{"data"}) - - require.Equal(t, "test", properties["name"]) - require.Nil(t, properties["password"]) - require.Nil(t, properties["apiKey"]) - require.Nil(t, properties["data"]) -} - -func TestRedactField_NestedDotPath(t *testing.T) { - // Test redacting a nested field via dot-separated path - properties := map[string]any{ - "config": map[string]any{ - "password": "secret", - "host": "localhost", - }, - } - - schema.RedactFields(properties, []string{"config.password"}) - - config, ok := properties["config"].(map[string]any) - require.True(t, ok) - require.Nil(t, config["password"]) - require.Equal(t, "localhost", config["host"]) -} - -func TestRedactField_DeeplyNestedDotPath(t *testing.T) { - // Test redacting a deeply nested field - properties := map[string]any{ - "level1": map[string]any{ - "level2": map[string]any{ - "secret": "top-secret", - "other": "keep-this", - }, - }, - } - - schema.RedactFields(properties, []string{"level1.level2.secret"}) - - level1 := properties["level1"].(map[string]any) - level2 := level1["level2"].(map[string]any) - require.Nil(t, level2["secret"]) - require.Equal(t, "keep-this", level2["other"]) -} - -func TestRedactField_ArrayWildcard(t *testing.T) { - // Test redacting fields within array elements using [*] - properties := map[string]any{ - "secrets": []any{ - map[string]any{"name": "secret1", "value": "s1"}, - map[string]any{"name": "secret2", "value": "s2"}, - }, - } - - schema.RedactFields(properties, []string{"secrets[*].value"}) - - secrets := properties["secrets"].([]any) - s0 := secrets[0].(map[string]any) - s1 := secrets[1].(map[string]any) - require.Nil(t, s0["value"]) - require.Nil(t, s1["value"]) - require.Equal(t, "secret1", s0["name"]) - require.Equal(t, "secret2", s1["name"]) -} - -func TestRedactField_MapWildcard(t *testing.T) { - // Test redacting all values of a map using [*] - properties := map[string]any{ - "config": map[string]any{ - "key1": "value1", - "key2": "value2", - }, - } - - schema.RedactFields(properties, []string{"config[*]"}) - - config := properties["config"].(map[string]any) - require.Nil(t, config["key1"]) - require.Nil(t, config["key2"]) -} - -func TestRedactField_MapWildcardWithNestedField(t *testing.T) { - // Test redacting a nested field within map values using [*] - properties := map[string]any{ - "backends": map[string]any{ - "kv": map[string]any{"url": "http://vault", "token": "secret-token"}, - "azure": map[string]any{"url": "http://azure", "token": "azure-token"}, - }, - } - - schema.RedactFields(properties, []string{"backends[*].token"}) - - backends := properties["backends"].(map[string]any) - kv := backends["kv"].(map[string]any) - azure := backends["azure"].(map[string]any) - require.Nil(t, kv["token"]) - require.Nil(t, azure["token"]) - require.Equal(t, "http://vault", kv["url"]) - require.Equal(t, "http://azure", azure["url"]) -} - -func TestRedactField_NestedPathFieldNotFound(t *testing.T) { - // Test that a non-existent nested path doesn't cause errors - properties := map[string]any{ - "config": map[string]any{ - "host": "localhost", - }, - } - - // Should not panic - field doesn't exist at this nested path - schema.RedactFields(properties, []string{"config.nonexistent"}) - - config := properties["config"].(map[string]any) - require.Equal(t, "localhost", config["host"]) -} - -func TestRedactField_EmptyPath(t *testing.T) { - // Test that empty path doesn't cause errors - properties := map[string]any{ - "data": "value", - } - - schema.RedactFields(properties, []string{""}) - - require.Equal(t, "value", properties["data"]) -} - -func TestRedactField_ArrayWildcardAllElements(t *testing.T) { - // Test redacting all elements in an array using [*] as the final segment - properties := map[string]any{ - "tokens": []any{"token1", "token2", "token3"}, - } - - schema.RedactFields(properties, []string{"tokens[*]"}) + }) - tokens := properties["tokens"].([]any) - for _, token := range tokens { - require.Nil(t, token) - } -} + storeObject := rpctest.FakeStoreObject(resource) + storeObject.Metadata = database.Metadata{ID: testResourceID, ETag: "etag-1"} -func TestRedactField_FieldWithNilValue(t *testing.T) { - // Test redacting a field that already has nil value - properties := map[string]any{ - "name": "test", - "password": nil, - } + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), testResourceID). + Return(storeObject, nil) - schema.RedactFields(properties, []string{"password"}) + ucpClient, err := testUCPClientFactoryWithError() + require.NoError(t, err) - require.Equal(t, "test", properties["name"]) - require.Nil(t, properties["password"]) -} + c := newTestGetController(t, databaseClient, ucpClient) -func TestRedactField_FieldWithDifferentTypes(t *testing.T) { - // Test redacting fields of various types - testCases := []struct { - name string - value any - fieldName string - properties map[string]any - }{ - { - name: "string field", - fieldName: "secret", - properties: map[string]any{"secret": "password123"}, - }, - { - name: "map field", - fieldName: "data", - properties: map[string]any{"data": map[string]any{"key": "value"}}, - }, - { - name: "slice field", - fieldName: "tokens", - properties: map[string]any{"tokens": []string{"token1", "token2"}}, - }, - { - name: "int field", - fieldName: "pin", - properties: map[string]any{"pin": 1234}, - }, - { - name: "bool field", - fieldName: "enabled", - properties: map[string]any{"enabled": true}, - }, - } + req, err := http.NewRequest(http.MethodGet, testGetURL, nil) + require.NoError(t, err) + ctx := rpctest.NewARMRequestContext(req) + w := httptest.NewRecorder() - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - schema.RedactFields(tc.properties, []string{tc.fieldName}) - require.Nil(t, tc.properties[tc.fieldName]) - }) - } -} + resp, err := c.Run(ctx, w, req) + require.NoError(t, err) + require.NotNil(t, resp) -func TestParseRedactPath(t *testing.T) { - tests := []struct { - name string - path string - expected []schema.FieldPathSegment - }{ - { - name: "simple field", - path: "data", - expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "data"}}, - }, - { - name: "nested dot path", - path: "credentials.password", - expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "credentials"}, {Type: schema.SegmentTypeField, Value: "password"}}, - }, - { - name: "array wildcard", - path: "secrets[*].value", - expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "secrets"}, {Type: schema.SegmentTypeWildcard}, {Type: schema.SegmentTypeField, Value: "value"}}, - }, - { - name: "map wildcard", - path: "config[*]", - expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "config"}, {Type: schema.SegmentTypeWildcard}}, - }, - { - name: "deeply nested", - path: "a.b.c.d", - expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "a"}, {Type: schema.SegmentTypeField, Value: "b"}, {Type: schema.SegmentTypeField, Value: "c"}, {Type: schema.SegmentTypeField, Value: "d"}}, - }, - { - name: "wildcard with nested field", - path: "backends[*].token", - expected: []schema.FieldPathSegment{{Type: schema.SegmentTypeField, Value: "backends"}, {Type: schema.SegmentTypeWildcard}, {Type: schema.SegmentTypeField, Value: "token"}}, - }, - { - name: "empty path", - path: "", - expected: nil, - }, - } + _ = resp.Apply(ctx, w, req) + // Expect fail-safe behavior: return error instead of exposing potentially sensitive data + require.Equal(t, http.StatusInternalServerError, w.Result().StatusCode) - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - result := schema.ParseFieldPath(tc.path) - require.Equal(t, tc.expected, result) - }) - } + var body v1.ErrorResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&body)) + require.Equal(t, v1.CodeInternal, body.Error.Code) + require.Contains(t, body.Error.Message, "Failed to fetch schema for security validation") } diff --git a/pkg/schema/annotations_test.go b/pkg/schema/annotations_test.go index 7859757381..1d7ee4d099 100644 --- a/pkg/schema/annotations_test.go +++ b/pkg/schema/annotations_test.go @@ -584,3 +584,432 @@ func TestGetSensitiveFieldPaths_InvalidResourceID(t *testing.T) { require.Error(t, err) } + +// ============================================================================= +// ParseFieldPath tests +// ============================================================================= + +func TestParseFieldPath(t *testing.T) { + tests := []struct { + name string + path string + expected []FieldPathSegment + }{ + { + name: "simple field", + path: "data", + expected: []FieldPathSegment{{Type: SegmentTypeField, Value: "data"}}, + }, + { + name: "nested dot path", + path: "credentials.password", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "credentials"}, + {Type: SegmentTypeField, Value: "password"}, + }, + }, + { + name: "deeply nested", + path: "config.database.connection.password", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "config"}, + {Type: SegmentTypeField, Value: "database"}, + {Type: SegmentTypeField, Value: "connection"}, + {Type: SegmentTypeField, Value: "password"}, + }, + }, + { + name: "array wildcard", + path: "secrets[*].value", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "secrets"}, + {Type: SegmentTypeWildcard}, + {Type: SegmentTypeField, Value: "value"}, + }, + }, + { + name: "map wildcard", + path: "config[*]", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "config"}, + {Type: SegmentTypeWildcard}, + }, + }, + { + name: "specific index", + path: "items[0].name", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "items"}, + {Type: SegmentTypeIndex, Value: "0"}, + {Type: SegmentTypeField, Value: "name"}, + }, + }, + { + name: "multiple wildcards", + path: "data[*].secrets[*].value", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "data"}, + {Type: SegmentTypeWildcard}, + {Type: SegmentTypeField, Value: "secrets"}, + {Type: SegmentTypeWildcard}, + {Type: SegmentTypeField, Value: "value"}, + }, + }, + { + name: "wildcard with nested field", + path: "backends[*].token", + expected: []FieldPathSegment{ + {Type: SegmentTypeField, Value: "backends"}, + {Type: SegmentTypeWildcard}, + {Type: SegmentTypeField, Value: "token"}, + }, + }, + { + name: "empty path", + path: "", + expected: nil, + }, + { + name: "unterminated bracket", + path: "secrets[*", + expected: nil, + }, + { + name: "unterminated bracket with index", + path: "items[0", + expected: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := ParseFieldPath(tc.path) + require.Equal(t, tc.expected, result) + }) + } +} + +// ============================================================================= +// RedactFields tests +// ============================================================================= + +func TestRedactFields_SimpleField(t *testing.T) { + properties := map[string]any{ + "name": "test-resource", + "password": "secret123", + "data": map[string]any{"key": "value"}, + } + + RedactFields(properties, []string{"password"}) + + require.Equal(t, "test-resource", properties["name"]) + require.Nil(t, properties["password"]) + require.NotNil(t, properties["data"]) +} + +func TestRedactFields_DataField(t *testing.T) { + properties := map[string]any{ + "environment": "/planes/radius/local/resourcegroups/default/providers/Radius.Core/environments/test", + "application": "/planes/radius/local/resourcegroups/default/providers/Applications.Core/applications/test", + "data": map[string]any{ + "password": map[string]any{ + "value": "secret123", + "encoding": "string", + }, + "apiKey": map[string]any{ + "value": "my-api-key", + }, + }, + } + + RedactFields(properties, []string{"data"}) + + require.NotNil(t, properties["environment"]) + require.NotNil(t, properties["application"]) + require.Nil(t, properties["data"]) +} + +func TestRedactFields_NonExistentField(t *testing.T) { + properties := map[string]any{ + "name": "test-resource", + "value": "test-value", + } + + RedactFields(properties, []string{"nonexistent"}) + + require.Equal(t, "test-resource", properties["name"]) + require.Equal(t, "test-value", properties["value"]) +} + +func TestRedactFields_NilProperties(t *testing.T) { + var properties map[string]any + + // Should not panic + RedactFields(properties, []string{"anyfield"}) +} + +func TestRedactFields_EmptyProperties(t *testing.T) { + properties := map[string]any{} + + RedactFields(properties, []string{"password"}) + + require.Empty(t, properties) +} + +func TestRedactFields_MultipleFields(t *testing.T) { + properties := map[string]any{ + "name": "test", + "password": "secret", + "apiKey": "key123", + "data": "sensitive-data", + } + + RedactFields(properties, []string{"password", "apiKey", "data"}) + + require.Equal(t, "test", properties["name"]) + require.Nil(t, properties["password"]) + require.Nil(t, properties["apiKey"]) + require.Nil(t, properties["data"]) +} + +func TestRedactFields_NestedDotPath(t *testing.T) { + properties := map[string]any{ + "config": map[string]any{ + "password": "secret", + "host": "localhost", + }, + } + + RedactFields(properties, []string{"config.password"}) + + config, ok := properties["config"].(map[string]any) + require.True(t, ok) + require.Nil(t, config["password"]) + require.Equal(t, "localhost", config["host"]) +} + +func TestRedactFields_DeeplyNestedDotPath(t *testing.T) { + properties := map[string]any{ + "level1": map[string]any{ + "level2": map[string]any{ + "secret": "top-secret", + "other": "keep-this", + }, + }, + } + + RedactFields(properties, []string{"level1.level2.secret"}) + + level1 := properties["level1"].(map[string]any) + level2 := level1["level2"].(map[string]any) + require.Nil(t, level2["secret"]) + require.Equal(t, "keep-this", level2["other"]) +} + +func TestRedactFields_ArrayWildcard(t *testing.T) { + properties := map[string]any{ + "secrets": []any{ + map[string]any{"name": "secret1", "value": "s1"}, + map[string]any{"name": "secret2", "value": "s2"}, + }, + } + + RedactFields(properties, []string{"secrets[*].value"}) + + secrets := properties["secrets"].([]any) + s0 := secrets[0].(map[string]any) + s1 := secrets[1].(map[string]any) + require.Nil(t, s0["value"]) + require.Nil(t, s1["value"]) + require.Equal(t, "secret1", s0["name"]) + require.Equal(t, "secret2", s1["name"]) +} + +func TestRedactFields_MapWildcard(t *testing.T) { + properties := map[string]any{ + "config": map[string]any{ + "key1": "value1", + "key2": "value2", + }, + } + + RedactFields(properties, []string{"config[*]"}) + + config := properties["config"].(map[string]any) + require.Nil(t, config["key1"]) + require.Nil(t, config["key2"]) +} + +func TestRedactFields_MapWildcardWithNestedField(t *testing.T) { + properties := map[string]any{ + "backends": map[string]any{ + "kv": map[string]any{"url": "http://vault", "token": "secret-token"}, + "azure": map[string]any{"url": "http://azure", "token": "azure-token"}, + }, + } + + RedactFields(properties, []string{"backends[*].token"}) + + backends := properties["backends"].(map[string]any) + kv := backends["kv"].(map[string]any) + azure := backends["azure"].(map[string]any) + require.Nil(t, kv["token"]) + require.Nil(t, azure["token"]) + require.Equal(t, "http://vault", kv["url"]) + require.Equal(t, "http://azure", azure["url"]) +} + +func TestRedactFields_NestedPathFieldNotFound(t *testing.T) { + properties := map[string]any{ + "config": map[string]any{ + "host": "localhost", + }, + } + + RedactFields(properties, []string{"config.nonexistent"}) + + config := properties["config"].(map[string]any) + require.Equal(t, "localhost", config["host"]) +} + +func TestRedactFields_EmptyPath(t *testing.T) { + properties := map[string]any{ + "data": "value", + } + + RedactFields(properties, []string{""}) + + require.Equal(t, "value", properties["data"]) +} + +func TestRedactFields_ArrayWildcardAllElements(t *testing.T) { + properties := map[string]any{ + "tokens": []any{"token1", "token2", "token3"}, + } + + RedactFields(properties, []string{"tokens[*]"}) + + tokens := properties["tokens"].([]any) + for _, token := range tokens { + require.Nil(t, token) + } +} + +func TestRedactFields_FieldWithNilValue(t *testing.T) { + properties := map[string]any{ + "name": "test", + "password": nil, + } + + RedactFields(properties, []string{"password"}) + + require.Equal(t, "test", properties["name"]) + require.Nil(t, properties["password"]) +} + +func TestRedactFields_AlreadyNilIdempotent(t *testing.T) { + data := map[string]any{ + "secret": nil, + "username": "admin", + } + + RedactFields(data, []string{"secret"}) + require.Nil(t, data["secret"]) + require.Equal(t, "admin", data["username"]) + + // Redacting again is still a no-op + RedactFields(data, []string{"secret"}) + require.Nil(t, data["secret"]) +} + +func TestRedactFields_SpecificIndex(t *testing.T) { + properties := map[string]any{ + "items": []any{ + map[string]any{"name": "public", "password": "keep-this"}, + map[string]any{"name": "secret", "password": "redact-this"}, + map[string]any{"name": "public2", "password": "keep-this-too"}, + }, + } + + RedactFields(properties, []string{"items[1].password"}) + + items := properties["items"].([]any) + // First and third items should be unchanged + require.Equal(t, "keep-this", items[0].(map[string]any)["password"]) + require.Equal(t, "keep-this-too", items[2].(map[string]any)["password"]) + // Second item's password should be redacted + require.Nil(t, items[1].(map[string]any)["password"]) + // Names should be untouched + require.Equal(t, "public", items[0].(map[string]any)["name"]) + require.Equal(t, "secret", items[1].(map[string]any)["name"]) + require.Equal(t, "public2", items[2].(map[string]any)["name"]) +} + +func TestRedactFields_SpecificIndex_OutOfBounds(t *testing.T) { + properties := map[string]any{ + "items": []any{ + map[string]any{"value": "keep"}, + }, + } + + // Index 5 is out of bounds — should silently skip, no panic + RedactFields(properties, []string{"items[5].value"}) + + items := properties["items"].([]any) + require.Equal(t, "keep", items[0].(map[string]any)["value"]) +} + +func TestRedactFields_SpecificIndex_BareElement(t *testing.T) { + properties := map[string]any{ + "tokens": []any{"token0", "token1", "token2"}, + } + + // Redact the second element directly + RedactFields(properties, []string{"tokens[1]"}) + + tokens := properties["tokens"].([]any) + require.Equal(t, "token0", tokens[0]) + require.Nil(t, tokens[1]) + require.Equal(t, "token2", tokens[2]) +} + +func TestRedactFields_FieldWithDifferentTypes(t *testing.T) { + testCases := []struct { + name string + fieldName string + properties map[string]any + }{ + { + name: "string field", + fieldName: "secret", + properties: map[string]any{"secret": "password123"}, + }, + { + name: "map field", + fieldName: "data", + properties: map[string]any{"data": map[string]any{"key": "value"}}, + }, + { + name: "slice field", + fieldName: "tokens", + properties: map[string]any{"tokens": []string{"token1", "token2"}}, + }, + { + name: "int field", + fieldName: "pin", + properties: map[string]any{"pin": 1234}, + }, + { + name: "bool field", + fieldName: "enabled", + properties: map[string]any{"enabled": true}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + RedactFields(tc.properties, []string{tc.fieldName}) + require.Nil(t, tc.properties[tc.fieldName]) + }) + } +} From 843c26376c7739cba848d66eaf9ee19b9d9ac974 Mon Sep 17 00:00:00 2001 From: sk593 Date: Thu, 19 Feb 2026 07:59:59 -0800 Subject: [PATCH 10/10] update terraform state Signed-off-by: sk593 --- pkg/recipes/terraform/execute.go | 6 ++ pkg/schema/validator.go | 59 +++++++----- pkg/schema/validator_test.go | 148 ++++++++++++++++++++++++++----- 3 files changed, 172 insertions(+), 41 deletions(-) diff --git a/pkg/recipes/terraform/execute.go b/pkg/recipes/terraform/execute.go index dea1ca988d..aa0c7fc969 100644 --- a/pkg/recipes/terraform/execute.go +++ b/pkg/recipes/terraform/execute.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "io" "os" "strings" "time" @@ -396,6 +397,11 @@ func initAndApply(ctx context.Context, tf *tfexec.Terraform, stateLockTimeout st } } + // Suppress stdout during tf.Show to prevent Terraform state (which may + // contain sensitive values) from being written to the Radius logs. + tf.SetStdout(io.Discard) + defer tf.SetStdout(&tfLogWrapper{logger: logger}) + return tf.Show(ctx) } diff --git a/pkg/schema/validator.go b/pkg/schema/validator.go index c3b3c47a42..18c95af209 100644 --- a/pkg/schema/validator.go +++ b/pkg/schema/validator.go @@ -110,9 +110,15 @@ func normalizePlatformOptionsAnyWithPath(schema *openapi3.Schema, path string) { } } -// normalizeSensitiveFieldTypes normalizes the type constraint on sensitive string fields -// so that the OpenAPI validator accepts both the original plaintext string and the encrypted object form. -// Only string types need this adjustment — object types remain objects after encryption and pass validation. +// normalizeSensitiveFieldTypes normalizes the type constraints on sensitive fields +// so that the OpenAPI validator accepts both the original plaintext value and the encrypted form. +// +// For string fields: the encrypted form is an object with encrypted/nonce/version keys, +// so the schema is widened to accept either the original string or an object. +// +// For object fields: the encrypted form is still an object but with entirely different keys +// (encrypted/nonce/version instead of the original properties/additionalProperties), so the +// schema is widened to accept either the original object or an unconstrained object. func normalizeSensitiveFieldTypes(schema *openapi3.Schema) { if schema == nil { return @@ -123,7 +129,7 @@ func normalizeSensitiveFieldTypes(schema *openapi3.Schema) { continue } - if normalizeSensitiveStringType(propRef) { + if normalizeSensitiveType(propRef) { continue } @@ -133,23 +139,28 @@ func normalizeSensitiveFieldTypes(schema *openapi3.Schema) { // Handle array items — check the items schema's own annotation first. if schema.Items != nil && schema.Items.Value != nil { - if !normalizeSensitiveStringType(schema.Items) { + if !normalizeSensitiveType(schema.Items) { normalizeSensitiveFieldTypes(schema.Items.Value) } } // Handle additionalProperties (maps) — check its own annotation first. if schema.AdditionalProperties.Schema != nil && schema.AdditionalProperties.Schema.Value != nil { - if !normalizeSensitiveStringType(schema.AdditionalProperties.Schema) { + if !normalizeSensitiveType(schema.AdditionalProperties.Schema) { normalizeSensitiveFieldTypes(schema.AdditionalProperties.Schema.Value) } } } -// normalizeSensitiveStringType checks whether a SchemaRef points to a string with a sensitive -// annotation. If so, it expands the type constraint so it accepts either the original string -// schema or object (the encrypted form). Returns true if the schema was normalized, false otherwise. -func normalizeSensitiveStringType(ref *openapi3.SchemaRef) bool { +// normalizeSensitiveType checks whether a SchemaRef has a sensitive annotation and expands +// the type constraint so the OpenAPI validator accepts both the original value and the encrypted form. +// +// For string fields: widens to accept either the original string or an object (the encrypted envelope). +// For object fields: widens to accept either the original object schema or an unconstrained object +// (since the encrypted envelope replaces all properties/additionalProperties with encrypted/nonce/version). +// +// Returns true if the schema was normalized, false otherwise. +func normalizeSensitiveType(ref *openapi3.SchemaRef) bool { if ref == nil || ref.Value == nil { return false } @@ -164,31 +175,37 @@ func normalizeSensitiveStringType(ref *openapi3.SchemaRef) bool { return false } - if prop.Type == nil || !prop.Type.Is("string") { + isString := prop.Type != nil && prop.Type.Is("string") + isObject := prop.Type != nil && prop.Type.Is("object") + + if !isString && !isObject { return false } // Copy the original so we don't create a self-referencing cycle. - originalString := *prop + original := *prop // Remove the sensitive annotation from the copy to avoid re-processing. // Other vendor extensions are preserved. - if len(originalString.Extensions) > 0 { - extCopy := make(map[string]any, len(originalString.Extensions)) - for k, v := range originalString.Extensions { + if len(original.Extensions) > 0 { + extCopy := make(map[string]any, len(original.Extensions)) + for k, v := range original.Extensions { extCopy[k] = v } delete(extCopy, annotationRadiusSensitive) - originalString.Extensions = extCopy + original.Extensions = extCopy + } + + // The encrypted envelope is always an object with encrypted/nonce/version keys. + encryptedForm := &openapi3.Schema{ + Type: &openapi3.Types{"object"}, + MinProps: 1, // reject empty objects — encrypted data always has fields } ref.Value = &openapi3.Schema{ AnyOf: openapi3.SchemaRefs{ - {Value: &originalString}, - {Value: &openapi3.Schema{ - Type: &openapi3.Types{"object"}, - MinProps: 1, // reject empty objects — encrypted data always has fields - }}, + {Value: &original}, + {Value: encryptedForm}, }, } diff --git a/pkg/schema/validator_test.go b/pkg/schema/validator_test.go index c115e912da..29664b1f65 100644 --- a/pkg/schema/validator_test.go +++ b/pkg/schema/validator_test.go @@ -2687,13 +2687,25 @@ func TestNormalizeSensitiveFieldTypes(t *testing.T) { require.True(t, schema.Properties["name"].Value.Type.Is("string")) }) - t.Run("sensitive object field retains type", func(t *testing.T) { + t.Run("sensitive object field is normalized to AnyOf and preserves original schema", func(t *testing.T) { schema := &openapi3.Schema{ Type: &openapi3.Types{"object"}, Properties: openapi3.Schemas{ "credentials": { Value: &openapi3.Schema{ Type: &openapi3.Types{"object"}, + Properties: openapi3.Schemas{ + "username": { + Value: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + }, + }, + "password": { + Value: &openapi3.Schema{ + Type: &openapi3.Types{"string"}, + }, + }, + }, Extensions: map[string]any{ annotationRadiusSensitive: true, }, @@ -2704,9 +2716,23 @@ func TestNormalizeSensitiveFieldTypes(t *testing.T) { normalizeSensitiveFieldTypes(schema) - // Sensitive object field should retain its type (objects stay objects after encryption) - require.NotNil(t, schema.Properties["credentials"].Value.Type) - require.True(t, schema.Properties["credentials"].Value.Type.Is("object")) + cred := schema.Properties["credentials"].Value + require.NotNil(t, cred.AnyOf) + require.Len(t, cred.AnyOf, 2) + + // First branch: original object schema with properties preserved + original := cred.AnyOf[0].Value + require.True(t, original.Type.Is("object")) + require.Contains(t, original.Properties, "username") + require.Contains(t, original.Properties, "password") + + // Second branch: unconstrained object (encrypted envelope) + require.True(t, cred.AnyOf[1].Value.Type.Is("object")) + require.Equal(t, uint64(1), cred.AnyOf[1].Value.MinProps) + + // Sensitive annotation should be removed from original branch + _, hasSensitive := original.Extensions[annotationRadiusSensitive] + require.False(t, hasSensitive, "x-radius-sensitive should be removed from original branch") }) t.Run("nested sensitive string field is normalized to AnyOf", func(t *testing.T) { @@ -2951,7 +2977,7 @@ func TestNormalizeSensitiveFieldTypes(t *testing.T) { Type: &openapi3.Types{"string"}, Extensions: map[string]any{ annotationRadiusSensitive: true, - "x-custom-metadata": "some-value", + "x-custom-metadata": "some-value", }, }, }, @@ -3002,7 +3028,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "string", }, "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3030,7 +3056,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3058,7 +3084,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "string", }, "secret": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3089,7 +3115,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "credentials": map[string]any{ - "type": "object", + "type": "object", "x-radius-sensitive": true, }, }, @@ -3116,10 +3142,10 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "apiKey": map[string]any{ - "type": "string", - "minLength": 8, - "maxLength": 128, - "pattern": "^[A-Za-z0-9]+$", + "type": "string", + "minLength": 8, + "maxLength": 128, + "pattern": "^[A-Za-z0-9]+$", "x-radius-sensitive": true, }, }, @@ -3150,7 +3176,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "string", }, "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3173,7 +3199,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3195,7 +3221,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3217,7 +3243,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3234,6 +3260,88 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { require.Contains(t, err.Error(), "resource data validation failed") }) + t.Run("encrypted sensitive object with properties passes validation", func(t *testing.T) { + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "credentials": map[string]any{ + "type": "object", + "properties": map[string]any{ + "username": map[string]any{"type": "string"}, + "password": map[string]any{"type": "string"}, + }, + "x-radius-sensitive": true, + }, + }, + } + + // After encryption, the object properties are replaced by the encrypted envelope + resourceData := map[string]any{ + "properties": map[string]any{ + "credentials": map[string]any{ + "encrypted": "base64-ciphertext", + "nonce": "base64-nonce", + "ad": "base64-hash", + "version": 1, + }, + }, + } + + err := ValidateResourceAgainstSchema(ctx, resourceData, schema) + require.NoError(t, err) + }) + + t.Run("unencrypted sensitive object with properties still passes validation", func(t *testing.T) { + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "credentials": map[string]any{ + "type": "object", + "properties": map[string]any{ + "username": map[string]any{"type": "string"}, + "password": map[string]any{"type": "string"}, + }, + "x-radius-sensitive": true, + }, + }, + } + + // Before encryption, the value matches the original schema + resourceData := map[string]any{ + "properties": map[string]any{ + "credentials": map[string]any{ + "username": "admin", + "password": "secret", + }, + }, + } + + err := ValidateResourceAgainstSchema(ctx, resourceData, schema) + require.NoError(t, err) + }) + + t.Run("sensitive object field rejects non-object value", func(t *testing.T) { + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "credentials": map[string]any{ + "type": "object", + "x-radius-sensitive": true, + }, + }, + } + + resourceData := map[string]any{ + "properties": map[string]any{ + "credentials": "not-an-object", // String value for an object field + }, + } + + err := ValidateResourceAgainstSchema(ctx, resourceData, schema) + require.Error(t, err) + require.Contains(t, err.Error(), "resource data validation failed") + }) + t.Run("sensitive string directly on items schema accepts encrypted values", func(t *testing.T) { schema := map[string]any{ "type": "object", @@ -3241,7 +3349,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "tokens": map[string]any{ "type": "array", "items": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3268,7 +3376,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "secretMap": map[string]any{ "type": "object", "additionalProperties": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, }, @@ -3293,7 +3401,7 @@ func TestValidateResourceAgainstSchema_EncryptedSensitiveFields(t *testing.T) { "type": "object", "properties": map[string]any{ "password": map[string]any{ - "type": "string", + "type": "string", "x-radius-sensitive": true, }, },