diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/SecretsInParamsMustBeSecureTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/SecretsInParamsMustBeSecureTests.cs index 26000a4d9e6..35c58cff289 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/SecretsInParamsMustBeSecureTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/SecretsInParamsMustBeSecureTests.cs @@ -125,6 +125,136 @@ public void AllowedListExceptions(bool shouldPass, string bicep) CompileAndTest(bicep, shouldPass ? 0 : 1); } + [TestMethod] + public void Unsecure_user_defined_type_reference_with_secret_name_is_flagged() + { + CompileAndTest(""" +type ContainerAppSecretType = { + name: string + value: string +} + +type ContainerAppSecretListType = { + secureList: ContainerAppSecretType[] +} + +param containerAppSecrets ContainerAppSecretListType +""", 1); + } + + [TestMethod] + public void Unsecure_user_defined_string_type_reference_with_secret_name_is_flagged() + { + CompileAndTest(""" +type SecureStringType = string + +param password SecureStringType +""", 1); + } + + [TestMethod] + public void Secure_user_defined_type_reference_with_secret_name_is_not_flagged() + { + CompileAndTest(""" +@secure() +type SecureStringType = string + +param password SecureStringType +""", 0); + } + + [TestMethod] + public void Secure_user_defined_object_type_reference_with_secret_name_is_not_flagged() + { + CompileAndTest(""" +type ContainerAppSecretType = { + name: string + value: string +} + +@secure() +type ContainerAppSecretListType = { + secureList: ContainerAppSecretType[] +} + +param containerAppSecrets ContainerAppSecretListType +""", 0); + } + + [TestMethod] + public void Imported_type_reference_with_secret_name_uses_secure_flag_from_imported_type() + { + var options = new Options(AdditionalFiles: [("types.bicep", """ +@export() +type SecureStringType = string + +@export() +@secure() +type ActuallySecureStringType = string +""")]); + + AssertLinterRuleDiagnostics(SecretsInParamsMustBeSecureRule.Code, """ +import { SecureStringType, ActuallySecureStringType } from './types.bicep' + +param password SecureStringType +param securePassword ActuallySecureStringType +""", 1, options); + } + + [TestMethod] + public void Wildcard_imported_type_reference_with_secret_name_uses_secure_flag_from_imported_type() + { + var options = new Options(AdditionalFiles: [("types.bicep", """ +@export() +type SecureStringType = string + +@export() +@secure() +type ActuallySecureStringType = string +""")]); + + AssertLinterRuleDiagnostics(SecretsInParamsMustBeSecureRule.Code, """ +import * as types from './types.bicep' + +param password types.SecureStringType +param securePassword types.ActuallySecureStringType +""", 1, options); + } + + [TestMethod] + public void Direct_object_with_secret_name_is_still_flagged() + { + CompileAndTest(""" +param containerAppSecrets object +""", 1); + } + + [TestMethod] + public void User_defined_type_reference_defaulting_to_secure_param_is_still_flagged() + { + CompileAndTest(""" +@secure() +param secureParam string + +type SecureStringType = string + +param insecureParam SecureStringType = secureParam +""", 1); + } + + [TestMethod] + public void Codefix_marks_local_type_alias_as_secure() + => AssertCodeFix(SecretsInParamsMustBeSecureRule.Code, "Mark type as secure", """ +type SecureStringType = string + +param pass|word SecureStringType +""", """ +@secure() +type SecureStringType = string + +param password SecureStringType +"""); + [TestMethod] public void FullExample() { diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/SecretsInParamsMustBeSecureRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/SecretsInParamsMustBeSecureRule.cs index 160f590337d..b4a665176ba 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/SecretsInParamsMustBeSecureRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/SecretsInParamsMustBeSecureRule.cs @@ -11,6 +11,7 @@ using Bicep.Core.Syntax; using Bicep.Core.Text; using Bicep.Core.TypeSystem; +using Bicep.Core.TypeSystem.Types; namespace Bicep.Core.Analyzers.Linter.Rules { @@ -49,7 +50,7 @@ override public IEnumerable AnalyzeInternal(SemanticModel model, Di { foreach (var param in model.Root.ParameterDeclarations) { - if (!param.IsSecure()) + if (!param.IsSecure() && !DeclaredTypeReferenceIsSecure(model, param)) { if (AnalyzeUnsecuredParameter(model, diagnosticLevel, param) is IDiagnostic diag) { @@ -86,19 +87,83 @@ override public IEnumerable AnalyzeInternal(SemanticModel model, Di return null; } + private static bool DeclaredTypeReferenceIsSecure(SemanticModel model, ParameterSymbol parameterSymbol) + => RefersToTypeAlias(parameterSymbol.DeclaringParameter.Type, model) && + model.GetDeclaredType(parameterSymbol.DeclaringParameter.Type) is { } declaredType && + DeclaredTypeIsSecure(declaredType); + + private static bool DeclaredTypeIsSecure(TypeSymbol type) + { + var nonNullable = TypeHelper.TryRemoveNullability(type) ?? type; + + return nonNullable.ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure); + } + + private static bool RefersToTypeAlias(SyntaxBase? typeSyntax, SemanticModel model) => UnwrapNullableSyntax(typeSyntax) switch + { + TypeVariableAccessSyntax variableAccess + => model.Binder.GetSymbolInfo(variableAccess) is TypeAliasSymbol or ImportedTypeSymbol or WildcardImportSymbol, + TypePropertyAccessSyntax typePropertyAccess => RefersToTypeAlias(typePropertyAccess.BaseExpression, model), + TypeAdditionalPropertiesAccessSyntax typeAdditionalPropertiesAccess => RefersToTypeAlias(typeAdditionalPropertiesAccess.BaseExpression, model), + TypeArrayAccessSyntax typeArrayAccess + => RefersToTypeAlias(typeArrayAccess.BaseExpression, model) || RefersToTypeAlias(typeArrayAccess.IndexExpression, model), + TypeItemsAccessSyntax typeItemsAccess => RefersToTypeAlias(typeItemsAccess.BaseExpression, model), + _ => false, + }; + + private static SyntaxBase? UnwrapNullableSyntax(SyntaxBase? maybeNullable) => maybeNullable switch + { + NullableTypeSyntax nullable => nullable.Base, + var otherwise => otherwise, + }; + + private static TypeAliasSymbol? TryGetDirectLocalTypeAlias(SyntaxBase typeSyntax, SemanticModel model) => UnwrapNullableSyntax(typeSyntax) switch + { + TypeVariableAccessSyntax variableAccess => model.Binder.GetSymbolInfo(variableAccess) as TypeAliasSymbol, + _ => null, + }; + + private static bool TypeCanBeMarkedSecure(TypeSymbol type) + { + var unwrapped = type is TypeType typeType ? typeType.Unwrapped : type; + var nonNullable = TypeHelper.TryRemoveNullability(unwrapped) ?? unwrapped; + + return nonNullable.IsObject() || nonNullable.IsString(); + } + private IDiagnostic CreateDiagnostic(SemanticModel model, ParameterSymbol parameterSymbol, DiagnosticLevel diagnosticLevel) + { + var fixableDiagnostic = TryCreateCodeFix(model, parameterSymbol) is { } codeFix + ? CreateFixableDiagnosticForSpan(diagnosticLevel, parameterSymbol.NameSource.Span, codeFix, parameterSymbol.Name) + : null; + + return fixableDiagnostic ?? CreateDiagnosticForSpan(diagnosticLevel, parameterSymbol.NameSource.Span, parameterSymbol.Name); + } + + private static CodeFix? TryCreateCodeFix(SemanticModel model, ParameterSymbol parameterSymbol) { var decorator = SyntaxFactory.CreateDecorator("secure"); var newline = model.Configuration.Formatting.Data.NewlineKind.ToEscapeSequence(); var decoratorText = $"{decorator}{newline}"; - var fixSpan = new TextSpan(parameterSymbol.DeclaringSyntax.Span.Position, 0); - var codeReplacement = new CodeReplacement(fixSpan, decoratorText); - - return CreateFixableDiagnosticForSpan( - diagnosticLevel, - parameterSymbol.NameSource.Span, - new CodeFix("Mark parameter as secure", isPreferred: true, CodeFixKind.QuickFix, codeReplacement), - parameterSymbol.Name); + + if (TryGetDirectLocalTypeAlias(parameterSymbol.DeclaringParameter.Type, model) is { } typeAliasSymbol && + TypeCanBeMarkedSecure(typeAliasSymbol.Type)) + { + var fixSpan = new TextSpan(typeAliasSymbol.DeclaringSyntax.Span.Position, 0); + var codeReplacement = new CodeReplacement(fixSpan, decoratorText); + + return new CodeFix("Mark type as secure", isPreferred: true, CodeFixKind.QuickFix, codeReplacement); + } + + if (!RefersToTypeAlias(parameterSymbol.DeclaringParameter.Type, model)) + { + var fixSpan = new TextSpan(parameterSymbol.DeclaringSyntax.Span.Position, 0); + var codeReplacement = new CodeReplacement(fixSpan, decoratorText); + + return new CodeFix("Mark parameter as secure", isPreferred: true, CodeFixKind.QuickFix, codeReplacement); + } + + return null; } } }