Use os.Root for path checking os.Root uses system primitive rather than stringy securejoin. Ideally, git.Open should accept fs.FS (or *os.Root) but go-git in its current form (even v6) does not expose a function for "smart" open method that can open both bare and non-bare repositories. This patch also accidentally fixes following bugs: * checked out "description" file in a non-bare repository is treated as a description file. * ignored repository reveal using URL-encoded slashes (see the second test added in the parent commit) Load tests on a minimal setup (nix run .#testing) shows no differences before and after this patch. This is not a clean solution, but code is less duplicated and more correct and secure. I may revisit and refactor in near future.

Changes

14 changed files (+141/-157)

• git/git.go
  • @@ -7,11 +7,13 @@ import (
    "io/fs" "path" "sort" + "strings" "time" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/storage/filesystem" ) type GitRepo struct {
  • @@ -104,6 +106,29 @@ func Open(path string, ref string) (*GitRepo, error) {
    return &g, nil } + // GitwebDescription returns description text. + // See https://git-scm.com/docs/gitweb#Documentation/gitweb.txt-descriptionorgitwebdescription + func (r *GitRepo) GitwebDescription() string { + if storage, ok := r.r.Storer.(*filesystem.Storage); ok { + file, err := storage.Filesystem().Open("description") + if err == nil { + defer file.Close() + contents, err := io.ReadAll(file) + if err == nil { + text := string(contents) + // git by default copies "description" file from template directory, + // and there is no way to detect it other than this "heuristic". + if strings.Index(text, "Unnamed repository;") != 0 { + return text + } + } + } + } + + // TODO: Read from "gitweb.description" + return "" + } + func (g *GitRepo) LastCommit() (*object.Commit, error) { c, err := g.r.CommitObject(g.h) if err != nil {
• go.mod
  • @@ -5,7 +5,6 @@ go 1.25
    require ( github.com/alecthomas/chroma/v2 v2.27.0 github.com/bluekeyes/go-gitdiff v0.8.0 - github.com/cyphar/filepath-securejoin v0.4.1 github.com/go-git/go-billy/v5 v5.6.2 github.com/go-git/go-git/v5 v5.13.2 github.com/landlock-lsm/go-landlock v0.9.0
  • @@ -21,6 +20,7 @@ require (
    github.com/ProtonMail/go-crypto v1.1.5 // indirect github.com/aymerick/douceur v0.2.0 // indirect github.com/cloudflare/circl v1.6.0 // indirect + github.com/cyphar/filepath-securejoin v0.4.1 // indirect github.com/dlclark/regexp2/v2 v2.2.1 // indirect github.com/emirpasic/gods v1.18.1 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
• main.go
  • @@ -165,7 +165,12 @@ func main() {
    templatesDir = embed.TemplatesDir() } - mux := routes.Handler(c, staticDir, templatesDir) + scanDir, err := os.OpenRoot(c.Repo.ScanPath) + if err != nil { + log.Fatal(err) + } + + mux := routes.Handler(c, scanDir, staticDir, templatesDir) addr := fmt.Sprintf("%s:%d", c.Server.Host, c.Server.Port) log.Println("starting server on", addr) log.Fatal(http.ListenAndServe(addr, mux))
• routes/deps.go
  • @@ -4,19 +4,29 @@
    package routes import ( + "errors" "html/template" "io/fs" "log" "net/http" + "os" + "path/filepath" "github.com/pocka/legit/config" + "github.com/pocka/legit/git" "github.com/pocka/legit/renderer/html" ) + var ( + errRepositoryIsIgnored = errors.New("repository is ignored") + ) + // deps holds data required for serving routes. Dependencies. type deps struct { c *config.Config + scanRoot *os.Root + // staticDir should be path traversal attack resilient FS, such as the one // returned by "os.Root.FS". staticDir fs.FS
  • @@ -31,6 +41,32 @@ type deps struct {
    plaintext html.PlaintextRenderer } + // resolveRepository returns filepath to a repository. + func (d *deps) resolveRepository(name string) (string, error) { + entry, err := d.scanRoot.Stat(name) + if err != nil { + return "", err + } + + dirname := entry.Name() + if d.isIgnored(dirname) { + return "", errRepositoryIsIgnored + } + + return filepath.Join(d.scanRoot.Name(), dirname), nil + } + + func (d *deps) openRepository(name string, ref string) (repo *git.GitRepo, dirname string, err error) { + path, err := d.resolveRepository(name) + if err != nil { + return nil, "", err + } + + dirname = filepath.Base(path) + repo, err = git.Open(path, ref) + return + } + // template returns compiled HTML templates. // Every routes should access templates through this method. func (d *deps) template() *template.Template {
• routes/git.go
  • @@ -5,19 +5,13 @@ import (
    "io" "log" "net/http" - "path/filepath" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/pocka/legit/git/service" ) func (d *deps) serveInfoRefs(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - name = filepath.Clean(name) - - repo, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) + repo, err := d.resolveRepository(r.PathValue("name")) if err != nil { - log.Printf("securejoin error: %v", err) d.write404(w) return }
  • @@ -38,12 +32,8 @@ func (d *deps) serveInfoRefs(w http.ResponseWriter, r *http.Request) {
    } func (d *deps) serveUploadPack(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - name = filepath.Clean(name) - - repo, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) + repo, err := d.resolveRepository(r.PathValue("name")) if err != nil { - log.Printf("securejoin error: %v", err) d.write404(w) return }
• routes/git_test.go
  • @@ -54,7 +54,13 @@ func TestGitCloneOK(t *testing.T) {
    c.Repo.Readme = []string{"README.md"} c.Repo.MainBranch = []string{"trunk"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(repos) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() cloneURL, err := url.JoinPath(server.URL, "/foo")
  • @@ -129,7 +135,13 @@ I'm [Markdown](https://commonmark.org/) file for *load* **testing**.
    c.Repo.Readme = []string{"README.md"} c.Repo.MainBranch = []string{"trunk"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(repos) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() cloneURL, err := url.JoinPath(server.URL, "/foo.git")
  • @@ -204,7 +216,13 @@ func TestCloneRespectVisibilityConfig(t *testing.T) {
    c.Repo.Ignore = []string{"ignored.git"} c.Repo.Unlisted = []string{"unlisted.git"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(repos) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() publicUrl, err := url.JoinPath(server.URL, "/public.git")
• routes/handler.go
  • @@ -4,6 +4,7 @@ import (
    "html/template" "io/fs" "net/http" + "os" "github.com/microcosm-cc/bluemonday" "github.com/pocka/legit/config"
  • @@ -39,12 +40,13 @@ func (d *deps) multiplex(w http.ResponseWriter, r *http.Request) {
    } } - func Handler(c *config.Config, staticDir fs.FS, templatesDir fs.FS) *http.ServeMux { + func Handler(c *config.Config, scanRoot *os.Root, staticDir fs.FS, templatesDir fs.FS) *http.ServeMux { mux := http.NewServeMux() ugcPolicy := bluemonday.UGCPolicy() d := deps{ c: c, + scanRoot: scanRoot, staticDir: staticDir, templatesDir: templatesDir, markdown: html.NewMarkdownRenderer(ugcPolicy),
• routes/handler_test.go
  • @@ -58,7 +58,13 @@ func TestIgnoreSymlinkByDefault(t *testing.T) {
    c.Repo.Readme = []string{"README.md"} c.Repo.MainBranch = []string{"trunk"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(dest) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() topPage, err := http.Get(server.URL)
• routes/routes.go
  • @@ -9,9 +9,6 @@ import (
    "path/filepath" "sort" "strings" - - securejoin "github.com/cyphar/filepath-securejoin" - "github.com/pocka/legit/git" ) func (d *deps) serveIndex(w http.ResponseWriter, r *http.Request) {
  • @@ -25,21 +22,17 @@ func (d *deps) serveIndex(w http.ResponseWriter, r *http.Request) {
    summaries := []repositorySummary{} for _, dir := range dirs { - name := dir.Name() - if !dir.IsDir() || d.isIgnored(name) || d.isUnlisted(name) { + if !dir.IsDir() { continue } - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) + gr, name, err := d.openRepository(dir.Name(), "") if err != nil { - log.Printf("securejoin error: %v", err) d.write404(w) return } - gr, err := git.Open(path, "") - if err != nil { - log.Println(err) + if d.isIgnored(name) || d.isUnlisted(name) { continue }
  • @@ -53,7 +46,7 @@ func (d *deps) serveIndex(w http.ResponseWriter, r *http.Request) {
    summaries = append(summaries, repositorySummary{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), LastCommit: c, }) }
  • @@ -74,26 +67,13 @@ func (d *deps) serveIndex(w http.ResponseWriter, r *http.Request) {
    } func (d *deps) serveRepoTree(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } - treePath := r.PathValue("rest") ref := r.PathValue("ref") - - name = filepath.Clean(name) - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - gr, err := git.Open(path, ref) + gr, name, err := d.openRepository(r.PathValue("name"), ref) if err != nil { d.write404(w) return } + treePath := r.PathValue("rest") files, err := gr.FileTree(treePath) if err != nil {
  • @@ -112,7 +92,7 @@ func (d *deps) serveRepoTree(w http.ResponseWriter, r *http.Request) {
    Meta: repositoryMeta{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), Ref: ref, }, Path: relpath,
  • @@ -127,11 +107,6 @@ func (d *deps) serveRepoTree(w http.ResponseWriter, r *http.Request) {
    func (d *deps) serveArchive(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } - file := r.PathValue("file") // TODO: extend this to add more files compression (e.g.: xz)
  • @@ -148,14 +123,7 @@ func (d *deps) serveArchive(w http.ResponseWriter, r *http.Request) {
    setContentDisposition(w, filename) setGZipMIME(w) - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - - gr, err := git.Open(path, ref) + gr, name, err := d.openRepository(name, ref) if err != nil { d.write404(w) return
  • @@ -183,20 +151,8 @@ func (d *deps) serveArchive(w http.ResponseWriter, r *http.Request) {
    } func (d *deps) serveDiff(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } ref := r.PathValue("ref") - - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - gr, err := git.Open(path, ref) + gr, name, err := d.openRepository(r.PathValue("name"), ref) if err != nil { d.write404(w) return
  • @@ -214,7 +170,7 @@ func (d *deps) serveDiff(w http.ResponseWriter, r *http.Request) {
    Meta: repositoryMeta{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), Ref: diff.Commit.Hash.String(), }, Commit: diff.Commit,
  • @@ -229,20 +185,7 @@ func (d *deps) serveDiff(w http.ResponseWriter, r *http.Request) {
    } func (d *deps) serveRefs(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } - - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - - gr, err := git.Open(path, "") + gr, name, err := d.openRepository(r.PathValue("name"), "") if err != nil { d.write404(w) return
  • @@ -273,7 +216,7 @@ func (d *deps) serveRefs(w http.ResponseWriter, r *http.Request) {
    Meta: repositoryMeta{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), Ref: mainBranch, }, Tags: tags,
• routes/servefilecontent.go
  • @@ -4,37 +4,19 @@ import (
    "html/template" "log" "net/http" - "path/filepath" "strings" - - securejoin "github.com/cyphar/filepath-securejoin" - "github.com/pocka/legit/git" ) func (d *deps) serveFileContent(w http.ResponseWriter, r *http.Request) { raw := r.URL.Query().Has("raw") - name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } - treePath := r.PathValue("rest") ref := r.PathValue("ref") - - name = filepath.Clean(name) - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - - gr, err := git.Open(path, ref) + gr, name, err := d.openRepository(r.PathValue("name"), ref) if err != nil { d.write404(w) return } + treePath := r.PathValue("rest") file, err := gr.File(treePath) if err != nil {
  • @@ -62,7 +44,7 @@ func (d *deps) serveFileContent(w http.ResponseWriter, r *http.Request) {
    meta := repositoryMeta{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), Ref: ref, }
• routes/servelog.go
  • @@ -5,27 +5,13 @@ import (
    "log" "net/http" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/go-git/go-git/v5/plumbing" "github.com/pocka/legit/git" ) func (d *deps) serveLog(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } ref := r.PathValue("ref") - - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - - gr, err := git.Open(path, ref) + gr, name, err := d.openRepository(r.PathValue("name"), ref) if err != nil { d.write404(w) return
  • @@ -68,7 +54,7 @@ func (d *deps) serveLog(w http.ResponseWriter, r *http.Request) {
    Meta: repositoryMeta{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), Ref: ref, }, Commits: commits,
• routes/serverepoindex.go
  • @@ -4,27 +4,12 @@ import (
    "html/template" "log" "net/http" - "path/filepath" - securejoin "github.com/cyphar/filepath-securejoin" "github.com/pocka/legit/git" ) func (d *deps) serveRepoIndex(w http.ResponseWriter, r *http.Request) { - name := r.PathValue("name") - if d.isIgnored(name) { - d.write404(w) - return - } - name = filepath.Clean(name) - path, err := securejoin.SecureJoin(d.c.Repo.ScanPath, name) - if err != nil { - log.Printf("securejoin error: %v", err) - d.write404(w) - return - } - - gr, err := git.Open(path, "") + gr, name, err := d.openRepository(r.PathValue("name"), "") if err != nil { d.write404(w) return
  • @@ -78,7 +63,7 @@ func (d *deps) serveRepoIndex(w http.ResponseWriter, r *http.Request) {
    Meta: repositoryMeta{ DisplayName: getDisplayName(name), DirName: name, - Description: getDescription(path), + Description: gr.GitwebDescription(), Ref: mainBranch, }, Readme: readmeContent,
• routes/serverepoindex_test.go
  • @@ -54,7 +54,13 @@ func TestServeRepoIndexOK(t *testing.T) {
    c.Repo.Readme = []string{"README.md"} c.Repo.MainBranch = []string{"trunk"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(repos) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() target, err := url.JoinPath(server.URL, "/foo")
  • @@ -155,7 +161,13 @@ func TestServeRepoIndexPreventPathTraversal(t *testing.T) {
    c.Repo.Readme = []string{"README.md"} c.Repo.MainBranch = []string{"trunk"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(child) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() target, err := url.JoinPath(server.URL, "..%2Fprivate")
  • @@ -240,7 +252,13 @@ func TestServeRepoIndexPreventIgnoredRepoReveal(t *testing.T) {
    c.Repo.MainBranch = []string{"trunk"} c.Repo.Ignore = []string{"private"} - server := httptest.NewServer(Handler(&c, embed.StaticDir(), embed.TemplatesDir())) + scanRoot, err := os.OpenRoot(repos) + if err != nil { + t.Fatal(err) + } + defer scanRoot.Close() + + server := httptest.NewServer(Handler(&c, scanRoot, embed.StaticDir(), embed.TemplatesDir())) defer server.Close() target, err := url.JoinPath(server.URL, ".%2Fprivate")
• routes/util.go
  • @@ -6,8 +6,6 @@ import (
    "html/template" "io" "net/http" - "os" - "path/filepath" "slices" "strings"
  • @@ -27,16 +25,6 @@ func getDisplayName(name string) string {
    return strings.TrimSuffix(name, ".git") } - func getDescription(path string) (desc string) { - db, err := os.ReadFile(filepath.Join(path, "description")) - if err == nil { - desc = string(db) - } else { - desc = "" - } - return - } - func (d *deps) isUnlisted(name string) bool { return slices.Contains(d.c.Repo.Unlisted, name) }