Skip to content

Commit 17ae6d0

Browse files
tonistiigicrazy-max
tonistiigi
authored and
crazy-max
committed
git: verify checksum early and more tests
Signed-off-by: Tonis Tiigi <[email protected]> (cherry picked from commit 58f956b)
1 parent 13f36e6 commit 17ae6d0

File tree

2 files changed

+157
-15
lines changed

2 files changed

+157
-15
lines changed

‎frontend/dockerfile/dockerfile_addgit_test.go

Lines changed: 139 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,13 @@ func testAddGit(t *testing.T, sb integration.Sandbox) {
5858
revParseCmd.Dir = gitDir
5959
commitHashB, err := revParseCmd.Output()
6060
require.NoError(t, err)
61-
commitHash := strings.TrimSpace(string(commitHashB))
61+
commitHashV2 := strings.TrimSpace(string(commitHashB))
62+
63+
revParseCmd = exec.Command("git", "rev-parse", "v0.0.3")
64+
revParseCmd.Dir = gitDir
65+
commitHashB, err = revParseCmd.Output()
66+
require.NoError(t, err)
67+
commitHashV3 := strings.TrimSpace(string(commitHashB))
6268

6369
server := httptest.NewServer(http.FileServer(http.Dir(filepath.Clean(gitDir))))
6470
defer server.Close()
@@ -86,10 +92,9 @@ RUN cd /buildkit-chowned && \
8692
[ -z "$(git status -s)" ]
8793
`, map[string]string{
8894
"ServerURL": serverURL,
89-
"Checksum": commitHash,
95+
"Checksum": commitHashV2,
9096
})
9197
require.NoError(t, err)
92-
t.Logf("dockerfile=%s", dockerfile)
9398

9499
dir := integration.Tmpdir(t,
95100
fstest.CreateFile("Dockerfile", []byte(dockerfile), 0600),
@@ -106,6 +111,137 @@ RUN cd /buildkit-chowned && \
106111
},
107112
}, nil)
108113
require.NoError(t, err)
114+
115+
// Additional test: ADD from Git URL with checksum but without keep-git-dir flag
116+
dockerfile2, err := applyTemplate(`
117+
FROM alpine
118+
ARG REPO="{{.ServerURL}}/.git"
119+
ARG TAG="v0.0.3"
120+
ADD --checksum={{.Checksum}} ${REPO}#${TAG} /nogitdir
121+
RUN [ -f /nogitdir/foo ]
122+
RUN [ "$(cat /nogitdir/foo)" = "foo of v0.0.3" ]
123+
RUN [ ! -d /nogitdir/.git ]
124+
`, map[string]string{
125+
"ServerURL": serverURL,
126+
"Checksum": commitHashV3,
127+
})
128+
require.NoError(t, err)
129+
130+
dir2 := integration.Tmpdir(t,
131+
fstest.CreateFile("Dockerfile", []byte(dockerfile2), 0600),
132+
)
133+
134+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
135+
LocalMounts: map[string]fsutil.FS{
136+
dockerui.DefaultLocalNameDockerfile: dir2,
137+
dockerui.DefaultLocalNameContext: dir2,
138+
},
139+
}, nil)
140+
require.NoError(t, err)
141+
142+
// access initial ref again that was already pulled
143+
dockerfile3, err := applyTemplate(`
144+
FROM alpine
145+
ARG REPO="{{.ServerURL}}/.git"
146+
ARG TAG="v0.0.2"
147+
ADD --keep-git-dir --checksum={{.Checksum}} ${REPO}#${TAG} /nogitdir
148+
RUN [ -f /nogitdir/foo ]
149+
RUN [ "$(cat /nogitdir/foo)" = "foo of v0.0.2" ]
150+
RUN [ -d /nogitdir/.git ]
151+
`, map[string]string{
152+
"ServerURL": serverURL,
153+
"Checksum": commitHashV2,
154+
})
155+
require.NoError(t, err)
156+
157+
dir3 := integration.Tmpdir(t,
158+
fstest.CreateFile("Dockerfile", []byte(dockerfile3), 0600),
159+
)
160+
161+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
162+
LocalMounts: map[string]fsutil.FS{
163+
dockerui.DefaultLocalNameDockerfile: dir3,
164+
dockerui.DefaultLocalNameContext: dir3,
165+
},
166+
}, nil)
167+
require.NoError(t, err)
168+
169+
// Additional test: ADD from Git URL using commitHashV3 for both checksum and ref
170+
dockerfile4, err := applyTemplate(`
171+
FROM alpine
172+
ARG REPO="{{.ServerURL}}/.git"
173+
ARG COMMIT="{{.Checksum}}"
174+
ADD --keep-git-dir=true --checksum={{.Checksum}} ${REPO}#${COMMIT} /commitdir
175+
RUN [ -f /commitdir/foo ]
176+
RUN [ "$(cat /commitdir/foo)" = "foo of v0.0.3" ]
177+
RUN [ -d /commitdir/.git ]
178+
`, map[string]string{
179+
"ServerURL": serverURL,
180+
"Checksum": commitHashV3,
181+
})
182+
require.NoError(t, err)
183+
184+
dir4 := integration.Tmpdir(t,
185+
fstest.CreateFile("Dockerfile", []byte(dockerfile4), 0600),
186+
)
187+
188+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
189+
LocalMounts: map[string]fsutil.FS{
190+
dockerui.DefaultLocalNameDockerfile: dir4,
191+
dockerui.DefaultLocalNameContext: dir4,
192+
},
193+
}, nil)
194+
require.NoError(t, err)
195+
196+
// checksum does not match
197+
dockerfile5, err := applyTemplate(`
198+
FROM alpine
199+
ARG REPO="{{.ServerURL}}/.git"
200+
ARG TAG="v0.0.3"
201+
ADD --checksum={{.WrongChecksum}} ${REPO}#${TAG} /faildir
202+
`, map[string]string{
203+
"ServerURL": serverURL,
204+
"WrongChecksum": commitHashV2, // v0.0.2 hash, but ref is v0.0.3
205+
})
206+
require.NoError(t, err)
207+
208+
dir5 := integration.Tmpdir(t,
209+
fstest.CreateFile("Dockerfile", []byte(dockerfile5), 0600),
210+
)
211+
212+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
213+
LocalMounts: map[string]fsutil.FS{
214+
dockerui.DefaultLocalNameDockerfile: dir5,
215+
dockerui.DefaultLocalNameContext: dir5,
216+
},
217+
}, nil)
218+
require.Error(t, err)
219+
require.Contains(t, err.Error(), "expected checksum to match")
220+
221+
// checksum is garbage
222+
dockerfile6, err := applyTemplate(`
223+
FROM alpine
224+
ARG REPO="{{.ServerURL}}/.git"
225+
ARG TAG="v0.0.3"
226+
ADD --checksum=foobar ${REPO}#${TAG} /faildir
227+
`, map[string]string{
228+
"ServerURL": serverURL,
229+
})
230+
require.NoError(t, err)
231+
232+
dir6 := integration.Tmpdir(t,
233+
fstest.CreateFile("Dockerfile", []byte(dockerfile6), 0600),
234+
)
235+
236+
_, err = f.Solve(sb.Context(), c, client.SolveOpt{
237+
LocalMounts: map[string]fsutil.FS{
238+
dockerui.DefaultLocalNameDockerfile: dir6,
239+
dockerui.DefaultLocalNameContext: dir6,
240+
},
241+
}, nil)
242+
require.Error(t, err)
243+
require.Contains(t, err.Error(), "invalid checksum")
244+
require.Contains(t, err.Error(), "expected hex commit hash")
109245
}
110246

111247
func applyTemplate(tmpl string, x any) (string, error) {

‎source/git/source.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,13 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
351351
gs.locker.Lock(remote)
352352
defer gs.locker.Unlock(remote)
353353

354+
if gs.src.Checksum != "" {
355+
matched, err := regexp.MatchString("^[a-fA-F0-9]+$", gs.src.Checksum)
356+
if err != nil || !matched {
357+
return "", "", nil, false, errors.Errorf("invalid checksum %s for Git URL, expected hex commit hash", gs.src.Checksum)
358+
}
359+
}
360+
354361
var refCommitFullHash, ref2 string
355362
if gitutil.IsCommitSHA(gs.src.Checksum) && !gs.src.KeepGitDir {
356363
refCommitFullHash = gs.src.Checksum
@@ -549,7 +556,17 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
549556
subdir = "."
550557
}
551558

552-
checkedoutRef := "HEAD"
559+
if gs.src.Checksum != "" {
560+
actualHashBuf, err := git.Run(ctx, "rev-parse", ref)
561+
if err != nil {
562+
return nil, errors.Wrapf(err, "failed to rev-parse %s for %s", ref, urlutil.RedactCredentials(gs.src.Remote))
563+
}
564+
actualHash := strings.TrimSpace(string(actualHashBuf))
565+
if !strings.HasPrefix(actualHash, gs.src.Checksum) {
566+
return nil, errors.Errorf("expected checksum to match %s, got %s", gs.src.Checksum, actualHash)
567+
}
568+
}
569+
553570
if gs.src.KeepGitDir && subdir == "." {
554571
checkoutDirGit := filepath.Join(checkoutDir, ".git")
555572
if err := os.MkdirAll(checkoutDir, 0711); err != nil {
@@ -619,7 +636,6 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
619636
if err != nil {
620637
return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote))
621638
}
622-
checkedoutRef = ref // HEAD may not exist
623639
if subdir != "." {
624640
d, err := os.Open(filepath.Join(cd, subdir))
625641
if err != nil {
@@ -650,16 +666,6 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
650666
}
651667

652668
git = git.New(gitutil.WithWorkTree(checkoutDir), gitutil.WithGitDir(gitDir))
653-
if gs.src.Checksum != "" {
654-
actualHashBuf, err := git.Run(ctx, "rev-parse", checkedoutRef)
655-
if err != nil {
656-
return nil, errors.Wrapf(err, "failed to rev-parse %s for %s", checkedoutRef, urlutil.RedactCredentials(gs.src.Remote))
657-
}
658-
actualHash := strings.TrimSpace(string(actualHashBuf))
659-
if !strings.HasPrefix(actualHash, gs.src.Checksum) {
660-
return nil, errors.Errorf("expected checksum to match %s, got %s", gs.src.Checksum, actualHash)
661-
}
662-
}
663669
_, err = git.Run(ctx, "submodule", "update", "--init", "--recursive", "--depth=1")
664670
if err != nil {
665671
return nil, errors.Wrapf(err, "failed to update submodules for %s", urlutil.RedactCredentials(gs.src.Remote))

0 commit comments

Comments
 (0)