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 }