Skip to content

Commit

Permalink
#13619 secure parameter linter fix (#13717)
Browse files Browse the repository at this point in the history
Fixes  #13619

What was happening was that the linter rule was falling into the default
behaviour and using the default error string as it wasn't checking it
was a variable assignment. Added 1 unit test to cover the basic premise
of the lint rule failure.

One thing which did occur to me was that assignment of the secure
parameter to a variable which is a hardcoded string isnt caught - but
seemed like it should be a lint rule in of itself? Thoughts?

eg.

```
var blah string = 'something'

@secure()
param myparam string = blah
```

Should we overcomplicate the lint rule to find a reassignment of a
secure parameter? Seems an illogical but valid thing to do though
  • Loading branch information
davidlloyduk committed Apr 3, 2024
1 parent 6163c5f commit 2094f3b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -201,5 +201,32 @@ public void HandlesSyntaxErrors(int diagnosticCount, string text)
AssertLinterRuleDiagnostics(SecureParameterDefaultRule.Code, text, diagnosticCount, new Options(OnCompileErrors.Ignore));
}

[DataRow(0, @"
@secure()
param param1 string
@secure()
param param2 string = param1
")]
[DataRow(0, @"
@secure()
param param1 string = ''
@secure()
param param2 string = param1
")]
[DataRow(1, @"
@secure()
param param1 string = 'abc'
@secure()
param param2 string = param1
")]
[DataTestMethod]
public void ParameterReassignment_TestPasses(int diagnosticCount, string text)
{
AssertLinterRuleDiagnostics(SecureParameterDefaultRule.Code, text, diagnosticCount);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Bicep.Core.Diagnostics;
using Bicep.Core.Semantics;
using Bicep.Core.Syntax;
using Bicep.Core.TypeSystem;

namespace Bicep.Core.Analyzers.Linter.Rules
{
Expand Down Expand Up @@ -34,6 +35,11 @@ override public IEnumerable<IDiagnostic> AnalyzeInternal(SemanticModel model, Di
// Empty string - okay
continue;
}
else if (model.GetTypeInfo(defaultValue).ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure))
{
// has @secure attribute - okay
continue;
}
else if (defaultValue is ObjectSyntax objectSyntax && !objectSyntax.Properties.Any())
{
// Empty object - okay
Expand Down

0 comments on commit 2094f3b

Please sign in to comment.