refactor: enhance error resilience & add missing NPM userAgent

.changeset/orange-points-reply.md

@@ -0,0 +1,7 @@
+---
+"@nodesecure/tree-walker": major
+"@nodesecure/scanner": patch
+"@nodesecure/tarball": patch
+---
+
+Enhance error resilience and add missing pacote (npm) userAgent for all HTTP requests

workspaces/scanner/src/depWalker.ts

@@ -134,7 +134,11 @@ export async function depWalker(
   const tokenStore = new RegistryTokenStore(npmRcConfig, NPM_TOKEN.token, npmRcEntries);
 
   const npmProjectConfig = tokenStore.getConfig(registry);
-  const pacoteScopedConfig = { ...npmProjectConfig, ...npmRcEntries };
+  const pacoteScopedConfig = {
+    ...npmProjectConfig,
+    ...npmRcEntries,
+    userAgent: `@nodesecure/scanner node/${process.version}`
+  };
 
   const pacoteProvider: PacoteProvider = {
     async extract(spec, dest, opts): Promise<void> {
@@ -201,29 +205,30 @@ export async function depWalker(
       () => npmRegistrySDK.org(namespace)
     )
   };
-  {
-    logger
-      .start(ScannerLoggerEvents.analysis.tree)
-      .start(ScannerLoggerEvents.analysis.tarball)
-      .start(ScannerLoggerEvents.analysis.registry);
-    const fetchedMetadataPackages = new Set<string>();
-    const operationsQueue: Promise<void>[] = [];
-
-    await using tarballScanner = new TarballScanner({
-      tempDir,
-      statsCollector,
-      pacoteProvider,
-      collectables,
-      maxConcurrency,
-      logger,
-      workers
-    });
 
-    const rootDepsOptions: npm.WalkOptions = {
-      maxDepth,
-      includeDevDeps,
-      packageLock
-    };
+  logger
+    .start(ScannerLoggerEvents.analysis.tree)
+    .start(ScannerLoggerEvents.analysis.tarball)
+    .start(ScannerLoggerEvents.analysis.registry);
+  const fetchedMetadataPackages = new Set<string>();
+  const operationsQueue: Promise<void>[] = [];
+
+  await using tarballScanner = new TarballScanner({
+    tempDir,
+    statsCollector,
+    pacoteProvider,
+    collectables,
+    maxConcurrency,
+    logger,
+    workers
+  });
+
+  const rootDepsOptions: npm.WalkOptions = {
+    maxDepth,
+    includeDevDeps,
+    packageLock
+  };
+  try {
     for await (const current of npmTreeWalker.walk(manifest, rootDepsOptions)) {
       const { name, version, integrity, ...currentVersion } = current;
       const dependency: Dependency = {
@@ -316,7 +321,8 @@ export async function depWalker(
         })
       );
     }
-
+  }
+  finally {
     logger.end(ScannerLoggerEvents.analysis.tree);
     await Promise.allSettled(operationsQueue);
 

workspaces/scanner/src/index.ts

@@ -86,7 +86,8 @@ export async function from(
 
   logger.start(ScannerLoggerEvents.manifest.fetch);
   const manifest = await pacote.manifest(packageName, {
-    ...NPM_TOKEN, registry, cache: `${os.homedir()}/.npm`
+    ...NPM_TOKEN, registry, cache: `${os.homedir()}/.npm`,
+    userAgent: `@nodesecure/scanner node/${process.version}`
   });
   logger.end(ScannerLoggerEvents.manifest.fetch);
 

workspaces/tarball/src/tarball.ts

@@ -226,7 +226,8 @@ export async function extractAndResolve(
     {
       ...NPM_TOKEN,
       registry,
-      cache: `${os.homedir()}/.npm`
+      cache: `${os.homedir()}/.npm`,
+      userAgent: `@nodesecure/tarball node/${process.version}`
     }
   );
 

workspaces/tree-walker/src/npm/walker.ts

@@ -127,7 +127,8 @@ export class TreeWalker {
     return {
       ...utils.NPM_TOKEN,
       registry: this.registry,
-      cache: `${os.homedir()}/.npm`
+      cache: `${os.homedir()}/.npm`,
+      userAgent: `@nodesecure/tree-walker node/${process.version}`
     };
   }
 
@@ -177,10 +178,20 @@ export class TreeWalker {
   ): AsyncIterableIterator<Dependency> {
     const { currDepth = 1, parent, maxDepth, gitURL } = options;
 
-    const { name, version, deprecated, ...pkg } = await this.providers.pacote.manifest(
-      gitURL ?? spec,
-      this.registryOptions
-    );
+    let manifest: pacote.AbbreviatedManifest & pacote.ManifestResult;
+    try {
+      manifest = await this.providers.pacote.manifest(
+        gitURL ?? spec,
+        this.registryOptions
+      );
+    }
+    catch {
+      // Unable to fetch manifest for this dependency (e.g. registry unreachable, 403, etc.)
+      // Skip this branch of the tree entirely rather than crashing the whole walk.
+      return;
+    }
+
+    const { name, version, deprecated, ...pkg } = manifest;
     const { dependencies, customResolvers, alias } = utils.mergeDependencies(pkg);
 
     const current = new Dependency(name, version, {
@@ -367,7 +378,7 @@ export class TreeWalker {
           ...packageLockOptions
         }));
 
-      for await (const dep of combineAsyncIterators({}, ...iterators)) {
+      for await (const dep of combineAsyncIterators({ throwError: false }, ...iterators)) {
         yield dep.exportAsPlainObject();
       }
 
@@ -392,7 +403,7 @@ export class TreeWalker {
         .map(dependencies.entries(), ([name, ver]) => this.walkRemoteDependency(`${name}@${ver}`, walkRemoteOptions))
     ];
 
-    for await (const dep of combineAsyncIterators({}, ...iterators)) {
+    for await (const dep of combineAsyncIterators({ throwError: false }, ...iterators)) {
       yield dep.exportAsPlainObject();
     }
 

workspaces/tree-walker/test/npm/TreeWalker.spec.ts

@@ -239,6 +239,159 @@ describe("npm.TreeWalker", () => {
       );
     });
   });
+
+  describe("error resilience", () => {
+    test(`Given a manifest with transitive dependencies and a mocked pacote that fails on transitive fetches
+      it should gracefully skip the failing dependencies and still yield the root`, async(t) => {
+      const rootManifest = {
+        name: "test-pkg",
+        version: "1.0.0",
+        dependencies: {
+          "dep-a": "^1.0.0",
+          "dep-b": "^2.0.0"
+        }
+      };
+
+      let callCount = 0;
+      const mockedPacoteProvider = {
+        manifest: t.mock.fn(async(_: string) => {
+          callCount++;
+          // First call is for root integrity check, let it fail (existOnRemoteRegistry = false)
+          if (callCount === 1) {
+            throw new Error("root not found");
+          }
+          // Transitive dependency fetches all fail (simulating registry 403)
+          throw new Error("403 Forbidden");
+        }),
+        packument: t.mock.fn(async() => {
+          throw new Error("403 Forbidden");
+        })
+      };
+
+      const walker = new npm.TreeWalker({
+        providers: {
+          pacote: mockedPacoteProvider
+        }
+      });
+
+      const dependencies: DependencyJSON[] = [];
+      for await (const dependency of walker.walk(rootManifest as any, { maxDepth: 10 })) {
+        dependencies.push(dependency);
+      }
+
+      // Should still yield the root dependency despite all transitive fetches failing
+      assert.strictEqual(dependencies.length, 1);
+      assert.strictEqual(dependencies[0].name, "test-pkg");
+      assert.strictEqual(dependencies[0].id, 0);
+      assert.strictEqual(dependencies[0].existOnRemoteRegistry, false);
+    });
+
+    test(`Given a manifest where some transitive dependencies fail and others succeed
+      it should yield the successful ones and skip the failures without leaking promises`, async(t) => {
+      const rootManifest = {
+        name: "test-pkg",
+        version: "1.0.0",
+        dependencies: {
+          "good-dep": "^1.0.0",
+          "bad-dep": "^1.0.0"
+        }
+      };
+
+      let callCount = 0;
+      const mockedPacoteProvider = {
+        manifest: t.mock.fn(async(spec: string) => {
+          callCount++;
+          // First call: root integrity check -> fail
+          if (callCount === 1) {
+            throw new Error("root not found");
+          }
+          // good-dep resolves successfully
+          if (typeof spec === "string" && spec.includes("good-dep")) {
+            return {
+              name: "good-dep",
+              version: "1.0.0",
+              dependencies: {},
+              _integrity: "sha512-good"
+            };
+          }
+
+          // bad-dep always fails
+          throw new Error("503 Service Unavailable");
+        }),
+        packument: t.mock.fn(async(name: string) => {
+          if (name === "good-dep") {
+            return {
+              "dist-tags": { latest: "1.0.0" },
+              versions: {
+                "1.0.0": { version: "1.0.0" }
+              }
+            };
+          }
+
+          throw new Error("503 Service Unavailable");
+        })
+      };
+
+      const walker = new npm.TreeWalker({
+        providers: {
+          // @ts-expect-error
+          pacote: mockedPacoteProvider
+        }
+      });
+
+      const dependencies: DependencyJSON[] = [];
+      for await (const dependency of walker.walk(rootManifest, { maxDepth: 10 })) {
+        dependencies.push(dependency);
+      }
+
+      const names = dependencies
+        .map((dependency) => dependency.name)
+        .sort();
+      // Should yield good-dep and root, but not bad-dep
+      assert.ok(names.includes("test-pkg"), "root dependency should be present");
+      assert.ok(names.includes("good-dep"), "successful dependency should be present");
+      assert.ok(!names.includes("bad-dep"), "failed dependency should be skipped");
+    });
+
+    test(`Given a manifest where all transitive dependency fetches throw
+      it should not produce unhandled promise rejections`, async(t) => {
+      const rootManifest = {
+        name: "test-pkg",
+        version: "1.0.0",
+        dependencies: {
+          "fail-a": "^1.0.0",
+          "fail-b": "^1.0.0",
+          "fail-c": "^1.0.0"
+        }
+      };
+
+      const mockedPacoteProvider = {
+        manifest: t.mock.fn(async() => {
+          throw new Error("registry unreachable");
+        }),
+        packument: t.mock.fn(async() => {
+          throw new Error("registry unreachable");
+        })
+      };
+
+      const walker = new npm.TreeWalker({
+        providers: {
+          pacote: mockedPacoteProvider
+        }
+      });
+
+      // This should complete without throwing and without leaking unhandled rejections
+      const dependencies: DependencyJSON[] = [];
+      for await (const dependency of walker.walk(rootManifest, { maxDepth: 10 })) {
+        dependencies.push(dependency);
+      }
+
+      // Only root should be yielded
+      assert.strictEqual(dependencies.length, 1);
+      assert.strictEqual(dependencies[0].name, "test-pkg");
+      assert.strictEqual(dependencies[0].existOnRemoteRegistry, false);
+    });
+  });
 });
 
 function sortByName(left: string, right: string) {