-
-
Notifications
You must be signed in to change notification settings - Fork 638
Fix licensing vulnerabilities and strengthen freemium model legal protections #1823
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
Fix licensing vulnerabilities and strengthen freemium model legal protections #1823
Conversation
…tion requirements - Enhanced the LICENSE.md to specify that Pro-licensed directories require a valid subscription and outline prohibitions against circumventing license validation. - Updated package.json to reflect a dual license of MIT and Proprietary. - Revised REACT-ON-RAILS-PRO-LICENSE.md to include new clauses regarding license validation and compliance. - Modified DIRECTORY_LICENSING.md to clarify the nature of Pro features in the repository. - Adjusted LICENSING_FAQ.md to accurately describe the licensing structure and requirements for Pro features.
Warning Rate limit exceeded@AbanoubGhadban has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughSplit repository licensing into MIT vs Pro scopes in LICENSE.md; introduced SPDX/Pro license marker and Pro directories; upgraded REACT-ON-RAILS-PRO-LICENSE.md to v2.1 with expanded restrictions, audits, and governance sections; updated docs to mark Pro files; adjusted package.json license field to reference LICENSE.md. Changes
Sequence Diagram(s)(omitted — changes are documentation and license text only; no control-flow or runtime changes to diagram) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Code Review FeedbackCritical Legal & Technical Issues1. LICENSE.md - MIT License Conflict Recommendation: MIT-licensed code cannot include anti-circumvention clauses. Consider moving interface files to Pro-licensed directories OR restructuring the licensing model. Consult with IP counsel on dual-licensing enforceability. 2. package.json License Field - Invalid SPDX Expression Recommendation: Change to "SEE LICENSE IN LICENSE.md" 3. Legal Enforceability Concerns Recommendation: Consider actual technical protection measures rather than license-based restrictions on MIT code. Best Practices & Code Quality4. Documentation Inconsistencies 5. Version Bumping Without Changelog Security & Performance6. No Actual Security Improvements 7. Missing Test Coverage Minor Issues8. Formatting
9. Redundant Language Positives
Recommended Actions Before Merging
Rating: Needs significant revision - Legal contradictions must be resolved before merge. |
There was a problem hiding this 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
📒 Files selected for processing (5)
LICENSE.md
(1 hunks)REACT-ON-RAILS-PRO-LICENSE.md
(4 hunks)docs/DIRECTORY_LICENSING.md
(1 hunks)docs/LICENSING_FAQ.md
(1 hunks)package.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
package.json
docs/LICENSING_FAQ.md
docs/DIRECTORY_LICENSING.md
LICENSE.md
REACT-ON-RAILS-PRO-LICENSE.md
**/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all non-Ruby file formatting; never manually format these files
Files:
package.json
docs/LICENSING_FAQ.md
docs/DIRECTORY_LICENSING.md
LICENSE.md
REACT-ON-RAILS-PRO-LICENSE.md
🧠 Learnings (5)
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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.
Applied to files:
docs/LICENSING_FAQ.md
docs/DIRECTORY_LICENSING.md
LICENSE.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
docs/LICENSING_FAQ.md
docs/DIRECTORY_LICENSING.md
LICENSE.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/DIRECTORY_LICENSING.md
LICENSE.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#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:
docs/DIRECTORY_LICENSING.md
LICENSE.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
PR: shakacode/react_on_rails#1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
LICENSE.md
⏰ 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). (9)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: claude-review
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: build
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: markdown-link-check
🔇 Additional comments (4)
docs/LICENSING_FAQ.md (1)
98-99
: Clearer gating note LGTM.The clarification that Pro features remain gated behind license validation keeps the FAQ aligned with the strengthened licensing language in LICENSE.md.
REACT-ON-RAILS-PRO-LICENSE.md (2)
55-58
: Anti-circumvention coverage looks solid.Explicitly prohibiting tampering with license validation closes the loophole that let MIT consumers shim Pro APIs, which is exactly what we needed for the freemium protections.
92-94
: Audit escalation clause checks out.Section 9.1 adds the necessary audit teeth without overreaching—the cost recovery trigger on material violations is a good balance.
docs/DIRECTORY_LICENSING.md (1)
15-23
: Directory exception note is consistent.Calling out
node_package/src/pro/
as bundled-but-gated keeps the structure doc in sync with the updated FAQ and license.
package.json
Outdated
], | ||
"author": "[email protected]", | ||
"license": "MIT", | ||
"license": "(MIT AND Proprietary)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid SPDX expression in license
field
(MIT AND Proprietary)
is not a valid SPDX expression—Proprietary
is not on the SPDX list and will cause npm pack
/npm publish
to error out. Please switch to a valid SPDX expression (for example, use a LicenseRef-
identifier) so the package metadata remains compliant.
- "license": "(MIT AND Proprietary)",
+ "license": "MIT AND LicenseRef-Proprietary",
Remember to define LicenseRef-Proprietary
in the referenced license file per SPDX guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"license": "(MIT AND Proprietary)", | |
"license": "MIT AND LicenseRef-Proprietary", |
🤖 Prompt for AI Agents
In package.json at line 121, the current license "(MIT AND Proprietary)" is not
a valid SPDX expression; replace it with a valid SPDX expression such as "MIT
AND LicenseRef-Proprietary" in package.json and then add or update your
repository LICENSE file to include a "LicenseRef-Proprietary" section detailing
the proprietary terms (and an SPDX-License-Identifier header if appropriate) so
the LicenseRef identifier is defined per SPDX guidelines.
- Added a new section in LICENSE.md detailing the proprietary license, usage restrictions, and directories associated with Pro features. - Updated package.json to specify the proprietary license using the SPDX identifier format.
Code Review: PR #1823 - Licensing Vulnerabilities and Legal ProtectionsOverviewThis PR updates licensing documentation and legal terms. The changes are documentation-only with no functional code changes, which minimizes technical risk. ✅ Strengths1. Clear Documentation Structure
2. Legal Completeness
3. Alignment Across Files
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
LICENSE.md
(2 hunks)package.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
package.json
LICENSE.md
**/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all non-Ruby file formatting; never manually format these files
Files:
package.json
LICENSE.md
🧠 Learnings (4)
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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.
Applied to files:
package.json
LICENSE.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
LICENSE.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
LICENSE.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#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:
LICENSE.md
🪛 markdownlint-cli2 (0.18.1)
LICENSE.md
8-8: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (8)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: markdown-link-check
- GitHub Check: build
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
🔇 Additional comments (2)
package.json (1)
121-121
: SPDX expression looks good
MIT AND LicenseRef-Proprietary
is a valid SPDX expression and aligns with the new proprietary block inLICENSE.md
. 👍LICENSE.md (1)
34-34
: Reword anti-circumvention note to avoid contradicting MIT rightsBy saying circumvention “constitutes a violation of this license,” we’re effectively claiming the MIT portion forbids conduct that the MIT grant expressly allows, which introduces a compliance conflict. Please scope the prohibition purely to the Pro-licensed code (or refer to contractual terms) so the MIT-licensed files remain unrestricted under MIT.
@claude suggest the changes that will make if you will implement MIT LICENSE applies to lib/react_on_rails/** (except pro/) and node_package/src/** (except pro/) |
Claude finished @AbanoubGhadban's task —— View job Implementation Complete ✅I've successfully implemented the alternative approach for cleaner license separation. Changes Made:1. LICENSE.md - Restructured for Clear Separation
2. DIRECTORY_LICENSING.md - Added Important Distinction
3. LICENSING_FAQ.md - Enhanced with New FAQ
Key Legal Improvement:This approach resolves the fundamental legal contradiction in the previous version where:
This maintains strong legal protection for Pro features while honoring the MIT license's grant of modification rights. Commit:
Branch: |
Implements cleaner license separation approach: - MIT License applies to lib/react_on_rails/** (except pro/) and node_package/src/** (except pro/) - Pro License applies exclusively to pro/ directories - Removes contradictory language about prohibiting MIT code modification - Clarifies that MIT code can be modified, but using it to access Pro features without a license is prohibited Changes: - LICENSE.md: Restructured to clearly define license scopes upfront - DIRECTORY_LICENSING.md: Added distinction between MIT modification rights and Pro usage restrictions - LICENSING_FAQ.md: Added FAQ explaining modification rights vs usage restrictions This resolves the legal contradiction where MIT-licensed code included anti-circumvention restrictions, which conflicts with MIT's grant of unrestricted modification rights. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Abanoub Ghadban <[email protected]>
Code Review: PR #1823 - Licensing Vulnerabilities and Legal Protection Updates Executive Summary: This PR strengthens the freemium model legal framework by clarifying dual licensing, adding anti-circumvention clauses, and enhancing audit provisions. STRENGTHS: CRITICAL ISSUES:
CODE QUALITY ASSESSMENT: RECOMMENDATIONS SUMMARY: Must Fix:
Should Consider:
Nice to Have:
CONCLUSION: This PR significantly strengthens the legal framework for the freemium model. Documentation is clear and comprehensive. However, package.json license field needs correction to avoid NPM ecosystem issues, and several legal clauses would benefit from attorney review to ensure enforceability and avoid unintended consequences. Recommendation: ✅ Approve with changes (fix package.json, consider legal review) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
LICENSE.md (1)
1-70
: LICENSE.md missing trailing newline Add a newline at end of file per repo guidelines and re-run Prettier to format.
🧹 Nitpick comments (3)
docs/DIRECTORY_LICENSING.md (2)
11-20
: Add language hints to fenced directory treesMarkdown lint typically requires a language tag; add
text
to these fences.-``` +```text react_on_rails/ ├── lib/react_on_rails/ # Core Ruby code (MIT) ├── node_package/src/ # Core JS/TS code (MIT) │ └── pro/ # Pro features with license validation (Pro licensed) ├── spec/ # Core tests (MIT) ├── docs/ # Documentation (MIT) ├── .github/ # GitHub workflows (MIT) └── [all other directories] # MIT -``` +```-``` +```text react_on_rails_pro/ ├── lib/react_on_rails_pro/ # Pro Ruby code ├── packages/node-renderer/ # Pro Node.js renderer ├── spec/ # Pro tests ├── .circleci/ # CircleCI config └── [all other directories] # Pro licensed -``` +```-``` +```text react_on_rails/ (monorepo root) ├── lib/react_on_rails/ # Core Ruby code ├── packages/react-on-rails/ # Core NPM package ├── spec/ruby/react_on_rails/ # Core Ruby tests ├── spec/packages/react-on-rails/ # Core JS tests ├── docs/ # Shared documentation ├── tools/ # Shared development tools ├── .github/ # Unified GitHub workflows └── [shared config files] # Build configs, etc. -``` +```-``` +```text react_on_rails/ (monorepo root) ├── lib/react_on_rails_pro/ # Pro Ruby code ├── packages/react-on-rails-pro/ # Pro NPM package ├── packages/react-on-rails-pro-node-renderer/ # Pro Node renderer ├── spec/ruby/react_on_rails_pro/ # Pro Ruby tests ├── spec/packages/react-on-rails-pro/ # Pro JS tests └── spec/packages/react-on-rails-pro-node-renderer/ # Pro Node renderer tests -``` +```Also applies to: 33-40, 49-58, 63-70
9-16
: Avoid “All directories are MIT” phrasing that’s contradicted by the exceptionReword to reduce confusion and align with the exception below.
-All directories in the `react_on_rails` repository are MIT licensed: +Most directories in the `react_on_rails` repository are MIT licensed. The Pro-licensed exceptions are noted below:docs/LICENSING_FAQ.md (1)
29-39
: Add language hint to fenced directory treeAdd
text
to satisfy markdownlint and keep formatting consistent with LICENSE.md/DIRECTORY_LICENSING.md.-``` +```text react_on_rails/ (monorepo root) ├── lib/ │ ├── react_on_rails/ # MIT Licensed │ └── react_on_rails_pro/ # Pro Licensed ├── packages/ │ ├── react-on-rails/ # MIT Licensed │ ├── react-on-rails-pro/ # Pro Licensed │ └── react-on-rails-pro-node-renderer/ # Pro Licensed └── LICENSE.md # Documents which directories use which license -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
LICENSE.md
(2 hunks)docs/DIRECTORY_LICENSING.md
(1 hunks)docs/LICENSING_FAQ.md
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
LICENSE.md
docs/DIRECTORY_LICENSING.md
docs/LICENSING_FAQ.md
**/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all non-Ruby file formatting; never manually format these files
Files:
LICENSE.md
docs/DIRECTORY_LICENSING.md
docs/LICENSING_FAQ.md
🧠 Learnings (4)
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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.
Applied to files:
LICENSE.md
docs/DIRECTORY_LICENSING.md
docs/LICENSING_FAQ.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/DIRECTORY_LICENSING.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#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:
docs/DIRECTORY_LICENSING.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
docs/DIRECTORY_LICENSING.md
🪛 GitHub Actions: Lint JS and Ruby
docs/DIRECTORY_LICENSING.md
[error] 1-1: Prettier formatting check failed. File has formatting issues. Run 'prettier --write' to fix. Command: prettier --check .
⏰ 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). (8)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: markdown-link-check
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: claude-review
🔇 Additional comments (8)
docs/DIRECTORY_LICENSING.md (3)
22-27
: LGTM: MIT-interface vs Pro usage distinction is clear and consistentThis matches the requested separation: MIT permits modification; Pro usage without license remains prohibited.
Please also ensure the same distinction appears (verbatim or close) in LICENSE.md and LICENSING_FAQ.md for consistency.
1-140
: Format docs/DIRECTORY_LICENSING.md with Prettier and ensure trailing newline
Runnpx prettier --write docs/DIRECTORY_LICENSING.md
and confirm the file ends with a single trailing newline.
140-140
: Referenced document exists; no update needed. The filedocs/MONOREPO_MERGER_PLAN_REF.md
is present and the relative link indocs/DIRECTORY_LICENSING.md
is valid.LICENSE.md (2)
20-23
: LGTM: SPDX block uses a language hint and avoids markdownlint errorsThe fenced SPDX block is now annotated with
text
and looks good.
73-77
: Good: MIT modification rights preserved; Pro misuse called outWording avoids restricting MIT rights while clarifying Pro usage limits.
docs/LICENSING_FAQ.md (3)
98-100
: LGTM: Clear separation of MIT modification rights vs Pro usage restrictionsThis Q&A mirrors the agreed approach and aligns with LICENSE.md.
Also applies to: 111-111, 113-120
1-135
: Run Prettier via npx and ensure trailing newline
Runnpx prettier --write docs/LICENSING_FAQ.md
(or yournpm run format
script) and commit the changes. Add a newline at the end of the file.
78-79
: Verified referenced docs exist and links resolve
MONOREPO_MERGER_PLAN_REF.md and REACT-ON-RAILS-PRO-LICENSE.md are present at expected paths; relative links in docs/LICENSING_FAQ.md resolve correctly.
The following directories and all their contents are licensed under the **MIT License** (see full text below): | ||
|
||
## MIT License for Core React on Rails | ||
- `lib/react_on_rails/` (excluding `lib/react_on_rails/pro/`) | ||
- `node_package/src/` (excluding `node_package/src/pro/`) | ||
- All other directories in this repository not explicitly listed as Pro-licensed | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Consistency checks: align metadata and license text with docs
- Confirm package.json license expression is acceptable to npm tooling.
- Ensure Pro license text actually contains the “education/personal/non-production” allowance and anti-circumvention language referenced here.
Also applies to: 24-31, 66-77
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect package.json license fields across the repo
rg -n --type=json -H '"license"\s*:\s*' -C1
# 2) Validate that the Pro license file exists and includes key terms
fd -a '^REACT-ON-RAILS-PRO-LICENSE\.md$' -x realpath
rg -n --heading -C2 -e 'educational|classroom|personal|non-?production|evaluation' \
-e 'anti[- ]?circumvent|circumvention|bypass|reverse engineer' \
REACT-ON-RAILS-PRO-LICENSE.md
# 3) Cross-check that directories listed here match docs/DIRECTORY_LICENSING.md
echo "Dirs listed in LICENSE.md (MIT vs Pro):"
rg -n 'lib/react_on_rails/pro/|node_package/src/pro/' LICENSE.md
rg -n 'lib/react_on_rails/|node_package/src/' LICENSE.md
echo "Compare with DIRECTORY_LICENSING.md:"
rg -n 'lib/react_on_rails/pro/|node_package/src/pro/' docs/DIRECTORY_LICENSING.md
rg -n 'lib/react_on_rails/|node_package/src/' docs/DIRECTORY_LICENSING.md
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) List Pro license files in the repo
echo "Pro license files:"
fd -e md -g '*PRO-LICENSE.md'
# 2) Search for educational/personal/non-production and anti-circumvention terms in each Pro license file
echo
echo "Key terms in Pro license:"
for file in $(fd -e md -g '*PRO-LICENSE.md'); do
echo "== $file =="
rg -n --heading -C2 -e 'educational|classroom|personal|non-?production|evaluation' \
-e 'anti[- ]?circumvent|circumvention|bypass|reverse engineer' "$file" || echo " (no matches)"
echo
done
# 3) Verify LICENSE.md directory listings
echo "LICENSE.md directory entries:"
rg -n -C1 'lib/react_on_rails' LICENSE.md
rg -n -C1 'node_package/src' LICENSE.md
# 4) Verify docs/DIRECTORY_LICENSING.md directory listings
echo
echo "docs/DIRECTORY_LICENSING.md directory entries:"
rg -n -C1 'lib/react_on_rails' docs/DIRECTORY_LICENSING.md
rg -n -C1 'node_package/src' docs/DIRECTORY_LICENSING.md
Length of output: 3429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) License field context in package.json:"
rg -n -C2 '"license"\s*:' package.json || echo "license field not found"
echo
echo "2) Presence of custom license file reference:"
rg -n '"licenseFile"\s*:' package.json || echo "no \"licenseFile\" key"
rg -n '"licenseFilename"\s*:' package.json || echo "no \"licenseFilename\" key"
Length of output: 321
Sync package.json and LICENSE.md with SPDX expression and docs
- Add
"licenseFile": "REACT-ON-RAILS-PRO-LICENSE.md"
to package.json so npm tooling resolvesLicenseRef-Proprietary
. - In LICENSE.md’s Pro section, replace
lib/react_on_rails/pro/
withlib/react_on_rails_pro/
and addpackages/node-renderer/
andpackages/react-on-rails-pro/
to match docs/DIRECTORY_LICENSING.md.
License validation mechanisms include but are not limited to:
|
Claude finished @AbanoubGhadban's task —— View job Implementation Complete ✅Tasks Completed:
Changes Made:1. package.json
2. LICENSE.md
Commit:
Branch: |
…xamples - Changed package.json license field to 'SEE LICENSE IN LICENSE.md' for better npm tooling compatibility - Added 'License Validation Mechanisms' section to LICENSE.md with: - Description of what license validation mechanisms are (runtime checks, authentication systems, etc.) - Specific examples of prohibited modifications (helper.rb, utils.rb, interface files) - Clear statement that MIT modifications are allowed but using them to access Pro features violates the Pro License Co-authored-by: Abanoub Ghadban <[email protected]>
Code Review - PR #1823This PR strengthens the legal framework with important additions. However, I've identified several issues: Critical Issues1. Invalid package.json License FieldLocation: package.json:121 2. Potential MIT License ConflictLocation: LICENSE.md:75-76 Medium Priority Issues3. Hardcoded File ReferencesLocation: LICENSE.md:88-91 4. No License Version ChangelogVersion bumped from 2.0 to 2.1 without documenting what changed. Organizations need to track legal obligation changes. 5. Undefined "Material" in Audit Costs"Material unauthorized use" is undefined and could lead to disputes. Recommend defining threshold (e.g., exceeding 10% of capacity or 000). Positive Aspects
Recommendations
The direction is sound and additions are professional. Would you like me to propose specific fixes? |
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
88-90
: Include Pro license files in published npm package
Add both LICENSE.md and REACT-ON-RAILS-PRO-LICENSE.md to thefiles
array in package.json to ensure the Pro license text is shipped and avoid broken references:"files": [ - "node_package/lib" + "node_package/lib", + "LICENSE.md", + "REACT-ON-RAILS-PRO-LICENSE.md" ],
🧹 Nitpick comments (1)
package.json (1)
121-121
: License field change is acceptable; verify tooling compatibilitySwitching to "SEE LICENSE IN LICENSE.md" resolves the invalid SPDX issue and is accepted by npm. Please verify that publint and your release pipeline don’t warn on this format. If you need SPDX compatibility, consider the alternative below.
Alternative SPDX option (only if required by your tooling):
- "license": "SEE LICENSE IN LICENSE.md", + "license": "MIT AND LicenseRef-Proprietary",If you adopt the SPDX option, add an SPDX marker to REACT-ON-RAILS-PRO-LICENSE.md (it likely already exists) and keep LICENSE.md as the canonical explainer.
Run locally to confirm no warnings:
- npx publint
- npm pack --dry-run
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
LICENSE.md
(2 hunks)package.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
LICENSE.md
package.json
**/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all non-Ruby file formatting; never manually format these files
Files:
LICENSE.md
package.json
🧠 Learnings (3)
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#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.
Applied to files:
LICENSE.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
LICENSE.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
package.json
⏰ 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). (9)
- GitHub Check: dummy-app-integration-tests (3.4, 22)
- GitHub Check: dummy-app-integration-tests (3.2, 20)
- GitHub Check: claude-review
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: markdown-link-check
- GitHub Check: build
🔇 Additional comments (6)
LICENSE.md (6)
3-7
: Clear dual-licensing intro — LGTM
20-22
: Pro SPDX marker block — LGTM
29-33
: Pro usage statement — LGTM
80-85
: License validation mechanisms list — LGTM
86-94
: Prohibited modifications + MIT clarification — LGTM; keep this orderingExamples are concrete and the concluding clarification resolves prior MIT-rights conflict.
Optionally add the same SPDX marker (“SPDX-License-Identifier: LicenseRef-Proprietary”) at the top of REACT-ON-RAILS-PRO-LICENSE.md for tooling consistency.
71-77
: Confirmed free-use allowances match Pro license
REACT-ON-RAILS-PRO-LICENSE.md includes the same educational, personal, and non-production use provisions, so terms are aligned.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@claude fixing package.json licence issue |
Claude finished @AbanoubGhadban's task —— View job Package.json License Issue - Already Fixed ✅Summary:The package.json license field issue has already been resolved in commit Current Status:
What Was Fixed:
No further action needed - the issue identified in code reviews has been fully addressed. Branch: |
Code Review: Licensing Vulnerability Fixes and Legal ProtectionsI've reviewed PR #1823 which addresses licensing vulnerabilities and strengthens the freemium model's legal protections. ✅ Strengths1. Comprehensive Legal Coverage
2. Clear Documentation
3. Correct package.json License Field
4. Formatting Compliance
|
|
5. Redundancy in LICENSE.md Lines 80-94 list License Validation Mechanisms and Examples of prohibited modifications in great detail. This level of specificity might:
Recommendation: Consider more general language like 'any attempt to circumvent license validation' rather than specific file examples. 🔍 Security Considerations1. Security Through Obscurity This PR attempts to prevent license circumvention through legal threats rather than technical measures. This is fundamentally security through obscurity, which is generally considered weak. Recommendation: Consider:
2. Threat Model The PR description mentions preventing hackers but the actual threat model seems to be legitimate users trying to avoid paying for Pro features, not external attackers or actual security vulnerabilities. Recommendation: Be clear about the threat model - this is anti-piracy, not security hardening. |
🎯 Final Recommendations
✅ Approval StatusConditionally Approve pending:
The changes are well-intentioned and improve license clarity, but some legal and practical considerations need addressing before merging. 📊 Summary
Note: I'm providing technical and structural feedback. For legal advice, please consult with a licensed attorney. |
…line the document and focus on the core licensing terms.
Code Review - PR #1823SummaryThis PR addresses licensing concerns for the freemium model. Important intent, but several issues need resolution before merging. Critical Issues1. package.json License Field - NPM Standards Violation (line 121) Current: "SEE LICENSE IN LICENSE.md" NPM tooling expects valid SPDX identifiers. The current format is deprecated and may cause package manager warnings. 2. Legal Language Enforceability Concerns (LICENSE.md:84-89) The PR lists MIT-licensed files (clientStartup.ts, etc.) then prohibits their modification. This contradicts MIT license terms which explicitly grant unrestricted modification rights. Legal Risk: Cannot add restrictions to MIT code that contradict the MIT license itself. Recommendation: Consult IP counsel, or move interface files to Pro license, or rely solely on Pro directory protection. 3. Missing Trailing Newline (LICENSE.md:92) File lacks trailing newline - violates CLAUDE.md requirement and will fail CI. Major Concerns4. No Test Coverage 5. Overly Broad Anti-Circumvention (REACT-ON-RAILS-PRO-LICENSE.md:55-58) 6. Audit Rights Undefined Terms (REACT-ON-RAILS-PRO-LICENSE.md:92-93)
Strengths
Pre-Merge Checklist
RecommendationDO NOT MERGE until critical issues resolved:
Generated with Claude Code |
Code Review: Licensing UpdatesI've reviewed PR #1823 and have the following feedback: ✅ Strengths
|
|
||
- **MIT-licensed interface files** (outside `pro/` directories) can be freely modified under MIT terms | ||
- **Using those modifications to access Pro features** without a license violates the Pro License | ||
- **Pro-licensed files** (inside `pro/` directories) require a Pro license to use in any way |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"in any way."
@claude can we change this to add:
"Note, ShakaCode offers FREE licenses for educational and demonstration purposes."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude finished @justin808's task —— View job • claude/pr-1823-20250925-2106
• [Create PR ➔](https://github.com/shakacode/react_on_rails/compare/master...claude/pr-1823-20250925-2106?quick_pull=1&title=PR%20%231823%3A%20Changes%20from%20Claude&body=This%20PR%20addresses%20pr%20%231823%0A%0AGenerated%20with%20%5BClaude%20Code%5D(https%3A%2F%2Fclaude.ai%2Fcode)
✅ Task Completed
- Read docs/DIRECTORY_LICENSING.md to understand context
- Add the educational/demonstration license note at line 28
- Commit and push the change
Summary
Successfully added the note about free educational and demonstration licenses to docs/DIRECTORY_LICENSING.md
at line 28. The text now reads:
Pro-licensed files (inside
pro/
directories) require a Pro license to use in any way. Note, ShakaCode offers FREE licenses for educational and demonstration purposes.
This clarification helps users understand that while Pro features require a license, ShakaCode offers free options for educational and demonstration use cases.
Branch: claude/pr-1823-20250925-2106
|
**A:** We maintain two separate repositories: | ||
|
||
- **react_on_rails** (MIT) - Core functionality, completely free except pro directories as stated in LICENSE.md | ||
- **react_on_rails** (MIT + Pro) - Core functionality is MIT-licensed and completely free. Pro features (in `pro/` directories) are Pro-licensed and require a subscription for production use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing period.
Add:
Note, ShakaCode offers FREE licenses for educational and demonstration purposes.
@AbanoubGhadban added a couple super minor comments. GREAT JOB! |
🚨 Critical Security & Legal Fixes
This PR addresses critical vulnerabilities in our freemium licensing model and strengthens legal protections against license circumvention.
🔒 Security Vulnerabilities Fixed
JavaScript Interface Protection:
clientStartup.ts
,serverRenderReactComponent.ts
,ReactOnRails.client.ts
,ReactOnRails.node.ts
Ruby License Validation Protection:
rails_context
androrPro
fieldReactOnRails::Utils.react_on_rails_pro?
methodlib/react_on_rails/helper.rb
,lib/react_on_rails/utils.rb
⚖️ Legal Enhancements
LICENSE.md:
REACT-ON-RAILS-PRO-LICENSE.md:
package.json:
"MIT"
to"(MIT AND Proprietary)"
📚 Documentation Updates
LICENSING_FAQ.md:
🛡️ Impact
Some legal provisions (like Indemnification) were copied from the old react_on_rails_pro license.
This PR makes our freemium model legally bulletproof against common hacking attempts.
This change is
Summary by CodeRabbit
Documentation
Chores
Notes