Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block nested deployment resources with inner scoped evaluation and symbolic references to outer scope #11884

125 changes: 119 additions & 6 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5069,12 +5069,125 @@ public void Test_Issue11742()
""");

result.Template.Should().NotBeNull();
// uncomment after merging https://github.com/Azure/bicep/pull/11740
// result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
// {
// ("BCP333", DiagnosticLevel.Warning, "The provided value (whose length will always be less than or equal to 8) is too short to assign to a target for which the minimum allowable length is 36."),
// ("BCP333", DiagnosticLevel.Warning, "The provided value (whose length will always be less than or equal to 8) is too short to assign to a target for which the minimum allowable length is 36."),
// });
result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP333", DiagnosticLevel.Warning, "The provided value (whose length will always be less than or equal to 8) is too short to assign to a target for which the minimum allowable length is 36."),
("BCP333", DiagnosticLevel.Warning, "The provided value (whose length will always be less than or equal to 8) is too short to assign to a target for which the minimum allowable length is 36."),
});
}

// https://github.com/Azure/bicep/issues/11846
[TestMethod]
public void Test_Issue11846()
{
var withOuterScopeEvaluation = """
param tags object
param tag1 string
var tag2 = 'tag2'
var deploymentName = 'name'
var deploymentMode = 'Incremental'

resource nestedDeployment 'Microsoft.Resources/deployments@2020-10-01' = {
name: deploymentName
properties: {
mode: deploymentMode
template: {
'$schema': 'https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#'
contentVersion: '1.0.0.0'
resources: [
{
apiVersion: '2022-09-01'
type: 'Microsoft.Resources/tags'
name: 'default'
properties: {
tags: union(tags, {tag1: tag1, tag2: tag2})
}
}
]
}
}
}
""";
var withExplicitInnerScopeEvaluation = """
param tags object
param tag1 string
var tag2 = 'tag2'
var deploymentName = 'name'
var deploymentMode = 'Incremental'

resource nestedDeployment 'Microsoft.Resources/deployments@2020-10-01' = {
name: deploymentName
properties: {
expressionEvaluationOptions: {
scope: 'inner'
}
mode: deploymentMode
template: {
'$schema': 'https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#'
contentVersion: '1.0.0.0'
resources: [
{
apiVersion: '2022-09-01'
type: 'Microsoft.Resources/tags'
name: 'default'
properties: {
tags: union(tags, {tag1: tag1, tag2: tag2, tag3: join(map(['a'], x => x), ',')})
}
}
]
}
}
}
""";
var withImplicitInnerScopeEvaluation = """
param tags {*: string}
param tag1 string
var tag2 = 'tag2'
var deploymentName = 'name'
var deploymentMode = 'Incremental'

resource nestedDeployment 'Microsoft.Resources/deployments@2020-10-01' = {
name: deploymentName
properties: {
mode: deploymentMode
template: {
'$schema': 'https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#'
contentVersion: '1.0.0.0'
resources: [
{
apiVersion: '2022-09-01'
type: 'Microsoft.Resources/tags'
name: 'default'
properties: {
tags: union(tags, {tag1: tag1, tag2: tag2, tag3: join(map(['a'], x => x), ',')})
}
}
]
}
}
}
""";

using (new AssertionScope())
{
var result = CompilationHelper.Compile(withOuterScopeEvaluation);
result.Should().HaveDiagnostics(new[]
{
("no-deployments-resources", DiagnosticLevel.Warning, "Resource 'nestedDeployment' of type 'Microsoft.Resources/deployments@2020-10-01' should instead be declared as a Bicep module."),
});

foreach (var innerScoped in new[] { withExplicitInnerScopeEvaluation, withImplicitInnerScopeEvaluation })
{
result = CompilationHelper.Compile(innerScoped);
result.Should().HaveDiagnostics(new[]
{
("no-deployments-resources", DiagnosticLevel.Warning, "Resource 'nestedDeployment' of type 'Microsoft.Resources/deployments@2020-10-01' should instead be declared as a Bicep module."),
("nested-deployment-template-scoping", DiagnosticLevel.Error, "The symbol \"tags\" is declared in the context of the outer deployment and cannot be accessed by expressions within a nested deployment template that uses inner scoping for expression evaluation."),
("nested-deployment-template-scoping", DiagnosticLevel.Error, "The symbol \"tag1\" is declared in the context of the outer deployment and cannot be accessed by expressions within a nested deployment template that uses inner scoping for expression evaluation."),
("nested-deployment-template-scoping", DiagnosticLevel.Error, "The symbol \"tag2\" is declared in the context of the outer deployment and cannot be accessed by expressions within a nested deployment template that uses inner scoping for expression evaluation."),
});
}
}
}

// https://github.com/Azure/bicep/issues/11883
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ public void ListKeys_Fail()
]
}
}
}
}
",
new string[]
{
Expand Down Expand Up @@ -621,7 +621,7 @@ public void AzListKeys_Fail()
]
}
}
}
}
",
new string[]
{
Expand Down Expand Up @@ -661,7 +661,7 @@ public void FooListKeys_Ignore()
]
}
}
}
}
",
new string[]
{
Expand Down Expand Up @@ -704,7 +704,7 @@ public void IsDeployment_CaseInsensitive_Fail()
}",
new[]
{
"[6] 'nested' is an outer scoped nested deployment that accesses secure string parameters ('stgAccountName'), which could expose their values in deployment history. Either set the deployment's properties.expressionEvaluationOptions.scope to 'inner' or use a Bicep module instead.",
"[6] 'nested' is an outer scoped nested deployment that accesses secure string parameters ('stgAccountName'), which could expose their values in deployment history. Either set the deployment's properties.expressionEvaluationOptions.scope to 'inner' or use a Bicep module instead.",
});
}

Expand Down Expand Up @@ -740,7 +740,7 @@ public void ListKeys_InLoop_Fail()
]
}
}
}]
}]
",
new string[]
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using Bicep.Core.CodeAction;
using Bicep.Core.Diagnostics;
using Bicep.Core.Navigation;
using Bicep.Core.Parsing;
using Bicep.Core.Semantics;
using Bicep.Core.Syntax;
using Bicep.Core.TypeSystem;
using Bicep.Core.TypeSystem.Az;
using Bicep.Core.Workspaces;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace Bicep.Core.Analyzers.Linter.Rules
{
public sealed class NoSymbolicReferencesInInnerScopedDeploymentResources : LinterRuleBase
{
public new const string Code = "nested-deployment-template-scoping";

public NoSymbolicReferencesInInnerScopedDeploymentResources() : base(
code: Code,
description: CoreResources.NoSymbolicReferencesInInnerScopedDeploymentResourcesDescription,
docUri: new Uri($"https://aka.ms/bicep/linter/{Code}"),
diagnosticLevel: DiagnosticLevel.Error)
{
}

public override string FormatMessage(params object[] values)
=> string.Format(CoreResources.NoSymbolicReferencesInInnerScopedDeploymentResourcesMessageFormat, values);

public override IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model, DiagnosticLevel diagnosticLevel)
{
var innerScopedEvaluationIsDefault = model.EmitterSettings.EnableSymbolicNames;
var topLevelSymbols = model.Root.Declarations.ToHashSet();

foreach (var resource in model.DeclaredResources.Where(r => r.IsAzResource))
{
if (!LanguageConstants.ResourceTypeComparer.Equals(resource.TypeReference.FormatType(), AzResourceTypeProvider.ResourceTypeDeployments) ||
resource.Symbol.DeclaringResource.TryGetBody()?.TryGetPropertyByName("properties", StringComparison.OrdinalIgnoreCase)?.Value is not ObjectSyntax propertiesObject ||
propertiesObject.TryGetPropertyByName("template", StringComparison.OrdinalIgnoreCase)?.Value is not SyntaxBase nestedTemplate)
{
continue;
}

bool explicitInnerScope = (propertiesObject.TryGetPropertyByName("expressionEvaluationOptions", StringComparison.OrdinalIgnoreCase)?.Value as ObjectSyntax)
?.TryGetPropertyByName("scope", StringComparison.OrdinalIgnoreCase)?.Value is SyntaxBase explicitScope &&
model.GetTypeInfo(explicitScope) is StringLiteralType folded &&
folded.RawStringValue.Equals("inner", StringComparison.OrdinalIgnoreCase);

if (explicitInnerScope || innerScopedEvaluationIsDefault)
{
foreach (var (symbolReferenced, references) in SymbolicReferenceCollector.CollectSymbolsReferenced(model.Binder, nestedTemplate))
{
if (!topLevelSymbols.Contains(symbolReferenced))
{
continue;
}

foreach (var reference in references)
{
yield return CreateDiagnosticForSpan(diagnosticLevel, reference.Span, symbolReferenced.Name);
}
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Bicep.Core.Syntax;
using Bicep.Core.Syntax.Visitors;
using Bicep.Core.TypeSystem;
using Bicep.Core.TypeSystem.Az;
using System;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -39,6 +40,12 @@ public override string FormatMessage(params object[] values)

override public IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model, DiagnosticLevel diagnosticLevel)
{
// outer scoped expression evaluation is blocked in symbolic name templates
if (model.EmitterSettings.EnableSymbolicNames)
{
yield break;
}

foreach (ResourceSymbol resource in model.Root.ResourceDeclarations)
{
if (GetPropertiesIfOuterScopedDeployment(resource) is ObjectSyntax propertiesObject)
Expand Down Expand Up @@ -80,7 +87,7 @@ override public IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model, Di
return null;
}

if (!resourceType.TypeReference.FormatType().Equals("microsoft.resources/deployments", LanguageConstants.ResourceTypeComparison))
if (!resourceType.TypeReference.FormatType().Equals(AzResourceTypeProvider.ResourceTypeDeployments, LanguageConstants.ResourceTypeComparison))
{
// Not a deployment resource
return null;
Expand Down
32 changes: 27 additions & 5 deletions src/Bicep.Core/CoreResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Bicep.Core/CoreResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -485,4 +485,10 @@
<data name="ExperimentalFeatureNames_MicrosoftGraphPreview" xml:space="preserve">
<value>MicrosoftGraph Preview</value>
</data>
<data name="NoSymbolicReferencesInInnerScopedDeploymentResourcesDescription" xml:space="preserve">
<value>Nested deployment resources cannot refer to top-level symbols from within the 'template' property when inner-scoped evaluation is used.</value>
</data>
<data name="NoSymbolicReferencesInInnerScopedDeploymentResourcesMessageFormat" xml:space="preserve">
<value>The symbol "{0}" is declared in the context of the outer deployment and cannot be accessed by expressions within a nested deployment template that uses inner scoping for expression evaluation.</value>
</data>
</root>
Loading
Loading