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

Improving AzureFunctions and build project #91

Merged
merged 9 commits into from
Jan 8, 2021

Conversation

eddynaka
Copy link
Collaborator

@eddynaka eddynaka commented Jan 8, 2021

No description provided.

/// </param>
/// <returns>Return the validation state.</returns>
#pragma warning disable IDE0060 // Remove unused parameter
public static string IsValid(
ref string matchedPattern,
ref Dictionary<string, string> groups,
ref bool performDynamicValidation,
ref string failureLevel)
ref string failureLevel,
ref string fingerprint)
Copy link
Collaborator Author

@eddynaka eddynaka Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref string fingerprint [](start = 12, length = 22)

We were missing the fingerprint (fixed) #ByDesign

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see the fingerprints generated in result with this change, do we intended to show them?
e.g.
"fingerprints": {
"SecretFingerprint/v1": "[pat/vs=h5lxeqkz4zujkyvzg5emkejgelxj4x64tvyzclprekuloawbbz5q]"
}


In reply to: 553894953 [](ancestors = 553894953)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that is by design.


In reply to: 554117253 [](ancestors = 554117253,553894953)

@@ -1,4 +1,5 @@
{
"ValidatorsAssemblyName": "Security.dll",
Copy link
Collaborator Author

@eddynaka eddynaka Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ValidatorsAssemblyName": "Security.dll", [](start = 2, length = 41)

We were missing the assembly name (fixed) #ByDesign

@@ -36,6 +36,12 @@ public class AnalyzeCommand : MultithreadedAnalyzeCommandBase<AnalyzeContext, An
SearchDefinitions definitions =
JsonConvert.DeserializeObject<SearchDefinitions>(searchDefinitionsText);

// This would skip files that does not look like rules.
if (definitions == null || definitions.Definitions == null)
Copy link
Collaborator Author

@eddynaka eddynaka Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (definitions == null || definitions.Definitions == null) [](start = 16, length = 59)

We might find files that aren't rules. If that happen, the project fails. With this, we will just continue. #ByDesign

@@ -35,7 +36,7 @@ public static class HttpAnalyzeFunction
SarifLog sariflog = await Task.Run(() => SpamAnalyzer.Analyze(sourceFilePath, text, definitionsFolder));

log.LogInformation($"Completed analyzing file {file.FileName}");
return new JsonResult(sariflog);
return new OkObjectResult(sariflog);
Copy link
Collaborator Author

@eddynaka eddynaka Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OkObjectResult [](start = 31, length = 14)

Changing from JsonResult to OkObjectResult, we prevent object -> string. #ByDesign

<ProjectReference Include="..\Sarif.PatternMatcher\Sarif.PatternMatcher.csproj" />
</ItemGroup>
<Target Name="CopyingSpamRules" BeforeTargets="Build">
<Copy SourceFiles="@(SpamFiles)" DestinationFolder="$(MSBuildThisFileDirectory)\Rules\" OverwriteReadOnlyFiles="true" SkipUnchangedFiles="false" />
</Target>
Copy link
Collaborator Author

@eddynaka eddynaka Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't need this, since I added a project reference to always publish the security.csproj as well #ByDesign

@@ -15,6 +16,7 @@ namespace Microsoft.CodeAnalysis.Sarif.PatternMatcher.Function
internal static class SpamAnalyzer
{
internal static readonly IFileSystem FileSystem;
internal static ISet<Skimmer<AnalyzeContext>> Skimmers;
Copy link
Collaborator Author

@eddynaka eddynaka Jan 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal static ISet<Skimmer> Skimmers [](start = 8, length = 54)

Added a naive cache to prevent loading rules all the time. Since the rules won't change unless we publish a new version, that can be read once. #ByDesign

@eddynaka
Copy link
Collaborator Author

eddynaka commented Jan 8, 2021

@yongyan-gh , you can take this branch and add the fix that you checked with Jeff (merging the two functions in one). Let me know what do you think. #Closed

@yongyan-gh
Copy link
Collaborator

Added the change


In reply to: 756711861 [](ancestors = 756711861)

@eddynaka eddynaka merged commit 7e4150b into main Jan 8, 2021
@eddynaka eddynaka deleted the users/ednakamu/improving-azure-functions branch January 8, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants