Skip to content

fix: correct HTTP status check logic in auth#3984

Open
srivilliamsai wants to merge 4 commits intokeploy:mainfrom
srivilliamsai:fix/auth-status-check
Open

fix: correct HTTP status check logic in auth#3984
srivilliamsai wants to merge 4 commits intokeploy:mainfrom
srivilliamsai:fix/auth-status-check

Conversation

@srivilliamsai
Copy link
Copy Markdown

Summary

Fixes incorrect HTTP status code validation in the authentication flow.

Problem

The condition res.StatusCode != 200 || res.StatusCode >= 300 has a logic error:

Status Code != 200 >= 300 Result (OR) Correct?
200 false false false ✓ Yes
201 true false true ✗ No - 201 is valid
404 true true true ✓ Yes

The OR operator causes any non-200 success code (like 201 Created) to be incorrectly rejected.

Solution

Changed to proper range check using http package constants:

// Before (buggy)
if res.StatusCode != 200 || res.StatusCode >= 300 {

// After (correct)  
if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusMultipleChoices {

The condition 'StatusCode != 200 || StatusCode >= 300' incorrectly
rejects valid 2xx success codes like 201 Created.

Changed to use http package constants and proper range check:
'StatusCode < http.StatusOK || StatusCode >= http.StatusMultipleChoices'

Signed-off-by: SRI VILLIAM SAI <srivilliamsai@gmail.com>
Copilot AI review requested due to automatic review settings March 27, 2026 11:51
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Mar 27, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This PR correctly fixes a logic bug in the HTTP status code validation. The original condition res.StatusCode != 200 || res.StatusCode >= 300 would incorrectly reject valid 2xx status codes (like 201, 202, 204) because the !=200 check would be true for them.

The new condition res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusMultipleChoices properly accepts any status code in the 200-299 range while rejecting codes below 200 or 300+.

Additional improvements:

  • Uses http.StatusOK and http.StatusMultipleChoices constants instead of magic numbers
  • Follows idiomatic Go patterns for HTTP status validation
Files Reviewed (1 file)
  • pkg/platform/auth/auth.go - 0 issues

Reviewed by claude-opus-4.5 · 67,015 tokens

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes HTTP status validation in the GitHub authentication flow so that any 2xx success response is accepted (instead of rejecting all non-200 success codes).

Changes:

  • Replace incorrect != 200 || >= 300 status check with a proper 2xx range check using net/http constants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

@charankamarapu charankamarapu left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants