Dependency vulnerabilities (4 high in prod, critical in dev) and no SCA/secret scanning in CI

[tcheeric]

Summary

npm audit reports 6 vulnerabilities in the production dependency tree (4 high, 1 moderate, 1 low) and 18 overall. Every one has a fix available. CI runs npm ci, typecheck and test with no dependency scanning, secret scanning, or SAST, so none of this is visible on a pull request.

Severity: Medium — two of the four high findings bear directly on NAP's own security properties.

Production dependencies

$ npm audit --omit=dev
fastify         high      Request/Response spoofing + schema validation bypass
fast-uri        high      Host confusion (7 CVEs), SSRF via IPv6 normalization
find-my-way     high      HTTP/2 DDoS
path-to-regexp  high      ReDoS via multiple route parameters
qs              moderate  DoS via attacker-controlled isBuffer
body-parser     low       Size enforcement silently disabled on invalid limit

Two of these are not generic transitive noise — they undercut controls this repo implements deliberately:

fastify: "request.protocol and request.host spoofable via X-Forwarded-Proto/Host from untrusted connections." createRequestDerivedBaseUrlResolver() in nap-adapter-fastify resolves the NIP-98 audience with allow(req.headers.host, req.protocol). The audience allowlist is the control that makes a spoofable Host safe, and audience.ts is explicit that a scheme-pinned entry is "the only way to stop an X-Forwarded-Proto a misconfigured trust proxy believes from downgrading the audience to http." A framework-level spoofing bug is the layer underneath that defence.

body-parser: "size enforcement silently disabled when the limit value is invalid." createNapExpressJsonParser() passes bodyLimit straight to express.json({ limit }), and its docstring explains the 1 kB cap exists because "100 kB on an unauthenticated endpoint is 100 kB of parsing an anonymous caller can buy per request." A malformed limit string silently removing that cap defeats the control without any visible symptom.

fast-uri: SSRF via malformed IPv6 normalization + host confusion. Worth calling out given nap-voucher makes outbound calls to mint URLs and treats SSRF as the top-severity concern in allowlist.ts and mintClient.ts. The mint allowlist uses WHATWG URL, not fast-uri, so this is not currently exploitable through that path — but it is the same vulnerability class the extension is built to prevent.

Dev dependencies

vitest is critical (arbitrary file read and execution when the UI server is listening; path traversal via @vitest/mocker), and vite/esbuild are high. These do not ship to consumers, but they run on every CI job and every contributor's machine, which is the SolarWinds-shaped risk: a build-system compromise does not need to reach production to matter.

The vitest and testcontainers fixes are semver-major (vitest@2 → 4, testcontainers@10 → 12), so those need a deliberate upgrade rather than npm audit fix.

Fix

Two parts, both small:

1. Update the dependencies.

npm audit fix                    # clears the 6 production findings
npm i -D vitest@^4 testcontainers@^12   # major, needs a test run

2. Add scanning to CI, so this does not silently regrow. Append to .github/workflows/ci.yml:

  audit:
    name: Dependency audit
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-node@v4
        with: { node-version: '22.x', cache: npm }
      - run: npm ci
      # Production tree fails the build; the dev tree is reported but does not
      # block, so a vitest advisory cannot wedge every PR on an unrelated change.
      - name: Audit production dependencies
        run: npm audit --omit=dev --audit-level=high
      - name: Audit all dependencies (advisory)
        run: npm audit --audit-level=moderate || true

Splitting production from dev is the part that keeps this tuned rather than noisy — the AppSec rule of thumb is that a scanner developers learn to ignore has negative value. The production gate is small, actionable, and should always be green.

Worth adding alongside:

• Dependabot (.github/dependabot.yml, weekly, npm) so upgrades arrive as reviewable PRs rather than as audit debt.
• Secret scanning + push protection in repository settings. A manual scan of the tree found no committed secrets today, which is the right moment to turn the guard on.
• CodeQL for JavaScript/TypeScript. It would not have caught the two logic bugs filed as Unhandled TypeError on a malformed NIP-98 u tag turns /auth/complete into a 500 (unauthenticated) #33 and writeNapCookieSuccess() defaults to a session cookie with no HttpOnly, Secure or SameSite #34, but it is close to free and covers the classes nobody is reading for.

Note

Nothing here is a vulnerability in NAP's own code. The protocol implementation is careful and well-reasoned. This is about the 80% of an application that is third-party code and the pipeline that keeps it current.

[tcheeric]

Fixed in 79f7a65 (branch develop, not pushed)

Production tree clean. npm audit --omit=dev went from {low 1, moderate 1, high 4, critical 0} to {low 0, moderate 1, high 0, critical 0}. All four high-severity findings are gone, including the two that bore on NAP's own controls: the Fastify request.protocol / request.host spoofing under createRequestDerivedBaseUrlResolver(), and the body-parser limit bypass under createNapExpressJsonParser().

The one remaining moderate has no non-breaking fix available and does not sit on a security-critical path.

The dev upgrade was attempted and kept. vitest 2 to 4 and testcontainers 10 to 12 are both semver-major, so the instruction was to revert if they needed non-trivial test rewrites. They did not: the full suite passes on vitest 4 at 708 tests, 51 files, 15 skipped, and npm run typecheck is clean. That clears the critical vitest advisory (arbitrary file read and execution when the UI server is listening) rather than carrying it.

CI scanning added as a separate audit job, deliberately split:

- run: npm audit --omit=dev --audit-level=high   # gates the build
- run: npm audit --audit-level=moderate || true  # advisory only

The production tree gates; the dev tree reports. That split is what keeps this tuned rather than noisy, since a future dev-only advisory should not wedge every unrelated PR. A scanner developers learn to ignore has negative value.

.github/dependabot.yml added (weekly, npm), so upgrades arrive as reviewable PRs instead of accumulating as audit debt.

Still open from this issue

Secret scanning and push protection are repository settings, not files, so they cannot be committed. A manual scan found nothing committed today, which is the right moment to enable them. CodeQL was not added; worth a follow-up but it would not have caught either logic bug in #33 or #34.

[tcheeric]

Fixed in #40 (branch security/appsec-audit-remediation, released as 0.11.0). The PR body carries the per-finding verification; this issue's earlier comments carry the detail and the reproduction. Will close on merge.

[tcheeric]

Amended: three moderate qs advisories were under the gate

This issue was closed out on "four high-severity findings to none". That was true at --audit-level=high, which is where the gate was set, and three moderate qs advisories were sitting underneath it the whole time:

• GHSA-4mjr-xmp4-gh2g (DoS via attacker-controlled isBuffer)
• GHSA-q8mj-m7cp-5q26 (remotely triggerable DoS in qs.stringify)
• GHSA-x5fp-wj9c-mxmx (array-limit bypass via bracket-key comma parsing)

qs is Express's query parser, so these are on the request path of every deployment using the Express adapter, not a build-time concern.

Found by cross-checking the lockfile against OSV, which applies no severity floor, after the same check on nap-java turned up a critical in bcprov. A threshold only holds the line it is set at, and "no highs" was doing a lot of quiet work in that sentence.

npm audit fix does not reach it: express pins qs at ~6.14.0 and ~ locks the minor, so no express 4.x release resolves past 6.14.x. Fixed with an overrides entry scoped under express, plus the gate moved to --audit-level=moderate now that the production tree is genuinely at zero.

Two mistakes worth recording, both caught by checking rather than assuming:

• The first override I wrote was ^6.15.4, taken from the advisory's "fixed in" boundary. That version does not exist (the range is 2.2.5 - 6.15.3; the next published release is 6.16.0). npm silently ignores an unsatisfiable override rather than failing, so it looked applied and was not.
• Regenerating the lockfile to pick up the override rehoisted vite and broke typecheck with two copies of its types. Reverted and patched the existing lockfile instead: churn went from 9274 lines to 9.

Verified both directions: the new gate passes on the fixed tree and fails on the tree as it stood before. 740 passed, typecheck clean, npm ci reproduces qs 6.16.0.