Raise legacy PHPStan to level 8 (+ type-bug fixes)

integration-tests/activity_monitor_null_timestamp_test.go

@@ -0,0 +1,120 @@
+package tests
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/upsun/cli/pkg/mockapi"
+)
+
+// TestActivityMonitorNullTimestamp drives `environment:redeploy` against a
+// response whose embedded activities have null started_at and created_at.
+// This exercises Platformsh\Cli\Service\ActivityMonitor::getStart() (line 677)
+// and the strtotime() call in waitMultiple() (line 481), both of which feed
+// $activity->created_at directly to strtotime() without a null guard. The same
+// shape of bug was previously fixed in Model\Activity (commit 915a95af) but
+// these two sites in ActivityMonitor were missed.
+//
+// To skip the `count(nonIntegrationActivities) === 1` short-circuit in
+// waitMultiple() (which would route through waitAndLog() and getLogStream(),
+// requiring more mocking), the redeploy response embeds two non-integration
+// activities, so the multi-activity branch (lines 449-515) runs.
+func TestActivityMonitorNullTimestamp(t *testing.T) {
+	authServer := mockapi.NewAuthServer(t)
+	defer authServer.Close()
+
+	apiHandler := mockapi.NewHandler(t)
+
+	projectID := mockapi.ProjectID()
+
+	apiHandler.SetProjects([]*mockapi.Project{{
+		ID: projectID,
+		Links: mockapi.MakeHALLinks(
+			"self=/projects/"+projectID,
+			"environments=/projects/"+projectID+"/environments",
+		),
+		DefaultBranch: "main",
+	}})
+
+	main := makeEnv(projectID, "main", "production", "active", nil)
+	main.Links["#redeploy"] = mockapi.HALLink{
+		HREF: "/projects/" + projectID + "/environments/main/redeploy",
+	}
+	main.Links["#activities"] = mockapi.HALLink{
+		HREF: "/projects/" + projectID + "/environments/main/activities",
+	}
+	apiHandler.SetEnvironments([]*mockapi.Environment{main})
+
+	// Two complete non-integration activities with null timestamps. The pair
+	// avoids the count(nonIntegration)===1 short-circuit and forces the
+	// multi-activity progress-bar branch in waitMultiple(), which calls
+	// getStart() (line 456) and strtotime($activity->created_at) (line 481).
+	// completion_percent=100 lets the wait loop exit on the first iteration.
+	activityJSON := func(id string) string {
+		return `{
+			"id": "` + id + `",
+			"type": "environment.redeploy",
+			"state": "complete",
+			"result": "success",
+			"completion_percent": 100,
+			"completed_at": null,
+			"started_at": null,
+			"created_at": null,
+			"updated_at": null,
+			"project": "` + projectID + `",
+			"environments": ["main"],
+			"description": "Redeploy with null timestamps",
+			"text": "Redeploy with null timestamps",
+			"payload": {}
+		}`
+	}
+	redeployResponse := `{
+		"_embedded": {
+			"activities": [` + activityJSON("actA") + `,` + activityJSON("actB") + `]
+		}
+	}`
+	// Project-level activities endpoint, queried by waitMultiple() to refresh
+	// activity state on each poll (line 495).
+	projectActivitiesJSON := "[" + activityJSON("actA") + "," + activityJSON("actB") + "]"
+
+	redeployPath := "/projects/" + projectID + "/environments/main/redeploy"
+	projectActivitiesPath := "/projects/" + projectID + "/activities"
+
+	wrapped := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodPost && r.URL.Path == redeployPath {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(redeployResponse))
+			return
+		}
+		if r.Method == http.MethodGet && r.URL.Path == projectActivitiesPath {
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(projectActivitiesJSON))
+			return
+		}
+		apiHandler.ServeHTTP(w, r)
+	})
+
+	apiServer := httptest.NewServer(wrapped)
+	defer apiServer.Close()
+
+	f := newCommandFactory(t, apiServer.URL, authServer.URL)
+
+	stdout, stderr, err := f.RunCombinedOutput(
+		"environment:redeploy", "-p", projectID, "-e", "main", "-y", "--wait",
+	)
+	t.Logf("environment:redeploy err=%v\nstdout=%s\nstderr=%s", err, stdout, stderr)
+
+	// Surface TypeError / fatal errors loudly.
+	if strings.Contains(stderr, "TypeError") ||
+		strings.Contains(stderr, "must be of type string") ||
+		strings.Contains(stderr, "Fatal error") ||
+		strings.Contains(stderr, "Uncaught") {
+		t.Fatalf("legacy CLI raised a PHP error on null timestamp:\n%s", stderr)
+	}
+
+	if err != nil {
+		t.Fatalf("environment:redeploy failed unexpectedly: %v\nstderr=%s", err, stderr)
+	}
+}

integration-tests/activity_null_timestamp_test.go

@@ -0,0 +1,126 @@
+package tests
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/upsun/cli/pkg/mockapi"
+)
+
+// TestActivityNullTimestamp checks whether the legacy PHP code crashes when an
+// API activity is returned with null completed_at / updated_at / created_at /
+// started_at fields. PHPStan level 8 flags strtotime() calls on those nullable
+// properties in legacy/src/Model/Activity.php, and new DateTime() on
+// $activity->created_at in legacy/src/Service/ActivityLoader.php.
+//
+// We bypass mockapi's typed Activity model (which would always emit valid
+// timestamps) by wrapping the handler and serving raw JSON for the activities
+// endpoints.
+func TestActivityNullTimestamp(t *testing.T) {
+	authServer := mockapi.NewAuthServer(t)
+	defer authServer.Close()
+
+	apiHandler := mockapi.NewHandler(t)
+
+	projectID := mockapi.ProjectID()
+
+	apiHandler.SetProjects([]*mockapi.Project{{
+		ID: projectID,
+		Links: mockapi.MakeHALLinks(
+			"self=/projects/"+projectID,
+			"environments=/projects/"+projectID+"/environments",
+		),
+		DefaultBranch: "main",
+	}})
+
+	main := makeEnv(projectID, "main", "production", "active", nil)
+	main.Links["#activities"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/activities"}
+	apiHandler.SetEnvironments([]*mockapi.Environment{main})
+
+	// Raw JSON for a single "complete" activity (completion_percent=100, so
+	// isComplete() returns true) with completed_at, updated_at, created_at and
+	// started_at all literal null. This is the worst case for the PHPStan-flagged
+	// strtotime() and new DateTime() calls.
+	activityJSON := `{
+		"id": "actnull",
+		"type": "environment.variable.create",
+		"state": "complete",
+		"result": "success",
+		"completion_percent": 100,
+		"completed_at": null,
+		"started_at": null,
+		"created_at": null,
+		"updated_at": null,
+		"project": "` + projectID + `",
+		"environments": ["main"],
+		"description": "Activity with null timestamps",
+		"text": "Activity with null timestamps",
+		"payload": {}
+	}`
+	listJSON := "[" + activityJSON + "]"
+
+	listPath := "/projects/" + projectID + "/environments/main/activities"
+	getPath := "/projects/" + projectID + "/activities/actnull"
+	getEnvPath := "/projects/" + projectID + "/environments/main/activities/actnull"
+
+	wrapped := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case listPath:
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(listJSON))
+			return
+		case getPath, getEnvPath:
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(activityJSON))
+			return
+		}
+		apiHandler.ServeHTTP(w, r)
+	})
+
+	apiServer := httptest.NewServer(wrapped)
+	defer apiServer.Close()
+
+	f := newCommandFactory(t, apiServer.URL, authServer.URL)
+
+	// Drive activity:list. This exercises ActivityLoader::load() which calls
+	// new DateTime($activity->created_at) on line 130 when paginating.
+	stdout, stderr, err := f.RunCombinedOutput("activity:list", "-p", projectID, "-e", "main", "--format", "plain")
+	t.Logf("activity:list err=%v\nstdout=%s\nstderr=%s", err, stdout, stderr)
+
+	// Drive activity:get. This exercises Model\Activity::getDuration() which
+	// calls strtotime() on completed_at / updated_at / created_at (lines 17, 24,
+	// 26).
+	stdout2, stderr2, err2 := f.RunCombinedOutput("activity:get", "-p", projectID, "-e", "main", "actnull")
+	t.Logf("activity:get err=%v\nstdout=%s\nstderr=%s", err2, stdout2, stderr2)
+
+	// Also explicitly request the duration property so getDuration() is invoked
+	// even if the table path skips it.
+	stdout3, stderr3, err3 := f.RunCombinedOutput(
+		"activity:get", "-p", projectID, "-e", "main", "actnull", "-P", "duration",
+	)
+	t.Logf("activity:get -P duration err=%v\nstdout=%s\nstderr=%s", err3, stdout3, stderr3)
+
+	// Surface TypeError / fatal errors loudly.
+	for _, out := range []string{stderr, stderr2, stderr3} {
+		if strings.Contains(out, "TypeError") ||
+			strings.Contains(out, "must be of type string") ||
+			strings.Contains(out, "Fatal error") ||
+			strings.Contains(out, "Uncaught") {
+			t.Fatalf("legacy CLI raised a PHP error on null timestamp:\n%s", out)
+		}
+	}
+
+	// If we reach here without any error, the strtotime / new DateTime calls
+	// either tolerated null input or the null was sanitized before reaching them.
+	if err != nil {
+		t.Fatalf("activity:list failed unexpectedly: %v\nstderr=%s", err, stderr)
+	}
+	if err2 != nil {
+		t.Fatalf("activity:get failed unexpectedly: %v\nstderr=%s", err2, stderr2)
+	}
+	if err3 != nil {
+		t.Fatalf("activity:get -P duration failed unexpectedly: %v\nstderr=%s", err3, stderr3)
+	}
+}

integration-tests/autoscaling_missing_defaults_test.go

@@ -0,0 +1,109 @@
+package tests
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/upsun/cli/pkg/mockapi"
+)
+
+// TestAutoscalingSettingsSetMissingDefaults verifies that autoscaling:set
+// fails gracefully when the autoscaling-settings API response omits the
+// "defaults" key (or returns null for it). The pre-fix code unconditionally
+// dereferenced $defaults['instances']['max'] and called
+// getSupportedMetrics($defaults), which is typed array, so PHP raised a
+// TypeError on null.
+func TestAutoscalingSettingsSetMissingDefaults(t *testing.T) {
+	authServer := mockapi.NewAuthServer(t)
+	defer authServer.Close()
+
+	apiHandler := mockapi.NewHandler(t)
+
+	projectID := mockapi.ProjectID()
+
+	apiHandler.SetProjects([]*mockapi.Project{{
+		ID: projectID,
+		Links: mockapi.MakeHALLinks(
+			"self=/projects/"+projectID,
+			"environments=/projects/"+projectID+"/environments",
+		),
+		DefaultBranch: "main",
+	}})
+
+	main := makeEnv(projectID, "main", "production", "active", nil)
+	main.Links["#autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"}
+	main.Links["#manage-autoscaling"] = mockapi.HALLink{HREF: "/projects/" + projectID + "/environments/main/autoscaling"}
+	apiHandler.SetEnvironments([]*mockapi.Environment{main})
+
+	apiHandler.Get("/projects/"+projectID+"/capabilities", func(w http.ResponseWriter, _ *http.Request) {
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"autoscaling": map[string]any{
+				"enabled":                              true,
+				"supports_horizontal_scaling_services": false,
+			},
+		})
+	})
+
+	deploymentPath := "/projects/" + projectID + "/environments/main/deployments/current"
+	apiHandler.Get(deploymentPath, func(w http.ResponseWriter, _ *http.Request) {
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"webapps": map[string]any{
+				"app": map[string]any{
+					"name":           "app",
+					"type":           "golang:1.23",
+					"instance_count": 1,
+					"disk":           512,
+					"resources": map[string]any{
+						"profile_size": "0.1",
+					},
+				},
+			},
+			"services": map[string]any{},
+			"workers":  map[string]any{},
+			"routes":   map[string]any{},
+			"_links": mockapi.MakeHALLinks(
+				"self=" + deploymentPath,
+			),
+		})
+	})
+
+	// Autoscaling settings with no "defaults" key — the API payload is
+	// otherwise valid. The unfixed CLI assumed $defaults was always present.
+	autoscalingPath := "/projects/" + projectID + "/environments/main/autoscaling"
+	apiHandler.Get(autoscalingPath, func(w http.ResponseWriter, _ *http.Request) {
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"services": map[string]any{},
+			"_links": mockapi.MakeHALLinks(
+				"self=" + autoscalingPath,
+			),
+		})
+	})
+
+	apiServer := httptest.NewServer(apiHandler)
+	defer apiServer.Close()
+
+	f := newCommandFactory(t, apiServer.URL, authServer.URL)
+
+	stdout, stderr, err := f.RunCombinedOutput(
+		"autoscaling:set",
+		"-p", projectID,
+		"-e", "main",
+		"--service", "app",
+		"--metric", "cpu",
+		"--enabled", "true",
+		"--dry-run",
+	)
+
+	combined := stdout + "\n---\n" + stderr
+	assert.NotContains(t, combined, "TypeError", "stdout: %s\nstderr: %s", stdout, stderr)
+	assert.NotContains(t, combined, "must be of type array, null given")
+	assert.NotContains(t, combined, "Cannot access offset")
+	assert.NotContains(t, combined, "Fatal error", "stdout: %s\nstderr: %s", stdout, stderr)
+	// The CLI should exit non-zero with an actionable message.
+	assert.Error(t, err, "expected non-zero exit when defaults are missing")
+	assert.Contains(t, stderr, "autoscaling", "stderr: %s", stderr)
+}