From 0cfb9c910423cc773f3a8e18a6d335e876328130 Mon Sep 17 00:00:00 2001 From: Axenos-dev Date: Fri, 7 Aug 2026 10:05:10 -0700 Subject: [PATCH] Add move operation for commits --- README.md | 22 + internal/domain/commit.go | 1 + internal/gitbackend/backend.go | 2 +- internal/gitbackend/errors.go | 1 + internal/gitbackend/local.go | 430 ------------- internal/gitbackend/local_commit.go | 599 ++++++++++++++++++ internal/gitbackend/local_test.go | 113 +++- internal/gitbackend/types.go | 14 +- .../server/control/repositories/commits.go | 5 +- .../control/repositories/handlers_test.go | 26 +- internal/server/control/repositories/types.go | 17 +- internal/server/response/codes.go | 1 + internal/services/repositories/errors.go | 1 + internal/services/repositories/service.go | 42 +- .../services/repositories/service_test.go | 30 +- 15 files changed, 833 insertions(+), 471 deletions(-) create mode 100644 internal/gitbackend/local_commit.go diff --git a/README.md b/README.md index d814642..deb1135 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,28 @@ or a repository-scoped LFS object already uploaded and verified through the LFS `blobSha` and `lfs` are mutually exclusive. `executable` is optional for puts. A `delete` operation takes only `op` and `path`. +A `move` relocates a file or a whole directory tree: + +```json +{ + "op": "move", + "fromPath": "plugins", + "path": "server/plugins" +} +``` + +Operations are applied in array order. After the move, later operations in the same request use the destination path: + +```json +{ + "operations": [ + { "op": "move", "fromPath": "plugins", "path": "server/plugins" }, + { "op": "put", "path": "server/plugins/config.yml", "blobSha": "" }, + { "op": "delete", "path": "server/plugins/something.yaml" } + ] +} +``` + `expectedHeadSha` controls concurrency: | Value | Meaning | diff --git a/internal/domain/commit.go b/internal/domain/commit.go index c1c4666..d670eae 100644 --- a/internal/domain/commit.go +++ b/internal/domain/commit.go @@ -23,6 +23,7 @@ type CommitFileLfsObject struct { type CommitFileOp struct { Delete bool + MoveFrom string Path string BlobSHA *string Lfs *CommitFileLfsObject diff --git a/internal/gitbackend/backend.go b/internal/gitbackend/backend.go index 72a1f50..a7bf298 100644 --- a/internal/gitbackend/backend.go +++ b/internal/gitbackend/backend.go @@ -20,6 +20,6 @@ type Backend interface { StatBlob(ctx context.Context, storagePath, rev, treePath string) (BlobInfo, error) ReadBlob(ctx context.Context, storagePath, blobSHA string, out io.Writer) error WriteBlob(ctx context.Context, storagePath string, r io.Reader) (string, int64, error) - ApplyCommit(ctx context.Context, storagePath string, spec CommitSpec, ops []CommitOp, clean CleanFunc) (RefChange, error) + ApplyCommit(ctx context.Context, storagePath string, spec CommitSpec, ops []CommitOp, clean CleanFunc, checkWrite CheckWriteFunc) (RefChange, error) GC(ctx context.Context, storagePath string) error } diff --git a/internal/gitbackend/errors.go b/internal/gitbackend/errors.go index 757f055..95d799b 100644 --- a/internal/gitbackend/errors.go +++ b/internal/gitbackend/errors.go @@ -7,6 +7,7 @@ var ( ErrInvalidPath = errors.New("invalid tree path") ErrRevNotFound = errors.New("revision not found") ErrPathNotFound = errors.New("path not found in tree") + ErrPathExists = errors.New("path already exists in tree") ErrNotABlob = errors.New("path is not a blob") // commit creation diff --git a/internal/gitbackend/local.go b/internal/gitbackend/local.go index c2feabe..c33d9b7 100644 --- a/internal/gitbackend/local.go +++ b/internal/gitbackend/local.go @@ -480,436 +480,6 @@ func (l *Local) GC(ctx context.Context, storagePath string) error { return nil } -// creates a commit on a branch from already-uploaded blobs -func (l *Local) ApplyCommit(ctx context.Context, storagePath string, spec CommitSpec, ops []CommitOp, clean CleanFunc) (RefChange, error) { - dir, err := l.resolve(storagePath) - if err != nil { - return RefChange{}, err - } - - spec, err = l.validateCommitSpec(ctx, dir, spec) - if err != nil { - return RefChange{}, err - } - ops, err = validateCommitOps(ops) - if err != nil { - return RefChange{}, err - } - - // resolve the current branch head - ref := "refs/heads/" + spec.Branch - oldSHA, err := l.revParse(ctx, dir, ref) - unborn := err != nil - switch { - case unborn && spec.ExpectedOld != zeroSHA: - return RefChange{}, fmt.Errorf("%w: branch %s", ErrRevNotFound, spec.Branch) - case !unborn && spec.ExpectedOld == zeroSHA: - return RefChange{}, fmt.Errorf("%w: branch %s already exists", ErrHeadMismatch, spec.Branch) - case !unborn && spec.ExpectedOld != "" && spec.ExpectedOld != oldSHA: - return RefChange{}, fmt.Errorf("%w: expected %s, head is %s", ErrHeadMismatch, spec.ExpectedOld, oldSHA) - } - if unborn { - oldSHA = zeroSHA - } - - ops, err = l.materializeLFSPointers(ctx, dir, ops) - if err != nil { - return RefChange{}, err - } - - ops, sizes, err := l.verifyCommitInputs(ctx, dir, oldSHA, unborn, ops) - if err != nil { - return RefChange{}, err - } - - // private index file: commits never touch the repo's real index (bare - // repos have none) and concurrent commits cannot see each other - idx, err := os.CreateTemp(dir, "headlessgit-index-*") - if err != nil { - return RefChange{}, fmt.Errorf("create temp index: %w", err) - } - idx.Close() - defer os.Remove(idx.Name()) - env := []string{"GIT_INDEX_FILE=" + idx.Name()} - - if unborn { - if _, err := l.runGit(ctx, dir, env, nil, "read-tree", "--empty"); err != nil { - return RefChange{}, err - } - } else { - if _, err := l.runGit(ctx, dir, env, nil, "read-tree", oldSHA); err != nil { - return RefChange{}, err - } - } - - // .gitattributes changes land first, so lfs tracking added in this very - // commit already applies to the files committed alongside it - attrOps, fileOps := splitAttrOps(ops) - if err := l.updateIndex(ctx, dir, env, attrOps); err != nil { - return RefChange{}, err - } - - fileOps, err = l.cleanLFSTracked(ctx, dir, env, fileOps, sizes, clean) - if err != nil { - return RefChange{}, err - } - if err := l.updateIndex(ctx, dir, env, fileOps); err != nil { - return RefChange{}, err - } - - treeSHA, err := l.runGit(ctx, dir, env, nil, "write-tree") - if err != nil { - return RefChange{}, fmt.Errorf("%w: %s", ErrInvalidOps, err) - } - if !unborn { - oldTree, err := l.revParse(ctx, dir, oldSHA+"^{tree}") - if err != nil { - return RefChange{}, err - } - if treeSHA == oldTree { - return RefChange{}, ErrNothingToCommit - } - } - - commitEnv := []string{ - "GIT_AUTHOR_NAME=" + spec.Author.Name, - "GIT_AUTHOR_EMAIL=" + spec.Author.Email, - "GIT_COMMITTER_NAME=" + spec.Committer.Name, - "GIT_COMMITTER_EMAIL=" + spec.Committer.Email, - } - args := []string{"commit-tree", treeSHA} - if !unborn { - args = append(args, "-p", oldSHA) - } - args = append(args, "-m", spec.Message) - newSHA, err := l.runGit(ctx, dir, commitEnv, nil, args...) - if err != nil { - return RefChange{}, err - } - - // update-ref only moves the branch if it still points at oldSHA - if _, err := l.runGit(ctx, dir, nil, nil, "update-ref", ref, newSHA, oldSHA); err != nil { - return RefChange{}, fmt.Errorf("%w: %s", ErrHeadMismatch, err) - } - - return RefChange{Ref: ref, OldSHA: oldSHA, NewSHA: newSHA}, nil -} - -// validateCommitSpec checks the branch name and identities -func (l *Local) validateCommitSpec(ctx context.Context, dir string, spec CommitSpec) (CommitSpec, error) { - if spec.Branch == "" || strings.HasPrefix(spec.Branch, "-") { - return spec, fmt.Errorf("%w: %q", ErrInvalidBranch, spec.Branch) - } - for _, r := range spec.Branch { - if r < 0x20 || r == 0x7f { - return spec, fmt.Errorf("%w: control character", ErrInvalidBranch) - } - } - // git itself is the authority on ref name rules - if _, err := l.runGit(ctx, dir, nil, nil, "check-ref-format", "--branch", spec.Branch); err != nil { - return spec, fmt.Errorf("%w: %q", ErrInvalidBranch, spec.Branch) - } - - if spec.ExpectedOld != "" && !isHexSHA(spec.ExpectedOld) { - return spec, fmt.Errorf("%w: expected old %q", ErrInvalidRev, spec.ExpectedOld) - } - if spec.Author.Name == "" || spec.Author.Email == "" { - return spec, fmt.Errorf("%w: author name and email are required", ErrInvalidOps) - } - if spec.Committer.Name == "" { - spec.Committer = spec.Author - } - if spec.Message == "" { - return spec, fmt.Errorf("%w: message is required", ErrInvalidOps) - } - return spec, nil -} - -// normalizes paths and enforces the op rules -func validateCommitOps(ops []CommitOp) ([]CommitOp, error) { - if len(ops) == 0 { - return nil, fmt.Errorf("%w: no operations", ErrInvalidOps) - } - if len(ops) > maxCommitOps { - return nil, fmt.Errorf("%w: more than %d operations", ErrInvalidOps, maxCommitOps) - } - - out := make([]CommitOp, len(ops)) - seen := make(map[string]bool, len(ops)) - for i, op := range ops { - p, err := normalizeTreePath(op.Path) - if err != nil || p == "" { - return nil, fmt.Errorf("%w: path %q", ErrInvalidOps, op.Path) - } - // the batch-check and check-attr line protocols cannot carry these - for _, r := range p { - if r < 0x20 || r == 0x7f { - return nil, fmt.Errorf("%w: control character in path", ErrInvalidOps) - } - } - for other := range seen { - if p == other { - return nil, fmt.Errorf("%w: duplicate path %q", ErrInvalidOps, p) - } - if strings.HasPrefix(p, other+"/") || strings.HasPrefix(other, p+"/") { - return nil, fmt.Errorf("%w: overlapping paths %q and %q", ErrInvalidOps, other, p) - } - } - seen[p] = true - - op.Path = p - if op.Delete { - if op.BlobSHA != "" || op.Lfs != nil { - return nil, fmt.Errorf("%w: delete %q takes no object", ErrInvalidOps, p) - } - } else { - hasBlob := op.BlobSHA != "" - hasLFS := op.Lfs != nil - - if hasBlob == hasLFS { - return nil, fmt.Errorf("%w: put %q requires exactly one of blob sha or lfs object", ErrInvalidOps, p) - } - if hasBlob && !isHexSHA(op.BlobSHA) { - return nil, fmt.Errorf("%w: blob sha %q", ErrInvalidOps, op.BlobSHA) - } - if hasLFS && (!isLFSOID(op.Lfs.OID) || op.Lfs.Size < 0) { - return nil, fmt.Errorf("%w: invalid lfs object for %q", ErrInvalidOps, p) - } - if hasLFS && isAttributesPath(p) { - return nil, fmt.Errorf("%w: attributes file %q cannot be an lfs object", ErrInvalidOps, p) - } - switch op.Mode { - case "": - op.Mode = "100644" - case "100644", "100755": - default: - return nil, fmt.Errorf("%w: mode %q", ErrInvalidOps, op.Mode) - } - } - out[i] = op - } - return out, nil -} - -// transforms lfs objects to its pointers -func (l *Local) materializeLFSPointers(ctx context.Context, dir string, ops []CommitOp) ([]CommitOp, error) { - for i := range ops { - if ops[i].Lfs == nil { - continue - } - - pointer := fmt.Sprintf( - "version https://git-lfs.github.com/spec/v1\noid sha256:%s\nsize %d\n", - ops[i].Lfs.OID, - ops[i].Lfs.Size, - ) - // generate sha for handcrafted pointer - sha, err := l.runGit(ctx, dir, nil, strings.NewReader(pointer), "hash-object", "-w", "--stdin") - if err != nil { - return nil, fmt.Errorf("write lfs pointer for %q: %w", ops[i].Path, err) - } - - if !isHexSHA(sha) { - return nil, fmt.Errorf("write lfs pointer for %q returned invalid sha %q", ops[i].Path, sha) - } - - ops[i].BlobSHA = sha - } - return ops, nil -} - -// runs one cat-file --batch-check over every put blob sha (returning their sizes) -func (l *Local) verifyCommitInputs(ctx context.Context, dir, oldSHA string, unborn bool, ops []CommitOp) ([]CommitOp, map[string]int64, error) { - var in strings.Builder - type query struct { - op CommitOp - isPut bool - } - var queries []query - for _, op := range ops { - if op.Delete { - if unborn { - return nil, nil, fmt.Errorf("%w: %q", ErrPathNotFound, op.Path) - } - in.WriteString(oldSHA + ":" + op.Path + "\n") - } else { - in.WriteString(op.BlobSHA + "\n") - } - queries = append(queries, query{op: op, isPut: !op.Delete}) - } - - out, err := l.runGit(ctx, dir, nil, strings.NewReader(in.String()), "cat-file", "--batch-check") - if err != nil { - return nil, nil, err - } - - lines := strings.Split(out, "\n") - if len(lines) != len(queries) { - return nil, nil, fmt.Errorf("unexpected batch-check output: %d lines for %d queries", len(lines), len(queries)) - } - - expanded := make([]CommitOp, 0, len(ops)) - sizes := make(map[string]int64) - for i, line := range lines { - q := queries[i] - fields := strings.Fields(line) - switch { - case len(fields) >= 2 && fields[len(fields)-1] == "missing": - if q.isPut { - return nil, nil, fmt.Errorf("%w: %s", ErrUnknownBlob, q.op.BlobSHA) - } - return nil, nil, fmt.Errorf("%w: %q", ErrPathNotFound, q.op.Path) - case len(fields) == 3 && fields[1] == "blob": - size, err := strconv.ParseInt(fields[2], 10, 64) - if err != nil { - return nil, nil, fmt.Errorf("malformed batch-check size %q: %w", fields[2], err) - } - if q.isPut { - sizes[q.op.BlobSHA] = size - } - expanded = append(expanded, q.op) - case len(fields) == 3 && !q.isPut && fields[1] == "tree": - deletes, err := l.expandTreeDelete(ctx, dir, q.op.Path, fields[0]) - if err != nil { - return nil, nil, err - } - expanded = append(expanded, deletes...) - case len(fields) == 3 && !q.isPut && fields[1] == "commit": - expanded = append(expanded, q.op) - case len(fields) == 3: - // a delete target resolving to a tree or a put sha naming a non-blob - if q.isPut { - return nil, nil, fmt.Errorf("%w: %s is a %s", ErrUnknownBlob, q.op.BlobSHA, fields[1]) - } - return nil, nil, fmt.Errorf("%w: cannot delete %q of type %s", ErrInvalidOps, q.op.Path, fields[1]) - default: - return nil, nil, fmt.Errorf("malformed batch-check line: %q", line) - } - } - return expanded, sizes, nil -} - -// recursive path listing for given directory, using "ls-tree -r" -func (l *Local) expandTreeDelete(ctx context.Context, dir, treePath, treeSHA string) ([]CommitOp, error) { - out, err := l.runGitBytes(ctx, dir, nil, nil, - "ls-tree", "-r", "-z", "--name-only", "--end-of-options", treeSHA, - ) - if err != nil { - return nil, fmt.Errorf("list delete tree %q: %w", treePath, err) - } - - var deletes []CommitOp - for name := range bytes.SplitSeq(out, []byte{0}) { - if len(name) == 0 { - continue - } - deletes = append(deletes, CommitOp{ - Delete: true, - Path: treePath + "/" + string(name), - }) - } - return deletes, nil -} - -// separates .gitattributes changes from regular file changes -func splitAttrOps(ops []CommitOp) (attr, files []CommitOp) { - for _, op := range ops { - if isAttributesPath(op.Path) { - attr = append(attr, op) - } else { - files = append(files, op) - } - } - return attr, files -} - -// asks git which put paths are lfs-tracked, -// and swaps their blobs for the pointer blobs produced by the clean filter -func (l *Local) cleanLFSTracked(ctx context.Context, dir string, env []string, ops []CommitOp, sizes map[string]int64, clean CleanFunc) ([]CommitOp, error) { - var paths []string - byPath := make(map[string]int) - pendingLFS := make(map[string]struct{}) - - for i, op := range ops { - if !op.Delete { - paths = append(paths, op.Path) - byPath[op.Path] = i - if op.Lfs != nil { - pendingLFS[op.Path] = struct{}{} - } - } - } - if len(paths) == 0 { - return ops, nil - } - - args := append([]string{"check-attr", "-z", "--cached", "filter", "--"}, paths...) - out, err := l.runGit(ctx, dir, env, nil, args...) - if err != nil { - return nil, err - } - - // -z output is NUL-separated (path, attr, value) triples - fields := strings.Split(out, "\x00") - for i := 0; i+2 < len(fields); i += 3 { - path, value := fields[i], fields[i+2] - idx, ok := byPath[path] - if !ok { - continue - } - - if ops[idx].Lfs != nil { - delete(pendingLFS, path) - // do not allow commiting lfs objects, if they are not tracked as lfs - if value != "lfs" { - return nil, fmt.Errorf("%w: %q", ErrLFSNotTracked, path) - } - continue - } - - if value != "lfs" { - continue - } - if clean == nil { - return nil, fmt.Errorf("%w: %q", ErrLFSRequired, path) - } - - pointerSHA, err := clean(path, ops[idx].BlobSHA, sizes[ops[idx].BlobSHA]) - if err != nil { - return nil, fmt.Errorf("lfs clean %q: %w", path, err) - } - if !isHexSHA(pointerSHA) { - return nil, fmt.Errorf("lfs clean %q returned invalid sha %q", path, pointerSHA) - } - ops[idx].BlobSHA = pointerSHA - } - - if len(pendingLFS) != 0 { - return nil, fmt.Errorf("check-attr omitted explicit lfs paths") - } - - return ops, nil -} - -// applies puts and deletes in one subprocess -func (l *Local) updateIndex(ctx context.Context, dir string, env []string, ops []CommitOp) error { - if len(ops) == 0 { - return nil - } - var in strings.Builder - for _, op := range ops { - if op.Delete { - in.WriteString("0 " + zeroSHA + "\t" + op.Path + "\x00") - } else { - in.WriteString(op.Mode + " " + op.BlobSHA + "\t" + op.Path + "\x00") - } - } - if _, err := l.runGit(ctx, dir, env, strings.NewReader(in.String()), "update-index", "-z", "--index-info"); err != nil { - return fmt.Errorf("%w: %s", ErrInvalidOps, err) - } - return nil -} - // runGit executes one short-lived git command with the repo as context, // applying the standard timeout, and returns its trimmed stdout func (l *Local) runGit(ctx context.Context, dir string, env []string, stdin io.Reader, args ...string) (string, error) { diff --git a/internal/gitbackend/local_commit.go b/internal/gitbackend/local_commit.go new file mode 100644 index 0000000..ad309e2 --- /dev/null +++ b/internal/gitbackend/local_commit.go @@ -0,0 +1,599 @@ +package gitbackend + +import ( + "bytes" + "context" + "fmt" + "maps" + "os" + "path" + "sort" + "strconv" + "strings" +) + +type indexEntry struct { + Mode string + SHA string + Path string +} + +// creates a commit on a branch from already-uploaded blobs +func (l *Local) ApplyCommit(ctx context.Context, storagePath string, spec CommitSpec, ops []CommitOp, clean CleanFunc, checkWrite CheckWriteFunc) (RefChange, error) { + dir, err := l.resolve(storagePath) + if err != nil { + return RefChange{}, err + } + + spec, err = l.validateCommitSpec(ctx, dir, spec) + if err != nil { + return RefChange{}, err + } + ops, err = validateCommitOps(ops) + if err != nil { + return RefChange{}, err + } + + // resolve the current branch head + ref := "refs/heads/" + spec.Branch + oldSHA, err := l.revParse(ctx, dir, ref) + unborn := err != nil + switch { + case unborn && spec.ExpectedOld != zeroSHA: + return RefChange{}, fmt.Errorf("%w: branch %s", ErrRevNotFound, spec.Branch) + case !unborn && spec.ExpectedOld == zeroSHA: + return RefChange{}, fmt.Errorf("%w: branch %s already exists", ErrHeadMismatch, spec.Branch) + case !unborn && spec.ExpectedOld != "" && spec.ExpectedOld != oldSHA: + return RefChange{}, fmt.Errorf("%w: expected %s, head is %s", ErrHeadMismatch, spec.ExpectedOld, oldSHA) + } + if unborn { + oldSHA = zeroSHA + } + + ops, err = l.materializeLFSPointers(ctx, dir, ops) + if err != nil { + return RefChange{}, err + } + + sizes, err := l.verifyPutInputs(ctx, dir, ops) + if err != nil { + return RefChange{}, err + } + + // private index file: commits never touch the repo's real index (bare + // repos have none) and concurrent commits cannot see each other + idx, err := os.CreateTemp(dir, "headlessgit-index-*") + if err != nil { + return RefChange{}, fmt.Errorf("create temp index: %w", err) + } + idx.Close() + defer os.Remove(idx.Name()) + env := []string{"GIT_INDEX_FILE=" + idx.Name()} + + if unborn { + if _, err := l.runGit(ctx, dir, env, nil, "read-tree", "--empty"); err != nil { + return RefChange{}, err + } + } else { + if _, err := l.runGit(ctx, dir, env, nil, "read-tree", oldSHA); err != nil { + return RefChange{}, err + } + } + + // Stage in request order. A move therefore changes the paths seen by every + // operation that follows it in the same request. + puts, err := l.stageCommitOps(ctx, dir, env, ops, checkWrite) + if err != nil { + return RefChange{}, err + } + + // All .gitattributes changes are now in the index. Clean only explicit puts; + // moved entries keep their existing object ids, just like git mv. + puts, err = l.cleanLFSTracked(ctx, dir, env, puts, sizes, clean) + if err != nil { + return RefChange{}, err + } + if err := l.updateIndex(ctx, dir, env, puts); err != nil { + return RefChange{}, err + } + + treeSHA, err := l.runGit(ctx, dir, env, nil, "write-tree") + if err != nil { + return RefChange{}, fmt.Errorf("%w: %s", ErrInvalidOps, err) + } + if !unborn { + oldTree, err := l.revParse(ctx, dir, oldSHA+"^{tree}") + if err != nil { + return RefChange{}, err + } + if treeSHA == oldTree { + return RefChange{}, ErrNothingToCommit + } + } + + commitEnv := []string{ + "GIT_AUTHOR_NAME=" + spec.Author.Name, + "GIT_AUTHOR_EMAIL=" + spec.Author.Email, + "GIT_COMMITTER_NAME=" + spec.Committer.Name, + "GIT_COMMITTER_EMAIL=" + spec.Committer.Email, + } + args := []string{"commit-tree", treeSHA} + if !unborn { + args = append(args, "-p", oldSHA) + } + args = append(args, "-m", spec.Message) + newSHA, err := l.runGit(ctx, dir, commitEnv, nil, args...) + if err != nil { + return RefChange{}, err + } + + // update-ref only moves the branch if it still points at oldSHA + if _, err := l.runGit(ctx, dir, nil, nil, "update-ref", ref, newSHA, oldSHA); err != nil { + return RefChange{}, fmt.Errorf("%w: %s", ErrHeadMismatch, err) + } + + return RefChange{Ref: ref, OldSHA: oldSHA, NewSHA: newSHA}, nil +} + +// validateCommitSpec checks the branch name and identities +func (l *Local) validateCommitSpec(ctx context.Context, dir string, spec CommitSpec) (CommitSpec, error) { + if spec.Branch == "" || strings.HasPrefix(spec.Branch, "-") { + return spec, fmt.Errorf("%w: %q", ErrInvalidBranch, spec.Branch) + } + for _, r := range spec.Branch { + if r < 0x20 || r == 0x7f { + return spec, fmt.Errorf("%w: control character", ErrInvalidBranch) + } + } + // git itself is the authority on ref name rules + if _, err := l.runGit(ctx, dir, nil, nil, "check-ref-format", "--branch", spec.Branch); err != nil { + return spec, fmt.Errorf("%w: %q", ErrInvalidBranch, spec.Branch) + } + + if spec.ExpectedOld != "" && !isHexSHA(spec.ExpectedOld) { + return spec, fmt.Errorf("%w: expected old %q", ErrInvalidRev, spec.ExpectedOld) + } + if spec.Author.Name == "" || spec.Author.Email == "" { + return spec, fmt.Errorf("%w: author name and email are required", ErrInvalidOps) + } + if spec.Committer.Name == "" { + spec.Committer = spec.Author + } + if spec.Message == "" { + return spec, fmt.Errorf("%w: message is required", ErrInvalidOps) + } + return spec, nil +} + +// normalizes paths and enforces the op rules +func validateCommitOps(ops []CommitOp) ([]CommitOp, error) { + if len(ops) == 0 { + return nil, fmt.Errorf("%w: no operations", ErrInvalidOps) + } + if len(ops) > maxCommitOps { + return nil, fmt.Errorf("%w: more than %d operations", ErrInvalidOps, maxCommitOps) + } + + out := make([]CommitOp, len(ops)) + seen := make(map[string]bool, len(ops)) + for i, op := range ops { + p, err := normalizeTreePath(op.Path) + if err != nil || p == "" { + return nil, fmt.Errorf("%w: path %q", ErrInvalidOps, op.Path) + } + // the batch-check and check-attr line protocols cannot carry these + for _, r := range p { + if r < 0x20 || r == 0x7f { + return nil, fmt.Errorf("%w: control character in path", ErrInvalidOps) + } + } + op.Path = p + if op.MoveFrom != "" { + from, err := normalizeTreePath(op.MoveFrom) + if err != nil || from == "" { + return nil, fmt.Errorf("%w: source path %q", ErrInvalidOps, op.MoveFrom) + } + for _, r := range from { + if r < 0x20 || r == 0x7f { + return nil, fmt.Errorf("%w: control character in source path", ErrInvalidOps) + } + } + if op.Delete || op.BlobSHA != "" || op.Lfs != nil || op.Mode != "" { + return nil, fmt.Errorf("%w: move %q takes no object or mode", ErrInvalidOps, from) + } + if from == p || strings.HasPrefix(p, from+"/") { + return nil, fmt.Errorf("%w: cannot move %q to %q", ErrInvalidOps, from, p) + } + op.MoveFrom = from + out[i] = op + continue + } + + for other := range seen { + if p == other { + return nil, fmt.Errorf("%w: duplicate path %q", ErrInvalidOps, p) + } + if strings.HasPrefix(p, other+"/") || strings.HasPrefix(other, p+"/") { + return nil, fmt.Errorf("%w: overlapping paths %q and %q", ErrInvalidOps, other, p) + } + } + seen[p] = true + + if op.Delete { + if op.BlobSHA != "" || op.Lfs != nil { + return nil, fmt.Errorf("%w: delete %q takes no object", ErrInvalidOps, p) + } + } else { + hasBlob := op.BlobSHA != "" + hasLFS := op.Lfs != nil + + if hasBlob == hasLFS { + return nil, fmt.Errorf("%w: put %q requires exactly one of blob sha or lfs object", ErrInvalidOps, p) + } + if hasBlob && !isHexSHA(op.BlobSHA) { + return nil, fmt.Errorf("%w: blob sha %q", ErrInvalidOps, op.BlobSHA) + } + if hasLFS && (!isLFSOID(op.Lfs.OID) || op.Lfs.Size < 0) { + return nil, fmt.Errorf("%w: invalid lfs object for %q", ErrInvalidOps, p) + } + if hasLFS && isAttributesPath(p) { + return nil, fmt.Errorf("%w: attributes file %q cannot be an lfs object", ErrInvalidOps, p) + } + switch op.Mode { + case "": + op.Mode = "100644" + case "100644", "100755": + default: + return nil, fmt.Errorf("%w: mode %q", ErrInvalidOps, op.Mode) + } + } + out[i] = op + } + return out, nil +} + +// transforms lfs objects to its pointers +func (l *Local) materializeLFSPointers(ctx context.Context, dir string, ops []CommitOp) ([]CommitOp, error) { + for i := range ops { + if ops[i].Lfs == nil { + continue + } + + pointer := fmt.Sprintf( + "version https://git-lfs.github.com/spec/v1\noid sha256:%s\nsize %d\n", + ops[i].Lfs.OID, + ops[i].Lfs.Size, + ) + // generate sha for handcrafted pointer + sha, err := l.runGit(ctx, dir, nil, strings.NewReader(pointer), "hash-object", "-w", "--stdin") + if err != nil { + return nil, fmt.Errorf("write lfs pointer for %q: %w", ops[i].Path, err) + } + + if !isHexSHA(sha) { + return nil, fmt.Errorf("write lfs pointer for %q returned invalid sha %q", ops[i].Path, sha) + } + + ops[i].BlobSHA = sha + } + return ops, nil +} + +// runs one cat-file --batch-check over every put blob sha (returning their sizes) +func (l *Local) verifyPutInputs(ctx context.Context, dir string, ops []CommitOp) (map[string]int64, error) { + var in strings.Builder + var puts []CommitOp + for _, op := range ops { + if op.Delete || op.MoveFrom != "" { + continue + } + in.WriteString(op.BlobSHA + "\n") + puts = append(puts, op) + } + if len(puts) == 0 { + return map[string]int64{}, nil + } + + out, err := l.runGit(ctx, dir, nil, strings.NewReader(in.String()), "cat-file", "--batch-check") + if err != nil { + return nil, err + } + + lines := strings.Split(out, "\n") + if len(lines) != len(puts) { + return nil, fmt.Errorf("unexpected batch-check output: %d lines for %d queries", len(lines), len(puts)) + } + + sizes := make(map[string]int64) + for i, line := range lines { + op := puts[i] + fields := strings.Fields(line) + switch { + case len(fields) >= 2 && fields[len(fields)-1] == "missing": + return nil, fmt.Errorf("%w: %s", ErrUnknownBlob, op.BlobSHA) + case len(fields) == 3 && fields[1] == "blob": + size, err := strconv.ParseInt(fields[2], 10, 64) + if err != nil { + return nil, fmt.Errorf("malformed batch-check size %q: %w", fields[2], err) + } + sizes[op.BlobSHA] = size + case len(fields) == 3: + return nil, fmt.Errorf("%w: %s is a %s", ErrUnknownBlob, op.BlobSHA, fields[1]) + default: + return nil, fmt.Errorf("malformed batch-check line: %q", line) + } + } + return sizes, nil +} + +// apply ops sequentially against temporary index, and return the ops that "survived" +func (l *Local) stageCommitOps(ctx context.Context, dir string, env []string, ops []CommitOp, checkWrite CheckWriteFunc) ([]CommitOp, error) { + pendingPuts := make(map[string]CommitOp) + + for _, op := range ops { + switch { + case op.MoveFrom != "": + if err := l.moveIndexPath(ctx, dir, env, op.MoveFrom, op.Path, pendingPuts, checkWrite); err != nil { + return nil, err + } + + case op.Delete: + entries, err := l.listIndexEntries(ctx, dir, env, op.Path) + if err != nil { + return nil, err + } + if len(entries) == 0 { + return nil, fmt.Errorf("%w: %q", ErrPathNotFound, op.Path) + } + + deletes := make([]CommitOp, len(entries)) + for i, entry := range entries { + deletes[i] = CommitOp{Delete: true, Path: entry.Path} + } + if err := l.updateIndex(ctx, dir, env, deletes); err != nil { + return nil, err + } + // no puts can happen under this path, after delete + removePendingPuts(pendingPuts, op.Path) + + default: + if checkWrite != nil { + if err := checkWrite(op.Path); err != nil { + return nil, err + } + } + if err := l.updateIndex(ctx, dir, env, []CommitOp{op}); err != nil { + return nil, err + } + removePendingPuts(pendingPuts, op.Path) + pendingPuts[op.Path] = op + } + } + + paths := make([]string, 0, len(pendingPuts)) + for path := range pendingPuts { + paths = append(paths, path) + } + sort.Strings(paths) + + puts := make([]CommitOp, 0, len(paths)) + for _, path := range paths { + puts = append(puts, pendingPuts[path]) + } + return puts, nil +} + +func (l *Local) moveIndexPath(ctx context.Context, dir string, env []string, from, destination string, pendingPuts map[string]CommitOp, checkWrite CheckWriteFunc) error { + entries, err := l.listIndexEntries(ctx, dir, env, from) + if err != nil { + return err + } + if len(entries) == 0 { + return fmt.Errorf("%w: %q", ErrPathNotFound, from) + } + + occupied, err := l.listIndexEntries(ctx, dir, env, destination) + if err != nil { + return err + } + parentOccupied, err := l.indexParentOccupied(ctx, dir, env, destination) + if err != nil { + return err + } + if len(occupied) != 0 || parentOccupied { + return fmt.Errorf("%w: %q", ErrPathExists, destination) + } + + changes := make([]CommitOp, 0, len(entries)*2) + for _, entry := range entries { + changes = append(changes, CommitOp{Delete: true, Path: entry.Path}) + } + for _, entry := range entries { + target := destination + strings.TrimPrefix(entry.Path, from) + if checkWrite != nil { + if err := checkWrite(target); err != nil { + return err + } + } + changes = append(changes, CommitOp{Path: target, BlobSHA: entry.SHA, Mode: entry.Mode}) + } + + if err := l.updateIndex(ctx, dir, env, changes); err != nil { + return err + } + movePendingPuts(pendingPuts, from, destination) + return nil +} + +// git ls-files at treePath +func (l *Local) listIndexEntries(ctx context.Context, dir string, env []string, treePath string) ([]indexEntry, error) { + out, err := l.runGitBytes(ctx, dir, env, nil, + "ls-files", "--stage", "-z", "--full-name", "--", ":(top,literal)"+treePath, + ) + if err != nil { + return nil, err + } + + var entries []indexEntry + for record := range bytes.SplitSeq(out, []byte{0}) { + if len(record) == 0 { + continue + } + header, name, ok := bytes.Cut(record, []byte{'\t'}) + if !ok { + return nil, fmt.Errorf("malformed ls-files record: %q", record) + } + fields := strings.Fields(string(header)) + if len(fields) != 3 || !isHexSHA(fields[1]) || fields[2] != "0" { + return nil, fmt.Errorf("malformed ls-files header: %q", header) + } + entries = append(entries, indexEntry{Mode: fields[0], SHA: fields[1], Path: string(name)}) + } + return entries, nil +} + +// checks wheather the destination is free AND non of the parent path is a file. +// e.g. dest="a/b", but "a" is a file and not a dir +func (l *Local) indexParentOccupied(ctx context.Context, dir string, env []string, destination string) (bool, error) { + var parents []string + for parent := path.Dir(destination); parent != "."; parent = path.Dir(parent) { + parents = append(parents, parent) + } + if len(parents) == 0 { + return false, nil + } + + var in strings.Builder + for _, parent := range parents { + in.WriteString(":" + parent + "\n") + } + out, err := l.runGit(ctx, dir, env, strings.NewReader(in.String()), "cat-file", "--batch-check") + if err != nil { + return false, err + } + + lines := strings.Split(out, "\n") + if len(lines) != len(parents) { + return false, fmt.Errorf("unexpected index batch-check output: %d lines for %d paths", len(lines), len(parents)) + } + + for _, line := range lines { + fields := strings.Fields(line) + if len(fields) < 2 { + return false, fmt.Errorf("malformed index batch-check line: %q", line) + } + // if some of the parent paths are NOT missing => it exists as a file, which is bad + if fields[len(fields)-1] != "missing" { + return true, nil + } + } + return false, nil +} + +func removePendingPuts(pending map[string]CommitOp, treePath string) { + for path := range pending { + if path == treePath || strings.HasPrefix(path, treePath+"/") { + delete(pending, path) + } + } +} + +func movePendingPuts(pending map[string]CommitOp, from, destination string) { + moved := make(map[string]CommitOp) + for path, op := range pending { + if path != from && !strings.HasPrefix(path, from+"/") { + continue + } + delete(pending, path) + op.Path = destination + strings.TrimPrefix(path, from) + moved[op.Path] = op + } + maps.Copy(pending, moved) +} + +// asks git which put paths are lfs-tracked, +// and swaps their blobs for the pointer blobs produced by the clean filter +func (l *Local) cleanLFSTracked(ctx context.Context, dir string, env []string, ops []CommitOp, sizes map[string]int64, clean CleanFunc) ([]CommitOp, error) { + var paths []string + byPath := make(map[string]int) + pendingLFS := make(map[string]struct{}) + + for i, op := range ops { + if !op.Delete { + paths = append(paths, op.Path) + byPath[op.Path] = i + if op.Lfs != nil { + pendingLFS[op.Path] = struct{}{} + } + } + } + if len(paths) == 0 { + return ops, nil + } + + args := append([]string{"check-attr", "-z", "--cached", "filter", "--"}, paths...) + out, err := l.runGit(ctx, dir, env, nil, args...) + if err != nil { + return nil, err + } + + // -z output is NUL-separated (path, attr, value) triples + fields := strings.Split(out, "\x00") + for i := 0; i+2 < len(fields); i += 3 { + path, value := fields[i], fields[i+2] + idx, ok := byPath[path] + if !ok { + continue + } + + if ops[idx].Lfs != nil { + delete(pendingLFS, path) + // do not allow commiting lfs objects, if they are not tracked as lfs + if value != "lfs" { + return nil, fmt.Errorf("%w: %q", ErrLFSNotTracked, path) + } + continue + } + + if value != "lfs" { + continue + } + if clean == nil { + return nil, fmt.Errorf("%w: %q", ErrLFSRequired, path) + } + + pointerSHA, err := clean(path, ops[idx].BlobSHA, sizes[ops[idx].BlobSHA]) + if err != nil { + return nil, fmt.Errorf("lfs clean %q: %w", path, err) + } + if !isHexSHA(pointerSHA) { + return nil, fmt.Errorf("lfs clean %q returned invalid sha %q", path, pointerSHA) + } + ops[idx].BlobSHA = pointerSHA + } + + if len(pendingLFS) != 0 { + return nil, fmt.Errorf("check-attr omitted explicit lfs paths") + } + + return ops, nil +} + +// applies puts and deletes in one subprocess +func (l *Local) updateIndex(ctx context.Context, dir string, env []string, ops []CommitOp) error { + if len(ops) == 0 { + return nil + } + var in strings.Builder + for _, op := range ops { + if op.Delete { + in.WriteString("0 " + zeroSHA + "\t" + op.Path + "\x00") + } else { + in.WriteString(op.Mode + " " + op.BlobSHA + "\t" + op.Path + "\x00") + } + } + if _, err := l.runGit(ctx, dir, env, strings.NewReader(in.String()), "update-index", "-z", "--index-info"); err != nil { + return fmt.Errorf("%w: %s", ErrInvalidOps, err) + } + return nil +} diff --git a/internal/gitbackend/local_test.go b/internal/gitbackend/local_test.go index 22e11ec..e9b8339 100644 --- a/internal/gitbackend/local_test.go +++ b/internal/gitbackend/local_test.go @@ -1022,7 +1022,7 @@ func TestApplyCommit(t *testing.T) { script := blob("#!/bin/sh\n") // creating a branch requires explicitly expecting non-existence - if _, err := l.ApplyCommit(ctx, repo, spec("", "init"), []CommitOp{{Path: "README.md", BlobSHA: hello}}, nil); !errors.Is(err, ErrRevNotFound) { + if _, err := l.ApplyCommit(ctx, repo, spec("", "init"), []CommitOp{{Path: "README.md", BlobSHA: hello}}, nil, nil); !errors.Is(err, ErrRevNotFound) { t.Fatalf("missing branch without zero expected-old: want ErrRevNotFound, got %v", err) } @@ -1030,7 +1030,7 @@ func TestApplyCommit(t *testing.T) { {Path: "README.md", BlobSHA: hello}, {Path: "src/run.sh", BlobSHA: script, Mode: "100755"}, {Path: "src/config/default.txt", BlobSHA: hello}, - }, nil) + }, nil, nil) if err != nil { t.Fatal(err) } @@ -1065,7 +1065,7 @@ func TestApplyCommit(t *testing.T) { second, err := l.ApplyCommit(ctx, repo, spec(first.NewSHA, "update"), []CommitOp{ {Path: "README.md", BlobSHA: v2}, {Path: "src", Delete: true}, - }, nil) + }, nil, nil) if err != nil { t.Fatal(err) } @@ -1081,6 +1081,45 @@ func TestApplyCommit(t *testing.T) { t.Errorf("src directory should be deleted recursively, stat err = %v", err) } + // A move reuses the staged entries recursively. Operations after it address + // the destination, so callers can reorganize and edit in one commit. + third, err := l.ApplyCommit(ctx, repo, spec(second.NewSHA, "add plugins"), []CommitOp{ + {Path: "plugins/config.yml", BlobSHA: hello}, + {Path: "plugins/obsolete.yml", BlobSHA: hello}, + {Path: "plugins/bin/run", BlobSHA: script, Mode: "100755"}, + }, nil, nil) + if err != nil { + t.Fatal(err) + } + fourth, err := l.ApplyCommit(ctx, repo, spec(third.NewSHA, "move plugins"), []CommitOp{ + {MoveFrom: "plugins", Path: "server/plugins"}, + {Path: "server/plugins/config.yml", BlobSHA: v2}, + {Path: "server/plugins/obsolete.yml", Delete: true}, + }, nil, nil) + if err != nil { + t.Fatal(err) + } + if _, err := l.StatBlob(ctx, repo, fourth.NewSHA, "plugins/config.yml"); !errors.Is(err, ErrPathNotFound) { + t.Errorf("old path still exists: %v", err) + } + config, err := l.StatBlob(ctx, repo, fourth.NewSHA, "server/plugins/config.yml") + if err != nil { + t.Fatal(err) + } + if config.BlobSHA != v2 { + t.Errorf("moved config blob = %s, want %s", config.BlobSHA, v2) + } + if _, err := l.StatBlob(ctx, repo, fourth.NewSHA, "server/plugins/obsolete.yml"); !errors.Is(err, ErrPathNotFound) { + t.Errorf("deleted moved path still exists: %v", err) + } + listing, err := l.ListTree(ctx, repo, fourth.NewSHA, "server/plugins/bin", ListTreeOptions{}) + if err != nil { + t.Fatal(err) + } + if len(listing.Entries) != 1 || listing.Entries[0].Path != "server/plugins/bin/run" || listing.Entries[0].Mode != "100755" { + t.Errorf("moved executable = %+v", listing.Entries) + } + t.Run("errors", func(t *testing.T) { cases := []struct { name string @@ -1093,6 +1132,10 @@ func TestApplyCommit(t *testing.T) { {"unknown blob", spec("", "x"), []CommitOp{{Path: "a", BlobSHA: strings.Repeat("d", 40)}}, ErrUnknownBlob}, {"nothing to commit", spec("", "x"), []CommitOp{{Path: "README.md", BlobSHA: v2}}, ErrNothingToCommit}, {"delete missing path", spec("", "x"), []CommitOp{{Path: "nope.txt", Delete: true}}, ErrPathNotFound}, + {"move missing path", spec("", "x"), []CommitOp{{MoveFrom: "nope", Path: "elsewhere"}}, ErrPathNotFound}, + {"move destination exists", spec("", "x"), []CommitOp{{MoveFrom: "server/plugins", Path: "README.md"}}, ErrPathExists}, + {"move destination parent is a file", spec("", "x"), []CommitOp{{MoveFrom: "README.md", Path: "server/plugins/bin/run/child"}}, ErrPathExists}, + {"move into itself", spec("", "x"), []CommitOp{{MoveFrom: "server", Path: "server/nested"}}, ErrInvalidOps}, {"bad branch", CommitSpec{Branch: "a..b", ExpectedOld: "", Author: author, Message: "x"}, []CommitOp{{Path: "a", BlobSHA: hello}}, ErrInvalidBranch}, {"hostile branch", CommitSpec{Branch: "--help", Author: author, Message: "x"}, []CommitOp{{Path: "a", BlobSHA: hello}}, ErrInvalidBranch}, {"no ops", spec("", "x"), nil, ErrInvalidOps}, @@ -1109,13 +1152,32 @@ func TestApplyCommit(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - if _, err := l.ApplyCommit(ctx, repo, tc.spec, tc.ops, nil); !errors.Is(err, tc.want) { + if _, err := l.ApplyCommit(ctx, repo, tc.spec, tc.ops, nil, nil); !errors.Is(err, tc.want) { t.Errorf("ApplyCommit = %v, want %v", err, tc.want) } }) } }) + t.Run("move checks every destination path before changing the ref", func(t *testing.T) { + blocked := errors.New("blocked destination") + check := func(path string) error { + if path == "blocked/plugins/bin/run" { + return blocked + } + return nil + } + _, err := l.ApplyCommit(ctx, repo, spec(fourth.NewSHA, "blocked move"), []CommitOp{ + {MoveFrom: "server/plugins", Path: "blocked/plugins"}, + }, nil, check) + if !errors.Is(err, blocked) { + t.Fatalf("want blocked destination, got %v", err) + } + if head := gitOut(t, filepath.Join(root, repo), "rev-parse", "refs/heads/main"); head != fourth.NewSHA { + t.Errorf("head changed after rejected move: %s", head) + } + }) + t.Run("lfs clean", func(t *testing.T) { attrs := blob("*.bin filter=lfs diff=lfs merge=lfs -text\n") payload := blob("REAL BINARY CONTENT") @@ -1126,7 +1188,7 @@ func TestApplyCommit(t *testing.T) { if _, err := l.ApplyCommit(ctx, repo, spec("", "track"), []CommitOp{ {Path: ".gitattributes", BlobSHA: attrs}, {Path: "big.bin", BlobSHA: payload}, - }, nil); !errors.Is(err, ErrLFSRequired) { + }, nil, nil); !errors.Is(err, ErrLFSRequired) { t.Fatalf("want ErrLFSRequired, got %v", err) } @@ -1141,7 +1203,7 @@ func TestApplyCommit(t *testing.T) { {Path: ".gitattributes", BlobSHA: attrs}, {Path: "big.bin", BlobSHA: payload}, {Path: "notes.txt", BlobSHA: hello}, // untracked, must NOT be cleaned - }, clean) + }, clean, nil) if err != nil { t.Fatal(err) } @@ -1170,7 +1232,7 @@ func TestApplyCommit(t *testing.T) { explicitOID := strings.Repeat("cd", 32) explicit, err := l.ApplyCommit(ctx, repo, spec(change.NewSHA, "explicit lfs"), []CommitOp{ {Path: "direct.bin", Lfs: &LfsObject{OID: explicitOID, Size: 23}}, - }, nil) + }, nil, nil) if err != nil { t.Fatal(err) } @@ -1189,9 +1251,40 @@ func TestApplyCommit(t *testing.T) { if _, err := l.ApplyCommit(ctx, repo, spec(explicit.NewSHA, "untracked lfs"), []CommitOp{ {Path: "direct.dat", Lfs: &LfsObject{OID: explicitOID, Size: 23}}, - }, nil); !errors.Is(err, ErrLFSNotTracked) { + }, nil, nil); !errors.Is(err, ErrLFSNotTracked) { t.Fatalf("untracked explicit lfs: want ErrLFSNotTracked, got %v", err) } + + moved, err := l.ApplyCommit(ctx, repo, spec(explicit.NewSHA, "move lfs pointer"), []CommitOp{ + {MoveFrom: "direct.bin", Path: "assets/direct.bin"}, + }, func(string, string, int64) (string, error) { + return "", errors.New("move must not invoke lfs clean") + }, nil) + if err != nil { + t.Fatal(err) + } + movedDirect, err := l.StatBlob(ctx, repo, moved.NewSHA, "assets/direct.bin") + if err != nil { + t.Fatal(err) + } + if movedDirect.BlobSHA != direct.BlobSHA { + t.Errorf("moved lfs pointer blob = %s, want %s", movedDirect.BlobSHA, direct.BlobSHA) + } + + var cleanedPath string + _, err = l.ApplyCommit(ctx, repo, spec(moved.NewSHA, "move attributes and add"), []CommitOp{ + {MoveFrom: ".gitattributes", Path: "assets/.gitattributes"}, + {Path: "assets/new.bin", BlobSHA: payload}, + }, func(path, _ string, _ int64) (string, error) { + cleanedPath = path + return pointer, nil + }, nil) + if err != nil { + t.Fatal(err) + } + if cleanedPath != "assets/new.bin" { + t.Errorf("moved attributes cleaned %q, want assets/new.bin", cleanedPath) + } }) } @@ -1285,7 +1378,7 @@ func TestPreReceive(t *testing.T) { expected = "" gitRun(t, repoDir, "update-ref", "refs/heads/scratch", base) } - change, err := l.ApplyCommit(ctx, repo, CommitSpec{Branch: "scratch", ExpectedOld: expected, Author: author, Message: "staged"}, ops, nil) + change, err := l.ApplyCommit(ctx, repo, CommitSpec{Branch: "scratch", ExpectedOld: expected, Author: author, Message: "staged"}, ops, nil, nil) if err != nil { t.Fatal(err) } @@ -1329,7 +1422,7 @@ func TestPreReceive(t *testing.T) { // the blocked path already exists on a real branch (added before the // policy); a push that only deletes it must pass base, err := l.ApplyCommit(ctx, repo, CommitSpec{Branch: "cleanup", ExpectedOld: zeroSHA, Author: author, Message: "pre-policy"}, - []CommitOp{{Path: "runtime/state.json", BlobSHA: blob("old\n")}}, nil) + []CommitOp{{Path: "runtime/state.json", BlobSHA: blob("old\n")}}, nil, nil) if err != nil { t.Fatal(err) } diff --git a/internal/gitbackend/types.go b/internal/gitbackend/types.go index d604140..fde2222 100644 --- a/internal/gitbackend/types.go +++ b/internal/gitbackend/types.go @@ -130,11 +130,12 @@ type LfsObject struct { } type CommitOp struct { - Delete bool - Path string - Lfs *LfsObject - BlobSHA string // puts only; must exist as a blob in this repo's odb - Mode string // puts only: "100644" (default) or "100755" + Delete bool + MoveFrom string // moves only; Path is the destination + Path string + Lfs *LfsObject + BlobSHA string // puts only; must exist as a blob in this repo's odb + Mode string // puts only: "100644" (default) or "100755" } type Identity struct { @@ -153,3 +154,6 @@ type CommitSpec struct { // write-side mirror of archive.SmudgeFunc type CleanFunc func(path, blobSHA string, size int64) (string, error) + +// rejects writes to paths that are blocked by repository policy +type CheckWriteFunc func(path string) error diff --git a/internal/server/control/repositories/commits.go b/internal/server/control/repositories/commits.go index dc73b5e..c566c8e 100644 --- a/internal/server/control/repositories/commits.go +++ b/internal/server/control/repositories/commits.go @@ -53,6 +53,7 @@ func (h *handlers) createCommit(w http.ResponseWriter, r *http.Request) error { for i, op := range req.Operations { ops[i] = domain.CommitFileOp{ Delete: op.Op == "delete", + MoveFrom: op.FromPath, Path: op.Path, BlobSHA: op.BlobSHA, Executable: op.Executable, @@ -80,7 +81,9 @@ func (h *handlers) createCommit(w http.ResponseWriter, r *http.Request) error { case errors.Is(err, reposervice.ErrRefNotFound): return response.NewError(http.StatusNotFound, response.CodeRefNotFound, "branch not found; pass the all-zero expectedHeadSha to create it") case errors.Is(err, reposervice.ErrPathNotFound): - return response.NewError(http.StatusNotFound, response.CodePathNotFound, "delete target not found") + return response.NewError(http.StatusNotFound, response.CodePathNotFound, "delete target or move source not found") + case errors.Is(err, reposervice.ErrPathConflict): + return response.NewError(http.StatusConflict, response.CodePathConflict, "move destination already exists") case errors.Is(err, reposervice.ErrHeadMismatch): return response.NewError(http.StatusConflict, response.CodeHeadMismatch, "branch head does not match expectedHeadSha") case errors.Is(err, reposervice.ErrUnknownBlob): diff --git a/internal/server/control/repositories/handlers_test.go b/internal/server/control/repositories/handlers_test.go index c4c0ed1..f68100e 100644 --- a/internal/server/control/repositories/handlers_test.go +++ b/internal/server/control/repositories/handlers_test.go @@ -889,6 +889,26 @@ func TestCreateCommit(t *testing.T) { } } +func TestCreateCommitMove(t *testing.T) { + fake := &fakeManager{commitResult: domain.CommitResult{Branch: "main", CommitSHA: testSHA}} + body := `{ + "branch":"main", + "message":"reorganize", + "author":{"name":"api-user","email":"api@test"}, + "operations":[{"op":"move","fromPath":"plugins","path":"server/plugins"}] + }` + rec := httptest.NewRecorder() + newTestRouter(fake).ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/repositories/7/commits", strings.NewReader(body))) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, body %s", rec.Code, rec.Body.String()) + } + if len(fake.commitReq.Operations) != 1 || fake.commitReq.Operations[0].MoveFrom != "plugins" || + fake.commitReq.Operations[0].Path != "server/plugins" { + t.Errorf("service operations = %+v", fake.commitReq.Operations) + } +} + func TestCreateCommitValidation(t *testing.T) { cases := []struct { name string @@ -899,7 +919,10 @@ func TestCreateCommitValidation(t *testing.T) { {"missing message", `{"branch":"main","author":{"name":"a","email":"e"},"operations":[{"op":"delete","path":"a"}]}`}, {"missing author", `{"branch":"main","message":"x","operations":[{"op":"delete","path":"a"}]}`}, {"no operations", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[]}`}, - {"bad op kind", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"move","path":"a"}]}`}, + {"bad op kind", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"copy","path":"a"}]}`}, + {"move without source", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"move","path":"a"}]}`}, + {"move with blob", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"move","fromPath":"a","path":"b","blobSha":"abc"}]}`}, + {"put with source", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"put","fromPath":"old","path":"a","blobSha":"abc"}]}`}, {"put without object", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"put","path":"a"}]}`}, {"put with both sources", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"put","path":"a","blobSha":"abc","lfs":{"oid":"def","size":1}}]}`}, {"put with zero lfs size", `{"branch":"main","message":"x","author":{"name":"a","email":"e"},"operations":[{"op":"put","path":"a","lfs":{"oid":"def","size":0}}]}`}, @@ -934,6 +957,7 @@ func TestCreateCommitErrors(t *testing.T) { {"repo not found", reposervice.ErrRepositoryNotFound, http.StatusNotFound, "repository_not_found"}, {"branch not found", reposervice.ErrRefNotFound, http.StatusNotFound, "ref_not_found"}, {"delete target missing", reposervice.ErrPathNotFound, http.StatusNotFound, "path_not_found"}, + {"move destination exists", reposervice.ErrPathConflict, http.StatusConflict, "path_conflict"}, {"head mismatch", reposervice.ErrHeadMismatch, http.StatusConflict, "head_mismatch"}, {"unknown blob", reposervice.ErrUnknownBlob, http.StatusUnprocessableEntity, "unknown_blob"}, {"unknown lfs object", reposervice.ErrLFSObjectNotFound, http.StatusUnprocessableEntity, "lfs_object_not_found"}, diff --git a/internal/server/control/repositories/types.go b/internal/server/control/repositories/types.go index 9cc7cbb..bf8934f 100644 --- a/internal/server/control/repositories/types.go +++ b/internal/server/control/repositories/types.go @@ -216,8 +216,9 @@ type CommitObjectLfs struct { } type CommitOperation struct { - Op string `json:"op"` // "put" | "delete" + Op string `json:"op"` // "put" | "delete" | "move" Path string `json:"path"` + FromPath string `json:"fromPath,omitempty"` // moves only Lfs *CommitObjectLfs `json:"lfs,omitempty"` // puts only, from POST .../lfs/objects/batch BlobSHA *string `json:"blobSha,omitempty"` // puts only, from POST /blobs Executable bool `json:"executable,omitempty"` // puts only @@ -251,6 +252,9 @@ func (r CreateCommitRequest) Validate() error { } switch op.Op { case "put": + if op.FromPath != "" { + return fmt.Errorf("operations[%d]: fromPath is only valid for move", i) + } if (op.BlobSHA == nil) == (op.Lfs == nil) { return fmt.Errorf("operations[%d]: exactly one of blobSha or lfs is required for put", i) } @@ -267,11 +271,18 @@ func (r CreateCommitRequest) Validate() error { } } case "delete": + if op.FromPath != "" || op.BlobSHA != nil || op.Lfs != nil || op.Executable { + return fmt.Errorf("operations[%d]: delete takes only op and path", i) + } + case "move": + if op.FromPath == "" { + return fmt.Errorf("operations[%d]: fromPath is required for move", i) + } if op.BlobSHA != nil || op.Lfs != nil || op.Executable { - return fmt.Errorf("operations[%d]: delete takes no blobSha, lfs, or executable", i) + return fmt.Errorf("operations[%d]: move takes only op, fromPath, and path", i) } default: - return fmt.Errorf("operations[%d]: op must be 'put' or 'delete'", i) + return fmt.Errorf("operations[%d]: op must be 'put', 'delete', or 'move'", i) } } return nil diff --git a/internal/server/response/codes.go b/internal/server/response/codes.go index 9a078d4..e9a341a 100644 --- a/internal/server/response/codes.go +++ b/internal/server/response/codes.go @@ -13,6 +13,7 @@ const ( CodeRefNotFound = "ref_not_found" CodeCommitNotFound = "commit_not_found" CodePathNotFound = "path_not_found" + CodePathConflict = "path_conflict" CodeLFSObjectNotFound = "lfs_object_not_found" CodeHeadMismatch = "head_mismatch" diff --git a/internal/services/repositories/errors.go b/internal/services/repositories/errors.go index 66e4195..fd2b094 100644 --- a/internal/services/repositories/errors.go +++ b/internal/services/repositories/errors.go @@ -10,6 +10,7 @@ var ( ErrRefNotFound = errors.New("ref not found") ErrPathNotFound = errors.New("path not found") + ErrPathConflict = errors.New("path already exists") ErrInvalidRef = errors.New("invalid ref") ErrInvalidPath = errors.New("invalid path") diff --git a/internal/services/repositories/service.go b/internal/services/repositories/service.go index d5f9a43..a0f2e67 100644 --- a/internal/services/repositories/service.go +++ b/internal/services/repositories/service.go @@ -45,7 +45,7 @@ type RepositoryStorage interface { StatBlob(ctx context.Context, storagePath, rev, treePath string) (gitbackend.BlobInfo, error) ReadBlob(ctx context.Context, storagePath, blobSHA string, out io.Writer) error WriteBlob(ctx context.Context, storagePath string, r io.Reader) (string, int64, error) - ApplyCommit(ctx context.Context, storagePath string, spec gitbackend.CommitSpec, ops []gitbackend.CommitOp, clean gitbackend.CleanFunc) (gitbackend.RefChange, error) + ApplyCommit(ctx context.Context, storagePath string, spec gitbackend.CommitSpec, ops []gitbackend.CommitOp, clean gitbackend.CleanFunc, checkWrite gitbackend.CheckWriteFunc) (gitbackend.RefChange, error) GC(ctx context.Context, storagePath string) error } @@ -517,10 +517,20 @@ func (s *Service) Commit(ctx context.Context, repositoryID int64, req domain.Com return domain.CommitResult{}, err } - if err := s.checkPathPolicies(ctx, repositoryID, req.Operations); err != nil { + checkWrite, err := s.pathPolicyChecker(ctx, repositoryID) + if err != nil { return domain.CommitResult{}, err } + for _, op := range req.Operations { + if !op.Delete && op.MoveFrom == "" && checkWrite != nil { + cleaned := path.Clean("/" + op.Path)[1:] + if err := checkWrite(cleaned); err != nil { + return domain.CommitResult{}, err + } + } + } + ops := make([]gitbackend.CommitOp, len(req.Operations)) for i, op := range req.Operations { mode := "" @@ -528,9 +538,10 @@ func (s *Service) Commit(ctx context.Context, repositoryID int64, req domain.Com mode = "100755" } ops[i] = gitbackend.CommitOp{ - Delete: op.Delete, - Path: op.Path, - Mode: mode, + Delete: op.Delete, + MoveFrom: op.MoveFrom, + Path: op.Path, + Mode: mode, } if op.BlobSHA != nil { ops[i].BlobSHA = *op.BlobSHA @@ -564,7 +575,7 @@ func (s *Service) Commit(ctx context.Context, repositoryID int64, req domain.Com clean = s.lfsCleanFunc(ctx, repo, req.PusherID) } - change, err := s.storage.ApplyCommit(ctx, repo.StoragePath, spec, ops, clean) + change, err := s.storage.ApplyCommit(ctx, repo.StoragePath, spec, ops, clean, checkWrite) switch { case errors.Is(err, gitbackend.ErrInvalidBranch): return domain.CommitResult{}, ErrInvalidBranch @@ -574,6 +585,8 @@ func (s *Service) Commit(ctx context.Context, repositoryID int64, req domain.Com return domain.CommitResult{}, ErrRefNotFound case errors.Is(err, gitbackend.ErrPathNotFound): return domain.CommitResult{}, ErrPathNotFound + case errors.Is(err, gitbackend.ErrPathExists): + return domain.CommitResult{}, ErrPathConflict case errors.Is(err, gitbackend.ErrNotABlob): return domain.CommitResult{}, ErrNotAFile case errors.Is(err, gitbackend.ErrHeadMismatch): @@ -687,13 +700,13 @@ func (s *Service) dispatchPush(ctx context.Context, repo domain.Repository, req } } -func (s *Service) checkPathPolicies(ctx context.Context, repositoryID int64, ops []domain.CommitFileOp) error { +func (s *Service) pathPolicyChecker(ctx context.Context, repositoryID int64) (gitbackend.CheckWriteFunc, error) { rows, err := s.registry.ListRepositoryPathPolicies(ctx, repositoryID) if err != nil { - return err + return nil, err } if len(rows) == 0 { - return nil + return nil, nil } patterns := make([]string, 0, len(rows)) @@ -705,12 +718,7 @@ func (s *Service) checkPathPolicies(ctx context.Context, repositoryID int64, ops } } - for _, op := range ops { - if op.Delete { // never block for deletions - continue - } - - cleaned := path.Clean("/" + op.Path)[1:] + return func(cleaned string) error { if pattern, blocked := domain.PathBlocked(patterns, cleaned); blocked { // return nice error with reason, if present if reason := reasons[pattern]; reason != "" { @@ -718,8 +726,8 @@ func (s *Service) checkPathPolicies(ctx context.Context, repositoryID int64, ops } return fmt.Errorf("%w: %q matches %q", ErrPathBlocked, cleaned, pattern) } - } - return nil + return nil + }, nil } func toDomain(r gen.Repository) domain.Repository { diff --git a/internal/services/repositories/service_test.go b/internal/services/repositories/service_test.go index 5c30ed1..f2e50ba 100644 --- a/internal/services/repositories/service_test.go +++ b/internal/services/repositories/service_test.go @@ -90,7 +90,8 @@ type fakeStorage struct { applyChange gitbackend.RefChange applyErr error // optional hook to inspect (and exercise) what ApplyCommit received - applyFn func(spec gitbackend.CommitSpec, ops []gitbackend.CommitOp, clean gitbackend.CleanFunc) error + applyFn func(spec gitbackend.CommitSpec, ops []gitbackend.CommitOp, clean gitbackend.CleanFunc) error + checkWritePath []string } func (f fakeStorage) InitBare(ctx context.Context, storagePath string) error { @@ -149,7 +150,14 @@ func (f fakeStorage) WriteBlob(ctx context.Context, storagePath string, r io.Rea return f.writeBlobSHA, n, nil } -func (f fakeStorage) ApplyCommit(ctx context.Context, storagePath string, spec gitbackend.CommitSpec, ops []gitbackend.CommitOp, clean gitbackend.CleanFunc) (gitbackend.RefChange, error) { +func (f fakeStorage) ApplyCommit(ctx context.Context, storagePath string, spec gitbackend.CommitSpec, ops []gitbackend.CommitOp, clean gitbackend.CleanFunc, checkWrite gitbackend.CheckWriteFunc) (gitbackend.RefChange, error) { + for _, path := range f.checkWritePath { + if checkWrite != nil { + if err := checkWrite(path); err != nil { + return gitbackend.RefChange{}, err + } + } + } if f.applyFn != nil { if err := f.applyFn(spec, ops, clean); err != nil { return gitbackend.RefChange{}, err @@ -682,6 +690,7 @@ func TestCommit(t *testing.T) { Operations: []domain.CommitFileOp{ {Path: "run.sh", BlobSHA: stringPtr(blobSHA), Executable: true}, {Path: "old.txt", Delete: true}, + {MoveFrom: "plugins", Path: "server/plugins"}, }, } @@ -707,7 +716,8 @@ func TestCommit(t *testing.T) { if gotSpec.Branch != "main" || gotSpec.ExpectedOld != req.ExpectedHeadSHA || gotSpec.Author.Name != "api-user" { t.Errorf("spec = %+v", gotSpec) } - if len(gotOps) != 2 || gotOps[0].Mode != "100755" || !gotOps[1].Delete { + if len(gotOps) != 3 || gotOps[0].Mode != "100755" || !gotOps[1].Delete || + gotOps[2].MoveFrom != "plugins" || gotOps[2].Path != "server/plugins" { t.Errorf("ops = %+v", gotOps) } if gotClean == nil { @@ -796,6 +806,7 @@ func TestCommit(t *testing.T) { {gitbackend.ErrInvalidOps, ErrInvalidCommitOps}, {gitbackend.ErrRevNotFound, ErrRefNotFound}, {gitbackend.ErrPathNotFound, ErrPathNotFound}, + {gitbackend.ErrPathExists, ErrPathConflict}, {gitbackend.ErrNotABlob, ErrNotAFile}, {gitbackend.ErrHeadMismatch, ErrHeadMismatch}, {gitbackend.ErrUnknownBlob, ErrUnknownBlob}, @@ -987,4 +998,17 @@ func TestCommitPathPolicies(t *testing.T) { t.Errorf("reason missing from error: %v", err) } }) + + t.Run("move descendants are checked", func(t *testing.T) { + st := fakeStorage{ + applyChange: change, + checkWritePath: []string{"runtime/moved/state.json"}, + } + svc := NewService(zap.NewNop(), fakeRegistry{repo: row, policies: policies}, st, nil, nil) + req := base + req.Operations = []domain.CommitFileOp{{MoveFrom: "plugins", Path: "runtime/moved"}} + if _, err := svc.Commit(context.Background(), row.ID, req); !errors.Is(err, ErrPathBlocked) { + t.Fatalf("want ErrPathBlocked, got %v", err) + } + }) }