Update licence public key #1978
Conversation
WalkthroughLicense key rotation applied across the React on Rails Pro codebase: updated GitHub Actions workflows to reference Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
PR Review: Update licence public keySummaryThis PR updates the RSA public key used for React on Rails Pro license verification across both the Ruby gem and Node.js package, and updates GitHub Actions workflows to use a new license secret ( Code Quality & Best PracticesStrengths:
Areas for Improvement:
Security ConcernsCritical Questions:
Security Best Practices:
Potential Bugs & IssuesHigh Priority:
Recommendation:
Test CoverageConcerns:
Recommended Tests: # Add to license_validator_spec.rb
describe "public key validation" do
it "has a valid RSA public key" do
expect(ReactOnRailsPro::LicensePublicKey::KEY).to be_a(OpenSSL::PKey::RSA)
expect(ReactOnRailsPro::LicensePublicKey::KEY.public?).to be true
end
it "has 2048-bit key size" do
key_size = ReactOnRailsPro::LicensePublicKey::KEY.n.num_bits
expect(key_size).to eq(2048)
end
endPerformance Considerations
CHANGELOG RequirementsAccording to
However, if existing users need to take action (get new licenses), a CHANGELOG entry would be appropriate. Action ItemsBefore Merging:
Post-Merge:
Overall AssessmentRisk Level: 🟡 Medium-High This is a straightforward but critical security infrastructure change. The code changes themselves are correct, but the coordination between private key, public key, issued licenses, and CI secrets is complex and error-prone. Recommendation: Approve with conditions - ensure all action items above are addressed, particularly verification of the GitHub secret and coordination of license re-issuance. Review generated following repository guidelines in CLAUDE.md |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb (1)
8-8: Consider updating the timestamp comment.The "Last updated" date (2025-10-09) predates this PR by about a month. If this represents when the key was rotated into the codebase, consider updating it to reflect the actual update date.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
react_on_rails_pro/spec/dummy/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/pro-integration-tests.yml(3 hunks).github/workflows/pro-lint.yml(1 hunks).github/workflows/pro-package-tests.yml(3 hunks)react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb(1 hunks)react_on_rails_pro/packages/node-renderer/src/shared/licensePublicKey.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
.github/workflows/pro-integration-tests.yml
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build
- GitHub Check: lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: claude-review
🔇 Additional comments (6)
.github/workflows/pro-lint.yml (1)
39-39: LGTM! Secret rotation applied consistently.The update to use
REACT_ON_RAILS_PRO_LICENSE_V2aligns with the license key rotation across the repository..github/workflows/pro-package-tests.yml (1)
40-40: LGTM! Consistent secret rotation across all jobs.All three jobs (
build-dummy-app-webpack-test-bundles,package-js-tests, andrspec-package-specs) correctly reference the updatedREACT_ON_RAILS_PRO_LICENSE_V2secret.Also applies to: 141-141, 214-214
react_on_rails_pro/packages/node-renderer/src/shared/licensePublicKey.ts (1)
12-18: Public key rotation looks correct.The TypeScript public key has been updated consistently with the Ruby version. The verification script in the previous file will confirm they match exactly.
.github/workflows/pro-integration-tests.yml (2)
40-40: LGTM! Secret rotation applied across all integration test jobs.All three jobs (
build-dummy-app-webpack-test-bundles,rspec-dummy-app-node-renderer, anddummy-app-node-renderer-e2e-tests) correctly reference the updatedREACT_ON_RAILS_PRO_LICENSE_V2secret.Also applies to: 130-130, 310-310
436-436: Clarify the timeout increase rationale.The Rails server startup timeout has been increased from 60 to 300 seconds (5x increase) in the E2E test job. This seems unrelated to the license key rotation. Can you clarify:
- Was this change intentional or accidentally bundled?
- If intentional, what issue does it address?
- Should the timeout in the
rspec-dummy-app-node-rendererjob (line 245) also be increased for consistency?react_on_rails_pro/lib/react_on_rails_pro/license_public_key.rb (1)
20-26: Public key rotation verified and consistent across both implementations.The new RSA public key is properly formatted and matches between the Ruby and TypeScript versions, confirming the key rotation is complete and synchronized.
| if: github.ref == 'refs/heads/master' || needs.detect-changes.outputs.run_pro_tests == 'true' | ||
| runs-on: ubuntu-22.04 | ||
| env: | ||
| REACT_ON_RAILS_PRO_LICENSE: ${{ secrets.REACT_ON_RAILS_PRO_LICENSE }} |
There was a problem hiding this comment.
I just updated the secret name, I don't need to change the licence and at the old secret and PRs that use it have a failure on CI
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 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.
This change is
Summary by CodeRabbit