From c02e7e875439366f2b68b08ed7aeb2569eb83a17 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Fri, 10 Mar 2023 09:34:47 -0500
Subject: [PATCH] return early from opPlan when the plan is nil

While the returned plan is checked for nil in most cases, there was
a single point where the plan was dereferenced which could panic. Rather
than always guarding the dereferences, return early when the plan is
nil.
---
 internal/backend/backend.go                 |  5 +++--
 internal/backend/local/backend_plan.go      | 24 +++++++++++++--------
 internal/backend/local/backend_plan_test.go | 24 +++++++++++++++++++++
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/internal/backend/backend.go b/internal/backend/backend.go
index 48a049615d..58e4398a69 100644
--- a/internal/backend/backend.go
+++ b/internal/backend/backend.go
@@ -366,8 +366,9 @@ type RunningOperation struct {
 	// operation has completed.
 	Result OperationResult
 
-	// PlanEmpty is populated after a Plan operation completes without error
-	// to note whether a plan is empty or has changes.
+	// PlanEmpty is populated after a Plan operation completes to note whether
+	// a plan is empty or has changes. This is only used in the CLI to determine
+	// the exit status because the plan value is not available at that point.
 	PlanEmpty bool
 
 	// State is the final state after the operation completed. Persisting
diff --git a/internal/backend/local/backend_plan.go b/internal/backend/local/backend_plan.go
index 1513aaa43e..52aaad11e0 100644
--- a/internal/backend/local/backend_plan.go
+++ b/internal/backend/local/backend_plan.go
@@ -100,11 +100,19 @@ func (b *Local) opPlan(
 	// generate a partial saved plan file for external analysis.
 	diags = diags.Append(planDiags)
 
+	// Even if there are errors we need to handle anything that may be
+	// contained within the plan, so only exit if there is no data at all.
+	if plan == nil {
+		runningOp.PlanEmpty = true
+		op.ReportResult(runningOp, diags)
+		return
+	}
+
 	// Record whether this plan includes any side-effects that could be applied.
 	runningOp.PlanEmpty = !plan.CanApply()
 
 	// Save the plan to disk
-	if path := op.PlanOutPath; path != "" && plan != nil {
+	if path := op.PlanOutPath; path != "" {
 		if op.PlanOutBackend == nil {
 			// This is always a bug in the operation caller; it's not valid
 			// to set PlanOutPath without also setting PlanOutBackend.
@@ -154,15 +162,13 @@ func (b *Local) opPlan(
 
 	// Render the plan, if we produced one.
 	// (This might potentially be a partial plan with Errored set to true)
-	if plan != nil {
-		schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
-		diags = diags.Append(moreDiags)
-		if moreDiags.HasErrors() {
-			op.ReportResult(runningOp, diags)
-			return
-		}
-		op.View.Plan(plan, schemas)
+	schemas, moreDiags := lr.Core.Schemas(lr.Config, lr.InputState)
+	diags = diags.Append(moreDiags)
+	if moreDiags.HasErrors() {
+		op.ReportResult(runningOp, diags)
+		return
 	}
+	op.View.Plan(plan, schemas)
 
 	// If we've accumulated any diagnostics along the way then we'll show them
 	// here just before we show the summary and next steps. This can potentially
diff --git a/internal/backend/local/backend_plan_test.go b/internal/backend/local/backend_plan_test.go
index 5121d45e70..9dbed99532 100644
--- a/internal/backend/local/backend_plan_test.go
+++ b/internal/backend/local/backend_plan_test.go
@@ -880,3 +880,27 @@ func planFixtureSchema() *terraform.ProviderSchema {
 		},
 	}
 }
+
+func TestLocal_invalidOptions(t *testing.T) {
+	b := TestLocal(t)
+	TestLocalProvider(t, b, "test", planFixtureSchema())
+
+	op, configCleanup, done := testOperationPlan(t, "./testdata/plan")
+	defer configCleanup()
+	op.PlanRefresh = true
+	op.PlanMode = plans.RefreshOnlyMode
+	op.ForceReplace = []addrs.AbsResourceInstance{mustResourceInstanceAddr("test_instance.foo")}
+
+	run, err := b.Operation(context.Background(), op)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+	<-run.Done()
+	if run.Result == backend.OperationSuccess {
+		t.Fatalf("plan operation failed")
+	}
+
+	if errOutput := done(t).Stderr(); errOutput == "" {
+		t.Fatal("expected error output")
+	}
+}