fix: match GitLab MR against repository remote_url identity (#1996)
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -36,8 +37,7 @@ func parseMRURLForHost(rawURL, configuredHost string) (string, int, error) {
|
||||
if err != nil {
|
||||
return "", 0, ErrInvalidMRURL
|
||||
}
|
||||
expectedOrigin, err := normalizeHostOrigin(configuredHost)
|
||||
if err != nil || !strings.EqualFold(origin, expectedOrigin) {
|
||||
if !sameConfiguredOrigin(origin, configuredHost) {
|
||||
return "", 0, fmt.Errorf("%w: host does not match workspace connection", ErrInvalidMRURL)
|
||||
}
|
||||
const marker = "/-/merge_requests/"
|
||||
@@ -58,6 +58,103 @@ func parseMRURLForHost(rawURL, configuredHost string) (string, int, error) {
|
||||
return projectPath, iid, nil
|
||||
}
|
||||
|
||||
// parseGitLabRemoteURLIdentity extracts a normalized (host origin, project
|
||||
// path) pair from a git remote URL, accepting HTTPS, ssh://, and scp-style
|
||||
// (git@host:path) forms. SSH remotes are normalized to an HTTPS-style origin
|
||||
// so they can be host-compared against a configured GitLab connection host,
|
||||
// which is always stored as an HTTP(S) origin. Returns empty strings when the
|
||||
// URL cannot be parsed as an owner/repo remote.
|
||||
func parseGitLabRemoteURLIdentity(remoteURL string) (host, projectPath string) {
|
||||
const sshScheme = "ssh"
|
||||
remoteURL = strings.TrimSpace(remoteURL)
|
||||
if remoteURL == "" {
|
||||
return "", ""
|
||||
}
|
||||
scheme, hostname, path, ok := splitRemoteURLIdentity(remoteURL)
|
||||
if !ok {
|
||||
return "", ""
|
||||
}
|
||||
scheme = strings.ToLower(scheme)
|
||||
if scheme != mentionHTTPScheme && scheme != mentionHTTPSScheme && scheme != sshScheme {
|
||||
return "", ""
|
||||
}
|
||||
if scheme == sshScheme {
|
||||
scheme = mentionHTTPSScheme
|
||||
}
|
||||
path = strings.TrimSuffix(strings.Trim(strings.TrimSpace(path), "/"), ".git")
|
||||
if hostname == "" || path == "" || !strings.Contains(path, "/") {
|
||||
return "", ""
|
||||
}
|
||||
return scheme + "://" + hostname, path
|
||||
}
|
||||
|
||||
// splitRemoteURLIdentity resolves the (scheme, hostname, path) triple for
|
||||
// either a scp-style shorthand (git@host:path) or a URL-form remote
|
||||
// (https://, ssh://, etc). Extracted from parseGitLabRemoteURLIdentity to
|
||||
// keep its cyclomatic complexity within the linter budget.
|
||||
func splitRemoteURLIdentity(remoteURL string) (scheme, hostname, path string, ok bool) {
|
||||
const sshScheme = "ssh"
|
||||
if strings.Contains(remoteURL, "@") && !strings.Contains(remoteURL, "://") {
|
||||
// scp-style shorthand: git@host:group/sub/project.git (host may be a
|
||||
// bracketed IPv6 literal, e.g. git@[::1]:group/project.git, whose
|
||||
// embedded colons must not be mistaken for the host/path separator).
|
||||
_, rest, _ := strings.Cut(remoteURL, "@")
|
||||
h, p, cutOK := cutSCPHostPath(rest)
|
||||
if !cutOK {
|
||||
return "", "", "", false
|
||||
}
|
||||
return sshScheme, h, p, true
|
||||
}
|
||||
parsed, err := url.Parse(remoteURL)
|
||||
if err != nil || parsed.Scheme == "" || parsed.Host == "" {
|
||||
return "", "", "", false
|
||||
}
|
||||
hostname = remoteHost(parsed)
|
||||
if strings.EqualFold(parsed.Scheme, sshScheme) {
|
||||
// SSH transport ports (e.g. ssh://git@host:2222/...) are unrelated to
|
||||
// the GitLab web origin's port, so compare hostname only. Bracket
|
||||
// IPv6 literals so the synthesized origin stays a valid URL (an
|
||||
// unbracketed "https://::1" is not).
|
||||
hostname = bracketedHostname(parsed.Hostname())
|
||||
}
|
||||
return parsed.Scheme, hostname, parsed.Path, true
|
||||
}
|
||||
|
||||
// cutSCPHostPath splits an scp-style remote's "host:path" segment (the part
|
||||
// after "git@"), honoring bracketed IPv6 literals such as
|
||||
// "[::1]:group/project.git" whose embedded colons would otherwise be
|
||||
// mistaken for the host/path separator by a plain strings.Cut on ":".
|
||||
func cutSCPHostPath(rest string) (host, path string, ok bool) {
|
||||
if strings.HasPrefix(rest, "[") {
|
||||
closeIdx := strings.Index(rest, "]")
|
||||
if closeIdx < 0 || closeIdx+1 >= len(rest) || rest[closeIdx+1] != ':' {
|
||||
return "", "", false
|
||||
}
|
||||
return rest[:closeIdx+1], rest[closeIdx+2:], true
|
||||
}
|
||||
return strings.Cut(rest, ":")
|
||||
}
|
||||
|
||||
func remoteHost(parsed *url.URL) string {
|
||||
hostname := parsed.Hostname()
|
||||
if hostname == "" {
|
||||
return ""
|
||||
}
|
||||
if port := parsed.Port(); port != "" {
|
||||
return net.JoinHostPort(hostname, port)
|
||||
}
|
||||
return bracketedHostname(hostname)
|
||||
}
|
||||
|
||||
// bracketedHostname wraps an IPv6 literal in brackets (as required for a
|
||||
// valid URL host) and returns other hostnames unchanged.
|
||||
func bracketedHostname(hostname string) string {
|
||||
if hostname == "" || !strings.Contains(hostname, ":") {
|
||||
return hostname
|
||||
}
|
||||
return "[" + hostname + "]"
|
||||
}
|
||||
|
||||
// AssociateExistingMRByURL validates a workspace-owned task/repository pair,
|
||||
// fetches the configured-host MR, and idempotently persists its association.
|
||||
func (s *Service) AssociateExistingMRByURL(
|
||||
|
||||
@@ -15,10 +15,11 @@ func seedTaskMRLinkFixture(t *testing.T, store *Store, workspaceID, taskID, repo
|
||||
if _, err := store.db.Exec(`CREATE TABLE IF NOT EXISTS repositories (
|
||||
id TEXT PRIMARY KEY,
|
||||
workspace_id TEXT NOT NULL,
|
||||
provider TEXT NOT NULL DEFAULT '',
|
||||
provider_host TEXT NOT NULL DEFAULT '',
|
||||
provider_owner TEXT NOT NULL DEFAULT '',
|
||||
provider_name TEXT NOT NULL DEFAULT ''
|
||||
provider TEXT DEFAULT '',
|
||||
provider_host TEXT DEFAULT '',
|
||||
provider_owner TEXT DEFAULT '',
|
||||
provider_name TEXT DEFAULT '',
|
||||
remote_url TEXT DEFAULT ''
|
||||
); CREATE TABLE IF NOT EXISTS task_repositories (
|
||||
id TEXT PRIMARY KEY,
|
||||
task_id TEXT NOT NULL,
|
||||
@@ -57,6 +58,30 @@ func setTaskMRRepositoryIdentity(
|
||||
}
|
||||
}
|
||||
|
||||
func setTaskMRRepositoryRemoteURL(t *testing.T, store *Store, repositoryID, remoteURL string) {
|
||||
t.Helper()
|
||||
if _, err := store.db.Exec(
|
||||
`UPDATE repositories SET remote_url = ? WHERE id = ?`, remoteURL, repositoryID,
|
||||
); err != nil {
|
||||
t.Fatalf("set repository remote_url: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// setTaskMRRepositoryIdentityColumnsNull sets every provider identity column
|
||||
// (provider, provider_host, provider_owner, provider_name, remote_url) to
|
||||
// SQL NULL, mirroring rows persisted before those columns existed or by code
|
||||
// paths that never populated them. The production schema declares them
|
||||
// nullable, so ValidateTaskMRRepositoryIdentity must tolerate NULL scans.
|
||||
func setTaskMRRepositoryIdentityColumnsNull(t *testing.T, store *Store, repositoryID string) {
|
||||
t.Helper()
|
||||
if _, err := store.db.Exec(`UPDATE repositories
|
||||
SET provider = NULL, provider_host = NULL, provider_owner = NULL,
|
||||
provider_name = NULL, remote_url = NULL
|
||||
WHERE id = ?`, repositoryID); err != nil {
|
||||
t.Fatalf("null repository identity columns: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func newTaskMRLinkService(t *testing.T, host string) (*Service, *Store, *MockClient) {
|
||||
t.Helper()
|
||||
store := newTestStore(t)
|
||||
@@ -103,6 +128,28 @@ func TestAssociateExistingMRByURLCreatesIdempotentWorkspaceScopedLink(t *testing
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLAcceptsMRURLWithExplicitDefaultPort(t *testing.T) {
|
||||
// The workspace's configured GitLab host and the incoming MR URL should
|
||||
// match even when only one side spells out the scheme's implicit default
|
||||
// port (":443" for https), since they identify the same web origin.
|
||||
const configuredHost = "https://gitlab.acme.test:443"
|
||||
const mrURLHost = "https://gitlab.acme.test"
|
||||
svc, store, client := newTaskMRLinkService(t, configuredHost)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryIdentity(t, store, "repo-1", configuredHost, "acme/api")
|
||||
client.SeedMR("acme/api", &MR{
|
||||
IID: 21, Title: "MR", WebURL: mrURLHost + "/acme/api/-/merge_requests/21",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
if _, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
mrURLHost+"/acme/api/-/merge_requests/21",
|
||||
); err != nil {
|
||||
t.Fatalf("AssociateExistingMRByURL: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLRejectsWrongHostAndCrossWorkspaceResources(t *testing.T) {
|
||||
const host = "http://gitlab.internal.test"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
@@ -151,7 +198,6 @@ func TestAssociateExistingMRByURLRejectsRepositoryIdentityMismatch(t *testing.T)
|
||||
repositoryHost string
|
||||
repositoryPath string
|
||||
}{
|
||||
{name: "same hostname with HTTPS origin", repositoryHost: "https://gitlab.internal.test:8080", repositoryPath: "group/subgroup/project"},
|
||||
{name: "different GitLab host", repositoryHost: "http://other.internal.test:8080", repositoryPath: "group/subgroup/project"},
|
||||
{name: "different subgroup project", repositoryHost: host, repositoryPath: "group/other/project"},
|
||||
{name: "unknown legacy host", repositoryHost: "", repositoryPath: "group/subgroup/project"},
|
||||
@@ -195,6 +241,197 @@ func TestAssociateExistingMRByURLAcceptsExactSelfManagedHTTPSRepositoryIdentity(
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLAcceptsRepositoryIdentityWithExplicitDefaultPort(t *testing.T) {
|
||||
const host = "https://gitlab.internal.test"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
// An explicit default HTTPS port (:443) must compare equal to the
|
||||
// workspace host, which omits it.
|
||||
setTaskMRRepositoryIdentity(t, store, "repo-1", "https://gitlab.internal.test:443", "group/subgroup/project")
|
||||
client.SeedMR("group/subgroup/project", &MR{
|
||||
IID: 13, Title: "MR", WebURL: host + "/group/subgroup/project/-/merge_requests/13",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
if _, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/group/subgroup/project/-/merge_requests/13",
|
||||
); err != nil {
|
||||
t.Fatalf("AssociateExistingMRByURL: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLAcceptsScpStyleRemoteWithBracketedIPv6Host(t *testing.T) {
|
||||
const host = "https://[::1]"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryRemoteURL(t, store, "repo-1", "git@[::1]:clients/socodevi/laravel/co-up.git")
|
||||
client.SeedMR("clients/socodevi/laravel/co-up", &MR{
|
||||
IID: 92, Title: "MR", WebURL: host + "/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
if _, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
); err != nil {
|
||||
t.Fatalf("AssociateExistingMRByURL: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLAcceptsSSHURLRemoteWithIPv6Host(t *testing.T) {
|
||||
const host = "https://[::1]"
|
||||
tests := []struct {
|
||||
name string
|
||||
remoteURL string
|
||||
}{
|
||||
{name: "no port", remoteURL: "ssh://git@[::1]/clients/socodevi/laravel/co-up.git"},
|
||||
{name: "explicit non-web port", remoteURL: "ssh://git@[::1]:2222/clients/socodevi/laravel/co-up.git"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryRemoteURL(t, store, "repo-1", tt.remoteURL)
|
||||
client.SeedMR("clients/socodevi/laravel/co-up", &MR{
|
||||
IID: 92, Title: "MR", WebURL: host + "/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
if _, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
); err != nil {
|
||||
t.Fatalf("AssociateExistingMRByURL: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLAcceptsRepositoryIdentityWithSameHostDifferentScheme(t *testing.T) {
|
||||
const host = "http://gitlab.internal.test:8080"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryIdentity(t, store, "repo-1", "https://gitlab.internal.test:8080", "group/subgroup/project")
|
||||
client.SeedMR("group/subgroup/project", &MR{
|
||||
IID: 12, Title: "MR", WebURL: host + "/group/subgroup/project/-/merge_requests/12",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
if _, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/group/subgroup/project/-/merge_requests/12",
|
||||
); err != nil {
|
||||
t.Fatalf("AssociateExistingMRByURL: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLAcceptsRemoteURLWhenDurableIdentityEmpty(t *testing.T) {
|
||||
// Reproduces the reported bug: a self-hosted GitLab repository added as a
|
||||
// local clone never gets a durable provider identity (only github.com and
|
||||
// gitlab.com are tagged at discovery time), but its remote_url still
|
||||
// clearly identifies the same GitLab host and nested-subgroup project.
|
||||
const host = "https://gitlab.savoirfairelinux.com"
|
||||
tests := []struct {
|
||||
name string
|
||||
remoteURL string
|
||||
}{
|
||||
{name: "https remote", remoteURL: host + "/clients/socodevi/laravel/co-up.git"},
|
||||
{name: "https remote without .git suffix", remoteURL: host + "/clients/socodevi/laravel/co-up"},
|
||||
{name: "https remote with trailing slash", remoteURL: host + "/clients/socodevi/laravel/co-up/"},
|
||||
{name: "https remote with different case", remoteURL: "https://GitLab.SavoirFaireLinux.com/Clients/Socodevi/Laravel/Co-Up.git"},
|
||||
{name: "http remote for https workspace host", remoteURL: "http://gitlab.savoirfairelinux.com/clients/socodevi/laravel/co-up.git"},
|
||||
{name: "ssh url remote", remoteURL: "ssh://git@gitlab.savoirfairelinux.com/clients/socodevi/laravel/co-up.git"},
|
||||
{name: "scp-style remote", remoteURL: "git@gitlab.savoirfairelinux.com:clients/socodevi/laravel/co-up.git"},
|
||||
// SSH transport ports (used by hosts that put SSH on a non-standard
|
||||
// port) are unrelated to the GitLab web origin's port and must not
|
||||
// be compared against it.
|
||||
{name: "ssh url remote with explicit non-web port", remoteURL: "ssh://git@gitlab.savoirfairelinux.com:2222/clients/socodevi/laravel/co-up.git"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryRemoteURL(t, store, "repo-1", tt.remoteURL)
|
||||
client.SeedMR("clients/socodevi/laravel/co-up", &MR{
|
||||
IID: 92, Title: "MR", WebURL: host + "/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
if _, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
); err != nil {
|
||||
t.Fatalf("AssociateExistingMRByURL: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLRejectsRemoteURLPointingElsewhere(t *testing.T) {
|
||||
const host = "https://gitlab.savoirfairelinux.com"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
// Durable identity empty (legacy/unresolved row) and remote_url points to
|
||||
// a different project than the MR being linked.
|
||||
setTaskMRRepositoryRemoteURL(t, store, "repo-1", host+"/clients/socodevi/laravel/other-project.git")
|
||||
client.SeedMR("clients/socodevi/laravel/co-up", &MR{
|
||||
IID: 92, Title: "MR", WebURL: host + "/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
_, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
)
|
||||
if !errors.Is(err, ErrTaskMRRepositoryMismatch) {
|
||||
t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLRejectsRemoteURLOnDifferentHost(t *testing.T) {
|
||||
const host = "https://gitlab.savoirfairelinux.com"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryRemoteURL(t, store, "repo-1", "https://gitlab.other.test/clients/socodevi/laravel/co-up.git")
|
||||
client.SeedMR("clients/socodevi/laravel/co-up", &MR{
|
||||
IID: 92, Title: "MR", WebURL: host + "/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
_, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
)
|
||||
if !errors.Is(err, ErrTaskMRRepositoryMismatch) {
|
||||
t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLRejectsRepositoryWithNullIdentityColumns(t *testing.T) {
|
||||
// The production schema declares provider/provider_host/provider_owner/
|
||||
// provider_name/remote_url as nullable columns, so rows can legitimately
|
||||
// have SQL NULL there. ValidateTaskMRRepositoryIdentity must coalesce
|
||||
// those to empty strings and fail closed with the normal mismatch error
|
||||
// instead of an internal scan error.
|
||||
const host = "https://gitlab.savoirfairelinux.com"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1")
|
||||
setTaskMRRepositoryIdentityColumnsNull(t, store, "repo-1")
|
||||
client.SeedMR("clients/socodevi/laravel/co-up", &MR{
|
||||
IID: 92, Title: "MR", WebURL: host + "/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
State: "opened", CreatedAt: time.Now().UTC(),
|
||||
})
|
||||
|
||||
_, err := svc.AssociateExistingMRByURL(
|
||||
context.Background(), "ws-1", "task-1", "repo-1",
|
||||
host+"/clients/socodevi/laravel/co-up/-/merge_requests/92",
|
||||
)
|
||||
if !errors.Is(err, ErrTaskMRRepositoryMismatch) {
|
||||
t.Fatalf("error = %v, want ErrTaskMRRepositoryMismatch", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssociateExistingMRByURLInfersSoleRepositoryAndRejectsMultiRepoAmbiguity(t *testing.T) {
|
||||
const host = "https://gitlab.acme.test"
|
||||
svc, store, client := newTaskMRLinkService(t, host)
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@@ -40,9 +41,13 @@ func (s *Store) ValidateTaskMRScope(ctx context.Context, workspaceID, taskID, re
|
||||
return nil
|
||||
}
|
||||
|
||||
// ValidateTaskMRRepositoryIdentity verifies the durable provider identity for
|
||||
// the repository selected by the task. Legacy rows without a provider host
|
||||
// fail closed because owner/name alone cannot distinguish GitLab instances.
|
||||
// ValidateTaskMRRepositoryIdentity verifies the repository selected by the
|
||||
// task identifies the same GitLab host and project as the MR. It accepts a
|
||||
// match against either the repository's durable provider identity or its
|
||||
// remote_url, since self-hosted GitLab repositories added as local clones
|
||||
// never get a durable provider identity (only github.com/gitlab.com are
|
||||
// tagged at discovery time) yet still carry a resolvable remote_url. Rows
|
||||
// with neither signal resolving to a GitLab identity fail closed.
|
||||
func (s *Store) ValidateTaskMRRepositoryIdentity(
|
||||
ctx context.Context,
|
||||
workspaceID, taskID, repositoryID, configuredHost, projectPath string,
|
||||
@@ -51,13 +56,18 @@ func (s *Store) ValidateTaskMRRepositoryIdentity(
|
||||
return ErrTaskMRRepositoryMismatch
|
||||
}
|
||||
var identity struct {
|
||||
Provider string `db:"provider"`
|
||||
Host string `db:"provider_host"`
|
||||
Owner string `db:"provider_owner"`
|
||||
Name string `db:"provider_name"`
|
||||
Provider string `db:"provider"`
|
||||
Host string `db:"provider_host"`
|
||||
Owner string `db:"provider_owner"`
|
||||
Name string `db:"provider_name"`
|
||||
RemoteURL string `db:"remote_url"`
|
||||
}
|
||||
err := s.ro.GetContext(ctx, &identity, `
|
||||
SELECT r.provider, r.provider_host, r.provider_owner, r.provider_name
|
||||
SELECT COALESCE(r.provider, '') AS provider,
|
||||
COALESCE(r.provider_host, '') AS provider_host,
|
||||
COALESCE(r.provider_owner, '') AS provider_owner,
|
||||
COALESCE(r.provider_name, '') AS provider_name,
|
||||
COALESCE(r.remote_url, '') AS remote_url
|
||||
FROM task_repositories tr
|
||||
JOIN repositories r ON r.id = tr.repository_id
|
||||
JOIN tasks t ON t.id = tr.task_id
|
||||
@@ -74,18 +84,67 @@ func (s *Store) ValidateTaskMRRepositoryIdentity(
|
||||
if err != nil {
|
||||
return ErrTaskMRRepositoryMismatch
|
||||
}
|
||||
gotHost, err := normalizeHostOrigin(identity.Host)
|
||||
if err != nil {
|
||||
return ErrTaskMRRepositoryMismatch
|
||||
}
|
||||
gotProject := strings.Trim(strings.TrimSpace(identity.Owner+"/"+identity.Name), "/")
|
||||
wantProject := strings.Trim(strings.TrimSpace(projectPath), "/")
|
||||
if !strings.EqualFold(identity.Provider, "gitlab") ||
|
||||
!strings.EqualFold(gotHost, wantHost) ||
|
||||
!strings.EqualFold(gotProject, wantProject) {
|
||||
return ErrTaskMRRepositoryMismatch
|
||||
|
||||
type candidate struct{ host, project string }
|
||||
var candidates []candidate
|
||||
if strings.EqualFold(identity.Provider, "gitlab") {
|
||||
if gotHost, err := normalizeHostOrigin(identity.Host); err == nil {
|
||||
gotProject := strings.Trim(strings.TrimSpace(identity.Owner+"/"+identity.Name), "/")
|
||||
candidates = append(candidates, candidate{host: gotHost, project: gotProject})
|
||||
}
|
||||
}
|
||||
if remoteURLHost, remoteProject := parseGitLabRemoteURLIdentity(identity.RemoteURL); remoteURLHost != "" {
|
||||
if gotHost, err := normalizeHostOrigin(remoteURLHost); err == nil {
|
||||
candidates = append(candidates, candidate{host: gotHost, project: remoteProject})
|
||||
}
|
||||
}
|
||||
for _, c := range candidates {
|
||||
if sameGitLabHost(c.host, wantHost) && strings.EqualFold(c.project, wantProject) {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
return ErrTaskMRRepositoryMismatch
|
||||
}
|
||||
|
||||
func sameGitLabHost(left, right string) bool {
|
||||
leftURL, leftErr := validateHost(strings.TrimSpace(left))
|
||||
rightURL, rightErr := validateHost(strings.TrimSpace(right))
|
||||
if leftErr != nil || rightErr != nil {
|
||||
return false
|
||||
}
|
||||
return strings.EqualFold(hostWithoutDefaultPort(leftURL), hostWithoutDefaultPort(rightURL))
|
||||
}
|
||||
|
||||
// sameConfiguredOrigin reports whether candidate and configured identify the
|
||||
// same GitLab web origin: same scheme, and hosts equal once each side's
|
||||
// implicit default port is stripped. Used to validate an incoming MR URL's
|
||||
// origin against the workspace's configured GitLab host, so an explicit
|
||||
// "https://gitlab.example.com:443" on either side still matches the
|
||||
// equivalent unported form.
|
||||
func sameConfiguredOrigin(candidate, configured string) bool {
|
||||
candidateURL, candidateErr := validateHost(strings.TrimSpace(candidate))
|
||||
configuredURL, configuredErr := validateHost(strings.TrimSpace(configured))
|
||||
if candidateErr != nil || configuredErr != nil {
|
||||
return false
|
||||
}
|
||||
return strings.EqualFold(candidateURL.Scheme, configuredURL.Scheme) &&
|
||||
strings.EqualFold(hostWithoutDefaultPort(candidateURL), hostWithoutDefaultPort(configuredURL))
|
||||
}
|
||||
|
||||
// hostWithoutDefaultPort strips a scheme's implicit default port (":443" for
|
||||
// https, ":80" for http) so an explicitly-ported origin like
|
||||
// "https://gitlab.example.com:443" compares equal to the equivalent
|
||||
// "https://gitlab.example.com" without one.
|
||||
func hostWithoutDefaultPort(u *url.URL) string {
|
||||
switch u.Scheme {
|
||||
case mentionHTTPSScheme:
|
||||
return strings.TrimSuffix(u.Host, ":443")
|
||||
case mentionHTTPScheme:
|
||||
return strings.TrimSuffix(u.Host, ":80")
|
||||
default:
|
||||
return u.Host
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ResolveTaskMRRepository validates an explicit repository or infers the sole
|
||||
|
||||
@@ -917,7 +917,7 @@ func ParseGitRemoteIdentity(remoteURL string) (origin, projectPath string) {
|
||||
}
|
||||
if strings.Contains(remoteURL, "@") && !strings.Contains(remoteURL, "://") {
|
||||
_, remoteURL, _ = strings.Cut(remoteURL, "@")
|
||||
host, path, ok := strings.Cut(remoteURL, ":")
|
||||
host, path, ok := cutSCPHostPath(remoteURL)
|
||||
if !ok {
|
||||
return "", ""
|
||||
}
|
||||
@@ -930,6 +930,21 @@ func ParseGitRemoteIdentity(remoteURL string) (origin, projectPath string) {
|
||||
return normalizeRemoteIdentity(parsed.Scheme, parsed.Host, parsed.Path)
|
||||
}
|
||||
|
||||
// cutSCPHostPath splits an scp-style remote's "host:path" segment (the part
|
||||
// after "git@"), honoring bracketed IPv6 literals such as
|
||||
// "[::1]:group/project.git" whose embedded colons would otherwise be
|
||||
// mistaken for the host/path separator by a plain strings.Cut on ":".
|
||||
func cutSCPHostPath(rest string) (host, path string, ok bool) {
|
||||
if strings.HasPrefix(rest, "[") {
|
||||
closeIdx := strings.Index(rest, "]")
|
||||
if closeIdx < 0 || closeIdx+1 >= len(rest) || rest[closeIdx+1] != ':' {
|
||||
return "", "", false
|
||||
}
|
||||
return rest[:closeIdx+1], rest[closeIdx+2:], true
|
||||
}
|
||||
return strings.Cut(rest, ":")
|
||||
}
|
||||
|
||||
func normalizeRemoteIdentity(scheme, host, path string) (string, string) {
|
||||
scheme = strings.ToLower(strings.TrimSpace(scheme))
|
||||
host = strings.ToLower(strings.TrimSpace(host))
|
||||
|
||||
@@ -550,6 +550,7 @@ func TestParseGitRemoteIdentityPreservesOriginAndFullSubgroupPath(t *testing.T)
|
||||
{"self-managed HTTPS", "https://gitlab.internal/group/subgroup/project", "https://gitlab.internal", "group/subgroup/project"},
|
||||
{"SSH URL", "ssh://git@gitlab.internal/group/subgroup/project.git", "ssh://gitlab.internal", "group/subgroup/project"},
|
||||
{"SCP SSH", "git@gitlab.internal:group/subgroup/project.git", "ssh://gitlab.internal", "group/subgroup/project"},
|
||||
{"SCP SSH bracketed IPv6", "git@[::1]:group/subgroup/project.git", "ssh://[::1]", "group/subgroup/project"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
||||
@@ -754,11 +754,6 @@ func (s *Service) createRepository(
|
||||
if resolveProvider {
|
||||
resolveRepositoryProviderIdentity(repository)
|
||||
}
|
||||
if repository.LocalPath != "" && repository.RemoteURL == "" {
|
||||
if origin := canonicalCloneOrigin(repository.LocalPath); origin != "" {
|
||||
repository.RemoteURL = origin
|
||||
}
|
||||
}
|
||||
|
||||
if err := s.repoEntities.CreateRepository(ctx, repository); err != nil {
|
||||
s.logger.Error("failed to create repository", zap.Error(err))
|
||||
@@ -770,19 +765,42 @@ func (s *Service) createRepository(
|
||||
return repository, nil
|
||||
}
|
||||
|
||||
// resolveRepositoryProviderIdentity fills missing provider metadata from a local repository origin.
|
||||
// resolveRepositoryProviderIdentity fills missing provider metadata from a
|
||||
// local repository origin. RemoteURL is resolved independently of the
|
||||
// provider/host/owner/name fields: those are only tagged for the well-known
|
||||
// github.com/gitlab.com hosts (see ResolveGitRemoteProviderIdentity), but
|
||||
// self-hosted GitLab/GitHub Enterprise instances still need a populated
|
||||
// RemoteURL so downstream identity matching (e.g. GitLab MR-task linking)
|
||||
// has something to compare against instead of failing closed.
|
||||
//
|
||||
// canonicalCloneOrigin is tried first because it produces the exact
|
||||
// provider-canonical clone URL (e.g. the ".git"-suffixed GitHub/GitLab.com
|
||||
// form) that other code, including test fixtures rewriting Git's clone
|
||||
// transport via "insteadOf" config, matches against verbatim. The broader
|
||||
// ResolveGitRemoteIdentity-based fallback only runs when canonicalCloneOrigin
|
||||
// doesn't recognize the host (e.g. a self-hosted GitLab/GitHub Enterprise
|
||||
// instance), since it doesn't guarantee a byte-identical canonical form.
|
||||
func resolveRepositoryProviderIdentity(repository *models.Repository) {
|
||||
if repository.LocalPath == "" || (repository.Provider != "" && repository.ProviderHost != "") {
|
||||
if repository.LocalPath == "" {
|
||||
return
|
||||
}
|
||||
p, h, o, n := ResolveGitRemoteProviderIdentity(repository.LocalPath)
|
||||
if repository.Provider == "" {
|
||||
repository.Provider = p
|
||||
if repository.Provider == "" || repository.ProviderHost == "" {
|
||||
p, h, o, n := ResolveGitRemoteProviderIdentity(repository.LocalPath)
|
||||
if repository.Provider == "" {
|
||||
repository.Provider = p
|
||||
}
|
||||
if repository.Provider != "" && (strings.HasPrefix(h, "http://") || strings.HasPrefix(h, "https://")) {
|
||||
repository.ProviderHost = h
|
||||
repository.ProviderOwner = o
|
||||
repository.ProviderName = n
|
||||
}
|
||||
}
|
||||
if repository.Provider != "" && (strings.HasPrefix(h, "http://") || strings.HasPrefix(h, "https://")) {
|
||||
repository.ProviderHost = h
|
||||
repository.ProviderOwner = o
|
||||
repository.ProviderName = n
|
||||
if repository.RemoteURL == "" {
|
||||
if origin := canonicalCloneOrigin(repository.LocalPath); origin != "" {
|
||||
repository.RemoteURL = origin
|
||||
} else if origin, owner, name := ResolveGitRemoteIdentity(repository.LocalPath); origin != "" && owner != "" && name != "" {
|
||||
repository.RemoteURL = origin + "/" + owner + "/" + name
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -131,6 +131,147 @@ func TestService_CreateRepositoryCanonicalizesExplicitLocalPath(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestService_CreateRepositoryResolvesRemoteURLForSelfHostedGitLabOrigin
|
||||
// covers the bug where a local repository cloned from a self-hosted GitLab
|
||||
// instance (e.g. gitlab.example.com, not gitlab.com) got no RemoteURL
|
||||
// persisted at creation time: resolveRepositoryProviderIdentity only tagged
|
||||
// Provider/ProviderHost/ProviderOwner/ProviderName for the well-known
|
||||
// github.com/gitlab.com hosts and never populated RemoteURL at all, so
|
||||
// downstream GitLab MR-task-link identity matching had nothing to compare
|
||||
// against and rejected every attach attempt for self-hosted repositories.
|
||||
func TestService_CreateRepositoryResolvesRemoteURLForSelfHostedGitLabOrigin(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
|
||||
repoPath := t.TempDir()
|
||||
makeRepo(t, repoPath)
|
||||
config := `[core]
|
||||
repositoryformatversion = 0
|
||||
[remote "origin"]
|
||||
url = git@gitlab.example.com:clients/socodevi/laravel/co-up.git
|
||||
fetch = +refs/heads/*:refs/remotes/origin/*
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(repoPath, ".git", "config"), []byte(config), 0o644); err != nil {
|
||||
t.Fatalf("write .git/config: %v", err)
|
||||
}
|
||||
|
||||
created, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{
|
||||
WorkspaceID: "ws-1",
|
||||
Name: "co-up",
|
||||
SourceType: sourceTypeLocal,
|
||||
LocalPath: repoPath,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
// ResolveGitRemoteIdentity normalizes scp-style remotes to an ssh://
|
||||
// origin; the GitLab MR-link identity matcher already treats ssh:// and
|
||||
// https:// origins as equivalent for the same host+path.
|
||||
const wantRemoteURL = "ssh://gitlab.example.com/clients/socodevi/laravel/co-up"
|
||||
if created.RemoteURL != wantRemoteURL {
|
||||
t.Fatalf("RemoteURL = %q, want %q", created.RemoteURL, wantRemoteURL)
|
||||
}
|
||||
// Self-hosted hosts are deliberately left untagged: only github.com and
|
||||
// gitlab.com get a durable Provider/ProviderHost identity.
|
||||
if created.Provider != "" {
|
||||
t.Fatalf("Provider = %q, want empty for self-hosted host", created.Provider)
|
||||
}
|
||||
stored, err := repo.GetRepository(ctx, created.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetRepository: %v", err)
|
||||
}
|
||||
if stored.RemoteURL != wantRemoteURL {
|
||||
t.Fatalf("stored RemoteURL = %q, want %q", stored.RemoteURL, wantRemoteURL)
|
||||
}
|
||||
}
|
||||
|
||||
// TestService_CreateRepositoryPrefersCanonicalCloneOriginForKnownProviders
|
||||
// is a regression guard: for a local checkout whose origin is a well-known
|
||||
// provider host (gitlab.com here), the RemoteURL fallback must resolve to
|
||||
// canonicalCloneOrigin's exact ".git"-suffixed canonical form and not the
|
||||
// broader ResolveGitRemoteIdentity-based form (which omits the suffix).
|
||||
// Other code, including E2E test fixtures that rewrite Git's clone
|
||||
// transport via an "insteadOf" config keyed on that exact canonical URL,
|
||||
// depends on this byte-identical form to route the clone correctly.
|
||||
func TestService_CreateRepositoryPrefersCanonicalCloneOriginForKnownProviders(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
|
||||
repoPath := t.TempDir()
|
||||
makeRepo(t, repoPath)
|
||||
config := `[core]
|
||||
repositoryformatversion = 0
|
||||
[remote "origin"]
|
||||
url = https://gitlab.com/fixture/docker-second-source.git
|
||||
fetch = +refs/heads/*:refs/remotes/origin/*
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(repoPath, ".git", "config"), []byte(config), 0o644); err != nil {
|
||||
t.Fatalf("write .git/config: %v", err)
|
||||
}
|
||||
|
||||
created, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{
|
||||
WorkspaceID: "ws-1",
|
||||
Name: "fixture/docker-second-source",
|
||||
SourceType: sourceTypeLocal,
|
||||
LocalPath: repoPath,
|
||||
Provider: "gitlab",
|
||||
ProviderHost: "https://gitlab.com",
|
||||
ProviderOwner: "fixture",
|
||||
ProviderName: "docker-second-source",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
const wantRemoteURL = "https://gitlab.com/fixture/docker-second-source.git"
|
||||
if created.RemoteURL != wantRemoteURL {
|
||||
t.Fatalf("RemoteURL = %q, want %q", created.RemoteURL, wantRemoteURL)
|
||||
}
|
||||
}
|
||||
|
||||
// TestService_CreateRepositoryDoesNotOverwriteExplicitRemoteURL ensures the
|
||||
// new local-discovery RemoteURL fallback only fills a gap and never clobbers
|
||||
// a caller-supplied RemoteURL.
|
||||
func TestService_CreateRepositoryDoesNotOverwriteExplicitRemoteURL(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
|
||||
repoPath := t.TempDir()
|
||||
makeRepo(t, repoPath)
|
||||
config := `[core]
|
||||
repositoryformatversion = 0
|
||||
[remote "origin"]
|
||||
url = git@gitlab.example.com:group/discovered-repo.git
|
||||
fetch = +refs/heads/*:refs/remotes/origin/*
|
||||
`
|
||||
if err := os.WriteFile(filepath.Join(repoPath, ".git", "config"), []byte(config), 0o644); err != nil {
|
||||
t.Fatalf("write .git/config: %v", err)
|
||||
}
|
||||
const explicitRemoteURL = "https://gitlab.example.com/group/explicit-repo"
|
||||
|
||||
created, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{
|
||||
WorkspaceID: "ws-1",
|
||||
Name: "explicit-repo",
|
||||
SourceType: sourceTypeLocal,
|
||||
LocalPath: repoPath,
|
||||
RemoteURL: explicitRemoteURL,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRepository: %v", err)
|
||||
}
|
||||
if created.RemoteURL != explicitRemoteURL {
|
||||
t.Fatalf("RemoteURL = %q, want unmodified explicit value %q", created.RemoteURL, explicitRemoteURL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestService_CreateRepositoryRejectsInvalidLocalPathWithoutPersistence(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
|
||||
Reference in New Issue
Block a user