From 9f46e4efbd23f15b41b861b9ca2d86317fdaea7a Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho@tycho.pizza>
Date: Fri, 18 Sep 2020 16:14:29 -0700
Subject: [PATCH] import: handle readonly parent directories

in the case where a parent directory of an import is read-only, things may
fail in the unprivileged case because non-root cannot rm -rf a file that's
-w.

To fix this, we chmod +w the parent directory, and chmod it back at the
end.

Note that we can't defer the chmod back in chmodParentAndRemove(), since
the subsequent copy will also not be able to overwrite the file, since the
dir will be -w again.

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
---
 import.go              | 44 +++++++++++++++++++++++++++++++++++++++---
 test/unprivileged.bats | 21 ++++++++++++++++++++
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/import.go b/import.go
index 5f24b6e2..4ed7edba 100644
--- a/import.go
+++ b/import.go
@@ -65,6 +65,23 @@ func filesDiffer(p1 string, info1 os.FileInfo, p2 string, info2 os.FileInfo) (bo
 	return !eq, nil
 }
 
+func chmodParentAndRemove(destpath string) (func(), error) {
+	// in the unpriv case, the dir might be -w (centos distributes its
+	// /root this way), and non-real root can't delete stuff that's -w,
+	// even if it is the owner. so let's chmod +w .. and try again.
+	dir := path.Dir(destpath)
+	orig, err := os.Stat(dir)
+	if err != nil {
+		return nil, errors.Wrapf(err, "couldn't chmod +w ..")
+	}
+
+	err = os.Chmod(dir, 0700)
+	if err != nil {
+		return nil, errors.Wrapf(err, "couldn't chmod +w ..")
+	}
+	return func() { os.Chmod(dir, orig.Mode()) }, os.RemoveAll(destpath)
+}
+
 func importFile(imp string, cacheDir string) (string, error) {
 	e1, err := os.Lstat(imp)
 	if err != nil {
@@ -121,9 +138,20 @@ func importFile(imp string, cacheDir string) (string, error) {
 	for _, d := range diff {
 		switch d.Type() {
 		case mtree.Missing:
-			err := os.RemoveAll(path.Join(cacheDir, path.Base(imp), d.Path()))
+			p := path.Join(cacheDir, path.Base(imp), d.Path())
+			err := os.RemoveAll(p)
 			if err != nil {
-				return "", errors.Wrapf(err, "couldn't remove missing import %s", path.Join(cacheDir, path.Base(imp), d.Path()))
+				if os.IsPermission(err) {
+					var cleanup func()
+					cleanup, err = chmodParentAndRemove(p)
+					if cleanup != nil {
+						defer cleanup()
+					}
+				}
+
+				if err != nil {
+					return "", errors.Wrapf(err, "couldn't remove missing import %s", path.Join(cacheDir, path.Base(imp), d.Path()))
+				}
 			}
 		case mtree.Modified:
 			fallthrough
@@ -133,7 +161,17 @@ func importFile(imp string, cacheDir string) (string, error) {
 
 			err = os.RemoveAll(destpath)
 			if err != nil && !os.IsNotExist(err) {
-				return "", errors.Wrapf(err, "couldn't remove to replace import %s", destpath)
+				if os.IsPermission(err) {
+					var cleanup func()
+					cleanup, err = chmodParentAndRemove(destpath)
+					if cleanup != nil {
+						defer cleanup()
+					}
+				}
+
+				if err != nil {
+					return "", errors.Wrapf(err, "couldn't remove to replace import %s", destpath)
+				}
 			}
 
 			sdirinfo, err := os.Lstat(path.Dir(srcpath))
diff --git a/test/unprivileged.bats b/test/unprivileged.bats
index ed314082..b45ffc62 100644
--- a/test/unprivileged.bats
+++ b/test/unprivileged.bats
@@ -85,3 +85,24 @@ EOF
     sudo -u $SUDO_USER "${ROOT_DIR}/stacker" build
     stacker clean --all
 }
+
+@test "unprivileged read-only imports can be re-cached" {
+    [ -z "$TRAVIS" ] || skip "skipping unprivileged test in travis"
+
+    mkdir -p import
+    touch import/this
+    chmod -w import
+
+    cat > stacker.yaml <<EOF
+centos:
+    from:
+        type: docker
+        url: docker://centos:latest
+    import:
+        - import
+EOF
+    chown -R $SUDO_USER:$SUDO_USER .
+    sudo -u $SUDO_USER stacker build
+    echo that > import/this
+    sudo -u $SUDO_USER stacker build
+}