SONARPY-314 Rule S4790: Hashing data is security-sensitive

[andrea-guarino-sonarsource]

No description provided.

[ghost]

Why using an immutable Set to then stream on it?! Please use:

Stream.of("a", "b").map(...

[ghost]

Same approach as before, no need for the intermediate ImmutableSet

[ghost]

and again.

[ghost]

Rather than chained if-else-ifI would prefer to have a switch:

switch(node.getType()) {
  case PythonGrammar.CALL_EXPR:
    checkQuestionableHashingAlgorithm(node);
    break;
  case /*...*/ :
    // ...
    break;
  default:
    // do nothing - reacting on all the registered nodes
}

[ghost]

this deserve a comment, otherwise I would tend to remove it.

[ghost]

don't you want to explicitly return here?
It sounds strange to me that we could raise 2 issues in one run (by reaching both line 128 and 135), but if you think it's OK, then fine.

[andrea-guarino-sonarsource]

yes indeed, I agree. I'll add a return

[ghost]

what if there is parenthesis?

[andrea-guarino-sonarsource]

In case of parenthesis, TESTLIST_STAR_EXPR will still be the first child of EXPRESSION_STMT.
Parenthesis will be consumed lower in the grammar (in ATOM production rule) and at line 123 I'm taking the firstDescendant(PythonGrammar.ATTRIBUTE_REF).
I'm adding a test case to cover this.

However I just realized I won't raise an issue in this case:

foo.bar, mySettings.PASSWORD_HASHERS = value # Noncompliant

I will change my implementation to cover this case

[ghost]

should be annotated @CheckForNull

[ghost]

return symbol != null ? symbol.qualifiedName() : "";

[ghost]

.filter(atom -> questionableDjangoHashers.contains(getQualifiedName(atom)))
.forEach(atom -> addIssue(atom, MESSAGE));