Commit fa31441639
Verified · cmc
Layout: unified · split
internal/control/ghimport.go +16 −4
| @@ -14,6 +14,7 @@ import ( | ||
| 14 | 14 | "strings" |
| 15 | 15 | "time" |
| 16 | 16 | |
| 17 | "gitbay.org/gitbay/internal/gitpin" | |
| 17 | 18 | "gitbay.org/gitbay/internal/gitutil" |
| 18 | 19 | "gitbay.org/gitbay/internal/policy" |
| 19 | 20 | "gitbay.org/gitbay/internal/protocol" |
| @@ -361,8 +362,21 @@ esac | ||
| 361 | 362 | // Forgejo both publish pull heads under that name. Reports whether it |
| 362 | 363 | // worked; a failure is not fatal, since importing issues from a |
| 363 | 364 | // repository whose git data is not here yet is a reasonable thing to do. |
| 365 | // git connects only to the addresses the remote's host resolved to and | |
| 366 | // passed the check here, as repo import does (#298, #301). | |
| 364 | 367 | func fetchPullHeads(c *Ctx, dir, remote, token string) bool { |
| 365 | env := []string{"GIT_TERMINAL_PROMPT=0", "HOME=" + c.Cfg.Server.Root} | |
| 368 | ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) | |
| 369 | defer cancel() | |
| 370 | if err := gitpin.CheckGit(ctx); err != nil { | |
| 371 | fmt.Fprintf(c.Stderr, "pull heads not fetched: %v\n", err) | |
| 372 | return false | |
| 373 | } | |
| 374 | pinned, err := gitpin.Resolve(ctx, importLookup, remote, c.Cfg.Webhooks.AllowLocal) | |
| 375 | if err != nil { | |
| 376 | fmt.Fprintf(c.Stderr, "pull heads not fetched: %v\n", err) | |
| 377 | return false | |
| 378 | } | |
| 379 | env := gitpin.Env(c.Cfg.Server.Root) | |
| 366 | 380 | if token != "" { |
| 367 | 381 | askpass := filepath.Join(c.Cfg.Server.Root, "gh-import-askpass.sh") |
| 368 | 382 | if err := os.WriteFile(askpass, []byte(ghAskpass), 0o700); err != nil { |
| @@ -370,9 +384,7 @@ func fetchPullHeads(c *Ctx, dir, remote, token string) bool { | ||
| 370 | 384 | } |
| 371 | 385 | env = append(env, "GIT_ASKPASS="+askpass, "GITBAY_GH_TOKEN="+token) |
| 372 | 386 | } |
| 373 | ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) | |
| 374 | defer cancel() | |
| 375 | if err := gitutil.FetchPullHeads(ctx, dir, remote, io.Discard, env); err != nil { | |
| 387 | if err := gitutil.FetchPullHeads(ctx, dir, remote, io.Discard, pinned.Args(), env); err != nil { | |
| 376 | 388 | return false |
| 377 | 389 | } |
| 378 | 390 | return true |
internal/control/ghimport_test.go added +105
| @@ -0,0 +1,105 @@ | ||
| 1 | package control | |
| 2 | ||
| 3 | import ( | |
| 4 | "context" | |
| 5 | "net" | |
| 6 | "net/http/cgi" | |
| 7 | "net/http/httptest" | |
| 8 | "net/url" | |
| 9 | "os" | |
| 10 | "path/filepath" | |
| 11 | "slices" | |
| 12 | "strings" | |
| 13 | "testing" | |
| 14 | ||
| 15 | "gitbay.org/gitbay/internal/gitutil" | |
| 16 | ) | |
| 17 | ||
| 18 | // pullUpstream serves a bare repository whose refs/pull/1/head is one | |
| 19 | // commit over smart HTTP, and returns its port and that commit. | |
| 20 | func pullUpstream(t *testing.T) (string, string) { | |
| 21 | t.Helper() | |
| 22 | git := gitRunner(t) | |
| 23 | parent := t.TempDir() | |
| 24 | bare := filepath.Join(parent, "o", "r.git") | |
| 25 | work := filepath.Join(parent, "work") | |
| 26 | git(parent, "init", "-q", "--bare", "--initial-branch=main", bare) | |
| 27 | git(parent, "init", "-q", "--initial-branch=main", work) | |
| 28 | git(work, "commit", "-q", "--allow-empty", "-m", "pr") | |
| 29 | git(work, "push", "-q", bare, "HEAD:refs/pull/1/head") | |
| 30 | sha := strings.TrimSpace(git(work, "rev-parse", "HEAD")) | |
| 31 | execPath := strings.TrimSpace(git(parent, "--exec-path")) | |
| 32 | srv := httptest.NewServer(&cgi.Handler{ | |
| 33 | Path: filepath.Join(execPath, "git-http-backend"), | |
| 34 | Env: []string{"GIT_PROJECT_ROOT=" + parent, "GIT_HTTP_EXPORT_ALL=1"}, | |
| 35 | }) | |
| 36 | t.Cleanup(srv.Close) | |
| 37 | u, _ := url.Parse(srv.URL) | |
| 38 | return u.Port(), sha | |
| 39 | } | |
| 40 | ||
| 41 | // stubLookup answers every host with ip and records what was asked. | |
| 42 | func stubLookup(t *testing.T, ip string) *[]string { | |
| 43 | t.Helper() | |
| 44 | var asked []string | |
| 45 | prev := importLookup | |
| 46 | importLookup = func(ctx context.Context, host string) ([]net.IP, error) { | |
| 47 | asked = append(asked, host) | |
| 48 | return []net.IP{net.ParseIP(ip)}, nil | |
| 49 | } | |
| 50 | t.Cleanup(func() { importLookup = prev }) | |
| 51 | return &asked | |
| 52 | } | |
| 53 | ||
| 54 | // pulls.test does not resolve, and the daemon's environment points git | |
| 55 | // at a proxy and rewrites the URL to a dead port: the fetch works only | |
| 56 | // because git was pinned to the checked address and ran with its own | |
| 57 | // environment (#301). | |
| 58 | func TestFetchPullHeadsIsPinned(t *testing.T) { | |
| 59 | port, sha := pullUpstream(t) | |
| 60 | t.Setenv("http_proxy", "http://127.0.0.1:1") | |
| 61 | t.Setenv("HTTP_PROXY", "http://127.0.0.1:1") | |
| 62 | global := filepath.Join(t.TempDir(), "gitconfig") | |
| 63 | rewrite := "[url \"http://127.0.0.1:1/\"]\n\tinsteadOf = http://pulls.test:" + port + "/\n" | |
| 64 | if err := os.WriteFile(global, []byte(rewrite), 0o644); err != nil { | |
| 65 | t.Fatal(err) | |
| 66 | } | |
| 67 | t.Setenv("GIT_CONFIG_GLOBAL", global) | |
| 68 | c, errOut, _, root := importCtx(t, true) | |
| 69 | asked := stubLookup(t, "127.0.0.1") | |
| 70 | dir := filepath.Join(root, "dest.git") | |
| 71 | if err := gitutil.InitBare(dir, "main", ""); err != nil { | |
| 72 | t.Fatal(err) | |
| 73 | } | |
| 74 | if !fetchPullHeads(c, dir, "http://pulls.test:"+port+"/o/r.git", "") { | |
| 75 | t.Fatalf("fetch failed: %s", errOut.String()) | |
| 76 | } | |
| 77 | got := strings.TrimSpace(gitRunner(t)(dir, "rev-parse", "refs/gh-pull/1")) | |
| 78 | if got != sha { | |
| 79 | t.Fatalf("refs/gh-pull/1 = %s, want %s", got, sha) | |
| 80 | } | |
| 81 | if !slices.Equal(*asked, []string{"pulls.test"}) { | |
| 82 | t.Fatalf("looked up %v", *asked) | |
| 83 | } | |
| 84 | } | |
| 85 | ||
| 86 | // A git host that resolves to loopback is refused on a default | |
| 87 | // instance; the fetch reports it and fetches nothing. | |
| 88 | func TestFetchPullHeadsRefusesALocalAddress(t *testing.T) { | |
| 89 | port, _ := pullUpstream(t) | |
| 90 | c, errOut, _, root := importCtx(t, false) | |
| 91 | stubLookup(t, "127.0.0.1") | |
| 92 | dir := filepath.Join(root, "dest.git") | |
| 93 | if err := gitutil.InitBare(dir, "main", ""); err != nil { | |
| 94 | t.Fatal(err) | |
| 95 | } | |
| 96 | if fetchPullHeads(c, dir, "http://pulls.test:"+port+"/o/r.git", "sekrit") { | |
| 97 | t.Fatal("fetched from a local address") | |
| 98 | } | |
| 99 | if !strings.Contains(errOut.String(), "private or local address") || strings.Contains(errOut.String(), "sekrit") { | |
| 100 | t.Fatalf("stderr %q", errOut.String()) | |
| 101 | } | |
| 102 | if refExists(dir, "refs/gh-pull/1") { | |
| 103 | t.Fatal("refs/gh-pull/1 was fetched") | |
| 104 | } | |
| 105 | } | |
internal/control/import.go +2 −1
| @@ -39,7 +39,8 @@ case "$1" in | ||
| 39 | 39 | esac |
| 40 | 40 | ` |
| 41 | 41 | |
| 42 | // importLookup resolves an import's host; tests replace it. | |
| 42 | // importLookup resolves the hosts repo import and repo import-issues | |
| 43 | // connect to; tests replace it. | |
| 43 | 44 | var importLookup gitpin.Lookup = gitpin.LookupIP |
| 44 | 45 | |
| 45 | 46 | func runRepoImport(c *Ctx, args []string) int { |
internal/gitutil/gitutil.go +6 −5
| @@ -242,12 +242,13 @@ func FetchMirror(ctx context.Context, dir, url string, errW io.Writer, pin, env | ||
| 242 | 242 | // |
| 243 | 243 | // One fetch for every pull request rather than one each: the ref count is |
| 244 | 244 | // the repository's history, and asking a hundred times is a hundred |
| 245 | // handshakes. extraEnv carries credentials via GIT_ASKPASS; the URL must | |
| 246 | // never contain them. | |
| 247 | func FetchPullHeads(ctx context.Context, dir, url string, errW io.Writer, extraEnv []string) error { | |
| 248 | cmd := exec.CommandContext(ctx, toolpath.Look("git"), "-C", dir, "fetch", "--no-write-fetch-head", "--no-tags", | |
| 245 | // handshakes. pin and env are as for FetchMirror; env carries | |
| 246 | // credentials via GIT_ASKPASS, and the URL must never contain them. | |
| 247 | func FetchPullHeads(ctx context.Context, dir, url string, errW io.Writer, pin, env []string) error { | |
| 248 | args := append(append([]string{}, pin...), "-C", dir, "fetch", "--no-write-fetch-head", "--no-tags", | |
| 249 | 249 | url, "+refs/pull/*/head:refs/gh-pull/*") |
| 250 | cmd.Env = append(os.Environ(), extraEnv...) | |
| 250 | cmd := exec.CommandContext(ctx, toolpath.Look("git"), args...) | |
| 251 | cmd.Env = env | |
| 251 | 252 | cmd.Stderr = errW |
| 252 | 253 | if err := cmd.Run(); err != nil { |
| 253 | 254 | return fmt.Errorf("fetch pull heads from %s: %w", url, err) |