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..c93f6ec879 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 ( @@ -212,7 +213,7 @@ func (h *SensitiveDataHandler) processFieldAtPath(data map[string]any, path stri return ErrInvalidFieldPath } - segments := parseFieldPath(path) + segments := schema.ParseFieldPath(path) if len(segments) == 0 { return ErrInvalidFieldPath } @@ -221,7 +222,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 } @@ -229,20 +230,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 @@ -268,7 +268,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 { @@ -319,7 +319,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 @@ -478,29 +478,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 @@ -510,7 +509,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 @@ -597,74 +596,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 990656cb93..10d275669b 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" ) @@ -28,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 []pathSegment - }{ - { - name: "simple-field", - path: "password", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "password"}, - }, - }, - { - name: "nested-field", - path: "credentials.password", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "credentials"}, - {segmentType: 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"}, - }, - }, - { - name: "array-wildcard", - path: "secrets[*].value", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "secrets"}, - {segmentType: segmentTypeWildcard}, - {segmentType: segmentTypeField, value: "value"}, - }, - }, - { - name: "map-wildcard", - path: "config[*]", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "config"}, - {segmentType: segmentTypeWildcard}, - }, - }, - { - name: "specific-index", - path: "items[0].name", - expected: []pathSegment{ - {segmentType: segmentTypeField, value: "items"}, - {segmentType: segmentTypeIndex, value: "0"}, - {segmentType: 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"}, - }, - }, - { - 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 := parseFieldPath(tt.path) - require.Equal(t, tt.expected, result) - }) - } -} - func TestSensitiveDataHandler_EncryptDecrypt_SimpleField(t *testing.T) { key, err := GenerateKey() require.NoError(t, err) @@ -1003,6 +916,188 @@ 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) + schema.RedactFields(redactedProperties, sensitivePaths) + + // 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_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{ + "config": map[string]any{ + "host": "localhost", + "port": 5432, + }, + } + + // 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) + + require.Equal(t, true, data["secretFlag"]) +} + // 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/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 new file mode 100644 index 0000000000..e19bb42a9e --- /dev/null +++ b/pkg/dynamicrp/frontend/getresource.go @@ -0,0 +1,109 @@ +/* +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. +// +// 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) + + resource, etag, err := c.GetResource(ctx, serviceCtx.ResourceID) + if err != nil { + return nil, err + } + if resource == nil { + return rest.NewNotFoundResponse(serviceCtx.ResourceID), 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() + + // 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, + c.ucpClient, + resourceID, + resourceType, + apiVersion, + ) + if err != nil { + logger.Error(err, "Failed to fetch sensitive field paths for GET redaction", + "resourceType", resourceType, "apiVersion", apiVersion) + // 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) + } + } + + return c.ConstructSyncResponse(ctx, req.Method, etag, resource) +} diff --git a/pkg/dynamicrp/frontend/getresource_test.go b/pkg/dynamicrp/frontend/getresource_test.go new file mode 100644 index 0000000000..4ff2aa0b8e --- /dev/null +++ b/pkg/dynamicrp/frontend/getresource_test.go @@ -0,0 +1,228 @@ +/* +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 ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + 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" + "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 ( + 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 { + 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: testResourceID, + Name: "myResource", + Type: "Applications.Test/testResources", + }, + InternalMetadata: v1.InternalMetadata{ + UpdatedAPIVersion: testAPIVersion, + 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: testResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), testResourceID). + Return(storeObject, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + 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: testResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), testResourceID). + Return(storeObject, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + 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_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{}) + + storeObject := rpctest.FakeStoreObject(resource) + storeObject.Metadata = database.Metadata{ID: testResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), testResourceID). + Return(storeObject, nil) + + ucpClient, err := testUCPClientFactoryWithSensitiveFields() + 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) +} + +func TestGetResourceWithRedaction_SchemaFetchErrorReturnsError(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: testResourceID, ETag: "etag-1"} + + databaseClient := database.NewMockClient(mctrl) + databaseClient.EXPECT(). + Get(gomock.Any(), testResourceID). + Return(storeObject, nil) + + ucpClient, err := testUCPClientFactoryWithError() + 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) + // 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") +} diff --git a/pkg/dynamicrp/frontend/listresources.go b/pkg/dynamicrp/frontend/listresources.go new file mode 100644 index 0000000000..921189a3cc --- /dev/null +++ b/pkg/dynamicrp/frontend/listresources.go @@ -0,0 +1,143 @@ +/* +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 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 { + resource := &datamodel.DynamicResource{} + if err := item.As(resource); err != nil { + return nil, err + } + + // 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 { + // 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, + resource.ID, + serviceCtx.ResourceID.Type(), + apiVersion, + ) + if err != nil { + logger.Error(err, "Failed to fetch sensitive field paths for LIST redaction", + "resourceType", serviceCtx.ResourceID.Type(), "apiVersion", apiVersion) + // 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 + } + sensitiveFieldPathsCache[apiVersion] = sensitiveFieldPaths + } + + if len(sensitiveFieldPaths) > 0 { + schema.RedactFields(resource.Properties, sensitiveFieldPaths) + } + } + + versioned, err := c.ResponseConverter()(resource, serviceCtx.APIVersion) + if err != nil { + return nil, err + } + items = append(items, versioned) + } + + // 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", + "totalSensitiveFields", totalSensitiveFields, + "apiVersions", len(sensitiveFieldPathsCache), + "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/listresources_test.go b/pkg/dynamicrp/frontend/listresources_test.go new file mode 100644 index 0000000000..8442ad96f2 --- /dev/null +++ b/pkg/dynamicrp/frontend/listresources_test.go @@ -0,0 +1,623 @@ +/* +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 ( + "encoding/json" + "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 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) + + _ = 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) { + 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/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/portableresources/backend/controller/createorupdateresource.go b/pkg/portableresources/backend/controller/createorupdateresource.go index 06d9ba5027..e0ae4f06bd 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,80 @@ 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 { + 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 { + 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 + } + + // 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 + } + schemautil.RedactFields(redactedProperties, sensitiveFieldPaths) + + 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 +161,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 +171,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, redactionCompleted) } } @@ -102,6 +183,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 +207,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 } @@ -132,7 +219,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)) @@ -147,12 +236,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 } @@ -164,13 +262,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 +322,41 @@ func (c *CreateOrUpdateResource[P, T]) executeRecipeIfNeeded(ctx context.Context }) } +func getResourceAPIVersion[P rpv1.RadiusResourceModel](resource P) string { + return resource.GetBaseResource().InternalMetadata.UpdatedAPIVersion +} + +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..5bb293bf36 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" 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" "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,609 @@ 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, + "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, + "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) (resp azfake.Responder[v20231001preview.APIVersionsClientGetResponse], errResp azfake.ErrorResponder) { + response := v20231001preview.APIVersionsClientGetResponse{ + APIVersionResource: v20231001preview.APIVersionResource{ + 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 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, + "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, + "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, + "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/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/annotations.go b/pkg/schema/annotations.go index f593e3583d..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" @@ -38,6 +39,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 +74,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. @@ -95,7 +104,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 @@ -149,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/annotations_test.go b/pkg/schema/annotations_test.go index f242fb47ef..1d7ee4d099 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{ @@ -549,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]) + }) + } +} 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, }, },