fix(files): 编辑器保存和打开也逐跳核对 SHA-256，途中变了的内容不写盘也不打开

internal/fileedit/editor.go

@@ -77,6 +77,9 @@ var (
 	// ErrExists is a create, mkdir, rename or upload whose target is already
 	// there.
 	ErrExists = errors.New("fileedit: the target already exists")
+	// ErrReadDamaged is a read whose bytes do not hash to the digest the Job
+	// sent with them: they changed on the way to felis-api.
+	ErrReadDamaged = errors.New("fileedit: the file's bytes changed on their way from the file Job")
 )
 
 // Runner is the cluster-side half of a file operation. Run renders and creates
@@ -243,11 +246,17 @@ func (e *Editor) List(ctx context.Context, server, path string) (Listing, error)
 
 // Read returns a file's bytes, resolved under the server's world root, and the
 // SHA-256 of the file as it is on disk — the value to hand back as Write's expect.
+// Bytes that do not hash to the digest the Job computed over what it sent
+// (Result.ContentSHA256) are ErrReadDamaged: an editor that saves back a
+// damaged read would write the damage.
 func (e *Editor) Read(ctx context.Context, server, path string) ([]byte, string, error) {
 	res, err := e.run(ctx, server, JobParams{Op: OpRead, Path: path})
 	if err != nil {
 		return nil, "", err
 	}
+	if got := digest(res.Content); got != res.ContentSHA256 {
+		return nil, "", fmt.Errorf("%w: they hash to %s, sent as %q", ErrReadDamaged, got, res.ContentSHA256)
+	}
 	// A zero-length file unmarshals Content as nil, which is a legitimate result,
 	// not an error — normalise so the caller never has to distinguish nil from empty.
 	if res.Content == nil {
@@ -457,6 +466,8 @@ func resultError(res Result) error {
 		return fmt.Errorf("%w: %s", ErrNoSpace, res.Error)
 	case CodeExists:
 		return fmt.Errorf("%w: %s", ErrExists, res.Error)
+	case CodeDigestMismatch:
+		return fmt.Errorf("%w: %s", ErrDigestMismatch, res.Error)
 	default:
 		return fmt.Errorf("fileedit: file operation failed (%s): %s", res.Code, res.Error)
 	}

internal/fileedit/editor_test.go

@@ -2,6 +2,7 @@ package fileedit
 
 import (
 	"context"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"testing"
@@ -79,7 +80,7 @@ func TestEditorRendersParams(t *testing.T) {
 	})
 
 	t.Run("read", func(t *testing.T) {
-		r := &fakeRunner{payload: mustPayload(t, Result{Content: []byte("motd=hi\n"), SHA256: "abc"})}
+		r := &fakeRunner{payload: mustPayload(t, Result{Content: []byte("motd=hi\n"), SHA256: "abc", ContentSHA256: hex.EncodeToString(sumOf("motd=hi\n"))})}
 		e := &Editor{Runner: r, Config: Config{Image: "img"}}
 
 		got, sum, err := e.Read(context.Background(), "survival", "server.properties")
@@ -162,7 +163,7 @@ func TestEditorRendersParams(t *testing.T) {
 // every time. If it ever cached one, two operations would collide on a name
 // felis-api has no permission to delete.
 func TestEditorMintsAFreshOpID(t *testing.T) {
-	r := &fakeRunner{payload: mustPayload(t, Result{})}
+	r := &fakeRunner{payload: mustPayload(t, Result{ContentSHA256: hex.EncodeToString(sumOf(""))})}
 	e := &Editor{Runner: r, Config: Config{Image: "img"}}
 
 	for range 3 {
@@ -198,6 +199,7 @@ func TestEditorMapsResultCodes(t *testing.T) {
 		{"changed since read", CodeConflict, ErrConflict},
 		{"volume full", CodeNoSpace, ErrNoSpace},
 		{"already there", CodeExists, ErrExists},
+		{"changed on the way", CodeDigestMismatch, ErrDigestMismatch},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -251,11 +253,33 @@ func TestEditorRefusesOversizedWriteBeforeTheCluster(t *testing.T) {
 	}
 }
 
+// A read whose bytes do not hash to the digest the Job sent with them changed
+// on the way, and none of them is handed on: an editor saving a damaged read
+// would write the damage back.
+func TestEditorReadRefusesBytesChangedOnTheWay(t *testing.T) {
+	for _, tc := range []struct {
+		name string
+		sent string
+	}{
+		{"hashed otherwise", hex.EncodeToString(sumOf("motd=hi\n"))},
+		{"with no digest", ""},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := &fakeRunner{payload: mustPayload(t, Result{Content: []byte("motd=ho\n"), SHA256: "abc", ContentSHA256: tc.sent})}
+			e := &Editor{Runner: r, Config: Config{Image: "img"}}
+			got, sum, err := e.Read(context.Background(), "survival", "server.properties")
+			if !errors.Is(err, ErrReadDamaged) || got != nil || sum != "" {
+				t.Fatalf("Read = %q, %q, %v; want nothing and ErrReadDamaged", got, sum, err)
+			}
+		})
+	}
+}
+
 // TestEditorNormalisesEmptyResults pins that "nothing there" is a success, not a
 // nil surprise: an empty directory lists as [] and a zero-length file reads as
 // empty bytes, so no caller has to distinguish nil from empty.
 func TestEditorNormalisesEmptyResults(t *testing.T) {
-	r := &fakeRunner{payload: mustPayload(t, Result{})}
+	r := &fakeRunner{payload: mustPayload(t, Result{ContentSHA256: hex.EncodeToString(sumOf(""))})}
 	e := &Editor{Runner: r, Config: Config{Image: "img"}}
 
 	ls, err := e.List(context.Background(), "survival", "empty")

internal/fileedit/exec.go

@@ -84,6 +84,10 @@ const (
 	// there. None of them replaces anything unless told to (an upload's
 	// Overwrite), so a name collision is reported rather than resolved.
 	CodeExists = "exists"
+	// CodeDigestMismatch is a write whose bytes do not hash to the SHA-256
+	// felis-api computed over them (Request.ContentSHA256): they changed on the
+	// way to the Job, and nothing was written.
+	CodeDigestMismatch = "digest_mismatch"
 )
 
 // ResultPrefix marks the single stdout line carrying the JSON Result. The Job's
@@ -186,6 +190,10 @@ type Result struct {
 	// conflict, the file as it is now. A client hands it back as the expected
 	// hash of its next write (see write).
 	SHA256 string `json:"sha256,omitempty"`
+	// ContentSHA256 is, after a read, the hex digest of Content as handed out
+	// (after any redaction), so felis-api can tell the bytes it got from the
+	// bytes this Job sent (Editor.Read).
+	ContentSHA256 string `json:"content_sha256,omitempty"`
 
 	// Conflicts lists, relative to the root and sorted, the existing files an
 	// unzip would replace: the first of them, up to MaxConflicts and 8 KiB of
@@ -213,9 +221,11 @@ type Request struct {
 	To string
 	// Content and Expect are a write's bytes and precondition: when Expect is
 	// non-empty, the write lands only if the file's current SHA-256 (hex) equals
-	// it.
-	Content []byte
-	Expect  string
+	// it. ContentSHA256, when set, is the SHA-256 (hex) felis-api computed over
+	// Content; bytes that hash otherwise are not written (CodeDigestMismatch).
+	Content       []byte
+	Expect        string
+	ContentSHA256 string
 	// CreateOnly makes a write refuse a path that already exists. It is the
 	// panel's "new file", which must never truncate a file it did not know was
 	// there.
@@ -295,7 +305,7 @@ func Execute(root string, req Request) (Result, error) {
 	case OpRead:
 		return read(r, path), nil
 	case OpWrite:
-		return write(r, path, req.Content, req.Expect, req.CreateOnly), nil
+		return write(r, path, req.Content, req.ContentSHA256, req.Expect, req.CreateOnly), nil
 	case OpMkdir:
 		return mkdir(r, path), nil
 	case OpDelete:
@@ -432,7 +442,7 @@ func read(r *os.Root, name string) Result {
 	if redact {
 		content = RedactProps(b)
 	}
-	return Result{Content: content, SHA256: digest(b)}
+	return Result{Content: content, SHA256: digest(b), ContentSHA256: digest(content)}
 }
 
 // propsPath is the server's main config file, and rconPasswordKey the one line in
@@ -492,7 +502,17 @@ func redactSecretProps(name string, content []byte) []byte {
 // being overwritten, which is how two people editing the same file find out.
 // The world lock (internal/maintenance) already serialises writes, so the check
 // and the rename cannot interleave with another write.
-func write(r *os.Root, name string, content []byte, expect string, createOnly bool) Result {
+//
+// sum, when set, is the SHA-256 felis-api computed over content before handing
+// it to the Job: content that hashes otherwise changed on the way, and is
+// refused with CodeDigestMismatch before anything is touched.
+func write(r *os.Root, name string, content []byte, sum, expect string, createOnly bool) Result {
+	if sum != "" {
+		if got := digest(content); got != sum {
+			return Result{Code: CodeDigestMismatch, Error: fmt.Sprintf(
+				"the content hashes to %s and was sent as %s; nothing was written", got, sum)}
+		}
+	}
 	if len(content) > MaxWriteBytes {
 		// Defence in depth: felis-api already refuses an oversized write with a 413
 		// before rendering the Job. Re-checking here keeps the ceiling true even if

internal/fileedit/exec_test.go

@@ -435,6 +435,11 @@ func TestReadRedactsRconPassword(t *testing.T) {
 	if sum := sha256.Sum256([]byte(props)); res.SHA256 != hex.EncodeToString(sum[:]) {
 		t.Fatalf("sha256 = %s, want the hash of the file as stored", res.SHA256)
 	}
+	// The content digest is of the copy handed out, so the bytes that arrive
+	// can be checked against it.
+	if sum := sha256.Sum256(res.Content); res.ContentSHA256 != hex.EncodeToString(sum[:]) || res.ContentSHA256 == res.SHA256 {
+		t.Fatalf("content_sha256 = %s, want the hash of the redacted copy (%x), apart from sha256 %s", res.ContentSHA256, sum, res.SHA256)
+	}
 	got := string(res.Content)
 	if strings.Contains(got, "hunter2") {
 		t.Fatalf("read returned the RCON password (spec §286):\n%s", got)
@@ -633,6 +638,34 @@ func TestWriteDetectsConcurrentChange(t *testing.T) {
 	}
 }
 
+// TestWriteChecksTheContentDigest: a write lands only bytes that hash to the
+// SHA-256 felis-api sent with them; bytes changed on the way touch nothing.
+func TestWriteChecksTheContentDigest(t *testing.T) {
+	root, _ := worldRoot(t)
+	props := filepath.Join(root, "server.properties")
+	if err := os.WriteFile(props, []byte("motd=hello\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	sent := sha256.Sum256([]byte("motd=mine\n"))
+	req := Request{Op: OpWrite, Path: "server.properties", Content: []byte("motd=mint\n"), ContentSHA256: hex.EncodeToString(sent[:])}
+	res, err := Execute(root, req)
+	if err != nil || res.Code != CodeDigestMismatch {
+		t.Fatalf("changed write = %+v, %v; want %s", res, err, CodeDigestMismatch)
+	}
+	if b, _ := os.ReadFile(props); string(b) != "motd=hello\n" {
+		t.Fatalf("a changed write replaced the file with %q", b)
+	}
+	assertNoTemporaries(t, root)
+
+	req.Content = []byte("motd=mine\n")
+	if res, err := Execute(root, req); err != nil || res.Code != "" {
+		t.Fatalf("write as sent = %+v, %v", res, err)
+	}
+	if b, _ := os.ReadFile(props); string(b) != "motd=mine\n" {
+		t.Fatalf("on disk %q, want the bytes as sent", b)
+	}
+}
+
 func assertNoTemporaries(t *testing.T, dir string) {
 	t.Helper()
 	des, err := os.ReadDir(dir)

internal/fileedit/jobspec.go

@@ -195,6 +195,9 @@ func FilesJob(p JobParams) (*batchv1.Job, error) {
 	// secret, so it rides argv; only the content itself needs the env channel.
 	switch p.Op {
 	case OpWrite:
+		// The content's own SHA-256 goes beside it, so the Job writes only the
+		// bytes felis-api handed over (Request.ContentSHA256).
+		args = append(args, "--sha256", digest(p.Content))
 		if p.Expect != "" {
 			args = append(args, "--expect-sha256", p.Expect)
 		}

internal/fileedit/jobspec_test.go

@@ -2,7 +2,9 @@ package fileedit
 
 import (
 	"bytes"
+	"crypto/sha256"
 	"encoding/base64"
+	"encoding/hex"
 	"slices"
 	"strconv"
 	"strings"
@@ -274,6 +276,12 @@ func TestFilesJobContentEnv(t *testing.T) {
 		if strings.Contains(strings.Join(job.Spec.Template.Spec.Containers[0].Args, " "), "motd=hello") {
 			t.Fatal("content must not appear in the container arguments")
 		}
+		// Its SHA-256 does, so the Job writes only the bytes felis-api handed over.
+		sum := sha256.Sum256(p.Content)
+		args := job.Spec.Template.Spec.Containers[0].Args
+		if i := slices.Index(args, "--sha256"); i < 0 || i+1 >= len(args) || args[i+1] != hex.EncodeToString(sum[:]) {
+			t.Fatalf("args %q, want --sha256 %x", args, sum)
+		}
 	})
 
 	// execve refuses one environment string over 128 KiB and the container never
@@ -353,7 +361,8 @@ func TestFilesJobOpArgs(t *testing.T) {
 
 	create := testParams(OpWrite)
 	create.CreateOnly = true
-	if got := args(create); !slices.Equal(got, []string{"--create-only"}) {
+	// The content (none here) goes with its SHA-256.
+	if got := args(create); !slices.Equal(got, []string{"--sha256", "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "--create-only"}) {
 		t.Errorf("create-only write args = %v", got)
 	}
 	readCreate := testParams(OpRead)