Skip to content

fix(redirect): stop /redirect from following arbitrary destinations - #11

Merged
Kvrnn merged 1 commit into
mainfrom
fix/redirect-open-redirect
Sep 28, 2026
Merged

Kvrnn merged 1 commit into
mainfrom
fix/redirect-open-redirect

Conversation

@Kvrnn

@Kvrnn Kvrnn commented Sep 28, 2026

Copy link
Copy Markdown
Member

Problem

src/pages/redirect.astro took its destination from the dest query parameter and navigated there. On every site built from this template:

  • Open redirect: a link on the site's own domain could send visitors to any external site.
  • Reflected XSS: a javascript: value for dest passed through new URL() unchanged and ran on the site's origin, through both window.location.href and the "click here" link.

Fix

  • New resolveRedirect() in src/data/promoRedirects.ts. It checks promo redirects (campaigns/coupons) first, then static ones, the same order the middleware already used.
  • The interstitial reads only source and gets destination, type, and category from resolveRedirect(). Resolved destinations must start with http(s):// or be site-relative (/…, not //…), so a javascript: ctaUrl in coupon content is rejected too. Unknown or expired sources go to /.
  • The middleware uses the same resolveRedirect() and no longer passes dest, type, or category. UTM and user query params are still forwarded and merged into the destination.
  • CLAUDE.md redirect flow updated.

Testing

Built and ran with wrangler dev --local:

Request Before After
/redirect/?source=x&dest=javascript:… href="javascript:…" "Invalid redirect" → /
/redirect/?source=/go&dest=https://evil.example evil.example /contact-us/
/redirect/?source=/go&utm_term=abc works /contact-us/?utm_term=abc
/redirect/?source=/wellness20&utm_term=abc (coupon, expiry temporarily extended) works /contact-us/?coupon=WELLNESS20&utm_term=abc
Coupon with ctaUrl: "javascript:…" (temporary fixture) would execute "Invalid redirect"
Expired campaign /ny2025 n/a "Invalid redirect"

astro check: 0 errors. npm run check (ESLint/Prettier) already fails on main (66 ESLint errors, 57 unformatted files), and this PR adds no new failures.

Not addressed here (existing issue)

With output: 'static', the short links themselves (/go, /wellness20) return 404 on main, because the middleware only runs for on-demand routes. This PR doesn't change that. nightsquawk-tech works around it with output: 'server'.

Rollout

Every client repo built from this template has the vulnerable interstitial until it merges upstream/main and redeploys.

🤖 Generated with Claude Code

The /redirect interstitial read its destination from the `dest` query
parameter and navigated to it, so any link on a template site could send
visitors anywhere (open redirect), and `dest=javascript:...` ran script
on the site's origin (reflected XSS).

The interstitial now reads only `source` and resolves the destination,
type, and category from the redirect config via a shared
resolveRedirect() (promo redirects first, then static, the same order the
middleware uses). Configured destinations must still be http(s) or
site-relative, so a `javascript:` ctaUrl in coupon content is rejected
too. Unknown sources go to `/`. The middleware no longer passes
dest/type/category.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-28T03:04:51.193229Z 6dde452 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Risk: high. This change touches redirect middleware and the interstitial used for navigation, which is security-sensitive (open redirect / XSS surface), so it is above the medium approval threshold. Cursor Bugbot was not present after the first check poll; no reviewers were assigned because the repository has no other eligible reviewers besides the author. Human review is needed before merge.

Open in Web View Automation 

Sent by Cursor Approval Agent: NST: PR Approver

@Kvrnn
Kvrnn merged commit 0afab1a into main Sep 28, 2026
8 of 9 checks passed
@Kvrnn
Kvrnn deleted the fix/redirect-open-redirect branch September 28, 2026 03:08
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.

1 participant