Storage: Propagate insufficient-storage to CLI exit and index 507 #5613

internal/api/README.md

@@ -58,6 +58,28 @@ The API package exposes PhotoPrism’s HTTP endpoints via Gin handlers. Each fil
   }, refID)
   ```
 
+### User-Visible Notifications vs Audit Log
+
+`event.AuditInfo` / `AuditWarn` / `AuditErr` write to the audit log and broadcast on `audit.log.<level>` — the toast component on the frontend does NOT subscribe to that channel, so an audit entry alone produces no UI feedback. To raise a red or green toast in the browser, publish on the `notify.*` channel via `event.Error(msg)` / `event.ErrorMsg(id, …)` (red) or `event.Success(msg)` (green).
+
+The two helpers have distinct subscribers; choose based on who the message is for:
+
+- **Short endpoints whose response the frontend reads** (single-shot CRUD, login, settings updates). The calling component renders the response, so `AuditErr` plus an HTTP error is enough — the UI gets the error string from the response body.
+- **Long-running endpoints that the UI drives via the event hub** (`POST /api/v1/index`, `POST /api/v1/import/*path`, and similar). The frontend cancels the in-flight HTTP request on the first `index.*` / `import.*` wire event, so the response body is invisible in normal operation. In-flight failures that need a specific toast MUST be published via `event.ErrorMsg(...)` on `notify.error`; an HTTP error alone produces only the frontend's generic fallback toast (or nothing, if the cancel has already fired).
+- **Forensic events that don't need UI surfacing** (rate limiting, ACL denials, internal aborts whose user-visible signal comes from a sibling channel). `AuditErr` alone is the right call.
+
+When in doubt, ask: "after this handler returns, what does the user see?" If the answer is "the frontend will read the response", `AuditErr` covers it. If the answer is "the page is already subscribed to wire events and the response is discarded", publish on `notify.*` as well.
+
+```go
+// Forensic audit only — frontend will read the response body and render the error.
+event.AuditErr([]string{ClientIP(c), "session %s", "delete album", status.Failed}, s.RefID)
+AbortBadRequest(c, err)
+
+// Forensic audit + specific red toast — needed when the request was already canceled by the wire.
+event.AuditErr([]string{ClientIP(c), "session %s", "index files", status.Failed}, s.RefID)
+event.ErrorMsg(i18n.ErrIndexingFailed)
+```
+
 ### Swagger Documentation
 
 - Annotate handlers with Swagger comments that include full `/api/v1/...` paths, request/response schemas, and security definitions. Only annotate routes that are externally accessible.

internal/api/index.go

@@ -16,6 +16,7 @@ import (
 	"github.com/photoprism/photoprism/internal/photoprism/get"
 	"github.com/photoprism/photoprism/pkg/clean"
 	"github.com/photoprism/photoprism/pkg/i18n"
+	"github.com/photoprism/photoprism/pkg/log/status"
 	"github.com/photoprism/photoprism/pkg/txt"
 )
 
@@ -46,6 +47,16 @@ func StartIndexing(router *gin.RouterGroup) {
 			return
 		}
 
+		// Abort if the storage filesystem is critically low on free space. Indexing only
+		// writes sidecars and thumbnails, so the disk-low check is sufficient here; the
+		// FilesQuota gate used by import/upload/avatar/zip/video does not apply because
+		// indexing catalogs existing files rather than adding to the quota.
+		if _, low, _ := conf.StorageLow(); low {
+			event.AuditErr([]string{ClientIP(c), "session %s", "index files", status.InsufficientStorage}, s.RefID)
+			Abort(c, http.StatusInsufficientStorage, i18n.ErrInsufficientStorage)
+			return
+		}
+
 		start := time.Now()
 
 		var frm form.IndexOptions
@@ -170,7 +181,11 @@ func StartIndexing(router *gin.RouterGroup) {
 
 		msg := i18n.Msg(i18n.MsgIndexingCompletedIn, elapsed)
 
-		event.Success(msg)
+		// Report success only if at least one file was indexed.
+		if indexed > 0 {
+			event.Success(msg)
+		}
+
 		event.Publish("index.completed", event.Data{
 			"uid":     indOpt.UID,
 			"action":  indOpt.Action,

internal/api/index_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/photoprism/photoprism/pkg/fs/disk"
 	"github.com/photoprism/photoprism/pkg/i18n"
 )
 
@@ -29,3 +30,18 @@ func TestCancelIndex(t *testing.T) {
 		assert.Equal(t, http.StatusOK, resp.Code)
 	})
 }
+
+func TestStartIndexing(t *testing.T) {
+	t.Run("InsufficientStorage", func(t *testing.T) {
+		app, router, conf := NewApiTest()
+
+		disk.FlushFree()
+		t.Cleanup(disk.FlushFree)
+		disk.SetFree(conf.StoragePath(), 1, 1000)
+
+		StartIndexing(router)
+		r := PerformRequestWithBody(app, "POST", "/api/v1/index", "{}")
+
+		assert.Equal(t, http.StatusInsufficientStorage, r.Code)
+	})
+}

internal/commands/README.md

@@ -38,6 +38,24 @@ Helper behavior:
 
 The underlying parser limitation is tracked as a known issue for a broader fix; until a global arg-reorder pass lands, all new leaf actions that accept a positional MUST call `RejectTrailingFlags` before applying flag values.
 
+### Exit Codes
+
+Wrap errors in `cli.Exit(err, <code>)` so the binary terminates with a non-zero status. A plain `return err` is logged but exits `0`, which hides failures from CI and shell scripts. Pick the code from the table below; cluster, JWT, and Portal commands set the precedent.
+
+| Code | Meaning                                  | Typical Use                                                                       |
+|------|------------------------------------------|-----------------------------------------------------------------------------------|
+| `0`  | Success, or user-initiated cancel        | normal completion; `ErrCanceled` from a long-running operation                    |
+| `1`  | Runtime/execution failure                | DB or I/O error, backup/restore failure, indexing failure, `InitConfig` error     |
+| `2`  | Input validation or precondition failure | missing/invalid flag, `RejectTrailingFlags`, read-only mode, malformed identifier |
+| `3`  | Resource not found                       | user, node, theme, or other named entity does not exist                           |
+| `4`  | Authentication or authorization failure  | Portal returned `401` to the CLI                                                  |
+| `5`  | Forbidden / conflict                     | Portal returned `403` or `409` to the CLI                                         |
+| `6`  | Rate limited                             | Portal returned `429` to the CLI                                                  |
+
+`urfave/cli`'s default `ExitErrHandler` calls `os.Exit(c.ExitCode())` only for values that implement `cli.ExitCoder`. A bare `error` flows up to `main()`, which logs it and returns normally — that is, exits `0`. Use `cli.Exit(...)` whenever a non-zero status matters; reserve plain `return err` for helpers that propagate to a caller which itself wraps the result.
+
+For long-running operations (indexing, importing, backup) that may be canceled by the user, return the underlying `status.ErrCanceled` (or a wrapper) without `cli.Exit` so the CLI exits `0`, and use `cli.Exit(err, 1)` for `status.ErrInsufficientStorage` and other runtime failures so scripts and CI can detect them.
+
 ### Configuration & Flags Integration
 
 - Define new options in `internal/config/options.go` with the appropriate struct tags (`yaml`, `json`, `flag`) so they propagate to YAML, CLI, and API layers consistently.

internal/commands/auth_add.go

@@ -51,14 +51,14 @@ func authAddAction(ctx *cli.Context) error {
 		// Reject flags placed after the username; the stdlib flag parser
 		// would silently drop them and create the token with the defaults.
 		if err := RejectTrailingFlags(ctx); err != nil {
-			return err
+			return cli.Exit(err, 2)
 		}
 
 		// Find user account.
 		user := entity.FindUserByName(userName)
 
 		if user == nil && userName != "" {
-			return fmt.Errorf("user %s not found", clean.LogQuote(userName))
+			return cli.Exit(fmt.Errorf("user %s not found", clean.LogQuote(userName)), 3)
 		}
 
 		// Get client name from command flag or ask for it.
@@ -73,7 +73,7 @@ func authAddAction(ctx *cli.Context) error {
 			res, err := prompt.Run()
 
 			if err != nil {
-				return err
+				return cli.Exit(err, 1)
 			}
 
 			clientName = clean.Name(res)
@@ -91,7 +91,7 @@ func authAddAction(ctx *cli.Context) error {
 			res, err := prompt.Run()
 
 			if err != nil {
-				return err
+				return cli.Exit(err, 1)
 			}
 
 			authScope = clean.Scope(res)
@@ -101,19 +101,18 @@ func authAddAction(ctx *cli.Context) error {
 		sess, err := entity.AddClientSession(clientName, ctx.Int64("expires"), authScope, authn.GrantCLI, user)
 
 		if err != nil {
-			return fmt.Errorf("failed to create authentication secret: %s", err)
-		} else {
-			// Show client authentication credentials.
-			if sess.UserUID == "" {
-				fmt.Printf("\nPLEASE COPY THE FOLLOWING RANDOMLY GENERATED ACCESS TOKEN AND KEEP IT IN A SAFE PLACE, AS YOU WILL NOT BE ABLE TO SEE IT AGAIN:\n")
-				fmt.Printf("\n%s\n", report.Credentials("Access Token", sess.AuthToken(), "Authorization Scope", sess.Scope()))
-			} else {
-				fmt.Printf("\nPLEASE COPY THE FOLLOWING RANDOMLY GENERATED APP PASSWORD AND KEEP IT IN A SAFE PLACE, AS YOU WILL NOT BE ABLE TO SEE IT AGAIN:\n")
-				fmt.Printf("\n%s\n", report.Credentials("App Password", sess.AuthToken(), "Authorization Scope", sess.Scope()))
-			}
-
+			return cli.Exit(fmt.Errorf("failed to create authentication secret: %s", err), 1)
 		}
 
-		return err
+		// Show client authentication credentials.
+		if sess.UserUID == "" {
+			fmt.Printf("\nPLEASE COPY THE FOLLOWING RANDOMLY GENERATED ACCESS TOKEN AND KEEP IT IN A SAFE PLACE, AS YOU WILL NOT BE ABLE TO SEE IT AGAIN:\n")
+			fmt.Printf("\n%s\n", report.Credentials("Access Token", sess.AuthToken(), "Authorization Scope", sess.Scope()))
+		} else {
+			fmt.Printf("\nPLEASE COPY THE FOLLOWING RANDOMLY GENERATED APP PASSWORD AND KEEP IT IN A SAFE PLACE, AS YOU WILL NOT BE ABLE TO SEE IT AGAIN:\n")
+			fmt.Printf("\n%s\n", report.Credentials("App Password", sess.AuthToken(), "Authorization Scope", sess.Scope()))
+		}
+
+		return nil
 	})
 }

internal/commands/auth_list.go

@@ -35,7 +35,7 @@ func authListAction(ctx *cli.Context) error {
 		results, err := query.Sessions(ctx.Int("count"), 0, "", ctx.Args().First())
 
 		if err != nil {
-			return err
+			return cli.Exit(err, 1)
 		}
 
 		// Show log message.

internal/commands/backup.go

@@ -17,7 +17,9 @@ import (
 const backupDescription = `A custom filename for the database backup (or - to send the backup to stdout) can optionally be passed as argument.
 The --database flag can be omitted in this case. When using Docker, please run the docker command with the -T flag
 to prevent log messages from being sent to stdout. If nothing else is specified, the database and album backup paths
-will be automatically determined based on the current configuration.`
+will be automatically determined based on the current configuration.
+
+Backups to stdout (-), bypass the insufficient storage check, so dumps can be streamed even when the local storage is full.`
 
 // BackupCommand configures the command name, flags, and action.
 var BackupCommand = &cli.Command{
@@ -87,7 +89,7 @@ func backupAction(ctx *cli.Context) error {
 	defer cancel()
 
 	if err != nil {
-		return err
+		return cli.Exit(err, 1)
 	}
 
 	defer conf.Shutdown()
@@ -110,7 +112,7 @@ func backupAction(ctx *cli.Context) error {
 		}
 
 		if err = backup.Database(databasePath, fileName, fileName == "-", force, retain); err != nil {
-			return fmt.Errorf("failed to create database backup: %w", err)
+			return cli.Exit(fmt.Errorf("failed to create database backup: %w", err), 1)
 		}
 	}
 
@@ -124,7 +126,7 @@ func backupAction(ctx *cli.Context) error {
 		}
 
 		if count, backupErr := backup.Albums(albumsPath, true); backupErr != nil {
-			return backupErr
+			return cli.Exit(backupErr, 1)
 		} else {
 			log.Infof("backup: saved %s", english.Plural(count, "album backup", "album backups"))
 		}

internal/commands/import.go

@@ -39,12 +39,12 @@ func importAction(ctx *cli.Context) error {
 	defer cancel()
 
 	if err != nil {
-		return err
+		return cli.Exit(err, 1)
 	}
 
 	// very if copy directory exist and is writable
 	if conf.ReadOnly() {
-		return config.ErrReadOnly
+		return cli.Exit(config.ErrReadOnly, 2)
 	}
 
 	conf.InitDb()
@@ -56,10 +56,10 @@ func importAction(ctx *cli.Context) error {
 	if sourcePath == "" {
 		sourcePath = conf.ImportPath()
 	} else {
-		abs, err := filepath.Abs(sourcePath)
+		abs, pathErr := filepath.Abs(sourcePath)
 
-		if err != nil {
-			return err
+		if pathErr != nil {
+			return cli.Exit(pathErr, 2)
 		}
 
 		sourcePath = abs
@@ -74,7 +74,7 @@ func importAction(ctx *cli.Context) error {
 	if ctx.IsSet("dest") {
 		destFolder, err = sanitizeDestinationArg(ctx.String("dest"))
 		if err != nil {
-			return err
+			return cli.Exit(err, 2)
 		}
 	} else {
 		destFolder = conf.ImportDest()

internal/commands/index.go

@@ -51,7 +51,7 @@ func indexAction(ctx *cli.Context) error {
 	defer cancel()
 
 	if err != nil {
-		return err
+		return cli.Exit(err, 1)
 	}
 
 	conf.InitDb()
@@ -61,7 +61,7 @@ func indexAction(ctx *cli.Context) error {
 	subPath, err := sanitizeSubfolderArg(ctx.Args().First())
 
 	if err != nil {
-		return err
+		return cli.Exit(err, 2)
 	}
 
 	if subPath == "" {
@@ -118,8 +118,8 @@ func indexAction(ctx *cli.Context) error {
 
 		// Start index and cache cleanup.
 		cleanupStart := time.Now()
-		if thumbnails, _, sidecars, err := w.Start(opt); err != nil {
-			return err
+		if thumbnails, _, sidecars, cleanupErr := w.Start(opt); cleanupErr != nil {
+			return cli.Exit(cleanupErr, 1)
 		} else if total := thumbnails + sidecars; total > 0 {
 			log.Infof("cleanup: deleted %s in total [%s]", english.Plural(total, "file", "files"), time.Since(cleanupStart))
 		}

internal/commands/restore.go

@@ -76,7 +76,7 @@ func restoreAction(ctx *cli.Context) error {
 	defer cancel()
 
 	if err != nil {
-		return err
+		return cli.Exit(err, 1)
 	}
 
 	defer conf.Shutdown()
@@ -85,7 +85,7 @@ func restoreAction(ctx *cli.Context) error {
 	if !restoreDatabase {
 		// Do nothing.
 	} else if err = backup.RestoreDatabase(databasePath, databaseFile, databaseFile == "-", force); err != nil {
-		return err
+		return cli.Exit(err, 1)
 	}
 
 	log.Infoln("restore: migrating index database schema")
@@ -106,7 +106,7 @@ func restoreAction(ctx *cli.Context) error {
 			log.Infof("restore: restoring album backups from %s", clean.Log(albumsPath))
 
 			if count, restoreErr := backup.RestoreAlbums(albumsPath, true); restoreErr != nil {
-				return restoreErr
+				return cli.Exit(restoreErr, 1)
 			} else {
 				log.Infof("restore: restored %s from YAML files", english.Plural(count, "album", "albums"))
 			}

internal/photoprism/backup/database.go

@@ -35,6 +35,11 @@ func Database(backupPath, fileName string, toStdOut, force bool, retain int) (er
 	// Get configuration.
 	c := get.Config()
 
+	// Storage-gate the on-disk path only: when toStdOut is true the dump
+	// streams to stdout and does not consume the local storage volume, so an
+	// operator can still offload a backup over ssh or similar when the disk
+	// is full. Path validation and the InsufficientStorage check therefore
+	// run inside this branch only.
 	if !toStdOut {
 		if backupPath == "" {
 			backupPath = c.BackupDatabasePath()

internal/photoprism/backup/storage_test.go

@@ -48,3 +48,19 @@ func TestDatabase_InsufficientStorage(t *testing.T) {
 
 	assert.True(t, errors.Is(err, status.ErrInsufficientStorage), "expected status.ErrInsufficientStorage, got %v", err)
 }
+
+func TestDatabase_StdoutBypassesStorageGate(t *testing.T) {
+	conf := get.Config()
+	disk.FlushFree()
+	t.Cleanup(disk.FlushFree)
+	disk.SetFree(conf.StoragePath(), 1, 1000)
+
+	// Stdout dumps must not be blocked by the storage gate so an operator can
+	// still offload a backup (for example over ssh) when the local volume is
+	// full. The actual mariadb-dump invocation will fail in this minimal
+	// fixture, but the error must not be status.ErrInsufficientStorage.
+	err := Database("", "-", true, true, 0)
+
+	assert.False(t, errors.Is(err, status.ErrInsufficientStorage),
+		"stdout backups must bypass the storage gate, got %v", err)
+}