Skip to content

fix(auth,env): add missing return statements after error responses#3792

Open
cursor[bot] wants to merge 1 commit intodevelopfrom
cursor/critical-bug-inspection-b349
Open

fix(auth,env): add missing return statements after error responses#3792
cursor[bot] wants to merge 1 commit intodevelopfrom
cursor/critical-bug-inspection-b349

Conversation

@cursor
Copy link
Copy Markdown

@cursor cursor Bot commented Apr 18, 2026

Bug and Impact

Two critical missing-return bugs that allow control flow to fall through after writing error HTTP responses:

1. Authentication bypass in api/login.go (lines 349-363)

Root cause: When loginByPassword or loginByLDAP returns a non-ErrNotFound error (e.g., DB connection error, validation error), the handler writes HTTP 500 but does NOT return. Execution falls through to createSession(), which creates a valid authenticated session for a zero-value db.User{} struct.

Trigger scenario:

  1. A transient database error occurs during loginByPassword (e.g., connection timeout)
  2. The error is not ErrNotFound, so the 401 early return is skipped
  3. The handler writes 500 to the response but continues executing
  4. createSession() is called with a zero-value user (ID=0), creating a session cookie
  5. The attacker receives both a 500 status and a valid session cookie

Impact: Session created for user ID 0 (or stale user data from a partial lookup), potentially allowing unauthorized access.

2. Authorization bypass in api/projects/environment.go AddEnvironment (lines 253-257)

Root cause: When project.ID != env.ProjectID, the handler writes HTTP 400 but does NOT return. Execution falls through to CreateEnvironment(), which creates the environment using the request body's ProjectID.

Trigger scenario:

  1. User has access to project A (ID=1) but not project B (ID=2)
  2. User sends POST /api/project/1/environment with body {"project_id": 2, ...}
  3. Middleware validates access for project 1 (from URL) — passes
  4. Handler detects project.ID (1) != env.ProjectID (2) and writes 400
  5. But execution continues, creating the environment in project 2
  6. The client receives the 400 error body followed by the 201 created response

Impact: Users can create environments in projects they don't have access to.

Fix

Added the missing return statements after both error responses. Both are single-line, minimal fixes.

Validation

  • go vet ./api/projects/... passes
  • go test ./api/projects/... passes (7 existing tests)
  • go test ./services/server/... passes
Open in Web View Automation 

Two critical missing-return bugs found in recent code:

1. api/login.go: When login encounters a non-ErrNotFound error (e.g., DB
   error, validation error from LDAP), the handler writes HTTP 500 but
   does NOT return. Execution falls through to createSession(), which
   creates a valid session for a zero-value or stale user struct. This
   could allow authentication bypass after transient backend failures.

2. api/projects/environment.go AddEnvironment: When project.ID != env.ProjectID,
   the handler writes HTTP 400 but does NOT return. Execution falls through
   to CreateEnvironment(), which creates an environment using the body's
   project ID. Since middleware only checks access for the URL's project,
   this is an authorization bypass allowing users to create environments
   in projects they may not have access to.

Both fixes add the missing 'return' statement after the error response.

Co-authored-by: Denis Gukov <fiftin@outlook.com>
@fiftin fiftin marked this pull request as ready for review April 19, 2026 18:32
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