diff --git a/controller/internal/sync/r616_credentials_test.go b/controller/internal/sync/r616_credentials_test.go
new file mode 100644
index 0000000..0e36228
--- /dev/null
+++ b/controller/internal/sync/r616_credentials_test.go
@@ -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..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())
+ }
+}
diff --git a/controller/internal/sync/sync.go b/controller/internal/sync/sync.go
index 0457eea..df4027e 100644
--- a/controller/internal/sync/sync.go
+++ b/controller/internal/sync/sync.go
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"crypto/sha256"
+ "encoding/base64"
"encoding/hex"
"fmt"
"io"
@@ -300,7 +301,10 @@ func (s *Syncer) gitCloneOrPull() error {
// Clone
s.logger.Printf("[INFO] [sync] Cloning %s → %s", s.cfg.Git.RepoURL, s.cacheDir)
args := []string{"clone", "--depth", "1", "--branch", s.cfg.Git.Branch}
- repoURL := s.buildRepoURL()
+ // R-616: clone the PLAIN URL — git persists the clone URL as `origin`, so a credentialed URL
+ // left the token readable in .git/config and printed by any `git remote -v`. The credentials
+ // ride per command in the environment instead (gitAuthEnv).
+ repoURL := s.cfg.Git.RepoURL
args = append(args, repoURL, s.cacheDir)
if s.isDebug() {
s.logger.Printf("[DEBUG] [sync] git clone URL: %s, branch: %s, cacheDir: %s", maskRepoURL(repoURL), s.cfg.Git.Branch, s.cacheDir)
@@ -312,22 +316,25 @@ func (s *Syncer) gitCloneOrPull() error {
s.removeGitLockFiles()
// R-615: the clone remembers the repository it was made from. A changed `git.repo_url` used to be
- // INERT — every later fetch went to the stored origin and reported success. Compare and follow:
- // a different repository (credentials aside) → drop the cache and clone the new one; the same
- // repository with different credentials (a rotated token) → point origin at the new URL.
+ // INERT — every later fetch went to the stored origin and reported success. Compare and follow: a
+ // different repository (credentials aside) → drop the cache and clone the new one.
+ // R-616: credentials are NEVER part of the stored origin (they ride per command, gitAuthEnv), so the
+ // comparison is between credential-free forms, and a stored origin that still carries a
+ // `user:token@` (a clone made before R-616) is rewritten without it. A rotated token therefore needs
+ // no origin change at all. Pinned by TestR616_TokenSetSameRepoNoRecloneOriginClean.
if cur, err := s.gitOutput(s.cacheDir, "config", "--get", "remote.origin.url"); err != nil {
s.logger.Printf("[WARN] [sync] cannot read the catalog cache's origin (%v) — fetching from it as before", err)
- } else if want := s.buildRepoURL(); cur != want {
- if stripURLCreds(cur) != stripURLCreds(want) {
- s.logger.Printf("[WARN] [sync] git.repo_url changed (cache was cloned from %s, config says %s) — re-cloning the catalog cache from the configured repository (R-615)", maskRepoURL(cur), maskRepoURL(want))
- if err := os.RemoveAll(s.cacheDir); err != nil {
- return fmt.Errorf("removing the catalog cache for a re-clone: %w", err)
- }
- return s.gitCloneOrPull()
+ } else if want := s.cfg.Git.RepoURL; stripURLCreds(cur) != stripURLCreds(want) {
+ s.logger.Printf("[WARN] [sync] git.repo_url changed (cache was cloned from %s, config says %s) — re-cloning the catalog cache from the configured repository (R-615)", maskRepoURL(cur), maskRepoURL(want))
+ if err := os.RemoveAll(s.cacheDir); err != nil {
+ return fmt.Errorf("removing the catalog cache for a re-clone: %w", err)
}
- s.logger.Printf("[INFO] [sync] catalog repository credentials changed — updating the cache's origin (R-615)")
- if err := s.gitCmd(s.cacheDir, "remote", "set-url", "origin", want); err != nil {
- return fmt.Errorf("git remote set-url: %w", err)
+ return s.gitCloneOrPull()
+ } else if clean := stripURLCreds(cur); clean != cur {
+ if err := s.gitCmd(s.cacheDir, "remote", "set-url", "origin", clean); err != nil {
+ s.logger.Printf("[WARN] [sync] could not remove stored credentials from the catalog clone origin (%s): %v", maskRepoURL(cur), err)
+ } else {
+ s.logger.Printf("[INFO] [sync] removed stored credentials from the catalog clone origin (R-616): now %s", clean)
}
}
@@ -362,14 +369,50 @@ func (s *Syncer) removeGitLockFiles() {
}
}
-// buildRepoURL constructs the repo URL with optional auth credentials.
-func (s *Syncer) buildRepoURL() string {
- url := s.cfg.Git.RepoURL
- if s.cfg.Git.Username != "" && s.cfg.Git.Token != "" {
- // Inject credentials into HTTPS URL: https://user:token@host/path
- url = strings.Replace(url, "https://", fmt.Sprintf("https://%s:%s@", s.cfg.Git.Username, s.cfg.Git.Token), 1)
+// gitAuthEnv returns the environment that authenticates one git command, or nil when no credentials
+// are configured or the remote is not HTTPS.
+//
+// R-616: the credentials used to be injected into the clone URL (https://user:token@host/…), which
+// git then stored as the clone's `origin` — a plaintext token in /catalog-cache/.git/config,
+// printed by every `git remote -v`. They now travel as an `http..extraHeader`
+// carrying the same HTTP Basic credentials the URL form sent, set through GIT_CONFIG_COUNT (git ≥
+// 2.31; the runtime image is bookworm, 2.39). The environment is neither persisted by git nor in
+// the process argv nor in any log line here. The header is scoped to the remote's own origin so a
+// redirect to another host never receives it. Pinned by TestR616_CloneStoresNoCredentials.
+func (s *Syncer) gitAuthEnv() []string {
+ u, p := s.cfg.Git.Username, s.cfg.Git.Token
+ if u == "" || p == "" {
+ return nil
}
- return url
+ origin := httpsOrigin(s.cfg.Git.RepoURL)
+ if origin == "" {
+ return nil
+ }
+ basic := base64.StdEncoding.EncodeToString([]byte(u + ":" + p))
+ return []string{
+ "GIT_CONFIG_COUNT=1",
+ "GIT_CONFIG_KEY_0=http." + origin + ".extraHeader",
+ "GIT_CONFIG_VALUE_0=Authorization: Basic " + basic,
+ }
+}
+
+// httpsOrigin returns "https://host[:port]/" for an https URL (userinfo dropped), or "" otherwise.
+func httpsOrigin(raw string) string {
+ rest, ok := strings.CutPrefix(raw, "https://")
+ if !ok {
+ return ""
+ }
+ host := rest
+ if i := strings.IndexByte(rest, '/'); i >= 0 {
+ host = rest[:i]
+ }
+ if i := strings.LastIndexByte(host, '@'); i >= 0 {
+ host = host[i+1:]
+ }
+ if host == "" {
+ return ""
+ }
+ return "https://" + host + "/"
}
// copyTemplates copies docker-compose.yml and .felhom.yml from the catalog cache
@@ -696,6 +739,9 @@ func (s *Syncer) runGitInDir(ctx context.Context, dir string, args ...string) er
if dir != "" {
cmd.Dir = dir
}
+ if env := s.gitAuthEnv(); env != nil {
+ cmd.Env = append(os.Environ(), env...)
+ }
var stderr bytes.Buffer
cmd.Stdout = io.Discard