Merge pull request #979 from Molecule-AI/fix/security-adminauth-c4

fix(security): C4 — close AdminAuth fail-open race on hosted-SaaS fresh install
This commit is contained in:
Hongming Wang 2026-04-19 01:29:54 -07:00 committed by GitHub
commit a5d6e5319f
2 changed files with 25 additions and 10 deletions

View File

@ -93,6 +93,7 @@ func WorkspaceAuth(database *sql.DB) gin.HandlerFunc {
func AdminAuth(database *sql.DB) gin.HandlerFunc {
return func(c *gin.Context) {
ctx := c.Request.Context()
adminSecret := os.Getenv("ADMIN_TOKEN")
hasLive, err := wsauth.HasAnyLiveTokenGlobal(ctx, database)
if err != nil {
@ -101,9 +102,17 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
return
}
if !hasLive {
// Tier 1: fail-open on fresh install / pre-Phase-30 upgrade.
c.Next()
return
// Tier 1: fail-open is ONLY safe when ADMIN_TOKEN is unset
// (self-hosted dev, pre-Phase-30 upgrade). Hosted SaaS always
// sets ADMIN_TOKEN at provision time, and C4 (SaaS-launch
// blocker) showed that without this guard an attacker can
// pre-empt the first user by POSTing /org/import before any
// token gets minted. When ADMIN_TOKEN is set we fall through
// into the same bearer-check path Tier-2 uses below.
if adminSecret == "" {
c.Next()
return
}
}
// Bearer token is the ONLY accepted credential for admin routes.
@ -115,7 +124,7 @@ func AdminAuth(database *sql.DB) gin.HandlerFunc {
// Tier 2 (#684 fix): dedicated ADMIN_TOKEN — workspace bearer tokens
// must not grant access to admin routes.
if adminSecret := os.Getenv("ADMIN_TOKEN"); adminSecret != "" {
if adminSecret != "" {
if subtle.ConstantTimeCompare([]byte(tok), []byte(adminSecret)) != 1 {
c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "invalid admin auth token"})
return

View File

@ -1137,9 +1137,14 @@ func TestAdminAuth_684_AdminTokenNotSet_FallsBackToWorkspaceToken(t *testing.T)
}
// TestAdminAuth_684_FailOpen_AdminTokenSet_NoGlobalTokens — even when
// ADMIN_TOKEN is set, a fresh install (no tokens globally) must still
// fail-open (tier-1 contract unchanged).
func TestAdminAuth_684_FailOpen_AdminTokenSet_NoGlobalTokens(t *testing.T) {
// Regression for SaaS-launch blocker C4: when ADMIN_TOKEN is set, a
// fresh install (zero live workspace tokens) MUST fail closed. Hosted
// SaaS tenants boot with ADMIN_TOKEN set but an empty tokens table —
// without this guard, an anonymous caller can POST /org/import or
// /workspaces before the first real user and pre-empt the instance.
// Fail-open is only acceptable when ADMIN_TOKEN is also unset
// (self-hosted dev with zero auth configured).
func TestAdminAuth_C4_AdminTokenSet_FreshInstall_FailsClosed(t *testing.T) {
mockDB, mock, err := sqlmock.New()
if err != nil {
t.Fatalf("sqlmock.New: %v", err)
@ -1159,11 +1164,12 @@ func TestAdminAuth_684_FailOpen_AdminTokenSet_NoGlobalTokens(t *testing.T) {
w := httptest.NewRecorder()
req, _ := http.NewRequest(http.MethodGet, "/admin/secrets", nil)
// No bearer — but fail-open should still pass.
// No bearer — ADMIN_TOKEN is set so the no-tokens tier-1 escape
// no longer applies; the request must be rejected.
r.ServeHTTP(w, req)
if w.Code != http.StatusOK {
t.Errorf("#684 fail-open w/ ADMIN_TOKEN set (no global tokens): expected 200, got %d: %s",
if w.Code != http.StatusUnauthorized {
t.Errorf("C4 fresh-install w/ ADMIN_TOKEN set: expected 401, got %d: %s",
w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {