R-616 (on top of R-615): the catalog clone stores no credentials

Replaces 365eff6 for main, which now carries R-615 (0b349e2). The plain URL is cloned; the Basic credentials ride per git command as http.<origin>.extraHeader via GIT_CONFIG_COUNT. R-615's origin compare now compares credential-free forms against the configured URL, so a set token never re-clones; a stored origin with user:token@ is rewritten without it. Pinned by TestR616_* incl. TestR616_TokenSetSameRepoNoRecloneOriginClean. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
2026-10-05 21:38:24 +02:00
parent 6fac245d24
commit 28a5203d01
2 changed files with 239 additions and 21 deletions
@@ -0,0 +1,172 @@
+package sync
+
+import (
+	"bytes"
+	"encoding/base64"
+	"log"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// R-616: the catalog credentials must never be stored in the clone. A fake https remote is served from
+// a local repository through git's own url.<base>.insteadOf (in a test-only HOME), so the real clone
+// and fetch paths run with no network: git REWRITES the URL it dials but STORES the URL it was given —
+// which is exactly the URL the product chose, credentialed or not.
+func r616Fixture(t *testing.T) (s *Syncer, logs *bytes.Buffer, src string) {
+	t.Helper()
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not on PATH")
+	}
+	root := t.TempDir()
+	src = filepath.Join(root, "src")
+	run := func(dir string, args ...string) {
+		t.Helper()
+		cmd := exec.Command("git", args...)
+		cmd.Dir = dir
+		if out, err := cmd.CombinedOutput(); err != nil {
+			t.Fatalf("git %v: %v\n%s", args, err, out)
+		}
+	}
+	home := filepath.Join(root, "home")
+	os.MkdirAll(home, 0o755)
+	t.Setenv("HOME", home)
+	t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
+	t.Setenv("GIT_CONFIG_NOSYSTEM", "1")
+	gitcfg := "[user]\n\tname = t\n\temail = t@example.invalid\n" +
+		"[url \"file://" + src + "\"]\n" +
+		"\tinsteadOf = https://user:tok-SECRET@catalog.example.invalid/admin/catalog.git\n" +
+		"\tinsteadOf = https://catalog.example.invalid/admin/catalog.git\n"
+	if err := os.WriteFile(filepath.Join(home, ".gitconfig"), []byte(gitcfg), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	os.MkdirAll(src, 0o755)
+	run(src, "init", "-q", "-b", "main")
+	os.WriteFile(filepath.Join(src, "README"), []byte("x\n"), 0o644)
+	run(src, "add", "README")
+	run(src, "commit", "-q", "-m", "init")
+
+	s = newTestSyncer(t, "https://catalog.example.invalid/admin/catalog.git")
+	s.cfg.Git.Username = "user"
+	s.cfg.Git.Token = "tok-SECRET"
+	logs = &bytes.Buffer{}
+	s.logger = log.New(logs, "", 0)
+	return s, logs, src
+}
+
+func storedOrigin(t *testing.T, dir string) string {
+	t.Helper()
+	out, err := exec.Command("git", "-C", dir, "config", "--get", "remote.origin.url").Output()
+	if err != nil {
+		t.Fatalf("read origin: %v", err)
+	}
+	return strings.TrimSpace(string(out))
+}
+
+func TestR616_CloneStoresNoCredentials(t *testing.T) {
+	s, logs, _ := r616Fixture(t)
+	if err := s.gitCloneOrPull(); err != nil {
+		t.Fatalf("clone: %v", err)
+	}
+	origin := storedOrigin(t, s.cacheDir)
+	if strings.Contains(origin, "@") || strings.Contains(origin, "tok-SECRET") {
+		t.Errorf("the clone's stored origin carries credentials: %q", origin)
+	}
+	cfgBytes, _ := os.ReadFile(filepath.Join(s.cacheDir, ".git", "config"))
+	if strings.Contains(string(cfgBytes), "tok-SECRET") {
+		t.Errorf(".git/config holds the token in the clear:\n%s", cfgBytes)
+	}
+	// A pull (fetch + reset) still works on the clean clone.
+	if err := s.gitCloneOrPull(); err != nil {
+		t.Fatalf("pull: %v", err)
+	}
+	if strings.Contains(logs.String(), "tok-SECRET") {
+		t.Errorf("the token reached the log:\n%s", logs.String())
+	}
+}
+
+// A clone made BEFORE the fix (origin with userinfo) is scrubbed on the next pull.
+func TestR616_PreFixCloneIsScrubbedOnPull(t *testing.T) {
+	s, logs, _ := r616Fixture(t)
+	cmd := exec.Command("git", "clone", "-q", "--depth", "1", "--branch", "main",
+		"https://user:tok-SECRET@catalog.example.invalid/admin/catalog.git", s.cacheDir)
+	if out, err := cmd.CombinedOutput(); err != nil {
+		t.Fatalf("pre-fix clone: %v\n%s", err, out)
+	}
+	if !strings.Contains(storedOrigin(t, s.cacheDir), "tok-SECRET") {
+		t.Fatal("fixture: the pre-fix clone should carry the token")
+	}
+	if err := s.gitCloneOrPull(); err != nil {
+		t.Fatalf("pull: %v", err)
+	}
+	if got := storedOrigin(t, s.cacheDir); got != "https://catalog.example.invalid/admin/catalog.git" {
+		t.Errorf("stored origin not scrubbed: %q", got)
+	}
+	if strings.Contains(logs.String(), "tok-SECRET") {
+		t.Errorf("the token reached the log:\n%s", logs.String())
+	}
+}
+
+// The credentials still reach git: the header is scoped to the remote's origin and carries the same
+// Basic pair the URL form sent. Checked with git's own URL matcher, no network.
+func TestR616_AuthHeaderScopedToTheRemote(t *testing.T) {
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not on PATH")
+	}
+	s := newTestSyncer(t, "https://catalog.example.invalid/admin/catalog.git")
+	if env := s.gitAuthEnv(); env != nil {
+		t.Errorf("no credentials configured -> no auth env, got %v", env)
+	}
+	s.cfg.Git.Username, s.cfg.Git.Token = "user", "tok-SECRET"
+	env := s.gitAuthEnv()
+	want := "Authorization: Basic " + base64.StdEncoding.EncodeToString([]byte("user:tok-SECRET"))
+	match := func(url string) string {
+		cmd := exec.Command("git", "config", "--get-urlmatch", "http.extraHeader", url)
+		cmd.Env = append(os.Environ(), env...)
+		out, _ := cmd.Output()
+		return strings.TrimSpace(string(out))
+	}
+	t.Setenv("GIT_CONFIG_NOSYSTEM", "1")
+	t.Setenv("HOME", t.TempDir())
+	if got := match("https://catalog.example.invalid/admin/catalog.git"); got != want {
+		t.Errorf("header for the remote = %q, want the Basic pair", got)
+	}
+	if got := match("https://other.example.invalid/x.git"); got != "" {
+		t.Errorf("the header must not reach another host, got %q", got)
+	}
+	s.cfg.Git.RepoURL = "/local/path/catalog"
+	if env := s.gitAuthEnv(); env != nil {
+		t.Errorf("a non-https remote gets no auth env, got %v", env)
+	}
+}
+
+// R-616 × R-615: with a token set and the SAME repository configured, repeated syncs never re-clone
+// (the configured URL carries no credentials to differ from the stored origin) and the stored origin
+// stays free of '@'. A re-clone is detected by a marker file planted inside the cache: RemoveAll would
+// take it with it.
+func TestR616_TokenSetSameRepoNoRecloneOriginClean(t *testing.T) {
+	s, logs, _ := r616Fixture(t)
+	if err := s.gitCloneOrPull(); err != nil {
+		t.Fatalf("clone: %v", err)
+	}
+	marker := filepath.Join(s.cacheDir, ".git", "r616-no-reclone")
+	if err := os.WriteFile(marker, []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	for i := 0; i < 3; i++ {
+		if err := s.gitCloneOrPull(); err != nil {
+			t.Fatalf("pull %d: %v", i, err)
+		}
+	}
+	if _, err := os.Stat(marker); err != nil {
+		t.Error("the cache was RE-CLONED on a same-repo sync with a token set")
+	}
+	if o := storedOrigin(t, s.cacheDir); strings.Contains(o, "@") {
+		t.Errorf("stored origin carries credentials: %q", o)
+	}
+	if strings.Contains(logs.String(), "re-cloning") || strings.Contains(logs.String(), "tok-SECRET") {
+		t.Errorf("unexpected re-clone or token in the log:\n%s", logs.String())
+	}
+}