diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 7bbce06..1e03be6 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -903,51 +903,98 @@ func (h *Handlers) addKeyValueToApp( http.Redirect(writer, request, "/apps/"+application.ID, http.StatusSeeOther) } -// HandleEnvVarAdd handles adding an environment variable. -func (h *Handlers) HandleEnvVarAdd() http.HandlerFunc { +// HandleEnvVarSave handles bulk saving of all environment variables. +// It deletes all existing env vars for the app and inserts the full +// submitted set (monolithic list approach). +func (h *Handlers) HandleEnvVarSave() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { - h.addKeyValueToApp( + appID := chi.URLParam(request, "id") + + application, findErr := models.FindApp(request.Context(), h.db, appID) + if findErr != nil || application == nil { + http.NotFound(writer, request) + + return + } + + parseErr := request.ParseForm() + if parseErr != nil { + http.Error(writer, "Bad Request", http.StatusBadRequest) + + return + } + + pairs := parseEnvPairs(request.FormValue("env_vars")) + ctx := request.Context() + + // 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, + ) + + return + } + + // 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 + + saveErr := envVar.Save(ctx) + if saveErr != nil { + h.log.Error( + "failed to save env var", + "key", p.key, + "error", saveErr, + ) + } + } + + http.Redirect( writer, request, - func(ctx context.Context, application *models.App, key, value string) error { - envVar := models.NewEnvVar(h.db) - envVar.AppID = application.ID - envVar.Key = key - envVar.Value = value - - return envVar.Save(ctx) - }, + "/apps/"+appID+"?success=env-updated", + http.StatusSeeOther, ) } } -// HandleEnvVarDelete handles deleting an environment variable. -func (h *Handlers) HandleEnvVarDelete() http.HandlerFunc { - return func(writer http.ResponseWriter, request *http.Request) { - appID := chi.URLParam(request, "id") - envVarIDStr := chi.URLParam(request, "varID") +// envPair holds a parsed key-value pair from the env vars textarea. +type envPair struct { + key string + value string +} - envVarID, parseErr := strconv.ParseInt(envVarIDStr, 10, 64) - if parseErr != nil { - http.NotFound(writer, request) +// parseEnvPairs parses KEY=VALUE lines from a textarea string. +// Blank lines and lines starting with # are skipped. +func parseEnvPairs(text string) []envPair { + var pairs []envPair - return + for line := range strings.SplitSeq(text, "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue } - envVar, findErr := models.FindEnvVar(request.Context(), h.db, envVarID) - if findErr != nil || envVar == nil || envVar.AppID != appID { - http.NotFound(writer, request) - - return + key, value, ok := strings.Cut(line, "=") + if !ok || strings.TrimSpace(key) == "" { + continue } - deleteErr := envVar.Delete(request.Context()) - if deleteErr != nil { - h.log.Error("failed to delete env var", "error", deleteErr) - } - - http.Redirect(writer, request, "/apps/"+appID, http.StatusSeeOther) + pairs = append(pairs, envPair{ + key: strings.TrimSpace(key), + value: value, + }) } + + return pairs } // HandleLabelAdd handles adding a label. @@ -1205,59 +1252,6 @@ func ValidateVolumePath(p string) error { return nil } -// HandleEnvVarEdit handles editing an existing environment variable. -func (h *Handlers) HandleEnvVarEdit() http.HandlerFunc { - return func(writer http.ResponseWriter, request *http.Request) { - appID := chi.URLParam(request, "id") - envVarIDStr := chi.URLParam(request, "varID") - - envVarID, parseErr := strconv.ParseInt(envVarIDStr, 10, 64) - if parseErr != nil { - http.NotFound(writer, request) - - return - } - - envVar, findErr := models.FindEnvVar(request.Context(), h.db, envVarID) - if findErr != nil || envVar == nil || envVar.AppID != appID { - http.NotFound(writer, request) - - return - } - - formErr := request.ParseForm() - if formErr != nil { - http.Error(writer, "Bad Request", http.StatusBadRequest) - - return - } - - key := request.FormValue("key") - value := request.FormValue("value") - - if key == "" || value == "" { - http.Redirect(writer, request, "/apps/"+appID, http.StatusSeeOther) - - return - } - - envVar.Key = key - envVar.Value = value - - saveErr := envVar.Save(request.Context()) - if saveErr != nil { - h.log.Error("failed to update env var", "error", saveErr) - } - - http.Redirect( - writer, - request, - "/apps/"+appID+"?success=env-updated", - http.StatusSeeOther, - ) - } -} - // HandleLabelEdit handles editing an existing label. func (h *Handlers) HandleLabelEdit() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 412935a..86d1323 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -560,45 +560,89 @@ func testOwnershipVerification(t *testing.T, cfg ownedResourceTestConfig) { cfg.verifyFn(t, testCtx, resourceID) } -// TestDeleteEnvVarOwnershipVerification tests that deleting an env var -// via another app's URL path returns 404 (IDOR prevention). -func TestDeleteEnvVarOwnershipVerification(t *testing.T) { //nolint:dupl // intentionally similar IDOR test pattern +// TestHandleEnvVarSaveBulk tests that HandleEnvVarSave replaces all env vars +// for an app with the submitted set (monolithic delete-all + insert-all). +func TestHandleEnvVarSaveBulk(t *testing.T) { t.Parallel() - testOwnershipVerification(t, ownedResourceTestConfig{ - appPrefix1: "envvar-owner-app", - appPrefix2: "envvar-other-app", - createFn: func(t *testing.T, tc *testContext, ownerApp *models.App) int64 { - t.Helper() + testCtx := setupTestHandlers(t) + createdApp := createTestApp(t, testCtx, "envvar-bulk-app") - envVar := models.NewEnvVar(tc.database) - envVar.AppID = ownerApp.ID - envVar.Key = "SECRET" - envVar.Value = "hunter2" - require.NoError(t, envVar.Save(context.Background())) + // Create some pre-existing env vars + for _, kv := range [][2]string{{"OLD_KEY", "old_value"}, {"REMOVE_ME", "gone"}} { + ev := models.NewEnvVar(testCtx.database) + ev.AppID = createdApp.ID + ev.Key = kv[0] + ev.Value = kv[1] + require.NoError(t, ev.Save(context.Background())) + } - return envVar.ID - }, - deletePath: func(appID string, resourceID int64) string { - return "/apps/" + appID + "/env/" + strconv.FormatInt(resourceID, 10) + "/delete" - }, - chiParams: func(appID string, resourceID int64) map[string]string { - return map[string]string{"id": appID, "varID": strconv.FormatInt(resourceID, 10)} - }, - handler: func(h *handlers.Handlers) http.HandlerFunc { return h.HandleEnvVarDelete() }, - verifyFn: func(t *testing.T, tc *testContext, resourceID int64) { - t.Helper() + // Submit a new set via textarea + form := url.Values{} + form.Set("env_vars", "NEW_KEY=new_value\nANOTHER=42\n# comment line\n") - found, findErr := models.FindEnvVar(context.Background(), tc.database, resourceID) - require.NoError(t, findErr) - assert.NotNil(t, found, "env var should still exist after IDOR attempt") - }, - }) + r := chi.NewRouter() + r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave()) + + request := httptest.NewRequest( + http.MethodPost, + "/apps/"+createdApp.ID+"/env", + strings.NewReader(form.Encode()), + ) + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + recorder := httptest.NewRecorder() + r.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusSeeOther, recorder.Code) + + // Verify old env vars are gone and new ones exist + envVars, err := models.FindEnvVarsByAppID( + context.Background(), testCtx.database, createdApp.ID, + ) + require.NoError(t, err) + assert.Len(t, envVars, 2) + + keys := make(map[string]string) + for _, ev := range envVars { + keys[ev.Key] = ev.Value + } + + assert.Equal(t, "new_value", keys["NEW_KEY"]) + assert.Equal(t, "42", keys["ANOTHER"]) + assert.Empty(t, keys["OLD_KEY"], "old env vars should be deleted") + assert.Empty(t, keys["REMOVE_ME"], "old env vars should be deleted") +} + +// TestHandleEnvVarSaveAppNotFound tests that HandleEnvVarSave returns 404 +// for a non-existent app. +func TestHandleEnvVarSaveAppNotFound(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + form := url.Values{} + form.Set("env_vars", "KEY=value\n") + + r := chi.NewRouter() + r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave()) + + request := httptest.NewRequest( + http.MethodPost, + "/apps/nonexistent-id/env", + strings.NewReader(form.Encode()), + ) + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + recorder := httptest.NewRecorder() + r.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusNotFound, recorder.Code) } // TestDeleteLabelOwnershipVerification tests that deleting a label // via another app's URL path returns 404 (IDOR prevention). -func TestDeleteLabelOwnershipVerification(t *testing.T) { //nolint:dupl // intentionally similar IDOR test pattern +func TestDeleteLabelOwnershipVerification(t *testing.T) { t.Parallel() testOwnershipVerification(t, ownedResourceTestConfig{ @@ -714,41 +758,46 @@ func TestDeletePortOwnershipVerification(t *testing.T) { assert.NotNil(t, found, "port should still exist after IDOR attempt") } -// TestHandleEnvVarDeleteUsesCorrectRouteParam verifies that HandleEnvVarDelete -// reads the "varID" chi URL parameter (matching the route definition {varID}), -// not a mismatched name like "envID". -func TestHandleEnvVarDeleteUsesCorrectRouteParam(t *testing.T) { +// TestHandleEnvVarSaveEmptyClears verifies that submitting an empty textarea +// deletes all existing env vars for the app. +func TestHandleEnvVarSaveEmptyClears(t *testing.T) { t.Parallel() testCtx := setupTestHandlers(t) + createdApp := createTestApp(t, testCtx, "envvar-clear-app") - createdApp := createTestApp(t, testCtx, "envdelete-param-app") + // Create a pre-existing env var + ev := models.NewEnvVar(testCtx.database) + ev.AppID = createdApp.ID + ev.Key = "DELETE_ME" + ev.Value = "gone" + require.NoError(t, ev.Save(context.Background())) - envVar := models.NewEnvVar(testCtx.database) - envVar.AppID = createdApp.ID - envVar.Key = "DELETE_ME" - envVar.Value = "gone" - require.NoError(t, envVar.Save(context.Background())) + // Submit empty textarea + form := url.Values{} + form.Set("env_vars", "") - // Use chi router with the real route pattern to test param name r := chi.NewRouter() - r.Post("/apps/{id}/env/{varID}/delete", testCtx.handlers.HandleEnvVarDelete()) + r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave()) request := httptest.NewRequest( http.MethodPost, - "/apps/"+createdApp.ID+"/env/"+strconv.FormatInt(envVar.ID, 10)+"/delete", - nil, + "/apps/"+createdApp.ID+"/env", + strings.NewReader(form.Encode()), ) - recorder := httptest.NewRecorder() + request.Header.Set("Content-Type", "application/x-www-form-urlencoded") + recorder := httptest.NewRecorder() r.ServeHTTP(recorder, request) assert.Equal(t, http.StatusSeeOther, recorder.Code) - // Verify the env var was actually deleted - found, findErr := models.FindEnvVar(context.Background(), testCtx.database, envVar.ID) - require.NoError(t, findErr) - assert.Nil(t, found, "env var should be deleted when using correct route param") + // Verify all env vars are gone + envVars, err := models.FindEnvVarsByAppID( + context.Background(), testCtx.database, createdApp.ID, + ) + require.NoError(t, err) + assert.Empty(t, envVars, "all env vars should be deleted") } // TestHandleVolumeAddValidatesPaths verifies that HandleVolumeAdd validates diff --git a/internal/server/routes.go b/internal/server/routes.go index b4f5d8c..ebddba9 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -82,10 +82,8 @@ func (s *Server) SetupRoutes() { r.Post("/apps/{id}/stop", s.handlers.HandleAppStop()) r.Post("/apps/{id}/start", s.handlers.HandleAppStart()) - // Environment variables - r.Post("/apps/{id}/env", s.handlers.HandleEnvVarAdd()) - r.Post("/apps/{id}/env/{varID}/edit", s.handlers.HandleEnvVarEdit()) - r.Post("/apps/{id}/env/{varID}/delete", s.handlers.HandleEnvVarDelete()) + // Environment variables (monolithic bulk save) + r.Post("/apps/{id}/env", s.handlers.HandleEnvVarSave()) // Labels r.Post("/apps/{id}/labels", s.handlers.HandleLabelAdd()) diff --git a/templates/app_detail.html b/templates/app_detail.html index 7a6e106..63cbb45 100644 --- a/templates/app_detail.html +++ b/templates/app_detail.html @@ -103,57 +103,14 @@

Environment Variables

- {{if .EnvVars}} -
- - - - - - - - - - {{range .EnvVars}} - - - - - - - {{end}} - -
KeyValueActions
-
- {{end}} -
- {{ .CSRFField }} - - - + + {{ .CSRFField }} +

One KEY=value per line. Lines starting with # are ignored.

+ +
+ +

⚠ Container restart needed after env var changes.

+