fix(security): clean up temp tarballs after registry push

Add WithImageCleanup option and wire it into Push() via defer.
defaultImageSource now produces a cleanup callback that removes the temp
tar file. Previously, temp tarballs containing full image contents were
left in /tmp indefinitely. The public ImageSource type signature is
unchanged for backward compatibility.

commit: 658f480e01cfacfb9c7b0ee986a920100a0aabe1
author: brice · 7mo ago · 2026-03-14 22:15 UTC
parent: 92dab38
signature: unsigned

2 files · +65 -16

M pkg/registrypush/push.go +40 -16


@@ -36,6 +36,7 @@ type Pusher struct {
36	36		transport http.RoundTripper
37	37		insecureTLS bool
38	38		imageSource ImageSource
	39	+	cleanupFn func()
39	40		progress io.Writer
40	41		}
41	42

@@ -63,6 +64,12 @@ func WithImageSource(src ImageSource) Option {
63	64		return func(p *Pusher) { p.imageSource = src }
64	65		}
65	66
	67	+	// WithImageCleanup sets a function called after Push completes to clean up
	68	+	// resources created by the image source (e.g., temp tar files).
	69	+	func WithImageCleanup(fn func()) Option {
	70	+	return func(p *Pusher) { p.cleanupFn = fn }
	71	+	}
	72	+
66	73		// WithProgress sets a writer for progress messages.
67	74		func WithProgress(w io.Writer) Option {
68	75		return func(p *Pusher) { p.progress = w }

@@ -82,7 +89,14 @@ func New(opts ...Option) *Pusher {
82	89		}
83	90		}
84	91		if p.imageSource == nil {
85		-	p.imageSource = defaultImageSource
	92	+	p.imageSource = func(ctx context.Context, ref string) (v1.Image, error) {
	93	+	img, cleanup, err := defaultImageSourceWithCleanup(ctx, ref)
	94	+	if err != nil {
	95	+	return nil, err
	96	+	}
	97	+	p.cleanupFn = cleanup
	98	+	return img, nil
	99	+	}
86	100		}
87	101		return p
88	102		}

@@ -102,6 +116,9 @@ func (p *Pusher) Push(ctx context.Context, ref string) error {
102	116		return fmt.Errorf("image ref %s must include a tag", ref)
103	117		}
104	118
	119	+	cleanup := p.cleanupFn
	120	+	p.cleanupFn = nil
	121	+
105	122		authHeader, err := resolveAuthHeader(parsedRef)
106	123		if err != nil {
107	124		return err

@@ -111,6 +128,12 @@ func (p *Pusher) Push(ctx context.Context, ref string) error {
111	128		if err != nil {
112	129		return err
113	130		}
	131	+	if p.cleanupFn != nil {
	132	+	cleanup = p.cleanupFn
	133	+	}
	134	+	if cleanup != nil {
	135	+	defer cleanup()
	136	+	}
114	137
115	138		baseURL := registryBaseURL(parsedRef.Context().RegistryStr())
116	139		repo := parsedRef.Context().RepositoryStr()

@@ -155,30 +178,30 @@ func (p *Pusher) UploadBlob(ctx context.Context, baseURL, repo, digest string, s
155	178		return p.finalizeBlobUpload(ctx, baseURL, uploadURL, digest, auth)
156	179		}
157	180
158		-	// defaultImageSource reads an image from the local Docker/Podman daemon.
159		-	func defaultImageSource(ctx context.Context, ref string) (v1.Image, error) {
	181	+	// defaultImageSourceWithCleanup reads an image from the local Docker/Podman daemon.
	182	+	func defaultImageSourceWithCleanup(ctx context.Context, ref string) (v1.Image, func(), error) {
160	183		cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
161	184		if err != nil {
162		-	return nil, fmt.Errorf("failed to create Docker client: %w", err)
	185	+	return nil, nil, fmt.Errorf("failed to create Docker client: %w", err)
163	186		}
164	187		defer cli.Close()
165	188
166	189		tag, err := name.NewTag(ref)
167	190		if err != nil {
168		-	return nil, fmt.Errorf("failed to parse image ref %s: %w", ref, err)
	191	+	return nil, nil, fmt.Errorf("failed to parse image ref %s: %w", ref, err)
169	192		}
170	193
171	194		rc, err := cli.ImageSave(ctx, []string{ref})
172	195		if err != nil {
173		-	return nil, fmt.Errorf("failed to export image %s: %w", ref, err)
	196	+	return nil, nil, fmt.Errorf("failed to export image %s: %w", ref, err)
174	197		}
175	198
176	199		tmpFile, err := os.CreateTemp("", "gordon-push-*.tar")
177	200		if err != nil {
178	201		if closeErr := rc.Close(); closeErr != nil {
179		-	return nil, fmt.Errorf("failed to create temp file for image export: %w (also failed to close image export stream: %v)", err, closeErr)
	202	+	return nil, nil, fmt.Errorf("failed to create temp file for image export: %w (also failed to close image export stream: %v)", err, closeErr)
180	203		}
181		-	return nil, fmt.Errorf("failed to create temp file for image export: %w", err)
	204	+	return nil, nil, fmt.Errorf("failed to create temp file for image export: %w", err)
182	205		}
183	206		tmpPath := tmpFile.Name()
184	207

@@ -186,33 +209,34 @@ func defaultImageSource(ctx context.Context, ref string) (v1.Image, error) {
186	209		if closeErr := rc.Close(); closeErr != nil {
187	210		_ = tmpFile.Close()
188	211		_ = os.Remove(tmpPath)
189		-	return nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close image export stream: %v)", err, closeErr)
	212	+	return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close image export stream: %v)", err, closeErr)
190	213		}
191	214		if closeErr := tmpFile.Close(); closeErr != nil {
192	215		_ = os.Remove(tmpPath)
193		-	return nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close temp file: %v)", err, closeErr)
	216	+	return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close temp file: %v)", err, closeErr)
194	217		}
195	218		if removeErr := os.Remove(tmpPath); removeErr != nil {
196		-	return nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to remove temp file %s: %v)", err, tmpPath, removeErr)
	219	+	return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to remove temp file %s: %v)", err, tmpPath, removeErr)
197	220		}
198		-	return nil, fmt.Errorf("failed to write image tar to temp file: %w", err)
	221	+	return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w", err)
199	222		}
200	223		if err := rc.Close(); err != nil {
201	224		_ = tmpFile.Close()
202	225		_ = os.Remove(tmpPath)
203		-	return nil, fmt.Errorf("failed to close image export stream: %w", err)
	226	+	return nil, nil, fmt.Errorf("failed to close image export stream: %w", err)
204	227		}
205	228		if err := tmpFile.Close(); err != nil {
206	229		_ = os.Remove(tmpPath)
207		-	return nil, fmt.Errorf("failed to close temp file %s: %w", tmpPath, err)
	230	+	return nil, nil, fmt.Errorf("failed to close temp file %s: %w", tmpPath, err)
208	231		}
209	232
210	233		img, err := tarball.ImageFromPath(tmpPath, &tag)
211	234		if err != nil {
212	235		_ = os.Remove(tmpPath)
213		-	return nil, fmt.Errorf("failed to parse image tar for %s: %w", ref, err)
	236	+	return nil, nil, fmt.Errorf("failed to parse image tar for %s: %w", ref, err)
214	237		}
215		-	return img, nil
	238	+	cleanup := func() { _ = os.Remove(tmpPath) }
	239	+	return img, cleanup, nil
216	240		}
217	241
218	242		func (p *Pusher) uploadImageLayers(ctx context.Context, img v1.Image, baseURL, repo, auth string) error {

 	transport   http.RoundTripper
 	insecureTLS bool
 	imageSource ImageSource
+	cleanupFn   func()
 	progress    io.Writer
 }
 	return func(p *Pusher) { p.imageSource = src }
 }
+// WithImageCleanup sets a function called after Push completes to clean up
+// resources created by the image source (e.g., temp tar files).
+func WithImageCleanup(fn func()) Option {
+	return func(p *Pusher) { p.cleanupFn = fn }
+}
 // WithProgress sets a writer for progress messages.
 func WithProgress(w io.Writer) Option {
 	return func(p *Pusher) { p.progress = w }
 		}
 	}
 	if p.imageSource == nil {
-		p.imageSource = defaultImageSource
+		p.imageSource = func(ctx context.Context, ref string) (v1.Image, error) {
+			img, cleanup, err := defaultImageSourceWithCleanup(ctx, ref)
+			if err != nil {
+				return nil, err
+			}
+			p.cleanupFn = cleanup
+			return img, nil
+		}
 	}
 	return p
 }
 		return fmt.Errorf("image ref %s must include a tag", ref)
 	}
+	cleanup := p.cleanupFn
+	p.cleanupFn = nil
 	authHeader, err := resolveAuthHeader(parsedRef)
 	if err != nil {
 		return err
 	if err != nil {
 		return err
 	}
+	if p.cleanupFn != nil {
+		cleanup = p.cleanupFn
+	}
+	if cleanup != nil {
+		defer cleanup()
+	}
 	baseURL := registryBaseURL(parsedRef.Context().RegistryStr())
 	repo := parsedRef.Context().RepositoryStr()
 	return p.finalizeBlobUpload(ctx, baseURL, uploadURL, digest, auth)
 }
-// defaultImageSource reads an image from the local Docker/Podman daemon.
+// defaultImageSourceWithCleanup reads an image from the local Docker/Podman daemon.
-func defaultImageSource(ctx context.Context, ref string) (v1.Image, error) {
+func defaultImageSourceWithCleanup(ctx context.Context, ref string) (v1.Image, func(), error) {
 	cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation())
 	if err != nil {
-		return nil, fmt.Errorf("failed to create Docker client: %w", err)
+		return nil, nil, fmt.Errorf("failed to create Docker client: %w", err)
 	}
 	defer cli.Close()
 	tag, err := name.NewTag(ref)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse image ref %s: %w", ref, err)
+		return nil, nil, fmt.Errorf("failed to parse image ref %s: %w", ref, err)
 	}
 	rc, err := cli.ImageSave(ctx, []string{ref})
 	if err != nil {
-		return nil, fmt.Errorf("failed to export image %s: %w", ref, err)
+		return nil, nil, fmt.Errorf("failed to export image %s: %w", ref, err)
 	}
 	tmpFile, err := os.CreateTemp("", "gordon-push-*.tar")
 	if err != nil {
 		if closeErr := rc.Close(); closeErr != nil {
-			return nil, fmt.Errorf("failed to create temp file for image export: %w (also failed to close image export stream: %v)", err, closeErr)
+			return nil, nil, fmt.Errorf("failed to create temp file for image export: %w (also failed to close image export stream: %v)", err, closeErr)
 		}
-		return nil, fmt.Errorf("failed to create temp file for image export: %w", err)
+		return nil, nil, fmt.Errorf("failed to create temp file for image export: %w", err)
 	}
 	tmpPath := tmpFile.Name()
 		if closeErr := rc.Close(); closeErr != nil {
 			_ = tmpFile.Close()
 			_ = os.Remove(tmpPath)
-			return nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close image export stream: %v)", err, closeErr)
+			return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close image export stream: %v)", err, closeErr)
 		}
 		if closeErr := tmpFile.Close(); closeErr != nil {
 			_ = os.Remove(tmpPath)
-			return nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close temp file: %v)", err, closeErr)
+			return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to close temp file: %v)", err, closeErr)
 		}
 		if removeErr := os.Remove(tmpPath); removeErr != nil {
-			return nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to remove temp file %s: %v)", err, tmpPath, removeErr)
+			return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w (also failed to remove temp file %s: %v)", err, tmpPath, removeErr)
 		}
-		return nil, fmt.Errorf("failed to write image tar to temp file: %w", err)
+		return nil, nil, fmt.Errorf("failed to write image tar to temp file: %w", err)
 	}
 	if err := rc.Close(); err != nil {
 		_ = tmpFile.Close()
 		_ = os.Remove(tmpPath)
-		return nil, fmt.Errorf("failed to close image export stream: %w", err)
+		return nil, nil, fmt.Errorf("failed to close image export stream: %w", err)
 	}
 	if err := tmpFile.Close(); err != nil {
 		_ = os.Remove(tmpPath)
-		return nil, fmt.Errorf("failed to close temp file %s: %w", tmpPath, err)
+		return nil, nil, fmt.Errorf("failed to close temp file %s: %w", tmpPath, err)
 	}
 	img, err := tarball.ImageFromPath(tmpPath, &tag)
 	if err != nil {
 		_ = os.Remove(tmpPath)
-		return nil, fmt.Errorf("failed to parse image tar for %s: %w", ref, err)
+		return nil, nil, fmt.Errorf("failed to parse image tar for %s: %w", ref, err)
 	}
-	return img, nil
+	cleanup := func() { _ = os.Remove(tmpPath) }
+	return img, cleanup, nil
 }
 func (p *Pusher) uploadImageLayers(ctx context.Context, img v1.Image, baseURL, repo, auth string) error {

M pkg/registrypush/push_test.go +25 -0


@@ -217,6 +217,31 @@ func TestPusher_Push(t *testing.T) {
217	217		}
218	218		}
219	219
	220	+	func TestPusher_Push_InvokesCleanup(t *testing.T) {
	221	+	var cleanupCalled bool
	222	+	img, err := random.Image(1024, 1)
	223	+	require.NoError(t, err)
	224	+
	225	+	registry := httptest.NewServer(newFakePushRegistry(t, img, map[string]bool{}).handler())
	226	+	defer registry.Close()
	227	+
	228	+	p := registrypush.New(
	229	+	registrypush.WithChunkSize(1024),
	230	+	registrypush.WithImageSource(func(ctx context.Context, ref string) (v1.Image, error) {
	231	+	return img, nil
	232	+	}),
	233	+	registrypush.WithImageCleanup(func() {
	234	+	cleanupCalled = true
	235	+	}),
	236	+	)
	237	+
	238	+	registryHost := strings.TrimPrefix(registry.URL, "http://")
	239	+	err = p.Push(context.Background(), registryHost+"/demo/app:v1")
	240	+
	241	+	require.NoError(t, err)
	242	+	assert.True(t, cleanupCalled, "cleanup callback should have been invoked after push")
	243	+	}
	244	+
220	245		func TestPusher_WithInsecureTLS_AllowsSelfSignedCertificate(t *testing.T) {
221	246		t.Parallel()
222	247


@@ -217,6 +217,31 @@ func TestPusher_Push(t *testing.T) {
217	}	217		}
218	}	218		}
219		219
		220	+	func TestPusher_Push_InvokesCleanup(t *testing.T) {
		221	+	var cleanupCalled bool
		222	+	img, err := random.Image(1024, 1)
		223	+	require.NoError(t, err)
		224	+
		225	+	registry := httptest.NewServer(newFakePushRegistry(t, img, map[string]bool{}).handler())
		226	+	defer registry.Close()
		227	+
		228	+	p := registrypush.New(
		229	+	registrypush.WithChunkSize(1024),
		230	+	registrypush.WithImageSource(func(ctx context.Context, ref string) (v1.Image, error) {
		231	+	return img, nil
		232	+	}),
		233	+	registrypush.WithImageCleanup(func() {
		234	+	cleanupCalled = true
		235	+	}),
		236	+	)
		237	+
		238	+	registryHost := strings.TrimPrefix(registry.URL, "http://")
		239	+	err = p.Push(context.Background(), registryHost+"/demo/app:v1")
		240	+
		241	+	require.NoError(t, err)
		242	+	assert.True(t, cleanupCalled, "cleanup callback should have been invoked after push")
		243	+	}
		244	+
220	func TestPusher_WithInsecureTLS_AllowsSelfSignedCertificate(t *testing.T) {	245		func TestPusher_WithInsecureTLS_AllowsSelfSignedCertificate(t *testing.T) {
221	t.Parallel()	246		t.Parallel()
222		247