Skip to content

Commit 1ed2195

Browse files
joahgampagent
joahg
and
ampagent
authored
Support pinning git package sources to a full commit SHA (#602)
Context: #597 — composing agent-skills-style content packages from existing Hermit primitives. This adds full commit SHA support alongside branch and tag refs. Git sources currently pass the `#ref` fragment to `git clone --branch`, which only accepts branch and tag names, so packages cannot be pinned to an exact commit — and tags are mutable, so there is no immutable pin at all. A `#ref` that is a full lowercase hex commit hash (40 or 64 characters) is now fetched as a commit: initialise an empty repository and fetch just that commit rather than cloning a branch. Fetching by SHA requires the server to allow it (GitHub and GitLab do). Abbreviated hashes are not treated as commits since they are indistinguishable from branch names. Because git also permits branch/tag names that are exactly 40/64 hex characters, an advertised exact branch or tag is resolved first (branches take precedence over tags, matching `git clone --branch`), and commit-SHA fetching is only the fallback when the remote advertises no ref with that name — existing behaviour for such refs is preserved. Merged current master, retaining Git transport restrictions, system-command execution, and credential redaction for SHA fetches and ref resolution. Regression coverage checks the protected transport arguments and credential scrubbing on the new paths. Tracking: AGNTOPS-430. An integration test installs a non-HEAD commit from a local Git repository, verifies its content, and checks compatibility with a branch whose name looks like a full SHA. All cases run through the existing `./integration` harness against a freshly built CLI in Bash and Zsh. The new regression cases were also checked against current master without this PR’s implementation and failed at the behavior this PR fixes. Validation: `bin/go test ./...`, `bin/go test -count=1 -tags integration ./integration -run '^TestIntegration$'`, and `bin/golangci-lint run`. The separate Fish integration test is not included locally because Fish is unavailable. --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent 24b2bca commit 1ed2195

3 files changed

Lines changed: 257 additions & 7 deletions

File tree

‎cache/git.go‎

Lines changed: 94 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,27 @@ func (s *gitSource) Download(b *ui.Task, cache *Cache, checksum string) (string,
2828
if err != nil {
2929
return "", "", "", err
3030
}
31-
args := redact.Args(util.GitArgs("clone", "--depth=1")...)
32-
if tag != "" {
33-
args = append(args, redact.Plain("--branch="+tag))
31+
pinned := false
32+
if isFullGitSHA(tag) {
33+
// A full-hex name may still be a valid branch or tag name. Only treat
34+
// it as a commit pin when the remote does not advertise a ref with
35+
// that exact name, preserving prior "git clone --branch" behaviour.
36+
advertised, aerr := s.resolveAdvertisedRef(b, repo, tag)
37+
if aerr != nil {
38+
return "", "", "", aerr
39+
}
40+
pinned = advertised == ""
41+
}
42+
if pinned {
43+
err = s.checkoutGitCommit(b, cache.root, repo, tag, checkoutDir)
44+
} else {
45+
args := redact.Args(util.GitArgs("clone", "--depth=1")...)
46+
if tag != "" {
47+
args = append(args, redact.Plain("--branch="+tag))
48+
}
49+
args = append(args, redact.Plain("--"), repo, redact.Plain(checkoutDir))
50+
err = s.runner.RunInDir(b, cache.root, args...)
3451
}
35-
args = append(args, redact.Plain("--"), repo, redact.Plain(checkoutDir))
36-
err = s.runner.RunInDir(b, cache.root, args...)
3752
if err != nil {
3853
return "", "", "", errors.WithStack(err)
3954
}
@@ -48,7 +63,27 @@ func (s *gitSource) Download(b *ui.Task, cache *Cache, checksum string) (string,
4863
}
4964

5065
func (s *gitSource) ETag(b *ui.Task) (etag string, err error) {
51-
bts, err := s.lsRemote(b)
66+
repo, tag, err := parseGitURL(s.URL)
67+
if err != nil {
68+
return "", err
69+
}
70+
if isFullGitSHA(tag) {
71+
advertised, err := s.resolveAdvertisedRef(b, repo, tag)
72+
if err != nil {
73+
return "", errors.Wrap(err, s.URL.String())
74+
}
75+
if advertised != "" {
76+
return advertised, nil
77+
}
78+
// Not an advertised branch or tag, so it is a pinned commit, which is
79+
// immutable and its own ETag.
80+
return tag, nil
81+
}
82+
if tag == "" {
83+
tag = "HEAD"
84+
}
85+
args := append(redact.Args(util.GitArgs("ls-remote", "--")...), repo, redact.Plain(tag))
86+
bts, err := s.runner.CaptureInDir(b, "", args...)
5287
if err != nil {
5388
return "", errors.Wrap(err, s.URL.String())
5489
}
@@ -71,13 +106,65 @@ func (s *gitSource) lsRemote(log ui.Logger) ([]byte, error) {
71106
if err != nil {
72107
return nil, err
73108
}
74-
if tag == "" {
109+
if tag == "" || isFullGitSHA(tag) {
75110
tag = "HEAD"
76111
}
77112
args := append(redact.Args(util.GitArgs("ls-remote", "--")...), repo, redact.Plain(tag))
78113
return s.runner.CaptureInDir(log, "", args...)
79114
}
80115

116+
// resolveAdvertisedRef returns the commit the remote advertises for the
117+
// branch or tag with the exact name ref, or "" when the remote advertises no
118+
// such ref. Branches take precedence over tags, matching "git clone --branch".
119+
func (s *gitSource) resolveAdvertisedRef(b *ui.Task, repo redact.URL, ref string) (string, error) {
120+
args := append(redact.Args(util.GitArgs("ls-remote", "--")...), repo, redact.Plain("refs/heads/"+ref), redact.Plain("refs/tags/"+ref))
121+
bts, err := s.runner.CaptureInDir(b, "", args...)
122+
if err != nil {
123+
return "", errors.WithStack(err)
124+
}
125+
out := strings.TrimSpace(string(bts))
126+
if out == "" {
127+
return "", nil
128+
}
129+
// ls-remote output is sorted by ref name, so refs/heads sorts first.
130+
line, _, _ := strings.Cut(out, "\n")
131+
sha, _, ok := strings.Cut(line, "\t")
132+
if !ok {
133+
return "", errors.Errorf("invalid ls-remote output: %s", line)
134+
}
135+
return sha, nil
136+
}
137+
138+
// checkoutGitCommit fetches a single commit by SHA and checks it out.
139+
//
140+
// A commit SHA cannot be passed to "git clone --branch", so initialise an
141+
// empty repository and fetch just the commit instead. This requires the
142+
// server to allow fetching by commit SHA (GitHub and GitLab both do).
143+
func (s *gitSource) checkoutGitCommit(b *ui.Task, root string, repo redact.URL, sha, checkoutDir string) error {
144+
if err := s.runner.RunInDir(b, root, redact.Args("git", "init", "--", checkoutDir)...); err != nil {
145+
return errors.WithStack(err)
146+
}
147+
args := append(redact.Args(util.GitArgs("fetch", "--depth=1", "--")...), repo, redact.Plain(sha))
148+
if err := s.runner.RunInDir(b, checkoutDir, args...); err != nil {
149+
return errors.WithStack(err)
150+
}
151+
return errors.WithStack(s.runner.RunInDir(b, checkoutDir, redact.Args("git", "checkout", "--detach", "FETCH_HEAD")...))
152+
}
153+
154+
// isFullGitSHA reports whether ref is a full lowercase hex commit hash
155+
// (40 characters for SHA-1, 64 for SHA-256).
156+
func isFullGitSHA(ref string) bool {
157+
if len(ref) != 40 && len(ref) != 64 {
158+
return false
159+
}
160+
for _, r := range ref {
161+
if (r < '0' || r > '9') && (r < 'a' || r > 'f') {
162+
return false
163+
}
164+
}
165+
return true
166+
}
167+
81168
func parseGitURL(source redact.URL) (repo redact.URL, tag string, err error) {
82169
parts := strings.SplitN(source.Reveal(), "#", 2)
83170
repo = redact.URL(parts[0])

‎cache/source_test.go‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,127 @@ func TestGitSourceDoesNotLeakURLCredentials(t *testing.T) {
193193

194194
assert.NotContains(t, buf.String(), "sekret")
195195
}
196+
197+
func TestIsFullGitSHA(t *testing.T) {
198+
tests := []struct {
199+
ref string
200+
want bool
201+
}{
202+
{"6bccbcae2934bdd10ede93d493ee1eeeef5f24e2", true},
203+
{strings.Repeat("a", 64), true},
204+
{"", false},
205+
{"main", false},
206+
{"v1.2.3", false},
207+
// Abbreviated SHAs are indistinguishable from branch names.
208+
{"6bccbca", false},
209+
{strings.Repeat("a", 39), false},
210+
{strings.Repeat("a", 41), false},
211+
// Git prints SHAs in lowercase.
212+
{strings.Repeat("A", 40), false},
213+
{strings.Repeat("g", 40), false},
214+
}
215+
for _, tt := range tests {
216+
assert.Equal(t, tt.want, isFullGitSHA(tt.ref), tt.ref)
217+
}
218+
}
219+
220+
// TestGitSourceCommitSHAPinning verifies that a git source can be pinned to a
221+
// full commit SHA rather than a branch or tag.
222+
func TestGitSourceCommitSHAPinning(t *testing.T) {
223+
if _, err := exec.LookPath("git"); err != nil {
224+
t.Skip("git command not found")
225+
}
226+
227+
tmpDir := t.TempDir()
228+
repoDir := filepath.Join(tmpDir, "repo")
229+
assert.NoError(t, os.MkdirAll(repoDir, 0750))
230+
mustGit := func(args ...string) string {
231+
t.Helper()
232+
cmd := exec.Command("git", args...)
233+
cmd.Dir = repoDir
234+
out, err := cmd.CombinedOutput()
235+
assert.NoError(t, err, string(out))
236+
return strings.TrimSpace(string(out))
237+
}
238+
mustGit("init")
239+
mustGit("config", "user.email", "test@example.com")
240+
mustGit("config", "user.name", "Test")
241+
// Fetching an unadvertised commit from a local repository requires this;
242+
// hosting services such as GitHub and GitLab allow it by default.
243+
mustGit("config", "uploadpack.allowReachableSHA1InWant", "true")
244+
assert.NoError(t, os.WriteFile(filepath.Join(repoDir, "file.txt"), []byte("first"), 0600))
245+
mustGit("add", "file.txt")
246+
mustGit("commit", "-m", "first")
247+
pinned := mustGit("rev-parse", "HEAD")
248+
assert.NoError(t, os.WriteFile(filepath.Join(repoDir, "file.txt"), []byte("second"), 0600))
249+
mustGit("add", "file.txt")
250+
mustGit("commit", "-m", "second")
251+
252+
src := &gitSource{URL: redact.URL("file://" + repoDir + "#" + pinned), runner: &util.RealCommandRunner{}}
253+
log, _ := ui.NewForTesting()
254+
255+
// The remote advertises no branch or tag by this name, so it is treated
256+
// as a pinned commit and is its own ETag.
257+
etag, err := src.ETag(log.Task("test"))
258+
assert.NoError(t, err)
259+
assert.Equal(t, pinned, etag)
260+
261+
assert.NoError(t, src.Validate())
262+
263+
cacheRoot := filepath.Join(tmpDir, "cache")
264+
assert.NoError(t, os.MkdirAll(cacheRoot, 0750))
265+
cache := &Cache{root: cacheRoot}
266+
dir, etag, _, err := src.Download(log.Task("test"), cache, "checksum")
267+
assert.NoError(t, err)
268+
assert.Equal(t, pinned, etag)
269+
content, err := os.ReadFile(filepath.Join(dir, "file.txt"))
270+
assert.NoError(t, err)
271+
assert.Equal(t, "first", string(content))
272+
273+
// A branch whose name is exactly a full-hex string must still resolve as
274+
// a branch, not be reinterpreted as a commit object ID.
275+
hexBranch := strings.Repeat("a", 40)
276+
mustGit("branch", hexBranch, "HEAD")
277+
branchSrc := &gitSource{URL: redact.URL("file://" + repoDir + "#" + hexBranch), runner: &util.RealCommandRunner{}}
278+
279+
etag, err = branchSrc.ETag(log.Task("test"))
280+
assert.NoError(t, err)
281+
assert.Equal(t, mustGit("rev-parse", "HEAD"), etag)
282+
283+
dir, etag, _, err = branchSrc.Download(log.Task("test"), cache, "checksum2")
284+
assert.NoError(t, err)
285+
assert.Equal(t, mustGit("rev-parse", "HEAD"), etag)
286+
content, err = os.ReadFile(filepath.Join(dir, "file.txt"))
287+
assert.NoError(t, err)
288+
assert.Equal(t, "second", string(content))
289+
}
290+
291+
func TestGitCommitSourceUsesProtectedTransport(t *testing.T) {
292+
runner := &capturingRunner{}
293+
repo := redact.URL("https://x-access-token:sekret@github.com/owner/repo.git")
294+
sha := strings.Repeat("a", 40)
295+
src := &gitSource{URL: redact.URL(repo.Reveal() + "#" + sha), runner: runner}
296+
log, _ := ui.NewForTesting()
297+
_, _, _, err := src.Download(log.Task("test"), &Cache{root: t.TempDir()}, "checksum")
298+
assert.NoError(t, err)
299+
assert.Equal(t, append(util.GitArgs("ls-remote", "--"), repo.Reveal(), "refs/heads/"+sha, "refs/tags/"+sha), runner.commands[0])
300+
assert.Equal(t, append(util.GitArgs("fetch", "--depth=1", "--"), repo.Reveal(), sha), runner.commands[2])
301+
}
302+
303+
func TestGitCommitSourceDoesNotLeakURLCredentials(t *testing.T) {
304+
log, buf := ui.NewForTesting()
305+
task := log.Task("test")
306+
repo := redact.URL("https://x-access-token:sekret@127.0.0.1:1/owner/repo.git")
307+
sha := strings.Repeat("a", 40)
308+
src := &gitSource{URL: redact.URL(repo.Reveal() + "#" + sha), runner: &util.RealCommandRunner{}}
309+
_, _, _, err := src.Download(task, &Cache{root: t.TempDir()}, "checksum")
310+
assert.Error(t, err)
311+
assert.NotContains(t, err.Error(), "sekret")
312+
_, err = src.ETag(task)
313+
assert.Error(t, err)
314+
assert.NotContains(t, err.Error(), "sekret")
315+
err = src.checkoutGitCommit(task, t.TempDir(), repo, sha, filepath.Join(t.TempDir(), "checkout"))
316+
assert.Error(t, err)
317+
assert.NotContains(t, err.Error(), "sekret")
318+
assert.NotContains(t, buf.String(), "sekret")
319+
}

‎integration/integration_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,45 @@ EOF
596596
assert test "$(testenv2/bin/hermit env TESTENV2)" = "yes"
597597
`,
598598
},
599+
600+
{
601+
name: "GitPackageInstallsPinnedCommitAndPreservesHexBranchRefs",
602+
script: `
603+
hermit init --no-git --sources env:///packages .
604+
mkdir -p packages upstream.git
605+
git -C upstream.git init -q
606+
git -C upstream.git config user.name 'Hermit Test'
607+
git -C upstream.git config user.email hermit@example.com
608+
git -C upstream.git config commit.gpgsign false
609+
printf original > upstream.git/payload.txt
610+
git -C upstream.git add payload.txt
611+
git -C upstream.git commit -qm original
612+
pinned=$(git -C upstream.git rev-parse HEAD)
613+
printf newer > upstream.git/payload.txt
614+
git -C upstream.git add payload.txt
615+
git -C upstream.git commit -qm newer
616+
hexbranch=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
617+
git -C upstream.git branch "$hexbranch"
618+
cat > packages/pinned.hcl <<EOF
619+
description = "Pinned Git content"
620+
source = "file://$PWD/upstream.git#$pinned"
621+
binaries = ["payload.txt"]
622+
version "1.0.0" {}
623+
EOF
624+
cat > packages/named.hcl <<EOF
625+
description = "SHA-shaped branch name"
626+
source = "file://$PWD/upstream.git#$hexbranch"
627+
binaries = ["payload.txt"]
628+
version "1.0.0" {}
629+
EOF
630+
./bin/hermit install pinned
631+
assert test "$(cat "$HERMIT_STATE_DIR/pkg/pinned-1.0.0/payload.txt")" = original
632+
./bin/hermit uninstall pinned
633+
./bin/hermit install named
634+
assert test "$(cat "$HERMIT_STATE_DIR/pkg/named-1.0.0/payload.txt")" = newer
635+
assert test "$(cat "$HERMIT_STATE_DIR/pkg/pinned-1.0.0/payload.txt")" = original
636+
`,
637+
},
599638
{
600639
name: "InstallDirectScriptPackage",
601640
preparations: prep{fixture("testenv2"), activate(".")},

0 commit comments

Comments
 (0)