From 1f0862b2878fd256b9c7b073a98339c7fae56757 Mon Sep 17 00:00:00 2001 From: user-fPoq4eSbDgj__95j Date: Wed, 28 Oct 2026 04:40:47 +0000 Subject: [PATCH 1/4] fix(security): sanitize cached git mirrors --- internal/gitclone/command.go | 6 +- internal/gitclone/manager.go | 166 +++++++++++++++++---- internal/gitclone/manager_test.go | 55 ++++++- internal/strategy/git/backend.go | 2 + internal/strategy/git/info_refs_test.go | 3 +- internal/strategy/git/lfs_security_test.go | 78 ++++++++++ internal/strategy/git/refs_test.go | 1 + internal/strategy/git/snapshot.go | 3 +- internal/strategy/git/snapshot_test.go | 5 + internal/strategy/gomod/private_fetcher.go | 1 + 10 files changed, 282 insertions(+), 38 deletions(-) diff --git a/internal/gitclone/command.go b/internal/gitclone/command.go index 891b2314..4c755a46 100644 --- a/internal/gitclone/command.go +++ b/internal/gitclone/command.go @@ -5,14 +5,14 @@ package gitclone import ( "bufio" "context" + "os" "os/exec" "strings" "github.com/alecthomas/errors" ) -// GitCommand returns a git subprocess configured with repository-scoped -// authentication and any per-URL git config overrides disabled. +// GitCommand returns a Git subprocess with repository hooks disabled. func (r *Repository) GitCommand(ctx context.Context, args ...string) (*exec.Cmd, error) { repoURL := r.upstreamURL var token string @@ -30,7 +30,7 @@ func (r *Repository) GitCommand(ctx context.Context, args ...string) (*exec.Cmd, return nil, errors.Wrap(err, "get insteadOf disable args") } - var allArgs []string + allArgs := []string{"-c", "core.hooksPath=" + os.DevNull} if len(configArgs) > 0 { allArgs = append(allArgs, configArgs...) } diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index a75ee270..1608a21f 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -180,7 +180,7 @@ func (m *Manager) Config() Config { return m.config } -func (m *Manager) GetOrCreate(_ context.Context, upstreamURL string) (*Repository, error) { +func (m *Manager) GetOrCreate(ctx context.Context, upstreamURL string) (*Repository, error) { m.clonesMu.RLock() repo, exists := m.clones[upstreamURL] m.clonesMu.RUnlock() @@ -211,8 +211,15 @@ func (m *Manager) GetOrCreate(_ context.Context, upstreamURL string) (*Repositor } headFile := filepath.Join(clonePath, "HEAD") - if _, err := os.Stat(headFile); err == nil { - repo.state = StateReady + if _, err := os.Lstat(headFile); err == nil { + if err := repo.ConfigureMirror(ctx, clonePath); err != nil { + if removeErr := os.RemoveAll(clonePath); removeErr != nil { + return nil, errors.Join(errors.Wrap(err, "sanitize existing mirror"), + errors.Wrap(removeErr, "remove unsafe existing mirror")) + } + } else { + repo.state = StateReady + } } repo.fetchSem <- struct{}{} @@ -267,32 +274,14 @@ func (m *Manager) DiscoverExisting(ctx context.Context) ([]*Repository, error) { } upstreamURL := "https://" + host + "/" + repoPath - repo := &Repository{ - state: StateReady, - config: m.config, - path: path, - upstreamURL: upstreamURL, - fetchSem: make(chan struct{}, 1), - credentialProvider: m.credentialProvider, - } - repo.fetchSem <- struct{}{} - - if err := configureMirror(ctx, path, m.config.PackThreads); err != nil { - return errors.Wrapf(err, "configure mirror for %s", upstreamURL) + repo, err := m.GetOrCreate(ctx, upstreamURL) + if err != nil { + return errors.Wrapf(err, "discover mirror for %s", upstreamURL) } - - if m.config.Maintenance { - if err := registerMaintenance(ctx, path); err != nil { - return errors.Wrapf(err, "register maintenance for %s", upstreamURL) - } + if repo.State() == StateReady { + discovered = append(discovered, repo) } - m.clonesMu.Lock() - m.clones[upstreamURL] = repo - m.clonesMu.Unlock() - - discovered = append(discovered, repo) - return fs.SkipDir }) @@ -310,7 +299,24 @@ func RepoPathFromURL(upstreamURL string) (string, error) { if err != nil { return "", errors.Wrap(err, "parse upstream URL") } - return filepath.Join(parsed.Host, strings.TrimSuffix(parsed.Path, ".git")), nil + if parsed.Scheme != "https" || parsed.Host == "" || parsed.User != nil || parsed.RawQuery != "" || + parsed.Fragment != "" || strings.ContainsAny(parsed.Host, `/\`) { + return "", errors.Errorf("unsupported upstream URL %q", upstreamURL) + } + repoPath := strings.TrimSuffix(strings.TrimSuffix(strings.TrimPrefix(parsed.Path, "/"), "/"), ".git") + if repoPath == "" { + return "", errors.Errorf("upstream URL %q has no repository path", upstreamURL) + } + for segment := range strings.SplitSeq(repoPath, "/") { + if segment == "" || segment == "." || segment == ".." || strings.Contains(segment, `\`) { + return "", errors.Errorf("unsupported upstream URL path %q", parsed.Path) + } + } + repoPath = filepath.Join(parsed.Host, filepath.FromSlash(repoPath)) + if !filepath.IsLocal(repoPath) { + return "", errors.Errorf("upstream URL path %q is not local", parsed.Path) + } + return repoPath, nil } func (m *Manager) clonePathForURL(upstreamURL string) (string, error) { @@ -400,9 +406,11 @@ func (r *Repository) WithFetchExclusion(ctx context.Context, fn func() error) er } } -// ConfigureMirror configures a git directory at repoPath as a mirror with -// the repository's pack threads and optional maintenance settings. +// ConfigureMirror sanitizes and configures a git directory at repoPath as a mirror. func (r *Repository) ConfigureMirror(ctx context.Context, repoPath string) error { + if err := resetMirrorMetadata(ctx, repoPath, r.upstreamURL); err != nil { + return errors.Wrap(err, "reset mirror metadata") + } if err := configureMirror(ctx, repoPath, r.config.PackThreads); err != nil { return errors.Wrap(err, "configure mirror") } @@ -414,6 +422,106 @@ func (r *Repository) ConfigureMirror(ctx context.Context, repoPath string) error return nil } +func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) error { + configPath := filepath.Join(repoPath, "config") + info, err := os.Lstat(configPath) + if err != nil { + return errors.Wrap(err, "stat mirror config") + } + if !info.Mode().IsRegular() { + return errors.New("mirror config is not a regular file") + } + configValues, err := mirrorConfigValues(ctx, configPath) + if err != nil { + return err + } + objectFormat := configValues["extensions.objectformat"] + if objectFormat != "" && objectFormat != "sha1" && objectFormat != "sha256" { + return errors.Errorf("unsupported mirror object format %q", objectFormat) + } + refStorage := configValues["extensions.refstorage"] + if refStorage != "" && refStorage != "files" && refStorage != "reftable" { + return errors.Errorf("unsupported mirror ref storage %q", refStorage) + } + if err := filepath.WalkDir(repoPath, func(path string, entry fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.Type()&os.ModeSymlink != 0 || (!entry.IsDir() && !entry.Type().IsRegular()) { + return errors.Errorf("unsafe mirror entry %s", path) + } + return nil + }); err != nil { + return errors.Wrap(err, "validate mirror entries") + } + + for _, path := range []string{ + filepath.Join(repoPath, ".git"), + filepath.Join(repoPath, ".lfsconfig"), + filepath.Join(repoPath, "commondir"), + filepath.Join(repoPath, "config"), + filepath.Join(repoPath, "config.worktree"), + filepath.Join(repoPath, "hooks"), + filepath.Join(repoPath, "index"), + filepath.Join(repoPath, "objects", "info", "alternates"), + filepath.Join(repoPath, "objects", "info", "http-alternates"), + filepath.Join(repoPath, "modules"), + filepath.Join(repoPath, "worktrees"), + } { + if err := os.RemoveAll(path); err != nil { + return errors.Wrapf(err, "remove unsafe mirror metadata %s", path) + } + } + + repositoryFormat := "0" + if objectFormat != "" || refStorage != "" { + repositoryFormat = "1" + } + escapedRemoteURL, err := gitConfigValue(upstreamURL) + if err != nil { + return err + } + config := "[core]\n\trepositoryformatversion = " + repositoryFormat + "\n\tfilemode = true\n\tbare = true\n" + + "[remote \"origin\"]\n\turl = " + escapedRemoteURL + "\n\tfetch = +refs/*:refs/*\n\tmirror = true\n" + if objectFormat != "" || refStorage != "" { + config += "[extensions]\n" + } + if objectFormat != "" { + config += "\tobjectFormat = " + objectFormat + "\n" + } + if refStorage != "" { + config += "\trefStorage = " + refStorage + "\n" + } + if err := os.WriteFile(configPath, []byte(config), 0o600); err != nil { + return errors.Wrap(err, "create mirror config") + } + return nil +} + +func gitConfigValue(value string) (string, error) { + if strings.ContainsAny(value, "\x00\r\n") { + return "", errors.New("mirror origin contains a control character") + } + value = strings.ReplaceAll(strings.ReplaceAll(value, `\`, `\\`), `"`, `\"`) + return `"` + value + `"`, nil +} + +func mirrorConfigValues(ctx context.Context, configPath string) (map[string]string, error) { + cmd := exec.CommandContext(ctx, "git", "config", "--file", configPath, "--no-includes", "--null", "--list") // #nosec G204 + output, err := cmd.CombinedOutput() + if err != nil { + return nil, errors.Wrapf(err, "read mirror config: %s", string(output)) + } + values := map[string]string{} + for entry := range strings.SplitSeq(string(output), "\x00") { + key, value, ok := strings.Cut(entry, "\n") + if ok { + values[strings.ToLower(key)] = value + } + } + return values, nil +} + // MarkReady transitions the repository to StateReady, indicating it is // up-to-date and can serve requests from the local mirror. func (r *Repository) MarkReady() { diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index 8557483d..3cfa4b72 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -120,6 +120,54 @@ func TestManager_GetOrCreate(t *testing.T) { assert.True(t, repo == repo2, "expected same repository instance") } +func TestRepoPathFromURLRejectsTraversal(t *testing.T) { + path, err := RepoPathFromURL("https://github.com/org/repo.git") + assert.NoError(t, err) + assert.Equal(t, filepath.Join("github.com", "org", "repo"), path) + + for _, upstream := range []string{ + "http://github.com/org/repo", + "https://user@github.com/org/repo", + "https://github.com/org/repo?x", + "https://github.com/../../tmp/repo", + "https://github.com/%2e%2e/%2e%2e/tmp/repo", + "https://github.com/org/repo%2f..%2f..%2ftmp/repo", + } { + _, err := RepoPathFromURL(upstream) + assert.Error(t, err) + } +} + +func TestConfigureMirrorSanitizesExecutableMetadata(t *testing.T) { + _, ctx := logging.Configure(t.Context(), logging.Config{Level: slog.LevelError}) + repoPath := createBareRepo(t, t.TempDir()) + hookPath := filepath.Join(repoPath, "hooks", "reference-transaction") + proofPath := filepath.Join(t.TempDir(), "proof") + assert.NoError(t, os.WriteFile(hookPath, []byte("#!/bin/sh\ntouch \""+proofPath+"\"\n"), 0o755)) + assert.NoError(t, exec.Command("git", "-C", repoPath, "config", "remote.origin.url", "https://github.com/org/repo").Run()) + assert.NoError(t, exec.Command("git", "-C", repoPath, "config", "credential.helper", "!touch "+proofPath).Run()) + assert.NoError(t, exec.Command("git", "-C", repoPath, "config", "core.alternateRefsCommand", "touch "+proofPath).Run()) + assert.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects", "info", "alternates"), []byte(repoPath+"/objects\n"), 0o600)) + assert.NoError(t, os.WriteFile(filepath.Join(repoPath, "commondir"), []byte("payload\n"), 0o600)) + + repo := &Repository{config: testRepoConfig(), upstreamURL: "https://github.com/org/repo"} + assert.NoError(t, repo.ConfigureMirror(ctx, repoPath)) + + _, err := os.Lstat(hookPath) + assert.True(t, os.IsNotExist(err)) + _, err = os.Lstat(filepath.Join(repoPath, "objects", "info", "alternates")) + assert.True(t, os.IsNotExist(err)) + _, err = os.Lstat(filepath.Join(repoPath, "commondir")) + assert.True(t, os.IsNotExist(err)) + cmd := exec.Command("git", "-C", repoPath, "config", "--get", "credential.helper") + assert.Error(t, cmd.Run()) + output, err := exec.Command("git", "-C", repoPath, "config", "--get", "remote.origin.url").Output() + assert.NoError(t, err) + assert.Equal(t, "https://github.com/org/repo", strings.TrimSpace(string(output))) + _, err = os.Lstat(proofPath) + assert.True(t, os.IsNotExist(err)) +} + func TestManager_GetOrCreate_ExistingClone(t *testing.T) { _, ctx := logging.Configure(t.Context(), logging.Config{Level: slog.LevelError}) tmpDir := t.TempDir() @@ -133,8 +181,7 @@ func TestManager_GetOrCreate_ExistingClone(t *testing.T) { assert.NoError(t, err) repoPath := filepath.Join(tmpDir, "github.com", "user", "repo") - assert.NoError(t, os.MkdirAll(repoPath, 0o755)) - assert.NoError(t, os.WriteFile(filepath.Join(repoPath, "HEAD"), []byte("ref: refs/heads/main\n"), 0o644)) + assert.NoError(t, exec.Command("git", "init", "--bare", repoPath).Run()) upstreamURL := "https://github.com/user/repo" repo, err := manager.GetOrCreate(context.Background(), upstreamURL) @@ -763,7 +810,7 @@ func TestRepositoryEnsureRefsUpToDateDoesNotCacheInFlightCheck(t *testing.T) { started := filepath.Join(binDir, "started") release := filepath.Join(binDir, "release") script := fmt.Sprintf(`#!/bin/sh -if [ "$1" = "ls-remote" ]; then +if echo " $* " | grep -q ' ls-remote '; then touch %q while [ ! -e %q ]; do sleep 0.01; done fi @@ -839,7 +886,7 @@ func TestRepositoryEnsureRefsUpToDateConcurrentStaleResultInvalidatesFreshResult staleStarted := filepath.Join(binDir, "stale-started") releaseStale := filepath.Join(binDir, "release-stale") script := fmt.Sprintf(`#!/bin/sh -if [ "$1" = "ls-remote" ]; then +if echo " $* " | grep -q ' ls-remote '; then if mkdir %q 2>/dev/null; then touch %q while [ ! -e %q ]; do sleep 0.01; done diff --git a/internal/strategy/git/backend.go b/internal/strategy/git/backend.go index 4c70d179..9a13dea9 100644 --- a/internal/strategy/git/backend.go +++ b/internal/strategy/git/backend.go @@ -146,6 +146,8 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, repo "GIT_PROJECT_ROOT=" + absRoot, "GIT_HTTP_EXPORT_ALL=1", "GIT_HTTP_MAX_REQUEST_BUFFER=100M", + "GIT_CONFIG_NOSYSTEM=1", + "GIT_CONFIG_GLOBAL=" + os.DevNull, "PATH=" + os.Getenv("PATH"), }, } diff --git a/internal/strategy/git/info_refs_test.go b/internal/strategy/git/info_refs_test.go index affc4912..fb5f419a 100644 --- a/internal/strategy/git/info_refs_test.go +++ b/internal/strategy/git/info_refs_test.go @@ -82,6 +82,7 @@ func newInfoRefsFixture(t *testing.T) *infoRefsFixture { func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil assert.NoError(t, err) waitForReady(t, strategy) + runInfoRefsGit(t, "-C", mirrorPath, "config", "remote.origin.url", upstreamPath) transport := &infoRefsTransport{} strategy.SetHTTPTransport(transport) @@ -121,7 +122,7 @@ func setInfoRefsLsRemote(t *testing.T, target string, fail bool) { exit 1` } script := fmt.Sprintf(`#!/bin/sh -if [ "$1" = "ls-remote" ]; then +if echo " $* " | grep -q ' ls-remote '; then %s fi exec %q "$@" diff --git a/internal/strategy/git/lfs_security_test.go b/internal/strategy/git/lfs_security_test.go index 3f2a67e5..75a0dd59 100644 --- a/internal/strategy/git/lfs_security_test.go +++ b/internal/strategy/git/lfs_security_test.go @@ -1,12 +1,20 @@ package git //nolint:testpackage // white-box testing is required for process-boundary hardening import ( + "context" "os" + "os/exec" "path/filepath" "runtime" "testing" + "time" "github.com/alecthomas/assert/v2" + + "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/gitclone" + "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/snapshot" ) func TestGitLFSVersionSupported(t *testing.T) { @@ -60,3 +68,73 @@ func TestReplaceRepositoryLFSConfigDoesNotFollowSymlink(t *testing.T) { assert.NoError(t, err) assert.False(t, info.Mode()&os.ModeSymlink != 0) } + +func TestRestoreMirrorSnapshotSanitizesCachedCode(t *testing.T) { + _, ctx := logging.Configure(context.Background(), logging.Config{}) + upstream := "https://github.com/org/repo" + manager, err := gitclone.NewManager(ctx, gitclone.Config{MirrorRoot: t.TempDir()}, nil) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(ctx, upstream) + assert.NoError(t, err) + + archiveDir := filepath.Join(t.TempDir(), "repo") + assert.NoError(t, exec.Command("git", "init", "--bare", archiveDir).Run()) + proofPath := filepath.Join(t.TempDir(), "proof") + assert.NoError(t, os.WriteFile(filepath.Join(archiveDir, "hooks", "reference-transaction"), + []byte("#!/bin/sh\ntouch \""+proofPath+"\"\n"), 0o755)) + assert.NoError(t, exec.Command("git", "-C", archiveDir, "config", "credential.helper", "!touch "+proofPath).Run()) + + mem, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) + assert.NoError(t, err) + assert.NoError(t, snapshot.Create(ctx, mem, mirrorSnapshotCacheKey(upstream), archiveDir, time.Hour, nil, 1)) + strategy := &Strategy{cache: mem, cloneManager: manager, config: Config{ZstdThreads: 1}} + assert.NoError(t, strategy.tryRestoreSnapshot(ctx, repo)) + + _, err = os.Lstat(filepath.Join(repo.Path(), "hooks", "reference-transaction")) + assert.True(t, os.IsNotExist(err)) + cmd := exec.Command("git", "-C", repo.Path(), "config", "--get", "credential.helper") + assert.Error(t, cmd.Run()) + _, err = os.Lstat(proofPath) + assert.True(t, os.IsNotExist(err)) +} + +func TestCloneForSnapshotIgnoresGlobalFilters(t *testing.T) { + _, ctx := logging.Configure(t.Context(), logging.Config{}) + workDir := t.TempDir() + for _, args := range [][]string{ + {"git", "-C", workDir, "init"}, + {"git", "-C", workDir, "config", "user.email", "test@example.com"}, + {"git", "-C", workDir, "config", "user.name", "Test"}, + } { + assert.NoError(t, exec.Command(args[0], args[1:]...).Run()) + } + assert.NoError(t, os.WriteFile(filepath.Join(workDir, ".gitattributes"), []byte("* filter=poc\n"), 0o600)) + assert.NoError(t, os.WriteFile(filepath.Join(workDir, "data"), []byte("cached code\n"), 0o600)) + for _, args := range [][]string{ + {"git", "-C", workDir, "add", "."}, + {"git", "-C", workDir, "commit", "-m", "initial"}, + } { + assert.NoError(t, exec.Command(args[0], args[1:]...).Run()) + } + + mirrorRoot := t.TempDir() + mirrorPath := filepath.Join(mirrorRoot, "github.com", "org", "repo") + assert.NoError(t, exec.Command("git", "clone", "--mirror", workDir, mirrorPath).Run()) + manager, err := gitclone.NewManager(ctx, gitclone.Config{MirrorRoot: mirrorRoot}, nil) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(ctx, "https://github.com/org/repo") + assert.NoError(t, err) + + proofPath := filepath.Join(t.TempDir(), "proof") + filterPath := filepath.Join(t.TempDir(), "filter") + assert.NoError(t, os.WriteFile(filterPath, []byte("#!/bin/sh\ntouch \""+proofPath+"\"\nexit 1\n"), 0o700)) + configPath := filepath.Join(t.TempDir(), "gitconfig") + assert.NoError(t, os.WriteFile(configPath, []byte("[filter \"poc\"]\n\tprocess = "+filterPath+"\n\trequired = true\n"), 0o600)) + t.Setenv("GIT_CONFIG_GLOBAL", configPath) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + + strategy := &Strategy{cloneManager: manager} + assert.NoError(t, strategy.cloneForSnapshot(ctx, repo, filepath.Join(t.TempDir(), "snapshot"), "")) + _, err = os.Lstat(proofPath) + assert.True(t, os.IsNotExist(err)) +} diff --git a/internal/strategy/git/refs_test.go b/internal/strategy/git/refs_test.go index 67aae0eb..59f7759a 100644 --- a/internal/strategy/git/refs_test.go +++ b/internal/strategy/git/refs_test.go @@ -64,6 +64,7 @@ func TestEnsureRefsHandler(t *testing.T) { func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil assert.NoError(t, err) waitForReady(t, s) + assert.NoError(t, exec.Command("git", "-C", mirrorPath, "remote", "set-url", "origin", upstreamPath).Run()) handler := mux.handlers["POST /git/{host}/{path...}"] assert.NotZero(t, handler) diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index 0c7d35bc..01af089d 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -145,7 +145,8 @@ func (s *Strategy) cloneForSnapshot(ctx context.Context, repo *gitclone.Reposito cmd := exec.CommandContext(ctx, "git", args...) // LC_ALL=C pins git's messages to English so the filter fallback // warning below is detectable regardless of the host locale. - cmd.Env = append(os.Environ(), "GIT_LFS_SKIP_SMUDGE=1", "LC_ALL=C") + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, + "GIT_LFS_SKIP_SMUDGE=1", "LC_ALL=C") output, err := cmd.CombinedOutput() if err != nil { return errors.Wrapf(err, "git clone for snapshot: %s", string(output)) diff --git a/internal/strategy/git/snapshot_test.go b/internal/strategy/git/snapshot_test.go index 0ce1b998..db4a400f 100644 --- a/internal/strategy/git/snapshot_test.go +++ b/internal/strategy/git/snapshot_test.go @@ -1334,6 +1334,9 @@ func commitUpstream(t *testing.T, upstream, msg string) string { func newBundleTestStrategy(ctx context.Context, t *testing.T, mirrorRoot string, refCheckInterval time.Duration) *testMux { t.Helper() + mirrorPath := filepath.Join(mirrorRoot, "example.com", "org", "repo") + upstreamURL, err := exec.Command("git", "-C", mirrorPath, "remote", "get-url", "origin").Output() + assert.NoError(t, err) memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) assert.NoError(t, err) mux := newTestMux() @@ -1341,6 +1344,8 @@ func newBundleTestStrategy(ctx context.Context, t *testing.T, mirrorRoot string, s, err := git.New(ctx, git.Config{}, newTestScheduler(ctx, t), memCache, mux, cm, func() (*githubapp.TokenManager, error) { return nil, nil }) //nolint:nilnil assert.NoError(t, err) waitForReady(t, s) + assert.NoError(t, exec.Command("git", "-C", mirrorPath, "remote", "set-url", "origin", + strings.TrimSpace(string(upstreamURL))).Run()) return mux } diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index a91e026d..186fd837 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -291,6 +291,7 @@ func (p *privateFetcher) generateZip(ctx context.Context, repo *gitclone.Reposit } // #nosec G204 - cloneDir and gitRef are controlled by us cmd = exec.CommandContext(ctx, "git", "-C", cloneDir, "checkout", gitRef) + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull) if output, err := cmd.CombinedOutput(); err != nil { return nil, errors.Wrapf(err, "git checkout %s: %s", gitRef, string(output)) } From 3fb6908d3b09944c90aee791ef7da5256efb824b Mon Sep 17 00:00:00 2001 From: user-fPoq4eSbDgj__95j Date: Wed, 28 Oct 2026 05:15:40 +0000 Subject: [PATCH 2/4] docs(security): explain cached git trust boundaries --- internal/gitclone/command.go | 1 + internal/gitclone/manager.go | 4 ++++ internal/strategy/git/backend.go | 1 + internal/strategy/git/snapshot.go | 1 + internal/strategy/gomod/private_fetcher.go | 1 + 5 files changed, 8 insertions(+) diff --git a/internal/gitclone/command.go b/internal/gitclone/command.go index 4c755a46..dfaae0b4 100644 --- a/internal/gitclone/command.go +++ b/internal/gitclone/command.go @@ -30,6 +30,7 @@ func (r *Repository) GitCommand(ctx context.Context, args ...string) (*exec.Cmd, return nil, errors.Wrap(err, "get insteadOf disable args") } + // Cache operations do not need hooks, and enforcing this at execution prevents restored hook files from running. allArgs := []string{"-c", "core.hooksPath=" + os.DevNull} if len(configArgs) > 0 { allArgs = append(allArgs, configArgs...) diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 1608a21f..12a1eb9f 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -212,6 +212,7 @@ func (m *Manager) GetOrCreate(ctx context.Context, upstreamURL string) (*Reposit headFile := filepath.Join(clonePath, "HEAD") if _, err := os.Lstat(headFile); err == nil { + // Mirrors can survive from an older process or cache restore, so they must cross the same trust boundary as new restores. if err := repo.ConfigureMirror(ctx, clonePath); err != nil { if removeErr := os.RemoveAll(clonePath); removeErr != nil { return nil, errors.Join(errors.Wrap(err, "sanitize existing mirror"), @@ -431,6 +432,7 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro if !info.Mode().IsRegular() { return errors.New("mirror config is not a regular file") } + // Cached metadata can invoke commands; retain only storage features needed to read objects, then rebuild trusted settings. configValues, err := mirrorConfigValues(ctx, configPath) if err != nil { return err @@ -443,6 +445,7 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro if refStorage != "" && refStorage != "files" && refStorage != "reftable" { return errors.Errorf("unsupported mirror ref storage %q", refStorage) } + // Cleanup must not resolve through a cached symlink and remove files outside the mirror. if err := filepath.WalkDir(repoPath, func(path string, entry fs.DirEntry, walkErr error) error { if walkErr != nil { return walkErr @@ -455,6 +458,7 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro return errors.Wrap(err, "validate mirror entries") } + // Git interprets these paths as configuration, hooks, or object-store indirection rather than inert cached data. for _, path := range []string{ filepath.Join(repoPath, ".git"), filepath.Join(repoPath, ".lfsconfig"), diff --git a/internal/strategy/git/backend.go b/internal/strategy/git/backend.go index 9a13dea9..a475e73a 100644 --- a/internal/strategy/git/backend.go +++ b/internal/strategy/git/backend.go @@ -138,6 +138,7 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, repo var stderrBuf bytes.Buffer + // Host Git configuration is unrelated to serving mirrors and can add command-backed upload-pack behavior. handler := &cgi.Handler{ Path: gitPath, Args: []string{"http-backend"}, diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index 01af089d..fe3556e7 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -143,6 +143,7 @@ func (s *Strategy) cloneForSnapshot(ctx context.Context, repo *gitclone.Reposito args = append(args, repo.Path(), destDir) // #nosec G204 - repo.Path() and destDir are controlled by us cmd := exec.CommandContext(ctx, "git", args...) + // Repository files choose checkout filters, so host-defined filters must not become an execution path for cached data. // LC_ALL=C pins git's messages to English so the filter fallback // warning below is detectable regardless of the host locale. cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index 186fd837..dcc6b998 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -291,6 +291,7 @@ func (p *privateFetcher) generateZip(ctx context.Context, repo *gitclone.Reposit } // #nosec G204 - cloneDir and gitRef are controlled by us cmd = exec.CommandContext(ctx, "git", "-C", cloneDir, "checkout", gitRef) + // Module files choose checkout filters, so host-defined process filters must not execute while assembling the cached zip. cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull) if output, err := cmd.CombinedOutput(); err != nil { return nil, errors.Wrapf(err, "git checkout %s: %s", gitRef, string(output)) From 05eeb4377ad1c7b73b7c930bbe0f267129d99813 Mon Sep 17 00:00:00 2001 From: user-fPoq4eSbDgj__95j Date: Wed, 28 Oct 2026 05:37:53 +0000 Subject: [PATCH 3/4] fix(security): address git mirror review findings --- internal/gitclone/manager.go | 21 ++++++++++++++++----- internal/gitclone/manager_test.go | 21 +++++++++++++++++++++ internal/strategy/git/lfs_security_test.go | 6 ++++++ internal/strategy/git/snapshot.go | 2 +- internal/strategy/gomod/private_fetcher.go | 2 +- 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 12a1eb9f..102d15f4 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -21,6 +21,8 @@ import ( type State int +var errUnsafeMirrorMetadata = errors.New("unsafe mirror metadata") + const ( StateEmpty State = iota // Not cloned yet StateCloning // Clone in progress @@ -213,11 +215,16 @@ func (m *Manager) GetOrCreate(ctx context.Context, upstreamURL string) (*Reposit headFile := filepath.Join(clonePath, "HEAD") if _, err := os.Lstat(headFile); err == nil { // Mirrors can survive from an older process or cache restore, so they must cross the same trust boundary as new restores. - if err := repo.ConfigureMirror(ctx, clonePath); err != nil { + if err := resetMirrorMetadata(ctx, clonePath, repo.upstreamURL); err != nil { + if !errors.Is(err, errUnsafeMirrorMetadata) { + return nil, errors.Wrap(err, "sanitize existing mirror") + } if removeErr := os.RemoveAll(clonePath); removeErr != nil { return nil, errors.Join(errors.Wrap(err, "sanitize existing mirror"), errors.Wrap(removeErr, "remove unsafe existing mirror")) } + } else if err := repo.configureMirror(ctx, clonePath); err != nil { + return nil, errors.Wrap(err, "configure existing mirror") } else { repo.state = StateReady } @@ -412,6 +419,10 @@ func (r *Repository) ConfigureMirror(ctx context.Context, repoPath string) error if err := resetMirrorMetadata(ctx, repoPath, r.upstreamURL); err != nil { return errors.Wrap(err, "reset mirror metadata") } + return r.configureMirror(ctx, repoPath) +} + +func (r *Repository) configureMirror(ctx context.Context, repoPath string) error { if err := configureMirror(ctx, repoPath, r.config.PackThreads); err != nil { return errors.Wrap(err, "configure mirror") } @@ -430,7 +441,7 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro return errors.Wrap(err, "stat mirror config") } if !info.Mode().IsRegular() { - return errors.New("mirror config is not a regular file") + return errors.Wrap(errUnsafeMirrorMetadata, "mirror config is not a regular file") } // Cached metadata can invoke commands; retain only storage features needed to read objects, then rebuild trusted settings. configValues, err := mirrorConfigValues(ctx, configPath) @@ -439,11 +450,11 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro } objectFormat := configValues["extensions.objectformat"] if objectFormat != "" && objectFormat != "sha1" && objectFormat != "sha256" { - return errors.Errorf("unsupported mirror object format %q", objectFormat) + return errors.Wrapf(errUnsafeMirrorMetadata, "unsupported mirror object format %q", objectFormat) } refStorage := configValues["extensions.refstorage"] if refStorage != "" && refStorage != "files" && refStorage != "reftable" { - return errors.Errorf("unsupported mirror ref storage %q", refStorage) + return errors.Wrapf(errUnsafeMirrorMetadata, "unsupported mirror ref storage %q", refStorage) } // Cleanup must not resolve through a cached symlink and remove files outside the mirror. if err := filepath.WalkDir(repoPath, func(path string, entry fs.DirEntry, walkErr error) error { @@ -451,7 +462,7 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro return walkErr } if entry.Type()&os.ModeSymlink != 0 || (!entry.IsDir() && !entry.Type().IsRegular()) { - return errors.Errorf("unsafe mirror entry %s", path) + return errors.Wrapf(errUnsafeMirrorMetadata, "unsafe mirror entry %s", path) } return nil }); err != nil { diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index 3cfa4b72..37a4d94f 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -191,6 +191,27 @@ func TestManager_GetOrCreate_ExistingClone(t *testing.T) { assert.Equal(t, StateReady, repo.State()) } +func TestManager_GetOrCreate_PreservesExistingCloneWhenSanitizationCanceled(t *testing.T) { + _, ctx := logging.Configure(t.Context(), logging.Config{Level: slog.LevelError}) + tmpDir := t.TempDir() + manager, err := NewManager(ctx, Config{MirrorRoot: tmpDir}, nil) + assert.NoError(t, err) + + repoPath := filepath.Join(tmpDir, "github.com", "user", "repo") + assert.NoError(t, exec.Command("git", "init", "--bare", repoPath).Run()) + canceledCtx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err = manager.GetOrCreate(canceledCtx, "https://github.com/user/repo") + assert.Error(t, err) + assert.True(t, errors.Is(err, context.Canceled)) + _, err = os.Stat(filepath.Join(repoPath, "HEAD")) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(context.Background(), "https://github.com/user/repo") + assert.NoError(t, err) + assert.Equal(t, StateReady, repo.State()) +} + func TestManager_Get(t *testing.T) { _, ctx := logging.Configure(t.Context(), logging.Config{Level: slog.LevelError}) tmpDir := t.TempDir() diff --git a/internal/strategy/git/lfs_security_test.go b/internal/strategy/git/lfs_security_test.go index 75a0dd59..336ea798 100644 --- a/internal/strategy/git/lfs_security_test.go +++ b/internal/strategy/git/lfs_security_test.go @@ -132,6 +132,12 @@ func TestCloneForSnapshotIgnoresGlobalFilters(t *testing.T) { assert.NoError(t, os.WriteFile(configPath, []byte("[filter \"poc\"]\n\tprocess = "+filterPath+"\n\trequired = true\n"), 0o600)) t.Setenv("GIT_CONFIG_GLOBAL", configPath) t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + t.Setenv("GIT_CONFIG_COUNT", "2") + t.Setenv("GIT_CONFIG_KEY_0", "filter.poc.process") + t.Setenv("GIT_CONFIG_VALUE_0", filterPath) + t.Setenv("GIT_CONFIG_KEY_1", "filter.poc.required") + t.Setenv("GIT_CONFIG_VALUE_1", "true") + t.Setenv("GIT_CONFIG_PARAMETERS", "'filter.poc.process'='"+filterPath+"' 'filter.poc.required'='true'") strategy := &Strategy{cloneManager: manager} assert.NoError(t, strategy.cloneForSnapshot(ctx, repo, filepath.Join(t.TempDir(), "snapshot"), "")) diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index fe3556e7..eaae783b 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -147,7 +147,7 @@ func (s *Strategy) cloneForSnapshot(ctx context.Context, repo *gitclone.Reposito // LC_ALL=C pins git's messages to English so the filter fallback // warning below is detectable regardless of the host locale. cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, - "GIT_LFS_SKIP_SMUDGE=1", "LC_ALL=C") + "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=", "GIT_LFS_SKIP_SMUDGE=1", "LC_ALL=C") output, err := cmd.CombinedOutput() if err != nil { return errors.Wrapf(err, "git clone for snapshot: %s", string(output)) diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index dcc6b998..c951e95f 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -292,7 +292,7 @@ func (p *privateFetcher) generateZip(ctx context.Context, repo *gitclone.Reposit // #nosec G204 - cloneDir and gitRef are controlled by us cmd = exec.CommandContext(ctx, "git", "-C", cloneDir, "checkout", gitRef) // Module files choose checkout filters, so host-defined process filters must not execute while assembling the cached zip. - cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull) + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=") if output, err := cmd.CombinedOutput(); err != nil { return nil, errors.Wrapf(err, "git checkout %s: %s", gitRef, string(output)) } From 594b968e407fdcfcccbb5939241293cf7f254b38 Mon Sep 17 00:00:00 2001 From: user-fPoq4eSbDgj__95j Date: Wed, 28 Oct 2026 05:58:16 +0000 Subject: [PATCH 4/4] fix(security): isolate git clone metadata --- internal/gitclone/manager.go | 55 ++++++++++++------- internal/strategy/git/lfs_security_test.go | 15 ++++- internal/strategy/git/snapshot.go | 6 +- internal/strategy/gomod/export_test.go | 6 ++ internal/strategy/gomod/private_fetcher.go | 11 ++-- .../gomod/private_fetcher_security_test.go | 54 ++++++++++++++++++ 6 files changed, 117 insertions(+), 30 deletions(-) create mode 100644 internal/strategy/gomod/private_fetcher_security_test.go diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 102d15f4..da44b001 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -469,25 +469,6 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro return errors.Wrap(err, "validate mirror entries") } - // Git interprets these paths as configuration, hooks, or object-store indirection rather than inert cached data. - for _, path := range []string{ - filepath.Join(repoPath, ".git"), - filepath.Join(repoPath, ".lfsconfig"), - filepath.Join(repoPath, "commondir"), - filepath.Join(repoPath, "config"), - filepath.Join(repoPath, "config.worktree"), - filepath.Join(repoPath, "hooks"), - filepath.Join(repoPath, "index"), - filepath.Join(repoPath, "objects", "info", "alternates"), - filepath.Join(repoPath, "objects", "info", "http-alternates"), - filepath.Join(repoPath, "modules"), - filepath.Join(repoPath, "worktrees"), - } { - if err := os.RemoveAll(path); err != nil { - return errors.Wrapf(err, "remove unsafe mirror metadata %s", path) - } - } - repositoryFormat := "0" if objectFormat != "" || refStorage != "" { repositoryFormat = "1" @@ -507,8 +488,40 @@ func resetMirrorMetadata(ctx context.Context, repoPath, upstreamURL string) erro if refStorage != "" { config += "\trefStorage = " + refStorage + "\n" } - if err := os.WriteFile(configPath, []byte(config), 0o600); err != nil { - return errors.Wrap(err, "create mirror config") + configFile, err := os.CreateTemp(repoPath, ".cachew-config-*") + if err != nil { + return errors.Wrap(err, "create temporary mirror config") + } + configTempPath := configFile.Name() + defer os.Remove(configTempPath) //nolint:errcheck // best-effort cleanup on failure + if _, err := configFile.WriteString(config); err != nil { + _ = configFile.Close() + return errors.Wrap(err, "write temporary mirror config") + } + if err := configFile.Close(); err != nil { + return errors.Wrap(err, "close temporary mirror config") + } + + // Git interprets these paths as configuration, hooks, or object-store indirection rather than inert cached data. + for _, path := range []string{ + filepath.Join(repoPath, ".git"), + filepath.Join(repoPath, ".lfsconfig"), + filepath.Join(repoPath, "commondir"), + filepath.Join(repoPath, "config.worktree"), + filepath.Join(repoPath, "hooks"), + filepath.Join(repoPath, "index"), + filepath.Join(repoPath, "objects", "info", "alternates"), + filepath.Join(repoPath, "objects", "info", "http-alternates"), + filepath.Join(repoPath, "modules"), + filepath.Join(repoPath, "worktrees"), + } { + if err := os.RemoveAll(path); err != nil { + return errors.Wrapf(err, "remove unsafe mirror metadata %s", path) + } + } + // #nosec G703 - both paths are created within the validated mirror directory + if err := os.Rename(configTempPath, configPath); err != nil { + return errors.Wrap(err, "replace mirror config") } return nil } diff --git a/internal/strategy/git/lfs_security_test.go b/internal/strategy/git/lfs_security_test.go index 336ea798..694934d4 100644 --- a/internal/strategy/git/lfs_security_test.go +++ b/internal/strategy/git/lfs_security_test.go @@ -98,7 +98,7 @@ func TestRestoreMirrorSnapshotSanitizesCachedCode(t *testing.T) { assert.True(t, os.IsNotExist(err)) } -func TestCloneForSnapshotIgnoresGlobalFilters(t *testing.T) { +func TestCloneForSnapshotIgnoresHostConfiguration(t *testing.T) { _, ctx := logging.Configure(t.Context(), logging.Config{}) workDir := t.TempDir() for _, args := range [][]string{ @@ -138,9 +138,20 @@ func TestCloneForSnapshotIgnoresGlobalFilters(t *testing.T) { t.Setenv("GIT_CONFIG_KEY_1", "filter.poc.required") t.Setenv("GIT_CONFIG_VALUE_1", "true") t.Setenv("GIT_CONFIG_PARAMETERS", "'filter.poc.process'='"+filterPath+"' 'filter.poc.required'='true'") + templateDir := t.TempDir() + assert.NoError(t, os.MkdirAll(filepath.Join(templateDir, "hooks"), 0o700)) + templateProofPath := filepath.Join(t.TempDir(), "template-proof") + assert.NoError(t, os.WriteFile(filepath.Join(templateDir, "hooks", "post-checkout"), + []byte("#!/bin/sh\ntouch \""+templateProofPath+"\"\n"), 0o700)) + t.Setenv("GIT_TEMPLATE_DIR", templateDir) strategy := &Strategy{cloneManager: manager} - assert.NoError(t, strategy.cloneForSnapshot(ctx, repo, filepath.Join(t.TempDir(), "snapshot"), "")) + snapshotDir := filepath.Join(t.TempDir(), "snapshot") + assert.NoError(t, strategy.cloneForSnapshot(ctx, repo, snapshotDir, "")) _, err = os.Lstat(proofPath) assert.True(t, os.IsNotExist(err)) + _, err = os.Lstat(templateProofPath) + assert.True(t, os.IsNotExist(err)) + _, err = os.Lstat(filepath.Join(snapshotDir, ".git", "hooks", "post-checkout")) + assert.True(t, os.IsNotExist(err)) } diff --git a/internal/strategy/git/snapshot.go b/internal/strategy/git/snapshot.go index eaae783b..94ea9fdc 100644 --- a/internal/strategy/git/snapshot.go +++ b/internal/strategy/git/snapshot.go @@ -132,7 +132,7 @@ func lfsSnapshotCacheKey(upstreamURL string) cache.Key { // metadata but only the blobs needed for the HEAD checkout. func (s *Strategy) cloneForSnapshot(ctx context.Context, repo *gitclone.Repository, destDir, filter string) error { if err := repo.WithReadLock(func() error { - args := []string{"clone"} + args := []string{"-c", "core.hooksPath=" + os.DevNull, "clone", "--template="} if filter != "" { // Local (hardlink) clones copy the entire object store and silently // ignore --filter; --no-local forces the transport path so the filter @@ -143,11 +143,11 @@ func (s *Strategy) cloneForSnapshot(ctx context.Context, repo *gitclone.Reposito args = append(args, repo.Path(), destDir) // #nosec G204 - repo.Path() and destDir are controlled by us cmd := exec.CommandContext(ctx, "git", args...) - // Repository files choose checkout filters, so host-defined filters must not become an execution path for cached data. + // Repository files choose checkout filters, while inherited templates can add hooks that run and leak into snapshots. // LC_ALL=C pins git's messages to English so the filter fallback // warning below is detectable regardless of the host locale. cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, - "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=", "GIT_LFS_SKIP_SMUDGE=1", "LC_ALL=C") + "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=", "GIT_TEMPLATE_DIR=", "GIT_LFS_SKIP_SMUDGE=1", "LC_ALL=C") output, err := cmd.CombinedOutput() if err != nil { return errors.Wrapf(err, "git clone for snapshot: %s", string(output)) diff --git a/internal/strategy/gomod/export_test.go b/internal/strategy/gomod/export_test.go index 15a35845..efa67f07 100644 --- a/internal/strategy/gomod/export_test.go +++ b/internal/strategy/gomod/export_test.go @@ -5,6 +5,7 @@ import ( "io" "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/gitclone" ) // TestableGoproxyCacher exposes goproxyCacher.Put for external tests. @@ -21,3 +22,8 @@ func NewTestableGoproxyCacher(c cache.Cache) *TestableGoproxyCacher { func (t *TestableGoproxyCacher) Put(ctx context.Context, name string, content io.ReadSeeker) error { return t.inner.Put(ctx, name, content) } + +// GenerateZipForTest lets external tests cover generation without widening the production API. +func GenerateZipForTest(ctx context.Context, repo *gitclone.Repository, modulePath, version string) (io.ReadSeekCloser, error) { + return (&privateFetcher{}).generateZip(ctx, repo, modulePath, version) +} diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index c951e95f..aaaf9cdf 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -285,14 +285,17 @@ func (p *privateFetcher) generateZip(ctx context.Context, repo *gitclone.Reposit // We use --no-checkout then checkout the specific ref so that both tagged versions and // raw commit hashes (from pseudo-versions) work; --branch only accepts branch/tag names. // #nosec G204 - repo.Path() and cloneDir are controlled by us - cmd := exec.CommandContext(ctx, "git", "clone", "--no-checkout", repo.Path(), cloneDir) + cmd := exec.CommandContext(ctx, "git", "-c", "core.hooksPath="+os.DevNull, "clone", "--template=", "--no-checkout", repo.Path(), cloneDir) + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, + "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=", "GIT_TEMPLATE_DIR=") if output, err := cmd.CombinedOutput(); err != nil { return nil, errors.Wrapf(err, "git clone: %s", string(output)) } // #nosec G204 - cloneDir and gitRef are controlled by us - cmd = exec.CommandContext(ctx, "git", "-C", cloneDir, "checkout", gitRef) - // Module files choose checkout filters, so host-defined process filters must not execute while assembling the cached zip. - cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=") + cmd = exec.CommandContext(ctx, "git", "-c", "core.hooksPath="+os.DevNull, "-C", cloneDir, "checkout", gitRef) + // Module files can activate host filters, and inherited templates can add hooks that checkout executes. + cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL="+os.DevNull, + "GIT_CONFIG_COUNT=0", "GIT_CONFIG_PARAMETERS=", "GIT_TEMPLATE_DIR=") if output, err := cmd.CombinedOutput(); err != nil { return nil, errors.Wrapf(err, "git checkout %s: %s", gitRef, string(output)) } diff --git a/internal/strategy/gomod/private_fetcher_security_test.go b/internal/strategy/gomod/private_fetcher_security_test.go new file mode 100644 index 00000000..430e13f1 --- /dev/null +++ b/internal/strategy/gomod/private_fetcher_security_test.go @@ -0,0 +1,54 @@ +package gomod_test + +import ( + "context" + "log/slog" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/alecthomas/assert/v2" + + "github.com/block/cachew/internal/gitclone" + "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/strategy/gomod" +) + +func TestGenerateZipIgnoresHostTemplates(t *testing.T) { + _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) + workDir := t.TempDir() + for _, args := range [][]string{ + {"-C", workDir, "init"}, + {"-C", workDir, "config", "user.email", "test@example.com"}, + {"-C", workDir, "config", "user.name", "Test"}, + } { + assert.NoError(t, exec.Command("git", args...).Run()) + } + assert.NoError(t, os.WriteFile(filepath.Join(workDir, "go.mod"), []byte("module example.com/mod\n"), 0o600)) + assert.NoError(t, exec.Command("git", "-C", workDir, "add", "go.mod").Run()) + assert.NoError(t, exec.Command("git", "-C", workDir, "commit", "-m", "initial").Run()) + assert.NoError(t, exec.Command("git", "-C", workDir, "tag", "v1.0.0").Run()) + + upstream := "https://example.com/mod" + mirrorRoot := t.TempDir() + mirrorPath := filepath.Join(mirrorRoot, "example.com", "mod") + assert.NoError(t, exec.Command("git", "clone", "--mirror", workDir, mirrorPath).Run()) + manager, err := gitclone.NewManager(ctx, gitclone.Config{MirrorRoot: mirrorRoot}, nil) + assert.NoError(t, err) + repo, err := manager.GetOrCreate(ctx, upstream) + assert.NoError(t, err) + + templateDir := t.TempDir() + assert.NoError(t, os.MkdirAll(filepath.Join(templateDir, "hooks"), 0o700)) + proofPath := filepath.Join(t.TempDir(), "proof") + assert.NoError(t, os.WriteFile(filepath.Join(templateDir, "hooks", "post-checkout"), + []byte("#!/bin/sh\ntouch \""+proofPath+"\"\n"), 0o700)) + t.Setenv("GIT_TEMPLATE_DIR", templateDir) + + content, err := gomod.GenerateZipForTest(ctx, repo, "example.com/mod", "v1.0.0") + assert.NoError(t, err) + assert.NoError(t, content.Close()) + _, err = os.Lstat(proofPath) + assert.True(t, os.IsNotExist(err)) +}