Skip to content

Commit 42d97bb

Browse files
authored
Emit an error diagnostic when a secure output is dereferenced indirectly (#17453)
Resolves #16992 Indirect references to module outputs don't support compilation to the `listOutputsWithSecureValues` function due to a combination of limitations in the compiler and the backend. This PR adds an error diagnostic to expressions that will cause a deployment error today. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/17453)
1 parent a2cf5a9 commit 42d97bb

File tree

7 files changed

+124
-1
lines changed

7 files changed

+124
-1
lines changed

src/Bicep.Core.IntegrationTests/SecureOutputsTests.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,4 +344,56 @@ public void Non_sensitive_outputs_of_collection_module_with_secure_outputs_shoul
344344
result.Template.Should().HaveValueAtPath("$.outputs.sensitive.value", "[listOutputsWithSecureValues(format('mod[{0}]', 0), '2022-09-01').sensitive]");
345345
result.Template.Should().HaveValueAtPath("$.outputs.notSensitive.value", "[reference(format('mod[{0}]', 0)).outputs.notSensitive.value]");
346346
}
347+
348+
[TestMethod]
349+
public void Secure_output_access_should_be_blocked_on_reference_via_inlined_variable()
350+
{
351+
var result = CompilationHelper.Compile(
352+
("mod.bicep", """
353+
@secure()
354+
output sensitive string = 'foo'
355+
356+
output notSensitive string = 'bar'
357+
"""),
358+
("main.bicep", """
359+
param condition bool
360+
361+
module mod 'mod.bicep' = {}
362+
module mod2 'mod.bicep' = {}
363+
364+
var modToUse = condition ? mod : mod2
365+
366+
@secure()
367+
output sensitive string = modToUse.outputs.sensitive
368+
output notSensitive string = modToUse.outputs.notSensitive
369+
"""));
370+
371+
result.Should().HaveDiagnostics(
372+
[
373+
("BCP426", DiagnosticLevel.Error, "Secure outputs may only be accessed via a direct module reference. Only non-sensitive outputs are supported when dereferencing a module indirectly via a variable or lambda."),
374+
]);
375+
}
376+
377+
[TestMethod]
378+
public void Secure_output_access_should_be_blocked_on_reference_via_lambda_variable()
379+
{
380+
var result = CompilationHelper.Compile(
381+
("mod.bicep", """
382+
@secure()
383+
output sensitive string = 'foo'
384+
385+
output notSensitive string = 'bar'
386+
"""),
387+
("main.bicep", """
388+
module mod 'mod.bicep' = [for i in range(0, 1): {}]
389+
390+
output out1 array = map(mod, m => m.outputs.sensitive)
391+
output out2 array = map(mod, m => m.outputs.notSensitive)
392+
"""));
393+
394+
result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(
395+
[
396+
("BCP426", DiagnosticLevel.Error, "Secure outputs may only be accessed via a direct module reference. Only non-sensitive outputs are supported when dereferencing a module indirectly via a variable or lambda."),
397+
]);
398+
}
347399
}

src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,6 +1937,10 @@ public Diagnostic MissingExtensionConfigAssignments(IEnumerable<string> identifi
19371937
public Diagnostic ExtensionConfigAssignmentDoesNotMatchToExtension(string identifier) => CoreError(
19381938
"BCP425",
19391939
$"The extension configuration assignment for \"{identifier}\" does not match an extension in the Bicep file.");
1940+
1941+
public Diagnostic SecureOutputsOnlyAllowedOnDirectModuleReference() => CoreError(
1942+
"BCP426",
1943+
"Secure outputs may only be accessed via a direct module reference. Only non-sensitive outputs are supported when dereferencing a module indirectly via a variable or lambda.");
19401944
}
19411945

19421946
public static DiagnosticBuilderInternal ForPosition(TextSpan span)

src/Bicep.Core/Emit/EmitLimitationCalculator.cs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public static EmitLimitationInfo Calculate(SemanticModel model)
5656
BlockResourceDerivedTypesThatDoNotDereferenceProperties(model, diagnostics);
5757
BlockSpreadInUnsupportedLocations(model, diagnostics);
5858
BlockSecureOutputsWithLocalDeploy(model, diagnostics);
59+
BlockSecureOutputAccessOnIndirectReference(model, diagnostics);
5960
BlockExtendsWithoutFeatureFlagEnabled(model, diagnostics);
6061

6162
var paramAssignments = CalculateParameterAssignments(model, diagnostics);
@@ -992,5 +993,37 @@ private static void BlockSecureOutputsWithLocalDeploy(SemanticModel model, IDiag
992993
}
993994
}
994995
}
996+
997+
private static void BlockSecureOutputAccessOnIndirectReference(
998+
SemanticModel model,
999+
IDiagnosticWriter diagnostics)
1000+
{
1001+
// we're looking for access expressions...
1002+
foreach (var accessExpr in SyntaxAggregator.AggregateByType<AccessExpressionSyntax>(model.Root.Syntax))
1003+
{
1004+
// ... whose base expression is a match for `<something>.outputs`
1005+
if (accessExpr.BaseExpression is AccessExpressionSyntax baseAccessExpr &&
1006+
baseAccessExpr.AccessExpressionMatches(
1007+
SyntaxFactory.CreateStringLiteral(LanguageConstants.ModuleOutputsPropertyName)) &&
1008+
// ... and whose base expression is a module...
1009+
TypeHelper.SatisfiesCondition(model.GetTypeInfo(baseAccessExpr.BaseExpression), t => t is ModuleType) &&
1010+
// ... when the type of the output dereferenced would trigger the use of `listOutputsWithSecureValues`
1011+
TypeHelper.IsOrContainsSecureType(model.GetTypeInfo(accessExpr)))
1012+
{
1013+
if (model.GetSymbolInfo(baseAccessExpr.BaseExpression) is ModuleSymbol ||
1014+
(baseAccessExpr.BaseExpression is ArrayAccessSyntax grandBaseArrayAccess &&
1015+
model.GetSymbolInfo(grandBaseArrayAccess.BaseExpression) is ModuleSymbol greatGrandBaseModule &&
1016+
greatGrandBaseModule.IsCollection))
1017+
{
1018+
// if the module reference is **direct** (e.g., `mod.outputs.sensitive` or
1019+
// `mod[0].outputs.sensitive`, don't raise a diagnostic
1020+
continue;
1021+
}
1022+
1023+
diagnostics.Write(DiagnosticBuilder.ForPosition(accessExpr.IndexExpression)
1024+
.SecureOutputsOnlyAllowedOnDirectModuleReference());
1025+
}
1026+
}
1027+
}
9951028
}
9961029
}

src/Bicep.Core/Syntax/AccessExpressionSyntax.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ protected AccessExpressionSyntax(SyntaxBase baseExpression, Token? safeAccessMar
2222

2323
public Token? SafeAccessMarker { get; }
2424

25+
public abstract SyntaxBase IndexExpression { get; }
26+
2527
public bool IsSafeAccess => SafeAccessMarker is not null;
2628

2729
public abstract AccessExpressionSyntax AsSafeAccess();

src/Bicep.Core/Syntax/ArrayAccessSyntax.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public ArrayAccessSyntax(
3030

3131
public Token? FromEndMarker { get; }
3232

33-
public SyntaxBase IndexExpression { get; }
33+
public override SyntaxBase IndexExpression { get; }
3434

3535
public Token CloseSquare { get; }
3636

src/Bicep.Core/Syntax/PropertyAccessSyntax.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ public PropertyAccessSyntax(SyntaxBase baseExpression, Token dot, Token? safeAcc
2020

2121
public IdentifierSyntax PropertyName { get; }
2222

23+
public override SyntaxBase IndexExpression => PropertyName;
24+
2325
public override PropertyAccessSyntax AsSafeAccess() => SafeAccessMarker is null
2426
? new(BaseExpression, Dot, SyntaxFactory.QuestionToken, PropertyName)
2527
: this;

src/Bicep.Core/TypeSystem/TypeHelper.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,40 @@ public static TypeSymbol GetNamedPropertyType(
183183
isSafeAccess,
184184
shouldWarn,
185185
diagnostics),
186+
null when TryGetModuleUnionBodyType(unionType) is UnionType bodyUnion
187+
=> GetNamedPropertyType(
188+
bodyUnion,
189+
propertyExpressionPositionable,
190+
propertyName,
191+
isSafeAccess,
192+
shouldWarn,
193+
diagnostics),
186194
// TODO improve later here if necessary - we should be able to block stuff that is obviously wrong
187195
_ => LanguageConstants.Any,
188196
};
189197

198+
private static UnionType? TryGetModuleUnionBodyType(UnionType union)
199+
{
200+
if (union.Members.Length < 2)
201+
{
202+
return null;
203+
}
204+
205+
var memberModuleBodies = ImmutableArray.CreateBuilder<ITypeReference>(union.Members.Length);
206+
207+
foreach (var member in union.Members)
208+
{
209+
if (member.Type is not ModuleType moduleType)
210+
{
211+
return null;
212+
}
213+
214+
memberModuleBodies.Add(moduleType.Body.Type);
215+
}
216+
217+
return new(string.Empty, memberModuleBodies.ToImmutable());
218+
}
219+
190220
public static TypeSymbol GetNamedPropertyType(
191221
DiscriminatedObjectType discriminatedObjectType,
192222
IPositionable propertyExpressionPositionable,

0 commit comments

Comments
 (0)