dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(gitcredentials): reject unsafe clone authorities (#2869)

Browse Source 
Co-authored-by: ayattara <alassane.yattara@savoirfairelinux.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Alassane
2026-08-21 05:34:19 -04:00
committed by GitHub
parent 08f07b709e
commit 05fe86fd76
5 changed files with 269 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/gitcredentials/identity.go
+60
View File
@@ -97,6 +97,9 @@ type parsedRemote struct {
func parseRemote(raw string) (parsedRemote, error) {
value := strings.TrimSpace(raw)
if !hasSafeRemoteAuthority(value) {
return parsedRemote{}, fmt.Errorf("invalid clone URL authority")
}
canonical := repoclone.CanonicalHTTPSCloneURL(value)
if canonical != "" {
if err := validateSSHRemote(value); err != nil {
@@ -138,6 +141,9 @@ func validateSSHRemote(value string) error {
func trustedOrigin(input RepositoryIdentityInput, remoteHost string) (string, error) {
if host := strings.TrimSpace(input.ProviderHost); host != "" {
if !hasSafeProviderHostAuthority(host) {
return "", repositoryIdentityError(input.RepositoryID, "has an invalid provider host")
}
origin, err := repoclone.HTTPSProviderOrigin(host)
if err != nil {
return "", repositoryIdentityError(input.RepositoryID, "has an invalid provider host")
@@ -168,3 +174,57 @@ func repositoryIdentityError(repositoryID, detail string) error {
}
return fmt.Errorf("repository %q %s; repair its clone URL or provider host", repositoryID, detail)
}
func hasSafeRemoteAuthority(value string) bool {
authority, found := rawRemoteAuthority(value)
return found && repoclone.IsSafeRawAuthority(authority)
}
func rawRemoteAuthority(value string) (string, bool) {
if schemeEnd := strings.Index(value, "://"); schemeEnd >= 0 && isURLScheme(value[:schemeEnd]) {
return authorityBeforePath(value[schemeEnd+len("://"):])
}
user, hostAndPath, found := strings.Cut(value, "@")
if !found || user == "" {
return "", false
}
host, _, found := strings.Cut(hostAndPath, ":")
if !found || host == "" {
return "", false
}
return user + "@" + host, true
}
func hasSafeProviderHostAuthority(value string) bool {
if schemeEnd := strings.Index(value, "://"); schemeEnd >= 0 {
if !isURLScheme(value[:schemeEnd]) {
return false
}
value = value[schemeEnd+len("://"):]
}
authority, found := authorityBeforePath(value)
return found && repoclone.IsSafeRawAuthority(authority)
}
func authorityBeforePath(value string) (string, bool) {
if authorityEnd := strings.IndexAny(value, "/?#"); authorityEnd >= 0 {
value = value[:authorityEnd]
}
return value, value != ""
}
func isURLScheme(value string) bool {
if value == "" || !isASCIILetter(value[0]) {
return false
}
for i := 1; i < len(value); i++ {
if !isASCIILetter(value[i]) && (value[i] < '0' || value[i] > '9') && value[i] != '+' && value[i] != '-' && value[i] != '.' {
return false
}
}
return true
}
func isASCIILetter(value byte) bool {
return value >= 'a' && value <= 'z' || value >= 'A' && value <= 'Z'
}
apps/backend/internal/gitcredentials/identity_hostile_test.go
+99
View File
@@ -0,0 +1,99 @@
package gitcredentials
import (
"strings"
"testing"
)
func TestResolveRepositoryIdentityRejectsUnsafeRemoteAuthorities(t *testing.T) {
t.Parallel()
remotes := []string{
"git@gİthub.com:acme/widgets.git",
"ssh://git@gİthub.com/acme/widgets.git",
"https://gİthub.com/acme/widgets.git",
"git@g%C4%B0thub.com:acme/widgets.git",
"ssh://git@g%C4%B0thub.com/acme/widgets.git",
"https://g%C4%B0thub.com/acme/widgets.git",
"git@g%25C4%25B0thub.com:acme/widgets.git",
"ssh://git@g%25C4%25B0thub.com/acme/widgets.git",
"https://g%25C4%25B0thub.com/acme/widgets.git",
}
for _, providerHost := range []string{"", "https://github.com"} {
for _, remoteURL := range remotes {
t.Run(providerHost+"/"+remoteURL, func(t *testing.T) {
t.Parallel()
_, err := ResolveRepositoryIdentity(RepositoryIdentityInput{
RepositoryID: "repo-1", ProviderHost: providerHost, RemoteURL: remoteURL,
})
if err == nil {
t.Fatal("ResolveRepositoryIdentity() error = nil, want rejection")
}
if strings.Contains(err.Error(), remoteURL) || !strings.Contains(err.Error(), "repair its clone URL or provider host") {
t.Fatalf("error = %q, want generic remediation without hostile remote", err)
}
})
}
}
}
func TestParseRemoteRejectsUnsafeHTTPSAuthorities(t *testing.T) {
t.Parallel()
for _, remoteURL := range []string{
"https://gİthub.com/acme/widgets.git",
"https://g%C4%B0thub.com/acme/widgets.git",
"https://g%25C4%25B0thub.com/acme/widgets.git",
} {
t.Run(remoteURL, func(t *testing.T) {
t.Parallel()
if _, err := parseRemote(remoteURL); err == nil {
t.Fatalf("parseRemote(%q) error = nil, want rejection", remoteURL)
}
})
}
}
func TestResolveRepositoryIdentityRejectsUnsafeProviderHost(t *testing.T) {
t.Parallel()
for _, providerHost := range []string{
"gİthub.com",
"g%C4%B0thub.com",
"g%25C4%25B0thub.com",
} {
t.Run(providerHost, func(t *testing.T) {
t.Parallel()
_, err := ResolveRepositoryIdentity(RepositoryIdentityInput{
RepositoryID: "repo-1", ProviderHost: providerHost,
RemoteURL: "https://github.com/acme/widgets.git",
})
if err == nil {
t.Fatal("ResolveRepositoryIdentity() error = nil, want rejection")
}
if strings.Contains(err.Error(), providerHost) || !strings.Contains(err.Error(), "repair its clone URL or provider host") {
t.Fatalf("error = %q, want generic remediation without hostile provider host", err)
}
})
}
}
func TestResolveRepositoryIdentityPreservesUnicodeAndEncodedPaths(t *testing.T) {
t.Parallel()
for _, remoteURL := range []string{
"git@github.com:acme/wídgets.git",
"ssh://git@github.com/acme/wídgets.git",
"https://github.com/acme/wídgets.git",
"git@github.com:acme/w%C3%ADdgets.git",
"ssh://git@github.com/acme/w%C3%ADdgets.git",
"https://github.com/acme/w%C3%ADdgets.git",
} {
t.Run(remoteURL, func(t *testing.T) {
t.Parallel()
identity, err := ResolveRepositoryIdentity(RepositoryIdentityInput{RepositoryID: "repo-1", RemoteURL: remoteURL})
if err != nil {
t.Fatalf("ResolveRepositoryIdentity() error = %v", err)
}
if identity.Path != "/acme/wídgets.git" {
t.Fatalf("identity.Path = %q, want %q", identity.Path, "/acme/wídgets.git")
}
})
}
}
apps/backend/internal/repoclone/origin.go
+56 -1
View File
@@ -17,6 +17,9 @@ func HTTPSProviderOrigin(providerHost string) (string, error) {
if !strings.Contains(value, "://") {
value = ProtocolHTTPS + "://" + value
}
if !hasSafeRawAuthority(value) {
return "", fmt.Errorf("provider host must be a credential-free HTTPS URL")
}
parsed, err := url.Parse(value)
if err != nil || !strings.EqualFold(parsed.Scheme, ProtocolHTTPS) || parsed.Host == "" ||
parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" {
@@ -41,6 +44,9 @@ func CanonicalHTTPSCloneURL(raw string) string {
if !strings.Contains(value, "://") {
return scpStyleHTTPSCloneURL(value)
}
if !hasSafeRawAuthority(value) {
return ""
}
parsed, err := url.Parse(value)
if err != nil || !strings.EqualFold(parsed.Scheme, ProtocolSSH) ||
parsed.Hostname() == "" || parsed.Path == "" || parsed.Path == "/" {
@@ -60,6 +66,9 @@ func scpStyleHTTPSCloneURL(value string) string {
if !found || host == "" || strings.Contains(host, "/") {
return ""
}
if !IsSafeRawAuthority(user + "@" + host) {
return ""
}
path = strings.TrimPrefix(path, "/")
if path == "" {
return ""
@@ -71,7 +80,11 @@ func scpStyleHTTPSCloneURL(value string) string {
// and requires their origin to match the provider identity that authorized
// credential use.
func ValidateHTTPSCloneOrigin(cloneURL, providerHost string) error {
parsed, err := url.Parse(strings.TrimSpace(cloneURL))
value := strings.TrimSpace(cloneURL)
if !hasSafeRawAuthority(value) {
return fmt.Errorf("clone URL must be a credential-free HTTPS URL")
}
parsed, err := url.Parse(value)
if err != nil || !strings.EqualFold(parsed.Scheme, ProtocolHTTPS) || parsed.Host == "" ||
parsed.User != nil || parsed.Path == "" || parsed.RawQuery != "" || parsed.Fragment != "" {
return fmt.Errorf("clone URL must be a credential-free HTTPS URL")
@@ -86,3 +99,45 @@ func ValidateHTTPSCloneOrigin(cloneURL, providerHost string) error {
}
return nil
}
func hasSafeRawAuthority(value string) bool {
authority, found := rawAuthority(value)
return found && IsSafeRawAuthority(authority)
}
func rawAuthority(value string) (string, bool) {
if schemeEnd := strings.Index(value, "://"); schemeEnd >= 0 && isURLScheme(value[:schemeEnd]) {
value = value[schemeEnd+len("://"):]
}
if authorityEnd := strings.IndexAny(value, "/?#"); authorityEnd >= 0 {
value = value[:authorityEnd]
}
return value, value != ""
}
// IsSafeRawAuthority reports whether a raw authority stays ASCII-only and
// percent-free before any URL parsing or case folding can reinterpret it.
func IsSafeRawAuthority(authority string) bool {
for i := 0; i < len(authority); i++ {
if authority[i] > 0x7f || authority[i] == '%' {
return false
}
}
return true
}
func isURLScheme(value string) bool {
if value == "" || !isASCIILetter(value[0]) {
return false
}
for i := 1; i < len(value); i++ {
if !isASCIILetter(value[i]) && (value[i] < '0' || value[i] > '9') && value[i] != '+' && value[i] != '-' && value[i] != '.' {
return false
}
}
return true
}
func isASCIILetter(value byte) bool {
return value >= 'a' && value <= 'z' || value >= 'A' && value <= 'Z'
}
apps/backend/internal/repoclone/origin_test.go
+51
View File
@@ -16,6 +16,14 @@ func TestCanonicalHTTPSCloneURL(t *testing.T) {
{name: "ssh scheme", raw: "ssh://git@github.com/acme/widgets.git", want: "https://github.com/acme/widgets.git"},
{name: "ssh scheme with port", raw: "ssh://git@gitlab.example:2222/group/widgets.git", want: "https://gitlab.example/group/widgets.git"},
{name: "ssh scheme drops password", raw: "ssh://git:secret@github.com/acme/widgets.git", want: "https://github.com/acme/widgets.git"},
{name: "scp style rejects Unicode authority", raw: "git@gİthub.com:acme/widgets.git"},
{name: "ssh scheme rejects Unicode authority", raw: "ssh://git@gİthub.com/acme/widgets.git"},
{name: "scp style rejects encoded authority", raw: "git@g%C4%B0thub.com:acme/widgets.git"},
{name: "ssh scheme rejects encoded authority", raw: "ssh://git@g%C4%B0thub.com/acme/widgets.git"},
{name: "scp style rejects double encoded authority", raw: "git@g%25C4%25B0thub.com:acme/widgets.git"},
{name: "ssh scheme rejects double encoded authority", raw: "ssh://git@g%25C4%25B0thub.com/acme/widgets.git"},
{name: "scp style preserves Unicode path", raw: "git@github.com:acme/wídgets.git", want: "https://github.com/acme/wídgets.git"},
{name: "ssh scheme preserves Unicode path", raw: "ssh://git@github.com/acme/wídgets.git", want: "https://github.com/acme/wídgets.git"},
{name: "https passes through untouched", raw: "https://github.com/acme/widgets.git"},
{name: "file remote", raw: "file:///tmp/widgets.git"},
{name: "local path", raw: "/home/user/widgets"},
@@ -35,6 +43,25 @@ func TestCanonicalHTTPSCloneURL(t *testing.T) {
}
}
func TestHTTPSProviderOriginRejectsUnsafeAuthority(t *testing.T) {
t.Parallel()
for _, providerHost := range []string{
"gİthub.com",
"g%C4%B0thub.com",
"g%25C4%25B0thub.com",
"https://gİthub.com",
"https://g%C4%B0thub.com",
"https://g%25C4%25B0thub.com",
} {
t.Run(providerHost, func(t *testing.T) {
t.Parallel()
if _, err := HTTPSProviderOrigin(providerHost); err == nil {
t.Fatalf("HTTPSProviderOrigin(%q) error = nil, want rejection", providerHost)
}
})
}
}
func TestValidateHTTPSCloneOrigin(t *testing.T) {
t.Parallel()
tests := []struct {
@@ -77,6 +104,30 @@ func TestValidateHTTPSCloneOrigin(t *testing.T) {
providerHost: "https://bitbucket.example.test",
wantErr: true,
},
{
name: "Unicode clone authority",
cloneURL: "https://gİthub.com/acme/widgets.git",
providerHost: "https://github.com",
wantErr: true,
},
{
name: "encoded clone authority",
cloneURL: "https://g%C4%B0thub.com/acme/widgets.git",
providerHost: "https://github.com",
wantErr: true,
},
{
name: "unsafe provider authority",
cloneURL: "https://github.com/acme/widgets.git",
providerHost: "https://gİthub.com",
wantErr: true,
},
{
name: "bare Unicode provider authority",
cloneURL: "https://github.com/acme/widgets.git",
providerHost: "gİthub.com",
wantErr: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
docs/specs/integrations/github-authentication.md
+3 -2
View File
@@ -435,8 +435,9 @@ post-signature processing failures produce `failing`; a later valid successful d
- Public base URL validation requires a canonical HTTPS origin with no credentials, query, or
fragment and rejects loopback, private, link-local, or non-globally-routable DNS results. Kandev
does not fetch the supplied URL as validation.
- Repository identity accepts credential-free HTTPS and SSH remote forms. It rejects an SSH
password, query, or fragment before canonicalization, and an error never contains the rejected
- Repository identity accepts credential-free HTTPS and SSH remote forms. Authorities must remain
ASCII-only and percent-free before URL parsing or case folding. It rejects an SSH password,
query, or fragment before canonicalization, and an error never contains the rejected
secret-bearing remote.
- App private keys, client secrets, webhook secrets, personal tokens, and live installation tokens
never enter executor environments. Only brokered PAT/CLI tokens or repository-restricted