fix: make new file creation atomic and non-destructive

agent/internal/files/access_linux_test.go

@@ -13,6 +13,15 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+// A named ACL user need not exist in passwd. Keep the fixture different from
+// the current UID without overflow or producing the reserved ACL ID sentinel.
+func otherACLUserID() uint32 {
+	if os.Getuid() == 1 {
+		return 2
+	}
+	return 1
+}
+
 func TestReplacementACLRetainsOriginalOwnerWithoutBroadeningOthers(t *testing.T) {
 	for _, mode := range []uint32{0o600, 0o644} {
 		st := unix.Stat_t{Uid: 1000, Mode: mode}
@@ -66,7 +75,7 @@ func TestApplyReplacementACLOnRealFile(t *testing.T) {
 	defer func() { _ = f.Close() }()
 	// A real unprivileged file owner may install an exact-UID ACL for a
 	// different UID without CAP_CHOWN. No privileged test setup is needed.
-	access := fileAccess{exists: true, stat: unix.Stat_t{Uid: uint32(os.Getuid() + 1), Gid: uint32(os.Getgid()), Mode: 0o600}}
+	access := fileAccess{exists: true, stat: unix.Stat_t{Uid: otherACLUserID(), Gid: uint32(os.Getgid()), Mode: 0o600}}
 	if err := access.apply(f); err != nil {
 		if errors.Is(err, unix.EOPNOTSUPP) {
 			t.Skip("test filesystem lacks POSIX ACL support")
@@ -82,7 +91,7 @@ func TestApplyReplacementACLOnRealFile(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	want := []aclEntry{{aclUserObj, 6, aclUndefined}, {aclUser, 6, uint32(os.Getuid() + 1)}, {aclGroupObj, 0, aclUndefined}, {aclMask, 6, aclUndefined}, {aclOther, 0, aclUndefined}}
+	want := []aclEntry{{aclUserObj, 6, aclUndefined}, {aclUser, 6, otherACLUserID()}, {aclGroupObj, 0, aclUndefined}, {aclMask, 6, aclUndefined}, {aclOther, 0, aclUndefined}}
 	if !reflect.DeepEqual(entries, want) {
 		t.Fatalf("on-disk ACL=%v", entries)
 	}

agent/internal/files/create_linux_test.go

@@ -0,0 +1,212 @@
+package files
+
+import (
+	"bytes"
+	"errors"
+	"net/http"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"golang.org/x/sys/unix"
+)
+
+func assertNoCreateTemp(t *testing.T, dir string) {
+	t.Helper()
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, entry := range entries {
+		if strings.HasPrefix(entry.Name(), ".create-") {
+			t.Fatalf("temporary file remains: %s", entry.Name())
+		}
+	}
+}
+
+func TestCreateHTTPRefusesExistingEntries(t *testing.T) {
+	for _, kind := range []string{"file", "directory", "symlink"} {
+		t.Run(kind, func(t *testing.T) {
+			url, root := newServer(t)
+			target := filepath.Join(root, "existing")
+			switch kind {
+			case "file":
+				put(t, target, "keep this content")
+			case "directory":
+				put(t, filepath.Join(target, "child"), "keep child")
+			case "symlink":
+				// A dotfile target must still report a collision, without inspecting it.
+				put(t, filepath.Join(root, ".secret"), "keep secret")
+				if err := os.Symlink(".secret", target); err != nil {
+					t.Fatal(err)
+				}
+			}
+			var before unix.Stat_t
+			if err := unix.Lstat(target, &before); err != nil {
+				t.Fatal(err)
+			}
+			resp, err := testPost(t, url+"/files/create?path=/existing", "application/octet-stream", strings.NewReader(""))
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer resp.Body.Close()
+			if resp.StatusCode != http.StatusConflict {
+				t.Fatalf("status = %d, want 409", resp.StatusCode)
+			}
+			var after unix.Stat_t
+			if err := unix.Lstat(target, &after); err != nil || before != after {
+				t.Fatalf("existing entry metadata changed: before=%+v after=%+v err=%v", before, after, err)
+			}
+			contentPath, want := target, "keep this content"
+			switch kind {
+			case "directory":
+				contentPath, want = filepath.Join(target, "child"), "keep child"
+			case "symlink":
+				contentPath, want = filepath.Join(root, ".secret"), "keep secret"
+				if link, err := os.Readlink(target); err != nil || link != ".secret" {
+					t.Fatalf("symlink changed: %q %v", link, err)
+				}
+			}
+			got, err := os.ReadFile(contentPath)
+			if err != nil || string(got) != want {
+				t.Fatalf("content = %q, error = %v", got, err)
+			}
+			assertNoCreateTemp(t, root)
+		})
+	}
+}
+
+func TestCreateHTTPAndWriteOverwrite(t *testing.T) {
+	url, root := newServer(t)
+	for _, operation := range []struct{ path, body string }{{"create", ""}, {"write", "edited"}} {
+		resp, err := testPost(t, url+"/files/"+operation.path+"?path=/sub/new.txt", "application/octet-stream", strings.NewReader(operation.body))
+		if err != nil {
+			t.Fatal(err)
+		}
+		_ = resp.Body.Close()
+		if resp.StatusCode != http.StatusNoContent {
+			t.Fatalf("%s status = %d", operation.path, resp.StatusCode)
+		}
+		got, err := os.ReadFile(filepath.Join(root, "sub", "new.txt"))
+		if err != nil || string(got) != operation.body {
+			t.Fatalf("content = %q, error = %v", got, err)
+		}
+		info, err := os.Stat(filepath.Join(root, "sub", "new.txt"))
+		if err != nil || info.Mode().Perm() != 0o644 {
+			t.Fatalf("new-file permissions = %v, error = %v", info, err)
+		}
+		assertNoCreateTemp(t, filepath.Join(root, "sub"))
+	}
+}
+
+func TestCreateCommitCollision(t *testing.T) {
+	for _, kind := range []string{"file", "directory", "symlink"} {
+		t.Run(kind, func(t *testing.T) {
+			h, root, outside := raceHandler(t)
+			target := filepath.Join(root, "new.txt")
+			raceAt(t, h, "commit", "", "new.txt", 1, func() {
+				switch kind {
+				case "file":
+					put(t, target, "concurrent creator")
+				case "directory":
+					if err := os.Mkdir(target, 0o700); err != nil {
+						t.Fatal(err)
+					}
+				case "symlink":
+					if err := os.Symlink(filepath.Join(outside, "secret.txt"), target); err != nil {
+						t.Fatal(err)
+					}
+				}
+			})
+			rr := serve(t, h.create, http.MethodPost, "/files/create?path=/new.txt", strings.NewReader("new body"))
+			if rr.Code != http.StatusConflict || !strings.Contains(rr.Body.String(), "already exists") {
+				t.Fatalf("status=%d body=%q", rr.Code, rr.Body.String())
+			}
+			info, err := os.Lstat(target)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if kind == "file" {
+				got, err := os.ReadFile(target)
+				if err != nil || string(got) != "concurrent creator" {
+					t.Fatalf("concurrent content=%q err=%v", got, err)
+				}
+			} else if (kind == "directory" && !info.IsDir()) || (kind == "symlink" && info.Mode()&os.ModeSymlink == 0) {
+				t.Fatalf("concurrent entry replaced: %v", info)
+			}
+			assertNoCreateTemp(t, root)
+			assertOutsideUntouched(t, outside)
+		})
+	}
+}
+
+func TestCreateFailureDoesNotPublish(t *testing.T) {
+	for _, failure := range []string{"body", "metadata", "body limit"} {
+		t.Run(failure, func(t *testing.T) {
+			h, root, _ := raceHandler(t)
+			body := &errAfterReader{n: 4}
+			switch failure {
+			case "metadata":
+				h.preserveAccess = func(*os.File, fileAccess) error { return errors.New("metadata failure") }
+				rr := serve(t, h.create, http.MethodPost, "/files/create?path=/new.txt", strings.NewReader("payload"))
+				if rr.Code != http.StatusConflict {
+					t.Fatalf("status=%d", rr.Code)
+				}
+			case "body limit":
+				rr := serve(t, h.create, http.MethodPost, "/files/create?path=/new.txt", bytes.NewReader(bytes.Repeat([]byte("a"), maxWriteBytes+1)))
+				if rr.Code != http.StatusRequestEntityTooLarge {
+					t.Fatalf("oversized body status=%d", rr.Code)
+				}
+			default:
+				rr := serve(t, h.create, http.MethodPost, "/files/create?path=/new.txt", body)
+				if rr.Code != http.StatusInternalServerError {
+					t.Fatalf("failed body status=%d", rr.Code)
+				}
+			}
+			if _, err := os.Lstat(filepath.Join(root, "new.txt")); !errors.Is(err, os.ErrNotExist) {
+				t.Fatalf("target published: %v", err)
+			}
+			assertNoCreateTemp(t, root)
+		})
+	}
+}
+
+func TestCreateRemovesInheritedAccessACL(t *testing.T) {
+	h, root, _ := raceHandler(t)
+	acl := encodeACL([]aclEntry{{aclUserObj, 7, aclUndefined}, {aclUser, 4, otherACLUserID()}, {aclGroupObj, 0, aclUndefined}, {aclMask, 4, aclUndefined}, {aclOther, 0, aclUndefined}})
+	if err := unix.Setxattr(root, "system.posix_acl_default", acl, 0); err != nil {
+		if errors.Is(err, unix.EOPNOTSUPP) {
+			t.Skip("test filesystem lacks POSIX ACL support")
+		}
+		t.Fatal(err)
+	}
+	rr := serve(t, h.create, http.MethodPost, "/files/create?path=/new.txt", strings.NewReader(""))
+	if rr.Code != http.StatusNoContent {
+		t.Fatalf("status=%d body=%q", rr.Code, rr.Body.String())
+	}
+	if _, err := unix.Getxattr(filepath.Join(root, "new.txt"), accessACLName, nil); !errors.Is(err, unix.ENODATA) {
+		t.Fatalf("inherited access ACL remains: %v", err)
+	}
+}
+
+func TestCreateConfinedPaths(t *testing.T) {
+	h, root, outside := raceHandler(t)
+	for _, tc := range []struct {
+		path   string
+		status int
+	}{{"/", http.StatusConflict}, {"/.hidden", http.StatusBadRequest}, {"/sub/.hidden/new", http.StatusBadRequest}} {
+		rr := serve(t, h.create, http.MethodPost, "/files/create?path="+tc.path, strings.NewReader(""))
+		if rr.Code != tc.status {
+			t.Fatalf("path=%s status=%d body=%q", tc.path, rr.Code, rr.Body.String())
+		}
+	}
+	if err := os.Mkdir(filepath.Join(root, "sub"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	raceAt(t, h, "open", "", "sub", 1, func() { swapForSymlink(t, filepath.Join(root, "sub"), outside) })
+	rr := serve(t, h.create, http.MethodPost, "/files/create?path=/sub/new.txt", strings.NewReader(""))
+	assertRejected(t, rr)
+	assertOutsideUntouched(t, outside)
+	assertNoCreateTemp(t, root)
+}

agent/internal/files/files.go

@@ -35,6 +35,9 @@ var errPathOutOfRoot = errors.New("path escapes root")
 // part of the file-browser surface. Like errPathOutOfRoot it is safe to echo.
 var errDotfile = errors.New("dotfile access denied")
 
+// errFileExists is a client-safe conflict for create-only operations.
+var errFileExists = errors.New("file already exists")
+
 // hasDotComponent reports whether the slash-separated relative path rel
 // (already stripped of its leading slash) has a dot-prefixed component.
 func hasDotComponent(rel string) bool {
@@ -67,6 +70,7 @@ func Mount(r chi.Router, root string) {
 			bounded.Get("/list", h.list)
 			bounded.Get("/read", h.read)
 			bounded.Post("/write", h.write)
+			bounded.Post("/create", h.create)
 			bounded.Post("/mkdir", h.mkdir)
 			bounded.Delete("/delete", h.del)
 		})
@@ -129,7 +133,7 @@ func (h *handler) badRequest(w http.ResponseWriter, err error) {
 
 // fail maps an error from path validation or a descriptor walk to a response:
 // bad-path classes are 400s via badRequest, everything else goes through
-// httpErr (404/403/500).
+// httpErr (404/403/409/413/500).
 func (h *handler) fail(w http.ResponseWriter, err error) {
 	if errors.Is(err, errPathOutOfRoot) || errors.Is(err, errDotfile) || errors.Is(err, errBadPath) {
 		h.badRequest(w, err)
@@ -207,7 +211,7 @@ func (h *handler) download(w http.ResponseWriter, req *http.Request) {
 	http.ServeContent(w, req, fi.Name(), fi.ModTime(), f)
 }
 
-// maxWriteBytes caps a single /files/write body. The API-side ws proxy
+// maxWriteBytes caps a single /files/write or /files/create body. The API-side ws proxy
 // already enforces 64 MiB, so this is defense-in-depth against direct
 // agent access with a valid mTLS cert.
 const maxWriteBytes = 64 << 20
@@ -224,6 +228,18 @@ const maxUploadFileBytes = 64 << 20
 const maxUploadFiles = 64
 
 func (h *handler) write(w http.ResponseWriter, req *http.Request) {
+	h.storeRequest(w, req, false)
+}
+
+// create publishes a new file only if the destination remains absent; an
+// existing entry returns 409, and an oversized body returns 413 without publishing.
+func (h *handler) create(w http.ResponseWriter, req *http.Request) {
+	h.storeRequest(w, req, true)
+}
+
+// storeRequest shares the bounded, atomic write path for create and replace.
+// Copy failures leave the destination untouched; body-limit failures are 413.
+func (h *handler) storeRequest(w http.ResponseWriter, req *http.Request, createOnly bool) {
 	comps, err := h.components(req.URL.Query().Get("path"))
 	if err != nil {
 		h.fail(w, err)
@@ -235,7 +251,11 @@ func (h *handler) write(w http.ResponseWriter, req *http.Request) {
 	// only on success, so any error after the copy starts (ENOSPC, a body that
 	// ends early, an agent restart mid-copy) leaves the previous file intact
 	// (F-102). The whole sequence runs through one parent directory descriptor.
-	err = h.writeFile(comps, func(dst io.Writer) error {
+	store := h.writeFile
+	if createOnly {
+		store = h.createFile
+	}
+	err = store(comps, func(dst io.Writer) error {
 		_, copyErr := io.Copy(dst, body)
 		return copyErr
 	})
@@ -344,7 +364,7 @@ func (h *handler) storePart(comps []string, filename string, src io.Reader, limi
 	if strings.HasPrefix(name, ".") {
 		return errDotfile
 	}
-	return h.storeIn(comps, name, ".upload-", false, func(dst io.Writer) error {
+	return h.storeIn(comps, name, ".upload-", false, storeReplace, func(dst io.Writer) error {
 		// Read one byte past the cap: if that byte materializes the part is
 		// over the limit, whatever its multipart headers claimed.
 		n, err := io.Copy(dst, io.LimitReader(src, limit+1))
@@ -391,8 +411,15 @@ func (h *handler) del(w http.ResponseWriter, req *http.Request) {
 	w.WriteHeader(http.StatusNoContent)
 }
 
+// httpErr maps storage errors to safe HTTP responses, including wrapped body
+// limits as 413. Internal error details are logged rather than returned.
 func httpErr(w http.ResponseWriter, err error) {
+	var tooLarge *http.MaxBytesError
 	switch {
+	case errors.As(err, &tooLarge):
+		http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
+	case errors.Is(err, errFileExists):
+		http.Error(w, errFileExists.Error(), http.StatusConflict)
 	case errors.Is(err, errPreserveAccess):
 		http.Error(w, "cannot preserve existing file access; the file changed or the filesystem does not support its group/POSIX ACL permissions", http.StatusConflict)
 	case errors.Is(err, os.ErrNotExist):

agent/internal/files/files_gaps_test.go

@@ -34,8 +34,8 @@ func TestWrite_BodyTooLarge(t *testing.T) {
 		t.Fatalf("post: %v", err)
 	}
 	defer resp.Body.Close()
-	if resp.StatusCode == http.StatusNoContent {
-		t.Fatal("oversize write should not succeed")
+	if resp.StatusCode != http.StatusRequestEntityTooLarge {
+		t.Fatalf("oversized write status=%d, want 413", resp.StatusCode)
 	}
 }
 

agent/internal/files/files_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"mime/multipart"
 	"net/http"
@@ -927,6 +928,19 @@ func TestHttpErr_PermissionMappedTo403(t *testing.T) {
 	}
 }
 
+func TestHttpErr_BodyLimitMappedTo413(t *testing.T) {
+	for _, err := range []error{
+		&http.MaxBytesError{Limit: maxWriteBytes},
+		fmt.Errorf("write /private/data/file: %w", &http.MaxBytesError{Limit: maxWriteBytes}),
+	} {
+		rr := httptest.NewRecorder()
+		httpErr(rr, err)
+		if rr.Code != http.StatusRequestEntityTooLarge || rr.Body.String() != "request body too large\n" {
+			t.Fatalf("body limit response: status=%d body=%q", rr.Code, rr.Body.String())
+		}
+	}
+}
+
 func TestHttpErr_GenericMappedTo500(t *testing.T) {
 	rr := httptest.NewRecorder()
 	httpErr(rr, errors.New("disk on fire"))