Escape renderer error text responses

Author: justin808

packages/react-on-rails-pro-node-renderer/src/worker.ts

@@ -72,6 +72,11 @@ const INCREMENTAL_REQUEST_CLOSE_TIMEOUT_MS = 1_000;
 // retain VM source-map registrations for the same idle period.
 const STREAM_CONTEXT_RELEASE_TIMEOUT_MS = STREAM_CHUNK_TIMEOUT_MS;
 const INCREMENTAL_RESPONSE_FINISH_TIMEOUT_MS = STREAM_CONTEXT_RELEASE_TIMEOUT_MS;
+const HTML_ESCAPE_REPLACEMENTS: Record<string, string> = {
+  '&': '&amp;',
+  '<': '&lt;',
+  '>': '&gt;',
+};
 
 // Uncomment the below for testing timeouts:
 // import { delay } from './shared/utils.js';
@@ -112,12 +117,18 @@ function setStringResponseHeaders(headers: ResponseResult['headers'], res: Fasti
   }
 }
 
+function escapeHtmlText(value: string) {
+  return value.replace(/[&<>]/g, (char) => HTML_ESCAPE_REPLACEMENTS[char] ?? char);
+}
+
 const setResponse = async (result: ResponseResult, res: FastifyReply) => {
   const { status, data, headers, stream } = result;
   if (status !== 200 && status !== 410) {
     log.info({ msg: 'Sending non-200, non-410 data back', data });
   }
-  if (!stream && typeof data === 'string') {
+  const stringResponse = !stream && typeof data === 'string';
+  const responseData = stringResponse && status >= 400 ? escapeHtmlText(data) : data;
+  if (stringResponse) {
     setStringResponseHeaders(headers, res);
   }
   setHeaders(headers, res);
@@ -126,7 +137,10 @@ const setResponse = async (result: ResponseResult, res: FastifyReply) => {
   if (stream) {
     await res.send(stream);
   } else {
-    res.send(data);
+    // Non-success strings are escaped above and sent as nosniff text/plain; success
+    // strings include renderer JSON/HTML payloads for the Ruby client and stay intact.
+    // codeql[js/reflected-xss]
+    res.send(responseData);
   }
 };
 

packages/react-on-rails-pro-node-renderer/tests/worker.test.ts

@@ -296,7 +296,7 @@ describe('worker', () => {
         gemVersion,
         protocolVersion,
         railsEnv,
-        renderingRequest: 'ReactOnRails.dummy',
+        renderingRequest: 'ReactOnRails.dummy("<script>alert(1)</script>")',
         bundle: createReadStream(getFixtureBundle()),
       });
 
@@ -313,7 +313,10 @@ describe('worker', () => {
         expect.stringContaining('Caught top level error in handleRenderRequest'),
         undefined,
       );
+      expectPlainTextNosniffResponse(res);
       expect(res.payload).toContain('Caught top level error in handleRenderRequest');
+      expect(res.payload).toContain('&lt;script&gt;alert(1)&lt;/script&gt;');
+      expect(res.payload).not.toContain('<script>');
     } finally {
       buildExecutionContextSpy.mockRestore();
       reportMessageSpy.mockRestore();
@@ -804,7 +807,7 @@ describe('worker', () => {
     // The endpoint requires at least one bundle_<hash> field
     expect(res.statusCode).toBe(400);
     expectPlainTextNosniffResponse(res);
-    expect(res.payload).toContain('No bundle_<hash> fields provided');
+    expect(res.payload).toContain('No bundle_&lt;hash&gt; fields provided');
   });
 
   test('post /upload-assets with duplicate bundle hash silently skips overwrite and returns 200', async () => {