fix: reject negative process counts when scaling

A process count below zero was accepted and stored verbatim, leaving the scheduler to reject the value long after the command reported success. Counts below zero are now rejected, as are process tuples that omit a process type, across `ps:scale`, the `ps-set-scale` trigger and the `formation` key of `app.json`. A negative count persisted by an earlier version stays readable so it can be corrected with `ps:scale`.

docs/appendices/file-formats/app-json.md

@@ -158,7 +158,7 @@ On subsequent deploys:
 
 - `autoscaling` (map of string to object, optional) autoscaling rules. See the autoscaling section for more details
 - `max_parallel`: (int, optional) number of instances to deploy in parallel at a given time
-- `quantity`: (int, optional) number of processes to maintain. Default 1 for web processes, 0 for all others.
+- `quantity`: (int, optional) number of processes to maintain. Must be zero or greater. Default 1 for web processes, 0 for all others.
 - `service`: (map of string to oject, optional) governs how non-web processes are exposed as services on the network
 
 ### Autoscaling

docs/development/plugin-triggers.md

@@ -2396,7 +2396,7 @@ set -eo pipefail; [[ $DOKKU_TRACE ]] && set -x
 
 ### `ps-set-scale`
 
-- Description: Sets the scale for an app based on a specified formation (process-type=quantity). Any unspecified process types will be left as is, or have their process count set to zero when `$CLEAR_EXISTING` is `true`.
+- Description: Sets the scale for an app based on a specified formation (process-type=quantity). Any unspecified process types will be left as is, or have their process count set to zero when `$CLEAR_EXISTING` is `true`. Each process tuple must name a process type, and process counts must be zero or greater.
 - Invoked by:
 - Arguments: `$APP $SKIP_DEPLOY $CLEAR_EXISTING [$PROCESS_TUPLE...]`
 - Example:

docs/processes/process-management.md

@@ -248,6 +248,9 @@ Multiple process types can be scaled at once:
 dokku ps:scale node-js-app web=1 worker=1
 ```
 
+> [!NOTE]
+> Each argument must name a process type, and process counts must be zero or greater. Scaling a process type to `0` stops all of its containers, while a negative count is rejected.
+
 If desired, the corresponding deploy will be skipped by using the `--skip-deploy` flag:
 
 ```shell

plugins/app-json/functions.go

@@ -431,6 +431,10 @@ func setScale(appName string) error {
 	args := []string{appName, strconv.FormatBool(skipDeploy), strconv.FormatBool(clearExisting)}
 	for processType, formation := range appJSON.Formation {
 		if formation.Quantity != nil {
+			if *formation.Quantity < 0 {
+				return fmt.Errorf("Invalid quantity for process type %s in app.json: value must be zero or greater", processType)
+			}
+
 			args = append(args, fmt.Sprintf("%s=%d", processType, *formation.Quantity))
 		}
 	}

plugins/ps/functions.go

@@ -196,7 +196,7 @@ func parseProcessTuples(processTuples []string) (FormationSlice, error) {
 		processType := strings.TrimSpace(s[0])
 		quantity, err := strconv.Atoi(strings.TrimSpace(s[1]))
 		if err != nil {
-			return formations, fmt.Errorf("Invalid count for process type %s", s[0])
+			return formations, fmt.Errorf("Invalid count for process type %s", processType)
 		}
 
 		if foundFormations[processType] {
@@ -351,6 +351,10 @@ func scaleSet(input scaleSetInput) error {
 		return err
 	}
 
+	if err := validateFormations(formations); err != nil {
+		return err
+	}
+
 	if input.clearExisting {
 		formations = appendClearedFormations(formations, existingFormations)
 	}
@@ -484,3 +488,20 @@ func updateScale(appName string, clearExisting bool, formationUpdates FormationS
 
 	return common.PropertyListWrite("ps", appName, "scale", values)
 }
+
+// validateFormations returns an error when a user specified formation cannot be scaled to.
+// Formations read back from the scale property are deliberately not validated so that a value
+// persisted by an older dokku version remains visible and repairable via ps:scale.
+func validateFormations(formations FormationSlice) error {
+	for _, formation := range formations {
+		if formation.ProcessType == "" {
+			return fmt.Errorf("Missing process type for count %d", formation.Quantity)
+		}
+
+		if formation.Quantity < 0 {
+			return fmt.Errorf("Invalid count for process type %s: value must be zero or greater", formation.ProcessType)
+		}
+	}
+
+	return nil
+}

plugins/ps/functions_test.go

@@ -90,3 +90,160 @@ func TestDefaultProcessTuplesWithoutProcfile(t *testing.T) {
 		t.Fatalf("expected [web=1], got %v", processTuples)
 	}
 }
+
+func TestParseProcessTuples(t *testing.T) {
+	testCases := []struct {
+		name          string
+		processTuples []string
+		expected      []Formation
+		expectedError string
+	}{
+		{
+			name:          "no process tuples",
+			processTuples: []string{},
+			expected:      []Formation{},
+		},
+		{
+			name:          "single process type",
+			processTuples: []string{"web=1"},
+			expected:      []Formation{{ProcessType: "web", Quantity: 1}},
+		},
+		{
+			name:          "multiple process types",
+			processTuples: []string{"web=1", "worker=2"},
+			expected:      []Formation{{ProcessType: "web", Quantity: 1}, {ProcessType: "worker", Quantity: 2}},
+		},
+		{
+			name:          "trims surrounding whitespace",
+			processTuples: []string{" web = 2 "},
+			expected:      []Formation{{ProcessType: "web", Quantity: 2}},
+		},
+		{
+			name:          "first specified quantity wins",
+			processTuples: []string{"web=1", "web=3"},
+			expected:      []Formation{{ProcessType: "web", Quantity: 1}},
+		},
+		{
+			// persisted formations are parsed without validation so that a negative
+			// quantity written by an older dokku version remains readable
+			name:          "negative quantities are parsed",
+			processTuples: []string{"web=-1"},
+			expected:      []Formation{{ProcessType: "web", Quantity: -1}},
+		},
+		{
+			name:          "missing count",
+			processTuples: []string{"web"},
+			expectedError: "Missing count for process type web",
+		},
+		{
+			name:          "non-numeric count",
+			processTuples: []string{"web=one"},
+			expectedError: "Invalid count for process type web",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			formations, err := parseProcessTuples(testCase.processTuples)
+			if testCase.expectedError != "" {
+				if err == nil {
+					t.Fatalf("expected error %q, got none", testCase.expectedError)
+				}
+
+				if err.Error() != testCase.expectedError {
+					t.Fatalf("expected error %q, got %q", testCase.expectedError, err.Error())
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("unexpected error parsing process tuples: %v", err)
+			}
+
+			assertFormations(t, formations, testCase.expected)
+		})
+	}
+}
+
+func TestValidateFormations(t *testing.T) {
+	testCases := []struct {
+		name          string
+		processTuples []string
+		expectedError string
+	}{
+		{
+			name:          "zero is a valid quantity",
+			processTuples: []string{"web=0"},
+		},
+		{
+			name:          "positive quantities are valid",
+			processTuples: []string{"web=1", "worker=10"},
+		},
+		{
+			name:          "negative quantity",
+			processTuples: []string{"web=-1"},
+			expectedError: "Invalid count for process type web: value must be zero or greater",
+		},
+		{
+			name:          "names the offending process type",
+			processTuples: []string{"web=1", "worker=-2"},
+			expectedError: "Invalid count for process type worker: value must be zero or greater",
+		},
+		{
+			name:          "missing process type",
+			processTuples: []string{"=1"},
+			expectedError: "Missing process type for count 1",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			formations, err := parseProcessTuples(testCase.processTuples)
+			if err != nil {
+				t.Fatalf("unexpected error parsing process tuples: %v", err)
+			}
+
+			err = validateFormations(formations)
+			if testCase.expectedError == "" {
+				if err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
+
+				return
+			}
+
+			if err == nil {
+				t.Fatalf("expected error %q, got none", testCase.expectedError)
+			}
+
+			if err.Error() != testCase.expectedError {
+				t.Fatalf("expected error %q, got %q", testCase.expectedError, err.Error())
+			}
+		})
+	}
+}
+
+func TestScaleSetRejectsNegativeQuantity(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "ps-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	t.Setenv("DOKKU_LIB_ROOT", tmpDir)
+	t.Setenv("DOKKU_PID", "")
+
+	err = scaleSet(scaleSetInput{
+		appName:       "test-app",
+		skipDeploy:    true,
+		processTuples: []string{"web=-1"},
+	})
+	if err == nil {
+		t.Fatal("expected an error scaling to a negative quantity, got none")
+	}
+
+	if err.Error() != "Invalid count for process type web: value must be zero or greater" {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}

tests/unit/app-json-3.bats

@@ -58,6 +58,23 @@ teardown() {
   assert_output_contains "cron: 1" 1
 }
 
+@test "(app-json) negative formation quantity" {
+  mkdir -p /var/lib/dokku/data/app-json/$TEST_APP
+  negative_formation_callback "$TEST_APP" /var/lib/dokku/data/app-json/$TEST_APP
+
+  run_plugn_trigger post-release-builder herokuish $TEST_APP dokku/$TEST_APP:latest
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Invalid quantity for process type web in app.json: value must be zero or greater"
+
+  run /bin/bash -c "dokku --quiet ps:scale $TEST_APP"
+  output=$(echo "$output" | tr -s " ")
+  echo "output: ($output)"
+  echo "status: $status"
+  assert_output "web: 1"
+}
+
 persist_scale_callback_a() {
   local APP="$1"
   local APP_REPO_DIR="$2"
@@ -81,3 +98,19 @@ persist_scale_callback_b() {
   echo "cron: python3 worker.py" >>"$APP_REPO_DIR/Procfile"
   mv "$APP_REPO_DIR/app-5205b.json" "$APP_REPO_DIR/app.json"
 }
+
+negative_formation_callback() {
+  local APP="$1"
+  local APP_REPO_DIR="$2"
+  [[ -z "$APP" ]] && local APP="$TEST_APP"
+
+  cat >"$APP_REPO_DIR/app.json" <<EOF
+{
+  "formation": {
+    "web": {
+      "quantity": -1
+    }
+  }
+}
+EOF
+}

tests/unit/ps-general-1.bats

@@ -357,6 +357,77 @@ worker=1" >/var/lib/dokku/config/ps/$TEST_APP/scale
   assert_output_contains "The --clear and --replace flags cannot be specified together"
 }
 
+@test "(ps:scale) invalid process counts" {
+  run /bin/bash -c "dokku ps:scale $TEST_APP web=-1"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Invalid count for process type web: value must be zero or greater"
+
+  run /bin/bash -c "dokku ps:scale --replace $TEST_APP web=1 worker=-2"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Invalid count for process type worker: value must be zero or greater"
+
+  run /bin/bash -c "dokku ps:scale $TEST_APP =1"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Missing process type for count 1"
+
+  run_plugn_trigger ps-set-scale $TEST_APP true false web=-1
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+  assert_output_contains "Invalid count for process type web: value must be zero or greater"
+
+  run /bin/bash -c "dokku --quiet ps:scale $TEST_APP"
+  output=$(echo "$output" | tr -s " ")
+  echo "output: ($output)"
+  echo "status: $status"
+  assert_output "web: 1"
+
+  run /bin/bash -c "dokku ps:scale --skip-deploy $TEST_APP web=0"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet ps:scale $TEST_APP"
+  output=$(echo "$output" | tr -s " ")
+  echo "output: ($output)"
+  echo "status: $status"
+  assert_output "web: 0"
+}
+
+@test "(ps:scale) repairing a negative process count" {
+  echo "web=-1
+worker=2" >/var/lib/dokku/config/ps/$TEST_APP/scale
+
+  run /bin/bash -c "dokku --quiet ps:scale $TEST_APP"
+  output=$(echo "$output" | tr -s " ")
+  echo "output: ($output)"
+  echo "status: $status"
+  assert_success
+  assert_output $'web: -1\nworker: 2'
+
+  run_plugn_trigger ps-current-scale $TEST_APP
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku ps:scale --skip-deploy $TEST_APP web=1"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+
+  run /bin/bash -c "dokku --quiet ps:scale $TEST_APP"
+  output=$(echo "$output" | tr -s " ")
+  echo "output: ($output)"
+  echo "status: $status"
+  assert_output $'web: 1\nworker: 2'
+}
+
 @test "(ps) handle windows newlines in procfile" {
   run deploy_app python dokku@$DOKKU_DOMAIN:$TEST_APP procfile_line_endings_to_windows
   echo "output: $output"