v5.17.1 regression: fixPermissions sets pack files to 0444, breaking subsequent installs into the same directory #1942

Summary

v5.17.1 introduced fixPermissions() which chmods .idx and .pack files to 0o444 (read-only) after writing them. Combined with the upgrade of go-billy to v5.8.0 (which made ChrootHelper.Chmod actually execute), this causes subsequent clones/installs into the same destination directory to fail with:

open .git/objects/pack/pack-<HASH>.idx: permission denied

Reverting to v5.16.5 fixes the issue.

Root cause

Two changes combined to create the bug

Change 1 — fixPermissions() added in storage/filesystem/dotgit/writers_unix.go:

func fixPermissions(fs billy.Filesystem, path string) {
    if chmodFS, ok := fs.(billy.Chmod); ok {
        if err := chmodFS.Chmod(path, 0o444); err != nil {
            trace.General.Printf("failed to chmod %s: %v", path, err)
        }
    }
}

Called after writing .idx and .pack files (lines 148, 154, 303 in writers.go).

Change 2 — go-billy upgraded from v5.6.2 → v5.8.0:

• go-billy v5.7.0 introduced billy.Chmod as a dedicated interface (previously Chmod was only embedded inside billy.Change with no standalone interface type).
• go-billy v5.8.0 added ChrootHelper.Chmod implementing that interface:

func (fs *ChrootHelper) Chmod(path string, mode os.FileMode) error {
    fullpath, err := fs.underlyingPath(path)
    ...
    return c.Chmod(fullpath, mode)
}

With v5.16.5 + go-billy v5.6.2: ChrootOS/ChrootHelper did not implement the standalone billy.Chmod interface (only the embedded-in-billy.Change version), so the type assertion fs.(billy.Chmod) returned ok=false. fixPermissions was a no-op. Pack files kept their default mode (0644).

With v5.17.1 + go-billy v5.8.0: The type assertion succeeds. Chmod(path, 0o444) is called on every .idx and .pack file written into the filesystem — including temporary shadow directories used by callers.

Failure scenario

Any code that:

1. Uses go-git to clone/fetch into a temporary location, then
2. Recursively copies that location to a persistent destination (using os.Create or any O_WRONLY/O_RDWR open), then
3. Repeats the operation on the same destination

…will fail on the second iteration.

Step-by-step:

1. Run 1: go-git clones into shadow dir → fixPermissions sets shadow's .idx to 0o444 → copy tool copies shadow → dest → os.Create(dest) succeeds (file is new) → copy tool preserves permissions → dest .idx now has 0o444
2. Run 2: go-git clones into new shadow dir → fixPermissions sets shadow's .idx to 0o444 → copy tool tries os.Create(dest) → dest exists with 0o444 → kernel returns EACCES → open .git/objects/pack/pack-XXX.idx: permission denied

The open in the error message is the open(2) syscall name emitted by Go's os.Create (which is open(O_RDWR|O_CREATE|O_TRUNC)). A 0o444 file has no write bits for anyone, including the owner, so the syscall fails.

Affected versions

Suggested fix

The intent of fixPermissions — protecting pack files from accidental modification — is reasonable. However, 0o444 (no write for anyone) is more restrictive than necessary and breaks callers that legitimately need to overwrite pack files during subsequent operations on the same path.

Option A: Change the mode to 0o444 only when writing to the final (non-temporary) pack storage location, not in shadow/working directories.

Option B: Use 0o644 instead of 0o444. This preserves the "this file should not normally be written" signal while not breaking os.Create on existing files.

Option C: Document that callers using go-git to write into directories that are later overwritten via recursive copy must walk and chmod 0644 all pack files before the copy.

Option B is the most backward-compatible with the least blast radius.