From b4b72a35624bb5f6aed722519dde334b5345e7f8 Mon Sep 17 00:00:00 2001
From: Chlins Zhang <chlins.zhang@gmail.com>
Date: Tue, 4 Aug 2026 21:50:37 +0800
Subject: [PATCH] Add regression test for additional item with invalid JSON
 (#10103)

archive.Unmarshal returns (nil, err) when an item file contains malformed
JSON, and restoreItem dereferences its obj argument on entry, so an
additional item that fails to unmarshal must be skipped rather than passed
on. The loop only records the error and continues today; nothing covers
that, so removing the continue reintroduces a nil pointer dereference in
the restore reconciler without failing any test.

The item file is added to the tarball so the existing Stat check passes and
the unmarshal is actually reached.

Signed-off-by: chlins <chlins.zhang@gmail.com>
---
 pkg/restore/restore_test.go | 56 +++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
index 935586e63..46667b1f9 100644
--- a/pkg/restore/restore_test.go
+++ b/pkg/restore/restore_test.go
@@ -2299,6 +2299,62 @@ func TestRestoreActionAdditionalItems(t *testing.T) {
 	}
 }
 
+// TestRestoreActionAdditionalItemsInvalidJSON verifies that an additional item whose file
+// exists in the backup but does not contain valid JSON is reported as an error and skipped,
+// rather than being passed to restoreItem as a nil object.
+//
+// archive.Unmarshal returns (nil, err) for malformed JSON, and restoreItem dereferences its
+// obj argument immediately, so failing to skip the item panics the restore reconciler.
+func TestRestoreActionAdditionalItemsInvalidJSON(t *testing.T) {
+	h := newHarness(t)
+
+	for _, r := range []*test.APIResource{test.Pods(), test.PVs()} {
+		h.AddItems(t, r)
+	}
+
+	// pv-1.json exists so the Stat check passes, but its contents are not valid JSON.
+	tarball := test.NewTarWriter(t).
+		AddItems("pods", builder.ForPod("ns-1", "pod-1").Result()).
+		Add("resources/persistentvolumes/cluster/pv-1.json", []byte("not-json")).
+		Done()
+
+	actions := []riav2.RestoreItemAction{
+		&pluggableAction{
+			executeFunc: func(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) {
+				return &velero.RestoreItemActionExecuteOutput{
+					UpdatedItem: input.Item,
+					AdditionalItems: []velero.ResourceIdentifier{
+						{GroupResource: kuberesource.PersistentVolumes, Name: "pv-1"},
+					},
+				}, nil
+			},
+		},
+	}
+
+	data := &Request{
+		Log:          h.log,
+		Restore:      defaultRestore().Result(),
+		Backup:       defaultBackup().Result(),
+		BackupReader: tarball,
+	}
+
+	// A nil additional item passed on to restoreItem panics here rather than failing.
+	warnings, errs := h.restorer.Restore(data, actions, nil)
+
+	assertWantErrsOrWarnings(t, Result{}, warnings)
+	assertWantErrsOrWarnings(t, Result{
+		Namespaces: map[string][]string{
+			"ns-1": {"error restoring additional item persistentvolumes/pv-1"},
+		},
+	}, errs)
+
+	// The item that triggered the action is still restored, so the loop continued.
+	assertAPIContents(t, h, map[*test.APIResource][]string{
+		test.Pods(): {"ns-1/pod-1"},
+		test.PVs():  {},
+	})
+}
+
 // TestRestoreMustIncludeAdditionalItems covers restore must-include edge cases beyond the
 // basic filter-bypass cases in TestRestoreActionAdditionalItems.
 func TestRestoreMustIncludeAdditionalItems(t *testing.T) {