🛡️ Sentinel: [MEDIUM] Fix SSRF risk in push notification endpoints#1169
Conversation
Enforces HTTPS for web push notification endpoints to prevent potential Server-Side Request Forgery against internal services. Co-authored-by: rdale-dev <203160809+rdale-dev@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 |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ddb437138
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const subscribeSchema = z.object({ | ||
| endpoint: z.url(), | ||
| // 🛡️ Sentinel: Enforce HTTPS to prevent SSRF against internal/local services | ||
| endpoint: z.string().url().refine((url) => url.startsWith("https://"), { |
There was a problem hiding this comment.
The new startsWith("https://") refinement rejects valid HTTPS URLs whose scheme is uppercase or mixed case (for example HTTPS://...), even though z.string().url() accepts them and URL schemes are case-insensitive. That creates a regression where otherwise valid subscription/unsubscription payloads can fail validation with 400 responses depending on client normalization. Parse the URL and check new URL(url).protocol === "https:" (or use a case-insensitive protocol constraint) instead of a case-sensitive prefix check.
Useful? React with 👍 / 👎.
Default Project
 |
||||||||||||||||||||||||||||||
| Project |
Default Project
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 02m 23s |
| Commit |
|
| Committer | daler91 |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
70
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
1 participant
🚨 Severity: MEDIUM
💡 Vulnerability: The web push subscription endpoint
/api/v1/push/subscribepreviously accepted any valid URL for the pushendpointfield, including internalhttp://URLs.🎯 Impact: When dealing with external callback URLs, failing to strictly enforce HTTPS allows an attacker to supply an internal IP or local domain (e.g.,
http://localhostorhttp://169.254.169.254). The server's web push library could then unknowingly make a POST request to this internal address, resulting in a blind Server-Side Request Forgery (SSRF) attack.🔧 Fix: Updated the Zod schemas in
server/routes/push.tsto explicitly enforcehttps://URLs using.refine.✅ Verification: Ran the full test suite (
pnpm test), type check (pnpm check), and linting (pnpm lint) to ensure no regressions. The change purely restricts input validation safely, as valid Web Push subscriptions use secure HTTPS endpoints (fcm.googleapis.com,updates.push.services.mozilla.com, etc.).PR created automatically by Jules for task 10700333223020819974 started by @rdale-dev