Skip to content

Added rules for ciphers for Crypto library - #3

Closed
jrepucci-rh wants to merge 4 commits into
mainfrom
js_crypto_algo
Closed

jrepucci-rh wants to merge 4 commits into
mainfrom
js_crypto_algo

Conversation

@jrepucci-rh

Copy link
Copy Markdown
Collaborator

Description

Type of change

  • New rule(s)
  • New / updated test fixtures
  • Bug fix (false positive / false negative)
  • Test framework or tooling
  • Documentation

Checklist

  • New / changed rules have matching test fixtures
  • make test passes locally
  • Rule files validate with opengrep --validate
  • Commits follow Conventional Commits
  • No private or internal data is included in this change

Notes for reviewers

jrepucci-rh and others added 4 commits September 11, 2026 11:38
The 22 JS crypto algorithm rule files reduce to four distinct rule
bodies - the sink patterns are byte-identical across algorithms and only
the metavariable-regex and crypto: metadata differ.

Keep one file per captured API so each rule has exactly one copy to
review: aes (Cipheriv.update / Decipheriv.update and their AEAD
variants), sha-2 (Hash.digest), hmac (Hmac.digest). Untrack the other 19
algorithms and their integration fixtures.

Removed from the index only - the files remain on disk untouched, so
re-adding is a plain git add.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@isasmendiagus

Copy link
Copy Markdown

Hi @jrepucci-rh, Thanks, and sorry for the delay. Three things:

  1. The PR conflicts with main. The AES rules and fixture already landed in feat(javascript): add Node.js crypto library algorithms #4, so please rebase and remove the AES files from this PR.
  2. The one-shot crypto.hash('sha256', data) (Node 21.7+) is not detected. Could you add a rule for it, with a case in the fixture?
  3. Node also accepts uppercase names like 'SHA256'. The AES and RSA rules already handle that with (?i); please do the same here and add a case for it.

@isasmendiagus isasmendiagus left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks, and sorry for the delay. Three things:

  1. The PR conflicts with main. The AES rules and fixture already landed in #4, so please rebase and remove the AES files from this PR.
  2. The one-shot crypto.hash('sha256', data) (Node 21.7+) is not detected. Could you add a rule for it, following the same shape as rsa.sign, with a case in the fixture?
  3. Node also accepts uppercase names like 'SHA256'. The AES and RSA rules already handle that with (?i); please do the same here and add a case for it.

@isasmendiagus

Copy link
Copy Markdown

Closing as agreed. HMAC/SHA-2 rules for Node.js crypto can come back in a new PR if they become a priority.

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.

2 participants