From 1642794922cbdd21a741ce8e86ce574e07347e80 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 1 Oct 2021 17:32:59 -0700 Subject: [PATCH 01/23] new rule: outputs-should-not-contain-secrets --- .../outputs-should-not-contain-secrets.md | 44 +++++ .../Diagnostics/LinterAnalyzerTests.cs | 5 +- .../LinterRuleTests/LinterRuleTestsBase.cs | 8 + ...OutputsShouldNotContainSecretsRuleTests.cs | 179 ++++++++++++++++++ .../AdminUsernameShouldNotBeLiteralRule.cs | 2 +- .../Rules/NoHardcodedEnvironmentUrlsRule.cs | 2 +- .../Linter/Rules/NoUnusedParametersRule.cs | 3 +- .../Linter/Rules/NoUnusedVariablesRule.cs | 3 +- .../OutputsShouldNotContainSecretsRule.cs | 138 ++++++++++++++ .../Linter/Rules/PreferInterpolationRule.cs | 2 +- .../Rules/SecureParameterDefaultRule.cs | 3 +- .../Linter/Rules/SimplifyInterpolationRule.cs | 3 +- src/Bicep.Core/CoreResources.Designer.cs | 12 ++ src/Bicep.Core/CoreResources.resx | 7 + .../schemas/bicepconfig.schema.json | 13 ++ 15 files changed, 415 insertions(+), 9 deletions(-) create mode 100644 docs/linter-rules/outputs-should-not-contain-secrets.md create mode 100644 src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs create mode 100644 src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs diff --git a/docs/linter-rules/outputs-should-not-contain-secrets.md b/docs/linter-rules/outputs-should-not-contain-secrets.md new file mode 100644 index 00000000000..c437b97791d --- /dev/null +++ b/docs/linter-rules/outputs-should-not-contain-secrets.md @@ -0,0 +1,44 @@ +# Outputs should not contain secrets + +**Code**: outputs-should-not-contain-secrets + +**Description**: Don't include any values in an output that could potentially expose secrets. For example, secure parameters of type secureString or secureObject, or list* functions such as listKeys. + +The output from a template is stored in the deployment history, so a malicious user could find that information. + +The following example fails because it includes a secure parameter in an output value. +```bicep +@secure() +param secureParam string + +output badResult string = 'this is the value ${secureParam}' +``` + +The following example fails because it uses a list* member function in an output. +```bicep +param storageName string +resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName +} + +output badResult object = { + value: stg.listKeys().keys[0].value +} +``` + +The following example fails because it uses a list* function in an output. +```bicep +param storageName string +resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName +} + +output badResult object = { + value: listKeys(resourceId('Microsoft.Storage/storageAccounts', stg.name), '2021-02-01') +} +``` + +The following example fails because the output name contains 'password', indicating that it may contains a secret +```bicep +output accountPassword string = '...' +``` \ No newline at end of file diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterAnalyzerTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterAnalyzerTests.cs index a3573f72e83..8499faeb419 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterAnalyzerTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterAnalyzerTests.cs @@ -29,13 +29,14 @@ public void HasBuiltInRules() } [DataTestMethod] + [DataRow(AdminUsernameShouldNotBeLiteralRule.Code)] [DataRow(NoHardcodedEnvironmentUrlsRule.Code)] [DataRow(PreferInterpolationRule.Code)] [DataRow(NoUnusedParametersRule.Code)] + [DataRow(NoUnusedVariablesRule.Code)] + [DataRow(OutputsShouldNotContainSecretsRule.Code)] [DataRow(SecureParameterDefaultRule.Code)] [DataRow(SimplifyInterpolationRule.Code)] - [DataRow(NoUnusedVariablesRule.Code)] - [DataRow(AdminUsernameShouldNotBeLiteralRule.Code)] public void BuiltInRulesExist(string ruleCode) { var linter = new LinterAnalyzer(configuration); diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs index bef06167b8e..17d016b1de3 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs @@ -22,6 +22,14 @@ public enum OnCompileErrors Ignore, } + protected void AssertLinterRuleDiagnostics(string ruleCode, string bicepText, string[] expectedMessagesForCode, OnCompileErrors onCompileErrors = OnCompileErrors.Fail) + { + AssertLinterRuleDiagnostics(ruleCode, bicepText, onCompileErrors, diags => + { + diags.Select(d => d.Message).Should().BeEquivalentTo(expectedMessagesForCode); + }); + } + protected void AssertLinterRuleDiagnostics(string ruleCode, string bicepText, int expectedDiagnosticCountForCode, OnCompileErrors onCompileErrors = OnCompileErrors.Fail) { AssertLinterRuleDiagnostics(ruleCode, bicepText, onCompileErrors, diags => diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs new file mode 100644 index 00000000000..1565b2f2c56 --- /dev/null +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs @@ -0,0 +1,179 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Bicep.Core.Analyzers.Linter.Rules; +using Bicep.Core.UnitTests.Assertions; +using FluentAssertions; +using FluentAssertions.Execution; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Linq; + +namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests +{ + [TestClass] + public class OutputsShouldNotContainSecretsRuleTests : LinterRuleTestsBase + { + private void CompileAndTest(string text, OnCompileErrors onCompileErrors, string[] expectedMessages) + { + AssertLinterRuleDiagnostics(OutputsShouldNotContainSecretsRule.Code, text, expectedMessages, onCompileErrors); + } + + [DataRow(@" // This is a failing example from the docs + @secure() + param secureParam string + + output badResult string = 'this is the value ${secureParam}' + ", + "Don't include secrets in an output. Found: 'secureParam'" + )] + [DataRow(@" + @secure() + param secureParam string + + output badResult object = { + value1: { + value2: 'this is the value ${secureParam}' + } + } + ", + "Don't include secrets in an output. Found: 'secureParam'" + )] + [DataRow(@" + @secure() + param secureParam object = { + value: 'hello' + } + + output badResult string = 'this is the value ${secureParam.value}' + ", + // TTK output: + // [-] Outputs Must Not Contain Secrets + // Output contains secureObject parameter: badResult + "Don't include secrets in an output. Found: 'secureParam'" + )] + [DataTestMethod] + public void If_OutputReferencesSecureParam_ShouldFail(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Fail, expectedMessages); + } + + [DataRow(@" + param secureParam object = { + value: 'hello' + } + + output badResult string = 'this is the value ${secureParam}' + " + )] + [DataRow(@" + param secureParam string + + output badResult object = { + value1: { + value2: 'this is the value ${secureParam}' + } + } + " + )] + [DataTestMethod] + public void If_ParamNotSecure_ShouldPass(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Fail, expectedMessages); + } + + [DataRow(@" + param storageName string + + resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName + } + + output badResult object = { + value: stg.listKeys().keys[0].value + } + ", + "Don't include secrets in an output. Found: 'listKeys'" + )] + [DataRow(@" + param storageName string + + resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName + } + + output badResult object = { + value: stg.listAnything().keys[0].value + } + ", + "Don't include secrets in an output. Found: 'listAnything'" + )] + [DataTestMethod] + public void If_ListFunctionInOutput_AsResourceMethod_ShouldFail(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); + } + + [DataRow(@" + param storageName string + + resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName + } + + output badResult object = listKeys(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01') + ", + // TTK output: + // [-] Outputs Must Not Contain Secrets(6 ms) + // Output contains secret: badResult + "Don't include secrets in an output. Found: 'listKeys'" + )] + [DataRow(@" + param storageName string + + resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName + } + + output badResult object = listAnything(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01') + ", + // TTK output: + // [-] Outputs Must Not Contain Secrets(6 ms) + // Output contains secret: badResult + "Don't include secrets in an output. Found: 'listAnything'" + )] + [DataTestMethod] + public void If_ListFunctionInOutput_AsStandaloneFunction_ShouldFail(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); + } + + [DataRow(@" + output badResultPassword string = 'hello' + ", + // TTK output: + // [-] Outputs Must Not Contain Secrets(6 ms) + // Output name suggests secret: badResultPassword + "Don't include secrets in an output. Found: 'badResultPassword'" + )] + [DataRow(@" + output possiblepassword string = 'hello' + ", + "Don't include secrets in an output. Found: 'possiblepassword'" + )] + [DataRow(@" + output password string = 'hello' + ", + "Don't include secrets in an output. Found: 'password'" + )] + [DataRow(@" + output passwordNumber1 string = 'hello' + ", + "Don't include secrets in an output. Found: 'passwordNumber1'" + )] + [DataTestMethod] + public void If_OutputNameLooksLikePassword_ShouldFail(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); + } + } +} diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs index 39ff93b705b..95899a9cb5f 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs @@ -20,7 +20,7 @@ public sealed class AdminUsernameShouldNotBeLiteralRule : LinterRuleBase public AdminUsernameShouldNotBeLiteralRule() : base( code: Code, description: CoreResources.AdminUsernameShouldNotBeLiteralRuleDescription, - docUri: new Uri("https://aka.ms/bicep/linter/adminusername-should-not-be-literal") + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}") ) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs index 001b8f87b46..a86158891bb 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs @@ -29,7 +29,7 @@ public sealed class NoHardcodedEnvironmentUrlsRule : LinterRuleBase public NoHardcodedEnvironmentUrlsRule() : base( code: Code, description: CoreResources.EnvironmentUrlHardcodedRuleDescription, - docUri: new Uri("https://aka.ms/bicep/linter/no-hardcoded-env-urls")) + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs index d7291d84ed0..edc38543239 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs @@ -5,6 +5,7 @@ using Bicep.Core.Parsing; using Bicep.Core.Semantics; using Bicep.Core.Syntax; +using System; using System.Collections.Generic; using System.Linq; @@ -16,7 +17,7 @@ public sealed class NoUnusedParametersRule : LinterRuleBase public NoUnusedParametersRule() : base( code: Code, description: CoreResources.ParameterMustBeUsedRuleDescription, - docUri: new System.Uri("https://aka.ms/bicep/linter/no-unused-params"), + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}"), diagnosticLabel: Diagnostics.DiagnosticLabel.Unnecessary) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs index 5fc648e14b7..80d42ecdf27 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs @@ -4,6 +4,7 @@ using Bicep.Core.Diagnostics; using Bicep.Core.Semantics; using Bicep.Core.Syntax; +using System; using System.Collections.Generic; using System.Linq; @@ -16,7 +17,7 @@ public sealed class NoUnusedVariablesRule : LinterRuleBase public NoUnusedVariablesRule() : base( code: Code, description: CoreResources.UnusedVariableRuleDescription, - docUri: new System.Uri("https://aka.ms/bicep/linter/no-unused-vars"), + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}"), diagnosticLabel: Diagnostics.DiagnosticLabel.Unnecessary) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs new file mode 100644 index 00000000000..bc8d66d0878 --- /dev/null +++ b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs @@ -0,0 +1,138 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Deployments.Core.Extensions; +using Bicep.Core.Diagnostics; +using Bicep.Core.Parsing; +using Bicep.Core.Semantics; +using Bicep.Core.Syntax; +using Bicep.Core.TypeSystem; +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; + +namespace Bicep.Core.Analyzers.Linter.Rules +{ + public sealed class OutputsShouldNotContainSecretsRule : LinterRuleBase + { + public new const string Code = "outputs-should-not-contain-secrets"; + + public OutputsShouldNotContainSecretsRule() : base( + code: Code, + description: CoreResources.OutputsShouldNotContainSecretsRuleDescription, + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}") + ) + { + } + + public override string FormatMessage(params object[] values) + { + return string.Format( + CoreResources.OutputsShouldNotContainSecretsMessageFormat, + this.Description, + values.First()); + } + + public override IEnumerable AnalyzeInternal(SemanticModel model) + { + var visitor = new OutputVisitor(this, model); + visitor.Visit(model.SourceFile.ProgramSyntax); + return visitor.diagnostics; + } + + private class OutputVisitor : SyntaxVisitor + { + public List diagnostics = new List(); + + private readonly OutputsShouldNotContainSecretsRule parent; + private readonly SemanticModel model; + + public OutputVisitor(OutputsShouldNotContainSecretsRule parent, SemanticModel model) + { + this.parent = parent; + this.model = model; + } + + public override void VisitOutputDeclarationSyntax(OutputDeclarationSyntax syntax) + { + // Does the output name contain 'password' (suggesting it contains an actual password)? + if (syntax.Name.IdentifierName.ToLowerInvariant().Contains("password")) + { + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, syntax.Name.IdentifierName)); + } + + var visitor = new OutputValueVisitor(this.parent, diagnostics, model); + visitor.Visit(syntax); + + // Note: No need to navigate deeper, don't call base + } + } + + private class OutputValueVisitor : SyntaxVisitor + { + private List diagnostics; + + private readonly OutputsShouldNotContainSecretsRule parent; + private readonly SemanticModel model; + private const string ListFunctionPrefix = "list"; + + public OutputValueVisitor(OutputsShouldNotContainSecretsRule parent, List diagnostics, SemanticModel model) + { + this.parent = parent; + this.model = model; + this.diagnostics = diagnostics; + } + + public override void VisitVariableAccessSyntax(VariableAccessSyntax syntax) + { + // Look for references of secure parameters, e.g.: + // + // @secure() + // param secureParam string + // output badResult string = 'this is the value ${secureParam}' + + Symbol? symbol = model.GetSymbolInfo(syntax); + if (symbol is ParameterSymbol param) + { + if (param.IsSecure()) + { + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, param.Name)); + } + } + + base.VisitVariableAccessSyntax(syntax); + } + + public override void VisitFunctionCallSyntax(FunctionCallSyntax syntax) + { + // Look for usage of list*() stand-alone function, e.g.: + // + // output badResult object = listKeys(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01') + // + + if (syntax.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) + { + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, syntax.Name.IdentifierName)); + } + + base.VisitFunctionCallSyntax(syntax); + } + + public override void VisitInstanceFunctionCallSyntax(InstanceFunctionCallSyntax syntax) + { + // Look for usage of list*() member function, e.g.: + // + // output badResult object = stg.listKeys().keys[0].value + // + + if (syntax.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) + { + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, syntax.Name.IdentifierName)); + } + + base.VisitInstanceFunctionCallSyntax(syntax); + } + } + } +} diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs index 665977f777d..6e7ff98e1da 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs @@ -21,7 +21,7 @@ public sealed class PreferInterpolationRule : LinterRuleBase public PreferInterpolationRule() : base( code: Code, description: CoreResources.InterpolateNotConcatRuleDescription, - docUri: new Uri("https://aka.ms/bicep/linter/prefer-interpolation")) + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) { } public override IEnumerable AnalyzeInternal(SemanticModel model) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs index 7d2d6989a2a..cbfdcfd9f2e 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs @@ -5,6 +5,7 @@ using Bicep.Core.Diagnostics; using Bicep.Core.Semantics; using Bicep.Core.Syntax; +using System; using System.Collections.Generic; using System.Linq; @@ -17,7 +18,7 @@ public sealed class SecureParameterDefaultRule : LinterRuleBase public SecureParameterDefaultRule() : base( code: Code, description: CoreResources.SecureParameterDefaultRuleDescription, - docUri: new System.Uri("https://aka.ms/bicep/linter/secure-parameter-default")) + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) { } override public IEnumerable AnalyzeInternal(SemanticModel model) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs index ba4c6e03aca..8fa691f99de 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs @@ -7,6 +7,7 @@ using Bicep.Core.Semantics; using Bicep.Core.Syntax; using Bicep.Core.TypeSystem; +using System; using System.Collections.Generic; using System.Linq; @@ -19,7 +20,7 @@ public sealed class SimplifyInterpolationRule : LinterRuleBase public SimplifyInterpolationRule() : base( code: Code, description: CoreResources.SimplifyInterpolationRuleDescription, - docUri: new System.Uri("https://aka.ms/bicep/linter/simplify-interpolation")) + docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) { } public override IEnumerable AnalyzeInternal(SemanticModel model) diff --git a/src/Bicep.Core/CoreResources.Designer.cs b/src/Bicep.Core/CoreResources.Designer.cs index e9a3491a842..dce98653a8a 100644 --- a/src/Bicep.Core/CoreResources.Designer.cs +++ b/src/Bicep.Core/CoreResources.Designer.cs @@ -65,6 +65,18 @@ internal static string AdminUsernameShouldNotBeLiteralRuleDescription { } } + internal static string OutputsShouldNotContainSecretsRuleDescription { + get { + return ResourceManager.GetString("OutputsShouldNotContainSecretsRuleDescription", resourceCulture); + } + } + + internal static string OutputsShouldNotContainSecretsMessageFormat { + get { + return ResourceManager.GetString("OutputsShouldNotContainSecretsMessageFormat", resourceCulture); + } + } + internal static string EnvironmentUrlHardcodedRuleDescription { get { return ResourceManager.GetString("EnvironmentUrlHardcodedRuleDescription", resourceCulture); diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 800bdb2f2ed..9b74a22ec6d 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -127,6 +127,13 @@ When setting an adminUserName property, don't use a literal value. + + Don't include secrets in an output. + + + {0} Found: '{1}' + {0} is the rule description as a sentence ending with a period, {1} is the name of what was found, e.g. 'myParameter' + Environment URLs should not be hardcoded. Use the environment() function to ensure compatibility across clouds. diff --git a/src/vscode-bicep/schemas/bicepconfig.schema.json b/src/vscode-bicep/schemas/bicepconfig.schema.json index 7f552e2b1b5..3dc3982f6a2 100644 --- a/src/vscode-bicep/schemas/bicepconfig.schema.json +++ b/src/vscode-bicep/schemas/bicepconfig.schema.json @@ -57,6 +57,9 @@ "no-unused-vars": { "level": "warning" }, + "outputs-should-not-contain-secrets": { + "level": "warning" + }, "prefer-interpolation": { "level": "warning" }, @@ -161,6 +164,16 @@ } ] }, + "outputs-should-not-contain-secrets": { + "allOf": [ + { + "description": "Don't include secrets in an output. See https://aka.ms/bicep/linter/outputs-should-not-contain-secrets" + }, + { + "$ref": "#/definitions/rule" + } + ] + }, "prefer-interpolation": { "allOf": [ { From 5a24eabb340545dd8a9a1c9175de65052cce87cb Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Mon, 4 Oct 2021 11:39:18 -0700 Subject: [PATCH 02/23] better messages --- ...OutputsShouldNotContainSecretsRuleTests.cs | 22 +++++++++---------- .../OutputsShouldNotContainSecretsRule.cs | 12 ++++++---- src/Bicep.Core/CoreResources.Designer.cs | 18 +++++++++++++++ src/Bicep.Core/CoreResources.resx | 14 +++++++++++- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs index 1565b2f2c56..e09c02684dc 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs @@ -24,7 +24,7 @@ param secureParam string output badResult string = 'this is the value ${secureParam}' ", - "Don't include secrets in an output. Found: 'secureParam'" + "Don't include secrets in an output. Found: secure parameter 'secureParam'" )] [DataRow(@" @secure() @@ -36,7 +36,7 @@ param secureParam string } } ", - "Don't include secrets in an output. Found: 'secureParam'" + "Don't include secrets in an output. Found: secure parameter 'secureParam'" )] [DataRow(@" @secure() @@ -49,7 +49,7 @@ param secureParam string // TTK output: // [-] Outputs Must Not Contain Secrets // Output contains secureObject parameter: badResult - "Don't include secrets in an output. Found: 'secureParam'" + "Don't include secrets in an output. Found: secure parameter 'secureParam'" )] [DataTestMethod] public void If_OutputReferencesSecureParam_ShouldFail(string text, params string[] expectedMessages) @@ -92,7 +92,7 @@ param storageName string value: stg.listKeys().keys[0].value } ", - "Don't include secrets in an output. Found: 'listKeys'" + "Don't include secrets in an output. Found: function 'listKeys'" )] [DataRow(@" param storageName string @@ -105,7 +105,7 @@ param storageName string value: stg.listAnything().keys[0].value } ", - "Don't include secrets in an output. Found: 'listAnything'" + "Don't include secrets in an output. Found: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsResourceMethod_ShouldFail(string text, params string[] expectedMessages) @@ -125,7 +125,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found: 'listKeys'" + "Don't include secrets in an output. Found: function 'listKeys'" )] [DataRow(@" param storageName string @@ -139,7 +139,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found: 'listAnything'" + "Don't include secrets in an output. Found: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsStandaloneFunction_ShouldFail(string text, params string[] expectedMessages) @@ -153,22 +153,22 @@ public void If_ListFunctionInOutput_AsStandaloneFunction_ShouldFail(string text, // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output name suggests secret: badResultPassword - "Don't include secrets in an output. Found: 'badResultPassword'" + "Don't include secrets in an output. Found: output name 'badResultPassword' suggests a secret" )] [DataRow(@" output possiblepassword string = 'hello' ", - "Don't include secrets in an output. Found: 'possiblepassword'" + "Don't include secrets in an output. Found: output name 'possiblepassword' suggests a secret" )] [DataRow(@" output password string = 'hello' ", - "Don't include secrets in an output. Found: 'password'" + "Don't include secrets in an output. Found: output name 'password' suggests a secret" )] [DataRow(@" output passwordNumber1 string = 'hello' ", - "Don't include secrets in an output. Found: 'passwordNumber1'" + "Don't include secrets in an output. Found: output name 'passwordNumber1' suggests a secret" )] [DataTestMethod] public void If_OutputNameLooksLikePassword_ShouldFail(string text, params string[] expectedMessages) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs index bc8d66d0878..905d9f856b9 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs @@ -59,7 +59,8 @@ public override void VisitOutputDeclarationSyntax(OutputDeclarationSyntax syntax // Does the output name contain 'password' (suggesting it contains an actual password)? if (syntax.Name.IdentifierName.ToLowerInvariant().Contains("password")) { - this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, syntax.Name.IdentifierName)); + string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsOutputName, syntax.Name.IdentifierName); + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); } var visitor = new OutputValueVisitor(this.parent, diagnostics, model); @@ -97,7 +98,8 @@ public override void VisitVariableAccessSyntax(VariableAccessSyntax syntax) { if (param.IsSecure()) { - this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, param.Name)); + string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsSecureParam, syntax.Name.IdentifierName); + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); } } @@ -113,7 +115,8 @@ public override void VisitFunctionCallSyntax(FunctionCallSyntax syntax) if (syntax.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) { - this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, syntax.Name.IdentifierName)); + string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsFunction, syntax.Name.IdentifierName); + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); } base.VisitFunctionCallSyntax(syntax); @@ -128,7 +131,8 @@ public override void VisitInstanceFunctionCallSyntax(InstanceFunctionCallSyntax if (syntax.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) { - this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, syntax.Name.IdentifierName)); + string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsFunction, syntax.Name.IdentifierName); + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); } base.VisitInstanceFunctionCallSyntax(syntax); diff --git a/src/Bicep.Core/CoreResources.Designer.cs b/src/Bicep.Core/CoreResources.Designer.cs index dce98653a8a..7f08eea958b 100644 --- a/src/Bicep.Core/CoreResources.Designer.cs +++ b/src/Bicep.Core/CoreResources.Designer.cs @@ -77,6 +77,24 @@ internal static string OutputsShouldNotContainSecretsMessageFormat { } } + internal static string OutputsShouldNotContainSecretsOutputName { + get { + return ResourceManager.GetString("OutputsShouldNotContainSecretsOutputName", resourceCulture); + } + } + + internal static string OutputsShouldNotContainSecretsSecureParam { + get { + return ResourceManager.GetString("OutputsShouldNotContainSecretsSecureParam", resourceCulture); + } + } + + internal static string OutputsShouldNotContainSecretsFunction { + get { + return ResourceManager.GetString("OutputsShouldNotContainSecretsFunction", resourceCulture); + } + } + internal static string EnvironmentUrlHardcodedRuleDescription { get { return ResourceManager.GetString("EnvironmentUrlHardcodedRuleDescription", resourceCulture); diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 9b74a22ec6d..57b5e4ebe81 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -131,9 +131,21 @@ Don't include secrets in an output. - {0} Found: '{1}' + {0} Found: {1} {0} is the rule description as a sentence ending with a period, {1} is the name of what was found, e.g. 'myParameter' + + output name '{0}' suggests a secret + {0} name of a programmatic output + + + secure parameter '{0}' + {0} name of a programmatic parameter + + + function '{0}' + {0} name of a programmatic function + Environment URLs should not be hardcoded. Use the environment() function to ensure compatibility across clouds. From 965792781020c8483bf695b9d138c0f1976acd3c Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Mon, 4 Oct 2021 12:03:57 -0700 Subject: [PATCH 03/23] Fix help uris --- .../Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs | 2 +- .../Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs | 2 +- .../Analyzers/Linter/Rules/NoUnusedParametersRule.cs | 2 +- .../Analyzers/Linter/Rules/NoUnusedVariablesRule.cs | 2 +- .../Linter/Rules/OutputsShouldNotContainSecretsRule.cs | 4 ++-- .../Analyzers/Linter/Rules/PreferInterpolationRule.cs | 2 +- .../Analyzers/Linter/Rules/SecureParameterDefaultRule.cs | 2 +- .../Analyzers/Linter/Rules/SimplifyInterpolationRule.cs | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs index 95899a9cb5f..a4845d8595a 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/AdminUsernameShouldNotBeLiteralRule.cs @@ -20,7 +20,7 @@ public sealed class AdminUsernameShouldNotBeLiteralRule : LinterRuleBase public AdminUsernameShouldNotBeLiteralRule() : base( code: Code, description: CoreResources.AdminUsernameShouldNotBeLiteralRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}") + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}") ) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs index a86158891bb..1ffde553ed0 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/NoHardcodedEnvironmentUrlsRule.cs @@ -29,7 +29,7 @@ public sealed class NoHardcodedEnvironmentUrlsRule : LinterRuleBase public NoHardcodedEnvironmentUrlsRule() : base( code: Code, description: CoreResources.EnvironmentUrlHardcodedRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}")) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs index edc38543239..ac922849f3e 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedParametersRule.cs @@ -17,7 +17,7 @@ public sealed class NoUnusedParametersRule : LinterRuleBase public NoUnusedParametersRule() : base( code: Code, description: CoreResources.ParameterMustBeUsedRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}"), + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}"), diagnosticLabel: Diagnostics.DiagnosticLabel.Unnecessary) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs index 80d42ecdf27..e1480b0c0aa 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/NoUnusedVariablesRule.cs @@ -17,7 +17,7 @@ public sealed class NoUnusedVariablesRule : LinterRuleBase public NoUnusedVariablesRule() : base( code: Code, description: CoreResources.UnusedVariableRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}"), + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}"), diagnosticLabel: Diagnostics.DiagnosticLabel.Unnecessary) { } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs index 905d9f856b9..1a8ae071c65 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs @@ -21,7 +21,7 @@ public sealed class OutputsShouldNotContainSecretsRule : LinterRuleBase public OutputsShouldNotContainSecretsRule() : base( code: Code, description: CoreResources.OutputsShouldNotContainSecretsRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}") + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}") ) { } @@ -99,7 +99,7 @@ public override void VisitVariableAccessSyntax(VariableAccessSyntax syntax) if (param.IsSecure()) { string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsSecureParam, syntax.Name.IdentifierName); - this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Name.Span, foundMessage)); } } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs index 6e7ff98e1da..b108570e0b6 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/PreferInterpolationRule.cs @@ -21,7 +21,7 @@ public sealed class PreferInterpolationRule : LinterRuleBase public PreferInterpolationRule() : base( code: Code, description: CoreResources.InterpolateNotConcatRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}")) { } public override IEnumerable AnalyzeInternal(SemanticModel model) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs index cbfdcfd9f2e..733ee749a10 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/SecureParameterDefaultRule.cs @@ -18,7 +18,7 @@ public sealed class SecureParameterDefaultRule : LinterRuleBase public SecureParameterDefaultRule() : base( code: Code, description: CoreResources.SecureParameterDefaultRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}")) { } override public IEnumerable AnalyzeInternal(SemanticModel model) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs index 8fa691f99de..1ed3d2a6fad 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/SimplifyInterpolationRule.cs @@ -20,7 +20,7 @@ public sealed class SimplifyInterpolationRule : LinterRuleBase public SimplifyInterpolationRule() : base( code: Code, description: CoreResources.SimplifyInterpolationRuleDescription, - docUri: new Uri($"https://aka.ms/bicep/linter/${Code}")) + docUri: new Uri($"https://aka.ms/bicep/linter/{Code}")) { } public override IEnumerable AnalyzeInternal(SemanticModel model) From af627c4ad8818de8c637d947b3afc9205f36cac6 Mon Sep 17 00:00:00 2001 From: Bicep Automation Date: Mon, 4 Oct 2021 19:13:05 +0000 Subject: [PATCH 04/23] Update test baselines --- src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep index 578d2985d81..2a11b53bec7 100644 --- a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep +++ b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep @@ -60,6 +60,7 @@ output expressionBasedIndexer string = { var secondaryKeyIntermediateVar = listKeys(resourceId('Mock.RP/type', 'steve'), '2020-01-01').secondaryKey output primaryKey string = listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01').primaryKey +//@[27:86) [outputs-should-not-contain-secrets (Warning)] Don't include secrets in an output. Found: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| output secondaryKey string = secondaryKeyIntermediateVar var varWithOverlappingOutput = 'hello' From 2bcfcf4fb649771af1b421b183ab4c95cd3de2cd Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Tue, 5 Oct 2021 19:15:45 -0700 Subject: [PATCH 05/23] fix tests --- src/Bicep.Core.IntegrationTests/ScenarioTests.cs | 7 ++++--- .../Scenarios/ResourceListFunctionTests.cs | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs index 8f853c4841d..c437c3bce61 100644 --- a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs +++ b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs @@ -43,7 +43,7 @@ string randomString() { const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; return new string(Enumerable.Repeat(chars, generateRandomInt()) - .Select(s => s[generateRandomInt(0, s.Length-1)]).ToArray()); + .Select(s => s[generateRandomInt(0, s.Length - 1)]).ToArray()); } var file = "param adminuser string\nvar adminstring = 'xyx ${adminuser} 123'\n"; @@ -164,7 +164,8 @@ param serverFarmId string } "); - result.Should().NotHaveAnyDiagnostics(); + // Ignore outputs-should-not-contain-secrets warnings which are expected + result.Diagnostics.Where(d => d.Code != "outputs-should-not-contain-secrets").Should().BeEmpty(); result.Template.Should().HaveValueAtPath("$.outputs['config'].value", "[list(format('{0}/config/appsettings', resourceId('Microsoft.Web/sites', parameters('functionApp').name)), '2020-06-01')]"); } @@ -2590,7 +2591,7 @@ 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() diff --git a/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs b/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs index c8fb96c2f20..705bc5c6738 100644 --- a/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs +++ b/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Linq; using Bicep.Core.Diagnostics; using Bicep.Core.UnitTests.Assertions; using Bicep.Core.UnitTests.Utils; @@ -32,7 +33,8 @@ public void List_wildcard_function_on_resource_references() }) "); - result.Should().NotHaveAnyDiagnostics(); + // Ignore outputs-should-not-contain-secrets warnings which are expected + result.Diagnostics.Where(d => d.Code != "outputs-should-not-contain-secrets").Should().BeEmpty(); result.Template.Should().HaveValueAtPath("$.outputs['pkStandard'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethod'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethodVersionOverride'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2021-01-01').keys[0].value]"); @@ -56,7 +58,8 @@ public void List_wildcard_function_on_cross_scope_resource_references() }) "); - result.Should().NotHaveAnyDiagnostics(); + // Ignore outputs-should-not-contain-secrets warnings which are expected + result.Diagnostics.Where(d => d.Code != "outputs-should-not-contain-secrets").Should().BeEmpty(); result.Template.Should().HaveValueAtPath("$.outputs['pkStandard'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethod'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethodVersionOverride'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2021-01-01').keys[0].value]"); From be052d8a4e7f48292e132996f7416b8f4f39627e Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Tue, 12 Oct 2021 18:06:25 -0700 Subject: [PATCH 06/23] Fix PR comment --- .../LinterRuleTests/LinterRuleTestsBase.cs | 33 +++++++----- ...OutputsShouldNotContainSecretsRuleTests.cs | 51 +++++++++++++++++++ .../OutputsShouldNotContainSecretsRule.cs | 33 +++++++++--- 3 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs index 17d016b1de3..9582408a337 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs @@ -54,23 +54,28 @@ protected void AssertLinterRuleDiagnostics(string ruleCode, string bicepText, On private void RunWithDiagnosticAnnotations(string bicepText, Func filterFunc, OnCompileErrors onCompileErrors, Action> assertAction) { - var result = CompilationHelper.Compile(bicepText); - result.Should().NotHaveDiagnosticsWithCodes(new[] { LinterAnalyzer.FailedRuleCode }, "There should never be linter FailedRuleCode errors"); - - if (onCompileErrors == OnCompileErrors.Fail) + using (AssertionScope scope = new AssertionScope()) { - var compileErrors = result.Diagnostics.Where(d => d.Level == DiagnosticLevel.Error); + scope.AddReportable("bicep code", bicepText); + + var result = CompilationHelper.Compile(bicepText); + result.Should().NotHaveDiagnosticsWithCodes(new[] { LinterAnalyzer.FailedRuleCode }, "There should never be linter FailedRuleCode errors"); + + if (onCompileErrors == OnCompileErrors.Fail) + { + var compileErrors = result.Diagnostics.Where(d => d.Level == DiagnosticLevel.Error); + DiagnosticAssertions.DoWithDiagnosticAnnotations( + result.Compilation.SourceFileGrouping.EntryPoint, + compileErrors, + diags => diags.Should().HaveCount(0)); + } + + IDiagnostic[] diagnosticsMatchingCode = result.Diagnostics.Where(filterFunc).ToArray(); DiagnosticAssertions.DoWithDiagnosticAnnotations( - result.Compilation.SourceFileGrouping.EntryPoint, - compileErrors, - diags => diags.Should().HaveCount(0)); + result.Compilation.SourceFileGrouping.EntryPoint, + result.Diagnostics.Where(filterFunc), + assertAction); } - - IDiagnostic[] diagnosticsMatchingCode = result.Diagnostics.Where(filterFunc).ToArray(); - DiagnosticAssertions.DoWithDiagnosticAnnotations( - result.Compilation.SourceFileGrouping.EntryPoint, - result.Diagnostics.Where(filterFunc), - assertAction); } } } diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs index e09c02684dc..7a719111246 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs @@ -107,12 +107,43 @@ param storageName string ", "Don't include secrets in an output. Found: function 'listAnything'" )] + [DataRow(@" + param storageName string + + var v = {} + + output badResult object = { + value: v.listAnything().keys[0].value // storage is not a resource, so no failure + } + " + )] [DataTestMethod] public void If_ListFunctionInOutput_AsResourceMethod_ShouldFail(string text, params string[] expectedMessages) { CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); } + [DataRow(@" + param storageName string + + resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName + } + + var storage = stg + + output badResult object = { + value: storage.listAnything().keys[0].value + } + ", + "Don't include secrets in an output. Found: function 'listAnything'" + )] + [DataTestMethod] + public void If_ListFunctionInOutput_AsResourceMethod_ThroughVariable_ShouldFail(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); + } + [DataRow(@" param storageName string @@ -147,6 +178,26 @@ public void If_ListFunctionInOutput_AsStandaloneFunction_ShouldFail(string text, CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); } + [DataRow(@" + param storageName string + + resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { + name: storageName + } + + output badResult object = az.listAnything(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01') + ", + // TTK output: + // [-] Outputs Must Not Contain Secrets(6 ms) + // Output contains secret: badResult + "Don't include secrets in an output. Found: function 'listAnything'" + )] + [DataTestMethod] + public void If_ListFunctionInOutput_AsAzInstanceFunction_ShouldFail(string text, params string[] expectedMessages) + { + CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); + } + [DataRow(@" output badResultPassword string = 'hello' ", diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs index 1a8ae071c65..cc41ae943a7 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs @@ -5,6 +5,7 @@ using Bicep.Core.Diagnostics; using Bicep.Core.Parsing; using Bicep.Core.Semantics; +using Bicep.Core.Semantics.Namespaces; using Bicep.Core.Syntax; using Bicep.Core.TypeSystem; using System; @@ -124,15 +125,33 @@ public override void VisitFunctionCallSyntax(FunctionCallSyntax syntax) public override void VisitInstanceFunctionCallSyntax(InstanceFunctionCallSyntax syntax) { - // Look for usage of list*() member function, e.g.: - // - // output badResult object = stg.listKeys().keys[0].value - // - if (syntax.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) { - string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsFunction, syntax.Name.IdentifierName); - this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); + bool isFailure = false; + + Symbol? baseSymbol = model.GetSymbolInfo(syntax.BaseExpression); + if (baseSymbol is ResourceSymbol resource) + { + // It's a usage of a list*() member function for a resource value, e.g.: + // + // output badResult object = stg.listKeys().keys[0].value + // + isFailure = true; + } + else if (baseSymbol is BuiltInNamespaceSymbol namespaceType) + { + // It's a usage of a built-in list*() function as a member of the built-in "az" module, e.g.: + // + // output badResult object = az.listKeys(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01') + // + isFailure = true; + } + + if (isFailure) + { + string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsFunction, syntax.Name.IdentifierName); + this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); + } } base.VisitInstanceFunctionCallSyntax(syntax); From 1aac4ca54038fa825417d1175bb9689121956968 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Tue, 12 Oct 2021 18:21:55 -0700 Subject: [PATCH 07/23] Skip test blocked by 4833 --- .../LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs index 7a719111246..676be812e0f 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs @@ -123,6 +123,7 @@ public void If_ListFunctionInOutput_AsResourceMethod_ShouldFail(string text, par CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages); } + [Ignore("TODO: blocked by https://github.com/Azure/bicep/issues/4833")] [DataRow(@" param storageName string From 0e291822b40dfb655e99066b16c71c258d48b5cf Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Tue, 12 Oct 2021 18:27:43 -0700 Subject: [PATCH 08/23] add "possible" to error --- ...OutputsShouldNotContainSecretsRuleTests.cs | 26 +++++++++---------- src/Bicep.Core/CoreResources.resx | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs index 676be812e0f..7515ab8214e 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs @@ -24,7 +24,7 @@ param secureParam string output badResult string = 'this is the value ${secureParam}' ", - "Don't include secrets in an output. Found: secure parameter 'secureParam'" + "Don't include secrets in an output. Found possible secret: secure parameter 'secureParam'" )] [DataRow(@" @secure() @@ -36,7 +36,7 @@ param secureParam string } } ", - "Don't include secrets in an output. Found: secure parameter 'secureParam'" + "Don't include secrets in an output. Found possible secret: secure parameter 'secureParam'" )] [DataRow(@" @secure() @@ -49,7 +49,7 @@ param secureParam string // TTK output: // [-] Outputs Must Not Contain Secrets // Output contains secureObject parameter: badResult - "Don't include secrets in an output. Found: secure parameter 'secureParam'" + "Don't include secrets in an output. Found possible secret: secure parameter 'secureParam'" )] [DataTestMethod] public void If_OutputReferencesSecureParam_ShouldFail(string text, params string[] expectedMessages) @@ -92,7 +92,7 @@ param storageName string value: stg.listKeys().keys[0].value } ", - "Don't include secrets in an output. Found: function 'listKeys'" + "Don't include secrets in an output. Found possible secret: function 'listKeys'" )] [DataRow(@" param storageName string @@ -105,7 +105,7 @@ param storageName string value: stg.listAnything().keys[0].value } ", - "Don't include secrets in an output. Found: function 'listAnything'" + "Don't include secrets in an output. Found possible secret: function 'listAnything'" )] [DataRow(@" param storageName string @@ -137,7 +137,7 @@ param storageName string value: storage.listAnything().keys[0].value } ", - "Don't include secrets in an output. Found: function 'listAnything'" + "Don't include secrets in an output. Found possible secret: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsResourceMethod_ThroughVariable_ShouldFail(string text, params string[] expectedMessages) @@ -157,7 +157,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found: function 'listKeys'" + "Don't include secrets in an output. Found possible secret: function 'listKeys'" )] [DataRow(@" param storageName string @@ -171,7 +171,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found: function 'listAnything'" + "Don't include secrets in an output. Found possible secret: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsStandaloneFunction_ShouldFail(string text, params string[] expectedMessages) @@ -191,7 +191,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found: function 'listAnything'" + "Don't include secrets in an output. Found possible secret: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsAzInstanceFunction_ShouldFail(string text, params string[] expectedMessages) @@ -205,22 +205,22 @@ public void If_ListFunctionInOutput_AsAzInstanceFunction_ShouldFail(string text, // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output name suggests secret: badResultPassword - "Don't include secrets in an output. Found: output name 'badResultPassword' suggests a secret" + "Don't include secrets in an output. Found possible secret: output name 'badResultPassword' suggests a secret" )] [DataRow(@" output possiblepassword string = 'hello' ", - "Don't include secrets in an output. Found: output name 'possiblepassword' suggests a secret" + "Don't include secrets in an output. Found possible secret: output name 'possiblepassword' suggests a secret" )] [DataRow(@" output password string = 'hello' ", - "Don't include secrets in an output. Found: output name 'password' suggests a secret" + "Don't include secrets in an output. Found possible secret: output name 'password' suggests a secret" )] [DataRow(@" output passwordNumber1 string = 'hello' ", - "Don't include secrets in an output. Found: output name 'passwordNumber1' suggests a secret" + "Don't include secrets in an output. Found possible secret: output name 'passwordNumber1' suggests a secret" )] [DataTestMethod] public void If_OutputNameLooksLikePassword_ShouldFail(string text, params string[] expectedMessages) diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 57b5e4ebe81..8193357a086 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -131,7 +131,7 @@ Don't include secrets in an output. - {0} Found: {1} + {0} Found possible secret: {1} {0} is the rule description as a sentence ending with a period, {1} is the name of what was found, e.g. 'myParameter' From 3b27ea09dab81f125854785961d49c8277a63c55 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Tue, 12 Oct 2021 18:48:33 -0700 Subject: [PATCH 09/23] PR fix --- .../Linter/Rules/OutputsShouldNotContainSecretsRule.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs index cc41ae943a7..20f8bb50124 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs @@ -114,7 +114,8 @@ public override void VisitFunctionCallSyntax(FunctionCallSyntax syntax) // output badResult object = listKeys(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01') // - if (syntax.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) + if (SemanticModelHelper.TryGetFunctionInNamespace(model, AzNamespaceType.BuiltInName, syntax) is FunctionCallSyntaxBase listFunction + && listFunction.Name.IdentifierName.StartsWithOrdinalInsensitively(ListFunctionPrefix)) { string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsFunction, syntax.Name.IdentifierName); this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); @@ -138,7 +139,7 @@ public override void VisitInstanceFunctionCallSyntax(InstanceFunctionCallSyntax // isFailure = true; } - else if (baseSymbol is BuiltInNamespaceSymbol namespaceType) + else if (baseSymbol is BuiltInNamespaceSymbol) { // It's a usage of a built-in list*() function as a member of the built-in "az" module, e.g.: // From c1772b797b294a71460717535ea130ee07022df0 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Tue, 12 Oct 2021 20:51:03 -0700 Subject: [PATCH 10/23] update baseline --- .../Files/Outputs_CRLF/main.diagnostics.bicep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep index 2a11b53bec7..6599b071dd7 100644 --- a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep +++ b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep @@ -60,7 +60,7 @@ output expressionBasedIndexer string = { var secondaryKeyIntermediateVar = listKeys(resourceId('Mock.RP/type', 'steve'), '2020-01-01').secondaryKey output primaryKey string = listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01').primaryKey -//@[27:86) [outputs-should-not-contain-secrets (Warning)] Don't include secrets in an output. Found: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| +//@[27:86) [outputs-should-not-contain-secrets (Warning)] Don't include secrets in an output. Found possible secret: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| output secondaryKey string = secondaryKeyIntermediateVar var varWithOverlappingOutput = 'hello' From 67fc91bee9415ebaa31ce7be8dbee3b651ae7426 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 19 Nov 2021 11:14:54 -0800 Subject: [PATCH 11/23] accidental change --- src/Bicep.Core/CoreResources.Designer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bicep.Core/CoreResources.Designer.cs b/src/Bicep.Core/CoreResources.Designer.cs index 03a46c2d28b..c2b315d1f27 100644 --- a/src/Bicep.Core/CoreResources.Designer.cs +++ b/src/Bicep.Core/CoreResources.Designer.cs @@ -191,7 +191,7 @@ internal static string SimplifyInterpolationRuleDescription { } } - internal static string UnusedVariableRuleDescriptioni { + internal static string UnusedVariableRuleDescription { get { return ResourceManager.GetString("UnusedVariableRuleDescription", resourceCulture); } From 6d9e9f23b52510467a1a6f72e4da20d0f4eef337 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 19 Nov 2021 11:22:58 -0800 Subject: [PATCH 12/23] fix resx/designer formatting --- src/Bicep.Core/CoreResources.Designer.cs | 74 ++++---- src/Bicep.Core/CoreResources.resx | 206 +++++++++++------------ 2 files changed, 140 insertions(+), 140 deletions(-) diff --git a/src/Bicep.Core/CoreResources.Designer.cs b/src/Bicep.Core/CoreResources.Designer.cs index c2b315d1f27..e6049453a0f 100644 --- a/src/Bicep.Core/CoreResources.Designer.cs +++ b/src/Bicep.Core/CoreResources.Designer.cs @@ -11,21 +11,21 @@ namespace Bicep.Core { using System; using System.Reflection; - - + + [System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] [System.Diagnostics.DebuggerNonUserCodeAttribute()] [System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class CoreResources { - + private static System.Resources.ResourceManager resourceMan; - + private static System.Globalization.CultureInfo resourceCulture; - + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] internal CoreResources() { } - + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] internal static System.Resources.ResourceManager ResourceManager { get { @@ -36,7 +36,7 @@ internal static System.Resources.ResourceManager ResourceManager { return resourceMan; } } - + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] internal static System.Globalization.CultureInfo Culture { get { @@ -46,181 +46,181 @@ internal static System.Globalization.CultureInfo Culture { resourceCulture = value; } } - + internal static string BicepConfigCustomSettingsFoundFormatMessage { get { return ResourceManager.GetString("BicepConfigCustomSettingsFoundFormatMessage", resourceCulture); } } - + internal static string BicepConfigNoCustomSettingsMessage { get { return ResourceManager.GetString("BicepConfigNoCustomSettingsMessage", resourceCulture); } } - + internal static string AdminUsernameShouldNotBeLiteralRuleDescription { get { return ResourceManager.GetString("AdminUsernameShouldNotBeLiteralRuleDescription", resourceCulture); } } - + internal static string OutputsShouldNotContainSecretsRuleDescription { get { return ResourceManager.GetString("OutputsShouldNotContainSecretsRuleDescription", resourceCulture); } } - + internal static string OutputsShouldNotContainSecretsMessageFormat { get { return ResourceManager.GetString("OutputsShouldNotContainSecretsMessageFormat", resourceCulture); } } - + internal static string OutputsShouldNotContainSecretsOutputName { get { return ResourceManager.GetString("OutputsShouldNotContainSecretsOutputName", resourceCulture); } } - + internal static string OutputsShouldNotContainSecretsSecureParam { get { return ResourceManager.GetString("OutputsShouldNotContainSecretsSecureParam", resourceCulture); } } - + internal static string OutputsShouldNotContainSecretsFunction { get { return ResourceManager.GetString("OutputsShouldNotContainSecretsFunction", resourceCulture); } } - + internal static string EnvironmentUrlHardcodedRuleDescription { get { return ResourceManager.GetString("EnvironmentUrlHardcodedRuleDescription", resourceCulture); } } - + internal static string InterpolateNotConcatFixTitle { get { return ResourceManager.GetString("InterpolateNotConcatFixTitle", resourceCulture); } } - + internal static string InterpolateNotConcatRuleDescription { get { return ResourceManager.GetString("InterpolateNotConcatRuleDescription", resourceCulture); } } - + internal static string LinterDisabledFormatMessage { get { return ResourceManager.GetString("LinterDisabledFormatMessage", resourceCulture); } } - + internal static string LinterRuleExceptionMessageFormat { get { return ResourceManager.GetString("LinterRuleExceptionMessageFormat", resourceCulture); } } - + internal static string LocationSetByParameterRuleDescription { get { return ResourceManager.GetString("LocationSetByParameterRuleDescription", resourceCulture); } } - + internal static string NoUnnecessaryDependsOnRuleDescription { get { return ResourceManager.GetString("NoUnnecessaryDependsOnRuleDescription", resourceCulture); } } - + internal static string NoUnnecessaryDependsOnRuleMessage { get { return ResourceManager.GetString("NoUnnecessaryDependsOnRuleMessage", resourceCulture); } } - + internal static string ParameterMustBeUsedRuleDescription { get { return ResourceManager.GetString("ParameterMustBeUsedRuleDescription", resourceCulture); } } - + internal static string ParameterMustBeUsedRuleMessageFormat { get { return ResourceManager.GetString("ParameterMustBeUsedRuleMessageFormat", resourceCulture); } } - + internal static string PossibleSecretMessageFunction { get { return ResourceManager.GetString("PossibleSecretMessageFunction", resourceCulture); } } - + internal static string PossibleSecretMessageSecureParam { get { return ResourceManager.GetString("PossibleSecretMessageSecureParam", resourceCulture); } } - + internal static string SecureParameterDefaultFixTitle { get { return ResourceManager.GetString("SecureParameterDefaultFixTitle", resourceCulture); } } - + internal static string SecureParameterDefaultRuleDescription { get { return ResourceManager.GetString("SecureParameterDefaultRuleDescription", resourceCulture); } } - + internal static string SimplifyInterpolationFixTitle { get { return ResourceManager.GetString("SimplifyInterpolationFixTitle", resourceCulture); } } - + internal static string SimplifyInterpolationRuleDescription { get { return ResourceManager.GetString("SimplifyInterpolationRuleDescription", resourceCulture); } } - + internal static string UnusedVariableRuleDescription { get { return ResourceManager.GetString("UnusedVariableRuleDescription", resourceCulture); } } - + internal static string UnusedVariableRuleMessageFormat { get { return ResourceManager.GetString("UnusedVariableRuleMessageFormat", resourceCulture); } } - + internal static string UseProtectedSettingsForCommandToExecuteSecretsRuleDescription { get { return ResourceManager.GetString("UseProtectedSettingsForCommandToExecuteSecretsRuleDescription", resourceCulture); } } - + internal static string UseProtectedSettingsForCommandToExecuteSecretsRuleMessage { get { return ResourceManager.GetString("UseProtectedSettingsForCommandToExecuteSecretsRuleMessage", resourceCulture); } } - + internal static string UseStableVMImage { get { return ResourceManager.GetString("UseStableVMImage", resourceCulture); } } - + internal static string UseStableVMImageRuleFixMessageFormat { get { return ResourceManager.GetString("UseStableVMImageRuleFixMessageFormat", resourceCulture); diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index a7652531d59..94b684fa125 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -1,5 +1,5 @@ - - + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - text/microsoft-resx - - - 2.0 - - - System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - - System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - + --> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + Custom bicepconfig.json file found ({0}). message to indicate that a custom settings file was found {0} is the file location - + No bicepconfig.json found for configuration override. - + When setting an adminUserName property, don't use a literal value. - - + + Don't include secrets in an output. - - + + {0} Found possible secret: {1} {0} is the rule description as a sentence ending with a period, {1} is the name of what was found, e.g. 'myParameter' - - + + output name '{0}' suggests a secret {0} name of a programmatic output - - + + secure parameter '{0}' {0} name of a programmatic parameter - - + + function '{0}' {0} name of a programmatic function - - + + Environment URLs should not be hardcoded. Use the environment() function to ensure compatibility across clouds. - + Use string interpolation: {0}. {0} is a programmatic language expression - + Use string interpolation instead of the concat function. - + Linter is disabled in settings file located at {0} {0} is the location of the settings file - + Analyzer '{0}' encountered an unexpected exception. {1} string format to display error exception thrown in Linter Rule - + Resource location should be specified by a parameter with no default value or one that defaults to 'global' or resourceGroup().location. - - + + No unnecessary dependsOn. - - + + Remove unnecessary dependsOn entry '{0}'. Text for linter rule error. {0} is a programmatic expression - + No unused parameters. - + Parameter "{0}" is declared but never used. {0} is the paramenter name - - + + function '{0}' {0} is the function name - - + + secure parameter '{0}' {0} is the parameter name - + Remove insecure default value. - + Secure parameters should not have hardcoded defaults (except for empty or newGuid()). - + Remove unnecessary string interpolation. - + Remove unnecessary string interpolation. - + No unused variables. - + Variable "{0}" is declared but never used. {0} is the variable name - - + + Use protectedSettings for commandToExecute secrets - - + + Use protectedSettings for commandToExecute secrets. Found possible secret: {0} {0} is the description of what was found - + Virtual machines shouldn't use preview images. - + Virtual machines shouldn't use preview images. Use stable version in imageReference property "{0}". - + \ No newline at end of file From e32ca1e31d3a8bca04d2a4ae9836d6ba2f2ea91e Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 19 Nov 2021 12:40:29 -0800 Subject: [PATCH 13/23] fix spaces in resx --- src/Bicep.Core/CoreResources.resx | 244 +++++++++++++++--------------- 1 file changed, 122 insertions(+), 122 deletions(-) diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 94b684fa125..58c0a5a2a88 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -1,6 +1,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - text/microsoft-resx - - - 2.0 - - - System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - - System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - - + --> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + Custom bicepconfig.json file found ({0}). message to indicate that a custom settings file was found {0} is the file location - - + + No bicepconfig.json found for configuration override. - - + + When setting an adminUserName property, don't use a literal value. - - + + Don't include secrets in an output. - - + + {0} Found possible secret: {1} {0} is the rule description as a sentence ending with a period, {1} is the name of what was found, e.g. 'myParameter' - - + + output name '{0}' suggests a secret {0} name of a programmatic output - - + + secure parameter '{0}' {0} name of a programmatic parameter - - + + function '{0}' {0} name of a programmatic function - - + + Environment URLs should not be hardcoded. Use the environment() function to ensure compatibility across clouds. - - + + Use string interpolation: {0}. {0} is a programmatic language expression - - + + Use string interpolation instead of the concat function. - - + + Linter is disabled in settings file located at {0} {0} is the location of the settings file - - + + Analyzer '{0}' encountered an unexpected exception. {1} string format to display error exception thrown in Linter Rule - - + + Resource location should be specified by a parameter with no default value or one that defaults to 'global' or resourceGroup().location. - - + + No unnecessary dependsOn. - - + + Remove unnecessary dependsOn entry '{0}'. Text for linter rule error. {0} is a programmatic expression - - + + No unused parameters. - - + + Parameter "{0}" is declared but never used. {0} is the paramenter name - - + + function '{0}' {0} is the function name - - + + secure parameter '{0}' {0} is the parameter name - - + + Remove insecure default value. - - + + Secure parameters should not have hardcoded defaults (except for empty or newGuid()). - - + + Remove unnecessary string interpolation. - - + + Remove unnecessary string interpolation. - - + + No unused variables. - - + + Variable "{0}" is declared but never used. {0} is the variable name - - + + Use protectedSettings for commandToExecute secrets - - + + Use protectedSettings for commandToExecute secrets. Found possible secret: {0} {0} is the description of what was found - - + + Virtual machines shouldn't use preview images. - - + + Virtual machines shouldn't use preview images. Use stable version in imageReference property "{0}". - + \ No newline at end of file From 03f3527a61061f1d98f407efddcf3d50d1d64189 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 19 Nov 2021 13:14:43 -0800 Subject: [PATCH 14/23] Remove doc file --- .../outputs-should-not-contain-secrets.md | 44 ------------------- 1 file changed, 44 deletions(-) delete mode 100644 docs/linter-rules/outputs-should-not-contain-secrets.md diff --git a/docs/linter-rules/outputs-should-not-contain-secrets.md b/docs/linter-rules/outputs-should-not-contain-secrets.md deleted file mode 100644 index c437b97791d..00000000000 --- a/docs/linter-rules/outputs-should-not-contain-secrets.md +++ /dev/null @@ -1,44 +0,0 @@ -# Outputs should not contain secrets - -**Code**: outputs-should-not-contain-secrets - -**Description**: Don't include any values in an output that could potentially expose secrets. For example, secure parameters of type secureString or secureObject, or list* functions such as listKeys. - -The output from a template is stored in the deployment history, so a malicious user could find that information. - -The following example fails because it includes a secure parameter in an output value. -```bicep -@secure() -param secureParam string - -output badResult string = 'this is the value ${secureParam}' -``` - -The following example fails because it uses a list* member function in an output. -```bicep -param storageName string -resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { - name: storageName -} - -output badResult object = { - value: stg.listKeys().keys[0].value -} -``` - -The following example fails because it uses a list* function in an output. -```bicep -param storageName string -resource stg 'Microsoft.Storage/storageAccounts@2021-04-01' existing = { - name: storageName -} - -output badResult object = { - value: listKeys(resourceId('Microsoft.Storage/storageAccounts', stg.name), '2021-02-01') -} -``` - -The following example fails because the output name contains 'password', indicating that it may contains a secret -```bicep -output accountPassword string = '...' -``` \ No newline at end of file From ba14ad9866d710b0f0a8376fb37418deef2d008e Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 3 Dec 2021 11:59:52 -0800 Subject: [PATCH 15/23] Update string --- .../Files/Outputs_CRLF/main.diagnostics.bicep | 2 +- .../Common/FindPossibleSecretsVisitorTests.cs | 2 +- ...OutputsShouldNotContainSecretsRuleTests.cs | 28 +++++++------ src/Bicep.Core/CoreResources.resx | 2 +- .../LangServerResources.Designer.cs | 42 ++++++------------- .../schemas/bicepconfig.schema.json | 2 +- 6 files changed, 32 insertions(+), 46 deletions(-) diff --git a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep index 6599b071dd7..a599a2ab50d 100644 --- a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep +++ b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep @@ -60,7 +60,7 @@ output expressionBasedIndexer string = { var secondaryKeyIntermediateVar = listKeys(resourceId('Mock.RP/type', 'steve'), '2020-01-01').secondaryKey output primaryKey string = listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01').primaryKey -//@[27:86) [outputs-should-not-contain-secrets (Warning)] Don't include secrets in an output. Found possible secret: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| +//@[27:86) [outputs-should-not-contain-secrets (Warning)] Outputs should not expose secrets. Found possible secret: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| output secondaryKey string = secondaryKeyIntermediateVar var varWithOverlappingOutput = 'hello' diff --git a/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs index ff8f3649d4b..bd2ff33272a 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs @@ -121,7 +121,7 @@ param storageName string value: storage.listAnything().keys[0].value } ", - "Don't include secrets in an output. function 'listAnything'" + "Outputs should not contain secrets. function 'listAnything'" )]*/ [DataRow( @" diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs index 7515ab8214e..18398f97935 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/OutputsShouldNotContainSecretsRuleTests.cs @@ -18,13 +18,15 @@ private void CompileAndTest(string text, OnCompileErrors onCompileErrors, string AssertLinterRuleDiagnostics(OutputsShouldNotContainSecretsRule.Code, text, expectedMessages, onCompileErrors); } + const string description = "Outputs should not contain secrets."; + [DataRow(@" // This is a failing example from the docs @secure() param secureParam string output badResult string = 'this is the value ${secureParam}' ", - "Don't include secrets in an output. Found possible secret: secure parameter 'secureParam'" + $"{description} Found possible secret: secure parameter 'secureParam'" )] [DataRow(@" @secure() @@ -36,7 +38,7 @@ param secureParam string } } ", - "Don't include secrets in an output. Found possible secret: secure parameter 'secureParam'" + $"{description} Found possible secret: secure parameter 'secureParam'" )] [DataRow(@" @secure() @@ -49,7 +51,7 @@ param secureParam string // TTK output: // [-] Outputs Must Not Contain Secrets // Output contains secureObject parameter: badResult - "Don't include secrets in an output. Found possible secret: secure parameter 'secureParam'" + $"{description} Found possible secret: secure parameter 'secureParam'" )] [DataTestMethod] public void If_OutputReferencesSecureParam_ShouldFail(string text, params string[] expectedMessages) @@ -92,7 +94,7 @@ param storageName string value: stg.listKeys().keys[0].value } ", - "Don't include secrets in an output. Found possible secret: function 'listKeys'" + $"{description} Found possible secret: function 'listKeys'" )] [DataRow(@" param storageName string @@ -105,7 +107,7 @@ param storageName string value: stg.listAnything().keys[0].value } ", - "Don't include secrets in an output. Found possible secret: function 'listAnything'" + $"{description} Found possible secret: function 'listAnything'" )] [DataRow(@" param storageName string @@ -137,7 +139,7 @@ param storageName string value: storage.listAnything().keys[0].value } ", - "Don't include secrets in an output. Found possible secret: function 'listAnything'" + $"{description} Found possible secret: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsResourceMethod_ThroughVariable_ShouldFail(string text, params string[] expectedMessages) @@ -157,7 +159,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found possible secret: function 'listKeys'" + $"{description} Found possible secret: function 'listKeys'" )] [DataRow(@" param storageName string @@ -171,7 +173,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found possible secret: function 'listAnything'" + $"{description} Found possible secret: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsStandaloneFunction_ShouldFail(string text, params string[] expectedMessages) @@ -191,7 +193,7 @@ param storageName string // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output contains secret: badResult - "Don't include secrets in an output. Found possible secret: function 'listAnything'" + $"{description} Found possible secret: function 'listAnything'" )] [DataTestMethod] public void If_ListFunctionInOutput_AsAzInstanceFunction_ShouldFail(string text, params string[] expectedMessages) @@ -205,22 +207,22 @@ public void If_ListFunctionInOutput_AsAzInstanceFunction_ShouldFail(string text, // TTK output: // [-] Outputs Must Not Contain Secrets(6 ms) // Output name suggests secret: badResultPassword - "Don't include secrets in an output. Found possible secret: output name 'badResultPassword' suggests a secret" + $"{description} Found possible secret: output name 'badResultPassword' suggests a secret" )] [DataRow(@" output possiblepassword string = 'hello' ", - "Don't include secrets in an output. Found possible secret: output name 'possiblepassword' suggests a secret" + $"{description} Found possible secret: output name 'possiblepassword' suggests a secret" )] [DataRow(@" output password string = 'hello' ", - "Don't include secrets in an output. Found possible secret: output name 'password' suggests a secret" + $"{description} Found possible secret: output name 'password' suggests a secret" )] [DataRow(@" output passwordNumber1 string = 'hello' ", - "Don't include secrets in an output. Found possible secret: output name 'passwordNumber1' suggests a secret" + $"{description} Found possible secret: output name 'passwordNumber1' suggests a secret" )] [DataTestMethod] public void If_OutputNameLooksLikePassword_ShouldFail(string text, params string[] expectedMessages) diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 204b334d376..66b88566fc5 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -128,7 +128,7 @@ When setting an adminUserName property, don't use a literal value. - Don't include secrets in an output. + Outputs should not contain secrets. {0} Found possible secret: {1} diff --git a/src/Bicep.LangServer/LangServerResources.Designer.cs b/src/Bicep.LangServer/LangServerResources.Designer.cs index 66e77585093..d6f38edb68b 100644 --- a/src/Bicep.LangServer/LangServerResources.Designer.cs +++ b/src/Bicep.LangServer/LangServerResources.Designer.cs @@ -10,48 +10,35 @@ namespace Bicep.LanguageServer { using System; + using System.Reflection; - /// - /// A strongly-typed resource class, for looking up localized strings, etc. - /// - // This class was auto-generated by the StronglyTypedResourceBuilder - // class via a tool like ResGen or Visual Studio. - // To add or remove a member, edit your .ResX file then rerun ResGen - // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] - [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] - [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + [System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "4.0.0.0")] + [System.Diagnostics.DebuggerNonUserCodeAttribute()] + [System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class LangServerResources { - private static global::System.Resources.ResourceManager resourceMan; + private static System.Resources.ResourceManager resourceMan; - private static global::System.Globalization.CultureInfo resourceCulture; + private static System.Globalization.CultureInfo resourceCulture; - [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] + [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")] internal LangServerResources() { } - /// - /// Returns the cached ResourceManager instance used by this class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Resources.ResourceManager ResourceManager { + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] + internal static System.Resources.ResourceManager ResourceManager { get { - if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Bicep.LanguageServer.LangServerResources", typeof(LangServerResources).Assembly); + if (object.Equals(null, resourceMan)) { + System.Resources.ResourceManager temp = new System.Resources.ResourceManager("Bicep.LanguageServer.LangServerResources", typeof(LangServerResources).Assembly); resourceMan = temp; } return resourceMan; } } - /// - /// Overrides the current thread's CurrentUICulture property for all - /// resource lookups using this strongly typed resource class. - /// - [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)] - internal static global::System.Globalization.CultureInfo Culture { + [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Advanced)] + internal static System.Globalization.CultureInfo Culture { get { return resourceCulture; } @@ -60,9 +47,6 @@ internal LangServerResources() { } } - /// - /// Looks up a localized string similar to Disable {0}. - /// internal static string DisableDiagnostic { get { return ResourceManager.GetString("DisableDiagnostic", resourceCulture); diff --git a/src/vscode-bicep/schemas/bicepconfig.schema.json b/src/vscode-bicep/schemas/bicepconfig.schema.json index bec3251e039..36eb3924d27 100644 --- a/src/vscode-bicep/schemas/bicepconfig.schema.json +++ b/src/vscode-bicep/schemas/bicepconfig.schema.json @@ -336,7 +336,7 @@ "outputs-should-not-contain-secrets": { "allOf": [ { - "description": "Don't include secrets in an output. See https://aka.ms/bicep/linter/outputs-should-not-contain-secrets" + "description": "Outputs should not contain secrets. See https://aka.ms/bicep/linter/outputs-should-not-contain-secrets" }, { "$ref": "#/definitions/rule" From cd841805f919cfd08d1ba687d64d6f273970ee26 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Fri, 3 Dec 2021 12:51:47 -0800 Subject: [PATCH 16/23] fix baseline --- .../Files/Outputs_CRLF/main.diagnostics.bicep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep index a599a2ab50d..3a5d9a9eba4 100644 --- a/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep +++ b/src/Bicep.Core.Samples/Files/Outputs_CRLF/main.diagnostics.bicep @@ -60,7 +60,7 @@ output expressionBasedIndexer string = { var secondaryKeyIntermediateVar = listKeys(resourceId('Mock.RP/type', 'steve'), '2020-01-01').secondaryKey output primaryKey string = listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01').primaryKey -//@[27:86) [outputs-should-not-contain-secrets (Warning)] Outputs should not expose secrets. Found possible secret: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| +//@[27:86) [outputs-should-not-contain-secrets (Warning)] Outputs should not contain secrets. Found possible secret: function 'listKeys' (CodeDescription: bicep core(https://aka.ms/bicep/linter/outputs-should-not-contain-secrets)) |listKeys(resourceId('Mock.RP/type', 'nigel'), '2020-01-01')| output secondaryKey string = secondaryKeyIntermediateVar var varWithOverlappingOutput = 'hello' From 6247657d3f6996e0f6e84b3ac9bbf60588b95cad Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Mon, 6 Dec 2021 10:19:53 -0800 Subject: [PATCH 17/23] CR comments --- src/Bicep.Core.IntegrationTests/ScenarioTests.cs | 4 ++-- .../Scenarios/ResourceListFunctionTests.cs | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs index 4de49878c02..c2d658dfdd2 100644 --- a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs +++ b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs @@ -146,6 +146,7 @@ public void Test_Issue982() param functionApp object param serverFarmId string +#disable-next-line outputs-should-not-contain-secrets output config object = list(format('{0}/config/appsettings', functionAppResource.id), functionAppResource.apiVersion) resource functionAppResource 'Microsoft.Web/sites@2020-06-01' = { @@ -159,8 +160,7 @@ param serverFarmId string } "); - // Ignore outputs-should-not-contain-secrets warnings which are expected - result.Diagnostics.Where(d => d.Code != "outputs-should-not-contain-secrets").Should().BeEmpty(); + result.Diagnostics.Should().BeEmpty(); result.Template.Should().HaveValueAtPath("$.outputs['config'].value", "[list(format('{0}/config/appsettings', resourceId('Microsoft.Web/sites', parameters('functionApp').name)), '2020-06-01')]"); } diff --git a/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs b/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs index fab0f92a0c6..7b6ade3e12d 100644 --- a/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs +++ b/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs @@ -25,16 +25,19 @@ public void List_wildcard_function_on_resource_references() } } +#disable-next-line outputs-should-not-contain-secrets output pkStandard string = listKeys(stg.id, stg.apiVersion).keys[0].value +#disable-next-line outputs-should-not-contain-secrets output pkMethod string = stg.listKeys().keys[0].value +#disable-next-line outputs-should-not-contain-secrets output pkMethodVersionOverride string = stg.listKeys('2021-01-01').keys[0].value +#disable-next-line outputs-should-not-contain-secrets output pkMethodPayload string = stg.listKeys(stg.apiVersion, { key1: 'val1' }) "); - // Ignore outputs-should-not-contain-secrets warnings which are expected - result.Diagnostics.Where(d => d.Code != "outputs-should-not-contain-secrets").Should().BeEmpty(); + result.Diagnostics.Should().BeEmpty(); result.Template.Should().HaveValueAtPath("$.outputs['pkStandard'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethod'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethodVersionOverride'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2021-01-01').keys[0].value]"); @@ -50,16 +53,19 @@ public void List_wildcard_function_on_cross_scope_resource_references() name: 'testacc' } +#disable-next-line outputs-should-not-contain-secrets output pkStandard string = listKeys(stg.id, stg.apiVersion).keys[0].value +#disable-next-line outputs-should-not-contain-secrets output pkMethod string = stg.listKeys().keys[0].value +#disable-next-line outputs-should-not-contain-secrets output pkMethodVersionOverride string = stg.listKeys('2021-01-01').keys[0].value +#disable-next-line outputs-should-not-contain-secrets output pkMethodPayload string = stg.listKeys(stg.apiVersion, { key1: 'val1' }) "); - // Ignore outputs-should-not-contain-secrets warnings which are expected - result.Diagnostics.Where(d => d.Code != "outputs-should-not-contain-secrets").Should().BeEmpty(); + result.Diagnostics.Should().BeEmpty(); result.Template.Should().HaveValueAtPath("$.outputs['pkStandard'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethod'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethodVersionOverride'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2021-01-01').keys[0].value]"); From 09126a22d92a82dd0a2302c0ccee0d5f1b659d22 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Mon, 6 Dec 2021 10:24:57 -0800 Subject: [PATCH 18/23] clean-up --- src/Bicep.Core.IntegrationTests/ScenarioTests.cs | 2 +- .../Scenarios/ResourceListFunctionTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs index c2d658dfdd2..b8a6bbcbac9 100644 --- a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs +++ b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs @@ -160,7 +160,7 @@ param serverFarmId string } "); - result.Diagnostics.Should().BeEmpty(); + result.Should().NotHaveAnyDiagnostics(); result.Template.Should().HaveValueAtPath("$.outputs['config'].value", "[list(format('{0}/config/appsettings', resourceId('Microsoft.Web/sites', parameters('functionApp').name)), '2020-06-01')]"); } diff --git a/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs b/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs index 7b6ade3e12d..b4c7317e88c 100644 --- a/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs +++ b/src/Bicep.Core.IntegrationTests/Scenarios/ResourceListFunctionTests.cs @@ -37,7 +37,7 @@ public void List_wildcard_function_on_resource_references() }) "); - result.Diagnostics.Should().BeEmpty(); + result.Should().NotHaveAnyDiagnostics(); result.Template.Should().HaveValueAtPath("$.outputs['pkStandard'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethod'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethodVersionOverride'].value", "[listKeys(resourceId('Microsoft.Storage/storageAccounts', 'testacc'), '2021-01-01').keys[0].value]"); @@ -65,7 +65,7 @@ public void List_wildcard_function_on_cross_scope_resource_references() }) "); - result.Diagnostics.Should().BeEmpty(); + result.Should().NotHaveAnyDiagnostics(); result.Template.Should().HaveValueAtPath("$.outputs['pkStandard'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethod'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2019-06-01').keys[0].value]"); result.Template.Should().HaveValueAtPath("$.outputs['pkMethodVersionOverride'].value", "[listKeys(extensionResourceId(format('/subscriptions/{0}/resourceGroups/{1}', subscription().subscriptionId, 'other'), 'Microsoft.Storage/storageAccounts', 'testacc'), '2021-01-01').keys[0].value]"); From 03caea3ffaafead21e09a47d9ce157bd01690a8b Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Wed, 8 Dec 2021 12:57:33 -0800 Subject: [PATCH 19/23] CR fix --- .../Linter/Rules/OutputsShouldNotContainSecretsRule.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs index 20f8bb50124..3afa116b24d 100644 --- a/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs +++ b/src/Bicep.Core/Analyzers/Linter/Rules/OutputsShouldNotContainSecretsRule.cs @@ -58,7 +58,7 @@ public OutputVisitor(OutputsShouldNotContainSecretsRule parent, SemanticModel mo public override void VisitOutputDeclarationSyntax(OutputDeclarationSyntax syntax) { // Does the output name contain 'password' (suggesting it contains an actual password)? - if (syntax.Name.IdentifierName.ToLowerInvariant().Contains("password")) + if (syntax.Name.IdentifierName.Contains("password", StringComparison.OrdinalIgnoreCase)) { string foundMessage = string.Format(CoreResources.OutputsShouldNotContainSecretsOutputName, syntax.Name.IdentifierName); this.diagnostics.Add(parent.CreateDiagnosticForSpan(syntax.Span, foundMessage)); From 65f8d6f979f5ae504e7894d42be37b5cf21617c9 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Wed, 8 Dec 2021 16:38:27 -0800 Subject: [PATCH 20/23] unit test fixes --- src/Bicep.LangServer.IntegrationTests/TelemetryTests.cs | 3 ++- .../BicepCompilationManagerTests.cs | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Bicep.LangServer.IntegrationTests/TelemetryTests.cs b/src/Bicep.LangServer.IntegrationTests/TelemetryTests.cs index d7379fbc857..35d1308efa8 100644 --- a/src/Bicep.LangServer.IntegrationTests/TelemetryTests.cs +++ b/src/Bicep.LangServer.IntegrationTests/TelemetryTests.cs @@ -354,7 +354,8 @@ public async Task BicepFileOpen_ShouldFireTelemetryEvent() { "no-unnecessary-dependson", "warning" }, { "adminusername-should-not-be-literal", "warning" }, { "use-stable-vm-image", "warning" }, - { "secure-parameter-default", "warning" } + { "secure-parameter-default", "warning" }, + { "outputs-should-not-contain-secrets", "warning" }, }; bicepTelemetryEvent.EventName.Should().Be(TelemetryConstants.EventNames.LinterStateOnBicepFileOpen); diff --git a/src/Bicep.LangServer.UnitTests/BicepCompilationManagerTests.cs b/src/Bicep.LangServer.UnitTests/BicepCompilationManagerTests.cs index a285a491dda..740bbd7ef24 100644 --- a/src/Bicep.LangServer.UnitTests/BicepCompilationManagerTests.cs +++ b/src/Bicep.LangServer.UnitTests/BicepCompilationManagerTests.cs @@ -1156,7 +1156,8 @@ public void GetLinterStateTelemetryOnBicepFileOpen_ShouldReturnTelemetryEvent() { "no-unnecessary-dependson", "warning" }, { "adminusername-should-not-be-literal", "warning" }, { "use-stable-vm-image", "warning" }, - { "secure-parameter-default", "warning" } + { "secure-parameter-default", "warning" }, + { "outputs-should-not-contain-secrets", "warning" }, }; telemetryEvent.Properties.Should().Equal(properties); @@ -1227,7 +1228,8 @@ public void GetLinterStateTelemetryOnBicepFileOpen_WithNoContents_ShouldUseDefau { "no-unnecessary-dependson", "warning" }, { "adminusername-should-not-be-literal", "warning" }, { "use-stable-vm-image", "warning" }, - { "secure-parameter-default", "warning" } + { "secure-parameter-default", "warning" }, + { "outputs-should-not-contain-secrets", "warning" }, }; telemetryEvent.Properties.Should().Equal(properties); From 009b856ea441f99aef74c91c5026bc05286c9ad2 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Wed, 8 Dec 2021 17:47:03 -0800 Subject: [PATCH 21/23] fix merge --- src/Bicep.Core/CoreResources.resx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 7ead32acac0..29ecdc254d8 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -125,7 +125,7 @@ No bicepconfig.json found for configuration override. - When setting an adminUserName property, don't use a literal value. + Property 'adminUserName' should not use a literal value. Use a param instead. Outputs should not contain secrets. @@ -165,7 +165,7 @@ string format to display error exception thrown in Linter Rule - Resource location should be specified by a parameter with no default value or one that defaults to 'global' or resourceGroup().location. + Resource location should be specified by a parameter without a default value or one that defaults to 'global' or resourceGroup().location. No unnecessary dependsOn. @@ -175,7 +175,7 @@ Text for linter rule error. {0} is a programmatic expression - No unused parameters. + All parameters must be used. Parameter "{0}" is declared but never used. @@ -187,7 +187,7 @@ secure parameter '{0}' - {0} is the parameter name + {0} is the parameter name. This string appears in the middle of a sentence, so it doesn't start with a capital letter. Something similar to: Found: secure parameter 'parameter1' Remove insecure default value. @@ -202,7 +202,7 @@ Remove unnecessary string interpolation. - No unused variables. + All variables must be used. Variable "{0}" is declared but never used. From 2350045cc2bcc9b15667d67d905d94da3122d1cd Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Wed, 8 Dec 2021 17:49:36 -0800 Subject: [PATCH 22/23] fix merge --- src/Bicep.Core/CoreResources.resx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 29ecdc254d8..7b0b78799b6 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -144,7 +144,7 @@ function '{0}' - {0} name of a programmatic function + {0} is the function name. This string appears in the middle of a sentence, hence doesn't start with capital letter. Something similar to: Found: function 'listKeys()' Environment URLs should not be hardcoded. Use the environment() function to ensure compatibility across clouds. @@ -187,7 +187,7 @@ secure parameter '{0}' - {0} is the parameter name. This string appears in the middle of a sentence, so it doesn't start with a capital letter. Something similar to: Found: secure parameter 'parameter1' + {0} is the parameter name. This string appears in the middle of a sentence, so it doesn't start with a capital letter. Something similar to: Found: secure parameter 'parameter1' Remove insecure default value. From fda8dfa6dfd0816ebaa7c4d080ec2a776697d5d9 Mon Sep 17 00:00:00 2001 From: Stephen Weatherford Date: Wed, 8 Dec 2021 17:52:10 -0800 Subject: [PATCH 23/23] fix merge --- src/Bicep.Core/CoreResources.resx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 7b0b78799b6..ca4a6461fe9 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -183,11 +183,11 @@ function '{0}' - {0} is the function name + {0} is the function name. This string appears in the middle of a sentence, hence doesn't start with capital letter. Something similar to: Found: function 'listKeys()' secure parameter '{0}' - {0} is the parameter name. This string appears in the middle of a sentence, so it doesn't start with a capital letter. Something similar to: Found: secure parameter 'parameter1' + {0} is the parameter name. This string appears in the middle of a sentence, so it doesn't start with a capital letter. Something similar to: Found: secure parameter 'parameter1' Remove insecure default value.