diff options
-rw-r--r-- | internal/config/config.go | 11 | ||||
-rw-r--r-- | internal/httpfs/http_fs.go | 66 | ||||
-rw-r--r-- | internal/httpfs/http_fs_test.go | 19 | ||||
-rw-r--r-- | internal/vfs/zip/vfs.go | 2 | ||||
-rw-r--r-- | test/acceptance/zip_test.go | 2 |
5 files changed, 88 insertions, 12 deletions
diff --git a/internal/config/config.go b/internal/config/config.go index 0842a195..528b586a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -146,6 +146,10 @@ type ZipServing struct { RefreshInterval time.Duration OpenTimeout time.Duration AllowedPaths []string + // TODO: this is a temporary workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/326117#note_546346101 + // where daemon-inplace-chroot=true fails to serve zip archives when pages_serve_with_zip_file_protocol is enabled + // To be removed along with chroot support https://gitlab.com/gitlab-org/gitlab-pages/-/issues/561 + ChrootPath string } func gitlabServerFromFlags() string { @@ -329,6 +333,13 @@ func loadConfig() *Config { setGitLabAPISecretKey(*gitLabAPISecretKey, config) } + // TODO: this is a temporary workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/326117#note_546346101 + // where daemon-inplace-chroot=true fails to serve zip archives when pages_serve_with_zip_file_protocol is enabled + // To be removed along with chroot support https://gitlab.com/gitlab-org/gitlab-pages/-/issues/561 + if config.Daemon.InplaceChroot { + config.Zip.ChrootPath = *pagesRoot + } + validateConfig(config) return config diff --git a/internal/httpfs/http_fs.go b/internal/httpfs/http_fs.go index cd2edb83..3578352f 100644 --- a/internal/httpfs/http_fs.go +++ b/internal/httpfs/http_fs.go @@ -8,6 +8,7 @@ package httpfs import ( "errors" + "fmt" "net/http" "os" "path" @@ -24,15 +25,28 @@ var ( // fileSystemPaths implements the http.FileSystem interface type fileSystemPaths struct { allowedPaths []string + // workaround for https://gitlab.com/gitlab-org/gitlab/-/issues/326117#note_546346101 + // where daemon-inplace-chroot=true fails to serve zip archives when + // pages_serve_with_zip_file_protocol is enabled + // TODO: evaluate if we need to remove this field when we remove + // chroot https://gitlab.com/gitlab-org/gitlab-pages/-/issues/561 + chrootPath string } // NewFileSystemPath creates a new fileSystemPaths that can be used to register -// a file:// protocol with an http.Transport -func NewFileSystemPath(allowedPaths []string) (http.FileSystem, error) { - for k, path := range allowedPaths { - var err error +// a file:// protocol with an http.Transport. +// When the daemon runs inside a chroot we need to strip chrootPath out of each +// of the allowedPaths so that we are able to find the file correctly inside +// the chroot. When Open is called, the same chrootPath will be stripped out of +// the full filepath. +func NewFileSystemPath(allowedPaths []string, chrootPath string) (http.FileSystem, error) { + for k, allowedPath := range allowedPaths { + strippedPath, err := stripChrootPath(ensureEndingSlash(allowedPath), chrootPath) + if err != nil { + return nil, err + } - allowedPaths[k], err = filepath.Abs(path) + allowedPaths[k], err = filepath.Abs(strippedPath) if err != nil { return nil, err } @@ -40,6 +54,7 @@ func NewFileSystemPath(allowedPaths []string) (http.FileSystem, error) { return &fileSystemPaths{ allowedPaths: allowedPaths, + chrootPath: chrootPath, }, nil } @@ -50,12 +65,26 @@ func (p *fileSystemPaths) Open(name string) (http.File, error) { return nil, errInvalidChar } - absPath, err := filepath.Abs(filepath.FromSlash(path.Clean("/" + name))) + cleanedPath := filepath.FromSlash(path.Clean("/" + name)) + + // since deamon can run in a chroot, we allow to define a chroot path that will be stripped from + // the FS location + // TODO: evaluate if we need to remove this check when we remove chroot + // https://gitlab.com/gitlab-org/gitlab-pages/-/issues/561 + strippedPath, err := stripChrootPath(cleanedPath, p.chrootPath) + if err != nil { + log.WithError(err).Error(os.ErrPermission) + + return nil, os.ErrPermission + } + + absPath, err := filepath.Abs(strippedPath) if err != nil { return nil, err } for _, allowedPath := range p.allowedPaths { - if strings.HasPrefix(absPath, allowedPath+"/") { + // allowedPath may be a single / in chroot so we need to ensure it's not double slash + if strings.HasPrefix(absPath, ensureEndingSlash(allowedPath)) { return os.Open(absPath) } } @@ -67,3 +96,26 @@ func (p *fileSystemPaths) Open(name string) (http.File, error) { // https://github.com/golang/go/blob/release-branch.go1.15/src/net/http/fs.go#L635 return nil, os.ErrPermission } + +func ensureEndingSlash(path string) string { + if strings.HasSuffix(path, "/") { + return path + } + + return path + "/" +} + +func stripChrootPath(path, chrootPath string) (string, error) { + if chrootPath == "" { + return path, nil + } + + if !strings.HasPrefix(path, chrootPath+"/") { + return "", fmt.Errorf("allowed path %q is not in chroot path %q", path, chrootPath) + } + + // path will contain a leading `/` + path = path[len(chrootPath):] + + return path, nil +} diff --git a/internal/httpfs/http_fs_test.go b/internal/httpfs/http_fs_test.go index 3d1092b6..7885dbf2 100644 --- a/internal/httpfs/http_fs_test.go +++ b/internal/httpfs/http_fs_test.go @@ -19,6 +19,7 @@ func TestFSOpen(t *testing.T) { tests := map[string]struct { allowedPaths []string + chrootPath string fileName string expectedContent string expectedErrMsg string @@ -58,11 +59,17 @@ func TestFSOpen(t *testing.T) { fileName: wd + "/../httpfs/testdata/file1.txt", expectedErrMsg: os.ErrPermission.Error(), }, + "chroot_path_not_found_when_not_in_real_chroot": { + allowedPaths: []string{wd + "/testdata"}, + fileName: wd + "/testdata/file1.txt", + expectedErrMsg: "no such file or directory", + chrootPath: wd + "/testdata", + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - p, err := NewFileSystemPath(test.allowedPaths) + p, err := NewFileSystemPath(test.allowedPaths, test.chrootPath) require.NoError(t, err) got, err := p.Open(test.fileName) @@ -88,6 +95,7 @@ func TestFileSystemPathCanServeHTTP(t *testing.T) { tests := map[string]struct { path string + chrootPath string fileName string escapeURL bool expectedStatusCode int @@ -136,12 +144,19 @@ func TestFileSystemPathCanServeHTTP(t *testing.T) { expectedStatusCode: http.StatusForbidden, expectedContent: "403 Forbidden\n", }, + "chroot_path_fails_in_unit_test_not_found_when_not_in_real_chroot": { + path: wd + "/testdata", + fileName: "file1.txt", + chrootPath: wd + "/testdata", + expectedStatusCode: http.StatusNotFound, + expectedContent: "404 page not found\n", + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { transport := httptransport.NewTransport() - fs, err := NewFileSystemPath([]string{test.path}) + fs, err := NewFileSystemPath([]string{test.path}, test.chrootPath) require.NoError(t, err) transport.RegisterProtocol("file", http.NewFileTransport(fs)) diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go index b69522f9..9c09384f 100644 --- a/internal/vfs/zip/vfs.go +++ b/internal/vfs/zip/vfs.go @@ -104,7 +104,7 @@ func (fs *zipVFS) Reconfigure(cfg *config.Config) error { } func (fs *zipVFS) reconfigureTransport(cfg *config.Config) error { - fsTransport, err := httpfs.NewFileSystemPath(cfg.Zip.AllowedPaths) + fsTransport, err := httpfs.NewFileSystemPath(cfg.Zip.AllowedPaths, cfg.Zip.ChrootPath) if err != nil { return err } diff --git a/test/acceptance/zip_test.go b/test/acceptance/zip_test.go index a7e82d27..91d5df99 100644 --- a/test/acceptance/zip_test.go +++ b/test/acceptance/zip_test.go @@ -107,8 +107,6 @@ func TestZipServing(t *testing.T) { } func TestZipServingFromDisk(t *testing.T) { - skipUnlessEnabled(t, "not-inplace-chroot") - chdir := false defer testhelpers.ChdirInPath(t, "../../shared/pages", &chdir)() |