fix(server): decode base64 Lambda Function URL request bodies before parsing (follow-up to #889) - #1483
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughLambda Function URL handling now decodes base64-encoded request bodies before downstream processing, rejects malformed input with HTTP 400, and preserves plain bodies. Tests cover encoded, plain, and invalid base64 form submissions. ChangesLambda body decoding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LambdaFunctionURL
participant handleLambdaHTTPEvent
participant Base64Decoder
participant DownstreamProcessing
LambdaFunctionURL->>handleLambdaHTTPEvent: request body and IsBase64Encoded
handleLambdaHTTPEvent->>Base64Decoder: decode flagged body
Base64Decoder-->>handleLambdaHTTPEvent: decoded body or decode error
handleLambdaHTTPEvent->>DownstreamProcessing: decoded request body
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
30a843a to
d23b270
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
…parsing AWS Lambda Function URLs base64-encode POST bodies whenever the Content-Type isn't recognized as plain text, which is the case for the one-click revoke confirmation form's application/x-www-form-urlencoded POST added in #889. handleLambdaHTTPEvent never checked request.IsBase64Encoded, so resolveApprovalToken received a base64 blob instead of "token=...", and every revoke via the email link 401'd in Lambda mode (the primary deploy mode). The HTTP/Fargate adapter was unaffected since it builds its own request with IsBase64Encoded always false. Decode the body once, centrally, right after the raw event is unmarshaled and before anything downstream (OIDC, static files, the API router) reads it. A decode failure returns the same 400 response already used for a malformed event instead of silently proceeding with garbage.
d23b270 to
278a5e7
Compare
Summary
application/x-www-form-urlencodedPOST.handleLambdaHTTPEventnever checkedrequest.IsBase64Encoded, soresolveApprovalTokenreceived a base64 blob instead oftoken=...and every revoke via the email link 401'd in Lambda Function URL mode -- the primary deploy mode for this app. The HTTP/Fargate adapter was unaffected since it builds its own request withIsBase64Encodedalwaysfalse.internal/server/lambda.go'shandleLambdaHTTPEvent, right after the raw event is unmarshaled and before anything downstream (OIDC, static files, the API router) readsrequest.Body. A decode failure returns the same 400 response already used for a malformed event, instead of silently proceeding with garbage.internal/api/does its own base64 decode of the inbound body (confirmed viagit grep -n IsBase64Encodedand a scan of every.Bodyusage), so centralizing the decode here fixes this class of bug for any other Content-Type-sensitive body parsing on the Lambda path too, without any double-decode risk.Test plan
TestHandleLambdaHTTPEvent_DecodesBase64FormBodyininternal/server/lambda_coverage_test.go, driving the real failing scenario: a base64-encoded, form-urlencoded POST body hittingPOST /api/purchases/revoke/{execID}. Confirmed it FAILS against the pre-fix code (401 "sign in or use the revocation link...") and PASSES after the fix (401 "sign in with the account's contact email...", proving the token was correctly resolved from the decoded body).go build ./...-- exit 0go vet ./...-- exit 0go test ./...-- 5898 passed in 38 packages, exit 0golangci-lint runat the CI-pinned v2.10.1 (both./internal/server/... ./internal/api/...and the full./...) -- 0 issues, exit 0gocyclo -over 10 -ignore "_test\.go"on the touched packages -- exit 0, no findingsSummary by CodeRabbit
Bug Fixes
Tests