diff --git a/docs/analyzer-rules/AZFW0017.md b/docs/analyzer-rules/AZFW0017.md new file mode 100644 index 000000000..1092fb17f --- /dev/null +++ b/docs/analyzer-rules/AZFW0017.md @@ -0,0 +1,27 @@ +# AZFW0017: Redundant usage of local.settings.json in the worker configuration + +| | Value | +|-|-| +| **Rule ID** |AZFW0017| +| **Category** |[Usage]| +| **Severity** |Warning| + +## Cause + +This rule is triggered when `local.settings.json` is passed to `AddJsonFile()` method of the `IConfigurationBuilder`. + +## Rule description + +`local.settings.json` is a file that is automatically loaded to the configuration by Azure Functions Core Tools. +There is no need to explicitly add it to the configuration of the worker during development. And in production scenarios `local.settings.json` should never be used because it means checking it into the source control. +And this a securiy risk because this file usually contains sensitive information like connection strings. + + +## How to fix violations + +Don't pass `local.settings.json` to `AddJsonFile()` when configuring the Azure Functions Worker. + + +## When to suppress + +It is okay to suppress the warning if it happens in a code block that is guaranteed to execute only during the development (Debug configuration) and you want to be explicit about the source of configuration. It should never be suppressed for the code that will run in production. \ No newline at end of file diff --git a/sdk/Sdk.Analyzers/DiagnosticDescriptors.cs b/sdk/Sdk.Analyzers/DiagnosticDescriptors.cs index 7762bb268..76098077c 100644 --- a/sdk/Sdk.Analyzers/DiagnosticDescriptors.cs +++ b/sdk/Sdk.Analyzers/DiagnosticDescriptors.cs @@ -5,7 +5,7 @@ namespace Microsoft.Azure.Functions.Worker.Sdk.Analyzers { - internal class DiagnosticDescriptors + internal static class DiagnosticDescriptors { private static DiagnosticDescriptor Create(string id, string title,string messageFormat, string category, DiagnosticSeverity severity) { @@ -32,5 +32,9 @@ private static DiagnosticDescriptor Create(string id, string title,string messag public static DiagnosticDescriptor IterableBindingTypeExpectedForBlobContainer { get; } = Create(id: "AZFW0011", title: "Invalid binding type", messageFormat: "The binding type '{0}' must be iterable for container path.", category: Constants.DiagnosticsCategories.Usage, severity: DiagnosticSeverity.Error); + + public static DiagnosticDescriptor LocalSettingsJsonNotAllowedAsConfiguration { get; } + = Create(id: "AZFW0017", title: "local.settings.json should not be used as a configuration file", messageFormat: "There is no need to use local.settings.json as a configuration file. During development, it's automatically loaded by Functions Core Tools; in production scenarios, configuration should be handled via App Settings in Azure.", + category: Constants.DiagnosticsCategories.Usage, severity: DiagnosticSeverity.Warning); } } diff --git a/sdk/Sdk.Analyzers/LocalSettingsJsonNotAllowedAsConfiguration.cs b/sdk/Sdk.Analyzers/LocalSettingsJsonNotAllowedAsConfiguration.cs new file mode 100644 index 000000000..2c1ac023a --- /dev/null +++ b/sdk/Sdk.Analyzers/LocalSettingsJsonNotAllowedAsConfiguration.cs @@ -0,0 +1,143 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.IO; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Microsoft.Azure.Functions.Worker.Sdk.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class LocalSettingsJsonNotAllowedAsConfiguration : DiagnosticAnalyzer + { + private const string ConfigurationBuilderFullName = "Microsoft.Extensions.Configuration.IConfigurationBuilder"; + private const string LocalSettingsJsonFileName = "local.settings.json"; + private const string AddJsonFileMethodName = "AddJsonFile"; + + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(DiagnosticDescriptors.LocalSettingsJsonNotAllowedAsConfiguration); + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze); + context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression); + } + + // This analyzer handles only the overloads of AddJsonFile() with a string as first argument + private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) + { + var invocation = (InvocationExpressionSyntax)context.Node; + if (!IsAddJsonFileMethodWithAtLeastOneArgument(invocation, context)) + { + return; + } + + var firstArgument = invocation.ArgumentList.Arguments.First().Expression; + if (!IsOfStringType(firstArgument, context)) + { + return; + } + + if (FindLiterals(firstArgument, context).Any(IsPathToLocalSettingsJson)) + { + var diagnostic = CreateDiagnostic(firstArgument); + context.ReportDiagnostic(diagnostic); + } + } + + // This method ensures that the analyzer doesn't check more than it should. + // It looks for the literal values of the argument, from the easiest to hardest to find. + // Current order: literal -> constant -> declaration -> assignment in containing method. + private static IEnumerable FindLiterals(ExpressionSyntax argument, SyntaxNodeAnalysisContext context) + { + if (argument is LiteralExpressionSyntax literal) + { + yield return literal.Token.ValueText; + yield break; // no need to check further + } + + var constant = context.SemanticModel.GetConstantValue(argument); + if (constant.HasValue) + { + yield return constant.Value.ToString(); + yield break; // no need to check further + } + + if (argument is not IdentifierNameSyntax identifier) + { + yield break; + } + + var identifierSymbol = context.SemanticModel.GetSymbolInfo(identifier).Symbol; + if (identifierSymbol is null) + { + yield break; + } + + var declarationLiterals = identifierSymbol.DeclaringSyntaxReferences + .Select(reference => reference.GetSyntax(context.CancellationToken)) + .OfType() + .Select(variable => variable.Initializer?.Value) + .OfType(); + + // usually there should be only 1 declaration but better safe than sorry + foreach (var declarationLiteral in declarationLiterals) + { + yield return declarationLiteral.Token.ValueText; + } + + // let's check assignments in the containing method + var containingMethod = argument.Ancestors() + .OfType() + .FirstOrDefault(); + + if (containingMethod is null) + { + yield break; + } + + var assignmentsInContainingMethod = containingMethod.DescendantNodes() + .OfType() + .Where(assignment => assignment.SpanStart < argument.SpanStart); + + foreach (var assignment in assignmentsInContainingMethod) + { + var leftSymbol = context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol; + if (SymbolEqualityComparer.Default.Equals(leftSymbol, identifierSymbol) + && assignment.Right is LiteralExpressionSyntax assignmentLiteral) + { + yield return assignmentLiteral.Token.ValueText; + } + } + } + + private static bool IsAddJsonFileMethodWithAtLeastOneArgument( + InvocationExpressionSyntax invocation, SyntaxNodeAnalysisContext context) + { + return invocation.ArgumentList.Arguments.Count > 0 + && invocation.Expression is MemberAccessExpressionSyntax { Name.Identifier.Text: AddJsonFileMethodName } memberAccess + && context.SemanticModel.GetSymbolInfo(memberAccess).Symbol is IMethodSymbol methodSymbol + && methodSymbol.ReceiverType?.ToDisplayString() == ConfigurationBuilderFullName; + } + + private static bool IsOfStringType(ExpressionSyntax expression, SyntaxNodeAnalysisContext context) + { + return context.SemanticModel.GetTypeInfo(expression).Type?.SpecialType == SpecialType.System_String; + } + + private static bool IsPathToLocalSettingsJson(string literal) + { + return Path.GetFileName(literal) == LocalSettingsJsonFileName; + } + + private static Diagnostic CreateDiagnostic(ExpressionSyntax expression) + { + return Diagnostic.Create( + DiagnosticDescriptors.LocalSettingsJsonNotAllowedAsConfiguration, + expression.GetLocation()); + } + } +} diff --git a/test/Sdk.Analyzers.Tests/LocalSettingsJsonNotValidConfigurationTests.cs b/test/Sdk.Analyzers.Tests/LocalSettingsJsonNotValidConfigurationTests.cs new file mode 100644 index 000000000..7410819c7 --- /dev/null +++ b/test/Sdk.Analyzers.Tests/LocalSettingsJsonNotValidConfigurationTests.cs @@ -0,0 +1,463 @@ +using System.Collections.Immutable; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using Xunit; +using AnalyzerTest = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerTest; +using AnalyzerVerifier = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier; + +namespace Sdk.Analyzers.Tests; + +public class LocalSettingsJsonNotValidConfigurationTests +{ + private static readonly ReferenceAssemblies _referenceAssemblies = ReferenceAssemblies.Net.Net70.WithPackages( + ImmutableArray.Create(new PackageIdentity("Microsoft.Azure.Functions.Worker", "1.23.0"))); + + [Fact] + public async Task LocalSettingsJsonPassedToConfigurationIssuesWarning() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile("local.settings.json"); + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(12, 36, 12, 57) + } + }.RunAsync(); + } + + [Fact] + public async Task NotLocalSettingsJsonDoesNotGenerateWarning() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile("settings.json"); + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + // No diagnostics expected + } + }.RunAsync(); + } + + [Fact] + public async Task CustomAddJsonFileMethodDoesNotGenerateWarning() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + public class MyCustomConfig + { + public void AddJsonFile(string fileName) + { + // Custom implementation + } + } + + public static class Program + { + public static void Main() + { + var config = new MyCustomConfig(); + config.AddJsonFile("local.settings.json"); // Should not trigger warning + } + } + """, + + ExpectedDiagnostics = { + // No diagnostics expected + } + }.RunAsync(); + } + + [Fact] + public async Task ConstLocalSettingsJsonIsCaught() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + const string fileName = "local.settings.json"; + + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(fileName); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(14, 36, 14, 44) + } + }.RunAsync(); + } + + [Fact] + public async Task LocalSettingsJsonInVariableDeclarationIsCaught() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var fileName = "local.settings.json"; + + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(fileName); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(14, 36, 14, 44) + } + }.RunAsync(); + } + + [Fact] + public async Task OverloadWithOptionalBooleanIsHandled() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile("local.settings.json", optional: true); + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(12, 36, 12, 57) + } + }.RunAsync(); + } + + [Fact] + public async Task OverloadWithReloadOnChangeIsHandled() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile("local.settings.json", optional: false, reloadOnChange: true); + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(12, 36, 12, 57) + } + }.RunAsync(); + } + + [Fact] + public async Task LocalSettingsJsonAsVariableWithReassignment() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var fileName = "settings.json"; + fileName = "local.settings.json"; + + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(fileName); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(15, 36, 15, 44) + } + }.RunAsync(); + } + + [Fact] + public async Task LocalSettingsJsonAsVariableWithMultipleReassignments() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var fileName = "todo"; + fileName = "local.settings.json"; + fileName = "my.settings.json"; + + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(fileName); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(16, 36, 16, 44) + } + }.RunAsync(); + } + + [Fact] + public async Task LocalSettingsJsonCanBeDetectedForBothDeclarationsAndAssignments() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + public static void Main() + { + var fileName = "local.settings.json"; + + if (false) + { + fileName = "my.settings.json"; + } + + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(fileName); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(19, 36, 19, 44) + } + }.RunAsync(); + } + + [Fact] + public async Task StaticReadonlyLocalSettingsJsonCanBeDetected() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + private static readonly string _settingsFile = "local.settings.json"; + + public static void Main() + { + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(_settingsFile); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(14, 36, 14, 49) + } + }.RunAsync(); + } + + [Fact] + public async Task ConstantFiledLocalSettingsJsonCanBeDetected() + { + await new AnalyzerTest + { + ReferenceAssemblies = _referenceAssemblies, + + TestCode = """ + using Microsoft.Extensions.Hosting; + using Microsoft.Extensions.Configuration; + + public static class Program + { + private const string _settingsFile = "local.settings.json"; + + public static void Main() + { + var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .ConfigureAppConfiguration((context, config) => + { + config.AddJsonFile(_settingsFile); // Should trigger a warning + }) + .Build(); + + host.Run(); + } + } + """, + + ExpectedDiagnostics = { + AnalyzerVerifier.Diagnostic() + .WithSeverity(DiagnosticSeverity.Warning) + .WithSpan(14, 36, 14, 49) + } + }.RunAsync(); + } +}