Merge pull request #185 from Molecule-AI/fix/issue-180-approvals-auth

fix(security): gate GET /approvals/pending behind AdminAuth (#180)

platform/internal/middleware/wsauth_middleware_test.go

@@ -486,6 +486,82 @@ func TestAdminAuth_InvalidBearer_Returns401(t *testing.T) {
 // requests — the route-level fix in router.go is the enforcement point.
 // ────────────────────────────────────────────────────────────────────────────
 
+// ────────────────────────────────────────────────────────────────────────────
+// Issue #180 regression — unauthenticated GET /approvals/pending
+//
+// GET /approvals/pending was registered on the open router (no middleware)
+// and returned all pending approvals across every workspace to any caller,
+// with no token required.
+// Attack vector confirmed by Security Auditor:
+//   curl http://host/approvals/pending → 200 with full cross-workspace list
+//
+// Fixed by adding inline AdminAuth to the route registration in router.go.
+// This test asserts the gate blocks unauthenticated reads.
+// ────────────────────────────────────────────────────────────────────────────
+
+// TestAdminAuth_Issue180_ApprovalsListing_NoBearer_Returns401 documents the #180
+// attack vector and verifies that AdminAuth returns 401 for GET without a token.
+func TestAdminAuth_Issue180_ApprovalsListing_NoBearer_Returns401(t *testing.T) {
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock.New: %v", err)
+	}
+	defer mockDB.Close()
+
+	// HasAnyLiveTokenGlobal returns 1 — at least one workspace is token-enrolled.
+	mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
+
+	r := gin.New()
+	// Mirror the router.go fix: GET /approvals/pending is behind AdminAuth.
+	r.GET("/approvals/pending", AdminAuth(mockDB), func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"approvals": []interface{}{}})
+	})
+
+	w := httptest.NewRecorder()
+	// #180 attack: no Authorization header on GET /approvals/pending.
+	req, _ := http.NewRequest(http.MethodGet, "/approvals/pending", nil)
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("#180 GET /approvals/pending no-bearer: expected 401, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// TestAdminAuth_Issue180_ApprovalsListing_FailOpen_NoTokens documents the
+// fail-open contract: on a fresh install (no tokens anywhere), the middleware
+// must not block the canvas from polling /approvals/pending.
+func TestAdminAuth_Issue180_ApprovalsListing_FailOpen_NoTokens(t *testing.T) {
+	mockDB, mock, err := sqlmock.New()
+	if err != nil {
+		t.Fatalf("sqlmock.New: %v", err)
+	}
+	defer mockDB.Close()
+
+	// HasAnyLiveTokenGlobal returns 0 — fresh install, no tokens yet.
+	mock.ExpectQuery(hasAnyLiveTokenGlobalQuery).
+		WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
+
+	r := gin.New()
+	r.GET("/approvals/pending", AdminAuth(mockDB), func(c *gin.Context) {
+		c.JSON(http.StatusOK, gin.H{"approvals": []interface{}{}})
+	})
+
+	w := httptest.NewRecorder()
+	req, _ := http.NewRequest(http.MethodGet, "/approvals/pending", nil)
+	r.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("#180 fail-open (no tokens): expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
 // TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401 documents the #120
 // attack vector and verifies that AdminAuth returns 401 for PATCH without a token.
 func TestAdminAuth_Issue120_PatchWorkspace_NoBearer_Returns401(t *testing.T) {

platform/internal/router/router.go

@@ -166,8 +166,11 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
 		wsAuth.POST("/approvals", apph.Create)
 		wsAuth.GET("/approvals", apph.List)
 		wsAuth.POST("/approvals/:approvalId/decide", apph.Decide)
-		// /approvals/pending is a cross-workspace admin path; keep on root router outside wsAuth.
-		r.GET("/approvals/pending", apph.ListAll)
+		// /approvals/pending is a cross-workspace admin path; WorkspaceAuth cannot
+		// be used here (no workspace scope), but it still needs auth so an
+		// unauthenticated caller cannot enumerate all pending approvals across the
+		// entire platform. Gated behind AdminAuth (issue #180).
+		r.GET("/approvals/pending", middleware.AdminAuth(db.DB), apph.ListAll)
 
 		// Team Expansion
 		teamh := handlers.NewTeamHandler(broadcaster, prov, platformURL, configsDir)