Skip to content

[Smart Hashing] Add eslint rule to limit usage of createHash from crypto module #2861

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

Merged
merged 2 commits into from
May 7, 2025

Conversation

harsh-joshi99
Copy link
Contributor

@harsh-joshi99 harsh-joshi99 commented Apr 8, 2025

This PR adds a Es lint rule that restricts the usage of createHash from crypto module, to encourage the usage of Smart hashing module for hashing.

For other usage of Crypto module, Eslint disable is added.

Testing

Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

varadarajan-tw
varadarajan-tw previously approved these changes Apr 9, 2025
.eslintrc.js Outdated
{
selector: "ImportDeclaration[source.value='crypto'] ImportSpecifier[imported.name='createHash']",
message:
'Avoid importing the "createHash" function from "crypto". Use "destination-actions/lib/hashing-utils" instead.'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also mention why they need to use hashing-utils and when?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Avoid importing the "createHash" function from "crypto". Use "destination-actions/lib/hashing-utils" instead.'
'The "destination-actions/lib/hashing-utils/processHashing" can autodetect prehashed values and avoid double hashing [link to doc]. Avoid importing the "createHash" function from "crypto"'

Copy link

codecov bot commented Apr 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.93%. Comparing base (7a40bef) to head (34605d1).
Report is 40 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2861      +/-   ##
==========================================
+ Coverage   77.61%   77.93%   +0.32%     
==========================================
  Files        1047     1047              
  Lines       19219    19246      +27     
  Branches     3750     3751       +1     
==========================================
+ Hits        14916    14999      +83     
+ Misses       2992     2925      -67     
- Partials     1311     1322      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@harsh-joshi99 harsh-joshi99 merged commit 8f04e9a into main May 7, 2025
14 checks passed
@harsh-joshi99 harsh-joshi99 deleted the smart-hashing/lint-rule branch May 7, 2025 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants