From 2c101f22a750fc411c633137554a7337b0ebed59 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Wed, 25 Mar 2026 08:15:11 -0700 Subject: [PATCH] Fix nil panic when user declines preflight warnings (#7305) Extract isDeploymentSkipped() helper that checks SkippedReason != "" instead of matching only DeploymentStateSkipped. This catches PreflightAbortedSkipped (and any future skip reasons), preventing a nil pointer dereference on Deployment.Outputs when Deployment is nil. Add regression test covering all skip reason variants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/internal/cmd/provision.go | 13 +++- cli/azd/internal/cmd/provision_test.go | 86 ++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 cli/azd/internal/cmd/provision_test.go diff --git a/cli/azd/internal/cmd/provision.go b/cli/azd/internal/cmd/provision.go index cc07433ba80..d0494e332e8 100644 --- a/cli/azd/internal/cmd/provision.go +++ b/cli/azd/internal/cmd/provision.go @@ -424,10 +424,10 @@ func (p *ProvisionAction) Run(ctx context.Context) (*actions.ActionResult, error }, nil } - skipped := deployResult.SkippedReason == provisioning.DeploymentStateSkipped + skipped := isDeploymentSkipped(deployResult) allSkipped = allSkipped && skipped if skipped { - // Simply continue here; message is printed in the provider implementation + // Simply continue here; message is printed in the provider implementation. continue } @@ -550,3 +550,12 @@ func GetCmdProvisionHelpDescription(c *cobra.Command) string { " When omitted, provisions resources for all layers defined in the project."), }) } + +// isDeploymentSkipped returns true if the deployment was skipped and the caller +// should not access deployResult.Deployment (which may be nil). +// A deployment is considered skipped when SkippedReason is non-empty, which includes +// states such as DeploymentStateSkipped (no changes) and PreflightAbortedSkipped +// (user declined after preflight warnings). +func isDeploymentSkipped(deployResult *provisioning.DeployResult) bool { + return deployResult != nil && deployResult.SkippedReason != "" +} diff --git a/cli/azd/internal/cmd/provision_test.go b/cli/azd/internal/cmd/provision_test.go new file mode 100644 index 00000000000..7921a87c3d7 --- /dev/null +++ b/cli/azd/internal/cmd/provision_test.go @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" + "github.com/stretchr/testify/require" +) + +// TestIsDeploymentSkipped_AllSkipReasons verifies that isDeploymentSkipped +// correctly identifies ALL skip reasons, not just DeploymentStateSkipped. +// +// Regression test for https://github.com/Azure/azure-dev/issues/7305: +// When the user declines preflight validation warnings, Deploy returns +// PreflightAbortedSkipped with a nil Deployment. If this skip reason is not +// detected, the caller dereferences nil Deployment.Outputs and panics. +func TestIsDeploymentSkipped_AllSkipReasons(t *testing.T) { + tests := []struct { + name string + result *provisioning.DeployResult + expectSkipped bool + nilDeployment bool + }{ + { + name: "DeploymentStateSkipped", + result: &provisioning.DeployResult{ + SkippedReason: provisioning.DeploymentStateSkipped, + }, + expectSkipped: true, + nilDeployment: true, + }, + { + // This is the regression case from issue #7305. + // Before the fix, this was NOT detected as skipped, causing a nil + // pointer dereference when accessing Deployment.Outputs. + name: "PreflightAbortedSkipped", + result: &provisioning.DeployResult{ + SkippedReason: provisioning.PreflightAbortedSkipped, + }, + expectSkipped: true, + nilDeployment: true, + }, + { + name: "NotSkipped_WithDeployment", + result: &provisioning.DeployResult{ + Deployment: &provisioning.Deployment{ + Outputs: map[string]provisioning.OutputParameter{}, + }, + }, + expectSkipped: false, + nilDeployment: false, + }, + { + name: "NilDeployResult", + result: nil, + expectSkipped: false, + nilDeployment: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + skipped := isDeploymentSkipped(tt.result) + require.Equal(t, tt.expectSkipped, skipped, + "isDeploymentSkipped returned unexpected value") + + if tt.result == nil { + return + } + + // Verify the Deployment nil/non-nil state matches expectations. + // This is important: the bug in #7305 was caused by accessing + // Deployment.Outputs when Deployment was nil. + if tt.nilDeployment { + require.Nil(t, tt.result.Deployment, + "when skipped, Deployment may be nil (accessing it would panic)") + } else { + require.NotNil(t, tt.result.Deployment, + "when not skipped, Deployment must not be nil (callers access Deployment.Outputs)") + } + }) + } +}