fix(vnda): filter proxy cookies to prevent RequestHeaderSectionTooLarge#1633
Conversation
Add allowedCookies option to proxy handler and configure VNDA proxy to only forward VNDA-relevant cookies (vnda_cart_id, cart_id, ahoy_visit, ahoy_visitor), stripping analytics/tracking cookies that inflate headers beyond the 8KB limit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughAdds a ChangesProxy Cookie Filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ProxyHandler
participant filterCookies
participant Target
Client->>ProxyHandler: Request with cookie header
ProxyHandler->>ProxyHandler: Remove CF headers
alt allowedCookies provided and non-empty
ProxyHandler->>filterCookies: filterCookies(headers, allowedCookies)
filterCookies->>filterCookies: Filter pairs by allowed names
filterCookies-->>ProxyHandler: Updated or deleted cookie header
end
ProxyHandler->>Target: Forward proxied request
Related PRs: None identified. Suggested labels: enhancement, proxy, security Suggested reviewers: None identified. 🐰 A cookie jar with just a few, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/handlers/proxy.ts (1)
152-173: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCookie filter can be bypassed by
customHeaders.
filterCookiesruns before thecustomHeadersloop (Lines 162-173), which unconditionally appends/sets thecookieheader fromcustomHeadersentries without re-applying the allowlist. Today VNDA's config doesn't pass acookiecustom header, so there's no live bypass, but the invariant "only allowlisted cookies reach upstream" isn't actually enforced end-to-end — a future config that injects a cookie viacustomHeaderswould silently reintroduce unfiltered cookies. Consider re-applying (or moving) theallowedCookiesfilter after thecustomHeadersmerge so the guarantee holds regardless of config.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/handlers/proxy.ts` around lines 152 - 173, The cookie allowlist in proxy handling can be bypassed by the customHeaders merge in proxy.ts. Update the request-building flow in the proxy handler so any cookie introduced through customHeaders is also passed through the same allowedCookies enforcement, either by filtering again after the customHeaders loop or by moving the filter to run after all cookie sources are merged; keep the logic centered around filterCookies, customHeaders, and the headers cookie handling in the proxy function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@website/handlers/proxy.ts`:
- Around line 152-173: The cookie allowlist in proxy handling can be bypassed by
the customHeaders merge in proxy.ts. Update the request-building flow in the
proxy handler so any cookie introduced through customHeaders is also passed
through the same allowedCookies enforcement, either by filtering again after the
customHeaders loop or by moving the filter to run after all cookie sources are
merged; keep the logic centered around filterCookies, customHeaders, and the
headers cookie handling in the proxy function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8fd3446-d3a5-4502-b6fe-9e29ddb9853e
📒 Files selected for processing (2)
vnda/loaders/proxy.tswebsite/handlers/proxy.ts
Summary
allowedCookiesoption to the generic proxy handler (website/handlers/proxy.ts) that filters cookies to an allowlist before forwardingvnda_cart_id,cart_id,ahoy_visit,ahoy_visitor_ga,_fbp,_gcl_au,_hj*,_pin_unauth, etc.) and Cloudflare cookies (__cf_bm,_cfuvid,cf_clearance) that were inflating request headers beyond the 8KB limit, causingRequestHeaderSectionTooLargeerrors on checkoutTest plan
RequestHeaderSectionTooLargeerrors on/checkout/*routes/entrar,/conta) still work🤖 Generated with Claude Code
Summary by cubic
Filters cookies in the proxy to prevent
RequestHeaderSectionTooLargeerrors on VNDA checkout. Adds anallowedCookiesoption and enables it for VNDA to forward only required cookies.allowedCookiesprop inwebsite/handlers/proxy.tsto whitelist cookie names.vnda_cart_id,cart_id,ahoy_visit,ahoy_visitor./checkout/*.Written for commit 517d85b. Summary will update on new commits.
Summary by CodeRabbit