fix(client): make target dirs writable before extracting

client/archive.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -52,7 +53,12 @@ func Archive(ctx context.Context, w io.Writer, baseDir string, includePaths []st
 // Extract decompresses a zstd+tar stream from r into directory, preserving
 // file permissions, ownership, and symlinks. threads controls zstd
 // parallelism; 0 uses all CPU cores.
-func Extract(ctx context.Context, r io.Reader, directory string, threads int) error {
+//
+// Existing read-only directories are made owner-writable so tar can replace
+// their contents. tar re-applies archived modes, and a failed extract
+// restores the originals; read-only directories absent from the archive are
+// left owner-writable.
+func Extract(ctx context.Context, r io.Reader, directory string, threads int) (rerr error) {
 	if threads <= 0 {
 		threads = runtime.GOMAXPROCS(0)
 	}
@@ -61,6 +67,18 @@ func Extract(ctx context.Context, r io.Reader, directory string, threads int) er
 		return errors.Wrap(err, "failed to create target directory")
 	}
 
+	// Relax read-only dirs (e.g. Hermit package trees) so tar can overwrite
+	// their contents, restoring the original modes if anything fails.
+	relaxed, err := makeTreeWritable(directory)
+	defer func() {
+		if rerr != nil {
+			restoreDirModes(relaxed)
+		}
+	}()
+	if err != nil {
+		return errors.Wrap(err, "failed to make target directory writable")
+	}
+
 	zstdCmd := exec.CommandContext(ctx, "zstd", "-dc", fmt.Sprintf("-T%d", threads)) //nolint:gosec
 	tarCmd := exec.CommandContext(ctx, "tar", "-xpf", "-", "-C", directory)
 
@@ -103,6 +121,49 @@ func Extract(ctx context.Context, r io.Reader, directory string, threads int) er
 	return errors.Join(errs...)
 }
 
+// dirMode is a directory path and its pre-relaxation mode.
+type dirMode struct {
+	path string
+	mode fs.FileMode
+}
+
+// makeTreeWritable adds owner-write to root and every directory beneath it,
+// returning the changed directories with their original modes. tar overwrites
+// a file by unlinking it, which requires a writable parent directory.
+func makeTreeWritable(root string) ([]dirMode, error) {
+	var changed []dirMode
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, walkErr error) error {
+		switch {
+		case walkErr != nil:
+			return walkErr
+		case !d.IsDir():
+			return nil
+		}
+		info, err := d.Info()
+		if err != nil {
+			return errors.Wrapf(err, "failed to stat %s", path)
+		}
+		// Keep setuid/setgid/sticky so relaxing and restoring don't strip them.
+		mode := info.Mode() & (fs.ModePerm | fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky)
+		if mode&0o200 == 0 {
+			if err := os.Chmod(path, mode|0o200); err != nil {
+				return errors.Wrapf(err, "failed to make %s writable", path)
+			}
+			changed = append(changed, dirMode{path: path, mode: mode})
+		}
+		return nil
+	})
+	return changed, errors.Wrap(err, "failed to walk target directory")
+}
+
+// restoreDirModes best-effort restores modes relaxed by makeTreeWritable;
+// failures are ignored so the extraction error stays the one reported.
+func restoreDirModes(dirs []dirMode) {
+	for _, d := range dirs {
+		os.Chmod(d.path, d.mode) //nolint:errcheck,gosec
+	}
+}
+
 // runTarZstdPipeline runs tar piped through pzstd, writing compressed output
 // to w. The caller is responsible for closing w after this returns.
 func runTarZstdPipeline(ctx context.Context, tarArgs []string, threads int, w io.Writer) error {

client/client_test.go

@@ -387,6 +387,69 @@ func TestArchiveExtract(t *testing.T) {
 	assert.False(t, slices.Contains(names, "y.log"))
 }
 
+func TestExtractOverReadOnlyTree(t *testing.T) {
+	src := t.TempDir()
+	assert.NoError(t, os.MkdirAll(filepath.Join(src, "pkg", "conf"), 0o755))
+	assert.NoError(t, os.WriteFile(filepath.Join(src, "pkg", "conf", "cacerts"), []byte("v2"), 0o444))
+	assert.NoError(t, os.Chmod(filepath.Join(src, "pkg", "conf"), 0o555))
+	assert.NoError(t, os.Chmod(filepath.Join(src, "pkg"), 0o555))
+	t.Cleanup(func() { makeWritable(t, src) })
+
+	var buf bytes.Buffer
+	assert.NoError(t, client.Archive(t.Context(), &buf, src, []string{"pkg"}, nil, 0))
+
+	dst := t.TempDir()
+	assert.NoError(t, os.MkdirAll(filepath.Join(dst, "pkg", "conf"), 0o755))
+	assert.NoError(t, os.WriteFile(filepath.Join(dst, "pkg", "conf", "cacerts"), []byte("v1"), 0o444))
+	assert.NoError(t, os.Chmod(filepath.Join(dst, "pkg", "conf"), 0o555))
+	assert.NoError(t, os.Chmod(filepath.Join(dst, "pkg"), 0o555))
+	t.Cleanup(func() { makeWritable(t, dst) })
+
+	assert.NoError(t, client.Extract(t.Context(), &buf, dst, 0))
+
+	got, err := os.ReadFile(filepath.Join(dst, "pkg", "conf", "cacerts"))
+	assert.NoError(t, err)
+	assert.Equal(t, "v2", string(got))
+
+	assertReadOnlyDir(t, filepath.Join(dst, "pkg"))
+	assertReadOnlyDir(t, filepath.Join(dst, "pkg", "conf"))
+}
+
+func TestExtractFailureRestoresDirModes(t *testing.T) {
+	dst := t.TempDir()
+	assert.NoError(t, os.MkdirAll(filepath.Join(dst, "pkg", "conf"), 0o755))
+	assert.NoError(t, os.WriteFile(filepath.Join(dst, "pkg", "conf", "cacerts"), []byte("v1"), 0o444))
+	assert.NoError(t, os.Chmod(filepath.Join(dst, "pkg", "conf"), 0o555))
+	assert.NoError(t, os.Chmod(filepath.Join(dst, "pkg"), 0o555|os.ModeSetgid))
+	t.Cleanup(func() { makeWritable(t, dst) })
+
+	err := client.Extract(t.Context(), bytes.NewReader([]byte("not a zstd stream")), dst, 0)
+	assert.Error(t, err)
+
+	info, err := os.Stat(filepath.Join(dst, "pkg"))
+	assert.NoError(t, err)
+	assert.Equal(t, os.FileMode(0o555)|os.ModeSetgid, info.Mode()&(os.ModePerm|os.ModeSetgid))
+	assertReadOnlyDir(t, filepath.Join(dst, "pkg", "conf"))
+}
+
+func assertReadOnlyDir(t *testing.T, path string) {
+	t.Helper()
+	info, err := os.Stat(path)
+	assert.NoError(t, err)
+	assert.Equal(t, os.FileMode(0o555), info.Mode().Perm())
+}
+
+// makeWritable re-adds owner-write under root so t.TempDir cleanup can remove it.
+func makeWritable(t *testing.T, root string) {
+	t.Helper()
+	_ = filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
+		if err == nil && d.IsDir() {
+			_ = os.Chmod(path, 0o755)
+		}
+		return nil
+	})
+}
+
 func TestOpenIfNoneMatch(t *testing.T) {
 	srv := newFakeServer(nil)
 	defer srv.Close()