Merge pull request #133 from AikidoSec/remove-ts-suppressions

packages/safe-chain/bin/aikido-bun.js

@@ -7,5 +7,4 @@ const packageManagerName = "bun";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/bin/aikido-bunx.js

@@ -7,5 +7,4 @@ const packageManagerName = "bunx";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/bin/aikido-npm.js

@@ -7,5 +7,4 @@ const packageManagerName = "npm";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/bin/aikido-npx.js

@@ -7,5 +7,4 @@ const packageManagerName = "npx";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/bin/aikido-pnpm.js

@@ -7,5 +7,4 @@ const packageManagerName = "pnpm";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/bin/aikido-pnpx.js

@@ -7,5 +7,4 @@ const packageManagerName = "pnpx";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/bin/aikido-yarn.js

@@ -7,5 +7,4 @@ const packageManagerName = "yarn";
 initializePackageManager(packageManagerName);
 var exitCode = await main(process.argv.slice(2));
 
-// @ts-expect-error scanCommand can return an empty array in main
 process.exit(exitCode);

packages/safe-chain/src/main.js

@@ -9,7 +9,7 @@ import chalk from "chalk";
 
 /**
  * @param {string[]} args
- * @returns {Promise<number | never[]>}
+ * @returns {Promise<number>}
  */
 export async function main(args) {
   process.on("SIGINT", handleProcessTermination);

packages/safe-chain/src/packagemanager/bun/createBunPackageManager.js

@@ -39,7 +39,6 @@ async function runBunCommand(command, args) {
   try {
     const result = await safeSpawn(command, args, {
       stdio: "inherit",
-      // @ts-expect-error values of process.env can be string | undefined
       env: mergeSafeChainProxyEnvironmentVariables(process.env),
     });
     return { status: result.status };

packages/safe-chain/src/packagemanager/npm/runNpmCommand.js

@@ -11,7 +11,6 @@ export async function runNpm(args) {
   try {
     const result = await safeSpawn("npm", args, {
       stdio: "inherit",
-      // @ts-expect-error values of process.env can be string | undefined
       env: mergeSafeChainProxyEnvironmentVariables(process.env),
     });
     return { status: result.status };
@@ -24,37 +23,3 @@ export async function runNpm(args) {
     }
   }
 }
-
-/**
- * @param {string[]} args
- * @returns {Promise<{status: number, output?: string}>}
- */
-export async function dryRunNpmCommandAndOutput(args) {
-  try {
-    const result = await safeSpawn(
-      "npm",
-      [...args, "--ignore-scripts", "--dry-run"],
-      {
-        stdio: "pipe",
-        // @ts-expect-error values of process.env can be string | undefined
-        env: mergeSafeChainProxyEnvironmentVariables(process.env),
-      }
-    );
-    return {
-      status: result.status,
-      output: result.status === 0 ? result.stdout : result.stderr,
-    };
-  } catch (/** @type any */ error) {
-    if (error.status) {
-      const output =
-        error.stdout?.toString() ??
-        error.stderr?.toString() ??
-        error.message ??
-        "";
-      return { status: error.status, output };
-    } else {
-      ui.writeError("Error executing command:", error.message);
-      return { status: 1 };
-    }
-  }
-}

packages/safe-chain/src/packagemanager/npx/runNpxCommand.js

@@ -11,7 +11,6 @@ export async function runNpx(args) {
   try {
     const result = await safeSpawn("npx", args, {
       stdio: "inherit",
-      // @ts-expect-error values of process.env can be string | undefined
       env: mergeSafeChainProxyEnvironmentVariables(process.env),
     });
     return { status: result.status };

packages/safe-chain/src/packagemanager/pnpm/runPnpmCommand.js

@@ -13,13 +13,11 @@ export async function runPnpmCommand(args, toolName = "pnpm") {
     if (toolName === "pnpm") {
       result = await safeSpawn("pnpm", args, {
         stdio: "inherit",
-        // @ts-expect-error values of process.env can be string | undefined
         env: mergeSafeChainProxyEnvironmentVariables(process.env),
       });
     } else if (toolName === "pnpx") {
       result = await safeSpawn("pnpx", args, {
         stdio: "inherit",
-        // @ts-expect-error values of process.env can be string | undefined
         env: mergeSafeChainProxyEnvironmentVariables(process.env),
       });
     } else {

packages/safe-chain/src/packagemanager/yarn/runYarnCommand.js

@@ -9,7 +9,6 @@ import { mergeSafeChainProxyEnvironmentVariables } from "../../registryProxy/reg
  */
 export async function runYarnCommand(args) {
   try {
-    // @ts-expect-error values of process.env can be string | undefined
     const env = mergeSafeChainProxyEnvironmentVariables(process.env);
     await fixYarnProxyEnvironmentVariables(env);
 

packages/safe-chain/src/registryProxy/mitmRequestHandler.js

@@ -5,7 +5,7 @@ import { ui } from "../environment/userInteraction.js";
 
 /**
  * @param {import("http").IncomingMessage} req
- * @param {import("net").Socket} clientSocket
+ * @param {import("http").ServerResponse} clientSocket
  * @param {(target: string) => Promise<boolean>} isAllowed
  */
 export function mitmConnect(req, clientSocket, isAllowed) {
@@ -25,7 +25,6 @@ export function mitmConnect(req, clientSocket, isAllowed) {
 
   server.on("error", (err) => {
     ui.writeError(`Safe-chain: HTTPS server error: ${err.message}`);
-    // @ts-expect-error Property 'headersSent' does not exist on type 'Socket'
     if (!clientSocket.headersSent) {
       clientSocket.end("HTTP/1.1 502 Bad Gateway\r\n\r\n");
     } else if (clientSocket.writable) {
@@ -55,7 +54,13 @@ function createHttpsServer(hostname, isAllowed) {
    * @returns {Promise<void>}
    */
   async function handleRequest(req, res) {
-    // @ts-expect-error req.url might be undefined
+    if (!req.url) {
+      ui.writeError("Safe-chain: Request missing URL");
+      res.writeHead(400, "Bad Request");
+      res.end("Bad Request: Missing URL");
+      return;
+    }
+
     const pathAndQuery = getRequestPathAndQuery(req.url);
     const targetUrl = `https://${hostname}${pathAndQuery}`;
 
@@ -163,7 +168,13 @@ function createProxyRequest(hostname, req, res) {
       }
     });
 
-    // @ts-expect-error statusCode might be undefined
+    if (!proxyRes.statusCode) {
+      ui.writeError("Safe-chain: Proxy response missing status code");
+      res.writeHead(500);
+      res.end("Internal Server Error");
+      return;
+    }
+
     res.writeHead(proxyRes.statusCode, proxyRes.headers);
     proxyRes.pipe(res);
   });

packages/safe-chain/src/registryProxy/plainHttpProxy.js

@@ -1,5 +1,6 @@
 import * as http from "http";
 import * as https from "https";
+import { ui } from "../environment/userInteraction.js";
 
 /**
  * @param {import("http").IncomingMessage} req
@@ -8,7 +9,13 @@ import * as https from "https";
  * @returns {void}
  */
 export function handleHttpProxyRequest(req, res) {
-  // @ts-expect-error req.url might be undefined
+  if (!req.url) {
+    ui.writeError("Safe-chain: Request missing URL");
+    res.writeHead(400, "Bad Request");
+    res.end("Bad Request: Missing URL");
+    return;
+  }
+
   const url = new URL(req.url);
 
   // The protocol for the plainHttpProxy should usually only be http:
@@ -27,11 +34,16 @@ export function handleHttpProxyRequest(req, res) {
 
   const proxyRequest = protocol
     .request(
-      // @ts-expect-error req.url might be undefined
       req.url,
       { method: req.method, headers: req.headers },
       (proxyRes) => {
-        // @ts-expect-error statusCode might be undefined
+        if (!proxyRes.statusCode) {
+          ui.writeError("Safe-chain: Proxy response missing status code");
+          res.writeHead(500);
+          res.end("Internal Server Error");
+          return;
+        }
+
         res.writeHead(proxyRes.statusCode, proxyRes.headers);
         proxyRes.pipe(res);
 

packages/safe-chain/src/registryProxy/registryProxy.js

@@ -43,7 +43,7 @@ function getSafeChainProxyEnvironmentVariables() {
 }
 
 /**
- * @param {Record<string, string>} env
+ * @param {Record<string, string | undefined>} env
  *
  * @returns {Record<string, string>}
  */
@@ -56,7 +56,7 @@ export function mergeSafeChainProxyEnvironmentVariables(env) {
     // So we only copy the variable if it's not already set in a different case
     const upperKey = key.toUpperCase();
 
-    if (!proxyEnv[upperKey]) {
+    if (!proxyEnv[upperKey] && env[key]) {
       proxyEnv[key] = env[key];
     }
   }
@@ -122,7 +122,7 @@ function stopServer(server) {
 
 /**
  * @param {import("http").IncomingMessage} req
- * @param {import("net").Socket} clientSocket
+ * @param {import("http").ServerResponse} clientSocket
  * @param {Buffer} head
  *
  * @returns {void}
@@ -130,9 +130,9 @@ function stopServer(server) {
 function handleConnect(req, clientSocket, head) {
   // CONNECT method is used for HTTPS requests
   // It establishes a tunnel to the server identified by the request URL
+  const url = req.url;
 
-  // @ts-expect-error req.url might be undefined
+  if (url && knownRegistries.some((reg) => url.includes(reg))) {
-  if (knownRegistries.some((reg) => req.url.includes(reg))) {
     // For npm and yarn registries, we want to intercept and inspect the traffic
     // so we can block packages with malware
     mitmConnect(req, clientSocket, isAllowedUrl);

packages/safe-chain/src/registryProxy/tunnelRequestHandler.js

@@ -3,7 +3,7 @@ import { ui } from "../environment/userInteraction.js";
 
 /**
  * @param {import("http").IncomingMessage} req
- * @param {import("net").Socket} clientSocket
+ * @param {import("http").ServerResponse} clientSocket
  * @param {Buffer} head
  *
  * @returns {void}
@@ -30,7 +30,7 @@ export function tunnelRequest(req, clientSocket, head) {
 
 /**
  * @param {import("http").IncomingMessage} req
- * @param {import("net").Socket} clientSocket
+ * @param {import("http").ServerResponse} clientSocket
  * @param {Buffer} head
  *
  * @returns {void}
@@ -38,13 +38,16 @@ export function tunnelRequest(req, clientSocket, head) {
 function tunnelRequestToDestination(req, clientSocket, head) {
   const { port, hostname } = new URL(`http://${req.url}`);
 
-  // @ts-expect-error port from URL is a string but net.connect accepts number
+  const serverSocket = net.connect(
-  const serverSocket = net.connect(port || 443, hostname, () => {
+    Number.parseInt(port) || 443,
-    clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
+    hostname,
-    serverSocket.write(head);
+    () => {
-    serverSocket.pipe(clientSocket);
+      clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n");
-    clientSocket.pipe(serverSocket);
+      serverSocket.write(head);
-  });
+      serverSocket.pipe(clientSocket);
+      clientSocket.pipe(serverSocket);
+    }
+  );
 
   clientSocket.on("error", () => {
     // This can happen if the client TCP socket sends RST instead of FIN.
@@ -66,7 +69,7 @@ function tunnelRequestToDestination(req, clientSocket, head) {
 
 /**
  * @param {import("http").IncomingMessage} req
- * @param {import("net").Socket} clientSocket
+ * @param {import("http").ServerResponse} clientSocket
  * @param {Buffer} head
  * @param {string} proxyUrl
  */
@@ -75,10 +78,9 @@ function tunnelRequestViaProxy(req, clientSocket, head, proxyUrl) {
   const proxy = new URL(proxyUrl);
 
   // Connect to proxy server
-  // @ts-expect-error net.connect wants port as number but proxy.port is string
   const proxySocket = net.connect({
     host: proxy.hostname,
-    port: proxy.port,
+    port: Number.parseInt(proxy.port) || 80,
   });
 
   proxySocket.on("connect", () => {

packages/safe-chain/src/scanning/index.js

@@ -21,11 +21,11 @@ export function shouldScanCommand(args) {
 /**
  * @param {string[]} args
  *
- * @returns {Promise<number | never[]>}
+ * @returns {Promise<number>}
  */
 export async function scanCommand(args) {
   if (!shouldScanCommand(args)) {
-    return [];
+    return 0;
   }
 
   let timedOut = false;

packages/safe-chain/src/scanning/malwareDatabase.js

@@ -71,10 +71,19 @@ async function getMalwareDatabase() {
     }
 
     const { malwareDatabase, version } = await fetchMalwareDatabase();
-    // @ts-expect-error version can be undefined
-    writeDatabaseToLocalCache(malwareDatabase, version);
 
-    return malwareDatabase;
+    if (version) {
+      // Only cache the malware database when we have a version.
+      writeDatabaseToLocalCache(malwareDatabase, version);
+      return malwareDatabase;
+    } else {
+      // We received a valid malware database, but the response
+      // did not contain an etag header with the version
+      ui.writeWarning(
+        "The malware database was downloaded, but could not be cached due to a missing version."
+      );
+      return malwareDatabase;
+    }
   } catch (/** @type any */ error) {
     if (cachedDatabase) {
       ui.writeWarning(

packages/safe-chain/src/utils/safeSpawn.js

@@ -67,8 +67,6 @@ function resolveCommandPath(command) {
   // Use 'command -v' to find the full path
   const fullPath = execSync(`command -v ${command}`, {
     encoding: "utf8",
-    // @ts-expect-error shell is a string option
-    shell: true,
   }).trim();
 
   if (!fullPath) {
@@ -120,8 +118,12 @@ export async function safeSpawn(command, args, options = {}) {
     });
 
     child.on("close", (code) => {
+      // Code is null if it terminated by a signal. This should never
+      // happen in our code. If this happens, return 1 error code.
+
+      code = code ?? 1;
+
       resolve({
-        // @ts-expect-error code can be null
         status: code,
         stdout: stdout,
         stderr: stderr,