From 240123372ab9ac0b469301ea5a0eec274b87bf70 Mon Sep 17 00:00:00 2001
From: Sander Declerck <sander@sdsoftware.be>
Date: Wed, 8 Oct 2025 10:49:04 +0200
Subject: [PATCH] Handle PR Comments

---
 packages/safe-chain/src/main.js                    | 14 ++++++++------
 .../safe-chain/src/registryProxy/registryProxy.js  | 13 +++++++++----
 packages/safe-chain/src/scanning/index.js          |  7 +++----
 .../safe-chain/src/scanning/malwareDatabase.js     |  2 ++
 packages/safe-chain/src/utils/safeSpawn.js         |  2 ++
 5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/packages/safe-chain/src/main.js b/packages/safe-chain/src/main.js
index da36a59..e106e83 100644
--- a/packages/safe-chain/src/main.js
+++ b/packages/safe-chain/src/main.js
@@ -16,18 +16,20 @@ export async function main(args) {
     args = initializeCliArguments(args);
 
     if (shouldScanCommand(args)) {
-      const resultCode = await scanCommand(args);
+      const commandScanResult = await scanCommand(args);
 
       // Returning the exit code back to the caller allows the promise
       //  to be awaited in the bin files and return the correct exit code
-      if (resultCode !== 0) {
-        return resultCode;
+      if (commandScanResult !== 0) {
+        return commandScanResult;
       }
     }
 
-    var result = await getPackageManager().runCommand(args);
+    const packageManagerResult = await getPackageManager().runCommand(args);
 
-    proxy.verifyNoMaliciousPackages();
+    if (!proxy.verifyNoMaliciousPackages()) {
+      return 1;
+    }
 
     ui.emptyLine();
     ui.writeInformation(
@@ -38,7 +40,7 @@ export async function main(args) {
 
     // Returning the exit code back to the caller allows the promise
     //  to be awaited in the bin files and return the correct exit code
-    return result.status;
+    return packageManagerResult.status;
   } catch (error) {
     ui.writeError("Failed to check for malicious packages:", error.message);
 
diff --git a/packages/safe-chain/src/registryProxy/registryProxy.js b/packages/safe-chain/src/registryProxy/registryProxy.js
index 9880129..3558673 100644
--- a/packages/safe-chain/src/registryProxy/registryProxy.js
+++ b/packages/safe-chain/src/registryProxy/registryProxy.js
@@ -7,6 +7,7 @@ import { knownRegistries, parsePackageFromUrl } from "./parsePackageFromUrl.js";
 import { ui } from "../environment/userInteraction.js";
 import chalk from "chalk";
 
+const SERVER_STOP_TIMEOUT_MS = 1000;
 const state = {
   port: null,
   blockedRequests: [],
@@ -19,12 +20,15 @@ export function createSafeChainProxy() {
   return {
     startServer: () => startServer(server),
     stopServer: () => stopServer(server),
-    getBlockedRequests: () => state.blockedRequests,
     verifyNoMaliciousPackages,
   };
 }
 
 function getSafeChainProxyEnvironmentVariables() {
+  if (!state.port) {
+    return {};
+  }
+
   return {
     HTTPS_PROXY: `http://localhost:${state.port}`,
     GLOBAL_AGENT_HTTP_PROXY: `http://localhost:${state.port}`,
@@ -63,6 +67,7 @@ function createProxyServer() {
 
 function startServer(server) {
   return new Promise((resolve, reject) => {
+    // Passing port 0 makes the OS assign an available port
     server.listen(0, () => {
       const address = server.address();
       if (address && typeof address === "object") {
@@ -88,7 +93,7 @@ function stopServer(server) {
     } catch {
       resolve();
     }
-    setTimeout(() => resolve(), 1000);
+    setTimeout(() => resolve(), SERVER_STOP_TIMEOUT_MS);
   });
 }
 
@@ -130,7 +135,7 @@ async function isAllowedUrl(url) {
 function verifyNoMaliciousPackages() {
   if (state.blockedRequests.length === 0) {
     // No malicious packages were blocked, so nothing to block
-    return;
+    return true;
   }
 
   ui.emptyLine();
@@ -149,5 +154,5 @@ function verifyNoMaliciousPackages() {
   ui.writeError("Exiting without installing malicious packages.");
   ui.emptyLine();
 
-  process.exit(1);
+  return false;
 }
diff --git a/packages/safe-chain/src/scanning/index.js b/packages/safe-chain/src/scanning/index.js
index 91314ae..36f62ca 100644
--- a/packages/safe-chain/src/scanning/index.js
+++ b/packages/safe-chain/src/scanning/index.js
@@ -65,8 +65,7 @@ export async function scanCommand(args) {
     return 0;
   } else {
     printMaliciousChanges(audit.disallowedChanges, spinner);
-    await onMalwareFound();
-    return 1;
+    return await onMalwareFound();
   }
 }
 
@@ -90,11 +89,11 @@ async function onMalwareFound() {
 
     if (continueInstall) {
       ui.writeWarning("Continuing with the installation despite the risks...");
-      return;
+      return 0;
     }
   }
 
   ui.writeError("Exiting without installing malicious packages.");
   ui.emptyLine();
-  process.exit(1);
+  return 1;
 }
diff --git a/packages/safe-chain/src/scanning/malwareDatabase.js b/packages/safe-chain/src/scanning/malwareDatabase.js
index 0181e5e..1cb781b 100644
--- a/packages/safe-chain/src/scanning/malwareDatabase.js
+++ b/packages/safe-chain/src/scanning/malwareDatabase.js
@@ -31,6 +31,8 @@ export async function openMalwareDatabase() {
     return packageData.reason;
   }
 
+  // This implicitely caches the malware database
+  //  that's closed over by the getPackageStatus function
   cachedMalwareDatabase = {
     getPackageStatus,
     isMalware: (name, version) => {
diff --git a/packages/safe-chain/src/utils/safeSpawn.js b/packages/safe-chain/src/utils/safeSpawn.js
index 5af4740..c5cd913 100644
--- a/packages/safe-chain/src/utils/safeSpawn.js
+++ b/packages/safe-chain/src/utils/safeSpawn.js
@@ -22,6 +22,8 @@ export async function safeSpawn(command, args, options = {}) {
   const fullCommand = buildCommand(command, args);
   return new Promise((resolve, reject) => {
     const child = spawn(fullCommand, { ...options, shell: true });
+
+    // When stdio is piped, we need to collect the output
     let stdout = "";
     let stderr = "";