From 2d4d34ddfc879b05dab75866eb2c857c8454b989 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 22:27:46 +0200 Subject: [PATCH] feat: accept repository shorthands and name instances in account errors Repository arguments and --repo accept NAME (owned by the active account, or the --sudo user), OWNER/NAME.git and HOST/OWNER/NAME next to URLs; --host, TEA_HOST and GITEA_INSTANCE_URL accept a bare HOST[:PORT][/PATH], which takes the scheme of the configured instance it names, else https. --repo origin in a clone with that remote suggests --remote origin. Account errors name the instance, the accounts that are logged in and where the instance came from, such as "no account for https://gitea.com (instance from git remote \"origin\")", instead of "no selected account". Signed-off-by: bircni --- internal/app/target.go | 362 +++++++++++++++++++++---------- internal/app/target_test.go | 70 ++++++ internal/settings/remote.go | 108 ++++++++- internal/settings/remote_test.go | 55 +++++ 4 files changed, 470 insertions(+), 125 deletions(-) diff --git a/internal/app/target.go b/internal/app/target.go index bb061b54..e6ae7098 100644 --- a/internal/app/target.go +++ b/internal/app/target.go @@ -9,7 +9,7 @@ import ( "errors" "fmt" "maps" - "sort" + "slices" "strconv" "strings" @@ -56,7 +56,7 @@ func (a *application) environmentHosts(hosts map[string]settings.Host) (map[stri if a.deps.Getenv("GITEA_TOKEN") == "" || (insecure == "" && sshHost == "") { return hosts, nil } - instance, err := settings.NormalizeHost(a.tokenInstance()) + instance, err := settings.ResolveHost(a.tokenInstance(), hosts) if err != nil { return hosts, nil // The token's instance check reports this. } @@ -80,7 +80,36 @@ func (a *application) environmentHosts(hosts map[string]settings.Host) (map[stri return result, nil } +// targetRequest says what a command needs resolved. +type targetRequest struct { + requireRepo, authenticate bool + // repo names the repository a command argument selected, such as the repository of an + // issue URL; it takes precedence over discovery and must agree with --repo. + repo, repoSource string +} + func (a *application) resolveTarget(ctx context.Context, requireRepo, authenticate bool) (target, error) { + return a.resolveRequest(ctx, targetRequest{requireRepo: requireRepo, authenticate: authenticate}) +} + +func (a *application) resolveRequest(ctx context.Context, req targetRequest) (target, error) { + t, err := a.connect(ctx, req) + if err != nil { + return t, err + } + // A bare repository NAME belongs to the account tea acts as, which is known only now. + if t.Repo != "" && !strings.Contains(t.Repo, "/") { + owner := cmp.Or(a.options.sudo, t.User) + if owner == "" { + return t, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot tell who owns repository %q; use OWNER/%s", t.Repo, t.Repo)) + } + t.Repo = owner + "/" + t.Repo + } + return t, nil +} + +// connect selects the instance, account and repository and builds the authenticated client. +func (a *application) connect(ctx context.Context, req targetRequest) (target, error) { s, err := a.deps.Settings.Load() if err != nil { return target{}, err @@ -89,142 +118,38 @@ func (a *application) resolveTarget(ctx context.Context, requireRepo, authentica return target{}, err } t := target{} - rawRepo := a.options.repo - if rawRepo != "" { - t.RepoSource = "--repo" - } else if rawRepo = a.deps.Getenv("TEA_REPO"); rawRepo != "" { - t.RepoSource = "TEA_REPO" - } - if rawRepo == "" && requireRepo { - // Discovery probes fail outside a checkout; Git's diagnostics would only precede tea's own error. - probe := func(args ...string) (string, error) { return a.gitProbe().Output(ctx, a.options.cwd, false, args...) } - if saved, saveErr := probe("config", "--get", "tea.repository"); saveErr == nil && saved != "" { - rawRepo = saved - t.RepoSource = "git config tea.repository" - } else { - remote := a.options.remote - if remote == "" { - remote = s.Preferences.Remote - } - remote, err = git.DiscoverRemote(ctx, a.gitProbe(), a.options.cwd, remote) - if errors.Is(err, git.ErrAmbiguousRemote) { - return t, clierr.Wrap(clierr.Usage, err) - } - if err != nil { - return t, clierr.Wrap(clierr.Usage, errRepositoryRequired) - } - rawRepo, err = probe("remote", "get-url", remote) - if err != nil { - return t, fmt.Errorf("cannot read the URL of Git remote %q: %w", remote, err) - } - t.RepoSource = "remote " + remote - } + rawRepo, err := a.repositorySource(ctx, &t, s, req) + if err != nil { + return t, err } if rawRepo != "" { - if settings.ValidSlug(rawRepo) { - t.Repo = rawRepo - } else { - configured := s.Hosts.Hosts - if a.options.host != "" { - explicit, normalizeErr := settings.NormalizeHost(a.options.host) - if normalizeErr != nil { - return t, clierr.Wrap(clierr.Usage, normalizeErr) - } - if host, exists := configured[explicit]; exists { - configured = map[string]settings.Host{explicit: host} - } - } - t.Host, t.Repo, err = settings.RepositoryURL(rawRepo, configured) - if err != nil { - return t, clierr.Wrap(clierr.Usage, err) - } - t.HostSource = t.RepoSource + if err := a.selectRepository(ctx, &t, s, rawRepo); err != nil { + return t, err } } - if a.options.host != "" { - explicit, err := settings.NormalizeHost(a.options.host) - if err != nil { - return t, clierr.Wrap(clierr.Usage, err) - } - if t.Host != "" && t.Host != explicit { - return t, clierr.Wrap(clierr.Usage, errors.New("--host conflicts with repository instance")) - } - t.Host, t.HostSource = explicit, "--host" - } - if t.Host == "" { - for _, key := range []string{"TEA_HOST", "GITEA_INSTANCE_URL"} { - if value := a.deps.Getenv(key); value != "" { - t.Host, err = settings.NormalizeHost(value) - if err != nil { - return t, clierr.Wrap(clierr.Usage, err) - } - t.HostSource = key - break - } - } - } - if t.Host == "" && s.Preferences.DefaultHost != "" { - t.Host, t.HostSource = s.Preferences.DefaultHost, "config.yml default_host" - } - if t.Host == "" && len(s.Hosts.Hosts) == 1 { - for host := range s.Hosts.Hosts { - t.Host, t.HostSource = host, "sole configured instance" - } - } - if t.Host == "" { - return t, clierr.Wrap(clierr.Usage, errors.New("instance required: select --host or set default_host")) + if err := a.selectHost(&t, s); err != nil { + return t, err } host := s.Hosts.Hosts[t.Host] t.host = host t.gitProtocol = cmp.Or(host.GitProtocol, s.Preferences.GitProtocol) - t.User, t.UserSource = a.selectedUser() - if matched, ok := matchAccount(host.Users, t.User); ok { - t.User = matched + if err := a.selectAccount(ctx, &t, s, req.authenticate); err != nil { + return t, err } - if t.User == "" { - t.User, t.UserSource = host.User, "active instance account" - } - if t.User == "" { - switch len(host.Users) { - case 1: - for user := range host.Users { - t.User, t.UserSource = user, "sole configured account" - } - case 0: - default: - if authenticate && a.deps.Getenv("GITEA_TOKEN") == "" && a.policy(s).CanPrompt(a.deps.Streams) && a.deps.Prompter != nil { - names := make([]string, 0, len(host.Users)) - for user := range host.Users { - names = append(names, user) - } - sort.Strings(names) - selected, err := a.deps.Prompter.Input(ctx, "account ("+strings.Join(names, ", ")+")") - if err != nil { - return t, err - } - selected = strings.TrimSpace(selected) - matched, ok := matchAccount(host.Users, selected) - if !ok { - return t, clierr.Wrap(clierr.Usage, fmt.Errorf("account %q is not configured on %s", selected, t.Host)) - } - t.User, t.UserSource = matched, "interactive account selection" - } - } - } - if !authenticate { + if !req.authenticate { return t, nil } token := a.deps.Getenv("GITEA_TOKEN") environmentCredential := token != "" if token != "" { - normalized, err := settings.NormalizeHost(a.tokenInstance()) + normalized, err := settings.ResolveHost(a.tokenInstance(), s.Hosts.Hosts) if err != nil || normalized != t.Host { return t, clierr.Wrap(clierr.Authentication, errors.New("GITEA_TOKEN requires GITEA_INSTANCE_URL or TEA_HOST matching the selected instance")) } } else { account, exists := host.Users[t.User] if !exists { - return t, clierr.Wrap(clierr.Authentication, fmt.Errorf("no selected account on %s; run tea auth login or tea auth switch", t.Host)) + return t, clierr.Wrap(clierr.Authentication, t.accountError(t.User)) } if account.Tokenless() { signer, err := auth.SignerForAccount(account, a.sshPassphrase(ctx, a.policy(s), account.SSHKey)) @@ -272,6 +197,207 @@ func (a *application) resolveTarget(ctx context.Context, requireRepo, authentica return t, nil } +// repositorySource returns the raw repository the command targets and records its source: an +// argument's repository, --repo, TEA_REPO, the saved tea.repository, or a discovered Git remote. +func (a *application) repositorySource(ctx context.Context, t *target, s settings.Snapshot, req targetRequest) (string, error) { + if req.repo != "" { + if err := a.agreesWithRepoFlag(req, s.Hosts.Hosts); err != nil { + return "", err + } + t.RepoSource = req.repoSource + return req.repo, nil + } + if a.options.repo != "" { + t.RepoSource = "--repo" + return a.options.repo, nil + } + if raw := a.deps.Getenv("TEA_REPO"); raw != "" { + t.RepoSource = "TEA_REPO" + return raw, nil + } + if !req.requireRepo { + return "", nil + } + // Discovery probes fail outside a checkout; Git's diagnostics would only precede tea's own error. + probe := func(args ...string) (string, error) { return a.gitProbe().Output(ctx, a.options.cwd, false, args...) } + if saved, err := probe("config", "--get", "tea.repository"); err == nil && saved != "" { + t.RepoSource = "git config tea.repository" + return saved, nil + } + remote := cmp.Or(a.options.remote, s.Preferences.Remote) + remote, err := git.DiscoverRemote(ctx, a.gitProbe(), a.options.cwd, remote) + if errors.Is(err, git.ErrAmbiguousRemote) { + return "", clierr.Wrap(clierr.Usage, err) + } + if err != nil { + return "", clierr.Wrap(clierr.Usage, errRepositoryRequired) + } + raw, err := probe("remote", "get-url", remote) + if err != nil { + return "", fmt.Errorf("cannot read the URL of Git remote %q: %w", remote, err) + } + t.RepoSource = "remote " + remote + return raw, nil +} + +// agreesWithRepoFlag rejects an argument naming another repository than --repo. +func (a *application) agreesWithRepoFlag(req targetRequest, hosts map[string]settings.Host) error { + if a.options.repo == "" { + return nil + } + selected, selectedErr := settings.ParseRepository(req.repo, hosts) + explicit, explicitErr := settings.ParseRepository(a.options.repo, hosts) + if selectedErr != nil || explicitErr != nil { + return nil // Resolution reports the invalid one. + } + sameHost := selected.Host == "" || explicit.Host == "" || selected.Host == explicit.Host + sameRepo := strings.EqualFold(selected.Name, explicit.Name) && (explicit.Owner == "" || strings.EqualFold(selected.Owner, explicit.Owner)) + if sameHost && sameRepo { + return nil + } + return clierr.Wrap(clierr.Usage, fmt.Errorf("%s points to %s but --repo is %s", req.repoSource, selected.Slug(), a.options.repo)) +} + +// selectRepository parses raw into the target's repository and, for URLs and HOST/OWNER/REPO, +// its instance. With --host, URLs match only that instance. +func (a *application) selectRepository(ctx context.Context, t *target, s settings.Snapshot, raw string) error { + configured := s.Hosts.Hosts + if a.options.host != "" { + explicit, err := settings.ResolveHost(a.options.host, configured) + if err != nil { + return clierr.Wrap(clierr.Usage, err) + } + if host, exists := configured[explicit]; exists { + configured = map[string]settings.Host{explicit: host} + } + } + ref, err := settings.ParseRepository(raw, configured) + if err != nil { + return clierr.Wrap(clierr.Usage, err) + } + if ref.Owner == "" && ref.Host == "" && a.isGitRemote(ctx, ref.Name) { + return clierr.Wrap(clierr.Usage, fmt.Errorf("invalid repository %q: use OWNER/REPO, HOST/OWNER/REPO or a URL; did you mean --remote %s?", raw, ref.Name)) + } + t.Repo = ref.Slug() + if ref.Host != "" { + t.Host, t.HostSource = ref.Host, t.RepoSource + } + return nil +} + +// isGitRemote reports whether name is a remote of the working directory's Git checkout. +func (a *application) isGitRemote(ctx context.Context, name string) bool { + remotes, err := a.gitProbe().Output(ctx, a.options.cwd, false, "remote") + return err == nil && slices.Contains(strings.Fields(remotes), name) +} + +// selectHost picks the instance: the repository's, --host, TEA_HOST or GITEA_INSTANCE_URL, +// default_host, or the only configured instance. +func (a *application) selectHost(t *target, s settings.Snapshot) error { + if a.options.host != "" { + explicit, err := settings.ResolveHost(a.options.host, s.Hosts.Hosts) + if err != nil { + return clierr.Wrap(clierr.Usage, err) + } + if t.Host != "" && t.Host != explicit { + return clierr.Wrap(clierr.Usage, fmt.Errorf("--host %s conflicts with %s from %s", explicit, t.Host, t.HostSource)) + } + t.Host, t.HostSource = explicit, "--host" + } + if t.Host == "" { + for _, key := range []string{"TEA_HOST", "GITEA_INSTANCE_URL"} { + if value := a.deps.Getenv(key); value != "" { + host, err := settings.ResolveHost(value, s.Hosts.Hosts) + if err != nil { + return clierr.Wrap(clierr.Usage, fmt.Errorf("invalid %s: %w", key, err)) + } + t.Host, t.HostSource = host, key + break + } + } + } + if t.Host == "" && s.Preferences.DefaultHost != "" { + t.Host, t.HostSource = s.Preferences.DefaultHost, "config.yml default_host" + } + if t.Host == "" && len(s.Hosts.Hosts) == 1 { + for host := range s.Hosts.Hosts { + t.Host, t.HostSource = host, "sole configured instance" + } + } + if t.Host == "" { + if len(s.Hosts.Hosts) == 0 { + return clierr.Wrap(clierr.Usage, errors.New("no instance selected: log in with tea auth login, or pass --host")) + } + return clierr.Wrap(clierr.Usage, fmt.Errorf("instance required: select --host or set default_host (instances: %s)", strings.Join(slices.Sorted(maps.Keys(s.Hosts.Hosts)), ", "))) + } + return nil +} + +// selectAccount picks the account: --user or TEA_USER, the instance's active account, its only +// account, or an interactive choice. +func (a *application) selectAccount(ctx context.Context, t *target, s settings.Snapshot, authenticate bool) error { + users := t.host.Users + t.User, t.UserSource = a.selectedUser() + if matched, ok := matchAccount(users, t.User); ok { + t.User = matched + } + if t.User == "" { + t.User, t.UserSource = t.host.User, "active instance account" + } + if t.User != "" || len(users) == 0 { + return nil + } + if len(users) == 1 { + for user := range users { + t.User, t.UserSource = user, "sole configured account" + } + return nil + } + if !authenticate || a.deps.Getenv("GITEA_TOKEN") != "" || !a.policy(s).CanPrompt(a.deps.Streams) || a.deps.Prompter == nil { + return nil + } + selected, err := a.deps.Prompter.Input(ctx, "account ("+accountNames(users)+")") + if err != nil { + return err + } + matched, ok := matchAccount(users, strings.TrimSpace(selected)) + if !ok { + return clierr.Wrap(clierr.Usage, t.accountError(strings.TrimSpace(selected))) + } + t.User, t.UserSource = matched, "interactive account selection" + return nil +} + +// accountError explains why user has no stored credential on the target's instance. +func (t target) accountError(user string) error { + users := t.host.Users + switch { + case user != "" && len(users) > 0: + return fmt.Errorf("account %q is not logged in on %s (accounts: %s)", user, t.Host, accountNames(users)) + case user != "": + return fmt.Errorf("account %q is not logged in on %s; run tea auth login --host %s", user, t.Host, t.Host) + case len(users) > 0: + return fmt.Errorf("no active account on %s (accounts: %s); pass --user or run tea auth switch", t.Host, accountNames(users)) + } + text := "no account for " + t.Host + if t.HostSource != "--host" { + text += " (instance from " + describeSource(t.HostSource) + ")" + } + return errors.New(text + "; run tea auth login --host " + t.Host + ", or use -R or --host to target another instance") +} + +// describeSource words a target source for messages, such as git remote "origin". +func describeSource(source string) string { + if remote, ok := strings.CutPrefix(source, "remote "); ok { + return "git remote " + strconv.Quote(remote) + } + return source +} + +func accountNames(users map[string]settings.Account) string { + return strings.Join(slices.Sorted(maps.Keys(users)), ", ") +} + // selectedUser returns the account named by --user or TEA_USER, and which one named it. func (a *application) selectedUser() (string, string) { if a.options.user != "" { diff --git a/internal/app/target_test.go b/internal/app/target_test.go index d2f3d292..3b4efc3f 100644 --- a/internal/app/target_test.go +++ b/internal/app/target_test.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "gitea.dev/tea/internal/clierr" @@ -232,6 +233,75 @@ func TestRepositoryResolutionPrecedence(t *testing.T) { require.False(t, runner.prompt) } +func TestRepositoryArgumentsAcceptShorthands(t *testing.T) { + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"full_name":"` + strings.TrimPrefix(r.URL.Path, "/api/v1/repos/") + `"}`)) + })) + defer server.Close() + bare := strings.TrimPrefix(server.URL, "http://") + for _, tc := range []struct { + name string + args []string + want string + }{ + {"bare name belongs to the active account", []string{"repo", "view", "project"}, "alice/project"}, + {"bare name belongs to the sudo user", []string{"repo", "view", "project", "--sudo", "bob"}, "bob/project"}, + {".git suffix", []string{"repo", "view", "bob/tool.git"}, "bob/tool"}, + {"HOST/OWNER/REPO", []string{"repo", "view", bare + "/bob/tool"}, "bob/tool"}, + {"--repo HOST/OWNER/REPO", []string{"repo", "view", "-R", bare + "/bob/tool.git"}, "bob/tool"}, + {"bare --host", []string{"repo", "view", "bob/tool", "--host", bare}, "bob/tool"}, + } { + t.Run(tc.name, func(t *testing.T) { + deps, out, _ := testDeps(t) + configureAccounts(t, deps, server.URL, "alice", "alice", "bob") + require.NoError(t, Execute(context.Background(), deps, append(tc.args, "--no-input", "--json=full_name"))) + require.JSONEq(t, `{"full_name":"`+tc.want+`"}`, out.String()) + }) + } +} + +func TestRepositoryNamedLikeAGitRemoteSuggestsRemote(t *testing.T) { + deps, _, _ := testDeps(t) + deps.Getenv = serverEnv("https://example.org", nil) + deps.Git = &fakeGit{results: map[string]string{"remote": "origin\nupstream"}} + err := Execute(context.Background(), deps, []string{"issue", "list", "-R", "origin"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err)) + require.EqualError(t, err, `invalid repository "origin": use OWNER/REPO, HOST/OWNER/REPO or a URL; did you mean --remote origin?`) + err = Execute(context.Background(), deps, []string{"issue", "list", "-R", "a/b/c/d"}) + require.EqualError(t, err, `invalid repository "a/b/c/d": use OWNER/REPO, HOST/OWNER/REPO or a URL`) +} + +func TestAccountErrorsNameTheInstanceAndAccounts(t *testing.T) { + const instance = "https://example.org" + remote := map[string]string{"remote": "origin", "remote get-url origin": "https://gitea.com/alice/project.git"} + for _, tc := range []struct { + name string + users []string + active string + args []string + git map[string]string + want string + }{ + {"unknown --user", []string{"alice", "bob"}, "alice", []string{"-R", "alice/project", "--user", "nobody"}, nil, `account "nobody" is not logged in on https://example.org (accounts: alice, bob)`}, + {"unknown --user without accounts", nil, "", []string{"-R", "alice/project", "--user", "nobody", "--host", instance}, nil, `account "nobody" is not logged in on https://example.org; run tea auth login --host https://example.org`}, + {"no active account", []string{"alice", "bob"}, "", []string{"-R", "alice/project"}, nil, "no active account on https://example.org (accounts: alice, bob); pass --user or run tea auth switch"}, + {"instance from a remote", []string{"alice"}, "alice", nil, remote, `no account for https://gitea.com (instance from git remote "origin"); run tea auth login --host https://gitea.com, or use -R or --host to target another instance`}, + } { + t.Run(tc.name, func(t *testing.T) { + deps, _, _ := testDeps(t) + if tc.users != nil { + configureAccounts(t, deps, instance, tc.active, tc.users...) + } + if tc.git != nil { + deps.Git = &fakeGit{results: tc.git} + } + err := Execute(context.Background(), deps, append([]string{"issue", "list", "--no-input"}, tc.args...)) + require.Equal(t, clierr.Authentication, clierr.ExitCode(err), "%v", err) + require.EqualError(t, err, tc.want) + }) + } +} + func TestRepositoryDiscoveryOutsideCheckoutWritesNoGitDiagnostics(t *testing.T) { if _, err := exec.LookPath("git"); err != nil { t.Skip("Git is not installed") diff --git a/internal/settings/remote.go b/internal/settings/remote.go index a982d4e2..3c02a2d8 100644 --- a/internal/settings/remote.go +++ b/internal/settings/remote.go @@ -5,6 +5,7 @@ package settings import ( "errors" + "fmt" "net/url" "strings" ) @@ -126,14 +127,107 @@ func looksLikeWebResourcePath(parts []string) bool { // ValidSlug reports whether slug is exactly owner/name. func ValidSlug(slug string) bool { pieces := strings.Split(slug, "/") - if len(pieces) != 2 { - return false + return len(pieces) == 2 && validPiece(pieces[0]) && validPiece(pieces[1]) +} + +// validPiece reports whether piece can be an owner or repository name. +func validPiece(piece string) bool { + // ':' keeps scp-style remotes such as git@host:owner/name.git from passing as a slug. + return piece != "" && piece != "." && piece != ".." && !strings.ContainsAny(piece, ":?#\\ \t\r\n") +} + +// RepositoryRef is a parsed repository argument. +type RepositoryRef struct { + // Host is the instance URL the argument names, or "" when it names none. + Host string + // Owner is "" for a bare NAME, which belongs to the active account. + Owner, Name string +} + +// Slug returns owner/name, or the bare name when the owner is not known yet. +func (r RepositoryRef) Slug() string { + if r.Owner == "" { + return r.Name } - for _, piece := range pieces { - // ':' keeps scp-style remotes such as git@host:owner/name.git from passing as a slug. - if piece == "" || piece == "." || piece == ".." || strings.ContainsAny(piece, ":?#\\ \t\r\n") { - return false + return r.Owner + "/" + r.Name +} + +// ParseRepository reads a repository argument: NAME, OWNER/NAME, HOST/OWNER/NAME (with an +// optional .git suffix), or a clone or web URL. HOST may carry a port and a configured +// instance's base path; it takes the scheme of the configured instance it names, else https. +func ParseRepository(raw string, hosts map[string]Host) (RepositoryRef, error) { + raw = strings.TrimSpace(raw) + if strings.Contains(raw, "://") || scpLike(raw) { + host, slug, err := RepositoryURL(raw, hosts) + if err != nil { + return RepositoryRef{}, err + } + owner, name, _ := strings.Cut(slug, "/") + return RepositoryRef{Host: host, Owner: owner, Name: name}, nil + } + invalid := fmt.Errorf("invalid repository %q: use OWNER/REPO, HOST/OWNER/REPO or a URL", raw) + trimmed := strings.TrimSuffix(raw, ".git") + pieces := strings.Split(trimmed, "/") + switch { + case len(pieces) == 1 && validPiece(pieces[0]): + return RepositoryRef{Name: pieces[0]}, nil + case len(pieces) == 2 && ValidSlug(trimmed): + return RepositoryRef{Owner: pieces[0], Name: pieces[1]}, nil + case len(pieces) < 3: + return RepositoryRef{}, invalid + } + // The longest configured instance prefix wins, so example.org/gitea/owner/name selects the + // instance installed below /gitea rather than example.org. + best := "" + for instance := range hosts { + prefix := bareInstance(instance) + if len(prefix) > len(bareInstance(best)) && strings.HasPrefix(strings.ToLower(trimmed), strings.ToLower(prefix)+"/") && ValidSlug(trimmed[len(prefix)+1:]) { + best = instance } } - return true + if best != "" { + owner, name, _ := strings.Cut(trimmed[len(bareInstance(best))+1:], "/") + return RepositoryRef{Host: best, Owner: owner, Name: name}, nil + } + if len(pieces) != 3 || !ValidSlug(pieces[1]+"/"+pieces[2]) { + return RepositoryRef{}, invalid + } + host, err := ResolveHost(pieces[0], hosts) + if err != nil { + return RepositoryRef{}, invalid + } + return RepositoryRef{Host: host, Owner: pieces[1], Name: pieces[2]}, nil +} + +// ResolveHost normalizes an instance given as a URL or as a bare HOST[:PORT][/PATH]. A bare +// host takes the scheme of the configured instance it names, else https. +func ResolveHost(raw string, hosts map[string]Host) (string, error) { + raw = strings.TrimSpace(raw) + if strings.Contains(raw, "://") { + return NormalizeHost(raw) + } + bare := strings.TrimSuffix(raw, "/") + var matches []string + for instance := range hosts { + if strings.EqualFold(bareInstance(instance), bare) { + matches = append(matches, instance) + } + } + switch len(matches) { + case 0: + instance, err := NormalizeHost("https://" + raw) + if err != nil { + return "", fmt.Errorf("invalid instance %q: use HOST[:PORT][/PATH] or an http(s) URL", raw) + } + return instance, nil + case 1: + return matches[0], nil + } + return "", fmt.Errorf("instance %q is configured for both http and https; include the scheme", raw) +} + +// bareInstance returns an instance URL without its scheme, such as localhost:3001/gitea. +func bareInstance(instance string) string { + _, bare, _ := strings.Cut(instance, "://") + return bare } diff --git a/internal/settings/remote_test.go b/internal/settings/remote_test.go index 1bf546b5..56b7b826 100644 --- a/internal/settings/remote_test.go +++ b/internal/settings/remote_test.go @@ -68,6 +68,61 @@ func TestParseRemoteReadsSCPStyleRemotes(t *testing.T) { require.Equal(t, "ssh://git@git.example.org/alice/project.git", u.String()) } +func TestParseRepositoryAcceptsShorthands(t *testing.T) { + hosts := map[string]Host{"http://localhost:3001": {}, "https://example.org/gitea": {}, "https://example.org": {}} + for _, tc := range []struct { + raw string + want RepositoryRef + err string + }{ + {raw: "project", want: RepositoryRef{Name: "project"}}, + {raw: "alice/project", want: RepositoryRef{Owner: "alice", Name: "project"}}, + {raw: "alice/project.git", want: RepositoryRef{Owner: "alice", Name: "project"}}, + {raw: " alice/project ", want: RepositoryRef{Owner: "alice", Name: "project"}}, + {raw: "localhost:3001/alice/project", want: RepositoryRef{Host: "http://localhost:3001", Owner: "alice", Name: "project"}}, + {raw: "LOCALHOST:3001/alice/project.git", want: RepositoryRef{Host: "http://localhost:3001", Owner: "alice", Name: "project"}}, + {raw: "example.org/gitea/alice/project", want: RepositoryRef{Host: "https://example.org/gitea", Owner: "alice", Name: "project"}}, + {raw: "example.org/alice/project", want: RepositoryRef{Host: "https://example.org", Owner: "alice", Name: "project"}}, + {raw: "gitea.com/gitea/tea", want: RepositoryRef{Host: "https://gitea.com", Owner: "gitea", Name: "tea"}}, + {raw: "http://localhost:3001/alice/project.git", want: RepositoryRef{Host: "http://localhost:3001", Owner: "alice", Name: "project"}}, + {raw: "a/b/c/d", err: `invalid repository "a/b/c/d": use OWNER/REPO, HOST/OWNER/REPO or a URL`}, + {raw: "alice/", err: "invalid repository"}, + {raw: "../x", err: "invalid repository"}, + {raw: "", err: "invalid repository"}, + } { + got, err := ParseRepository(tc.raw, hosts) + if tc.err != "" { + require.ErrorContains(t, err, tc.err, tc.raw) + continue + } + require.NoError(t, err, tc.raw) + require.Equal(t, tc.want, got, tc.raw) + } + require.Equal(t, "project", RepositoryRef{Name: "project"}.Slug()) + require.Equal(t, "alice/project", RepositoryRef{Owner: "alice", Name: "project"}.Slug()) +} + +func TestResolveHostAcceptsBareHosts(t *testing.T) { + hosts := map[string]Host{"http://localhost:3001": {}, "https://example.org/gitea": {}} + for raw, want := range map[string]string{ + "localhost:3001": "http://localhost:3001", + "localhost:3001/": "http://localhost:3001", + "example.org/gitea": "https://example.org/gitea", + "gitea.com": "https://gitea.com", + "https://localhost:3001": "https://localhost:3001", + "https://gitea.com:443/x/": "https://gitea.com/x", + "http://example.org/gitea/": "http://example.org/gitea", + } { + got, err := ResolveHost(raw, hosts) + require.NoError(t, err, raw) + require.Equal(t, want, got, raw) + } + _, err := ResolveHost("localhost:3001", map[string]Host{"http://localhost:3001": {}, "https://localhost:3001": {}}) + require.ErrorContains(t, err, "include the scheme") + _, err = ResolveHost("bad host", hosts) + require.ErrorContains(t, err, `invalid instance "bad host"`) +} + func TestValidSlugRejectsSCPStyleRemotes(t *testing.T) { require.True(t, ValidSlug("alice/project")) require.False(t, ValidSlug("git@example.org:alice/project.git"))