diff --git a/internal/handlers/app.go b/internal/handlers/app.go index b10de59..d55c27c 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -912,13 +912,12 @@ type envPairJSON struct { // envVarMaxBodyBytes is the maximum allowed request body size for env var saves (1 MB). const envVarMaxBodyBytes = 1 << 20 -// deduplicateEnvPairs validates and deduplicates env var pairs. -// It rejects empty keys (returns a non-empty error string) and -// deduplicates by key, keeping the last occurrence. -func deduplicateEnvPairs(pairs []envPairJSON) ([]models.EnvVarPair, string) { - seen := make(map[string]int, len(pairs)) +// validateEnvPairs validates env var pairs. +// It rejects empty keys and duplicate keys (returns a non-empty error string). +func validateEnvPairs(pairs []envPairJSON) ([]models.EnvVarPair, string) { + seen := make(map[string]bool, len(pairs)) - var result []models.EnvVarPair + result := make([]models.EnvVarPair, 0, len(pairs)) for _, p := range pairs { trimmedKey := strings.TrimSpace(p.Key) @@ -926,12 +925,13 @@ func deduplicateEnvPairs(pairs []envPairJSON) ([]models.EnvVarPair, string) { return nil, "empty environment variable key is not allowed" } - if idx, exists := seen[trimmedKey]; exists { - result[idx] = models.EnvVarPair{Key: trimmedKey, Value: p.Value} - } else { - seen[trimmedKey] = len(result) - result = append(result, models.EnvVarPair{Key: trimmedKey, Value: p.Value}) + if seen[trimmedKey] { + return nil, "duplicate environment variable key: " + trimmedKey } + + seen[trimmedKey] = true + + result = append(result, models.EnvVarPair{Key: trimmedKey, Value: p.Value}) } return result, "" @@ -941,7 +941,7 @@ func deduplicateEnvPairs(pairs []envPairJSON) ([]models.EnvVarPair, string) { // 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 atomically within a database transaction. -// Duplicate keys are deduplicated server-side (last occurrence wins). +// Duplicate keys are rejected with a 400 Bad Request error. func (h *Handlers) HandleEnvVarSave() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { appID := chi.URLParam(request, "id") @@ -967,7 +967,7 @@ func (h *Handlers) HandleEnvVarSave() http.HandlerFunc { return } - modelPairs, validationErr := deduplicateEnvPairs(pairs) + modelPairs, validationErr := validateEnvPairs(pairs) if validationErr != "" { h.respondJSON(writer, request, map[string]string{ "error": validationErr, diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 178d200..6a7d0b3 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -664,15 +664,15 @@ func TestHandleEnvVarSaveEmptyKeyRejected(t *testing.T) { assert.Equal(t, http.StatusBadRequest, recorder.Code) } -// TestHandleEnvVarSaveDuplicateKeyDedup verifies that when the client -// sends duplicate keys, the server deduplicates them (last wins). -func TestHandleEnvVarSaveDuplicateKeyDedup(t *testing.T) { +// TestHandleEnvVarSaveDuplicateKeyRejected verifies that when the client +// sends duplicate keys, the server rejects them with 400 Bad Request. +func TestHandleEnvVarSaveDuplicateKeyRejected(t *testing.T) { t.Parallel() testCtx := setupTestHandlers(t) createdApp := createTestApp(t, testCtx, "envvar-dedup-app") - // Send two entries with the same key — last should win + // Send two entries with the same key — should be rejected body := `[{"key":"FOO","value":"first"},{"key":"BAR","value":"bar"},{"key":"FOO","value":"second"}]` r := chi.NewRouter() @@ -688,21 +688,8 @@ func TestHandleEnvVarSaveDuplicateKeyDedup(t *testing.T) { recorder := httptest.NewRecorder() r.ServeHTTP(recorder, request) - assert.Equal(t, http.StatusOK, recorder.Code) - - envVars, err := models.FindEnvVarsByAppID( - context.Background(), testCtx.database, createdApp.ID, - ) - require.NoError(t, err) - assert.Len(t, envVars, 2, "duplicate key should be deduplicated") - - keys := make(map[string]string) - for _, ev := range envVars { - keys[ev.Key] = ev.Value - } - - assert.Equal(t, "second", keys["FOO"], "last occurrence should win") - assert.Equal(t, "bar", keys["BAR"]) + assert.Equal(t, http.StatusBadRequest, recorder.Code) + assert.Contains(t, recorder.Body.String(), "duplicate environment variable key: FOO") } // TestHandleEnvVarSaveCrossAppIsolation verifies that posting env vars