diff --git a/pkg/attestation/crafter/cioverride.go b/pkg/attestation/crafter/cioverride.go index dd15ab5ac..06f6c163a 100644 --- a/pkg/attestation/crafter/cioverride.go +++ b/pkg/attestation/crafter/cioverride.go @@ -95,6 +95,8 @@ func overrideHeadWithPRCommit(headCommit *HeadCommit, path, actualSHA string, lo return } + defer releaseGitDescriptors(repo, logger) + hash := plumbing.NewHash(actualSHA) commit, err := repo.CommitObject(hash) if err != nil { diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 7e719e9ab..2bf383ed5 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -40,6 +40,7 @@ import ( "github.com/chainloop-dev/chainloop/pkg/policies" "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" + "github.com/go-git/go-git/v6/plumbing/storer" "github.com/google/go-containerregistry/pkg/authn" intoto "github.com/in-toto/attestation/go/v1" "github.com/rs/zerolog" @@ -338,6 +339,8 @@ func gracefulGitRepoHead(path string, logger *zerolog.Logger) (*HeadCommit, erro return nil, fmt.Errorf("opening repository: %w", err) } + defer releaseGitDescriptors(repo, logger) + head, err := repo.Head() if err != nil { if errors.Is(err, plumbing.ErrReferenceNotFound) { @@ -385,6 +388,24 @@ func gracefulGitRepoHead(path string, logger *zerolog.Logger) (*HeadCommit, erro return c, nil } +// releaseGitDescriptors closes the packfile descriptors go-git keeps open after +// a read. Since go-git v6.0.0-alpha.5 every PlainOpen* call builds its own +// 256-entry descriptor pool holding roughly 3 descriptors per packfile +// (.pack/.idx/.rev). Those are released only on LRU eviction, which never fires +// below 256 entries, so without this call they stay open for the lifetime of the +// process. Descriptors reopen lazily, so the repository remains usable +// afterwards. +func releaseGitDescriptors(repo *git.Repository, logger *zerolog.Logger) { + releaser, ok := repo.Storer.(storer.IdleReleaser) + if !ok { + return + } + + if err := releaser.CloseIdleDescriptors(); err != nil && logger != nil { + logger.Debug().Err(err).Msg("releasing git pack descriptors") + } +} + // Clear any basic auth credentials from the remote URL func sanitizeRemoteURL(remoteURL string) (string, error) { uri, err := url.Parse(remoteURL) diff --git a/pkg/attestation/crafter/git_descriptors_test.go b/pkg/attestation/crafter/git_descriptors_test.go new file mode 100644 index 000000000..79c5d7989 --- /dev/null +++ b/pkg/attestation/crafter/git_descriptors_test.go @@ -0,0 +1,47 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crafter + +import ( + "testing" + + "github.com/go-git/go-git/v6" + "github.com/go-git/go-git/v6/plumbing/storer" + "github.com/stretchr/testify/require" +) + +// TestGitStorerImplementsIdleReleaser pins the interface that +// releaseGitDescriptors depends on. That helper deliberately fails open: if a +// future go-git release stops satisfying storer.IdleReleaser it would silently +// stop releasing descriptors, reintroducing the retention regression with +// nothing failing. This assertion turns that into a build-time-visible test +// failure instead. +// +// The behavioural counterpart lives in +// TestGracefulGitRepoHeadReleasesDescriptors, which is unix-only; this test +// carries the guard on every platform. +func TestGitStorerImplementsIdleReleaser(t *testing.T) { + repoDir := t.TempDir() + _, err := git.PlainInit(repoDir, false) + require.NoError(t, err) + + repo, err := git.PlainOpenWithOptions(repoDir, &git.PlainOpenOptions{DetectDotGit: true}) + require.NoError(t, err) + + _, ok := repo.Storer.(storer.IdleReleaser) + require.True(t, ok, + "go-git storer must implement storer.IdleReleaser; releaseGitDescriptors is a no-op without it") +} diff --git a/pkg/attestation/crafter/git_descriptors_unix_test.go b/pkg/attestation/crafter/git_descriptors_unix_test.go new file mode 100644 index 000000000..b764a862f --- /dev/null +++ b/pkg/attestation/crafter/git_descriptors_unix_test.go @@ -0,0 +1,177 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// The descriptor probe below has no Windows equivalent, and the CLI is built for +// Windows too, so this guard is scoped to unix. Linux CI and macOS development +// hosts — where descriptor limits actually bind — are both covered. +//go:build unix + +package crafter + +import ( + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "syscall" + "testing" + "time" + + "github.com/go-git/go-git/v6" + "github.com/go-git/go-git/v6/plumbing/object" + "github.com/stretchr/testify/require" +) + +// TestGitDescriptorsReleasedAtAllOpenSites guards the file-descriptor retention +// regression that go-git v6.0.0-alpha.5 introduced: every PlainOpenWithOptions +// call builds its own 256-entry descriptor pool holding roughly 3 descriptors +// per packfile (.pack/.idx/.rev), released only on LRU eviction — which never +// fires below 256 entries. Without an explicit release those descriptors stay +// open for the lifetime of the process. +// +// Every function that opens a repository must appear here: the release is a +// per-call-site defer, so a site omitted from this table can silently drop its +// release without any test failing. +// +// Each case repeats its open so a genuine leak accumulates into an unmistakable +// signal (~3 descriptors per iteration) while correct behaviour stays flat. A +// single open leaks too few descriptors to separate from incidental churn. +func TestGitDescriptorsReleasedAtAllOpenSites(t *testing.T) { + const ( + iterations = 40 + // The assertion is "flat", not "exact". Runtime and harness activity can + // shift the count by a few descriptors, whereas a real regression grows + // by ~3 per iteration (~120 total). + maxRetained = 20 + ) + + repoDir, headSHA := initPackedRepo(t) + + testCases := []struct { + name string + // open performs one complete open-read-release cycle against the fixture. + // It must assert that the read actually reached packed object data, + // otherwise a short-circuiting code path would make the measurement + // vacuous. + open func(t *testing.T) + }{ + { + name: "gracefulGitRepoHead", + open: func(t *testing.T) { + got, err := gracefulGitRepoHead(repoDir, nil) + require.NoError(t, err) + require.NotNil(t, got) + // Proves the commit object was read, not just the ref. + require.Equal(t, headSHA, got.Hash) + require.Equal(t, "John Doe", got.AuthorName) + }, + }, + { + name: "overrideHeadWithPRCommit", + open: func(t *testing.T) { + headCommit := &HeadCommit{} + overrideHeadWithPRCommit(headCommit, repoDir, headSHA, nil) + // Hash is assigned on the shallow-clone path too, so it cannot + // distinguish a real commit read from a lookup miss. AuthorName is + // only populated once the commit object resolves, which is what + // puts pack descriptors in play. + require.Equal(t, headSHA, headCommit.Hash) + require.Equal(t, "John Doe", headCommit.AuthorName) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Warm up once so lazily-initialised state is not counted as retention. + tc.open(t) + + before := countOpenDescriptors() + for range iterations { + tc.open(t) + } + retained := countOpenDescriptors() - before + + t.Logf("retained %d descriptors across %d opens", retained, iterations) + require.Less(t, retained, maxRetained, + "%s retained %d descriptors across %d opens: go-git pools packfile "+ + "descriptors per repository open and frees them only via "+ + "CloseIdleDescriptors (see releaseGitDescriptors)", + tc.name, retained, iterations) + }) + } +} + +// initPackedRepo builds a single-commit repository whose objects live in a +// packfile, returning its path and HEAD SHA. Packing is essential rather than +// incidental: reads of loose objects never touch the pooled pack descriptors, so +// an unpacked fixture would make every retention assertion pass even with the +// regression fully present. +func initPackedRepo(t *testing.T) (repoDir, headSHA string) { + t.Helper() + + // go-git cannot repack, so the packing step needs the git CLI. + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not found in PATH; needed to pack objects") + } + + repoDir = t.TempDir() + repo, err := git.PlainInit(repoDir, false) + require.NoError(t, err) + require.NoError(t, disableGPGSign(repo)) + + wt, err := repo.Worktree() + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(repoDir, "f.txt"), []byte("hello"), 0o600)) + _, err = wt.Add("f.txt") + require.NoError(t, err) + hash, err := wt.Commit("initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "John Doe", Email: "john@doe.org", When: time.Now()}, + }) + require.NoError(t, err) + + // A GIT_DIR or GIT_WORK_TREE inherited from the ambient environment would + // redirect gc at a different repository, leaving this fixture unpacked while + // the command still reports success. + cmd := exec.Command("git", "-C", repoDir, "gc", "--quiet") + cmd.Env = slices.DeleteFunc(os.Environ(), func(kv string) bool { + return strings.HasPrefix(kv, "GIT_DIR=") || strings.HasPrefix(kv, "GIT_WORK_TREE=") + }) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git gc: %s", out) + + // Assert the outcome rather than trusting the command: this is what turns a + // silently vacuous guard into a loud failure. + packs, err := filepath.Glob(filepath.Join(repoDir, ".git", "objects", "pack", "*.pack")) + require.NoError(t, err) + require.NotEmpty(t, packs, + "fixture must be packed or the descriptor guards below are vacuous") + + return repoDir, hash.String() +} + +// countOpenDescriptors counts live descriptors by probing F_GETFD. /proc is +// absent on darwin and /dev/fd enumeration is unreliable there, so probe +// directly rather than reading a pseudo-filesystem. +func countOpenDescriptors() int { + var n int + for fd := 0; fd < 4096; fd++ { + if _, _, errno := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), syscall.F_GETFD, 0); errno == 0 { + n++ + } + } + return n +}