[3.1] Credscan followup #43054
[3.1] Credscan followup #43054
Conversation
| @@ -571,6 +571,7 @@ public enum FedAuthLibrary : byte | |||
| // Login data validation Rules | |||
| // | |||
| internal const ushort MAXLEN_HOSTNAME = 128; // the client machine name | |||
| // [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Not a password.")] | |||
There was a problem hiding this comment.
should this be on the line below?
There was a problem hiding this comment.
This one is weird. The
internal const ushort MAXLEN_USERNAME = 128; // the client user id
internal const ushort MAXLEN_PASSWORD = 128; // ...
is considered as password even with the new line (something like MAXLEN_USERNAME = MAXLEN_PASSWORD = 128 )
This is why the suppression message is on line which doesn't look like password. Perhaps the justification message can be better.
|
We should not merge any suppressions until we get exemption mail. Would it make sense to have two PR's, one for suppression and one for PLACEHOLDER changes? |
|
Now it is in two PRs, PLACEHOLDERS needs to be merged prior we can close work items, for suppress messages I think we are fine if we do when we have the exception and we can merge it in next branch open window. This PR will clean a lot when we merge the #43051. |
I'm confused, this PR contains both suppressions and PLACEHOLDERS? |
|
Yep, it is based on last commit of the #43051. When the previous one is merged, this one will contain the suppression only. It is because I needed to run the tool on whole repository with all changes. |
|
Oh of course - got it! |
…dscan-3.1followup
|
Failure is Writing3MBBase64Bytes - unrelated. |
danmoseley
changed the title
Adds suppression messages.