fix(auth): hand off privileges before state creation; fail closed on store errors

Fresh boots opened the owner/token/session stores before EnsureDataRoot
dropped to the host user, leaving a root-owned maestro/ state dir whose
EACCES surfaced as a bogus ERR_INVALID_CREDENTIALS on /api/auth/setup.
main() now runs the handoff first; EnsureDataRoot heals pre-existing
root-owned state dirs while still root (healOwnerState).

Storage failures from the owner store now answer 500 ERR_INTERNAL
everywhere (status/setup/login/change-password/RequireSession/
GateWebSocket) instead of blaming credentials or re-advertising setup
mode; SetupRequired returns the store error instead of swallowing it,
and FileOwnerStore.Verify propagates read errors rather than masking
them as unknown-user.

spec.yaml

@@ -2,7 +2,7 @@ openapi: 3.0.3
 info:
   title: Dcntrlized Maestro API
   version: 1.0.0
-  description: The API of the Maestro orchestration router. Maestro manages the lifecycle of Dcntrlized service containers and routes inter-service action calls to them. Every /api/* route except /api/auth/* and the deliberately public GET /api/instance (hub-app theme) requires an instance-owner session (cookies issued by the auth endpoints); in setup mode gated routes answer 428 ERR_SETUP_REQUIRED, otherwise 401 ERR_UNAUTHENTICATED. Mutating requests from browser origins are additionally checked against the request host (CSRF).
+  description: The API of the Maestro orchestration router. Maestro manages the lifecycle of Dcntrlized service containers and routes inter-service action calls to them. Every /api/* route except /api/auth/* and the deliberately public GET /api/instance (hub-app theme) requires an instance-owner session (cookies issued by the auth endpoints); in setup mode gated routes answer 428 ERR_SETUP_REQUIRED, otherwise 401 ERR_UNAUTHENTICATED (an unreadable owner store fails closed with 500 ERR_INTERNAL). Mutating requests from browser origins are additionally checked against the request host (CSRF).
 
 components:
   schemas:
@@ -616,7 +616,8 @@ paths:
         The SPA bootstraps from this endpoint. All other /api/* routes are
         gated by the instance-owner session (onboarding theme M1): in setup
         mode they answer 428 ERR_SETUP_REQUIRED, otherwise 401
-        ERR_UNAUTHENTICATED without a valid session.
+        ERR_UNAUTHENTICATED without a valid session; an unreadable owner store
+        fails closed with 500 ERR_INTERNAL everywhere.
       responses:
         '200':
           description: Gate state
@@ -650,6 +651,8 @@ paths:
           description: Setup already completed (ERR_SETUP_COMPLETED)
         '429':
           description: Rate limited (ERR_RATE_LIMITED)
+        '500':
+          description: Owner store unreadable/unwritable (ERR_INTERNAL)
 
   /api/auth/login:
     post:
@@ -674,6 +677,8 @@ paths:
           description: Unknown user or wrong password (ERR_INVALID_CREDENTIALS)
         '429':
           description: Rate limited (ERR_RATE_LIMITED) with Retry-After
+        '500':
+          description: Owner store unreadable (ERR_INTERNAL)
 
   /api/auth/refresh:
     post:
@@ -721,6 +726,8 @@ paths:
           description: Not authenticated
         '429':
           description: Rate limited (ERR_RATE_LIMITED)
+        '500':
+          description: Owner store unreadable/unwritable (ERR_INTERNAL)
 
   /api/ws:
     get:

src/handlers/auth/auth.go

@@ -22,6 +22,7 @@ package auth
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/url"
 	"os"
@@ -99,14 +100,24 @@ type passwordChange struct {
 }
 
 // SetupRequired reports whether the instance still needs its first owner.
-func (s *Service) SetupRequired(ctx context.Context) bool {
+// Storage failures are returned, never folded into "setup required" — an
+// unreadable owner store must fail closed (500), not advertise setup mode.
+func (s *Service) SetupRequired(ctx context.Context) (bool, error) {
 	count, err := s.owners.Count(ctx)
-	return err != nil || count == 0
+	if err != nil {
+		return false, err
+	}
+	return count == 0, nil
 }
 
 // Status serves GET /api/auth/status — public; the SPA bootstraps from it.
 func (s *Service) Status(w http.ResponseWriter, r *http.Request) {
-	setupRequired := s.SetupRequired(r.Context())
+	setupRequired, err := s.SetupRequired(r.Context())
+	if err != nil {
+		logger.Error("Owner store unreadable: %v", err)
+		writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+		return
+	}
 	username := ""
 	if !setupRequired {
 		if c, err := r.Cookie(accessCookie); err == nil {
@@ -130,7 +141,13 @@ func (s *Service) Setup(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusBadRequest, "ERR_INVALID_REQUEST")
 		return
 	}
-	if count, _ := s.owners.Count(r.Context()); count > 0 {
+	count, err := s.owners.Count(r.Context())
+	if err != nil {
+		logger.Error("Owner store unreadable during setup: %v", err)
+		writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+		return
+	}
+	if count > 0 {
 		writeError(w, http.StatusConflict, "ERR_SETUP_COMPLETED")
 		return
 	}
@@ -140,8 +157,17 @@ func (s *Service) Setup(w http.ResponseWriter, r *http.Request) {
 	}
 	o, err := s.owners.Create(r.Context(), creds.Username, creds.Password, true)
 	if err != nil {
+		switch {
+		case errors.Is(err, owner.ErrInvalidUsername), errors.Is(err, owner.ErrWeakPassword):
 			s.throttle.Fail(creds.Username)
 			writeError(w, http.StatusBadRequest, "ERR_INVALID_CREDENTIALS")
+		case errors.Is(err, os.ErrExist):
+			// Lost a setup race; the instance already has its first owner.
+			writeError(w, http.StatusConflict, "ERR_SETUP_COMPLETED")
+		default:
+			logger.Error("Owner store write failed during setup: %v", err)
+			writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+		}
 		return
 	}
 	logger.Info("Owner account created: %s", o.Username)
@@ -160,9 +186,14 @@ func (s *Service) Login(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	o, err := s.owners.Verify(r.Context(), creds.Username, creds.Password)
-	if err != nil || o == nil {
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
 			s.throttle.Fail(creds.Username)
 			writeError(w, http.StatusUnauthorized, "ERR_INVALID_CREDENTIALS")
+		} else {
+			logger.Error("Owner store unreadable during login: %v", err)
+			writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+		}
 		return
 	}
 	s.throttle.Reset(creds.Username)
@@ -298,8 +329,14 @@ func (s *Service) ChangePassword(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if _, err := store.ChangePassword(r.Context(), sub, change.CurrentPassword, change.NewPassword); err != nil {
+		switch {
+		case errors.Is(err, os.ErrPermission), errors.Is(err, os.ErrNotExist):
 			s.throttle.Fail(sub)
 			writeError(w, http.StatusBadRequest, "ERR_INVALID_CREDENTIALS")
+		default:
+			logger.Error("Owner store write failed during password change: %v", err)
+			writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+		}
 		return
 	}
 	// Every existing session dies: the password changed under them.
@@ -315,7 +352,13 @@ func (s *Service) ChangePassword(w http.ResponseWriter, r *http.Request) {
 // get the uniform 401; mutating requests pass the CSRF origin check.
 func (s *Service) RequireSession(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if s.SetupRequired(r.Context()) {
+		setupRequired, err := s.SetupRequired(r.Context())
+		if err != nil {
+			logger.Error("Owner store unreadable: %v", err)
+			writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+			return
+		}
+		if setupRequired {
 			writeError(w, http.StatusPreconditionRequired, "ERR_SETUP_REQUIRED")
 			return
 		}
@@ -342,7 +385,13 @@ func (s *Service) RequireSession(next http.Handler) http.Handler {
 // rejected handshakes close with 401 instead of switching protocols.
 func (s *Service) GateWebSocket(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if s.SetupRequired(r.Context()) {
+		setupRequired, err := s.SetupRequired(r.Context())
+		if err != nil {
+			logger.Error("Owner store unreadable: %v", err)
+			writeError(w, http.StatusInternalServerError, "ERR_INTERNAL")
+			return
+		}
+		if setupRequired {
 			writeError(w, http.StatusPreconditionRequired, "ERR_SETUP_REQUIRED")
 			return
 		}

src/handlers/auth/auth_test.go

@@ -19,6 +19,8 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -394,3 +396,83 @@ func TestGateWebSocketRejectsAnonymousUpgrade(t *testing.T) {
 		t.Fatalf("anonymous ws = %d, want 401", rec.Code)
 	}
 }
+
+// newBrokenStoreServer wires the same mux as newTestServer but pre-corrupts
+// owners.json so every owner-store read fails — the on-disk stand-in for an
+// unreadable (e.g. root-owned after a pre-handoff boot) state directory.
+func newBrokenStoreServer(t *testing.T) (*Service, http.Handler) {
+	t.Helper()
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "owners.json"), []byte("{not json"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	owners, err := owner.NewFileOwnerStore(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tokens, err := owner.NewTokenService(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	sessions, err := owner.NewFileSessionStore(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s := New(owners, tokens, sessions)
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/api/auth/status", s.Status)
+	mux.HandleFunc("/api/auth/setup", s.Setup)
+	mux.HandleFunc("/api/auth/login", s.Login)
+	mux.Handle("/api/gated", s.RequireSession(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})))
+	return s, mux
+}
+
+func assertErrorCode(t *testing.T, rec *httptest.ResponseRecorder, code string) {
+	t.Helper()
+	var e errorBody
+	if err := json.Unmarshal(rec.Body.Bytes(), &e); err != nil {
+		t.Fatalf("body %q: %v", rec.Body.String(), err)
+	}
+	if e.Error.Code != code {
+		t.Fatalf("error code = %q, want %q", e.Error.Code, code)
+	}
+}
+
+// TestStorageFailuresAnswerInternalError pins the fail-closed contract: an
+// unreadable owner store is a 500 everywhere — never a 4xx that blames the
+// credentials (the ERR_INVALID_CREDENTIALS-on-EACCES bug) and never a 428
+// that re-advertises setup mode.
+func TestStorageFailuresAnswerInternalError(t *testing.T) {
+	_, h := newBrokenStoreServer(t)
+
+	rec := do(t, h, http.MethodGet, "/api/auth/status", "", nil)
+	if rec.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, want 500", rec.Code)
+	}
+	assertErrorCode(t, rec, "ERR_INTERNAL")
+
+	rec = do(t, h, http.MethodPost, "/api/auth/setup",
+		`{"username":"admin","password":"first-owner-pass"}`, nil)
+	if rec.Code != http.StatusInternalServerError {
+		t.Fatalf("setup = %d, want 500", rec.Code)
+	}
+	assertErrorCode(t, rec, "ERR_INTERNAL")
+
+	rec = do(t, h, http.MethodPost, "/api/auth/login",
+		`{"username":"admin","password":"first-owner-pass"}`, nil)
+	if rec.Code != http.StatusInternalServerError {
+		t.Fatalf("login = %d, want 500", rec.Code)
+	}
+	assertErrorCode(t, rec, "ERR_INTERNAL")
+
+	req := httptest.NewRequest(http.MethodGet, "/api/gated", nil)
+	rec2 := httptest.NewRecorder()
+	h.ServeHTTP(rec2, req)
+	if rec2.Code != http.StatusInternalServerError {
+		t.Fatalf("gated route = %d, want 500 (fail closed)", rec2.Code)
+	}
+	assertErrorCode(t, rec2, "ERR_INTERNAL")
+}

src/lib/containers/install.go

@@ -202,6 +202,9 @@ func EnsureDataRoot() error {
 		if err != nil {
 			return err
 		}
+		if err := healOwnerState(root, identity); err != nil {
+			return fmt.Errorf("failed to hand owner state dir to uid %d: %w", identity.uid, err)
+		}
 		if err := os.Chown(root, identity.uid, identity.gid); err != nil {
 			return fmt.Errorf("failed to chown data root %s to %d:%d: %w", root, identity.uid, identity.gid, err)
 		}
@@ -212,6 +215,33 @@ func EnsureDataRoot() error {
 	return nil
 }
 
+// healOwnerState gives the instance-owner gate's state directory to the
+// runtime user. Boots before the privilege drop opened the gate's stores as
+// root first, leaving a root-owned <root>/maestro behind; after the handoff
+// every write then failed with EACCES (surfacing as a bogus
+// ERR_INVALID_CREDENTIALS on /api/auth/setup). Repairs such installs here,
+// while the process can still chown. Missing dirs are fine — nothing to heal.
+func healOwnerState(root string, id runtimeIdentity) error {
+	dir := filepath.Join(root, "maestro")
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("read owner state dir: %w", err)
+	}
+	if err := os.Chown(dir, id.uid, id.gid); err != nil {
+		return fmt.Errorf("chown %s: %w", dir, err)
+	}
+	for _, e := range entries {
+		p := filepath.Join(dir, e.Name())
+		if err := os.Chown(p, id.uid, id.gid); err != nil && !os.IsNotExist(err) {
+			return fmt.Errorf("chown %s: %w", p, err)
+		}
+	}
+	return nil
+}
+
 // runtimeIdentity is the user the container should run as, plus any
 // supplementary groups needed to keep working (e.g. the docker socket group).
 type runtimeIdentity struct {

src/lib/owner/owner.go

@@ -27,6 +27,7 @@ import (
 	"crypto/subtle"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -249,14 +250,19 @@ func (s *FileOwnerStore) Create(_ context.Context, username, password string, is
 
 func (s *FileOwnerStore) Verify(_ context.Context, username, password string) (*Owner, error) {
 	o, err := s.Get(context.Background(), username)
-	if err != nil || o == nil {
+	if err != nil {
+		// Unknown user is a credentials miss; keep storage failures
+		// distinguishable so callers can answer 500 instead of 401.
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, os.ErrNotExist
+		}
+		return nil, err
+	}
+	if !VerifyPassword(o.PasswordHash, password) {
 		return nil, os.ErrNotExist
 	}
-	if VerifyPassword(o.PasswordHash, password) {
 	return o, nil
 }
-	return nil, os.ErrNotExist
-}
 
 func (s *FileOwnerStore) Get(_ context.Context, username string) (*Owner, error) {
 	s.mu.Lock()

src/lib/owner/owner_test.go

@@ -166,3 +166,29 @@ func TestOwnerFilePermissions(t *testing.T) {
 		t.Fatalf("owners.json mode = %o, want 600", perm)
 	}
 }
+
+// TestVerifyDistinguishesStorageFailureFromUnknownUser pins the contract the
+// auth handlers rely on: an unknown user or wrong password surfaces as
+// os.ErrNotExist (→ 401), while a broken store keeps its own error (→ 500).
+func TestVerifyDistinguishesStorageFailureFromUnknownUser(t *testing.T) {
+	ctx := context.Background()
+	dir := t.TempDir()
+	s, err := NewFileOwnerStore(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Create(ctx, "admin", "verify-check-pass", true); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Verify(ctx, "ghost", "verify-check-pass"); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("unknown user: %v", err)
+	}
+
+	broken := []byte("{not json")
+	if err := os.WriteFile(filepath.Join(dir, "owners.json"), broken, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.Verify(ctx, "admin", "verify-check-pass"); err == nil || errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("storage failure collapsed into ErrNotExist: %v", err)
+	}
+}

src/main.go

@@ -75,6 +75,14 @@ func main() {
 	if err != nil {
 		log.Fatalf("Failed to resolve data root: %v", err)
 	}
+	// Drop to the host user BEFORE any state is created: the stores below
+	// mkdir/write under <dataRoot>/maestro, and files made during the root
+	// phase stay root-owned after the handoff, locking the owner gate out of
+	// its own store (EACCES surfaced as a bogus ERR_INVALID_CREDENTIALS on
+	// /api/auth/setup).
+	if err := libcontainers.EnsureDataRoot(); err != nil {
+		log.Fatalf("Data root check failed: %v", err)
+	}
 	maestroDir := filepath.Join(dataRoot, "maestro")
 	ownerStore, err := owner.NewFileOwnerStore(maestroDir)
 	if err != nil {
@@ -100,9 +108,6 @@ func main() {
 			log.Fatalf("Failed to init docker client: %v", err)
 		}
 		defer dc.Close()
-		if err := libcontainers.EnsureDataRoot(); err != nil {
-			log.Fatalf("Data root check failed: %v", err)
-		}
 		// Provision the instance CA on first run (security theme M1). The CA is
 		// the trust anchor for every peer edge on dcntrlized_net; services get
 		// their identity certs from it at start/install.