diff --git a/src/Bicep.Core.IntegrationTests/Scenarios/ParamKeyVaultSecretReferenceTests.cs b/src/Bicep.Core.IntegrationTests/Scenarios/ParamKeyVaultSecretReferenceTests.cs index ba693ecc927..cc726afd172 100644 --- a/src/Bicep.Core.IntegrationTests/Scenarios/ParamKeyVaultSecretReferenceTests.cs +++ b/src/Bicep.Core.IntegrationTests/Scenarios/ParamKeyVaultSecretReferenceTests.cs @@ -213,6 +213,175 @@ param testParam array result.Should().OnlyContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator."); } + /// + /// https://github.com/Azure/bicep/issues/4270 - getSecret nested inside object should fail + /// + [TestMethod] + public void InvalidKeyVaultSecretReferenceUsageInNestedObject() + { + var result = CompilationHelper.Compile( + ("main.bicep", @" +resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = { + name: 'testkeyvault' +} + +module apim 'apim.bicep' = { + name: 'apim-deployment' + params: { + customUrlInfo: { + url: 'https://api-dev.example.com' + sslInfo: { + certificate: kv.getSecret('api-cert') + certificatePassword: kv.getSecret('api-cert-pwd') + } + } + } +} +"), + ("apim.bicep", @" +param customUrlInfo urlInfo + +type urlInfo = { + url: string + sslInfo: certificateInfo +} + +type certificateInfo = { + @secure() + certificate: string + @secure() + certificatePassword: string +} +")); + + result.Should().NotGenerateATemplate(); + result.Diagnostics.Should().SatisfyRespectively( + x => x.Code.Should().Be("BCP180"), + x => x.Code.Should().Be("BCP180")); + } + + [TestMethod] + public void InvalidKeyVaultSecretReferenceUsageInDeeplyNestedObject() + { + var result = CompilationHelper.Compile( + ("main.bicep", @" +resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = { + name: 'testkeyvault' +} + +module m 'mod.bicep' = { + name: 'deployment' + params: { + level1: { + level2: { + level3: { + secret: kv.getSecret('mySecret') + } + } + } + } +} +"), + ("mod.bicep", @" +param level1 object +")); + + result.Should().NotGenerateATemplate(); + result.Should().ContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator."); + } + + [TestMethod] + public void InvalidKeyVaultSecretReferenceUsageInLoopWithNestedObject() + { + var result = CompilationHelper.Compile( + ("main.bicep", @" +resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = { + name: 'testkeyvault' +} + +var configs = [ + { name: 'config1' } + { name: 'config2' } +] + +module m 'mod.bicep' = [for config in configs: { + name: config.name + params: { + settings: { + secret: kv.getSecret('${config.name}-secret') + } + } +}] +"), + ("mod.bicep", @" +param settings object +")); + + result.Should().NotGenerateATemplate(); + result.Should().ContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator."); + } + + [TestMethod] + public void InvalidKeyVaultSecretReferenceUsageInTernaryReturningNestedObject() + { + var result = CompilationHelper.Compile( + ("main.bicep", @" +resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = { + name: 'testkeyvault' +} + +module m 'mod.bicep' = { + name: 'deployment' + params: { + config: true ? { + secret: kv.getSecret('mySecret') + } : { + secret: 'default' + } + } +} +"), + ("mod.bicep", @" +param config object +")); + + result.Should().NotGenerateATemplate(); + result.Should().ContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator."); + } + + [TestMethod] + public void InvalidKeyVaultSecretReferenceUsageInArrayOfObjects() + { + var result = CompilationHelper.Compile( + ("main.bicep", @" +resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = { + name: 'testkeyvault' +} + +module m 'mod.bicep' = { + name: 'deployment' + params: { + items: [ + { + secret: kv.getSecret('secret1') + } + { + secret: kv.getSecret('secret2') + } + ] + } +} +"), + ("mod.bicep", @" +param items array +")); + + result.Should().NotGenerateATemplate(); + result.Diagnostics.Should().SatisfyRespectively( + x => x.Code.Should().Be("BCP180"), + x => x.Code.Should().Be("BCP180")); + } + [TestMethod] public void ValidKeyVaultSecretReferenceInLoopedModule() diff --git a/src/Bicep.Core/Emit/FunctionPlacementValidatorVisitor.cs b/src/Bicep.Core/Emit/FunctionPlacementValidatorVisitor.cs index 5ed39c13f26..9a20bcc4d59 100644 --- a/src/Bicep.Core/Emit/FunctionPlacementValidatorVisitor.cs +++ b/src/Bicep.Core/Emit/FunctionPlacementValidatorVisitor.cs @@ -94,9 +94,43 @@ private void VerifyModuleSecureParameterFunctionPlacement(FunctionCallSyntaxBase { // we can check placement only for functions that were matched and has a proper placement flag var (_, levelUpSymbol) = syntaxRecorder.Skip(1).SkipWhile(x => x.syntax is TernaryOperationSyntax).FirstOrDefault(); - if (!(elementsRecorder.TryPeek(out var head) && head is VisitedElement.ModuleParams or VisitedElement.ModuleExtensionConfigs) + + // Check if getSecret is nested inside an object structure (not a direct child of params/extensionConfigs) + // Invalid for params: params: { config: { secret: kv.getSecret(...) } } <- ObjectSyntax between function and parameter property + // Valid for params: params: { secret: kv.getSecret(...) } <- No ObjectSyntax between + // Valid for params: params: { secret: cond ? kv.getSecret(...) : 'x' } <- Ternaries are skipped + // Valid for extensionConfigs: extensionConfigs: { alias: { prop: kv.getSecret(...) } } <- 1 ObjectSyntax (alias) is OK + // Invalid for extensionConfigs: extensionConfigs: { alias: { obj: { prop: kv.getSecret(...) } } } <- 2+ ObjectSyntax is invalid + + // Count ObjectSyntax nodes between the immediate property and the params/extensionConfigs value object + // The params value object is the ObjectSyntax that immediately follows the params ObjectPropertySyntax + var ancestors = syntaxRecorder + .Skip(1) // Skip the function call + .SkipWhile(x => x.syntax is TernaryOperationSyntax) // Skip ternary operators + .Skip(1) // Skip the immediate ObjectPropertySyntax (the property containing getSecret, e.g., mySecret, certificate) + .ToList(); + + // Find the params/extensionConfigs property + var paramsPropertyIndex = ancestors.FindIndex(x => + x.syntax is ObjectPropertySyntax ops && + ops.TryGetKeyText() is string key && + (string.Equals(key, LanguageConstants.ModuleParamsPropertyName, LanguageConstants.IdentifierComparison) || + string.Equals(key, LanguageConstants.ModuleExtensionConfigsPropertyName, LanguageConstants.IdentifierComparison))); + + // Count ObjectSyntax nodes before the params property (excluding the params value object) + // Stop one element BEFORE the params property (since the element before params property is the params value object) + var objectSyntaxCount = paramsPropertyIndex >= 1 + ? ancestors.Take(paramsPropertyIndex - 1).Count(x => x.syntax is ObjectSyntax) + : 0; + + var isInExtensionConfigs = elementsRecorder.TryPeek(out var head) && head is VisitedElement.ModuleExtensionConfigs; + var maxAllowedNesting = isInExtensionConfigs ? 1 : 0; // Extension configs allow 1 level (the alias), params allow 0 + var isNestedInObject = levelUpSymbol is PropertySymbol && objectSyntaxCount > maxAllowedNesting; + + if (!(elementsRecorder.TryPeek(out head) && head is VisitedElement.ModuleParams or VisitedElement.ModuleExtensionConfigs) || levelUpSymbol is not PropertySymbol propertySymbol - || !(TypeHelper.TryRemoveNullability(propertySymbol.Type) ?? propertySymbol.Type).ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure)) + || !(TypeHelper.TryRemoveNullability(propertySymbol.Type) ?? propertySymbol.Type).ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure) + || isNestedInObject) { diagnosticWriter.Write(DiagnosticBuilder.ForPosition(syntax) .FunctionOnlyValidInModuleSecureParameterAndExtensionConfigAssignment(functionSymbol.Name, semanticModel.Features.ModuleExtensionConfigsEnabled));