From 0c5759a3dfe572ab1b3f862a9d63113352423378 Mon Sep 17 00:00:00 2001
From: Anthony Martin <antmarti@microsoft.com>
Date: Wed, 29 Sep 2021 22:28:44 -0400
Subject: [PATCH] Fix cycle detection with resource access syntax (#4684)

---
 .../ScenarioTests.cs                          | 100 ++++++++++++++++++
 .../TypeSystem/CyclicCheckVisitor.cs          |  12 +++
 2 files changed, 112 insertions(+)

diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs
index 88dec0dccb7..d4f8b8a283e 100644
--- a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs
+++ b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs
@@ -2588,6 +2588,106 @@ public void Test_Issue4212()
                 ("BCP036", DiagnosticLevel.Error, "The property \"parent\" expected a value of type \"Microsoft.Network/virtualNetworks\" but the provided value is of type \"tenant\"."),
             });
         }
+        
+        // https://github.com/Azure/bicep/issues/4542
+        [TestMethod]
+        public void Test_Issue4542()
+        {
+            var result = CompilationHelper.Compile(@"
+param sasTokenBaseTime string = utcNow('u')
+param permissions string = 'adlrwu'
+
+var sasTokenParams = {
+  signedPermission: permissions
+  signedExpiry: dateTimeAdd(sasTokenBaseTime, 'PT96H')
+  signedProtocol: 'https'
+  signedResourceTypes: 'sco'
+  signedServices: 'b'
+}
+
+resource storageAccount 'Microsoft.Storage/storageAccounts@2019-04-01' = {
+  name: 'foo'
+  sku: {
+    name: 'Standard_RAGRS'
+  }
+  kind: 'StorageV2'
+  location: 'westus'
+
+  resource blob 'blobServices' = {
+    name: 'default'
+    resource container 'containers' = {
+      name: 'foo'
+      properties: {
+        publicAccess: 'None'
+      }
+    }
+  }
+}
+
+resource registry 'Microsoft.ContainerRegistry/registries@2019-12-01-preview' = {
+  name: 'foo'
+  location: 'westus'
+  sku: {
+    name: 'Premium'
+  }
+
+  resource importPipeline 'importPipelines' = {
+    name: 'foo'
+    location: 'westus'
+    identity: {
+      type: 'SystemAssigned'
+    }
+    properties: {
+      source: {
+        type: 'AzureStorageBlobContainer'
+        uri: uri(storageAccount.properties.primaryEndpoints.blob, storageAccount::blob::container.name)
+        keyVaultUri: kv::secret.properties.secretUri
+      }
+    }
+  }
+}
+
+resource kv 'Microsoft.KeyVault/vaults@2021-06-01-preview' existing = {
+  name: 'foo'
+
+  resource ap 'accessPolicies' = {
+    name: 'add'
+    properties: {
+      accessPolicies: [
+        {
+          tenantId: registry::importPipeline.identity.tenantId
+          objectId: registry::importPipeline.identity.principalId
+          permissions: {
+            secrets: [
+              'get'
+            ]
+          }
+        }
+      ]
+    }
+  }
+
+  resource secret 'secrets' = {
+    name: 'secretname'
+    properties: {
+      value: storageAccount.listAccountSas(storageAccount.apiVersion, sasTokenParams).accountSasToken
+    }
+    dependsOn: [
+      // the below dependency gets a stack overflow
+      ap
+    ]
+  }
+}
+");
+
+            result.Template.Should().NotHaveValue();
+            result.Should().HaveDiagnostics(new[] {
+                ("BCP080", DiagnosticLevel.Error, "The expression is involved in a cycle (\"secret\" -> \"ap\" -> \"importPipeline\")."),
+                ("BCP080", DiagnosticLevel.Error, "The expression is involved in a cycle (\"importPipeline\" -> \"secret\" -> \"ap\")."),
+                ("BCP080", DiagnosticLevel.Error, "The expression is involved in a cycle (\"importPipeline\" -> \"secret\" -> \"ap\")."),
+                ("BCP080", DiagnosticLevel.Error, "The expression is involved in a cycle (\"ap\" -> \"importPipeline\" -> \"secret\")."),
+            });
+        }
 
         /// <summary>
         /// https://github.com/Azure/bicep/issues/2703
diff --git a/src/Bicep.Core/TypeSystem/CyclicCheckVisitor.cs b/src/Bicep.Core/TypeSystem/CyclicCheckVisitor.cs
index 96251060a0e..715fd5db011 100644
--- a/src/Bicep.Core/TypeSystem/CyclicCheckVisitor.cs
+++ b/src/Bicep.Core/TypeSystem/CyclicCheckVisitor.cs
@@ -119,6 +119,18 @@ public override void VisitVariableAccessSyntax(VariableAccessSyntax syntax)
             base.VisitVariableAccessSyntax(syntax);
         }
 
+        public override void VisitResourceAccessSyntax(ResourceAccessSyntax syntax)
+        {
+            if (!currentDeclarations.TryPeek(out var currentDeclaration))
+            {
+                // we're not inside a declaration, so there should be no risk of a cycle
+                return;
+            }
+
+            declarationAccessDict[currentDeclaration].Add(syntax);
+            base.VisitResourceAccessSyntax(syntax);
+        }
+
         public override void VisitFunctionCallSyntax(FunctionCallSyntax syntax)
         {
             if (!currentDeclarations.TryPeek(out var currentDeclaration))