Skip to content

Validate diff input: restrict globs and options, return 400/404 instead of 500 - #1071

Merged
oBusk merged 12 commits into
mainfrom
claude/project-audit-findings-iezfst-input-validation
Oct 1, 2026
Merged

oBusk merged 12 commits into
mainfrom
claude/project-audit-findings-iezfst-input-validation

Conversation

@oBusk

@oBusk oBusk commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Fixes audit findings H1, M3 (options), L2 and L3.

H1: CPU DoS via diffFiles globs

minimatch compiles globs to backtracking regexes, and libnpmdiff matches every pattern against every file in both tarballs. Patterns like *(?|?)Z or *?*?*?*?Z take seconds per file. diffFiles is now limited to:

  • at most 10 patterns of at most 256 characters
  • at most 16 brace alternatives in total, counted after libnpmdiff's own normalization, so \{a,b} can't sneak through
  • no extglobs (the default **/!(*.map|*.min.js) is still allowed)
  • at most two * outside of ** segments

Part of the per-file cost is libnpmdiff recompiling every glob for each file; that's fixed upstream in npm/cli#10037. Once that lands, the 16-alternative cap can be relaxed. The extglob and * limits stay, since that cost is in matching, not compiling.

M3: option values

  • diffUnified must be an integer from 0 to 100.
  • diffSrcPrefix and diffDstPrefix can be at most 64 characters, with no control characters.

L2: invalid input returned 500

  • An invalid spec gives 400 from the API and a not-found page on the site.
  • Invalid options give 400 from the API and an "Invalid options" box on the site.
  • A package or version missing from the registry gives 404 from the API and a not-found page on the site.

L3: query strings rebuilt without encoding

Redirects and links now build query strings with URLSearchParams, so values stay encoded and repeated diffFiles are kept.

Behavior changes

  • Custom diffFiles that use extglobs, or more than two *, are rejected.
  • Not-found on the page is streamed, so the status is 200 (Next adds noindex). The API returns a real 404.

🤖 Generated with Claude Code

@vercel

vercel Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
npm-diff Ready Ready Preview Oct 1, 2026 10:10pm UTC

Comment thread src/lib/query/searchParams.ts Fixed
… instead of 500

- Reject diffFiles with more than 10 patterns, patterns longer than 256
  characters, or patterns whose brace expansion exceeds 64 alternatives
  (checked with a capped minimatch braceExpand) before calling libnpmdiff.
- Require diffUnified to be an integer 0-100 and diffSrcPrefix/diffDstPrefix
  to be at most 64 characters; invalid values are rejected, not coerced.
- Share parts parsing (decode, split, validate, versionsToSpecs) between the
  API and the diff page; malformed parts give 400 in the API and notFound()
  on the page.
- Registry not-found (E404/ETARGET) while resolving specs surfaces as
  SpecNotFoundError (resolved inside the cache boundary so the error code is
  not lost), mapped to 404 in the API and notFound() on the page.
- Build redirect and navigation query strings with URLSearchParams so values
  stay encoded and repeated keys are preserved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0125jPsCGgQhzzC88zAyk33B
@oBusk
oBusk force-pushed the claude/project-audit-findings-iezfst-input-validation branch from 95f87fb to e483928 Compare October 1, 2026 21:01
Number() already returns NaN for trailing garbage, and parseDiffOptions
enforces integer range, so the regex was redundant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
oBusk and others added 10 commits October 1, 2026 23:49
Number() accepts " ", "0x10", "1e2" and "0b11", which then pass the
integer range check, so diffUnified=%20 was silently treated as 0.

This reverts commit 9573b20.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
parseParts ran versionsToSpecs on every request, which throws for a
single spec like `x` or `1`, so those catalog pages 404ed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
libnpmdiff writes the prefixes verbatim into the ---/+++ header lines, so
a newline could inject fake header lines into the diff.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Assigning user-controlled keys into {} let a repeated __proto__ key
replace the query object's prototype.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
minimatch compiles globs to backtracking regexes, and libnpmdiff matches
every pattern against every file in both tarballs. Measured with the
installed minimatch, `*(?|?)Z` takes 375 ms per file and `*?` repeated
six times takes over 15 s against a 200 character file name.

- Normalize patterns the way libnpmdiff does before checking, so braces
  escaped with a backslash are counted.
- Reject extglobs, except the exact default glob the UI sends.
- Allow at most two `*` runs outside of `**` segments.
- Cap brace alternatives at 16 across all patterns instead of 64 per
  pattern.

Allowed patterns take at most ~0.16 ms per match against hostile paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ePrefix

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oBusk oBusk changed the title Validate diff input: cap glob patterns, clamp options, return 400/404 instead of 500 Validate diff input: restrict globs and options, return 400/404 instead of 500 Oct 1, 2026
@oBusk
oBusk merged commit f76a175 into main Oct 1, 2026
14 checks passed
@oBusk
oBusk deleted the claude/project-audit-findings-iezfst-input-validation branch October 1, 2026 22:17

This branch was successfully deployed

1 active deployment
Preview — 1af2ca2b Deployed Oct 1, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants