Skip to content

Commit 38d5408

Browse files
justin808claude
justin808
and
claude
committed
Address PR review feedback: fix security issues and enhance Pro features
- Fix XSS security issues in JS string interpolation by adding CSS.escape() - Add runtime license validation for Pro features (force loading, immediate hydration) - Implement package.json export controls to prevent Pro module exposure in OSS builds - Rename pro_features/ to pro/ directory for consistency across Ruby and JS - Add comprehensive test coverage for Pro license validation - Update knip configuration for new directory structure 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d8a4b94 commit 38d5408

File tree

7 files changed

+205
-18
lines changed

7 files changed

+205
-18
lines changed

‎knip.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,32 @@ const config: KnipConfig = {
66
'.': {
77
entry: [
88
'node_package/src/ReactOnRails.node.ts!',
9-
'node_package/src/ReactOnRailsRSC.ts!',
10-
'node_package/src/registerServerComponent/client.tsx!',
11-
'node_package/src/registerServerComponent/server.tsx!',
12-
'node_package/src/registerServerComponent/server.rsc.ts!',
13-
'node_package/src/wrapServerComponentRenderer/server.tsx!',
14-
'node_package/src/wrapServerComponentRenderer/server.rsc.tsx!',
15-
'node_package/src/RSCRoute.tsx!',
16-
'node_package/src/ServerComponentFetchError.ts!',
9+
'node_package/src/pro/ReactOnRailsRSC.ts!',
10+
'node_package/src/pro/registerServerComponent/client.tsx!',
11+
'node_package/src/pro/registerServerComponent/server.tsx!',
12+
'node_package/src/pro/registerServerComponent/server.rsc.ts!',
13+
'node_package/src/pro/wrapServerComponentRenderer/server.tsx!',
14+
'node_package/src/pro/wrapServerComponentRenderer/server.rsc.tsx!',
15+
'node_package/src/pro/RSCRoute.tsx!',
16+
'node_package/src/pro/ServerComponentFetchError.ts!',
17+
'node_package/src/pro/getReactServerComponent.server.ts!',
18+
'node_package/src/pro/transformRSCNodeStream.ts!',
19+
'node_package/src/loadJsonFile.ts!',
1720
'eslint.config.ts',
1821
],
1922
project: ['node_package/src/**/*.[jt]s{x,}!', 'node_package/tests/**/*.[jt]s{x,}'],
2023
babel: {
2124
config: ['node_package/babel.config.js'],
2225
},
23-
ignore: ['node_package/tests/emptyForTesting.js'],
26+
ignore: [
27+
'node_package/tests/emptyForTesting.js',
28+
// Pro features exported for external consumption
29+
'node_package/src/pro/streamServerRenderedReactComponent.ts:transformRenderStreamChunksToResultObject',
30+
'node_package/src/pro/streamServerRenderedReactComponent.ts:streamServerRenderedComponent',
31+
'node_package/src/pro/ServerComponentFetchError.ts:isServerComponentFetchError',
32+
'node_package/src/pro/RSCRoute.tsx:RSCRouteProps',
33+
'node_package/src/pro/streamServerRenderedReactComponent.ts:StreamingTrackers',
34+
],
2435
ignoreBinaries: [
2536
// Knip fails to detect it's declared in devDependencies
2637
'nps',

‎lib/react_on_rails/helper.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
require "react_on_rails/utils"
1212
require "react_on_rails/json_output"
1313
require "active_support/concern"
14-
require "react_on_rails/pro_features/helper"
14+
require "react_on_rails/pro/helper"
1515

1616
module ReactOnRails
1717
module Helper
1818
include ReactOnRails::Utils::Required
19-
include ReactOnRails::ProFeatures::Helper
19+
include ReactOnRails::Pro::Helper
2020

2121
COMPONENT_HTML_KEY = "componentHtml"
2222

‎lib/react_on_rails/pro_features/helper.rb‎ renamed to ‎lib/react_on_rails/pro/helper.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# */
1616

1717
module ReactOnRails
18-
module ProFeatures
18+
module Pro
1919
module Helper
2020
IMMEDIATE_HYDRATION_PRO_WARNING = "[REACT ON RAILS] The 'immediate_hydration' feature requires a " \
2121
"React on Rails Pro license. " \

‎node_package/src/pro/ClientSideRenderer.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ const IMMEDIATE_HYDRATION_PRO_WARNING =
3232
"[REACT ON RAILS] The 'immediate_hydration' feature requires a React on Rails Pro license. " +
3333
'Please visit https://shakacode.com/react-on-rails-pro to get a license.';
3434

35+
const FORCE_LOADING_PRO_WARNING =
36+
"[REACT ON RAILS] The 'force_loading' feature requires a React on Rails Pro license. " +
37+
'Please visit https://shakacode.com/react-on-rails-pro to get a license.';
38+
3539
async function delegateToRenderer(
3640
componentObj: RegisteredComponent,
3741
props: Record<string, unknown>,
@@ -73,7 +77,9 @@ class ComponentRenderer {
7377
this.domNodeId = domId;
7478
this.state = 'rendering';
7579
const el =
76-
typeof domIdOrElement === 'string' ? document.querySelector(`[data-dom-id=${domId}]`) : domIdOrElement;
80+
typeof domIdOrElement === 'string'
81+
? document.querySelector(`[data-dom-id="${CSS.escape(domId)}"]`)
82+
: domIdOrElement;
7783
if (!el) return;
7884

7985
const storeDependencies = el.getAttribute('data-store-dependencies');
@@ -294,7 +300,7 @@ export async function hydrateStore(storeNameOrElement: string | Element) {
294300
if (!storeRenderer) {
295301
const storeDataElement =
296302
typeof storeNameOrElement === 'string'
297-
? document.querySelector(`[${REACT_ON_RAILS_STORE_ATTRIBUTE}="${storeNameOrElement}"]`)
303+
? document.querySelector(`[${REACT_ON_RAILS_STORE_ATTRIBUTE}="${CSS.escape(storeNameOrElement)}"]`)
298304
: storeNameOrElement;
299305
if (!storeDataElement) {
300306
return;
@@ -306,8 +312,17 @@ export async function hydrateStore(storeNameOrElement: string | Element) {
306312
await storeRenderer.waitUntilHydrated();
307313
}
308314

309-
export const hydrateForceLoadedStores = () =>
310-
forAllElementsAsync(`[${REACT_ON_RAILS_STORE_ATTRIBUTE}][data-force-load="true"]`, hydrateStore);
315+
export const hydrateForceLoadedStores = () => {
316+
const railsContext = getRailsContext();
317+
const hasProLicense = railsContext?.rorPro;
318+
319+
if (!hasProLicense) {
320+
console.warn(FORCE_LOADING_PRO_WARNING);
321+
return Promise.resolve();
322+
}
323+
324+
return forAllElementsAsync(`[${REACT_ON_RAILS_STORE_ATTRIBUTE}][data-force-load="true"]`, hydrateStore);
325+
};
311326

312327
export const hydrateAllStores = () =>
313328
forAllElementsAsync(`[${REACT_ON_RAILS_STORE_ATTRIBUTE}]`, hydrateStore);

‎node_package/tests/proLicenseValidation.test.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/**
2+
* @jest-environment jsdom
3+
*/
4+
5+
import { hydrateForceLoadedStores } from '../src/pro/ClientSideRenderer.ts';
6+
import { getRailsContext } from '../src/context.ts';
7+
8+
// Mock the getRailsContext function
9+
jest.mock('../src/context.ts', () => ({
10+
getRailsContext: jest.fn(),
11+
}));
12+
13+
describe('Pro License Validation', () => {
14+
let consoleSpy;
15+
16+
beforeEach(() => {
17+
consoleSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
18+
document.body.innerHTML = '';
19+
});
20+
21+
afterEach(() => {
22+
consoleSpy.mockRestore();
23+
});
24+
25+
describe('hydrateForceLoadedStores', () => {
26+
it('should warn and return early when no Pro license is detected', async () => {
27+
getRailsContext.mockReturnValue({ rorPro: false });
28+
29+
const result = await hydrateForceLoadedStores();
30+
31+
expect(consoleSpy).toHaveBeenCalledWith(
32+
"[REACT ON RAILS] The 'force_loading' feature requires a React on Rails Pro license. " +
33+
'Please visit https://shakacode.com/react-on-rails-pro to get a license.',
34+
);
35+
expect(result).toBeUndefined();
36+
});
37+
38+
it('should proceed normally when Pro license is detected', () => {
39+
getRailsContext.mockReturnValue({ rorPro: true });
40+
41+
// Test that it doesn't warn when license is valid (no force-load elements present)
42+
const result = hydrateForceLoadedStores();
43+
44+
expect(consoleSpy).not.toHaveBeenCalled();
45+
expect(result).toBeDefined();
46+
});
47+
48+
it('should return early when no rails context is available', async () => {
49+
getRailsContext.mockReturnValue(null);
50+
51+
const result = await hydrateForceLoadedStores();
52+
53+
expect(consoleSpy).toHaveBeenCalledWith(
54+
"[REACT ON RAILS] The 'force_loading' feature requires a React on Rails Pro license. " +
55+
'Please visit https://shakacode.com/react-on-rails-pro to get a license.',
56+
);
57+
expect(result).toBeUndefined();
58+
});
59+
});
60+
});

‎package.oss.json‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
{
2+
"name": "react-on-rails",
3+
"version": "16.0.0",
4+
"description": "react-on-rails JavaScript for react_on_rails Ruby gem",
5+
"main": "node_package/lib/ReactOnRails.full.js",
6+
"type": "module",
7+
"exports": {
8+
".": {
9+
"node": "./node_package/lib/ReactOnRails.node.js",
10+
"default": "./node_package/lib/ReactOnRails.full.js"
11+
},
12+
"./client": "./node_package/lib/ReactOnRails.client.js"
13+
},
14+
"directories": {
15+
"doc": "docs"
16+
},
17+
"files": ["node_package/lib", "!node_package/lib/pro"],
18+
"scripts": {
19+
"test": "jest node_package/tests",
20+
"clean": "rm -rf node_package/lib",
21+
"start": "nps",
22+
"prepack": "nps build.prepack",
23+
"prepare": "nps build.prepack",
24+
"prepublishOnly": "yarn run build",
25+
"build": "yarn run clean && yarn run tsc --declaration",
26+
"build-watch": "yarn run clean && yarn run tsc --watch",
27+
"lint": "nps eslint",
28+
"check": "yarn run lint && yarn run test && yarn run type-check",
29+
"type-check": "yarn run tsc --noEmit --noErrorTruncation",
30+
"release:patch": "node_package/scripts/release patch",
31+
"release:minor": "node_package/scripts/release minor",
32+
"release:major": "node_package/scripts/release major"
33+
},
34+
"repository": {
35+
"type": "git",
36+
"url": "git+https://github.com/shakacode/react_on_rails.git"
37+
},
38+
"keywords": ["react", "webpack", "JavaScript", "Ruby", "on", "Rails"],
39+
"author": "justin.gordon@gmail.com",
40+
"license": "MIT",
41+
"bugs": {
42+
"url": "https://github.com/shakacode/react_on_rails/issues"
43+
},
44+
"homepage": "https://github.com/shakacode/react_on_rails#readme",
45+
"packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e",
46+
"dependencies": {},
47+
"devDependencies": {
48+
"@arethetypeswrong/cli": "^0.17.4",
49+
"@babel/core": "^7.20.12",
50+
"@babel/eslint-parser": "^7.26.10",
51+
"@babel/preset-env": "^7.20.2",
52+
"@babel/preset-react": "^7.26.3",
53+
"@eslint/compat": "^1.2.7",
54+
"@testing-library/dom": "^10.4.0",
55+
"@testing-library/jest-dom": "^6.6.3",
56+
"@testing-library/react": "^16.2.0",
57+
"@tsconfig/node14": "^14.1.2",
58+
"@types/jest": "^29.5.14",
59+
"@types/node": "^20.17.16",
60+
"@types/react": "^18.3.18",
61+
"@types/react-dom": "^18.3.5",
62+
"@types/turbolinks": "^5.2.2",
63+
"create-react-class": "^15.7.0",
64+
"eslint": "^9.22.0",
65+
"eslint-config-prettier": "^10.1.1",
66+
"eslint-config-shakacode": "^19.0.0",
67+
"eslint-import-resolver-alias": "^1.1.2",
68+
"eslint-plugin-import": "^2.31.0",
69+
"eslint-plugin-jest": "^28.11.0",
70+
"eslint-plugin-jsx-a11y": "^6.10.2",
71+
"eslint-plugin-prettier": "^5.2.3",
72+
"eslint-plugin-react": "^7.37.4",
73+
"eslint-plugin-react-hooks": "^5.2.0",
74+
"eslint-plugin-testing-library": "^7.5.3",
75+
"globals": "^16.0.0",
76+
"jest": "^29.7.0",
77+
"jest-environment-jsdom": "^29.7.0",
78+
"jest-fetch-mock": "^3.0.3",
79+
"jsdom": "^22.1.0",
80+
"knip": "^5.46.0",
81+
"nps": "^5.9.3",
82+
"prettier": "^3.5.2",
83+
"prop-types": "^15.8.1",
84+
"publint": "^0.3.8",
85+
"react": "^19.0.0",
86+
"react-dom": "^19.0.0",
87+
"react-on-rails-rsc": "19.0.2",
88+
"redux": "^4.2.1",
89+
"ts-jest": "^29.2.5",
90+
"typescript": "^5.8.3",
91+
"typescript-eslint": "^8.35.0"
92+
},
93+
"peerDependencies": {
94+
"react": ">= 16",
95+
"react-dom": ">= 16"
96+
}
97+
}

‎spec/dummy/client/app/startup/HelloWorldRehydratable.jsx‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,16 @@ class HelloWorldRehydratable extends React.Component {
4242
const { railsContext } = this.props;
4343

4444
// Target all instances of the component in the DOM
45-
const match = document.querySelectorAll(`[id^=${registeredComponentName}-react-component-]`);
45+
const match = document.querySelectorAll(
46+
`[id^="${CSS.escape(registeredComponentName)}-react-component-"]`,
47+
);
4648
// Not all browsers support forEach on NodeList so we go with a classic for-loop
4749
for (let i = 0; i < match.length; i += 1) {
4850
const component = match[i];
4951
// Get component specification <script> tag
50-
const componentSpecificationTag = document.querySelector(`script[data-dom-id=${component.id}]`);
52+
const componentSpecificationTag = document.querySelector(
53+
`script[data-dom-id="${CSS.escape(component.id)}"]`,
54+
);
5155
// Read props from the component specification tag and merge railsContext
5256
const mergedProps = { ...JSON.parse(componentSpecificationTag.textContent), railsContext };
5357
// Hydrate

0 commit comments

Comments
 (0)