Skip to content

Conversation

@Seifeldin7
Copy link

@Seifeldin7 Seifeldin7 commented Dec 10, 2025

Adds pnpm override to force jws to version 3.2.3, which fixes the improper HMAC signature verification vulnerability in the auth0/node-jws package. This vulnerability affected the jsonwebtoken package used for license validation in react-on-rails-pro-node-renderer.

Summary

Remove this paragraph and provide a general description of the code changes in your pull
request... were there any bugs you had fixed? If so, mention them. If
these bugs have open GitHub issues, be sure to tag them here as well,
to keep the conversation linked together.

Pull Request checklist

Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by ~.

  • Add/update test to cover these changes
  • Update documentation
  • Update CHANGELOG file

Add the CHANGELOG entry at the top of the file.

Other Information

Remove this paragraph and mention any other important and relevant information such as benchmarks.

Summary by CodeRabbit

  • Chores
    • Updated project dependency overrides configuration to ensure compatible package versions.

✏️ Tip: You can customize this high-level summary in your review settings.

Adds pnpm override to force jws to version 3.2.3, which fixes the
improper HMAC signature verification vulnerability in the auth0/node-jws
package. This vulnerability affected the jsonwebtoken package used for
license validation in react-on-rails-pro-node-renderer.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

A new pnpm dependency override for the "jws" package is added to package.json, pinning it to version 3.2.3. This is a single-line configuration change that specifies a specific version constraint for this transitive dependency.

Changes

Cohort / File(s) Change Summary
Dependency Override Configuration
package.json
Added pnpm override entry for "jws" pinned to version "3.2.3" under pnpm.overrides

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Note: While the change itself is straightforward, reviewers should verify the rationale for pinning "jws" to this specific version, as dependency overrides typically indicate addressing a security issue, compatibility problem, or transitive dependency conflict.

Suggested labels

bug, review-needed, full-ci

Suggested reviewers

  • alexeyr-ci2

Poem

🐰 A version so precise, like a rabbit's keen sight,
jws pinned at 3.2.3, locked down just right,
Dependencies tamed, no conflicts in sight,
Overrides in place, keeping chaos in flight! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly summarizes the main change: fixing a HIGH severity security vulnerability (CVE-2023-35916) in the jws package. It accurately reflects the changeset which adds a pnpm override to enforce jws version 3.2.3.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch seif/fix-jws-vulnerability

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Dec 10, 2025

Code Review

Summary

This PR addresses CVE-2023-35916, a HIGH severity vulnerability in the jws package related to improper HMAC signature verification. The fix uses pnpm's override mechanism to force jws to version 3.2.3, which patches the vulnerability.

✅ Positive Aspects

  1. Security Fix: Addresses a critical vulnerability in JWT signature verification
  2. Correct Approach: Using pnpm overrides is the right way to force a transitive dependency version
  3. Minimal Impact: The change is surgical - only affects the vulnerable dependency
  4. Already Mitigated in Code: The codebase already enforces RS256 algorithm in licenseValidator.ts:122, which prevents the vulnerability from being exploited even with the vulnerable version

⚠️ Issues & Required Changes

1. PR Description Incomplete (CRITICAL)

The PR description template hasn't been filled out. Please:

2. Missing CHANGELOG Entry (REQUIRED per CLAUDE.md)

According to CLAUDE.md, security fixes are user-visible changes that MUST be documented. Please add an entry to /react_on_rails_pro/CHANGELOG.md:

### Security
- Fixed HIGH severity vulnerability (CVE-2023-35916) in jws package by forcing upgrade to v3.2.3. The vulnerability allowed HMAC signature verification bypass in certain configurations. Our code already enforces RS256 algorithm-only verification (see licenseValidator.ts:122), which prevented exploitation, but this update eliminates the vulnerability at the dependency level. [PR 2203](https://github.com/shakacode/react_on_rails/pull/2203) by [Seifeldin7](https://github.com/Seifeldin7)

Note: This should go in the Pro changelog (/react_on_rails_pro/CHANGELOG.md) since it affects the react-on-rails-pro-node-renderer package which uses jsonwebtoken.

3. Missing Test Coverage Checkbox (PR Checklist)

The checklist shows "Add/update test to cover these changes" is unchecked. For this specific case:

  • No new tests needed - existing tests in licenseValidator.test.ts already verify RS256-only algorithm enforcement
  • Please check the box and note: "Existing tests cover algorithm enforcement - no new tests needed"

4. Update Documentation Checkbox (PR Checklist)

  • Check the "Update documentation" box (the CHANGELOG entry counts)

🔍 Security Analysis

The Vulnerability (CVE-2023-35916):
The jws package had an improper HMAC signature verification vulnerability that could allow attackers to forge tokens if the verifier uses symmetric algorithms (HS256, HS384, etc.) instead of asymmetric ones (RS256).

Why This Codebase Was Already Protected:
Looking at packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts:117-122:

const decoded = jwt.verify(licenseString, PUBLIC_KEY, {
  // Enforce RS256 algorithm only to prevent "alg=none" and downgrade attacks.
  algorithms: ['RS256'],  // ← This prevents the vulnerability
  ignoreExpiration: true,
});

The code explicitly allows only RS256, which uses asymmetric cryptography and is not affected by the HMAC bypass. However, fixing the dependency is still important for:

  1. Defense in depth
  2. Eliminating the vulnerability for any future code paths
  3. Resolving security scanner alerts

📋 Testing Recommendations

Before merging:

  1. ✅ Verify the override works: pnpm list jws should show 3.2.3
  2. ✅ Run existing license validation tests: cd packages/react-on-rails-pro-node-renderer && pnpm test
  3. ✅ Ensure no breaking changes in jws 3.2.3 vs 3.2.2 (changelog appears minimal)

🎯 Action Items

  • Fill out PR description with CVE details
  • Add CHANGELOG entry to /react_on_rails_pro/CHANGELOG.md
  • Check all completed items in PR checklist
  • Verify pnpm list jws shows 3.2.3
  • Run cd packages/react-on-rails-pro-node-renderer && pnpm test

📚 References


Overall Assessment: This is a good security fix with the right approach. Once the documentation is updated, this is ready to merge. The code was already protected due to algorithm enforcement, but fixing the dependency is the right thing to do.

@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
react-on-rails/client bundled (gzip) 62.33 KB (0%)
react-on-rails/client bundled (gzip) (time) 62.33 KB (0%)
react-on-rails/client bundled (brotli) 53.57 KB (0%)
react-on-rails/client bundled (brotli) (time) 53.57 KB (0%)
react-on-rails-pro/client bundled (gzip) 63.39 KB (0%)
react-on-rails-pro/client bundled (gzip) (time) 63.39 KB (0%)
react-on-rails-pro/client bundled (brotli) 54.53 KB (0%)
react-on-rails-pro/client bundled (brotli) (time) 54.53 KB (0%)
registerServerComponent/client bundled (gzip) 71.12 KB (0%)
registerServerComponent/client bundled (gzip) (time) 71.12 KB (0%)
registerServerComponent/client bundled (brotli) 61.27 KB (0%)
registerServerComponent/client bundled (brotli) (time) 61.27 KB (0%)
wrapServerComponentRenderer/client bundled (gzip) 65.75 KB (0%)
wrapServerComponentRenderer/client bundled (gzip) (time) 65.75 KB (0%)
wrapServerComponentRenderer/client bundled (brotli) 56.42 KB (0%)
wrapServerComponentRenderer/client bundled (brotli) (time) 56.42 KB (0%)

@Seifeldin7 Seifeldin7 marked this pull request as ready for review December 10, 2025 17:36
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57de0bc and 86ce07d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (1)
  • package.json (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md}: ALWAYS run bundle exec rubocop and fix ALL violations before every commit/push
ALWAYS ensure files end with a newline character before committing

Files:

  • package.json
**/*.{js,ts,jsx,tsx,json,css,scss,md}

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS let Prettier handle ALL formatting - never manually format code

Files:

  • package.json
**/*.{json,yml,yaml}

📄 CodeRabbit inference engine (CLAUDE.md)

When resolving merge conflicts in configuration files, verify file paths are correct and test affected scripts before committing

Files:

  • package.json
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.093Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for open-source features, bug fixes, breaking changes, deprecations, and performance improvements using format: `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)`
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.093Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Only use `pnpm` as the JavaScript package manager, never use `npm` or `yarn`
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.

Applied to files:

  • package.json
📚 Learning: 2025-12-10T09:27:45.093Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.093Z
Learning: Applies to **/*.{js,ts,jsx,tsx} : Only use `pnpm` as the JavaScript package manager, never use `npm` or `yarn`

Applied to files:

  • package.json
🔇 Additional comments (2)
package.json (2)

119-119: No action needed. The jws dependency at version 3.2.3 is isolated to the root package.json and doesn't require version consistency checks across other workspace files. spec/dummy/package.json doesn't exist in this repository, and no other package.json files reference jws.


119-119: Use CVE-2025-65945 instead of CVE-2023-35916 in the PR description.

Version 3.2.3 does fix the HMAC signature verification vulnerability (CVE-2025-65945). This CVE affects jws versions ≤ 3.2.2 and 4.0.0, where HMAC (HS256) signature verification can be bypassed when using jws.createVerify() with user-controlled header/payload in secret-lookup logic. The fix is confirmed in jws 3.2.3 (and 4.0.1). Update the PR description to reference the correct CVE identifier and link to the GitHub advisory (GHSA-869p-cjfg-cm3x) for clarity.

Comment on lines +118 to 120
"sentry-testkit>express": "npm:[email protected]",
"jws": "3.2.3"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add documentation explaining the security fix.

The PR objectives indicate that documentation updates are incomplete. For a security vulnerability (CVE-2023-35916 - improper HMAC signature verification in node-jws), add documentation describing the vulnerability and how the jws 3.2.3 override mitigates it.

Where should the security fix documentation be added (e.g., a SECURITY.md file, the README, or a docs directory)?

🤖 Prompt for AI Agents
In package.json around lines 118 to 120, the PR updated jws to 3.2.3 as a
security override but lacks documentation; add a SECURITY.md at the repository
root (and a short pointer in README.md) that briefly describes CVE-2023-35916
(improper HMAC signature verification), lists affected versions, states that we
override/lock jws to 3.2.3 to mitigate the issue, shows the exact package.json
override snippet and upgrade/remediation steps for users, includes
references/links to the CVE and upstream patch, the PR/commit that applied the
change, and a date + contact for incident questions so consumers can verify the
fix and upgrade guidance.

⚠️ Potential issue | 🟠 Major

Add a CHANGELOG entry for the security vulnerability fix.

The PR objectives indicate that the CHANGELOG entry is incomplete. A security vulnerability fix (CVE-2023-35916) must be documented in CHANGELOG.md. Based on learnings, use the format: [PR 2203](https://github.com/shakacode/react_on_rails/pull/2203) by [Seifeldin7](https://github.com/Seifeldin7) followed by a description of the fix.

Would you like me to generate the CHANGELOG entry following the documented format?

🤖 Prompt for AI Agents
In package.json around lines 118 to 120, the PR omitted a CHANGELOG entry
documenting the security vulnerability fix (CVE-2023-35916); add a new entry to
CHANGELOG.md using the prescribed format: include the PR link and author link
like `[PR 2203](https://github.com/shakacode/react_on_rails/pull/2203) by
[Seifeldin7](https://github.com/Seifeldin7)` followed by a concise sentence
describing the fix for CVE-2023-35916 (e.g., dependency update or patch
applied), place it under the appropriate unreleased or version heading, and save
the file.

"sentry-testkit>body-parser": "npm:[email protected]",
"sentry-testkit>express": "npm:[email protected]"
"sentry-testkit>express": "npm:[email protected]",
"jws": "3.2.3"
Copy link
Collaborator

@alexeyr-ci2 alexeyr-ci2 Dec 10, 2025

Choose a reason for hiding this comment

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

@Seifeldin7 Keeping it as an override means that even if jsonwebtoken is updated in a later PR and wants a later jws version, it won't get updated automatically. Instead, my suggestion in such a case is to use overrides to update the lockfile and then restore the original package.json. The new lockfile should still satisfy it.

You could make the override something like >=3.2.3 but I think that has its own problems.

If overrides influenced our clients' resolution, it would be different, but I believe they don't.

@claude
Copy link

claude bot commented Dec 11, 2025

Code Review - PR #2203

Thank you for addressing this security vulnerability! This is an important fix. Here's my comprehensive review:


Security Fix - Approved

Vulnerability: CVE-2023-35916 - Improper HMAC signature verification in jws package
Impact: HIGH severity - affects JWT signature validation in license verification
Fix: Force upgrade from jws 3.2.2 → 3.2.3 via pnpm override


📋 Code Quality Assessment

Strengths

  1. Correct approach: Using pnpm overrides is the right way to force a transitive dependency upgrade
  2. Minimal change: Only touches what's necessary (package.json and pnpm-lock.yaml)
  3. CI passed: All 30+ CI checks are green ✓
  4. Proper scope: Fixes vulnerability in jsonwebtoken dependency used by react-on-rails-pro-node-renderer

⚠️ Issues to Address

1. Missing PR Checklist Items (from PR template)

  • Tests: No new tests added (acceptable for dependency bump, but consider verifying license validation still works)
  • Documentation: No docs updated (not critical for security patch)
  • CHANGELOG: ❗ REQUIRED - Must add entry to CHANGELOG_PRO.md

2. PR Description Incomplete
The PR template sections weren't filled out:

  • Remove placeholder text about "bugs you had fixed"
  • Add brief explanation of CVE-2023-35916 impact
  • Mention this affects Pro package's license validation system

🔍 Technical Analysis

Dependency Chain

react-on-rails-pro-node-renderer
  └─ jsonwebtoken@^9.0.2 
      └─ [email protected] (vulnerable) → 3.2.3 (fixed)

Affected Code

  • packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts (line 1)
  • Uses jsonwebtoken for JWT verification of Pro licenses
  • Vulnerability could theoretically allow signature bypass (though actual exploit risk is low given controlled use case)

Override Strategy

The pnpm override in root package.json forces ALL workspace packages to use [email protected]:

"pnpm": {
  "overrides": {
    "jws": "3.2.3"
  }
}

This is better than updating jsonwebtoken directly because:

  • No breaking changes to main dependency
  • Fixes vulnerability at source
  • Will persist until jsonwebtoken updates its own dependency

📝 Required Actions Before Merge

CRITICAL - Add CHANGELOG entry:

Per CLAUDE.md guidelines, add to CHANGELOG_PRO.md (not CHANGELOG.md since this affects Pro package):

### [Unreleased]

#### Security
- [PR 2203](https://github.com/shakacode/react_on_rails/pull/2203) by [Seifeldin7](https://github.com/Seifeldin7): Fix HIGH severity jws vulnerability (CVE-2023-35916) in jsonwebtoken dependency used for license validation. Force upgrade to [email protected] via pnpm override.

Format requirements (per CLAUDE.md):

  • No hash in PR number ✓
  • Use "by username" format ✓
  • Place in "Security" section under "Unreleased"

🎯 Optional Enhancements (not blockers)

  1. Verification test: Consider adding a test that verifies license validation still works after the dependency update (though existing tests likely cover this)

  2. Version pinning consideration: Using "jws": "3.2.3" (exact version) vs "jws": "^3.2.3" (allow patches)

    • Current approach (exact) is more conservative ✓
    • Consider allowing patches: "jws": "^3.2.3" to get future security fixes automatically
  3. Security advisory reference: Could add link to advisory in commit message for future reference


🚀 Performance & Compatibility

  • ✅ No performance impact (patch-level dependency update)
  • ✅ No breaking changes
  • ✅ Compatible with all supported Node/Ruby versions
  • ✅ CI confirms no test failures across all matrices

🔒 Security Considerations

Risk Assessment: ✅ LOW

  • Vulnerability affects signature verification in jws library
  • In this codebase: Only used for internal license validation
  • Not directly exposed to user input
  • Still important to patch as defense-in-depth

Fix Verification: ✅ CORRECT


Final Recommendation

APPROVED pending CHANGELOG update.

Once you add the CHANGELOG_PRO.md entry, this is ready to merge. Great work catching and fixing this vulnerability!


📚 References

  • CVE-2023-35916: Improper HMAC signature verification in auth0/node-jws
  • Affected code: licenseValidator.ts:1
  • CLAUDE.md guidelines: /CLAUDE.md#changelog

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