Repository navigation
Validate diff input: restrict globs and options, return 400/404 instead of 500 - #1071
Merged
oBusk merged 12 commits intoOct 1, 2026
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… 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
force-pushed
the
claude/project-audit-findings-iezfst-input-validation
branch
from
October 1, 2026 21:01
95f87fb to
e483928
Compare
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>
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
deleted the
claude/project-audit-findings-iezfst-input-validation
branch
October 1, 2026 22:17
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes audit findings H1, M3 (options), L2 and L3.
H1: CPU DoS via
diffFilesglobsminimatch compiles globs to backtracking regexes, and libnpmdiff matches every pattern against every file in both tarballs. Patterns like
*(?|?)Zor*?*?*?*?Ztake seconds per file.diffFilesis now limited to:\{a,b}can't sneak through**/!(*.map|*.min.js)is still allowed)*outside of**segmentsPart 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
diffUnifiedmust be an integer from 0 to 100.diffSrcPrefixanddiffDstPrefixcan be at most 64 characters, with no control characters.L2: invalid input returned 500
L3: query strings rebuilt without encoding
Redirects and links now build query strings with
URLSearchParams, so values stay encoded and repeateddiffFilesare kept.Behavior changes
diffFilesthat use extglobs, or more than two*, are rejected.noindex). The API returns a real 404.🤖 Generated with Claude Code