Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

linter rule: outputs-should-not-contain-secrets #4716

Merged
merged 31 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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' = {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -24,9 +25,13 @@ 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'
})
Expand All @@ -48,9 +53,13 @@ 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'
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)] 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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
@"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public void HasBuiltInRules()
[DataRow(NoUnnecessaryDependsOnRule.Code)]
[DataRow(NoUnusedParametersRule.Code)]
[DataRow(NoUnusedVariablesRule.Code)]
[DataRow(OutputsShouldNotContainSecretsRule.Code)]
[DataRow(PreferInterpolationRule.Code)]
[DataRow(SecureParameterDefaultRule.Code)]
[DataRow(SimplifyInterpolationRule.Code)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,28 @@ protected void AssertLinterRuleDiagnostics(string ruleCode, string bicepText, On

private void RunWithDiagnosticAnnotations(string bicepText, Func<IDiagnostic, bool> filterFunc, OnCompileErrors onCompileErrors, Action<IEnumerable<IDiagnostic>> 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);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
// 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);
}

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}'
",
$"{description} Found possible secret: secure parameter 'secureParam'"
)]
[DataRow(@"
@secure()
param secureParam string

output badResult object = {
value1: {
value2: 'this is the value ${secureParam}'
}
}
",
$"{description} Found possible secret: secure parameter '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
$"{description} Found possible secret: secure parameter '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
}
",
$"{description} Found possible secret: function '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
}
",
$"{description} Found possible secret: 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);
}

[Ignore("TODO: blocked by https://github.com/Azure/bicep/issues/4833")]
[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
}
",
$"{description} Found possible secret: function 'listAnything'"
)]
[DataTestMethod]
public void If_ListFunctionInOutput_AsResourceMethod_ThroughVariable_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
$"{description} Found possible secret: function '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
$"{description} Found possible secret: function 'listAnything'"
)]
[DataTestMethod]
public void If_ListFunctionInOutput_AsStandaloneFunction_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 = az.listAnything(resourceId('Microsoft.Storage/storageAccounts', 'storageName'), '2021-02-01')
",
// TTK output:
// [-] Outputs Must Not Contain Secrets(6 ms)
// Output contains secret: badResult
$"{description} Found possible secret: function 'listAnything'"
)]
[DataTestMethod]
public void If_ListFunctionInOutput_AsAzInstanceFunction_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
$"{description} Found possible secret: output name 'badResultPassword' suggests a secret"
)]
[DataRow(@"
output possiblepassword string = 'hello'
",
$"{description} Found possible secret: output name 'possiblepassword' suggests a secret"
)]
[DataRow(@"
output password string = 'hello'
",
$"{description} Found possible secret: output name 'password' suggests a secret"
)]
[DataRow(@"
output passwordNumber1 string = 'hello'
",
$"{description} Found possible secret: output name 'passwordNumber1' suggests a secret"
)]
[DataTestMethod]
public void If_OutputNameLooksLikePassword_ShouldFail(string text, params string[] expectedMessages)
{
CompileAndTest(text, OnCompileErrors.Ignore, expectedMessages);
}
}
}
Loading