From 97c2cbac55aa022c769880f1774c5096b59af166 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Fri, 6 Nov 2026 22:32:25 +0530 Subject: [PATCH 1/7] Add a factory package that builds a Client from any repo URL ParseRepoURL, Client.RegisterDomain and ForgeFor cover URL parsing and detection, but the default domains and the fallback to detection lived in internal/resolve, so library consumers had to rebuild them. Add a public factory package that imports the providers. factory.NewClient returns a Client that knows github.com, gitlab.com, codeberg.org, bitbucket.org and tangled.org, and that detects other domains on first use. factory.DefaultForges and factory.Builders expose the same wiring, and internal/resolve now uses them rather than keeping its own copy. The root package cannot import providers, so first-use detection is an opt-in Client option. WithDetection takes the builders to use, and the new ForgeForURL returns the forge, owner and repo for a URL. FetchRepository, FetchTags, ResolveCommit and ListRepositories go through the same lookup. Clients without the option behave as before and never probe. A plain http:// URL detects over http so self-hosted instances on plain HTTP work. Every other form, including scp-like and ssh remotes, uses https. Detection makes lookups write to the registry, so a mutex now guards the forge and token maps. Tokens returns a copy, because handing out the live map would race with registration. Fixes #18 --- README.md | 11 ++ commits.go | 6 +- factory/factory.go | 58 +++++++++++ factory/factory_test.go | 194 ++++++++++++++++++++++++++++++++++++ forge.go | 74 +++++++++++--- forges_test.go | 127 +++++++++++++++++++++++ internal/resolve/resolve.go | 22 +--- 7 files changed, 456 insertions(+), 36 deletions(-) create mode 100644 factory/factory.go create mode 100644 factory/factory_test.go diff --git a/README.md b/README.md index 668039d..3c4f752 100644 --- a/README.md +++ b/README.md @@ -177,6 +177,17 @@ issues, _ := f.Issues().List(ctx, "octocat", "hello-world", forges.ListIssueOpts pr, _ := f.PullRequests().Get(ctx, "octocat", "hello-world", 42) ``` +The `factory` package imports every provider and returns a `Client` that already knows github.com, gitlab.com, codeberg.org, bitbucket.org and tangled.org. It detects and registers any other domain the first time a repository URL on it is used, which sends requests to that domain: + +```go +import "github.com/git-pkgs/forge/factory" + +client := factory.NewClient(forges.WithToken("git.example.com", token)) + +f, owner, repo, err := client.ForgeForURL(ctx, "https://git.example.com/octocat/hello-world") +issues, err := f.Issues().List(ctx, owner, repo, forges.ListIssueOpts{State: "open"}) +``` + Incoming webhook deliveries from GitHub, GitLab and Gitea/Forgejo can be verified and parsed into a normalized `WebhookEvent`. Event types use the same names as `Webhook.Events`. Push, pull request, issue, comment and release deliveries carry typed fields. The raw JSON is always available in `Payload`. An empty secret skips verification, so read the secret once at startup and refuse to run without it: diff --git a/commits.go b/commits.go index df4dcf8..3749fd8 100644 --- a/commits.go +++ b/commits.go @@ -75,11 +75,7 @@ func ResolvedCommitSHA(owner, repo, ref, sha string) (string, error) { // ResolveCommit resolves ref to a full commit SHA for the repository at // repoURL, routing to the forge registered for that URL's domain. func (c *Client) ResolveCommit(ctx context.Context, repoURL, ref string) (string, error) { - domain, owner, repo, err := ParseRepoURL(repoURL) - if err != nil { - return "", err - } - f, err := c.forgeFor(domain) + f, owner, repo, err := c.ForgeForURL(ctx, repoURL) if err != nil { return "", err } diff --git a/factory/factory.go b/factory/factory.go new file mode 100644 index 0000000..f71dfa0 --- /dev/null +++ b/factory/factory.go @@ -0,0 +1,58 @@ +// Package factory builds forges.Client values wired up with every provider, so +// callers can pass any repository URL without choosing a backend themselves. +package factory + +import ( + "net/http" + + "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge/bitbucket" + "github.com/git-pkgs/forge/gerrit" + "github.com/git-pkgs/forge/gitea" + "github.com/git-pkgs/forge/github" + "github.com/git-pkgs/forge/gitlab" + "github.com/git-pkgs/forge/radicle" + "github.com/git-pkgs/forge/tangled" +) + +// Builders returns the constructors for every provider that can be detected on +// a self-hosted domain. +func Builders() forges.ForgeBuilders { + return forges.ForgeBuilders{ + GitHub: github.NewWithBase, + GitLab: gitlab.New, + Gitea: gitea.New, + Gerrit: gerrit.New, + Tangled: tangled.New, + Radicle: radicle.New, + } +} + +// DefaultForges returns a Forge for each well-known public domain, keyed by +// domain. The token function supplies the token for each domain and may return +// an empty string for anonymous access. +func DefaultForges(token func(domain string) string, hc *http.Client) map[string]forges.Forge { + return map[string]forges.Forge{ + "github.com": github.New(token("github.com"), hc), + "gitlab.com": gitlab.New("https://gitlab.com", token("gitlab.com"), hc), + "codeberg.org": gitea.New("https://codeberg.org", token("codeberg.org"), hc), + "bitbucket.org": bitbucket.New(token("bitbucket.org"), hc), + "tangled.org": tangled.New("https://tangled.org", token("tangled.org"), hc), + } +} + +// NewClient returns a Client with the default forges registered and detection +// enabled for every other domain. Options apply first, so WithToken and +// WithHTTPClient reach the default forges, and a domain registered with +// WithForge keeps that registration. +func NewClient(opts ...forges.Option) *forges.Client { + c := forges.NewClient(append([]forges.Option{forges.WithDetection(Builders())}, opts...)...) + tokens := c.Tokens() + defaults := DefaultForges(func(domain string) string { return tokens[domain] }, c.HTTPClient()) + for domain, f := range defaults { + if _, err := c.ForgeFor(domain); err != nil { + forges.WithForge(domain, f)(c) + } + } + return c +} diff --git a/factory/factory_test.go b/factory/factory_test.go new file mode 100644 index 0000000..72e7fd3 --- /dev/null +++ b/factory/factory_test.go @@ -0,0 +1,194 @@ +package factory + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/git-pkgs/forge" +) + +type stubForge struct { + forges.Forge +} + +// recordingTransport answers every request with a 404 and records the +// Authorization header sent to each host. +type recordingTransport struct { + mu sync.Mutex + auth map[string]string +} + +func (rt *recordingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + rt.mu.Lock() + defer rt.mu.Unlock() + if rt.auth == nil { + rt.auth = make(map[string]string) + } + rt.auth[req.URL.Host] = req.Header.Get("Authorization") + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: make(http.Header), + Body: http.NoBody, + Request: req, + }, nil +} + +func TestNewClientRegistersDefaultForges(t *testing.T) { + c := NewClient() + for domain, wantAPI := range map[string]string{ + "github.com": "https://api.github.com", + "gitlab.com": "https://gitlab.com/api/v4", + "codeberg.org": "https://codeberg.org/api/v1", + "bitbucket.org": "https://api.bitbucket.org/2.0", + "tangled.org": "https://tangled.org/xrpc", + } { + f, err := c.ForgeFor(domain) + if err != nil { + t.Errorf("ForgeFor(%q): %v", domain, err) + continue + } + provider, ok := f.(forges.APIBaseURLProvider) + if !ok { + t.Errorf("forge for %q does not expose its API URL", domain) + continue + } + if got := provider.APIBaseURL(); got != wantAPI { + t.Errorf("API URL for %q = %q, want %q", domain, got, wantAPI) + } + } +} + +func TestNewClientAppliesOptionsToDefaultForges(t *testing.T) { + rt := &recordingTransport{} + c := NewClient( + forges.WithHTTPClient(&http.Client{Transport: rt}), + forges.WithToken("codeberg.org", "secret"), + ) + + f, owner, repo, err := c.ForgeForURL(context.Background(), "https://codeberg.org/owner/repo") + if err != nil { + t.Fatalf("ForgeForURL: %v", err) + } + if owner != "owner" || repo != "repo" { + t.Errorf("owner/repo = %q/%q", owner, repo) + } + if _, err := f.Repos().Get(context.Background(), owner, repo); err == nil { + t.Fatal("expected not found error from recording transport") + } + if got := rt.auth["codeberg.org"]; got != "token secret" { + t.Errorf("Authorization = %q, want the configured token", got) + } +} + +func TestNewClientKeepsRegisteredForge(t *testing.T) { + want := &stubForge{} + c := NewClient(forges.WithForge("github.com", want)) + got, err := c.ForgeFor("github.com") + if err != nil { + t.Fatalf("ForgeFor: %v", err) + } + if got != forges.Forge(want) { + t.Errorf("ForgeFor returned %T, want the registered forge", got) + } +} + +func TestDefaultForgesAskForEachToken(t *testing.T) { + var asked []string + defaults := DefaultForges(func(domain string) string { + asked = append(asked, domain) + return "" + }, nil) + if len(asked) != len(defaults) { + t.Errorf("asked for tokens for %v, want one per default forge (%d)", asked, len(defaults)) + } + for _, domain := range asked { + if defaults[domain] == nil { + t.Errorf("no default forge for %q", domain) + } + } +} + +func TestForgeForURLDetectsUnknownDomainOnce(t *testing.T) { + var probes, apiCalls atomic.Int32 + var gotAuth atomic.Value + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/" { + probes.Add(1) + w.Header().Set("X-Forgejo-Version", "7.0.0") + return + } + apiCalls.Add(1) + gotAuth.Store(r.Header.Get("Authorization")) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "full_name": "owner/repo", + "name": "repo", + "owner": map[string]any{"login": "owner"}, + }) + })) + defer srv.Close() + host := strings.TrimPrefix(srv.URL, "http://") + + c := NewClient(forges.WithToken(host, "secret")) + ctx := context.Background() + repoURL := srv.URL + "/owner/repo" + + if _, _, _, err := c.ForgeForURL(ctx, repoURL); err != nil { + t.Fatalf("ForgeForURL: %v", err) + } + r, err := c.FetchRepository(ctx, repoURL) + if err != nil { + t.Fatalf("FetchRepository: %v", err) + } + if r.FullName != "owner/repo" { + t.Errorf("FullName = %q", r.FullName) + } + if probes.Load() != 1 { + t.Errorf("detection probes = %d, want 1", probes.Load()) + } + if apiCalls.Load() == 0 { + t.Error("expected the detected forge to reach the API") + } + if got := gotAuth.Load(); got != "token secret" { + t.Errorf("Authorization = %q, want the configured token", got) + } + if _, err := c.ForgeFor(host); err != nil { + t.Errorf("ForgeFor(%q) after detection: %v", host, err) + } +} + +func TestForgeForURLReportsUndetectedDomain(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + defer srv.Close() + + c := NewClient() + _, _, _, err := c.ForgeForURL(context.Background(), srv.URL+"/owner/repo") + if err == nil || !strings.Contains(err.Error(), "could not detect forge type") { + t.Fatalf("ForgeForURL error = %v, want a detection failure", err) + } +} + +func TestForgeForURLDetectsConcurrently(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Gitea-Version", "1.22.0") + })) + defer srv.Close() + + c := NewClient() + var wg sync.WaitGroup + for range 8 { + wg.Go(func() { + if _, _, _, err := c.ForgeForURL(context.Background(), srv.URL+"/owner/repo"); err != nil { + t.Errorf("ForgeForURL: %v", err) + } + _ = c.Tokens() + }) + } + wg.Wait() +} diff --git a/forge.go b/forge.go index cca91f8..4bd47c8 100644 --- a/forge.go +++ b/forge.go @@ -4,10 +4,12 @@ import ( "context" "errors" "fmt" + "maps" "net/http" "net/url" "strconv" "strings" + "sync" "github.com/git-pkgs/purl" ) @@ -91,9 +93,11 @@ type Forge interface { // Client routes requests to the appropriate Forge based on the URL domain. type Client struct { + mu sync.RWMutex // guards forges and tokens once the Client is in use forges map[string]Forge tokens map[string]string httpClient *http.Client + detection *ForgeBuilders } // Option configures a Client. @@ -120,6 +124,17 @@ func WithForge(domain string, f Forge) Option { } } +// WithDetection makes the Client detect and register domains it hasn't seen, +// using the given builders, the first time a repository URL on that domain is +// used. Detection sends unauthenticated requests to the domain, so callers +// handling untrusted URLs should validate them first. Detection failures are +// not cached. +func WithDetection(builders ForgeBuilders) Option { + return func(c *Client) { + c.detection = &builders + } +} + // NewClient creates a Client and applies the given options. func NewClient(opts ...Option) *Client { c := &Client{ @@ -132,9 +147,12 @@ func NewClient(opts ...Option) *Client { return c } -// Tokens returns the token map so callers can read tokens set via WithToken. +// Tokens returns a copy of the token map so callers can read tokens set via +// WithToken. func (c *Client) Tokens() map[string]string { - return c.tokens + c.mu.RLock() + defer c.mu.RUnlock() + return maps.Clone(c.tokens) } // HTTPClient returns the HTTP client configured on this Client, or nil. @@ -185,6 +203,8 @@ func (c *Client) RegisterDomain(ctx context.Context, domain, token string, build if err != nil { return fmt.Errorf("detecting forge type for %s: %w", domain, err) } + c.mu.Lock() + defer c.mu.Unlock() c.tokens[domain] = token switch ft { case GitHub: @@ -226,6 +246,8 @@ type ForgeBuilders struct { } func (c *Client) forgeFor(domain string) (Forge, error) { + c.mu.RLock() + defer c.mu.RUnlock() f, ok := c.forges[domain] if !ok { return nil, fmt.Errorf("no forge registered for domain %q", domain) @@ -233,18 +255,50 @@ func (c *Client) forgeFor(domain string) (Forge, error) { return f, nil } +// forgeOrDetect returns the Forge registered for domain. If there is none and +// detection is enabled, it probes baseURL and registers what it finds. +func (c *Client) forgeOrDetect(ctx context.Context, domain, baseURL string) (Forge, error) { + f, err := c.forgeFor(domain) + if err == nil || c.detection == nil { + return f, err + } + c.mu.RLock() + token := c.tokens[domain] + c.mu.RUnlock() + if err := c.RegisterDomain(ctx, baseURL, token, *c.detection); err != nil { + return nil, err + } + return c.forgeFor(domain) +} + // ForgeFor returns the Forge implementation registered for the given domain. func (c *Client) ForgeFor(domain string) (Forge, error) { return c.forgeFor(domain) } -// FetchRepository fetches normalized repository metadata from a URL string. -func (c *Client) FetchRepository(ctx context.Context, repoURL string) (*Repository, error) { +// ForgeForURL returns the Forge for a repository URL along with the owner and +// repo parsed from it. Without WithDetection, a domain that has not been +// registered is an error. With it, the domain is detected and registered on +// first use, over plain HTTP only if the URL itself is http://. +func (c *Client) ForgeForURL(ctx context.Context, repoURL string) (f Forge, owner, repo string, err error) { domain, owner, repo, err := ParseRepoURL(repoURL) if err != nil { - return nil, err + return nil, "", "", err } - f, err := c.forgeFor(domain) + baseURL := domain + if u, err := url.Parse(strings.TrimSpace(repoURL)); err == nil && strings.EqualFold(u.Scheme, "http") { + baseURL = "http://" + domain + } + f, err = c.forgeOrDetect(ctx, domain, baseURL) + if err != nil { + return nil, "", "", err + } + return f, owner, repo, nil +} + +// FetchRepository fetches normalized repository metadata from a URL string. +func (c *Client) FetchRepository(ctx context.Context, repoURL string) (*Repository, error) { + f, owner, repo, err := c.ForgeForURL(ctx, repoURL) if err != nil { return nil, err } @@ -263,11 +317,7 @@ func (c *Client) FetchRepositoryFromPURL(ctx context.Context, p *purl.PURL) (*Re // FetchTags fetches git tags from a URL string. func (c *Client) FetchTags(ctx context.Context, repoURL string) ([]Tag, error) { - domain, owner, repo, err := ParseRepoURL(repoURL) - if err != nil { - return nil, err - } - f, err := c.forgeFor(domain) + f, owner, repo, err := c.ForgeForURL(ctx, repoURL) if err != nil { return nil, err } @@ -276,7 +326,7 @@ func (c *Client) FetchTags(ctx context.Context, repoURL string) ([]Tag, error) { // ListRepositories lists all repositories for an owner on the given domain. func (c *Client) ListRepositories(ctx context.Context, domain, owner string, opts ListRepoOpts) ([]Repository, error) { - f, err := c.forgeFor(domain) + f, err := c.forgeOrDetect(ctx, domain, domain) if err != nil { return nil, err } diff --git a/forges_test.go b/forges_test.go index 6b77e7a..8f7e0f6 100644 --- a/forges_test.go +++ b/forges_test.go @@ -657,6 +657,133 @@ func (t rewriteHostTransport) RoundTrip(req *http.Request) (*http.Response, erro return http.DefaultTransport.RoundTrip(clone) } +// detectionTransport identifies every host as Gitea and records the requested +// URLs. It lets tests exercise https and scp-like URLs without a real server. +type detectionTransport struct { + urls []string +} + +func (rt *detectionTransport) RoundTrip(req *http.Request) (*http.Response, error) { + rt.urls = append(rt.urls, req.URL.String()) + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"X-Gitea-Version": []string{"1.22.0"}}, + Body: http.NoBody, + Request: req, + }, nil +} + +func TestForgeForURLWithoutDetectionDoesNotProbe(t *testing.T) { + rt := &detectionTransport{} + c := NewClient(WithHTTPClient(&http.Client{Transport: rt})) + + _, _, _, err := c.ForgeForURL(context.Background(), "https://git.example.com/owner/repo") + if err == nil || !strings.Contains(err.Error(), "no forge registered") { + t.Fatalf("ForgeForURL error = %v, want no forge registered", err) + } + if len(rt.urls) != 0 { + t.Errorf("probed %v, want no requests", rt.urls) + } +} + +func TestForgeForURLDetection(t *testing.T) { + tests := []struct { + name string + repoURL string + wantDomain string + wantBaseURL string + }{ + {"https", "https://git.example.com/owner/repo.git", "git.example.com", "https://git.example.com"}, + {"http with port", "http://git.example.com:3000/owner/repo", "git.example.com:3000", "http://git.example.com:3000"}, + {"uppercase http scheme", "HTTP://git.example.com/owner/repo", "git.example.com", "http://git.example.com"}, + {"bare host", "git.example.com/owner/repo", "git.example.com", "https://git.example.com"}, + {"scp-like", "git@git.example.com:owner/repo.git", "git.example.com", "https://git.example.com"}, + {"ssh with port", "ssh://git@git.example.com:2222/owner/repo.git", "git.example.com", "https://git.example.com"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotBaseURL, gotToken string + want := &mockForge{} + c := NewClient( + WithHTTPClient(&http.Client{Transport: &detectionTransport{}}), + WithToken(tt.wantDomain, "tok"), + WithDetection(ForgeBuilders{ + Gitea: func(baseURL, token string, _ *http.Client) Forge { + gotBaseURL, gotToken = baseURL, token + return want + }, + }), + ) + + f, owner, repo, err := c.ForgeForURL(context.Background(), tt.repoURL) + if err != nil { + t.Fatalf("ForgeForURL: %v", err) + } + if f != Forge(want) { + t.Errorf("forge = %T, want the built forge", f) + } + assertEqual(t, "owner", "owner", owner) + assertEqual(t, "repo", "repo", repo) + assertEqual(t, "baseURL", tt.wantBaseURL, gotBaseURL) + assertEqual(t, "token", "tok", gotToken) + if _, err := c.ForgeFor(tt.wantDomain); err != nil { + t.Errorf("ForgeFor(%q): %v", tt.wantDomain, err) + } + }) + } +} + +func TestForgeForURLInvalidURL(t *testing.T) { + rt := &detectionTransport{} + c := NewClient(WithHTTPClient(&http.Client{Transport: rt}), WithDetection(ForgeBuilders{})) + if _, _, _, err := c.ForgeForURL(context.Background(), "https://git.example.com/owner"); err == nil { + t.Fatal("expected error for URL without a repo") + } + if len(rt.urls) != 0 { + t.Errorf("probed %v, want no requests", rt.urls) + } +} + +func TestClientMethodsDetectOnFirstUse(t *testing.T) { + builders := ForgeBuilders{ + Gitea: func(string, string, *http.Client) Forge { + return &mockForge{ + repoService: &mockRepoService{ + repos: []Repository{{FullName: "owner/repo"}}, + tags: []Tag{{Name: "v1"}}, + repo: &Repository{FullName: "owner/repo"}, + }, + commitService: &mockCommitService{sha: "abc"}, + } + }, + } + newClient := func() *Client { + return NewClient(WithHTTPClient(&http.Client{Transport: &detectionTransport{}}), WithDetection(builders)) + } + ctx := context.Background() + repoURL := "https://git.example.com/owner/repo" + + if r, err := newClient().FetchRepository(ctx, repoURL); err != nil || r.FullName != "owner/repo" { + t.Errorf("FetchRepository = %+v, %v", r, err) + } + if tags, err := newClient().FetchTags(ctx, repoURL); err != nil || len(tags) != 1 { + t.Errorf("FetchTags = %+v, %v", tags, err) + } + if repos, err := newClient().ListRepositories(ctx, "git.example.com", "owner", ListRepoOpts{}); err != nil || len(repos) != 1 { + t.Errorf("ListRepositories = %+v, %v", repos, err) + } + if sha, err := newClient().ResolveCommit(ctx, repoURL, "main"); err != nil || sha != "abc" { + t.Errorf("ResolveCommit = %q, %v", sha, err) + } +} + +func TestTokensReturnsCopy(t *testing.T) { + c := NewClient(WithToken("github.com", "tok")) + c.Tokens()["github.com"] = "changed" + assertEqual(t, "token", "tok", c.Tokens()["github.com"]) +} + func TestClientListRepositoriesRoutes(t *testing.T) { mock := &mockForge{ repoService: &mockRepoService{ diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 647fb03..2b1c502 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -12,7 +12,7 @@ import ( "strings" "github.com/git-pkgs/forge" - "github.com/git-pkgs/forge/bitbucket" + "github.com/git-pkgs/forge/factory" "github.com/git-pkgs/forge/gerrit" "github.com/git-pkgs/forge/gitea" ghforge "github.com/git-pkgs/forge/github" @@ -106,15 +106,6 @@ func ResetTestForge() { testDomain = "" } -var builders = forges.ForgeBuilders{ - GitHub: ghforge.NewWithBase, - GitLab: glforge.New, - Gitea: gitea.New, - Gerrit: gerrit.New, - Tangled: tangled.New, - Radicle: radicle.New, -} - // Repo figures out the forge, owner, and repo name from flags or a git remote. // The --repo (-R) flag takes precedence; otherwise it parses the remote selected // by --remote, config, or the "origin" fallback. @@ -411,14 +402,7 @@ func newClientForBaseURL(domain, baseURL string) *forges.Client { // Register default forges first, so config-based registrations override them. hc := HTTPClient() - defaults := map[string]forges.Forge{ - "github.com": ghforge.New(TokenForDomain("github.com"), hc), - "gitlab.com": glforge.New("https://gitlab.com", TokenForDomain("gitlab.com"), hc), - "codeberg.org": gitea.New("https://codeberg.org", TokenForDomain("codeberg.org"), hc), - "bitbucket.org": bitbucket.New(TokenForDomain("bitbucket.org"), hc), - "tangled.org": tangled.New("https://tangled.org", TokenForDomain("tangled.org"), hc), - } - for d, f := range defaults { + for d, f := range factory.DefaultForges(TokenForDomain, hc) { opts = append(opts, forges.WithForge(d, f)) } @@ -482,7 +466,7 @@ func forgeForDomainAtBaseURL(ctx context.Context, client *forges.Client, domain, return f, nil } token := TokenForDomain(domain) - if regErr := client.RegisterDomain(ctx, baseURL, token, builders); regErr != nil { + if regErr := client.RegisterDomain(ctx, baseURL, token, factory.Builders()); regErr != nil { return nil, fmt.Errorf("unknown forge at %s: %w (use --forge-type, or set type under [%s] in config, to skip detection)", domain, regErr, domain) } return client.ForgeFor(domain) From 2392d0a0006e32cf148b1ac7d53a0ba2cd6c63b3 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Sat, 7 Nov 2026 22:41:48 +0530 Subject: [PATCH 2/7] Always detect unregistered domains over https ForgeForURL used the scheme of the first URL seen for a host, so a single http:// repository URL registered that host over plain HTTP and sent its configured token in cleartext on every later call. Detection now always probes over https, and ListRepositories no longer detects a domain that carries a scheme. Plain-HTTP instances can still be registered explicitly with RegisterDomain. --- README.md | 2 +- factory/factory_test.go | 84 +++++++++++++++++++++++++++++++++++++---- forge.go | 26 ++++++------- forges_test.go | 29 ++++++++++++-- 4 files changed, 116 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 3c4f752..bc69df2 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,7 @@ issues, _ := f.Issues().List(ctx, "octocat", "hello-world", forges.ListIssueOpts pr, _ := f.PullRequests().Get(ctx, "octocat", "hello-world", 42) ``` -The `factory` package imports every provider and returns a `Client` that already knows github.com, gitlab.com, codeberg.org, bitbucket.org and tangled.org. It detects and registers any other domain the first time a repository URL on it is used, which sends requests to that domain: +The `factory` package imports every provider and returns a `Client` that already knows github.com, gitlab.com, codeberg.org, bitbucket.org and tangled.org. It detects and registers any other domain over https the first time a repository URL on it is used, which sends requests to that domain. A plain-HTTP instance has to be registered with `RegisterDomain`: ```go import "github.com/git-pkgs/forge/factory" diff --git a/factory/factory_test.go b/factory/factory_test.go index 72e7fd3..9a3050e 100644 --- a/factory/factory_test.go +++ b/factory/factory_test.go @@ -3,6 +3,7 @@ package factory import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" "strings" @@ -39,6 +40,34 @@ func (rt *recordingTransport) RoundTrip(req *http.Request) (*http.Response, erro }, nil } +// giteaTransport answers like a Gitea instance on every host and records each +// request's URL and Authorization header. +type giteaTransport struct { + mu sync.Mutex + reqs []recordedRequest +} + +type recordedRequest struct { + url string + auth string +} + +func (rt *giteaTransport) RoundTrip(req *http.Request) (*http.Response, error) { + rt.mu.Lock() + rt.reqs = append(rt.reqs, recordedRequest{url: req.URL.String(), auth: req.Header.Get("Authorization")}) + rt.mu.Unlock() + body := `{"full_name":"owner/repo","name":"repo","owner":{"login":"owner"}}` + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "X-Gitea-Version": []string{"1.22.0"}, + "Content-Type": []string{"application/json"}, + }, + Body: io.NopCloser(strings.NewReader(body)), + Request: req, + }, nil +} + func TestNewClientRegistersDefaultForges(t *testing.T) { c := NewClient() for domain, wantAPI := range map[string]string{ @@ -117,7 +146,7 @@ func TestDefaultForgesAskForEachToken(t *testing.T) { func TestForgeForURLDetectsUnknownDomainOnce(t *testing.T) { var probes, apiCalls atomic.Int32 var gotAuth atomic.Value - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/" { probes.Add(1) w.Header().Set("X-Forgejo-Version", "7.0.0") @@ -133,9 +162,9 @@ func TestForgeForURLDetectsUnknownDomainOnce(t *testing.T) { }) })) defer srv.Close() - host := strings.TrimPrefix(srv.URL, "http://") + host := strings.TrimPrefix(srv.URL, "https://") - c := NewClient(forges.WithToken(host, "secret")) + c := NewClient(forges.WithHTTPClient(srv.Client()), forges.WithToken(host, "secret")) ctx := context.Background() repoURL := srv.URL + "/owner/repo" @@ -164,10 +193,10 @@ func TestForgeForURLDetectsUnknownDomainOnce(t *testing.T) { } func TestForgeForURLReportsUndetectedDomain(t *testing.T) { - srv := httptest.NewServer(http.NotFoundHandler()) + srv := httptest.NewTLSServer(http.NotFoundHandler()) defer srv.Close() - c := NewClient() + c := NewClient(forges.WithHTTPClient(srv.Client())) _, _, _, err := c.ForgeForURL(context.Background(), srv.URL+"/owner/repo") if err == nil || !strings.Contains(err.Error(), "could not detect forge type") { t.Fatalf("ForgeForURL error = %v, want a detection failure", err) @@ -175,12 +204,12 @@ func TestForgeForURLReportsUndetectedDomain(t *testing.T) { } func TestForgeForURLDetectsConcurrently(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.Header().Set("X-Gitea-Version", "1.22.0") })) defer srv.Close() - c := NewClient() + c := NewClient(forges.WithHTTPClient(srv.Client())) var wg sync.WaitGroup for range 8 { wg.Go(func() { @@ -192,3 +221,44 @@ func TestForgeForURLDetectsConcurrently(t *testing.T) { } wg.Wait() } + +func TestForgeForURLDetectsHTTPURLOverHTTPS(t *testing.T) { + rt := &giteaTransport{} + c := NewClient( + forges.WithHTTPClient(&http.Client{Transport: rt}), + forges.WithToken("git.example.com", "secret"), + ) + ctx := context.Background() + + if _, err := c.FetchRepository(ctx, "http://git.example.com/owner/repo"); err != nil { + t.Fatalf("FetchRepository over http URL: %v", err) + } + if _, err := c.FetchRepository(ctx, "https://git.example.com/owner/repo"); err != nil { + t.Fatalf("FetchRepository over https URL: %v", err) + } + + var sawToken bool + for _, r := range rt.reqs { + if !strings.HasPrefix(r.url, "https://") { + t.Errorf("request to %q, want https", r.url) + } + if r.auth == "token secret" { + sawToken = true + } + } + if !sawToken { + t.Error("expected the configured token on the https API requests") + } + + f, err := c.ForgeFor("git.example.com") + if err != nil { + t.Fatalf("ForgeFor: %v", err) + } + provider, ok := f.(forges.APIBaseURLProvider) + if !ok { + t.Fatal("detected forge does not expose its API URL") + } + if got, want := provider.APIBaseURL(), "https://git.example.com/api/v1"; got != want { + t.Errorf("API URL = %q, want %q", got, want) + } +} diff --git a/forge.go b/forge.go index 4bd47c8..3bb6d8c 100644 --- a/forge.go +++ b/forge.go @@ -126,9 +126,10 @@ func WithForge(domain string, f Forge) Option { // WithDetection makes the Client detect and register domains it hasn't seen, // using the given builders, the first time a repository URL on that domain is -// used. Detection sends unauthenticated requests to the domain, so callers -// handling untrusted URLs should validate them first. Detection failures are -// not cached. +// used. Detection always uses https, whatever the scheme of the URL, so a +// plain-HTTP instance has to be registered explicitly with RegisterDomain. +// Detection sends unauthenticated requests to the domain, so callers handling +// untrusted URLs should validate them first. Detection failures are not cached. func WithDetection(builders ForgeBuilders) Option { return func(c *Client) { c.detection = &builders @@ -256,16 +257,17 @@ func (c *Client) forgeFor(domain string) (Forge, error) { } // forgeOrDetect returns the Forge registered for domain. If there is none and -// detection is enabled, it probes baseURL and registers what it finds. -func (c *Client) forgeOrDetect(ctx context.Context, domain, baseURL string) (Forge, error) { +// detection is enabled, it probes the domain over https and registers what it +// finds. +func (c *Client) forgeOrDetect(ctx context.Context, domain string) (Forge, error) { f, err := c.forgeFor(domain) - if err == nil || c.detection == nil { + if err == nil || c.detection == nil || strings.Contains(domain, "://") { return f, err } c.mu.RLock() token := c.tokens[domain] c.mu.RUnlock() - if err := c.RegisterDomain(ctx, baseURL, token, *c.detection); err != nil { + if err := c.RegisterDomain(ctx, "https://"+domain, token, *c.detection); err != nil { return nil, err } return c.forgeFor(domain) @@ -279,17 +281,13 @@ func (c *Client) ForgeFor(domain string) (Forge, error) { // ForgeForURL returns the Forge for a repository URL along with the owner and // repo parsed from it. Without WithDetection, a domain that has not been // registered is an error. With it, the domain is detected and registered on -// first use, over plain HTTP only if the URL itself is http://. +// first use over https, even if the URL itself is http://. func (c *Client) ForgeForURL(ctx context.Context, repoURL string) (f Forge, owner, repo string, err error) { domain, owner, repo, err := ParseRepoURL(repoURL) if err != nil { return nil, "", "", err } - baseURL := domain - if u, err := url.Parse(strings.TrimSpace(repoURL)); err == nil && strings.EqualFold(u.Scheme, "http") { - baseURL = "http://" + domain - } - f, err = c.forgeOrDetect(ctx, domain, baseURL) + f, err = c.forgeOrDetect(ctx, domain) if err != nil { return nil, "", "", err } @@ -326,7 +324,7 @@ func (c *Client) FetchTags(ctx context.Context, repoURL string) ([]Tag, error) { // ListRepositories lists all repositories for an owner on the given domain. func (c *Client) ListRepositories(ctx context.Context, domain, owner string, opts ListRepoOpts) ([]Repository, error) { - f, err := c.forgeOrDetect(ctx, domain, domain) + f, err := c.forgeOrDetect(ctx, domain) if err != nil { return nil, err } diff --git a/forges_test.go b/forges_test.go index 8f7e0f6..e646441 100644 --- a/forges_test.go +++ b/forges_test.go @@ -694,8 +694,8 @@ func TestForgeForURLDetection(t *testing.T) { wantBaseURL string }{ {"https", "https://git.example.com/owner/repo.git", "git.example.com", "https://git.example.com"}, - {"http with port", "http://git.example.com:3000/owner/repo", "git.example.com:3000", "http://git.example.com:3000"}, - {"uppercase http scheme", "HTTP://git.example.com/owner/repo", "git.example.com", "http://git.example.com"}, + {"http with port", "http://git.example.com:3000/owner/repo", "git.example.com:3000", "https://git.example.com:3000"}, + {"uppercase http scheme", "HTTP://git.example.com/owner/repo", "git.example.com", "https://git.example.com"}, {"bare host", "git.example.com/owner/repo", "git.example.com", "https://git.example.com"}, {"scp-like", "git@git.example.com:owner/repo.git", "git.example.com", "https://git.example.com"}, {"ssh with port", "ssh://git@git.example.com:2222/owner/repo.git", "git.example.com", "https://git.example.com"}, @@ -705,8 +705,9 @@ func TestForgeForURLDetection(t *testing.T) { t.Run(tt.name, func(t *testing.T) { var gotBaseURL, gotToken string want := &mockForge{} + rt := &detectionTransport{} c := NewClient( - WithHTTPClient(&http.Client{Transport: &detectionTransport{}}), + WithHTTPClient(&http.Client{Transport: rt}), WithToken(tt.wantDomain, "tok"), WithDetection(ForgeBuilders{ Gitea: func(baseURL, token string, _ *http.Client) Forge { @@ -730,10 +731,32 @@ func TestForgeForURLDetection(t *testing.T) { if _, err := c.ForgeFor(tt.wantDomain); err != nil { t.Errorf("ForgeFor(%q): %v", tt.wantDomain, err) } + for _, u := range rt.urls { + if !strings.HasPrefix(u, "https://") { + t.Errorf("probed %q, want https", u) + } + } }) } } +func TestListRepositoriesDoesNotDetectSchemeDomain(t *testing.T) { + rt := &detectionTransport{} + c := NewClient( + WithHTTPClient(&http.Client{Transport: rt}), + WithDetection(ForgeBuilders{Gitea: func(string, string, *http.Client) Forge { return &mockForge{} }}), + ) + if _, err := c.ListRepositories(context.Background(), "http://git.example.com", "owner", ListRepoOpts{}); err == nil { + t.Fatal("expected error for a domain with a scheme") + } + if len(rt.urls) != 0 { + t.Errorf("probed %v, want no requests", rt.urls) + } + if _, err := c.ForgeFor("git.example.com"); err == nil { + t.Error("git.example.com was registered, want no registration") + } +} + func TestForgeForURLInvalidURL(t *testing.T) { rt := &detectionTransport{} c := NewClient(WithHTTPClient(&http.Client{Transport: rt}), WithDetection(ForgeBuilders{})) From 037535e5ca8f05b29a02e7418de4203c53dd64dc Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Mon, 9 Nov 2026 15:41:33 +0530 Subject: [PATCH 3/7] Return an error when the detected forge type has no builder RegisterDomain only checked for a nil builder for Gerrit, Tangled and Radicle, so a partial ForgeBuilders passed to WithDetection could panic when a host was detected as GitHub, GitLab, Gitea or Forgejo. Pick the builder first and check it once for every forge type, before taking the lock and storing the token. --- forge.go | 32 ++++++++++++++------------------ forges_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/forge.go b/forge.go index 3bb6d8c..7ee7e38 100644 --- a/forge.go +++ b/forge.go @@ -204,34 +204,30 @@ func (c *Client) RegisterDomain(ctx context.Context, domain, token string, build if err != nil { return fmt.Errorf("detecting forge type for %s: %w", domain, err) } - c.mu.Lock() - defer c.mu.Unlock() - c.tokens[domain] = token + var build func(baseURL, token string, hc *http.Client) Forge switch ft { case GitHub: - c.forges[domain] = builders.GitHub(baseURL, token, c.httpClient) + build = builders.GitHub case GitLab: - c.forges[domain] = builders.GitLab(baseURL, token, c.httpClient) + build = builders.GitLab case Gitea, Forgejo: - c.forges[domain] = builders.Gitea(baseURL, token, c.httpClient) + build = builders.Gitea case Gerrit: - if builders.Gerrit == nil { - return fmt.Errorf("no builder registered for forge type %q at %s", ft, domain) - } - c.forges[domain] = builders.Gerrit(baseURL, token, c.httpClient) + build = builders.Gerrit case Tangled: - if builders.Tangled == nil { - return fmt.Errorf("no builder registered for forge type %q at %s", ft, domain) - } - c.forges[domain] = builders.Tangled(baseURL, token, c.httpClient) + build = builders.Tangled case Radicle: - if builders.Radicle == nil { - return fmt.Errorf("no builder registered for forge type %q at %s", ft, domain) - } - c.forges[domain] = builders.Radicle(baseURL, token, c.httpClient) + build = builders.Radicle default: return fmt.Errorf("unsupported forge type %q for %s", ft, domain) } + if build == nil { + return fmt.Errorf("no builder registered for forge type %q at %s", ft, domain) + } + c.mu.Lock() + defer c.mu.Unlock() + c.tokens[domain] = token + c.forges[domain] = build(baseURL, token, c.httpClient) return nil } diff --git a/forges_test.go b/forges_test.go index e646441..2239131 100644 --- a/forges_test.go +++ b/forges_test.go @@ -646,6 +646,48 @@ func TestRegisterDomainTangledMissingBuilderReturnsError(t *testing.T) { } } +func TestDetectionMissingBuilderReturnsError(t *testing.T) { + stub := func(string, string, *http.Client) Forge { return &mockForge{} } + tests := []struct { + name string + header string + value string + builders ForgeBuilders + }{ + {"github", "X-GitHub-Request-Id", "abc123", ForgeBuilders{GitLab: stub, Gitea: stub}}, + {"gitlab", "X-Gitlab-Meta", `{"cors":"abc"}`, ForgeBuilders{GitHub: stub, Gitea: stub}}, + {"gitea", "X-Gitea-Version", "1.21.0", ForgeBuilders{GitHub: stub, GitLab: stub}}, + {"forgejo", "X-Forgejo-Version", "7.0.0", ForgeBuilders{GitHub: stub, GitLab: stub}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(tt.header, tt.value) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + base, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + c := NewClient( + WithHTTPClient(&http.Client{Transport: rewriteHostTransport{base: base}}), + WithDetection(tt.builders), + ) + + _, _, _, err = c.ForgeForURL(context.Background(), "https://git.example.com/owner/repo") + if err == nil || !strings.Contains(err.Error(), "no builder registered") || !strings.Contains(err.Error(), tt.name) { + t.Fatalf("ForgeForURL error = %v, want missing %s builder error", err, tt.name) + } + if _, err := c.ForgeFor("git.example.com"); err == nil { + t.Error("git.example.com was registered, want no registration") + } + }) + } +} + type rewriteHostTransport struct { base *url.URL } From 85e420c8b4e5ca61fe5436d63d35f4d375a60631 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Tue, 10 Nov 2026 17:45:03 +0530 Subject: [PATCH 4/7] Match forge domains case-insensitively ParseRepoURL returned the host as written while the forge and token maps are keyed by that string, so a URL such as https://GitHub.com/owner/repo missed the github.com entry. With detection enabled it was then probed and registered a second time as a GitHub Enterprise client without the configured token. Lowercase the host in ParseRepoURL and normalizeBaseURL, and use lowercased keys in WithToken, WithForge, ForgeFor and the detection lookup, so lookup, token and registration all agree on one key. Also make the factory README example check the ForgeForURL error and show how to register a plain-HTTP instance. --- README.md | 13 +++++++++++- factory/factory_test.go | 23 +++++++++++++++++++++ forge.go | 29 ++++++++++++++++---------- forges_test.go | 46 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index bc69df2..9ddce3d 100644 --- a/README.md +++ b/README.md @@ -177,7 +177,7 @@ issues, _ := f.Issues().List(ctx, "octocat", "hello-world", forges.ListIssueOpts pr, _ := f.PullRequests().Get(ctx, "octocat", "hello-world", 42) ``` -The `factory` package imports every provider and returns a `Client` that already knows github.com, gitlab.com, codeberg.org, bitbucket.org and tangled.org. It detects and registers any other domain over https the first time a repository URL on it is used, which sends requests to that domain. A plain-HTTP instance has to be registered with `RegisterDomain`: +The `factory` package imports every provider and returns a `Client` that already knows github.com, gitlab.com, codeberg.org, bitbucket.org and tangled.org. It detects and registers any other domain over https the first time a repository URL on it is used, which sends requests to that domain: ```go import "github.com/git-pkgs/forge/factory" @@ -185,9 +185,20 @@ import "github.com/git-pkgs/forge/factory" client := factory.NewClient(forges.WithToken("git.example.com", token)) f, owner, repo, err := client.ForgeForURL(ctx, "https://git.example.com/octocat/hello-world") +if err != nil { + return err +} issues, err := f.Issues().List(ctx, owner, repo, forges.ListIssueOpts{State: "open"}) ``` +Detection never uses plain HTTP, so an http-only instance has to be registered with `RegisterDomain` before its URLs are used: + +```go +if err := client.RegisterDomain(ctx, "http://git.internal.example", token, factory.Builders()); err != nil { + return err +} +``` + Incoming webhook deliveries from GitHub, GitLab and Gitea/Forgejo can be verified and parsed into a normalized `WebhookEvent`. Event types use the same names as `Webhook.Events`. Push, pull request, issue, comment and release deliveries carry typed fields. The raw JSON is always available in `Payload`. An empty secret skips verification, so read the secret once at startup and refuse to run without it: diff --git a/factory/factory_test.go b/factory/factory_test.go index 9a3050e..c49539f 100644 --- a/factory/factory_test.go +++ b/factory/factory_test.go @@ -127,6 +127,29 @@ func TestNewClientKeepsRegisteredForge(t *testing.T) { } } +func TestForgeForURLMatchesDefaultForgeCaseInsensitively(t *testing.T) { + rt := &recordingTransport{} + c := NewClient(forges.WithHTTPClient(&http.Client{Transport: rt})) + + f, owner, repo, err := c.ForgeForURL(context.Background(), "https://GitHub.com/owner/repo") + if err != nil { + t.Fatalf("ForgeForURL: %v", err) + } + if owner != "owner" || repo != "repo" { + t.Errorf("owner/repo = %q/%q", owner, repo) + } + want, err := c.ForgeFor("github.com") + if err != nil { + t.Fatalf("ForgeFor: %v", err) + } + if f != want { + t.Errorf("ForgeForURL returned %T, want the default github.com forge", f) + } + if len(rt.auth) != 0 { + t.Errorf("requests sent to %v, want no detection probe", rt.auth) + } +} + func TestDefaultForgesAskForEachToken(t *testing.T) { var asked []string defaults := DefaultForges(func(domain string) string { diff --git a/forge.go b/forge.go index 7ee7e38..7615883 100644 --- a/forge.go +++ b/forge.go @@ -103,10 +103,11 @@ type Client struct { // Option configures a Client. type Option func(*Client) -// WithToken sets the API token for the given domain. +// WithToken sets the API token for the given domain. Domains are matched +// case-insensitively. func WithToken(domain, token string) Option { return func(c *Client) { - c.tokens[domain] = token + c.tokens[strings.ToLower(domain)] = token } } @@ -117,10 +118,11 @@ func WithHTTPClient(hc *http.Client) Option { } } -// WithForge registers a Forge implementation for the given domain. +// WithForge registers a Forge implementation for the given domain. Domains are +// matched case-insensitively. func WithForge(domain string, f Forge) Option { return func(c *Client) { - c.forges[domain] = f + c.forges[strings.ToLower(domain)] = f } } @@ -149,7 +151,7 @@ func NewClient(opts ...Option) *Client { } // Tokens returns a copy of the token map so callers can read tokens set via -// WithToken. +// WithToken. The map is keyed by lowercased domain. func (c *Client) Tokens() map[string]string { c.mu.RLock() defer c.mu.RUnlock() @@ -162,7 +164,8 @@ func (c *Client) HTTPClient() *http.Client { } // normalizeBaseURL accepts either a bare host[:port] or a full http(s) URL and -// returns (baseURL, host). A bare host is assumed https. +// returns (baseURL, host). A bare host is assumed https. The host is lowercased +// in both return values. func normalizeBaseURL(s string) (baseURL, host string, err error) { if !strings.Contains(s, "://") { s = "https://" + s @@ -186,6 +189,7 @@ func normalizeBaseURL(s string) (baseURL, host string, err error) { return "", "", fmt.Errorf("invalid forge URL %q: query and fragment are not supported", s) } + u.Host = strings.ToLower(u.Host) u.Path = strings.TrimRight(u.Path, "/") u.RawPath = strings.TrimRight(u.RawPath, "/") return u.String(), u.Host, nil @@ -194,7 +198,7 @@ func normalizeBaseURL(s string) (baseURL, host string, err error) { // RegisterDomain detects the forge type for a domain and registers the // appropriate Forge using the provided builder functions. The domain may // include an http:// or https:// prefix; without one, https is assumed. -// The bare host[:port] is used as the registry key. +// The lowercased bare host[:port] is used as the registry key. func (c *Client) RegisterDomain(ctx context.Context, domain, token string, builders ForgeBuilders) error { baseURL, domain, err := normalizeBaseURL(domain) if err != nil { @@ -245,7 +249,7 @@ type ForgeBuilders struct { func (c *Client) forgeFor(domain string) (Forge, error) { c.mu.RLock() defer c.mu.RUnlock() - f, ok := c.forges[domain] + f, ok := c.forges[strings.ToLower(domain)] if !ok { return nil, fmt.Errorf("no forge registered for domain %q", domain) } @@ -256,6 +260,7 @@ func (c *Client) forgeFor(domain string) (Forge, error) { // detection is enabled, it probes the domain over https and registers what it // finds. func (c *Client) forgeOrDetect(ctx context.Context, domain string) (Forge, error) { + domain = strings.ToLower(domain) f, err := c.forgeFor(domain) if err == nil || c.detection == nil || strings.Contains(domain, "://") { return f, err @@ -270,6 +275,7 @@ func (c *Client) forgeOrDetect(ctx context.Context, domain string) (Forge, error } // ForgeFor returns the Forge implementation registered for the given domain. +// Domains are matched case-insensitively. func (c *Client) ForgeFor(domain string) (Forge, error) { return c.forgeFor(domain) } @@ -387,7 +393,8 @@ func (c *Client) FetchTagsFromPURL(ctx context.Context, p *purl.PURL) ([]Tag, er // ParseRepoURL extracts the domain, owner, and repo from a repository URL. // It handles https://, schemeless, and scp-like SSH URLs (sshuser@host:owner/repo -// or host:owner/repo), and strips .git suffixes and extra path segments. +// or host:owner/repo), and strips .git suffixes and extra path segments. The +// domain is lowercased, so it can be used as a Client registry key. func ParseRepoURL(rawURL string) (domain, owner, repo string, err error) { rawURL = strings.TrimSpace(rawURL) if rawURL == "" { @@ -397,7 +404,7 @@ func ParseRepoURL(rawURL string) (domain, owner, repo string, err error) { // Handle scp-like syntax: user@host:owner/repo (e.g. git@github.com:user/repo). if !strings.Contains(rawURL, "://") { if domain, path, ok := splitSCPLike(rawURL); ok { - return splitOwnerRepo(domain, path) + return splitOwnerRepo(strings.ToLower(domain), path) } rawURL = "https://" + rawURL } @@ -410,7 +417,7 @@ func ParseRepoURL(rawURL string) (domain, owner, repo string, err error) { if scheme := strings.ToLower(u.Scheme); scheme == "http" || scheme == "https" { domain = u.Host } - return splitOwnerRepo(domain, u.Path) + return splitOwnerRepo(strings.ToLower(domain), u.Path) } // splitSCPLike recognizes git's scp-like remote syntax host:path, optionally diff --git a/forges_test.go b/forges_test.go index 2239131..6829e03 100644 --- a/forges_test.go +++ b/forges_test.go @@ -116,6 +116,18 @@ func TestParseRepoURL(t *testing.T) { input: "ssh://git@forge.example.com:2222/owner/repo.git", domain: "forge.example.com", owner: "owner", repo: "repo", }, + { + input: "https://GitHub.com/Octocat/Hello-World", + domain: "github.com", owner: "Octocat", repo: "Hello-World", + }, + { + input: "git@GitLab.com:Group/Project.git", + domain: "gitlab.com", owner: "Group", repo: "Project", + }, + { + input: "ssh://git@Forge.Example.com:2222/owner/repo.git", + domain: "forge.example.com", owner: "owner", repo: "repo", + }, { input: "", wantErr: true, @@ -287,6 +299,15 @@ func TestRegisterDomainAcceptsHTTPURL(t *testing.T) { } } +func TestNormalizeBaseURLLowercasesHost(t *testing.T) { + baseURL, host, err := normalizeBaseURL("HTTPS://Git.Example.com:3000/Forge/") + if err != nil { + t.Fatalf("normalizeBaseURL: %v", err) + } + assertEqual(t, "baseURL", "https://git.example.com:3000/Forge", baseURL) + assertEqual(t, "host", "git.example.com:3000", host) +} + func TestNormalizeBaseURLRejectsUnsupportedURLParts(t *testing.T) { for _, input := range []string{ "ftp://forge.example.com", @@ -849,6 +870,31 @@ func TestTokensReturnsCopy(t *testing.T) { assertEqual(t, "token", "tok", c.Tokens()["github.com"]) } +func TestClientMatchesDomainsCaseInsensitively(t *testing.T) { + mock := &mockForge{repoService: &mockRepoService{repos: []Repository{{FullName: "org/repo"}}}} + rt := &detectionTransport{} + c := NewClient( + WithHTTPClient(&http.Client{Transport: rt}), + WithForge("Example.com", mock), + WithToken("Example.com", "tok"), + WithDetection(ForgeBuilders{Gitea: func(string, string, *http.Client) Forge { return &mockForge{} }}), + ) + + if f, err := c.ForgeFor("EXAMPLE.COM"); err != nil || f != Forge(mock) { + t.Errorf("ForgeFor = %T, %v, want the registered forge", f, err) + } + if repos, err := c.ListRepositories(context.Background(), "EXAMPLE.com", "org", ListRepoOpts{}); err != nil || len(repos) != 1 { + t.Errorf("ListRepositories = %+v, %v", repos, err) + } + if f, _, _, err := c.ForgeForURL(context.Background(), "https://eXample.COM/org/repo"); err != nil || f != Forge(mock) { + t.Errorf("ForgeForURL = %T, %v, want the registered forge", f, err) + } + assertEqual(t, "token", "tok", c.Tokens()["example.com"]) + if len(rt.urls) != 0 { + t.Errorf("probed %v, want no requests", rt.urls) + } +} + func TestClientListRepositoriesRoutes(t *testing.T) { mock := &mockForge{ repoService: &mockRepoService{ From 1cd40c4777f06b1da11b89761f2b2d67486888e0 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Tue, 10 Nov 2026 22:18:52 +0530 Subject: [PATCH 5/7] Lowercase config domains and build forges outside the client lock Config section names are now lowercased on load and in SetDomain, and the domain lookups and ssh_host matching ignore case, so a mixed-case remote still finds its token, type, scheme and git_protocol after ParseRepoURL started lowercasing hosts. RegisterDomain calls the builder before taking the client lock so a builder can call Tokens or ForgeFor without deadlocking. --- forge.go | 4 +- forges_test.go | 41 +++++++++++++++++++ internal/config/config.go | 23 ++++++++++- internal/config/config_test.go | 70 ++++++++++++++++++++++++++++++++ internal/resolve/resolve.go | 6 +-- internal/resolve/resolve_test.go | 38 +++++++++++++++++ 6 files changed, 176 insertions(+), 6 deletions(-) diff --git a/forge.go b/forge.go index 7615883..d67a526 100644 --- a/forge.go +++ b/forge.go @@ -228,10 +228,12 @@ func (c *Client) RegisterDomain(ctx context.Context, domain, token string, build if build == nil { return fmt.Errorf("no builder registered for forge type %q at %s", ft, domain) } + // Build before locking so a builder can call back into the Client. + f := build(baseURL, token, c.httpClient) c.mu.Lock() defer c.mu.Unlock() c.tokens[domain] = token - c.forges[domain] = build(baseURL, token, c.httpClient) + c.forges[domain] = f return nil } diff --git a/forges_test.go b/forges_test.go index 6829e03..ea3e239 100644 --- a/forges_test.go +++ b/forges_test.go @@ -10,6 +10,7 @@ import ( "os" "strings" "testing" + "time" ) func assertEqual(t *testing.T, field, want, got string) { @@ -1786,3 +1787,43 @@ func (m *mockReviewService) RemoveReviewers(_ context.Context, owner, repo strin m.lastNumber = number return nil } + +func TestRegisterDomainBuilderMayUseClient(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Gitea-Version", "1.21.0") + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + base, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + c := NewClient(WithHTTPClient(&http.Client{Transport: rewriteHostTransport{base: base}})) + + done := make(chan error, 1) + go func() { + done <- c.RegisterDomain(context.Background(), "git.example.com", "tok", ForgeBuilders{ + Gitea: func(string, string, *http.Client) Forge { + _ = c.Tokens() + _, _ = c.ForgeFor("git.example.com") + return &mockForge{} + }, + }) + }() + + select { + case err := <-done: + if err != nil { + t.Fatalf("RegisterDomain: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("RegisterDomain deadlocked when the builder called back into the Client") + } + if _, err := c.ForgeFor("git.example.com"); err != nil { + t.Errorf("ForgeFor after RegisterDomain: %v", err) + } + if got := c.Tokens()["git.example.com"]; got != "tok" { + t.Errorf("token = %q, want tok", got) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 141c817..9cbf93d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -59,7 +59,7 @@ func (c *Config) DomainForSSHHost(sshHost string) string { return "" } for name, ds := range c.Domains { - if ds.SSHHost == sshHost { + if strings.EqualFold(ds.SSHHost, sshHost) { return name } } @@ -88,7 +88,7 @@ func GitProtocolFor(domain string) string { if err != nil || cfg == nil { return "https" } - if ds, ok := cfg.Domains[domain]; ok && ds.GitProtocol != "" { + if ds, ok := cfg.Domains[strings.ToLower(domain)]; ok && ds.GitProtocol != "" { return ds.GitProtocol } if cfg.Default.GitProtocol != "" { @@ -218,6 +218,8 @@ func loadFile(cfg *Config, path string, userConfig bool) error { if name == "default" { continue } + // Domains are matched case-insensitively, so key them by lowercase. + name = strings.ToLower(name) ds := cfg.Domains[name] if v, ok := kv["type"]; ok { ds.Type = v @@ -374,6 +376,23 @@ func SetDomain(domain, token, tokenCmd, forgeType, scheme string) error { } } + domain = strings.ToLower(domain) + // Fold any existing section spelled with different case into the + // lowercase one so the file never holds two sections for one domain. + for name, kv := range sections { + if name == domain || !strings.EqualFold(name, domain) { + continue + } + if _, ok := sections[domain]; !ok { + sections[domain] = make(map[string]string) + } + for k, v := range kv { + if _, ok := sections[domain][k]; !ok { + sections[domain][k] = v + } + } + delete(sections, name) + } if _, ok := sections[domain]; !ok { sections[domain] = make(map[string]string) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 51c137a..68b9499 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -780,3 +780,73 @@ git_protocol = typo t.Errorf("expected error about invalid git_protocol, got: %v", loadErr) } } + +func TestLoadFileLowercasesSectionNames(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config") + _ = os.WriteFile(path, []byte(`[Git.Example.com] +type = gitea +scheme = http +token = tok +git_protocol = ssh +`), 0600) + + cfg := &Config{Domains: make(map[string]DomainSection)} + if err := loadFile(cfg, path, true); err != nil { + t.Fatal(err) + } + + if _, ok := cfg.Domains["Git.Example.com"]; ok { + t.Error("section name kept its original case") + } + ds, ok := cfg.Domains["git.example.com"] + if !ok { + t.Fatal("expected a git.example.com section") + } + if ds.Type != "gitea" || ds.Scheme != "http" || ds.Token != "tok" || ds.GitProtocol != "ssh" { + t.Errorf("unexpected section: %+v", ds) + } +} + +func TestDomainForSSHHostIgnoresCase(t *testing.T) { + cfg := &Config{ + Domains: map[string]DomainSection{ + "gitlab.test": {SSHHost: "SSH.GitLab.test"}, + }, + } + if got := cfg.DomainForSSHHost("ssh.gitlab.test"); got != "gitlab.test" { + t.Errorf("DomainForSSHHost = %q, want gitlab.test", got) + } +} + +func TestSetDomainLowercasesAndMergesSection(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + + cfgDir := filepath.Join(dir, "forge") + _ = os.MkdirAll(cfgDir, 0700) + _ = os.WriteFile(filepath.Join(cfgDir, "config"), []byte(`[Git.Example.com] +ssh_host = ssh.example.com +`), 0600) + + if err := SetDomain("GIT.example.com", "tok", "", "gitea", ""); err != nil { + t.Fatalf("SetDomain: %v", err) + } + + f, err := os.Open(filepath.Join(cfgDir, "config")) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + sections, err := parseINI(f) + if err != nil { + t.Fatal(err) + } + if len(sections) != 1 { + t.Fatalf("expected one section, got %v", sections) + } + got := sections["git.example.com"] + if got["token"] != "tok" || got["type"] != "gitea" || got["ssh_host"] != "ssh.example.com" { + t.Errorf("unexpected section: %v", got) + } +} diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 2b1c502..c43183a 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -479,7 +479,7 @@ func configForgeType(domain string) string { if err != nil || cfg == nil { return "" } - return cfg.Domains[domain].Type + return cfg.Domains[strings.ToLower(domain)].Type } // configScheme returns the API scheme for a domain from config files, @@ -489,7 +489,7 @@ func configScheme(domain string) string { if err != nil || cfg == nil { return "" } - return cfg.Domains[domain].Scheme + return cfg.Domains[strings.ToLower(domain)].Scheme } // TokenForDomain looks up an auth token. Checks environment variables first @@ -503,7 +503,7 @@ func TokenForDomain(domain string) string { if err != nil || cfg == nil { return "" } - token, err := cfg.Domains[domain].ResolveToken(domain) + token, err := cfg.Domains[strings.ToLower(domain)].ResolveToken(domain) if err != nil { fmt.Fprintf(os.Stderr, "forge: token command for %s failed: %v\n", domain, err) } diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index 9651019..2302e6e 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -1130,3 +1130,41 @@ func TestDomainRadicleHasNoDefault(t *testing.T) { t.Errorf("Domain(radicle) = %q, want empty", got) } } + +func TestMixedCaseRemoteMatchesMixedCaseConfigSection(t *testing.T) { + clearTokenEnv(t) + config.ResetCache() + defer config.ResetCache() + + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + cfgDir := filepath.Join(dir, "forge") + _ = os.MkdirAll(cfgDir, 0700) + _ = os.WriteFile(filepath.Join(cfgDir, "config"), []byte(`[Git.Example.com] +type = gitea +scheme = http +token = secret +ssh_host = SSH.Example.com +`), 0600) + + domain, _, _, err := forges.ParseRepoURL("git@Git.Example.com:owner/repo") + if err != nil { + t.Fatal(err) + } + if got := TokenForDomain(domain); got != "secret" { + t.Errorf("TokenForDomain(%q) = %q, want secret", domain, got) + } + if got := configForgeType(domain); got != "gitea" { + t.Errorf("configForgeType(%q) = %q, want gitea", domain, got) + } + if got := configScheme(domain); got != "http" { + t.Errorf("configScheme(%q) = %q, want http", domain, got) + } + if got := mapSSHHost("ssh.example.com"); got != "git.example.com" { + t.Errorf("mapSSHHost = %q, want git.example.com", got) + } + // A mixed-case --host style value resolves the same section. + if got := TokenForDomain("GIT.example.com"); got != "secret" { + t.Errorf("TokenForDomain mixed case = %q, want secret", got) + } +} From 5bc3f867d701e5165c4e5eb07979657c247de7eb Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Thu, 12 Nov 2026 11:44:26 +0530 Subject: [PATCH 6/7] Keep a registration made while first-use detection is running Detection builds the forge outside the client lock, so an explicit RegisterDomain for the same domain could land in between and then be overwritten by the detected https forge. Detection now only registers the domain if it is still unregistered when it takes the lock. RegisterDomain still replaces an existing registration. --- factory/factory_test.go | 36 ++++++++++++++++++++++++++++++++++++ forge.go | 11 ++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/factory/factory_test.go b/factory/factory_test.go index c49539f..9d97427 100644 --- a/factory/factory_test.go +++ b/factory/factory_test.go @@ -245,6 +245,42 @@ func TestForgeForURLDetectsConcurrently(t *testing.T) { wg.Wait() } +func TestForgeForURLKeepsRegistrationMadeDuringDetection(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("X-Gitea-Version", "1.22.0") + })) + defer srv.Close() + host := strings.TrimPrefix(srv.URL, "https://") + + explicit := &stubForge{} + var c *forges.Client + detecting := Builders() + detecting.Gitea = func(string, string, *http.Client) forges.Forge { + // An explicit registration lands while detection is still building. + stub := func(string, string, *http.Client) forges.Forge { return explicit } + err := c.RegisterDomain(context.Background(), srv.URL, "explicit", forges.ForgeBuilders{Gitea: stub}) + if err != nil { + t.Errorf("RegisterDomain: %v", err) + } + return &stubForge{} + } + c = forges.NewClient(forges.WithHTTPClient(srv.Client()), forges.WithDetection(detecting)) + + if _, _, _, err := c.ForgeForURL(context.Background(), srv.URL+"/owner/repo"); err != nil { + t.Fatalf("ForgeForURL: %v", err) + } + got, err := c.ForgeFor(host) + if err != nil { + t.Fatalf("ForgeFor: %v", err) + } + if got != forges.Forge(explicit) { + t.Error("detection replaced a registration made while it was running") + } + if tok := c.Tokens()[host]; tok != "explicit" { + t.Errorf("token = %q, want the explicit registration's", tok) + } +} + func TestForgeForURLDetectsHTTPURLOverHTTPS(t *testing.T) { rt := &giteaTransport{} c := NewClient( diff --git a/forge.go b/forge.go index d67a526..c431b20 100644 --- a/forge.go +++ b/forge.go @@ -200,6 +200,12 @@ func normalizeBaseURL(s string) (baseURL, host string, err error) { // include an http:// or https:// prefix; without one, https is assumed. // The lowercased bare host[:port] is used as the registry key. func (c *Client) RegisterDomain(ctx context.Context, domain, token string, builders ForgeBuilders) error { + return c.registerDomain(ctx, domain, token, builders, false) +} + +// registerDomain implements RegisterDomain. With keepExisting set, a domain +// that was registered while detection was running is left as it is. +func (c *Client) registerDomain(ctx context.Context, domain, token string, builders ForgeBuilders, keepExisting bool) error { baseURL, domain, err := normalizeBaseURL(domain) if err != nil { return err @@ -232,6 +238,9 @@ func (c *Client) RegisterDomain(ctx context.Context, domain, token string, build f := build(baseURL, token, c.httpClient) c.mu.Lock() defer c.mu.Unlock() + if _, ok := c.forges[domain]; ok && keepExisting { + return nil + } c.tokens[domain] = token c.forges[domain] = f return nil @@ -270,7 +279,7 @@ func (c *Client) forgeOrDetect(ctx context.Context, domain string) (Forge, error c.mu.RLock() token := c.tokens[domain] c.mu.RUnlock() - if err := c.RegisterDomain(ctx, "https://"+domain, token, *c.detection); err != nil { + if err := c.registerDomain(ctx, "https://"+domain, token, *c.detection, true); err != nil { return nil, err } return c.forgeFor(domain) From 74a2d1901505da8fc8e3dc43729a433adcd4fc3a Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:54:15 +0530 Subject: [PATCH 7/7] Normalize detection lookups and merge case-variant config sections in order forgeOrDetect now uses the same host key as registerDomain, so a domain with a trailing slash finds its registration and doesn't trigger a probe. Config sections that differ only in case are applied in sorted order with the lowercase section last, in loadFile and SetDomain, so the result no longer depends on map iteration. --- forge.go | 7 ++++ forges_test.go | 23 +++++++++++ internal/config/config.go | 53 +++++++++++++++++++----- internal/config/config_test.go | 74 ++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 11 deletions(-) diff --git a/forge.go b/forge.go index 38c671f..32127ef 100644 --- a/forge.go +++ b/forge.go @@ -326,6 +326,13 @@ func (c *Client) forgeFor(domain string) (Forge, error) { // finds. func (c *Client) forgeOrDetect(ctx context.Context, domain string) (Forge, error) { domain = strings.ToLower(domain) + if !strings.Contains(domain, "://") { + // Use the key registerDomain writes, so a trailing slash or similar + // doesn't miss a registered host. + if _, host, err := normalizeBaseURL(domain); err == nil { + domain = host + } + } f, err := c.forgeFor(domain) if err == nil || c.detection == nil || strings.Contains(domain, "://") { return f, err diff --git a/forges_test.go b/forges_test.go index ea3e239..898fa78 100644 --- a/forges_test.go +++ b/forges_test.go @@ -896,6 +896,29 @@ func TestClientMatchesDomainsCaseInsensitively(t *testing.T) { } } +func TestListRepositoriesTrailingSlashKeepsRegistration(t *testing.T) { + mock := &mockForge{repoService: &mockRepoService{repos: []Repository{{FullName: "org/repo"}}}} + rt := &detectionTransport{} + c := NewClient( + WithHTTPClient(&http.Client{Transport: rt}), + WithForge("git.example.com", mock), + WithToken("git.example.com", "tok"), + WithDetection(ForgeBuilders{Gitea: func(string, string, *http.Client) Forge { return &mockForge{} }}), + ) + + repos, err := c.ListRepositories(context.Background(), "Git.Example.com/", "org", ListRepoOpts{}) + if err != nil || len(repos) != 1 { + t.Fatalf("ListRepositories = %+v, %v", repos, err) + } + if f, err := c.ForgeFor("git.example.com"); err != nil || f != Forge(mock) { + t.Errorf("ForgeFor = %T, %v, want the registered forge", f, err) + } + assertEqual(t, "token", "tok", c.Tokens()["git.example.com"]) + if len(rt.urls) != 0 { + t.Errorf("probed %v, want no requests", rt.urls) + } +} + func TestClientListRepositoriesRoutes(t *testing.T) { mock := &mockForge{ repoService: &mockRepoService{ diff --git a/internal/config/config.go b/internal/config/config.go index 9cbf93d..05bb630 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "runtime" + "slices" "strings" "sync" ) @@ -214,12 +215,13 @@ func loadFile(cfg *Config, path string, userConfig bool) error { } } - for name, kv := range sections { - if name == "default" { + for _, rawName := range sectionOrder(sections) { + if rawName == "default" { continue } + kv := sections[rawName] // Domains are matched case-insensitively, so key them by lowercase. - name = strings.ToLower(name) + name := strings.ToLower(rawName) ds := cfg.Domains[name] if v, ok := kv["type"]; ok { ds.Type = v @@ -267,6 +269,31 @@ func loadFile(cfg *Config, path string, userConfig bool) error { return nil } +// sectionOrder returns the section names in the order they should be applied +// when case variants of one domain are merged: sorted, except that a name that +// is already lowercase comes after the other spellings of the same domain, so +// it wins conflicts. The order does not depend on map iteration. +func sectionOrder(sections map[string]map[string]string) []string { + names := make([]string, 0, len(sections)) + for name := range sections { + names = append(names, name) + } + slices.SortFunc(names, func(a, b string) int { + if c := strings.Compare(strings.ToLower(a), strings.ToLower(b)); c != 0 { + return c + } + aLower, bLower := a == strings.ToLower(a), b == strings.ToLower(b) + switch { + case aLower && !bLower: + return 1 + case bLower && !aLower: + return -1 + } + return strings.Compare(a, b) + }) + return names +} + // parseINI parses a simple INI file into section -> key -> value. // Supports # and ; comments, blank lines, and [section] headers. func parseINI(r io.Reader) (map[string]map[string]string, error) { @@ -379,20 +406,24 @@ func SetDomain(domain, token, tokenCmd, forgeType, scheme string) error { domain = strings.ToLower(domain) // Fold any existing section spelled with different case into the // lowercase one so the file never holds two sections for one domain. - for name, kv := range sections { - if name == domain || !strings.EqualFold(name, domain) { + // Sections are applied in sectionOrder, so the exact-lowercase section + // wins any conflict. + var folded map[string]string + for _, name := range sectionOrder(sections) { + if !strings.EqualFold(name, domain) { continue } - if _, ok := sections[domain]; !ok { - sections[domain] = make(map[string]string) + if folded == nil { + folded = make(map[string]string) } - for k, v := range kv { - if _, ok := sections[domain][k]; !ok { - sections[domain][k] = v - } + for k, v := range sections[name] { + folded[k] = v } delete(sections, name) } + if folded != nil { + sections[domain] = folded + } if _, ok := sections[domain]; !ok { sections[domain] = make(map[string]string) } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 68b9499..3ce86d5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -850,3 +850,77 @@ ssh_host = ssh.example.com t.Errorf("unexpected section: %v", got) } } + +func TestLoadFileCaseVariantSectionsLowercaseWins(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config") + _ = os.WriteFile(path, []byte(`[git.example.com] +token = lower +[Git.Example.com] +token = mixed +type = gitea +scheme = http +[GIT.EXAMPLE.COM] +token = upper +scheme = https +git_protocol = ssh +`), 0600) + + // Map iteration order is random, so repeat to catch order dependence. + for range 20 { + cfg := &Config{Domains: make(map[string]DomainSection)} + if err := loadFile(cfg, path, true); err != nil { + t.Fatal(err) + } + ds := cfg.Domains["git.example.com"] + if ds.Token != "lower" { + t.Fatalf("token = %q, want the lowercase section's", ds.Token) + } + // Settings the lowercase section doesn't set come from the other + // spellings in sorted order, so "GIT.EXAMPLE.COM" sorts before "Git.Example.com". + if ds.Type != "gitea" || ds.Scheme != "http" || ds.GitProtocol != "ssh" { + t.Fatalf("unexpected section: %+v", ds) + } + } +} + +func TestSetDomainCaseVariantSectionsLowercaseWins(t *testing.T) { + for range 20 { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + cfgDir := filepath.Join(dir, "forge") + _ = os.MkdirAll(cfgDir, 0700) + _ = os.WriteFile(filepath.Join(cfgDir, "config"), []byte(`[Git.Example.com] +token = mixed +ssh_host = mixed.example.com +type = forgejo +[GIT.EXAMPLE.COM] +token = upper +ssh_host = upper.example.com +[git.example.com] +token = lower +ssh_host = lower.example.com +`), 0600) + + if err := SetDomain("git.example.com", "", "", "gitea", ""); err != nil { + t.Fatalf("SetDomain: %v", err) + } + + f, err := os.Open(filepath.Join(cfgDir, "config")) + if err != nil { + t.Fatal(err) + } + sections, err := parseINI(f) + _ = f.Close() + if err != nil { + t.Fatal(err) + } + if len(sections) != 1 { + t.Fatalf("expected one section, got %v", sections) + } + got := sections["git.example.com"] + if got["token"] != "lower" || got["ssh_host"] != "lower.example.com" || got["type"] != "gitea" { + t.Fatalf("unexpected section: %v", got) + } + } +}