auth0/node-jsonwebtoken
As a library whose entire job is security, nearly every line in verify.js is a security-sensitive path, and the ordering of its checks is the load-bearing invariant — any reordering that lets `jws.verify` run before the algorithm allow-list check reintroduces the classic JWT confusion attacks. The most concrete correctness defect is in sign.js's `options_to_payload` loop, where `return failure(...)` inside a `forEach` callback does not abort the enclosing function, so in async mode the callback can fire twice. Secondary risks cluster around Node-version-conditional code that CI does not fully exercise, deliberately silent error swallowing in `decode`, a very old eslint pinned at v4, and release workflows that handle several third-party secrets. Blast radius for any change here is unusually wide: this package sits in the dependency tree of a large fraction of Node auth stacks.
- CORRECTNESS BUG: in the `options_to_payload` loop, `return failure(...)` only returns from the `forEach` arrow — the outer sign function continues. In async mode `failure` invokes the raw callback with an error at line 221, and since `callback` is not yet wrapped by `once()` (that happens at line 230), signing proceeds and the callback is invoked a second time with a valid token.
- SECURITY-CRITICAL ORDERING: the allow-list check (`options.algorithms.indexOf(header.alg) === -1`), the key-type symmetry check, and `validateAsymmetricKey` all sit between `decode` and `jws.verify`. Any edit that moves `jws.verify` earlier, or that trusts `header.alg` before line 144, reopens the RS256→HS256 confusion class.
- `options.header` is `Object.assign`ed LAST when building the sign header, so a caller passing `{header: {alg: 'none'}}` silently overrides `options.algorithm` — and the downstream key/alg checks then run against the overridden value. This is by design but is a sharp edge for callers who build headers from user input.
- `decode` swallows all `JSON.parse` errors in an empty `catch (e) { }`, returning the raw string payload instead. Callers that assume an object back from `decode` will see a string on malformed tokens with no signal.
- The `signature.length < 256` heuristic is an acknowledged stopgap for the 2048-bit modulus rule on Node < 15, duplicated in both the sync and async branches with matching `TODO: Remove ...` comments — a two-site invariant that can drift.
- No source file uses `'use strict'`, so sloppy-mode semantics apply — e.g. `delete payload.iat` on a primitive string payload under `noTimestamp` is a silent no-op rather than a TypeError. Notably `npm run coverage` runs mocha with `--use_strict` while `npm test` does not, so the two commands exercise different semantics.
- The EC curve check dereferences `key.asymmetricKeyDetails.namedCurve` without a null guard; it is protected only by the module-load `ASYMMETRIC_KEY_DETAILS_SUPPORTED` semver boolean, and that whole block is excluded from coverage by `istanbul ignore next`.
- The verified payload returned by `verify` is the same object `decode` produced, unsanitized and fully attacker-controlled in content; the README carries an explicit warning to treat it as user input, which implies downstream misuse is a known recurring problem.
- `allowInvalidAsymmetricKeyTypes: true` disables `validateAsymmetricKey` on both sign and verify, and a test asserts it lets an ES256-labelled token be processed with an RSA key — a documented footgun kept only for backwards compatibility.
- `release.yml` handles six secrets (`PRODSEC_TOOLS_ARN`, `PRODSEC_TOOLS_USER`, `PRODSEC_TOOLS_TOKEN`, `RLSECURE_LICENSE`, `RLSECURE_SITE_KEY`, `SIGNAL_HANDLER_TOKEN`) and embeds credentials in a pip `--index-url`. Mitigations are in place — `persist-credentials: false`, `mask-aws-account-id: true`, SHA-pinned actions — but this job is the highest-value supply-chain target in the repo.
- `prepare-release.yml` parses the next version out of semantic-release's human-readable stdout with `awk '/The next release version is/{print $NF}'` — a brittle coupling to log wording that would silently produce an empty version if the message text changes.
- DEAD CODE: `bin/changelog` is a standalone conventional-changelog script with hardcoded `releaseCount: 19` and a commented-out `preset: 'jshint'`, but nothing references it — `package.json` and `prepare-release.yml` both go through the `update-changelog` npm script instead (per commit b924272).
- STALE TOOLING: `eslint` is pinned at `^4.19.1` (with `ecmaVersion: 6` in the parser options, below the ES2018+ syntax the sources actually use), and `nsp@^2.6.2` — a long-deprecated security tool — remains a devDependency with no script invoking it.
- The README still advertises Travis CI and david-dm badges even though CI has moved to GitHub Actions, and the TODO section admits `X.509 certificate chain is not checked` — a real, documented limitation for anyone using x509 PEMs as verification keys.
- The `files` allow-list omits `index.js` despite it being `main`; npm force-includes the `main` file so the package works today, but the list is one refactor (e.g. moving the entry to `lib/`) away from shipping a broken tarball. (inferred)