diff --git a/cmd/nerdctl/container/container_export.go b/cmd/nerdctl/container/container_export.go index d7fc7abc580..84b12fc7777 100644 --- a/cmd/nerdctl/container/container_export.go +++ b/cmd/nerdctl/container/container_export.go @@ -68,7 +68,10 @@ func exportAction(cmd *cobra.Command, args []string) error { writer := cmd.OutOrStdout() if output != "" { - f, err := os.OpenFile(output, os.O_CREATE|os.O_WRONLY, 0644) + // O_TRUNC: writing a smaller archive over a bigger one would otherwise leave the tail of + // the bigger one past its end. A tar reader stops at the end-of-archive marker and would + // not notice, but the file would carry the bytes of another container. + f, err := os.OpenFile(output, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) if err != nil { return err } diff --git a/cmd/nerdctl/container/container_export_test.go b/cmd/nerdctl/container/container_export_test.go index ee25fe2dc9d..dd9945f5da6 100644 --- a/cmd/nerdctl/container/container_export_test.go +++ b/cmd/nerdctl/container/container_export_test.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "testing" "gotest.tools/v3/assert" @@ -157,6 +158,64 @@ func TestExportRunningContainer(t *testing.T) { testCase.Run(t) } +func TestExportReplacesExistingFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("export is not supported on Windows") + } + + testCase := nerdtest.Setup() + testCase.NoParallel = true + + testCase.Setup = func(data test.Data, helpers test.Helpers) { + bigger := data.Identifier("bigger") + smaller := data.Identifier("smaller") + outFile := filepath.Join(data.Temp().Path(), "reused.tar") + + helpers.Ensure("create", "--name", bigger, testutil.NginxAlpineImage) + helpers.Ensure("create", "--name", smaller, testutil.CommonImage) + + // The bigger filesystem first, so that the smaller one written over it has something to + // leave behind. + helpers.Ensure("export", "-o", outFile, bigger) + info, err := os.Stat(outFile) + assert.NilError(t, err) + + data.Labels().Set("bigger", bigger) + data.Labels().Set("smaller", smaller) + data.Labels().Set("outFile", outFile) + data.Labels().Set("biggerSize", strconv.FormatInt(info.Size(), 10)) + } + testCase.Cleanup = func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Labels().Get("bigger")) + helpers.Anyhow("rm", "-f", data.Labels().Get("smaller")) + } + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("export", "-o", data.Labels().Get("outFile"), data.Labels().Get("smaller")) + } + testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: 0, + Output: func(stdout string, t tig.T) { + info, err := os.Stat(data.Labels().Get("outFile")) + assert.NilError(t, err) + biggerSize, err := strconv.ParseInt(data.Labels().Get("biggerSize"), 10, 64) + assert.NilError(t, err) + + // The file must hold the smaller archive and nothing else. A tar reader stops at + // the end-of-archive marker, so a tail left over from the bigger archive would go + // unnoticed on read, but the file would still carry the bytes of another + // container. + assert.Assert(t, info.Size() < biggerSize, + "expected the file to shrink to the new archive, still %d of %d bytes", + info.Size(), biggerSize) + }, + } + } + + testCase.Run(t) +} + func TestExportNonexistentContainer(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("export is not supported on Windows") diff --git a/cmd/nerdctl/image/image_save.go b/cmd/nerdctl/image/image_save.go index 61bcbab54a3..9695f6c69a5 100644 --- a/cmd/nerdctl/image/image_save.go +++ b/cmd/nerdctl/image/image_save.go @@ -95,7 +95,10 @@ func saveAction(cmd *cobra.Command, args []string) error { if err != nil { return err } else if outputPath != "" { - f, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY, 0644) + // O_TRUNC: writing a smaller archive over a bigger one would otherwise leave the tail of + // the bigger one past its end. A tar reader stops at the end-of-archive marker and would + // not notice, but the file would carry the bytes of an unrelated image. + f, err := os.OpenFile(outputPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) if err != nil { return err } diff --git a/cmd/nerdctl/image/image_save_test.go b/cmd/nerdctl/image/image_save_test.go index 7671570d31c..c4135890477 100644 --- a/cmd/nerdctl/image/image_save_test.go +++ b/cmd/nerdctl/image/image_save_test.go @@ -20,6 +20,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "testing" @@ -66,6 +67,53 @@ func TestSaveContent(t *testing.T) { testCase.Run(t) } +func TestSaveReplacesExistingFile(t *testing.T) { + nerdtest.Setup() + + const reused = "reused.tar" + + testCase := &test.Case{ + // FIXME: move to busybox for windows? + Require: require.Not(require.Windows), + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("pull", "--quiet", testutil.NginxAlpineImage) + helpers.Ensure("pull", "--quiet", testutil.CommonImage) + + // A bigger archive first, so that the smaller one written over it has something to + // leave behind. + path := filepath.Join(data.Temp().Path(), reused) + helpers.Ensure("save", "-o", path, testutil.NginxAlpineImage) + info, err := os.Stat(path) + assert.NilError(t, err) + data.Labels().Set("bigger", strconv.FormatInt(info.Size(), 10)) + }, + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("save", "-o", filepath.Join(data.Temp().Path(), reused), testutil.CommonImage) + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + Output: func(stdout string, t tig.T) { + info, err := os.Stat(filepath.Join(data.Temp().Path(), reused)) + assert.NilError(t, err) + bigger, err := strconv.ParseInt(data.Labels().Get("bigger"), 10, 64) + assert.NilError(t, err) + + // The file must hold the smaller archive and nothing else. A tar reader stops + // at the end-of-archive marker, so a tail left over from the bigger archive + // would go unnoticed on read, but the file would still carry the bytes of an + // unrelated image. + assert.Assert(t, info.Size() < bigger, + "expected the file to shrink to the new archive, still %d of %d bytes", + info.Size(), bigger) + }, + } + }, + } + + testCase.Run(t) +} + func TestSaveQuiet(t *testing.T) { nerdtest.Setup()