Let gitea_commit take a large patch from a hash-checked spool file (issue 852)

A feature's diff is megabytes, too large to type into a call. gitea_commit now also takes patch_file, the
random hex name of a file in the module's spool directory /var/lib/mesh/forge-patches (mode 0733, owned by
the machine's account), with patch_sha256. The tool reads only that directory, never through a symlink or a
hard link, refuses a file whose SHA-256 differs, applies it as it applies patch, and removes it.

Co-Authored-By: Claude <noreply@anthropic.com>

modules/gitea/README.md

@@ -44,7 +44,7 @@ base64-encoded. The Go bundle `cmd/forge` serves the commit, and the reads an ag
 
 | tool | | what |
 |---|---|---|
-| `gitea_commit` | a | one commit on a branch of its own, created from `base` (default the default branch) when absent, carrying either `files` (each `{path, content \| content_base64 \| delete: true}`) or `patch` (a unified diff, as `git diff` writes it); answers the commit and its parent |
+| `gitea_commit` | a | one commit on a branch of its own, created from `base` (default the default branch) when absent, carrying either `files` (each `{path, content \| content_base64 \| delete: true}`) `patch` (a unified diff, as `git diff` writes it), or `patch_file` with `patch_sha256` (a patch too large to type, see below); answers the commit and its parent |
 | `gitea_commits` | r | a ref's history, newest first, optionally only commits touching a path: sha, author, date, subject |
 | `gitea_compare` | r | what head holds that base does not: its commits and the files they touch |
 | `gitea_pull_reviews` | r | a pull request's reviews — state, the commit reviewed and whether it is the head now, stale, dismissed — with their line comments |
@@ -62,6 +62,18 @@ the forge cannot compile covers its own text. `cmd/forge/rules.go` and `guard.ts
 commit. Nothing about merging changes: a merge still needs an independent review at the head and both
 required checks green, through `gitea_merge_pull_request`.
 
+**A patch too large to type** (novox/hq issue 852: a feature's diff is megabytes) goes as `patch_file`, with
+`patch_sha256`. The caller writes the patch into the module's spool directory `forge-patches`
+(`/var/lib/mesh/forge-patches`, mode 0733 and owned by the machine's account, which the tools run as: anyone writes,
+nobody lists; the bundle is given its path as `MESH_GITEA_PATCH_DIR`) and names it: `patch_file` is the file's NAME,
+32 to 64 characters of `0-9a-f` that the caller chooses at random, never a path. The tool then reads that one file,
+applies it exactly as it applies `patch` (same size limit, same answer) and removes it once it has found it, whether
+the commit was made or not. A file refused before that (a bad name, a missing hash or spool directory, a name with no
+file) stays in the spool. It refuses, sending nothing, a name of another shape, a file that is missing, empty, over
+4 MiB, with more than one hard link or not a regular file (a symlink is never followed), and a file whose SHA-256 is not `patch_sha256`: the directory has no sticky bit
+(the manifest allows none), so another writer could replace a file, and the hash is how the caller knows the tool read
+the bytes it wrote. It reads nothing outside the directory.
+
 **How the commit is made.** `files` become one call to the forge's multi-file contents endpoint: a path the
 ref holds is updated, one it does not is created, and an update or delete carries the blob read just
 before, so a file someone changed meanwhile is refused by the forge rather than overwritten. The blobs are read at

modules/gitea/cmd/forge/commit.go

@@ -35,6 +35,11 @@ type CommitAsk struct {
 	Message     string
 	Files       []FileChange
 	Patch       string // a unified diff, as git diff writes it, instead of Files
+	// PatchFile is the name of a file in the spool directory holding the patch (novox/hq issue 852), with the
+	// SHA-256 its caller says it has; commitAsk reads it into Patch.
+	PatchFile      string
+	PatchSHA256    string
+	patchFileGiven bool
 }
 
 // Committed is what gitea_commit made.
@@ -69,6 +74,11 @@ func readCommitAsk(args map[string]any) (CommitAsk, error) {
 	a := CommitAsk{Owner: text(args, "owner"), Repo: text(args, "repo"), Branch: text(args, "branch"),
 		Base: text(args, "base"), Message: strings.TrimSpace(raw(args, "message"))}
 	a.Patch = raw(args, "patch")
+	if v, ok := args["patch_file"]; ok && v != nil {
+		a.patchFileGiven = true
+		a.PatchFile = raw(args, "patch_file")
+	}
+	a.PatchSHA256 = strings.ToLower(strings.TrimSpace(raw(args, "patch_sha256")))
 	if a.Branch == "" {
 		return a, refuse("branch is required: the branch the commit goes on, a branch of its own (created from base when absent). Nothing was sent")
 	}
@@ -111,11 +121,23 @@ func readCommitAsk(args map[string]any) (CommitAsk, error) {
 			a.Files = append(a.Files, fc)
 		}
 	}
+	sources := 0
+	for _, g := range []bool{len(a.Files) > 0, strings.TrimSpace(a.Patch) != "", a.patchFileGiven} {
+		if g {
+			sources++
+		}
+	}
 	switch {
-	case len(a.Files) == 0 && strings.TrimSpace(a.Patch) == "":
-		return a, refuse("give files (each {path, content | content_base64 | delete: true}) or patch (a unified diff, as git diff writes it). Nothing was sent")
-	case len(a.Files) > 0 && strings.TrimSpace(a.Patch) != "":
-		return a, refuse("give files or patch, not both: one commit carries one of them. Nothing was sent")
+	case sources == 0:
+		return a, refuse("give files (each {path, content | content_base64 | delete: true}), patch (a unified diff, as git diff writes it) or patch_file (the name of a patch in the spool directory). Nothing was sent")
+	case sources > 1:
+		return a, refuse("give files, patch or patch_file, not more than one: one commit carries one of them. Nothing was sent")
+	case a.PatchSHA256 != "" && !a.patchFileGiven:
+		return a, refuse("patch_sha256 goes with patch_file only. Nothing was sent")
+	case a.patchFileGiven:
+		if err := checkPatchFileAsk(a); err != nil {
+			return a, err
+		}
 	case len(a.Files) > maxFiles:
 		return a, refuse("%d files is more than the %d one commit carries. Nothing was sent", len(a.Files), maxFiles)
 	case len(a.Patch) > maxPatchBytes:
@@ -134,13 +156,13 @@ func readCommitAsk(args map[string]any) (CommitAsk, error) {
 			return a, refuse("%s is named twice in files. Nothing was sent", fc.Path)
 		}
 		seen[fc.Path] = true
-		given := 0
+		have := 0
 		for _, g := range []bool{fc.Content != nil, fc.ContentBase64 != nil, fc.Delete} {
 			if g {
-				given++
+				have++
 			}
 		}
-		if given != 1 {
+		if have != 1 {
 			return a, refuse("files[%d] (%s) needs exactly one of content, content_base64 or delete: true. Nothing was sent", i, fc.Path)
 		}
 		if fc.Content != nil && !utf8.ValidString(*fc.Content) {

modules/gitea/cmd/forge/main.go

@@ -55,7 +55,9 @@ func Tools(forge func() (*Forge, error)) []stdio.Tool {
 				"the branch holds them — or patch, a unified diff as git diff writes it, which the forge applies. " +
 				"At most 200 files, 4 MiB each and 16 MiB together, or a 4 MiB patch. Answers the commit it made and " +
 				"the parent the forge made it on, read back from the branch, and says when the branch moved under " +
-				"it. Replaces git commit and git " +
+				"it. A patch too large to type goes as patch_file with patch_sha256: the name of a file in the " +
+				"spool directory (read once, applied as patch is, removed afterwards, never read from anywhere " +
+				"else). Replaces git commit and git " +
 				"push of a feature branch. (a)",
 			Input: map[string]any{
 				"type": "object",
@@ -79,7 +81,9 @@ func Tools(forge func() (*Forge, error)) []stdio.Tool {
 							"required": []string{"path"},
 						},
 					},
-					"patch": str("a unified diff of the change, as git diff or git format-patch writes it; give this or files"),
+					"patch":        str("a unified diff of the change, as git diff or git format-patch writes it; give this, patch_file or files"),
+					"patch_file":   str("for a patch too large to type: the NAME (32 to 64 characters of 0-9a-f, chosen at random by the caller, no path) of a file the caller wrote into the forge patch spool directory (the module's forge-patches directory); needs patch_sha256; give this, patch or files"),
+					"patch_sha256": str("with patch_file: the file's SHA-256, in hex; the commit is refused, and nothing is sent, when the file's differs"),
 				},
 				"required": []string{"owner", "repo", "branch", "message"},
 			},
@@ -88,7 +92,7 @@ func Tools(forge func() (*Forge, error)) []stdio.Tool {
 				if err != nil {
 					return nil, err
 				}
-				return commit(ctx, f, a)
+				return commitAsk(ctx, f, os.Getenv(patchDirEnv), a)
 			}),
 		},
 		{

modules/gitea/cmd/forge/patchfile.go

@@ -0,0 +1,109 @@
+package main
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"errors"
+	"io"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"syscall"
+)
+
+// A patch too large to type into a call is handed over as a file (novox/hq issue 852). The caller writes it into
+// one spool directory the module declares (mode 0733: anyone writes, nobody lists) and names it in patch_file,
+// with the file's SHA-256. The tool runs as the machine's account, so it reads nothing but that directory: the
+// name is 32 to 64 characters of [0-9a-f], so it holds no separator and no dot; the file must be a regular file
+// (never a symlink); and its hash must be the caller's, because without the sticky bit another writer could have
+// replaced it. The file is removed once taken, whether the commit was made or not.
+
+const patchDirEnv = "MESH_GITEA_PATCH_DIR"
+
+var (
+	patchFileName = regexp.MustCompile(`^[0-9a-f]{32,64}$`)
+	patchSHA      = regexp.MustCompile(`^[0-9a-f]{64}$`)
+)
+
+// checkPatchFileAsk judges the arguments of a patch_file ask, before any file is looked at.
+func checkPatchFileAsk(a CommitAsk) error {
+	if !patchFileName.MatchString(a.PatchFile) {
+		return refuse("patch_file %q is not a name: 32 to 64 characters of 0-9a-f, no path (the file's name in the spool directory, chosen at random). Nothing was sent", a.PatchFile)
+	}
+	if !patchSHA.MatchString(a.PatchSHA256) {
+		return refuse("patch_file needs patch_sha256: the file's SHA-256, 64 hex characters. Nothing was sent")
+	}
+	return nil
+}
+
+// commitAsk makes the commit the ask names; a patch_file is first read from the spool directory into the patch
+// and removed afterwards.
+func commitAsk(ctx context.Context, f *Forge, spool string, a CommitAsk) (*Committed, error) {
+	if !a.patchFileGiven {
+		return commit(ctx, f, a)
+	}
+	if err := checkPatchFileAsk(a); err != nil {
+		return nil, err
+	}
+	if spool == "" {
+		return nil, refuse("patch_file is not available: %s is not set, so there is no spool directory. Nothing was sent", patchDirEnv)
+	}
+	path := filepath.Join(spool, a.PatchFile)
+	defer func() {
+		// Whatever happened, the file is not left: removing a symlink removes the link, never its target.
+		if st, err := os.Lstat(path); err == nil && !st.IsDir() {
+			_ = os.Remove(path)
+		}
+	}()
+	body, err := readSpooled(path, a.PatchFile)
+	if err != nil {
+		return nil, err
+	}
+	got := sha256.Sum256(body)
+	if hex.EncodeToString(got[:]) != a.PatchSHA256 {
+		return nil, refuse("the file %s is not the patch the caller hashed: its SHA-256 is %s, patch_sha256 says %s. Nothing was sent",
+			a.PatchFile, hex.EncodeToString(got[:]), a.PatchSHA256)
+	}
+	a.Patch = string(body)
+	return commit(ctx, f, a)
+}
+
+// readSpooled reads a regular file of at most maxPatchBytes, never through a symlink.
+func readSpooled(path, name string) ([]byte, error) {
+	st, err := os.Lstat(path)
+	switch {
+	case errors.Is(err, os.ErrNotExist):
+		return nil, refuse("patch_file %s: no such file in the spool directory. Nothing was sent", name)
+	case err != nil:
+		return nil, refuse("patch_file %s cannot be read: %v. Nothing was sent", name, err)
+	case !st.Mode().IsRegular():
+		return nil, refuse("patch_file %s is not a regular file (a symlink or a directory is never followed). Nothing was sent", name)
+	case st.Size() > maxPatchBytes:
+		return nil, refuse("patch_file %s is %s, more than the %s one commit carries: split it into several commits. Nothing was sent", name, mib(int(st.Size())), mib(maxPatchBytes))
+	}
+	fh, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
+	if err != nil {
+		return nil, refuse("patch_file %s cannot be opened: %v. Nothing was sent", name, err)
+	}
+	defer fh.Close()
+	fst, err := fh.Stat()
+	if err != nil || !fst.Mode().IsRegular() {
+		return nil, refuse("patch_file %s is not a regular file. Nothing was sent", name)
+	}
+	// A hard link to a file elsewhere is no symlink, but it is not the caller's own file either.
+	if sys, ok := fst.Sys().(*syscall.Stat_t); !ok || sys.Nlink > 1 {
+		return nil, refuse("patch_file %s has more than one hard link: it must be a file of its own. Nothing was sent", name)
+	}
+	body, err := io.ReadAll(io.LimitReader(fh, maxPatchBytes+1))
+	switch {
+	case err != nil:
+		return nil, refuse("patch_file %s cannot be read: %v. Nothing was sent", name, err)
+	case len(body) > maxPatchBytes:
+		return nil, refuse("patch_file %s is more than the %s one commit carries: split it into several commits. Nothing was sent", name, mib(maxPatchBytes))
+	case strings.TrimSpace(string(body)) == "":
+		return nil, refuse("patch_file %s is empty. Nothing was sent", name)
+	}
+	return body, nil
+}

modules/gitea/cmd/forge/patchfile_test.go

@@ -0,0 +1,183 @@
+package main
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// novox/hq issue 852: a patch too large to type into a call is handed over as a file in one spool directory.
+
+const spoolName = "0123456789abcdef0123456789abcdef"
+const spoolPatch = "diff --git a/README.md b/README.md\n--- a/README.md\n+++ b/README.md\n@@ -1 +1 @@\n-hello\n+hi\n"
+
+func sum(s string) string { h := sha256.Sum256([]byte(s)); return hex.EncodeToString(h[:]) }
+
+func spoolWith(t *testing.T, name, content string) string {
+	t.Helper()
+	dir := t.TempDir()
+	if name != "" {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	return dir
+}
+
+func fileAsk(t *testing.T, name, sha string) CommitAsk {
+	t.Helper()
+	return ask(t, map[string]any{"branch": "fix/9", "patch_file": name, "patch_sha256": sha})
+}
+
+func TestAPatchFileIsAppliedAsThePatchWouldBeAndTheFileIsGone(t *testing.T) {
+	f := newFake(t)
+	dir := spoolWith(t, spoolName, spoolPatch)
+	got, err := commitAsk(context.Background(), f.serve(), dir, fileAsk(t, spoolName, sum(spoolPatch)))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(f.patches) != 1 || f.patches[0]["content"] != spoolPatch || f.patches[0]["new_branch"] != "fix/9" || got.Commit != "p1" {
+		t.Fatalf("%v %+v", f.patches, got)
+	}
+	if _, err := os.Lstat(filepath.Join(dir, spoolName)); !os.IsNotExist(err) {
+		t.Fatalf("the file is still there: %v", err)
+	}
+}
+
+func TestAPatchFileIsRemovedWhenTheForgeRefusesIt(t *testing.T) {
+	f := newFake(t)
+	dir := spoolWith(t, spoolName, spoolPatch)
+	// the default branch is refused after the file was read
+	a := ask(t, map[string]any{"branch": "main", "patch_file": spoolName, "patch_sha256": sum(spoolPatch)})
+	if _, err := commitAsk(context.Background(), f.serve(), dir, a); err == nil {
+		t.Fatal("taken")
+	}
+	if _, err := os.Lstat(filepath.Join(dir, spoolName)); !os.IsNotExist(err) {
+		t.Fatalf("the file is still there: %v", err)
+	}
+}
+
+func TestAPatchFileThatCannotBeTakenIsRefusedBeforeAnythingIsSent(t *testing.T) {
+	outside := filepath.Join(t.TempDir(), "secret")
+	if err := os.WriteFile(outside, []byte(spoolPatch), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	type tc struct {
+		name  string
+		setup func(dir string)
+		file  string
+		sha   string
+		want  string
+	}
+	for _, c := range []tc{
+		{"wrong sha", func(d string) { _ = os.WriteFile(filepath.Join(d, spoolName), []byte(spoolPatch), 0o600) }, spoolName, sum("other"), "sha256"},
+		{"not found", func(string) {}, spoolName, sum(spoolPatch), "no such file"},
+		{"symlink", func(d string) { _ = os.Symlink(outside, filepath.Join(d, spoolName)) }, spoolName, sum(spoolPatch), "regular file"},
+		{"hard link", func(d string) { _ = os.Link(outside, filepath.Join(d, spoolName)) }, spoolName, sum(spoolPatch), "hard link"},
+		{"directory", func(d string) { _ = os.Mkdir(filepath.Join(d, spoolName), 0o700) }, spoolName, sum(spoolPatch), "regular file"},
+		{"too large", func(d string) {
+			_ = os.WriteFile(filepath.Join(d, spoolName), []byte(strings.Repeat("x", maxPatchBytes+1)), 0o600)
+		}, spoolName, sum(spoolPatch), "MiB"},
+		{"empty", func(d string) { _ = os.WriteFile(filepath.Join(d, spoolName), nil, 0o600) }, spoolName, sum(""), "empty"},
+	} {
+		f := newFake(t)
+		dir := t.TempDir()
+		c.setup(dir)
+		_, err := commitAsk(context.Background(), f.serve(), dir, fileAsk(t, c.file, c.sha))
+		if err == nil || !strings.Contains(err.Error(), c.want) || !strings.Contains(err.Error(), "Nothing was sent") {
+			t.Errorf("%s: %v", c.name, err)
+		}
+		if len(f.patches)+len(f.writes) != 0 {
+			t.Errorf("%s: something was sent", c.name)
+		}
+	}
+	if b, err := os.ReadFile(outside); err != nil || string(b) != spoolPatch {
+		t.Errorf("the file outside the spool was touched: %v", err)
+	}
+}
+
+func TestAPatchFileAskThatCannotBeReadIsRefused(t *testing.T) {
+	ok := sum(spoolPatch)
+	for name, args := range map[string]map[string]any{
+		"no sha":         {"patch_file": spoolName},
+		"sha not hex":    {"patch_file": spoolName, "patch_sha256": strings.Repeat("z", 64)},
+		"sha too short":  {"patch_file": spoolName, "patch_sha256": ok[:63]},
+		"empty name":     {"patch_file": "", "patch_sha256": ok, "files": filesArg()},
+		"dotdot":         {"patch_file": "../" + spoolName, "patch_sha256": ok},
+		"slash":          {"patch_file": "a/" + spoolName, "patch_sha256": ok},
+		"absolute":       {"patch_file": "/etc/passwd", "patch_sha256": ok},
+		"too short":      {"patch_file": strings.Repeat("a", 31), "patch_sha256": ok},
+		"too long":       {"patch_file": strings.Repeat("a", 65), "patch_sha256": ok},
+		"upper case":     {"patch_file": strings.ToUpper(spoolName), "patch_sha256": ok},
+		"not hex":        {"patch_file": strings.Repeat("g", 32), "patch_sha256": ok},
+		"with patch":     {"patch_file": spoolName, "patch_sha256": ok, "patch": "diff"},
+		"with files":     {"patch_file": spoolName, "patch_sha256": ok, "files": filesArg(map[string]any{"path": "x", "content": "x"})},
+		"sha without it": {"patch": "diff", "patch_sha256": ok},
+	} {
+		full := map[string]any{"owner": "acme", "repo": "app", "message": "m", "branch": "b"}
+		for k, v := range args {
+			full[k] = v
+		}
+		_, err := readCommitAsk(full)
+		if err == nil || !strings.Contains(err.Error(), "Nothing was sent") {
+			t.Errorf("%s: %v", name, err)
+		}
+	}
+	// 64 characters is taken; so is an upper-case sha.
+	for _, n := range []string{spoolName, strings.Repeat("ab", 32)} {
+		if _, err := readCommitAsk(map[string]any{"owner": "acme", "repo": "app", "message": "m", "branch": "b",
+			"patch_file": n, "patch_sha256": strings.ToUpper(sum(spoolPatch))}); err != nil {
+			t.Errorf("%s: %v", n, err)
+		}
+	}
+}
+
+func TestAPatchFileWithoutASpoolDirectoryIsRefused(t *testing.T) {
+	f := newFake(t)
+	_, err := commitAsk(context.Background(), f.serve(), "", fileAsk(t, spoolName, sum(spoolPatch)))
+	if err == nil || !strings.Contains(err.Error(), "MESH_GITEA_PATCH_DIR") || !strings.Contains(err.Error(), "Nothing was sent") {
+		t.Fatalf("%v", err)
+	}
+}
+
+func TestTheManifestDeclaresTheSpoolDirectoryAndGivesItToTheBundle(t *testing.T) {
+	raw, err := os.ReadFile("../../module.json")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var m struct {
+		Resources []struct {
+			ID, Type, Path, Mode, Owner string
+		} `json:"resources"`
+		Build struct {
+			Artifacts []struct {
+				From string            `json:"from"`
+				Env  map[string]string `json:"env"`
+			} `json:"artifacts"`
+		} `json:"build"`
+	}
+	if err := jsonInto(raw, &m); err != nil {
+		t.Fatal(err)
+	}
+	found := false
+	for _, r := range m.Resources {
+		if r.ID == "forge-patches" {
+			found = true
+			if r.Type != "directory" || r.Mode != "0733" || r.Owner != "${machine:account}" || r.Path == "" {
+				t.Errorf("%+v", r)
+			}
+		}
+	}
+	if !found {
+		t.Error("no forge-patches directory")
+	}
+	for _, a := range m.Build.Artifacts {
+		if a.From == "cmd/forge" && a.Env["MESH_GITEA_PATCH_DIR"] != "${dir:forge-patches}" {
+			t.Errorf("env %v", a.Env)
+		}
+	}
+}

modules/gitea/module.json

@@ -184,6 +184,13 @@
       "trusted": true,
       "content": "# The accounts whose \"ready at <sha>\" comment marks a pull request ready (novox/hq issue 722), from the setting\n# ready-authors, written by the mesh. Replaced on every push; change the setting, never this file.\nauthors=${setting:ready-authors}\n"
     },
+    {
+      "id": "forge-patches",
+      "type": "directory",
+      "path": "/var/lib/mesh/forge-patches",
+      "mode": "0733",
+      "owner": "${machine:account}"
+    },
     {
       "id": "state",
       "type": "directory",
@@ -347,7 +354,8 @@
         "env": {
           "MESH_GITEA_URL": "http://127.0.0.1:${port:3000}",
           "MESH_GITEA_TOOLS_USER": "mesh-tools",
-          "MESH_GITEA_TOKEN_FILE": "${dir:runtime-state}/token"
+          "MESH_GITEA_TOKEN_FILE": "${dir:runtime-state}/token",
+          "MESH_GITEA_PATCH_DIR": "${dir:forge-patches}"
         }
       }
     ]