Commit 1c6440454b
Verified · cmc
Layout: unified · split
internal/control/import.go +27 −20
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | "strings" | 10 | "strings" |
| 11 | "time" | 11 | "time" |
| 12 | 12 | ||
| 13 | "gitbay.org/gitbay/internal/gitpin" | ||
| 13 | "gitbay.org/gitbay/internal/gitutil" | 14 | "gitbay.org/gitbay/internal/gitutil" |
| 14 | "gitbay.org/gitbay/internal/policy" | 15 | "gitbay.org/gitbay/internal/policy" |
| 15 | "gitbay.org/gitbay/internal/protocol" | 16 | "gitbay.org/gitbay/internal/protocol" |
| @@ -38,6 +39,9 @@ case "$1" in | |||
| 38 | esac | 39 | esac |
| 39 | ` | 40 | ` |
| 40 | 41 | ||
| 42 | // importLookup resolves an import's host; tests replace it. | ||
| 43 | var importLookup gitpin.Lookup = gitpin.LookupIP | ||
| 44 | |||
| 41 | func runRepoImport(c *Ctx, args []string) int { | 45 | func runRepoImport(c *Ctx, args []string) int { |
| 42 | f, err := c.parseArgs(args, flagSpec{Values: []string{"--from"}, Bools: []string{"--private", "--token-stdin"}, MaxPos: 1, | 46 | f, err := c.parseArgs(args, flagSpec{Values: []string{"--from"}, Bools: []string{"--private", "--token-stdin"}, MaxPos: 1, |
| 43 | Usage: "repo import <owner/name> --from <url> [--private] [--token-stdin]"}) | 47 | Usage: "repo import <owner/name> --from <url> [--private] [--token-stdin]"}) |
| @@ -77,12 +81,12 @@ func runRepoImport(c *Ctx, args []string) int { | |||
| 77 | } | 81 | } |
| 78 | } | 82 | } |
| 79 | 83 | ||
| 80 | // Scheme allowlist. file:// (and anything else local) would read the | 84 | // http and https only. git:// has no equivalent of curl's resolve |
| 81 | // server's filesystem; ssh:// would use the server's own keys. | 85 | // list, so its connection cannot be held to a checked address; |
| 82 | switch { | 86 | // file:// would read the server's filesystem, and ssh:// would use |
| 83 | case strings.HasPrefix(from, "https://"), strings.HasPrefix(from, "http://"), strings.HasPrefix(from, "git://"): | 87 | // the server's own keys. |
| 84 | default: | 88 | if !strings.HasPrefix(from, "https://") && !strings.HasPrefix(from, "http://") { |
| 85 | return c.fail(protocol.ExitUsage, "import supports https://, http://, and git:// URLs only") | 89 | return c.fail(protocol.ExitUsage, "import fetches over http:// and https:// only; use the repository's https:// URL") |
| 86 | } | 90 | } |
| 87 | if strings.ContainsAny(from, "@") { | 91 | if strings.ContainsAny(from, "@") { |
| 88 | // Credentials belong on stdin, not in the URL where they would | 92 | // Credentials belong on stdin, not in the URL where they would |
| @@ -90,9 +94,22 @@ func runRepoImport(c *Ctx, args []string) int { | |||
| 90 | return c.fail(protocol.ExitUsage, "do not embed credentials in the URL; use --token-stdin") | 94 | return c.fail(protocol.ExitUsage, "do not embed credentials in the URL; use --token-stdin") |
| 91 | } | 95 | } |
| 92 | 96 | ||
| 97 | // Resolve and check the host now and hold git to those addresses, | ||
| 98 | // as mirror sync does (#298). | ||
| 99 | timeout := time.Duration(c.Cfg.Limits.CloneTimeoutSec) * time.Second | ||
| 100 | ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| 101 | defer cancel() | ||
| 102 | if err := gitpin.CheckGit(ctx); err != nil { | ||
| 103 | return c.fail(protocol.ExitFailure, "import unavailable: %v", err) | ||
| 104 | } | ||
| 105 | remote, err := gitpin.Resolve(ctx, importLookup, from, c.Cfg.Webhooks.AllowLocal) | ||
| 106 | if err != nil { | ||
| 107 | return c.fail(protocol.ExitFailure, "%v", err) | ||
| 108 | } | ||
| 109 | |||
| 93 | // The token is read from stdin and handed to git via GIT_ASKPASS and | 110 | // The token is read from stdin and handed to git via GIT_ASKPASS and |
| 94 | // the environment — never argv, never the database, never a log line. | 111 | // the environment — never argv, never the database, never a log line. |
| 95 | var env []string | 112 | env := gitpin.Env(c.Cfg.Server.Root) |
| 96 | if tokenStdin { | 113 | if tokenStdin { |
| 97 | token, err := bufio.NewReader(io.LimitReader(c.Stdin, 4096)).ReadString('\n') | 114 | token, err := bufio.NewReader(io.LimitReader(c.Stdin, 4096)).ReadString('\n') |
| 98 | if err != nil && err != io.EOF { | 115 | if err != nil && err != io.EOF { |
| @@ -106,13 +123,7 @@ func runRepoImport(c *Ctx, args []string) int { | |||
| 106 | if err := os.WriteFile(askpass, []byte(askpassScript), 0o700); err != nil { | 123 | if err := os.WriteFile(askpass, []byte(askpassScript), 0o700); err != nil { |
| 107 | return c.fail(protocol.ExitFailure, "%v", err) | 124 | return c.fail(protocol.ExitFailure, "%v", err) |
| 108 | } | 125 | } |
| 109 | env = []string{ | 126 | env = append(env, "GIT_ASKPASS="+askpass, "GITBAY_IMPORT_TOKEN="+token) |
| 110 | "GIT_ASKPASS=" + askpass, | ||
| 111 | "GITBAY_IMPORT_TOKEN=" + token, | ||
| 112 | "GIT_TERMINAL_PROMPT=0", | ||
| 113 | } | ||
| 114 | } else { | ||
| 115 | env = []string{"GIT_TERMINAL_PROMPT=0"} | ||
| 116 | } | 127 | } |
| 117 | 128 | ||
| 118 | visibility := "public" | 129 | visibility := "public" |
| @@ -143,17 +154,13 @@ func runRepoImport(c *Ctx, args []string) int { | |||
| 143 | return c.fail(protocol.ExitFailure, "%v", err) | 154 | return c.fail(protocol.ExitFailure, "%v", err) |
| 144 | } | 155 | } |
| 145 | 156 | ||
| 146 | timeout := time.Duration(c.Cfg.Limits.CloneTimeoutSec) * time.Second | ||
| 147 | ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| 148 | defer cancel() | ||
| 149 | |||
| 150 | fmt.Fprintf(c.Stderr, "importing %s into %s ...\n", from, path) | 157 | fmt.Fprintf(c.Stderr, "importing %s into %s ...\n", from, path) |
| 151 | if err := gitutil.FetchMirror(ctx, dir, from, c.Stderr, env); err != nil { | 158 | if err := gitutil.FetchMirror(ctx, dir, from, c.Stderr, remote.Args(), env); err != nil { |
| 152 | cleanup() | 159 | cleanup() |
| 153 | return c.fail(protocol.ExitFailure, "import failed: %v", err) | 160 | return c.fail(protocol.ExitFailure, "import failed: %v", err) |
| 154 | } | 161 | } |
| 155 | 162 | ||
| 156 | branch, err := gitutil.RemoteDefaultBranch(ctx, from, env) | 163 | branch, err := gitutil.RemoteDefaultBranch(ctx, from, remote.Args(), env) |
| 157 | if err != nil { | 164 | if err != nil { |
| 158 | branch = "main" // remote gone quiet after the fetch; keep the default | 165 | branch = "main" // remote gone quiet after the fetch; keep the default |
| 159 | } | 166 | } |
internal/control/import_test.go added +111
| @@ -0,0 +1,111 @@ | |||
| 1 | package control | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "bytes" | ||
| 5 | "context" | ||
| 6 | "net" | ||
| 7 | "net/http/cgi" | ||
| 8 | "net/http/httptest" | ||
| 9 | "net/url" | ||
| 10 | "os" | ||
| 11 | "path/filepath" | ||
| 12 | "slices" | ||
| 13 | "strings" | ||
| 14 | "testing" | ||
| 15 | |||
| 16 | "gitbay.org/gitbay/internal/protocol" | ||
| 17 | "gitbay.org/gitbay/internal/store" | ||
| 18 | ) | ||
| 19 | |||
| 20 | func importCtx(t *testing.T, allowLocal bool) (*Ctx, *bytes.Buffer, *store.Store, string) { | ||
| 21 | t.Helper() | ||
| 22 | st, _, uid := newQueueTestRepo(t) | ||
| 23 | root := t.TempDir() | ||
| 24 | c, errOut := pruneCtx(st, root, store.User{ID: uid, Username: "alice"}) | ||
| 25 | c.Cfg.Limits.WriteRate = -1 | ||
| 26 | c.Cfg.Limits.CloneTimeoutSec = 60 | ||
| 27 | c.Cfg.Webhooks.AllowLocal = allowLocal | ||
| 28 | c.Stdin = strings.NewReader("") | ||
| 29 | return c, errOut, st, root | ||
| 30 | } | ||
| 31 | |||
| 32 | // importUpstream serves a bare repository with one commit on main over | ||
| 33 | // smart HTTP and returns its URL and that commit. | ||
| 34 | func importUpstream(t *testing.T) (string, string) { | ||
| 35 | t.Helper() | ||
| 36 | git := gitRunner(t) | ||
| 37 | parent := t.TempDir() | ||
| 38 | bare := filepath.Join(parent, "remote.git") | ||
| 39 | work := filepath.Join(parent, "work") | ||
| 40 | git(parent, "init", "-q", "--bare", "--initial-branch=main", bare) | ||
| 41 | git(parent, "init", "-q", "--initial-branch=main", work) | ||
| 42 | git(work, "commit", "-q", "--allow-empty", "-m", "one") | ||
| 43 | git(work, "push", "-q", bare, "main") | ||
| 44 | sha := strings.TrimSpace(git(work, "rev-parse", "HEAD")) | ||
| 45 | execPath := strings.TrimSpace(git(parent, "--exec-path")) | ||
| 46 | srv := httptest.NewServer(&cgi.Handler{ | ||
| 47 | Path: filepath.Join(execPath, "git-http-backend"), | ||
| 48 | Env: []string{"GIT_PROJECT_ROOT=" + parent, "GIT_HTTP_EXPORT_ALL=1"}, | ||
| 49 | }) | ||
| 50 | t.Cleanup(srv.Close) | ||
| 51 | return srv.URL + "/remote.git", sha | ||
| 52 | } | ||
| 53 | |||
| 54 | // git:// cannot be held to a checked address, so import refuses it | ||
| 55 | // before creating anything (#298). | ||
| 56 | func TestRepoImportRefusesGitScheme(t *testing.T) { | ||
| 57 | c, errOut, st, _ := importCtx(t, true) | ||
| 58 | code := Dispatch(c, []string{"repo", "import", "alice/x", "--from", "git://example.org/x.git"}) | ||
| 59 | if code != protocol.ExitUsage || !strings.Contains(errOut.String(), "use the repository's https:// URL") { | ||
| 60 | t.Fatalf("exit %d, %q", code, errOut.String()) | ||
| 61 | } | ||
| 62 | if _, err := st.RepoByPath("alice/x"); err == nil { | ||
| 63 | t.Fatal("a refused import created a repository") | ||
| 64 | } | ||
| 65 | } | ||
| 66 | |||
| 67 | // A source on loopback is refused on a default instance, and nothing is | ||
| 68 | // left behind. | ||
| 69 | func TestRepoImportRefusesALocalAddress(t *testing.T) { | ||
| 70 | c, errOut, st, root := importCtx(t, false) | ||
| 71 | code := Dispatch(c, []string{"repo", "import", "alice/x", "--from", "http://127.0.0.1:9/x.git"}) | ||
| 72 | if code != protocol.ExitFailure || !strings.Contains(errOut.String(), "127.0.0.1") { | ||
| 73 | t.Fatalf("exit %d, %q", code, errOut.String()) | ||
| 74 | } | ||
| 75 | if _, err := st.RepoByPath("alice/x"); err == nil { | ||
| 76 | t.Fatal("a refused import created a repository") | ||
| 77 | } | ||
| 78 | if _, err := os.Stat(RepoDir(root, "alice", "x")); !os.IsNotExist(err) { | ||
| 79 | t.Fatalf("a refused import left a directory: %v", err) | ||
| 80 | } | ||
| 81 | } | ||
| 82 | |||
| 83 | // import.test does not resolve; the import works only because git was | ||
| 84 | // pinned to the address import looked up and checked. | ||
| 85 | func TestRepoImportConnectsToTheCheckedAddress(t *testing.T) { | ||
| 86 | remote, sha := importUpstream(t) | ||
| 87 | u, _ := url.Parse(remote) | ||
| 88 | c, errOut, _, root := importCtx(t, true) | ||
| 89 | var asked []string | ||
| 90 | prev := importLookup | ||
| 91 | importLookup = func(ctx context.Context, host string) ([]net.IP, error) { | ||
| 92 | asked = append(asked, host) | ||
| 93 | return []net.IP{net.ParseIP("127.0.0.1")}, nil | ||
| 94 | } | ||
| 95 | t.Cleanup(func() { importLookup = prev }) | ||
| 96 | |||
| 97 | code := Dispatch(c, []string{"repo", "import", "alice/copy", "--from", "http://import.test:" + u.Port() + "/remote.git"}) | ||
| 98 | if code != protocol.ExitOK { | ||
| 99 | t.Fatalf("exit %d: %s", code, errOut.String()) | ||
| 100 | } | ||
| 101 | if out := c.Stdout.(*bytes.Buffer).String(); !strings.Contains(out, "default main") { | ||
| 102 | t.Fatalf("output %q: the default branch was not read through the pin", out) | ||
| 103 | } | ||
| 104 | got := strings.TrimSpace(gitRunner(t)(RepoDir(root, "alice", "copy"), "rev-parse", "refs/heads/main")) | ||
| 105 | if got != sha { | ||
| 106 | t.Fatalf("main = %s, want %s", got, sha) | ||
| 107 | } | ||
| 108 | if !slices.Equal(asked, []string{"import.test"}) { | ||
| 109 | t.Fatalf("looked up %v", asked) | ||
| 110 | } | ||
| 111 | } | ||
internal/gitutil/gitutil.go +13 −9
| @@ -217,14 +217,16 @@ func ReadCommit(dir, sha string) ([]byte, error) { | |||
| 217 | 217 | ||
| 218 | // FetchMirror pulls all branches, tags, and notes from a foreign URL into | 218 | // FetchMirror pulls all branches, tags, and notes from a foreign URL into |
| 219 | // the bare repository at dir, forcing updates. Progress streams to errW so | 219 | // the bare repository at dir, forcing updates. Progress streams to errW so |
| 220 | // an interactive caller can watch. extraEnv carries credentials via | 220 | // an interactive caller can watch. pin is git's leading -c options |
| 221 | // GIT_ASKPASS; the URL itself must never contain them. | 221 | // (gitpin.Remote.Args); env is git's whole environment and carries |
| 222 | func FetchMirror(ctx context.Context, dir, url string, errW io.Writer, extraEnv []string) error { | 222 | // credentials via GIT_ASKPASS: the URL itself must never contain them. |
| 223 | cmd := exec.CommandContext(ctx, toolpath.Look("git"), "-C", dir, "fetch", "--progress", "--no-write-fetch-head", url, | 223 | func FetchMirror(ctx context.Context, dir, url string, errW io.Writer, pin, env []string) error { |
| 224 | args := append(append([]string{}, pin...), "-C", dir, "fetch", "--progress", "--no-write-fetch-head", url, | ||
| 224 | "+refs/heads/*:refs/heads/*", | 225 | "+refs/heads/*:refs/heads/*", |
| 225 | "+refs/tags/*:refs/tags/*", | 226 | "+refs/tags/*:refs/tags/*", |
| 226 | "+refs/notes/*:refs/notes/*") | 227 | "+refs/notes/*:refs/notes/*") |
| 227 | cmd.Env = append(os.Environ(), extraEnv...) | 228 | cmd := exec.CommandContext(ctx, toolpath.Look("git"), args...) |
| 229 | cmd.Env = env | ||
| 228 | cmd.Stderr = errW | 230 | cmd.Stderr = errW |
| 229 | if err := cmd.Run(); err != nil { | 231 | if err := cmd.Run(); err != nil { |
| 230 | return fmt.Errorf("fetch from %s: %w", url, err) | 232 | return fmt.Errorf("fetch from %s: %w", url, err) |
| @@ -253,10 +255,12 @@ func FetchPullHeads(ctx context.Context, dir, url string, errW io.Writer, extraE | |||
| 253 | return nil | 255 | return nil |
| 254 | } | 256 | } |
| 255 | 257 | ||
| 256 | // RemoteDefaultBranch asks the remote which branch HEAD points at. | 258 | // RemoteDefaultBranch asks the remote which branch HEAD points at. pin |
| 257 | func RemoteDefaultBranch(ctx context.Context, url string, extraEnv []string) (string, error) { | 259 | // and env are as for FetchMirror. |
| 258 | cmd := exec.CommandContext(ctx, toolpath.Look("git"), "ls-remote", "--symref", url, "HEAD") | 260 | func RemoteDefaultBranch(ctx context.Context, url string, pin, env []string) (string, error) { |
| 259 | cmd.Env = append(os.Environ(), extraEnv...) | 261 | args := append(append([]string{}, pin...), "ls-remote", "--symref", url, "HEAD") |
| 262 | cmd := exec.CommandContext(ctx, toolpath.Look("git"), args...) | ||
| 263 | cmd.Env = env | ||
| 260 | out, err := cmd.Output() | 264 | out, err := cmd.Output() |
| 261 | if err != nil { | 265 | if err != nil { |
| 262 | return "", fmt.Errorf("ls-remote %s: %w", url, err) | 266 | return "", fmt.Errorf("ls-remote %s: %w", url, err) |