nodesecure/scanner

⚡️ A package API to run a static analysis of your module's dependencies. This is the CLI engine!

scanner's Issues

Add UT for src/logger.class.js

The goal is to add unit tests for the class ./src/logger.class.js.

Scripts to run tests:

• npm run test-only
• npm run coverage

We work with the tape lib and when there is need to mock/stub/spy we use Sinon.js. If ever a snapshot is needed we use snap-shot-core

Contributions are welcome.

TSConfig (scanner) cannot write file

Spent hours trying to fix that bug, doesn't impact compilation or anything but quite anoying. If someone has any ideas on how to fix this, please feel free to open a PR.

Improve TypeScript interfaces and types

The goal of this task is to improve our TypeScript interfaces and types in the root folder ./types. Most of them have zero description/context.

What does i mean by description ? Check this Sindresorhus guide: https://github.com/sindresorhus/typescript-definition-style-guide#documentation

An example in the NodeSecure vuln project: https://github.com/NodeSecure/vuln/blob/main/types/snyk-strategy.d.ts

Add banner

Add the scanner banner at the top of the README file like in RC project

Verify the behavior for a package alias

Package manager alow to alias packages, for example with npm

$ npm i test@npm:fastify

will install fastify aliased as test in the manifest.

"dependencies": {
  "test": "npm:fastify@^3.28.0"
}

[Tree-walker] Do not use resolveDependencyVersion with local scan strategy

The current implementation of the walk() method uses resolveDependencyVersion, which is useful when combined with walkRemoteDependency.

However, in scenarios where we provide a package-lock.json (or node_modules directory) location to be scanned by Arborist, we could simply iterate over edgesOut.

It should be quite similar to the implementation in walkLocalDependency

for (const [packageName, { to: toNode }] of node.edgesOut) {
  if (toNode === null || (!includeDevDeps && toNode.dev)) {
    continue;
  }
  const spec: NpmSpec = `${packageName}@${toNode.package.version}`;

  if (this.relationsMap.has(spec)) {
    this.relationsMap.get(spec)!.add(current.spec);
  }
}

However not sure how we should re-implement hasOutdatedDependency flag.

• Search the information in Arborist (if it exist)
• Use packument instead of manifest in the try/catch (the one who check existence of the Spec in the remote registry)

Unclear behaviour with vulnerabilities when using cwd

Hey!
I found unclear behaviour with vulnerabilities reporting when using cwd.

With this package.json:

{
  "private": true,
  "dependencies": {
    "classnames": "^2.1.5",
    "director": "^1.2.0",
    "react": "^0.13.3",
    "todomvc-app-css": "^2.0.0",
    "todomvc-common": "^1.0.1"
  },
  "devDependencies": {
    "bin-up": "^1.1.0"
  }
}

I do npm audit and get this output:

# npm audit report

react  >=0.0.1 <0.14.0
Severity: high
Cross-Site Scripting in react - https://github.com/advisories/GHSA-hg79-j56m-fxgv
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/react

1 high severity vulnerability

To address all issues (including breaking changes), run:
  npm audit fix --force

but when I use cwd:

await cwd(path, {
    vulnerabilityStrategy: "npm",
    fullLockMode: true
  })

I get vulnerabilities: [].

What am I doing wrong? Could you please clarify how this work?

verdaccio's pkg info does not contain the 'maintainers' field

We use Verdaccio as our private proxy registry. However the pkg info it returns does not contain the 'maintainers' & 'author' fields. As a result an error was thrown:

xxxx/@nodesecure/cli/node_modules/@nodesecure/scanner/src/utils/warnings.js:31
    for (const { name, email } of dependency.metadata.maintainers) {
                                                      ^
TypeError: dependency.metadata.maintainers is not iterable
    at getDependenciesWarnings (xxxx/@nodesecure/cli/node_modules/@nodesecure/scanner/src/utils/warnings.js:31:55)
    at depWalker (xxxx/@nodesecure/cli/node_modules/@nodesecure/scanner/src/depWalker.js:312:24)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async cwd (xxxx/@nodesecure/cli/src/commands/scanner.js:49:19)

---

Verdaccio: 5.8.0
Node.js: 16.14.2
NPM: 8.5.0

integrity doesn't match

There is probably too much warnings (detected from NodeSecure/CLI).

Add option to donate for this project

Hey!
I like this project and planning to use it in my pet project.
@fraxken Have you thought about adding github sponsorships? I would like to donate)

New warning idea - linting lockfile dependencies urls

There is a lockfile-lint project.
It would be great to add similar feature to warn users if some dependency url in package.lock (or yarn.lock) file has fake url.
Maybe even adopt and use lockfile-lint for it. What do you think?

Roadmap v6

Roadmap for the next major release:

• Migrate to TypeScript (all the codebase).
• Split codebase into multiple workspaces
• Refactor codebase (focus on testing).

Re-implement external packages into the Scanner workspace:

• license-conformance, npm-tarball-license-parser
• authors (see #224)

Difference between two payloads (new API)

Hello 👋

The goal of the feature is to compare two scanner payloads (one older and one recent) and get informations and delta between them. A feature like this could be very useful in a tool like CI.

For example, to find out if new licences have appeared in between. But we can apply this principle to several other elements:

• New maintainers (or some has been removed since).
• New dependencies added

To date I do not know what would be the ideal approach to develop this feature. Do not hesitate to give ideas!

[Compare] use V6 payload in UT

Current UT for compare payload use the payload from V5 but a lot of breaking changes has occured since, we need to update it.

Add support for npm github: dependency resolver.

I think pacote support the github: dependency resolver (like git+url.git). It would be really cool to add the possibility to analyze github packages automatically.

[email protected] no author?

JSON payload for @nodesecure/cli output three packages with absolutely no author (probably need to verify if everything is ok).

Add UT for src/tarball.js

The goal is to add unit tests for the class ./src/tarball.js.

Scripts to run tests:

• npm run test-only
• npm run coverage

We work with the tape lib and when there is need to mock/stub/spy we use Sinon.js. If ever a snapshot is needed we use snap-shot-core

Contributions are welcome.

Allow to set registry as options of the Scanner

There is actually no way to run cwd or from API with a given private registry (like verdaccio). Under the hood the registry is get/set with the @nodesecure/npm-registry-sdk package.

A workaround without this would be the following code

import fs from "node:fs";

import * as scanner from "@nodesecure/scanner";
import { setLocalRegistryURL } from "@nodesecure/npm-registry-sdk";

setLocalRegistryURL("http://localhost:4873");
const result = await scanner.cwd("...path...");

fs.writeFileSync("./nsecure-result.json", JSON.stringify(result, null, 2));

It would be cool to add the property registry that could take a string or WHATWG URL as argument in the Options interface:

scanner/types/scanner.d.ts

Lines 152 to 192 in a2bae88

	export interface Options {
	/**
	* Maximum tree depth
	*
	* @default 4
	*/
	readonly maxDepth?: number;
	/**
	* Use root package-lock.json. This will have the effect of triggering the Arborist package.
	*
	* @default false for from() API
	* @default true for cwd() API
	*/
	readonly usePackageLock?: boolean;
	/**
	* Include project devDependencies (only available for cwd command)
	*
	* @default false
	*/
	readonly includeDevDeps?: boolean;
	/**
	* Vulnerability strategy name (npm, snyk, node)
	*
	* @default NONE
	*/
	readonly vulnerabilityStrategy: Vuln.Strategy.Kind;
	/**
	* Analyze root package.
	*
	* @default false for from() API
	* @default true for cwd() API
	*/
	readonly forceRootAnalysis?: boolean;
	/**
	* Deeper dependencies analysis with cwd() API.
	*
	* @default false
	*/
	readonly fullLockMode?: boolean;
	}
	}

It would be possible to use the API as follows

await scanner.cwd("...path...", {
  registry: "http://localhost:4873"
});

Rework tarball

Tarball workspace need a rework. Current implementation is quite hard to test and evolve.

Fix UT (broken snapshot)

Current UT are broken by the verify snapshot (still not sure exactly why). My first guess was to force a .posix.normalize on file path but it didn't resolve the issue.

Re-implement Authors package from zero in the workspaces

Current authors package require a complete rewrite (from zero).

That's what I was trying to do in: NodeSecure/authors#52 (but kinda failed because it was more complicated that I imaginated).

The domain-check package should also be merged here (I don't think we need a dedicated workspace for it however).

One of the main goal is to simplify the integration with Scanner. But it will also drastically reduce the maintenance required.

@Kawacrepe

Synchronize @nodesecure/scanner type definitions with definitions from the latest @nodesecure/js-x-ray version

It appears that in the scanner.d.ts file some definitions are out of sync since last updates of @nodesecure/js-x-ray type definitions.

For instance:

scanner.d.ts

warnings: JSXRay.Warning<JSXRay.BaseWarning>[];

Major payload architecture enhancement

Current payload is described by the following TypeScript interface (in ./types/scanner.d.ts):

export interface VersionDescriptor {
  metadata: {
    dependencyCount: number;
    publishedCount: number;
    lastUpdateAt: number;
    lastVersion: number;
    hasChangedAuthor: boolean;
    hasManyPublishers: boolean;
    hasReceivedUpdateInOneYear: boolean;
    author: Maintainer;
    homepage: string | null;
    maintainers: Maintainer[];
    publishers: Publisher[];
  }
  versions: string[];
  vulnerabilities: (NpmStrategy.Vulnerability | NodeStrategy.Vulnerability)[];
  [version: string]: {
    id: number;
    usedBy: Record<string, string>;
    size: number;
    description: string;
    author: Maintainer;
    warnings: Warning<BaseWarning>[];
    composition: {
      extensions: string[];
      files: string[];
      minified: string[];
      required_files: string[];
      required_thirdparty: string[];
      required_nodejs: string[];
      unused: string[];
      missing: string[];
    };
    license: string | License[];
    flags: Flags;
    gitUrl: null | string;
  };
}

The versions list should be Record to get something close to:

export interface VersionDescriptor {
  metadata: {}
  vulnerabilities: (NpmStrategy.Vulnerability | NodeStrategy.Vulnerability)[];
  versions: Record<string, any>;
}

This update will have a major impact on front-end components and projects using the scanner. So it's SemVer major.

Add support for npm workspaces

See example in https://github.com/targos/nsecure-workspace

Result of nsecure cwd is included in the repo.

Modules structure:

node-secure data:

We see that only the top-level lodash dependency was analyzed.

Wrong number of dependencyCount

Sometimes the value of dependencyCount is wrong because of the limitation of how the walker work (it doesn't scan recursively, so package with many same parents may be ignored).

For example when scanning express package, the raw-body dependency must have 4 dependency.

Adding a new diff API

One of the needs that is more and more requested is to make the difference between two analyses (two payload).

We probably want to get out information like:

• New licenses
• New authors/maintainers
• New packages
• New warnings/flags

This would make it possible to build nice tools especially for CIs

Migrate NPM dependency walker in workspaces

The idea is the same as the work done for tarball workspace. Migrate the codebase in src/depWalker.js into a separated workspace tree-walker (including migrating to TypeScript).

Note that the root function depWalker should remain in Scanner, what we need to migrate is:

• getRootDependencies
• searchDeepDependencies
• deepReadEdges

And all related utils.

The idea is not to make any major changes for the time being (except thoses required to be typesafe). We'll be doing some refactoring later on.

Implement SAST warnings for SemVer 0.0.0

Implement a warning for package.json with version equal to zero:

• 0.0.0
• 0.0
• 0

My idea is to dynamically inject an "Information" SAST warning. It just mean the source is not JS-X-Ray but Scanner (we must check what's the impact of that).

Detect if root project (or git projects) are also available on NPM

Right now the scanner doesn't scan for an NPM package when this is the root project or any git+ resolver.

However this kind of info could be useful in the payload to be sure in the UI how to react and what to show. Example if i launch a cwd analysis at the root of this project then i should be able to tell that @nodesecure/scanner is also a valid published package on npm.

However some local project are not npm package (so in the UI we can't show a link to Unpkg etc).

For the root project, we should be able to add an HTTP call with pacote somewhere close to this line:

scanner/src/depWalker.js

Line 125 in adf01b9

const parent = new Dependency(manifest.name, manifest.version);

Something like this probably do the work:

try {
  await pacote.manifest(`${manifest.name}@${manifest.version}`);
  parent.hasNpmPackage = true;
}
catch {
  parent.hasNpmPackage = false;
}

Transfer back required i18n translation

Hello 👋

We are currently using @nodesecure/i18n in ./src/utils/warnings.js to load translation for some unsafe package warnings.

scanner/src/utils/warnings.js

Lines 11 to 14 in 21e9175

	const kDependencyWarnMessage = Object.freeze({
	"@scarf/scarf": await getToken("warnings.disable_scarf"),
	iohook: await getToken("warnings.keylogging")
	});

The goal of the issue would be to use the new i18n extendFromSystemPath API to transfer back those translations in this project (in a root i18n folder).

import * as i18n from "@nodesecure/i18n";

// execute when package is loaded
i18n.extendFromSystemPath("path/to/i18n");

const kDependencyWarnMessage = Object.freeze({
  "@scarf/scarf": await getToken("warnings.disable_scarf"),
  iohook: await getToken("warnings.keylogging")
});

This would greatly simplify maintenance by allowing an update of the translations right here.

Translations can be retrieved from i18n repository:

• https://github.com/NodeSecure/i18n/blob/c48806bf2d9cb74627ef5657d0b7c2b463604704/languages/french.js#L118-L121
• https://github.com/NodeSecure/i18n/blob/c48806bf2d9cb74627ef5657d0b7c2b463604704/languages/english.js#L118-L121

Add documentation for cwd and from API

Add a root ./docs folder with complete documentation on how work cwd and from API.

contributeurs

Compare and validate packument and tarball manifest

The scanner should search for obvious tentative of confusion between the packument and the manifest in the tarball.

See/read: https://blog.vlt.sh/blog/the-massive-hole-in-the-npm-ecosystem

Sort dependencies & scripts for integrity

Integrity hash may differ because the order of the keys in scripts and dependencies are not the same between the manifest in the tarball and the manifest in the registry.

[Tree-walker] Optimize remote strategy

The remote strategy require resolveDependencyVersion method that will use packument to be able to resolve the latest version of the packing matching the SemVer range provided (in the package.json payload).

This is quite a lot of HTTP requests and a lot of resource wasted. We probably need to rethink how we should resolve the tree do reduce the quantity of requests required to fetch everything.

Migrate licenses-conformance and npm-tarball-license-parser in the workspaces

My idea is to create a conformance workspace in here in Scanner and combine npm-tarball-license-parser and licenses-conformance` in one package.

The idea behind this choice is that it is currently very complex to evolve these components separately. Maintenance would be significantly improved!

Flag git package with cwd (Arborist) analysis

Current version only flag isGit when we analyze package without Arborist. With the deepReadEdges function however there is no code to detect if we are analyzing a custom resolver or not.

In the package-lock.json we can detect if the package is git by looking at the resolution (or something like this).

We can use utils.isGitDependency(resolved) and flag the package.

Enhance logs & errors management

Current implementation doesn't really deal well with logging and errors.

For example it would be could to remove the async-cli-spinner from this package and provide a mechanism to be able to reproduce the same behavior in the NodeSecure CLI.

Also a lot of errors are currently not catched (just ignored because there is no catch).