fix: validate names that are used as directory names

internal/server/auth.go

@@ -274,6 +274,12 @@ func (s *Server) handleRegisterPost(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if !isValidUsername(username) {
+		slog.Warn("Registration rejected: invalid username", "username", username)
+		s.renderTemplate(w, r, "register", TemplateData{Flashes: []string{localizer.MustLocalize(&i18n.LocalizeConfig{MessageID: "Invalid username."})}})
+		return
+	}
+
 	if _, err := gorm.G[data.User](s.DB).Where("username = ?", username).First(r.Context()); err == nil {
 		s.renderTemplate(w, r, "register", TemplateData{Flashes: []string{localizer.MustLocalize(&i18n.LocalizeConfig{MessageID: "Username already exists"})}})
 		return

internal/server/handlers_app.go

@@ -1183,9 +1183,9 @@ func (s *Server) handleUploadAppPost(w http.ResponseWriter, r *http.Request) {
 	appName := strings.TrimSuffix(filename, ext)
 
 	userAppsDir := filepath.Join(s.DataDir, "users", user.Username, "apps")
-	appDir, err := securejoin.SecureJoin(userAppsDir, appName)
+	appDir, err := userAppDir(userAppsDir, appName)
 	if err != nil {
-		slog.Warn("Path traversal attempt blocked", "error", err)
+		slog.Warn("Rejected upload with invalid app name", "filename", filename, "error", err)
 		http.Error(w, "Invalid app name", http.StatusBadRequest)
 		return
 	}
@@ -1243,6 +1243,23 @@ func (s *Server) handleUploadAppPost(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, fmt.Sprintf("/devices/%s/addapp", device.ID), http.StatusSeeOther)
 }
 
+// userAppDir returns the directory for one uploaded app inside userAppsDir.
+// The app name must be non-empty, must not start with a dot, and must
+// resolve to a subdirectory of userAppsDir.
+func userAppDir(userAppsDir, appName string) (string, error) {
+	if strings.TrimSpace(appName) == "" || strings.HasPrefix(appName, ".") {
+		return "", fmt.Errorf("invalid app name %q", appName)
+	}
+	appDir, err := securejoin.SecureJoin(userAppsDir, appName)
+	if err != nil {
+		return "", err
+	}
+	if appDir == filepath.Clean(userAppsDir) {
+		return "", fmt.Errorf("app name %q resolves to the apps directory", appName)
+	}
+	return appDir, nil
+}
+
 func (s *Server) parseManifest(tempExtractDir string) (string, error) {
 	manifestPath := filepath.Join(tempExtractDir, "manifest.yaml")
 	data, err := os.ReadFile(manifestPath)
@@ -1317,9 +1334,9 @@ func (s *Server) handleZipUpload(w http.ResponseWriter, r *http.Request, user *d
 	}
 
 	// Re-calculate appDir with potentially new appName
-	appDir, err := securejoin.SecureJoin(userAppsDir, appName)
+	appDir, err := userAppDir(userAppsDir, appName)
 	if err != nil {
-		slog.Warn("Path traversal attempt blocked", "error", err)
+		slog.Warn("Rejected zip upload with invalid app name", "app_name", appName, "error", err)
 		return err
 	}
 

internal/server/handlers_device.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"log/slog"
@@ -30,6 +31,29 @@ func slugifyDeviceName(name string) string {
 	return s
 }
 
+// uniqueDeviceIDFromName builds a device ID from the device name. Device IDs
+// are global, so if the slug is taken (possibly by another user's device) a
+// numeric suffix is added. A name with no letters or digits gets a random ID.
+func (s *Server) uniqueDeviceIDFromName(ctx context.Context, name string) (string, error) {
+	base := slugifyDeviceName(name)
+	if base == "" {
+		return generateSecureToken(8)
+	}
+
+	candidate := base
+	for i := 2; i <= 100; i++ {
+		count, err := gorm.G[data.Device](s.DB).Where("id = ?", candidate).Count(ctx, "*")
+		if err != nil {
+			return "", err
+		}
+		if count == 0 {
+			return candidate, nil
+		}
+		candidate = fmt.Sprintf("%s-%d", base, i)
+	}
+	return generateSecureToken(8)
+}
+
 func (s *Server) handleCreateDeviceGet(w http.ResponseWriter, r *http.Request) {
 	user := GetUser(r)
 
@@ -148,7 +172,13 @@ func (s *Server) handleCreateDevicePost(w http.ResponseWriter, r *http.Request)
 	} else {
 		switch formData.DeviceIDMode {
 		case "from_name":
-			deviceID = slugifyDeviceName(formData.Name)
+			var err error
+			deviceID, err = s.uniqueDeviceIDFromName(r.Context(), formData.Name)
+			if err != nil {
+				slog.Error("Failed to generate device ID", "error", err)
+				http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+				return
+			}
 		case "hex16":
 			var err error
 			deviceID, err = generateSecureToken(16)

internal/server/handlers_user.go

@@ -119,6 +119,11 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) {
 
 	// Clean up files
 	for _, d := range targetUser.Devices {
+		if !isSinglePathComponent(d.ID) {
+			// Devices created by older versions may have an empty ID.
+			slog.Warn("Skipping webp cleanup for device with unsafe ID", "device_id", d.ID)
+			continue
+		}
 		deviceWebpDir, err := s.ensureDeviceImageDir(d.ID)
 		if err != nil {
 			slog.Error("Failed to get device webp directory for deletion", "device_id", d.ID, "error", err)
@@ -129,9 +134,14 @@ func (s *Server) handleDeleteUser(w http.ResponseWriter, r *http.Request) {
 			slog.Error("Failed to remove device webp directory", "device_id", d.ID, "error", err)
 		}
 	}
-	userAppsDir := filepath.Join(s.DataDir, "users", targetUsername)
-	if err := os.RemoveAll(userAppsDir); err != nil {
-		slog.Error("Failed to remove user apps directory", "username", targetUsername, "error", err)
+	// Only remove the directory when the stored username is a single path component.
+	if isSinglePathComponent(targetUsername) {
+		userAppsDir := filepath.Join(s.DataDir, "users", targetUsername)
+		if err := os.RemoveAll(userAppsDir); err != nil {
+			slog.Error("Failed to remove user apps directory", "username", targetUsername, "error", err)
+		}
+	} else {
+		slog.Warn("Skipping removal of user directory for unsafe username", "username", targetUsername)
 	}
 
 	err = s.DB.Transaction(func(tx *gorm.DB) error {

internal/server/helpers.go

@@ -372,6 +372,24 @@ func generateSecureToken(length int) (string, error) {
 	return hex.EncodeToString(b)[:length], nil // Take only requested length
 }
 
+// validUsernameRe allows letters, digits and the punctuation found in email
+// addresses (OIDC usernames are often emails). The first character must be a
+// letter or digit, which rules out "." and "..".
+var validUsernameRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._@+-]{0,127}$`)
+
+// isValidUsername reports whether a username can be created. Usernames are
+// used as directory names under DataDir/users.
+func isValidUsername(username string) bool {
+	return validUsernameRe.MatchString(username)
+}
+
+// isSinglePathComponent reports whether name is a single, non-empty path
+// component other than "." or "..". It covers usernames stored before
+// isValidUsername existed.
+func isSinglePathComponent(name string) bool {
+	return name != "" && name != "." && name != ".." && !strings.ContainsAny(name, `/\`)
+}
+
 // flashAndRedirect adds a flash message and redirects to the specified URL.
 func (s *Server) flashAndRedirect(w http.ResponseWriter, r *http.Request, messageID string, redirectURL string, status int) {
 	localizer := s.getLocalizer(r)
@@ -575,6 +593,10 @@ func (s *Server) saveSession(w http.ResponseWriter, r *http.Request, session *se
 
 // ensureDeviceImageDir is a helper to get and ensure the device webp directory exists.
 func (s *Server) ensureDeviceImageDir(deviceID string) (string, error) {
+	// Each device gets its own subdirectory of webp.
+	if !isSinglePathComponent(deviceID) {
+		return "", fmt.Errorf("invalid device ID for webp directory: %q", deviceID)
+	}
 	path, err := securejoin.SecureJoin(filepath.Join(s.DataDir, "webp"), deviceID)
 	if err != nil {
 		return "", fmt.Errorf("failed to securejoin path for device webp directory %s: %w", deviceID, err)

internal/server/oidc.go

@@ -496,6 +496,14 @@ func (s *Server) handleOIDCNewIdentity(w http.ResponseWriter, r *http.Request, u
 // handleOIDCCreateUser creates a new user from OIDC and logs them in.
 func (s *Server) handleOIDCCreateUser(w http.ResponseWriter, r *http.Request, username, email string, claims map[string]any, prov *OIDCProvider, localizer *i18n.Localizer) {
 	ctx := r.Context()
+	if !isValidUsername(username) {
+		slog.Warn("OIDC login: refusing to auto-create user with invalid username", "username", username, "subject", claims["sub"])
+		s.renderTemplate(w, r, "login", TemplateData{
+			Flashes: []string{localizer.MustLocalize(&i18n.LocalizeConfig{MessageID: "Invalid username."})},
+		})
+		return
+	}
+
 	apiKey, err := generateSecureToken(32)
 	if err != nil {
 		slog.Error("Failed to generate API key for new user", "error", err)

internal/server/path_validation_test.go

@@ -0,0 +1,192 @@
+package server
+
+import (
+	"bytes"
+	"context"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"tronbyt-server/internal/data"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
+)
+
+func TestIsValidUsername(t *testing.T) {
+	for _, name := range []string{"admin", "Alice", "user1", "jane.doe", "jane_doe", "jane-doe", "jane+tag@example.com", "1"} {
+		assert.True(t, isValidUsername(name), "expected %q to be valid", name)
+	}
+	for _, name := range []string{"", ".", "..", "../admin", "a/b", `a\b`, ".hidden", "-dash", "has space", "colon:name", strings.Repeat("a", 129)} {
+		assert.False(t, isValidUsername(name), "expected %q to be invalid", name)
+	}
+}
+
+func TestHandleRegisterPost_RejectsInvalidUsername(t *testing.T) {
+	s := newTestServer(t)
+	ctx := context.Background()
+
+	for _, username := range []string{"..", "../users/alice", "a/b"} {
+		form := url.Values{}
+		form.Add("username", username)
+		form.Add("password", "password123")
+
+		req, _ := http.NewRequest(http.MethodPost, "/auth/register", strings.NewReader(form.Encode()))
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+		rr := httptest.NewRecorder()
+		http.HandlerFunc(s.handleRegisterPost).ServeHTTP(rr, req)
+
+		assert.Equal(t, http.StatusOK, rr.Code, "username %q", username)
+		count, err := gorm.G[data.User](s.DB).Where("username = ?", username).Count(ctx, "*")
+		require.NoError(t, err)
+		assert.Zero(t, count, "user %q should not have been created", username)
+	}
+}
+
+// Deleting a user whose stored name predates validation only removes that
+// user's own files.
+func TestHandleDeleteUser_LegacyUsername(t *testing.T) {
+	s := newTestServer(t)
+	ctx := context.Background()
+
+	// api_key is unique, so each user needs its own.
+	admin := data.User{Username: "admin", IsAdmin: true, APIKey: "admin-key"}
+	require.NoError(t, gorm.G[data.User](s.DB).Create(ctx, &admin))
+	bad := data.User{Username: "..", APIKey: "bad-key"}
+	require.NoError(t, gorm.G[data.User](s.DB).Create(ctx, &bad))
+	// Older versions could create a device with an empty ID.
+	require.NoError(t, gorm.G[data.Device](s.DB).Create(ctx, &data.Device{ID: "", Username: ".."}))
+
+	sentinel := filepath.Join(s.DataDir, "keep-me")
+	require.NoError(t, os.WriteFile(sentinel, []byte("x"), 0644))
+	otherImage := filepath.Join(s.DataDir, "webp", "otherdevice", "img.webp")
+	require.NoError(t, os.MkdirAll(filepath.Dir(otherImage), 0755))
+	require.NoError(t, os.WriteFile(otherImage, []byte("x"), 0644))
+
+	req, _ := http.NewRequest(http.MethodPost, "/admin/users/../delete", nil)
+	req.SetPathValue("username", "..")
+	req = req.WithContext(context.WithValue(req.Context(), userContextKey, &admin))
+
+	rr := httptest.NewRecorder()
+	http.HandlerFunc(s.handleDeleteUser).ServeHTTP(rr, req)
+
+	assert.FileExists(t, sentinel, "files outside the user's directory are kept")
+	assert.FileExists(t, otherImage, "other devices' images are kept")
+	count, err := gorm.G[data.User](s.DB).Where("username = ?", "..").Count(ctx, "*")
+	require.NoError(t, err)
+	assert.Zero(t, count, "user should still be deleted from the database")
+}
+
+func TestEnsureDeviceImageDir_RejectsUnsafeIDs(t *testing.T) {
+	s := newTestServer(t)
+	for _, id := range []string{"", ".", ".."} {
+		_, err := s.ensureDeviceImageDir(id)
+		assert.Error(t, err, "device ID %q", id)
+	}
+}
+
+func TestUniqueDeviceIDFromName(t *testing.T) {
+	s := newTestServer(t)
+	ctx := context.Background()
+
+	id, err := s.uniqueDeviceIDFromName(ctx, "Kitchen")
+	require.NoError(t, err)
+	assert.Equal(t, "kitchen", id)
+
+	// Device IDs are global: another user's "Kitchen" must not collide.
+	require.NoError(t, gorm.G[data.Device](s.DB).Create(ctx, &data.Device{ID: "kitchen", Username: "someone-else"}))
+	id, err = s.uniqueDeviceIDFromName(ctx, "Kitchen")
+	require.NoError(t, err)
+	assert.Equal(t, "kitchen-2", id)
+
+	// A name with no letters or digits must not produce an empty ID.
+	for _, name := range []string{"!!!", "🎉", "---"} {
+		id, err := s.uniqueDeviceIDFromName(ctx, name)
+		require.NoError(t, err)
+		assert.Len(t, id, 8, "name %q", name)
+		assert.Regexp(t, validDeviceIDRe, id)
+	}
+}
+
+func TestHandleCreateDevicePost_FromNameWithoutAlphanumerics(t *testing.T) {
+	s := newTestServer(t)
+	ctx := context.Background()
+
+	user := data.User{Username: "testuser"}
+	require.NoError(t, gorm.G[data.User](s.DB).Create(ctx, &user))
+
+	form := url.Values{}
+	form.Add("name", "🎉🎉")
+	form.Add("device_type", "tidbyt_gen1")
+	form.Add("brightness", "2")
+	form.Add("device_id_mode", "from_name")
+
+	req, _ := http.NewRequest(http.MethodPost, "/devices/create", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req = req.WithContext(context.WithValue(req.Context(), userContextKey, &user))
+
+	rr := httptest.NewRecorder()
+	http.HandlerFunc(s.handleCreateDevicePost).ServeHTTP(rr, req)
+	require.Equal(t, http.StatusSeeOther, rr.Code, "body: %s", rr.Body.String())
+
+	device, err := gorm.G[data.Device](s.DB).Where("name = ?", "🎉🎉").First(ctx)
+	require.NoError(t, err)
+	assert.NotEmpty(t, device.ID)
+}
+
+// Uploads whose app name is empty or starts with a dot are rejected.
+func TestHandleUploadAppPost_RejectsDotNames(t *testing.T) {
+	s := newTestServer(t)
+	ctx := context.Background()
+
+	user := data.User{Username: "testuser"}
+	require.NoError(t, gorm.G[data.User](s.DB).Create(ctx, &user))
+	device := data.Device{ID: "testdevice", Username: "testuser"}
+	require.NoError(t, gorm.G[data.Device](s.DB).Create(ctx, &device))
+
+	existing := filepath.Join(s.DataDir, "users", "testuser", "apps", "myapp", "myapp.star")
+	require.NoError(t, os.MkdirAll(filepath.Dir(existing), 0755))
+	require.NoError(t, os.WriteFile(existing, []byte("x"), 0644))
+
+	for _, filename := range []string{".zip", "..zip", ".star"} {
+		body := &bytes.Buffer{}
+		writer := multipart.NewWriter(body)
+		part, err := writer.CreateFormFile("file", filename)
+		require.NoError(t, err)
+		_, err = part.Write([]byte("not really a zip"))
+		require.NoError(t, err)
+		require.NoError(t, writer.Close())
+
+		req, _ := http.NewRequest(http.MethodPost, "/devices/testdevice/uploadapp", body)
+		req.Header.Set("Content-Type", writer.FormDataContentType())
+		reqCtx := context.WithValue(req.Context(), userContextKey, &user)
+		reqCtx = context.WithValue(reqCtx, deviceContextKey, &device)
+		req = req.WithContext(reqCtx)
+
+		rr := httptest.NewRecorder()
+		http.HandlerFunc(s.handleUploadAppPost).ServeHTTP(rr, req)
+
+		assert.Equal(t, http.StatusBadRequest, rr.Code, "filename %q", filename)
+		assert.FileExists(t, existing, "existing app is kept after upload of %q", filename)
+	}
+}
+
+func TestUserAppDir(t *testing.T) {
+	root := t.TempDir()
+
+	dir, err := userAppDir(root, "myapp")
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Join(root, "myapp"), dir)
+
+	for _, name := range []string{"", " ", ".", "..", ".hidden", "../other"} {
+		_, err := userAppDir(root, name)
+		assert.Error(t, err, "app name %q", name)
+	}
+}

web/i18n/de.json

@@ -1787,6 +1787,9 @@
   "Username already exists": {
     "other": "Benutzername existiert bereits."
   },
+  "Invalid username.": {
+    "other": "Ungültiger Benutzername. Nur Buchstaben, Ziffern und . _ @ + - sind erlaubt; das erste Zeichen muss ein Buchstabe oder eine Ziffer sein."
+  },
   "Invalid username or password": {
     "other": "Ungültiger Benutzername oder Passwort."
   },