Merge branch '505-support-pages-artifact-path' into 'master'

Make `/pages` to be an actual `artifacts-path`

Closes #505

See merge request gitlab-org/gitlab-pages!411

6 files changed, 113 insertions, 18 deletions

diff --git a/Makefile.util.mk b/Makefile.util.mk
index 4f190ea4..da29e0c8 100644
--- a/Makefile.util.mk
+++ b/Makefile.util.mk
@@ -17,7 +17,7 @@ race: .GOPATH/.ok gitlab-pages
 	CGO_ENABLED=1 go test -race $(if $V,-v) $(allpackages)
 
 acceptance: .GOPATH/.ok gitlab-pages
-	go test $(if $V,-v) ./test/acceptance 2>&1 | tee tests.out
+	go test $(if $V,-v) ./test/acceptance ${ARGS} 2>&1 | tee tests.out
 
 bench: .GOPATH/.ok gitlab-pages
 	go test -bench=. -run=^$$ $(allpackages)
diff --git a/daemon.go b/daemon.go
index bf0472bb..e73a0e95 100644
--- a/daemon.go
+++ b/daemon.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"os/signal"
+	"path/filepath"
 	"strings"
 	"syscall"
 
@@ -19,8 +20,6 @@ import (
 
 const (
 	daemonRunProgram = "gitlab-pages-unprivileged"
-
-	pagesRootInChroot = "/pages"
 )
 
 func daemonMain() {
@@ -249,34 +248,41 @@ func jailCreate(cmd *exec.Cmd) (*jail.Jail, error) {
 	return cage, nil
 }
 
-func jailDaemon(cmd *exec.Cmd) (*jail.Jail, error) {
+func jailDaemon(pagesRoot string, cmd *exec.Cmd) (*jail.Jail, error) {
 	cage, err := jailCreate(cmd)
 	if err != nil {
 		return nil, err
 	}
 
-	wd, err := os.Getwd()
-	if err != nil {
-		return nil, err
-	}
-
 	// Bind mount shared folder
-	cage.MkDir(pagesRootInChroot, 0755)
-	cage.Bind(pagesRootInChroot, wd)
+	cage.MkDirAll(pagesRoot, 0755)
+	cage.Bind(pagesRoot, pagesRoot)
 
 	// Update command to use chroot
 	cmd.SysProcAttr.Chroot = cage.Path()
 	cmd.Path = "/gitlab-pages"
-	cmd.Dir = pagesRootInChroot
+	cmd.Dir = pagesRoot
 
 	return cage, nil
 }
 
-func daemonize(config appConfig, uid, gid uint, inPlace bool) error {
+func daemonize(config appConfig, uid, gid uint, inPlace bool, pagesRoot string) error {
+	// Ensure pagesRoot is an absolute path. This will produce a different path
+	// if any component of pagesRoot is a symlink (not likely). For example,
+	// -pages-root=/some-path where ln -s /other-path /some-path
+	// pagesPath will become: /other-path and we will fail to serve files from /some-path.
+	// GitLab Rails also resolves the absolute path for `pages_path`
+	// https://gitlab.com/gitlab-org/gitlab/blob/981ad651d8bd3690e28583eec2363a79f775af89/config/initializers/1_settings.rb#L296
+	pagesRoot, err := filepath.Abs(pagesRoot)
+	if err != nil {
+		return err
+	}
+
 	log.WithFields(log.Fields{
-		"uid":      uid,
-		"gid":      gid,
-		"in-place": inPlace,
+		"uid":        uid,
+		"gid":        gid,
+		"in-place":   inPlace,
+		"pages-root": pagesRoot,
 	}).Info("running the daemon as unprivileged user")
 
 	cmd, err := daemonReexec(uid, gid, daemonRunProgram)
@@ -290,7 +296,7 @@ func daemonize(config appConfig, uid, gid uint, inPlace bool) error {
 	if inPlace {
 		wrapper, err = chrootDaemon(cmd)
 	} else {
-		wrapper, err = jailDaemon(cmd)
+		wrapper, err = jailDaemon(pagesRoot, cmd)
 	}
 	if err != nil {
 		log.WithError(err).Print("chroot failed")
diff --git a/internal/jail/jail.go b/internal/jail/jail.go
index 13b39374..40eb2820 100644
--- a/internal/jail/jail.go
+++ b/internal/jail/jail.go
@@ -5,6 +5,7 @@ import (
 	"io"
 	"os"
 	"path"
+	"strings"
 	"syscall"
 	"time"
 
@@ -145,6 +146,21 @@ func (j *Jail) MkDir(path string, perm os.FileMode) {
 	j.directories = append(j.directories, pathAndMode{path: j.ExternalPath(path), mode: perm})
 }
 
+// MkDirAll enqueue a mkdir operation at jail building time for all directories
+// in dir to be created one by one
+func (j *Jail) MkDirAll(dir string, perm os.FileMode) {
+	subPaths := strings.Split(dir, "/")
+	currentParent := ""
+
+	// skip first subPath as is an empty string given dir starts with "/"
+	for _, subPath := range subPaths[1:] {
+		currentPath := path.Join(currentParent, subPath)
+		j.directories = append(j.directories, pathAndMode{path: j.ExternalPath(currentPath), mode: perm})
+
+		currentParent = currentPath
+	}
+}
+
 // CharDev enqueues an mknod operation for the given character device at jail
 // building time
 func (j *Jail) CharDev(path string) error {
diff --git a/internal/jail/jail_linux_test.go b/internal/jail/jail_linux_test.go
new file mode 100644
index 00000000..9900a769
--- /dev/null
+++ b/internal/jail/jail_linux_test.go
@@ -0,0 +1,29 @@
+package jail_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-pages/internal/jail"
+)
+
+func TestJailCreateAndCleanSubPaths(t *testing.T) {
+	if os.Geteuid() != 0 {
+		t.Skip("skipping test for non root user")
+	}
+
+	cage := jail.CreateTimestamped("gitlab-pages", 0755)
+	subPath := "/pages/sub/path"
+
+	// Bind mount shared folder
+	cage.MkDirAll(subPath, 0755)
+	cage.Bind(subPath, cage.Path()+subPath)
+
+	require.NoError(t, cage.Build())
+	require.NoError(t, cage.Dispose())
+
+	_, err := os.Stat(cage.Path())
+	require.True(t, os.IsNotExist(err), "jail was removed")
+}
diff --git a/internal/jail/jail_test.go b/internal/jail/jail_test.go
index 75150da3..bf374a24 100644
--- a/internal/jail/jail_test.go
+++ b/internal/jail/jail_test.go
@@ -296,3 +296,47 @@ func TestJailIntoCleansNestedDirs(t *testing.T) {
 	_, err := os.Stat(jailPath)
 	require.NoError(t, err, "/ in jail (corresponding to external directory) was removed")
 }
+
+func TestJailIntoMkDirFails(t *testing.T) {
+	jailPath := tmpJailPath()
+	require.NoError(t, os.MkdirAll(jailPath, 0755))
+	defer os.RemoveAll(jailPath)
+
+	pagesRoot := "/pages/sub/path"
+
+	chroot := jail.Into(jailPath)
+	chroot.MkDir(pagesRoot, 0755)
+
+	err := chroot.Build()
+
+	require.Error(t, err, "err")
+	require.Contains(t, err.Error(), "no such file or directory")
+
+	_, err = os.Stat(path.Join(jailPath, pagesRoot))
+	require.True(t, os.IsNotExist(err), "%s in jail was not removed", pagesRoot)
+}
+
+func TestJailIntoMkDirAll(t *testing.T) {
+	jailPath := tmpJailPath()
+	require.NoError(t, os.MkdirAll(jailPath, 0755))
+	defer os.RemoveAll(jailPath)
+
+	chroot := jail.Into(jailPath)
+
+	chroot.MkDirAll("/way/down/here", 0755)
+
+	require.NoError(t, chroot.Build())
+	require.NoError(t, chroot.Dispose())
+
+	verify := func(inPath string) {
+		_, err := os.Stat(path.Join(jailPath, inPath))
+		require.True(t, os.IsNotExist(err), "{} in jail was not removed", inPath)
+	}
+
+	verify("/way")
+	verify("/way/down")
+	verify("/way/down/here")
+
+	_, err := os.Stat(jailPath)
+	require.NoError(t, err, "/ in jail (corresponding to external directory) was not removed")
+}
diff --git a/main.go b/main.go
index 04afaccf..ec822143 100644
--- a/main.go
+++ b/main.go
@@ -343,7 +343,7 @@ func appMain() {
 	}
 
 	if *daemonUID != 0 || *daemonGID != 0 {
-		if err := daemonize(config, *daemonUID, *daemonGID, *daemonInplaceChroot); err != nil {
+		if err := daemonize(config, *daemonUID, *daemonGID, *daemonInplaceChroot, *pagesRoot); err != nil {
 			errortracking.Capture(err)
 			fatal(err, "could not create pages daemon")
 		}