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

Add dummy passwords to documentation for rule 'js/hardcoded-passwords' #16359

Closed
ebickle opened this issue Apr 29, 2024 · 4 comments
Closed

Add dummy passwords to documentation for rule 'js/hardcoded-passwords' #16359

ebickle opened this issue Apr 29, 2024 · 4 comments
Assignees
Labels
JS question Further information is requested

Comments

@ebickle
Copy link
Contributor

ebickle commented Apr 29, 2024

Description of the issue

The rule 'js/hardcoded-passwords' often returns false positives for our developers, often in their test code. This is somewhat expected, since the rule can't be expected to find every possible dummy/stub value added to unit tests.

Internally the rule depends on isDummyPassword from the PasswordHeuristics module to avoid triggering on the most common dummy/stub values.

Would it be possible to add some of these example dummy passwords to the documentation for the rule? That way, when our developers get an alert for 'js/hardcoded-passwords' for unit tests they can immediately see a fix - e.g. using sample, example, or fake as the value to avoid the alert from being detected in the first place.

@ebickle ebickle added the question Further information is requested label Apr 29, 2024
@sidshank sidshank added the JS label Apr 30, 2024
@ginsbach
Copy link
Contributor

ginsbach commented May 3, 2024

Thank you for the suggestion. I have forwarded this to the relevant team and they are working on it!

@aibaars
Copy link
Contributor

aibaars commented May 6, 2024

This should be fixed by #16417 once that pull request is merged.

@erik-krogh
Copy link
Contributor

Hi Eric (nice name 😉)

I just un-drafted #16417, which should fix this issue, and your related issue (#16360).
I tested the effect of the change, and the change in results look OK.

Your points (in both of these issues) are definitely reasonable, and it seems like the right thing to do.

Thanks for the reports.

@erik-krogh
Copy link
Contributor

Hi 👋

I just merged #16417 which I think should solve your issue.
If you have feedback and other comments just let me know, nothing is set in stone.

Thanks for the nice report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JS question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants