Skip to content

storage: dotgit, fix pack file permissions#1802

Merged
pjbgf merged 1 commit intogo-git:mainfrom
Ch00k:fix-pack-permissions
Dec 23, 2025
Merged

storage: dotgit, fix pack file permissions#1802
pjbgf merged 1 commit intogo-git:mainfrom
Ch00k:fix-pack-permissions

Conversation

@Ch00k
Copy link
Contributor

@Ch00k Ch00k commented Dec 15, 2025

Fixes #588

@Ch00k Ch00k changed the title Fix pack file permissions storage: dotgit, fix pack file permissions Dec 15, 2025
@Ch00k Ch00k force-pushed the fix-pack-permissions branch 2 times, most recently from 52d5aef to 96310a5 Compare December 15, 2025 08:15
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ch00k thanks for looking into this. Here's some feedback:

@Ch00k Ch00k force-pushed the fix-pack-permissions branch 7 times, most recently from 1708183 to 97f5788 Compare December 23, 2025 06:40
@pjbgf pjbgf force-pushed the fix-pack-permissions branch from 97f5788 to c74ed0e Compare December 23, 2025 21:09
@Ch00k
Copy link
Contributor Author

Ch00k commented Dec 23, 2025

@pjbgf I did some more research on Windows, and came to a conclusion that fixing pack file permissions is not necessary there. This is what I found.

A fresh clonse of a repository with Git command line.

PS C:\Users\aycp> git clone https://github.com/go-git/go-billy.git
Cloning into 'go-billy'...
remote: Enumerating objects: 2424, done.
remote: Counting objects: 100% (452/452), done.
remote: Compressing objects: 100% (183/183), done.
remote: Total 2424 (delta 369), reused 277 (delta 269), pack-reused 1972 (from 3)
Receiving objects: 100% (2424/2424), 665.80 KiB | 6.05 MiB/s, done.
Resolving deltas: 100% (1307/1307), done.
PS C:\Users\aycp>
PS C:\Users\aycp>
PS C:\Users\aycp> icacls .\go-billy\.git\objects\pack\*
.\go-billy\.git\objects\pack\pack-c7d2a28e9530e0dcab4b24de493ec350f7dff5de.idx NT AUTHORITY\SYSTEM:(I)(F)
                                                                               BUILTIN\Administrators:(I)(F)
                                                                               AYWIN\aycp:(I)(F)

.\go-billy\.git\objects\pack\pack-c7d2a28e9530e0dcab4b24de493ec350f7dff5de.pack NT AUTHORITY\SYSTEM:(I)(F)
                                                                                BUILTIN\Administrators:(I)(F)
                                                                                AYWIN\aycp:(I)(F)

.\go-billy\.git\objects\pack\pack-c7d2a28e9530e0dcab4b24de493ec350f7dff5de.rev NT AUTHORITY\SYSTEM:(I)(F)
                                                                               BUILTIN\Administrators:(I)(F)
                                                                               AYWIN\aycp:(I)(F)

Successfully processed 3 files; Failed processing 0 files

The (I)(F) means Inherited Full Control, which translates to 777 in Unix permission notation.

A fresh clone made with git-go:

PS C:\Users\aycp> cat .\test_clone\go.mod
module test_clone

go 1.24.0

require github.com/go-git/go-git/v6 v6.0.0-20251223165309-e697feb6e2c6

require (
 github.com/Microsoft/go-winio v0.6.2 // indirect
 github.com/ProtonMail/go-crypto v1.3.0 // indirect
 github.com/cloudflare/circl v1.6.1 // indirect
 github.com/cyphar/filepath-securejoin v0.6.1 // indirect
 github.com/emirpasic/gods v1.18.1 // indirect
 github.com/go-git/gcfg/v2 v2.0.2 // indirect
 github.com/go-git/go-billy/v6 v6.0.0-20251209065551-8afc3eb64e4d // indirect
 github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect
 github.com/kevinburke/ssh_config v1.4.0 // indirect
 github.com/klauspost/cpuid/v2 v2.3.0 // indirect
 github.com/pjbgf/sha1cd v0.5.0 // indirect
 github.com/sergi/go-diff v1.4.0 // indirect
 golang.org/x/crypto v0.46.0 // indirect
 golang.org/x/net v0.48.0 // indirect
 golang.org/x/sys v0.39.0 // indirect
)
PS C:\Users\aycp> cat .\test_clone\test_clone.go
package main

import (
    "github.com/go-git/go-git/v6"
)

func main() {
    git.PlainClone("go-billy", &git.CloneOptions{
        URL: "https://github.com/go-git/go-billy",
    })
}

PS C:\Users\aycp> .\test_clone\test_clone.exe
PS C:\Users\aycp> icacls .\go-billy\.git\objects\pack\*
.\go-billy\.git\objects\pack\pack-82e696cfc8152a8084420cc19f61fd4c63aa3fa0.idx NT AUTHORITY\SYSTEM:(I)(F)
                                                                               BUILTIN\Administrators:(I)(F)
                                                                               AYWIN\aycp:(I)(F)

.\go-billy\.git\objects\pack\pack-82e696cfc8152a8084420cc19f61fd4c63aa3fa0.pack NT AUTHORITY\SYSTEM:(I)(F)
                                                                                BUILTIN\Administrators:(I)(F)
                                                                                AYWIN\aycp:(I)(F)

.\go-billy\.git\objects\pack\pack-82e696cfc8152a8084420cc19f61fd4c63aa3fa0.rev NT AUTHORITY\SYSTEM:(I)(F)
                                                                               BUILTIN\Administrators:(I)(F)
                                                                               AYWIN\aycp:(I)(F)

Successfully processed 3 files; Failed processing 0 files

The permissions are the same as the ones made by Git command line.

In my opinion the code and tests in the current state is correct: we should not be executing fixPermissions if running on Windows.

@pjbgf
Copy link
Member

pjbgf commented Dec 23, 2025

@Ch00k thanks for the clarifications. Let's go ahead and get this merged so that it is improved for posix. In the future, perhaps some compatibility tests could be added to just ensure that permissions in go-git are consistently aligned with git across the different OSes.

Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ch00k thanks for working on this. 🙇

The Fuzz failure seems to be orthogonal to the changes.

@pjbgf pjbgf merged commit 8cea26c into go-git:main Dec 23, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cloning repo has unexpected packfile permissions

2 participants