fix: harden MSP domain hostname validation

cli/src/api/client.js

@@ -2,28 +2,59 @@ const axios = require('axios');
 
 const getCleanDomain = (domain) => {
   const targetDomain = domain || process.env.FIREWALLA_MSP_ID || 'api.firewalla.net';
-  const cleanDomain = targetDomain.replace(/^https?:\/\//, '').replace(/\/$/, '');
-
-  // Security: Only allow Firewalla domains to prevent token theft
-  if (!cleanDomain.endsWith('.firewalla.net') && cleanDomain !== 'api.firewalla.net') {
+  const invalidDomain = () => {
     console.error(JSON.stringify({
       error: "Invalid domain",
       hint: "For security, only *.firewalla.net domains are allowed"
     }));
     process.exit(1);
+  };
+
+  // Accept only a hostname, optionally prefixed by http(s) and/or followed
+  // by a single trailing slash. Everything else is rejected before URL
+  // parsing so an explicit default port such as :443 cannot be normalized away.
+  if (!/^(?:https?:\/\/)?[A-Za-z0-9.-]+\/?$/i.test(targetDomain)) {
+    invalidDomain();
+  }
+
+  const targetUrl = /^https?:\/\//i.test(targetDomain)
+    ? targetDomain
+    : `https://${targetDomain}`;
+
+  let url;
+  try {
+    url = new URL(targetUrl);
+  } catch (_) {
+    invalidDomain();
   }
 
-  return cleanDomain;
+  const hostname = url.hostname.toLowerCase();
+
+  // Security: validate the parsed hostname, not the raw input, to prevent
+  // URL parser confusion from redirecting the MSP token to an attacker host.
+  if (
+    url.username ||
+    url.password ||
+    url.port ||
+    url.pathname !== '/' ||
+    url.search ||
+    url.hash ||
+    (hostname !== 'api.firewalla.net' && !hostname.endsWith('.firewalla.net'))
+  ) {
+    invalidDomain();
+  }
+
+  return hostname;
 };
 
 const getBaseUrl = (domain) => `https://${getCleanDomain(domain)}/v2`;
 
 const getClient = (options = {}) => {
   const token = process.env.FIREWALLA_MSP_TOKEN;
   if (!token) {
-    console.error(JSON.stringify({ 
-      error: "Auth missing.", 
-      hint: "Run: export FIREWALLA_MSP_TOKEN='your_msp_api_token_here' or add to .env" 
+    console.error(JSON.stringify({
+      error: "Auth missing.",
+      hint: "Run: export FIREWALLA_MSP_TOKEN='your_msp_api_token_here' or add to .env"
     }));
     process.exit(1);
   }
@@ -51,7 +82,7 @@ const resolveBoxGid = async (input, options) => {
     const envGid = process.env.FIREWALLA_BOX_GID;
     if (envGid) return envGid;
     if (boxes.length === 1) return boxes[0].gid;
-    
+
     console.error(JSON.stringify({ error: "Ambiguous request. Specify --box <name|gid>." }));
     process.exit(1);
   }
@@ -82,4 +113,4 @@ const getClientV1 = (options = {}) => {
   });
 };
 
-module.exports = { getClient, getClientV1, resolveBoxGid };
+module.exports = { getClient, getClientV1, resolveBoxGid };

package.json

@@ -7,7 +7,7 @@
     "fw": "./src/index.js"
   },
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "node --test"
   },
   "dependencies": {
     "axios": "^1.6.0",

test/client.test.js

@@ -0,0 +1,107 @@
+const test = require('node:test');
+const assert = require('node:assert/strict');
+
+const CLIENT_PATH = require.resolve('../cli/src/api/client');
+
+function withClient(fn) {
+  delete require.cache[CLIENT_PATH];
+  return fn(require(CLIENT_PATH));
+}
+
+function assertDomainRejected(input) {
+  const previousToken = process.env.FIREWALLA_MSP_TOKEN;
+  const previousExit = process.exit;
+  const previousError = console.error;
+
+  process.env.FIREWALLA_MSP_TOKEN = 'test-token';
+
+  let exitCode;
+  process.exit = (code) => {
+    exitCode = code;
+    throw new Error('process.exit');
+  };
+  console.error = () => {};
+
+  try {
+    withClient(({ getClient }) => {
+      assert.throws(
+        () => getClient({ domain: input }),
+        /process\.exit/
+      );
+    });
+    assert.equal(exitCode, 1);
+  } finally {
+    process.exit = previousExit;
+    console.error = previousError;
+
+    if (previousToken === undefined) {
+      delete process.env.FIREWALLA_MSP_TOKEN;
+    } else {
+      process.env.FIREWALLA_MSP_TOKEN = previousToken;
+    }
+  }
+}
+
+test('accepts the Firewalla API hostname', () => {
+  const previousToken = process.env.FIREWALLA_MSP_TOKEN;
+  process.env.FIREWALLA_MSP_TOKEN = 'test-token';
+
+  try {
+    withClient(({ getClient }) => {
+      const client = getClient({ domain: 'api.firewalla.net' });
+      assert.equal(client.defaults.baseURL, 'https://api.firewalla.net/v2');
+    });
+  } finally {
+    if (previousToken === undefined) {
+      delete process.env.FIREWALLA_MSP_TOKEN;
+    } else {
+      process.env.FIREWALLA_MSP_TOKEN = previousToken;
+    }
+  }
+});
+
+test('accepts Firewalla subdomains with an optional scheme', () => {
+  const previousToken = process.env.FIREWALLA_MSP_TOKEN;
+  process.env.FIREWALLA_MSP_TOKEN = 'test-token';
+
+  try {
+    withClient(({ getClient }) => {
+      const client = getClient({ domain: 'https://msp.example.firewalla.net' });
+      assert.equal(client.defaults.baseURL, 'https://msp.example.firewalla.net/v2');
+    });
+  } finally {
+    if (previousToken === undefined) {
+      delete process.env.FIREWALLA_MSP_TOKEN;
+    } else {
+      process.env.FIREWALLA_MSP_TOKEN = previousToken;
+    }
+  }
+});
+
+test('rejects URL parser confusion that changes the destination hostname', () => {
+  const attackerControlledDomains = [
+    'attacker.example?.firewalla.net',
+    'attacker.example#.firewalla.net',
+    'attacker.example/.firewalla.net',
+    'firewalla.net.attacker.example',
+    'https://attacker.example?.firewalla.net',
+  ];
+
+  for (const input of attackerControlledDomains) {
+    assertDomainRejected(input);
+  }
+});
+
+test('rejects credentials, ports, paths, queries, and fragments', () => {
+  const invalidDomains = [
+    'user:pass@api.firewalla.net',
+    'api.firewalla.net:443',
+    'api.firewalla.net/v2',
+    'api.firewalla.net?redirect=attacker.example',
+    'api.firewalla.net#attacker.example',
+  ];
+
+  for (const input of invalidDomains) {
+    assertDomainRejected(input);
+  }
+});