Skip to content

Commit

Permalink
import: handle readonly parent directories
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
tych0 committed Sep 18, 2020
1 parent 33565b5 commit 9f46e4e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 3 deletions.
44 changes: 41 additions & 3 deletions import.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down
21 changes: 21 additions & 0 deletions test/unprivileged.bats
Expand Up @@ -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
}

0 comments on commit 9f46e4e

Please sign in to comment.