Reuse already updated object types when recursively removing type pro…

…perty flags (#16235) Resolves #16219 The server crash reported in #16219 was caused by https://github.com/Azure/bicep/pull/15825/files#diff-7d66583c4922f57735a4eac96cd074d356e8eace34ec5900860f093f3d0cbc69R701. The FindPossibleSecretsVisitor relies on object identity to detect and stop infinitely walking recursive types, and the linked line caused otherwise identical object types within the assigned type of a `resource` symbol to have different identities. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/16235)
Azure · Jan 28, 2025 · e845112 · e845112
1 parent bfd6836
commit e845112
Show file tree

Hide file tree

Showing 13 changed files with 91 additions and 46 deletions.
diff --git a/global.json b/global.json
@@ -4,7 +4,7 @@
   },
   "sdk": {
     "allowPrerelease": false,
-    "version": "8.0.404",
+    "version": "8.0.405",
     "rollForward": "disable"
   }
 }
diff --git a/src/Bicep.Cli/packages.lock.json b/src/Bicep.Cli/packages.lock.json
@@ -27,9 +27,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.SourceLink.GitHub": {
         "type": "Direct",

diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs
@@ -6747,4 +6747,34 @@ param descriptionParam string
             ("BCP265", DiagnosticLevel.Error, "The name \"description\" is not a function. Did you mean \"sys.description\"?"),
         });
     }
+
+    [TestMethod]
+    public void Test_Issue16219()
+    {
+        // https://www.github.com/Azure/bicep/issues/16219
+        var result = CompilationHelper.Compile("""
+            @description('Required. The name of the Public IP Address.')
+            param name string
+
+            resource publicIpAddress 'Microsoft.Network/publicIPAddresses@2023-09-01' = {
+              name: name
+              location: resourceGroup().location
+              sku: {
+                name: 'Basic'
+                tier: 'Regional'
+              }
+              properties: {
+                publicIPAddressVersion: 'IPv4'
+                publicIPAllocationMethod: 'Static'
+                idleTimeoutInMinutes: 4
+                ipTags: []
+              }
+            }
+
+            @description('The public IP address of the public IP address resource.')
+            output ipAddress string = contains(publicIpAddress.properties, 'ipAddress') ? publicIpAddress.properties.ipAddress : ''
+            """);
+
+        result.ExcludingDiagnostics("use-safe-access").Should().NotHaveAnyDiagnostics();
+    }
 }
diff --git a/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs
@@ -105,7 +105,6 @@ param storageName string
                 output test = v.listAnything().keys[0].value
             "
             )]
-        /* TODO: blocked by https://github.com/Azure/bicep/issues/4833
         [DataRow(@"
                 param storageName string
 
@@ -119,8 +118,8 @@ param storageName string
                   value: storage.listAnything().keys[0].value
                 }
             ",
-            "Outputs should not contain secrets. function 'listAnything'"
-        )]*/
+            "function 'listAnything'"
+        )]
         [DataRow(
             @"
                 param storageName string

diff --git a/src/Bicep.Core/Analyzers/Linter/Common/FindPossibleSecretsVisitor.cs b/src/Bicep.Core/Analyzers/Linter/Common/FindPossibleSecretsVisitor.cs
@@ -17,6 +17,7 @@ public record PossibleSecret(SyntaxBase Syntax, string FoundMessage) { }
 
         private readonly SemanticModel semanticModel;
         private readonly List<PossibleSecret> possibleSecrets = new();
+        private readonly HashSet<TypeSymbol> currentlyProcessing = new();
         private uint trailingAccessExpressions = 0;
 
         /// <summary>
@@ -70,26 +71,24 @@ public override void VisitArrayAccessSyntax(ArrayAccessSyntax syntax)
 
         private static string PossibleSecretMessage(string possibleSecretName) => string.Format(CoreResources.PossibleSecretMessageSecureParam, possibleSecretName);
 
-        private IEnumerable<string> FindPathsToSecureTypeComponents(TypeSymbol type) => FindPathsToSecureTypeComponents(type, "", new());
+        private IEnumerable<string> FindPathsToSecureTypeComponents(TypeSymbol type) => FindPathsToSecureTypeComponents(type, "");
 
-        private IEnumerable<string> FindPathsToSecureTypeComponents(TypeSymbol type, string path, HashSet<TypeSymbol> visited)
+        private IEnumerable<string> FindPathsToSecureTypeComponents(TypeSymbol type, string path)
         {
             // types can be recursive. cut out early if we've already seen this type
-            if (visited.Contains(type))
+            if (!currentlyProcessing.Add(type))
             {
                 yield break;
             }
 
-            visited.Add(type);
-
             if (type.ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure))
             {
                 yield return path;
             }
 
             if (type is UnionType union)
             {
-                foreach (var variantPath in union.Members.SelectMany(m => FindPathsToSecureTypeComponents(m.Type, path, visited)))
+                foreach (var variantPath in union.Members.SelectMany(m => FindPathsToSecureTypeComponents(m.Type, path)))
                 {
                     yield return variantPath;
                 }
@@ -118,33 +117,33 @@ private IEnumerable<string> FindPathsToSecureTypeComponents(TypeSymbol type, str
                     case ObjectType obj:
                         if (obj.AdditionalProperties?.TypeReference.Type is TypeSymbol addlPropsType)
                         {
-                            foreach (var dictMemberPath in FindPathsToSecureTypeComponents(addlPropsType, $"{path}.*", visited))
+                            foreach (var dictMemberPath in FindPathsToSecureTypeComponents(addlPropsType, $"{path}.*"))
                             {
                                 yield return dictMemberPath;
                             }
                         }
 
-                        foreach (var propertyPath in obj.Properties.SelectMany(p => FindPathsToSecureTypeComponents(p.Value.TypeReference.Type, $"{path}.{p.Key}", visited)))
+                        foreach (var propertyPath in obj.Properties.SelectMany(p => FindPathsToSecureTypeComponents(p.Value.TypeReference.Type, $"{path}.{p.Key}")))
                         {
                             yield return propertyPath;
                         }
                         break;
                     case TupleType tuple:
-                        foreach (var pathFromIndex in tuple.Items.SelectMany((ITypeReference typeAtIndex, int index) => FindPathsToSecureTypeComponents(typeAtIndex.Type, $"{path}[{index}]", visited)))
+                        foreach (var pathFromIndex in tuple.Items.SelectMany((ITypeReference typeAtIndex, int index) => FindPathsToSecureTypeComponents(typeAtIndex.Type, $"{path}[{index}]")))
                         {
                             yield return pathFromIndex;
                         }
                         break;
                     case ArrayType array:
-                        foreach (var pathFromElement in FindPathsToSecureTypeComponents(array.Item.Type, $"{path}[*]", visited))
+                        foreach (var pathFromElement in FindPathsToSecureTypeComponents(array.Item.Type, $"{path}[*]"))
                         {
                             yield return pathFromElement;
                         }
                         break;
                 }
             }
 
-            visited.Remove(type);
+            currentlyProcessing.Remove(type);
         }
 
 

diff --git a/src/Bicep.Core/Emit/ExpressionConverter.cs b/src/Bicep.Core/Emit/ExpressionConverter.cs
@@ -448,9 +448,9 @@ LanguageExpression getResourceInfoExpression()
 
                 case "outputs":
                     var moduleSymbol = reference.Module;
-                    var hasSecureValues = expression.SourceSyntax is null
-                        || FindPossibleSecretsVisitor.FindPossibleSecretsInExpression(context.SemanticModel, expression.SourceSyntax).Any();
-                    if (context.SemanticModel.Features.SecureOutputsEnabled && hasSecureValues)
+                    if (context.SemanticModel.Features.SecureOutputsEnabled &&
+                        (expression.SourceSyntax is null ||
+                        FindPossibleSecretsVisitor.FindPossibleSecretsInExpression(context.SemanticModel, expression.SourceSyntax).Any()))
                     {
                         var deploymentResourceId = GetFullyQualifiedResourceId(moduleSymbol, reference.IndexContext?.Index);
                         var apiVersion = new JTokenExpression(EmitConstants.NestedDeploymentResourceApiVersion);

diff --git a/src/Bicep.Core/TypeSystem/TypeHelper.cs b/src/Bicep.Core/TypeSystem/TypeHelper.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+using System.Collections.Concurrent;
 using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
@@ -693,14 +694,30 @@ private static ObjectType TransformProperties(ObjectType input, Func<NamedTypePr
         public static ObjectType MakeRequiredPropertiesOptional(ObjectType input)
             => TransformProperties(input, p => p with { Flags = p.Flags & ~TypePropertyFlags.Required });
 
-        public static TypeSymbol RemovePropertyFlagsRecursively(TypeSymbol type, TypePropertyFlags flagsToRemove) => type switch
-        {
-            ObjectType @object => TransformProperties(@object, property => new(
+        public static TypeSymbol RemovePropertyFlagsRecursively(TypeSymbol type, TypePropertyFlags flagsToRemove)
+            => RemovePropertyFlagsRecursively(type, flagsToRemove, new());
+
+        private static TypeSymbol RemovePropertyFlagsRecursively(
+            TypeSymbol type,
+            TypePropertyFlags flagsToRemove,
+            ConcurrentDictionary<ObjectType, ObjectType> transformedObjectCache) => type switch
+            {
+                ObjectType @object => transformedObjectCache.GetOrAdd(
+                    @object,
+                    obj => RemovePropertyFlagsRecursively(obj, flagsToRemove, transformedObjectCache)),
+                _ => type,
+            };
+
+        private static ObjectType RemovePropertyFlagsRecursively(
+            ObjectType @object,
+            TypePropertyFlags flagsToRemove,
+            ConcurrentDictionary<ObjectType, ObjectType> cache) => TransformProperties(@object, property => new(
                 property.Name,
-                new DeferredTypeReference(() => RemovePropertyFlagsRecursively(property.TypeReference.Type, flagsToRemove)),
+                new DeferredTypeReference(() => RemovePropertyFlagsRecursively(
+                    property.TypeReference.Type,
+                    flagsToRemove,
+                    cache)),
                 property.Flags & ~flagsToRemove,
-                property.Description)),
-            _ => type,
-        };
+                property.Description));
     }
 }
diff --git a/src/Bicep.Core/packages.lock.json b/src/Bicep.Core/packages.lock.json
@@ -186,9 +186,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.PowerPlatform.ResourceStack": {
         "type": "Direct",

diff --git a/src/Bicep.Decompiler/packages.lock.json b/src/Bicep.Decompiler/packages.lock.json
@@ -16,9 +16,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.SourceLink.GitHub": {
         "type": "Direct",

diff --git a/src/Bicep.IO/packages.lock.json b/src/Bicep.IO/packages.lock.json
@@ -16,9 +16,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.SourceLink.GitHub": {
         "type": "Direct",

diff --git a/src/Bicep.Local.Deploy/packages.lock.json b/src/Bicep.Local.Deploy/packages.lock.json
@@ -52,9 +52,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.SourceLink.GitHub": {
         "type": "Direct",

diff --git a/src/Bicep.Local.Extension.Mock/packages.lock.json b/src/Bicep.Local.Extension.Mock/packages.lock.json
@@ -21,9 +21,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.SourceLink.GitHub": {
         "type": "Direct",

diff --git a/src/Bicep.Wasm/packages.lock.json b/src/Bicep.Wasm/packages.lock.json
@@ -23,9 +23,9 @@
       },
       "Microsoft.NET.ILLink.Tasks": {
         "type": "Direct",
-        "requested": "[8.0.11, )",
-        "resolved": "8.0.11",
-        "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ=="
+        "requested": "[8.0.12, )",
+        "resolved": "8.0.12",
+        "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig=="
       },
       "Microsoft.NET.Sdk.WebAssembly.Pack": {
         "type": "Direct",