From c2487c2997de50c11892382c08c592d44773d5d4 Mon Sep 17 00:00:00 2001
From: Nick Santos <nick.santos@docker.com>
Date: Mon, 20 Feb 2023 19:51:57 -0500
Subject: [PATCH] context: avoid corrupt file writes

Write to a tempfile then move, so that if the
process dies mid-write it doesn't corrupt the store.

Also improve error messaging so that if a file does
get corrupted, the user has some hope of figuring
out which file is broken.

For background, see:
https://github.com/docker/for-win/issues/13180
https://github.com/docker/for-win/issues/12561

For a repro case, see:
https://github.com/nicks/contextstore-sandbox

Signed-off-by: Nick Santos <nick.santos@docker.com>
---
 cli/context/store/metadatastore.go | 13 ++++++++-----
 cli/context/store/store_test.go    | 27 +++++++++++++++++++++++++++
 cli/context/store/tlsstore.go      |  3 ++-
 3 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/cli/context/store/metadatastore.go b/cli/context/store/metadatastore.go
index ba3ea6c05a..62c3f82a6a 100644
--- a/cli/context/store/metadatastore.go
+++ b/cli/context/store/metadatastore.go
@@ -2,12 +2,14 @@ package store
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
 	"sort"
 
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/pkg/ioutils"
 	"github.com/fvbommel/sortorder"
 	"github.com/pkg/errors"
 )
@@ -35,7 +37,7 @@ func (s *metadataStore) createOrUpdate(meta Metadata) error {
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(filepath.Join(contextDir, metaFile), bytes, 0o644)
+	return ioutils.AtomicWriteFile(filepath.Join(contextDir, metaFile), bytes, 0o644)
 }
 
 func parseTypedOrMap(payload []byte, getter TypeGetter) (interface{}, error) {
@@ -65,7 +67,8 @@ func (s *metadataStore) get(name string) (Metadata, error) {
 }
 
 func (s *metadataStore) getByID(id contextdir) (Metadata, error) {
-	bytes, err := os.ReadFile(filepath.Join(s.contextDir(id), metaFile))
+	fileName := filepath.Join(s.contextDir(id), metaFile)
+	bytes, err := os.ReadFile(fileName)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
 			return Metadata{}, errdefs.NotFound(errors.Wrap(err, "context not found"))
@@ -77,15 +80,15 @@ func (s *metadataStore) getByID(id contextdir) (Metadata, error) {
 		Endpoints: make(map[string]interface{}),
 	}
 	if err := json.Unmarshal(bytes, &untyped); err != nil {
-		return Metadata{}, err
+		return Metadata{}, fmt.Errorf("parsing %s: %v", fileName, err)
 	}
 	r.Name = untyped.Name
 	if r.Metadata, err = parseTypedOrMap(untyped.Metadata, s.config.contextType); err != nil {
-		return Metadata{}, err
+		return Metadata{}, fmt.Errorf("parsing %s: %v", fileName, err)
 	}
 	for k, v := range untyped.Endpoints {
 		if r.Endpoints[k], err = parseTypedOrMap(v, s.config.endpointTypes[k]); err != nil {
-			return Metadata{}, err
+			return Metadata{}, fmt.Errorf("parsing %s: %v", fileName, err)
 		}
 	}
 	return r, err
diff --git a/cli/context/store/store_test.go b/cli/context/store/store_test.go
index c918be4962..235870a954 100644
--- a/cli/context/store/store_test.go
+++ b/cli/context/store/store_test.go
@@ -7,9 +7,11 @@ import (
 	"bytes"
 	"crypto/rand"
 	"encoding/json"
+	"fmt"
 	"io"
 	"os"
 	"path"
+	"path/filepath"
 	"testing"
 
 	"github.com/docker/docker/errdefs"
@@ -230,3 +232,28 @@ func TestImportZipInvalid(t *testing.T) {
 	err = Import("zipInvalid", s, r)
 	assert.ErrorContains(t, err, "unexpected context file")
 }
+
+func TestCorruptMetadata(t *testing.T) {
+	tempDir := t.TempDir()
+	s := New(tempDir, testCfg)
+	err := s.CreateOrUpdate(
+		Metadata{
+			Endpoints: map[string]interface{}{
+				"ep1": endpoint{Foo: "bar"},
+			},
+			Metadata: context{Bar: "baz"},
+			Name:     "source",
+		})
+	assert.NilError(t, err)
+
+	// Simulate the meta.json file getting corrupted
+	// by some external process.
+	contextDir := s.meta.contextDir(contextdirOf("source"))
+	contextFile := filepath.Join(contextDir, metaFile)
+	err = os.WriteFile(contextFile, nil, 0o600)
+	assert.NilError(t, err)
+
+	// Assert that the error message gives the user some clue where to look.
+	_, err = s.GetMetadata("source")
+	assert.ErrorContains(t, err, fmt.Sprintf("parsing %s: unexpected end of JSON input", contextFile))
+}
diff --git a/cli/context/store/tlsstore.go b/cli/context/store/tlsstore.go
index c61a7f549c..ffbbde7c0d 100644
--- a/cli/context/store/tlsstore.go
+++ b/cli/context/store/tlsstore.go
@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/pkg/ioutils"
 	"github.com/pkg/errors"
 )
 
@@ -31,7 +32,7 @@ func (s *tlsStore) createOrUpdate(name, endpointName, filename string, data []by
 	if err := os.MkdirAll(endpointDir, 0o700); err != nil {
 		return err
 	}
-	return os.WriteFile(filepath.Join(endpointDir, filename), data, 0o600)
+	return ioutils.AtomicWriteFile(filepath.Join(endpointDir, filename), data, 0o600)
 }
 
 func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) {