From c050619971493cb52198b5cfab816fd6025005c1 Mon Sep 17 00:00:00 2001 From: Nayana Bidari Date: Mon, 3 Mar 2025 23:27:44 -0800 Subject: [PATCH] Fix spec validation for entrypoint. The OCI spec has entrypoint as the first argument in specs.Process.Args. During testing, we observed inconsistencies in entrypoint resolution: it was resolved during checkpoint but not during restore (or vice versa), leading to spec validation failures. This CL corrects this by comparing the absolute paths of entrypoints in spec validation. PiperOrigin-RevId: 733200146 --- runsc/container/container_test.go | 25 ++++++++++++++++++ runsc/specutils/restore.go | 44 +++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index 76de1162a..410c54071 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -3994,6 +3994,31 @@ func TestSpecValidationIgnore(t *testing.T) { } } +func TestSpecValidationForArgs(t *testing.T) { + conf := testutil.TestConfig(t) + oldSpecs := make(map[string]*specs.Spec) + spec, _ := sleepSpecConf(t) + spec.Process.Cwd = "/bin" + spec.Process.Args[0] = "/bin/sleep" + oldSpecs["container1"] = spec + + newSpecs := make(map[string]*specs.Spec) + restoreSpec, _ := sleepSpecConf(t) + restoreSpec.Process.Cwd = "/bin" + restoreSpec.Process.Args[0] = "./sleep" + newSpecs["container1"] = restoreSpec + + if err := specutils.RestoreValidateSpec(oldSpecs, newSpecs, conf); err != nil { + t.Errorf("spec validation failed, got: %v, want: nil", err) + } + + spec.Process.Args = append(spec.Process.Args, "1") + restoreSpec.Process.Args = append(restoreSpec.Process.Args, "infinity") + if err := specutils.RestoreValidateSpec(oldSpecs, newSpecs, conf); err == nil { + t.Errorf("spec validation passed when we expected it to fail") + } +} + func TestCheckpointResume(t *testing.T) { for name, conf := range configs(t, true /* noOverlay */) { t.Run(name, func(t *testing.T) { diff --git a/runsc/specutils/restore.go b/runsc/specutils/restore.go index 14876c088..417e3e84d 100644 --- a/runsc/specutils/restore.go +++ b/runsc/specutils/restore.go @@ -16,6 +16,7 @@ package specutils import ( "fmt" + "path" "reflect" "slices" "sort" @@ -294,6 +295,44 @@ func ifNil[T any](v *T) *T { return &t } +// TODO(b/397790973): Check why the exec path gets resolved inconsistently. We +// are not resolving any paths in the spec, maybe this is happening at a higher +// layer. Ideally we should not be resolving any paths, this is a +// workaround fix. +func validateArgs(oldArgs, newArgs []string, cwd, cName string) error { + if len(oldArgs) != len(newArgs) { + return validateError("Args", cName, oldArgs, newArgs) + } + + if len(oldArgs) == 0 { + return nil + } + + oldExecPath := oldArgs[0] + newExecPath := newArgs[0] + hasPrefixOld := strings.HasPrefix(oldExecPath, "./") + hasPrefixNew := strings.HasPrefix(newExecPath, "./") + if hasPrefixOld == hasPrefixNew { + if !slices.Equal(oldArgs, newArgs) { + return validateError("Args", cName, oldArgs, newArgs) + } + return nil + } + + // One of the entrypoints across checkpoint restore is not resolved. + // Resolve the entrypoint using cwd to get the absolute path and compare. + if hasPrefixOld { + oldExecPath = path.Join(cwd, oldExecPath) + } + if hasPrefixNew { + newExecPath = path.Join(cwd, newExecPath) + } + if oldExecPath != newExecPath || !slices.Equal(oldArgs[1:], newArgs[1:]) { + return validateError("Args", cName, oldArgs, newArgs) + } + return nil +} + func validateSpecForContainer(oSpec, nSpec *specs.Spec, cName string) error { oldSpec := *oSpec newSpec := *nSpec @@ -341,9 +380,10 @@ func validateSpecForContainer(oSpec, nSpec *specs.Spec, cName string) error { return err } oldProcess.Rlimits, newProcess.Rlimits = nil, nil - if ok := slices.Equal(oldProcess.Args, newProcess.Args); !ok { - return validateError("Args", cName, oldProcess.Args, newProcess.Args) + if err := validateArgs(oldProcess.Args, newProcess.Args, oldProcess.Cwd, cName); err != nil { + return err } + oldProcess.Args, newProcess.Args = nil, nil if err := validateCapabilities("Capabilities", cName, oldProcess.Capabilities, newProcess.Capabilities); err != nil { return err }