Convert OMM to OMM Plugin & Integrate Build Tool Functionality #363

jvigliotta · 2025-11-24T20:26:59Z

closes #360

Convert Open MCT for MCWS from using OpenM CT as a dependency to being a plugin for Open MCT. Also, update Open MCT for MCWS Plugin to work with the Open MCT Configurator build tool.

Changes:

Convert any remaining non ES6 Modules
Consolidated AMMOSPlugins.js and loader.js into the new plugin.js
Removed Open MCT plugins from plugin and added to recipes for OMM Plugin default.yaml development.yaml
Convert OMM to a plugin, it now accepts it's previous ocnfiguration (config.js) as plugin options
Reduce the complexity of the configuration, default non-required config in plugin and start with only required config options
Removed webpack.dev.js configuration
Created dev server to work with new plugin/build tool format
Updated README.md for new plugin format

serve-instance.js

+    }
+
+    // Make a request to the target URL
+    const response = await fetch(targetUrl, fetchOptions);


The best way to fix this SSRF is to restrict outbound requests so that only specific, pre-approved hosts or endpoints can be targeted. Do not allow the user to pick an arbitrary URL, especially in the hostname. Instead, maintain a list of allowed hosts or patterns, and use the user's input only to select among these, or to build paths on these hosts in a controlled way. You can safely handle query/path construction for known hosts, but for absolute URLs, you should parse and validate them against your allow-list before permitting any outbound request.

Specifically:

Before making a request with fetch(targetUrl, ...), parse the URL (new URL(targetUrl)) and check that the hostname matches an allowed set.

If the user requests a host not on the list, reject the request with an HTTP 400 or 403.

The allow-list may be defined at the top of the file or as an environment variable, but it must not be influenced by user input.

Perform this check just before the fetch call.

You'll need to:

Import/use the standard url module to safely parse hostnames.

Add an allow-list (e.g., an array of allowed hosts like ['example.com', 'my-api.com', 'localhost']), configurable if necessary.

Validate the parsed hostname matches an entry before fetching.

Respond with an error if not allowed.

plugin.js

+    openmct.install(new RealtimeTelemetryPlugin(vistaTime, config));
+    openmct.install(new TypePlugin());
+    openmct.install(new TaxonomyPlugin(config.taxonomy));
+    openmct.install(new LinkPlugin(config));


To fix the problem, remove the unnecessary argument (config) when calling LinkPlugin in the call to openmct.install within openmctMCWSPlugin. Specifically, edit line 189 to call new LinkPlugin() with no arguments instead of new LinkPlugin(config). This will align the function call with the callee's signature, avoid possible miscommunication about what LinkPlugin is supposed to do, and resolve the flagged issue. No additional imports or methods are necessary, and changes should be limited to this line in plugin.js.

serve-instance.js

+    if (urlMatch) {
+      try {
+        const decodedUrl = decodeURIComponent(urlMatch[1]);
+        console.log('Decoded URL from query:', decodedUrl);


To fix the problem, we need to ensure that user input (decodedUrl) is sanitized before it is logged. Specifically, we should remove CR (\r) and LF (\n) characters, as recommended. Logging should also clearly label user input, to help identify it as such in logs.

Only the code line logging decodedUrl (line 70 in serve-instance.js) needs to be modified. Before logging, we should sanitize the decoded value, e.g., with .replace(/\r|\n/g, "") to remove all newline-related control characters. No changes elsewhere, and no extra dependency is needed—this can be done directly in the block.

Required:

Edit the logging statement to sanitize the value.

Possibly label the user input in logs, e.g., quote or bracket it.

serve-instance.js

+        }
+      } catch (e) {
+        // Invalid URL encoding
+        console.error('Error decoding URL:', e, 'Raw value:', urlMatch[1]);


To fix this issue, the value logged from user input needs to be sanitized before being included in the log entry. For plain text logs, the recommended practice is to remove newline characters (\n, \r) from the value. This can be done by using String.prototype.replace with a regular expression. The change should be limited to the specific value logged—urlMatch[1] on line 90 in serve-instance.js. The sanitized value should replace the direct use of urlMatch[1] in the error log.

Specifically:

On line 90, before logging urlMatch[1], apply .replace(/\r|\n/g, "") to sanitize it.

It is not necessary to add any imports, as this uses standard JavaScript functionality.

No functional change for non-error flows.

serve-instance.js

+  }
+
+  if (!targetUrl) {
+    console.error('Failed to extract valid URL from request:', req.originalUrl || req.url);


To address this log injection issue, sanitize all user-controlled input before including it in logs. In this context, remove or escape any potentially dangerous control characters from req.originalUrl or req.url before logging them. A standard fix for plain text logs is to remove all carriage return (\r) and newline (\n) characters by using String.prototype.replace, ensuring any log entry cannot be split into multiple lines by attacker input. The fix should only affect the relevant log statement(s) to avoid changing existing functionality. The required change involves assigning a sanitized version of the URL to a local variable and logging that variable in place of the raw input. No new dependencies are required.

src/services/dataset/ChannelDictionary.js

+// Helper function to replace lodash groupBy
+function groupBy(array, key) {
+  return array.reduce((result, item) => {
+    const groupKey = typeof key === 'function' ? key(item) : item[key];


src/services/dataset/ChannelDictionary.js

+
+// Helper function to replace lodash map
+function map(array, key) {
+  return array.map((item) => (typeof key === 'function' ? key(item) : item[key]));


To fix the problem, we should simplify the implementation of the map helper function in src/services/dataset/ChannelDictionary.js, specifically lines 37-39. The conditional that checks typeof key === 'function' is unnecessary if callers never supply a function as the key parameter. We can safely remove the conditional, and have the function simply use the property lookup (item[key]). The fix requires editing just the map function definition in this file. No new methods or imports are needed, nor any other changes elsewhere.

src/services/dataset/EVRDictionary.js

+// Helper function to replace lodash groupBy
+function groupBy(array, key) {
+  return array.reduce((result, item) => {
+    const groupKey = typeof key === 'function' ? key(item) : item[key];


To fix this problem, we should simplify the groupBy function by removing the typeof key === 'function' ? key(item) : item[key] check and always using item[key], as key is always a string. This involves editing the groupBy implementation in src/services/dataset/EVRDictionary.js to remove the type test and just access the property via item[key]. No changes to other parts of the code, imports, or extra definitions are required.

src/services/dataset/EVRDictionary.js

+// Helper function to replace lodash keyBy
+function keyBy(array, key) {
+  return array.reduce((result, item) => {
+    const groupKey = typeof key === 'function' ? key(item) : item[key];


The fix is to remove the unneeded guard typeof key === 'function' in the helper keyBy, as it always evaluates to false in this context. Specifically, change line 47 to directly assign groupKey = item[key]; (dropping the ternary operator). This change makes the code simpler and avoids misleading future maintainers about the function’s allowed polymorphism. The change is entirely local to the keyBy function in src/services/dataset/EVRDictionary.js.

src/services/dataset/EVRDictionary.js

+function uniqBy(array, key) {
+  const seen = new Set();
+  return array.filter((item) => {
+    const val = typeof key === 'function' ? key(item) : item[key];


To fix this issue, remove the unnecessary guard in the uniqBy function for the key argument. Currently, the code uses typeof key === 'function' ? key(item) : item[key]; to support either a property string or a function. Since CodeQL has flagged that typeof key === 'function' is always false here, this can be simplified to always use item[key], which removes unneeded complexity without changing the actual behaviour of the function in this codebase. Make the corresponding change on line 57 in src/services/dataset/EVRDictionary.js.

sonarqubecloud · 2025-11-24T20:29:23Z

Quality Gate failed

Failed conditions
1 Security Hotspot
0.0% Coverage on New Code (required ≥ 80%)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

akhenry · 2025-11-25T22:04:13Z

src/historical/HistoricalProvider.js

-            const valuesByChannelId = _.groupBy(res, 'channel_id');
-            const toFulfill = _.keyBy(requests, 'domainObject.telemetry.channel_id');
+// CHANNEL LAD PROVIDER
+const channelLADProvider = {


We should separate each of these pseudo-providers out to their own separate files and makes them ES6 classes. We can then instantiate them into an array further down.

akhenry · 2025-11-25T22:19:40Z

src/realtime/MCWSStreamWorker.js

-
-const objectUrl = URL.createObjectURL(blob);
+const workerInstance = new Worker(
+  new URL('./MCWSStreamWorkerScript.js', import.meta.url),


I don't think this is functionally equivalent to the code it's replacing. The original code imports the script at build time, and encodes its contents into a data URL. The new code defers resolving the web worker script until runtime, requiring the web worker script to be available on the web server.

Is there a reason why we switched?

akhenry · 2025-11-25T22:22:19Z

src/services/mcws/MCWSClient.js


    if (this.config.proxyUrl) {
      const params = options.params;
+      let isJsonResponse = false;


Changes in this file appear unrelated to the other changes?

…ility functions

… into omm-plugin merge remote

jvigliotta added 8 commits November 7, 2025 13:46

wip

4c2aa34

es6 conversion and pluginization

6af15bc

axe wp dev server, config, index.html

db53a85

remove instances

9b93d5b

updated readme

905a423

updated readme with todo

cbe8840

update to version 1 of plugin, this is up for debate

734847a

update pom

4321d04

jvigliotta requested a review from davetsay November 24, 2025 20:27

github-advanced-security bot found potential problems Nov 24, 2025

View reviewed changes

jvigliotta changed the base branch from main to topic/omm-plugin November 24, 2025 23:04

akhenry reviewed Nov 25, 2025

View reviewed changes

akhenry added 4 commits November 26, 2025 13:05

Remove handling for local proxy

f9b9fab

Remove local dev server

262a2bb

Restore local dev server for development

455e908

Move recipes to build tool

a3a14ae

akhenry changed the title ~~Convert OMM to OMM Plugjn & Integrate Build Tool Functionality~~ Convert OMM to OMM Plugin & Integrate Build Tool Functionality Nov 26, 2025

jvigliotta and others added 8 commits December 3, 2025 10:28

wip: pulling out providers and utilty functions

6b5d1b6

Restored legacy build process

4ccc637

Support legacy hosting options

4a91dae

Update peer dependencies

ec760eb

separating out providers to classes in their own files pulling out ut…

5fbb3bb

…ility functions

handle streamworker loading at buildtime

78cb8c1

Merge branch 'omm-plugin' of https://github.com/NASA-AMMOS/openmct-mcws…

0bfd749

… into omm-plugin merge remote

rever to see if can run

47bc7b1

@@ -23,6 +23,15 @@
             // Proxy configuration (matching webpack.dev.js)
             const proxyUrl = process.env.PROXY_URL || 'http://localhost:8080';
+            // SSRF protection: restrict allowed destination hostnames
+            // You may configure this list as needed for your deployment
+            const ALLOWED_PROXY_HOSTS = [
+              'localhost',
+              '127.0.0.1',
+              // Add any other allowed domains, e.g. 'api.mycompany.com', 'dev.example.com'
+              // 'example.com'
+            ];
             const apiUrl = process.env.API_URL || '';
             const proxyHeaders = {};
@@ -100,7 +109,25 @@
               try {
                 console.log('Generic URL Proxy to:', targetUrl);
+                // SSRF protection: validate target URL host is allowed
+                let parsedTargetUrl;
+                try {
+                  parsedTargetUrl = new URL(targetUrl);
+                } catch (e) {
+                  console.error('Invalid target URL:', targetUrl, 'Error:', e);
+                  return res.status(400).send('Invalid target URL.');
+                }
+                const allowed = ALLOWED_PROXY_HOSTS.some((allowedHost) => {
+                  // Allow both exact match and subdomains for allowed hosts
+                  return parsedTargetUrl.hostname === allowedHost ||
+                         parsedTargetUrl.hostname.endsWith(`.${allowedHost}`);
+                });
+                if (!allowed) {
+                  console.error('Blocked SSRF attempt to host:', parsedTargetUrl.hostname);
+                  return res.status(403).send('Proxying to this host is not allowed.');
+                }
                 // Prepare headers - exclude headers that shouldn't be forwarded
                 const headersToForward = { ...proxyHeaders };
                 const headersToExclude = ['host', 'connection', 'content-length', 'transfer-encoding'];
@@ -121,7 +148,7 @@
                   fetchOptions.body = req.body;
                 }
-                // Make a request to the target URL
+                // Make a request to the target URL (now validated)
                 const response = await fetch(targetUrl, fetchOptions);
                 // Forward response headers

@@ -186,7 +186,7 @@
                 openmct.install(new RealtimeTelemetryPlugin(vistaTime, config));
                 openmct.install(new TypePlugin());
                 openmct.install(new TaxonomyPlugin(config.taxonomy));
-                openmct.install(new LinkPlugin(config));
+                openmct.install(new LinkPlugin());
                 openmct.install(new VenuePlugin(config));
                 openmct.install(FrameWatchViewPlugin(config.tablePerformanceOptions));
                 openmct.install(FrameEventFilterViewPlugin(config.tablePerformanceOptions));

@@ -67,7 +67,8 @@
                 if (urlMatch) {
                   try {
                     const decodedUrl = decodeURIComponent(urlMatch[1]);
-                    console.log('Decoded URL from query:', decodedUrl);
+                    const sanitizedDecodedUrl = decodedUrl.replace(/[\r\n]/g, "");
+                    console.log('Decoded URL from query:', `"${sanitizedDecodedUrl}"`);
                     // Handle relative URLs or query strings (like webpack dev server does)
                     if (decodedUrl.startsWith('http://') || decodedUrl.startsWith('https://')) {

@@ -87,7 +87,9 @@
                     }
                   } catch (e) {
                     // Invalid URL encoding
-                    console.error('Error decoding URL:', e, 'Raw value:', urlMatch[1]);
+                    // Sanitize the user-provided value to prevent log injection
+                    const sanitizedUrlMatchVal = urlMatch[1].replace(/\r|\n/g, "");
+                    console.error('Error decoding URL:', e, 'Raw value:', sanitizedUrlMatchVal);
                     targetUrl = null;
                   }
                 }

@@ -94,7 +94,9 @@
               }
               if (!targetUrl) {
-                console.error('Failed to extract valid URL from request:', req.originalUrl || req.url);
+                // Sanitize user input to prevent log injection
+                const sanitizedUrl = (req.originalUrl || req.url || '').replace(/[\r\n]/g, '');
+                console.error('Failed to extract valid URL from request:', sanitizedUrl);
                 return res.status(400).send('Missing or invalid url query parameter. The url parameter must be a valid HTTP/HTTPS URL.');
               }

Convert OMM to OMM Plugin & Integrate Build Tool Functionality #363

Are you sure you want to change the base?

Convert OMM to OMM Plugin & Integrate Build Tool Functionality #363

Conversation

jvigliotta commented Nov 24, 2025

Uh oh!

Check failure

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Uh oh!

Copilot Autofix

Check warning

Uh oh!

Uh oh!

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

Check notice

Copilot Autofix

sonarqubecloud bot commented Nov 24, 2025

Quality Gate failed

Uh oh!

akhenry Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

akhenry Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

akhenry Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants