🔒 Add timeout to unbounded fetch request in npm-registry#87
Conversation
Co-authored-by: mbayue <70324722+mbayue@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant Client as Client (Browser/User)
participant App as Node.js Application
participant Handler as Package Handler
participant NpmService as NpmRegistry Service
participant NpmRegistry as External NPM Registry
Note over Client, NpmRegistry: Package lookup request flow
Client->>App: HTTP GET /package/[name]
App->>Handler: route request
Handler->>NpmService: fetchNpmRegistryPackage(packageName)
Note over NpmService: 5-second timeout enforced
alt Request completes within 5s
NpmService->>NpmRegistry: fetch(url, { signal: AbortSignal.timeout(5000) })
NpmRegistry-->>NpmService: HTTP 200 + package data
NpmService-->>Handler: NpmRegistryPackage object
Handler-->>App: Process and return response
App-->>Client: HTTP 200 + package info
else Timeout exceeded (no response in 5s)
NpmService->>NpmService: AbortSignal.timeout fires
NpmService->>NpmRegistry: fetch aborted (connection released)
Note over NpmService: catch block handles abort error
NpmService-->>Handler: typedError('network-error', ...)
Handler-->>App: Handle network error
App-->>Client: HTTP 503 (Service Unavailable) or error message
else Network error (connection refused, DNS failure, etc.)
NpmService->>NpmRegistry: fetch(url, { signal: AbortSignal.timeout(5000) })
Note over NpmService: Connection fails before timeout
NpmService-->>Handler: typedError('network-error', ...)
Handler-->>App: Handle network error
App-->>Client: Error response
end
Note over NpmService: Resources freed immediately in all paths
Co-authored-by: mbayue <70324722+mbayue@users.noreply.github.com>
🎯 What: The vulnerability fixed is an unbounded network request in
⚠️ Risk: If left unfixed, a slow or unresponsive external server could cause these requests to hang indefinitely. This unbounded waiting can exhaust application resources (such as memory, sockets, or thread pool limits) over time, ultimately leading to a Denial of Service (DoS) where the application becomes unresponsive to legitimate users.
server/services/npm-registry.tswhich lacked a timeout duringfetchcalls to the NPM registry.🛡️ Solution: The fix addresses the vulnerability by passing an
AbortSignal.timeout(5000)to thefetchoptions. This ensures that the network request will forcibly abort and throw after 5 seconds if a response hasn't been received, which is then safely caught by the existingtry...catchblock and returned as a standard'network-error', freeing up system resources.PR created automatically by Jules for task 4792009729057608153 started by @mbayue
Summary by cubic
Add a 5s timeout to NPM registry requests to stop hanging
fetchcalls and prevent resource exhaustion/DoS. Timed-out requests abort and are handled as'network-error'like other network failures.AbortSignal.timeout(5000)infetchwithinserver/services/npm-registry.ts..jules/sentinel.md.Written for commit a5ccc12. Summary will update on new commits.