Skip to content

Dev#148

Merged
smalho01 merged 39 commits intomainfrom
dev
Feb 12, 2026
Merged

Dev#148
smalho01 merged 39 commits intomainfrom
dev

Conversation

@smalho01
Copy link
Contributor

Describe your changes

Please include a summary of the changes and the related issue/task. Please also include relevant motivation and context. List any dependencies that are required for this change, including links to other pull requests/branches in other repositories if applicable.

Issue ticket number and Jira link

Please include the Jira Ticket Number and Link for this issue/task.

Checklist before requesting a review

  • I have performed a self-review of my code
  • Ensure the target / base branch for any feature PR is set to dev not main (the only exception to this is releases from dev and hotfix branches)

Checklist for conducting a review

  • Review the code changes and make sure they all make sense and are necessary.
  • Pull the PR branch locally and test by running through workflow and making sure everything works as it is supposed to.

Workflow

Owner of the Pull Request will be responsible for merge after all requirements are met, including approval from at least one reviewer. Additional changes made after a review will dismiss any approvals and require re-review of the additional updates. Auto merging can be enabled below if additional changes are likely not to be needed. The bot will auto assign reviewers to your Pull Request for you.

@smalho01 smalho01 merged commit d1acc21 into main Feb 12, 2026
12 of 22 checks passed
Comment on lines +287 to +289
await axios.post(endpoint, rxFill, {
headers: { 'Content-Type': 'application/xml' }
});

Check failure

Code scanning / CodeQL

Server-side request forgery

The [URL](1) of this request depends on a [user-provided value](2).

Copilot Autofix

AI 7 days ago

To fix the problem, user input must not be allowed to arbitrarily control the hostname/URL used in outgoing requests. Instead, configuration updates from /api/config should either (a) not be allowed to change network endpoints at all, or (b) be strictly validated against an allow‑list or strong URL/host constraints before being stored and later used in getRxFillEndpoint().

The least intrusive and safest fix, without changing overall behavior, is:

  1. In backend/src/lib/pharmacyConfig.js:

    • Add a helper sanitizeUrl that:
      • Uses the built‑in URL class to parse a candidate URL.
      • Enforces that protocol is http: or https:.
      • Enforces that hostname is within a small, explicit allow‑list (e.g., set from environment variables, which are assumed trusted).
      • Returns either the validated URL string or null/the previous value if invalid.
    • Add an applyConfigUpdate function that:
      • Starts from the existing config.
      • For each updatable key (useIntermediary, intermediaryUrl, remsAdminUrl, ehrUrl), validates and normalizes the value.
      • For URL fields, calls sanitizeUrl and only updates if it passes validation; otherwise, keeps the previous value.
    • Update updateConfig(newConfig) to call applyConfigUpdate instead of spreading newConfig blindly into config.
  2. In backend/src/routes/doctorOrders.js:

    • Leave the use of getRxFillEndpoint() and axios.post(endpoint, ...) as‑is; they will now only ever use validated URLs.

This maintains the external API (clients can still POST config changes), but ensures that only safe, pre‑approved endpoints can be set, preventing an attacker from directing the server to arbitrary hosts.


Suggested changeset 1
backend/src/lib/pharmacyConfig.js
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/src/lib/pharmacyConfig.js b/backend/src/lib/pharmacyConfig.js
--- a/backend/src/lib/pharmacyConfig.js
+++ b/backend/src/lib/pharmacyConfig.js
@@ -7,15 +7,92 @@
   ehrUrl: process.env.EHR_NCPDP_URL
 };
 
+// Allowed hostnames for outbound HTTP requests, derived from initial trusted config
+const allowedHostnames = new Set(
+  [
+    process.env.INTERMEDIARY_URL,
+    process.env.REMS_ADMIN_NCPDP,
+    process.env.EHR_NCPDP_URL
+  ]
+    .filter(Boolean)
+    .map((urlString) => {
+      try {
+        return new URL(urlString).hostname;
+      } catch (_e) {
+        return null;
+      }
+    })
+    .filter(Boolean)
+);
 
+function sanitizeUrl(urlString) {
+  if (typeof urlString !== 'string') {
+    return null;
+  }
+  let parsed;
+  try {
+    parsed = new URL(urlString);
+  } catch (_e) {
+    return null;
+  }
 
+  // Only allow HTTP(S)
+  if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
+    return null;
+  }
+
+  // Restrict to known hostnames
+  if (!allowedHostnames.has(parsed.hostname)) {
+    return null;
+  }
+
+  return parsed.toString().replace(/\/+$/, '');
+}
+
+function applyConfigUpdate(newConfig) {
+  const updated = { ...config };
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'useIntermediary')) {
+    // Coerce to boolean if a value is provided
+    const value = newConfig.useIntermediary;
+    if (typeof value === 'boolean') {
+      updated.useIntermediary = value;
+    } else if (typeof value === 'string') {
+      updated.useIntermediary = value === 'true';
+    }
+  }
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'intermediaryUrl')) {
+    const safe = sanitizeUrl(newConfig.intermediaryUrl);
+    if (safe) {
+      updated.intermediaryUrl = safe;
+    }
+  }
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'remsAdminUrl')) {
+    const safe = sanitizeUrl(newConfig.remsAdminUrl);
+    if (safe) {
+      updated.remsAdminUrl = safe;
+    }
+  }
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'ehrUrl')) {
+    const safe = sanitizeUrl(newConfig.ehrUrl);
+    if (safe) {
+      updated.ehrUrl = safe;
+    }
+  }
+
+  return updated;
+}
+
 export function getConfig() {
   return { ...config };
 }
 
 
 export function updateConfig(newConfig) {
-  config = { ...config, ...newConfig };
+  config = applyConfigUpdate(newConfig || {});
   console.log('Configuration updated:', config);
   return { ...config };
 }
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +534 to +543
const response = await axios.post(
endpoint,
initiationRequest,
{
headers: {
Accept: 'application/xml',
'Content-Type': 'application/xml'
}
}
);

Check failure

Code scanning / CodeQL

Server-side request forgery

The [URL](1) of this request depends on a [user-provided value](2).

Copilot Autofix

AI 7 days ago

In general, to fix SSRF issues where URLs are derived from user input, you must prevent arbitrary hostnames/URLs from being configured. Typical mitigations are: (1) strict validation and normalization of the configured URLs, (2) allow‑listing hosts or base URLs, and optionally (3) blocking private/loopback IPs in dynamic URLs.

For this case, the best fix with minimal behavior change is to constrain what can be written into the config object via /api/config. We can add a helper in pharmacyConfig.js that sanitizes/validates newConfig before merging: it should accept only known keys (e.g., useIntermediary, intermediaryUrl, remsAdminUrl, ehrUrl), enforce types, and validate URL fields with the built‑in URL class. Optionally, it can also enforce that hosts belong to an allowed set or at least are not localhost or private IPs. Then we modify updateConfig to call this validator and merge only the sanitized values. No changes are needed in sendREMSInitiationRequest itself, since it will now only ever get safe endpoints from getNCPDPEndpoint().

Concretely:

  • In backend/src/lib/pharmacyConfig.js, introduce a sanitizeConfigUpdate(newConfig) function (above updateConfig) that:
    • Creates a new object.
    • If useIntermediary is present, coerces it to a boolean (or keeps it if already boolean).
    • For each of intermediaryUrl, remsAdminUrl, ehrUrl present in newConfig, verifies:
      • It is a string.
      • It parses as an absolute URL via new URL(value).
      • Optionally, checks host against a simple allowlist or at least disallows localhost, 127.0.0.1, ::1, etc.
    • Returns the sanitized object.
  • Change updateConfig so that instead of config = { ...config, ...newConfig }; it does const safeUpdate = sanitizeConfigUpdate(newConfig); config = { ...config, ...safeUpdate };.
  • No changes are required in backend/src/routes/doctorOrders.js for this particular SSRF path.

This keeps existing configuration semantics (same keys, same endpoints) but prevents arbitrary unvalidated URLs from being injected through /api/config.


Suggested changeset 1
backend/src/lib/pharmacyConfig.js
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/src/lib/pharmacyConfig.js b/backend/src/lib/pharmacyConfig.js
--- a/backend/src/lib/pharmacyConfig.js
+++ b/backend/src/lib/pharmacyConfig.js
@@ -7,15 +7,84 @@
   ehrUrl: process.env.EHR_NCPDP_URL
 };
 
+/**
+ * Sanitize and validate incoming configuration updates to prevent
+ * unsafe values (for example, arbitrary URLs) from being stored.
+ */
+function sanitizeConfigUpdate(newConfig) {
+  const safe = {};
 
+  if (!newConfig || typeof newConfig !== 'object') {
+    return safe;
+  }
 
+  // Only allow known keys
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'useIntermediary')) {
+    // Coerce to boolean if passed as string
+    const value = newConfig.useIntermediary;
+    if (typeof value === 'boolean') {
+      safe.useIntermediary = value;
+    } else if (typeof value === 'string') {
+      const lowered = value.toLowerCase();
+      if (lowered === 'true' || lowered === '1') {
+        safe.useIntermediary = true;
+      } else if (lowered === 'false' || lowered === '0') {
+        safe.useIntermediary = false;
+      }
+    }
+  }
+
+  // Helper to validate and normalize a URL string.
+  function validateUrl(urlValue) {
+    if (typeof urlValue !== 'string') {
+      return null;
+    }
+    try {
+      const parsed = new URL(urlValue);
+      // Basic SSRF hardening: avoid obvious loopback hosts.
+      const hostname = parsed.hostname.toLowerCase();
+      if (hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') {
+        return null;
+      }
+      return parsed.toString();
+    } catch (_e) {
+      return null;
+    }
+  }
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'intermediaryUrl')) {
+    const valid = validateUrl(newConfig.intermediaryUrl);
+    if (valid) {
+      safe.intermediaryUrl = valid;
+    }
+  }
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'remsAdminUrl')) {
+    const valid = validateUrl(newConfig.remsAdminUrl);
+    if (valid) {
+      safe.remsAdminUrl = valid;
+    }
+  }
+
+  if (Object.prototype.hasOwnProperty.call(newConfig, 'ehrUrl')) {
+    const valid = validateUrl(newConfig.ehrUrl);
+    if (valid) {
+      safe.ehrUrl = valid;
+    }
+  }
+
+  return safe;
+}
+
+
 export function getConfig() {
   return { ...config };
 }
 
 
 export function updateConfig(newConfig) {
-  config = { ...config, ...newConfig };
+  const safeUpdate = sanitizeConfigUpdate(newConfig);
+  config = { ...config, ...safeUpdate };
   console.log('Configuration updated:', config);
   return { ...config };
 }
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +585 to +594
const response = await axios.post(
endpoint,
remsRequest,
{
headers: {
Accept: 'application/xml',
'Content-Type': 'application/xml'
}
}
);

Check failure

Code scanning / CodeQL

Server-side request forgery

The [URL](1) of this request depends on a [user-provided value](2).

Copilot Autofix

AI 7 days ago

In general, to fix SSRF in this pattern you must stop using arbitrary client-provided URLs as HTTP targets. Instead, only allow safe, validated values (for example, selecting from an allow-list of hosts or validating that provided URLs meet strict criteria and do not point to internal or unsupported schemes).

For this codebase, the minimal, non‑breaking fix is to validate and sanitize any configuration fields that influence outbound URLs (intermediaryUrl, remsAdminUrl, ehrUrl) when updateConfig merges req.body into config. We can implement a local sanitizeConfig helper in pharmacyConfig.js that:

  • Ensures useIntermediary is boolean.
  • Validates that each URL, if provided, is a well-formed HTTP(S) URL using Node’s URL class.
  • Rejects URLs with non-http/https protocols.
  • Rejects obviously unsafe hosts such as localhost, 127.0.0.1, and other loopback forms (to mitigate SSRF to local services).
  • Optionally strips trailing slashes to keep existing behavior stable, but still returns the same endpoints for valid values.

updateConfig will then apply this sanitizer before updating the global config object. The route handler in doctorOrders.js can stay the same, since updateConfig will enforce constraints centrally. No changes to sendREMSRequest are needed, as it will now always receive a vetted URL from getNCPDPEndpoint. All edits are confined to backend/src/lib/pharmacyConfig.js.


Suggested changeset 1
backend/src/lib/pharmacyConfig.js
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/src/lib/pharmacyConfig.js b/backend/src/lib/pharmacyConfig.js
--- a/backend/src/lib/pharmacyConfig.js
+++ b/backend/src/lib/pharmacyConfig.js
@@ -7,15 +7,88 @@
   ehrUrl: process.env.EHR_NCPDP_URL
 };
 
+/**
+ * Basic URL validation and normalization for configuration values
+ * Only allows http/https URLs and rejects obvious loopback targets.
+ */
+function sanitizeConfig(inputConfig) {
+  const sanitized = { ...inputConfig };
 
+  // Normalize useIntermediary to a boolean if provided
+  if (Object.prototype.hasOwnProperty.call(sanitized, 'useIntermediary')) {
+    const val = sanitized.useIntermediary;
+    if (typeof val === 'string') {
+      sanitized.useIntermediary = val === 'true' || val === '1';
+    } else {
+      sanitized.useIntermediary = Boolean(val);
+    }
+  }
 
+  const disallowLoopbackHost = (urlString) => {
+    try {
+      const parsed = new URL(urlString);
+      const hostname = parsed.hostname.toLowerCase();
+      if (
+        hostname === 'localhost' ||
+        hostname === '127.0.0.1' ||
+        hostname === '::1'
+      ) {
+        return false;
+      }
+      if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
+        return false;
+      }
+      return true;
+    } catch (_e) {
+      return false;
+    }
+  };
+
+  const normalizeUrl = (urlString) => {
+    try {
+      const parsed = new URL(urlString);
+      // Remove trailing slash for consistency
+      parsed.pathname = parsed.pathname.replace(/\/+$/, '');
+      return parsed.toString();
+    } catch (_e) {
+      return urlString;
+    }
+  };
+
+  const maybeSanitizeUrlField = (fieldName) => {
+    if (!Object.prototype.hasOwnProperty.call(sanitized, fieldName)) {
+      return;
+    }
+    const value = sanitized[fieldName];
+    if (typeof value !== 'string' || value.trim() === '') {
+      // Do not update the field if value is not a non-empty string
+      delete sanitized[fieldName];
+      return;
+    }
+    if (!disallowLoopbackHost(value)) {
+      // Unsafe URL; drop the update for this field
+      delete sanitized[fieldName];
+      return;
+    }
+    sanitized[fieldName] = normalizeUrl(value);
+  };
+
+  maybeSanitizeUrlField('intermediaryUrl');
+  maybeSanitizeUrlField('remsAdminUrl');
+  maybeSanitizeUrlField('ehrUrl');
+
+  return sanitized;
+}
+
+
 export function getConfig() {
   return { ...config };
 }
 
 
 export function updateConfig(newConfig) {
-  config = { ...config, ...newConfig };
+  const safeConfigUpdate = sanitizeConfig(newConfig || {});
+  config = { ...config, ...safeConfigUpdate };
   console.log('Configuration updated:', config);
   return { ...config };
 }
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments