forked from molecule-ai/molecule-core
fix(middleware): add missing return after AbortWithStatusJSON in CanvasOrBearer
P0 security: CanvasOrBearer final else branch aborts with 401 but continues execution to c.Next() — allowing the downstream handler to overwrite the 401 response. Regression tests added to verify the handler is not called after AbortWithStatusJSON in both no-cred and wrong-origin paths. Confirmed on origin/main @69408ab6and origin/staging @6b62391e. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
a59f1a6ce4
commit
4034f0dc55
@ -304,6 +304,7 @@ func CanvasOrBearer(database *sql.DB) gin.HandlerFunc {
|
||||
}
|
||||
|
||||
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "admin auth required"})
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -1011,8 +1011,10 @@ func TestCanvasOrBearer_TokensExist_NoCreds_Returns401(t *testing.T) {
|
||||
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
|
||||
|
||||
handlerCalled := false
|
||||
r := gin.New()
|
||||
r.PUT("/canvas/viewport", CanvasOrBearer(mockDB), func(c *gin.Context) {
|
||||
handlerCalled = true
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
})
|
||||
|
||||
@ -1023,6 +1025,47 @@ func TestCanvasOrBearer_TokensExist_NoCreds_Returns401(t *testing.T) {
|
||||
if w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("no creds: got %d, want 401", w.Code)
|
||||
}
|
||||
if handlerCalled {
|
||||
t.Error("handler was called after AbortWithStatusJSON — missing return allows fall-through")
|
||||
}
|
||||
if body := w.Body.String(); body == `{"ok":true}` {
|
||||
t.Error("handler body written after AbortWithStatusJSON")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOrBearer_TokensExist_WrongOrigin_Returns401(t *testing.T) {
|
||||
mockDB, mock, err := sqlmock.New()
|
||||
if err != nil {
|
||||
t.Fatalf("sqlmock: %v", err)
|
||||
}
|
||||
defer mockDB.Close()
|
||||
|
||||
mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
|
||||
|
||||
t.Setenv("CORS_ORIGINS", "https://acme.moleculesai.app")
|
||||
|
||||
handlerCalled := false
|
||||
r := gin.New()
|
||||
r.PUT("/canvas/viewport", CanvasOrBearer(mockDB), func(c *gin.Context) {
|
||||
handlerCalled = true
|
||||
c.JSON(http.StatusOK, gin.H{"ok": true})
|
||||
})
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest(http.MethodPut, "/canvas/viewport", nil)
|
||||
req.Header.Set("Origin", "https://evil.example.com")
|
||||
r.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("wrong origin: got %d, want 401", w.Code)
|
||||
}
|
||||
if handlerCalled {
|
||||
t.Error("handler was called after AbortWithStatusJSON — missing return allows fall-through")
|
||||
}
|
||||
if body := w.Body.String(); body == `{"ok":true}` {
|
||||
t.Error("handler body written after AbortWithStatusJSON")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanvasOrBearer_TokensExist_CanvasOrigin_Passes(t *testing.T) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user