diff --git a/internal/handlers/app.go b/internal/handlers/app.go index c42b8d9..9064951 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -912,7 +912,7 @@ type envPairJSON struct { // HandleEnvVarSave handles bulk saving of all environment variables. // It reads a JSON array of {key, value} objects from the request body, // deletes all existing env vars for the app, and inserts the full -// submitted set. +// submitted set atomically within a database transaction. func (h *Handlers) HandleEnvVarSave() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { appID := chi.URLParam(request, "id") @@ -933,36 +933,36 @@ func (h *Handlers) HandleEnvVarSave() http.HandlerFunc { return } - ctx := request.Context() + // Validate: reject entries with empty keys + var modelPairs []models.EnvVarPair - // Delete all existing env vars for this app - deleteErr := models.DeleteEnvVarsByAppID(ctx, h.db, application.ID) - if deleteErr != nil { - h.log.Error("failed to delete env vars", "error", deleteErr) - http.Error( - writer, - "Internal Server Error", - http.StatusInternalServerError, - ) + for _, p := range pairs { + trimmedKey := strings.TrimSpace(p.Key) + if trimmedKey == "" { + h.respondJSON(writer, request, map[string]string{ + "error": "empty environment variable key is not allowed", + }, http.StatusBadRequest) - return + return + } + + modelPairs = append(modelPairs, models.EnvVarPair{ + Key: trimmedKey, + Value: p.Value, + }) } - // Insert the full new set - for _, p := range pairs { - envVar := models.NewEnvVar(h.db) - envVar.AppID = application.ID - envVar.Key = p.Key - envVar.Value = p.Value + // Atomically replace all env vars in a transaction + ctx := request.Context() - saveErr := envVar.Save(ctx) - if saveErr != nil { - h.log.Error( - "failed to save env var", - "key", p.Key, - "error", saveErr, - ) - } + replaceErr := models.ReplaceEnvVarsByAppID(ctx, h.db, application.ID, modelPairs) + if replaceErr != nil { + h.log.Error("failed to replace env vars", "error", replaceErr) + h.respondJSON(writer, request, map[string]string{ + "error": "failed to save environment variables", + }, http.StatusInternalServerError) + + return } h.respondJSON(writer, request, map[string]bool{"ok": true}, http.StatusOK) diff --git a/internal/models/env_var.go b/internal/models/env_var.go index 6e5221c..e800cc8 100644 --- a/internal/models/env_var.go +++ b/internal/models/env_var.go @@ -1,4 +1,3 @@ -//nolint:dupl // Active Record pattern - similar structure to label.go is intentional package models import ( @@ -139,3 +138,49 @@ func DeleteEnvVarsByAppID( return err } + +// EnvVarPair is a key-value pair for bulk env var operations. +type EnvVarPair struct { + Key string + Value string +} + +// ReplaceEnvVarsByAppID atomically replaces all env vars for an app +// within a single database transaction. It deletes all existing env +// vars and inserts the provided pairs. If any operation fails, the +// entire transaction is rolled back. +func ReplaceEnvVarsByAppID( + ctx context.Context, + db *database.Database, + appID string, + pairs []EnvVarPair, +) error { + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("beginning transaction: %w", err) + } + + defer func() { _ = tx.Rollback() }() + + _, err = tx.ExecContext(ctx, "DELETE FROM app_env_vars WHERE app_id = ?", appID) + if err != nil { + return fmt.Errorf("deleting env vars: %w", err) + } + + for _, p := range pairs { + _, err = tx.ExecContext(ctx, + "INSERT INTO app_env_vars (app_id, key, value) VALUES (?, ?, ?)", + appID, p.Key, p.Value, + ) + if err != nil { + return fmt.Errorf("inserting env var %q: %w", p.Key, err) + } + } + + err = tx.Commit() + if err != nil { + return fmt.Errorf("committing transaction: %w", err) + } + + return nil +} diff --git a/internal/models/label.go b/internal/models/label.go index e8aadcd..6910421 100644 --- a/internal/models/label.go +++ b/internal/models/label.go @@ -1,4 +1,3 @@ -//nolint:dupl // Active Record pattern - similar structure to env_var.go is intentional package models import ( diff --git a/static/js/app-detail.js b/static/js/app-detail.js index 60f6ded..eec57c5 100644 --- a/static/js/app-detail.js +++ b/static/js/app-detail.js @@ -73,11 +73,30 @@ document.addEventListener("alpine:init", () => { body: JSON.stringify( this.vars.map((e) => ({ key: e.key, value: e.value })), ), - }).then((res) => { - if (res.ok) { - window.location.reload(); - } - }); + }) + .then((res) => { + if (res.ok) { + window.location.reload(); + return; + } + res.json() + .then((data) => { + window.alert( + data.error || + "Failed to save environment variables.", + ); + }) + .catch(() => { + window.alert( + "Failed to save environment variables.", + ); + }); + }) + .catch(() => { + window.alert( + "Network error: could not save environment variables.", + ); + }); }, }));