Unverified Commit e1c325d5 authored by Lemon-miaow's avatar Lemon-miaow
Browse files

fix(files): 配置文件改为同目录临时文件 fsync 后原子改名写入,读取返回内容哈希、保存带期望哈希冲突返回 409,磁盘满返回 507,面板提示载入最新或仍然覆盖

parent 074bd178
Loading
Loading
Loading
Loading
+3 −2
Changes for cmd/felis/files.go: 3 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -19,7 +19,7 @@ import (
// Like cmdRestore it deliberately holds NO database credentials and never calls
// config.Load: felis-api made the authorization decision (the caller owns this
// server, and the server is stopped so the RWO world volume is free); this process
// is the unprivileged hands that touch bytes. Its entire input is the three flags
// is the unprivileged hands that touch bytes. Its entire input is the flags
// below plus, for a write, one environment variable. Every isolation guarantee
// lives in the Pod spec (internal/fileedit/jobspec.go), and the path-containment
// guarantee lives in fileedit.Execute, which resolves the path through os.Root and
@@ -37,6 +37,7 @@ func cmdFiles(args []string, stdout, stderr io.Writer) int {
	op := fs.String("op", "", "operation: list, read, or write")
	path := fs.String("path", "", "path to operate on, relative to the world root (empty = the root itself)")
	worldsRoot := fs.String("worlds-root", "/data", "mount path of the world PVC; every path resolves under it")
	expect := fs.String("expect-sha256", "", "write only: refuse unless the file's current SHA-256 (hex) is this")
	if err := fs.Parse(args); err != nil {
		return 2
	}
@@ -67,7 +68,7 @@ func cmdFiles(args []string, stdout, stderr io.Writer) int {
		content = decoded
	}

	res, err := fileedit.Execute(*worldsRoot, *op, *path, content)
	res, err := fileedit.Execute(*worldsRoot, *op, *path, content, *expect)
	if err != nil {
		// The operation could not be attempted — infrastructure, not caller fault.
		fmt.Fprintf(stderr, "felis files: %v\n", err)
+26 −4
Changes for docs/openapi.yaml: 26 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -1305,7 +1305,7 @@ paths:
            application/json:
              schema: { $ref: '#/components/schemas/Error' }
        '409':
          description: Server is not stopped (not_stopped), or a restore, backup or file write already holds its world volume (maintenance_in_progress).
          description: Server is not stopped (not_stopped), a restore, backup or file write already holds its world volume (maintenance_in_progress), or the file changed since expect_sha256 was read (file_changed).
          content:
            application/json:
              schema: { $ref: '#/components/schemas/Error' }
@@ -3244,10 +3244,16 @@ paths:
            application/json:
              schema:
                type: object
                required: [path, content]
                required: [path, content, sha256]
                properties:
                  path: { type: string }
                  content: { type: string, format: byte, description: Base64-encoded file bytes. }
                  sha256:
                    type: string
                    pattern: '^[0-9a-f]{64}$'
                    description: >-
                      SHA-256 of the file as stored (before the rcon.password redaction in
                      server.properties). Send it back as expect_sha256 on the next write.
        '400':
          description: Missing path, invalid server name, or a path that escapes the world root.
          content:
@@ -3289,7 +3295,10 @@ paths:
        survive intact. Writes are capped at 256 KiB — the Job spec carries the content,
        and etcd bounds the object — so a larger body is 413. Same stopped-gate and
        os.Root containment as the read; a write through a symlink leaving the world
        root is refused. Audited as file.write.
        root is refused. The replacement is atomic (a synced temporary sibling renamed
        over the file, keeping its mode), so a failed write leaves the old file whole.
        With expect_sha256 the write lands only if the file still has that hash;
        otherwise 409 file_changed. Audited as file.write.
      x-felis-face: [external]
      x-felis-tier: app
      security: [{ accessJWT: [] }]
@@ -3309,6 +3318,13 @@ paths:
              required: [content]
              properties:
                content: { type: string, format: byte, description: Base64-encoded file bytes. }
                expect_sha256:
                  type: string
                  pattern: '^[0-9a-f]{64}$'
                  description: >-
                    The sha256 a read returned. When present, the write is refused with
                    409 file_changed if the file has changed (or been deleted) since.
                    Omit it to write unconditionally.
      responses:
        '200':
          description: File written.
@@ -3316,10 +3332,11 @@ paths:
            application/json:
              schema:
                type: object
                required: [path, status]
                required: [path, status, sha256]
                properties:
                  path: { type: string }
                  status: { type: string, const: written }
                  sha256: { type: string, pattern: '^[0-9a-f]{64}$', description: SHA-256 of the bytes written. }
        '400':
          description: Missing path, malformed body, invalid server name, or a path that escapes the world root.
          content:
@@ -3344,6 +3361,11 @@ paths:
          content:
            application/json:
              schema: { $ref: '#/components/schemas/Error' }
        '507':
          description: The world volume has no room for the write (volume_full); the file is unchanged.
          content:
            application/json:
              schema: { $ref: '#/components/schemas/Error' }
        '503':
          $ref: '#/components/responses/ServiceUnavailable'
        '504':
+33 −8
Changes for internal/api/handlers_files.go: 33 added lines, 8 removed lines.
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ import (
	"context"
	"errors"
	"net/http"
	"regexp"

	"felis.lolicon.best/internal/apis/felis/v1alpha1"
	"felis.lolicon.best/internal/fileedit"
@@ -30,12 +31,16 @@ import (
// mirrors how ImageBuilder uses build.Request/build.Image rather than restating a
// parallel type on this side of the seam.
//
// It returns fileedit.ErrNotFound / ErrBadPath / ErrTooLarge for caller-fault
// failures, which writeFileEditError maps to 404 / 400 / 413.
// It returns fileedit.ErrNotFound / ErrBadPath / ErrTooLarge / ErrConflict /
// ErrNoSpace, which writeFileEditError maps to 404 / 400 / 413 / 409 / 507.
//
// Read and Write both return the file's SHA-256 (hex). Write's expect is the hash
// a client read the file at; when set, a file that changed since is refused with
// ErrConflict instead of being overwritten.
type FileEditor interface {
	List(ctx context.Context, server, path string) (entries []fileedit.Entry, truncated bool, err error)
	Read(ctx context.Context, server, path string) ([]byte, error)
	Write(ctx context.Context, server, path string, content []byte) error
	Read(ctx context.Context, server, path string) (content []byte, sha256 string, err error)
	Write(ctx context.Context, server, path string, content []byte, expect string) (sha256 string, err error)
}

// writeFileRequest is the PUT /servers/{name}/file body. Content is []byte, so
@@ -49,8 +54,14 @@ type FileEditor interface {
// answering 200 — a client serialisation bug silently destroying the very config
// the caller opened this endpoint to repair. nil now means "the field was omitted"
// and is refused; an explicit "" is still a legitimate deliberate truncate.
//
// ExpectSHA256 is optional. The panel always sends the hash its read returned,
// so a save over a file someone else changed in the meantime answers 409
// file_changed; omitting it (a script, or "overwrite anyway") writes
// unconditionally.
type writeFileRequest struct {
	Content      *[]byte `json:"content"`
	ExpectSHA256 string  `json:"expect_sha256,omitempty"`
}

// handleListFiles serves GET /api/v1/servers/{name}/files?path=… — one directory's
@@ -98,12 +109,12 @@ func (a *API) handleReadFile(w http.ResponseWriter, r *http.Request) {
		return
	}

	content, err := a.Files.Read(r.Context(), name, path)
	content, sum, err := a.Files.Read(r.Context(), name, path)
	if err != nil {
		writeFileEditError(w, r, err)
		return
	}
	writeJSON(w, http.StatusOK, map[string]any{"path": path, "content": content})
	writeJSON(w, http.StatusOK, map[string]any{"path": path, "content": content, "sha256": sum})
}

// handleWriteFile serves PUT /api/v1/servers/{name}/file?path=… — replace a file's
@@ -149,6 +160,13 @@ func (a *API) handleWriteFile(w http.ResponseWriter, r *http.Request) {
			len(*body.Content), fileedit.MaxWriteBytes))
		return
	}
	// The hash rides the Job's argv, so only its one legitimate shape is let
	// through: 64 lowercase hex digits, exactly what a read returned.
	if body.ExpectSHA256 != "" && !sha256Hex.MatchString(body.ExpectSHA256) {
		writeError(w, r, newError(http.StatusBadRequest, "bad_request",
			"expect_sha256 must be the 64-digit lowercase hex sha256 a read returned"))
		return
	}

	// A write holds the world volume for its Job's lifetime (internal/maintenance);
	// reads and listings do not, since a read-only mount cannot hurt a server
@@ -159,15 +177,18 @@ func (a *API) handleWriteFile(w http.ResponseWriter, r *http.Request) {
	}
	defer release()

	if err := a.Files.Write(r.Context(), name, path, *body.Content); err != nil {
	sum, err := a.Files.Write(r.Context(), name, path, *body.Content, body.ExpectSHA256)
	if err != nil {
		writeFileEditError(w, r, err)
		return
	}

	a.audit(r, "file.write", name+":"+path)
	writeJSON(w, http.StatusOK, map[string]any{"path": path, "status": "written"})
	writeJSON(w, http.StatusOK, map[string]any{"path": path, "status": "written", "sha256": sum})
}

var sha256Hex = regexp.MustCompile(`^[0-9a-f]{64}$`)

// authorizeFileOp is the shared front half of all three file handlers — the gate
// that decides whether this caller may touch this server's world at all. It
// mirrors the backup/restore gate step for step, because it is guarding the same
@@ -260,6 +281,10 @@ func writeFileEditError(w http.ResponseWriter, r *http.Request, err error) {
		writeError(w, r, newError(http.StatusBadRequest, "bad_path", "%s", err.Error()))
	case errors.Is(err, fileedit.ErrTooLarge):
		writeError(w, r, newError(http.StatusRequestEntityTooLarge, "too_large", "%s", err.Error()))
	case errors.Is(err, fileedit.ErrConflict):
		writeError(w, r, newError(http.StatusConflict, "file_changed", "%s", err.Error()))
	case errors.Is(err, fileedit.ErrNoSpace):
		writeError(w, r, newError(http.StatusInsufficientStorage, "volume_full", "%s", err.Error()))
	case errors.Is(err, context.DeadlineExceeded):
		writeError(w, r, newError(http.StatusGatewayTimeout, "files_timeout",
			"the file operation did not finish in time; retry shortly"))
+63 −7
Changes for internal/api/handlers_files_test.go: 63 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -23,10 +23,12 @@ type fakeFileEditor struct {
	gotServer  string
	gotPath    string
	gotContent []byte
	gotExpect  string

	entries   []fileedit.Entry
	truncated bool
	content   []byte
	sum       string
}

func (f *fakeFileEditor) List(_ context.Context, server, path string) ([]fileedit.Entry, bool, error) {
@@ -35,18 +37,21 @@ func (f *fakeFileEditor) List(_ context.Context, server, path string) ([]fileedi
	return f.entries, f.truncated, f.err
}

func (f *fakeFileEditor) Read(_ context.Context, server, path string) ([]byte, error) {
func (f *fakeFileEditor) Read(_ context.Context, server, path string) ([]byte, string, error) {
	f.calls++
	f.gotServer, f.gotPath = server, path
	return f.content, f.err
	return f.content, f.sum, f.err
}

func (f *fakeFileEditor) Write(_ context.Context, server, path string, content []byte) error {
func (f *fakeFileEditor) Write(_ context.Context, server, path string, content []byte, expect string) (string, error) {
	f.calls++
	f.gotServer, f.gotPath, f.gotContent = server, path, content
	return f.err
	f.gotServer, f.gotPath, f.gotContent, f.gotExpect = server, path, content, expect
	return f.sum, f.err
}

// testSum is a well-formed sha256 hex digest for the fake to hand out.
var testSum = strings.Repeat("a", 64)

// mkFiles builds an API whose "survival" server is STOPPED and owned by owner1,
// with a wired fakeFileEditor — the state in which every file operation is
// permitted, so each subtest changes exactly the one thing it is about.
@@ -310,9 +315,10 @@ func TestFileEditorHandlers(t *testing.T) {
		}
	})

	t.Run("read returns base64 content", func(t *testing.T) {
	t.Run("read returns base64 content and its hash", func(t *testing.T) {
		api, _, _, files := mkFiles()
		files.content = []byte("motd=hello\n")
		files.sum = testSum
		api.External = staticExternal{p: owner}

		w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/file?path=server.properties", "", nil)
@@ -322,11 +328,12 @@ func TestFileEditorHandlers(t *testing.T) {
		var resp struct {
			Path    string `json:"path"`
			Content []byte `json:"content"`
			SHA256  string `json:"sha256"`
		}
		if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
			t.Fatalf("body not JSON: %v", err)
		}
		if resp.Path != "server.properties" || string(resp.Content) != "motd=hello\n" {
		if resp.Path != "server.properties" || string(resp.Content) != "motd=hello\n" || resp.SHA256 != testSum {
			t.Fatalf("unexpected response %+v (%q)", resp, resp.Content)
		}
	})
@@ -361,6 +368,53 @@ func TestFileEditorHandlers(t *testing.T) {
		}
	})

	t.Run("write passes the expected hash through and returns the new one", func(t *testing.T) {
		api, _, _, files := mkFiles()
		files.sum = strings.Repeat("b", 64)
		api.External = staticExternal{p: owner}
		w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=server.properties",
			`{"content":"aGk=","expect_sha256":"`+testSum+`"}`, jsonHeader)
		if w.Code != http.StatusOK {
			t.Fatalf("code = %d (%s)", w.Code, w.Body.String())
		}
		if files.gotExpect != testSum {
			t.Fatalf("executor got expect %q, want %q", files.gotExpect, testSum)
		}
		var resp struct {
			SHA256 string `json:"sha256"`
		}
		if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil || resp.SHA256 != files.sum {
			t.Fatalf("response sha256 = %q (%v), want %q", resp.SHA256, err, files.sum)
		}
	})

	t.Run("a malformed expected hash -> 400 before the executor", func(t *testing.T) {
		for _, bad := range []string{"abc", strings.Repeat("A", 64), strings.Repeat("a", 63) + " ", "--op=list"} {
			api, _, _, files := mkFiles()
			api.External = staticExternal{p: owner}
			body, _ := json.Marshal(map[string]any{"content": []byte("hi"), "expect_sha256": bad})
			w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=server.properties",
				string(body), jsonHeader)
			if w.Code != http.StatusBadRequest || files.calls != 0 {
				t.Fatalf("expect %q: code = %d calls = %d, want 400 and no Job", bad, w.Code, files.calls)
			}
		}
	})

	t.Run("a stale write -> 409 file_changed, not audited", func(t *testing.T) {
		api, repo, _, files := mkFiles()
		files.err = fmt.Errorf("%w: server.properties has changed", fileedit.ErrConflict)
		api.External = staticExternal{p: owner}
		w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=server.properties",
			`{"content":"aGk=","expect_sha256":"`+testSum+`"}`, jsonHeader)
		if w.Code != http.StatusConflict || decodeErr(t, w) != "file_changed" {
			t.Fatalf("code = %d body %s, want 409 file_changed", w.Code, w.Body.String())
		}
		if len(repo.audits) != 0 {
			t.Fatalf("a refused write was audited: %+v", repo.audits)
		}
	})

	t.Run("reads are not audited", func(t *testing.T) {
		api, repo, _, _ := mkFiles()
		api.External = staticExternal{p: owner}
@@ -457,6 +511,8 @@ func TestFileEditorErrorMapping(t *testing.T) {
		{"escaping path", fmt.Errorf("%w: nope", fileedit.ErrBadPath), http.StatusBadRequest, "bad_path"},
		{"missing file", fmt.Errorf("%w: nope", fileedit.ErrNotFound), http.StatusNotFound, "not_found"},
		{"oversized file", fmt.Errorf("%w: nope", fileedit.ErrTooLarge), http.StatusRequestEntityTooLarge, "too_large"},
		{"changed since read", fmt.Errorf("%w: nope", fileedit.ErrConflict), http.StatusConflict, "file_changed"},
		{"volume full", fmt.Errorf("%w: nope", fileedit.ErrNoSpace), http.StatusInsufficientStorage, "volume_full"},
		{"timeout", fmt.Errorf("waiting: %w", context.DeadlineExceeded), http.StatusGatewayTimeout, "files_timeout"},
	}

+29 −12
Changes for internal/fileedit/editor.go: 29 added lines, 12 removed lines.
Original line number Diff line number Diff line
@@ -64,6 +64,12 @@ var (
	// ErrTooLarge is a read of a file over MaxReadBytes or a write over
	// MaxWriteBytes.
	ErrTooLarge = errors.New("fileedit: file is too large for the editor")
	// ErrConflict is a write whose expected hash no longer matches: the file
	// changed after the caller read it.
	ErrConflict = errors.New("fileedit: file changed since it was read")
	// ErrNoSpace is a write the world volume had no room for; the file is
	// unchanged.
	ErrNoSpace = errors.New("fileedit: the world volume is full")
)

// Runner is the cluster-side half of one file operation: render and create the
@@ -181,7 +187,7 @@ type Editor struct {
// List returns one directory's entries, resolved under the server's world root.
// An empty path lists the world root itself.
func (e *Editor) List(ctx context.Context, server, path string) ([]Entry, bool, error) {
	res, err := e.run(ctx, server, OpList, path, nil)
	res, err := e.run(ctx, server, OpList, path, nil, "")
	if err != nil {
		return nil, false, err
	}
@@ -193,25 +199,31 @@ func (e *Editor) List(ctx context.Context, server, path string) ([]Entry, bool,
	return res.Entries, res.Truncated, nil
}

// Read returns a file's bytes, resolved under the server's world root.
func (e *Editor) Read(ctx context.Context, server, path string) ([]byte, error) {
	res, err := e.run(ctx, server, OpRead, path, nil)
// 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.
func (e *Editor) Read(ctx context.Context, server, path string) ([]byte, string, error) {
	res, err := e.run(ctx, server, OpRead, path, nil, "")
	if err != nil {
		return nil, err
		return nil, "", err
	}
	// 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 {
		res.Content = []byte{}
	}
	return res.Content, nil
	return res.Content, res.SHA256, nil
}

// Write replaces a file's contents, creating it if absent (but never creating
// parent directories — see the write helper in exec.go).
func (e *Editor) Write(ctx context.Context, server, path string, content []byte) error {
	_, err := e.run(ctx, server, OpWrite, path, content)
	return err
// Write atomically replaces a file's contents, creating it if absent (but never
// creating parent directories — see the write helper in exec.go), and returns the
// new SHA-256. A non-empty expect makes it conditional: ErrConflict if the file no
// longer hashes to it.
func (e *Editor) Write(ctx context.Context, server, path string, content []byte, expect string) (string, error) {
	res, err := e.run(ctx, server, OpWrite, path, content, expect)
	if err != nil {
		return "", err
	}
	return res.SHA256, nil
}

// run is the shared body of all three operations: mint an op id, render the
@@ -221,7 +233,7 @@ func (e *Editor) Write(ctx context.Context, server, path string, content []byte)
// That is not redundancy for its own sake: an oversized write would otherwise be
// rejected by the API SERVER (etcd's object limit) as an opaque failure, long after
// felis-api had committed to the request, instead of as a clean 413.
func (e *Editor) run(ctx context.Context, server, op, path string, content []byte) (Result, error) {
func (e *Editor) run(ctx context.Context, server, op, path string, content []byte, expect string) (Result, error) {
	if op == OpWrite && len(content) > MaxWriteBytes {
		return Result{}, fmt.Errorf("%w: content is %d bytes, the limit is %d",
			ErrTooLarge, len(content), MaxWriteBytes)
@@ -246,6 +258,7 @@ func (e *Editor) run(ctx context.Context, server, op, path string, content []byt
		Op:               op,
		Path:             path,
		Content:          content,
		Expect:           expect,
		WorldPVC:         naming.WorldPVCName(server),
		Namespace:        cfg.Namespace,
		ServiceAccount:   cfg.ServiceAccount,
@@ -284,6 +297,10 @@ func resultError(res Result) error {
		return fmt.Errorf("%w: %s", ErrBadPath, res.Error)
	case CodeTooLarge:
		return fmt.Errorf("%w: %s", ErrTooLarge, res.Error)
	case CodeConflict:
		return fmt.Errorf("%w: %s", ErrConflict, res.Error)
	case CodeNoSpace:
		return fmt.Errorf("%w: %s", ErrNoSpace, res.Error)
	default:
		return fmt.Errorf("fileedit: file operation failed (%s): %s", res.Code, res.Error)
	}
Loading