Skip to content

Commit 9bcf264

Browse files
jiangmingzhejiangmingzhe
committed
Fix #4270: Emit BCP180 error for nested getSecret() instead of crashing
Previously, when getSecret() was used inside nested objects in module parameters (e.g., params: { config: { secret: kv.getSecret('x') } }), the compiler would crash with NotImplementedException during emit instead of validating the placement during semantic analysis. This fix adds validation logic in FunctionPlacementValidatorVisitor to detect when getSecret() is nested inside object structures by: - Walking up the syntax tree from the function call - Counting ObjectSyntax nodes between the property and params boundary - Allowing 0 levels of nesting for params, 1 for extensionConfigs - Emitting BCP180 diagnostic for invalid nested cases The ARM template schema only supports KeyVault references at the top parameter level (as { reference: {...} }), not nested within value objects, so this validation prevents generating invalid ARM templates. Added comprehensive test coverage: - Nested objects at various depths (2-3 levels) - Nested objects in loops and ternary expressions - Nested objects inside arrays - All existing tests continue to pass (valid cases unaffected) Fixes #4270
1 parent 93396c4 commit 9bcf264

2 files changed

Lines changed: 205 additions & 2 deletions

File tree

‎src/Bicep.Core.IntegrationTests/Scenarios/ParamKeyVaultSecretReferenceTests.cs‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,175 @@ param testParam array
213213
result.Should().OnlyContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.");
214214
}
215215

216+
/// <summary>
217+
/// https://github.com/Azure/bicep/issues/4270 - getSecret nested inside object should fail
218+
/// </summary>
219+
[TestMethod]
220+
public void InvalidKeyVaultSecretReferenceUsageInNestedObject()
221+
{
222+
var result = CompilationHelper.Compile(
223+
("main.bicep", @"
224+
resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = {
225+
name: 'testkeyvault'
226+
}
227+
228+
module apim 'apim.bicep' = {
229+
name: 'apim-deployment'
230+
params: {
231+
customUrlInfo: {
232+
url: 'https://api-dev.example.com'
233+
sslInfo: {
234+
certificate: kv.getSecret('api-cert')
235+
certificatePassword: kv.getSecret('api-cert-pwd')
236+
}
237+
}
238+
}
239+
}
240+
"),
241+
("apim.bicep", @"
242+
param customUrlInfo urlInfo
243+
244+
type urlInfo = {
245+
url: string
246+
sslInfo: certificateInfo
247+
}
248+
249+
type certificateInfo = {
250+
@secure()
251+
certificate: string
252+
@secure()
253+
certificatePassword: string
254+
}
255+
"));
256+
257+
result.Should().NotGenerateATemplate();
258+
result.Diagnostics.Should().SatisfyRespectively(
259+
x => x.Code.Should().Be("BCP180"),
260+
x => x.Code.Should().Be("BCP180"));
261+
}
262+
263+
[TestMethod]
264+
public void InvalidKeyVaultSecretReferenceUsageInDeeplyNestedObject()
265+
{
266+
var result = CompilationHelper.Compile(
267+
("main.bicep", @"
268+
resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = {
269+
name: 'testkeyvault'
270+
}
271+
272+
module m 'mod.bicep' = {
273+
name: 'deployment'
274+
params: {
275+
level1: {
276+
level2: {
277+
level3: {
278+
secret: kv.getSecret('mySecret')
279+
}
280+
}
281+
}
282+
}
283+
}
284+
"),
285+
("mod.bicep", @"
286+
param level1 object
287+
"));
288+
289+
result.Should().NotGenerateATemplate();
290+
result.Should().ContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.");
291+
}
292+
293+
[TestMethod]
294+
public void InvalidKeyVaultSecretReferenceUsageInLoopWithNestedObject()
295+
{
296+
var result = CompilationHelper.Compile(
297+
("main.bicep", @"
298+
resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = {
299+
name: 'testkeyvault'
300+
}
301+
302+
var configs = [
303+
{ name: 'config1' }
304+
{ name: 'config2' }
305+
]
306+
307+
module m 'mod.bicep' = [for config in configs: {
308+
name: config.name
309+
params: {
310+
settings: {
311+
secret: kv.getSecret('${config.name}-secret')
312+
}
313+
}
314+
}]
315+
"),
316+
("mod.bicep", @"
317+
param settings object
318+
"));
319+
320+
result.Should().NotGenerateATemplate();
321+
result.Should().ContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.");
322+
}
323+
324+
[TestMethod]
325+
public void InvalidKeyVaultSecretReferenceUsageInTernaryReturningNestedObject()
326+
{
327+
var result = CompilationHelper.Compile(
328+
("main.bicep", @"
329+
resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = {
330+
name: 'testkeyvault'
331+
}
332+
333+
module m 'mod.bicep' = {
334+
name: 'deployment'
335+
params: {
336+
config: true ? {
337+
secret: kv.getSecret('mySecret')
338+
} : {
339+
secret: 'default'
340+
}
341+
}
342+
}
343+
"),
344+
("mod.bicep", @"
345+
param config object
346+
"));
347+
348+
result.Should().NotGenerateATemplate();
349+
result.Should().ContainDiagnostic("BCP180", DiagnosticLevel.Error, "Function \"getSecret\" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator.");
350+
}
351+
352+
[TestMethod]
353+
public void InvalidKeyVaultSecretReferenceUsageInArrayOfObjects()
354+
{
355+
var result = CompilationHelper.Compile(
356+
("main.bicep", @"
357+
resource kv 'Microsoft.KeyVault/vaults@2019-09-01' existing = {
358+
name: 'testkeyvault'
359+
}
360+
361+
module m 'mod.bicep' = {
362+
name: 'deployment'
363+
params: {
364+
items: [
365+
{
366+
secret: kv.getSecret('secret1')
367+
}
368+
{
369+
secret: kv.getSecret('secret2')
370+
}
371+
]
372+
}
373+
}
374+
"),
375+
("mod.bicep", @"
376+
param items array
377+
"));
378+
379+
result.Should().NotGenerateATemplate();
380+
result.Diagnostics.Should().SatisfyRespectively(
381+
x => x.Code.Should().Be("BCP180"),
382+
x => x.Code.Should().Be("BCP180"));
383+
}
384+
216385

217386
[TestMethod]
218387
public void ValidKeyVaultSecretReferenceInLoopedModule()

‎src/Bicep.Core/Emit/FunctionPlacementValidatorVisitor.cs‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,43 @@ private void VerifyModuleSecureParameterFunctionPlacement(FunctionCallSyntaxBase
9494
{
9595
// we can check placement only for functions that were matched and has a proper placement flag
9696
var (_, levelUpSymbol) = syntaxRecorder.Skip(1).SkipWhile(x => x.syntax is TernaryOperationSyntax).FirstOrDefault();
97-
if (!(elementsRecorder.TryPeek(out var head) && head is VisitedElement.ModuleParams or VisitedElement.ModuleExtensionConfigs)
97+
98+
// Check if getSecret is nested inside an object structure (not a direct child of params/extensionConfigs)
99+
// Invalid for params: params: { config: { secret: kv.getSecret(...) } } <- ObjectSyntax between function and parameter property
100+
// Valid for params: params: { secret: kv.getSecret(...) } <- No ObjectSyntax between
101+
// Valid for params: params: { secret: cond ? kv.getSecret(...) : 'x' } <- Ternaries are skipped
102+
// Valid for extensionConfigs: extensionConfigs: { alias: { prop: kv.getSecret(...) } } <- 1 ObjectSyntax (alias) is OK
103+
// Invalid for extensionConfigs: extensionConfigs: { alias: { obj: { prop: kv.getSecret(...) } } } <- 2+ ObjectSyntax is invalid
104+
105+
// Count ObjectSyntax nodes between the immediate property and the params/extensionConfigs value object
106+
// The params value object is the ObjectSyntax that immediately follows the params ObjectPropertySyntax
107+
var ancestors = syntaxRecorder
108+
.Skip(1) // Skip the function call
109+
.SkipWhile(x => x.syntax is TernaryOperationSyntax) // Skip ternary operators
110+
.Skip(1) // Skip the immediate ObjectPropertySyntax (the property containing getSecret, e.g., mySecret, certificate)
111+
.ToList();
112+
113+
// Find the params/extensionConfigs property
114+
var paramsPropertyIndex = ancestors.FindIndex(x =>
115+
x.syntax is ObjectPropertySyntax ops &&
116+
ops.TryGetKeyText() is string key &&
117+
(string.Equals(key, LanguageConstants.ModuleParamsPropertyName, LanguageConstants.IdentifierComparison) ||
118+
string.Equals(key, LanguageConstants.ModuleExtensionConfigsPropertyName, LanguageConstants.IdentifierComparison)));
119+
120+
// Count ObjectSyntax nodes before the params property (excluding the params value object)
121+
// Stop one element BEFORE the params property (since the element before params property is the params value object)
122+
var objectSyntaxCount = paramsPropertyIndex >= 1
123+
? ancestors.Take(paramsPropertyIndex - 1).Count(x => x.syntax is ObjectSyntax)
124+
: 0;
125+
126+
var isInExtensionConfigs = elementsRecorder.TryPeek(out var head) && head is VisitedElement.ModuleExtensionConfigs;
127+
var maxAllowedNesting = isInExtensionConfigs ? 1 : 0; // Extension configs allow 1 level (the alias), params allow 0
128+
var isNestedInObject = levelUpSymbol is PropertySymbol && objectSyntaxCount > maxAllowedNesting;
129+
130+
if (!(elementsRecorder.TryPeek(out head) && head is VisitedElement.ModuleParams or VisitedElement.ModuleExtensionConfigs)
98131
|| levelUpSymbol is not PropertySymbol propertySymbol
99-
|| !(TypeHelper.TryRemoveNullability(propertySymbol.Type) ?? propertySymbol.Type).ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure))
132+
|| !(TypeHelper.TryRemoveNullability(propertySymbol.Type) ?? propertySymbol.Type).ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure)
133+
|| isNestedInObject)
100134
{
101135
diagnosticWriter.Write(DiagnosticBuilder.ForPosition(syntax)
102136
.FunctionOnlyValidInModuleSecureParameterAndExtensionConfigAssignment(functionSymbol.Name, semanticModel.Features.ModuleExtensionConfigsEnabled));

0 commit comments

Comments
 (0)