Skip to content

Commit

Permalink
Remove windows warning about running containers as root, fixes #800 (#…
Browse files Browse the repository at this point in the history
…803)

* Omit warning about uid 0 in Windows

* Add TestWriteableFilesDirectory to make sure we always have files directory working

* Use path.Join instead of filepath.Join for items that are in container
  • Loading branch information
rfay committed May 9, 2018
1 parent 8429912 commit 90cb194
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 2 deletions.
7 changes: 5 additions & 2 deletions pkg/ddevapp/ddevapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/fsouza/go-dockerclient"
"github.com/lextoumbourou/goodhosts"
"github.com/mattn/go-shellwords"
"runtime"
)

const containerWaitTimeout = 61
Expand Down Expand Up @@ -739,8 +740,10 @@ func (app *DdevApp) DockerEnv() {
if gidInt, err = strconv.Atoi(curUser.Gid); err != nil {
gidStr = "0"
}
if uidInt > 60000 || gidInt > 60000 || uidInt == 0 {
util.Warning("Warning: containers will run as root. This is fine on Docker for Windows or Docker for Mac, but could be a security risk on Linux.")

// Warn about running as root if we're not on windows.
if runtime.GOOS != "windows" && (uidInt > 60000 || gidInt > 60000 || uidInt == 0) {
util.Warning("Warning: containers will run as root. This could be a security risk on Linux.")
}

// If the uidStr or gidStr is outside the range possible in container, use root
Expand Down
73 changes: 73 additions & 0 deletions pkg/ddevapp/ddevapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ddevapp_test
import (
"fmt"
"os"
"path"
"path/filepath"
"regexp"
"runtime"
Expand Down Expand Up @@ -455,6 +456,78 @@ func TestDdevImportDB(t *testing.T) {
}
}

// TestWriteableFilesDirectory tests to make sure that files created on host are writeable on container
// and files ceated in container are correct user on host.
func TestWriteableFilesDirectory(t *testing.T) {
assert := asrt.New(t)
app := &ddevapp.DdevApp{}

for _, site := range TestSites {
switchDir := site.Chdir()
runTime := testcommon.TimeTrack(time.Now(), fmt.Sprintf("%s DdevImportDB", site.Name))

testcommon.ClearDockerEnv()
err := app.Init(site.Dir)
assert.NoError(err)

err = app.Start()
assert.NoError(err)

uploadDir := app.GetUploadDir()
if uploadDir != "" {

// Use exec to touch a file in the container and see what the result is. Make sure it comes out with ownership
// making it writeable on the host.
filename := fileutil.RandomFilenameBase()
dirname := fileutil.RandomFilenameBase()
// Use path.Join for items on th container (linux) and filepath.Join for items on the host.
inContainerDir := path.Join(uploadDir, dirname)
onHostDir := filepath.Join(app.Docroot, inContainerDir)
inContainerRelativePath := path.Join(inContainerDir, filename)
onHostRelativePath := path.Join(onHostDir, filename)

err = os.MkdirAll(onHostDir, 0775)
assert.NoError(err)
_, _, err = app.Exec("web", "sh", "-c", "echo 'content created inside container\n' >"+inContainerRelativePath)
assert.NoError(err)

// Now try to append to the file on the host.
// os.OpenFile() for append here fails if the file does not already exist.
f, err := os.OpenFile(onHostRelativePath, os.O_APPEND|os.O_WRONLY, 0660)
assert.NoError(err)
_, err = f.WriteString("this addition to the file was added on the host side")
assert.NoError(err)
_ = f.Close()

// Create a file on the host and see what the result is. Make sure we can not append/write to it in the container.
filename = fileutil.RandomFilenameBase()
dirname = fileutil.RandomFilenameBase()
inContainerDir = path.Join(uploadDir, dirname)
onHostDir = filepath.Join(app.Docroot, inContainerDir)
inContainerRelativePath = path.Join(inContainerDir, filename)
onHostRelativePath = filepath.Join(onHostDir, filename)

err = os.MkdirAll(onHostDir, 0775)
assert.NoError(err)

f, err = os.OpenFile(onHostRelativePath, os.O_CREATE|os.O_RDWR, 0660)
assert.NoError(err)
_, err = f.WriteString("this base content was inserted on the host side\n")
assert.NoError(err)
_ = f.Close()
// if the file exists, add to it. We don't want to add if it's not already there.
_, _, err = app.Exec("web", "sh", "-c", "if [ -f "+inContainerRelativePath+" ]; then echo 'content added inside container\n' >>"+inContainerRelativePath+"; fi")
assert.NoError(err)
// grep the file for both the content added on host and that added in container.
_, _, err = app.Exec("web", "sh", "-c", "grep 'base content was inserted on the host' "+inContainerRelativePath+"&& grep 'content added inside container' "+inContainerRelativePath)
assert.NoError(err)
}

runTime()
switchDir()
}
}

// TestDdevImportFiles tests the functionality that is called when "ddev import-files" is executed
func TestDdevImportFiles(t *testing.T) {
assert := asrt.New(t)
Expand Down
10 changes: 10 additions & 0 deletions pkg/fileutil/files.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package fileutil

import (
"encoding/hex"
"fmt"
"io"
"io/ioutil"
"math/rand"
"os"
"path/filepath"

Expand Down Expand Up @@ -179,3 +181,11 @@ func ListFilesInDir(path string) ([]string, error) {
}
return fileList, nil
}

// RandomFilenameBase generates a temporary filename for use in testing or whatever.
// From https://stackoverflow.com/a/28005931/215713
func RandomFilenameBase() string {
randBytes := make([]byte, 16)
_, _ = rand.Read(randBytes)
return hex.EncodeToString(randBytes)
}

0 comments on commit 90cb194

Please sign in to comment.