Merge pull request #628 from Molecule-AI/fix/issue-623-adminauth-origin-bypass

Merge gate passed (all 7 gates). Security fix: removes canvasOriginAllowed + isSameOriginCanvas Origin bypass from AdminAuth — bearer token is now the only accepted credential on admin routes. 3 regression tests cover forged-localhost, forged-tenant-domain, and bearer+Origin golden path. Auth PR — CEO explicit approval confirmed in chat. UNSTABLE = known GitHub App token scope gap.

platform/internal/middleware/wsauth_middleware.go

@@ -67,10 +67,17 @@ func WorkspaceAuth(database *sql.DB) gin.HandlerFunc {
 // Same lazy-bootstrap contract as WorkspaceAuth: if no live token exists
 // anywhere on the platform (fresh install / pre-Phase-30 upgrade), requests
 // are let through so existing deployments keep working. Once any workspace
-// has a live token every request to these routes MUST present a valid one.
+// has a live token every request to these routes MUST present a valid bearer
+// token — no Origin-based bypass. (#623)
 //
 // Any valid workspace bearer token is accepted — the route is not scoped to
 // a specific workspace so we only verify the token is live and unrevoked.
+//
+// NOTE: canvasOriginAllowed / isSameOriginCanvas are intentionally NOT called
+// here.  The Origin header is trivially forgeable by any container on the
+// Docker network; using it as an auth bypass would let an attacker reach
+// /settings/secrets, /bundles/import, /events, etc. without a bearer token.
+// Those short-circuits belong ONLY in CanvasOrBearer (cosmetic routes).
 func AdminAuth(database *sql.DB) gin.HandlerFunc {
 	return func(c *gin.Context) {
 		ctx := c.Request.Context()
@@ -82,7 +89,7 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
 			return
 		}
 		if hasLive {
-			// Bearer token path — agents, CLI, and API clients.
+			// Bearer token is the ONLY accepted credential for admin routes.
 			tok := wsauth.BearerTokenFromHeader(c.GetHeader("Authorization"))
 			if tok != "" {
 				if err := wsauth.ValidateAnyToken(ctx, database, tok); err != nil {
@@ -92,16 +99,6 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
 				c.Next()
 				return
 			}
-			// Canvas origin path — cross-origin canvas (CORS_ORIGINS match).
-			if canvasOriginAllowed(c.GetHeader("Origin")) {
-				c.Next()
-				return
-			}
-			// Same-origin canvas path — tenant image where canvas + API share a host.
-			if isSameOriginCanvas(c) {
-				c.Next()
-				return
-			}
 			c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "admin auth required"})
 			return
 		}

platform/internal/middleware/wsauth_middleware_test.go

@@ -778,3 +778,116 @@ func TestCanvasOriginAllowed_LocalhostDefault(t *testing.T) {
 		t.Error("random origin should not be allowed")
 	}
 }
+
+// ── Issue #623 regression ─────────────────────────────────────────────────────
+// AdminAuth must NOT accept forged Origin headers. Any container on the Docker
+// network can set Origin: http://localhost:3000 without a bearer token, which
+// previously bypassed AdminAuth on ALL admin-gated routes. (#623, dup #626)
+
+// TestAdminAuth_623_ForgedOrigin_Returns401 — the main regression test:
+// a request with a matching CORS origin but no bearer token must be rejected.
+func TestAdminAuth_623_ForgedOrigin_Returns401(t *testing.T) {
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer mockDB.Close()
+
+	// Platform has live tokens — AdminAuth is active.
+	mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
+
+	t.Setenv("CORS_ORIGINS", "http://localhost:3000")
+
+	r := gin.New()
+	r.GET("/settings/secrets", AdminAuth(mockDB), func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"secrets": []string{"OPENAI_API_KEY"}})
+	})
+
+	w := httptest.NewRecorder()
+	// #623 attack: forge the canvas Origin header — no bearer token.
+	req, _ := http.NewRequest(http.MethodGet, "/settings/secrets", nil)
+	req.Header.Set("Origin", "http://localhost:3000")
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("#623 forged Origin bypass: expected 401, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// TestAdminAuth_623_ForgedCORSOrigin_Returns401 — variant: attacker uses the
+// tenant-domain CORS origin from CORS_ORIGINS (not just localhost).
+func TestAdminAuth_623_ForgedCORSOrigin_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")
+
+	r := gin.New()
+	r.GET("/admin/secrets", AdminAuth(mockDB), func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"ok": true})
+	})
+
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest(http.MethodGet, "/admin/secrets", nil)
+	req.Header.Set("Origin", "https://acme.moleculesai.app")
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("#623 forged tenant Origin: expected 401, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// TestAdminAuth_623_ValidBearer_WithOrigin_Passes — bearer + matching Origin
+// should still work (the Origin is irrelevant once the bearer validates).
+func TestAdminAuth_623_ValidBearer_WithOrigin_Passes(t *testing.T) {
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock: %v", err)
+	}
+	defer mockDB.Close()
+
+	goodToken := "valid-bearer-token-xyz"
+	tokenHash := sha256.Sum256([]byte(goodToken))
+
+	mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
+	mock.ExpectQuery(validateAnyTokenSelectQuery).
+		WithArgs(tokenHash[:]).
+		WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("tok-1"))
+	mock.ExpectExec(validateTokenUpdateQuery).
+		WithArgs("tok-1").
+		WillReturnResult(sqlmock.NewResult(0, 1))
+
+	t.Setenv("CORS_ORIGINS", "http://localhost:3000")
+
+	r := gin.New()
+	r.GET("/settings/secrets", AdminAuth(mockDB), func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"ok": true})
+	})
+
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest(http.MethodGet, "/settings/secrets", nil)
+	req.Header.Set("Authorization", "Bearer "+goodToken)
+	req.Header.Set("Origin", "http://localhost:3000") // present but irrelevant
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("bearer+origin: expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}