Fix credential protocol parsing and path matching

README.md

@@ -43,7 +43,16 @@ the credential.helper config sequence is important, git will match against it un
 
 so organization specific token should goes first.
 
+If a system-level config already defines a helper (for example,
+`osxkeychain` on macOS), reset the inherited helper list before adding scoped
+and fallback helpers. An empty `helper` resets every helper seen before it, so
+its position is significant.
+
 ```ini
+# reset inherited helpers before adding the ordered helper list
+[credential]
+	helper =
+
 # organization specific token
 [credential "https://github.com/your-org/"]
 	helper = readonly --file ~/.git-credentials-org
@@ -79,6 +88,10 @@ you can also use personal and org tokens in one file:
 the config:
 
 ```ini
+# reset inherited helpers before adding the GitHub-specific helper
+[credential]
+	helper =
+
 # github specific token
 [credential "https://github.com/"]
 	helper = readonly --file ~/.git-credentials-org

main.go

@@ -7,10 +7,10 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"net/url"
 	"os"
 	"os/user"
 	"strings"
-	"net/url"
 )
 
 const (
@@ -100,51 +100,42 @@ func (c *credential) match(req *credential) bool {
 	}
 
 	if req.path != "" {
-		// get username or org from repo path like `username-or-org/reponame.git`
-		reqOrg, _, hasSep := strings.Cut(req.path, "/")
-		if hasSep && reqOrg != "" {
-			matchReqPath := strings.TrimRight(reqOrg, "/")
-			matchConfigPath := strings.TrimRight(c.path, "/")
-			match = match && matchReqPath == matchConfigPath
-			log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v",
-				matchReqPath, matchConfigPath, match)
-		} else {
-			match = match && c.path == req.path
-			log.Printf("match path: req.path=%v,other.path=%v,result=%v",
-				c.path, req.path, match)
-		}
+		pathMatch := matchCredentialPath(c.path, req.path)
+		match = match && pathMatch
+		log.Printf("match path: req.path=%v,config.path=%v,result=%v",
+			req.path, c.path, match)
 	}
 	return match
 }
 
-func parseGitCredentialRequest(r io.Reader) (*credential, error) {
-	rd := bufio.NewReader(r)
-	req := &credential{}
-	for {
-		key, err := rd.ReadString('=')
-		if err != nil {
-			if err == io.EOF {
-				if key == "" {
-					return req, nil
-				}
+func matchCredentialPath(configPath, requestPath string) bool {
+	configPath = strings.TrimRight(configPath, "/")
+	requestPath = strings.TrimRight(requestPath, "/")
 
-				return nil, io.ErrUnexpectedEOF
-			}
+	// Preserve the standard credential-store behavior for repository-specific
+	// entries before trying the owner/organization shorthand supported here.
+	if configPath == requestPath {
+		return true
+	}
 
-			return nil, err
-		}
+	requestOwner, _, hasSeparator := strings.Cut(requestPath, "/")
+	return hasSeparator && requestOwner != "" && configPath == requestOwner
+}
 
-		key = strings.TrimSuffix(key, "=")
-		val, err := rd.ReadString('\n')
-		if err != nil {
-			if errors.Is(err, io.EOF) {
-				err = io.ErrUnexpectedEOF
-			}
+func parseGitCredentialRequest(r io.Reader) (*credential, error) {
+	scanner := bufio.NewScanner(r)
+	req := &credential{}
+	for scanner.Scan() {
+		line := scanner.Text()
+		if line == "" {
+			break
+		}
 
-			return nil, err
+		key, val, found := strings.Cut(line, "=")
+		if !found {
+			return nil, errors.New("malformed credential attribute")
 		}
 
-		val = strings.TrimSuffix(val, "\n")
 		switch key {
 		case "protocol":
 			req.protocol = val
@@ -158,6 +149,11 @@ func parseGitCredentialRequest(r io.Reader) (*credential, error) {
 			req.password = val
 		}
 	}
+	if err := scanner.Err(); err != nil {
+		return nil, err
+	}
+
+	return req, nil
 }
 
 func parseCredential(line string) *credential {
@@ -181,31 +177,31 @@ func parseCredential(line string) *credential {
 		// malformed line, ignore
 		return nil
 	}
-	username,err := url.QueryUnescape(credFields[0])
-    	if err != nil {
-        	return nil
+	username, err := url.QueryUnescape(credFields[0])
+	if err != nil {
+		return nil
 	}
-	password,err := url.QueryUnescape(credFields[1])
-    	if err != nil {
-        	return nil
+	password, err := url.QueryUnescape(credFields[1])
+	if err != nil {
+		return nil
 	}
-	
+
 	hostAndPath := fields[1]
 	hostFields := strings.SplitN(hostAndPath, "/", 2)
 	if len(hostFields) != 1 && len(hostFields) != 2 {
 		// malformed line, ignore
 		return nil
 	}
-	host,err := url.QueryUnescape(hostFields[0])
+	host, err := url.QueryUnescape(hostFields[0])
 	if err != nil {
-        	return nil
+		return nil
 	}
-	
+
 	var path string
 	if len(hostFields) == 2 {
-		path,err = url.QueryUnescape(hostFields[1])
+		path, err = url.QueryUnescape(hostFields[1])
 		if err != nil {
-        		return nil
+			return nil
 		}
 	}
 

main_test.go

@@ -2,40 +2,168 @@ package main
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 )
 
 func TestGetCredential(t *testing.T) {
-	// create a temporary credential file
-	credFile, err := ioutil.TempFile("", "test-cred")
+	credFile := filepath.Join(t.TempDir(), "credentials")
+	credentials := []string{
+		"https://john:password@github.com/foo/bar",
+		"https://octocat:org-password@github.com/acme",
+		"https://jane:password@bitbucket.org/foo/bar.git",
+	}
+	file, err := os.Create(credFile)
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.Remove(credFile.Name())
-
-	// write some credentials to the file
-	creds := []string{
-		"https://john:password@github.com/foo/bar",
-		"https://jane:password@bitbucket.org/foo/bar.git",
+	for _, value := range credentials {
+		fmt.Fprintln(file, value)
 	}
-	for _, cred := range creds {
-		fmt.Fprintln(credFile, cred)
+	if err := file.Close(); err != nil {
+		t.Fatal(err)
 	}
 
-	// test getting a credential that exists in the file
-	c := getCredential(&credential{username: "john", protocol: "https", host: "github.com", path: "foo/bar"}, credFile.Name())
-	if c == nil {
-		t.Errorf("expected to find a credential for github.com/foo/bar")
+	tests := []struct {
+		name     string
+		request  *credential
+		username string
+		password string
+	}{
+		{
+			name:     "full repository path",
+			request:  &credential{protocol: "https", host: "github.com", path: "foo/bar"},
+			username: "john",
+			password: "password",
+		},
+		{
+			name:     "owner path",
+			request:  &credential{protocol: "https", host: "github.com", path: "acme/widgets.git"},
+			username: "octocat",
+			password: "org-password",
+		},
 	}
-	if c.username != "john" || c.password != "password" {
-		t.Errorf("unexpected credential found: %+v", c)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := getCredential(tt.request, credFile)
+			if got == nil {
+				t.Fatal("expected to find a credential")
+			}
+			if got.username != tt.username || got.password != tt.password {
+				t.Errorf("unexpected credential: %+v", got)
+			}
+		})
 	}
 
-	// test getting a credential that does not exist in the file
-	c = getCredential(&credential{username: "john", protocol: "https", host: "bitbucket.org", path: "foo/bar"}, credFile.Name())
-	if c != nil {
+	got := getCredential(
+		&credential{username: "john", protocol: "https", host: "bitbucket.org", path: "foo/bar"},
+		credFile,
+	)
+	if got != nil {
 		t.Errorf("expected to not find a credential for bitbucket.org/foo/bar")
 	}
 }
+
+func TestMatchCredentialPath(t *testing.T) {
+	tests := []struct {
+		name        string
+		configPath  string
+		requestPath string
+		want        bool
+	}{
+		{
+			name:        "full repository path",
+			configPath:  "acme/widgets.git",
+			requestPath: "acme/widgets.git",
+			want:        true,
+		},
+		{
+			name:        "owner path",
+			configPath:  "acme",
+			requestPath: "acme/widgets.git",
+			want:        true,
+		},
+		{
+			name:        "trailing slash",
+			configPath:  "acme/",
+			requestPath: "acme/widgets.git/",
+			want:        true,
+		},
+		{
+			name:        "different repository",
+			configPath:  "acme/widgets.git",
+			requestPath: "acme/gadgets.git",
+			want:        false,
+		},
+		{
+			name:        "different owner",
+			configPath:  "acme",
+			requestPath: "other/widgets.git",
+			want:        false,
+		},
+		{
+			name:        "empty config path",
+			requestPath: "acme/widgets.git",
+			want:        false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := matchCredentialPath(tt.configPath, tt.requestPath); got != tt.want {
+				t.Errorf("matchCredentialPath(%q, %q) = %v, want %v",
+					tt.configPath, tt.requestPath, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestParseGitCredentialRequest(t *testing.T) {
+	want := credential{
+		protocol: "https",
+		host:     "github.com",
+		path:     "acme/widgets.git",
+		username: "octocat",
+	}
+
+	tests := []struct {
+		name  string
+		input string
+	}{
+		{
+			name: "EOF terminated",
+			input: "protocol=https\n" +
+				"host=github.com\n" +
+				"path=acme/widgets.git\n" +
+				"username=octocat\n",
+		},
+		{
+			name: "blank line terminated",
+			input: "protocol=https\n" +
+				"host=github.com\n" +
+				"path=acme/widgets.git\n" +
+				"username=octocat\n\nignored=value\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := parseGitCredentialRequest(strings.NewReader(tt.input))
+			if err != nil {
+				t.Fatalf("parse credential request: %v", err)
+			}
+			if *got != want {
+				t.Errorf("unexpected credential request: got %+v, want %+v", *got, want)
+			}
+		})
+	}
+}
+
+func TestParseGitCredentialRequestRejectsMalformedAttribute(t *testing.T) {
+	_, err := parseGitCredentialRequest(strings.NewReader("protocol=https\nmalformed\n"))
+	if err == nil {
+		t.Fatal("expected malformed attribute to return an error")
+	}
+}